#610 Packaging guidelines: Check upstream tarball signatures
Opened 8 years ago by dwmw2. Modified 4 years ago

https://fedoraproject.org/wiki/PackagingDrafts:GPGSignatures

If there's a signature available for the upstream source tarball, check it as a matter of course in %prep.

How did we ever not write this down before? :)


I think this is a good idea, but.... can't we give a URL for that GPG key? How does one obtain it? (Assume the packager knows nothing of GPG.) Is it reasonable to have it just stored in the SRPM and our lookaside with no obvious way to re-fetch it?

I was just trying to work that one out. Why do we need to provide it at all? The signature itself will contain enough of the key. Why can't I just specify the fingerprint of the key I find acceptable?

gpgv2 --key-id 1234567890ABCDEF1234567890ABCDEF %{SOURCE1} %{SOURCE0}

I'm slightly unhappy with the example I've given with --keyring for other reasons too. The gpgv2 man page suggests that it'll use the given keyring in ''addition'' to the default keyring, although that doesn't seem to be the case. I want to be '''absolutely''' clear that it'll only use the ''one'' key we expect, though. I don't ever want a potential compromise vector which involves another rogue package dropping files into /root/.gnupg or wherever would be required to affect koji builds.

The actual answer to your question, if it wasn't a rhetorical one, is:
{{{
gpg2 --no-default-keyring --keyring ./gpgkey-$KEYID.gpg --recv-keys $KEYID
}}}
Of course, you should check the fingerprint of the resulting key matches what you expect, and there's scope for problems there. But once it's ''done'' we protect against future compromised tarballs.

You raise a good point — regardless of whether gpgv2 can be improved to make the mechanism simpler, we still need to give clear documentation about how to safely obtain the signing key the upstream uses.

It wasn't a rhetorical question; I know only a little about gpg.

There was talk of extending spectool or fedpkg or something to handle doing something like this. I'd planned on adding at least some related functionality to my spectool rewrite, but then the original author of spectool did an incompatible rewrite and so I deferred to him. Now we're stuck with the original spectool and two rewrites and nothing happening. And it's not particularly easy to add things to fedpkg.

Also, there's an interesting question of doing "too much" in prep. We don't have any guidelines about it, but it's not uncommon to just prep a package in order to examine it. What happens if the gpg check fails? In your example, I can't tell if the gpg call is intended to go above %autosetup, but if so then a sig failure would prevent this use case.

I suppose ''"how you trust that the signing key is the real one"'' is a per-project question. Exim publishes the set of PGP keys which might sign releases, and you'd want them all in the keyring: http://ftp.exim.org/pub/exim/Exim-Maintainers-Keyring.asc

Other projects like Open''''''''''Connect just link to the keyserver: http://www.infradead.org/openconnect/download.html

Yes, the GPG check failing in %prep would cause %setup or %autosetup to fail. But that's fine because you'll be too busy putting the fire out, to continue with whatever you were originally trying to do. And if there is some genuine reason for the GPG check failing, then it's not as if it's hard to comment out that one line and do it again. Making things break if the GPG check fails is kind of the ''point'' :)

We discussed this at this weeks meeting (http://meetbot.fedoraproject.org/fedora-meeting-1/2016-03-24/fpc.2016-03-24-16.01.txt) open floor.

I think checking upstream data against published checksums and/or signatures is a great idea, but the build system doesn't seem like a useful place to put any of it. After more thought I'm still against it but not as much as before, assuming it's local and is "instant".

All of this should be checked before it hits the build system, and what's the point of rechecking it for every release bump?

This seems like a great feature for anitya (release-monitoring.org), along with clickable links to the upstream data and anitya supplied/proxied checksums. Also would be cool for some tool that allows a packager to import upstream data into sources and check upstream checksums/signatures.

It's local and it's instant... and it pushes the check as far down the pipe as possible. Rather than depending on a packager to painstakingly check the download signature ''manually'' every time they do an update, we do it automatically. It also helps to protect against other problems via the lookaside cache (although those are fairly unlikely).

Rather than depending on a packager to painstakingly check the download signature manually every time they do an update, we do it automatically

Right, this is obviously a win. Going from almost no checking to lots of checking, +1. That's the main reason I went from "100% not in %setup" to "please not in %setup", because if that's the only place someone will work to put it then I guess we just have to live with it.

But it feels pretty wrong to put something in for a build just to see if we downloaded the correct data, and mostly stupid to recheck it everytime we bump a release.

The check is inexpensive, far less than rpmlint even (which yes, I know, happens after the build, out of line). The protection is against unauthorized changes to the tarball, which fortunately we haven't had happen in Fedora, but has hit SourceForge and others from time to time. Having the signatures in the lookaside cache, and the keys in version control, would make the task of revalidating after an "incident" trivial. Running the checks at every build means we could detect "incidents" at build time rather than via other non-automated asynchronous means.

I agree with doing this in principle; it's all in the details.

I still think that it's problematic to have this at the beginning of %prep instead of the end, since without you'd have to unpack and patch manually and that's just annoying.

I've been playing with %autosetup to just have it take an argument with the necessary information (probably -G L,M,N to check Source L and signature M with key N) so that this can be done without relying on packagers getting the gpg command line correct.

Also, if any of this has a gpg version requirement, please let us know what it is.

Also, could you provide a sample package?

Replying to [comment:12 tibbs]:

Also, could you provide a sample package?

Easiest is probably exim, as the .asc is just %{source}.asc, and they have an official keyring (which David gave above: http://ftp.exim.org/pub/exim/Exim-Maintainers-Keyring.asc)

This example seems to work (can alter the source and get success/failure by exit code), the double keys import is stupid but about par. for the course with gpg IME.

I was looking for a package where things are actually checked in/in the lookaside/implemented as suggested.

Replying to [comment:15 tibbs]:

I was looking for a package where things are actually checked in/in the lookaside/implemented as suggested.

http://pkgs.fedoraproject.org/cgit/rpms/youtube-dl.git/tree/youtube-dl.spec#n35 does the check. (That package does not (yet) use the --dearmor argument to allow storing the key rather than a keyring, but since Till is a maintaner of youtube-dl and the one who noted the option, I suspect it will be at some point.)

Replying to [comment:14 james]:

This example seems to work (can alter the source and get success/failure by exit code), the double keys import is stupid but about par. for the course with gpg IME.

Just curious, is there any particular reason to import the key as opposed to using gpgv2 --keyring ./gpgkey-$KEYID.gpg ? Combined with gpg2 --dearmor, it allows an ascii-armored file to be stored in git rather than the binary keyring file.

Here's a diff against the test script which uses gpgv2 and avoids importing the key at all. It also sets the gpg homedir to a temp dir to ensure that no local keyrings or settings are used (not an issue with mock builds, but definitely nice for package maintainers and other users).

{{{
--- dl-exim-chk.sh~ 2016-03-28 18:04:25.609630390 -0400
+++ dl-exim-chk.sh 2016-03-28 18:16:35.256580044 -0400
@@ -4,8 +4,6 @@
asc=$url.asc
key=http://ftp.exim.org/pub/exim/Exim-Maintainers-Keyring.asc

-KEYID=abcd

function dl
{
if [ ! -f $(basename "$1") ] ; then
@@ -18,14 +16,18 @@
dl "$asc"
dl "$key"

  • rm -f ./gpgkey-$KEYID.gpg*
  • lulz ... doesn't import due to missing public keys, but then works.

  • if gpg2 -q --no-default-keyring --keyring ./gpgkey-$KEYID.gpg --import \
  • $(basename "$key"); then
  • gpg2 -q --no-default-keyring --keyring ./gpgkey-$KEYID.gpg --import \
  • $(basename "$key")
  • fi
    +# Ensure we don't use any existing gpg keyrings or settings
    +gpghome="$(mktemp -qd)"
    +
    +# Convert the ascii-armored key to a suitable keyring
    +#
    +# With GnuPG 2.0 (EL 5-7), there is some noisy output, so it's ideal to
    +# redirect stdout to /dev/null. That's intentionally not done here so all
    +# output can be seen while testing.
    +#
    +# https://bugs.gnupg.org/gnupg/issue2290 may make this step moot, eventually
    +gpg2 --dearmor --quiet --batch --yes $(basename $key)

  • gpg2 --no-default-keyring --keyring ./gpgkey-$KEYID.gpg --list-sigs

  • gpg2 --no-default-keyring --keyring ./gpgkey-$KEYID.gpg --verify \
  • $(basename "$asc") $(basename "$url")
    +# Verify the signature
    +gpgv2 --homedir "$gpghome" --quiet --keyring ./$(basename $key).gpg \
  • $(basename $asc) $(basename $url)
    }}}

The output is:

{{{
$ ./dl-exim-chk.sh
ftp://ftp.exim.org/pub/exim/exim4/exim-4.86.2.tar.bz2

################################################################## 100.0%

ftp://ftp.exim.org/pub/exim/exim4/exim-4.86.2.tar.bz2.asc

################################################################## 100.0%

http://ftp.exim.org/pub/exim/Exim-Maintainers-Keyring.asc

################################################################## 100.0%

gpgv: Signature made Wed 02 Mar 2016 12:52:53 PM EST using RSA key ID A0450CF5
gpgv: Good signature from "Heiko Schlittermann (Dresden) hs@schlittermann.de"
gpgv: aka "Heiko Schlittermann (HS12-RIPE) hs@schlittermann.de"
gpgv: aka "[invalid image]"
gpgv: aka "Heiko Schlittermann (Exim MTA Maintainer) heiko@exim.org"
}}}

(Downloading the keyring via http is less than ideal for someone who doesn't already have the keys to establish trust, but that's up to each project and maintainer.)

I worked on adding signature checking to the git package over the weekend, as it's something I've done manually for ages and wanted to get the upstream signatures re-included in the spec after they got lost over time. The git tarballs are signed before they are compressed, which makes it a little different than most upstream projects. We also have several tarballs in the git spec file (some prebuilt docs for older releases that cannot build them from source -- as an aid to end-users who want to keep current). I haven't pushed the changes to pkgs.fpo yet, but the code is [https://fedorapeople.org/cgit/tmz/public_git/git-package.git/tree/git.spec?h=verify-gpg-signatures#n350 in my fedorapeople repo].

dl-exim-chk.sh using gpgv2 to avoid keyring imports
dl-exim-chk.2.sh

Excuse me, but this statement quite contradicts with the idea:

It is possible for source tarballs to be compromised at any point in time, from the download site or within the Fedora lookaside cache.

If you admit that the files from the download site or within the Fedora lookaside cache are compromised, I can't see any reason why also the signature should not be compromised. This way you'll be checking just that the compromised files and they signatures are in sync, nothing more.

The attacker would have to have the ability to change at least two, perhaps three things: the source tarball and signature file in the lookaside cache certainly. Then have the ability to either change the public key in source control to a different public key they control, or have the ability to use upstream's signing key to generate a new signature file. All without being detected (git logs, surprises when maintainers do a 'git pull', emails on source upload, etc). That's as high a bar as we could set.

We discussed this at this weeks meeting (http://meetbot.fedoraproject.org/fedora-meeting-1/2016-03-31/fpc.2016-03-31-16.00.txt):

  • 612 Python naming convention makes some Python tools unusable


    (geppetto, 16:40:12)

...just discussed it more, tibbs going to try working on macros given examples in the ticket but any help/advise from crypto professionals would be appreciated.

Here's a working two-liner using just gpg2 (key, asc and src are downloaded following the example in comment:17 :
{{{
gpg2 --homedir $gpghome --no-default-keyring --quiet --yes --output $gpghome/${key}.gpg --dearmor $key
gpg2 --homedir $gpghome --no-default-keyring --keyring ${key}.gpg --verify $asc $src
}}}

spectool rewrite now supports signature verification when downloading files:
https://pagure.io/spectool
(This essentially wraps the second command in previous comment.)

It should be easy enough to add support for downloading dearmored keys.
(I.e. wrapping the first command in previous commit.)

How strongly followed are the conventions for naming keys and detached signatures? How much can you depend on the signatures being named after the tarball with ".asc" appended? Is it relatively safe to look for "key" in what's left after pairing up tarballs with ".asc" files?

Obviously that can't be 100%; I'm just trying to figure out if there's a way that works for 90% and let things be directly specified for the rest.

Things in Fedora spec files should be a pretty good sample. There might be other conventions in the wild, but we don't really care about them, right?

I downloaded rpm-specs-latest.tar.xz and grepped for gpgv2:\
.keyring × 4\
.gpg × 10

Is it relatively safe to look for "key" in what's left after pairing up tarballs with ".asc" files?
I'm not sure. I tried mime type detection and using file on the signature files and the automatic detection didn't seem to work at all. Nevertheless, armored ascii files have a pretty standard format, so it should be possible to detect them reliably.

^That was about the keyring.

For the signature itself, the split seems to be:\
.sig × 9\
.asc × 4\
.sign × 2

After some experimentation, I found that it's actually not particularly difficult to just look in the actual files for the necessary "BEGIN whatever" strings. If you want to be automatic, though, you still have to pair the signatures up with the matching files.

in any case, (sig|asc|sign) looks to be a good start, so I'll go with that for now.

OK, I've banged out a very preliminary macro, %gpg_verify. There's a repo at https://pagure.io/gpg-macros which contains enough to run fedpkg prep or fedpkg mockbuild. The you can see the sample spec, and thus the macro in use, at https://pagure.io/gpg-macros/blob/master/f/gpg-macros.spec

I'm still working on this, and the actual macro code isn't in its final form (so don't go "aargh the horrible Lua my eyes it burns"). But it does actually check a signature and fail out of %prep if there's an issue.

Currently not much is automatic; you have to specify (by number) source,signature,keyring triples or source,signature pairs and let it find a keyring.

BTW, could someone tell me how to generate the "gpgkey-loadsofhex.gpg" file that youtube-dl has? And if possible the thing starting with "-----BEGIN PGP PUBLIC KEY BLOCK-----" instead of that binary blob? (There doesn't be any way at all to detect by inspection that the blob is a gpg key, which is rather suboptimal.)

@tibbs, Converting an ascii armored key to a usable gpg keyring file can be done like this:

{{{
gpg2 --dearmor --quiet --batch --yes $key >/dev/null
}}}

This will create $key.gpg which can then be used with gpgv2 ... --keyring $key.gpg for verification.

@till filed a [[https://bugs.gnupg.org/gnupg/issue2290|gnupg bug]] request support for gpgv2's --keyring argument to accept ascii-armored key files. So at some point this conversion won't be needed.

It's definitely much nicer to use the ascii-armored key file instead of the binary gpg keyring file. It's also far more common for upstreams to provide the ascii-armored key file than a binary gpg keyring. ;)

Thank you for working to make this easier for all packagers!

Thanks, but that wasn't what I was asking. I was wondering how to generate the file in the first place.

I.e upstream just gives key IDs and fingerprints. How to you generate the ascii armored keyring file from that? (Keep in mind that I know zilch about gpg.)

youtube-dl has the binary key in the lookaside and includes no instructions for generating it, which I see as extremely suboptimal. I think if we're going to do this, we will need to have some suggestions about handling this case.

Ahh, sorry for the confusion.

The key file isn't really something which will be generated. There are various ways in which a packager may acquire the key, depending on what upstream provides. Often, upstream will provide a key file on their site, much like Fedora provides key files for verifying our images (https://getfedora.org/keys/).

If upstream doesn't publish their key on their web site and instead only publishes it to a keyserver, then the packager would need to download it from the keyserver. That can be done with gpg --recv-keys or via one of the various http front-ends to the keyserver.

Getting the key is a step that should be done manually, by the packager. We very likely do not want to automatically download keys, as we would have no way to verify them at all and then the signature checking is nothing more than security theater.

I agree that putting the binary key in the spec file/package repo is not the ideal method. I don't think the youtube-dl maintainers would disagree on that either. I think they were just ahead of the curve in doing such verification. After the discussions on the devel list which showed the gpg2 --dearmor command can convert an ascii-armored key into a binary gpg keyring, I think that's the best method. (And at some point, gpgv2 may learn how to use an ascii-armored key as the argument to --keyring making the --dearmor step obsolete.)

You may be looking for some suggested documentation to help guide new packagers in how they might go about acquiring a suitable ascii-armored key from upstream. In that case, here's a very basic starting point using your example where upstream provides signatures on their downloads and the fingerprints to verify the signing keys, but doesn't publish the keys themselves.

A packager would download the tarball and detached signature (foo-1.0.tar.gz and foo-1.0.tar.gz.asc, let's say). Attempting to verify a signature on foo for the first time, they would get output something like this:

{{{
$ gpg --verify foo-1.0.tar.gz.asc foo-1.0.tar.gz
gpg: Signature made Wed 04 May 2016 03:58:03 PM EDT using RSA key ID BEAF0CE3
gpg: Can't check signature: public key not found
}}}

They could then download the key from a keyserver:

{{{
$ gpg --recv-keys BEAF0CE3
gpg: requesting key BEAF0CE3 from hkp server keys.gnupg.net
gpg: key BEAF0CE3: public key "Todd M. Zullinger tmz@pobox.com" imported
gpg: no ultimately trusted keys found
gpg: Total number processed: 1
gpg: imported: 1 (RSA: 1)
}}}

Then they could verify the fingerprint published by upstream:

{{{
$ gpg --fingerprint BEAF0CE3
pub 2048R/BEAF0CE3 2006-07-04
Key fingerprint = 2067 23CE 8C4A 9D39 9FFF B8E7 4325 938B BEAF 0CE3
uid Todd M. Zullinger tmz@pobox.com
sub 4096R/8550ADF3 2006-07-04 [expires: 2017-12-21]
}}}

Assuming it matches, the packager exports the key to an ascii-armored file, using options to minimize the key (which removes unneeded signatures on the key, photo id's, and other parts which are not needed to verify signatures made by this key):

{{{
$ gpg --armor --export-options export-minimal --export BEAF0CE3 > foo-signing-key.asc
}}}

At this point foo-signing-key.asc is ready to import to git. Being small and ascii-armored, I think the key is best placed in git rather than in the lookaside cache, as git makes any changes to the key more obvious. The key is also unlikely to change frequently. (The diff of changes to an ascii-armored key aren't exactly human-readable, but I think it would be harder to miss someone changing a key file in git than in the lookaside cache, as the only visible change in the lookaside case would be the update of the hash in the sources file.)

Things are much simpler if upstream provides a copy of their key on their website too, which more than a few do.

Hopefully this is a decent starting point and provides more illumination than confusion.

Also, to be clear, when using the ascii-armored key file, the verify macro(s) would then use the gpg2 --dearmor $key command followed by gpgv2 --keyring $key.gpg.

If we need to make the macros understand both an ascii-armored key file or a binry gpg keyring, I imagine the lua code could look for the -----BEGIN PGP PUBLIC KEY BLOCK----- and if it was present use the gpg2 --dearmor command. If it's not present, assume it's a binary gpg keyring.

(Or make the verify macro take options if the preference is to make the packager specify which format is being used explicitly. I don't have a strong opinion on that.)

I never intended to say that the keys should ever be automatically fetched from a keyserver. I only wanted to know how the keyring in the youtube-dl package was generated, since the packager neglected to document that. --recv-keys is what I was looking for. BTW, if upstream gives multiple key IDs (as youtube-dl does), I assume you just --recv and --export all of them.

The macros I gave in comment 34 already look through and classify all of the provided source files by searching for "BEGIN PGP PUBLIC KEY BLOCK" and "BEGIN PGP SIGNATURE". The code checks the first ten lines of each file, though I should make that configurable. I don't know how common it is for those to not appear near the beginning of each file. You can always provide file,sig or file,sig,key tuples if the automatic detection doesn't work. (And I haven't actually implemented the automatic mode anyway, though that shouldn't take long.)

Based on what's in here, I will try to cook up some kind of a rough estimate of what I think a guideline would look like. Someone who actually knows what they're doing with gpg should certainly double check, though.

Replying to [comment:35 tibbs]:
...

Based on what's in here, I will try to cook up some kind of a rough estimate of what I think a guideline would look like. Someone who actually knows what they're doing with gpg should certainly double check, though.

I've just applied the PackagingDrafts:GPGSignatures to my cryptobone.spec
and I see a few issues to be clarified:

The %prep SHOULD be:

%prep

gpg2 --no-default-keyring --keyring %{SOURCE2} --verify %{SOURCE1} %{SOURCE0}

because without the option --verify it won't work. In addition to this, the manual says that if you'd like to make sure that ONLY the keyring given
by --keyring file is used, you need to add the option --no-default-keyring.
Without this option the additional keyring is added to the keys already in use. That could cause a confusion that should be prevented.

man gpg:
"Note that this adds a keyring to the current list. If the intent is to use the specified keyring alone, use --keyring along with --no-default-keyring."

Then this generates a problem with the binary gpgv2 which is not the same
as gpg2. gpgv2 complains that "--no-default-keyring" is not a valid option which is certainly not true for gpg2. As the buildrequires mentions gnupg2, it is only logical to use gpg2 (not gpgv2).

In the draft text the 40 character hex string should not be called an ID,
because it is the fingerprint ( actually the sha1sum of the public key). The ID is only the last 8 characters of the fingerprint. This terminology has been applied since the early days of PGP.

The xzcat line should also be corrected.

Yeah, that draft isn't realy what I was talking about writing, since I'd like to show what it would look like if the %gpg_verify macro was feature-complete and actually in the default buildroot so that you could use it. Plus, I don't think that draft has been updated at all based on the discussion in this ticket. We've kind of moved on a bit.

If you wouldn't mind, have a look the end of https://pagure.io/gpg-macros/blob/master/f/macros.gpg and see if I'm calling gpg correctly. I know that does pass on valid signatures and fail for invalid ones, at least.

Replying to [comment:34 tmz]:

If we need to make the macros understand both an ascii-armored key file or a binry gpg keyring, I imagine the lua code could look for the -----BEGIN PGP PUBLIC KEY BLOCK----- and if it was present use the gpg2 --dearmor command. If it's not present, assume it's a binary gpg keyring.

I experimented a little and found that the --dearmor command does not complain if the input isn't actually ASCII-armored. I passed it a binary keyring and it wrote out a copy of the keyring. Thus it seems like the macro can support both formats by simply always doing the --dearmor step.

Replying to [comment:35 tibbs]:

The macros I gave in comment 34 already look through and classify all of the provided source files by searching for "BEGIN PGP PUBLIC KEY BLOCK" and "BEGIN PGP SIGNATURE". The code checks the first ten lines of each file, though I should make that configurable. I don't know how common it is for those to not appear near the beginning of each file.

I suppose somebody might publish their key as a pre-formatted block embedded in a longer web page, but in that case I think it's reasonable for the packager to cut out the key block and save it in a file of its own.

Replying to [comment:36 senderek]:

Then this generates a problem with the binary gpgv2 which is not the same as gpg2. gpgv2 complains that "--no-default-keyring" is not a valid option which is certainly not true for gpg2. As the buildrequires mentions gnupg2, it is only logical to use gpg2 (not gpgv2).

gpgv2 is a tool for verifying a signature assuming that all the keys in the keyring are trusted, which is exactly what we want here. Using --homedir with a temporary directory should ensure that it doesn't use any default keyring.

gpg2 --verify also works, but it prints a warning that we might not want.

Replying to [comment:37 tibbs]:

If you wouldn't mind, have a look the end of https://pagure.io/gpg-macros/blob/master/f/macros.gpg and see if I'm calling gpg correctly. I know that does pass on valid signatures and fail for invalid ones, at least.

Thanks for the link!

Line 298 of your macro looks fine to me, it's what I have proposed, it uses gpg2 and --verify together with both keyring options.
I have to admit that lua is a bit new to me, so I cannot comment on the
way the arguments are generated. That said, I don't know if the automatic
option will work under all circumstances. So I would prefer the triple argument call where everything is explicitly provided.
But that's only my 2c.

Can you give me a hint how to test your macro locally on my laptop. Is that possible?

Replying to [comment:40 senderek]:

Sure, you can test things locally. If you check out the pagure repo, you can just edit the spec and run fedpkg prep/fedpkg mockbuild directly in there. (Assuming I didn't commit something which breaks things, which isn't unlikely.) The spec pulls in the macros automatically. Note that those macro files are in a format suitable for %include'ing; I will convert them to the proper form for living in %rpmmacrodir once they're ready.

My plan for the "automatic" thing is that if it found any signatures at all in the sources table, it must find a corresponding source file (by stripping any extension) and they must all verify. I think that would fail closed appropriately. It only falls down if you have multiple keyrings, and at that point you'd have to just specify things manually.

Re the --dearmor trick, is that something upon which we can actually rely? I'd hate for the behavior to change and break things. I could also just not dearmor if the keyring was manually specified but doesn't look like a key.

Note also that I'm calling gpg2 in the macro, not gpgv2. What's the warning that gpg2 gives which is unwanted?

Replying to [comment:41 tibbs]:

Replying to [comment:40 senderek]:

Sure, you can test things locally.

I'll try that tomorrow.

Re the --dearmor trick, is that something upon which we can actually rely? I'd hate for the behavior to change and break things. I could also just not dearmor if the keyring was manually specified but doesn't look like a key.

I'll check that too.

Note also that I'm calling gpg2 in the macro, not gpgv2. What's the warning that gpg2 gives which is unwanted?

For instance:

  • gpg2 --no-default-keyring --keyring /x/GIT/cryptobone/rpmbuild/SOURCES/gpgkey-3274CB29956498038A9C874BFBF6E2C28E9C98DD.gpg --verify /x/GIT/cryptobone/rpmbuild/SOURCES/cryptobone-1.0.2.tar.gz.asc /x/GIT/cryptobone/rpmbuild/SOURCES/cryptobone-1.0.2.tar.gz

gpg: Signature made Thu May 5 15:08:37 2016 CEST using RSA key ID 8E9C98DD

gpg: Good signature from "Senderek Web Security Codesigning Key opensource@senderek.ie"

gpg: WARNING: This key is not certified with a trusted signature!

gpg: There is no indication that the signature belongs to the owner.

This warning is CORRECTLY being generated, because the keyring does not contain any trust information, so the public key is always regarded as not trustworthy. It might scare some uninformed gpg users, but it is
true.

We discussed this at this weeks meeting (http://meetbot.fedoraproject.org/fedora-meeting-1/2016-05-05/fpc.2016-05-05-16.00.txt):

  • 610 Packaging guidelines: Check upstream tarball signatures


    (geppetto, 16:18:33)
  • Please try the signature checking macros:
    https://pagure.io/gpg-macros (geppetto, 16:19:40)

Oops, somehow I broke the ability to fedpkg prep in that repo. I'll try to get it fixed.

Thanks @tibbs, this is shaping up nicely.

I agree that gpgv2 would be better in this case than gpg2 (for the actual verification, not for the --dearmor or any other setup commands). Using gpgv2 avoids the spurious warnings about trust, as we are explicitly trusting the keys used for the source verification. This is the change I would suggest (modulo any improvements to the lua string concatenation):

{{{
diff --git i/macros.gpg w/macros.gpg
index afd1e48..ff7dd2e 100644
--- i/macros.gpg
+++ w/macros.gpg
@@ -295,7 +295,7 @@
print('gpg2 --homedir $GPGHOME --no-default-keyring --quiet --yes ')
print('--output '.. gpgfile .. ' --dearmor ' .. arg.keyfile .. "\n")
print('ls -lR $GPGHOME\n')
- print('gpg2 --homedir $GPGHOME --no-default-keyring --keyring ' .. gpgfile .. ' --verify ' .. arg.sigfile .. ' ' .. arg.srcfile .. '\n')
+ print('gpgv2 --homedir $GPGHOME --quiet --keyring ' .. gpgfile .. ' ' .. arg.sigfile .. ' ' .. arg.srcfile .. '\n')
print('rm -rf $GPGHOME\n')
end
}}}

I manually downloaded the exim source files and tested with rpmbuild -bp gpg-macros.spec. The first gpg_verify command works well (%gpg_verify 0,2,3). The other two both produce errors. I don't know if they're intended to work yet or not. Here's what I get with each of them:

{{{
No default key yet
Checking 0,2
Looks OK; needs default key
error: No default key specified or found, yet the arguments require one.
error: lua script failed: [string "<lua>"]:14: bad argument #1 to 'gsub' (string expected, got nil)
error: line 36: %gpg_verify 0,2
}}}

{{{
t[1]
t[2]
/home/tmz/src/packages/gpg-macros/Exim-Maintainers-Keyring.asc
error: Provided default keyring 3 does not appear to contain a keyring.
Defkey: /home/tmz/src/packages/gpg-macros/Exim-Maintainers-Keyring.asc
error: lua script failed: [string "<lua>"]:14: bad argument #1 to 'gsub' (string expected, got nil)
error: line 39: %gpg_verify -f 0 -s 2 -k 3
}}}

I am testing on F22, FWIW.

Yeah, I fixed the repo up. get-sources will download everything for you. spectool really doesn't like that spec for whatever reason; I suspect it doesn't know how to include the files. That method is a hack but it works well enough for testing.

And I think you just cought the repo in an odd state. I'm committing all the time so make sure you pull if you run into some issue.

A few hours ago I committed support for "full auto" mode. If you have any signature files (either those containing "BEGIN PGP SIGNATURE" or, because of non-armored signatures, ending in ".sig") then they must all have a matching source file, and all are verified. For the basic case of exim:

{{{
Source: ftp://ftp.exim.org/pub/exim/exim4/exim-4.87.tar.bz2
Source2: ftp://ftp.exim.org/pub/exim/exim4/exim-4.87.tar.bz2.asc
Source3: http://ftp.exim.org/pub/exim/Exim-Maintainers-Keyring.asc
}}}
or youtube-dl:
{{{
Source10: https://yt-dl.org/downloads/2016.05.01/youtube-dl-2016.05.01.tar.gz
Source11: https://yt-dl.org/downloads/2016.05.01/youtube-dl-2016.05.01.tar.gz.sig
Source12: gpgkey-7D33D762FD6C35130481347FDB4B54CBA4826A18.gpg
}}}
this simply works without doing anything besides sticking %gpg_verify somewhere in prep.

I went ahead and played with %autosetup, but I think there's a bug in %autopatch which corrupts the argument list somehow. It's probably unnecessary but I felt like experimenting anyway.

FYI

I spent the best part of the morning to convince KOJI to build my cryptobone on F23-F25 with GPG source code signature checks.
I used my armored key block from my web site and eventually arrived at this solution:

Source0: https://crypto-bone.com/release/source/cryptobone-%{version}-1.tar.gz

Source1: https://crypto-bone.com/release/source/cryptobone-%{version}-1.tar.gz.asc

Source2: gpgkey-3274CB29956498038A9C874BFBF6E2C28E9C98DD.asc

and

%prep

KEYRING=$(echo %{SOURCE2})

KEYRING=${KEYRING%%.asc}.gpg

mkdir -p .gnupg

gpg2 --homedir .gnupg --no-default-keyring --quiet --yes --output $KEYRING --dearmor %{SOURCE2}

gpg2 --homedir .gnupg --no-default-keyring --keyring $KEYRING --verify %{SOURCE1} %{SOURCE0}

%setup

On rawhide, KOJI worked without the mkdir .gnupg and --homedir .gnupg
and created this directory itself, but on F24 the build failed and I had to add the mkdir .gnupg to the specfile.

Replying to [comment:41 tibbs]:

Re the --dearmor trick, is that something upon which we can actually rely? I'd hate for the behavior to change and break things.

I suppose it can be argued that one should only rely on what the manual says, which is that --dearmor decodes ASCII armor. On the other hand GnuPG seems to take both backward and forward compatibility very seriously. They're even still maintaining the version 1 series nine years after version 2 was released.

I could also just not dearmor if the keyring was manually specified but doesn't look like a key.

That would also be sensible.

Replying to [comment:42 senderek]:

gpg: WARNING: This key is not certified with a trusted signature!
gpg: There is no indication that the signature belongs to the owner.

This warning is CORRECTLY being generated, because the keyring does not contain any trust information, so the public key is always regarded as not trustworthy. It might scare some uninformed gpg users, but it is true.

It's true when the key is found in your personal keyring in your home directory. Your keyring may contain many keys of unknown origin, and you certify the ones you have verified by signing them with your own key. But what we're trying to do here is to verify a signature against a specific key. The fact that the maintainer has added the key to the repository means that the build process shall trust that key.

Replying to [comment:45 tmz]:

{{{
+ print('gpgv2 --homedir $GPGHOME --quiet --keyring ' .. gpgfile .. ' ' .. arg.sigfile .. ' ' .. arg.srcfile .. '\n')
}}}

May I ask why you included --quiet? It doesn't seem to make any difference. Both for a valid and an invalid signature, I get the same output with --quiet as without it. If something unexpected happens that can cause additional messages to be printed, then it might be useful to see that in the build log.

Would have probably been simpler to just add the macros.gpg file as another Source: line, %include it, and use %gpg_verify. The automatic mode should work just fine with the three sources you have, and it does create a randomly named temp directory for the homedir.

I don't know why rawhide would be any different regarding the necessity of a home dirrectory.

Also, I worked around the issues with %autosetup, so it can now just call %gpg_verify invisibly. Of course, I'm not sure we'd want to push this due to the potential for breakage but it's actually not too hard to just prep the entire package set and see what goes wrong. I'd be surprised if there are more than a couple of packages where the automatic stuff wouldn't work.

I mean, getting this checking for free (just make sure signatures and the keyring are there as Source:s) seems to be a pretty nice benefit.

Replying to [comment:49 rombobeorn]:

Replying to [comment:42 senderek]:

gpg: WARNING: This key is not certified with a trusted signature!
gpg: There is no indication that the signature belongs to the owner.

This warning is CORRECTLY being generated, because the keyring does not contain any trust information, so the public key is always regarded as not trustworthy. It might scare some uninformed gpg users, but it is true.

It's true when the key is found in your personal keyring in your home directory. Your keyring may contain many keys of unknown origin, and you certify the ones you have verified by signing them with your own key. But what we're trying to do here is to verify a signature against a specific key. The fact that the maintainer has added the key to the repository means that the build process shall trust that key.

The idea is correct, but the gpg logic is against it. This warning will
only dissappear, if the key in question is either signed by a trusted key
(WebOfTrust) or if it is trustworthy by itself. Just using a keyring is
not enough to make GPG think that a key is trustworthy. To achieve this,
the trust on a key must explicitly being set and recorded in the file
.gnupg/trustdb.gpg. So without any fiddling with this database, the warning will remain forever.

Replying to [comment:50 tibbs]:

I don't know why rawhide would be any different regarding the necessity of a home directory.

Well in KOJI, rawhide created it automatically, while F24 didn't.

I was busy building new cryptobone packages today, so I didn't get the chance to explore and test your macro yet. Please be patient.

Just using a keyring is not enough to make GPG think that a key is trustworthy. To achieve this,
the trust on a key must explicitly being set and recorded in the file
.gnupg/trustdb.gpg. So without any fiddling with this database, the warning will remain forever.

Well there is an option that can be added to the gpg2 command to get rid of the warning

This is an example:

{{{
gpg2 --trusted-key FBF6E2C28E9C98DD --homedir /tmp --no-default-keyring
--keyring ./gpgkey-3274CB29956498038A9C874BFBF6E2C28E9C98DD.gpg
--verify cryptobone-1.0.3-1.tar.gz.asc cryptobone-1.0.3-1.tar.gz

gpg: Signature made Fri May 6 10:26:27 2016 CEST using RSA key ID 8E9C98DD
gpg: key 8E9C98DD marked as ultimately trusted
gpg: checking the trustdb
gpg: 3 marginal(s) needed, 1 complete(s) needed, PGP trust model
gpg: depth: 0 valid: 1 signed: 0 trust: 0-, 0q, 0n, 0m, 0f, 1u
gpg: Good signature from "Senderek Web Security Codesigning Key opensource@senderek.ie"
}}}

We'd need the "long key ID", which turns out to be the least significant
16 hexcharacters of the key's fingerprint.

I think this is it.

Replying to [comment:49 rombobeorn]:

Replying to [comment:45 tmz]:

{{{
+ print('gpgv2 --homedir $GPGHOME --quiet --keyring ' .. gpgfile .. ' ' .. arg.sigfile .. ' ' .. arg.srcfile .. '\n')
}}}

May I ask why you included --quiet? It doesn't seem to make any difference. Both for a valid and an invalid signature, I get the same output with --quiet as without it. If something unexpected happens that can cause additional messages to be printed, then it might be useful to see that in the build log.

At least some versions of gpgv2 benefit from --quiet. I used it when I added checking to the git package, which I tested as far back as EL5 (though EL6's gnupg2 version isn't much different).

Using --quiet won't suppress error output. If there's a problem, removing it and adding more verbosity is easy enough for a packager. Having extra output all the time doesn't serve much purpose. It may even be harmful as it trains people to skim over "all this gpg output."

Replying to [comment:53 senderek]:

Just using a keyring is not enough to make GPG think that a key is trustworthy. To achieve this,
the trust on a key must explicitly being set and recorded in the file
.gnupg/trustdb.gpg. So without any fiddling with this database, the warning will remain forever.

Well there is an option that can be added to the gpg2 command to get rid of the warning

Using gpgv2 is the much simpler way to achieve this, which is what the current macros.gpg does.

@tibbs, should fedpkg prep work without error now? Testing on f22, I still get failures:

{{{
$ git rev-parse HEAD
ce2621327d2cb22cdf2679bdb82a1f7ed2471c24

$ fedpkg prep

No default key yet
error: No keyring specified and none found; cannot auto-check.


No default key yet
Checking 0,2,3
Looks OK


error: Provided default keyring 3 does not appear to contain a keyring.
No default key yet
Checking 0,2
Looks OK; needs default key
error: No default key specified or found, yet the arguments require one.
error: lua script failed: [string "<lua>"]:8: bad argument #1 to 'gsub' (string expected, got nil)
error: line 41: %gpg_verify -k 3 0,2

Could not execute prep: Command 'rpmbuild --define '_sourcedir /home/tmz/src/gpg-macros' --define '_specdir /home/tmz/src/gpg-macros' --define '_builddir /home/tmz/src/gpg-macros' --define '_srcrpmdir /home/tmz/src/gpg-macros' --define '_rpmdir /home/tmz/src/gpg-macros' --define 'dist .fc25' --define 'fedora 25' --eval '%undefine rhel' --define 'fc25 1' --eval '%undefine fc22' --nodeps -bp /home/tmz/src/gpg-macros/gpg-macros.spec' returned non-zero exit status 1
}}}

It works without errors on F23, at least. For me, at least, on a fresh checkout. I got rid of my last F22 machine the other day.

I'll try to fire up a VM some time soon, but honestly, I don't think there's much chance of pushing these macros into F22 given the EOL date.

I can duplicate on EL7 and with fedpkg --dist f22 mockbuild, at least, so I'll dig into it. I suppose I must be relying on some rpm 4.13 behavior, but I'm not doing so intentionally.

Replying to [comment:50 tibbs]:

I don't know why rawhide would be any different regarding the necessity of a home dirrectory.

Well it might be a difference between GnuPG 2.1.11 and 2.1.12.

Replying to [comment:51 senderek]:

The idea is correct, but the gpg logic is against it. This warning will
only dissappear, if the key in question is either signed by a trusted key
(WebOfTrust) or if it is trustworthy by itself. Just using a keyring is
not enough to make GPG think that a key is trustworthy. To achieve this,
the trust on a key must explicitly being set and recorded in the file
.gnupg/trustdb.gpg. So without any fiddling with this database, the warning will remain forever.

Right, but gpgv2 uses different logic that matches our use case better. With gpgv2 using a keyring ''is'' enough, because it assumes that the keyring you give it contains only trustworthy keys, and therefore it doesn't print the warning. gpgv2 exists for this very purpose. Why should we not use the tool that meets our needs best?

And perhaps I should point out that gpgv2 is a part of GnuPG version 2, in case someone thinks it's a different and less trustworthy program or something:

{{{
$ rpm -q --file which gpg2
gnupg2-2.1.11-1.fc23.x86_64
$ rpm -q --file which gpgv2
gnupg2-2.1.11-1.fc23.x86_64
}}}

Replying to [comment:58 tibbs]:

I can duplicate on EL7 and with fedpkg --dist f22 mockbuild, at least, so I'll dig into it. I suppose I must be relying on some rpm 4.13 behavior, but I'm not doing so intentionally.

Cool, I just wasn't sure if it was expected, so I hadn't done testing on other releases yet either. Do you think these changes will make it back to any EPEL releases (not necessarily immeditately, of course)? If not, I certainly agree it really isn't worth much time for f22, which will be EOL so soon.

I'll run further tests on f24 and/or an f23 mock instance.

So this is, it seems, due to another in a long line of bizarre RPM behaviors. In RPM 4.12 and older, the location of the BuildArch: tag has a profound effect on the generation of the sources and patches tables which were added in rpm 4.6. Specifically if BuildArch: is above the Source: and Patch: lines then somehow those necessary tables aren't generated.

Isn't extreme rpm programming fun? Actually, you don't have to get all that extreme; autosetup doesn't even work if you put BuildArch: too high up. Anyway, I've pushed a fix.

Oh, and another bit of trivia: RPM doesn't accept an empty %files list for an archful build, which is why I added BuildArch: in the first place.

Just noting that I owe the committee a draft on the use of %gpg_verify.

I found a round tuit lying about so I played with gpg_verify a bit.

In my testing so far, the macro works well when called correctly, but the error messages I get when there's something wrong with the parameters are rather unhelpful. If I give it a pair where one of the numbers isn't a source number, then I get these messages:
{{{
error: lua script failed: [string "<lua>"]:118: attempt to concatenate a nil value (field 'sigfile')
error: line 56: %gpg_verify 1,2

error: query of specfile /path/to/anet.spec failed, can't parse
}}}

If I give it a triple where one of the numbers isn't a source number, then I get this:
{{{
error: lua script failed: [string "<lua>"]:8: bad argument #1 to 'gsub' (string expected, got nil)
error: line 56: %gpg_verify 1,2,3

error: query of specfile /path/to/anet.spec failed, can't parse
}}}

Those need to be replaced with a message that states clearly that x isn't a valid source number.

If I give it just a single number (perhaps believing that I can specify only the signature and the corresponding signed file will be found automatically, which someone will inevitably believe because that usually works with GnuPG), then the message "Provided argument 1 is not valid." is printed several times, but it's not at all clear that that message is from gpg_verify, which makes it hard to understand what argument it complains about.

Furthermore, I think that named parameters instead of a comma-separated list would make the macro easier to use and read. It's hard to remember in which order the arguments shall be given, especially since the signature and the signed file shall be given in reverse order from what GnuPG accepts. I would prefer something like
{{{
%gpg_verify --keyring=1 --signature=2 --data=3
}}}
if such a syntax is possible for RPM macros, or else
{{{
%gpg_verify -k 1 -s 2 -d 3
}}}

Passing a pair or a triple of filenames to gpg_verify works, but if I use macros such as %{SOURCE3}, which expand to full pathnames, then I get the same error messages as when a source number is invalid – even if the numbers in the macros are correct.

And by the way, the way that gpg_verify and README.rst use the term "default keyring" is confusing. It's too easily understood as GnuPG's default keyring, {{{~/.gnupg/pubring.gpg}}}, and a value that's specified in a command line isn't actually a default.

For the second comment, please remember what I've said all along: I know zilch about gpg. So there is relatively little chance that I have used gpg-related terminology with any precision, and things that make sense to me might not make sense to someone who knows about gpg.

Or, to put it another way, if that term is confusing then please tell me what phrasing I should use instead.

To be able to tell you what phrasing to use I need to fully understand what you intended when you wrote "default".

A keyring can be provided in three ways if I understand correctly:
1. It can be specified as the third element of a triple.
2. It can be specified with -k.
3. It can be found automatically if it's ASCII-armored or has the suffix ".gpg".

Cases 1 and 2 seem equivalent to me. The keyring is specified explicitly in both cases. I'd probably call it "the specified keyring" or "the given keyring", or just "the keyring" depending on the context. But you seem to call it "default keyring" in case 2 but not in case 1. I assume that you had some kind of intention with that.

Case 3 is different. Here the keyring is not specified explicitly. I'd call it something like "autodetected" or "found". You seem to treat cases 2 and 3 together, and I haven't yet figured out why.

Replying to [comment:66 rombobeorn]:

To be able to tell you what phrasing to use I need to fully understand what you intended when you wrote "default".

In this context, advice on the phrasing is really easy:
The "default keyring" should be referred to as the "trusted keyring" or
the "upstream keyring" (I prefer the former) because the use of gpgv2 in the macro removes all warnings about lacking trust in the keyring that the use of gpg2 would produce.

An attacker that manages to get the wrong information into what's called the default key has the ability to fake a signature on the source code.

That's the reason why the keyring (preferably in the form of an ascii armored file, not a binary) must be provided via a trusted source. i.e DIST-GIT and not via a URL. The upstream source code and it's accompanying (detached) signature may well be downloaded from a web site, but providing the (default aka trusted) keyring from the same source should always be regarded as a mortal sin.

For this reason alone, the %gpg_verify macro should always require a --trustedkeyring=N option that is set correctly by the packager to a file that he has to check thoroughly before adding it to source.
In this case, the most important thing %gpg_verify has to do is to make sure that this source file is not an URL.

In my opinion, calling %gpg_verify without any options tries to simplify things that (partially) are the responsibility of the packager.

Let's find out why there is a need for a fully automated solution, where the packager can add anything between the necessary triple and nothing to the spec file and the macro has to find out and reliably act in every conceivable situation?

I'm open for suggestions, but IMHO providing the triple information would be the only option we'd really need to get the job done. Everything
else would complicate things.

Replying to [comment:63 rombobeorn]:
I would prefer something like

{{{
%gpg_verify --keyring=1 --signature=2 --data=3
}}}
if such a syntax is possible for RPM macros, or else
{{{
%gpg_verify -k 1 -s 2 -d 3
}}}

I would very much support this approach, with the addition to mandatory checking that 1 is not an URL.

Jason, I'd like to comment on your lua script "macro.gpg".

But first things first: I'm not familiar with lua, I'm reading it as a form of improved python.

In case a user specifies -k keyring, the variable defkeyspec is set and in
line 214 the variable defkey is set:

{{{
defkey = check_sources_list({defkeyspec})[1]
}}}

The following code exits with the rpmerror function, if either defkey is empty or if it does not exist in the keyring_table, that you've created by parsing the arguments before. But line 220 is dead code, because the macro exits with the rpmerror in line 219. I think you might have missed an additional else here.

{{{
215 if not defkey then
216 rpmerror('Provided default keyring ' .. defkeyspec .. ' is not a valid source number.')
217 \
218 elseif not keyring_table[defkey] then
219 rpmerror('Provided default keyring ' .. defkeyspec .. ' does not appear to contain a keyring.')
new else
221 defkey = nil
222 end
223 end
}}}

Otherwise defkey may not be defined in line 233, if it is initially empty and nothing can be found in the datastructure keyring at position 1 in line 229.

I haven't found the time to explore if this creates an issue by feeding the macro the appropriate input, but lines 256 and 284 seem to assume that defkey at that point contains a trusted keyring.

Sorry James,

I forgot to mention that I'm very impressed by your attempt to handle all these different use cases of the macro in lua.

I think this very important piece of macro code is on a good way to become a real help to improve the security of packaging, but at the same time I think that reducing complexity and looking for the holes that may create an opportunity for an attacker is necessary. This has to be rock-solid, so testing is essential. And I'll promise to help, if I can.

To answer the rest:

Some of that is just bad input validation and I'll try to fix up some more cases once I finish the Fedora mirror optimization thing I'm working on.

Regarding long options, RPM macro definitions don't support that and doing full option parsing manually in Lua is beyond what I'm really willing to do. It is certainly possible, though, if someone wants to do the work. But note that no other RPM macro in use takes long options.

I had originally implemented the "all parameter" syntax, but took it out because that works for checking only one signature so all of the extra code for that one case seemed quite pointless. Especially since if you have one file, one signature and one key, you don't need to provide any parameters at all. I htin

I hope I never gave the impression that passing %SOURCE10 to any of these macros would work. I could make it work, I guess; it's just more code. The %SOURCE macros expand to full pathnames, not filenames. So if the macro is going to actually take a pathname, then I have to figure out whether to take any pathname, always take the basename, how to handle a relative path, etc.)

And some of that is just poor input validation, which I will work on fixing. But in general I think you're expecting a bit much from the functionality that I can reasonably write into this macro when it's already up there with the most insanely complicated macros around.

I think the more fundamental question is: Is there actually a case where just using the macro with no arguments fails to do the right thing? I would hope that it would handle all but the weirdest cases?

I made some tests with the %gpg_verify macros. Here are the results:

== Tests with two different signing keys and two different sources. ==

I deleted everything from prep except a single %gpg_verify without parameters :

Result: the macro failed as expected, because the second source was verified with the first key found in the keyring-table (not with gpgkey-7D33D762FD6C35130481347FDB4B54CBA4826A18.gpg).

The two different sources are correctly verified using different keyrings with:

{{{
%gpg_verify 0,2,3 10,11,12
}}}

The following tests all gave the expected results:

{{{
%gpg_verify 10,11,12 (one good sig)
%gpg_verify 0,2,3 0,11,3 (failed: No public key)
%gpg_verify -k 3 0,2 0,11 (failed: No public key)
%gpg_verify -k 3 0,2 10,11,12 (two different good sigs)

}}}

== Tests with one single signing key and multiple signed sources: ==

I deleted all exim and youtube files and created a test signing key and signed three different files. I put these files into the spec instead of the old sources and used the macro without parameters
{{{
%gpg_verify
}}}

Results: All tests gave the expected three good signatures:

{{{
A: when signingkey is Source4 and all other files start from Source5
upwards without gaps.

B: when signature or data files were swapped in order

C: when there were gaps in the numbering

D: when the signing key had a lower number and was placed after a higher
numbered sig or data file

E: when there were great gaps between data file and signature and other
data placed in between

F: and even if the (only) keyfile was placed somewhere in the middle.

}}}

Every time I got three good signatures.

== Tests with false data, missing data or missing signatures ==

'''Good News'''

Tests with manipulated source files failed as expected (gpgv2 failed)
Tests with missing source files failed as expected, because the macro detects that there is no matching source file for the signature file.

'''Bad News'''

Unfortunately, a missing signature file lets the macro continue with the
remaining two good signatures. It does not detect that there is no matching sig file missing. That means that unsigned sources will go through undetected.

''But maybe this '''is intended''' to make the transition to signed source code smooth for all users and to allow for mixing signed and unsigned source code.''

I'd suggest that for the (desired) use case, where all sources must be verified to let the build process commence, an option -a (all) can cover this?

I will be looking into all of these, but might I ask if you'd be willing to file issues in pagure for any of these? It would be much easier for me to keep track of them that way. The repo is at https://pagure.io/gpg-macros and your regular Fedora account gets you in.

Feel free to send pull requests, too. I know that few people understand the Lua or the RPM internals but maybe there's a chance....

Replying to [comment:67 senderek]:

For this reason alone, the %gpg_verify macro should always require a --trustedkeyring=N option that is set correctly by the packager to a file that he has to check thoroughly before adding it to source.
In this case, the most important thing %gpg_verify has to do is to make sure that this source file is not an URL.

If upstream publishes a keyring then its URL shall be included in the package one way or another, so that anyone can see where the keyring came from and compare it to the source. Forcing a packager to write only the filename instead of the URL doesn't in any way force them to check the keyring.

Replying to [comment:71 tibbs]:

I had originally implemented the "all parameter" syntax, but took it out because that works for checking only one signature so all of the extra code for that one case seemed quite pointless.

Ah OK, I hadn't understood that it accepts multiple pairs or triples. That makes it easier to understand why some things are the way they are. It does seem a little bit like optimizing for the rare case though.

I hope I never gave the impression that passing %SOURCE10 to any of these macros would work. I could make it work, I guess; it's just more code. The %SOURCE macros expand to full pathnames, not filenames. So if the macro is going to actually take a pathname, then I have to figure out whether to take any pathname, always take the basename, how to handle a relative path, etc.)

It's clearly not necessary to use the SOURCE macros when one can use just the numbers. If pathnames would be needed, then it would be in some really weird case where a source tarball or signature is in a subdirectory inside another tarball. I don't think you need to design for that hypothetical case. It would be possible to work around anyway. But people ''will'' misunderstand and try to use SOURCE macros or pathnames, so informative error messages are necessary.

I think the more fundamental question is: Is there actually a case where just using the macro with no arguments fails to do the right thing? I would hope that it would handle all but the weirdest cases?

The likely case that I can think of is when the keyring and/or the signature is unarmored and doesn't have the expected suffix. Especially keys are likely to have filenames ending in ".key" or ".pub" or something else. GnuPG exports keys to the standard output stream or to the file the user specifies, and doesn't enforce any particular suffix, so the filename is entirely up to the user.

Replying to [comment:75 rombobeorn]:

Replying to [comment:67 senderek]:

For this reason alone, the %gpg_verify macro should always require a --trustedkeyring=N option that is set correctly by the packager to a file that he has to check thoroughly before adding it to source.
In this case, the most important thing %gpg_verify has to do is to make sure that this source file is not an URL.

If upstream publishes a keyring then its URL shall be included in the package one way or another, so that anyone can see where the keyring came from and compare it to the source.

Yes, the url doesn't hurt unless it is the one and only source which is been used during gpg verification for the keyring.

I gave the reason above (comment # 67).

The situation we need to avoid seriously is where all three pieces of information originate from the same website as a URL, and are used as a URL by the macro, because in this case we make it easy to subvert the verification. An attacker only needs to forge the website. If we insist on the keyring being provided via DISTGIT, then and only then the system is much, much safer.

Forcing a packager to write only the filename instead of the URL doesn't in any way force them to check the keyring.

No it doesn't. A sloppy packager can download a keyring from the web and store it in DISTGIT without any checking. This is undesirable (and will happen) but it's not preventable at all, unfortunately. The most we can do is make the fingerprint verification of the signing key a mandatory task in the packaging guidelines. It may at least induce a conciousness, that the packager has done something risky.

We discussed this at this weeks meeting (http://meetbot.fedoraproject.org/fedora-meeting-1/2016-06-30/fpc.2016-06-30-16.00.txt):

  • 610 Packaging guidelines: Check upstream tarball signatures


    (geppetto, 16:12:44)
  • ACTION: tibbs Push it to rawhide, so people can try using it
    (geppetto, 16:17:48)

Thank you so much for implementing all this. It looks great.

One question: do we recommend that the keyring be included in the lookaside cache, or directly as an ASCII-armoured file in the git tree?

I tend to favour the latter now that you've done the work to make it use ASCII keyrings directly.

To be fair, I am completely unable to weigh the security considerations here. The macro shouldn't care either way as long as the files exist.

Strictly speaking, there isn't that much of a difference ''(setting aside the lookaside-still-validated-by-obsolete-md5 issue)''. It all comes down to "what's in the git tree".

However, if a possible attack model is to change the trusted keyring at the same time as introducing hacked sources, then it seems much easier to hide that in a single update to the sources file when it's all in lookaside together.

Having the keyring actually present in the git tree directly, makes it more obvious when that changes. And perhaps also helps to avoid the mistake I made with the openconnect package this week, when I used 'fedpkg new-sources' on the new tarball+sig, but forgot to include the keyring. So the keyring was removed from the sources, my local build succeeded but the mock build failed.

So the arguments for having it directly in the git repository are relatively minor, but on balance it probably makes sense to suggest doing it that way.

I'm back from vacation and flock now and I'd like to actually get these macros going. I submitted a review for fedora-rpm-macros where I can store this stuff without having to mess with redhat-rpm-config. Plus the monolithic package really isn't maintainable anyway.

The review ticket is https://bugzilla.redhat.com/show_bug.cgi?id=1366411

It should be a trivial review since the package builds nothing and contains a single file. I even included the rpmlint output with explanation.

OK, fedora-rpm-macros (with no actual macro content) is now in rawhide, and redhat-rpm-config now depends upon it. This conveniently gives us a way to do some fedora-specific distro wide macros without touching redhat-rpm-config. So as soon as I work out a few things, these macros will show up in rawhide.

Before I go file a bug against fedpkg, is there anything I'm doing stupidly wrong in OpenConnect? It used to work but now (on F25) when attempting to do 'fedpkg update' for the F25 branch I get this...

{{{
$ fedpkg update
Found source: /home/dwmw2/fedora/openconnect/f25/openconnect-7.08.tar.gz
Found source: /home/dwmw2/fedora/openconnect/f25/openconnect-7.08.tar.gz.asc
Found keyring: /home/dwmw2/fedora/openconnect/f25/gpgkey-BE07D9FD54809AB2C4B0FF5F63762CDA67E2F359.asc
Found source: /home/dwmw2/fedora/openconnect/f25/macros.gpg
No common key yet
Using first found keyring file: /home/dwmw2/fedora/openconnect/f25/gpgkey-BE07D9FD54809AB2C4B0FF5F63762CDA67E2F359.asc


Could not execute update: error: Unable to open /home/dwmw2/rpmbuild/SOURCES/macros.gpg: No such file or directory
error: query of specfile /home/dwmw2/fedora/openconnect/f25/openconnect.spec failed, can't parse
}}}

The spec is fairly simple:
{{{
gpgkey-BE07D9FD54809AB2C4B0FF5F63762CDA67E2F359.asc
Source3: macros.gpg
...
%include %SOURCE3
%prep
%if 0%{?gitcount} == 0
%gpg_verify
%endif
}}}

%include in itself is problematic because it means the spec can no longer be parsed as an independent entity.

Spec parse does not usually need sources or patches present, but this changes the picture entirely as %gpg_verify is evaluated during spec parse and requires the (some of) the sources to be present, eg without the gpgkey source file it barfs up and the spec is not parseable at all.

Something that's to be done in %prep would be better done as an external (shell) script that is really never invoked outside real %prep execution. Some macro glue/wrapper might well be necessary for that too but rpm macros are not generally intended for doing arbitrary work, rpm macro engine is best considered as a somewhat peculiar text preprocessor.

Metadata Update from @dwmw2:
- Issue assigned to tibbs

7 years ago

Metadata Update from @tibbs:
- Issue close_status updated to: None
- Issue tagged with: committee

7 years ago

The current draft: https://fedoraproject.org/wiki/PackagingDrafts:GPGSignatures is working pretty well in the simple case with one source tarball, detached sig and a GPG keyring or ASCII-armored key. Can we go with the draft since the macro development seems to be stuck? @tibbs @dwmw2 .

@senderek Could you take a look at the bottom of the draft page? gpg2 has an option (--import-options import-export) to "convert" an ASCII-armored key into a keyring.

@rathann, --import-option import-export mentioned in the draft is only available in gnupg >= 2.1.14. That means it won't work for EL-6, EL-7, or F-25 (though the latter will be EOL soon). Is there a reason to prefer that over:

gpg2 --dearmor --quiet --batch --yes $ascii_key_file >/dev/null

which works on EL-6, EL-7, and all current Fedora releases (F-25 - F28 at this time)? I've been using the latter without issue in the git packages for the past year and a half, building for all Fedora and EL releases (EL builds in copr, to provide current git for those who don't want the older version provided in EL).

@senderek Could you take a look at the bottom of the draft page? gpg2 has an option (--import-options import-export) to "convert" an ASCII-armored key into a keyring.

Having re-read my (two year old) comments , I'd like to object to the way you draft the verification with gpgv2, because if you lack to use an empty HOMEDIR
the option --keyring alone does add the keyring stored in the file to the one
that may be in the existing HOMEDIR. If we do not require the key file to be
the only trusted file containing keys, we're asking for trouble. See the comments
above.

@tmz I didn't know that. Would using --no-tty fix the noisy output issue? What does that line actually do? Convert the key from ASCII-armored to a binary keyring? If yes, it's not easy to guess that. Hence, I still think we should use the newer option in Fedora and make a note for EPEL.
@senderek I agree with your comment. Could you make the appropriate changes to the Draft?

@tmz I didn't know that. Would using --no-tty fix the noisy output issue?

You mean instead of using >/dev/null? I believe the options cover us and the >/dev/null was there to be cautious and certain that no potentially confusing output was shown.

I can't reproduce any output to stdout when running in mock (locally or in koji) on either EL-6 or EL-7 currently, even removing the --quiet and --batch options. I'm not sure what has changed since, but I get no warnings with only gpg2 --dearmor $ascii_key now. I'd keep the other options just to be safe, but that's more of a "belt and suspenders" thing. :)

(For example, without --yes we'd get a prompt that the file existed when running %prep a second time. That's not relevant for mock builds, but while working locally it would come up and cause an unnecessary error/prompt.)

What does that line actually do? Convert the key from ASCII-armored to a binary keyring? If yes, it's not easy to guess that. Hence, I still think we should use the newer option in Fedora and make a note for EPEL.

Yeah, it removes the ASCII-armor. I don't particularly care what's used by default, as long as it's known and documented that using --import-options import-export only works on F-26+. We don't have to care about F-25 at this point, but even it lacks a GnuPG with such an option, so it's a very recent addition (at least in terms of stable GnuPG releases)

I'll personally use the options which work on all supported Fedora and EPEL releases in packages like git, which are intended to build on all those systems.

I honestly hope we'll see GnuPG support for using ASCII-armored keys directly and then this all becomes moot. But that's not going to happen terribly soon, if it does.

Having re-read my (two year old) comments , I'd like to object to the way you draft the verification with gpgv2, because if you lack to use an empty HOMEDIR
the option --keyring alone does add the keyring stored in the file to the one
that may be in the existing HOMEDIR. If we do not require the key file to be
the only trusted file containing keys, we're asking for trouble. See the comments
above.

I have tested this on Fedora 27 (gnupg2-2.2.1-1.fc27.x86_64). I found that if --keyring is used and contains a slash, then gpgv2 uses only that keyring and doesn't look for a default keyring.

I placed the right key in ~/.gnupg/trustedkeys.kbx and passed another key to gpgv2 with --keyring. The verification failed because the key corresponding to the signature wasn't found, which shows that gpgv2 didn't look in the default keyring.

As far as I can tell --dearmor processes only the input file and doesn't touch any keyrings.

Thus --homedir shouldn't be necessary for either of those commands, and the only purpose of including it would be to make extra double sure that no default keyring will be touched in case another version of GnuPG works differently.

I have tested this on Fedora 27 (gnupg2-2.2.1-1.fc27.x86_64). I found that if --keyring is used and contains a slash, then gpgv2 uses only that keyring and doesn't look for a default keyring.

I have successfully reproduced your test results (with version 2.1.21). The use
of --keyring filename seems to disable looking up the default keyring in the home
directory. Additional keys will only be used if the option --keyring is used multiple
times.

@rombobeorn: In the draft (section Alternatives), the code is correct and fool-prove, IF the exact path of the keyring matches in both commands. If not, then
a keyring file with the same name in ~/.gnupg will be used instead. So I added a note to point that out.

I think we still need to have these guidelines. Where can I find latest/greatest proposal for this?

I think this guidelines does not solve anything IMO and on top of it is overly complex and fragile. I would rather see if we had something like this:

Source0: ftp://ftp.example.com/pub/foo/%{name}-%{version}.tar.gz
Checksum0: SHA512  = 2c8211ae5f1578502dc9b29babe7d03ec61f500b3c2dd309be2bbd34fd194abba29d95812e7dab4bfacda13e342323921663464bab4cbf4af0a198e8437233f4

That, admittedly, would need to teach RPM about the Checksum keyword, but OTOH, it would allow us to get rid of the "sources" file and let the infrastructure automatically download the file from its URL (if it is not available in lookaside cache yet) and be reasonably sure that the file is what we expect. This would tremendously help with rebase PRs. This would avoid the quite common "Ah, I forgot to upload sources" commits, etc.

I think this guidelines does not solve anything IMO and on top of it is overly complex and fragile. I would rather see if we had something like this:
Source0: ftp://ftp.example.com/pub/foo/%{name}-%{version}.tar.gz
Checksum0: SHA512 = 2c8211ae5f1578502dc9b29babe7d03ec61f500b3c2dd309be2bbd34fd194abba29d95812e7dab4bfacda13e342323921663464bab4cbf4af0a198e8437233f4

That, admittedly, would need to teach RPM about the Checksum keyword, but OTOH, it would allow us to get rid of the "sources" file and let the infrastructure automatically download the file from its URL (if it is not available in lookaside cache yet) and be reasonably sure that the file is what we expect. This would tremendously help with rebase PRs. This would avoid the quite common "Ah, I forgot to upload sources" commits, etc.

Moving the checksum from the sources file to the spec changes nothing from a security point of view. Such a change should therefore be discussed as a separate issue.

FWIW, I updated the example at the end of the draft and provided a solution that works across Fedora and EPEL7 without any conditionals (I haven't tested on EPEL6). I think it also addresses @senderek 's concerns about not touching existing keyring.

Thanks for pushing this forward @rathann.

I'd still be mildly concerned about not touching any user files in ~/.gnupg unless you use a separate gnupg home dir (via --homedir or the GNUPGHOME env). The ./ in the keyring path and --no-default-keyring should cover the common keyring settings, but it's impossible to know what other options a user may set in their local ~/.gnupg that could cause unintended consequences (either for them or for the package build). I think it's worth setting the home dir in any macro to avoid any use of ~/.gnupg. Something like gpghome="$(mktemp -qd)" and then passing --homedir "$gpghome" to the gpg2/gpgv2 calls.

It still seems like using the --import method is a more complex command, compared to gpg2 --dearmor --quiet $key (which works for EPEL (both 6 & 7) and Fedora). What does doing that gain? You end up with a file containing a single key that can be used as an option to --keyring either way.

Moving the checksum from the sources file to the spec changes nothing from a security point of view.

Possibly, but only the "sources" file (stored at a different place than the actual sources) ATM can ensure that the sources which are coming from lookaside cache are the expected sources. I fail to see how checking GPG signatures could improve anything neither it is explained in the draft.

The macro effort seems to have failed on Panu's objections, but I still think some code is needed to automate dearmoring and ensure that no default keyring is used, so here's a less ambitious approach. This script doesn't try to find its inputs automatically, so it can be an external script and not affect the parsing of the spec. To keep the command-line syntax simple and obvious it checks only one signature. If there are multiple sources to verify then the script can simply be invoked multiple times.

  1
  2
  3
  4
  5
  6
  7
  8
  9
 10
 11
 12
 13
 14
 15
 16
 17
 18
 19
 20
 21
 22
 23
 24
 25
 26
 27
 28
 29
 30
 31
 32
 33
 34
 35
 36
 37
 38
 39
 40
 41
 42
 43
 44
 45
 46
 47
 48
 49
 50
 51
 52
 53
 54
 55
 56
 57
 58
 59
 60
 61
 62
 63
 64
 65
 66
 67
 68
 69
 70
 71
 72
 73
 74
 75
 76
 77
 78
 79
 80
 81
 82
 83
 84
 85
 86
 87
 88
 89
 90
 91
 92
 93
 94
 95
 96
 97
 98
 99
100
101
102
103
104
105
106
107
108
109
110
111
#!/bin/bash

# Copyright 2018 B. Persson, Bjorn@Rombobeorn.se
#
# This material is provided as is, with absolutely no warranty expressed
# or implied. Any use is at your own risk.
#
# Permission is hereby granted to use or copy this shellscript
# for any purpose, provided the above notices are retained on all copies.
# Permission to modify the code and to distribute modified code is granted,
# provided the above notices are retained, and a notice that the code was
# modified is included with the above copyright notice.


function print_help {
    cat <<'EOF'
Usage: gpgverify --keyring=<pathname> --signature=<pathname> --data=<pathname>

gpgverify is a wrapper around gpgv designed for easy and safe scripting. It
verifies a file against a detached OpenPGP signature and a keyring. The keyring
shall contain all the keys that are trusted to certify the authenticity of the
file, and must not contain any untrusted keys.

The differences, compared to invoking gpgv directly, are that gpgverify accepts
the keyring in either ASCII-armored or unarmored form, and that it will not
accidentally use a default keyring in addition to the specified one.

Parameters:
--keyring=<pathname>    keyring with all the trusted keys and no others
--signature=<pathname>  detached signature to verify
--data=<pathname>       file to verify against the signature
EOF
}


fatal_error() {
    message="$1"  # an error message
    status=$2     # a number to use as the exit code
    echo "gpgverify: $message" >&2
    exit $status
}


require_parameter() {
    term="$1"   # a term for a required parameter
    value="$2"  # Complain and terminate if this value is empty.
    if test -z "${value}" ; then
        fatal_error "No ${term} was provided." 2
    fi
}


check_status() {
    action="$1"  # a string that describes the action that was attempted
    status=$2    # the exit code of the command
    if test $status != 0 ; then
        fatal_error "$action failed." $status
    fi
}


# Parse the command line.
keyring=
signature=
data=
for parameter in "$@" ; do
    case "${parameter}" in
        (--help)
            print_help
            exit
            ;;
        (--keyring=*)
            keyring="${parameter#*=}"
            ;;
        (--signature=*)
            signature="${parameter#*=}"
            ;;
        (--data=*)
            data="${parameter#*=}"
            ;;
        (*)
            fatal_error "Unknown parameter: \"${parameter}\"" 2
            ;;
    esac
done
require_parameter 'keyring' "${keyring}"
require_parameter 'signature' "${signature}"
require_parameter 'data file' "${data}"

# Make a temporary working directory.
workdir="$(mktemp --directory)"
check_status 'Making a temporary directory' $?
workring="${workdir}/keyring.gpg"

# Decode any ASCII armor on the keyring. This is harmless if the keyring isn't
# ASCII-armored.
gpg2 --homedir="${workdir}" --yes --output="${workring}" --dearmor "${keyring}"
check_status 'Decoding the keyring' $?

# Verify the signature using the decoded keyring.
gpgv2 --homedir="${workdir}" --keyring="${workring}" "${signature}" "${data}"
check_status 'Signature verification' $?

# (--homedir isn't actually necessary. --dearmor processes only the input file,
# and if --keyring is used and contains a slash, then gpgv2 uses only that
# keyring. Thus neither command will look for a default keyring, but --homedir
# makes extra double sure that no default keyring will be touched in case
# another version of GPG works differently.)

# Clean up. (This is not done in case of an error that may need inspection.)
rm --recursive --force ${workdir}

I suggest invoking it like this:

gpgverify --keyring='%{SOURCE3}' --signature='%{SOURCE2}' --data='%{SOURCE1}'

That's as easy to use as we can make it I think. Shall we put this script in some RPM-related package? Or does it need its own package?

These two pull requests are my proposal for a resolution to this issue:
https://src.fedoraproject.org/rpms/fedora-rpm-macros/pull-request/1
https://pagure.io/packaging-committee/pull-request/836

Compared to the proposal in the wiki, simplified the code templates by using gpgverify. I have also expanded the policy on how to obtain the keys, as that's the step where everything depends on the vigilance of the packager.

It turned out that fedora-rpm-macros was the wrong package. I propose this addition to redhat-rpm-config instead:
https://src.fedoraproject.org/rpms/redhat-rpm-config/pull-request/54

I have also updated the policy proposal:
https://pagure.io/packaging-committee/pull-request/836

The software is now in place in Fedora 29, 30 and Rawhide, and the proposed policy is waiting to be merged. As far as I can see this proposal is ready for voting.

We talked about this issue this week:

https://meetbot-raw.fedoraproject.org/fedora-meeting-1/2019-07-11/fpc.2019-07-11-16.00.txt

Metadata Update from @james:
- Issue tagged with: writeup

4 years ago

Login to comment on this ticket.

Metadata