guix: Add guix-{attest,verify} scripts

dongcarl commented at 8:22 pm on March 17, 2021: member

Adds replacements for gsign and gverify.

Personally I’m not a big fan of using the word “sign” as it’s been used to refer to both codesigning and GPG signing.

dongcarl added this to the "PRs" column in a project

MarcoFalke added this to the milestone 22.0 on Mar 17, 2021

DrahtBot commented at 8:46 pm on March 17, 2021: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

DrahtBot added the label Build system on Mar 17, 2021

DrahtBot added the label Docs on Mar 17, 2021

DrahtBot added the label Scripts and tools on Mar 17, 2021

MarcoFalke removed the label Docs on Mar 18, 2021

MarcoFalke removed the label Build system on Mar 18, 2021

MarcoFalke removed the label Scripts and tools on Mar 18, 2021

DrahtBot added the label Build system on Mar 18, 2021

DrahtBot added the label Scripts and tools on Mar 18, 2021

dongcarl moved this from the "PRs" to the "Next (Not based on any other PRs)" column in a project

dongcarl force-pushed on Apr 8, 2021

dongcarl force-pushed on Apr 9, 2021

dongcarl marked this as ready for review on Apr 9, 2021

dongcarl commented at 1:36 am on April 9, 2021: member

Perhaps we can test this by doing a trial run? I’ve PR’ed my attestations of 5da0e361b8b5a6d730a244e774c6810946619229 here: https://github.com/bitcoin-core/guix.sigs/pull/1

What I did:

Run guix-build
Run env GUIX_SIGS_REPO=<path/to/guix.sigs> SIGNER=0x96AB007F1A7ED999=dongcarl ./contrib/guix/guix-attest

Once you’ve run ./contrib/guix/guix-attest, you can verify against my signatures using:

0$ env GUIX_SIGS_REPO=<path/to/guix.sigs> ./contrib/guix/guix-verify

hebasto commented at 2:42 pm on April 9, 2021: member

Tested 5da0e361b8b5a6d730a244e774c6810946619229.

Result: https://github.com/bitcoin-core/guix.sigs/pull/2

Once you’ve run ./contrib/guix/guix-attest, you can verify against my signatures using:
0$ env GUIX_SIGS_REPO=<path/to/guix.sigs> ./contrib/guix/guix-verify

I think it is possible when https://github.com/bitcoin-core/guix.sigs/pull/1 is merged, no?

fanquake commented at 1:00 am on April 10, 2021: member

I think it is possible when bitcoin-core/guix.sigs#1 is merged, no?

The PR doesn’t have to be merged, you should be able to just fetch and checkout the branch you want to verify against.

fanquake commented at 3:10 am on April 10, 2021: member

I have done a trial run through the attestation / verify steps as well. https://github.com/bitcoin-core/guix.sigs/pull/3.

Note that for anyone attesting/verifying on macOS, the invocations of find and xargs used in the attestation script are not currently compatible with BSD find or xargs. I just swapped them out for gfind and gxargs for now.

in contrib/guix/guix-attest:151 in 8dd3c4a7ce outdated

146+        (
147+            cd "$outdir"
148+            find . -type f -printf '%P\0' | env LC_ALL=C sort -z | xargs -r0 sha256sum >> "$outsigdir"/SHA256SUMS
149+        )
150+        echo "${outname}: Signing SHA256SUMS to produce SHA256SUMS.asc"
151+        gpg --detach-sign --local-user "$gpg_key_name" --output "$outsigdir"/SHA256SUMS.asc "$outsigdir"/SHA256SUMS

sipa commented at 7:20 pm on April 10, 2021:

If you’re producing .asc files I think it would be expected that they’re ascii-armored (--armor); otherwise use the .sig extension (this doesn’t really matter, but it’s a convention).

RonSherfey changes_requested

RonSherfey commented at 3:29 pm on April 11, 2021: none

requested change

RonSherfey approved

laanwj commented at 2:04 pm on April 12, 2021: member

Personally I’m not a big fan of using the word “sign” as it’s been used to refer to both codesigning and GPG signing.

I tend to agree, “gitian signature” has always been somewhat ambiguous and confusing, attestation is a better word.

laanwj commented at 2:24 pm on April 12, 2021: member

FWIW: I have a multi-step flow:

On the build VM, gitian-sign with -p true to generate .assert files but bypass the actual signing
Copy .assert files to signing machine
On signing machine, generate detached signatures and check them into git

I’m not sure how far it is reasonable to accommodate my specific way of working (though there may be more people doing this, it’s good to be careful with signing keys). But it would be nice to be able to skip the gpg invocation in guix-attest.

