[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

[Viktor Dukhovni]

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.

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.

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.

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

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.

-- 
	Viktor.