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

dnf5 install/upgrade/... --downloadonly and dnf5 download should check package signatures #1985

Open
marmarek opened this issue Jan 4, 2025 · 7 comments
Labels
Priority: LOW RFE Request For Enhancement (as opposed to a bug) Triaged Someone on the DNF 5 team has read the issue and determined the next steps to take

Comments

@marmarek
Copy link

marmarek commented Jan 4, 2025

It looks like dnf5 checks signatures only just before installing packages, but not after downloading them. dnf4 verified signatures just after downloading. This has significant security implication for --downloadonly option! Thankfully I have a CI check for this case...

Reproducer:

[user@testbuilder dd]$ cat /etc/yum.repos.d/unknown-key.repo 
[unknown-key]
name=unknown-key
gpgkey=file:///etc/pki/rpm-gpg/RPM-GPG-KEY-fedora-41-primary
baseurl=https://ftp.qubes-os.org/~marmarek/repo-verify-unknown-key
gpgcheck=1

[user@testbuilder dd]$ sudo dnf4 install --repoid=unknown-key -y --downloadonly tmux
unknown-key                                     2.6 kB/s | 580  B     00:00    
Dependencies resolved.
================================================================================
 Package      Architecture   Version                  Repository           Size
================================================================================
Installing:
 tmux         x86_64         1000:500-1.fc30          unknown-key         5.8 k

Transaction Summary
================================================================================
Install  1 Package

Total download size: 5.8 k
Installed size: 0  
DNF will only download packages for the transaction.
Downloading Packages:
tmux-500-1.fc30.x86_64.rpm                       53 kB/s | 5.8 kB     00:00    
--------------------------------------------------------------------------------
Total                                            51 kB/s | 5.8 kB     00:00     
unknown-key                                     1.6 MB/s | 1.6 kB     00:00    
GPG key at file:///etc/pki/rpm-gpg/RPM-GPG-KEY-fedora-41-primary (0xE99D6AD1) is already installed
The GPG keys listed for the "unknown-key" repository are already installed but they are not correct for this package.
Check that the correct key URLs are configured for this repository.. Failing package is: tmux-1000:500-1.fc30.x86_64
 GPG Keys are configured as: file:///etc/pki/rpm-gpg/RPM-GPG-KEY-fedora-41-primary
The downloaded packages were saved in cache until the next successful transaction.
You can remove cached packages by executing 'dnf clean packages'.
Error: GPG check FAILED
[user@testbuilder dd]$ echo $?
1
[user@testbuilder dd]$ sudo dnf5 install --repoid=unknown-key -y --downloadonly tmux
Updating and loading repositories:
Repositories loaded.
Package                 Arch    Version                  Repository         Size
Installing:
 tmux                   x86_64  1000:500-1.fc30          unknown-key     0.0   B

Transaction Summary:
 Installing:         1 package

Total size of inbound packages is 6 KiB. Need to download 0 B.
After this operation, 0 B extra will be used (install 0 B, remove 0 B).
The operation will only download packages for the transaction.
[1/1] tmux-1000:500-1.fc30.x86_64       100% |   0.0   B/s |   0.0   B |  00m00s
>>> Already downloaded
--------------------------------------------------------------------------------
[1/1] Total                             100% |   0.0   B/s |   0.0   B |  00m00s
Complete!
[user@testbuilder dd]$ echo $?
0
marmarek added a commit to marmarek/qubes-builderv2 that referenced this issue Jan 4, 2025
dnf5 install --downloadonly does not verifies signatures, so use the old
one for the time being.

Workaround for rpm-software-management/dnf5#1985
marmarek added a commit to marmarek/qubes-builderv2 that referenced this issue Jan 6, 2025
dnf5 install --downloadonly does not verifies signatures, so use the old
one for the time being.

Workaround for rpm-software-management/dnf5#1985
@ppisar
Copy link
Contributor

ppisar commented Jan 6, 2025

In my opinion this is a feature. What's the point in verifying the signatures twice? Once at download and later at installation.

@marmarek
Copy link
Author

marmarek commented Jan 6, 2025

The current (new) behavior of --downloadonly easily lead to a situation where signatures are not verified at all, which is clearly worse.

This is especially the case when somebody relied on previous behavior for verifying signatures - dnf5 is supposed to be a drop-in replacement, and when you call it via simply "dnf" name, you may even not notice that your script suddenly skip signature verification completely.
As I said in the description, I have comprehensive tests for signature verification, so I found this out early enough, but some users may not be that lucky...

@ppisar
Copy link
Contributor

ppisar commented Jan 6, 2025

DNF5 verifies signatures at installation, i.e. when passing the packages to RPM library which performs the RPM transaction. Can you please show me where DNF5 does not verify signatures which are being installed?

DNF5 is not a bug-compatible clone of DNF4. There are certainly some differences and some are deliberate. Like this one.

We could add a feature to DNF5 to verify signature at download. But I don't see a reason why it should be enabled by default. Please show us the attack vector so we can understand why it is necessary to check signatures with "dnf install --downloadonly".

@marmarek
Copy link
Author

marmarek commented Jan 6, 2025

DNF5 is not a bug-compatible clone of DNF4.

Well, then IMHO /usr/bin/dnf shouldn't point at DNF5...
But also, skipping signature check on --downloadonly should at the very least be a documented behavior change - I can't find it at https://dnf5.readthedocs.io/en/latest/changes_from_dnf4.7.html