For checking gitian signatures, I have been using my own script for this as well, see: bitcoin-core/bitcoin-maintainer-tools#71 bitcoin-core/bitcoin-maintainer-tools#90 . I think I can straightforwardly adapt it to Guix attestations when needed.

dongcarl commented at 10:36 pm on April 12, 2021: member

Pushed 5da0e361b8b5a6d730a244e774c6810946619229 -> 0c238fd118742d6c059dc114b5cc0747adf7f6c5

Added commits to address:

laanwj: Please let me know if the NO_SIGN flag is enough for your workflow!

My attestations: https://github.com/dongcarl/guix.sigs/commit/fb313a28a57dc7020309b1521cef0b783d3ca81f

RonSherfey approved

laanwj commented at 9:44 am on April 14, 2021: member

laanwj: Please let me know if the NO_SIGN flag is enough for your workflow!

Yes seems perfect!

in contrib/guix/guix-attest:21 in 0c238fd118 outdated

16+
17+################
18+# Required non-builtin commands should be invokable
19+################
20+
21+check_tools cat env basename mkdir xargs find gpg

laanwj commented at 3:24 pm on April 14, 2021:

gpg check can be skipped in case of NO_SIGN?

in contrib/guix/guix-attest:90 in 0c238fd118 outdated

85+IFS='=' read -r gpg_key_name signer_name <<< "$SIGNER"
86+if [ -z "${signer_name}" ]; then
87+    signer_name="$gpg_key_name"
88+fi
89+
90+if ! gpg --dry-run --list-secret-keys "${gpg_key_name}" >/dev/null 2>&1; then

laanwj commented at 3:24 pm on April 14, 2021:

This check can be skipped in case of NO_SIGN

laanwj commented at 3:44 pm on April 14, 2021: member

I succeeded in generating attestations and signing them externally after removing the above checks and

0GUIX_SIGS_REPO=/home/guest/guix.sigs SIGNER=laanwj NO_SIGN=1 contrib/guix/guix-attest

My only comment really is that having one attestation file per architecture means having to create a lot of signatures. I need to push the button on my external signer device for every one of them :smile:

dongcarl force-pushed on Apr 14, 2021

dongcarl commented at 4:15 pm on April 14, 2021: member

Pushed 0c238fd118742d6c059dc114b5cc0747adf7f6c5 -> 816c70c3cef2dde50436461f3fe43f8915840a26

Addressed #21462#pullrequestreview-635756951
Addressed #21462#pullrequestreview-635757547

sipa commented at 9:01 pm on April 14, 2021: member

Building 816c70c3cef2dde50436461f3fe43f8915840a26

EDIT: see attestation result https://github.com/bitcoin-core/guix.sigs/pull/5

laanwj commented at 12:37 pm on April 15, 2021: member

Tested ACK 816c70c3cef2dde50436461f3fe43f8915840a26 Results at bitcoin-core/guix.sigs#4

sipa commented at 6:33 pm on April 15, 2021: member

~~Tested ACK 816c70c3cef2dde50436461f3fe43f8915840a26~~. I tried building, attesting, and verifying (as well as testing that verification fails with invalid signature).

As a follow-up, I think the output of guix-verify could be improved a bit (giving a global success/fail output, and perhaps grouping the output by signer).

hebasto commented at 6:52 pm on April 15, 2021: member

Tested 816c70c3cef2dde50436461f3fe43f8915840a26: https://github.com/bitcoin-core/guix.sigs/pull/6

Tested both contrib/guix/guix-attest and contrib/guix/guix-verify.

sipa commented at 6:58 pm on April 15, 2021: member

Actually, I think it would be better if the signed data contained a stronger commitment to the source code it was built from than just a truncated git commit id.

Attestations are really claims by the builder that certain input source code corresponds to the resulting build output. As the gpg-signed SHA256SUMS file doesn’t actually commit the source code, someone could try to grind git commit ids to a collide with honest source code, get signers to accidentally guix attest the malicious source code, and then distribute the malicious build output along with a claim it was built from honest source code.

A possibility would be including the dist-archive source tgz in every SHA256SUMS file.

dongcarl commented at 7:09 pm on April 15, 2021: member

You’re right, attesting to truncated git commit ids by themselves is problematic and we should attest to the source code as well. I’m wondering what specific malicious activity can be thwarted by attesting to the source tgz in every SHA256SUMS file vs. attesting to it in dist-archive/$SIGNER/SHA256SUMS like we do now?

