Add public key validation (#1598)

* Add public key validation

Signed-off-by: Kavitha Krishnan <krishnanka@vmware.com>

* fix: key validation

Signed-off-by: hectorj2f <hectorf@vmware.com>

* fix: test valid policy using wrong key data

Signed-off-by: hectorj2f <hectorf@vmware.com>

Co-authored-by: hectorj2f <hectorf@vmware.com>

pkg/apis/cosigned/v1alpha1/clusterimagepolicy_validation.go

@@ -18,6 +18,7 @@ import (
 	"context"
 	"strings"
 
+	"github.com/sigstore/cosign/pkg/apis/utils"
 	"knative.dev/pkg/apis"
 )
 
@@ -93,6 +94,12 @@ func (key *KeyRef) Validate(ctx context.Context) *apis.FieldError {
 		if key.KMS != "" || key.SecretRef != nil {
 			errs = errs.Also(apis.ErrMultipleOneOf("data", "kms", "secretref"))
 		}
+		validPubkey := utils.IsValidKey([]byte(key.Data))
+		if !validPubkey {
+			errs = errs.Also(
+				apis.ErrInvalidValue(key.Data, "data"),
+			)
+		}
 	} else if key.KMS != "" && key.SecretRef != nil {
 		errs = errs.Also(apis.ErrMultipleOneOf("data", "kms", "secretref"))
 	}

pkg/apis/cosigned/v1alpha1/clusterimagepolicy_validation_test.go

@@ -115,14 +115,56 @@ func TestKeyValidation(t *testing.T) {
 					Authorities: []Authority{
 						{
 							Key: &KeyRef{
-								Data: "---some key data----",
+								Data: "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEaEOVJCFtduYr3xqTxeRWSW32CY/s\nTBNZj4oIUPl8JvhVPJ1TKDPlNcuT4YphSt6t3yOmMvkdQbCj8broX6vijw==\n-----END PUBLIC KEY-----",
 								KMS:  "kms://key/path",
 							},
 						},
 					},
 				},
 			},
 		},
+		{
+			name:        "Should fail when key has mixed valid and invalid data: %v",
+			expectErr:   true,
+			errorString: "invalid value: -----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEaEOVJCFtduYr3xqTxeRWSW32CY/s\nTBNZj4oIUPl8JvhVPJ1TKDPlNcuT4YphSt6t3yOmMvkdQbCj8broX6vijw==\n-----END PUBLIC KEY-----\n---somedata---",
+			policy: ClusterImagePolicy{
+				Spec: ClusterImagePolicySpec{
+					Images: []ImagePattern{
+						{
+							Glob: "myglob",
+						},
+					},
+					Authorities: []Authority{
+						{
+							Key: &KeyRef{
+								Data: "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEaEOVJCFtduYr3xqTxeRWSW32CY/s\nTBNZj4oIUPl8JvhVPJ1TKDPlNcuT4YphSt6t3yOmMvkdQbCj8broX6vijw==\n-----END PUBLIC KEY-----\n---somedata---",
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			name:        "Should fail when key has malformed pubkey data: %v",
+			expectErr:   true,
+			errorString: "invalid value: ---some key data----",
+			policy: ClusterImagePolicy{
+				Spec: ClusterImagePolicySpec{
+					Images: []ImagePattern{
+						{
+							Glob: "myglob",
+						},
+					},
+					Authorities: []Authority{
+						{
+							Key: &KeyRef{
+								Data: "---some key data----",
+							},
+						},
+					},
+				},
+			},
+		},
 		{
 			name:        "Should fail when key is empty: %v",
 			expectErr:   true,

pkg/apis/utils/key_validations.go

@@ -0,0 +1,52 @@
+// 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.
+
+package utils
+
+import (
+	"crypto/x509"
+	"encoding/pem"
+)
+
+func IsValidKey(b []byte) bool {
+	valid := true
+	pems, validPEM := parsePEMKey(b)
+	if !validPEM {
+		return false
+	}
+
+	for _, p := range pems {
+		_, err := x509.ParsePKIXPublicKey(p.Bytes)
+		if err != nil {
+			return false
+		}
+	}
+
+	return valid
+}
+
+func parsePEMKey(b []byte) ([]*pem.Block, bool) {
+	pemKey, rest := pem.Decode(b)
+	valid := true
+	if pemKey == nil {
+		return nil, false
+	}
+	pemBlocks := []*pem.Block{pemKey}
+
+	if len(rest) > 0 {
+		list, check := parsePEMKey(rest)
+		return append(pemBlocks, list...), check
+	}
+	return pemBlocks, valid
+}

test/testdata/cosigned/invalid/both-regex-and-pattern.yaml

@@ -22,4 +22,8 @@ spec:
     regex: image.*
   authorities:
   - key:
-      data: "---somedata---"
+      data: |
+        -----BEGIN PUBLIC KEY-----
+        MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEZxAfzrQG1EbWyCI8LiSB7YgSFXoI
+        FNGTyQGKHFc6/H8TQumT9VLS78pUwtv3w7EfKoyFZoP32KrO7nzUy2q6Cw==
+        -----END PUBLIC KEY-----

test/testdata/cosigned/invalid/invalid-pubkey.yaml

@@ -0,0 +1,24 @@
+# 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: cosigned.sigstore.dev/v1alpha1
+kind: ClusterImagePolicy
+metadata:
+  name: image-policy
+spec:
+  images:
+  - glob: image*
+  authorities:
+  - key:
+      data: "---somedata---"

test/testdata/cosigned/invalid/keyref-with-multiple-properties.yaml

@@ -21,5 +21,9 @@ spec:
   - glob: image*
   authorities:
   - key:
-      data: "---somedata---"
+      data: |
+        -----BEGIN PUBLIC KEY-----
+        MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEZxAfzrQG1EbWyCI8LiSB7YgSFXoI
+        FNGTyQGKHFc6/H8TQumT9VLS78pUwtv3w7EfKoyFZoP32KrO7nzUy2q6Cw==
+        -----END PUBLIC KEY-----
       kms: "kms://url"

test/testdata/cosigned/invalid/valid-keyref-and-keylessref.yaml

@@ -24,4 +24,4 @@ spec:
       identities:
       - issuer: "issue-details"
     key:
-      data: "---somekey---"
+      kms: "kms://url"

test/testdata/cosigned/valid/valid-policy.yaml

@@ -21,8 +21,6 @@ spec:
   - glob: images.*
   - glob: image*
   authorities:
-  - key:
-      data: "---another-public-key---"
   - keyless:
       ca-key:
         secretRef:
@@ -36,7 +34,11 @@ spec:
       identities:
       - issuer: "issue-details1"
   - key:
-      data: "---some-key---"
+      data: |
+        -----BEGIN PUBLIC KEY-----
+        MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEaEOVJCFtduYr3xqTxeRWSW32CY/s
+        TBNZj4oIUPl8JvhVPJ1TKDPlNcuT4YphSt6t3yOmMvkdQbCj8broX6vijw==
+        -----END PUBLIC KEY-----
   - key:
       kms: "kms://key/path"
   - key: