diff --git a/.vac/boringssl-upstream.json b/.vac/boringssl-upstream.json index 5e8ac6942c..72a2ada078 100644 --- a/.vac/boringssl-upstream.json +++ b/.vac/boringssl-upstream.json @@ -5,9 +5,9 @@ "ref": "refs/heads/main", "mirror_url": "https://github.com/google/boringssl.git" }, - "previous_upstream_sha": "05fb4bcd239ec7bd7db0d606dcf131e62e24509e", - "current_upstream_sha": "beddb582d9e8786a07d79f1cf054d4792b3dd81f", - "last_synced_at": "2026-05-18T07:46:08Z", + "previous_upstream_sha": "beddb582d9e8786a07d79f1cf054d4792b3dd81f", + "current_upstream_sha": "956bac6e4db9b33789dcedb5ea3f28e51030cead", + "last_synced_at": "2026-05-25T07:56:06Z", "local_patch": { "description": "Use portable C code for fiat_p256 mul/sqr on Windows by removing ADX assembly dispatch.", "commit": "d6238994c547f070c1e053aada216ee3ce67e8e0", @@ -15,7 +15,7 @@ "sha256": "97f9d35fe2ac3e6c9d488558faaa40a36135394e595926391e9224c6599ccd19" }, "last_verification": { - "github_mirror_sha": "beddb582d9e8786a07d79f1cf054d4792b3dd81f", + "github_mirror_sha": "956bac6e4db9b33789dcedb5ea3f28e51030cead", "patch_replayed": true, "fiat_p256_adx_dispatch_absent": true }, @@ -39,6 +39,13 @@ "new_upstream_sha": "beddb582d9e8786a07d79f1cf054d4792b3dd81f", "github_mirror_sha": "beddb582d9e8786a07d79f1cf054d4792b3dd81f", "local_patch_sha256": "97f9d35fe2ac3e6c9d488558faaa40a36135394e595926391e9224c6599ccd19" + }, + { + "synced_at": "2026-05-25T07:56:06Z", + "previous_upstream_sha": "beddb582d9e8786a07d79f1cf054d4792b3dd81f", + "new_upstream_sha": "956bac6e4db9b33789dcedb5ea3f28e51030cead", + "github_mirror_sha": "956bac6e4db9b33789dcedb5ea3f28e51030cead", + "local_patch_sha256": "97f9d35fe2ac3e6c9d488558faaa40a36135394e595926391e9224c6599ccd19" } ] } diff --git a/crypto/bio/bio.cc b/crypto/bio/bio.cc index a3be772845..9250be0fbd 100644 --- a/crypto/bio/bio.cc +++ b/crypto/bio/bio.cc @@ -421,7 +421,7 @@ static int bio_read_all(Bio *bio, uint8_t **out, size_t *out_len, if (n == 0) { *out_len = done; return 1; - } else if (n == -1) { + } else if (n < 0) { OPENSSL_free(*out); return 0; } diff --git a/crypto/bio/bio_test.cc b/crypto/bio/bio_test.cc index 5c9317486f..861eb2cfb5 100644 --- a/crypto/bio/bio_test.cc +++ b/crypto/bio/bio_test.cc @@ -513,6 +513,46 @@ TEST(BIOTest, ReadASN1) { } } +TEST(BIOTest, ReadASN1ErrorNegative) { + // A custom BIO whose bread callback returns a negative value other than -1. + BIO_METHOD *meth = BIO_meth_new(BIO_TYPE_SOURCE_SINK, "evil"); + ASSERT_TRUE(meth); + BIO_meth_set_read(meth, [](BIO *bio, char *buf, int len) -> int { + int *call_count = reinterpret_cast(BIO_get_data(bio)); + (*call_count)++; + if (*call_count == 1) { + if (len < 2) { + return -1; + } + buf[0] = '\x30'; + buf[1] = '\x80'; + return 2; + } + return -2; + }); + BIO_meth_set_ctrl( + meth, [](BIO *bio, int cmd, long larg, void *parg) -> long { return 1; }); + + UniquePtr bio(BIO_new(meth)); + ASSERT_TRUE(bio); + + int call_count = 0; + BIO_set_data(bio.get(), &call_count); + BIO_set_init(bio.get(), 1); + + uint8_t *out = nullptr; + size_t out_len = 0; + int ok = BIO_read_asn1(bio.get(), &out, &out_len, 1000); + EXPECT_EQ(ok, 0); + if (ok == 1) { + OPENSSL_free(out); + } + + bio.reset(); + BIO_meth_free(meth); +} + + TEST(BIOTest, MemReadOnly) { // A memory BIO created from |BIO_new_mem_buf| is a read-only buffer. static const char kData[] = "abcdefghijklmno"; diff --git a/crypto/bytestring/bytestring_test.cc b/crypto/bytestring/bytestring_test.cc index 0e36a2e3c4..e0c200d776 100644 --- a/crypto/bytestring/bytestring_test.cc +++ b/crypto/bytestring/bytestring_test.cc @@ -59,7 +59,8 @@ TEST(CBSTest, Skip) { TEST(CBSTest, GetUint) { static const uint8_t kData[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, - 11, 12, 13, 14, 15, 16, 17, 18, 19, 20}; + 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, + 21, 22, 23, 24, 25, 26}; uint8_t u8; uint16_t u16; uint32_t u32; @@ -75,12 +76,14 @@ TEST(CBSTest, GetUint) { EXPECT_EQ(0x40506u, u32); ASSERT_TRUE(CBS_get_u32(&data, &u32)); EXPECT_EQ(0x708090au, u32); + ASSERT_TRUE(CBS_get_u48(&data, &u64)); + EXPECT_EQ(0xb0c0d0e0f10u, u64); ASSERT_TRUE(CBS_get_u64(&data, &u64)); - EXPECT_EQ(0xb0c0d0e0f101112u, u64); + EXPECT_EQ(0x1112131415161718u, u64); ASSERT_TRUE(CBS_get_last_u8(&data, &u8)); - EXPECT_EQ(0x14u, u8); + EXPECT_EQ(0x1au, u8); ASSERT_TRUE(CBS_get_last_u8(&data, &u8)); - EXPECT_EQ(0x13u, u8); + EXPECT_EQ(0x19u, u8); EXPECT_FALSE(CBS_get_u8(&data, &u8)); EXPECT_FALSE(CBS_get_last_u8(&data, &u8)); diff --git a/crypto/bytestring/cbs.cc b/crypto/bytestring/cbs.cc index dbaa3e8af5..f9011e3ae0 100644 --- a/crypto/bytestring/cbs.cc +++ b/crypto/bytestring/cbs.cc @@ -146,6 +146,8 @@ int CBS_get_u32le(CBS *cbs, uint32_t *out) { return 1; } +int CBS_get_u48(CBS *cbs, uint64_t *out) { return cbs_get_u(cbs, out, 6); } + int CBS_get_u64(CBS *cbs, uint64_t *out) { return cbs_get_u(cbs, out, 8); } int CBS_get_u64le(CBS *cbs, uint64_t *out) { diff --git a/crypto/fipsmodule/rsa/rsa_impl.cc.inc b/crypto/fipsmodule/rsa/rsa_impl.cc.inc index f745eb28e7..ba08ae772d 100644 --- a/crypto/fipsmodule/rsa/rsa_impl.cc.inc +++ b/crypto/fipsmodule/rsa/rsa_impl.cc.inc @@ -752,6 +752,23 @@ static int rsa_generate_key_impl(RSAImpl *rsa, int bits, const BIGNUM *e_value, return 0; } + // The smallest reasonable RSA exponent is 3, and it definitely must be odd. + // Catching these here prevents endless loops or slow computation when trying + // to generate keys later, and results in a better error code. + if ( + // Would fail in |bn_lcm_consttime| as it only allows positive integers. + BN_is_negative(e_value) || + // Would fail in |generate_prime| as only one |rsa->p|-1 is coprime with + // an even |e_value| and that one is a little bit short. (The R in RSA + // doesn't stand for Rabin.) + !BN_is_odd(e_value) || + // Would loop endlessly because it'll always compute an |rsa->d| exponent + // of 1, which is too small. + BN_is_one(e_value)) { + OPENSSL_PUT_ERROR(RSA, RSA_R_BAD_E_VALUE); + return 0; + } + UniquePtr ctx(BN_CTX_new()); if (ctx == nullptr) { OPENSSL_PUT_ERROR(RSA, ERR_LIB_BN); @@ -800,8 +817,8 @@ static int rsa_generate_key_impl(RSAImpl *rsa, int bits, const BIGNUM *e_value, if (!generate_prime(rsa->p.get(), prime_bits, rsa->e.get(), nullptr, pow2_prime_bits_100, ctx.get(), cb) || !BN_GENCB_call(cb, 3, 0) || - !generate_prime(rsa->q.get(), prime_bits, rsa->e.get(), rsa->p.get(), pow2_prime_bits_100, - ctx.get(), cb) || + !generate_prime(rsa->q.get(), prime_bits, rsa->e.get(), rsa->p.get(), + pow2_prime_bits_100, ctx.get(), cb) || !BN_GENCB_call(cb, 3, 1)) { OPENSSL_PUT_ERROR(RSA, ERR_LIB_BN); return 0; diff --git a/crypto/rsa/rsa_test.cc b/crypto/rsa/rsa_test.cc index ad6f644874..a7052b36e8 100644 --- a/crypto/rsa/rsa_test.cc +++ b/crypto/rsa/rsa_test.cc @@ -1019,6 +1019,51 @@ TEST(RSATest, CheckKey) { ERR_clear_error(); } +TEST(RSATest, KeygenBadExponent) { + UniquePtr rsa_opaque(RSA_new()); + RSAImpl *rsa = FromOpaque(rsa_opaque.get()); + ASSERT_TRUE(rsa); + + UniquePtr e(BN_new()); + ASSERT_TRUE(e); + + // 3 is the smallest allowed public key value. + ASSERT_TRUE(BN_set_word(e.get(), 3)); + ASSERT_TRUE(RSA_generate_key_ex(rsa, 2048, e.get(), nullptr)); + + // Maybe I prefer the Rabin scheme. But this is an RSA API! + ASSERT_TRUE(BN_set_word(e.get(), 2)); + EXPECT_FALSE(RSA_generate_key_ex(rsa, 2048, e.get(), nullptr)); + EXPECT_TRUE(ErrorEquals(ERR_get_error(), ERR_LIB_RSA, RSA_R_BAD_E_VALUE)); + + // Rabin-Shamir-Adleman? Nope. + ASSERT_TRUE(BN_set_word(e.get(), 6)); + EXPECT_FALSE(RSA_generate_key_ex(rsa, 2048, e.get(), nullptr)); + EXPECT_TRUE(ErrorEquals(ERR_get_error(), ERR_LIB_RSA, RSA_R_BAD_E_VALUE)); + + // RSA with exponent 1 is a joke. But we already use ROT26 for that purpose. + ASSERT_TRUE(BN_set_word(e.get(), 1)); + EXPECT_FALSE(RSA_generate_key_ex(rsa, 2048, e.get(), nullptr)); + EXPECT_TRUE(ErrorEquals(ERR_get_error(), ERR_LIB_RSA, RSA_R_BAD_E_VALUE)); + + // RSA with exponent 0 is also known as /dev/null, and not supported here. + ASSERT_TRUE(BN_set_word(e.get(), 0)); + EXPECT_FALSE(RSA_generate_key_ex(rsa, 2048, e.get(), nullptr)); + EXPECT_TRUE(ErrorEquals(ERR_get_error(), ERR_LIB_RSA, RSA_R_BAD_E_VALUE)); + + // Now this is just silly - perfectly fine RSA with e=3, except with an extra + // inversion that cryptographically does nothing at all except waste cycles. + // Throw it away. + ASSERT_TRUE(BN_set_word(e.get(), 3)); + BN_set_negative(e.get(), 1); + EXPECT_FALSE(RSA_generate_key_ex(rsa, 2048, e.get(), nullptr)); + EXPECT_TRUE(ErrorEquals(ERR_get_error(), ERR_LIB_RSA, RSA_R_BAD_E_VALUE)); + + // To validate nothing got corrupted, try good old 65537. + ASSERT_TRUE(BN_set_word(e.get(), RSA_F4)); + EXPECT_TRUE(RSA_generate_key_ex(rsa, 2048, e.get(), nullptr)); +} + TEST(RSATest, KeygenFail) { UniquePtr rsa_opaque(RSA_new()); RSAImpl *rsa = FromOpaque(rsa_opaque.get()); diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc index d3ab671228..cc79b09016 100644 --- a/crypto/x509/x509_test.cc +++ b/crypto/x509/x509_test.cc @@ -8434,6 +8434,36 @@ TEST(X509Test, ParamInheritance) { // The new value is used. EXPECT_EQ(X509_VERIFY_PARAM_get_depth(dest.get()), 10); } + + // |X509_VERIFY_PARAM_inherit| and |X509_VERIFY_PARAM_set1| must fail if the + // source parameter is poisoned. + { + UniquePtr dest(X509_VERIFY_PARAM_new()); + ASSERT_TRUE(dest); + UniquePtr src(X509_VERIFY_PARAM_new()); + ASSERT_TRUE(src); + + // Poison the source parameter (using an embedded NUL in hostname). + ASSERT_FALSE(X509_VERIFY_PARAM_set1_host(src.get(), "a", 2)); + + EXPECT_FALSE(X509_VERIFY_PARAM_inherit(dest.get(), src.get())); + EXPECT_FALSE(X509_VERIFY_PARAM_set1(dest.get(), src.get())); + } + + // |X509_VERIFY_PARAM_inherit| and |X509_VERIFY_PARAM_set1| must fail if the + // destination parameter is poisoned. + { + UniquePtr dest(X509_VERIFY_PARAM_new()); + ASSERT_TRUE(dest); + UniquePtr src(X509_VERIFY_PARAM_new()); + ASSERT_TRUE(src); + + // Poison the destination parameter (using an embedded NUL in hostname). + ASSERT_FALSE(X509_VERIFY_PARAM_set1_host(dest.get(), "a", 2)); + + EXPECT_FALSE(X509_VERIFY_PARAM_inherit(dest.get(), src.get())); + EXPECT_FALSE(X509_VERIFY_PARAM_set1(dest.get(), src.get())); + } } TEST(X509Test, PublicKeyCache) { diff --git a/crypto/x509/x509_vpm.cc b/crypto/x509/x509_vpm.cc index a0f8a71723..7507981441 100644 --- a/crypto/x509/x509_vpm.cc +++ b/crypto/x509/x509_vpm.cc @@ -117,16 +117,21 @@ static void copy_int_param(T *dest, const T *src, T default_val, } } -// x509_verify_param_copy copies fields from |src| to |dest|. If both |src| and +// x509_verify_param_merge merges fields from |src| to |dest|. If both |src| and // |dest| have some field set, |prefer_src| determines whether |src| or |dest|'s // version is used. -static int x509_verify_param_copy(X509_VERIFY_PARAM *dest, - const X509_VERIFY_PARAM *src, - bool prefer_src) { +static int x509_verify_param_merge(X509_VERIFY_PARAM *dest, + const X509_VERIFY_PARAM *src, + bool prefer_src) { if (src == nullptr) { return 1; } + if (src->poison || dest->poison) { + OPENSSL_PUT_ERROR(X509, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); + return 0; + } + copy_int_param(&dest->purpose, &src->purpose, /*default_val=*/0, prefer_src); copy_int_param(&dest->trust, &src->trust, /*default_val=*/0, prefer_src); copy_int_param(&dest->depth, &src->depth, /*default_val=*/-1, prefer_src); @@ -175,7 +180,6 @@ static int x509_verify_param_copy(X509_VERIFY_PARAM *dest, } } - dest->poison = src->poison; return 1; } @@ -183,14 +187,14 @@ int X509_VERIFY_PARAM_inherit(X509_VERIFY_PARAM *dest, const X509_VERIFY_PARAM *src) { // Prefer the destination. That is, this function only changes unset // parameters in |dest|. - return x509_verify_param_copy(dest, src, /*prefer_src=*/false); + return x509_verify_param_merge(dest, src, /*prefer_src=*/false); } int X509_VERIFY_PARAM_set1(X509_VERIFY_PARAM *to, const X509_VERIFY_PARAM *from) { // Prefer the source. That is, values in |to| are only preserved if they were // unset in |from|. - return x509_verify_param_copy(to, from, /*prefer_src=*/true); + return x509_verify_param_merge(to, from, /*prefer_src=*/true); } static int int_x509_param_set1(char **pdest, size_t *pdestlen, const char *src, diff --git a/include/openssl/base.h b/include/openssl/base.h index 961e542a04..1ce0e1bb22 100644 --- a/include/openssl/base.h +++ b/include/openssl/base.h @@ -73,7 +73,7 @@ extern "C" { // A consumer may use this symbol in the preprocessor to temporarily build // against multiple revisions of BoringSSL at the same time. It is not // recommended to do so for longer than is necessary. -#define BORINGSSL_API_VERSION 40 +#define BORINGSSL_API_VERSION 41 #if defined(BORINGSSL_SHARED_LIBRARY) diff --git a/include/openssl/bytestring.h b/include/openssl/bytestring.h index 3e1cfe5d27..50263cb3d9 100644 --- a/include/openssl/bytestring.h +++ b/include/openssl/bytestring.h @@ -121,6 +121,10 @@ OPENSSL_EXPORT int CBS_get_u32(CBS *cbs, uint32_t *out); // |cbs| and advances |cbs|. It returns one on success and zero on error. OPENSSL_EXPORT int CBS_get_u32le(CBS *cbs, uint32_t *out); +// CBS_get_u48 sets |*out| to the next, big-endian 48-bit value from |cbs| and +// advances |cbs|. It returns one on success and zero on error. +OPENSSL_EXPORT int CBS_get_u48(CBS *cbs, uint64_t *out); + // CBS_get_u64 sets |*out| to the next, big-endian uint64_t value from |cbs| // and advances |cbs|. It returns one on success and zero on error. OPENSSL_EXPORT int CBS_get_u64(CBS *cbs, uint64_t *out); diff --git a/include/openssl/prefix_symbols.h b/include/openssl/prefix_symbols.h index c25242da86..4d970a3f2a 100644 --- a/include/openssl/prefix_symbols.h +++ b/include/openssl/prefix_symbols.h @@ -568,6 +568,7 @@ #pragma redefine_extname CBS_get_u24_length_prefixed BORINGSSL_ADD_USER_LABEL_AND_PREFIX(CBS_get_u24_length_prefixed) #pragma redefine_extname CBS_get_u32 BORINGSSL_ADD_USER_LABEL_AND_PREFIX(CBS_get_u32) #pragma redefine_extname CBS_get_u32le BORINGSSL_ADD_USER_LABEL_AND_PREFIX(CBS_get_u32le) +#pragma redefine_extname CBS_get_u48 BORINGSSL_ADD_USER_LABEL_AND_PREFIX(CBS_get_u48) #pragma redefine_extname CBS_get_u64 BORINGSSL_ADD_USER_LABEL_AND_PREFIX(CBS_get_u64) #pragma redefine_extname CBS_get_u64_decimal BORINGSSL_ADD_USER_LABEL_AND_PREFIX(CBS_get_u64_decimal) #pragma redefine_extname CBS_get_u64le BORINGSSL_ADD_USER_LABEL_AND_PREFIX(CBS_get_u64le) @@ -3682,6 +3683,7 @@ #define CBS_get_u24_length_prefixed BORINGSSL_ADD_PREFIX(CBS_get_u24_length_prefixed) #define CBS_get_u32 BORINGSSL_ADD_PREFIX(CBS_get_u32) #define CBS_get_u32le BORINGSSL_ADD_PREFIX(CBS_get_u32le) +#define CBS_get_u48 BORINGSSL_ADD_PREFIX(CBS_get_u48) #define CBS_get_u64 BORINGSSL_ADD_PREFIX(CBS_get_u64) #define CBS_get_u64_decimal BORINGSSL_ADD_PREFIX(CBS_get_u64_decimal) #define CBS_get_u64le BORINGSSL_ADD_PREFIX(CBS_get_u64le) diff --git a/ssl/d1_both.cc b/ssl/d1_both.cc index c98dae49e9..7b7a1184fb 100644 --- a/ssl/d1_both.cc +++ b/ssl/d1_both.cc @@ -318,7 +318,7 @@ bool dtls1_process_handshake_fragments(SSL *ssl, uint8_t *out_alert, implicit_ack = true; } - if (msg_hdr.seq - ssl->d1->handshake_read_seq > SSL_MAX_HANDSHAKE_FLIGHT) { + if (msg_hdr.seq - ssl->d1->handshake_read_seq >= SSL_MAX_HANDSHAKE_FLIGHT) { // Ignore fragments too far in the future. skipped_fragments = true; continue; diff --git a/ssl/ssl_privkey.cc b/ssl/ssl_privkey.cc index 29475296af..b48becdd10 100644 --- a/ssl/ssl_privkey.cc +++ b/ssl/ssl_privkey.cc @@ -261,8 +261,16 @@ enum ssl_private_key_result_t ssl_private_key_sign( assert(!hs->can_release_private_key); if (key_method != nullptr) { + if (key_method->sign == nullptr) { + OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); + return ssl_private_key_failure; + } enum ssl_private_key_result_t ret; if (hs->pending_private_key_op) { + if (key_method->complete == nullptr) { + OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); + return ssl_private_key_failure; + } ret = key_method->complete(ssl, out, out_len, max_out); } else { ret = key_method->sign(ssl, out, out_len, max_out, sigalg, in.data(), @@ -321,8 +329,16 @@ enum ssl_private_key_result_t ssl_private_key_decrypt(SSL_HANDSHAKE *hs, const SSLCredential *const cred = hs->credential.get(); assert(!hs->can_release_private_key); if (cred->key_method != nullptr) { + if (cred->key_method->decrypt == nullptr) { + OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); + return ssl_private_key_failure; + } enum ssl_private_key_result_t ret; if (hs->pending_private_key_op) { + if (cred->key_method->complete == nullptr) { + OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); + return ssl_private_key_failure; + } ret = cred->key_method->complete(ssl, out, out_len, max_out); } else { ret = cred->key_method->decrypt(ssl, out, out_len, max_out, in.data(), diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index a876718a78..a91a6268ca 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc @@ -5504,6 +5504,57 @@ TEST(SSLTest, OverrideKeyMethodWithKey) { ASSERT_TRUE(ConnectClientAndServer(&client, &server, ctx.get(), ctx.get())); } +TEST(SSLTest, NullDecryptPrivateKeyMethod) { + bssl::UniquePtr key = GetTestKey(); + ASSERT_TRUE(key); + bssl::UniquePtr leaf = GetTestCertificate(); + ASSERT_TRUE(leaf); + + bssl::UniquePtr client_ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(client_ctx); + bssl::UniquePtr server_ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(server_ctx); + + ASSERT_TRUE(SSL_CTX_use_certificate(server_ctx.get(), leaf.get())); + + // Configure an SSL_PRIVATE_KEY_METHOD on the server with sign and complete, + // but NULL decrypt. + static const SSL_PRIVATE_KEY_METHOD kNullDecryptMethod = { + [](SSL *ssl, uint8_t *out, size_t *out_len, size_t max_out, + uint16_t signature_algorithm, const uint8_t *in, + size_t in_len) { return ssl_private_key_failure; }, + nullptr, // decrypt + [](SSL *ssl, uint8_t *out, size_t *out_len, size_t max_out) { + return ssl_private_key_failure; + }, + }; + + SSL_CTX_set_private_key_method(server_ctx.get(), &kNullDecryptMethod); + + // Negotiate RSA key exchange (SSL_kRSA). We do this by restricting both + // client and server to TLS 1.2 and configuring an RSA key exchange cipher + // suite. + ASSERT_TRUE(SSL_CTX_set_max_proto_version(client_ctx.get(), TLS1_2_VERSION)); + ASSERT_TRUE(SSL_CTX_set_max_proto_version(server_ctx.get(), TLS1_2_VERSION)); + + ASSERT_TRUE(SSL_CTX_set_cipher_list(client_ctx.get(), "AES128-GCM-SHA256")); + ASSERT_TRUE(SSL_CTX_set_cipher_list(server_ctx.get(), "AES128-GCM-SHA256")); + + // Use a custom verify on client to accept the self-signed test cert. + SSL_CTX_set_custom_verify(client_ctx.get(), SSL_VERIFY_PEER, + AcceptAnyCertificate); + + bssl::UniquePtr client, server; + // Since the decrypt hook is NULL, the server's decryption during the RSA key + // exchange should fail cleanly. + EXPECT_FALSE(ConnectClientAndServer(&client, &server, client_ctx.get(), + server_ctx.get())); + uint32_t err = ERR_get_error(); + EXPECT_TRUE(ErrorEquals(err, ERR_LIB_SSL, ERR_R_INTERNAL_ERROR)); + EXPECT_EQ(0u, ERR_get_error()); +} + + // Configuring a chain and then overwriting it with a different chain should // clear the old one. TEST(SSLTest, OverrideChain) { diff --git a/ssl/test/runner/dtls_tests.go b/ssl/test/runner/dtls_tests.go index 45bbf6742c..215c8b5be2 100644 --- a/ssl/test/runner/dtls_tests.go +++ b/ssl/test/runner/dtls_tests.go @@ -1610,3 +1610,75 @@ func addDTLSReorderTests() { }) } } + +func addDTLSFragmentWindowTests() { + for _, vers := range allVersions(dtls) { + // A handshake fragment with a sequence number exactly at the upper bound + // of the sliding window (handshake_read_seq + SSL_MAX_HANDSHAKE_FLIGHT, + // which is +7) should be silently discarded rather than triggering a + // fatal internal error alert. + testCases = append(testCases, testCase{ + protocol: dtls, + testType: serverTest, + name: "DTLS-FragmentWindow-UpperLimit-Discard-Server-" + vers.name, + config: Config{ + MaxVersion: vers.version, + Bugs: ProtocolBugs{ + WriteFlightDTLS: func(c *DTLSController, prev, received, next []DTLSMessage, records []DTLSRecordNumberInfo) { + for _, msg := range next { + if msg.Sequence == 0 && !msg.IsChangeCipherSpec { + // Injected future fragment with offset +7. + // Since SSL_MAX_HANDSHAKE_FLIGHT is 7, sequence + 7 is at the upper limit. + // This must be silently discarded by the shim. + shouldDiscard := DTLSFragment{ + Epoch: msg.Epoch, + Type: msg.Type, + Sequence: msg.Sequence + 7, + Offset: 0, + TotalLength: len(msg.Data), + Data: msg.Data, + ShouldDiscard: true, + } + c.WriteFragments([]DTLSFragment{shouldDiscard, msg.Fragment(0, len(msg.Data))}) + } else { + c.WriteFragments([]DTLSFragment{msg.Fragment(0, len(msg.Data))}) + } + } + }, + }, + }, + }) + + testCases = append(testCases, testCase{ + protocol: dtls, + testType: clientTest, + name: "DTLS-FragmentWindow-UpperLimit-Discard-Client-" + vers.name, + config: Config{ + MaxVersion: vers.version, + Bugs: ProtocolBugs{ + PackHandshakeFragments: 4096, + WriteFlightDTLS: func(c *DTLSController, prev, received, next []DTLSMessage, records []DTLSRecordNumberInfo) { + for _, msg := range next { + if msg.Sequence == 0 && !msg.IsChangeCipherSpec { + // Injected future fragment with offset +7. + // This must be silently discarded by the shim. + shouldDiscard := DTLSFragment{ + Epoch: msg.Epoch, + Type: msg.Type, + Sequence: msg.Sequence + 7, + Offset: 0, + TotalLength: len(msg.Data), + Data: msg.Data, + ShouldDiscard: true, + } + c.WriteFragments([]DTLSFragment{shouldDiscard, msg.Fragment(0, len(msg.Data))}) + } else { + c.WriteFragments([]DTLSFragment{msg.Fragment(0, len(msg.Data))}) + } + } + }, + }, + }, + }) + } +} diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index 14c6c8d461..f92e3b1b46 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -2363,6 +2363,7 @@ func main() { addSignatureAlgorithmTests() addDTLSRetransmitTests() addDTLSReorderTests() + addDTLSFragmentWindowTests() addExportKeyingMaterialTests() addExportTrafficSecretsTests() addTLSUniqueTests()