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 <m...@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$> >> >
