[PATCH] Some fixes for buildtools, flapping tests and libxcrypt support

Samuel Cabrero scabrero at suse.de
Thu Mar 21 08:46:16 UTC 2019


Hi,

please consider the following patches that fix several problems found
while debugging failed tests in latest openSUSE Tumbleweed:

* Fix "makedev()" detection in glibc >= 2.25
* Fix flapping samba3.blackbox.shadow_copy_torture
* Workaround uid wrapper issues when using bash shell
* Fix password_hash LDB module when crypt(_r) is provided by libxcrypt
* Fix password_hash_fl2008 test when using libxcrypt, it requires a
minumum of 1000 rounds for CryptSHA256

Gitlab pipeline:
https://gitlab.com/scabrero/samba/pipelines/52798218

Gitlab branch:
https://gitlab.com/scabrero/samba/commits/master-opensuse-ci

Cheers.



-------------- next part --------------
From f246e39bd9154b0a8af47ca95a804759fc5a5b18 Mon Sep 17 00:00:00 2001
From: Samuel Cabrero <scabrero at suse.de>
Date: Tue, 12 Mar 2019 18:53:04 +0100
Subject: [PATCH 1/5] waf: Include sys/sysmacros.h to check for makedev
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

From glibc 2.25 release notes:

  The inclusion of <sys/sysmacros.h> by <sys/types.h> is deprecated.
  This means that in a future release, the macros “major”, “minor”, and
  “makedev” will only be available from <sys/sysmacros.h>.

Signed-off-by: Samuel Cabrero <scabrero at suse.de>
---
 source3/torture/test_cleanup.c | 2 ++
 source3/wscript                | 1 +
 2 files changed, 3 insertions(+)

diff --git a/source3/torture/test_cleanup.c b/source3/torture/test_cleanup.c
index 8efdf35e080..9d69d5ef9db 100644
--- a/source3/torture/test_cleanup.c
+++ b/source3/torture/test_cleanup.c
@@ -281,6 +281,8 @@ bool run_cleanup3(int dummy)
 		{ create_duplicate_batch, "create_duplicate_batch" },
 	};
 
+	/* This test requires HAVE_MAKEDEV to get id.st_ex_rdev */
+
 	printf("CLEANUP3: Checking that a share mode is cleaned up on "
 	       "conflict\n");
 
diff --git a/source3/wscript b/source3/wscript
index e0db9839795..a83563fc4a5 100644
--- a/source3/wscript
+++ b/source3/wscript
@@ -1219,6 +1219,7 @@ main() {
 #include <unistd.h>
 #endif
 #include <sys/types.h>
+#include <sys/sysmacros.h>
 main() { dev_t dev = makedev(1,2); return 0; }
 ''',
         'HAVE_MAKEDEV',
-- 
2.21.0


From cd8edc51c46d88dd0a7f3c723202113b9c590248 Mon Sep 17 00:00:00 2001
From: Samuel Cabrero <scabrero at suse.de>
Date: Wed, 13 Mar 2019 10:46:36 +0100
Subject: [PATCH 2/5] s4:torture: Initialize tm struct

The samba3.blackbox.shadow_copy_torture tests call to strptime passing
an uninitalized tm structure as an argument, but the strptime function
does not write the tm.tm_isdst field.

These tm structures are passed later as the mktime argument, which
produces different values depending on whether the arbitrary value
of the tm.tm_isdst field is lower or equal to zero or greather than
zero.

Signed-off-by: Samuel Cabrero <scabrero at suse.de>
---
 source4/torture/smb2/create.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/source4/torture/smb2/create.c b/source4/torture/smb2/create.c
index 912efaac272..ee0b334ff54 100644
--- a/source4/torture/smb2/create.c
+++ b/source4/torture/smb2/create.c
@@ -1759,6 +1759,11 @@ static bool test_twrp_write(struct torture_context *tctx, struct smb2_tree *tree
 	torture_comment(tctx, "Testing timewarp (%s) (%s)\n", file, snapshot);
 
 	setenv("TZ", "GMT", 1);
+
+	/* strptime does not set tm.tm_isdst but mktime assumes DST is in
+	 * effect if it is greather than 1. */
+	memset(&tm, 0, sizeof(struct tm));
+
 	p = strptime(snapshot, "@GMT-%Y.%m.%d-%H.%M.%S", &tm);
 	torture_assert_goto(tctx, p != NULL, ret, done, "strptime\n");
 	torture_assert_goto(tctx, *p == '\0', ret, done, "strptime\n");
@@ -1858,6 +1863,11 @@ static bool test_twrp_stream(struct torture_context *tctx,
 	torture_assert_not_null_goto(tctx, buf, ret, done, "buf\n");
 
 	setenv("TZ", "GMT", 1);
+
+	/* strptime does not set tm.tm_isdst but mktime assumes DST is in
+	 * effect if it is greather than 1. */
+	memset(&tm, 0, sizeof(struct tm));
+
 	p = strptime(snapshot, "@GMT-%Y.%m.%d-%H.%M.%S", &tm);
 	torture_assert_goto(tctx, p != NULL, ret, done, "strptime\n");
 	torture_assert_goto(tctx, *p == '\0', ret, done, "strptime\n");
-- 
2.21.0


From 7209bfc1a00790eefb90d2a7f5c5a010bd80946a Mon Sep 17 00:00:00 2001
From: Samuel Cabrero <scabrero at suse.de>
Date: Thu, 14 Mar 2019 10:20:52 +0100
Subject: [PATCH 3/5] selftest: Woraround uid wrapper issues when using bash
 shell

UID_WRAPPER_ROOT=1 is not working properly when tests run in bash shell
instead of dash. After some debugging the reason may be dash spawns a
subshell to run commands, but bash calls execve instead. Traces attached
as reference:

/bin/sh -> dash:

[2(2)/2 at 17s, 1 errors] samba.blackbox.pdbtest(nt4_dc)(nt4_dc:local)
UWRAP_DEBUG(3145) - uwrap_init: Initialize uid_wrapper
UWRAP_DEBUG(3145) - uwrap_init_env: uwrap_init_env
UWRAP_DEBUG(3145) - uwrap_init: Enabled uid_wrapper as root (real uid=1000)
UWRAP_DEBUG(3145) - uwrap_init: Successfully initialized uid_wrapper
UWRAP_DEBUG(3144) - uwrap_init: Initialize uid_wrapper
UWRAP_DEBUG(3144) - uwrap_init_env: uwrap_init_env
UWRAP_DEBUG(3144) - uwrap_init: Enabled uid_wrapper as root (real uid=1000)
UWRAP_DEBUG(3144) - uwrap_init: Successfully initialized uid_wrapper

/bin/sh -> bash:

[2(2)/2 at 17s, 1 errors] samba.blackbox.pdbtest(nt4_dc)(nt4_dc:local)
UWRAP_DEBUG(3352) - uwrap_export_ids: uwrap_export_ids
UWRAP_DEBUG(3354) - uwrap_export_ids: uwrap_export_ids
UWRAP_DEBUG(3354) - uwrap_init: Initialize uid_wrapper
UWRAP_DEBUG(3354) - uwrap_init_env: uwrap_init_env
UWRAP_DEBUG(3354) - uwrap_init_env: Initialize ruid with 1000
UWRAP_DEBUG(3354) - uwrap_init_env: Initalize euid with 1000
UWRAP_DEBUG(3354) - uwrap_init_env: Initalize suid with 1000
UWRAP_DEBUG(3354) - uwrap_init_env: Initialize ruid with 1000
UWRAP_DEBUG(3354) - uwrap_init_env: Initalize egid with 1000
UWRAP_DEBUG(3354) - uwrap_init_env: Initalize sgid with 1000
UWRAP_DEBUG(3354) - uwrap_init_env: Initalize groups with 4,24,27,30,46,108,1000
UWRAP_DEBUG(3354) - uwrap_init: Enabled uid_wrapper as user (real uid=1000)
UWRAP_DEBUG(3354) - uwrap_init: Successfully initialized uid_wrapper
UWRAP_DEBUG(3353) - uwrap_export_ids: uwrap_export_ids
UWRAP_DEBUG(3353) - uwrap_init: Initialize uid_wrapper
UWRAP_DEBUG(3353) - uwrap_init_env: uwrap_init_env
UWRAP_DEBUG(3353) - uwrap_init_env: Initialize ruid with 1000
UWRAP_DEBUG(3353) - uwrap_init_env: Initalize euid with 1000
UWRAP_DEBUG(3353) - uwrap_init_env: Initalize suid with 1000
UWRAP_DEBUG(3353) - uwrap_init_env: Initialize ruid with 1000
UWRAP_DEBUG(3353) - uwrap_init_env: Initalize egid with 1000
UWRAP_DEBUG(3353) - uwrap_init_env: Initalize sgid with 1000
UWRAP_DEBUG(3353) - uwrap_init_env: Initalize groups with 4,24,27,30,46,108,1000
UWRAP_DEBUG(3353) - uwrap_init: Enabled uid_wrapper as user (real uid=1000)
UWRAP_DEBUG(3353) - uwrap_init: Successfully initialized uid_wrapper

Signed-off-by: Samuel Cabrero <scabrero at suse.de>
---
 source3/script/tests/test_net_rpc_oldjoin.sh | 23 +++++++++++-
 testprogs/blackbox/test_password_settings.sh | 35 +++++++++++++-----
 testprogs/blackbox/test_pdbtest.sh           | 37 +++++++++++++++-----
 3 files changed, 78 insertions(+), 17 deletions(-)

diff --git a/source3/script/tests/test_net_rpc_oldjoin.sh b/source3/script/tests/test_net_rpc_oldjoin.sh
index 070fcc1add7..97586874be9 100755
--- a/source3/script/tests/test_net_rpc_oldjoin.sh
+++ b/source3/script/tests/test_net_rpc_oldjoin.sh
@@ -22,8 +22,29 @@ export UID_WRAPPER_ROOT
 
 OPTIONS="--configfile $SMB_CONF_PATH --option=netbiosname=$maccount --option=security=domain --option=domainlogons=no --option=privatedir=$privatedir"
 
+test_smbpasswd()
+{
+	account=$1
+
+	echo "set password with smbpasswd"
+
+	cmd='UID_WRAPPER_INITIAL_RUID=0 UID_WRAPPER_INITIAL_EUID=0 $VALGRIND $BINDIR/smbpasswd -L -c $SMB_CONF_PATH -a -m "$account"'
+	eval echo "$cmd"
+	out=$(eval $cmd)
+	ret=$?
+	rm -f $tmpfile
+
+	if [ $ret -ne 0 ]; then
+		echo "Failed to change user password $user"
+		return 1
+	fi
+}
+
+
 testit "mkdir -p $privatedir" mkdir -p $privatedir || failed=`expr $failed + 1`
-testit "smbpasswd -a -m" $VALGRIND $BINDIR/smbpasswd -L -c $SMB_CONF_PATH -a -m "$maccount" || failed=`expr $failed + 1`
+testit "smbpasswd -a -m" \
+	test_smbpasswd $maccount \
+	|| failed=$(expr $failed + 1)
 testit "net_rpc_oldjoin" $VALGRIND $BINDIR/net rpc oldjoin -S $SERVER $OPTIONS || failed=`expr $failed + 1`
 testit "net_rpc_testjoin1" $VALGRIND $BINDIR/net rpc testjoin -S $SERVER $OPTIONS || failed=`expr $failed + 1`
 testit "net_rpc_changetrustpw" $VALGRIND $BINDIR/net rpc changetrustpw -S $SERVER $OPTIONS || failed=`expr $failed + 1`
diff --git a/testprogs/blackbox/test_password_settings.sh b/testprogs/blackbox/test_password_settings.sh
index 93e03770ed2..600bf0d0751 100755
--- a/testprogs/blackbox/test_password_settings.sh
+++ b/testprogs/blackbox/test_password_settings.sh
@@ -52,6 +52,31 @@ do_kinit() {
 	fi
 }
 
+test_smbpasswd()
+{
+	user=$1
+	newpass=$2
+
+	tmpfile=$PREFIX/smbpasswd_change_password_script
+	cat > $tmpfile <<EOF
+expect New SMB password:
+send ${newpass}\n
+expect Retype new SMB password:
+send ${newpass}\n
+EOF
+
+	cmd='UID_WRAPPER_INITIAL_RUID=0 UID_WRAPPER_INITIAL_EUID=0 $texpect $tmpfile $smbpasswd -L -c $PREFIX/etc/smb.conf $user'
+	eval echo "$cmd"
+	out=$(eval $cmd)
+	ret=$?
+	rm -f $tmpfile
+
+	if [ $ret -ne 0 ]; then
+		echo "Failed to change user password $user"
+		return 1
+	fi
+}
+
 UID_WRAPPER_ROOT=1
 export UID_WRAPPER_ROOT
 
@@ -139,15 +164,9 @@ rm -f $KRB5CCNAME_PATH
 ### Set the password with smbpasswd
 ###########################################################
 
-cat > $PREFIX/tmpsmbpasswdscript <<EOF
-expect New SMB password:
-send ${TEST_PASSWORD_NEW}\n
-expect Retype new SMB password:
-send ${TEST_PASSWORD_NEW}\n
-EOF
-
 testit "set user password with smbpasswd" \
-	$texpect $PREFIX/tmpsmbpasswdscript $smbpasswd -L -c $PREFIX/etc/smb.conf $TEST_USERNAME || failed=`expr $failed + 1`
+	test_smbpasswd $TEST_USERNAME $TEST_PASSWORD_NEW \
+	|| failed=$(expr $failed + 1)
 
 TEST_PASSWORD=$TEST_PASSWORD_NEW
 TEST_PASSWORD_NEW="testPaSS at 03%"
diff --git a/testprogs/blackbox/test_pdbtest.sh b/testprogs/blackbox/test_pdbtest.sh
index 02615094451..d5c37f0acc9 100755
--- a/testprogs/blackbox/test_pdbtest.sh
+++ b/testprogs/blackbox/test_pdbtest.sh
@@ -32,6 +32,32 @@ unc="//$SERVER/tmp"
 UID_WRAPPER_ROOT=1
 export UID_WRAPPER_ROOT
 
+test_smbpasswd()
+{
+	user=$1
+	newpass=$2
+
+	echo "set password with smbpasswd"
+	tmpfile=$PREFIX/smbpasswd_change_password_script
+	cat > $tmpfile <<EOF
+expect New SMB password:
+send ${newpass}\n
+expect Retype new SMB password:
+send ${newpass}\n
+EOF
+
+	cmd='UID_WRAPPER_INITIAL_RUID=0 UID_WRAPPER_INITIAL_EUID=0 $texpect $tmpfile $smbpasswd -L $user -c $SMB_CONF'
+	eval echo "$cmd"
+	out=$(eval $cmd)
+	ret=$?
+	rm -f $tmpfile
+
+	if [ $ret -ne 0 ]; then
+		echo "Failed to change user password $user"
+		return 1
+	fi
+}
+
 testit "pdbtest" $VALGRIND $BINDIR/pdbtest -u $USER $@ || failed=`expr $failed + 1`
 
 NEWUSERPASS=testPaSS at 01%
@@ -55,15 +81,10 @@ test_smbclient "Test login with user (ntlm)" 'ls' "$unc" -k no -U$USER%$NEWUSERP
 
 NEWUSERPASS=testPaSS at 02%
 
-echo "set password with smbpasswd"
-cat > ./tmpsmbpasswdscript <<EOF
-expect New SMB password:
-send ${NEWUSERPASS}\n
-expect Retype new SMB password:
-send ${NEWUSERPASS}\n
-EOF
+testit "set user password with smbpasswd" \
+	test_smbpasswd $USER $NEWUSERPASS \
+	|| failed=$(expr $failed + 1)
 
-testit "set user password with smbpasswd" $texpect ./tmpsmbpasswdscript $smbpasswd -L $USER -c $SMB_CONF || failed=`expr $failed + 1`
 USERPASS=$NEWUSERPASS
 
 test_smbclient "Test login with user (ntlm)" 'ls' "$unc" -k no -U$USER%$NEWUSERPASS $@|| failed=`expr $failed + 1`
-- 
2.21.0


From 4f2927972ec419bbbc7a276ba2d5ac16c2041ced Mon Sep 17 00:00:00 2001
From: Samuel Cabrero <scabrero at suse.de>
Date: Thu, 14 Mar 2019 18:54:20 +0100
Subject: [PATCH 4/5] s4:dsdb: Check errno to determine if crypt or crypt_r
 succeeded

The behavior of these functions upon errors depends on the implementation.
The GNU libc implementation seems to return a null hash, but others like
libxcrypt returns a invalid hash string '*0'.

Signed-off-by: Samuel Cabrero <scabrero at suse.de>
---
 source4/dsdb/samdb/ldb_modules/password_hash.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/source4/dsdb/samdb/ldb_modules/password_hash.c b/source4/dsdb/samdb/ldb_modules/password_hash.c
index 804be6a4307..70b63bbd8a7 100644
--- a/source4/dsdb/samdb/ldb_modules/password_hash.c
+++ b/source4/dsdb/samdb/ldb_modules/password_hash.c
@@ -1484,6 +1484,7 @@ static int setup_primary_userPassword_hash(
 	 * Relies on the assertion that cleartext_utf8->data is a zero
 	 * terminated UTF-8 string
 	 */
+	errno = 0;
 #ifdef HAVE_CRYPT_R
 	hash = crypt_r((char *)io->n.cleartext_utf8->data, cmd, &crypt_data);
 #else
@@ -1493,7 +1494,10 @@ static int setup_primary_userPassword_hash(
 	 */
 	hash = crypt((char *)io->n.cleartext_utf8->data, cmd);
 #endif
-	if (hash == NULL) {
+	/* crypt_r and crypt may return a null pointer upon error depending on
+	 * how libcrypt was configured. POSIX specifies returning a null
+	 * pointer and setting errno. */
+	if (hash == NULL || errno) {
 		char buf[1024];
 		int err = strerror_r(errno, buf, sizeof(buf));
 		if (err != 0) {
-- 
2.21.0


From 9d723dc79f8009656afdad01b9678cb24c33fdca Mon Sep 17 00:00:00 2001
From: Samuel Cabrero <scabrero at suse.de>
Date: Thu, 14 Mar 2019 19:14:25 +0100
Subject: [PATCH 5/5] selftests:password_hash: Raise SHA256 rounds to 5000

Some crypt_r implementations like libxcrypt require a higher value.

Signed-off-by: Samuel Cabrero <scabrero at suse.de>
---
 python/samba/tests/password_hash_fl2008.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/python/samba/tests/password_hash_fl2008.py b/python/samba/tests/password_hash_fl2008.py
index 917e973cdbc..685c73edaf2 100644
--- a/python/samba/tests/password_hash_fl2008.py
+++ b/python/samba/tests/password_hash_fl2008.py
@@ -158,7 +158,7 @@ class PassWordHashFl2008Tests(PassWordHashTests):
     def test_userPassword_cleartext_sha256(self):
         self.add_user(clear_text=True,
                       options=[("password hash userPassword schemes",
-                                "CryptSHA256:rounds=100")])
+                                "CryptSHA256:rounds=5000")])
 
         sc = self.get_supplemental_creds()
 
@@ -206,5 +206,5 @@ class PassWordHashFl2008Tests(PassWordHashTests):
         #
         up = ndr_unpack(drsblobs.package_PrimaryUserPasswordBlob,
                         binascii.a2b_hex(up_package.data))
-        self.checkUserPassword(up, [("{CRYPT}", "5", 100)])
+        self.checkUserPassword(up, [("{CRYPT}", "5", 5000)])
         self.checkNtHash(USER_PASS, up.current_nt_hash.hash)
-- 
2.21.0



More information about the samba-technical mailing list