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

Use RPMSIG_SIGNATURE_TYPE and rpmcliVerifySignatures() for all RPM-based signature verification #307

Open
DemiMarie opened this issue Mar 30, 2021 · 10 comments · May be fixed by #308
Open

Comments

@DemiMarie
Copy link

DemiMarie commented Mar 30, 2021

This is libzypp’s version of CVE-2021-3445. Only users who have turned off repository signature verification for at least one repository are vulnerable, unlike DNF which is vulnerable by default. See rpm-software-management/libdnf#1179 and rpm-software-management/dnf#1752.

Reporting publicly because this is already public in other repositories and because default configurations are not vulnerable.

@mlandres
Copy link
Member

mlandres commented Apr 6, 2021

@DemiMarie do you have some more details about the nature of CVE-2021-3445? Why should be calling ::rpmcliVerifySignatures more secure than ::rpmVerifySignatures, which is what zypp calls (plus parsing the output to grep keyids and unsigned packages).

Do you happen to have some 'bad' package we can use for testing?

@mlschroe
Copy link
Member

mlschroe commented Apr 6, 2021

rpmVerifySignatures and rpmcliVerifySignatures are pretty much the same. The difference is that you can verify multiple rpms with rpmcliVerifySignatures. I don't see any other meaningful difference.

Mayby this is about rpmtsSetVfyLevel(ts, RPMSIG_SIGNATURE_TYPE);, but that would disable signature checks, so do not do that...

@mlschroe
Copy link
Member

mlschroe commented Apr 6, 2021

Wait, I mixed up SetVfyLevel() with SerVfyFlags.

@mlschroe
Copy link
Member

mlschroe commented Apr 6, 2021

rpmtsSetVfyLevel(ts, RPMSIG_SIGNATURE_TYPE); would make rpm insist on a signature.

@mlschroe
Copy link
Member

mlschroe commented Apr 6, 2021

Libzypp already checks the output of the signature verification and (if configured) rejects packages with no signature anyway, so this is not needed.

@mlandres
Copy link
Member

mlandres commented Apr 8, 2021

@DemiMarie do you have some more details about the nature of CVE-2021-3445? Why should be calling ::rpmcliVerifySignatures be more secure than ::rpmVerifySignatures, which is what zypp calls (plus parsing the output to grep keyids and unsigned packages).

Where do you think our code is vulnerable, or was his issue just filed to make us aware of the CVE?

@DemiMarie
Copy link
Author

@DemiMarie do you have some more details about the nature of CVE-2021-3445? Why should be calling ::rpmcliVerifySignatures be more secure than ::rpmVerifySignatures, which is what zypp calls (plus parsing the output to grep keyids and unsigned packages).

CVE-2021-3445 is a signature verification bypass in libdnf: it turns out that old versions of librpm (before the CVE-2021-3421 fix) allow for signatures to be located in the immutable header, which RPM will not check. This allowed libdnf to be tricked into thinking a package was validly signed when it was not. Using ::rpmVerifySignatures instead of ::rpmcliVerifySignatures is fine.

Where do you think our code is vulnerable, or was his issue just filed to make us aware of the CVE?

As per discussion with Red Hat Product Security, RPM does not guarantee that signatures will be checked unless rpmtsSetVfyLevel(ts, flags) is called, with flags including RPMSIG_SIGNATURE_TYPE. I strongly recommend rpmtsSetVfyFlags(ts, 0) as well, to guard against user misconfigurations. I sent the details privately to SUSE Security since I would rather not make them public until DNF, libdnf, and libzypp are patched and the patched versions released.

@mlandres
Copy link
Member

mlandres commented Apr 8, 2021

Thanks. We'll review the code.

@DemiMarie
Copy link
Author

#308

@DemiMarie DemiMarie linked a pull request Apr 11, 2021 that will close this issue
@DemiMarie
Copy link
Author

Libzypp already checks the output of the signature verification and (if configured) rejects packages with no signature anyway, so this is not needed.

This is not sufficient; details have been sent privately to SUSE security.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants