[Cosigned] Fix publicKey unmarshal

.github/workflows/kind-cluster-image-policy.yaml

@@ -120,9 +120,9 @@ jobs:
         echo Created image $demoimage2
         popd
 
-    - name: Deploy ClusterImagePolicy
+    - name: Deploy ClusterImagePolicy With Keyless Signing
       run: |
-        kubectl apply -f ./test/testdata/cosigned/e2e/cip.yaml
+        kubectl apply -f ./test/testdata/cosigned/e2e/cip-keyless.yaml
 
     - name: Sign demoimage with cosign
       run: |
@@ -134,12 +134,12 @@ jobs:
 
     - name: Deploy jobs and verify signed works, unsigned fails
       run: |
-        kubectl create namespace demo
-        kubectl label namespace demo cosigned.sigstore.dev/include=true
+        kubectl create namespace demo-keyless-signing
+        kubectl label namespace demo-keyless-signing cosigned.sigstore.dev/include=true
 
         echo '::group:: test job success'
         # We signed this above, this should work
-        if ! kubectl create -n demo job demo --image=${{ env.demoimage }} ; then
+        if ! kubectl create -n demo-keyless-signing job demo --image=${{ env.demoimage }} ; then
           echo Failed to create Job in namespace without label!
           exit 1
         else
@@ -149,7 +149,56 @@ jobs:
 
         echo '::group:: test job rejection'
         # We did not sign this, should fail
-        if kubectl create -n demo job demo2 --image=${{ env.demoimage2 }} ; then
+        if kubectl create -n demo-keyless-signing job demo2 --image=${{ env.demoimage2 }} ; then
+          echo Failed to block unsigned Job creation!
+          exit 1
+        else
+          echo Successfully blocked Job creation with unsigned image
+        fi
+        echo '::endgroup::'
+
+    - name: Generate New Signing Key
+      run: |
+        COSIGN_PASSWORD="" ./cosign generate-key-pair
+
+    - name: Deploy ClusterImagePolicy With Key Signing
+      run: |
+        yq '. | .spec.authorities[0].key.data |= load_str("cosign.pub")' ./test/testdata/cosigned/e2e/cip-key.yaml | \
+          kubectl apply -f -
+
+    - name: Verify with two CIP, one not signed with public key
+      run: |
+        if kubectl create -n demo-key-signing job demo --image=${{ env.demoimage }}; then
+          echo Failed to block unsigned Job creation!
+          exit 1
+        fi
+
+    - name: Sign demoimage with cosign key
+      run: |
+        ./cosign sign --key cosign.key --force --allow-insecure-registry ${{ env.demoimage }}
+
+    - name: Verify with cosign
+      run: |
+        ./cosign verify --key cosign.pub --allow-insecure-registry ${{ env.demoimage }}
+
+    - name: Deploy jobs and verify signed works, unsigned fails
+      run: |
+        kubectl create namespace demo-key-signing
+        kubectl label namespace demo-key-signing cosigned.sigstore.dev/include=true
+
+        echo '::group:: test job success'
+        # We signed this above, this should work
+        if ! kubectl create -n demo-key-signing job demo --image=${{ env.demoimage }} ; then
+          echo Failed to create Job in namespace without label!
+          exit 1
+        else
+          echo Succcessfully created Job with signed image
+        fi
+        echo '::endgroup:: test job success'
+
+        echo '::group:: test job rejection'
+        # We did not sign this, should fail
+        if kubectl create -n demo-key-signing job demo2 --image=${{ env.demoimage2 }} ; then
           echo Failed to block unsigned Job creation!
           exit 1
         else

internal/pkg/apis/cosigned/clusterimagepolicy_types.go

@@ -16,6 +16,9 @@ package clusterimagepolicy
 
 import (
 	"crypto/ecdsa"
+	"crypto/x509"
+	"encoding/json"
+	"encoding/pem"
 
 	"github.com/sigstore/cosign/pkg/apis/cosigned/v1alpha1"
 )
@@ -50,6 +53,60 @@ type KeyRef struct {
 	// 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:"publicKeys,omitempty"`
+	PublicKeys []*ecdsa.PublicKey `json:"-"`
+}
+
+// UnmarshalJSON populates the PublicKeys using Data because
+// JSON unmashalling errors for *big.Int
+func (k *KeyRef) UnmarshalJSON(data []byte) error {
+	var publicKeys []*ecdsa.PublicKey
+	var err error
+
+	ret := make(map[string]string)
+	if err = json.Unmarshal(data, &ret); err != nil {
+		return err
+	}
+
+	k.Data = ret["data"]
+
+	if ret["data"] != "" {
+		publicKeys, err = convertKeyDataToPublicKeys(ret["data"])
+		if err != nil {
+			return err
+		}
+	}
+
+	k.PublicKeys = publicKeys
+
+	return nil
+}
+
+func convertKeyDataToPublicKeys(pubKey string) ([]*ecdsa.PublicKey, error) {
+	keys := []*ecdsa.PublicKey{}
+
+	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
 }

