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>
diff --git a/ssl/d1_both.c b/ssl/d1_both.c
index 1390863..16c3e08 100644
--- a/ssl/d1_both.c
+++ b/ssl/d1_both.c
@@ -359,10 +359,10 @@
}
-/* 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 @@
* 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 @@
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 e8df1aa..b505c8f 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -625,6 +625,10 @@
// 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 9c920d6..50cc20b 100644
--- a/ssl/test/runner/handshake_server.go
+++ b/ssl/test/runner/handshake_server.go
@@ -490,8 +490,12 @@
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 4079863..6857358 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -788,6 +788,29 @@
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 {
