kris/boringssl
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Reject empty cipher suite lists early.

Browse Source 
See upstream's 3ae91cfb327c9ed689b9aaf7bca01a3f5a0657cb.

I misread that code and thought it was allowing empty cipher suites when there
*is* a session ID, but it was allowing them when there isn't. Which doesn't
make much sense because it'll get rejected later anyway. (Verified by toying
with handshake_client.go.)

Change-Id: Ia870a1518bca36fce6f3018892254f53ab49f460
Reviewed-on: https://boringssl-review.googlesource.com/4401
Reviewed-by: Adam Langley <agl@google.com>
This commit is contained in:
David Benjamin 2015-04-17 22:33:25 -04:00 committed by Adam Langley
parent 3fa27774b4
commit 93de5e5c11
1 changed files with 3 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
ssl/s3_srvr.c
View File

@ -1093,6 +1093,8 @@ int ssl3_get_client_hello(SSL *s) {
} }
if (!CBS_get_u16_length_prefixed(&client_hello, &cipher_suites) || if (!CBS_get_u16_length_prefixed(&client_hello, &cipher_suites) ||
CBS_len(&cipher_suites) == 0 ||
CBS_len(&cipher_suites) % 2 != 0 ||
!CBS_get_u8_length_prefixed(&client_hello, &compression_methods) || !CBS_get_u8_length_prefixed(&client_hello, &compression_methods) ||
CBS_len(&compression_methods) == 0) { CBS_len(&compression_methods) == 0) {
al = SSL_AD_DECODE_ERROR; al = SSL_AD_DECODE_ERROR;
@ -1100,25 +1102,13 @@ int ssl3_get_client_hello(SSL *s) {
goto f_err; goto f_err;
} }
/* TODO(davidben): Per spec, cipher_suites can never be empty (specified at
* the ClientHello structure level). This logic allows it to be empty if
* resuming a session. Can we always require non-empty? If a client sends
* empty cipher_suites because it's resuming a session, it could always fail
* to resume a session, so it's unlikely to actually work. */
if (CBS_len(&cipher_suites) == 0 && CBS_len(&session_id) != 0) {
/* We need a cipher if we are not resuming a session. */
al = SSL_AD_ILLEGAL_PARAMETER;
OPENSSL_PUT_ERROR(SSL, ssl3_get_client_hello, SSL_R_NO_CIPHERS_SPECIFIED);
goto f_err;
}
ciphers = ssl_bytes_to_cipher_list(s, &cipher_suites); ciphers = ssl_bytes_to_cipher_list(s, &cipher_suites);
if (ciphers == NULL) { if (ciphers == NULL) {
goto err; goto err;
} }
/* If it is a hit, check that the cipher is in the list. */ /* If it is a hit, check that the cipher is in the list. */
if (s->hit && CBS_len(&cipher_suites) > 0) { if (s->hit) {
size_t j; size_t j;
int found_cipher = 0; int found_cipher = 0;
uint32_t id = s->session->cipher->id; uint32_t id = s->session->cipher->id;