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

perl: verify_SSL=>1 by default in HTTP::Tiny #187480

Merged
merged 1 commit into from
Aug 20, 2022

Conversation

stigtsp
Copy link
Member

@stigtsp stigtsp commented Aug 19, 2022

Description of changes

Perl's core HTTP::Tiny module does not verify TLS/SSL connections by default. This PR applies a proposed patch from Debian by @jmdh to fix that.

Upstream has decided against changing the default due to backwards compatibility concerns.

So this is an opinionated patch which would break for users of HTTP::Tiny that rely on the insecure default.

Example:

$ perl -MIO::Socket::SSL -MHTTP::Tiny -E 'my $res = HTTP::Tiny->new->get(q(https://wrong.host.badssl.com/)); say qq($res->{status} $res->{reason})'
200 OK
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.

@stigtsp stigtsp force-pushed the perl/http-tiny-verify-ssl-default branch from 4078f27 to 5b70927 Compare August 19, 2022 17:58
@stigtsp
Copy link
Member Author

stigtsp commented Aug 19, 2022

@GrahamcOfBorg build perl

@stigtsp stigtsp marked this pull request as ready for review August 19, 2022 18:18
@stigtsp stigtsp requested a review from zakame as a code owner August 19, 2022 18:18
@stigtsp stigtsp requested review from dasJ and mweinelt August 19, 2022 18:18
Copy link
Member

@zakame zakame left a comment

Choose a reason for hiding this comment

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

💯

@bobby285271 bobby285271 added the 12.approvals: 3+ This PR was reviewed and approved by three or more reputable people label Aug 20, 2022
@dasJ dasJ merged commit 6b36021 into NixOS:staging Aug 20, 2022
@stigtsp
Copy link
Member Author

stigtsp commented Aug 20, 2022

Some thoughts:

It's likely that this will break stuff, like tests that connect to TLS servers with self signed certificates. I'll keep an eye on Hydra for this when it reaches staging-next.

But, breakage from modules/tools that connect to (invalid) HTTPS resources on the internet will be harder to catch since our tests are network isolated. But breaking insecure HTTPS usage would be a good thing, imho.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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+ 10.rebuild-linux-stdenv This PR causes stdenv to rebuild 12.approvals: 3+ This PR was reviewed and approved by three or more reputable people
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants