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: add compatibility check in KMP while fetching certs/keys [multi-tenancy PR 6] #1395

Merged
merged 7 commits into from
Apr 25, 2024
15 changes: 8 additions & 7 deletions pkg/controllers/keymanagementprovider_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import (

configv1beta1 "github.com/deislabs/ratify/api/v1beta1"
c "github.com/deislabs/ratify/config"
"github.com/deislabs/ratify/pkg/keymanagementprovider"
kmp "github.com/deislabs/ratify/pkg/keymanagementprovider"
"github.com/deislabs/ratify/pkg/keymanagementprovider/config"
"github.com/deislabs/ratify/pkg/keymanagementprovider/factory"
"github.com/deislabs/ratify/pkg/keymanagementprovider/types"
Expand All @@ -60,7 +60,8 @@ func (r *KeyManagementProviderReconciler) Reconcile(ctx context.Context, req ctr
if err := r.Get(ctx, req.NamespacedName, &keyManagementProvider); err != nil {
if apierrors.IsNotFound(err) {
logger.Infof("deletion detected, removing key management provider %v", resource)
keymanagementprovider.DeleteCertificatesFromMap(resource)
kmp.DeleteCertificatesFromMap(resource)
kmp.DeleteKeysFromMap(resource)
} else {
logger.Error(err, "unable to fetch key management provider")
}
Expand Down Expand Up @@ -103,8 +104,8 @@ func (r *KeyManagementProviderReconciler) Reconcile(ctx context.Context, req ctr
writeKMProviderStatus(ctx, r, &keyManagementProvider, logger, isFetchSuccessful, err.Error(), lastFetchedTime, nil)
return ctrl.Result{}, fmt.Errorf("Error fetching keys in KMProvider %v with %v provider, error: %w", resource, keyManagementProvider.Spec.Type, err)
}
keymanagementprovider.SetCertificatesInMap(resource, certificates)
keymanagementprovider.SetKeysInMap(resource, keyManagementProvider.Spec.Type, keys)
kmp.SetCertificatesInMap(resource, certificates)
kmp.SetKeysInMap(resource, keyManagementProvider.Spec.Type, keys)
// merge certificates and keys status into one
maps.Copy(keyAttributes, certAttributes)
isFetchSuccessful = true
Expand All @@ -130,7 +131,7 @@ func (r *KeyManagementProviderReconciler) SetupWithManager(mgr ctrl.Manager) err
}

// specToKeyManagementProvider creates KeyManagementProviderProvider from KeyManagementProviderSpec config
func specToKeyManagementProvider(spec configv1beta1.KeyManagementProviderSpec) (keymanagementprovider.KeyManagementProvider, error) {
func specToKeyManagementProvider(spec configv1beta1.KeyManagementProviderSpec) (kmp.KeyManagementProvider, error) {
kmProviderConfig, err := rawToKeyManagementProviderConfig(spec.Parameters.Raw, spec.Type)
if err != nil {
return nil, fmt.Errorf("failed to parse key management provider config: %w", err)
Expand Down Expand Up @@ -162,7 +163,7 @@ func rawToKeyManagementProviderConfig(raw []byte, keyManagamentSystemName string
}

// writeKMProviderStatus updates the status of the key management provider resource
func writeKMProviderStatus(ctx context.Context, r client.StatusClient, keyManagementProvider *configv1beta1.KeyManagementProvider, logger *logrus.Entry, isSuccess bool, errorString string, operationTime metav1.Time, kmProviderStatus keymanagementprovider.KeyManagementProviderStatus) {
func writeKMProviderStatus(ctx context.Context, r client.StatusClient, keyManagementProvider *configv1beta1.KeyManagementProvider, logger *logrus.Entry, isSuccess bool, errorString string, operationTime metav1.Time, kmProviderStatus kmp.KeyManagementProviderStatus) {
if isSuccess {
updateKMProviderSuccessStatus(keyManagementProvider, &operationTime, kmProviderStatus)
} else {
Expand All @@ -188,7 +189,7 @@ func updateKMProviderErrorStatus(keyManagementProvider *configv1beta1.KeyManagem

// updateKMProviderSuccessStatus updates the key management provider status if status argument is non nil
// Success status includes last fetched time and other provider-specific properties
func updateKMProviderSuccessStatus(keyManagementProvider *configv1beta1.KeyManagementProvider, lastOperationTime *metav1.Time, kmProviderStatus keymanagementprovider.KeyManagementProviderStatus) {
func updateKMProviderSuccessStatus(keyManagementProvider *configv1beta1.KeyManagementProvider, lastOperationTime *metav1.Time, kmProviderStatus kmp.KeyManagementProviderStatus) {
keyManagementProvider.Status.IsSuccess = true
keyManagementProvider.Status.Error = ""
keyManagementProvider.Status.BriefError = ""
Expand Down
17 changes: 15 additions & 2 deletions pkg/keymanagementprovider/keymanagementprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,10 @@ func SetCertificatesInMap(resource string, certs map[KMPMapKey][]*x509.Certifica
}

// GetCertificatesFromMap gets the certificates from the map and returns an empty map of certificate arrays if not found
func GetCertificatesFromMap(resource string) map[KMPMapKey][]*x509.Certificate {
func GetCertificatesFromMap(ctx context.Context, resource string) map[KMPMapKey][]*x509.Certificate {
if !isCompatibleNamespace(ctx, resource) {
return map[KMPMapKey][]*x509.Certificate{}
}
certs, ok := certificatesMap.Load(resource)
if !ok {
return map[KMPMapKey][]*x509.Certificate{}
Expand Down Expand Up @@ -143,7 +146,10 @@ func SetKeysInMap(resource string, providerType string, keys map[KMPMapKey]crypt
}

// GetKeysFromMap gets the keys from the map and returns an empty map with false boolean if not found
func GetKeysFromMap(resource string) (map[KMPMapKey]PublicKey, bool) {
func GetKeysFromMap(ctx context.Context, resource string) (map[KMPMapKey]PublicKey, bool) {
if !isCompatibleNamespace(ctx, resource) {
return map[KMPMapKey]PublicKey{}, false
}
keys, ok := keyMap.Load(resource)
if !ok {
return map[KMPMapKey]PublicKey{}, false
Expand All @@ -155,3 +161,10 @@ func GetKeysFromMap(resource string) (map[KMPMapKey]PublicKey, bool) {
func DeleteKeysFromMap(resource string) {
keyMap.Delete(resource)
}

// Namespaced verifiers could access both cluster-scoped and namespaced certStores.
// But cluster-wide verifiers could only access cluster-scoped certStores.
// TODO: current implementation always returns true. Check the namespace once we support multi-tenancy.
func isCompatibleNamespace(_ context.Context, _ string) bool {
return true
}
9 changes: 5 additions & 4 deletions pkg/keymanagementprovider/keymanagementprovider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ limitations under the License.
package keymanagementprovider

import (
"context"
"crypto"
"crypto/rsa"
"crypto/x509"
Expand Down Expand Up @@ -142,7 +143,7 @@ func TestSetCertificatesInMap(t *testing.T) {
func TestGetCertificatesFromMap(t *testing.T) {
certificatesMap.Delete("test")
SetCertificatesInMap("test", map[KMPMapKey][]*x509.Certificate{{}: {{Raw: []byte("testcert")}}})
certs := GetCertificatesFromMap("test")
certs := GetCertificatesFromMap(context.Background(), "test")
if len(certs) != 1 {
t.Fatalf("certificates should have been fetched from the map")
}
Expand All @@ -151,7 +152,7 @@ func TestGetCertificatesFromMap(t *testing.T) {
// TestGetCertificatesFromMap_FailedToFetch checks if certificates are fetched from the map
func TestGetCertificatesFromMap_FailedToFetch(t *testing.T) {
certificatesMap.Delete("test")
certs := GetCertificatesFromMap("test")
certs := GetCertificatesFromMap(context.Background(), "test")
if len(certs) != 0 {
t.Fatalf("certificates should not have been fetched from the map")
}
Expand Down Expand Up @@ -188,7 +189,7 @@ func TestSetKeysInMap(t *testing.T) {
func TestGetKeysFromMap(t *testing.T) {
keyMap.Delete("test")
SetKeysInMap("test", "", map[KMPMapKey]crypto.PublicKey{{}: &rsa.PublicKey{}})
keys, _ := GetKeysFromMap("test")
keys, _ := GetKeysFromMap(context.Background(), "test")
if len(keys) != 1 {
t.Fatalf("keys should have been fetched from the map")
}
Expand All @@ -197,7 +198,7 @@ func TestGetKeysFromMap(t *testing.T) {
// TestGetKeysFromMap_FailedToFetch checks if keys fail to fetch from map
func TestGetKeysFromMap_FailedToFetch(t *testing.T) {
keyMap.Delete("test")
keys, _ := GetKeysFromMap("test")
keys, _ := GetKeysFromMap(context.Background(), "test")
if len(keys) != 0 {
t.Fatalf("keys should not have been fetched from the map")
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/verifier/cosign/cosign.go
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,7 @@ func getKeysMapsDefault(ctx context.Context, trustPolicies *TrustPolicies, refer
logger.GetLogger(ctx, logOpt).Debugf("selected trust policy %s for reference %s", trustPolicy.GetName(), reference)

// get the map of keys for that reference
keysMap, err := trustPolicy.GetKeys(namespace)
keysMap, err := trustPolicy.GetKeys(ctx, namespace)
if err != nil {
return nil, cosign.CheckOpts{}, err
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/verifier/cosign/trustpolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ type trustPolicy struct {

type TrustPolicy interface {
GetName() string
GetKeys(string) (map[PKKey]keymanagementprovider.PublicKey, error)
GetKeys(ctx context.Context, namespace string) (map[PKKey]keymanagementprovider.PublicKey, error)
GetScopes() []string
GetCosignOpts(context.Context) (cosign.CheckOpts, error)
}
Expand Down Expand Up @@ -138,7 +138,7 @@ func (tp *trustPolicy) GetName() string {
}

// GetKeys returns the public keys defined in the trust policy
func (tp *trustPolicy) GetKeys(namespace string) (map[PKKey]keymanagementprovider.PublicKey, error) {
func (tp *trustPolicy) GetKeys(ctx context.Context, namespace string) (map[PKKey]keymanagementprovider.PublicKey, error) {
keyMap := make(map[PKKey]keymanagementprovider.PublicKey)
// preload the local keys into the map of keys to be returned
for key, pubKey := range tp.localKeys {
Expand All @@ -153,7 +153,7 @@ func (tp *trustPolicy) GetKeys(namespace string) (map[PKKey]keymanagementprovide
// must prepend namespace to key management provider name if not provided since namespace is prepended during key management provider intialization
namespacedKMP := prependNamespaceToKMPName(keyConfig.Provider, namespace)
// get the key management provider resource which contains a map of keys
kmpResource, ok := keymanagementprovider.GetKeysFromMap(namespacedKMP)
kmpResource, ok := keymanagementprovider.GetKeysFromMap(ctx, namespacedKMP)
if !ok {
return nil, re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithPluginName(tp.verifierName).WithDetail(fmt.Sprintf("trust policy %s failed: key management provider %s not found", tp.config.Name, namespacedKMP))
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/verifier/cosign/trustpolicy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ limitations under the License.
package cosign

import (
"context"
"crypto"
"crypto/ecdsa"
"testing"
Expand Down Expand Up @@ -188,7 +189,7 @@ func TestGetKeys(t *testing.T) {
if err != nil {
t.Fatalf("expected no error, got %v", err)
}
keys, err := trustPolicy.GetKeys("ns")
keys, err := trustPolicy.GetKeys(context.Background(), "ns")
if (err != nil) != tt.wantErr {
t.Fatalf("expected %v, got %v", tt.wantErr, err)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/verifier/notation/truststore.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func (s trustStore) getCertificatesInternal(ctx context.Context, namedStore stri
if certGroup := s.certStores[namedStore]; len(certGroup) > 0 {
for _, certStore := range certGroup {
logger.GetLogger(ctx, logOpt).Debugf("truststore getting certStore %v", certStore)
result := keymanagementprovider.FlattenKMPMap(keymanagementprovider.GetCertificatesFromMap(certStore))
result := keymanagementprovider.FlattenKMPMap(keymanagementprovider.GetCertificatesFromMap(ctx, certStore))
// notation verifier does not consider specific named/versioned certificates within a key management provider resource
if len(result) == 0 {
logger.GetLogger(ctx, logOpt).Warnf("no certificate fetched for Key Management Provider %+v", certStore)
Expand Down
Loading