pkg/apis/config/image_policies_test.go

@@ -15,6 +15,10 @@
 package config
 
 import (
+	"crypto/ecdsa"
+	"crypto/x509"
+	"encoding/pem"
+	"strings"
 	"testing"
 
 	internalcip "github.com/sigstore/cosign/internal/pkg/apis/cosigned"
@@ -81,6 +85,7 @@ func TestGetAuthorities(t *testing.T) {
 	if got := c[matchedPolicy][0].Key.Data; got != want {
 		t.Errorf("Did not get what I wanted %q, got %+v", want, got)
 	}
+	checkPublicKey(t, c[matchedPolicy][0].Key.PublicKeys[0])
 
 	// Test multiline yaml cert
 	c, err = defaults.GetMatchingPolicies("inlinecert")
@@ -90,6 +95,8 @@ func TestGetAuthorities(t *testing.T) {
 	if got := c[matchedPolicy][0].Key.Data; got != want {
 		t.Errorf("Did not get what I wanted %q, got %+v", want, got)
 	}
+	checkPublicKey(t, c[matchedPolicy][0].Key.PublicKeys[0])
+
 	// Test multiline cert but json encoded
 	c, err = defaults.GetMatchingPolicies("ghcr.io/example/*")
 	checkGetMatches(t, c, err)
@@ -98,6 +105,8 @@ func TestGetAuthorities(t *testing.T) {
 	if got := c[matchedPolicy][0].Key.Data; got != want {
 		t.Errorf("Did not get what I wanted %q, got %+v", want, got)
 	}
+	checkPublicKey(t, c[matchedPolicy][0].Key.PublicKeys[0])
+
 	// Test multiple matches
 	c, err = defaults.GetMatchingPolicies("regexstringtoo")
 	checkGetMatches(t, c, err)
@@ -109,6 +118,8 @@ func TestGetAuthorities(t *testing.T) {
 	if got := c[matchedPolicy][0].Key.Data; got != want {
 		t.Errorf("Did not get what I wanted %q, got %+v", want, got)
 	}
+	checkPublicKey(t, c[matchedPolicy][0].Key.PublicKeys[0])
+
 	matchedPolicy = "cluster-image-policy-5"
 	want = "inlinedata here"
 	if got := c[matchedPolicy][0].Key.Data; got != want {
@@ -131,3 +142,24 @@ func checkGetMatches(t *testing.T, c map[string][]internalcip.Authority, err err
 	}
 	t.Error("Wanted a config and non-zero authorities, got no authorities")
 }
+
+func checkPublicKey(t *testing.T, gotKey *ecdsa.PublicKey) {
+	t.Helper()
+
+	derBytes, err := x509.MarshalPKIXPublicKey(gotKey)
+	if err != nil {
+		t.Error("Failed to Marshal Key =", err)
+	}
+
+	pemBytes := pem.EncodeToMemory(&pem.Block{
+		Type:  "PUBLIC KEY",
+		Bytes: derBytes,
+	})
+
+	// 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))
+	}
+}

pkg/reconciler/clusterimagepolicy/clusterimagepolicy.go

@@ -162,7 +162,6 @@ func (r *Reconciler) convertKeyData(ctx context.Context, cip *internalcip.Cluste
 			}
 			// When publicKeys are successfully converted, clear out Data
 			authority.Key.PublicKeys = keys
-			authority.Key.Data = ""
 		}
 	}
 	return cip, nil

pkg/reconciler/clusterimagepolicy/clusterimagepolicy_test.go

@@ -19,7 +19,7 @@ import (
 	"crypto/ecdsa"
 	"crypto/elliptic"
 	"crypto/rand"
-	"fmt"
+	"strings"
 	"testing"
 
 	logtesting "knative.dev/pkg/logging/testing"
@@ -64,10 +64,10 @@ RCTfQ5s1kD+hGMSE1rH7s46hmXEeyhnlRnaGF8eMU/SBJE/2NKPnxE7WzQ==
 -----END PUBLIC KEY-----`
 
 	// This is the patch for replacing a single entry in the ConfigMap
-	replaceCIPPatch = `[{"op":"replace","path":"/data/test-cip","value":"{\"images\":[{\"glob\":\"ghcr.io/example/*\"}],\"authorities\":[{\"key\":{\"publicKeys\":[{\"Curve\":{\"P\":115792089210356248762697446949407573530086143415290314195533631308867097853951,\"N\":115792089210356248762697446949407573529996955224135760342422259061068512044369,\"B\":41058363725152142129326129780047268409114441015993725554835256314039467401291,\"Gx\":48439561293906451759052585252797914202762949526041747995844080717082404635286,\"Gy\":36134250956749795798585127919587881956611106672985015071877198253568414405109,\"BitSize\":256,\"Name\":\"P-256\"},\"X\":88707635920070641755721733257804385133633538985728333477702186279848359565508,\"Y\":60084629679813308197062610650536235858799576629081565335301707208097929287373}]}}]}"}]`
+	replaceCIPPatch = `[{"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 adding an entry for non-existing KMS for cipName2
-	addCIP2Patch = `[{"op":"add","path":"/data/test-cip-2","value":"{\"images\":[{\"glob\":\"ghcr.io/example/*\"}],\"authorities\":[{\"key\":{}}]}"}]`
+	addCIP2Patch = `[{"op":"add","path":"/data/test-cip-2","value":"{\"images\":[{\"glob\":\"ghcr.io/example/*\"}],\"authorities\":[{\"key\":{\"data\":\"azure-kms://foo/bar\"}}]}"}]`
 
 	// This is the patch for removing the last entry, leaving just the
 	// configmap objectmeta, no data.
@@ -82,7 +82,7 @@ RCTfQ5s1kD+hGMSE1rH7s46hmXEeyhnlRnaGF8eMU/SBJE/2NKPnxE7WzQ==
 	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\":{\"publicKeys\":[{\"Curve\":{\"P\":115792089210356248762697446949407573530086143415290314195533631308867097853951,\"N\":115792089210356248762697446949407573529996955224135760342422259061068512044369,\"B\":41058363725152142129326129780047268409114441015993725554835256314039467401291,\"Gx\":48439561293906451759052585252797914202762949526041747995844080717082404635286,\"Gy\":36134250956749795798585127919587881956611106672985015071877198253568414405109,\"BitSize\":256,\"Name\":\"P-256\"},\"X\":88707635920070641755721733257804385133633538985728333477702186279848359565508,\"Y\":60084629679813308197062610650536235858799576629081565335301707208097929287373}]}}]}"}]`
+	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-----\"}}}]}"}]`
@@ -580,7 +580,7 @@ func makeConfigMap() *corev1.ConfigMap {
 			Name:      config.ImagePoliciesConfigName,
 		},
 		Data: map[string]string{
-			cipName: `{"images":[{"glob":"ghcr.io/example/*"}],"authorities":[{"key":{"publicKeys":[{"Curve":{"P":115792089210356248762697446949407573530086143415290314195533631308867097853951,"N":115792089210356248762697446949407573529996955224135760342422259061068512044369,"B":41058363725152142129326129780047268409114441015993725554835256314039467401291,"Gx":48439561293906451759052585252797914202762949526041747995844080717082404635286,"Gy":36134250956749795798585127919587881956611106672985015071877198253568414405109,"BitSize":256,"Name":"P-256"},"X":88707635920070641755721733257804385133633538985728333477702186279848359565508,"Y":60084629679813308197062610650536235858799576629081565335301707208097929287373}]}}]}`,
+			cipName: `{"images":[{"glob":"ghcr.io/example/*"}],"authorities":[{"key":{"data":"-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAExB6+H6054/W1SJgs5JR6AJr6J35J\nRCTfQ5s1kD+hGMSE1rH7s46hmXEeyhnlRnaGF8eMU/SBJE/2NKPnxE7WzQ==\n-----END PUBLIC KEY-----"}}]}`,
 		},
 	}
 }
@@ -591,13 +591,7 @@ func patchKMS(ctx context.Context, t *testing.T, kmsKey string) clientgotesting.
 		t.Fatalf("Failed to read KMS key ID %q: %v", kmsKey, err)
 	}
 
