From af2db04c9979554ada88d969da6332a827a47599 Mon Sep 17 00:00:00 2001 From: Todd Short Date: Sat, 5 Mar 2016 08:47:55 -0500 Subject: [PATCH] Fix ALPN MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit * Perform ALPN after the SNI callback; the SSL_CTX may change due to that processing * Add flags to indicate that we actually sent ALPN, to properly error out if unexpectedly received. * document ALPN functions * unit tests Backport of commit 817cd0d52f0462039d1fe60462150be7f59d2002 Reviewed-by: Emilia Käsper Reviewed-by: Dr. Stephen Henson --- CHANGES | 4 + doc/ssl/SSL_CTX_set_alpn_select_cb.pod | 126 +++++++++++++++++ ssl/ssl_cert.c | 2 + ssl/ssl_lib.c | 17 ++- ssl/ssl_locl.h | 4 + ssl/ssltest.c | 181 +++++++++++++++++++++++-- ssl/t1_lib.c | 88 ++++++++---- test/testssl | 19 +++ 8 files changed, 399 insertions(+), 42 deletions(-) create mode 100644 doc/ssl/SSL_CTX_set_alpn_select_cb.pod diff --git a/CHANGES b/CHANGES index 4e118e6d11..2d73627aaa 100644 --- a/CHANGES +++ b/CHANGES @@ -4,6 +4,10 @@ Changes between 1.0.2g and 1.0.2h [xx XXX xxxx] + *) Modify behavior of ALPN to invoke callback after SNI/servername + callback, such that updates to the SSL_CTX affect ALPN. + [Todd Short] + *) Remove LOW from the DEFAULT cipher list. This removes singles DES from the default. [Kurt Roeckx] diff --git a/doc/ssl/SSL_CTX_set_alpn_select_cb.pod b/doc/ssl/SSL_CTX_set_alpn_select_cb.pod new file mode 100644 index 0000000000..80ba8ab9c4 --- /dev/null +++ b/doc/ssl/SSL_CTX_set_alpn_select_cb.pod @@ -0,0 +1,126 @@ +=pod + +=head1 NAME + +SSL_CTX_set_alpn_protos, SSL_set_alpn_protos, SSL_CTX_set_alpn_select_cb, +SSL_select_next_proto, SSL_get0_alpn_selected - handle application layer +protocol negotiation (ALPN) + +=head1 SYNOPSIS + + #include + + int SSL_CTX_set_alpn_protos(SSL_CTX *ctx, const unsigned char *protos, + unsigned protos_len); + int SSL_set_alpn_protos(SSL *ssl, const unsigned char *protos, + unsigned protos_len); + void SSL_CTX_set_alpn_select_cb(SSL_CTX *ctx, + int (*cb) (SSL *ssl, + const unsigned char **out, + unsigned char *outlen, + const unsigned char *in, + unsigned int inlen, + void *arg), void *arg); + int SSL_select_next_proto(unsigned char **out, unsigned char *outlen, + const unsigned char *server, + unsigned int server_len, + const unsigned char *client, + unsigned int client_len) + void SSL_get0_alpn_selected(const SSL *ssl, const unsigned char **data, + unsigned int *len); + +=head1 DESCRIPTION + +SSL_CTX_set_alpn_protos() and SSL_set_alpn_protos() are used by the client to +set the list of protocols available to be negotiated. The B must be in +protocol-list format, described below. The length of B is specified in +B. + +SSL_CTX_set_alpn_select_cb() sets the application callback B used by a +server to select which protocol to use for the incoming connection. When B +is NULL, ALPN is not used. The B value is a pointer which is passed to +the application callback. + +B is the application defined callback. The B, B parameters are a +vector in protocol-list format. The value of the B, B vector +should be set to the value of a single protocol selected from the B, +B vector. The B parameter is the pointer set via +SSL_CTX_set_alpn_select_cb(). + +SSL_select_next_proto() is a helper function used to select protocols. It +implements the standard protocol selection. It is expected that this function +is called from the application callback B. The protocol data in B, +B and B, B must be in the protocol-list format +described below. The first item in the B, B list that +matches an item in the B, B list is selected, and returned +in B, B. The B value will point into either B or +B, so it should be copied immediately. If no match is found, the first +item in B, B is returned in B, B. This +function can also be used in the NPN callback. + +SSL_get0_alpn_selected() returns a pointer to the selected protocol in B +with length B. It is not NUL-terminated. B is set to NULL and B +is set to 0 if no protocol has been selected. B must not be freed. + +=head1 NOTES + +The protocol-lists must be in wire-format, which is defined as a vector of +non-empty, 8-bit length-prefixed, byte strings. The length-prefix byte is not +included in the length. Each string is limited to 255 bytes. A byte-string +length of 0 is invalid. A truncated byte-string is invalid. The length of the +vector is not in the vector itself, but in a separate variable. + +Example: + + unsigned char vector[] = { + 6, 's', 'p', 'd', 'y', '/', '1', + 8, 'h', 't', 't', 'p', '/', '1', '.', '1' + }; + unsigned int length = sizeof(vector); + +The ALPN callback is executed after the servername callback; as that servername +callback may update the SSL_CTX, and subsequently, the ALPN callback. + +If there is no ALPN proposed in the ClientHello, the ALPN callback is not +invoked. + +=head1 RETURN VALUES + +SSL_CTX_set_alpn_protos() and SSL_set_alpn_protos() return 0 on success, and +non-0 on failure. WARNING: these functions reverse the return value convention. + +SSL_select_next_proto() returns one of the following: + +=over 4 + +=item OPENSSL_NPN_NEGOTIATED + +A match was found and is returned in B, B. + +=item OPENSSL_NPN_NO_OVERLAP + +No match was found. The first item in B, B is returned in +B, B. + +=back + +The ALPN select callback B, must return one of the following: + +=over 4 + +=item SSL_TLSEXT_ERR_OK + +ALPN protocol selected. + +=item SSL_TLSEXT_ERR_NOACK + +ALPN protocol not selected. + +=back + +=head1 SEE ALSO + +L, L, +L + +=cut diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c index a73f866cb9..acc5361544 100644 --- a/ssl/ssl_cert.c +++ b/ssl/ssl_cert.c @@ -504,6 +504,8 @@ void ssl_cert_free(CERT *c) #ifndef OPENSSL_NO_TLSEXT custom_exts_free(&c->cli_ext); custom_exts_free(&c->srv_ext); + if (c->alpn_proposed) + OPENSSL_free(c->alpn_proposed); #endif OPENSSL_free(c); } diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index f1279bbf91..fd94325bb3 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -244,7 +244,16 @@ int SSL_clear(SSL *s) ssl_clear_hash_ctx(&s->write_hash); s->first_packet = 0; - +#ifndef OPENSSL_NO_TLSEXT + if (s->cert != NULL) { + if (s->cert->alpn_proposed) { + OPENSSL_free(s->cert->alpn_proposed); + s->cert->alpn_proposed = NULL; + } + s->cert->alpn_proposed_len = 0; + s->cert->alpn_sent = 0; + } +#endif #if 1 /* * Check to see if we were changed into a different method, if so, revert @@ -3174,6 +3183,12 @@ SSL_CTX *SSL_set_SSL_CTX(SSL *ssl, SSL_CTX *ctx) ssl->cert->ciphers_rawlen = ocert->ciphers_rawlen; ocert->ciphers_raw = NULL; } +#ifndef OPENSSL_NO_TLSEXT + ssl->cert->alpn_proposed = ocert->alpn_proposed; + ssl->cert->alpn_proposed_len = ocert->alpn_proposed_len; + ocert->alpn_proposed = NULL; + ssl->cert->alpn_sent = ocert->alpn_sent; +#endif ssl_cert_free(ocert); } diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 54c191a7bb..747e718a52 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -688,6 +688,10 @@ typedef struct cert_st { custom_ext_methods cli_ext; custom_ext_methods srv_ext; int references; /* >1 only if SSL_copy_session_id is used */ + /* non-optimal, but here due to compatibility */ + unsigned char *alpn_proposed; /* server */ + unsigned int alpn_proposed_len; + int alpn_sent; /* client */ } CERT; typedef struct sess_cert_st { diff --git a/ssl/ssltest.c b/ssl/ssltest.c index aaf6c6bd89..1db84ad5f9 100644 --- a/ssl/ssltest.c +++ b/ssl/ssltest.c @@ -217,6 +217,9 @@ # define TEST_CLIENT_CERT "../apps/client.pem" #endif +static SSL_CTX *s_ctx = NULL; +static SSL_CTX *s_ctx2 = NULL; + /* * There is really no standard for this, so let's assign some tentative * numbers. In any case, these numbers are only for this test @@ -300,9 +303,51 @@ static BIO *bio_err = NULL; static BIO *bio_stdout = NULL; static const char *alpn_client; -static const char *alpn_server; +static char *alpn_server; +static char *alpn_server2; static const char *alpn_expected; static unsigned char *alpn_selected; +static const char *sn_client; +static const char *sn_server1; +static const char *sn_server2; +static int sn_expect = 0; + +static int servername_cb(SSL *s, int *ad, void *arg) +{ + const char *servername = SSL_get_servername(s, TLSEXT_NAMETYPE_host_name); + if (sn_server2 == NULL) { + BIO_printf(bio_stdout, "Servername 2 is NULL\n"); + return SSL_TLSEXT_ERR_NOACK; + } + + if (servername != NULL) { + if (s_ctx2 != NULL && sn_server2 != NULL && + !strcasecmp(servername, sn_server2)) { + BIO_printf(bio_stdout, "Switching server context.\n"); + SSL_set_SSL_CTX(s, s_ctx2); + } + } + return SSL_TLSEXT_ERR_OK; +} +static int verify_servername(SSL *client, SSL *server) +{ + /* just need to see if sn_context is what we expect */ + SSL_CTX* ctx = SSL_get_SSL_CTX(server); + if (sn_expect == 0) + return 0; + if (sn_expect == 1 && ctx == s_ctx) + return 0; + if (sn_expect == 2 && ctx == s_ctx2) + return 0; + BIO_printf(bio_stdout, "Servername: expected context %d\n", sn_expect); + if (ctx == s_ctx2) + BIO_printf(bio_stdout, "Servername: context is 2\n"); + else if (ctx == s_ctx) + BIO_printf(bio_stdout, "Servername: context is 1\n"); + else + BIO_printf(bio_stdout, "Servername: context is unknown\n"); + return -1; +} /*- * next_protos_parse parses a comma separated list of strings into a string @@ -350,11 +395,12 @@ static int cb_server_alpn(SSL *s, const unsigned char **out, { unsigned char *protos; unsigned short protos_len; + char* alpn_str = arg; - protos = next_protos_parse(&protos_len, alpn_server); + protos = next_protos_parse(&protos_len, alpn_str); if (protos == NULL) { fprintf(stderr, "failed to parser ALPN server protocol string: %s\n", - alpn_server); + alpn_str); abort(); } @@ -417,8 +463,17 @@ static int verify_alpn(SSL *client, SSL *server) BIO_printf(bio_stdout, "', server: '"); BIO_write(bio_stdout, server_proto, server_proto_len); BIO_printf(bio_stdout, "'\n"); - BIO_printf(bio_stdout, "ALPN configured: client: '%s', server: '%s'\n", - alpn_client, alpn_server); + BIO_printf(bio_stdout, "ALPN configured: client: '%s', server: ", + alpn_client); + if (SSL_get_SSL_CTX(server) == s_ctx2) { + BIO_printf(bio_stdout, "'%s'\n", + alpn_server2); + } else if (SSL_get_SSL_CTX(server) == s_ctx){ + BIO_printf(bio_stdout, "'%s'\n", + alpn_server); + } else { + BIO_printf(bio_stdout, "unknown\n"); + } return -1; } @@ -756,8 +811,15 @@ static void sv_usage(void) " -custom_ext - try various custom extension callbacks\n"); fprintf(stderr, " -alpn_client - have client side offer ALPN\n"); fprintf(stderr, " -alpn_server - have server side offer ALPN\n"); + fprintf(stderr, " -alpn_server1 - alias for -alpn_server\n"); + fprintf(stderr, " -alpn_server2 - have server side context 2 offer ALPN\n"); fprintf(stderr, " -alpn_expected - the ALPN protocol that should be negotiated\n"); + fprintf(stderr, " -sn_client - have client request this servername\n"); + fprintf(stderr, " -sn_server1 - have server context 1 respond to this servername\n"); + fprintf(stderr, " -sn_server2 - have server context 2 respond to this servername\n"); + fprintf(stderr, " -sn_expect1 - expected server 1\n"); + fprintf(stderr, " -sn_expect2 - expected server 2\n"); } static void print_details(SSL *c_ssl, const char *prefix) @@ -896,7 +958,6 @@ int main(int argc, char *argv[]) #ifndef OPENSSL_NO_ECDH char *named_curve = NULL; #endif - SSL_CTX *s_ctx = NULL; SSL_CTX *c_ctx = NULL; const SSL_METHOD *meth = NULL; SSL *c_ssl, *s_ssl; @@ -1151,14 +1212,35 @@ int main(int argc, char *argv[]) if (--argc < 1) goto bad; alpn_client = *(++argv); - } else if (strcmp(*argv, "-alpn_server") == 0) { + } else if (strcmp(*argv, "-alpn_server") == 0 || + strcmp(*argv, "-alpn_server1") == 0) { if (--argc < 1) goto bad; alpn_server = *(++argv); + } else if (strcmp(*argv, "-alpn_server2") == 0) { + if (--argc < 1) + goto bad; + alpn_server2 = *(++argv); } else if (strcmp(*argv, "-alpn_expected") == 0) { if (--argc < 1) goto bad; alpn_expected = *(++argv); + } else if (strcmp(*argv, "-sn_client") == 0) { + if (--argc < 1) + goto bad; + sn_client = *(++argv); + } else if (strcmp(*argv, "-sn_server1") == 0) { + if (--argc < 1) + goto bad; + sn_server1 = *(++argv); + } else if (strcmp(*argv, "-sn_server2") == 0) { + if (--argc < 1) + goto bad; + sn_server2 = *(++argv); + } else if (strcmp(*argv, "-sn_expect1") == 0) { + sn_expect = 1; + } else if (strcmp(*argv, "-sn_expect2") == 0) { + sn_expect = 2; } else { fprintf(stderr, "unknown option %s\n", *argv); badop = 1; @@ -1304,7 +1386,8 @@ int main(int argc, char *argv[]) c_ctx = SSL_CTX_new(meth); s_ctx = SSL_CTX_new(meth); - if ((c_ctx == NULL) || (s_ctx == NULL)) { + s_ctx2 = SSL_CTX_new(meth); /* no SSL_CTX_dup! */ + if ((c_ctx == NULL) || (s_ctx == NULL) || (s_ctx2 == NULL)) { ERR_print_errors(bio_err); goto end; } @@ -1312,7 +1395,9 @@ int main(int argc, char *argv[]) if (cipher != NULL) { SSL_CTX_set_cipher_list(c_ctx, cipher); SSL_CTX_set_cipher_list(s_ctx, cipher); + SSL_CTX_set_cipher_list(s_ctx2, cipher); } + #ifndef OPENSSL_NO_DH if (!no_dhe) { if (dhe1024dsa) { @@ -1320,12 +1405,14 @@ int main(int argc, char *argv[]) * use SSL_OP_SINGLE_DH_USE to avoid small subgroup attacks */ SSL_CTX_set_options(s_ctx, SSL_OP_SINGLE_DH_USE); + SSL_CTX_set_options(s_ctx2, SSL_OP_SINGLE_DH_USE); dh = get_dh1024dsa(); } else if (dhe512) dh = get_dh512(); else dh = get_dh1024(); SSL_CTX_set_tmp_dh(s_ctx, dh); + SSL_CTX_set_tmp_dh(s_ctx2, dh); DH_free(dh); } #else @@ -1353,7 +1440,9 @@ int main(int argc, char *argv[]) } SSL_CTX_set_tmp_ecdh(s_ctx, ecdh); + SSL_CTX_set_tmp_ecdh(s_ctx2, ecdh); SSL_CTX_set_options(s_ctx, SSL_OP_SINGLE_ECDH_USE); + SSL_CTX_set_options(s_ctx2, SSL_OP_SINGLE_ECDH_USE); EC_KEY_free(ecdh); } #else @@ -1362,15 +1451,18 @@ int main(int argc, char *argv[]) #ifndef OPENSSL_NO_RSA SSL_CTX_set_tmp_rsa_callback(s_ctx, tmp_rsa_cb); + SSL_CTX_set_tmp_rsa_callback(s_ctx2, tmp_rsa_cb); #endif #ifdef TLSEXT_TYPE_opaque_prf_input SSL_CTX_set_tlsext_opaque_prf_input_callback(c_ctx, opaque_prf_input_cb); SSL_CTX_set_tlsext_opaque_prf_input_callback(s_ctx, opaque_prf_input_cb); + SSL_CTX_set_tlsext_opaque_prf_input_callback(s_ctx2, opaque_prf_input_cb); /* or &co2 or NULL */ SSL_CTX_set_tlsext_opaque_prf_input_callback_arg(c_ctx, &co1); /* or &so2 or NULL */ SSL_CTX_set_tlsext_opaque_prf_input_callback_arg(s_ctx, &so1); + SSL_CTX_set_tlsext_opaque_prf_input_callback_arg(s_ctx2, &so1); #endif if (!SSL_CTX_use_certificate_file(s_ctx, server_cert, SSL_FILETYPE_PEM)) { @@ -1383,6 +1475,16 @@ int main(int argc, char *argv[]) goto end; } + if (!SSL_CTX_use_certificate_file(s_ctx2, server_cert, SSL_FILETYPE_PEM)) { + ERR_print_errors(bio_err); + } else if (!SSL_CTX_use_PrivateKey_file(s_ctx2, + (server_key ? server_key : + server_cert), + SSL_FILETYPE_PEM)) { + ERR_print_errors(bio_err); + goto end; + } + if (client_auth) { SSL_CTX_use_certificate_file(c_ctx, client_cert, SSL_FILETYPE_PEM); SSL_CTX_use_PrivateKey_file(c_ctx, @@ -1392,6 +1494,8 @@ int main(int argc, char *argv[]) if ((!SSL_CTX_load_verify_locations(s_ctx, CAfile, CApath)) || (!SSL_CTX_set_default_verify_paths(s_ctx)) || + (!SSL_CTX_load_verify_locations(s_ctx2, CAfile, CApath)) || + (!SSL_CTX_set_default_verify_paths(s_ctx2)) || (!SSL_CTX_load_verify_locations(c_ctx, CAfile, CApath)) || (!SSL_CTX_set_default_verify_paths(c_ctx))) { /* fprintf(stderr,"SSL_load_verify_locations\n"); */ @@ -1406,6 +1510,11 @@ int main(int argc, char *argv[]) verify_callback); SSL_CTX_set_cert_verify_callback(s_ctx, app_verify_callback, &app_verify_arg); + SSL_CTX_set_verify(s_ctx2, + SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, + verify_callback); + SSL_CTX_set_cert_verify_callback(s_ctx2, app_verify_callback, + &app_verify_arg); } if (server_auth) { BIO_printf(bio_err, "server authentication\n"); @@ -1418,6 +1527,8 @@ int main(int argc, char *argv[]) int session_id_context = 0; SSL_CTX_set_session_id_context(s_ctx, (void *)&session_id_context, sizeof session_id_context); + SSL_CTX_set_session_id_context(s_ctx2, (void *)&session_id_context, + sizeof session_id_context); } /* Use PSK only if PSK key is given */ @@ -1436,6 +1547,7 @@ int main(int argc, char *argv[]) #ifndef OPENSSL_NO_PSK SSL_CTX_set_psk_client_callback(c_ctx, psk_client_callback); SSL_CTX_set_psk_server_callback(s_ctx, psk_server_callback); + SSL_CTX_set_psk_server_callback(s_ctx2, psk_server_callback); if (debug) BIO_printf(bio_err, "setting PSK identity hint to s_ctx\n"); if (!SSL_CTX_use_psk_identity_hint(s_ctx, "ctx server identity_hint")) { @@ -1443,6 +1555,11 @@ int main(int argc, char *argv[]) ERR_print_errors(bio_err); goto end; } + if (!SSL_CTX_use_psk_identity_hint(s_ctx2, "ctx server identity_hint")) { + BIO_printf(bio_err, "error setting PSK identity hint to s_ctx2\n"); + ERR_print_errors(bio_err); + goto end; + } #endif } #ifndef OPENSSL_NO_SRP @@ -1461,8 +1578,11 @@ int main(int argc, char *argv[]) if (srp_server_arg.expected_user != NULL) { SSL_CTX_set_verify(s_ctx, SSL_VERIFY_NONE, verify_callback); + SSL_CTX_set_verify(s_ctx2, SSL_VERIFY_NONE, verify_callback); SSL_CTX_set_srp_cb_arg(s_ctx, &srp_server_arg); + SSL_CTX_set_srp_cb_arg(s_ctx2, &srp_server_arg); SSL_CTX_set_srp_username_callback(s_ctx, ssl_srp_server_param_cb); + SSL_CTX_set_srp_username_callback(s_ctx2, ssl_srp_server_param_cb); } #endif @@ -1475,11 +1595,16 @@ int main(int argc, char *argv[]) NULL, NULL, NULL, serverinfo_cli_parse_cb, NULL); - if (serverinfo_file) + if (serverinfo_file) { if (!SSL_CTX_use_serverinfo_file(s_ctx, serverinfo_file)) { BIO_printf(bio_err, "missing serverinfo file\n"); goto end; } + if (!SSL_CTX_use_serverinfo_file(s_ctx2, serverinfo_file)) { + BIO_printf(bio_err, "missing serverinfo file\n"); + goto end; + } + } if (custom_ext) { SSL_CTX_add_client_custom_ext(c_ctx, CUSTOM_EXT_TYPE_0, @@ -1515,10 +1640,29 @@ int main(int argc, char *argv[]) custom_ext_3_srv_add_cb, NULL, NULL, custom_ext_3_srv_parse_cb, NULL); + + SSL_CTX_add_server_custom_ext(s_ctx2, CUSTOM_EXT_TYPE_0, + custom_ext_0_srv_add_cb, + NULL, NULL, + custom_ext_0_srv_parse_cb, NULL); + SSL_CTX_add_server_custom_ext(s_ctx2, CUSTOM_EXT_TYPE_1, + custom_ext_1_srv_add_cb, + NULL, NULL, + custom_ext_1_srv_parse_cb, NULL); + SSL_CTX_add_server_custom_ext(s_ctx2, CUSTOM_EXT_TYPE_2, + custom_ext_2_srv_add_cb, + NULL, NULL, + custom_ext_2_srv_parse_cb, NULL); + SSL_CTX_add_server_custom_ext(s_ctx2, CUSTOM_EXT_TYPE_3, + custom_ext_3_srv_add_cb, + NULL, NULL, + custom_ext_3_srv_parse_cb, NULL); } if (alpn_server) - SSL_CTX_set_alpn_select_cb(s_ctx, cb_server_alpn, NULL); + SSL_CTX_set_alpn_select_cb(s_ctx, cb_server_alpn, alpn_server); + if (alpn_server2) + SSL_CTX_set_alpn_select_cb(s_ctx2, cb_server_alpn, alpn_server2); if (alpn_client) { unsigned short alpn_len; @@ -1532,9 +1676,15 @@ int main(int argc, char *argv[]) OPENSSL_free(alpn); } + if (sn_server1 || sn_server2) + SSL_CTX_set_tlsext_servername_callback(s_ctx, servername_cb); + c_ssl = SSL_new(c_ctx); s_ssl = SSL_new(s_ctx); + if (sn_client) + SSL_set_tlsext_host_name(c_ssl, sn_client); + #ifndef OPENSSL_NO_KRB5 if (c_ssl && c_ssl->kssl_ctx) { char localhost[MAXHOSTNAMELEN + 2]; @@ -1588,12 +1738,19 @@ int main(int argc, char *argv[]) #endif } + if (verify_alpn(c_ssl, s_ssl) < 0) + ret = 1; + if (verify_servername(c_ssl, s_ssl) < 0) + ret = 1; + SSL_free(s_ssl); SSL_free(c_ssl); end: if (s_ctx != NULL) SSL_CTX_free(s_ctx); + if (s_ctx2 != NULL) + SSL_CTX_free(s_ctx2); if (c_ctx != NULL) SSL_CTX_free(c_ctx); @@ -1961,10 +2118,6 @@ int doit_biopair(SSL *s_ssl, SSL *c_ssl, long count, ret = 1; goto err; } - if (verify_alpn(c_ssl, s_ssl) < 0) { - ret = 1; - goto err; - } if (custom_ext_error) { ret = 1; diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 0bf0ea5363..dd5bd0050d 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -1539,6 +1539,7 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf, s2n(s->alpn_client_proto_list_len, ret); memcpy(ret, s->alpn_client_proto_list, s->alpn_client_proto_list_len); ret += s->alpn_client_proto_list_len; + s->cert->alpn_sent = 1; } # ifndef OPENSSL_NO_SRTP if (SSL_IS_DTLS(s) && SSL_get_srtp_profiles(s)) { @@ -1906,7 +1907,7 @@ static void ssl_check_for_safari(SSL *s, const unsigned char *data, # endif /* !OPENSSL_NO_EC */ /* - * tls1_alpn_handle_client_hello is called to process the ALPN extension in a + * tls1_alpn_handle_client_hello is called to save the ALPN extension in a * ClientHello. data: the contents of the extension, not including the type * and length. data_len: the number of bytes in |data| al: a pointer to the * alert value to send in the event of a non-zero return. returns: 0 on @@ -1917,12 +1918,6 @@ static int tls1_alpn_handle_client_hello(SSL *s, const unsigned char *data, { unsigned i; unsigned proto_len; - const unsigned char *selected; - unsigned char selected_len; - int r; - - if (s->ctx->alpn_select_cb == NULL) - return 0; if (data_len < 2) goto parse_error; @@ -1953,19 +1948,15 @@ static int tls1_alpn_handle_client_hello(SSL *s, const unsigned char *data, i += proto_len; } - r = s->ctx->alpn_select_cb(s, &selected, &selected_len, data, data_len, - s->ctx->alpn_select_cb_arg); - if (r == SSL_TLSEXT_ERR_OK) { - if (s->s3->alpn_selected) - OPENSSL_free(s->s3->alpn_selected); - s->s3->alpn_selected = OPENSSL_malloc(selected_len); - if (!s->s3->alpn_selected) { - *al = SSL_AD_INTERNAL_ERROR; - return -1; - } - memcpy(s->s3->alpn_selected, selected, selected_len); - s->s3->alpn_selected_len = selected_len; + if (s->cert->alpn_proposed != NULL) + OPENSSL_free(s->cert->alpn_proposed); + s->cert->alpn_proposed = OPENSSL_malloc(data_len); + if (s->cert->alpn_proposed == NULL) { + *al = SSL_AD_INTERNAL_ERROR; + return -1; } + memcpy(s->cert->alpn_proposed, data, data_len); + s->cert->alpn_proposed_len = data_len; return 0; parse_error: @@ -1973,6 +1964,43 @@ static int tls1_alpn_handle_client_hello(SSL *s, const unsigned char *data, return -1; } +/* + * Process the ALPN extension in a ClientHello. + * ret: a pointer to the TLSEXT return value: SSL_TLSEXT_ERR_* + * al: a pointer to the alert value to send in the event of a failure. + * returns 1 on success, 0 on failure: al/ret set only on failure + */ +static int tls1_alpn_handle_client_hello_late(SSL *s, int *ret, int *al) +{ + const unsigned char *selected = NULL; + unsigned char selected_len = 0; + + if (s->ctx->alpn_select_cb != NULL && s->cert->alpn_proposed != NULL) { + int r = s->ctx->alpn_select_cb(s, &selected, &selected_len, + s->cert->alpn_proposed, + s->cert->alpn_proposed_len, + s->ctx->alpn_select_cb_arg); + + if (r == SSL_TLSEXT_ERR_OK) { + OPENSSL_free(s->s3->alpn_selected); + s->s3->alpn_selected = OPENSSL_malloc(selected_len); + if (s->s3->alpn_selected == NULL) { + *al = SSL_AD_INTERNAL_ERROR; + *ret = SSL_TLSEXT_ERR_ALERT_FATAL; + return 0; + } + memcpy(s->s3->alpn_selected, selected, selected_len); + s->s3->alpn_selected_len = selected_len; +# ifndef OPENSSL_NO_NEXTPROTONEG + /* ALPN takes precedence over NPN. */ + s->s3->next_proto_neg_seen = 0; +# endif + } + } + + return 1; +} + static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *limit, int *al) { @@ -1992,6 +2020,12 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p, OPENSSL_free(s->s3->alpn_selected); s->s3->alpn_selected = NULL; } + s->s3->alpn_selected_len = 0; + if (s->cert->alpn_proposed) { + OPENSSL_free(s->cert->alpn_proposed); + s->cert->alpn_proposed = NULL; + } + s->cert->alpn_proposed_len = 0; # ifndef OPENSSL_NO_HEARTBEATS s->tlsext_heartbeat &= ~(SSL_TLSEXT_HB_ENABLED | SSL_TLSEXT_HB_DONT_SEND_REQUESTS); @@ -2359,8 +2393,7 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p, # endif # ifndef OPENSSL_NO_NEXTPROTONEG else if (type == TLSEXT_TYPE_next_proto_neg && - s->s3->tmp.finish_md_len == 0 && - s->s3->alpn_selected == NULL) { + s->s3->tmp.finish_md_len == 0) { /*- * We shouldn't accept this extension on a * renegotiation. @@ -2383,13 +2416,9 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p, # endif else if (type == TLSEXT_TYPE_application_layer_protocol_negotiation && - s->ctx->alpn_select_cb && s->s3->tmp.finish_md_len == 0) { + s->s3->tmp.finish_md_len == 0) { if (tls1_alpn_handle_client_hello(s, data, size, al) != 0) return 0; -# ifndef OPENSSL_NO_NEXTPROTONEG - /* ALPN takes precedence over NPN. */ - s->s3->next_proto_neg_seen = 0; -# endif } /* session ticket processed earlier */ @@ -2698,7 +2727,7 @@ static int ssl_scan_serverhello_tlsext(SSL *s, unsigned char **p, unsigned len; /* We must have requested it. */ - if (s->alpn_client_proto_list == NULL) { + if (!s->cert->alpn_sent) { *al = TLS1_AD_UNSUPPORTED_EXTENSION; return 0; } @@ -2863,6 +2892,7 @@ int ssl_prepare_clienthello_tlsext(SSL *s) } # endif + s->cert->alpn_sent = 0; return 1; } @@ -3066,6 +3096,10 @@ int ssl_check_clienthello_tlsext_late(SSL *s) } else s->tlsext_status_expected = 0; + if (!tls1_alpn_handle_client_hello_late(s, &ret, &al)) { + goto err; + } + err: switch (ret) { case SSL_TLSEXT_ERR_ALERT_FATAL: diff --git a/test/testssl b/test/testssl index 747e4ba381..a6f9fa770b 100644 --- a/test/testssl +++ b/test/testssl @@ -236,6 +236,17 @@ $ssltest -bio_pair -tls1 -serverinfo_file $serverinfo -serverinfo_tack || exit 1 $ssltest -bio_pair -tls1 -serverinfo_file $serverinfo -serverinfo_sct -serverinfo_tack || exit 1 $ssltest -bio_pair -tls1 -custom_ext -serverinfo_file $serverinfo -serverinfo_sct -serverinfo_tack || exit 1 +############################################################################# +# SNI tests + +$ssltest -bio_pair -sn_client foo || exit 1 +$ssltest -bio_pair -sn_server1 foo || exit 1 +$ssltest -bio_pair -sn_client foo -sn_server1 foo -sn_expect1 || exit 1 +$ssltest -bio_pair -sn_client foo -sn_server1 bar -sn_expect1 || exit 1 +$ssltest -bio_pair -sn_client foo -sn_server1 foo -sn_server2 bar -sn_expect1 || exit 1 +$ssltest -bio_pair -sn_client bar -sn_server1 foo -sn_server2 bar -sn_expect2 || exit 1 +# Negative test - make sure it doesn't crash, and doesn't switch contexts +$ssltest -bio_pair -sn_client foobar -sn_server1 foo -sn_server2 bar -sn_expect1 || exit 1 ############################################################################# # ALPN tests @@ -249,6 +260,14 @@ $ssltest -bio_pair -tls1 -alpn_client bar,foo -alpn_server bar,foo -alpn_expecte $ssltest -bio_pair -tls1 -alpn_client foo,bar -alpn_server bar,foo -alpn_expected bar || exit 1 $ssltest -bio_pair -tls1 -alpn_client baz -alpn_server bar,foo || exit 1 + +############################################################################# +# ALPN + SNI + +$ssltest -bio_pair -alpn_client foo,bar -sn_client alice -alpn_server1 foo,123 -sn_server1 alice -alpn_server2 bar,456 -sn_server2 bob -alpn_expected foo || exit 1 +$ssltest -bio_pair -alpn_client foo,bar -sn_client bob -alpn_server1 foo,123 -sn_server1 alice -alpn_server2 bar,456 -sn_server2 bob -alpn_expected bar || exit 1 +$ssltest -bio_pair -alpn_client foo,bar -sn_client bob -sn_server1 alice -alpn_server2 bar,456 -sn_server2 bob -alpn_expected bar || exit 1 + if ../util/shlib_wrap.sh ../apps/openssl no-srp; then echo skipping SRP tests else -- 2.25.1