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>

crypto/err/ssl.errordata

@@ -86,6 +86,7 @@ SSL,160,INVALID_SSL_SESSION
 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,1060,TLSV1_ALERT_EXPORT_RESTRICTION
 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

include/openssl/base.h

@@ -184,7 +184,7 @@ extern "C" {
 // 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)
 

include/openssl/ssl.h

@@ -5197,6 +5197,7 @@ BSSL_NAMESPACE_END
 #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 @@ BSSL_NAMESPACE_END
 #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

ssl/ssl_test.cc

@@ -5070,6 +5070,22 @@ class QUICMethodTest : public testing::Test {
     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) {

ssl/t1_lib.cc

@@ -1245,6 +1245,12 @@ static bool ext_sct_add_serverhello(SSL_HANDSHAKE *hs, CBB *out) {
 
 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 @@ static bool ext_alpn_parse_serverhello(SSL_HANDSHAKE *hs, uint8_t *out_alert,
                                        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 @@ bool ssl_negotiate_alpn(SSL_HANDSHAKE *hs, uint8_t *out_alert,
       !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 @@ bool ssl_negotiate_alpn(SSL_HANDSHAKE *hs, uint8_t *out_alert,
       *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;

ssl/test/runner/alert.go

@@ -45,6 +45,7 @@ const (
 	alertBadCertificateStatusResponse alert = 113
 	alertUnknownPSKIdentity           alert = 115
 	alertCertificateRequired          alert = 116
+	alertNoApplicationProtocol        alert = 120
 )
 
 var alertText = map[alert]string{
@@ -78,6 +79,7 @@ var alertText = map[alert]string{
 	alertUnrecognizedName:             "unrecognized name",
 	alertUnknownPSKIdentity:           "unknown PSK identity",
 	alertCertificateRequired:          "certificate required",
+	alertNoApplicationProtocol:        "no application protocol",
 }
 
 func (e alert) String() string {

ssl/test/runner/runner.go

@@ -656,6 +656,9 @@ type testCase struct {
 	// 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 @@ func runTest(statusChan chan statusMsg, test *testCase, shimPath string, mallocN
 					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 @@ func addExtensionTests() {
 			})
 		}
 
+		// 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