Attachment 17184 Details for Bug 14984 – Patches for v4-16-test

[patch] Patches for v4-16-test

bfixes-tmp416.txt (text/plain), 10.76 KB, created by Stefan Metzmacher on 2022-03-02 10:37:10 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2022-03-02 10:37:10 UTC

Size: 10.76 KB

patch

obsolete

>From 8c89386bb130d398ef1ed21c988b804d4a4f4bcd Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Mon, 21 Feb 2022 23:48:37 +0100
>Subject: [PATCH 1/7] 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@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Andreas Schneider <asn@samba.org>
>(cherry picked from commit 576bdb08c51c47c390cc390fbefdcfee275b7f0f)
>---
> source3/utils/py_net.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source3/utils/py_net.c b/source3/utils/py_net.c
>index 3142f83bc7f7..0d774bcb805d 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,
>-- 
>2.25.1
>
>
>From bfdfb812b696ee7f1848dc5c5e47ed0625d8f41e Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Mon, 21 Feb 2022 15:03:22 +0100
>Subject: [PATCH 2/7] 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@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Andreas Schneider <asn@samba.org>
>(cherry picked from commit 59ac782452c4993274fa837256a8b9c5675e707b)
>---
> python/samba/netcmd/domain.py | 2 --
> 1 file changed, 2 deletions(-)
>
>diff --git a/python/samba/netcmd/domain.py b/python/samba/netcmd/domain.py
>index 1bdc0ee535a7..e814a47233da 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,
>-- 
>2.25.1
>
>
>From c023167c11bbde3b08cf2dea082d9dba0f283c73 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Mon, 21 Feb 2022 15:08:34 +0100
>Subject: [PATCH 3/7] 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@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Andreas Schneider <asn@samba.org>
>(cherry picked from commit 3b91be36581de1007427d539daffdaa62752412d)
>---
> python/samba/provision/__init__.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/python/samba/provision/__init__.py b/python/samba/provision/__init__.py
>index 1723d9935d41..2f513f8383dd 100644
>--- a/python/samba/provision/__init__.py
>+++ b/python/samba/provision/__init__.py
>@@ -1928,7 +1928,7 @@ def provision_fill(samdb, secrets_ldb, logger, names, paths,
>     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:
>-- 
>2.25.1
>
>
>From 5ad8ef3b66e578c5fd100d3e4613eaf34d58e5ad Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Mon, 21 Feb 2022 15:22:06 +0100
>Subject: [PATCH 4/7] 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@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Andreas Schneider <asn@samba.org>
>(cherry picked from commit 6bb7c0f24918329804b7f4fb71908e8fab99e266)
>---
> python/samba/upgradehelpers.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/python/samba/upgradehelpers.py b/python/samba/upgradehelpers.py
>index 7f92b45f3fb2..febde7cec401 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,
>-- 
>2.25.1
>
>
>From 90f874a0bf6e06c40b659e14e69a9229ecb2daea Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Mon, 21 Feb 2022 15:22:50 +0100
>Subject: [PATCH 5/7] 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@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Andreas Schneider <asn@samba.org>
>(cherry picked from commit 725c94d57d3d656bc94633dacbac683a4c11d3e6)
>---
> python/samba/provision/__init__.py | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/python/samba/provision/__init__.py b/python/samba/provision/__init__.py
>index 2f513f8383dd..ff9b8fac916a 100644
>--- a/python/samba/provision/__init__.py
>+++ b/python/samba/provision/__init__.py
>@@ -1924,6 +1924,9 @@ 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)
>-- 
>2.25.1
>
>
>From 141a386cc1d4a856ecf988fba9fe51edee064e3d Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Mon, 21 Feb 2022 15:23:54 +0100
>Subject: [PATCH 6/7] 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@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Andreas Schneider <asn@samba.org>
>(cherry picked from commit ad0b5561b492dfa28acfc9604b2358bb8b490703)
>---
> python/samba/upgradehelpers.py | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
>diff --git a/python/samba/upgradehelpers.py b/python/samba/upgradehelpers.py
>index febde7cec401..c853668058e7 100644
>--- a/python/samba/upgradehelpers.py
>+++ b/python/samba/upgradehelpers.py
>@@ -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")
> 
>-- 
>2.25.1
>
>
>From ed85d5879c1ad0f5b2f914f6bc4bd666b98254c2 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Mon, 21 Feb 2022 15:28:53 +0100
>Subject: [PATCH 7/7] 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@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@catalyst.net.nz>
>        Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>        Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14984
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Andreas Schneider <asn@samba.org>
>
>Autobuild-User(master): Stefan Metzmacher <metze@samba.org>
>Autobuild-Date(master): Wed Feb 23 08:49:54 UTC 2022 on sn-devel-184
>
>(cherry picked from commit 5e2386336c49fab46c1192db972af5da1e916b32)
>---
> source3/libsmb/trusts_util.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
>diff --git a/source3/libsmb/trusts_util.c b/source3/libsmb/trusts_util.c
>index 55e3c74494ab..71e1a35eba7f 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:
>-- 
>2.25.1
>

Flags:

asn: review+

Actions: View

Attachments on bug 14984: 17184 | 17185 | 17186