Negotiate the cipher suite before ALPN.
HTTP/2 places requirements on the cipher suite. So that servers can
decline HTTP/2 when these requirements aren't met, defer ALPN
negotiation.
See also b/32553041.
Change-Id: Idbcf049f9c8bda06a8be52a0154fe76e84607268
Reviewed-on: https://boringssl-review.googlesource.com/11982
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index d0f4fec..417b194 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -2389,7 +2389,10 @@
* |*out_len| to the selected protocol and return |SSL_TLSEXT_ERR_OK| on
* success. It does not pass ownership of the buffer. Otherwise, it should
* return |SSL_TLSEXT_ERR_NOACK|. Other |SSL_TLSEXT_ERR_*| values are
- * unimplemented and will be treated as |SSL_TLSEXT_ERR_NOACK|. */
+ * unimplemented and will be treated as |SSL_TLSEXT_ERR_NOACK|.
+ *
+ * The cipher suite is selected before negotiating ALPN. The callback may use
+ * |SSL_get_pending_cipher| to query the cipher suite. */
OPENSSL_EXPORT void SSL_CTX_set_alpn_select_cb(
SSL_CTX *ctx, int (*cb)(SSL *ssl, const uint8_t **out, uint8_t *out_len,
const uint8_t *in, unsigned in_len, void *arg),
diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c
index 9200b85..d17e659 100644
--- a/ssl/handshake_server.c
+++ b/ssl/handshake_server.c
@@ -556,7 +556,7 @@
}
static int negotiate_version(
- SSL *ssl, int *out_alert,
+ SSL *ssl, uint8_t *out_alert,
const struct ssl_early_callback_ctx *client_hello) {
uint16_t min_version, max_version;
if (!ssl_get_version_range(ssl, &min_version, &max_version)) {
@@ -665,7 +665,8 @@
}
static int ssl3_get_client_hello(SSL *ssl) {
- int al = SSL_AD_INTERNAL_ERROR, ret = -1;
+ uint8_t al = SSL_AD_INTERNAL_ERROR;
+ int ret = -1;
SSL_SESSION *session = NULL;
if (ssl->state == SSL3_ST_SR_CLNT_HELLO_A) {
@@ -887,6 +888,12 @@
}
}
+ /* Resolve ALPN after the cipher suite is selected. HTTP/2 negotiation depends
+ * on the cipher suite. */
+ if (!ssl_negotiate_alpn(ssl, &al, &client_hello)) {
+ goto f_err;
+ }
+
/* Now that the cipher is known, initialize the handshake hash. */
if (!ssl3_init_handshake_hash(ssl)) {
goto f_err;
diff --git a/ssl/internal.h b/ssl/internal.h
index b6f0203..461de2c 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1083,6 +1083,12 @@
SSL *ssl, uint8_t **out, size_t *out_len,
enum ssl_cert_verify_context_t cert_verify_context);
+/* ssl_negotiate_alpn negotiates the ALPN extension, if applicable. It returns
+ * one on successful negotiation or if nothing was negotiated. It returns zero
+ * and sets |*out_alert| to an alert on error. */
+int ssl_negotiate_alpn(SSL *ssl, uint8_t *out_alert,
+ const struct ssl_early_callback_ctx *client_hello);
+
/* SSLKEYLOGFILE functions. */
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index ab1a599..eb60550 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -2347,6 +2347,64 @@
return true;
}
+// Tests that that |SSL_get_pending_cipher| is available during the ALPN
+// selection callback.
+static bool TestALPNCipherAvailable() {
+ static const uint8_t kALPNProtos[] = {0x03, 'f', 'o', 'o'};
+
+ bssl::UniquePtr<X509> cert = GetTestCertificate();
+ bssl::UniquePtr<EVP_PKEY> key = GetTestKey();
+ if (!cert || !key) {
+ return false;
+ }
+
+ for (uint16_t version : kTLSVersions) {
+ // SSL 3.0 lacks extensions.
+ if (version == SSL3_VERSION) {
+ continue;
+ }
+
+ bssl::UniquePtr<SSL_CTX> ctx(SSL_CTX_new(TLS_method()));
+ if (!ctx ||
+ !SSL_CTX_use_certificate(ctx.get(), cert.get()) ||
+ !SSL_CTX_use_PrivateKey(ctx.get(), key.get()) ||
+ !SSL_CTX_set_min_proto_version(ctx.get(), version) ||
+ !SSL_CTX_set_max_proto_version(ctx.get(), version) ||
+ SSL_CTX_set_alpn_protos(ctx.get(), kALPNProtos, sizeof(kALPNProtos)) !=
+ 0) {
+ return false;
+ }
+
+ // The ALPN callback does not fail the handshake on error, so have the
+ // callback write a boolean.
+ bool pending_cipher_known = false;
+ SSL_CTX_set_alpn_select_cb(
+ ctx.get(),
+ [](SSL *ssl, const uint8_t **out, uint8_t *out_len, const uint8_t *in,
+ unsigned in_len, void *arg) -> int {
+ bool *result = reinterpret_cast<bool *>(arg);
+ *result = SSL_get_pending_cipher(ssl) != nullptr;
+ return SSL_TLSEXT_ERR_NOACK;
+ },
+ &pending_cipher_known);
+
+ bssl::UniquePtr<SSL> client, server;
+ if (!ConnectClientAndServer(&client, &server, ctx.get(), ctx.get(),
+ nullptr /* no session */)) {
+ return false;
+ }
+
+ if (!pending_cipher_known) {
+ fprintf(stderr,
+ "%x: The pending cipher was not known in the ALPN callback.\n",
+ version);
+ return false;
+ }
+ }
+
+ return true;
+}
+
int main() {
CRYPTO_library_init();
@@ -2384,7 +2442,8 @@
!TestSNICallback() ||
!TestEarlyCallbackVersionSwitch() ||
!TestSetVersion() ||
- !TestVersions()) {
+ !TestVersions() ||
+ !TestALPNCipherAvailable()) {
ERR_print_errors_fp(stderr);
return 1;
}
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index fffde09..fbc723b 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1342,10 +1342,6 @@
if (contents == NULL ||
ssl->s3->initial_handshake_complete ||
- /* If the ALPN extension is seen before NPN, ignore it. (If ALPN is seen
- * afterwards, parsing the ALPN extension will clear
- * |next_proto_neg_seen|. */
- ssl->s3->alpn_selected != NULL ||
ssl->ctx->next_protos_advertised_cb == NULL ||
SSL_is_dtls(ssl)) {
return 1;
@@ -1545,14 +1541,14 @@
return 1;
}
-static int ext_alpn_parse_clienthello(SSL *ssl, uint8_t *out_alert,
- CBS *contents) {
- if (contents == NULL) {
- return 1;
- }
-
+int ssl_negotiate_alpn(SSL *ssl, uint8_t *out_alert,
+ const struct ssl_early_callback_ctx *client_hello) {
+ CBS contents;
if (ssl->ctx->alpn_select_cb == NULL ||
- ssl->s3->initial_handshake_complete) {
+ !ssl_early_callback_get_extension(
+ client_hello, &contents,
+ TLSEXT_TYPE_application_layer_protocol_negotiation)) {
+ /* Ignore ALPN if not configured or no extension was supplied. */
return 1;
}
@@ -1560,9 +1556,11 @@
ssl->s3->hs->next_proto_neg_seen = 0;
CBS protocol_name_list;
- if (!CBS_get_u16_length_prefixed(contents, &protocol_name_list) ||
- CBS_len(contents) != 0 ||
+ if (!CBS_get_u16_length_prefixed(&contents, &protocol_name_list) ||
+ CBS_len(&contents) != 0 ||
CBS_len(&protocol_name_list) < 2) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_PARSE_TLSEXT);
+ *out_alert = SSL_AD_DECODE_ERROR;
return 0;
}
@@ -1574,6 +1572,8 @@
if (!CBS_get_u8_length_prefixed(&protocol_name_list_copy, &protocol_name) ||
/* Empty protocol names are forbidden. */
CBS_len(&protocol_name) == 0) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_PARSE_TLSEXT);
+ *out_alert = SSL_AD_DECODE_ERROR;
return 0;
}
}
@@ -2461,7 +2461,8 @@
ext_alpn_init,
ext_alpn_add_clienthello,
ext_alpn_parse_serverhello,
- ext_alpn_parse_clienthello,
+ /* ALPN is negotiated late in |ssl_negotiate_alpn|. */
+ ignore_parse_clienthello,
ext_alpn_add_serverhello,
},
{
diff --git a/ssl/tls13_server.c b/ssl/tls13_server.c
index 2280a2b..574e6dd 100644
--- a/ssl/tls13_server.c
+++ b/ssl/tls13_server.c
@@ -211,6 +211,13 @@
ssl->s3->tmp.new_cipher = ssl->s3->new_session->cipher;
ssl->method->received_flight(ssl);
+ /* Resolve ALPN after the cipher suite is selected. HTTP/2 negotiation depends
+ * on the cipher suite. */
+ uint8_t alert;
+ if (!ssl_negotiate_alpn(ssl, &alert, &client_hello)) {
+ ssl3_send_alert(ssl, SSL3_AL_FATAL, alert);
+ return ssl_hs_error;
+ }
/* The PRF hash is now known. */
size_t hash_len =
