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

cosign doesn't correctly respect individual TSA certificate chains #4098

Open
bkabrda opened this issue Mar 4, 2025 · 3 comments
Open

cosign doesn't correctly respect individual TSA certificate chains #4098

bkabrda opened this issue Mar 4, 2025 · 3 comments
Labels
bug Something isn't working

Comments

@bkabrda
Copy link
Contributor

bkabrda commented Mar 4, 2025

Description

I have a private Sigstore instance, in which I attempted to rotate the TSA certificate chain. When trying to verify any blob/image after the rotation, I get the following error:

$ cosign --verbose verify-blob [email protected] --bundle signed.bundle to-sign --rfc3161-timestamp=timestamp.txt --use-signed-timestamps
Error: unable to load TSA certificates: TSA certificate chain must contain exactly one leaf certificate
main.go:74: error during command execution: unable to load TSA certificates: TSA certificate chain must contain exactly one leaf certificate

This is because the GetTSACerts function in pkg/cosign/tsa.go only expects to parse out a single certificate chain, because of the assumption that these individual chains are provided as individual TUF targets.

However in order to be able to properly rotate the TSA certificate chain, one needs to provide the whole TSA certificate chain as a single TUF target (which is possible and I do it). However, when I attempt to introduce a new TSA chain as a new TUF target and keeping the old one as "Expired", I get the above error when verifying any artifact.

I think the GetTSACerts function should behave more like GetRekorPubs, which returns multiple keys and all of them are considered valid for verification.

I think this will actually work fine in the future with #3844 because of how the timestamp verification function from sigstore-go works. I can submit a PR to fix this if you think it's still worth it before #3844 gets merged - I think it's worth because of private Sigstore deployments that might be slower to upgrade.

Version

2.4.1, but applies to latest main branch as well AFAICS

@bkabrda bkabrda added the bug Something isn't working label Mar 4, 2025
@haydentherapper
Copy link
Contributor

A fix would be welcome, though if it’s a significant change, feel free to not and we will fix it indirectly with Sigstore-go.

@bkabrda
Copy link
Contributor Author

bkabrda commented Mar 5, 2025

@haydentherapper I see two ways to fix this basically:

  • Either changing GetTSACerts to return multiple cert chains (easier but would break the API because it's a public function, so probably not great)
  • Or introducing a new function, e.g. GetAllTSACertChains that would return all valid found TSA certificate chains and using that throughout the codebase; AIUI all of this code will get removed eventually after we fully move to trusted_root.json, so adding a small new function temporarily shouldn't be much of a problem.

I think going for the second option makes more sense (a little more code required, but not breaking API), WDYT?

@haydentherapper
Copy link
Contributor

We don't offer any gurantees around API breakages for Cosign so making a breaking change would be fine. I'm OK with either approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants