autobuild failure due to python replication tests - why ?

Jeremy Allison jra at samba.org
Wed Aug 3 16:14:38 UTC 2016


On Wed, Aug 03, 2016 at 05:33:55PM +1200, Andrew Bartlett wrote:
> On Tue, 2016-08-02 at 16:37 -0700, Jeremy Allison wrote:
> > On Tue, Aug 02, 2016 at 05:13:34PM +1200, Andrew Bartlett wrote:
> > > 
> > > On Mon, 2016-08-01 at 19:47 -0700, Jeremy Allison wrote:
> > > > 
> > > > On Mon, Aug 01, 2016 at 07:32:03PM -0700, Jeremy Allison wrote:
> > > > > 
> > > > > 
> > > > > Then the replication tests and ctdb tests NEED TO BE REMOVED
> > > > > NOW !
> > > > 
> > > > Apologies. Error EIAMTOOMUCHOFANASSHOLE :-).
> > > > 
> > > > That was a little too strong. However I hope you understand
> > > > my frustrations with this, we really do need to get a fix
> > > > for this sooner (this week ?) rather than later. This week
> > > > please ?
> > > > 
> > > > In the meantime I'll resubmit, but that can't be the long
> > > > term solution.
> > > 
> > > I'm running tests to see if
> > > reverting 055544321389de5d5cea8e129e0923369e9f558b or
> > > e0b6d6bb10479759905932ea5438fc57713f0a8f helps, as this got much
> > > worse
> > > just as we forked for 4.5, which is where this commit was.
> > > 
> > > It may simply be that we don't correctly notice the new schema, or
> > > the
> > > need for new schema to replicate schema.
> > 
> > Unfortunately the autobuild failed again. That's 3 times in a row.
> > I'm sorry but these replication tests have completely broken
> > development on Samba, and we can't continue with this.
> 
> Did you try with the tests referenced by those commits skipped, or with
> those patches reverted?

Just to be clear, here is the patchset I'm trying to push at the moment
(attached).

Do you want me to try with 055544321389de5d5cea8e129e0923369e9f558b
and e0b6d6bb10479759905932ea5438fc57713f0a8f reverted also ?
-------------- next part --------------
From fc657228c6b8d3b0e2f69926985351e2801af71d Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 20 Jul 2016 10:50:14 +0200
Subject: [PATCH 01/13] pyrpc: Fix CID 1364169 Explicit null dereferenced

Signed-off-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
 source4/librpc/rpc/pyrpc_util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source4/librpc/rpc/pyrpc_util.c b/source4/librpc/rpc/pyrpc_util.c
index a9807a8..95443f7 100644
--- a/source4/librpc/rpc/pyrpc_util.c
+++ b/source4/librpc/rpc/pyrpc_util.c
@@ -234,7 +234,7 @@ PyObject *py_dcerpc_interface_init_helper(PyTypeObject *type, PyObject *args, Py
 	}
 
 	/* reset timeout for the handle */
-	if (timeout != ((unsigned int)-1)) {
+	if ((timeout != ((unsigned int)-1)) && (ret->binding_handle != NULL)) {
 		dcerpc_binding_handle_set_timeout(ret->binding_handle, timeout);
 	}
 
-- 
2.8.0.rc3.226.g39d4020


From 3415fe88f193e320d471b5b67c5505bd9fa53a44 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 29 Jul 2016 10:53:20 +0200
Subject: [PATCH 02/13] ctdb: Fix uninitialized variable warnings

Signed-off-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Martin Schwenke <martin at meltin.net>
---
 ctdb/tools/ctdb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ctdb/tools/ctdb.c b/ctdb/tools/ctdb.c
index d0381a5..0497e89 100644
--- a/ctdb/tools/ctdb.c
+++ b/ctdb/tools/ctdb.c
@@ -4951,7 +4951,7 @@ static int control_setlmasterrole(TALLOC_CTX *mem_ctx,
 				  struct ctdb_context *ctdb,
 				  int argc, const char **argv)
 {
-	uint32_t lmasterrole;
+	uint32_t lmasterrole = 0;
 	int ret;
 
 	if (argc != 1) {
@@ -4979,7 +4979,7 @@ static int control_setrecmasterrole(TALLOC_CTX *mem_ctx,
 				    struct ctdb_context *ctdb,
 				    int argc, const char **argv)
 {
-	uint32_t recmasterrole;
+	uint32_t recmasterrole = 0;
 	int ret;
 
 	if (argc != 1) {
-- 
2.8.0.rc3.226.g39d4020


From 7e41a90cc419a815137a9fb7703697b8c1452347 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 29 Jul 2016 10:54:39 +0200
Subject: [PATCH 03/13] lib: Fix a pointless error check

According to susv4, addr.s6_addr is a

uint8_t s6_addr[16]

which is always != 0

Signed-off-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Martin Schwenke <martin at meltin.net>
---
 source4/lib/socket/socket_ip.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/source4/lib/socket/socket_ip.c b/source4/lib/socket/socket_ip.c
index d531053..6ec5252 100644
--- a/source4/lib/socket/socket_ip.c
+++ b/source4/lib/socket/socket_ip.c
@@ -861,7 +861,8 @@ static NTSTATUS ipv6_sendto(struct socket_context *sock,
 		
 		ZERO_STRUCT(srv_addr);
 		addr                     = interpret_addr6(dest_addr->addr);
-		if (addr.s6_addr == 0) {
+		if (memcmp(&addr.s6_addr, &in6addr_any,
+			   sizeof(addr.s6_addr)) == 0) {
 			return NT_STATUS_HOST_UNREACHABLE;
 		}
 		srv_addr.sin6_addr = addr;
-- 
2.8.0.rc3.226.g39d4020


From 3461c8cca14b8d54ee39d0b5908b27402654c460 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 29 Jul 2016 14:00:10 +0200
Subject: [PATCH 04/13] ldb: Fix two signed/unsigned hickups

Signed-off-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
 lib/ldb/common/ldb_dn.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/ldb/common/ldb_dn.c b/lib/ldb/common/ldb_dn.c
index 3fa5ab5..b23ee17 100644
--- a/lib/ldb/common/ldb_dn.c
+++ b/lib/ldb/common/ldb_dn.c
@@ -1699,7 +1699,7 @@ bool ldb_dn_remove_child_components(struct ldb_dn *dn, unsigned int num)
  */
 bool ldb_dn_replace_components(struct ldb_dn *dn, struct ldb_dn *new_dn)
 {
-	int i;
+	unsigned int i;
 
 	if ( ! ldb_dn_validate(dn) || ! ldb_dn_validate(new_dn)) {
 		return false;
@@ -1904,11 +1904,11 @@ int ldb_dn_set_component(struct ldb_dn *dn, int num,
 		return LDB_ERR_OTHER;
 	}
 
-	if (num >= dn->comp_num) {
+	if (num < 0) {
 		return LDB_ERR_OTHER;
 	}
 
-	if (num < 0) {
+	if ((unsigned)num >= dn->comp_num) {
 		return LDB_ERR_OTHER;
 	}
 
-- 
2.8.0.rc3.226.g39d4020


From 73693421b4878b9a1ba4a8b487cf3405d8d74483 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Tue, 2 Aug 2016 13:16:35 +1200
Subject: [PATCH 05/13] torture/backupkey: Allow WERR_INVALID_ACCESS,
 WERR_INVALID_PARAM or WERR_INVALID_DATA

The use of the wrong key can still create structures that parse as a SID,
therefore we can sometimes get an unusual error, which becomes a flapping test

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
 source4/torture/rpc/backupkey_heimdal.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/source4/torture/rpc/backupkey_heimdal.c b/source4/torture/rpc/backupkey_heimdal.c
index c0db48d..370700a 100644
--- a/source4/torture/rpc/backupkey_heimdal.c
+++ b/source4/torture/rpc/backupkey_heimdal.c
@@ -1979,10 +1979,11 @@ static bool test_ServerWrap_decrypt_wrong_stuff(struct torture_context *tctx,
 					  WERR_INVALID_ACCESS,
 					  "decrypt should fail with WERR_INVALID_ACCESS");
 	} else {
-		if (!W_ERROR_EQUAL(r.out.result, WERR_INVALID_PARAM)) {
+		if (!W_ERROR_EQUAL(r.out.result, WERR_INVALID_ACCESS)
+		    && !W_ERROR_EQUAL(r.out.result, WERR_INVALID_PARAM)) {
 			torture_assert_werr_equal(tctx, r.out.result,
 						  WERR_INVALID_DATA,
-						  "decrypt should fail with WERR_INVALID_PARAM or WERR_INVALID_DATA");
+						  "decrypt should fail with WERR_INVALID_ACCESS, WERR_INVALID_PARAM or WERR_INVALID_DATA");
 		}
 	}
 
-- 
2.8.0.rc3.226.g39d4020


From c702c992985268704256eb66ef1f9da97576aa1e Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Tue, 2 Aug 2016 13:20:46 +1200
Subject: [PATCH 06/13] selftest: Merge alternate error codes into backupkey
 from backupkey_heimdal

This is from cea4a4b9b22c78f9736e2290d302a88644db4031 and
613d085a63ee554084cb99d2150921dd108f6b77

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
 source4/torture/rpc/backupkey.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/source4/torture/rpc/backupkey.c b/source4/torture/rpc/backupkey.c
index ca981c2..276fa7f 100644
--- a/source4/torture/rpc/backupkey.c
+++ b/source4/torture/rpc/backupkey.c
@@ -2217,10 +2217,12 @@ static bool test_ServerWrap_decrypt_wrong_stuff(struct torture_context *tctx,
 					  WERR_INVALID_ACCESS,
 					  "decrypt should fail with WERR_INVALID_ACCESS");
 	} else {
-		torture_assert_werr_equal(tctx,
-					  r.out.result,
-					  WERR_INVALID_PARAM,
-					  "decrypt should fail with WERR_INVALID_PARAM");
+		if (!W_ERROR_EQUAL(r.out.result, WERR_INVALID_ACCESS)
+		    && !W_ERROR_EQUAL(r.out.result, WERR_INVALID_PARAM)) {
+			torture_assert_werr_equal(tctx, r.out.result,
+						  WERR_INVALID_DATA,
+						  "decrypt should fail with WERR_INVALID_ACCESS, WERR_INVALID_PARAM or WERR_INVALID_DATA");
+		}
 	}
 
 	/* Decrypt */
-- 
2.8.0.rc3.226.g39d4020


From 51226cd68180ce5b9ed21ce98f8e74347c072c05 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 2 Aug 2016 12:33:34 +0200
Subject: [PATCH 07/13] tests:samba_tool: pass stdout and stderr to
 assertCmdSuccess()

This allows us to generate better assert messages and give the
developer some ideas why the command wasn't able to run.

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
 python/samba/tests/samba_tool/base.py              |  5 ++--
 python/samba/tests/samba_tool/fsmo.py              |  2 +-
 python/samba/tests/samba_tool/gpo.py               | 14 +++++-----
 python/samba/tests/samba_tool/group.py             | 10 +++----
 python/samba/tests/samba_tool/ntacl.py             | 24 ++++++++--------
 python/samba/tests/samba_tool/processes.py         |  4 +--
 python/samba/tests/samba_tool/rodc.py              | 14 +++++-----
 python/samba/tests/samba_tool/sites.py             |  4 +--
 python/samba/tests/samba_tool/timecmd.py           |  2 +-
 python/samba/tests/samba_tool/user.py              | 32 ++++++++++------------
 .../tests/samba_tool/user_check_password_script.py |  8 +++---
 source4/torture/drs/python/fsmo.py                 |  2 +-
 12 files changed, 60 insertions(+), 61 deletions(-)

diff --git a/python/samba/tests/samba_tool/base.py b/python/samba/tests/samba_tool/base.py
index de6cf18..8f00534 100644
--- a/python/samba/tests/samba_tool/base.py
+++ b/python/samba/tests/samba_tool/base.py
@@ -81,8 +81,9 @@ class SambaToolCmdTest(samba.tests.BlackboxTestCase):
         result = cmd._run("samba-tool %s %s" % (name, sub), *args)
         return (result, cmd.outf.getvalue(), cmd.errf.getvalue())
 
-    def assertCmdSuccess(self, val, msg=""):
-        self.assertIsNone(val, msg)
+    def assertCmdSuccess(self, exit, out, err, msg=""):
+        self.assertIsNone(exit, msg="exit[%s] stdout[%s] stderr[%s]: %s" % (
+                          exit, out, err, msg))
 
     def assertCmdFail(self, val, msg=""):
         self.assertIsNotNone(val, msg)
diff --git a/python/samba/tests/samba_tool/fsmo.py b/python/samba/tests/samba_tool/fsmo.py
index 207aa17..c985830 100644
--- a/python/samba/tests/samba_tool/fsmo.py
+++ b/python/samba/tests/samba_tool/fsmo.py
@@ -25,7 +25,7 @@ class FsmoCmdTestCase(SambaToolCmdTest):
         """Run fsmo show to see if it errors"""
         (result, out, err) = self.runsubcmd("fsmo", "show")
 
-        self.assertCmdSuccess(result)
+        self.assertCmdSuccess(result, out, err)
         self.assertEquals(err,"","Shouldn't be any error messages")
 
         # Check that the output is sensible
diff --git a/python/samba/tests/samba_tool/gpo.py b/python/samba/tests/samba_tool/gpo.py
index e20a977..cd22706 100644
--- a/python/samba/tests/samba_tool/gpo.py
+++ b/python/samba/tests/samba_tool/gpo.py
@@ -30,7 +30,7 @@ class GpoCmdTestCase(SambaToolCmdTest):
     def test_gpo_list(self):
         """Run gpo list against the server and make sure it looks accurate"""
         (result, out, err) = self.runsubcmd("gpo", "listall", "-H", "ldap://%s" % os.environ["SERVER"])
-        self.assertCmdSuccess(result, "Ensuring gpo listall ran successfully")
+        self.assertCmdSuccess(result, out, err, "Ensuring gpo listall ran successfully")
 
     def test_fetchfail(self):
         """Run against a non-existent GPO, and make sure it fails (this hard-coded UUID is very unlikely to exist"""
@@ -40,23 +40,23 @@ class GpoCmdTestCase(SambaToolCmdTest):
     def test_fetch(self):
         """Run against a real GPO, and make sure it passes"""
         (result, out, err) = self.runsubcmd("gpo", "fetch", self.gpo_guid, "-H", "ldap://%s" % os.environ["SERVER"], "--tmpdir", self.tempdir)
-        self.assertCmdSuccess(result, "Ensuring gpo fetched successfully")
+        self.assertCmdSuccess(result, out, err, "Ensuring gpo fetched successfully")
         shutil.rmtree(os.path.join(self.tempdir, "policy"))
 
     def test_show(self):
         """Show a real GPO, and make sure it passes"""
         (result, out, err) = self.runsubcmd("gpo", "show", self.gpo_guid, "-H", "ldap://%s" % os.environ["SERVER"])
-        self.assertCmdSuccess(result, "Ensuring gpo fetched successfully")
+        self.assertCmdSuccess(result, out, err, "Ensuring gpo fetched successfully")
 
     def test_show_as_admin(self):
         """Show a real GPO, and make sure it passes"""
         (result, out, err) = self.runsubcmd("gpo", "show", self.gpo_guid, "-H", "ldap://%s" % os.environ["SERVER"], "-U%s%%%s" % (os.environ["USERNAME"], os.environ["PASSWORD"]))
-        self.assertCmdSuccess(result, "Ensuring gpo fetched successfully")
+        self.assertCmdSuccess(result, out, err, "Ensuring gpo fetched successfully")
 
     def test_aclcheck(self):
         """Check all the GPOs on the remote server have correct ACLs"""
         (result, out, err) = self.runsubcmd("gpo", "aclcheck", "-H", "ldap://%s" % os.environ["SERVER"], "-U%s%%%s" % (os.environ["USERNAME"], os.environ["PASSWORD"]))
-        self.assertCmdSuccess(result, "Ensuring gpo checked successfully")
+        self.assertCmdSuccess(result, out, err, "Ensuring gpo checked successfully")
 
     def setUp(self):
         """set up a temporary GPO to work with"""
@@ -66,7 +66,7 @@ class GpoCmdTestCase(SambaToolCmdTest):
                                             "-U%s%%%s" % (os.environ["USERNAME"], os.environ["PASSWORD"]),
                                             "--tmpdir", self.tempdir)
         shutil.rmtree(os.path.join(self.tempdir, "policy"))
-        self.assertCmdSuccess(result, "Ensuring gpo created successfully")
+        self.assertCmdSuccess(result, out, err, "Ensuring gpo created successfully")
         try:
             self.gpo_guid = "{%s}" % out.split("{")[1].split("}")[0]
         except IndexError:
@@ -75,5 +75,5 @@ class GpoCmdTestCase(SambaToolCmdTest):
     def tearDown(self):
         """remove the temporary GPO to work with"""
         (result, out, err) = self.runsubcmd("gpo", "del", self.gpo_guid, "-H", "ldap://%s" % os.environ["SERVER"], "-U%s%%%s" % (os.environ["USERNAME"], os.environ["PASSWORD"]))
-        self.assertCmdSuccess(result, "Ensuring gpo deleted successfully")
+        self.assertCmdSuccess(result, out, err, "Ensuring gpo deleted successfully")
         super(GpoCmdTestCase, self).tearDown()
diff --git a/python/samba/tests/samba_tool/group.py b/python/samba/tests/samba_tool/group.py
index 2c0c46e..09ba59d 100644
--- a/python/samba/tests/samba_tool/group.py
+++ b/python/samba/tests/samba_tool/group.py
@@ -43,7 +43,7 @@ class GroupCmdTestCase(SambaToolCmdTest):
         for group in self.groups:
             (result, out, err) = self._create_group(group)
 
-            self.assertCmdSuccess(result)
+            self.assertCmdSuccess(result, out, err)
             self.assertEquals(err, "", "There shouldn't be any error message")
             self.assertIn("Added group %s" % group["name"], out)
 
@@ -73,7 +73,7 @@ class GroupCmdTestCase(SambaToolCmdTest):
         # try to delete all the groups we just added
         for group in self.groups:
             (result, out, err) = self.runsubcmd("group", "delete", group["name"])
-            self.assertCmdSuccess(result,
+            self.assertCmdSuccess(result, out, err,
                                   "Failed to delete group '%s'" % group["name"])
             found = self._find_group(group["name"])
             self.assertIsNone(found,
@@ -87,7 +87,7 @@ class GroupCmdTestCase(SambaToolCmdTest):
                                                  "-U%s%%%s" % (os.environ["DC_USERNAME"],
                                                  os.environ["DC_PASSWORD"]))
 
-            self.assertCmdSuccess(result)
+            self.assertCmdSuccess(result, out, err)
             self.assertEquals(err,"","There shouldn't be any error message")
             self.assertIn("Added group %s" % group["name"], out)
 
@@ -102,7 +102,7 @@ class GroupCmdTestCase(SambaToolCmdTest):
                                             "-H", "ldap://%s" % os.environ["DC_SERVER"],
                                             "-U%s%%%s" % (os.environ["DC_USERNAME"],
                                                           os.environ["DC_PASSWORD"]))
-        self.assertCmdSuccess(result, "Error running list")
+        self.assertCmdSuccess(result, out, err, "Error running list")
 
         search_filter = "(objectClass=group)"
 
@@ -123,7 +123,7 @@ class GroupCmdTestCase(SambaToolCmdTest):
                                             "-H", "ldap://%s" % os.environ["DC_SERVER"],
                                             "-U%s%%%s" % (os.environ["DC_USERNAME"],
                                                           os.environ["DC_PASSWORD"]))
-        self.assertCmdSuccess(result, "Error running listmembers")
+        self.assertCmdSuccess(result, out, err, "Error running listmembers")
 
         search_filter = "(|(primaryGroupID=513)(memberOf=CN=Domain Users,CN=Users,%s))" % self.samdb.domain_dn()
 
diff --git a/python/samba/tests/samba_tool/ntacl.py b/python/samba/tests/samba_tool/ntacl.py
index 2a329fe..c01d5ac 100644
--- a/python/samba/tests/samba_tool/ntacl.py
+++ b/python/samba/tests/samba_tool/ntacl.py
@@ -31,7 +31,7 @@ class NtACLCmdSysvolTestCase(SambaToolCmdTest):
     def test_ntvfs(self):
         (result, out, err) =  self.runsubcmd("ntacl", "sysvolreset",
                                              "--use-ntvfs")
-        self.assertCmdSuccess(result)
+        self.assertCmdSuccess(result, out, err)
         self.assertEquals(out,"","Shouldn't be any output messages")
         self.assertIn("Please note that POSIX permissions have NOT been changed, only the stored NT ACL", err)
 
@@ -39,19 +39,19 @@ class NtACLCmdSysvolTestCase(SambaToolCmdTest):
         (result, out, err) =  self.runsubcmd("ntacl", "sysvolreset",
                                              "--use-s3fs")
 
-        self.assertCmdSuccess(result)
+        self.assertCmdSuccess(result, out, err)
         self.assertEquals(err,"","Shouldn't be any error messages")
         self.assertEquals(out,"","Shouldn't be any output messages")
 
     def test_ntvfs_check(self):
         (result, out, err) =  self.runsubcmd("ntacl", "sysvolreset",
                                              "--use-ntvfs")
-        self.assertCmdSuccess(result)
+        self.assertCmdSuccess(result, out, err)
         self.assertEquals(out,"","Shouldn't be any output messages")
         self.assertIn("Please note that POSIX permissions have NOT been changed, only the stored NT ACL", err)
         # Now check they were set correctly
         (result, out, err) =  self.runsubcmd("ntacl", "sysvolcheck")
-        self.assertCmdSuccess(result)
+        self.assertCmdSuccess(result, out, err)
         self.assertEquals(err,"","Shouldn't be any error messages")
         self.assertEquals(out,"","Shouldn't be any output messages")
 
@@ -59,13 +59,13 @@ class NtACLCmdSysvolTestCase(SambaToolCmdTest):
         (result, out, err) =  self.runsubcmd("ntacl", "sysvolreset",
                                              "--use-s3fs")
 
-        self.assertCmdSuccess(result)
+        self.assertCmdSuccess(result, out, err)
         self.assertEquals(err,"","Shouldn't be any error messages")
         self.assertEquals(out,"","Shouldn't be any output messages")
 
         # Now check they were set correctly
         (result, out, err) =  self.runsubcmd("ntacl", "sysvolcheck")
-        self.assertCmdSuccess(result)
+        self.assertCmdSuccess(result, out, err)
         self.assertEquals(err,"","Shouldn't be any error messages")
         self.assertEquals(out,"","Shouldn't be any output messages")
 
@@ -82,7 +82,7 @@ class NtACLCmdGetSetTestCase(SambaToolCmdTest):
 
         (result, out, err) =  self.runsubcmd("ntacl", "set", self.acl, tempf,
                                              "--use-ntvfs")
-        self.assertCmdSuccess(result)
+        self.assertCmdSuccess(result, out, err)
         self.assertEquals(out,"","Shouldn't be any output messages")
         self.assertIn("Please note that POSIX permissions have NOT been changed, only the stored NT ACL", err)
 
@@ -94,7 +94,7 @@ class NtACLCmdGetSetTestCase(SambaToolCmdTest):
         (result, out, err) =  self.runsubcmd("ntacl", "set", self.acl, tempf,
                                              "--use-s3fs")
 
-        self.assertCmdSuccess(result)
+        self.assertCmdSuccess(result, out, err)
         self.assertEquals(err,"","Shouldn't be any error messages")
         self.assertEquals(out,"","Shouldn't be any output messages")
 
@@ -105,14 +105,14 @@ class NtACLCmdGetSetTestCase(SambaToolCmdTest):
 
         (result, out, err) =  self.runsubcmd("ntacl", "set", self.acl, tempf,
                                              "--use-ntvfs")
-        self.assertCmdSuccess(result)
+        self.assertCmdSuccess(result, out, err)
         self.assertEquals(out,"","Shouldn't be any output messages")
         self.assertIn("Please note that POSIX permissions have NOT been changed, only the stored NT ACL", err)
 
         # Now check they were set correctly
         (result, out, err) =  self.runsubcmd("ntacl",  "get", tempf,
                                              "--use-ntvfs", "--as-sddl")
-        self.assertCmdSuccess(result)
+        self.assertCmdSuccess(result, out, err)
         self.assertEquals(err,"","Shouldn't be any error messages")
         self.assertEquals(self.acl+"\n", out, "Output should be the ACL")
 
@@ -123,13 +123,13 @@ class NtACLCmdGetSetTestCase(SambaToolCmdTest):
 
         (result, out, err) =  self.runsubcmd("ntacl", "set", self.acl, tempf,
                                              "--use-s3fs")
-        self.assertCmdSuccess(result)
+        self.assertCmdSuccess(result, out, err)
         self.assertEquals(out,"","Shouldn't be any output messages")
         self.assertEquals(err,"","Shouldn't be any error messages")
 
         # Now check they were set correctly
         (result, out, err) =  self.runsubcmd("ntacl",  "get", tempf,
                                              "--use-s3fs", "--as-sddl")
-        self.assertCmdSuccess(result)
+        self.assertCmdSuccess(result, out, err)
         self.assertEquals(err,"","Shouldn't be any error messages")
         self.assertEquals(self.acl+"\n", out,"Output should be the ACL")
diff --git a/python/samba/tests/samba_tool/processes.py b/python/samba/tests/samba_tool/processes.py
index 91a5266..5b8f502 100644
--- a/python/samba/tests/samba_tool/processes.py
+++ b/python/samba/tests/samba_tool/processes.py
@@ -27,9 +27,9 @@ class ProcessCmdTestCase(SambaToolCmdTest):
     def test_name(self):
         """Run processes command"""
         (result, out, err) = self.runcmd("processes", "--name", "samba")
-        self.assertCmdSuccess(result, "Ensuring processes ran successfully")
+        self.assertCmdSuccess(result, out, err, "Ensuring processes ran successfully")
 
     def test_all(self):
         """Run processes command"""
         (result, out, err) = self.runcmd("processes")
-        self.assertCmdSuccess(result, "Ensuring processes ran successfully")
+        self.assertCmdSuccess(result, out, err, "Ensuring processes ran successfully")
diff --git a/python/samba/tests/samba_tool/rodc.py b/python/samba/tests/samba_tool/rodc.py
index 798bc17..4851a53 100644
--- a/python/samba/tests/samba_tool/rodc.py
+++ b/python/samba/tests/samba_tool/rodc.py
@@ -66,26 +66,26 @@ class RodcCmdTestCase(SambaToolCmdTest):
     def test_single_by_account_name(self):
         (result, out, err) = self.runsubcmd("rodc", "preload", "sambatool1",
                                             "--server", os.environ["DC_SERVER"])
-        self.assertCmdSuccess(result, "ensuring rodc prefetch ran successfully")
+        self.assertCmdSuccess(result, out, err, "ensuring rodc prefetch ran successfully")
         self.assertEqual(out, "Replicating DN CN=sambatool1,CN=Users,%s\n" % self.base_dn)
         self.assertEqual(err, "")
 
     def test_single_by_dn(self):
         (result, out, err) = self.runsubcmd("rodc", "preload", "cn=sambatool2,cn=users,%s" % self.base_dn,
                                             "--server", os.environ["DC_SERVER"])
-        self.assertCmdSuccess(result, "ensuring rodc prefetch ran successfully")
+        self.assertCmdSuccess(result, out, err, "ensuring rodc prefetch ran successfully")
         self.assertEqual(out, "Replicating DN CN=sambatool2,CN=Users,%s\n" % self.base_dn)
 
     def test_multi_by_account_name(self):
         (result, out, err) = self.runsubcmd("rodc", "preload", "sambatool1", "sambatool2",
                                             "--server", os.environ["DC_SERVER"])
-        self.assertCmdSuccess(result, "ensuring rodc prefetch ran successfully")
+        self.assertCmdSuccess(result, out, err, "ensuring rodc prefetch ran successfully")
         self.assertEqual(out, "Replicating DN CN=sambatool1,CN=Users,%s\nReplicating DN CN=sambatool2,CN=Users,%s\n" % (self.base_dn, self.base_dn))
 
     def test_multi_by_dn(self):
         (result, out, err) = self.runsubcmd("rodc", "preload", "cn=sambatool3,cn=users,%s" % self.base_dn, "cn=sambatool4,cn=users,%s" % self.base_dn,
                                             "--server", os.environ["DC_SERVER"])
-        self.assertCmdSuccess(result, "ensuring rodc prefetch ran successfully")
+        self.assertCmdSuccess(result, out, err, "ensuring rodc prefetch ran successfully")
         self.assertEqual(out, "Replicating DN CN=sambatool3,CN=Users,%s\nReplicating DN CN=sambatool4,CN=Users,%s\n" % (self.base_dn, self.base_dn))
 
     def test_multi_in_file(self):
@@ -93,7 +93,7 @@ class RodcCmdTestCase(SambaToolCmdTest):
         open(tempf, 'w').write("sambatool1\nsambatool2")
         (result, out, err) = self.runsubcmd("rodc", "preload", "--file", tempf,
                                             "--server", os.environ["DC_SERVER"])
-        self.assertCmdSuccess(result, "ensuring rodc prefetch ran successfully")
+        self.assertCmdSuccess(result, out, err, "ensuring rodc prefetch ran successfully")
         self.assertEqual(out, "Replicating DN CN=sambatool1,CN=Users,%s\nReplicating DN CN=sambatool2,CN=Users,%s\n" % (self.base_dn, self.base_dn))
         os.unlink(tempf)
 
@@ -103,7 +103,7 @@ class RodcCmdTestCase(SambaToolCmdTest):
                                             "nonexistentuser2",
                                             "--server", os.environ["DC_SERVER"],
                                             "--ignore-errors")
-        self.assertCmdSuccess(result, "ensuring rodc prefetch ran successfully")
+        self.assertCmdSuccess(result, out, err, "ensuring rodc prefetch ran successfully")
         self.assertEqual(out, "Replicating DN CN=sambatool5,CN=Users,%s\n" % self.base_dn)
 
     def test_multi_with_missing_name_failure(self):
@@ -118,7 +118,7 @@ class RodcCmdTestCase(SambaToolCmdTest):
                                             "sambatool6", "sambatool5",
                                             "--server", os.environ["DC_SERVER"],
                                             "--ignore-errors")
-        self.assertCmdSuccess(result, "ensuring rodc prefetch ran successfully")
+        self.assertCmdSuccess(result, out, err, "ensuring rodc prefetch ran successfully")
         self.assertEqual(out, "Replicating DN CN=sambatool6,CN=Users,%s\nReplicating DN CN=sambatool5,CN=Users,%s\n" % (self.base_dn, self.base_dn))
 
     def test_multi_without_group_failure(self):
diff --git a/python/samba/tests/samba_tool/sites.py b/python/samba/tests/samba_tool/sites.py
index ee28109..8f96bba 100644
--- a/python/samba/tests/samba_tool/sites.py
+++ b/python/samba/tests/samba_tool/sites.py
@@ -44,7 +44,7 @@ class SitesCmdTestCase(BaseSitesCmdTestCase):
 
         result, out, err = self.runsubcmd("sites", "create", sitename,
                                           "-H", self.dburl, self.creds_string)
-        self.assertCmdSuccess(result)
+        self.assertCmdSuccess(result, out, err)
 
         dnsites = ldb.Dn(self.samdb, "CN=Sites,%s" % self.config_dn)
         dnsite = ldb.Dn(self.samdb, "CN=%s,%s" % (sitename, dnsites))
@@ -89,7 +89,7 @@ class SitesSubnetCmdTestCase(BaseSitesCmdTestCase):
                                               cidr, sitename,
                                               "-H", self.dburl,
                                               self.creds_string)
-            self.assertCmdSuccess(result)
+            self.assertCmdSuccess(result, out, err)
 
             ret = self.samdb.search(base=self.config_dn,
                                     scope=ldb.SCOPE_SUBTREE,
diff --git a/python/samba/tests/samba_tool/timecmd.py b/python/samba/tests/samba_tool/timecmd.py
index 000f0f2..310f861 100644
--- a/python/samba/tests/samba_tool/timecmd.py
+++ b/python/samba/tests/samba_tool/timecmd.py
@@ -25,7 +25,7 @@ class TimeCmdTestCase(SambaToolCmdTest):
     def test_timeget(self):
         """Run time against the server and make sure it looks accurate"""
         (result, out, err) = self.runcmd("time", os.environ["SERVER"])
-        self.assertCmdSuccess(result, "Ensuring time ran successfully")
+        self.assertCmdSuccess(result, out, err, "Ensuring time ran successfully")
 
         timefmt = strptime(out, "%a %b %d %H:%M:%S %Y %Z\n")
         servertime = int(mktime(timefmt))
diff --git a/python/samba/tests/samba_tool/user.py b/python/samba/tests/samba_tool/user.py
index 91e45c3..d2eb0e6 100644
--- a/python/samba/tests/samba_tool/user.py
+++ b/python/samba/tests/samba_tool/user.py
@@ -51,7 +51,7 @@ class UserCmdTestCase(SambaToolCmdTest):
         for user in self.users:
             (result, out, err) = user["createUserFn"](user)
 
-            self.assertCmdSuccess(result)
+            self.assertCmdSuccess(result, out, err)
             self.assertEquals(err,"","Shouldn't be any error messages")
             self.assertIn("User '%s' created successfully" % user["name"], out)
 
@@ -76,7 +76,7 @@ class UserCmdTestCase(SambaToolCmdTest):
         # try to delete all the 4 users we just added
         for user in self.users:
             (result, out, err) = self.runsubcmd("user", "delete", user["name"])
-            self.assertCmdSuccess(result, "Can we delete users")
+            self.assertCmdSuccess(result, out, err, "Can we delete users")
             found = self._find_user(user["name"])
             self.assertIsNone(found)
 
@@ -93,7 +93,7 @@ class UserCmdTestCase(SambaToolCmdTest):
                                                  "-H", "ldap://%s" % os.environ["DC_SERVER"],
                                                  "-U%s%%%s" % (os.environ["DC_USERNAME"], os.environ["DC_PASSWORD"]))
 
-            self.assertCmdSuccess(result)
+            self.assertCmdSuccess(result, out, err)
             self.assertEquals(err,"","Shouldn't be any error messages")
             self.assertIn("User '%s' created successfully" % user["name"], out)
 
@@ -193,7 +193,7 @@ class UserCmdTestCase(SambaToolCmdTest):
                                                 "--newpassword=%s" % newpasswd,
                                                 "-H", "ldap://%s" % os.environ["DC_SERVER"],
                                                 "-U%s%%%s" % (os.environ["DC_USERNAME"], os.environ["DC_PASSWORD"]))
-            # self.assertCmdSuccess(result, "Ensure setpassword runs")
+            self.assertCmdSuccess(result, out, err, "Ensure setpassword runs")
             self.assertEquals(err,"","setpassword with url")
             self.assertMatch(out, "Changed password OK", "setpassword with url")
 
@@ -202,7 +202,7 @@ class UserCmdTestCase(SambaToolCmdTest):
                                             "--cache-ldb-initialize",
                                             "--attributes=%s" % attributes,
                                             "--decrypt-samba-gpg")
-        self.assertCmdSuccess(result, "Ensure syncpasswords --cache-ldb-initialize runs")
+        self.assertCmdSuccess(result, out, err, "Ensure syncpasswords --cache-ldb-initialize runs")
         self.assertEqual(err,"","getpassword without url")
         cache_attrs = {
             "objectClass": { "value": "userSyncPasswords" },
@@ -220,7 +220,7 @@ class UserCmdTestCase(SambaToolCmdTest):
                 "syncpasswords --cache-ldb-initialize: %s: %s out[%s]" % (a, v, out))
 
         (result, out, err) = self.runsubcmd("user", "syncpasswords", "--no-wait")
-        self.assertCmdSuccess(result, "Ensure syncpasswords --no-wait runs")
+        self.assertCmdSuccess(result, out, err, "Ensure syncpasswords --no-wait runs")
         self.assertEqual(err,"","syncpasswords --no-wait")
         self.assertMatch(out, "dirsync_loop(): results 0",
             "syncpasswords --no-wait: 'dirsync_loop(): results 0': out[%s]" % (out))
@@ -241,12 +241,12 @@ class UserCmdTestCase(SambaToolCmdTest):
             (result, out, err) = self.runsubcmd("user", "setpassword",
                                                 user["name"],
                                                 "--newpassword=%s" % newpasswd)
-            self.assertCmdSuccess(result, "Ensure setpassword runs")
+            self.assertCmdSuccess(result, out, err, "Ensure setpassword runs")
             self.assertEquals(err,"","setpassword without url")
             self.assertMatch(out, "Changed password OK", "setpassword without url")
 
             (result, out, err) = self.runsubcmd("user", "syncpasswords", "--no-wait")
-            self.assertCmdSuccess(result, "Ensure syncpasswords --no-wait runs")
+            self.assertCmdSuccess(result, out, err, "Ensure syncpasswords --no-wait runs")
             self.assertEqual(err,"","syncpasswords --no-wait")
             self.assertMatch(out, "dirsync_loop(): results 0",
                 "syncpasswords --no-wait: 'dirsync_loop(): results 0': out[%s]" % (out))
@@ -272,7 +272,7 @@ class UserCmdTestCase(SambaToolCmdTest):
                                                 user["name"],
                                                 "--attributes=%s" % attributes,
                                                 "--decrypt-samba-gpg")
-            self.assertCmdSuccess(result, "Ensure getpassword runs")
+            self.assertCmdSuccess(result, out, err, "Ensure getpassword runs")
             self.assertEqual(err,"","getpassword without url")
             self.assertMatch(out, "Got password OK", "getpassword without url")
             self.assertMatch(out, "sAMAccountName: %s" % (user["name"]),
@@ -298,7 +298,7 @@ class UserCmdTestCase(SambaToolCmdTest):
                                                 "--must-change-at-next-login",
                                                 "-H", "ldap://%s" % os.environ["DC_SERVER"],
                                                 "-U%s%%%s" % (os.environ["DC_USERNAME"], os.environ["DC_PASSWORD"]))
-            # self.assertCmdSuccess(result, "Ensure setpassword runs")
+            self.assertCmdSuccess(result, out, err, "Ensure setpassword runs")
             self.assertEquals(err,"","setpassword with forced change")
             self.assertMatch(out, "Changed password OK", "setpassword with forced change")
 
@@ -313,7 +313,7 @@ class UserCmdTestCase(SambaToolCmdTest):
                                                 "--days=2",
                                                 "-H", "ldap://%s" % os.environ["DC_SERVER"],
                                                 "-U%s%%%s" % (os.environ["DC_USERNAME"], os.environ["DC_PASSWORD"]))
-            self.assertCmdSuccess(result, "Can we run setexpiry with names")
+            self.assertCmdSuccess(result, out, err, "Can we run setexpiry with names")
             self.assertIn("Expiry for user '%s' set to 2 days." % user["name"], out)
 
         for user in self.users:
@@ -333,7 +333,7 @@ class UserCmdTestCase(SambaToolCmdTest):
                                                 "--days=4",
                                                 "-H", "ldap://%s" % os.environ["DC_SERVER"],
                                                 "-U%s%%%s" % (os.environ["DC_USERNAME"], os.environ["DC_PASSWORD"]))
-        self.assertCmdSuccess(result, "Can we run setexpiry with a filter")
+        self.assertCmdSuccess(result, out, err, "Can we run setexpiry with a filter")
 
         for user in self.users:
             found = self._find_user(user["name"])
@@ -350,7 +350,7 @@ class UserCmdTestCase(SambaToolCmdTest):
                                             "-H", "ldap://%s" % os.environ["DC_SERVER"],
                                             "-U%s%%%s" % (os.environ["DC_USERNAME"],
                                                           os.environ["DC_PASSWORD"]))
-        self.assertCmdSuccess(result, "Error running list")
+        self.assertCmdSuccess(result, out, err, "Error running list")
 
         search_filter = ("(&(objectClass=user)(userAccountControl:%s:=%u))" %
                          (ldb.OID_COMPARATOR_AND, dsdb.UF_NORMAL_ACCOUNT))
