[PATCH] Fix for bug 11320

Jeremy Allison jra at samba.org
Tue Jul 28 18:35:04 UTC 2015


Marc, you've already reviewed Justin's patch,
here is the complete patchset including the
regression test.

Please review !

Cheers,

	Jeremy.
-------------- next part --------------
From a1a42b6207a721fcb95651e290d06297afd36765 Mon Sep 17 00:00:00 2001
From: Justin Maggard <jmaggard at netgear.com>
Date: Tue, 21 Jul 2015 15:17:30 -0700
Subject: [PATCH 1/2] s3-passdb: Respect LOOKUP_NAME_GROUP flag in sid lookup.

Somewhere along the line, a config line like "valid users = @foo"
broke when "foo" also exists as a user.

user_ok_token() already does the right thing by adding the LOOKUP_NAME_GROUP
flag; but lookup_name() was not respecting that flag, and went ahead and looked
for users anyway.

Regression test to follow.

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

Signed-off-by: Justin Maggard <jmaggard at netgear.com>
Reviewed-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Marc Muehlfeld <mmuehlfeld at samba.org>
---
 source3/passdb/lookup_sid.c | 4 ++--
 source3/passdb/lookup_sid.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/source3/passdb/lookup_sid.c b/source3/passdb/lookup_sid.c
index 3cc64de..3f99ee1 100644
--- a/source3/passdb/lookup_sid.c
+++ b/source3/passdb/lookup_sid.c
@@ -120,7 +120,7 @@ bool lookup_name(TALLOC_CTX *mem_ctx,
 			goto ok;
 	}
 
