From 777045738b3a89cbe3ccc2e2323fff80b2188cf4 Mon Sep 17 00:00:00 2001 From: jsing <> Date: Thu, 26 Jan 2017 12:28:00 +0000 Subject: Convert ssl3_get_client_hello() to CBS. ok beck@ --- src/lib/libssl/ssl_srvr.c | 147 ++++++++++++++++++++++------------------------ 1 file changed, 71 insertions(+), 76 deletions(-) (limited to 'src') diff --git a/src/lib/libssl/ssl_srvr.c b/src/lib/libssl/ssl_srvr.c index 0b110d6a72..217ecafeec 100644 --- a/src/lib/libssl/ssl_srvr.c +++ b/src/lib/libssl/ssl_srvr.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_srvr.c,v 1.4 2017/01/26 12:16:13 beck Exp $ */ +/* $OpenBSD: ssl_srvr.c,v 1.5 2017/01/26 12:28:00 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -719,8 +719,12 @@ ssl3_send_hello_request(SSL *s) int ssl3_get_client_hello(SSL *s) { + CBS cbs, client_random, session_id, cookie, cipher_suites; + CBS compression_methods; + uint16_t client_version; + uint8_t comp_method; + int comp_null; int i, j, ok, al, ret = -1; - unsigned int cookie_len; long n; unsigned long id; unsigned char *p, *d; @@ -729,6 +733,7 @@ ssl3_get_client_hello(SSL *s) unsigned long alg_k; const SSL_METHOD *method; uint16_t shared_version; + unsigned char *end; /* * We do this so that we will respond with our native type. @@ -745,23 +750,26 @@ ssl3_get_client_hello(SSL *s) n = s->method->internal->ssl_get_message(s, SSL3_ST_SR_CLNT_HELLO_B, SSL3_ST_SR_CLNT_HELLO_C, SSL3_MT_CLIENT_HELLO, SSL3_RT_MAX_PLAIN_LENGTH, &ok); - if (!ok) return ((int)n); s->internal->first_packet = 0; + if (n < 0) + goto err; + d = p = (unsigned char *)s->internal->init_msg; + end = d + n; + + CBS_init(&cbs, s->internal->init_msg, n); - if (2 > n) - goto truncated; /* * Use version from inside client hello, not from record header. * (may differ: see RFC 2246, Appendix E, second paragraph) */ - s->client_version = (((int)p[0]) << 8)|(int)p[1]; - p += 2; + if (!CBS_get_u16(&cbs, &client_version)) + goto truncated; - if (ssl_max_shared_version(s, s->client_version, &shared_version) != 1) { + if (ssl_max_shared_version(s, client_version, &shared_version) != 1) { SSLerror(SSL_R_WRONG_VERSION_NUMBER); if ((s->client_version >> 8) == SSL3_VERSION_MAJOR && !s->internal->enc_write_ctx && !s->internal->write_hash) { @@ -774,6 +782,7 @@ ssl3_get_client_hello(SSL *s) al = SSL_AD_PROTOCOL_VERSION; goto f_err; } + s->client_version = client_version; s->version = shared_version; if ((method = tls1_get_server_method(shared_version)) == NULL) @@ -784,39 +793,31 @@ ssl3_get_client_hello(SSL *s) } s->method = method; + if (!CBS_get_bytes(&cbs, &client_random, SSL3_RANDOM_SIZE)) + goto truncated; + if (!CBS_get_u8_length_prefixed(&cbs, &session_id)) + goto truncated; + /* * If we require cookies (DTLS) and this ClientHello doesn't * contain one, just return since we do not want to * allocate any memory yet. So check cookie length... */ - if (SSL_get_options(s) & SSL_OP_COOKIE_EXCHANGE) { - unsigned int session_length, cookie_length; - - if (p - d + SSL3_RANDOM_SIZE + 1 >= n) - goto truncated; - session_length = *(p + SSL3_RANDOM_SIZE); - - if (p - d + SSL3_RANDOM_SIZE + session_length + 1 >= n) + if (SSL_IS_DTLS(s)) { + if (!CBS_get_u8_length_prefixed(&cbs, &cookie)) goto truncated; - cookie_length = p[SSL3_RANDOM_SIZE + session_length + 1]; - - if (cookie_length == 0) - return (1); + if (SSL_get_options(s) & SSL_OP_COOKIE_EXCHANGE) { + if (CBS_len(&cookie) == 0) + return (1); + } } - if (p - d + SSL3_RANDOM_SIZE + 1 > n) - goto truncated; - - /* load the client random */ - memcpy(s->s3->client_random, p, SSL3_RANDOM_SIZE); - p += SSL3_RANDOM_SIZE; - - /* get the session-id */ - j= *(p++); - if (p - d + j > n) - goto truncated; + if (!CBS_write_bytes(&client_random, s->s3->client_random, + sizeof(s->s3->client_random), NULL)) + goto err; s->internal->hit = 0; + /* * Versions before 0.9.7 always allow clients to resume sessions in * renegotiation. 0.9.7 and later allow this by default, but optionally @@ -837,7 +838,10 @@ ssl3_get_client_hello(SSL *s) if (!ssl_get_new_session(s, 1)) goto err; } else { - i = ssl_get_prev_session(s, p, j, d + n); + /* XXX - pass CBS through instead... */ + i = ssl_get_prev_session(s, + (unsigned char *)CBS_data(&session_id), + CBS_len(&session_id), end); if (i == 1) { /* previous session */ s->internal->hit = 1; } else if (i == -1) @@ -849,33 +853,27 @@ ssl3_get_client_hello(SSL *s) } } - p += j; - if (SSL_IS_DTLS(s)) { - /* cookie stuff */ - if (p - d + 1 > n) - goto truncated; - cookie_len = *(p++); - /* - * The ClientHello may contain a cookie even if the - * HelloVerify message has not been sent--make sure that it - * does not cause an overflow. + * The ClientHello may contain a cookie even if the HelloVerify + * message has not been sent - make sure that it does not cause + * an overflow. */ - if (cookie_len > sizeof(D1I(s)->rcvd_cookie)) { - /* too much data */ + if (CBS_len(&cookie) > sizeof(D1I(s)->rcvd_cookie)) { al = SSL_AD_DECODE_ERROR; SSLerror(SSL_R_COOKIE_MISMATCH); goto f_err; } - if (p - d + cookie_len > n) - goto truncated; - - /* verify the cookie if appropriate option is set. */ + /* Verify the cookie if appropriate option is set. */ if ((SSL_get_options(s) & SSL_OP_COOKIE_EXCHANGE) && - cookie_len > 0) { - memcpy(D1I(s)->rcvd_cookie, p, cookie_len); + CBS_len(&cookie) > 0) { + size_t cookie_len; + + /* XXX - rcvd_cookie seems to only be used here... */ + if (!CBS_write_bytes(&cookie, D1I(s)->rcvd_cookie, + sizeof(D1I(s)->rcvd_cookie), &cookie_len)) + goto err; if (s->ctx->internal->app_verify_cookie_cb != NULL) { if (s->ctx->internal->app_verify_cookie_cb(s, @@ -885,39 +883,37 @@ ssl3_get_client_hello(SSL *s) goto f_err; } /* else cookie verification succeeded */ - } else if (timingsafe_memcmp(D1I(s)->rcvd_cookie, D1I(s)->cookie, - D1I(s)->cookie_len) != 0) { + /* XXX - can d1->cookie_len > sizeof(rcvd_cookie) ? */ + } else if (timingsafe_memcmp(D1I(s)->rcvd_cookie, + D1I(s)->cookie, D1I(s)->cookie_len) != 0) { /* default verification */ al = SSL_AD_HANDSHAKE_FAILURE; SSLerror(SSL_R_COOKIE_MISMATCH); goto f_err; } - ret = 2; } - - p += cookie_len; } - if (p - d + 2 > n) + if (!CBS_get_u16_length_prefixed(&cbs, &cipher_suites)) goto truncated; - n2s(p, i); - if ((i == 0) && (j != 0)) { + + /* XXX - This logic seems wrong... */ + 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; SSLerror(SSL_R_NO_CIPHERS_SPECIFIED); goto f_err; } - if (p - d + i > n) - goto truncated; - if (i > 0) { - if ((ciphers = ssl_bytes_to_cipher_list(s, p, i)) == NULL) + + if (CBS_len(&cipher_suites) > 0) { + if ((ciphers = ssl_bytes_to_cipher_list(s, + CBS_data(&cipher_suites), CBS_len(&cipher_suites))) == NULL) goto err; } - p += i; /* If it is a hit, check that the cipher is in the list */ - if ((s->internal->hit) && (i > 0)) { + if (s->internal->hit && CBS_len(&cipher_suites) > 0) { j = 0; id = s->session->cipher->id; @@ -939,25 +935,24 @@ ssl3_get_client_hello(SSL *s) } } - /* compression */ - if (p - d + 1 > n) - goto truncated; - i= *(p++); - if (p - d + i > n) + if (!CBS_get_u8_length_prefixed(&cbs, &compression_methods)) goto truncated; - for (j = 0; j < i; j++) { - if (p[j] == 0) - break; - } - p += i; - if (j >= i) { - /* no compress */ + comp_null = 0; + while (CBS_len(&compression_methods) > 0) { + if (!CBS_get_u8(&compression_methods, &comp_method)) + goto truncated; + if (comp_method == 0) + comp_null = 1; + } + if (comp_null == 0) { al = SSL_AD_DECODE_ERROR; SSLerror(SSL_R_NO_COMPRESSION_SPECIFIED); goto f_err; } + p = (unsigned char *)CBS_data(&cbs); + /* TLS extensions*/ if (!ssl_parse_clienthello_tlsext(s, &p, d, n, &al)) { /* 'al' set by ssl_parse_clienthello_tlsext */ -- cgit v1.2.3-55-g6feb