Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix fetching updated targets from TUF root (based off of #1921) #1932

Closed
wants to merge 9 commits into from
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ cross:
golangci-lint:
rm -f $(GOLANGCI_LINT_BIN) || :
set -e ;\
GOBIN=$(GOLANGCI_LINT_DIR) go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.43.0 ;\
GOBIN=$(GOLANGCI_LINT_DIR) go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.46.2 ;\

lint: golangci-lint ## Run golangci-lint linter
$(GOLANGCI_LINT_BIN) run -n
Expand Down
2 changes: 1 addition & 1 deletion cmd/cosign/cli/fulcio/fulcioroots/fulcioroots.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func initRoots() (*x509.CertPool, *x509.CertPool, error) {
// call is made to update the root.
targets, err := tufClient.GetTargetsByMeta(tuf.Fulcio, []string{fulcioTargetStr, fulcioV1TargetStr})
if err != nil {
return nil, nil, errors.New("error getting targets")
return nil, nil, fmt.Errorf("error getting targets: %w", err)
}
if len(targets) == 0 {
return nil, nil, errors.New("none of the Fulcio roots have been found")
Expand Down
44 changes: 30 additions & 14 deletions pkg/cosign/tlog.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,18 +77,13 @@ 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:
// 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) {
tufClient, err := tuf.NewFromEnv(ctx)
if err != nil {
return nil, err
}
defer tufClient.Close()
targets, err := tufClient.GetTargetsByMeta(tuf.Rekor, []string{rekorTargetStr})
if err != nil {
return nil, err
}
publicKeys := make(map[string]RekorPubKey)
altRekorPub := os.Getenv(altRekorPublicKey)

if altRekorPub != "" {
fmt.Fprintf(os.Stderr, "**Warning** Using a non-standard public key for Rekor: %s\n", altRekorPub)
raw, err := os.ReadFile(altRekorPub)
Expand All @@ -105,6 +100,15 @@ func GetRekorPubs(ctx context.Context) (map[string]RekorPubKey, error) {
}
publicKeys[keyID] = RekorPubKey{PubKey: extra, Status: tuf.Active}
} else {
tufClient, err := tuf.NewFromEnv(ctx)
if err != nil {
return nil, err
}
defer tufClient.Close()
targets, err := tufClient.GetTargetsByMeta(tuf.Rekor, []string{rekorTargetStr})
if err != nil {
return nil, err
}
for _, t := range targets {
rekorPubKey, err := PemToECDSAKey(t.Target)
if err != nil {
Expand Down Expand Up @@ -326,6 +330,9 @@ 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.
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")
Expand Down Expand Up @@ -358,11 +365,9 @@ func VerifyTLogEntry(ctx context.Context, rekorClient *client.Rekor, e *models.L
LogID: *e.LogID,
}

rekorPubKeys, err := GetRekorPubs(ctx)
if err != nil {
return fmt.Errorf("unable to fetch Rekor public keys from TUF repository: %w", err)
}

// 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)
Expand All @@ -380,6 +385,17 @@ func VerifyTLogEntry(ctx context.Context, rekorClient *client.Rekor, e *models.L
rekorPubKeys[keyID] = RekorPubKey{PubKey: pubFromAPI, Status: tuf.Active}
}

rekorPubKeysTuf, err := GetRekorPubs(ctx)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be cleaner to have all the override behaviour in one place, and move L371-386 to GetRekorPubs and have everything there. But, that would mean plumbing RekorClient through to GetRekorPubs, so wasn't sure if we wanted to do that. One thing that we might want to consider is plumbing the RekorClient into the ctx passed in and pull it from there so we wouldn't have to change the signature. I'd be happy to do that if we think that makes sense.

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)
}

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")
Expand Down