diff --git a/cmd/kube-controller-manager/app/options/csrsigningcontroller.go b/cmd/kube-controller-manager/app/options/csrsigningcontroller.go index ebb49886803..35179a42023 100644 --- 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 index aea3a179de6..c9711eb7f37 100644 --- 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 index a2a15a68649..7a7b71f21ef 100644 --- 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/validation/validation.go b/pkg/apis/certificates/validation/validation.go index f182027e439..b019ac57cfd 100644 --- 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 index dd1e055901e..477891914c1 100644 --- 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/controller/certificates/signer/config/types.go b/pkg/controller/certificates/signer/config/types.go index edf60b17dca..dec003843cd 100644 --- 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 index 604765515e0..d4799a0fdd4 100644 --- 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) { diff --git a/pkg/controller/certificates/signer/signer_test.go b/pkg/controller/certificates/signer/signer_test.go index 215838d7dba..caa42841f77 100644 --- a/pkg/controller/certificates/signer/signer_test.go +++ b/pkg/controller/certificates/signer/signer_test.go @@ -32,11 +32,15 @@ import ( capi "k8s.io/api/certificates/v1" "k8s.io/apimachinery/pkg/util/clock" "k8s.io/apimachinery/pkg/util/diff" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/kubernetes/fake" testclient "k8s.io/client-go/testing" "k8s.io/client-go/util/cert" + "k8s.io/client-go/util/certificate/csr" + featuregatetesting "k8s.io/component-base/featuregate/testing" capihelper "k8s.io/kubernetes/pkg/apis/certificates/v1" "k8s.io/kubernetes/pkg/controller/certificates" + "k8s.io/kubernetes/pkg/features" ) func TestSigner(t *testing.T) { @@ -61,7 +65,11 @@ func TestSigner(t *testing.T) { capi.UsageKeyEncipherment, capi.UsageServerAuth, capi.UsageClientAuth, - }, fakeClock.Now) + }, + // requesting a duration that is greater than TTL is ignored + csr.DurationToExpirationSeconds(3*time.Hour), + fakeClock.Now, + ) if err != nil { t.Fatalf("failed to sign CSR: %v", err) } @@ -344,3 +352,91 @@ func makeTestCSR(b csrBuilder) *capi.CertificateSigningRequest { } return csr } + +func Test_signer_duration(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + certTTL time.Duration + expirationSeconds *int32 + wantGateEnabled time.Duration + wantGateDisabled time.Duration + }{ + { + name: "can request shorter duration than TTL", + certTTL: time.Hour, + expirationSeconds: csr.DurationToExpirationSeconds(30 * time.Minute), + wantGateEnabled: 30 * time.Minute, + wantGateDisabled: time.Hour, + }, + { + name: "cannot request longer duration than TTL", + certTTL: time.Hour, + expirationSeconds: csr.DurationToExpirationSeconds(3 * time.Hour), + wantGateEnabled: time.Hour, + wantGateDisabled: time.Hour, + }, + { + name: "cannot request negative duration", + certTTL: time.Hour, + expirationSeconds: csr.DurationToExpirationSeconds(-time.Minute), + wantGateEnabled: 10 * time.Minute, + wantGateDisabled: time.Hour, + }, + { + name: "cannot request duration less than 10 mins", + certTTL: time.Hour, + expirationSeconds: csr.DurationToExpirationSeconds(10*time.Minute - time.Second), + wantGateEnabled: 10 * time.Minute, + wantGateDisabled: time.Hour, + }, + { + name: "can request duration of exactly 10 mins", + certTTL: time.Hour, + expirationSeconds: csr.DurationToExpirationSeconds(10 * time.Minute), + wantGateEnabled: 10 * time.Minute, + wantGateDisabled: time.Hour, + }, + { + name: "can request duration equal to the default", + certTTL: time.Hour, + expirationSeconds: csr.DurationToExpirationSeconds(time.Hour), + wantGateEnabled: time.Hour, + wantGateDisabled: time.Hour, + }, + { + name: "can choose not to request a duration to get the default", + certTTL: time.Hour, + expirationSeconds: nil, + wantGateEnabled: time.Hour, + wantGateDisabled: time.Hour, + }, + } + for _, tt := range tests { + tt := tt + + f := func(t *testing.T, want time.Duration) { + s := &signer{ + certTTL: tt.certTTL, + } + if got := s.duration(tt.expirationSeconds); got != want { + t.Errorf("duration() = %v, want %v", got, want) + } + } + + // regular tests + t.Run(tt.name, func(t *testing.T) { + t.Parallel() // these are safe to run in parallel but not the feature gate disabled tests + + f(t, tt.wantGateEnabled) + }) + + // same tests with the feature gate disabled + t.Run("feature gate disabled - "+tt.name, func(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSRDuration, false)() + f(t, tt.wantGateDisabled) + }) + + } +} diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 28a927ee9b5..86f59e6fa4b 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -740,6 +740,12 @@ const ( // // Enables usage of the ReadWriteOncePod PersistentVolume access mode. ReadWriteOncePod featuregate.Feature = "ReadWriteOncePod" + + // owner: @enj + // beta: v1.22 + // + // Allows clients to request a duration for certificates issued via the Kubernetes CSR API. + CSRDuration featuregate.Feature = "CSRDuration" ) func init() { @@ -851,6 +857,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS SeccompDefault: {Default: false, PreRelease: featuregate.Alpha}, PodSecurity: {Default: false, PreRelease: featuregate.Alpha}, ReadWriteOncePod: {Default: false, PreRelease: featuregate.Alpha}, + CSRDuration: {Default: true, PreRelease: featuregate.Beta}, // inherited features from generic apiserver, relisted here to get a conflict if it is changed // unintentionally on either side: diff --git a/pkg/kubelet/certificate/bootstrap/bootstrap.go b/pkg/kubelet/certificate/bootstrap/bootstrap.go index 90dd28c36e8..0db0f1dc3f4 100644 --- a/pkg/kubelet/certificate/bootstrap/bootstrap.go +++ b/pkg/kubelet/certificate/bootstrap/bootstrap.go @@ -344,7 +344,7 @@ func requestNodeCertificate(ctx context.Context, client clientset.Interface, pri return nil, err } - reqName, reqUID, err := csr.RequestCertificate(client, csrData, name, certificatesv1.KubeAPIServerClientKubeletSignerName, usages, privateKey) + reqName, reqUID, err := csr.RequestCertificate(client, csrData, name, certificatesv1.KubeAPIServerClientKubeletSignerName, nil, usages, privateKey) if err != nil { return nil, err } diff --git a/pkg/printers/internalversion/printers.go b/pkg/printers/internalversion/printers.go index df9a140c4b5..7d7693c8a99 100644 --- a/pkg/printers/internalversion/printers.go +++ b/pkg/printers/internalversion/printers.go @@ -48,6 +48,7 @@ import ( "k8s.io/apimachinery/pkg/util/duration" "k8s.io/apimachinery/pkg/util/sets" utilfeature "k8s.io/apiserver/pkg/util/feature" + "k8s.io/client-go/util/certificate/csr" "k8s.io/kubernetes/pkg/apis/admissionregistration" "k8s.io/kubernetes/pkg/apis/apiserverinternal" "k8s.io/kubernetes/pkg/apis/apps" @@ -413,6 +414,7 @@ func AddHandlers(h printers.PrintHandler) { {Name: "Age", Type: "string", Description: metav1.ObjectMeta{}.SwaggerDoc()["creationTimestamp"]}, {Name: "SignerName", Type: "string", Description: certificatesv1beta1.CertificateSigningRequestSpec{}.SwaggerDoc()["signerName"]}, {Name: "Requestor", Type: "string", Description: certificatesv1beta1.CertificateSigningRequestSpec{}.SwaggerDoc()["request"]}, + {Name: "RequestedDuration", Type: "string", Description: certificatesv1beta1.CertificateSigningRequestSpec{}.SwaggerDoc()["expirationSeconds"]}, {Name: "Condition", Type: "string", Description: certificatesv1beta1.CertificateSigningRequestStatus{}.SwaggerDoc()["conditions"]}, } h.TableHandler(certificateSigningRequestColumnDefinitions, printCertificateSigningRequest) @@ -1902,7 +1904,11 @@ func printCertificateSigningRequest(obj *certificates.CertificateSigningRequest, if obj.Spec.SignerName != "" { signerName = obj.Spec.SignerName } - row.Cells = append(row.Cells, obj.Name, translateTimestampSince(obj.CreationTimestamp), signerName, obj.Spec.Username, status) + requestedDuration := "" + if obj.Spec.ExpirationSeconds != nil { + requestedDuration = duration.HumanDuration(csr.ExpirationSecondsToDuration(*obj.Spec.ExpirationSeconds)) + } + row.Cells = append(row.Cells, obj.Name, translateTimestampSince(obj.CreationTimestamp), signerName, obj.Spec.Username, requestedDuration, status) return []metav1.TableRow{row}, nil } diff --git a/pkg/printers/internalversion/printers_test.go b/pkg/printers/internalversion/printers_test.go index d48d045fe66..4c19d4c20c2 100644 --- a/pkg/printers/internalversion/printers_test.go +++ b/pkg/printers/internalversion/printers_test.go @@ -27,6 +27,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/client-go/util/certificate/csr" "k8s.io/kubernetes/pkg/apis/apiserverinternal" "k8s.io/kubernetes/pkg/apis/apps" "k8s.io/kubernetes/pkg/apis/autoscaling" @@ -3990,8 +3991,8 @@ func TestPrintCertificateSigningRequest(t *testing.T) { Spec: certificates.CertificateSigningRequestSpec{}, Status: certificates.CertificateSigningRequestStatus{}, }, - // Columns: Name, Age, Requestor, Condition - expected: []metav1.TableRow{{Cells: []interface{}{"csr1", "0s", "", "", "Pending"}}}, + // Columns: Name, Age, SignerName, Requestor, RequestedDuration, Condition + expected: []metav1.TableRow{{Cells: []interface{}{"csr1", "0s", "", "", "", "Pending"}}}, }, // Basic CSR with Spec and Status=Approved. { @@ -4011,8 +4012,8 @@ func TestPrintCertificateSigningRequest(t *testing.T) { }, }, }, - // Columns: Name, Age, Requestor, Condition - expected: []metav1.TableRow{{Cells: []interface{}{"csr2", "0s", "", "CSR Requestor", "Approved"}}}, + // Columns: Name, Age, SignerName, Requestor, RequestedDuration, Condition + expected: []metav1.TableRow{{Cells: []interface{}{"csr2", "0s", "", "CSR Requestor", "", "Approved"}}}, }, // Basic CSR with Spec and SignerName set { @@ -4033,8 +4034,31 @@ func TestPrintCertificateSigningRequest(t *testing.T) { }, }, }, - // Columns: Name, Age, Requestor, Condition - expected: []metav1.TableRow{{Cells: []interface{}{"csr2", "0s", "example.com/test-signer", "CSR Requestor", "Approved"}}}, + // Columns: Name, Age, SignerName, Requestor, RequestedDuration, Condition + expected: []metav1.TableRow{{Cells: []interface{}{"csr2", "0s", "example.com/test-signer", "CSR Requestor", "", "Approved"}}}, + }, + // Basic CSR with Spec, SignerName and ExpirationSeconds set + { + csr: certificates.CertificateSigningRequest{ + ObjectMeta: metav1.ObjectMeta{ + Name: "csr2", + CreationTimestamp: metav1.Time{Time: time.Now().Add(1.9e9)}, + }, + Spec: certificates.CertificateSigningRequestSpec{ + Username: "CSR Requestor", + SignerName: "example.com/test-signer", + ExpirationSeconds: csr.DurationToExpirationSeconds(7*24*time.Hour + time.Hour), // a little bit more than a week + }, + Status: certificates.CertificateSigningRequestStatus{ + Conditions: []certificates.CertificateSigningRequestCondition{ + { + Type: certificates.CertificateApproved, + }, + }, + }, + }, + // Columns: Name, Age, SignerName, Requestor, RequestedDuration, Condition + expected: []metav1.TableRow{{Cells: []interface{}{"csr2", "0s", "example.com/test-signer", "CSR Requestor", "7d1h", "Approved"}}}, }, // Basic CSR with Spec and Status=Approved; certificate issued. { @@ -4055,8 +4079,8 @@ func TestPrintCertificateSigningRequest(t *testing.T) { Certificate: []byte("cert data"), }, }, - // Columns: Name, Age, Requestor, Condition - expected: []metav1.TableRow{{Cells: []interface{}{"csr2", "0s", "", "CSR Requestor", "Approved,Issued"}}}, + // Columns: Name, Age, SignerName, Requestor, RequestedDuration, Condition + expected: []metav1.TableRow{{Cells: []interface{}{"csr2", "0s", "", "CSR Requestor", "", "Approved,Issued"}}}, }, // Basic CSR with Spec and Status=Denied. { @@ -4076,8 +4100,8 @@ func TestPrintCertificateSigningRequest(t *testing.T) { }, }, }, - // Columns: Name, Age, Requestor, Condition - expected: []metav1.TableRow{{Cells: []interface{}{"csr3", "0s", "", "CSR Requestor", "Denied"}}}, + // Columns: Name, Age, SignerName, Requestor, RequestedDuration, Condition + expected: []metav1.TableRow{{Cells: []interface{}{"csr3", "0s", "", "CSR Requestor", "", "Denied"}}}, }, } diff --git a/pkg/registry/certificates/certificates/storage/metrics.go b/pkg/registry/certificates/certificates/storage/metrics.go new file mode 100644 index 00000000000..26ede25b839 --- /dev/null +++ b/pkg/registry/certificates/certificates/storage/metrics.go @@ -0,0 +1,169 @@ +/* +Copyright 2021 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 storage + +import ( + "context" + "fmt" + "strings" + "sync" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + genericregistry "k8s.io/apiserver/pkg/registry/generic/registry" + "k8s.io/apiserver/pkg/util/dryrun" + utilfeature "k8s.io/apiserver/pkg/util/feature" + "k8s.io/client-go/util/cert" + "k8s.io/client-go/util/certificate/csr" + "k8s.io/component-base/metrics" + "k8s.io/component-base/metrics/legacyregistry" + "k8s.io/kubernetes/pkg/apis/certificates" + "k8s.io/kubernetes/pkg/features" +) + +const ( + namespace = "apiserver" + subsystem = "certificates_registry" +) + +var ( + // csrDurationRequested counts and categorizes how many certificates were issued when the client requested a duration. + csrDurationRequested = metrics.NewCounterVec( + &metrics.CounterOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "csr_requested_duration_total", + Help: "Total number of issued CSRs with a requested duration, sliced by signer (only kubernetes.io signer names are specifically identified)", + StabilityLevel: metrics.ALPHA, + }, + []string{"signerName"}, + ) + + // csrDurationHonored counts and categorizes how many certificates were issued when the client requested a duration and the signer honored it. + csrDurationHonored = metrics.NewCounterVec( + &metrics.CounterOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "csr_honored_duration_total", + Help: "Total number of issued CSRs with a requested duration that was honored, sliced by signer (only kubernetes.io signer names are specifically identified)", + StabilityLevel: metrics.ALPHA, + }, + []string{"signerName"}, + ) +) + +func init() { + registerMetricsOnce.Do(func() { + legacyregistry.MustRegister(csrDurationRequested) + legacyregistry.MustRegister(csrDurationHonored) + }) +} + +var registerMetricsOnce sync.Once + +type counterVecMetric interface { + WithLabelValues(...string) metrics.CounterMetric +} + +func countCSRDurationMetric(requested, honored counterVecMetric) genericregistry.BeginUpdateFunc { + return func(ctx context.Context, obj, old runtime.Object, options *metav1.UpdateOptions) (genericregistry.FinishFunc, error) { + return func(ctx context.Context, success bool) { + if !success { + return // ignore failures + } + + if dryrun.IsDryRun(options.DryRun) { + return // ignore things that would not get persisted + } + + if !utilfeature.DefaultFeatureGate.Enabled(features.CSRDuration) { + return + } + + oldCSR, ok := old.(*certificates.CertificateSigningRequest) + if !ok { + return + } + + // if the old CSR already has a certificate, do not double count it + if len(oldCSR.Status.Certificate) > 0 { + return + } + + if oldCSR.Spec.ExpirationSeconds == nil { + return // ignore CSRs that are not using the CSR duration feature + } + + newCSR, ok := obj.(*certificates.CertificateSigningRequest) + if !ok { + return + } + issuedCert := newCSR.Status.Certificate + + // new CSR has no issued certificate yet so do not count it. + // note that this means that we will ignore CSRs that set a duration + // but never get approved/signed. this is fine because the point + // of these metrics is to understand if the duration is honored + // by the signer. we are not checking the behavior of the approver. + if len(issuedCert) == 0 { + return + } + + signer := compressSignerName(oldCSR.Spec.SignerName) + + // at this point we know that this CSR is going to be persisted and + // the cert was just issued and the client requested a duration + requested.WithLabelValues(signer).Inc() + + certs, err := cert.ParseCertsPEM(issuedCert) + if err != nil { + utilruntime.HandleError(fmt.Errorf("metrics recording failed to parse certificate for CSR %s: %w", oldCSR.Name, err)) + return + } + + // now we check to see if the signer honored the requested duration + certificate := certs[0] + wantDuration := csr.ExpirationSecondsToDuration(*oldCSR.Spec.ExpirationSeconds) + actualDuration := certificate.NotAfter.Sub(certificate.NotBefore) + if isDurationHonored(wantDuration, actualDuration) { + honored.WithLabelValues(signer).Inc() + } + }, nil + } +} + +func isDurationHonored(want, got time.Duration) bool { + delta := want - got + if delta < 0 { + delta = -delta + } + + // short-lived cert backdating + 5% of want + maxDelta := 5*time.Minute + (want / 20) + + return delta < maxDelta +} + +func compressSignerName(name string) string { + if strings.HasPrefix(name, "kubernetes.io/") { + return name + } + + return "other" +} diff --git a/pkg/registry/certificates/certificates/storage/metrics_test.go b/pkg/registry/certificates/certificates/storage/metrics_test.go new file mode 100644 index 00000000000..57d6b87f220 --- /dev/null +++ b/pkg/registry/certificates/certificates/storage/metrics_test.go @@ -0,0 +1,496 @@ +/* +Copyright 2021 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 storage + +import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "math/big" + "testing" + "time" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + utilfeature "k8s.io/apiserver/pkg/util/feature" + certutil "k8s.io/client-go/util/cert" + "k8s.io/client-go/util/certificate/csr" + featuregatetesting "k8s.io/component-base/featuregate/testing" + "k8s.io/component-base/metrics" + "k8s.io/kubernetes/pkg/apis/certificates" + "k8s.io/kubernetes/pkg/features" + "k8s.io/utils/pointer" +) + +func Test_countCSRDurationMetric(t *testing.T) { + caPrivateKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + caCert, err := certutil.NewSelfSignedCACert(certutil.Config{CommonName: "test-ca"}, caPrivateKey) + if err != nil { + t.Fatal(err) + } + + tests := []struct { + name string + disableFeatureGate bool + success bool + obj, old runtime.Object + options *metav1.UpdateOptions + wantSigner string + wantRequested, wantHonored bool + }{ + { + name: "cert parse failure", + success: true, + obj: &certificates.CertificateSigningRequest{ + Status: certificates.CertificateSigningRequestStatus{ + Certificate: []byte("junk"), + }, + }, + old: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + SignerName: "fancy", + ExpirationSeconds: pointer.Int32(77), + }, + }, + options: &metav1.UpdateOptions{}, + wantSigner: "other", + wantRequested: true, + wantHonored: false, + }, + { + name: "kube signer honors duration exactly", + success: true, + obj: &certificates.CertificateSigningRequest{ + Status: certificates.CertificateSigningRequestStatus{ + Certificate: createCert(t, time.Hour, caPrivateKey, caCert), + }, + }, + old: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + SignerName: "kubernetes.io/educate-dolphins", + ExpirationSeconds: csr.DurationToExpirationSeconds(time.Hour), + }, + }, + options: &metav1.UpdateOptions{}, + wantSigner: "kubernetes.io/educate-dolphins", + wantRequested: true, + wantHonored: true, + }, + { + name: "signer honors duration exactly", + success: true, + obj: &certificates.CertificateSigningRequest{ + Status: certificates.CertificateSigningRequestStatus{ + Certificate: createCert(t, time.Hour, caPrivateKey, caCert), + }, + }, + old: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + SignerName: "pandas", + ExpirationSeconds: csr.DurationToExpirationSeconds(time.Hour), + }, + }, + options: &metav1.UpdateOptions{}, + wantSigner: "other", + wantRequested: true, + wantHonored: true, + }, + { + name: "signer honors duration but just a little bit less", + success: true, + obj: &certificates.CertificateSigningRequest{ + Status: certificates.CertificateSigningRequestStatus{ + Certificate: createCert(t, time.Hour-6*time.Minute, caPrivateKey, caCert), + }, + }, + old: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + SignerName: "pandas", + ExpirationSeconds: csr.DurationToExpirationSeconds(time.Hour), + }, + }, + options: &metav1.UpdateOptions{}, + wantSigner: "other", + wantRequested: true, + wantHonored: true, + }, + { + name: "signer honors duration but just a little bit more", + success: true, + obj: &certificates.CertificateSigningRequest{ + Status: certificates.CertificateSigningRequestStatus{ + Certificate: createCert(t, time.Hour+6*time.Minute, caPrivateKey, caCert), + }, + }, + old: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + SignerName: "pandas", + ExpirationSeconds: csr.DurationToExpirationSeconds(time.Hour), + }, + }, + options: &metav1.UpdateOptions{}, + wantSigner: "other", + wantRequested: true, + wantHonored: true, + }, + { + name: "honors duration lower bound", + success: true, + obj: &certificates.CertificateSigningRequest{ + Status: certificates.CertificateSigningRequestStatus{ + Certificate: createCert(t, 651*time.Second, caPrivateKey, caCert), + }, + }, + old: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + SignerName: "kubernetes.io/educate-dolphins", + ExpirationSeconds: csr.DurationToExpirationSeconds(1_000 * time.Second), + }, + }, + options: &metav1.UpdateOptions{}, + wantSigner: "kubernetes.io/educate-dolphins", + wantRequested: true, + wantHonored: true, + }, + { + name: "does not honor duration just outside of lower bound", + success: true, + obj: &certificates.CertificateSigningRequest{ + Status: certificates.CertificateSigningRequestStatus{ + Certificate: createCert(t, 650*time.Second, caPrivateKey, caCert), + }, + }, + old: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + SignerName: "kubernetes.io/educate-dolphins", + ExpirationSeconds: csr.DurationToExpirationSeconds(1_000 * time.Second), + }, + }, + options: &metav1.UpdateOptions{}, + wantSigner: "kubernetes.io/educate-dolphins", + wantRequested: true, + wantHonored: false, + }, + { + name: "honors duration upper bound", + success: true, + obj: &certificates.CertificateSigningRequest{ + Status: certificates.CertificateSigningRequestStatus{ + Certificate: createCert(t, 1349*time.Second, caPrivateKey, caCert), + }, + }, + old: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + SignerName: "kubernetes.io/educate-dolphins", + ExpirationSeconds: csr.DurationToExpirationSeconds(1_000 * time.Second), + }, + }, + options: &metav1.UpdateOptions{}, + wantSigner: "kubernetes.io/educate-dolphins", + wantRequested: true, + wantHonored: true, + }, + { + name: "does not honor duration just outside of upper bound", + success: true, + obj: &certificates.CertificateSigningRequest{ + Status: certificates.CertificateSigningRequestStatus{ + Certificate: createCert(t, 1350*time.Second, caPrivateKey, caCert), + }, + }, + old: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + SignerName: "kubernetes.io/educate-dolphins", + ExpirationSeconds: csr.DurationToExpirationSeconds(1_000 * time.Second), + }, + }, + options: &metav1.UpdateOptions{}, + wantSigner: "kubernetes.io/educate-dolphins", + wantRequested: true, + wantHonored: false, + }, + { + name: "failed update is ignored", + success: false, + obj: &certificates.CertificateSigningRequest{ + Status: certificates.CertificateSigningRequestStatus{ + Certificate: createCert(t, time.Hour, caPrivateKey, caCert), + }, + }, + old: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + SignerName: "pandas", + ExpirationSeconds: csr.DurationToExpirationSeconds(time.Hour), + }, + }, + options: &metav1.UpdateOptions{}, + wantSigner: "", + wantRequested: false, + wantHonored: false, + }, + { + name: "dry run is ignored", + success: true, + obj: &certificates.CertificateSigningRequest{ + Status: certificates.CertificateSigningRequestStatus{ + Certificate: createCert(t, time.Hour, caPrivateKey, caCert), + }, + }, + old: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + SignerName: "pandas", + ExpirationSeconds: csr.DurationToExpirationSeconds(time.Hour), + }, + }, + options: &metav1.UpdateOptions{DryRun: []string{"stuff"}}, + wantSigner: "", + wantRequested: false, + wantHonored: false, + }, + { + name: "no metrics when feature gate is turned off", + disableFeatureGate: true, + success: true, + obj: &certificates.CertificateSigningRequest{ + Status: certificates.CertificateSigningRequestStatus{ + Certificate: createCert(t, time.Hour, caPrivateKey, caCert), + }, + }, + old: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + SignerName: "pandas", + ExpirationSeconds: csr.DurationToExpirationSeconds(time.Hour), + }, + }, + options: &metav1.UpdateOptions{}, + wantSigner: "", + wantRequested: false, + wantHonored: false, + }, + { + name: "old CSR already has a cert so it is ignored", + success: true, + obj: &certificates.CertificateSigningRequest{ + Status: certificates.CertificateSigningRequestStatus{ + Certificate: createCert(t, time.Hour, caPrivateKey, caCert), + }, + }, + old: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + SignerName: "pandas", + ExpirationSeconds: csr.DurationToExpirationSeconds(time.Hour), + }, + Status: certificates.CertificateSigningRequestStatus{ + Certificate: []byte("junk"), + }, + }, + options: &metav1.UpdateOptions{}, + wantSigner: "", + wantRequested: false, + wantHonored: false, + }, + { + name: "CSRs with no duration are ignored", + success: true, + obj: &certificates.CertificateSigningRequest{ + Status: certificates.CertificateSigningRequestStatus{ + Certificate: createCert(t, time.Hour, caPrivateKey, caCert), + }, + }, + old: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + SignerName: "pandas", + ExpirationSeconds: nil, + }, + }, + options: &metav1.UpdateOptions{}, + wantSigner: "", + wantRequested: false, + wantHonored: false, + }, + { + name: "unissued CSRs are ignored", + success: true, + obj: &certificates.CertificateSigningRequest{ + Status: certificates.CertificateSigningRequestStatus{ + Certificate: nil, + }, + }, + old: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + SignerName: "pandas", + ExpirationSeconds: csr.DurationToExpirationSeconds(time.Hour), + }, + }, + options: &metav1.UpdateOptions{}, + wantSigner: "", + wantRequested: false, + wantHonored: false, + }, + { + name: "invalid data - nil old object", + success: true, + obj: &certificates.CertificateSigningRequest{ + Status: certificates.CertificateSigningRequestStatus{ + Certificate: createCert(t, time.Hour, caPrivateKey, caCert), + }, + }, + old: nil, + options: &metav1.UpdateOptions{}, + wantSigner: "", + wantRequested: false, + wantHonored: false, + }, + { + name: "invalid data - nil new object", + success: true, + obj: nil, + old: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + SignerName: "pandas", + ExpirationSeconds: csr.DurationToExpirationSeconds(time.Hour), + }, + }, + options: &metav1.UpdateOptions{}, + wantSigner: "", + wantRequested: false, + wantHonored: false, + }, + { + name: "invalid data - junk old object", + success: true, + obj: &certificates.CertificateSigningRequest{ + Status: certificates.CertificateSigningRequestStatus{ + Certificate: createCert(t, time.Hour, caPrivateKey, caCert), + }, + }, + old: &corev1.Pod{}, + options: &metav1.UpdateOptions{}, + wantSigner: "", + wantRequested: false, + wantHonored: false, + }, + { + name: "invalid data - junk new object", + success: true, + obj: &corev1.Pod{}, + old: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + SignerName: "pandas", + ExpirationSeconds: csr.DurationToExpirationSeconds(time.Hour), + }, + }, + options: &metav1.UpdateOptions{}, + wantSigner: "", + wantRequested: false, + wantHonored: false, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + if tt.disableFeatureGate { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSRDuration, false)() + } else { + t.Parallel() + } + + testReq := &testCounterVecMetric{} + testHon := &testCounterVecMetric{} + + finishFunc, err := countCSRDurationMetric(testReq, testHon)(nil, tt.obj, tt.old, tt.options) + if err != nil { + t.Fatal(err) + } + + finishFunc(nil, tt.success) + + if got := testReq.signer; tt.wantRequested && tt.wantSigner != got { + t.Errorf("requested signer: want %v, got %v", tt.wantSigner, got) + } + + if got := testHon.signer; tt.wantHonored && tt.wantSigner != got { + t.Errorf("honored signer: want %v, got %v", tt.wantSigner, got) + } + + if got := testReq.called; tt.wantRequested != got { + t.Errorf("requested inc: want %v, got %v", tt.wantRequested, got) + } + + if got := testHon.called; tt.wantHonored != got { + t.Errorf("honored inc: want %v, got %v", tt.wantHonored, got) + } + }) + } +} + +func createCert(t *testing.T, duration time.Duration, caPrivateKey *ecdsa.PrivateKey, caCert *x509.Certificate) []byte { + t.Helper() + + crPublicKey := &caPrivateKey.PublicKey // this is supposed to be public key of the signee but it does not matter for this test + + now := time.Now() + tmpl := &x509.Certificate{Subject: pkix.Name{CommonName: "panda"}, SerialNumber: big.NewInt(1234), NotBefore: now, NotAfter: now.Add(duration)} + + der, err := x509.CreateCertificate(rand.Reader, tmpl, caCert, crPublicKey, caPrivateKey) + if err != nil { + t.Fatal(err) + } + + return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}) +} + +type testCounterVecMetric struct { + metrics.CounterMetric + + signer string + called bool +} + +func (m *testCounterVecMetric) WithLabelValues(lv ...string) metrics.CounterMetric { + if len(lv) != 1 { + panic(lv) + } + + if len(m.signer) != 0 { + panic("unexpected multiple WithLabelValues() calls") + } + + signer := lv[0] + + if len(signer) == 0 { + panic("invalid empty signer") + } + + m.signer = signer + return m +} + +func (m *testCounterVecMetric) Inc() { + if m.called { + panic("unexpected multiple Inc() calls") + } + + m.called = true +} diff --git a/pkg/registry/certificates/certificates/storage/storage.go b/pkg/registry/certificates/certificates/storage/storage.go index dd4bd8c88ae..4df8f17a22a 100644 --- a/pkg/registry/certificates/certificates/storage/storage.go +++ b/pkg/registry/certificates/certificates/storage/storage.go @@ -62,6 +62,7 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, *Approva statusStore := *store statusStore.UpdateStrategy = csrregistry.StatusStrategy statusStore.ResetFieldsStrategy = csrregistry.StatusStrategy + statusStore.BeginUpdate = countCSRDurationMetric(csrDurationRequested, csrDurationHonored) approvalStore := *store approvalStore.UpdateStrategy = csrregistry.ApprovalStrategy diff --git a/pkg/registry/certificates/certificates/strategy.go b/pkg/registry/certificates/certificates/strategy.go index 076f5a59442..41281ee177d 100644 --- a/pkg/registry/certificates/certificates/strategy.go +++ b/pkg/registry/certificates/certificates/strategy.go @@ -30,9 +30,11 @@ import ( genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/generic" "k8s.io/apiserver/pkg/storage/names" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/apis/certificates" "k8s.io/kubernetes/pkg/apis/certificates/validation" + "k8s.io/kubernetes/pkg/features" "sigs.k8s.io/structured-merge-diff/v4/fieldpath" ) @@ -91,10 +93,14 @@ func (csrStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) { if extra := user.GetExtra(); len(extra) > 0 { csr.Spec.Extra = map[string]certificates.ExtraValue{} for k, v := range extra { - csr.Spec.Extra[k] = certificates.ExtraValue(v) + csr.Spec.Extra[k] = v } } } + // clear expirationSeconds if the CSRDuration feature is disabled + if !utilfeature.DefaultFeatureGate.Enabled(features.CSRDuration) { + csr.Spec.ExpirationSeconds = nil + } // Be explicit that users cannot create pre-approved certificate requests. csr.Status = certificates.CertificateSigningRequestStatus{} diff --git a/pkg/registry/certificates/certificates/strategy_test.go b/pkg/registry/certificates/certificates/strategy_test.go index 0c940b296f2..fb1318d3ea5 100644 --- a/pkg/registry/certificates/certificates/strategy_test.go +++ b/pkg/registry/certificates/certificates/strategy_test.go @@ -27,14 +27,19 @@ import ( "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apiserver/pkg/authentication/user" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" certapi "k8s.io/kubernetes/pkg/apis/certificates" + "k8s.io/kubernetes/pkg/features" + "k8s.io/utils/pointer" ) func TestStrategyCreate(t *testing.T) { tests := map[string]struct { - ctx context.Context - obj runtime.Object - expectedObj runtime.Object + ctx context.Context + disableFeatureGate bool + obj runtime.Object + expectedObj runtime.Object }{ "no user in context, no user in obj": { ctx: genericapirequest.NewContext(), @@ -112,15 +117,51 @@ func TestStrategyCreate(t *testing.T) { }, expectedObj: &certapi.CertificateSigningRequest{ Status: certapi.CertificateSigningRequestStatus{Conditions: []certapi.CertificateSigningRequestCondition{}}, - }}, + }, + }, + "expirationSeconds set with gate enabled": { + ctx: genericapirequest.NewContext(), + obj: &certapi.CertificateSigningRequest{ + Spec: certapi.CertificateSigningRequestSpec{ + ExpirationSeconds: pointer.Int32(1234), + }, + }, + expectedObj: &certapi.CertificateSigningRequest{ + Spec: certapi.CertificateSigningRequestSpec{ + ExpirationSeconds: pointer.Int32(1234), + }, + Status: certapi.CertificateSigningRequestStatus{Conditions: []certapi.CertificateSigningRequestCondition{}}, + }, + }, + "expirationSeconds set with gate disabled": { + ctx: genericapirequest.NewContext(), + disableFeatureGate: true, + obj: &certapi.CertificateSigningRequest{ + Spec: certapi.CertificateSigningRequestSpec{ + ExpirationSeconds: pointer.Int32(5678), + }, + }, + expectedObj: &certapi.CertificateSigningRequest{ + Spec: certapi.CertificateSigningRequestSpec{ + ExpirationSeconds: nil, + }, + Status: certapi.CertificateSigningRequestStatus{Conditions: []certapi.CertificateSigningRequestCondition{}}, + }, + }, } for k, tc := range tests { - obj := tc.obj - Strategy.PrepareForCreate(tc.ctx, obj) - if !reflect.DeepEqual(obj, tc.expectedObj) { - t.Errorf("%s: object diff: %s", k, diff.ObjectDiff(obj, tc.expectedObj)) - } + tc := tc + t.Run(k, func(t *testing.T) { + if tc.disableFeatureGate { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSRDuration, false)() + } + obj := tc.obj + Strategy.PrepareForCreate(tc.ctx, obj) + if !reflect.DeepEqual(obj, tc.expectedObj) { + t.Errorf("object diff: %s", diff.ObjectDiff(obj, tc.expectedObj)) + } + }) } } diff --git a/staging/src/k8s.io/api/certificates/v1/types.go b/staging/src/k8s.io/api/certificates/v1/types.go index 8d3b2305ecf..4bfd1fb613a 100644 --- a/staging/src/k8s.io/api/certificates/v1/types.go +++ b/staging/src/k8s.io/api/certificates/v1/types.go @@ -44,7 +44,7 @@ type CertificateSigningRequest struct { metav1.ObjectMeta `json:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"` // spec contains the certificate request, and is immutable after creation. - // Only the request, signerName, and usages fields can be set on creation. + // Only the request, signerName, expirationSeconds, and usages fields can be set on creation. // Other fields are derived by Kubernetes and cannot be modified by users. Spec CertificateSigningRequestSpec `json:"spec" protobuf:"bytes,2,opt,name=spec"` @@ -84,6 +84,30 @@ type CertificateSigningRequestSpec struct { // 6. Whether or not requests for CA certificates are allowed. SignerName string `json:"signerName" protobuf:"bytes,7,opt,name=signerName"` + // 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 `json:"expirationSeconds,omitempty" protobuf:"varint,8,opt,name=expirationSeconds"` + // usages specifies a set of key usages requested in the issued certificate. // // Requests for TLS client certificates typically request: "digital signature", "key encipherment", "client auth". diff --git a/staging/src/k8s.io/api/certificates/v1beta1/types.go b/staging/src/k8s.io/api/certificates/v1beta1/types.go index 9e61c67ff4d..031ef77550b 100644 --- a/staging/src/k8s.io/api/certificates/v1beta1/types.go +++ b/staging/src/k8s.io/api/certificates/v1beta1/types.go @@ -36,18 +36,17 @@ type CertificateSigningRequest struct { // +optional metav1.ObjectMeta `json:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"` - // The certificate request itself and any additional information. - // +optional - Spec CertificateSigningRequestSpec `json:"spec,omitempty" protobuf:"bytes,2,opt,name=spec"` + // spec contains the certificate request, and is immutable after creation. + // Only the request, signerName, expirationSeconds, and usages fields can be set on creation. + // Other fields are derived by Kubernetes and cannot be modified by users. + Spec CertificateSigningRequestSpec `json:"spec" protobuf:"bytes,2,opt,name=spec"` // Derived information about the request. // +optional Status CertificateSigningRequestStatus `json:"status,omitempty" protobuf:"bytes,3,opt,name=status"` } -// This information is immutable after the request is created. Only the Request -// and Usages fields can be set on creation, other fields are derived by -// Kubernetes and cannot be modified by users. +// CertificateSigningRequestSpec contains the certificate request. type CertificateSigningRequestSpec struct { // Base64-encoded PKCS#10 CSR data // +listType=atomic @@ -66,6 +65,30 @@ type CertificateSigningRequestSpec struct { // +optional SignerName *string `json:"signerName,omitempty" protobuf:"bytes,7,opt,name=signerName"` + // 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 `json:"expirationSeconds,omitempty" protobuf:"varint,8,opt,name=expirationSeconds"` + // allowedUsages 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/staging/src/k8s.io/client-go/util/certificate/certificate_manager.go b/staging/src/k8s.io/client-go/util/certificate/certificate_manager.go index 26a01a1d2ca..dee21cf3089 100644 --- a/staging/src/k8s.io/client-go/util/certificate/certificate_manager.go +++ b/staging/src/k8s.io/client-go/util/certificate/certificate_manager.go @@ -88,6 +88,11 @@ type Config struct { // SignerName is the name of the certificate signer that should sign certificates // generated by the manager. SignerName string + // RequestedCertificateLifetime is the requested lifetime length for certificates generated by the manager. + // Optional. + // This will set the spec.expirationSeconds field on the CSR. Controlling the lifetime of + // the issued certificate is not guaranteed as the signer may choose to ignore the request. + RequestedCertificateLifetime *time.Duration // Usages is the types of usages that certificates generated by the manager // can be used for. Usages []certificates.KeyUsage @@ -184,10 +189,11 @@ type manager struct { lastRequestCancel context.CancelFunc lastRequest *x509.CertificateRequest - dynamicTemplate bool - signerName string - usages []certificates.KeyUsage - forceRotation bool + dynamicTemplate bool + signerName string + requestedCertificateLifetime *time.Duration + usages []certificates.KeyUsage + forceRotation bool certStore Store @@ -230,18 +236,19 @@ func NewManager(config *Config) (Manager, error) { } m := manager{ - stopCh: make(chan struct{}), - clientsetFn: config.ClientsetFn, - getTemplate: getTemplate, - dynamicTemplate: config.GetTemplate != nil, - signerName: config.SignerName, - usages: config.Usages, - certStore: config.CertificateStore, - cert: cert, - forceRotation: forceRotation, - certificateRotation: config.CertificateRotation, - certificateRenewFailure: config.CertificateRenewFailure, - now: time.Now, + stopCh: make(chan struct{}), + clientsetFn: config.ClientsetFn, + getTemplate: getTemplate, + dynamicTemplate: config.GetTemplate != nil, + signerName: config.SignerName, + requestedCertificateLifetime: config.RequestedCertificateLifetime, + usages: config.Usages, + certStore: config.CertificateStore, + cert: cert, + forceRotation: forceRotation, + certificateRotation: config.CertificateRotation, + certificateRenewFailure: config.CertificateRenewFailure, + now: time.Now, } name := config.Name @@ -459,7 +466,7 @@ func (m *manager) rotateCerts() (bool, error) { // Call the Certificate Signing Request API to get a certificate for the // new private key. - reqName, reqUID, err := csr.RequestCertificate(clientSet, csrPEM, "", m.signerName, m.usages, privateKey) + reqName, reqUID, err := csr.RequestCertificate(clientSet, csrPEM, "", m.signerName, m.requestedCertificateLifetime, m.usages, privateKey) if err != nil { utilruntime.HandleError(fmt.Errorf("%s: Failed while requesting a signed certificate from the control plane: %v", m.name, err)) if m.certificateRenewFailure != nil { diff --git a/staging/src/k8s.io/client-go/util/certificate/csr/csr.go b/staging/src/k8s.io/client-go/util/certificate/csr/csr.go index ec117663487..0017007a2d2 100644 --- a/staging/src/k8s.io/client-go/util/certificate/csr/csr.go +++ b/staging/src/k8s.io/client-go/util/certificate/csr/csr.go @@ -25,8 +25,6 @@ import ( "reflect" "time" - "k8s.io/klog/v2" - certificatesv1 "k8s.io/api/certificates/v1" certificatesv1beta1 "k8s.io/api/certificates/v1beta1" "k8s.io/apimachinery/pkg/api/errors" @@ -41,12 +39,16 @@ import ( "k8s.io/client-go/tools/cache" watchtools "k8s.io/client-go/tools/watch" certutil "k8s.io/client-go/util/cert" + "k8s.io/klog/v2" + "k8s.io/utils/pointer" ) // RequestCertificate will either use an existing (if this process has run // before but not to completion) or create a certificate signing request using the -// PEM encoded CSR and send it to API server. -func RequestCertificate(client clientset.Interface, csrData []byte, name string, signerName string, usages []certificatesv1.KeyUsage, privateKey interface{}) (reqName string, reqUID types.UID, err error) { +// PEM encoded CSR and send it to API server. An optional requestedDuration may be passed +// to set the spec.expirationSeconds field on the CSR to control the lifetime of the issued +// certificate. This is not guaranteed as the signer may choose to ignore the request. +func RequestCertificate(client clientset.Interface, csrData []byte, name, signerName string, requestedDuration *time.Duration, usages []certificatesv1.KeyUsage, privateKey interface{}) (reqName string, reqUID types.UID, err error) { csr := &certificatesv1.CertificateSigningRequest{ // Username, UID, Groups will be injected by API server. TypeMeta: metav1.TypeMeta{Kind: "CertificateSigningRequest"}, @@ -62,6 +64,9 @@ func RequestCertificate(client clientset.Interface, csrData []byte, name string, if len(csr.Name) == 0 { csr.GenerateName = "csr-" } + if requestedDuration != nil { + csr.Spec.ExpirationSeconds = DurationToExpirationSeconds(*requestedDuration) + } reqName, reqUID, err = create(client, csr) switch { @@ -85,6 +90,14 @@ func RequestCertificate(client clientset.Interface, csrData []byte, name string, } } +func DurationToExpirationSeconds(duration time.Duration) *int32 { + return pointer.Int32(int32(duration / time.Second)) +} + +func ExpirationSecondsToDuration(expirationSeconds int32) time.Duration { + return time.Duration(expirationSeconds) * time.Second +} + func get(client clientset.Interface, name string) (*certificatesv1.CertificateSigningRequest, error) { v1req, v1err := client.CertificatesV1().CertificateSigningRequests().Get(context.TODO(), name, metav1.GetOptions{}) if v1err == nil || !apierrors.IsNotFound(v1err) { diff --git a/staging/src/k8s.io/kube-controller-manager/config/v1alpha1/types.go b/staging/src/k8s.io/kube-controller-manager/config/v1alpha1/types.go index 9acd2b98c91..247f3fce24c 100644 --- a/staging/src/k8s.io/kube-controller-manager/config/v1alpha1/types.go +++ b/staging/src/k8s.io/kube-controller-manager/config/v1alpha1/types.go @@ -193,8 +193,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/staging/src/k8s.io/kubectl/pkg/describe/describe.go b/staging/src/k8s.io/kubectl/pkg/describe/describe.go index 35d13705229..b366c044154 100644 --- a/staging/src/k8s.io/kubectl/pkg/describe/describe.go +++ b/staging/src/k8s.io/kubectl/pkg/describe/describe.go @@ -33,7 +33,6 @@ import ( "unicode" "github.com/fatih/camelcase" - "k8s.io/apimachinery/pkg/runtime" appsv1 "k8s.io/api/apps/v1" autoscalingv1 "k8s.io/api/autoscaling/v1" @@ -60,6 +59,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/duration" @@ -72,6 +72,7 @@ import ( corev1client "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/rest" "k8s.io/client-go/tools/reference" + utilcsr "k8s.io/client-go/util/certificate/csr" "k8s.io/klog/v2" "k8s.io/kubectl/pkg/scheme" "k8s.io/kubectl/pkg/util/certificate" @@ -3690,12 +3691,13 @@ type CertificateSigningRequestDescriber struct { func (p *CertificateSigningRequestDescriber) Describe(namespace, name string, describerSettings DescriberSettings) (string, error) { var ( - crBytes []byte - metadata metav1.ObjectMeta - status string - signerName string - username string - events *corev1.EventList + crBytes []byte + metadata metav1.ObjectMeta + status string + signerName string + expirationSeconds *int32 + username string + events *corev1.EventList ) if csr, err := p.client.CertificatesV1().CertificateSigningRequests().Get(context.TODO(), name, metav1.GetOptions{}); err == nil { @@ -3707,6 +3709,7 @@ func (p *CertificateSigningRequestDescriber) Describe(namespace, name string, de } status = extractCSRStatus(conditionTypes, csr.Status.Certificate) signerName = csr.Spec.SignerName + expirationSeconds = csr.Spec.ExpirationSeconds username = csr.Spec.Username if describerSettings.ShowEvents { events, _ = searchEvents(p.client.CoreV1(), csr, describerSettings.ChunkSize) @@ -3722,6 +3725,7 @@ func (p *CertificateSigningRequestDescriber) Describe(namespace, name string, de if csr.Spec.SignerName != nil { signerName = *csr.Spec.SignerName } + expirationSeconds = csr.Spec.ExpirationSeconds username = csr.Spec.Username if describerSettings.ShowEvents { events, _ = searchEvents(p.client.CoreV1(), csr, describerSettings.ChunkSize) @@ -3735,10 +3739,10 @@ func (p *CertificateSigningRequestDescriber) Describe(namespace, name string, de return "", fmt.Errorf("Error parsing CSR: %v", err) } - return describeCertificateSigningRequest(metadata, signerName, username, cr, status, events) + return describeCertificateSigningRequest(metadata, signerName, expirationSeconds, username, cr, status, events) } -func describeCertificateSigningRequest(csr metav1.ObjectMeta, signerName string, username string, cr *x509.CertificateRequest, status string, events *corev1.EventList) (string, error) { +func describeCertificateSigningRequest(csr metav1.ObjectMeta, signerName string, expirationSeconds *int32, username string, cr *x509.CertificateRequest, status string, events *corev1.EventList) (string, error) { printListHelper := func(w PrefixWriter, prefix, name string, values []string) { if len(values) == 0 { return @@ -3758,6 +3762,9 @@ func describeCertificateSigningRequest(csr metav1.ObjectMeta, signerName string, if len(signerName) > 0 { w.Write(LEVEL_0, "Signer:\t%s\n", signerName) } + if expirationSeconds != nil { + w.Write(LEVEL_0, "Requested Duration:\t%s\n", duration.HumanDuration(utilcsr.ExpirationSecondsToDuration(*expirationSeconds))) + } w.Write(LEVEL_0, "Status:\t%s\n", status) w.Write(LEVEL_0, "Subject:\n") diff --git a/test/integration/certificates/duration_test.go b/test/integration/certificates/duration_test.go new file mode 100644 index 00000000000..163ec860a9f --- /dev/null +++ b/test/integration/certificates/duration_test.go @@ -0,0 +1,293 @@ +/* +Copyright 2021 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 certificates + +import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509/pkix" + "encoding/pem" + "io/ioutil" + "os" + "path" + "strings" + "testing" + "time" + + "github.com/google/go-cmp/cmp" + + certificatesv1 "k8s.io/api/certificates/v1" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructuredscheme" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apiserver/pkg/server/dynamiccertificates" + "k8s.io/client-go/informers" + clientset "k8s.io/client-go/kubernetes" + "k8s.io/client-go/rest" + certutil "k8s.io/client-go/util/cert" + "k8s.io/client-go/util/certificate/csr" + "k8s.io/client-go/util/keyutil" + kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing" + "k8s.io/kubernetes/pkg/controller/certificates/signer" + "k8s.io/kubernetes/test/integration/framework" +) + +func TestCSRDuration(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) + t.Cleanup(cancel) + + s := kubeapiservertesting.StartTestServerOrDie(t, nil, nil, framework.SharedEtcd()) + t.Cleanup(s.TearDownFn) + + // assert that the metrics we collect during the test run match expectations + // we have 7 valid test cases below that request a duration of which 6 should have their duration honored + wantMetricStrings := []string{ + `apiserver_certificates_registry_csr_honored_duration_total{signerName="kubernetes.io/kube-apiserver-client"} 6`, + `apiserver_certificates_registry_csr_requested_duration_total{signerName="kubernetes.io/kube-apiserver-client"} 7`, + } + t.Cleanup(func() { + copyConfig := rest.CopyConfig(s.ClientConfig) + copyConfig.GroupVersion = &schema.GroupVersion{} + copyConfig.NegotiatedSerializer = unstructuredscheme.NewUnstructuredNegotiatedSerializer() + rc, err := rest.RESTClientFor(copyConfig) + if err != nil { + t.Fatal(err) + } + body, err := rc.Get().AbsPath("/metrics").DoRaw(ctx) + if err != nil { + t.Fatal(err) + } + var gotMetricStrings []string + for _, line := range strings.Split(string(body), "\n") { + if strings.HasPrefix(line, "apiserver_certificates_registry_") { + gotMetricStrings = append(gotMetricStrings, line) + } + } + if diff := cmp.Diff(wantMetricStrings, gotMetricStrings); diff != "" { + t.Errorf("unexpected metrics diff (-want +got): %s", diff) + } + }) + + client := clientset.NewForConfigOrDie(s.ClientConfig) + informerFactory := informers.NewSharedInformerFactory(client, 0) + + caPrivateKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + caCert, err := certutil.NewSelfSignedCACert(certutil.Config{CommonName: "test-ca"}, caPrivateKey) + if err != nil { + t.Fatal(err) + } + caPublicKeyFile := path.Join(s.TmpDir, "test-ca-public-key") + if err := ioutil.WriteFile(caPublicKeyFile, pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: caCert.Raw}), os.FileMode(0600)); err != nil { + t.Fatal(err) + } + caPrivateKeyBytes, err := keyutil.MarshalPrivateKeyToPEM(caPrivateKey) + if err != nil { + t.Fatal(err) + } + caPrivateKeyFile := path.Join(s.TmpDir, "test-ca-private-key") + if err := ioutil.WriteFile(caPrivateKeyFile, caPrivateKeyBytes, os.FileMode(0600)); err != nil { + t.Fatal(err) + } + + c, err := signer.NewKubeAPIServerClientCSRSigningController(client, informerFactory.Certificates().V1().CertificateSigningRequests(), caPublicKeyFile, caPrivateKeyFile, 24*time.Hour) + if err != nil { + t.Fatal(err) + } + + stopCh := make(chan struct{}) + t.Cleanup(func() { + close(stopCh) + }) + + informerFactory.Start(stopCh) + go c.Run(1, stopCh) + + tests := []struct { + name, csrName string + duration, wantDuration time.Duration + wantError string + }{ + { + name: "no duration set", + duration: 0, + wantDuration: 24 * time.Hour, + wantError: "", + }, + { + name: "same duration set as certTTL", + duration: 24 * time.Hour, + wantDuration: 24 * time.Hour, + wantError: "", + }, + { + name: "longer duration than certTTL", + duration: 48 * time.Hour, + wantDuration: 24 * time.Hour, + wantError: "", + }, + { + name: "slightly shorter duration set", + duration: 20 * time.Hour, + wantDuration: 20 * time.Hour, + wantError: "", + }, + { + name: "even shorter duration set", + duration: 10 * time.Hour, + wantDuration: 10 * time.Hour, + wantError: "", + }, + { + name: "short duration set", + duration: 2 * time.Hour, + wantDuration: 2*time.Hour + 5*time.Minute, + wantError: "", + }, + { + name: "very short duration set", + duration: 30 * time.Minute, + wantDuration: 30*time.Minute + 5*time.Minute, + wantError: "", + }, + { + name: "shortest duration set", + duration: 10 * time.Minute, + wantDuration: 10*time.Minute + 5*time.Minute, + wantError: "", + }, + { + name: "just too short duration set", + csrName: "invalid-csr-001", + duration: 10*time.Minute - time.Second, + wantDuration: 0, + wantError: `cannot create certificate signing request: ` + + `CertificateSigningRequest.certificates.k8s.io "invalid-csr-001" is invalid: spec.expirationSeconds: Invalid value: 599: may not specify a duration less than 600 seconds (10 minutes)`, + }, + { + name: "really too short duration set", + csrName: "invalid-csr-002", + duration: 3 * time.Minute, + wantDuration: 0, + wantError: `cannot create certificate signing request: ` + + `CertificateSigningRequest.certificates.k8s.io "invalid-csr-002" is invalid: spec.expirationSeconds: Invalid value: 180: may not specify a duration less than 600 seconds (10 minutes)`, + }, + { + name: "negative duration set", + csrName: "invalid-csr-003", + duration: -7 * time.Minute, + wantDuration: 0, + wantError: `cannot create certificate signing request: ` + + `CertificateSigningRequest.certificates.k8s.io "invalid-csr-003" is invalid: spec.expirationSeconds: Invalid value: -420: may not specify a duration less than 600 seconds (10 minutes)`, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + privateKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + csrData, err := certutil.MakeCSR(privateKey, &pkix.Name{CommonName: "panda"}, nil, nil) + if err != nil { + t.Fatal(err) + } + + csrName, csrUID, errReq := csr.RequestCertificate(client, csrData, tt.csrName, certificatesv1.KubeAPIServerClientSignerName, + durationPtr(tt.duration), []certificatesv1.KeyUsage{certificatesv1.UsageClientAuth}, privateKey) + + if diff := cmp.Diff(tt.wantError, errStr(errReq)); len(diff) > 0 { + t.Fatalf("CSR input duration %v err diff (-want, +got):\n%s", tt.duration, diff) + } + + if len(tt.wantError) > 0 { + return + } + + csrObj, err := client.CertificatesV1().CertificateSigningRequests().Get(ctx, csrName, metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + csrObj.Status.Conditions = []certificatesv1.CertificateSigningRequestCondition{ + { + Type: certificatesv1.CertificateApproved, + Status: v1.ConditionTrue, + Reason: "TestCSRDuration", + Message: t.Name(), + }, + } + _, err = client.CertificatesV1().CertificateSigningRequests().UpdateApproval(ctx, csrName, csrObj, metav1.UpdateOptions{}) + if err != nil { + t.Fatal(err) + } + + certData, err := csr.WaitForCertificate(ctx, client, csrName, csrUID) + if err != nil { + t.Fatal(err) + } + + certs, err := certutil.ParseCertsPEM(certData) + if err != nil { + t.Fatal(err) + } + + switch l := len(certs); l { + case 1: + // good + default: + t.Errorf("expected 1 cert, got %d", l) + for i, certificate := range certs { + t.Log(i, dynamiccertificates.GetHumanCertDetail(certificate)) + } + t.FailNow() + } + + cert := certs[0] + + if got := cert.NotAfter.Sub(cert.NotBefore); got != tt.wantDuration { + t.Errorf("CSR input duration %v got duration = %v, want %v\n%s", tt.duration, got, tt.wantDuration, dynamiccertificates.GetHumanCertDetail(cert)) + } + }) + } +} + +func durationPtr(duration time.Duration) *time.Duration { + if duration == 0 { + return nil + } + return &duration +} + +func errStr(err error) string { + if err == nil { + return "" + } + es := err.Error() + if len(es) == 0 { + panic("invalid empty error") + } + return es +}