OK, Thanks.
On 4/27/2016 11:31 PM, Richard Levitte wrote:
In message <5720fd7d.3050...@gmail.com> on Wed, 27 Apr 2016 12:57:17 -0500, Douglas E
Engert said:
deengert> You can call it a documentation problem. The problem only showed up
deengert> with trying to update d
In message <5720fd7d.3050...@gmail.com> on Wed, 27 Apr 2016 12:57:17 -0500,
Douglas E Engert said:
deengert> You can call it a documentation problem. The problem only showed up
deengert> with trying to update d
deengert> in an existing rsa key. RSA_set0_key requires n, e,
You can call it a documentation problem. The problem only showed up with trying
to update d
in an existing rsa key. RSA_set0_key requires n, e, and d == NULL OR n, e,
and d to all be set at the same time.
(In the case I found, one routine created the key with only n and e, then d was
added
After quite a lot of discussion, we finally came to a solution. Commits
1da12e34ed69cec206f3a251a1e62ceeb694a6ea and
4c5e6b2cb95a4332829af140e5edba965c9685ce
That closes this ticket.
Cheers,
Richard
--
Richard Levitte
levi...@openssl.org
--
Ticket here:
In message <571fccee.8010...@roumenpetrov.info> on Tue, 26 Apr 2016 23:17:50
+0300, Roumen Petrov said:
openssl> For protocol "0009-sshkey.c-opaque-DSA-structure.patch" is practical
openssl> sample of an upgrade to 1.1 API. RSA is similar.
A quick side remark: check
On Út, 2016-04-26 at 18:25 +, Blumenthal, Uri - 0553 - MITLL wrote:
> On 4/26/16, 14:20 , "openssl-dev on behalf of Salz, Rich"
>
> wrote:
>
> >
> > >
> > > Look. If Doug noticed this, programmers less intimate with this
> > >
In message <571fccee.8010...@roumenpetrov.info> on Tue, 26 Apr 2016 23:17:50
+0300, Roumen Petrov said:
openssl> Hello Richard,
openssl>
openssl> Richard Levitte wrote:
openssl> > In message <571f2941.4040...@openssl.org> on Tue, 26 Apr 2016
09:39:29
openssl> >
Hello Richard,
Richard Levitte wrote:
In message <571f2941.4040...@openssl.org> on Tue, 26 Apr 2016 09:39:29 +0100, Matt
Caswell said:
[SNIP]
I've seen no other opinion, so I went with it. Would you mind having
a look at GH#995? I did a bit of change in the docs, but
On 4/26/2016 1:20 PM, Salz, Rich wrote:
Look. If Doug noticed this, programmers less intimate with this API are much
more likely to get stung by it. The protection against such a misunderstanding
is cheap.
Is it? And what is that protection? Without introducing memory leaks.
In
On 4/26/16, 15:15 , "openssl-dev on behalf of Viktor Dukhovni"
wrote:
>On Tue, Apr 26, 2016 at 12:55:28PM -0500, Douglas E Engert wrote:
>> Adding the test "if (n != rsa->n)" before the BN_free in the
>>RSA_set0_key
>>
On Tue, Apr 26, 2016 at 12:55:28PM -0500, Douglas E Engert wrote:
> Adding the test "if (n != rsa->n)" before the BN_free in the RSA_set0_key
> would catch this.
The correct test is to return an error in that case, not to skip
the free. The caller is doing the wrong thing, and we should not
On 4/26/16, 14:20 , "openssl-dev on behalf of Salz, Rich"
wrote:
>> Look. If Doug noticed this, programmers less intimate with this API are
>>much
>> more likely to get stung by it. The protection against such a
>>misunderstanding
>>
> Look. If Doug noticed this, programmers less intimate with this API are much
> more likely to get stung by it. The protection against such a misunderstanding
> is cheap.
Is it? And what is that protection? Without introducing memory leaks.
--
openssl-dev mailing list
To unsubscribe:
On 4/26/16, 14:03 , "openssl-dev on behalf of Salz, Rich via RT"
wrote:
>That code is still wrong. Once you "get0" something you can only look at
>it. You cannot pass it off to a "set0" function. Get0 gives you a
>pointer that
That code is still wrong. Once you "get0" something you can only look at it.
You cannot pass it off to a "set0" function. Get0 gives you a pointer that
*you do not own* and *set0* takes a pointer that you DO own and are giving
away. You can't give away something that isn't yours :)
The
On 4/26/16, 13:56 , "openssl-dev on behalf of Douglas E Engert"
wrote:
>...
>RSA_get0_key(rsa, _n, _e, NULL); /* note this is a GET0 */
>
>/* my_n now points to the BIGNUM as does rsa->n */
>/* my_e now points to the BIGNUM as does
OK there was an error in my example. The get needed 2 "&":
RSA_get0_key(rsa, _n, _e, NULL); /* note this is a GET0 */
/* my_n now points to the BIGNUM as does rsa->n */
/* my_e now points to the BIGNUM as does rsa->e */
/* other stuff done, such as calculating d */
RSA_set0_key(rsa, my_n,
Yes, there was an error in my example, the first line should have read:
RSA_get0_key(rsa, , , NULL);
The rsa was created in a different routine, so n and e were already set.
I am not the one freeing it is your RSA_set0_key that is doing the free.
Adding the test "if (n != rsa->n)" before the
On 4/26/16, 11:43 , "openssl-dev on behalf of Tomas Mraz"
wrote:
>On Út, 2016-04-26 at 10:16 -0500, Douglas E Engert wrote:
>> Let me update my response.
>> If I am reading GH#995 correctly it still has an issue if a user
>> does:
>>
On Út, 2016-04-26 at 10:16 -0500, Douglas E Engert wrote:
> Let me update my response.
> If I am reading GH#995 correctly it still has an issue if a user
> does:
>
> RSA_get0_key(rsa, n, e, NULL); /* note this is a GET0 */
> /* other stuff done, such as calculating d */
> RSA_set0_key(rsa, n, e,
On 4/26/16, 11:21 , "openssl-dev on behalf of Salz, Rich via RT"
wrote:
>> RSA_get0_key(rsa, n, e, NULL); /* note this is a GET0 */
>> /* other stuff done, such as calculating d */
>>RSA_set0_key(rsa, n, e, d);
>>
>> rsa is left
On 26/04/16 16:16, Douglas E Engert wrote:
> Let me update my response.
> If I am reading GH#995 correctly it still has an issue if a user does:
>
> RSA_get0_key(rsa, n, e, NULL); /* note this is a GET0 */
> /* other stuff done, such as calculating d */
> RSA_set0_key(rsa, n, e, d);
>
> rsa is
> RSA_get0_key(rsa, n, e, NULL); /* note this is a GET0 */
> /* other stuff done, such as calculating d */ RSA_set0_key(rsa, n, e, d);
>
> rsa is left with n and e pointing to unallocated storage.
That code is incorrect.
--
Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4518
Please
> I can live with it.
> The only solution without some type of change was :
>
> RSA_set0_key(rsa, n, e, NULL);
> /* other stuff done, such as calculating d */
> n_new = BN_dup(n);
> e_new = BN_dup(e);
> RSA_set0_key(rsa, n_new, e_new, d);
>
> It is really gross, and is
Let me update my response.
If I am reading GH#995 correctly it still has an issue if a user does:
RSA_get0_key(rsa, n, e, NULL); /* note this is a GET0 */
/* other stuff done, such as calculating d */
RSA_set0_key(rsa, n, e, d);
rsa is left with n and e pointing to unallocated storage.
On
-pre5 RSA_set0_key
and related RSA_get0_*, RSA_set0_*, DSA_set0_* and DSA_get0_* problems
In message <571f2941.4040...@openssl.org> on Tue, 26 Apr 2016 09:39:29 +0100,
Matt Caswell <m...@openssl.org> said:
matt>
matt>
matt> On 26/04/16 08:26, Richard Levitte wrote:
matt&
In message <571f2941.4040...@openssl.org> on Tue, 26 Apr 2016 09:39:29 +0100,
Matt Caswell said:
matt>
matt>
matt> On 26/04/16 08:26, Richard Levitte wrote:
matt> > [temporarly taking this thread away from RT]
matt> >
matt> > Basically, I can see two solutions:
matt> >
On 26/04/16 08:26, Richard Levitte wrote:
> [temporarly taking this thread away from RT]
>
> Basically, I can see two solutions:
>
> - Allow calls like RSA_set0_key(rsa, NULL, NULL, d);
>
> That's what's implemented in GH#995, except it doesn't check if the
> input parameters are NULL
[temporarly taking this thread away from RT]
Basically, I can see two solutions:
- Allow calls like RSA_set0_key(rsa, NULL, NULL, d);
That's what's implemented in GH#995, except it doesn't check if the
input parameters are NULL before setting the corresponding fields,
so that call ends up
Unfortunately, the solution in that PR is flawed. Back to the drawing board.
Vid Mon, 25 apr 2016 kl. 18.39.24, skrev levitte:
> So, listening to what everyone had to say, perhaps this PR is better
> then:
>
> https://github.com/openssl/openssl/pull/995
>
> In message
So, listening to what everyone had to say, perhaps this PR is better
then:
https://github.com/openssl/openssl/pull/995
In message
on Mon,
25 Apr 2016 17:45:05 +, "Salz, Rich" said:
rsalz>
rsalz> >
On Mon, Apr 25, 2016 at 05:45:05PM +, Salz, Rich wrote:
> After a "set0" call, set your pointer to NULL, it's no longer yours :)
That half of the ruleset. The other half is:
You must "own" any object passed to a set0 call that takes
ownership of its argument (we have a few that don't
> The 3-slot function is I think cleaner.
>
> I'll leave the decision of whether and when to support NULL parameters to
> the folks working on that code, but it is pretty clear that one must not pass
> an
> object one does not "own", such as one returned from a "get0" function, to a
> function
> Yes, but this difference adds convenience, IMHO. My preference is this:
> RSA_set0_key(rsa, n, e, d); with any parameter (except for rsa :) potentially
> NULL.
This defeats a main point: partial construction is a bad thing.
--
Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4518
On Mon, Apr 25, 2016 at 07:21:56PM +0200, Richard Levitte wrote:
> openssl-users> Perhaps the documentation can be made more clear. If users
> really
> openssl-users> need an interface for modifying a subset of the components of
> an
> openssl-users> already initialized key, then (if we don't
>There isn't much difference between this:
>
>RSA_set0_key(rsa, n, NULL, NULL);
>RSA_set0_key(rsa, NULL, e, NULL);
>RSA_set0_key(rsa, NULL, NULL, d);
>
>and something like this:
>
>RSA_set0_n(rsa, n);
>RSA_set0_e(rsa, e);
>RSA_set0_d(rsa, d);
The attractiveness of
In message <20160425141410.gx26...@mournblade.imrryr.org> on Mon, 25 Apr 2016
14:14:10 +, Viktor Dukhovni said:
openssl-users> Perhaps the documentation can be made more clear. If users
really
openssl-users> need an interface for modifying a subset of the
On Mon, Apr 25, 2016 at 02:08:09PM +, Richard Levitte via RT wrote:
> I'm not sure how I'd change the following:
>
> 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
On Mon, Apr 25, 2016 at 01:39:09PM +, Richard Levitte via RT wrote:
> rt> I agree it shouldn't happen, but do we want to protect against that? I
> could be convinced either way.
>
> Ah ok... sorry, I misread the intention.
>
> Agreed that we could make sure not to free the pointers in
In message on Mon, 25
Apr 2016 14:04:27 +, Tomas Mraz via RT said:
rt> On Po, 2016-04-25 at 13:39 +, Richard Levitte via RT wrote:
rt> > In message on
rt> >
On Po, 2016-04-25 at 13:39 +, Richard Levitte via RT wrote:
> In message on
> Mon, 25 Apr 2016 13:19:38 +, "Salz, Rich via RT"
> said:
>
> rt> No, he means setting the same value twice. For example, making
> this
On Po, 2016-04-25 at 13:39 +, Richard Levitte via RT wrote:
> In message on
> Mon, 25 Apr 2016 13:19:38 +, "Salz, Rich via RT"
> said:
>
> rt> No, he means setting the same value twice. For example, making
> this
I believe this PR fixes the issue for RSA, DSA and DH (they all share
the same concept).
https://github.com/openssl/openssl/pull/994
Cheers,
Richard
In message on Mon, 25 Apr
2016 13:39:09 +, Richard Levitte via RT
In message on Mon, 25
Apr 2016 13:19:38 +, "Salz, Rich via RT" said:
rt> No, he means setting the same value twice. For example, making this change:
rt> If (r=->n != n) BN_free(r->n);
rt> If(r->e != e)
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,
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,
On Po, 2016-04-25 at 13:08 +, Richard Levitte via RT wrote:
>
> 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_*
>
On Po, 2016-04-25 at 13:08 +, Richard Levitte via RT wrote:
>
> 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_*
>
No, he means setting the same value twice. For example, making this change:
If (r=->n != n) BN_free(r->n);
If(r->e != e) BN_free(r->e);
If (r->d != d) BN_free(r->d);
I agree it shouldn't happen, but do we want to protect against that? I could
be convinced either way.
--
Ticket
No, he means setting the same value twice. For example, making this change:
If (r=->n != n) BN_free(r->n);
If(r->e != e) BN_free(r->e);
If (r->d != d) BN_free(r->d);
I agree it shouldn't happen, but do we want to protect against that? I could
be convinced either way.
--
In message
<6b097acbe9d94724ac545f2529e45...@usma1ex-dag1mb1.msg.corp.akamai.com> on Mon,
25 Apr 2016 11:38:47 +, "Salz, Rich" 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.
> Would not a set of routines like:
> BIGNUM* RSA_get0_key_n(RSA *rsa);
> int RSA_set0_key_n(RSA *rsa, BIGNUM *n); (A set for: n, e, d, p, q, idmp1,
> idmq1, iqmp) be much more backward compatible?
We had discussed this in the team, and decided that it was better to have a
single API that took
> Would not a set of routines like:
> BIGNUM* RSA_get0_key_n(RSA *rsa);
> int RSA_set0_key_n(RSA *rsa, BIGNUM *n); (A set for: n, e, d, p, q, idmp1,
> idmq1, iqmp) be much more backward compatible?
We had discussed this in the team, and decided that it was better to have a
single API that took
The new routines in OpenSSL-1.1.0-pre5 RSA_get0_key and RSA_set0_key with their
multiple
arguments are not very user friendly requiring much more code being replaced
and may lead to freeing
an active pointers.
Would not a set of routines like:
BIGNUM* RSA_get0_key_n(RSA *rsa);
int
54 matches
Mail list logo