Skip to content

Commit

Permalink
Create convert functions for internal CIP (sigstore#1736)
Browse files Browse the repository at this point in the history 
Signed-off-by: Denny Hoang <dhoang@vmware.com>
  • Loading branch information
@DennyHoang @mlieberman85
DennyHoang authored and mlieberman85 committed May 6, 2022
1 parent 3e45ffd commit 93d5b5c
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 101 deletions.
1 change: 0 additions & 1 deletion pkg/apis/config/image_policies_test.go
View file
Expand Up @@ -158,7 +158,6 @@ func checkPublicKey(t *testing.T, gotKey *ecdsa.PublicKey) {

// 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))
}
Expand Down
72 changes: 65 additions & 7 deletions pkg/cosign/kubernetes/webhook/clusterimagepolicy/clusterimagepolicy_types.go
View file
Expand Up @@ -21,6 +21,8 @@ import (
"encoding/pem"

"github.com/sigstore/cosign/pkg/apis/cosigned/v1alpha1"

"knative.dev/pkg/apis"
)

// ClusterImagePolicy defines the images that go through verification
Expand All @@ -37,7 +39,7 @@ type Authority struct {
// +optional
Key *KeyRef `json:"key,omitempty"`
// +optional
Keyless *v1alpha1.KeylessRef `json:"keyless,omitempty"`
Keyless *KeylessRef `json:"keyless,omitempty"`
// +optional
Sources []v1alpha1.Source `json:"source,omitempty"`
// +optional
Expand All @@ -50,15 +52,21 @@ type KeyRef struct {
// Data contains the inline public key
// +optional
Data string `json:"data,omitempty"`
// 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:"-"`
}

type KeylessRef struct {
// +optional
URL *apis.URL `json:"url,omitempty"`
// +optional
Identities []v1alpha1.Identity `json:"identities,omitempty"`
// +optional
CACert *KeyRef `json:"ca-cert,omitempty"`
}

// UnmarshalJSON populates the PublicKeys using Data because
// JSON unmashalling errors for *big.Int
func (k *KeyRef) UnmarshalJSON(data []byte) error {
Expand All @@ -73,7 +81,7 @@ func (k *KeyRef) UnmarshalJSON(data []byte) error {
k.Data = ret["data"]

if ret["data"] != "" {
publicKeys, err = convertKeyDataToPublicKeys(ret["data"])
publicKeys, err = ConvertKeyDataToPublicKeys(ret["data"])
if err != nil {
return err
}
Expand All @@ -84,9 +92,59 @@ func (k *KeyRef) UnmarshalJSON(data []byte) error {
return nil
}

func convertKeyDataToPublicKeys(pubKey string) ([]*ecdsa.PublicKey, error) {
keys := []*ecdsa.PublicKey{}
func ConvertClusterImagePolicyV1alpha1ToWebhook(in *v1alpha1.ClusterImagePolicy) *ClusterImagePolicy {
copyIn := in.DeepCopy()

outAuthorities := make([]Authority, 0)
for _, authority := range copyIn.Spec.Authorities {
outAuthority := convertAuthorityV1Alpha1ToWebhook(authority)
outAuthorities = append(outAuthorities, *outAuthority)
}

return &ClusterImagePolicy{
Images: copyIn.Spec.Images,
Authorities: outAuthorities,
}
}

func convertAuthorityV1Alpha1ToWebhook(in v1alpha1.Authority) *Authority {
keyRef := convertKeyRefV1Alpha1ToWebhook(in.Key)
keylessRef := convertKeylessRefV1Alpha1ToWebhook(in.Keyless)

return &Authority{
Key: keyRef,
Keyless: keylessRef,
Sources: in.Sources,
CTLog: in.CTLog,
}
}

func convertKeyRefV1Alpha1ToWebhook(in *v1alpha1.KeyRef) *KeyRef {
if in == nil {
return nil
}

return &KeyRef{
Data: in.Data,
}
}

func convertKeylessRefV1Alpha1ToWebhook(in *v1alpha1.KeylessRef) *KeylessRef {
if in == nil {
return nil
}

CACertRef := convertKeyRefV1Alpha1ToWebhook(in.CACert)

return &KeylessRef{
URL: in.URL,
Identities: in.Identities,
CACert: CACertRef,
}
}

func ConvertKeyDataToPublicKeys(pubKey string) ([]*ecdsa.PublicKey, error) {
keys := []*ecdsa.PublicKey{}
pems := parsePems([]byte(pubKey))
for _, p := range pems {
key, err := x509.ParsePKIXPublicKey(p.Bytes)
Expand Down
6 changes: 3 additions & 3 deletions pkg/cosign/kubernetes/webhook/validator_test.go
View file
Expand Up @@ -272,7 +272,7 @@ UoJou2P8sbDxpLiE/v3yLw1/jyOrCPWYHWFXnyyeGlkgSVefG54tNoK7Uw==
}},
Authorities: []webhookcip.Authority{
{
Keyless: &v1alpha1.KeylessRef{
Keyless: &webhookcip.KeylessRef{
URL: badURL,
},
},
Expand Down Expand Up @@ -315,7 +315,7 @@ UoJou2P8sbDxpLiE/v3yLw1/jyOrCPWYHWFXnyyeGlkgSVefG54tNoK7Uw==
}},
Authorities: []webhookcip.Authority{
{
Keyless: &v1alpha1.KeylessRef{
Keyless: &webhookcip.KeylessRef{
URL: fulcioURL,
},
},
Expand Down Expand Up @@ -358,7 +358,7 @@ UoJou2P8sbDxpLiE/v3yLw1/jyOrCPWYHWFXnyyeGlkgSVefG54tNoK7Uw==
}},
Authorities: []webhookcip.Authority{
{
Keyless: &v1alpha1.KeylessRef{
Keyless: &webhookcip.KeylessRef{
URL: fulcioURL,
},
CTLog: &v1alpha1.TLog{
Expand Down
73 changes: 4 additions & 69 deletions pkg/reconciler/clusterimagepolicy/clusterimagepolicy.go
View file
Expand Up @@ -17,10 +17,6 @@ package clusterimagepolicy
import (
"context"
"crypto"
"crypto/ecdsa"
"crypto/x509"
"encoding/json"
"encoding/pem"
"fmt"
"strings"

Expand All @@ -30,12 +26,14 @@ import (
clusterimagepolicyreconciler "github.com/sigstore/cosign/pkg/client/injection/reconciler/cosigned/v1alpha1/clusterimagepolicy"
webhookcip "github.com/sigstore/cosign/pkg/cosign/kubernetes/webhook/clusterimagepolicy"
"github.com/sigstore/cosign/pkg/reconciler/clusterimagepolicy/resources"

corev1 "k8s.io/api/core/v1"
apierrs "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/kubernetes"
corev1listers "k8s.io/client-go/listers/core/v1"

"knative.dev/pkg/logging"
"knative.dev/pkg/reconciler"
"knative.dev/pkg/system"
Expand Down Expand Up @@ -66,31 +64,15 @@ var _ clusterimagepolicyreconciler.Finalizer = (*Reconciler)(nil)
// ReconcileKind implements Interface.ReconcileKind.
func (r *Reconciler) ReconcileKind(ctx context.Context, cip *v1alpha1.ClusterImagePolicy) reconciler.Event {
cipCopy, cipErr := r.inlinePublicKeys(ctx, cip)
if cipErr != nil {
// Handle error here
r.handleCIPError(ctx, cip.Name)
return cipErr
}

// Converting external CIP to webhook CIP
bytes, err := json.Marshal(&cipCopy.Spec)
if err != nil {
return err
}

var webhookCIP *webhookcip.ClusterImagePolicy
if err := json.Unmarshal(bytes, &webhookCIP); err != nil {
return err
}

webhookCIP, cipErr = r.convertKeyData(ctx, webhookCIP)
if cipErr != nil {
r.handleCIPError(ctx, cip.Name)
// Note that we return the error about the Invalid cip here to make
// sure that it's surfaced.
return cipErr
}

webhookCIP := webhookcip.ConvertClusterImagePolicyV1alpha1ToWebhook(cipCopy)

// See if the CM holding configs exists
existing, err := r.configmaplister.ConfigMaps(system.Namespace()).Get(config.ImagePoliciesConfigName)
if err != nil {
Expand Down Expand Up @@ -142,24 +124,6 @@ func (r *Reconciler) FinalizeKind(ctx context.Context, cip *v1alpha1.ClusterImag
return r.removeCIPEntry(ctx, existing, cip.Name)
}

// convertKeyData will go through the CIP and try to convert key data
// to ecdsa.PublicKey and store it in the returned CIP
// When PublicKeys are successfully set, the authority key's data will be
// cleared out
func (r *Reconciler) convertKeyData(ctx context.Context, cip *webhookcip.ClusterImagePolicy) (*webhookcip.ClusterImagePolicy, error) {
for _, authority := range cip.Authorities {
if authority.Key != nil && authority.Key.Data != "" {
keys, err := convertAuthorityKeys(ctx, authority.Key.Data)
if err != nil {
return nil, err
}
// When publicKeys are successfully converted, clear out Data
authority.Key.PublicKeys = keys
}
}
return cip, nil
}

func (r *Reconciler) handleCIPError(ctx context.Context, cipName string) {
// The CIP is invalid, try to remove CIP from the configmap
existing, err := r.configmaplister.ConfigMaps(system.Namespace()).Get(config.ImagePoliciesConfigName)
Expand All @@ -172,35 +136,6 @@ func (r *Reconciler) handleCIPError(ctx context.Context, cipName string) {
}
}

func convertAuthorityKeys(ctx context.Context, pubKey string) ([]*ecdsa.PublicKey, error) {
keys := []*ecdsa.PublicKey{}

logging.FromContext(ctx).Debugf("Got public key: %v", pubKey)

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
}

// inlinePublicKeys will go through the CIP and try to read the referenced
// secrets, KMS keys and convert them into inlined data. Makes a copy of the CIP
// before modifying it and returns the copy.
Expand Down
23 changes: 2 additions & 21 deletions pkg/reconciler/clusterimagepolicy/clusterimagepolicy_test.go
View file
Expand Up @@ -81,9 +81,6 @@ RCTfQ5s1kD+hGMSE1rH7s46hmXEeyhnlRnaGF8eMU/SBJE/2NKPnxE7WzQ==
// two entries but only one is being removed. For keyless entry.
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\":{\"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-----\"}}}]}"}]`
)
Expand Down Expand Up @@ -475,11 +472,10 @@ func TestReconcile(t *testing.T) {
},
}}),
),
makeConfigMapWithTwoEntriesNotPublicKeyFromSecret(),
makeSecret(keySecretName, validPublicKeyData),
},
WantPatches: []clientgotesting.PatchActionImpl{
makePatch(inlinedSecretKeyPatch),
WantCreates: []runtime.Object{
makeConfigMap(),
},
PostConditions: []func(*testing.T, *TableRow){
AssertTrackingSecret(system.Namespace(), keySecretName),
Expand Down Expand Up @@ -629,21 +625,6 @@ func makeConfigMapWithTwoEntries() *corev1.ConfigMap {
}
}

// Same as MakeConfigMapWithTwoEntries but the inline data is not the secret
// so we will replace it with the secret data
func makeConfigMapWithTwoEntriesNotPublicKeyFromSecret() *corev1.ConfigMap {
return &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Namespace: system.Namespace(),
Name: config.ImagePoliciesConfigName,
},
Data: map[string]string{
cipName: `{"images":[{"glob":"ghcr.io/example/*"}],"authorities":[{"key":{"data":"NOT A REAL PUBLIC KEY"}}]}`,
cipName2: "remove me please",
},
}
}

func makePatch(patch string) clientgotesting.PatchActionImpl {
return clientgotesting.PatchActionImpl{
ActionImpl: clientgotesting.ActionImpl{
Expand Down

0 comments on commit 93d5b5c

Please sign in to comment.