From 3333622e50c87172b08ff1e1c95332eccf1912e6 Mon Sep 17 00:00:00 2001 From: Alexander Bokovoy Date: Fri, 16 Feb 2018 18:15:28 +0200 Subject: [PATCH 1/5] s4:selftest: test kinit with the interdomain trust user account To test it, add a blackbox test that ensures we pass a keytab-based authentication with the trust user account for a trusted domain. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13539 Pair-Programmed-With: Stefan Metzmacher Signed-off-by: Alexander Bokovoy Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 7df505298f71432d5adbcffccde8f97c117a57a6) --- source4/selftest/tests.py | 1 + testprogs/blackbox/test_trust_user_account.sh | 44 +++++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100755 testprogs/blackbox/test_trust_user_account.sh diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index 09cd55b930a1..3ccf2130c5db 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -431,6 +431,7 @@ plantestsuite("samba4.blackbox.trust_token", "fl2008r2dc", [os.path.join(bbdir, plantestsuite("samba4.blackbox.trust_token", "fl2003dc", [os.path.join(bbdir, "test_trust_token.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$REALM', '$DOMAIN', '$DOMSID', '$TRUST_USERNAME', '$TRUST_PASSWORD', '$TRUST_REALM', '$TRUST_DOMAIN', '$TRUST_DOMSID', 'external']) plantestsuite("samba4.blackbox.ktpass(ad_dc_ntvfs)", "ad_dc_ntvfs", [os.path.join(bbdir, "test_ktpass.sh"), '$PREFIX/ad_dc_ntvfs']) plantestsuite("samba4.blackbox.password_settings(ad_dc_ntvfs:local)", "ad_dc_ntvfs:local", [os.path.join(bbdir, "test_password_settings.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$REALM', '$DOMAIN', "$PREFIX/ad_dc_ntvfs"]) +plantestsuite("samba4.blackbox.trust_user_account", "fl2008r2dc:local", [os.path.join(bbdir, "test_trust_user_account.sh"), '$PREFIX', '$REALM', '$DOMAIN', '$TRUST_REALM', '$TRUST_DOMAIN']) plantestsuite("samba4.blackbox.cifsdd(ad_dc_ntvfs)", "ad_dc_ntvfs", [os.path.join(samba4srcdir, "client/tests/test_cifsdd.sh"), '$SERVER', '$USERNAME', '$PASSWORD', "$DOMAIN"]) plantestsuite("samba4.blackbox.nmblookup(ad_dc_ntvfs)", "ad_dc_ntvfs", [os.path.join(samba4srcdir, "utils/tests/test_nmblookup.sh"), '$NETBIOSNAME', '$NETBIOSALIAS', '$SERVER', '$SERVER_IP', nmblookup4]) plantestsuite("samba4.blackbox.locktest(ad_dc_ntvfs)", "ad_dc_ntvfs", [os.path.join(samba4srcdir, "torture/tests/test_locktest.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$DOMAIN', '$PREFIX']) diff --git a/testprogs/blackbox/test_trust_user_account.sh b/testprogs/blackbox/test_trust_user_account.sh new file mode 100755 index 000000000000..9fbe25e16a3c --- /dev/null +++ b/testprogs/blackbox/test_trust_user_account.sh @@ -0,0 +1,44 @@ +#!/bin/sh + +if [ $# -lt 1 ]; then +cat < Date: Tue, 4 Sep 2018 10:16:59 +0200 Subject: [PATCH 2/5] samba-tool: add virtualKerberosSalt attribute to 'user getpassword/syncpasswords' This might be useful for someone, but at least it's very useful for tests. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13539 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 39c281a23673691bab621de1a632d64df2c1c102) --- python/samba/netcmd/user.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/python/samba/netcmd/user.py b/python/samba/netcmd/user.py index f211b5158ced..cc43c08a8249 100644 --- a/python/samba/netcmd/user.py +++ b/python/samba/netcmd/user.py @@ -199,6 +199,9 @@ for (alg, attr) in [("5", "virtualCryptSHA256"), ("6", "virtualCryptSHA512")]: for x in range(1, 30): virtual_attributes["virtualWDigest%02d" % x] = {} +# Add Kerberos virtual attributes +virtual_attributes["virtualKerberosSalt"] = {} + virtual_attributes_help = "The attributes to display (comma separated). " virtual_attributes_help += "Possible supported virtual attributes: %s" % ", ".join(sorted(virtual_attributes.keys())) if len(disabled_virtual_attributes) != 0: @@ -1220,6 +1223,16 @@ class GetPasswordCommand(Command): # first matching scheme return (None, scheme_match) + def get_kerberos_ctr(): + primary_krb5 = get_package("Primary:Kerberos-Newer-Keys") + if primary_krb5 is None: + primary_krb5 = get_package("Primary:Kerberos") + if primary_krb5 is None: + return (0, None) + krb5_blob = ndr_unpack(drsblobs.package_PrimaryKerberosBlob, + primary_krb5) + return (krb5_blob.version, krb5_blob.ctr) + # We use sort here in order to have a predictable processing order for a in sorted(virtual_attributes.keys()): if not a.lower() in lower_attrs: @@ -1271,6 +1284,11 @@ class GetPasswordCommand(Command): v = get_package("Primary:SambaGPG", min_idx=-1) if v is None: continue + elif a == "virtualKerberosSalt": + (krb5_v, krb5_ctr) = get_kerberos_ctr() + if krb5_v not in [3, 4]: + continue + v = krb5_ctr.salt.string elif a.startswith("virtualWDigest"): primary_wdigest = get_package("Primary:WDigest") if primary_wdigest is None: @@ -1387,6 +1405,9 @@ for which virtual attributes are supported in your environment): https://msdn.microsoft.com/en-us/library/cc245680.aspx is incorrect + virtualKerberosSalt: This results the salt string that is used to compute + Kerberos keys from a UTF-8 cleartext password. + virtualSambaGPG: The raw cleartext as stored in the 'Primary:SambaGPG' buffer inside of the supplementalCredentials attribute. @@ -1554,6 +1575,9 @@ for supported virtual attributes in your environment): https://msdn.microsoft.com/en-us/library/cc245680.aspx is incorrect. + virtualKerberosSalt: This results the salt string that is used to compute + Kerberos keys from a UTF-8 cleartext password. + virtualSambaGPG: The raw cleartext as stored in the 'Primary:SambaGPG' buffer inside of the supplementalCredentials attribute. -- 2.17.1 From 416a6634354192c4a9acb9852805abd9d9a60275 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 4 Sep 2018 10:38:44 +0200 Subject: [PATCH 3/5] testprogs/blackbox: add testit[_expect_failure]_grep() to subunit.sh BUG: https://bugzilla.samba.org/show_bug.cgi?id=13539 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 8526feb100e59bc5a15ceb940e6cecce0de59247) --- testprogs/blackbox/subunit.sh | 50 +++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/testprogs/blackbox/subunit.sh b/testprogs/blackbox/subunit.sh index aca53f72536d..bcc5bd6a928b 100755 --- a/testprogs/blackbox/subunit.sh +++ b/testprogs/blackbox/subunit.sh @@ -90,6 +90,31 @@ testit () { return $status } +# This returns 0 if the command gave success and the grep value was found +# all other cases return != 0 +testit_grep () { + name="$1" + shift + grep="$1" + shift + cmdline="$@" + subunit_start_test "$name" + output=`$cmdline 2>&1` + status=$? + if [ x$status != x0 ]; then + printf '%s' "$output" | subunit_fail_test "$name" + return $status + fi + printf '%s' "$output" | grep -q "$grep" + gstatus=$? + if [ x$gstatus = x0 ]; then + subunit_pass_test "$name" + else + printf 'GREP: "%s" not found in output:\n%s' "$grep" "$output" | subunit_fail_test "$name" + fi + return $status +} + testit_expect_failure () { name="$1" shift @@ -105,6 +130,31 @@ testit_expect_failure () { return $status } +# This returns 0 if the command gave a failure and the grep value was found +# all other cases return != 0 +testit_expect_failure_grep () { + name="$1" + shift + grep="$1" + shift + cmdline="$@" + subunit_start_test "$name" + output=`$cmdline 2>&1` + status=$? + if [ x$status = x0 ]; then + printf '%s' "$output" | subunit_fail_test "$name" + return 1 + fi + printf '%s' "$output" | grep -q "$grep" + gstatus=$? + if [ x$gstatus = x0 ]; then + subunit_pass_test "$name" + else + printf 'GREP: "%s" not found in output:\n%s' "$grep" "$output" | subunit_fail_test "$name" + fi + return $status +} + testok () { name=`basename $1` failed=$2 -- 2.17.1 From d04245efa341211bb7bbb283bc8a38cf4a44e22f Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 4 Sep 2018 10:53:52 +0200 Subject: [PATCH 4/5] testprogs/blackbox: let test_trust_user_account.sh check the correct kerberos salt This demonstrates the bug we currently have. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13539 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 1b31fa62567ec549e32c9177b322cfbfb3b6ec1a) --- selftest/knownfail.d/trust_user_account | 1 + testprogs/blackbox/test_trust_user_account.sh | 14 ++++++++++++++ 2 files changed, 15 insertions(+) create mode 100644 selftest/knownfail.d/trust_user_account diff --git a/selftest/knownfail.d/trust_user_account b/selftest/knownfail.d/trust_user_account new file mode 100644 index 000000000000..1de5052b11d6 --- /dev/null +++ b/selftest/knownfail.d/trust_user_account @@ -0,0 +1 @@ +^samba4.blackbox.trust_user_account.get.virtualKerberosSalt.for.TDA diff --git a/testprogs/blackbox/test_trust_user_account.sh b/testprogs/blackbox/test_trust_user_account.sh index 9fbe25e16a3c..b0dc8a9f7736 100755 --- a/testprogs/blackbox/test_trust_user_account.sh +++ b/testprogs/blackbox/test_trust_user_account.sh @@ -37,6 +37,20 @@ export KRB5CCNAME rm -f $KRB5CCNAME +EXPECTED_SALT="${OUR_REALM}krbtgt${REMOTE_FLAT}" +# +# Note the \$ is for the end of line in grep +# +# There must be no trailing '$' in the SALT string itself, +# it's removed from the sAMAccountName value (which includes the trailing '$') +# before construting the salt! +# +# Otherwise this would be: +# "^virtualKerberosSalt: ${EXPECTED_SALT}\\\$\$" +# +EXPECTED_GREP="^virtualKerberosSalt: ${EXPECTED_SALT}\$" +testit_grep "get virtualKerberosSalt for TDA of $REMOTE_FLAT\$" "$EXPECTED_GREP" $samba_tool user getpassword "$REMOTE_FLAT\$" $CONFIGURATION --attributes=virtualKerberosSalt || failed=`expr $failed + 1` + testit "kinit with keytab for TDA of $REMOTE_REALM" $samba4kinit -t $KEYTAB "$REMOTE_FLAT\$@$OUR_REALM" || failed=`expr $failed + 1` rm -f $KRB5CCNAME $KEYTAB -- 2.17.1 From 853fee491735388e45e243404234b565c68594c3 Mon Sep 17 00:00:00 2001 From: Alexander Bokovoy Date: Fri, 16 Feb 2018 18:15:28 +0200 Subject: [PATCH 5/5] krb5-samba: interdomain trust uses different salt principal Salt principal for the interdomain trust is krbtgt/DOMAIN@REALM where DOMAIN is the sAMAccountName without the dollar sign ($) The salt principal for the BLA$ user object was generated wrong. dn: CN=bla.base,CN=System,DC=w4edom-l4,DC=base securityIdentifier: S-1-5-21-4053568372-2049667917-3384589010 trustDirection: 3 trustPartner: bla.base trustPosixOffset: -2147483648 trustType: 2 trustAttributes: 8 flatName: BLA dn: CN=BLA$,CN=Users,DC=w4edom-l4,DC=base userAccountControl: 2080 primaryGroupID: 513 objectSid: S-1-5-21-278041429-3399921908-1452754838-1597 accountExpires: 9223372036854775807 sAMAccountName: BLA$ sAMAccountType: 805306370 pwdLastSet: 131485652467995000 The salt stored by Windows in the package_PrimaryKerberosBlob (within supplementalCredentials) seems to be 'W4EDOM-L4.BASEkrbtgtBLA' for the above trust and Samba stores 'W4EDOM-L4.BASEBLA$'. While the salt used when building the keys from trustAuthOutgoing/trustAuthIncoming is 'W4EDOM-L4.BASEkrbtgtBLA.BASE', which we handle correct. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13539 Pair-Programmed-With: Stefan Metzmacher Signed-off-by: Alexander Bokovoy Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett Autobuild-User(master): Andrew Bartlett Autobuild-Date(master): Wed Sep 5 03:57:22 CEST 2018 on sn-devel-144 (cherry picked from commit f3e349bebc443133fdbe4e14b148ca8db8237060) --- auth/credentials/credentials_krb5.c | 16 +++-- lib/krb5_wrap/krb5_samba.c | 61 ++++++++++++++----- lib/krb5_wrap/krb5_samba.h | 2 +- selftest/knownfail.d/trust_user_account | 1 - source3/passdb/machine_account_secrets.c | 3 +- .../dsdb/samdb/ldb_modules/password_hash.c | 6 +- 6 files changed, 63 insertions(+), 26 deletions(-) delete mode 100644 selftest/knownfail.d/trust_user_account diff --git a/auth/credentials/credentials_krb5.c b/auth/credentials/credentials_krb5.c index 9da1aa09250d..d36797bf0f37 100644 --- a/auth/credentials/credentials_krb5.c +++ b/auth/credentials/credentials_krb5.c @@ -34,6 +34,7 @@ #include "auth/kerberos/kerberos_util.h" #include "auth/kerberos/pac_utils.h" #include "param/param.h" +#include "../libds/common/flags.h" #undef DBGC_CLASS #define DBGC_CLASS DBGC_AUTH @@ -974,7 +975,7 @@ _PUBLIC_ int cli_credentials_get_keytab(struct cli_credentials *cred, const char *upn = NULL; const char *realm = cli_credentials_get_realm(cred); char *salt_principal = NULL; - bool is_computer = false; + uint32_t uac_flags = 0; if (cred->keytab_obtained >= (MAX(cred->principal_obtained, cred->username_obtained))) { @@ -999,9 +1000,15 @@ _PUBLIC_ int cli_credentials_get_keytab(struct cli_credentials *cred, switch (cred->secure_channel_type) { case SEC_CHAN_WKSTA: - case SEC_CHAN_BDC: case SEC_CHAN_RODC: - is_computer = true; + uac_flags = UF_WORKSTATION_TRUST_ACCOUNT; + break; + case SEC_CHAN_BDC: + uac_flags = UF_SERVER_TRUST_ACCOUNT; + break; + case SEC_CHAN_DOMAIN: + case SEC_CHAN_DNS_DOMAIN: + uac_flags = UF_INTERDOMAIN_TRUST_ACCOUNT; break; default: upn = cli_credentials_get_principal(cred, mem_ctx); @@ -1009,13 +1016,14 @@ _PUBLIC_ int cli_credentials_get_keytab(struct cli_credentials *cred, TALLOC_FREE(mem_ctx); return ENOMEM; } + uac_flags = UF_NORMAL_ACCOUNT; break; } ret = smb_krb5_salt_principal(realm, username, /* sAMAccountName */ upn, /* userPrincipalName */ - is_computer, + uac_flags, mem_ctx, &salt_principal); if (ret) { diff --git a/lib/krb5_wrap/krb5_samba.c b/lib/krb5_wrap/krb5_samba.c index 7e90913beb0f..a6ff97640cae 100644 --- a/lib/krb5_wrap/krb5_samba.c +++ b/lib/krb5_wrap/krb5_samba.c @@ -24,6 +24,7 @@ #include "system/filesys.h" #include "krb5_samba.h" #include "lib/crypto/crypto.h" +#include "../libds/common/flags.h" #ifdef HAVE_COM_ERR_H #include @@ -445,8 +446,7 @@ int smb_krb5_get_pw_salt(krb5_context context, * @param[in] userPrincipalName The userPrincipalName attribute of the object * or NULL is not available. * - * @param[in] is_computer The indication of the object includes - * objectClass=computer. + * @param[in] uac_flags UF_ACCOUNT_TYPE_MASKed userAccountControl field * * @param[in] mem_ctx The TALLOC_CTX to allocate _salt_principal. * @@ -459,7 +459,7 @@ int smb_krb5_get_pw_salt(krb5_context context, int smb_krb5_salt_principal(const char *realm, const char *sAMAccountName, const char *userPrincipalName, - bool is_computer, + uint32_t uac_flags, TALLOC_CTX *mem_ctx, char **_salt_principal) { @@ -480,6 +480,23 @@ int smb_krb5_salt_principal(const char *realm, return EINVAL; } + if (uac_flags & ~UF_ACCOUNT_TYPE_MASK) { + /* + * catch callers which still + * pass 'true'. + */ + TALLOC_FREE(frame); + return EINVAL; + } + if (uac_flags == 0) { + /* + * catch callers which still + * pass 'false'. + */ + TALLOC_FREE(frame); + return EINVAL; + } + upper_realm = strupper_talloc(frame, realm); if (upper_realm == NULL) { TALLOC_FREE(frame); @@ -493,7 +510,7 @@ int smb_krb5_salt_principal(const char *realm, /* * Determine a salting principal */ - if (is_computer) { + if (uac_flags & UF_TRUST_ACCOUNT_MASK) { int computer_len = 0; char *tmp = NULL; @@ -502,20 +519,32 @@ int smb_krb5_salt_principal(const char *realm, computer_len -= 1; } - tmp = talloc_asprintf(frame, "host/%*.*s.%s", - computer_len, computer_len, - sAMAccountName, realm); - if (tmp == NULL) { - TALLOC_FREE(frame); - return ENOMEM; - } + if (uac_flags & UF_INTERDOMAIN_TRUST_ACCOUNT) { + principal = talloc_asprintf(frame, "krbtgt/%*.*s", + computer_len, computer_len, + sAMAccountName); + if (principal == NULL) { + TALLOC_FREE(frame); + return ENOMEM; + } + } else { - principal = strlower_talloc(frame, tmp); - TALLOC_FREE(tmp); - if (principal == NULL) { - TALLOC_FREE(frame); - return ENOMEM; + tmp = talloc_asprintf(frame, "host/%*.*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) { + TALLOC_FREE(frame); + return ENOMEM; + } } + principal_len = strlen(principal); } else if (userPrincipalName != NULL) { diff --git a/lib/krb5_wrap/krb5_samba.h b/lib/krb5_wrap/krb5_samba.h index 315d3c3492e6..8305c1f77af0 100644 --- a/lib/krb5_wrap/krb5_samba.h +++ b/lib/krb5_wrap/krb5_samba.h @@ -353,7 +353,7 @@ int smb_krb5_get_pw_salt(krb5_context context, int smb_krb5_salt_principal(const char *realm, const char *sAMAccountName, const char *userPrincipalName, - bool is_computer, + uint32_t uac_flags, TALLOC_CTX *mem_ctx, char **_salt_principal); int smb_krb5_salt_principal2data(krb5_context context, diff --git a/selftest/knownfail.d/trust_user_account b/selftest/knownfail.d/trust_user_account deleted file mode 100644 index 1de5052b11d6..000000000000 --- a/selftest/knownfail.d/trust_user_account +++ /dev/null @@ -1 +0,0 @@ -^samba4.blackbox.trust_user_account.get.virtualKerberosSalt.for.TDA diff --git a/source3/passdb/machine_account_secrets.c b/source3/passdb/machine_account_secrets.c index 94a7e21c8098..a96bf1c0b6ac 100644 --- a/source3/passdb/machine_account_secrets.c +++ b/source3/passdb/machine_account_secrets.c @@ -36,6 +36,7 @@ #include "lib/crypto/crypto.h" #include "lib/krb5_wrap/krb5_samba.h" #include "lib/util/time_basic.h" +#include "../libds/common/flags.h" #undef DBGC_CLASS #define DBGC_CLASS DBGC_PASSDB @@ -1601,7 +1602,7 @@ NTSTATUS secrets_store_JoinCtx(const struct libnet_JoinCtx *r) ret = smb_krb5_salt_principal(info->domain_info.dns_domain.string, info->account_name, NULL /* userPrincipalName */, - true /* is_computer */, + UF_WORKSTATION_TRUST_ACCOUNT, info, &p); if (ret != 0) { status = krb5_to_nt_status(ret); diff --git a/source4/dsdb/samdb/ldb_modules/password_hash.c b/source4/dsdb/samdb/ldb_modules/password_hash.c index 58ae64537eb6..5f5710330044 100644 --- a/source4/dsdb/samdb/ldb_modules/password_hash.c +++ b/source4/dsdb/samdb/ldb_modules/password_hash.c @@ -130,7 +130,6 @@ struct setup_password_fields_io { NTTIME pwdLastSet; const char *sAMAccountName; const char *user_principal_name; - bool is_computer; bool is_krbtgt; uint32_t restrictions; struct dom_sid *account_sid; @@ -678,15 +677,17 @@ static int setup_kerberos_keys(struct setup_password_fields_io *io) krb5_data salt; krb5_keyblock key; krb5_data cleartext_data; + uint32_t uac_flags = 0; ldb = ldb_module_get_ctx(io->ac->module); cleartext_data.data = (char *)io->n.cleartext_utf8->data; 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, io->u.sAMAccountName, io->u.user_principal_name, - io->u.is_computer, + uac_flags, io->ac, &salt_principal); if (krb5_ret) { @@ -3190,7 +3191,6 @@ static int setup_io(struct ph_context *ac, "sAMAccountName", NULL); io->u.user_principal_name = ldb_msg_find_attr_as_string(info_msg, "userPrincipalName", NULL); - io->u.is_computer = ldb_msg_check_string_attribute(info_msg, "objectClass", "computer"); /* Ensure it has an objectSID too */ io->u.account_sid = samdb_result_dom_sid(ac, info_msg, "objectSid"); -- 2.17.1