From 4bd2c3998f5b3cb8e7fdec304368dbd2f54edf2b Mon Sep 17 00:00:00 2001 From: Mike Danese Date: Sat, 2 Nov 2019 12:26:45 -0700 Subject: [PATCH] don't use cfssl in signer --- pkg/controller/.import-restrictions | 7 +- .../certificates/approver/sarapprove.go | 4 +- pkg/controller/certificates/signer/BUILD | 11 +- .../certificates/signer/cfssl_signer.go | 153 ------------- pkg/controller/certificates/signer/signer.go | 215 ++++++++++++++++++ .../{cfssl_signer_test.go => signer_test.go} | 74 +++++- 6 files changed, 294 insertions(+), 170 deletions(-) delete mode 100644 pkg/controller/certificates/signer/cfssl_signer.go create mode 100644 pkg/controller/certificates/signer/signer.go rename pkg/controller/certificates/signer/{cfssl_signer_test.go => signer_test.go} (68%) diff --git a/pkg/controller/.import-restrictions b/pkg/controller/.import-restrictions index 74c0942beb4..a56a130fa3e 100644 --- a/pkg/controller/.import-restrictions +++ b/pkg/controller/.import-restrictions @@ -118,6 +118,7 @@ { "SelectorRegexp": "k8s[.]io/client-go/", "AllowedPrefixes": [ + "k8s.io/client-go/util/keyutil", "k8s.io/client-go/discovery", "k8s.io/client-go/dynamic", "k8s.io/client-go/informers", @@ -146,9 +147,9 @@ "k8s.io/client-go/listers/autoscaling/v1", "k8s.io/client-go/listers/batch/v1", "k8s.io/client-go/listers/certificates/v1beta1", + "k8s.io/client-go/listers/coordination/v1", "k8s.io/client-go/listers/core/v1", "k8s.io/client-go/listers/discovery/v1alpha1", - "k8s.io/client-go/listers/coordination/v1", "k8s.io/client-go/listers/extensions/v1beta1", "k8s.io/client-go/listers/policy/v1beta1", "k8s.io/client-go/listers/rbac/v1", @@ -164,12 +165,12 @@ "k8s.io/client-go/tools/record", "k8s.io/client-go/tools/reference", "k8s.io/client-go/tools/watch", + "k8s.io/client-go/transport", "k8s.io/client-go/util/cert", "k8s.io/client-go/util/flowcontrol", "k8s.io/client-go/util/retry", - "k8s.io/client-go/util/workqueue", "k8s.io/client-go/util/testing", - "k8s.io/client-go/transport" + "k8s.io/client-go/util/workqueue" ] }, { diff --git a/pkg/controller/certificates/approver/sarapprove.go b/pkg/controller/certificates/approver/sarapprove.go index 58b497755eb..af8fe29d8ab 100644 --- a/pkg/controller/certificates/approver/sarapprove.go +++ b/pkg/controller/certificates/approver/sarapprove.go @@ -27,7 +27,7 @@ import ( capi "k8s.io/api/certificates/v1beta1" certificatesinformers "k8s.io/client-go/informers/certificates/v1beta1" clientset "k8s.io/client-go/kubernetes" - k8s_certificates_v1beta1 "k8s.io/kubernetes/pkg/apis/certificates/v1beta1" + capihelper "k8s.io/kubernetes/pkg/apis/certificates/v1beta1" "k8s.io/kubernetes/pkg/controller/certificates" ) @@ -79,7 +79,7 @@ func (a *sarApprover) handle(csr *capi.CertificateSigningRequest) error { if approved, denied := certificates.GetCertApprovalCondition(&csr.Status); approved || denied { return nil } - x509cr, err := k8s_certificates_v1beta1.ParseCSR(csr) + x509cr, err := capihelper.ParseCSR(csr) if err != nil { return fmt.Errorf("unable to parse csr %q: %v", csr.Name, err) } diff --git a/pkg/controller/certificates/signer/BUILD b/pkg/controller/certificates/signer/BUILD index f7f9d0744fa..96f4f691f22 100644 --- a/pkg/controller/certificates/signer/BUILD +++ b/pkg/controller/certificates/signer/BUILD @@ -8,7 +8,7 @@ load( go_test( name = "go_default_test", - srcs = ["cfssl_signer_test.go"], + srcs = ["signer_test.go"], data = [ "testdata/ca.crt", "testdata/ca.key", @@ -23,17 +23,16 @@ go_test( go_library( name = "go_default_library", - srcs = ["cfssl_signer.go"], + srcs = ["signer.go"], importpath = "k8s.io/kubernetes/pkg/controller/certificates/signer", deps = [ + "//pkg/apis/certificates/v1beta1:go_default_library", "//pkg/controller/certificates:go_default_library", "//staging/src/k8s.io/api/certificates/v1beta1:go_default_library", "//staging/src/k8s.io/client-go/informers/certificates/v1beta1:go_default_library", "//staging/src/k8s.io/client-go/kubernetes:go_default_library", - "//vendor/github.com/cloudflare/cfssl/config:go_default_library", - "//vendor/github.com/cloudflare/cfssl/helpers:go_default_library", - "//vendor/github.com/cloudflare/cfssl/signer:go_default_library", - "//vendor/github.com/cloudflare/cfssl/signer/local:go_default_library", + "//staging/src/k8s.io/client-go/util/cert:go_default_library", + "//staging/src/k8s.io/client-go/util/keyutil:go_default_library", ], ) diff --git a/pkg/controller/certificates/signer/cfssl_signer.go b/pkg/controller/certificates/signer/cfssl_signer.go deleted file mode 100644 index eab7e8a939c..00000000000 --- a/pkg/controller/certificates/signer/cfssl_signer.go +++ /dev/null @@ -1,153 +0,0 @@ -/* -Copyright 2016 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 signer implements a CA signer that uses keys stored on local disk. -package signer - -import ( - "crypto" - "crypto/x509" - "fmt" - "io/ioutil" - "os" - "time" - - capi "k8s.io/api/certificates/v1beta1" - certificatesinformers "k8s.io/client-go/informers/certificates/v1beta1" - clientset "k8s.io/client-go/kubernetes" - "k8s.io/kubernetes/pkg/controller/certificates" - - "github.com/cloudflare/cfssl/config" - "github.com/cloudflare/cfssl/helpers" - "github.com/cloudflare/cfssl/signer" - "github.com/cloudflare/cfssl/signer/local" -) - -func NewCSRSigningController( - client clientset.Interface, - csrInformer certificatesinformers.CertificateSigningRequestInformer, - caFile, caKeyFile string, - certificateDuration time.Duration, -) (*certificates.CertificateController, error) { - signer, err := newCFSSLSigner(caFile, caKeyFile, client, certificateDuration) - if err != nil { - return nil, err - } - return certificates.NewCertificateController( - "csrsigning", - client, - csrInformer, - signer.handle, - ), nil -} - -type cfsslSigner struct { - ca *x509.Certificate - priv crypto.Signer - sigAlgo x509.SignatureAlgorithm - client clientset.Interface - certificateDuration time.Duration - - // nowFn returns the current time. We have here for unit testing - nowFn func() time.Time -} - -func newCFSSLSigner(caFile, caKeyFile string, client clientset.Interface, certificateDuration time.Duration) (*cfsslSigner, error) { - ca, err := ioutil.ReadFile(caFile) - if err != nil { - return nil, fmt.Errorf("error reading CA cert file %q: %v", caFile, err) - } - cakey, err := ioutil.ReadFile(caKeyFile) - if err != nil { - return nil, fmt.Errorf("error reading CA key file %q: %v", caKeyFile, err) - } - - parsedCa, err := helpers.ParseCertificatePEM(ca) - if err != nil { - return nil, fmt.Errorf("error parsing CA cert file %q: %v", caFile, err) - } - - strPassword := os.Getenv("CFSSL_CA_PK_PASSWORD") - password := []byte(strPassword) - if strPassword == "" { - password = nil - } - - priv, err := helpers.ParsePrivateKeyPEMWithPassword(cakey, password) - if err != nil { - return nil, fmt.Errorf("malformed private key %v", err) - } - return &cfsslSigner{ - priv: priv, - ca: parsedCa, - sigAlgo: signer.DefaultSigAlgo(priv), - client: client, - certificateDuration: certificateDuration, - nowFn: time.Now, - }, nil -} - -func (s *cfsslSigner) handle(csr *capi.CertificateSigningRequest) error { - if !certificates.IsCertificateRequestApproved(csr) { - return nil - } - csr, err := s.sign(csr) - if err != nil { - return fmt.Errorf("error auto signing csr: %v", err) - } - _, err = s.client.CertificatesV1beta1().CertificateSigningRequests().UpdateStatus(csr) - if err != nil { - return fmt.Errorf("error updating signature for csr: %v", err) - } - return nil -} - -func (s *cfsslSigner) sign(csr *capi.CertificateSigningRequest) (*capi.CertificateSigningRequest, error) { - var usages []string - for _, usage := range csr.Spec.Usages { - usages = append(usages, string(usage)) - } - - certExpiryDuration := s.certificateDuration - durationUntilExpiry := s.ca.NotAfter.Sub(s.nowFn()) - if durationUntilExpiry <= 0 { - return nil, fmt.Errorf("the signer has expired: %v", s.ca.NotAfter) - } - if durationUntilExpiry < certExpiryDuration { - certExpiryDuration = durationUntilExpiry - } - - policy := &config.Signing{ - Default: &config.SigningProfile{ - Usage: usages, - Expiry: certExpiryDuration, - ExpiryString: certExpiryDuration.String(), - }, - } - cfs, err := local.NewSigner(s.priv, s.ca, s.sigAlgo, policy) - if err != nil { - return nil, err - } - - csr.Status.Certificate, err = cfs.Sign(signer.SignRequest{ - Request: string(csr.Spec.Request), - }) - if err != nil { - return nil, err - } - - return csr, nil -} diff --git a/pkg/controller/certificates/signer/signer.go b/pkg/controller/certificates/signer/signer.go new file mode 100644 index 00000000000..e76794deb0f --- /dev/null +++ b/pkg/controller/certificates/signer/signer.go @@ -0,0 +1,215 @@ +/* +Copyright 2019 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 signer implements a CA signer that uses keys stored on local disk. +package signer + +import ( + "crypto/rand" + "crypto/x509" + "encoding/pem" + "fmt" + "io/ioutil" + "math/big" + "time" + + capi "k8s.io/api/certificates/v1beta1" + certificatesinformers "k8s.io/client-go/informers/certificates/v1beta1" + clientset "k8s.io/client-go/kubernetes" + "k8s.io/client-go/util/cert" + "k8s.io/client-go/util/keyutil" + capihelper "k8s.io/kubernetes/pkg/apis/certificates/v1beta1" + "k8s.io/kubernetes/pkg/controller/certificates" +) + +func NewCSRSigningController( + client clientset.Interface, + csrInformer certificatesinformers.CertificateSigningRequestInformer, + caFile, caKeyFile string, + certificateDuration time.Duration, +) (*certificates.CertificateController, error) { + signer, err := newSigner(caFile, caKeyFile, client, certificateDuration) + if err != nil { + return nil, err + } + return certificates.NewCertificateController( + "csrsigning", + client, + csrInformer, + signer.handle, + ), nil +} + +type signer struct { + ca *x509.Certificate + priv interface{} + client clientset.Interface + certTTL time.Duration + + // now is mocked for testing. + now func() time.Time +} + +func newSigner(caFile, caKeyFile string, client clientset.Interface, certificateDuration time.Duration) (*signer, error) { + ca, err := ioutil.ReadFile(caFile) + if err != nil { + return nil, fmt.Errorf("error reading CA cert file %q: %v", caFile, err) + } + cakey, err := ioutil.ReadFile(caKeyFile) + if err != nil { + return nil, fmt.Errorf("error reading CA key file %q: %v", caKeyFile, err) + } + + certs, err := cert.ParseCertsPEM(ca) + if err != nil { + return nil, fmt.Errorf("error parsing CA cert file %q: %v", caFile, err) + } + if len(certs) != 1 { + return nil, fmt.Errorf("error parsing CA cert file %q: expected one certificate block", caFile) + } + + priv, err := keyutil.ParsePrivateKeyPEM(cakey) + if err != nil { + return nil, fmt.Errorf("malformed private key %v", err) + } + return &signer{ + priv: priv, + ca: certs[0], + client: client, + certTTL: certificateDuration, + now: time.Now, + }, nil +} + +func (s *signer) handle(csr *capi.CertificateSigningRequest) error { + if !certificates.IsCertificateRequestApproved(csr) { + return nil + } + csr, err := s.sign(csr) + if err != nil { + return fmt.Errorf("error auto signing csr: %v", err) + } + _, err = s.client.CertificatesV1beta1().CertificateSigningRequests().UpdateStatus(csr) + if err != nil { + return fmt.Errorf("error updating signature for csr: %v", err) + } + return nil +} + +func (s *signer) sign(csr *capi.CertificateSigningRequest) (*capi.CertificateSigningRequest, error) { + x509cr, err := capihelper.ParseCSR(csr) + if err != nil { + return nil, fmt.Errorf("unable to parse csr %q: %v", csr.Name, err) + } + + usage, extUsages, err := keyUsagesFromStrings(csr.Spec.Usages) + if err != nil { + return nil, err + } + + now := s.now() + expiry := now.Add(s.certTTL) + if s.ca.NotAfter.Before(expiry) { + expiry = s.ca.NotAfter + } + if expiry.Before(now) { + return nil, fmt.Errorf("the signer has expired: NotAfter=%v", s.ca.NotAfter) + } + + serialNumberLimit := new(big.Int).Lsh(big.NewInt(1), 128) + serialNumber, err := rand.Int(rand.Reader, serialNumberLimit) + if err != nil { + return nil, fmt.Errorf("unable to generate a serial number for %s: %v", x509cr.Subject.CommonName, err) + } + + template := &x509.Certificate{ + SerialNumber: serialNumber, + Subject: x509cr.Subject, + DNSNames: x509cr.DNSNames, + IPAddresses: x509cr.IPAddresses, + EmailAddresses: x509cr.EmailAddresses, + URIs: x509cr.URIs, + PublicKeyAlgorithm: x509cr.PublicKeyAlgorithm, + PublicKey: x509cr.PublicKey, + NotBefore: now, + NotAfter: expiry, + KeyUsage: usage, + ExtKeyUsage: extUsages, + BasicConstraintsValid: true, + IsCA: false, + } + der, err := x509.CreateCertificate(rand.Reader, template, s.ca, x509cr.PublicKey, s.priv) + if err != nil { + return nil, err + } + csr.Status.Certificate = pem.EncodeToMemory(&pem.Block{Type: cert.CertificateBlockType, Bytes: der}) + _ = der + + return csr, nil +} + +var keyUsageDict = map[capi.KeyUsage]x509.KeyUsage{ + capi.UsageSigning: x509.KeyUsageDigitalSignature, + capi.UsageDigitalSignature: x509.KeyUsageDigitalSignature, + capi.UsageContentCommitment: x509.KeyUsageContentCommitment, + capi.UsageKeyEncipherment: x509.KeyUsageKeyEncipherment, + capi.UsageKeyAgreement: x509.KeyUsageKeyAgreement, + capi.UsageDataEncipherment: x509.KeyUsageDataEncipherment, + capi.UsageCertSign: x509.KeyUsageCertSign, + capi.UsageCRLSign: x509.KeyUsageCRLSign, + capi.UsageEncipherOnly: x509.KeyUsageEncipherOnly, + capi.UsageDecipherOnly: x509.KeyUsageDecipherOnly, +} + +var extKeyUsageDict = map[capi.KeyUsage]x509.ExtKeyUsage{ + capi.UsageAny: x509.ExtKeyUsageAny, + capi.UsageServerAuth: x509.ExtKeyUsageServerAuth, + capi.UsageClientAuth: x509.ExtKeyUsageClientAuth, + capi.UsageCodeSigning: x509.ExtKeyUsageCodeSigning, + capi.UsageEmailProtection: x509.ExtKeyUsageEmailProtection, + capi.UsageSMIME: x509.ExtKeyUsageEmailProtection, + capi.UsageIPsecEndSystem: x509.ExtKeyUsageIPSECEndSystem, + capi.UsageIPsecTunnel: x509.ExtKeyUsageIPSECTunnel, + capi.UsageIPsecUser: x509.ExtKeyUsageIPSECUser, + capi.UsageTimestamping: x509.ExtKeyUsageTimeStamping, + capi.UsageOCSPSigning: x509.ExtKeyUsageOCSPSigning, + capi.UsageMicrosoftSGC: x509.ExtKeyUsageMicrosoftServerGatedCrypto, + capi.UsageNetscapeSGC: x509.ExtKeyUsageNetscapeServerGatedCrypto, +} + +// keyUsagesFromStrings will translate a slice of usage strings from the +// certificates API ("pkg/apis/certificates".KeyUsage) to x509.KeyUsage and +// x509.ExtKeyUsage types. +func keyUsagesFromStrings(usages []capi.KeyUsage) (x509.KeyUsage, []x509.ExtKeyUsage, error) { + var keyUsage x509.KeyUsage + var extKeyUsage []x509.ExtKeyUsage + var unrecognized []capi.KeyUsage + for _, usage := range usages { + if val, ok := keyUsageDict[usage]; ok { + keyUsage |= val + } else if val, ok := extKeyUsageDict[usage]; ok { + extKeyUsage = append(extKeyUsage, val) + } else { + unrecognized = append(unrecognized, usage) + } + } + + if len(unrecognized) > 0 { + return 0, nil, fmt.Errorf("unrecognized usage values: %q", unrecognized) + } + + return keyUsage, extKeyUsage, nil +} diff --git a/pkg/controller/certificates/signer/cfssl_signer_test.go b/pkg/controller/certificates/signer/signer_test.go similarity index 68% rename from pkg/controller/certificates/signer/cfssl_signer_test.go rename to pkg/controller/certificates/signer/signer_test.go index d7ea1a95de1..5792c77c116 100644 --- a/pkg/controller/certificates/signer/cfssl_signer_test.go +++ b/pkg/controller/certificates/signer/signer_test.go @@ -18,6 +18,7 @@ package signer import ( "crypto/x509" + "fmt" "io/ioutil" "reflect" "strings" @@ -34,11 +35,11 @@ func TestSigner(t *testing.T) { return testNow } - s, err := newCFSSLSigner("./testdata/ca.crt", "./testdata/ca.key", nil, 1*time.Hour) + s, err := newSigner("./testdata/ca.crt", "./testdata/ca.key", nil, 1*time.Hour) if err != nil { t.Fatalf("failed to create signer: %v", err) } - s.nowFn = testNowFn + s.now = testNowFn csrb, err := ioutil.ReadFile("./testdata/kubelet.csr") if err != nil { @@ -101,11 +102,11 @@ func TestSignerExpired(t *testing.T) { hundredYearsFromNowFn := func() time.Time { return time.Now().Add(24 * time.Hour * 365 * 100) } - s, err := newCFSSLSigner("./testdata/ca.crt", "./testdata/ca.key", nil, 1*time.Hour) + s, err := newSigner("./testdata/ca.crt", "./testdata/ca.key", nil, 1*time.Hour) if err != nil { t.Fatalf("failed to create signer: %v", err) } - s.nowFn = hundredYearsFromNowFn + s.now = hundredYearsFromNowFn csrb, err := ioutil.ReadFile("./testdata/kubelet.csr") if err != nil { @@ -140,11 +141,11 @@ func TestDurationLongerThanExpiry(t *testing.T) { } hundredYears := 24 * time.Hour * 365 * 100 - s, err := newCFSSLSigner("./testdata/ca.crt", "./testdata/ca.key", nil, hundredYears) + s, err := newSigner("./testdata/ca.crt", "./testdata/ca.key", nil, hundredYears) if err != nil { t.Fatalf("failed to create signer: %v", err) } - s.nowFn = testNowFn + s.now = testNowFn csrb, err := ioutil.ReadFile("./testdata/kubelet.csr") if err != nil { @@ -193,3 +194,64 @@ func TestDurationLongerThanExpiry(t *testing.T) { t.Fatal(crt.NotAfter) } } + +func TestKeyUsagesFromStrings(t *testing.T) { + testcases := []struct { + usages []capi.KeyUsage + expectedKeyUsage x509.KeyUsage + expectedExtKeyUsage []x509.ExtKeyUsage + expectErr bool + }{ + { + usages: []capi.KeyUsage{"signing"}, + expectedKeyUsage: x509.KeyUsageDigitalSignature, + expectedExtKeyUsage: nil, + expectErr: false, + }, + { + usages: []capi.KeyUsage{"client auth"}, + expectedKeyUsage: 0, + expectedExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth}, + expectErr: false, + }, + { + usages: []capi.KeyUsage{"cert sign", "encipher only"}, + expectedKeyUsage: x509.KeyUsageCertSign | x509.KeyUsageEncipherOnly, + expectedExtKeyUsage: nil, + expectErr: false, + }, + { + usages: []capi.KeyUsage{"ocsp signing", "crl sign", "s/mime", "content commitment"}, + expectedKeyUsage: x509.KeyUsageCRLSign | x509.KeyUsageContentCommitment, + expectedExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageOCSPSigning, x509.ExtKeyUsageEmailProtection}, + expectErr: false, + }, + { + usages: []capi.KeyUsage{"unsupported string"}, + expectedKeyUsage: 0, + expectedExtKeyUsage: nil, + expectErr: true, + }, + } + + for _, tc := range testcases { + t.Run(fmt.Sprint(tc.usages), func(t *testing.T) { + ku, eku, err := keyUsagesFromStrings(tc.usages) + + if tc.expectErr { + if err == nil { + t.Errorf("did not return an error, but expected one") + } + return + } + + if err != nil { + t.Errorf("unexpected error: %v", err) + } + + if ku != tc.expectedKeyUsage || !reflect.DeepEqual(eku, tc.expectedExtKeyUsage) { + t.Errorf("got=(%v, %v), want=(%v, %v)", ku, eku, tc.expectedKeyUsage, tc.expectedExtKeyUsage) + } + }) + } +}