[SCM] Samba Shared Repository - branch v4-16-test updated

Jule Anger janger at samba.org
Mon Mar 7 11:50:01 UTC 2022


The branch, v4-16-test has been updated
       via  de8fc990b21 s3: smbd: Fix our leases code to return the correct error in the non-dynamic share case.
       via  7995e03b39e s4: torture: Add new SMB2 lease test test_lease_duplicate_open().
       via  423bbea002e s4: torture: Add new SMB2 lease test test_lease_duplicate_create().
       via  5caac70d8d4 s3:trusts_utils: use a password length of 120 for machine accounts
       via  a31721982fe upgradehelpers.py: add a comment to update_krbtgt_account_password()
       via  8c9bb2cafd6 provision: add a comment that the value of krbtgtpass is ignored in the backend
       via  66d8622b646 upgradehelpers.py: let update_machine_account_password() use 120 character passwords
       via  4872e1af2c1 provision: use 120 characters for the dns account password
       via  e13a72df5f2 samba-tool/join_member: let py_net_join_member() choose the password
       via  ac61afa5022 s3:py_net: allow machinepass=None to py_net_join_member()
      from  c240b977dbe s4/auth/simple_bind: correctly report TLS state

https://git.samba.org/?p=samba.git;a=shortlog;h=v4-16-test


- Log -----------------------------------------------------------------
commit de8fc990b21aeb6741e76bfd33772384b66682d9
Author: Jeremy Allison <jra at samba.org>
Date:   Thu Feb 17 11:12:39 2022 -0800

    s3: smbd: Fix our leases code to return the correct error in the non-dynamic share case.
    
    We now return INVALID_PARAMETER when trying to open a
    different file with a duplicate lease key on the same
    (non-dynamic) share. This will enable us to pass another
    Windows test suite leases test.
    
    We now behave the same as Windows10.
    
    Remove knownfail.d/smb2-lease-duplicateopen
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14737
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: David Mulder <dmulder at suse.com>
    
    Autobuild-User(master): Jeremy Allison <jra at samba.org>
    Autobuild-Date(master): Fri Feb 18 20:12:12 UTC 2022 on sn-devel-184
    
    (cherry picked from commit 408be54323861c24b6377b804be4428cf45b471e)
    
    Autobuild-User(v4-16-test): Jule Anger <janger at samba.org>
    Autobuild-Date(v4-16-test): Mon Mar  7 11:49:31 UTC 2022 on sn-devel-184

commit 7995e03b39e7cb972dc76be574d25eed7c8c2da7
Author: Jeremy Allison <jra at samba.org>
Date:   Thu Feb 17 10:58:32 2022 -0800

    s4: torture: Add new SMB2 lease test test_lease_duplicate_open().
    
    Checks we return INVALID_PARAMETER when trying to open a
    different file with a duplicate lease key on the same share.
    
    Checked against Windows10. Currently fails against smbd
    so add knownfail.d/smb2-lease-duplicateopen
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14737
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: David Mulder <dmulder at suse.com>
    (cherry picked from commit ca3896b6f8bbcad68f042720feceedfa29ddbd83)

commit 423bbea002e6877c7d7cffde69fe66d52dcf0d96
Author: Jeremy Allison <jra at samba.org>
Date:   Thu Feb 17 09:58:27 2022 -0800

    s4: torture: Add new SMB2 lease test test_lease_duplicate_create().
    
    Checks we return INVALID_PARAMETER when trying to create a
    new file with a duplicate lease key on the same share.
    
    Checked against Windows10. Samba already passes this
    but we didn't have a test before.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14737
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: David Mulder <dmulder at suse.com>
    (cherry picked from commit bf22548d11fe67ea3f4ec10dff81773d626e4703)

