ssl: Support ML-KEM by default. Update-Note: `SSL_GROUP_X25519_MLKEM768` requires quite much data in the ClientHello (see https://tldr.fail). Bug: 504987865 Change-Id: Ie4d8b4a4800ae006623bd816f831a1116a6a6964 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/94347 Commit-Queue: Rudolf Polzer <rpolzer@google.com> Reviewed-by: Xiangfei Ding <xfding@google.com>
diff --git a/ssl/ssl_key_share.cc b/ssl/ssl_key_share.cc index 5923110..841a6df 100644 --- a/ssl/ssl_key_share.cc +++ b/ssl/ssl_key_share.cc
@@ -455,6 +455,7 @@ Span<const uint16_t> DefaultSupportedGroupIds() { static const uint16_t kDefaultSupportedGroupIds[] = { + SSL_GROUP_X25519_MLKEM768, SSL_GROUP_X25519, SSL_GROUP_SECP256R1, SSL_GROUP_SECP384R1,
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index f9ba6a9..af516e7 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc
@@ -684,8 +684,8 @@ } TEST(SSLTest, DefaultCurves) { - const uint16_t kDefaults[] = {SSL_GROUP_X25519, SSL_GROUP_SECP256R1, - SSL_GROUP_SECP384R1}; + const uint16_t kDefaults[] = {SSL_GROUP_X25519_MLKEM768, SSL_GROUP_X25519, + SSL_GROUP_SECP256R1, SSL_GROUP_SECP384R1}; // Test the group ID APIs. { @@ -1666,6 +1666,13 @@ static size_t GetClientHelloLen(uint16_t max_version, uint16_t session_version, size_t ticket_len) { bssl::UniquePtr<SSL_CTX> ctx(SSL_CTX_new(TLS_method())); + + // Reduce the number of supported groups, as we need ClientHellos smaller + // than 254 bytes for SSLTest.Padding. + uint16_t groups[] = {SSL_GROUP_X25519, SSL_GROUP_SECP256R1, + SSL_GROUP_SECP384R1}; + SSL_CTX_set1_group_ids(ctx.get(), groups, sizeof(groups) / sizeof(*groups)); + bssl::UniquePtr<SSL_SESSION> session = CreateSessionWithTicket(session_version, ticket_len); if (!ctx || !session) { @@ -2340,7 +2347,8 @@ // Test that the SSL group flags are defaulted to zero when zero groups are set // (i.e. using the default groups). TEST(SSLTest, SetGroupIdsWithFlags_DefaultGroups) { - const uint16_t kDefaultGroups[] = {SSL_GROUP_X25519, SSL_GROUP_SECP256R1, + const uint16_t kDefaultGroups[] = {SSL_GROUP_X25519_MLKEM768, + SSL_GROUP_X25519, SSL_GROUP_SECP256R1, SSL_GROUP_SECP384R1}; const uint32_t kBogusFlags[] = {SSL_GROUP_FLAG_EQUAL_PREFERENCE_WITH_NEXT, SSL_GROUP_FLAG_EQUAL_PREFERENCE_WITH_NEXT, 0}; @@ -7209,8 +7217,8 @@ // The default list of groups is used before applying the handoff. EXPECT_THAT(FromOpaque(server.get())->config->supported_group_list, - ElementsAreArray({SSL_GROUP_X25519, SSL_GROUP_SECP256R1, - SSL_GROUP_SECP384R1})); + ElementsAreArray({SSL_GROUP_X25519_MLKEM768, SSL_GROUP_X25519, + SSL_GROUP_SECP256R1, SSL_GROUP_SECP384R1})); ASSERT_TRUE(SSL_apply_handoff(server.get(), handoff)); EXPECT_EQ(1u, FromOpaque(server.get())->config->supported_group_list.size()); EXPECT_EQ(SSL_GROUP_SECP256R1,
diff --git a/ssl/test/runner/basic_tests.go b/ssl/test/runner/basic_tests.go index 54d4963..46f44bc 100644 --- a/ssl/test/runner/basic_tests.go +++ b/ssl/test/runner/basic_tests.go
@@ -125,6 +125,11 @@ }, }, flags: []string{ + // Exact message trace of the handshake depends on selected curves, + // as MLKEM causes messages to be split due to its handshake size + // and DTLS trying to respect a MTU here. + // TODO(crbug.com/507830312): Eventually fix that? + "-curves", strconv.Itoa(int(CurveX25519)), "-enable-ocsp-stapling", // This test involves an optional message. Test the message callback // trace to ensure we do not miss or double-report any. @@ -1998,6 +2003,10 @@ MinVersion: VersionTLS13, }, flags: []string{ + // Exact message trace of the handshake depends on selected curves, + // as MLKEM causes messages to be split due to its handshake size + // and DTLS trying to respect a MTU here. + "-curves", strconv.Itoa(int(CurveX25519)), "-expect-msg-callback", `read hs 1 write hs 2
diff --git a/ssl/test/runner/curve_tests.go b/ssl/test/runner/curve_tests.go index 956a732..7ea5513 100644 --- a/ssl/test/runner/curve_tests.go +++ b/ssl/test/runner/curve_tests.go
@@ -582,14 +582,13 @@ }) } - // ML-KEM and Kyber should not be offered by default as a client. + // ML-KEM should be offered by default as a client. testCases = append(testCases, testCase{ - name: "PostQuantumNotEnabledByDefaultInClients", + name: "PostQuantumEnabledByDefaultInClients", config: Config{ - MinVersion: VersionTLS13, - Bugs: ProtocolBugs{ - FailIfPostQuantumOffered: true, - }, + MinVersion: VersionTLS13, + CurvePreferences: []CurveID{CurveX25519MLKEM768}, + Bugs: ProtocolBugs{}, }, }) @@ -682,18 +681,17 @@ }) } - // As a server, ML-KEMs and Kyber are not yet supported by default. testCases = append(testCases, testCase{ testType: serverTest, - name: "PostQuantumNotEnabledByDefaultForAServer", + name: "PostQuantumEnabledByDefaultForAServer", config: Config{ MinVersion: VersionTLS13, - CurvePreferences: []CurveID{CurveX25519MLKEM768, CurveMLKEM1024, CurveX25519Kyber768, CurveX25519}, - DefaultCurves: []CurveID{CurveX25519MLKEM768, CurveMLKEM1024, CurveX25519Kyber768}, + CurvePreferences: []CurveID{CurveX25519MLKEM768}, + DefaultCurves: []CurveID{CurveX25519MLKEM768}, }, flags: []string{ "-server-preference", - "-expect-curve-id", strconv.Itoa(int(CurveX25519)), + "-expect-curve-id", strconv.Itoa(int(CurveX25519MLKEM768)), }, })
diff --git a/ssl/test/runner/dtls.go b/ssl/test/runner/dtls.go index 1106aed..b425544 100644 --- a/ssl/test/runner/dtls.go +++ b/ssl/test/runner/dtls.go
@@ -271,7 +271,7 @@ } if c.maxPacketLen != 0 { if n > c.maxPacketLen { - return 0, nil, fmt.Errorf("dtls: exceeded maximum packet length") + return 0, nil, fmt.Errorf("dtls: exceeded maximum packet length: got %d, want <=%d", n, c.maxPacketLen) } c.bytesAvailableInPacket = c.maxPacketLen - n } else { @@ -288,7 +288,7 @@ } if n > maxCiphertext || c.rawInput.Len() < recordHeaderLen+n { c.sendAlert(alertRecordOverflow) - return 0, nil, c.in.setErrorLocked(fmt.Errorf("dtls: oversized record received with length %d", n)) + return 0, nil, c.in.setErrorLocked(fmt.Errorf("dtls: oversized record received with length: got %d, want <=%d", n, maxCiphertext)) } b := c.rawInput.Next(recordHeaderLen + n)
diff --git a/ssl/test/runner/ech_tests.go b/ssl/test/runner/ech_tests.go index 2cd3c10..22ee8ec 100644 --- a/ssl/test/runner/ech_tests.go +++ b/ssl/test/runner/ech_tests.go
@@ -484,6 +484,10 @@ "-ech-is-retry-config", "1", "-expect-ech-accept", "-expect-msg-callback", expectMsgCallback, + // Exact message trace of the handshake depends on selected curves, + // as MLKEM causes messages to be split due to its handshake size + // and DTLS trying to respect a MTU here. + "-curves", strconv.Itoa(int(CurveX25519)), }, expectations: connectionExpectations{ echAccepted: true, @@ -2383,6 +2387,10 @@ "-ech-config-list", base64FlagValue(CreateECHConfigList(echConfig.ECHConfig.Raw)), "-expect-ech-accept", "-expect-msg-callback", clientAndServerHelloInitial + finishHandshake, + // Exact message trace of the handshake depends on selected curves, + // as MLKEM causes messages to be split due to its handshake size + // and DTLS trying to respect a MTU here. + "-curves", strconv.Itoa(int(CurveX25519)), }, expectations: connectionExpectations{echAccepted: true}, }) @@ -2405,6 +2413,10 @@ "-expect-ech-accept", "-expect-hrr", // Check we triggered HRR. "-expect-msg-callback", clientAndServerHelloInitial + clientAndServerHello + finishHandshake, + // Exact message trace of the handshake depends on selected curves, + // as MLKEM causes messages to be split due to its handshake size + // and DTLS trying to respect a MTU here. + "-curves", strconv.Itoa(int(CurveX25519)), "-curves", strconv.Itoa(int(CurveP384)), }, expectations: connectionExpectations{echAccepted: true}, })
diff --git a/ssl/test/runner/extension_tests.go b/ssl/test/runner/extension_tests.go index ab9b0b0..19f5326 100644 --- a/ssl/test/runner/extension_tests.go +++ b/ssl/test/runner/extension_tests.go
@@ -16,6 +16,7 @@ import ( "fmt" + "strconv" ) func addExtensionTests() { @@ -1998,10 +1999,13 @@ RequireClientHelloSize: 512, }, }, - // This hostname just needs to be long enough to push the - // ClientHello into F5's danger zone between 256 and 511 bytes - // long. - flags: []string{"-host-name", "01234567890123456789012345678901234567890123456789012345678901234567890123456789.com"}, + flags: []string{ + // This hostname just needs to be long enough to push the + // ClientHello into F5's danger zone between 256 and 511 bytes long. + "-host-name", "01234567890123456789012345678901234567890123456789012345678901234567890123456789.com", + // Curve chosen to make handshakes short enough to end up in the danger zone. + "-curves", strconv.Itoa(int(CurveX25519)), + }, }) // Test that illegal extensions in TLS 1.3 are rejected by the client if
diff --git a/ssl/test/runner/key_update_tests.go b/ssl/test/runner/key_update_tests.go index f985282..39ad24b 100644 --- a/ssl/test/runner/key_update_tests.go +++ b/ssl/test/runner/key_update_tests.go
@@ -14,7 +14,9 @@ package runner -import "slices" +import ( + "slices" +) func addKeyUpdateTests() { // TLS tests. @@ -266,7 +268,6 @@ config: Config{ MaxVersion: VersionTLS13, Bugs: ProtocolBugs{ - MaxPacketLength: 512, ACKFlightDTLS: func(c *DTLSController, prev, received []DTLSMessage, records []DTLSRecordNumberInfo) { if received[0].Type != typeKeyUpdate { c.WriteACK(c.OutEpoch(), records)