@@ -410,8 +410,7 @@ class UserCmdTestCase(SambaToolCmdTest):
                                                 "-H", "ldap://%s" % os.environ["DC_SERVER"],
                                                 "-U%s%%%s" % (os.environ["DC_USERNAME"], os.environ["DC_PASSWORD"]))
 
-        msg = "command should return %s" % err
-        self.assertCmdSuccess(result, msg)
+        self.assertCmdSuccess(result, out, err)
         self.assertEquals(err,"","Shouldn't be any error messages")
         self.assertIn("User '%s' created successfully" % user["name"], out)
 
@@ -440,8 +439,7 @@ class UserCmdTestCase(SambaToolCmdTest):
                                                 "-H", "ldap://%s" % os.environ["DC_SERVER"],
                                                 "-U%s%%%s" % (os.environ["DC_USERNAME"], os.environ["DC_PASSWORD"]))
 
-        msg = "command should return %s" % err
-        self.assertCmdSuccess(result, msg)
+        self.assertCmdSuccess(result, out, err)
         self.assertEquals(err,"","Shouldn't be any error messages")
         self.assertIn("User '%s' created successfully" % user["name"], out)
 
diff --git a/python/samba/tests/samba_tool/user_check_password_script.py b/python/samba/tests/samba_tool/user_check_password_script.py
index 5d4fbae..f1deb88 100644
--- a/python/samba/tests/samba_tool/user_check_password_script.py
+++ b/python/samba/tests/samba_tool/user_check_password_script.py
@@ -54,12 +54,12 @@ class UserCheckPwdTestCase(SambaToolCmdTest):
         (result, out, err) = self.runsubcmd("user", "delete", user["name"],
                                             "-H", "ldap://%s" % os.environ["DC_SERVER"],
                                             "-U%s%%%s" % (os.environ["DC_USERNAME"], os.environ["DC_PASSWORD"]))
