dsdb: Allow special chars like "@" in samAccountName when generating the salt
authorAndrew Bartlett <abartlet@samba.org>
Tue, 19 Oct 2021 03:01:36 +0000 (16:01 +1300)
committerStefan Metzmacher <metze@samba.org>
Wed, 20 Oct 2021 12:54:54 +0000 (12:54 +0000)
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14874

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
Autobuild-User(master): Stefan Metzmacher <metze@samba.org>
Autobuild-Date(master): Wed Oct 20 12:54:54 UTC 2021 on sn-devel-184

auth/credentials/credentials_krb5.c
lib/krb5_wrap/krb5_samba.c
lib/krb5_wrap/krb5_samba.h
selftest/knownfail.d/kdc-salt
source3/passdb/machine_account_secrets.c
source4/dsdb/samdb/ldb_modules/password_hash.c

index c03d80ac440ddad2cf385a52382c1d2ae286d31d..d2e7a76a69e4af2ea42e1830d0649ee2220bef89 100644 (file)
@@ -1200,12 +1200,12 @@ _PUBLIC_ int cli_credentials_get_keytab(struct cli_credentials *cred,
                break;
        }
 
-       ret = smb_krb5_salt_principal(realm,
-                                     username, /* sAMAccountName */
-                                     upn, /* userPrincipalName */
-                                     uac_flags,
-                                     mem_ctx,
-                                     &salt_principal);
+       ret = smb_krb5_salt_principal_str(realm,
+                                         username, /* sAMAccountName */
+                                         upn, /* userPrincipalName */
+                                         uac_flags,
+                                         mem_ctx,
+                                         &salt_principal);
        if (ret) {
                talloc_free(mem_ctx);
                return ret;
index 20ce86c708dc27282aeaf8166fbcd91f953ae1d0..63a6e951f80dcc1d3d27fa086a71944861d9dee3 100644 (file)
@@ -456,19 +456,20 @@ int smb_krb5_get_pw_salt(krb5_context context,
  *
  * @see smb_krb5_salt_principal2data
  */
-int smb_krb5_salt_principal(const char *realm,
+int smb_krb5_salt_principal(krb5_context krb5_ctx,
+                           const char *realm,
                            const char *sAMAccountName,
                            const char *userPrincipalName,
                            uint32_t uac_flags,
-                           TALLOC_CTX *mem_ctx,
-                           char **_salt_principal)
+                           krb5_principal *salt_princ)
 {
        TALLOC_CTX *frame = talloc_stackframe();
        char *upper_realm = NULL;
        const char *principal = NULL;
        int principal_len = 0;
+       krb5_error_code krb5_ret;
 
-       *_salt_principal = NULL;
+       *salt_princ = NULL;
 
        if (sAMAccountName == NULL) {
                TALLOC_FREE(frame);
@@ -512,7 +513,6 @@ int smb_krb5_salt_principal(const char *realm,
         */
        if (uac_flags & UF_TRUST_ACCOUNT_MASK) {
                int computer_len = 0;
-               char *tmp = NULL;
 
                computer_len = strlen(sAMAccountName);
                if (sAMAccountName[computer_len-1] == '$') {
@@ -520,60 +520,186 @@ int smb_krb5_salt_principal(const char *realm,
                }
 
                if (uac_flags & UF_INTERDOMAIN_TRUST_ACCOUNT) {
-                       principal = talloc_asprintf(frame, "krbtgt/%*.*s",
-                                                   computer_len, computer_len,
-                                                   sAMAccountName);
-                       if (principal == NULL) {
+                       const char *krbtgt = "krbtgt";
+                       krb5_ret = krb5_build_principal_ext(krb5_ctx,
+                                                           salt_princ,
+                                                           strlen(upper_realm),
+                                                           upper_realm,
+                                                           strlen(krbtgt),
+                                                           krbtgt,
+                                                           computer_len,
+                                                           sAMAccountName,
+                                                           0);
+                       if (krb5_ret != 0) {
                                TALLOC_FREE(frame);
-                               return ENOMEM;
+                               return krb5_ret;
                        }
                } else {
-
-                       tmp = talloc_asprintf(frame, "host/%*.*s.%s",
-                                             computer_len, computer_len,
-                                             sAMAccountName, realm);
+                       const char *host = "host";
+                       char *tmp = NULL;
+                       char *tmp_lower = NULL;
+
+                       tmp = talloc_asprintf(frame, "%*.*s.%s",
+                                             computer_len,
+                                             computer_len,
+                                             sAMAccountName,
+                                             realm);
                        if (tmp == NULL) {
                                TALLOC_FREE(frame);
                                return ENOMEM;
                        }
 
-                       principal = strlower_talloc(frame, tmp);
-                       TALLOC_FREE(tmp);
-                       if (principal == NULL) {
+                       tmp_lower = strlower_talloc(frame, tmp);
+                       if (tmp_lower == NULL) {
                                TALLOC_FREE(frame);
                                return ENOMEM;
                        }
-               }
 
-               principal_len = strlen(principal);
+                       krb5_ret = krb5_build_principal_ext(krb5_ctx,
+                                                           salt_princ,
+                                                           strlen(upper_realm),
+                                                           upper_realm,
+                                                           strlen(host),
+                                                           host,
+                                                           strlen(tmp_lower),
+                                                           tmp_lower,
+                                                           0);
+                       if (krb5_ret != 0) {
+                               TALLOC_FREE(frame);
+                               return krb5_ret;
+                       }
+               }
 
        } else if (userPrincipalName != NULL) {
-               char *p;
+               /*
+                * We parse the name not only to allow an easy
+                * replacement of the realm (no matter the realm in
+                * the UPN, the salt comes from the upper-case real
+                * realm, but also to correctly provide a salt when
+                * the UPN is host/foo.bar
+                *
+                * This can fail for a UPN of the form foo@bar@REALM
+                * (which is accepted by windows) however.
+                */
+               krb5_ret = krb5_parse_name(krb5_ctx,
+                                          userPrincipalName,
+                                          salt_princ);
 
-               principal = userPrincipalName;
-               p = strchr(principal, '@');
-               if (p != NULL) {
-                       principal_len = PTR_DIFF(p, principal);
-               } else {
-                       principal_len = strlen(principal);
+               if (krb5_ret != 0) {
+                       TALLOC_FREE(frame);
+                       return krb5_ret;
+               }
+
+               /*
+                * No matter what realm (including none) in the UPN,
+                * the realm is replaced with our upper-case realm
+                */
+               smb_krb5_principal_set_realm(krb5_ctx,
+                                            *salt_princ,
+                                            upper_realm);
+               if (krb5_ret != 0) {
+                       krb5_free_principal(krb5_ctx, *salt_princ);
+                       TALLOC_FREE(frame);
+                       return krb5_ret;
                }
        } else {
                principal = sAMAccountName;
                principal_len = strlen(principal);
-       }
 
-       *_salt_principal = talloc_asprintf(mem_ctx, "%*.*s@%s",
-                                          principal_len, principal_len,
-                                          principal, upper_realm);
-       if (*_salt_principal == NULL) {
-               TALLOC_FREE(frame);
-               return ENOMEM;
+               krb5_ret = krb5_build_principal_ext(krb5_ctx,
+                                                   salt_princ,
+                                                   strlen(upper_realm),
+                                                   upper_realm,
+                                                   principal_len,
+                                                   principal,
+                                                   0);
+               if (krb5_ret != 0) {
+                       TALLOC_FREE(frame);
+                       return krb5_ret;
+               }
        }
 
        TALLOC_FREE(frame);
        return 0;
 }
 
+/**
+ * @brief This constructs the salt principal used by active directory
+ *
+ * Most Kerberos encryption types require a salt in order to
+ * calculate the long term private key for user/computer object
+ * based on a password.
+ *
+ * The returned _salt_principal is a string in forms like this:
+ * - host/somehost.example.com@EXAMPLE.COM
+ * - SomeAccount@EXAMPLE.COM
+ * - SomePrincipal@EXAMPLE.COM
+ *
+ * This is not the form that's used as salt, it's just
+ * the human readable form. It needs to be converted by
+ * smb_krb5_salt_principal2data().
+ *
+ * @param[in]  realm              The realm the user/computer is added too.
+ *
+ * @param[in]  sAMAccountName     The sAMAccountName attribute of the object.
+ *
+ * @param[in]  userPrincipalName  The userPrincipalName attribute of the object
+ *                                or NULL is not available.
+ *
+ * @param[in]  uac_flags          UF_ACCOUNT_TYPE_MASKed userAccountControl field
+ *
+ * @param[in]  mem_ctx            The TALLOC_CTX to allocate _salt_principal.
+ *
+ * @param[out]  _salt_principal   The resulting principal as string.
+ *
+ * @retval 0 Success; otherwise - Kerberos error codes
+ *
+ * @see smb_krb5_salt_principal2data
+ */
+int smb_krb5_salt_principal_str(const char *realm,
+                               const char *sAMAccountName,
+                               const char *userPrincipalName,
+                               uint32_t uac_flags,
+                               TALLOC_CTX *mem_ctx,
+                               char **_salt_principal_str)
+{
+       krb5_principal salt_principal = NULL;
+       char *salt_principal_malloc;
+       krb5_context krb5_ctx;
+       krb5_error_code krb5_ret
+               = smb_krb5_init_context_common(&krb5_ctx);
+       if (krb5_ret != 0) {
+               DBG_ERR("kerberos init context failed (%s)\n",
+                       error_message(krb5_ret));
+               return krb5_ret;
+       }
+
+       krb5_ret = smb_krb5_salt_principal(krb5_ctx,
+                                          realm,
+                                          sAMAccountName,
+                                          userPrincipalName,
+                                          uac_flags,
+                                          &salt_principal);
+
+       krb5_ret = krb5_unparse_name(krb5_ctx, salt_principal,
+                                    &salt_principal_malloc);
+       if (krb5_ret != 0) {
+               krb5_free_principal(krb5_ctx, salt_principal);
+               DBG_ERR("kerberos unparse of salt principal failed (%s)\n",
+                       error_message(krb5_ret));
+               return krb5_ret;
+       }
+       krb5_free_principal(krb5_ctx, salt_principal);
+       *_salt_principal_str
+               = talloc_strdup(mem_ctx, salt_principal_malloc);
+       krb5_free_unparsed_name(krb5_ctx, salt_principal_malloc);
+
+       if (*_salt_principal_str == NULL) {
+               return ENOMEM;
+       }
+       return 0;
+}
+
 /**
  * @brief Converts the salt principal string into the salt data blob
  *
index 01a9806b67060c45b741927469aef197fd18d335..a66b7465530ae44c0357da88777145d491c59196 100644 (file)
@@ -340,12 +340,19 @@ krb5_error_code ms_suptypes_to_ietf_enctypes(TALLOC_CTX *mem_ctx,
 int smb_krb5_get_pw_salt(krb5_context context,
                         krb5_const_principal host_princ,
                         krb5_data *psalt);
-int smb_krb5_salt_principal(const char *realm,
+int smb_krb5_salt_principal(krb5_context krb5_ctx,
+                           const char *realm,
                            const char *sAMAccountName,
                            const char *userPrincipalName,
                            uint32_t uac_flags,
-                           TALLOC_CTX *mem_ctx,
-                           char **_salt_principal);
+                           krb5_principal *salt_princ);
+
+int smb_krb5_salt_principal_str(const char *realm,
+                               const char *sAMAccountName,
+                               const char *userPrincipalName,
+                               uint32_t uac_flags,
+                               TALLOC_CTX *mem_ctx,
+                               char **_salt_principal);
 int smb_krb5_salt_principal2data(krb5_context context,
                                 const char *salt_principal,
                                 TALLOC_CTX *mem_ctx,
index 1a4ecd4462499c1c7ab13a37ecc80e3f978e57d5..a671e4d93eb18230fb8d40b48a4b926506dc656f 100644 (file)
@@ -1,12 +1 @@
-^samba.tests.krb5.salt_tests.samba.tests.krb5.salt_tests.SaltTests.test_salt_at_case_mac
-^samba.tests.krb5.salt_tests.samba.tests.krb5.salt_tests.SaltTests.test_salt_at_case_user
-^samba.tests.krb5.salt_tests.samba.tests.krb5.salt_tests.SaltTests.test_salt_at_end_mac
-^samba.tests.krb5.salt_tests.samba.tests.krb5.salt_tests.SaltTests.test_salt_at_end_no_dollar_mac
-^samba.tests.krb5.salt_tests.samba.tests.krb5.salt_tests.SaltTests.test_salt_at_end_user
-^samba.tests.krb5.salt_tests.samba.tests.krb5.salt_tests.SaltTests.test_salt_at_mac
-^samba.tests.krb5.salt_tests.samba.tests.krb5.salt_tests.SaltTests.test_salt_at_start_mac
-^samba.tests.krb5.salt_tests.samba.tests.krb5.salt_tests.SaltTests.test_salt_at_start_user
-^samba.tests.krb5.salt_tests.samba.tests.krb5.salt_tests.SaltTests.test_salt_at_user
-^samba.tests.krb5.salt_tests.samba.tests.krb5.salt_tests.SaltTests.test_salt_double_at_mac
-^samba.tests.krb5.salt_tests.samba.tests.krb5.salt_tests.SaltTests.test_salt_double_at_user
 ^samba.tests.krb5.salt_tests.samba.tests.krb5.salt_tests.SaltTests.test_salt_upn_at_realm_user
index d81f79c705b4fa19b2cf694e4a8776e7e403ec56..1964eb5a4489b073e5a3caba2e58425ad8b0d703 100644 (file)
@@ -1574,11 +1574,11 @@ NTSTATUS secrets_store_JoinCtx(const struct libnet_JoinCtx *r)
        if (info->salt_principal == NULL && r->out.domain_is_ad) {
                char *p = NULL;
 
-               ret = smb_krb5_salt_principal(info->domain_info.dns_domain.string,
-                                             info->account_name,
-                                             NULL /* userPrincipalName */,
-                                             UF_WORKSTATION_TRUST_ACCOUNT,
-                                             info, &p);
+               ret = smb_krb5_salt_principal_str(info->domain_info.dns_domain.string,
+                                                 info->account_name,
+                                                 NULL /* userPrincipalName */,
+                                                 UF_WORKSTATION_TRUST_ACCOUNT,
+                                                 info, &p);
                if (ret != 0) {
                        status = krb5_to_nt_status(ret);
                        DBG_ERR("smb_krb5_salt_principal() failed "
index c98b3401320b1033b2696710313ea78326738e9d..7ecf8c4304051dd974a379bcae07c64a1154c417 100644 (file)
@@ -688,8 +688,8 @@ static int setup_kerberos_keys(struct setup_password_fields_io *io)
 {
        struct ldb_context *ldb;
        krb5_error_code krb5_ret;
-       char *salt_principal = NULL;
-       char *salt_data = NULL;
+       krb5_principal salt_principal = NULL;
+       krb5_data salt_data;
        krb5_data salt;
        krb5_keyblock key;
        krb5_data cleartext_data;
@@ -700,11 +700,11 @@ static int setup_kerberos_keys(struct setup_password_fields_io *io)
        cleartext_data.length = io->n.cleartext_utf8->length;
 
        uac_flags = io->u.userAccountControl & UF_ACCOUNT_TYPE_MASK;
-       krb5_ret = smb_krb5_salt_principal(io->ac->status->domain_data.realm,
+       krb5_ret = smb_krb5_salt_principal(io->smb_krb5_context->krb5_context,
+                                          io->ac->status->domain_data.realm,
                                           io->u.sAMAccountName,
                                           io->u.user_principal_name,
                                           uac_flags,
-                                          io->ac,
                                           &salt_principal);
        if (krb5_ret) {
                ldb_asprintf_errstring(ldb,
@@ -718,8 +718,10 @@ static int setup_kerberos_keys(struct setup_password_fields_io *io)
        /*
         * create salt from salt_principal
         */
-       krb5_ret = smb_krb5_salt_principal2data(io->smb_krb5_context->krb5_context,
-                                               salt_principal, io->ac, &salt_data);
+       krb5_ret = smb_krb5_get_pw_salt(io->smb_krb5_context->krb5_context,
+                                       salt_principal, &salt_data);
+
+       krb5_free_principal(io->smb_krb5_context->krb5_context, salt_principal);
        if (krb5_ret) {
                ldb_asprintf_errstring(ldb,
                                       "setup_kerberos_keys: "
@@ -728,12 +730,17 @@ static int setup_kerberos_keys(struct setup_password_fields_io *io)
                                                                  krb5_ret, io->ac));
                return LDB_ERR_OPERATIONS_ERROR;
        }
-       io->g.salt = salt_data;
 
        /* now use the talloced copy of the salt */
-       salt.data       = discard_const(io->g.salt);
+       salt.data       = talloc_strndup(io->ac,
+                                        (char *)salt_data.data,
+                                        salt_data.length);
+       io->g.salt      = salt.data;
        salt.length     = strlen(io->g.salt);
 
+       smb_krb5_free_data_contents(io->smb_krb5_context->krb5_context,
+                                   &salt_data);
+
        /*
         * create ENCTYPE_AES256_CTS_HMAC_SHA1_96 key out of
         * the salt and the cleartext password