From: jsing Date: Sat, 20 Jul 2024 04:04:23 +0000 (+0000) Subject: Remove cipher from SSL_SESSION. X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=387303bbbb9d754b4fb827b18476432ebdba1f40;p=openbsd Remove cipher from SSL_SESSION. For a long time SSL_SESSION has had both a cipher ID and a pointer to an SSL_CIPHER (and not both are guaranteed to be populated). There is also a pointer to an SSL_CIPHER in the SSL_HANDSHAKE that denotes the cipher being used for this connection. Some code has been using the cipher from SSL_SESSION and some code has been using the cipher from SSL_HANDSHAKE. Remove cipher from SSL_SESSION and use the version in SSL_HANDSHAKE everywhere. If resuming from a session then we need to use the SSL_SESSION cipher ID to set the SSL_HANDSHAKE cipher. And we still need to ensure that we update the cipher ID in the SSL_SESSION whenever the SSL_HANDSHAKE cipher changes (this only occurs in a few places). ok tb@ --- diff --git a/lib/libssl/d1_pkt.c b/lib/libssl/d1_pkt.c index df9581a3cef..cf32ca8cd65 100644 --- a/lib/libssl/d1_pkt.c +++ b/lib/libssl/d1_pkt.c @@ -1,4 +1,4 @@ -/* $OpenBSD: d1_pkt.c,v 1.128 2023/07/02 20:16:47 tb Exp $ */ +/* $OpenBSD: d1_pkt.c,v 1.129 2024/07/20 04:04:23 jsing Exp $ */ /* * DTLS implementation written by Nagendra Modadugu * (nagendra@cs.stanford.edu) for the OpenSSL project 2005. @@ -596,7 +596,7 @@ dtls1_read_handshake_unexpected(SSL *s) * It should be impossible to hit this, but keep the safety * harness for now... */ - if (s->session == NULL || s->session->cipher == NULL) + if (s->session == NULL || s->s3->hs.cipher == NULL) return 1; /* @@ -650,7 +650,7 @@ dtls1_read_handshake_unexpected(SSL *s) return -1; } - if (s->session == NULL || s->session->cipher == NULL) { + if (s->session == NULL || s->s3->hs.cipher == NULL) { SSLerror(s, ERR_R_INTERNAL_ERROR); return -1; } diff --git a/lib/libssl/ssl_asn1.c b/lib/libssl/ssl_asn1.c index f4552f1c945..ef34cbdb04b 100644 --- a/lib/libssl/ssl_asn1.c +++ b/lib/libssl/ssl_asn1.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_asn1.c,v 1.67 2023/07/08 16:40:13 beck Exp $ */ +/* $OpenBSD: ssl_asn1.c,v 1.68 2024/07/20 04:04:23 jsing Exp $ */ /* * Copyright (c) 2016 Joel Sing * @@ -70,10 +70,7 @@ SSL_SESSION_encode(SSL_SESSION *s, unsigned char **out, size_t *out_len, goto err; /* Cipher suite ID. */ - /* XXX - require cipher to be non-NULL or always/only use cipher_id. */ cid = (uint16_t)(s->cipher_id & SSL3_CK_VALUE_MASK); - if (s->cipher != NULL) - cid = ssl3_cipher_get_value(s->cipher); if (!CBB_add_asn1(&session, &cipher_suite, CBS_ASN1_OCTETSTRING)) goto err; if (!CBB_add_u16(&cipher_suite, cid)) @@ -196,7 +193,7 @@ SSL_SESSION_ticket(SSL_SESSION *ss, unsigned char **out, size_t *out_len) if (ss == NULL) return 0; - if (ss->cipher == NULL && ss->cipher_id == 0) + if (ss->cipher_id == 0) return 0; return SSL_SESSION_encode(ss, out, out_len, 1); @@ -212,7 +209,7 @@ i2d_SSL_SESSION(SSL_SESSION *ss, unsigned char **pp) if (ss == NULL) return 0; - if (ss->cipher == NULL && ss->cipher_id == 0) + if (ss->cipher_id == 0) return 0; if (!SSL_SESSION_encode(ss, &data, &data_len, 0)) @@ -287,9 +284,6 @@ d2i_SSL_SESSION(SSL_SESSION **a, const unsigned char **pp, long length) goto err; if (CBS_len(&cipher_suite) != 0) goto err; - - /* XXX - populate cipher instead? */ - s->cipher = NULL; s->cipher_id = SSL3_CK_ID | cipher_value; /* Session ID. */ diff --git a/lib/libssl/ssl_ciph.c b/lib/libssl/ssl_ciph.c index 246d64e7d51..7c32354902b 100644 --- a/lib/libssl/ssl_ciph.c +++ b/lib/libssl/ssl_ciph.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_ciph.c,v 1.144 2024/07/16 14:38:04 jsing Exp $ */ +/* $OpenBSD: ssl_ciph.c,v 1.145 2024/07/20 04:04:23 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -408,25 +408,27 @@ static const SSL_CIPHER cipher_aliases[] = { }; int -ssl_cipher_get_evp(const SSL_SESSION *ss, const EVP_CIPHER **enc, - const EVP_MD **md, int *mac_pkey_type, int *mac_secret_size) +ssl_cipher_get_evp(SSL *s, const EVP_CIPHER **enc, const EVP_MD **md, + int *mac_pkey_type, int *mac_secret_size) { + const SSL_CIPHER *cipher; + *enc = NULL; *md = NULL; *mac_pkey_type = NID_undef; *mac_secret_size = 0; - if (ss->cipher == NULL) + if ((cipher = s->s3->hs.cipher) == NULL) return 0; /* * This function does not handle EVP_AEAD. * See ssl_cipher_get_evp_aead instead. */ - if (ss->cipher->algorithm_mac & SSL_AEAD) + if (cipher->algorithm_mac & SSL_AEAD) return 0; - switch (ss->cipher->algorithm_enc) { + switch (cipher->algorithm_enc) { case SSL_3DES: *enc = EVP_des_ede3_cbc(); break; @@ -450,7 +452,7 @@ ssl_cipher_get_evp(const SSL_SESSION *ss, const EVP_CIPHER **enc, break; } - switch (ss->cipher->algorithm_mac) { + switch (cipher->algorithm_mac) { case SSL_MD5: *md = EVP_md5(); break; @@ -487,16 +489,18 @@ ssl_cipher_get_evp(const SSL_SESSION *ss, const EVP_CIPHER **enc, * for s->cipher. It returns 1 on success and 0 on error. */ int -ssl_cipher_get_evp_aead(const SSL_SESSION *ss, const EVP_AEAD **aead) +ssl_cipher_get_evp_aead(SSL *s, const EVP_AEAD **aead) { + const SSL_CIPHER *cipher; + *aead = NULL; - if (ss->cipher == NULL) + if ((cipher = s->s3->hs.cipher) == NULL) return 0; - if ((ss->cipher->algorithm_mac & SSL_AEAD) == 0) + if ((cipher->algorithm_mac & SSL_AEAD) == 0) return 0; - switch (ss->cipher->algorithm_enc) { + switch (cipher->algorithm_enc) { case SSL_AES128GCM: *aead = EVP_aead_aes_128_gcm(); return 1; @@ -515,12 +519,14 @@ ssl_cipher_get_evp_aead(const SSL_SESSION *ss, const EVP_AEAD **aead) int ssl_get_handshake_evp_md(SSL *s, const EVP_MD **md) { + const SSL_CIPHER *cipher; + *md = NULL; - if (s->s3->hs.cipher == NULL) + if ((cipher = s->s3->hs.cipher) == NULL) return 0; - switch (s->s3->hs.cipher->algorithm2 & SSL_HANDSHAKE_MAC_MASK) { + switch (cipher->algorithm2 & SSL_HANDSHAKE_MAC_MASK) { case SSL_HANDSHAKE_MAC_SHA256: *md = EVP_sha256(); return 1; diff --git a/lib/libssl/ssl_clnt.c b/lib/libssl/ssl_clnt.c index 6cf0ee4a4a3..7b2e05d23d2 100644 --- a/lib/libssl/ssl_clnt.c +++ b/lib/libssl/ssl_clnt.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_clnt.c,v 1.166 2024/07/19 08:56:17 jsing Exp $ */ +/* $OpenBSD: ssl_clnt.c,v 1.167 2024/07/20 04:04:23 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -481,7 +481,7 @@ ssl3_connect(SSL *s) s->s3->hs.state = SSL3_ST_CW_FINISHED_A; s->init_num = 0; - s->session->cipher = s->s3->hs.cipher; + s->session->cipher_id = s->s3->hs.cipher->id; if (!tls1_setup_key_block(s)) { ret = -1; @@ -946,8 +946,8 @@ ssl3_get_server_hello(SSL *s) * client cannot change the cipher at this stage, * as the server has already made a selection. */ - if ((s->session->cipher = pref_cipher) == NULL) - s->session->cipher = + if ((s->s3->hs.cipher = pref_cipher) == NULL) + s->s3->hs.cipher = ssl3_get_cipher_by_value(cipher_suite); s->s3->flags |= SSL3_FLAGS_CCS_OK; } @@ -1016,14 +1016,13 @@ ssl3_get_server_hello(SSL *s) * and/or cipher_id values may not be set. Make sure that * cipher_id is set and use it for comparison. */ - if (s->session->cipher) - s->session->cipher_id = s->session->cipher->id; if (s->hit && (s->session->cipher_id != cipher->id)) { al = SSL_AD_ILLEGAL_PARAMETER; SSLerror(s, SSL_R_OLD_SESSION_CIPHER_NOT_RETURNED); goto fatal_err; } s->s3->hs.cipher = cipher; + s->session->cipher_id = cipher->id; if (!tls1_transcript_hash_init(s)) goto err; diff --git a/lib/libssl/ssl_lib.c b/lib/libssl/ssl_lib.c index 4b86b70db85..4cf5c46fda6 100644 --- a/lib/libssl/ssl_lib.c +++ b/lib/libssl/ssl_lib.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_lib.c,v 1.327 2024/07/19 08:54:31 jsing Exp $ */ +/* $OpenBSD: ssl_lib.c,v 1.328 2024/07/20 04:04:23 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -3073,11 +3073,10 @@ LSSL_ALIAS(SSL_get_privatekey); const SSL_CIPHER * SSL_get_current_cipher(const SSL *s) { - if ((s->session != NULL) && (s->session->cipher != NULL)) - return (s->session->cipher); - return (NULL); + return s->s3->hs.cipher; } LSSL_ALIAS(SSL_get_current_cipher); + const void * SSL_get_current_compression(SSL *s) { diff --git a/lib/libssl/ssl_local.h b/lib/libssl/ssl_local.h index e9b6a62bbe1..79f41e6dc36 100644 --- a/lib/libssl/ssl_local.h +++ b/lib/libssl/ssl_local.h @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_local.h,v 1.20 2024/07/19 08:54:31 jsing Exp $ */ +/* $OpenBSD: ssl_local.h,v 1.21 2024/07/20 04:04:23 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -438,7 +438,6 @@ struct ssl_session_st { time_t time; int references; - const SSL_CIPHER *cipher; unsigned long cipher_id; /* when ASN.1 loaded, this * needs to be used to load * the 'cipher' structure */ @@ -1267,9 +1266,9 @@ int ssl_merge_cipherlists(STACK_OF(SSL_CIPHER) *cipherlist, STACK_OF(SSL_CIPHER) *cipherlist_tls13, STACK_OF(SSL_CIPHER) **out_cipherlist); void ssl_update_cache(SSL *s, int mode); -int ssl_cipher_get_evp(const SSL_SESSION *s, const EVP_CIPHER **enc, +int ssl_cipher_get_evp(SSL *s, const EVP_CIPHER **enc, const EVP_MD **md, int *mac_pkey_type, int *mac_secret_size); -int ssl_cipher_get_evp_aead(const SSL_SESSION *s, const EVP_AEAD **aead); +int ssl_cipher_get_evp_aead(SSL *s, const EVP_AEAD **aead); int ssl_get_handshake_evp_md(SSL *s, const EVP_MD **md); int ssl_verify_cert_chain(SSL *s, STACK_OF(X509) *sk); diff --git a/lib/libssl/ssl_pkt.c b/lib/libssl/ssl_pkt.c index 2c33c45386d..7d6785a3de0 100644 --- a/lib/libssl/ssl_pkt.c +++ b/lib/libssl/ssl_pkt.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_pkt.c,v 1.66 2023/07/11 17:02:47 tb Exp $ */ +/* $OpenBSD: ssl_pkt.c,v 1.67 2024/07/20 04:04:23 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -904,7 +904,7 @@ ssl3_read_handshake_unexpected(SSL *s) * It should be impossible to hit this, but keep the safety * harness for now... */ - if (s->session == NULL || s->session->cipher == NULL) + if (s->session == NULL || s->s3->hs.cipher == NULL) return 1; /* @@ -953,7 +953,7 @@ ssl3_read_handshake_unexpected(SSL *s) return -1; } - if (s->session == NULL || s->session->cipher == NULL) { + if (s->session == NULL || s->s3->hs.cipher == NULL) { SSLerror(s, ERR_R_INTERNAL_ERROR); return -1; } @@ -1235,7 +1235,8 @@ ssl3_do_change_cipher_spec(SSL *s) return (0); } - s->session->cipher = s->s3->hs.cipher; + s->session->cipher_id = s->s3->hs.cipher->id; + if (!tls1_setup_key_block(s)) return (0); } diff --git a/lib/libssl/ssl_sess.c b/lib/libssl/ssl_sess.c index 76f194ca78c..c2bd1bf1351 100644 --- a/lib/libssl/ssl_sess.c +++ b/lib/libssl/ssl_sess.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_sess.c,v 1.126 2024/07/19 08:54:31 jsing Exp $ */ +/* $OpenBSD: ssl_sess.c,v 1.127 2024/07/20 04:04:23 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -287,7 +287,6 @@ ssl_session_dup(SSL_SESSION *sess, int include_ticket) copy->time = sess->time; copy->references = 1; - copy->cipher = sess->cipher; copy->cipher_id = sess->cipher_id; if (sess->tlsext_hostname != NULL) { @@ -707,12 +706,6 @@ ssl_get_prev_session(SSL *s, CBS *session_id, CBS *ext_block, int *alert) goto err; } - if (sess->cipher == NULL) { - sess->cipher = ssl3_get_cipher_by_id(sess->cipher_id); - if (sess->cipher == NULL) - goto err; - } - if (sess->timeout < (time(NULL) - sess->time)) { s->session_ctx->stats.sess_timeout++; if (!ticket_decrypted) { @@ -991,7 +984,7 @@ LSSL_ALIAS(SSL_SESSION_get_protocol_version); const SSL_CIPHER * SSL_SESSION_get0_cipher(const SSL_SESSION *s) { - return s->cipher; + return ssl3_get_cipher_by_id(s->cipher_id); } LSSL_ALIAS(SSL_SESSION_get0_cipher); diff --git a/lib/libssl/ssl_srvr.c b/lib/libssl/ssl_srvr.c index 01155a8d6d1..be6bd7402ca 100644 --- a/lib/libssl/ssl_srvr.c +++ b/lib/libssl/ssl_srvr.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_srvr.c,v 1.163 2024/07/19 08:56:17 jsing Exp $ */ +/* $OpenBSD: ssl_srvr.c,v 1.164 2024/07/20 04:04:23 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -651,7 +651,7 @@ ssl3_accept(SSL *s) goto end; s->s3->hs.state = SSL3_ST_SW_FINISHED_A; s->init_num = 0; - s->session->cipher = s->s3->hs.cipher; + s->session->cipher_id = s->s3->hs.cipher->id; if (!tls1_setup_key_block(s)) { ret = -1; @@ -978,7 +978,7 @@ ssl3_get_client_hello(SSL *s) /* XXX - CBS_len(&cipher_suites) will always be zero here... */ if (s->hit && CBS_len(&cipher_suites) > 0) { j = 0; - id = s->session->cipher->id; + id = s->session->cipher_id; for (i = 0; i < sk_SSL_CIPHER_num(ciphers); i++) { c = sk_SSL_CIPHER_value(ciphers, i); @@ -1098,7 +1098,7 @@ ssl3_get_client_hello(SSL *s) SSLerror(s, SSL_R_NO_SHARED_CIPHER); goto fatal_err; } - s->session->cipher = pref_cipher; + s->s3->hs.cipher = pref_cipher; /* XXX - why? */ sk_SSL_CIPHER_free(s->cipher_list); @@ -1127,8 +1127,11 @@ ssl3_get_client_hello(SSL *s) goto fatal_err; } s->s3->hs.cipher = c; + s->session->cipher_id = s->s3->hs.cipher->id; } else { - s->s3->hs.cipher = s->session->cipher; + s->s3->hs.cipher = ssl3_get_cipher_by_id(s->session->cipher_id); + if (s->s3->hs.cipher == NULL) + goto fatal_err; } if (!tls1_transcript_hash_init(s)) diff --git a/lib/libssl/ssl_txt.c b/lib/libssl/ssl_txt.c index ee3d218d68a..26b631d5abb 100644 --- a/lib/libssl/ssl_txt.c +++ b/lib/libssl/ssl_txt.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_txt.c,v 1.37 2023/07/08 16:40:13 beck Exp $ */ +/* $OpenBSD: ssl_txt.c,v 1.38 2024/07/20 04:04:23 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -108,6 +108,7 @@ LSSL_ALIAS(SSL_SESSION_print_fp); int SSL_SESSION_print(BIO *bp, const SSL_SESSION *x) { + const SSL_CIPHER *cipher; size_t i; int ret = 0; @@ -121,15 +122,15 @@ SSL_SESSION_print(BIO *bp, const SSL_SESSION *x) ssl_version_string(x->ssl_version)) <= 0) goto err; - if (x->cipher == NULL) { + if ((cipher = ssl3_get_cipher_by_id(x->cipher_id)) == NULL) { if (BIO_printf(bp, " Cipher : %04lX\n", x->cipher_id & SSL3_CK_VALUE_MASK) <= 0) goto err; } else { const char *cipher_name = "unknown"; - if (x->cipher->name != NULL) - cipher_name = x->cipher->name; + if (cipher->name != NULL) + cipher_name = cipher->name; if (BIO_printf(bp, " Cipher : %s\n", cipher_name) <= 0) goto err; diff --git a/lib/libssl/t1_enc.c b/lib/libssl/t1_enc.c index c6140e9b34b..64e1dd5b635 100644 --- a/lib/libssl/t1_enc.c +++ b/lib/libssl/t1_enc.c @@ -1,4 +1,4 @@ -/* $OpenBSD: t1_enc.c,v 1.157 2022/11/26 16:08:56 tb Exp $ */ +/* $OpenBSD: t1_enc.c,v 1.158 2024/07/20 04:04:23 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -357,15 +357,17 @@ tls1_setup_key_block(SSL *s) if (s->s3->hs.tls12.key_block != NULL) return (1); - if (s->session->cipher && - (s->session->cipher->algorithm_mac & SSL_AEAD)) { - if (!ssl_cipher_get_evp_aead(s->session, &aead)) { + if (s->s3->hs.cipher == NULL) + return (0); + + if ((s->s3->hs.cipher->algorithm_mac & SSL_AEAD) != 0) { + if (!ssl_cipher_get_evp_aead(s, &aead)) { SSLerror(s, SSL_R_CIPHER_OR_HASH_UNAVAILABLE); return (0); } } else { /* XXX - mac_type and mac_secret_size are now unused. */ - if (!ssl_cipher_get_evp(s->session, &cipher, &mac_hash, + if (!ssl_cipher_get_evp(s, &cipher, &mac_hash, &mac_type, &mac_secret_size)) { SSLerror(s, SSL_R_CIPHER_OR_HASH_UNAVAILABLE); return (0); @@ -395,12 +397,12 @@ tls1_setup_key_block(SSL *s) */ s->s3->need_empty_fragments = 1; - if (s->session->cipher != NULL) { - if (s->session->cipher->algorithm_enc == SSL_eNULL) + if (s->s3->hs.cipher != NULL) { + if (s->s3->hs.cipher->algorithm_enc == SSL_eNULL) s->s3->need_empty_fragments = 0; #ifndef OPENSSL_NO_RC4 - if (s->session->cipher->algorithm_enc == SSL_RC4) + if (s->s3->hs.cipher->algorithm_enc == SSL_RC4) s->s3->need_empty_fragments = 0; #endif } diff --git a/lib/libssl/tls13_client.c b/lib/libssl/tls13_client.c index 053cf1689b1..8f6894fd888 100644 --- a/lib/libssl/tls13_client.c +++ b/lib/libssl/tls13_client.c @@ -1,4 +1,4 @@ -/* $OpenBSD: tls13_client.c,v 1.102 2023/06/10 15:34:36 tb Exp $ */ +/* $OpenBSD: tls13_client.c,v 1.103 2024/07/20 04:04:23 jsing Exp $ */ /* * Copyright (c) 2018, 2019 Joel Sing * @@ -347,7 +347,7 @@ tls13_client_engage_record_protection(struct tls13_ctx *ctx) &shared_key_len)) goto err; - s->session->cipher = ctx->hs->cipher; + s->session->cipher_id = ctx->hs->cipher->id; s->session->ssl_version = ctx->hs->tls13.server_version; if ((ctx->aead = tls13_cipher_aead(ctx->hs->cipher)) == NULL) diff --git a/lib/libssl/tls13_server.c b/lib/libssl/tls13_server.c index f9cdbdd690e..6bd2993cf3a 100644 --- a/lib/libssl/tls13_server.c +++ b/lib/libssl/tls13_server.c @@ -1,4 +1,4 @@ -/* $OpenBSD: tls13_server.c,v 1.107 2024/07/19 08:54:31 jsing Exp $ */ +/* $OpenBSD: tls13_server.c,v 1.108 2024/07/20 04:04:23 jsing Exp $ */ /* * Copyright (c) 2019, 2020 Joel Sing * Copyright (c) 2020 Bob Beck @@ -383,7 +383,7 @@ tls13_server_engage_record_protection(struct tls13_ctx *ctx) &shared_key_len)) goto err; - s->session->cipher = ctx->hs->cipher; + s->session->cipher_id = ctx->hs->cipher->id; if ((ctx->aead = tls13_cipher_aead(ctx->hs->cipher)) == NULL) goto err;