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@

diff --git a/lib/libssl/d1_lib.c b/lib/libssl/d1_lib.c
index ae6a665..69db8a0 100644 (file)
--- 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 d30eb6d..86b32ae 100644 (file)
--- 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 dce1411..2478d70 100644 (file)
--- 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 34197e5..4cbc13f 100644 (file)
--- 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 ca80da6..dee52de 100644 (file)
--- 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,
 };