OTC Vote: Backport PR #19681 to 3.0 as a bug fix

[Nicola Tuveri]

Topic: Backport PR #19681 to 3.0 as a bug fix
Comment: Fundamentally OTC must decide if in the 3.0 release we should
 honor OSSL_PKEY_PARAM_EC_POINT_CONVERSION_FORMAT as set (and
 default to UNCOMPRESSED) in our provider implementations, and
 apply corresponding changes for handling legacy keys.
Proposed by: Nicola Tuveri
Issue link: https://github.com/openssl/technical-policies/issues/60
Public: yes
Opened: 2022-12-01
Closed: -MM-DD
Accepted:  yes/no  (for: X, against: Y, abstained: Z, not voted: W)

  Dmitry [  ]
  Matt   [  ]
  Pauli  [  ]
  Tim[  ]
  Hugo   [  ]
  Richard[  ]
  Shane  [  ]
  Tomas  [  ]
  Kurt   [  ]
  Matthias   [  ]
  Nicola [+1]

Re: OTC Vote: Accept PR #19681 in 3.1 subject to normal review process

[Nicola Tuveri]

The vote is now closed as accepted.

As recorded [0], the final tally was:

~~~
Topic: Accept PR #19681 in 3.1 subject to normal review process
Comment: Fundamentally OTC must decide if in the 3.1 release we should
 honor OSSL_PKEY_PARAM_EC_POINT_CONVERSION_FORMAT as set (and
 default to UNCOMPRESSED) in our provider implementations, and
 apply corresponding changes for handling legacy keys.
Proposed by: Nicola Tuveri
Issue link: https://github.com/openssl/technical-policies/issues/59
Public: yes
Opened: 2022-11-15
Closed: 2022-11-27
Accepted:  yes  (for: 7, against: 0, abstained: 2, not voted: 2)

  Dmitry [+1]
  Matt   [+0]
  Pauli  [ 0]
  Tim[+1]
  Hugo   [+1]
  Richard[+1]
  Shane  [+1]
  Tomas  [+1]
  Kurt   [  ]
  Matthias   [  ]
  Nicola [+1]
~~~

[0]: 
https://github.com/openssl/technical-policies/blob/master/votes/vote-20221115-honor-point-conversion-format-in-3.1.txt

[openssl/technical-policies] 70312d: [VOTE] Honor EC point conversion format in 3.1

[Nicola Tuveri]

  Branch: refs/heads/master
  Home:   https://github.com/openssl/technical-policies
  Commit: 70312d1e8ccb605df19bcab76e9d95e8590b49be
  
https://github.com/openssl/technical-policies/commit/70312d1e8ccb605df19bcab76e9d95e8590b49be
  Author: Nicola Tuveri 
  Date:   2022-11-27 (Sun, 27 Nov 2022)

  Changed paths:
A votes/vote-20221115-honor-point-conversion-format-in-3.1.txt

  Log Message:
  ---
  [VOTE] Honor EC point conversion format in 3.1

[otc/technical-policies] 70312d: [VOTE] Honor EC point conversion format in 3.1

[Nicola Tuveri]

  Branch: refs/heads/master
  Home:   https://github.openssl.org/otc/technical-policies
  Commit: 70312d1e8ccb605df19bcab76e9d95e8590b49be
  
https://github.openssl.org/otc/technical-policies/commit/70312d1e8ccb605df19bcab76e9d95e8590b49be
  Author: Nicola Tuveri 
  Date:   2022-11-27 (Sun, 27 Nov 2022)

  Changed paths:
A votes/vote-20221115-honor-point-conversion-format-in-3.1.txt

  Log Message:
  ---
  [VOTE] Honor EC point conversion format in 3.1

OTC Vote: Accept PR #19681 in 3.1 subject to normal review process

[Nicola Tuveri]

Topic: Accept PR #19681 in 3.1 subject to normal review process
Comment: Fundamentally OTC must decide if in the 3.1 release we should
 honor OSSL_PKEY_PARAM_EC_POINT_CONVERSION_FORMAT as set (and
 default to UNCOMPRESSED) in our provider implementations, and
 apply corresponding changes for handling legacy keys.
Proposed by: Nicola Tuveri
Issue link: https://github.com/openssl/technical-policies/issues/59
Public: yes
Opened: 2022-11-15
Closed: -MM-DD
Accepted:  yes/no  (for: X, against: Y, abstained: Z, not voted: W)

  Dmitry [  ]
  Matt   [  ]
  Pauli  [  ]
  Tim[  ]
  Hugo   [  ]
  Richard[  ]
  Shane  [  ]
  Tomas  [  ]
  Kurt   [  ]
  Matthias   [  ]
  Nicola [+1]

[openssl/technical-policies] 1fb420: [VOTE][late update] test labelling policy

[Nicola Tuveri]

  Branch: refs/heads/master
  Home:   https://github.com/openssl/technical-policies
  Commit: 1fb420362b34b1481c7ab41f01bdba3d3139b10b
  
https://github.com/openssl/technical-policies/commit/1fb420362b34b1481c7ab41f01bdba3d3139b10b
  Author: Nicola Tuveri 
  Date:   2022-11-15 (Tue, 15 Nov 2022)

  Changed paths:
M votes/vote-20221107-test-labels.txt

  Log Message:
  ---
  [VOTE][late update] test labelling policy

[otc/technical-policies] a7a5c8: Revise testing policy with labels

[Nicola Tuveri]

  Branch: refs/heads/master
  Home:   https://github.openssl.org/otc/technical-policies
  Commit: a7a5c8b297240e7c6be3c70b0e76522c4a7b1b0a
  
https://github.openssl.org/otc/technical-policies/commit/a7a5c8b297240e7c6be3c70b0e76522c4a7b1b0a
  Author: Hugo Landau 
  Date:   2022-11-10 (Thu, 10 Nov 2022)

  Changed paths:
M policies/testing.md

  Log Message:
  ---
  Revise testing policy with labels


  Commit: 95becf10e995269b73eccbb8768be0bc3417e0e1
  
https://github.openssl.org/otc/technical-policies/commit/95becf10e995269b73eccbb8768be0bc3417e0e1
  Author: Hugo Landau 
  Date:   2022-11-10 (Thu, 10 Nov 2022)

  Changed paths:
M votes/template.txt
A votes/vote-20221107-test-labels.txt

  Log Message:
  ---
  Add vote for new test labels policy


  Commit: 1fb420362b34b1481c7ab41f01bdba3d3139b10b
  
https://github.openssl.org/otc/technical-policies/commit/1fb420362b34b1481c7ab41f01bdba3d3139b10b
  Author: Nicola Tuveri 
  Date:   2022-11-15 (Tue, 15 Nov 2022)

  Changed paths:
M votes/vote-20221107-test-labels.txt

  Log Message:
  ---
  [VOTE][late update] test labelling policy


Compare: 
https://github.openssl.org/otc/technical-policies/compare/290d95d9a567...1fb420362b34

[openssl/technical-policies] 490819: Record vote-20220531-accept-pr18407-for-backfit-to...

[Nicola Tuveri]

  Branch: refs/heads/master
  Home:   https://github.com/openssl/technical-policies
  Commit: 4908192cdab7c872ca018bfa3bbe7a11d8493876
  
https://github.com/openssl/technical-policies/commit/4908192cdab7c872ca018bfa3bbe7a11d8493876
  Author: Nicola Tuveri 
  Date:   2022-08-01 (Mon, 01 Aug 2022)

  Changed paths:
A votes/vote-20220531-accept-pr18407-for-backfit-to-3.0.txt

  Log Message:
  ---
  Record vote-20220531-accept-pr18407-for-backfit-to-3.0.txt

[otc/technical-policies] 490819: Record vote-20220531-accept-pr18407-for-backfit-to...

[Nicola Tuveri]

  Branch: refs/heads/master
  Home:   https://github.openssl.org/otc/technical-policies
  Commit: 4908192cdab7c872ca018bfa3bbe7a11d8493876
  
https://github.openssl.org/otc/technical-policies/commit/4908192cdab7c872ca018bfa3bbe7a11d8493876
  Author: Nicola Tuveri 
  Date:   2022-08-01 (Mon, 01 Aug 2022)

  Changed paths:
A votes/vote-20220531-accept-pr18407-for-backfit-to-3.0.txt

  Log Message:
  ---
  Record vote-20220531-accept-pr18407-for-backfit-to-3.0.txt

Vote: Accept PR#18407 for backfit to 3.0 as a policy exception

[Nicola Tuveri]

Topic: Accept PR#18407 for backfit to 3.0 as a policy exception.
Proposed by: Nicola
Issue link: https://github.com/openssl/technical-policies/issues/52
Public: yes
Opened: 2022-05-31
Closed:
Accepted:(for: , against: , abstained: , not voted: )

   Dmitry [+1]
   Matt   [-1]
   Pauli  [+1]
   Tim[ 0]
   Richard[ 0]
   Shane  [-1]
   Tomas  [ 0]
   Kurt   [  ]
   Matthias   [-0]
   Nicola [+1]

The vote was opened during the weekly OTC meeting, and is still open.
OTC members, please vote.

Nicola Tuveri

Re: OTC VOTE: Accept Policy change process proposal

[Nicola Tuveri]

+1

On Tue, Nov 2, 2021 at 1:00 PM Tomas Mraz  wrote:
>
> There are already enough votes to close this one, so I intend to close
> the vote tomorrow.
>
> Tomas
>
>
> On Mon, 2021-11-01 at 11:23 +0100, Tomas Mraz wrote:
> > topic: Accept openssl/technical-policies PR#1 - the policy change
> > process proposal as of commit 3bccdf6. This will become an official
> > OTC
> > policy.
> >
> > comment: This will implement the formal policy change process so we
> > can
> > introduce and amend further policies as set by OTC via a public
> > process.
> >
> > Proposed by Tomáš Mráz
> > Public: yes
> > opened: 2021-11-01
> > closed: 2021-mm-dd
> > accepted:  yes/no  (for: X, against: Y, abstained: Z, not voted: T)
> >
> >Dmitry [  ]
> >Matt   [  ]
> >Pauli  [  ]
> >Tim[  ]
> >Richard[  ]
> >Shane  [  ]
> >Tomas  [+1]
> >Kurt   [  ]
> >Matthias   [  ]
> >Nicola [  ]
> >
> >
> >
>
> --
> Tomáš Mráz
> No matter how far down the wrong road you've gone, turn back.
>   Turkish proverb
> [You'll know whether the road is wrong if you carefully listen to your
> conscience.]
>
>
> ___
> otc mailing list
> o...@openssl.org
> https://mta.openssl.org/mailman/listinfo/otc

Re: OTC VOTE: Accept PR#16725

[Nicola Tuveri]

I believe Matt will find the time at some point to post the minutes
from today's meeting, but until then here is my recap.

The discussion mostly focused on why the changes in #16725 are a
bugfix and not a new feature, which would be a prerequisite to be
admissible to be merged in the 3.0 branch.
As I recall it, there were no objections to the final outcome of the
PR to be desirable, the vote is entirely about this being a bugfix or
not.

It would be on those who voted +1 to properly argument why this is a
bugfix and not a new feature, but the short version of that argument
is that the outcome of #16725 was the "intended behavior" for 3.0.0.
The counterargument is that we could not find written evidence (i.e.,
GH issues/PRs, documentation, and/or tests) that indeed the project
ever committed to have this behavior in 3.0.0.


The Strategic Architecture document has some text that could be
somewhat related and used to support the "intend behavior" view, but
the document clearly states

> This document outlines the OpenSSL strategic architecture. It will take 
> multiple releases, starting from 3.0.0, to move the architecture from the 
> current "as-is" (1.1.1), to the future "to-be" architecture.

Hence, it does not really prove that this functionality was always
planned for the 3.0.0 release.

Accepting this PR for the next minor release would not require a vote.



I hope this recap is helpful to inform your decision.



Cheers,

Nicola

On Tue, Oct 19, 2021 at 9:10 PM Kurt Roeckx  wrote:
>
> On Tue, Oct 19, 2021 at 11:07:26AM +0100, Matt Caswell wrote:
> > topic: Accept PR#16725 as a bug fix for backport into 3.0 subject to the
> > normal review process
>
> So we have various people voting -1. Does someone want to explain
> why they vote -1?
>
>
> Kurt
>

Re: OTC VOTE: Revert the commits merged from PR #16027 in 1.1.1

[Nicola Tuveri]

On the other hand, 1.1.1 is not in its last year of support so it is not
limited to security fixes only.

The commits which this vote proposes to revert fixed a bug that produced
invalid output from functions with a clear intent.
This might have security repercussions, as the user might end up signing
something which is unexpectedly invalid.
But even without concrete security vulnerabilities on record, if we
classify invalid output as a bug this should be fixed in 1.1.1.

There are applications that might be broken, because they relied on the
buggy behavior for producing invalid output as intermediate data, but, as
mentioned in #16266, there are ways of producing the required non-x509 data
without abusing functions meant to produce valid x509.

It is unfortunate for existing applications to break upon a patch release,
but given that patch releases for 1.1.1 are meant to fix security defects
and bugs, this is inevitable for any application relying on buggy behavior
(especially as in the case that triggered this discussion, which configures
a clear abuse of the API, while alternative non-abusive ways of achieving
the intended result exist).



Nicola



On Wed, Aug 11, 2021, 11:00 Tomas Mraz  wrote:

> As this vote is still ongoing I am going to somewhat promote its case.
> I really suspect that many applications albeit somewhat special ones
> will be broken by this change. So although the change can be surely
> viewed as a bug fix it is IMO wrong that it was merged to the 1.1.1
> branch.
>
> Although there might be security implications of exporting an
> incomplete/broken DER encoding, no concrete security issue was
> presented that exists unless this bug is fixed.
>
> On Tue, 2021-08-10 at 11:53 +0100, Matt Caswell wrote:
> > topic: Revert the commits merged from PR #16027 in 1.1.1
> > Comment: Refer to issue #16266 for background
> > Proposed by Tomas Mraz
> > Public: yes
> > opened: 2021-08-10
> > closed: 2021-mm-dd
> > accepted:  yes/no  (for: X, against: Y, abstained: Z, not voted: T)
> >
> >Dmitry [+1]
> >Matt   [+1]
> >Pauli  [  ]
> >Tim[-1]
> >Richard[  ]
> >Shane  [-1]
> >Tomas  [+1]
> >Kurt   [  ]
> >Matthias   [  ]
> >Nicola [-1]
>
> --
> Tomáš Mráz
> No matter how far down the wrong road you've gone, turn back.
>   Turkish proverb
> [You'll know whether the road is wrong if you carefully listen to your
> conscience.]
>
>
>

Re: OTC VOTE: Accept PR 16128

[Nicola Tuveri]

+1

On Thu, Jul 22, 2021, 16:56 Dr. Matthias St. Pierre <
matthias.st.pie...@ncp-e.com> wrote:

> +1
>
> > -Original Message-
> > From: openssl-project  On Behalf
> Of Tomas Mraz
> > Sent: Thursday, July 22, 2021 3:06 PM
> > To: openssl-project@openssl.org
> > Subject: Re: OTC VOTE: Accept PR 16128
> >
> > +1 it is a safe change and it improves consistency
> >
> > On Thu, 2021-07-22 at 13:51 +0100, Matt Caswell wrote:
> > > topic: Accept PR 16128 in 3.0 subject to our normal review process
> > > Proposed by Matt Caswell
> > > Public: yes
> > > opened: 2021-07-22
> > > closed: 2021-mm-dd
> > > accepted:  yes/no  (for: X, against: Y, abstained: Z, not voted: T)
> > >
> > >Matt   [+1]
> > >Pauli  [  ]
> > >Tim[  ]
> > >Richard[  ]
> > >Shane  [  ]
> > >Tomas  [  ]
> > >Kurt   [  ]
> > >Matthias   [  ]
> > >Nicola [  ]
> >
> > --
> > Tomáš Mráz
> > No matter how far down the wrong road you've gone, turn back.
> >   Turkish proverb
> > [You'll know whether the road is wrong if you carefully listen to your
> > conscience.]
>
>

Re: OTC VOTE: Reject PR#14759

[Nicola Tuveri]

The vote is now closed: it passed, and PR#14759 is rejected.


topic: Reject PR#14759
Proposed by Nicola Tuveri
Public: yes
opened: 2021-04-20
closed: 2021-05-04
accepted:  yes  (for: 3, against: 2, abstained: 4, not voted: 2)
  Matt   [ 0]
  Mark   [  ]
  Pauli  [-0]
  Viktor [  ]
  Tim[-1]
  Richard[ 0]
  Shane  [+1]
  Tomas  [-1]
  Kurt   [+1]
  Matthias   [ 0]
  Nicola [+1]


Best regards,

Nicola Tuveri

OpenSSF Security Metrics Initiative

[Nicola Tuveri]

Hi,

I wanted to point out to the OMC and to openssl-project a new
initiative from the [Open Source Security Foundation](www.openssf.org): the
Security Metrics Initiative.

A more detailed description is available at <
https://openssf.org/blog/2021/05/03/introducing-the-security-metrics-project/
>.
It should be remarked that the  service is to be
considered alpha, and that changes in the API, in data sources might occur
at this stage, and that there might be inaccuracies in the reported data.

Here is a direct link to what the initiative reports for the OpenSSL
project:
<
https://metrics.openssf.org/grafana/d/default/metric-dashboard?orgId=1=pkg%3Agithub/openssl/openssl
>.

In particular it seems we score quite low on the OpenSSF Scorecard (30.8%
as I am writing this mail) and, also for the data coming from the OpenSSF
Best Practices Badge Program, it looks like the project has many negative
marks.

It should also be noted that the description field in the project
information for `github:/openssl/openssl` reports:

> This is a historical badge entry for the OpenSSL project before the
Heartbleed vulnerability was reported, circa February 2014. Please note
that the OpenSSL project's status has changed substantially since then. For
the current state of OpenSSL, see the current OpenSSL badge entry. [...]

So maybe it is not too alarming that many of the negative marks are coming
from unexpected entries: e.g. it seems at the moment it reports we don't
have/use static/dynamic analysis, we don't have vulnerability reporting,
code review, CI Tests or Pull Requests.

Nonetheless given this tool might soon be used to pick among alternatives
when making critical infrastructure design choices, or affect funding
decisions or resource planning, it might be a good thing for the OMC to get
proactive and reach out to straighten the record for current OpenSSL
releases, to offer suggestions on alternative metrics to be considered, on
redefining criteria for existing metrics, and possibly incorporate feedback
from the Security Metrics initiative to adapt plans regarding future
roadmap for OpenSSL.

I finish reporting in this email the last paragraph from the Security
Metrics Initiative announcement, as it might be of interest for all
subscribers to openssl-project:

> Your [feedback](https://github.com/ossf/Project-Security-Metrics/issues)
is most welcome, and if you're interested in learning more or joining this
effort, please reach out to [Michael Scovetta](mailto://
michael.scove...@microsoft.com) or join us at our next [working group](
https://github.com/ossf/wg-identifying-security-threats) meeting.



Best regards,

Nicola Tuveri

OTC VOTE: Reject PR#14759

[Nicola Tuveri]

Following up on
https://www.mail-archive.com/openssl-project@openssl.org/msg02407.html we
had a discussion on this during last week OTC meeting, and opened a vote
limited exclusively to the matter of rejecting PR#14759.

We lost the record of the votes collected during the call, so opening it
officially today with a clean slate.



topic: Reject PR#14759
Proposed by Nicola Tuveri
Public: yes
opened: 2021-04-20
closed: 2021-mm-dd
accepted:  yes/no  (for: X, against: Y, abstained: Z, not voted: T)
  Matt   [  ]
  Mark   [  ]
  Pauli  [  ]
  Viktor [  ]
  Tim[  ]
  Richard[  ]
  Shane  [  ]
  Tomas  [  ]
  Kurt   [  ]
  Matthias   [  ]
  Nicola [+1]

Re: [OTC VOTE PROPOSAL] Don't merge PR#14759 (blinding=yes and similar properties)

[Nicola Tuveri]

But I am not opposed to separate the 2 votes if that is perceived as better
and we are ready to deal with the possible delays introduced in the
development.

I am not entirely sure if this PR can be retriaged by OTC as not-blocking
for the beta release, but that could also be an option to buy more time
while we define a policy and then vote to accept or reject based on that.

Nicola

On Fri, Apr 9, 2021, 14:24 Nicola Tuveri  wrote:

> I agree with what Tomàš said, and that is the reason why I convoluted them
> in a single vote: we need to merge or reject the PR based on a policy, but
> if we do 2 separate votes we risk to create delays in the already quite
> loaded development cycles left!
>
> Nicola
>
> On Fri, Apr 9, 2021, 10:53 Tomas Mraz  wrote:
>
>> On Fri, 2021-04-09 at 08:44 +0100, Matt Caswell wrote:
>> >
>> > On 08/04/2021 18:02, Nicola Tuveri wrote:
>> > > Proposed vote text
>> > > ==
>> > >
>> > >  Do not merge PR#14759, prevent declaring properties similar to
>> > >  `blinding=yes` or `consttime=yes` in our implementations and
>> > >  discourage 3rd parties from adopting similar designs.
>> >
>> > I think this vote tries to cover too much ground in a single vote. I
>> > would prefer to see a simple vote of "Do not merge PR#14759"
>> > *possibly*
>> > followed up by separate votes on what our own policies should be for
>> > provider implementations, and what we should or should not encourage
>> > 3rd
>> > parties to do.
>>
>> I disagree partially. IMO we should primarily have a policy vote and
>> the closing or merging of PR#14759 should come out of it naturally.
>>
>> --
>> Tomáš Mráz
>> No matter how far down the wrong road you've gone, turn back.
>>   Turkish proverb
>> [You'll know whether the road is wrong if you carefully listen to your
>> conscience.]
>>
>>
>>

Re: [OTC VOTE PROPOSAL] Don't merge PR#14759 (blinding=yes and similar properties)

[Nicola Tuveri]

I agree with what Tomàš said, and that is the reason why I convoluted them
in a single vote: we need to merge or reject the PR based on a policy, but
if we do 2 separate votes we risk to create delays in the already quite
loaded development cycles left!

Nicola

On Fri, Apr 9, 2021, 10:53 Tomas Mraz  wrote:

> On Fri, 2021-04-09 at 08:44 +0100, Matt Caswell wrote:
> >
> > On 08/04/2021 18:02, Nicola Tuveri wrote:
> > > Proposed vote text
> > > ==
> > >
> > >  Do not merge PR#14759, prevent declaring properties similar to
> > >  `blinding=yes` or `consttime=yes` in our implementations and
> > >  discourage 3rd parties from adopting similar designs.
> >
> > I think this vote tries to cover too much ground in a single vote. I
> > would prefer to see a simple vote of "Do not merge PR#14759"
> > *possibly*
> > followed up by separate votes on what our own policies should be for
> > provider implementations, and what we should or should not encourage
> > 3rd
> > parties to do.
>
> I disagree partially. IMO we should primarily have a policy vote and
> the closing or merging of PR#14759 should come out of it naturally.
>
> --
> Tomáš Mráz
> No matter how far down the wrong road you've gone, turn back.
>   Turkish proverb
> [You'll know whether the road is wrong if you carefully listen to your
> conscience.]
>
>
>

Re: [OTC VOTE PROPOSAL] Don't merge PR#14759 (blinding=yes and similar properties)

[Nicola Tuveri]

ions when multiple providers are
> available is performed by properties
>
> Algorithm implementations should declare whatever set of properties they
> feel is appropriate for their implementation.
> Applications (and in this context most likely directly by end-user
> configuration) should be able to select which properties are considered
> most important for their context.
> That decision capability must be left to the end user as only the end user
> knows the security context in which they are operating - we don't know that
> ourselves.
>
> The vast majority of your lengthy email below is actually focused on one
> issue - what should the default behaviour be for selection of
> implementations - and your viewpoint that we should not mark different
> properties at all that might impact security. I don't think that position
> is supportable - in that you are basically arguing that we should never
> declare anything about properties of implementations and should never
> select between different implementations except at a "provider level"
> approach. Your approach is that all implementations should be considered
> equal and that pretty much defies logic in my view.
>
> Different implementations have different characteristics. Even looking at
> something like constant time - not all of our implementations are constant
> time.
>
> Your statement that the approach of declaring properties "promotes
> insecure by default" is simply flawed logic - and following the same logic
> I could state that your approach "promotes ignorance by default" as it
> effectively states that users shouldn't know the properties of the
> implementation in a manner that allows selection to be performed.
>
> Not all implementations are the same and different implementations can
> implement different mitigations. Properties allow us to declare those
> mitigations and allow users to make different decisions on the basis of the
> properties that providers declare for algorithms. Having that information
> available has to be a better thing than having nothing available - as with
> nothing available then no selection between alternatives is possible.
>
> Separately arguing about what the default set of properties should be
> (i.e. what mitigations should we configure as required by default) would
> make sense to do so - but arguing that the information for making such
> decisions shouldn't be present simply makes no sense.
>
> Tim.
>
>
> On Fri, Apr 9, 2021 at 3:02 AM Nicola Tuveri  wrote:
>
>> Background
>> ==
>>
>> [PR#14759](https://github.com/openssl/openssl/pull/14759) (Set
>> blinding=yes property on some algorithm implementations) is a fix for
>> [Issue#14654](https://github.com/openssl/openssl/issues/14654) which
>> itself is a spin-off of
>> [Issue#14616](https://github.com/openssl/openssl/issues/14616).
>>
>> The original issue is about _Deprecated low level key API's that have no
>> replacements_, among which the following received special attention and
>> were issued a dedicated issue during an OTC meeting:
>>
>> ~~~c
>> // RSA functions on RSA_FLAG_*
>> void RSA_clear_flags(RSA *r, int flags);
>> int RSA_test_flags(const RSA *r, int flags);
>> void RSA_set_flags(RSA *r, int flags);
>>
>> // RSA_FLAG_* about blinding
>> #define RSA_FLAG_BLINDING
>> #define RSA_FLAG_NO_BLINDING
>>
>> // RSA functions directly on blinding
>> int RSA_blinding_on(RSA *rsa, BN_CTX *ctx);
>> void RSA_blinding_off(RSA *rsa);
>> BN_BLINDING *RSA_setup_blinding(RSA *rsa, BN_CTX *ctx);
>> ~~~
>>
>> The decision the sprung Issue#14616 and PR#14759 was to use the
>> propquery mechanism to let providers advertise algorithms as
>> `blinding=yes` to select secure implementations if there are insecure
>> ones present as well.
>>
>> Similarly it was discussed the potential `consttime=yes` property that
>> would work in a similar way: if applied properly for our current
>> implementations that are not fully constant time it would allow a
>> user/sysadmin/developer to prefer a third party implementation for the
>> same algorithm with better security guarantees.
>> In some contexts the consttime implementation might be seriously
>> penalizing in terms of performance and in the contexts where const time
>> is not required this would allow to select accordingly.
>>
>> Definition for the blinding property
>> 
>>
>> The current definition of the `blinding` property applies to
>> provider-native algorithm implementations for the `asym_cipher` and
>> `signature` operations:

[OTC VOTE PROPOSAL] Don't merge PR#14759 (blinding=yes and similar properties)

[Nicola Tuveri]

Background
==

[PR#14759](https://github.com/openssl/openssl/pull/14759) (Set
blinding=yes property on some algorithm implementations) is a fix for
[Issue#14654](https://github.com/openssl/openssl/issues/14654) which
itself is a spin-off of
[Issue#14616](https://github.com/openssl/openssl/issues/14616).

The original issue is about _Deprecated low level key API's that have no
replacements_, among which the following received special attention and
were issued a dedicated issue during an OTC meeting:

~~~c
// RSA functions on RSA_FLAG_*
void RSA_clear_flags(RSA *r, int flags);
int RSA_test_flags(const RSA *r, int flags);
void RSA_set_flags(RSA *r, int flags);

// RSA_FLAG_* about blinding
#define RSA_FLAG_BLINDING
#define RSA_FLAG_NO_BLINDING

// RSA functions directly on blinding
int RSA_blinding_on(RSA *rsa, BN_CTX *ctx);
void RSA_blinding_off(RSA *rsa);
BN_BLINDING *RSA_setup_blinding(RSA *rsa, BN_CTX *ctx);
~~~

The decision the sprung Issue#14616 and PR#14759 was to use the
propquery mechanism to let providers advertise algorithms as
`blinding=yes` to select secure implementations if there are insecure
ones present as well.

Similarly it was discussed the potential `consttime=yes` property that
would work in a similar way: if applied properly for our current
implementations that are not fully constant time it would allow a
user/sysadmin/developer to prefer a third party implementation for the
same algorithm with better security guarantees.
In some contexts the consttime implementation might be seriously
penalizing in terms of performance and in the contexts where const time
is not required this would allow to select accordingly.

Definition for the blinding property


The current definition of the `blinding` property applies to
provider-native algorithm implementations for the `asym_cipher` and
`signature` operations:

```pod
=head2 Properties

The following property is set by some of the OpenSSL signature
algorithms.

=over 4

=item "blinding"

This boolean property is set to "yes" if the implementation performs
blinding to prevent some side-channel attacks.
```

Rationale
=

Property queries are our decision making process for implementation
selection, and has been part of the design for 3.0 since the Brisbane
design meetings: I am not opposing the use of the property query
mechanism to select algorithms here, but the semantics of the properties
we decide to adopt (and thus endorse and encourage also for use by 3rd
parties).
In particular I see the following issues with choices like
`blinding=yes` or `consttime=yes`.

Design promotes insecure by default
---

This design is a slippery slope into patterns that go against the
"secure by default" best practices.
Users/administrators/developers should not have to opt-in for the safer
implementation, but the other way around: the opt-in should be for the
less safe but more performant implementations after evaluating the
consequences of such a choice in specific contexts.
We shouldn't have users having to query for `consttime=yes` algorithms
but rather for `consttime=no` explicitly in specific conditions.
So if this was the only issue with PR#14759 my recommendation would be
to rather flag the non-blinding implementations as such rather than the
other way around as is currently done.

The scenario in which 3rd party providers offer insecure algorithms not
flagged as such with `consttime=no` or `blinding=no` IMO would then fall
under the "know your providers" umbrella: if you are installing provider
X you should be well aware that you are trusting X's authors "to do the
right thing".

The project history showed us how the risk for insecure-by-default
designs and its consequences are not just an hypothetical matter of
debate: for example, `BN_FLG_CONSTTIME` has been with us since version
0.9.8f (and even before that under the `BN_FLG_EXP_CONSTTIME` name),
well over 14 years, in which the decision of having the flag off by
default and requiring to manually enabling it when desired has been the
cause of many vulnerabilities and fixes, that are still a concern today,
as we fixed yet another instance of forgetting to set it just some weeks
ago (PR#13889).

In this case, though, I expect that just flipping the default is not
enough to accept this PR: the problem of going with the negative
versions of these flags is that such design can't be made future proof
as we wish to do: we can't know in advance all possible properties that
should be declared as `=no` by an algorithm released today as part of
our providers when new relevant properties might be defined in the
future.
E.g., what if after 3.0 is released a ECCKiila provider was released and
opted to tag its implementations as `formally_verified`?
They could add `formally_verified=yes` but then they would fall into the
"insecure by default" pattern: users would need to opt-in for the safer
version instead of getting it by default, 

Re: OTC VOTE: Disallow SM2 with a non-SM2 curve

[Nicola Tuveri]

h
on the inner curve OID, so parsing the PEM above results in a key object
belonging to the SM2 keymgmt.
Because `EVP_PKEY_set_alias_type()` is not supported, in current 3.0 it is
hard (if not impossible) to flexibly use EC/SM2 keys as in 1.1.1, but it is
still possible to do something with ephemeral keys (e.g. one can initialize
an SM2 keygen EVP_PKEY_CTX and set P-521 as the curve, which ultimately
delivers an SM2 key object over an arbitrary curve that can be used, for
example, for SM2DSA operations as demonstrated by
0001-drop-3.0.0-test-SM2DSA-and-SM2ENC-DEC-over-NIST-P-52.patch).


Nicola


On Tue, Mar 9, 2021 at 8:50 PM Kurt Roeckx  wrote:

> On Tue, Mar 09, 2021 at 03:57:32PM +0200, Nicola Tuveri wrote:
> > It is tangent to the vote in itself, but I'd like to highlight that in
> part
> > this vote is motivated by getting rid of cases where there is a need to
> > convert between compatible key types (e.g. SM2 & EC, DH & DHX).
> >
> > The vote of this text does not cover it, but another use case that we
> will
> > likely lose in 3.0 when this vote passes is using ECDH/ECDSA over the
> curve
> > identified by the SM2 OID.
> >
> > It's likely another niche use, not relevant for any production system,
> but
> > it is creating a precedent where OpenSSL accepts that the SM2 standard
> > overrides the SECG standard.
> >
> > In 1.1.1 we do _default_ to decode a key serialized as "SECG EC over the
> > SM2 OID curve" as a SM2 key, but we are not preventing developers from
> > creating an EC key object over that curve, nor an SM2 key object over any
> > other curve (serialization of such "weird" key objects would be
> identical,
> > deserialization would require an explicit set_alias call to mutate from
> the
> > default type).
> >
> > With the changes proposed as a corollary to this vote that were discussed
> > on the last OTC meeting, we would remove this ability: we could maybe
> still
> > create an ephemeral "EC over SM2 OID" key object but there would be no
> way
> > to deserialize one.
> >
> >
> > I would instead prefer to vote on requiring for 3.0 a mechanism to
> > "convert" key types: an EVP_PKEY_todata function to mirror
> > EVP_PKEY_fromdata.
> > It will be then up to the individual keymgmt of the source key object to
> > decide if it can be exported to OSSL_PARAMs, and to the receiving keymgmt
> > on which fromdata is called to determine if the input OSSL_PARAMs are
> valid
> > to create a proper key object.
> > Yes, we would still have a breaking change (set_alias needs to be
> replaced
> > by the todata/fromdata dance) but we wouldn't lose functionality.
> >
> > Such a mechanism could be useful also beyond the current problem of key
> > conversions within the builtin providers, and allow future proofing and
> > interoperability between different providers that might end up using
> > different names for compatible key types (for whatever reason it might
> > happen).
>
> This text does not seem to help me to make a vote, it seems I just
> get more questions.
>
> The EVP_PKEY_set_alias_type() manpage says that SM2 use the same
> encoding an ECDSA. I assume that during deserialization we don't
> know if we should create an ECDSA key or an SM2 key, because we
> don't look at the OID when deserializing and always create an
> ECDSA key. My understanding is that there is code that needs to
> know an SM2 key is stored in that object, and is not looking at
> the OID, but instead looks at what the pkey type is. My suggested
> fix for that would be to make the deserialization smarter so it
> sets the correct pkey type base on the OID.
>
> Assuming that we can get an SM2 key, I currently fail to see why
> ECDSA or ECDH wouldn't work with them, but I know very little
> about SM2.
>
> I also don't understand what the vote is really about. Can 1.1.1
> do SM2 on one of the nist curves?
>
>
> Kurt
>
>
From af5495b1624bb69aafc38578056c7bee9efefb73 Mon Sep 17 00:00:00 2001
From: Nicola Tuveri 
Date: Wed, 10 Mar 2021 01:58:37 +0200
Subject: [PATCH 1/2] drop! test SM2DSA and SM2ENC/DEC over NIST P-521 curve

---
 test/evp_extra_test.c | 23 +--
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/test/evp_extra_test.c b/test/evp_extra_test.c
index a74f6332ac..81826954b7 100644
--- a/test/evp_extra_test.c
+++ b/test/evp_extra_test.c
@@ -693,11 +693,8 @@ static int test_EVP_SM2(void)
 EVP_MD_CTX *md_ctx_verify = NULL;
 EVP_PKEY_CTX *cctx = NULL;
 
-uint8_t ciphertext[128];
-size_t ctext_len = sizeof(ciphertext);
-
-uint8_t plaintext[8];
-size_t ptext_len = sizeof(plaintext);
+uint8_t *

Re: OTC VOTE: Disallow SM2 with a non-SM2 curve

[Nicola Tuveri]

It is tangent to the vote in itself, but I'd like to highlight that in part
this vote is motivated by getting rid of cases where there is a need to
convert between compatible key types (e.g. SM2 & EC, DH & DHX).

The vote of this text does not cover it, but another use case that we will
likely lose in 3.0 when this vote passes is using ECDH/ECDSA over the curve
identified by the SM2 OID.

It's likely another niche use, not relevant for any production system, but
it is creating a precedent where OpenSSL accepts that the SM2 standard
overrides the SECG standard.

In 1.1.1 we do _default_ to decode a key serialized as "SECG EC over the
SM2 OID curve" as a SM2 key, but we are not preventing developers from
creating an EC key object over that curve, nor an SM2 key object over any
other curve (serialization of such "weird" key objects would be identical,
deserialization would require an explicit set_alias call to mutate from the
default type).

With the changes proposed as a corollary to this vote that were discussed
on the last OTC meeting, we would remove this ability: we could maybe still
create an ephemeral "EC over SM2 OID" key object but there would be no way
to deserialize one.


I would instead prefer to vote on requiring for 3.0 a mechanism to
"convert" key types: an EVP_PKEY_todata function to mirror
EVP_PKEY_fromdata.
It will be then up to the individual keymgmt of the source key object to
decide if it can be exported to OSSL_PARAMs, and to the receiving keymgmt
on which fromdata is called to determine if the input OSSL_PARAMs are valid
to create a proper key object.
Yes, we would still have a breaking change (set_alias needs to be replaced
by the todata/fromdata dance) but we wouldn't lose functionality.

Such a mechanism could be useful also beyond the current problem of key
conversions within the builtin providers, and allow future proofing and
interoperability between different providers that might end up using
different names for compatible key types (for whatever reason it might
happen).




Nicola

On Tue, Mar 9, 2021, 12:24 Matt Caswell  wrote:

> topic: In 3.0 it will not be possible to use SM2 with a non-SM2 curve. This
> should be documented.
> Proposed by Matt Caswell
> Public: yes
> opened: 2021-03-09
> closed: 2021-mm-dd
> accepted:  yes/no  (for: X, against: Y, abstained: Z, not voted: T)
>
>Matt   [+1]
>Mark   [  ]
>Pauli  [+1]
>Viktor [  ]
>Tim[+1]
>Richard[+1]
>Shane  [+1]
>Tomas  [ 0]
>Kurt   [  ]
>Matthias   [  ]
>Nicola [-1]
>
>

Re: OTC VOTE: Fixing missing failure exit status is a bug fix

[Nicola Tuveri]

The vote is now closed, and accepted!



> topic: In the context of the OpenSSL apps, the OTC qualifies as bug
>fixes the changes to return a failure exit status when a called
>function fails with an unhandled return value.
>Even when these bug fixes change the apps behavior triggering
>early exits (compared to previous versions of the apps), as bug
>fixes, they do not qualify as behavior changes that require an
>explicit OMC approval.
> Proposed by Nicola Tuveri
> Public: yes
> opened: 2020-11-30
> closed: 2020-12-21
> accepted:  yes  (for: 9, against: 0, abstained: 0, not voted: 2)
>
>   Matt   [+1]
>   Mark   [  ]
>   Pauli  [+1]
>   Viktor [  ]
>   Tim[+1]
>   Richard[+1]
>   Shane  [+1]
>   Tomas  [+1]
>   Kurt   [+1]
>   Matthias   [+1]
>   Nicola [+1]

On Mon, Nov 30, 2020 at 2:03 PM Nicola Tuveri  wrote:
>
> Vote background
> ---
>
> This follows up on a [previous proposal] that was abandoned in favor of
> an OMC vote on the behavior change introduced in [PR#13359].
> Within today's OTC meeting this was further discussed with the attending
> members that also sit in the OMC.
>
> The suggestion was to improve the separation of the OTC and OMC domains
> here, by having a more generic OTC vote to qualify as bug fixes the
> changes to let any OpenSSL app return an (early) failure exit status
> when a called function fails.
>
> The idea is that, if we agree on this technical definition, then no OMC
> vote to allow a behavior change in the apps would be required in
> general, unless, on a case-by-case basis, the "OMC hold" process is
> invoked for whatever reason on the specific bug fix, triggering the
> usual OMC decision process.
>
> [previous proposal]:
> <https://www.mail-archive.com/openssl-project@openssl.org/msg02241.html>
> [PR#13359]: <https://github.com/openssl/openssl/pull/13359>
>
>
>
> Vote text
> -
>
> topic: In the context of the OpenSSL apps, the OTC qualifies as bug
>fixes the changes to return a failure exit status when a called
>function fails with an unhandled return value.
>Even when these bug fixes change the apps behavior triggering
>    early exits (compared to previous versions of the apps), as bug
>fixes, they do not qualify as behavior changes that require an
>explicit OMC approval.
> Proposed by Nicola Tuveri
> Public: yes
> opened: 2020-11-30

Re: OpenSSL Cryticality Score

[Nicola Tuveri]

As an update on the issue of some fields being not entirely accurate.

I am forwarding a message on behalf of @inferno-chromium, the
maintainer of the https://github.com/ossf/criticality_score project
that followed up on the [Github issue] I opened about this.

> Thanks for notifying us of the issue with incorrect project creation
> date issue, we do plan to look into it and see feasibility of picking
> the first commit date for accuracy. In case of openssl, it would have
> little to no-impact on criticality score, as other factors clearly
> indicate it is a super-critical project. These include things like
> users dependent on openssl library, number of project contributors and
> user activity in terms of issues filed, updated.


[Github issue]: https://github.com/ossf/criticality_score/issues/14

On Fri, Dec 11, 2020 at 11:54 AM Nicola Tuveri  wrote:
>
> On Fri, Dec 11, 2020 at 11:23 AM Matt Caswell  wrote:
> >
> >
> > Actually according to the spreadsheet we are 5th (not 6th) - line 1 in
> > the sheet is the title row. Linux takes 2 of the top spots, with git and
> > php taking the other spots ahead of OpenSSL.
>
>
> Good, it's good that the double review process catches my off-by-one
> errors also on the mailing list ;)
>
> >
> >
> > Not sure I understand the "Releases (last yr)" column which says we did
> > 41 releases - that's a number I can't reconcile with the actual number
> > of releases we did.
> >
>
> https://github.com/ossf/criticality_score/blob/59e449d5598de4f27a83070297e5779a4a3407b2/criticality_score/run.py#L96-L114
>
> It seems to be an estimate based on the number of tags, as we don't do
> github releases:
>
> ```
> RELEASE_LOOKBACK_DAYS=365
> (total_tags / days_since_creation) * RELEASE_LOOKBACK_DAYS
> ```
>
> This is definitely skewed by considering the project 95 months old
> (2887 days) instead of ~264 months (8026 days).
>
>
> Nicola

Re: OpenSSL Cryticality Score

[Nicola Tuveri]

On Fri, Dec 11, 2020 at 11:23 AM Matt Caswell  wrote:
>
>
> Actually according to the spreadsheet we are 5th (not 6th) - line 1 in
> the sheet is the title row. Linux takes 2 of the top spots, with git and
> php taking the other spots ahead of OpenSSL.


Good, it's good that the double review process catches my off-by-one
errors also on the mailing list ;)

>
>
> Not sure I understand the "Releases (last yr)" column which says we did
> 41 releases - that's a number I can't reconcile with the actual number
> of releases we did.
>

https://github.com/ossf/criticality_score/blob/59e449d5598de4f27a83070297e5779a4a3407b2/criticality_score/run.py#L96-L114

It seems to be an estimate based on the number of tags, as we don't do
github releases:

```
RELEASE_LOOKBACK_DAYS=365
(total_tags / days_since_creation) * RELEASE_LOOKBACK_DAYS
```

This is definitely skewed by considering the project 95 months old
(2887 days) instead of ~264 months (8026 days).


Nicola

OpenSSL Cryticality Score

[Nicola Tuveri]

Hi all,

just sharing an interesting factoid I came across today about the project.

Google, as part of the Open Source Security Foundation, yesterday released
a new project dubbed "Criticality Score", attempting (I am simplifying here
for brevity) to create a metric of "how critical" a software is in the
software ecosystem.
You can read more accurate info about it here:
https://opensource.googleblog.com/2020/12/finding-critical-open-source-projects.html

They publish the collected metadata and the resulting score (based on the
formula described at ) online as
a CSV file.

Sidenote: Notice the data seems to refer only to whatever the github API
for a repo says, so for example OpenSSL is only 95 months old because
that's when the github mirror was created (I opened an issue about this).

Anyway, they split the data by language, and, among the analyzed C
projects, OpenSSL expectedly scores quite high, being 6th in the top 200
measured C projects.

Here is a link directly to the data:
https://commondatastorage.googleapis.com/ossf-criticality-score/index.html


Cheers,

Nicola

OTC VOTE: Fixing missing failure exit status is a bug fix

[Nicola Tuveri]

Vote background
---

This follows up on a [previous proposal] that was abandoned in favor of
an OMC vote on the behavior change introduced in [PR#13359].
Within today's OTC meeting this was further discussed with the attending
members that also sit in the OMC.

The suggestion was to improve the separation of the OTC and OMC domains
here, by having a more generic OTC vote to qualify as bug fixes the
changes to let any OpenSSL app return an (early) failure exit status
when a called function fails.

The idea is that, if we agree on this technical definition, then no OMC
vote to allow a behavior change in the apps would be required in
general, unless, on a case-by-case basis, the "OMC hold" process is
invoked for whatever reason on the specific bug fix, triggering the
usual OMC decision process.

[previous proposal]:
<https://www.mail-archive.com/openssl-project@openssl.org/msg02241.html>
[PR#13359]: <https://github.com/openssl/openssl/pull/13359>



Vote text
-

topic: In the context of the OpenSSL apps, the OTC qualifies as bug
   fixes the changes to return a failure exit status when a called
   function fails with an unhandled return value.
   Even when these bug fixes change the apps behavior triggering
   early exits (compared to previous versions of the apps), as bug
   fixes, they do not qualify as behavior changes that require an
   explicit OMC approval.
Proposed by Nicola Tuveri
Public: yes
opened: 2020-11-30

Re: [OTC VOTE PROPOSAL] Fixing missing failure exit status is a bug fix

[Nicola Tuveri]

Off-list I received a suggestion to use "unhandled" (thanks again!).

I am not sure about it (I actually originally discarded in my first
draft of this proposal) because I am afraid that talking of "unhandled
return value" from a function might be misinterpreted as cases in
which we are ignoring the return value of a callee or in which we are
not considering the whole set of possible return values.

Anyway, now that I disclaimed my doubts about it, I would submit to
your consideration this alternative to the original vote text:

In the context of the OpenSSL apps, we qualify as bug fixes the
changes to return a failure exit status when a called function
fails with an unhandled return value.
Even when these bug fixes change the apps behavior triggering
early exits (compared to previous versions of the apps), as bug
fixes, they do not qualify as behavior changes that require an
explicit OMC approval.


Would this be preferable to the original proposal?


Cheers,

Nicola

On Tue, Nov 24, 2020 at 7:33 PM Nicola Tuveri  wrote:
>
> Uhm, thanks Kurt! I think you have a point here, "unrecoverably" might
> be a poor choice.
>
> Do you have a proposal to qualify these cases in general? Or a better
> way to rephrase it?
>
> I wasn't happy with "In the context of the OpenSSL apps, we qualify as
> bug fixes the changes to return a failure exit status when a called
> function fails." (i.e., omitting unrecoverably or a better term): it
> is not uncommon to have function failures that are intended to be
> handled, taking a different course of action.
>
>
> @tjh, as the main driver of a more generic vote like this, do you have
> a better vote text proposal?
>
>
> Nicola
>
>
> On Tue, Nov 24, 2020, 19:18 Kurt Roeckx  wrote:
> >
> > On Tue, Nov 24, 2020 at 01:51:44PM +0200, Nicola Tuveri wrote:
> > > Background
> > > --
> > >
> > > This follows up on a [previous proposal] that was abandoned in favor of
> > > an OMC vote on the behavior change introduced in [PR#13359].
> > > Within today's OTC meeting this was further discussed with the attending
> > > members that also sit in the OMC.
> > >
> > > The suggestion was to improve the separation of the OTC and OMC domains
> > > here, by having a more generic OTC vote to qualify as bug fixes the
> > > changes to let any OpenSSL app return an (early) failure exit status
> > > when a called function fails.
> > >
> > > The idea is that, if we agree on this technical definition, then no OMC
> > > vote to allow a behavior change in the apps would be required in
> > > general, unless, on a case-by-case basis, the "OMC hold" process is
> > > invoked for whatever reason on the specific bug fix, triggering the
> > > usual OMC decision process.
> > >
> > > Proposed vote text
> > > --
> > >
> > > In the context of the OpenSSL apps, we qualify as bug fixes the
> > > changes to return a failure exit status when a called function
> > > fails unrecoverably.
> >
> > I'm not sure the unrecoverably should be there. I think this was
> > about verifying is a public or private key was valid. If such a
> > function returns it's not valid, I don't call that an
> > unrecoverable error. But I do expect the application to return an
> > error exit code.
> >
> > An other example is s_client, which ignores verification errors by
> > default. You can change that behaviour with -verify_return_error. If
> > you do that, s_client will actually exit with return value 1 in case
> > of a verification error.
> >
> >
> > Kurt
> >
> >

Re: [OTC VOTE PROPOSAL] Fixing missing failure exit status is a bug fix

[Nicola Tuveri]

Uhm, thanks Kurt! I think you have a point here, "unrecoverably" might
be a poor choice.

Do you have a proposal to qualify these cases in general? Or a better
way to rephrase it?

I wasn't happy with "In the context of the OpenSSL apps, we qualify as
bug fixes the changes to return a failure exit status when a called
function fails." (i.e., omitting unrecoverably or a better term): it
is not uncommon to have function failures that are intended to be
handled, taking a different course of action.


@tjh, as the main driver of a more generic vote like this, do you have
a better vote text proposal?


Nicola


On Tue, Nov 24, 2020, 19:18 Kurt Roeckx  wrote:
>
> On Tue, Nov 24, 2020 at 01:51:44PM +0200, Nicola Tuveri wrote:
> > Background
> > --
> >
> > This follows up on a [previous proposal] that was abandoned in favor of
> > an OMC vote on the behavior change introduced in [PR#13359].
> > Within today's OTC meeting this was further discussed with the attending
> > members that also sit in the OMC.
> >
> > The suggestion was to improve the separation of the OTC and OMC domains
> > here, by having a more generic OTC vote to qualify as bug fixes the
> > changes to let any OpenSSL app return an (early) failure exit status
> > when a called function fails.
> >
> > The idea is that, if we agree on this technical definition, then no OMC
> > vote to allow a behavior change in the apps would be required in
> > general, unless, on a case-by-case basis, the "OMC hold" process is
> > invoked for whatever reason on the specific bug fix, triggering the
> > usual OMC decision process.
> >
> > Proposed vote text
> > --
> >
> > In the context of the OpenSSL apps, we qualify as bug fixes the
> > changes to return a failure exit status when a called function
> > fails unrecoverably.
>
> I'm not sure the unrecoverably should be there. I think this was
> about verifying is a public or private key was valid. If such a
> function returns it's not valid, I don't call that an
> unrecoverable error. But I do expect the application to return an
> error exit code.
>
> An other example is s_client, which ignores verification errors by
> default. You can change that behaviour with -verify_return_error. If
> you do that, s_client will actually exit with return value 1 in case
> of a verification error.
>
>
> Kurt
>
>

[OTC VOTE PROPOSAL] Fixing missing failure exit status is a bug fix

[Nicola Tuveri]

Background
--

This follows up on a [previous proposal] that was abandoned in favor of
an OMC vote on the behavior change introduced in [PR#13359].
Within today's OTC meeting this was further discussed with the attending
members that also sit in the OMC.

The suggestion was to improve the separation of the OTC and OMC domains
here, by having a more generic OTC vote to qualify as bug fixes the
changes to let any OpenSSL app return an (early) failure exit status
when a called function fails.

The idea is that, if we agree on this technical definition, then no OMC
vote to allow a behavior change in the apps would be required in
general, unless, on a case-by-case basis, the "OMC hold" process is
invoked for whatever reason on the specific bug fix, triggering the
usual OMC decision process.

Proposed vote text
--

In the context of the OpenSSL apps, we qualify as bug fixes the
changes to return a failure exit status when a called function
fails unrecoverably.
Even when these bug fixes change the apps behavior triggering
early exits (compared to previous versions of the apps), as bug
fixes, they do not qualify as behavior changes that require an
explicit OMC approval.


[previous proposal]:
https://www.mail-archive.com/openssl-project@openssl.org/msg02241.html
[PR#13359]: https://github.com/openssl/openssl/pull/13359

Re: OTC VOTE: EVP_PKEY private/public key components

[Nicola Tuveri]

I will only answer what I see as new points in your last email, to
avoid repeating ourselves.

Validity of a serialized key:
- ASN.1 validity says little about the validity of the key, as ASN.1
cannot enforce limits on the private scalar range
- validity as a serialized EC key requires that in addition to be
valid ASN.1, other criteria are satisfied:
  - (this is assuming a named curve, with explicit parameters things
get more involved, as we need to validate those before looking at
private and public components)
  - private scalar is in the prescribed range for the curve
  - if optional public component is present, the public key encoding
matches the SECG standard in one of the supported compression formats
  - if optional public component is present, the public key point
belongs to the curve
  - if optional public component is present, the public key point
belongs to the prime subgroup of the curve (this collapses into the
previous point only if the cofactor for this curve is 1)
  - if optional public component is present, the public key point matches `k*G`


I confirm what you saw in your experiments, the validity checks (if
invoked!) catch these corner cases (I did similar experiments when
preparing the EC part of #13359).


The fact that you had to alter `ec_key_simple_check_key()` to skip the
public component validation, which is necessary also in 1.1.1 and
earlier, is the reason why I raised this side discussion on key
validation, as disproving the "lack of enforcement" argument: but I
posted this on the public openssl-project list only after verifying we
did not have gaping security holes.


Nicola

On Mon, Nov 16, 2020 at 8:51 PM Richard Levitte  wrote:
>
> This is what I read:
>
> - the key in p256_invalid.pem is invalid in other ways than merely the
>   lack of the public key in the file.
>   (the public key is *not* lacking in run-time in this example, since
>   d2i_ECPrivateKey() autogenerates it, which happens before it's
>   checked or used)
>
> - you confirmed that the public key isn't really relevant for this
>   problem, apart from triggering a check error that's really emanating
>   from some other invalid data.
>
> This does not, however, demonstrate what happens when the EVP_PKEY
> holds an EC_KEY that lacks a public key, because in run-time, it in
> fact does *not* lack a public key.  So while that's an interesting
> discussion, it doesn't really demonstrate your concerns about this
> vote.
>
> BTW, regarding checking, I did a bit of an experiment; I removed the
> autogeneration of the public key from d2i_ECPrivateKey(), and relaxed
> ec_key_simple_check_key() so it only checks eckey->pub_key if it's
> actually present...  and of course removeed a few eckey->pub_key NULL
> checks along the way.  EVP_PKEY_check still fails, but now with the
> detailed error "wrong order", which sounds perfectly reasonable
> considering your description, so it looks to me like the invalidity
> of the key can be caught either way.
>
> From a contents point of view, p256_invalid.pem is actually perfectly
> valid, or you would have gotten ASN1 errors when trying to load it.
>
> Cheers,
> Richard
>
> P.S.  I'd like to pause the debate at this point...  it looks like
> we're getting back into locked positions, and I'm not keen on
> continuing under those conditions.
>
> On Mon, 16 Nov 2020 19:06:51 +0100,
> Nicola Tuveri wrote:
> >
> > The issue with that key is that the secret scalar `k` is equal to `n`
> > (where `n` is the order of the generator point `G`), while for EC keys
> > the validity range is `1 <= k < n`.
> >
> > If the scalar `k` is equal to `n`, it means that the associated pubkey
> > is `k*G` = `n*G` = `0*G mod n` = `P_infinity`.
> > The pubkey generation in the `d2i_` routines is correctly being
> > triggered because the PEM file I generated only includes the secret
> > scalar: if we did not catch the point at infinity validating the
> > public component we would reach the private component validity checks
> > and we would trigger the private scalar range check.
> >
> > The infinite loop happens not because of the public component (that as
> > we know is not touched during signature generation) but because the
> > secret scalar is effectively congruent to `0 mod n` in the computation
> > to generate the `s` value of the signature.
> >
> > I would not classify this as a bug, but as a programmer error: the
> > user is using an invalid key (this has nothing to do with the "keypair
> > assumption", literally `k` is a value out of range according to the
> > relevant spec).
> > Input key material should be validated: if not at each run (for
> > performance reason), once after it has been serializ

Re: Introducing Paul Nelson

[Nicola Tuveri]

That's great news!



Welcome Paul!


Nicola

On Mon, Nov 16, 2020 at 7:15 PM Matt Caswell  wrote:
>
> You may recall that some while ago we posted a blog where we were
> looking for an Administrator and Manager to help run the project:
>
> https://www.openssl.org/blog/blog/2020/09/05/OpenSSL.ProjectAdminRole/
>
> I am delighted to be able to tell you that we have appointed Paul Nelson
> to this position. He is starting in the role today. I will let him
> introduce himself once he has managed to get his email access sorted!
>
> I'd like to welcome Paul to the project!
>
> Matt
>

Re: OTC VOTE: EVP_PKEY private/public key components

[Nicola Tuveri]

The issue with that key is that the secret scalar `k` is equal to `n`
(where `n` is the order of the generator point `G`), while for EC keys
the validity range is `1 <= k < n`.

If the scalar `k` is equal to `n`, it means that the associated pubkey
is `k*G` = `n*G` = `0*G mod n` = `P_infinity`.
The pubkey generation in the `d2i_` routines is correctly being
triggered because the PEM file I generated only includes the secret
scalar: if we did not catch the point at infinity validating the
public component we would reach the private component validity checks
and we would trigger the private scalar range check.

The infinite loop happens not because of the public component (that as
we know is not touched during signature generation) but because the
secret scalar is effectively congruent to `0 mod n` in the computation
to generate the `s` value of the signature.

I would not classify this as a bug, but as a programmer error: the
user is using an invalid key (this has nothing to do with the "keypair
assumption", literally `k` is a value out of range according to the
relevant spec).
Input key material should be validated: if not at each run (for
performance reason), once after it has been serialized to disk and
protected with proper measures to ensure the validated key material is
not tampered with (or leaked).


If we consider this a bug, or a potential DoS attack vector, we would
likely fix it by running validation of the `EVP_PKEY` object on load
(and with some caching mechanism to validate keys created manually via
`EC_KEY` objects): this would once again reveal that the use pattern
in #12612 was invalid to begin with, as the validity checks were
enforcing the "keypair assumption" in 1.1.1 and previous versions.


Nicola

On Mon, Nov 16, 2020 at 7:44 PM Richard Levitte  wrote:
>
> Er, Nicola, I'm unsure how that endless loop has anything to do with
> the presence or the absence of a public key, as it happens in
> ossl_ecdsa_sign_sig(), which doesn't even look at the public key, at
> all.
>
> Your check does say that the key you have there is invalid, but that
> would rather be because from that private key and group, it seems that
> d2i_ECPrivateKey() generates a public key with Z == 0, which is indeed
> infinity as I understand it.  You can see that for yourself with a
> breakpoint at d2i_ECPrivateKey(), step down to about line 1042
> (current OpenSSL_1_1_1-stable, btw), and simply look:
>
> (gdb) p *ret->pub_key
> $16 = {meth = 0x77f0dc00 , curve_name = 415, X = 0x556450f0,
>   Y = 0x55645090, Z = 0x556450b0, Z_is_one = 0}
> (gdb) p *ret->pub_key->Z
> $17 = {d = 0x55641170, top = 0, dmax = 4, neg = 0, flags = 1}
> (gdb) p *ret->pub_key->X
> $18 = {d = 0x55641ec0, top = 4, dmax = 4, neg = 0, flags = 1}
> (gdb) p *ret->pub_key->Y
> $19 = {d = 0x55641e40, top = 4, dmax = 4, neg = 0, flags = 1}
>
> (ret->pub_key->Z->top == 0, that's a bignum zero)
>
> That still has no impact on the infinite loop as far as I can see, but
> that may be an indication that something else is wrong with that
> private key.
>
> It's also possible that if there are conditions that may cause an
> infinite loop, that is a bug in itself that needs a fix.
>
> I believe this loop is due for a raised issue, unless there already is
> one.
> (if there isn't, I wonder why)
>
> Cheers,
> Richard
>
> On Thu, 12 Nov 2020 11:33:13 +0100,
> Nicola Tuveri wrote:
> >
> > > Nonsense.  Each iteration involves a new PRN, which by definition you 
> > > cannot predict.
> >
> > ~~~sh
> > ; which openssl; openssl version
> > /usr/bin/openssl
> > OpenSSL 1.1.1f  31 Mar 2020
> > ; cat > /tmp/p256_invalid.pem
> > -BEGIN PRIVATE KEY-
> > MEECAQAwEwYHKoZIzj0CAQYIKoZIzj0DAQcEJzAlAgEBBCD/AP//
> > vOb6racXnoTzucrC/GMlUQ==
> > -END PRIVATE KEY-
> > ; openssl pkey -check -text -noout -in /tmp/p256_invalid.pem
> > Key is invalid
> > Detailed error: point at infinity
> > Private-Key: (256 bit)
> > priv:
> > ff:ff:ff:ff:00:00:00:00:ff:ff:ff:ff:ff:ff:ff:
> > ff:bc:e6:fa:ad:a7:17:9e:84:f3:b9:ca:c2:fc:63:
> > 25:51
> > pub:
> > 00
> > ASN1 OID: prime256v1
> > NIST CURVE: P-256
> > ; dd if=/dev/zero of=/tmp/foo.hash bs=1 count=32
> > ; openssl pkeyutl -sign -inkey /tmp/p256_invalid.pem -in /tmp/foo.hash
> > -out /tmp/sig.der
> > # here is the infinite loop
> > ~~~
>
> --
> Richard Levitte levi...@openssl.org
> OpenSSL Project http://www.openssl.org/~levitte/

Re: OTC VOTE: EVP_PKEY private/public key components

[Nicola Tuveri]

It exists in master but not in 1.1.1 or previous: the incomplete EVP_PKEY
object of #12612 in previous versions would have failed validation, where
its correctness as a math entity and as a programming object is tested.

That is why I am baffled by the use of "regression" to describe this issue,
and the reason why I commented on the "lack of enforcement" argument.

Cheers,

Nicola

On Mon, Nov 16, 2020, 18:53 Richard Levitte  wrote:

> On Wed, 11 Nov 2020 23:34:53 +0100,
> Nicola Tuveri wrote:
> >
> > By design the consistency checks and validation checks are omitted
> > unless these functions are called, and there is no
> > EVP_PKEY_private_check.
>
> Just a small point, this is in master:
>
> $ grep private_check include/openssl/evp.h
> int EVP_PKEY_private_check(EVP_PKEY_CTX *ctx);
>
> Cheers,
> Richard
>
> --
> Richard Levitte levi...@openssl.org
> OpenSSL Project http://www.openssl.org/~levitte/
>

Re: OTC VOTE: EVP_PKEY private/public key components

[Nicola Tuveri]

On Thu, Nov 12, 2020 at 11:42 AM Dick Franks  wrote:
>
> On Wed, 11 Nov 2020 at 14:14, Nicola Tuveri  wrote:
>>
>>
>> In particular in 1.1.1, the key created as depicted in #12612 that
>> triggered this discussion (Matt posted a useful reproducer among the
>> first comments), is indeed capable of signing in the used pattern, but
>> the pattern is conveniently omitting the validation pass that should
>> be required in any serious use of the API.
>
>
> The private key is a random or pseudo-random 256-bit integer.
> How do you propose to "validate" that?

For ECDSA it's not a a random or pseudo-random 256-bit integer: it's a
random or pseudo-random integer `k`, with `1 <= k < n`, not all
256-bit integers fit into this definition for a 256-bit prime `n`
(where `n` is the order of the generator point for the curve.
Validating the private key guarantees that the input private scalar is
within the correct range.

Sidenote: I don't know how the software in question does keygen, if it
is happening outside of OpenSSL or not, but validating the key
generation step is also crucial, because the random integer generation
should have a uniform distribution over the whole range without any
biases.

>>
>> `EVP_PKEY_check()`
>> (https://www.openssl.org/docs/man1.1.1/man3/EVP_PKEY_check.html) is
>> one of the many places in 1.1.1 where both the documentation and the
>> behavior assume that an `EVP_PKEY` object is a keypair.
>> Even in the version used by the user that posted the issue, running
>> `EVP_PKEY_check()` on the created key would have revealed that the
>> user was abusing the API.
>
>
>  I was not "abusing the API" as you put it, merely pointing out that the 
> public key is not a required item for performing ECDSA signature generation.  
> This is a mathematical fact of life that you are going to have to learn to 
> live with.
>

If the documentation of the API says that an `EVP_PKEY` must include a
full keypair, and the functions to validate a manually initialized
`EVP_PKEY` object enforce this, then I stand by my assessment that
writing software that intentionally initializes a non-compliant
`EVP_PKEY` object which would not pass validation functions of the API
(where the documented assumption is enforced) configures as an "API
abuse" from a programming point of view.

I agree that performing an ECDSA signature generation does not require
the public key, and that would be reason in favor of relaxing the
assumption.
I am not saying it is wrong to want to generate signatures only with
the private key, I am saying that it does not fit well with the
OpenSSL API design (and with most other cryptographic libraries, not
just OpenSSL forks, that also have similar "keypair assumptions").

A counter argument to yours is that ECDSA signature verification
requires only the public component, and it is a mathematical fact of
life that knowledge of the private component in this case implies
knowledge of the public component.
A user of a cryptographic library can therefore expect that an
abstract key object embedding knowledge of the private key is capable
of being used in operations requiring knowledge of the public
component, including ECDSA signature verification.
Computing the public component on demand only when such operation is
requested is a sub-optimal design choice from both performance and
security points of views, and robust cryptographic libraries have to
deal with it.

We can reach different compromises, relaxing the documented assumption
as this vote proposes, and then offering API functions for the most
generic use cases and specialized API functions for specific use cases
like yours. I'm not arguing against doing that moving forward, I am
arguing about categorizing the current strict behavior of EC keymgmt
in 3.0 as a "breaking change" and considering the current use pattern
of the reproducer in #12612 against 1.1.1 as "supported" in the
current LTS release.

>>
>> Omitting the `EVP_PKEY_check()` in the reproducer and the user
>> application, would for example allow me to write a DoS attack: the
>> secret scalar could easily be hand-picked to trigger an endless loop
>> in the sign operation.
>
>
> Nonsense.  Each iteration involves a new PRN, which by definition you cannot 
> predict.

~~~sh
; which openssl; openssl version
/usr/bin/openssl
OpenSSL 1.1.1f  31 Mar 2020
; cat > /tmp/p256_invalid.pem
-BEGIN PRIVATE KEY-
MEECAQAwEwYHKoZIzj0CAQYIKoZIzj0DAQcEJzAlAgEBBCD/AP//
vOb6racXnoTzucrC/GMlUQ==
-END PRIVATE KEY-
; openssl pkey -check -text -noout -in /tmp/p256_invalid.pem
Key is invalid
Detailed error: point at infinity
Private-Key: (256 bit)
priv:
ff:ff:ff:ff:00:00:00:00:ff:ff:ff:ff:ff:ff:ff:
ff:bc:e6:fa:ad:a7:17:9e:84:f3:b9

Re: OTC VOTE: EVP_PKEY private/public key components

[Nicola Tuveri]

Usually the labs provide assurances on the key generation and the
other cryptographic operations, including validation, but have special
provisions to avoid providing assurances on key serialization and
deserialization.

This seems particularly true in the context of the FIPS validation for
OpenSSL, where the encoders/decoders are outside the scope of the FIPS
module and of validation.

The issue that triggered this discussion is particularly an issue of
encoding/decoding: the user application uses a custom format, and that
is the reason why the `EVP_PKEY` object is built from an `EC_KEY`
object built from scratch.
The application has only these ways to validate if the created
`EVP_PKEY` object is valid from a programmer's perspective AND as a
cryptographic object:

- EVP_PKEY_check (check an EVP_PKEY keypair)
- EVP_PKEY_public_check (check an EVP_PKEY pubkey, or just the public
component of an EVP_PKEY keypair)
- EVP_PKEY_param_check (check an EVP_PKEY params object, or just the
params component of an EVP_PKEY keypair or pubkey)

By design the consistency checks and validation checks are omitted
unless these functions are called, and there is no
EVP_PKEY_private_check.

To me this proves that what was described as a "breaking change" in
3.0 with strict EC keymgmt, is instead a programmer error in the
application that abused the API with an incomplete initialization,
which would have been detected if the dedicated validation functions
were called appropriately.
Even if the security policies that apply to the users of that
application wouldn't require to validate the cryptographic properties
of the key every time they are loaded because of external processes
and assurances, the developers of the application would have
discovered they were abusing the API if they tried validating the
half-initialized objects even just in development builds, which in my
opinion debunks the "lack of enforcement" argument that partially
supported the discussion that led to this vote.



In light of this I would dare say that:

a) the current "strict" state of EC keymgmt in 3.0 does not represent
a "breaking change" w.r.t 1.1.1, as it only catches existing API
abuses that were catched in 1.1.1, if only the half-initialized
objects were validated
b) relaxing the "keypair assumption" for EVP_PKEY objects is entirely
a new feature


Neither of these points makes the vote "invalid", but they could
change some of the casted votes, and they would strengthen my -1 in a
-2 if it were an option.

--

Nicola


P.S. For the record, given that I expressed this mostly in the virtual
meetings rather than in the discussion thread in the mailing list,
while from these emails it might seem like I hate with fiery rage the
idea of relaxing the keypair assumption, this is not really the case.
My main concern is changing this deeply rooted and pervasive
assumption in the timeframe of the 3.0 release, and with (IMO)
insufficient test coverage to make sure that we can transition to a
more relaxed model without introducing defects that could translate
into security risks.

On Wed, Nov 11, 2020 at 11:58 PM SHANE LONTIS  wrote:
>
> Key assurance  really depends on what you are doing, so I don't think this is 
> quite true.
> For example if you generate a key using a valid algorithm (that is validated 
> by a lab), then there is already an assurance that the key is valid without 
> needing to run EVP_PKEY_check().
>
> There are many operations that don't need EVP_PKEY_check, but they may need a 
> sub check such as
> EVP_PKEY_public_check, EVP_PKEY_param_check, EVP_PKEY_private_check or 
> EVP_PKEY_pairwise_check (or some combination of those).
> Keys (and certs) that are received from another entity should be validated 
> unless there is some other mechanism for establishing that the key is trusted.
>
>
> On 12 Nov 2020, at 12:14 am, Nicola Tuveri  wrote:
>
> In light of recently working more closely to `EVP_PKEY_check()` I
> would add to the discussion on this vote that it is not entirely true
> that we were not enforcing the keypair assumption and that the current
> strict behavior of the EC keymgmt in 3.0 is a breaking change w.r.t.
> some uses that work in 1.1.1.
>
> In particular in 1.1.1, the key created as depicted in #12612 that
> triggered this discussion (Matt posted a useful reproducer among the
> first comments), is indeed capable of signing in the used pattern, but
> the pattern is conveniently omitting the validation pass that should
> be required in any serious use of the API.
>
> `EVP_PKEY_check()`
> (https://urldefense.com/v3/__https://www.openssl.org/docs/man1.1.1/man3/EVP_PKEY_check.html__;!!GqivPVa7Brio!PdWuiQz52CFt9zZk9s6He6IimqtSPbLrvJ1ck_RKJIydCE80FY4lRl-WD8v1m4r6Zg$
>  ) is
> one of the many places in 1.1.1 where both the documentation and the
> behavior assume that an

Re: OTC VOTE: EVP_PKEY private/public key components

[Nicola Tuveri]

In light of recently working more closely to `EVP_PKEY_check()` I
would add to the discussion on this vote that it is not entirely true
that we were not enforcing the keypair assumption and that the current
strict behavior of the EC keymgmt in 3.0 is a breaking change w.r.t.
some uses that work in 1.1.1.

In particular in 1.1.1, the key created as depicted in #12612 that
triggered this discussion (Matt posted a useful reproducer among the
first comments), is indeed capable of signing in the used pattern, but
the pattern is conveniently omitting the validation pass that should
be required in any serious use of the API.

`EVP_PKEY_check()`
(https://www.openssl.org/docs/man1.1.1/man3/EVP_PKEY_check.html) is
one of the many places in 1.1.1 where both the documentation and the
behavior assume that an `EVP_PKEY` object is a keypair.
Even in the version used by the user that posted the issue, running
`EVP_PKEY_check()` on the created key would have revealed that the
user was abusing the API.

The "lack of enforcement" argument, that was partially at the base of
formulating the vote text as it is, and conditioned our votes, seems
to me an intentional design choice, as part of preferring the usage
pattern "validate once, and reuse many times": for performance reasons
we are not running `EVP_PKEY_check()` on every single key loaded from
a PEM file or created by the user, but there is an assumption that the
user did validate at least once the key material using
`EVP_PKEY_check()` or `openssl pkey -check`.
So enforcement was never lacking, but it was relegated together with
more expensive checks, into functions that the user should execute at
least once, possibly according to well documented security policies
concerning the management of key material in transit and at rest.

Omitting the `EVP_PKEY_check()` in the reproducer and the user
application, would for example allow me to write a DoS attack: the
secret scalar could easily be hand-picked to trigger an endless loop
in the sign operation.


--

Nicola



On Tue, Nov 3, 2020 at 2:11 PM Matt Caswell  wrote:
>
> Background to the vote:
>
> The OTC meeting today discussed the problems raised by issue #12612. In
> summary the problem is that there has been a long standing, widespread
> and documented assumption that an EVP_PKEY with a private key will
> always also have the public key component.
>
> In spite of this it turns out that in the EC implementation in 1.1.1 it
> was perfectly possible to create an EVP_PKEY with only a private key and
> no public key - and it was also possible to use such an EVP_PKEY in a
> signing operation.
>
> The current 3.0 code in master introduced an explicit check (in line
> with the long held assumption) that the public key was present and
> rejected keys where this was not the case. This caused a backwards
> compatibility break for some users (as discussed at length in #12612).
>
> The OTC discussed a proposal that we should relax our conceptual model
> in this regards and conceptually allow EVP_PKEYs to exist that only have
> the private component without the public component - although individual
> algorithm implementations may still require both.
>
> It is possible to automatically generate the public component from the
> private for many algorithms, although this may come at a performance and
> (potentially) a security cost.
>
> The proposal discussed was that while relaxing the conceptual model,
> most of the existing implementations would still require both. The EC
> implementation would be relaxed however. This essentially gives largely
> compatible behaviour between 1.1.1 and 3.0.
>
> The vote text is as follows:
>
> topic: For 3.0 EVP_PKEY keys, the OTC accepts the following resolution:
> * relax the conceptual model to allow private keys to exist without public
>   components;
> * all implementations apart from EC require the public component to be
> present;
> * relax implementation for EC key management to allow private keys that
> do not
>   contain public keys and
> * our decoders unconditionally generate the public key (where possible).
>
> Proposed by Matt Caswell
> Public: yes
> opened: 2020-11-03
> closed: 2020-mm-dd
> accepted:  yes/no  (for: X, against: Y, abstained: Z, not voted: T)
>
>   Matt   [+1]
>   Mark   [  ]
>   Pauli  [+1]
>   Viktor [  ]
>   Tim[  ]
>   Richard[+1]
>   Shane  [+1]
>   Tomas  [+1]
>   Kurt   [  ]
>   Matthias   [  ]
>   Nicola [-1]

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

[Nicola Tuveri]

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  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  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 consis

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

[Nicola Tuveri]

That is a very good point I completely missed!

Would you be available do "adopt" my vote and raise it as an OMC vote?


Nicola

On Wed, Nov 11, 2020, 11:10 Matt Caswell  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://github.com/openssl/openssl/issues/8435>
> > [PR 13359]: <https://github.com/openssl/openssl/pull/13359>
> > [apps/pkey.c changes]:
> > <
> https://github.com/openssl/openssl/pull/13359/files#diff-810c047ab642e686cab85b16802e9190bbc9f2f9bfce8a55fb8d6b744caa9091
> >
> >
>

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

[Nicola Tuveri]

## 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]: 
[PR 13359]: 
[apps/pkey.c changes]:

Re: VOTE: Accept the Fully Pluggable TLSv1.3 KEM functionality

[Nicola Tuveri]

As defined by the [OTC Voting Procedures][0], I am declaring the
vote closed, as the number of uncast votes cannot affect the outcome of
the vote.

The vote is accepted.

topic: We should accept the Fully Pluggable TLSv1.3 KEM functionality as shown
in PR #13018 into the 3.0 release
Proposed by Matt Caswell
Public: yes
opened: 2020-10-08
closed: 2020-10-13
accepted:  yes  (for: 8, against: 1, abstained: 1, not voted: 1)

  Matt   [+1]
  Mark   [ 0]
  Pauli  [+1]
  Viktor [  ]
  Tim[+1]
  Richard[+1]
  Shane  [-1]
  Tomas  [+1]
  Kurt   [+1]
  Matthias   [+1]
  Nicola [+1]


-- Nicola

[0]: https://www.openssl.org/policies/omc-bylaws.html

Re: VOTE: Weekly OTC meetings until 3.0 beta1 is released

[Nicola Tuveri]

As defined by the [OTC Voting Procedures][0], I am declaring the
vote closed, as the number of uncast votes cannot affect the outcome of
the vote.

The vote is accepted.

topic: Hold online weekly OTC meetings starting on Tuesday 2020-10-13
   and until 3.0 beta1 is released, in lieu of the weekly "developer
   meetings".
Proposed by Nicola Tuveri
Public: yes
opened: 2020-10-09
closed: 2020-10-11
accepted:  yes  (for: 9, against: 0, abstained: 0, not voted: 2)

  Matt   [+1]
  Mark   [+1]
  Pauli  [+1]
  Viktor [  ]
  Tim[+1]
  Richard[  ]
  Shane  [+1]
  Tomas  [+1]
  Kurt   [+1]
  Matthias   [+1]
  Nicola     [+1]


Nicola Tuveri

[0]: https://www.openssl.org/policies/omc-bylaws.html

On Fri, 9 Oct 2020 at 15:00, Nicola Tuveri  wrote:
>
> topic: Hold online weekly OTC meetings starting on Tuesday 2020-10-13
>and until 3.0 beta1 is released, in lieu of the weekly "developer
>    meetings".
> Proposed by Nicola Tuveri
> Public: yes
> opened: 2020-10-09
> closed: 2020-mm-dd
> accepted:  yes/no  (for: X, against: Y, abstained: Z, not voted: T)
>
>   Matt   [  ]
>   Mark   [  ]
>   Pauli  [  ]
>   Viktor [  ]
>   Tim[  ]
>   Richard[  ]
>   Shane  [  ]
>   Tomas  [  ]
>   Kurt   [  ]
>   Matthias   [  ]
>   Nicola [+1]

Re: OTC VOTE: The PR #11359 (Allow to continue with further checks on UNABLE_TO_VERIFY_LEAF_SIGNATURE) is acceptable for 1.1.1 branch

[Nicola Tuveri]

+1

I am basing my vote on the feedback provided by @DDvO [0] and @t8m [1].
In particular I am convinced to vote in favor, as I can see this as a
bug fix, fixing an undocumented inconsistency, and that it is very
unlikely it would affect existing applications.


Nicola


[0]: https://github.com/openssl/openssl/pull/11359#issuecomment-706189632
[1]: https://github.com/openssl/openssl/pull/11359#issuecomment-706191205


On Fri, 9 Oct 2020 at 15:02, Tomas Mraz  wrote:
>
> topic: The PR #11359 (Allow to continue with further checks on
>  UNABLE_TO_VERIFY_LEAF_SIGNATURE) is acceptable for 1.1.1 branch
> As the change is borderline on bug fix/behaviour change OTC needs
> to decide whether it is acceptable for 1.1.1 branch.
> Proposed by Tomas Mraz
> Public: yes
> opened: 2020-10-09
> closed: 2020-mm-dd
> accepted:  yes/no  (for: X, against: Y, abstained: Z, not voted: T)
>
>   Matt   [  ]
>   Mark   [  ]
>   Pauli  [  ]
>   Viktor [  ]
>   Tim[  ]
>   Richard[  ]
>   Shane  [  ]
>   Tomas  [+1]
>   Kurt   [  ]
>   Matthias   [  ]
>   Nicola [  ]
>
> --
> Tomáš Mráz
> No matter how far down the wrong road you've gone, turn back.
>   Turkish proverb
> [You'll know whether the road is wrong if you carefully listen to your
> conscience.]
>
>

VOTE: Weekly OTC meetings until 3.0 beta1 is released

[Nicola Tuveri]

topic: Hold online weekly OTC meetings starting on Tuesday 2020-10-13
   and until 3.0 beta1 is released, in lieu of the weekly "developer
   meetings".
Proposed by Nicola Tuveri
Public: yes
opened: 2020-10-09
closed: 2020-mm-dd
accepted:  yes/no  (for: X, against: Y, abstained: Z, not voted: T)

  Matt   [  ]
  Mark   [  ]
  Pauli  [  ]
  Viktor [  ]
  Tim[  ]
  Richard[  ]
  Shane  [  ]
  Tomas  [  ]
  Kurt   [  ]
  Matthias   [  ]
  Nicola [+1]

Re: VOTE: Technical Items still to be done

[Nicola Tuveri]

+1

On Thu, 8 Oct 2020 at 17:47, Matt Caswell  wrote:
>
> topic: The following items are required prerequisites for the first beta
> release:
>  1) EVP is the recommended API, it must be feature-complete compared with
> the functionality available using lower-level APIs.
>- Anything that isn’t available must be put to an OTC vote to exclude.
>- The apps are the minimum bar for this, subject to exceptions noted
> below.
>  2) Deprecation List Proposal: DH_, DSA_, ECDH_, ECDSA_, EC_KEY_, RSA_,
> RAND_METHOD_.
>- Does not include macros defining useful constants (e.g.
>  SHA512_DIGEST_LENGTH).
>- Excluded from Deprecation: `EC_`, `DSA_SIG_`, `ECDSA_SIG_`.
>- There might be some others.
>- Review for exceptions.
>- The apps are the minimum bar to measure feature completeness for
> the EVP
>  interface: rewrite them so they do not use internal nor deprecated
>  functions (except speed, engine, list, passwd -crypt and the code
> to handle
>  the -engine CLI option).  That is, remove the suppression of deprecated
>  define.
>  - Proposal: drop passwd -crypt (OMC vote required)
>- Compile and link 1.1.1 command line app against the master headers and
>  library.  Run 1.1.1 app test cases against the chimera.  Treat this
> as an
>  external test using a special 1.1.1 branch. Deprecated functions
> used by
>  libssl should be moved to independent file(s), to limit the
> suppression of
>  deprecated defines to the absolute minimum scope.
>  3) Draft documentation (contents but not pretty)
>- Need a list of things we know are not present - including things we
> have
>  removed.
>- We need to have mapping tables for various d2i/i2d functions.
>- We need to have a mapping table from “old names” for things into the
>  OSSL_PARAMS names.
>  - Documentation addition to old APIs to refer to new ones (man7).
>  - Documentation needs to reference name mapping.
>  - All the legacy interfaces need to have their documentation
> pointing to
>the replacement interfaces.
>  4) Review (and maybe clean up) legacy bridge code.
>  5) Review TODO(3.0) items #12224.
>  6) Source checksum script.
>  7) Review of functions previously named _with_libctx.
>  8) Encoder fixes (PKCS#8, PKCS#1, etc).
>  9) Encoder DER to PEM refactor.
> 10) Builds and passes tests on all primary, secondary and FIPS platforms.
> 11) Query provider parameters (name, version, ...) from the command line.
> 12) Setup buildbot infrastructure and associated instructions.
> 13) Complete make fipsinstall.
> 14) More specific decoding selection (e.g. params or keys).
> 15) Example code covering replacements for deprecated APIs.
> 16) Drop C code output options from the apps (OMC approval required).
> 17) Address issues and PRs in the 3.0beta1 milestone.
> Proposed by .
> Public: yes
> opened: 2020-10-08
> closed: 2020-mm-dd
> accepted:  yes/no  (for: X, against: Y, abstained: Z, not voted: T)
>
>   Matt   [+1]
>   Mark   [  ]
>   Pauli  [  ]
>   Viktor [  ]
>   Tim[  ]
>   Richard[  ]
>   Shane  [  ]
>   Tomas  [  ]
>   Kurt   [  ]
>   Matthias   [  ]
>   Nicola [  ]

Re: Vote proposal: Private keys can exist independently of public keys

[Nicola Tuveri]

On Wed, 7 Oct 2020 at 20:45, Richard Levitte  wrote:
>
> I'm as culpable as anyone re pushing the "convention" that an EVP_PKEY
> with a private key should have a public key.  I was incorrect.

Sure, my example was not about pointing fingers!
It's just to give recent data points on how the documentation written
in 2006 is probably not something as stale as one can think, because
until recently we were leveraging that assumption at least in some
sections of the lower layers.

> Regarding avoiding NULL checks, that's actually a separate story,
> evem though it overlaps conveniently.  There was the idea, for a
> while, that we should avoid NULL checks everywhere (unless it's valid
> input), and indeed make it a programmer error if they happened to pass
> NULL where they shouldn't.
> One can see that as an hard assertion, and that has indeed produced
> crashes that uncovered bugs we might otherwise have missed.  The tide
> has changed though, and it seem like the fashion du jour is to check
> NULLs and error with an ERR_R_PASSED_NULL_PARAMETER.  I'm not sure
> that we've made an actual hard decision in either direction, though.
>
> However, I'm not sure where the NULL check problem comes in.
> Operations that don't use the public key parts don't need to look at
> the public key parts, so a NULL check there is simply not necessary.

I think the lack of NULL checks is intertwined with this problem as
long as in some code that does access the public key component we
ensured we dereference without making the NULL check because of the
assumption.

The problem I see with spot checking rather than writing a
comprehensive suite of tests, is that given our wide API surface and
the compromises taken in the transition from non-opaque stack
allocated objects in 1.0.2 to opaque objects in 1.1.1+, we might have
non-obvious places where dereferencing the public key without checking
for NULL can bite us.


I would add, although it's feedback for the vote, not the proposal,
that I am not opposed to changing the documented assumption in
principle, but with changing it this late in the development of 3.0.

I am giving feedback for the text proposal to ensure that during the
voting we don't underestimate the high risk of critical bugs coming
with adopting this change at this stage, and that it does call for
vastly extending our current test coverage beyond what we were
currently planning as acceptable.

We all feel quite under pressure due to the time constraints, and I
feel that in general we have privileged working on implementing the
required features (with sufficient level of tests as per our policy)
rather than spending major resources in further developing our test
coverage with exhaustive positive and negative tests for regular and
far-fetched cases, this should probably also have a weight on the
decision of committing to this change at this point.



Nicola

Re: Vote proposal: Private keys can exist independently of public keys

[Nicola Tuveri]

On Wed, 7 Oct 2020 at 20:17, Richard Levitte  wrote:
>
> There's no reason for the EVP layer to check for the presence or the
> absence or the public key, for one very simple reason: it doesn't have
> access to the backend key structure and therefore has no possibility
> to make any such checks.  Such checks should be made in the backend.

The very case that triggered this discussion happens because the
provider backend is doing the tests that libcrypto is not doing, and
upholding the core to the documented assumption.
On import/export the backend is enforcing that privkey alone is never
imported/exported without its public counterpart.


> That part of your proposed vote text that mentions checks in EVP
> doesn't make any sense with that in mind, and I fear that it would
> lead us into a rabbit hole that's not really necessary.  Also, this
> contradicts the intentions of the design for libcrypto vs providers,
> which was that libcrypto (and therefore EVP) would be a rather thin
> layer that simply passes stuff to the providers and gets results back
> (there are things that complicate this a bit, but this is essentially
> what we do), and leave it to the provider to decide what to do and how
> to do it.

Re: contradicting design principles

I would also remark here that pushing quirkiness into providers to
satisfy "weird" requirements that only serve the purpose of dealing
with the legacy promises of libcrypto (or anyway the extra constraints
enforced by libcrypto surface that is not directly facing the
providers) also contradicts designs of libcrypto vs providers:
libcrypto takes care of libcrypto user-facing and provider-facing API,
the provider only should care about the new and well defined
core-provider API.

Re: why test at EVP layer?

In my proposal the test at the EVP (and lower) layers is because that
is part of taking care of libcrypto user-facing API: the long-standing
(locally betrayed) assumption is established for `EVP_PKEY` objects,
so `EVP` is the natural layer to have a first layer of testing to
ensure that pervasively we can guarantee that the current documented
assumption can be safely disregarded because not relevant anymore.
That assumption, for which we have long-standing documentation at the
EVP layer has at some point (even in the past 4 years in my limited
experience) been enforced also on lower levels (it is not particularly
relevant if chronologically it percolated down to the lower levels or
conversely was already there all along undocumented and was put in
writing only when documenting `EVP_PKEY`): that is why I included in
my proposal to also build coverage for this change not only in EVP but
also in the other layers.



I can agree with you that some of this testing at different layers can
appear quite redundant, especially in terms of which "security
critical" or "potentially failing" code is tested in the current
snapshot of the code: but my counter argument is that if we were less
worried about redundant tests and were more rigorous in always
including both positive and negative tests for all the things we put
in our documentation, maybe we would have already had the tests that
asserted that the code reflects our documented assumption (and in
which layers) and we wouldn't have ended up violating our documented
assumption by accident creating something that works in 1.1.1 by
(partial) accident and that we are now considering as a potential
regression in 3.0.


Nicola

Re: Vote proposal: Private keys can exist independently of public keys

[Nicola Tuveri]

Re; "enforcement"

My impression is that there is no such enforcement, because the
project has (had?) a stance on "avoid NULL checks" and "consider
things that break our documented assumptions as programmer errors".

The natural result of these two principles may very well be the
deliberate reason why "enforcement" cannot be found and we might
perceive documentation written in 2006 as not current.

I can say that in the past 4 years within PR I authored or
co-authored, even before becoming a Committer, I can recall reviews
requesting to amend the code to avoid NULL checks on the public key
component because it is our convention that an EVP_PKEY with key
material is either a public key or a key pair.


Nicola

On Wed, 7 Oct 2020 at 19:52, Richard Levitte  wrote:
>
> As far as I can tell, the existing "enforcement" is in the algorithm
> implementations.  I had a look through the following files in OpenSSL
> 1.1.1:
>
> crypto/rsa/rsa_ossl.c
> crypto/dsa/dsa_ossl.c
> crypto/dh/dh_key.c
> crypto/ec/ecdsa_ossl.c
> crypto/ec/ecdh_ossl.c
>
> I first had a look at the key computing functions in dh_key.c and
> ecdh_ossl.c, because I was told that the public key is looked at
> there.  This turns out to be somewhat false; they do look at a public
> key, the public key of the peer key that they get passed, which isn't
> quite the same.  However, when it comes to the DH or EC_KEY they work
> with, only the private half is looked at,
>
> Looking at dsa_ossl.c and ecdsa_ossl.c, I can see that the signing
> functions do NOT look a |pub_key|, and the verification functions do
> NOT look at |priv_key|.  This seems perfectly normal to me.
>
> Similarly, the signing functions in rsa_ossl.c do NOT seem to look at
> |d|, and the verification functions do NOT seem to look at |e|.
>
> I took a moment to search through the corresponding *meth.c files to
> see if I could quickly grep for some kind of check, and found none.
> Mind you, that was a *quick* grep, someone more thorough might want to
> confirm or contradict.
>
> So, having looked through that code, my sense is that the "enforcement"
> that's talked about is non-existent, or at least very unclear, and I
> suspect that if there is some sort of "enforcement" at that level,
> it's more accidental than by design...
>
> I'm not sure what to make of the documentation from 2006.  Considering
> the level of involvement there was from diverse people (2006 wasn't
> exactly the most eventful time), there's a possibility that it was a
> precaution ("don't touch that can of worms!") than a firm decision.
>
> Cheers,
> Richard
>
> On Wed, 07 Oct 2020 17:34:25 +0200,
> 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), u

