rpm-software-management / dnf5

Next-generation RPM package management system
Other
248 stars 85 forks source link

Keyring directory for repo_gpgcheck not removed by `dnf5 clean all` from /var/cache #838

Open evan-goode opened 1 year ago

evan-goode commented 1 year ago

Probably NOTABUG, but this was noticed by a participant in the 2023-08-11 test week. The pubring directory for repositories is not touched by dnf5 clean all, so some data may still exist in /var/cache/libdnf5 after dnf5 clean all.

Steps to reproduce using the third-party VSCodium repository:

  1. From https://gitlab.com/paulcarroty/vscodium-deb-rpm-repo:

    sudo tee -a /etc/yum.repos.d/vscodium.repo << 'EOF'
    [gitlab.com_paulcarroty_vscodium_repo]
    name=gitlab.com_paulcarroty_vscodium_repo
    baseurl=https://paulcarroty.gitlab.io/vscodium-deb-rpm-repo/rpms/
    enabled=1
    gpgcheck=1
    repo_gpgcheck=1
    gpgkey=https://gitlab.com/paulcarroty/vscodium-deb-rpm-repo/raw/master/pub.gpg
    metadata_expire=1h
    EOF
  2. sudo dnf5 makecache (import necessary PGP keys)

  3. sudo dnf5 clean all

  4. ls /var/cache/libdnf5

ppisar commented 1 year ago

I confirm that this happens. A trigger is "repo_gpgcheck=1" which leads DNF5 to download the key, create a GnuPG keyring in ./pubring and import the key there. "dnf clean all" does not remove that directory:

# dnf5 clean all
Removed 4 files, 2 directories. 0 errors occurred.
root@fedora-40:/var/cache/libdnf5/gitlab.com_paulcarroty_vscodium_repo-82ebb956b01ee6dd # find
.
./pubring
./pubring/pubring.kbx
./pubring/private-keys-v1.d
./pubring/pubring.kbx~
./pubring/trustdb.gpg

I guess this keyring is only used for verifying PGP signatures of the repository metadata. RPM package signature verification uses a different keyring, probably an RPM keyring. I'm not sure whether DNF5 actually can verify signatures of repository metadata.

If I remove that ./pubring directory manually, next time "dnf5 makecache" recreates it and asks a user again for accepting that key. If I keep the directory there, "dnf5 makecache" does not recreate it and does not ask for the import.

I believe that DNF5 should store the accepted key (or at least its fingerprint) in a permanent storage different from /var/cache, probably in DNF5 database. The key acceptance should survive cleaning a cache. The life cycle of the key should be bound to a definition of the repository (/etc/yum.repos.d/) not to a repository metadata which possibly changes with every makecache.

I conclude that "dnf5 clean all" should prune the cached ./pubring directory. But first, I'd like to hear from DNF5 developers what's the status of repository metadata signature verification. If the verification is not implemented, maybe creating the cached ./pubring should be disabled until the verification starts doing anything useful.

m-blaha commented 1 year ago

I conclude that "dnf5 clean all" should prune the cached ./pubring directory. But first, I'd like to hear from DNF5 developers what's the status of repository metadata signature verification. If the verification is not implemented, maybe creating the cached ./pubring should be disabled until the verification starts doing anything useful.

The repodata signature verification feature is implemented in librepo and libdnf5 uses it. The keyring location is defined here: https://github.com/rpm-software-management/dnf5/blob/da05ceb1e3d1129fd66b8c840bdf28c41f9149ca/libdnf5/repo/repo_pgp.hpp#L51

j-mracek commented 1 year ago

I think we should about a functionality for better handling of keys. We still need to allow key import for non priviligge user to user specific key-ring.

Suggesting following steps:

  1. Allow pgp verification from RPM DB key-rings for all repositories - (in root, and user cache)
  2. Optional - Allow disablement of repo key import even when --assumeyes is used (possible to achieve it by not providing import urls)
  3. Redesign repo_gpg_check meaning - long term - requires system wide change in Fedora (not before Fedora 41)
  4. Allow key management - RPMDB, repository cache
jrohel commented 1 year ago

Not wiping the keyring after calling dnf5 clean all was intentional. The goal was to make repository keys "permanent". So that the user does not have to re-import the keys after each cache deletion. And yes, permanent stuff should be stored somewhere other than the cache. This approach was copied from DNF4. Which deletes only the files corresponding to the defined pattern from the cache - it does not delete the keyring.

ppisar commented 1 year ago

On meeting we concluded that we need to find a permanent storage for the key which can be maintained by a non-superuser. That excludes a system-wide RPM database and /etc/pki/.