sipa commented at 7:10 pm on April 15, 2021: member

@dongcarl You can selectively combine (dist-archive/$SIGNER/SHA256SUMS from the honest source code, binary-platform/$SIGNER/SHA256SUMS from the malicious one).

dongcarl commented at 7:14 pm on April 15, 2021: member

Let’s say we do attest to the source tgz in every SHA256SUMS file. If I were a malicious signer, I could still manually construct the SHA256SUMS to attest to honest source code and malicious binaries, no?

sipa commented at 7:16 pm on April 15, 2021: member

@dongcarl You can’t modify the SHA256SUMS without invalidating its signatures. The signatures are per-SHA256SUMS-file, so you can mix and match different SHA256SUMS files, but you can’t mix and match lines within one file.

I’m not talking about a malicious signer. I’m talking about for example someone creating a PR with a hidden backdoor (which never gets merged or even reviewed) that leads to a collision in the short git commit id with honest code, and then they get honest signers to produce an attested guix build for that. They can then take the dist-archive SHA256SUMS + honest sigs on it from the good source code, and the x86_64 SHA256SUMS file + its (honestly created) sigs on the malicious code.

My point is: a guix attestation isn’t and shouldn’t be interpreted as the builder/attester vouching for the code they are building - we have the review process for that. It is purely a statement that particular source code (good or bad) corresponds to particular binaries, and in order to do so with cryptographic assurances, the SHA256SUMS file (or whatever is signed) needs to commit to both the input and the output of the build process.

dongcarl commented at 7:42 pm on April 15, 2021: member

They can then take the dist-archive SHA256SUMS + honest sigs on it from the good source code, and the x86_64 SHA256SUMS file + its (honestly created) sigs on the malicious code.

Okay I see… And if the malicious person combines them, they can make it seem like the honest signer is producing malicious code… Am I understanding that right?

sipa commented at 7:44 pm on April 15, 2021: member

Right, they can then make a distribution of the software along with SHA256SUMS{,.asc} with malicious binaries and honest source code, signed by honest signers.

dongcarl commented at 7:47 pm on April 15, 2021: member

Ah that makes sense! Thanks for spotting it and being patient with me. I will make the changes :-)

sipa commented at 7:49 pm on April 15, 2021: member

So, if you’d want to do this exactly, I think you’d want to have a build process that first creates the dist-archive, and then afterwards builds the binaries, using just the dist-archive output as input (rather than the source tree you were working on).

Perhaps something for later, as you obviously trust the guix build scripts themselves not to do anything crazy here.

Emzy commented at 1:18 pm on April 16, 2021: contributor

Tested ACK. Works with Yubikey as signing device.

dongcarl commented at 8:33 pm on April 20, 2021: member

Pushed 816c70c3cef2dde50436461f3fe43f8915840a26 -> 7cd57f3ac308ba296842289542b15e902f105964

The build output is now constructed in $DISTSRC/output, then moved (likely atomically) to $OUTDIR when everything is done
Inputs are now attested to, thanks to sipa’s suggestions

Please see commit messages for more explanations.

Signatures up here: https://github.com/dongcarl/guix.sigs/tree/2021-04-PR21462/7cd57f3ac308

RonSherfey approved

dongcarl commented at 3:55 pm on April 28, 2021: member

Wondering if the latest changes look alright, and what else needs fixing up here before it’s ready for merge!

in contrib/guix/guix-attest:147 in 7cd57f3ac3 outdated

142+
143+echo "Attesting to build outputs for version: '${VERSION}'"
144+echo ""
145+
146+# MAIN LOGIC: Loop through each output for VERSION and attest to output in
147+#             GUIX_SIGS_REPO as SIGNERif attestation does not exist

MarcoFalke commented at 9:49 am on April 30, 2021:

8dd3c4a7ce9ac744668517a365cf8757713e7e7a:

0#             GUIX_SIGS_REPO as SIGNER, if attestation does not exist

in contrib/guix/guix-attest:167 in 7cd57f3ac3 outdated

160-            files="$(find . -type f)"
161+
162+            if [ -e inputs.SHA256SUMS ]; then
163+                echo "${outname}: Including existent input SHA256SUMS"
164+                cat inputs.SHA256SUMS >> "$outsigdir"/SHA256SUMS
165+            fi

MarcoFalke commented at 10:17 am on April 30, 2021:

shouldn’t this fail when the file is missing?

dongcarl commented at 6:05 pm on April 30, 2021:

