Actually check that the message has the expected type in DTLS.

That might be a reasonable check to make, maybe.

DTLS handshake message reading has a ton of other bugs and needs a complete
rewrite. But let's fix this and get a test in now.

Change-Id: I4981fc302feb9125908bb6161ed1a18288c39e2b
Reviewed-on: https://boringssl-review.googlesource.com/3600
Reviewed-by: Adam Langley <agl@google.com>

4 files changed, 44 insertions, 7 deletions

diff --git a/ssl/d1_both.c b/ssl/d1_both.c
index 1390863d..16c3e084 100644
--- a/ssl/d1_both.c
+++ b/ssl/d1_both.c
@@ -359,10 +359,10 @@ int dtls1_do_write(SSL *s, int type) {
 }
 
 
-/* Obtain handshake message of message type 'mt' (any if mt == -1), maximum
- * acceptable body length 'max'. Read an entire handshake message. Handshake
- * messages arrive in fragments. */
-long dtls1_get_message(SSL *s, int st1, int stn, int mt, long max,
+/* dtls1_get_message reads a handshake message of message type |msg_type| (any
+ * if |msg_type| == -1), maximum acceptable body length |max|. Read an entire
+ * handshake message. Handshake messages arrive in fragments. */
+long dtls1_get_message(SSL *s, int st1, int stn, int msg_type, long max,
                        int hash_message, int *ok) {
   int i, al;
   struct hm_header_st *msg_hdr;
@@ -377,7 +377,7 @@ long dtls1_get_message(SSL *s, int st1, int stn, int mt, long max,
      * would have to have been applied to the previous call. */
     assert(hash_message != SSL_GET_MESSAGE_DONT_HASH_MESSAGE);
     s->s3->tmp.reuse_message = 0;
-    if (mt >= 0 && s->s3->tmp.message_type != mt) {
+    if (msg_type >= 0 && s->s3->tmp.message_type != msg_type) {
       al = SSL_AD_UNEXPECTED_MESSAGE;
       OPENSSL_PUT_ERROR(SSL, dtls1_get_message, SSL_R_UNEXPECTED_MESSAGE);
       goto f_err;
@@ -401,6 +401,12 @@ again:
     return i;
   }
 
+  if (msg_type >= 0 && msg_hdr->type != msg_type) {
+    al = SSL_AD_UNEXPECTED_MESSAGE;
+    OPENSSL_PUT_ERROR(SSL, dtls1_get_message, SSL_R_UNEXPECTED_MESSAGE);
+    goto f_err;
+  }
+
   p = (uint8_t *)s->init_buf->data;
   msg_len = msg_hdr->msg_len;
 
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index e8df1aa9..b505c8f7 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -625,6 +625,10 @@ type ProtocolBugs struct {
 	// SendInvalidRecordType, if true, causes a record with an invalid
 	// 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
 }
 
 func (c *Config) serverInit() {
diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go
index 9c920d6e..50cc20bb 100644
--- a/ssl/test/runner/handshake_server.go
+++ b/ssl/test/runner/handshake_server.go
@@ -490,8 +490,12 @@ func (hs *serverHandshakeState) doFullHandshake() error {
 		certMsg := new(certificateMsg)
 		certMsg.certificates = hs.cert.Certificate
 		if !config.Bugs.UnauthenticatedECDH {
-			hs.writeServerHash(certMsg.marshal())
-			c.writeRecord(recordTypeHandshake, certMsg.marshal())
+			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 40798634..68573586 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -788,6 +788,29 @@ var testCases = []testCase{
 		expectedError:      ":CONNECTION_REJECTED:",
 		expectedLocalError: "remote error: access denied",
 	},
+	{
+		name: "WrongMessageType",
+		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",
+	},
 }
 
 func doExchange(test *testCase, config *Config, conn net.Conn, messageLen int, isResume bool) error {