-        self.assertCmdSuccess(result, "Should delete user with bad password.")
+        self.assertCmdSuccess(result, out, err, "Should delete user with bad password.")
 
         (result, out, err) = self.runsubcmd("user", "add", user["name"], good_password,
                                             "-H", "ldap://%s" % os.environ["DC_SERVER"],
                                             "-U%s%%%s" % (os.environ["DC_USERNAME"], os.environ["DC_PASSWORD"]))
-        self.assertCmdSuccess(result, "Should succeed adding a user with good password.")
+        self.assertCmdSuccess(result, out, err, "Should succeed adding a user with good password.")
 
         # Set password
         (result, out, err) = self.runsubcmd("user", "setpassword", user["name"],
@@ -72,7 +72,7 @@ class UserCheckPwdTestCase(SambaToolCmdTest):
                                             "--newpassword=%s" % good_password,
                                             "-H", "ldap://%s" % os.environ["DC_SERVER"],
                                             "-U%s%%%s" % (os.environ["DC_USERNAME"], os.environ["DC_PASSWORD"]))
-        self.assertCmdSuccess(result, "Should succeed setting a user's password to a good one.")
+        self.assertCmdSuccess(result, out, err, "Should succeed setting a user's password to a good one.")
 
         # Password=
 
@@ -86,7 +86,7 @@ class UserCheckPwdTestCase(SambaToolCmdTest):
                                             "--newpassword=%s" % good_password + 'XYZ',
                                             "--ipaddress", os.environ["DC_SERVER_IP"],
                                             "-U%s%%%s" % (user["name"], good_password))
