From 5f3d93e4a336c590d7b56a889dde4a93b725e058 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Fri, 6 Nov 2015 16:31:21 +0000 Subject: [PATCH] Ensure all EVP calls have their returns checked where appropriate There are lots of calls to EVP functions from within libssl There were various places where we should probably check the return value but don't. This adds these checks. Reviewed-by: Richard Levitte --- include/openssl/ssl.h | 3 +- ssl/record/ssl3_record.c | 68 ++++++++++++++++++++--------------- ssl/s3_cbc.c | 45 +++++++++++++++-------- ssl/s3_enc.c | 63 ++++++++++++++++++-------------- ssl/ssl_ciph.c | 11 +++--- ssl/ssl_err.c | 2 ++ ssl/ssl_lib.c | 7 ++-- ssl/ssl_locl.h | 18 +++++----- ssl/ssl_rsa.c | 23 ++++++++++-- ssl/statem/statem_clnt.c | 78 ++++++++++++++++++++++++---------------- ssl/statem/statem_srvr.c | 60 +++++++++++++++++++------------ ssl/t1_enc.c | 21 ++++++++--- ssl/t1_lib.c | 28 +++++++++------ 13 files changed, 271 insertions(+), 156 deletions(-) diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index cf9f83a975..5ee403b966 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -1986,6 +1986,7 @@ void ERR_load_SSL_strings(void); # define SSL_F_SSL3_DO_CHANGE_CIPHER_SPEC 292 # define SSL_F_SSL3_ENC 134 # define SSL_F_SSL3_GENERATE_KEY_BLOCK 238 +# define SSL_F_SSL3_GENERATE_MASTER_SECRET 388 # define SSL_F_SSL3_GET_CERTIFICATE_REQUEST 135 # define SSL_F_SSL3_GET_CERT_STATUS 289 # define SSL_F_SSL3_GET_CERT_VERIFY 136 @@ -2285,8 +2286,8 @@ void ERR_load_SSL_strings(void); # define SSL_R_INVALID_TICKET_KEYS_LENGTH 325 # define SSL_R_INVALID_TRUST 279 # define SSL_R_LENGTH_MISMATCH 159 -# define SSL_R_LENGTH_TOO_SHORT 160 # define SSL_R_LENGTH_TOO_LONG 404 +# define SSL_R_LENGTH_TOO_SHORT 160 # define SSL_R_LIBRARY_BUG 274 # define SSL_R_LIBRARY_HAS_NO_CIPHERS 161 # define SSL_R_MISSING_DH_DSA_CERT 162 diff --git a/ssl/record/ssl3_record.c b/ssl/record/ssl3_record.c index 359d247bbb..381872df9f 100644 --- a/ssl/record/ssl3_record.c +++ b/ssl/record/ssl3_record.c @@ -846,33 +846,36 @@ int n_ssl3_mac(SSL *ssl, unsigned char *md, int send) header[j++] = rec->length & 0xff; /* Final param == is SSLv3 */ - ssl3_cbc_digest_record(hash, - md, &md_size, - header, rec->input, - rec->length + md_size, rec->orig_len, - mac_sec, md_size, 1); + if (ssl3_cbc_digest_record(hash, + md, &md_size, + header, rec->input, + rec->length + md_size, rec->orig_len, + mac_sec, md_size, 1) <= 0) + return -1; } else { unsigned int md_size_u; /* Chop the digest off the end :-) */ EVP_MD_CTX_init(&md_ctx); - EVP_MD_CTX_copy_ex(&md_ctx, hash); - EVP_DigestUpdate(&md_ctx, mac_sec, md_size); - EVP_DigestUpdate(&md_ctx, ssl3_pad_1, npad); - EVP_DigestUpdate(&md_ctx, seq, 8); rec_char = rec->type; - EVP_DigestUpdate(&md_ctx, &rec_char, 1); p = md; s2n(rec->length, p); - EVP_DigestUpdate(&md_ctx, md, 2); - EVP_DigestUpdate(&md_ctx, rec->input, rec->length); - EVP_DigestFinal_ex(&md_ctx, md, NULL); - - EVP_MD_CTX_copy_ex(&md_ctx, hash); - EVP_DigestUpdate(&md_ctx, mac_sec, md_size); - EVP_DigestUpdate(&md_ctx, ssl3_pad_2, npad); - EVP_DigestUpdate(&md_ctx, md, md_size); - EVP_DigestFinal_ex(&md_ctx, md, &md_size_u); + if (EVP_MD_CTX_copy_ex(&md_ctx, hash) <= 0 + || EVP_DigestUpdate(&md_ctx, mac_sec, md_size) <= 0 + || EVP_DigestUpdate(&md_ctx, ssl3_pad_1, npad) <= 0 + || EVP_DigestUpdate(&md_ctx, seq, 8) <= 0 + || EVP_DigestUpdate(&md_ctx, &rec_char, 1) <= 0 + || EVP_DigestUpdate(&md_ctx, md, 2) <= 0 + || EVP_DigestUpdate(&md_ctx, rec->input, rec->length) <= 0 + || EVP_DigestFinal_ex(&md_ctx, md, NULL) <= 0 + || EVP_MD_CTX_copy_ex(&md_ctx, hash) <= 0 + || EVP_DigestUpdate(&md_ctx, mac_sec, md_size) <= 0 + || EVP_DigestUpdate(&md_ctx, ssl3_pad_2, npad) <= 0 + || EVP_DigestUpdate(&md_ctx, md, md_size) <= 0 + || EVP_DigestFinal_ex(&md_ctx, md, &md_size_u) <= 0) { + EVP_MD_CTX_cleanup(&md_ctx); + return -1; + } md_size = md_size_u; EVP_MD_CTX_cleanup(&md_ctx); @@ -944,18 +947,24 @@ int tls1_mac(SSL *ssl, unsigned char *md, int send) * are hashing because that gives an attacker a timing-oracle. */ /* Final param == not SSLv3 */ - ssl3_cbc_digest_record(mac_ctx, - md, &md_size, - header, rec->input, - rec->length + md_size, rec->orig_len, - ssl->s3->read_mac_secret, - ssl->s3->read_mac_secret_size, 0); + if (ssl3_cbc_digest_record(mac_ctx, + md, &md_size, + header, rec->input, + rec->length + md_size, rec->orig_len, + ssl->s3->read_mac_secret, + ssl->s3->read_mac_secret_size, 0) <= 0) { + if (!stream_mac) + EVP_MD_CTX_cleanup(&hmac); + return -1; + } } else { - EVP_DigestSignUpdate(mac_ctx, header, sizeof(header)); - EVP_DigestSignUpdate(mac_ctx, rec->input, rec->length); - t = EVP_DigestSignFinal(mac_ctx, md, &md_size); - if (t <= 0) + if (EVP_DigestSignUpdate(mac_ctx, header, sizeof(header)) <= 0 + || EVP_DigestSignUpdate(mac_ctx, rec->input, rec->length) <= 0 + || EVP_DigestSignFinal(mac_ctx, md, &md_size) <= 0) { + if (!stream_mac) + EVP_MD_CTX_cleanup(&hmac); return -1; + } if (!send && !SSL_USE_ETM(ssl) && FIPS_mode()) tls_fips_digest_extra(ssl->enc_read_ctx, mac_ctx, rec->input, @@ -964,6 +973,7 @@ int tls1_mac(SSL *ssl, unsigned char *md, int send) if (!stream_mac) EVP_MD_CTX_cleanup(&hmac); + #ifdef TLS_DEBUG fprintf(stderr, "seq="); { diff --git a/ssl/s3_cbc.c b/ssl/s3_cbc.c index f7997eaae3..5225222913 100644 --- a/ssl/s3_cbc.c +++ b/ssl/s3_cbc.c @@ -172,8 +172,9 @@ char ssl3_cbc_record_digest_supported(const EVP_MD_CTX *ctx) * functions, above, we know that data_plus_mac_size is large enough to contain * a padding byte and MAC. (If the padding was invalid, it might contain the * padding too. ) + * Returns 1 on success or 0 on error */ -void ssl3_cbc_digest_record(const EVP_MD_CTX *ctx, +int ssl3_cbc_digest_record(const EVP_MD_CTX *ctx, unsigned char *md_out, size_t *md_out_size, const unsigned char header[13], @@ -217,7 +218,8 @@ void ssl3_cbc_digest_record(const EVP_MD_CTX *ctx, switch (EVP_MD_CTX_type(ctx)) { case NID_md5: - MD5_Init((MD5_CTX *)md_state.c); + if (MD5_Init((MD5_CTX *)md_state.c) <= 0) + return 0; md_final_raw = tls1_md5_final_raw; md_transform = (void (*)(void *ctx, const unsigned char *block))MD5_Transform; @@ -226,28 +228,32 @@ void ssl3_cbc_digest_record(const EVP_MD_CTX *ctx, length_is_big_endian = 0; break; case NID_sha1: - SHA1_Init((SHA_CTX *)md_state.c); + if (SHA1_Init((SHA_CTX *)md_state.c) <= 0) + return 0; md_final_raw = tls1_sha1_final_raw; md_transform = (void (*)(void *ctx, const unsigned char *block))SHA1_Transform; md_size = 20; break; case NID_sha224: - SHA224_Init((SHA256_CTX *)md_state.c); + if (SHA224_Init((SHA256_CTX *)md_state.c) <= 0) + return 0; md_final_raw = tls1_sha256_final_raw; md_transform = (void (*)(void *ctx, const unsigned char *block))SHA256_Transform; md_size = 224 / 8; break; case NID_sha256: - SHA256_Init((SHA256_CTX *)md_state.c); + if (SHA256_Init((SHA256_CTX *)md_state.c) <= 0) + return 0; md_final_raw = tls1_sha256_final_raw; md_transform = (void (*)(void *ctx, const unsigned char *block))SHA256_Transform; md_size = 32; break; case NID_sha384: - SHA384_Init((SHA512_CTX *)md_state.c); + if (SHA384_Init((SHA512_CTX *)md_state.c) <= 0) + return 0; md_final_raw = tls1_sha512_final_raw; md_transform = (void (*)(void *ctx, const unsigned char *block))SHA512_Transform; @@ -256,7 +262,8 @@ void ssl3_cbc_digest_record(const EVP_MD_CTX *ctx, md_length_size = 16; break; case NID_sha512: - SHA512_Init((SHA512_CTX *)md_state.c); + if (SHA512_Init((SHA512_CTX *)md_state.c) <= 0) + return 0; md_final_raw = tls1_sha512_final_raw; md_transform = (void (*)(void *ctx, const unsigned char *block))SHA512_Transform; @@ -272,7 +279,7 @@ void ssl3_cbc_digest_record(const EVP_MD_CTX *ctx, OPENSSL_assert(0); if (md_out_size) *md_out_size = -1; - return; + return 0; } OPENSSL_assert(md_length_size <= MAX_HASH_BIT_COUNT_BYTES); @@ -410,7 +417,7 @@ void ssl3_cbc_digest_record(const EVP_MD_CTX *ctx, */ if (header_length <= md_block_size) { /* Should never happen */ - return; + return 0; } overhang = header_length - md_block_size; md_transform(md_state.c, header); @@ -491,26 +498,34 @@ void ssl3_cbc_digest_record(const EVP_MD_CTX *ctx, } EVP_MD_CTX_init(&md_ctx); - EVP_DigestInit_ex(&md_ctx, ctx->digest, NULL /* engine */ ); + if (EVP_DigestInit_ex(&md_ctx, ctx->digest, NULL /* engine */ ) <= 0) + goto err; if (is_sslv3) { /* We repurpose |hmac_pad| to contain the SSLv3 pad2 block. */ memset(hmac_pad, 0x5c, sslv3_pad_length); - EVP_DigestUpdate(&md_ctx, mac_secret, mac_secret_length); - EVP_DigestUpdate(&md_ctx, hmac_pad, sslv3_pad_length); - EVP_DigestUpdate(&md_ctx, mac_out, md_size); + if (EVP_DigestUpdate(&md_ctx, mac_secret, mac_secret_length) <= 0 + || EVP_DigestUpdate(&md_ctx, hmac_pad, sslv3_pad_length) <= 0 + || EVP_DigestUpdate(&md_ctx, mac_out, md_size) <= 0) + goto err; } else { /* Complete the HMAC in the standard manner. */ for (i = 0; i < md_block_size; i++) hmac_pad[i] ^= 0x6a; - EVP_DigestUpdate(&md_ctx, hmac_pad, md_block_size); - EVP_DigestUpdate(&md_ctx, mac_out, md_size); + if (EVP_DigestUpdate(&md_ctx, hmac_pad, md_block_size) <= 0 + || EVP_DigestUpdate(&md_ctx, mac_out, md_size) <= 0) + goto err; } ret = EVP_DigestFinal(&md_ctx, md_out, &md_out_size_u); if (ret && md_out_size) *md_out_size = md_out_size_u; EVP_MD_CTX_cleanup(&md_ctx); + + return 1; +err: + EVP_MD_CTX_cleanup(&md_ctx); + return 0; } /* diff --git a/ssl/s3_enc.c b/ssl/s3_enc.c index 02e07bac94..1a5aaeceaa 100644 --- a/ssl/s3_enc.c +++ b/ssl/s3_enc.c @@ -253,7 +253,7 @@ int ssl3_change_cipher_state(SSL *s, int which) EVP_CIPHER_CTX_init(s->enc_read_ctx); dd = s->enc_read_ctx; - if (!ssl_replace_hash(&s->read_hash, m)) { + if (ssl_replace_hash(&s->read_hash, m) == NULL) { SSLerr(SSL_F_SSL3_CHANGE_CIPHER_STATE, ERR_R_INTERNAL_ERROR); goto err2; } @@ -286,7 +286,7 @@ int ssl3_change_cipher_state(SSL *s, int which) */ EVP_CIPHER_CTX_init(s->enc_write_ctx); dd = s->enc_write_ctx; - if (!ssl_replace_hash(&s->write_hash, m)) { + if (ssl_replace_hash(&s->write_hash, m) == NULL) { SSLerr(SSL_F_SSL3_CHANGE_CIPHER_STATE, ERR_R_INTERNAL_ERROR); goto err2; } @@ -617,19 +617,21 @@ static int ssl3_handshake_mac(SSL *s, int md_nid, return 0; npad = (48 / n) * n; - if (sender != NULL) - EVP_DigestUpdate(&ctx, sender, len); - EVP_DigestUpdate(&ctx, s->session->master_key, - s->session->master_key_length); - EVP_DigestUpdate(&ctx, ssl3_pad_1, npad); - EVP_DigestFinal_ex(&ctx, md_buf, &i); - - EVP_DigestInit_ex(&ctx, EVP_MD_CTX_md(&ctx), NULL); - EVP_DigestUpdate(&ctx, s->session->master_key, - s->session->master_key_length); - EVP_DigestUpdate(&ctx, ssl3_pad_2, npad); - EVP_DigestUpdate(&ctx, md_buf, i); - EVP_DigestFinal_ex(&ctx, p, &ret); + if ((sender != NULL && EVP_DigestUpdate(&ctx, sender, len) <= 0) + || EVP_DigestUpdate(&ctx, s->session->master_key, + s->session->master_key_length) <= 0 + || EVP_DigestUpdate(&ctx, ssl3_pad_1, npad) <= 0 + || EVP_DigestFinal_ex(&ctx, md_buf, &i) <= 0 + + || EVP_DigestInit_ex(&ctx, EVP_MD_CTX_md(&ctx), NULL) <= 0 + || EVP_DigestUpdate(&ctx, s->session->master_key, + s->session->master_key_length) <= 0 + || EVP_DigestUpdate(&ctx, ssl3_pad_2, npad) <= 0 + || EVP_DigestUpdate(&ctx, md_buf, i) <= 0 + || EVP_DigestFinal_ex(&ctx, p, &ret) <= 0) { + SSLerr(SSL_F_SSL3_HANDSHAKE_MAC, ERR_R_INTERNAL_ERROR); + ret = 0; + } EVP_MD_CTX_cleanup(&ctx); @@ -660,24 +662,31 @@ int ssl3_generate_master_secret(SSL *s, unsigned char *out, unsigned char *p, EVP_MD_CTX_init(&ctx); for (i = 0; i < 3; i++) { - EVP_DigestInit_ex(&ctx, s->ctx->sha1, NULL); - EVP_DigestUpdate(&ctx, salt[i], strlen((const char *)salt[i])); - EVP_DigestUpdate(&ctx, p, len); - EVP_DigestUpdate(&ctx, &(s->s3->client_random[0]), SSL3_RANDOM_SIZE); - EVP_DigestUpdate(&ctx, &(s->s3->server_random[0]), SSL3_RANDOM_SIZE); - EVP_DigestFinal_ex(&ctx, buf, &n); - - EVP_DigestInit_ex(&ctx, s->ctx->md5, NULL); - EVP_DigestUpdate(&ctx, p, len); - EVP_DigestUpdate(&ctx, buf, n); - EVP_DigestFinal_ex(&ctx, out, &n); + if (EVP_DigestInit_ex(&ctx, s->ctx->sha1, NULL) <= 0 + || EVP_DigestUpdate(&ctx, salt[i], + strlen((const char *)salt[i])) <= 0 + || EVP_DigestUpdate(&ctx, p, len) <= 0 + || EVP_DigestUpdate(&ctx, &(s->s3->client_random[0]), + SSL3_RANDOM_SIZE) <= 0 + || EVP_DigestUpdate(&ctx, &(s->s3->server_random[0]), + SSL3_RANDOM_SIZE) <= 0 + || EVP_DigestFinal_ex(&ctx, buf, &n) <= 0 + + || EVP_DigestInit_ex(&ctx, s->ctx->md5, NULL) <= 0 + || EVP_DigestUpdate(&ctx, p, len) <= 0 + || EVP_DigestUpdate(&ctx, buf, n) <= 0 + || EVP_DigestFinal_ex(&ctx, out, &n) <= 0) { + SSLerr(SSL_F_SSL3_GENERATE_MASTER_SECRET, ERR_R_INTERNAL_ERROR); + ret = 0; + break; + } out += n; ret += n; } EVP_MD_CTX_cleanup(&ctx); #ifdef OPENSSL_SSL_TRACE_CRYPTO - if (s->msg_callback) { + if (ret > 0 && s->msg_callback) { s->msg_callback(2, s->version, TLS1_RT_CRYPTO_PREMASTER, p, len, s, s->msg_callback_arg); s->msg_callback(2, s->version, TLS1_RT_CRYPTO_CLIENT_RANDOM, diff --git a/ssl/ssl_ciph.c b/ssl/ssl_ciph.c index 580178e085..fe30ab47a1 100644 --- a/ssl/ssl_ciph.c +++ b/ssl/ssl_ciph.c @@ -439,10 +439,11 @@ static int get_optional_pkey_id(const char *pkey_name) const EVP_PKEY_ASN1_METHOD *ameth; int pkey_id = 0; ameth = EVP_PKEY_asn1_find_str(NULL, pkey_name, -1); - if (ameth) { - EVP_PKEY_asn1_get0_info(&pkey_id, NULL, NULL, NULL, NULL, ameth); + if (ameth && EVP_PKEY_asn1_get0_info(&pkey_id, NULL, NULL, NULL, NULL, + ameth) > 0) { + return pkey_id; } - return pkey_id; + return 0; } #else @@ -454,7 +455,9 @@ static int get_optional_pkey_id(const char *pkey_name) int pkey_id = 0; ameth = EVP_PKEY_asn1_find_str(&tmpeng, pkey_name, -1); if (ameth) { - EVP_PKEY_asn1_get0_info(&pkey_id, NULL, NULL, NULL, NULL, ameth); + if (EVP_PKEY_asn1_get0_info(&pkey_id, NULL, NULL, NULL, NULL, + ameth) <= 0) + pkey_id = 0; } if (tmpeng) ENGINE_finish(tmpeng); diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c index b66ebc457f..82fb614298 100644 --- a/ssl/ssl_err.c +++ b/ssl/ssl_err.c @@ -140,6 +140,8 @@ static ERR_STRING_DATA SSL_str_functs[] = { "ssl3_do_change_cipher_spec"}, {ERR_FUNC(SSL_F_SSL3_ENC), "ssl3_enc"}, {ERR_FUNC(SSL_F_SSL3_GENERATE_KEY_BLOCK), "ssl3_generate_key_block"}, + {ERR_FUNC(SSL_F_SSL3_GENERATE_MASTER_SECRET), + "ssl3_generate_master_secret"}, {ERR_FUNC(SSL_F_SSL3_GET_CERTIFICATE_REQUEST), "ssl3_get_certificate_request"}, {ERR_FUNC(SSL_F_SSL3_GET_CERT_STATUS), "ssl3_get_cert_status"}, diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index d8d2244ae5..1c3b726674 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -3165,8 +3165,11 @@ EVP_MD_CTX *ssl_replace_hash(EVP_MD_CTX **hash, const EVP_MD *md) { ssl_clear_hash_ctx(hash); *hash = EVP_MD_CTX_create(); - if (md) - EVP_DigestInit_ex(*hash, md, NULL); + if (*hash == NULL || (md && EVP_DigestInit_ex(*hash, md, NULL) <= 0)) { + EVP_MD_CTX_destroy(*hash); + *hash = NULL; + return NULL; + } return *hash; } diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 1295b7bdaf..6ccdbe4942 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -2114,15 +2114,15 @@ __owur int ssl_handshake_hash(SSL *s, unsigned char *out, int outlen); /* s3_cbc.c */ __owur char ssl3_cbc_record_digest_supported(const EVP_MD_CTX *ctx); -void ssl3_cbc_digest_record(const EVP_MD_CTX *ctx, - unsigned char *md_out, - size_t *md_out_size, - const unsigned char header[13], - const unsigned char *data, - size_t data_plus_mac_size, - size_t data_plus_mac_plus_padding_size, - const unsigned char *mac_secret, - unsigned mac_secret_length, char is_sslv3); +__owur int ssl3_cbc_digest_record(const EVP_MD_CTX *ctx, + unsigned char *md_out, + size_t *md_out_size, + const unsigned char header[13], + const unsigned char *data, + size_t data_plus_mac_size, + size_t data_plus_mac_plus_padding_size, + 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, diff --git a/ssl/ssl_rsa.c b/ssl/ssl_rsa.c index be552c1d36..96353c18bc 100644 --- a/ssl/ssl_rsa.c +++ b/ssl/ssl_rsa.c @@ -157,7 +157,10 @@ int SSL_use_RSAPrivateKey(SSL *ssl, RSA *rsa) } RSA_up_ref(rsa); - EVP_PKEY_assign_RSA(pkey, rsa); + if (EVP_PKEY_assign_RSA(pkey, rsa) <= 0) { + RSA_free(rsa); + return 0; + } ret = ssl_set_pkey(ssl->cert, pkey); EVP_PKEY_free(pkey); @@ -192,6 +195,15 @@ static int ssl_set_pkey(CERT *c, EVP_PKEY *pkey) if (c->pkeys[i].x509 != NULL) { EVP_PKEY *pktmp; pktmp = X509_get_pubkey(c->pkeys[i].x509); + if (pktmp == NULL) { + SSLerr(SSL_F_SSL_SET_PKEY, ERR_R_MALLOC_FAILURE); + EVP_PKEY_free(pktmp); + return 0; + } + /* + * The return code from EVP_PKEY_copy_parameters is deliberately + * ignored. Some EVP_PKEY types cannot do this. + */ EVP_PKEY_copy_parameters(pktmp, pkey); EVP_PKEY_free(pktmp); ERR_clear_error(); @@ -386,6 +398,10 @@ static int ssl_set_cert(CERT *c, X509 *x) } if (c->pkeys[i].privatekey != NULL) { + /* + * The return code from EVP_PKEY_copy_parameters is deliberately + * ignored. Some EVP_PKEY types cannot do this. + */ EVP_PKEY_copy_parameters(pkey, c->pkeys[i].privatekey); ERR_clear_error(); @@ -498,7 +514,10 @@ int SSL_CTX_use_RSAPrivateKey(SSL_CTX *ctx, RSA *rsa) } RSA_up_ref(rsa); - EVP_PKEY_assign_RSA(pkey, rsa); + if (EVP_PKEY_assign_RSA(pkey, rsa) <= 0) { + RSA_free(rsa); + return 0; + } ret = ssl_set_pkey(ctx->cert, pkey); EVP_PKEY_free(pkey); diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index f6b95d6d75..7b7fc229d9 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -1961,15 +1961,21 @@ MSG_PROCESS_RETURN tls_process_key_exchange(SSL *s, PACKET *pkt) q = md_buf; for (num = 2; num > 0; num--) { EVP_MD_CTX_set_flags(&md_ctx, EVP_MD_CTX_FLAG_NON_FIPS_ALLOW); - EVP_DigestInit_ex(&md_ctx, (num == 2) - ? s->ctx->md5 : s->ctx->sha1, NULL); - EVP_DigestUpdate(&md_ctx, &(s->s3->client_random[0]), - SSL3_RANDOM_SIZE); - EVP_DigestUpdate(&md_ctx, &(s->s3->server_random[0]), - SSL3_RANDOM_SIZE); - EVP_DigestUpdate(&md_ctx, PACKET_data(¶ms), - PACKET_remaining(¶ms)); - EVP_DigestFinal_ex(&md_ctx, q, &size); + if (EVP_DigestInit_ex(&md_ctx, + (num == 2) ? s->ctx->md5 : s->ctx->sha1, + NULL) <= 0 + || EVP_DigestUpdate(&md_ctx, &(s->s3->client_random[0]), + SSL3_RANDOM_SIZE) <= 0 + || EVP_DigestUpdate(&md_ctx, &(s->s3->server_random[0]), + SSL3_RANDOM_SIZE) <= 0 + || EVP_DigestUpdate(&md_ctx, PACKET_data(¶ms), + PACKET_remaining(¶ms)) <= 0 + || EVP_DigestFinal_ex(&md_ctx, q, &size) <= 0) { + SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, + ERR_R_INTERNAL_ERROR); + al = SSL_AD_INTERNAL_ERROR; + goto f_err; + } q += size; j += size; } @@ -1990,13 +1996,17 @@ MSG_PROCESS_RETURN tls_process_key_exchange(SSL *s, PACKET *pkt) } else #endif { - EVP_VerifyInit_ex(&md_ctx, md, NULL); - EVP_VerifyUpdate(&md_ctx, &(s->s3->client_random[0]), - SSL3_RANDOM_SIZE); - EVP_VerifyUpdate(&md_ctx, &(s->s3->server_random[0]), - SSL3_RANDOM_SIZE); - EVP_VerifyUpdate(&md_ctx, PACKET_data(¶ms), - PACKET_remaining(¶ms)); + if (EVP_VerifyInit_ex(&md_ctx, md, NULL) <= 0 + || EVP_VerifyUpdate(&md_ctx, &(s->s3->client_random[0]), + SSL3_RANDOM_SIZE) <= 0 + || EVP_VerifyUpdate(&md_ctx, &(s->s3->server_random[0]), + SSL3_RANDOM_SIZE) <= 0 + || EVP_VerifyUpdate(&md_ctx, PACKET_data(¶ms), + PACKET_remaining(¶ms)) <= 0) { + al = SSL_AD_INTERNAL_ERROR; + SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, ERR_R_EVP_LIB); + goto f_err; + } if (EVP_VerifyFinal(&md_ctx, PACKET_data(&signature), PACKET_remaining(&signature), pkey) <= 0) { /* bad signature */ @@ -2786,16 +2796,16 @@ psk_err: } /* * If we have send a certificate, and certificate key - * - * * parameters match those of server certificate, use + * parameters match those of server certificate, use * certificate key for key exchange */ /* Otherwise, generate ephemeral key pair */ - EVP_PKEY_encrypt_init(pkey_ctx); - /* Generate session key */ - if (RAND_bytes(pms, pmslen) <= 0) { + if (pkey_ctx == NULL + || EVP_PKEY_encrypt_init(pkey_ctx) <= 0 + /* Generate session key */ + || RAND_bytes(pms, pmslen) <= 0) { EVP_PKEY_CTX_free(pkey_ctx); SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR); @@ -2819,13 +2829,18 @@ psk_err: * data */ ukm_hash = EVP_MD_CTX_create(); - EVP_DigestInit(ukm_hash, - EVP_get_digestbynid(NID_id_GostR3411_94)); - EVP_DigestUpdate(ukm_hash, s->s3->client_random, - SSL3_RANDOM_SIZE); - EVP_DigestUpdate(ukm_hash, s->s3->server_random, - SSL3_RANDOM_SIZE); - EVP_DigestFinal_ex(ukm_hash, shared_ukm, &md_len); + if (EVP_DigestInit(ukm_hash, + EVP_get_digestbynid(NID_id_GostR3411_94)) <= 0 + || EVP_DigestUpdate(ukm_hash, s->s3->client_random, + SSL3_RANDOM_SIZE) <= 0 + || EVP_DigestUpdate(ukm_hash, s->s3->server_random, + SSL3_RANDOM_SIZE) <= 0 + || EVP_DigestFinal_ex(ukm_hash, shared_ukm, &md_len) <= 0) { + EVP_MD_CTX_destroy(ukm_hash); + SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_KEY_EXCHANGE, + ERR_R_INTERNAL_ERROR); + goto err; + } EVP_MD_CTX_destroy(ukm_hash); if (EVP_PKEY_CTX_ctrl (pkey_ctx, -1, EVP_PKEY_OP_ENCRYPT, EVP_PKEY_CTRL_SET_IV, 8, @@ -2840,7 +2855,7 @@ psk_err: */ *(p++) = V_ASN1_SEQUENCE | V_ASN1_CONSTRUCTED; msglen = 255; - if (EVP_PKEY_encrypt(pkey_ctx, tmp, &msglen, pms, pmslen) < 0) { + if (EVP_PKEY_encrypt(pkey_ctx, tmp, &msglen, pms, pmslen) <= 0) { SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_KEY_EXCHANGE, SSL_R_LIBRARY_BUG); goto err; @@ -3006,7 +3021,10 @@ int tls_construct_client_verify(SSL *s) SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_VERIFY, ERR_R_MALLOC_FAILURE); goto err; } - EVP_PKEY_sign_init(pctx); + if (EVP_PKEY_sign_init(pctx) <= 0) { + SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_VERIFY, ERR_R_INTERNAL_ERROR); + goto err; + } if (EVP_PKEY_CTX_set_signature_md(pctx, EVP_sha1()) > 0) { if (!SSL_USE_SIGALGS(s)) s->method->ssl3_enc->cert_verify_mac(s, diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index c4187876fd..a7498d8402 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -2109,14 +2109,20 @@ int tls_construct_server_key_exchange(SSL *s) for (num = 2; num > 0; num--) { EVP_MD_CTX_set_flags(&md_ctx, EVP_MD_CTX_FLAG_NON_FIPS_ALLOW); - EVP_DigestInit_ex(&md_ctx, (num == 2) - ? s->ctx->md5 : s->ctx->sha1, NULL); - EVP_DigestUpdate(&md_ctx, &(s->s3->client_random[0]), - SSL3_RANDOM_SIZE); - EVP_DigestUpdate(&md_ctx, &(s->s3->server_random[0]), - SSL3_RANDOM_SIZE); - EVP_DigestUpdate(&md_ctx, d, n); - EVP_DigestFinal_ex(&md_ctx, q, (unsigned int *)&i); + if (EVP_DigestInit_ex(&md_ctx, (num == 2) + ? s->ctx->md5 : s->ctx->sha1, NULL) <= 0 + || EVP_DigestUpdate(&md_ctx, &(s->s3->client_random[0]), + SSL3_RANDOM_SIZE) <= 0 + || EVP_DigestUpdate(&md_ctx, &(s->s3->server_random[0]), + SSL3_RANDOM_SIZE) <= 0 + || EVP_DigestUpdate(&md_ctx, d, n) <= 0 + || EVP_DigestFinal_ex(&md_ctx, q, + (unsigned int *)&i) <= 0) { + SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE, + ERR_LIB_EVP); + al = SSL_AD_INTERNAL_ERROR; + goto f_err; + } q += i; j += i; } @@ -2144,16 +2150,17 @@ int tls_construct_server_key_exchange(SSL *s) #ifdef SSL_DEBUG fprintf(stderr, "Using hash %s\n", EVP_MD_name(md)); #endif - EVP_SignInit_ex(&md_ctx, md, NULL); - EVP_SignUpdate(&md_ctx, &(s->s3->client_random[0]), - SSL3_RANDOM_SIZE); - EVP_SignUpdate(&md_ctx, &(s->s3->server_random[0]), - SSL3_RANDOM_SIZE); - EVP_SignUpdate(&md_ctx, d, n); - if (!EVP_SignFinal(&md_ctx, &(p[2]), - (unsigned int *)&i, pkey)) { + if (EVP_SignInit_ex(&md_ctx, md, NULL) <= 0 + || EVP_SignUpdate(&md_ctx, &(s->s3->client_random[0]), + SSL3_RANDOM_SIZE) <= 0 + || EVP_SignUpdate(&md_ctx, &(s->s3->server_random[0]), + SSL3_RANDOM_SIZE) <= 0 + || EVP_SignUpdate(&md_ctx, d, n) <= 0 + || EVP_SignFinal(&md_ctx, &(p[2]), + (unsigned int *)&i, pkey) <= 0) { SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE, ERR_LIB_EVP); - goto err; + al = SSL_AD_INTERNAL_ERROR; + goto f_err; } s2n(i, p); n += i + 2; @@ -2812,7 +2819,11 @@ MSG_PROCESS_RETURN tls_process_client_key_exchange(SSL *s, PACKET *pkt) SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, ERR_R_MALLOC_FAILURE); goto f_err; } - EVP_PKEY_decrypt_init(pkey_ctx); + if (EVP_PKEY_decrypt_init(pkey_ctx) <= 0) { + al = SSL_AD_INTERNAL_ERROR; + SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR); + goto f_err; + } /* * If client certificate is present and is of the same type, maybe * use it for key exchange. Don't mind errors from @@ -2829,12 +2840,13 @@ MSG_PROCESS_RETURN tls_process_client_key_exchange(SSL *s, PACKET *pkt) if (!PACKET_get_bytes(pkt, &data, sess_key_len)) { al = SSL_AD_INTERNAL_ERROR; SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR); - goto f_err; + goto gerr; } if (ASN1_get_object ((const unsigned char **)&data, &Tlen, &Ttag, &Tclass, sess_key_len) != V_ASN1_CONSTRUCTED || Ttag != V_ASN1_SEQUENCE || Tclass != V_ASN1_UNIVERSAL) { + al = SSL_AD_DECODE_ERROR; SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, SSL_R_DECRYPTION_FAILED); goto gerr; @@ -2852,7 +2864,7 @@ MSG_PROCESS_RETURN tls_process_client_key_exchange(SSL *s, PACKET *pkt) sizeof(premaster_secret), 0)) { al = SSL_AD_INTERNAL_ERROR; SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR); - goto f_err; + goto gerr; } /* Check if pubkey from client certificate was used */ if (EVP_PKEY_CTX_ctrl @@ -2865,7 +2877,7 @@ MSG_PROCESS_RETURN tls_process_client_key_exchange(SSL *s, PACKET *pkt) gerr: EVP_PKEY_free(client_pub_pkey); EVP_PKEY_CTX_free(pkey_ctx); - goto err; + goto f_err; } else { al = SSL_AD_HANDSHAKE_FAILURE; SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, SSL_R_UNKNOWN_CIPHER_TYPE); @@ -3150,7 +3162,11 @@ MSG_PROCESS_RETURN tls_process_cert_verify(SSL *s, PACKET *pkt) SSLerr(SSL_F_TLS_PROCESS_CERT_VERIFY, ERR_R_MALLOC_FAILURE); goto f_err; } - EVP_PKEY_verify_init(pctx); + if (EVP_PKEY_verify_init(pctx) <= 0) { + al = SSL_AD_INTERNAL_ERROR; + SSLerr(SSL_F_TLS_PROCESS_CERT_VERIFY, ERR_R_INTERNAL_ERROR); + goto f_err; + } if (len != 64) { fprintf(stderr, "GOST signature length is %d", len); } diff --git a/ssl/t1_enc.c b/ssl/t1_enc.c index 1f539aa174..729ceccb7e 100644 --- a/ssl/t1_enc.c +++ b/ssl/t1_enc.c @@ -353,6 +353,8 @@ int tls1_change_cipher_state(SSL *s, int which) EVP_CIPHER_CTX_init(s->enc_read_ctx); dd = s->enc_read_ctx; mac_ctx = ssl_replace_hash(&s->read_hash, NULL); + if (mac_ctx == NULL) + goto err; #ifndef OPENSSL_NO_COMP COMP_CTX_free(s->expand); s->expand = NULL; @@ -386,11 +388,14 @@ int tls1_change_cipher_state(SSL *s, int which) dd = s->enc_write_ctx; if (SSL_IS_DTLS(s)) { mac_ctx = EVP_MD_CTX_create(); - if (!mac_ctx) + if (mac_ctx == NULL) goto err; s->write_hash = mac_ctx; - } else + } else { mac_ctx = ssl_replace_hash(&s->write_hash, NULL); + if (mac_ctx == NULL) + goto err; + } #ifndef OPENSSL_NO_COMP COMP_CTX_free(s->compress); s->compress = NULL; @@ -463,7 +468,12 @@ int tls1_change_cipher_state(SSL *s, int which) if (!(EVP_CIPHER_flags(c) & EVP_CIPH_FLAG_AEAD_CIPHER)) { mac_key = EVP_PKEY_new_mac_key(mac_type, NULL, mac_secret, *mac_secret_size); - EVP_DigestSignInit(mac_ctx, NULL, m, NULL, mac_key); + if (mac_key == NULL + || EVP_DigestSignInit(mac_ctx, NULL, m, NULL, mac_key) <= 0) { + EVP_PKEY_free(mac_key); + SSLerr(SSL_F_TLS1_CHANGE_CIPHER_STATE, ERR_R_INTERNAL_ERROR); + goto err2; + } EVP_PKEY_free(mac_key); } #ifdef TLS_DEBUG @@ -711,8 +721,9 @@ int tls1_cert_verify_mac(SSL *s, int md_nid, unsigned char *out) } EVP_MD_CTX_init(&ctx); - EVP_MD_CTX_copy_ex(&ctx, d); - EVP_DigestFinal_ex(&ctx, out, &ret); + if (EVP_MD_CTX_copy_ex(&ctx, d) <=0 + || EVP_DigestFinal_ex(&ctx, out, &ret) <= 0) + ret = 0; EVP_MD_CTX_cleanup(&ctx); return ((int)ret); } diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index ffc95d848b..1439eaab22 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -3079,10 +3079,13 @@ static int tls_decrypt_ticket(SSL *s, const unsigned char *etick, /* Check key name matches */ if (memcmp(etick, tctx->tlsext_tick_key_name, 16)) return 2; - HMAC_Init_ex(&hctx, tctx->tlsext_tick_hmac_key, 16, - EVP_sha256(), NULL); - EVP_DecryptInit_ex(&ctx, EVP_aes_128_cbc(), NULL, - tctx->tlsext_tick_aes_key, etick + 16); + if (HMAC_Init_ex(&hctx, tctx->tlsext_tick_hmac_key, 16, + EVP_sha256(), NULL) <= 0 + || EVP_DecryptInit_ex(&ctx, EVP_aes_128_cbc(), NULL, + tctx->tlsext_tick_aes_key, + etick + 16) <= 0) { + goto err; + } } /* * Attempt to process session ticket, first conduct sanity and integrity @@ -3090,13 +3093,14 @@ static int tls_decrypt_ticket(SSL *s, const unsigned char *etick, */ mlen = HMAC_size(&hctx); if (mlen < 0) { - EVP_CIPHER_CTX_cleanup(&ctx); - return -1; + goto err; } eticklen -= mlen; /* Check HMAC of encrypted ticket */ - HMAC_Update(&hctx, etick, eticklen); - HMAC_Final(&hctx, tick_hmac, NULL); + if (HMAC_Update(&hctx, etick, eticklen) <= 0 + || HMAC_Final(&hctx, tick_hmac, NULL) <= 0) { + goto err; + } HMAC_CTX_cleanup(&hctx); if (CRYPTO_memcmp(tick_hmac, etick + eticklen, mlen)) { EVP_CIPHER_CTX_cleanup(&ctx); @@ -3107,11 +3111,11 @@ static int tls_decrypt_ticket(SSL *s, const unsigned char *etick, p = etick + 16 + EVP_CIPHER_CTX_iv_length(&ctx); eticklen -= 16 + EVP_CIPHER_CTX_iv_length(&ctx); sdec = OPENSSL_malloc(eticklen); - if (sdec == NULL) { + if (sdec == NULL + || EVP_DecryptUpdate(&ctx, sdec, &slen, p, eticklen) <= 0) { EVP_CIPHER_CTX_cleanup(&ctx); return -1; } - EVP_DecryptUpdate(&ctx, sdec, &slen, p, eticklen); if (EVP_DecryptFinal(&ctx, sdec + slen, &mlen) <= 0) { EVP_CIPHER_CTX_cleanup(&ctx); OPENSSL_free(sdec); @@ -3144,6 +3148,10 @@ static int tls_decrypt_ticket(SSL *s, const unsigned char *etick, * For session parse failure, indicate that we need to send a new ticket. */ return 2; +err: + EVP_CIPHER_CTX_cleanup(&ctx); + HMAC_CTX_cleanup(&hctx); + return -1; } /* Tables to translate from NIDs to TLS v1.2 ids */ -- 2.34.1