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

curl: 8.0.1 -> 8.1.1 #232531

Merged
merged 1 commit into from May 25, 2023
Merged

curl: 8.0.1 -> 8.1.1 #232531

merged 1 commit into from May 25, 2023

Conversation

mweinelt
Copy link
Member

@mweinelt mweinelt commented May 18, 2023

https://daniel.haxx.se/blog/2023/05/17/curl-8-1-0-http2-over-proxy/
https://curl.se/changes.html#8_1_0
https://curl.se/changes.html#8_1_1

https://www.openwall.com/lists/oss-security/2023/05/17/1
https://www.openwall.com/lists/oss-security/2023/05/17/2
https://www.openwall.com/lists/oss-security/2023/05/17/3
https://www.openwall.com/lists/oss-security/2023/05/17/4

Fixes: CVE-2023-28319, CVE-2023-28320, CVE-2023-28321, CVE-2023-28322

Description of changes
Things done
  • Built on platform(s) (only for 8.1.0)
    • 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/)
  • 23.05 Release Notes (or backporting 22.11 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.

Copy link
Member

@tomodachi94 tomodachi94 left a comment

Choose a reason for hiding this comment

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

Lgtm. Thank you for contributing to Nixpkgs!

@mweinelt
Copy link
Member Author

https://curl.se/mail/lib-2023-05/0029.html

Drafting.

@mweinelt mweinelt marked this pull request as draft May 18, 2023 10:27
@ivan
Copy link
Member

ivan commented May 23, 2023

curl 8.1.1 is available now: https://curl.se/mail/lib-2023-05/0040.html

@mweinelt mweinelt changed the title curl: 8.0.1 -> 8.1.0 curl: 8.0.1 -> 8.1.1 May 23, 2023
@mweinelt mweinelt marked this pull request as ready for review May 23, 2023 19:44
@risicle
Copy link
Contributor

risicle commented May 24, 2023

Assume we'll be backporting this to 23.05?

@mweinelt
Copy link
Member Author

The question was just whether we go with 8.1.1 or pick the patches like for 22.11.

@risicle
Copy link
Contributor

risicle commented May 25, 2023

Built curl.tests successfully on macos 10.15.

@vcunat vcunat merged commit 5d8c27d into NixOS:staging May 25, 2023
21 checks passed
@github-actions
Copy link
Contributor

Successfully created backport PR for staging-23.05:

@mweinelt mweinelt deleted the curl-8.1.0 branch May 25, 2023 07:09
@vcunat
Copy link
Member

vcunat commented May 28, 2023

This broke build of curlHTTP3. Most likely it will need update of ngtcp2 and/or nghttp3.

EDIT: details
https://github.com/curl/curl/blob/curl-8_1_1/docs/HTTP3.md#ngtcp2-version

Hydra only has darwin logs for now, but I confirmed on linux locally:
https://hydra.nixos.org/build/221673473/nixlog/2/tail

@vcunat
Copy link
Member

vcunat commented May 28, 2023

I think this part is maintained primarily by @Izorkin?

Fixes would primarily target staging-next now: PR #233944

@vcunat
Copy link
Member

vcunat commented May 28, 2023

ngtcp2 certainly does suffer from incompatible changes coming relatively often (on most of minor-number bumps IIRC). That's why I separated ngtcp2-gnutls which is only directly used by knot-dns and thus can be updated in lock-step.

Maybe we should start keeping ngtcp2 + nghttp3 on the versions needed by our current curl? (fortunately, curlHTTP3 seems to be the only consumer right now)

@Izorkin
Copy link
Contributor

Izorkin commented May 28, 2023

Upstream curl wrote that an update of nghttp2 to 1.53.0 is required. The PR to update ngtcp2 is there too, only to the master branch.
If needed, I can put all the updates into 1 PR and throw it into the staging branch.

@vcunat
Copy link
Member

vcunat commented May 28, 2023

I don't see any problems related to nghttp2. (that's on in the default curl)

@Izorkin
Copy link
Contributor

Izorkin commented May 28, 2023

I don't see any problems related to nghttp2. (that's on in the default curl)

curl/curl#11031 (comment)

@risicle
Copy link
Contributor

risicle commented May 28, 2023

Which of the passthru.tests would have exhibited this problem? Or did we fail to test curlFull?

@vcunat
Copy link
Member

vcunat commented May 28, 2023

curlFull apparently does not enable HTTP/3

@Izorkin: I thought you referred to nixpkgs master, not curl master. I see them writing that nghttp2 1.52 is bad for curl, but we don't use that (yet). I don't see hint that our current version is bad. (update to 1.53 could be done, but it's a huge rebuild)

EDIT: curl*.tests.nginx-http3 seems to (always) depend on curlHTTP3, but I image any nixos test will be very expensive in case you rebuild curl...

@vcunat vcunat mentioned this pull request May 28, 2023
12 tasks
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

6 participants