-	authorityKeys, err := convertAuthorityKeys(ctx, pubKey)
-	if err != nil || len(authorityKeys) == 0 {
-		t.Fatalf("Failed to convert KMS public key ID %q: %v", kmsKey, err)
-	}
-
-	pubKeyString := fmt.Sprintf(`{\"Curve\":{\"P\":115792089210356248762697446949407573530086143415290314195533631308867097853951,\"N\":115792089210356248762697446949407573529996955224135760342422259061068512044369,\"B\":41058363725152142129326129780047268409114441015993725554835256314039467401291,\"Gx\":48439561293906451759052585252797914202762949526041747995844080717082404635286,\"Gy\":36134250956749795798585127919587881956611106672985015071877198253568414405109,\"BitSize\":256,\"Name\":\"P-256\"},\"X\":%d,\"Y\":%d}`, authorityKeys[0].X, authorityKeys[0].Y)
-	patch := `[{"op":"add","path":"/data","value":{"test-kms-cip":"{\"images\":[{\"glob\":\"ghcr.io/example/*\"}],\"authorities\":[{\"key\":{\"publicKeys\":[` + pubKeyString + `]}}]}"}}]`
+	patch := `[{"op":"add","path":"/data","value":{"test-kms-cip":"{\"images\":[{\"glob\":\"ghcr.io/example/*\"}],\"authorities\":[{\"key\":{\"data\":\"` + strings.ReplaceAll(pubKey, "\n", "\\\\n") + `\"}}]}"}}]`
 
 	return clientgotesting.PatchActionImpl{
 		ActionImpl: clientgotesting.ActionImpl{
@@ -616,7 +610,7 @@ func makeDifferentConfigMap() *corev1.ConfigMap {
 			Name:      config.ImagePoliciesConfigName,
 		},
 		Data: map[string]string{
-			cipName: `{"images":[{"glob":"ghcr.io/example/*"}],"authorities":[{"key":{"data":"-----BEGIN NOTPUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAExB6+H6054/W1SJgs5JR6AJr6J35J\nRCTfQ5s1kD+hGMSE1rH7s46hmXEeyhnlRnaGF8eMU/SBJE/2NKPnxE7WzQ==\n-----END NOTPUBLIC KEY-----","publicKeys":[{"Curve":{"P":115792089210356248762697446949407573530086143415290314195533631308867097853951,"N":115792089210356248762697446949407573529996955224135760342422259061068512044369,"B":41058363725152142129326129780047268409114441015993725554835256314039467401291,"Gx":48439561293906451759052585252797914202762949526041747995844080717082404635286,"Gy":36134250956749795798585127919587881956611106672985015071877198253568414405109,"BitSize":256,"Name":"P-256"},"X":88707635920070641755721733257804385133633538985728333477702186279848359565508,"Y":60084629679813308197062610650536235858799576629081565335301707208097929287373}]}}]}`,
+			cipName: `{"images":[{"glob":"ghcr.io/example/*"}],"authorities":[{"key":{"data":"-----BEGIN NOTPUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAExB6+H6054/W1SJgs5JR6AJr6J35J\nRCTfQ5s1kD+hGMSE1rH7s46hmXEeyhnlRnaGF8eMU/SBJE/2NKPnxE7WzQ==\n-----END NOTPUBLIC KEY-----"}}]}`,
 		},
 	}
 }
