update comments

Signed-off-by: Asra Ali <asraa@google.com>

cmd/cosign/cli/verify/verify_blob.go

@@ -497,7 +497,11 @@ func verifyRekorBundle(ctx context.Context, bundle *bundle.RekorBundle,
 		return nil, fmt.Errorf("retrieving rekor public key: %w", err)
 	}
 
-	pubKey, ok := publicKeys[bundle.Payload.LogID]
+	if publicKeys == nil || publicKeys.Keys == nil {
+		return nil, errors.New("rekor log public key not found for payload")
+	}
+
+	pubKey, ok := publicKeys.Keys[bundle.Payload.LogID]
 	if !ok {
 		return nil, errors.New("rekor log public key not found for payload")
 	}

cmd/cosign/cli/verify/verify_blob_test.go

@@ -229,10 +229,12 @@ func TestVerifyBlob(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	rekorPubKeys := map[string]cosign.RekorPubKey{
-		logID: {
-			PubKey: &rekorPriv.PublicKey,
-			Status: tuf.Active,
+	rekorPubKeys := cosign.TrustedRekorPubKeys{
+		Keys: map[string]cosign.RekorPubKey{
+			logID: {
+				PubKey: &rekorPriv.PublicKey,
+				Status: tuf.Active,
+			},
 		},
 	}
 
@@ -556,7 +558,7 @@ func TestVerifyBlob(t *testing.T) {
 			co := &cosign.CheckOpts{
 				SigVerifier:  tt.sigVerifier,
 				RootCerts:    rootPool,
-				RekorPubKeys: rekorPubKeys,
+				RekorPubKeys: &rekorPubKeys,
 			}
 			// if expermental is enabled, add RekorClient to co.
 			if tt.experimental {

pkg/cosign/tlog.go

@@ -57,6 +57,13 @@ type RekorPubKey struct {
 	Status tuf.StatusKind
 }
 
+// This is a map of RekorPubKeys indexed by log ID that's used in verification
+// for the trusted set of RekorPubKeys.
+type TrustedRekorPubKeys struct {
+	// A map of keys indexed by log ID
+	Keys map[string]RekorPubKey
+}
+
 const treeIDHexStringLen = 16
 const uuidHexStringLen = 64
 const entryIDHexStringLen = treeIDHexStringLen + uuidHexStringLen
@@ -91,24 +98,19 @@ func intotoEntry(ctx context.Context, signature, pubKey []byte) (models.Proposed
 // There are two Env variable that can be used to override this behaviour:
 // SIGSTORE_REKOR_PUBLIC_KEY - If specified, location of the file that contains
 // the Rekor Public Key on local filesystem
-func GetRekorPubs(ctx context.Context) (map[string]RekorPubKey, error) {
-	publicKeys := make(map[string]RekorPubKey)
+func GetRekorPubs(ctx context.Context) (*TrustedRekorPubKeys, error) {
+	publicKeys := NewTrustedRekorPubKeys()
 	altRekorPub := env.Getenv(env.VariableSigstoreRekorPublicKey)
 
 	if altRekorPub != "" {
 		raw, err := os.ReadFile(altRekorPub)
 		if err != nil {
 			return nil, fmt.Errorf("error reading alternate Rekor public key file: %w", err)
 		}
-		extra, err := PemToECDSAKey(raw)
+		publicKeys.AddRekorPubKey(raw, tuf.Active)
 		if err != nil {
-			return nil, fmt.Errorf("error converting PEM to ECDSAKey: %w", err)
+			return nil, fmt.Errorf("AddRekorPubKey: %w", err)
 		}
-		keyID, err := getLogID(extra)
-		if err != nil {
-			return nil, fmt.Errorf("error generating log ID: %w", err)
-		}
-		publicKeys[keyID] = RekorPubKey{PubKey: extra, Status: tuf.Active}
 	} else {
 		tufClient, err := tuf.NewFromEnv(ctx)
 		if err != nil {
@@ -119,44 +121,34 @@ func GetRekorPubs(ctx context.Context) (map[string]RekorPubKey, error) {
 			return nil, err
 		}
 		for _, t := range targets {
-			rekorPubKey, err := PemToECDSAKey(t.Target)
-			if err != nil {
-				return nil, fmt.Errorf("pem to ecdsa: %w", err)
-			}
-			keyID, err := getLogID(rekorPubKey)
+			publicKeys.AddRekorPubKey(t.Target, t.Status)
 			if err != nil {
-				return nil, fmt.Errorf("error generating log ID: %w", err)
+				return nil, fmt.Errorf("AddRekorPubKey: %w", err)
 			}
-			publicKeys[keyID] = RekorPubKey{PubKey: rekorPubKey, Status: t.Status}
 		}
 	}
 
-	if len(publicKeys) == 0 {
+	if len(publicKeys.Keys) == 0 {
 		return nil, errors.New("none of the Rekor public keys have been found")
 	}
 
-	return publicKeys, nil
+	return &publicKeys, nil
 }
 
 // rekorPubsFromClient returns a RekorPubKey keyed by the log ID from the Rekor client.
-// NOTE: This should **not** be used in the verification path, but may be used in the
+// NOTE: This **must not** be used in the verification path, but may be used in the
 // sign path to validate return responses are consistent from Rekor.
-func rekorPubsFromClient(rekorClient *client.Rekor) (map[string]RekorPubKey, error) {
-	publicKeys := make(map[string]RekorPubKey)
+func rekorPubsFromClient(rekorClient *client.Rekor) (*TrustedRekorPubKeys, error) {
+	publicKeys := NewTrustedRekorPubKeys()
 	pubOK, err := rekorClient.Pubkey.GetPublicKey(nil)
 	if err != nil {
 		return nil, fmt.Errorf("unable to fetch rekor public key from rekor: %w", err)
 	}
-	pubFromAPI, err := PemToECDSAKey([]byte(pubOK.Payload))
-	if err != nil {
-		return nil, fmt.Errorf("error converting rekor PEM public key from rekor to ECDSAKey: %w", err)
-	}
-	keyID, err := getLogID(pubFromAPI)
+	publicKeys.AddRekorPubKey([]byte(pubOK.Payload), tuf.Active)
 	if err != nil {
-		return nil, fmt.Errorf("error generating log ID: %w", err)
+		return nil, fmt.Errorf("constructRekorPubKey: %w", err)
 	}
-	publicKeys[keyID] = RekorPubKey{PubKey: pubFromAPI, Status: tuf.Active}
-	return publicKeys, nil
+	return &publicKeys, nil
 }
 
 // TLogUpload will upload the signature, public key and payload to the transparency log.
@@ -427,9 +419,9 @@ func FindTLogEntriesByPayload(ctx context.Context, rekorClient *client.Rekor, pa
 	return searchIndex.GetPayload(), nil
 }
 
-// VerityTLogEntry verifies a TLog entry against the trusted map of rekorPubKeys.
-// The argument *client.Rekor is unused and may be nil.
-func VerifyTLogEntry(e *models.LogEntryAnon, rekorPubKeys map[string]RekorPubKey) error {
+// VerityTLogEntry verifies a TLog entry against a map of trusted rekorPubKeys indexed
+// by log id.
+func VerifyTLogEntry(e *models.LogEntryAnon, rekorPubKeys *TrustedRekorPubKeys) error {
 	if e.Verification == nil || e.Verification.InclusionProof == nil {
 		return errors.New("inclusion proof not provided")
 	}
@@ -461,7 +453,7 @@ func VerifyTLogEntry(e *models.LogEntryAnon, rekorPubKeys map[string]RekorPubKey
 		LogID:          *e.LogID,
 	}
 
-	pubKey, ok := rekorPubKeys[payload.LogID]
+	pubKey, ok := rekorPubKeys.Keys[payload.LogID]
 	if !ok {
 		return errors.New("rekor log public key not found for payload")
 	}
@@ -474,3 +466,22 @@ func VerifyTLogEntry(e *models.LogEntryAnon, rekorPubKeys map[string]RekorPubKey
 	}
 	return nil
 }
+
+func NewTrustedRekorPubKeys() TrustedRekorPubKeys {
+	return TrustedRekorPubKeys{Keys: make(map[string]RekorPubKey, 0)}
+}
+
+// constructRekorPubkey returns a log ID and RekorPubKey from a given
+// byte-array representing the PEM-encoded Rekor key and a status.
+func (t *TrustedRekorPubKeys) AddRekorPubKey(pemBytes []byte, status tuf.StatusKind) error {
+	pubKey, err := PemToECDSAKey(pemBytes)
+	if err != nil {
+		return err
+	}
+	keyID, err := getLogID(pubKey)
+	if err != nil {
+		return err
+	}
+	t.Keys[keyID] = RekorPubKey{PubKey: pubKey, Status: status}
+	return nil
+}

pkg/cosign/tlog_test.go

@@ -24,11 +24,11 @@ func TestGetRekorPubKeys(t *testing.T) {
 	if err != nil {
 		t.Errorf("Unexpected error calling GetRekorPubs, expected nil: %v", err)
 	}
-	if len(keys) == 0 {
+	if len(keys.Keys) == 0 {
 		t.Errorf("expected 1 or more keys, got 0")
 	}
 	// check that the mapping of key digest to key is correct
-	for logID, key := range keys {
+	for logID, key := range keys.Keys {
 		expectedLogID, err := getLogID(key.PubKey)
 		if err != nil {
 			t.Fatalf("unexpected error generated log ID: %v", err)

pkg/cosign/verify.go

@@ -80,10 +80,11 @@ type CheckOpts struct {
 
 	// RekorClient, if set, is used to make online tlog calls use to verify signatures and public keys.
 	RekorClient *client.Rekor
-	// RekorPubKeys, if set, is used to validate signatures on log entries from Rekor.
-	// Note: We use RekorPubKey that contains other information like status besides the
-	// raw public key.
-	RekorPubKeys map[string]RekorPubKey
+	// TrustedRekorPubKeys, if set, is used to validate signatures on log entries from Rekor.
+	// It is a map from log id to RekorPubKey.
+	// Note: The RekorPubKey values contains information like status along with the
+	// raw public key information.
+	RekorPubKeys *TrustedRekorPubKeys
 
 	// SigVerifier is used to verify signatures.
 	SigVerifier signature.Verifier
@@ -402,7 +403,7 @@ func ValidateAndUnpackCertWithChain(cert *x509.Certificate, chain []*x509.Certif
 	return ValidateAndUnpackCert(cert, co)
 }
 
-func tlogValidatePublicKey(ctx context.Context, rekorClient *client.Rekor, rekorPubKeys map[string]RekorPubKey,
+func tlogValidatePublicKey(ctx context.Context, rekorClient *client.Rekor, rekorPubKeys *TrustedRekorPubKeys,
 	pub crypto.PublicKey, sig oci.Signature) error {
 	pemBytes, err := cryptoutils.MarshalPublicKeyToPEM(pub)
 	if err != nil {
@@ -412,7 +413,7 @@ func tlogValidatePublicKey(ctx context.Context, rekorClient *client.Rekor, rekor
 	return err
 }
 
-func tlogValidateCertificate(ctx context.Context, rekorClient *client.Rekor, rekorPubKeys map[string]RekorPubKey,
+func tlogValidateCertificate(ctx context.Context, rekorClient *client.Rekor, rekorPubKeys *TrustedRekorPubKeys,
 	sig oci.Signature) error {
 	cert, err := sig.Cert()
 	if err != nil {
@@ -430,7 +431,7 @@ func tlogValidateCertificate(ctx context.Context, rekorClient *client.Rekor, rek
 	return CheckExpiry(cert, time.Unix(*e.IntegratedTime, 0))
 }
 
-func tlogValidateEntry(ctx context.Context, client *client.Rekor, rekorPubKeys map[string]RekorPubKey,
+func tlogValidateEntry(ctx context.Context, client *client.Rekor, rekorPubKeys *TrustedRekorPubKeys,
 	sig oci.Signature, pem []byte) (*models.LogEntryAnon, error) {
 	b64sig, err := sig.Base64Signature()
 	if err != nil {
@@ -886,7 +887,7 @@ func CheckExpiry(cert *x509.Certificate, it time.Time) error {
 
 // This verifies an offline bundle contained in the sig against the trusted
 // Rekor publicKeys.
-func VerifyBundle(sig oci.Signature, publicKeys map[string]RekorPubKey) (bool, error) {
+func VerifyBundle(sig oci.Signature, publicKeys *TrustedRekorPubKeys) (bool, error) {
 	bundle, err := sig.Bundle()
 	if err != nil {
 		return false, err
@@ -898,7 +899,11 @@ func VerifyBundle(sig oci.Signature, publicKeys map[string]RekorPubKey) (bool, e
 		return false, err
 	}
 
-	pubKey, ok := publicKeys[bundle.Payload.LogID]
+	if publicKeys == nil || publicKeys.Keys == nil {
+		return false, &VerificationError{"no trusted rekor public key found for payload"}
+	}
+
+	pubKey, ok := publicKeys.Keys[bundle.Payload.LogID]
 	if !ok {
 		return false, &VerificationError{"rekor log public key not found for payload"}
 	}

pkg/cosign/verify_test.go

@@ -18,6 +18,7 @@ import (
 	"bytes"
 	"context"
 	"crypto"
+	"crypto/ecdsa"
 	"crypto/elliptic"
 	"crypto/rand"
 	"crypto/sha256"
@@ -53,6 +54,7 @@ import (
 	"github.com/sigstore/sigstore/pkg/cryptoutils"
 	"github.com/sigstore/sigstore/pkg/signature"
 	"github.com/sigstore/sigstore/pkg/signature/options"
+	"github.com/sigstore/sigstore/pkg/tuf"
 	"github.com/stretchr/testify/require"
 	"github.com/transparency-dev/merkle/rfc6962"
 )
@@ -248,9 +250,17 @@ func TestVerifyImageSignatureWithNoChain(t *testing.T) {
 	opts := []static.Option{static.WithCertChain(pemLeaf, []byte{}), static.WithBundle(rekorBundle)}
 	ociSig, _ := static.NewSignature(payload, base64.StdEncoding.EncodeToString(signature), opts...)
 
+	logId, _ := getLogID(sv.Public)
+	ecdsaKey, _ := sv.PublicKey()
+
 	// TODO(asraa): Re-enable passing test when Rekor public keys can be set in CheckOpts,
 	// instead of relying on the singleton TUF instance.
-	verified, err := VerifyImageSignature(context.TODO(), ociSig, v1.Hash{}, &CheckOpts{RootCerts: rootPool})
+	verified, err := VerifyImageSignature(context.TODO(), ociSig, v1.Hash{}, &CheckOpts{
+		RootCerts: rootPool,
+		RekorPubKeys: &TrustedRekorPubKeys{
+			Keys: map[string]RekorPubKey{logId: {PubKey: ecdsaKey.(*ecdsa.PublicKey), Status: tuf.Active}},
+		},
+	})
 	if err == nil {
 		t.Fatalf("expected error due to custom Rekor public key")
 	}