From: Matt Caswell Date: Thu, 15 Sep 2022 16:36:52 +0000 (+0100) Subject: Move the SSLv3 crypto code into the new record layer X-Git-Tag: openssl-3.2.0-alpha1~1961 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=a8572674f12ceb39f7e66ccbaa8918b922c76739;p=thirdparty%2Fopenssl.git Move the SSLv3 crypto code into the new record layer Reviewed-by: Tomas Mraz Reviewed-by: Hugo Landau (Merged from https://github.com/openssl/openssl/pull/19217) --- diff --git a/ssl/record/methods/ssl3_meth.c b/ssl/record/methods/ssl3_meth.c index 6ff67df7d77..e48ca5bc700 100644 --- a/ssl/record/methods/ssl3_meth.c +++ b/ssl/record/methods/ssl3_meth.c @@ -24,6 +24,7 @@ static int ssl3_set_crypto_state(OSSL_RECORD_LAYER *rl, int level, COMP_METHOD *comp) { EVP_CIPHER_CTX *ciph_ctx; + int enc = (rl->direction == OSSL_RECORD_DIRECTION_WRITE) ? 1 : 0; if (md == NULL) { ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR); @@ -41,6 +42,12 @@ static int ssl3_set_crypto_state(OSSL_RECORD_LAYER *rl, int level, ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR); return OSSL_RECORD_RETURN_FATAL; } + + if ((md != NULL && EVP_DigestInit_ex(rl->md_ctx, md, NULL) <= 0)) { + ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR); + return OSSL_RECORD_RETURN_FATAL; + } + #ifndef OPENSSL_NO_COMP if (comp != NULL) { rl->compctx = COMP_CTX_new(comp); @@ -51,7 +58,7 @@ static int ssl3_set_crypto_state(OSSL_RECORD_LAYER *rl, int level, } #endif - if (!EVP_DecryptInit_ex(ciph_ctx, ciph, NULL, key, iv)) { + if (!EVP_CipherInit_ex(ciph_ctx, ciph, NULL, key, iv, enc)) { ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR); return OSSL_RECORD_RETURN_FATAL; } diff --git a/ssl/record/methods/tls_common.c b/ssl/record/methods/tls_common.c index ef5f8e5e8fe..f07cd8ae8c8 100644 --- a/ssl/record/methods/tls_common.c +++ b/ssl/record/methods/tls_common.c @@ -1521,11 +1521,10 @@ int tls_write_records_default(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *thistempl; /* - * TODO(RECLAYER): Remove this once SSLv3/TLSv1.3/DTLS crypto has + * TODO(RECLAYER): Remove this once TLSv1.3/DTLS crypto has * been moved to the new write record layer. */ - if (rl->version == SSL3_VERSION - || rl->version == TLS1_3_VERSION + if (rl->version == TLS1_3_VERSION || rl->isdtls) { SSL_SESSION *sess = s->session; @@ -1767,11 +1766,10 @@ int tls_write_records_default(OSSL_RECORD_LAYER *rl, unsigned char *mac; /* - * TODO(RECLAYER): Remove this once SSLv3/TLSv1.3/DTLS crypto has + * TODO(RECLAYER): Remove this once TLSv1.3/DTLS crypto has * been moved to the new write record layer. */ - if (rl->version == SSL3_VERSION - || rl->version == TLS1_3_VERSION + if (rl->version == TLS1_3_VERSION || rl->isdtls) { if (!WPACKET_allocate_bytes(thispkt, mac_size, &mac) || !ssl->method->ssl3_enc->mac(s, thiswr, mac, 1)) { @@ -1826,30 +1824,17 @@ int tls_write_records_default(OSSL_RECORD_LAYER *rl, } else { if (!using_ktls) { if (prefix) { - /* - * TODO(RECLAYER): Remove this once SSLv3 crypto has been moved - * to the new write record layer. - */ - if (rl->version == SSL3_VERSION) { - if (ssl->method->ssl3_enc->enc(s, wr, 1, 1, NULL, mac_size) < 1) { - if (!ossl_statem_in_error(s)) - RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - goto err; - } - } else { - if (rl->funcs->cipher(rl, wr, 1, 1, NULL, mac_size) < 1) { - if (!ossl_statem_in_error(s)) - RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - goto err; - } + if (rl->funcs->cipher(rl, wr, 1, 1, NULL, mac_size) < 1) { + if (!ossl_statem_in_error(s)) + RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); + goto err; } } /* - * TODO(RECLAYER): Remove this once SSLv3/TLSv1.3/DTLS crypto has + * TODO(RECLAYER): Remove this once TLSv1.3/DTLS crypto has * been moved to the new write record layer. */ - if (rl->version == SSL3_VERSION - || rl->version == TLS1_3_VERSION + if (rl->version == TLS1_3_VERSION || rl->isdtls) { if (ssl->method->ssl3_enc->enc(s, wr + prefix, numtempl, 1, NULL, mac_size) < 1) { @@ -1894,11 +1879,10 @@ int tls_write_records_default(OSSL_RECORD_LAYER *rl, unsigned char *mac; /* - * TODO(RECLAYER): Remove this once SSLv3/TLSv1.3/DTLS crypto has + * TODO(RECLAYER): Remove this once TLSv1.3/DTLS crypto has * been moved to the new write record layer. */ - if (rl->version == SSL3_VERSION - || rl->version == TLS1_3_VERSION + if (rl->version == TLS1_3_VERSION || rl->isdtls) { if (!WPACKET_allocate_bytes(thispkt, mac_size, &mac) || !ssl->method->ssl3_enc->mac(s, thiswr, mac, 1)) { diff --git a/ssl/record/record.h b/ssl/record/record.h index 76b3314cb98..18a33d70dc0 100644 --- a/ssl/record/record.h +++ b/ssl/record/record.h @@ -226,10 +226,6 @@ __owur int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, size_t len, int peek, size_t *readbytes); __owur int ssl3_setup_buffers(SSL_CONNECTION *s); -__owur int ssl3_enc(SSL_CONNECTION *s, SSL3_RECORD *inrecs, size_t n_recs, - int send, SSL_MAC_BUF *mac, size_t macsize); -__owur int n_ssl3_mac(SSL_CONNECTION *s, SSL3_RECORD *rec, unsigned char *md, - int send); __owur int tls1_enc(SSL_CONNECTION *s, SSL3_RECORD *recs, size_t n_recs, int sending, SSL_MAC_BUF *mac, size_t macsize); __owur int tls1_mac_old(SSL_CONNECTION *s, SSL3_RECORD *rec, unsigned char *md, diff --git a/ssl/record/ssl3_record.c b/ssl/record/ssl3_record.c index fdf694a0e72..984b377cf75 100644 --- a/ssl/record/ssl3_record.c +++ b/ssl/record/ssl3_record.c @@ -15,24 +15,6 @@ #include "record_local.h" #include "internal/cryptlib.h" -static const unsigned char ssl3_pad_1[48] = { - 0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36, - 0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36, - 0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36, - 0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36, - 0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36, - 0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36 -}; - -static const unsigned char ssl3_pad_2[48] = { - 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, - 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, - 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, - 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, - 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, - 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c -}; - /* * Clear the contents of an SSL3_RECORD but retain any memory allocated */ @@ -161,85 +143,6 @@ int ssl3_do_compress(SSL_CONNECTION *sc, SSL3_RECORD *wr) return 1; } -/*- - * ssl3_enc encrypts/decrypts |n_recs| records in |inrecs|. Calls SSLfatal on - * internal error, but not otherwise. It is the responsibility of the caller to - * report a bad_record_mac - * - * Returns: - * 0: if the record is publicly invalid, or an internal error - * 1: Success or Mac-then-encrypt decryption failed (MAC will be randomised) - */ -int ssl3_enc(SSL_CONNECTION *s, SSL3_RECORD *inrecs, size_t n_recs, int sending, - SSL_MAC_BUF *mac, size_t macsize) -{ - SSL3_RECORD *rec; - EVP_CIPHER_CTX *ds; - size_t l, i; - size_t bs; - const EVP_CIPHER *enc; - - assert(sending); - rec = inrecs; - /* - * We shouldn't ever be called with more than one record in the SSLv3 case - */ - if (n_recs != 1) - return 0; - - ds = s->enc_write_ctx; - if (s->enc_write_ctx == NULL) - enc = NULL; - else - enc = EVP_CIPHER_CTX_get0_cipher(s->enc_write_ctx); - - if ((s->session == NULL) || (ds == NULL) || (enc == NULL)) { - memmove(rec->data, rec->input, rec->length); - rec->input = rec->data; - } else { - int provided = (EVP_CIPHER_get0_provider(enc) != NULL); - - l = rec->length; - bs = EVP_CIPHER_CTX_get_block_size(ds); - - /* COMPRESS */ - - if ((bs != 1) && !provided) { - /* - * We only do this for legacy ciphers. Provided ciphers add the - * padding on the provider side. - */ - i = bs - (l % bs); - - /* we need to add 'i-1' padding bytes */ - l += i; - /* - * the last of these zero bytes will be overwritten with the - * padding length. - */ - memset(&rec->input[rec->length], 0, i); - rec->length += i; - rec->input[l - 1] = (unsigned char)(i - 1); - } - - if (EVP_CIPHER_get0_provider(enc) != NULL) { - int outlen; - - if (!EVP_CipherUpdate(ds, rec->data, &outlen, rec->input, - (unsigned int)l)) - return 0; - rec->length = outlen; - } else { - if (EVP_Cipher(ds, rec->data, rec->input, (unsigned int)l) < 1) { - /* Shouldn't happen */ - SSLfatal(s, SSL_AD_BAD_RECORD_MAC, ERR_R_INTERNAL_ERROR); - return 0; - } - } - } - return 1; -} - #define MAX_PADDING 256 /*- * tls1_enc encrypts/decrypts |n_recs| in |recs|. Calls SSLfatal on internal @@ -465,66 +368,6 @@ int tls1_enc(SSL_CONNECTION *s, SSL3_RECORD *recs, size_t n_recs, int sending, return 1; } -int n_ssl3_mac(SSL_CONNECTION *sc, SSL3_RECORD *rec, unsigned char *md, - int sending) -{ - unsigned char *mac_sec, *seq; - const EVP_MD_CTX *hash; - unsigned char *p, rec_char; - size_t md_size; - size_t npad; - int t; - unsigned int md_size_u; - EVP_MD_CTX *md_ctx; - - /* - * All read record layer operations should have been moved to the new - * record layer code - */ - assert(sending); - - mac_sec = &(sc->s3.write_mac_secret[0]); - seq = RECORD_LAYER_get_write_sequence(&sc->rlayer); - hash = sc->write_hash; - - t = EVP_MD_CTX_get_size(hash); - if (t < 0) - return 0; - md_size = t; - npad = (48 / md_size) * md_size; - - /* Chop the digest off the end :-) */ - md_ctx = EVP_MD_CTX_new(); - - if (md_ctx == NULL) - return 0; - - rec_char = rec->type; - p = md; - s2n(rec->length, p); - if (EVP_MD_CTX_copy_ex(md_ctx, hash) <= 0 - || EVP_DigestUpdate(md_ctx, mac_sec, md_size) <= 0 - || EVP_DigestUpdate(md_ctx, ssl3_pad_1, npad) <= 0 - || EVP_DigestUpdate(md_ctx, seq, 8) <= 0 - || EVP_DigestUpdate(md_ctx, &rec_char, 1) <= 0 - || EVP_DigestUpdate(md_ctx, md, 2) <= 0 - || EVP_DigestUpdate(md_ctx, rec->input, rec->length) <= 0 - || EVP_DigestFinal_ex(md_ctx, md, NULL) <= 0 - || EVP_MD_CTX_copy_ex(md_ctx, hash) <= 0 - || EVP_DigestUpdate(md_ctx, mac_sec, md_size) <= 0 - || EVP_DigestUpdate(md_ctx, ssl3_pad_2, npad) <= 0 - || EVP_DigestUpdate(md_ctx, md, md_size) <= 0 - || EVP_DigestFinal_ex(md_ctx, md, &md_size_u) <= 0) { - EVP_MD_CTX_free(md_ctx); - return 0; - } - - EVP_MD_CTX_free(md_ctx); - - ssl3_record_sequence_update(seq); - return 1; -} - int tls1_mac_old(SSL_CONNECTION *sc, SSL3_RECORD *rec, unsigned char *md, int sending) { diff --git a/ssl/s3_enc.c b/ssl/s3_enc.c index d575d28dcdf..26471c3784b 100644 --- a/ssl/s3_enc.c +++ b/ssl/s3_enc.c @@ -92,13 +92,11 @@ int ssl3_change_cipher_state(SSL_CONNECTION *s, int which) unsigned char *p, *mac_secret; size_t md_len; unsigned char *key, *iv; - EVP_CIPHER_CTX *dd; const EVP_CIPHER *ciph; const SSL_COMP *comp = NULL; const EVP_MD *md; int mdi; size_t n, iv_len, key_len; - int reuse_dd = 0; int direction = (which & SSL3_CC_READ) != 0 ? OSSL_RECORD_DIRECTION_READ : OSSL_RECORD_DIRECTION_WRITE; @@ -145,6 +143,9 @@ int ssl3_change_cipher_state(SSL_CONNECTION *s, int which) goto err; } + if ((which & SSL3_CC_WRITE) != 0) + s->statem.enc_write_state = ENC_WRITE_STATE_INVALID; + if (!ssl_set_new_record_layer(s, SSL3_VERSION, direction, OSSL_RECORD_PROTECTION_LEVEL_APPLICATION, @@ -154,56 +155,6 @@ int ssl3_change_cipher_state(SSL_CONNECTION *s, int which) goto err; } - if (which & SSL3_CC_READ) { - s->statem.enc_write_state = ENC_WRITE_STATE_VALID; - return 1; - } - - s->statem.enc_write_state = ENC_WRITE_STATE_INVALID; - if (s->enc_write_ctx != NULL) { - reuse_dd = 1; - } else if ((s->enc_write_ctx = EVP_CIPHER_CTX_new()) == NULL) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_EVP_LIB); - goto err; - } else { - /* make sure it's initialised in case we exit later with an error */ - EVP_CIPHER_CTX_reset(s->enc_write_ctx); - } - dd = s->enc_write_ctx; - if (ssl_replace_hash(&s->write_hash, md) == NULL) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_SSL_LIB); - goto err; - } -#ifndef OPENSSL_NO_COMP - /* COMPRESS */ - COMP_CTX_free(s->compress); - s->compress = NULL; - if (comp != NULL) { - s->compress = COMP_CTX_new(comp->method); - if (s->compress == NULL) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, - SSL_R_COMPRESSION_LIBRARY_ERROR); - goto err; - } - } -#endif - RECORD_LAYER_reset_write_sequence(&s->rlayer); - memcpy(&(s->s3.write_mac_secret[0]), mac_secret, md_len); - - if (reuse_dd) - EVP_CIPHER_CTX_reset(dd); - - if (!EVP_CipherInit_ex(dd, ciph, NULL, key, iv, (which & SSL3_CC_WRITE))) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - goto err; - } - - if (EVP_CIPHER_get0_provider(ciph) != NULL - && !tls_provider_set_tls_params(s, dd, ciph, md)) { - /* SSLfatal already called */ - goto err; - } - s->statem.enc_write_state = ENC_WRITE_STATE_VALID; return 1; err: diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index 190f42cd0e0..0896be02329 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -3270,8 +3270,8 @@ static int sslcon_undefined_function_1(SSL_CONNECTION *sc, unsigned char *r, } const SSL3_ENC_METHOD SSLv3_enc_data = { - ssl3_enc, - n_ssl3_mac, + NULL, + NULL, ssl3_setup_key_block, ssl3_generate_master_secret, ssl3_change_cipher_state,