[PATCH] Fix bug 11044 - Samba 4.2 using Kerberos (not AD) fails to authenticate

Jeremy Allison jra at samba.org
Tue Jan 13 15:22:35 MST 2015


Turns out this is a generic problem with
unix users not existing in the Samba passdb
and is easy to reproduce (and test thank
goodness :-).

Given the following share:

[utest]
        path = /tmp
        force user = gdm

where gdm doesn't exist in the tdb password database but is a UNIX user.

Connect as user foo, who does exist in the tdb passdb.

We get:

/usr/local/samba/bin/smbclient //127.0.0.1/utest -Ufoo%PASSWORD
Domain=[WORKGROUP] OS=[Windows 6.1] Server=[Samba 4.3.0pre1-GIT-42aee0e]
tree connect failed: NT_STATUS_INVALID_SID

Patchset + test attched.

Please review and push. Note this is a blocker
for 4.2.0.

Cheers,

	Jeremy.
-------------- next part --------------
From ddd65036129b96775685c4bc449920aa13ae9e01 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 13 Jan 2015 13:35:56 -0800
Subject: [PATCH 1/5] s3: auth: Add a utility function - SamInfo3_handle_sids()
 that factors out the code to handle "Unix Users" and "Unix Groups".

Based on code from Michael Zeis <mzeis.quantum at gmail.com>

https://bugzilla.samba.org/show_bug.cgi?id=11044

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/auth/server_info.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/source3/auth/server_info.c b/source3/auth/server_info.c
index 8fd3b0d..cbb941b 100644
--- a/source3/auth/server_info.c
+++ b/source3/auth/server_info.c
@@ -330,6 +330,76 @@ NTSTATUS create_info3_from_pac_logon_info(TALLOC_CTX *mem_ctx,
 	return NT_STATUS_OK;
 }
 
