Re: Beta1 PR deadline

[Dr Paul Dale]

Shane has done the IG D.9 work: it is in #12835 and is awaiting review.  I 
think it should be in beta if possible.  It’s mostly adding a second test case 
due to NIST changing guidance recently, so low risk and helpful for the lab.

As Shane notes in 12801, there will be some conflicts generated over the FIPS 
error handling and self tests, hopefully they won’t slow progress.

12754 could be considered optional and something better should likely be done 
for 3.1 that what I’m planning there.  With RAND_DRBG removed, there is nothing 
remotely comparable.  Even RAND_DRBG is a bit of a stretch as a comparison.


The API renaming discussion *must* reach a conclusion before beta.


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




> On 10 Sep 2020, at 8:40 pm, Matt Caswell  wrote:
> 
> 
> 
> On 09/09/2020 13:03, Kurt Roeckx wrote:
>> On Wed, Aug 26, 2020 at 04:58:26PM +0100, Matt Caswell wrote:
>>> Please can anyone with PRs that they wish to have included in OpenSSL
>>> 3.0 beta1 ensure that they are merged to master by 8th September.
>> 
>> So that date has passed now. Can someone give an overview of what
>> we think is still needed to be done before the beta 1 release? Are
>> there open PRs for everything? Do we a milestone on github to
>> indicate which PRs need to go in before beta1?
> 
> 
> I believe the "blockers" to be:
> 
> https://urldefense.com/v3/__https://github.com/openssl/openssl/pull/12536__;!!GqivPVa7Brio!O5gzpHuWIANvRtRYOHA9m7SK4SqMUb2uLzEGQbUrX9nFSRhT7-sA0F-JQc5z_l4$
>  
> https://urldefense.com/v3/__https://github.com/openssl/openssl/pull/12754__;!!GqivPVa7Brio!O5gzpHuWIANvRtRYOHA9m7SK4SqMUb2uLzEGQbUrX9nFSRhT7-sA0F-JseFWMvQ$
>  
> https://urldefense.com/v3/__https://github.com/openssl/openssl/pull/12750__;!!GqivPVa7Brio!O5gzpHuWIANvRtRYOHA9m7SK4SqMUb2uLzEGQbUrX9nFSRhT7-sA0F-J0OAUQ5s$
>  
> https://urldefense.com/v3/__https://github.com/openssl/openssl/pull/12781__;!!GqivPVa7Brio!O5gzpHuWIANvRtRYOHA9m7SK4SqMUb2uLzEGQbUrX9nFSRhT7-sA0F-JYPbPTZA$
>  
> https://urldefense.com/v3/__https://github.com/openssl/openssl/pull/12801__;!!GqivPVa7Brio!O5gzpHuWIANvRtRYOHA9m7SK4SqMUb2uLzEGQbUrX9nFSRhT7-sA0F-J_mL-S18$
>  
> 
> Aside from these there are potential PRs not yet opened but slated for
> beta1 regarding:
> - Refactor serialization? (owner: Richard)
> - lhash refactor? (owner: Me)
> - IG D.9 update? (owner: Shane)
> 
> There's also this issue - but I'm not sure that it needs to be resolved
> for beta1:
> https://urldefense.com/v3/__https://github.com/openssl/openssl/issues/12195__;!!GqivPVa7Brio!O5gzpHuWIANvRtRYOHA9m7SK4SqMUb2uLzEGQbUrX9nFSRhT7-sA0F-J1-VRwBk$
>  
> 
> Matt

Re: Beta1 PR deadline

[Matt Caswell]

On 10/09/2020 11:40, Matt Caswell wrote:
> 
> 
> On 09/09/2020 13:03, Kurt Roeckx wrote:
>> On Wed, Aug 26, 2020 at 04:58:26PM +0100, Matt Caswell wrote:
>>> Please can anyone with PRs that they wish to have included in OpenSSL
>>> 3.0 beta1 ensure that they are merged to master by 8th September.
>>
>> So that date has passed now. Can someone give an overview of what
>> we think is still needed to be done before the beta 1 release? Are
>> there open PRs for everything? Do we a milestone on github to
>> indicate which PRs need to go in before beta1?
> 
> 
> I believe the "blockers" to be:
> 
> https://github.com/openssl/openssl/pull/12536
> https://github.com/openssl/openssl/pull/12754
> https://github.com/openssl/openssl/pull/12750
> https://github.com/openssl/openssl/pull/12781
> https://github.com/openssl/openssl/pull/12801
> 
> Aside from these there are potential PRs not yet opened but slated for
> beta1 regarding:
> - Refactor serialization? (owner: Richard)
> - lhash refactor? (owner: Me)
> - IG D.9 update? (owner: Shane)

