Merge pull request kubernetes#99494 from enj/enj/i/not_after_ttl_hint

csr: add expirationSeconds field to control cert lifetime
ii · Jul 2, 2021 · 659c7e7 · 659c7e7
2 parents 2627808 + 8d49502
commit 659c7e7
Show file tree

Hide file tree

Showing 46 changed files with 1,769 additions and 251 deletions.
diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json
diff --git a/cmd/kube-controller-manager/app/options/csrsigningcontroller.go b/cmd/kube-controller-manager/app/options/csrsigningcontroller.go
@@ -45,8 +45,8 @@ func (o *CSRSigningControllerOptions) AddFlags(fs *pflag.FlagSet) {
 	fs.StringVar(&o.KubeAPIServerClientSignerConfiguration.KeyFile, "cluster-signing-kube-apiserver-client-key-file", o.KubeAPIServerClientSignerConfiguration.KeyFile, "Filename containing a PEM-encoded RSA or ECDSA private key used to sign certificates for the kubernetes.io/kube-apiserver-client signer.  If specified, --cluster-signing-{cert,key}-file must not be set.")
 	fs.StringVar(&o.LegacyUnknownSignerConfiguration.CertFile, "cluster-signing-legacy-unknown-cert-file", o.LegacyUnknownSignerConfiguration.CertFile, "Filename containing a PEM-encoded X509 CA certificate used to issue certificates for the kubernetes.io/legacy-unknown signer.  If specified, --cluster-signing-{cert,key}-file must not be set.")
 	fs.StringVar(&o.LegacyUnknownSignerConfiguration.KeyFile, "cluster-signing-legacy-unknown-key-file", o.LegacyUnknownSignerConfiguration.KeyFile, "Filename containing a PEM-encoded RSA or ECDSA private key used to sign certificates for the kubernetes.io/legacy-unknown signer.  If specified, --cluster-signing-{cert,key}-file must not be set.")
-	fs.DurationVar(&o.ClusterSigningDuration.Duration, "cluster-signing-duration", o.ClusterSigningDuration.Duration, "The length of duration signed certificates will be given.")
-	fs.DurationVar(&o.ClusterSigningDuration.Duration, "experimental-cluster-signing-duration", o.ClusterSigningDuration.Duration, "The length of duration signed certificates will be given.")
+	fs.DurationVar(&o.ClusterSigningDuration.Duration, "cluster-signing-duration", o.ClusterSigningDuration.Duration, "The max length of duration signed certificates will be given.  Individual CSRs may request shorter certs by setting spec.expirationSeconds.")
+	fs.DurationVar(&o.ClusterSigningDuration.Duration, "experimental-cluster-signing-duration", o.ClusterSigningDuration.Duration, "The max length of duration signed certificates will be given.  Individual CSRs may request shorter certs by setting spec.expirationSeconds.")
 	fs.MarkDeprecated("experimental-cluster-signing-duration", "use --cluster-signing-duration")
 }
 

diff --git a/pkg/apis/certificates/fuzzer/fuzzer.go b/pkg/apis/certificates/fuzzer/fuzzer.go
@@ -17,9 +17,12 @@ limitations under the License.
 package fuzzer
 
 import (
+	"time"
+
 	fuzz "github.com/google/gofuzz"
 
 	runtimeserializer "k8s.io/apimachinery/pkg/runtime/serializer"
+	"k8s.io/client-go/util/certificate/csr"
 	"k8s.io/kubernetes/pkg/apis/certificates"
 	api "k8s.io/kubernetes/pkg/apis/core"
 )
