Allow AD samAccountName generation to be customized (#454)

* Add option to customize AD samAccountName generation

* CRD docs

* Update CRD

* Changelog

* Mark samAccountName customization as experimental

* Document samAccountName customization

* Clean up dead code

* Cleanup

* Remove unused callouts

* Cache randomness dictionaries

* Expand name generation docs

* Only apply experimental prefix in serde

CHANGELOG.md

@@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file.
 
 ## [Unreleased]
 
+### Added
+
+- Active Directory's `samAccountName` generation can now be customized ([#454]).
+
+[#454]: https://github.com/stackabletech/secret-operator/pull/454
+
 ## [24.7.0] - 2024-07-24
 
 ### Added

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

@@ -114,6 +114,24 @@ spec:
                             activeDirectory:
                               description: Credentials should be provisioned in a Microsoft Active Directory domain.
                               properties:
+                                experimentalGenerateSamAccountName:
+                                  description: Allows samAccountName generation for new accounts to be customized. Note that setting this field (even if empty) makes the Secret Operator take over the generation duty from the domain controller.
+                                  nullable: true
+                                  properties:
+                                    prefix:
+                                      default: ''
+                                      description: A prefix to be prepended to generated samAccountNames.
+                                      type: string
+                                    totalLength:
+                                      default: 20
+                                      description: |-
+                                        The total length of generated samAccountNames, _including_ `prefix`. Must be larger than the length of `prefix`, but at most `20`.
+
+                                        Note that this should be as large as possible, to minimize the risk of collisions.
+                                      format: uint8
+                                      minimum: 0.0
+                                      type: integer
+                                  type: object
                                 ldapServer:
                                   description: An AD LDAP server, such as the AD Domain Controller. This must match the server’s FQDN, or GSSAPI authentication will fail.
                                   type: string

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

@@ -122,6 +122,37 @@ If the same AD domain _is_ shared between multiple Kubernetes clusters, the foll
 - The Kubernetes Nodes' names and fully qualified domain names
 - The Kubernetes Namespaces' names (only Namespaces that use Kerberos)
 
+[#ad-samaccountname]
+===== Custom `samAccountName` generation
+
+IMPORTANT: `samAccountName` customization is an experimental preview feature, and may be changed or removed at any time.
+
+By default, the `samAccountName` for created Active Directory accounts will be generated by Active Directory.
+
+These can be customized if required, such as for compliance with internal policies. When customization is enabled,
+the name will follow the pattern `\{prefix}\{random}`, where `\{random}` is a sequence of random alphanumeric characters.
+The full name will be exactly `totalLength` characters.
+
+For example, the following configuration will generate samAccountNames such as "myprefix-abcd".
+
+[source,yaml]
+----
+spec:
+  backend:
+    kerberosKeytab:
+      admin:
+        activeDirectory:
+          experimentalGenerateSamAccountName:
+            prefix: myprefix-
+            totalLength: 13
+----
+
+`kerberosKeytab.admin.activeDirectory.experimentalGenerateSamAccountName.prefix`:: A prefix that will be prepended to all generated `samAccountName` values.
+`kerberosKeytab.admin.activeDirectory.experimentalGenerateSamAccountName.totalLength`:: The desired length of `samAccountName` values, _including_ `prefix`. Must not be larger than 20.
+
+NOTE: These options only affect _newly created_ accounts. Existing accounts will keep their respective old `samAccountName`.
+
+[#ad-reference]
 ==== Reference
 
 [source,yaml]

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

@@ -1,10 +1,15 @@
-use std::ffi::{CString, NulError};
+use std::{
+    collections::HashSet,
+    ffi::{CString, NulError},
+    sync::OnceLock,
+};
 
 use byteorder::{LittleEndian, WriteBytesExt};
 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_krb5_provision_keytab::ActiveDirectorySamAccountNameRules;
 use stackable_operator::{k8s_openapi::api::core::v1::Secret, kube::runtime::reflector::ObjectRef};
 use stackable_secret_operator_crd_utils::SecretReference;
 
@@ -61,6 +66,9 @@ pub enum Error {
 
     #[snafu(display("failed to add key to keytab"))]
     AddToKeytab { source: krb5::Error },
+
+    #[snafu(display("configured samAccountName prefix is longer than the requested length"))]
+    SamAccountNamePrefixLongerThanRequestedLength,
 }
 pub type Result<T, E = Error> = std::result::Result<T, E>;
 
@@ -79,6 +87,7 @@ pub struct AdAdmin<'a> {
     password_cache: CredentialCache,
     user_distinguished_name: String,
     schema_distinguished_name: String,
+    generate_sam_account_name: Option<ActiveDirectorySamAccountNameRules>,
 }
 
 impl<'a> AdAdmin<'a> {
@@ -89,6 +98,7 @@ impl<'a> AdAdmin<'a> {
         password_cache_secret: SecretReference,
         user_distinguished_name: String,
         schema_distinguished_name: String,
+        generate_sam_account_name: Option<ActiveDirectorySamAccountNameRules>,
     ) -> Result<AdAdmin<'a>> {
         let kube = stackable_operator::client::create_client(None)
             .await
@@ -117,6 +127,7 @@ impl<'a> AdAdmin<'a> {
             password_cache,
             user_distinguished_name,
             schema_distinguished_name,
+            generate_sam_account_name,
         })
     }
 
@@ -143,6 +154,7 @@ impl<'a> AdAdmin<'a> {
                     &self.user_distinguished_name,
                     &self.schema_distinguished_name,
                     ctx.cache_ref,
+                    self.generate_sam_account_name.as_ref(),
                 )
                 .await?;
                 Ok(password.into_bytes())
@@ -186,21 +198,40 @@ async fn get_ldap_ca_certificate(
     native_tls::Certificate::from_pem(&ca_cert_pem).context(ParseLdapTlsCaSnafu)
 }
 
-fn generate_ad_password(len: usize) -> String {
+fn generate_random_string(len: usize, dict: &[char]) -> String {
     let mut rng = thread_rng();
     // Assert that `rng` is crypto-safe
     let _: &dyn CryptoRng = &rng;
-    // Allow all ASCII alphanumeric characters as well as punctuation
-    // Exclude double quotes (") since they are used by the AD password update protocol...
-    let dict: Vec<char> = (1..=127)
-        .filter_map(char::from_u32)
-        .filter(|c| *c != '"' && (c.is_ascii_alphanumeric() || c.is_ascii_punctuation()))
-        .collect();
-    let pw = (0..len)
+    let str = (0..len)
         .map(|_| *dict.choose(&mut rng).expect("dictionary must be non-empty"))
         .collect::<String>();
-    assert_eq!(pw.len(), len);
-    pw
+    assert_eq!(str.len(), len);
+    str
+}
+
+fn generate_ad_password(len: usize) -> String {
+    // Allow all ASCII alphanumeric characters as well as punctuation
+    // Exclude double quotes (") since they are used by the AD password update protocol...
+    static DICT: OnceLock<Vec<char>> = OnceLock::new();
+    let dict = DICT.get_or_init(|| {
+        (1..=127)
+            .filter_map(char::from_u32)
+            .filter(|c| *c != '"' && (c.is_ascii_alphanumeric() || c.is_ascii_punctuation()))
+            .collect()
+    });
+    generate_random_string(len, dict)
+}
+
+fn generate_username(len: usize) -> String {
+    // Allow ASCII alphanumerics
+    static DICT: OnceLock<Vec<char>> = OnceLock::new();
+    let dict = DICT.get_or_init(|| {
+        (1..=127)
+            .filter_map(char::from_u32)
+            .filter(|c| c.is_ascii_alphanumeric())
+            .collect()
+    });
+    generate_random_string(len, dict)
 }
 
 fn encode_password_for_ad_update(password: &str) -> Vec<u8> {
@@ -220,6 +251,7 @@ async fn create_ad_user(
     user_dn_base: &str,
     schema_dn_base: &str,
     password_cache_ref: SecretReference,
+    generate_sam_account_name: Option<&ActiveDirectorySamAccountNameRules>,
 ) -> 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;
@@ -242,10 +274,20 @@ async fn create_ad_user(
     let principal_cn = ldap3::dn_escape(&princ_name);
     // FIXME: AD restricts RDNs to 64 characters
     let principal_cn = principal_cn.get(..64).unwrap_or(&*principal_cn);
+    let sam_account_name = generate_sam_account_name
+        .map(|sam_rules| {
+            let mut name = sam_rules.prefix.clone();
+            let random_part_len = usize::from(sam_rules.total_length)
+                .checked_sub(name.len())
+                .context(SamAccountNamePrefixLongerThanRequestedLengthSnafu)?;
+            name += &generate_username(random_part_len);
+            Ok(name)
+        })
+        .transpose()?;
     let create_user_result = ldap
         .add(
             &format!("CN={principal_cn},{user_dn_base}"),
-            vec![
+            [
                 ("cn".as_bytes(), [principal_cn.as_bytes()].into()),
                 ("objectClass".as_bytes(), ["user".as_bytes()].into()),
                 ("instanceType".as_bytes(), ["4".as_bytes()].into()),
@@ -280,7 +322,14 @@ async fn create_ad_user(
                         .as_bytes()]
                     .into(),
                 ),
-            ],
+            ]
+            .into_iter()
+            .chain(
+                sam_account_name
+                    .as_ref()
+                    .map(|san| ("samAccountName".as_bytes(), HashSet::from([san.as_bytes()]))),
+            )
+            .collect(),
         )
         .await
         .context(CreateLdapUserSnafu)?;

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

@@ -31,8 +31,14 @@ pub enum AdminBackend {
         password_cache_secret: SecretReference,
         user_distinguished_name: String,
         schema_distinguished_name: String,
+        generate_sam_account_name: Option<ActiveDirectorySamAccountNameRules>,
     },
 }
+#[derive(Serialize, Deserialize, Debug)]
+pub struct ActiveDirectorySamAccountNameRules {
+    pub prefix: String,
+    pub total_length: u8,
+}
 
 #[derive(Serialize, Deserialize)]
 pub struct Response {}

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

@@ -94,6 +94,7 @@ async fn run() -> Result<Response, Error> {
             password_cache_secret,
             user_distinguished_name,
             schema_distinguished_name,
+            generate_sam_account_name,
         } => AdminConnection::ActiveDirectory(
             active_directory::AdAdmin::connect(
                 &ldap_server,
@@ -102,6 +103,7 @@ async fn run() -> Result<Response, Error> {
                 password_cache_secret,
                 user_distinguished_name,
                 schema_distinguished_name,
+                generate_sam_account_name,
             )
             .await
             .context(ActiveDirectoryInitSnafu)?,

rust/operator-binary/src/backend/kerberos_keytab.rs

@@ -1,6 +1,10 @@
 use async_trait::async_trait;
 use snafu::{OptionExt, ResultExt, Snafu};
-use stackable_krb5_provision_keytab::provision_keytab;
+use stackable_krb5_provision_keytab::{
+    // Some qualified paths get long enough to break rustfmt, alias the crate name to work around that
+    self as provision,
+    provision_keytab,
+};
 use stackable_operator::{k8s_openapi::api::core::v1::Secret, kube::runtime::reflector::ObjectRef};
 use stackable_secret_operator_crd_utils::SecretReference;
 use tempfile::tempdir;
@@ -10,7 +14,10 @@ use tokio::{
 };
 
 use crate::{
-    crd::{Hostname, InvalidKerberosPrincipal, KerberosKeytabBackendAdmin, KerberosPrincipal},
+    crd::{
+        ActiveDirectorySamAccountNameRules, Hostname, InvalidKerberosPrincipal,
+        KerberosKeytabBackendAdmin, KerberosPrincipal,
+    },
     format::{well_known, SecretData, WellKnownSecretData},
     utils::Unloggable,
 };
@@ -229,12 +236,24 @@ cluster.local = {realm_name}
                         password_cache_secret,
                         user_distinguished_name,
                         schema_distinguished_name,
+                        generate_sam_account_name,
                     } => stackable_krb5_provision_keytab::AdminBackend::ActiveDirectory {
                         ldap_server: ldap_server.to_string(),
                         ldap_tls_ca_secret: ldap_tls_ca_secret.clone(),
                         password_cache_secret: password_cache_secret.clone(),
                         user_distinguished_name: user_distinguished_name.clone(),
                         schema_distinguished_name: schema_distinguished_name.clone(),
+                        generate_sam_account_name: generate_sam_account_name.clone().map(
+                            |ActiveDirectorySamAccountNameRules {
+                                 prefix,
+                                 total_length,
+                             }| {
+                                provision::ActiveDirectorySamAccountNameRules {
+                                    prefix,
+                                    total_length,
+                                }
+                            },
+                        ),
                     },
                 },
             },

rust/operator-binary/src/crd.rs

@@ -175,9 +175,36 @@ pub enum KerberosKeytabBackendAdmin {
         /// The root Distinguished Name (DN) for AD-managed schemas,
         /// typically `CN=Schema,CN=Configuration,{domain_dn}`.
         schema_distinguished_name: String,
+
+        /// Allows samAccountName generation for new accounts to be customized.
+        /// Note that setting this field (even if empty) makes the Secret Operator take
+        /// over the generation duty from the domain controller.
+        #[serde(rename = "experimentalGenerateSamAccountName")]
+        generate_sam_account_name: Option<ActiveDirectorySamAccountNameRules>,
     },
 }
 
+#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
+#[serde(rename_all = "camelCase")]
+pub struct ActiveDirectorySamAccountNameRules {
+    /// A prefix to be prepended to generated samAccountNames.
+    #[serde(default)]
+    pub prefix: String,
+    /// The total length of generated samAccountNames, _including_ `prefix`.
+    /// Must be larger than the length of `prefix`, but at most `20`.
+    ///
+    /// Note that this should be as large as possible, to minimize the risk of collisions.
+    #[serde(default = "ActiveDirectorySamAccountNameRules::default_total_length")]
+    pub total_length: u8,
+}
+
+impl ActiveDirectorySamAccountNameRules {
+    fn default_total_length() -> u8 {
+        // Default AD samAccountName length limit
+        20
+    }
+}
+
 #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
 #[serde(try_from = "String", into = "String")]
 pub struct Hostname(String);

tests/templates/kuttl/kerberos-ad/secretclass.yaml → tests/templates/kuttl/kerberos-ad/secretclass.yaml.j2

@@ -23,6 +23,11 @@ spec:
           # Subfolder must be created manually, of type "msDS-ShadowPrincipalContainer"
           userDistinguishedName: CN=Stackable,CN=Users,DC=sble,DC=test
           schemaDistinguishedName: CN=Schema,CN=Configuration,DC=sble,DC=test
+{% if test_scenario['values']['ad-custom-samaccountname'] == 'true' %}
+          experimentalGenerateSamAccountName:
+            prefix: sble-
+            totalLength: 15
+{% endif %}
       adminKeytabSecret:
         # Created by AD administrator
         name: secret-operator-keytab

tests/test-definition.yaml

@@ -6,6 +6,10 @@ dimensions:
   - name: openshift
     values:
       - "false"
+  - name: ad-custom-samaccountname
+    values:
+      - "false"
+      - "true"
 tests:
   - name: kerberos
     dimensions:
@@ -15,6 +19,7 @@ tests:
     # - name: kerberos-ad
     #   dimensions:
     #     - krb5
+    #     - ad-custom-samaccountname
   - name: listener
     dimensions:
       - openshift