diff --git a/pkg/apis/config/image_policies_test.go b/pkg/apis/config/image_policies_test.go index 7ecea12521d5..7c9c4015c44c 100644 --- a/pkg/apis/config/image_policies_test.go +++ b/pkg/apis/config/image_policies_test.go @@ -158,7 +158,6 @@ func checkPublicKey(t *testing.T, gotKey *ecdsa.PublicKey) { // pem.EncodeToMemory has an extra newline at the end got := strings.TrimSuffix(string(pemBytes), "\n") - if got != inlineKeyData { t.Errorf("Did not get what I wanted %s, got %s", inlineKeyData, string(pemBytes)) } diff --git a/pkg/cosign/kubernetes/webhook/clusterimagepolicy/clusterimagepolicy_types.go b/pkg/cosign/kubernetes/webhook/clusterimagepolicy/clusterimagepolicy_types.go index 9ac247c04c8f..3de82474416b 100644 --- a/pkg/cosign/kubernetes/webhook/clusterimagepolicy/clusterimagepolicy_types.go +++ b/pkg/cosign/kubernetes/webhook/clusterimagepolicy/clusterimagepolicy_types.go @@ -15,12 +15,25 @@ package clusterimagepolicy import ( + "context" + "crypto" "crypto/ecdsa" "crypto/x509" "encoding/json" "encoding/pem" + "fmt" + "strings" "github.com/sigstore/cosign/pkg/apis/cosigned/v1alpha1" + "github.com/sigstore/cosign/pkg/apis/utils" + sigs "github.com/sigstore/cosign/pkg/signature" + "github.com/sigstore/sigstore/pkg/signature/kms" + signatureoptions "github.com/sigstore/sigstore/pkg/signature/options" + corev1listers "k8s.io/client-go/listers/core/v1" + "knative.dev/pkg/apis" + "knative.dev/pkg/logging" + "knative.dev/pkg/system" + "knative.dev/pkg/tracker" ) // ClusterImagePolicy defines the images that go through verification @@ -37,7 +50,7 @@ type Authority struct { // +optional Key *KeyRef `json:"key,omitempty"` // +optional - Keyless *v1alpha1.KeylessRef `json:"keyless,omitempty"` + Keyless *KeylessRef `json:"keyless,omitempty"` // +optional Sources []v1alpha1.Source `json:"source,omitempty"` // +optional @@ -50,15 +63,21 @@ type KeyRef struct { // Data contains the inline public key // +optional Data string `json:"data,omitempty"` - // KMS contains the KMS url of the public key - // +optional - KMS string `json:"kms,omitempty"` // PublicKeys are not marshalled because JSON unmarshalling // errors for *big.Int // +optional PublicKeys []*ecdsa.PublicKey `json:"-"` } +type KeylessRef struct { + // +optional + URL *apis.URL `json:"url,omitempty"` + // +optional + Identities []v1alpha1.Identity `json:"identities,omitempty"` + // +optional + CACert *KeyRef `json:"ca-cert,omitempty"` +} + // UnmarshalJSON populates the PublicKeys using Data because // JSON unmashalling errors for *big.Int func (k *KeyRef) UnmarshalJSON(data []byte) error { @@ -73,7 +92,7 @@ func (k *KeyRef) UnmarshalJSON(data []byte) error { k.Data = ret["data"] if ret["data"] != "" { - publicKeys, err = convertKeyDataToPublicKeys(ret["data"]) + publicKeys, err = ConvertKeyDataToPublicKeys(context.Background(), ret["data"]) if err != nil { return err } @@ -84,9 +103,103 @@ func (k *KeyRef) UnmarshalJSON(data []byte) error { return nil } -func convertKeyDataToPublicKeys(pubKey string) ([]*ecdsa.PublicKey, error) { +func ConvertClusterImagePolicyV1alpha1ToWebhook(ctx context.Context, in *v1alpha1.ClusterImagePolicy, rtracker tracker.Interface, secretLister corev1listers.SecretLister) (*ClusterImagePolicy, error) { + copyIn := in.DeepCopy() + outAuthorities := make([]Authority, 0) + for _, authority := range copyIn.Spec.Authorities { + outAuthority, err := convertAuthorityV1Alpha1ToWebhook(ctx, copyIn, authority, rtracker, secretLister) + if err != nil { + return nil, err + } + outAuthorities = append(outAuthorities, *outAuthority) + } + + return &ClusterImagePolicy{ + Images: copyIn.Spec.Images, + Authorities: outAuthorities, + }, nil +} + +// convertAuthorityV1Alpha1ToWebhook will evaluate Key and Keyless +// references and return the converted Authority +func convertAuthorityV1Alpha1ToWebhook(ctx context.Context, cipIn *v1alpha1.ClusterImagePolicy, in v1alpha1.Authority, rtracker tracker.Interface, secretLister corev1listers.SecretLister) (*Authority, error) { + var err error + var keyRef *KeyRef + var keylessRef *KeylessRef + + if in.Key != nil { + if keyRef, err = convertKeyRefV1Alpha1ToWebhook(ctx, in.Key, cipIn, rtracker, secretLister); err != nil { + return nil, err + } + } + + if in.Keyless != nil { + if keylessRef, err = convertKeylessRefV1Alpha1ToWebhook(ctx, in.Keyless, cipIn, rtracker, secretLister); err != nil { + return nil, err + } + } + + return &Authority{ + Key: keyRef, + Keyless: keylessRef, + Sources: in.Sources, + CTLog: in.CTLog, + }, nil +} + +// convertKeyRefV1Alpha1ToWebhook will evaluate secretRef or KMS +// and return KeyRef with inlined data +func convertKeyRefV1Alpha1ToWebhook(ctx context.Context, in *v1alpha1.KeyRef, cipIn *v1alpha1.ClusterImagePolicy, rtracker tracker.Interface, secretLister corev1listers.SecretLister) (*KeyRef, error) { + var data string + var err error + + if in != nil { + data = in.Data + } + + if in != nil && in.SecretRef != nil { + if data, err = dataAndTrackSecret(ctx, cipIn, in, rtracker, secretLister); err != nil { + logging.FromContext(ctx).Errorf("Failed to read secret %q: %v", in.SecretRef.Name, err) + return nil, err + } + } else if in != nil && in.KMS != "" { + if strings.Contains(in.KMS, "://") { + if data, err = GetKMSPublicKey(ctx, in.KMS); err != nil { + return nil, err + } + } + } + + return &KeyRef{ + Data: data, + }, nil +} + +// convertKeylessRefV1Alpha1ToWebhook will evaluate CACert KeyRef +// Returns new KeylessRef with new KeyRef with inlined data +func convertKeylessRefV1Alpha1ToWebhook(ctx context.Context, in *v1alpha1.KeylessRef, cipIn *v1alpha1.ClusterImagePolicy, rtracker tracker.Interface, secretLister corev1listers.SecretLister) (*KeylessRef, error) { + var CACertRef *KeyRef + var err error + + CACertRef, err = convertKeyRefV1Alpha1ToWebhook(ctx, in.CACert, cipIn, rtracker, secretLister) + if err != nil { + return nil, err + } + + return &KeylessRef{ + URL: in.URL, + Identities: in.Identities, + CACert: CACertRef, + }, nil +} + +func ConvertKeyDataToPublicKeys(ctx context.Context, pubKey string) ([]*ecdsa.PublicKey, error) { keys := []*ecdsa.PublicKey{} + if ctx != nil { + logging.FromContext(ctx).Debugf("Got public key: %v", pubKey) + } + pems := parsePems([]byte(pubKey)) for _, p := range pems { key, err := x509.ParsePKIXPublicKey(p.Bytes) @@ -110,3 +223,54 @@ func parsePems(b []byte) []*pem.Block { } return pems } + +// dataAndTrackSecret will take in a KeyRef and tries to read the Secret, finding the +// first key from it and return the data. +// Additionally, we set up a tracker so we will be notified if the secret +// is modified. +// There's still some discussion about how to handle multiple keys in a secret +// for now, just grab one from it. For reference, the discussion is here: +// TODO(vaikas): https://github.com/sigstore/cosign/issues/1573 +func dataAndTrackSecret(ctx context.Context, cip *v1alpha1.ClusterImagePolicy, keyref *v1alpha1.KeyRef, rtracker tracker.Interface, secretLister corev1listers.SecretLister) (string, error) { + if err := rtracker.TrackReference(tracker.Reference{ + APIVersion: "v1", + Kind: "Secret", + Namespace: system.Namespace(), + Name: keyref.SecretRef.Name, + }, cip); err != nil { + return "", fmt.Errorf("failed to track changes to secret %q : %w", keyref.SecretRef.Name, err) + } + secret, err := secretLister.Secrets(system.Namespace()).Get(keyref.SecretRef.Name) + if err != nil { + return "", err + } + if len(secret.Data) == 0 { + return "", fmt.Errorf("secret %q contains no data", keyref.SecretRef.Name) + } + if len(secret.Data) > 1 { + return "", fmt.Errorf("secret %q contains multiple data entries, only one is supported", keyref.SecretRef.Name) + } + for k, v := range secret.Data { + logging.FromContext(ctx).Infof("inlining secret %q key %q", keyref.SecretRef.Name, k) + if !utils.IsValidKey(v) { + return "", fmt.Errorf("secret %q contains an invalid public key", keyref.SecretRef.Name) + } + + return string(v), nil + } + return "", nil +} + +// GetKMSPublicKey returns the public key as a string from the configured KMS service using the key ID +func GetKMSPublicKey(ctx context.Context, keyID string) (string, error) { + kmsSigner, err := kms.Get(ctx, keyID, crypto.SHA256) + if err != nil { + logging.FromContext(ctx).Errorf("Failed to read KMS key ID %q: %v", keyID, err) + return "", err + } + pemBytes, err := sigs.PublicKeyPem(kmsSigner, signatureoptions.WithContext(ctx)) + if err != nil { + return "", err + } + return string(pemBytes), nil +} diff --git a/pkg/cosign/kubernetes/webhook/validator_test.go b/pkg/cosign/kubernetes/webhook/validator_test.go index ad9b3ac56d84..f66772f4766e 100644 --- a/pkg/cosign/kubernetes/webhook/validator_test.go +++ b/pkg/cosign/kubernetes/webhook/validator_test.go @@ -272,7 +272,7 @@ UoJou2P8sbDxpLiE/v3yLw1/jyOrCPWYHWFXnyyeGlkgSVefG54tNoK7Uw== }}, Authorities: []webhookcip.Authority{ { - Keyless: &v1alpha1.KeylessRef{ + Keyless: &webhookcip.KeylessRef{ URL: badURL, }, }, @@ -315,7 +315,7 @@ UoJou2P8sbDxpLiE/v3yLw1/jyOrCPWYHWFXnyyeGlkgSVefG54tNoK7Uw== }}, Authorities: []webhookcip.Authority{ { - Keyless: &v1alpha1.KeylessRef{ + Keyless: &webhookcip.KeylessRef{ URL: fulcioURL, }, }, @@ -358,7 +358,7 @@ UoJou2P8sbDxpLiE/v3yLw1/jyOrCPWYHWFXnyyeGlkgSVefG54tNoK7Uw== }}, Authorities: []webhookcip.Authority{ { - Keyless: &v1alpha1.KeylessRef{ + Keyless: &webhookcip.KeylessRef{ URL: fulcioURL, }, CTLog: &v1alpha1.TLog{ diff --git a/pkg/reconciler/clusterimagepolicy/clusterimagepolicy.go b/pkg/reconciler/clusterimagepolicy/clusterimagepolicy.go index 7b95067a3af6..f96dd541b017 100644 --- a/pkg/reconciler/clusterimagepolicy/clusterimagepolicy.go +++ b/pkg/reconciler/clusterimagepolicy/clusterimagepolicy.go @@ -16,17 +16,9 @@ package clusterimagepolicy import ( "context" - "crypto" - "crypto/ecdsa" - "crypto/x509" - "encoding/json" - "encoding/pem" - "fmt" - "strings" "github.com/sigstore/cosign/pkg/apis/config" "github.com/sigstore/cosign/pkg/apis/cosigned/v1alpha1" - "github.com/sigstore/cosign/pkg/apis/utils" clusterimagepolicyreconciler "github.com/sigstore/cosign/pkg/client/injection/reconciler/cosigned/v1alpha1/clusterimagepolicy" webhookcip "github.com/sigstore/cosign/pkg/cosign/kubernetes/webhook/clusterimagepolicy" "github.com/sigstore/cosign/pkg/reconciler/clusterimagepolicy/resources" @@ -41,10 +33,6 @@ import ( "knative.dev/pkg/system" "knative.dev/pkg/tracker" - sigs "github.com/sigstore/cosign/pkg/signature" - "github.com/sigstore/sigstore/pkg/signature/kms" - signatureoptions "github.com/sigstore/sigstore/pkg/signature/options" - // Register the provider-specific plugins _ "github.com/sigstore/sigstore/pkg/signature/kms/aws" _ "github.com/sigstore/sigstore/pkg/signature/kms/azure" @@ -72,25 +60,7 @@ var _ clusterimagepolicyreconciler.Finalizer = (*Reconciler)(nil) // ReconcileKind implements Interface.ReconcileKind. func (r *Reconciler) ReconcileKind(ctx context.Context, cip *v1alpha1.ClusterImagePolicy) reconciler.Event { - cipCopy, cipErr := r.inlinePublicKeys(ctx, cip) - if cipErr != nil { - // Handle error here - r.handleCIPError(ctx, cip.Name) - return cipErr - } - - // Converting external CIP to webhook CIP - bytes, err := json.Marshal(&cipCopy.Spec) - if err != nil { - return err - } - - var webhookCIP *webhookcip.ClusterImagePolicy - if err := json.Unmarshal(bytes, &webhookCIP); err != nil { - return err - } - - webhookCIP, cipErr = r.convertKeyData(ctx, webhookCIP) + webhookCIP, cipErr := webhookcip.ConvertClusterImagePolicyV1alpha1ToWebhook(ctx, cip, r.tracker, r.secretlister) if cipErr != nil { r.handleCIPError(ctx, cip.Name) // Note that we return the error about the Invalid cip here to make @@ -149,24 +119,6 @@ func (r *Reconciler) FinalizeKind(ctx context.Context, cip *v1alpha1.ClusterImag return r.removeCIPEntry(ctx, existing, cip.Name) } -// convertKeyData will go through the CIP and try to convert key data -// to ecdsa.PublicKey and store it in the returned CIP -// When PublicKeys are successfully set, the authority key's data will be -// cleared out -func (r *Reconciler) convertKeyData(ctx context.Context, cip *webhookcip.ClusterImagePolicy) (*webhookcip.ClusterImagePolicy, error) { - for _, authority := range cip.Authorities { - if authority.Key != nil && authority.Key.Data != "" { - keys, err := convertAuthorityKeys(ctx, authority.Key.Data) - if err != nil { - return nil, err - } - // When publicKeys are successfully converted, clear out Data - authority.Key.PublicKeys = keys - } - } - return cip, nil -} - func (r *Reconciler) handleCIPError(ctx context.Context, cipName string) { // The CIP is invalid, try to remove CIP from the configmap existing, err := r.configmaplister.ConfigMaps(system.Namespace()).Get(config.ImagePoliciesConfigName) @@ -179,122 +131,6 @@ func (r *Reconciler) handleCIPError(ctx context.Context, cipName string) { } } -func convertAuthorityKeys(ctx context.Context, pubKey string) ([]*ecdsa.PublicKey, error) { - keys := []*ecdsa.PublicKey{} - - logging.FromContext(ctx).Debugf("Got public key: %v", pubKey) - - pems := parsePems([]byte(pubKey)) - for _, p := range pems { - key, err := x509.ParsePKIXPublicKey(p.Bytes) - if err != nil { - return nil, err - } - keys = append(keys, key.(*ecdsa.PublicKey)) - } - return keys, nil -} - -func parsePems(b []byte) []*pem.Block { - p, rest := pem.Decode(b) - if p == nil { - return nil - } - pems := []*pem.Block{p} - - if rest != nil { - return append(pems, parsePems(rest)...) - } - return pems -} - -// inlinePublicKeys will go through the CIP and try to read the referenced -// secrets, KMS keys and convert them into inlined data. Makes a copy of the CIP -// before modifying it and returns the copy. -func (r *Reconciler) inlinePublicKeys(ctx context.Context, cip *v1alpha1.ClusterImagePolicy) (*v1alpha1.ClusterImagePolicy, error) { - ret := cip.DeepCopy() - for _, authority := range ret.Spec.Authorities { - if authority.Key != nil && authority.Key.SecretRef != nil { - if err := r.inlineAndTrackSecret(ctx, ret, authority.Key); err != nil { - logging.FromContext(ctx).Errorf("Failed to read secret %q: %v", authority.Key.SecretRef.Name, err) - return nil, err - } - } - if authority.Keyless != nil && authority.Keyless.CACert != nil && - authority.Keyless.CACert.SecretRef != nil { - if err := r.inlineAndTrackSecret(ctx, ret, authority.Keyless.CACert); err != nil { - logging.FromContext(ctx).Errorf("Failed to read secret %q: %v", authority.Keyless.CACert.SecretRef.Name, err) - return nil, err - } - } - if authority.Key != nil && authority.Key.KMS != "" { - if strings.Contains(authority.Key.KMS, "://") { - pubKeyString, err := getKMSPublicKey(ctx, authority.Key.KMS) - if err != nil { - return nil, err - } - - authority.Key.Data = pubKeyString - authority.Key.KMS = "" - } - } - } - return ret, nil -} - -// getKMSPublicKey returns the public key as a string from the configured KMS service using the key ID -func getKMSPublicKey(ctx context.Context, keyID string) (string, error) { - kmsSigner, err := kms.Get(ctx, keyID, crypto.SHA256) - if err != nil { - logging.FromContext(ctx).Errorf("Failed to read KMS key ID %q: %v", keyID, err) - return "", err - } - pemBytes, err := sigs.PublicKeyPem(kmsSigner, signatureoptions.WithContext(ctx)) - if err != nil { - return "", err - } - return string(pemBytes), nil -} - -// inlineSecret will take in a KeyRef and tries to read the Secret, finding the -// first key from it and will inline it in place of Data and then clear out -// the SecretRef and return it. -// Additionally, we set up a tracker so we will be notified if the secret -// is modified. -// There's still some discussion about how to handle multiple keys in a secret -// for now, just grab one from it. For reference, the discussion is here: -// TODO(vaikas): https://github.com/sigstore/cosign/issues/1573 -func (r *Reconciler) inlineAndTrackSecret(ctx context.Context, cip *v1alpha1.ClusterImagePolicy, keyref *v1alpha1.KeyRef) error { - if err := r.tracker.TrackReference(tracker.Reference{ - APIVersion: "v1", - Kind: "Secret", - Namespace: system.Namespace(), - Name: keyref.SecretRef.Name, - }, cip); err != nil { - return fmt.Errorf("failed to track changes to secret %q : %w", keyref.SecretRef.Name, err) - } - secret, err := r.secretlister.Secrets(system.Namespace()).Get(keyref.SecretRef.Name) - if err != nil { - return err - } - if len(secret.Data) == 0 { - return fmt.Errorf("secret %q contains no data", keyref.SecretRef.Name) - } - if len(secret.Data) > 1 { - return fmt.Errorf("secret %q contains multiple data entries, only one is supported", keyref.SecretRef.Name) - } - for k, v := range secret.Data { - logging.FromContext(ctx).Infof("inlining secret %q key %q", keyref.SecretRef.Name, k) - if !utils.IsValidKey(v) { - return fmt.Errorf("secret %q contains an invalid public key", keyref.SecretRef.Name) - } - - keyref.Data = string(v) - keyref.SecretRef = nil - } - return nil -} - // removeCIPEntry removes an entry from a CM. If no entry exists, it's a nop. func (r *Reconciler) removeCIPEntry(ctx context.Context, cm *corev1.ConfigMap, cipName string) error { patchBytes, err := resources.CreateRemovePatch(system.Namespace(), config.ImagePoliciesConfigName, cm.DeepCopy(), cipName) diff --git a/pkg/reconciler/clusterimagepolicy/clusterimagepolicy_test.go b/pkg/reconciler/clusterimagepolicy/clusterimagepolicy_test.go index 61e496cf3ad5..70fbb9d752a2 100644 --- a/pkg/reconciler/clusterimagepolicy/clusterimagepolicy_test.go +++ b/pkg/reconciler/clusterimagepolicy/clusterimagepolicy_test.go @@ -22,6 +22,7 @@ import ( "strings" "testing" + webhookcip "github.com/sigstore/cosign/pkg/cosign/kubernetes/webhook/clusterimagepolicy" logtesting "knative.dev/pkg/logging/testing" "github.com/sigstore/cosign/pkg/apis/config" @@ -475,11 +476,10 @@ func TestReconcile(t *testing.T) { }, }}), ), - makeConfigMapWithTwoEntriesNotPublicKeyFromSecret(), makeSecret(keySecretName, validPublicKeyData), }, - WantPatches: []clientgotesting.PatchActionImpl{ - makePatch(inlinedSecretKeyPatch), + WantCreates: []runtime.Object{ + makeConfigMap(), }, PostConditions: []func(*testing.T, *TableRow){ AssertTrackingSecret(system.Namespace(), keySecretName), @@ -586,7 +586,7 @@ func makeConfigMap() *corev1.ConfigMap { } func patchKMS(ctx context.Context, t *testing.T, kmsKey string) clientgotesting.PatchActionImpl { - pubKey, err := getKMSPublicKey(ctx, kmsKey) + pubKey, err := webhookcip.GetKMSPublicKey(ctx, kmsKey) if err != nil { t.Fatalf("Failed to read KMS key ID %q: %v", kmsKey, err) } @@ -629,21 +629,6 @@ func makeConfigMapWithTwoEntries() *corev1.ConfigMap { } } -// Same as MakeConfigMapWithTwoEntries but the inline data is not the secret -// so we will replace it with the secret data -func makeConfigMapWithTwoEntriesNotPublicKeyFromSecret() *corev1.ConfigMap { - return &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: system.Namespace(), - Name: config.ImagePoliciesConfigName, - }, - Data: map[string]string{ - cipName: `{"images":[{"glob":"ghcr.io/example/*"}],"authorities":[{"key":{"data":"NOT A REAL PUBLIC KEY"}}]}`, - cipName2: "remove me please", - }, - } -} - func makePatch(patch string) clientgotesting.PatchActionImpl { return clientgotesting.PatchActionImpl{ ActionImpl: clientgotesting.ActionImpl{