Fail timestamp verification if no root is provided

[haydentherapper]

The bug was that we would conditionally check a timestamp if a root was provided. If no root was provided even if a timestamp was provided, then signature verification would succeed.

The good news is this will not show a successful signature if the transparency log does not contain the entry too, for timestamp verification.

Summary

Release Note

Documentation

[codecov]

Codecov Report

Merging #3224 (65843b4) into main (3a6f2ab) will increase coverage by 0.03%.
Report is 5 commits behind head on main.
The diff coverage is 63.63%.

@@            Coverage Diff             @@
##             main    #3224      +/-   ##
==========================================
+ Coverage   30.33%   30.37%   +0.03%     
==========================================
  Files         155      155              
  Lines        9827     9828       +1     
==========================================
+ Hits         2981     2985       +4     
+ Misses       6398     6397       -1     
+ Partials      448      446       -2     

Files Changed	Coverage Δ
pkg/cosign/verify.go	34.60% <63.63%> (+0.38%)	⬆️

📢 Have feedback on the report? Share it here.

[zhaoyonghe]

Hi @haydentherapper, just to clarify, is this fix handling the scenario when we verify the keyless signature within the validity of the signing cert? Because if we verify the keyless signature after the signing cert expired and we do not provide the TSA root, the verification will fail like this:

Error: no matching signatures: expected a signed timestamp to verify an expired certificate
main.go:69: error during command execution: no matching signatures: expected a signed timestamp to verify an expired certificate

Thank you!

[haydentherapper]

@zhaoyonghe, no, that's working as intended. If no signed entry timestamp (from the transparency log) or timestamp is provided, certificate verification should fail.

This is fixing a bug reported on https://sigstore.slack.com/archives/C01PZKDL4DP/p1693930315937879, where if no TSA root is provided, timestamp verification is skipped.

[zhaoyonghe]

@zhaoyonghe, no, that's working as intended. If no signed entry timestamp (from the transparency log) or timestamp is provided, certificate verification should fail.

This is fixing a bug reported on https://sigstore.slack.com/archives/C01PZKDL4DP/p1693930315937879, where if no TSA root is provided, timestamp verification is skipped.

@haydentherapper Thank you for the reply!

I asked that question because I could not reproduce the same bug based on the PR title and description. After studying the original bug report, I finally realized that this bug is only triggered when users use the public key (not the cert, and that is what I used when I tried to reproduce it) in the verification. The bug reporter did not use a cert in verification, so cosign will not execute these code and raise the error "expected a signed timestamp to verify an expired certificate".

[mtrmac]

This is fixing a bug reported on https://sigstore.slack.com/archives/C01PZKDL4DP/p1693930315937879, where if no TSA root is provided, timestamp verification is skipped.

Why is that a problem?

The way I think about security, the client decides what the policy is, and what gets verified:

• If the client wants a timestamp, the client decides what the root of thrust is, and potentially whether to require a timestamp.
• If the client doesn't provide a trust root for a timestamp, the client doesn’t care and the timestamp, even if present, is ignored.

With the changes in this PR, the image decides whether the timestamp is validated. Why is it up to the image to do that?

The practical impact is, AFACS, that adding a timestamp the client is not configured to trust triggers a “no TSA root certificate(s) provided to verify timestamp” verification failure. To me, it is deeply surprising that clients can accept an image with no timestamp (i.e. they clearly don’t need the timestamp to exist) but they will refuse the same image with a timestamp added (even if the timestamp is completely correct).

[haydentherapper]

If the client doesn't provide a trust root for a timestamp, the client doesn’t care and the timestamp, even if present, is ignored.

I would guess that most of the time, if a user provides something that needs to be verified, they expect the client to verify it. If the user doesn't want to verify time, then a user should not provide signed time. We do the same thing with other signed material - if a certificate is provided without a trust root, we err out.

[mtrmac]

In the context of signature verification, I think the natural threat model implies that the image is not coming from the trusted user: it is coming from a potentially-malicious registry. So a paranoid signature verifier should, I think, minimize the number of decisions, or code patch choices, that potentially-malicious registry can affect.