Oh, I think this last one might be:

https://github.com/openssl/openssl/pull/12835

Matt

> 
> There's also this issue - but I'm not sure that it needs to be resolved
> for beta1:
> https://github.com/openssl/openssl/issues/12195
> 
> Matt
> 

Re: Beta1 PR deadline

[David von Oheimb]

Hi Matt et al.,

thanks to good collaboration with OTC members I've been able to get approved
most of my open PRs that would be good be included before feature freeze,
while a few such PRs appear 'nearly' approved.

Given the 24-hours grace period, the PRs #12478
, #12741
, and #12769
 won't make it today,
or is there a chance to still get them into beta1 a little later?

    David

On 26.08.20 17:58, Matt Caswell wrote:
> Hi all
>
> The OMC had a meeting today.
>
> Please can anyone with PRs that they wish to have included in OpenSSL
> 3.0 beta1 ensure that they are merged to master by 8th September.
>
> Note in particular that there is no PR at the moment to incorporate SM2
> asymmetric encryption into OpenSSL 3.0. This feature currently exists in
> 1.1.1, so if no PR emerges by the above date then this feature is liable
> to be dropped in 3.0. (Note: PRs for SM2 signatures *do* exist and are
> expected to be present).
>
>
> Matt
>
>

Re: Beta1 PR deadline

[David von Oheimb]

Hi Matt et al.,

thanks to good collaboration with OTC members I've been able to get approved
most of my open PRs that would be good be included before feature freeze,
while a few such PRs appear 'nearly' approved.

Given the 24-hours grace period, the PRs #12478
, #12741
, and #12769
 won't make it today,
or is there a chance to still get them into beta1 a little later?

    David

On 26.08.20 17:58, Matt Caswell wrote:
> Hi all
>
> The OMC had a meeting today.
>
> Please can anyone with PRs that they wish to have included in OpenSSL
> 3.0 beta1 ensure that they are merged to master by 8th September.
>
> Note in particular that there is no PR at the moment to incorporate SM2
> asymmetric encryption into OpenSSL 3.0. This feature currently exists in
> 1.1.1, so if no PR emerges by the above date then this feature is liable
> to be dropped in 3.0. (Note: PRs for SM2 signatures *do* exist and are
> expected to be present).
>
>
> Matt
>
>

Re: Beta1 PR deadline

[David von Oheimb]

On 09.09.20 14:03, Kurt Roeckx wrote:
> On Wed, Aug 26, 2020 at 04:58:26PM +0100, Matt Caswell wrote:
>> Please can anyone with PRs that they wish to have included in OpenSSL
>> 3.0 beta1 ensure that they are merged to master by 8th September.
> So that date has passed now. Can someone give an overview of what
> we think is still needed to be done before the beta 1 release?
> Are there open PRs for everything? Do we a milestone on github to
> indicate which PRs need to go in before beta1?
As far as my PRs are concerned (most of which have been related to some
extent to the CMP contribution):

Thanks to intensified exchange with OTC reviewers, everything that I see
urgent for inclusion before feature freeze
has either been merged already or meanwhile has been approved and is
going to be merged by tomorrow.

Just very few fixes/cleanups are not yet fully reviewed but I believe
they can still be handled during the beta phase.

    David

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

[Michael Richardson]

Richard Levitte  wrote:
> There are many red herrings in here, and I would argue that trying to
> be too uniform in the way you think about all functions may be
> harmful, because not all functions are classified the same.

> We cannot deny that many of our interfaces have an OOP flair.  We may
> be programming in C, but we very obviously have that kind of pattern,
> a "poor man's OOP" in C.  So speaking about our interfaces in OOP
> terms is not far fetched, as long as we keep in mind that we can only
> take the argument so far.

It would be very nice if there was object-focused (rather than "oriented")
documentation for OpenSSL.
Tell me what all the structures/typedefs are, and then tell me what things 
operate on them.
(I tried to start documentation like that for the CMS parts, but I got 
distracted.)

--
]   Never tell me the odds! | ipv6 mesh networks [
]   Michael Richardson, Sandelman Software Works|IoT architect   [
] m...@sandelman.ca  http://www.sandelman.ca/|   ruby on rails[



signature.asc
Description: PGP signature

Re: More GitHub labels

[Dmitry Belyavsky]

