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 <alangley@gmail.com>
diff --git a/crypto/bytestring/bytestring_test.cc b/crypto/bytestring/bytestring_test.cc
index e987e1b..eae88d9 100644
--- a/crypto/bytestring/bytestring_test.cc
+++ b/crypto/bytestring/bytestring_test.cc
@@ -365,6 +365,55 @@
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 @@
!TestCBBFinishChild() ||
!TestCBBMisuse() ||
!TestCBBPrefixed() ||
+ !TestCBBDiscardChild() ||
!TestCBBASN1() ||
!TestBerConvert() ||
!TestASN1Uint64() ||
diff --git a/crypto/bytestring/cbb.c b/crypto/bytestring/cbb.c
index 5d7485a..434ec13 100644
--- a/crypto/bytestring/cbb.c
+++ b/crypto/bytestring/cbb.c
@@ -360,6 +360,20 @@
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 e44c6ed..2ce4526 100644
--- a/crypto/test/scoped_types.h
+++ b/crypto/test/scoped_types.h
@@ -117,6 +117,7 @@
using ScopedX509Stack = ScopedOpenSSLStack<STACK_OF(X509), X509, X509_free>;
+using ScopedCBB = ScopedOpenSSLContext<CBB, void, CBB_zero, CBB_cleanup>;
using ScopedEVP_AEAD_CTX = ScopedOpenSSLContext<EVP_AEAD_CTX, void,
EVP_AEAD_CTX_zero,
EVP_AEAD_CTX_cleanup>;
diff --git a/include/openssl/bytestring.h b/include/openssl/bytestring.h
index 6faab3c..2fa065e 100644
--- a/include/openssl/bytestring.h
+++ b/include/openssl/bytestring.h
@@ -344,6 +344,10 @@
* 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 6fb8dbe..7f13ebc 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -976,7 +976,6 @@
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 @@
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_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 0bc9df1..2786acf 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -589,140 +589,139 @@
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 97d917c..7b36940 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -1498,51 +1498,6 @@
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 6e7cba6..b521d06 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -2228,52 +2228,48 @@
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 @@
}
}
- 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,
