release: create and verify detached signatures, fix hashing command on MacOS #5019
Conversation
guggero
added
golang/build system
| cat "$signature" | ||
| exit 1 | ||
| fi | ||
|
|
||
| if ! grep -q "$LNCLI_SUM" "$signature"; then |
There was a problem hiding this comment.
interesting, so we could actually be signing different manifests, but the signatures would verify anyway?
There was a problem hiding this comment.
We used the --clear-sign flag before which overwrote the --detach-sign flag. So we didn't actually produce detached signatures.
If we start creating detached signatures, then the manifest they signed must be specified when verifying. So the manifest must match exactly and be present on the file system. The .sig file will only contain the raw signature, nothing more.
That's why we now have to look in the manifest text file for the hash.
I didn't do this at first because the manifest text file wasn't yet fully reproducible (because of the vendor mismatch and the sorting of the file itself). But now that that problem is out of the way we can actually start all signing the same manifest file and just upload the detached signature.
scripts/verify-install.sh
Outdated
| if ! grep -q "$LND_SUM" "$signature"; then | ||
| echo "ERROR: Hash $LND_SUM for lnd not found in $signature: " | ||
| # manifest. | ||
| if ! grep -q "$LND_SUM" "$MANIFEST"; then |
There was a problem hiding this comment.
prbably out of scope for this PR, but could be useful to verify arbitrary lnd binary without having to install it.
Usecase is I build the release and verify the signatures without having to install it (I might have a dev build installed that I'm running)
There was a problem hiding this comment.
That's a good idea! Created a separate PR: #5021
There was a problem hiding this comment.
Is it possible to make the check stricter by grepping for ”^<hash> “ so it is extracted from the first column?
There was a problem hiding this comment.
Yes, good idea. Done.
|
Tested, and works on macOS 👍 Only thing I noticed is that the |
scripts/verify-install.sh
Outdated
| if ! grep -q "$LND_SUM" "$signature"; then | ||
| echo "ERROR: Hash $LND_SUM for lnd not found in $signature: " | ||
| # manifest. | ||
| if ! grep -q "$LND_SUM" "$MANIFEST"; then |
There was a problem hiding this comment.
Is it possible to make the check stricter by grepping for ”^<hash> “ so it is extracted from the first column?
scripts/verify-install.sh
Outdated
| exit 1 | ||
| fi | ||
|
|
||
| echo "Verified lnd and lncli hashes against $signature" | ||
| echo "Verified lnd and lncli hashes against $MANIFEST" |
There was a problem hiding this comment.
nit: I think the hashes only need to be verified once now, e.g.:
for sig in signatures:
verify sig
#everyone agrees on manifest
check lnd hash
check lncli hash
There was a problem hiding this comment.
Agreed! Though if the hashes don't match we don't need to look at the signatures. So I put the hash check first.
There was a problem hiding this comment.
My bad, on mobile it looked like this check was still happening per signature. However I still think we want the hash checking should occur after the signature verification.
Though if the hashes don't match we don't need to look at the signatures. So I put the hash check first.
As an optimization, that's true. This is more about what surfacing what is actually wrong with the verification.
If we check hashes first, you don't know whether the manifest is actually agreed upon by all developers. If all the developers signed the correct manifest, but the manifest on github is invalid the user will see that the hash wasn't included. They might think that they've built the wrong binary, but the error that user should see is that the signatures disagree with the github manifest.
Once we know everyone agrees, then we can return whether the hash of your binary is not in the signed manifest.
Signature disagreement represents a faulty/backdoored release process, the latter just means you don't have the correct binary. Perhaps we should also be more specific about these failure modes in the error messages, e.g. "the expected release binaries have been verified with the developer signatures, but you seem to have an invalid binary." vs "the developer signatures disagree on the expected release binaries, the release may have been faulty or backdoored"
There was a problem hiding this comment.
Yes, the hash check was still in the loop over each signature in the version you initially reviewed.
I now moved it to after the signature check. It totally makes sense with the scenario you describe, I just didn't think of this and hence the premature optimization 😅
I also updated some of the error messages to reflect your suggestion.
Yes, fixed as well. |
Because the sha256sum binary isn't available on MacOS we instead use the shasum -a 256 command that was used before.
To make sure we've actually calculated the hash correctly, we make sure it's 64 characters long.
Due to a misunderstanding of how the gpg command line options work, we didn't actually create detached signatures because the --clear-sign flag would overwrite that. We update our verification script to now only download the detached signatures and verify them against the main manifest file. We also update the signing instructions.
We want to be more precise in what exactly went wrong and what the cause could be.
Fixes #4966.
This PR updates the verification script to now look for detached signatures instead of clear signed ones.
Can be tested on
v0.12.1-beta.rc3, I've uploaded a detached signature.Also fixes an issue with the
sha256sumcommand not being available on MacOS.