mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2026-02-05 09:21:42 +01:00
Code Issues Projects Releases Wiki Activity

BUG/MINOR: ssl_ckch: Don't duplicate path when replacing a cert entry

Browse Source 
When a certificate entry is replaced (via 'set ssl cert' command), the path
is duplicated and used to identify the ongoing transaction. However, if the
same command is repeated, the path is still duplicated but the transaction
is not changed and the duplicated path is not released. Thus there is a
memory leak.

By reviewing the code, it appears there is no reason to duplicate the
path. It is always the path of the old entry. So, a reference on it is now
used. This simplifies the code and this fixes the memory leak.

This patch must be backported as far as 2.2.
This commit is contained in:
Christopher Faulet 2022-05-31 18:07:59 +02:00
parent 1f08fa46fb
commit e2ef4dd3c5
1 changed files with 3 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
src/ssl_ckch.c
View File

@ -101,7 +101,6 @@ struct commit_cert_ctx {
struct set_cert_ctx {
struct ckch_store *old_ckchs;
struct ckch_store *new_ckchs;
char *path;
};
/* CLI context used by "set ca-file" */
@ -2126,9 +2125,9 @@ static int cli_io_handler_commit_cert(struct appctx *appctx)
/* fallthrough */
case CERT_ST_FIN:
/* we achieved the transaction, we can set everything to NULL */
ha_free(&ckchs_transaction.path);
ckchs_transaction.new_ckchs = NULL;
ckchs_transaction.old_ckchs = NULL;
ckchs_transaction.path = NULL;
goto end;
}
}
@ -2326,16 +2325,6 @@ static int cli_parse_set_cert(char **args, char *payload, struct appctx *appctx,
goto end;
}
if (!ctx->path) {
/* this is a new transaction, set the path of the transaction */
ctx->path = strdup(ctx->old_ckchs->path);
if (!ctx->path) {
memprintf(&err, "%sCan't allocate memory\n", err ? err : "");
errcode |= ERR_ALERT | ERR_FATAL;
goto end;
}
}
old_ckchs = ctx->old_ckchs;
/* duplicate the ckch store */
@ -2363,7 +2352,7 @@ static int cli_parse_set_cert(char **args, char *payload, struct appctx *appctx,
/* if there wasn't a transaction, update the old ckchs */
if (!ckchs_transaction.old_ckchs) {
ckchs_transaction.old_ckchs = ctx->old_ckchs;
ckchs_transaction.path = ctx->path;
ckchs_transaction.path = ctx->old_ckchs->path;
err = memprintf(&err, "Transaction created for certificate %s!\n", ckchs_transaction.path);
} else {
err = memprintf(&err, "Transaction updated for certificate %s!\n", ckchs_transaction.path);
@ -2385,8 +2374,6 @@ end:
ckch_store_free(ctx->new_ckchs);
ctx->new_ckchs = NULL;
ctx->old_ckchs = NULL;
ha_free(&ctx->path);
HA_SPIN_UNLOCK(CKCH_LOCK, &ckch_lock);
return cli_dynerr(appctx, memprintf(&err, "%sCan't update %s!\n", err ? err : "", args[3]));
} else {
@ -2426,7 +2413,7 @@ static int cli_parse_abort_cert(char **args, char *payload, struct appctx *appct
ckch_store_free(ckchs_transaction.new_ckchs);
ckchs_transaction.new_ckchs = NULL;
ckchs_transaction.old_ckchs = NULL;
ha_free(&ckchs_transaction.path);
ckchs_transaction.path = NULL;
HA_SPIN_UNLOCK(CKCH_LOCK, &ckch_lock);