diff --git a/cmd/kubelet/app/BUILD b/cmd/kubelet/app/BUILD index b9551f8ad93..b54ad02353a 100644 --- a/cmd/kubelet/app/BUILD +++ b/cmd/kubelet/app/BUILD @@ -14,15 +14,14 @@ go_test( ], embed = [":go_default_library"], deps = [ + "//pkg/apis/certificates/v1beta1:go_default_library", + "//pkg/controller/certificates/authority:go_default_library", "//staging/src/k8s.io/api/certificates/v1beta1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", "//staging/src/k8s.io/client-go/rest:go_default_library", "//staging/src/k8s.io/client-go/util/cert:go_default_library", - "//vendor/github.com/cloudflare/cfssl/config:go_default_library", - "//vendor/github.com/cloudflare/cfssl/signer:go_default_library", - "//vendor/github.com/cloudflare/cfssl/signer/local:go_default_library", ], ) diff --git a/cmd/kubelet/app/server_bootstrap_test.go b/cmd/kubelet/app/server_bootstrap_test.go index 5b686e2e550..7da288ab1b5 100644 --- a/cmd/kubelet/app/server_bootstrap_test.go +++ b/cmd/kubelet/app/server_bootstrap_test.go @@ -34,16 +34,14 @@ import ( "testing" "time" - cfsslconfig "github.com/cloudflare/cfssl/config" - cfsslsigner "github.com/cloudflare/cfssl/signer" - cfssllocal "github.com/cloudflare/cfssl/signer/local" - certapi "k8s.io/api/certificates/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" restclient "k8s.io/client-go/rest" certutil "k8s.io/client-go/util/cert" + capihelper "k8s.io/kubernetes/pkg/apis/certificates/v1beta1" + "k8s.io/kubernetes/pkg/controller/certificates/authority" ) // Test_buildClientCertificateManager validates that we can build a local client cert @@ -297,28 +295,22 @@ func (s *csrSimulator) ServeHTTP(w http.ResponseWriter, req *http.Request) { csr = csr.DeepCopy() csr.ResourceVersion = "2" - var usages []string - for _, usage := range csr.Spec.Usages { - usages = append(usages, string(usage)) + ca := &authority.CertificateAuthority{ + Certificate: s.serverCA, + PrivateKey: s.serverPrivateKey, + Backdate: s.backdate, } - policy := &cfsslconfig.Signing{ - Default: &cfsslconfig.SigningProfile{ - Usage: usages, - Expiry: time.Hour, - ExpiryString: time.Hour.String(), - Backdate: s.backdate, - }, - } - cfs, err := cfssllocal.NewSigner(s.serverPrivateKey, s.serverCA, cfsslsigner.DefaultSigAlgo(s.serverPrivateKey), policy) + cr, err := capihelper.ParseCSR(csr) if err != nil { t.Fatal(err) } - csr.Status.Certificate, err = cfs.Sign(cfsslsigner.SignRequest{ - Request: string(csr.Spec.Request), + der, err := ca.Sign(cr.Raw, authority.PermissiveSigningPolicy{ + TTL: time.Hour, }) if err != nil { t.Fatal(err) } + csr.Status.Certificate = pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}) csr.Status.Conditions = []certapi.CertificateSigningRequestCondition{ {Type: certapi.CertificateApproved}, } diff --git a/pkg/controller/certificates/BUILD b/pkg/controller/certificates/BUILD index 26af7005886..d6075fcb7a8 100644 --- a/pkg/controller/certificates/BUILD +++ b/pkg/controller/certificates/BUILD @@ -42,6 +42,7 @@ filegroup( srcs = [ ":package-srcs", "//pkg/controller/certificates/approver:all-srcs", + "//pkg/controller/certificates/authority:all-srcs", "//pkg/controller/certificates/cleaner:all-srcs", "//pkg/controller/certificates/rootcacertpublisher:all-srcs", "//pkg/controller/certificates/signer:all-srcs", diff --git a/pkg/controller/certificates/authority/BUILD b/pkg/controller/certificates/authority/BUILD new file mode 100644 index 00000000000..7c109cc8f18 --- /dev/null +++ b/pkg/controller/certificates/authority/BUILD @@ -0,0 +1,47 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "go_default_library", + srcs = [ + "authority.go", + "policies.go", + ], + importpath = "k8s.io/kubernetes/pkg/controller/certificates/authority", + visibility = [ + # This cmd/kubelet dependency is used for testing. We should migrate + # that test to an integration test that uses the certificates + # controller, and remove the dependency. + "//cmd/kubelet/app:__pkg__", + "//pkg/controller/certificates:__subpackages__", + ], + deps = ["//staging/src/k8s.io/api/certificates/v1beta1:go_default_library"], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) + +go_test( + name = "go_default_test", + srcs = [ + "authority_test.go", + "policies_test.go", + ], + embed = [":go_default_library"], + deps = [ + "//staging/src/k8s.io/api/certificates/v1beta1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library", + "//vendor/github.com/google/go-cmp/cmp:go_default_library", + "//vendor/github.com/google/go-cmp/cmp/cmpopts:go_default_library", + ], +) diff --git a/pkg/controller/certificates/authority/authority.go b/pkg/controller/certificates/authority/authority.go new file mode 100644 index 00000000000..a02833625b6 --- /dev/null +++ b/pkg/controller/certificates/authority/authority.go @@ -0,0 +1,93 @@ +/* +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 authority + +import ( + "crypto" + "crypto/rand" + "crypto/x509" + "fmt" + "math/big" + "time" +) + +var serialNumberLimit = new(big.Int).Lsh(big.NewInt(1), 128) + +// CertificateAuthority implements a certificate authority that supports policy +// based signing. It's used by the signing controller. +type CertificateAuthority struct { + Certificate *x509.Certificate + PrivateKey crypto.Signer + Backdate time.Duration + Now func() time.Time +} + +// Sign signs a certificate request, applying a SigningPolicy and returns a DER +// encoded x509 certificate. +func (ca *CertificateAuthority) Sign(crDER []byte, policy SigningPolicy) ([]byte, error) { + now := time.Now() + if ca.Now != nil { + now = ca.Now() + } + + nbf := now.Add(-ca.Backdate) + if !nbf.Before(ca.Certificate.NotAfter) { + return nil, fmt.Errorf("the signer has expired: NotAfter=%v", ca.Certificate.NotAfter) + } + + cr, err := x509.ParseCertificateRequest(crDER) + if err != nil { + return nil, fmt.Errorf("unable to parse certificate request: %v", err) + } + if err := cr.CheckSignature(); err != nil { + return nil, fmt.Errorf("unable to verify certificate request signature: %v", err) + } + + serialNumber, err := rand.Int(rand.Reader, serialNumberLimit) + if err != nil { + return nil, fmt.Errorf("unable to generate a serial number for %s: %v", cr.Subject.CommonName, err) + } + + tmpl := &x509.Certificate{ + SerialNumber: serialNumber, + Subject: cr.Subject, + DNSNames: cr.DNSNames, + IPAddresses: cr.IPAddresses, + EmailAddresses: cr.EmailAddresses, + PublicKeyAlgorithm: cr.PublicKeyAlgorithm, + PublicKey: cr.PublicKey, + Extensions: cr.Extensions, + ExtraExtensions: cr.ExtraExtensions, + NotBefore: nbf, + } + if err := policy.apply(tmpl); err != nil { + return nil, err + } + + if !tmpl.NotAfter.Before(ca.Certificate.NotAfter) { + tmpl.NotAfter = ca.Certificate.NotAfter + } + if !now.Before(ca.Certificate.NotAfter) { + return nil, fmt.Errorf("refusing to sign a certificate that expired in the past") + } + + der, err := x509.CreateCertificate(rand.Reader, tmpl, ca.Certificate, cr.PublicKey, ca.PrivateKey) + if err != nil { + return nil, fmt.Errorf("failed to sign certificate: %v", err) + } + return der, nil +} diff --git a/pkg/controller/certificates/authority/authority_test.go b/pkg/controller/certificates/authority/authority_test.go new file mode 100644 index 00000000000..f2891aec9ff --- /dev/null +++ b/pkg/controller/certificates/authority/authority_test.go @@ -0,0 +1,177 @@ +/* +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 authority + +import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "math/big" + "testing" + "time" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + + capi "k8s.io/api/certificates/v1beta1" + "k8s.io/apimachinery/pkg/util/diff" +) + +func TestCertificateAuthority(t *testing.T) { + caKey, err := ecdsa.GenerateKey(elliptic.P224(), rand.Reader) + if err != nil { + t.Fatal(err) + } + now := time.Now() + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(42), + Subject: pkix.Name{ + CommonName: "test-ca", + }, + NotBefore: now.Add(-24 * time.Hour), + NotAfter: now.Add(24 * time.Hour), + KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, + BasicConstraintsValid: true, + IsCA: true, + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, caKey.Public(), caKey) + if err != nil { + t.Fatal(err) + } + caCert, err := x509.ParseCertificate(der) + if err != nil { + t.Fatal(err) + } + + tests := []struct { + name string + cr x509.CertificateRequest + backdate time.Duration + policy SigningPolicy + + want x509.Certificate + wantErr bool + }{ + { + name: "ca info", + policy: PermissiveSigningPolicy{TTL: time.Hour}, + want: x509.Certificate{ + Issuer: caCert.Subject, + AuthorityKeyId: caCert.SubjectKeyId, + NotBefore: now, + NotAfter: now.Add(1 * time.Hour), + BasicConstraintsValid: true, + }, + }, + { + name: "key usage", + policy: PermissiveSigningPolicy{TTL: time.Hour, Usages: []capi.KeyUsage{"signing"}}, + want: x509.Certificate{ + NotBefore: now, + NotAfter: now.Add(1 * time.Hour), + BasicConstraintsValid: true, + KeyUsage: x509.KeyUsageDigitalSignature, + }, + }, + { + name: "ext key usage", + policy: PermissiveSigningPolicy{TTL: time.Hour, Usages: []capi.KeyUsage{"client auth"}}, + want: x509.Certificate{ + NotBefore: now, + NotAfter: now.Add(1 * time.Hour), + BasicConstraintsValid: true, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth}, + }, + }, + { + name: "backdate", + policy: PermissiveSigningPolicy{TTL: time.Hour}, + backdate: 5 * time.Minute, + want: x509.Certificate{ + NotBefore: now.Add(-5 * time.Minute), + NotAfter: now.Add(55 * time.Minute), + BasicConstraintsValid: true, + }, + }, + { + name: "truncate expiration", + policy: PermissiveSigningPolicy{TTL: 48 * time.Hour}, + want: x509.Certificate{ + NotBefore: now, + NotAfter: now.Add(24 * time.Hour), + BasicConstraintsValid: true, + }, + }, + } + + crKey, err := ecdsa.GenerateKey(elliptic.P224(), rand.Reader) + if err != nil { + t.Fatal(err) + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + ca := &CertificateAuthority{ + Certificate: caCert, + PrivateKey: caKey, + Now: func() time.Time { + return now + }, + Backdate: test.backdate, + } + + csr, err := x509.CreateCertificateRequest(rand.Reader, &test.cr, crKey) + if err != nil { + t.Fatal(err) + } + + certDER, err := ca.Sign(csr, test.policy) + if err != nil { + t.Fatal(err) + } + if test.wantErr { + if err == nil { + t.Fatal("expected error") + } + return + } + + cert, err := x509.ParseCertificate(certDER) + if err != nil { + t.Fatal(err) + } + + opts := cmp.Options{ + cmpopts.IgnoreFields(x509.Certificate{}, + "SignatureAlgorithm", + "PublicKeyAlgorithm", + "Version", + "MaxPathLen", + ), + diff.IgnoreUnset(), + cmp.Transformer("RoundTime", func(x time.Time) time.Time { + return x.Truncate(time.Second) + }), + } + if !cmp.Equal(*cert, test.want, opts) { + t.Errorf("unexpected diff: %v", cmp.Diff(*cert, test.want, opts)) + } + }) + } +} diff --git a/pkg/controller/certificates/authority/policies.go b/pkg/controller/certificates/authority/policies.go new file mode 100644 index 00000000000..d83f89ddea3 --- /dev/null +++ b/pkg/controller/certificates/authority/policies.go @@ -0,0 +1,142 @@ +/* +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 authority + +import ( + "crypto/x509" + "fmt" + "sort" + "time" + + capi "k8s.io/api/certificates/v1beta1" +) + +// SigningPolicy validates a CertificateRequest before it's signed by the +// CertificateAuthority. It may default or otherwise mutate a certificate +// template. +type SigningPolicy interface { + // not-exporting apply forces signing policy implementations to be internal + // to this package. + apply(template *x509.Certificate) error +} + +// PermissiveSigningPolicy is the signing policy historically used by the local +// signer. +// +// * It forwards all SANs from the original signing request. +// * It sets allowed usages as configured in the policy. +// * It sets NotAfter based on the TTL configured in the policy. +// * It zeros all extensions. +// * It sets BasicConstraints to true. +// * It sets IsCA to false. +type PermissiveSigningPolicy struct { + // TTL is the certificate TTL. It's used to calculate the NotAfter value of + // the certificate. + TTL time.Duration + // Usages are the allowed usages of a certficate. + Usages []capi.KeyUsage +} + +func (p PermissiveSigningPolicy) apply(tmpl *x509.Certificate) error { + usage, extUsages, err := keyUsagesFromStrings(p.Usages) + if err != nil { + return err + } + tmpl.KeyUsage = usage + tmpl.ExtKeyUsage = extUsages + tmpl.NotAfter = tmpl.NotBefore.Add(p.TTL) + + tmpl.ExtraExtensions = nil + tmpl.Extensions = nil + tmpl.BasicConstraintsValid = true + tmpl.IsCA = false + + return 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 unrecognized []capi.KeyUsage + extKeyUsages := make(map[x509.ExtKeyUsage]struct{}) + for _, usage := range usages { + if val, ok := keyUsageDict[usage]; ok { + keyUsage |= val + } else if val, ok := extKeyUsageDict[usage]; ok { + extKeyUsages[val] = struct{}{} + } else { + unrecognized = append(unrecognized, usage) + } + } + + var sorted sortedExtKeyUsage + for eku := range extKeyUsages { + sorted = append(sorted, eku) + } + sort.Sort(sorted) + + if len(unrecognized) > 0 { + return 0, nil, fmt.Errorf("unrecognized usage values: %q", unrecognized) + } + + return keyUsage, []x509.ExtKeyUsage(sorted), nil +} + +type sortedExtKeyUsage []x509.ExtKeyUsage + +func (s sortedExtKeyUsage) Len() int { + return len(s) +} + +func (s sortedExtKeyUsage) Swap(i, j int) { + s[i], s[j] = s[j], s[i] +} + +func (s sortedExtKeyUsage) Less(i, j int) bool { + return s[i] < s[j] +} diff --git a/pkg/controller/certificates/authority/policies_test.go b/pkg/controller/certificates/authority/policies_test.go new file mode 100644 index 00000000000..f7053dbfcdc --- /dev/null +++ b/pkg/controller/certificates/authority/policies_test.go @@ -0,0 +1,93 @@ +/* +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 authority + +import ( + "crypto/x509" + "fmt" + "reflect" + "testing" + + capi "k8s.io/api/certificates/v1beta1" +) + +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{"client auth", "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.ExtKeyUsageEmailProtection, x509.ExtKeyUsageOCSPSigning}, + 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) + } + }) + } +} diff --git a/pkg/controller/certificates/certificate_controller.go b/pkg/controller/certificates/certificate_controller.go index 962180fe130..f1d2f8ac08d 100644 --- a/pkg/controller/certificates/certificate_controller.go +++ b/pkg/controller/certificates/certificate_controller.go @@ -23,7 +23,6 @@ import ( "time" "golang.org/x/time/rate" - "k8s.io/klog" certificates "k8s.io/api/certificates/v1beta1" "k8s.io/apimachinery/pkg/api/errors" @@ -36,6 +35,7 @@ import ( "k8s.io/client-go/tools/cache" "k8s.io/client-go/tools/record" "k8s.io/client-go/util/workqueue" + "k8s.io/klog" "k8s.io/kubernetes/pkg/controller" ) diff --git a/pkg/controller/certificates/certificate_controller_utils_test.go b/pkg/controller/certificates/certificate_controller_utils_test.go index 920467ae6c7..d601db4db8e 100644 --- a/pkg/controller/certificates/certificate_controller_utils_test.go +++ b/pkg/controller/certificates/certificate_controller_utils_test.go @@ -17,10 +17,12 @@ limitations under the License. package certificates import ( - "github.com/stretchr/testify/assert" - "k8s.io/api/certificates/v1beta1" - "k8s.io/apimachinery/pkg/apis/meta/v1" "testing" + + "github.com/stretchr/testify/assert" + + "k8s.io/api/certificates/v1beta1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func TestIsCertificateRequestApproved(t *testing.T) { diff --git a/pkg/controller/certificates/rootcacertpublisher/publisher_test.go b/pkg/controller/certificates/rootcacertpublisher/publisher_test.go index c38a4be40de..ed8f16e3130 100644 --- a/pkg/controller/certificates/rootcacertpublisher/publisher_test.go +++ b/pkg/controller/certificates/rootcacertpublisher/publisher_test.go @@ -20,7 +20,7 @@ import ( "reflect" "testing" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/diff" "k8s.io/client-go/informers" diff --git a/pkg/controller/certificates/signer/BUILD b/pkg/controller/certificates/signer/BUILD index 96f4f691f22..2b8cb62aa45 100644 --- a/pkg/controller/certificates/signer/BUILD +++ b/pkg/controller/certificates/signer/BUILD @@ -17,7 +17,10 @@ go_test( embed = [":go_default_library"], deps = [ "//staging/src/k8s.io/api/certificates/v1beta1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/clock:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library", "//staging/src/k8s.io/client-go/util/cert:go_default_library", + "//vendor/github.com/google/go-cmp/cmp:go_default_library", ], ) @@ -28,6 +31,7 @@ go_library( deps = [ "//pkg/apis/certificates/v1beta1:go_default_library", "//pkg/controller/certificates:go_default_library", + "//pkg/controller/certificates/authority: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", diff --git a/pkg/controller/certificates/signer/signer.go b/pkg/controller/certificates/signer/signer.go index e76794deb0f..8b8f85b619c 100644 --- a/pkg/controller/certificates/signer/signer.go +++ b/pkg/controller/certificates/signer/signer.go @@ -18,12 +18,10 @@ limitations under the License. package signer import ( - "crypto/rand" - "crypto/x509" + "crypto" "encoding/pem" "fmt" "io/ioutil" - "math/big" "time" capi "k8s.io/api/certificates/v1beta1" @@ -33,15 +31,16 @@ import ( "k8s.io/client-go/util/keyutil" capihelper "k8s.io/kubernetes/pkg/apis/certificates/v1beta1" "k8s.io/kubernetes/pkg/controller/certificates" + "k8s.io/kubernetes/pkg/controller/certificates/authority" ) func NewCSRSigningController( client clientset.Interface, csrInformer certificatesinformers.CertificateSigningRequestInformer, caFile, caKeyFile string, - certificateDuration time.Duration, + certTTL time.Duration, ) (*certificates.CertificateController, error) { - signer, err := newSigner(caFile, caKeyFile, client, certificateDuration) + signer, err := newSigner(caFile, caKeyFile, client, certTTL) if err != nil { return nil, err } @@ -54,43 +53,46 @@ func NewCSRSigningController( } type signer struct { - ca *x509.Certificate - priv interface{} + ca *authority.CertificateAuthority 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) + certPEM, 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) + + certs, err := cert.ParseCertsPEM(certPEM) + if err != nil { + return nil, fmt.Errorf("error reading CA cert file %q: %v", caFile, err) + } + if len(certs) != 1 { + return nil, fmt.Errorf("error reading CA cert file %q: expected 1 certificate, found %d", caFile, len(certs)) + } + + keyPEM, 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) + key, err := keyutil.ParsePrivateKeyPEM(keyPEM) if err != nil { - return nil, fmt.Errorf("error parsing CA cert file %q: %v", caFile, err) + return nil, fmt.Errorf("error reading CA key file %q: %v", caKeyFile, err) } - if len(certs) != 1 { - return nil, fmt.Errorf("error parsing CA cert file %q: expected one certificate block", caFile) + priv, ok := key.(crypto.Signer) + if !ok { + return nil, fmt.Errorf("error reading CA key file %q: key did not implement crypto.Signer", caKeyFile) } - priv, err := keyutil.ParsePrivateKeyPEM(cakey) - if err != nil { - return nil, fmt.Errorf("malformed private key %v", err) - } return &signer{ - priv: priv, - ca: certs[0], + ca: &authority.CertificateAuthority{ + Certificate: certs[0], + PrivateKey: priv, + Backdate: 5 * time.Minute, + }, client: client, certTTL: certificateDuration, - now: time.Now, }, nil } @@ -115,101 +117,13 @@ func (s *signer) sign(csr *capi.CertificateSigningRequest) (*capi.CertificateSig return nil, fmt.Errorf("unable to parse csr %q: %v", csr.Name, err) } - usage, extUsages, err := keyUsagesFromStrings(csr.Spec.Usages) + der, err := s.ca.Sign(x509cr.Raw, authority.PermissiveSigningPolicy{ + TTL: s.certTTL, + Usages: 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 - + csr.Status.Certificate = pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: 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/signer_test.go b/pkg/controller/certificates/signer/signer_test.go index 5792c77c116..bd6c92535b9 100644 --- a/pkg/controller/certificates/signer/signer_test.go +++ b/pkg/controller/certificates/signer/signer_test.go @@ -1,5 +1,5 @@ /* -Copyright 2017 The Kubernetes Authors. +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. @@ -18,28 +18,28 @@ package signer import ( "crypto/x509" - "fmt" + "crypto/x509/pkix" "io/ioutil" - "reflect" - "strings" "testing" "time" + "github.com/google/go-cmp/cmp" + capi "k8s.io/api/certificates/v1beta1" + "k8s.io/apimachinery/pkg/util/clock" + "k8s.io/apimachinery/pkg/util/diff" "k8s.io/client-go/util/cert" ) func TestSigner(t *testing.T) { - testNow := time.Now() - testNowFn := func() time.Time { - return testNow - } + clock := clock.FakeClock{} 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.now = testNowFn + s.ca.Now = clock.Now + s.ca.Backdate = 0 csrb, err := ioutil.ReadFile("./testdata/kubelet.csr") if err != nil { @@ -75,183 +75,22 @@ func TestSigner(t *testing.T) { t.Fatalf("expected one certificate") } - crt := certs[0] - - if crt.Subject.CommonName != "system:node:k-a-node-s36b" { - t.Errorf("expected common name of 'system:node:k-a-node-s36b', but got: %v", certs[0].Subject.CommonName) - } - if !reflect.DeepEqual(crt.Subject.Organization, []string{"system:nodes"}) { - t.Errorf("expected organization to be [system:nodes] but got: %v", crt.Subject.Organization) - } - if crt.KeyUsage != x509.KeyUsageDigitalSignature|x509.KeyUsageKeyEncipherment { - t.Errorf("bad key usage") - } - if !reflect.DeepEqual(crt.ExtKeyUsage, []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth}) { - t.Errorf("bad extended key usage") + want := x509.Certificate{ + Version: 3, + Subject: pkix.Name{ + CommonName: "system:node:k-a-node-s36b", + Organization: []string{"system:nodes"}, + }, + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth}, + BasicConstraintsValid: true, + NotAfter: clock.Now().Add(1 * time.Hour), + PublicKeyAlgorithm: x509.ECDSA, + SignatureAlgorithm: x509.SHA256WithRSA, + MaxPathLen: -1, } - expectedTime := testNow.Add(1 * time.Hour) - // there is some jitter that we need to tolerate - diff := expectedTime.Sub(crt.NotAfter) - if diff > 10*time.Minute || diff < -10*time.Minute { - t.Fatal(crt.NotAfter) - } -} - -func TestSignerExpired(t *testing.T) { - hundredYearsFromNowFn := func() time.Time { - return time.Now().Add(24 * time.Hour * 365 * 100) - } - 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.now = hundredYearsFromNowFn - - csrb, err := ioutil.ReadFile("./testdata/kubelet.csr") - if err != nil { - t.Fatalf("failed to read CSR: %v", err) - } - - csr := &capi.CertificateSigningRequest{ - Spec: capi.CertificateSigningRequestSpec{ - Request: []byte(csrb), - Usages: []capi.KeyUsage{ - capi.UsageSigning, - capi.UsageKeyEncipherment, - capi.UsageServerAuth, - capi.UsageClientAuth, - }, - }, - } - - _, err = s.sign(csr) - if err == nil { - t.Fatal("missing error") - } - if !strings.HasPrefix(err.Error(), "the signer has expired") { - t.Fatal(err) - } -} - -func TestDurationLongerThanExpiry(t *testing.T) { - testNow := time.Now() - testNowFn := func() time.Time { - return testNow - } - - hundredYears := 24 * time.Hour * 365 * 100 - s, err := newSigner("./testdata/ca.crt", "./testdata/ca.key", nil, hundredYears) - if err != nil { - t.Fatalf("failed to create signer: %v", err) - } - s.now = testNowFn - - csrb, err := ioutil.ReadFile("./testdata/kubelet.csr") - if err != nil { - t.Fatalf("failed to read CSR: %v", err) - } - - csr := &capi.CertificateSigningRequest{ - Spec: capi.CertificateSigningRequestSpec{ - Request: []byte(csrb), - Usages: []capi.KeyUsage{ - capi.UsageSigning, - capi.UsageKeyEncipherment, - capi.UsageServerAuth, - capi.UsageClientAuth, - }, - }, - } - - _, err = s.sign(csr) - if err != nil { - t.Fatalf("failed to sign CSR: %v", err) - } - - // now we just need to verify that the expiry is based on the signing cert - certData := csr.Status.Certificate - if len(certData) == 0 { - t.Fatalf("expected a certificate after signing") - } - - certs, err := cert.ParseCertsPEM(certData) - if err != nil { - t.Fatalf("failed to parse certificate: %v", err) - } - if len(certs) != 1 { - t.Fatalf("expected one certificate") - } - - crt := certs[0] - expected, err := time.Parse("2006-01-02 15:04:05.999999999 -0700 MST", "2044-05-09 00:20:11 +0000 UTC") - if err != nil { - t.Fatal(err) - } - // there is some jitter that we need to tolerate - diff := expected.Sub(crt.NotAfter) - if diff > 10*time.Minute || diff < -10*time.Minute { - 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) - } - }) + if !cmp.Equal(*certs[0], want, diff.IgnoreUnset()) { + t.Errorf("unexpected diff: %v", cmp.Diff(certs[0], want, diff.IgnoreUnset())) } } diff --git a/staging/src/k8s.io/apimachinery/pkg/util/diff/diff.go b/staging/src/k8s.io/apimachinery/pkg/util/diff/diff.go index a006b925a9e..fa9ffa51b74 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/diff/diff.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/diff/diff.go @@ -19,6 +19,7 @@ package diff import ( "bytes" "fmt" + "reflect" "strings" "text/tabwriter" @@ -116,3 +117,41 @@ func ObjectGoPrintSideBySide(a, b interface{}) string { w.Flush() return buf.String() } + +// IgnoreUnset is an option that ignores fields that are unset on the right +// hand side of a comparison. This is useful in testing to assert that an +// object is a derivative. +func IgnoreUnset() cmp.Option { + return cmp.Options{ + // ignore unset fields in v2 + cmp.FilterPath(func(path cmp.Path) bool { + _, v2 := path.Last().Values() + switch v2.Kind() { + case reflect.Slice, reflect.Map: + if v2.IsNil() || v2.Len() == 0 { + return true + } + case reflect.String: + if v2.Len() == 0 { + return true + } + case reflect.Interface, reflect.Ptr: + if v2.IsNil() { + return true + } + } + return false + }, cmp.Ignore()), + // ignore map entries that aren't set in v2 + cmp.FilterPath(func(path cmp.Path) bool { + switch i := path.Last().(type) { + case cmp.MapIndex: + if _, v2 := i.Values(); !v2.IsValid() { + fmt.Println("E") + return true + } + } + return false + }, cmp.Ignore()), + } +}