[OTC VOTE PROPOSAL] Approve behavior change for `pkey -[pub]check`

[Dr Paul Dale]

An OMC vote deeming that adding error checks like this are or are not considered breaking changes.

My view is that detecting an error condition and returning an error code is a bug fix rather than a breaking change.


Pauli
-- 
Dr Paul Dale | Distinguished Architect | Cryptographic Foundations 
Phone +61 7 3031 7217
Oracle Australia




> On 11 Nov 2020, at 7:10 pm, Matt Caswell <matt at openssl.org> wrote:
> 
> I have no problem  with the proposal or the vote text. I only wonder
> whether, as a breaking change an OMC vote is required? Or is an OTC vote
> sufficient?
> 
> Matt
> 
> 
> On 10/11/2020 16:15, Nicola Tuveri wrote:
>> ## Background
>> 
>> As part of the discussion in [issue #8435], it was highlighted that both
>> in 1.1.1 and master we are missing tests to validate decoded SM2 keys.
>> The `openssl pkey -check` (or `pkey -pubcheck` to validate only the
>> public key component) command allows to explicitly execute the
>> validation checks, while on regular operations (e.g., using `pkeyutl` or
>> `dgst`) no validation of the input keys is normally done (probably by
>> design).
>> 
>> In [PR 13359] I am adding new tests, using `openssl pkey -check` to
>> validate specific key corner cases, for P-256 as an exemplar for EC keys
>> and for SM2 keys.
>> Unfortunately `openssl pkey -check` behavior currently shows the result
>> of the validation only in the text output, so parsing of `stdout` would
>> require measuring the test results.
>> In the PR I am proposing to change the behavior of `openssl pkey` so
>> that an early exit is triggered if `-check` or `-pubcheck` validation
>> fails, exiting with a failure exit status: [apps/pkey.c changes].
>> 
>> This is a breaking change in the behavior of `openssl pkey` and the
>> reason why I am planning to raise a vote to approve this change.
>> 
>> Note that during our OTC meeting today it was proposed as an alternative
>> to have a more generic vote on approving this kind of change in general
>> for all similar situations across all the apps.
>> While I am not opposed to such a decision, I am afraid having such a
>> generic vote might be quite difficult:
>> 
>> - I am not sure I can express in a clear and unambiguous what are the
>>  conditions to fall within "similar situations", making such a
>>  decision hard to execute in practical terms;
>> - I personally cannot commit to execute such vote across all apps and
>>  all relevant conditions: doing so requires extensive review of the
>>  apps logic in parsing the CLI switches, of conformity with existing
>>  documentation and an exploration of existing use patterns in the user
>>  codebase to see what are the expectations and estimate the impact of
>>  such changes. It would also require writing an extensive battery of
>>  tests to ensure we behave as documented/expected across apps and CLI
>>  switches.
>> - The amount of work to which we would commit after such a generic
>>  decision, and the impact it could have on the behavior consistency
>>  w.r.t. 3.0 and 1.1.1, would make me think such a decision is more in
>>  the realm of strategic objectives, for which an OMC vote would be more
>>  appropriate.
>> 
>> For these reasons, at this time, I am opting to propose an OTC vote on
>> the single case of the behavior change of `pkey -check`/`pkey -pubcheck`
>> rather than a more generic one, and I will let others decide if a more
>> generic vote is also required/appropriate and if it can be framed
>> exclusively within the technical prerogatives of the OTC decision
>> process.
>> 
>> ## Proposed vote text
>> 
>>    Change the behavior of `pkey -check`
>>    and `pkey -pubcheck` in 3.0 to trigger an
>>    early exit if validation fails, returning a
>>    failure exit status to the parent process.
>> 
>> ---
>> 
>> Please leave feedback relevant to the proposed vote text and the scope
>> of the vote.
>> In absence of feedback I plan to open the actual vote in 24h.
>> 
>> 
>> 
>> Cheers,
>> 
>> Nicola
>> 
>> ---
>> 
>> [issue #8435]: <https://urldefense.com/v3/__https://github.com/openssl/openssl/issues/8435__;!!GqivPVa7Brio!Iiq8eSK65cKttwL_M-jL1JwQw-V9roGNZaxYAPw08zcLEiuEMvGtYPDvrBSYf2E$ <https://urldefense.com/v3/__https://github.com/openssl/openssl/issues/8435__;!!GqivPVa7Brio!Iiq8eSK65cKttwL_M-jL1JwQw-V9roGNZaxYAPw08zcLEiuEMvGtYPDvrBSYf2E$> >
>> [PR 13359]: <https://urldefense.com/v3/__https://github.com/openssl/openssl/pull/13359__;!!GqivPVa7Brio!Iiq8eSK65cKttwL_M-jL1JwQw-V9roGNZaxYAPw08zcLEiuEMvGtYPDvd5hDbWk$ <https://urldefense.com/v3/__https://github.com/openssl/openssl/pull/13359__;!!GqivPVa7Brio!Iiq8eSK65cKttwL_M-jL1JwQw-V9roGNZaxYAPw08zcLEiuEMvGtYPDvd5hDbWk$> >
>> [apps/pkey.c changes]:
>> <https://urldefense.com/v3/__https://github.com/openssl/openssl/pull/13359/files*diff-810c047ab642e686cab85b16802e9190bbc9f2f9bfce8a55fb8d6b744caa9091__;Iw!!GqivPVa7Brio!Iiq8eSK65cKttwL_M-jL1JwQw-V9roGNZaxYAPw08zcLEiuEMvGtYPDv8zOO9ys$ <https://urldefense.com/v3/__https://github.com/openssl/openssl/pull/13359/files*diff-810c047ab642e686cab85b16802e9190bbc9f2f9bfce8a55fb8d6b744caa9091__;Iw!!GqivPVa7Brio!Iiq8eSK65cKttwL_M-jL1JwQw-V9roGNZaxYAPw08zcLEiuEMvGtYPDv8zOO9ys$> >

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mta.openssl.org/pipermail/openssl-project/attachments/20201111/7e97708f/attachment.html>