Skip to content

Commit

Permalink
Merge pull request #1671 from binbin-li/verification-response
Browse files Browse the repository at this point in the history
feat: Add more fields to verification response
  • Loading branch information
binbin-li authored Aug 6, 2024
2 parents 300401c + e757310 commit 294a715
Show file tree
Hide file tree
Showing 8 changed files with 113 additions and 71 deletions.
6 changes: 4 additions & 2 deletions httpserver/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions httpserver/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,12 @@ func TestFromVerifyResult(t *testing.T) {
{
name: "Rego policy",
policyType: pt.RegoPolicy,
expectedVersion: "1.0.0",
expectedVersion: "1.1.0",
},
{
name: "Config policy",
policyType: pt.ConfigPolicy,
expectedVersion: "0.1.0",
expectedVersion: "0.2.0",
},
}

Expand Down
23 changes: 14 additions & 9 deletions pkg/executor/core/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,19 +175,22 @@ func (executor Executor) verifyReferenceForJSONPolicy(ctx context.Context, subje
if verifier.CanVerify(ctx, referenceDesc) {
verifierStartTime := time.Now()
verifyResult, err := verifier.Verify(ctx, subjectRef, referenceDesc, referrerStore)
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 {
executor.addNestedVerifierResult(ctx, referenceDesc, subjectRef, &verifyResult)
}

verifyResult.Subject = subjectRef.String()
verifyResult.ReferenceDigest = referenceDesc.Digest.String()
verifyResult.ArtifactType = referenceDesc.ArtifactType
verifyResults = append(verifyResults, verifyResult)
isSuccess = verifyResult.IsSuccess
Expand Down Expand Up @@ -227,10 +230,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)
}
Expand Down
21 changes: 13 additions & 8 deletions pkg/verifier/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,19 @@ import (

// 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"`
Type string `json:"type,omitempty"`
Message string `json:"message,omitempty"`
Extensions interface{} `json:"extensions,omitempty"`
NestedResults []VerifierResult `json:"nestedResults,omitempty"`
ArtifactType string `json:"artifactType,omitempty"`
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
Expand Down
35 changes: 21 additions & 14 deletions pkg/verifier/cosign/cosign.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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(),
}
}

Expand Down
47 changes: 30 additions & 17 deletions pkg/verifier/cosign/cosign_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}

Expand Down Expand Up @@ -562,21 +562,24 @@ mmBwUAwwW0Uc+Nt3bDOCiB1nUsICv1ry
cosignOpts cosign.CheckOpts
store *mocks.MemoryTestStore
expectedResultMessagePrefix string
expectedErrorReason string
defaultCosignOpts bool
}{
{
name: "get keys error",
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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
},
}

Expand Down Expand Up @@ -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)
}
})
}
Expand Down
12 changes: 7 additions & 5 deletions pkg/verifier/notation/notation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
Loading

0 comments on commit 294a715

Please sign in to comment.