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/crypto/err/ssl.errordata b/crypto/err/ssl.errordata
index cd6f87a..44c3904 100644
--- a/crypto/err/ssl.errordata
+++ b/crypto/err/ssl.errordata
@@ -87,7 +87,6 @@
SSL,161,INVALID_TICKET_KEYS_LENGTH
SSL,302,KEY_USAGE_BIT_INCORRECT
SSL,162,LENGTH_MISMATCH
-SSL,307,MISSING_ALPN
SSL,164,MISSING_EXTENSION
SSL,258,MISSING_KEY_SHARE
SSL,165,MISSING_RSA_CERTIFICATE
@@ -99,6 +98,7 @@
SSL,170,NEGOTIATED_BOTH_NPN_AND_ALPN
SSL,285,NEGOTIATED_TB_WITHOUT_EMS_OR_RI
SSL,171,NESTED_GROUP
+SSL,307,NO_APPLICATION_PROTOCOL
SSL,172,NO_CERTIFICATES_RETURNED
SSL,173,NO_CERTIFICATE_ASSIGNED
SSL,174,NO_CERTIFICATE_SET
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 7ff7e72..b932d24 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -2743,18 +2743,34 @@
// SSL_CTX_set_alpn_select_cb sets a callback function on |ctx| that is called
// during ClientHello processing in order to select an ALPN protocol from the
-// client's list of offered protocols. Configuring this callback enables ALPN on
-// a server.
+// client's list of offered protocols.
//
// The callback is passed a wire-format (i.e. a series of non-empty, 8-bit
-// length-prefixed strings) ALPN protocol list in |in|. It should set |*out| and
-// |*out_len| to the selected protocol and return |SSL_TLSEXT_ERR_OK| on
-// success. It does not pass ownership of the buffer. Otherwise, it should
-// return |SSL_TLSEXT_ERR_NOACK|. Other |SSL_TLSEXT_ERR_*| values are
-// unimplemented and will be treated as |SSL_TLSEXT_ERR_NOACK|.
+// length-prefixed strings) ALPN protocol list in |in|. To select a protocol,
+// the callback should set |*out| and |*out_len| to the selected protocol and
+// return |SSL_TLSEXT_ERR_OK| on success. It does not pass ownership of the
+// buffer, so |*out| should point to a static string, a buffer that outlives the
+// callback call, or the corresponding entry in |in|.
+//
+// If the server supports ALPN, but there are no protocols in common, the
+// callback should return |SSL_TLSEXT_ERR_ALERT_FATAL| to abort the connection
+// with a no_application_protocol alert.
+//
+// If the server does not support ALPN, it can return |SSL_TLSEXT_ERR_NOACK| to
+// continue the handshake without negotiating a protocol. This may be useful if
+// multiple server configurations share an |SSL_CTX|, only some of which have
+// ALPN protocols configured.
+//
+// |SSL_TLSEXT_ERR_ALERT_WARNING| is ignored and will be treated as
+// |SSL_TLSEXT_ERR_NOACK|.
+//
+// The callback will only be called if the client supports ALPN. Callers that
+// wish to require ALPN for all clients must check |SSL_get0_alpn_selected|
+// after the handshake. In QUIC connections, this is done automatically.
//
// The cipher suite is selected before negotiating ALPN. The callback may use
-// |SSL_get_pending_cipher| to query the cipher suite.
+// |SSL_get_pending_cipher| to query the cipher suite. This may be used to
+// implement HTTP/2's cipher suite constraints.
OPENSSL_EXPORT void SSL_CTX_set_alpn_select_cb(
SSL_CTX *ctx, int (*cb)(SSL *ssl, const uint8_t **out, uint8_t *out_len,
const uint8_t *in, unsigned in_len, void *arg),
@@ -5286,7 +5302,7 @@
#define SSL_R_CIPHER_MISMATCH_ON_EARLY_DATA 304
#define SSL_R_QUIC_TRANSPORT_PARAMETERS_MISCONFIGURED 305
#define SSL_R_UNEXPECTED_COMPATIBILITY_MODE 306
-#define SSL_R_MISSING_ALPN 307
+#define SSL_R_NO_APPLICATION_PROTOCOL 307
#define SSL_R_NEGOTIATED_ALPS_WITHOUT_ALPN 308
#define SSL_R_ALPS_MISMATCH_ON_EARLY_DATA 309
#define SSL_R_SSLV3_ALERT_CLOSE_NOTIFY 1000
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;
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 07c0aa6..9af820e 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -6756,7 +6756,7 @@
if protocol == quic {
// ALPN is mandatory in QUIC.
shouldDeclineALPNFail = true
- declineALPNError = ":MISSING_ALPN:"
+ declineALPNError = ":NO_APPLICATION_PROTOCOL:"
declineALPNLocalError = "remote error: no application protocol"
}
testCases = append(testCases, testCase{
@@ -6778,6 +6778,21 @@
expectedLocalError: declineALPNLocalError,
})
+ testCases = append(testCases, testCase{
+ protocol: protocol,
+ testType: serverTest,
+ skipQUICALPNConfig: true,
+ name: "ALPNServer-Reject-" + suffix,
+ config: Config{
+ MaxVersion: ver.version,
+ NextProtos: []string{"foo", "bar", "baz"},
+ },
+ flags: []string{"-reject-alpn"},
+ shouldFail: true,
+ expectedError: ":NO_APPLICATION_PROTOCOL:",
+ expectedLocalError: "remote error: no application protocol",
+ })
+
// Test that the server implementation catches itself if the
// callback tries to return an invalid empty ALPN protocol.
testCases = append(testCases, testCase{
@@ -6957,7 +6972,7 @@
},
skipQUICALPNConfig: true,
shouldFail: true,
- expectedError: ":MISSING_ALPN:",
+ expectedError: ":NO_APPLICATION_PROTOCOL:",
})
testCases = append(testCases, testCase{
testType: clientTest,
@@ -6972,7 +6987,7 @@
},
skipQUICALPNConfig: true,
shouldFail: true,
- expectedError: ":MISSING_ALPN:",
+ expectedError: ":NO_APPLICATION_PROTOCOL:",
expectedLocalError: "remote error: no application protocol",
})
testCases = append(testCases, testCase{
@@ -6985,7 +7000,7 @@
},
skipQUICALPNConfig: true,
shouldFail: true,
- expectedError: ":MISSING_ALPN:",
+ expectedError: ":NO_APPLICATION_PROTOCOL:",
expectedLocalError: "remote error: no application protocol",
})
testCases = append(testCases, testCase{
@@ -7002,7 +7017,7 @@
},
skipQUICALPNConfig: true,
shouldFail: true,
- expectedError: ":MISSING_ALPN:",
+ expectedError: ":NO_APPLICATION_PROTOCOL:",
expectedLocalError: "remote error: no application protocol",
})
}
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index c1d215b..59c5e39 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -73,6 +73,7 @@
{"-shim-writes-first", &TestConfig::shim_writes_first},
{"-expect-session-miss", &TestConfig::expect_session_miss},
{"-decline-alpn", &TestConfig::decline_alpn},
+ {"-reject-alpn", &TestConfig::reject_alpn},
{"-select-empty-alpn", &TestConfig::select_empty_alpn},
{"-defer-alps", &TestConfig::defer_alps},
{"-expect-extended-master-secret",
@@ -669,6 +670,9 @@
if (config->decline_alpn) {
return SSL_TLSEXT_ERR_NOACK;
}
+ if (config->reject_alpn) {
+ return SSL_TLSEXT_ERR_ALERT_FATAL;
+ }
if (!config->expect_advertised_alpn.empty() &&
(config->expect_advertised_alpn.size() != inlen ||
@@ -1274,7 +1278,8 @@
NULL);
}
- if (!select_alpn.empty() || decline_alpn || select_empty_alpn) {
+ if (!select_alpn.empty() || decline_alpn || reject_alpn ||
+ select_empty_alpn) {
SSL_CTX_set_alpn_select_cb(ssl_ctx.get(), AlpnSelectCallback, NULL);
}
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index 7d77994..c24c376 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -68,6 +68,7 @@
std::string expect_advertised_alpn;
std::string select_alpn;
bool decline_alpn = false;
+ bool reject_alpn = false;
bool select_empty_alpn = false;
bool defer_alps = false;
std::vector<std::pair<std::string, std::string>> application_settings;
