diff options
author | David Benjamin <davidben@google.com> | 2016-08-01 19:05:50 +0300 |
---|---|---|
committer | CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> | 2016-08-01 21:56:31 +0300 |
commit | 0c40a96455b0f720267b9eeb47704c85ee883121 (patch) | |
tree | 54af115c9af97b33e7561e48826e418219fe67d8 | |
parent | 636ff1cb7e71f8914a3b6d79d83b348f0e866a16 (diff) |
Send unsupported_extension on unexpected ServerHello extensions.
We were sending decode_error, but the spec explicitly says (RFC 5246):
unsupported_extension
sent by clients that receive an extended server hello containing
an extension that they did not put in the corresponding client
hello. This message is always fatal.
Also add a test for this when it's a known but unoffered extension. We
actually end up putting these in different codepaths now due to the
custom extensions stuff.
Thanks to Eric Rescorla for pointing this out.
Change-Id: If6c8033d4cfe69ef8af5678b873b25e0dbadfc4f
Reviewed-on: https://boringssl-review.googlesource.com/9061
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
-rw-r--r-- | ssl/custom_extensions.c | 2 | ||||
-rw-r--r-- | ssl/t1_lib.c | 2 | ||||
-rw-r--r-- | ssl/test/runner/common.go | 5 | ||||
-rw-r--r-- | ssl/test/runner/handshake_server.go | 4 | ||||
-rw-r--r-- | ssl/test/runner/runner.go | 38 |
5 files changed, 45 insertions, 6 deletions
diff --git a/ssl/custom_extensions.c b/ssl/custom_extensions.c index c94543d8..4a9baa88 100644 --- a/ssl/custom_extensions.c +++ b/ssl/custom_extensions.c @@ -139,7 +139,7 @@ int custom_ext_parse_serverhello(SSL *ssl, int *out_alert, uint16_t value, !(ssl->s3->tmp.custom_extensions.sent & (1u << index))) { OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION); ERR_add_error_dataf("extension: %u", (unsigned)value); - *out_alert = SSL_AD_DECODE_ERROR; + *out_alert = SSL_AD_UNSUPPORTED_EXTENSION; return 0; } diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 6b0c16ab..e35a38c8 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -2642,7 +2642,7 @@ static int ssl_scan_serverhello_tlsext(SSL *ssl, CBS *cbs, int *out_alert) { /* If the extension was never sent then it is illegal. */ OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION); ERR_add_error_dataf("extension :%u", (unsigned)type); - *out_alert = SSL_AD_DECODE_ERROR; + *out_alert = SSL_AD_UNSUPPORTED_EXTENSION; return 0; } diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go index dfd5b30d..e9a3fdb5 100644 --- a/ssl/test/runner/common.go +++ b/ssl/test/runner/common.go @@ -866,6 +866,11 @@ type ProtocolBugs struct { // ALPN and NPN in the same connetion. 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. + SendALPN string + // SendEmptySessionTicket, if true, causes the server to send an empty // session ticket. SendEmptySessionTicket bool diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go index ff8005bd..4d8b5c16 100644 --- a/ssl/test/runner/handshake_server.go +++ b/ssl/test/runner/handshake_server.go @@ -851,6 +851,10 @@ func (hs *serverHandshakeState) processClientExtensions(serverExtensions *server } } + if len(c.config.Bugs.SendALPN) > 0 { + serverExtensions.alpnProtocol = c.config.Bugs.SendALPN + } + if c.vers < VersionTLS13 || config.Bugs.NegotiateNPNAtAllVersions { if len(hs.clientHello.alpnProtocols) == 0 || c.config.Bugs.NegotiateALPNAndNPN { // Although sending an empty NPN extension is reasonable, Firefox has diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index b45e42ae..5e049b11 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -6440,8 +6440,9 @@ func addCustomExtensionTests() { CustomExtension: expectedContents, }, }, - shouldFail: true, - expectedError: ":UNEXPECTED_EXTENSION:", + shouldFail: true, + expectedError: ":UNEXPECTED_EXTENSION:", + expectedLocalError: "remote error: unsupported extension", }) testCases = append(testCases, testCase{ testType: clientTest, @@ -6452,8 +6453,37 @@ func addCustomExtensionTests() { CustomExtension: expectedContents, }, }, - shouldFail: true, - expectedError: ":UNEXPECTED_EXTENSION:", + shouldFail: true, + expectedError: ":UNEXPECTED_EXTENSION:", + expectedLocalError: "remote error: unsupported extension", + }) + + // Test a known but unoffered extension from the server. + testCases = append(testCases, testCase{ + testType: clientTest, + name: "UnofferedExtension-Client", + config: Config{ + MaxVersion: VersionTLS12, + Bugs: ProtocolBugs{ + SendALPN: "alpn", + }, + }, + shouldFail: true, + expectedError: ":UNEXPECTED_EXTENSION:", + expectedLocalError: "remote error: unsupported extension", + }) + testCases = append(testCases, testCase{ + testType: clientTest, + name: "UnofferedExtension-Client-TLS13", + config: Config{ + MaxVersion: VersionTLS13, + Bugs: ProtocolBugs{ + SendALPN: "alpn", + }, + }, + shouldFail: true, + expectedError: ":UNEXPECTED_EXTENSION:", + expectedLocalError: "remote error: unsupported extension", }) } |