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>
diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c
index 060ef69..3e674e6 100644
--- a/ssl/s3_pkt.c
+++ b/ssl/s3_pkt.c
@@ -411,7 +411,8 @@
/* 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 cefdde3..d01643c 100644
--- a/ssl/test/runner/conn.go
+++ b/ssl/test/runner/conn.go
@@ -1274,6 +1274,20 @@
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 bfd9faf..35fe585 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -301,6 +301,9 @@
// 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 @@
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 @@
},
})
+ 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",
