From 5f26ea7eb51c135e972f0cd09c8061c0606d4b30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Tue, 30 Jan 2024 16:56:04 +0100 Subject: [PATCH 1/6] Make CA lifetimes configurable Fixes #354 --- deploy/helm/secret-operator/crds/crds.yaml | 7 +++++++ rust/operator-binary/src/backend/dynamic.rs | 9 ++------- rust/operator-binary/src/backend/tls/mod.rs | 17 ++++++++++------- rust/operator-binary/src/crd.rs | 20 +++++++++++++++++--- 4 files changed, 36 insertions(+), 17 deletions(-) diff --git a/deploy/helm/secret-operator/crds/crds.yaml b/deploy/helm/secret-operator/crds/crds.yaml index 5d7f1951..652644c5 100644 --- a/deploy/helm/secret-operator/crds/crds.yaml +++ b/deploy/helm/secret-operator/crds/crds.yaml @@ -47,6 +47,13 @@ spec: default: false description: Whether the certificate authority should be managed by Secret Operator, including being generated if it does not already exist. type: boolean + caLifetime: + default: 730d + description: |- + The lifetime of each generated certificate authority. + + Ignored if `autoGenerate: false`. Should always be more than double `maxCertificateLifetime`. + type: string secret: description: Reference (name and namespace) to a Kubernetes Secret object where the CA certificate and key is stored in the keys `ca.crt` and `ca.key` respectively. properties: diff --git a/rust/operator-binary/src/backend/dynamic.rs b/rust/operator-binary/src/backend/dynamic.rs index d28f09b8..54d31ebf 100644 --- a/rust/operator-binary/src/backend/dynamic.rs +++ b/rust/operator-binary/src/backend/dynamic.rs @@ -101,17 +101,12 @@ pub async fn from_class( }) } crd::SecretClassBackend::AutoTls(crd::AutoTlsBackend { - ca: - crd::AutoTlsCa { - secret, - auto_generate, - }, + ca, max_certificate_lifetime, }) => from( super::TlsGenerate::get_or_create_k8s_certificate( client, - &secret, - auto_generate, + &ca, max_certificate_lifetime, ) .await?, diff --git a/rust/operator-binary/src/backend/tls/mod.rs b/rust/operator-binary/src/backend/tls/mod.rs index a42ac92b..5ef5213c 100644 --- a/rust/operator-binary/src/backend/tls/mod.rs +++ b/rust/operator-binary/src/backend/tls/mod.rs @@ -24,10 +24,10 @@ use stackable_operator::{ k8s_openapi::chrono::{self, FixedOffset, TimeZone}, time::Duration, }; -use stackable_secret_operator_crd_utils::SecretReference; use time::OffsetDateTime; use crate::{ + crd, format::{well_known, SecretData, WellKnownSecretData}, utils::iterator_try_concat_bytes, }; @@ -130,18 +130,21 @@ impl TlsGenerate { /// an independent self-signed CA. pub async fn get_or_create_k8s_certificate( client: &stackable_operator::client::Client, - secret_ref: &SecretReference, - auto_generate: bool, + crd::AutoTlsCa { + secret: ca_secret, + auto_generate: auto_generate_ca, + ca_lifetime, + }: &crd::AutoTlsCa, max_cert_lifetime: Duration, ) -> Result { Ok(Self { ca_manager: ca::Manager::load_or_create( client, - secret_ref, + ca_secret, &ca::Config { - manage_ca: auto_generate, - ca_lifetime: Duration::from_days_unchecked(2 * 365), - rotate_if_ca_expires_before: Some(Duration::from_days_unchecked(365)), + manage_ca: *auto_generate_ca, + ca_lifetime: *ca_lifetime, + rotate_if_ca_expires_before: Some(*ca_lifetime / 2), }, ) .await diff --git a/rust/operator-binary/src/crd.rs b/rust/operator-binary/src/crd.rs index 20b0ea91..d3d0f935 100644 --- a/rust/operator-binary/src/crd.rs +++ b/rust/operator-binary/src/crd.rs @@ -82,12 +82,14 @@ pub struct AutoTlsBackend { /// In case consumers request a longer lifetime than allowed by this setting, /// the lifetime will be the minimum of both, so this setting takes precedence. /// The default value is 15 days. - #[serde(default = "default_max_certificate_lifetime")] + #[serde(default = "AutoTlsBackend::default_max_certificate_lifetime")] pub max_certificate_lifetime: Duration, } -fn default_max_certificate_lifetime() -> Duration { - DEFAULT_MAX_CERT_LIFETIME +impl AutoTlsBackend { + fn default_max_certificate_lifetime() -> Duration { + DEFAULT_MAX_CERT_LIFETIME + } } #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)] @@ -102,6 +104,18 @@ pub struct AutoTlsCa { // TODO: Consider renaming to `manage` for v1alpha2 #[serde(default)] pub auto_generate: bool, + + /// The lifetime of each generated certificate authority. + /// + /// Ignored if `autoGenerate: false`. Should always be more than double `maxCertificateLifetime`. + #[serde(default = "AutoTlsCa::default_ca_lifetime")] + pub ca_lifetime: Duration, +} + +impl AutoTlsCa { + fn default_ca_lifetime() -> Duration { + Duration::from_days_unchecked(365 * 2) + } } #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)] From b11647f3441b967b7a72da4f34d673fceac155ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Tue, 30 Jan 2024 16:58:14 +0100 Subject: [PATCH 2/6] Changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9208c282..9f9be01e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,11 +14,13 @@ All notable changes to this project will be documented in this file. - Use new annotation builder ([#341]). - `autoTLS` certificate authorities will now be rotated regularly ([#350]). - [BREAKING] This changes the format of the CA secrets. Old secrets will be migrated automatically, but manual intervention will be required to downgrade back to 23.11.x. +- `autoTLS` certificate authority lifetimes are now configurable ([#357]). [#333]: https://github.com/stackabletech/secret-operator/pull/333 [#341]: https://github.com/stackabletech/secret-operator/pull/341 [#350]: https://github.com/stackabletech/secret-operator/pull/350 [#352]: https://github.com/stackabletech/secret-operator/pull/352 +[#357]: https://github.com/stackabletech/secret-operator/pull/357 ## [23.11.0] - 2023-11-24 From 21359e01d437c7ccd6b34144feb3f43fb7d2de8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Wed, 31 Jan 2024 12:17:34 +0100 Subject: [PATCH 3/6] Rename autoTls.ca.caLifetime to caCertificateLifetime --- deploy/helm/secret-operator/crds/crds.yaml | 6 ++++-- rust/operator-binary/src/backend/tls/ca.rs | 4 ++-- rust/operator-binary/src/backend/tls/mod.rs | 6 +++--- rust/operator-binary/src/crd.rs | 11 +++++++---- 4 files changed, 16 insertions(+), 11 deletions(-) diff --git a/deploy/helm/secret-operator/crds/crds.yaml b/deploy/helm/secret-operator/crds/crds.yaml index 652644c5..2bac783c 100644 --- a/deploy/helm/secret-operator/crds/crds.yaml +++ b/deploy/helm/secret-operator/crds/crds.yaml @@ -47,12 +47,14 @@ spec: default: false description: Whether the certificate authority should be managed by Secret Operator, including being generated if it does not already exist. type: boolean - caLifetime: + caCertificateLifetime: default: 730d description: |- The lifetime of each generated certificate authority. - Ignored if `autoGenerate: false`. Should always be more than double `maxCertificateLifetime`. + Should always be more than double `maxCertificateLifetime`. + + If `autoGenerate: true` then the Secret Operator will prepare a new CA certificate the old CA approaches expiration. If `autoGenerate: false` then the Secret Operator will log a warning instead. type: string secret: description: Reference (name and namespace) to a Kubernetes Secret object where the CA certificate and key is stored in the keys `ca.crt` and `ca.key` respectively. diff --git a/rust/operator-binary/src/backend/tls/ca.rs b/rust/operator-binary/src/backend/tls/ca.rs index f1a69467..42a502ed 100644 --- a/rust/operator-binary/src/backend/tls/ca.rs +++ b/rust/operator-binary/src/backend/tls/ca.rs @@ -140,7 +140,7 @@ pub struct Config { pub manage_ca: bool, /// The duration of any new CA certificates provisioned. - pub ca_lifetime: Duration, + pub ca_certificate_lifetime: Duration, /// If no existing CA certificate outlives `rotate_if_ca_expires_before`, a new /// certificate will be generated. @@ -185,7 +185,7 @@ impl CertificateAuthority { .build(); let now = OffsetDateTime::now_utc(); let not_before = now - Duration::from_minutes_unchecked(5); - let not_after = now + config.ca_lifetime; + let not_after = now + config.ca_certificate_lifetime; let conf = Conf::new(ConfMethod::default()).unwrap(); let private_key = Rsa::generate(2048) .and_then(PKey::try_from) diff --git a/rust/operator-binary/src/backend/tls/mod.rs b/rust/operator-binary/src/backend/tls/mod.rs index 5ef5213c..239a4db0 100644 --- a/rust/operator-binary/src/backend/tls/mod.rs +++ b/rust/operator-binary/src/backend/tls/mod.rs @@ -133,7 +133,7 @@ impl TlsGenerate { crd::AutoTlsCa { secret: ca_secret, auto_generate: auto_generate_ca, - ca_lifetime, + ca_certificate_lifetime, }: &crd::AutoTlsCa, max_cert_lifetime: Duration, ) -> Result { @@ -143,8 +143,8 @@ impl TlsGenerate { ca_secret, &ca::Config { manage_ca: *auto_generate_ca, - ca_lifetime: *ca_lifetime, - rotate_if_ca_expires_before: Some(*ca_lifetime / 2), + ca_certificate_lifetime: *ca_certificate_lifetime, + rotate_if_ca_expires_before: Some(*ca_certificate_lifetime / 2), }, ) .await diff --git a/rust/operator-binary/src/crd.rs b/rust/operator-binary/src/crd.rs index d3d0f935..664fa9a9 100644 --- a/rust/operator-binary/src/crd.rs +++ b/rust/operator-binary/src/crd.rs @@ -107,13 +107,16 @@ pub struct AutoTlsCa { /// The lifetime of each generated certificate authority. /// - /// Ignored if `autoGenerate: false`. Should always be more than double `maxCertificateLifetime`. - #[serde(default = "AutoTlsCa::default_ca_lifetime")] - pub ca_lifetime: Duration, + /// Should always be more than double `maxCertificateLifetime`. + /// + /// If `autoGenerate: true` then the Secret Operator will prepare a new CA certificate the old CA approaches expiration. + /// If `autoGenerate: false` then the Secret Operator will log a warning instead. + #[serde(default = "AutoTlsCa::default_ca_certificate_lifetime")] + pub ca_certificate_lifetime: Duration, } impl AutoTlsCa { - fn default_ca_lifetime() -> Duration { + fn default_ca_certificate_lifetime() -> Duration { Duration::from_days_unchecked(365 * 2) } } From a04ab656fbea09e74399821463fa9a41659a225a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Wed, 31 Jan 2024 12:26:19 +0100 Subject: [PATCH 4/6] Docs --- docs/modules/secret-operator/pages/secretclass.adoc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/modules/secret-operator/pages/secretclass.adoc b/docs/modules/secret-operator/pages/secretclass.adoc index b026f7af..7811aadf 100644 --- a/docs/modules/secret-operator/pages/secretclass.adoc +++ b/docs/modules/secret-operator/pages/secretclass.adoc @@ -62,8 +62,8 @@ Users can use podOverrides to extend the certificate lifetime by adding volume a Certificate authorities also have a limited lifetime, and need to be rotated before they expire to avoid cluster disruption. -If configured to provision its own CA (`autoTls.ca.autoGenerate`), the Secret Operator will create CA certificates that are valid for 2 years, -and initiate rotation when there is less than 1 year remaining. If configured _not_ to provision its own CA, a warning will be issued when there is less than 1 year remaining. +If configured to provision its own CA (`autoTls.ca.autoGenerate`), the Secret Operator will create CA certificates that are valid for 2 years (`autoTls.ca.caCertificateLifetime`), +and initiate rotation once less than half of that time remains. If configured _not_ to provision its own CA, a warning will instead be issued in that case. To avoid disruption and let the new CA propagate through the cluster, the Secret Operator will prefer using the oldest CA that will last for the entire lifetime of the issued certificate. @@ -81,6 +81,7 @@ spec: name: secret-provisioner-tls-ca namespace: default autoGenerate: true + caCertificateLifetime: 700d maxCertificateLifetime: 15d # optional ---- @@ -89,6 +90,7 @@ spec: `autoTls.ca.secret`:: Reference (`name` and `namespace`) to a K8s `Secret` object where the CA certificate and key is stored in the keys `ca.crt` and `ca.key` respectively. `autoTls.ca.autoGenerate`:: Whether the certificate authority should be provisioned and managed by the Secret Operator. +`autoTls.ca.caCertificateLifetime` :: The lifetime of the certificate authority's root certificate. `autoTls.maxCertificateLifetime`:: Maximum lifetime the created certificates are allowed to have. In case consumers request a longer lifetime than allowed by this setting, the lifetime will be the minimum of both. [#backend-kerberoskeytab] From eda44f2f40343bfe0a612dab66729d3070bf9375 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Wed, 31 Jan 2024 16:47:48 +0100 Subject: [PATCH 5/6] =?UTF-8?q?Fix=20b=C3=B8rked=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- rust/operator-binary/src/backend/tls/mod.rs | 4 ++++ rust/operator-binary/src/crd.rs | 11 +++++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/rust/operator-binary/src/backend/tls/mod.rs b/rust/operator-binary/src/backend/tls/mod.rs index 239a4db0..e058e9c3 100644 --- a/rust/operator-binary/src/backend/tls/mod.rs +++ b/rust/operator-binary/src/backend/tls/mod.rs @@ -40,6 +40,10 @@ use super::{ mod ca; +/// How long CA certificates should last for. Also used for calculating when they should be rotated. +/// Must be less than half of [`DEFAULT_MAX_CERT_LIFETIME`]. +pub const DEFAULT_CA_CERT_LIFETIME: Duration = Duration::from_days_unchecked(365 * 2); + /// As the Pods will be evicted [`DEFAULT_CERT_RESTART_BUFFER`] before /// the cert actually expires, this results in a restart in approx every 2 weeks, /// which matches the rolling re-deploy of k8s nodes of e.g.: diff --git a/rust/operator-binary/src/crd.rs b/rust/operator-binary/src/crd.rs index 664fa9a9..14435d54 100644 --- a/rust/operator-binary/src/crd.rs +++ b/rust/operator-binary/src/crd.rs @@ -9,7 +9,7 @@ use stackable_operator::{ }; use stackable_secret_operator_crd_utils::SecretReference; -use crate::backend::tls::DEFAULT_MAX_CERT_LIFETIME; +use crate::backend; /// A [SecretClass](DOCS_BASE_URL_PLACEHOLDER/secret-operator/secretclass) is a cluster-global Kubernetes resource /// that defines a category of secrets that the Secret Operator knows how to provision. @@ -88,7 +88,7 @@ pub struct AutoTlsBackend { impl AutoTlsBackend { fn default_max_certificate_lifetime() -> Duration { - DEFAULT_MAX_CERT_LIFETIME + backend::tls::DEFAULT_MAX_CERT_LIFETIME } } @@ -117,7 +117,7 @@ pub struct AutoTlsCa { impl AutoTlsCa { fn default_ca_certificate_lifetime() -> Duration { - Duration::from_days_unchecked(365 * 2) + backend::tls::DEFAULT_CA_CERT_LIFETIME } } @@ -273,7 +273,7 @@ mod test { use super::*; use crate::{ - backend::tls::DEFAULT_MAX_CERT_LIFETIME, + backend::tls::{DEFAULT_CA_CERT_LIFETIME, DEFAULT_MAX_CERT_LIFETIME}, crd::{AutoTlsBackend, SecretClass, SecretClassSpec}, }; @@ -305,6 +305,7 @@ mod test { namespace: "default".to_string(), }, auto_generate: false, + ca_certificate_lifetime: DEFAULT_CA_CERT_LIFETIME, }, max_certificate_lifetime: DEFAULT_MAX_CERT_LIFETIME, }) @@ -324,6 +325,7 @@ mod test { name: secret-provisioner-tls-ca namespace: default autoGenerate: true + caCertificateLifetime: 100d maxCertificateLifetime: 31d "#; let deserializer = serde_yaml::Deserializer::from_str(input); @@ -339,6 +341,7 @@ mod test { namespace: "default".to_string(), }, auto_generate: true, + ca_certificate_lifetime: Duration::from_days_unchecked(100) }, max_certificate_lifetime: Duration::from_days_unchecked(31), }) From 48eac63d45b0c8140094e496a1c1b10bb6b8c61b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Thu, 1 Feb 2024 11:45:18 +0100 Subject: [PATCH 6/6] Fix broken doc link --- rust/operator-binary/src/backend/tls/ca.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/operator-binary/src/backend/tls/ca.rs b/rust/operator-binary/src/backend/tls/ca.rs index 42a502ed..40c1c2b4 100644 --- a/rust/operator-binary/src/backend/tls/ca.rs +++ b/rust/operator-binary/src/backend/tls/ca.rs @@ -151,7 +151,7 @@ pub struct Config { /// pods' truststores. /// /// Hence, this value _should_ be larger than the PKI's maximum certificate lifetime, - /// and smaller than [`Self::ca_lifetime`]. + /// and smaller than [`Self::ca_certificate_lifetime`]. pub rotate_if_ca_expires_before: Option, }