diff options
author | Dr. Stephen Henson <steve@openssl.org> | 2016-06-18 15:46:13 +0100 |
---|---|---|
committer | Dr. Stephen Henson <steve@openssl.org> | 2016-07-15 14:09:05 +0100 |
commit | d166ed8c11e10e9fdaeac182effb9dd318843924 (patch) | |
tree | fd47ffb1f5d42b121b04d14c1a8f6bdc659637f6 /ssl | |
parent | 1fc431ba57d12189a9bdacd3999ea2a7b91458d8 (diff) |
check return values for EVP_Digest*() APIs
Reviewed-by: Richard Levitte <levitte@openssl.org>
Diffstat (limited to 'ssl')
-rw-r--r-- | ssl/record/ssl3_record.c | 9 | ||||
-rw-r--r-- | ssl/s3_cbc.c | 8 | ||||
-rw-r--r-- | ssl/s3_enc.c | 40 | ||||
-rw-r--r-- | ssl/ssl_locl.h | 8 | ||||
-rw-r--r-- | ssl/statem/statem_clnt.c | 9 | ||||
-rw-r--r-- | ssl/statem/statem_dtls.c | 6 | ||||
-rw-r--r-- | ssl/statem/statem_lib.c | 23 |
7 files changed, 64 insertions, 39 deletions
diff --git a/ssl/record/ssl3_record.c b/ssl/record/ssl3_record.c index d3b2bea40a..3630cd900a 100644 --- a/ssl/record/ssl3_record.c +++ b/ssl/record/ssl3_record.c @@ -1015,9 +1015,12 @@ int tls1_mac(SSL *ssl, SSL3_RECORD *rec, unsigned char *md, int send) return -1; } if (!send && !SSL_USE_ETM(ssl) && FIPS_mode()) - tls_fips_digest_extra(ssl->enc_read_ctx, - mac_ctx, rec->input, - rec->length, rec->orig_len); + if (!tls_fips_digest_extra(ssl->enc_read_ctx, + mac_ctx, rec->input, + rec->length, rec->orig_len)) { + EVP_MD_CTX_free(hmac); + return -1; + } } EVP_MD_CTX_free(hmac); diff --git a/ssl/s3_cbc.c b/ssl/s3_cbc.c index 7cdabbb761..febd88a117 100644 --- a/ssl/s3_cbc.c +++ b/ssl/s3_cbc.c @@ -490,13 +490,13 @@ err: * digesting additional data. */ -void tls_fips_digest_extra(const EVP_CIPHER_CTX *cipher_ctx, +int tls_fips_digest_extra(const EVP_CIPHER_CTX *cipher_ctx, EVP_MD_CTX *mac_ctx, const unsigned char *data, size_t data_len, size_t orig_len) { size_t block_size, digest_pad, blocks_data, blocks_orig; if (EVP_CIPHER_CTX_mode(cipher_ctx) != EVP_CIPH_CBC_MODE) - return; + return 1; block_size = EVP_MD_CTX_block_size(mac_ctx); /*- * We are in FIPS mode if we get this far so we know we have only SHA* @@ -526,6 +526,6 @@ void tls_fips_digest_extra(const EVP_CIPHER_CTX *cipher_ctx, * The "data" pointer should always have enough space to perform this * operation as it is large enough for a maximum length TLS buffer. */ - EVP_DigestSignUpdate(mac_ctx, data, - (blocks_orig - blocks_data + 1) * block_size); + return EVP_DigestSignUpdate(mac_ctx, data, + (blocks_orig - blocks_data + 1) * block_size); } diff --git a/ssl/s3_enc.c b/ssl/s3_enc.c index b6de38d90b..3240735fab 100644 --- a/ssl/s3_enc.c +++ b/ssl/s3_enc.c @@ -70,23 +70,26 @@ static int ssl3_generate_key_block(SSL *s, unsigned char *km, int num) for (j = 0; j < k; j++) buf[j] = c; c++; - EVP_DigestInit_ex(s1, EVP_sha1(), NULL); - EVP_DigestUpdate(s1, buf, k); - EVP_DigestUpdate(s1, s->session->master_key, - s->session->master_key_length); - EVP_DigestUpdate(s1, s->s3->server_random, SSL3_RANDOM_SIZE); - EVP_DigestUpdate(s1, s->s3->client_random, SSL3_RANDOM_SIZE); - EVP_DigestFinal_ex(s1, smd, NULL); - - EVP_DigestInit_ex(m5, EVP_md5(), NULL); - EVP_DigestUpdate(m5, s->session->master_key, - s->session->master_key_length); - EVP_DigestUpdate(m5, smd, SHA_DIGEST_LENGTH); + if (!EVP_DigestInit_ex(s1, EVP_sha1(), NULL) + || !EVP_DigestUpdate(s1, buf, k) + || !EVP_DigestUpdate(s1, s->session->master_key, + s->session->master_key_length) + || !EVP_DigestUpdate(s1, s->s3->server_random, SSL3_RANDOM_SIZE) + || !EVP_DigestUpdate(s1, s->s3->client_random, SSL3_RANDOM_SIZE) + || !EVP_DigestFinal_ex(s1, smd, NULL) + || !EVP_DigestInit_ex(m5, EVP_md5(), NULL) + || !EVP_DigestUpdate(m5, s->session->master_key, + s->session->master_key_length) + || !EVP_DigestUpdate(m5, smd, SHA_DIGEST_LENGTH)) + goto err; if ((int)(i + MD5_DIGEST_LENGTH) > num) { - EVP_DigestFinal_ex(m5, smd, NULL); + if (!EVP_DigestFinal_ex(m5, smd, NULL)) + goto err; memcpy(km, smd, (num - i)); - } else - EVP_DigestFinal_ex(m5, km, NULL); + } else { + if (!EVP_DigestFinal_ex(m5, km, NULL)) + goto err; + } km += MD5_DIGEST_LENGTH; } @@ -353,12 +356,13 @@ void ssl3_free_digest_list(SSL *s) s->s3->handshake_dgst = NULL; } -void ssl3_finish_mac(SSL *s, const unsigned char *buf, int len) +int ssl3_finish_mac(SSL *s, const unsigned char *buf, int len) { if (s->s3->handshake_dgst == NULL) - BIO_write(s->s3->handshake_buffer, (void *)buf, len); + /* Note: this writes to a memory BIO so a failure is a fatal error */ + return BIO_write(s->s3->handshake_buffer, (void *)buf, len) == len; else - EVP_DigestUpdate(s->s3->handshake_dgst, buf, len); + return EVP_DigestUpdate(s->s3->handshake_dgst, buf, len); } int ssl3_digest_cached_records(SSL *s, int keep) diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 35fd3fc7ac..8287077bba 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -1875,7 +1875,7 @@ int ssl3_renegotiate_check(SSL *ssl); __owur int ssl3_dispatch_alert(SSL *s); __owur int ssl3_final_finish_mac(SSL *s, const char *sender, int slen, unsigned char *p); -void ssl3_finish_mac(SSL *s, const unsigned char *buf, int len); +__owur int ssl3_finish_mac(SSL *s, const unsigned char *buf, int len); void ssl3_free_digest_list(SSL *s); __owur unsigned long ssl3_output_cert_chain(SSL *s, CERT_PKEY *cpk); __owur const SSL_CIPHER *ssl3_choose_cipher(SSL *ssl, @@ -2085,9 +2085,9 @@ __owur int ssl3_cbc_digest_record(const EVP_MD_CTX *ctx, const unsigned char *mac_secret, unsigned mac_secret_length, char is_sslv3); -void tls_fips_digest_extra(const EVP_CIPHER_CTX *cipher_ctx, - EVP_MD_CTX *mac_ctx, const unsigned char *data, - size_t data_len, size_t orig_len); +__owur int tls_fips_digest_extra(const EVP_CIPHER_CTX *cipher_ctx, + EVP_MD_CTX *mac_ctx, const unsigned char *data, + size_t data_len, size_t orig_len); __owur int srp_generate_server_master_secret(SSL *s); __owur int srp_generate_client_master_secret(SSL *s); diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index be4ba9cead..5eefa2aa74 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -1890,9 +1890,12 @@ MSG_PROCESS_RETURN tls_process_new_session_ticket(SSL *s, PACKET *pkt) * elsewhere in OpenSSL. The session ID is set to the SHA256 (or SHA1 is * SHA256 is disabled) hash of the ticket. */ - EVP_Digest(s->session->tlsext_tick, ticklen, - s->session->session_id, &s->session->session_id_length, - EVP_sha256(), NULL); + if (!EVP_Digest(s->session->tlsext_tick, ticklen, + s->session->session_id, &s->session->session_id_length, + EVP_sha256(), NULL)) { + SSLerr(SSL_F_TLS_PROCESS_NEW_SESSION_TICKET, ERR_R_EVP_LIB); + goto err; + } return MSG_PROCESS_CONTINUE_READING; f_err: ssl3_send_alert(s, SSL3_AL_FATAL, al); diff --git a/ssl/statem/statem_dtls.c b/ssl/statem/statem_dtls.c index 946dee0841..3979bf3835 100644 --- a/ssl/statem/statem_dtls.c +++ b/ssl/statem/statem_dtls.c @@ -294,7 +294,8 @@ int dtls1_do_write(SSL *s, int type) xlen = ret - DTLS1_HM_HEADER_LENGTH; } - ssl3_finish_mac(s, p, xlen); + if (!ssl3_finish_mac(s, p, xlen)) + return -1; } if (ret == s->init_num) { @@ -375,7 +376,8 @@ int dtls_get_message(SSL *s, int *mt, unsigned long *len) msg_len += DTLS1_HM_HEADER_LENGTH; } - ssl3_finish_mac(s, p, msg_len); + if (!ssl3_finish_mac(s, p, msg_len)) + return 0; if (s->msg_callback) s->msg_callback(0, s->version, SSL3_RT_HANDSHAKE, p, msg_len, s, s->msg_callback_arg); diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c index aab4e76aa8..258b897a52 100644 --- a/ssl/statem/statem_lib.c +++ b/ssl/statem/statem_lib.c @@ -40,8 +40,10 @@ int ssl3_do_write(SSL *s, int type) * should not be done for 'Hello Request's, but in that case we'll * ignore the result anyway */ - ssl3_finish_mac(s, (unsigned char *)&s->init_buf->data[s->init_off], - ret); + if (!ssl3_finish_mac(s, + (unsigned char *)&s->init_buf->data[s->init_off], + ret)) + return -1; if (ret == s->init_num) { if (s->msg_callback) @@ -481,13 +483,24 @@ int tls_get_message_body(SSL *s, unsigned long *len) /* Feed this message into MAC computation. */ if(RECORD_LAYER_is_sslv2_record(&s->rlayer)) { - ssl3_finish_mac(s, (unsigned char *)s->init_buf->data, s->init_num); + if (!ssl3_finish_mac(s, (unsigned char *)s->init_buf->data, + s->init_num)) { + SSLerr(SSL_F_TLS_GET_MESSAGE_BODY, ERR_R_EVP_LIB); + ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); + *len = 0; + return 0; + } if (s->msg_callback) s->msg_callback(0, SSL2_VERSION, 0, s->init_buf->data, (size_t)s->init_num, s, s->msg_callback_arg); } else { - ssl3_finish_mac(s, (unsigned char *)s->init_buf->data, - s->init_num + SSL3_HM_HEADER_LENGTH); + if (!ssl3_finish_mac(s, (unsigned char *)s->init_buf->data, + s->init_num + SSL3_HM_HEADER_LENGTH)) { + SSLerr(SSL_F_TLS_GET_MESSAGE_BODY, ERR_R_EVP_LIB); + ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); + *len = 0; + return 0; + } if (s->msg_callback) s->msg_callback(0, s->version, SSL3_RT_HANDSHAKE, s->init_buf->data, (size_t)s->init_num + SSL3_HM_HEADER_LENGTH, s, |