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

Support PGP Ed25519 keys #1438

Open
jas4711 opened this issue Apr 14, 2023 · 6 comments
Open

Support PGP Ed25519 keys #1438

jas4711 opened this issue Apr 14, 2023 · 6 comments
Labels
enhancement New feature or request

Comments

@jas4711
Copy link

jas4711 commented Apr 14, 2023

Hi. It seems rekor does not support OpenPGP Ed25519 keys, please add support for them. Thank you!

jas@kaka:~$ echo foo > foo.txt
jas@kaka:~$ gpg --detach-sign foo.txt
jas@kaka:~$ gpg --verify foo.txt.sig foo.txt
gpg: Signature made Fri Apr 14 19:03:46 2023 CEST
gpg:                using EDDSA key A3CC9C870B9D310ABAD4CF2F51722B08FE4745A2
gpg: Good signature from "Simon Josefsson <simon@josefsson.org>" [ultimate]
jas@kaka:~$ gpg --export A3CC9C870B9D310ABAD4CF2F51722B08FE4745A2 > key.txt
jas@kaka:~$ rekor-cli upload --signature foo.txt.sig --artifact foo.txt --public-key key.txt 
error: error retrieving external entities: invalid PGP signature: openpgp: unsupported feature: public key algorithm 22
jas@kaka:~$ 
@jas4711 jas4711 added the enhancement New feature or request label Apr 14, 2023
@haydentherapper
Copy link
Contributor

#286 is the blocker, the pgp library we're using is deprecated but another part of the code depends on it still. At a glance it looks like https://github.com/ProtonMail/go-crypto support ed25519

@haydentherapper
Copy link
Contributor

See the update on the linked issue, we should be able to start using the newer library, but we still need to rely on the deprecated one for RPM packages.

Do you know if Debian packages or other packages you're working with use the openpgp v4 format?

@jas4711
Copy link
Author

jas4711 commented Apr 17, 2023

The relevant apt-signer OpenPGP keys I am uploading signed artifacts for are here:

https://gitlab.com/debdistutils/debdistcanary/-/tree/main/openpgp

I had to use gpg --export-options export-minimal on them to have canonical versions in rekor -- and interestingly, just having a Ed25519 signature on one of those keys caused rekor to reject an upload signed by that key, even though the key itself is RSA. This took some time to figure out, but makes somewhat sense given what I'm learning about rekor's PGP-support now.

It seems actually ALL of those keys are RSA v4 keys?! Uploading things signed by these keys works fine though.

jas@kaka:~/src/debdistcanary/openpgp$ for f in openpgp-*; do gpg --list-packets < $f; done | grep ':public key packet' -A 1|grep -v -e ':public key packet' -e^--
	version 4, algo 1, created 1555228135, expires 0
	version 4, algo 1, created 1472461287, expires 0
	version 4, algo 1, created 1336770936, expires 0
	version 4, algo 1, created 1555228608, expires 0
	version 4, algo 1, created 1555228608, expires 0
	version 4, algo 1, created 1483829507, expires 0
	version 4, algo 1, created 1549399120, expires 0
	version 4, algo 1, created 1417539282, expires 0
	version 4, algo 1, created 1537196506, expires 0
	version 4, algo 1, created 1588537086, expires 0
	version 4, algo 1, created 1613238862, expires 0
	version 4, algo 1, created 1610882316, expires 0
	version 4, algo 1, created 1610882224, expires 0
	version 4, algo 1, created 1610882224, expires 0
	version 4, algo 1, created 1537196506, expires 0
jas@kaka:~/src/debdistcanary/openpgp$ 

@haydentherapper
Copy link
Contributor

I think we might actually support V4 signatures currently, since I believe x/go/crypto does in addition to V3 signatures. For ed25519, another thing to note is that the hashedrekord Rekor type doesn't support it currently, we need to switch to ed25519ph (but this is a moot point since our pgp library doesnt support either!). You may find more rough edges too, we appreciate the filed issues!

@jas4711
Copy link
Author

jas4711 commented Apr 18, 2023

Yes I think you do support V4 signature. The output above was to prove that the keys were OpenPGP v4, however what is also relevant is the signatures, and they are also all V4 for the keys that I care about. See output below. It is critical to strip all non-selfsignatures for rekor to accept these keys though, otherwise rekor will reject them if they contain a ed25519 signature from someone else.

The three concerns I have with PGP-handling in rekor are:

I believe if rekor would minimize/canonicalize the public keys, the #1429 would be solved as well, since it is really just a side-effect of not performing any sanitation of the received public key.

