expired-gpg-keys: new plugin to detect expired GPG keys

[FrostyX]

Workaround for: rpm-software-management/dnf#2075

[FrostyX]

The implementation is of course not great (not using logger, not using _, etc) but as a proof of concept it works. What do you think of this approach as a temporary workaround until rpm-software-management/dnf#2075 gets properly fixed?

[jan-kolarik]

Hi and thanks a lot for working on this! I'm planning to dig into it this week. Additionally, some related work appeared recently on the RPM side (link), so I'll also check that one as it might significantly affect the current situation of handling expired keys.

[FrostyX]

Hello @jan-kolarik, I am glad to hear that.

Additionally, some related work appeared recently on the RPM side (rpm-software-management/rpm#3083), so I'll also check that one as it might significantly affect the current situation of handling expired keys.

I noticed the RPM PR just a couple of minutes after proposing this plugin, and at first, I was mad at myself because I thought this plugin was a wasted effort. But I don't think so anymore.

I suppose the RPM change won't land into any currently stable Fedora version but rather only in something like F41+? That's just my unsubstantiated assumption, we should get the correct information on this. But if true, any DNF fix based on the new RPM code won't be available to the users for many months. So maybe having this workaround will be useful in the meantime, what do you think?

Hi and thanks a lot for working on this! I'm planning to dig into it this week.

Feel free to take the code from this PR and make it production-ready.

Edit: I sent you an invite to be a collaborator to my dnf-plugins-core repository, so you can push directly to this PR if you want to.

[jan-kolarik]

I'll check back probably next week. For now, I can say this seems to be the direction we want to take, but we need to agree on it as a team and decide whether to implement it as core functionality or leave it as a plugin.

[jan-kolarik]

Following-up on the comment from the dnf5 issue, we are planning to start integrating the work based on this PR into dnf4 and dnf5, probably during the next sprint (next week).

[FrostyX]

Perfect, I am glad to hear that @jan-kolarik. Please close the PR once it is no longer relevant.

[jan-kolarik]

I did some refactoring, added checks, and am now using the RPM API directly for package removal.

The current state is not final, though. Here are some thoughts:

• Detection of expired GPG keys now uses Python's subprocess. This isn't ideal, but we probably don't want to introduce a new dependency on the GPG Python API from the python-gnupg package or others.
• When should we trigger the user prompt and removal? Currently, it’s during config time, but it might be better to move this to pre-transaction, as additional downloads of GPG keys already happen then. At that point, we could also filter which repositories we're using and check if those specific repositories have GPG checks enabled, etc.
• How should we deploy the plugin? Should we offer it to users by default or explicitly through a separate package? Since it’s a plugin that changes DNF behavior (like local, which is distributed and configured separately), we need to consider the best approach.

[FrostyX]

Here are some thoughts:

I suppose the questions are mainly for other DNF devs and @ppisar who have been requested for a review. But I will answer them from the point of the Copr team:

Detection of expired GPG keys now uses Python's subprocess. This isn't ideal, but we probably don't want to introduce a new dependency on the GPG Python API from the python-gnupg package or others.

With your last commit, we minimalized the number of subprocess calls. I don't mind the one remaining call.

When should we trigger the user prompt and removal?

No preference here, both config time and pre-transaction time are fine. Whatever your team prefers more.

How should we deploy the plugin? Should we offer it to users by default or explicitly through a separate package?

Here I would like to implore you to ship this to as many users as possible. As I mentioned in several other discussions, this issue happens to a lot of users and they are confused about what it means and what to do. Pointing them to install a DNF plugin is a bit (but not much) helpful then telling them manual steps to remove the broken GPG keys. I would like the users to not even encounter the issue in the first place.

[ppisar]

• Detection of expired GPG keys now uses Python's subprocess. This isn't ideal, but we probably don't want to introduce a new dependency on the GPG Python API from the python-gnupg package or others.

I guess python-gnupg also executes /usr/bin/gpg underneath. Then executing gpg directly is fine.

We removed using gpg from libdnf to prevent from starting GnuPG agent, creating temporary GnuPG directories etc. From my first checks it seems that gpg --show-keys does do any of the nasty things. But we should double check it.

• When should we trigger the user prompt and removal? Currently, it’s during config time, but it might be better to move this to pre-transaction, as additional downloads of GPG keys already happen then. At that point, we could also filter which repositories we're using and check if those specific repositories have GPG checks enabled, etc.

pre-transaction makes more sense as we would touch the keys only when a user intents to edit the RPM database. Does the current implementation raise a question for any DNF invocation (e.g. repoquery) even when run by a non-superuser?

• How should we deploy the plugin? Should we offer it to users by default or explicitly through a separate package? Since it’s a plugin that changes DNF behavior (like local, which is distributed and configured separately), we need to consider the best approach.

Ideally we need to get it everywhere. But as you wrote, it's a change.

We could start with a separate package. Make a publicity to get a real-life testing and then in case of no serious problems, open a Fedora system-wide change and make this plugin default.

One controversial issue I foresee is removing keys used for already installed packages when the repository does not offer an updated key: Users will remove the key becasuse it was advised by DNF. Then "rpm --verify" will start failing because of missing a key (that could be defendable as expired keys are support not to be trusted). Later if the users attempts to install an old package (e.g. the repository is not maintained anymore, I fear to say a common case for various COPR repositories), rpm will reject importing the missing key because it will be expired then.

What we now do is hardening a security and people will be mouthful because their insecure workflows will stop working for them.

Despite all of that, I'm generally fine with this code.

[ppisar]

One thing: This pull request is missing a manual page for the new plugin.

[pep8speaks]

Hello @FrostyX! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-07-19 07:31:10 UTC

[jan-kolarik]

I made several changes in the latest version:

• Execute the action at pre-transaction time. This way, it won't affect any queries or commands that don't involve PGP keys.
• Execute the action only when there is a forward action in the transaction (e.g., install, reinstall, upgrade).
• Fixed text and changed GPG references to PGP.

There is still room for improvement, such as checking only the keys of repositories affected by the transaction. However, this may be complex since the GPG keys in repo files might not be limited to local files.

Given the urgency to address issues in Copr, I am more inclined towards incorporating this functionality as the default behavior. There will be a configuration option to disable it, allowing for an easy rollback if needed.

It is a valid point about filing a Fedora change for this. However, this would delay the delivery to the next release and not (yet) backport it to older releases, which may not be acceptable to the reporters here (@FrostyX @praiskup ?)

Later if the users attempts to install an old package (e.g. the repository is not maintained anymore, I fear to say a common case for various COPR repositories), rpm will reject importing the missing key because it will be expired then.

@praiskup @FrostyX, do you have any feedback on this concern?

[FrostyX]

I made several changes in the latest version:

Everything sounds reasonable, thank you very much @jan-kolarik

It is a valid point about filing a Fedora change for this.

From my point of view, a Fedora change (are we talking about this, right?) shouldn't be needed. Even though this is a new plugin, the whole thing is basically a bugfix. DNF users in general, shouldn't notice any difference and everything should behave exactly the same as before. Except for this one case when there are expired keys. Then instead of basically a traceback, they get a friendly prompt. And just in case, we have an opt-out for this.

That being said, I haven't yet maintained anything so fundamental, that its change would require a Fedora change proposal, so I don't have enough experience with the process. If you think it's needed, I will trust your judgment.

Later if the users attempts to install an old package (e.g. the repository is not maintained anymore, I fear to say a common case for various COPR repositories), rpm will reject importing the missing key because it will be expired then.

I don't know how often this will happen and if people will complain. But if this happens, should/could this be fixed on the Copr side by making sure all packages are signed with not-expired keys?

[jan-kolarik]

From my point of view, a Fedora change (are we talking about this, right?)

Yep.

Even though this is a new plugin, the whole thing is basically a bugfix. DNF users in general, shouldn't notice any difference and everything should behave exactly the same as before. Except for this one case when there are expired keys. Then instead of basically a traceback, they get a friendly prompt. And just in case, we have an opt-out for this.

I agree with you. However, the the whole thing is basically a bugfix is rather subjective I'd say. I’m not sure if there was ever a specified functionality for managing expired PGP keys in DNF, so it could also be considered a new feature.

But if this happens, should/could this be fixed on the Copr side by making sure all packages are signed with not-expired keys?

I assume so, but it would be better to get @praiskup's view on this.

[praiskup]

Later if the users attempts to install an old package (e.g. the repository is not maintained anymore, I fear to say a common case for various COPR repositories), rpm will reject importing the missing key because it will be expired then.

This shouldn't be a concern. Copr prolongs the keys even for "inactive" projects (actually, the project activity isn't monitored at this point in time, practically there aren't active/inactive projects). There are some end-of-life policies which though cause a project/build removals (then RPMs are removed, too). Any RPM being installed from Copr thus has a valid signature.

It is a valid point about filing a Fedora change for this. However, this would delay the delivery to the next release and not (yet) backport it to older releases, which may not be acceptable to the reporters here (@FrostyX @praiskup ?)

Ideal case would be to go through a normal Fedora change policy, and for the stable releases have an option to turn this feature ON. Not sure if this is doable? My personal opinion is that the problem isn't a burning fresh regression, it is already quite old one. So IMVHO we don't have to hurry that much and risk problems in production systems ...

[ppisar]

I'm thinking about enhancing the message with information about drawbacks of removing the key:

print(_("As a result, installing packages signed with this key will fail.\n"
        "It is recommended to remove the expired key to allow importing\n"
        "an updated key. This might leave already installed packages unverifiable."))

That way the user will be able able to evaluate pros and cons and make an informed decision about removing the key.

[jan-kolarik]

Thanks for the feedback everyone! Since there’s no urgency to make this behavior the default, I propose making it a Fedora change. With dnf5 already the default package manager in Fedora 41+, this change will be connected with dnf5. A follow-up PR based on this one is planned right after it’s merged.

I’ve modified the plugin to be turned off by default and improved the prompt message based on feedback from @ppisar.

[ppisar]

Great. @jan-kolarik, feel free to merge it.