Vote proposal: Private keys can exist independently of public keys

[Nicola Tuveri]

I believe the current formulation is slightly concealing part of the problem.

The way I see it, the intention if the vote passes is to do more than stated:
1. change the long-documented assumption
2. fix "regressions" in 3.0 limited to things that worked in a certain
way in 1.1.1 (and maybe before), _despite_ the documented assumption
3. test all existing functions that access the public key component of
`EVP_PKEY` (or lower level) objects to ensure they don't rely on the
long-documented assumption
4. fix all the places that intentionally relied on the long-documented
assumption and that were required to avoid "useless NULL checks"

I believe that to change a widely and longed-enforced assumption like
this, we can't rely on a single case or a list of single cases that
worked _despite_ the documented assumption as proof that things can
(and should) work a certain way or another, and that before taking the
decision this late in the development process the results of thorough
testing are a prerequisite to assess the extent of code changes
required by changing the assumption and the potential for instability
and disruption that this can cause for our direct and indirect users
if segfaults slip through our currently limited coverage as a
consequence of this change.

I feel that we are currently underestimating the potential impact of
this change, and currently motivated by a handful of isolated cases in
which under a very specific set of conditions things aligned in a way
that allowed certain usage patterns to succeed despite the documented
assumption.
I also feel that the "burden of the proof" of improving the test
coverage to exhaustively cover all kinds of keys (both in their legacy
form and in provider-native form), under all possible operations at
different abstraction levels (e.g. limiting this example to
sign/verify as the operation, testing should not be limited to
`EVP_DigestSign/Verify()`, but also through
`EVP_DigestSign/Verify{Init,Update,Final}()`,
`EVP_PKEY_sign/verify*()`, `(EC)DSA_(do_)sign/verify()`, etc.,
external testing with ENGINEs that might rely on the long-documented
assumption), should fall on who is proposing this change, rather than
committing that we will be able to increase our coverage to prevent
unforeseen consequences of changing the assumption, before we can
decide to commit to alter the assumption.

So, to better capture the extent of work required to apply the change
and the risks associated with it I would rephrase the text vote as
follows:

> We should change the 3.0 code to explicitly allow private components
> to exist in keys without the public components also being present,
> ensuring that, in `EVP` and in the lower abstraction layers, both in
> legacy and in provider-native codepaths, _every_ instance in which any
> public key component is dereferenced it is preceded by a NULL pointer
> check or guaranteed non-NULL from any caller.
> To change the long documented assumption, we commit to improve test
> coverage of all public functions directly or indirectly triggering
> potential access to public key components, to prevent the risk of user
> facing crashes.


Nicola




On Wed, 7 Oct 2020 at 14:29, Matt Caswell <matt at openssl.org> wrote:
>
> Issue #12612 exposes a problem with how we handle keys that contain
> private components but not public components.
>
> There is a widespread assumption in the code that keys with private
> components must have public components. There is text in our public
> documentation that states this (and that text dates back to 2006).
>
> OTOH, the code has not always enforced this. Issue #12612 describes a
> scenario where this has not historically been enforced, and it now is in
> the current 3.0 code causing a regression.
>
> There are differences of opinion on how this should be handled. Some
> have the opinion that we should change the model so that we explicitly
> allow private keys to exists without the public components. Others feel
> that we should continue with the old model.
>
> It seems we need a vote to decide this. Here is my proposed vote text:
>
> We should change the 3.0 code to explicitly allow private components to
> exist in keys without the public components also being present.
>
> Feedback please on the proposed vote text.
>
> Matt