Port ssl3_get_server_hello to CBS. Change-Id: I8ec531cf327653b4779ff6a29cac62e240502269 Reviewed-on: https://boringssl-review.googlesource.com/1167 Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c index e590832..837d8ff 100644 --- a/ssl/s3_clnt.c +++ b/ssl/s3_clnt.c
@@ -867,11 +867,12 @@ STACK_OF(SSL_CIPHER) *sk; const SSL_CIPHER *c; CERT *ct = s->cert; - unsigned char *p,*d; int al=SSL_AD_INTERNAL_ERROR,ok; - unsigned int j; long n; - CBS cbs; + CBS server_hello, server_random, session_id; + uint16_t server_version; + const uint8_t *cipher_ptr; + uint8_t compression_method; /* Hello verify request and/or server hello version may not * match so set first packet if we're negotiating version. */ @@ -913,58 +914,55 @@ goto f_err; } - d = p = s->init_msg; + CBS_init(&server_hello, s->init_msg, n); + + 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) + { + al = SSL_AD_DECODE_ERROR; + OPENSSL_PUT_ERROR(SSL, ssl3_get_server_hello, SSL_R_DECODE_ERROR); + goto f_err; + } + if (s->method->version == DTLS_ANY_VERSION) { /* Work out correct protocol version to use */ - int hversion = (p[0] << 8)|p[1]; int options = s->options; - if (hversion == DTLS1_2_VERSION + if (server_version == DTLS1_2_VERSION && !(options & SSL_OP_NO_DTLSv1_2)) s->method = DTLSv1_2_client_method(); else if (tls1_suiteb(s)) { OPENSSL_PUT_ERROR(SSL, ssl3_get_server_hello, SSL_R_ONLY_DTLS_1_2_ALLOWED_IN_SUITEB_MODE); - s->version = hversion; + s->version = server_version; al = SSL_AD_PROTOCOL_VERSION; goto f_err; } - else if (hversion == DTLS1_VERSION + else if (server_version == DTLS1_VERSION && !(options & SSL_OP_NO_DTLSv1)) s->method = DTLSv1_client_method(); else { OPENSSL_PUT_ERROR(SSL, ssl3_get_server_hello, SSL_R_WRONG_SSL_VERSION); - s->version = hversion; + s->version = server_version; al = SSL_AD_PROTOCOL_VERSION; goto f_err; } s->version = s->client_version = s->method->version; } - if ((p[0] != (s->version>>8)) || (p[1] != (s->version&0xff))) + if (server_version != s->version) { OPENSSL_PUT_ERROR(SSL, ssl3_get_server_hello, SSL_R_WRONG_SSL_VERSION); - s->version=(s->version&0xff00)|p[1]; - al=SSL_AD_PROTOCOL_VERSION; + s->version = (s->version & 0xff00) | (server_version & 0xff); + al = SSL_AD_PROTOCOL_VERSION; goto f_err; } - p+=2; - /* load the server hello data */ - /* load the server random */ - memcpy(s->s3->server_random,p,SSL3_RANDOM_SIZE); - p+=SSL3_RANDOM_SIZE; - - /* get the session-id */ - j= *(p++); - - if ((j > sizeof s->session->session_id) || (j > SSL3_SESSION_ID_SIZE)) - { - al=SSL_AD_ILLEGAL_PARAMETER; - OPENSSL_PUT_ERROR(SSL, ssl3_get_server_hello, SSL_R_SSL3_SESSION_ID_TOO_LONG); - goto f_err; - } + /* Copy over the server random. */ + memcpy(s->s3->server_random, CBS_data(&server_random), SSL3_RANDOM_SIZE); /* check if we want to resume the session based on external pre-shared secret */ if (s->version >= TLS1_VERSION && s->tls_session_secret_cb) @@ -976,26 +974,30 @@ 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, p+j); + pref_cipher : + ssl_get_cipher_by_char(s, CBS_data(&server_hello)); s->s3->flags |= SSL3_FLAGS_CCS_OK; } } - if (j != 0 && j == s->session->session_id_length - && memcmp(p,s->session->session_id,j) == 0) - { - if(s->sid_ctx_length != s->session->sid_ctx_length - || memcmp(s->session->sid_ctx,s->sid_ctx,s->sid_ctx_length)) + if (CBS_len(&session_id) != 0 && + CBS_len(&session_id) == s->session->session_id_length && + memcmp(CBS_data(&session_id), s->session->session_id, CBS_len(&session_id)) == 0) { - /* actually a client application bug */ - al=SSL_AD_ILLEGAL_PARAMETER; - OPENSSL_PUT_ERROR(SSL, ssl3_get_server_hello, SSL_R_ATTEMPT_TO_REUSE_SESSION_IN_DIFFERENT_CONTEXT); - goto f_err; + if(s->sid_ctx_length != s->session->sid_ctx_length + || memcmp(s->session->sid_ctx, s->sid_ctx, s->sid_ctx_length)) + { + /* actually a client application bug */ + al = SSL_AD_ILLEGAL_PARAMETER; + OPENSSL_PUT_ERROR(SSL, ssl3_get_server_hello, SSL_R_ATTEMPT_TO_REUSE_SESSION_IN_DIFFERENT_CONTEXT); + goto f_err; + } + s->s3->flags |= SSL3_FLAGS_CCS_OK; + s->hit = 1; } - s->s3->flags |= SSL3_FLAGS_CCS_OK; - s->hit=1; - } else /* a miss or crap from the other end */ { /* If we were trying for session-id reuse, make a new @@ -1008,15 +1010,25 @@ goto f_err; } } - s->session->session_id_length=j; - memcpy(s->session->session_id,p,j); /* j could be 0 */ + /* Note: session_id could be empty. */ + s->session->session_id_length = CBS_len(&session_id); + memcpy(s->session->session_id, CBS_data(&session_id), CBS_len(&session_id)); } - p+=j; - c=ssl_get_cipher_by_char(s,p); + + /* 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); if (c == NULL) { /* unknown cipher */ - al=SSL_AD_ILLEGAL_PARAMETER; + al = SSL_AD_ILLEGAL_PARAMETER; OPENSSL_PUT_ERROR(SSL, ssl3_get_server_hello, SSL_R_UNKNOWN_CIPHER_RETURNED); goto f_err; } @@ -1031,7 +1043,6 @@ OPENSSL_PUT_ERROR(SSL, ssl3_get_server_hello, SSL_R_WRONG_CIPHER_RETURNED); goto f_err; } - p+=ssl_put_cipher_by_char(s,NULL,NULL); sk=ssl_get_ciphers_by_id(s); if (!sk_SSL_CIPHER_find(sk, NULL, c)) @@ -1049,16 +1060,9 @@ s->session->cipher_id = s->session->cipher->id; if (s->hit && (s->session->cipher_id != c->id)) { -/* Workaround is now obsolete */ -#if 0 - if (!(s->options & - SSL_OP_NETSCAPE_REUSE_CIPHER_CHANGE_BUG)) -#endif - { - al=SSL_AD_ILLEGAL_PARAMETER; - OPENSSL_PUT_ERROR(SSL, ssl3_get_server_hello, SSL_R_OLD_SESSION_CIPHER_NOT_RETURNED); - goto f_err; - } + al = SSL_AD_ILLEGAL_PARAMETER; + OPENSSL_PUT_ERROR(SSL, ssl3_get_server_hello, SSL_R_OLD_SESSION_CIPHER_NOT_RETURNED); + goto f_err; } s->s3->tmp.new_cipher=c; /* Don't digest cached records if no sigalgs: we may need them for @@ -1066,26 +1070,30 @@ */ if (!SSL_USE_SIGALGS(s) && !ssl3_digest_cached_records(s)) goto f_err; - /* lets get the compression algorithm */ - /* COMPRESSION */ - if (*(p++) != 0) + + if (!CBS_get_u8(&server_hello, &compression_method)) { - al=SSL_AD_ILLEGAL_PARAMETER; + 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) + { + al = SSL_AD_ILLEGAL_PARAMETER; OPENSSL_PUT_ERROR(SSL, ssl3_get_server_hello, SSL_R_UNSUPPORTED_COMPRESSION_ALGORITHM); goto f_err; } - /* TODO(fork): Port the rest of this function to CBS. */ - CBS_init(&cbs, p, d + n - p); - /* TLS extensions*/ - if (!ssl_parse_serverhello_tlsext(s, &cbs)) + /* TLS extensions */ + if (!ssl_parse_serverhello_tlsext(s, &server_hello)) { OPENSSL_PUT_ERROR(SSL, ssl3_get_server_hello, SSL_R_PARSE_TLSEXT); goto err; } /* There should be nothing left over in the record. */ - if (CBS_len(&cbs) != 0) + if (CBS_len(&server_hello) != 0) { /* wrong packet length */ al=SSL_AD_DECODE_ERROR;