Process alerts between ChangeCipherSpec and Finished.

This mostly[*] doesn't matter for TLS since the message would have been
rejected anyway, but, in DTLS, if the peer rejects our Finished, it will send
an encrypted alert. This will then cause it to hang, which isn't very helpful.

I've made the change on both TLS and DTLS so the two protocols don't diverge on
this point. It is true that we're accepting nominally encrypted and
authenticated alerts before Finished, but, prior to ChangeCipherSpec, the
alerts are sent in the clear anyway so an attacker could already inject alerts.
A consumer could only be sensitive to it being post-CCS if it was watching
msg_callback. The only non-debug consumer of msg_callback I've found anywhere
is some hostapd code to detect Heartbeat.

See https://code.google.com/p/webrtc/issues/detail?id=4403 for an instance
where the equivalent behavior in OpenSSL masks an alert.

[*] This does change behavior slightly if the peer sends a warning alert
between CCS and Finished. I believe this is benign as warning alerts are
usually ignored apart from info_callback and msg_callback. The one exception is
a close_notify which is a slightly new state (accepting close_notify during a
handshake seems questionable...), but they're processed pre-CCS too.

Change-Id: Idd0d49b9f9aa9d35374a9f5e2f815cdb931f5254
Reviewed-on: https://boringssl-review.googlesource.com/3883
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index 4aead4e..b8cc44a 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -610,6 +610,10 @@
 	// be sent immediately after ChangeCipherSpec.
 	AppDataAfterChangeCipherSpec []byte
 
+	// AlertAfterChangeCipherSpec, if non-zero, causes an alert to be sent
+	// immediately after ChangeCipherSpec.
+	AlertAfterChangeCipherSpec alert
+
 	// TimeoutSchedule is the schedule of packet drops and simulated
 	// timeouts for before each handshake leg from the peer.
 	TimeoutSchedule []time.Duration
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go
index 17cfc01..a4fab0c 100644
--- a/ssl/test/runner/handshake_client.go
+++ b/ssl/test/runner/handshake_client.go
@@ -863,6 +863,10 @@
 	if c.config.Bugs.AppDataAfterChangeCipherSpec != nil {
 		c.writeRecord(recordTypeApplicationData, c.config.Bugs.AppDataAfterChangeCipherSpec)
 	}
+	if c.config.Bugs.AlertAfterChangeCipherSpec != 0 {
+		c.sendAlert(c.config.Bugs.AlertAfterChangeCipherSpec)
+		return errors.New("tls: simulating post-CCS alert")
+	}
 
 	if !c.config.Bugs.SkipFinished {
 		c.writeRecord(recordTypeHandshake, postCCSBytes)
diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go
index e1d49e5..9085faf 100644
--- a/ssl/test/runner/handshake_server.go
+++ b/ssl/test/runner/handshake_server.go
@@ -854,6 +854,10 @@
 	if c.config.Bugs.AppDataAfterChangeCipherSpec != nil {
 		c.writeRecord(recordTypeApplicationData, c.config.Bugs.AppDataAfterChangeCipherSpec)
 	}
+	if c.config.Bugs.AlertAfterChangeCipherSpec != 0 {
+		c.sendAlert(c.config.Bugs.AlertAfterChangeCipherSpec)
+		return errors.New("tls: simulating post-CCS alert")
+	}
 
 	if !c.config.Bugs.SkipFinished {
 		c.writeRecord(recordTypeHandshake, postCCSBytes)
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index dc27513..d1dbe87 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -691,6 +691,27 @@
 		},
 	},
 	{
+		name: "AlertAfterChangeCipherSpec",
+		config: Config{
+			Bugs: ProtocolBugs{
+				AlertAfterChangeCipherSpec: alertRecordOverflow,
+			},
+		},
+		shouldFail:    true,
+		expectedError: ":TLSV1_ALERT_RECORD_OVERFLOW:",
+	},
+	{
+		protocol: dtls,
+		name:     "AlertAfterChangeCipherSpec-DTLS",
+		config: Config{
+			Bugs: ProtocolBugs{
+				AlertAfterChangeCipherSpec: alertRecordOverflow,
+			},
+		},
+		shouldFail:    true,
+		expectedError: ":TLSV1_ALERT_RECORD_OVERFLOW:",
+	},
+	{
 		protocol: dtls,
 		name:     "ReorderHandshakeFragments-Small-DTLS",
 		config: Config{