Skip to content

Add support for ed25519 keys #1377

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

Merged
merged 12 commits into from
May 14, 2025
Merged

Add support for ed25519 keys #1377

merged 12 commits into from
May 14, 2025

Conversation

ramonpetgrave64
Copy link
Contributor

@ramonpetgrave64 ramonpetgrave64 commented May 9, 2025

Client support for Rekor V2: sigstore-python

Summary

Resolves #1376, #1378

Adds support for ed25519 keys. In the cryptography library, is not yet any support for ed25519ph operations.

Fixes the CI test for timestamp-authority to use the latest release, not the latest tag, since we could have new tags without associated release artifacts to download.

Release Note

  • Added support for ed25519 keys.
  • CI: Timestamp Authority tests use latest release, not latest tag, of
    sigstore/timestamp-authority

Documentation

None

Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
@ramonpetgrave64 ramonpetgrave64 marked this pull request as draft May 9, 2025 16:40
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
@ramonpetgrave64 ramonpetgrave64 marked this pull request as ready for review May 9, 2025 20:16
@ramonpetgrave64
Copy link
Contributor Author

@woodruffw @jku

@jku
Copy link
Member

jku commented May 10, 2025

LGTM. Getting this tested should be easy... I don't think there is any problem just editing the trusted_root/trustedroot.v1.json asset and making sure one of the keys is ed25519 (you could add a new log instance for example) -- the test only tests loading the key but it's better than nothing

Copy link
Member

@jku jku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm marking "request changes" for the test: let me know if it seems to not be straight forward

@ramonpetgrave64
Copy link
Contributor Author

ramonpetgrave64 commented May 12, 2025

@jku I tried to get sigstore-python to generate a ed25519 key for its signing certificate, changed all references from sha256 to sha512, but I still can't get it to work.

  File "/usr/local/google/home/rpetgrave/src/ssp/sigstore-python/sigstore/verify/verifier.py", line 374, in _verify_common_signing_cert
    raise VerificationError(f"invalid log entry: {exc}")
sigstore.errors.VerificationError: invalid log entry: checkpoint: invalid signature: keyring: invalid signature

I think the linters we use, mypy and ruff, are enough to be sure I didn't mistype or use incorrect method signatures.

@jku
Copy link
Member

jku commented May 13, 2025

@jku I tried to get sigstore-python to generate a ed25519 key for its signing certificate, changed all references from sha256 to sha512, but I still can't get it to work.

I don't think using sigstore-python for key generation is a good idea:

$ openssl genpkey -algorithm ed25519 -out tmp.key
$ openssl pkey -in tmp.key -pubout
-----BEGIN PUBLIC KEY-----
MCowBQYDK2VwAyEAma70u3EPxalYWDfRh8jat6UwIQ9Mza9gGO2uNzqT5AE=
-----END PUBLIC KEY-----

I would expect something like that to work as the key value in trustedroot.v1.json. I'm just looking for a test that verifies that we are feeding load_der_public_key() something it expects to get in the ed25519 case (obviously we don't know 100% what ends up in the trustedroot but at least we'd document what type of value we expect to see)

@ramonpetgrave64
Copy link
Contributor Author

@jku I added test case for the trusted_root that contains a the default tlog_key in rekor-tiles.

Signed-off-by: Ramon Petgrave <[email protected]>
@ramonpetgrave64 ramonpetgrave64 requested a review from jku May 13, 2025 18:42
Copy link
Member

@jku jku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cheers, lgtm.

Testing changes are not really needed in changelog, but I don't mind: trimming the changelog down is easy when doing the release prep.

@jku
Copy link
Member

jku commented May 14, 2025

/gcbrun

@jku jku merged commit 71da8a7 into sigstore:main May 14, 2025
23 checks passed
@jku jku mentioned this pull request May 14, 2025
@woodruffw
Copy link
Member

Clarifying: this is adding Ed25519 support at the trust layer, correct? Presumably we still need some API changes at the user sign/verify layers to support ephemeral Ed25519 keygen and signing.

@ramonpetgrave64
Copy link
Contributor Author

@woodruffw yes, we still hardcore using an EC key for signing.

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 this pull request may close these issues.

Add support for ed25519 keys
3 participants