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

Wed Nov 11 10:11:46 UTC 2020

I could see reasons to vote negatively for such a resolution in generic
terms if I was voting on it: often the OpenSSL apps are part of critical
PKI procedures and scripts.
Such scripts might very well rely on the fact that this or that openssl
command does not return a failure exit status in certain cases even if it
reports the failure in its output or that as a side effect of reporting a
failure some output file is not produced or created empty.
A breaking change in such context can be strategically very bad for the
project, with unintentional negative publicity consequences.
A one by one decision on the mailing list would ring more bells and
potentially trigger feedback about things that are going to be broken we
were not aware of, compared to a silent change in a PR based on the generic
decision, that could either inform the vote to decide to adopt different
technical solutions or make the new breaking behavior opt-in, or give an
opportunity to give more visibility to a breaking change and adapt the
critical scripts and procedures to the new behavior before mayhem happens.

Also the formulating a decision with "error checks like this" (or a more
formal text description) has the risk of being difficult to apply: it
shifts the responsibility of deciding if the case is similar enough onto
the reviewers of the individual PR.

Anyway, I agree that general or specific there should be an OMC vote about
this rather than an OTC vote: I'll drop my  OTC vote proposal and wait for
a OMC resolution on the matter.
Can someone from the OMC take the initiative to propose the OMC vote that
they believe is the more appropriate and move this forward? (can that
person also covert the OTC hold label I put on #13359 into an OMC hold
label?)

Cheers,

Nicola

On Wed, Nov 11, 2020, 11:16 Dr Paul Dale <paul.dale at oracle.com> wrote:

> 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$
>  >
> [PR 13359]: <
> 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$
>  >
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mta.openssl.org/pipermail/openssl-project/attachments/20201111/5fbe1d15/attachment.html>