On Thu, Sep 10, 2020 at 9:20 AM Dr. Matthias St. Pierre <
matthias.st.pie...@ncp-e.com> wrote:

> > ... I think we should change that. This does not mean that a reviewer
> who made a change request
> > two months ago and lost interest is forced to re-review, only that such
> stale reviews must be dismissed
> > explicitly, if the reviewer does not respond to a re-review request
> within a certain time period.
>
> I would refine this procedure as follows: the team member who intends to
> do the merge (the "merger"),
> needs to issue re-review requests for all unresolved change requests
> (there is a spinning button next the
> name of the reviewer to do this). The person who receives the re-review
> request can either dismiss its
> review or indicate that it intends to review within x hours. Otherwise,
> the merger can dismiss the stale review.
>
> Sorry, it seems a bit overengineering for me.
I'd prefer a procedure with explicit hold and explanation in the comments.

-- 
SY, Dmitry Belyavsky

RE: More GitHub labels

[Dr. Matthias St. Pierre]

> ... I think we should change that. This does not mean that a reviewer who 
> made a change request
> two months ago and lost interest is forced to re-review, only that such stale 
> reviews must be dismissed
> explicitly, if the reviewer does not respond to a re-review request within a 
> certain time period.

I would refine this procedure as follows: the team member who intends to do the 
merge (the "merger"),
needs to issue re-review requests for all unresolved change requests (there is 
a spinning button next the
name of the reviewer to do this). The person who receives the re-review request 
can either dismiss its
review or indicate that it intends to review within x hours. Otherwise, the 
merger can dismiss the stale review.
 
Matthias

RE: More GitHub labels

[Dr. Matthias St. Pierre]

> Your suggestion seems workable too.  PRs are merged with outstanding change 
> requests indicated
> — a reviewer comments, the comments are addressed then a different reviewer 
> approves without
> the original review being removed.  The labels are a bit more in your face.  
> A hybrid “hold: required changes see review/comments” might be workable.

GitHub can be configured to require approval. It then gives a clear visual 
indication that the PR is
not ready to be merged, and the Merge button is greyed out. This should be 
obvious enough,
even more obvious than a label, which can also easily be ignored.

Of course, our merge procedure circumvents the merge button, I'm only talking 
about the visual
indicator. Alternatively, the pre-receive-hook on the git.openssl.org server 
could enforce the policy
by checking the  reviewer state via GitHub API queries.

Matthias

Re: More GitHub labels

[Dr Paul Dale]

Matthias,

Your suggestion seems workable too.  PRs are merged with outstanding change 
requests indicated — a reviewer comments, the comments are addressed then a 
different reviewer approves without the original review being removed.  The 
labels are a bit more in your face.  A hybrid “hold: required changes see 
review/comments” might be workable.


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




> On 10 Sep 2020, at 4:03 pm, Dr. Matthias St. Pierre 
>  wrote:
> 
>> Just wondering if we should have two new labels: “hold: tests needed” and 
>> “hold: documentation needed” labels?
>> There are a number of PRs that come through where one or both of these are 
>> missing missing.
> 
> The two use cases you mention are actually better handled by a change request 
> (via GitHub review) instead of a label:
> A team member can formulate a change request to block the merging. Here he 
> has more freedom to formulate what
> needs to be done. This includes your two use cases, as well as the use case 
> for the label [hold: need rebase].
> 
> The problem with it is that currently we count only approvals and don’t 
> consider unresolved change requests
> as a blocker. I think we should change that. This does not mean that a 
> reviewer who made a change request
> two months ago and lost interest is forced to re-review, only that such stale 
> reviews must be dismissed
> explicitly, if the reviewer does not respond to a re-review request within a 
> certain time period.
> 
> Matthias
> 

RE: More GitHub labels

[Dr. Matthias St. Pierre]

> Just wondering if we should have two new labels: “hold: tests needed” and 
> “hold: documentation needed” labels?
> There are a number of PRs that come through where one or both of these are 
> missing missing.

The two use cases you mention are actually better handled by a change request 
(via GitHub review) instead of a label:
A team member can formulate a change request to block the merging. Here he has 
more freedom to formulate what
needs to be done. This includes your two use cases, as well as the use case for 
the label [hold: need rebase].

The problem with it is that currently we count only approvals and don’t 
consider unresolved change requests
as a blocker. I think we should change that. This does not mean that a reviewer 
who made a change request
two months ago and lost interest is forced to re-review, only that such stale 
reviews must be dismissed
explicitly, if the reviewer does not respond to a re-review request within a 
certain time period.

Matthias