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>kris/onging/CECPQ3_patch15
@@ -59,6 +59,7 @@ SSL,154,HANDSHAKE_FAILURE_ON_CLIENT_HELLO | |||||
SSL,155,HTTPS_PROXY_REQUEST | SSL,155,HTTPS_PROXY_REQUEST | ||||
SSL,156,HTTP_REQUEST | SSL,156,HTTP_REQUEST | ||||
SSL,157,INAPPROPRIATE_FALLBACK | SSL,157,INAPPROPRIATE_FALLBACK | ||||
SSL,259,INVALID_ALPN_PROTOCOL | |||||
SSL,158,INVALID_COMMAND | SSL,158,INVALID_COMMAND | ||||
SSL,256,INVALID_COMPRESSION_LIST | SSL,256,INVALID_COMPRESSION_LIST | ||||
SSL,159,INVALID_MESSAGE | SSL,159,INVALID_MESSAGE | ||||
@@ -4803,6 +4803,7 @@ OPENSSL_EXPORT int SSL_set_ssl_method(SSL *s, const SSL_METHOD *method); | |||||
#define SSL_R_INVALID_COMPRESSION_LIST 256 | #define SSL_R_INVALID_COMPRESSION_LIST 256 | ||||
#define SSL_R_DUPLICATE_EXTENSION 257 | #define SSL_R_DUPLICATE_EXTENSION 257 | ||||
#define SSL_R_MISSING_KEY_SHARE 258 | #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_CLOSE_NOTIFY 1000 | ||||
#define SSL_R_SSLV3_ALERT_UNEXPECTED_MESSAGE 1010 | #define SSL_R_SSLV3_ALERT_UNEXPECTED_MESSAGE 1010 | ||||
#define SSL_R_SSLV3_ALERT_BAD_RECORD_MAC 1020 | #define SSL_R_SSLV3_ALERT_BAD_RECORD_MAC 1020 | ||||
@@ -1517,6 +1517,32 @@ static int ext_alpn_parse_serverhello(SSL *ssl, uint8_t *out_alert, | |||||
return 0; | 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, | if (!CBS_stow(&protocol_name, &ssl->s3->alpn_selected, | ||||
&ssl->s3->alpn_selected_len)) { | &ssl->s3->alpn_selected_len)) { | ||||
*out_alert = SSL_AD_INTERNAL_ERROR; | *out_alert = SSL_AD_INTERNAL_ERROR; | ||||
@@ -875,8 +875,8 @@ type ProtocolBugs struct { | |||||
NegotiateALPNAndNPN bool | NegotiateALPNAndNPN bool | ||||
// SendALPN, if non-empty, causes the server to send the specified | // 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 | SendALPN string | ||||
// SendEmptySessionTicket, if true, causes the server to send an empty | // SendEmptySessionTicket, if true, causes the server to send an empty | ||||
@@ -4316,6 +4316,22 @@ func addExtensionTests() { | |||||
expectedNextProtoType: alpn, | expectedNextProtoType: alpn, | ||||
resumeSession: resumeSession, | 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{ | testCases = append(testCases, testCase{ | ||||
testType: serverTest, | testType: serverTest, | ||||
name: "ALPNServer-" + ver.name, | name: "ALPNServer-" + ver.name, | ||||