Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

S390x backports for 1.1.1 #11188

Closed

Conversation

xnox
Copy link
Contributor

@xnox xnox commented Feb 26, 2020

@p-steuer

this rebases existing ubuntu s390x patches on top of OpenSSL_1_1_1-stable branch
... cherrypicks more things from master
... or from your ecc111 branch, which was first rebased on top of OpenSSL_1_1_1-stable

I'll try to ship all of this into ubuntu, if it like builds and passes tests. So far I have only statically assembled this, without like even test building it.

@openssl-machine openssl-machine added the hold: cla required The contributor needs to submit a license agreement label Feb 26, 2020
@kroeckx
Copy link
Member

kroeckx commented Feb 26, 2020

I think you want to drop commit f993cc2f80abcac516e6b7b1ef56fc58270a0975 from that list

@p-steuer
Copy link
Member

The following diff is needed for make update to work (and fix failing travis):

diff --git a/crypto/ec/ecdsa_ossl.c b/crypto/ec/ecdsa_ossl.c
index a517e99..6f1edea 100644
--- a/crypto/ec/ecdsa_ossl.c
+++ b/crypto/ec/ecdsa_ossl.c
@@ -202,35 +202,35 @@ ECDSA_SIG *ecdsa_simple_sign_sig(const unsigned char *dgst, int dgst_len,
     priv_key = EC_KEY_get0_private_key(eckey);
 
     if (group == NULL) {
-        ECerr(EC_F_OSSL_ECDSA_SIGN_SIG, ERR_R_PASSED_NULL_PARAMETER);
+        ECerr(EC_F_ECDSA_SIMPLE_SIGN_SIG, ERR_R_PASSED_NULL_PARAMETER);
         return NULL;
     }
     if (priv_key == NULL) {
-        ECerr(EC_F_OSSL_ECDSA_SIGN_SIG, EC_R_MISSING_PRIVATE_KEY);
+        ECerr(EC_F_ECDSA_SIMPLE_SIGN_SIG, EC_R_MISSING_PRIVATE_KEY);
         return NULL;
     }
 
     if (!EC_KEY_can_sign(eckey)) {
-        ECerr(EC_F_OSSL_ECDSA_SIGN_SIG, EC_R_CURVE_DOES_NOT_SUPPORT_SIGNING);
+        ECerr(EC_F_ECDSA_SIMPLE_SIGN_SIG, EC_R_CURVE_DOES_NOT_SUPPORT_SIGNING);
         return NULL;
     }
 
     ret = ECDSA_SIG_new();
     if (ret == NULL) {
-        ECerr(EC_F_OSSL_ECDSA_SIGN_SIG, ERR_R_MALLOC_FAILURE);
+        ECerr(EC_F_ECDSA_SIMPLE_SIGN_SIG, ERR_R_MALLOC_FAILURE);
         return NULL;
     }
     ret->r = BN_new();
     ret->s = BN_new();
     if (ret->r == NULL || ret->s == NULL) {
-        ECerr(EC_F_OSSL_ECDSA_SIGN_SIG, ERR_R_MALLOC_FAILURE);
+        ECerr(EC_F_ECDSA_SIMPLE_SIGN_SIG, ERR_R_MALLOC_FAILURE);
         goto err;
     }
     s = ret->s;
 
     if ((ctx = BN_CTX_new()) == NULL
         || (m = BN_new()) == NULL) {
-        ECerr(EC_F_OSSL_ECDSA_SIGN_SIG, ERR_R_MALLOC_FAILURE);
+        ECerr(EC_F_ECDSA_SIMPLE_SIGN_SIG, ERR_R_MALLOC_FAILURE);
         goto err;
     }

I checked it and the changes are also missing on my ecc111 branch (make update would fail here also). I either lost them when back-fitting the commits from master, or those are needed in addition.

@xnox
Copy link
Contributor Author

xnox commented Feb 27, 2020

I think you want to drop commit f993cc2 from that list

for upstream yes; for ubuntu no. As I am cherrypicking things from master, that are not present on 1.1.1 branch, my license gets polluted.

This is the best way I can request IBM to review what i'm trying to get into Ubuntu s390x. At the moment this is to request a public review of what I'm trying to put into Ubuntu which is basically 1_1_1-stable + a bunch of s390x backports.

@p-steuer
Copy link
Member

p-steuer commented Feb 27, 2020

The error constant fixup is basically a fixup for 7f06dc9 : id propose to squash those.

The include header fixup is because the 1.1.1 backport of 25f2138 (which is 0c994d5) does not include this change. Reason is, the original header include was introduced in f39ad8d , which is not on 1.1.1. Since c2c567e45b56d8e45466669b930b7c9f26544fd4 is the 1.1.1 backport of said commit, id propose to squash it with the fixup.

I ll continue with my review later today.

@xnox
Copy link
Contributor Author

xnox commented Feb 27, 2020

Travis is now green, but I'll do another upload into Ubuntu Autopkgtest infra, as the tests there did not look good.

@xnox xnox force-pushed the s390x-backport-on-stable+ecc111 branch from 25f8dbf to b3a2685 Compare February 27, 2020 13:01
@xnox
Copy link
Contributor Author

xnox commented Feb 27, 2020

Rebased, and squashed together the two commits as suggested.

@p-steuer
Copy link
Member

I did the following tests:

  • run the test suite with cpu models zEC12, z13, z14, z15.
  • verified the vector code paths for chacha20 cipher and poly1305 mac for >= z13.
  • verified the MSA9 code paths for P-256/384/521 ECDSA/ECDH, Ed/X25519, Ed/X448 signatures/keyexchange for z15.

All tests tests were successful.

My only finding is related to additional tests i did for the OPENSSL_s390xcap env variable:
The backport of 19bd1fa misses the env variable part. You picked it up from my ecc111 branch which does not include the whole env var backport, so those parts were lost. This leads to z15 HW support accidentially being disabled when setting OPENSSL_s390xcap=z15.

I think its the following chunks which are missing:

diff --git a/crypto/s390xcap.c b/crypto/s390xcap.c
index 3e6aeae..64de9d0 100644
--- a/crypto/s390xcap.c
+++ b/crypto/s390xcap.c
@@ -646,14 +646,22 @@ static int parse_env(struct OPENSSL_s390xcap_st *cap)
         /*.pcc    = */{S390X_CAPBIT(S390X_QUERY),
                        S390X_CAPBIT(S390X_SCALAR_MULTIPLY_P256)
                        | S390X_CAPBIT(S390X_SCALAR_MULTIPLY_P384)
-                       | S390X_CAPBIT(S390X_SCALAR_MULTIPLY_P521)},
+                       | S390X_CAPBIT(S390X_SCALAR_MULTIPLY_P521)
+                       | S390X_CAPBIT(S390X_SCALAR_MULTIPLY_ED25519)
+                       | S390X_CAPBIT(S390X_SCALAR_MULTIPLY_ED448)
+                       | S390X_CAPBIT(S390X_SCALAR_MULTIPLY_X25519)
+                       | S390X_CAPBIT(S390X_SCALAR_MULTIPLY_X448)},
         /*.kdsa   = */{S390X_CAPBIT(S390X_QUERY)
                        | S390X_CAPBIT(S390X_ECDSA_VERIFY_P256)
                        | S390X_CAPBIT(S390X_ECDSA_VERIFY_P384)
                        | S390X_CAPBIT(S390X_ECDSA_VERIFY_P521)
                        | S390X_CAPBIT(S390X_ECDSA_SIGN_P256)
                        | S390X_CAPBIT(S390X_ECDSA_SIGN_P384)
-                       | S390X_CAPBIT(S390X_ECDSA_SIGN_P521),
+                       | S390X_CAPBIT(S390X_ECDSA_SIGN_P521)
+                       | S390X_CAPBIT(S390X_EDDSA_VERIFY_ED25519)
+                       | S390X_CAPBIT(S390X_EDDSA_VERIFY_ED448)
+                       | S390X_CAPBIT(S390X_EDDSA_SIGN_ED25519)
+                       | S390X_CAPBIT(S390X_EDDSA_SIGN_ED448),
                        0ULL},
     };


