diff --git a/pkg/apis/config/image_policies_test.go b/pkg/apis/config/image_policies_test.go index 7ecea12521d..7c9c4015c44 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 9ac247c04c8..41706d53fc1 100644 --- a/pkg/cosign/kubernetes/webhook/clusterimagepolicy/clusterimagepolicy_types.go +++ b/pkg/cosign/kubernetes/webhook/clusterimagepolicy/clusterimagepolicy_types.go @@ -21,6 +21,8 @@ import ( "encoding/pem" "github.com/sigstore/cosign/pkg/apis/cosigned/v1alpha1" + + "knative.dev/pkg/apis" ) // ClusterImagePolicy defines the images that go through verification @@ -37,7 +39,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 +52,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 +81,7 @@ func (k *KeyRef) UnmarshalJSON(data []byte) error { k.Data = ret["data"] if ret["data"] != "" { - publicKeys, err = convertKeyDataToPublicKeys(ret["data"]) + publicKeys, err = ConvertKeyDataToPublicKeys(ret["data"]) if err != nil { return err } @@ -84,9 +92,59 @@ func (k *KeyRef) UnmarshalJSON(data []byte) error { return nil } -func convertKeyDataToPublicKeys(pubKey string) ([]*ecdsa.PublicKey, error) { - keys := []*ecdsa.PublicKey{} +func ConvertClusterImagePolicyV1alpha1ToWebhook(in *v1alpha1.ClusterImagePolicy) *ClusterImagePolicy { + copyIn := in.DeepCopy() + outAuthorities := make([]Authority, 0) + for _, authority := range copyIn.Spec.Authorities { + outAuthority := convertAuthorityV1Alpha1ToWebhook(authority) + outAuthorities = append(outAuthorities, *outAuthority) + } + + return &ClusterImagePolicy{ + Images: copyIn.Spec.Images, + Authorities: outAuthorities, + } +} + +func convertAuthorityV1Alpha1ToWebhook(in v1alpha1.Authority) *Authority { + keyRef := convertKeyRefV1Alpha1ToWebhook(in.Key) + keylessRef := convertKeylessRefV1Alpha1ToWebhook(in.Keyless) + + return &Authority{ + Key: keyRef, + Keyless: keylessRef, + Sources: in.Sources, + CTLog: in.CTLog, + } +} + +func convertKeyRefV1Alpha1ToWebhook(in *v1alpha1.KeyRef) *KeyRef { + if in == nil { + return nil + } + + return &KeyRef{ + Data: in.Data, + } +} + +func convertKeylessRefV1Alpha1ToWebhook(in *v1alpha1.KeylessRef) *KeylessRef { + if in == nil { + return nil + } + + CACertRef := convertKeyRefV1Alpha1ToWebhook(in.CACert) + + return &KeylessRef{ + URL: in.URL, + Identities: in.Identities, + CACert: CACertRef, + } +} + +func ConvertKeyDataToPublicKeys(pubKey string) ([]*ecdsa.PublicKey, error) { + keys := []*ecdsa.PublicKey{} pems := parsePems([]byte(pubKey)) for _, p := range pems { key, err := x509.ParsePKIXPublicKey(p.Bytes) diff --git a/pkg/cosign/kubernetes/webhook/validator_test.go b/pkg/cosign/kubernetes/webhook/validator_test.go index ad9b3ac56d8..f66772f4766 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 5ccfd51cb8e..4fef03bfd3c 100644 --- a/pkg/reconciler/clusterimagepolicy/clusterimagepolicy.go +++ b/pkg/reconciler/clusterimagepolicy/clusterimagepolicy.go @@ -17,10 +17,6 @@ package clusterimagepolicy import ( "context" "crypto" - "crypto/ecdsa" - "crypto/x509" - "encoding/json" - "encoding/pem" "fmt" "strings" @@ -30,12 +26,14 @@ import ( 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" + corev1 "k8s.io/api/core/v1" apierrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes" corev1listers "k8s.io/client-go/listers/core/v1" + "knative.dev/pkg/logging" "knative.dev/pkg/reconciler" "knative.dev/pkg/system" @@ -66,24 +64,6 @@ 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) if cipErr != nil { r.handleCIPError(ctx, cip.Name) // Note that we return the error about the Invalid cip here to make @@ -91,6 +71,8 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, cip *v1alpha1.ClusterIma return cipErr } + webhookCIP := webhookcip.ConvertClusterImagePolicyV1alpha1ToWebhook(cipCopy) + // See if the CM holding configs exists existing, err := r.configmaplister.ConfigMaps(system.Namespace()).Get(config.ImagePoliciesConfigName) if err != nil { @@ -142,24 +124,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) @@ -172,35 +136,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. diff --git a/pkg/reconciler/clusterimagepolicy/clusterimagepolicy_test.go b/pkg/reconciler/clusterimagepolicy/clusterimagepolicy_test.go index 61e496cf3ad..1220f8304bc 100644 --- a/pkg/reconciler/clusterimagepolicy/clusterimagepolicy_test.go +++ b/pkg/reconciler/clusterimagepolicy/clusterimagepolicy_test.go @@ -81,9 +81,6 @@ RCTfQ5s1kD+hGMSE1rH7s46hmXEeyhnlRnaGF8eMU/SBJE/2NKPnxE7WzQ== // two entries but only one is being removed. For keyless entry. removeSingleEntryKeylessPatch = `[{"op":"remove","path":"/data/test-cip-2"}]` - // This is the patch for inlined secret for key ref data - inlinedSecretKeyPatch = `[{"op":"replace","path":"/data/test-cip","value":"{\"images\":[{\"glob\":\"ghcr.io/example/*\"}],\"authorities\":[{\"key\":{\"data\":\"-----BEGIN PUBLIC KEY-----\\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAExB6+H6054/W1SJgs5JR6AJr6J35J\\nRCTfQ5s1kD+hGMSE1rH7s46hmXEeyhnlRnaGF8eMU/SBJE/2NKPnxE7WzQ==\\n-----END PUBLIC KEY-----\"}}]}"}]` - // This is the patch for inlined secret for keyless cakey ref data inlinedSecretKeylessPatch = `[{"op":"replace","path":"/data/test-cip-2","value":"{\"images\":[{\"glob\":\"ghcr.io/example/*\"}],\"authorities\":[{\"keyless\":{\"ca-cert\":{\"data\":\"-----BEGIN PUBLIC KEY-----\\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAExB6+H6054/W1SJgs5JR6AJr6J35J\\nRCTfQ5s1kD+hGMSE1rH7s46hmXEeyhnlRnaGF8eMU/SBJE/2NKPnxE7WzQ==\\n-----END PUBLIC KEY-----\"}}}]}"}]` ) @@ -475,11 +472,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), @@ -629,21 +625,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{