Skip to content

OpenSSL maintenance 2024/09#339614

Merged
emilazy merged 8 commits intoNixOS:stagingfrom
thillux:openssl-maintenance
Sep 16, 2024
Merged

OpenSSL maintenance 2024/09#339614
emilazy merged 8 commits intoNixOS:stagingfrom
thillux:openssl-maintenance

Conversation

@thillux
Copy link
Contributor

@thillux thillux commented Sep 4, 2024

Description of changes

This PR combines the usual OpenSSL version bump with some maintenance:

  • OpenSSL now releases only on Github -> switch to URLs on Github for all versions
  • Revisit if we can use the latest OpenSSL version as the default now (set it to openssl_3_3).
  • Bump all OpenSSL packages to the latest available version for CVE fixes.

Fixed CVEs:

  • Fixed possible denial of service in X.509 name checks. (CVE-2024-6119)
  • Fixed possible buffer overread in SSL_select_next_proto(). (CVE-2024-5535)

Changelog(s):
The changes are the same for all updated OpenSSL versions. See changelog.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@emilazy
Copy link
Member

emilazy commented Sep 4, 2024

Happy with the general approach here as discussed on Matrix, but I’m −1 on introducing 3.1 as yet another version, when we’ll already have 3.0, 3.2, and 3.3. Ideally we’d only have one version of OpenSSL; Arch, Alpine, and Void ship 3.3 and 1.1, and Fedora ships only 3.2.

I think that we should rename openssl_3 to openssl_3_0 if it’s no longer going to be the default, though that might be unnecessary churn if we can hopefully remove it after the 24.11 release.

@thillux
Copy link
Contributor Author

thillux commented Sep 4, 2024

Happy with the general approach here as discussed on Matrix, but I’m −1 on introducing 3.1 as yet another version, when we’ll already have 3.0, 3.2, and 3.3. Ideally we’d only have one version of OpenSSL; Arch, Alpine, and Void ship 3.3 and 1.1, and Fedora ships only 3.2.

I think that we should rename openssl_3 to openssl_3_0 if it’s no longer going to be the default, though that might be unnecessary churn if we can hopefully remove it after the 24.11 release.

Fair, reduce to 3_3, 3_0, 1_1 for 24.11?

@thillux
Copy link
Contributor Author

thillux commented Sep 4, 2024

I'm currently testing if some important packages like systemd still build with OpenSSL 3.3. Therefore some more patches are included. I needed to bump ibm-sw-tpm2 and use a better maintained swtpm for tests in tpm2-tss to work.

@thillux thillux force-pushed the openssl-maintenance branch from d2fc505 to 4c9d6a0 Compare September 4, 2024 22:09
@emilazy
Copy link
Member

emilazy commented Sep 4, 2024

Fair, reduce to 3_3, 3_0, 1_1 for 24.11?

Sounds good to me! I’d also be okay with leaving 3.2 in and just dropping the other versions in bulk after the release if everything goes well; whatever your preference is.

@ofborg ofborg bot added 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 8.has: package (new) This PR adds a new package labels Sep 4, 2024
@ofborg ofborg bot added 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Sep 4, 2024
@ofborg ofborg bot requested a review from tomfitzhenry September 4, 2024 22:33
@thillux thillux force-pushed the openssl-maintenance branch from 4c9d6a0 to 1fbb3ec Compare September 7, 2024 08:45
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation 8.has: changelog This PR adds or changes release notes labels Sep 7, 2024
@thillux thillux force-pushed the openssl-maintenance branch from bce5a62 to 1580298 Compare September 7, 2024 09:10
@thillux thillux changed the title WIP: OpenSSL maintenance 2024/09 OpenSSL maintenance 2024/09 Sep 7, 2024
@thillux
Copy link
Contributor Author

thillux commented Sep 7, 2024

After some issues in the past, where e.g. systemd failed to build with a recent OpenSSL version, I tried to fix these.

Older discussions about updating OpenSSL default:

@thillux thillux force-pushed the openssl-maintenance branch from 1580298 to f794674 Compare September 7, 2024 09:18
@thillux thillux marked this pull request as ready for review September 7, 2024 09:18
@thillux thillux requested review from emilazy and mweinelt September 7, 2024 09:18
Copy link
Member

@emilazy emilazy left a comment

Choose a reason for hiding this comment

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

LGTM; a few tweaks to the release note.

I’ll let a couple others take a look at this rather than unilaterally merging, since it’s a significant change :)

I look forward to dropping the other versions after branch‐off.

@thillux thillux marked this pull request as draft September 7, 2024 11:46
@thillux thillux force-pushed the openssl-maintenance branch from 4d591c9 to 29ff14c Compare September 7, 2024 11:55
@thillux thillux requested a review from emilazy September 7, 2024 11:56
@thillux thillux marked this pull request as ready for review September 7, 2024 11:58
Copy link
Member

@emilazy emilazy left a comment

Choose a reason for hiding this comment

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

LGTM. Will let someone else take a look too.

@wegank wegank added 12.approvals: 1 This PR was reviewed and approved by one person. 2.status: merge conflict This PR has merge conflicts with the target branch labels Sep 7, 2024
@reckenrode
Copy link
Contributor

OpenSSL 3.x is supposed to be API- and ABI-stable. If a package pins the version, that’s a problem with the package. If OpenSSL breaks API, that’s an upstream bug. I’m all for doing what every other package ecosystem seemingly does, which is ship the latest OpenSSL by default. Both MacPorts and Homebrew ship OpenSSL 3.3. Homebrew also offers OpenSSL 3.0 as a separate package in case something really needs it.

https://openssl-library.org/policies/technical/stable-release-updates/

@emilazy
Copy link
Member

emilazy commented Sep 15, 2024

This needs rebasing for conflicts, but I’m otherwise minded to merge.

OpenSSL used to provide their software downloads on openssl.org.
Now they use links to Github releases.

OpenSSL 1.1.1w is also available at Github, but with a small
difference in the URL scheme.

Signed-off-by: Markus Theil <theil.markus@gmail.com>
OpenSSL releases different versions in parallel (currently active are
3.0.x, 3.1.x, 3.2.x, 3.3.x). IMHO We should try to stay on the most recent
release line, as probably not all security relevant fixes are identified
as such upstream and get backported.

Signed-off-by: Markus Theil <theil.markus@gmail.com>
Contains two CVE fixes.

* Fixed possible denial of service in X.509 name checks. (CVE-2024-6119)
* Fixed possible buffer overread in SSL_select_next_proto(). (CVE-2024-5535)

Changelog: https://github.com/openssl/openssl/blob/openssl-3.3/CHANGES.md#changes-between-331-and-332-3-sep-2024

Signed-off-by: Markus Theil <theil.markus@gmail.com>
Contains two CVE fixes.

* Fixed possible denial of service in X.509 name checks. (CVE-2024-6119)
* Fixed possible buffer overread in SSL_select_next_proto(). (CVE-2024-5535)

Changelog: https://github.com/openssl/openssl/blob/openssl-3.0/CHANGES.md#changes-between-3014-and-3015-3-sep-2024

Signed-off-by: Markus Theil <theil.markus@gmail.com>
Contains two CVE fixes.

* Fixed possible denial of service in X.509 name checks. (CVE-2024-6119)
* Fixed possible buffer overread in SSL_select_next_proto(). (CVE-2024-5535)

Changelog: https://github.com/openssl/openssl/blob/openssl-3.2/CHANGES.md#changes-between-322-and-323-3-sep-2024

Signed-off-by: Markus Theil <theil.markus@gmail.com>
In order to fix build with more recent OpenSSL versions (3.2.x+)
use new Github upstream URL, which is more recent than the sourceforge
release.

Only a tag and no release is made, so use the unstable versioning
scheme.

Signed-off-by: Markus Theil <theil.markus@gmail.com>
Switch tpm2-tss to swtpm, which is more widely used than the IBM one.
tpm2-tss contains tests for both TPM 2.0 emulators.

This fixes a failing test, with the updated IBM sw tpm version.

Signed-off-by: Markus Theil <theil.markus@gmail.com>
The new OpenSSL default 3.3.x increased the default security level,
mention this in release notes.

Signed-off-by: Markus Theil <theil.markus@gmail.com>
@thillux thillux force-pushed the openssl-maintenance branch from 29ff14c to 5b19e71 Compare September 16, 2024 07:46
@thillux
Copy link
Contributor Author

thillux commented Sep 16, 2024

@emilazy: Rebased.

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Sep 16, 2024
@Aleksanaa Aleksanaa added the 1.severity: security Issues which raise a security issue, or PRs that fix one label Sep 16, 2024
@emilazy
Copy link
Member

emilazy commented Sep 16, 2024

After discussion on Matrix I think we have consensus to ignore the LTS releases and just package and track the latest stable releases going forward. Thank you for your work on this!

@emilazy emilazy merged commit 3fbe70f into NixOS:staging Sep 16, 2024
@trofi
Copy link
Contributor

trofi commented Sep 22, 2024

Bisect says 7932bf5 openssl: set default to openssl_3_3 broke mumble in staging as:

$ nix build --no-link github:NixOS/nixpkgs/staging#mumble -L
...
mumble> [ 97%] Built target mumble_autogen
mumble> make[2]: *** No rule to make target '/nix/store/kdazpap3bnl355f3nv7p01dpwad8c90w-openssl-3.3.2-dev/lib/libcrypto.so', needed by 'mumble'.  Stop.
mumble> make[2]: *** Waiting for unfinished jobs....

@thillux
Copy link
Contributor Author

thillux commented Sep 23, 2024

@trofi I looked into this, but have not found the root cause yet. In this error message, somehow the dev-Output got used to provide the lib folder. While it technically contains a lib folder, only the pkg-config and cmake-Files are there. The "useful" lib folder is in the out-Output. I don't see that anything changed between 3.0.x and 3.3.x in the output structure. OpenSSL 3.0.x also places a lib folder in both the out and dev derivation.

@thillux
Copy link
Contributor Author

thillux commented Sep 23, 2024

Something like this will probably fix it (builds still in progress):

diff --git a/pkgs/development/libraries/openssl/default.nix b/pkgs/development/libraries/openssl/default.nix
index 0313841dce30..951505d1fd08 100644
--- a/pkgs/development/libraries/openssl/default.nix
+++ b/pkgs/development/libraries/openssl/default.nix
@@ -233,6 +233,10 @@ let
         echo "Found an erroneous dependency on perl ^^^" >&2
         exit 1
       fi
+    '' + lib.optionalString (lib.versionAtLeast version "3.3.0") ''
+      # cleanup cmake helpers for now (for OpenSSL >= 3.3), only rely on pkg-config.
+      # pkg-config gets its paths fixed correctly
+      rm -r $dev/lib/cmake
     '';
 
     passthru.tests.pkg-config = testers.testMetaPkgConfig finalAttrs.finalPackage;

When I have more time, I'll may also patch the CMake-Scripts and don't delete them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1.severity: security Issues which raise a security issue, or PRs that fix one 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants