[openssl-dev] [openssl.org #4518] OpenSSL-1.1.0-pre5 RSA_set0_key and related RSA_get0_*, RSA_set0_*, DSA_set0_* and DSA_get0_* problems

[Blumenthal, Uri - 0553 - MITLL]

On 4/26/16, 15:15 , "openssl-dev on behalf of Viktor Dukhovni"
<openssl-dev-bounces at openssl.org on behalf of openssl-users at dukhovni.org>
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
>> 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
>silently ignore the mistake.

I’m very certain that this test should be done.

What’s the correct behavior if/when the caller is “caught” doing the wrong
thing - I leave to you guys to decide, as your experience and
understanding of these API is better than mine.

>There may be other pointers that the caller does not own that he
>might be tempted to pass into these functions, say get0 the data
>from one RSA object and attempt to set0 the same parameters on
>another.

I hear you… :-(

>The only systemic fix is much more complex.  We'd need to manage
>and set "library-owned" boolean fields in all the structures returned
>by get0 functions and refuse to accept these in "set0" functions.
>
>Thus a new() constructor would produce a caller owned structure,
>as would a get1() accessor, but a get0() access would return a
>library owned structure, which would be unsuitable for a set0()
>call that takes ownership.

Right now get0() returns a library-owned structure. But there isn’t a
get1() accessor (unless I’m too tired to search correctly :).

>Implementing this pattern would be a major overhaul of the library.

I hear you.

>For now, yes we could detect just one class of mistake, but I
>don't think we should "correct" the mistake, rather we should
>report it as such to the caller.

I think that detecting just one class of mistake makes one mistake class
less to worry about. So it would be great to catch at least this one class
for now.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4324 bytes
Desc: not available
URL: <http://mta.openssl.org/pipermail/openssl-dev/attachments/20160426/60f655e5/attachment.bin>