diff options
author | Todd Short <tshort@akamai.com> | 2016-03-05 08:47:55 -0500 |
---|---|---|
committer | Rich Salz <rsalz@openssl.org> | 2016-03-08 09:03:05 -0500 |
commit | 817cd0d52f0462039d1fe60462150be7f59d2002 (patch) | |
tree | dd075e91d6add68a3c4f493db1e66cce11c990a9 /ssl | |
parent | f18ce934889a36db42b7988e8acca9ac4f23299f (diff) |
GH787: Fix ALPN
* 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.
* clean up ssl3_free() no need to explicitly clear when doing memset
* document ALPN functions
Signed-off-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Emilia Käsper <emilia@openssl.org>
Diffstat (limited to 'ssl')
-rw-r--r-- | ssl/s3_lib.c | 20 | ||||
-rw-r--r-- | ssl/ssl_lib.c | 12 | ||||
-rw-r--r-- | ssl/ssl_locl.h | 7 | ||||
-rw-r--r-- | ssl/t1_lib.c | 77 |
4 files changed, 68 insertions, 48 deletions
diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index 78aaf7bae8..134c7e636c 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -3047,6 +3047,7 @@ void ssl3_free(SSL *s) OPENSSL_free(s->s3->tmp.peer_sigalgs); ssl3_free_digest_list(s); OPENSSL_free(s->s3->alpn_selected); + OPENSSL_free(s->s3->alpn_proposed); #ifndef OPENSSL_NO_SRP SSL_SRP_CTX_free(s); @@ -3060,37 +3061,24 @@ void ssl3_clear(SSL *s) ssl3_cleanup_key_block(s); sk_X509_NAME_pop_free(s->s3->tmp.ca_names, X509_NAME_free); OPENSSL_free(s->s3->tmp.ciphers_raw); - s->s3->tmp.ciphers_raw = NULL; OPENSSL_clear_free(s->s3->tmp.pms, s->s3->tmp.pmslen); - s->s3->tmp.pms = NULL; OPENSSL_free(s->s3->tmp.peer_sigalgs); - s->s3->tmp.peer_sigalgs = NULL; -#ifndef OPENSSL_NO_EC - s->s3->is_probably_safari = 0; -#endif #if !defined(OPENSSL_NO_EC) || !defined(OPENSSL_NO_DH) EVP_PKEY_free(s->s3->tmp.pkey); - s->s3->tmp.pkey = NULL; EVP_PKEY_free(s->s3->peer_tmp); - s->s3->peer_tmp = NULL; #endif /* !OPENSSL_NO_EC */ ssl3_free_digest_list(s); - if (s->s3->alpn_selected) { - OPENSSL_free(s->s3->alpn_selected); - s->s3->alpn_selected = NULL; - } + OPENSSL_free(s->s3->alpn_selected); + OPENSSL_free(s->s3->alpn_proposed); + /* NULL/zero-out everything in the s3 struct */ memset(s->s3, 0, sizeof(*s->s3)); ssl_free_wbio_buffer(s); - s->s3->renegotiate = 0; - s->s3->total_renegotiations = 0; - s->s3->num_renegotiations = 0; - s->s3->in_read_app_data = 0; s->version = SSL3_VERSION; #if !defined(OPENSSL_NO_NEXTPROTONEG) diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index 13f4ccdc4a..a1c8da8890 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -2220,15 +2220,14 @@ void SSL_CTX_set_next_proto_select_cb(SSL_CTX *ctx, * length-prefixed strings). Returns 0 on success. */ int SSL_CTX_set_alpn_protos(SSL_CTX *ctx, const unsigned char *protos, - unsigned protos_len) + unsigned int protos_len) { OPENSSL_free(ctx->alpn_client_proto_list); - ctx->alpn_client_proto_list = OPENSSL_malloc(protos_len); + ctx->alpn_client_proto_list = OPENSSL_memdup(protos, protos_len); if (ctx->alpn_client_proto_list == NULL) { SSLerr(SSL_F_SSL_CTX_SET_ALPN_PROTOS, ERR_R_MALLOC_FAILURE); return 1; } - memcpy(ctx->alpn_client_proto_list, protos, protos_len); ctx->alpn_client_proto_list_len = protos_len; return 0; @@ -2240,15 +2239,14 @@ int SSL_CTX_set_alpn_protos(SSL_CTX *ctx, const unsigned char *protos, * length-prefixed strings). Returns 0 on success. */ int SSL_set_alpn_protos(SSL *ssl, const unsigned char *protos, - unsigned protos_len) + unsigned int protos_len) { OPENSSL_free(ssl->alpn_client_proto_list); - ssl->alpn_client_proto_list = OPENSSL_malloc(protos_len); + ssl->alpn_client_proto_list = OPENSSL_memdup(protos, protos_len); if (ssl->alpn_client_proto_list == NULL) { SSLerr(SSL_F_SSL_SET_ALPN_PROTOS, ERR_R_MALLOC_FAILURE); return 1; } - memcpy(ssl->alpn_client_proto_list, protos, protos_len); ssl->alpn_client_proto_list_len = protos_len; return 0; @@ -2278,7 +2276,7 @@ void SSL_CTX_set_alpn_select_cb(SSL_CTX *ctx, * respond with a negotiated protocol then |*len| will be zero. */ void SSL_get0_alpn_selected(const SSL *ssl, const unsigned char **data, - unsigned *len) + unsigned int *len) { *data = NULL; if (ssl->s3) diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 064c22c25a..4d816de18d 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -1372,7 +1372,12 @@ typedef struct ssl3_state_st { * that the server selected once the ServerHello has been processed. */ unsigned char *alpn_selected; - unsigned alpn_selected_len; + size_t alpn_selected_len; + /* used by the server to know what options were proposed */ + unsigned char *alpn_proposed; + size_t alpn_proposed_len; + /* used by the client to know if it actually sent alpn */ + int alpn_sent; # ifndef OPENSSL_NO_EC /* diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 70c47c8e65..2161d155e8 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -1413,6 +1413,11 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf, } #endif + /* + * finish_md_len is non-zero during a renegotiation, so + * this avoids sending ALPN during the renegotiation + * (see longer comment below) + */ if (s->alpn_client_proto_list && !s->s3->tmp.finish_md_len) { if ((size_t)(limit - ret) < 6 + s->alpn_client_proto_list_len) return NULL; @@ -1421,6 +1426,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->s3->alpn_sent = 1; } #ifndef OPENSSL_NO_SRTP if (SSL_IS_DTLS(s) && SSL_get_srtp_profiles(s)) { @@ -1701,9 +1707,9 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf, s2n(0, ret); } - if (s->s3->alpn_selected) { + if (s->s3->alpn_selected != NULL) { const unsigned char *selected = s->s3->alpn_selected; - unsigned len = s->s3->alpn_selected_len; + unsigned int len = s->s3->alpn_selected_len; if ((long)(limit - ret - 4 - 2 - 1 - len) < 0) return NULL; @@ -1725,16 +1731,13 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf, } /* - * Process the ALPN extension in a ClientHello. + * Save the ALPN extension in a ClientHello. * pkt: the contents of the ALPN extension, not including type and length. * al: a pointer to the alert value to send in the event of a failure. * returns: 1 on success, 0 on error. */ static int tls1_alpn_handle_client_hello(SSL *s, PACKET *pkt, int *al) { - const unsigned char *selected; - unsigned char selected_len; - int r; PACKET protocol_list, save_protocol_list, protocol; *al = SSL_AD_DECODE_ERROR; @@ -1753,25 +1756,47 @@ static int tls1_alpn_handle_client_hello(SSL *s, PACKET *pkt, int *al) } } while (PACKET_remaining(&protocol_list) != 0); - if (s->ctx->alpn_select_cb == NULL) - return 1; + if (!PACKET_memdup(&save_protocol_list, + &s->s3->alpn_proposed, + &s->s3->alpn_proposed_len)) { + *al = TLS1_AD_INTERNAL_ERROR; + return 0; + } + + 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 + */ +static int tls1_alpn_handle_client_hello_late(SSL *s, int *ret, int *al) +{ + const unsigned char *selected = NULL; + unsigned char selected_len = 0; - r = s->ctx->alpn_select_cb(s, &selected, &selected_len, - PACKET_data(&save_protocol_list), - PACKET_remaining(&save_protocol_list), - 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; + if (s->ctx->alpn_select_cb != NULL && s->s3->alpn_proposed != NULL) { + int r = s->ctx->alpn_select_cb(s, &selected, &selected_len, + s->s3->alpn_proposed, + s->s3->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_memdup(selected, selected_len); + if (s->s3->alpn_selected == NULL) { + *al = SSL_AD_INTERNAL_ERROR; + *ret = SSL_TLSEXT_ERR_ALERT_FATAL; + return 0; + } + s->s3->alpn_selected_len = selected_len; + } else { + *al = SSL_AD_NO_APPLICATION_PROTOCOL; + *ret = SSL_TLSEXT_ERR_ALERT_FATAL; return 0; } - memcpy(s->s3->alpn_selected, selected, selected_len); - s->s3->alpn_selected_len = selected_len; - } else { - *al = SSL_AD_NO_APPLICATION_PROTOCOL; - return 0; } return 1; @@ -2484,7 +2509,7 @@ static int ssl_scan_serverhello_tlsext(SSL *s, PACKET *pkt, int *al) else if (type == TLSEXT_TYPE_application_layer_protocol_negotiation) { unsigned len; /* We must have requested it. */ - if (s->alpn_client_proto_list == NULL) { + if (!s->s3->alpn_sent) { *al = TLS1_AD_UNSUPPORTED_EXTENSION; return 0; } @@ -2617,7 +2642,7 @@ static int ssl_scan_serverhello_tlsext(SSL *s, PACKET *pkt, int *al) int ssl_prepare_clienthello_tlsext(SSL *s) { - + s->s3->alpn_sent = 0; return 1; } @@ -2776,6 +2801,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: |