Skip to content

Commit a008a9c

Browse files
kkavithahectorj2f
authored andcommitted
Add public key validation (sigstore#1598)
* Add public key validation Signed-off-by: Kavitha Krishnan <[email protected]> * fix: key validation Signed-off-by: hectorj2f <[email protected]> * fix: test valid policy using wrong key data Signed-off-by: hectorj2f <[email protected]> Co-authored-by: hectorj2f <[email protected]>
1 parent 3327dae commit a008a9c

8 files changed

+142
-7
lines changed

pkg/apis/cosigned/v1alpha1/clusterimagepolicy_validation.go

+7
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"context"
1919
"strings"
2020

21+
"github.com/sigstore/cosign/pkg/apis/utils"
2122
"knative.dev/pkg/apis"
2223
)
2324

@@ -93,6 +94,12 @@ func (key *KeyRef) Validate(ctx context.Context) *apis.FieldError {
9394
if key.KMS != "" || key.SecretRef != nil {
9495
errs = errs.Also(apis.ErrMultipleOneOf("data", "kms", "secretref"))
9596
}
97+
validPubkey := utils.IsValidKey([]byte(key.Data))
98+
if !validPubkey {
99+
errs = errs.Also(
100+
apis.ErrInvalidValue(key.Data, "data"),
101+
)
102+
}
96103
} else if key.KMS != "" && key.SecretRef != nil {
97104
errs = errs.Also(apis.ErrMultipleOneOf("data", "kms", "secretref"))
98105
}

pkg/apis/cosigned/v1alpha1/clusterimagepolicy_validation_test.go

+43-1
Original file line numberDiff line numberDiff line change
@@ -115,14 +115,56 @@ func TestKeyValidation(t *testing.T) {
115115
Authorities: []Authority{
116116
{
117117
Key: &KeyRef{
118-
Data: "---some key data----",
118+
Data: "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEaEOVJCFtduYr3xqTxeRWSW32CY/s\nTBNZj4oIUPl8JvhVPJ1TKDPlNcuT4YphSt6t3yOmMvkdQbCj8broX6vijw==\n-----END PUBLIC KEY-----",
119119
KMS: "kms://key/path",
120120
},
121121
},
122122
},
123123
},
124124
},
125125
},
126+
{
127+
name: "Should fail when key has mixed valid and invalid data: %v",
128+
expectErr: true,
129+
errorString: "invalid value: -----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEaEOVJCFtduYr3xqTxeRWSW32CY/s\nTBNZj4oIUPl8JvhVPJ1TKDPlNcuT4YphSt6t3yOmMvkdQbCj8broX6vijw==\n-----END PUBLIC KEY-----\n---somedata---",
130+
policy: ClusterImagePolicy{
131+
Spec: ClusterImagePolicySpec{
132+
Images: []ImagePattern{
133+
{
134+
Glob: "myglob",
135+
},
136+
},
137+
Authorities: []Authority{
138+
{
139+
Key: &KeyRef{
140+
Data: "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEaEOVJCFtduYr3xqTxeRWSW32CY/s\nTBNZj4oIUPl8JvhVPJ1TKDPlNcuT4YphSt6t3yOmMvkdQbCj8broX6vijw==\n-----END PUBLIC KEY-----\n---somedata---",
141+
},
142+
},
143+
},
144+
},
145+
},
146+
},
147+
{
148+
name: "Should fail when key has malformed pubkey data: %v",
149+
expectErr: true,
150+
errorString: "invalid value: ---some key data----",
151+
policy: ClusterImagePolicy{
152+
Spec: ClusterImagePolicySpec{
153+
Images: []ImagePattern{
154+
{
155+
Glob: "myglob",
156+
},
157+
},
158+
Authorities: []Authority{
159+
{
160+
Key: &KeyRef{
161+
Data: "---some key data----",
162+
},
163+
},
164+
},
165+
},
166+
},
167+
},
126168
{
127169
name: "Should fail when key is empty: %v",
128170
expectErr: true,

pkg/apis/utils/key_validations.go

+52
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
// Copyright 2022 The Sigstore Authors.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package utils
16+
17+
import (
18+
"crypto/x509"
19+
"encoding/pem"
20+
)
21+
22+
func IsValidKey(b []byte) bool {
23+
valid := true
24+
pems, validPEM := parsePEMKey(b)
25+
if !validPEM {
26+
return false
27+
}
28+
29+
for _, p := range pems {
30+
_, err := x509.ParsePKIXPublicKey(p.Bytes)
31+
if err != nil {
32+
return false
33+
}
34+
}
35+
36+
return valid
37+
}
38+
39+
func parsePEMKey(b []byte) ([]*pem.Block, bool) {
40+
pemKey, rest := pem.Decode(b)
41+
valid := true
42+
if pemKey == nil {
43+
return nil, false
44+
}
45+
pemBlocks := []*pem.Block{pemKey}
46+
47+
if len(rest) > 0 {
48+
list, check := parsePEMKey(rest)
49+
return append(pemBlocks, list...), check
50+
}
51+
return pemBlocks, valid
52+
}

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

+5-1
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,8 @@ spec:
2222
regex: image.*
2323
authorities:
2424
- key:
25-
data: "---somedata---"
25+
data: |
26+
-----BEGIN PUBLIC KEY-----
27+
MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEZxAfzrQG1EbWyCI8LiSB7YgSFXoI
28+
FNGTyQGKHFc6/H8TQumT9VLS78pUwtv3w7EfKoyFZoP32KrO7nzUy2q6Cw==
29+
-----END PUBLIC KEY-----
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
# Copyright 2022 The Sigstore Authors.
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http:#www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
---
15+
apiVersion: cosigned.sigstore.dev/v1alpha1
16+
kind: ClusterImagePolicy
17+
metadata:
18+
name: image-policy
19+
spec:
20+
images:
21+
- glob: image*
22+
authorities:
23+
- key:
24+
data: "---somedata---"

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

+5-1
Original file line numberDiff line numberDiff line change
@@ -21,5 +21,9 @@ spec:
2121
- glob: image*
2222
authorities:
2323
- key:
24-
data: "---somedata---"
24+
data: |
25+
-----BEGIN PUBLIC KEY-----
26+
MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEZxAfzrQG1EbWyCI8LiSB7YgSFXoI
27+
FNGTyQGKHFc6/H8TQumT9VLS78pUwtv3w7EfKoyFZoP32KrO7nzUy2q6Cw==
28+
-----END PUBLIC KEY-----
2529
kms: "kms://url"

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,4 @@ spec:
2424
identities:
2525
- issuer: "issue-details"
2626
key:
27-
data: "---somekey---"
27+
kms: "kms://url"

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

+5-3
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,6 @@ spec:
2121
- glob: images.*
2222
- glob: image*
2323
authorities:
24-
- key:
25-
data: "---another-public-key---"
2624
- keyless:
2725
ca-key:
2826
secretRef:
@@ -36,7 +34,11 @@ spec:
3634
identities:
3735
- issuer: "issue-details1"
3836
- key:
39-
data: "---some-key---"
37+
data: |
38+
-----BEGIN PUBLIC KEY-----
39+
MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEaEOVJCFtduYr3xqTxeRWSW32CY/s
40+
TBNZj4oIUPl8JvhVPJ1TKDPlNcuT4YphSt6t3yOmMvkdQbCj8broX6vijw==
41+
-----END PUBLIC KEY-----
4042
- key:
4143
kms: "kms://key/path"
4244
- key:

0 commit comments

Comments
 (0)