-
Notifications
You must be signed in to change notification settings - Fork 164
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
Proposal to deprecate: Removing support for certificate chain on entry upload #1944
Comments
No concerns with removing that chunk of code, beyond it potentially breaking the verification flow for entries that are already in the log. |
Can confirm that |
sigstore-java doesn't upload the chain. Just the leaf. FYI I recently uploaded a full chain to check that it could be done: https://search.sigstore.dev/?uuid=24296fb24b8ad77a77ea71e1bcc032f1e6b2bd0c5cbb28a244aaf67f679c9c02ab1c51599de425d7 so ignore that one? It looks like we've mostly always been uploading leafs to rekor, but until aug 2023 were storing full certs in the bundle? |
|
Thanks all. We'll implement this in such a way we don't break verification for existing entries, so likely just blocking chains or canonicalizing to just a leaf on upload going forward. |
I would like to suggest a valid use case for storing the certificate chain: identity monitoring that match only trusted certificate. TLDR: the rekor binary transparency log should have all information to verify the certificate, otherwise it would need a verifiable way to retrieve the most recent, and all the missing info to verify and monitor the certificates and signatures. Imagine a software secured by certificate issued by fulcio. The auto update system would accept the update if 1. the bundled certificate chain, and the signature is valid 2. the signature and the leaf certificate is included in the binary transparency log. In an attack, the developer login the signer primary Github account at employer/university's MITMed corporate network and leaked the cookie that can be used to request misissued certificate. (P.S.: a role github is not possible as these account are marked as spam, an attack surface created by Github) Currently, rekor-monitor does not support excluding untrusted certificate from log, and open users to spamming attack. The attacker could spam the monitor with fake certificate with matching email and other extensions but issued by a non-existent intermediary certificate. With this attack, a signature produced by a misissused certificate would be buried within spam and could not be detected at a reasonable speed. With the stolen cookie and spamming attack to neutralize the rekor-monitor, the attacker would be able to create trusted fake software updates. In order to prevent this kind of attack, there is following possible routes without including the certificate in the rekor binary transparency log:
|
Hey, thanks for the comment! A community member noticed this issue last week actually, and I've filed sigstore/rekor-monitor#378 to track this. I'm planning to implement (1), that a monitor can be configured with a set of trusted roots and chain building intermediates, and the monitor will only alert on certificates that can be verified using that chain.
This is what I would recommend. For any users of the public instance or of private instances using TheUpdateFramework to manage their roots of trust, we can easily fetch the set of trusted CA certificates. Otherwise, a user can provide the CA certificates.
This is most certainly an option, Rekor supports entries where the signature verifier is only a key. Note that Sigstore was designed to remove the need to manage keys by issuing identity-based certificates that certify ephemeral signing keys. You can always choose to self-manage keys, it just comes with the trade-off of managing the lifecycle and security of key material.
As a public log, we've chosen to allow any entry currently. We have considered adding a policy layer that would allow deployments to choose which signatures can be uploaded based on a set of configured roots (this is what certificate transparency logs do, for example) |
Thanks for the reply!
Yes! For monitors, TheUpdateFramework itself is not misbehaving evident on its own, so scanning the certificate transparency is always necessary to get a all the intermediary certificates with a delay of 1 day, requiring the rekor-monitor to defer scan for 1 day. If we are trusting TUF is not compromised, and don't scan the certificate transparency log, then it would take 7 days for a freezing attack to be spotted, requiring the rekor-monitor to defer scan for 7 days. |
That’s a good point. We could always fetch the latest TUF metadata to mitigate this, or of course a user can provide the chain directly. |
Description
Support for uploading a certificate chain, not just a leaf certificate, was added awhile ago (#747). I don't recall if there was a specific motivation at the time, but there's a few possible reasons:
Whether or not to store the chain in Rekor came up in a recent thread on Slack and was discussed further in sig-clients. The issue is that a client cannot know if the entry has either a leaf or the full chain when verifying, unless explicitly told by the user. This is bad UX as a user is not expected to know how their entry they're verifying was recorded in Rekor. Either the client would need a flag to control this behavior or search for both entries. This also allows for entry malleability since the same signing event can be uploaded twice, once with a chain and once with a leaf.
Proposal: Remove support for uploading a certificate chain. Specifically, this chunk of code. We'll want to deprecate this over a few months, though I've some TODOs to determine how much this is actually used.
An open question that was discussed in sig-clients is whether to fail when a user uploads a full chain or canonicalize the chain to only a leaf. Personally I'm leaning towards canonicalization, but the clients preferred to fail when uploading a chain.
Clients and maintainers for any comments: @bobcallaway @woodruffw @loosebazooka @bdehamer @kommendorkapten @codysoyland @di
TODOs:
The text was updated successfully, but these errors were encountered: