From 0c991796091a19d78722454f52daed50b25d4eec Mon Sep 17 00:00:00 2001 From: Binbin Li Date: Mon, 9 Sep 2024 08:37:05 +0000 Subject: [PATCH] chore: address comments --- pkg/referrerstore/oras/cosign.go | 4 ++-- pkg/referrerstore/oras/oras.go | 2 +- pkg/verifier/cosign/cosign.go | 12 ++++++------ pkg/verifier/cosign/cosign_test.go | 24 ++++++++++++------------ pkg/verifier/cosign/trustpolicies.go | 4 ++-- pkg/verifier/cosign/trustpolicy.go | 8 ++++---- 6 files changed, 27 insertions(+), 27 deletions(-) diff --git a/pkg/referrerstore/oras/cosign.go b/pkg/referrerstore/oras/cosign.go index 2251a7ba35..3db4b23bbd 100644 --- a/pkg/referrerstore/oras/cosign.go +++ b/pkg/referrerstore/oras/cosign.go @@ -46,7 +46,7 @@ func getCosignReferences(ctx context.Context, subjectReference common.Reference, return nil, nil } evictOnError(ctx, err, subjectReference.Original) - return nil, re.ErrorCodeRepositoryOperationFailure.WithDetail("Failed to resolve Cosign signature reference to descriptor").WithError(err) + return nil, re.ErrorCodeRepositoryOperationFailure.WithDetail(fmt.Sprintf("Failed to validate the signature of the artifact: %+v", subjectReference)).WithError(err) } references = append(references, ocispecs.ReferenceDescriptor{ @@ -64,7 +64,7 @@ func getCosignReferences(ctx context.Context, subjectReference common.Reference, func attachedImageTag(subjectReference common.Reference, tagSuffix string) (string, error) { // sha256:d34db33f -> sha256-d34db33f.suffix if subjectReference.Digest.String() == "" { - return "", re.ErrorCodeReferenceInvalid.WithDetail("Cosign subject digest is empty").WithRemediation("Ensure the Cosign signature was generated correctly.") + return "", re.ErrorCodeReferenceInvalid.WithDetail("The digest of the artifact is empty") } tagStr := strings.ReplaceAll(subjectReference.Digest.String(), ":", "-") + tagSuffix return fmt.Sprintf("%s:%s", subjectReference.Path, tagStr), nil diff --git a/pkg/referrerstore/oras/oras.go b/pkg/referrerstore/oras/oras.go index 5b7d7de8e2..64679dd0a4 100644 --- a/pkg/referrerstore/oras/oras.go +++ b/pkg/referrerstore/oras/oras.go @@ -208,7 +208,7 @@ func (store *orasStore) GetConfig() *config.StoreConfig { func (store *orasStore) ListReferrers(ctx context.Context, subjectReference common.Reference, _ []string, _ string, subjectDesc *ocispecs.SubjectDescriptor) (referrerstore.ListReferrersResult, error) { repository, err := store.createRepository(ctx, store, subjectReference) if err != nil { - return referrerstore.ListReferrersResult{}, re.ErrorCodeRepositoryOperationFailure.WithDetail("Failed to create client to remote registry").WithError(err) + return referrerstore.ListReferrersResult{}, re.ErrorCodeRepositoryOperationFailure.WithDetail("Failed to connect to the remote registry").WithError(err) } // resolve subject descriptor if not provided diff --git a/pkg/verifier/cosign/cosign.go b/pkg/verifier/cosign/cosign.go index 29337395b4..4480a81ecf 100644 --- a/pkg/verifier/cosign/cosign.go +++ b/pkg/verifier/cosign/cosign.go @@ -148,7 +148,7 @@ func (f *cosignVerifierFactory) Create(_ string, verifierConfig config.VerifierC config, err := parseVerifierConfig(verifierConfig) if err != nil { - return nil, re.ErrorCodeConfigInvalid.WithDetail("Failed to create Cosign Verifier").WithError(err) + return nil, re.ErrorCodeConfigInvalid.WithDetail("Failed to create the Cosign Verifier").WithError(err) } // if key or rekorURL is provided, trustPolicies should not be provided @@ -163,7 +163,7 @@ func (f *cosignVerifierFactory) Create(_ string, verifierConfig config.VerifierC logger.GetLogger(context.Background(), logOpt).Debugf("legacy cosign verifier configuration not found, creating trust policies") trustPolicies, err = CreateTrustPolicies(config.TrustPolicies, verifierName) if err != nil { - return nil, re.ErrorCodePluginInitFailure.WithDetail("Failed to create Cosign Verifier").WithError(err) + return nil, re.ErrorCodePluginInitFailure.WithDetail("Failed to create the Cosign Verifier").WithError(err) } legacy = false } @@ -229,13 +229,13 @@ func (v *cosignVerifier) verifyInternal(ctx context.Context, subjectReference co // manifest must be an OCI Image if referenceManifest.MediaType != imgspec.MediaTypeImageManifest { - return errorToVerifyResult(v.name, v.verifierType, re.ErrorCodeVerifyPluginFailure.WithDetail("The artifact metadata is not an oci image")), nil + return errorToVerifyResult(v.name, v.verifierType, re.ErrorCodeVerifyPluginFailure.WithDetail("The artifact metadata is not an OCI image")), nil } // get the subject image descriptor subjectDesc, err := referrerStore.GetSubjectDescriptor(ctx, subjectReference) if err != nil { - return errorToVerifyResult(v.name, v.verifierType, re.ErrorCodeGetSubjectDescriptorFailure.WithDetail(fmt.Sprintf("Failed to get the descriptor for subject: %s", subjectReference.Digest)).WithError(err)), nil + return errorToVerifyResult(v.name, v.verifierType, re.ErrorCodeVerifyReferenceFailure.WithDetail(fmt.Sprintf("Failed to validate the signature of the artifact: %+v", subjectReference)).WithError(err)), nil } // create the hash of the subject image descriptor (used as the hashed payload) @@ -485,7 +485,7 @@ func staticLayerOpts(desc imgspec.Descriptor) ([]static.Option, error) { // ErrorToVerifyResult returns a verifier result with the error message and isSuccess set to false func errorToVerifyResult(name string, verifierType string, err error) verifier.VerifierResult { - verifierErr := re.ErrorCodeVerifyReferenceFailure.WithDetail("Cosign signature verification failed").WithError(err) + verifierErr := re.ErrorCodeVerifyReferenceFailure.WithDetail("Failed to validate the Cosign signature").WithError(err) return verifier.NewVerifierResult( "", name, @@ -540,7 +540,7 @@ func verifyWithKeys(ctx context.Context, keysMap map[PKKey]keymanagementprovider if pubKey.ProviderType == azurekeyvault.ProviderName { hashType, sig, err = processAKVSignature(sigEncoded, sig, pubKey.Key, payload, staticOpts) if err != nil { - return verifications, false, re.ErrorCodeVerifyPluginFailure.WithDetail("Failed to process AKV signature").WithError(err) + return verifications, false, re.ErrorCodeVerifyPluginFailure.WithDetail("Failed to validate the signature generated by AKV").WithError(err) } } diff --git a/pkg/verifier/cosign/cosign_test.go b/pkg/verifier/cosign/cosign_test.go index 70efece32d..aea1cf5a7a 100644 --- a/pkg/verifier/cosign/cosign_test.go +++ b/pkg/verifier/cosign/cosign_test.go @@ -428,8 +428,8 @@ func TestErrorToVerifyResult(t *testing.T) { if verifierResult.Type != "cosign" { t.Errorf("errorToVerifyResult() = %v, want %v", verifierResult.Type, "cosign") } - if verifierResult.Message != "Cosign signature verification failed" { - t.Errorf("errorToVerifyResult() = %v, want %v", verifierResult.Message, "Cosign signature verification failed") + if verifierResult.Message != "Failed to validate the Cosign signature" { + t.Errorf("errorToVerifyResult() = %v, want %v", verifierResult.Message, "Failed to validate the Cosign signature") } if verifierResult.ErrorReason != "test error" { t.Errorf("errorToVerifyResult() = %v, want %v", verifierResult.ErrorReason, "test error") @@ -594,7 +594,7 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry keys: map[PKKey]keymanagementprovider.PublicKey{}, getKeysError: true, store: &mocks.MemoryTestStore{}, - expectedResultMessagePrefix: "Cosign signature verification failed", + expectedResultMessagePrefix: "Failed to validate the Cosign signature", expectedErrorReason: "error", }, { @@ -602,7 +602,7 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry keys: map[PKKey]keymanagementprovider.PublicKey{}, getKeysError: false, store: &mocks.MemoryTestStore{}, - expectedResultMessagePrefix: "Cosign signature verification failed", + expectedResultMessagePrefix: "Failed to validate the Cosign signature", expectedErrorReason: "manifest not found", }, { @@ -616,8 +616,8 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry }, }, }, - expectedResultMessagePrefix: "Cosign signature verification failed", - expectedErrorReason: "The artifact metadata is not an oci image", + expectedResultMessagePrefix: "Failed to validate the Cosign signature", + expectedErrorReason: "The artifact metadata is not an OCI image", }, { name: "failed subject descriptor fetch", @@ -630,7 +630,7 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry }, }, }, - expectedResultMessagePrefix: "Cosign signature verification failed", + expectedResultMessagePrefix: "Failed to validate the Cosign signature", expectedErrorReason: "subject not found for sha256:5678", }, { @@ -657,7 +657,7 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry }, }, }, - expectedResultMessagePrefix: "Cosign signature verification failed", + expectedResultMessagePrefix: "Failed to validate the Cosign signature", expectedErrorReason: "blob not found", }, { @@ -689,7 +689,7 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry blobDigest: validSignatureBlob, }, }, - expectedResultMessagePrefix: "Cosign signature verification failed", + expectedResultMessagePrefix: "Failed to validate the Cosign signature", expectedErrorReason: "unsupported public key type [*ecdh.PublicKey]", }, { @@ -721,7 +721,7 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry blobDigest: validSignatureBlob, }, }, - expectedResultMessagePrefix: "Cosign signature verification failed", + expectedResultMessagePrefix: "Failed to validate the Cosign signature", expectedErrorReason: "RSA key check: unsupported key size: 128", }, { @@ -753,7 +753,7 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry blobDigest: validSignatureBlob, }, }, - expectedResultMessagePrefix: "Cosign signature verification failed", + expectedResultMessagePrefix: "Failed to validate the Cosign signature", expectedErrorReason: "ECDSA key check: unsupported key curve [P-224]", }, { @@ -986,7 +986,7 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry "sha256:d1226e36bc8502978324cb2cb2116c6aa48edb2ea8f15b1c6f6f256ed43388f6": []byte(`{"critical":{"identity":{"docker-reference":"wabbitnetworks.azurecr.io/test/cosign-image"},"image":{"docker-manifest-digest":"sha256:623621b56649b5e0c2c7cf3ffd987932f8f9a5a01036e00d6f3ae9480087621c"},"type":"cosign container image signature"},"optional":null}`), }, }, - expectedResultMessagePrefix: "Cosign signature verification failed", + expectedResultMessagePrefix: "Failed to validate the Cosign signature:", expectedErrorReason: "failed to unmarshal bundle from blob payload: illegal base64 data at input byte 91", }, } diff --git a/pkg/verifier/cosign/trustpolicies.go b/pkg/verifier/cosign/trustpolicies.go index cb86ec22fa..6ba5521ed7 100644 --- a/pkg/verifier/cosign/trustpolicies.go +++ b/pkg/verifier/cosign/trustpolicies.go @@ -35,7 +35,7 @@ var validScopeRegex = regexp.MustCompile(`^[a-z0-9\.\-:@\/]*\*?$`) // CreateTrustPolicies creates a set of trust policies from the given configuration func CreateTrustPolicies(configs []TrustPolicyConfig, verifierName string) (*TrustPolicies, error) { if len(configs) == 0 { - return nil, re.ErrorCodeConfigInvalid.WithDetail("Failed to create trust policies: no policies found").WithRemediation("Ensure that the trust policy configuration is correct.") + return nil, re.ErrorCodeConfigInvalid.WithDetail("Failed to create trust policies: policy configuration not found").WithRemediation("Ensure that the trust policy configuration is correct.") } policies := make([]TrustPolicy, 0, len(configs)) @@ -86,7 +86,7 @@ func (tps *TrustPolicies) GetScopedPolicy(reference string) (TrustPolicy, error) if globalPolicy != nil { return globalPolicy, nil } - return nil, re.ErrorCodeConfigInvalid.WithDetail(fmt.Sprintf("No policy found for reference %s", reference)) + return nil, re.ErrorCodeConfigInvalid.WithDetail(fmt.Sprintf("No policy found for the artifact %s", reference)) } // validateScopes validates the scopes in the trust policies diff --git a/pkg/verifier/cosign/trustpolicy.go b/pkg/verifier/cosign/trustpolicy.go index c9b1d137f0..ed32cebebc 100644 --- a/pkg/verifier/cosign/trustpolicy.go +++ b/pkg/verifier/cosign/trustpolicy.go @@ -193,7 +193,7 @@ func (tp *trustPolicy) GetCosignOpts(ctx context.Context) (cosign.CheckOpts, err // Fetches the Rekor public keys from the Rekor server cosignOpts.RekorPubKeys, err = cosign.GetRekorPubs(ctx) if err != nil { - return cosignOpts, re.ErrorCodeVerifyPluginFailure.WithDetail("Failed to fetch Rekor public keys").WithRemediation(fmt.Sprintf("Please check if the Rekor server %s is experiencing any outages", tp.config.RekorURL)).WithError(err) + return cosignOpts, re.ErrorCodeVerifyPluginFailure.WithDetail("Failed to fetch Rekor public keys").WithRemediation(fmt.Sprintf("Please check if the Rekor server %s is available", tp.config.RekorURL)).WithError(err) } } else { cosignOpts.IgnoreTlog = true @@ -203,20 +203,20 @@ func (tp *trustPolicy) GetCosignOpts(ctx context.Context) (cosign.CheckOpts, err if tp.isKeyless { roots, err := fulcio.GetRoots() if err != nil || roots == nil { - return cosignOpts, re.ErrorCodeVerifyPluginFailure.WithDetail("Failed to get fulcio root").WithError(err).WithRemediation("Please check if Fulcio is experiencing any outages") + return cosignOpts, re.ErrorCodeVerifyPluginFailure.WithDetail("Failed to get fulcio root").WithError(err).WithRemediation("Please check if Fulcio is available") } cosignOpts.RootCerts = roots if tp.config.Keyless.CTLogVerify != nil && *tp.config.Keyless.CTLogVerify { cosignOpts.CTLogPubKeys, err = cosign.GetCTLogPubs(ctx) if err != nil { - return cosignOpts, re.ErrorCodeVerifyPluginFailure.WithDetail("Failed to fetch certificate transparency log public keys").WithError(err).WithRemediation("Please check if TUF root is experiencing any outages") + return cosignOpts, re.ErrorCodeVerifyPluginFailure.WithDetail("Failed to fetch certificate transparency log public keys").WithError(err).WithRemediation("Please check if TUF root is available") } } else { cosignOpts.IgnoreSCT = true } cosignOpts.IntermediateCerts, err = fulcio.GetIntermediates() if err != nil { - return cosignOpts, re.ErrorCodeVerifyPluginFailure.WithDetail("Failed to get fulcio intermediate certificates").WithError(err).WithRemediation("Please check if Fulcio is experiencing any outages") + return cosignOpts, re.ErrorCodeVerifyPluginFailure.WithDetail("Failed to get fulcio intermediate certificates").WithError(err).WithRemediation("Please check if Fulcio is available") } // Set the certificate identity and issuer for keyless verification cosignOpts.Identities = []cosign.Identity{