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 <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3467)
(cherry picked from commit e655f5494100d93307726b23f4718ead0cadc0c3)

diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index bbfed912aa35db5f5181791c2941ef8b70a227e6..d45a2469b5ae34e8e5b301b67347e39143f6748c 100644 (file)
--- 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 06e5e5b669263fb9865f02ae7a4c3d119f8abd73..5af08d7fec3bb10a865169060563d23eb13a432b 100644 (file)
--- 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);
@@ -52,6 +53,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);
@@ -105,7 +107,7 @@ int SSL_SRP_CTX_init(struct ssl_st *s)
     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.info = NULL;
     s->srp_ctx.strength = ctx->srp_ctx.strength;
 
     if (((ctx->srp_ctx.N != NULL) &&
@@ -132,11 +134,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);
@@ -272,7 +280,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))