Re: 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  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

Re: Vote proposal: Technical items still to be done

[Nicola Tuveri]

I support the edit proposed by Tomas.

First comment that I have is that I'd prefer the first-level items to
be actually numbered, as done in the drafts: we might put a disclaimer
that the numbers are not indicative of priority and just meant to be
used to address (equally important) tasks by index.

The second comment is just a quirk of mine: I prefer plain text emails
and Markdown formatting. If others shared the feeling (and nobody
objected to Markdown for our records) I'd volunteer to reformat the
draft of this vote text using Markdown syntax.

Nicola

On Wed, 7 Oct 2020 at 14:58, Tomas Mraz  wrote:
>
> On Wed, 2020-10-07 at 12:35 +0100, Matt Caswell wrote:
> > I had an action from the OTC meeting today to raise a vote on the OTC
> > list of technical items still to be done. Here is my proposed vote
> > text.
> > There will be a subsequent vote on the "beta readiness checklist"
> > which
> > is a separate list.
> >
> > Feedback please on the proposed vote text below.
> >
> > The following items are required prerequisites for the first beta
> > release:
> > * EVP is the recommended API, it must be feature-complete compared
> > with
> > the functionality available using lower-level APIs.
> >   - Anything that isn’t available must be put to an OTC vote to
> > exclude.
> >   - The apps are the minimum bar for this, subject to exceptions
> > noted
> > below.
> > * Deprecation List Proposal: DH_, DSA_, ECDH_, ECDSA_, EC_KEY_, RSA_,
> > RAND_METHOD_.
> >   - Does not include macros defining useful constants (e.g.
> > SHA512_DIGEST_LENGTH).
> >   - Excluded from Deprecation: `EC_`, `DSA_SIG_`, `ECDSA_SIG_`.
> >   - There might be some others.
> >   - Review for exceptions.
> >   - The apps are the minimum bar to measure feature completeness for
> > the
> > EVP interface: rewrite them so they do not use internal nor
> > deprecated
> > functions (except speed, engine, list, passwd -crypt and the code to
> > handle the -engine CLI option).  That is, remove the suppression of
> > deprecated define.
> > - Proposal: drop passwd -crypt (OMC vote required)
> >   - Compile and link 1.1.1 command line app against the master
> > headers
> > and library.  Run 1.1.1 app test cases against the chimera.  Treat
> > this
> > as an external test using a special 1.1.1 branch.
> > Deprecated functions used by libssl should be moved to independent
> > file(s), to limit the suppression of deprecated defines to the
> > absolute
> > minimum scope.
> > * Draft documentation (contents but not pretty)
> >   - Need a list of things we know are not present - including things
> > we
> > have removed.
> >   - We need to have mapping tables for various d2i/i2d functions.
> >   - We need to have a mapping table from “old names” for things into
> > the
> > OSSL_PARAMS names.
> > - Documentation addition to old APIs to refer to new ones (man7).
> > - Documentation needs to reference name mapping.
> > - All the legacy interfaces need to have their documentation
> > pointing to the replacement interfaces.
> > * Review (and maybe clean up) legacy bridge code.
> > * Review TODO(3.0) items #12224.
> > * Source checksum script.
> > * Review of functions previously named _with_libctx.
> > * Encoder fixers (PKCS#8, PKCS#1, etc).
> > * Encoder DER to PEM refactor.
> > * Builds and passes tests on all primary, secondary and FIPS
> > platforms.
> > * Query provider parameters (name, version, …) from the command line.
> > * Setup buildbot infrastructure and associated instructions.
> > * Complete make fipsinstall.
> > * More specific decoding selection (e.g. params or keys).
> > * Example code covering replacements for deprecated APIs.
> > * Drop C code output options from the apps (OMC approval required).
> > * Address 3.0beta1 milestones.
>
> Address issues and PRs in the 3.0beta1 milestone.
>
> --
> Tomáš Mráz
> No matter how far down the wrong road you've gone, turn back.
>   Turkish proverb
> [You'll know whether the road is wrong if you carefully listen to your
> conscience.]
>
>

[VOTE PROPOSAL] Weekly OTC meeting until 3.0 beta1 is released

[Nicola Tuveri]

Background
--

Since the two virtual face-to-face OTC meetings of last week, and again
in the two OTC meetings this week, we repeatedly discussed replacing the
weekly "developer meetings" with official OTC meetings.

The "developer meetings" have so far seen frequent participation from a
subset of OTC members heavily involved in day to day tasks for the
[`3.0 New Core + FIPS` project][new_core+FIPS], and have been open and
advertised to all OTC members since the end of January, but they were
not qualified as official OTC meetings: they were meant to assess the
weekly progress within the project, and determine the coming week's
priorities, discussing and resolving any difficulties encountered.

Rationale
-

At this point in the development process, in which we are refining the
state of development branch to finally qualify for the transition into
the beta release stage, upgrading these meetings to proper OTC meetings
seems appropriate and desirable, as discussion and assessment of the
remaining technical tasks increasingly requires discussion within the
OTC and OTC decisions, and additionally provides OTC steering on the
preferred way of completing some of the allocated tasks before the work
is done, rather than after the fact.

Personally, I also expect that upgrading the developer meetings into
full-fledged OTC meetings will increase attendance by the wider OTC
audience: this would provide the OTC as a whole with better oversight on
the progress towards the upcoming release, and would further improve the
quality of the release process by leveraging the plurality of OTC
perspectives and insights.

Meeting Schedule


So far, the "developer meetings" have been regularly scheduled each
Tuesday from 08:00 to 09:30 (UTC, 24h format), with a tendency to spill
into the next half-hour.
There have been cases in which we had to hit the 3h mark, but I wouldn't
describe 3h as the norm.
My proposal for the OTC meeting is to adopt the same 1h30m (+30m buffer)
approach.
Tuesday, 08:00 UTC as the starting time, has been so far a good
compromise between folks working in European and Australian timezone,
and keeping it is the current proposal, but is admittedly not the best
time for attendance from the American continents.

Vote: why and when?
---

We are proposing a vote, rather than agreeing on it during the last
call, to invite discussion from the OTC members that could not
participate in the meetings during these two weeks, for example
regarding alternative time slots to improve overall attendance.

I plan to wait until Friday to open the actual vote: please provide
feedback before then if you would like to amend the letter of the vote
topic.

One item on which I'd like to ask for feedback is if the actual vote
text should include the specifics of the timeslot or not.

Proposed vote text
--

Hold online weekly OTC meetings starting on Tuesday 2020-10-13
and until 3.0 beta1 is released, in lieu of the weekly
"developer meetings".

Refs


[new_core+FIPS]: https://github.com/openssl/openssl/projects/2

Re: [OTC] Agenda proposal for OTC meeting on 2020-10-06

[Nicola Tuveri]

 all subsequent versions.
03. Triage all open issues and decide:
not an issue, won’t fix, fix for beta or fix after beta for each.
04. Triage all TODO(current release) items:
done, won’t fix, fix for beta or fix after beta for each.
05. Triage all Coverity issues and close either:
by marking it as false positive or by fixing it.
06. Coveralls coverage has not decreased overall from the previous
release.
07. Have a pass through all new/changed APIs to ensure consistency,
fixing those that aren't.
08. Confirm that all new public APIs are documented.
09. Check that new exported symbols (in the static build) are named
correctly (`OSSL_` resp. `ossl_` prefix).
10. Run the previous (all relevant supported) release test suites
against the beta version.
11. Clean builds in Travis and Appveyor for two days.
12. run-checker.sh succeeds on 2 consecutive days before release.
13. A passing OTC "ready for beta" vote.


Proposed "Technical items still to be done" list


1. The `EVP` interface is the recommended API, it must be
   feature-complete compared with the functionality available using
   lower-level APIs.
   - Anything that isn't available must be put to an OTC vote to
 exclude.
   - The `apps/` are the minimum bar for this, they should not use
 internal nor deprecated functions (except `speed`, `engine` and
 the code to handle the `-engine` CLI option).
   - Deprecated functions used by `libssl` should be moved to an
 independent file.
2. Deprecation List Proposal: The entire `DH_`, `DSA_`, `ECDH_`,
   `ECDSA_`, `EC_KEY_`, `RSA_`, and `RAND_METHOD_` interfaces.
   - Does not include macros defining useful values (e.g. DH maximum
 size).
   - Excluded from Deprecation: `EC_`, `DSA_SIG_`, `ECDSA_SIG_`.
   - There might be some others.
   - Review for exceptions.
   - Rewrite apps to use non-deprecated interfaces as far as possible.
   - Transfer old apps to test cases (either directly or via 1.1.1).
3. Compile a list of things we know are not present - including things
   we have removed.
4. We need to have a mapping table from “old names” for things into the
   `OSSL_PARAMS` names.
5. We need to have mapping tables for various `d2i`/`i2d` functions.
6. We need to rewrite the apps to use only the non-deprecated interfaces
   (except for the list, speed and engine apps and the engine parameter
   in various places).
7. All the legacy interfaces need to have their documentation pointing
   to the replacement interfaces.


---

Best regards,

Nicola Tuveri

[Release Strategy]: https://www.openssl.org/policies/releasestrat.html
[OpenSSL Bylaws]: https://www.openssl.org/policies/omc-bylaws.html
[3.0 design document]: https://www.openssl.org/docs/OpenSSL300Design.html

On Fri, 2 Oct 2020 at 23:55, Nicola Tuveri  wrote:
>
> I am writing this on behalf of the OTC, as an update on the latest OTC
> developments.
> This week we had three days of virtual face to face meetings (two OTC
> sessions with one Committers session in the middle) and we scheduled the
> next OTC meeting already next week, on Tuesday 2020-10-06.
>
> This email presents some of the points already under discussion since
> last week, and an agenda proposal for the next OTC meeting.
> This is done in the interest of transparency and so that the community
> can give us feedback during the discussion.
>
>
> Background
> --
>
> Understandably, it doesn't come as a surprise that this week we mostly
> discussed about the upcoming beta release and associated items.
> According to our [Release Schedule][], among our pre-releases, the
> transition from _alpha_ releases to _beta_ releases is defined by the
> following definitions:
>
> - an _alpha_ release means:
>   - Not (necessarily) feature complete
>   - Not necessarily all new APIs in place yet
> - a _beta_ release means:
>   - Feature complete/Feature freeze
>   - Bug fixes only
>
> From the discussion, it transpired that the OTC needs to better
> formalize, in technical terms, how to assess its level of confidence on
> beta readiness, and to collectively agree on which technical tasks still
> need to be completed before the transition to beta to ensure feature
> completeness, and which remaining tasks would count as bug fixes that
> can be planned for the beta stage.
>
> At the next OTC meeting, we plan to discuss mainly two items in our
> agenda:
>
> - discuss, refine and vote on the "Beta Release Readiness Checklist"
> - discuss, refine and vote on the "Technical items still to be done"
>   list
>
> The "Beta Release Readiness Checklist" aims at establishing the
> technical checks to be performed to assess the OTC degree of confidence
> on the state of the development branch before calling the OTC "Ready for
> beta&q

[OTC] Agenda proposal for OTC meeting on 2020-10-06

[Nicola Tuveri]

’t fix, fix for beta or fix after beta for each.
04. Triage all TODO(current release) items:
done, won’t fix, fix for beta or fix after beta for each.
05. Triage all Coverity issues and close either:
by marking it as false positive or by fixing it.
06. Coveralls coverage has not decreased overall from the previous
release.
07. Have a pass through all new/changed APIs to ensure consistency,
fixing those that aren't.
08. Confirm that all new public APIs are documented.
09. Check that new exported symbols (in the static build) are named
correctly (`OSSL_` resp. `ossl_` prefix).
10. Run the previous (all relevant supported) release test suites
against the beta version.
11. Clean builds in Travis and Appveyor for two days.
12. run-checker.sh succeeds on 2 consecutive days before release.
13. A passing OTC "ready for beta" vote.


Proposed "Technical items still to be done" list


1. The `EVP` interface is the recommended API, it must be
   feature-complete compared with the functionality available using
   lower-level APIs.
   - Anything that isn't available must be put to an OTC vote to
 exclude.
   - The `apps/` are the minimum bar for this, they should not use
 internal nor deprecated functions (except `speed`, `engine` and
 the code to handle the `-engine` CLI option).
   - Deprecated functions used by `libssl` should be moved to an
 independent file.
2. Deprecation List Proposal: The entire `DH_`, `DSA_`, `ECDH_`,
   `ECDSA_`, `EC_KEY_`, `RSA_`, and `RAND_METHOD_` interfaces.
   - Does not include macros defining useful values (e.g. DH maximum
 size).
   - Excluded from Deprecation: `EC_`, `DSA_SIG_`, `ECDSA_SIG_`.
   - There might be some others.
   - Review for exceptions.
   - Rewrite apps to use non-deprecated interfaces as far as possible.
   - Transfer old apps to test cases (either directly or via 1.1.1).
3. Compile a list of things we know are not present - including things
   we have removed.
4. We need to have a mapping table from “old names” for things into the
5. We need to have mapping tables for various `d2i`/`i2d` functions.
   `OSSL_PARAMS` names.
6. We need to rewrite the apps to use only the non-deprecated interfaces
   (except for the list, speed and engine apps and the engine parameter
   in various places).
7. All the legacy interfaces need to have their documentation pointing
   to the replacement interfaces.


---

Best regards,

Nicola Tuveri

[Release Strategy]: https://www.openssl.org/policies/releasestrat.html
[OpenSSL Bylaws]: https://www.openssl.org/policies/omc-bylaws.html
[3.0 design document]: https://www.openssl.org/docs/OpenSSL300Design.html

Re: VOTE: Accept the OTC voting policy as defined:

[Nicola Tuveri]

+1, as expressed during the f2f meeting.

Nicola

On Mon, Sep 28, 2020, 15:02 Dr. Matthias St. Pierre <
matthias.st.pie...@ncp-e.com> wrote:

> topic: Accept the OTC voting policy as defined:
>
>The proposer of a vote is ultimately responsible for updating the
> votes.txt
>file in the repository.  Outside of a face to face meeting, voters
> MUST reply
>to the vote email indicating their preference and optionally their
> reasoning.
>Voters MAY update the votes.txt file in addition.
>
>The proposed vote text SHOULD be raised for discussion before
> calling the vote.
>
>Public votes MUST be called on the project list, not the OTC list
> and the
>subject MUST begin with "VOTE:".  Private votes MUST be called on
> the
>OTC list with "PRIVATE VOTE:" beginning subject.
>
> Proposed by Matthias St. Pierre (on behalf of the OTC)
> Public: yes
> opened: 2020-09-28
> closed: 2020-mm-dd
> accepted:  yes/no  (for: X, against: Y, abstained: Z, not voted: T)
>
>   Matt   [  ]
>   Mark   [  ]
>   Pauli  [  ]
>   Viktor [  ]
>   Tim[  ]
>   Richard[  ]
>   Shane  [  ]
>   Tomas  [  ]
>   Kurt   [  ]
>   Matthias   [+1]
>   Nicola [  ]
>
>

Re: New GitHub label for release blockers

[Nicola Tuveri]

I was not referring to this page in particular, but in general to the
various pages on github doc (and more in general among the first Google
results) about github labels and milestones and how to use them.

There seem to be, in the wild, different documented approaches, probably
even influenced by the introduction over time of milestones and ways of
using labels and milestones.

I think having a common understanding of how we want to use these tools is
a good idea, so I definitely welcome the proposition of talking about it
during the vf2f!

Nicola

On Sun, Sep 13, 2020, 17:26 Dr. Matthias St. Pierre <
matthias.st.pie...@ncp-e.com> wrote:

> Since we were able to collect some experiences working on the ‘3.0 New
> Core + FIPS’ project, I think that’s a perfect subject
> to be discussed on the face-to-face meeting. I will add it to the list of
> proposed topics. As for the official documentation you
> mentioned, are you talking about this one?
> https://github.com/features/project-management
>
> Matthias
>
>
> From: Nicola Tuveri 
> Sent: Sunday, September 13, 2020 4:17 PM
> To: Dr. Matthias St. Pierre 
> Cc: openssl-project@openssl.org
> Subject: Re: New GitHub label for release blockers
>
> Matthias overcredits me: I just wanted to know his opinion about when we
> should use labels and when milestones (and that is why I wrote to him
> off-list, as a very confused and shy pupil asking a sensei for wisdom
> pearls).
>
> All the alleged convincing was self-inflicted :P
>
>
> And now that my ignorance is out of the closet...
> ... I still have very confused ideas regarding the "best" conventional
> usage of github features like labels, milestones and projects: I read the
> official documentation about them and I grasp the general ideas behind
> them, but too often the boundaries are too foggy for me to navigate and
> pick the right tool for the job in a consistent and organic manner.
>
> Nicola
>

Re: New GitHub label for release blockers

[Nicola Tuveri]

Matthias overcredits me: I just wanted to know his opinion about when we
should use labels and when milestones (and that is why I wrote to him
off-list, as a very confused and shy pupil asking a sensei for wisdom
pearls).

All the alleged convincing was self-inflicted :P


And now that my ignorance is out of the closet...
... I still have very confused ideas regarding the "best" conventional
usage of github features like labels, milestones and projects: I read the
official documentation about them and I grasp the general ideas behind
them, but too often the boundaries are too foggy for me to navigate and
pick the right tool for the job in a consistent and organic manner.

Nicola

On Sun, Sep 13, 2020, 17:01 Dr. Matthias St. Pierre <
matthias.st.pie...@ncp-e.com> wrote:

> Nicola suggested and convinced me, that it would be better to have a
> dedicated
> milestone for the 3.0.0 beta1 release instead of adding a new label.
>
> So here it is, I already added all the tickets with the release blocker
> label and will
> remove the label again.
>
> https://github.com/openssl/openssl/milestone/17
>
> Matthias
>
>
>
> > -Original Message-
> > From: Dr. Matthias St. Pierre
> > Sent: Sunday, September 13, 2020 3:17 PM
> > To: openssl-project@openssl.org
> > Subject: New GitHub label for release blockers
> >
> > Hi all,
> >
> > taking up again the discussion from openssl-project where I suggested to
> (ab)use
> > the 3.0.0 milestone for release blockers, (see link and citation at the
> end of the mail),
> > I propose to add a new label for this purpose instead. In fact, I
> already created the label
> >
> >   [urgent: release blocker]   (see link below)
> >
> > and will add the mentioned tickets within shortly. So you can take a
> look and tell
> > me whether you like it or not. (If not, no problem. I'll just delete the
> label again.)
> >
> > Matthias
> >
> >
> > BTW: It took me all my force of will to resist the temptation of making
> a pun
> >  by naming the label [urgent: beta blocker].
> >
> >
> > References:
> > ==
> >
> > [urgent: release blocker]:
> >
> https://github.com/openssl/openssl/labels/urgent%3A%20release%20blocker
> >
> > [openssl-project message]:
> >
> https://mta.openssl.org/pipermail/openssl-project/2020-September/002191.html
> >
> >
> > > > > For a more accurate and timely public overview over the current
> state of the blockers,
> > > > > it might be helpful to manage them via the 3.0.0  milestone
> > > > >
> > > > > https://github.com/openssl/openssl/milestone/15
> > > > >
> > > > > Some of the tickets listed below were already associated to the
> milestone, the others
> > > > > were added by me now.
> > > >
> > > > I think the 3.0.0 milestone is what we expect to be in the
> > > > 3.0.0 release, not the beta release. That is bug fixes don't need
> > > > to be in the beta release, but if it adds new functionallity it
> > > > needs to be in the beta release.
> > >
> > > I was aware of this subtlety but I thought that we just could (ab-)use
> the milestone for
> > > the beta1 release and reuse it later for the final release, instead of
> creating a new milestone.
> > >
> > > Practically all of the relevant PRs are associated to the [3.0 New
> Core + FIPS] GitHub Project
> > > anyway, so it would be possible to remove the post-beta PRs from the
> milestone and restore
> > > them later.  (In my mind, I see project managers running away
> screeming...)
> > >
> > > Matthias
> > >
> > >
> > > [3.0 New Core + FIPS]:  https://github.com/openssl/openssl/projects/2
>
>

Re: Reordering new API's that have a libctx, propq

[Nicola Tuveri]

On Sat, Sep 5, 2020, 14:01 Tim Hudson  wrote:

> On Sat, Sep 5, 2020 at 8:45 PM Nicola Tuveri  wrote:
>
>> Or is your point that we are writing in C, all the arguments are
>> positional, none is ever really optional, there is no difference between
>> passing a `(void*) NULL` or a valid `(TYPE*) ptr` as the value of a `TYPE
>> *` argument, so "importance" is the only remaining sorting criteria, hence
>> (libctx, propq) are always the most important and should go to the
>> beginning of the args list (with the exception of the `self/this` kind of
>> argument that always goes first)?
>>
>
> That's a reasonable way to express things.
>
> The actual underlying point I am making is that we should have a rule in
> place that is documented and that works within the programming language we
> are using and that over time doesn't turn into a mess.
> We do add parameters (in new function names) and we do like to keep the
> order of the old function - and ending up with a pile of things in the
> "middle" is in my view is one of the messes that we should be avoiding.
>

We are already adding new functions, with the ex suffix, to allow users to
keep using the old version, so given that the users passing to the "_ex"
function are already altering their source, why are we limiting us from
rationalizing the signature from the PoV of new users and old users alike
that are in general quite critic of our API usability, even if it means
that applying both "required-ness" and "importance" as sorting criteria
sometime we end up adding in the middle?

I don't think I am being dismissive of the needs of existing applications
here: if a maintainer is altering their code from using "EVP_foo()" to
"EVP_foo_ex()", they will likely also be looking at the documentation of
the old and the new API versions and there shouldn't be really any
additional significanf cost in adding a parameter a the edges or in the
middle.

I think the importance argument is the one that helps for setting order and
> on your "required args" coming first approach the importance argument also
> applies because it is actually a required argument simply with an implied
> default - which again puts it not at the end of the argument list. The
> library context is required - always - there is just a default - and that
> parameter must be present because we are working in C.
>

I think I disagree with this, from the API CONSUMER PoV there is a clear
difference between a function where they don't need to pass a valid libctx
pointer and instead pass NULL (because there is a default associated with
passing NULL) and a function like an hypothetical
OSSL_LIBCTX_get_this(libctx) or OSSL_LIBCTX_set_that(libctx, value).

In the first case the function operates despite the programmer not
providing an explicit libctx (because a default one is used), in the latter
the programmer is querying/altering directly the libctx and one must be
provided.

*… actually only now that I wrote out the greyed text above I think I am
starting to understand what you meant all along!*

Your point is that any API that uses a `libctx` (and `propq`) is always
querying/altering a libctx, even if we use NULL as a shortcut to mean "the
global one", so if we are fetching an algorithm, getting/setting something
on a libctx scope, creating a new object, we are always doing so from a
given libctx.
In that sense, when an API consumer is passing NULL they are not passing
"nothing" (on which my "optional arguments" approach is based), but they
are passing NULL as a valid reference to a very specific libctx.

I now see what you meant, and I can see how it is a relevant thing to make
evident also in the function signature/usage.

But I guess we can also agree that passing NULL as a very specific
reference instead of as "nothing", can create a lot of confusion for
external consumers of the API, if it took this long for one of us — ­*ok,
it's me, so probably everyone else understood your point immediately, but
still…* — to understand the difference you were pointing out, even knowing
how these functions work internally and being somewhat familiar with
dealing with libctx-s.
If we want to convey this difference properly to our users, would it be
better to use a new define?

#define OSSL_DEFAULT ((void*)42)

- OSSL_DFLT could be a shorter alternative, if we prefer brevity.
- I am intentionally not specifying _LIBCTX as we might reuse a similar
mechanism to avoid overloading NULL in other similar situations: e.g. for
propq, where NULL is again a shortcut for the global ones, compared with
passing "" as an empty properties query, seems like another good candidate
for using a symbol to explicitly highlight that the default propq will be
used
- I would avoid making OSSL_DFLT an alias for NU

Re: Reordering new API's that have a libctx, propq

[Nicola Tuveri]

On Sat, Sep 5, 2020, 12:13 Tim Hudson  wrote:

> On Sat, Sep 5, 2020 at 6:38 PM Nicola Tuveri  wrote:
>
>> In most (if not all) cases in our functions, both libctx and propquery
>> are optional arguments, as we have global defaults for them based on the
>> loaded config file.
>> As such, explicitly passing non-NULL libctx and propquery, is likely to
>> be an exceptional occurrence rather than the norm.
>>
>
> And that is where we have a conceptual difference, the libctx is *always* 
> used.
> If it is provided as a NULL parameter then it is a shortcut to make the
> call to get the default or to get the already set one.
> Conceptually it is always required for the function to operate.
>
> And the conceptual issue is actually important here - all of these
> functions require the libctx to do their work - if it is not available then
> they are unable to do their work.
> We just happen to have a default-if-NULL.
>
> If C offered the ability to default a parameter if not provided (and many
> languages offer that) I would expect we would be using it.
> But it doesn't - we are coding in C.
>
> So it is really where-is-what-this-function-needs-coming-from that
> actually is the important thing - the source of the information the
> function needs to make its choice.
> It isn't which algorithm is being selected - the critical thing is from
> which pool of algorithm implementations are we operating. The pool must be
> specified (this is C code), but we have a default value.
>
> And that is why I think the conceptual approach here is getting confused
> by the arguments appearing to be optional - conceptually they are not - we
> just have a defaulting mechanism and that isn't the same conceptually as
> the arguments actually being optional.
>
> Clearer?
>

It's not yet clear to me the distinction you are trying to make.

I'll try to spell out what I extrapolated from your answer, and I apologize
in advance if I am misunderstanding your argument, please be patient and
correct me in that case so I can better understand your point!

It seems to me you are making a conceptual difference between
- a function that internally requires an instance of foo to work (and has a
default if foo is given as NULL among the arguments); e.g libctx is
necessary for a fetch, if a NULL libctx is given a mechanism is in place to
retrieve the global default one
- a function that internally uses an instance of foo only if a non-NULL one
is passed as argument; e.g. bnctx, if the user provides it this is used by
the callee and passed to its callee, if the user passes NULL the function
creates a fresh one for itself and/or its callees


But as a consumer of the API that difference is not visible and probably
not even interesting, as we are programming in C and passing pointers,
there are certain arguments that are required and must be passed as valid
pointers, others that appear optional because as a consumer of that API I
can pass NULL and let the function internally default to a reasonable
behavior (and whatever this "reasonable behavior" is — whether the first or
the second case from above, or another one I did not list —, it's part of
the documentation of that API).

IMHO, in the consumer POV, libctx and propq are optional arguments (even in
C where optional or default arguments do not technically exist and the
caller needs to always specify a value for each argument, which are always
positional) in the sense that they can pass NULL as a value rather than a
pointer to a fully instantiated object of the required type.
Even more so given that, excluding a minority of cases, we can expect
consumers of the APIs taking libctx and propq as arguments to pass NULL for
both of them and rely on the openssl config mechanism.

So while I agree with Tim that sometime it is valuable to make a difference
among the consequences of passing NULL as arguments in the context of one
kind of function and another, I believe the place for that is the
documentation not its signature.
The signature of the function should be designed for consumer usability,
and the conventional pattern there seems to be
- required args
- optional args
- callback+cb_args
and inside each group the "importance" factor should be the secondary
sorting criteria.

"importance" is probably going to be affected by the difference you are
making (or my understanding of it): e.g. if a function took both libctx and
bnctx, the fact that a valid pre-existing libctx is required to work (and a
global already existing one will be retrieved in case none is given), while
a fresh short-lived bnctx is going to be created only for the lifetime of
the called function in case none is given seems to indicate that libctx is
of vital importance for the API functionality, while bnctx is of minor
relevance.

But... going this way as a generalize

Re: Reordering new API's that have a libctx, propq

[Nicola Tuveri]

Thanks Tim for the writeup!


I tend to agree with Tim's conclusions in general, but I fear the analysis
here is missing an important premise that could influence the outcome of
our decision.

In most (if not all) cases in our functions, both libctx and propquery are
optional arguments, as we have global defaults for them based on the loaded
config file.
As such, explicitly passing non-NULL libctx and propquery, is likely to be
an exceptional occurrence rather than the norm.

For optional parameters most developers from C and a variety of languages,
would expect them to come at the end of the list of parameters, and this
also follows the rule of thumb of "importance" used by Tim to pick 1) and
B/A).

