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

feat: fill ErrorReason and Remediation during verifierReport generation #1682

Merged
merged 14 commits into from
Aug 21, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 4 additions & 14 deletions pkg/executor/core/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,13 +176,8 @@ func (executor Executor) verifyReferenceForJSONPolicy(ctx context.Context, subje
verifierStartTime := time.Now()
verifyResult, err := verifier.Verify(ctx, subjectRef, referenceDesc, referrerStore)
if err != nil {
verifyResult = vr.VerifierResult{
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()}
verifierErr := errors.ErrorCodeVerifyReferenceFailure.NewError(errors.Verifier, verifier.Name(), errors.EmptyLink, err, nil, errors.HideStackTrace)
verifyResult = vr.NewVerifierResult("", verifier.Name(), verifier.Type(), "", false, &verifierErr, nil)
}

if len(verifier.GetNestedReferences()) > 0 {
Expand Down Expand Up @@ -229,13 +224,8 @@ func (executor Executor) verifyReferenceForRegoPolicy(ctx context.Context, subje
verifierStartTime := time.Now()
verifierResult, err := verifier.Verify(errCtx, subjectRef, referenceDesc, referrerStore)
if err != nil {
verifierReport = vt.VerifierResult{
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()}
verifierErr := errors.ErrorCodeVerifyReferenceFailure.NewError(errors.Verifier, verifier.Name(), errors.EmptyLink, err, nil, errors.HideStackTrace)
verifierReport = vt.CreateVerifierResult(verifier.Name(), verifier.Type(), "", false, &verifierErr)
} else {
verifierReport = vt.NewVerifierResult(verifierResult)
}
Expand Down
1 change: 1 addition & 0 deletions pkg/executor/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ type VerifyResult struct {

// NestedVerifierReport describes the results of verifying an artifact and its
// nested artifacts by available verifiers.
// Note: NestedVerifierReport is used for verification results in v1.
type NestedVerifierReport struct {
Subject string `json:"subject"`
ReferenceDigest string `json:"referenceDigest"`
Expand Down
7 changes: 2 additions & 5 deletions pkg/policyprovider/configpolicy/configpolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,8 @@ func (enforcer PolicyEnforcer) ContinueVerifyOnFailure(_ context.Context, _ comm

// ErrorToVerifyResult converts an error to a properly formatted verify result
func (enforcer PolicyEnforcer) ErrorToVerifyResult(_ context.Context, subjectRefString string, verifyError error) types.VerifyResult {
errorReport := verifier.VerifierResult{
Subject: subjectRefString,
IsSuccess: false,
Message: fmt.Sprintf("verification failed: %v", verifyError),
}
verifierErr := re.ErrorCodeVerifyReferenceFailure.WithDetail(fmt.Sprintf("failed to verify artifact: %s", subjectRefString)).WithError(verifyError)
errorReport := verifier.NewVerifierResult(subjectRefString, "", "", "", false, &verifierErr, nil)
var reports []interface{}
reports = append(reports, errorReport)
return types.VerifyResult{IsSuccess: false, VerifierReports: reports}
Expand Down
17 changes: 0 additions & 17 deletions pkg/verifier/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,23 +23,6 @@ import (
"github.com/ratify-project/ratify/pkg/referrerstore"
)

// VerifierResult describes the result of verifying a reference manifest for a subject
type VerifierResult struct { //nolint:revive // ignore linter to have unique type name
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"`
ReferenceDigest string `json:"referenceDigest,omitempty"`
Remediation string `json:"remediation,omitempty"`
}

// ReferenceVerifier is an interface that defines methods to verify a reference
// for a subject by a verifier.
type ReferenceVerifier interface {
Expand Down
55 changes: 28 additions & 27 deletions pkg/verifier/cosign/cosign.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,15 +284,15 @@ func (v *cosignVerifier) verifyInternal(ctx context.Context, subjectReference co
}

if hasValidSignature {
return verifier.VerifierResult{
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
return verifier.NewVerifierResult(
"",
v.name,
v.verifierType,
"Verification success. Valid signatures found. Please refer to extensions field for verifications performed.",
true,
nil,
Extension{SignatureExtension: sigExtensions, TrustPolicy: trustPolicy.GetName()},
), nil
}

errorResult := errorToVerifyResult(v.name, v.verifierType, fmt.Errorf("no valid signatures found"))
Expand Down Expand Up @@ -397,15 +397,15 @@ func (v *cosignVerifier) verifyLegacy(ctx context.Context, subjectReference comm
}

if len(signatures) > 0 {
return verifier.VerifierResult{
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
return verifier.NewVerifierResult(
"",
v.name,
v.verifierType,
"Verification success. Valid signatures found",
true,
nil,
LegacyExtension{SignatureExtension: sigExtensions},
), nil
}

errorResult := errorToVerifyResult(v.name, v.verifierType, fmt.Errorf("no valid signatures found"))
Expand Down Expand Up @@ -485,15 +485,16 @@ 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,
VerifierName: name,
VerifierType: verifierType,
Message: "Verification failed",
ErrorReason: err.Error(),
}
verifierErr := re.ErrorCodeVerifyReferenceFailure.WithDetail("Verification failed").WithError(err)
return verifier.NewVerifierResult(
"",
name,
verifierType,
"",
false,
&verifierErr,
nil,
)
}

// decodeASN1Signature decodes the ASN.1 signature to raw signature bytes
Expand Down
3 changes: 3 additions & 0 deletions pkg/verifier/cosign/cosign_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,9 @@ func TestErrorToVerifyResult(t *testing.T) {
if verifierResult.Message != "Verification failed" {
t.Errorf("errorToVerifyResult() = %v, want %v", verifierResult.Message, "Verification failed")
}
if verifierResult.ErrorReason != "test error" {
t.Errorf("errorToVerifyResult() = %v, want %v", verifierResult.ErrorReason, "test error")
}
}

// TestDecodeASN1Signature tests the decodeASN1Signature function
Expand Down
10 changes: 1 addition & 9 deletions pkg/verifier/notation/notation.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,15 +174,7 @@ func (v *notationPluginVerifier) Verify(ctx context.Context,
extensions["SN"] = cert.Subject.String()
}

return verifier.VerifierResult{
Name: v.name,
Type: v.verifierType,
VerifierName: v.name,
VerifierType: v.verifierType,
IsSuccess: true,
Message: "Signature verification success",
Extensions: extensions,
}, nil
return verifier.NewVerifierResult("", v.name, v.verifierType, "Signature verification success", true, nil, extensions), nil
}

func getVerifierService(conf *NotationPluginVerifierConfig, pluginDirectory string) (notation.Verifier, error) {
Expand Down
62 changes: 62 additions & 0 deletions pkg/verifier/result.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
Copyright The Ratify Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package verifier

import "github.com/ratify-project/ratify/errors"

// VerifierResult describes the result of verifying a reference manifest for a subject.
// Note: This struct is used to represent the result of verification in v0.
type VerifierResult struct { //nolint:revive // ignore linter to have unique type name
Subject string `json:"subject,omitempty"`
IsSuccess bool `json:"isSuccess"`
// Name will be deprecated in v2, tracking issue: https://github.com/ratify-project/ratify/issues/1707
Name string `json:"name,omitempty"`
VerifierName string `json:"verifierName,omitempty"`
// Type will be deprecated in v2, tracking issue: https://github.com/ratify-project/ratify/issues/1707
Type string `json:"type,omitempty"`
VerifierType string `json:"verifierType,omitempty"`
ReferenceDigest string `json:"referenceDigest,omitempty"`
ArtifactType string `json:"artifactType,omitempty"`
Message string `json:"message,omitempty"`
ErrorReason string `json:"errorReason,omitempty"`
Remediation string `json:"remediation,omitempty"`
Extensions interface{} `json:"extensions,omitempty"`
NestedResults []VerifierResult `json:"nestedResults,omitempty"`
}

// NewVerifierResult creates a new VerifierResult object with the given parameters.
func NewVerifierResult(subject, verifierName, verifierType, message string, isSuccess bool, err *errors.Error, extensions interface{}) VerifierResult {
var errorReason, remediation string
if err != nil {
if err.GetDetail() != "" {
message = err.GetDetail()
}
errorReason = err.GetErrorReason()
remediation = err.GetRemediation()
}
return VerifierResult{
Subject: subject,
IsSuccess: isSuccess,
Name: verifierName,
Type: verifierType,
VerifierName: verifierName,
VerifierType: verifierType,
Message: message,
ErrorReason: errorReason,
Remediation: remediation,
Extensions: extensions,
}
}
83 changes: 83 additions & 0 deletions pkg/verifier/result_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/*
Copyright The Ratify Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package verifier

import (
"fmt"
"testing"

"github.com/ratify-project/ratify/errors"
)

const (
testMsg1 = "test message 1"
testMsg2 = "test message 2"
testErrReason = "test error reason"
testRemediation = "test remediation"
)

func TestNewVerifierResult(t *testing.T) {
tests := []struct {
name string
message string
err errors.Error
expectedMsg string
expectedErrReason string
expectedRemediation string
}{
{
name: "nil error",
message: testMsg1,
err: errors.Error{},
expectedMsg: testMsg1,
},
{
name: "error without detail",
message: testMsg1,
err: errors.ErrorCodeUnknown.WithError(fmt.Errorf(testErrReason)).WithRemediation(testRemediation),
expectedMsg: testMsg1,
expectedErrReason: testErrReason,
expectedRemediation: testRemediation,
},
{
name: "error with detail",
message: testMsg1,
err: errors.ErrorCodeUnknown.WithError(fmt.Errorf(testErrReason)).WithRemediation(testRemediation).WithDetail(testMsg2),
expectedMsg: testMsg2,
expectedErrReason: testErrReason,
expectedRemediation: testRemediation,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := &tt.err
if tt.err == (errors.Error{}) {
err = nil
}

result := NewVerifierResult("", "", "", tt.message, false, err, nil)
if result.Message != tt.expectedMsg {
t.Errorf("expected message %s, got %s", tt.expectedMsg, result.Message)
}
if result.ErrorReason != tt.expectedErrReason {
t.Errorf("expected error reason %s, got %s", tt.expectedErrReason, result.ErrorReason)
}
if result.Remediation != tt.expectedRemediation {
t.Errorf("expected remediation %s, got %s", tt.expectedRemediation, result.Remediation)
}
})
}
}
40 changes: 35 additions & 5 deletions pkg/verifier/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"encoding/json"
"io"

"github.com/ratify-project/ratify/errors"
"github.com/ratify-project/ratify/pkg/verifier"
)

Expand Down Expand Up @@ -47,11 +48,14 @@ const (

// VerifierResult describes the verification result returned from the verifier plugin
type VerifierResult struct {
IsSuccess bool `json:"isSuccess"`
Message string `json:"message"`
ErrorReason string `json:"errorReason,omitempty"`
Name string `json:"name"`
VerifierName string `json:"verifierName,omitempty"`
IsSuccess bool `json:"isSuccess"`
Message string `json:"message"`
ErrorReason string `json:"errorReason,omitempty"`
Remediation string `json:"remediation,omitempty"`
// Name will be deprecated in v2, tracking issue: https://github.com/ratify-project/ratify/issues/1707
Name string `json:"name"`
VerifierName string `json:"verifierName,omitempty"`
// Type will be deprecated in v2, tracking issue: https://github.com/ratify-project/ratify/issues/1707
Type string `json:"type,omitempty"`
VerifierType string `json:"verifierType,omitempty"`
Extensions interface{} `json:"extensions"`
Expand Down Expand Up @@ -79,6 +83,30 @@ func WriteVerifyResultResult(result *verifier.VerifierResult, w io.Writer) error
return json.NewEncoder(w).Encode(result)
}

// CreateVerifierResult creates a new verifier result object from given input.
func CreateVerifierResult(verifierName, verifierType, message string, isSuccess bool, err *errors.Error) VerifierResult {
akashsinghal marked this conversation as resolved.
Show resolved Hide resolved
var errorReason string
var remediation string
if err != nil {
if err.GetDetail() != "" {
message = err.GetDetail()
}
errorReason = err.GetErrorReason()
remediation = err.GetRemediation()
}

return VerifierResult{
IsSuccess: isSuccess,
Name: verifierName,
Type: verifierType,
VerifierName: verifierName,
VerifierType: verifierType,
Message: message,
ErrorReason: errorReason,
Remediation: remediation,
}
}

// NewVerifierResult creates a new verifier result object from the given
// verifier.VerifierResult.
func NewVerifierResult(result verifier.VerifierResult) VerifierResult {
Expand All @@ -90,5 +118,7 @@ func NewVerifierResult(result verifier.VerifierResult) VerifierResult {
VerifierName: result.VerifierName,
VerifierType: result.VerifierType,
Extensions: result.Extensions,
ErrorReason: result.ErrorReason,
Remediation: result.Remediation,
}
}
Loading
Loading