[SCM] Samba Shared Repository - branch master updated

Alexander Bokovoy ab at samba.org
Wed Nov 4 16:24:03 UTC 2020


The branch, master has been updated
       via  f9016912098 lookup_name: allow lookup for own realm
       via  00f4262ed0b cli_credentials: add a helper to parse user or group names
       via  eb0474d27ba cli_credentials_parse_string: fix parsing of principals
      from  a1b021200e3 selftest: add test for new "samba-tool user unlock" command

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit f901691209867b32c2d7c5c9274eee196f541654
Author: Alexander Bokovoy <ab at samba.org>
Date:   Wed Nov 4 14:21:33 2020 +0200

    lookup_name: allow lookup for own realm
    
    When using a security tab in Windows Explorer, a lookup over a trusted
    forest might come as realm\name instead of NetBIOS domain name:
    
    --------------------------------------------------------------------
    [2020/01/13 11:12:39.859134,  1, pid=33253, effective(1732401004, 1732401004), real(1732401004, 0), class=rpc_parse] ../../librpc/ndr/ndr.c:471(ndr_print_function_debug)
           lsa_LookupNames3: struct lsa_LookupNames3
              in: struct lsa_LookupNames3
                  handle                   : *
                      handle: struct policy_handle
                          handle_type              : 0x00000000 (0)
                          uuid                     : 0000000e-0000-0000-1c5e-a750e5810000
                  num_names                : 0x00000001 (1)
                  names: ARRAY(1)
                      names: struct lsa_String
                          length                   : 0x001e (30)
                          size                     : 0x0020 (32)
                          string                   : *
                              string                   : 'ipa.test\admins'
                  sids                     : *
                      sids: struct lsa_TransSidArray3
                          count                    : 0x00000000 (0)
                          sids                     : NULL
                  level                    : LSA_LOOKUP_NAMES_UPLEVEL_TRUSTS_ONLY2 (6)
                  count                    : *
                      count                    : 0x00000000 (0)
                  lookup_options           : LSA_LOOKUP_OPTION_SEARCH_ISOLATED_NAMES (0)
                  client_revision          : LSA_CLIENT_REVISION_2 (2)
    --------------------------------------------------------------------
    
    Allow this lookup using realm to be done against primary domain when we
    are a domain controller. This corresponds to FreeIPA use of Samba as a
    DC. For normal domain members a realm-based lookup falls back to a
    lookup over to its own domain controller with the help of winbindd.
    
    Refactor user name parsing code to reuse cli_credentials_* API to be
    consistent with other places. cli_credentials_parse_name() handles
    both domain and realm-based user name variants.
    
    Signed-off-by: Alexander Bokovoy <ab at samba.org>
    Reviewed-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    
    Autobuild-User(master): Alexander Bokovoy <ab at samba.org>
    Autobuild-Date(master): Wed Nov  4 16:23:40 UTC 2020 on sn-devel-184

commit 00f4262ed0b22f6e333e5a29c5590b62c783905c
Author: Alexander Bokovoy <ab at samba.org>
Date:   Wed Nov 4 14:00:58 2020 +0200

    cli_credentials: add a helper to parse user or group names
    
    cli_credentials_parse_string() parses a string specified for -U option
    in command line tools. It has a side-effect that '%' character is always
    considered to be a separator after which a password is specified.
    
    Active Directory does allow to create user or group objects with '%' in
    the name. It means cli_credentials_parse_string() will not be able to
    properly parse such name.
    
    Introduce cli_credentials_parse_name() for the cases when a password is
    not expected in the name and call to cli_credentials_parse_name() from
    cli_credentials_parse_string().
    
    Test cli_credentials_parse_name() with its intended use in lookup_name()
    refactoring.
    
    Signed-off-by: Alexander Bokovoy <ab at samba.org>
    Reviewed-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit eb0474d27bae4592b25ac6bf600da29c6a1cb9f8
