From 3e963d2d68a5f406e884474c3de504e187195562 Mon Sep 17 00:00:00 2001 From: junczhu Date: Mon, 3 Jun 2024 18:41:42 +0000 Subject: [PATCH 01/18] feat: support certStoresByType 1 --- pkg/verifier/notation/certStoresByType.go | 61 ++++++++++++++++++++++ pkg/verifier/notation/notation.go | 63 ++++++++++++++++++++--- pkg/verifier/notation/notation_test.go | 8 +-- pkg/verifier/notation/truststore.go | 27 +++++++--- pkg/verifier/notation/truststore_test.go | 41 +++++++++------ 5 files changed, 167 insertions(+), 33 deletions(-) create mode 100644 pkg/verifier/notation/certStoresByType.go diff --git a/pkg/verifier/notation/certStoresByType.go b/pkg/verifier/notation/certStoresByType.go new file mode 100644 index 000000000..32b1c29e6 --- /dev/null +++ b/pkg/verifier/notation/certStoresByType.go @@ -0,0 +1,61 @@ +/* +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 notation + +import ( + "context" + + re "github.com/deislabs/ratify/errors" + "github.com/deislabs/ratify/internal/logger" + "github.com/notaryproject/notation-go/verifier/truststore" +) + +type verificationCertStores map[string]interface{} + +// type certStores map[string][]string +type certStoresByType map[string]map[string][]string + +func NewCertStoreByType(verificationCertStores verificationCertStores) (certStoresByType, error) { + certStoresByType := make(map[string]map[string][]string) + for certStoreType, certStores := range verificationCertStores { + if reformedCertStores, ok := certStores.(map[string]interface{}); ok { + certStoresByType[certStoreType] = make(map[string][]string) + for certStore, certs := range reformedCertStores { + var reformedCerts []string + for _, cert := range certs.([]interface{}) { + if reformedCert, ok := cert.(string); ok { + reformedCerts = append(reformedCerts, reformedCert) + } + } + certStoresByType[certStoreType][certStore] = reformedCerts + } + } else { + return nil, re.ErrorCodeEnvNotSet.WithComponentType(re.Verifier).WithDetail("") + } + } + return certStoresByType, nil +} + +// GetCertGroupFromStore returns certain type of certs from namedStore +func GetCertGroupFromStore(ctx context.Context, certStoresByType certStoresByType, storeType truststore.Type, namedStore string) (certGroup []string) { + if certStores, ok := certStoresByType[string(storeType)]; ok { + if certGroup, ok = certStores[namedStore]; ok { + return + } + } + logger.GetLogger(ctx, logOpt).Debugf("unable to fetch certGroup from namedStore: %+v in type: %v", namedStore, storeType) + return +} diff --git a/pkg/verifier/notation/notation.go b/pkg/verifier/notation/notation.go index a1db7a18b..26bc8edd8 100644 --- a/pkg/verifier/notation/notation.go +++ b/pkg/verifier/notation/notation.go @@ -41,12 +41,15 @@ import ( "github.com/notaryproject/notation-go" notationVerifier "github.com/notaryproject/notation-go/verifier" "github.com/notaryproject/notation-go/verifier/trustpolicy" + "github.com/notaryproject/notation-go/verifier/truststore" oci "github.com/opencontainers/image-spec/specs-go/v1" ) const ( - verifierType = "notation" - defaultCertPath = "ratify-certs/notation/truststore" + verifierType = "notation" + defaultCertPath = "ratify-certs/notation/truststore" + trustStoreTypeCA = string(truststore.TypeCA) + trustStoreTypeypeSigningAuthority = string(truststore.TypeSigningAuthority) ) // NotationPluginVerifierConfig describes the configuration of notation verifier @@ -56,8 +59,21 @@ type NotationPluginVerifierConfig struct { //nolint:revive // ignore linter to h // VerificationCerts is array of directories containing certificates. VerificationCerts []string `json:"verificationCerts"` - // VerificationCerts is map defining which keyvault certificates belong to which trust store - VerificationCertStores map[string][]string `json:"verificationCertStores"` + // VerificationCertStores is map defining which keyvault certificates belong to which trust store name. + // VerificationCertStores accepts new format map[string]map[string][]string + // { + // "ca": { + // "certs": {"kv1", "kv2"}, + // }, + // "signingauthority": { + // "certs": {"kv3"} + // }, + // } + // VerificationCertStores accepts legacy format map[string][]string as well. + // { + // "certs": {"kv1", "kv2"}, + // }, + VerificationCertStores verificationCertStores `json:"verificationCertStores"` // TrustPolicyDoc represents a trustpolicy.json document. Reference: https://pkg.go.dev/github.com/notaryproject/notation-go@v0.12.0-beta.1.0.20221125022016-ab113ebd2a6c/verifier/trustpolicy#Document TrustPolicyDoc trustpolicy.Document `json:"trustPolicyDoc"` } @@ -168,11 +184,14 @@ func (v *notationPluginVerifier) Verify(ctx context.Context, } func getVerifierService(conf *NotationPluginVerifierConfig, pluginDirectory string) (notation.Verifier, error) { - store := &trustStore{ - certPaths: conf.VerificationCerts, - certStores: conf.VerificationCertStores, + // store := &trustStore{ + // certPaths: conf.VerificationCerts, + // certStores: conf.VerificationCertStores, + // } + store, err := NewTrustStore(conf.VerificationCerts, conf.VerificationCertStores) + if err != nil { + return nil, err } - return notationVerifier.New(&conf.TrustPolicyDoc, store, NewRatifyPluginManager(pluginDirectory)) } @@ -201,6 +220,12 @@ func parseVerifierConfig(verifierConfig config.VerifierConfig, _ string) (*Notat defaultCertsDir := paths.Join(homedir.Get(), ratifyconfig.ConfigFileDir, defaultCertPath) conf.VerificationCerts = append(conf.VerificationCerts, defaultCertsDir) + if len(conf.VerificationCertStores) > 0 { + err := normalizeVerificationCertsStores(conf) + if err != nil { + return nil, err + } + } return conf, nil } @@ -208,3 +233,25 @@ func parseVerifierConfig(verifierConfig config.VerifierConfig, _ string) (*Notat func (v *notationPluginVerifier) GetNestedReferences() []string { return []string{} } + +// normalizeVerificationCertsStores normalize the structure does not match the latest spec +func normalizeVerificationCertsStores(conf *NotationPluginVerifierConfig) error { + isCertStoresByType, isLegacyCertStore := false, false + for key := range conf.VerificationCertStores { + if key != trustStoreTypeCA && key != trustStoreTypeypeSigningAuthority { + isLegacyCertStore = true + logger.GetLogger(context.Background(), logOpt).Debugf("Get VerificationCertStores in legacy format") + } else { + isCertStoresByType = true + } + } + if isCertStoresByType && isLegacyCertStore { + return re.ErrorCodeConfigInvalid.NewError(re.Verifier, conf.Name, re.EmptyLink, nil, "both old VerificationCertStores and new VerificationCertStores are provided, please provide only one", re.HideStackTrace) + } else if !isCertStoresByType && isLegacyCertStore { + // normalize : to ca: if no store type is provided + conf.VerificationCertStores = map[string]interface{}{ + trustStoreTypeCA: conf.VerificationCertStores, + } + } + return nil +} diff --git a/pkg/verifier/notation/notation_test.go b/pkg/verifier/notation/notation_test.go index 876e1da74..4d2efc7a8 100644 --- a/pkg/verifier/notation/notation_test.go +++ b/pkg/verifier/notation/notation_test.go @@ -256,9 +256,11 @@ func TestParseVerifierConfig(t *testing.T) { expect: &NotationPluginVerifierConfig{ Name: test, VerificationCerts: []string{testPath, defaultCertDir}, - VerificationCertStores: map[string][]string{ - "certstore1": {"defaultns/akv1", "akv2"}, - "certstore2": {"akv3", "akv4"}, + VerificationCertStores: verificationCertStores{ + "ca": map[string][]string{ + "certstore1": {"defaultns/akv1", "akv2"}, + "certstore2": {"akv3", "akv4"}, + }, }, }, }, diff --git a/pkg/verifier/notation/truststore.go b/pkg/verifier/notation/truststore.go index 03d4c21c1..fae13a5ec 100644 --- a/pkg/verifier/notation/truststore.go +++ b/pkg/verifier/notation/truststore.go @@ -33,27 +33,40 @@ var logOpt = logger.Option{ } type trustStore struct { - certPaths []string - certStores map[string][]string + certPaths []string + certStoresByType certStoresByType +} + +func NewTrustStore(certPaths []string, verificationCertStores verificationCertStores) (*trustStore, error) { + certStoresByType, err := NewCertStoreByType(verificationCertStores) + if err != nil { + return nil, err + } + store := &trustStore{ + certPaths: certPaths, + certStoresByType: certStoresByType, + } + return store, nil } // trustStore implements GetCertificates API of X509TrustStore interface: [https://pkg.go.dev/github.com/notaryproject/notation-go@v1.0.0-rc.3/verifier/truststore#X509TrustStore] // Note: this api gets invoked when Ratify calls verify API, so the certificates // will be loaded for each signature verification. // And this API must follow the Notation Trust Store spec: https://github.com/notaryproject/notaryproject/blob/main/specs/trust-store-trust-policy.md#trust-store -func (s trustStore) GetCertificates(ctx context.Context, _ truststore.Type, namedStore string) ([]*x509.Certificate, error) { - certs, err := s.getCertificatesInternal(ctx, namedStore) +func (s *trustStore) GetCertificates(ctx context.Context, trustStoreType truststore.Type, namedStore string) ([]*x509.Certificate, error) { + certs, err := s.getCertificatesInternal(ctx, trustStoreType, namedStore) if err != nil { return nil, err } return s.filterValidCerts(certs) } -func (s trustStore) getCertificatesInternal(ctx context.Context, namedStore string) ([]*x509.Certificate, error) { +func (s *trustStore) getCertificatesInternal(ctx context.Context, storeType truststore.Type, namedStore string) ([]*x509.Certificate, error) { certs := make([]*x509.Certificate, 0) + certGroup := GetCertGroupFromStore(ctx, s.certStoresByType, storeType, namedStore) // certs configured for this namedStore overrides cert path - if certGroup := s.certStores[namedStore]; len(certGroup) > 0 { + if len(certGroup) > 0 { for _, certStore := range certGroup { logger.GetLogger(ctx, logOpt).Debugf("truststore getting certStore %v", certStore) certMap, err := keymanagementprovider.GetCertificatesFromMap(ctx, certStore) @@ -93,7 +106,7 @@ func (s trustStore) getCertificatesInternal(ctx context.Context, namedStore stri } // filterValidCerts keeps CA certificates and self-signed certs. -func (s trustStore) filterValidCerts(certs []*x509.Certificate) ([]*x509.Certificate, error) { +func (s *trustStore) filterValidCerts(certs []*x509.Certificate) ([]*x509.Certificate, error) { filteredCerts := make([]*x509.Certificate, 0) for _, cert := range certs { if !cert.IsCA { diff --git a/pkg/verifier/notation/truststore_test.go b/pkg/verifier/notation/truststore_test.go index 15145f036..fa0f6bf24 100644 --- a/pkg/verifier/notation/truststore_test.go +++ b/pkg/verifier/notation/truststore_test.go @@ -22,6 +22,7 @@ import ( "testing" "github.com/deislabs/ratify/pkg/controllers" + "github.com/notaryproject/notation-go/verifier/truststore" ) const ( @@ -48,26 +49,36 @@ func (m *mockCertStores) DeleteStore(_ string) {} func TestGetCertificates_EmptyCertMap(t *testing.T) { resetCertStore() - certStore := map[string][]string{} - certStore["store1"] = []string{"kv1"} - certStore["store2"] = []string{"kv2"} - store := &trustStore{ - certStores: certStore, + certStore := verificationCertStores{ + "certstore1": []string{ + "akv1", + }, + "certstore2": []string{ + "akv2", + }, } - - if _, err := store.getCertificatesInternal(context.Background(), "store1"); err == nil { + store, err := NewTrustStore(nil, certStore) + if err != nil { + panic("failed to parse verificationCertStores: " + err.Error()) + } + if _, err := store.getCertificatesInternal(context.Background(), truststore.TypeCA, "store1"); err == nil { t.Fatalf("error expected if cert map is empty") } } func TestGetCertificates_NamedStore(t *testing.T) { resetCertStore() - certStore := map[string][]string{} - certStore["store1"] = []string{"default/kv1"} - certStore["store2"] = []string{"projecta/kv2"} - - store := &trustStore{ - certStores: certStore, + certStore := verificationCertStores{ + "certstore1": []string{ + "default/kv1", + }, + "certstore2": []string{ + "projecta/kv2", + }, + } + store, err := NewTrustStore(nil, certStore) + if err != nil { + panic("failed to parse verificationCertStores: " + err.Error()) } kv1Cert := getCert(certStr) @@ -81,7 +92,7 @@ func TestGetCertificates_NamedStore(t *testing.T) { } // only the certificate in the specified namedStore should be returned - result, _ := store.getCertificatesInternal(context.Background(), "store1") + result, _ := store.getCertificatesInternal(context.Background(), truststore.TypeCA, "store1") expectedLen := 1 if len(result) != expectedLen { @@ -107,7 +118,7 @@ func TestGetCertificates_certPath(t *testing.T) { trustStore := &trustStore{ certPaths: []string{tmpFile.Name()}, } - certs, err := trustStore.getCertificatesInternal(context.Background(), "") + certs, err := trustStore.getCertificatesInternal(context.Background(), truststore.TypeCA, "") if err != nil { t.Fatalf("failed to get certs: %v", err) } From 7afefe5b4aaa9483ea552a1b239b441a051f9f86 Mon Sep 17 00:00:00 2001 From: junczhu Date: Mon, 3 Jun 2024 20:15:42 +0000 Subject: [PATCH 02/18] feat: support certStoresByType 2 --- pkg/verifier/notation/certStoresByType.go | 23 +++++++++-------------- pkg/verifier/notation/notation.go | 4 ++-- pkg/verifier/notation/notation_test.go | 8 ++++---- pkg/verifier/notation/truststore.go | 4 ++-- pkg/verifier/notation/truststore_test.go | 14 ++++++-------- 5 files changed, 23 insertions(+), 30 deletions(-) diff --git a/pkg/verifier/notation/certStoresByType.go b/pkg/verifier/notation/certStoresByType.go index 32b1c29e6..c0c69632c 100644 --- a/pkg/verifier/notation/certStoresByType.go +++ b/pkg/verifier/notation/certStoresByType.go @@ -18,7 +18,6 @@ package notation import ( "context" - re "github.com/deislabs/ratify/errors" "github.com/deislabs/ratify/internal/logger" "github.com/notaryproject/notation-go/verifier/truststore" ) @@ -28,22 +27,18 @@ type verificationCertStores map[string]interface{} // type certStores map[string][]string type certStoresByType map[string]map[string][]string -func NewCertStoreByType(verificationCertStores verificationCertStores) (certStoresByType, error) { +func newCertStoreByType(confVerificationCertStores verificationCertStores) (certStoresByType, error) { certStoresByType := make(map[string]map[string][]string) - for certStoreType, certStores := range verificationCertStores { - if reformedCertStores, ok := certStores.(map[string]interface{}); ok { - certStoresByType[certStoreType] = make(map[string][]string) - for certStore, certs := range reformedCertStores { - var reformedCerts []string - for _, cert := range certs.([]interface{}) { - if reformedCert, ok := cert.(string); ok { - reformedCerts = append(reformedCerts, reformedCert) - } + for certStoreType, certStores := range confVerificationCertStores { + certStoresByType[certStoreType] = make(map[string][]string) + for certStore, certs := range certStores.(verificationCertStores) { + var reformedCerts []string + for _, cert := range certs.([]interface{}) { + if reformedCert, ok := cert.(string); ok { + reformedCerts = append(reformedCerts, reformedCert) } - certStoresByType[certStoreType][certStore] = reformedCerts } - } else { - return nil, re.ErrorCodeEnvNotSet.WithComponentType(re.Verifier).WithDetail("") + certStoresByType[certStoreType][certStore] = reformedCerts } } return certStoresByType, nil diff --git a/pkg/verifier/notation/notation.go b/pkg/verifier/notation/notation.go index 26bc8edd8..b98f8278e 100644 --- a/pkg/verifier/notation/notation.go +++ b/pkg/verifier/notation/notation.go @@ -188,7 +188,7 @@ func getVerifierService(conf *NotationPluginVerifierConfig, pluginDirectory stri // certPaths: conf.VerificationCerts, // certStores: conf.VerificationCertStores, // } - store, err := NewTrustStore(conf.VerificationCerts, conf.VerificationCertStores) + store, err := newTrustStore(conf.VerificationCerts, conf.VerificationCertStores) if err != nil { return nil, err } @@ -249,7 +249,7 @@ func normalizeVerificationCertsStores(conf *NotationPluginVerifierConfig) error return re.ErrorCodeConfigInvalid.NewError(re.Verifier, conf.Name, re.EmptyLink, nil, "both old VerificationCertStores and new VerificationCertStores are provided, please provide only one", re.HideStackTrace) } else if !isCertStoresByType && isLegacyCertStore { // normalize : to ca: if no store type is provided - conf.VerificationCertStores = map[string]interface{}{ + conf.VerificationCertStores = verificationCertStores{ trustStoreTypeCA: conf.VerificationCertStores, } } diff --git a/pkg/verifier/notation/notation_test.go b/pkg/verifier/notation/notation_test.go index 4d2efc7a8..7fad5e198 100644 --- a/pkg/verifier/notation/notation_test.go +++ b/pkg/verifier/notation/notation_test.go @@ -248,7 +248,7 @@ func TestParseVerifierConfig(t *testing.T) { "name": test, "verificationCerts": []string{testPath}, "verificationCertStores": map[string][]string{ - "certstore1": {"defaultns/akv1", "akv2"}, + "certstore1": {"akv1", "akv2"}, "certstore2": {"akv3", "akv4"}, }, }, @@ -257,9 +257,9 @@ func TestParseVerifierConfig(t *testing.T) { Name: test, VerificationCerts: []string{testPath, defaultCertDir}, VerificationCertStores: verificationCertStores{ - "ca": map[string][]string{ - "certstore1": {"defaultns/akv1", "akv2"}, - "certstore2": {"akv3", "akv4"}, + trustStoreTypeCA: verificationCertStores{ + "certstore1": []interface{}{"akv1", "akv2"}, + "certstore2": []interface{}{"akv3", "akv4"}, }, }, }, diff --git a/pkg/verifier/notation/truststore.go b/pkg/verifier/notation/truststore.go index fae13a5ec..62aa9c4a3 100644 --- a/pkg/verifier/notation/truststore.go +++ b/pkg/verifier/notation/truststore.go @@ -37,8 +37,8 @@ type trustStore struct { certStoresByType certStoresByType } -func NewTrustStore(certPaths []string, verificationCertStores verificationCertStores) (*trustStore, error) { - certStoresByType, err := NewCertStoreByType(verificationCertStores) +func newTrustStore(certPaths []string, verificationCertStores verificationCertStores) (*trustStore, error) { + certStoresByType, err := newCertStoreByType(verificationCertStores) if err != nil { return nil, err } diff --git a/pkg/verifier/notation/truststore_test.go b/pkg/verifier/notation/truststore_test.go index fa0f6bf24..d11e643d5 100644 --- a/pkg/verifier/notation/truststore_test.go +++ b/pkg/verifier/notation/truststore_test.go @@ -50,18 +50,16 @@ func (m *mockCertStores) DeleteStore(_ string) {} func TestGetCertificates_EmptyCertMap(t *testing.T) { resetCertStore() certStore := verificationCertStores{ - "certstore1": []string{ - "akv1", - }, - "certstore2": []string{ - "akv2", + trustStoreTypeCA: verificationCertStores{ + "certstore1": []interface{}{"akv1", "akv2"}, + "certstore2": []interface{}{"akv3", "akv4"}, }, } - store, err := NewTrustStore(nil, certStore) + store, err := newTrustStore([]string{}, certStore) if err != nil { panic("failed to parse verificationCertStores: " + err.Error()) } - if _, err := store.getCertificatesInternal(context.Background(), truststore.TypeCA, "store1"); err == nil { + if _, err := store.getCertificatesInternal(context.Background(), truststore.TypeCA, "certstore1"); err == nil { t.Fatalf("error expected if cert map is empty") } } @@ -76,7 +74,7 @@ func TestGetCertificates_NamedStore(t *testing.T) { "projecta/kv2", }, } - store, err := NewTrustStore(nil, certStore) + store, err := newTrustStore(nil, certStore) if err != nil { panic("failed to parse verificationCertStores: " + err.Error()) } From 1a220de4bf717011677e5e7253e2f1c0f471ff4a Mon Sep 17 00:00:00 2001 From: junczhu Date: Mon, 3 Jun 2024 20:23:29 +0000 Subject: [PATCH 03/18] feat: support certStoresByType 3 --- pkg/verifier/notation/truststore_test.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/pkg/verifier/notation/truststore_test.go b/pkg/verifier/notation/truststore_test.go index d11e643d5..6464747a2 100644 --- a/pkg/verifier/notation/truststore_test.go +++ b/pkg/verifier/notation/truststore_test.go @@ -67,11 +67,9 @@ func TestGetCertificates_EmptyCertMap(t *testing.T) { func TestGetCertificates_NamedStore(t *testing.T) { resetCertStore() certStore := verificationCertStores{ - "certstore1": []string{ - "default/kv1", - }, - "certstore2": []string{ - "projecta/kv2", + trustStoreTypeCA: verificationCertStores{ + "certstore1": []interface{}{"default/kv1"}, + "certstore2": []interface{}{"projecta/kv2"}, }, } store, err := newTrustStore(nil, certStore) @@ -90,7 +88,7 @@ func TestGetCertificates_NamedStore(t *testing.T) { } // only the certificate in the specified namedStore should be returned - result, _ := store.getCertificatesInternal(context.Background(), truststore.TypeCA, "store1") + result, _ := store.getCertificatesInternal(context.Background(), truststore.TypeCA, "certstore1") expectedLen := 1 if len(result) != expectedLen { From bebd12401a223cb61cc530275d1e0666b469ee16 Mon Sep 17 00:00:00 2001 From: junczhu Date: Wed, 5 Jun 2024 05:57:17 +0000 Subject: [PATCH 04/18] feat: support certStoresByType add test --- pkg/verifier/notation/notation_test.go | 47 ++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/pkg/verifier/notation/notation_test.go b/pkg/verifier/notation/notation_test.go index 7fad5e198..5f592baa0 100644 --- a/pkg/verifier/notation/notation_test.go +++ b/pkg/verifier/notation/notation_test.go @@ -410,3 +410,50 @@ func TestGetNestedReferences(t *testing.T) { t.Fatalf("notation signature should not have nested references") } } + +func TestNormalizeVerificationCertsStores(t *testing.T) { + tests := []struct { + name string + conf *NotationPluginVerifierConfig + expectErr bool + }{ + { + name: "successfully normalizaVerificationCertsStores", + conf: &NotationPluginVerifierConfig{ + Name: test, + VerificationCerts: []string{testPath, defaultCertDir}, + VerificationCertStores: verificationCertStores{ + trustStoreTypeCA: verificationCertStores{ + "certstore1": []interface{}{"akv1", "akv2"}, + "certstore2": []interface{}{"akv3", "akv4"}, + }, + }, + }, + expectErr: false, + }, + { + + name: "failed normalizaVerificationCertsStores with both old VerificationCertStores and new VerificationCertStores are provided", + conf: &NotationPluginVerifierConfig{ + Name: test, + VerificationCerts: []string{testPath, defaultCertDir}, + VerificationCertStores: verificationCertStores{ + trustStoreTypeCA: verificationCertStores{ + "certstore1": []interface{}{"akv1", "akv2"}, + }, + "certstore2": []interface{}{"akv3", "akv4"}, + }, + }, + expectErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := normalizeVerificationCertsStores(tt.conf) + if (err != nil) != tt.expectErr { + t.Errorf("error = %v, expectErr = %v", err, tt.expectErr) + } + }) + } +} From 7cb49cb1976f88cb932a97d7994f5f188daa5149 Mon Sep 17 00:00:00 2001 From: junczhu Date: Fri, 7 Jun 2024 03:31:54 +0000 Subject: [PATCH 05/18] chore: e2e update --- Makefile | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 7c16b2aa9..d2875c4e9 100644 --- a/Makefile +++ b/Makefile @@ -542,7 +542,7 @@ e2e-deploy-base-ratify: e2e-notation-setup e2e-notation-leaf-cert-setup e2e-cosi --set-file provider.tls.caCert=${CERT_DIR}/ca.crt \ --set-file provider.tls.caKey=${CERT_DIR}/ca.key \ --set provider.tls.cabundle="$(shell cat ${CERT_DIR}/ca.crt | base64 | tr -d '\n')" \ - --set notationCerts[0]="$$(cat ~/.config/notation/localkeys/ratify-bats-test.crt)" \ + --set notationCerts={"$$(cat ~/.config/notation/localkeys/ratify-bats-test.crt)"} \ --set cosignKeys[0]="$$(cat .staging/cosign/cosign.pub)" \ --set cosign.key="$$(cat .staging/cosign/cosign.pub)" \ --set oras.useHttp=true \ @@ -589,7 +589,7 @@ e2e-helm-deploy-ratify: --set-file provider.tls.caCert=${CERT_DIR}/ca.crt \ --set-file provider.tls.caKey=${CERT_DIR}/ca.key \ --set provider.tls.cabundle="$(shell cat ${CERT_DIR}/ca.crt | base64 | tr -d '\n')" \ - --set notationCerts[0]="$$(cat ~/.config/notation/localkeys/ratify-bats-test.crt)" \ + --set notationCerts={"$$(cat ~/.config/notation/localkeys/ratify-bats-test.crt)"} \ --set cosignKeys[0]="$$(cat .staging/cosign/cosign.pub)" \ --set cosign.key="$$(cat .staging/cosign/cosign.pub)" \ --set oras.useHttp=true \ @@ -608,7 +608,7 @@ e2e-helm-deploy-ratify-without-tls-certs: --set image.tag=test \ --set gatekeeper.version=${GATEKEEPER_VERSION} \ --set featureFlags.RATIFY_CERT_ROTATION=${CERT_ROTATION_ENABLED} \ - --set notaryCert="$$(cat ~/.config/notation/localkeys/ratify-bats-test.crt)" \ + --set notationCerts={"$$(cat ~/.config/notation/localkeys/ratify-bats-test.crt)"} \ --set cosign.key="$$(cat .staging/cosign/cosign.pub)" \ --set cosignKeys[0]="$$(cat .staging/cosign/cosign.pub)" \ --set oras.useHttp=true \ @@ -649,7 +649,7 @@ e2e-helm-deploy-ratify-replica: e2e-helm-deploy-redis e2e-notation-setup e2e-bui --set-file provider.tls.caCert=${CERT_DIR}/ca.crt \ --set-file provider.tls.caKey=${CERT_DIR}/ca.key \ --set provider.tls.cabundle="$(shell cat ${CERT_DIR}/ca.crt | base64 | tr -d '\n')" \ - --set notationCerts[0]="$$(cat ~/.config/notation/localkeys/ratify-bats-test.crt)" \ + --set notationCerts={"$$(cat ~/.config/notation/localkeys/ratify-bats-test.crt)"} \ --set oras.useHttp=true \ --set cosign.enabled=false \ --set-file dockerConfig="mount_config.json" \ From cd3c6c1d6ff3821981aba8cb62a9ae9a0f915023 Mon Sep 17 00:00:00 2001 From: junczhu Date: Mon, 17 Jun 2024 17:02:30 +0000 Subject: [PATCH 06/18] feat: update implement of certstorebytype 1 --- ...ertStoresByType.go => certstoresbytype.go} | 49 ++++++++++++++----- pkg/verifier/notation/notation.go | 23 ++++----- 2 files changed, 46 insertions(+), 26 deletions(-) rename pkg/verifier/notation/{certStoresByType.go => certstoresbytype.go} (50%) diff --git a/pkg/verifier/notation/certStoresByType.go b/pkg/verifier/notation/certstoresbytype.go similarity index 50% rename from pkg/verifier/notation/certStoresByType.go rename to pkg/verifier/notation/certstoresbytype.go index c0c69632c..bdc9fba2d 100644 --- a/pkg/verifier/notation/certStoresByType.go +++ b/pkg/verifier/notation/certstoresbytype.go @@ -22,23 +22,48 @@ import ( "github.com/notaryproject/notation-go/verifier/truststore" ) +type certStoreType string + +const ( + CA certStoreType = "CA" + SigningAuthority certStoreType = "signingAuthority" +) + +func (certstoretype certStoreType) String() string { + return string(certstoretype) +} + +// verificationCertStores describes the configuration of verificationCertStores type verificationCertStores map[string]interface{} -// type certStores map[string][]string -type certStoresByType map[string]map[string][]string +// certStoresByType describes the configuration of verificationCertStores by certStoreType +// +// { +// "ca": { +// "certs": {"kv1", "kv2"}, +// }, +// "signingauthority": { +// "certs": {"kv3"} +// }, +// } +type certStoresByType map[certStoreType]map[string][]string func newCertStoreByType(confVerificationCertStores verificationCertStores) (certStoresByType, error) { - certStoresByType := make(map[string]map[string][]string) - for certStoreType, certStores := range confVerificationCertStores { - certStoresByType[certStoreType] = make(map[string][]string) - for certStore, certs := range certStores.(verificationCertStores) { - var reformedCerts []string - for _, cert := range certs.([]interface{}) { - if reformedCert, ok := cert.(string); ok { - reformedCerts = append(reformedCerts, reformedCert) + certStoresByType := make(map[certStoreType]map[string][]string) + for certstoretype, certStores := range confVerificationCertStores { + certStoresByType[certStoreType(certstoretype)] = make(map[string][]string) + if convertedCertStores, ok := certStores.(verificationCertStores); ok { + for certStore, certs := range convertedCertStores { + var reformedCerts []string + if convertedCerts, ok := certs.([]interface{}); ok { + for _, cert := range convertedCerts { + if reformedCert, ok := cert.(string); ok { + reformedCerts = append(reformedCerts, reformedCert) + } + } + certStoresByType[certStoreType(certstoretype)][certStore] = reformedCerts } } - certStoresByType[certStoreType][certStore] = reformedCerts } } return certStoresByType, nil @@ -46,7 +71,7 @@ func newCertStoreByType(confVerificationCertStores verificationCertStores) (cert // GetCertGroupFromStore returns certain type of certs from namedStore func GetCertGroupFromStore(ctx context.Context, certStoresByType certStoresByType, storeType truststore.Type, namedStore string) (certGroup []string) { - if certStores, ok := certStoresByType[string(storeType)]; ok { + if certStores, ok := certStoresByType[certStoreType(storeType)]; ok { if certGroup, ok = certStores[namedStore]; ok { return } diff --git a/pkg/verifier/notation/notation.go b/pkg/verifier/notation/notation.go index b98f8278e..392c6f687 100644 --- a/pkg/verifier/notation/notation.go +++ b/pkg/verifier/notation/notation.go @@ -59,19 +59,19 @@ type NotationPluginVerifierConfig struct { //nolint:revive // ignore linter to h // VerificationCerts is array of directories containing certificates. VerificationCerts []string `json:"verificationCerts"` - // VerificationCertStores is map defining which keyvault certificates belong to which trust store name. + // VerificationCertStores defines a collection of Notary Project Trust Stores. // VerificationCertStores accepts new format map[string]map[string][]string // { - // "ca": { - // "certs": {"kv1", "kv2"}, - // }, - // "signingauthority": { - // "certs": {"kv3"} - // }, + // "ca": { + // "certs": {"kv1", "kv2"}, + // }, + // "signingauthority": { + // "certs": {"kv3"} + // }, // } // VerificationCertStores accepts legacy format map[string][]string as well. // { - // "certs": {"kv1", "kv2"}, + // "certs": {"kv1", "kv2"}, // }, VerificationCertStores verificationCertStores `json:"verificationCertStores"` // TrustPolicyDoc represents a trustpolicy.json document. Reference: https://pkg.go.dev/github.com/notaryproject/notation-go@v0.12.0-beta.1.0.20221125022016-ab113ebd2a6c/verifier/trustpolicy#Document @@ -184,10 +184,6 @@ func (v *notationPluginVerifier) Verify(ctx context.Context, } func getVerifierService(conf *NotationPluginVerifierConfig, pluginDirectory string) (notation.Verifier, error) { - // store := &trustStore{ - // certPaths: conf.VerificationCerts, - // certStores: conf.VerificationCertStores, - // } store, err := newTrustStore(conf.VerificationCerts, conf.VerificationCertStores) if err != nil { return nil, err @@ -221,8 +217,7 @@ func parseVerifierConfig(verifierConfig config.VerifierConfig, _ string) (*Notat defaultCertsDir := paths.Join(homedir.Get(), ratifyconfig.ConfigFileDir, defaultCertPath) conf.VerificationCerts = append(conf.VerificationCerts, defaultCertsDir) if len(conf.VerificationCertStores) > 0 { - err := normalizeVerificationCertsStores(conf) - if err != nil { + if err := normalizeVerificationCertsStores(conf); err != nil { return nil, err } } From b3a26e062fc2d307b86eaf8d6325d81fac46bcde Mon Sep 17 00:00:00 2001 From: junczhu Date: Mon, 17 Jun 2024 17:46:34 +0000 Subject: [PATCH 07/18] feat: update implement of certstorebytype 2 --- pkg/verifier/notation/certStores.go | 28 +++++++++++++++++++++++ pkg/verifier/notation/certstoresbytype.go | 17 +++++++------- pkg/verifier/notation/truststore.go | 12 +++++----- 3 files changed, 43 insertions(+), 14 deletions(-) create mode 100644 pkg/verifier/notation/certStores.go diff --git a/pkg/verifier/notation/certStores.go b/pkg/verifier/notation/certStores.go new file mode 100644 index 000000000..382ec3eb7 --- /dev/null +++ b/pkg/verifier/notation/certStores.go @@ -0,0 +1,28 @@ +/* +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 notation + +import ( + "context" + + "github.com/notaryproject/notation-go/verifier/truststore" +) + +// certStores is an interface that defines the methods for managing certificate stores. +type certStores interface { + // GetCertGroupFromStore returns certain type of cert group from namedStore + GetCertGroupFromStore(ctx context.Context, storeType truststore.Type, namedStore string) (certGroup []string) +} diff --git a/pkg/verifier/notation/certstoresbytype.go b/pkg/verifier/notation/certstoresbytype.go index bdc9fba2d..0285c198f 100644 --- a/pkg/verifier/notation/certstoresbytype.go +++ b/pkg/verifier/notation/certstoresbytype.go @@ -33,7 +33,7 @@ func (certstoretype certStoreType) String() string { return string(certstoretype) } -// verificationCertStores describes the configuration of verificationCertStores +// verificationCertStores implements certStores interface type verificationCertStores map[string]interface{} // certStoresByType describes the configuration of verificationCertStores by certStoreType @@ -48,10 +48,11 @@ type verificationCertStores map[string]interface{} // } type certStoresByType map[certStoreType]map[string][]string -func newCertStoreByType(confVerificationCertStores verificationCertStores) (certStoresByType, error) { - certStoresByType := make(map[certStoreType]map[string][]string) +// newCertStoreByType do type assertion and convert certStores configuration into certStoresByType +func newCertStoreByType(confVerificationCertStores verificationCertStores) (certStores, error) { + s := make(certStoresByType) for certstoretype, certStores := range confVerificationCertStores { - certStoresByType[certStoreType(certstoretype)] = make(map[string][]string) + s[certStoreType(certstoretype)] = make(map[string][]string) if convertedCertStores, ok := certStores.(verificationCertStores); ok { for certStore, certs := range convertedCertStores { var reformedCerts []string @@ -61,17 +62,17 @@ func newCertStoreByType(confVerificationCertStores verificationCertStores) (cert reformedCerts = append(reformedCerts, reformedCert) } } - certStoresByType[certStoreType(certstoretype)][certStore] = reformedCerts + s[certStoreType(certstoretype)][certStore] = reformedCerts } } } } - return certStoresByType, nil + return s, nil } // GetCertGroupFromStore returns certain type of certs from namedStore -func GetCertGroupFromStore(ctx context.Context, certStoresByType certStoresByType, storeType truststore.Type, namedStore string) (certGroup []string) { - if certStores, ok := certStoresByType[certStoreType(storeType)]; ok { +func (s certStoresByType) GetCertGroupFromStore(ctx context.Context, storeType truststore.Type, namedStore string) (certGroup []string) { + if certStores, ok := s[certStoreType(storeType)]; ok { if certGroup, ok = certStores[namedStore]; ok { return } diff --git a/pkg/verifier/notation/truststore.go b/pkg/verifier/notation/truststore.go index 62aa9c4a3..dec836b1e 100644 --- a/pkg/verifier/notation/truststore.go +++ b/pkg/verifier/notation/truststore.go @@ -33,18 +33,18 @@ var logOpt = logger.Option{ } type trustStore struct { - certPaths []string - certStoresByType certStoresByType + certPaths []string + certStores certStores } func newTrustStore(certPaths []string, verificationCertStores verificationCertStores) (*trustStore, error) { - certStoresByType, err := newCertStoreByType(verificationCertStores) + certStores, err := newCertStoreByType(verificationCertStores) if err != nil { return nil, err } store := &trustStore{ - certPaths: certPaths, - certStoresByType: certStoresByType, + certPaths: certPaths, + certStores: certStores, } return store, nil } @@ -64,7 +64,7 @@ func (s *trustStore) GetCertificates(ctx context.Context, trustStoreType trustst func (s *trustStore) getCertificatesInternal(ctx context.Context, storeType truststore.Type, namedStore string) ([]*x509.Certificate, error) { certs := make([]*x509.Certificate, 0) - certGroup := GetCertGroupFromStore(ctx, s.certStoresByType, storeType, namedStore) + certGroup := s.certStores.GetCertGroupFromStore(ctx, storeType, namedStore) // certs configured for this namedStore overrides cert path if len(certGroup) > 0 { for _, certStore := range certGroup { From 6808dd55a76c1ec5b3368e36a5104b71be74aaab Mon Sep 17 00:00:00 2001 From: junczhu Date: Tue, 18 Jun 2024 01:46:29 +0000 Subject: [PATCH 08/18] feat: update implement of certstorebytype 3 --- Makefile | 8 ++++---- pkg/verifier/notation/truststore_test.go | 3 ++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index d2875c4e9..08509f8f4 100644 --- a/Makefile +++ b/Makefile @@ -542,7 +542,7 @@ e2e-deploy-base-ratify: e2e-notation-setup e2e-notation-leaf-cert-setup e2e-cosi --set-file provider.tls.caCert=${CERT_DIR}/ca.crt \ --set-file provider.tls.caKey=${CERT_DIR}/ca.key \ --set provider.tls.cabundle="$(shell cat ${CERT_DIR}/ca.crt | base64 | tr -d '\n')" \ - --set notationCerts={"$$(cat ~/.config/notation/localkeys/ratify-bats-test.crt)"} \ + --set notationCerts[0]="$$(cat ~/.config/notation/localkeys/ratify-bats-test.crt)" \ --set cosignKeys[0]="$$(cat .staging/cosign/cosign.pub)" \ --set cosign.key="$$(cat .staging/cosign/cosign.pub)" \ --set oras.useHttp=true \ @@ -589,7 +589,7 @@ e2e-helm-deploy-ratify: --set-file provider.tls.caCert=${CERT_DIR}/ca.crt \ --set-file provider.tls.caKey=${CERT_DIR}/ca.key \ --set provider.tls.cabundle="$(shell cat ${CERT_DIR}/ca.crt | base64 | tr -d '\n')" \ - --set notationCerts={"$$(cat ~/.config/notation/localkeys/ratify-bats-test.crt)"} \ + --set notationCerts[0]="$$(cat ~/.config/notation/localkeys/ratify-bats-test.crt)" \ --set cosignKeys[0]="$$(cat .staging/cosign/cosign.pub)" \ --set cosign.key="$$(cat .staging/cosign/cosign.pub)" \ --set oras.useHttp=true \ @@ -608,7 +608,7 @@ e2e-helm-deploy-ratify-without-tls-certs: --set image.tag=test \ --set gatekeeper.version=${GATEKEEPER_VERSION} \ --set featureFlags.RATIFY_CERT_ROTATION=${CERT_ROTATION_ENABLED} \ - --set notationCerts={"$$(cat ~/.config/notation/localkeys/ratify-bats-test.crt)"} \ + --set notationCerts[0]="$$(cat ~/.config/notation/localkeys/ratify-bats-test.crt)" \ --set cosign.key="$$(cat .staging/cosign/cosign.pub)" \ --set cosignKeys[0]="$$(cat .staging/cosign/cosign.pub)" \ --set oras.useHttp=true \ @@ -649,7 +649,7 @@ e2e-helm-deploy-ratify-replica: e2e-helm-deploy-redis e2e-notation-setup e2e-bui --set-file provider.tls.caCert=${CERT_DIR}/ca.crt \ --set-file provider.tls.caKey=${CERT_DIR}/ca.key \ --set provider.tls.cabundle="$(shell cat ${CERT_DIR}/ca.crt | base64 | tr -d '\n')" \ - --set notationCerts={"$$(cat ~/.config/notation/localkeys/ratify-bats-test.crt)"} \ + --set notationCerts[0]="$$(cat ~/.config/notation/localkeys/ratify-bats-test.crt)" \ --set oras.useHttp=true \ --set cosign.enabled=false \ --set-file dockerConfig="mount_config.json" \ diff --git a/pkg/verifier/notation/truststore_test.go b/pkg/verifier/notation/truststore_test.go index 6464747a2..97ff0fc83 100644 --- a/pkg/verifier/notation/truststore_test.go +++ b/pkg/verifier/notation/truststore_test.go @@ -112,7 +112,8 @@ func TestGetCertificates_certPath(t *testing.T) { } trustStore := &trustStore{ - certPaths: []string{tmpFile.Name()}, + certPaths: []string{tmpFile.Name()}, + certStores: certStoresByType{}, } certs, err := trustStore.getCertificatesInternal(context.Background(), truststore.TypeCA, "") if err != nil { From aba5e38bfc0b94c86ef216c25945ede223ed6d67 Mon Sep 17 00:00:00 2001 From: junczhu Date: Thu, 20 Jun 2024 17:45:14 +0000 Subject: [PATCH 09/18] feat: update implement of certstorebytype 4 --- Makefile | 2 +- pkg/verifier/notation/certstoresbytype.go | 49 +++++++++++++++++------ 2 files changed, 37 insertions(+), 14 deletions(-) diff --git a/Makefile b/Makefile index 08509f8f4..7c16b2aa9 100644 --- a/Makefile +++ b/Makefile @@ -608,7 +608,7 @@ e2e-helm-deploy-ratify-without-tls-certs: --set image.tag=test \ --set gatekeeper.version=${GATEKEEPER_VERSION} \ --set featureFlags.RATIFY_CERT_ROTATION=${CERT_ROTATION_ENABLED} \ - --set notationCerts[0]="$$(cat ~/.config/notation/localkeys/ratify-bats-test.crt)" \ + --set notaryCert="$$(cat ~/.config/notation/localkeys/ratify-bats-test.crt)" \ --set cosign.key="$$(cat .staging/cosign/cosign.pub)" \ --set cosignKeys[0]="$$(cat .staging/cosign/cosign.pub)" \ --set oras.useHttp=true \ diff --git a/pkg/verifier/notation/certstoresbytype.go b/pkg/verifier/notation/certstoresbytype.go index 0285c198f..834d30631 100644 --- a/pkg/verifier/notation/certstoresbytype.go +++ b/pkg/verifier/notation/certstoresbytype.go @@ -17,6 +17,7 @@ package notation import ( "context" + "fmt" "github.com/deislabs/ratify/internal/logger" "github.com/notaryproject/notation-go/verifier/truststore" @@ -33,10 +34,26 @@ func (certstoretype certStoreType) String() string { return string(certstoretype) } -// verificationCertStores implements certStores interface +// verificationCertStores describes the configuration of verification certStores +// type verificationCertStores supports new format map[string]map[string][]string +// +// { +// "ca": { +// "certs": {"kv1", "kv2"}, +// }, +// "signingauthority": { +// "certs": {"kv3"} +// }, +// } +// +// type verificationCertStores supports legacy format map[string][]string as well. +// +// { +// "certs": {"kv1", "kv2"}, +// }, type verificationCertStores map[string]interface{} -// certStoresByType describes the configuration of verificationCertStores by certStoreType +// certStoresByType implements certStores interface and place certs under the trustStoreType // // { // "ca": { @@ -53,18 +70,24 @@ func newCertStoreByType(confVerificationCertStores verificationCertStores) (cert s := make(certStoresByType) for certstoretype, certStores := range confVerificationCertStores { s[certStoreType(certstoretype)] = make(map[string][]string) - if convertedCertStores, ok := certStores.(verificationCertStores); ok { - for certStore, certs := range convertedCertStores { - var reformedCerts []string - if convertedCerts, ok := certs.([]interface{}); ok { - for _, cert := range convertedCerts { - if reformedCert, ok := cert.(string); ok { - reformedCerts = append(reformedCerts, reformedCert) - } - } - s[certStoreType(certstoretype)][certStore] = reformedCerts + convertedCertStores, ok := certStores.(verificationCertStores) + if !ok { + return nil, fmt.Errorf("certStores: %s assertion to type verificationCertStores failed", certStores) + } + for certStore, certProviders := range convertedCertStores { + var reformedCerts []string + convertedCerts, ok := certProviders.([]interface{}) + if !ok { + return nil, fmt.Errorf("certProviders: %s assertion to type []interface{} failed", certProviders) + } + for _, certProvider := range convertedCerts { + reformedCert, ok := certProvider.(string) + if !ok { + return nil, fmt.Errorf("certProvider: %s assertion to type string failed", certProvider) } + reformedCerts = append(reformedCerts, reformedCert) } + s[certStoreType(certstoretype)][certStore] = reformedCerts } } return s, nil @@ -77,6 +100,6 @@ func (s certStoresByType) GetCertGroupFromStore(ctx context.Context, storeType t return } } - logger.GetLogger(ctx, logOpt).Debugf("unable to fetch certGroup from namedStore: %+v in type: %v", namedStore, storeType) + logger.GetLogger(ctx, logOpt).Warnf("unable to fetch certGroup from namedStore: %+v in type: %v", namedStore, storeType) return } From ca8000df57f5f20f05e7637f114426fefea4cf0b Mon Sep 17 00:00:00 2001 From: junczhu Date: Fri, 21 Jun 2024 00:20:34 +0000 Subject: [PATCH 10/18] chore: fix the dependency path --- pkg/verifier/notation/certstoresbytype.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/verifier/notation/certstoresbytype.go b/pkg/verifier/notation/certstoresbytype.go index 834d30631..026710b0a 100644 --- a/pkg/verifier/notation/certstoresbytype.go +++ b/pkg/verifier/notation/certstoresbytype.go @@ -19,8 +19,8 @@ import ( "context" "fmt" - "github.com/deislabs/ratify/internal/logger" "github.com/notaryproject/notation-go/verifier/truststore" + "github.com/ratify-project/ratify/internal/logger" ) type certStoreType string From 539ae229fab77b709cbf02e60175b01bacbf50f1 Mon Sep 17 00:00:00 2001 From: junczhu Date: Mon, 24 Jun 2024 08:28:14 +0000 Subject: [PATCH 11/18] chore: rename variables --- pkg/verifier/notation/certStores.go | 4 ++-- pkg/verifier/notation/certstoresbytype.go | 18 +++++++++--------- pkg/verifier/notation/truststore.go | 2 +- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/pkg/verifier/notation/certStores.go b/pkg/verifier/notation/certStores.go index 382ec3eb7..495e26b1c 100644 --- a/pkg/verifier/notation/certStores.go +++ b/pkg/verifier/notation/certStores.go @@ -23,6 +23,6 @@ import ( // certStores is an interface that defines the methods for managing certificate stores. type certStores interface { - // GetCertGroupFromStore returns certain type of cert group from namedStore - GetCertGroupFromStore(ctx context.Context, storeType truststore.Type, namedStore string) (certGroup []string) + // GetCertGroup returns certain type of cert group from namedStore + GetCertGroup(ctx context.Context, storeType truststore.Type, namedStore string) (certGroup []string) } diff --git a/pkg/verifier/notation/certstoresbytype.go b/pkg/verifier/notation/certstoresbytype.go index 026710b0a..008088777 100644 --- a/pkg/verifier/notation/certstoresbytype.go +++ b/pkg/verifier/notation/certstoresbytype.go @@ -66,16 +66,16 @@ type verificationCertStores map[string]interface{} type certStoresByType map[certStoreType]map[string][]string // newCertStoreByType do type assertion and convert certStores configuration into certStoresByType -func newCertStoreByType(confVerificationCertStores verificationCertStores) (certStores, error) { +func newCertStoreByType(confInNewFormat verificationCertStores) (certStores, error) { s := make(certStoresByType) - for certstoretype, certStores := range confVerificationCertStores { + for certstoretype, certStores := range confInNewFormat { s[certStoreType(certstoretype)] = make(map[string][]string) - convertedCertStores, ok := certStores.(verificationCertStores) + storesMapOfInterface, ok := certStores.(verificationCertStores) if !ok { return nil, fmt.Errorf("certStores: %s assertion to type verificationCertStores failed", certStores) } - for certStore, certProviders := range convertedCertStores { - var reformedCerts []string + for certStore, certProviders := range storesMapOfInterface { + var certProviderStr []string convertedCerts, ok := certProviders.([]interface{}) if !ok { return nil, fmt.Errorf("certProviders: %s assertion to type []interface{} failed", certProviders) @@ -85,16 +85,16 @@ func newCertStoreByType(confVerificationCertStores verificationCertStores) (cert if !ok { return nil, fmt.Errorf("certProvider: %s assertion to type string failed", certProvider) } - reformedCerts = append(reformedCerts, reformedCert) + certProviderStr = append(certProviderStr, reformedCert) } - s[certStoreType(certstoretype)][certStore] = reformedCerts + s[certStoreType(certstoretype)][certStore] = certProviderStr } } return s, nil } -// GetCertGroupFromStore returns certain type of certs from namedStore -func (s certStoresByType) GetCertGroupFromStore(ctx context.Context, storeType truststore.Type, namedStore string) (certGroup []string) { +// GetCertGroup returns certain type of certs from namedStore +func (s certStoresByType) GetCertGroup(ctx context.Context, storeType truststore.Type, namedStore string) (certGroup []string) { if certStores, ok := s[certStoreType(storeType)]; ok { if certGroup, ok = certStores[namedStore]; ok { return diff --git a/pkg/verifier/notation/truststore.go b/pkg/verifier/notation/truststore.go index e9f823ac0..cb71ac684 100644 --- a/pkg/verifier/notation/truststore.go +++ b/pkg/verifier/notation/truststore.go @@ -64,7 +64,7 @@ func (s *trustStore) GetCertificates(ctx context.Context, trustStoreType trustst func (s *trustStore) getCertificatesInternal(ctx context.Context, storeType truststore.Type, namedStore string) ([]*x509.Certificate, error) { certs := make([]*x509.Certificate, 0) - certGroup := s.certStores.GetCertGroupFromStore(ctx, storeType, namedStore) + certGroup := s.certStores.GetCertGroup(ctx, storeType, namedStore) // certs configured for this namedStore overrides cert path if len(certGroup) > 0 { for _, certStore := range certGroup { From 4d27581c5b8b3b92ccb31667f46a57d9748d7893 Mon Sep 17 00:00:00 2001 From: junczhu Date: Mon, 24 Jun 2024 08:34:05 +0000 Subject: [PATCH 12/18] chore: rename variables 2 --- pkg/verifier/notation/certstoresbytype.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/verifier/notation/certstoresbytype.go b/pkg/verifier/notation/certstoresbytype.go index 008088777..455f13375 100644 --- a/pkg/verifier/notation/certstoresbytype.go +++ b/pkg/verifier/notation/certstoresbytype.go @@ -75,19 +75,19 @@ func newCertStoreByType(confInNewFormat verificationCertStores) (certStores, err return nil, fmt.Errorf("certStores: %s assertion to type verificationCertStores failed", certStores) } for certStore, certProviders := range storesMapOfInterface { - var certProviderStr []string + var certProvidersStr []string convertedCerts, ok := certProviders.([]interface{}) if !ok { return nil, fmt.Errorf("certProviders: %s assertion to type []interface{} failed", certProviders) } for _, certProvider := range convertedCerts { - reformedCert, ok := certProvider.(string) + certProviderStr, ok := certProvider.(string) if !ok { return nil, fmt.Errorf("certProvider: %s assertion to type string failed", certProvider) } - certProviderStr = append(certProviderStr, reformedCert) + certProvidersStr = append(certProvidersStr, certProviderStr) } - s[certStoreType(certstoretype)][certStore] = certProviderStr + s[certStoreType(certstoretype)][certStore] = certProvidersStr } } return s, nil From 3e74a8914b8cce1efcff57fd5a144779ce72fa76 Mon Sep 17 00:00:00 2001 From: Juncheng Zhu <74894646+junczhu@users.noreply.github.com> Date: Tue, 25 Jun 2024 00:00:38 +0800 Subject: [PATCH 13/18] Update pkg/verifier/notation/certstoresbytype.go Co-authored-by: Binbin Li Signed-off-by: Juncheng Zhu <74894646+junczhu@users.noreply.github.com> --- pkg/verifier/notation/certstoresbytype.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/verifier/notation/certstoresbytype.go b/pkg/verifier/notation/certstoresbytype.go index 455f13375..fdc88d5dd 100644 --- a/pkg/verifier/notation/certstoresbytype.go +++ b/pkg/verifier/notation/certstoresbytype.go @@ -65,7 +65,7 @@ type verificationCertStores map[string]interface{} // } type certStoresByType map[certStoreType]map[string][]string -// newCertStoreByType do type assertion and convert certStores configuration into certStoresByType +// newCertStoreByType performs type assertion and converts certificate stores configuration into certStoresByType func newCertStoreByType(confInNewFormat verificationCertStores) (certStores, error) { s := make(certStoresByType) for certstoretype, certStores := range confInNewFormat { From 0fc8fb2d67f93a9193e06b8c22d8b2e888863166 Mon Sep 17 00:00:00 2001 From: Juncheng Zhu <74894646+junczhu@users.noreply.github.com> Date: Tue, 25 Jun 2024 00:01:22 +0800 Subject: [PATCH 14/18] Update pkg/verifier/notation/certstoresbytype.go Co-authored-by: Binbin Li Signed-off-by: Juncheng Zhu <74894646+junczhu@users.noreply.github.com> --- pkg/verifier/notation/certstoresbytype.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/pkg/verifier/notation/certstoresbytype.go b/pkg/verifier/notation/certstoresbytype.go index fdc88d5dd..a62895b05 100644 --- a/pkg/verifier/notation/certstoresbytype.go +++ b/pkg/verifier/notation/certstoresbytype.go @@ -68,26 +68,26 @@ type certStoresByType map[certStoreType]map[string][]string // newCertStoreByType performs type assertion and converts certificate stores configuration into certStoresByType func newCertStoreByType(confInNewFormat verificationCertStores) (certStores, error) { s := make(certStoresByType) - for certstoretype, certStores := range confInNewFormat { + for certstoretype, storeData:= range confInNewFormat { s[certStoreType(certstoretype)] = make(map[string][]string) - storesMapOfInterface, ok := certStores.(verificationCertStores) + parsedStoreData, ok := storeData.(verificationCertStores) if !ok { - return nil, fmt.Errorf("certStores: %s assertion to type verificationCertStores failed", certStores) + return nil, fmt.Errorf("certStores: %s assertion to type verificationCertStores failed", storeData) } - for certStore, certProviders := range storesMapOfInterface { - var certProvidersStr []string - convertedCerts, ok := certProviders.([]interface{}) + for storeName, certProviderList := range parsedStoreData { + var certProviderNames []string + parsedCertProviders, ok := certProviderList.([]interface{}) if !ok { return nil, fmt.Errorf("certProviders: %s assertion to type []interface{} failed", certProviders) } - for _, certProvider := range convertedCerts { - certProviderStr, ok := certProvider.(string) + for _, certProvider := range parsedCertProviders { + certProviderName, ok := certProvider.(string) if !ok { return nil, fmt.Errorf("certProvider: %s assertion to type string failed", certProvider) } - certProvidersStr = append(certProvidersStr, certProviderStr) + certProviderNames = append(certProviderNames , certProviderName) } - s[certStoreType(certstoretype)][certStore] = certProvidersStr + s[certStoreType(certstoretype)][storeName] = certProviderNames } } return s, nil From 97e3706e620e7e9690e560df19af8b8752139ac5 Mon Sep 17 00:00:00 2001 From: junczhu Date: Tue, 25 Jun 2024 03:09:31 +0000 Subject: [PATCH 15/18] chore: rename variables 3 --- pkg/verifier/notation/certstoresbytype.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/verifier/notation/certstoresbytype.go b/pkg/verifier/notation/certstoresbytype.go index a62895b05..db9d37d3d 100644 --- a/pkg/verifier/notation/certstoresbytype.go +++ b/pkg/verifier/notation/certstoresbytype.go @@ -68,7 +68,7 @@ type certStoresByType map[certStoreType]map[string][]string // newCertStoreByType performs type assertion and converts certificate stores configuration into certStoresByType func newCertStoreByType(confInNewFormat verificationCertStores) (certStores, error) { s := make(certStoresByType) - for certstoretype, storeData:= range confInNewFormat { + for certstoretype, storeData := range confInNewFormat { s[certStoreType(certstoretype)] = make(map[string][]string) parsedStoreData, ok := storeData.(verificationCertStores) if !ok { @@ -78,16 +78,16 @@ func newCertStoreByType(confInNewFormat verificationCertStores) (certStores, err var certProviderNames []string parsedCertProviders, ok := certProviderList.([]interface{}) if !ok { - return nil, fmt.Errorf("certProviders: %s assertion to type []interface{} failed", certProviders) + return nil, fmt.Errorf("certProviderList: %s assertion to type []interface{} failed", certProviderList) } for _, certProvider := range parsedCertProviders { certProviderName, ok := certProvider.(string) if !ok { return nil, fmt.Errorf("certProvider: %s assertion to type string failed", certProvider) } - certProviderNames = append(certProviderNames , certProviderName) + certProviderNames = append(certProviderNames, certProviderName) } - s[certStoreType(certstoretype)][storeName] = certProviderNames + s[certStoreType(certstoretype)][storeName] = certProviderNames } } return s, nil From adf497867bcc2448fe6a43b58012f1df601d7a51 Mon Sep 17 00:00:00 2001 From: junczhu Date: Tue, 25 Jun 2024 03:45:07 +0000 Subject: [PATCH 16/18] test: increase test cov 1 --- .../notation/certstoresbytype_test.go | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 pkg/verifier/notation/certstoresbytype_test.go diff --git a/pkg/verifier/notation/certstoresbytype_test.go b/pkg/verifier/notation/certstoresbytype_test.go new file mode 100644 index 000000000..1ae34ea14 --- /dev/null +++ b/pkg/verifier/notation/certstoresbytype_test.go @@ -0,0 +1,61 @@ +/* +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 notation + +import "testing" + +func TestNewCertStoreByTypInvalidInput(t *testing.T) { + tests := []struct { + name string + conf verificationCertStores + expectErr bool + }{ + { + name: "", + conf: verificationCertStores{ + trustStoreTypeCA: []string{}, + }, + expectErr: true, + }, + { + name: "", + conf: verificationCertStores{ + trustStoreTypeCA: verificationCertStores{ + "certstore1": "akv1", + "certstore2": []interface{}{"akv3", "akv4"}, + }, + }, + expectErr: true, + }, + { + name: "", + conf: verificationCertStores{ + trustStoreTypeCA: verificationCertStores{ + "certstore1": []interface{}{"akv1", []string{}}, + }, + }, + expectErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := newCertStoreByType(tt.conf) + if (err != nil) != tt.expectErr { + t.Errorf("error = %v, expectErr = %v", err, tt.expectErr) + } + }) + } +} From 166143d4222179dc5ac7708a92f88daa0ae0994e Mon Sep 17 00:00:00 2001 From: junczhu Date: Tue, 25 Jun 2024 07:25:51 +0000 Subject: [PATCH 17/18] chore: fix typo --- pkg/verifier/notation/certstoresbytype_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/verifier/notation/certstoresbytype_test.go b/pkg/verifier/notation/certstoresbytype_test.go index 1ae34ea14..53ed6b05c 100644 --- a/pkg/verifier/notation/certstoresbytype_test.go +++ b/pkg/verifier/notation/certstoresbytype_test.go @@ -17,7 +17,7 @@ package notation import "testing" -func TestNewCertStoreByTypInvalidInput(t *testing.T) { +func TestNewCertStoreByTypeInvalidInput(t *testing.T) { tests := []struct { name string conf verificationCertStores From 75cc2b084f394a8c0188841a981507367c1f5191 Mon Sep 17 00:00:00 2001 From: junczhu Date: Tue, 25 Jun 2024 07:28:45 +0000 Subject: [PATCH 18/18] chore: fix typo 2 --- pkg/verifier/notation/certstoresbytype_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/verifier/notation/certstoresbytype_test.go b/pkg/verifier/notation/certstoresbytype_test.go index 53ed6b05c..dc3d0dab2 100644 --- a/pkg/verifier/notation/certstoresbytype_test.go +++ b/pkg/verifier/notation/certstoresbytype_test.go @@ -24,14 +24,14 @@ func TestNewCertStoreByTypeInvalidInput(t *testing.T) { expectErr bool }{ { - name: "", + name: "invalid certStores type", conf: verificationCertStores{ trustStoreTypeCA: []string{}, }, expectErr: true, }, { - name: "", + name: "invalid certProviderList type", conf: verificationCertStores{ trustStoreTypeCA: verificationCertStores{ "certstore1": "akv1", @@ -41,7 +41,7 @@ func TestNewCertStoreByTypeInvalidInput(t *testing.T) { expectErr: true, }, { - name: "", + name: "invalid certProvider type", conf: verificationCertStores{ trustStoreTypeCA: verificationCertStores{ "certstore1": []interface{}{"akv1", []string{}},