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/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,
 };