Add much more aggressive WrongMessageType tests. Not only test that we can enforce the message type correctly (this is currently in protocol-specific code though really should not be), but also test that each individual message is checked correctly. Change-Id: I5ed0f4033f011186f020ea46940160c7639f688b Reviewed-on: https://boringssl-review.googlesource.com/8793 Reviewed-by: Steven Valdez <svaldez@google.com> 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/test/runner/common.go b/ssl/test/runner/common.go index 8290444..5fd3436 100644 --- a/ssl/test/runner/common.go +++ b/ssl/test/runner/common.go
@@ -736,9 +736,9 @@ // content type to be sent immediately following the handshake. SendInvalidRecordType bool - // WrongCertificateMessageType, if true, causes Certificate message to - // be sent with the wrong message type. - WrongCertificateMessageType bool + // SendWrongMessageType, if non-zero, causes messages of the specified + // type to be sent with the wrong value. + SendWrongMessageType byte // FragmentMessageTypeMismatch, if true, causes all non-initial // handshake fragments in DTLS to have the wrong message type.
diff --git a/ssl/test/runner/conn.go b/ssl/test/runner/conn.go index f10a495..c9bf2b1 100644 --- a/ssl/test/runner/conn.go +++ b/ssl/test/runner/conn.go
@@ -941,6 +941,15 @@ // to the connection and updates the record layer state. // c.out.Mutex <= L. func (c *Conn) writeRecord(typ recordType, data []byte) (n int, err error) { + if wrongType := c.config.Bugs.SendWrongMessageType; wrongType != 0 { + if typ == recordTypeHandshake && data[0] == wrongType { + newData := make([]byte, len(data)) + copy(newData, data) + newData[0] += 42 + data = newData + } + } + if c.isDTLS { return c.dtlsWriteRecord(typ, data) }
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go index 7b93313..a2ea50e 100644 --- a/ssl/test/runner/handshake_client.go +++ b/ssl/test/runner/handshake_client.go
@@ -1161,7 +1161,7 @@ func (hs *clientHandshakeState) sendFinished(out []byte, isResume bool) error { c := hs.c - var postCCSBytes []byte + var postCCSMsgs [][]byte seqno := hs.c.sendHandshakeSeq if hs.serverHello.extensions.nextProtoNeg { nextProto := new(nextProtoMsg) @@ -1173,7 +1173,7 @@ nextProtoBytes := nextProto.marshal() hs.writeHash(nextProtoBytes, seqno) seqno++ - postCCSBytes = append(postCCSBytes, nextProtoBytes...) + postCCSMsgs = append(postCCSMsgs, nextProtoBytes) } if hs.serverHello.extensions.channelIDRequested { @@ -1201,7 +1201,7 @@ channelIDMsgBytes := channelIDMsg.marshal() hs.writeHash(channelIDMsgBytes, seqno) seqno++ - postCCSBytes = append(postCCSBytes, channelIDMsgBytes...) + postCCSMsgs = append(postCCSMsgs, channelIDMsgBytes) } finished := new(finishedMsg) @@ -1217,14 +1217,14 @@ c.clientVerify = append(c.clientVerify[:0], finished.verifyData...) hs.finishedBytes = finished.marshal() hs.writeHash(hs.finishedBytes, seqno) - postCCSBytes = append(postCCSBytes, hs.finishedBytes...) + postCCSMsgs = append(postCCSMsgs, hs.finishedBytes) if c.config.Bugs.FragmentAcrossChangeCipherSpec { - c.writeRecord(recordTypeHandshake, postCCSBytes[:5]) - postCCSBytes = postCCSBytes[5:] + c.writeRecord(recordTypeHandshake, postCCSMsgs[0][:5]) + postCCSMsgs[0] = postCCSMsgs[0][5:] } else if c.config.Bugs.SendUnencryptedFinished { - c.writeRecord(recordTypeHandshake, postCCSBytes) - postCCSBytes = nil + c.writeRecord(recordTypeHandshake, postCCSMsgs[0]) + postCCSMsgs = postCCSMsgs[1:] } c.flushHandshake() @@ -1245,8 +1245,10 @@ return errors.New("tls: simulating post-CCS alert") } - if !c.config.Bugs.SkipFinished && len(postCCSBytes) > 0 { - c.writeRecord(recordTypeHandshake, postCCSBytes) + if !c.config.Bugs.SkipFinished { + for _, msg := range postCCSMsgs { + c.writeRecord(recordTypeHandshake, msg) + } c.flushHandshake() } return nil
diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go index 554eae2..d4dba75 100644 --- a/ssl/test/runner/handshake_server.go +++ b/ssl/test/runner/handshake_server.go
@@ -459,9 +459,6 @@ certMsg.certificates = hs.cert.Certificate } certMsgBytes := certMsg.marshal() - if config.Bugs.WrongCertificateMessageType { - certMsgBytes[0] += 42 - } hs.writeServerHash(certMsgBytes) c.writeRecord(recordTypeHandshake, certMsgBytes) @@ -938,9 +935,6 @@ } if !config.Bugs.UnauthenticatedECDH { certMsgBytes := certMsg.marshal() - if config.Bugs.WrongCertificateMessageType { - certMsgBytes[0] += 42 - } hs.writeServerHash(certMsgBytes) c.writeRecord(recordTypeHandshake, certMsgBytes) }
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index a222021..a0edf77 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go
@@ -1626,41 +1626,6 @@ expectedLocalError: "remote error: access denied", }, { - name: "WrongMessageType", - config: Config{ - MaxVersion: VersionTLS12, - Bugs: ProtocolBugs{ - WrongCertificateMessageType: true, - }, - }, - shouldFail: true, - expectedError: ":UNEXPECTED_MESSAGE:", - expectedLocalError: "remote error: unexpected message", - }, - { - name: "WrongMessageType-TLS13", - config: Config{ - Bugs: ProtocolBugs{ - WrongCertificateMessageType: true, - }, - }, - shouldFail: true, - expectedError: ":UNEXPECTED_MESSAGE:", - expectedLocalError: "remote error: unexpected message", - }, - { - protocol: dtls, - name: "WrongMessageType-DTLS", - config: Config{ - Bugs: ProtocolBugs{ - WrongCertificateMessageType: true, - }, - }, - shouldFail: true, - expectedError: ":UNEXPECTED_MESSAGE:", - expectedLocalError: "remote error: unexpected message", - }, - { protocol: dtls, name: "FragmentMessageTypeMismatch-DTLS", config: Config{ @@ -6365,6 +6330,265 @@ }) } +func addWrongMessageTypeTests() { + for _, protocol := range []protocol{tls, dtls} { + var suffix string + if protocol == dtls { + suffix = "-DTLS" + } + + testCases = append(testCases, testCase{ + protocol: protocol, + testType: serverTest, + name: "WrongMessageType-ClientHello" + suffix, + config: Config{ + MaxVersion: VersionTLS12, + Bugs: ProtocolBugs{ + SendWrongMessageType: typeClientHello, + }, + }, + shouldFail: true, + expectedError: ":UNEXPECTED_MESSAGE:", + expectedLocalError: "remote error: unexpected message", + }) + + if protocol == dtls { + testCases = append(testCases, testCase{ + protocol: protocol, + name: "WrongMessageType-HelloVerifyRequest" + suffix, + config: Config{ + MaxVersion: VersionTLS12, + Bugs: ProtocolBugs{ + SendWrongMessageType: typeHelloVerifyRequest, + }, + }, + shouldFail: true, + expectedError: ":UNEXPECTED_MESSAGE:", + expectedLocalError: "remote error: unexpected message", + }) + } + + testCases = append(testCases, testCase{ + protocol: protocol, + name: "WrongMessageType-ServerHello" + suffix, + config: Config{ + MaxVersion: VersionTLS12, + Bugs: ProtocolBugs{ + SendWrongMessageType: typeServerHello, + }, + }, + shouldFail: true, + expectedError: ":UNEXPECTED_MESSAGE:", + expectedLocalError: "remote error: unexpected message", + }) + + testCases = append(testCases, testCase{ + protocol: protocol, + name: "WrongMessageType-ServerCertificate" + suffix, + config: Config{ + MaxVersion: VersionTLS12, + Bugs: ProtocolBugs{ + SendWrongMessageType: typeCertificate, + }, + }, + shouldFail: true, + expectedError: ":UNEXPECTED_MESSAGE:", + expectedLocalError: "remote error: unexpected message", + }) + + testCases = append(testCases, testCase{ + protocol: protocol, + name: "WrongMessageType-CertificateStatus" + suffix, + config: Config{ + MaxVersion: VersionTLS12, + Bugs: ProtocolBugs{ + SendWrongMessageType: typeCertificateStatus, + }, + }, + flags: []string{"-enable-ocsp-stapling"}, + shouldFail: true, + expectedError: ":UNEXPECTED_MESSAGE:", + expectedLocalError: "remote error: unexpected message", + }) + + testCases = append(testCases, testCase{ + protocol: protocol, + name: "WrongMessageType-ServerKeyExchange" + suffix, + config: Config{ + MaxVersion: VersionTLS12, + CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}, + Bugs: ProtocolBugs{ + SendWrongMessageType: typeServerKeyExchange, + }, + }, + shouldFail: true, + expectedError: ":UNEXPECTED_MESSAGE:", + expectedLocalError: "remote error: unexpected message", + }) + + testCases = append(testCases, testCase{ + protocol: protocol, + name: "WrongMessageType-CertificateRequest" + suffix, + config: Config{ + MaxVersion: VersionTLS12, + ClientAuth: RequireAnyClientCert, + Bugs: ProtocolBugs{ + SendWrongMessageType: typeCertificateRequest, + }, + }, + shouldFail: true, + expectedError: ":UNEXPECTED_MESSAGE:", + expectedLocalError: "remote error: unexpected message", + }) + + testCases = append(testCases, testCase{ + protocol: protocol, + name: "WrongMessageType-ServerHelloDone" + suffix, + config: Config{ + MaxVersion: VersionTLS12, + Bugs: ProtocolBugs{ + SendWrongMessageType: typeServerHelloDone, + }, + }, + shouldFail: true, + expectedError: ":UNEXPECTED_MESSAGE:", + expectedLocalError: "remote error: unexpected message", + }) + + testCases = append(testCases, testCase{ + testType: serverTest, + protocol: protocol, + name: "WrongMessageType-ClientCertificate" + suffix, + config: Config{ + Certificates: []Certificate{rsaCertificate}, + MaxVersion: VersionTLS12, + Bugs: ProtocolBugs{ + SendWrongMessageType: typeCertificate, + }, + }, + flags: []string{"-require-any-client-certificate"}, + shouldFail: true, + expectedError: ":UNEXPECTED_MESSAGE:", + expectedLocalError: "remote error: unexpected message", + }) + + testCases = append(testCases, testCase{ + testType: serverTest, + protocol: protocol, + name: "WrongMessageType-CertificateVerify" + suffix, + config: Config{ + Certificates: []Certificate{rsaCertificate}, + MaxVersion: VersionTLS12, + Bugs: ProtocolBugs{ + SendWrongMessageType: typeCertificateVerify, + }, + }, + flags: []string{"-require-any-client-certificate"}, + shouldFail: true, + expectedError: ":UNEXPECTED_MESSAGE:", + expectedLocalError: "remote error: unexpected message", + }) + + testCases = append(testCases, testCase{ + testType: serverTest, + protocol: protocol, + name: "WrongMessageType-ClientKeyExchange" + suffix, + config: Config{ + MaxVersion: VersionTLS12, + Bugs: ProtocolBugs{ + SendWrongMessageType: typeClientKeyExchange, + }, + }, + shouldFail: true, + expectedError: ":UNEXPECTED_MESSAGE:", + expectedLocalError: "remote error: unexpected message", + }) + + if protocol != dtls { + testCases = append(testCases, testCase{ + testType: serverTest, + protocol: protocol, + name: "WrongMessageType-NextProtocol" + suffix, + config: Config{ + MaxVersion: VersionTLS12, + NextProtos: []string{"bar"}, + Bugs: ProtocolBugs{ + SendWrongMessageType: typeNextProtocol, + }, + }, + flags: []string{"-advertise-npn", "\x03foo\x03bar\x03baz"}, + shouldFail: true, + expectedError: ":UNEXPECTED_MESSAGE:", + expectedLocalError: "remote error: unexpected message", + }) + + testCases = append(testCases, testCase{ + testType: serverTest, + protocol: protocol, + name: "WrongMessageType-ChannelID" + suffix, + config: Config{ + MaxVersion: VersionTLS12, + ChannelID: channelIDKey, + Bugs: ProtocolBugs{ + SendWrongMessageType: typeChannelID, + }, + }, + flags: []string{ + "-expect-channel-id", + base64.StdEncoding.EncodeToString(channelIDBytes), + }, + shouldFail: true, + expectedError: ":UNEXPECTED_MESSAGE:", + expectedLocalError: "remote error: unexpected message", + }) + } + + testCases = append(testCases, testCase{ + testType: serverTest, + protocol: protocol, + name: "WrongMessageType-ClientFinished" + suffix, + config: Config{ + MaxVersion: VersionTLS12, + Bugs: ProtocolBugs{ + SendWrongMessageType: typeFinished, + }, + }, + shouldFail: true, + expectedError: ":UNEXPECTED_MESSAGE:", + expectedLocalError: "remote error: unexpected message", + }) + + testCases = append(testCases, testCase{ + protocol: protocol, + name: "WrongMessageType-NewSessionTicket" + suffix, + config: Config{ + MaxVersion: VersionTLS12, + Bugs: ProtocolBugs{ + SendWrongMessageType: typeNewSessionTicket, + }, + }, + shouldFail: true, + expectedError: ":UNEXPECTED_MESSAGE:", + expectedLocalError: "remote error: unexpected message", + }) + + testCases = append(testCases, testCase{ + protocol: protocol, + name: "WrongMessageType-ServerFinished" + suffix, + config: Config{ + MaxVersion: VersionTLS12, + Bugs: ProtocolBugs{ + SendWrongMessageType: typeFinished, + }, + }, + shouldFail: true, + expectedError: ":UNEXPECTED_MESSAGE:", + expectedLocalError: "remote error: unexpected message", + }) + + } +} + func worker(statusChan chan statusMsg, c chan *testCase, shimPath string, wg *sync.WaitGroup) { defer wg.Done() @@ -6469,6 +6693,7 @@ addTLS13RecordTests() addAllStateMachineCoverageTests() addChangeCipherSpecTests() + addWrongMessageTypeTests() var wg sync.WaitGroup