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

libressl: fixing nc for version>=2.9 #68387

Merged
merged 1 commit into from Sep 17, 2019

Conversation

d-goldin
Copy link
Contributor

@d-goldin d-goldin commented Sep 9, 2019

Motivation for this change

This addresses #68286

When -R (CA file location) is not specified, nc tries to fall back to
a default location. In 2.8 this was still configurable at compile time,
but was changed somewhere after. This replaces /etc/ssl/cert.pem
with $out/etc/ssl/cert.pem in the code directly.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @thoughtpolice
cc @fpletz

cc @andir

@thoughtpolice
Copy link
Member

I'm left wondering, what was the actual cause here? Why did this work before without any special handling on our part? Note the generic builder does not, AFAICS, set any particular special directory to install the certs into, so I'm guessing it defaults to whatever CMAKE_INSTALL_PREFIX is specified as.

@thoughtpolice
Copy link
Member

thoughtpolice commented Sep 10, 2019

libressl/openbsd@fae9241

libressl/portable#160 and libressl/portable@1988b8f and https://github.com/libressl-portable/portable/blob/master/apps/nc/CMakeLists.txt

These all provide some clues. It would be nice to put the puzzle pieces together and see if there's actually some other behavior to fix, here.

@thoughtpolice
Copy link
Member

Oh, I didn't read through #68286 as I should have, my apologies. I'll look over everything and review sometime later today if possible.

@d-goldin
Copy link
Contributor Author

I carried over the change from the other PR to use cacert. I committed it separately for clarity and can squash once reviewers agree.

@thoughtpolice
Copy link
Member

I think using cacert here is probably correct as outlined in #68456. Feel free to squash this, or I can squash it upon actually doing the merge.

/cc @fpletz any feedback would be appreciated, but otherwise I think this seems OK?

This addresses NixOS#68286

When `-R` (CA file location) is not specified, nc tries to fall back to
a default location. In 2.8 this was still configurable at compile time,
but was changed somewhere after. This replaces `/etc/ssl/cert.pem`
with `${cacert}/etc/ssl/cert.pem` in the code directly.

For a discussion of this, see NixOS#68456
@d-goldin
Copy link
Contributor Author

Squashed and adjusted commit message a little bit.

@ofborg ofborg bot requested a review from thoughtpolice September 17, 2019 21:21
@matthewbauer matthewbauer merged commit 8f2eb80 into NixOS:master Sep 17, 2019
@d-goldin d-goldin deleted the libressl_fix_nc branch September 17, 2019 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants