One of the busiest functions in rpm was getting passed a new copy of a
big unordered_map on each call during fingerprinting because "somebody"
forgot a & in commit 08a6a5e848.
This isn't even noticeable in the test-suite or daily "update a few
dozen packages" operation but when attempting to install/update a few
thousand packages dragged this little buglet to the light...
Why oh why C++ do you behave in such an idiotic manner in the face of
stupid mistakes. Raw pointers at least give you a compiler error.
This is needed for "make check" to work on a Fedora 41 host where the
FROM directive is overridden to fedora:41.
- Drop the workaround for gdb-headless on F41, it's no longer available
there and the bug is fixed on F40+:
https://bugzilla.redhat.com/show_bug.cgi?id=2310156
- Don't try to remove rpm-sign-libs and python3-rpm on F41 since dnf5 no
longer pulls them in.
- Make sure to call "dnf clean all" before "rpm -e" as python3-rpm is
needed by dnf4 to function (hence the duplicated call).
- Add a comment at the top saying which Fedora versions are supported by
the Dockerfile. This could be checked automatically during cmake
config but let's leave that for later.
- Detect F41 vs. older by looking for the dnf5 binary, it's the easiest
way.
Dynamic spec parsing emits "Reading <path>" messages to make it clear
something out of the ordinary is going on, but this is undesireable
on rpmspec queries. Luckily there's a trivial solution: rpmbuild where
you want these messages runs in verbose mode by default whereas rpmspec
does not. So just moving the message to INFO level does exactly the
right thing in both the above cases. Add a test to go.
Fies: #3413
Add an abstract rpm::keystore class and port our existing rpmdb and fs
keystores to that. The keystore code as such doesn't really change at
all in here, the bigger change is the way it's initialized because it's
an object instead of just an integer in the rpmts struct.
As a kind of side-effect, we introduce the rpm:: namespace here.
Fixes: #3342
This used to return number of keys loaded but it didn't really mean
anything, and so nothing was checking the results either. So for
now keystore load simply always returns success, that's how it has
technically always behaved anyway.
Take read-only transaction lock at the rpmts side, pass it down the
rpmKeystoreLoad() hatch.
Somehow this feels like an overkill but then our keystores haven't
been designed with concurrent access in mind *at all*, so play safe
and just take a read lock while loading they keystore. It also balances
the API: everything just takes a txn item now.
Related: #3342
Commit 364d28459a moved a little too
much to the keystore side, revert the part that moved keyring
type detection back to the rpmts level.
No functional changes, needed for the next steps.
Pass pubkeys down to the keystore on import, and let the keystore handle
the actual format. For rpmdb keystore this remains the header, so it's a
no-op there, but on the fs keystore we drop the trailing "time or something"
hex garbage from the filename because we no longer have it at hand but
also because it's just not relevant for anything. The key load glob is
relaxed enough that we still pick the old format keys too.
Related: #3342
Make sure (text editor) backup files, such as those with the tilde (~)
at the end, aren't processed by our macrofiles globs. These can appear
while editing a macro file in place and may result in confusing behavior
where an old version of a macro overrides the one being written, like
seen in the ticket #3373.
Rather than enumerating any specific suffixes, just mandate that macro
files end with alphanumerics. That's more of a name sanity check than
anything but fits the bill here.
Co-authored-by: Peter Oliver <git@mavit.org.uk>
Remove all other subkey handling code
Inline the remaining few lines of keyringAdd in keystore.cc
This slightly changes the DEBUG messages as the keyring does not have
access to the origin of the keys. So rpmtsLoadKeyringFrom* still gives
the location the keys came from while the keyring only lists the
fingerprint of the primary keys and the number for the sub keys.
This changes the return value of rpmKeystoreLoad to the number of
primary keys and no longer accounts for the subkeys.
Subkeys are covered by multiple test already - including merging a newer
key. So this does not add additional tests.
Resolves: #3350
Loop over the candidates during signature verification and use the one
verifying it - iff any. Otherwise print error messages from all
candidates.
Resolves: #3334
This is needlessly duplicated identical logic in two places so it's
an obvious cleanup, but also lets #3350 proceed without having to
worry about changing multiple places.
Related: #3350
Except for making the three keystore API functions non-static, there
are no code changes here, it's all just moving stuff around. Thus,
there cannot be any functionality changes either.
The internal API here should not be considered final, this is just
a stepping stone to hide the keystore internals from the rest of
the rpmts layer.
Related: #3342
On install/update, most files are laid down with a temporary suffix
and if the update fails, removing those at the end of the loop is
the right thing to do. However FA_TOUCH'ed files were already there,
we only update their metadata, and we better not remove them!
AFAICS this all versions since rpm >= 4.14 in one way or the other.
If %_minimize_writes is enabled then it affects way more than just
unmodified config files.
The test is a simplified version of pam update failing in the original
report. Technically, --nomtime should not be needed for the test
verification but we don't even try to restore the metadata on failure,
and fixing that is way out of scope here.
Fixes: RHEL-54386
Fixes: #3284
Change the gpg-pubkey version from short keyid to full fingerprint.
As the header is currently used for formatting the fs keystore filename
too, this changes both keystores to saving by fingerprint.
Update tests accordingly.
Fixes: #3360
Subkeys are more of an implementation detail that shouldn't be exposed
to users, and nothing at all needs these. Worse, these block us from
moving to fingerprints in the gpg-pubkey versions because we don't
even have the subkey fingerprints.
Related: #3360
Lets us use the newly added helper rpmPubkeyKeyIDAsHex() instead
of doing the same manually, and paving way for other similar things.
No functional changes.
Despite being bound to "make check" and reasonably fast, our tests sit
at least one level above what you'd normally call "self tests" or "unit
tests" and require external container tooling to run.
Therefore, disable them by default, they're only really useful for RPM
development, certainly not for building RPM itself, e.g. from a release
tarball. Update INSTALL and Dockerfile accordingly.
Fixes: #3388
Except for the reference count, the keys are actually immutable once
created, merge only ever creates a new instance. So with the atomic
reference counting from a695776047 we
can now just toss the stupid locks. Yay.
The hex string is by far the most common way the fingerprint is used,
so it makes sense to initialize it when constructing the key already.
Then we can return a const pointer to the string, which makes this
much nicer to use from C.
No functional changes.
The hex format is the way it's most common way the keyid is used,
and makes no difference to the computer. This allows us to return
a const pointer from rpmPubkeyKeyIDAsHex() which makes it much nicer
to use from C.
No functional changes.
Use the matchingKeys() in rpmkeys to acquire those rpmPubkey instances.
Use EXIT_FAILURE as exit code for rpmkeys --delete instead of the
count of errors.
We need to test for these as long as we carry the rpmdb keystore,
but make it explicit so these wont cause false positives when the
default keystore changes.
These data structures use the content lock (aka mutex) to also protect the
reference counter. This is wrong and complicates freeing interconnected
data structures. This lock should only protect the content of the
instance.
Turn nrefs members into atomic_int so they are thread-save themselves.
Make sure to only use ++ and -- operators and avoid non atomic
combinations of reading and writing. As the nref can't change again
after reaching zero this is thread-save even as we look at the result
an instruction later.
Make sure the data lock is not aquired for updating the nrefs members.
Technically the *Free() functions should *Link() the instance after the
ref count has reached 0 and the decision to free it has been made.
Otherwise the cleanup code linking and freeing to will lead to a
recusions. This is not neccessary here so it is omitted.
Throughout the code base reference counting is done in a thread-unsave
way.
Turn nrefs into atomic_int so they are thread-save themselves. As the
nref can't change again after reaching zero this is thread-save even
as we look at the result an instruction later.
FD_t is special in that fdFree return the instance after free iff it
still has not reached a ref count of 0. Unfortunately code in
rpmShowProgress() relies on that.
This does not make the data structures thread save on its own. But it
gives a foundation on which data locking can be implemented.
Throughout the code base reference counting is done in a thread-unsave
way.
Turn nrefs into atomic_int so it is thread-save.
Make sure to only use ++ and -- operators and avoid non atomic
combinations of reading and writing. As the nref can't change again
after reaching zero this is thread-save even as we look at the result
an instruction later. Drop all the static *Unlink functions.
Technically all the *Free() functions should *Link() the instance after the
ref count has reached 0 and the decision to free it has been made.
Otherwise the cleanup code linking and freeing to will lead to a
recusions. Right now only rpmts needs that. So we only do that here.
This does not make the data structures thread save on its own. But it
gives a good foundation.
Throughout the code base reference counting is done in a thread-unsave
way.
Turn nrefs members into atomic_int so they are thread-save
themselves.
Make sure to only use ++ and -- operators and avoid non atomic
combinations of reading and writing. As the nref can't change again
after reaching zero this is thread-save even as we look at the result
an instruction later. Drop all the static *Unlink functions.
Technically the *Free() functions should *Link() the instance after the
ref count has reached 0 and the decision to free it has been made.
Otherwise the cleanup code linking and freeing to will lead to a
recusions. This is omitted here as it is not needed.
This does not make the data structures thread save on its own. But it
gives a foundation on which further thread safety work can be based on.
Read-only media doesn't need any locking by definition. But if the
file we're trying to open doesn't exist there, we can't very well open
it for even reading, causing the unnecessary locking to fail
unnecessarily. Feed it a dup of our STDIN_FILENO to keep the beast happy.
Couldn't figure how to get a read-only mount inside the test-root
so we need to add a pre-generated rpmdb and then run it without
having the test-image read-write mounted. The rpmdb file is ridiculously
big just for testing this corner-case purpose, but maybe it could be
used to speed up some other tests later on.
Fixes: #3371#1266
Now that the underlying rpmlock supports it, have rpmtxnBegin()
actually differentiate between read and write locks. This makes
it possible for a normal user to export the rpmdb.
Add test for export as a regular user (ie the fix here) and import
in general, we didn't have any tests for that before now.
It's a bit hysterical that rpmlock has this elaborate fallback code
when it cannot create a lock file, but then it didn't actually
support taking a read-only lock at all. Well, now it does.
Panu Matilainen