diff options
author | David Benjamin <davidben@google.com> | 2016-07-28 18:29:18 +0300 |
---|---|---|
committer | CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> | 2016-07-29 18:44:42 +0300 |
commit | 4792110b2bb1a5833e761e49f41b6c1fd83066aa (patch) | |
tree | 50ff12e814a98d2dfcbe010f989966b554e0f1aa | |
parent | 17e1292fe43cf96fbdd633a686d718ef44ceceee (diff) |
Forbid interleaving app data in a HelloRequest.
We already forbid renego/app-data interleave. Forbid it within a
HelloRequest too because that's nonsense. No one would ever send:
[hs:HelloReq-] [app:Hello world] [hs:-uest]
Add tests for this case.
This is in preparation for our more complex TLS 1.3 post-handshake logic
which is going to go through the usual handshake reassembly logic and,
for sanity, will want to enforce this anyway.
BUG=83
Change-Id: I80eb9f3333da3d751f98f25d9469860d1993a97a
Reviewed-on: https://boringssl-review.googlesource.com/9000
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/s3_pkt.c | 3 | ||||
-rw-r--r-- | ssl/test/runner/conn.go | 14 | ||||
-rw-r--r-- | ssl/test/runner/runner.go | 21 |
3 files changed, 37 insertions, 1 deletions
diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c index 060ef691..3e674e6f 100644 --- a/ssl/s3_pkt.c +++ b/ssl/s3_pkt.c @@ -411,7 +411,8 @@ start: /* we now have a packet which can be read and processed */ - if (type == rr->type) { + /* Do not allow interleaving application data and HelloRequest. */ + if (type == rr->type && ssl->s3->hello_request_len == 0) { /* Discard empty records. */ if (rr->length == 0) { goto start; diff --git a/ssl/test/runner/conn.go b/ssl/test/runner/conn.go index cefdde33..d01643c9 100644 --- a/ssl/test/runner/conn.go +++ b/ssl/test/runner/conn.go @@ -1274,6 +1274,20 @@ func (c *Conn) simulatePacketLoss(resendFunc func()) error { return nil } +func (c *Conn) SendHalfHelloRequest() error { + if err := c.Handshake(); err != nil { + return err + } + + c.out.Lock() + defer c.out.Unlock() + + if _, err := c.writeRecord(recordTypeHandshake, []byte{typeHelloRequest, 0}); err != nil { + return err + } + return c.flushHandshake() +} + // Write writes data to the connection. func (c *Conn) Write(b []byte) (int, error) { if err := c.Handshake(); err != nil { diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index bfd9faf3..35fe585e 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -301,6 +301,9 @@ type testCase struct { // renegotiate indicates the number of times the connection should be // renegotiated during the exchange. renegotiate int + // sendHalfHelloRequest, if true, causes the server to send half a + // HelloRequest when the handshake completes. + sendHalfHelloRequest bool // renegotiateCiphers is a list of ciphersuite ids that will be // switched in just before renegotiation. renegotiateCiphers []uint16 @@ -572,6 +575,10 @@ func doExchange(test *testCase, config *Config, conn net.Conn, isResume bool) er tlsConn.SendAlert(alertLevelWarning, alertUnexpectedMessage) } + if test.sendHalfHelloRequest { + tlsConn.SendHalfHelloRequest() + } + if test.renegotiate > 0 { if test.renegotiateCiphers != nil { config.CipherSuites = test.renegotiateCiphers @@ -3383,6 +3390,20 @@ func addStateMachineCoverageTests(config stateMachineTestConfig) { }, }) + tests = append(tests, testCase{ + name: "SendHalfHelloRequest", + config: Config{ + MaxVersion: VersionTLS12, + Bugs: ProtocolBugs{ + PackHelloRequestWithFinished: config.packHandshakeFlight, + }, + }, + sendHalfHelloRequest: true, + flags: []string{"-renegotiate-ignore"}, + shouldFail: true, + expectedError: ":UNEXPECTED_RECORD:", + }) + // NPN on client and server; results in post-handshake message. tests = append(tests, testCase{ name: "NPN-Client", |