Author: Alexander Bokovoy <ab at samba.org>
Date:   Wed Oct 7 19:25:24 2020 +0300

    cli_credentials_parse_string: fix parsing of principals
    
    When parsing a principal-like name, user name was left with full
    principal instead of taking only the left part before '@' sign.
    
    >>> from samba import credentials
    >>> t = credentials.Credentials()
    >>> t.parse_string('admin at realm.test', credentials.SPECIFIED)
    >>> t.get_username()
    'admin at realm.test'
    
    The issue is that cli_credentials_set_username() does a talloc_strdup()
    of the argument, so we need to change order of assignment to allow
    talloc_strdup() to copy the right part of the string.
    
    Signed-off-by: Alexander Bokovoy <ab at samba.org>
    Reviewed-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 auth/credentials/credentials.c      | 23 +++++++++--
 auth/credentials/credentials.h      |  1 +
 auth/credentials/tests/test_creds.c | 58 +++++++++++++++++++++++++++-
 python/samba/tests/credentials.py   |  4 +-
 source3/passdb/lookup_sid.c         | 76 +++++++++++++++++++++++++++----------
 5 files changed, 135 insertions(+), 27 deletions(-)


Changeset truncated at 500 lines:

diff --git a/auth/credentials/credentials.c b/auth/credentials/credentials.c
index 1bdd6f15a09..53bba78176b 100644
--- a/auth/credentials/credentials.c
+++ b/auth/credentials/credentials.c
@@ -836,17 +836,34 @@ _PUBLIC_ void cli_credentials_parse_string(struct cli_credentials *credentials,
 		cli_credentials_set_password(credentials, p+1, obtained);
 	}
 
