From f560c1cee8a386cbe3b910593f14648964d3d3de Mon Sep 17 00:00:00 2001 From: Binbin Li Date: Thu, 25 Jul 2024 15:47:01 +0000 Subject: [PATCH] feat: add verifierName, verifierType and errorReason fields to verifierReport --- httpserver/types.go | 6 ++-- pkg/executor/core/executor.go | 20 ++++++++----- pkg/verifier/api.go | 3 ++ pkg/verifier/cosign/cosign.go | 35 +++++++++++++--------- pkg/verifier/cosign/cosign_test.go | 47 +++++++++++++++++++----------- pkg/verifier/notation/notation.go | 12 ++++---- pkg/verifier/types/types.go | 36 ++++++++++++++--------- 7 files changed, 99 insertions(+), 60 deletions(-) diff --git a/httpserver/types.go b/httpserver/types.go index f307bc55d9..8e3655cb55 100644 --- a/httpserver/types.go +++ b/httpserver/types.go @@ -22,9 +22,11 @@ import ( const ( VerificationResultVersion = "0.1.0" + ResultVersion0_2_0 = "0.2.0" // Starting from this version, the verification result can be // evaluated by Ratify embedded OPA engine. ResultVersionSupportingRego = "1.0.0" + ResultVersion1_1_0 = "1.1.0" ) type VerificationResponse struct { @@ -34,9 +36,9 @@ type VerificationResponse struct { } func fromVerifyResult(res types.VerifyResult, policyType string) VerificationResponse { - version := VerificationResultVersion + version := ResultVersion0_2_0 if policyType == pt.RegoPolicy { - version = ResultVersionSupportingRego + version = ResultVersion1_1_0 } return VerificationResponse{ Version: version, diff --git a/pkg/executor/core/executor.go b/pkg/executor/core/executor.go index ef9d27210a..4e37bd48d3 100644 --- a/pkg/executor/core/executor.go +++ b/pkg/executor/core/executor.go @@ -178,10 +178,12 @@ func (executor Executor) verifyReferenceForJSONPolicy(ctx context.Context, subje verifyResult.Subject = subjectRef.String() if err != nil { verifyResult = vr.VerifierResult{ - IsSuccess: false, - Name: verifier.Name(), - Type: verifier.Type(), - Message: errors.ErrorCodeVerifyReferenceFailure.NewError(errors.Verifier, verifier.Name(), errors.EmptyLink, err, nil, errors.HideStackTrace).Error()} + IsSuccess: false, + Name: verifier.Name(), + Type: verifier.Type(), + VerifierName: verifier.Name(), + VerifierType: verifier.Type(), + Message: errors.ErrorCodeVerifyReferenceFailure.NewError(errors.Verifier, verifier.Name(), errors.EmptyLink, err, nil, errors.HideStackTrace).Error()} } if len(verifier.GetNestedReferences()) > 0 { @@ -227,10 +229,12 @@ func (executor Executor) verifyReferenceForRegoPolicy(ctx context.Context, subje verifierResult, err := verifier.Verify(errCtx, subjectRef, referenceDesc, referrerStore) if err != nil { verifierReport = vt.VerifierResult{ - IsSuccess: false, - Name: verifier.Name(), - Type: verifier.Type(), - Message: errors.ErrorCodeVerifyReferenceFailure.NewError(errors.Verifier, verifier.Name(), errors.EmptyLink, err, nil, errors.HideStackTrace).Error()} + IsSuccess: false, + Name: verifier.Name(), // Deprecating Name in next release, reference to VerifierName instead. + Type: verifier.Type(), // Deprecating Type in next release, reference to VerifierType instead. + VerifierName: verifier.Name(), + VerifierType: verifier.Type(), + Message: errors.ErrorCodeVerifyReferenceFailure.NewError(errors.Verifier, verifier.Name(), errors.EmptyLink, err, nil, errors.HideStackTrace).Error()} } else { verifierReport = vt.NewVerifierResult(verifierResult) } diff --git a/pkg/verifier/api.go b/pkg/verifier/api.go index dcc5496620..53c14e150c 100644 --- a/pkg/verifier/api.go +++ b/pkg/verifier/api.go @@ -28,8 +28,11 @@ type VerifierResult struct { //nolint:revive // ignore linter to have unique typ Subject string `json:"subject,omitempty"` IsSuccess bool `json:"isSuccess"` Name string `json:"name,omitempty"` + VerifierName string `json:"verifierName,omitempty"` Type string `json:"type,omitempty"` + VerifierType string `json:"verifierType,omitempty"` Message string `json:"message,omitempty"` + ErrorReason string `json:"errorReason,omitempty"` Extensions interface{} `json:"extensions,omitempty"` NestedResults []VerifierResult `json:"nestedResults,omitempty"` ArtifactType string `json:"artifactType,omitempty"` diff --git a/pkg/verifier/cosign/cosign.go b/pkg/verifier/cosign/cosign.go index 40a67fbfbf..a5a3bc4c41 100644 --- a/pkg/verifier/cosign/cosign.go +++ b/pkg/verifier/cosign/cosign.go @@ -285,11 +285,13 @@ func (v *cosignVerifier) verifyInternal(ctx context.Context, subjectReference co if hasValidSignature { return verifier.VerifierResult{ - Name: v.name, - Type: v.verifierType, - IsSuccess: true, - Message: "cosign verification success. valid signatures found. please refer to extensions field for verifications performed.", - Extensions: Extension{SignatureExtension: sigExtensions, TrustPolicy: trustPolicy.GetName()}, + Name: v.name, + Type: v.verifierType, + VerifierName: v.name, + VerifierType: v.verifierType, + IsSuccess: true, + Message: "Verification success. Valid signatures found. Please refer to extensions field for verifications performed.", + Extensions: Extension{SignatureExtension: sigExtensions, TrustPolicy: trustPolicy.GetName()}, }, nil } @@ -396,11 +398,13 @@ func (v *cosignVerifier) verifyLegacy(ctx context.Context, subjectReference comm if len(signatures) > 0 { return verifier.VerifierResult{ - Name: v.name, - Type: v.verifierType, - IsSuccess: true, - Message: "cosign verification success. valid signatures found", - Extensions: LegacyExtension{SignatureExtension: sigExtensions}, + Name: v.name, + Type: v.verifierType, + VerifierName: v.name, + VerifierType: v.verifierType, + IsSuccess: true, + Message: "Verification success. Valid signatures found", + Extensions: LegacyExtension{SignatureExtension: sigExtensions}, }, nil } @@ -482,10 +486,13 @@ 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 { return verifier.VerifierResult{ - IsSuccess: false, - Name: name, - Type: verifierType, - Message: errors.Wrap(err, "cosign verification failed").Error(), + IsSuccess: false, + Name: name, + Type: verifierType, + VerifierName: name, + VerifierType: verifierType, + Message: "Verification failed", + ErrorReason: err.Error(), } } diff --git a/pkg/verifier/cosign/cosign_test.go b/pkg/verifier/cosign/cosign_test.go index 4ceb03a96e..fde624a3e9 100644 --- a/pkg/verifier/cosign/cosign_test.go +++ b/pkg/verifier/cosign/cosign_test.go @@ -407,8 +407,8 @@ func TestErrorToVerifyResult(t *testing.T) { if verifierResult.Type != "cosign" { t.Errorf("errorToVerifyResult() = %v, want %v", verifierResult.Type, "cosign") } - if verifierResult.Message != "cosign verification failed: test error" { - t.Errorf("errorToVerifyResult() = %v, want %v", verifierResult.Message, "cosign verification failed: test error") + if verifierResult.Message != "Verification failed" { + t.Errorf("errorToVerifyResult() = %v, want %v", verifierResult.Message, "Verification failed") } } @@ -562,6 +562,7 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry cosignOpts cosign.CheckOpts store *mocks.MemoryTestStore expectedResultMessagePrefix string + expectedErrorReason string defaultCosignOpts bool }{ { @@ -569,14 +570,16 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry keys: map[PKKey]keymanagementprovider.PublicKey{}, getKeysError: true, store: &mocks.MemoryTestStore{}, - expectedResultMessagePrefix: "cosign verification failed: error", + expectedResultMessagePrefix: "Verification failed", + expectedErrorReason: "error", }, { name: "manifest fetch error", keys: map[PKKey]keymanagementprovider.PublicKey{}, getKeysError: false, store: &mocks.MemoryTestStore{}, - expectedResultMessagePrefix: "cosign verification failed: failed to get reference manifest", + expectedResultMessagePrefix: "Verification failed", + expectedErrorReason: "failed to get reference manifest: manifest not found", }, { name: "incorrect reference manifest media type error", @@ -589,7 +592,8 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry }, }, }, - expectedResultMessagePrefix: "cosign verification failed: reference manifest is not an image", + expectedResultMessagePrefix: "Verification failed", + expectedErrorReason: "reference manifest is not an image", }, { name: "failed subject descriptor fetch", @@ -602,7 +606,8 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry }, }, }, - expectedResultMessagePrefix: "cosign verification failed: failed to create subject hash", + expectedResultMessagePrefix: "Verification failed", + expectedErrorReason: "failed to create subject hash: subject not found for sha256:5678", }, { name: "failed to fetch blob", @@ -628,7 +633,8 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry }, }, }, - expectedResultMessagePrefix: "cosign verification failed: failed to get blob content", + expectedResultMessagePrefix: "Verification failed", + expectedErrorReason: "failed to get blob content: blob not found", }, { name: "invalid key type for AKV", @@ -659,7 +665,8 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry blobDigest: validSignatureBlob, }, }, - expectedResultMessagePrefix: "cosign verification failed: failed to verify with keys: failed to process AKV signature: unsupported public key type", + expectedResultMessagePrefix: "Verification failed", + expectedErrorReason: "failed to verify with keys: failed to process AKV signature: unsupported public key type: *ecdh.PublicKey", }, { name: "invalid RSA key size for AKV", @@ -690,7 +697,8 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry blobDigest: validSignatureBlob, }, }, - expectedResultMessagePrefix: "cosign verification failed: failed to verify with keys: failed to process AKV signature: RSA key check: unsupported key size", + expectedResultMessagePrefix: "Verification failed", + expectedErrorReason: "failed to verify with keys: failed to process AKV signature: RSA key check: unsupported key size: 128", }, { name: "invalid ECDSA curve type for AKV", @@ -721,7 +729,8 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry blobDigest: validSignatureBlob, }, }, - expectedResultMessagePrefix: "cosign verification failed: failed to verify with keys: failed to process AKV signature: ECDSA key check: unsupported key curve", + expectedResultMessagePrefix: "Verification failed", + expectedErrorReason: "failed to verify with keys: failed to process AKV signature: ECDSA key check: unsupported key curve: P-224", }, { name: "valid hash: 256 keysize: 2048 RSA key AKV", @@ -761,7 +770,7 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry "sha256:6e1ffef2ba058cda5d1aa7ed792cb1e63b4207d8195a469bee1b5fc662cd9b70": []byte(`{"critical":{"identity":{"docker-reference":"artifactstest.azurecr.io/4-15-24/cosign/hello-world"},"image":{"docker-manifest-digest":"sha256:d37ada95d47ad12224c205a938129df7a3e52345828b4fa27b03a98825d1e2e7"},"type":"cosign container image signature"},"optional":null}`), }, }, - expectedResultMessagePrefix: "cosign verification success", + expectedResultMessagePrefix: "Verification success", }, { name: "valid hash: 256 keysize: 3072 RSA key", @@ -801,7 +810,7 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry "sha256:6e1ffef2ba058cda5d1aa7ed792cb1e63b4207d8195a469bee1b5fc662cd9b70": []byte(`{"critical":{"identity":{"docker-reference":"artifactstest.azurecr.io/4-15-24/cosign/hello-world"},"image":{"docker-manifest-digest":"sha256:d37ada95d47ad12224c205a938129df7a3e52345828b4fa27b03a98825d1e2e7"},"type":"cosign container image signature"},"optional":null}`), }, }, - expectedResultMessagePrefix: "cosign verification success", + expectedResultMessagePrefix: "Verification success", }, { name: "valid hash: 256 curve: P256 ECDSA key AKV", @@ -841,7 +850,7 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry "sha256:6e1ffef2ba058cda5d1aa7ed792cb1e63b4207d8195a469bee1b5fc662cd9b70": []byte(`{"critical":{"identity":{"docker-reference":"artifactstest.azurecr.io/4-15-24/cosign/hello-world"},"image":{"docker-manifest-digest":"sha256:d37ada95d47ad12224c205a938129df7a3e52345828b4fa27b03a98825d1e2e7"},"type":"cosign container image signature"},"optional":null}`), }, }, - expectedResultMessagePrefix: "cosign verification success", + expectedResultMessagePrefix: "Verification success", }, { name: "valid hash: 256 curve: P384 ECDSA key", @@ -881,7 +890,7 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry "sha256:6e1ffef2ba058cda5d1aa7ed792cb1e63b4207d8195a469bee1b5fc662cd9b70": []byte(`{"critical":{"identity":{"docker-reference":"artifactstest.azurecr.io/4-15-24/cosign/hello-world"},"image":{"docker-manifest-digest":"sha256:d37ada95d47ad12224c205a938129df7a3e52345828b4fa27b03a98825d1e2e7"},"type":"cosign container image signature"},"optional":null}`), }, }, - expectedResultMessagePrefix: "cosign verification success", + expectedResultMessagePrefix: "Verification success", }, { name: "successful keyless verification", @@ -917,7 +926,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 verification success", + expectedResultMessagePrefix: "Verification success", }, { name: "failed keyless verification", @@ -953,7 +962,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: "cosign verification failed", + expectedResultMessagePrefix: "Verification failed", + expectedErrorReason: "failed to parse static signature opts: failed to unmarshal bundle from blob payload: illegal base64 data at input byte 91", }, } @@ -991,7 +1001,10 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry } result, _ := cosignVerifier.Verify(context.Background(), subjectRef, refDescriptor, tt.store) if !strings.HasPrefix(result.Message, tt.expectedResultMessagePrefix) { - t.Errorf("Verify() = %v, want %v", result.Message, tt.expectedResultMessagePrefix) + t.Errorf("result.Message = %v, want %v", result.Message, tt.expectedResultMessagePrefix) + } + if result.ErrorReason != tt.expectedErrorReason { + t.Fatalf("expected error reason: %s, but got: %s", tt.expectedErrorReason, result.ErrorReason) } }) } diff --git a/pkg/verifier/notation/notation.go b/pkg/verifier/notation/notation.go index 6fb0ccf0f6..1a43493470 100644 --- a/pkg/verifier/notation/notation.go +++ b/pkg/verifier/notation/notation.go @@ -175,11 +175,13 @@ func (v *notationPluginVerifier) Verify(ctx context.Context, } return verifier.VerifierResult{ - Name: v.name, - Type: v.verifierType, - IsSuccess: true, - Message: "signature verification success", - Extensions: extensions, + Name: v.name, + Type: v.verifierType, + VerifierName: v.name, + VerifierType: v.verifierType, + IsSuccess: true, + Message: "Signature verification success", + Extensions: extensions, }, nil } diff --git a/pkg/verifier/types/types.go b/pkg/verifier/types/types.go index 1c12dc0869..2eed0a669e 100644 --- a/pkg/verifier/types/types.go +++ b/pkg/verifier/types/types.go @@ -47,11 +47,14 @@ const ( // VerifierResult describes the verification result returned from the verifier plugin type VerifierResult struct { - IsSuccess bool `json:"isSuccess"` - Message string `json:"message"` - Name string `json:"name"` - Type string `json:"type,omitempty"` - Extensions interface{} `json:"extensions"` + IsSuccess bool `json:"isSuccess"` + Message string `json:"message"` + ErrorReason string `json:"errorReason,omitempty"` + Name string `json:"name"` + VerifierName string `json:"verifierName,omitempty"` + Type string `json:"type,omitempty"` + VerifierType string `json:"verifierType,omitempty"` + Extensions interface{} `json:"extensions"` } // GetVerifierResult encodes the given JSON data into verify result object @@ -61,11 +64,13 @@ func GetVerifierResult(result []byte) (*verifier.VerifierResult, error) { return nil, err } return &verifier.VerifierResult{ - IsSuccess: vResult.IsSuccess, - Message: vResult.Message, - Name: vResult.Name, - Type: vResult.Type, - Extensions: vResult.Extensions, + IsSuccess: vResult.IsSuccess, + Message: vResult.Message, + Name: vResult.Name, + Type: vResult.Type, + VerifierName: vResult.Name, + VerifierType: vResult.Type, + Extensions: vResult.Extensions, }, nil } @@ -78,9 +83,12 @@ func WriteVerifyResultResult(result *verifier.VerifierResult, w io.Writer) error // verifier.VerifierResult. func NewVerifierResult(result verifier.VerifierResult) VerifierResult { return VerifierResult{ - IsSuccess: result.IsSuccess, - Message: result.Message, - Name: result.Name, - Extensions: result.Extensions, + IsSuccess: result.IsSuccess, + Message: result.Message, + Name: result.Name, + Type: result.Type, + VerifierName: result.VerifierName, + VerifierType: result.VerifierType, + Extensions: result.Extensions, } }