DNF5 verifies signatures at installation, i.e. when passing the packages to RPM library which performs the RPM transaction. Can you please show me where DNF5 does not verify signatures which are being installed?

Sure, imagine you have an offline system where you want to install some packages/updates. You use another (online) system to download packages and then copy them on on USB stick. You have a set of scripts for that, and they used to correctly verify signatures with previous DNF version. After updating from Fedora 40 to Fedora 41, all still appear to work without errors, so you wouldn't suspect the update actually introduced a security hole...

For simplicity, this is how the two sides could look like in one go:

[user@testbuilder d]$ sudo dnf5 install --downloadonly tmux
Updating and loading repositories:
 Fedora 41 - x86_64 - Updates           100% |   3.8 MiB/s |   7.8 MiB |  00m02s
 Fedora 41 openh264 (From Cisco) - x86_ 100% |   4.2 KiB/s |   6.0 KiB |  00m01s
 Qubes OS Repository for VM (updates)   100% | 186.5 KiB/s |  77.0 KiB |  00m00s
 Fedora 41 - x86_64                     100% |  14.5 MiB/s |  35.4 MiB |  00m02s
 unknown-key                            100% |  18.9 KiB/s |   3.7 KiB |  00m00s
Repositories loaded.
Package                 Arch    Version                  Repository         Size
Installing:
 tmux                   x86_64  1000:500-1.fc30          unknown-key     0.0   B

Transaction Summary:
 Installing:         1 package

Total size of inbound packages is 6 KiB. Need to download 6 KiB.
After this operation, 0 B extra will be used (install 0 B, remove 0 B).
The operation will only download packages for the transaction.
Is this ok [y/N]: y
[1/1] tmux-1000:500-1.fc30.x86_64       100% |  62.8 KiB/s |   5.8 KiB |  00m00s
--------------------------------------------------------------------------------
[1/1] Total                             100% |  56.7 KiB/s |   5.8 KiB |  00m00s
Complete!

# now simulate copying the package to the other system

[user@testbuilder d]$ find /var/cache/dnf -name 'tmux*rpm' -exec cp -t . {} +
[user@testbuilder d]$ sudo dnf install ./tmux-500-1.fc30.x86_64.rpm
Updating and loading repositories:
Repositories loaded.
Package                 Arch   Version                  Repository          Size
Installing:
 tmux                   x86_64 1000:500-1.fc30          @commandline     0.0   B

Transaction Summary:
 Installing:         1 package

Total size of inbound packages is 6 KiB. Need to download 0 B.
After this operation, 0 B extra will be used (install 0 B, remove 0 B).
Is this ok [y/N]: y
Running transaction
[1/3] Verify package files              100% | 200.0   B/s |   1.0   B |  00m00s
[2/3] Prepare transaction               100% |  12.0   B/s |   1.0   B |  00m00s
[3/3] Installing tmux-1000:500-1.fc30.x 100% |  59.0   B/s | 124.0   B |  00m02s
Warning: skipped OpenPGP checks for 1 package from repository: @commandline
Complete!

It does warn me that signature was not verified at the install stage, but since the download stage was clean (and used to verify signature) one can reasonably assume all is fine.

PS The DNF4 version could use --destdir to avoid copying files from DNF cache, but that isn't implemented in DNF5.
PPS One could use dnf download instead, but that doesn't verify signatures (in neither DNF4 nor DNF5) and manually verifying signatures can be quite tedious, since different repositories may have different keys (and some may have intentional gpgcheck=0, which you'd need to handle manually then). Furthermore, dnf download doesn't support groups (#1129)

@ppisar ppisar added RFE Request For Enhancement (as opposed to a bug) Priority: LOW Triaged Someone on the DNF 5 team has read the issue and determined the next steps to take labels Jan 7, 2025
@ppisar
Copy link
Contributor

ppisar commented Jan 7, 2025

In short you used "dnf download" for verifying signatures and then you neglected the verification when installing them from a local file system. Your workflow has a flaw that tampering the package after the download won't be detected. I recommend you setting "localpkg_gpgcheck" to true on your final system.

I can see that "dnf download" or "dnf install --downloadonly" could be used as a tool for checking the signatures. I think we can enable the verification in that case. I believe that "dnf install" without the "--downloadonly" option should keep not verifying the signatures.

@ppisar ppisar changed the title dnf5 install --downloadonly does not check package signatures dnf5 install/upgrade/... --downloadonly and dnf5 download should check package signatures Jan 7, 2025
@marmarek
Copy link
Author

marmarek commented Jan 7, 2025

What should happen to packages that fails signature verification? Should those be handled the same as failing checksum against repo metadata? AFAIK many package managers will remove such failing packages instead of keeping them in cache, so it gets re-downloaded next time. Should DNF5 do the same?

I recommend you setting "localpkg_gpgcheck" to true on your final system.

Generally I agree this is a good idea, but with packages coming from different repositories with different keys, and some from local repo with gpgcheck=0, it's tricky.

@ppisar
Copy link
Contributor

ppisar commented Jan 7, 2025

If the verification fails, I would keep the packages there, but print an error and exit with a non-zero code. That way the user can inspect the failed packages.

Actively removing the packages is another option. Then the user would have to use --no-gpgchecks option to prevent from the infinite downloading.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: LOW RFE Request For Enhancement (as opposed to a bug) Triaged Someone on the DNF 5 team has read the issue and determined the next steps to take
Projects
None yet
Development

No branches or pull requests

2 participants