For this reason I would instead suggest to go with 2) and C) in this case
(with the caveat of keeping callback and its args as the very last
arguments, again this is a non-written convention not only for us but quite
widespread).



Nicola



On Sat, Sep 5, 2020, 10:10 Tim Hudson  wrote:

> I place the OTC hold because I don't believe we should be making parameter
> reordering decisions without actually having documented what approach we
> want to take so there is clear guidance.
> This was the position I expressed in the last face to face OTC meeting -
> that we need to write such things down - so that we avoid this precise
> situation - where we have new functions that are added that introduce the
> inconsistency that has been noted here that PR#12778 is attempting to
> address.
>
> However, this is a general issue and not a specific one to OPENSSL_CTX and
> it should be discussed in the broader context and not just be a last minute
> (before beta1) API argument reordering.
> That does not provide anyone with sufficient time to consider whether or
> not the renaming makes sense in the broader context.
> I also think that things like API argument reordering should have been
> discussed on openssl-project so that the broader OpenSSL community has an
> opportunity to express their views.
>
> Below is a quick write up on APIs in an attempt to make it easier to hold
> an email discussion about the alternatives and implications of the various
> alternatives over time.
> I've tried to outline the different options.
>
> In general, the OpenSSL API approach is of the following form:
>
>
>
> *rettype  TYPE_get_something(TYPE *,[args])rettype  TYPE_do_something(TYPE
> *,[args])TYPE*TYPE_new([args])*
>
> This isn't universally true, but it is the case for the majority of
> OpenSSL functions.
>
> In general, the majority of the APIs place the "important" parameters
> first, and the ancillary information afterwards.
>
> In general, for output parameters we tend to have those as the return
> value of the function or an output parameter that
> tends to be at the end of the argument list. This is less consistent in
> the OpenSSL APIs.
>
> We also have functions which operate on "global" information where the
> information used or updated or changed
> is not explicitly provided as an API parameter - e.g. all the byname
> functions.
>
> Adding the OPENSSL_CTX is basically providing where to get items from that
> used to be "global".
> When performing a lookup, the query string is a parameter to modify the
> lookup being performed.
>
> OPENSSL_CTX is a little different, as we are adding in effectively an
> explicit parameter where there was an implicit (global)
> usage in place. But the concept of adding parameters to functions over
> time is one that we should have a policy for IMHO.
>
> For many APIs we basically need the ability to add the OPENSSL_CTX that is
> used to the constructor so that
> it can be used for handling what used to be "global" information where
> such things need the ability to
> work with other-than-the-default OPENSSL_CTX (i.e. not the previous single
> "global" usage).
>
> That usage works without a query string - as it isn't a lookup as such -
> so there is no modifier present.
> For that form of API usage we have three choices as to where we place
> things:
>
> 1) place the context first
>
> *TYPE *TYPE_new(OPENSSL_CTX *,[args])*
>
> 2) place the context last
>
> *TYPE *TYPE_new([args],OPENSSL_CTX *)*
>
> 3) place the context neither first nor last
>
> *TYPE *TYPE_new([some-args],OPENSSL_CTX *,[more-args])*
>
> Option 3 really isn't a sensible choice to make IMHO.
>
> When we are performing effectively a lookup that needs a query string, we
> have a range of options.
> If we basically state that for a given type, you must use the OPENSSL_CTX
> you have been provided with on construction,
> (not an unreasonable position to take), then you don't need to modify the
> existing APIs.
>
> If we want to allow for a different OPENSSL_CTX for a specific existing
> function, then we have to add items.
> Again we have a range of choices:
>
> A) place the new arguments first
>
>
> *rettype  TYPE_get_something(OPENSSL_CTX *,TYPE *,[args])rettype
>  TYPE_do_something(OPENSSL_CTX *,TYPE *,[args])*

