Make SSL_get_servername work in the early callback.
This avoids early callback users writing their own SNI parser and gives us a
place to surface the server name from ESNI in the future.
Update-Note: This isn't a breaking change, but users of
SSL_CTX_set_select_certificate_cb can likely drop a bit of code after this CL.
Bug: 275
Change-Id: I9685ae5cca8e0483de76229d12dac45ff8e9ec32
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/36784
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 9285b3f..c3735cd 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -3714,6 +3714,8 @@
// high-level operation on |ssl| to be retried at a later time, which will
// result in another call to |cb|.
//
+// |SSL_get_servername| may be used during this callback.
+//
// Note: The |SSL_CLIENT_HELLO| is only valid for the duration of the callback
// and is not valid while the handshake is paused.
OPENSSL_EXPORT void SSL_CTX_set_select_certificate_cb(
diff --git a/ssl/handshake_server.cc b/ssl/handshake_server.cc
index 8ee39a2..36aa560 100644
--- a/ssl/handshake_server.cc
+++ b/ssl/handshake_server.cc
@@ -503,6 +503,54 @@
return true;
}
+static bool extract_sni(SSL_HANDSHAKE *hs, uint8_t *out_alert,
+ const SSL_CLIENT_HELLO *client_hello) {
+ SSL *const ssl = hs->ssl;
+ CBS sni;
+ if (!ssl_client_hello_get_extension(client_hello, &sni,
+ TLSEXT_TYPE_server_name)) {
+ // No SNI extension to parse.
+ return true;
+ }
+
+ CBS server_name_list, host_name;
+ uint8_t name_type;
+ if (!CBS_get_u16_length_prefixed(&sni, &server_name_list) ||
+ !CBS_get_u8(&server_name_list, &name_type) ||
+ // Although the server_name extension was intended to be extensible to
+ // new name types and multiple names, OpenSSL 1.0.x had a bug which meant
+ // different name types will cause an error. Further, RFC 4366 originally
+ // defined syntax inextensibly. RFC 6066 corrected this mistake, but
+ // adding new name types is no longer feasible.
+ //
+ // Act as if the extensibility does not exist to simplify parsing.
+ !CBS_get_u16_length_prefixed(&server_name_list, &host_name) ||
+ CBS_len(&server_name_list) != 0 ||
+ CBS_len(&sni) != 0) {
+ *out_alert = SSL_AD_DECODE_ERROR;
+ return false;
+ }
+
+ if (name_type != TLSEXT_NAMETYPE_host_name ||
+ CBS_len(&host_name) == 0 ||
+ CBS_len(&host_name) > TLSEXT_MAXLEN_host_name ||
+ CBS_contains_zero_byte(&host_name)) {
+ *out_alert = SSL_AD_UNRECOGNIZED_NAME;
+ return false;
+ }
+
+ // Copy the hostname as a string.
+ char *raw = nullptr;
+ if (!CBS_strdup(&host_name, &raw)) {
+ *out_alert = SSL_AD_INTERNAL_ERROR;
+ return false;
+ }
+ ssl->s3->hostname.reset(raw);
+
+ hs->should_ack_sni = true;
+ return true;
+}
+
static enum ssl_hs_wait_t do_read_client_hello(SSL_HANDSHAKE *hs) {
SSL *const ssl = hs->ssl;
@@ -526,6 +574,12 @@
return ssl_hs_handoff;
}
+ uint8_t alert = SSL_AD_DECODE_ERROR;
+ if (!extract_sni(hs, &alert, &client_hello)) {
+ ssl_send_alert(ssl, SSL3_AL_FATAL, alert);
+ return ssl_hs_error;
+ }
+
// Run the early callback.
if (ssl->ctx->select_certificate_cb != NULL) {
switch (ssl->ctx->select_certificate_cb(&client_hello)) {
@@ -553,7 +607,6 @@
hs->apply_jdk11_workaround = true;
}
- uint8_t alert = SSL_AD_DECODE_ERROR;
if (!negotiate_version(hs, &alert, &client_hello)) {
ssl_send_alert(ssl, SSL3_AL_FATAL, alert);
return ssl_hs_error;
diff --git a/ssl/t1_lib.cc b/ssl/t1_lib.cc
index 88685c8..c1c41a8 100644
--- a/ssl/t1_lib.cc
+++ b/ssl/t1_lib.cc
@@ -633,45 +633,7 @@
static bool ext_sni_parse_clienthello(SSL_HANDSHAKE *hs, uint8_t *out_alert,
CBS *contents) {
- SSL *const ssl = hs->ssl;
- if (contents == NULL) {
- return true;
- }
-
- CBS server_name_list, host_name;
- uint8_t name_type;
- if (!CBS_get_u16_length_prefixed(contents, &server_name_list) ||
- !CBS_get_u8(&server_name_list, &name_type) ||
- // Although the server_name extension was intended to be extensible to
- // new name types and multiple names, OpenSSL 1.0.x had a bug which meant
- // different name types will cause an error. Further, RFC 4366 originally
- // defined syntax inextensibly. RFC 6066 corrected this mistake, but
- // adding new name types is no longer feasible.
- //
- // Act as if the extensibility does not exist to simplify parsing.
- !CBS_get_u16_length_prefixed(&server_name_list, &host_name) ||
- CBS_len(&server_name_list) != 0 ||
- CBS_len(contents) != 0) {
- return false;
- }
-
- if (name_type != TLSEXT_NAMETYPE_host_name ||
- CBS_len(&host_name) == 0 ||
- CBS_len(&host_name) > TLSEXT_MAXLEN_host_name ||
- CBS_contains_zero_byte(&host_name)) {
- *out_alert = SSL_AD_UNRECOGNIZED_NAME;
- return false;
- }
-
- // Copy the hostname as a string.
- char *raw = nullptr;
- if (!CBS_strdup(&host_name, &raw)) {
- *out_alert = SSL_AD_INTERNAL_ERROR;
- return false;
- }
- ssl->s3->hostname.reset(raw);
-
- hs->should_ack_sni = true;
+ // SNI has already been parsed earlier in the handshake. See |extract_sni|.
return true;
}
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index 19f94ba..a53aed2 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -1102,34 +1102,15 @@
GetTestState(client_hello->ssl)->early_callback_called = true;
if (!config->expect_server_name.empty()) {
- const uint8_t *extension_data;
- size_t extension_len;
- CBS extension, server_name_list, host_name;
- uint8_t name_type;
-
- if (!SSL_early_callback_ctx_extension_get(
- client_hello, TLSEXT_TYPE_server_name, &extension_data,
- &extension_len)) {
- fprintf(stderr, "Could not find server_name extension.\n");
+ const char *server_name =
+ SSL_get_servername(client_hello->ssl, TLSEXT_NAMETYPE_host_name);
+ if (server_name == nullptr ||
+ std::string(server_name) != config->expect_server_name) {
+ fprintf(stderr,
+ "Server name mismatch in early callback (got %s; want %s).\n",
+ server_name, config->expect_server_name.c_str());
return ssl_select_cert_error;
}
-
- CBS_init(&extension, extension_data, extension_len);
- if (!CBS_get_u16_length_prefixed(&extension, &server_name_list) ||
- CBS_len(&extension) != 0 ||
- !CBS_get_u8(&server_name_list, &name_type) ||
- name_type != TLSEXT_NAMETYPE_host_name ||
- !CBS_get_u16_length_prefixed(&server_name_list, &host_name) ||
- CBS_len(&server_name_list) != 0) {
- fprintf(stderr, "Could not decode server_name extension.\n");
- return ssl_select_cert_error;
- }
-
- if (!CBS_mem_equal(&host_name,
- (const uint8_t *)config->expect_server_name.data(),
- config->expect_server_name.size())) {
- fprintf(stderr, "Server name mismatch.\n");
- }
}
if (config->fail_early_callback) {
