PR: 1794

Submitted by: Peter Sylvester <peter.sylvester@edelweb.fr>
Reviewed by: steve

Remove unnecessary code for srp and to add some comments to
s_client.

- the callback to provide a user during client connect is
no longer necessary since rfc 5054 a connection attempt
with an srp cipher and no user is terminated when the
cipher is acceptable

- comments to indicate in s_client the (non-)usefulness of
th primalaty tests for non known group parameters.

1 files changed, 21 insertions, 14 deletions

diff --git a/apps/s_client.c b/apps/s_client.c
index 1621a0158a..5c9e1c56a8 100644
--- a/apps/s_client.c
+++ b/apps/s_client.c
@@ -403,18 +403,18 @@ typedef struct srp_arg_st
 
 #define SRP_NUMBER_ITERATIONS_FOR_PRIME 64
 
-static int SRP_Verify_N_and_g(const BIGNUM *N, const BIGNUM *g)
+static int srp_Verify_N_and_g(const BIGNUM *N, const BIGNUM *g)
 	{
 	BN_CTX *bn_ctx = BN_CTX_new();
 	BIGNUM *p = BN_new();
 	BIGNUM *r = BN_new();
 	int ret =
 		g != NULL && N != NULL && bn_ctx != NULL && BN_is_odd(N) &&
-		BN_is_prime_ex(N,SRP_NUMBER_ITERATIONS_FOR_PRIME,bn_ctx,NULL) &&
+		BN_is_prime_ex(N, SRP_NUMBER_ITERATIONS_FOR_PRIME, bn_ctx, NULL) &&
 		p != NULL && BN_rshift1(p, N) &&
 
 		/* p = (N-1)/2 */
-		BN_is_prime_ex(p,SRP_NUMBER_ITERATIONS_FOR_PRIME,bn_ctx,NULL) &&
+		BN_is_prime_ex(p, SRP_NUMBER_ITERATIONS_FOR_PRIME, bn_ctx, NULL) &&
 		r != NULL &&
 
 		/* verify g^((N-1)/2) == -1 (mod N) */
@@ -431,6 +431,21 @@ static int SRP_Verify_N_and_g(const BIGNUM *N, const BIGNUM *g)
 	return ret;
 	}
 
+/* This callback is used here for two purposes:
+   - extended debugging
+   - making some primality tests for unknown groups
+   The callback is only called for a non default group.
+
+   An application does not need the call back at all if
+   only the stanard groups are used.  In real life situations, 
+   client and server already share well known groups, 
+   thus there is no need to verify them. 
+   Furthermore, in case that a server actually proposes a group that
+   is not one of those defined in RFC 5054, it is more appropriate 
+   to add the group to a static list and then compare since 
+   primality tests are rather cpu consuming.
+*/
+
 static int MS_CALLBACK ssl_srp_verify_param_cb(SSL *s, void *arg)
 	{
 	SRP_ARG *srp_arg = (SRP_ARG *)arg;
@@ -453,11 +468,11 @@ static int MS_CALLBACK ssl_srp_verify_param_cb(SSL *s, void *arg)
 		if (srp_arg->debug)
 			BIO_printf(bio_err, "SRP param N and g are not known params, going to check deeper.\n");
 
-/* The srp_moregroups must be used with caution, testing primes costs time. 
+/* The srp_moregroups is a real debugging feature.
    Implementors should rather add the value to the known ones.
    The minimal size has already been tested.
 */
-		if (BN_num_bits(g) <= BN_BITS && SRP_Verify_N_and_g(N,g))
+		if (BN_num_bits(g) <= BN_BITS && srp_Verify_N_and_g(N,g))
 			return 1;
 		}	
 	BIO_printf(bio_err, "SRP param N and g rejected.\n");
@@ -486,12 +501,6 @@ static char * MS_CALLBACK ssl_give_srp_client_pwd_cb(SSL *s, void *arg)
 	return pass;
 	}
 
-static char * MS_CALLBACK missing_srp_username_callback(SSL *s, void *arg)
-	{
-	SRP_ARG *srp_arg = (SRP_ARG *)arg;
-	return BUF_strdup(srp_arg->srplogin);
-	}
-
 #endif
 	char *srtp_profiles = NULL;
 
@@ -1182,9 +1191,7 @@ bad:
 #ifndef OPENSSL_NO_SRP
         if (srp_arg.srplogin)
 		{
-		if (srp_lateuser) 
-			SSL_CTX_set_srp_missing_srp_username_callback(ctx,missing_srp_username_callback);
-		else if (!SSL_CTX_set_srp_username(ctx, srp_arg.srplogin))
+		if (!srp_lateuser && !SSL_CTX_set_srp_username(ctx, srp_arg.srplogin))
 			{
 			BIO_printf(bio_err,"Unable to set SRP username\n");
 			goto end;