diff --git a/pkg/apis/config/image_policies_test.go b/pkg/apis/config/image_policies_test.go index 1b78c78a4..54a515e2e 100644 --- a/pkg/apis/config/image_policies_test.go +++ b/pkg/apis/config/image_policies_test.go @@ -63,7 +63,7 @@ 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) } @@ -71,14 +71,14 @@ func TestGetAuthorities(t *testing.T) { 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 { diff --git a/pkg/apis/config/testdata/config-image-policies.yaml b/pkg/apis/config/testdata/config-image-policies.yaml index 71f729406..4077a2a94 100644 --- a/pkg/apis/config/testdata/config-image-policies.yaml +++ b/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,7 +47,11 @@ 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 @@ -51,7 +59,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 @@ -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: diff --git a/pkg/apis/config/testdata/config-invalid-image-policy.yaml b/pkg/apis/config/testdata/config-invalid-image-policy.yaml new file mode 100644 index 000000000..0c54380ae --- /dev/null +++ b/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----- diff --git a/pkg/webhook/clusterimagepolicy/clusterimagepolicy_types.go b/pkg/webhook/clusterimagepolicy/clusterimagepolicy_types.go index 8751802e8..8ab9b7740 100644 --- a/pkg/webhook/clusterimagepolicy/clusterimagepolicy_types.go +++ b/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 { diff --git a/pkg/webhook/validator.go b/pkg/webhook/validator.go index cada9d3e1..f571805a6 100644 --- a/pkg/webhook/validator.go +++ b/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. diff --git a/pkg/webhook/validator_test.go b/pkg/webhook/validator_test.go index eec6c267e..d89a5c68c 100644 --- a/pkg/webhook/validator_test.go +++ b/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{{