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

rpm segfaults when importing keys downloaded from keys.openpgp.org #3001

Open
signed-log opened this issue Mar 27, 2024 · 7 comments
Open

rpm segfaults when importing keys downloaded from keys.openpgp.org #3001

signed-log opened this issue Mar 27, 2024 · 7 comments
Labels
Milestone

Comments

@signed-log
Copy link

Describe the bug
A clear and concise description of what the bug is.

Importing a key with User-ID removed (like those of keys.openpgp.org) causes RPM to segfault

To Reproduce
Steps to reproduce the behavior:

  1. Start condition e.g. installed packages
  2. Command (line) executed
  3. Error encountered

1: Download a Armored and User-ID stripped key from keys.openpgp.org (like the one for Tailscale)
2: Try to import it

Expected behavior
A clear and concise description of what you expected to happen.

Import the key normally

Output
If applicable, add copy of the output on the command line or a screenshots to help explain your problem.

[1] 23347 segmentation fault sudo rpm --import ./tailscale_ko.asc

Strace output https://paste.opensuse.org/pastes/07914597fa8b

Environment

  • OS / Distribution: [e.g. Fedora 42]
  • Version [e.g. rpm-4.19.0]
    openSUSE Tumbleweed 20240325
    4.19.1.1

Additional context
Add any other context about the problem here.

@pmatilai pmatilai self-assigned this Mar 28, 2024
@pmatilai
Copy link
Member

pmatilai commented Apr 2, 2024

Right, this is specific to the internal pgp parser. With rpm-sequoia I get:

$ tools/rpmkeys --dbpath /tmp/kdb --import /tmp/2596A99EAAB33821893C0A79458CA832957F5868
error: Certificate 458CA832957F5868:
Policy rejects 458CA832957F5868: No binding signature at time 2024-04-02T10:42:20Z
error: /tmp/2596A99EAAB33821893C0A79458CA832957F5868: key 1 import failed.

but easy enough to reproduce with the old parser. This is enough prevents the crash:

--- a/rpmio/rpmpgp_internal.c
+++ b/rpmio/rpmpgp_internal.c
@@ -1079,6 +1079,8 @@ int pgpPrtParamsSubkeys(const uint8_t *pkts, size_t pktlen,
 
            digps[count] = pgpDigParamsNew(PGPTAG_PUBLIC_SUBKEY);
            /* Copy UID from main key to subkey */
+           if (!mainkey->userid)
+               break;
            digps[count]->userid = xstrdup(mainkey->userid);
 
            if (getKeyID(pkt.body, pkt.blen, digps[count]->signid)) {

It wont import the key, but if main userid is missing in the key then maybe it shouldn't.
The internal parser is now gone from the upstream rpm repo but it's of course still there in 4.19.x. @mlschroe, do you want to have a closer look or just go with something like the above as minimal bandaid?

@pmatilai
Copy link
Member

pmatilai commented Apr 2, 2024

Oh and, thanks @signed-log for reporting!

@pmatilai
Copy link
Member

pmatilai commented Apr 2, 2024

Heh, so a more careful reading of the report... the userid is intentionally removed here.
So assuming that's a reasonable thing to do (considering where these keys are coming from), the minimal fix would probably be this instead:

-           digps[count]->userid = xstrdup(mainkey->userid);
+           if (mainkey->userid)
+               digps[count]->userid = xstrdup(mainkey->userid);

There could be other places that rely on the userid being there besides this.

@mlschroe
Copy link
Contributor

mlschroe commented Apr 2, 2024

Yeah, that's also what I was going to implement. The userid seems to be optional.

@mlschroe
Copy link
Contributor

mlschroe commented Apr 2, 2024

Fixed with rpm-software-management/rpmpgp_legacy@31c2f3d

@pmatilai pmatilai added this to the 4.19.2 milestone Apr 2, 2024
@pmatilai pmatilai added the bug label Apr 2, 2024
@pmatilai pmatilai removed their assignment Apr 4, 2024
@signed-log
Copy link
Author

Thanks a lot !

@pmatilai
Copy link
Member

pmatilai commented Apr 8, 2024

Reopening - we want to track this for the next 4.19.x bugfix release.

@pmatilai pmatilai reopened this Apr 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Todo
Development

No branches or pull requests

3 participants