Enforce the server ALPN protocol was advertised.
The server should not be allowed select a protocol that wasn't
advertised. Callers tend to not really notice and act as if some default
were chosen which is unlikely to work very well.
Change-Id: Ib6388db72f05386f854d275bab762ca79e8174e6
Reviewed-on: https://boringssl-review.googlesource.com/10284
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/crypto/err/ssl.errordata b/crypto/err/ssl.errordata
index 5800d41..7753f17 100644
--- a/crypto/err/ssl.errordata
+++ b/crypto/err/ssl.errordata
@@ -59,6 +59,7 @@
SSL,155,HTTPS_PROXY_REQUEST
SSL,156,HTTP_REQUEST
SSL,157,INAPPROPRIATE_FALLBACK
+SSL,259,INVALID_ALPN_PROTOCOL
SSL,158,INVALID_COMMAND
SSL,256,INVALID_COMPRESSION_LIST
SSL,159,INVALID_MESSAGE
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 931b100..fe6bac2 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -4803,6 +4803,7 @@
#define SSL_R_INVALID_COMPRESSION_LIST 256
#define SSL_R_DUPLICATE_EXTENSION 257
#define SSL_R_MISSING_KEY_SHARE 258
+#define SSL_R_INVALID_ALPN_PROTOCOL 259
#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
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 62f3824..5e790a4 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1517,6 +1517,32 @@
return 0;
}
+ /* Check that the protcol name is one of the ones we advertised. */
+ int protocol_ok = 0;
+ CBS client_protocol_name_list, client_protocol_name;
+ CBS_init(&client_protocol_name_list, ssl->alpn_client_proto_list,
+ ssl->alpn_client_proto_list_len);
+ while (CBS_len(&client_protocol_name_list) > 0) {
+ if (!CBS_get_u8_length_prefixed(&client_protocol_name_list,
+ &client_protocol_name)) {
+ *out_alert = SSL_AD_INTERNAL_ERROR;
+ return 0;
+ }
+
+ if (CBS_len(&client_protocol_name) == CBS_len(&protocol_name) &&
+ memcmp(CBS_data(&client_protocol_name), CBS_data(&protocol_name),
+ CBS_len(&protocol_name)) == 0) {
+ protocol_ok = 1;
+ break;
+ }
+ }
+
+ if (!protocol_ok) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_ALPN_PROTOCOL);
+ *out_alert = SSL_AD_ILLEGAL_PARAMETER;
+ return 0;
+ }
+
if (!CBS_stow(&protocol_name, &ssl->s3->alpn_selected,
&ssl->s3->alpn_selected_len)) {
*out_alert = SSL_AD_INTERNAL_ERROR;
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index b6befe4..f3d57c7 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -875,8 +875,8 @@
NegotiateALPNAndNPN bool
// SendALPN, if non-empty, causes the server to send the specified
- // string in the ALPN extension regardless of whether the client
- // advertised it.
+ // string in the ALPN extension regardless of the content or presence of
+ // the client offer.
SendALPN string
// SendEmptySessionTicket, if true, causes the server to send an empty
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 4cb22b1..45d3e13 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -4317,6 +4317,22 @@
resumeSession: resumeSession,
})
testCases = append(testCases, testCase{
+ testType: clientTest,
+ name: "ALPNClient-Mismatch-" + ver.name,
+ config: Config{
+ MaxVersion: ver.version,
+ Bugs: ProtocolBugs{
+ SendALPN: "baz",
+ },
+ },
+ flags: []string{
+ "-advertise-alpn", "\x03foo\x03bar",
+ },
+ shouldFail: true,
+ expectedError: ":INVALID_ALPN_PROTOCOL:",
+ expectedLocalError: "remote error: illegal parameter",
+ })
+ testCases = append(testCases, testCase{
testType: serverTest,
name: "ALPNServer-" + ver.name,
config: Config{
