Vote proposal: Private keys can exist independently of public keys

[Nicola Tuveri]

On Wed, 7 Oct 2020 at 20:45, Richard Levitte <levitte at openssl.org> wrote:
>
> I'm as culpable as anyone re pushing the "convention" that an EVP_PKEY
> with a private key should have a public key.  I was incorrect.

Sure, my example was not about pointing fingers!
It's just to give recent data points on how the documentation written
in 2006 is probably not something as stale as one can think, because
until recently we were leveraging that assumption at least in some
sections of the lower layers.

> Regarding avoiding NULL checks, that's actually a separate story,
> evem though it overlaps conveniently.  There was the idea, for a
> while, that we should avoid NULL checks everywhere (unless it's valid
> input), and indeed make it a programmer error if they happened to pass
> NULL where they shouldn't.
> One can see that as an hard assertion, and that has indeed produced
> crashes that uncovered bugs we might otherwise have missed.  The tide
> has changed though, and it seem like the fashion du jour is to check
> NULLs and error with an ERR_R_PASSED_NULL_PARAMETER.  I'm not sure
> that we've made an actual hard decision in either direction, though.
>
> However, I'm not sure where the NULL check problem comes in.
> Operations that don't use the public key parts don't need to look at
> the public key parts, so a NULL check there is simply not necessary.

I think the lack of NULL checks is intertwined with this problem as
long as in some code that does access the public key component we
ensured we dereference without making the NULL check because of the
assumption.

The problem I see with spot checking rather than writing a
comprehensive suite of tests, is that given our wide API surface and
the compromises taken in the transition from non-opaque stack
allocated objects in 1.0.2 to opaque objects in 1.1.1+, we might have
non-obvious places where dereferencing the public key without checking
for NULL can bite us.


I would add, although it's feedback for the vote, not the proposal,
that I am not opposed to changing the documented assumption in
principle, but with changing it this late in the development of 3.0.

I am giving feedback for the text proposal to ensure that during the
voting we don't underestimate the high risk of critical bugs coming
with adopting this change at this stage, and that it does call for
vastly extending our current test coverage beyond what we were
currently planning as acceptable.

We all feel quite under pressure due to the time constraints, and I
feel that in general we have privileged working on implementing the
required features (with sufficient level of tests as per our policy)
rather than spending major resources in further developing our test
coverage with exhaustive positive and negative tests for regular and
far-fetched cases, this should probably also have a weight on the
decision of committing to this change at this point.



Nicola