Skip to content

Commit

Permalink
chore: address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
binbin-li committed Sep 9, 2024
1 parent 8893dac commit 51d56e5
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 27 deletions.
4 changes: 2 additions & 2 deletions pkg/referrerstore/oras/cosign.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/referrerstore/oras/oras.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Check warning on line 211 in pkg/referrerstore/oras/oras.go

View check run for this annotation

Codecov / codecov/patch

pkg/referrerstore/oras/oras.go#L211

Added line #L211 was not covered by tests
}

// resolve subject descriptor if not provided
Expand Down
12 changes: 6 additions & 6 deletions pkg/verifier/cosign/cosign.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
}
}

Expand Down
24 changes: 12 additions & 12 deletions pkg/verifier/cosign/cosign_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -594,15 +594,15 @@ 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",
},
{
name: "manifest fetch error",
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",
},
{
Expand All @@ -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",
Expand All @@ -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",
},
{
Expand All @@ -657,7 +657,7 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry
},
},
},
expectedResultMessagePrefix: "Cosign signature verification failed",
expectedResultMessagePrefix: "Failed to validate the Cosign signature",
expectedErrorReason: "blob not found",
},
{
Expand Down Expand Up @@ -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]",
},
{
Expand Down Expand Up @@ -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",
},
{
Expand Down Expand Up @@ -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]",
},
{
Expand Down Expand Up @@ -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",
},
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/verifier/cosign/trustpolicies.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions pkg/verifier/cosign/trustpolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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")

Check warning on line 206 in pkg/verifier/cosign/trustpolicy.go

View check run for this annotation

Codecov / codecov/patch

pkg/verifier/cosign/trustpolicy.go#L206

Added line #L206 was not covered by tests
}
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")

Check warning on line 219 in pkg/verifier/cosign/trustpolicy.go

View check run for this annotation

Codecov / codecov/patch

pkg/verifier/cosign/trustpolicy.go#L219

Added line #L219 was not covered by tests
}
// Set the certificate identity and issuer for keyless verification
cosignOpts.Identities = []cosign.Identity{
Expand Down

0 comments on commit 51d56e5

Please sign in to comment.