From 4d4d00c9d7a82f3005ece9d65c37ce87bde01ebe Mon Sep 17 00:00:00 2001 From: Binbin Li Date: Tue, 10 Sep 2024 14:24:41 +0800 Subject: [PATCH] feat: refactor cosign verification error messages (#1750) --- pkg/referrerstore/oras/cosign.go | 4 +- pkg/referrerstore/oras/oras.go | 8 +- pkg/verifier/cosign/cosign.go | 40 +++---- pkg/verifier/cosign/cosign_test.go | 137 ++++++++++++++++++++---- pkg/verifier/cosign/trustpolicies.go | 20 ++-- pkg/verifier/cosign/trustpolicy.go | 44 ++++---- pkg/verifier/cosign/trustpolicy_test.go | 90 +++++++++++++++- 7 files changed, 265 insertions(+), 78 deletions(-) diff --git a/pkg/referrerstore/oras/cosign.go b/pkg/referrerstore/oras/cosign.go index a60f458ae..3db4b23bb 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.WithError(err).WithComponentType(re.ReferrerStore) + 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.WithComponentType(re.ReferrerStore).WithDetail("Cosign subject digest is empty") + 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 a8462d201..64679dd0a 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.ErrorCodeCreateRepositoryFailure.WithError(err).WithComponentType(re.ReferrerStore) + return referrerstore.ListReferrersResult{}, re.ErrorCodeRepositoryOperationFailure.WithDetail("Failed to connect to the remote registry").WithError(err) } // resolve subject descriptor if not provided @@ -260,7 +260,7 @@ func (store *orasStore) GetBlobContent(ctx context.Context, subjectReference com repository, err := store.createRepository(ctx, store, subjectReference) if err != nil { - return nil, err + return nil, re.ErrorCodeGetBlobContentFailure.WithDetail("Failed to connect to the remote registry").WithError(err) } // create a dummy Descriptor to check the local store cache @@ -292,10 +292,10 @@ func (store *orasStore) GetBlobContent(ctx context.Context, subjectReference com blobDesc, rc, err := repository.Blobs().FetchReference(ctx, ref) if err != nil { evictOnError(ctx, err, subjectReference.Original) - return nil, err + return nil, re.ErrorCodeRepositoryOperationFailure.WithDetail("Failed to fetch the artifact metadata from the registry").WithError(err) } if blobContent, err = io.ReadAll(rc); err != nil { - return nil, re.ErrorCodeGetBlobContentFailure.WithError(err) + return nil, re.ErrorCodeRepositoryOperationFailure.WithDetail("Failed to parse the artifact metadata").WithError(err) } // push fetched content to local ORAS cache diff --git a/pkg/verifier/cosign/cosign.go b/pkg/verifier/cosign/cosign.go index fb3efd714..cba6a1e9a 100644 --- a/pkg/verifier/cosign/cosign.go +++ b/pkg/verifier/cosign/cosign.go @@ -143,17 +143,17 @@ func (f *cosignVerifierFactory) Create(_ string, verifierConfig config.VerifierC logger.GetLogger(context.Background(), logOpt).Debugf("creating cosign verifier with config %v, namespace '%v'", verifierConfig, namespace) verifierName, hasName := verifierConfig[types.Name].(string) if !hasName { - return nil, re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithPluginName(verifierName).WithDetail("missing name in verifier config") + return nil, re.ErrorCodeConfigInvalid.WithDetail("The name field is required in the Cosign Verifier configuration") } config, err := parseVerifierConfig(verifierConfig) if err != nil { - return nil, re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithPluginName(verifierName) + return nil, re.ErrorCodeConfigInvalid.WithDetail("Failed to create the Cosign Verifier").WithError(err) } // if key or rekorURL is provided, trustPolicies should not be provided if (config.KeyRef != "" || config.RekorURL != "") && len(config.TrustPolicies) > 0 { - return nil, re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithPluginName(verifierName).WithDetail("'key' and 'rekorURL' are part of cosign legacy configuration and cannot be used with `trustPolicies`") + return nil, re.ErrorCodeConfigInvalid.WithDetail("'key' and 'rekorURL' are part of Cosign legacy configuration and cannot be used with `trustPolicies` parameter") } var trustPolicies *TrustPolicies @@ -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, err + return nil, re.ErrorCodePluginInitFailure.WithDetail("Failed to create the Cosign Verifier").WithError(err) } legacy = false } @@ -224,18 +224,18 @@ func (v *cosignVerifier) verifyInternal(ctx context.Context, subjectReference co // get the reference manifest (cosign oci image) referenceManifest, err := referrerStore.GetReferenceManifest(ctx, subjectReference, referenceDescriptor) if err != nil { - return errorToVerifyResult(v.name, v.verifierType, fmt.Errorf("failed to get reference manifest: %w", err)), nil + return errorToVerifyResult(v.name, v.verifierType, re.ErrorCodeVerifyPluginFailure.WithDetail(fmt.Sprintf("Failed to get artifact metadata for %s", referenceDescriptor.Digest)).WithError(err)), nil } // manifest must be an OCI Image if referenceManifest.MediaType != imgspec.MediaTypeImageManifest { - return errorToVerifyResult(v.name, v.verifierType, fmt.Errorf("reference manifest is not an 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, fmt.Errorf("failed to create subject hash: %w", err)), nil + return errorToVerifyResult(v.name, v.verifierType, re.ErrorCodeVerifyReferenceFailure.WithDetail(fmt.Sprintf("Failed to validate the Cosign signature of the artifact: %+v", subjectReference)).WithError(err)), nil } // create the hash of the subject image descriptor (used as the hashed payload) @@ -255,23 +255,23 @@ func (v *cosignVerifier) verifyInternal(ctx context.Context, subjectReference co // fetch the blob content of the signature from the referrer store blobBytes, err := referrerStore.GetBlobContent(ctx, subjectReference, blob.Digest) if err != nil { - return errorToVerifyResult(v.name, v.verifierType, fmt.Errorf("failed to get blob content: %w", err)), nil + return errorToVerifyResult(v.name, v.verifierType, re.ErrorCodeGetBlobContentFailure.WithDetail(fmt.Sprintf("Failed to get Cosign signature with digest %s", blob.Digest)).WithError(err)), nil } // convert the blob to a static signature staticOpts, err := staticLayerOpts(blob) if err != nil { - return errorToVerifyResult(v.name, v.verifierType, fmt.Errorf("failed to parse static signature opts: %w", err)), nil + return errorToVerifyResult(v.name, v.verifierType, re.ErrorCodeVerifyPluginFailure.WithDetail(fmt.Sprintf("Failed to parse Cosign signature with digest %s", blob.Digest)).WithError(err)), nil } sig, err := static.NewSignature(blobBytes, blob.Annotations[static.SignatureAnnotationKey], staticOpts...) if err != nil { - return errorToVerifyResult(v.name, v.verifierType, fmt.Errorf("failed to generate static signature: %w", err)), nil + return errorToVerifyResult(v.name, v.verifierType, re.ErrorCodeVerifyPluginFailure.WithDetail("Failed to validate the Cosign signature").WithError(err)), nil } if len(keysMap) > 0 { // if keys are found, perform verification with keys var verifications []cosignExtension verifications, hasValidSignature, err = verifyWithKeys(ctx, keysMap, sig, blob.Annotations[static.SignatureAnnotationKey], blobBytes, staticOpts, &cosignOpts, subjectDescHash) if err != nil { - return errorToVerifyResult(v.name, v.verifierType, fmt.Errorf("failed to verify with keys: %w", err)), nil + return errorToVerifyResult(v.name, v.verifierType, re.ErrorCodeVerifyPluginFailure.WithDetail("Failed to validate the Cosign signature with keys").WithError(err)), nil } extensionListEntry.Verifications = append(extensionListEntry.Verifications, verifications...) } else { @@ -295,7 +295,7 @@ func (v *cosignVerifier) verifyInternal(ctx context.Context, subjectReference co ), nil } - errorResult := errorToVerifyResult(v.name, v.verifierType, fmt.Errorf("no valid signatures found")) + errorResult := errorToVerifyResult(v.name, v.verifierType, fmt.Errorf("no valid Cosign signatures found")) errorResult.Extensions = Extension{SignatureExtension: sigExtensions, TrustPolicy: trustPolicy.GetName()} return errorResult, nil } @@ -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("Verification failed").WithError(err) + verifierErr := re.ErrorCodeVerifyReferenceFailure.WithDetail("Failed to validate the Cosign signature").WithError(err) return verifier.NewVerifierResult( "", name, @@ -540,14 +540,14 @@ 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, fmt.Errorf("failed to process AKV signature: %w", err) + return verifications, false, re.ErrorCodeVerifyPluginFailure.WithDetail("Failed to validate the Cosign signature generated by AKV").WithError(err) } } // return the correct verifier based on public key type and bytes verifier, err := signature.LoadVerifier(pubKey.Key, hashType) if err != nil { - return verifications, false, fmt.Errorf("failed to load public key from provider [%s] name [%s] version [%s]: %w", mapKey.Provider, mapKey.Name, mapKey.Version, err) + return verifications, false, re.ErrorCodeVerifyPluginFailure.WithDetail(fmt.Sprintf("Failed to load public key from provider [%s] name [%s] version [%s]", mapKey.Provider, mapKey.Name, mapKey.Version)).WithError(err) } cosignOpts.SigVerifier = verifier // verify signature with cosign options + perform bundle verification @@ -627,17 +627,17 @@ func processAKVSignature(sigEncoded string, staticSig oci.Signature, publicKey c // EC verifiers in cosign have built in ASN.1 decoding, but RSA verifiers do not base64DecodedBytes, err := base64.StdEncoding.DecodeString(sigEncoded) if err != nil { - return crypto.SHA256, nil, fmt.Errorf("RSA key check: failed to decode base64 signature: %w", err) + return crypto.SHA256, nil, re.ErrorCodeVerifyPluginFailure.WithDetail("RSA key check: failed to decode base64 signature").WithError(err) } // decode ASN.1 signature to raw signature if it is ASN.1 encoded decodedSigBytes, err := decodeASN1Signature(base64DecodedBytes) if err != nil { - return crypto.SHA256, nil, fmt.Errorf("RSA key check: failed to decode ASN.1 signature: %w", err) + return crypto.SHA256, nil, re.ErrorCodeVerifyPluginFailure.WithDetail("RSA key check: failed to decode ASN.1 signature").WithError(err) } encodedBase64SigBytes := base64.StdEncoding.EncodeToString(decodedSigBytes) staticSig, err = static.NewSignature(payloadBytes, encodedBase64SigBytes, staticOpts...) if err != nil { - return crypto.SHA256, nil, fmt.Errorf("RSA key check: failed to generate static signature: %w", err) + return crypto.SHA256, nil, re.ErrorCodeVerifyPluginFailure.WithDetail("RSA key check: failed to generate static signature").WithError(err) } case *ecdsa.PublicKey: switch keyType.Curve { @@ -648,10 +648,10 @@ func processAKVSignature(sigEncoded string, staticSig oci.Signature, publicKey c case elliptic.P521(): hashType = crypto.SHA512 default: - return crypto.SHA256, nil, fmt.Errorf("ECDSA key check: unsupported key curve: %s", keyType.Params().Name) + return crypto.SHA256, nil, fmt.Errorf("ECDSA key check: unsupported key curve [%s]", keyType.Params().Name) } default: - return crypto.SHA256, nil, fmt.Errorf("unsupported public key type: %T", publicKey) + return crypto.SHA256, nil, fmt.Errorf("unsupported public key type [%T]", publicKey) } return hashType, staticSig, nil } diff --git a/pkg/verifier/cosign/cosign_test.go b/pkg/verifier/cosign/cosign_test.go index b4e368617..aea1cf5a7 100644 --- a/pkg/verifier/cosign/cosign_test.go +++ b/pkg/verifier/cosign/cosign_test.go @@ -23,6 +23,7 @@ import ( "crypto/elliptic" "crypto/rand" "crypto/rsa" + "encoding/base64" "fmt" "io" "slices" @@ -114,6 +115,26 @@ func TestCreate(t *testing.T) { }, wantErr: false, }, + { + name: "duplicate trust policies in config", + config: config.VerifierConfig{ + "name": "test", + "artifactTypes": "testtype", + "trustPolicies": []TrustPolicyConfig{ + { + Name: "test", + Keyless: KeylessConfig{CertificateIdentity: testIdentity, CertificateOIDCIssuer: testIssuer}, + Scopes: []string{"*"}, + }, + { + Name: "test", + Keyless: KeylessConfig{CertificateIdentity: testIdentity, CertificateOIDCIssuer: testIssuer}, + Scopes: []string{"*"}, + }, + }, + }, + wantErr: true, + }, { name: "invalid config with legacy and trust policies", config: config.VerifierConfig{ @@ -407,8 +428,8 @@ func TestErrorToVerifyResult(t *testing.T) { if verifierResult.Type != "cosign" { t.Errorf("errorToVerifyResult() = %v, want %v", verifierResult.Type, "cosign") } - if verifierResult.Message != "Verification failed" { - t.Errorf("errorToVerifyResult() = %v, want %v", verifierResult.Message, "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") @@ -573,7 +594,7 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry keys: map[PKKey]keymanagementprovider.PublicKey{}, getKeysError: true, store: &mocks.MemoryTestStore{}, - expectedResultMessagePrefix: "Verification failed", + expectedResultMessagePrefix: "Failed to validate the Cosign signature", expectedErrorReason: "error", }, { @@ -581,8 +602,8 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry keys: map[PKKey]keymanagementprovider.PublicKey{}, getKeysError: false, store: &mocks.MemoryTestStore{}, - expectedResultMessagePrefix: "Verification failed", - expectedErrorReason: "failed to get reference manifest: manifest not found", + expectedResultMessagePrefix: "Failed to validate the Cosign signature", + expectedErrorReason: "manifest not found", }, { name: "incorrect reference manifest media type error", @@ -595,8 +616,8 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry }, }, }, - expectedResultMessagePrefix: "Verification failed", - expectedErrorReason: "reference manifest is not an image", + expectedResultMessagePrefix: "Failed to validate the Cosign signature", + expectedErrorReason: "The artifact metadata is not an OCI image", }, { name: "failed subject descriptor fetch", @@ -609,8 +630,8 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry }, }, }, - expectedResultMessagePrefix: "Verification failed", - expectedErrorReason: "failed to create subject hash: subject not found for sha256:5678", + expectedResultMessagePrefix: "Failed to validate the Cosign signature", + expectedErrorReason: "subject not found for sha256:5678", }, { name: "failed to fetch blob", @@ -636,8 +657,8 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry }, }, }, - expectedResultMessagePrefix: "Verification failed", - expectedErrorReason: "failed to get blob content: blob not found", + expectedResultMessagePrefix: "Failed to validate the Cosign signature", + expectedErrorReason: "blob not found", }, { name: "invalid key type for AKV", @@ -668,8 +689,8 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry blobDigest: validSignatureBlob, }, }, - expectedResultMessagePrefix: "Verification failed", - expectedErrorReason: "failed to verify with keys: failed to process AKV signature: unsupported public key type: *ecdh.PublicKey", + expectedResultMessagePrefix: "Failed to validate the Cosign signature", + expectedErrorReason: "unsupported public key type [*ecdh.PublicKey]", }, { name: "invalid RSA key size for AKV", @@ -700,8 +721,8 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry blobDigest: validSignatureBlob, }, }, - expectedResultMessagePrefix: "Verification failed", - expectedErrorReason: "failed to verify with keys: failed to process AKV signature: RSA key check: unsupported key size: 128", + expectedResultMessagePrefix: "Failed to validate the Cosign signature", + expectedErrorReason: "RSA key check: unsupported key size: 128", }, { name: "invalid ECDSA curve type for AKV", @@ -732,8 +753,8 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry blobDigest: validSignatureBlob, }, }, - expectedResultMessagePrefix: "Verification failed", - expectedErrorReason: "failed to verify with keys: failed to process AKV signature: ECDSA key check: unsupported key curve: P-224", + expectedResultMessagePrefix: "Failed to validate the Cosign signature", + expectedErrorReason: "ECDSA key check: unsupported key curve [P-224]", }, { name: "valid hash: 256 keysize: 2048 RSA key AKV", @@ -965,8 +986,8 @@ 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: "Verification failed", - expectedErrorReason: "failed to parse static signature opts: failed to unmarshal bundle from blob payload: illegal base64 data at input byte 91", + expectedResultMessagePrefix: "Failed to validate the Cosign signature:", + expectedErrorReason: "failed to unmarshal bundle from blob payload: illegal base64 data at input byte 91", }, } @@ -1051,3 +1072,81 @@ func TestVerificationMessage(t *testing.T) { }) } } + +func TestProcessAKVSignature_RSAKey(t *testing.T) { + tests := []struct { + name string + keySize int + encodedSig string + expectedErr bool + expectedHashType crypto.Hash + expectedSigOut bool + }{ + { + name: "RSA 2048 bits", + keySize: 256, + expectedErr: false, + expectedHashType: crypto.SHA256, + expectedSigOut: true, + }, + { + name: "RSA 3072 bits", + keySize: 384, + expectedErr: false, + expectedHashType: crypto.SHA384, + expectedSigOut: true, + }, + { + name: "RSA 4096 bits", + keySize: 512, + expectedErr: false, + expectedHashType: crypto.SHA512, + expectedSigOut: true, + }, + { + name: "Unsupported key size", + keySize: 128, + expectedErr: true, + }, + { + name: "Invalid base64 encoded signature", + keySize: 256, + encodedSig: "ThisIsNot@ValidBase64%String!", + expectedErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a mock RSA public key + privateKey, err := rsa.GenerateKey(rand.Reader, tt.keySize*8) + if err != nil { + t.Fatalf("Failed to generate RSA key: %v", err) + } + rsaPublicKey := &privateKey.PublicKey + + // Mock the signature as base64 encoded string + sig := "dummy_signature" + encodedSig := base64.StdEncoding.EncodeToString([]byte(sig)) + if tt.encodedSig != "" { + encodedSig = tt.encodedSig + } + + // Process the signature + hashType, sigOut, err := processAKVSignature(encodedSig, nil, rsaPublicKey, []byte("test payload"), []static.Option{}) + + if tt.expectedErr { + if err == nil { + t.Fatalf("Expected error but got nil") + } + } else { + if hashType != tt.expectedHashType { + t.Fatalf("Expected hash type %v but got %v", tt.expectedHashType, hashType) + } + if tt.expectedSigOut != (sigOut != nil) { + t.Fatalf("Expected signature output to be %v but got %v", tt.expectedSigOut, sigOut) + } + } + }) + } +} diff --git a/pkg/verifier/cosign/trustpolicies.go b/pkg/verifier/cosign/trustpolicies.go index 4858cfd47..6ba5521ed 100644 --- a/pkg/verifier/cosign/trustpolicies.go +++ b/pkg/verifier/cosign/trustpolicies.go @@ -35,14 +35,14 @@ 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.WithComponentType(re.Verifier).WithPluginName(verifierName).WithDetail("failed to create trust policies: no policies found") + 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)) names := make(map[string]struct{}) for _, policyConfig := range configs { if _, ok := names[policyConfig.Name]; ok { - return nil, re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithPluginName(verifierName).WithDetail(fmt.Sprintf("failed to create trust policies: duplicate policy name %s", policyConfig.Name)) + return nil, re.ErrorCodeConfigInvalid.WithDetail(fmt.Sprintf("Failed to create trust policies: duplicate policy name %s", policyConfig.Name)).WithRemediation("Ensure that trust policy names are unique.") } names[policyConfig.Name] = struct{}{} policy, err := CreateTrustPolicy(policyConfig, verifierName) @@ -86,7 +86,7 @@ func (tps *TrustPolicies) GetScopedPolicy(reference string) (TrustPolicy, error) if globalPolicy != nil { return globalPolicy, nil } - return nil, re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithDetail(fmt.Sprintf("failed to get trust policy: 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 @@ -97,16 +97,16 @@ func validateScopes(policies []TrustPolicy) error { policyName := policy.GetName() scopes := policy.GetScopes() if len(scopes) == 0 { - return re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithDetail(fmt.Sprintf("failed to create trust policies: no scopes defined for trust policy %s", policyName)) + return re.ErrorCodeConfigInvalid.WithDetail(fmt.Sprintf("Failed to create trust policies: scope parameter is required for trust policy %s", policyName)) } // check for global wildcard character along with other scopes in the same policy if len(scopes) > 1 && slices.Contains(scopes, string(GlobalWildcardCharacter)) { - return re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithDetail(fmt.Sprintf("failed to create trust policies: global wildcard character %c cannot be used with other scopes within the same trust policy %s", GlobalWildcardCharacter, policyName)) + return re.ErrorCodeConfigInvalid.WithDetail(fmt.Sprintf("Failed to create trust policies: global wildcard character %c cannot be used with other scopes within the same trust policy %s", GlobalWildcardCharacter, policyName)) } // check for duplicate global wildcard characters across policies if slices.Contains(scopes, string(GlobalWildcardCharacter)) { if hasGlobalWildcard { - return re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithDetail(fmt.Sprintf("failed to create trust policies: global wildcard character %c can only be used once", GlobalWildcardCharacter)) + return re.ErrorCodeConfigInvalid.WithDetail(fmt.Sprintf("Failed to create trust policies: global wildcard character %c can only be used once", GlobalWildcardCharacter)) } hasGlobalWildcard = true continue @@ -114,15 +114,15 @@ func validateScopes(policies []TrustPolicy) error { for _, scope := range scopes { // check for empty scope if scope == "" { - return re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithDetail(fmt.Sprintf("failed to create trust policies: scope defined is empty for trust policy %s", policyName)) + return re.ErrorCodeConfigInvalid.WithDetail(fmt.Sprintf("Failed to create trust policies: scope value cannot be empty in trust policy %s", policyName)) } // check scope is formatted correctly if !validScopeRegex.MatchString(scope) { - return re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithDetail(fmt.Sprintf("failed to create trust policies: invalid scope %s for trust policy %s", scope, policyName)) + return re.ErrorCodeConfigInvalid.WithDetail(fmt.Sprintf("Failed to create trust policies: invalid scope %s for trust policy %s", scope, policyName)) } // check for duplicate scopes if _, ok := scopesMap[scope]; ok { - return re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithDetail(fmt.Sprintf("failed to create trust policies: duplicate scope %s for trust policy %s", scope, policyName)) + return re.ErrorCodeConfigInvalid.WithDetail(fmt.Sprintf("Failed to create trust policies: duplicate scope %s for trust policy %s", scope, policyName)) } // check wildcard overlaps for existingScope := range scopesMap { @@ -144,7 +144,7 @@ func validateScopes(policies []TrustPolicy) error { isConflict = strings.HasPrefix(existingScope, trimmedScope) } if isConflict { - return re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithDetail(fmt.Sprintf("failed to create trust policies: overlapping scopes %s and %s for trust policy %s", scope, existingScope, policyName)) + return re.ErrorCodeConfigInvalid.WithDetail(fmt.Sprintf("Failed to create trust policies: overlapping scopes %s and %s for trust policy %s", scope, existingScope, policyName)) } } scopesMap[scope] = struct{}{} diff --git a/pkg/verifier/cosign/trustpolicy.go b/pkg/verifier/cosign/trustpolicy.go index c49b86aa8..8d20a7320 100644 --- a/pkg/verifier/cosign/trustpolicy.go +++ b/pkg/verifier/cosign/trustpolicy.go @@ -97,7 +97,7 @@ func CreateTrustPolicy(config TrustPolicyConfig, verifierName string) (TrustPoli config.Version = DefaultTrustPolicyConfigVersion } - if err := validate(config, verifierName); err != nil { + if err := validate(config); err != nil { return nil, err } @@ -107,7 +107,7 @@ func CreateTrustPolicy(config TrustPolicyConfig, verifierName string) (TrustPoli if keyConfig.File != "" { pubKey, err := loadKeyFromPath(keyConfig.File) if err != nil { - return nil, re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithPluginName(verifierName).WithDetail(fmt.Sprintf("trust policy %s failed: failed to load key from file %s", config.Name, keyConfig.File)).WithError(err) + return nil, re.ErrorCodeConfigInvalid.WithDetail(fmt.Sprintf("Invalid trust policy [%s]: failed to load the key from file %s", config.Name, keyConfig.File)).WithError(err).WithRemediation("Ensure that the key file path is correct and public key is correctly saved.") } keyMap[PKKey{Provider: fileProviderName, Name: keyConfig.File}] = keymanagementprovider.PublicKey{Key: pubKey, ProviderType: fileProviderName} } @@ -155,13 +155,13 @@ func (tp *trustPolicy) GetKeys(ctx context.Context, _ string) (map[PKKey]keymana // get the key management provider resource which contains a map of keys kmpResource, kmpErr := keymanagementprovider.GetKeysFromMap(ctx, keyConfig.Provider) if kmpErr != nil { - return nil, re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithPluginName(tp.verifierName).WithDetail(fmt.Sprintf("trust policy [%s] failed to access key management provider %s, err: %s", tp.config.Name, keyConfig.Provider, kmpErr.Error())) + return nil, re.ErrorCodeConfigInvalid.WithDetail(fmt.Sprintf("Invalid trust policy [%s]: failed to access key management provider %s", tp.config.Name, keyConfig.Provider)).WithError(kmpErr) } // get a specific key from the key management provider resource if keyConfig.Name != "" { pubKey, exists := kmpResource[keymanagementprovider.KMPMapKey{Name: keyConfig.Name, Version: keyConfig.Version}] if !exists { - return nil, re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithPluginName(tp.verifierName).WithDetail(fmt.Sprintf("trust policy %s failed: key %s with version %s not found in key management provider %s", tp.config.Name, keyConfig.Name, keyConfig.Version, keyConfig.Provider)) + return nil, re.ErrorCodeConfigInvalid.WithDetail(fmt.Sprintf("Invalid trust policy [%s]: key %s with version %s not found in key management provider %s", tp.config.Name, keyConfig.Name, keyConfig.Version, keyConfig.Provider)) } keyMap[PKKey{Provider: keyConfig.Provider, Name: keyConfig.Name, Version: keyConfig.Version}] = pubKey } else { @@ -188,12 +188,12 @@ func (tp *trustPolicy) GetCosignOpts(ctx context.Context) (cosign.CheckOpts, err // create the rekor client cosignOpts.RekorClient, err = rekor.NewClient(tp.config.RekorURL) if err != nil { - return cosignOpts, fmt.Errorf("failed to create Rekor client from URL %s: %w", tp.config.RekorURL, err) + return cosignOpts, re.ErrorCodeConfigInvalid.WithDetail(fmt.Errorf("Failed to create Rekor client from URL %s", tp.config.RekorURL)).WithRemediation("Ensure that the Rekor URL is valid.").WithError(err) } // Fetches the Rekor public keys from the Rekor server cosignOpts.RekorPubKeys, err = cosign.GetRekorPubs(ctx) if err != nil { - return cosignOpts, fmt.Errorf("failed to fetch Rekor public keys: %w", 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, fmt.Errorf("failed to get fulcio roots: %w", err) + 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, fmt.Errorf("failed to fetch certificate transparency log public keys: %w", err) + 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, fmt.Errorf("failed to get fulcio intermediate certificates: %w", err) + 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{ @@ -234,42 +234,42 @@ func (tp *trustPolicy) GetCosignOpts(ctx context.Context) (cosign.CheckOpts, err // validate checks if the trust policy configuration is valid // returns an error if the configuration is invalid -func validate(config TrustPolicyConfig, verifierName string) error { +func validate(config TrustPolicyConfig) error { // check if the trust policy version is supported if !slices.Contains(SupportedTrustPolicyConfigVersions, config.Version) { - return re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithPluginName(verifierName).WithDetail(fmt.Sprintf("trust policy %s failed: unsupported version %s", config.Name, config.Version)) + return re.ErrorCodeConfigInvalid.WithDetail(fmt.Sprintf("Invalid trust policy %s: unsupported version %s", config.Name, config.Version)).WithRemediation(fmt.Sprintf("Supported versions are: %v", SupportedTrustPolicyConfigVersions)) } if config.Name == "" { - return re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithPluginName(verifierName).WithDetail("missing trust policy name") + return re.ErrorCodeConfigInvalid.WithDetail("name parameter is required in trust policy configuration").WithRemediation("Please provide a name for the trust policy.") } if len(config.Scopes) == 0 { - return re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithPluginName(verifierName).WithDetail(fmt.Sprintf("trust policy %s failed: no scopes defined", config.Name)) + return re.ErrorCodeConfigInvalid.WithDetail(fmt.Sprintf("scopes parameter is required in trust policy configuration %s", config.Name)).WithRemediation("Please provide at least one scope for the trust policy.") } // keys or keyless must be defined if len(config.Keys) == 0 && config.Keyless == (KeylessConfig{}) { - return re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithPluginName(verifierName).WithDetail(fmt.Sprintf("trust policy %s failed: no keys defined and keyless section not configured", config.Name)) + return re.ErrorCodeConfigInvalid.WithDetail(fmt.Sprintf("keys or keyless parameter is required in trust policy configuration %s", config.Name)) } // only one of keys or keyless can be defined if len(config.Keys) > 0 && config.Keyless != (KeylessConfig{}) { - return re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithPluginName(verifierName).WithDetail(fmt.Sprintf("trust policy %s failed: both keys and keyless sections are defined", config.Name)) + return re.ErrorCodeConfigInvalid.WithDetail(fmt.Sprintf("Only one of keys or keyless parameter is required in trust policy configuration %s", config.Name)) } for _, keyConfig := range config.Keys { // check if the key is defined by file path or by key management provider if keyConfig.File == "" && keyConfig.Provider == "" { - return re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithPluginName(verifierName).WithDetail(fmt.Sprintf("trust policy %s failed: key management provider name is required when not using file path", config.Name)) + return re.ErrorCodeConfigInvalid.WithDetail(fmt.Sprintf("Invalid trust policy %s: key management provider name is required when not using file path", config.Name)) } // both file path and key management provider cannot be defined together if keyConfig.File != "" && keyConfig.Provider != "" { - return re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithPluginName(verifierName).WithDetail(fmt.Sprintf("trust policy %s failed: 'name' and 'file' cannot be configured together", config.Name)) + return re.ErrorCodeConfigInvalid.WithDetail(fmt.Sprintf("Invalid trust policy %s: 'name' and 'file' cannot be configured together", config.Name)) } // key name is required when key version is defined if keyConfig.Version != "" && keyConfig.Name == "" { - return re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithPluginName(verifierName).WithDetail(fmt.Sprintf("trust policy %s failed: key name is required when key version is defined", config.Name)) + return re.ErrorCodeConfigInvalid.WithDetail(fmt.Sprintf("Invalid trust policy %s: key name is required when key version is defined", config.Name)) } } @@ -277,19 +277,19 @@ func validate(config TrustPolicyConfig, verifierName string) error { if config.Keyless != (KeylessConfig{}) { // validate certificate identity specified if config.Keyless.CertificateIdentity == "" && config.Keyless.CertificateIdentityRegExp == "" { - return re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithPluginName(verifierName).WithDetail(fmt.Sprintf("trust policy %s failed: certificate identity or identity regex pattern is required", config.Name)) + return re.ErrorCodeConfigInvalid.WithDetail(fmt.Sprintf("Invalid trust policy %s: certificate identity or identity regex pattern is required", config.Name)) } // validate certificate OIDC issuer specified if config.Keyless.CertificateOIDCIssuer == "" && config.Keyless.CertificateOIDCIssuerRegExp == "" { - return re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithPluginName(verifierName).WithDetail(fmt.Sprintf("trust policy %s failed: certificate OIDC issuer or issuer regex pattern is required", config.Name)) + return re.ErrorCodeConfigInvalid.WithDetail(fmt.Sprintf("Invalid trust policy %s: certificate OIDC issuer or issuer regex pattern is required", config.Name)) } // validate only expression or value is specified for certificate identity if config.Keyless.CertificateIdentity != "" && config.Keyless.CertificateIdentityRegExp != "" { - return re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithPluginName(verifierName).WithDetail(fmt.Sprintf("trust policy %s failed: only one of certificate identity or identity regex pattern should be specified", config.Name)) + return re.ErrorCodeConfigInvalid.WithDetail(fmt.Sprintf("Invalid trust policy %s: only one of certificate identity or identity regex pattern should be specified", config.Name)) } // validate only expression or value is specified for certificate OIDC issuer if config.Keyless.CertificateOIDCIssuer != "" && config.Keyless.CertificateOIDCIssuerRegExp != "" { - return re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithPluginName(verifierName).WithDetail(fmt.Sprintf("trust policy %s failed: only one of certificate OIDC issuer or issuer regex pattern should be specified", config.Name)) + return re.ErrorCodeConfigInvalid.WithDetail(fmt.Sprintf("Invalid trust policy %s: only one of certificate OIDC issuer or issuer regex pattern should be specified", config.Name)) } } diff --git a/pkg/verifier/cosign/trustpolicy_test.go b/pkg/verifier/cosign/trustpolicy_test.go index ea720c50e..65dcb84da 100644 --- a/pkg/verifier/cosign/trustpolicy_test.go +++ b/pkg/verifier/cosign/trustpolicy_test.go @@ -21,6 +21,7 @@ import ( "crypto/ecdsa" "crypto/x509" "fmt" + "os" "testing" ctxUtils "github.com/ratify-project/ratify/internal/context" @@ -204,6 +205,20 @@ func TestGetKeys(t *testing.T) { }, wantErr: true, }, + { + name: "access nonexistent key from KMP", + cfg: TrustPolicyConfig{ + Name: "test", + Scopes: []string{"*"}, + Keys: []KeyConfig{ + { + Provider: "ns/kmp", + Name: "nonexistent", + }, + }, + }, + wantErr: true, + }, { name: "valid KMP", cfg: TrustPolicyConfig{ @@ -423,7 +438,7 @@ func TestValidate(t *testing.T) { for _, tt := range tc { t.Run(tt.name, func(t *testing.T) { - actual := validate(tt.policyConfig, "test-verifier") + actual := validate(tt.policyConfig) if (actual != nil) != tt.wantErr { t.Fatalf("expected %v, got %v", tt.wantErr, actual) } @@ -447,3 +462,76 @@ func TestLoadKeyFromPath(t *testing.T) { t.Fatalf("expected ecdsa.PublicKey, got %v", keyType) } } + +func TestGetCosignOpts(t *testing.T) { + testCases := []struct { + name string + tlogVerify bool + rekorURL string + rekorPubKeyEnv string + isKeyless bool + CTLogVerify bool + CTLogPubKeyEnv string + expectedErr bool + }{ + { + name: "invalid rekor url", + tlogVerify: true, + rekorURL: string([]byte{0x7f}), + expectedErr: true, + }, + { + name: "failed to get rekor public key", + tlogVerify: true, + rekorURL: "https://rekor.sigstore.dev", + rekorPubKeyEnv: "invalid", + expectedErr: true, + }, + { + name: "failed to get CT log public key", + tlogVerify: false, + isKeyless: true, + CTLogVerify: true, + CTLogPubKeyEnv: "invalid", + expectedErr: true, + }, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + if tc.rekorPubKeyEnv != "" { + val := os.Getenv("SIGSTORE_REKOR_PUBLIC_KEY") + os.Setenv("SIGSTORE_REKOR_PUBLIC_KEY", tc.rekorPubKeyEnv) + t.Cleanup(func() { + os.Setenv("SIGSTORE_REKOR_PUBLIC_KEY", val) + }) + } + + if tc.CTLogPubKeyEnv != "" { + val := os.Getenv("SIGSTORE_CT_LOG_PUBLIC_KEY_FILE") + os.Setenv("SIGSTORE_CT_LOG_PUBLIC_KEY_FILE", tc.CTLogPubKeyEnv) + t.Cleanup(func() { + os.Setenv("SIGSTORE_CT_LOG_PUBLIC_KEY_FILE", val) + }) + } + + tp := trustPolicy{ + config: TrustPolicyConfig{ + TLogVerify: &tc.tlogVerify, + RekorURL: tc.rekorURL, + Keyless: KeylessConfig{ + CTLogVerify: &tc.CTLogVerify, + }, + }, + isKeyless: tc.isKeyless, + } + _, err := tp.GetCosignOpts(context.Background()) + if tc.expectedErr { + if err == nil { + t.Fatalf("expected error, got nil") + } + } + }) + } +}