-
Notifications
You must be signed in to change notification settings - Fork 34
Bug 2009927 - Add support for rpmsign in the gpg2 signer #1187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
c9fdf77 to
e4a12ac
Compare
|
Love CI failures that I can't repro locally T_T |
a4b3fcf to
78313dd
Compare
alexcottner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good to me. Asking for a second set of eyes to review just for a reality check.
Was this the |
|
Yeah I had issues with docker containers not building locally when I tried to run autograph so I actually never tried to run tests in it. But it ended up working sooooo 🤷 |
say-yawn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for submitting this. Here's my first pass with change suggestions and questions. Did not complete the passphrase part in gpg2.go and the tests. Will resume this work tomorrow.
| oOuLM3lEWUfh | ||
| =SRmt | ||
| -----END PGP PRIVATE KEY BLOCK----- | ||
| publickey: | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Validated that this is the correct key-pair by running:
gpg --armor --export A2910E4FBEA076009BCDE536DD0A5D99AAAB1F1Aand checked that the output from the private key above matches the public key below.
| // debsign can call gpg multiple times per file | ||
| passphraseRepeat = len(inputFilePaths) * 4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you elaborate more on why we are multiplying by 4 here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
passphrasesForStdin := strings.Repeat(fmt.Sprintf("%s\n", s.passphrase), len(inputFilePaths)*4) because this was the code before.
say-yawn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More recommendation to update error message to explicitly state the mode and a question on how the overwriting passphrase-fd works.
| args := append([]string{ | ||
| "--addsign", | ||
| "--key-id", s.KeyID, | ||
| "--define", "__gpg /usr/bin/gpg", | ||
| "--define", "_gpg_sign_cmd_extra_args --passphrase-fd 3", | ||
| }, inputFilePaths...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Eijebong, @alexcottner and I checked the documentation and do not see what --define does and how --passphrase-fd is passed to gpg. Can you point to some documentations on both?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--define is an rpm option:
$ rpmsign --help
Usage: rpmsign [OPTION...]
Signature options:
--addsign sign package(s), adding a new signature
--resign resign package(s), deleting any previous signatures
--delsign delete package signatures
--delfilesign delete IMA and fsverity file signatures
--rpmv3 request legacy rpm v3 header+payload signatures on v4 packages
--rpmv4 request legacy rpm v4 header signatures on v6 packages
--rpmv6 request rpm v6 header signature on v4 packages
--signverity generate fsverity signatures for package(s) files
--verityalgo=<algorithm> algorithm to use for verity signatures, default sha256
--certpath=<cert> use file signing cert <cert>
--fskpath=<key> use file signing key <key>
--fskpass prompt for file signing key password
Common options for all rpm modes and executables:
-D, --define='MACRO EXPR' define MACRO with value EXPR
--undefine=MACRO undefine MACRO
-E, --eval='EXPR' print macro expansion of EXPR
--target=CPU-VENDOR-OS Specify target platform
--macros=<FILE:...> read <FILE:...> instead of default file(s)
--load=<FILE> load a single macro file
--noplugins don't enable any plugins
--nodigest don't verify package digest(s)
--nosignature don't verify package signature(s)
--rcfile=<FILE:...> read <FILE:...> instead of default file(s)
-r, --root=ROOT use ROOT as top level directory (default: "/")
--dbpath=DIRECTORY use database in DIRECTORY
--querytags display known query tags
--showrc display final rpmrc and macro configuration
--quiet provide less detailed output
-v, --verbose provide more detailed output
--version print the version of rpm being used
Options implemented via popt alias/exec:
--key-id=<id> key id/name to sign with
--digest-algo=<algorithm> override default digest algorithm (eg sha1/sha256)
Help options:
-?, --help Show this help message
--usage Display brief usage message
rpm --showrc shows how _gpg_sign_cmd_extra_args is used, e.g.
-13: __gpg_sign_cmd %{shescape:%{__gpg}}
--no-verbose --no-armor --no-secmem-warning
%{?_gpg_path:--homedir %{shescape:%{_gpg_path}}}
%{?_gpg_digest_algo:--digest-algo=%{_gpg_digest_algo}}
%{?_gpg_sign_cmd_extra_args}
%{?_openpgp_sign_id:-u %{shescape:%{_openpgp_sign_id}}}
-sbo %{shescape:%{2}} -- %{shescape:%{1}}
This mimics what autograph is doing for debsign. While the signature of RPM is simple gpg, we can't use the plain gpg2 signer for it because we cannot attach a detached signature easily, all the tools that are available for this would require some fairly serious hacking. The addition itself is fairly straightforward, there's a bit of plumbing to add it as a signer type, but overall it's sharing all the same code paths as the debsign signer. The one main difference is how we're passing the passphrase to it... The key's passphrase must be given to gpg2 itself, which both debsign and rpmsign would normally do through pinentry. The config says to use the caller (rpm/debsign) to get the passphrase from (`pinentry-mode loopback` in gpg.conf). Autograph uses that and `passphrase-fd 0` to write the passphrase onto debsign's stdin which is forwarded to the gpg subprocess and everything works fine there. Rpmsign on the other hand invokes gpg and changes the stdin for it to a pipe in which it passes the value to be signed [1]. That means that when we try to write a passphrase onto rpmsign's stdin, it ends up in the "void" since it's not forwarded to the gpg subprocess. And `passphrase-fd 0` means that it will read the value to be signed instead of the actual passphrase which as you can expect doesn't go very well... Instead of doing that, we force the fd 3 for rpmsign to be a pipe on our control, and we tell gpg to read from that for the passphrase, and write it there. It used to do that automatically on its own (setup a fd 3 in which one could pipe in a passphrase) until rpm 4.13 [2] but got removed because gpg 2.1 doesn't allow `passphrase-fd` by default anymore. [1]: https://github.com/rpm-software-management/rpm/blob/4623d4601ee83b5e0ecd16dd3f54d2182b519b30/sign/rpmgensig.c#L254 [2]: rpm-software-management/rpm@0bce5fc
This mimics what autograph is doing for debsign. While the signature of RPM is simple gpg, we can't use the plain gpg2 signer for it because we cannot attach a detached signature easily, all the tools that are available for this would require some fairly serious hacking.
The addition itself is fairly straightforward, there's a bit of plumbing to add it as a signer type, but overall it's sharing all the same code paths as the debsign signer. The one main difference is how we're passing the passphrase to it...
The key's passphrase must be given to gpg2 itself, which both debsign and rpmsign would normally do through pinentry. The config says to use the caller (rpm/debsign) to get the passphrase from (
pinentry-mode loopbackin gpg.conf). Autograph uses that andpassphrase-fd 0to write the passphrase onto debsign's stdin which is forwarded to the gpg subprocess and everything works fine there.Rpmsign on the other hand invokes gpg and changes the stdin for it to a pipe in which it passes the value to be signed 1. That means that when we try to write a passphrase onto rpmsign's stdin, it ends up in the "void" since it's not forwarded to the gpg subprocess. And
passphrase-fd 0means that it will read the value to be signed instead of the actual passphrase which as you can expect doesn't go very well...Instead of doing that, we force the fd 3 for rpmsign to be a pipe on our control, and we tell gpg to read from that for the passphrase, and write it there. It used to do that automatically on its own (setup a fd 3 in which one could pipe in a passphrase) until rpm 4.13 2 but got removed because gpg 2.1 doesn't allow
passphrase-fdby default anymore.