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@

5 files changed, 32 insertions, 73 deletions

diff --git a/src/lib/libssl/d1_lib.c b/src/lib/libssl/d1_lib.c
index ae6a6650ab..69db8a0df4 100644
--- a/src/lib/libssl/d1_lib.c
+++ b/src/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/src/lib/libssl/s3_lib.c b/src/lib/libssl/s3_lib.c
index d30eb6deb7..86b32aec15 100644
--- a/src/lib/libssl/s3_lib.c
+++ b/src/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/src/lib/libssl/ssl_ciph.c b/src/lib/libssl/ssl_ciph.c
index dce141101d..2478d70eac 100644
--- a/src/lib/libssl/ssl_ciph.c
+++ b/src/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/src/lib/libssl/ssl_local.h b/src/lib/libssl/ssl_local.h
index 34197e5920..4cbc13f8ac 100644
--- a/src/lib/libssl/ssl_local.h
+++ b/src/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/src/lib/libssl/ssl_methods.c b/src/lib/libssl/ssl_methods.c
index ca80da62fd..dee52decf1 100644
--- a/src/lib/libssl/ssl_methods.c
+++ b/src/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,
 };