diff --git a/doc/man3/OPENSSL_s390xcap.pod b/doc/man3/OPENSSL_s390xcap.pod
index 1f4ee85..172cf60 100644
--- a/doc/man3/OPENSSL_s390xcap.pod
+++ b/doc/man3/OPENSSL_s390xcap.pod
@@ -145,6 +145,10 @@ the numbering is continuous across 64-bit mask boundaries.
       # 64    1<<63    PCC-Scalar-Multiply-P256
       # 65    1<<62    PCC-Scalar-Multiply-P384
       # 66    1<<61    PCC-Scalar-Multiply-P521
+      # 72    1<<55    PCC-Scalar-Multiply-Ed25519
+      # 73    1<<54    PCC-Scalar-Multiply-Ed448
+      # 80    1<<47    PCC-Scalar-Multiply-X25519
+      # 81    1<<46    PCC-Scalar-Multiply-X448
 
  kdsa :
       #  1    1<<62    KDSA-ECDSA-Verify-P256
@@ -153,6 +157,10 @@ the numbering is continuous across 64-bit mask boundaries.
       #  9    1<<54    KDSA-ECDSA-Sign-P256
       # 10    1<<53    KDSA-ECDSA-Sign-P384
       # 11    1<<52    KDSA-ECDSA-Sign-P521
+      # 32    1<<31    KDSA-EdDSA-Verify-Ed25519
+      # 36    1<<27    KDSA-EdDSA-Verify-Ed448
+      # 40    1<<23    KDSA-EdDSA-Sign-Ed25519
+      # 44    1<<19    KDSA-EdDSA-Sign-Ed448
       :
 
 =head1 RETURN VALUES

I propose to squash these changes with 3ed77a3d527a459726cef230d1811488e323a4ff

@kroeckx
Copy link
Member

kroeckx commented Feb 27, 2020 via email

@xnox
Copy link
Contributor Author

xnox commented Mar 4, 2020

for upstream yes; for ubuntu no. As I am cherrypicking things from master, that are not present on 1.1.1 branch, my license gets polluted.
You might not be able to put the 1.1.1 code under the Apache license, we actually had to remove code that was present in 1.1.1. And this is an upstream pull request, so we don't want to merge that commit.

Ok, so how do you handle cherry-picks of Apache code from master onto the 1.1.1 branch? All of the code I'm trying to cherrypick above is not mine, and all of it is under Apache license right? Or is everything in master dual licensed? Or can only the original authors cherrypick from master and thus dual-license their contributions?

@xnox xnox force-pushed the s390x-backport-on-stable+ecc111 branch 2 times, most recently from a7e9237 to 8afbe3e Compare March 4, 2020 09:48
@p-steuer
Copy link
Member

p-steuer commented Mar 4, 2020

@xnox Looks good to me now and passes all tests. See also p-steuer#1 (comment) .

@kroeckx
Copy link
Member

kroeckx commented Mar 4, 2020 via email

@p-steuer
Copy link
Member

p-steuer commented Mar 4, 2020

Or can only the original authors cherrypick from master and thus dual-license their contributions?

afaik not the author but the copyright owner could always chose to do that. But as Kurt mentioned, this case should be covered by the cla anyway.

@p-steuer
Copy link
Member

p-steuer commented Mar 5, 2020

I discussed with @xnox about actually trying to get this merged to 1_1_1-stable (i.e. really treating HW acceleration as fixes): backports based on a single upstream stable branch instead of many different distro packages would IMO benefit both HW vendors and distributors. See discussion here p-steuer#1, especially comments p-steuer#1 (comment) and p-steuer#1 (comment) . I would also like to hear @t8m 's point of view.

@t8m
Copy link
Member

t8m commented Mar 5, 2020

We already treat performance fixes as bug fixes. However a completely new implementation is a little bit different beast so I am not sure. With my distro hat on, I would be fine with it, but it is stretching the openssl release policy.

@kroeckx
Copy link
Member

kroeckx commented Mar 5, 2020 via email

@mspncp
Copy link
Contributor

mspncp commented Mar 5, 2020

With my distro hat on, ...

What a nice pun! :-)

@xnox xnox force-pushed the s390x-backport-on-stable+ecc111 branch from 8afbe3e to 1d39b9a Compare March 30, 2020 11:10
@xnox
Copy link
Contributor Author

