From 7837c7ec45a9115fe3af1ae22bccde4e3fadcb0c Mon Sep 17 00:00:00 2001 From: "Dr. Stephen Henson" Date: Mon, 17 May 2010 11:27:22 +0000 Subject: [PATCH] PR: 2259 Submitted By: Artem Chuprina Check return values of HMAC in tls_P_hash and tls1_generate_key_block. Although the previous version could in theory crash that would only happen if a digest call failed. The standard software methods can never fail and only one ENGINE currently uses digests and it is not compiled in by default. --- ssl/t1_enc.c | 151 +++++++++++++++++++++++++++++++++------------------ 1 file changed, 97 insertions(+), 54 deletions(-) diff --git a/ssl/t1_enc.c b/ssl/t1_enc.c index 028f6493d1..01884906df 100644 --- a/ssl/t1_enc.c +++ b/ssl/t1_enc.c @@ -149,7 +149,7 @@ #endif /* seed1 through seed5 are virtually concatenated */ -static void tls1_P_hash(const EVP_MD *md, const unsigned char *sec, +static int tls1_P_hash(const EVP_MD *md, const unsigned char *sec, int sec_len, const void *seed1, int seed1_len, const void *seed2, int seed2_len, @@ -164,55 +164,79 @@ static void tls1_P_hash(const EVP_MD *md, const unsigned char *sec, HMAC_CTX ctx_tmp; unsigned char A1[EVP_MAX_MD_SIZE]; unsigned int A1_len; + int ret = 0; chunk=EVP_MD_size(md); OPENSSL_assert(chunk >= 0); HMAC_CTX_init(&ctx); HMAC_CTX_init(&ctx_tmp); - HMAC_Init_ex(&ctx,sec,sec_len,md, NULL); - HMAC_Init_ex(&ctx_tmp,sec,sec_len,md, NULL); - if (seed1 != NULL) HMAC_Update(&ctx,seed1,seed1_len); - if (seed2 != NULL) HMAC_Update(&ctx,seed2,seed2_len); - if (seed3 != NULL) HMAC_Update(&ctx,seed3,seed3_len); - if (seed4 != NULL) HMAC_Update(&ctx,seed4,seed4_len); - if (seed5 != NULL) HMAC_Update(&ctx,seed5,seed5_len); - HMAC_Final(&ctx,A1,&A1_len); + if (!HMAC_Init_ex(&ctx,sec,sec_len,md, NULL)) + goto err; + if (!HMAC_Init_ex(&ctx_tmp,sec,sec_len,md, NULL)) + goto err; + if (seed1 != NULL && !HMAC_Update(&ctx,seed1,seed1_len)) + goto err; + if (seed2 != NULL && !HMAC_Update(&ctx,seed2,seed2_len)) + goto err; + if (seed3 != NULL && !HMAC_Update(&ctx,seed3,seed3_len)) + goto err; + if (seed4 != NULL && !HMAC_Update(&ctx,seed4,seed4_len)) + goto err; + if (seed5 != NULL && !HMAC_Update(&ctx,seed5,seed5_len)) + goto err; + if (!HMAC_Final(&ctx,A1,&A1_len)) + goto err; n=0; for (;;) { - HMAC_Init_ex(&ctx,NULL,0,NULL,NULL); /* re-init */ - HMAC_Init_ex(&ctx_tmp,NULL,0,NULL,NULL); /* re-init */ - HMAC_Update(&ctx,A1,A1_len); - HMAC_Update(&ctx_tmp,A1,A1_len); - if (seed1 != NULL) HMAC_Update(&ctx,seed1,seed1_len); - if (seed2 != NULL) HMAC_Update(&ctx,seed2,seed2_len); - if (seed3 != NULL) HMAC_Update(&ctx,seed3,seed3_len); - if (seed4 != NULL) HMAC_Update(&ctx,seed4,seed4_len); - if (seed5 != NULL) HMAC_Update(&ctx,seed5,seed5_len); + if (!HMAC_Init_ex(&ctx,NULL,0,NULL,NULL)) /* re-init */ + goto err; + if (!HMAC_Init_ex(&ctx_tmp,NULL,0,NULL,NULL)) /* re-init */ + goto err; + if (!HMAC_Update(&ctx,A1,A1_len)) + goto err; + if (!HMAC_Update(&ctx_tmp,A1,A1_len)) + goto err; + if (seed1 != NULL && !HMAC_Update(&ctx,seed1,seed1_len)) + goto err; + if (seed2 != NULL && !HMAC_Update(&ctx,seed2,seed2_len)) + goto err; + if (seed3 != NULL && !HMAC_Update(&ctx,seed3,seed3_len)) + goto err; + if (seed4 != NULL && !HMAC_Update(&ctx,seed4,seed4_len)) + goto err; + if (seed5 != NULL && !HMAC_Update(&ctx,seed5,seed5_len)) + goto err; if (olen > chunk) { - HMAC_Final(&ctx,out,&j); + if (!HMAC_Final(&ctx,out,&j)) + goto err; out+=j; olen-=j; - HMAC_Final(&ctx_tmp,A1,&A1_len); /* calc the next A1 value */ + if (!HMAC_Final(&ctx_tmp,A1,&A1_len)) /* calc the next A1 value */ + goto err; } else /* last one */ { - HMAC_Final(&ctx,A1,&A1_len); + if (!HMAC_Final(&ctx,A1,&A1_len)) + goto err; memcpy(out,A1,olen); break; } } + ret = 1; +err: HMAC_CTX_cleanup(&ctx); HMAC_CTX_cleanup(&ctx_tmp); OPENSSL_cleanse(A1,sizeof(A1)); + return ret; } /* seed1 through seed5 are virtually concatenated */ -static void tls1_PRF(long digest_mask, +static int tls1_PRF(long digest_mask, const void *seed1, int seed1_len, const void *seed2, int seed2_len, const void *seed3, int seed3_len, @@ -226,6 +250,7 @@ static void tls1_PRF(long digest_mask, const unsigned char *S1; long m; const EVP_MD *md; + int ret = 0; /* Count number of digests and partition sec evenly */ count=0; @@ -240,11 +265,12 @@ static void tls1_PRF(long digest_mask, if (!md) { SSLerr(SSL_F_TLS1_PRF, SSL_R_UNSUPPORTED_DIGEST_TYPE); - return; + goto err; } - tls1_P_hash(md ,S1,len+(slen&1), - seed1,seed1_len,seed2,seed2_len,seed3,seed3_len,seed4,seed4_len,seed5,seed5_len, - out2,olen); + if (!tls1_P_hash(md ,S1,len+(slen&1), + seed1,seed1_len,seed2,seed2_len,seed3,seed3_len,seed4,seed4_len,seed5,seed5_len, + out2,olen)) + goto err; S1+=len; for (i=0; is3->tmp.new_cipher->algorithm2, + int ret; + ret = tls1_PRF(s->s3->tmp.new_cipher->algorithm2, TLS_MD_KEY_EXPANSION_CONST,TLS_MD_KEY_EXPANSION_CONST_SIZE, s->s3->server_random,SSL3_RANDOM_SIZE, s->s3->client_random,SSL3_RANDOM_SIZE, @@ -275,6 +304,7 @@ static void tls1_generate_key_block(SSL *s, unsigned char *km, } printf("\n"); } #endif /* KSSL_DEBUG */ + return ret; } int tls1_change_cipher_state(SSL *s, int which) @@ -462,22 +492,24 @@ printf("which = %04X\nmac key=",which); /* In here I set both the read and write key/iv to the * same value since only the correct one will be used :-). */ - tls1_PRF(s->s3->tmp.new_cipher->algorithm2, - exp_label,exp_label_len, - s->s3->client_random,SSL3_RANDOM_SIZE, - s->s3->server_random,SSL3_RANDOM_SIZE, - NULL,0,NULL,0, - key,j,tmp1,tmp2,EVP_CIPHER_key_length(c)); + if (!tls1_PRF(s->s3->tmp.new_cipher->algorithm2, + exp_label,exp_label_len, + s->s3->client_random,SSL3_RANDOM_SIZE, + s->s3->server_random,SSL3_RANDOM_SIZE, + NULL,0,NULL,0, + key,j,tmp1,tmp2,EVP_CIPHER_key_length(c))) + goto err2; key=tmp1; if (k > 0) { - tls1_PRF(s->s3->tmp.new_cipher->algorithm2, - TLS_MD_IV_BLOCK_CONST,TLS_MD_IV_BLOCK_CONST_SIZE, - s->s3->client_random,SSL3_RANDOM_SIZE, - s->s3->server_random,SSL3_RANDOM_SIZE, - NULL,0,NULL,0, - empty,0,iv1,iv2,k*2); + if (!tls1_PRF(s->s3->tmp.new_cipher->algorithm2, + TLS_MD_IV_BLOCK_CONST,TLS_MD_IV_BLOCK_CONST_SIZE, + s->s3->client_random,SSL3_RANDOM_SIZE, + s->s3->server_random,SSL3_RANDOM_SIZE, + NULL,0,NULL,0, + empty,0,iv1,iv2,k*2)) + goto err2; if (client_write) iv=iv1; else @@ -519,12 +551,13 @@ err2: int tls1_setup_key_block(SSL *s) { - unsigned char *p1,*p2; + unsigned char *p1,*p2=NULL; const EVP_CIPHER *c; const EVP_MD *hash; int num; SSL_COMP *comp; int mac_type= NID_undef,mac_secret_size=0; + int ret=0; #ifdef KSSL_DEBUG printf ("tls1_setup_key_block()\n"); @@ -549,13 +582,19 @@ int tls1_setup_key_block(SSL *s) ssl3_cleanup_key_block(s); if ((p1=(unsigned char *)OPENSSL_malloc(num)) == NULL) + { + SSLerr(SSL_F_TLS1_SETUP_KEY_BLOCK,ERR_R_MALLOC_FAILURE); goto err; - if ((p2=(unsigned char *)OPENSSL_malloc(num)) == NULL) - goto err; + } s->s3->tmp.key_block_length=num; s->s3->tmp.key_block=p1; + if ((p2=(unsigned char *)OPENSSL_malloc(num)) == NULL) + { + SSLerr(SSL_F_TLS1_SETUP_KEY_BLOCK,ERR_R_MALLOC_FAILURE); + goto err; + } #ifdef TLS_DEBUG printf("client random\n"); @@ -565,9 +604,8 @@ printf("server random\n"); printf("pre-master\n"); { int z; for (z=0; zsession->master_key_length; z++) printf("%02X%c",s->session->master_key[z],((z+1)%16)?' ':'\n'); } #endif - tls1_generate_key_block(s,p1,p2,num); - OPENSSL_cleanse(p2,num); - OPENSSL_free(p2); + if (!tls1_generate_key_block(s,p1,p2,num)) + goto err; #ifdef TLS_DEBUG printf("\nkey block\n"); { int z; for (z=0; zs3->tmp.new_cipher->algorithm2, - str,slen, buf,(int)(q-buf), NULL,0, NULL,0, NULL,0, - s->session->master_key,s->session->master_key_length, - out,buf2,sizeof buf2); + if (!tls1_PRF(s->s3->tmp.new_cipher->algorithm2, + str,slen, buf,(int)(q-buf), NULL,0, NULL,0, NULL,0, + s->session->master_key,s->session->master_key_length, + out,buf2,sizeof buf2)) + err = 1; EVP_MD_CTX_cleanup(&ctx); if (err) -- 2.25.1