Skip to content

[DRAFT] Rekor v1 and v2 support via signing-config #1387

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

Draft
wants to merge 103 commits into
base: main
Choose a base branch
from

Conversation

ramonpetgrave64
Copy link
Contributor

@ramonpetgrave64 ramonpetgrave64 commented May 16, 2025

Client support for Rekor V2: sigstore-python #289

Summary

Adds Rekor V2 support, while being compatible with Rekor v1, swicthable with the signing-config, when signing and verify an artifact.

Lots of files in this PR:

Testing

Unit tests pass locally with python 3.12.9. CI tests fails on python 3.9. And linters are not yet expected to pass.

Signing and verifying both work.

Invoke with

sigstore --trust-config ./trust_config.json sign README.md --overwrite -v

sigstore --trust-config ./trust_config.json verify identity --bundle README.md.sigstore.json --cert-oidc-issuer https://accounts.google.com --cert-identity <your cert identity> README.md --verbose
sigstore --trust-config ./trust_config_v2.json sign README.md --overwrite -v

sigstore --trust-config ./trust_config_v2.json verify identity --bundle README.md.sigstore.json --cert-oidc-issuer https://accounts.google.com --cert-identity <your cert identity> README.md --verbose

TODO

Lots of items, so the last few items regarding testing will probably come in a separate PR.

Release Note

Documentation

ramonpetgrave64 and others added 30 commits April 25, 2025 18:26
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
This reverts commit 79a6d31.

Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
This reverts commit e4470a9.

Signed-off-by: Ramon Petgrave <[email protected]>
sigstore/sign.py Outdated
@@ -293,6 +310,43 @@ def sign_artifact(
),
)

# Create the proposed hashedrekord entry
if self._signing_ctx._rekor.major_api_version == REKOR_V1_API_MAJOR_VERSION:
Copy link
Member

@jku jku May 16, 2025

Choose a reason for hiding this comment

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

What do you think about this: RekorClient provides a build_proposed_entry() that takes artifatc_signature, b64_cert and hashed_input (and dsse/hashedrekord selection) as argument -- then this code (and the dsse code) could just call self._signing_context._rekor.build_proposed_entry()

This way we could keep the differences between the two rekor implementations more contained in the RekorClient(s).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good idea, especially since this patch is building the request body.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed for just hashedrekord. dsse to come later.

@jku jku mentioned this pull request May 16, 2025
@jku
Copy link
Member

jku commented May 16, 2025

I mentioned this elsewhere but for clarity: I would prefer the Rekorv2 changes to not touch the v1 RekorClient code if possible and instead be a separate implementation.

  • the rekorclient(s) could also take care of building the correct proposed entry (mentioned this in a code comment)
  • currently it looks like the rekor v2 client supports all the methods in RekorClient... but I don't think it does and I don't think it needs to

@ramonpetgrave64
Copy link
Contributor Author

ramonpetgrave64 commented May 16, 2025

@jku Yes, like we discussed offline, it perhaps would be nicer if the V2 changes would not edit the current RekorClient class. I'll have to see how viable the auto-generated v2 client RekorBase or RekorStub is.

If not, I also mentioned that subclassing could make more sense. Abstract the common methods of V1 and V2 into another base class. If that, I don't think removing V1 support in the future after the estimated 18 months would be difficult.

Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
@ramonpetgrave64 ramonpetgrave64 mentioned this pull request May 20, 2025
6 tasks
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
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.

2 participants