https://bugzilla.redhat.com/show_bug.cgi?id=2141686 revealed that much
of the rpm-ecosystem is still using the obsolete v3 OpenPGP signature
format, I think largely due to workarounds for legacy rpm versions (from
around the turn of the millennium) that have just been forgotten in
place. Lets at least issue a wake-up warning when that happens.
Unfortunately this is can't really be tested as current GnuPG versions
just ignore any --force-v3-sigs arguments.
Fixes: #2286
How do we even _know_ user wants to debug malloc in rpmbuild,
maybe user wants to debug it in _the child_?
Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
pgpPrtParams() may leave sig2 unchanged and if we're not in the very
first iteration of the while() loop, we could pass a freed pointer to
pgpDigParamsCmp(). Fix by setting it to NULL after freeing.
Found by Coverity, after commit bd36c5d (subkey binding validation),
although note that the commit didn't introduce this bug; it just seems
to have been a false negative that got "fixed" by the changes in
pgpPrtParams() in that commit.
There's no way to safely validate an object to which only a void
pointer is given. Use headerImport() and pass a size to make
verification possible, headerCopyLoad() has been long deprecated anyway.
The argument --verity-algo can be used to specify the algorithm for
the fsverity signatures. If nothing is specified, this will default to
sha256. The available algorithms depend on libfsverity, currently
sha256 and sha512 are supported.
Signed-off-by: Jes Sorensen <jsorensen@fb.com>
This allows a user to remove both types of file signatures from the
package. Previously there was no way to delete IMA signatures, only
replace them by first removing the package signature and then
resigning the package and the files.
Signed-off-by: Jes Sorensen <jsorensen@fb.com>
Various proprietary packages in the wild have subtly malformed data
in the signature header, in particular wrt the immutable region size,
presumably from using some in-house/3rd party signing tools which do
not understand the immutable region business at all. This can prevent
resigning and signature deletion on such packages due to the more
thorough checking that rpmsign does.
As the old wisdom goes, be liberal in what you accept... we can easily
work around the crud by just taking a fresh copy of the contents that
are legit as such (otherwise the package would be uninstallable).
When it comes to IPC with the GPG subprocess that we spawn to sign
packages for us, there has been an evolution of changes over the years.
Initially, we used temporary files to dump package data (header+payload)
to disk, and pointed GPG to those. That being an insanely expensive
operation, we later opted for a pipe bound to stdin on the reader side
[1] to avoid the unnecessary I/O. When GPG 2.1 came along, our own
passphrase entry logic stopped working so we decided to offload [3] that
to GPG instead, for which we also needed to swap the pipe for a named
pipe (FIFO), to free [2] stdin for GPG's own passphrase entry program
(pinentry).
Fast forward to the present, it has become apparent that the FIFO
semantics is not the right choice for us either, as it brings about a
handful of synchronization issues, all of which stem from the fact that
the GPG subprocess, which we cannot control, may or may not open the
input file (our FIFO) for reading before it terminates, causing either
process to hang. That's because an open(2) call on a FIFO blocks until
the other end is also opened. One particular case where such a deadlock
can happen is an expired key [4]; GPG would error out without ever
opening the input file (and rightly so), making RPM stuck.
In a nutshell, here's what the two processes do:
RPM parent:
* spawn GPG child
* open FIFO for writing (blocks)
* write data
* close FIFO
GPG child:
* check key validity <-- may terminate here
* open FIFO for reading (blocks)
* read data
* close FIFO
Which may result in the following execution order:
* parent: spawn GPG child
* parent: open FIFO for writing (blocks)
* child: check key validity (key expired!)
* child: terminate with an error (sends SIGCHLD)
* parent: (continues blocking...)
Now, one way to unblock the parent would be to install a SIGCHLD handler
with SA_RESTART and make it open the FIFO for reading (with O_NONBLOCK)
iff the signal comes from our GPG child (note that RPM can also be used
as a library) so that the restarted open call unblocks right away. This
would, however, still not be entirely safe from race conditions; if
there were other subprocesses, all terminating about the same time,
their SIGCHLD instances could prevent us from receiving the one from
GPG; this is because standard signals do not queue (see signal(7) for a
detailed explanation).
Another way would be to spawn GPG from a wrapper process whose sole
purpose would be to wait(2) for GPG and to open the FIFO if it failed
(without O_NONBLOCK, to ensure the wrapper doesn't exit before the
parent's open call). This would work as long as we could tell exactly
which errors cause GPG to not open the input file, but since that's not
documented, we would have to rely on implementation details, not to
mention the added complexity and ugliness of such a solution, so that's
also a no-go.
Lastly, why not just use O_NONBLOCK in the parent, to prevent it from
blocking in the first place? Since GPG does not use O_NONBLOCK, this
would just push the problem down to the child; if the parent is too fast
and manages to close the FIFO before the child gets a chance to open it,
the child would get stuck once it gets there (thus making the parent
stuck on the wait call).
Luckily, it turns out we actually can take over stdin *and* have GPG use
its pinentry program at the same time! All we need is to make sure the
GPG_TTY environment variable is set to the current terminal (see
gpg-agent(1)). This variable is used [5] by GPG as a fall-back when it
fails to obtain the terminal connected to stdin using ttyname(3), such
as when it is redirected to a pipe.
Thus, in this commit (mostly adapted from the reverse of [2]), we revert
to using the good old pipe(7) which not only avoids race conditions (as
it does not cause open to block), but is also more suitable for this
task in general. (For the record, GPGme also uses [6] a regular pipe
for the IPC.) Since we have access to the original stdin before
redirection, we set GPG_TTY accordingly (if previously unset), to keep
pinentry working.
This commit also resolves the RHBZ linked in [4]. Note that, in the
case of key expiration, GPG isn't particularly specific in the error
message or exit code; all it says is "Unusable secret key" and returns
code 2. For that reason, we can't print anything helpful to stderr
either.
[1] commit 1aace27fb9
[2] commit 6a8924b4c9
[3] commit 0bce5fcf27
[4] https://bugzilla.redhat.com/show_bug.cgi?id=1746353
[5] f3df8dbb69/agent/gpg-agent.c (L1115)
[6] 52f930c1ed/src/engine-gpg.c (L1133)
On packages where a separate payload digest exists (ie those built with
rpm >= 4.14), rpm v3 header+payload signatures are nothing but expensive
legacy baggage, as the payload digest will be signed by a header-only
signature already, without having to recalculate the entire file.
Automatically detect the payload digest presence and only add V3
signatures on packages that need it, but also add an override switch
to force their addition if needed for compatibility or so. A particular
use-case would be ability to signature-level verify the entire package
on rpm older than 4.14.
Fixes: #863
There will be any number of signing flags in the future, and we don't
want to break the ABI for every single one of them by adding new
fields to the sign argument struct. Replace the signfiles field
with a bitfield in the common rpm style. No functional changes.
This is an API change of course, but we'll have to bump the soname for
the next release anyway so might as well do it now.
Permitting corrupted packages to be signed is bad business for everybody
involved, this is something we should've always done. Besides being an
actual security risk, it can lead to odd results with verification
especially with the payload digest on signed packages.
One point worth noting is that this means that pre 4.14-packages cannot
be signed in FIPS mode now because there's no way to validate the package
payload range due to MD5 being disabled. This seems like a feature and
not a limitation, so disabler for the verify step intentionally left out.
Optimally we'd verify the package on the same read that's passed
to gpg but for simplicitys sake that's left as an future exercise,
now we simply read the package twice.
Lotsa unnecessary stuff here:
- No need for rpmtdReset(), headerGet() takes care of that
- In this path, we're always modifying an existing tag so we can
just use headerMod() instead of constructing a new tag
- As we're always shrinking the existing tag, we don't need to
allocate new zeros, just reduce the size. Also make sure we *are*
really shrinking, previously it was only assumed.
No functional changes expected.
Up to commit 45c2f3ffa6 we were still
using the lead to confirm source vs binary package, but now we dont
need even that. Means the zombie data structure that's supposedly been
dead for the last 20+ years is truly gone now. We still dutifully
read/write and validate it, but no actual data from it is used.
The original file signing puts the file signatures into the main header
immutable region, invalidating all previous signatures and digests so
the package no longer appears to be what it was when it came out of the
assembly line. Which is bad. Doing that also requires recalculating
everything again which is just added complexity, and since it adds
stuff to different place from the rest of the signing, it requires yet
complexity to deal with that. Moving the file signatures into the
signature header solves all that and allows removing a big pile of
now unnecessary code.
Because this means retrofitting tags bass-ackwards into the signature
header, the tag definitions are backwards to everything else. Other
options would certainly be possible, but this makes things look more
normal on the signature header side. "Users" only ever see the
unchanged file signature tags as they have always been.
This also means the signature header can be MUCH bigger than ever before,
so bump up the limit (to 64MB, arbitrary something for now), and
permit string array types to be migrated from the signature header
on package read.
Caveats:
This loses the check for identical existing signatures to keep the
complexity down, it's hardly a critical thing and can be added back later.
While file signing could now be done separately to other signing, that
is not handled here.
The replaceSigDigests function is only used in includeFileSignatures
when WITH_IMAEVM is defined. If not warning is generated.
Signed-off-by: Mark Wielaard <mark@klomp.org>
rpmsign.c used to actually use "environ" to pass to execve(), but
that call moved to librpmsign a long, long time ago. rpmdb.c and
rpmkeys.c never used it at all but guess it was copy-paste inherited
from rpmsign.c back in the day (dfbaa77152)
rpmgensig.c actually refers to environ, but this is a POSIX required
variable and while Apple has managed to screw it up, it's handled
in system.h and that must be sufficient for all relevant systems
as we also refer to environ in rpmfileutil.c open_dso() and there's
no fake environ definition there. So drop the one in rpmgensig.c too.
Give a nice and clear error message if file signing is requested
on a build that doesn't support it, and ifdef the entire
contents of includeFileSignatures() so we wont try to compile
in a call to rpmSignFiles(), which doesn't get built at all
due to makefile conditionals when imaevm is not enabled.
On a related note, drop the unnecessary conditional and error message
from rpmsignfiles.c: the whole file does not get built if imaevm
is not enabled so there's no need for these.
The signing library is separated from librpm and librpmbuild
specifically so it can have more exotic dependencies than is
desireable for librpm, ditto for plugin existence. However
rpmSignFiles() being in librpm drags that libimaevm dependency
to main rpm for no good reason at all. Move it where it belongs,
does not actually affect functionality.
Refactor rpmsign and python bindings to be more similar on both
addsign/delsign operations, and always pass the signing arguments
along. Deletion doesn't actually (yet) use the arguments for anything
but makes things more symmetric (I remember having doubts about
this when adding - reminder to self: if in doubt, add more arguments ;)
Yet another API break, but what the hey... Other than that, behavior is
not supposed to change here.
Now that the lower level is returning the actual data we can
compare directly *before* inserting new data, so we dont need
to copy either. Eliminate redundant (and inadequate) helpers
while at it, preliminaries for array data as well.
putSignature() is awfully far away from the originally calling code
to make decisions, refactor to only create the signature tag there
and return the rpmtd up through the callers to make a more informed
decision.
Doesn't actually change anything, just paving way for next steps.
Too many layers of helpers leftover from past times only getting
in the way of doing stuff. Also rename replaceSignature() arguments
to make it more obvious which is which.
This doesn't *really* change the signing, but it does optimize the
case where the package is already has a signature by the same key
and parameters: we generate the much cheaper V4 (ie header only)
signature and compare that, and only then do the V3 signature
which has to read the entire payload. This can make a huge difference
on large packages.
This saves us from having to unnecessarily guess what the lower
level helpers ended up using so simplifies the code a bit, and
also paves way for other types of signatures. What's not to like?
SHA1 has been getting a bit long in the tooth for many years by now,
add a more modern digest to eventually supplant it, for now just
prefer SHA256 over SHA1 if present when verifying. Using a hardwired
algorithm instead of configurable one to keep things on the simple
side when dealing with the signature header.
Signing could add the new digest for older packages but we don't do
that to avoid surprises when people are signing older packages.
There's no point comparing the weaker MD5 for equivalence when we
can determine it by looking at a stronger digest already. Also we
don't need the digest lengths here since SHA1 is ascii anyway.
The missing space style-error has been recently coming common enough
somebody might think that IS the expected style, its not. Some of these
are actually very old, but fix across the board for consistency..
Strictly white-space only change.
The only "user" of the lead data structure is the signing code
which just wants to copy it over to the signed package. We can
just as well regenerate it from the header. Even if it doesn't
end up identical (which it should), it doesn't make a damnest
difference because nothing that matters looks at the lead anyway.
These are not deprecated at all no matter what the header has been
saying for the past 15+ years, they're used by rpm itself all over
the place as rpmDefineMacro() serves a slightly different purpose
and there's no rpmUndefineMacro() anyway.
Lets make 'em into proper citizens and move them into rpm namespace,
and while at it, call the operations push and pop since that's much
closer to what actually happens.
Finally, add simple wrapper macros to keep external code compilable
while getting the non-namespaced stuff out of ABI.
Header signatures were the new hot almost exactly twenty years ago, we
haven't supported anything else in a very, very, very, very very long time.
Drop the useless argument to rpmReadSignature() and bury the last remaining
related constant into rpmlead.c which is the only place that "needs" it.
No functional changes.
the doc explicitely describe "args" as "signing parameters (or NULL for
defaults)"
This no more true since commit 6e9eab345a
As such, with rpm-4.13, some callers will segfault (eg: perl-RPM4's
testsuite)
Removed setting LC_ALL to "C" because since commit [1] the gpg program
gets password by yourself from terminal so there is no sense in
setting LC_ALL to "C" if the terminal settings is e. g. UTF-8. That was
only confusing gpg program and it was not able to properly get and
display non-ASCII characters.
[1] 0bce5fcf27
When file signatures are added to a package the signature digests are
replaced. Sometimes the resulting signature header has a different size.
To solve this, gpg reserved space is omitted. This forces the rpm to be
rewritten when file signatures are added.
Changelog:
-no longer effects delsign
Panu Matilainen