Freeing of the values by the caller is not the issue.
The issue is RSA_set0_key requires n and e to be none NULL.
It the caller use RSA_get0_key to find the n and e then calculates a new d,
than calls RSA_set0_key with the the same n and e pointers and the new d.
RSA_set0_key will free n and e, and replace the pointer with the same pointer
which just got freed.
An untested patch for rsa_lib.c is attached
DSA has the same problems. Are there other new modules that may have the same
issue?
On 4/25/2016 8:08 AM, Richard Levitte via RT wrote:
In message <6b097acbe9d94724ac545f2529e45...@usma1ex-dag1mb1.msg.corp.akamai.com> on Mon, 25
Apr 2016 11:38:47 +0000, "Salz, Rich" <[email protected]> said:
rsalz> > If nothing else, all the RSA_set0 routines should test if the same
pointer
rsalz> > value is being replaced if so do not free it.
rsalz> >
rsalz> > The same logic need to be done for all the RSA_set0_* functions as
well as
rsalz> > the DSA_set0_* functions.
rsalz>
rsalz> That seems like a bug we should fix.
No, it's by design:
: ; perldoc doc/crypto/RSA_get0_key.pod
...
The n, e and d parameter values can be set by calling RSA_set0_key()
and
passing the new values for n, e and d as parameters to the function.
Calling this function transfers the memory management of the values to
the
RSA object, and therefore the values that have been passed in should
not
be freed by the caller after this function has been called.
...
: ; perldoc doc/crypto/DSA_get0_pqg.pod
...
The p, q and g values can be set by calling DSA_set0_pqg() and passing
the
new values for p, q and g as parameters to the function. Calling this
function transfers the memory management of the values to the DSA
object,
and therefore the values that have been passed in should not be freed
directly after this function has been called.
...
Cheers,
Richard
--
Douglas E. Engert <[email protected]>
diff --git a/crypto/rsa/rsa_lib.c b/crypto/rsa/rsa_lib.c
index 7ee575d..f106cf4 100644
--- a/crypto/rsa/rsa_lib.c
+++ b/crypto/rsa/rsa_lib.c
@@ -290,9 +290,12 @@ int RSA_set0_key(RSA *r, BIGNUM *n, BIGNUM *e, BIGNUM *d)
if (n == NULL || e == NULL)
return 0;
- BN_free(r->n);
- BN_free(r->e);
- BN_free(r->d);
+ if (r->n != n)
+ BN_free(r->n);
+ if (r->e != e)
+ BN_free(r->e);
+ if (r->d != d)
+ BN_free(r->d);
r->n = n;
r->e = e;
r->d = d;
@@ -305,8 +308,10 @@ int RSA_set0_factors(RSA *r, BIGNUM *p, BIGNUM *q)
if (p == NULL || q == NULL)
return 0;
- BN_free(r->p);
- BN_free(r->q);
+ if (r->p != p)
+ BN_free(r->p);
+ if (r->q != q)
+ BN_free(r->q);
r->p = p;
r->q = q;
@@ -318,9 +323,12 @@ int RSA_set0_crt_params(RSA *r, BIGNUM *dmp1, BIGNUM *dmq1, BIGNUM *iqmp)
if (dmp1 == NULL || dmq1 == NULL || iqmp == NULL)
return 0;
- BN_free(r->dmp1);
- BN_free(r->dmq1);
- BN_free(r->iqmp);
+ if (r->dmp1 != dmp1)
+ BN_free(r->dmp1);
+ if (r->dmq1 != dmq1)
+ BN_free(r->dmq1);
+ if (r->iqmp != iqmp)
+ BN_free(r->iqmp);
r->dmp1 = dmp1;
r->dmq1 = dmq1;
r->iqmp = iqmp;
--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
