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

OpenPGP: Function pgpParsePkts parses only first ascii-armor block #2487

Closed
jrohel opened this issue Apr 17, 2023 · 7 comments
Closed

OpenPGP: Function pgpParsePkts parses only first ascii-armor block #2487

jrohel opened this issue Apr 17, 2023 · 7 comments
Labels
crypto Signatures, keys, hashes and their verification

Comments

@jrohel
Copy link

jrohel commented Apr 17, 2023

Problem: After first ascii-armor block function returns. Next blocks are ignored.

Example solutions:

  1. Parse all blocks.
  2. New function that parse one block but returns one more parameter with offset of the block ends. To be able to call the function again from that point.
@jrohel jrohel changed the title Function pgpParsePkts parses only first packet OpenPGP: Function pgpParsePkts parses only first packet Apr 17, 2023
@nwalfield
Copy link
Contributor

Here's the documentation for pgpParsePkts:

/** \ingroup rpmpgp
 * Parse armored OpenPGP packets from memory.
 * @param armor		armored OpenPGP packet string
 * @param[out] pkt	dearmored OpenPGP packet(s) (malloced)
 * @param[out] pktlen	dearmored OpenPGP packet(s) length in bytes
 * @return		type of armor found
 */
pgpArmor pgpParsePkts(const char *armor, uint8_t ** pkt, size_t * pktlen);

I'm pretty certain this doesn't parse one packet, but all of the packets in one ascii-armor block. Perhaps you mean that you have multiple arscii-armor blocks. Here's how rpmkeys --import does that. It would be helpful if you provided a reproducer so I could better understand your problem.

@jrohel
Copy link
Author

jrohel commented Apr 18, 2023

@nwalfield

  1. Yes. I meant the case when there are multiple ascii-armor blocks in one file (or memory block). I apologize for the terminology confusion.

  2. I'm looking into how the code you linked for rpmkeys --import does this. The code shows exactly the same problem I ran into. It solves the problem as I suggested in point 2. That is, the pgpParsePkts function is called multiple times in a do loop. And the fact that it doesn't return the offset of the end of the processed ascii-armor block is solved (a dirty hack from my point of view) by searching the already parsed block with the strstr function (search for string "-----BEGIN PGP ").
    So I'll do the same, but correctly my code shouldn't know that the ascii-armor block starts with the string "-----BEGIN PGP ". The pgpParsePkts function will find the first occurrence of the beginning and end of the block by itself. It's its job to parse the ascii-armor data. And it's a shame that it doesn't return the offset of the end of the parsed block (even though it knows that), and instead I have to search the block again to find where the next one starts. But OK, it's still better than writing a whole parser.

Anyway, thanks for the response.

@jrohel jrohel changed the title OpenPGP: Function pgpParsePkts parses only first packet OpenPGP: Function pgpParsePkts parses only first ascii-armor block Apr 18, 2023
@pmatilai
Copy link
Member

pmatilai commented Apr 18, 2023

The pgp-related APIs in rpm are indeed rather terrible. The loop in rpm's --import was a bandaid fix for a case where RHEL 6 (yes, really) needed to deliver multiple keys in a single file. This worked with yum but then it was discovered that rpm doesn't, and I had to cobble up something quick. And then promptly stuffed in the closet of API horrors, dirty kludges and other skeletons and forgotten.

@nwalfield
Copy link
Contributor

I've added a note to #2401. I think this issue can be closed.

@pmatilai
Copy link
Member

The above should probably be #2041

@nwalfield
Copy link
Contributor

Indeed, sorry.

@pmatilai pmatilai added the crypto Signatures, keys, hashes and their verification label Sep 14, 2023
@pmatilai
Copy link
Member

Clarified the documentation in bccf58f (and managed to typo the ticket number, sigh)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crypto Signatures, keys, hashes and their verification
Projects
None yet
Development

No branches or pull requests

3 participants