acme-client(1): don't leak RSA keys

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

acme-client(1): don't leak RSA keys

Florian Obser-2
I'm not sure why we use get1 and not get0.
get0 should be ok here, no?

Someone in the know please apply a clue bat, thanks.

diff --git acctproc.c acctproc.c
index 09ac8c83372..fd0a22629db 100644
--- acctproc.c
+++ acctproc.c
@@ -86,6 +86,7 @@ op_thumb_rsa(EVP_PKEY *pkey)
  else if ((json = json_fmt_thumb_rsa(exp, mod)) == NULL)
  warnx("json_fmt_thumb_rsa");
 
+ RSA_free(r);
  free(exp);
  free(mod);
  return json;
@@ -219,6 +220,7 @@ op_sign_rsa(char **prot, EVP_PKEY *pkey, const char *nonce, const char *url)
  else
  rc = 1;
 
+ RSA_free(r);
  free(exp);
  free(mod);
  return rc;


--
I'm not entirely sure you are real.

Reply | Threaded
Open this post in threaded view
|

Re: acme-client(1): don't leak RSA keys

Theo Buehler-4
On Sun, Jun 16, 2019 at 10:05:55AM +0200, Florian Obser wrote:
> I'm not sure why we use get1 and not get0.
> get0 should be ok here, no?
>
> Someone in the know please apply a clue bat, thanks.

Since the references are local to the functions and there are no
threads, no-one's going to invalidate the reference underneath you,
so get0 (without matching RSA_free()) would be ok.

Your diff is ok, too.

>
> diff --git acctproc.c acctproc.c
> index 09ac8c83372..fd0a22629db 100644
> --- acctproc.c
> +++ acctproc.c
> @@ -86,6 +86,7 @@ op_thumb_rsa(EVP_PKEY *pkey)
>   else if ((json = json_fmt_thumb_rsa(exp, mod)) == NULL)
>   warnx("json_fmt_thumb_rsa");
>  
> + RSA_free(r);
>   free(exp);
>   free(mod);
>   return json;
> @@ -219,6 +220,7 @@ op_sign_rsa(char **prot, EVP_PKEY *pkey, const char *nonce, const char *url)
>   else
>   rc = 1;
>  
> + RSA_free(r);
>   free(exp);
>   free(mod);
>   return rc;
>
>
> --
> I'm not entirely sure you are real.
>