Add a helper function for parsing extensions blocks.
TLS 1.3 adds a number of places with extensions blocks that don't easily
fit into our ClientHello/EncryptedExtensions callbacks. Between
HelloRetryRequest, ServerHello, draft 18 going nuts with Certificate,
and NewSessionTicket when we do 0-RTT, this passes the "abstract things
that are repeated three times" sniff test.
For now, it rejects unknown extensions, but it will probably grow an
allow_unknown parameter for NewSessionTicket.
This involves disabling some MSVC warnings, but they're invalid as of
C99 which we otherwise require. See
https://connect.microsoft.com/VisualStudio/feedback/details/1230248/remove-c99-related-warnings-or-make-them-off-by-default
Change-Id: Iea8bf8ab216270c081dd63e79aaad9ec73b3b550
Reviewed-on: https://boringssl-review.googlesource.com/12233
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/ssl/internal.h b/ssl/internal.h
index 830e76b..c964a83 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1102,6 +1102,21 @@
int ssl_negotiate_alpn(SSL *ssl, uint8_t *out_alert,
const struct ssl_early_callback_ctx *client_hello);
+typedef struct {
+ uint16_t type;
+ int *out_present;
+ CBS *out_data;
+} SSL_EXTENSION_TYPE;
+
+/* ssl_parse_extensions parses a TLS extensions block out of |cbs| and advances
+ * it. It writes the parsed extensions to pointers denoted by |ext_types|. On
+ * success, it fills in the |out_present| and |out_data| fields and returns one.
+ * Otherwise, it sets |*out_alert| to an alert to send and returns zero. Unknown
+ * extensions are rejected. */
+int ssl_parse_extensions(const CBS *cbs, uint8_t *out_alert,
+ const SSL_EXTENSION_TYPE *ext_types,
+ size_t num_ext_types);
+
/* SSLKEYLOGFILE functions. */
diff --git a/ssl/s3_both.c b/ssl/s3_both.c
index 98ebf17..f59022f 100644
--- a/ssl/s3_both.c
+++ b/ssl/s3_both.c
@@ -767,3 +767,51 @@
return al;
}
+
+int ssl_parse_extensions(const CBS *cbs, uint8_t *out_alert,
+ const SSL_EXTENSION_TYPE *ext_types,
+ size_t num_ext_types) {
+ /* Reset everything. */
+ for (size_t i = 0; i < num_ext_types; i++) {
+ *ext_types[i].out_present = 0;
+ CBS_init(ext_types[i].out_data, NULL, 0);
+ }
+
+ CBS copy = *cbs;
+ while (CBS_len(©) != 0) {
+ uint16_t type;
+ CBS data;
+ if (!CBS_get_u16(©, &type) ||
+ !CBS_get_u16_length_prefixed(©, &data)) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_PARSE_TLSEXT);
+ *out_alert = SSL_AD_DECODE_ERROR;
+ return 0;
+ }
+
+ const SSL_EXTENSION_TYPE *ext_type = NULL;
+ for (size_t i = 0; i < num_ext_types; i++) {
+ if (type == ext_types[i].type) {
+ ext_type = &ext_types[i];
+ break;
+ }
+ }
+
+ if (ext_type == NULL) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION);
+ *out_alert = SSL_AD_UNSUPPORTED_EXTENSION;
+ return 0;
+ }
+
+ /* Duplicate ext_types are forbidden. */
+ if (*ext_type->out_present) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_DUPLICATE_EXTENSION);
+ *out_alert = SSL_AD_ILLEGAL_PARAMETER;
+ return 0;
+ }
+
+ *ext_type->out_present = 1;
+ *ext_type->out_data = data;
+ }
+
+ return 1;
+}
diff --git a/ssl/tls13_both.c b/ssl/tls13_both.c
index 242aa03..ab09240 100644
--- a/ssl/tls13_both.c
+++ b/ssl/tls13_both.c
@@ -25,6 +25,7 @@
#include <openssl/x509.h>
#include <openssl/x509v3.h>
+#include "../crypto/internal.h"
#include "internal.h"
@@ -221,40 +222,16 @@
/* Parse out the extensions. */
int have_status_request = 0, have_sct = 0;
CBS status_request, sct;
- while (CBS_len(&extensions) != 0) {
- uint16_t type;
- CBS extension;
- if (!CBS_get_u16(&extensions, &type) ||
- !CBS_get_u16_length_prefixed(&extensions, &extension)) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_PARSE_TLSEXT);
- ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
- goto err;
- }
+ const SSL_EXTENSION_TYPE ext_types[] = {
+ {TLSEXT_TYPE_status_request, &have_status_request, &status_request},
+ {TLSEXT_TYPE_certificate_timestamp, &have_sct, &sct},
+ };
- switch (type) {
- case TLSEXT_TYPE_status_request:
- if (have_status_request) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_DUPLICATE_EXTENSION);
- ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
- goto err;
- }
- status_request = extension;
- have_status_request = 1;
- break;
- case TLSEXT_TYPE_certificate_timestamp:
- if (have_sct) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_DUPLICATE_EXTENSION);
- ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
- goto err;
- }
- sct = extension;
- have_sct = 1;
- break;
- default:
- OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION);
- ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_UNSUPPORTED_EXTENSION);
- goto err;
- }
+ uint8_t alert;
+ if (!ssl_parse_extensions(&extensions, &alert, ext_types,
+ OPENSSL_ARRAY_SIZE(ext_types))) {
+ ssl3_send_alert(ssl, SSL3_AL_FATAL, alert);
+ goto err;
}
/* All Certificate extensions are parsed, but only the leaf extensions are
diff --git a/ssl/tls13_client.c b/ssl/tls13_client.c
index e540b77..ebe7703 100644
--- a/ssl/tls13_client.c
+++ b/ssl/tls13_client.c
@@ -24,6 +24,7 @@
#include <openssl/stack.h>
#include <openssl/x509.h>
+#include "../crypto/internal.h"
#include "internal.h"
@@ -67,97 +68,73 @@
return ssl_hs_error;
}
- while (CBS_len(&extensions) != 0) {
- uint16_t type;
- CBS extension;
- if (!CBS_get_u16(&extensions, &type) ||
- !CBS_get_u16_length_prefixed(&extensions, &extension)) {
+ int have_cookie, have_key_share;
+ CBS cookie, key_share;
+ const SSL_EXTENSION_TYPE ext_types[] = {
+ {TLSEXT_TYPE_key_share, &have_key_share, &key_share},
+ {TLSEXT_TYPE_cookie, &have_cookie, &cookie},
+ };
+
+ uint8_t alert;
+ if (!ssl_parse_extensions(&extensions, &alert, ext_types,
+ OPENSSL_ARRAY_SIZE(ext_types))) {
+ ssl3_send_alert(ssl, SSL3_AL_FATAL, alert);
+ return ssl_hs_error;
+ }
+
+ if (have_cookie) {
+ CBS cookie_value;
+ if (!CBS_get_u16_length_prefixed(&cookie, &cookie_value) ||
+ CBS_len(&cookie_value) == 0 ||
+ CBS_len(&cookie) != 0) {
OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
return ssl_hs_error;
}
- switch (type) {
- case TLSEXT_TYPE_cookie: {
- if (hs->cookie != NULL) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_DUPLICATE_EXTENSION);
- ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
- return ssl_hs_error;
- }
-
- /* Cookies may be requested whether or not advertised, so no need to
- * check. */
-
- CBS cookie;
- if (!CBS_get_u16_length_prefixed(&extension, &cookie) ||
- CBS_len(&cookie) == 0 ||
- CBS_len(&extension) != 0) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
- ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
- return ssl_hs_error;
- }
-
- if (!CBS_stow(&cookie, &hs->cookie, &hs->cookie_len)) {
- return ssl_hs_error;
- }
- break;
- }
-
- case TLSEXT_TYPE_key_share: {
- if (hs->retry_group != 0) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_DUPLICATE_EXTENSION);
- ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
- return ssl_hs_error;
- }
-
- /* key_share is always advertised, so no need to check. */
-
- uint16_t group_id;
- if (!CBS_get_u16(&extension, &group_id) ||
- CBS_len(&extension) != 0) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
- ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
- return ssl_hs_error;
- }
-
- /* The group must be supported. */
- const uint16_t *groups;
- size_t groups_len;
- tls1_get_grouplist(ssl, &groups, &groups_len);
- int found = 0;
- for (size_t i = 0; i < groups_len; i++) {
- if (groups[i] == group_id) {
- found = 1;
- break;
- }
- }
-
- if (!found) {
- ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
- OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_CURVE);
- return ssl_hs_error;
- }
-
- /* Check that the HelloRetryRequest does not request the key share that
- * was provided in the initial ClientHello. */
- if (SSL_ECDH_CTX_get_id(&hs->ecdh_ctx) == group_id) {
- ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
- OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_CURVE);
- return ssl_hs_error;
- }
-
- SSL_ECDH_CTX_cleanup(&hs->ecdh_ctx);
- hs->retry_group = group_id;
- break;
- }
-
- default:
- OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION);
- ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_UNSUPPORTED_EXTENSION);
- return ssl_hs_error;
+ if (!CBS_stow(&cookie_value, &hs->cookie, &hs->cookie_len)) {
+ return ssl_hs_error;
}
}
+ if (have_key_share) {
+ uint16_t group_id;
+ if (!CBS_get_u16(&key_share, &group_id) || CBS_len(&key_share) != 0) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
+ ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
+ return ssl_hs_error;
+ }
+
+ /* The group must be supported. */
+ const uint16_t *groups;
+ size_t groups_len;
+ tls1_get_grouplist(ssl, &groups, &groups_len);
+ int found = 0;
+ for (size_t i = 0; i < groups_len; i++) {
+ if (groups[i] == group_id) {
+ found = 1;
+ break;
+ }
+ }
+
+ if (!found) {
+ ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
+ OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_CURVE);
+ return ssl_hs_error;
+ }
+
+ /* Check that the HelloRetryRequest does not request the key share that
+ * was provided in the initial ClientHello. */
+ if (SSL_ECDH_CTX_get_id(&hs->ecdh_ctx) == group_id) {
+ ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
+ OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_CURVE);
+ return ssl_hs_error;
+ }
+
+ SSL_ECDH_CTX_cleanup(&hs->ecdh_ctx);
+ hs->retry_group = group_id;
+ }
+
hs->received_hello_retry_request = 1;
hs->state = state_send_second_client_hello;
return ssl_hs_ok;
@@ -225,40 +202,16 @@
/* Parse out the extensions. */
int have_key_share = 0, have_pre_shared_key = 0;
CBS key_share, pre_shared_key;
- while (CBS_len(&extensions) != 0) {
- uint16_t type;
- CBS extension;
- if (!CBS_get_u16(&extensions, &type) ||
- !CBS_get_u16_length_prefixed(&extensions, &extension)) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_PARSE_TLSEXT);
- ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
- return ssl_hs_error;
- }
+ const SSL_EXTENSION_TYPE ext_types[] = {
+ {TLSEXT_TYPE_key_share, &have_key_share, &key_share},
+ {TLSEXT_TYPE_pre_shared_key, &have_pre_shared_key, &pre_shared_key},
+ };
- switch (type) {
- case TLSEXT_TYPE_key_share:
- if (have_key_share) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_DUPLICATE_EXTENSION);
- ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
- return ssl_hs_error;
- }
- key_share = extension;
- have_key_share = 1;
- break;
- case TLSEXT_TYPE_pre_shared_key:
- if (have_pre_shared_key) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_DUPLICATE_EXTENSION);
- ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
- return ssl_hs_error;
- }
- pre_shared_key = extension;
- have_pre_shared_key = 1;
- break;
- default:
- OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION);
- ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_UNSUPPORTED_EXTENSION);
- return ssl_hs_error;
- }
+ uint8_t alert;
+ if (!ssl_parse_extensions(&extensions, &alert, ext_types,
+ OPENSSL_ARRAY_SIZE(ext_types))) {
+ ssl3_send_alert(ssl, SSL3_AL_FATAL, alert);
+ return ssl_hs_error;
}
/* We only support PSK_DHE_KE. */
@@ -268,7 +221,7 @@
return ssl_hs_error;
}
- uint8_t alert = SSL_AD_DECODE_ERROR;
+ alert = SSL_AD_DECODE_ERROR;
if (have_pre_shared_key) {
if (ssl->session == NULL) {
OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION);
