The branch, master has been updated via 112faff82f9 dsdb: modify unicodePwd requires encrypted connection via 928de1d61c8 dsdb/tests: Add test for modification of unicodePwd over a cleartext/signed connection via 5abda27f0e2 dsdb: fix spelling in password_hash.c via 479634e4cd6 dsdb/tests: Double number of expressions in large_ldap.py ldap_timeout test via e1c0c2066c2 dsdb/tests: Move SD modification on class-created objects to classSetUp from b74b9f4b06c CVE-2023-0922 set default ldap client sasl wrapping to seal
https://git.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit 112faff82f93f9b16f67905c5cbdd5806bd7c214 Author: Rob van der Linde <r...@catalyst.net.nz> Date: Mon Feb 20 11:50:36 2023 +1300 dsdb: modify unicodePwd requires encrypted connection Signed-off-by: Rob van der Linde <r...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> Reviewed-by: Joseph Sutton <josephsut...@catalyst.net.nz> Autobuild-User(master): Andrew Bartlett <abart...@samba.org> Autobuild-Date(master): Thu Apr 6 01:33:05 UTC 2023 on atb-devel-224 commit 928de1d61c884c7691b57fbe5fffa8f792ce68fd Author: Rob van der Linde <r...@catalyst.net.nz> Date: Wed Apr 5 12:30:03 2023 +1200 dsdb/tests: Add test for modification of unicodePwd over a cleartext/signed connection This demonstrates that the server did not detect CVE-2023-0922 Signed-off-by: Andrew Bartlett <abart...@samba.org> Signed-off-by: Rob van der Linde <r...@catalyst.net.nz> Reviewed-by: Joseph Sutton <josephsut...@catalyst.net.nz> commit 5abda27f0e2db9738f81c86a25929462ed6189ce Author: Rob van der Linde <r...@catalyst.net.nz> Date: Thu Feb 16 13:23:42 2023 +1300 dsdb: fix spelling in password_hash.c Signed-off-by: Rob van der Linde <r...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> Reviewed-by: Joseph Sutton <josephsut...@catalyst.net.nz> commit 479634e4cd6543d489eb4700aebde1a479b94fe5 Author: Andrew Bartlett <abart...@samba.org> Date: Thu Apr 6 08:59:17 2023 +1200 dsdb/tests: Double number of expressions in large_ldap.py ldap_timeout test By slowing the filter down more this makes the test reliable on the autobuild host. This is not a long-term solution, but is a quick tweak that can be done today to address current issues with getting commits past the host-based (compared with cloud-based) autobuild. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15351 Signed-off-by: Andrew Bartlett <abart...@samba.org> Reviewed-by: Joseph Sutton <josephsut...@catalyst.net.nz> commit e1c0c2066c2f29bb614e3386b796eec3cb289aea Author: Andrew Bartlett <abart...@samba.org> Date: Thu Apr 6 08:54:02 2023 +1200 dsdb/tests: Move SD modification on class-created objects to classSetUp These modifications persist, so should be done at the class level, not in the test. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15351 Signed-off-by: Andrew Bartlett <abart...@samba.org> Reviewed-by: Joseph Sutton <josephsut...@catalyst.net.nz> ----------------------------------------------------------------------- Summary of changes: source4/dsdb/samdb/ldb_modules/password_hash.c | 24 +++- source4/dsdb/samdb/samdb.h | 5 + source4/dsdb/tests/python/large_ldap.py | 20 ++- source4/dsdb/tests/python/unicodepwd_encrypted.py | 151 ++++++++++++++++++++++ source4/ldap_server/ldap_backend.c | 23 ++++ source4/selftest/tests.py | 1 + 6 files changed, 211 insertions(+), 13 deletions(-) create mode 100644 source4/dsdb/tests/python/unicodepwd_encrypted.py Changeset truncated at 500 lines: diff --git a/source4/dsdb/samdb/ldb_modules/password_hash.c b/source4/dsdb/samdb/ldb_modules/password_hash.c index 6a713b86736..417e34b79e6 100644 --- a/source4/dsdb/samdb/ldb_modules/password_hash.c +++ b/source4/dsdb/samdb/ldb_modules/password_hash.c @@ -252,7 +252,7 @@ static int password_hash_bypass(struct ldb_module *module, struct ldb_request *r GET_VALUES(nte, "unicodePwd"); /* - * Even as Samba contiuues to ignore the LM hash, and reset it + * Even as Samba continues to ignore the LM hash, and reset it * when practical, we keep the constraint that it must be a 16 * byte value if specified. */ @@ -2869,6 +2869,8 @@ static int check_password_restrictions(struct setup_password_fields_io *io, WERR struct loadparm_context *lp_ctx = talloc_get_type(ldb_get_opaque(ldb, "loadparm"), struct loadparm_context); + struct dsdb_encrypted_connection_state *opaque_connection_state = + ldb_get_opaque(ldb,DSDB_OPAQUE_ENCRYPTED_CONNECTION_STATE_NAME); *werror = WERR_INVALID_PARAMETER; @@ -2876,10 +2878,28 @@ static int check_password_restrictions(struct setup_password_fields_io *io, WERR return LDB_SUCCESS; } + /* + * Prevent update password on an insecure connection. + * The opaque is added in the ldap backend init. + */ + if (opaque_connection_state != NULL && + !opaque_connection_state->using_encrypted_connection) { + ret = LDB_ERR_UNWILLING_TO_PERFORM; + *werror = WERR_GEN_FAILURE; + ldb_asprintf_errstring(ldb, + "%08X: SvcErr: DSID-031A126C, " + "problem 5003 (WILL_NOT_PERFORM), " + "data 0\n" + "Password modification over LDAP " + "must be over an encrypted connection", + W_ERROR_V(*werror)); + return ret; + } + /* * First check the old password is correct, for password * changes when this hasn't already been checked by a - * trustwrothy layer above + * trustworthy layer above */ if (!io->ac->pwd_reset && !(io->ac->change && io->ac->change->old_password_checked == DSDB_PASSWORD_CHECKED_AND_CORRECT)) { diff --git a/source4/dsdb/samdb/samdb.h b/source4/dsdb/samdb/samdb.h index 8bc67301a98..7ca6d26f67a 100644 --- a/source4/dsdb/samdb/samdb.h +++ b/source4/dsdb/samdb/samdb.h @@ -375,6 +375,11 @@ struct dsdb_extended_dn_store_format { #define DSDB_FULL_JOIN_REPLICATION_COMPLETED_OPAQUE_NAME "DSDB_FULL_JOIN_REPLICATION_COMPLETED" +#define DSDB_OPAQUE_ENCRYPTED_CONNECTION_STATE_NAME "DSDB_OPAQUE_ENCRYPTED_CONNECTION_STATE_MSG" +struct dsdb_encrypted_connection_state { + bool using_encrypted_connection; +}; + #define DSDB_SAMDB_MINIMUM_ALLOWED_RID 1000 #define DSDB_METADATA_SCHEMA_SEQ_NUM "SCHEMA_SEQ_NUM" diff --git a/source4/dsdb/tests/python/large_ldap.py b/source4/dsdb/tests/python/large_ldap.py index 9f9e23fb76e..02472baef5a 100644 --- a/source4/dsdb/tests/python/large_ldap.py +++ b/source4/dsdb/tests/python/large_ldap.py @@ -145,6 +145,14 @@ class LargeLDAPTest(samba.tests.TestCase): "sAMAccountName": user_name, "jpegPhoto": b'a' * (2 * 1024 * 1024)}) + ace = "(OD;;RP;{6bc69afa-7bd9-4184-88f5-28762137eb6a};;S-1-%d)" % x + dn = ldb.Dn(cls.ldb, "cn=" + user_name + "," + str(cls.ou_dn)) + + # add an ACE that denies access to the above random attr + # for a not-existing user. This makes each SD distinct + # and so will slow SD parsing. + cls.sd_utils.dacl_add_ace(dn, ace) + @classmethod def tearDownClass(cls): # Remake the connection for tear-down (old Samba drops the socket) @@ -289,19 +297,9 @@ class LargeLDAPTest(samba.tests.TestCase): session_info=system_session(lp), lp=lp) - for x in range(200): - user_name = self.USER_NAME + format(x, "03") - ace = "(OD;;RP;{6bc69afa-7bd9-4184-88f5-28762137eb6a};;S-1-%d)" % x - dn = ldb.Dn(self.ldb, "cn=" + user_name + "," + str(self.ou_dn)) - - # add an ACE that denies access to the above random attr - # for a not-existing user. This makes each SD distinct - # and so will slow SD parsing. - self.sd_utils.dacl_add_ace(dn, ace) - # Create a large search expression that will take a long time to # evaluate. - expression = f'(jpegPhoto=*X*)' * 1000 + expression = '(jpegPhoto=*X*)' * 2000 expression = f'(|{expression})' # Perform the LDAP search. diff --git a/source4/dsdb/tests/python/unicodepwd_encrypted.py b/source4/dsdb/tests/python/unicodepwd_encrypted.py new file mode 100644 index 00000000000..768cbf83f8e --- /dev/null +++ b/source4/dsdb/tests/python/unicodepwd_encrypted.py @@ -0,0 +1,151 @@ +#!/usr/bin/env python3 +# -*- coding: utf-8 -*- +import sys +import optparse + +sys.path.insert(0, "bin/python") +import samba.getopt as options +from ldb import Message, MessageElement, Dn +from ldb import LdbError, FLAG_MOD_REPLACE, ERR_UNWILLING_TO_PERFORM, SCOPE_BASE +from samba import gensec +from samba.auth import system_session +from samba.samdb import SamDB +from samba.tests import delete_force +from samba.tests.password_test import PasswordTestCase +from samba.tests.subunitrun import SubunitOptions, TestProgram + +parser = optparse.OptionParser("unicodepwd_encrypted.py [options] <host>") +sambaopts = options.SambaOptions(parser) +parser.add_option_group(sambaopts) +parser.add_option_group(options.VersionOptions(parser)) +# use command line creds if available +credopts = options.CredentialsOptions(parser) +parser.add_option_group(credopts) +subunitopts = SubunitOptions(parser) +parser.add_option_group(subunitopts) +lp = sambaopts.get_loadparm() +opts, args = parser.parse_args() + +if len(args) < 1: + parser.print_usage() + sys.exit(1) + +host = args[0] +host_ldaps = f"ldaps://{host}" +host_ldap = f"ldap://{host}" + + +class UnicodePwdEncryptedConnectionTests(PasswordTestCase): + + def setUp(self): + super().setUp() + self.creds = self.insta_creds(template=credopts.get_credentials(lp)) + self.ldb = SamDB(host_ldap, credentials=self.creds, + session_info=system_session(lp), + lp=lp) + self.base_dn = self.ldb.domain_dn() + self.user_dn_str = f"cn=testuser,cn=users,{self.base_dn}" + self.user_dn = Dn(self.ldb, self.user_dn_str) + print(f"baseDN: {self.base_dn}\n") + + # permit password changes during this test + self.allow_password_changes() + + # (Re)adds the test user "testuser" with no password. + delete_force(self.ldb, str(self.user_dn)) + self.ldb.add({ + "dn": str(self.user_dn), + "objectclass": "user", + "sAMAccountName": "testuser" + }) + + # Set the test user initial password and enable account. + m = Message(self.user_dn) + m["0"] = MessageElement("Password#2", FLAG_MOD_REPLACE, "userPassword") + self.ldb.modify(m) + self.ldb.enable_account("(sAMAccountName=testuser)") + + def modify_unicode_pwd(self, ldb, password): + """Replaces user password using unicodePwd.""" + m = Message() + m.dn = self.user_dn + m["unicodePwd"] = MessageElement( + f'"{password}"'.encode('utf-16-le'), + FLAG_MOD_REPLACE, "unicodePwd" + ) + ldb.modify(m) + + def get_admin_sid(self, ldb): + res = self.ldb.search( + base="", expression="", scope=SCOPE_BASE, attrs=["tokenGroups"]) + + return self.ldb.schema_format_value( + "tokenGroups", res[0]["tokenGroups"][0]).decode("utf8") + + def test_with_seal(self): + """Test unicodePwd on connection with seal. + + This should allow unicodePwd. + """ + self.modify_unicode_pwd(self.ldb, "thatsAcomplPASS2") + + def test_without_seal(self): + """Test unicodePwd on connection without seal. + + Should not allow unicodePwd on an unencrypted connection. + + Requires --use-kerberos=required, or it automatically upgrades + to an encrypted connection. + """ + # Remove FEATURE_SEAL which gets added by insta_creds. + creds_noseal = self.insta_creds(template=credopts.get_credentials(lp)) + creds_noseal.set_gensec_features(creds_noseal.get_gensec_features() & + ~gensec.FEATURE_SEAL) + + sasl_wrap = lp.get('client ldap sasl wrapping') + self.addCleanup(lp.set, 'client ldap sasl wrapping', sasl_wrap) + lp.set('client ldap sasl wrapping', 'sign') + + # Create a second ldb connection without seal. + ldb = SamDB(host_ldap, credentials=creds_noseal, + session_info=system_session(lp), + lp=lp) + + with self.assertRaises(LdbError) as e: + self.modify_unicode_pwd(ldb, "thatsAcomplPASS2") + + # Server should not allow unicodePwd on an unencrypted connection. + self.assertEqual(e.exception.args[0], ERR_UNWILLING_TO_PERFORM) + self.assertIn( + "Password modification over LDAP must be over an encrypted connection", + e.exception.args[1] + ) + + def test_simple_bind_plain(self): + """Test unicodePwd using simple bind without encryption.""" + admin_sid = self.get_admin_sid(self.ldb) + + self.creds.set_bind_dn(admin_sid) + ldb = SamDB(url=host_ldap, credentials=self.creds, lp=lp) + + with self.assertRaises(LdbError) as e: + self.modify_unicode_pwd(ldb, "thatsAcomplPASS2") + + # Server should not allow unicodePwd on an unencrypted connection. + self.assertEqual(e.exception.args[0], ERR_UNWILLING_TO_PERFORM) + self.assertIn( + "Password modification over LDAP must be over an encrypted connection", + e.exception.args[1] + ) + + def test_simple_bind_tls(self): + """Test unicodePwd using simple bind with encryption.""" + admin_sid = self.get_admin_sid(self.ldb) + + self.creds.set_bind_dn(admin_sid) + ldb = SamDB(url=host_ldaps, credentials=self.creds, lp=lp) + + self.modify_unicode_pwd(ldb, "thatsAcomplPASS2") + + +TestProgram(module=__name__, opts=subunitopts) diff --git a/source4/ldap_server/ldap_backend.c b/source4/ldap_server/ldap_backend.c index 9429ba1561f..8db85c58fac 100644 --- a/source4/ldap_server/ldap_backend.c +++ b/source4/ldap_server/ldap_backend.c @@ -186,6 +186,11 @@ static int map_ldb_error(TALLOC_CTX *mem_ctx, int ldb_err, int ldapsrv_backend_Init(struct ldapsrv_connection *conn, char **errstring) { + bool using_tls = conn->sockets.active == conn->sockets.tls; + bool using_seal = conn->gensec != NULL && gensec_have_feature(conn->gensec, + GENSEC_FEATURE_SEAL); + struct dsdb_encrypted_connection_state *opaque_connection_state = NULL; + int ret = samdb_connect_url(conn, conn->connection->event.ctx, conn->lp_ctx, @@ -199,6 +204,24 @@ int ldapsrv_backend_Init(struct ldapsrv_connection *conn, return ret; } + /* + * We can safely call ldb_set_opaque() on this ldb as we have + * set remote_address above which avoids the ldb handle cache + */ + opaque_connection_state = talloc_zero(conn, struct dsdb_encrypted_connection_state); + if (opaque_connection_state == NULL) { + return LDB_ERR_OPERATIONS_ERROR; + } + opaque_connection_state->using_encrypted_connection = using_tls || using_seal; + ret = ldb_set_opaque(conn->ldb, + DSDB_OPAQUE_ENCRYPTED_CONNECTION_STATE_NAME, + opaque_connection_state); + if (ret != LDB_SUCCESS) { + DBG_ERR("ldb_set_opaque() failed to store our " + "encrypted connection state!"); + return ret; + } + if (conn->server_credentials) { struct gensec_security *gensec_security = NULL; const char **sasl_mechs = NULL; diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index f348adf347d..fc38c09966d 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -1321,6 +1321,7 @@ plantestsuite("samba4.asq.python(ad_dc_default)", "ad_dc_default", [python, os.p plantestsuite("samba4.user_account_control.python(ad_dc_default)", "ad_dc_default", [python, os.path.join(DSDB_PYTEST_DIR, "user_account_control.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN']) plantestsuite("samba4.priv_attrs.python(ad_dc_default)", "ad_dc_default", ["STRICT_CHECKING=0", python, os.path.join(DSDB_PYTEST_DIR, "priv_attrs.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN']) plantestsuite("samba4.priv_attrs.strict.python(ad_dc_default)", "ad_dc_default", [python, os.path.join(DSDB_PYTEST_DIR, "priv_attrs.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN']) +plantestsuite("samba4.unicodepwd_encrypted(fl2008r2dc)", "fl2008r2dc", [python, os.path.join(DSDB_PYTEST_DIR, "unicodepwd_encrypted.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN']) for env in ['ad_dc_default:local', 'schema_dc:local']: planoldpythontestsuite(env, "dsdb_schema_info", -- Samba Shared Repository