diff --git a/src/crypto/ex_data.c b/src/crypto/ex_data.c index 867ced3c9..d34769f95 100644 --- a/src/crypto/ex_data.c +++ b/src/crypto/ex_data.c @@ -116,7 +116,6 @@ #include #include #include -#include #include #include "internal.h" @@ -128,14 +127,14 @@ struct crypto_ex_data_func_st { long argl; // Arbitary long void *argp; // Arbitary void pointer CRYPTO_EX_free *free_func; + // next points to the next |CRYPTO_EX_DATA_FUNCS| or NULL if this is the last + // one. It may only be read if synchronized with a read from |num_funcs|. + CRYPTO_EX_DATA_FUNCS *next; }; int CRYPTO_get_ex_new_index(CRYPTO_EX_DATA_CLASS *ex_data_class, int *out_index, long argl, void *argp, CRYPTO_EX_free *free_func) { - CRYPTO_EX_DATA_FUNCS *funcs; - int ret = 0; - - funcs = OPENSSL_malloc(sizeof(CRYPTO_EX_DATA_FUNCS)); + CRYPTO_EX_DATA_FUNCS *funcs = OPENSSL_malloc(sizeof(CRYPTO_EX_DATA_FUNCS)); if (funcs == NULL) { return 0; } @@ -143,37 +142,32 @@ int CRYPTO_get_ex_new_index(CRYPTO_EX_DATA_CLASS *ex_data_class, int *out_index, funcs->argl = argl; funcs->argp = argp; funcs->free_func = free_func; + funcs->next = NULL; CRYPTO_STATIC_MUTEX_lock_write(&ex_data_class->lock); - if (ex_data_class->meth == NULL) { - ex_data_class->meth = sk_CRYPTO_EX_DATA_FUNCS_new_null(); - } - - if (ex_data_class->meth == NULL) { - goto err; - } - + uint32_t num_funcs = CRYPTO_atomic_load_u32(&ex_data_class->num_funcs); // The index must fit in |int|. - if (sk_CRYPTO_EX_DATA_FUNCS_num(ex_data_class->meth) > - (size_t)(INT_MAX - ex_data_class->num_reserved)) { + if (num_funcs > (size_t)(INT_MAX - ex_data_class->num_reserved)) { OPENSSL_PUT_ERROR(CRYPTO, ERR_R_OVERFLOW); - goto err; + CRYPTO_STATIC_MUTEX_unlock_write(&ex_data_class->lock); + return 0; } - if (!sk_CRYPTO_EX_DATA_FUNCS_push(ex_data_class->meth, funcs)) { - goto err; + // Append |funcs| to the linked list. + if (ex_data_class->last == NULL) { + assert(num_funcs == 0); + ex_data_class->funcs = funcs; + ex_data_class->last = funcs; + } else { + ex_data_class->last->next = funcs; + ex_data_class->last = funcs; } - funcs = NULL; // |sk_CRYPTO_EX_DATA_FUNCS_push| takes ownership. - *out_index = (int)sk_CRYPTO_EX_DATA_FUNCS_num(ex_data_class->meth) - 1 + - ex_data_class->num_reserved; - ret = 1; - -err: + CRYPTO_atomic_store_u32(&ex_data_class->num_funcs, num_funcs + 1); CRYPTO_STATIC_MUTEX_unlock_write(&ex_data_class->lock); - OPENSSL_free(funcs); - return ret; + *out_index = (int)num_funcs + ex_data_class->num_reserved; + return 1; } int CRYPTO_set_ex_data(CRYPTO_EX_DATA *ad, int index, void *val) { @@ -209,33 +203,6 @@ void *CRYPTO_get_ex_data(const CRYPTO_EX_DATA *ad, int idx) { return sk_void_value(ad->sk, idx); } -// get_func_pointers takes a copy of the CRYPTO_EX_DATA_FUNCS pointers, if any, -// for the given class. If there are some pointers, it sets |*out| to point to -// a fresh stack of them. Otherwise it sets |*out| to NULL. It returns one on -// success or zero on error. -static int get_func_pointers(STACK_OF(CRYPTO_EX_DATA_FUNCS) **out, - CRYPTO_EX_DATA_CLASS *ex_data_class) { - size_t n; - - *out = NULL; - - // CRYPTO_EX_DATA_FUNCS structures are static once set, so we can take a - // shallow copy of the list under lock and then use the structures without - // the lock held. - CRYPTO_STATIC_MUTEX_lock_read(&ex_data_class->lock); - n = sk_CRYPTO_EX_DATA_FUNCS_num(ex_data_class->meth); - if (n > 0) { - *out = sk_CRYPTO_EX_DATA_FUNCS_dup(ex_data_class->meth); - } - CRYPTO_STATIC_MUTEX_unlock_read(&ex_data_class->lock); - - if (n > 0 && *out == NULL) { - return 0; - } - - return 1; -} - void CRYPTO_new_ex_data(CRYPTO_EX_DATA *ad) { ad->sk = NULL; } @@ -247,26 +214,21 @@ void CRYPTO_free_ex_data(CRYPTO_EX_DATA_CLASS *ex_data_class, void *obj, return; } - STACK_OF(CRYPTO_EX_DATA_FUNCS) *func_pointers; - if (!get_func_pointers(&func_pointers, ex_data_class)) { - // TODO(davidben): This leaks memory on malloc error. - return; - } - + uint32_t num_funcs = CRYPTO_atomic_load_u32(&ex_data_class->num_funcs); // |CRYPTO_get_ex_new_index| will not allocate indices beyond |INT_MAX|. - assert(sk_CRYPTO_EX_DATA_FUNCS_num(func_pointers) <= - (size_t)(INT_MAX - ex_data_class->num_reserved)); - for (int i = 0; i < (int)sk_CRYPTO_EX_DATA_FUNCS_num(func_pointers); i++) { - CRYPTO_EX_DATA_FUNCS *func_pointer = - sk_CRYPTO_EX_DATA_FUNCS_value(func_pointers, i); - if (func_pointer->free_func) { - void *ptr = CRYPTO_get_ex_data(ad, i + ex_data_class->num_reserved); - func_pointer->free_func(obj, ptr, ad, i + ex_data_class->num_reserved, - func_pointer->argl, func_pointer->argp); - } - } + assert(num_funcs <= (size_t)(INT_MAX - ex_data_class->num_reserved)); - sk_CRYPTO_EX_DATA_FUNCS_free(func_pointers); + // Defer dereferencing |ex_data_class->funcs| and |funcs->next|. It must come + // after the |num_funcs| comparison to be correctly synchronized. + CRYPTO_EX_DATA_FUNCS *const *funcs = &ex_data_class->funcs; + for (uint32_t i = 0; i < num_funcs; i++) { + if ((*funcs)->free_func != NULL) { + int index = (int)i + ex_data_class->num_reserved; + void *ptr = CRYPTO_get_ex_data(ad, index); + (*funcs)->free_func(obj, ptr, ad, index, (*funcs)->argl, (*funcs)->argp); + } + funcs = &(*funcs)->next; + } sk_void_free(ad->sk); ad->sk = NULL; diff --git a/src/crypto/internal.h b/src/crypto/internal.h index 4b7d82c04..d15f7534b 100644 --- a/src/crypto/internal.h +++ b/src/crypto/internal.h @@ -850,22 +850,25 @@ OPENSSL_EXPORT int CRYPTO_set_thread_local( typedef struct crypto_ex_data_func_st CRYPTO_EX_DATA_FUNCS; -DECLARE_STACK_OF(CRYPTO_EX_DATA_FUNCS) - // CRYPTO_EX_DATA_CLASS tracks the ex_indices registered for a type which // supports ex_data. It should defined as a static global within the module // which defines that type. typedef struct { struct CRYPTO_STATIC_MUTEX lock; - STACK_OF(CRYPTO_EX_DATA_FUNCS) *meth; + // funcs is a linked list of |CRYPTO_EX_DATA_FUNCS| structures. It may be + // traversed without serialization only up to |num_funcs|. last points to the + // final entry of |funcs|, or NULL if empty. + CRYPTO_EX_DATA_FUNCS *funcs, *last; + // num_funcs is the number of entries in |funcs|. + CRYPTO_atomic_u32 num_funcs; // num_reserved is one if the ex_data index zero is reserved for legacy // |TYPE_get_app_data| functions. uint8_t num_reserved; } CRYPTO_EX_DATA_CLASS; -#define CRYPTO_EX_DATA_CLASS_INIT {CRYPTO_STATIC_MUTEX_INIT, NULL, 0} +#define CRYPTO_EX_DATA_CLASS_INIT {CRYPTO_STATIC_MUTEX_INIT, NULL, NULL, 0, 0} #define CRYPTO_EX_DATA_CLASS_INIT_WITH_APP_DATA \ - {CRYPTO_STATIC_MUTEX_INIT, NULL, 1} + {CRYPTO_STATIC_MUTEX_INIT, NULL, NULL, 0, 1} // CRYPTO_get_ex_new_index allocates a new index for |ex_data_class| and writes // it to |*out_index|. Each class of object should provide a wrapper function diff --git a/src/ssl/ssl_cipher.cc b/src/ssl/ssl_cipher.cc index abf5e3d4a..23af47483 100644 --- a/src/ssl/ssl_cipher.cc +++ b/src/ssl/ssl_cipher.cc @@ -540,12 +540,16 @@ static const CIPHER_ALIAS kCipherAliases[] = { {"PSK", SSL_kPSK, SSL_aPSK, ~0u, ~0u, 0}, // symmetric encryption aliases - {"3DES", ~0u, ~0u, SSL_3DES, ~0u, 0}, - {"AES128", ~0u, ~0u, SSL_AES128 | SSL_AES128GCM, ~0u, 0}, - {"AES256", ~0u, ~0u, SSL_AES256 | SSL_AES256GCM, ~0u, 0}, + {"3DES", ~0u, ~0u, SSL_3DES, ~0u, 0, /*include_deprecated=*/true}, + {"AES128", ~0u, ~0u, SSL_AES128 | SSL_AES128GCM, ~0u, 0, + /*include_deprecated=*/false}, + {"AES256", ~0u, ~0u, SSL_AES256 | SSL_AES256GCM, ~0u, 0, + /*include_deprecated=*/false}, {"AES", ~0u, ~0u, SSL_AES, ~0u, 0}, - {"AESGCM", ~0u, ~0u, SSL_AES128GCM | SSL_AES256GCM, ~0u, 0}, - {"CHACHA20", ~0u, ~0u, SSL_CHACHA20POLY1305, ~0u, 0}, + {"AESGCM", ~0u, ~0u, SSL_AES128GCM | SSL_AES256GCM, ~0u, 0, + /*include_deprecated=*/false}, + {"CHACHA20", ~0u, ~0u, SSL_CHACHA20POLY1305, ~0u, 0, + /*include_deprecated=*/false}, // MAC aliases {"SHA1", ~0u, ~0u, ~0u, SSL_SHA1, 0}, @@ -769,8 +773,8 @@ void SSLCipherPreferenceList::Remove(const SSL_CIPHER *cipher) { } bool ssl_cipher_is_deprecated(const SSL_CIPHER *cipher) { - // TODO(crbug.com/boringssl/599): Deprecate 3DES. - return cipher->id == TLS1_CK_ECDHE_RSA_WITH_AES_128_CBC_SHA256; + return cipher->id == TLS1_CK_ECDHE_RSA_WITH_AES_128_CBC_SHA256 || + cipher->algorithm_enc == SSL_3DES; } // ssl_cipher_apply_rule applies the rule type |rule| to ciphers matching its @@ -1070,8 +1074,6 @@ static bool ssl_cipher_process_rulestr(const char *rule_str, // can increase the set of matched ciphers. This is so that an alias // like "RSA" will only specifiy AES-based RSA ciphers, but // "RSA+3DES" will still specify 3DES. - // - // TODO(crbug.com/boringssl/599): Deprecate 3DES. alias.include_deprecated |= kCipherAliases[j].include_deprecated; if (alias.min_version != 0 && diff --git a/src/ssl/ssl_test.cc b/src/ssl/ssl_test.cc index be00e7c34..4f6a0e616 100644 --- a/src/ssl/ssl_test.cc +++ b/src/ssl/ssl_test.cc @@ -353,6 +353,81 @@ static const CipherTest kCipherTests[] = { // …but not in strict mode. true, }, + // 3DES ciphers are disabled by default. + { + "RSA", + { + {TLS1_CK_RSA_WITH_AES_128_GCM_SHA256, 0}, + {TLS1_CK_RSA_WITH_AES_256_GCM_SHA384, 0}, + {TLS1_CK_RSA_WITH_AES_128_SHA, 0}, + {TLS1_CK_RSA_WITH_AES_256_SHA, 0}, + }, + false, + }, + // But 3DES ciphers may be specified by name. + { + "TLS_RSA_WITH_3DES_EDE_CBC_SHA", + { + {SSL3_CK_RSA_DES_192_CBC3_SHA, 0}, + }, + false, + }, + { + "DES-CBC3-SHA", + { + {SSL3_CK_RSA_DES_192_CBC3_SHA, 0}, + }, + false, + }, + // Or by a selector that specifically includes deprecated ciphers. + { + "3DES", + { + {SSL3_CK_RSA_DES_192_CBC3_SHA, 0}, + }, + false, + }, + // Such selectors may be combined with other selectors that would otherwise + // not allow deprecated ciphers. + { + "RSA+3DES", + { + {SSL3_CK_RSA_DES_192_CBC3_SHA, 0}, + }, + false, + }, + // The cipher must still match all combined selectors, however. "ECDHE+3DES" + // matches nothing because we do not implement + // TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA. (The test includes + // TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 so the final list is not empty.) + { + "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256:ECDHE+3DES", + { + {TLS1_CK_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, 0}, + }, + false, + }, + // Although alises like "RSA" do not match 3DES when adding ciphers, they do + // match it when removing ciphers. + { + "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256:RSA:RSA+3DES:!RSA", + { + {TLS1_CK_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, 0}, + }, + false, + }, + // 3DES still participates in strength sorting. + { + "RSA:3DES:@STRENGTH", + { + {TLS1_CK_RSA_WITH_AES_256_GCM_SHA384, 0}, + {TLS1_CK_RSA_WITH_AES_256_SHA, 0}, + {TLS1_CK_RSA_WITH_AES_128_GCM_SHA256, 0}, + {TLS1_CK_RSA_WITH_AES_128_SHA, 0}, + {SSL3_CK_RSA_DES_192_CBC3_SHA, 0}, + }, + false, + }, }; static const char *kBadRules[] = { @@ -3192,39 +3267,39 @@ TEST(SSLTest, ClientHello) { uint16_t max_version; std::vector expected; } kTests[] = { - {TLS1_VERSION, - {0x16, 0x03, 0x01, 0x00, 0x5a, 0x01, 0x00, 0x00, 0x56, 0x03, 0x01, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0e, 0xc0, 0x09, - 0xc0, 0x13, 0xc0, 0x0a, 0xc0, 0x14, 0x00, 0x2f, 0x00, 0x35, 0x00, 0x0a, - 0x01, 0x00, 0x00, 0x1f, 0x00, 0x17, 0x00, 0x00, 0xff, 0x01, 0x00, 0x01, - 0x00, 0x00, 0x0a, 0x00, 0x08, 0x00, 0x06, 0x00, 0x1d, 0x00, 0x17, 0x00, - 0x18, 0x00, 0x0b, 0x00, 0x02, 0x01, 0x00, 0x00, 0x23, 0x00, 0x00}}, - {TLS1_1_VERSION, - {0x16, 0x03, 0x01, 0x00, 0x5a, 0x01, 0x00, 0x00, 0x56, 0x03, 0x02, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0e, 0xc0, 0x09, - 0xc0, 0x13, 0xc0, 0x0a, 0xc0, 0x14, 0x00, 0x2f, 0x00, 0x35, 0x00, 0x0a, - 0x01, 0x00, 0x00, 0x1f, 0x00, 0x17, 0x00, 0x00, 0xff, 0x01, 0x00, 0x01, - 0x00, 0x00, 0x0a, 0x00, 0x08, 0x00, 0x06, 0x00, 0x1d, 0x00, 0x17, 0x00, - 0x18, 0x00, 0x0b, 0x00, 0x02, 0x01, 0x00, 0x00, 0x23, 0x00, 0x00}}, - {TLS1_2_VERSION, - {0x16, 0x03, 0x01, 0x00, 0x82, 0x01, 0x00, 0x00, 0x7e, 0x03, 0x03, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x1e, 0xcc, 0xa9, - 0xcc, 0xa8, 0xc0, 0x2b, 0xc0, 0x2f, 0xc0, 0x2c, 0xc0, 0x30, 0xc0, 0x09, - 0xc0, 0x13, 0xc0, 0x0a, 0xc0, 0x14, 0x00, 0x9c, 0x00, 0x9d, 0x00, 0x2f, - 0x00, 0x35, 0x00, 0x0a, 0x01, 0x00, 0x00, 0x37, 0x00, 0x17, 0x00, 0x00, - 0xff, 0x01, 0x00, 0x01, 0x00, 0x00, 0x0a, 0x00, 0x08, 0x00, 0x06, 0x00, - 0x1d, 0x00, 0x17, 0x00, 0x18, 0x00, 0x0b, 0x00, 0x02, 0x01, 0x00, 0x00, - 0x23, 0x00, 0x00, 0x00, 0x0d, 0x00, 0x14, 0x00, 0x12, 0x04, 0x03, 0x08, - 0x04, 0x04, 0x01, 0x05, 0x03, 0x08, 0x05, 0x05, 0x01, 0x08, 0x06, 0x06, - 0x01, 0x02, 0x01}}, - // TODO(davidben): Add a change detector for TLS 1.3 once the spec and our - // implementation has settled enough that it won't change. + {TLS1_VERSION, + {0x16, 0x03, 0x01, 0x00, 0x58, 0x01, 0x00, 0x00, 0x54, 0x03, 0x01, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0c, 0xc0, 0x09, + 0xc0, 0x13, 0xc0, 0x0a, 0xc0, 0x14, 0x00, 0x2f, 0x00, 0x35, 0x01, 0x00, + 0x00, 0x1f, 0x00, 0x17, 0x00, 0x00, 0xff, 0x01, 0x00, 0x01, 0x00, 0x00, + 0x0a, 0x00, 0x08, 0x00, 0x06, 0x00, 0x1d, 0x00, 0x17, 0x00, 0x18, 0x00, + 0x0b, 0x00, 0x02, 0x01, 0x00, 0x00, 0x23, 0x00, 0x00}}, + {TLS1_1_VERSION, + {0x16, 0x03, 0x01, 0x00, 0x58, 0x01, 0x00, 0x00, 0x54, 0x03, 0x02, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0c, 0xc0, 0x09, + 0xc0, 0x13, 0xc0, 0x0a, 0xc0, 0x14, 0x00, 0x2f, 0x00, 0x35, 0x01, 0x00, + 0x00, 0x1f, 0x00, 0x17, 0x00, 0x00, 0xff, 0x01, 0x00, 0x01, 0x00, 0x00, + 0x0a, 0x00, 0x08, 0x00, 0x06, 0x00, 0x1d, 0x00, 0x17, 0x00, 0x18, 0x00, + 0x0b, 0x00, 0x02, 0x01, 0x00, 0x00, 0x23, 0x00, 0x00}}, + {TLS1_2_VERSION, + {0x16, 0x03, 0x01, 0x00, 0x80, 0x01, 0x00, 0x00, 0x7c, 0x03, 0x03, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x1c, 0xcc, 0xa9, + 0xcc, 0xa8, 0xc0, 0x2b, 0xc0, 0x2f, 0xc0, 0x2c, 0xc0, 0x30, 0xc0, 0x09, + 0xc0, 0x13, 0xc0, 0x0a, 0xc0, 0x14, 0x00, 0x9c, 0x00, 0x9d, 0x00, 0x2f, + 0x00, 0x35, 0x01, 0x00, 0x00, 0x37, 0x00, 0x17, 0x00, 0x00, 0xff, 0x01, + 0x00, 0x01, 0x00, 0x00, 0x0a, 0x00, 0x08, 0x00, 0x06, 0x00, 0x1d, 0x00, + 0x17, 0x00, 0x18, 0x00, 0x0b, 0x00, 0x02, 0x01, 0x00, 0x00, 0x23, 0x00, + 0x00, 0x00, 0x0d, 0x00, 0x14, 0x00, 0x12, 0x04, 0x03, 0x08, 0x04, 0x04, + 0x01, 0x05, 0x03, 0x08, 0x05, 0x05, 0x01, 0x08, 0x06, 0x06, 0x01, 0x02, + 0x01}}, + // TODO(davidben): Add a change detector for TLS 1.3 once the spec and our + // implementation has settled enough that it won't change. }; for (const auto &t : kTests) { @@ -5463,8 +5538,8 @@ TEST(SSLTest, ApplyHandoffRemovesUnsupportedCiphers) { ASSERT_TRUE(server); // handoff is a handoff message that has been artificially modified to pretend - // that only cipher 0x0A is supported. When it is applied to |server|, all - // ciphers but that one should be removed. + // that only TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (0xc02f) is supported. When + // it is applied to |server|, all ciphers but that one should be removed. // // To make a new one of these, try sticking this in the |Handoff| test above: // @@ -5485,12 +5560,12 @@ TEST(SSLTest, ApplyHandoffRemovesUnsupportedCiphers) { 0x0a, 0x00, 0x08, 0x00, 0x06, 0x00, 0x1d, 0x00, 0x17, 0x00, 0x18, 0x00, 0x0b, 0x00, 0x02, 0x01, 0x00, 0x00, 0x23, 0x00, 0x00, 0x00, 0x0d, 0x00, 0x14, 0x00, 0x12, 0x04, 0x03, 0x08, 0x04, 0x04, 0x01, 0x05, 0x03, 0x08, - 0x05, 0x05, 0x01, 0x08, 0x06, 0x06, 0x01, 0x02, 0x01, 0x04, 0x02, 0x00, - 0x0a, 0x04, 0x0a, 0x00, 0x15, 0x00, 0x17, 0x00, 0x18, 0x00, 0x19, 0x00, + 0x05, 0x05, 0x01, 0x08, 0x06, 0x06, 0x01, 0x02, 0x01, 0x04, 0x02, 0xc0, + 0x2f, 0x04, 0x0a, 0x00, 0x15, 0x00, 0x17, 0x00, 0x18, 0x00, 0x19, 0x00, 0x1d, }; - EXPECT_EQ(20u, sk_SSL_CIPHER_num(SSL_get_ciphers(server.get()))); + EXPECT_LT(1u, sk_SSL_CIPHER_num(SSL_get_ciphers(server.get()))); ASSERT_TRUE( SSL_apply_handoff(server.get(), {handoff, OPENSSL_ARRAY_SIZE(handoff)})); EXPECT_EQ(1u, sk_SSL_CIPHER_num(SSL_get_ciphers(server.get()))); diff --git a/src/ssl/test/fuzzer.h b/src/ssl/test/fuzzer.h index 864c643e1..4888a39f8 100644 --- a/src/ssl/test/fuzzer.h +++ b/src/ssl/test/fuzzer.h @@ -414,7 +414,7 @@ class TLSFuzzer { SSL_CTX_enable_ocsp_stapling(ctx_.get()); // Enable versions and ciphers that are off by default. - if (!SSL_CTX_set_strict_cipher_list(ctx_.get(), "ALL")) { + if (!SSL_CTX_set_strict_cipher_list(ctx_.get(), "ALL:3DES")) { return false; } diff --git a/src/ssl/test/runner/runner.go b/src/ssl/test/runner/runner.go index 86daca272..da0795bbb 100644 --- a/src/ssl/test/runner/runner.go +++ b/src/ssl/test/runner/runner.go @@ -3199,7 +3199,7 @@ read alert 1 0 // elliptic curves, so no extensions are // involved. MaxVersion: VersionTLS12, - CipherSuites: []uint16{TLS_RSA_WITH_3DES_EDE_CBC_SHA}, + CipherSuites: []uint16{TLS_RSA_WITH_AES_128_CBC_SHA}, Bugs: ProtocolBugs{ SendV2ClientHello: true, }, @@ -3221,7 +3221,7 @@ read alert 1 0 // elliptic curves, so no extensions are // involved. MaxVersion: VersionTLS12, - CipherSuites: []uint16{TLS_RSA_WITH_3DES_EDE_CBC_SHA}, + CipherSuites: []uint16{TLS_RSA_WITH_AES_128_CBC_SHA}, Bugs: ProtocolBugs{ SendV2ClientHello: true, }, @@ -3683,6 +3683,11 @@ func addTestForCipherSuite(suite testCipherSuite, ver tlsVersion, protocol proto "-psk-identity", pskIdentity) } + if hasComponent(suite.name, "3DES") { + // BoringSSL disables 3DES ciphers by default. + flags = append(flags, "-cipher", "3DES") + } + var shouldFail bool if isTLS12Only(suite.name) && ver.version < VersionTLS12 { shouldFail = true @@ -4230,6 +4235,8 @@ func addCBCSplittingTests() { "-async", "-write-different-record-sizes", "-cbc-record-splitting", + // BoringSSL disables 3DES by default. + "-cipher", "ALL:3DES", }, }) testCases = append(testCases, testCase{ @@ -4248,6 +4255,8 @@ func addCBCSplittingTests() { "-write-different-record-sizes", "-cbc-record-splitting", "-partial-write", + // BoringSSL disables 3DES by default. + "-cipher", "ALL:3DES", }, }) } @@ -5771,7 +5780,7 @@ func addStateMachineCoverageTests(config stateMachineTestConfig) { // elliptic curves, so no extensions are // involved. MaxVersion: VersionTLS12, - CipherSuites: []uint16{TLS_RSA_WITH_3DES_EDE_CBC_SHA}, + CipherSuites: []uint16{TLS_RSA_WITH_AES_128_CBC_SHA}, Bugs: ProtocolBugs{ SendV2ClientHello: true, V2ClientHelloChallengeLength: challengeLength, @@ -9321,7 +9330,7 @@ func addRenegotiationTests() { renegotiate: 1, config: Config{ MaxVersion: VersionTLS12, - CipherSuites: []uint16{TLS_RSA_WITH_3DES_EDE_CBC_SHA}, + CipherSuites: []uint16{TLS_RSA_WITH_AES_128_CBC_SHA}, }, renegotiateCiphers: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}, flags: []string{ @@ -9336,7 +9345,7 @@ func addRenegotiationTests() { MaxVersion: VersionTLS12, CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}, }, - renegotiateCiphers: []uint16{TLS_RSA_WITH_3DES_EDE_CBC_SHA}, + renegotiateCiphers: []uint16{TLS_RSA_WITH_AES_128_CBC_SHA}, flags: []string{ "-renegotiate-freely", "-expect-total-renegotiations", "1", @@ -11353,7 +11362,7 @@ func addRSAClientKeyExchangeTests() { // version are different, to detect if the // server uses the wrong one. MaxVersion: VersionTLS11, - CipherSuites: []uint16{TLS_RSA_WITH_3DES_EDE_CBC_SHA}, + CipherSuites: []uint16{TLS_RSA_WITH_AES_128_CBC_SHA}, Bugs: ProtocolBugs{ BadRSAClientKeyExchange: bad, },