@@ -629,7 +623,7 @@ func makeConfigMapWithTwoEntries() *corev1.ConfigMap {
 			Name:      config.ImagePoliciesConfigName,
 		},
 		Data: map[string]string{
-			cipName:  `{"images":[{"glob":"ghcr.io/example/*"}],"authorities":[{"key":{\"publicKeys\":[{\"Curve\":{\"P\":115792089210356248762697446949407573530086143415290314195533631308867097853951,\"N\":115792089210356248762697446949407573529996955224135760342422259061068512044369,\"B\":41058363725152142129326129780047268409114441015993725554835256314039467401291,\"Gx\":48439561293906451759052585252797914202762949526041747995844080717082404635286,\"Gy\":36134250956749795798585127919587881956611106672985015071877198253568414405109,\"BitSize\":256,\"Name\":\"P-256\"},\"X\":88707635920070641755721733257804385133633538985728333477702186279848359565508,\"Y\":60084629679813308197062610650536235858799576629081565335301707208097929287373}]}}]}`,
+			cipName:  `{"images":[{"glob":"ghcr.io/example/*"}],"authorities":[{"key":{"data":"-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAExB6+H6054/W1SJgs5JR6AJr6J35J\nRCTfQ5s1kD+hGMSE1rH7s46hmXEeyhnlRnaGF8eMU/SBJE/2NKPnxE7WzQ==\n-----END PUBLIC KEY-----"}}]}`,
 			cipName2: "remove me please",
 		},
 	}

test/testdata/cosigned/e2e/cip-key.yaml

@@ -0,0 +1,29 @@
+# Copyright 2022 The Sigstore 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
+#
+#     https://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.
+
+apiVersion: cosigned.sigstore.dev/v1alpha1
+kind: ClusterImagePolicy
+metadata:
+  name: image-policy-key
+spec:
+  images:
+  - glob: registry.local:5000/cosigned/demo*
+  authorities:
+  - key:
+      data: |
+        -----BEGIN PUBLIC KEY-----
+        MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEZxAfzrQG1EbWyCI8LiSB7YgSFXoI
+        FNGTyQGKHFc6/H8TQumT9VLS78pUwtv3w7EfKoyFZoP32KrO7nzUy2q6Cw==
+        -----END PUBLIC KEY-----
+

test/testdata/cosigned/e2e/cip.yaml → test/testdata/cosigned/e2e/cip-keyless.yaml

@@ -15,7 +15,7 @@
 apiVersion: cosigned.sigstore.dev/v1alpha1
 kind: ClusterImagePolicy
 metadata:
-  name: image-policy
+  name: image-policy-keyless
 spec:
   images:
   - glob: registry.local:5000/cosigned/demo*