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

gnupatch: Don't fetch from cgit URLs with unstable hashes #78799

Merged
merged 1 commit into from Jan 31, 2020

Conversation

chkno
Copy link
Member

@chkno chkno commented Jan 30, 2020

cgit cannot serve patches with stable hashes, so store these patches
in-tree. cgit community discussion about this problem:
https://lists.zx2c4.com/pipermail/cgit/2017-February/003470.html

Verification that the only difference between the live page, the
patch committed here, and the version cached under the old hash at
tarballs.nixos.org is the cgit version footer:

$ curl -s -L http://tarballs.nixos.org/sha256/"$(nix-hash --type sha256 --to-base16 0iw0lk0yhnhvfjzal48ij6zdr92mgb84jq7fwryy1hdhi47hhq64)" > Allow_input_files_to_be_missing_for_ed-style_patches.patch
$ diff -U0 --label cgit-live <( curl -s -L https://git.savannah.gnu.org/cgit/patch.git/patch/?id=b5a91a01e5d0897facdd0f49d64b76b0f02b43e1 ) Allow_input_files_to_be_missing_for_ed-style_patches.patch
--- cgit-live
+++ Allow_input_files_to_be_missing_for_ed-style_patches.patch  2020-01-29 17:22:00.077312937 -0800
@@ -32 +32 @@
-cgit v1.2.1
+cgit v1.0-41-gc330

$ curl -s -L http://tarballs.nixos.org/sha256/"$(nix-hash --type sha256 --to-base16 1bpy16n3hm5nv9xkrn6c4wglzsdzj3ss1biq16w9kfv48p4hx2vg)" > CVE-2018-1000156.patch
$ diff -U0 --label cgit-live <( curl -s -L https://git.savannah.gnu.org/cgit/patch.git/patch/?id=123eaff0d5d1aebe128295959435b9ca5909c26d ) CVE-2018-1000156.patch
--- cgit-live
+++ CVE-2018-1000156.patch      2020-01-29 17:23:41.021116969 -0800
@@ -210 +210 @@
-cgit v1.2.1
+cgit v1.0-41-gc330
Motivation for this change

Fix "hash mismatch" errors.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-review wip"
  • [i] 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.

@worldofpeace
Copy link
Contributor

fetchpatch's code comments mention cgit

# often change with updating of git or cgit.
.
This leads me to believe that if we used fetchpatch it normalizes the patch effectively mitigating this issue.

@chkno
Copy link
Member Author

chkno commented Jan 30, 2020

I tried using fetchpatch in chkno@b25d448 , but it didn't work:

$ nix-build . -A patch
error: anonymous function at /home/username/devel/nixpkgs/pkgs/build-support/fetchurl/boot.nix:5:1
called with unexpected argument 'meta', at /home/username/devel/nixpkgs/pkgs/build-support/fetchpatch/default.nix:14:1

#61471 (comment) suggests that this error indicates an attempt to use fetchpatch in one of fetchpatch's dependencies, and that fetchpatch's dependencies should just not use fetchpatch.

I updated the commit message with a note about this.

Thanks!

cgit cannot serve patches with stable hashes, so store these patches
in-tree.  cgit community discussion about this problem:
https://lists.zx2c4.com/pipermail/cgit/2017-February/003470.html

We pull the patches in-tree rather than strip cgit footers with fetchpatch
because per NixOS#61471 (comment)
dependencies of fetchpatch cannot use fetchpatch.

Verification that the only difference between the live page, the
patch committed here, and the version cached under the old hash at
tarballs.nixos.org is the cgit version footer:

$ curl -s -L http://tarballs.nixos.org/sha256/"$(nix-hash --type sha256 --to-base16 0iw0lk0yhnhvfjzal48ij6zdr92mgb84jq7fwryy1hdhi47hhq64)" > Allow_input_files_to_be_missing_for_ed-style_patches.patch
$ diff -U0 --label cgit-live <( curl -s -L https://git.savannah.gnu.org/cgit/patch.git/patch/?id=b5a91a01e5d0897facdd0f49d64b76b0f02b43e1 ) Allow_input_files_to_be_missing_for_ed-style_patches.patch
--- cgit-live
+++ Allow_input_files_to_be_missing_for_ed-style_patches.patch  2020-01-29 17:22:00.077312937 -0800
@@ -32 +32 @@
-cgit v1.2.1
+cgit v1.0-41-gc330

$ curl -s -L http://tarballs.nixos.org/sha256/"$(nix-hash --type sha256 --to-base16 1bpy16n3hm5nv9xkrn6c4wglzsdzj3ss1biq16w9kfv48p4hx2vg)" > CVE-2018-1000156.patch
$ diff -U0 --label cgit-live <( curl -s -L https://git.savannah.gnu.org/cgit/patch.git/patch/?id=123eaff0d5d1aebe128295959435b9ca5909c26d ) CVE-2018-1000156.patch
--- cgit-live
+++ CVE-2018-1000156.patch      2020-01-29 17:23:41.021116969 -0800
@@ -210 +210 @@
-cgit v1.2.1
+cgit v1.0-41-gc330
@chkno
Copy link
Member Author

chkno commented Jan 30, 2020

I see that this causes many rebuilds. Switching the target branch from master to staging...

@chkno chkno changed the base branch from master to staging January 30, 2020 22:25
@worldofpeace
Copy link
Contributor

@GrahamcOfBorg build gnupatch

@worldofpeace worldofpeace merged commit 356e228 into NixOS:staging Jan 31, 2020
@chkno chkno deleted the gnupatch-patches-in-tree branch February 1, 2020 01:33
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

2 participants