Enforce presence of ALPN when QUIC is in use.
Update-Note: If an SSL_QUIC_METHOD is set, connections will now fail if
ALPN is not negotiated. This new behavior can be detected by checking
if the value of BORINGSSL_API_VERSION is greater than 10.
Bug: 294
Change-Id: I42fb80aa09268e77cec4a51e49cdad79bd72fa58
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/42304
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
diff --git a/crypto/err/ssl.errordata b/crypto/err/ssl.errordata
index 44f2ef2..3759c69 100644
--- a/crypto/err/ssl.errordata
+++ b/crypto/err/ssl.errordata
@@ -86,6 +86,7 @@
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
@@ -185,6 +186,7 @@
SSL,1086,TLSV1_ALERT_INAPPROPRIATE_FALLBACK
SSL,1071,TLSV1_ALERT_INSUFFICIENT_SECURITY
SSL,1080,TLSV1_ALERT_INTERNAL_ERROR
+SSL,1120,TLSV1_ALERT_NO_APPLICATION_PROTOCOL
SSL,1100,TLSV1_ALERT_NO_RENEGOTIATION
SSL,1070,TLSV1_ALERT_PROTOCOL_VERSION
SSL,1022,TLSV1_ALERT_RECORD_OVERFLOW
diff --git a/include/openssl/base.h b/include/openssl/base.h
index 9235ed7..69f51e8 100644
--- a/include/openssl/base.h
+++ b/include/openssl/base.h
@@ -184,7 +184,7 @@
// A consumer may use this symbol in the preprocessor to temporarily build
// against multiple revisions of BoringSSL at the same time. It is not
// recommended to do so for longer than is necessary.
-#define BORINGSSL_API_VERSION 10
+#define BORINGSSL_API_VERSION 11
#if defined(BORINGSSL_SHARED_LIBRARY)
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index f15c006..07a39d2 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -5197,6 +5197,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_SSLV3_ALERT_CLOSE_NOTIFY 1000
#define SSL_R_SSLV3_ALERT_UNEXPECTED_MESSAGE 1010
#define SSL_R_SSLV3_ALERT_BAD_RECORD_MAC 1020
@@ -5229,5 +5230,6 @@
#define SSL_R_TLSV1_ALERT_BAD_CERTIFICATE_HASH_VALUE 1114
#define SSL_R_TLSV1_ALERT_UNKNOWN_PSK_IDENTITY 1115
#define SSL_R_TLSV1_ALERT_CERTIFICATE_REQUIRED 1116
+#define SSL_R_TLSV1_ALERT_NO_APPLICATION_PROTOCOL 1120
#endif // OPENSSL_HEADER_SSL_H
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 3c2d852..eb45700 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -5070,6 +5070,22 @@
SSL_CTX_set_max_proto_version(server_ctx_.get(), TLS1_3_VERSION);
SSL_CTX_set_min_proto_version(client_ctx_.get(), TLS1_3_VERSION);
SSL_CTX_set_max_proto_version(client_ctx_.get(), TLS1_3_VERSION);
+
+ static const uint8_t kALPNProtos[] = {0x03, 'f', 'o', 'o'};
+ ASSERT_EQ(SSL_CTX_set_alpn_protos(client_ctx_.get(), kALPNProtos,
+ sizeof(kALPNProtos)),
+ 0);
+ SSL_CTX_set_alpn_select_cb(
+ server_ctx_.get(),
+ [](SSL *ssl, const uint8_t **out, uint8_t *out_len, const uint8_t *in,
+ unsigned in_len, void *arg) -> int {
+ return SSL_select_next_proto(
+ const_cast<uint8_t **>(out), out_len, in, in_len,
+ kALPNProtos, sizeof(kALPNProtos)) == OPENSSL_NPN_NEGOTIATED
+ ? SSL_TLSEXT_ERR_OK
+ : SSL_TLSEXT_ERR_NOACK;
+ },
+ nullptr);
}
static MockQUICTransport *TransportFromSSL(const SSL *ssl) {
diff --git a/ssl/t1_lib.cc b/ssl/t1_lib.cc
index 5032af5..f274b11 100644
--- a/ssl/t1_lib.cc
+++ b/ssl/t1_lib.cc
@@ -1245,6 +1245,12 @@
static bool ext_alpn_add_clienthello(SSL_HANDSHAKE *hs, CBB *out) {
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);
+ return false;
+ }
+
if (hs->config->alpn_client_proto_list.empty() ||
ssl->s3->initial_handshake_complete) {
return true;
@@ -1267,6 +1273,12 @@
CBS *contents) {
SSL *const ssl = hs->ssl;
if (contents == NULL) {
+ 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;
}
@@ -1342,6 +1354,12 @@
!ssl_client_hello_get_extension(
client_hello, &contents,
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);
+ *out_alert = SSL_AD_NO_APPLICATION_PROTOCOL;
+ return false;
+ }
// Ignore ALPN if not configured or no extension was supplied.
return true;
}
@@ -1388,6 +1406,11 @@
*out_alert = SSL_AD_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/alert.go b/ssl/test/runner/alert.go
index c1f7f27..a6f61e9 100644
--- a/ssl/test/runner/alert.go
+++ b/ssl/test/runner/alert.go
@@ -45,6 +45,7 @@
alertBadCertificateStatusResponse alert = 113
alertUnknownPSKIdentity alert = 115
alertCertificateRequired alert = 116
+ alertNoApplicationProtocol alert = 120
)
var alertText = map[alert]string{
@@ -78,6 +79,7 @@
alertUnrecognizedName: "unrecognized name",
alertUnknownPSKIdentity: "unknown PSK identity",
alertCertificateRequired: "certificate required",
+ alertNoApplicationProtocol: "no application protocol",
}
func (e alert) String() string {
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index a340e1e..5527f96 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -656,6 +656,9 @@
// skipTransportParamsConfig, if true, will skip automatic configuration of
// sending QUIC transport parameters when protocol == quic.
skipTransportParamsConfig bool
+ // skipQUICALPNConfig, if true, will skip automatic configuration of
+ // sending a fake ALPN when protocol == quic.
+ skipQUICALPNConfig bool
}
var testCases []testCase
@@ -1280,6 +1283,20 @@
base64.StdEncoding.EncodeToString([]byte{1, 2}),
}...)
}
+ if !test.skipQUICALPNConfig {
+ flags = append(flags,
+ []string{
+ "-advertise-alpn", "\x03foo",
+ "-select-alpn", "foo",
+ "-expect-alpn", "foo",
+ }...)
+ test.config.NextProtos = []string{"foo"}
+ if test.resumeConfig != nil {
+ test.resumeConfig.NextProtos = []string{"foo"}
+ }
+ test.expectedNextProto = "foo"
+ test.expectedNextProtoType = alpn
+ }
}
var resumeCount int
@@ -6816,6 +6833,68 @@
})
}
+ // Test missing ALPN in QUIC
+ if ver.version >= VersionTLS13 {
+ testCases = append(testCases, testCase{
+ testType: clientTest,
+ protocol: quic,
+ name: "QUIC-Client-ALPNMissingFromConfig-" + ver.name,
+ config: Config{
+ MinVersion: ver.version,
+ MaxVersion: ver.version,
+ },
+ skipQUICALPNConfig: true,
+ shouldFail: true,
+ expectedError: ":MISSING_ALPN:",
+ })
+ testCases = append(testCases, testCase{
+ testType: clientTest,
+ protocol: quic,
+ name: "QUIC-Client-ALPNMissing-" + ver.name,
+ config: Config{
+ MinVersion: ver.version,
+ MaxVersion: ver.version,
+ },
+ flags: []string{
+ "-advertise-alpn", "\x03foo",
+ },
+ skipQUICALPNConfig: true,
+ shouldFail: true,
+ expectedError: ":MISSING_ALPN:",
+ expectedLocalError: "remote error: no application protocol",
+ })
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ protocol: quic,
+ name: "QUIC-Server-ALPNMissing-" + ver.name,
+ config: Config{
+ MinVersion: ver.version,
+ MaxVersion: ver.version,
+ },
+ skipQUICALPNConfig: true,
+ shouldFail: true,
+ expectedError: ":MISSING_ALPN:",
+ expectedLocalError: "remote error: no application protocol",
+ })
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ protocol: quic,
+ name: "QUIC-Server-ALPNMismatch-" + ver.name,
+ config: Config{
+ MinVersion: ver.version,
+ MaxVersion: ver.version,
+ NextProtos: []string{"foo"},
+ },
+ flags: []string{
+ "-decline-alpn",
+ },
+ skipQUICALPNConfig: true,
+ shouldFail: true,
+ expectedError: ":MISSING_ALPN:",
+ expectedLocalError: "remote error: no application protocol",
+ })
+ }
+
// Test Token Binding.
const maxTokenBindingVersion = 16