From e8d53508ca92f31c0ce90468faf6c3bb4bdee8a2 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Sat, 10 Oct 2015 14:13:23 -0400 Subject: [PATCH] Convert ssl3_send_client_hello to CBB. Start converting the ones we can right now. Some of the messier ones resize init_buf rather than assume the initial size is sufficient, so those will probably wait until init_buf is gone and the handshake's undergone some more invasive surgery. The async ones will also require some thought. But some can be incrementally converted now. BUG=468889 Change-Id: I0bc22e4dca37d9d671a488c42eba864c51933638 Reviewed-on: https://boringssl-review.googlesource.com/6190 Reviewed-by: Adam Langley --- crypto/bytestring/bytestring_test.cc | 50 ++++++ crypto/bytestring/cbb.c | 14 ++ crypto/test/scoped_types.h | 1 + include/openssl/bytestring.h | 4 + ssl/internal.h | 12 +- ssl/s3_clnt.c | 257 +++++++++++++-------------- ssl/ssl_lib.c | 45 ----- ssl/t1_lib.c | 50 ++---- 8 files changed, 224 insertions(+), 209 deletions(-) diff --git a/crypto/bytestring/bytestring_test.cc b/crypto/bytestring/bytestring_test.cc index e987e1b9..eae88d9d 100644 --- a/crypto/bytestring/bytestring_test.cc +++ b/crypto/bytestring/bytestring_test.cc @@ -365,6 +365,55 @@ static bool TestCBBPrefixed() { return buf_len == sizeof(kExpected) && memcmp(buf, kExpected, buf_len) == 0; } +static bool TestCBBDiscardChild() { + ScopedCBB cbb; + CBB contents, inner_contents, inner_inner_contents; + + if (!CBB_init(cbb.get(), 0) || + !CBB_add_u8(cbb.get(), 0xaa)) { + return false; + } + + // Discarding |cbb|'s children preserves the byte written. + CBB_discard_child(cbb.get()); + + if (!CBB_add_u8_length_prefixed(cbb.get(), &contents) || + !CBB_add_u8_length_prefixed(cbb.get(), &contents) || + !CBB_add_u8(&contents, 0xbb) || + !CBB_add_u16_length_prefixed(cbb.get(), &contents) || + !CBB_add_u16(&contents, 0xcccc) || + !CBB_add_u24_length_prefixed(cbb.get(), &contents) || + !CBB_add_u24(&contents, 0xdddddd) || + !CBB_add_u8_length_prefixed(cbb.get(), &contents) || + !CBB_add_u8(&contents, 0xff) || + !CBB_add_u8_length_prefixed(&contents, &inner_contents) || + !CBB_add_u8(&inner_contents, 0x42) || + !CBB_add_u16_length_prefixed(&inner_contents, &inner_inner_contents) || + !CBB_add_u8(&inner_inner_contents, 0x99)) { + return false; + } + + // Discard everything from |inner_contents| down. + CBB_discard_child(&contents); + + uint8_t *buf; + size_t buf_len; + if (!CBB_finish(cbb.get(), &buf, &buf_len)) { + return false; + } + ScopedOpenSSLBytes scoper(buf); + + static const uint8_t kExpected[] = { + 0xaa, + 0, + 1, 0xbb, + 0, 2, 0xcc, 0xcc, + 0, 0, 3, 0xdd, 0xdd, 0xdd, + 1, 0xff, + }; + return buf_len == sizeof(kExpected) && memcmp(buf, kExpected, buf_len) == 0; +} + static bool TestCBBMisuse() { CBB cbb, child, contents; uint8_t *buf; @@ -670,6 +719,7 @@ int main(void) { !TestCBBFinishChild() || !TestCBBMisuse() || !TestCBBPrefixed() || + !TestCBBDiscardChild() || !TestCBBASN1() || !TestBerConvert() || !TestASN1Uint64() || diff --git a/crypto/bytestring/cbb.c b/crypto/bytestring/cbb.c index 5d7485a8..434ec131 100644 --- a/crypto/bytestring/cbb.c +++ b/crypto/bytestring/cbb.c @@ -360,6 +360,20 @@ int CBB_add_u24(CBB *cbb, uint32_t value) { return cbb_buffer_add_u(cbb->base, value, 3); } +void CBB_discard_child(CBB *cbb) { + if (cbb->child == NULL) { + return; + } + + cbb->base->len = cbb->offset; + + cbb->child->base = NULL; + cbb->child = NULL; + cbb->pending_len_len = 0; + cbb->pending_is_asn1 = 0; + cbb->offset = 0; +} + int CBB_add_asn1_uint64(CBB *cbb, uint64_t value) { CBB child; size_t i; diff --git a/crypto/test/scoped_types.h b/crypto/test/scoped_types.h index e44c6ed9..2ce45266 100644 --- a/crypto/test/scoped_types.h +++ b/crypto/test/scoped_types.h @@ -117,6 +117,7 @@ using ScopedX509_SIG = ScopedOpenSSLType; using ScopedX509Stack = ScopedOpenSSLStack; +using ScopedCBB = ScopedOpenSSLContext; using ScopedEVP_AEAD_CTX = ScopedOpenSSLContext; diff --git a/include/openssl/bytestring.h b/include/openssl/bytestring.h index 6faab3c5..2fa065e3 100644 --- a/include/openssl/bytestring.h +++ b/include/openssl/bytestring.h @@ -344,6 +344,10 @@ OPENSSL_EXPORT int CBB_add_u16(CBB *cbb, uint16_t value); * returns one on success and zero otherwise. */ OPENSSL_EXPORT int CBB_add_u24(CBB *cbb, uint32_t value); +/* CBB_discard_child discards the current unflushed child of |cbb|. Neither the + * child's contents nor the length prefix will be included in the output. */ +OPENSSL_EXPORT void CBB_discard_child(CBB *cbb); + /* CBB_add_asn1_uint64 writes an ASN.1 INTEGER into |cbb| using |CBB_add_asn1| * and writes |value| in its contents. It returns one on success and zero on * error. */ diff --git a/ssl/internal.h b/ssl/internal.h index 6fb8dbe6..7f13ebc3 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -976,7 +976,6 @@ enum ssl_session_result_t ssl_get_prev_session( const struct ssl_early_callback_ctx *ctx); STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *s, const CBS *cbs); -int ssl_cipher_list_to_bytes(SSL *s, STACK_OF(SSL_CIPHER) *sk, uint8_t *p); struct ssl_cipher_preference_list_st *ssl_cipher_preference_list_dup( struct ssl_cipher_preference_list_st *cipher_list); void ssl_cipher_preference_list_free( @@ -1104,7 +1103,7 @@ unsigned int dtls1_min_mtu(void); void dtls1_hm_fragment_free(hm_fragment *frag); /* some client-only functions */ -int ssl3_send_client_hello(SSL *s); +int ssl3_send_client_hello(SSL *ssl); int ssl3_get_server_hello(SSL *s); int ssl3_get_certificate_request(SSL *s); int ssl3_get_new_session_ticket(SSL *s); @@ -1208,8 +1207,13 @@ int tls1_check_ec_tmp_key(SSL *s); int tls1_shared_list(SSL *s, const uint8_t *l1, size_t l1len, const uint8_t *l2, size_t l2len, int nmatch); -uint8_t *ssl_add_clienthello_tlsext(SSL *s, uint8_t *const buf, - uint8_t *const limit, size_t header_len); + +/* ssl_add_clienthello_tlsext writes ClientHello extensions to |out|. It + * returns one on success and zero on failure. The |header_len| argument is the + * length of the ClientHello written so far and is used to compute the padding + * length. (It does not include the record header.) */ +int ssl_add_clienthello_tlsext(SSL *ssl, CBB *out, size_t header_len); + uint8_t *ssl_add_serverhello_tlsext(SSL *s, uint8_t *const buf, uint8_t *const limit); int ssl_parse_clienthello_tlsext(SSL *s, CBS *cbs); diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c index 0bc9df15..2786acff 100644 --- a/ssl/s3_clnt.c +++ b/ssl/s3_clnt.c @@ -589,140 +589,139 @@ end: return ret; } -int ssl3_send_client_hello(SSL *s) { - uint8_t *buf, *p, *d; - int i; - unsigned long l; +static int ssl3_write_client_cipher_list(SSL *ssl, CBB *out) { + /* Prepare disabled cipher masks. */ + ssl_set_client_disabled(ssl); - buf = (uint8_t *)s->init_buf->data; - if (s->state == SSL3_ST_CW_CLNT_HELLO_A) { - if (!s->s3->have_version) { - uint16_t max_version = ssl3_get_max_client_version(s); - /* Disabling all versions is silly: return an error. */ - if (max_version == 0) { - OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_SSL_VERSION); - goto err; - } - s->version = max_version; - s->client_version = max_version; - } - - /* If the configured session was created at a version higher than our - * maximum version, drop it. */ - if (s->session && - (s->session->session_id_length == 0 || s->session->not_resumable || - (!SSL_IS_DTLS(s) && s->session->ssl_version > s->version) || - (SSL_IS_DTLS(s) && s->session->ssl_version < s->version))) { - SSL_set_session(s, NULL); - } - - /* else use the pre-loaded session */ - p = s->s3->client_random; - - /* If resending the ClientHello in DTLS after a HelloVerifyRequest, don't - * renegerate the client_random. The random must be reused. */ - if ((!SSL_IS_DTLS(s) || !s->d1->send_cookie) && - !ssl_fill_hello_random(p, sizeof(s->s3->client_random), - 0 /* client */)) { - goto err; - } - - /* Do the message type and length last. Note: the final argument to - * ssl_add_clienthello_tlsext below depends on the size of this prefix. */ - d = p = ssl_handshake_start(s); - - /* version indicates the negotiated version: for example from an SSLv2/v3 - * compatible client hello). The client_version field is the maximum - * version we permit and it is also used in RSA encrypted premaster - * secrets. Some servers can choke if we initially report a higher version - * then renegotiate to a lower one in the premaster secret. This didn't - * happen with TLS 1.0 as most servers supported it but it can with TLS 1.1 - * or later if the server only supports 1.0. - * - * Possible scenario with previous logic: - * 1. Client hello indicates TLS 1.2 - * 2. Server hello says TLS 1.0 - * 3. RSA encrypted premaster secret uses 1.2. - * 4. Handhaked proceeds using TLS 1.0. - * 5. Server sends hello request to renegotiate. - * 6. Client hello indicates TLS v1.0 as we now - * know that is maximum server supports. - * 7. Server chokes on RSA encrypted premaster secret - * containing version 1.0. - * - * For interoperability it should be OK to always use the maximum version - * we support in client hello and then rely on the checking of version to - * ensure the servers isn't being inconsistent: for example initially - * negotiating with TLS 1.0 and renegotiating with TLS 1.2. We do this by - * using client_version in client hello and not resetting it to the - * negotiated version. */ - *(p++) = s->client_version >> 8; - *(p++) = s->client_version & 0xff; - - /* Random stuff */ - memcpy(p, s->s3->client_random, SSL3_RANDOM_SIZE); - p += SSL3_RANDOM_SIZE; - - /* Session ID */ - if (s->s3->initial_handshake_complete || s->session == NULL) { - /* Renegotiations do not participate in session resumption. */ - i = 0; - } else { - i = s->session->session_id_length; - } - *(p++) = i; - if (i != 0) { - if (i > (int)sizeof(s->session->session_id)) { - OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); - goto err; - } - memcpy(p, s->session->session_id, i); - p += i; - } - - /* cookie stuff for DTLS */ - if (SSL_IS_DTLS(s)) { - if (s->d1->cookie_len > sizeof(s->d1->cookie)) { - OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); - goto err; - } - *(p++) = s->d1->cookie_len; - memcpy(p, s->d1->cookie, s->d1->cookie_len); - p += s->d1->cookie_len; - } - - /* Ciphers supported */ - i = ssl_cipher_list_to_bytes(s, SSL_get_ciphers(s), &p[2]); - if (i == 0) { - OPENSSL_PUT_ERROR(SSL, SSL_R_NO_CIPHERS_AVAILABLE); - goto err; - } - s2n(i, p); - p += i; - - /* COMPRESSION */ - *(p++) = 1; - *(p++) = 0; /* Add the NULL method */ - - /* TLS extensions*/ - p = ssl_add_clienthello_tlsext(s, p, buf + SSL3_RT_MAX_PLAIN_LENGTH, - p - buf); - if (p == NULL) { - OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); - goto err; - } - - l = p - d; - if (!ssl_set_handshake_header(s, SSL3_MT_CLIENT_HELLO, l)) { - goto err; - } - s->state = SSL3_ST_CW_CLNT_HELLO_B; + CBB child; + if (!CBB_add_u16_length_prefixed(out, &child)) { + return 0; } - /* SSL3_ST_CW_CLNT_HELLO_B */ - return ssl_do_write(s); + STACK_OF(SSL_CIPHER) *ciphers = SSL_get_ciphers(ssl); + + int any_enabled = 0; + size_t i; + for (i = 0; i < sk_SSL_CIPHER_num(ciphers); i++) { + const SSL_CIPHER *cipher = sk_SSL_CIPHER_value(ciphers, i); + /* Skip disabled ciphers */ + if (cipher->algorithm_ssl & ssl->cert->mask_ssl || + cipher->algorithm_mkey & ssl->cert->mask_k || + cipher->algorithm_auth & ssl->cert->mask_a) { + continue; + } + any_enabled = 1; + if (!CBB_add_u16(&child, ssl_cipher_get_value(cipher))) { + return 0; + } + } + + /* If all ciphers were disabled, return the error to the caller. */ + if (!any_enabled) { + OPENSSL_PUT_ERROR(SSL, SSL_R_NO_CIPHERS_AVAILABLE); + return 0; + } + + /* For SSLv3, the SCSV is added. Otherwise the renegotiation extension is + * added. */ + if (ssl->client_version == SSL3_VERSION && + !ssl->s3->initial_handshake_complete) { + if (!CBB_add_u16(&child, SSL3_CK_SCSV & 0xffff)) { + return 0; + } + /* The renegotiation extension is required to be at index zero. */ + ssl->s3->tmp.extensions.sent |= (1u << 0); + } + + if ((ssl->mode & SSL_MODE_SEND_FALLBACK_SCSV) && + !CBB_add_u16(&child, SSL3_CK_FALLBACK_SCSV & 0xffff)) { + return 0; + } + + return CBB_flush(out); +} + +int ssl3_send_client_hello(SSL *ssl) { + if (ssl->state == SSL3_ST_CW_CLNT_HELLO_B) { + return ssl_do_write(ssl); + } + + CBB cbb; + CBB_zero(&cbb); + + assert(ssl->state == SSL3_ST_CW_CLNT_HELLO_A); + if (!ssl->s3->have_version) { + uint16_t max_version = ssl3_get_max_client_version(ssl); + /* Disabling all versions is silly: return an error. */ + if (max_version == 0) { + OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_SSL_VERSION); + goto err; + } + + ssl->version = max_version; + /* Only set |ssl->client_version| on the initial handshake. Renegotiations, + * although locked to a version, reuse the value. When using the plain RSA + * key exchange, the ClientHello version is checked in the premaster secret. + * Some servers fail when this value changes. */ + ssl->client_version = max_version; + } + + /* If the configured session was created at a version higher than our + * maximum version, drop it. */ + if (ssl->session != NULL && + (ssl->session->session_id_length == 0 || ssl->session->not_resumable || + (!SSL_IS_DTLS(ssl) && ssl->session->ssl_version > ssl->version) || + (SSL_IS_DTLS(ssl) && ssl->session->ssl_version < ssl->version))) { + SSL_set_session(ssl, NULL); + } + + /* If resending the ClientHello in DTLS after a HelloVerifyRequest, don't + * renegerate the client_random. The random must be reused. */ + if ((!SSL_IS_DTLS(ssl) || !ssl->d1->send_cookie) && + !ssl_fill_hello_random(ssl->s3->client_random, + sizeof(ssl->s3->client_random), 0 /* client */)) { + goto err; + } + + /* Renegotiations do not participate in session resumption. */ + int has_session = ssl->session != NULL && + !ssl->s3->initial_handshake_complete; + + CBB child; + if (!CBB_init_fixed(&cbb, ssl_handshake_start(ssl), + ssl->init_buf->max - SSL_HM_HEADER_LENGTH(ssl)) || + !CBB_add_u16(&cbb, ssl->client_version) || + !CBB_add_bytes(&cbb, ssl->s3->client_random, SSL3_RANDOM_SIZE) || + !CBB_add_u8_length_prefixed(&cbb, &child) || + (has_session && + !CBB_add_bytes(&child, ssl->session->session_id, + ssl->session->session_id_length))) { + goto err; + } + + if (SSL_IS_DTLS(ssl)) { + if (!CBB_add_u8_length_prefixed(&cbb, &child) || + !CBB_add_bytes(&child, ssl->d1->cookie, ssl->d1->cookie_len)) { + goto err; + } + } + + size_t length; + if (!ssl3_write_client_cipher_list(ssl, &cbb) || + !CBB_add_u8(&cbb, 1 /* one compression method */) || + !CBB_add_u8(&cbb, 0 /* null compression */) || + !ssl_add_clienthello_tlsext(ssl, &cbb, + CBB_len(&cbb) + SSL_HM_HEADER_LENGTH(ssl)) || + !CBB_finish(&cbb, NULL, &length) || + !ssl_set_handshake_header(ssl, SSL3_MT_CLIENT_HELLO, length)) { + goto err; + } + + ssl->state = SSL3_ST_CW_CLNT_HELLO_B; + return ssl_do_write(ssl); err: + CBB_cleanup(&cbb); return -1; } diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index 97d917c7..7b36940a 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -1498,51 +1498,6 @@ int SSL_set_cipher_list(SSL *ssl, const char *str) { return 1; } -int ssl_cipher_list_to_bytes(SSL *s, STACK_OF(SSL_CIPHER) *sk, uint8_t *p) { - size_t i; - const SSL_CIPHER *c; - CERT *ct = s->cert; - uint8_t *q; - /* Set disabled masks for this session */ - ssl_set_client_disabled(s); - - if (sk == NULL) { - return 0; - } - q = p; - - for (i = 0; i < sk_SSL_CIPHER_num(sk); i++) { - c = sk_SSL_CIPHER_value(sk, i); - /* Skip disabled ciphers */ - if (c->algorithm_ssl & ct->mask_ssl || - c->algorithm_mkey & ct->mask_k || - c->algorithm_auth & ct->mask_a) { - continue; - } - s2n(ssl_cipher_get_value(c), p); - } - - /* If all ciphers were disabled, return the error to the caller. */ - if (p == q) { - return 0; - } - - /* For SSLv3, the SCSV is added. Otherwise the renegotiation extension is - * added. */ - if (s->client_version == SSL3_VERSION && - !s->s3->initial_handshake_complete) { - s2n(SSL3_CK_SCSV & 0xffff, p); - /* The renegotiation extension is required to be at index zero. */ - s->s3->tmp.extensions.sent |= (1u << 0); - } - - if (s->mode & SSL_MODE_SEND_FALLBACK_SCSV) { - s2n(SSL3_CK_FALLBACK_SCSV & 0xffff, p); - } - - return p - q; -} - STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *s, const CBS *cbs) { CBS cipher_suites = *cbs; const SSL_CIPHER *c; diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 6e7cba65..b521d066 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -2228,52 +2228,48 @@ int SSL_extension_supported(unsigned extension_value) { tls_extension_find(&index, extension_value) != NULL; } -/* header_len is the length of the ClientHello header written so far, used to - * compute padding. It does not include the record header. Pass 0 if no padding - * is to be done. */ -uint8_t *ssl_add_clienthello_tlsext(SSL *s, uint8_t *const buf, - uint8_t *const limit, size_t header_len) { +int ssl_add_clienthello_tlsext(SSL *ssl, CBB *out, size_t header_len) { /* don't add extensions for SSLv3 unless doing secure renegotiation */ - if (s->client_version == SSL3_VERSION && !s->s3->send_connection_binding) { - return buf; + if (ssl->client_version == SSL3_VERSION && + !ssl->s3->send_connection_binding) { + return 1; } - CBB cbb, extensions; - CBB_zero(&cbb); - if (!CBB_init_fixed(&cbb, buf, limit - buf) || - !CBB_add_u16_length_prefixed(&cbb, &extensions)) { + size_t orig_len = CBB_len(out); + CBB extensions; + if (!CBB_add_u16_length_prefixed(out, &extensions)) { goto err; } - s->s3->tmp.extensions.sent = 0; - s->s3->tmp.custom_extensions.sent = 0; + ssl->s3->tmp.extensions.sent = 0; + ssl->s3->tmp.custom_extensions.sent = 0; size_t i; for (i = 0; i < kNumExtensions; i++) { if (kExtensions[i].init != NULL) { - kExtensions[i].init(s); + kExtensions[i].init(ssl); } } for (i = 0; i < kNumExtensions; i++) { const size_t len_before = CBB_len(&extensions); - if (!kExtensions[i].add_clienthello(s, &extensions)) { + if (!kExtensions[i].add_clienthello(ssl, &extensions)) { OPENSSL_PUT_ERROR(SSL, SSL_R_ERROR_ADDING_EXTENSION); ERR_add_error_dataf("extension: %u", (unsigned)kExtensions[i].value); goto err; } if (CBB_len(&extensions) != len_before) { - s->s3->tmp.extensions.sent |= (1u << i); + ssl->s3->tmp.extensions.sent |= (1u << i); } } - if (!custom_ext_add_clienthello(s, &extensions)) { + if (!custom_ext_add_clienthello(ssl, &extensions)) { goto err; } - if (header_len > 0) { - header_len += CBB_len(&extensions); + if (!SSL_IS_DTLS(ssl)) { + header_len += CBB_len(&extensions) - orig_len; if (header_len > 0xff && header_len < 0x200) { /* Add padding to workaround bugs in F5 terminators. See * https://tools.ietf.org/html/draft-agl-tls-padding-03 @@ -2301,26 +2297,18 @@ uint8_t *ssl_add_clienthello_tlsext(SSL *s, uint8_t *const buf, } } - if (!CBB_flush(&cbb)) { - goto err; - } - - uint8_t *ret = buf; - const size_t cbb_len = CBB_len(&cbb); /* If only two bytes have been written then the extensions are actually empty * and those two bytes are the zero length. In that case, we don't bother * sending the extensions length. */ - if (cbb_len > 2) { - ret += cbb_len; + if (CBB_len(&extensions) - orig_len == 2) { + CBB_discard_child(out); } - CBB_cleanup(&cbb); - return ret; + return CBB_flush(out); err: - CBB_cleanup(&cbb); OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); - return NULL; + return 0; } uint8_t *ssl_add_serverhello_tlsext(SSL *s, uint8_t *const buf,