address reviewer comments

Signed-off-by: Hector Fernandez <hector@chainguard.dev>

cmd/cosign/cli/options/verify.go

@@ -20,12 +20,20 @@ import (
 )
 
 type CommonVerifyOptions struct {
-	Offline bool // Force offline verification
+	Offline          bool // Force offline verification
+	TSACertChainPath string
+	TSAServerURL     string
 }
 
 func (o *CommonVerifyOptions) AddFlags(cmd *cobra.Command) {
 	cmd.Flags().BoolVar(&o.Offline, "offline", false,
 		"only allow offline verification")
+
+	cmd.Flags().StringVar(&o.TSAServerURL, "timestamp-server-url", "",
+		"url to a timestamp RFC3161 server, default none")
+
+	cmd.Flags().StringVar(&o.TSACertChainPath, "timestamp-cert-chain", "",
+		"path to certificate chain PEM file for the Timestamp Authority")
 }
 
 // VerifyOptions is the top level wrapper for the `verify` command.
@@ -44,8 +52,6 @@ type VerifyOptions struct {
 	Registry            RegistryOptions
 	SignatureDigest     SignatureDigestOptions
 
-	TSACertChainPath string
-	TSAServerURL     string
 	AnnotationOptions
 }
 
@@ -79,12 +85,6 @@ func (o *VerifyOptions) AddFlags(cmd *cobra.Command) {
 
 	cmd.Flags().BoolVar(&o.LocalImage, "local-image", false,
 		"whether the specified image is a path to an image saved locally via 'cosign save'")
-
-	cmd.Flags().StringVar(&o.TSAServerURL, "timestamp-server-url", "",
-		"url to a timestamp RFC3161 server, default none")
-
-	cmd.Flags().StringVar(&o.TSACertChainPath, "timestamp-cert-chain", "",
-		"path to certificate chain PEM file for the Timestamp Authority")
 }
 
 // VerifyAttestationOptions is the top level wrapper for the `verify attestation` command.
@@ -141,9 +141,6 @@ type VerifyBlobOptions struct {
 	Rekor               RekorOptions
 	Registry            RegistryOptions
 	CommonVerifyOptions CommonVerifyOptions
-
-	TSAServerURL     string
-	TSACertChainPath string
 }
 
 var _ Interface = (*VerifyBlobOptions)(nil)

cmd/cosign/cli/verify.go

@@ -118,8 +118,8 @@ against the transparency log.`,
 				SignatureRef:                 o.SignatureRef,
 				LocalImage:                   o.LocalImage,
 				Offline:                      o.CommonVerifyOptions.Offline,
-				TSAServerURL:                 o.TSAServerURL,
-				TSACertChainPath:             o.TSACertChainPath,
+				TSAServerURL:                 o.CommonVerifyOptions.TSAServerURL,
+				TSACertChainPath:             o.CommonVerifyOptions.TSACertChainPath,
 			}
 
 			if o.Registry.AllowInsecure {

cmd/cosign/cli/verify/verify.go

@@ -377,7 +377,7 @@ func PrintVerification(imgRef string, verified []oci.Signature, output string) {
 				if ss.Optional == nil {
 					ss.Optional = make(map[string]interface{})
 				}
-				ss.Optional["Bundle"] = tsaBundle
+				ss.Optional["TSABundle"] = tsaBundle
 			}
 
 			outputKeys = append(outputKeys, ss)

internal/pkg/cosign/tsa/signer.go

@@ -42,9 +42,9 @@ func GetTimestampedSignature(sigBytes []byte, tsaClient *tsaclient.TimestampAuth
 	if err != nil {
 		return nil, err
 	}
-	fmt.Println("Calling TSA authority ...")
+	fmt.Fprintln(os.Stderr, "Calling TSA authority ...")
 	params := ts.NewGetTimestampResponseParams()
-	params.SetTimeout(time.Second * 30)
+	params.SetTimeout(time.Second * 10)
 	params.Request = io.NopCloser(bytes.NewReader(requestBytes))
 
 	var respBytes bytes.Buffer
@@ -59,7 +59,7 @@ func GetTimestampedSignature(sigBytes []byte, tsaClient *tsaclient.TimestampAuth
 		return nil, err
 	}
 
-	fmt.Fprintln(os.Stderr, "Timestamp authority entry created with time:", ts.Time)
+	fmt.Fprintln(os.Stderr, "Timestamp fetched with time:", ts.Time)
 
 	return respBytes.Bytes(), nil
 }
@@ -107,7 +107,7 @@ func (rs *signerWrapper) Sign(ctx context.Context, payload io.Reader) (oci.Signa
 func createTimestampAuthorityRequest(artifactBytes []byte, hash crypto.Hash, policyStr string) ([]byte, error) {
 	reqOpts := &timestamp.RequestOptions{
 		Hash:         hash,
-		Certificates: true, // if the timestamp response should contain a certificate chain
+		Certificates: true, // if the timestamp response should contain the leaf certificate
 	}
 	// specify a pseudo-random nonce in the request
 	nonce, err := cryptoutils.GenerateSerialNumber()

pkg/cosign/verify.go

@@ -33,6 +33,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/digitorus/timestamp"
 	"github.com/sigstore/cosign/cmd/cosign/cli/fulcio/fulcioverifier/ctl"
 	cbundle "github.com/sigstore/cosign/pkg/cosign/bundle"
 	"github.com/sigstore/sigstore/pkg/tuf"
@@ -959,7 +960,10 @@ func VerifyTSABundle(ctx context.Context, sig oci.Signature, tsaClient *tsaclien
 		return false, fmt.Errorf("reading base64signature: %w", err)
 	}
 
-	fmt.Println("Verifying TSA Bundle")
+	cert, err := sig.Cert()
+	if err != nil {
+		return false, err
+	}
 
 	sigBytes, err := base64.StdEncoding.DecodeString(b64Sig)
 	if err != nil {
@@ -977,7 +981,19 @@ func VerifyTSABundle(ctx context.Context, sig oci.Signature, tsaClient *tsaclien
 
 	err = tsaverification.VerifyTimestampResponse(bundle.SignedRFC3161Timestamp, bytes.NewReader(sigBytes), certPool)
 	if err != nil {
-		return false, fmt.Errorf("unable verifyTSRWithPEM: %w", err)
+		return false, fmt.Errorf("unable to verify TimestampResponse: %w", err)
+	}
+
+	if cert != nil {
+		ts, err := timestamp.ParseResponse(bundle.SignedRFC3161Timestamp)
+		if err != nil {
+			return false, fmt.Errorf("error parsing response into timestamp: %w", err)
+		}
+		// Verify the cert against the integrated time.
+		// Note that if the caller requires the certificate to be present, it has to ensure that itself.
+		if err := CheckExpiry(cert, ts.Time); err != nil {
+			return false, fmt.Errorf("checking expiry on cert: %w", err)
+		}
 	}
 
 	return true, nil