From 5dd24259cc276dd914aa1dec5d368504c95273d6 Mon Sep 17 00:00:00 2001
From: Hayden Blauzvern <hblauzvern@google.com>
Date: Tue, 6 Dec 2022 21:50:46 +0000
Subject: [PATCH] Address comments

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
---
 cmd/cosign/cli/options/verify.go |  2 +-
 cmd/cosign/cli/sign/sign_blob.go | 27 ++++++++++++++++-----------
 specs/SIGNATURE_SPEC.md          |  2 +-
 3 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/cmd/cosign/cli/options/verify.go b/cmd/cosign/cli/options/verify.go
index 2b1413b43b77..5e4ebb11a71d 100644
--- a/cmd/cosign/cli/options/verify.go
+++ b/cmd/cosign/cli/options/verify.go
@@ -165,7 +165,7 @@ func (o *VerifyBlobOptions) AddFlags(cmd *cobra.Command) {
 		"path to bundle FILE")
 
 	cmd.Flags().StringVar(&o.RFC3161TimestampPath, "rfc3161-timestamp", "",
-		"path to rfc3161 timestamp FILE")
+		"path to RFC3161 timestamp FILE")
 }
 
 // VerifyDockerfileOptions is the top level wrapper for the `dockerfile verify` command.
diff --git a/cmd/cosign/cli/sign/sign_blob.go b/cmd/cosign/cli/sign/sign_blob.go
index e5e6460cc4ee..5eb618a53ace 100644
--- a/cmd/cosign/cli/sign/sign_blob.go
+++ b/cmd/cosign/cli/sign/sign_blob.go
@@ -74,6 +74,10 @@ func SignBlobCmd(ro *options.RootOptions, ko options.KeyOpts, payloadPath string
 
 	var rfc3161Timestamp *cbundle.RFC3161Timestamp
 	if ko.TSAServerURL != "" {
+		if ko.RFC3161TimestampPath == "" {
+			return nil, fmt.Errorf("timestamp output path must be set")
+		}
+
 		clientTSA, err := tsaclient.GetTimestampClient(ko.TSAServerURL)
 		if err != nil {
 			return nil, fmt.Errorf("failed to create TSA client: %w", err)
@@ -86,6 +90,18 @@ func SignBlobCmd(ro *options.RootOptions, ko options.KeyOpts, payloadPath string
 
 		rfc3161Timestamp = cbundle.TimestampToRFC3161Timestamp(respBytes)
 		// TODO: Consider uploading RFC3161 TS to Rekor
+
+		if rfc3161Timestamp == nil {
+			return nil, fmt.Errorf("rfc3161 timestamp is nil")
+		}
+		ts, err := json.Marshal(rfc3161Timestamp)
+		if err != nil {
+			return nil, err
+		}
+		if err := os.WriteFile(ko.RFC3161TimestampPath, ts, 0600); err != nil {
+			return nil, fmt.Errorf("create rfc3161 timestamp file: %w", err)
+		}
+		fmt.Printf("RFC3161 timestamp bundle written to file %s\n", ko.RFC3161TimestampPath)
 	}
 	if ShouldUploadToTlog(ctx, ko, nil, tlogUpload) {
 		rekorBytes, err = sv.Bytes(ctx)
@@ -104,17 +120,6 @@ func SignBlobCmd(ro *options.RootOptions, ko options.KeyOpts, payloadPath string
 		signedPayload.Bundle = cbundle.EntryToBundle(entry)
 	}
 
-	if ko.RFC3161TimestampPath != "" && rfc3161Timestamp != nil {
-		ts, err := json.Marshal(rfc3161Timestamp)
-		if err != nil {
-			return nil, err
-		}
-		if err := os.WriteFile(ko.RFC3161TimestampPath, ts, 0600); err != nil {
-			return nil, fmt.Errorf("create rfc3161 timestamp file: %w", err)
-		}
-		fmt.Printf("RFC3161 timestamp bundle written to file %s\n", ko.RFC3161TimestampPath)
-	}
-
 	// if bundle is specified, just do that and ignore the rest
 	if ko.BundlePath != "" {
 		signedPayload.Base64Signature = base64.StdEncoding.EncodeToString(sig)
diff --git a/specs/SIGNATURE_SPEC.md b/specs/SIGNATURE_SPEC.md
index c0590674f497..20cf54605555 100644
--- a/specs/SIGNATURE_SPEC.md
+++ b/specs/SIGNATURE_SPEC.md
@@ -131,7 +131,7 @@ For instructions on using the `bundle` for verification, see [USAGE.md](../USAGE
 
 * `rfc3161timestamp` string
 
-  This OPTIONAL property contains a JSON formatted `RFC3161Timestamp` bundle containing the timestamp response from a
+  This OPTIONAL property contains a JSON formatted `RFC3161Timestamp` containing the timestamp response from a
   timestamp authority.
 
 ## Storage