Reject all invalid records.
The check on the DTLS side was broken anyway. On the TLS side, the spec does
say to ignore them, but there should be no need for this in future-proofing and
NSS doesn't appear to be lenient here. See also
https://boringssl-review.googlesource.com/#/c/3233/
Change-Id: I0846222936c5e08acdcfd9d6f854a99df767e468
Reviewed-on: https://boringssl-review.googlesource.com/3290
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/d1_pkt.c b/ssl/d1_pkt.c
index 6bc92a9..0b15fed 100644
--- a/ssl/d1_pkt.c
+++ b/ssl/d1_pkt.c
@@ -852,32 +852,11 @@
}
}
- switch (rr->type) {
- default:
- /* TLS just ignores unknown message types */
- if (s->version == TLS1_VERSION) {
- rr->length = 0;
- goto start;
- }
- al = SSL_AD_UNEXPECTED_MESSAGE;
- OPENSSL_PUT_ERROR(SSL, dtls1_read_bytes, SSL_R_UNEXPECTED_RECORD);
- goto f_err;
+ /* We already handled these. */
+ assert(rr->type != SSL3_RT_CHANGE_CIPHER_SPEC && rr->type != SSL3_RT_ALERT);
- case SSL3_RT_CHANGE_CIPHER_SPEC:
- case SSL3_RT_ALERT:
- /* We already handled all of these. */
- al = SSL_AD_UNEXPECTED_MESSAGE;
- OPENSSL_PUT_ERROR(SSL, dtls1_read_bytes, ERR_R_INTERNAL_ERROR);
- goto f_err;
-
- case SSL3_RT_HANDSHAKE:
- case SSL3_RT_APPLICATION_DATA:
- al = SSL_AD_UNEXPECTED_MESSAGE;
- OPENSSL_PUT_ERROR(SSL, dtls1_read_bytes, SSL_R_UNEXPECTED_RECORD);
- goto f_err;
- }
-
- /* not reached */
+ al = SSL_AD_UNEXPECTED_MESSAGE;
+ OPENSSL_PUT_ERROR(SSL, dtls1_read_bytes, SSL_R_UNEXPECTED_RECORD);
f_err:
ssl3_send_alert(s, SSL3_AL_FATAL, al);
diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c
index 4263cb0..368fb92 100644
--- a/ssl/s3_pkt.c
+++ b/ssl/s3_pkt.c
@@ -1072,26 +1072,16 @@
switch (rr->type) {
default:
- /* TLS up to v1.1 just ignores unknown message types. TLS v1.2 gives an
- * unexpected message alert. */
- if (s->version >= TLS1_VERSION && s->version <= TLS1_1_VERSION) {
- rr->length = 0;
- goto start;
- }
+ /* We already handled all of these, with the possible exception of
+ * SSL3_RT_HANDSHAKE when s->in_handshake is set, but that should not
+ * happen when type != rr->type. */
+ assert(rr->type != SSL3_RT_CHANGE_CIPHER_SPEC &&
+ rr->type != SSL3_RT_ALERT && rr->type != SSL3_RT_HANDSHAKE);
+
al = SSL_AD_UNEXPECTED_MESSAGE;
OPENSSL_PUT_ERROR(SSL, ssl3_read_bytes, SSL_R_UNEXPECTED_RECORD);
goto f_err;
- case SSL3_RT_CHANGE_CIPHER_SPEC:
- case SSL3_RT_ALERT:
- case SSL3_RT_HANDSHAKE:
- /* we already handled all of these, with the possible exception of
- * SSL3_RT_HANDSHAKE when s->in_handshake is set, but that should not
- * happen when type != rr->type */
- al = SSL_AD_UNEXPECTED_MESSAGE;
- OPENSSL_PUT_ERROR(SSL, ssl3_read_bytes, ERR_R_INTERNAL_ERROR);
- goto f_err;
-
case SSL3_RT_APPLICATION_DATA:
/* At this point we were expecting handshake data but have application
* data. If the library was running inside ssl3_read() (i.e.
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index 3e37f1d..f5ceefc 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -616,6 +616,10 @@
// pre-CCS flights to be sent twice. (Post-CCS flights consist of
// Finished and will trigger a spurious retransmit.)
ReorderHandshakeFragments bool
+
+ // SendInvalidRecordType, if true, causes a record with an invalid
+ // content type to be sent immediately following the handshake.
+ SendInvalidRecordType bool
}
func (c *Config) serverInit() {
diff --git a/ssl/test/runner/conn.go b/ssl/test/runner/conn.go
index 86a7b7d..90ce9bb 100644
--- a/ssl/test/runner/conn.go
+++ b/ssl/test/runner/conn.go
@@ -1250,6 +1250,9 @@
} else {
c.handshakeErr = c.serverHandshake()
}
+ if c.handshakeErr == nil && c.config.Bugs.SendInvalidRecordType {
+ c.writeRecord(recordType(42), []byte("invalid record"))
+ }
return c.handshakeErr
}
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index ef90882..6ee34ad 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -695,6 +695,27 @@
},
},
},
+ {
+ name: "SendInvalidRecordType",
+ config: Config{
+ Bugs: ProtocolBugs{
+ SendInvalidRecordType: true,
+ },
+ },
+ shouldFail: true,
+ expectedError: ":UNEXPECTED_RECORD:",
+ },
+ {
+ protocol: dtls,
+ name: "SendInvalidRecordType-DTLS",
+ config: Config{
+ Bugs: ProtocolBugs{
+ SendInvalidRecordType: true,
+ },
+ },
+ shouldFail: true,
+ expectedError: ":UNEXPECTED_RECORD:",
+ },
}
func doExchange(test *testCase, config *Config, conn net.Conn, messageLen int, isResume bool) error {