Re: Backports to 1.1.1 and what is allowed

[Nicola Tuveri]

Sorry yes, I meant to refer to the open PR with the s390 support, I picked
the wrong number!

On Thu, Jun 25, 2020, 17:54 Matt Caswell  wrote:

>
>
> On 25/06/2020 15:33, Nicola Tuveri wrote:
> > In light of how the discussion evolved I would say that not only there
> > is consensus on supporting the definition of a detailed policy on
> > backports and the definitions of what are the requirements for regular
> > releases vs LTS releases (other than the longer support timeframe),
> > but also highlights a need to do it sooner rather than later!
> >
> > This seems a job for the OMC, as it falls under:
> >
> >> makes all decisions regarding management and strategic direction of the
> project; including:
> >> - business requirements;
> >> - feature requirements;
> >> - platform requirements;
> >> - roadmap requirements and priority;
> >> - end-of-life decisions;
> >> - release timing and requirement decisions;
> >
> > My contribution to the discussion is to ask if the OMC has plans for
> > addressing this in the very short term.
>
> I think its unlikely we are going to get to a policy in the short term.
> It seems to me we are still some way away from a consensus here.
>
> > If working on a policy is going to be a medium-term effort, maybe it
> > would be opportune to call an OTC vote specific to #11968 under the
> > current release requirements defined by the OMC (or lack thereof).
>
> 11968 is already merged and, AFAIK, no one has proposed reverting it. If
> such a PR was raised then a vote might be a way forward for it.
>
> 11188 is the more pressing problem because that is currently unmerged
> and stuck. That has an OTC hold on it (placed there by me), so an OTC
> vote seems appropriate. If a vote is held it should be to decide whether
> backporting it is consistent with our current understanding of the
> policy such as it is. It is for the OMC to decide whether a different
> policy should be introduced at some point in the future.
>
> Matt
>
>
> >
> > We already saw a few comments in favor of evaluating backporting
> > #11968 as an exception, in light of the supporting arguments, even if
> > it was in conflict with the current policy understanding or the
> > upcoming policy formulation.
> > So if we could swiftly agree on this being an OTC or OMC vote, I would
> > urge to have a dedicated discussion/vote specific to #11968, while
> > more detailed policies and definitions are in the process of being
> > formulated.
> >
> > - What is the consensus on splitting the 2 discussions?
> > - If splitting the discussions, is deciding on #11968 an OTC or OMC
> matter?
> >
> >
> >
> > Cheers,
> >
> > Nicola
> >
>

