From f2fc30751e60f5877607eeef3ad1ad1c0fdfc430 Mon Sep 17 00:00:00 2001 From: "Dr. Stephen Henson" Date: Wed, 14 Dec 2011 22:17:06 +0000 Subject: [PATCH] PR: 1794 Submitted by: Peter Sylvester 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. --- apps/s_client.c | 35 +++++++++++++++++++++-------------- crypto/symhacks.h | 3 --- ssl/s3_lib.c | 4 ---- ssl/ssl.h | 12 +++++------- ssl/ssltest.c | 11 +---------- ssl/tls_srp.c | 23 +---------------------- 6 files changed, 28 insertions(+), 60 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; diff --git a/crypto/symhacks.h b/crypto/symhacks.h index 98e06b4300..1243627998 100644 --- a/crypto/symhacks.h +++ b/crypto/symhacks.h @@ -202,9 +202,6 @@ #define SSL_CTX_set_srp_verify_param_callback SSL_CTX_set_srp_vfy_param_cb #undef SSL_CTX_set_srp_username_callback #define SSL_CTX_set_srp_username_callback SSL_CTX_set_srp_un_cb -#undef SSL_CTX_set_srp_missing_srp_username_callback -#define SSL_CTX_set_srp_missing_srp_username_callback \ - SSL_CTX_set_srp_miss_srp_un_cb /* Hack some long ENGINE names */ #undef ENGINE_get_default_BN_mod_exp_crt diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index 3dcf50efed..972e7923cc 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -3679,10 +3679,6 @@ long ssl3_ctx_callback_ctrl(SSL_CTX *ctx, int cmd, void (*fp)(void)) ctx->srp_ctx.srp_Mask|=SSL_kSRP; ctx->srp_ctx.SRP_give_srp_client_pwd_callback=(char *(*)(SSL *,void *))fp; break; - case SSL_CTRL_SET_TLS_EXT_SRP_MISSING_CLIENT_USERNAME_CB: - ctx->srp_ctx.srp_Mask|=SSL_kSRP; - ctx->srp_ctx.SRP_TLS_ext_missing_srp_client_username_callback=(char *(*)(SSL *,void *))fp; - break; #endif #endif case SSL_CTRL_SET_NOT_RESUMABLE_SESS_CB: diff --git a/ssl/ssl.h b/ssl/ssl.h index 92c1f43899..e7b6bc555b 100644 --- a/ssl/ssl.h +++ b/ssl/ssl.h @@ -695,8 +695,6 @@ typedef struct srp_ctx_st int (*SRP_verify_param_callback)(SSL *, void *); /* set SRP client passwd callback */ char *(*SRP_give_srp_client_pwd_callback)(SSL *, void *); - /* set SRP client username callback */ - char *(*SRP_TLS_ext_missing_srp_client_username_callback)(SSL *, void *); char *login; BIGNUM *N,*g,*s,*B,*A; @@ -1581,11 +1579,11 @@ DECLARE_PEM_rw(SSL_SESSION, SSL_SESSION) #define SSL_CTRL_SET_TLS_EXT_SRP_USERNAME_CB 75 #define SSL_CTRL_SET_SRP_VERIFY_PARAM_CB 76 #define SSL_CTRL_SET_SRP_GIVE_CLIENT_PWD_CB 77 -#define SSL_CTRL_SET_TLS_EXT_SRP_MISSING_CLIENT_USERNAME_CB 78 -#define SSL_CTRL_SET_SRP_ARG 79 -#define SSL_CTRL_SET_TLS_EXT_SRP_USERNAME 80 -#define SSL_CTRL_SET_TLS_EXT_SRP_STRENGTH 81 -#define SSL_CTRL_SET_TLS_EXT_SRP_PASSWORD 82 + +#define SSL_CTRL_SET_SRP_ARG 78 +#define SSL_CTRL_SET_TLS_EXT_SRP_USERNAME 79 +#define SSL_CTRL_SET_TLS_EXT_SRP_STRENGTH 80 +#define SSL_CTRL_SET_TLS_EXT_SRP_PASSWORD 81 #endif #define DTLS_CTRL_GET_TIMEOUT 73 diff --git a/ssl/ssltest.c b/ssl/ssltest.c index cebd4e7c8f..faffbca42c 100644 --- a/ssl/ssltest.c +++ b/ssl/ssltest.c @@ -266,12 +266,6 @@ static char * MS_CALLBACK ssl_give_srp_client_pwd_cb(SSL *s, void *arg) return BUF_strdup((char *)srp_client_arg->srppassin); } -static char * MS_CALLBACK missing_srp_username_callback(SSL *s, void *arg) - { - SRP_CLIENT_ARG *srp_client_arg = (SRP_CLIENT_ARG *)arg; - return BUF_strdup(srp_client_arg->srplogin); - } - /* SRP server */ /* This is a context that we pass to SRP server callbacks */ typedef struct srp_server_arg_st @@ -617,7 +611,6 @@ int main(int argc, char *argv[]) #endif #ifndef OPENSSL_NO_SRP /* client */ - int srp_lateuser = 0; SRP_CLIENT_ARG srp_client_arg = {NULL,NULL}; /* server */ SRP_SERVER_ARG srp_server_arg = {NULL,NULL}; @@ -1147,9 +1140,7 @@ bad: #ifndef OPENSSL_NO_SRP if (srp_client_arg.srplogin) { - if (srp_lateuser) - SSL_CTX_set_srp_missing_srp_username_callback(c_ctx,missing_srp_username_callback); - else if (!SSL_CTX_set_srp_username(c_ctx, srp_client_arg.srplogin)) + if (!SSL_CTX_set_srp_username(c_ctx, srp_client_arg.srplogin)) { BIO_printf(bio_err,"Unable to set SRP username\n"); goto end; diff --git a/ssl/tls_srp.c b/ssl/tls_srp.c index febddc7624..c363fc309c 100644 --- a/ssl/tls_srp.c +++ b/ssl/tls_srp.c @@ -4,7 +4,7 @@ * for the EdelKey project and contributed to the OpenSSL project 2004. */ /* ==================================================================== - * Copyright (c) 2004 The OpenSSL Project. All rights reserved. + * Copyright (c) 2004-2011 The OpenSSL Project. All rights reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions @@ -82,7 +82,6 @@ int SSL_CTX_SRP_CTX_free(struct ssl_ctx_st *ctx) ctx->srp_ctx.SRP_cb_arg = NULL; ctx->srp_ctx.SRP_verify_param_callback = NULL; ctx->srp_ctx.SRP_give_srp_client_pwd_callback = NULL; - ctx->srp_ctx.SRP_TLS_ext_missing_srp_client_username_callback = NULL; ctx->srp_ctx.N = NULL; ctx->srp_ctx.g = NULL; ctx->srp_ctx.s = NULL; @@ -115,7 +114,6 @@ int SSL_SRP_CTX_free(struct ssl_st *s) s->srp_ctx.SRP_cb_arg = NULL; s->srp_ctx.SRP_verify_param_callback = NULL; s->srp_ctx.SRP_give_srp_client_pwd_callback = NULL; - s->srp_ctx.SRP_TLS_ext_missing_srp_client_username_callback = NULL; s->srp_ctx.N = NULL; s->srp_ctx.g = NULL; s->srp_ctx.s = NULL; @@ -144,7 +142,6 @@ int SSL_SRP_CTX_init(struct ssl_st *s) s->srp_ctx.SRP_verify_param_callback = ctx->srp_ctx.SRP_verify_param_callback; /* set SRP client passwd callback */ s->srp_ctx.SRP_give_srp_client_pwd_callback = ctx->srp_ctx.SRP_give_srp_client_pwd_callback; - s->srp_ctx.SRP_TLS_ext_missing_srp_client_username_callback = ctx->srp_ctx.SRP_TLS_ext_missing_srp_client_username_callback; s->srp_ctx.N = NULL; s->srp_ctx.g = NULL; @@ -212,7 +209,6 @@ int SSL_CTX_SRP_CTX_init(struct ssl_ctx_st *ctx) ctx->srp_ctx.SRP_verify_param_callback = NULL; /* set SRP client passwd callback */ ctx->srp_ctx.SRP_give_srp_client_pwd_callback = NULL; - ctx->srp_ctx.SRP_TLS_ext_missing_srp_client_username_callback = NULL; ctx->srp_ctx.N = NULL; ctx->srp_ctx.g = NULL; @@ -440,16 +436,6 @@ int SRP_Calc_A_param(SSL *s) return 1; } -int SRP_have_to_put_srp_username(SSL *s) - { - if (s->srp_ctx.SRP_TLS_ext_missing_srp_client_username_callback == NULL) - return 0; - if ((s->srp_ctx.login = s->srp_ctx.SRP_TLS_ext_missing_srp_client_username_callback(s,s->srp_ctx.SRP_cb_arg)) == NULL) - return 0; - s->srp_ctx.srp_Mask|=SSL_kSRP; - return 1; - } - BIGNUM *SSL_get_srp_g(SSL *s) { if (s->srp_ctx.g != NULL) @@ -521,11 +507,4 @@ int SSL_CTX_set_srp_client_pwd_callback(SSL_CTX *ctx, char *(*cb)(SSL *,void *)) (void (*)(void))cb); } -int SSL_CTX_set_srp_missing_srp_username_callback(SSL_CTX *ctx, - char *(*cb)(SSL *,void *)) - { - return tls1_ctx_callback_ctrl(ctx, - SSL_CTRL_SET_TLS_EXT_SRP_MISSING_CLIENT_USERNAME_CB, - (void (*)(void))cb); - } #endif -- 2.34.1