From abc06418359e7dfcb875c6c2dd25cc65d3fa7b18 Mon Sep 17 00:00:00 2001 From: Hector Fernandez Date: Tue, 15 Nov 2022 20:30:17 +0100 Subject: [PATCH] address reviewer comments Signed-off-by: Hector Fernandez --- cmd/cosign/cli/options/verify.go | 21 +++++++++------------ cmd/cosign/cli/verify.go | 4 ++-- cmd/cosign/cli/verify/verify.go | 2 +- internal/pkg/cosign/tsa/signer.go | 8 ++++---- pkg/cosign/verify.go | 20 ++++++++++++++++++-- 5 files changed, 34 insertions(+), 21 deletions(-) diff --git a/cmd/cosign/cli/options/verify.go b/cmd/cosign/cli/options/verify.go index 2d59566f0f6b..614cb8cffe05 100644 --- a/cmd/cosign/cli/options/verify.go +++ b/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) diff --git a/cmd/cosign/cli/verify.go b/cmd/cosign/cli/verify.go index d0dbb36428a4..c96f14e8bad5 100644 --- a/cmd/cosign/cli/verify.go +++ b/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 { diff --git a/cmd/cosign/cli/verify/verify.go b/cmd/cosign/cli/verify/verify.go index f84b3bbd6997..c8ad562266c4 100644 --- a/cmd/cosign/cli/verify/verify.go +++ b/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) diff --git a/internal/pkg/cosign/tsa/signer.go b/internal/pkg/cosign/tsa/signer.go index 7bc931e068e6..7d4f3a2e64ad 100644 --- a/internal/pkg/cosign/tsa/signer.go +++ b/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 := ×tamp.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() diff --git a/pkg/cosign/verify.go b/pkg/cosign/verify.go index 2a52eab86a26..f44a261a0831 100644 --- a/pkg/cosign/verify.go +++ b/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