-	if (((flags & LOOKUP_NAME_NO_NSS) == 0)
+	if (((flags & (LOOKUP_NAME_NO_NSS|LOOKUP_NAME_GROUP)) == 0)
 	    && strequal(domain, unix_users_domain_name())) {
 		if (lookup_unix_user_name(name, &sid)) {
 			type = SID_NAME_USER;
@@ -293,7 +293,7 @@ bool lookup_name(TALLOC_CTX *mem_ctx,
 	/* 11. Ok, windows would end here. Samba has two more options:
                Unmapped users and unmapped groups */
 
-	if (((flags & LOOKUP_NAME_NO_NSS) == 0)
+	if (((flags & (LOOKUP_NAME_NO_NSS|LOOKUP_NAME_GROUP)) == 0)
 	    && lookup_unix_user_name(name, &sid)) {
 		domain = talloc_strdup(tmp_ctx, unix_users_domain_name());
 		type = SID_NAME_USER;
diff --git a/source3/passdb/lookup_sid.h b/source3/passdb/lookup_sid.h
index 872f4ef..8b5edf6 100644
--- a/source3/passdb/lookup_sid.h
+++ b/source3/passdb/lookup_sid.h
@@ -31,7 +31,7 @@ struct unixid;
 #define LOOKUP_NAME_NONE		0x00000000
 #define LOOKUP_NAME_ISOLATED             0x00000001  /* Look up unqualified names */
 #define LOOKUP_NAME_REMOTE               0x00000002  /* Ask others */
-#define LOOKUP_NAME_GROUP                0x00000004  /* (unused) This is a NASTY hack for
+#define LOOKUP_NAME_GROUP                0x00000004  /* This is a NASTY hack for
 							valid users = @foo where foo also
 							exists in as user. */
 #define LOOKUP_NAME_NO_NSS		 0x00000008  /* no NSS calls to avoid
-- 
2.5.0.rc2.392.g76e840b


From e55fddcdaaf2f799b72afcfb7b6c3dba32141b69 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 28 Jul 2015 11:28:20 -0700
Subject: [PATCH 2/2] tests: Add regression test for s3-passdb: Respect
 LOOKUP_NAME_GROUP flag in sid lookup.

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

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 selftest/target/Samba3.pm                | 30 ++++++++++++--
 source3/script/tests/test_valid_users.sh | 70 ++++++++++++++++++++++++++++++++
 source3/selftest/tests.py                |  1 +
 3 files changed, 97 insertions(+), 4 deletions(-)
 create mode 100755 source3/script/tests/test_valid_users.sh

diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 9af8faa..7ceb4fa 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -583,6 +583,9 @@ sub setup_fileserver($$)
 	my $dfree_share_dir="$share_dir/dfree";
 	push(@dirs, $dfree_share_dir);
 
+	my $valid_users_sharedir="$share_dir/valid_users";
+	push(@dirs,$valid_users_sharedir);
+
 	my $fileserver_options = "
 [lowercase]
 	path = $lower_case_share_dir
@@ -602,11 +605,14 @@ sub setup_fileserver($$)
 	path = $dfree_share_dir
 	comment = smb username is [%U]
 	dfree command = $srcdir_abs/testprogs/blackbox/dfree.sh
+[valid-users-access]
+	path = $valid_users_sharedir
+	valid users = +SAMBA-TEST/userdup
 	";
 
 	my $vars = $self->provision($path,
 				    "FILESERVER",
-				    "fileserver_secret",
+				    "fileserver",
 				    $fileserver_options,
 				    undef,
 				    undef,
@@ -656,6 +662,17 @@ sub setup_fileserver($$)
 		close $fh;
 	}
 
+	##
+	## create a listable file in valid_users_share
+	##
+        my $valid_users_target = "$valid_users_sharedir/foo";
+        unless (open(VALID_USERS_TARGET, ">$valid_users_target")) {
+                warn("Unable to open $valid_users_target");
+                return undef;
+        }
+        close(VALID_USERS_TARGET);
+        chmod 0644, $valid_users_target;
+
 	return $vars;
 }
 
@@ -1193,10 +1210,11 @@ sub provision($$$$$$$$)
 	##
 
 	my ($max_uid, $max_gid);
-	my ($uid_nobody, $uid_root, $uid_pdbtest, $uid_pdbtest2);
+	my ($uid_nobody, $uid_root, $uid_pdbtest, $uid_pdbtest2, $uid_userdup);
 	my ($gid_nobody, $gid_nogroup, $gid_root, $gid_domusers, $gid_domadmins);
+	my ($gid_userdup);
 
-	if ($unix_uid < 0xffff - 4) {
+	if ($unix_uid < 0xffff - 5) {
 		$max_uid = 0xffff;
 	} else {
 		$max_uid = $unix_uid;
@@ -1206,8 +1224,9 @@ sub provision($$$$$$$$)
 	$uid_nobody = $max_uid - 2;
 	$uid_pdbtest = $max_uid - 3;
 	$uid_pdbtest2 = $max_uid - 4;
+	$uid_userdup = $max_uid - 5;
 
-	if ($unix_gids[0] < 0xffff - 5) {
+	if ($unix_gids[0] < 0xffff - 6) {
 		$max_gid = 0xffff;
 	} else {
 		$max_gid = $unix_gids[0];
@@ -1218,6 +1237,7 @@ sub provision($$$$$$$$)
 	$gid_root = $max_gid - 3;
 	$gid_domusers = $max_gid - 4;
 	$gid_domadmins = $max_gid - 5;
+	$gid_userdup = $max_gid - 6;
 
 	##
 	## create conffile
@@ -1488,6 +1508,7 @@ sub provision($$$$$$$$)
 $unix_name:x:$unix_uid:$unix_gids[0]:$unix_name gecos:$prefix_abs:/bin/false
 pdbtest:x:$uid_pdbtest:$gid_nogroup:pdbtest gecos:$prefix_abs:/bin/false
 pdbtest2:x:$uid_pdbtest2:$gid_nogroup:pdbtest gecos:$prefix_abs:/bin/false
+userdup:x:$uid_userdup:$gid_userdup:userdup gecos:$prefix_abs:/bin/false
 ";
 	if ($unix_uid != 0) {
 		print PASSWD "root:x:$uid_root:$gid_root:root gecos:$prefix_abs:/bin/false
@@ -1504,6 +1525,7 @@ nogroup:x:$gid_nogroup:nobody
 $unix_name-group:x:$unix_gids[0]:
 domusers:X:$gid_domusers:
 domadmins:X:$gid_domadmins:
+userdup:x:$gid_userdup:$unix_name
 ";
 	if ($unix_gids[0] != 0) {
 		print GROUP "root:x:$gid_root:
diff --git a/source3/script/tests/test_valid_users.sh b/source3/script/tests/test_valid_users.sh
new file mode 100755
index 0000000..a7f9333
--- /dev/null
+++ b/source3/script/tests/test_valid_users.sh
@@ -0,0 +1,70 @@
+#!/bin/sh
+#
+# Blackbox test for valid users.
+#
+
+if [ $# -lt 7 ]; then
+cat <<EOF
+Usage: valid_users SERVER SERVER_IP DOMAIN USERNAME PASSWORD PREFIX SMBCLIENT
+EOF
+exit 1;
+fi
+
+SERVER=${1}
+SERVER_IP=${2}
+DOMAIN=${3}
+USERNAME=${4}
+PASSWORD=${5}
+PREFIX=${6}
+SMBCLIENT=${7}
+shift 7
+SMBCLIENT="$VALGRIND ${SMBCLIENT}"
+ADDARGS="$*"
+
+incdir=`dirname $0`/../../../testprogs/blackbox
+. $incdir/subunit.sh
+
+failed=0
+
+# Test listing a share with valid users succeeds
+test_valid_users_access()
+{
+    tmpfile=$PREFIX/smbclient.in.$$
+    prompt="foo"
+    cat > $tmpfile <<EOF
+ls
+quit
+EOF
+
+    cmd='CLI_FORCE_INTERACTIVE=yes $SMBCLIENT -U$USERNAME%$PASSWORD "//$SERVER/$1" -I $SERVER_IP $ADDARGS < $tmpfile 2>&1'
+    eval echo "$cmd"
+    out=`eval $cmd`
+    ret=$?
+    rm -f $tmpfile
+
+    if [ $ret != 0 ] ; then
+        echo "$out"
+        echo "failed accessing share with valid users with error $ret"
+
+        false
+        return
+    fi
+
+    echo "$out" | grep "$prompt" >/dev/null 2>&1
+
+    ret=$?
+    if [ $ret = 0 ] ; then
+        # got the correct prompt .. succeed
+        true
+    else
+        echo "$out"
+        echo "failed listing share with valid users"
+        false
+    fi
+}
+
+testit "accessing a valid users share succeeds" \
+   test_valid_users_access valid-users-access || \
+   failed=`expr $failed + 1`
+
+exit $failed
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 1833b9f..58f2190 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -177,6 +177,7 @@ for env in ["nt4_dc"]:
 for env in ["fileserver"]:
     plantestsuite("samba3.blackbox.preserve_case (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_preserve_case.sh"), '$SERVER', '$DOMAIN', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3])
     plantestsuite("samba3.blackbox.dfree_command (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_dfree_command.sh"), '$SERVER', '$DOMAIN', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3])
+    plantestsuite("samba3.blackbox.valid_users (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_valid_users.sh"), '$SERVER', '$SERVER_IP', '$DOMAIN', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3])
 
     #
     # tar command tests
-- 
2.5.0.rc2.392.g76e840b



More information about the samba-technical mailing list