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>
diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index 78aaf7b..134c7e6 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -3047,6 +3047,7 @@
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 @@
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 13f4ccd..a1c8da8 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -2220,15 +2220,14 @@
* 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 @@
* 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 @@
* 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 064c22c..4d816de 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -1372,7 +1372,12 @@
* 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 70c47c8..2161d15 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1413,6 +1413,11 @@
}
#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 @@
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 @@
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 @@
}
/*
- * 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 @@
}
} 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;
+ }
- 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;
+ 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;
+
+ 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 @@
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 @@
int ssl_prepare_clienthello_tlsext(SSL *s)
{
-
+ s->s3->alpn_sent = 0;
return 1;
}
@@ -2776,6 +2801,10 @@
} 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:
