tuf: improve TUF client concurrency and caching (#1953)

* move rekor public key fetch inside GetRekorPubs Signed-off-by: Asra Ali <asraa@google.com> * use in-memory metadata and targets, sync to disk on start and updates Signed-off-by: Asra Ali <asraa@google.com> update Signed-off-by: Asra Ali <asraa@google.com> * Use TUF singleton. Co-authored-by: Ville Aikas <vaikas@chainguard.dev> Signed-off-by: Asra Ali <asraa@google.com> * hayden comment, sync.Once used Signed-off-by: Asra Ali <asraa@google.com> * return global error Signed-off-by: Asra Ali <asraa@google.com> Co-authored-by: Ville Aikas <vaikas@chainguard.dev>
sigstore · Jun 7, 2022 · 6deaffb · 6deaffb
1 parent d14e774
commit 6deaffb
Show file tree

Hide file tree

Showing 10 changed files with 241 additions and 147 deletions.
diff --git a/cmd/cosign/cli/fulcio/fulcioroots/fulcioroots.go b/cmd/cosign/cli/fulcio/fulcioroots/fulcioroots.go
@@ -116,7 +116,6 @@ func initRoots() (*x509.CertPool, *x509.CertPool, error) {
 		if err != nil {
 			return nil, nil, fmt.Errorf("initializing tuf: %w", err)
 		}
-		defer tufClient.Close()
 		// Retrieve from the embedded or cached TUF root. If expired, a network
 		// call is made to update the root.
 		targets, err := tufClient.GetTargetsByMeta(tuf.Fulcio, []string{fulcioTargetStr, fulcioV1TargetStr})

diff --git a/cmd/cosign/cli/fulcio/fulcioverifier/ctl/verify.go b/cmd/cosign/cli/fulcio/fulcioverifier/ctl/verify.go
@@ -81,7 +81,6 @@ func VerifySCT(ctx context.Context, certPEM, chainPEM, rawSCT []byte) error {
 		if err != nil {
 			return err
 		}
-		defer tufClient.Close()
 
 		targets, err := tufClient.GetTargetsByMeta(tuf.CTFE, []string{ctPublicKeyStr})
 		if err != nil {

diff --git a/cmd/cosign/cli/verify/verify_blob.go b/cmd/cosign/cli/verify/verify_blob.go
@@ -293,9 +293,16 @@ func payloadBytes(blobRef string) ([]byte, error) {
 }
 
 func verifyRekorEntry(ctx context.Context, ko options.KeyOpts, e *models.LogEntryAnon, pubKey signature.Verifier, cert *x509.Certificate, b64sig string, blobBytes []byte) error {
+	// TODO: This can be moved below offline bundle verification when SIGSTORE_TRUST_REKOR_API_PUBLIC_KEY
+	// is removed.
+	rekorClient, err := rekor.NewClient(ko.RekorURL)
+	if err != nil {
+		return err
+	}
+
 	// If we have a bundle with a rekor entry, let's first try to verify offline
 	if ko.BundlePath != "" {
-		if err := verifyRekorBundle(ctx, ko.BundlePath, cert); err == nil {
+		if err := verifyRekorBundle(ctx, ko.BundlePath, cert, rekorClient); err == nil {
 			fmt.Fprintf(os.Stderr, "tlog entry verified offline\n")
 			return nil
 		}
@@ -304,10 +311,6 @@ func verifyRekorEntry(ctx context.Context, ko options.KeyOpts, e *models.LogEntr
 		return nil
 	}
 
-	rekorClient, err := rekor.NewClient(ko.RekorURL)
-	if err != nil {
-		return err
-	}
 	// Only fetch from rekor tlog if we don't already have the entry.
 	if e == nil {
 		var pubBytes []byte
@@ -346,15 +349,15 @@ func verifyRekorEntry(ctx context.Context, ko options.KeyOpts, e *models.LogEntr
 	return cosign.CheckExpiry(cert, time.Unix(*e.IntegratedTime, 0))
 }
 
-func verifyRekorBundle(ctx context.Context, bundlePath string, cert *x509.Certificate) error {
+func verifyRekorBundle(ctx context.Context, bundlePath string, cert *x509.Certificate, rekorClient *client.Rekor) error {
 	b, err := cosign.FetchLocalSignedPayloadFromPath(bundlePath)
 	if err != nil {
 		return err
 	}
 	if b.Bundle == nil {
 		return fmt.Errorf("rekor entry is not available")
 	}
-	publicKeys, err := cosign.GetRekorPubs(ctx)
+	publicKeys, err := cosign.GetRekorPubs(ctx, rekorClient)
 	if err != nil {
 		return fmt.Errorf("retrieving rekor public key: %w", err)
 	}

diff --git a/pkg/cosign/tlog.go b/pkg/cosign/tlog.go
@@ -77,10 +77,15 @@ func getLogID(pub crypto.PublicKey) (string, error) {
 
 // GetRekorPubs retrieves trusted Rekor public keys from the embedded or cached
 // TUF root. If expired, makes a network call to retrieve the updated targets.
-// There is an Env variable that can be used to override this behaviour:
+// A Rekor client may optionally be provided in case using SIGSTORE_TRUST_REKOR_API_PUBLIC_KEY
+// (see below).
+// 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) {
+// SIGSTORE_TRUST_REKOR_API_PUBLIC_KEY - If specified, fetches the Rekor public
+// key from the Rekor server using the provided rekorClient.
+// TODO: Rename SIGSTORE_TRUST_REKOR_API_PUBLIC_KEY to be test-only or remove.
+func GetRekorPubs(ctx context.Context, rekorClient *client.Rekor) (map[string]RekorPubKey, error) {
 	publicKeys := make(map[string]RekorPubKey)
 	altRekorPub := os.Getenv(altRekorPublicKey)
 
@@ -104,7 +109,6 @@ func GetRekorPubs(ctx context.Context) (map[string]RekorPubKey, error) {
 		if err != nil {
 			return nil, err
 		}
-		defer tufClient.Close()
 		targets, err := tufClient.GetTargetsByMeta(tuf.Rekor, []string{rekorTargetStr})
 		if err != nil {
 			return nil, err
@@ -121,6 +125,26 @@ func GetRekorPubs(ctx context.Context) (map[string]RekorPubKey, error) {
 			publicKeys[keyID] = RekorPubKey{PubKey: rekorPubKey, Status: t.Status}
 		}
 	}
+
+	// If we have a Rekor client and we've been told to fetch the Public Key from Rekor,
+	// additionally fetch it here.
+	addRekorPublic := os.Getenv(addRekorPublicKeyFromRekor)
+	if addRekorPublic != "" && rekorClient != nil {
+		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)
+		if err != nil {
+			return nil, fmt.Errorf("error generating log ID: %w", err)
+		}
+		publicKeys[keyID] = RekorPubKey{PubKey: pubFromAPI, Status: tuf.Active}
+	}
+
 	if len(publicKeys) == 0 {
 		return nil, errors.New("none of the Rekor public keys have been found")
 	}
@@ -330,9 +354,7 @@ func FindTLogEntriesByPayload(ctx context.Context, rekorClient *client.Rekor, pa
 	return searchIndex.GetPayload(), nil
 }
 
-// VerityTLogEntry verifies a TLog entry. If the Env variable
-// SIGSTORE_TRUST_REKOR_API_PUBLIC_KEY is specified, fetches the Rekor public
-// key from the Rekor server using the provided rekorClient.
+// VerityTLogEntry verifies a TLog entry.
 func VerifyTLogEntry(ctx context.Context, rekorClient *client.Rekor, e *models.LogEntryAnon) error {
 	if e.Verification == nil || e.Verification.InclusionProof == nil {
 		return errors.New("inclusion proof not provided")
@@ -365,37 +387,11 @@ func VerifyTLogEntry(ctx context.Context, rekorClient *client.Rekor, e *models.L
 		LogID:          *e.LogID,
 	}
 
-	// If we've been told to fetch the Public Key from Rekor, fetch it here
-	// first before using the TUF code below.
-	rekorPubKeys := make(map[string]RekorPubKey)
-	addRekorPublic := os.Getenv(addRekorPublicKeyFromRekor)
-	if addRekorPublic != "" {
-		pubOK, err := rekorClient.Pubkey.GetPublicKey(nil)
-		if err != nil {
-			return fmt.Errorf("unable to fetch rekor public key from rekor: %w", err)
-		}
-		pubFromAPI, err := PemToECDSAKey([]byte(pubOK.Payload))
-		if err != nil {
-			return fmt.Errorf("error converting rekor PEM public key from rekor to ECDSAKey: %w", err)
-		}
-		keyID, err := getLogID(pubFromAPI)
-		if err != nil {
-			return fmt.Errorf("error generating log ID: %w", err)
-		}
-		rekorPubKeys[keyID] = RekorPubKey{PubKey: pubFromAPI, Status: tuf.Active}
-	}
-
-	rekorPubKeysTuf, err := GetRekorPubs(ctx)
+	rekorPubKeys, err := GetRekorPubs(ctx, rekorClient)
 	if err != nil {
-		if len(rekorPubKeys) == 0 {
-			return fmt.Errorf("unable to fetch Rekor public keys from TUF repository, and not trusting the Rekor API for fetching public keys: %w", err)
-		}
-		fmt.Fprintf(os.Stderr, "**Warning** Failed to fetch Rekor public keys from TUF, using the public key from Rekor API because %s was specified", addRekorPublicKeyFromRekor)
+		return fmt.Errorf("unable to fetch Rekor public keys: %w", err)
 	}
 
-	for k, v := range rekorPubKeysTuf {
-		rekorPubKeys[k] = v
-	}
 	pubKey, ok := rekorPubKeys[payload.LogID]
 	if !ok {
 		return errors.New("rekor log public key not found for payload")

diff --git a/pkg/cosign/tlog_test.go b/pkg/cosign/tlog_test.go
@@ -20,7 +20,7 @@ import (
 )
 
 func TestGetRekorPubKeys(t *testing.T) {
-	keys, err := GetRekorPubs(context.Background())
+	keys, err := GetRekorPubs(context.Background(), nil)
 	if err != nil {
 		t.Errorf("Unexpected error calling GetRekorPubs, expected nil: %v", err)
 	}