xnox commented Mar 30, 2020

Rebased on top of current 1.1.1-stable branch.
Dropped the "license change" patch from manpages.
I don't know what to do about cla-check -> although i assembled patches in here, all the code comes from master/p-steuer and is under cla.
Otherwise, this is ready to be merged from my point of view.

@mattcaswell
Copy link
Member

I don't know what to do about cla-check -> although i assembled patches in here, all the code comes from master/p-steuer and is under cla.

There are two separate CLA issues.

Firstly cla-check is detecting that you committed these changes (even though someone else authored them), and is complaining that there isn't a CLA on file for you. The simplest solution to this is for you to just submit a CLA.

The second issue is a missing CLA for Paul Yang (@InfoHunter). The reason is that you have cherry-picked a commit that was pushed under a CLA that was valid at the time it was pushed. That CLA is no longer valid for new contributions (there is a replacement CLA for Paul). The cla-bot isn't clever enough to detect that this is actually a backport of something that is legitimately in master - and just sees this as an invalid CLA. We might need an OMC member to temporarily re-enable that CLA when it comes to pushing this to workaround that problem. Or alternatively someone goes and fixes the cla bot to be a bit more intelligent.

@p-steuer
Copy link
Member

The only commit by paul that is included is aa65b5f895d0e9d58ee2d27796a6965793f112af which just moves the s390x man page's return value section above the example section. I think you could just drop it ?

Wrt to committing changes that i authored: Im okay with that.

@xnox xnox force-pushed the s390x-backport-on-stable+ecc111 branch from 1d39b9a to a809253 Compare March 30, 2020 20:30
p-steuer added 4 commits June 25, 2020 13:51
Signed-off-by: Patrick Steuer <[email protected]>