jas@kaka:~/src/debdistcanary/openpgp$ for f in openpgp-*; do gpg --list-packets < $f; done | grep -e ':signature' -A 3|grep version
	version 4, created 1555228268, md5len 0, sigclass 0x1f
	version 4, created 1555228268, md5len 0, sigclass 0x1f
	version 4, created 1555228268, md5len 0, sigclass 0x1f
	version 4, created 1555228268, md5len 0, sigclass 0x1f
	version 4, created 1555228268, md5len 0, sigclass 0x1f
	version 4, created 1555228135, md5len 0, sigclass 0x13
	version 4, created 1555228135, md5len 0, sigclass 0x18
	version 4, created 1474494078, md5len 0, sigclass 0x13
	version 4, created 1472461287, md5len 0, sigclass 0x18
	version 4, created 1336770936, md5len 0, sigclass 0x13
	version 4, created 1555228613, md5len 0, sigclass 0x1f
	version 4, created 1555228613, md5len 0, sigclass 0x1f
	version 4, created 1555228613, md5len 0, sigclass 0x1f
	version 4, created 1555228613, md5len 0, sigclass 0x1f
	version 4, created 1555228613, md5len 0, sigclass 0x1f
	version 4, created 1555228608, md5len 0, sigclass 0x13
	version 4, created 1555228608, md5len 0, sigclass 0x18
	version 4, created 1555228613, md5len 0, sigclass 0x1f
	version 4, created 1555228613, md5len 0, sigclass 0x1f
	version 4, created 1555228613, md5len 0, sigclass 0x1f
	version 4, created 1555228613, md5len 0, sigclass 0x1f
	version 4, created 1555228613, md5len 0, sigclass 0x1f
	version 4, created 1555228608, md5len 0, sigclass 0x13
	version 4, created 1555228608, md5len 0, sigclass 0x18
	version 4, created 1483829507, md5len 0, sigclass 0x13
	version 4, created 1483829507, md5len 0, sigclass 0x18
	version 4, created 1549399120, md5len 0, sigclass 0x13
	version 4, created 1417539282, md5len 0, sigclass 0x13
	version 4, created 1417539282, md5len 0, sigclass 0x18
	version 4, created 1461639076, md5len 0, sigclass 0x18
	version 4, created 1537196506, md5len 0, sigclass 0x13
	version 4, created 1651234024, md5len 0, sigclass 0x13
	version 4, created 1651234205, md5len 0, sigclass 0x18
	version 4, created 1613238862, md5len 0, sigclass 0x13
	version 4, created 1610882319, md5len 0, sigclass 0x1f
	version 4, created 1610882319, md5len 0, sigclass 0x1f
	version 4, created 1610882319, md5len 0, sigclass 0x1f
	version 4, created 1610882319, md5len 0, sigclass 0x1f
	version 4, created 1610882319, md5len 0, sigclass 0x1f
	version 4, created 1610882316, md5len 0, sigclass 0x13
	version 4, created 1610882316, md5len 0, sigclass 0x18
	version 4, created 1610882229, md5len 0, sigclass 0x1f
	version 4, created 1610882229, md5len 0, sigclass 0x1f
	version 4, created 1610882229, md5len 0, sigclass 0x1f
	version 4, created 1610882229, md5len 0, sigclass 0x1f
	version 4, created 1610882229, md5len 0, sigclass 0x1f
	version 4, created 1610882224, md5len 0, sigclass 0x13
	version 4, created 1610882224, md5len 0, sigclass 0x18
	version 4, created 1610882229, md5len 0, sigclass 0x1f
	version 4, created 1610882229, md5len 0, sigclass 0x1f
	version 4, created 1610882229, md5len 0, sigclass 0x1f
	version 4, created 1610882229, md5len 0, sigclass 0x1f
	version 4, created 1610882229, md5len 0, sigclass 0x1f
	version 4, created 1610882224, md5len 0, sigclass 0x13
	version 4, created 1610882224, md5len 0, sigclass 0x18
	version 4, created 1537196506, md5len 0, sigclass 0x13
jas@kaka:~/src/debdistcanary/openpgp$ 

@jas4711
Copy link
Author

jas4711 commented Apr 18, 2023

I think #1170 is another example of not canonicalizing the public keys received when uploading. Generally, rather than taking the public keys or certificates and store them verbatim, I think rekor should parse them, minimize and strip anything unrelated, and export it again and then store that public keys in the ledger log. Right now, I think that if I make two uploads with the same signature, artifact and the public keys only differ by some whitespace in the ASCII-armor (or a comment), rekor will create two different entries for them. Searching for particular public keys breaks in this case. A corner-case is how to handle signatures signed by multiple public keys, but I think rekor should require that all public keys mentioned in the signature file should be uploaded.

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

No branches or pull requests

2 participants