From: Dr. Stephen Henson <steve@openssl.org>
Date: Fri, 1 Aug 2014 13:56:56 +0000 (+0100)
Subject: Check SRP parameters early.
X-Git-Tag: OpenSSL_1_0_1i~4
X-Git-Url: https://git.openssl.org/?p=openssl.git;a=commitdiff_plain;h=d15d17bb6487a3e7b59a22882b6a22300fe5111d

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 0aba7e5e0a..ea0c82de47 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -1469,6 +1469,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 eb250931ff..2867501287 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -2795,6 +2795,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 0844391a4f..b73da5ee1e 100644
--- a/ssl/ssl.h
+++ b/ssl/ssl.h
@@ -2325,6 +2325,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 49ab43e0e5..d2f0dec71b 100644
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -329,6 +329,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 1dcce42df8..98888d2df3 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -1184,6 +1184,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 2315a7c0a2..e7368a8f6b 100644
--- 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;
 	}