Reviewed-by: Richard Levitte <[email protected]>
(Merged from openssl#10004)

(cherry picked from commit b3681e2)
Signed-off-by: Dimitri John Ledkov <[email protected]>
Signed-off-by: Patrick Steuer <[email protected]>

Reviewed-by: Richard Levitte <[email protected]>
(Merged from openssl#10004)

(cherry picked from commit ac037dc)
Signed-off-by: Dimitri John Ledkov <[email protected]>
using PCC and KDSA instructions.

Signed-off-by: Patrick Steuer <[email protected]>

Reviewed-by: Richard Levitte <[email protected]>
(Merged from openssl#10004)

Signed-off-by: Patrick Steuer <[email protected]>
(cherry picked from commit 56eb364ef5cf93e2658cd45fcffee435b80857b2)
Signed-off-by: Dimitri John Ledkov <[email protected]>
x25519 has such a test vector obtained from wycheproof but wycheproof
does not have a corresponding x448 test vector.
So add a self-generated test vector for that case.

Signed-off-by: Patrick Steuer <[email protected]>

Reviewed-by: Matt Caswell <[email protected]>
(Merged from openssl#10339)

(cherry picked from commit fd60f8da74c68ba56f828bcc59141856503ffa0a)
Signed-off-by: Dimitri John Ledkov <[email protected]>
@xnox xnox force-pushed the s390x-backport-on-stable+ecc111 branch from 087be09 to bf34f39 Compare June 25, 2020 12:52
@openssl-machine openssl-machine removed the hold: cla required The contributor needs to submit a license agreement label Jun 25, 2020
@xnox
Copy link
Contributor Author

xnox commented Jul 1, 2020

I started a thread on openssl-project about this. The arguments stated in favour of backport above are good, but IMO go beyond our current policy (which is far too vague and under specified).

@mattcaswell the discussion takes a very different path, to what LTS means in Ubuntu. And the wikipedia article referenced, also doesn't quite live up to what Ubuntu means by LTS which is the first distro to introduce it.

For example, the current OpenSSL -stable branches fail to meet Ubuntu's LTS requirements and we do not take stable branches updates as a whole to the latest release as a letter.

Yet at the some time, the changes we do cherrypick into our LTS releases do not meet OpenSSL Upstream requirements either.

It seems to me that there is a significant impedance mismatch, and people mix different definitions of what stable means, and when.

  • Stability - No changes at all ever. Just use 1.1.1a and never change it.
  • Stability - not changing API/ABI, or at least ensure that previous behavior is accessible.
  • Security - how long security patches will be provided for
  • Hardware support - adding support for new or faster HW, whilst keeping the same API/ABI

There are desires for all sorts of things, from various people. For example, a lot of deployments desire absolute stability want no changes at all. Such deployments typically operate against frozen package sets, do not apply updates, and/or fork things to only change things they want. Whilst accepting the fact they have bugs and security vulnerabilities, as they value no change in behavior over anything. Some requests from such deployments are for livepatching libssl library in memory of actively running daemons, to ensure no downtime or even interruption in serving requests or active connections. There is not much to do here, as typically such deployments freeze on a particular build of OpenSSL and just keep using it.

Some deployments, with good CI/CD, typically want all the bug fixes, as long as there is no ABI / API changes, such that updates may trigger rolling restarts/reboots, but not rebuilding software. Ie. something along the lines of drop-in updates. Even regressions are ok, as long as they are identified and fixed up quickly enough, and they can update to newer git commit of the stable branch. Currently OpenSSL project does not cater these deployments well, because release schedule of stable branches is not predictable, but only dictated by it seems security CRDs and regressions those cause. It would be nice if there was a stable release cut for LTS ABI every 8th Tuesday.

Security-only updates. A lot of deployments are in between the above two. They don't care about bugfixes, but only care to apply security updates out of caution or compliance. Currently OpenSSL project does not cater these deployments well. It would be nice, if every point release of the stable LTS ABI had a security-only branch of fixes, released as patch only (i.e. not even tarballs). I.e. 1.1.1a-security, 1.1.1b-security, 1.1.1c-security git branches, which only contained CVE bugfixes as clean cherry-picks, as small as possible, without any other changes that are unrelated. Possibly even just High and Critical, no Mediums or Lows. Many distributions make distinction between security and non-security updates, prioritize them differently, and have longer support timeframes for security, etc. For example, it would be nice if 1.1.1[adg]-security (every third point release) had a 20 year support time-frame,

New Hardware, stable ABI, cost per watt. When one runs out of capacity and needs more hardware, typically new-shiny hardware is purchased or services are rotated onto. Or for example it is not cost-effective to buy old hardware as it's simply is no longer available for sale. But because the deployment is not homogenous, one often wants to deploy software stack with the same stable LTS ABI. But at the same time, squeeze everything out of the available hardware that one possibly can because money was spent on shiny metal. To support these very large deployments, they want ABI stability yet any performance improvements they can possible get, whilst not breaking the ABI. This is especially true during the early timeframe of a stable LTS ABI, whilst no next one is available yet. One is stuck between wanting the performance that is available in v3/master, yet requiring -security availablility of 1.1.1 LTS ABI. These deployments don't even usual care about bugfixes. But really performance improvements only. Or for example if a bugfix regresses performance, they will concider it to be a savere regression and rollback and switch to the "no-changes" camp.

An Ubuntu LTS release goes through multiple stages over it lifecycle. At first, it caters for all of the above use cases. That is for the first 2 years, via -security and -updates streams. Once the next LTS is out, performance backports stop (no more HWE updates). Then for the next 3 years we provide bugfixes and security updates. Then for the next 5 years we provide select security updates for high and critical only. And then only extended security maintenance is available for an even smaller subset of packages.

In terms of naming, during first 2 years we push out point releases with updated HWE stacks (Hardware Enablement). Then point releases stop, and we only do USN & SRU (ubuntu security notice, and stable release updates). Then we reach end of basic support, start of extended security maintainance, meaning SRUs stop and only USN continue. And then eventually we reach end of ESM.

The policy of what goes into -stable branches, does not need to be fixed in time, and stay the same and forever.

A lot of hardware vendors, added a lot of performance and hardware specific features in v3/master. And that release has slipped. So a lot of people want to use v3-only performance patches with stable LTS 1.1.1 ABI. And there is nothing that can cater for them at the moment.

It would be useful for example to cut 1.1.1g-security branch which will only have security patches applied without any bugfixes or perfomance fixes. And accept bugfixes + performance changes into 1.1.1-stable, until v3 is out. Once v3 is out, only accept security fixes into 1.1.1-stable branch and nothing else, no more bugfixes and no more performance improvements.

I understand this is very different from what was done before, but nobody wants intermixed security patches with bugfixes with performance improvements. As each type of changes, have risks of introducing regressions which are not worth for the end user if they don't care about that type of changes.

@xnox
Copy link
Contributor Author

xnox commented Jul 1, 2020

Separately from the 1.1.1-stable policy discussions, I'd like to request exception on how the s390x port is handled in particular. The set of maintainers for it is small, and are common between upstream and downstream distributions.

And at the moment, there have been two mainframe generations with better crypto acceleration, that are supported by master/v3, but not by 1.1.1 series. I would like to request exception to the 1.1.1-stable branch policy to backport hardware acceleration patches from master to 1.1.1-stable to enable exploiting the performance features that z14 and z15 offer. I and s390x port maintainers commit to QA and support of these backports, and regressions that may arise from doing picking them up.

How can I apply for such exception to the 1.1.1-stable rules, without changing the rules for other ports / features?

@kroeckx
Copy link
Member

kroeckx commented Jul 1, 2020 via email

@xnox
Copy link
Contributor Author

xnox commented Jul 1, 2020

It would be nice if there was a stable release cut for LTS ABI every 8th Tuesday.

We have been doing stable releases about 3 months apart, even when there wasn't a security issue that needed to be released. We didn't announce this, nor do we always properly follow up on this. So you're suggesting that we actually make a policy of this?

Yes please. Predictable regular point release, create cadence, for everyone to align with. It will make it a lot easier to consume those releases. And make them quicker adopted.

It would be nice if security CRD happens there is an extra release in between the cadence. But make it just the security patch only, as a clean patch on top of last point release only, which is merged into stable, and will be rolled up with stable fixes when the regular cadence happens.

(d) * - * - * - * - * - *  - * (f)
       \               /
         CVE-patch (e)

Hopefully that makes sense. That would make it a lot easier to ship security updates. Because those that are already on (d) release, will take (e) straight away, becuase it contains security patch only. Whereas "regular cadence" (f) release will have rolled security fixes and bufixes. But is not as urgent to upgrade to if one is on (d) or (e) already, and happy with it.

ps. I guess 1.1.1d.1, 1.1.1d.2 are security only, whereas 1.1.1f is regular cadence stable release with both security and bug-fixes. And eventually stop doing bug-fixes, and continue security-only.

@richsalz
Copy link
Contributor

richsalz commented Jul 1, 2020

You folks do realize that there are only two full-time developers, and under a dozen volunteers, right?

@xnox
Copy link
Contributor Author

xnox commented Jul 2, 2020

You folks do realize that there are only two full-time developers, and under a dozen volunteers, right?

And 3 part-time people, maintaining this patchset on top of older snapshots of 1.1.1-stable in three distributions. Yes, we want to minimize the amount of total duplicated maintainance there is of this code for the 1.1.1-ABI globally. To free up time, to do interesting things with OpenSSL. Because backporting patches over and over again, and maintaining conflicts / rebases, is not productive. Sharing stable trees, with common features that all distros ship is the way to go.

@openssl-machine
Copy link
Collaborator

This PR is in a state where it requires action by @openssl/otc but the last update was 30 days ago

@romen
Copy link
Member

romen commented Aug 24, 2020

I started a thread on openssl-project about this. The arguments stated in favour of backport above are good, but IMO go beyond our current policy (which is far too vague and under specified).

That discussion evolved and splitted in 2 parts (as mentioned here):

  • OMC can work on detailing a new more explicit policy about the OpenSSL definitions of LTS and STS releases, and more explicit criteria to accept/reject certain grey-area contributions into the stable branches of currently supported releases
  • OTC might hold a vote to determine if merging this PR matches our understanding of the current OMC guidelines

@mattcaswell given you put the OTC hold, will you call the OTC vote on this? Should we wait until after beta1 release?

@mattcaswell
Copy link
Member

OMC can work on detailing a new more explicit policy about the OpenSSL definitions of LTS and STS releases, and more explicit criteria to accept/reject certain grey-area contributions into the stable branches of currently supported releases

BTW, there's no reason that OTC couldn't come up with a suggested policy for the OMC's consideration if someone wants to create a strawman proposal.

@mattcaswell given you put the OTC hold, will you call the OTC vote on this? Should we wait until after beta1 release?

I can call a vote on that if its helpful. The current policy is mentioned on the release strategy page as follows:

"Letter releases, such as 1.0.2a, exclusively contain bug and security fixes and no new features"

Suggested wording for the vote:

"The performance improvements provided in PR11188 should be considered a bug fix and therefore acceptable for backport to 1.1.1"

@mattcaswell
Copy link
Member

I have started the above mentioned vote and will report back here when we have a result.

@xnox
Copy link
Contributor Author

xnox commented Sep 14, 2020

I have started the above mentioned vote and will report back here when we have a result.

There has only one vote so far. Is there a way to reping voters?

@mattcaswell
Copy link
Member

There has only one vote so far. Is there a way to reping voters?

Votes automatically close after 2 weeks unless explicitly closed earlier. This actually closed last week but I forgot to report it back.

As a reminder the vote text was:

"topic: The performance improvements provided in PR11188 should be considered a bug fix and therefore acceptable for backport to 1.1.1"

The vote was not accepted:

for: 0, against: 8, abstained: 3, not voted: 0

So, the OTC considers this PR to not be in conformance with current policy. The only way forward would be to change the policy, or to gain an explicit exemption for this PR (or abandon the PR).

@romen
Copy link
Member

romen commented Sep 14, 2020

Do we need to call an OTC vote to recommend OMC to consider an explicit exemption? Or is someone from the OMC willing to start a vote to decide on such an exemption?

@mattcaswell
Copy link
Member

Do we need to call an OTC vote to recommend OMC to consider an explicit exemption?

IMO, I think such a vote is unlikely to pass - although there's nothing stopping you raising it if you want to go down that path.

I think a better way is to clarify and extend the existing policy. I suggest that someone creates a strawman proposal (preferably as a web PR). I'm undecided whether it is best to get OTC to vote on it first before passing it to OMC, or just go straight to OMC. Ultimately it will be an OMC decision I think.

@t8m
Copy link
Member

t8m commented Sep 14, 2020

I think the policy amendment should be OMC vote straight out. The OTC members can voice their concerns or support on the to-be-proposed PR.

@t8m
Copy link
Member

t8m commented Sep 14, 2020

It always seems to me to be unnecessary bureaucracy to have OTC vote to recommend something to OMC to do a vote about the matter.

@xnox
Copy link
Contributor Author

xnox commented Sep 15, 2020

@mattcaswell

Thank you for completing the vote!

In light of rejecting this PR, to reduce maintenance burden in Ubuntu, 20.04 LTS and 20.10 will thus remain on 1.1.1f point release plus cherry-picks. As I do not wish to maintain rebases of this code, together with all our other patches against multiple point releases, without these patches getting merged upstream to stable series.

The next major/minor bump of OpenSSL in Ubuntu will therefore be v3.0.0 once that is released as stable upstream, as that will be the first upstream release that will contain this PR, among other cherrypicks we already carry in 1.1.1f.

@p-steuer
Copy link
Member

Thanks for completing the vote.

I still hope we can come up with a better solution regarding HW enablement patches and LTS releases in the future.

@openssl-machine
Copy link
Collaborator

This PR is in a state where it requires action by @openssl/otc but the last update was 30 days ago

@mspncp
Copy link
Contributor

mspncp commented Oct 17, 2020

I still hope we can come up with a better solution regarding HW enablement patches and LTS releases in the future.

@p-steuer the @openssl/otc has discussed the possibility of having LTS+ releases accompanying LTS releases in the future, where such enhancements could find a place to go. But AFAIK the details have not been worked out yet. Also, since it would be a policy change, it would require an OMC discussion and vote.

@openssl-machine
Copy link
Collaborator

This PR is in a state where it requires action by @openssl/otc but the last update was 30 days ago

@xnox xnox closed this Nov 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch: 1.1.1 Merge to OpenSSL_1_1_1-stable branch hold: need otc decision The OTC needs to make a decision
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants