The branch OpenSSL_1_1_0-stable has been updated via 3ff170c6a137c330bb23731f1193c5005e9b85a9 (commit) via 0f2e444ee5cf1a2de68ab8edf2df376512f2508f (commit) from f389bb691130c5e393d9b35fa5961cceda00093f (commit)
- Log ----------------------------------------------------------------- commit 3ff170c6a137c330bb23731f1193c5005e9b85a9 Author: Diego Santa Cruz <diego.santac...@spinetix.com> Date: Tue May 16 10:35:49 2017 +0200 Use memset to clear SRP_CTX instead of NULL and zero assignments This uses memset() to clear all of the SRP_CTX when free'ing or initializing it as well as in error paths instead of having a series of NULL and zero assignments as it is safer. It also changes SSL_SRP_CTX_init() to reset all the SRP_CTX to zero in case or error, previously it could retain pointers to freed memory, potentially leading to a double free. Reviewed-by: Rich Salz <rs...@openssl.org> Reviewed-by: Matt Caswell <m...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/3467) (cherry picked from commit 135976b3dd24e674c202c20b5746fc04ebb1fc1a) commit 0f2e444ee5cf1a2de68ab8edf2df376512f2508f Author: Diego Santa Cruz <diego.santac...@spinetix.com> Date: Mon May 15 10:35:45 2017 +0200 Make SRP_CTX.info ownership and lifetime be the same as SRP_CTX.login. Ownership and lifetime rules of SRP_CTX.info are confusing and different from those of SRP_CTX.login, making it difficult to use correctly. This makes the ownership and lifetime be the same as those of SRP_CTX.login, thet is a copy is made when setting it and is freed when SRP_CTX is freed. Reviewed-by: Rich Salz <rs...@openssl.org> Reviewed-by: Matt Caswell <m...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/3467) (cherry picked from commit e655f5494100d93307726b23f4718ead0cadc0c3) ----------------------------------------------------------------------- Summary of changes: ssl/s3_lib.c | 7 +++++- ssl/tls_srp.c | 81 ++++++++++++++++------------------------------------------- 2 files changed, 27 insertions(+), 61 deletions(-) diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index bbfed91..d45a246 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -3384,7 +3384,12 @@ long ssl3_ctx_ctrl(SSL_CTX *ctx, int cmd, long larg, void *parg) case SSL_CTRL_SET_TLS_EXT_SRP_PASSWORD: ctx->srp_ctx.SRP_give_srp_client_pwd_callback = srp_password_from_info_cb; - ctx->srp_ctx.info = parg; + if (ctx->srp_ctx.info != NULL) + OPENSSL_free(ctx->srp_ctx.info); + if ((ctx->srp_ctx.info = BUF_strdup((char *)parg)) == NULL) { + SSLerr(SSL_F_SSL3_CTX_CTRL, ERR_R_INTERNAL_ERROR); + return 0; + } break; case SSL_CTRL_SET_SRP_ARG: ctx->srp_ctx.srp_Mask |= SSL_kSRP; diff --git a/ssl/tls_srp.c b/ssl/tls_srp.c index 06e5e5b..bfdbdf5 100644 --- a/ssl/tls_srp.c +++ b/ssl/tls_srp.c @@ -20,6 +20,7 @@ int SSL_CTX_SRP_CTX_free(struct ssl_ctx_st *ctx) if (ctx == NULL) return 0; OPENSSL_free(ctx->srp_ctx.login); + OPENSSL_free(ctx->srp_ctx.info); BN_free(ctx->srp_ctx.N); BN_free(ctx->srp_ctx.g); BN_free(ctx->srp_ctx.s); @@ -28,22 +29,8 @@ int SSL_CTX_SRP_CTX_free(struct ssl_ctx_st *ctx) BN_free(ctx->srp_ctx.a); BN_free(ctx->srp_ctx.b); BN_free(ctx->srp_ctx.v); - ctx->srp_ctx.TLS_ext_srp_username_callback = NULL; - 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.N = NULL; - ctx->srp_ctx.g = NULL; - ctx->srp_ctx.s = NULL; - ctx->srp_ctx.B = NULL; - ctx->srp_ctx.A = NULL; - ctx->srp_ctx.a = NULL; - ctx->srp_ctx.b = NULL; - ctx->srp_ctx.v = NULL; - ctx->srp_ctx.login = NULL; - ctx->srp_ctx.info = NULL; + memset(&ctx->srp_ctx, 0, sizeof(ctx->srp_ctx)); ctx->srp_ctx.strength = SRP_MINIMAL_N; - ctx->srp_ctx.srp_Mask = 0; return (1); } @@ -52,6 +39,7 @@ int SSL_SRP_CTX_free(struct ssl_st *s) if (s == NULL) return 0; OPENSSL_free(s->srp_ctx.login); + OPENSSL_free(s->srp_ctx.info); BN_free(s->srp_ctx.N); BN_free(s->srp_ctx.g); BN_free(s->srp_ctx.s); @@ -60,22 +48,8 @@ int SSL_SRP_CTX_free(struct ssl_st *s) BN_free(s->srp_ctx.a); BN_free(s->srp_ctx.b); BN_free(s->srp_ctx.v); - s->srp_ctx.TLS_ext_srp_username_callback = NULL; - 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.N = NULL; - s->srp_ctx.g = NULL; - s->srp_ctx.s = NULL; - s->srp_ctx.B = NULL; - s->srp_ctx.A = NULL; - s->srp_ctx.a = NULL; - s->srp_ctx.b = NULL; - s->srp_ctx.v = NULL; - s->srp_ctx.login = NULL; - s->srp_ctx.info = NULL; + memset(&s->srp_ctx, 0, sizeof(s->srp_ctx)); s->srp_ctx.strength = SRP_MINIMAL_N; - s->srp_ctx.srp_Mask = 0; return (1); } @@ -85,6 +59,9 @@ int SSL_SRP_CTX_init(struct ssl_st *s) if ((s == NULL) || ((ctx = s->ctx) == NULL)) return 0; + + memset(&s->srp_ctx, 0, sizeof(s->srp_ctx)); + s->srp_ctx.SRP_cb_arg = ctx->srp_ctx.SRP_cb_arg; /* set client Hello login callback */ s->srp_ctx.TLS_ext_srp_username_callback = @@ -96,16 +73,6 @@ int SSL_SRP_CTX_init(struct ssl_st *s) s->srp_ctx.SRP_give_srp_client_pwd_callback = ctx->srp_ctx.SRP_give_srp_client_pwd_callback; - s->srp_ctx.N = NULL; - s->srp_ctx.g = NULL; - s->srp_ctx.s = NULL; - s->srp_ctx.B = NULL; - s->srp_ctx.A = NULL; - s->srp_ctx.a = NULL; - s->srp_ctx.b = NULL; - s->srp_ctx.v = NULL; - s->srp_ctx.login = NULL; - s->srp_ctx.info = ctx->srp_ctx.info; s->srp_ctx.strength = ctx->srp_ctx.strength; if (((ctx->srp_ctx.N != NULL) && @@ -132,11 +99,17 @@ int SSL_SRP_CTX_init(struct ssl_st *s) SSLerr(SSL_F_SSL_SRP_CTX_INIT, ERR_R_INTERNAL_ERROR); goto err; } + if ((ctx->srp_ctx.info != NULL) && + ((s->srp_ctx.info = BUF_strdup(ctx->srp_ctx.info)) == NULL)) { + SSLerr(SSL_F_SSL_SRP_CTX_INIT, ERR_R_INTERNAL_ERROR); + goto err; + } s->srp_ctx.srp_Mask = ctx->srp_ctx.srp_Mask; return (1); err: OPENSSL_free(s->srp_ctx.login); + OPENSSL_free(s->srp_ctx.info); BN_free(s->srp_ctx.N); BN_free(s->srp_ctx.g); BN_free(s->srp_ctx.s); @@ -145,6 +118,7 @@ int SSL_SRP_CTX_init(struct ssl_st *s) BN_free(s->srp_ctx.a); BN_free(s->srp_ctx.b); BN_free(s->srp_ctx.v); + memset(&s->srp_ctx, 0, sizeof(s->srp_ctx)); return (0); } @@ -153,25 +127,7 @@ int SSL_CTX_SRP_CTX_init(struct ssl_ctx_st *ctx) if (ctx == NULL) return 0; - ctx->srp_ctx.SRP_cb_arg = NULL; - /* set client Hello login callback */ - ctx->srp_ctx.TLS_ext_srp_username_callback = NULL; - /* set SRP N/g param callback for verification */ - 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.N = NULL; - ctx->srp_ctx.g = NULL; - ctx->srp_ctx.s = NULL; - ctx->srp_ctx.B = NULL; - ctx->srp_ctx.A = NULL; - ctx->srp_ctx.a = NULL; - ctx->srp_ctx.b = NULL; - ctx->srp_ctx.v = NULL; - ctx->srp_ctx.login = NULL; - ctx->srp_ctx.srp_Mask = 0; - ctx->srp_ctx.info = NULL; + memset(&ctx->srp_ctx, 0, sizeof(ctx->srp_ctx)); ctx->srp_ctx.strength = SRP_MINIMAL_N; return (1); @@ -272,7 +228,12 @@ int SSL_set_srp_server_param(SSL *s, const BIGNUM *N, const BIGNUM *g, } else s->srp_ctx.v = BN_dup(v); } - s->srp_ctx.info = info; + if (info != NULL) { + if (s->srp_ctx.info) + OPENSSL_free(s->srp_ctx.info); + if ((s->srp_ctx.info = BUF_strdup(info)) == NULL) + return -1; + } if (!(s->srp_ctx.N) || !(s->srp_ctx.g) || !(s->srp_ctx.s) || !(s->srp_ctx.v)) _____ openssl-commits mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-commits