Enforce max_early_data_size on the server. BUG=76 Change-Id: I8b754ba17b3e0beee425929e4b53785b2e95f0ae Reviewed-on: https://boringssl-review.googlesource.com/15164 Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com> CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/crypto/err/ssl.errordata b/crypto/err/ssl.errordata index 9ac015c..641d873 100644 --- a/crypto/err/ssl.errordata +++ b/crypto/err/ssl.errordata
@@ -210,3 +210,4 @@ SSL,278,WRONG_VERSION_ON_EARLY_DATA SSL,248,X509_LIB SSL,249,X509_VERIFICATION_SETUP_PROBLEMS +SSL,1117,TOO_MUCH_READ_EARLY_DATA
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index b52d80c..1ee7e5c 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h
@@ -4664,5 +4664,6 @@ #define SSL_R_TLSV1_BAD_CERTIFICATE_HASH_VALUE 1114 #define SSL_R_TLSV1_UNKNOWN_PSK_IDENTITY 1115 #define SSL_R_TLSV1_CERTIFICATE_REQUIRED 1116 +#define SSL_R_TOO_MUCH_READ_EARLY_DATA 1117 #endif /* OPENSSL_HEADER_SSL_H */
diff --git a/ssl/internal.h b/ssl/internal.h index 4d148c6..1ed7e15 100644 --- a/ssl/internal.h +++ b/ssl/internal.h
@@ -1121,6 +1121,10 @@ /* client_version is the value sent or received in the ClientHello version. */ uint16_t client_version; + + /* early_data_read is the amount of early data that has been read by the + * record layer. */ + uint16_t early_data_read; } /* SSL_HANDSHAKE */; SSL_HANDSHAKE *ssl_handshake_new(SSL *ssl); @@ -1964,6 +1968,11 @@ #define SSL_KEY_UPDATE_NOT_REQUESTED 0 #define SSL_KEY_UPDATE_REQUESTED 1 +/* kMaxEarlyDataAccepted is the advertised number of plaintext bytes of early + * data that will be accepted. This value should be slightly below + * kMaxEarlyDataSkipped in tls_record.c, which is measured in ciphertext. */ +static const size_t kMaxEarlyDataAccepted = 14336; + CERT *ssl_cert_new(const SSL_X509_METHOD *x509_method); CERT *ssl_cert_dup(CERT *cert); void ssl_cert_clear_certs(CERT *c);
diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c index 8e3613a..23b39f2 100644 --- a/ssl/s3_pkt.c +++ b/ssl/s3_pkt.c
@@ -395,15 +395,17 @@ return -1; } + const int is_early_data_read = ssl->server && + ssl->s3->hs != NULL && + ssl->s3->hs->can_early_read && + ssl3_protocol_version(ssl) >= TLS1_3_VERSION; + /* Handle the end_of_early_data alert. */ if (rr->type == SSL3_RT_ALERT && rr->length == 2 && rr->data[0] == SSL3_AL_WARNING && rr->data[1] == TLS1_AD_END_OF_EARLY_DATA && - ssl->server && - ssl->s3->hs != NULL && - ssl->s3->hs->can_early_read && - ssl3_protocol_version(ssl) >= TLS1_3_VERSION) { + is_early_data_read) { /* Consume the record. */ rr->length = 0; ssl_read_buffer_discard(ssl); @@ -419,6 +421,16 @@ return -1; } + if (is_early_data_read) { + if (rr->length > kMaxEarlyDataAccepted - ssl->s3->hs->early_data_read) { + OPENSSL_PUT_ERROR(SSL, SSL_R_TOO_MUCH_READ_EARLY_DATA); + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL3_AD_UNEXPECTED_MESSAGE); + return -1; + } + + ssl->s3->hs->early_data_read += rr->length; + } + if (rr->length != 0) { return consume_record(ssl, buf, len, peek); }
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index b795d0a..cdef60c 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go
@@ -3633,6 +3633,28 @@ "-expect-accept-early-data", }, }) + + tests = append(tests, testCase{ + testType: serverTest, + name: "TLS13-MaxEarlyData-Server", + config: Config{ + MaxVersion: VersionTLS13, + MinVersion: VersionTLS13, + Bugs: ProtocolBugs{ + SendEarlyData: [][]byte{bytes.Repeat([]byte{1}, + 14336 + 1)}, + ExpectEarlyDataAccepted: true, + }, + }, + messageCount: 2, + resumeSession: true, + flags: []string{ + "-enable-early-data", + "-expect-accept-early-data", + }, + shouldFail: true, + expectedError: ":TOO_MUCH_READ_EARLY_DATA:", + }) } // TLS client auth.
diff --git a/ssl/tls13_server.c b/ssl/tls13_server.c index af33458..9e8513c 100644 --- a/ssl/tls13_server.c +++ b/ssl/tls13_server.c
@@ -29,11 +29,6 @@ #include "internal.h" -/* kMaxEarlyDataAccepted is the advertised number of plaintext bytes of early - * data that will be accepted. This value should be slightly below - * kMaxEarlyDataSkipped in tls_record.c, which is measured in ciphertext. */ -static const size_t kMaxEarlyDataAccepted = 14336; - enum server_hs_state_t { state_select_parameters = 0, state_select_session,
diff --git a/ssl/tls_record.c b/ssl/tls_record.c index 0f9720c..e67e0b4 100644 --- a/ssl/tls_record.c +++ b/ssl/tls_record.c
@@ -128,8 +128,8 @@ /* kMaxEarlyDataSkipped is the maximum number of rejected early data bytes that * will be skipped. Without this limit an attacker could send records at a * faster rate than we can process and cause trial decryption to loop forever. - * This value should be slightly above kMaxEarlyDataAccepted in tls13_server.c, - * which is measured in plaintext. */ + * This value should be slightly above kMaxEarlyDataAccepted, which is measured + * in plaintext. */ static const size_t kMaxEarlyDataSkipped = 16384; /* kMaxWarningAlerts is the number of consecutive warning alerts that will be