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: Enable multi verifier name #1187

Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
ae7da73
feat: enable showing verifier name
junczhu Nov 23, 2023
6e96827
feat: enable showing verifier name 2
junczhu Nov 23, 2023
2387b5b
feat: enable showing verifier name related ut
junczhu Nov 23, 2023
dcee0b2
feat: enable showing verifier name related ut2
junczhu Nov 23, 2023
e46748c
feat: enable showing verifier name related ut3
junczhu Nov 23, 2023
f7bc7ea
feat: enable showing verifier name related ut4
junczhu Nov 23, 2023
f52f0ab
feat: enable showing verifier name related ut5
junczhu Nov 23, 2023
8d81fae
feat: enable showing verifier name related ut6
junczhu Nov 23, 2023
fdbb175
feat: enable showing verifier name related ut7
junczhu Nov 23, 2023
a68ecbb
feat: enable showing verifier name related ut8
junczhu Nov 29, 2023
da1b552
feat:set verifier type as optional property, added to notation vResult
junczhu Nov 29, 2023
e8aea98
chore: create func verifierCreateConfig, added err catcher
junczhu Nov 29, 2023
004f8e4
fix: update verifier dynamic cli-test
junczhu Nov 29, 2023
9e15057
fix: update verifier creation llog
junczhu Nov 29, 2023
20a95c5
fix: update verifier creation log2
junczhu Nov 29, 2023
a549c06
fix: update verifier creation log3
junczhu Nov 29, 2023
218606f
Merge branch 'main' into enable-multi-verifier-name
junczhu Dec 1, 2023
6efa929
fix: update verifier plugin log for e2e test
junczhu Dec 1, 2023
78bfe26
fix: typo; CT for notation plugin
junczhu Dec 4, 2023
4a8e33c
test: e2e test for verifier with type and multi verifier
junczhu Dec 4, 2023
e2f5b41
feat: resolve comments; fix: e2e test
junczhu Dec 4, 2023
0dfeda7
fix: go-lint
junczhu Dec 4, 2023
62ae222
fix: golangci-lint
junczhu Dec 4, 2023
2e0bf7f
fix: rename variable
junczhu Dec 6, 2023
54304e4
Merge branch 'main' into enable-multi-verifier-name
junczhu Dec 6, 2023
67d93c9
test: update e2e test cases
junczhu Dec 6, 2023
8609a62
Merge branch 'enable-multi-verifier-name' of https://github.com/ZAFT-…
junczhu Dec 6, 2023
b12019d
test: update e2e test cases and reports
junczhu Dec 6, 2023
bdb93e2
test: update e2e test cases and reports2
junczhu Dec 6, 2023
053802c
test: update e2e test cases and reports3
junczhu Dec 6, 2023
41a1e92
test: update e2e test cases and reports4
junczhu Dec 6, 2023
8b138d0
test: update e2e test cases and reports5
junczhu Dec 6, 2023
46b460f
test: update e2e test cases and reports6
junczhu Dec 6, 2023
f34519c
test: update e2e test cases and reports7
junczhu Dec 6, 2023
9042ff8
test: update e2e test cases and reports8
junczhu Dec 6, 2023
516730d
test: update e2e test cases and reports9
junczhu Dec 6, 2023
869b45d
test: update e2e test cases and reports0
junczhu Dec 6, 2023
46a35c5
test: update e2e test cases and reports10
junczhu Dec 6, 2023
b06bd62
test: update e2e test cases and reports11
junczhu Dec 6, 2023
92a63a0
test: update e2e test cases and reports12
junczhu Dec 6, 2023
c8fa6bf
test: update e2e test cases and reports13
junczhu Dec 6, 2023
2ff1060
chore: merge conflict
junczhu Dec 7, 2023
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
4 changes: 2 additions & 2 deletions library/notation-issuer-validation/template.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ spec:
general_violation[{"result": result}] {
subject_results := remote_data.responses[_]
subject_result := subject_results[1]
notation_results := [res | subject_result.verifierReports[i].name == "notation"; res := subject_result.verifierReports[i]]
notation_results := [res | subject_result.verifierReports[i].type == "notation"; res := subject_result.verifierReports[i]]
issuer_results := [res | notation_results[i].extensions.Issuer == input.parameters.issuer; res := notation_results[i]]
count(issuer_results) == 0
result := sprintf("Subject %s has no signatures for certificate with Issuer: %s", [subject_results[0], input.parameters.issuer])
Expand All @@ -67,7 +67,7 @@ spec:
general_violation[{"result": result}] {
subject_results := remote_data.responses[_]
subject_result := subject_results[1]
notation_results := [res | subject_result.verifierReports[i].name == "notation"; res := subject_result.verifierReports[i]]
notation_results := [res | subject_result.verifierReports[i].type == "notation"; res := subject_result.verifierReports[i]]
notation_result := notation_results[_]
notation_result.isSuccess == false
notation_result.extensions.Issuer == input.parameters.issuer
Expand Down
2 changes: 1 addition & 1 deletion library/notation-validation/template.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ spec:
general_violation[{"result": result}] {
subject_results := remote_data.responses[_]
subject_result := subject_results[1]
notation_results := [res | subject_result.verifierReports[i].name == "notation"; res := subject_result.verifierReports[i]]
notation_results := [res | subject_result.verifierReports[i].type == "notation"; res := subject_result.verifierReports[i]]
successful_result := [ notation_result | notation_result := notation_results[_]; notation_result["isSuccess"] == true]
successful_result == []
result = sprintf("signature verification failed for all signatures associated with %s", [subject_results[0]])
Expand Down
19 changes: 9 additions & 10 deletions pkg/controllers/verifier_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,28 +97,27 @@ func (r *VerifierReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c

// creates a verifier reference from CRD spec and add store to map
func verifierAddOrReplace(spec configv1beta1.VerifierSpec, objectName string, namespace string) error {
verifierConfig, err := specToVerifierConfig(spec)

verifierConfig, err := specToVerifierConfig(spec, objectName)
if err != nil {
logrus.Error(err, "unable to convert crd specification to verifier config")
return fmt.Errorf("unable to convert crd specification to verifier config, err: %w", err)
}

// verifier factory only support a single version of configuration today
// when we support multi version verifier CRD, we will also pass in the corresponding config version so factory can create different version of the object
verifierConfigVersion := "1.0.0" // TODO: move default values to defaulting webhook in the future #413
if spec.Address == "" {
spec.Address = config.GetDefaultPluginPath()
logrus.Infof("Address was empty, setting to default path: %v", spec.Address)
}
verifierReference, err := vf.CreateVerifierFromConfig(verifierConfig, verifierConfigVersion, []string{spec.Address}, namespace)

if err != nil || verifierReference == nil {
referenceVerifier, err := vf.CreateVerifierFromConfig(verifierConfig, verifierConfigVersion, []string{spec.Address}, namespace)

if err != nil || referenceVerifier == nil {
logrus.Error(err, "unable to create verifier from verifier config")
return err
}
VerifierMap[objectName] = verifierReference
logrus.Infof("verifier '%v' added to verifier map", verifierReference.Name())
VerifierMap[objectName] = referenceVerifier
logrus.Infof("verifier '%v' added to verifier map", referenceVerifier.Name())

return nil
}
Expand All @@ -129,7 +128,7 @@ func verifierRemove(objectName string) {
}

// returns a verifier reference from spec
func specToVerifierConfig(verifierSpec configv1beta1.VerifierSpec) (vc.VerifierConfig, error) {
func specToVerifierConfig(verifierSpec configv1beta1.VerifierSpec, verifierName string) (vc.VerifierConfig, error) {
verifierConfig := vc.VerifierConfig{}

if string(verifierSpec.Parameters.Raw) != "" {
Expand All @@ -138,8 +137,8 @@ func specToVerifierConfig(verifierSpec configv1beta1.VerifierSpec) (vc.VerifierC
return vc.VerifierConfig{}, err
}
}

verifierConfig[types.Name] = verifierSpec.Name
verifierConfig[types.Name] = verifierName
verifierConfig[types.Type] = verifierSpec.Name
verifierConfig[types.ArtifactTypes] = verifierSpec.ArtifactTypes
if verifierSpec.Source != nil {
verifierConfig[types.Source] = verifierSpec.Source
Expand Down
2 changes: 2 additions & 0 deletions pkg/executor/core/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ func (executor Executor) verifyReferenceForJSONPolicy(ctx context.Context, subje
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()}
}

Expand Down Expand Up @@ -225,6 +226,7 @@ func (executor Executor) verifyReferenceForRegoPolicy(ctx context.Context, subje
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()}
} else {
verifierReport = vt.NewVerifierResult(verifierResult)
Expand Down
4 changes: 4 additions & 0 deletions pkg/executor/core/executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,10 @@ type mockVerifier struct {
}

func (v *mockVerifier) Name() string {
return "verifier-mockVerifier"
}

func (v *mockVerifier) Type() string {
return "mockVerifier"
junczhu marked this conversation as resolved.
Show resolved Hide resolved
}

Expand Down
6 changes: 5 additions & 1 deletion pkg/executor/core/testtypes.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@
}

func (s *TestVerifier) Name() string {
return "test-verifier"
return "verifier-testVerifier"
}

func (s *TestVerifier) Type() string {
return "testVerifier"

Check warning on line 38 in pkg/executor/core/testtypes.go

View check run for this annotation

Codecov / codecov/patch

pkg/executor/core/testtypes.go#L37-L38

Added lines #L37 - L38 were not covered by tests
}

func (s *TestVerifier) CanVerify(_ context.Context, referenceDescriptor ocispecs.ReferenceDescriptor) bool {
Expand Down
4 changes: 4 additions & 0 deletions pkg/verifier/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ 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"`
Type string `json:"type,omitempty"`
Message string `json:"message,omitempty"`
Extensions interface{} `json:"extensions,omitempty"`
NestedResults []VerifierResult `json:"nestedResults,omitempty"`
Expand All @@ -40,6 +41,9 @@ type ReferenceVerifier interface {
// Name returns the name of the verifier
Name() string

// Type returns the type name of the verifier
Type() string

// CanVerify returns if the verifier can verify the given reference
CanVerify(ctx context.Context, referenceDescriptor ocispecs.ReferenceDescriptor) bool

Expand Down
24 changes: 15 additions & 9 deletions pkg/verifier/factory/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,20 @@
// returns a single verifier from a verifierConfig
// namespace is only applicable in K8s environment, namespace is appended to the certstore of the truststore so it is uniquely identifiable in a cluster env
func CreateVerifierFromConfig(verifierConfig config.VerifierConfig, configVersion string, pluginBinDir []string, namespace string) (verifier.ReferenceVerifier, error) {
verifierName, ok := verifierConfig[types.Name]
if !ok {
// in cli mode both `type` and `name`` are read from config, if `type` is not specified, `name` is used as `type`
var verifierTypeStr string
if value, ok := verifierConfig[types.Name]; ok {
verifierTypeStr = value.(string)
} else {
return nil, re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithDetail(fmt.Sprintf("failed to find verifier name in the verifier config with key %s", "name"))
junczhu marked this conversation as resolved.
Show resolved Hide resolved
}

verifierNameStr := fmt.Sprintf("%s", verifierName)
if strings.ContainsRune(verifierNameStr, os.PathSeparator) {
return nil, re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithDetail(fmt.Sprintf("invalid plugin name for a verifier: %s", verifierNameStr))
if value, ok := verifierConfig[types.Type]; ok {
verifierTypeStr = value.(string)
}

if strings.ContainsRune(verifierTypeStr, os.PathSeparator) {
return nil, re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithDetail(fmt.Sprintf("invalid plugin name for a verifier: %s", verifierTypeStr))

Check warning on line 68 in pkg/verifier/factory/factory.go

View check run for this annotation

Codecov / codecov/patch

pkg/verifier/factory/factory.go#L68

Added line #L68 was not covered by tests
}

// if source is specified, download the plugin
Expand All @@ -70,18 +76,18 @@
return nil, re.ErrorCodeConfigInvalid.NewError(re.Verifier, "", re.EmptyLink, err, "failed to parse plugin source", re.HideStackTrace)
}

targetPath := path.Join(pluginBinDir[0], verifierNameStr)
targetPath := path.Join(pluginBinDir[0], verifierTypeStr)
junczhu marked this conversation as resolved.
Show resolved Hide resolved
err = pluginCommon.DownloadPlugin(source, targetPath)
if err != nil {
return nil, re.ErrorCodeDownloadPluginFailure.NewError(re.Verifier, "", re.EmptyLink, err, "failed to download plugin", re.HideStackTrace)
}
logrus.Infof("downloaded verifier plugin %s from %s to %s", verifierNameStr, source.Artifact, targetPath)
logrus.Infof("downloaded verifier plugin %s from %s to %s", verifierTypeStr, source.Artifact, targetPath)
} else {
logrus.Warnf("%s was specified for verifier plugin %s, but dynamic plugins are currently disabled", types.Source, verifierNameStr)
logrus.Warnf("%s was specified for verifier plugin type %s, but dynamic plugins are currently disabled", types.Source, verifierTypeStr)
}
}

verifierFactory, ok := builtInVerifiers[verifierNameStr]
verifierFactory, ok := builtInVerifiers[verifierTypeStr]
if ok {
return verifierFactory.Create(configVersion, verifierConfig, pluginBinDir[0], namespace)
}
Expand Down
18 changes: 12 additions & 6 deletions pkg/verifier/factory/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ type TestVerifier struct {
type TestVerifierFactory struct{}

func (s *TestVerifier) Name() string {
return "test-verifier-0"
}

func (s *TestVerifier) Type() string {
return "test-verifier"
}

Expand Down Expand Up @@ -63,7 +67,8 @@ func TestCreateVerifiersFromConfig_BuiltInVerifiers_ReturnsExpected(t *testing.T
}

verifierConfig := map[string]interface{}{
"name": "test-verifier",
"name": "test-verifier-0",
"type": "test-verifier",
}
verifiersConfig := config.VerifiersConfig{
Verifiers: []config.VerifierConfig{verifierConfig},
Expand All @@ -79,8 +84,8 @@ func TestCreateVerifiersFromConfig_BuiltInVerifiers_ReturnsExpected(t *testing.T
t.Fatalf("expected to have %d verifiers, actual count %d", 1, len(verifiers))
}

if verifiers[0].Name() != "test-verifier" {
t.Fatalf("expected to create test verifier")
if nameResult := verifiers[0].Name(); nameResult != "test-verifier-0" {
t.Fatalf("expected to create test-verifier-0 for test case but got %v", nameResult)
}

if _, ok := verifiers[0].(*plugin.VerifierPlugin); ok {
Expand All @@ -98,7 +103,8 @@ func TestCreateVerifiersFromConfig_BuiltInVerifiers_ReturnsExpected(t *testing.T

func TestCreateVerifiersFromConfig_PluginVerifiers_ReturnsExpected(t *testing.T) {
verifierConfig := map[string]interface{}{
"name": "plugin-verifier",
"name": "plugin-verifier-0",
"type": "plugin-verifier",
}
verifiersConfig := config.VerifiersConfig{
Verifiers: []config.VerifierConfig{verifierConfig},
Expand All @@ -114,8 +120,8 @@ func TestCreateVerifiersFromConfig_PluginVerifiers_ReturnsExpected(t *testing.T)
t.Fatalf("expected to have %d verifiers, actual count %d", 1, len(verifiers))
}

if verifiers[0].Name() != "plugin-verifier" {
t.Fatalf("expected to create plugin verifier")
if verifiers[0].Name() != "plugin-verifier-0" {
t.Fatalf("expected to create plugin-verifier-0")
}

if _, ok := verifiers[0].(*plugin.VerifierPlugin); !ok {
Expand Down
32 changes: 24 additions & 8 deletions pkg/verifier/notation/notation.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
"github.com/deislabs/ratify/pkg/verifier"
"github.com/deislabs/ratify/pkg/verifier/config"
"github.com/deislabs/ratify/pkg/verifier/factory"
"github.com/deislabs/ratify/pkg/verifier/types"
"github.com/notaryproject/notation-go/log"

_ "github.com/notaryproject/notation-core-go/signature/cose" // register COSE signature
Expand All @@ -45,7 +46,7 @@
)

const (
verifierName = "notation"
verifierType = "notation"
defaultCertPath = "ratify-certs/notation/truststore"
)

Expand All @@ -63,37 +64,50 @@
}

type notationPluginVerifier struct {
name string
verifierType string
artifactTypes []string
notationVerifier *notation.Verifier
}

type notationPluginVerifierFactory struct{}

func init() {
factory.Register(verifierName, &notationPluginVerifierFactory{})
factory.Register(verifierType, &notationPluginVerifierFactory{})
}

func (f *notationPluginVerifierFactory) Create(_ string, verifierConfig config.VerifierConfig, pluginDirectory string, namespace string) (verifier.ReferenceVerifier, error) {
logger.GetLogger(context.Background(), logOpt).Debugf("creating notation with config %v, namespace '%v'", verifierConfig, namespace)
verifierName := fmt.Sprintf("%s", verifierConfig[types.Name])
susanshi marked this conversation as resolved.
Show resolved Hide resolved
verifierTypeStr := ""
if _, ok := verifierConfig[types.Type]; ok {
verifierTypeStr = fmt.Sprintf("%s", verifierConfig[types.Type])
}
conf, err := parseVerifierConfig(verifierConfig, namespace)
if err != nil {
return nil, re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithPluginName(verifierName)
}

verfiyService, err := getVerifierService(conf, pluginDirectory)
verifyService, err := getVerifierService(conf, pluginDirectory)
if err != nil {
return nil, re.ErrorCodePluginInitFailure.WithComponentType(re.Verifier).WithPluginName(verifierName).WithError(err)
}

artifactTypes := strings.Split(conf.ArtifactTypes, ",")
return &notationPluginVerifier{
name: verifierName,
verifierType: verifierTypeStr,
artifactTypes: artifactTypes,
notationVerifier: &verfiyService,
notationVerifier: &verifyService,
}, nil
}

func (v *notationPluginVerifier) Name() string {
return verifierName
return v.name
}

func (v *notationPluginVerifier) Type() string {
return v.verifierType
}

func (v *notationPluginVerifier) CanVerify(_ context.Context, referenceDescriptor ocispecs.ReferenceDescriptor) bool {
Expand Down Expand Up @@ -122,7 +136,7 @@
}

if len(referenceManifest.Blobs) == 0 {
return verifier.VerifierResult{IsSuccess: false}, re.ErrorCodeSignatureNotFound.NewError(re.Verifier, verifierName, re.EmptyLink, nil, fmt.Sprintf("no signature content found for referrer: %s@%s", subjectReference.Path, referenceDescriptor.Digest.String()), re.HideStackTrace)
return verifier.VerifierResult{IsSuccess: false}, re.ErrorCodeSignatureNotFound.NewError(re.Verifier, v.name, re.EmptyLink, nil, fmt.Sprintf("no signature content found for referrer: %s@%s", subjectReference.Path, referenceDescriptor.Digest.String()), re.HideStackTrace)

Check warning on line 139 in pkg/verifier/notation/notation.go

View check run for this annotation

Codecov / codecov/patch

pkg/verifier/notation/notation.go#L139

Added line #L139 was not covered by tests
}

for _, blobDesc := range referenceManifest.Blobs {
Expand All @@ -136,7 +150,7 @@
subjectRef := fmt.Sprintf("%s@%s", subjectReference.Path, subjectReference.Digest.String())
outcome, err := v.verifySignature(ctx, subjectRef, blobDesc.MediaType, subjectDesc.Descriptor, refBlob)
if err != nil {
return verifier.VerifierResult{IsSuccess: false, Extensions: extensions}, re.ErrorCodeVerifyPluginFailure.NewError(re.Verifier, verifierName, re.NotationTsgLink, err, "failed to verify signature of digest", re.HideStackTrace)
return verifier.VerifierResult{IsSuccess: false, Extensions: extensions}, re.ErrorCodeVerifyPluginFailure.NewError(re.Verifier, v.name, re.NotationTsgLink, err, "failed to verify signature of digest", re.HideStackTrace)
}

// Note: notation verifier already validates certificate chain is not empty.
Expand All @@ -146,7 +160,8 @@
}

return verifier.VerifierResult{
Name: verifierName,
Name: v.name,
junczhu marked this conversation as resolved.
Show resolved Hide resolved
Type: v.verifierType,
IsSuccess: true,
Message: "signature verification success",
Extensions: extensions,
Expand All @@ -173,6 +188,7 @@
}

func parseVerifierConfig(verifierConfig config.VerifierConfig, namespace string) (*NotationPluginVerifierConfig, error) {
verifierName := verifierConfig[types.Name].(string)
conf := &NotationPluginVerifierConfig{}

verifierConfigBytes, err := json.Marshal(verifierConfig)
Expand Down
22 changes: 16 additions & 6 deletions pkg/verifier/notation/notation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,11 +143,19 @@ func (s mockStore) GetSubjectDescriptor(_ context.Context, _ common.Reference) (
}

func TestName(t *testing.T) {
v := &notationPluginVerifier{}
name := v.Name()
verifierConfig := map[string]interface{}{
"name": "notation-verifier-0",
"type": "notation",
"trustPolicyDoc": testTrustPolicy,
}

if name != "notation" {
t.Fatalf("expect name: notation, got: %s", name)
f := &notationPluginVerifierFactory{}
verifier, err := f.Create(testVersion, verifierConfig, "", "")
if err != nil {
t.Fatalf("failed create notation verifier got error = %v", err)
}
if name := verifier.Name(); name != "notation-verifier-0" {
t.Fatalf("expect name: notation-verifier-0, got: %s", name)
}
}

Expand Down Expand Up @@ -205,7 +213,8 @@ func TestParseVerifierConfig(t *testing.T) {
{
name: "failed unmarshalling to notation config",
configMap: map[string]interface{}{
"name": []string{test},
"name": test,
"verificationCerts": test,
},
expectErr: true,
expect: nil,
Expand Down Expand Up @@ -294,7 +303,8 @@ func TestCreate(t *testing.T) {
{
name: "failed parsing verifier config",
configMap: map[string]interface{}{
"name": []string{test},
"name": test,
"trustPolicyDoc": 1,
},
expectErr: true,
},
Expand Down
Loading
Loading