+	cli_credentials_parse_name(credentials, uname, obtained);
+}
+
+/**
+ * Given a string, parse it into a domain, username and realm fields
+ *
+ * The format accepted is [domain\\]user or user[@realm]
+ *
+ * @param credentials Credentials structure on which to set the components
+ * @param data the string containing the username, prefixed or suffixed with domain or realm
+ * @param obtained This enum describes how 'specified' this credential name is.
+ */
+
+_PUBLIC_ void cli_credentials_parse_name(struct cli_credentials *credentials, const char *data, enum credentials_obtained obtained)
+{
+	char *uname, *p;
+
+	uname = talloc_strdup(credentials, data);
 	if ((p = strchr_m(uname,'@'))) {
 		/*
 		 * We also need to set username and domain
 		 * in order to undo the effect of
 		 * cli_credentials_guess().
 		 */
-		cli_credentials_set_username(credentials, uname, obtained);
-		cli_credentials_set_domain(credentials, "", obtained);
-
 		cli_credentials_set_principal(credentials, uname, obtained);
 		*p = 0;
+		cli_credentials_set_username(credentials, uname, obtained);
+		cli_credentials_set_domain(credentials, "", obtained);
 		cli_credentials_set_realm(credentials, p+1, obtained);
 		return;
 	} else if ((p = strchr_m(uname,'\\'))
diff --git a/auth/credentials/credentials.h b/auth/credentials/credentials.h
index f468b8558dd..7c7120b9f55 100644
--- a/auth/credentials/credentials.h
+++ b/auth/credentials/credentials.h
@@ -155,6 +155,7 @@ bool cli_credentials_set_password(struct cli_credentials *cred,
 				  enum credentials_obtained obtained);
 struct cli_credentials *cli_credentials_init_anon(TALLOC_CTX *mem_ctx);
 void cli_credentials_parse_string(struct cli_credentials *credentials, const char *data, enum credentials_obtained obtained);
+void cli_credentials_parse_name(struct cli_credentials *credentials, const char *data, enum credentials_obtained obtained);
 struct samr_Password *cli_credentials_get_nt_hash(struct cli_credentials *cred,
 						  TALLOC_CTX *mem_ctx);
 struct samr_Password *cli_credentials_get_old_nt_hash(struct cli_credentials *cred,
diff --git a/auth/credentials/tests/test_creds.c b/auth/credentials/tests/test_creds.c
index d2d3d30d73d..38550d6ecf9 100644
--- a/auth/credentials/tests/test_creds.c
+++ b/auth/credentials/tests/test_creds.c
@@ -187,7 +187,7 @@ static void torture_creds_parse_string(void **state)
 	assert_string_equal(creds->domain, "");
 	assert_int_equal(creds->domain_obtained, CRED_SPECIFIED);
 
-	assert_string_equal(creds->username, "wurst at brot.realm");
+	assert_string_equal(creds->username, "wurst");
 	assert_int_equal(creds->username_obtained, CRED_SPECIFIED);
 
 	assert_string_equal(creds->principal, "wurst at brot.realm");
@@ -197,6 +197,61 @@ static void torture_creds_parse_string(void **state)
 	assert_int_equal(creds->password_obtained, CRED_SPECIFIED);
 }
 
+static void _parse_name_as_lookup_name(TALLOC_CTX *mem_ctx,
+				      const char *full_name,
+				      const char *expected_name,
+				      const char *expected_domain,
+				      const char *expected_realm)
+{
+	struct cli_credentials *creds = NULL;
+
+	creds = cli_credentials_init(mem_ctx);
+	assert_non_null(creds);
+
+	cli_credentials_parse_name(creds, full_name, CRED_SPECIFIED);
+
+	if (expected_name == NULL) {
+		assert_null(cli_credentials_get_username(creds));
+	} else {
+		assert_string_equal(cli_credentials_get_username(creds), expected_name);
+	}
+
+	if (expected_domain == NULL) {
+		assert_null(cli_credentials_get_domain(creds));
+	} else {
+		assert_string_equal(cli_credentials_get_domain(creds), expected_domain);
+	}
+
+	if (expected_realm == NULL) {
+		assert_null(cli_credentials_get_realm(creds));
+	} else {
+		assert_string_equal(cli_credentials_get_realm(creds), expected_realm);
+	}
+
+	TALLOC_FREE(creds);
+
+}
+
+static void torture_creds_parse_name(void **state)
+{
+	TALLOC_CTX *mem_ctx = *state;
+
+	_parse_name_as_lookup_name(mem_ctx, "XXL\\",
+				   "", "XXL", NULL);
+
+	_parse_name_as_lookup_name(mem_ctx, "XXL\\wurst",
+				   "wurst", "XXL", NULL);
+
+	_parse_name_as_lookup_name(mem_ctx, "wurst at brot.realm",
+				   "wurst", "", "BROT.REALM");
+
+	_parse_name_as_lookup_name(mem_ctx, "wur%t",
+				   "wur%t", NULL, NULL);
+
+	_parse_name_as_lookup_name(mem_ctx, "wurst",
+				   "wurst", NULL, NULL);
+}
+
 int main(int argc, char *argv[])
 {
 	int rc;
@@ -206,6 +261,7 @@ int main(int argc, char *argv[])
 		cmocka_unit_test(torture_creds_guess),
 		cmocka_unit_test(torture_creds_anon_guess),
 		cmocka_unit_test(torture_creds_parse_string),
+		cmocka_unit_test(torture_creds_parse_name),
 	};
 
 	if (argc == 2) {
diff --git a/python/samba/tests/credentials.py b/python/samba/tests/credentials.py
index bcd15b1130f..5d1378fb790 100644
--- a/python/samba/tests/credentials.py
+++ b/python/samba/tests/credentials.py
@@ -400,7 +400,7 @@ class CredentialsTests(samba.tests.TestCaseInTempDir):
         os.environ["USER"] = "env_user"
         creds.guess(lp)
         creds.parse_string("user at samba.org")
-        self.assertEqual(creds.get_username(), "user at samba.org")
+        self.assertEqual(creds.get_username(), "user")
         self.assertEqual(creds.get_domain(), "")
         self.assertEqual(creds.get_realm(), "SAMBA.ORG")
         self.assertEqual(creds.get_principal(), "user at samba.org")
@@ -441,7 +441,7 @@ class CredentialsTests(samba.tests.TestCaseInTempDir):
         os.environ["USER"] = "env_user"
         creds.guess(lp)
         creds.parse_string("user at samba.org%pass")
-        self.assertEqual(creds.get_username(), "user at samba.org")
+        self.assertEqual(creds.get_username(), "user")
         self.assertEqual(creds.get_domain(), "")
         self.assertEqual(creds.get_password(), "pass")
         self.assertEqual(creds.get_realm(), "SAMBA.ORG")
diff --git a/source3/passdb/lookup_sid.c b/source3/passdb/lookup_sid.c
index ff8a16619a8..dc32cd9753b 100644
--- a/source3/passdb/lookup_sid.c
+++ b/source3/passdb/lookup_sid.c
@@ -29,6 +29,7 @@
 #include "../libcli/security/security.h"
 #include "lib/winbind_util.h"
 #include "../librpc/gen_ndr/idmap.h"
+#include "auth/credentials/credentials.h"
 
 static bool lookup_unix_user_name(const char *name, struct dom_sid *sid)
 {
@@ -78,52 +79,85 @@ bool lookup_name(TALLOC_CTX *mem_ctx,
 		 const char **ret_domain, const char **ret_name,
 		 struct dom_sid *ret_sid, enum lsa_SidType *ret_type)
 {
-	char *p;
 	const char *tmp;
 	const char *domain = NULL;
 	const char *name = NULL;
+	const char *realm = NULL;
 	uint32_t rid;
 	struct dom_sid sid;
 	enum lsa_SidType type;
 	TALLOC_CTX *tmp_ctx = talloc_new(mem_ctx);
+	struct cli_credentials *creds = NULL;
 
 	if (tmp_ctx == NULL) {
 		DEBUG(0, ("talloc_new failed\n"));
 		return false;
 	}
 
-	p = strchr_m(full_name, '\\');
-
-	if (p != NULL) {
-		domain = talloc_strndup(tmp_ctx, full_name,
-					PTR_DIFF(p, full_name));
-		name = talloc_strdup(tmp_ctx, p+1);
-	} else {
-		domain = talloc_strdup(tmp_ctx, "");
-		name = talloc_strdup(tmp_ctx, full_name);
+	creds = cli_credentials_init(tmp_ctx);
+	if (creds == NULL) {
+		DEBUG(0, ("cli_credentials_init failed\n"));
+		return false;
 	}
 
-	if ((domain == NULL) || (name == NULL)) {
-		DEBUG(0, ("talloc failed\n"));
+	cli_credentials_parse_name(creds, full_name, CRED_SPECIFIED);
+	name = cli_credentials_get_username(creds);
+	domain = cli_credentials_get_domain(creds);
+	realm = cli_credentials_get_realm(creds);
+
+	/* At this point we have:
+	 * - name -- normal name or empty string
+	 * - domain -- either NULL or domain name
+	 * - realm -- either NULL or realm name
+	 *
+	 * domain and realm are exclusive to each other
+	 * the code below in lookup_name assumes domain
+	 * to be at least empty string, not NULL
+	*/
+
+	if (name == NULL) {
+		DEBUG(0, ("lookup_name with empty name, exit\n"));
 		TALLOC_FREE(tmp_ctx);
 		return false;
 	}
 
+	if ((domain == NULL) && (realm == NULL)) {
+		domain = talloc_strdup(creds, "");
+	}
+
 	DEBUG(10,("lookup_name: %s => domain=[%s], name=[%s]\n",
 		full_name, domain, name));
 	DEBUG(10, ("lookup_name: flags = 0x0%x\n", flags));
 
-	if (((flags & LOOKUP_NAME_DOMAIN) || (flags == 0)) &&
-	    strequal(domain, get_global_sam_name()))
-	{
+	/* Windows clients may send a LookupNames request with both NetBIOS
+	 * domain name- and realm-qualified user names. Thus, we need to check
+	 * both against both of the SAM domain name and realm, if set. Since
+	 * domain name and realm in the request are exclusive, test the one
+	 * that is specified.  cli_credentials_parse_string() will either set
+	 * realm or wouldn't so we can use it to detect if realm was specified.
+	 */
+	if ((flags & LOOKUP_NAME_DOMAIN) || (flags == 0)) {
+		const char *domain_name = realm ? realm : domain;
+		bool check_global_sam = false;
+
+		if (domain_name[0] != '\0') {
+			check_global_sam = strequal(domain_name, get_global_sam_name());
+			if (!check_global_sam && lp_realm() != NULL) {
+				/* Only consider realm when we are DC
+				 * otherwise use lookup through winbind */
+				check_global_sam = strequal(domain_name, lp_realm()) && IS_DC;
+			}
+		}
 
-		/* It's our own domain, lookup the name in passdb */
-		if (lookup_global_sam_name(name, flags, &rid, &type)) {
-			sid_compose(&sid, get_global_sam_sid(), rid);
-			goto ok;
+		if (check_global_sam) {
+			/* It's our own domain, lookup the name in passdb */
+			if (lookup_global_sam_name(name, flags, &rid, &type)) {
+				sid_compose(&sid, get_global_sam_sid(), rid);
+				goto ok;
+			}
+			TALLOC_FREE(tmp_ctx);
+			return false;
 		}
-		TALLOC_FREE(tmp_ctx);
-		return false;
 	}
 
 	if ((flags & LOOKUP_NAME_BUILTIN) &&


-- 
Samba Shared Repository



More information about the samba-cvs mailing list