-
Notifications
You must be signed in to change notification settings - Fork 102
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
expired-pgp-keys plugin checks an expiration key of the freshly downloaded key, not an expiration of the installed key #2099
Comments
For a reproducer purposes, this is the old, expired key I have in the RPM database:
|
Thanks Petr for reporting this. Could you please provide the output of:
Right now, we parse the information from the 7th field of the first row of this output, which should be the expiration date (based on, e.g., https://git.gnupg.org/cgi-bin/gitweb.cgi?p=gnupg.git;a=blob_plain;f=doc/DETAILS). So, the following command should give the human-readable expiration date in the output:
I was testing the functionality of expired-pgp-keys based on this presumption and it seemed to work using e.g. dnf-nightly COPR repo and |
You could have obtain the data from the key block I've already quoted. Nevertheless, here is the output:
Stdout was the second line with the date. |
If I understand the plugin code correctly then it does: (1) It downloads keys for all enabled repositories. The flaw is in the third step. It should query the expiration time of the installed key. Not the time of the downloaded key. |
Oh yeah, you're right. This dumb issue is really happening. It appeared after trying to improve the workflow of the original dnf4 expired-pgp-keys plugin where this issue should not be present. Maybe if we would keep the same approach with the generic removal of any expired keys on the system, it would also resolve this related issue: #2108. |
There was a flaw in what this plugin did: Instead of checking an expiration time of the keys in an RPM database, it downloaded keys of enabled repositories from the Internet and checked their expiration time. As a side effect, this plugin repeatedly downloaded the keys on every invocation. This patch fixes both the issues by checking keys which are in the RPM database instead. The new implementation removes all expired and confirmed keys in a single RPM transaction. It also removes the unnecessary crosscheck that a key obtained from an RPM database axtually exists in the database. All that speeds up the operation at the expense that reporting a success of the removal happens after the user selecting all the keys for the removal. The user query now reports a fingerprint instead of a path to the key file. The reason is that a name of the key file is a temporary file now because we take the keys from the RPM database. The current API does allow importing from memory, neither changing the file attribute of KeyInfo object. This patch intentionally does not enhance any interface, inluding newly identified bugs to be fixed independenlty later: Respect --install-root option. We should probably move some code to libdnf5/rpm/rpm_signature.cpp, or expose more code from there. libdnf5::repo callbacks API is badly designed as it cannot be used independetly from libdnf5::repo::RepoQuery object. That interface should be moved closer to base as it is deals with a user interface and not the repositories. Resolves rpm-software-management#2099 Resolves rpm-software-management#2108
There was a flaw in what this plugin did: Instead of checking an expiration time of the keys in an RPM database, it downloaded keys of enabled repositories from the Internet and checked their expiration time. As a side effect, this plugin repeatedly downloaded the keys on every invocation. This patch fixes both the issues by checking keys which are in the RPM database instead. The new implementation removes all expired and confirmed keys in a single RPM transaction. It also removes the unnecessary crosscheck that a key obtained from an RPM database axtually exists in the database. All that speeds up the operation at the expense that reporting a success of the removal happens after the user selecting all the keys for the removal. The user query now reports a fingerprint instead of a path to the key file. The reason is that a name of the key file is a temporary file now because we take the keys from the RPM database. The current API does allow importing from memory, neither changing the file attribute of KeyInfo object. This patch intentionally does not enhance any interface, inluding newly identified bugs to be fixed independenlty later: Respect --install-root option. We should probably move some code to libdnf5/rpm/rpm_signature.cpp, or expose more code from there. libdnf5::repo callbacks API is badly designed as it cannot be used independetly from libdnf5::repo::RepoQuery object. That interface should be moved closer to base as it is deals with a user interface and not the repositories. Resolves rpm-software-management#2099 Resolves rpm-software-management#2108
There was a flaw in what this plugin did: Instead of checking an expiration time of the keys in an RPM database, it downloaded keys of enabled repositories from the Internet and checked their expiration time. As a side effect, this plugin repeatedly downloaded the keys on every invocation. This patch fixes both the issues by checking keys which are in the RPM database instead. The new implementation removes all expired and confirmed keys in a single RPM transaction. It also removes the unnecessary crosscheck that a key obtained from an RPM database axtually exists in the database. All that speeds up the operation at the expense that reporting a success of the removal happens after the user selecting all the keys for the removal. The user query now reports a fingerprint instead of a path to the key file. The reason is that a name of the key file is a temporary file now because we take the keys from the RPM database. The current API does allow importing from memory, neither changing the file attribute of KeyInfo object. This patch intentionally does not enhance any interface, inluding newly identified bugs to be fixed independenlty later: Respect --install-root option. We should probably move some code to libdnf5/rpm/rpm_signature.cpp, or expose more code from there. libdnf5::repo callbacks API is badly designed as it cannot be used independetly from libdnf5::repo::RepoQuery object. That interface should be moved closer to base as it is deals with a user interface and not the repositories. Resolves rpm-software-management#2099 Resolves rpm-software-management#2108
I'm trying to upgrade rpminspect and librpminspect packages from this Copr repository:
and I get not unhelpful error message:
Direct update with rpm explains that the problem is an expired key:
And indeed the key in RPM database is expired:
When I download the key from the web, it has prolonged expiration:
I guess this is a perfect use case for the expired-pgp-keys plugin. But I have installed it:
it is enabled:
Yet, it does not attempt to remove that key from the database:
I confirmed with strace that the plugin is loaded. There is no relevant message in the log besides:
and literally last lines:
It seems that the plugin does nothing for me.
The text was updated successfully, but these errors were encountered: