Honor SSL_TLSEXT_ERR_ALERT_FATAL in the ALPN callback.
This aligns with OpenSSL's behavior. RFC7301 says servers should return
no_application_protocol if the client supported ALPN but no common
protocol was found. We currently interpret all values as
SSL_TLSEXT_ERR_NOACK. Instead, implement both modes and give guidance on
whne to use each. (NOACK is still useful because the callback may be
shared across multiple configurations, some of which don't support ALPN
at all. Those would want to return NOACK to ignore the list.)
To match upstream, I've also switched SSL_R_MISSING_ALPN, added for
QUIC, to SSL_R_NO_APPLICATION_PROTOCOL.
Update-Note: Callers that return SSL_TLSEXT_ERR_ALERT_FATAL from the
ALPN callback will change behavior. The old behavior may be restored by
returning SSL_TLSEXT_ERR_NOACK, though see the documentation for new
recommendations on return values.
Change-Id: Ib7917b5f8a098571bed764c79aa7a4ce0f728297
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45504
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/ssl/t1_lib.cc b/ssl/t1_lib.cc
index 342c170..484eee9 100644
--- a/ssl/t1_lib.cc
+++ b/ssl/t1_lib.cc
@@ -1428,7 +1428,7 @@
SSL *const ssl = hs->ssl;
if (hs->config->alpn_client_proto_list.empty() && ssl->quic_method) {
// ALPN MUST be used with QUIC.
- OPENSSL_PUT_ERROR(SSL, SSL_R_MISSING_ALPN);
+ OPENSSL_PUT_ERROR(SSL, SSL_R_NO_APPLICATION_PROTOCOL);
return false;
}
@@ -1456,7 +1456,7 @@
if (contents == NULL) {
if (ssl->quic_method) {
// ALPN is required when QUIC is used.
- OPENSSL_PUT_ERROR(SSL, SSL_R_MISSING_ALPN);
+ OPENSSL_PUT_ERROR(SSL, SSL_R_NO_APPLICATION_PROTOCOL);
*out_alert = SSL_AD_NO_APPLICATION_PROTOCOL;
return false;
}
@@ -1537,7 +1537,7 @@
TLSEXT_TYPE_application_layer_protocol_negotiation)) {
if (ssl->quic_method) {
// ALPN is required when QUIC is used.
- OPENSSL_PUT_ERROR(SSL, SSL_R_MISSING_ALPN);
+ OPENSSL_PUT_ERROR(SSL, SSL_R_NO_APPLICATION_PROTOCOL);
*out_alert = SSL_AD_NO_APPLICATION_PROTOCOL;
return false;
}
@@ -1572,25 +1572,39 @@
const uint8_t *selected;
uint8_t selected_len;
- if (ssl->ctx->alpn_select_cb(
- ssl, &selected, &selected_len, CBS_data(&protocol_name_list),
- CBS_len(&protocol_name_list),
- ssl->ctx->alpn_select_cb_arg) == SSL_TLSEXT_ERR_OK) {
- if (selected_len == 0) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_ALPN_PROTOCOL);
- *out_alert = SSL_AD_INTERNAL_ERROR;
+ int ret = ssl->ctx->alpn_select_cb(
+ ssl, &selected, &selected_len, CBS_data(&protocol_name_list),
+ CBS_len(&protocol_name_list), ssl->ctx->alpn_select_cb_arg);
+ // ALPN is required when QUIC is used.
+ if (ssl->quic_method &&
+ (ret == SSL_TLSEXT_ERR_NOACK || ret == SSL_TLSEXT_ERR_ALERT_WARNING)) {
+ ret = SSL_TLSEXT_ERR_ALERT_FATAL;
+ }
+ switch (ret) {
+ case SSL_TLSEXT_ERR_OK:
+ if (selected_len == 0) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_ALPN_PROTOCOL);
+ *out_alert = SSL_AD_INTERNAL_ERROR;
+ return false;
+ }
+ if (!ssl->s3->alpn_selected.CopyFrom(
+ MakeConstSpan(selected, selected_len))) {
+ *out_alert = SSL_AD_INTERNAL_ERROR;
+ return false;
+ }
+ break;
+ case SSL_TLSEXT_ERR_NOACK:
+ case SSL_TLSEXT_ERR_ALERT_WARNING:
+ break;
+ case SSL_TLSEXT_ERR_ALERT_FATAL:
+ *out_alert = SSL_AD_NO_APPLICATION_PROTOCOL;
+ OPENSSL_PUT_ERROR(SSL, SSL_R_NO_APPLICATION_PROTOCOL);
return false;
- }
- if (!ssl->s3->alpn_selected.CopyFrom(
- MakeConstSpan(selected, selected_len))) {
+ default:
+ // Invalid return value.
*out_alert = SSL_AD_INTERNAL_ERROR;
+ OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
return false;
- }
- } else if (ssl->quic_method) {
- // ALPN is required when QUIC is used.
- OPENSSL_PUT_ERROR(SSL, SSL_R_MISSING_ALPN);
- *out_alert = SSL_AD_NO_APPLICATION_PROTOCOL;
- return false;
}
return true;
