Handle failures in ssl3_finish_mac.
It may fail because the BIO_write to the memory BIO can allocate.
Unfortunately, this bubbles up pretty far up now that we've moved the handshake
hash to ssl3_set_handshake_header.
Change-Id: I58884347a4456bb974ac4783078131522167e29d
Reviewed-on: https://boringssl-review.googlesource.com/3483
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/d1_both.c b/ssl/d1_both.c
index 72dadc6..7891ed7 100644
--- a/ssl/d1_both.c
+++ b/ssl/d1_both.c
@@ -415,8 +415,9 @@
s->init_msg = (uint8_t *)s->init_buf->data + DTLS1_HM_HEADER_LENGTH;
- if (hash_message != SSL_GET_MESSAGE_DONT_HASH_MESSAGE) {
- ssl3_hash_current_message(s);
+ if (hash_message != SSL_GET_MESSAGE_DONT_HASH_MESSAGE &&
+ !ssl3_hash_current_message(s)) {
+ goto err;
}
if (s->msg_callback) {
s->msg_callback(0, s->version, SSL3_RT_HANDSHAKE, p, msg_len, s,
@@ -431,6 +432,7 @@
f_err:
ssl3_send_alert(s, SSL3_AL_FATAL, al);
+err:
*ok = 0;
return -1;
}
diff --git a/ssl/d1_lib.c b/ssl/d1_lib.c
index dcf86e5..6d2f77f 100644
--- a/ssl/d1_lib.c
+++ b/ssl/d1_lib.c
@@ -82,7 +82,7 @@
static void get_current_time(SSL *ssl, OPENSSL_timeval *out_clock);
static OPENSSL_timeval *dtls1_get_timeout(SSL *s, OPENSSL_timeval *timeleft);
-static void dtls1_set_handshake_header(SSL *s, int type, unsigned long len);
+static int dtls1_set_handshake_header(SSL *s, int type, unsigned long len);
static int dtls1_handshake_write(SSL *s);
const SSL3_ENC_METHOD DTLSv1_enc_data = {
@@ -415,7 +415,7 @@
#endif
}
-static void dtls1_set_handshake_header(SSL *s, int htype, unsigned long len) {
+static int dtls1_set_handshake_header(SSL *s, int htype, unsigned long len) {
uint8_t *message = (uint8_t *)s->init_buf->data;
const struct hm_header_st *msg_hdr = &s->d1->w_msg_hdr;
uint8_t serialised_header[DTLS1_HM_HEADER_LENGTH];
@@ -438,8 +438,8 @@
s2n(msg_hdr->seq, p);
l2n3(0, p);
l2n3(msg_hdr->msg_len, p);
- ssl3_finish_mac(s, serialised_header, sizeof(serialised_header));
- ssl3_finish_mac(s, message + DTLS1_HM_HEADER_LENGTH, len);
+ return ssl3_finish_mac(s, serialised_header, sizeof(serialised_header)) &&
+ ssl3_finish_mac(s, message + DTLS1_HM_HEADER_LENGTH, len);
}
static int dtls1_handshake_write(SSL *s) {
diff --git a/ssl/s3_both.c b/ssl/s3_both.c
index ddfc1fe..a32be5f 100644
--- a/ssl/s3_both.c
+++ b/ssl/s3_both.c
@@ -185,7 +185,9 @@
s->s3->previous_client_finished_len = n;
}
- ssl_set_handshake_header(s, SSL3_MT_FINISHED, n);
+ if (!ssl_set_handshake_header(s, SSL3_MT_FINISHED, n)) {
+ return 0;
+ }
s->state = b;
}
@@ -232,7 +234,9 @@
/* Snapshot the finished hash before incorporating the new message. */
ssl3_take_mac(s);
- ssl3_hash_current_message(s);
+ if (!ssl3_hash_current_message(s)) {
+ goto err;
+ }
/* If this occurs, we have missed a message.
* TODO(davidben): Is this check now redundant with SSL3_FLAGS_EXPECT_CCS? */
@@ -273,6 +277,7 @@
f_err:
ssl3_send_alert(s, SSL3_AL_FATAL, al);
+err:
return 0;
}
@@ -308,8 +313,7 @@
p = ssl_handshake_start(s);
l2n3(l, p);
l += 3;
- ssl_set_handshake_header(s, SSL3_MT_CERTIFICATE, l);
- return 1;
+ return ssl_set_handshake_header(s, SSL3_MT_CERTIFICATE, l);
}
/* Obtain handshake message of message type |msg_type| (any if |msg_type| == -1),
@@ -416,8 +420,9 @@
}
/* Feed this message into MAC computation. */
- if (hash_message != SSL_GET_MESSAGE_DONT_HASH_MESSAGE) {
- ssl3_hash_current_message(s);
+ if (hash_message != SSL_GET_MESSAGE_DONT_HASH_MESSAGE &&
+ !ssl3_hash_current_message(s)) {
+ goto err;
}
if (s->msg_callback) {
s->msg_callback(0, s->version, SSL3_RT_HANDSHAKE, s->init_buf->data,
@@ -434,11 +439,12 @@
return -1;
}
-void ssl3_hash_current_message(SSL *s) {
+int ssl3_hash_current_message(SSL *s) {
/* The handshake header (different size between DTLS and TLS) is included in
* the hash. */
size_t header_len = s->init_msg - (uint8_t *)s->init_buf->data;
- ssl3_finish_mac(s, (uint8_t *)s->init_buf->data, s->init_num + header_len);
+ return ssl3_finish_mac(s, (uint8_t *)s->init_buf->data,
+ s->init_num + header_len);
}
/* ssl3_cert_verify_hash is documented as needing EVP_MAX_MD_SIZE because that
diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index c6c2b42..f70cfcb 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -724,7 +724,9 @@
}
l = p - d;
- ssl_set_handshake_header(s, SSL3_MT_CLIENT_HELLO, l);
+ if (!ssl_set_handshake_header(s, SSL3_MT_CLIENT_HELLO, l)) {
+ goto err;
+ }
s->state = SSL3_ST_CW_CLNT_HELLO_B;
}
@@ -2001,7 +2003,9 @@
/* The message must be added to the finished hash before calculating the
* master secret. */
- ssl_set_handshake_header(s, SSL3_MT_CLIENT_KEY_EXCHANGE, n);
+ if (!ssl_set_handshake_header(s, SSL3_MT_CLIENT_KEY_EXCHANGE, n)) {
+ goto err;
+ }
s->state = SSL3_ST_CW_KEY_EXCH_B;
s->session->master_key_length = s->enc_method->generate_master_secret(
@@ -2097,7 +2101,9 @@
s2n(signature_length, p);
n += signature_length + 2;
- ssl_set_handshake_header(s, SSL3_MT_CERTIFICATE_VERIFY, n);
+ if (!ssl_set_handshake_header(s, SSL3_MT_CERTIFICATE_VERIFY, n)) {
+ goto err;
+ }
s->state = SSL3_ST_CW_CERT_VRFY_B;
}
@@ -2286,7 +2292,9 @@
memset(p, 0, padding_len);
p += padding_len;
- ssl_set_handshake_header(s, SSL3_MT_NEXT_PROTO, p - d);
+ if (!ssl_set_handshake_header(s, SSL3_MT_NEXT_PROTO, p - d)) {
+ return -1;
+ }
s->state = SSL3_ST_CW_NEXT_PROTO_B;
}
@@ -2397,8 +2405,10 @@
goto err;
}
- ssl_set_handshake_header(s, SSL3_MT_ENCRYPTED_EXTENSIONS,
- 2 + 2 + TLSEXT_CHANNEL_ID_SIZE);
+ if (!ssl_set_handshake_header(s, SSL3_MT_ENCRYPTED_EXTENSIONS,
+ 2 + 2 + TLSEXT_CHANNEL_ID_SIZE)) {
+ goto err;
+ }
s->state = SSL3_ST_CW_CHANNEL_ID_B;
ret = ssl_do_write(s);
diff --git a/ssl/s3_enc.c b/ssl/s3_enc.c
index 9630843..7bf325a 100644
--- a/ssl/s3_enc.c
+++ b/ssl/s3_enc.c
@@ -264,12 +264,11 @@
s->s3->handshake_dgst = NULL;
}
-void ssl3_finish_mac(SSL *s, const uint8_t *buf, int len) {
+int ssl3_finish_mac(SSL *s, const uint8_t *buf, int len) {
int i;
if (s->s3->handshake_buffer) {
- BIO_write(s->s3->handshake_buffer, (void *)buf, len);
- return;
+ return BIO_write(s->s3->handshake_buffer, (void *)buf, len) >= 0;
}
for (i = 0; i < SSL_MAX_DIGEST; i++) {
@@ -277,6 +276,7 @@
EVP_DigestUpdate(s->s3->handshake_dgst[i], buf, len);
}
}
+ return 1;
}
int ssl3_digest_cached_records(
diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index 8ed2be9..700fbaf 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -595,7 +595,7 @@
: 0;
}
-void ssl3_set_handshake_header(SSL *s, int htype, unsigned long len) {
+int ssl3_set_handshake_header(SSL *s, int htype, unsigned long len) {
uint8_t *p = (uint8_t *)s->init_buf->data;
*(p++) = htype;
l2n3(len, p);
@@ -603,7 +603,7 @@
s->init_off = 0;
/* Add the message to the handshake hash. */
- ssl3_finish_mac(s, (uint8_t *)s->init_buf->data, s->init_num);
+ return ssl3_finish_mac(s, (uint8_t *)s->init_buf->data, s->init_num);
}
int ssl3_handshake_write(SSL *s) { return ssl3_do_write(s, SSL3_RT_HANDSHAKE); }
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index c020b3a..4e60d63 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -813,7 +813,10 @@
/* The V2ClientHello without the length is incorporated into the Finished
* hash. */
- ssl3_finish_mac(s, CBS_data(&v2_client_hello), CBS_len(&v2_client_hello));
+ if (!ssl3_finish_mac(s, CBS_data(&v2_client_hello),
+ CBS_len(&v2_client_hello))) {
+ return -1;
+ }
if (s->msg_callback) {
s->msg_callback(0, SSL2_VERSION, 0, CBS_data(&v2_client_hello),
CBS_len(&v2_client_hello), s, s->msg_callback_arg);
@@ -912,7 +915,9 @@
int ssl3_send_hello_request(SSL *s) {
if (s->state == SSL3_ST_SW_HELLO_REQ_A) {
- ssl_set_handshake_header(s, SSL3_MT_HELLO_REQUEST, 0);
+ if (!ssl_set_handshake_header(s, SSL3_MT_HELLO_REQUEST, 0)) {
+ return -1;
+ }
s->state = SSL3_ST_SW_HELLO_REQ_B;
}
@@ -1299,7 +1304,9 @@
/* do the header */
l = (p - d);
- ssl_set_handshake_header(s, SSL3_MT_SERVER_HELLO, l);
+ if (!ssl_set_handshake_header(s, SSL3_MT_SERVER_HELLO, l)) {
+ return -1;
+ }
s->state = SSL3_ST_SW_SRVR_HELLO_B;
}
@@ -1309,7 +1316,9 @@
int ssl3_send_server_done(SSL *s) {
if (s->state == SSL3_ST_SW_SRVR_DONE_A) {
- ssl_set_handshake_header(s, SSL3_MT_SERVER_DONE, 0);
+ if (!ssl_set_handshake_header(s, SSL3_MT_SERVER_DONE, 0)) {
+ return -1;
+ }
s->state = SSL3_ST_SW_SRVR_DONE_B;
}
@@ -1612,7 +1621,9 @@
}
}
- ssl_set_handshake_header(s, SSL3_MT_SERVER_KEY_EXCHANGE, n);
+ if (!ssl_set_handshake_header(s, SSL3_MT_SERVER_KEY_EXCHANGE, n)) {
+ goto err;
+ }
}
s->state = SSL3_ST_SW_KEY_EXCH_B;
@@ -1685,7 +1696,9 @@
p = ssl_handshake_start(s) + off;
s2n(nl, p);
- ssl_set_handshake_header(s, SSL3_MT_CERTIFICATE_REQUEST, n);
+ if (!ssl_set_handshake_header(s, SSL3_MT_CERTIFICATE_REQUEST, n)) {
+ goto err;
+ }
s->state = SSL3_ST_SW_CERT_REQ_B;
}
@@ -2193,7 +2206,9 @@
!ssl3_digest_cached_records(s, free_handshake_buffer)) {
goto err;
}
- ssl3_hash_current_message(s);
+ if (!ssl3_hash_current_message(s)) {
+ goto err;
+ }
/* Parse and verify the signature. */
if (!CBS_get_u16_length_prefixed(&certificate_verify, &signature) ||
@@ -2466,7 +2481,9 @@
p += placeholder_len;
len = p - ssl_handshake_start(s);
- ssl_set_handshake_header(s, SSL3_MT_NEWSESSION_TICKET, len);
+ if (!ssl_set_handshake_header(s, SSL3_MT_NEWSESSION_TICKET, len)) {
+ return -1;
+ }
s->state = SSL3_ST_SW_SESSION_TICKET_B;
return ssl_do_write(s);
}
@@ -2537,7 +2554,9 @@
/* Skip ticket lifetime hint */
p = ssl_handshake_start(s) + 4;
s2n(len - 6, p);
- ssl_set_handshake_header(s, SSL3_MT_NEWSESSION_TICKET, len);
+ if (!ssl_set_handshake_header(s, SSL3_MT_NEWSESSION_TICKET, len)) {
+ return -1;
+ }
s->state = SSL3_ST_SW_SESSION_TICKET_B;
OPENSSL_free(session);
}
@@ -2638,7 +2657,9 @@
EVP_MD_CTX_cleanup(&md_ctx);
assert(channel_id_hash_len == SHA256_DIGEST_LENGTH);
- ssl3_hash_current_message(s);
+ if (!ssl3_hash_current_message(s)) {
+ return -1;
+ }
/* s->state doesn't reflect whether ChangeCipherSpec has been received in
* this handshake, but s->s3->change_cipher_spec does (will be reset by
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index c7abe79..9d77c91 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -583,7 +583,7 @@
/* Handshake header length */
unsigned int hhlen;
/* Set the handshake header */
- void (*set_handshake_header)(SSL *s, int type, unsigned long len);
+ int (*set_handshake_header)(SSL *s, int type, unsigned long len);
/* Write out handshake message */
int (*do_write)(SSL *s);
};
@@ -732,9 +732,9 @@
long ssl3_get_message(SSL *s, int header_state, int body_state, int msg_type,
long max, int hash_message, int *ok);
-/* ssl3_hash_current_message incorporates the current handshake message into
- * the handshake hash. */
-void ssl3_hash_current_message(SSL *s);
+/* ssl3_hash_current_message incorporates the current handshake message into the
+ * handshake hash. It returns one on success and zero on allocation failure. */
+int ssl3_hash_current_message(SSL *s);
/* ssl3_cert_verify_hash writes the CertificateVerify hash into the bytes
* pointed to by |out| and writes the number of bytes to |*out_len|. |out| must
@@ -756,7 +756,7 @@
int ssl3_write_bytes(SSL *s, int type, const void *buf, int len);
int ssl3_final_finish_mac(SSL *s, const char *sender, int slen, uint8_t *p);
int ssl3_cert_verify_mac(SSL *s, int md_nid, uint8_t *p);
-void ssl3_finish_mac(SSL *s, const uint8_t *buf, int len);
+int ssl3_finish_mac(SSL *s, const uint8_t *buf, int len);
void ssl3_free_digest_list(SSL *s);
int ssl3_output_cert_chain(SSL *s, CERT_PKEY *cpk);
const SSL_CIPHER *ssl3_choose_cipher(
@@ -791,7 +791,7 @@
void ssl3_record_sequence_update(uint8_t *seq);
int ssl3_do_change_cipher_spec(SSL *ssl);
-void ssl3_set_handshake_header(SSL *s, int htype, unsigned long len);
+int ssl3_set_handshake_header(SSL *s, int htype, unsigned long len);
int ssl3_handshake_write(SSL *s);
int dtls1_do_write(SSL *s, int type);
