Improve CCS/Handshake synchronization tests.

Test with and without PackHandshakeFlight enabled to cover when the
early post-CCS fragment will get packed into one of the pre-CCS
handshake records. Also test the resumption cases too to cover more
state transitions.

The various CCS-related tests (since CCS is kind of a mess) are pulled
into their own group.

Change-Id: I6384f2fb28d9885cd2b06d59e765e080e3822d8a
Reviewed-on: https://boringssl-review.googlesource.com/8661
Reviewed-by: David Benjamin <davidben@google.com>
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index b8f7088..64ec39f 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -1137,84 +1137,6 @@
 			expectedError: ":UNEXPECTED_MESSAGE:",
 		},
 		{
-			name: "SkipChangeCipherSpec-Client",
-			config: Config{
-				MaxVersion: VersionTLS12,
-				Bugs: ProtocolBugs{
-					SkipChangeCipherSpec: true,
-				},
-			},
-			shouldFail:    true,
-			expectedError: ":UNEXPECTED_RECORD:",
-		},
-		{
-			testType: serverTest,
-			name:     "SkipChangeCipherSpec-Server",
-			config: Config{
-				MaxVersion: VersionTLS12,
-				Bugs: ProtocolBugs{
-					SkipChangeCipherSpec: true,
-				},
-			},
-			shouldFail:    true,
-			expectedError: ":UNEXPECTED_RECORD:",
-		},
-		{
-			testType: serverTest,
-			name:     "SkipChangeCipherSpec-Server-NPN",
-			config: Config{
-				MaxVersion: VersionTLS12,
-				NextProtos: []string{"bar"},
-				Bugs: ProtocolBugs{
-					SkipChangeCipherSpec: true,
-				},
-			},
-			flags: []string{
-				"-advertise-npn", "\x03foo\x03bar\x03baz",
-			},
-			shouldFail:    true,
-			expectedError: ":UNEXPECTED_RECORD:",
-		},
-		{
-			name: "FragmentAcrossChangeCipherSpec-Client",
-			config: Config{
-				MaxVersion: VersionTLS12,
-				Bugs: ProtocolBugs{
-					FragmentAcrossChangeCipherSpec: true,
-				},
-			},
-			shouldFail:    true,
-			expectedError: ":UNEXPECTED_RECORD:",
-		},
-		{
-			testType: serverTest,
-			name:     "FragmentAcrossChangeCipherSpec-Server",
-			config: Config{
-				MaxVersion: VersionTLS12,
-				Bugs: ProtocolBugs{
-					FragmentAcrossChangeCipherSpec: true,
-				},
-			},
-			shouldFail:    true,
-			expectedError: ":UNEXPECTED_RECORD:",
-		},
-		{
-			testType: serverTest,
-			name:     "FragmentAcrossChangeCipherSpec-Server-NPN",
-			config: Config{
-				MaxVersion: VersionTLS12,
-				NextProtos: []string{"bar"},
-				Bugs: ProtocolBugs{
-					FragmentAcrossChangeCipherSpec: true,
-				},
-			},
-			flags: []string{
-				"-advertise-npn", "\x03foo\x03bar\x03baz",
-			},
-			shouldFail:    true,
-			expectedError: ":UNEXPECTED_RECORD:",
-		},
-		{
 			testType: serverTest,
 			name:     "Alert",
 			config: Config{
@@ -1288,43 +1210,6 @@
 			expectedError: ":BAD_ALERT:",
 		},
 		{
-			testType: serverTest,
-			name:     "EarlyChangeCipherSpec-server-1",
-			config: Config{
-				MaxVersion: VersionTLS12,
-				Bugs: ProtocolBugs{
-					EarlyChangeCipherSpec: 1,
-				},
-			},
-			shouldFail:    true,
-			expectedError: ":UNEXPECTED_RECORD:",
-		},
-		{
-			testType: serverTest,
-			name:     "EarlyChangeCipherSpec-server-2",
-			config: Config{
-				MaxVersion: VersionTLS12,
-				Bugs: ProtocolBugs{
-					EarlyChangeCipherSpec: 2,
-				},
-			},
-			shouldFail:    true,
-			expectedError: ":UNEXPECTED_RECORD:",
-		},
-		{
-			protocol: dtls,
-			name:     "StrayChangeCipherSpec",
-			config: Config{
-				// TODO(davidben): Once DTLS 1.3 exists, test
-				// that stray ChangeCipherSpec messages are
-				// rejected.
-				MaxVersion: VersionTLS12,
-				Bugs: ProtocolBugs{
-					StrayChangeCipherSpec: true,
-				},
-			},
-		},
-		{
 			name: "SkipNewSessionTicket",
 			config: Config{
 				MaxVersion: VersionTLS12,
@@ -2134,52 +2019,6 @@
 			expectResumeRejected: true,
 		},
 		{
-			name: "BadChangeCipherSpec-1",
-			config: Config{
-				MaxVersion: VersionTLS12,
-				Bugs: ProtocolBugs{
-					BadChangeCipherSpec: []byte{2},
-				},
-			},
-			shouldFail:    true,
-			expectedError: ":BAD_CHANGE_CIPHER_SPEC:",
-		},
-		{
-			name: "BadChangeCipherSpec-2",
-			config: Config{
-				MaxVersion: VersionTLS12,
-				Bugs: ProtocolBugs{
-					BadChangeCipherSpec: []byte{1, 1},
-				},
-			},
-			shouldFail:    true,
-			expectedError: ":BAD_CHANGE_CIPHER_SPEC:",
-		},
-		{
-			protocol: dtls,
-			name:     "BadChangeCipherSpec-DTLS-1",
-			config: Config{
-				MaxVersion: VersionTLS12,
-				Bugs: ProtocolBugs{
-					BadChangeCipherSpec: []byte{2},
-				},
-			},
-			shouldFail:    true,
-			expectedError: ":BAD_CHANGE_CIPHER_SPEC:",
-		},
-		{
-			protocol: dtls,
-			name:     "BadChangeCipherSpec-DTLS-2",
-			config: Config{
-				MaxVersion: VersionTLS12,
-				Bugs: ProtocolBugs{
-					BadChangeCipherSpec: []byte{1, 1},
-				},
-			},
-			shouldFail:    true,
-			expectedError: ":BAD_CHANGE_CIPHER_SPEC:",
-		},
-		{
 			name:        "BadHelloRequest-1",
 			renegotiate: 1,
 			config: Config{
@@ -5788,6 +5627,226 @@
 	})
 }
 
+func addChangeCipherSpecTests() {
+	// Test missing ChangeCipherSpecs.
+	testCases = append(testCases, testCase{
+		name: "SkipChangeCipherSpec-Client",
+		config: Config{
+			MaxVersion: VersionTLS12,
+			Bugs: ProtocolBugs{
+				SkipChangeCipherSpec: true,
+			},
+		},
+		shouldFail:    true,
+		expectedError: ":UNEXPECTED_RECORD:",
+	})
+	testCases = append(testCases, testCase{
+		testType: serverTest,
+		name:     "SkipChangeCipherSpec-Server",
+		config: Config{
+			MaxVersion: VersionTLS12,
+			Bugs: ProtocolBugs{
+				SkipChangeCipherSpec: true,
+			},
+		},
+		shouldFail:    true,
+		expectedError: ":UNEXPECTED_RECORD:",
+	})
+	testCases = append(testCases, testCase{
+		testType: serverTest,
+		name:     "SkipChangeCipherSpec-Server-NPN",
+		config: Config{
+			MaxVersion: VersionTLS12,
+			NextProtos: []string{"bar"},
+			Bugs: ProtocolBugs{
+				SkipChangeCipherSpec: true,
+			},
+		},
+		flags: []string{
+			"-advertise-npn", "\x03foo\x03bar\x03baz",
+		},
+		shouldFail:    true,
+		expectedError: ":UNEXPECTED_RECORD:",
+	})
+
+	// Test synchronization between the handshake and ChangeCipherSpec.
+	// Partial post-CCS handshake messages before ChangeCipherSpec should be
+	// rejected. Test both with and without handshake packing to handle both
+	// when the partial post-CCS message is in its own record and when it is
+	// attached to the pre-CCS message.
+	//
+	// TODO(davidben): Fix and test DTLS as well.
+	for _, packed := range []bool{false, true} {
+		var suffix string
+		if packed {
+			suffix = "-Packed"
+		}
+
+		testCases = append(testCases, testCase{
+			name: "FragmentAcrossChangeCipherSpec-Client" + suffix,
+			config: Config{
+				MaxVersion: VersionTLS12,
+				Bugs: ProtocolBugs{
+					FragmentAcrossChangeCipherSpec: true,
+					PackHandshakeFlight:            packed,
+				},
+			},
+			shouldFail:    true,
+			expectedError: ":UNEXPECTED_RECORD:",
+		})
+		testCases = append(testCases, testCase{
+			name: "FragmentAcrossChangeCipherSpec-Client-Resume" + suffix,
+			config: Config{
+				MaxVersion: VersionTLS12,
+			},
+			resumeSession: true,
+			resumeConfig: &Config{
+				MaxVersion: VersionTLS12,
+				Bugs: ProtocolBugs{
+					FragmentAcrossChangeCipherSpec: true,
+					PackHandshakeFlight:            packed,
+				},
+			},
+			shouldFail:    true,
+			expectedError: ":UNEXPECTED_RECORD:",
+		})
+		testCases = append(testCases, testCase{
+			testType: serverTest,
+			name:     "FragmentAcrossChangeCipherSpec-Server" + suffix,
+			config: Config{
+				MaxVersion: VersionTLS12,
+				Bugs: ProtocolBugs{
+					FragmentAcrossChangeCipherSpec: true,
+					PackHandshakeFlight:            packed,
+				},
+			},
+			shouldFail:    true,
+			expectedError: ":UNEXPECTED_RECORD:",
+		})
+		testCases = append(testCases, testCase{
+			testType: serverTest,
+			name:     "FragmentAcrossChangeCipherSpec-Server-Resume" + suffix,
+			config: Config{
+				MaxVersion: VersionTLS12,
+			},
+			resumeSession: true,
+			resumeConfig: &Config{
+				MaxVersion: VersionTLS12,
+				Bugs: ProtocolBugs{
+					FragmentAcrossChangeCipherSpec: true,
+					PackHandshakeFlight:            packed,
+				},
+			},
+			shouldFail:    true,
+			expectedError: ":UNEXPECTED_RECORD:",
+		})
+		testCases = append(testCases, testCase{
+			testType: serverTest,
+			name:     "FragmentAcrossChangeCipherSpec-Server-NPN" + suffix,
+			config: Config{
+				MaxVersion: VersionTLS12,
+				NextProtos: []string{"bar"},
+				Bugs: ProtocolBugs{
+					FragmentAcrossChangeCipherSpec: true,
+					PackHandshakeFlight:            packed,
+				},
+			},
+			flags: []string{
+				"-advertise-npn", "\x03foo\x03bar\x03baz",
+			},
+			shouldFail:    true,
+			expectedError: ":UNEXPECTED_RECORD:",
+		})
+	}
+
+	// Test that early ChangeCipherSpecs are handled correctly.
+	testCases = append(testCases, testCase{
+		testType: serverTest,
+		name:     "EarlyChangeCipherSpec-server-1",
+		config: Config{
+			MaxVersion: VersionTLS12,
+			Bugs: ProtocolBugs{
+				EarlyChangeCipherSpec: 1,
+			},
+		},
+		shouldFail:    true,
+		expectedError: ":UNEXPECTED_RECORD:",
+	})
+	testCases = append(testCases, testCase{
+		testType: serverTest,
+		name:     "EarlyChangeCipherSpec-server-2",
+		config: Config{
+			MaxVersion: VersionTLS12,
+			Bugs: ProtocolBugs{
+				EarlyChangeCipherSpec: 2,
+			},
+		},
+		shouldFail:    true,
+		expectedError: ":UNEXPECTED_RECORD:",
+	})
+	testCases = append(testCases, testCase{
+		protocol: dtls,
+		name:     "StrayChangeCipherSpec",
+		config: Config{
+			// TODO(davidben): Once DTLS 1.3 exists, test
+			// that stray ChangeCipherSpec messages are
+			// rejected.
+			MaxVersion: VersionTLS12,
+			Bugs: ProtocolBugs{
+				StrayChangeCipherSpec: true,
+			},
+		},
+	})
+
+	// Test that the contents of ChangeCipherSpec are checked.
+	testCases = append(testCases, testCase{
+		name: "BadChangeCipherSpec-1",
+		config: Config{
+			MaxVersion: VersionTLS12,
+			Bugs: ProtocolBugs{
+				BadChangeCipherSpec: []byte{2},
+			},
+		},
+		shouldFail:    true,
+		expectedError: ":BAD_CHANGE_CIPHER_SPEC:",
+	})
+	testCases = append(testCases, testCase{
+		name: "BadChangeCipherSpec-2",
+		config: Config{
+			MaxVersion: VersionTLS12,
+			Bugs: ProtocolBugs{
+				BadChangeCipherSpec: []byte{1, 1},
+			},
+		},
+		shouldFail:    true,
+		expectedError: ":BAD_CHANGE_CIPHER_SPEC:",
+	})
+	testCases = append(testCases, testCase{
+		protocol: dtls,
+		name:     "BadChangeCipherSpec-DTLS-1",
+		config: Config{
+			MaxVersion: VersionTLS12,
+			Bugs: ProtocolBugs{
+				BadChangeCipherSpec: []byte{2},
+			},
+		},
+		shouldFail:    true,
+		expectedError: ":BAD_CHANGE_CIPHER_SPEC:",
+	})
+	testCases = append(testCases, testCase{
+		protocol: dtls,
+		name:     "BadChangeCipherSpec-DTLS-2",
+		config: Config{
+			MaxVersion: VersionTLS12,
+			Bugs: ProtocolBugs{
+				BadChangeCipherSpec: []byte{1, 1},
+			},
+		},
+		shouldFail:    true,
+		expectedError: ":BAD_CHANGE_CIPHER_SPEC:",
+	})
+}
+
 func worker(statusChan chan statusMsg, c chan *testCase, shimPath string, wg *sync.WaitGroup) {
 	defer wg.Done()
 
@@ -5890,6 +5949,7 @@
 	addKeyExchangeInfoTests()
 	addTLS13RecordTests()
 	addAllStateMachineCoverageTests()
+	addChangeCipherSpecTests()
 
 	var wg sync.WaitGroup