Check SRP parameters early.

Check SRP parameters when they are received so we can send back an
appropriate alert.
Reviewed-by: Kurt Roeckx <kurt@openssl.org>

diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index d9b777701bfb9017e504fbf060baac2cd0a1e8ed..b4964f29e4bbd2d229599ca90ae5977c21fe9a01 100644 (file)
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -1555,6 +1555,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 ce14fb6f69d9c6b0dffc9e349be9c3dc41143549..0c998a1fde6499fe5d5989492cebcadba24e3a21 100644 (file)
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -2847,6 +2847,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 da36aaea074f2e90cfca0f53d1fdc84a97f2d3e8..3f5fc712e3aec87f5ea01a75bc590792c71a15dc 100644 (file)
--- a/ssl/ssl.h
+++ b/ssl/ssl.h
@@ -2752,6 +2752,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

diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index fb90aa800c68569b413dafcc017efcc498e20340..1eca0371758bc4e16b88cf7c7a0b53d3e25502f4 100644 (file)
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -341,6 +341,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"},

diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index bdb396913ca6174def3143131257027f787bce87..91d55f82c3520a9feee71ab90232e1ad48837fd4 100644 (file)
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -1359,6 +1359,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 2315a7c0a2e9f3ea84c3096ae66d3ede12c60dde..e7368a8f6bc25e92ee36562a30fadf3c8faa510b 100644 (file)
--- a/ssl/tls_srp.c
+++ b/ssl/tls_srp.c
@@ -408,16 +408,46 @@ 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)
-               return -1;
+       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))
-               return -1 ;
+       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];
 
        RAND_bytes(rnd, sizeof(rnd));
        s->srp_ctx.a = BN_bin2bn(rnd, sizeof(rnd), s->srp_ctx.a);
@@ -426,10 +456,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 -1;
 
-       /* 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;
        }