Simplify handshake message size limits.
A handshake message can go up to 2^24 bytes = 16MB which is a little large for
the peer to force us to buffer. Accordingly, we bound the size of a
handshake message.
Rather than have a global limit, the existing logic uses a different limit at
each state in the handshake state machine and, for certificates, allows
configuring the maximum certificate size. This is nice in that we engage larger
limits iff the relevant state is reachable from the handshake. Servers without
client auth get a tighter limit "for free".
However, this doesn't work for DTLS due to out-of-order messages and we use a
simpler scheme for DTLS. This scheme also is tricky on optional messages and
makes the handshake <-> message layer communication complex.
Apart from an ignored 20,000 byte limit on ServerHello, the largest
non-certificate limit is the common 16k limit on ClientHello. So this
complexity wasn't buying us anything. Unify everything on the DTLS scheme
except, so as not to regress bounds on client-auth-less servers, also correctly
check for whether client auth is configured. The value of 16k was chosen based
on this value.
(The 20,000 byte ServerHello limit makes no sense. We can easily bound the
ServerHello because servers may not send extensions we don't implement. But it
gets overshadowed by the certificate anyway.)
Change-Id: I00309b16d809a3c2a1543f99fd29c4163e3add81
Reviewed-on: https://boringssl-review.googlesource.com/7941
Reviewed-by: David Benjamin <davidben@google.com>
diff --git a/ssl/d1_both.c b/ssl/d1_both.c
index 9f60db4..7a62412 100644
--- a/ssl/d1_both.c
+++ b/ssl/d1_both.c
@@ -483,17 +483,6 @@
return frag;
}
-/* dtls1_max_handshake_message_len returns the maximum number of bytes
- * permitted in a DTLS handshake message for |ssl|. The minimum is 16KB, but may
- * be greater if the maximum certificate list size requires it. */
-static size_t dtls1_max_handshake_message_len(const SSL *ssl) {
- size_t max_len = DTLS1_HM_HEADER_LENGTH + SSL3_RT_MAX_ENCRYPTED_LENGTH;
- if (max_len < ssl->max_cert_list) {
- return ssl->max_cert_list;
- }
- return max_len;
-}
-
/* dtls1_process_fragment reads a handshake fragment and processes it. It
* returns one if a fragment was successfully processed and 0 or -1 on error. */
static int dtls1_process_fragment(SSL *ssl) {
@@ -521,7 +510,7 @@
const size_t msg_len = msg_hdr.msg_len;
if (frag_off > msg_len || frag_off + frag_len < frag_off ||
frag_off + frag_len > msg_len ||
- msg_len > dtls1_max_handshake_message_len(ssl) ||
+ msg_len > ssl_max_handshake_message_len(ssl) ||
frag_len > ssl->s3->rrec.length) {
OPENSSL_PUT_ERROR(SSL, SSL_R_EXCESSIVE_MESSAGE_SIZE);
ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
@@ -567,9 +556,9 @@
}
/* 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 *ssl, int st1, int stn, int msg_type, long max,
+ * if |msg_type| == -1). Read an entire handshake message. Handshake messages
+ * arrive in fragments. */
+long dtls1_get_message(SSL *ssl, int st1, int stn, int msg_type,
enum ssl_hash_message_t hash_message, int *ok) {
pitem *item = NULL;
hm_fragment *frag = NULL;
@@ -610,11 +599,6 @@
assert(ssl->d1->handshake_read_seq == frag->msg_header.seq);
assert(frag->reassembly == NULL);
- if (frag->msg_header.msg_len > (size_t)max) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_EXCESSIVE_MESSAGE_SIZE);
- goto err;
- }
-
/* Reconstruct the assembled message. */
size_t len;
CBB cbb;
diff --git a/ssl/d1_clnt.c b/ssl/d1_clnt.c
index e11c55b..8a93100 100644
--- a/ssl/d1_clnt.c
+++ b/ssl/d1_clnt.c
@@ -503,11 +503,9 @@
CBS hello_verify_request, cookie;
uint16_t server_version;
- n = ssl->method->ssl_get_message(
- ssl, DTLS1_ST_CR_HELLO_VERIFY_REQUEST_A,
- DTLS1_ST_CR_HELLO_VERIFY_REQUEST_B, -1,
- /* Use the same maximum size as ssl3_get_server_hello. */
- 20000, ssl_hash_message, &ok);
+ n = ssl->method->ssl_get_message(ssl, DTLS1_ST_CR_HELLO_VERIFY_REQUEST_A,
+ DTLS1_ST_CR_HELLO_VERIFY_REQUEST_B, -1,
+ ssl_hash_message, &ok);
if (!ok) {
return n;
diff --git a/ssl/internal.h b/ssl/internal.h
index 0e470b1..ddd95b1 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -582,6 +582,13 @@
const uint8_t *peer_key, size_t peer_key_len);
+/* Handshake messages. */
+
+/* ssl_max_handshake_message_len returns the maximum number of bytes permitted
+ * in a handshake message for |ssl|. */
+size_t ssl_max_handshake_message_len(const SSL *ssl);
+
+
/* Transport buffers. */
/* ssl_read_buffer returns a pointer to contents of the read buffer. */
@@ -829,8 +836,8 @@
int (*ssl_accept)(SSL *ssl);
int (*ssl_connect)(SSL *ssl);
long (*ssl_get_message)(SSL *ssl, int header_state, int body_state,
- int msg_type, long max,
- enum ssl_hash_message_t hash_message, int *ok);
+ int msg_type, enum ssl_hash_message_t hash_message,
+ int *ok);
int (*ssl_read_app_data)(SSL *ssl, uint8_t *buf, int len, int peek);
int (*ssl_read_change_cipher_spec)(SSL *ssl);
void (*ssl_read_close_notify)(SSL *ssl);
@@ -1026,7 +1033,7 @@
int ssl3_send_alert(SSL *ssl, int level, int desc);
int ssl3_get_req_cert_type(SSL *ssl, uint8_t *p);
long ssl3_get_message(SSL *ssl, int header_state, int body_state, int msg_type,
- long max, enum ssl_hash_message_t hash_message, int *ok);
+ enum ssl_hash_message_t hash_message, int *ok);
/* ssl3_hash_current_message incorporates the current handshake message into the
* handshake hash. It returns one on success and zero on allocation failure. */
@@ -1131,7 +1138,7 @@
int dtls1_connect(SSL *ssl);
void dtls1_free(SSL *ssl);
-long dtls1_get_message(SSL *ssl, int st1, int stn, int mt, long max,
+long dtls1_get_message(SSL *ssl, int st1, int stn, int mt,
enum ssl_hash_message_t hash_message, int *ok);
int dtls1_dispatch_alert(SSL *ssl);
diff --git a/ssl/s3_both.c b/ssl/s3_both.c
index 5d364ab..5235b97 100644
--- a/ssl/s3_both.c
+++ b/ssl/s3_both.c
@@ -216,8 +216,8 @@
long message_len;
uint8_t *p;
- message_len = ssl->method->ssl_get_message(
- ssl, a, b, SSL3_MT_FINISHED, EVP_MAX_MD_SIZE, ssl_dont_hash_message, &ok);
+ message_len = ssl->method->ssl_get_message(ssl, a, b, SSL3_MT_FINISHED,
+ ssl_dont_hash_message, &ok);
if (!ok) {
return message_len;
@@ -298,12 +298,23 @@
return ssl_set_handshake_header(ssl, SSL3_MT_CERTIFICATE, l);
}
-/* Obtain handshake message of message type |msg_type| (any if |msg_type| == -1),
- * maximum acceptable body length |max|. The first four bytes (msg_type and
- * length) are read in state |header_state|, the body is read in state
- * |body_state|. */
+size_t ssl_max_handshake_message_len(const SSL *ssl) {
+ /* kMaxMessageLen is the default maximum message size for handshakes which do
+ * not accept peer certificate chains. */
+ static const size_t kMaxMessageLen = 16384;
+
+ if ((!ssl->server || (ssl->verify_mode & SSL_VERIFY_PEER)) &&
+ kMaxMessageLen < ssl->max_cert_list) {
+ return ssl->max_cert_list;
+ }
+ return kMaxMessageLen;
+}
+
+/* Obtain handshake message of message type |msg_type| (any if |msg_type| ==
+ * -1). The first four bytes (msg_type and length) are read in state
+ * |header_state|, the body is read in state |body_state|. */
long ssl3_get_message(SSL *ssl, int header_state, int body_state, int msg_type,
- long max, enum ssl_hash_message_t hash_message, int *ok) {
+ enum ssl_hash_message_t hash_message, int *ok) {
uint8_t *p;
unsigned long l;
long n;
@@ -369,7 +380,7 @@
ssl->s3->tmp.message_type = *(p++);
n2l3(p, l);
- if (l > (unsigned long)max) {
+ if (l > ssl_max_handshake_message_len(ssl)) {
al = SSL_AD_ILLEGAL_PARAMETER;
OPENSSL_PUT_ERROR(SSL, SSL_R_EXCESSIVE_MESSAGE_SIZE);
goto f_err;
diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index 8655ad0..3681590 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -726,9 +726,8 @@
uint8_t compression_method;
n = ssl->method->ssl_get_message(ssl, SSL3_ST_CR_SRVR_HELLO_A,
- SSL3_ST_CR_SRVR_HELLO_B, SSL3_MT_SERVER_HELLO,
- 20000, /* ?? */
- ssl_hash_message, &ok);
+ SSL3_ST_CR_SRVR_HELLO_B,
+ SSL3_MT_SERVER_HELLO, ssl_hash_message, &ok);
if (!ok) {
uint32_t err = ERR_peek_error();
@@ -956,8 +955,7 @@
const uint8_t *data;
n = ssl->method->ssl_get_message(ssl, SSL3_ST_CR_CERT_A, SSL3_ST_CR_CERT_B,
- SSL3_MT_CERTIFICATE, (long)ssl->max_cert_list,
- ssl_hash_message, &ok);
+ SSL3_MT_CERTIFICATE, ssl_hash_message, &ok);
if (!ok) {
return n;
@@ -1045,11 +1043,9 @@
EC_KEY *ecdh = NULL;
EC_POINT *srvr_ecpoint = NULL;
- /* use same message size as in ssl3_get_certificate_request() as
- * ServerKeyExchange message may be skipped */
- long n = ssl->method->ssl_get_message(
- ssl, SSL3_ST_CR_KEY_EXCH_A, SSL3_ST_CR_KEY_EXCH_B, -1, ssl->max_cert_list,
- ssl_hash_message, &ok);
+ long n = ssl->method->ssl_get_message(ssl, SSL3_ST_CR_KEY_EXCH_A,
+ SSL3_ST_CR_KEY_EXCH_B, -1,
+ ssl_hash_message, &ok);
if (!ok) {
return n;
}
@@ -1294,9 +1290,9 @@
X509_NAME *xn = NULL;
STACK_OF(X509_NAME) *ca_sk = NULL;
- long n = ssl->method->ssl_get_message(
- ssl, SSL3_ST_CR_CERT_REQ_A, SSL3_ST_CR_CERT_REQ_B, -1, ssl->max_cert_list,
- ssl_hash_message, &ok);
+ long n = ssl->method->ssl_get_message(ssl, SSL3_ST_CR_CERT_REQ_A,
+ SSL3_ST_CR_CERT_REQ_B, -1,
+ ssl_hash_message, &ok);
if (!ok) {
return n;
@@ -1403,7 +1399,7 @@
int ok, al;
long n = ssl->method->ssl_get_message(
ssl, SSL3_ST_CR_SESSION_TICKET_A, SSL3_ST_CR_SESSION_TICKET_B,
- SSL3_MT_NEWSESSION_TICKET, 16384, ssl_hash_message, &ok);
+ SSL3_MT_NEWSESSION_TICKET, ssl_hash_message, &ok);
if (!ok) {
return n;
@@ -1479,9 +1475,9 @@
CBS certificate_status, ocsp_response;
uint8_t status_type;
- n = ssl->method->ssl_get_message(
- ssl, SSL3_ST_CR_CERT_STATUS_A, SSL3_ST_CR_CERT_STATUS_B,
- -1, 16384, ssl_hash_message, &ok);
+ n = ssl->method->ssl_get_message(ssl, SSL3_ST_CR_CERT_STATUS_A,
+ SSL3_ST_CR_CERT_STATUS_B, -1,
+ ssl_hash_message, &ok);
if (!ok) {
return n;
@@ -1523,9 +1519,8 @@
long n;
n = ssl->method->ssl_get_message(ssl, SSL3_ST_CR_SRVR_DONE_A,
- SSL3_ST_CR_SRVR_DONE_B, SSL3_MT_SERVER_DONE,
- 30, /* should be very small, like 0 :-) */
- ssl_hash_message, &ok);
+ SSL3_ST_CR_SRVR_DONE_B, SSL3_MT_SERVER_DONE,
+ ssl_hash_message, &ok);
if (!ok) {
return n;
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index 037bc96..9fec49c 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -769,8 +769,7 @@
case SSL3_ST_SR_CLNT_HELLO_B:
n = ssl->method->ssl_get_message(
ssl, SSL3_ST_SR_CLNT_HELLO_A, SSL3_ST_SR_CLNT_HELLO_B,
- SSL3_MT_CLIENT_HELLO, SSL3_RT_MAX_PLAIN_LENGTH,
- ssl_hash_message, &ok);
+ SSL3_MT_CLIENT_HELLO, ssl_hash_message, &ok);
if (!ok) {
return n;
@@ -1453,7 +1452,7 @@
int ok;
const long n = ssl->method->ssl_get_message(
ssl, SSL3_ST_SR_KEY_EXCH_A, SSL3_ST_SR_KEY_EXCH_B,
- SSL3_MT_CLIENT_KEY_EXCHANGE, 2048 /* ??? */, ssl_hash_message, &ok);
+ SSL3_MT_CLIENT_KEY_EXCHANGE, ssl_hash_message, &ok);
if (!ok) {
return n;
}
@@ -1735,8 +1734,7 @@
n = ssl->method->ssl_get_message(
ssl, SSL3_ST_SR_CERT_VRFY_A, SSL3_ST_SR_CERT_VRFY_B,
- SSL3_MT_CERTIFICATE_VERIFY, SSL3_RT_MAX_PLAIN_LENGTH,
- ssl_dont_hash_message, &ok);
+ SSL3_MT_CERTIFICATE_VERIFY, ssl_dont_hash_message, &ok);
if (!ok) {
return n;
@@ -1833,8 +1831,7 @@
assert(ssl->s3->tmp.cert_request);
n = ssl->method->ssl_get_message(ssl, SSL3_ST_SR_CERT_A, SSL3_ST_SR_CERT_B,
- -1, (long)ssl->max_cert_list,
- ssl_hash_message, &ok);
+ -1, ssl_hash_message, &ok);
if (!ok) {
return n;
@@ -2122,9 +2119,8 @@
}
n = ssl->method->ssl_get_message(ssl, SSL3_ST_SR_NEXT_PROTO_A,
- SSL3_ST_SR_NEXT_PROTO_B, SSL3_MT_NEXT_PROTO,
- 514, /* See the payload format below */
- ssl_hash_message, &ok);
+ SSL3_ST_SR_NEXT_PROTO_B, SSL3_MT_NEXT_PROTO,
+ ssl_hash_message, &ok);
if (!ok) {
return n;
@@ -2165,8 +2161,7 @@
n = ssl->method->ssl_get_message(
ssl, SSL3_ST_SR_CHANNEL_ID_A, SSL3_ST_SR_CHANNEL_ID_B,
- SSL3_MT_ENCRYPTED_EXTENSIONS, 2 + 2 + TLSEXT_CHANNEL_ID_SIZE,
- ssl_dont_hash_message, &ok);
+ SSL3_MT_ENCRYPTED_EXTENSIONS, ssl_dont_hash_message, &ok);
if (!ok) {
return n;
