[PATCH] DRAFT: Add new test which covers force user bug

Michael Adam obnox at samba.org
Thu Jan 7 08:43:17 UTC 2016


Hi Robin,

Thanks a lot for working on the testsuite!!

Some comments:

- Have you run a full 'make test' with your patch?
  (You should, in this case, since it modifies the test
  environment...)

- I don't quite understand the patch yet:
  See comments inline below.


On 2016-01-04 at 14:35 +0100, Robin Hack wrote:
> Patch is here :).
> 
> On Mon, Jan 4, 2016 at 2:28 PM, Robin Hack <rhack at redhat.com> wrote:
> 
> > Hi Jeremy.
> >
> > Sorry for such a big delay. I was on vacation. And thank you very much for
> > heads up.
> >
> > I looked at issue which you described and I hope that I fixed it.
> >
> > In previous case I used $unix_gids[0] for $unix_name user.
> > I use newly generated gid now.
> >
> > Could you please check and review my changes again and test them? I ran
> > $ make test TESTS=samba3.local.nss
> > and issue seems gone. I'm not sure if other issues will not raise.
> >
> > Sorry for issues with my patch. I didn't realized how fragile it can be :(.
> >
> >
> > Have nice day
> > Robin Hack


> From cae478de1f577b181eab68f0bef7d6e49ba1c1a7 Mon Sep 17 00:00:00 2001
> From: Robin Hack <rhack at redhat.com>
> Date: Fri, 4 Dec 2015 14:50:56 +0100
> Subject: [PATCH] samba3.blackbox.smbclient.forceuser: Add new test for force
>  user option.
> 
> Test covers commit
> a85d6eb355e8277fe69db6e5837a7f65f57d9c1d
> 
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=9878
> RH BUG: https://bugzilla.redhat.com/show_bug.cgi?id=1077651
> 
> Signed-off-by: Robin Hack <rhack at redhat.com>
> ---
>  selftest/target/Samba3.pm              | 31 ++++++++++++++++
>  source3/script/tests/test_forceuser.sh | 66 ++++++++++++++++++++++++++++++++++
>  source3/selftest/tests.py              |  6 ++--
>  3 files changed, 101 insertions(+), 2 deletions(-)
>  create mode 100755 source3/script/tests/test_forceuser.sh
> 
> diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
> index 94612b2..ccb2751 100755
> --- a/selftest/target/Samba3.pm
> +++ b/selftest/target/Samba3.pm
> @@ -790,11 +790,36 @@ $ret->{USERNAME} = KTEST\\Administrator
>  sub setup_maptoguest($$)
>  {
>  	my ($self, $path) = @_;
> +	my $prefix_abs = abs_path($path);
>  
>  	print "PROVISIONING maptoguest...";
>  
> +	my @dirs = ();
> +
> +	mkdir($prefix_abs, 0777);
> +	my $share_dir="$prefix_abs/share";
> +	push(@dirs, $share_dir);
> +
> +	my $force_user_not_work_dir = "$share_dir/force_user_not_work";
> +	push(@dirs, $force_user_not_work_dir);
> +
> +	my $unix_name = ($ENV{USER} or $ENV{LOGNAME} or `PATH=/usr/ucb:$ENV{PATH} whoami`);
> +	chomp $unix_name;
> +
> +
>  	my $options = "
>  map to guest = bad user
> +
> +# BUG: https://bugzilla.samba.org/show_bug.cgi?id=9878
> +# BUG: https://bugzilla.redhat.com/show_bug.cgi?id=1077651

These bugs are not at all related to map to guest.
But your test and comments seem to suggest that.

I think you should either remove the references to map-to-guest
and do it (maybe additionally?) with a non-map-to-guest share,
or else you should remove the above BZ references.

> +[force_user_not_work]
> +	path = $force_user_not_work_dir
> +        comment = force user test share with map to guest set to bad user
> +	valid users = \@$unix_name
> +	force user = $unix_name
> +	force group = everyone
> +	write list = $unix_name
> +	create mask = 0644
>  ";
>  
>  	my $vars = $self->provision($path,
> @@ -804,6 +829,9 @@ map to guest = bad user
>  
>  	$vars or return undef;
>  
> +	mkdir($_, 0777) foreach(@dirs);
> +	chmod(0777, $_) foreach(@dirs);
> +
>  	if (not $self->check_or_start($vars, "yes", "no", "yes")) {
>  	       return undef;
>  	}
> @@ -1231,6 +1259,7 @@ sub provision($$$$$$$$)
>  	my ($uid_pdbtest_wkn);
>  	my ($gid_nobody, $gid_nogroup, $gid_root, $gid_domusers, $gid_domadmins);
>  	my ($gid_userdup, $gid_everyone);
> +	my ($gid_same_name);
>  
>  	if ($unix_uid < 0xffff - 5) {
>  		$max_uid = 0xffff;
> @@ -1258,6 +1287,7 @@ sub provision($$$$$$$$)
>  	$gid_domadmins = $max_gid - 5;
>  	$gid_userdup = $max_gid - 6;
>  	$gid_everyone = $max_gid - 7;
> +	$gid_same_name = $max_gid - 8;
>  
>  	##
>  	## create conffile
> @@ -1551,6 +1581,7 @@ pdbtest_wkn:x:$uid_pdbtest_wkn:$gid_everyone:pdbtest_wkn gecos:$prefix_abs:/bin/
>          }
>  	print GROUP "nobody:x:$gid_nobody:
>  nogroup:x:$gid_nogroup:nobody
> +$unix_name:x:$gid_same_name:
>  $unix_name-group:x:$unix_gids[0]:
>  domusers:X:$gid_domusers:
>  domadmins:X:$gid_domadmins:
> diff --git a/source3/script/tests/test_forceuser.sh b/source3/script/tests/test_forceuser.sh
> new file mode 100755
> index 0000000..46fe251
> --- /dev/null
> +++ b/source3/script/tests/test_forceuser.sh
> @@ -0,0 +1,66 @@
> +#!/bin/sh
> +#
> +# Blackbox test for share with force user settings
> +#
> +
> +if [ $# -lt 6 ]; then
> +cat <<EOF
> +Usage: test_forceuser.sh SERVER DOMAIN USERNAME PASSWORD LOCAL_PATH SMBCLIENT <smbclient arguments>
> +EOF
> +exit 1;
> +fi
> +
> +SERVER="$1"
> +DOMAIN="$2"
> +USERNAME="$3"
> +PASSWORD="$4"
> +LOCAL_PATH="$5"
> +SMBCLIENT="$6"
> +SMBCLIENT="$VALGRIND ${SMBCLIENT}"
> +shift 6
> +ADDARGS="$*"
> +failed=0
> +
> +
> +incdir=`dirname $0`/../../../testprogs/blackbox
> +. $incdir/subunit.sh
> +
> +
> +run_cmd_nooutput() {
> +	CMD="$1"
> +
> +	out=`eval ${CMD} &> TESTOUT`
> +	if [ $? != 0 ] ; then
> +		cat TESTOUT
> +		rm -f TESTOUT
> +		echo "command failed"
> +		false
> +		return
> +	fi
> +
> +	rm -f TESTOUT
> +	true
> +	return
> +}
> +
> +test_force_user_not_work()
> +{
> +	SMB_SHARE="force_user_not_work"
> +	run_cmd_nooutput "${SMBCLIENT} //${SERVER}/${SMB_SHARE} -U$USERNAME%$PASSWORD -c 'put TESTFILE'"
> +}
> +
> +# Setup
> +pushd "${LOCAL_PATH}" > /dev/null
> +rm -f TESTFILE
> +echo "hello samba!" > TESTFILE
> +
> +# Test
> +testit "force user not works when map to guest is set" \
> +	test_force_user_not_work || failed=`expr $failed + 1`

I do not quite understand the logic here yet:

The above seems to test that the smbclient command succeeds.
But the name and comments suggest that it tests failure...

So I guess you should at least adapt the message and function
name? And the share name?

But again -- what is the real relationship yo 'map to guest'?

Thanks -- Michael




> +# Cleanup
> +rm -f TESTFILE
> +popd > /dev/null
> +
> +# Results
> +testok $0 $failed
> diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
> index 9c68943..0909da2 100755
> --- a/source3/selftest/tests.py
> +++ b/source3/selftest/tests.py
> @@ -154,8 +154,10 @@ plantestsuite("samba3.ntlm_auth.krb5(ktest:local)", "ktest:local", [os.path.join
>  for env in ["maptoguest", "simpleserver"]:
>      plantestsuite("samba3.blackbox.smbclient_auth.plain (%s) local creds" % env, env, [os.path.join(samba3srcdir, "script/tests/test_smbclient_auth.sh"), '$SERVER', '$SERVER_IP', '$USERNAME', '$PASSWORD', smbclient3, configuration + " --option=clientntlmv2auth=no --option=clientlanmanauth=yes"])
>  
> -env = "maptoguest"
> -plantestsuite("samba3.blackbox.smbclient_auth.plain (%s) bad username" % env, env, [os.path.join(samba3srcdir, "script/tests/test_smbclient_auth.sh"), '$SERVER', '$SERVER_IP', 'notmy$USERNAME', '$PASSWORD', smbclient3, configuration + " --option=clientntlmv2auth=no --option=clientlanmanauth=yes"])
> +for env in ["maptoguest"]:
> +    plantestsuite("samba3.blackbox.smbclient_auth.plain (%s) bad username" % env, env, [os.path.join(samba3srcdir, "script/tests/test_smbclient_auth.sh"), '$SERVER', '$SERVER_IP', 'notmy$USERNAME', '$PASSWORD', smbclient3, configuration + " --option=clientntlmv2auth=no --option=clientlanmanauth=yes"])
> +    plantestsuite("samba3.blackbox.smbclient.forceuser (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_forceuser.sh"), '$SERVER', '$DOMAIN', '$USERNAME', '$PASSWORD', '$LOCAL_PATH', smbclient3])
> +
>  
>  # plain
>  for env in ["nt4_dc"]:
> -- 
> 1.9.3
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160107/1255a637/signature.sig>


More information about the samba-technical mailing list