Thinking about this since dist-archive does not have an inputs.SHA256SUMS… Right now, we attest to the bitcoin repo’s git-archive in distsrc-archive, but I think perhaps it only makes sense to attest to it in the context of it being inputs to the host-specific outputs. When we add codesignatures, similarly it only makes sense to attest to the detached-sigs repo’s git-archive in the context of it being inputs to the host-specific codesigned outputs. Will make a change so that we don’t attest to dist-archive directly.

MarcoFalke commented at 10:17 am on April 30, 2021: member

Concept ACK 7cd57f3ac308ba296842289542b15e902f105964 🍠

Signature:

 0-----BEGIN PGP SIGNED MESSAGE-----
 1Hash: SHA512
 2
 3Concept ACK 7cd57f3ac308ba296842289542b15e902f105964 🍠
 4-----BEGIN PGP SIGNATURE-----
 5
 6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
 7pUiWMwv+MSUhxaO+wyNQ7iSyL1e3PrBcw5kLsXZDz8aWT27W8ZMJLoArJWD6K10p
 8o5QHYUJSg9HL/4QOQ+g+VCt6fdA2ktjpeKsrKfoivgMwfV5ZO2UowUHazuaxzi/k
 9oAR+xOLaWpsuKqvwSEsV9n0E53A7a5E/6jj5O2U9Nt2QFStRCW82q5Sm6cApy/mn
10wTrvHQn4cqFqe+Io2WKnIvLFP8V/blggXRbGynm8+v1PBGyVTXoBP/CVbklc7JNj
11Wl2hcY2Fzn0LfWgqRagtAlOTH/xxwbvPm/6PMfOqJg0u2Y4/e6KojTuFBYEQrXM4
12vrjI3rRqjzg6fMa3miSZAwfknCIQJbDX1u7QIie4cqDolVM0HyGZizFxIr3m6hsc
13VQ0Wu+eCrHzB3K2UdI5lo4Nn8O+quCICmue5d9Iix1obNdxCR4Q915EMWIejjkkR
1445e2IOwJOT0ADQy3Drd2X6CQAHZTHJGtaoPecFR6DRTUE2uKHTYiKLEEb4QYJHBC
151B6vN1+j
16=bEwp
17-----END PGP SIGNATURE-----

Timestamp of file with hash bc7b8ef6e6110dba1bb1148d42a8626cc8670590bd75fcef192668f241327364 -

DrahtBot commented at 9:33 am on May 3, 2021: member

🕵️ @practicalswift has been requested to review this pull request as specified in the REVIEWERS file.

guix: Add guix-attest script 30daf76a97

guix: Add guix-verify script 5926432ba6

guix-attest: Only use cross-platform flags for find+xargs b5fd89c4c8

guix-attest: Use ascii-armor signatures 0e1c2e448c

guix-attest: Allow skipping GPG signing with NO_SIGN c83c4fa5b7

guix: Minor quoting fix in libexec/build.sh 022abc85fc

guix: Construct $OUTDIR in ${DISTSRC}/output

While files are being output to $OUTDIR, it will be under
${DISTSRC}/output, and only when everything is done, will
${DISTSRC}/output be moved to the actual $OUTDIR.

This makes it so that a Ctrl-C in the middle of a build is less likely
to result in a partially-constructed $OUTDIR. In fact, if I understand
correctly, if $OUTDIR and $DISTSRC reside on the same filesystem, the
move (rename) is likely atomic.

Also, since the "working $OUTDIR" is under ${DISTSRC}/output, it will be
cleaned properly by the guix-clean script.

f9e2960c01

guix: Attest to inputs in inputs.SHA256SUMS

At build/codesigning-time, hash build inputs and output the digest to
${OUTDIR}/inputs.SHA256SUMS, which gets included in the final SHA256SUMS
constructed by guix-attest.

Example final SHA256SUMS:
ee832d2a35b7701bff581dea05a536118b118e3ad0a587a2855b6ee8cd6fba20  inputs/bitcoin-78199266af7b.tar.gz
ca765e70a0c12866dd63c0be228b675278a26329e5f8f5b5c52fd09200fedf21  bitcoin-78199266af7b-powerpc64le-linux-gnu-debug.tar.gz
dae95327d7f2c324e2728c4b73627be6cb2c0d2f2e5bea940d1d5e6463939327  bitcoin-78199266af7b-powerpc64le-linux-gnu.tar.gz

d522d8006b

guix: Skip attesting to dist-archive

We already attest to the relevant dist-archive in inputs.SHA256SUMS,
which is recorded at build-time.