+/*
+ * Check if this is a "Unix Users" domain user, or a
+ * "Unix Groups" domain group, we need to handle it
+ * in a special way if that's the case.
+ */
+
+static NTSTATUS SamInfo3_handle_sids(const char *username,
+			const struct dom_sid *user_sid,
+			const struct dom_sid *group_sid,
+			struct netr_SamInfo3 *info3,
+			struct dom_sid *domain_sid,
+			struct extra_auth_info *extra)
+{
+	if (sid_check_is_in_unix_users(user_sid)) {
+		/* in info3 you can only set rids for the user and the
+		 * primary group, and the domain sid must be that of
+		 * the sam domain.
+		 *
+		 * Store a completely bogus value here.
+		 * The real SID is stored in the extra sids.
+		 * Other code will know to look there if (-1) is found
+		 */
+		info3->base.rid = (uint32_t)(-1);
+		sid_copy(&extra->user_sid, user_sid);
+
+		DEBUG(10, ("Unix User found. Rid marked as "
+			"special and sid (%s) saved as extra sid\n",
+			sid_string_dbg(user_sid)));
+	} else {
+		sid_copy(domain_sid, user_sid);
+		sid_split_rid(domain_sid, &info3->base.rid);
+	}
+
+	if (is_null_sid(domain_sid)) {
+		sid_copy(domain_sid, get_global_sam_sid());
+	}
+
+	/* check if this is a "Unix Groups" domain group,
+	 * if so we need special handling */
+	if (sid_check_is_in_unix_groups(group_sid)) {
+		/* in info3 you can only set rids for the user and the
+		 * primary group, and the domain sid must be that of
+		 * the sam domain.
+		 *
+		 * Store a completely bogus value here.
+		 * The real SID is stored in the extra sids.
+		 * Other code will know to look there if (-1) is found
+		 */
+		info3->base.primary_gid = (uint32_t)(-1);
+		sid_copy(&extra->pgid_sid, group_sid);
+
+		DEBUG(10, ("Unix Group found. Rid marked as "
+			"special and sid (%s) saved as extra sid\n",
+			sid_string_dbg(group_sid)));
+	} else {
+		bool ok = sid_peek_check_rid(domain_sid, group_sid,
+					&info3->base.primary_gid);
+		if (!ok) {
+			DEBUG(1, ("The primary group domain sid(%s) does not "
+				"match the domain sid(%s) for %s(%s)\n",
+				sid_string_dbg(group_sid),
+				sid_string_dbg(domain_sid),
+				username,
+				sid_string_dbg(user_sid)));
+			return NT_STATUS_INVALID_SID;
+		}
+	}
+	return NT_STATUS_OK;
+}
+
 #define RET_NOMEM(ptr) do { \
 	if (!ptr) { \
 		TALLOC_FREE(info3); \
-- 
2.2.0.rc0.207.ga3a616c


From 614dbc5ee303f0dab12bbcc8428efa441908b689 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 13 Jan 2015 13:39:21 -0800
Subject: [PATCH 2/5] s3: auth: Convert samu_to_SamInfo3() to use the new
 utility function.

Based on code from Michael Zeis <mzeis.quantum at gmail.com>

https://bugzilla.samba.org/show_bug.cgi?id=11044

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/auth/server_info.c | 66 +++++++---------------------------------------
 1 file changed, 9 insertions(+), 57 deletions(-)

diff --git a/source3/auth/server_info.c b/source3/auth/server_info.c
index cbb941b..9f992ff 100644
--- a/source3/auth/server_info.c
+++ b/source3/auth/server_info.c
@@ -421,7 +421,6 @@ NTSTATUS samu_to_SamInfo3(TALLOC_CTX *mem_ctx,
 	const char *tmp;
 	gid_t *gids;
 	NTSTATUS status;
-	bool ok;
 
 	user_sid = pdb_get_user_sid(samu);
 	group_sid = pdb_get_group_sid(samu);
@@ -438,63 +437,16 @@ NTSTATUS samu_to_SamInfo3(TALLOC_CTX *mem_ctx,
 
 	ZERO_STRUCT(domain_sid);
 
-	/* check if this is a "Unix Users" domain user,
-	 * we need to handle it in a special way if that's the case */
-	if (sid_check_is_in_unix_users(user_sid)) {
-		/* in info3 you can only set rids for the user and the
-		 * primary group, and the domain sid must be that of
-		 * the sam domain.
-		 *
-		 * Store a completely bogus value here.
-		 * The real SID is stored in the extra sids.
-		 * Other code will know to look there if (-1) is found
-		 */
-		info3->base.rid = (uint32_t)(-1);
-		sid_copy(&extra->user_sid, user_sid);
-
-		DEBUG(10, ("Unix User found in struct samu. Rid marked as "
-			   "special and sid (%s) saved as extra sid\n",
-			   sid_string_dbg(user_sid)));
-	} else {
-		sid_copy(&domain_sid, user_sid);
-		sid_split_rid(&domain_sid, &info3->base.rid);
-	}
-
-	if (is_null_sid(&domain_sid)) {
-		sid_copy(&domain_sid, get_global_sam_sid());
-	}
+	status = SamInfo3_handle_sids(pdb_get_username(samu),
+				user_sid,
+				group_sid,
+				info3,
+				&domain_sid,
+				extra);
 
-	/* check if this is a "Unix Groups" domain group,
-	 * if so we need special handling */
-	if (sid_check_is_in_unix_groups(group_sid)) {
-		/* in info3 you can only set rids for the user and the
-		 * primary group, and the domain sid must be that of
-		 * the sam domain.
-		 *
-		 * Store a completely bogus value here.
-		 * The real SID is stored in the extra sids.
-		 * Other code will know to look there if (-1) is found
-		 */
-		info3->base.primary_gid = (uint32_t)(-1);
-		sid_copy(&extra->pgid_sid, group_sid);
-
-		DEBUG(10, ("Unix Group found in struct samu. Rid marked as "
-			   "special and sid (%s) saved as extra sid\n",
-			   sid_string_dbg(group_sid)));
-
-	} else {
-		ok = sid_peek_check_rid(&domain_sid, group_sid,
-					&info3->base.primary_gid);
-		if (!ok) {
-			DEBUG(1, ("The primary group domain sid(%s) does not "
-				  "match the domain sid(%s) for %s(%s)\n",
-				  sid_string_dbg(group_sid),
-				  sid_string_dbg(&domain_sid),
-				  pdb_get_username(samu),
-				  sid_string_dbg(user_sid)));
-			TALLOC_FREE(info3);
-			return NT_STATUS_UNSUCCESSFUL;
-		}
+	if (!NT_STATUS_IS_OK(status)) {
+		TALLOC_FREE(info3);
+		return status;
 	}
 
 	unix_to_nt_time(&info3->base.logon_time, pdb_get_logon_time(samu));
-- 
2.2.0.rc0.207.ga3a616c


From 6b6c0089a6d36c9a07fc0e789ee218bdd30f0690 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 13 Jan 2015 13:45:16 -0800
Subject: [PATCH 3/5] s3: auth: Plumb in the SamInfo3_handle_sids() utility
 function into passwd_to_SamInfo3().

Core fix for:

https://bugzilla.samba.org/show_bug.cgi?id=11044

Based on code from Michael Zeis <mzeis.quantum at gmail.com>

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/auth/auth_util.c   |  3 ++-
 source3/auth/proto.h       |  3 ++-
 source3/auth/server_info.c | 16 +++++++++++++---
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/source3/auth/auth_util.c b/source3/auth/auth_util.c
index dbc7d24..585afd3 100644
--- a/source3/auth/auth_util.c
+++ b/source3/auth/auth_util.c
@@ -671,7 +671,8 @@ NTSTATUS make_server_info_pw(TALLOC_CTX *mem_ctx,
 	status = passwd_to_SamInfo3(result,
 				    unix_username,
 				    pwd,
-				    &result->info3);
+				    &result->info3,
+				    &result->extra);
 	if (!NT_STATUS_IS_OK(status)) {
 		goto done;
 	}
diff --git a/source3/auth/proto.h b/source3/auth/proto.h
index da3c099..792e96d 100644
--- a/source3/auth/proto.h
+++ b/source3/auth/proto.h
@@ -305,7 +305,8 @@ NTSTATUS samu_to_SamInfo3(TALLOC_CTX *mem_ctx,
 NTSTATUS passwd_to_SamInfo3(TALLOC_CTX *mem_ctx,
 			    const char *unix_username,
 			    const struct passwd *pwd,
-			    struct netr_SamInfo3 **pinfo3);
+			    struct netr_SamInfo3 **pinfo3,
+			    struct extra_auth_info *extra);
 struct netr_SamInfo3 *copy_netr_SamInfo3(TALLOC_CTX *mem_ctx,
 					 const struct netr_SamInfo3 *orig);
 
diff --git a/source3/auth/server_info.c b/source3/auth/server_info.c
index 9f992ff..7b1cdd5 100644
--- a/source3/auth/server_info.c
+++ b/source3/auth/server_info.c
@@ -539,7 +539,8 @@ NTSTATUS samu_to_SamInfo3(TALLOC_CTX *mem_ctx,
 NTSTATUS passwd_to_SamInfo3(TALLOC_CTX *mem_ctx,
 			    const char *unix_username,
 			    const struct passwd *pwd,
-			    struct netr_SamInfo3 **pinfo3)
+			    struct netr_SamInfo3 **pinfo3,
+			    struct extra_auth_info *extra)
 {
 	struct netr_SamInfo3 *info3;
 	NTSTATUS status;
@@ -635,8 +636,17 @@ NTSTATUS passwd_to_SamInfo3(TALLOC_CTX *mem_ctx,
 
 	ZERO_STRUCT(domain_sid);
 
-	sid_copy(&domain_sid, &user_sid);
-	sid_split_rid(&domain_sid, &info3->base.rid);
+	status = SamInfo3_handle_sids(unix_username,
+				&user_sid,
+				&group_sid,
+				info3,
+				&domain_sid,
+				extra);
+
+	if (!NT_STATUS_IS_OK(status)) {
+		goto done;
+	}
+
 	info3->base.domain_sid = dom_sid_dup(info3, &domain_sid);
 
 	ok = sid_peek_check_rid(&domain_sid, &group_sid,
-- 
2.2.0.rc0.207.ga3a616c


From b344d19be88e9ae23df98279167b8e21450e265a Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 13 Jan 2015 13:49:36 -0800
Subject: [PATCH 4/5] s3: auth: Add previously missing allocation fail check.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/auth/server_info.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/source3/auth/server_info.c b/source3/auth/server_info.c
index 7b1cdd5..b537390 100644
--- a/source3/auth/server_info.c
+++ b/source3/auth/server_info.c
@@ -648,6 +648,10 @@ NTSTATUS passwd_to_SamInfo3(TALLOC_CTX *mem_ctx,
 	}
 
 	info3->base.domain_sid = dom_sid_dup(info3, &domain_sid);
+	if (info3->base.domain_sid == NULL) {
+		status = NT_STATUS_NO_MEMORY;
+		goto done;
+	}
 
 	ok = sid_peek_check_rid(&domain_sid, &group_sid,
 				&info3->base.primary_gid);
-- 
2.2.0.rc0.207.ga3a616c


From 4418130a44d0719b28142ef06224638a33e934af Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 13 Jan 2015 13:49:58 -0800
Subject: [PATCH 5/5] s3: auth - tests: Add test for "force user" being a
 unix-only user, not in passdb.

https://bugzilla.samba.org/show_bug.cgi?id=11044

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 selftest/target/Samba3.pm                   | 4 ++++
 source3/script/tests/test_smbclient_auth.sh | 1 +
 2 files changed, 5 insertions(+)

diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 289e464..8926ae1 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -1182,6 +1182,10 @@ sub provision($$$$$$$$)
 	path = $shrdir
         force user = $unix_name
         guest ok = yes
+[forceuser_unixonly]
+	path = $shrdir
+	force user = pdbtest
+	guest ok = yes
 [forcegroup]
 	path = $shrdir
         force group = nogroup
diff --git a/source3/script/tests/test_smbclient_auth.sh b/source3/script/tests/test_smbclient_auth.sh
index 3988095..24e98b1 100755
--- a/source3/script/tests/test_smbclient_auth.sh
+++ b/source3/script/tests/test_smbclient_auth.sh
@@ -27,5 +27,6 @@ testit "smbclient //$SERVER/tmpguest" $SMBCLIENT //$SERVER/tmpguest $CONFIGURATI
 testit "smbclient //$SERVER/tmpguest as anon" $SMBCLIENT //$SERVER/tmpguest $CONFIGURATION -U% -I $SERVER_IP -p 139 -c quit $ADDARGS
 testit "smbclient //$SERVER/forceuser" $SMBCLIENT //$SERVER/forceuser $CONFIGURATION -U$USERNAME%$PASSWORD -I $SERVER_IP -p 139 -c quit $ADDARGS
 testit "smbclient //$SERVER/forceuser as anon" $SMBCLIENT //$SERVER/forceuser $CONFIGURATION -U% -I $SERVER_IP -p 139 -c quit $ADDARGS
+testit "smbclient //$SERVER/forceuser_unixonly" $SMBCLIENT //$SERVER/forceuser_unixonly $CONFIGURATION -U$USERNAME%$PASSWORD -I $SERVER_IP -p 139 -c quit $ADDARGS
 testit "smbclient //$SERVER/forcegroup" $SMBCLIENT //$SERVER/forcegroup $CONFIGURATION -U$USERNAME%$PASSWORD -I $SERVER_IP -p 139 -c quit $ADDARGS
 testit "smbclient //$SERVER/forcegroup as anon" $SMBCLIENT //$SERVER/forcegroup $CONFIGURATION -U% -I $SERVER_IP -p 139 -c quit $ADDARGS
-- 
2.2.0.rc0.207.ga3a616c



More information about the samba-technical mailing list