Re: Backports to 1.1.1 and what is allowed

[Nicola Tuveri]

In light of how the discussion evolved I would say that not only there
is consensus on supporting the definition of a detailed policy on
backports and the definitions of what are the requirements for regular
releases vs LTS releases (other than the longer support timeframe),
but also highlights a need to do it sooner rather than later!

This seems a job for the OMC, as it falls under:

> makes all decisions regarding management and strategic direction of the 
> project; including:
> - business requirements;
> - feature requirements;
> - platform requirements;
> - roadmap requirements and priority;
> - end-of-life decisions;
> - release timing and requirement decisions;

My contribution to the discussion is to ask if the OMC has plans for
addressing this in the very short term.
If working on a policy is going to be a medium-term effort, maybe it
would be opportune to call an OTC vote specific to #11968 under the
current release requirements defined by the OMC (or lack thereof).

We already saw a few comments in favor of evaluating backporting
#11968 as an exception, in light of the supporting arguments, even if
it was in conflict with the current policy understanding or the
upcoming policy formulation.
So if we could swiftly agree on this being an OTC or OMC vote, I would
urge to have a dedicated discussion/vote specific to #11968, while
more detailed policies and definitions are in the process of being
formulated.

- What is the consensus on splitting the 2 discussions?
- If splitting the discussions, is deciding on #11968 an OTC or OMC matter?