-        self.assertCmdSuccess(result, "A user setting their own password to a good one should succeed.")
+        self.assertCmdSuccess(result, out, err, "A user setting their own password to a good one should succeed.")
 
     def _randomUser(self, base={}):
         """create a user with random attribute values, you can specify base attributes"""
diff --git a/source4/torture/drs/python/fsmo.py b/source4/torture/drs/python/fsmo.py
index df5a274..4dd652d 100644
--- a/source4/torture/drs/python/fsmo.py
+++ b/source4/torture/drs/python/fsmo.py
@@ -66,7 +66,7 @@ class DrsFsmoTestCase(drs_base.DrsBaseTestCase):
                                             "-H", "ldap://%s:389" % DC,
                                             cmd_line_auth)
 
-        self.assertCmdSuccess(result)
+        self.assertCmdSuccess(result, out, err)
         self.assertEquals(err,"","Shouldn't be any error messages")
         if noop == False:
             self.assertTrue("FSMO transfer of '%s' role successful" % role in out)
-- 
2.8.0.rc3.226.g39d4020


From 97a5d05e92c10a8251563a766244ddfc32186610 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 2 Aug 2016 12:35:38 +0200
Subject: [PATCH 08/13] tests:samba_tool: make use of assertCmdFail() in gpo.py

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
 python/samba/tests/samba_tool/gpo.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python/samba/tests/samba_tool/gpo.py b/python/samba/tests/samba_tool/gpo.py
index cd22706..6297e1a 100644
--- a/python/samba/tests/samba_tool/gpo.py
+++ b/python/samba/tests/samba_tool/gpo.py
@@ -35,7 +35,7 @@ class GpoCmdTestCase(SambaToolCmdTest):
     def test_fetchfail(self):
         """Run against a non-existent GPO, and make sure it fails (this hard-coded UUID is very unlikely to exist"""
         (result, out, err) = self.runsubcmd("gpo", "fetch", "c25cac17-a02a-4151-835d-fae17446ee43", "-H", "ldap://%s" % os.environ["SERVER"])
-        self.assertEquals(result, -1, "check for result code")
+        self.assertCmdFail(result, "check for result code")
 
     def test_fetch(self):
         """Run against a real GPO, and make sure it passes"""
-- 
2.8.0.rc3.226.g39d4020


From fe4f8b64c5b66d9bcd9e16dae3904cd092911d92 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 1 Aug 2016 16:48:53 +0200
Subject: [PATCH 09/13] script/autobuild.py: check for AUTOBUILD_SKIP_SAMBA_O3
 environment variable

We need to skip the samba-o3 target on older systems like sn-devel-104.

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
 script/autobuild.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/script/autobuild.py b/script/autobuild.py
