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

openssl: update to version 1.1.1h #4155

Merged
merged 19 commits into from
Oct 11, 2020
Merged

Conversation

th0ma7
Copy link
Contributor

@th0ma7 th0ma7 commented Sep 1, 2020

Motivation: Our version of OpenSSL is just way to old and needs to be updated. Suggesting to either update at same API level for now (e.g. 1.0.2) or to move to next version of openssl (e.g. 1.1.1g) and keep previous version for legacy purposes.
Linked issues: #3629, #3560 and possibly #3666, closes #4185

Checklist

  • Build rule all-supported completed successfully

Package updates

  • boxbackup-client 0.12-2
  • fossil-scm 2.12.1-2
  • git 2.28.0-16
  • haproxy 1.8.26-21
  • links 2.21-2
  • synocli-file 1.2-4 (update midnight commander to 4.8.25)

Packages with new revision due to openssl update

  • borgbackup
  • deluge
  • domoticz
  • homeassistant
  • icecast
  • irssi
  • itools
  • lidarr
  • mosquitto (ppc853x-5.2 is not supported anymore)
  • mtproxy
  • mutt
  • ntopng
  • nzbdrone (sonarr)
  • python3
  • python
  • radarr
  • synocli-disk
  • synocli-monitor
  • synocli-net
  • transmission
  • tvheadend
  • umurmur
  • znc

Dependent packages not updated due to WIP or failing build

  • ejabberd
  • ffsync
  • monit
  • rutorrent
  • shairport-sync

Packages depending on python (2 or 3) - need only python update

  • duplicity
  • mercurial
  • octoprint
  • rdiff-backup
  • sabnzbd
  • vim

@th0ma7
Copy link
Contributor Author

th0ma7 commented Sep 1, 2020

By the look of things only packages known to fail already on master have failed from the github-action. As such looking good for a merge unless I'm missing something.

@ymartin59
Copy link
Contributor

As far as I remember, it is not so obvious because all most all applications depends on openssl and some (old) libraries have not received updates to support recent openssl api. May you please confirm @m4tt075 @hgy59

@th0ma7
Copy link
Contributor Author

th0ma7 commented Sep 3, 2020

Its the reason why I've kept it to 1.0.2 API for now.
github-action output looked real good besides packages known to fail already.

@Safihre
Copy link
Contributor

Safihre commented Sep 4, 2020

@ymartin59 as long as we stick with the 1.0.2 branch even the old ones should compile. Indeed the OpenSSL 1.1.0 is much different.
I imagine we will need 2 OpenSSL packages, since many up to date applications benefit from the stuff in OpenSSL 1.1.

@th0ma7
Copy link
Contributor Author

th0ma7 commented Sep 5, 2020

Thnx @Safihre it's what I thought as well.

I imagine we will need 2 OpenSSL packages, since many up to date applications benefit from the stuff in OpenSSL 1.1.

An idea could be, with the upcoming arrival of DSM-7 (if ever), move current openssl 1.0.2 as openssl-legacy and create a new default openssl using latest version available (e.g. 2.x). Any application that do not build correctly gets adjusted to use openssl-legacy, and all that works continue using openssl. That could even be done before then I guess but could be a huge undertaking as github-actions won't provide in-depth build-testing on every platforms.

@publicarray
Copy link
Member

publicarray commented Sep 13, 2020

FYI There is a new 🦝 "Raccoon" Attack (CVE-2020-1968) with a Severity: Low

https://www.openssl.org/news/secadv/20200909.txt

https://www.openssl.org/news/vulnerabilities.html

@th0ma7
Copy link
Contributor Author

th0ma7 commented Sep 13, 2020

FYI There is a new raccoon "Raccoon" Attack (CVE-2020-1968) with a Severity: Low

Interesting although fixed in version 1.0.2w which is only available with premium support. Alghough this might lead into making version 1.0.2v publicly available.

OpenSSL 1.0.2f and above will only reuse a DH secret if a "static" DH
ciphersuite is used. These static "DH" ciphersuites are ones that start with the
text "DH-" (for example "DH-RSA-AES256-SHA"). The standard IANA names for these
ciphersuites all start with "TLS_DH_" but excludes those that start with
"TLS_DH_anon_".

@th0ma7
Copy link
Contributor Author

th0ma7 commented Sep 14, 2020

BTW, with this update we address the following low security issues:

Anyone see blockers to this or does it sound reasonable?

@publicarray
Copy link
Member

publicarray commented Sep 15, 2020

I quickly looked at the build logs and I'm a bit confused why the synocli-file package failed, It builds ok on master. I can spend some time looking at it once my power is back on ⚡ ...

@th0ma7
Copy link
Contributor Author

th0ma7 commented Sep 15, 2020

I quickly looked at the build logs and I'm a bit confused why the synocli-file package failed, It builds ok on master. I can spend some time looking at it once my power is back on zap ...

May well be due to not being rebased against master where a fix might already be?

@th0ma7
Copy link
Contributor Author

th0ma7 commented Sep 17, 2020

WARNING: This is a test for considerations, easy to roll-back to previous commit.

I wanted to have both a) an overview of the impact of migrating to openssl 1.1.1 branch and b) check the feasibility of telling openssl to use /etc/ssl by default for it's configurations, thus /etc/ssl/certs for its certificates.

As such, I've moved existing openssl to openssl-legacy and upgraded default openssl to 1.1.1 branch. This allow pointing incompatible application to older version of openssl.

Thoughts on this welcomed (while it gives it a try at compiling the entire stack) :-)

@Safihre
Copy link
Contributor

Safihre commented Sep 18, 2020

Unless you want to manually try every single package (just making it compile isn't enough), I would suggest the opposite approach: create a openssl-11 package that maintainers of the SPK packages can include when they do an update. Makes it lot less of a big bang approach.

@publicarray
Copy link
Member

publicarray commented Sep 18, 2020

I agree with @Safihre (mostly because that is how other package managers have handled the same condition) but I also do see the value of getting a one time report on what compiles with the new API. Which I think was your intent here anyway

@th0ma7
Copy link
Contributor Author

th0ma7 commented Sep 18, 2020

Thnx @Safihre and @publicarray for your feedbacks. The good news is that the build output difference seems to be around a handful of packages (fossil-scm, haproxy, links, memcached, monit, python, tmux, transmission).

There are two aspects to this proposal if we're intersted at moving this further (let me know if I've missed something obvious):

1) Method:
a) Enforce usage of the new openssl with expectation that new builds will be fixed/tested by package maintainers and move back to older version of openssl as needed.
b) Keep the old, unsupported and insecure openssl as default and expect package maintainers to gradually migrate to the new version.

2) Naming:
a) What ever the default version is, keep it as cross/openssl
b) Use version schema: cross/openssl-1.0 and cross/openssl-1.1
c) Use default naming and -legacy

Personally I would use a conjunction of 1a) + mix of 2a) + 2b) where default is always openssl and other releases gets tagged with a version (e.g. cross/openssl-1.0).

@ymartin59
Copy link
Contributor

@th0ma7 I agree with the method. My preference is 1a) with cross/openssl for latest and keep cross/openssl-1.0.

@th0ma7
Copy link
Contributor Author

th0ma7 commented Sep 20, 2020

I've renamed openssl-legacy as openssl-1.0 and kept latest version as openssl + rebased to master.

By the look of things, and please let me know if I'm mistaking, looks like there is interest to this from multiple people.

@ymartin59, @Safihre, @publicarray, @BenjV, @hgy59 : Would there be someone able to try it out with one or a few package(s) that heavily depends on openssl to see how it behave with both version 1.1 and using default system-wide /etc/ssl ?

@BenjV
Copy link

BenjV commented Sep 20, 2020

If someone can build a python version with Openssl 1.1.1 for Armada38X I wil be happy to test it on my test Nas (DS116)

@th0ma7
Copy link
Contributor Author

th0ma7 commented Sep 21, 2020

@BenjV I was able to build a python3 package set that I made available at:

python2 refuses to build. After investigating it seems M2Crypto requires to be updated to a newer version along with passing --openssl=$(STAGING_INSTALL_PREFIX) option at build time or something close to that. I guess other synocommunity developpoers more into python may have a better understanding on where the issue really is.

@ymartin59
Copy link
Contributor

@th0ma7 Thanks for Python 3 testing. I really think we should not bother with Python 2.7 any longer.

@ymartin59 ymartin59 changed the title openssl: Bump to version 1.0.2u openssl: update to version 1.0.2u and 1.1.1g Sep 21, 2020
@ymartin59 ymartin59 changed the title openssl: update to version 1.0.2u and 1.1.1g openssl: update to versions 1.0.2u and 1.1.1g Sep 21, 2020
@Safihre
Copy link
Contributor

Safihre commented Sep 21, 2020

Agreed, Python 2.7 is end-of-life and I can tell from experience that it doesn't handle OpenSSL 1.1.1 well.
@th0ma7 So I would suggest to keep Python 2.7 on the openssl-1.0 package.

@hgy59 hgy59 mentioned this pull request Oct 10, 2020
3 tasks
@th0ma7
Copy link
Contributor Author

th0ma7 commented Oct 10, 2020

@hgy59 @th0ma7 OK so my fears are confirmed... I will have to re-publish in a bunch a large number of packages... the same as previous python 2.7.18 update. I hope not to miss one in the check list...

I've created an issue for your consideration @hgy59 and @ymartin59 in order to track publishing of all updated packages at #4211 . The intent is to be able to share the load for publishing and ensure we track of things as we move along.

Is this PR good for merge? I think both myself and @hgy59 tends to agree on that (again, awsome work!). But considering the scale of the PR I want to make sure you are OK with that @ymartin59 before merging.

@hgy59 hgy59 mentioned this pull request Oct 10, 2020
3 tasks
th0ma7 and others added 16 commits October 11, 2020 00:54
- add patch to fix openssl path in m2crypto
- update htop to 3.0.2 (older versions are not available)
- include htop in synocli-monitor for all DSM versions
- remove REQUIRED_DSM in sonarr/Makefile for armv7-1.2 (rely on dependency of mono)
- cleanup umurmur/Makefile
- fix c-ares/Makefile
- downgrade slang to 2.2.4
- disable parallel make
- add .NOTPARALLEL target to disable parallel make
Copy link
Contributor

@ymartin59 ymartin59 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Large amount of work. Looks good to me.
I still propose some minor improvements.

cross/boxbackup/Makefile Outdated Show resolved Hide resolved
Comment on lines +48 to 50
.PHONY: haproxy_install
haproxy_install:
$(RUN) $(MAKE) install DESTDIR=$(INSTALL_DIR) PREFIX=$(INSTALL_PREFIX)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks look like standard default "make install" target. May you please confirm by discarding its replacement?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ymartin59 disagree, the small difference is prefix vs PREFIX

cross/haproxy/Makefile Outdated Show resolved Hide resolved
cross/links/Makefile Outdated Show resolved Hide resolved
cross/openssl/Makefile Outdated Show resolved Hide resolved
@@ -9,7 +9,7 @@ MAINTAINER = SynoCommunity
DESCRIPTION = Links is a web browser running in text mode.
DISPLAY_NAME = Links
STARTABLE = no
BETA = 1
CHANGELOG = "1. Update links to 2.21.<br/>2. Update openssl to 1.1."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would make sense to move this tool in synocli-net

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ymartin59 agree, this should be done in #4195, I updated #4211 accordingly.

@hgy59
Copy link
Contributor

hgy59 commented Oct 11, 2020

rebased @ master

@hgy59
Copy link
Contributor

hgy59 commented Oct 11, 2020

Large amount of work.

@ymartin59 I could take some tasks (only today, afterwards I am off my development environment for two weeks).

My preferences:

  1. synocli-net (merge Update synocli net #4195, build, test and publish)
  2. synocli-file (build, test and publish)
  3. ntopng (build, test and publish)

- libwebsockets fails to build for ppc (except qoriq) since update to openssl 1.1.
@hgy59
Copy link
Contributor

hgy59 commented Oct 11, 2020

Unfortunately arch-ppc853x fails to build libwebsockets and dependent mosquitto since update to openssl 1.1.

@hgy59 hgy59 merged commit 26ad900 into SynoCommunity:master Oct 11, 2020
@th0ma7 th0ma7 deleted the openssl-102u branch October 12, 2020 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Python(2 and 3) build with Openssl version 1.1.1 (feature request)
7 participants