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.

Sign up for GitHub

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

Jump to bottom

openssl_legacy: init and use #202126

Merged
merged 5 commits into from
Nov 26, 2022
Merged

openssl_legacy: init and use #202126

merged 5 commits into from
Nov 26, 2022

Conversation

ajs124
Copy link
Member

@ajs124 ajs124 commented Nov 21, 2022

Description of changes

Follow-up to #196906

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 22.11 Release Notes (or backporting 22.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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@ajs124 ajs124 requested review from mweinelt and vcunat November 21, 2022 02:06
@ofborg ofborg bot added 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild 8.has: package (new) This PR adds a new package labels Nov 21, 2022
@vcunat
Copy link
Member

vcunat commented Nov 21, 2022

I don't think we can catch this amount of rebuilds before branch-off, unless @mweinelt is OK with delaying it by a few days. (*-darwin stdenvs get rebuilt) Of course, that still doesn't prevent it from getting staged for both 21.11 and unstable.

@vcunat
Copy link
Member

vcunat commented Nov 21, 2022 •
edited
Loading

python310Packages.ntlm-auth builds with this, at least. (on x86_64-linux)

@vcunat vcunat mentioned this pull request Nov 21, 2022
Merged
13 tasks
@mweinelt
Copy link
Member

I think at this point we should go with staging-22.11.

@ajs124
Copy link
Member Author

ajs124 commented Nov 21, 2022

Alright, so I'll retarget this against staging and we backport it into staging-22.11, once that's created?

@ajs124
openssl_legacy: init
1996190 
openssl_3, but with a openssl.cnf that enables legacy ciphers
this way we can migrate away from openssl_1_1, while not breaking
applications relying on deprecated stuff
@ajs124
Revert "Revert "python3: pin to openssl_1_1""
f06f950 
This reverts commit 017fd89.
@ajs124
python3: use openssl_legacy
2b3c729
@ajs124
Revert "openldap: disable failing test"
578b6d3 
This reverts commit 66e6f6f.
@ajs124
openldap: use openssl_legacy
ff4dcc5
@ajs124 ajs124 force-pushed the init/openssl_legacy branch from aa36b1b to ff4dcc5 Compare November 21, 2022 12:46
@ajs124 ajs124 changed the base branch from staging-next to staging November 21, 2022 12:46
@ajs124 ajs124 marked this pull request as ready for review November 21, 2022 12:46
@ajs124 ajs124 mentioned this pull request Nov 21, 2022
Merged
13 tasks
@dasJ dasJ removed their request for review November 21, 2022 12:50
@ofborg ofborg bot added 10.rebuild-linux-stdenv This PR causes stdenv to rebuild and removed 10.rebuild-linux-stdenv This PR causes stdenv to rebuild labels Nov 21, 2022
@ofborg ofborg bot requested a review from dasJ November 21, 2022 13:37
SuperSandro2000
SuperSandro2000 reviewed Nov 21, 2022
View reviewed changes
pkgs/development/libraries/openssl/default.nix
Comment on lines +197 to 199

${lib.optionalString (conf != null) "cat ${conf} > $etc/etc/ssl/openssl.cnf"}
'';
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
${lib.optionalString (conf != null) "cat ${conf} > $etc/etc/ssl/openssl.cnf"}
'';
'' + lib.optionalString (conf != null) ''
cat ${conf} > $etc/etc/ssl/openssl.cnf
'';

Normally not a big fan of this but shouldn't this cut down the amount of rebuilds?

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably, but I'd guess not significantly, since this PR changes openldap and python3.

@mweinelt mweinelt merged commit 53d777c into NixOS:staging Nov 26, 2022
@ajs124 ajs124 deleted the init/openssl_legacy branch November 26, 2022 22:52
@github-actions github-actions bot mentioned this pull request Nov 26, 2022
Merged
1 task
@github-actions
Copy link
Contributor

Successfully created backport PR #203138 for staging-22.11.

@vcunat
Copy link
Member

vcunat commented Nov 30, 2022

This doesn't seem to work well on darwin, but I don't know why:
https://hydra.nixos.org/eval/1786377?filter=openldap#tabs-now-fail

@SuperSandro2000
Copy link
Member

It doesn't like rc4

additional info: SASL(-1): generic failure: internal error: failed to init cipher 'rc4'

@vcunat
Copy link
Member

vcunat commented Nov 30, 2022 •
edited
Loading

Yes, I thought the need for RC4 was one of the reasons to try keeping older openssl in there. (and now a new reconfigured one)

@ajs124
Copy link
Member Author

ajs124 commented Nov 30, 2022

Hm. It works just fine on linux though. Maybe the openssl config file doesn't work the same way on darwin?

@vcunat
Copy link
Member

vcunat commented Dec 4, 2022

We'll need some way forward. This breaks really many builds (roughly 1000*2).

I could simply put the rm line back as optionalString stdenv.isDarwin, but perhaps it would be better if someone checked that we're not leaving some big problem in there.

@tjni
Copy link
Contributor

tjni commented Dec 5, 2022

I don't know why exactly, but #204554 seems to fix the issue.

@vcunat
Copy link
Member

vcunat commented Dec 5, 2022 •
edited
Loading

I'm not familiar with library loading on darwin.

But I believe that even even on Linux+glibc these library overrides are a bit difficult. I'm sure libldap.so is typically not used stand-alone, and some binaries using it probably use openssl already (not in _legacy variant). In those cases it's not clear which version is loaded first and thus wins. What I'm certain of is that when you run an executable (without using extra loading magic), a particular SONAME is satisfied just once, including in all libraries – and thus each symbol name is consistent across the whole executable.

This mechanism can also cause problems when impure loading happens, e.g. in particular with /run/opengl-driver.

EDIT: I don't know openldap well; maybe it will almost always get used as the executables (and thus have separate library loading) and not as libraries.

@ajs124
Copy link
Member Author

ajs124 commented Dec 5, 2022

Usage of openldap as an executable or library are both common

@vcunat
Copy link
Member

vcunat commented Dec 10, 2022

This is in nixos-unstable now, so I assume we're still backporting to 22.11 even though it's long past release?

@ajs124 ajs124 mentioned this pull request Aug 8, 2023
Closed
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

@mweinelt mweinelt Awaiting requested review from mweinelt

@vcunat vcunat Awaiting requested review from vcunat

@FRidh FRidh Awaiting requested review from FRidh

@edolstra edolstra Awaiting requested review from edolstra

@np np Awaiting requested review from np

@adevress adevress Awaiting requested review from adevress

@dasJ dasJ Awaiting requested review from dasJ

Assignees
No one assigned
Labels
6.topic: python 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ajs124 @vcunat @mweinelt @SuperSandro2000 @tjni