Cheers,

Nicola

Re: The hold on PR 12089

[Nicola Tuveri]

I believe the OMC is called into action as some name changes might be seen
as breaking API or ABI compatibility and that has been considered so far as
part of the first item in the OMC prerogatives list.

The matter of OMC Vs OTC vote also depends on what kind of hold Tim is
applying with his - 1: is it a OMC or a OTC hold?
Of course OMC can always override what OTC decides, but the discussion/vote
should happen in OTC/OMC depending on which hat Tim was wearing when
placing the hold.


Cheers,

Nicola

On Wed, Jun 10, 2020, 10:43 Salz, Rich  wrote:

> What is the timetable for resolving
> https://github.com/openssl/openssl/pull/12089 ?
>
>
>
> The Beta is planned for a July 16 release.  There is a massive RAND/DRBG
> PR (https://github.com/openssl/openssl/pull/11682, the provider-friendly
> random) that is in the pipeline, and 12089 and 11682 will undoubtedly cause
> merge issues whichever gets merged first. That means extra time will be
> needed to reconcile. An OMC vote, once started, can be resolved in as
> quickly as 24 hours, but often take one or two weeks if most people
> abstain.
>
>
>
> Being conservative, then, the OMC needs to discuss and vote, before the
> end of this month.
>
>
>
> An additional complication is around the question of who votes: the OMC or
> the OTC. It is hard to justify this as requiring OMC action, unless the
> project is committing to avoiding such language in the future as a policy.
> But if the project wants to decide that, it can do so. Regardless of the
> policy, PR 12089 could be seen as purely an OTC issue, and OMC involvement
> is over-reach – what in https://www.openssl.org/policies/omc-bylaws.html
> justifies OMC involvement?. Nothing changes but some names; is the naming
> of things within OMC perview? I would love to know what OTC members think.
>
>
>
> So, what is the timetable, and what is the plan?
>
>
>

Re: addrev warning

[Nicola Tuveri]

Yes, I also got that since I updated my git installation from the upstream
source tarball.

With recent versions of git this warning has been showing for months
already, but I don't know enough about it to propose a fix!


Nicola

On Mon, Jun 8, 2020, 12:16 Matt Caswell  wrote:

> After upgrading Ubuntu over the weekend I'm suddenly seeing this warning
> from addrev. Is anyone else getting this?
>
> WARNING: git-filter-branch has a glut of gotchas generating mangled history
>  rewrites.  Hit Ctrl-C before proceeding to abort, then use an
>  alternative filtering tool such as 'git filter-repo'
>  (https://github.com/newren/git-filter-repo/) instead.  See the
>  filter-branch manual page for more details; to squelch this
> warning,
>  set FILTER_BRANCH_SQUELCH_WARNING=1.
> Proceeding with filter-branch...
>
>
> MMatt
>

Re: Cherry-pick proposal

[Nicola Tuveri]

I think we changed enough things in the test infrastructure that there is a
chance of creating subtle failures by merging cherry-picked commits from
master directly.

>From the burden perspective, from my point of view having a separate PR
that ran all the CI without failures is actually a benefit: I can do some
minimal testing on my machine before the final merge, instead of having to
push a branch to my personal github fork to run travis and appveyor to test
weird build options on platforms I don't have access to.

If we stick to opening the PR for backporting only after master is
completely approved, there shouldn't be too big a burden for the original
reviewers either: we can use `fixup!` commits if trivial changes are
required to clearly highlight what was changed compared to the original PR
for master, and other than that it's just a matter of checking that nothing
else changed from the originally approved changes. Approvals conditional to
the CI passing can also help to further reduce the burden of the grace
period for backport PRs.


Nicola

On Wed, 29 Apr 2020 at 14:24, Dr Paul Dale  wrote:

> My concern is are 1.1.1 and 3.0 really all that different?
> The core is quite different but the cryptographic algorithms aren’t.  CMS,
> x509, …?
>
> I’d rather not introduce a burden where it isn’t necessary.
>
> Pauli
> --
> Dr Paul Dale | Distinguished Architect | Cryptographic Foundations
> Phone +61 7 3031 7217
> Oracle Australia
>
>
>
>
> On 29 Apr 2020, at 10:08 pm, Matt Caswell  wrote:
>
>
> The OTC have received this proposal and a request that we vote on it:
>
> I would like to request that we do not allow cherry-picks between master
> and 1.1.1-stable because these two versions are now very different, if a
> cherry-pick succeeds, there is no guarantee that the result will work.
> Because we have no CI for the cherry-pick. If a cherry-pick is needed, a
> new PR for 1.1.1 should be done and approved independently.
>
> Before starting a vote I'd like to provide opportunity for comments, and
> also what the vote text should be.
>
> Thanks
>
> Matt
>
>
>

Re: Cherry-pick proposal

[Nicola Tuveri]

I can agree it is a good idea to always require backport as a separate PR,
with the following conditions:
- unless it's a 1.1.1 only issue, we MUST always wait to open the
backport-to-111 PR until after the master PR has been approved and merged
(to avoid splitting the discussions among different PRs, which make review
and revisiting our history very hard)
- trivial documentation changes, such as fixing typos, can be exempted

It must be clear that although things have changed a lot in the inner
plumbings, we have so far managed to keep crypto implementations very much
in sync between 1.1.1 and master, by applying the policy of "first merge to
master, then possibly backport".

What I am afraid of in Bernd's proposal, and recent discussions, is that
committers might be tempted to open PRs changing implementations against
1.1.1 first (to avoid frequent rebases due to unrelated changes) and let
the `master` PR stagnate indefinitely because it feels like too much hassle
to keep up with the development pace of master if your PR collaterally
changes certain files.

An example of what can go wrong if we open a 1.1.1 PR concurrently with a
PR for master can be seen here:
- `master` PR: https://github.com/openssl/openssl/pull/10828
- `1.1.1` PR: https://github.com/openssl/openssl/pull/11411

I highlight the following problems related to the above example:
- as you can see the `1.1.1` has been merged, even though the `master` one
has stalled while discussing which implementation we should pick. (this was
my fault, I should have applied the `hold` label after stating that my
approval for 1.1.1 was conditional to approving the `master` counterpart)
- discussion that is integral part of the decision process was split among
the 2 PRs, with over 40 comments each
- there is no clear link between the `master` PR and the `1.1.1` PR for the
same feature (this makes it very difficult to review what is the state of
the "main" PR, and if we or someone else in some months or years needs to
go check why we did things the way we did, will have a hard time connecting
the dots)

I also think that the proposal as written is a bit misleading: I would very
like to keep seeing in 1.1.1 a majority of commits cherry-picked from
commits merged to master, with explicit tags in the 1.1.1 commit messages
(this helps keeping the git history self-contained without a too strong
dependency on GitHub).
I would rephrase the proposal as follows:

Merge to 1.1.1 should only happen after approval of a dedicated PR
targeting the OpenSSL_1_1_1-stable branch.

Potential amendments that I'd like the OTC to consider are:
a) before the end of the sentence add ", with the optional exclusion of
trivial documentation-only changes"
b) after the end of the sentence add "In composing backport pull requests,
explicit cherry-picking (`git cherry-pick -x`) of relevant commits merged
to `master` or another stable branch is recommended and encouraged whenever
possible."
c) adopt a more general statement:

Merge to any stable branch should only happen after approval of a
dedicated PR targeting specifically that branch.




So, in summary, I would agree with the proposal, as I definitely think
Bernd has a good point about running the 1.1.1 CI for things we think
should be backported, but requires careful consideration of additional
requirements to avoid duplicating review efforts, splitting discussions
that should be kept together, or disrupting our processes making 1.1.1 and
master diverge more than necessary.


Cheers,


Nicola

On Wed, 29 Apr 2020 at 14:08, Matt Caswell  wrote:

>
> The OTC have received this proposal and a request that we vote on it:
>
> I would like to request that we do not allow cherry-picks between master
> and 1.1.1-stable because these two versions are now very different, if a
> cherry-pick succeeds, there is no guarantee that the result will work.
> Because we have no CI for the cherry-pick. If a cherry-pick is needed, a
> new PR for 1.1.1 should be done and approved independently.
>
> Before starting a vote I'd like to provide opportunity for comments, and
> also what the vote text should be.
>
> Thanks
>
> Matt
>

Re: Face to face

[Nicola Tuveri]

I would like to propose as a date for the OTC meeting somewhere close to
the projected release date for 3.0alpha1.

Ideally it would be nice if OMC and OTC could coordinate the dates to be
close enough to ease the discussion of agenda items that might require
coordination between OMC and OTC.

My rationale for proposing a date next to the alpha1 release is:
- there might be open items for the release process that might benefit from
the attention of the whole OTC and be expedited with f2f discussions
- OMC and OTC might have items to discuss in both directions regarding the
final stages of the 3.0 release roadmap and the milestones for alpha2,
alpha3,  beta1

---

Regarding time for the OTC meeting, I would personally prefer during the
week.
Considering that the bulk of OTC members seem to be distributed between
Europe and Australia, would 7:00 (AM) UTC be a suitable time?

Keep in mind that dates between 29.03 and 05.04 happen to be between
daylight saving time changes in Europe and Australia, so using 31.03 as the
tentative date for the meeting 7:00 AM UTC would mean [0]:
- 8 AM in London
- 9 AM in Berlin/Brussels/Prague/Stockholm
- 5 PM in Brisbane

Nicola

[0]: http://j.mp/39oYWhw

On Tue, 3 Mar 2020 at 10:22, Dr Paul Dale  wrote:

> I propose that we don’t have either an OMC or OTC face to face meeting at
> ICMC.
> Travel restrictions and the availability list make it look unlikely.
>
> Instead, I’d suggest a video conference for both.
>
> This will probably require some kind of vote (for both).
>
>
> Any suggestions as to date and time.
>
>
> Pauli
> --
> Dr Paul Dale | Distinguished Architect | Cryptographic Foundations
> Phone +61 7 3031 7217
> Oracle Australia
>
>
>
>
>

Re: Errored: openssl/openssl#31939 (master - 34b1676)

[Nicola Tuveri]

On Fri, 14 Feb 2020 at 14:00, Matt Caswell  wrote:

>
> To be clear the build that is timing out uses *msan* not *asan*.


>
As I understand it msan detects unitialised reads. whereas asan detects
> memory corruption, buffer overflows, use-after-free bugs, and memory leaks.
>
> The previous "home-made" checks only detected memory leaks, so it is not
> comparable with the functionality offered by msan.
>
>
Thanks for the clarification! I was indeed confused!



> The msan documentation
> (https://clang.llvm.org/docs/MemorySanitizer.html) suggests that a slow
> down of 3x is typical.
>
> It seems reasonable to me to disable msan checks in Travis entirely, and
> have them only in run-checker.
>
>
I agree with you.


> > Here is another idea that would be interesting if we restore the
> > previous checks:
> > I don't know what kind of options github offers on this, but would it be
> > possible to run triggered CI on something that is not Travis and does
> > not timeout and still have the results in the PR?
>
> I am sure there are hooks to do this. Richard has been talking for quite
> a while about setting up a buildbot infrastructure. If that could be
> integrated into github that would be really neat.
>
>
It would be neat indeed, when I first heard about buildbot I tried to set
aside some time to play with it, but failed in finding the time needed!
But at least from what I read it does indeed seem like a very interesting
and useful tool for our purposes!

I have no doubt sooner or later Richard will be more successful than I was
at finding the time to work on this item as well!

Nicola

Re: Errored: openssl/openssl#31939 (master - 34b1676)

[Nicola Tuveri]

If ASAN is too slow to run in the CI should we restore the previous
homemade checks for memory leaks as an alternative to be run in regular CI
runs and leave ASAN builds to run-checker on the master branch only?

Here is another idea that would be interesting if we restore the previous
checks:
I don't know what kind of options github offers on this, but would it be
possible to run triggered CI on something that is not Travis and does not
timeout and still have the results in the PR?
If something like that would be possible we could move the ASAN builds to
extended_tests, rely on the previous memleak detection for the regular CI
runs, and then trigger with a script or Github Action the extended_tests
when the approval:done label is added.

That way, by the time something is ready to be merged we should have a full
picture!


Nicola

On Wed, Feb 5, 2020, 10:25 Matt Caswell  wrote:

> Since we fixed the Travis builds 4 out of the 8 builds on master that
> have taken place have errored due to a timeout.
>
> The msan build is consistently taking a *very* long time to run. If it
> gets to 50 minutes then Travis cuts it off and the build fails.
>
> Should we disable the msan build?
>
> Matt
>
>
>  Forwarded Message 
> Subject:Errored: openssl/openssl#31939 (master - 34b1676)
> Date:   Wed, 05 Feb 2020 00:02:01 +
> From:   Travis CI 
> To: openssl-comm...@openssl.org
>
>
>
> openssl
>
> /
>
> openssl
>
> <
> https://travis-ci.org/openssl/openssl?utm_medium=notification_source=email
> >
>
>
> branch iconmaster 
>
> build has errored
> Build #31939 has errored
> <
> https://travis-ci.org/openssl/openssl/builds/646181069?utm_medium=notification_source=email
> >
> arrow to build time
> clock icon50 mins and 3 secs
>
> Pauli avatarPauli
>
> 34b1676 CHANGESET →
> 
>
> Make minimum size for secure memory a size_t.
>
> The minimum size argument to CRYPTO_secure_malloc_init() was an int but
> ought
> to be a size_t since it is a size.
>
> From an API perspective, this is a change. However, the minimum size is
> verified as being a positive power of two and it will typically be a small
> constant.
>
> Reviewed-by: David von Oheimb 
> (Merged from #11003)
>
> Want to know about upcoming build environment updates?
>
> Would you like to stay up-to-date with the upcoming Travis CI build
> environment updates? We set up a mailing list for you!
>
> SIGN UP HERE 
>
> book icon
>
> Documentation  about Travis CI
>
> Have any questions? We're here to help. 
> Unsubscribe
> <
> https://travis-ci.org/account/preferences/unsubscribe?repository=5849220_medium=notification_source=email
> >
> from build emails from the openssl/openssl repository.
> To unsubscribe from *all* build emails, please update your settings
> <
> https://travis-ci.org/account/preferences/unsubscribe?utm_medium=notification_source=email
> >.
>
> black and white travis ci logo 
>
> Travis CI GmbH, Rigaer Str. 8, 10427 Berlin, Germany | GF/CEO: Randy
> Jacops | Contact: cont...@travis-ci.com  |
> Amtsgericht Charlottenburg, Berlin, HRB 140133 B | Umsatzsteuer-ID gemäß
> §27 a Umsatzsteuergesetz: DE282002648
>
>