Reject empty records of unexpected type.
The old empty record logic discarded the records at a very low-level.
Let the error bubble up to ssl3_read_bytes so the type mismatch logic
may kick in before the empty record is skipped.
Add tests for when the record in question is application data, before
before the handshake and post ChangeCipherSpec.
BUG=521840
Change-Id: I47dff389cda65d6672b9be39d7d89490331063fa
Reviewed-on: https://boringssl-review.googlesource.com/5754
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/tls_record.c b/ssl/tls_record.c
index ae75495..5fd9792 100644
--- a/ssl/tls_record.c
+++ b/ssl/tls_record.c
@@ -116,6 +116,12 @@
#include "internal.h"
+/* kMaxEmptyRecords is the number of consecutive, empty records that will be
+ * processed. Without this limit an attacker could send empty records at a
+ * faster rate than we can process and cause record processing to loop
+ * forever. */
+static const uint8_t kMaxEmptyRecords = 32;
+
size_t ssl_record_prefix_len(const SSL *ssl) {
if (SSL_IS_DTLS(ssl)) {
return DTLS1_RT_HEADER_LENGTH +
@@ -224,6 +230,20 @@
return ssl_open_record_error;
}
+ /* Limit the number of consecutive empty records. */
+ if (plaintext_len == 0) {
+ ssl->s3->empty_record_count++;
+ if (ssl->s3->empty_record_count > kMaxEmptyRecords) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_TOO_MANY_EMPTY_FRAGMENTS);
+ *out_alert = SSL_AD_UNEXPECTED_MESSAGE;
+ return ssl_open_record_error;
+ }
+ /* Apart from the limit, empty records are returned up to the caller. This
+ * allows the caller to reject records of the wrong type. */
+ } else {
+ ssl->s3->empty_record_count = 0;
+ }
+
*out_type = type;
*out_len = plaintext_len;
*out_consumed = in_len - CBS_len(&cbs);
