I would also be ok with this vote text formulation. Lets see what others
say.

Matt


On 07/10/2020 16:34, Nicola Tuveri wrote:
> 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 <m...@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
> 

Reply via email to