Attachment 17186 Details for Bug 14984 – Patches for v4-14-test

[patch] Patches for v4-14-test

bfixes-tmp414.txt (text/plain), 11.69 KB, created by Stefan Metzmacher on 2022-03-02 10:38:08 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2022-03-02 10:38:08 UTC

Size: 11.69 KB

patch

obsolete

>From 466fbeb979e5f9d659da5d43f4908d6151ae073b Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Wed, 24 Feb 2021 02:03:25 +1300
>Subject: [PATCH 1/6] 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>
>(similar to commit 609ca657652862fd9c81fd11f818efb74f72ff55)
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14984
>---
> python/samba/join.py               | 2 +-
> python/samba/provision/__init__.py | 2 +-
> source4/libnet/libnet_vampire.c    | 2 +-
> source4/scripting/bin/renamedc     | 2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/python/samba/join.py b/python/samba/join.py
>index 79030cdfd297..d31cabb945a5 100644
>--- a/python/samba/join.py
>+++ b/python/samba/join.py
>@@ -136,7 +136,7 @@ class DCJoinContext(object):
>         if machinepass is not None:
>             ctx.acct_pass = machinepass
>         else:
>-            ctx.acct_pass = samba.generate_random_machine_password(128, 255)
>+            ctx.acct_pass = samba.generate_random_machine_password(120, 120)
> 
>         ctx.dnsdomain = ctx.samdb.domain_dns_name()
> 
>diff --git a/python/samba/provision/__init__.py b/python/samba/provision/__init__.py
>index 136267e7aad9..bdbe9b99a71a 100644
>--- a/python/samba/provision/__init__.py
>+++ b/python/samba/provision/__init__.py
>@@ -1926,7 +1926,7 @@ def provision_fill(samdb, secrets_ldb, logger, names, paths,
>     if krbtgtpass is None:
>         krbtgtpass = samba.generate_random_machine_password(128, 255)
>     if machinepass is None:
>-        machinepass = samba.generate_random_machine_password(128, 255)
>+        machinepass = samba.generate_random_machine_password(120, 120)
>     if dnspass is None:
>         dnspass = samba.generate_random_password(128, 255)
> 
>diff --git a/source4/libnet/libnet_vampire.c b/source4/libnet/libnet_vampire.c
>index a0de1b7d3e00..3f07b3f20d68 100644
>--- a/source4/libnet/libnet_vampire.c
>+++ b/source4/libnet/libnet_vampire.c
>@@ -164,7 +164,7 @@ NTSTATUS libnet_vampire_cb_prepare_db(void *private_data,
> 	settings.realm = s->realm;
> 	settings.domain = s->domain_name;
> 	settings.server_dn_str = p->dest_dsa->server_dn_str;
>-	settings.machine_password = generate_random_machine_password(s, 128, 255);
>+	settings.machine_password = generate_random_machine_password(s, 120, 120);
> 	settings.targetdir = s->targetdir;
> 	settings.use_ntvfs = true;
> 	status = provision_bare(s, s->lp_ctx, &settings, &result);
>diff --git a/source4/scripting/bin/renamedc b/source4/scripting/bin/renamedc
>index 6a9bd1c82bd6..ef3aa75db76d 100755
>--- a/source4/scripting/bin/renamedc
>+++ b/source4/scripting/bin/renamedc
>@@ -95,7 +95,7 @@ if __name__ == '__main__':
> 
>     # Then change password and samaccountname and dnshostname
>     msg = ldb.Message(newdn)
>-    machinepass = samba.generate_random_machine_password(128, 255)
>+    machinepass = samba.generate_random_machine_password(120, 120)
>     mputf16 = machinepass.encode('utf-16-le')
> 
>     account = "%s$" % opts.newname.upper()
>-- 
>2.25.1
>
>
>From 2a8d456018fe7dea84381edcb0faa9eb7844c49e Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Mon, 21 Feb 2022 15:08:34 +0100
>Subject: [PATCH 2/6] 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 bdbe9b99a71a..b21b411b51ae 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 2e65aabbbf27b6da57466845e9d5c2a7309b7885 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Mon, 21 Feb 2022 15:22:06 +0100
>Subject: [PATCH 3/6] 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 69f6e3675e8e..ee71b2972bc2 100644
>--- a/python/samba/upgradehelpers.py
>+++ b/python/samba/upgradehelpers.py
>@@ -584,7 +584,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 492725b397f39a69493448ea9dfbf4f86d8ced73 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Mon, 21 Feb 2022 15:22:50 +0100
>Subject: [PATCH 4/6] 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 b21b411b51ae..e8903ad846f5 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 2a3b6ae1899323843d8da656a196cf636b415270 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Mon, 21 Feb 2022 15:23:54 +0100
>Subject: [PATCH 5/6] 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 ee71b2972bc2..0f4e65fcaedc 100644
>--- a/python/samba/upgradehelpers.py
>+++ b/python/samba/upgradehelpers.py
>@@ -660,9 +660,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 0b4a1abd6c049ac3b76bcba4eefe41dbb7783fb1 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Mon, 21 Feb 2022 15:28:53 +0100
>Subject: [PATCH 6/6] 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