Skip to content
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: New plugin for detecting expired PGP keys #1592

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jan-kolarik
Copy link
Member

Based on the implementation from rpm-software-management/dnf-plugins-core#533.

Workaround for: #1192.

@jrohel
Copy link
Contributor

jrohel commented Jul 25, 2024

I looked into the source code. It's a rewrite of the new "expired-gpg-keys" plugin from DNF4 to libdnf5.

And here's the problem. The original plugin is in Python. It is not part of the libdnf library -> this makes it ignored by users using libdnf directly - microdnf, PackageKit, ... And it behaves more like an application plugin - it interacts with the user via the console.

This PR implements the libdnf5 plugin. So it is a library plugin. The advantage is that it will be active for all users of the libdnf5 library (dnf5 application, dnf5daemon-server, ...). However, the library must not interfere with the console. For example, dnf5daemon-server does not have a console to communicate with the user - it is a background daemon.

The dnf5 application only supports "command" plugins. And generally making this plugin as an application plugin is not a good way to go. The same problem would occur as with DNF4. It would not work for other applications (users of the libdnf5 library).

How to get out of this?
We need an interface defined in the libdnf5 library for this case.
Queries whether to do key import are done via callbacks. Similarly, queries to remove them (with possible information why) should be handled via callbacks.

@jan-kolarik
Copy link
Member Author

I looked into the source code. It's a rewrite of the new "expired-gpg-keys" plugin from DNF4 to libdnf5.

And here's the problem. The original plugin is in Python. It is not part of the libdnf library -> this makes it ignored by users using libdnf directly - microdnf, PackageKit, ... And it behaves more like an application plugin - it interacts with the user via the console.

This PR implements the libdnf5 plugin. So it is a library plugin. The advantage is that it will be active for all users of the libdnf5 library (dnf5 application, dnf5daemon-server, ...). However, the library must not interfere with the console. For example, dnf5daemon-server does not have a console to communicate with the user - it is a background daemon.

The dnf5 application only supports "command" plugins. And generally making this plugin as an application plugin is not a good way to go. The same problem would occur as with DNF4. It would not work for other applications (users of the libdnf5 library).

How to get out of this? We need an interface defined in the libdnf5 library for this case. Queries whether to do key import are done via callbacks. Similarly, queries to remove them (with possible information why) should be handled via callbacks.

That's a good point Jarek, I wasn't thinking about non-CLI users here. I will rework the plugin as you've suggested, thanks for that!

@jan-kolarik jan-kolarik added the blocked Further work on issue or PR is blocked by something else label Jul 25, 2024
@pmatilai
Copy link
Member

Looking at the expiriation date will miss what I think is the more common issue just now: all the obsolete SHA1 based signatures that will fail in various other ways. Sequoia generally defers this checking to verify rather than import time, but you might want to additionally call pgpPubKeyLint() on the key material to catch at least some of those issues too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Further work on issue or PR is blocked by something else test required
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

3 participants