[Bug] Incorrect Verification: Verification succeeds using an expired certificate with offline verification

[asraa]

Description

This is a bug about Cosign verification flow. When experimental mode is turned OFF (in "offline verification": we still use bundles), Cosign validates a signature successfully despite the fact that the certificate is expired. This occurs when TLOG upload is bypassed, or the offline bundle is removed. This is because without experimental mode, it has no access to make a network call.

So, for example, I can bypass TLOG upload, and allow successful verification. I can re-use an old, expired cert, and pass verification, and not TLOG upload. This is ESPECIALLY the case if I have clients who are only relying on offline bundles (so they turn off COSIGN_EXPERIMENTAL). Therefore, I can trick clients who were expecting to use cosign with offline verification.

In other standards, you will fail:

• https://datatracker.ietf.org/doc/html/rfc2632: Some of the many places where signature and certificate checking might fail include:[...] - the certificate is expired
• In https://www.rfc-editor.org/rfc/rfc5280#section-6.1.3, processing certificates requires checking that the certificate is valid.
• https://www.rfc-editor.org/rfc/rfc5750#section-5: Describes using a reliable time for a certificate validity check

Version

At head, and all versions.

Proposed Fix
Exit code 1 in this case. Signatures should only be valid if the certificate is expired OR a timestamp is fetched/provided proving that the signature was generated during the validity window.

The output states

The following checks were performed on each of these signatures:
- The cosign claims were validated
- The signatures were verified against the specified public key

Are the signatures really verified against the certificate? The certificate is expired. If a certificate is REVOKED in traditional systems, would you still pass signature validation?

Long term:
To me, this issue points to a flaw that cosign's validation flow allows signature to be passing UNLESS something like a timestamp has access to be checked, rather than a better, way more secure flow: a signature should be failing until the requirements are satisfied.

cc @znewman01 @codysoyland @haydentherapper and others who may be interested in security issues.

[dlorenc]

Are the signatures really verified against the certificate? The certificate is expired. If a certificate is REVOKED in traditional systems, would you still pass signature validation?

I think this is the subtle bit - they're verified against the certificate, but the certificate itself isn't necessarily verified. The fix is tricky because it generally means that we'd need to add warnings about the EXPERIMENTAL stuff into the non experimental flow.

Some of this gets easier to reason about after we drop the separation between experimental and not. WDYT about instead just dropping any certificate based verifications from cosign without EXPERIMENTAL? It hurts the BYO-PKI stuff a bit, but tbh without either one of RFC3161 or Rekor we shouldn't really be doing certificates anyway.

[asraa]

EXPERIMENTAL stuff into the non experimental flow.

If EXPERIMENTAL means network access, note that we do verify offline bundles in this flow. If the offline bundle is unavailable (or not present due to bypass), then it should fail.

but the certificate itself isn't necessarily verified.

If a certificate isn't verified I posit that the signature isn't verified: this is akin to verifying the signature successfully, but then passing validation even when it doesn't chain to the root CA.

Some of this gets easier to reason about after we drop the separation between experimental and not.

I agree, it will -- part of the reason I bring up this issue is that we could redesign this a lot better when the flag goes away: a signature must pass X, Y, Z in order to pass validation, rather than X, Y, and Z if we have network access as it is currently.

WDYT about instead just dropping any certificate based verifications from cosign without EXPERIMENTAL?

That may be a good way to fix it short term before the flag goes away. Something like exit code 1, and stderr output "certificate validation unsupported without experimental mode".

[dlorenc]

That may be a good way to fix it short term before the flag goes away. Something like exit code 1, and stderr output "certificate validation unsupported without experimental mode".

+1 to this, I think this is the best way forward then we can clearly document what will change as experimental goes away.

[asraa]

Note: what I find really interesting about this is that the presence of more info turns a signature success into a signature failure. I think that's why it bothers me so much.

[haydentherapper]

I don’t think we should entirely disable certificate validation. This risks impacting users, and it’s not a security risk to validate an unexpired certificate. We just published a post documenting how Sigstore is more than just its infrastructure too, so I’d prefer to not break those flows.

There are three valid certificate cases:

• an unexpired certificate
• An expired certificate with a valid 3161 time stamp
• An expired certificate with a valid Rekor bundle

We need to modify the verification function to support enforcing expiration. The second of these points, we can implement later, once we have TSA support.

[asraa]

There are three valid certificate cases:

• An expired certificate with online lookup to Rekor

one more as well.

I'll draft up a fix that tries to re-use as much validation logic as possible

[dlorenc]

Out of these 4, I think only these two should work without experimental right now (basically just drop support for the bundle stuff without experimental:

• an unexpired certificate
• An expired certificate with a valid 3161 time stamp

If you think you can get the expired cert stuff working without experimental that's fine, but I'm also fine moving that into experimental if that makes the code easier. We don't support timestamps at all yet, so that's really the only case for certs that we'd have outside experimental.

[haydentherapper]

Agreed, I think it's fair to require the EXPERIMENTAL flag for bundles (effectively meaning "doing something with rekor, offline or online").

I think we should fix "unexpired certificates" for non-experimental, and punt on expired cert + 3161 until we've added a TSA. So in the case you provide an expired certificate without either a rekor bundle, then we'll fail. If you provide an unexpired certificate without a rekor bundle for non-experimental, we'll succeed.