From 715d5aefa6ef8d68edfb816a5a8900b9fb5a3bb3 Mon Sep 17 00:00:00 2001 From: tb Date: Fri, 10 Sep 2021 09:25:29 +0000 Subject: [PATCH] Do not ignore SSL_TLSEXT_ERR_FATAL from the ALPN callback As reported by Jeremy Harris, we inherited a strange behavior from OpenSSL, in that we ignore the SSL_TLSEXT_ERR_FATAL return from the ALPN callback. RFC 7301, 3.2 states: 'In the event that the server supports no protocols that the client advertises, then the server SHALL respond with a fatal "no_application_protocol" alert.' Honor this requirement and succeed only on SSL_TLSEXT_ERR_{OK,NOACK} which is the current behavior of OpenSSL. The documentation change is taken from OpenSSL 1.1.1 as well. As pointed out by jsing, there is more to be fixed here: - ensure that the same protocol is selected on session resumption - should the callback be called even if no ALPN extension was sent? - ensure for TLSv1.2 and earlier that the SNI has already been processed ok beck jsing --- lib/libssl/man/SSL_CTX_set_alpn_select_cb.3 | 10 +++++++--- lib/libssl/ssl.h | 5 ++++- lib/libssl/ssl_err.c | 3 ++- lib/libssl/ssl_tlsext.c | 20 ++++++++++++++++++-- 4 files changed, 31 insertions(+), 7 deletions(-) diff --git a/lib/libssl/man/SSL_CTX_set_alpn_select_cb.3 b/lib/libssl/man/SSL_CTX_set_alpn_select_cb.3 index 540fd011f55..683b6696e3a 100644 --- a/lib/libssl/man/SSL_CTX_set_alpn_select_cb.3 +++ b/lib/libssl/man/SSL_CTX_set_alpn_select_cb.3 @@ -1,4 +1,4 @@ -.\" $OpenBSD: SSL_CTX_set_alpn_select_cb.3,v 1.7 2018/03/23 14:28:16 schwarze Exp $ +.\" $OpenBSD: SSL_CTX_set_alpn_select_cb.3,v 1.8 2021/09/10 09:25:29 tb Exp $ .\" OpenSSL 87b81496 Apr 19 12:38:27 2017 -0400 .\" OpenSSL b97fdb57 Nov 11 09:33:09 2016 +0100 .\" @@ -49,7 +49,7 @@ .\" ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED .\" OF THE POSSIBILITY OF SUCH DAMAGE. .\" -.Dd $Mdocdate: March 23 2018 $ +.Dd $Mdocdate: September 10 2021 $ .Dt SSL_CTX_SET_ALPN_SELECT_CB 3 .Os .Sh NAME @@ -252,8 +252,12 @@ must return one of the following: .Bl -tag -width Ds .It SSL_TLSEXT_ERR_OK ALPN protocol selected. +.It SSL_TLSEXT_ERR_ALERT_FATAL +There was no overlap between the client's supplied list and the +server configuration. .It SSL_TLSEXT_ERR_NOACK -ALPN protocol not selected. +ALPN protocol not selected, e.g., because no ALPN protocols are +configured for this connection. .El .Sh SEE ALSO .Xr ssl 3 , diff --git a/lib/libssl/ssl.h b/lib/libssl/ssl.h index 7da3658d3f6..fba9ea243f1 100644 --- a/lib/libssl/ssl.h +++ b/lib/libssl/ssl.h @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl.h,v 1.201 2021/09/10 08:59:56 tb Exp $ */ +/* $OpenBSD: ssl.h,v 1.202 2021/09/10 09:25:29 tb Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -1995,6 +1995,9 @@ void ERR_load_SSL_strings(void); #define SSL_R_MISSING_VERIFY_MESSAGE 174 #define SSL_R_MULTIPLE_SGC_RESTARTS 346 #define SSL_R_NON_SSLV2_INITIAL_PACKET 175 +#if defined(LIBRESSL_INTERNAL) +#define SSL_R_NO_APPLICATION_PROTOCOL 235 +#endif #define SSL_R_NO_CERTIFICATES_RETURNED 176 #define SSL_R_NO_CERTIFICATE_ASSIGNED 177 #define SSL_R_NO_CERTIFICATE_RETURNED 178 diff --git a/lib/libssl/ssl_err.c b/lib/libssl/ssl_err.c index 8507690f79f..9ea7cd469ad 100644 --- a/lib/libssl/ssl_err.c +++ b/lib/libssl/ssl_err.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_err.c,v 1.38 2021/05/16 08:24:21 jsing Exp $ */ +/* $OpenBSD: ssl_err.c,v 1.39 2021/09/10 09:25:29 tb Exp $ */ /* ==================================================================== * Copyright (c) 1999-2011 The OpenSSL Project. All rights reserved. * @@ -294,6 +294,7 @@ static ERR_STRING_DATA SSL_str_reasons[]= { {ERR_REASON(SSL_R_MISSING_VERIFY_MESSAGE), "missing verify message"}, {ERR_REASON(SSL_R_MULTIPLE_SGC_RESTARTS) , "multiple sgc restarts"}, {ERR_REASON(SSL_R_NON_SSLV2_INITIAL_PACKET), "non sslv2 initial packet"}, + {ERR_REASON(SSL_R_NO_APPLICATION_PROTOCOL), "no application protocol"}, {ERR_REASON(SSL_R_NO_CERTIFICATES_RETURNED), "no certificates returned"}, {ERR_REASON(SSL_R_NO_CERTIFICATE_ASSIGNED), "no certificate assigned"}, {ERR_REASON(SSL_R_NO_CERTIFICATE_RETURNED), "no certificate returned"}, diff --git a/lib/libssl/ssl_tlsext.c b/lib/libssl/ssl_tlsext.c index 4d426f1487a..3ad564964de 100644 --- a/lib/libssl/ssl_tlsext.c +++ b/lib/libssl/ssl_tlsext.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_tlsext.c,v 1.98 2021/09/02 11:10:43 beck Exp $ */ +/* $OpenBSD: ssl_tlsext.c,v 1.99 2021/09/10 09:25:29 tb Exp $ */ /* * Copyright (c) 2016, 2017, 2019 Joel Sing * Copyright (c) 2017 Doug Hogan @@ -85,9 +85,16 @@ tlsext_alpn_server_parse(SSL *s, uint16_t msg_types, CBS *cbs, int *alert) if (s->ctx->internal->alpn_select_cb == NULL) return 1; + /* + * XXX - A few things should be considered here: + * 1. Ensure that the same protocol is selected on session resumption. + * 2. Should the callback be called even if no ALPN extension was sent? + * 3. TLSv1.2 and earlier: ensure that SNI has already been processed. + */ r = s->ctx->internal->alpn_select_cb(s, &selected, &selected_len, CBS_data(&alpn), CBS_len(&alpn), s->ctx->internal->alpn_select_cb_arg); + if (r == SSL_TLSEXT_ERR_OK) { free(S3I(s)->alpn_selected); if ((S3I(s)->alpn_selected = malloc(selected_len)) == NULL) { @@ -97,9 +104,18 @@ tlsext_alpn_server_parse(SSL *s, uint16_t msg_types, CBS *cbs, int *alert) } memcpy(S3I(s)->alpn_selected, selected, selected_len); S3I(s)->alpn_selected_len = selected_len; + + return 1; } - return 1; + /* On SSL_TLSEXT_ERR_NOACK behave as if no callback was present. */ + if (r == SSL_TLSEXT_ERR_NOACK) + return 1; + + *alert = SSL_AD_NO_APPLICATION_PROTOCOL; + SSLerror(s, SSL_R_NO_APPLICATION_PROTOCOL); + + return 0; err: *alert = SSL_AD_DECODE_ERROR; -- 2.20.1