refactor into seperate authority package

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",
     ],
 )
 

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
+		ca := &authority.CertificateAuthority{
-		for _, usage := range csr.Spec.Usages {
+			Certificate: s.serverCA,
-			usages = append(usages, string(usage))
+			PrivateKey:  s.serverPrivateKey,
-		}
-		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{
+		der, err := ca.Sign(cr.Raw, authority.PermissiveSigningPolicy{
-			Request: string(csr.Spec.Request),
+			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},
 		}

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",

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",
+    ],
+)

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
+}

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))
+			}
+		})
+	}
+}

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]
+}

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)
+			}
+		})
+	}
+}

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"
 )
 

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) {

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"

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",

pkg/controller/certificates/signer/signer.go

@@ -18,12 +18,10 @@ limitations under the License.
 package signer
 
 import (
-	"crypto/rand"
+	"crypto"
-	"crypto/x509"
 	"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
+	ca      *authority.CertificateAuthority
-	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)
+	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)
 	}
-
+	key, err := keyutil.ParsePrivateKeyPEM(keyPEM)
-	certs, err := cert.ParseCertsPEM(ca)
 	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 {
+	priv, ok := key.(crypto.Signer)
-		return nil, fmt.Errorf("error parsing CA cert file %q: expected one certificate block", caFile)
+	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: &authority.CertificateAuthority{
-		ca:      certs[0],
+			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
 	}
-
+	csr.Status.Certificate = pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der})
-	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
-}

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()
+	clock := clock.FakeClock{}
-	testNowFn := func() time.Time {
-		return testNow
-	}
 
 	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]
+	want := x509.Certificate{
-
+		Version: 3,
-	if crt.Subject.CommonName != "system:node:k-a-node-s36b" {
+		Subject: pkix.Name{
-		t.Errorf("expected common name of 'system:node:k-a-node-s36b', but got: %v", certs[0].Subject.CommonName)
+			CommonName:   "system:node:k-a-node-s36b",
-	}
+			Organization: []string{"system:nodes"},
-	if !reflect.DeepEqual(crt.Subject.Organization, []string{"system:nodes"}) {
+		},
-		t.Errorf("expected organization to be [system:nodes] but got: %v", crt.Subject.Organization)
+		KeyUsage:              x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment,
-	}
+		ExtKeyUsage:           []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth},
-	if crt.KeyUsage != x509.KeyUsageDigitalSignature|x509.KeyUsageKeyEncipherment {
+		BasicConstraintsValid: true,
-		t.Errorf("bad key usage")
+		NotAfter:              clock.Now().Add(1 * time.Hour),
-	}
+		PublicKeyAlgorithm:    x509.ECDSA,
-	if !reflect.DeepEqual(crt.ExtKeyUsage, []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth}) {
+		SignatureAlgorithm:    x509.SHA256WithRSA,
-		t.Errorf("bad extended key usage")
+		MaxPathLen:            -1,
 	}
 
-	expectedTime := testNow.Add(1 * time.Hour)
+	if !cmp.Equal(*certs[0], want, diff.IgnoreUnset()) {
-	// there is some jitter that we need to tolerate
+		t.Errorf("unexpected diff: %v", cmp.Diff(certs[0], want, diff.IgnoreUnset()))
-	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)
-			}
-		})
 	}
 }

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()),
+	}
+}