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

2020-09-28 Thread 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

2020-09-13 Thread 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

2020-09-13 Thread 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

2020-09-05 Thread 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

2020-09-05 Thread 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

2020-09-05 Thread 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

2020-06-25 Thread 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

2020-06-25 Thread 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

2020-06-10 Thread 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

2020-06-08 Thread 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: Some more extra tests

2020-05-07 Thread Nicola Tuveri
I would be interested in seeing a PR to see what enabling these tests would
require!

I believe we do indeed need to test more thoroughly to ensure we are not
breaking the engine API!


Nicola

On Thu, May 7, 2020, 21:08 Dmitry Belyavsky  wrote:

> Dear colleagues,
>
> Let me draw your attention to a potentially reasonable set of extended
> tests for the openssl engines.
>
> The gost-engine project (https://github.com/gost-engine/engine) has some
> test scenarios robust enough for testing engine-provided algorithms and
> some basic RSA regression tests. It contains a rather eclectic set of C,
> Perl, and TCL(!) tests that are used by me on a regular basis.
>
> If these tests are included in the project extended test suite, they could
> reduce some regression that sometimes occurs (see
> https://github.com/gost-engine/engine/issues/232 as a current list of
> known problems).
>
> I will be happy to assist in enabling these tests as a part of openssl
> test suites.
> Many thanks!
>
> --
> SY, Dmitry Belyavsky
>


Re: Cherry-pick proposal

2020-04-29 Thread 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

2020-04-29 Thread 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

2020-03-04 Thread 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)

2020-02-14 Thread 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)

2020-02-14 Thread 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
>
>


Re: AW: [openssl] OpenSSL_1_1_1-stable update

2019-05-24 Thread Nicola Tuveri
I have always implicitly assumed Matt view, but I am happy to conform to
what the consensus is.

I believe this discussion is very useful and could contribute a new entry
in the commiter guidelines.

Nicola

On Fri, May 24, 2019, 07:21 Matt Caswell  wrote:

>
>
> On 24/05/2019 15:10, Richard Levitte wrote:
> > Not sure I see it as picking nits, it's rather about some fundamental
> > difference in what we thinking we're approving, and how we actually
> > act around that.
> >
> > My idea has always been that I approve a code change, i.e. essentially
> > a patch or a set of patches, without regard for exact branches it ends
> > up in.  With the in mind, the exact branches it gets applied to is a
> > *separate* question.
>
> That's not the way I've ever thought of it. In my mind an approval is for a
> change applied to a specific branch. Where a PR lists more than one branch
> in it
> and you approve the PR then effectively you are approving it multiple
> times all
> in one go - once for each branch.
>
>
> > If we go with the idea that an approval also involves approving what
> > branches it goes to, then what happens if someone realises after some
> > time that a set of commits (a PR) that was applied to master only
> > should really also be applied to 1.1.1?  Should the approval process
> > start over from scratch, i.e. all approvals that went to master should
> > be scratched and replaced with a new set of approvals (in principle)?
>
> No. If the PR was approved for master and applied to master then no
> problem - it
> stays in master. If it is later realised that it needs to be backported to
> other
> branches then, yes, new approvals need to be sought for that change to
> *those
> branches*.
>
> As far as I was aware we've always done this.
>
> This is essential in my mind. A change for one branch does not always make
> sense
> in another branch. So you can't just say "I approve this change" and *then*
> worry about what branches it applies to. A change only makes sense in the
> context of the branch it applies to.
>
> Matt
>