Rotate AutoTLS CAs (#350)

* Fix doccomment to be more neutral towards self-signed CAs

* Turn tls backend into a directory module

* Factor out CA management

* Rotate CA if about to expire

* Reorganize and document/test

* Rename ca::Manager::all_cas -> trust_roots

* Refactor internal naming

* Sort CAs by age to avoid spurious writes

* Docs

* Changelog

* Update CRD

* Reshuffle logging

* rustfmt

* Fix broken doclink

* Update rust/operator-binary/src/utils.rs

Co-authored-by: Sebastian Bernauer <[email protected]>

* Enforce secret reference correctness in CRD

* Check for time overflow

* Make cargo-deny happy

* Add note about cleaning up CAs

---------

Co-authored-by: Sebastian Bernauer <[email protected]>

Author: nightkr

CHANGELOG.md

@@ -10,10 +10,13 @@ All notable changes to this project will be documented in this file.
 
 ## Changed
 
-- Use new annotation builder ([#341])
+- 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.
 
 [#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
 
 ## [23.11.0] - 2023-11-24
 

Cargo.lock

@@ -1,5 +1,5 @@
 [workspace]
-members = ["rust/operator-binary", "rust/krb5", "rust/krb5-provision-keytab", "rust/krb5-sys"]
+members = ["rust/*"]
 default-members = ["rust/operator-binary"]
 resolver = "2"
 

Cargo.toml

@@ -45,17 +45,20 @@ spec:
                           properties:
                             autoGenerate:
                               default: false
-                              description: Whether a new certificate authority should be generated if it does not already exist.
+                              description: Whether the certificate authority should be managed by Secret Operator, including being generated if it does not already exist.
                               type: boolean
                             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:
                                 name:
-                                  description: name is unique within a namespace to reference a secret resource.
+                                  description: Name of the Secret being referred to.
                                   type: string
                                 namespace:
-                                  description: namespace defines the space within which the secret name must be unique.
+                                  description: Namespace of the Secret being referred to.
                                   type: string
+                              required:
+                                - name
+                                - namespace
                               type: object
                           required:
                             - secret
@@ -109,21 +112,27 @@ spec:
                                   description: Reference (name and namespace) to a Kubernetes Secret object containing the TLS CA (in `ca.crt`) that the LDAP server’s certificate should be authenticated against.
                                   properties:
                                     name:
-                                      description: name is unique within a namespace to reference a secret resource.
+                                      description: Name of the Secret being referred to.
                                       type: string
                                     namespace:
-                                      description: namespace defines the space within which the secret name must be unique.
+                                      description: Namespace of the Secret being referred to.
                                       type: string
+                                  required:
+                                    - name
+                                    - namespace
                                   type: object
                                 passwordCacheSecret:
                                   description: Reference (name and namespace) to a Kubernetes Secret object where workload passwords will be stored. This must not be accessible to end users.
                                   properties:
                                     name:
-                                      description: name is unique within a namespace to reference a secret resource.
+                                      description: Name of the Secret being referred to.
                                       type: string
                                     namespace:
-                                      description: namespace defines the space within which the secret name must be unique.
+                                      description: Namespace of the Secret being referred to.
                                       type: string
+                                  required:
+                                    - name
+                                    - namespace
                                   type: object
                                 schemaDistinguishedName:
                                   description: The root Distinguished Name (DN) for AD-managed schemas, typically `CN=Schema,CN=Configuration,{domain_dn}`.
@@ -152,11 +161,14 @@ spec:
                           description: Reference (`name` and `namespace`) to a K8s Secret object where a keytab with administrative privileges is stored in the key `keytab`.
                           properties:
                             name:
-                              description: name is unique within a namespace to reference a secret resource.
+                              description: Name of the Secret being referred to.
                               type: string
                             namespace:
-                              description: namespace defines the space within which the secret name must be unique.
+                              description: Namespace of the Secret being referred to.
                               type: string
+                          required:
+                            - name
+                            - namespace
                           type: object
                         adminPrincipal:
                           description: The admin principal.

deploy/helm/secret-operator/crds/crds.yaml

@@ -15,6 +15,7 @@ rules:
       - watch
       - create
       - patch
+      - update
   - apiGroups:
       - ""
     resources:

deploy/helm/secret-operator/templates/roles.yaml

@@ -56,7 +56,18 @@ We have spent a considerate amount of time thinking about this issue and decided
 Most of our product operators will not set any specific certificate lifetime, so the default applies.
 In case an operator sets a higher lifetime, a tracking issue must be created to document and track the steps to reduce the certificate lifetime.
 
-Users can use podOverrides to extend the certificate lifetime by adding volume annotations. We might add native support to customize certificate lifetimes in the future by using the product CRDs.
+Users can use podOverrides to extend the certificate lifetime by adding volume annotations. We might add native support for customizing certificate lifetimes in the future to the Stacklet CRDs.
+
+==== Certificate Authority rotation
+
+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.
+
+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.
+
+Expired certificates will currently not be deleted automatically, and should be cleaned up manually.
 
 ==== Reference
 
@@ -77,7 +88,7 @@ spec:
 `autoTls.ca`:: Configures the certificate authority used to issue `Pod` certificates.
 `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 if it can not be found.
+`autoTls.ca.autoGenerate`:: Whether the certificate authority should be provisioned and managed by the Secret Operator.
 `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]

docs/modules/secret-operator/pages/secretclass.adoc

@@ -0,0 +1,14 @@
+[package]
+name = "stackable-secret-operator-crd-utils"
+version.workspace = true
+authors.workspace = true
+license.workspace = true
+edition.workspace = true
+repository.workspace = true
+publish = false
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]
+serde.workspace = true
+stackable-operator.workspace = true

rust/crd-utils/Cargo.toml

@@ -0,0 +1,37 @@
+//! CRD types that are shared between secret-operator components, but aren't clearly owned by one of them.
+
+use std::fmt::Display;
+
+use serde::{Deserialize, Serialize};
+use stackable_operator::{
+    k8s_openapi::api::core::v1::Secret,
+    kube::runtime::reflector::ObjectRef,
+    schemars::{self, JsonSchema},
+};
+
+// Redefine SecretReference instead of reusing k8s-openapi's, in order to make name/namespace mandatory.
+#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
+#[serde(rename_all = "camelCase")]
+pub struct SecretReference {
+    /// Namespace of the Secret being referred to.
+    pub namespace: String,
+    /// Name of the Secret being referred to.
+    pub name: String,
+}
+
+// Use ObjectRef for logging/errors
+impl Display for SecretReference {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        ObjectRef::<Secret>::from(self).fmt(f)
+    }
+}
+impl From<SecretReference> for ObjectRef<Secret> {
+    fn from(val: SecretReference) -> Self {
+        ObjectRef::<Secret>::from(&val)
+    }
+}
+impl From<&SecretReference> for ObjectRef<Secret> {
+    fn from(val: &SecretReference) -> Self {
+        ObjectRef::<Secret>::new(&val.name).within(&val.namespace)
+    }
+}

rust/crd-utils/src/lib.rs

@@ -10,6 +10,7 @@ publish = false
 
 [dependencies]
 krb5 = { path = "../krb5" }
+stackable-secret-operator-crd-utils = { path = "../crd-utils" }
 
 byteorder.workspace = true
 futures.workspace = true

rust/krb5-provision-keytab/Cargo.toml

@@ -5,22 +5,17 @@ use krb5::{Keyblock, Keytab, KrbContext, Principal, PrincipalUnparseOptions};
 use ldap3::{Ldap, LdapConnAsync, LdapConnSettings};
 use rand::{seq::SliceRandom, thread_rng, CryptoRng};
 use snafu::{OptionExt, ResultExt, Snafu};
-use stackable_operator::k8s_openapi::api::core::v1::{Secret, SecretReference};
+use stackable_operator::{k8s_openapi::api::core::v1::Secret, kube::runtime::reflector::ObjectRef};
+use stackable_secret_operator_crd_utils::SecretReference;
 
-use crate::{
-    credential_cache::{self, CredentialCache},
-    secret_ref::{FullSecretRef, IncompleteSecretRef},
-};
+use crate::credential_cache::{self, CredentialCache};
 
 #[derive(Debug, Snafu)]
 pub enum Error {
-    #[snafu(display("LDAP TLS CA reference is invalid"))]
-    LdapTlsCaReferenceInvalid { source: IncompleteSecretRef },
-
     #[snafu(display("failed to retrieve LDAP TLS CA {ca_ref}"))]
     GetLdapTlsCa {
         source: stackable_operator::error::Error,
-        ca_ref: FullSecretRef,
+        ca_ref: ObjectRef<Secret>,
     },
 
     #[snafu(display("LDAP TLS CA secret is missing required key {key}"))]
@@ -29,9 +24,6 @@ pub enum Error {
     #[snafu(display("failed to parse LDAP TLS CA"))]
     ParseLdapTlsCa { source: native_tls::Error },
 
-    #[snafu(display("password cache reference is invalid"))]
-    PasswordCacheReferenceInvalid { source: IncompleteSecretRef },
-
     #[snafu(display("password cache error"))]
     PasswordCache { source: credential_cache::Error },
 
@@ -55,13 +47,13 @@ pub enum Error {
     #[snafu(display("failed to get password cache {password_cache_ref}"))]
     GetPasswordCache {
         source: stackable_operator::error::Error,
-        password_cache_ref: FullSecretRef,
+        password_cache_ref: ObjectRef<Secret>,
     },
 
     #[snafu(display("failed to update password cache {password_cache_ref}"))]
     UpdatePasswordCache {
         source: stackable_operator::error::Error,
-        password_cache_ref: FullSecretRef,
+        password_cache_ref: ObjectRef<Secret>,
     },
 
     #[snafu(display("failed to create LDAP user"))]
@@ -72,7 +64,7 @@ pub enum Error {
     ))]
     CreateLdapUserConflict {
         source: ldap3::LdapError,
-        password_cache_ref: FullSecretRef,
+        password_cache_ref: ObjectRef<Secret>,
     },
 
     #[snafu(display("failed to decode generated password"))]
@@ -121,15 +113,9 @@ impl<'a> AdAdmin<'a> {
         ldap.sasl_gssapi_bind(ldap_server)
             .await
             .context(LdapAuthnSnafu)?;
-        let password_cache = CredentialCache::new(
-            "AD passwords",
-            kube,
-            password_cache_secret
-                .try_into()
-                .context(PasswordCacheReferenceInvalidSnafu)?,
-        )
-        .await
-        .context(PasswordCacheSnafu)?;
+        let password_cache = CredentialCache::new("AD passwords", kube, password_cache_secret)
+            .await
+            .context(PasswordCacheSnafu)?;
         Ok(Self {
             ldap,
             krb,
@@ -190,9 +176,6 @@ async fn get_ldap_ca_certificate(
     kube: &stackable_operator::client::Client,
     ca_secret_ref: SecretReference,
 ) -> Result<native_tls::Certificate> {
-    let ca_secret_ref: FullSecretRef = ca_secret_ref
-        .try_into()
-        .context(LdapTlsCaReferenceInvalidSnafu)?;
     let ca_secret = kube
         .get::<Secret>(&ca_secret_ref.name, &ca_secret_ref.namespace)
         .await
@@ -241,7 +224,7 @@ async fn create_ad_user(
     password: &str,
     user_dn_base: &str,
     schema_dn_base: &str,
-    password_cache_ref: FullSecretRef,
+    password_cache_ref: SecretReference,
 ) -> Result<()> {
     // Flags are a subset of https://learn.microsoft.com/en-us/troubleshoot/windows-server/identity/useraccountcontrol-manipulate-account-properties
     const AD_UAC_NORMAL_ACCOUNT: u32 = 0x0200;

rust/krb5-provision-keytab/src/active_directory.rs

@@ -1,44 +1,46 @@
 use futures::{TryFuture, TryFutureExt};
 use snafu::{OptionExt, ResultExt, Snafu};
-use stackable_operator::k8s_openapi::{api::core::v1::Secret, ByteString};
-
-use crate::secret_ref::FullSecretRef;
+use stackable_operator::{
+    k8s_openapi::{api::core::v1::Secret, ByteString},
+    kube::runtime::reflector::ObjectRef,
+};
+use stackable_secret_operator_crd_utils::SecretReference;
 
 #[derive(Debug, Snafu)]
 pub enum Error {
     #[snafu(display("failed to load initial cache from {cache_ref}"))]
     GetInitialCache {
         source: stackable_operator::error::Error,
-        cache_ref: FullSecretRef,
+        cache_ref: ObjectRef<Secret>,
     },
 
     #[snafu(display("failed to save credential {key} to {cache_ref}"))]
     SaveToCache {
         source: stackable_operator::error::Error,
         key: String,
-        cache_ref: FullSecretRef,
+        cache_ref: ObjectRef<Secret>,
     },
 
     #[snafu(display("newly saved credential {key} was not found in {cache_ref}"))]
     SavedKeyNotFound {
         key: String,
-        cache_ref: FullSecretRef,
+        cache_ref: ObjectRef<Secret>,
     },
 }
 type Result<T, E = Error> = std::result::Result<T, E>;
 
 pub struct CredentialCache {
     name: &'static str,
     kube: stackable_operator::client::Client,
-    cache_ref: FullSecretRef,
+    cache_ref: SecretReference,
     current_state: Secret,
 }
 impl CredentialCache {
     #[tracing::instrument(skip(kube))]
     pub async fn new(
         name: &'static str,
         kube: stackable_operator::client::Client,
-        cache_ref: FullSecretRef,
+        cache_ref: SecretReference,
     ) -> Result<Self> {
         Ok(Self {
             name,
@@ -127,5 +129,5 @@ impl CredentialCache {
 
 /// Information that may be useful for generating error messages in get_or_insert handlers
 pub struct Ctx {
-    pub cache_ref: FullSecretRef,
+    pub cache_ref: SecretReference,
 }