From 466fbeb979e5f9d659da5d43f4908d6151ae073b Mon Sep 17 00:00:00 2001 From: Joseph Sutton 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 Reviewed-by: Douglas Bagnall Reviewed-by: Andrew Bartlett (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 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 Reviewed-by: Andrew Bartlett Reviewed-by: Andreas Schneider (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 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 Reviewed-by: Andrew Bartlett Reviewed-by: Andreas Schneider (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 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 Reviewed-by: Andrew Bartlett Reviewed-by: Andreas Schneider (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 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 Reviewed-by: Andrew Bartlett Reviewed-by: Andreas Schneider (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 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 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 Reviewed-by: Douglas Bagnall Reviewed-by: Andrew Bartlett BUG: https://bugzilla.samba.org/show_bug.cgi?id=14984 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett Reviewed-by: Andreas Schneider Autobuild-User(master): Stefan Metzmacher 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