From ae0a25824ab36826911195649772780808244ba2 Mon Sep 17 00:00:00 2001 From: jsing Date: Tue, 23 Jul 2024 14:40:53 +0000 Subject: [PATCH] Remove get_cipher from SSL_METHOD. Inline the get_cipher implementation (including the special handling for DTLS) in ssl_cipher_collect_ciphers() (the only consumer), remove the get_cipher member of SSL_METHOD and mop up dtls1_get_cipher(). ssl3_get_cipher() has always had a strange property of being a reverse index, which is relied on by the cipher list ordering code, since it currently assumes that high cipher suite values are preferable. Rather than complicating ssl3_get_cipher() (and regress), change the iteration order in ssl_cipher_collect_ciphers() to match what it requires. Lastly, rename ssl3_get_cipher() to be more descriptive. ok tb@ --- lib/libssl/d1_lib.c | 23 +------------------- lib/libssl/s3_lib.c | 12 +++++----- lib/libssl/ssl_ciph.c | 47 +++++++++++++++++++--------------------- lib/libssl/ssl_local.h | 6 ++--- lib/libssl/ssl_methods.c | 17 +-------------- 5 files changed, 32 insertions(+), 73 deletions(-) diff --git a/lib/libssl/d1_lib.c b/lib/libssl/d1_lib.c index ae6a6650ab1..69db8a0df4f 100644 --- a/lib/libssl/d1_lib.c +++ b/lib/libssl/d1_lib.c @@ -1,4 +1,4 @@ -/* $OpenBSD: d1_lib.c,v 1.64 2022/11/26 16:08:55 tb Exp $ */ +/* $OpenBSD: d1_lib.c,v 1.65 2024/07/23 14:40:53 jsing Exp $ */ /* * DTLS implementation written by Nagendra Modadugu * (nagendra@cs.stanford.edu) for the OpenSSL project 2005. @@ -250,27 +250,6 @@ dtls1_ctrl(SSL *s, int cmd, long larg, void *parg) return (ret); } -/* - * As it's impossible to use stream ciphers in "datagram" mode, this - * simple filter is designed to disengage them in DTLS. Unfortunately - * there is no universal way to identify stream SSL_CIPHER, so we have - * to explicitly list their SSL_* codes. Currently RC4 is the only one - * available, but if new ones emerge, they will have to be added... - */ -const SSL_CIPHER * -dtls1_get_cipher(unsigned int u) -{ - const SSL_CIPHER *cipher; - - if ((cipher = ssl3_get_cipher(u)) == NULL) - return NULL; - - if (cipher->algorithm_enc == SSL_RC4) - return NULL; - - return cipher; -} - void dtls1_start_timer(SSL *s) { diff --git a/lib/libssl/s3_lib.c b/lib/libssl/s3_lib.c index d30eb6deb70..86b32aec153 100644 --- a/lib/libssl/s3_lib.c +++ b/lib/libssl/s3_lib.c @@ -1,4 +1,4 @@ -/* $OpenBSD: s3_lib.c,v 1.256 2024/07/22 14:47:15 jsing Exp $ */ +/* $OpenBSD: s3_lib.c,v 1.257 2024/07/23 14:40:53 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -1127,12 +1127,12 @@ ssl3_num_ciphers(void) } const SSL_CIPHER * -ssl3_get_cipher(unsigned int u) +ssl3_get_cipher_by_index(int idx) { - if (u < SSL3_NUM_CIPHERS) - return (&(ssl3_ciphers[SSL3_NUM_CIPHERS - 1 - u])); - else - return (NULL); + if (idx < 0 || idx >= SSL3_NUM_CIPHERS) + return NULL; + + return &ssl3_ciphers[idx]; } static int diff --git a/lib/libssl/ssl_ciph.c b/lib/libssl/ssl_ciph.c index dce141101d4..2478d70eac4 100644 --- a/lib/libssl/ssl_ciph.c +++ b/lib/libssl/ssl_ciph.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_ciph.c,v 1.146 2024/07/22 14:47:15 jsing Exp $ */ +/* $OpenBSD: ssl_ciph.c,v 1.147 2024/07/23 14:40:53 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -576,22 +576,6 @@ ll_append_head(CIPHER_ORDER **head, CIPHER_ORDER *curr, *head = curr; } -/* XXX beck: remove this in a followon to removing GOST */ -static void -ssl_cipher_get_disabled(unsigned long *mkey, unsigned long *auth, - unsigned long *enc, unsigned long *mac, unsigned long *ssl) -{ - *mkey = 0; - *auth = 0; - *enc = 0; - *mac = 0; - *ssl = 0; - -#ifdef SSL_FORBID_ENULL - *enc |= SSL_eNULL; -#endif -} - static void ssl_cipher_collect_ciphers(const SSL_METHOD *ssl_method, int num_of_ciphers, unsigned long disabled_mkey, unsigned long disabled_auth, @@ -608,10 +592,15 @@ ssl_cipher_collect_ciphers(const SSL_METHOD *ssl_method, int num_of_ciphers, * a linked list with at most num entries. */ - /* Get the initial list of ciphers */ + /* + * Get the initial list of ciphers, iterating backwards over the + * cipher list - the list is ordered by cipher value and we currently + * hope that ciphers with higher cipher values are preferable... + */ co_list_num = 0; /* actual count of ciphers */ - for (i = 0; i < num_of_ciphers; i++) { - c = ssl_method->get_cipher(i); + for (i = num_of_ciphers - 1; i >= 0; i--) { + c = ssl3_get_cipher_by_index(i); + /* * Drop any invalid ciphers and any which use unavailable * algorithms. @@ -1153,11 +1142,19 @@ ssl_create_cipher_list(const SSL_METHOD *ssl_method, if (rule_str == NULL || cipher_list == NULL) goto err; - /* - * To reduce the work to do we only want to process the compiled - * in algorithms, so we first get the mask of disabled ciphers. - */ - ssl_cipher_get_disabled(&disabled_mkey, &disabled_auth, &disabled_enc, &disabled_mac, &disabled_ssl); + disabled_mkey = 0; + disabled_auth = 0; + disabled_enc = 0; + disabled_mac = 0; + disabled_ssl = 0; + +#ifdef SSL_FORBID_ENULL + disabled_enc |= SSL_eNULL; +#endif + + /* DTLS cannot be used with stream ciphers. */ + if (ssl_method->dtls) + disabled_enc |= SSL_RC4; /* * Now we have to collect the available ciphers from the compiled diff --git a/lib/libssl/ssl_local.h b/lib/libssl/ssl_local.h index 34197e5920d..4cbc13f8ac3 100644 --- a/lib/libssl/ssl_local.h +++ b/lib/libssl/ssl_local.h @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_local.h,v 1.22 2024/07/22 14:47:15 jsing Exp $ */ +/* $OpenBSD: ssl_local.h,v 1.23 2024/07/23 14:40:54 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -379,8 +379,6 @@ struct ssl_method_st { int peek); int (*ssl_write_bytes)(SSL *s, int type, const void *buf_, int len); - const SSL_CIPHER *(*get_cipher)(unsigned int ncipher); - unsigned int enc_flags; /* SSL_ENC_FLAG_* */ }; @@ -1290,7 +1288,7 @@ int ssl3_send_alert(SSL *s, int level, int desc); int ssl3_get_req_cert_types(SSL *s, CBB *cbb); int ssl3_get_message(SSL *s, int st1, int stn, int mt, long max); int ssl3_num_ciphers(void); -const SSL_CIPHER *ssl3_get_cipher(unsigned int u); +const SSL_CIPHER *ssl3_get_cipher_by_index(int idx); const SSL_CIPHER *ssl3_get_cipher_by_value(uint16_t value); int ssl3_renegotiate(SSL *ssl); diff --git a/lib/libssl/ssl_methods.c b/lib/libssl/ssl_methods.c index ca80da62fda..dee52decf14 100644 --- a/lib/libssl/ssl_methods.c +++ b/lib/libssl/ssl_methods.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_methods.c,v 1.31 2023/07/08 16:40:13 beck Exp $ */ +/* $OpenBSD: ssl_methods.c,v 1.32 2024/07/23 14:40:54 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -77,7 +77,6 @@ static const SSL_METHOD DTLS_method_data = { .ssl_pending = ssl3_pending, .ssl_read_bytes = dtls1_read_bytes, .ssl_write_bytes = dtls1_write_app_data_bytes, - .get_cipher = dtls1_get_cipher, .enc_flags = TLSV1_2_ENC_FLAGS, }; @@ -98,7 +97,6 @@ static const SSL_METHOD DTLS_client_method_data = { .ssl_pending = ssl3_pending, .ssl_read_bytes = dtls1_read_bytes, .ssl_write_bytes = dtls1_write_app_data_bytes, - .get_cipher = dtls1_get_cipher, .enc_flags = TLSV1_2_ENC_FLAGS, }; @@ -119,7 +117,6 @@ static const SSL_METHOD DTLSv1_method_data = { .ssl_pending = ssl3_pending, .ssl_read_bytes = dtls1_read_bytes, .ssl_write_bytes = dtls1_write_app_data_bytes, - .get_cipher = dtls1_get_cipher, .enc_flags = TLSV1_1_ENC_FLAGS, }; @@ -140,7 +137,6 @@ static const SSL_METHOD DTLSv1_client_method_data = { .ssl_pending = ssl3_pending, .ssl_read_bytes = dtls1_read_bytes, .ssl_write_bytes = dtls1_write_app_data_bytes, - .get_cipher = dtls1_get_cipher, .enc_flags = TLSV1_1_ENC_FLAGS, }; @@ -161,7 +157,6 @@ static const SSL_METHOD DTLSv1_2_method_data = { .ssl_pending = ssl3_pending, .ssl_read_bytes = dtls1_read_bytes, .ssl_write_bytes = dtls1_write_app_data_bytes, - .get_cipher = dtls1_get_cipher, .enc_flags = TLSV1_2_ENC_FLAGS, }; @@ -182,7 +177,6 @@ static const SSL_METHOD DTLSv1_2_client_method_data = { .ssl_pending = ssl3_pending, .ssl_read_bytes = dtls1_read_bytes, .ssl_write_bytes = dtls1_write_app_data_bytes, - .get_cipher = dtls1_get_cipher, .enc_flags = TLSV1_2_ENC_FLAGS, }; @@ -266,7 +260,6 @@ static const SSL_METHOD TLS_method_data = { .ssl_pending = tls13_legacy_pending, .ssl_read_bytes = tls13_legacy_read_bytes, .ssl_write_bytes = tls13_legacy_write_bytes, - .get_cipher = ssl3_get_cipher, .enc_flags = TLSV1_3_ENC_FLAGS, }; @@ -287,7 +280,6 @@ static const SSL_METHOD TLS_legacy_method_data = { .ssl_pending = ssl3_pending, .ssl_read_bytes = ssl3_read_bytes, .ssl_write_bytes = ssl3_write_bytes, - .get_cipher = ssl3_get_cipher, .enc_flags = TLSV1_2_ENC_FLAGS, }; @@ -308,7 +300,6 @@ static const SSL_METHOD TLS_client_method_data = { .ssl_pending = tls13_legacy_pending, .ssl_read_bytes = tls13_legacy_read_bytes, .ssl_write_bytes = tls13_legacy_write_bytes, - .get_cipher = ssl3_get_cipher, .enc_flags = TLSV1_3_ENC_FLAGS, }; @@ -329,7 +320,6 @@ static const SSL_METHOD TLSv1_method_data = { .ssl_pending = ssl3_pending, .ssl_read_bytes = ssl3_read_bytes, .ssl_write_bytes = ssl3_write_bytes, - .get_cipher = ssl3_get_cipher, .enc_flags = TLSV1_ENC_FLAGS, }; @@ -350,7 +340,6 @@ static const SSL_METHOD TLSv1_client_method_data = { .ssl_pending = ssl3_pending, .ssl_read_bytes = ssl3_read_bytes, .ssl_write_bytes = ssl3_write_bytes, - .get_cipher = ssl3_get_cipher, .enc_flags = TLSV1_ENC_FLAGS, }; @@ -371,7 +360,6 @@ static const SSL_METHOD TLSv1_1_method_data = { .ssl_pending = ssl3_pending, .ssl_read_bytes = ssl3_read_bytes, .ssl_write_bytes = ssl3_write_bytes, - .get_cipher = ssl3_get_cipher, .enc_flags = TLSV1_1_ENC_FLAGS, }; @@ -392,7 +380,6 @@ static const SSL_METHOD TLSv1_1_client_method_data = { .ssl_pending = ssl3_pending, .ssl_read_bytes = ssl3_read_bytes, .ssl_write_bytes = ssl3_write_bytes, - .get_cipher = ssl3_get_cipher, .enc_flags = TLSV1_1_ENC_FLAGS, }; @@ -413,7 +400,6 @@ static const SSL_METHOD TLSv1_2_method_data = { .ssl_pending = ssl3_pending, .ssl_read_bytes = ssl3_read_bytes, .ssl_write_bytes = ssl3_write_bytes, - .get_cipher = ssl3_get_cipher, .enc_flags = TLSV1_2_ENC_FLAGS, }; @@ -434,7 +420,6 @@ static const SSL_METHOD TLSv1_2_client_method_data = { .ssl_pending = ssl3_pending, .ssl_read_bytes = ssl3_read_bytes, .ssl_write_bytes = ssl3_write_bytes, - .get_cipher = ssl3_get_cipher, .enc_flags = TLSV1_2_ENC_FLAGS, }; -- 2.20.1