Skip to content

Commit

Permalink
feat: add compatibility check in KMP while fetching certs/keys [multi…
Browse files Browse the repository at this point in the history
…-tenancy PR 6] (#1395)
  • Loading branch information
binbin-li authored Apr 25, 2024
1 parent c5f252d commit 3d40b97
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 19 deletions.
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

0 comments on commit 3d40b97

Please sign in to comment.