index 0af8166..811f906 100755
--- a/script/autobuild.py
+++ b/script/autobuild.py
@@ -41,6 +41,9 @@ builddirs = {
 
 defaulttasks = [ "ctdb", "samba", "samba-xc", "samba-o3", "samba-ctdb", "samba-libs", "samba-static", "ldb", "tdb", "talloc", "replace", "tevent", "pidl" ]
 
+if os.environ.get("AUTOBUILD_SKIP_SAMBA_O3", "0") == "1":
+    defaulttasks.remove("samba-o3")
+
 samba_configure_params = " --picky-developer ${PREFIX} ${EXTRA_PYTHON} --with-profiling-data"
 
 samba_libs_envvars =  "PYTHONPATH=${PYTHON_PREFIX}/site-packages:$PYTHONPATH"
-- 
2.8.0.rc3.226.g39d4020


From 45267ab98e54d5932680de18173ba9e2f899f406 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Sun, 8 May 2016 15:45:44 +0300
Subject: [PATCH 10/13] s3-param: add kerberos encryption types parameter

Signed-off-by: Uri Simchoni <uri at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
 .../security/kerberosencryptiontypes.xml           | 53 ++++++++++++++++++++++
 lib/param/loadparm.c                               |  2 +
 lib/param/loadparm.h                               |  4 ++
 lib/param/param_table.c                            |  9 ++++
 4 files changed, 68 insertions(+)
 create mode 100644 docs-xml/smbdotconf/security/kerberosencryptiontypes.xml

diff --git a/docs-xml/smbdotconf/security/kerberosencryptiontypes.xml b/docs-xml/smbdotconf/security/kerberosencryptiontypes.xml
new file mode 100644
index 0000000..b19a8a8
--- /dev/null
+++ b/docs-xml/smbdotconf/security/kerberosencryptiontypes.xml
@@ -0,0 +1,53 @@
+<samba:parameter name="kerberos encryption types"
+                 context="G"
+                 type="enum"
+		 enumlist="enum_kerberos_encryption_types_vals"
+                 xmlns:samba="http://www.samba.org/samba/DTD/samba-doc">
+<description>
+    <para>This parameter determines the encryption types to use when operating
+    as a Kerberos client. Possible values are <emphasis>all</emphasis>,
+    <emphasis>strong</emphasis>, and <emphasis>legacy</emphasis>.
+    </para>
+
+    <para>Samba uses a Kerberos library (MIT or Heimdal) to obtain Kerberos
+    tickets. This library is normally configured outside of Samba, using
+    the krb5.conf file. This file may also include directives to configure
+    the encryption types to be used. However, Samba implements Active Directory
+    protocols and algorithms to locate a domain controller. In order to
+    force the Kerberos library into using the correct domain controller,
+    some Samba processes, such as
+    <citerefentry><refentrytitle>winbindd</refentrytitle>
+    <manvolnum>8</manvolnum></citerefentry> and
+    <citerefentry><refentrytitle>net</refentrytitle>
+    <manvolnum>8</manvolnum></citerefentry>, build a private krb5.conf
+    file for use by the Kerberos library while being invoked from Samba.
+    This private file controls all aspects of the Kerberos library operation,
+    and this parameter controls how the encryption types are configured
+    within this generated file, and therefore also controls the encryption
+    types negotiable by Samba.
+    </para>
+
+    <para>When set to <constant>all</constant>, all active directory
+    encryption types are allowed.
+    </para>
+
+    <para>When set to <constant>strong</constant>, only AES-based encyption
+    types are offered. This can be used in hardened environments to prevent
+    downgrade attacks.
+    </para>
+
+    <para>When set to <constant>legacy</constant>, only RC4-HMAC-MD5
+    is allowed. Avoiding AES this way has one a very specific use.
+    Normally, the encryption type is negotiated between the peers.
+    However, there is one scenario in which a Windows read-only domain
+    controller (RODC) advertises AES encryption, but then proxies the
+    request to a writeable DC which may not support AES encryption,
+    leading to failure of the handshake. Setting this parameter to
+    <constant>legacy</constant> would cause samba not to negotiate AES
+    encryption. It is assumed of course that the weaker legacy
+    encryption types are acceptable for the setup.
+    </para>
+</description>
+
+<value type="default">all</value>
+</samba:parameter>
diff --git a/lib/param/loadparm.c b/lib/param/loadparm.c
index 5f4610e..6aa757f 100644
--- a/lib/param/loadparm.c
+++ b/lib/param/loadparm.c
@@ -2900,6 +2900,8 @@ struct loadparm_context *loadparm_init(TALLOC_CTX *mem_ctx)
 
 	lpcfg_do_global_parameter(lp_ctx, "smb2 leases", "yes");
 
+	lpcfg_do_global_parameter(lp_ctx, "kerberos encryption types", "all");
+
 	/* Allow modules to adjust defaults */
 	for (defaults_hook = defaults_hooks; defaults_hook;
 		 defaults_hook = defaults_hook->next) {
diff --git a/lib/param/loadparm.h b/lib/param/loadparm.h
index aa256c1..d8f6975 100644
--- a/lib/param/loadparm.h
+++ b/lib/param/loadparm.h
@@ -178,6 +178,10 @@ struct file_lists {
 #define KERBEROS_VERIFY_DEDICATED_KEYTAB 2
 #define KERBEROS_VERIFY_SECRETS_AND_KEYTAB 3
 
+#define KERBEROS_ETYPES_ALL 0
+#define KERBEROS_ETYPES_STRONG 1
+#define KERBEROS_ETYPES_LEGACY 2
+
 /* ACL compatibility */
 enum acl_compatibility {ACL_COMPAT_AUTO, ACL_COMPAT_WINNT, ACL_COMPAT_WIN2K};
 
diff --git a/lib/param/param_table.c b/lib/param/param_table.c
index d8d9144..c8520d2 100644
--- a/lib/param/param_table.c
+++ b/lib/param/param_table.c
@@ -208,6 +208,15 @@ static const struct enum_list enum_kerberos_method[] = {
 	{-1, NULL}
 };
 
+/* Kerberos encryption types selection options */
+
+static const struct enum_list enum_kerberos_encryption_types_vals[] = {
+	{KERBEROS_ETYPES_ALL, "all"},
+	{KERBEROS_ETYPES_STRONG, "strong"},
+	{KERBEROS_ETYPES_LEGACY, "legacy"},
+	{-1, NULL}
+};
+
 static const struct enum_list enum_printing[] = {
 	{PRINT_SYSV, "sysv"},
 	{PRINT_AIX, "aix"},
-- 
2.8.0.rc3.226.g39d4020


From 3bd99ee00534e5988cf902483b4741a7557d4738 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Mon, 30 May 2016 21:21:41 +0300
Subject: [PATCH 11/13] libads: use "kerberos encryption types" parameter

When creating the custom krb.conf file, list etypes
according to kerberos encryption types

Also use proper directives for heimdal (heimdal recognizes
the MIT etype directives, but does not act upon them)

Signed-off-by: Uri Simchoni <uri at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
 source3/libads/kerberos.c | 106 ++++++++++++++++++++++++++++++++++------------
 1 file changed, 80 insertions(+), 26 deletions(-)

diff --git a/source3/libads/kerberos.c b/source3/libads/kerberos.c
index 53407fa..a47ab6c 100644
--- a/source3/libads/kerberos.c
+++ b/source3/libads/kerberos.c
@@ -813,6 +813,76 @@ out:
  run as root or will fail (which is a good thing :-).
 ************************************************************************/
 
+#if !defined(SAMBA4_USES_HEIMDAL) /* MIT version */
+static char *get_enctypes(TALLOC_CTX *mem_ctx)
+{
+	char *aes_enctypes = NULL;
+	const char *legacy_enctypes = "";
+	char *enctypes = NULL;
+
+	aes_enctypes = talloc_strdup(mem_ctx, "");
+	if (aes_enctypes == NULL) {
+		goto done;
+	}
+
+	if (lp_kerberos_encryption_types() == KERBEROS_ETYPES_ALL ||
+	    lp_kerberos_encryption_types() == KERBEROS_ETYPES_STRONG) {
+#ifdef HAVE_ENCTYPE_AES256_CTS_HMAC_SHA1_96
+		aes_enctypes = talloc_asprintf_append(
+		    aes_enctypes, "%s", "aes256-cts-hmac-sha1-96 ");
+		if (aes_enctypes == NULL) {
+			goto done;
+		}
+#endif
+#ifdef HAVE_ENCTYPE_AES128_CTS_HMAC_SHA1_96
+		aes_enctypes = talloc_asprintf_append(
+		    aes_enctypes, "%s", "aes128-cts-hmac-sha1-96");
+		if (aes_enctypes == NULL) {
+			goto done;
+		}
+#endif
+	}
+
+	if (lp_kerberos_encryption_types() == KERBEROS_ETYPES_ALL ||
+	    lp_kerberos_encryption_types() == KERBEROS_ETYPES_LEGACY) {
+		legacy_enctypes = "RC4-HMAC DES-CBC-CRC DES-CBC-MD5";
+	}
+
+	enctypes =
+	    talloc_asprintf(mem_ctx, "\tdefault_tgs_enctypes = %s %s\n"
+				     "\tdefault_tkt_enctypes = %s %s\n"
+				     "\tpreferred_enctypes = %s %s\n",
+			    aes_enctypes, legacy_enctypes, aes_enctypes,
+			    legacy_enctypes, aes_enctypes, legacy_enctypes);
+done:
+	TALLOC_FREE(aes_enctypes);
+	return enctypes;
+}
+#else /* Heimdal version */
+static char *get_enctypes(TALLOC_CTX *mem_ctx)
+{
+	const char *aes_enctypes = "";
+	const char *legacy_enctypes = "";
+	char *enctypes = NULL;
+
+	if (lp_kerberos_encryption_types() == KERBEROS_ETYPES_ALL ||
+	    lp_kerberos_encryption_types() == KERBEROS_ETYPES_STRONG) {
+		aes_enctypes =
+		    "aes256-cts-hmac-sha1-96 aes128-cts-hmac-sha1-96";
+	}
+
+	if (lp_kerberos_encryption_types() == KERBEROS_ETYPES_ALL ||
+	    lp_kerberos_encryption_types() == KERBEROS_ETYPES_LEGACY) {
+		legacy_enctypes = "arcfour-hmac-md5 des-cbc-crc des-cbc-md5";
+	}
+
+	enctypes = talloc_asprintf(mem_ctx, "\tdefault_etypes = %s %s\n",
+				   aes_enctypes, legacy_enctypes);
+
+	return enctypes;
+}
+#endif
+
 bool create_local_private_krb5_conf_for_domain(const char *realm,
 						const char *domain,
 						const char *sitename,
@@ -828,7 +898,7 @@ bool create_local_private_krb5_conf_for_domain(const char *realm,
 	int fd;
 	char *realm_upper = NULL;
 	bool result = false;
-	char *aes_enctypes = NULL;
+	char *enctypes = NULL;
 	mode_t mask;
 
 	if (!lp_create_krb5_conf()) {
@@ -879,34 +949,18 @@ bool create_local_private_krb5_conf_for_domain(const char *realm,
 		goto done;
 	}
 
-	aes_enctypes = talloc_strdup(fname, "");
-	if (aes_enctypes == NULL) {
+	enctypes = get_enctypes(fname);
+	if (enctypes == NULL) {
 		goto done;
 	}
 
-#ifdef HAVE_ENCTYPE_AES256_CTS_HMAC_SHA1_96
-	aes_enctypes = talloc_asprintf_append(aes_enctypes, "%s", "aes256-cts-hmac-sha1-96 ");
-	if (aes_enctypes == NULL) {
-		goto done;
-	}
-#endif
-#ifdef HAVE_ENCTYPE_AES128_CTS_HMAC_SHA1_96
-	aes_enctypes = talloc_asprintf_append(aes_enctypes, "%s", "aes128-cts-hmac-sha1-96");
-	if (aes_enctypes == NULL) {
-		goto done;
-	}
-#endif
-
-	file_contents = talloc_asprintf(fname,
-					"[libdefaults]\n\tdefault_realm = %s\n"
-					"\tdefault_tgs_enctypes = %s RC4-HMAC DES-CBC-CRC DES-CBC-MD5\n"
-					"\tdefault_tkt_enctypes = %s RC4-HMAC DES-CBC-CRC DES-CBC-MD5\n"
-					"\tpreferred_enctypes = %s RC4-HMAC DES-CBC-CRC DES-CBC-MD5\n"
-					"\tdns_lookup_realm = false\n\n"
-					"[realms]\n\t%s = {\n"
-					"%s\t}\n",
-					realm_upper, aes_enctypes, aes_enctypes, aes_enctypes,
-					realm_upper, kdc_ip_string);
+	file_contents =
+	    talloc_asprintf(fname, "[libdefaults]\n\tdefault_realm = %s\n"
+				   "%s"
+				   "\tdns_lookup_realm = false\n\n"
+				   "[realms]\n\t%s = {\n"
+				   "%s\t}\n",
+			    realm_upper, enctypes, realm_upper, kdc_ip_string);
 
 	if (!file_contents) {
 		goto done;
-- 
2.8.0.rc3.226.g39d4020


From fbef7813226b5b27e783ab41d2de9381d2787057 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Mon, 4 Jul 2016 09:50:33 +0300
Subject: [PATCH 12/13] heimdal: honor conf enctypes when obtaining a service
 ticket

This patch removes part of what's categorized in the code as
"hideous glue", which causes Heimdal to ignore krb5.conf
encryption types, and instead use either the application-
supplied values or the default compile-time values.

Signed-off-by: Uri Simchoni <uri at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
 source4/heimdal/lib/gssapi/krb5/init_sec_context.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/source4/heimdal/lib/gssapi/krb5/init_sec_context.c b/source4/heimdal/lib/gssapi/krb5/init_sec_context.c
index 0a89ae1..efc4215 100644
--- a/source4/heimdal/lib/gssapi/krb5/init_sec_context.c
+++ b/source4/heimdal/lib/gssapi/krb5/init_sec_context.c
@@ -427,15 +427,12 @@ init_auth
     /*
      * This is hideous glue for (NFS) clients that wants to limit the
      * available enctypes to what it can support (encryption in
-     * kernel). If there is no enctypes selected for this credential,
-     * reset it to the default set of enctypes.
+     * kernel).
      */
     {
-	krb5_enctype *enctypes = NULL;
-
-	if (cred && cred->enctypes)
-	    enctypes = cred->enctypes;
-	krb5_set_default_in_tkt_etypes(context, enctypes);
+	if (cred && cred->enctypes) {
+	    krb5_set_default_in_tkt_etypes(context, cred->enctypes);
+	}
     }
 
     /* canon name if needed for client + target realm */
-- 
2.8.0.rc3.226.g39d4020


From 97c32185172b7787a404d539cc7b350326a7f314 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Tue, 5 Jul 2016 06:07:04 +0300
Subject: [PATCH 13/13] selftest: tests for kerberos encryption types

This test uses tshark and cwrap's packet capturing capability
to observe the Kerberos handshakes and ensure the correct
encryption types are being used.

Signed-off-by: Uri Simchoni <uri at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
 source4/selftest/tests.py                |  3 ++
 testprogs/blackbox/test_client_etypes.sh | 54 ++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+)
 create mode 100755 testprogs/blackbox/test_client_etypes.sh

diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index e4af5f8..2619ecc 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -407,6 +407,9 @@ plantestsuite("samba4.blackbox.rfc2307_mapping(ad_dc_ntvfs:local)", "ad_dc_ntvfs
 plantestsuite("samba4.blackbox.chgdcpass", "chgdcpass", [os.path.join(bbdir, "test_chgdcpass.sh"), '$SERVER', "CHGDCPASS\$", '$REALM', '$DOMAIN', '$PREFIX', "aes256-cts-hmac-sha1-96", '$SELFTEST_PREFIX/chgdcpass', smbclient4])
 plantestsuite("samba4.blackbox.samba_upgradedns(chgdcpass:local)", "chgdcpass:local", [os.path.join(bbdir, "test_samba_upgradedns.sh"), '$SERVER', '$REALM', '$PREFIX', '$SELFTEST_PREFIX/chgdcpass'])
 plantestsuite("samba4.blackbox.net_ads(ad_dc:client)", "ad_dc:client", [os.path.join(bbdir, "test_net_ads.sh"), '$DC_SERVER', '$DC_USERNAME', '$DC_PASSWORD', '$PREFIX_ABS'])
+plantestsuite("samba4.blackbox.client_etypes_all(ad_dc:client)", "ad_dc:client", [os.path.join(bbdir, "test_client_etypes.sh"), '$DC_SERVER', '$DC_USERNAME', '$DC_PASSWORD', '$PREFIX_ABS', 'all', '17_18_23'])
+plantestsuite("samba4.blackbox.client_etypes_legacy(ad_dc:client)", "ad_dc:client", [os.path.join(bbdir, "test_client_etypes.sh"), '$DC_SERVER', '$DC_USERNAME', '$DC_PASSWORD', '$PREFIX_ABS', 'legacy', '23'])
+plantestsuite("samba4.blackbox.client_etypes_strong(ad_dc:client)", "ad_dc:client", [os.path.join(bbdir, "test_client_etypes.sh"), '$DC_SERVER', '$DC_USERNAME', '$DC_PASSWORD', '$PREFIX_ABS', 'strong', '17_18'])
 plantestsuite("samba4.blackbox.net_ads_dns(ad_member:local)", "ad_member:local", [os.path.join(bbdir, "test_net_ads_dns.sh"), '$DC_SERVER', '$DC_USERNAME', '$DC_PASSWORD', '$REALM', '$USERNAME', '$PASSWORD'])
 plantestsuite_loadlist("samba4.rpc.echo against NetBIOS alias", "ad_dc_ntvfs", [valgrindify(smbtorture4), "$LISTOPT", "$LOADLIST", 'ncacn_np:$NETBIOSALIAS', '-U$DOMAIN/$USERNAME%$PASSWORD', 'rpc.echo'])
 
diff --git a/testprogs/blackbox/test_client_etypes.sh b/testprogs/blackbox/test_client_etypes.sh
new file mode 100755
index 0000000..a878c81
--- /dev/null
+++ b/testprogs/blackbox/test_client_etypes.sh
@@ -0,0 +1,54 @@
+if [ $# -lt 6 ]; then
+cat <<EOF
+Usage: test_client_etypes.sh DC_SERVER DC_USERNAME DC_PASSWORD PREFIX_ABS ETYPE_CONF EXPECTED
+EOF
+exit 1;
+fi
+
+#requires tshark
+which tshark > /dev/null 2>&1 || exit 0
+
+DC_SERVER=$1
+DC_USERNAME=$2
+DC_PASSWORD=$3
+BASEDIR=$4
+ETYPE_CONF=$5
+EXPECTED_ETYPES="$6"
+
+HOSTNAME=`dd if=/dev/urandom bs=1 count=32 2>/dev/null | sha1sum | cut -b 1-10`
+
+RUNDIR=`pwd`
+cd $BASEDIR
+WORKDIR=`mktemp -d -p .`
+WORKDIR=`basename $WORKDIR`
+cp -a client/* $WORKDIR/
+sed -ri "s@(dir|directory) = (.*)/client/@\1 = \2/$WORKDIR/@" $WORKDIR/client.conf
+sed -ri "s/netbios name = .*/netbios name = $HOSTNAME/" $WORKDIR/client.conf
+rm -f $WORKDIR/private/secrets.tdb
+cd $RUNDIR
+
+failed=0
+
+net_tool="$BINDIR/net -s $BASEDIR/$WORKDIR/client.conf --option=security=ads --option=kerberosencryptiontypes=$ETYPE_CONF"
+pcap_file=$BASEDIR/$WORKDIR/test.pcap
+
+# Load test functions
+. `dirname $0`/subunit.sh
+
+export SOCKET_WRAPPER_PCAP_FILE=$pcap_file
+testit "join" $VALGRIND $net_tool ads join -kU$DC_USERNAME%$DC_PASSWORD || failed=`expr $failed + 1`
+
+testit "testjoin" $VALGRIND $net_tool ads testjoin -kP || failed=`expr $failed + 1`
+
+#The leave command does not use the locally-generated
+#krb5.conf
+export SOCKET_WRAPPER_PCAP_FILE=
+testit "leave" $VALGRIND $net_tool ads leave -U$DC_USERNAME%$DC_PASSWORD || failed=`expr $failed + 1`
+
+actual_types="`tshark -r $pcap_file  -nVY "kerberos" | sed -rn 's/[[:space:]]*ENCTYPE:.*\(([^\)]*)\)$/\1/p' | sort -u | tr '\n' '_' | sed s/_$//`"
+
+testit "verify types" test "x$actual_types" = "x$EXPECTED_ETYPES" || failed=`expr $failed + 1`
+
+rm -rf $BASEDIR/$WORKDIR
+
+exit $failed
-- 
2.8.0.rc3.226.g39d4020



More information about the samba-technical mailing list