Remove get_cipher_by_char and put_cipher_by_char.
Without SSLv2, all cipher suite values are 2 bytes. Represent them as a
uint16_t and make all functions pass those around rather than pointers.
This removes SSL_CIPHER_find as it's unused.
Change-Id: Iea0b75abee4352a8333a4b8e39a161430ae55ea6
Reviewed-on: https://boringssl-review.googlesource.com/1259
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/s23_clnt.c b/ssl/s23_clnt.c
index c95ea47..d7026f9 100644
--- a/ssl/s23_clnt.c
+++ b/ssl/s23_clnt.c
@@ -383,7 +383,7 @@
*(p++) = 0;
/* Ciphers supported (using SSL 3.0/TLS 1.0 format) */
- i=ssl_cipher_list_to_bytes(s,SSL_get_ciphers(s),&(p[2]),ssl3_put_cipher_by_char);
+ i = ssl_cipher_list_to_bytes(s, SSL_get_ciphers(s), &p[2]);
if (i == 0)
{
OPENSSL_PUT_ERROR(SSL, ssl23_client_hello, SSL_R_NO_CIPHERS_AVAILABLE);
diff --git a/ssl/s23_lib.c b/ssl/s23_lib.c
index 69b4c64..ab5b749 100644
--- a/ssl/s23_lib.c
+++ b/ssl/s23_lib.c
@@ -82,31 +82,6 @@
return(NULL);
}
-/* This function needs to check if the ciphers required are actually
- * available */
-const SSL_CIPHER *ssl23_get_cipher_by_char(const unsigned char *p)
- {
- const SSL_CIPHER *cp;
-
- cp=ssl3_get_cipher_by_char(p);
- return(cp);
- }
-
-int ssl23_put_cipher_by_char(const SSL_CIPHER *c, unsigned char *p)
- {
- long l;
-
- /* We can write SSLv2 and SSLv3 ciphers */
- if (p != NULL)
- {
- l=c->id;
- p[0]=((unsigned char)(l>>16L))&0xFF;
- p[1]=((unsigned char)(l>> 8L))&0xFF;
- p[2]=((unsigned char)(l ))&0xFF;
- }
- return(3);
- }
-
int ssl23_read(SSL *s, void *buf, int len)
{
int n;
diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index a191f9f..28f65c7 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -780,7 +780,7 @@
}
/* Ciphers supported */
- i=ssl_cipher_list_to_bytes(s,SSL_get_ciphers(s),&(p[2]),0);
+ i = ssl_cipher_list_to_bytes(s, SSL_get_ciphers(s), &p[2]);
if (i == 0)
{
OPENSSL_PUT_ERROR(SSL, ssl3_client_hello, SSL_R_NO_CIPHERS_AVAILABLE);
@@ -833,8 +833,7 @@
int al=SSL_AD_INTERNAL_ERROR,ok;
long n;
CBS server_hello, server_random, session_id;
- uint16_t server_version;
- const uint8_t *cipher_ptr;
+ uint16_t server_version, cipher_suite;
uint8_t compression_method;
/* Hello verify request and/or server hello version may not
* match so set first packet if we're negotiating version.
@@ -882,7 +881,9 @@
if (!CBS_get_u16(&server_hello, &server_version) ||
!CBS_get_bytes(&server_hello, &server_random, SSL3_RANDOM_SIZE) ||
!CBS_get_u8_length_prefixed(&server_hello, &session_id) ||
- CBS_len(&session_id) > SSL3_SESSION_ID_SIZE)
+ CBS_len(&session_id) > SSL3_SESSION_ID_SIZE ||
+ !CBS_get_u16(&server_hello, &cipher_suite) ||
+ !CBS_get_u8(&server_hello, &compression_method))
{
al = SSL_AD_DECODE_ERROR;
OPENSSL_PUT_ERROR(SSL, ssl3_get_server_hello, SSL_R_DECODE_ERROR);
@@ -939,11 +940,9 @@
NULL, &pref_cipher,
s->tls_session_secret_cb_arg))
{
- /* TODO(davidben): Make ssl_get_cipher_by_char
- * a bounds-checked function. */
s->session->cipher = pref_cipher ?
pref_cipher :
- ssl_get_cipher_by_char(s, CBS_data(&server_hello));
+ ssl3_get_cipher_by_value(cipher_suite);
s->s3->flags |= SSL3_FLAGS_CCS_OK;
s->hit = 1;
}
@@ -982,16 +981,7 @@
memcpy(s->session->session_id, CBS_data(&session_id), CBS_len(&session_id));
}
- /* TODO(davidben): Move the cipher_by_char hooks to CBS or
- * something else actually bounds-checked. */
- cipher_ptr = CBS_data(&server_hello);
- if (!CBS_skip(&server_hello, 2))
- {
- al = SSL_AD_DECODE_ERROR;
- OPENSSL_PUT_ERROR(SSL, ssl3_get_server_hello, SSL_R_DECODE_ERROR);
- goto f_err;
- }
- c = ssl_get_cipher_by_char(s, cipher_ptr);
+ c = ssl3_get_cipher_by_value(cipher_suite);
if (c == NULL)
{
/* unknown cipher */
@@ -1038,12 +1028,6 @@
if (!SSL_USE_SIGALGS(s) && !ssl3_digest_cached_records(s))
goto f_err;
- if (!CBS_get_u8(&server_hello, &compression_method))
- {
- al = SSL_AD_DECODE_ERROR;
- OPENSSL_PUT_ERROR(SSL, ssl3_get_server_hello, SSL_R_DECODE_ERROR);
- }
-
/* Only the NULL compression algorithm is supported. */
if (compression_method != 0)
{
diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index fa56a96..79dcc88 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -146,6 +146,7 @@
* OTHER ENTITY BASED ON INFRINGEMENT OF INTELLECTUAL PROPERTY RIGHTS OR
* OTHERWISE. */
+#include <assert.h>
#include <stdio.h>
#include <openssl/buf.h>
@@ -3566,35 +3567,26 @@
return(1);
}
-/* This function needs to check if the ciphers required are actually
- * available */
-const SSL_CIPHER *ssl3_get_cipher_by_char(const unsigned char *p)
+/* ssl3_get_cipher_by_value returns the SSL_CIPHER with value |value| or NULL if
+ * none exists.
+ *
+ * This function needs to check if the ciphers required are actually
+ * available. */
+const SSL_CIPHER *ssl3_get_cipher_by_value(uint16_t value)
{
SSL_CIPHER c;
- const SSL_CIPHER *cp;
- unsigned long id;
- id=0x03000000L|((unsigned long)p[0]<<8L)|(unsigned long)p[1];
- c.id=id;
- cp = bsearch(&c, ssl3_ciphers, SSL3_NUM_CIPHERS, sizeof(SSL_CIPHER), ssl_cipher_id_cmp);
-#ifdef DEBUG_PRINT_UNKNOWN_CIPHERSUITES
-if (cp == NULL) fprintf(stderr, "Unknown cipher ID %x\n", (p[0] << 8) | p[1]);
-#endif
- return cp;
+ c.id = 0x03000000L|value;
+ return bsearch(&c, ssl3_ciphers, SSL3_NUM_CIPHERS, sizeof(SSL_CIPHER), ssl_cipher_id_cmp);
}
-int ssl3_put_cipher_by_char(const SSL_CIPHER *c, unsigned char *p)
+/* ssl3_get_cipher_by_value returns the cipher value of |c|. */
+uint16_t ssl3_get_cipher_value(const SSL_CIPHER *c)
{
- long l;
-
- if (p != NULL)
- {
- l=c->id;
- if ((l & 0xff000000) != 0x03000000) return(0);
- p[0]=((unsigned char)(l>> 8L))&0xFF;
- p[1]=((unsigned char)(l ))&0xFF;
- }
- return(2);
+ unsigned long id = c->id;
+ /* All ciphers are SSLv3 now. */
+ assert((id & 0xff000000) == 0x03000000);
+ return id & 0xffff;
}
struct ssl_cipher_preference_list_st* ssl_get_cipher_preferences(SSL *s)
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index 4b82ded..4441a39 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -1118,9 +1118,7 @@
goto f_err;
}
- /* TODO(davidben): Port cipher-list handling to CBS. */
- if (ssl_bytes_to_cipher_list(s, CBS_data(&cipher_suites),
- CBS_len(&cipher_suites), &ciphers) == NULL)
+ if (ssl_bytes_to_cipher_list(s, &cipher_suites, &ciphers) == NULL)
{
goto err;
}
@@ -1343,7 +1341,7 @@
{
unsigned char *buf;
unsigned char *p,*d;
- int i,sl;
+ int sl;
unsigned long l;
if (s->state == SSL3_ST_SW_SRVR_HELLO_A)
@@ -1417,8 +1415,7 @@
p+=sl;
/* put the cipher */
- i=ssl3_put_cipher_by_char(s->s3->tmp.new_cipher,p);
- p+=i;
+ s2n(ssl3_get_cipher_value(s->s3->tmp.new_cipher), p);
/* put the compression method */
*(p++)=0;
diff --git a/ssl/ssl_ciph.c b/ssl/ssl_ciph.c
index 28287a2..bece2cd 100644
--- a/ssl/ssl_ciph.c
+++ b/ssl/ssl_ciph.c
@@ -1854,20 +1854,6 @@
return -1;
}
-const SSL_CIPHER *ssl_get_cipher_by_char(SSL *ssl, const unsigned char *ptr)
- {
- const SSL_CIPHER *c;
- c = ssl->method->get_cipher_by_char(ptr);
- if (c == NULL || c->valid == 0)
- return NULL;
- return c;
- }
-
-const SSL_CIPHER *SSL_CIPHER_find(SSL *ssl, const unsigned char *ptr)
- {
- return ssl->method->get_cipher_by_char(ptr);
- }
-
/* ssl_cipher_has_server_public_key returns 1 if |cipher| involves a
* server public key in the key exchange, sent in a server Certificate
* message. Otherwise it returns 0. */
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 477bb97..4ebea58 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -141,6 +141,7 @@
#include <stdio.h>
#include <assert.h>
+#include <openssl/bytestring.h>
#include <openssl/dh.h>
#include <openssl/engine.h>
#include <openssl/lhash.h>
@@ -1198,7 +1199,11 @@
return (int)s->cert->ciphers_rawlen;
}
else
- return ssl_put_cipher_by_char(s,NULL,NULL);
+ {
+ /* Passing a NULL |parg| returns the size of a single
+ * cipher suite value. */
+ return 2;
+ }
default:
return(s->method->ssl_ctrl(s,cmd,larg,parg));
}
@@ -1504,10 +1509,9 @@
return(buf);
}
-int ssl_cipher_list_to_bytes(SSL *s,STACK_OF(SSL_CIPHER) *sk,unsigned char *p,
- int (*put_cb)(const SSL_CIPHER *, unsigned char *))
+int ssl_cipher_list_to_bytes(SSL *s,STACK_OF(SSL_CIPHER) *sk,unsigned char *p)
{
- int i,j=0;
+ int i;
SSL_CIPHER *c;
CERT *ct = s->cert;
unsigned char *q;
@@ -1518,9 +1522,6 @@
if (sk == NULL) return(0);
q=p;
- if (put_cb == NULL)
- put_cb = s->method->put_cipher_by_char;
-
for (i=0; i<sk_SSL_CIPHER_num(sk); i++)
{
c=sk_SSL_CIPHER_value(sk,i);
@@ -1538,8 +1539,7 @@
no_scsv = 1;
}
#endif
- j = put_cb(c, p);
- p += j;
+ s2n(ssl3_get_cipher_value(c), p);
}
/* If p == q, no ciphers and caller indicates an error. Otherwise
* add SCSV if not renegotiating.
@@ -1552,8 +1552,7 @@
{
0, NULL, SSL3_CK_SCSV, 0, 0, 0, 0, 0, 0, 0, 0, 0
};
- j = put_cb(&scsv, p);
- p += j;
+ s2n(ssl3_get_cipher_value(&scsv), p);
#ifdef OPENSSL_RI_DEBUG
fprintf(stderr, "SCSV sent by client\n");
#endif
@@ -1564,25 +1563,24 @@
{
0, NULL, SSL3_CK_FALLBACK_SCSV, 0, 0, 0, 0, 0, 0, 0, 0, 0
};
- j = put_cb(&fallback_scsv, p);
- p += j;
+ s2n(ssl3_get_cipher_value(&fallback_scsv), p);
}
}
return(p-q);
}
-STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *s, const uint8_t *p,int num,
+STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *s, const CBS *cbs,
STACK_OF(SSL_CIPHER) **skp)
{
+ CBS cipher_suites = *cbs;
const SSL_CIPHER *c;
STACK_OF(SSL_CIPHER) *sk;
- int i,n;
+
if (s->s3)
s->s3->send_connection_binding = 0;
- n=ssl_put_cipher_by_char(s,NULL,NULL);
- if ((num%n) != 0)
+ if (CBS_len(&cipher_suites) % 2 != 0)
{
OPENSSL_PUT_ERROR(SSL, ssl_bytes_to_cipher_list, SSL_R_ERROR_IN_RECEIVED_CIPHER_LIST);
return(NULL);
@@ -1595,22 +1593,25 @@
sk_SSL_CIPHER_zero(sk);
}
- if (s->cert->ciphers_raw)
- OPENSSL_free(s->cert->ciphers_raw);
- s->cert->ciphers_raw = BUF_memdup(p, num);
- if (s->cert->ciphers_raw == NULL)
+ if (!CBS_stow(&cipher_suites,
+ &s->cert->ciphers_raw, &s->cert->ciphers_rawlen))
{
OPENSSL_PUT_ERROR(SSL, ssl_bytes_to_cipher_list, ERR_R_MALLOC_FAILURE);
goto err;
}
- s->cert->ciphers_rawlen = (size_t)num;
- for (i=0; i<num; i+=n)
+ while (CBS_len(&cipher_suites) > 0)
{
+ uint16_t cipher_suite;
+
+ if (!CBS_get_u16(&cipher_suites, &cipher_suite))
+ {
+ OPENSSL_PUT_ERROR(SSL, ssl_bytes_to_cipher_list, ERR_R_INTERNAL_ERROR);
+ goto err;
+ }
+
/* Check for SCSV */
- if (s->s3 && (n != 3 || !p[0]) &&
- (p[n-2] == ((SSL3_CK_SCSV >> 8) & 0xff)) &&
- (p[n-1] == (SSL3_CK_SCSV & 0xff)))
+ if (s->s3 && cipher_suite == (SSL3_CK_SCSV & 0xffff))
{
/* SCSV fatal if renegotiating */
if (s->renegotiate)
@@ -1620,7 +1621,6 @@
goto err;
}
s->s3->send_connection_binding = 1;
- p += n;
#ifdef OPENSSL_RI_DEBUG
fprintf(stderr, "SCSV received by server\n");
#endif
@@ -1628,9 +1628,7 @@
}
/* Check for FALLBACK_SCSV */
- if (s->s3 && n == 2 &&
- (p[0] == ((SSL3_CK_FALLBACK_SCSV >> 8) & 0xff)) &&
- (p[1] == (SSL3_CK_FALLBACK_SCSV & 0xff)) &&
+ if (s->s3 && cipher_suite == (SSL3_CK_FALLBACK_SCSV & 0xffff) &&
s->version < ssl_get_max_version(s))
{
OPENSSL_PUT_ERROR(SSL, ssl_bytes_to_cipher_list, SSL_R_INAPPROPRIATE_FALLBACK);
@@ -1638,8 +1636,7 @@
goto err;
}
- c=ssl_get_cipher_by_char(s,p);
- p+=n;
+ c = ssl3_get_cipher_by_value(cipher_suite);
if (c != NULL)
{
if (!sk_SSL_CIPHER_push(sk,c))
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 4c1e0e7..78432e0 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -668,8 +668,6 @@
/*#define IDEA_DEBUG */
#define FP_ICC (int (*)(const void *,const void *))
-#define ssl_put_cipher_by_char(ssl,ciph,ptr) \
- ((ssl)->method->put_cipher_by_char((ciph),(ptr)))
/* This is for the SSLv3/TLSv1.0 differences in crypto/hash stuff
* It is a bit of a mess of functions, but hell, think of it as
@@ -790,8 +788,6 @@
ssl3_dispatch_alert, \
ssl3_ctrl, \
ssl3_ctx_ctrl, \
- ssl3_get_cipher_by_char, \
- ssl3_put_cipher_by_char, \
ssl3_pending, \
ssl3_num_ciphers, \
ssl3_get_cipher, \
@@ -827,8 +823,6 @@
ssl3_dispatch_alert, \
ssl3_ctrl, \
ssl3_ctx_ctrl, \
- ssl3_get_cipher_by_char, \
- ssl3_put_cipher_by_char, \
ssl3_pending, \
ssl3_num_ciphers, \
ssl3_get_cipher, \
@@ -864,8 +858,6 @@
ssl3_dispatch_alert, \
ssl3_ctrl, \
ssl3_ctx_ctrl, \
- ssl23_get_cipher_by_char, \
- ssl23_put_cipher_by_char, \
ssl_undefined_const_function, \
ssl23_num_ciphers, \
ssl23_get_cipher, \
@@ -901,8 +893,6 @@
NULL, /* NULL - dispatch_alert */ \
ssl2_ctrl, /* local */ \
ssl2_ctx_ctrl, /* local */ \
- ssl2_get_cipher_by_char, \
- ssl2_put_cipher_by_char, \
ssl2_pending, \
ssl2_num_ciphers, \
ssl2_get_cipher, \
@@ -939,8 +929,6 @@
dtls1_dispatch_alert, \
dtls1_ctrl, \
ssl3_ctx_ctrl, \
- ssl3_get_cipher_by_char, \
- ssl3_put_cipher_by_char, \
ssl3_pending, \
ssl3_num_ciphers, \
dtls1_get_cipher, \
@@ -969,10 +957,9 @@
int ssl_get_prev_session(SSL *s, const struct ssl_early_callback_ctx *ctx);
int ssl_cipher_id_cmp(const void *in_a, const void *in_b);
int ssl_cipher_ptr_id_cmp(const SSL_CIPHER **ap, const SSL_CIPHER **bp);
-STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *s, const uint8_t *p,int num,
+STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *s, const CBS *cbs,
STACK_OF(SSL_CIPHER) **skp);
-int ssl_cipher_list_to_bytes(SSL *s,STACK_OF(SSL_CIPHER) *sk,unsigned char *p,
- int (*put_cb)(const SSL_CIPHER *, unsigned char *));
+int ssl_cipher_list_to_bytes(SSL *s, STACK_OF(SSL_CIPHER) *sk, unsigned char *p);
STACK_OF(SSL_CIPHER) *ssl_create_cipher_list(const SSL_METHOD *meth,
struct ssl_cipher_preference_list_st **pref,
STACK_OF(SSL_CIPHER) **sorted,
@@ -992,7 +979,6 @@
int ssl_get_handshake_digest(int i,long *mask,const EVP_MD **md);
int ssl_get_handshake_digest(int i,long *mask,const EVP_MD **md);
int ssl_cipher_get_cert_index(const SSL_CIPHER *c);
-const SSL_CIPHER *ssl_get_cipher_by_char(SSL *ssl, const unsigned char *ptr);
int ssl_cipher_has_server_public_key(const SSL_CIPHER *cipher);
int ssl_cipher_requires_server_key_exchange(const SSL_CIPHER *cipher);
@@ -1048,8 +1034,8 @@
int ssl2_pending(const SSL *s);
long ssl2_default_timeout(void );
-const SSL_CIPHER *ssl3_get_cipher_by_char(const unsigned char *p);
-int ssl3_put_cipher_by_char(const SSL_CIPHER *c,unsigned char *p);
+const SSL_CIPHER *ssl3_get_cipher_by_value(uint16_t value);
+uint16_t ssl3_get_cipher_value(const SSL_CIPHER *c);
void ssl3_init_finished_mac(SSL *s);
int ssl3_send_server_certificate(SSL *s);
int ssl3_send_newsession_ticket(SSL *s);
@@ -1115,8 +1101,6 @@
int ssl23_read(SSL *s, void *buf, int len);
int ssl23_peek(SSL *s, void *buf, int len);
int ssl23_write(SSL *s, const void *buf, int len);
-int ssl23_put_cipher_by_char(const SSL_CIPHER *c, unsigned char *p);
-const SSL_CIPHER *ssl23_get_cipher_by_char(const unsigned char *p);
long ssl23_default_timeout(void );
long tls1_default_timeout(void);
diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c
index 579627c..97ef3f8 100644
--- a/ssl/ssl_sess.c
+++ b/ssl/ssl_sess.c
@@ -560,16 +560,9 @@
if (ret->cipher == NULL)
{
- unsigned char buf[5],*p;
- unsigned long l;
-
- p=buf;
- l=ret->cipher_id;
- l2n(l,p);
- if ((ret->ssl_version>>8) >= SSL3_VERSION_MAJOR)
- ret->cipher=ssl_get_cipher_by_char(s,&(buf[2]));
- else
- ret->cipher=ssl_get_cipher_by_char(s,&(buf[1]));
+ /* The cipher id has a leading 0x03 to be removed (and then put
+ * back for the binary search) as a remnant of SSLv2 support. */
+ ret->cipher = ssl3_get_cipher_by_value(ret->cipher_id & 0xffff);
if (ret->cipher == NULL)
goto err;
}