commit 5caac70d8d426e1f3afa40d05515d96669f24569
Author: Stefan Metzmacher <metze at samba.org>
Date:   Mon Feb 21 15:28:53 2022 +0100

    s3:trusts_utils: use a password length of 120 for machine accounts
    
    This is important when we change the machine password against
    an RODC that proxies the request to an RWDC.
    
    An RODC using NetrServerPasswordSet2() to proxy PasswordUpdateForward via
    NetrLogonSendToSam() ignores a return of NT_STATUS_INVALID_PARAMETER
    and reports NT_STATUS_OK as result of NetrServerPasswordSet2().
    This hopefully found the last hole in our very robust machine account
    password handling logic inside of trust_pw_change().
    
    The lesson is: try to be as identical to how windows works as possible,
    everything else may use is untested code paths on Windows.
    
    A similar problem was fixed by this commit:
    
        commit 609ca657652862fd9c81fd11f818efb74f72ff55
        Author: Joseph Sutton <josephsutton at catalyst.net.nz>
        Date:   Wed Feb 24 02:03:25 2021 +1300
    
            provision: Decrease the length of random machine passwords
    
            The current length of 128-255 UTF-16 characters currently causes
            generation of crypt() passwords to typically fail. This commit
            decreases the length to 120 UTF-16 characters, which is the same as
            that used by Windows.
    
            BUG: https://bugzilla.samba.org/show_bug.cgi?id=14621
    
            Signed-off-by: Joseph Sutton <josephsutton at catalyst.net.nz>
            Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
            Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14984
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Andreas Schneider <asn at samba.org>
    
    Autobuild-User(master): Stefan Metzmacher <metze at samba.org>
    Autobuild-Date(master): Wed Feb 23 08:49:54 UTC 2022 on sn-devel-184
    
    (cherry picked from commit 5e2386336c49fab46c1192db972af5da1e916b32)

commit a31721982fe63775ab3d0ad7e3dc00f647ffb5cc
Author: Stefan Metzmacher <metze at samba.org>
Date:   Mon Feb 21 15:23:54 2022 +0100

    upgradehelpers.py: add a comment to update_krbtgt_account_password()
    
    The backend generates its own random krbtgt password values.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14984
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Andreas Schneider <asn at samba.org>
    (cherry picked from commit ad0b5561b492dfa28acfc9604b2358bb8b490703)

commit 8c9bb2cafd62411cb904a8199e96e3948bbe9c20
Author: Stefan Metzmacher <metze at samba.org>
Date:   Mon Feb 21 15:22:50 2022 +0100

    provision: add a comment that the value of krbtgtpass is ignored in the backend
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14984
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Andreas Schneider <asn at samba.org>
    (cherry picked from commit 725c94d57d3d656bc94633dacbac683a4c11d3e6)

commit 66d8622b6467419e7953100e752f448355e3a3ae
Author: Stefan Metzmacher <metze at samba.org>
Date:   Mon Feb 21 15:22:06 2022 +0100

    upgradehelpers.py: let update_machine_account_password() use 120 character passwords
    
    We already changed provision to use 120 character passwords with commit
    609ca657652862fd9c81fd11f818efb74f72ff55.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14984
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Andreas Schneider <asn at samba.org>
    (cherry picked from commit 6bb7c0f24918329804b7f4fb71908e8fab99e266)

commit 4872e1af2c1f826631fe45424af16a24dd8809d6
Author: Stefan Metzmacher <metze at samba.org>
Date:   Mon Feb 21 15:08:34 2022 +0100

    provision: use 120 characters for the dns account password
    
    We should use the same as for the computer account.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14984
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Andreas Schneider <asn at samba.org>
    (cherry picked from commit 3b91be36581de1007427d539daffdaa62752412d)

commit e13a72df5f2f36f4dce5e1a51c0e0b5db2231db0
Author: Stefan Metzmacher <metze at samba.org>
Date:   Mon Feb 21 15:03:22 2022 +0100

    samba-tool/join_member: let py_net_join_member() choose the password
    
    It means we'll let trust_pw_new_value() generate the password.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14984
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Andreas Schneider <asn at samba.org>
    (cherry picked from commit 59ac782452c4993274fa837256a8b9c5675e707b)

commit ac61afa50224a2ee6d3b521222b3c5210ba95947
Author: Stefan Metzmacher <metze at samba.org>
Date:   Mon Feb 21 23:48:37 2022 +0100

    s3:py_net: allow machinepass=None to py_net_join_member()
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14984
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Andreas Schneider <asn at samba.org>
    (cherry picked from commit 576bdb08c51c47c390cc390fbefdcfee275b7f0f)

-----------------------------------------------------------------------

Summary of changes:
 python/samba/netcmd/domain.py      |   2 -
 python/samba/provision/__init__.py |   5 +-
 python/samba/upgradehelpers.py     |  11 ++--
 source3/libsmb/trusts_util.c       |  14 ++++-
 source3/smbd/open.c                |  38 +++++++++++-
 source3/utils/py_net.c             |   2 +-
 source4/torture/smb2/lease.c       | 124 +++++++++++++++++++++++++++++++++++++
 7 files changed, 183 insertions(+), 13 deletions(-)


Changeset truncated at 500 lines:

diff --git a/python/samba/netcmd/domain.py b/python/samba/netcmd/domain.py
index 1bdc0ee535a..e814a47233d 100644
--- a/python/samba/netcmd/domain.py
+++ b/python/samba/netcmd/domain.py
@@ -691,8 +691,6 @@ class cmd_domain_join(Command):
                     os.rename(f.name, smb_conf)
                 s3_lp = s3param.get_context()
                 s3_lp.load(smb_conf)
-                if machinepass is None:
-                    machinepass = samba.generate_random_machine_password(14, 40)
                 s3_net = s3_Net(creds, s3_lp, server=server)
                 (sid, domain_name) = s3_net.join_member(netbios_name,
                                                         machinepass=machinepass,
diff --git a/python/samba/provision/__init__.py b/python/samba/provision/__init__.py
index 1723d9935d4..ff9b8fac916 100644
--- a/python/samba/provision/__init__.py
+++ b/python/samba/provision/__init__.py
@@ -1924,11 +1924,14 @@ def provision_fill(samdb, secrets_ldb, logger, names, paths,
         invocationid = str(uuid.uuid4())
 
     if krbtgtpass is None:
+        # Note that the machinepass value is ignored
+        # as the backend (password_hash.c) will generate its
+        # own random values for the krbtgt keys
         krbtgtpass = samba.generate_random_machine_password(128, 255)
     if machinepass is None:
         machinepass = samba.generate_random_machine_password(120, 120)
     if dnspass is None:
-        dnspass = samba.generate_random_password(128, 255)
+        dnspass = samba.generate_random_password(120, 120)
 
     samdb.transaction_start()
     try:
diff --git a/python/samba/upgradehelpers.py b/python/samba/upgradehelpers.py
index 7f92b45f3fb..c853668058e 100644
--- a/python/samba/upgradehelpers.py
+++ b/python/samba/upgradehelpers.py
@@ -582,7 +582,7 @@ def update_machine_account_password(samdb, secrets_ldb, names):
         assert(len(res) == 1)
 
         msg = ldb.Message(res[0].dn)
-        machinepass = samba.generate_random_machine_password(128, 255)
+        machinepass = samba.generate_random_machine_password(120, 120)
         mputf16 = machinepass.encode('utf-16-le')
         msg["clearTextPassword"] = ldb.MessageElement(mputf16,
                                                       ldb.FLAG_MOD_REPLACE,
@@ -658,9 +658,12 @@ def update_krbtgt_account_password(samdb):
     assert(len(res) == 1)
 
     msg = ldb.Message(res[0].dn)
-    machinepass = samba.generate_random_machine_password(128, 255)
-    mputf16 = machinepass.encode('utf-16-le')
-    msg["clearTextPassword"] = ldb.MessageElement(mputf16,
+    # Note that the machinepass value is ignored
+    # as the backend (password_hash.c) will generate its
+    # own random values for the krbtgt keys
+    krbtgtpass = samba.generate_random_machine_password(128, 255)
+    kputf16 = krbtgtpass.encode('utf-16-le')
+    msg["clearTextPassword"] = ldb.MessageElement(kputf16,
                                                   ldb.FLAG_MOD_REPLACE,
                                                   "clearTextPassword")
 
diff --git a/source3/libsmb/trusts_util.c b/source3/libsmb/trusts_util.c
index 55e3c74494a..71e1a35eba7 100644
--- a/source3/libsmb/trusts_util.c
+++ b/source3/libsmb/trusts_util.c
@@ -55,10 +55,18 @@ char *trust_pw_new_value(TALLOC_CTX *mem_ctx,
 			 int security)
 {
 	/*
-	 * use secure defaults.
+	 * use secure defaults, which match
+	 * what windows uses for computer passwords.
+	 *
+	 * We used to have min=128 and max=255 here, but
+	 * it's a bad idea because of bugs in the Windows
+	 * RODC/RWDC PasswordUpdateForward handling via
+	 * NetrLogonSendToSam.
+	 *
+	 * See https://bugzilla.samba.org/show_bug.cgi?id=14984
 	 */
-	size_t min = 128;
-	size_t max = 255;
+	size_t min = 120;
+	size_t max = 120;
 
 	switch (sec_channel_type) {
 	case SEC_CHAN_WKSTA:
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index a5664b319ad..5a3ac2c064a 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -5302,8 +5302,42 @@ static void lease_match_parser(
 
 		/* Everything should be the same. */
 		if (!file_id_equal(&state->id, &f->id)) {
-			/* This should catch all dynamic share cases. */
-			state->match_status = NT_STATUS_OPLOCK_NOT_GRANTED;
+			/*
+			 * The client asked for a lease on a
+			 * file that doesn't match the file_id
+			 * in the database.
+			 *
+			 * Maybe this is a dynamic share, i.e.
+			 * a share where the servicepath is
+			 * different for different users (e.g.
+			 * the [HOMES] share.
+			 *
+			 * If the servicepath is different, but the requested
+			 * file name + stream name is the same then this is
+			 * a dynamic share, the client is using the same share
+			 * name and doesn't know that the underlying servicepath
+			 * is different. It was expecting a lease on the
+			 * same file. Return NT_STATUS_OPLOCK_NOT_GRANTED
+			 * to break leases
+			 *
+			 * Otherwise the client has messed up, or is
+			 * testing our error codes, so return
+			 * NT_STATUS_INVALID_PARAMETER.
+			 */
+			if (!strequal(f->servicepath, state->servicepath) &&
+			    strequal(f->base_name, state->fname->base_name) &&
+			    strequal(f->stream_name, state->fname->stream_name))
+			{
+				/*
+				 * Name is the same but servicepath is
+				 * different, dynamic share. Break leases.
+				 */
+				state->match_status =
+					NT_STATUS_OPLOCK_NOT_GRANTED;
+			} else {
+				state->match_status =
+					NT_STATUS_INVALID_PARAMETER;
+			}
 			break;
 		}
 		if (!strequal(f->servicepath, state->servicepath)) {
diff --git a/source3/utils/py_net.c b/source3/utils/py_net.c
index 3142f83bc7f..0d774bcb805 100644
--- a/source3/utils/py_net.c
+++ b/source3/utils/py_net.c
@@ -88,7 +88,7 @@ static PyObject *py_net_join_member(py_net_Object *self, PyObject *args, PyObjec
 		return NULL;
 	}
 
-	if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|ssssssspp:Join",
+	if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|sssssszpp:Join",
 					 discard_const_p(char *, kwnames),
 					 &r->in.dnshostname,
 					 &r->in.upn,
diff --git a/source4/torture/smb2/lease.c b/source4/torture/smb2/lease.c
index 2da320483fe..43b418c5acf 100644
--- a/source4/torture/smb2/lease.c
+++ b/source4/torture/smb2/lease.c
@@ -4436,6 +4436,126 @@ done:
 	return ret;
 }
 
+static bool test_lease_duplicate_create(struct torture_context *tctx,
+				   struct smb2_tree *tree)
+{
+	TALLOC_CTX *mem_ctx = talloc_new(tctx);
+	struct smb2_create io;
+	struct smb2_lease ls;
+	struct smb2_handle h1 = {{0}};
+	struct smb2_handle h2 = {{0}};
+	NTSTATUS status;
+	const char *fname1 = "duplicate_create1.dat";
+	const char *fname2 = "duplicate_create2.dat";
+	bool ret = true;
+	uint32_t caps;
+
+	caps = smb2cli_conn_server_capabilities(
+		tree->session->transport->conn);
+	if (!(caps & SMB2_CAP_LEASING)) {
+		torture_skip(tctx, "leases are not supported");
+	}
+
+	/* Ensure files don't exist. */
+	smb2_util_unlink(tree, fname1);
+	smb2_util_unlink(tree, fname2);
+
+	/* Create file1 - LEASE1 key. */
+	smb2_lease_create(&io, &ls, false, fname1, LEASE1,
+			  smb2_util_lease_state("RWH"));
+	status = smb2_create(tree, mem_ctx, &io);
+	CHECK_STATUS(status, NT_STATUS_OK);
+	h1 = io.out.file.handle;
+	CHECK_CREATED(&io, CREATED, FILE_ATTRIBUTE_ARCHIVE);
+	CHECK_LEASE(&io, "RWH", true, LEASE1, 0);
+
+	/*
+	 * Create file2 with the same LEASE1 key - this should fail with.
+	 * INVALID_PARAMETER.
+	 */
+	smb2_lease_create(&io, &ls, false, fname2, LEASE1,
+			  smb2_util_lease_state("RWH"));
+	status = smb2_create(tree, mem_ctx, &io);
+	CHECK_STATUS(status, NT_STATUS_INVALID_PARAMETER);
+	smb2_util_close(tree, h1);
+
+done:
+	smb2_util_close(tree, h2);
+	smb2_util_close(tree, h1);
+	smb2_util_unlink(tree, fname1);
+	smb2_util_unlink(tree, fname2);
+	talloc_free(mem_ctx);
+	return ret;
+}
+
+static bool test_lease_duplicate_open(struct torture_context *tctx,
+				   struct smb2_tree *tree)
+{
+	TALLOC_CTX *mem_ctx = talloc_new(tctx);
+	struct smb2_create io;
+	struct smb2_lease ls;
+	struct smb2_handle h1 = {{0}};
+	struct smb2_handle h2 = {{0}};
+	NTSTATUS status;
+	const char *fname1 = "duplicate_open1.dat";
+	const char *fname2 = "duplicate_open2.dat";
+	bool ret = true;
+	uint32_t caps;
+
+	caps = smb2cli_conn_server_capabilities(
+		tree->session->transport->conn);
+	if (!(caps & SMB2_CAP_LEASING)) {
+		torture_skip(tctx, "leases are not supported");
+	}
+
+	/* Ensure files don't exist. */
+	smb2_util_unlink(tree, fname1);
+	smb2_util_unlink(tree, fname2);
+
+	/* Create file1 - LEASE1 key. */
+	smb2_lease_create(&io, &ls, false, fname1, LEASE1,
+			  smb2_util_lease_state("RWH"));
+	status = smb2_create(tree, mem_ctx, &io);
+	CHECK_STATUS(status, NT_STATUS_OK);
+	h1 = io.out.file.handle;
+	CHECK_CREATED(&io, CREATED, FILE_ATTRIBUTE_ARCHIVE);
+	CHECK_LEASE(&io, "RWH", true, LEASE1, 0);
+
+	/* Leave file1 open and leased. */
+
+	/* Create file2 - no lease. */
+	smb2_lease_create(&io, NULL, false, fname2, 0,
+			  smb2_util_lease_state("RWH"));
+	status = smb2_create(tree, mem_ctx, &io);
+	CHECK_STATUS(status, NT_STATUS_OK);
+	h2 = io.out.file.handle;
+	CHECK_CREATED(&io, CREATED, FILE_ATTRIBUTE_ARCHIVE);
+	/* Close it. */
+	smb2_util_close(tree, h2);
+
+	/*
+	 * Try and open file2 with the same LEASE1 key - this should fail with.
+	 * INVALID_PARAMETER.
+	 */
+	smb2_lease_create(&io, &ls, false, fname2, LEASE1,
+			  smb2_util_lease_state("RWH"));
+	status = smb2_create(tree, mem_ctx, &io);
+	CHECK_STATUS(status, NT_STATUS_INVALID_PARAMETER);
+	/*
+	 * If we did open this is an error, but save off
+	 * the handle so we close below.
+	 */
+	h2 = io.out.file.handle;
+
+done:
+	smb2_util_close(tree, h2);
+	smb2_util_close(tree, h1);
+	smb2_util_unlink(tree, fname1);
+	smb2_util_unlink(tree, fname2);
+	talloc_free(mem_ctx);
+	return ret;
+}
+
 struct torture_suite *torture_smb2_lease_init(TALLOC_CTX *ctx)
 {
 	struct torture_suite *suite =
@@ -4480,6 +4600,10 @@ struct torture_suite *torture_smb2_lease_init(TALLOC_CTX *ctx)
 	torture_suite_add_1smb2_test(suite, "timeout-disconnect", test_lease_timeout_disconnect);
 	torture_suite_add_1smb2_test(suite, "rename_wait",
 				test_lease_rename_wait);
+	torture_suite_add_1smb2_test(suite, "duplicate_create",
+				test_lease_duplicate_create);
+	torture_suite_add_1smb2_test(suite, "duplicate_open",
+				test_lease_duplicate_open);
 
 	suite->description = talloc_strdup(suite, "SMB2-LEASE tests");
 


-- 
Samba Shared Repository



More information about the samba-cvs mailing list