We use a SKIPATTEST.TAG file to indicate output directories which do not
require attestation (much like the CACHEDIR.TAG specification).
Generally, it's better to have build scripts declare properties of
directories instead of introducing name-based special cases in attest
scripts since build scripts have a more detailed context of what is
going on.

feda2c8e31

dongcarl force-pushed on May 3, 2021

dongcarl commented at 6:50 pm on May 3, 2021: member

Pushed 7cd57f3ac308ba296842289542b15e902f105964 -> feda2c8e3180cb983c35976d4440cea23a155b7f

Addressed: #21462 (review), #21462 (review) (see last commit)

As always, guix.sigs are up at: https://github.com/dongcarl/guix.sigs/tree/2021-04-PR21462/feda2c8e3180

guix-attest: Avoid incomplete sigdirs with ERR traps

Sometimes GPG connects to the wrong agent... or you don't have your
smartcard handy...

d420e5c1c0

dongcarl commented at 3:56 pm on May 4, 2021: member

Pushed feda2c8e31..d420e5c1c0:

Added an ERR trap to remove incomplete sigdirs if signing fails

guix.sigs up at: https://github.com/dongcarl/guix.sigs/tree/2021-04-PR21462/d420e5c1c015

laanwj commented at 8:09 pm on May 5, 2021: member

Tested ACK I get the same output.

0for x in *; do diff -du $x/dongcarl/SHA256SUMS $x/laanwj/SHA256SUMS; done

Would it be useful to have a concatenated SHA256SUMS (with all files) as well? I mean, depending on how we’re going to do the distribution on the website and torrent.

I guess if we’re redesigning this it makes sense to have the two match up, not sign a different file with the ‘distribution signing key’ anymore but the same one (or forego my distribution signing key completely, just collect the attestations).

Or is the idea is to start using a subdirectory per platform there (each with its own SHA256SUMS) too? In this case, having inputs/bitcoin-d420e5c1c015.tar.gz in every one might not be practical, it implies having to duplicate the tarball . Maybe ../bitcoin-d420e5c1c015.tar.gz then have the tarball in the top-level directory?

dongcarl commented at 7:41 pm on May 7, 2021: member

Or is the idea is to start using a subdirectory per platform there (each with its own SHA256SUMS) too? In this case, having inputs/bitcoin-d420e5c1c015.tar.gz in every one might not be practical, it implies having to duplicate the tarball .

Hmmm, are you talking about for the webserver? I think it can be done using hard/symlinks at the FS level, or just using virtualized paths at the HTTP level.

Wondering what particular user-flow you have in mind :-)

laanwj commented at 11:20 am on May 12, 2021: member

Code review and tested ACK d420e5c1c015f58d07aca4d6a805086488f74d03

Wondering what particular user-flow you have in mind :-)

After discussion on IRC: let’s leave this for another PR.

laanwj merged this on May 12, 2021

laanwj closed this on May 12, 2021

laanwj moved this from the "Next (Not based on any other PRs)" to the "Done" column in a project

sidhujag referenced this in commit bf89181b22 on May 12, 2021

in contrib/guix/guix-attest:169 in d420e5c1c0

164+        (
165+            cd "$outdir"
166+
167+            if [ -e inputs.SHA256SUMS ]; then
168+                echo "${outname}: Including existent input SHA256SUMS"
169+                cat inputs.SHA256SUMS >> "$outsigdir"/SHA256SUMS

Sjors commented at 8:43 am on May 14, 2021:

This line fails with “path not found” on macOS when GUIX_SIGS_REPO is a relative path like ../guix.sigs.

in contrib/guix/guix-attest:175 in d420e5c1c0

170+            fi
171+
172+            echo "${outname}: Hashing build outputs to produce SHA256SUMS"
173+            files="$(find -L . -type f ! -iname '*.SHA256SUMS')"
174+            if [ -n "$files" ]; then
175+                cut -c3- <<< "$files" | env LC_ALL=C sort | xargs sha256sum >> "$outsigdir"/SHA256SUMS

Sjors commented at 8:58 am on May 14, 2021:

On macOS this needs to be shasum -a 256 or the user has to make an alias or brew install coreutils

dongcarl commented at 8:32 pm on May 14, 2021:

Hmmm, does shasum -a 256 produce identical output to sha256sum?

Sjors commented at 9:12 pm on May 14, 2021:

If my uploaded attestation is correct, yes :-)

fanquake referenced this in commit e7441a6a45 on Jul 20, 2021

gwillen referenced this in commit 120787490d on Jun 1, 2022

DrahtBot locked this on Aug 16, 2022

guix: Add guix-{attest,verify} scripts #21462

Conflicts