From: Dr. Stephen Henson Date: Sun, 3 Aug 2014 20:25:22 +0000 (+0100) Subject: Check SRP parameters early. X-Git-Tag: master-post-reformat~539 X-Git-Url: https://git.librecmc.org/?a=commitdiff_plain;h=0989790b87efcfd40ae6770f11af28a005df28ac;p=oweals%2Fopenssl.git Check SRP parameters early. Check SRP parameters when they are received so we can send back an appropriate alert. Reviewed-by: Kurt Roeckx --- diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c index 0a006a7534..09fe64e834 100644 --- a/ssl/s3_clnt.c +++ b/ssl/s3_clnt.c @@ -1570,6 +1570,12 @@ int ssl3_get_key_exchange(SSL *s) p+=i; n-=param_len; + if (!srp_verify_server_param(s, &al)) + { + SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,SSL_R_BAD_SRP_PARAMETERS); + goto f_err; + } + /* We must check if there is a certificate */ #ifndef OPENSSL_NO_RSA if (alg_a & SSL_aRSA) diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c index 20e76cced4..63fc23c540 100644 --- a/ssl/s3_srvr.c +++ b/ssl/s3_srvr.c @@ -2929,6 +2929,13 @@ int ssl3_get_client_key_exchange(SSL *s) SSLerr(SSL_F_SSL3_GET_CLIENT_KEY_EXCHANGE,ERR_R_BN_LIB); goto err; } + if (BN_ucmp(s->srp_ctx.A, s->srp_ctx.N) >= 0 + || BN_is_zero(s->srp_ctx.A)) + { + al=SSL_AD_ILLEGAL_PARAMETER; + SSLerr(SSL_F_SSL3_GET_CLIENT_KEY_EXCHANGE,SSL_R_BAD_SRP_PARAMETERS); + goto f_err; + } if (s->session->srp_username != NULL) OPENSSL_free(s->session->srp_username); s->session->srp_username = BUF_strdup(s->srp_ctx.login); diff --git a/ssl/ssl.h b/ssl/ssl.h index 0a5e621554..256d1c32a6 100644 --- a/ssl/ssl.h +++ b/ssl/ssl.h @@ -2853,6 +2853,7 @@ void ERR_load_SSL_strings(void); #define SSL_R_BAD_SRP_B_LENGTH 348 #define SSL_R_BAD_SRP_G_LENGTH 349 #define SSL_R_BAD_SRP_N_LENGTH 350 +#define SSL_R_BAD_SRP_PARAMETERS 371 #define SSL_R_BAD_SRP_S_LENGTH 351 #define SSL_R_BAD_SRTP_MKI_VALUE 352 #define SSL_R_BAD_SRTP_PROTECTION_PROFILE_LIST 353 @@ -2914,7 +2915,6 @@ void ERR_load_SSL_strings(void); #define SSL_R_ILLEGAL_PADDING 283 #define SSL_R_ILLEGAL_SUITEB_DIGEST 380 #define SSL_R_INCONSISTENT_COMPRESSION 340 -#define SSL_R_INVALID_AUDIT_PROOF 371 #define SSL_R_INVALID_CHALLENGE_LENGTH 158 #define SSL_R_INVALID_COMMAND 280 #define SSL_R_INVALID_COMPRESSION_ALGORITHM 341 diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c index 70a2a73c36..5712e67887 100644 --- a/ssl/ssl_err.c +++ b/ssl/ssl_err.c @@ -343,6 +343,7 @@ static ERR_STRING_DATA SSL_str_reasons[]= {ERR_REASON(SSL_R_BAD_SRP_B_LENGTH) ,"bad srp b length"}, {ERR_REASON(SSL_R_BAD_SRP_G_LENGTH) ,"bad srp g length"}, {ERR_REASON(SSL_R_BAD_SRP_N_LENGTH) ,"bad srp n length"}, +{ERR_REASON(SSL_R_BAD_SRP_PARAMETERS) ,"bad srp parameters"}, {ERR_REASON(SSL_R_BAD_SRP_S_LENGTH) ,"bad srp s length"}, {ERR_REASON(SSL_R_BAD_SRTP_MKI_VALUE) ,"bad srtp mki value"}, {ERR_REASON(SSL_R_BAD_SRTP_PROTECTION_PROFILE_LIST),"bad srtp protection profile list"}, @@ -404,7 +405,6 @@ static ERR_STRING_DATA SSL_str_reasons[]= {ERR_REASON(SSL_R_ILLEGAL_PADDING) ,"illegal padding"}, {ERR_REASON(SSL_R_ILLEGAL_SUITEB_DIGEST) ,"illegal Suite B digest"}, {ERR_REASON(SSL_R_INCONSISTENT_COMPRESSION),"inconsistent compression"}, -{ERR_REASON(SSL_R_INVALID_AUDIT_PROOF) ,"invalid audit proof"}, {ERR_REASON(SSL_R_INVALID_CHALLENGE_LENGTH),"invalid challenge length"}, {ERR_REASON(SSL_R_INVALID_COMMAND) ,"invalid command"}, {ERR_REASON(SSL_R_INVALID_COMPRESSION_ALGORITHM),"invalid compression algorithm"}, diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 07c13fce5d..7b28886e6b 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -1390,6 +1390,9 @@ void ssl3_cbc_digest_record( 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); + +int srp_verify_server_param(SSL *s, int *al); + #else #define ssl_init_wbio_buffer SSL_test_functions()->p_ssl_init_wbio_buffer diff --git a/ssl/tls_srp.c b/ssl/tls_srp.c index c363fc309c..1aa90c5307 100644 --- a/ssl/tls_srp.c +++ b/ssl/tls_srp.c @@ -410,16 +410,45 @@ err: return ret; } -int SRP_Calc_A_param(SSL *s) +int srp_verify_server_param(SSL *s, int *al) { - unsigned char rnd[SSL_MAX_MASTER_KEY_LENGTH]; + SRP_CTX *srp = &s->srp_ctx; + /* Sanity check parameters: we can quickly check B % N == 0 + * by checking B != 0 since B < N + */ + if (BN_ucmp(srp->g, srp->N) >=0 || BN_ucmp(srp->B, srp->N) >= 0 + || BN_is_zero(srp->B)) + { + *al = SSL3_AD_ILLEGAL_PARAMETER; + return 0; + } - if (BN_num_bits(s->srp_ctx.N) < s->srp_ctx.strength) + if (BN_num_bits(srp->N) < srp->strength) + { + *al = TLS1_AD_INSUFFICIENT_SECURITY; return 0; + } - if (s->srp_ctx.SRP_verify_param_callback ==NULL && - !SRP_check_known_gN_param(s->srp_ctx.g,s->srp_ctx.N)) + if (srp->SRP_verify_param_callback) + { + if (srp->SRP_verify_param_callback(s, srp->SRP_cb_arg) <= 0) + { + *al = TLS1_AD_INSUFFICIENT_SECURITY; + return 0; + } + } + else if(!SRP_check_known_gN_param(srp->g, srp->N)) + { + *al = TLS1_AD_INSUFFICIENT_SECURITY; return 0; + } + + return 1; + } + +int SRP_Calc_A_param(SSL *s) + { + unsigned char rnd[SSL_MAX_MASTER_KEY_LENGTH]; if (RAND_bytes(rnd, sizeof(rnd)) <= 0) return 0; @@ -429,10 +458,6 @@ int SRP_Calc_A_param(SSL *s) if (!(s->srp_ctx.A = SRP_Calc_A(s->srp_ctx.a,s->srp_ctx.N,s->srp_ctx.g))) return 0; - /* We can have a callback to verify SRP param!! */ - if (s->srp_ctx.SRP_verify_param_callback !=NULL) - return s->srp_ctx.SRP_verify_param_callback(s,s->srp_ctx.SRP_cb_arg); - return 1; }