@@ -31,6 +34,7 @@ var Funcs = func(codecs runtimeserializer.CodecFactory) []interface{} {
 			c.FuzzNoCustom(obj) // fuzz self without calling this function again
 			obj.Usages = []certificates.KeyUsage{certificates.UsageKeyEncipherment}
 			obj.SignerName = "example.com/custom-sample-signer"
+			obj.ExpirationSeconds = csr.DurationToExpirationSeconds(time.Hour + time.Minute + time.Second)
 		},
 		func(obj *certificates.CertificateSigningRequestCondition, c fuzz.Continue) {
 			c.FuzzNoCustom(obj) // fuzz self without calling this function again

diff --git a/pkg/apis/certificates/types.go b/pkg/apis/certificates/types.go
@@ -68,6 +68,30 @@ type CertificateSigningRequestSpec struct {
 	//  6. Whether or not requests for CA certificates are allowed.
 	SignerName string
 
+	// expirationSeconds is the requested duration of validity of the issued
+	// certificate. The certificate signer may issue a certificate with a different
+	// validity duration so a client must check the delta between the notBefore and
+	// and notAfter fields in the issued certificate to determine the actual duration.
+	//
+	// The v1.22+ in-tree implementations of the well-known Kubernetes signers will
+	// honor this field as long as the requested duration is not greater than the
+	// maximum duration they will honor per the --cluster-signing-duration CLI
+	// flag to the Kubernetes controller manager.
+	//
+	// Certificate signers may not honor this field for various reasons:
+	//
+	//   1. Old signer that is unaware of the field (such as the in-tree
+	//      implementations prior to v1.22)
+	//   2. Signer whose configured maximum is shorter than the requested duration
+	//   3. Signer whose configured minimum is longer than the requested duration
+	//
+	// The minimum valid value for expirationSeconds is 600, i.e. 10 minutes.
+	//
+	// As of v1.22, this field is beta and is controlled via the CSRDuration feature gate.
+	//
+	// +optional
+	ExpirationSeconds *int32
+
 	// usages specifies a set of usage contexts the key will be
 	// valid for.
 	// See: https://tools.ietf.org/html/rfc5280#section-4.2.1.3

diff --git a/pkg/apis/certificates/v1/zz_generated.conversion.go b/pkg/apis/certificates/v1/zz_generated.conversion.go
diff --git a/pkg/apis/certificates/v1beta1/zz_generated.conversion.go b/pkg/apis/certificates/v1beta1/zz_generated.conversion.go
diff --git a/pkg/apis/certificates/validation/validation.go b/pkg/apis/certificates/validation/validation.go
@@ -205,6 +205,9 @@ func validateCertificateSigningRequest(csr *certificates.CertificateSigningReque
 	} else {
 		allErrs = append(allErrs, ValidateCertificateSigningRequestSignerName(specPath.Child("signerName"), csr.Spec.SignerName)...)
 	}
+	if csr.Spec.ExpirationSeconds != nil && *csr.Spec.ExpirationSeconds < 600 {
+		allErrs = append(allErrs, field.Invalid(specPath.Child("expirationSeconds"), *csr.Spec.ExpirationSeconds, "may not specify a duration less than 600 seconds (10 minutes)"))
+	}
 	allErrs = append(allErrs, validateConditions(field.NewPath("status", "conditions"), csr, opts)...)
 
 	if !opts.allowArbitraryCertificate {

diff --git a/pkg/apis/certificates/validation/validation_test.go b/pkg/apis/certificates/validation/validation_test.go
@@ -26,14 +26,17 @@ import (
 	"reflect"
 	"strings"
 	"testing"
+	"time"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/validation/field"
+	"k8s.io/client-go/util/certificate/csr"
 	capi "k8s.io/kubernetes/pkg/apis/certificates"
 	capiv1beta1 "k8s.io/kubernetes/pkg/apis/certificates/v1beta1"
 	"k8s.io/kubernetes/pkg/apis/core"
+	"k8s.io/utils/pointer"
 )
 
 var (
@@ -262,6 +265,74 @@ func TestValidateCertificateSigningRequestCreate(t *testing.T) {
 			},
 			errs: field.ErrorList{},
 		},
+		"negative duration": {
+			csr: capi.CertificateSigningRequest{
+				ObjectMeta: validObjectMeta,
+				Spec: capi.CertificateSigningRequestSpec{
+					Usages:            validUsages,
+					Request:           newCSRPEM(t),
+					SignerName:        validSignerName,
+					ExpirationSeconds: pointer.Int32(-1),
+				},
+			},
+			errs: field.ErrorList{
+				field.Invalid(specPath.Child("expirationSeconds"), int32(-1), "may not specify a duration less than 600 seconds (10 minutes)"),
+			},
+		},
+		"zero duration": {
+			csr: capi.CertificateSigningRequest{
+				ObjectMeta: validObjectMeta,
+				Spec: capi.CertificateSigningRequestSpec{
+					Usages:            validUsages,
+					Request:           newCSRPEM(t),
+					SignerName:        validSignerName,
+					ExpirationSeconds: pointer.Int32(0),
+				},
+			},
+			errs: field.ErrorList{
+				field.Invalid(specPath.Child("expirationSeconds"), int32(0), "may not specify a duration less than 600 seconds (10 minutes)"),
+			},
+		},
+		"one duration": {
+			csr: capi.CertificateSigningRequest{
+				ObjectMeta: validObjectMeta,
+				Spec: capi.CertificateSigningRequestSpec{
+					Usages:            validUsages,
+					Request:           newCSRPEM(t),
+					SignerName:        validSignerName,
+					ExpirationSeconds: pointer.Int32(1),
+				},
+			},
+			errs: field.ErrorList{
+				field.Invalid(specPath.Child("expirationSeconds"), int32(1), "may not specify a duration less than 600 seconds (10 minutes)"),
+			},
+		},
+		"too short duration": {
+			csr: capi.CertificateSigningRequest{
+				ObjectMeta: validObjectMeta,
+				Spec: capi.CertificateSigningRequestSpec{
+					Usages:            validUsages,
+					Request:           newCSRPEM(t),
+					SignerName:        validSignerName,
+					ExpirationSeconds: csr.DurationToExpirationSeconds(time.Minute),
+				},
+			},
+			errs: field.ErrorList{
+				field.Invalid(specPath.Child("expirationSeconds"), *csr.DurationToExpirationSeconds(time.Minute), "may not specify a duration less than 600 seconds (10 minutes)"),
+			},
+		},
+		"valid duration": {
+			csr: capi.CertificateSigningRequest{
+				ObjectMeta: validObjectMeta,
+				Spec: capi.CertificateSigningRequestSpec{
+					Usages:            validUsages,
+					Request:           newCSRPEM(t),
+					SignerName:        validSignerName,
+					ExpirationSeconds: csr.DurationToExpirationSeconds(10 * time.Minute),
+				},
+			},
+			errs: field.ErrorList{},
+		},
 		"missing usages": {
 			csr: capi.CertificateSigningRequest{
 				ObjectMeta: validObjectMeta,

diff --git a/pkg/apis/certificates/zz_generated.deepcopy.go b/pkg/apis/certificates/zz_generated.deepcopy.go
diff --git a/pkg/controller/certificates/signer/config/types.go b/pkg/controller/certificates/signer/config/types.go
@@ -38,8 +38,8 @@ type CSRSigningControllerConfiguration struct {
 	// legacyUnknownSignerConfiguration holds the certificate and key used to issue certificates for the kubernetes.io/legacy-unknown
 	LegacyUnknownSignerConfiguration CSRSigningConfiguration
 
-	// clusterSigningDuration is the length of duration signed certificates
-	// will be given.
+	// clusterSigningDuration is the max length of duration signed certificates will be given.
+	// Individual CSRs may request shorter certs by setting spec.expirationSeconds.
 	ClusterSigningDuration metav1.Duration
 }
 

diff --git a/pkg/controller/certificates/signer/signer.go b/pkg/controller/certificates/signer/signer.go
@@ -30,11 +30,14 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apiserver/pkg/server/dynamiccertificates"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	certificatesinformers "k8s.io/client-go/informers/certificates/v1"
 	clientset "k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/util/certificate/csr"
 	capihelper "k8s.io/kubernetes/pkg/apis/certificates"
 	"k8s.io/kubernetes/pkg/controller/certificates"
 	"k8s.io/kubernetes/pkg/controller/certificates/authority"
+	"k8s.io/kubernetes/pkg/features"
 )
 
 type CSRSigningController struct {
@@ -115,7 +118,7 @@ type signer struct {
 	caProvider *caProvider
 
 	client  clientset.Interface
-	certTTL time.Duration
+	certTTL time.Duration // max TTL; individual requests may request shorter certs by setting spec.expirationSeconds
 
 	signerName           string
 	isRequestForSignerFn isRequestForSignerFunc
@@ -173,7 +176,7 @@ func (s *signer) handle(csr *capi.CertificateSigningRequest) error {
 		// Ignore requests for kubernetes.io signerNames we don't recognize
 		return nil
 	}
-	cert, err := s.sign(x509cr, csr.Spec.Usages, nil)
+	cert, err := s.sign(x509cr, csr.Spec.Usages, csr.Spec.ExpirationSeconds, nil)
 	if err != nil {
 		return fmt.Errorf("error auto signing csr: %v", err)
 	}
@@ -185,15 +188,15 @@ func (s *signer) handle(csr *capi.CertificateSigningRequest) error {
 	return nil
 }
 
-func (s *signer) sign(x509cr *x509.CertificateRequest, usages []capi.KeyUsage, now func() time.Time) ([]byte, error) {
+func (s *signer) sign(x509cr *x509.CertificateRequest, usages []capi.KeyUsage, expirationSeconds *int32, now func() time.Time) ([]byte, error) {
 	currCA, err := s.caProvider.currentCA()
 	if err != nil {
 		return nil, err
 	}
 	der, err := currCA.Sign(x509cr.Raw, authority.PermissiveSigningPolicy{
-		TTL:      s.certTTL,
+		TTL:      s.duration(expirationSeconds),
 		Usages:   usages,
-		Backdate: 5 * time.Minute, // this must always be less than the minimum TTL requested by a user
+		Backdate: 5 * time.Minute, // this must always be less than the minimum TTL requested by a user (see sanity check requestedDuration below)
 		Short:    8 * time.Hour,   // 5 minutes of backdating is roughly 1% of 8 hours
 		Now:      now,
 	})
@@ -203,6 +206,30 @@ func (s *signer) sign(x509cr *x509.CertificateRequest, usages []capi.KeyUsage, n
 	return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}), nil
 }
 
+func (s *signer) duration(expirationSeconds *int32) time.Duration {
+	if !utilfeature.DefaultFeatureGate.Enabled(features.CSRDuration) {
+		return s.certTTL
+	}
+
+	if expirationSeconds == nil {
+		return s.certTTL
+	}
+
+	// honor requested duration is if it is less than the default TTL
+	// use 10 min (2x hard coded backdate above) as a sanity check lower bound
+	const min = 10 * time.Minute
+	switch requestedDuration := csr.ExpirationSecondsToDuration(*expirationSeconds); {
+	case requestedDuration > s.certTTL:
+		return s.certTTL
+
+	case requestedDuration < min:
+		return min
+
+	default:
+		return requestedDuration
+	}
+}
+
 // getCSRVerificationFuncForSignerName is a function that provides reliable mapping of signer names to verification so that
 // we don't have accidents with wiring at some later date.
 func getCSRVerificationFuncForSignerName(signerName string) (isRequestForSignerFunc, error) {