[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

[Douglas E Engert]

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 4/26/2016 10:02 AM, Douglas E Engert wrote:
> 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 not intuitive.
>
> Since you all appear to not want to support individual calls to set0 and get0 for each BIGNUM,
> a developer of other code is faced with a major rewrite of existing code just to work with OpenSSL-1.1.0-pre5.
>
>
> Using #if OPENSSL_VERSION_NUMBER everywhere leads to unreadable code. So to maintain a code base that can be compiled
> with OpenSSL version 0.9.7 through 1.1.0 with only a few #if OPENSSL_VERSION_NUMBER we are taking an approach to convert
> the code to the 1.1.0 API and create defines and macros for the older versions of OpenSSL in a header file
> The introduction of these *_get0_* *_set0_* have complicated the process even more, requiring us to inline versions
> of them for the older versions of OpenSSL.
>
> I suspect other developers are facing the same issues.
>
> On 4/26/2016 6:46 AM, Richard Levitte wrote:
>> In message <571F2941.4040004 at openssl.org> on Tue, 26 Apr 2016 09:39:29 +0100, Matt Caswell <matt at openssl.org> 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> >
>> matt> > - Allow calls like RSA_set0_key(rsa, NULL, NULL, d);
>> matt> >
>> matt> >   That's what's implemented in GH#995, except it doesn't check if the
>> matt> >   input parameters are NULL before setting the corresponding fields,
>> matt> >   so that call ends up clearing n and e.
>> matt> >
>> matt> >   GH#995 could be changed so that any input parameter can be NULL, and
>> matt> >   that the corresponding RSA structure fields are left untouched.  The
>> matt> >   consequence is that can never be made NULL.  I can live with that,
>> matt> >   as I can't imagine a reason to reset the fields to NULL.
>> matt>
>> matt> IMO this is the way to go. As long as we can't set private key values
>> matt> without first having set the public key, i.e. we should not be able to
>> matt> get into an inconsistent state.
>>
>> 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 could need
>> some help expressing it in a better manner.
>>
>> Also, I'd like to hear from Douglas and Tomas if these changes found
>> in said pull request would fit your bill better...  basically, it
>> allows (or should allow, unless I've goofed something up) a call set
>> like this:
>>
>>      RSA_set0_key(rsa, n, e, NULL);
>>      /* other stuff done, such as calculatig d */
>>      RSA_set0_key(rsa, NULL, NULL, d);
>>
>> Cheers,
>> Richard
>>
>

-- 

  Douglas E. Engert  <DEEngert at gmail.com>