From af096751e8d1cae9a570990b49b12527317c0003 Mon Sep 17 00:00:00 2001 From: Matt Braithwaite Date: Wed, 2 Sep 2015 19:48:16 -0700 Subject: [PATCH] Restore the NULL-SHA ciphersuite. (Alas.) Change-Id: Ia5398f3b86a13fb20dba053f730b51a0e57b9aa4 Reviewed-on: https://boringssl-review.googlesource.com/5791 Reviewed-by: Adam Langley --- crypto/cipher/e_ssl3.c | 22 +++++++++++++ crypto/cipher/e_tls.c | 22 +++++++++++++ include/openssl/aead.h | 3 ++ include/openssl/ssl.h | 6 ++++ ssl/d1_lib.c | 5 +-- ssl/internal.h | 1 + ssl/ssl_cipher.c | 53 ++++++++++++++++++++++++++------ ssl/ssl_test.cc | 39 +++++++++++++++++++++++ ssl/t1_enc.c | 2 +- ssl/test/runner/cipher_suites.go | 8 +++++ ssl/test/runner/conn.go | 4 +++ ssl/test/runner/runner.go | 7 ++++- 12 files changed, 159 insertions(+), 13 deletions(-) diff --git a/crypto/cipher/e_ssl3.c b/crypto/cipher/e_ssl3.c index a4b55c90..389c52f5 100644 --- a/crypto/cipher/e_ssl3.c +++ b/crypto/cipher/e_ssl3.c @@ -340,6 +340,13 @@ static int aead_des_ede3_cbc_sha1_ssl3_init(EVP_AEAD_CTX *ctx, EVP_sha1()); } +static int aead_null_sha1_ssl3_init(EVP_AEAD_CTX *ctx, const uint8_t *key, + size_t key_len, size_t tag_len, + enum evp_aead_direction_t dir) { + return aead_ssl3_init(ctx, key, key_len, tag_len, dir, EVP_enc_null(), + EVP_sha1()); +} + static const EVP_AEAD aead_rc4_md5_ssl3 = { MD5_DIGEST_LENGTH + 16, /* key len (MD5 + RC4) */ 0, /* nonce len */ @@ -405,6 +412,19 @@ static const EVP_AEAD aead_des_ede3_cbc_sha1_ssl3 = { NULL, /* get_rc4_state */ }; +static const EVP_AEAD aead_null_sha1_ssl3 = { + SHA_DIGEST_LENGTH, /* key len */ + 0, /* nonce len */ + SHA_DIGEST_LENGTH, /* overhead (SHA1) */ + SHA_DIGEST_LENGTH, /* max tag length */ + NULL, /* init */ + aead_null_sha1_ssl3_init, + aead_ssl3_cleanup, + aead_ssl3_seal, + aead_ssl3_open, + NULL, /* get_rc4_state */ +}; + const EVP_AEAD *EVP_aead_rc4_md5_ssl3(void) { return &aead_rc4_md5_ssl3; } const EVP_AEAD *EVP_aead_rc4_sha1_ssl3(void) { return &aead_rc4_sha1_ssl3; } @@ -420,3 +440,5 @@ const EVP_AEAD *EVP_aead_aes_256_cbc_sha1_ssl3(void) { const EVP_AEAD *EVP_aead_des_ede3_cbc_sha1_ssl3(void) { return &aead_des_ede3_cbc_sha1_ssl3; } + +const EVP_AEAD *EVP_aead_null_sha1_ssl3(void) { return &aead_null_sha1_ssl3; } diff --git a/crypto/cipher/e_tls.c b/crypto/cipher/e_tls.c index 7938c36d..27788813 100644 --- a/crypto/cipher/e_tls.c +++ b/crypto/cipher/e_tls.c @@ -444,6 +444,13 @@ static int aead_rc4_sha1_tls_get_rc4_state(const EVP_AEAD_CTX *ctx, return 1; } +static int aead_null_sha1_tls_init(EVP_AEAD_CTX *ctx, const uint8_t *key, + size_t key_len, size_t tag_len, + enum evp_aead_direction_t dir) { + return aead_tls_init(ctx, key, key_len, tag_len, dir, EVP_enc_null(), + EVP_sha1(), 1 /* implicit iv */); +} + static const EVP_AEAD aead_rc4_sha1_tls = { SHA_DIGEST_LENGTH + 16, /* key len (SHA1 + RC4) */ 0, /* nonce len */ @@ -574,6 +581,19 @@ static const EVP_AEAD aead_des_ede3_cbc_sha1_tls_implicit_iv = { NULL, /* get_rc4_state */ }; +static const EVP_AEAD aead_null_sha1_tls = { + SHA_DIGEST_LENGTH, /* key len */ + 0, /* nonce len */ + SHA_DIGEST_LENGTH, /* overhead (SHA1) */ + SHA_DIGEST_LENGTH, /* max tag length */ + NULL, /* init */ + aead_null_sha1_tls_init, + aead_tls_cleanup, + aead_tls_seal, + aead_tls_open, + NULL, /* get_rc4_state */ +}; + const EVP_AEAD *EVP_aead_rc4_sha1_tls(void) { return &aead_rc4_sha1_tls; } const EVP_AEAD *EVP_aead_aes_128_cbc_sha1_tls(void) { @@ -611,3 +631,5 @@ const EVP_AEAD *EVP_aead_des_ede3_cbc_sha1_tls(void) { const EVP_AEAD *EVP_aead_des_ede3_cbc_sha1_tls_implicit_iv(void) { return &aead_des_ede3_cbc_sha1_tls_implicit_iv; } + +const EVP_AEAD *EVP_aead_null_sha1_tls(void) { return &aead_null_sha1_tls; } diff --git a/include/openssl/aead.h b/include/openssl/aead.h index f61c495e..659f05fd 100644 --- a/include/openssl/aead.h +++ b/include/openssl/aead.h @@ -153,6 +153,8 @@ OPENSSL_EXPORT const EVP_AEAD *EVP_aead_aes_256_cbc_sha384_tls(void); OPENSSL_EXPORT const EVP_AEAD *EVP_aead_des_ede3_cbc_sha1_tls(void); OPENSSL_EXPORT const EVP_AEAD *EVP_aead_des_ede3_cbc_sha1_tls_implicit_iv(void); +OPENSSL_EXPORT const EVP_AEAD *EVP_aead_null_sha1_tls(void); + /* SSLv3-specific AEAD algorithms. * @@ -167,6 +169,7 @@ OPENSSL_EXPORT const EVP_AEAD *EVP_aead_rc4_sha1_ssl3(void); OPENSSL_EXPORT const EVP_AEAD *EVP_aead_aes_128_cbc_sha1_ssl3(void); OPENSSL_EXPORT const EVP_AEAD *EVP_aead_aes_256_cbc_sha1_ssl3(void); OPENSSL_EXPORT const EVP_AEAD *EVP_aead_des_ede3_cbc_sha1_ssl3(void); +OPENSSL_EXPORT const EVP_AEAD *EVP_aead_null_sha1_ssl3(void); /* Utility functions. */ diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 7109214d..ab702ee4 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -229,6 +229,12 @@ OPENSSL_EXPORT int SSL_CIPHER_is_AESGCM(const SSL_CIPHER *cipher); * CHACHA20_POLY1305. */ OPENSSL_EXPORT int SSL_CIPHER_is_CHACHA20POLY1305(const SSL_CIPHER *cipher); +/* SSL_CIPHER_is_NULL returns one if |cipher| does not encrypt. */ +OPENSSL_EXPORT int SSL_CIPHER_is_NULL(const SSL_CIPHER *cipher); + +/* SSL_CIPHER_is_block_cipher returns one if |cipher| is a block cipher. */ +OPENSSL_EXPORT int SSL_CIPHER_is_block_cipher(const SSL_CIPHER *cipher); + /* SSL_CIPHER_get_name returns the OpenSSL name of |cipher|. */ OPENSSL_EXPORT const char *SSL_CIPHER_get_name(const SSL_CIPHER *cipher); diff --git a/ssl/d1_lib.c b/ssl/d1_lib.c index 2dc55d62..db8a536b 100644 --- a/ssl/d1_lib.c +++ b/ssl/d1_lib.c @@ -152,8 +152,9 @@ void dtls1_free(SSL *s) { } int dtls1_supports_cipher(const SSL_CIPHER *cipher) { - /* DTLS does not support stream ciphers. */ - return cipher->algorithm_enc != SSL_RC4; + /* DTLS does not support stream ciphers. The NULL cipher is rejected because + * it's not needed. */ + return cipher->algorithm_enc != SSL_RC4 && cipher->algorithm_enc != SSL_eNULL; } void dtls1_start_timer(SSL *s) { diff --git a/ssl/internal.h b/ssl/internal.h index 5e0dcb60..a788b6e8 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -182,6 +182,7 @@ #define SSL_AES128GCM 0x00000010L #define SSL_AES256GCM 0x00000020L #define SSL_CHACHA20POLY1305 0x00000040L +#define SSL_eNULL 0x00000080L #define SSL_AES (SSL_AES128 | SSL_AES256 | SSL_AES128GCM | SSL_AES256GCM) diff --git a/ssl/ssl_cipher.c b/ssl/ssl_cipher.c index 1b8673e9..4894239f 100644 --- a/ssl/ssl_cipher.c +++ b/ssl/ssl_cipher.c @@ -155,6 +155,12 @@ /* kCiphers is an array of all supported ciphers, sorted by id. */ const SSL_CIPHER kCiphers[] = { /* The RSA ciphers */ + /* Cipher 02 */ + { + SSL3_TXT_RSA_NULL_SHA, SSL3_CK_RSA_NULL_SHA, SSL_kRSA, SSL_aRSA, + SSL_eNULL, SSL_SHA1, SSL_SSLV3, SSL_FIPS, SSL_HANDSHAKE_MAC_DEFAULT, 0, 0, + }, + /* Cipher 04 */ { SSL3_TXT_RSA_RC4_128_MD5, SSL3_CK_RSA_RC4_128_MD5, SSL_kRSA, SSL_aRSA, @@ -491,7 +497,8 @@ typedef struct cipher_alias_st { } CIPHER_ALIAS; static const CIPHER_ALIAS kCipherAliases[] = { - {SSL_TXT_ALL, ~0u, ~0u, ~0u, ~0u, ~0u, ~0u}, + /* "ALL" doesn't include eNULL (must be specifically enabled) */ + {SSL_TXT_ALL, ~0u, ~0u, ~SSL_eNULL, ~0u, ~0u, ~0u}, /* The "COMPLEMENTOFDEFAULT" rule is omitted. It matches nothing. */ @@ -512,7 +519,7 @@ static const CIPHER_ALIAS kCipherAliases[] = { {SSL_TXT_kPSK, SSL_kPSK, ~0u, ~0u, ~0u, ~0u, ~0u}, /* server authentication aliases */ - {SSL_TXT_aRSA, ~0u, SSL_aRSA, ~0u, ~0u, ~0u, ~0u}, + {SSL_TXT_aRSA, ~0u, SSL_aRSA, ~SSL_eNULL, ~0u, ~0u, ~0u}, {SSL_TXT_aECDSA, ~0u, SSL_aECDSA, ~0u, ~0u, ~0u, ~0u}, {SSL_TXT_ECDSA, ~0u, SSL_aECDSA, ~0u, ~0u, ~0u, ~0u}, {SSL_TXT_aPSK, ~0u, SSL_aPSK, ~0u, ~0u, ~0u, ~0u}, @@ -522,7 +529,7 @@ static const CIPHER_ALIAS kCipherAliases[] = { {SSL_TXT_EDH, SSL_kDHE, ~0u, ~0u, ~0u, ~0u, ~0u}, {SSL_TXT_ECDHE, SSL_kECDHE, ~0u, ~0u, ~0u, ~0u, ~0u}, {SSL_TXT_EECDH, SSL_kECDHE, ~0u, ~0u, ~0u, ~0u, ~0u}, - {SSL_TXT_RSA, SSL_kRSA, SSL_aRSA, ~0u, ~0u, ~0u, ~0u}, + {SSL_TXT_RSA, SSL_kRSA, SSL_aRSA, ~SSL_eNULL, ~0u, ~0u, ~0u}, {SSL_TXT_PSK, SSL_kPSK, SSL_aPSK, ~0u, ~0u, ~0u, ~0u}, /* symmetric encryption aliases */ @@ -536,21 +543,21 @@ static const CIPHER_ALIAS kCipherAliases[] = { /* MAC aliases */ {SSL_TXT_MD5, ~0u, ~0u, ~0u, SSL_MD5, ~0u, ~0u}, - {SSL_TXT_SHA1, ~0u, ~0u, ~0u, SSL_SHA1, ~0u, ~0u}, - {SSL_TXT_SHA, ~0u, ~0u, ~0u, SSL_SHA1, ~0u, ~0u}, + {SSL_TXT_SHA1, ~0u, ~0u, ~SSL_eNULL, SSL_SHA1, ~0u, ~0u}, + {SSL_TXT_SHA, ~0u, ~0u, ~SSL_eNULL, SSL_SHA1, ~0u, ~0u}, {SSL_TXT_SHA256, ~0u, ~0u, ~0u, SSL_SHA256, ~0u, ~0u}, {SSL_TXT_SHA384, ~0u, ~0u, ~0u, SSL_SHA384, ~0u, ~0u}, /* protocol version aliases */ - {SSL_TXT_SSLV3, ~0u, ~0u, ~0u, ~0u, SSL_SSLV3, ~0u}, - {SSL_TXT_TLSV1, ~0u, ~0u, ~0u, ~0u, SSL_TLSV1, ~0u}, - {SSL_TXT_TLSV1_2, ~0u, ~0u, ~0u, ~0u, SSL_TLSV1_2, ~0u}, + {SSL_TXT_SSLV3, ~0u, ~0u, ~SSL_eNULL, ~0u, SSL_SSLV3, ~0u}, + {SSL_TXT_TLSV1, ~0u, ~0u, ~SSL_eNULL, ~0u, SSL_TLSV1, ~0u}, + {SSL_TXT_TLSV1_2, ~0u, ~0u, ~SSL_eNULL, ~0u, SSL_TLSV1_2, ~0u}, /* strength classes */ {SSL_TXT_MEDIUM, ~0u, ~0u, ~0u, ~0u, ~0u, SSL_MEDIUM}, {SSL_TXT_HIGH, ~0u, ~0u, ~0u, ~0u, ~0u, SSL_HIGH}, /* FIPS 140-2 approved ciphersuite */ - {SSL_TXT_FIPS, ~0u, ~0u, ~0u, ~0u, ~0u, SSL_FIPS}, + {SSL_TXT_FIPS, ~0u, ~0u, ~SSL_eNULL, ~0u, ~0u, SSL_FIPS}, }; static const size_t kCipherAliasesLen = @@ -693,6 +700,20 @@ int ssl_cipher_get_evp_aead(const EVP_AEAD **out_aead, return 0; } + case SSL_eNULL: + switch (cipher->algorithm_mac) { + case SSL_SHA1: + if (version == SSL3_VERSION) { + *out_aead = EVP_aead_null_sha1_ssl3(); + } else { + *out_aead = EVP_aead_null_sha1_tls(); + } + *out_mac_secret_len = SHA_DIGEST_LENGTH; + return 1; + default: + return 0; + } + default: return 0; } @@ -1365,6 +1386,16 @@ int SSL_CIPHER_is_CHACHA20POLY1305(const SSL_CIPHER *cipher) { return (cipher->algorithm_enc & SSL_CHACHA20POLY1305) != 0; } +int SSL_CIPHER_is_NULL(const SSL_CIPHER *cipher) { + return (cipher->algorithm_enc & SSL_eNULL) != 0; +} + +int SSL_CIPHER_is_block_cipher(const SSL_CIPHER *cipher) { + /* Neither stream cipher nor AEAD. */ + return (cipher->algorithm_enc & (SSL_RC4 | SSL_eNULL)) == 0 && + cipher->algorithm_mac != SSL_AEAD; +} + /* return the actual cipher being used */ const char *SSL_CIPHER_get_name(const SSL_CIPHER *cipher) { if (cipher != NULL) { @@ -1589,6 +1620,10 @@ const char *SSL_CIPHER_description(const SSL_CIPHER *cipher, char *buf, enc = "ChaCha20-Poly1305"; break; + case SSL_eNULL: + enc="None"; + break; + default: enc = "unknown"; break; diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index ad96e063..29447487 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc @@ -214,6 +214,21 @@ static const char *kBadRules[] = { NULL, }; +static const char *kMustNotIncludeNull[] = { + "ALL", + "DEFAULT", + "ALL:!eNULL", + "ALL:!NULL", + "FIPS", + "SHA", + "SHA1", + "RSA", + "SSLv3", + "TLSv1", + "TLSv1.2", + NULL +}; + static void PrintCipherPreferenceList(ssl_cipher_preference_list_st *list) { bool in_group = false; for (size_t i = 0; i < sk_SSL_CIPHER_num(list->ciphers); i++) { @@ -267,6 +282,24 @@ static bool TestCipherRule(CipherTest *t) { return true; } +static bool TestRuleDoesNotIncludeNull(const char *rule) { + ScopedSSL_CTX ctx(SSL_CTX_new(SSLv23_server_method())); + if (!ctx) { + return false; + } + if (!SSL_CTX_set_cipher_list(ctx.get(), rule)) { + fprintf(stderr, "Error: cipher rule '%s' failed\n", rule); + return false; + } + for (size_t i = 0; i < sk_SSL_CIPHER_num(ctx->cipher_list->ciphers); i++) { + if (SSL_CIPHER_is_NULL(sk_SSL_CIPHER_value(ctx->cipher_list->ciphers, i))) { + fprintf(stderr, "Error: cipher rule '%s' includes NULL\n",rule); + return false; + } + } + return true; +} + static bool TestCipherRules() { for (size_t i = 0; kCipherTests[i].rule != NULL; i++) { if (!TestCipherRule(&kCipherTests[i])) { @@ -286,6 +319,12 @@ static bool TestCipherRules() { ERR_clear_error(); } + for (size_t i = 0; kMustNotIncludeNull[i] != NULL; i++) { + if (!TestRuleDoesNotIncludeNull(kMustNotIncludeNull[i])) { + return false; + } + } + return true; } diff --git a/ssl/t1_enc.c b/ssl/t1_enc.c index b14f32a4..bb30810a 100644 --- a/ssl/t1_enc.c +++ b/ssl/t1_enc.c @@ -363,7 +363,7 @@ int tls1_change_cipher_state(SSL *s, int which) { s->s3->need_record_splitting = 0; if (!SSL_USE_EXPLICIT_IV(s) && (s->mode & SSL_MODE_CBC_RECORD_SPLITTING) != 0 && - s->s3->tmp.new_cipher->algorithm_enc != SSL_RC4) { + SSL_CIPHER_is_block_cipher(s->s3->tmp.new_cipher)) { /* Enable 1/n-1 record-splitting to randomize the IV. See * https://www.openssl.org/~bodo/tls-cbc.txt and the BEAST attack. */ s->s3->need_record_splitting = 1; diff --git a/ssl/test/runner/cipher_suites.go b/ssl/test/runner/cipher_suites.go index 24e8efb5..ffc056d0 100644 --- a/ssl/test/runner/cipher_suites.go +++ b/ssl/test/runner/cipher_suites.go @@ -124,6 +124,13 @@ var cipherSuites = []*cipherSuite{ {TLS_PSK_WITH_AES_256_CBC_SHA, 32, 20, 16, pskKA, suitePSK, cipherAES, macSHA1, nil}, {TLS_ECDHE_PSK_WITH_AES_128_CBC_SHA, 16, 20, 16, ecdhePSKKA, suiteECDHE | suitePSK, cipherAES, macSHA1, nil}, {TLS_ECDHE_PSK_WITH_AES_256_CBC_SHA, 32, 20, 16, ecdhePSKKA, suiteECDHE | suitePSK, cipherAES, macSHA1, nil}, + {TLS_RSA_WITH_NULL_SHA, 0, 20, 0, rsaKA, suiteNoDTLS, cipherNull, macSHA1, nil}, +} + +type nullCipher struct{} + +func cipherNull(key, iv []byte, isRead bool) interface{} { + return nullCipher{} } func cipherRC4(key, iv []byte, isRead bool) interface{} { @@ -368,6 +375,7 @@ func mutualCipherSuite(have []uint16, want uint16) *cipherSuite { // A list of the possible cipher suite ids. Taken from // http://www.iana.org/assignments/tls-parameters/tls-parameters.xml const ( + TLS_RSA_WITH_NULL_SHA uint16 = 0x0002 TLS_RSA_WITH_RC4_128_MD5 uint16 = 0x0004 TLS_RSA_WITH_RC4_128_SHA uint16 = 0x0005 TLS_RSA_WITH_3DES_EDE_CBC_SHA uint16 = 0x000a diff --git a/ssl/test/runner/conn.go b/ssl/test/runner/conn.go index b23a104a..ed016e0f 100644 --- a/ssl/test/runner/conn.go +++ b/ssl/test/runner/conn.go @@ -416,6 +416,8 @@ func (hc *halfConn) decrypt(b *block) (ok bool, prefixLen int, alertValue alert) // // However, our behavior matches OpenSSL, so we leak // only as much as they do. + case nullCipher: + break default: panic("unknown cipher type") } @@ -521,6 +523,8 @@ func (hc *halfConn) encrypt(b *block, explicitIVLen int) (bool, alert) { b.resize(recordHeaderLen + explicitIVLen + len(prefix) + len(finalBlock)) c.CryptBlocks(b.data[recordHeaderLen+explicitIVLen:], prefix) c.CryptBlocks(b.data[recordHeaderLen+explicitIVLen+len(prefix):], finalBlock) + case nullCipher: + break default: panic("unknown cipher type") } diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index 5259dae4..1875a269 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -773,6 +773,7 @@ var testCipherSuites = []struct { {"PSK-RC4-SHA", TLS_PSK_WITH_RC4_128_SHA}, {"RC4-MD5", TLS_RSA_WITH_RC4_128_MD5}, {"RC4-SHA", TLS_RSA_WITH_RC4_128_SHA}, + {"NULL-SHA", TLS_RSA_WITH_NULL_SHA}, } func hasComponent(suiteName, component string) bool { @@ -787,7 +788,7 @@ func isTLS12Only(suiteName string) bool { } func isDTLSCipher(suiteName string) bool { - return !hasComponent(suiteName, "RC4") + return !hasComponent(suiteName, "RC4") && !hasComponent(suiteName, "NULL") } func bigFromHex(hex string) *big.Int { @@ -1968,6 +1969,10 @@ func addCipherSuiteTests() { "-psk", psk, "-psk-identity", pskIdentity) } + if hasComponent(suite.name, "NULL") { + // NULL ciphers must be explicitly enabled. + flags = append(flags, "-cipher", "DEFAULT:NULL-SHA") + } for _, ver := range tlsVersions { if ver.version < VersionTLS12 && isTLS12Only(suite.name) {