runner: Move some test bugs into the callback Now that we have this callback, we can make the general logic less complex and shift it to the tests. Change-Id: I2523bee122a16591fb32275f27a27a31d5a0b33c Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/73287 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Nick Harper <nharper@chromium.org>
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go index 36e5a8d..e5ba6d4 100644 --- a/ssl/test/runner/common.go +++ b/ssl/test/runner/common.go
@@ -779,11 +779,6 @@ // 1.0.1 and 0.9.8 modes, respectively. EarlyChangeCipherSpec int - // StrayChangeCipherSpec causes every pre-ChangeCipherSpec handshake - // message in DTLS to be prefaced by stray ChangeCipherSpec record. This - // may be used to test DTLS's handling of reordered ChangeCipherSpec. - StrayChangeCipherSpec bool - // FragmentAcrossChangeCipherSpec causes the implementation to fragment // the Finished (or NextProto) message around the ChangeCipherSpec // messages. @@ -797,10 +792,6 @@ // a ChangeCipherSpec record before every application data record. SendPostHandshakeChangeCipherSpec bool - // SendUnencryptedFinished, if true, causes the Finished message to be - // send unencrypted before ChangeCipherSpec rather than after it. - SendUnencryptedFinished bool - // PartialEncryptedExtensionsWithServerHello, if true, causes the TLS // 1.3 server to send part of EncryptedExtensions unencrypted // in the same record as ServerHello. @@ -1310,19 +1301,11 @@ // Finished and will trigger a spurious retransmit.) ReorderHandshakeFragments bool - // ReverseHandshakeFragments, if true, causes handshake fragments in - // DTLS to be reversed within a flight. - ReverseHandshakeFragments bool - // MixCompleteMessageWithFragments, if true, causes handshake // messages in DTLS to redundantly both fragment the message // and include a copy of the full one. MixCompleteMessageWithFragments bool - // RetransmitFinished, if true, causes the DTLS Finished message to be - // sent twice. - RetransmitFinished bool - // SendInvalidRecordType, if true, causes a record with an invalid // content type to be sent immediately following the handshake. SendInvalidRecordType bool @@ -1335,14 +1318,6 @@ // specified type to be sent with trailing data. SendTrailingMessageData byte - // FragmentMessageTypeMismatch, if true, causes all non-initial - // handshake fragments in DTLS to have the wrong message type. - FragmentMessageTypeMismatch bool - - // FragmentMessageLengthMismatch, if true, causes all non-initial - // handshake fragments in DTLS to have the wrong message length. - FragmentMessageLengthMismatch bool - // SplitFragments, if non-zero, causes the handshake fragments in DTLS // to be split across two records. The value of |SplitFragments| is the // number of bytes in the first fragment.
diff --git a/ssl/test/runner/dtls.go b/ssl/test/runner/dtls.go index 4ced9f2..5011815 100644 --- a/ssl/test/runner/dtls.go +++ b/ssl/test/runner/dtls.go
@@ -977,10 +977,6 @@ continue } - if msg.Epoch == 0 && config.Bugs.StrayChangeCipherSpec { - fragments = append(fragments, DTLSFragment{Epoch: msg.Epoch, IsChangeCipherSpec: true, Data: []byte{1}}) - } - maxLen := config.Bugs.MaxHandshakeRecordLength if maxLen <= 0 { maxLen = 1024 @@ -998,13 +994,6 @@ fragLen := min(len(msg.Data)-fragOffset, maxLen) fragment := msg.Fragment(fragOffset, fragLen) - if config.Bugs.FragmentMessageTypeMismatch && fragOffset > 0 { - fragment.Type++ - } - if config.Bugs.FragmentMessageLengthMismatch && fragOffset > 0 { - fragment.TotalLength++ - } - fragments = append(fragments, fragment) if config.Bugs.ReorderHandshakeFragments { // Don't duplicate Finished to avoid the peer @@ -1020,11 +1009,7 @@ } fragOffset += fragLen } - shouldSendTwice := config.Bugs.MixCompleteMessageWithFragments - if msg.Type == typeFinished { - shouldSendTwice = config.Bugs.RetransmitFinished - } - if shouldSendTwice { + if config.Bugs.MixCompleteMessageWithFragments { fragments = append(fragments, msg.Fragment(0, len(msg.Data))) } } @@ -1038,8 +1023,6 @@ chunk := fragments[start:end] if config.Bugs.ReorderHandshakeFragments { rand.Shuffle(len(chunk), func(i, j int) { chunk[i], chunk[j] = chunk[j], chunk[i] }) - } else if config.Bugs.ReverseHandshakeFragments { - slices.Reverse(chunk) } start = end }
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go index c71c295..4543339 100644 --- a/ssl/test/runner/handshake_client.go +++ b/ssl/test/runner/handshake_client.go
@@ -2272,9 +2272,6 @@ if c.config.Bugs.FragmentAcrossChangeCipherSpec { c.writeRecord(recordTypeHandshake, postCCSMsgs[0][:5]) postCCSMsgs[0] = postCCSMsgs[0][5:] - } else if c.config.Bugs.SendUnencryptedFinished { - c.writeRecord(recordTypeHandshake, postCCSMsgs[0]) - postCCSMsgs = postCCSMsgs[1:] } if !c.config.Bugs.SkipChangeCipherSpec &&
diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go index 819fbc9..7f890fd 100644 --- a/ssl/test/runner/handshake_server.go +++ b/ssl/test/runner/handshake_server.go
@@ -2173,9 +2173,6 @@ if c.config.Bugs.FragmentAcrossChangeCipherSpec { c.writeRecord(recordTypeHandshake, postCCSBytes[:5]) postCCSBytes = postCCSBytes[5:] - } else if c.config.Bugs.SendUnencryptedFinished { - c.writeRecord(recordTypeHandshake, postCCSBytes) - postCCSBytes = nil } if !c.config.Bugs.SkipChangeCipherSpec {
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index 54b19ee..72bf0a8 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go
@@ -2808,8 +2808,12 @@ name: "FragmentMessageTypeMismatch-DTLS", config: Config{ Bugs: ProtocolBugs{ - MaxHandshakeRecordLength: 2, - FragmentMessageTypeMismatch: true, + WriteFlightDTLS: func(c *DTLSController, prev, received, next []DTLSMessage, records []DTLSRecordNumberInfo) { + f1 := next[0].Fragment(0, 1) + f2 := next[0].Fragment(1, 1) + f2.Type++ + c.WriteFragments([]DTLSFragment{f1, f2}) + }, }, }, shouldFail: true, @@ -2820,8 +2824,12 @@ name: "FragmentMessageLengthMismatch-DTLS", config: Config{ Bugs: ProtocolBugs{ - MaxHandshakeRecordLength: 2, - FragmentMessageLengthMismatch: true, + WriteFlightDTLS: func(c *DTLSController, prev, received, next []DTLSMessage, records []DTLSRecordNumberInfo) { + f1 := next[0].Fragment(0, 1) + f2 := next[0].Fragment(1, 1) + f2.TotalLength++ + c.WriteFragments([]DTLSFragment{f1, f2}) + }, }, }, shouldFail: true, @@ -12577,7 +12585,14 @@ config: Config{ MaxVersion: VersionTLS12, Bugs: ProtocolBugs{ - RetransmitFinished: true, + WriteFlightDTLS: func(c *DTLSController, prev, received, next []DTLSMessage, records []DTLSRecordNumberInfo) { + c.WriteFlight(next) + for _, msg := range next { + if msg.Type == typeFinished { + c.WriteFlight([]DTLSMessage{msg}) + } + } + }, }, }, }) @@ -12591,7 +12606,14 @@ resumeConfig: &Config{ MaxVersion: VersionTLS12, Bugs: ProtocolBugs{ - RetransmitFinished: true, + WriteFlightDTLS: func(c *DTLSController, prev, received, next []DTLSMessage, records []DTLSRecordNumberInfo) { + c.WriteFlight(next) + for _, msg := range next { + if msg.Type == typeFinished { + c.WriteFlight([]DTLSMessage{msg}) + } + } + }, }, }, resumeSession: true, @@ -14315,19 +14337,24 @@ expectedLocalError: "remote error: unexpected message", }) - // Test that, in DTLS, ChangeCipherSpec is not allowed when there are - // messages in the handshake queue. Do this by testing the server - // reading the client Finished, reversing the flight so Finished comes - // first. + // Test that, in DTLS 1.2, key changes are not allowed when there are + // buffered messages. Do this sending all messages in reverse, so that later + // ones are buffered, and leaving Finished unencrypted. testCases = append(testCases, testCase{ protocol: dtls, testType: serverTest, - name: "SendUnencryptedFinished-DTLS", + name: "KeyChangeWithBufferedMessages-DTLS", config: Config{ MaxVersion: VersionTLS12, Bugs: ProtocolBugs{ - SendUnencryptedFinished: true, - ReverseHandshakeFragments: true, + WriteFlightDTLS: func(c *DTLSController, prev, received, next []DTLSMessage, records []DTLSRecordNumberInfo) { + next = slices.Clone(next) + slices.Reverse(next) + for i := range next { + next[i].Epoch = 0 + } + c.WriteFlight(next) + }, }, }, shouldFail: true, @@ -14425,7 +14452,10 @@ // rejected. MaxVersion: VersionTLS12, Bugs: ProtocolBugs{ - StrayChangeCipherSpec: true, + WriteFlightDTLS: func(c *DTLSController, prev, received, next []DTLSMessage, records []DTLSRecordNumberInfo) { + c.WriteFragments([]DTLSFragment{{IsChangeCipherSpec: true, Data: []byte{1}}}) + c.WriteFlight(next) + }, }, }, })