From 0c19a3c0da53240ab2000da4dcae7d61b50f4027 Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Tue, 3 Mar 2020 13:14:04 +0000 Subject: [PATCH] Extend client-go csr package to invalidate CSRs based on signerName Kubernetes-commit: c2367bd5da68112ad3031dd33933859dacf8db58 --- util/certificate/csr/csr.go | 3 + util/certificate/csr/csr_test.go | 146 +++++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+) create mode 100644 util/certificate/csr/csr_test.go diff --git a/util/certificate/csr/csr.go b/util/certificate/csr/csr.go index c5766d24..dd417724 100644 --- a/util/certificate/csr/csr.go +++ b/util/certificate/csr/csr.go @@ -150,6 +150,9 @@ func ensureCompatible(new, orig *certificates.CertificateSigningRequest, private if !reflect.DeepEqual(newCSR.Subject, origCSR.Subject) { return fmt.Errorf("csr subjects differ: new: %#v, orig: %#v", newCSR.Subject, origCSR.Subject) } + if new.Spec.SignerName != nil && orig.Spec.SignerName != nil && *new.Spec.SignerName != *orig.Spec.SignerName { + return fmt.Errorf("csr signerNames differ: new %q, orig: %q", *new.Spec.SignerName, *orig.Spec.SignerName) + } signer, ok := privateKey.(crypto.Signer) if !ok { return fmt.Errorf("privateKey is not a signer") diff --git a/util/certificate/csr/csr_test.go b/util/certificate/csr/csr_test.go new file mode 100644 index 00000000..63c2e827 --- /dev/null +++ b/util/certificate/csr/csr_test.go @@ -0,0 +1,146 @@ +/* +Copyright 2020 The Kubernetes 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 csr + +import ( + "crypto" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "testing" + + certificates "k8s.io/api/certificates/v1beta1" + "k8s.io/utils/pointer" +) + +func TestEnsureCompatible(t *testing.T) { + privateKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatal(err) + } + req := pemWithPrivateKey(privateKey) + + tests := map[string]struct { + new, orig *certificates.CertificateSigningRequest + privateKey interface{} + err string + }{ + "nil signerName on 'new' matches any signerName on 'orig'": { + new: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + Request: req, + }, + }, + orig: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + Request: req, + SignerName: pointer.StringPtr("example.com/test"), + }, + }, + privateKey: privateKey, + }, + "nil signerName on 'orig' matches any signerName on 'new'": { + new: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + Request: req, + SignerName: pointer.StringPtr("example.com/test"), + }, + }, + orig: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + Request: req, + }, + }, + privateKey: privateKey, + }, + "signerName on 'orig' matches signerName on 'new'": { + new: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + Request: req, + SignerName: pointer.StringPtr("example.com/test"), + }, + }, + orig: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + Request: req, + SignerName: pointer.StringPtr("example.com/test"), + }, + }, + privateKey: privateKey, + }, + "signerName on 'orig' does not match signerName on 'new'": { + new: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + Request: req, + SignerName: pointer.StringPtr("example.com/test"), + }, + }, + orig: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + Request: req, + SignerName: pointer.StringPtr("example.com/not-test"), + }, + }, + privateKey: privateKey, + err: `csr signerNames differ: new "example.com/test", orig: "example.com/not-test"`, + }, + } + for name, test := range tests { + t.Run(name, func(t *testing.T) { + err := ensureCompatible(test.new, test.orig, test.privateKey) + if err != nil && test.err == "" { + t.Errorf("expected no error, but got: %v", err) + } else if err != nil && test.err != err.Error() { + t.Errorf("error did not match as expected, got=%v, exp=%s", err, test.err) + } + if err == nil && test.err != "" { + t.Errorf("expected to get an error but got none") + } + }) + } +} + +func pemWithPrivateKey(pk crypto.PrivateKey) []byte { + template := &x509.CertificateRequest{ + Subject: pkix.Name{ + CommonName: "something", + Organization: []string{"test"}, + }, + } + return pemWithTemplate(template, pk) +} + +func pemWithTemplate(template *x509.CertificateRequest, key crypto.PrivateKey) []byte { + csrDER, err := x509.CreateCertificateRequest(rand.Reader, template, key) + if err != nil { + panic(err) + } + + csrPemBlock := &pem.Block{ + Type: "CERTIFICATE REQUEST", + Bytes: csrDER, + } + + p := pem.EncodeToMemory(csrPemBlock) + if p == nil { + panic("invalid pem block") + } + + return p +}