Add explicit check for invalid keys. This should not happen, but.

pkg/apis/config/image_policies_test.go

@@ -63,22 +63,22 @@ func TestGetAuthorities(t *testing.T) {
 	c, err := defaults.GetMatchingPolicies("rando")
 	checkGetMatches(t, c, err)
 	matchedPolicy := "cluster-image-policy-0"
-	want := "inlinedata here"
+	want := inlineKeyData
 	if got := getAuthority(t, c, matchedPolicy).Key.Data; got != want {
 		t.Errorf("Did not get what I wanted %q, got %+v", want, got)
 	}
 	// Make sure glob matches 'randomstuff*'
 	c, err = defaults.GetMatchingPolicies("randomstuffhere")
 	checkGetMatches(t, c, err)
 	matchedPolicy = "cluster-image-policy-1"
-	want = "otherinline here"
+	want = inlineKeyData
 	if got := getAuthority(t, c, matchedPolicy).Key.Data; got != want {
 		t.Errorf("Did not get what I wanted %q, got %+v", want, got)
 	}
 	c, err = defaults.GetMatchingPolicies("rando3")
 	checkGetMatches(t, c, err)
 	matchedPolicy = "cluster-image-policy-2"
-	want = "cacert chilling here"
+	want = inlineKeyData
 	if got := getAuthority(t, c, matchedPolicy).Keyless.CACert.Data; got != want {
 		t.Errorf("Did not get what I wanted %q, got %+v", want, got)
 	}
@@ -134,7 +134,7 @@ func TestGetAuthorities(t *testing.T) {
 	checkPublicKey(t, getAuthority(t, c, matchedPolicy).Key.PublicKeys[0])
 
 	matchedPolicy = "cluster-image-policy-5"
-	want = "inlinedata here"
+	want = inlineKeyData
 	if got := getAuthority(t, c, matchedPolicy).Key.Data; got != want {
 		t.Errorf("Did not get what I wanted %q, got %+v", want, got)
 	}
@@ -200,6 +200,17 @@ func TestGetAuthorities(t *testing.T) {
 	}
 }
 
+func TestFailsToLoadInvalid(t *testing.T) {
+	wantErr := "failed to parse the entry \"cluster-image-policy-0\""
+	_, example := ConfigMapsFromTestFile(t, "config-invalid-image-policy")
+	_, err := NewImagePoliciesConfigFromConfigMap(example)
+	if err == nil {
+		t.Error("Did not fail with invalid configmap")
+	} else if !strings.Contains(err.Error(), wantErr) {
+		t.Errorf("Unexpected error, wanted to contain %s : got %v", wantErr, err)
+	}
+}
+
 func checkGetMatches(t *testing.T, c map[string]webhookcip.ClusterImagePolicy, err error) {
 	t.Helper()
 	if err != nil {

pkg/apis/config/testdata/config-image-policies.yaml

@@ -33,7 +33,11 @@ data:
       authorities:
       - name: attestation-0
         key:
-          data: inlinedata here
+          data: |-
+            -----BEGIN PUBLIC KEY-----
+            MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAExB6+H6054/W1SJgs5JR6AJr6J35J
+            RCTfQ5s1kD+hGMSE1rH7s46hmXEeyhnlRnaGF8eMU/SBJE/2NKPnxE7WzQ==
+            -----END PUBLIC KEY-----
       - name: attestation-1
         key:
           kms: whatevs
@@ -43,15 +47,23 @@ data:
       authorities:
       - name: attestation-0
         key:
-          data: otherinline here
+          data: |-
+            -----BEGIN PUBLIC KEY-----
+            MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAExB6+H6054/W1SJgs5JR6AJr6J35J
+            RCTfQ5s1kD+hGMSE1rH7s46hmXEeyhnlRnaGF8eMU/SBJE/2NKPnxE7WzQ==
+            -----END PUBLIC KEY-----
     cluster-image-policy-2: |
       images:
       - glob: rando3
       authorities:
       - name: attestation-0
         keyless:
           ca-cert:
-            data: cacert chilling here
+            data: |-
+              -----BEGIN PUBLIC KEY-----
+              MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAExB6+H6054/W1SJgs5JR6AJr6J35J
+              RCTfQ5s1kD+hGMSE1rH7s46hmXEeyhnlRnaGF8eMU/SBJE/2NKPnxE7WzQ==
+              -----END PUBLIC KEY-----
           url: http://keylessurl.here
           identities:
           - issuer: issuer
@@ -84,7 +96,11 @@ data:
       authorities:
       - name: attestation-0
         key:
-          data: inlinedata here
+          data: |-
+            -----BEGIN PUBLIC KEY-----
+            MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAExB6+H6054/W1SJgs5JR6AJr6J35J
+            RCTfQ5s1kD+hGMSE1rH7s46hmXEeyhnlRnaGF8eMU/SBJE/2NKPnxE7WzQ==
+            -----END PUBLIC KEY-----
     cluster-image-policy-json: "{\"images\":[{\"glob\":\"ghcr.io/example/*\",\"regex\":\"\"}],\"authorities\":[{\"key\":{\"data\":\"-----BEGIN PUBLIC KEY-----\\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAExB6+H6054/W1SJgs5JR6AJr6J35J\\nRCTfQ5s1kD+hGMSE1rH7s46hmXEeyhnlRnaGF8eMU/SBJE/2NKPnxE7WzQ==\\n-----END PUBLIC KEY-----\"}}]}"
     cluster-image-policy-with-policy-attestations: |
       images:
@@ -93,7 +109,11 @@ data:
       - name: attestation-0
         keyless:
           ca-cert:
-            data: cacert chilling here
+            data: |-
+              -----BEGIN PUBLIC KEY-----
+              MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAExB6+H6054/W1SJgs5JR6AJr6J35J
+              RCTfQ5s1kD+hGMSE1rH7s46hmXEeyhnlRnaGF8eMU/SBJE/2NKPnxE7WzQ==
+              -----END PUBLIC KEY-----
           url: http://keylessurl.here
           identities:
           - issuer: issuer
@@ -111,7 +131,11 @@ data:
       authorities:
       - name: attestation-0
         key:
-          data: inlinedata here
+          data: |-
+            -----BEGIN PUBLIC KEY-----
+            MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAExB6+H6054/W1SJgs5JR6AJr6J35J
+            RCTfQ5s1kD+hGMSE1rH7s46hmXEeyhnlRnaGF8eMU/SBJE/2NKPnxE7WzQ==
+            -----END PUBLIC KEY-----
         source:
         - oci: "example.registry.com/alternative/signature"
     cluster-image-policy-source-oci-signature-pull-secrets: |
@@ -120,7 +144,11 @@ data:
       authorities:
       - name: attestation-0
         key:
-          data: inlinedata here
+          data: |-
+            -----BEGIN PUBLIC KEY-----
+            MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAExB6+H6054/W1SJgs5JR6AJr6J35J
+            RCTfQ5s1kD+hGMSE1rH7s46hmXEeyhnlRnaGF8eMU/SBJE/2NKPnxE7WzQ==
+            -----END PUBLIC KEY-----
         source:
         - oci: "example.registry.com/alternative/signature"
           signaturePullSecrets:

pkg/apis/config/testdata/config-invalid-image-policy.yaml

@@ -0,0 +1,40 @@
+# 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
+#
+#     http://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: v1
+kind: ConfigMap
+metadata:
+  name: config-image-policies
+  namespace: cosign-system
+  labels:
+    policy.sigstore.dev/release: devel
+
+data:
+  _example: |
+    ################################
+    #                              #
+    #    EXAMPLE CONFIGURATION     #
+    #                              #
+    ################################
+    cluster-image-policy-0: |
+      images:
+      - glob: invalidkey
+      authorities:
+      - name: attestation-0
+        key:
+          data: |-
+            -----BEGIN PUBLIC KEY-----
+            MFkwEwYHKoZIzINVALIDKEYHEREAExB6+H6054/W1SJgs5JR6AJr6J35J
+            RCTfQ5s1kD+hGMSE1rH7s46hmXEeyhnlRnaGF8eMU/SBJE/2NKPnxE7WzQ==
+            -----END PUBLIC KEY-----

pkg/webhook/clusterimagepolicy/clusterimagepolicy_types.go

@@ -305,8 +305,7 @@ func ConvertKeyDataToPublicKeys(pubKey string) ([]crypto.PublicKey, error) {
 	keys := []crypto.PublicKey{}
 	pems, validPEM := parsePEMKey([]byte(pubKey))
 	if !validPEM {
-		// TODO: If it is not valid report the error instead of ignore the key
-		return keys, nil
+		return keys, fmt.Errorf("failed to find a valid PEM key")
 	}
 
 	for _, p := range pems {

pkg/webhook/validator.go

@@ -598,7 +598,10 @@ func ValidatePolicySignaturesForAuthority(ctx context.Context, ref name.Referenc
 	}
 
 	switch {
-	case authority.Key != nil && len(authority.Key.PublicKeys) > 0:
+	case authority.Key != nil:
+		if len(authority.Key.PublicKeys) == 0 {
+			return nil, fmt.Errorf("there are no public keys for authority %s", name)
+		}
 		// TODO(vaikas): What should happen if there are multiple keys
 		// Is it even allowed? 'valid' returns success if any key
 		// matches.

pkg/webhook/validator_test.go

@@ -1617,6 +1617,15 @@ UoJou2P8sbDxpLiE/v3yLw1/jyOrCPWYHWFXnyyeGlkgSVefG54tNoK7Uw==
 		cvs           func(context.Context, name.Reference, *cosign.CheckOpts) ([]oci.Signature, bool, error)
 		customContext context.Context
 	}{{
+		name: "fail with no public key",
+		policy: webhookcip.ClusterImagePolicy{
+			Authorities: []webhookcip.Authority{{
+				Name: "authority-0",
+				Key:  &webhookcip.KeyRef{},
+			}},
+		},
+		wantErrs: []string{"there are no public keys for authority authority-0"},
+	}, {
 		name: "simple, public key, no matches",
 		policy: webhookcip.ClusterImagePolicy{
 			Authorities: []webhookcip.Authority{{