From eff72ffa8dfc8bdc0e79e0a0a5dce5f977946cdd Mon Sep 17 00:00:00 2001 From: Denny Hoang Date: Wed, 6 Apr 2022 11:18:39 -0400 Subject: [PATCH] Fix publicKey unmarshal - Add key sign verify for cosigned github workflow Signed-off-by: Denny Hoang --- .../workflows/kind-cluster-image-policy.yaml | 61 +++++++++++++++++-- .../apis/cosigned/clusterimagepolicy_types.go | 59 +++++++++++++++++- pkg/apis/config/image_policies_test.go | 32 ++++++++++ .../clusterimagepolicy/clusterimagepolicy.go | 1 - .../clusterimagepolicy_test.go | 22 +++---- test/testdata/cosigned/e2e/cip-key.yaml | 29 +++++++++ .../e2e/{cip.yaml => cip-keyless.yaml} | 2 +- 7 files changed, 183 insertions(+), 23 deletions(-) create mode 100644 test/testdata/cosigned/e2e/cip-key.yaml rename test/testdata/cosigned/e2e/{cip.yaml => cip-keyless.yaml} (96%) diff --git a/.github/workflows/kind-cluster-image-policy.yaml b/.github/workflows/kind-cluster-image-policy.yaml index e724abbca86..f63e353677d 100644 --- a/.github/workflows/kind-cluster-image-policy.yaml +++ b/.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 diff --git a/internal/pkg/apis/cosigned/clusterimagepolicy_types.go b/internal/pkg/apis/cosigned/clusterimagepolicy_types.go index b576c9e0a17..9ac247c04c8 100644 --- a/internal/pkg/apis/cosigned/clusterimagepolicy_types.go +++ b/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 } diff --git a/pkg/apis/config/image_policies_test.go b/pkg/apis/config/image_policies_test.go index eda62e590d3..f14efa77abd 100644 --- a/pkg/apis/config/image_policies_test.go +++ b/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)) + } +} diff --git a/pkg/reconciler/clusterimagepolicy/clusterimagepolicy.go b/pkg/reconciler/clusterimagepolicy/clusterimagepolicy.go index 1a1f1e04ae0..2401954d52b 100644 --- a/pkg/reconciler/clusterimagepolicy/clusterimagepolicy.go +++ b/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 diff --git a/pkg/reconciler/clusterimagepolicy/clusterimagepolicy_test.go b/pkg/reconciler/clusterimagepolicy/clusterimagepolicy_test.go index 29813ad37b9..61e496cf3ad 100644 --- a/pkg/reconciler/clusterimagepolicy/clusterimagepolicy_test.go +++ b/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", }, } diff --git a/test/testdata/cosigned/e2e/cip-key.yaml b/test/testdata/cosigned/e2e/cip-key.yaml new file mode 100644 index 00000000000..d4d8334905d --- /dev/null +++ b/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----- + diff --git a/test/testdata/cosigned/e2e/cip.yaml b/test/testdata/cosigned/e2e/cip-keyless.yaml similarity index 96% rename from test/testdata/cosigned/e2e/cip.yaml rename to test/testdata/cosigned/e2e/cip-keyless.yaml index fffd246b8cc..5408b8aabcb 100644 --- a/test/testdata/cosigned/e2e/cip.yaml +++ b/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*