Re: [PATCH] MINOR: ssl: check transaction path before assigning it
On Sun, Nov 24, 2019 at 12:15 AM William Lallemand wrote: > That's a remain of the previous way of doing this, which was done with an > array of 2 old_ckch, so the previous check was something like: > > > if (!old_ckchs[0] && !old_ckchs[1]) > > When a transaction is created the old_ckchs and the path are set. > So we can check the path OR the old_ckchs, but it's not needed to do both, > since there are both set and free during a transaction at the same time. > > The goal there was to check if a transaction exists, not to check if we free'd > the path before setting it. ok got it, thanks for the explanation. In that case I will send a cleanup patch shortly, but I let you decide whether it is worth merging it or wait for another rework to integrate it. -- William
Re: [PATCH] MINOR: ssl: check transaction path before assigning it
On Sat, Nov 23, 2019 at 10:44:31PM +0100, William Dauchy wrote:
> we were checking two times old_ckchs where we wanted to check whether
> the transaction path was freed (and so null) before
>
> this should fix issue #351
>
> Reported-by: Илья Шипицин
> Signed-off-by: William Dauchy
> ---
> src/ssl_sock.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/ssl_sock.c b/src/ssl_sock.c
> index bcfa3e71..4cdb795f 100644
> --- a/src/ssl_sock.c
> +++ b/src/ssl_sock.c
> @@ -10468,7 +10468,7 @@ static int cli_parse_set_cert(char **args, char
> *payload, struct appctx *appctx,
> /* we succeed, we can save the ckchs in the transaction */
>
> /* if there wasn't a transaction, update the old ckchs */
> - if (!ckchs_transaction.old_ckchs && !ckchs_transaction.old_ckchs) {
> + if (!ckchs_transaction.old_ckchs && !ckchs_transaction.path) {
> ckchs_transaction.old_ckchs = appctx->ctx.ssl.old_ckchs;
> ckchs_transaction.path = appctx->ctx.ssl.path;
> err = memprintf(, "Transaction created for certificate
> %s!\n", ckchs_transaction.path);
That's a remain of the previous way of doing this, which was done with an
array of 2 old_ckch, so the previous check was something like:
> if (!old_ckchs[0] && !old_ckchs[1])
When a transaction is created the old_ckchs and the path are set.
So we can check the path OR the old_ckchs, but it's not needed to do both,
since there are both set and free during a transaction at the same time.
The goal there was to check if a transaction exists, not to check if we free'd
the path before setting it.
--
William Lallemand
[PATCH] MINOR: ssl: check transaction path before assigning it
we were checking two times old_ckchs where we wanted to check whether
the transaction path was freed (and so null) before
this should fix issue #351
Reported-by: Илья Шипицин
Signed-off-by: William Dauchy
---
src/ssl_sock.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index bcfa3e71..4cdb795f 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -10468,7 +10468,7 @@ static int cli_parse_set_cert(char **args, char
*payload, struct appctx *appctx,
/* we succeed, we can save the ckchs in the transaction */
/* if there wasn't a transaction, update the old ckchs */
- if (!ckchs_transaction.old_ckchs && !ckchs_transaction.old_ckchs) {
+ if (!ckchs_transaction.old_ckchs && !ckchs_transaction.path) {
ckchs_transaction.old_ckchs = appctx->ctx.ssl.old_ckchs;
ckchs_transaction.path = appctx->ctx.ssl.path;
err = memprintf(, "Transaction created for certificate
%s!\n", ckchs_transaction.path);
--
2.24.0
