From c37b8982a355211b73ac8dbc5e94ab96ce2518a5 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 6 Jul 2022 15:36:10 +1200 Subject: [PATCH 01/21] CVE-2021-20251 smbdes: Make key parameter to E_old_pw_hash() const BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Joseph Sutton --- libcli/auth/proto.h | 2 +- libcli/auth/smbdes.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/libcli/auth/proto.h b/libcli/auth/proto.h index a62668f088f..9f3ea960ca4 100644 --- a/libcli/auth/proto.h +++ b/libcli/auth/proto.h @@ -229,7 +229,7 @@ int des_crypt56_gnutls(uint8_t out[8], const uint8_t in[8], const uint8_t key[7] enum samba_gnutls_direction encrypt); int E_P16(const uint8_t *p14,uint8_t *p16); int E_P24(const uint8_t *p21, const uint8_t *c8, uint8_t *p24); -int E_old_pw_hash( uint8_t *p14, const uint8_t *in, uint8_t *out); +int E_old_pw_hash(const uint8_t *p14, const uint8_t *in, uint8_t *out); int des_crypt128(uint8_t out[8], const uint8_t in[8], const uint8_t key[16]); int des_crypt112(uint8_t out[8], const uint8_t in[8], const uint8_t key[14], enum samba_gnutls_direction encrypt); diff --git a/libcli/auth/smbdes.c b/libcli/auth/smbdes.c index c6c44419306..0e7052d9a99 100644 --- a/libcli/auth/smbdes.c +++ b/libcli/auth/smbdes.c @@ -127,7 +127,7 @@ int E_P24(const uint8_t *p21, const uint8_t *c8, uint8_t *p24) return des_crypt56_gnutls(p24+16, c8, p21+14, SAMBA_GNUTLS_ENCRYPT); } -int E_old_pw_hash( uint8_t *p14, const uint8_t *in, uint8_t *out) +int E_old_pw_hash(const uint8_t *p14, const uint8_t *in, uint8_t *out) { int ret; -- 2.35.0 From 2819beec42b312ac068ed43a0b323d05e63618ba Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 6 Jul 2022 15:36:26 +1200 Subject: [PATCH 02/21] CVE-2021-20251 lib:crypto: Add old_pw_hash() for encrypting NT hash with DES We can't otherwise access single-DES from Python. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Joseph Sutton --- lib/crypto/py_crypto.c | 63 ++++++++++++++++++++++++++++++++++++++++ lib/crypto/wscript_build | 2 +- 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/lib/crypto/py_crypto.c b/lib/crypto/py_crypto.c index ad18d3ada0f..0368bea1e6c 100644 --- a/lib/crypto/py_crypto.c +++ b/lib/crypto/py_crypto.c @@ -25,6 +25,7 @@ #include #include #include "lib/crypto/gnutls_helpers.h" +#include "libcli/auth/libcli_auth.h" static PyObject *py_crypto_arcfour_crypt_blob(PyObject *module, PyObject *args) { @@ -100,13 +101,75 @@ static PyObject *py_crypto_set_strict_mode(PyObject *module) Py_RETURN_NONE; } +static PyObject *py_old_pw_hash(PyObject *self, PyObject *args) +{ + PyObject *key_obj = NULL; + uint8_t *key_buf = NULL; + Py_ssize_t key_size; + + PyObject *nt_hash_obj = NULL; + uint8_t *nt_hash_buf = NULL; + Py_ssize_t nt_hash_size; + + uint8_t result_buffer[16]; + + bool ok; + int ret; + + ok = PyArg_ParseTuple(args, "SS", + &key_obj, &nt_hash_obj); + if (!ok) { + return NULL; + } + + ret = PyBytes_AsStringAndSize(key_obj, + (char **)&key_buf, + &key_size); + if (ret != 0) { + return NULL; + } + + ret = PyBytes_AsStringAndSize(nt_hash_obj, + (char **)&nt_hash_buf, + &nt_hash_size); + if (ret != 0) { + return NULL; + } + + if (key_size != 14) { + return PyErr_Format(PyExc_ValueError, + "Expected key size of 14 bytes; got %zd", + key_size); + } + + if (nt_hash_size != 16) { + return PyErr_Format(PyExc_ValueError, + "Expected NT hash size of 16 bytes; got %zd", + nt_hash_size); + } + + ret = E_old_pw_hash(key_buf, nt_hash_buf, result_buffer); + if (ret != 0) { + return PyErr_Format(PyExc_RuntimeError, + "E_old_pw_hash() failed: %d", + ret); + } + + return PyBytes_FromStringAndSize((const char *)result_buffer, + sizeof(result_buffer)); +} + static const char py_crypto_arcfour_crypt_blob_doc[] = "arcfour_crypt_blob(data, key)\n" "Encrypt the data with RC4 algorithm using the key"; +static const char py_old_pw_hash_doc[] = "old_pw_hash(key, nt_hash) -> bytes\n" + "Returns the NT hash DES-encrypted with a 14-byte key."; + static PyMethodDef py_crypto_methods[] = { { "arcfour_crypt_blob", (PyCFunction)py_crypto_arcfour_crypt_blob, METH_VARARGS, py_crypto_arcfour_crypt_blob_doc }, { "set_relax_mode", (PyCFunction)py_crypto_set_relax_mode, METH_NOARGS, "Set fips to relax mode" }, { "set_strict_mode", (PyCFunction)py_crypto_set_strict_mode, METH_NOARGS, "Set fips to strict mode" }, + { "old_pw_hash", (PyCFunction)py_old_pw_hash, METH_VARARGS, py_old_pw_hash_doc }, {0}, }; diff --git a/lib/crypto/wscript_build b/lib/crypto/wscript_build index e5766042541..0d5b548e7f8 100644 --- a/lib/crypto/wscript_build +++ b/lib/crypto/wscript_build @@ -50,5 +50,5 @@ bld.SAMBA_SUBSYSTEM('TORTURE_LIBCRYPTO', bld.SAMBA_PYTHON('python_crypto', source='py_crypto.c', - deps='gnutls talloc', + deps='gnutls talloc LIBCLI_AUTH', realname='samba/crypto.so') -- 2.35.0 From 28f8243f64f64539425763d6724f6a8425c204c3 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Mon, 4 Jul 2022 20:48:48 +1200 Subject: [PATCH 03/21] CVE-2021-20251 tests/krb5: Add tests for password lockout race BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Joseph Sutton --- python/samba/tests/krb5/lockout_tests.py | 616 +++++++++++++++++++ python/samba/tests/krb5/raw_testcase.py | 3 +- python/samba/tests/krb5/rfc4120_constants.py | 1 + python/samba/tests/usage.py | 1 + selftest/knownfail_heimdal_kdc | 9 + selftest/knownfail_mit_kdc | 9 + source4/selftest/tests.py | 4 + 7 files changed, 642 insertions(+), 1 deletion(-) create mode 100755 python/samba/tests/krb5/lockout_tests.py diff --git a/python/samba/tests/krb5/lockout_tests.py b/python/samba/tests/krb5/lockout_tests.py new file mode 100755 index 00000000000..5113218d52b --- /dev/null +++ b/python/samba/tests/krb5/lockout_tests.py @@ -0,0 +1,616 @@ +#!/usr/bin/env python3 +# Unix SMB/CIFS implementation. +# Copyright (C) Stefan Metzmacher 2020 +# Copyright (C) Catalyst.Net Ltd +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +from concurrent import futures +from multiprocessing import Pipe +import os +import sys +import time + +from cryptography.hazmat.backends import default_backend +from cryptography.hazmat.primitives.ciphers.base import Cipher +from cryptography.hazmat.primitives.ciphers import algorithms +import hashlib + +import ldb + +from samba import ( + NTSTATUSError, + dsdb, + generate_random_password, + ntstatus, + unix2nttime, + werror, +) +from samba.credentials import DONT_USE_KERBEROS, MUST_USE_KERBEROS +from samba.crypto import old_pw_hash +from samba.dcerpc import lsa, samr +from samba.samdb import SamDB + +from samba.tests import connect_samdb, env_get_var_value, env_loadparm + +from samba.tests.krb5.as_req_tests import AsReqBaseTest +from samba.tests.krb5 import kcrypto +from samba.tests.krb5.kdc_base_test import KDCBaseTest +from samba.tests.krb5.raw_testcase import KerberosCredentials +import samba.tests.krb5.rfc4120_pyasn1 as krb5_asn1 +from samba.tests.krb5.rfc4120_constants import ( + KDC_ERR_CLIENT_REVOKED, + KDC_ERR_PREAUTH_FAILED, + NT_PRINCIPAL, + NT_SRV_INST, +) + +sys.path.insert(0, 'bin/python') +os.environ['PYTHONUNBUFFERED'] = '1' + +global_asn1_print = False +global_hexdump = False + + +def connect_kdc(pipe, + url, + hostname, + username, + password, + domain, + realm, + workstation, + dn, + is_correct_pw): + AsReqBaseTest.setUpClass() + as_req_base = AsReqBaseTest() + as_req_base.setUp() + + user_creds = KerberosCredentials() + user_creds.set_username(username) + user_creds.set_password(password) + user_creds.set_domain(domain) + user_creds.set_realm(realm) + user_creds.set_workstation(workstation) + user_creds.set_kerberos_state(DONT_USE_KERBEROS) + + user_name = user_creds.get_username() + cname = as_req_base.PrincipalName_create(name_type=NT_PRINCIPAL, + names=user_name.split('/')) + + krbtgt_creds = as_req_base.get_krbtgt_creds() + krbtgt_supported_etypes = krbtgt_creds.tgs_supported_enctypes + realm = krbtgt_creds.get_realm() + + krbtgt_account = krbtgt_creds.get_username() + sname = as_req_base.PrincipalName_create(name_type=NT_SRV_INST, + names=[krbtgt_account, realm]) + + expected_salt = user_creds.get_salt() + + till = as_req_base.get_KerberosTime(offset=36000) + + kdc_options = krb5_asn1.KDCOptions('postdated') + + preauth_key = as_req_base.PasswordKey_from_creds(user_creds, + kcrypto.Enctype.AES256) + + ts_enc_padata = as_req_base.get_enc_timestamp_pa_data_from_key(preauth_key) + padata = [ts_enc_padata] + + krbtgt_decryption_key = ( + as_req_base.TicketDecryptionKey_from_creds(krbtgt_creds)) + + etypes = as_req_base.get_default_enctypes() + + if is_correct_pw: + expected_error_mode = KDC_ERR_CLIENT_REVOKED + else: + expected_error_mode = KDC_ERR_PREAUTH_FAILED + + # Indicate that we're ready. This ensures we hit the right transaction + # lock. + pipe.send_bytes(b'0') + + # Wait for the main process to take out a transaction lock. + if not pipe.poll(timeout=5): + raise AssertionError('main process failed to indicate readiness') + + as_req_base.get_samdb() + as_req_base.get_admin_creds() + + # Try making a Kerberos AS-REQ to the KDC. This should fail, either due to + # the user's account being locked out or due to using the wrong password. + as_rep, kdc_exchange_dict = as_req_base._test_as_exchange( + cname=cname, + realm=realm, + sname=sname, + till=till, + client_as_etypes=etypes, + expected_error_mode=expected_error_mode, + expected_crealm=realm, + expected_cname=cname, + expected_srealm=realm, + expected_sname=sname, + expected_salt=expected_salt, + etypes=etypes, + padata=padata, + kdc_options=kdc_options, + expected_supported_etypes=krbtgt_supported_etypes, + expected_account_name=user_name, + preauth_key=preauth_key, + ticket_decryption_key=krbtgt_decryption_key, + pac_request=True) + as_req_base.assertIsNotNone(as_rep) + + +def connect_ntlm(pipe, + url, + hostname, + username, + password, + domain, + realm, + workstation, + dn, + is_correct_pw): + user_creds = KerberosCredentials() + user_creds.set_username(username) + user_creds.set_password(password) + user_creds.set_domain(domain) + user_creds.set_workstation(workstation) + user_creds.set_kerberos_state(DONT_USE_KERBEROS) + + # Indicate that we're ready. This ensures we hit the right transaction + # lock. + pipe.send_bytes(b'0') + + # Wait for the main process to take out a transaction lock. + if not pipe.poll(timeout=5): + raise AssertionError('main process failed to indicate readiness') + + try: + # Try connecting to SamDB. This should fail, either due to our account + # being locked out or due to using the wrong password. + SamDB(url=url, + credentials=user_creds, + lp=env_loadparm()) + except ldb.LdbError as err: + num, estr = err.args + if num != ldb.ERR_INVALID_CREDENTIALS: + raise AssertionError(f'connection raised wrong error code ({err})') + else: + raise AssertionError('connection should have raised an LdbError') + + +def connect_samr(pipe, + url, + hostname, + username, + password, + domain, + realm, + workstation, + dn, + is_correct_pw): + # Get the user's NT hash. + user_creds = KerberosCredentials() + user_creds.set_password(password) + nt_hash = user_creds.get_nt_hash() + + # Generate a new UTF-16 password. + new_password = generate_random_password(32, 32) + new_password = new_password.encode('utf-16') + + # Generate the MD4 hash of the password. + md4 = hashlib.new('md4', new_password) + new_password_md4 = md4.digest() + + # Prefix the password with padding so it is 512 bytes long. + new_password_len = len(new_password) + remaining_len = 512 - new_password_len + new_password = bytes(remaining_len) + new_password + + # Append the 32-bit length of the password.. + new_password += int.to_bytes(new_password_len, + length=4, + byteorder='little') + + # Encrypt the password with RC4 and the existing NT hash. + encryptor = Cipher(algorithms.ARC4(nt_hash), + None, + default_backend()).encryptor() + new_password = encryptor.update(new_password) + + # Create a key from the MD4 hash of the new password. + key = new_password_md4[:14] + + # Encrypt the old NT hash with DES to obtain the verifier. + verifier = old_pw_hash(key, nt_hash) + + server = lsa.String() + server.string = hostname + + account = lsa.String() + account.string = username + + nt_password = samr.CryptPassword() + nt_password.data = list(new_password) + + nt_verifier = samr.Password() + nt_verifier.hash = list(verifier) + + conn = samr.samr(f'ncacn_np:{hostname}[krb5,seal,smb2]') + + # Indicate that we're ready. This ensures we hit the right transaction + # lock. + pipe.send_bytes(b'0') + + # Wait for the main process to take out a transaction lock. + if not pipe.poll(timeout=5): + raise AssertionError('main process failed to indicate readiness') + + try: + # Try changing the password. This should fail, either due to our + # account being locked out or due to using the wrong password. + conn.ChangePasswordUser3(server=server, + account=account, + nt_password=nt_password, + nt_verifier=nt_verifier, + lm_change=True, + lm_password=None, + lm_verifier=None, + password3=None) + except NTSTATUSError as err: + num, estr = err.args + + if is_correct_pw: + expected_status = ntstatus.NT_STATUS_ACCOUNT_LOCKED_OUT + else: + expected_status = ntstatus.NT_STATUS_WRONG_PASSWORD + + if num != expected_status: + raise AssertionError(f'pwd change raised wrong error code ' + f'({num:08X}), expected ' + f'{expected_status:08X}') + + else: + raise AssertionError('pwd change should have raised an error') + + +def ldap_pwd_change(pipe, + url, + hostname, + username, + password, + domain, + realm, + workstation, + dn, + is_correct_pw): + lp = env_loadparm() + + admin_creds = KerberosCredentials() + admin_creds.guess(lp) + admin_creds.set_username(env_get_var_value('ADMIN_USERNAME')) + admin_creds.set_password(env_get_var_value('ADMIN_PASSWORD')) + admin_creds.set_kerberos_state(MUST_USE_KERBEROS) + + samdb = SamDB(url=url, + credentials=admin_creds, + lp=lp) + + old_utf16pw = f'"{password}"'.encode('utf-16-le') + + new_password = generate_random_password(32, 32) + new_utf16pw = f'"{new_password}"'.encode('utf-16-le') + + msg = ldb.Message(ldb.Dn(samdb, dn)) + msg['0'] = ldb.MessageElement(old_utf16pw, + ldb.FLAG_MOD_DELETE, + 'unicodePwd') + msg['1'] = ldb.MessageElement(new_utf16pw, + ldb.FLAG_MOD_ADD, + 'unicodePwd') + + # Indicate that we're ready. This ensures we hit the right transaction + # lock. + pipe.send_bytes(b'0') + + # Wait for the main process to take out a transaction lock. + if not pipe.poll(timeout=5): + raise AssertionError('main process failed to indicate readiness') + + # Try changing the user's password. This should fail, either due to the + # user's account being locked out or due to specifying the wrong password. + try: + samdb.modify(msg) + except ldb.LdbError as err: + num, estr = err.args + if num != ldb.ERR_CONSTRAINT_VIOLATION: + raise AssertionError(f'pwd change raised wrong error code ({err})') + + if is_correct_pw: + expected_error = werror.WERR_ACCOUNT_LOCKED_OUT + else: + expected_error = werror.WERR_INVALID_PASSWORD + + expected_code = f'{expected_error:08X}' + if expected_code not in estr: + raise AssertionError(f'pwd change raised wrong error code ' + f'({estr}), expected {expected_code}') + else: + raise AssertionError('pwd change should have raised an LdbError') + + +class LockoutTests(KDCBaseTest): + + def setUp(self): + super().setUp() + self.do_asn1_print = global_asn1_print + self.do_hexdump = global_hexdump + + # Set the lockout threshold. + + samdb = self.get_samdb() + base_dn = ldb.Dn(samdb, samdb.domain_dn()) + + def modify_lockout_threshold(threshold): + if threshold is None: + threshold = [] + flag = ldb.FLAG_MOD_DELETE + else: + threshold = str(threshold) + flag = ldb.FLAG_MOD_REPLACE + + msg = ldb.Message(base_dn) + msg['lockoutThreshold'] = ldb.MessageElement( + threshold, flag, + 'lockoutThreshold') + samdb.modify(msg) + + res = samdb.search(base_dn, + scope=ldb.SCOPE_BASE, + attrs=['lockoutThreshold']) + self.assertEqual(1, len(res)) + + # Reset the lockout threshold as it was before. + lockout_threshold = res[0].get('lockoutThreshold', idx=0) + self.addCleanup(modify_lockout_threshold, lockout_threshold) + + # Set the new lockout threshold. + modify_lockout_threshold(3) + + # Get the old 'dSHeuristics' if it was set. + dsheuristics = samdb.get_dsheuristics() + + # Reset the 'dSHeuristics' as they were before. + self.addCleanup(samdb.set_dsheuristics, dsheuristics) + + # Set the 'dSHeuristics' to activate the correct 'userPassword' + # behaviour. + samdb.set_dsheuristics('000000001') + + # Get the old 'minPwdAge'. + minPwdAge = samdb.get_minPwdAge() + + # Reset the 'minPwdAge' as it was before. + self.addCleanup(samdb.set_minPwdAge, minPwdAge) + + # Set it temporarily to '0'. + samdb.set_minPwdAge('0') + + def assertLocalSamDB(self, samdb): + if samdb.url.startswith('tdb://'): + return + if samdb.url.startswith('mdb://'): + return + + self.fail(f'connection to {samdb.url} is not local!') + + def test_lockout_transaction_kdc(self): + self.do_lockout_transaction(connect_kdc) + + def test_lockout_transaction_ntlm(self): + self.do_lockout_transaction(connect_ntlm) + + def test_lockout_transaction_samr(self): + self.do_lockout_transaction(connect_samr) + + def test_lockout_transaction_ldap_pw_change(self): + self.do_lockout_transaction(ldap_pwd_change) + + def do_lockout_transaction(self, connect_fn): + # Create the user account for testing. + user_creds = self.get_cached_creds(account_type=self.AccountType.USER, + use_cache=False) + user_dn = user_creds.get_dn() + + admin_creds = self.get_admin_creds() + lp = self.get_lp() + + # Get a connection to our local SamDB. + samdb = connect_samdb(samdb_url=lp.samdb_url(), lp=lp, + credentials=admin_creds) + self.assertLocalSamDB(samdb) + + # Prepare to connect to the server with a valid password. + our_pipe, their_pipe = Pipe(duplex=True) + with futures.ProcessPoolExecutor(max_workers=1) as executor: + connect_future = executor.submit( + connect_fn, + pipe=their_pipe, + url=f'ldap://{samdb.host_dns_name()}', + hostname=samdb.host_dns_name(), + username=user_creds.get_username(), + password=user_creds.get_password(), + domain=user_creds.get_domain(), + realm=user_creds.get_realm(), + workstation=user_creds.get_workstation(), + dn=str(user_dn), + is_correct_pw=True) + + # Wait until the test process indicates it's ready. + self.assertTrue(our_pipe.poll(timeout=5), + 'test failed to indicate readiness') + + # Take out a transaction. + samdb.transaction_start() + try: + # Inform the test process that it may proceed. + our_pipe.send_bytes(b'0') + + # Wait one second to ensure the test process hits the + # transaction lock. + time.sleep(1) + + # Lock out the account. + msg = ldb.Message(user_dn) + now = int(time.time()) + in_ten_hours = now + 10 * 60 * 60 + lockout_time = unix2nttime(in_ten_hours) + msg['lockoutTime'] = ldb.MessageElement(str(lockout_time), + ldb.FLAG_MOD_REPLACE, + 'lockoutTime') + samdb.modify(msg) + + # Ensure the account is locked out. + + res = samdb.search( + user_dn, scope=ldb.SCOPE_BASE, + attrs=['msDS-User-Account-Control-Computed']) + self.assertEqual(1, len(res)) + + uac = int(res[0].get('msDS-User-Account-Control-Computed', + idx=0)) + self.assertTrue(uac & dsdb.UF_LOCKOUT) + except Exception: + samdb.transaction_cancel() + raise + + # Commit the local transaction. + samdb.transaction_commit() + + connect_exception = connect_future.exception(timeout=5) + if connect_exception is not None: + raise connect_exception + + def test_bad_pwd_count_transaction_kdc(self): + self.do_bad_pwd_count_transaction(connect_kdc) + + def test_bad_pwd_count_transaction_ntlm(self): + self.do_bad_pwd_count_transaction(connect_ntlm) + + def test_bad_pwd_count_transaction_samr(self): + self.do_bad_pwd_count_transaction(connect_samr) + + def test_bad_pwd_count_transaction_ldap_pw_change(self): + self.do_bad_pwd_count_transaction(ldap_pwd_change) + + def do_bad_pwd_count_transaction(self, connect_fn): + # Create the user account for testing. + user_creds = self.get_cached_creds(account_type=self.AccountType.USER, + use_cache=False) + user_dn = user_creds.get_dn() + + admin_creds = self.get_admin_creds() + lp = self.get_lp() + + # Get a connection to our local SamDB. + samdb = connect_samdb(samdb_url=lp.samdb_url(), lp=lp, + credentials=admin_creds) + self.assertLocalSamDB(samdb) + + # Prepare to connect to the server with an invalid password. + our_pipe, their_pipe = Pipe(duplex=True) + with futures.ProcessPoolExecutor(max_workers=1) as executor: + connect_future = executor.submit( + connect_fn, + pipe=their_pipe, + url=f'ldap://{samdb.host_dns_name()}', + hostname=samdb.host_dns_name(), + username=user_creds.get_username(), + password=user_creds.get_password()[:-1], # invalid password + domain=user_creds.get_domain(), + realm=user_creds.get_realm(), + workstation=user_creds.get_workstation(), + dn=str(user_dn), + is_correct_pw=False) + + # Wait until the test process indicates it's ready. + self.assertTrue(our_pipe.poll(timeout=5), + 'test failed to indicate readiness') + + # Take out a transaction. + samdb.transaction_start() + try: + # Inform the test process that it may proceed. + our_pipe.send_bytes(b'0') + + # Wait one second to ensure the test process hits the + # transaction lock. + time.sleep(1) + + # Set badPwdCount to 1. + msg = ldb.Message(user_dn) + now = int(time.time()) + bad_pwd_time = unix2nttime(now) + msg['badPwdCount'] = ldb.MessageElement( + '1', + ldb.FLAG_MOD_REPLACE, + 'badPwdCount') + msg['badPasswordTime'] = ldb.MessageElement( + str(bad_pwd_time), + ldb.FLAG_MOD_REPLACE, + 'badPasswordTime') + samdb.modify(msg) + + # Ensure the account is not yet locked out. + + res = samdb.search( + user_dn, scope=ldb.SCOPE_BASE, + attrs=['msDS-User-Account-Control-Computed']) + self.assertEqual(1, len(res)) + + uac = int(res[0].get('msDS-User-Account-Control-Computed', + idx=0)) + self.assertFalse(uac & dsdb.UF_LOCKOUT) + except Exception: + samdb.transaction_cancel() + raise + + # Commit the local transaction. + samdb.transaction_commit() + + connect_exception = connect_future.exception(timeout=5) + if connect_exception is not None: + raise connect_exception + + # Check that badPwdCount has now increased to 2. + + res = samdb.search(user_dn, + scope=ldb.SCOPE_BASE, + attrs=['badPwdCount']) + self.assertEqual(1, len(res)) + + bad_pwd_count = int(res[0].get('badPwdCount', idx=0)) + self.assertEqual(2, bad_pwd_count) + + +if __name__ == '__main__': + global_asn1_print = False + global_hexdump = False + import unittest + unittest.main() diff --git a/python/samba/tests/krb5/raw_testcase.py b/python/samba/tests/krb5/raw_testcase.py index 7f9d9d17640..e35ee3e597d 100644 --- a/python/samba/tests/krb5/raw_testcase.py +++ b/python/samba/tests/krb5/raw_testcase.py @@ -46,6 +46,7 @@ from samba.tests.krb5.rfc4120_constants import ( AD_IF_RELEVANT, AD_WIN2K_PAC, FX_FAST_ARMOR_AP_REQUEST, + KDC_ERR_CLIENT_REVOKED, KDC_ERR_GENERIC, KDC_ERR_POLICY, KDC_ERR_PREAUTH_FAILED, @@ -3138,7 +3139,7 @@ class RawKerberosTest(TestCaseInTempDir): expected_patypes += (PADATA_ETYPE_INFO2,) if error_code not in (KDC_ERR_PREAUTH_FAILED, KDC_ERR_SKEW, - KDC_ERR_POLICY): + KDC_ERR_POLICY, KDC_ERR_CLIENT_REVOKED): if sent_fast: expected_patypes += (PADATA_ENCRYPTED_CHALLENGE,) else: diff --git a/python/samba/tests/krb5/rfc4120_constants.py b/python/samba/tests/krb5/rfc4120_constants.py index 28d83407ac5..a68fc64a6a3 100644 --- a/python/samba/tests/krb5/rfc4120_constants.py +++ b/python/samba/tests/krb5/rfc4120_constants.py @@ -86,6 +86,7 @@ KDC_ERR_POLICY = 12 KDC_ERR_BADOPTION = 13 KDC_ERR_ETYPE_NOSUPP = 14 KDC_ERR_SUMTYPE_NOSUPP = 15 +KDC_ERR_CLIENT_REVOKED = 18 KDC_ERR_TGT_REVOKED = 20 KDC_ERR_PREAUTH_FAILED = 24 KDC_ERR_PREAUTH_REQUIRED = 25 diff --git a/python/samba/tests/usage.py b/python/samba/tests/usage.py index 9297247152a..67b4e9f7b96 100644 --- a/python/samba/tests/usage.py +++ b/python/samba/tests/usage.py @@ -112,6 +112,7 @@ EXCLUDE_USAGE = { 'python/samba/tests/krb5/pac_align_tests.py', 'python/samba/tests/krb5/protected_users_tests.py', 'python/samba/tests/krb5/nt_hash_tests.py', + 'python/samba/tests/krb5/lockout_tests.py', } EXCLUDE_HELP = { diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc index 4ae27eacb09..c22c72d0619 100644 --- a/selftest/knownfail_heimdal_kdc +++ b/selftest/knownfail_heimdal_kdc @@ -54,3 +54,12 @@ ^samba.tests.krb5.protected_users_tests.samba.tests.krb5.protected_users_tests.ProtectedUsersTests.test_proxiable_as_protected.ad_dc # ^samba.tests.krb5.protected_users_tests.samba.tests.krb5.protected_users_tests.ProtectedUsersTests.test_samr_change_password_protected.ad_dc +# +# Lockout tests +# +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_kdc.ad_dc:local +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_ntlm.ad_dc:local +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_samr.ad_dc:local +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_transaction_kdc.ad_dc:local +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_transaction_ntlm.ad_dc:local +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_transaction_samr.ad_dc:local diff --git a/selftest/knownfail_mit_kdc b/selftest/knownfail_mit_kdc index 9b55627bbc8..17b06de14ff 100644 --- a/selftest/knownfail_mit_kdc +++ b/selftest/knownfail_mit_kdc @@ -433,3 +433,12 @@ samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_ ^samba.tests.krb5.protected_users_tests.samba.tests.krb5.protected_users_tests.ProtectedUsersTests.test_ts_rc4_not_protected.ad_dc ^samba.tests.krb5.protected_users_tests.samba.tests.krb5.protected_users_tests.ProtectedUsersTests.test_ts_rc4_protected.ad_dc ^samba.tests.krb5.protected_users_tests.samba.tests.krb5.protected_users_tests.ProtectedUsersTests.test_ts_rc4_protected_nested.ad_dc +# +# Lockout tests +# +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_kdc.ad_dc:local +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_ntlm.ad_dc:local +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_samr.ad_dc:local +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_transaction_kdc.ad_dc:local +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_transaction_ntlm.ad_dc:local +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_transaction_samr.ad_dc:local diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index 070879e6e44..d815bba4a13 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -1703,6 +1703,10 @@ for env, nt_hash in [("ad_dc:local", True), **krb5_environ, 'EXPECT_NT_HASH': int(nt_hash), }) +planoldpythontestsuite( + 'ad_dc:local', + 'samba.tests.krb5.lockout_tests', + environ=krb5_environ) for env in [ 'vampire_dc', -- 2.35.0 From 6e19368de193a144355718dea329397f4c15f83d Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 30 Mar 2021 10:51:26 +1300 Subject: [PATCH 04/21] CVE-2021-20251 s4-rpc_server: Use authsam_search_account() to find the user This helps the bad password and audit log handling code as it allows assumptions to be made about the attributes found in the variable "msg", such as that DSDB_SEARCH_SHOW_EXTENDED_DN was used. This ensures we can re-search on the DN via the embedded GUID, which in in turn rename-proof. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Andrew Bartlett Reviewed-by: Joseph Sutton --- source4/rpc_server/samr/samr_password.c | 40 +++++++++++-------------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/source4/rpc_server/samr/samr_password.c b/source4/rpc_server/samr/samr_password.c index 0ac5a5a17e1..70f969ffdf0 100644 --- a/source4/rpc_server/samr/samr_password.c +++ b/source4/rpc_server/samr/samr_password.c @@ -124,13 +124,7 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, struct ldb_context *sam_ctx = NULL; struct ldb_dn *user_dn = NULL; int ret; - struct ldb_result *res = NULL; - const char * const attrs[] = { "unicodePwd", "dBCSPwd", - "userAccountControl", - "msDS-ResultantPSO", - "msDS-User-Account-Control-Computed", - "badPwdCount", "badPasswordTime", - "objectSid", NULL }; + struct ldb_message *msg = NULL; struct samr_Password *nt_pwd; struct samr_DomInfo1 *dominfo = NULL; struct userPwdChangeFailureInformation *reject = NULL; @@ -168,26 +162,26 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, return NT_STATUS_INVALID_SYSTEM_SERVICE; } - /* we need the users dn and the domain dn (derived from the - user SID). We also need the current lm and nt password hashes - in order to decrypt the incoming passwords */ - ret = dsdb_search(sam_ctx, mem_ctx, &res, - ldb_get_default_basedn(sam_ctx), - LDB_SCOPE_SUBTREE, attrs, - DSDB_SEARCH_SHOW_EXTENDED_DN, - "(&(sAMAccountName=%s)(objectclass=user))", - ldb_binary_encode_string(mem_ctx, r->in.account->string)); - if (ret != LDB_SUCCESS || res->count != 1) { - status = NT_STATUS_NO_SUCH_USER; /* Converted to WRONG_PASSWORD below */ + /* + * We use authsam_search_account() to be consistent with the + * other callers in the bad password and audit log handling + * systems. It ensures we get DSDB_SEARCH_SHOW_EXTENDED_DN. + */ + status = authsam_search_account(mem_ctx, + sam_ctx, + r->in.account->string, + ldb_get_default_basedn(sam_ctx), + &msg); + if (!NT_STATUS_IS_OK(status)) { goto failed; } - user_dn = res->msgs[0]->dn; - user_samAccountName = ldb_msg_find_attr_as_string(res->msgs[0], "samAccountName", NULL); - user_objectSid = samdb_result_dom_sid(res, res->msgs[0], "objectSid"); + user_dn = msg->dn; + user_samAccountName = ldb_msg_find_attr_as_string(msg, "samAccountName", NULL); + user_objectSid = samdb_result_dom_sid(mem_ctx, msg, "objectSid"); status = samdb_result_passwords(mem_ctx, lp_ctx, - res->msgs[0], &nt_pwd); + msg, &nt_pwd); if (!NT_STATUS_IS_OK(status) ) { goto failed; } @@ -303,7 +297,7 @@ failed: /* Only update the badPwdCount if we found the user */ if (NT_STATUS_EQUAL(status, NT_STATUS_WRONG_PASSWORD)) { - authsam_update_bad_pwd_count(sam_ctx, res->msgs[0], ldb_get_default_basedn(sam_ctx)); + authsam_update_bad_pwd_count(sam_ctx, msg, ldb_get_default_basedn(sam_ctx)); } else if (NT_STATUS_EQUAL(status, NT_STATUS_NO_SUCH_USER)) { /* Don't give the game away: (don't allow anonymous users to prove the existence of usernames) */ status = NT_STATUS_WRONG_PASSWORD; -- 2.35.0 From 18edc5141514146c048cc2eed954d8f9d44f7d8d Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Tue, 16 Mar 2021 10:52:58 +1300 Subject: [PATCH 05/21] CVE-2021-20251 auth4: split samdb_result_msds_LockoutObservationWindow() out samdb_result_msds_LockoutObservationWindow() is split out of samdb_result_effective_badPwdCount() BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Gary Lockyer Reviewed-by: Joseph Sutton --- source4/dsdb/common/util.c | 45 +++++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c index 54997c2ad75..19667ee8611 100644 --- a/source4/dsdb/common/util.c +++ b/source4/dsdb/common/util.c @@ -5291,9 +5291,9 @@ int dsdb_create_partial_replica_NC(struct ldb_context *ldb, struct ldb_dn *dn) * This also requires that the domain_msg have (if present): * - lockOutObservationWindow */ -static int dsdb_effective_badPwdCount(const struct ldb_message *user_msg, - int64_t lockOutObservationWindow, - NTTIME now) +int dsdb_effective_badPwdCount(const struct ldb_message *user_msg, + int64_t lockOutObservationWindow, + NTTIME now) { int64_t badPasswordTime; badPasswordTime = ldb_msg_find_attr_as_int64(user_msg, "badPasswordTime", 0); @@ -5340,25 +5340,24 @@ static struct ldb_result *lookup_user_pso(struct ldb_context *sam_ldb, } /* - * Return the effective badPwdCount + * Return the msDS-LockoutObservationWindow for a user message * * This requires that the user_msg have (if present): - * - badPasswordTime - * - badPwdCount * - msDS-ResultantPSO */ -int samdb_result_effective_badPwdCount(struct ldb_context *sam_ldb, - TALLOC_CTX *mem_ctx, - struct ldb_dn *domain_dn, - const struct ldb_message *user_msg) +int64_t samdb_result_msds_LockoutObservationWindow( + struct ldb_context *sam_ldb, + TALLOC_CTX *mem_ctx, + struct ldb_dn *domain_dn, + const struct ldb_message *user_msg) { - struct timeval tv_now = timeval_current(); - NTTIME now = timeval_to_nttime(&tv_now); int64_t lockOutObservationWindow; struct ldb_result *res = NULL; const char *attrs[] = { "msDS-LockoutObservationWindow", NULL }; - + if (domain_dn == NULL) { + smb_panic("domain dn is NULL"); + } res = lookup_user_pso(sam_ldb, mem_ctx, user_msg, attrs); if (res != NULL) { @@ -5374,7 +5373,27 @@ int samdb_result_effective_badPwdCount(struct ldb_context *sam_ldb, samdb_search_int64(sam_ldb, mem_ctx, 0, domain_dn, "lockOutObservationWindow", NULL); } + return lockOutObservationWindow; +} +/* + * Return the effective badPwdCount + * + * This requires that the user_msg have (if present): + * - badPasswordTime + * - badPwdCount + * - msDS-ResultantPSO + */ +int samdb_result_effective_badPwdCount(struct ldb_context *sam_ldb, + TALLOC_CTX *mem_ctx, + struct ldb_dn *domain_dn, + const struct ldb_message *user_msg) +{ + struct timeval tv_now = timeval_current(); + NTTIME now = timeval_to_nttime(&tv_now); + int64_t lockOutObservationWindow = + samdb_result_msds_LockoutObservationWindow( + sam_ldb, mem_ctx, domain_dn, user_msg); return dsdb_effective_badPwdCount(user_msg, lockOutObservationWindow, now); } -- 2.35.0 From 0e45f85d5e52963976b5421a0d807c302ba60f45 Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Wed, 27 Jan 2021 14:24:58 +1300 Subject: [PATCH 06/21] CVE-2021-20251 s4 auth: Prepare to make bad password count increment atomic To ensure that the bad password count is incremented atomically, and that the successful logon accounting data is updated atomically, without always opening a transaction, we will need to make a note of all bad and successful passwords in a side-DB outside the transaction lock. This provides the functions needed for that and hooks them in (future commits will handle errors and use the results). Based on patches by Gary Lockyer BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Andrew Bartlett Reviewed-by: Joseph Sutton --- source4/auth/sam.c | 187 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 187 insertions(+) diff --git a/source4/auth/sam.c b/source4/auth/sam.c index 4bbf9aadc81..37f60f7b80f 100644 --- a/source4/auth/sam.c +++ b/source4/auth/sam.c @@ -31,6 +31,8 @@ #include "libcli/ldap/ldap_ndr.h" #include "param/param.h" #include "librpc/gen_ndr/ndr_winbind_c.h" +#include "lib/dbwrap/dbwrap.h" +#include "cluster/cluster.h" #undef DBGC_CLASS #define DBGC_CLASS DBGC_AUTH @@ -825,6 +827,177 @@ static int authsam_get_user_pso(struct ldb_context *sam_ctx, return LDB_SUCCESS; } +static struct db_context *authsam_get_bad_password_db( + TALLOC_CTX *mem_ctx, + struct ldb_context *sam_ctx) +{ + struct loadparm_context *lp_ctx = NULL; + const char *db_name = "bad_password"; + struct db_context *db_ctx = NULL; + + lp_ctx = ldb_get_opaque(sam_ctx, "loadparm"); + if (lp_ctx == NULL) { + DBG_ERR("Unable to get loadparm_context\n"); + return NULL; + } + + db_ctx = cluster_db_tmp_open(mem_ctx, lp_ctx, db_name, TDB_DEFAULT); + if (db_ctx == NULL) { + DBG_ERR("Unable to open bad password attempts database\n"); + return NULL; + } + return db_ctx; +} + +static NTSTATUS get_object_sid_as_tdb_data( + TALLOC_CTX *mem_ctx, + const struct ldb_message *msg, + struct dom_sid_buf *buf, + TDB_DATA *key) +{ + struct dom_sid *objectsid = NULL; + + /* + * Convert the objectSID to a human readable form to + * make debugging easier + */ + objectsid = samdb_result_dom_sid(mem_ctx, msg, "objectSID"); + if (objectsid == NULL) { + DBG_ERR("Unable to extract objectSID\n"); + return NT_STATUS_INTERNAL_ERROR; + } + dom_sid_str_buf(objectsid, buf); + key->dptr = (unsigned char *)buf->buf; + key->dsize = strlen(buf->buf); + + talloc_free(objectsid); + + return NT_STATUS_OK; +} + +/* + * Add the users objectSID to the bad password attempt database + * to indicate that last authentication failed due to a bad password + */ +static NTSTATUS authsam_set_bad_password_indicator( + struct ldb_context *sam_ctx, + TALLOC_CTX *mem_ctx, + const struct ldb_message *msg) +{ + NTSTATUS status = NT_STATUS_OK; + struct dom_sid_buf buf; + TDB_DATA key = {0}; + TDB_DATA value = {0}; + struct db_context *db = NULL; + + TALLOC_CTX *ctx = talloc_new(mem_ctx); + if (ctx == NULL) { + return NT_STATUS_NO_MEMORY; + } + + db = authsam_get_bad_password_db(ctx, sam_ctx); + if (db == NULL) { + status = NT_STATUS_INTERNAL_ERROR; + goto exit; + } + + status = get_object_sid_as_tdb_data(ctx, msg, &buf, &key); + if (!NT_STATUS_IS_OK(status)) { + goto exit; + } + + status = dbwrap_store(db, key, value, 0); + if (!NT_STATUS_IS_OK(status)) { + DBG_ERR("Unable to store bad password indicator\n"); + } +exit: + talloc_free(ctx); + return status; +} + +/* + * see if the users objectSID is in the bad password attempt database + */ +static NTSTATUS authsam_check_bad_password_indicator( + struct ldb_context *sam_ctx, + TALLOC_CTX *mem_ctx, + bool *exists, + const struct ldb_message *msg) +{ + NTSTATUS status = NT_STATUS_OK; + struct dom_sid_buf buf; + TDB_DATA key = {0}; + struct db_context *db = NULL; + + TALLOC_CTX *ctx = talloc_new(mem_ctx); + if (ctx == NULL) { + return NT_STATUS_NO_MEMORY; + } + + db = authsam_get_bad_password_db(ctx, sam_ctx); + if (db == NULL) { + status = NT_STATUS_INTERNAL_ERROR; + goto exit; + } + + status = get_object_sid_as_tdb_data(ctx, msg, &buf, &key); + if (!NT_STATUS_IS_OK(status)) { + goto exit; + } + + *exists = dbwrap_exists(db, key); +exit: + talloc_free(ctx); + return status; +} + +/* + * Remove the users objectSID to the bad password attempt database + * to indicate that last authentication succeeded. + */ +static NTSTATUS authsam_clear_bad_password_indicator( + struct ldb_context *sam_ctx, + TALLOC_CTX *mem_ctx, + const struct ldb_message *msg) +{ + NTSTATUS status = NT_STATUS_OK; + struct dom_sid_buf buf; + TDB_DATA key = {0}; + struct db_context *db = NULL; + + TALLOC_CTX *ctx = talloc_new(mem_ctx); + if (ctx == NULL) { + return NT_STATUS_NO_MEMORY; + } + + db = authsam_get_bad_password_db(ctx, sam_ctx); + if (db == NULL) { + status = NT_STATUS_INTERNAL_ERROR; + goto exit; + } + + status = get_object_sid_as_tdb_data(ctx, msg, &buf, &key); + if (!NT_STATUS_IS_OK(status)) { + goto exit; + } + + status = dbwrap_delete(db, key); + if (NT_STATUS_EQUAL(NT_STATUS_NOT_FOUND, status)) { + /* + * Ok there was no bad password indicator this is expected + */ + status = NT_STATUS_OK; + } + if (NT_STATUS_IS_ERR(status)) { + DBG_ERR("Unable to delete bad password indicator, %s %s\n", + nt_errstr(status), + get_friendly_nt_error_msg(status)); + } +exit: + talloc_free(ctx); + return status; +} + NTSTATUS authsam_update_bad_pwd_count(struct ldb_context *sam_ctx, struct ldb_message *msg, struct ldb_dn *domain_dn) @@ -894,6 +1067,10 @@ NTSTATUS authsam_update_bad_pwd_count(struct ldb_context *sam_ctx, ret = dsdb_autotransaction_request(sam_ctx, req); talloc_free(req); + + status = authsam_set_bad_password_indicator( + sam_ctx, mem_ctx, msg); + /* Failure is ignored for now */ } done: @@ -1057,12 +1234,19 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx, NTTIME now; NTTIME lastLogonTimestamp; bool am_rodc = false; + bool need_db_reread; mem_ctx = talloc_new(msg); if (mem_ctx == NULL) { return NT_STATUS_NO_MEMORY; } + status = authsam_check_bad_password_indicator( + sam_ctx, mem_ctx, &need_db_reread, msg); + if (!NT_STATUS_IS_OK(status)) { + return status; + } + lockoutTime = ldb_msg_find_attr_as_int64(msg, "lockoutTime", 0); dbBadPwdCount = ldb_msg_find_attr_as_int(msg, "badPwdCount", 0); if (interactive_or_kerberos) { @@ -1197,6 +1381,9 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx, talloc_free(req); } + status = authsam_clear_bad_password_indicator(sam_ctx, mem_ctx, msg); + /* Failure is ignored for now */ + done: if (ret != LDB_SUCCESS) { DEBUG(0, ("Failed to set badPwdCount and lockoutTime " -- 2.35.0 From ae82b7985c0193dac96cee206f43c0183211d7b9 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 30 Mar 2021 17:57:10 +1300 Subject: [PATCH 07/21] CVE-2021-20251 auth4: Reread the user record if a bad password is noticed. As is, this is pointless, as we need a transaction to make this any less of a race, but this provides the steps towards that goal. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Andrew Bartlett Reviewed-by: Joseph Sutton --- source4/auth/sam.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/source4/auth/sam.c b/source4/auth/sam.c index 37f60f7b80f..85a23075c4e 100644 --- a/source4/auth/sam.c +++ b/source4/auth/sam.c @@ -827,6 +827,68 @@ static int authsam_get_user_pso(struct ldb_context *sam_ctx, return LDB_SUCCESS; } +/* + * Re-read the bad password and successful logon data for a user. + * + * the passed user record, must contain the "objectGUID", as this is + * used as the re-read key + */ +NTSTATUS authsam_reread_user_logon_data( + struct ldb_context *sam_ctx, + TALLOC_CTX *mem_ctx, + const struct ldb_message *user_msg, + struct ldb_message **current) +{ + const struct ldb_val *v = NULL; + struct ldb_result *res = NULL; + uint16_t acct_flags = 0; + const char *attr_name = "msDS-User-Account-Control-Computed"; + + int ret; + + /* + * Re-read the account details, using the GUID in case the DN + * is being changed (this is automatic in LDB because the + * original search also used DSDB_SEARCH_SHOW_EXTENDED_DN) + * + * We re read all the attributes in user_attrs, rather than using a + * subset to ensure that we can reuse existing validation code. + */ + ret = dsdb_search_dn(sam_ctx, + mem_ctx, + &res, + user_msg->dn, + user_attrs, + DSDB_SEARCH_SHOW_EXTENDED_DN); + if (ret != LDB_SUCCESS) { + DBG_ERR("Unable to re-read account control data for %s\n", + ldb_dn_get_linearized(user_msg->dn)); + return NT_STATUS_INTERNAL_ERROR; + } + + /* + * Ensure the account has not been locked out by another request + */ + v = ldb_msg_find_ldb_val(res->msgs[0], attr_name); + if (v == NULL || v->data == NULL) { + DBG_ERR("No %s attribute for %s\n", + attr_name, + ldb_dn_get_linearized(user_msg->dn)); + TALLOC_FREE(res); + return NT_STATUS_INTERNAL_ERROR; + } + acct_flags = samdb_result_acct_flags(res->msgs[0], attr_name); + if (acct_flags & ACB_AUTOLOCK) { + DBG_WARNING( + "Account for user %s was locked out.\n", + ldb_dn_get_linearized(user_msg->dn)); + TALLOC_FREE(res); + return NT_STATUS_ACCOUNT_LOCKED_OUT; + } + *current = res->msgs[0]; + return NT_STATUS_OK; +} + static struct db_context *authsam_get_bad_password_db( TALLOC_CTX *mem_ctx, struct ldb_context *sam_ctx) @@ -1247,6 +1309,26 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx, return status; } + if (need_db_reread) { + struct ldb_message *current = NULL; + + /* + * Re-read the account details, using the GUID + * embedded in DN so this is safe against a race where + * it is being renamed. + */ + status = authsam_reread_user_logon_data( + sam_ctx, mem_ctx, msg, ¤t); + if (!NT_STATUS_IS_OK(status)) { + /* + * The re-read can return account locked out, as well + * as an internal error + */ + return status; + } + msg = current; + } + lockoutTime = ldb_msg_find_attr_as_int64(msg, "lockoutTime", 0); dbBadPwdCount = ldb_msg_find_attr_as_int(msg, "badPwdCount", 0); if (interactive_or_kerberos) { -- 2.35.0 From bfdae7cd1c351282f60895ac0be292d4796d18d3 Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Tue, 9 Feb 2021 11:59:05 +1300 Subject: [PATCH 08/21] CVE-2021-20251 s4 auth test: Unit tests for source4/auth/sam.c cmocka unit tests for the authsam_reread_user_logon_data in source4/auth/sam.c BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Gary Lockyer --- selftest/knownfail.d/auth-sam | 24 + selftest/tests.py | 2 + source4/auth/tests/sam.c | 2758 +++++++++++++++++++++++++++++++++ source4/auth/wscript_build | 11 + 4 files changed, 2795 insertions(+) create mode 100644 selftest/knownfail.d/auth-sam create mode 100644 source4/auth/tests/sam.c diff --git a/selftest/knownfail.d/auth-sam b/selftest/knownfail.d/auth-sam new file mode 100644 index 00000000000..c4002ec0c53 --- /dev/null +++ b/selftest/knownfail.d/auth-sam @@ -0,0 +1,24 @@ +^samba.unittests.auth.sam.test_reread_account_not_locked.none +^samba.unittests.auth.sam.test_success_accounting_add_control_failed.none +^samba.unittests.auth.sam.test_success_accounting_build_mod_req_failed.none +^samba.unittests.auth.sam.test_success_accounting_commit_failed.none +^samba.unittests.auth.sam.test_success_accounting_ldb_msg_new_failed.none +^samba.unittests.auth.sam.test_success_accounting_ldb_request_failed.none +^samba.unittests.auth.sam.test_success_accounting_ldb_wait_failed.none +^samba.unittests.auth.sam.test_success_accounting_reread_failed.none +^samba.unittests.auth.sam.test_success_accounting_rollback_failed.none +^samba.unittests.auth.sam.test_success_accounting_samdb_rodc_failed.none +^samba.unittests.auth.sam.test_success_accounting_start_txn_failed.none +^samba.unittests.auth.sam.test_success_accounting_update_lastlogon_failed.none +^samba.unittests.auth.sam.test_update_bad_add_control_failed.none +^samba.unittests.auth.sam.test_update_bad_build_mod_request_failed.none +^samba.unittests.auth.sam.test_update_bad_commit_failed.none +^samba.unittests.auth.sam.test_update_bad_get_pso_failed.none +^samba.unittests.auth.sam.test_update_bad_ldb_request_failed.none +^samba.unittests.auth.sam.test_update_bad_ldb_wait_failed.none +^samba.unittests.auth.sam.test_update_bad_no_update_required.none +^samba.unittests.auth.sam.test_update_bad_reread_failed.none +^samba.unittests.auth.sam.test_update_bad_reread_locked_out.none +^samba.unittests.auth.sam.test_update_bad_start_txn_failed.none +^samba.unittests.auth.sam.test_update_bad_txn_cancel_failed.none +^samba.unittests.auth.sam.test_update_bad_update_count_failed.none diff --git a/selftest/tests.py b/selftest/tests.py index 82d210d7a46..880f459e973 100644 --- a/selftest/tests.py +++ b/selftest/tests.py @@ -446,6 +446,8 @@ plantestsuite("samba.unittests.test_registry_regfio", "none", [os.path.join(bindir(), "default/source3/test_registry_regfio")]) plantestsuite("samba.unittests.test_oLschema2ldif", "none", [os.path.join(bindir(), "default/source4/utils/oLschema2ldif/test_oLschema2ldif")]) +plantestsuite("samba.unittests.auth.sam", "none", + [os.path.join(bindir(), "test_auth_sam")]) if with_elasticsearch_backend: plantestsuite("samba.unittests.mdsparser_es", "none", [os.path.join(bindir(), "default/source3/test_mdsparser_es")] + [configuration]) diff --git a/source4/auth/tests/sam.c b/source4/auth/tests/sam.c new file mode 100644 index 00000000000..dec8157ac04 --- /dev/null +++ b/source4/auth/tests/sam.c @@ -0,0 +1,2758 @@ +/* + * Unit tests for source4/auth/sam.c + * + * Copyright (C) Catalyst.NET Ltd 2021 + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +/* + * from cmocka.c: + * These headers or their equivalents should be included prior to + * including + * this header file. + * + * #include + * #include + * #include + * + * This allows test applications to use custom definitions of C standard + * library functions and types. + * + */ + +#include +#include +#include +#include +#include +#include +#include + +#include "includes.h" +#include "auth/sam.c" +#include "ldb.h" +#include "ntstatus.h" +#include "librpc/gen_ndr/ndr_security.h" + +/***************************************************************************** + * wrapped functions + * + *****************************************************************************/ +int __wrap_samdb_msg_add_int64( + struct ldb_context *sam_ldb, + TALLOC_CTX *mem_ctx, + struct ldb_message *msg, + const char *attr_name, + int64_t v); +int __real_samdb_msg_add_int64( + struct ldb_context *sam_ldb, + TALLOC_CTX *mem_ctx, + struct ldb_message *msg, + const char *attr_name, + int64_t v); +int __wrap_samdb_msg_add_int64( + struct ldb_context *sam_ldb, + TALLOC_CTX *mem_ctx, + struct ldb_message *msg, + const char *attr_name, + int64_t v) +{ + + int ret; + ret = (int)mock(); + if (ret != LDB_SUCCESS) { + return ret; + } + return __real_samdb_msg_add_int64(sam_ldb, mem_ctx, msg, attr_name, v); +} +/***************************************************************************** + * Mock implementations + *****************************************************************************/ + +static int check_dn(const LargestIntegralType left_value, + const LargestIntegralType right_value) +{ + /* + *We have to cast away const so we can get the linearized form with + * ldb_dn_get_extended_linearized(). + */ + struct ldb_dn *left_dn = (void *)left_value; + struct ldb_dn *right_dn = (void *)right_value; + char *left_dn_string = NULL; + char *right_dn_string = NULL; + bool ok; + + if (left_dn == NULL && right_dn == NULL) { + return true; + } + + if (left_dn != NULL) { + left_dn_string = ldb_dn_get_extended_linearized(NULL, left_dn, 1); + assert_non_null(left_dn_string); + } + + if (right_dn != NULL) { + right_dn_string = ldb_dn_get_extended_linearized(NULL, right_dn, 1); + assert_non_null(right_dn_string); + } + + if (left_dn_string == NULL || right_dn_string == NULL) { + ok = false; + print_error("\"%s\" != \"%s\"\n", + left_dn_string != NULL ? left_dn_string : "", + right_dn_string != NULL ? right_dn_string : ""); + } else { + ok = (strcmp(left_dn_string, right_dn_string) == 0); + if (!ok) { + print_error("\"%s\" != \"%s\"\n", + left_dn_string, + right_dn_string); + } + + } + + TALLOC_FREE(right_dn_string); + TALLOC_FREE(left_dn_string); + + return ok; +} + +int __wrap_dsdb_search_dn(struct ldb_context *ldb, + TALLOC_CTX *mem_ctx, + struct ldb_result **_result, + struct ldb_dn *basedn, + const char * const *attrs, + uint32_t dsdb_flags); +int __wrap_dsdb_search_dn(struct ldb_context *ldb, + TALLOC_CTX *mem_ctx, + struct ldb_result **_result, + struct ldb_dn *basedn, + const char * const *attrs, + uint32_t dsdb_flags) +{ + check_expected(basedn); + + *_result = talloc_steal(mem_ctx, mock_ptr_type(struct ldb_result *)); + + return mock(); +} + +int ldb_transaction_start_ret = LDB_SUCCESS; +bool in_transaction = false; +int ldb_transaction_start(struct ldb_context *ldb) { + assert_false(in_transaction); + if (ldb_transaction_start_ret == LDB_SUCCESS) { + in_transaction = true; + } + return ldb_transaction_start_ret; +} + +int ldb_transaction_cancel_ret = LDB_SUCCESS; +bool transaction_cancelled = false; +int ldb_transaction_cancel(struct ldb_context *ldb) { + assert_true(in_transaction); + if (ldb_transaction_cancel_ret == LDB_SUCCESS) { + in_transaction = false; + transaction_cancelled = true; + } + return ldb_transaction_cancel_ret; +} + +int ldb_transaction_commit_ret = LDB_SUCCESS; +bool transaction_committed = false; +int ldb_transaction_commit(struct ldb_context *ldb) { + assert_true(in_transaction); + if (ldb_transaction_commit_ret == LDB_SUCCESS) { + in_transaction = false; + transaction_committed = true; + } + return ldb_transaction_commit_ret; +} + +NTSTATUS dsdb_update_bad_pwd_count_ret = NT_STATUS_OK; +struct ldb_message *dsdb_update_bad_pwd_count_res = NULL; +NTSTATUS dsdb_update_bad_pwd_count(TALLOC_CTX *mem_ctx, + struct ldb_context *sam_ctx, + struct ldb_message *user_msg, + struct ldb_message *domain_msg, + struct ldb_message *pso_msg, + struct ldb_message **_mod_msg) { + + *_mod_msg = talloc_move(mem_ctx, &dsdb_update_bad_pwd_count_res); + return dsdb_update_bad_pwd_count_ret; +} + +int ldb_build_mod_req_ret = LDB_SUCCESS; +struct ldb_request *ldb_build_mod_req_res = NULL; +int ldb_build_mod_req(struct ldb_request **ret_req, + struct ldb_context *ldb, + TALLOC_CTX *mem_ctx, + const struct ldb_message *message, + struct ldb_control **controls, + void *context, + ldb_request_callback_t callback, + struct ldb_request *parent) +{ + *ret_req = talloc_move(mem_ctx, &ldb_build_mod_req_res); + return ldb_build_mod_req_ret; +} + +int ldb_request_add_control_ret = LDB_SUCCESS; +int ldb_request_add_control(struct ldb_request *req, + const char *oid, + bool critical, + void *data) +{ + return ldb_request_add_control_ret; +} + +int ldb_request_ret = LDB_SUCCESS; +int ldb_request(struct ldb_context *ldb, + struct ldb_request *req) +{ + return ldb_request_ret; +} + +int ldb_wait_ret = LDB_SUCCESS; +int ldb_wait(struct ldb_handle *handle, + enum ldb_wait_type type) +{ + return ldb_wait_ret; +} +bool ldb_msg_new_fail = false; +struct ldb_message *ldb_msg_new(TALLOC_CTX *mem_ctx) +{ + if (ldb_msg_new_fail) { + return NULL; + } else { + return talloc_zero(mem_ctx, struct ldb_message); + } +} + +int samdb_rodc_ret = LDB_SUCCESS; +bool samdb_rodc_res = false; + +int samdb_rodc( + struct ldb_context *sam_ctx, + bool *am_rodc) +{ + + *am_rodc = samdb_rodc_res; + return samdb_rodc_ret; +} + +struct loadparm_context *ldb_get_opaque_ret = NULL; +void *ldb_get_opaque(struct ldb_context *ldb, const char *name) +{ + return ldb_get_opaque_ret; +} + +struct db_context {}; +struct db_context *cluster_db_tmp_open_ret = NULL; +struct db_context *cluster_db_tmp_open( + TALLOC_CTX *mem_ctx, + struct loadparm_context *lp_ctx, + const char *dbbase, + int flags) +{ + return cluster_db_tmp_open_ret; +} + +NTSTATUS dbwrap_store_ret = NT_STATUS_OK; +NTSTATUS dbwrap_store(struct db_context *db, TDB_DATA key, + TDB_DATA data, int flags) +{ + return dbwrap_store_ret; +} +bool dbwrap_exists_ret = true; + +bool dbwrap_exists(struct db_context *db, TDB_DATA key) +{ + return dbwrap_exists_ret; +} + +NTSTATUS dbwrap_delete_ret = NT_STATUS_OK; +NTSTATUS dbwrap_delete(struct db_context *db, TDB_DATA key) +{ + return dbwrap_delete_ret; +} + +/* + * Set the globals used by the mocked functions to a known and consistent state + * + */ +static void init_mock_results(TALLOC_CTX *mem_ctx) +{ + ldb_transaction_start_ret = LDB_SUCCESS; + in_transaction = false; + + ldb_transaction_cancel_ret = LDB_SUCCESS; + transaction_cancelled = false; + + ldb_transaction_commit_ret = LDB_SUCCESS; + transaction_committed = false; + + dsdb_update_bad_pwd_count_ret = NT_STATUS_OK; + dsdb_update_bad_pwd_count_res = NULL; + + ldb_build_mod_req_ret = LDB_SUCCESS; + ldb_build_mod_req_res = NULL; + + ldb_request_add_control_ret = LDB_SUCCESS; + ldb_request_ret = LDB_SUCCESS; + ldb_wait_ret = LDB_SUCCESS; + + ldb_msg_new_fail = false; + + samdb_rodc_ret = LDB_SUCCESS; + samdb_rodc_res = false; + + ldb_get_opaque_ret = loadparm_init(mem_ctx); + + cluster_db_tmp_open_ret = talloc_zero(mem_ctx, struct db_context); + + dbwrap_store_ret = NT_STATUS_OK; + + dbwrap_exists_ret = true; + + dbwrap_delete_ret = NT_STATUS_OK; + +} + +/***************************************************************************** + * Unit test set up and tear down + *****************************************************************************/ +struct context { +}; + +static int setup(void **state) { + struct context *ctx = talloc_zero(NULL, struct context); + init_mock_results(ctx); + + *state = ctx; + return 0; +} + +static int teardown(void **state) { + struct context *ctx = *state; + TALLOC_FREE(ctx); + return 0; +} + +/****************************************************************************** + * Helper functions + ******************************************************************************/ + +/* + * Build the "Original" user details record, i.e. the user being + * authenticated + */ +static struct ldb_message* create_message(TALLOC_CTX *ctx) +{ + + int ret; + struct timeval tv_now = timeval_current(); + NTTIME now = timeval_to_nttime(&tv_now); + + struct ldb_message* msg = ldb_msg_new(ctx); + + assert_non_null(msg); + ret = samdb_msg_add_int(ctx, msg, msg, "badPwdCount", 10); + assert_int_equal(LDB_SUCCESS, ret); + ret = __real_samdb_msg_add_int64(ctx, msg, msg, "badPasswordTime", now); + assert_int_equal(LDB_SUCCESS, ret); + ret = __real_samdb_msg_add_int64(ctx, msg, msg, "lockoutTime", now); + assert_int_equal(LDB_SUCCESS, ret); + return msg; +} + +/* + * Add the "objectGUID element to a message + */ +static void add_object_guid(struct ldb_message *msg, + struct GUID *guid) +{ + + NTSTATUS status; + int ret; + DATA_BLOB guid_blob; + struct ldb_val *new_val; + + status = GUID_to_ndr_blob(guid, msg, &guid_blob); + assert_true(NT_STATUS_IS_OK(status)); + new_val = talloc(msg, struct ldb_val); + assert_non_null(new_val); + new_val->data = talloc_steal(new_val, guid_blob.data); + new_val->length = guid_blob.length; + ret = ldb_msg_add_value(msg, "objectGUID", new_val, NULL); + assert_int_equal(LDB_SUCCESS, ret); +} + +/* + * Add an objectSID in string form to the supplied message + * + * + */ +static void add_sid( + struct ldb_message *msg, + const char *sid_str) +{ + struct ldb_val v; + enum ndr_err_code ndr_err; + struct dom_sid *sid = NULL; + + sid = talloc_zero(msg, struct dom_sid); + assert_non_null(sid); + assert_true(string_to_sid(sid, sid_str)); + ndr_err = ndr_push_struct_blob( + &v, msg, sid, (ndr_push_flags_fn_t)ndr_push_dom_sid); + assert_true(NDR_ERR_CODE_IS_SUCCESS(ndr_err)); + assert_int_equal(0, ldb_msg_add_value(msg, "objectSID", &v, NULL)); +} + +/* + * Build an ldb_result, for the re-reading of a user record + * + * if account_control < 0 then the msDS-User-Account-Control-Computed + * element is not included + * otherwise it is set to the value passed in account_control. + * + */ +static struct ldb_result *build_reread_result( + struct ldb_context *ldb, + TALLOC_CTX *ctx, + int account_control) +{ + struct ldb_message *msg = NULL; + int ret; + + struct ldb_result *res = talloc_zero(ctx, struct ldb_result); + + assert_non_null(res); + res->count = 1; + res->msgs = talloc_array(res, struct ldb_message *, 1); + + msg = create_message(res); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + if (account_control >= 0) { + ret = samdb_msg_add_int( + ldb, + msg, + msg, + "msDS-User-Account-Control-Computed", + account_control); + assert_int_equal(LDB_SUCCESS, ret); + } + + res->msgs[0] = msg; + return res; +} + +/* + * Build a mock domain pso ldb_result + */ +static struct ldb_result *build_domain_pso_result( + struct ldb_context *ldb, + TALLOC_CTX *ctx) +{ + struct ldb_message *msg = NULL; + struct ldb_result *res = talloc_zero(ctx, struct ldb_result); + + assert_non_null(res); + res->count = 1; + res->msgs = talloc_array(res, struct ldb_message *, 1); + assert_non_null(res->msgs); + msg = talloc_zero(res, struct ldb_message); + assert_non_null(msg); + res->msgs[0] = msg; + return res; +} + +/***************************************************************************** + * authsam_reread_user_logon_data unit tests + *****************************************************************************/ +/* + * authsam_reread_user_logon_data unable to re-read the user record. + * + */ +static void test_reread_read_failure(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_message *cur = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, NULL); + will_return(__wrap_dsdb_search_dn, LDB_ERR_NO_SUCH_OBJECT); + + will_return_count(__wrap_samdb_msg_add_int64, LDB_SUCCESS, -3); + + status = authsam_reread_user_logon_data(ldb, ctx, msg, &cur); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_reread_user_logon_data account control flags missing from + * re-read data + * + */ +static void test_reread_missing_account_control(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_message *cur = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, build_reread_result(ldb, ctx, -1)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + status = authsam_reread_user_logon_data(ldb, ctx, msg, &cur); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_reread_user_logon_data account locked + * re-read data + * + */ +static void test_reread_account_locked(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_message *cur = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, build_reread_result(ldb, ctx, UF_LOCKOUT)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + status = authsam_reread_user_logon_data(ldb, ctx, msg, &cur); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_ACCOUNT_LOCKED_OUT)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_reread_user_logon_data account is not locked + * re-read data + * + */ +static void test_reread_account_not_locked(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_message *cur = NULL; + struct GUID guid; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + size_t result_size = 0; + NTSTATUS status; + struct ldb_result *res = NULL; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + msg = create_message(ctx); + guid = GUID_random(); + add_object_guid(msg, &guid); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + /* + * authsam_reread_user_logon_data returns the ldb_message portion + * of the ldb_result created by build_reread_result. + * So the tests for memory leaks will need to adjust for that + */ + res = build_reread_result(ldb, ctx, 0); + will_return(__wrap_dsdb_search_dn, res); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + result_size = talloc_total_size(res) - + talloc_total_size(res->msgs[0]); + before = talloc_total_size(ctx) - result_size; + + status = authsam_reread_user_logon_data(ldb, ctx, msg, &cur); + assert_true(NT_STATUS_IS_OK(status)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + + +/***************************************************************************** + * authsam_update_bad_pwd_count unit tests + *****************************************************************************/ + +/* + * authsam_update_bad_pwd_account + * + * Unable to read the domain_dn record + * + */ +static void test_update_bad_domain_dn_search_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = talloc_zero(ctx, struct ldb_message); + assert_non_null(msg); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, NULL); + will_return(__wrap_dsdb_search_dn, LDB_ERR_NO_SUCH_OBJECT); + + before = talloc_total_size(ctx); + + status = authsam_update_bad_pwd_count(ldb, msg, domain_dn); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_DB_CORRUPTION)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_update_bad_pwd_account + * + * authsam_get_user_pso failure + * + */ +static void test_update_bad_get_pso_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + struct ldb_dn *pso_dn = NULL; + const char *pso_dn_str = "CN=PSO"; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + int ret; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + pso_dn = ldb_dn_new(ctx, ldb, pso_dn_str); + assert_non_null(pso_dn); + + msg = talloc_zero(ctx, struct ldb_message); + assert_non_null(msg); + ret = ldb_msg_add_string(msg, "msDS-ResultantPSO", pso_dn_str); + assert_int_equal(LDB_SUCCESS, ret); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, pso_dn); + will_return(__wrap_dsdb_search_dn, NULL); + will_return(__wrap_dsdb_search_dn, LDB_ERR_NO_SUCH_OBJECT); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, build_reread_result(ldb, ctx, 0)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + status = authsam_update_bad_pwd_count(ldb, msg, domain_dn); + assert_true(NT_STATUS_IS_OK(status)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + + +/* + * authsam_update_bad_pwd_account + * + * start_transaction failure + * + */ +static void test_update_bad_start_txn_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = talloc_zero(ctx, struct ldb_message); + assert_non_null(msg); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + ldb_transaction_start_ret = LDB_ERR_OPERATIONS_ERROR; + + status = authsam_update_bad_pwd_count(ldb, msg, domain_dn); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_update_bad_pwd_account + * + * User details re-read failed + * + */ +static void test_update_bad_reread_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = talloc_zero(ctx, struct ldb_message); + assert_non_null(msg); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, NULL); + will_return(__wrap_dsdb_search_dn, LDB_ERR_NO_SUCH_OBJECT); + + status = authsam_update_bad_pwd_count(ldb, msg, domain_dn); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_true(transaction_cancelled); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_update_bad_pwd_account + * + * User details re-read reported locked out. + * + */ +static void test_update_bad_reread_locked_out(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + struct GUID guid; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + assert_non_null(msg); + guid = GUID_random(); + add_object_guid(msg, &guid); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, build_reread_result(ldb, ctx, UF_LOCKOUT)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + status = authsam_update_bad_pwd_count(ldb, msg, domain_dn); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_ACCOUNT_LOCKED_OUT)); + assert_false(transaction_cancelled); + assert_true(transaction_committed); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_update_bad_pwd_account + * + * Transaction cancel failure + */ +static void test_update_bad_txn_cancel_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = talloc_zero(ctx, struct ldb_message); + assert_non_null(msg); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, NULL); + will_return(__wrap_dsdb_search_dn, LDB_ERR_NO_SUCH_OBJECT); + + ldb_transaction_cancel_ret = LDB_ERR_OPERATIONS_ERROR; + + status = authsam_update_bad_pwd_count(ldb, msg, domain_dn); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_true(in_transaction); + assert_false(transaction_cancelled); + assert_false(transaction_committed); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * The following tests all expect the same setup, that is a normal + * good user object and empty domain object. + * + * returns the talloc size after result array setup for leak tests + */ +static size_t setup_bad_password_search_results(TALLOC_CTX *ctx, + struct ldb_context *ldb, + struct ldb_dn *domain_dn, + struct ldb_dn *user_dn) +{ + size_t before = 0; + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, user_dn); + will_return(__wrap_dsdb_search_dn, build_reread_result(ldb, ctx, 0)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + return before; +} + + +/* + * authsam_update_bad_pwd_account + * + * dsdb_update_bad_pwd_count failure + * + */ +static void test_update_bad_update_count_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + struct GUID guid; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + guid = GUID_random(); + add_object_guid(msg, &guid); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = setup_bad_password_search_results(ctx, ldb, + domain_dn, + msg->dn); + + dsdb_update_bad_pwd_count_ret = NT_STATUS_INTERNAL_ERROR; + + status = authsam_update_bad_pwd_count(ldb, msg, domain_dn); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_true(transaction_cancelled); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_update_bad_pwd_account + * + * No need to update the bad password stats + * + */ +static void test_update_bad_no_update_required(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + struct GUID guid; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + guid = GUID_random(); + add_object_guid(msg, &guid); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = setup_bad_password_search_results(ctx, ldb, + domain_dn, + msg->dn); + + status = authsam_update_bad_pwd_count(ldb, msg, domain_dn); + assert_true(NT_STATUS_IS_OK(status)); + assert_true(transaction_committed); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_update_bad_pwd_account + * + * Transaction commit failure + * + */ +static void test_update_bad_commit_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + struct GUID guid; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + guid = GUID_random(); + add_object_guid(msg, &guid); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = setup_bad_password_search_results(ctx, ldb, + domain_dn, + msg->dn); + + ldb_transaction_commit_ret = LDB_ERR_OPERATIONS_ERROR; + + status = authsam_update_bad_pwd_count(ldb, msg, domain_dn); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_true(in_transaction); + assert_false(transaction_cancelled); + assert_false(transaction_committed); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_update_bad_pwd_account + * + * ldb_build_mod_req failed building the user update details + * + */ +static void test_update_bad_build_mod_request_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + struct GUID guid; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + guid = GUID_random(); + add_object_guid(msg, &guid); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = setup_bad_password_search_results(ctx, ldb, + domain_dn, + msg->dn); + + dsdb_update_bad_pwd_count_res = talloc_zero(ctx, struct ldb_message); + ldb_build_mod_req_ret = LDB_ERR_OPERATIONS_ERROR; + + status = authsam_update_bad_pwd_count(ldb, msg, domain_dn); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_true(transaction_cancelled); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_update_bad_pwd_account + * + * ldb_request_add_control failed to add DSDB_CONTROL_FORCE_RODC_LOCAL_CHANGE + * to the user update record. + * + */ +static void test_update_bad_add_control_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + struct GUID guid; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + guid = GUID_random(); + add_object_guid(msg, &guid); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = setup_bad_password_search_results(ctx, ldb, + domain_dn, + msg->dn); + + dsdb_update_bad_pwd_count_res = talloc_zero(ctx, struct ldb_message); + ldb_build_mod_req_res = talloc_zero(ctx, struct ldb_request); + ldb_request_add_control_ret = LDB_ERR_OPERATIONS_ERROR; + + status = authsam_update_bad_pwd_count(ldb, msg, domain_dn); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_true(transaction_cancelled); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_update_bad_pwd_account + * + * call to ldb_request failed + * + */ +static void test_update_bad_ldb_request_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + struct GUID guid; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + guid = GUID_random(); + add_object_guid(msg, &guid); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = setup_bad_password_search_results(ctx, ldb, + domain_dn, + msg->dn); + + dsdb_update_bad_pwd_count_res = talloc_zero(ctx, struct ldb_message); + ldb_build_mod_req_res = talloc_zero(ctx, struct ldb_request); + ldb_request_ret = LDB_ERR_OPERATIONS_ERROR; + + status = authsam_update_bad_pwd_count(ldb, msg, domain_dn); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_true(transaction_cancelled); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_update_bad_pwd_account + * + * call to ldb_wait failed + * + */ +static void test_update_bad_ldb_wait_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + struct GUID guid; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + guid = GUID_random(); + add_object_guid(msg, &guid); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = setup_bad_password_search_results(ctx, ldb, + domain_dn, + msg->dn); + + dsdb_update_bad_pwd_count_res = talloc_zero(ctx, struct ldb_message); + ldb_build_mod_req_res = talloc_zero(ctx, struct ldb_request); + ldb_wait_ret = LDB_ERR_OPERATIONS_ERROR; + + status = authsam_update_bad_pwd_count(ldb, msg, domain_dn); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_true(transaction_cancelled); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/***************************************************************************** + * authsam_logon_success_accounting unit tests + *****************************************************************************/ +/* + * authsam_logon_success_accounting + * + * start_transaction failure + * + */ +static void test_success_accounting_start_txn_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + struct GUID guid; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + guid = GUID_random(); + add_object_guid(msg, &guid); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + ldb_transaction_start_ret = LDB_ERR_OPERATIONS_ERROR; + + status = authsam_logon_success_accounting( + ldb, msg, domain_dn, true, NULL); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_logon_success_accounting + * + * User details re-read failed + * + */ +static void test_success_accounting_reread_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + struct GUID guid; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + guid = GUID_random(); + add_object_guid(msg, &guid); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, NULL); + will_return(__wrap_dsdb_search_dn, LDB_ERR_NO_SUCH_OBJECT); + + status = authsam_logon_success_accounting( + ldb, msg, domain_dn, true, NULL); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_true(transaction_cancelled); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_logon_success_accounting + * + * ldb_msg_new failed + * + */ +static void test_success_accounting_ldb_msg_new_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + struct GUID guid; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + guid = GUID_random(); + add_object_guid(msg, &guid); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, build_reread_result(ldb, ctx, 0)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + ldb_msg_new_fail = true; + + status = authsam_logon_success_accounting( + ldb, msg, domain_dn, true, NULL); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_NO_MEMORY)); + assert_true(transaction_cancelled); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_logon_success_accounting + * + * samdb_rodc failed + * + */ +static void test_success_accounting_samdb_rodc_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + struct GUID guid; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + guid = GUID_random(); + add_object_guid(msg, &guid); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + samdb_rodc_ret = LDB_ERR_OPERATIONS_ERROR; + + status = authsam_logon_success_accounting( + ldb, msg, domain_dn, true, NULL); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_false(in_transaction); + assert_false(transaction_cancelled); + assert_false(transaction_committed); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_logon_success_accounting + * + * authsam_update_lastlogon_timestamp failed + * + */ +static void test_success_accounting_update_lastlogon_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + struct GUID guid; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + guid = GUID_random(); + add_object_guid(msg, &guid); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + ldb_build_mod_req_res = talloc_zero(ctx, struct ldb_request); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, build_reread_result(ldb, ctx, 0)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + will_return(__wrap_samdb_msg_add_int64, LDB_ERR_OPERATIONS_ERROR); + + status = authsam_logon_success_accounting( + ldb, msg, domain_dn, true, NULL); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_NO_MEMORY)); + assert_true(transaction_cancelled); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_logon_success_accounting + * + * ldb_build_mod_req failed + * + */ +static void test_success_accounting_build_mod_req_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + struct GUID guid; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + guid = GUID_random(); + add_object_guid(msg, &guid); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, build_reread_result(ldb, ctx, 0)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + ldb_build_mod_req_ret = LDB_ERR_OPERATIONS_ERROR; + + will_return(__wrap_samdb_msg_add_int64, LDB_SUCCESS); + will_return(__wrap_samdb_msg_add_int64, LDB_SUCCESS); + + status = authsam_logon_success_accounting( + ldb, msg, domain_dn, true, NULL); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_true(transaction_cancelled); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_logon_success_accounting + * + * ldb_request_add_control failed + * + */ +static void test_success_accounting_add_control_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + struct GUID guid; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + guid = GUID_random(); + add_object_guid(msg, &guid); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, build_reread_result(ldb, ctx, 0)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + ldb_build_mod_req_res = talloc_zero(ldb, struct ldb_request); + ldb_request_add_control_ret = LDB_ERR_OPERATIONS_ERROR; + + will_return(__wrap_samdb_msg_add_int64, LDB_SUCCESS); + will_return(__wrap_samdb_msg_add_int64, LDB_SUCCESS); + + status = authsam_logon_success_accounting( + ldb, msg, domain_dn, true, NULL); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_true(transaction_cancelled); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_logon_success_accounting + * + * ldb_request failed + * + */ +static void test_success_accounting_ldb_request_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + struct GUID guid; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + guid = GUID_random(); + add_object_guid(msg, &guid); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, build_reread_result(ldb, ctx, 0)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + ldb_build_mod_req_res = talloc_zero(ldb, struct ldb_request); + ldb_request_ret = LDB_ERR_OPERATIONS_ERROR; + + will_return(__wrap_samdb_msg_add_int64, LDB_SUCCESS); + will_return(__wrap_samdb_msg_add_int64, LDB_SUCCESS); + + status = authsam_logon_success_accounting( + ldb, msg, domain_dn, true, NULL); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_true(transaction_cancelled); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_logon_success_accounting + * + * ldb_wait failed + * + */ +static void test_success_accounting_ldb_wait_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + struct GUID guid; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + guid = GUID_random(); + add_object_guid(msg, &guid); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, build_reread_result(ldb, ctx, 0)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + ldb_build_mod_req_res = talloc_zero(ldb, struct ldb_request); + ldb_wait_ret = LDB_ERR_OPERATIONS_ERROR; + + will_return(__wrap_samdb_msg_add_int64, LDB_SUCCESS); + will_return(__wrap_samdb_msg_add_int64, LDB_SUCCESS); + + status = authsam_logon_success_accounting( + ldb, msg, domain_dn, true, NULL); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_true(transaction_cancelled); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_logon_success_accounting + * + * ldb_transaction_commit failed + * + */ +static void test_success_accounting_commit_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + struct GUID guid; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + guid = GUID_random(); + add_object_guid(msg, &guid); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, build_reread_result(ldb, ctx, 0)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + ldb_build_mod_req_res = talloc_zero(ldb, struct ldb_request); + ldb_transaction_commit_ret = LDB_ERR_OPERATIONS_ERROR; + + will_return(__wrap_samdb_msg_add_int64, LDB_SUCCESS); + will_return(__wrap_samdb_msg_add_int64, LDB_SUCCESS); + + status = authsam_logon_success_accounting( + ldb, msg, domain_dn, true, NULL); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_true(in_transaction); + assert_false(transaction_cancelled); + assert_false(transaction_committed); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_logon_success_accounting + * + * ldb_wait failed and then ldb_transaction_cancel failed + * + */ +static void test_success_accounting_rollback_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + struct GUID guid; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + guid = GUID_random(); + add_object_guid(msg, &guid); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, build_reread_result(ldb, ctx, 0)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + ldb_build_mod_req_res = talloc_zero(ldb, struct ldb_request); + ldb_wait_ret = LDB_ERR_OPERATIONS_ERROR; + ldb_transaction_cancel_ret = LDB_ERR_OPERATIONS_ERROR; + + will_return(__wrap_samdb_msg_add_int64, LDB_SUCCESS); + will_return(__wrap_samdb_msg_add_int64, LDB_SUCCESS); + + status = authsam_logon_success_accounting( + ldb, msg, domain_dn, true, NULL); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_true(in_transaction); + assert_false(transaction_cancelled); + assert_false(transaction_committed); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * get_bad_password_db + * + * ldb_get_opaque failure. + */ +static void test_get_bad_password_get_opaque_failed(void **state) { + struct ldb_context *ldb = NULL; + TALLOC_CTX *ctx = NULL; + struct db_context *db = NULL; + size_t before = 0; + size_t after = 0; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + /* + * clear the mock ldb_get_opaque return value, so that we get a null + * response. + */ + TALLOC_FREE(ldb_get_opaque_ret); + + before = talloc_total_size(ctx); + + db = authsam_get_bad_password_db(ctx, ldb); + assert_null(db); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * get_bad_password_db + * + * cluster_db_tmp_open failure. + */ +static void test_get_bad_password_db_open_failed(void **state) { + struct ldb_context *ldb = NULL; + TALLOC_CTX *ctx = NULL; + struct db_context *db = NULL; + size_t before = 0; + size_t after = 0; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + /* + * Clear the mock cluster_db_tmp_open return value so that + * it returns NULL + */ + TALLOC_FREE(cluster_db_tmp_open_ret); + before = talloc_total_size(ctx); + + db = authsam_get_bad_password_db(ctx, ldb); + assert_null(db); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * set_bad_password_indicator + * + * set_bad_password_indicator failure. + */ +static void test_set_bad_password_indicator_get_db_failed(void **state) { + struct ldb_context *ldb = NULL; + TALLOC_CTX *ctx = NULL; + NTSTATUS status; + size_t before = 0; + size_t after = 0; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + /* + * Clear the mock cluster_db_tmp_open return value so that + * it returns NULL + */ + TALLOC_FREE(cluster_db_tmp_open_ret); + before = talloc_total_size(ctx); + + status = authsam_set_bad_password_indicator(ldb, ctx, NULL); + assert_true(NT_STATUS_EQUAL(NT_STATUS_INTERNAL_ERROR, status)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * set_bad_password_indicator + * + * get_object_sid_as_tdb_data failure. + */ +static void test_set_bad_password_indicator_get_object_sid_failed( + void **state) +{ + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + TALLOC_CTX *ctx = NULL; + NTSTATUS status; + size_t before = 0; + size_t after = 0; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + /* + * The created message does not contain an objectSid, so + * get_object_sid_as_tdb_data will fail. + */ + msg = create_message(ctx); + + before = talloc_total_size(ctx); + + status = authsam_set_bad_password_indicator(ldb, ctx, msg); + assert_true(NT_STATUS_EQUAL(NT_STATUS_INTERNAL_ERROR, status)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * set_bad_password_indicator + * + * dbwrap_store failure. + */ +static void test_set_bad_password_indicator_dbwrap_store_failed( + void **state) +{ + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + TALLOC_CTX *ctx = NULL; + NTSTATUS status; + size_t before = 0; + size_t after = 0; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1010"); + + dbwrap_store_ret = NT_STATUS_INTERNAL_DB_CORRUPTION; + + before = talloc_total_size(ctx); + + status = authsam_set_bad_password_indicator(ldb, ctx, msg); + assert_true(NT_STATUS_EQUAL(NT_STATUS_INTERNAL_DB_CORRUPTION, status)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * check_bad_password_indicator + * + * set_bad_password_indicator failure. + */ +static void test_check_bad_password_indicator_get_db_failed(void **state) { + struct ldb_context *ldb = NULL; + TALLOC_CTX *ctx = NULL; + NTSTATUS status; + size_t before = 0; + size_t after = 0; + bool exists = false; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + /* + * Clear the mock cluster_db_tmp_open return value so that + * it returns NULL + */ + TALLOC_FREE(cluster_db_tmp_open_ret); + before = talloc_total_size(ctx); + + status = authsam_check_bad_password_indicator(ldb, ctx, &exists, NULL); + assert_true(NT_STATUS_EQUAL(NT_STATUS_INTERNAL_ERROR, status)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * check_bad_password_indicator + * + * get_object_sid_as_tdb_data failure. + */ +static void test_check_bad_password_indicator_get_object_sid_failed( + void **state) +{ + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + TALLOC_CTX *ctx = NULL; + NTSTATUS status; + size_t before = 0; + size_t after = 0; + bool exists = false; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + /* + * The created message does not contain an objectSid, so + * get_object_sid_as_tdb_data will fail. + */ + msg = create_message(ctx); + + before = talloc_total_size(ctx); + + status = authsam_check_bad_password_indicator(ldb, ctx, &exists, msg); + assert_true(NT_STATUS_EQUAL(NT_STATUS_INTERNAL_ERROR, status)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * clear_bad_password_indicator + * + * set_bad_password_indicator failure. + */ +static void test_clear_bad_password_indicator_get_db_failed(void **state) { + struct ldb_context *ldb = NULL; + TALLOC_CTX *ctx = NULL; + NTSTATUS status; + size_t before = 0; + size_t after = 0; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + /* + * Clear the mock cluster_db_tmp_open return value so that + * it returns NULL + */ + TALLOC_FREE(cluster_db_tmp_open_ret); + before = talloc_total_size(ctx); + + status = authsam_clear_bad_password_indicator(ldb, ctx, NULL); + assert_true(NT_STATUS_EQUAL(NT_STATUS_INTERNAL_ERROR, status)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * clear_bad_password_indicator + * + * get_object_sid_as_tdb_data failure. + */ +static void test_clear_bad_password_indicator_get_object_sid_failed( + void **state) +{ + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + TALLOC_CTX *ctx = NULL; + NTSTATUS status; + size_t before = 0; + size_t after = 0; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + /* + * The created message does not contain an objectSid, so + * get_object_sid_as_tdb_data will fail. + */ + msg = create_message(ctx); + + before = talloc_total_size(ctx); + + status = authsam_clear_bad_password_indicator(ldb, ctx, msg); + assert_true(NT_STATUS_EQUAL(NT_STATUS_INTERNAL_ERROR, status)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * clear_bad_password_indicator + * + * dbwrap_delete failure. + */ +static void test_clear_bad_password_indicator_dbwrap_store_failed( + void **state) +{ + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + TALLOC_CTX *ctx = NULL; + NTSTATUS status; + size_t before = 0; + size_t after = 0; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1010"); + + dbwrap_delete_ret = NT_STATUS_INTERNAL_DB_CORRUPTION; + + before = talloc_total_size(ctx); + + status = authsam_clear_bad_password_indicator(ldb, ctx, msg); + assert_true(NT_STATUS_EQUAL(NT_STATUS_INTERNAL_DB_CORRUPTION, status)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * clear_bad_password_indicator + * + * dbwrap_delete returns NT_STATUS_NOT_FOUND. + */ +static void test_clear_bad_pwd_indicator_dbwrap_store_not_found( + void **state) +{ + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + TALLOC_CTX *ctx = NULL; + NTSTATUS status; + size_t before = 0; + size_t after = 0; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1010"); + + dbwrap_delete_ret = NT_STATUS_NOT_FOUND; + + before = talloc_total_size(ctx); + + status = authsam_clear_bad_password_indicator(ldb, ctx, msg); + assert_true(NT_STATUS_IS_OK(status)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +int main(int argc, const char **argv) +{ + const struct CMUnitTest tests[] = { + cmocka_unit_test_setup_teardown( + test_reread_read_failure, setup, teardown), + cmocka_unit_test_setup_teardown( + test_reread_missing_account_control, setup, teardown), + cmocka_unit_test_setup_teardown( + test_reread_account_locked, setup, teardown), + cmocka_unit_test_setup_teardown( + test_reread_account_not_locked, setup, teardown), + cmocka_unit_test_setup_teardown( + test_update_bad_domain_dn_search_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_update_bad_get_pso_failed, setup, teardown), + cmocka_unit_test_setup_teardown( + test_update_bad_start_txn_failed, setup, teardown), + cmocka_unit_test_setup_teardown( + test_update_bad_reread_failed, setup, teardown), + cmocka_unit_test_setup_teardown( + test_update_bad_reread_locked_out, setup, teardown), + cmocka_unit_test_setup_teardown( + test_update_bad_update_count_failed, setup, teardown), + cmocka_unit_test_setup_teardown( + test_update_bad_no_update_required, setup, teardown), + cmocka_unit_test_setup_teardown( + test_update_bad_build_mod_request_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_update_bad_add_control_failed, setup, teardown), + cmocka_unit_test_setup_teardown( + test_update_bad_ldb_request_failed, setup, teardown), + cmocka_unit_test_setup_teardown( + test_update_bad_ldb_wait_failed, setup, teardown), + cmocka_unit_test_setup_teardown( + test_update_bad_txn_cancel_failed, setup, teardown), + cmocka_unit_test_setup_teardown( + test_update_bad_commit_failed, setup, teardown), + cmocka_unit_test_setup_teardown( + test_success_accounting_start_txn_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_success_accounting_reread_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_success_accounting_ldb_msg_new_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_success_accounting_samdb_rodc_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_success_accounting_update_lastlogon_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_success_accounting_build_mod_req_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_success_accounting_add_control_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_success_accounting_ldb_request_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_success_accounting_ldb_wait_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_success_accounting_commit_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_success_accounting_rollback_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_get_bad_password_get_opaque_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_get_bad_password_db_open_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_set_bad_password_indicator_get_db_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_set_bad_password_indicator_get_object_sid_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_set_bad_password_indicator_dbwrap_store_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_check_bad_password_indicator_get_db_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_check_bad_password_indicator_get_object_sid_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_clear_bad_password_indicator_get_db_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_clear_bad_password_indicator_get_object_sid_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_clear_bad_password_indicator_dbwrap_store_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_clear_bad_pwd_indicator_dbwrap_store_not_found, + setup, + teardown), + }; + + cmocka_set_message_output(CM_OUTPUT_SUBUNIT); + return cmocka_run_group_tests(tests, NULL, NULL); +} diff --git a/source4/auth/wscript_build b/source4/auth/wscript_build index 381a7b19bf0..ff1a61a9566 100644 --- a/source4/auth/wscript_build +++ b/source4/auth/wscript_build @@ -49,6 +49,17 @@ bld.SAMBA_BINARY('test_kerberos', for_selftest=True ) +bld.SAMBA_BINARY('test_auth_sam', + source='tests/sam.c', + deps='cmocka samdb samba-security ldb tevent', + local_include=False, + for_selftest=True, + ldflags=''' + -Wl,--wrap,dsdb_search_dn + -Wl,--wrap,samdb_msg_add_int64 + ''' + ) + pytalloc_util = bld.pyembed_libname('pytalloc-util') pyparam_util = bld.pyembed_libname('pyparam_util') pyldb_util = bld.pyembed_libname('pyldb-util') -- 2.35.0 From 585707e9c0ec25ea75ef9032450ad1f70c4c6dc5 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 5 Jul 2022 20:17:49 +1200 Subject: [PATCH 09/21] CVE-2021-20251 auth4: Detect ACCOUNT_LOCKED_OUT error for password change This is more specific than NT_STATUS_UNSUCCESSFUL, and for the SAMR password change, matches the result the call to samdb_result_passwords() would give. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Joseph Sutton --- selftest/knownfail_heimdal_kdc | 1 - selftest/knownfail_mit_kdc | 1 - source4/dsdb/common/util.c | 7 ++++++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc index c22c72d0619..e8e9258bef8 100644 --- a/selftest/knownfail_heimdal_kdc +++ b/selftest/knownfail_heimdal_kdc @@ -62,4 +62,3 @@ ^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_samr.ad_dc:local ^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_transaction_kdc.ad_dc:local ^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_transaction_ntlm.ad_dc:local -^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_transaction_samr.ad_dc:local diff --git a/selftest/knownfail_mit_kdc b/selftest/knownfail_mit_kdc index 17b06de14ff..9260ff0c096 100644 --- a/selftest/knownfail_mit_kdc +++ b/selftest/knownfail_mit_kdc @@ -441,4 +441,3 @@ samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_ ^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_samr.ad_dc:local ^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_transaction_kdc.ad_dc:local ^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_transaction_ntlm.ad_dc:local -^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_transaction_samr.ad_dc:local diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c index 19667ee8611..0f17cbac1d2 100644 --- a/source4/dsdb/common/util.c +++ b/source4/dsdb/common/util.c @@ -2230,7 +2230,8 @@ int samdb_set_password_callback(struct ldb_request *req, struct ldb_reply *ares) * change failed. * * Results: NT_STATUS_OK, NT_STATUS_INVALID_PARAMETER, NT_STATUS_UNSUCCESSFUL, - * NT_STATUS_WRONG_PASSWORD, NT_STATUS_PASSWORD_RESTRICTION + * NT_STATUS_WRONG_PASSWORD, NT_STATUS_PASSWORD_RESTRICTION, + * NT_STATUS_ACCESS_DENIED, NT_STATUS_ACCOUNT_LOCKED_OUT, NT_STATUS_NO_MEMORY */ static NTSTATUS samdb_set_password_internal(struct ldb_context *ldb, TALLOC_CTX *mem_ctx, struct ldb_dn *user_dn, struct ldb_dn *domain_dn, @@ -2403,6 +2404,9 @@ static NTSTATUS samdb_set_password_internal(struct ldb_context *ldb, TALLOC_CTX if (W_ERROR_EQUAL(werr, WERR_PASSWORD_RESTRICTION)) { status = NT_STATUS_PASSWORD_RESTRICTION; } + if (W_ERROR_EQUAL(werr, WERR_ACCOUNT_LOCKED_OUT)) { + status = NT_STATUS_ACCOUNT_LOCKED_OUT; + } } } else if (ret == LDB_ERR_NO_SUCH_OBJECT) { /* don't let the caller know if an account doesn't exist */ @@ -2452,6 +2456,7 @@ NTSTATUS samdb_set_password(struct ldb_context *ldb, TALLOC_CTX *mem_ctx, * Results: NT_STATUS_OK, NT_STATUS_INTERNAL_DB_CORRUPTION, * NT_STATUS_INVALID_PARAMETER, NT_STATUS_UNSUCCESSFUL, * NT_STATUS_WRONG_PASSWORD, NT_STATUS_PASSWORD_RESTRICTION, + * NT_STATUS_ACCESS_DENIED, NT_STATUS_ACCOUNT_LOCKED_OUT, NT_STATUS_NO_MEMORY * NT_STATUS_TRANSACTION_ABORTED, NT_STATUS_NO_SUCH_USER */ NTSTATUS samdb_set_password_sid(struct ldb_context *ldb, TALLOC_CTX *mem_ctx, -- 2.35.0 From f011d3edaf8ff072d6d4c9f903fac1caa6207cee Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 30 Mar 2021 18:01:39 +1300 Subject: [PATCH 10/21] CVE-2021-20251 s4 auth: make bad password count increment atomic Ensure that the bad password count is incremented atomically, and that the successful logon accounting data is updated atomically. Use bad password indicator (in a distinct TDB) to determine if to open a transaction We open a transaction when we have seen the hint that this user has recorded a bad password. This allows us to avoid always needing one, while not missing a possible lockout. We also go back and get a transation if we did not take out one out but we chose to do a write (eg for lastLogonTimestamp) Based on patches by Gary Lockyer BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Andrew Bartlett Reviewed-by: Joseph Sutton --- selftest/knownfail.d/auth-sam | 12 -- selftest/knownfail_heimdal_kdc | 4 - selftest/knownfail_mit_kdc | 3 - source4/auth/sam.c | 296 +++++++++++++++++++++++++++------ 4 files changed, 246 insertions(+), 69 deletions(-) diff --git a/selftest/knownfail.d/auth-sam b/selftest/knownfail.d/auth-sam index c4002ec0c53..aecca4ae088 100644 --- a/selftest/knownfail.d/auth-sam +++ b/selftest/knownfail.d/auth-sam @@ -10,15 +10,3 @@ ^samba.unittests.auth.sam.test_success_accounting_samdb_rodc_failed.none ^samba.unittests.auth.sam.test_success_accounting_start_txn_failed.none ^samba.unittests.auth.sam.test_success_accounting_update_lastlogon_failed.none -^samba.unittests.auth.sam.test_update_bad_add_control_failed.none -^samba.unittests.auth.sam.test_update_bad_build_mod_request_failed.none -^samba.unittests.auth.sam.test_update_bad_commit_failed.none -^samba.unittests.auth.sam.test_update_bad_get_pso_failed.none -^samba.unittests.auth.sam.test_update_bad_ldb_request_failed.none -^samba.unittests.auth.sam.test_update_bad_ldb_wait_failed.none -^samba.unittests.auth.sam.test_update_bad_no_update_required.none -^samba.unittests.auth.sam.test_update_bad_reread_failed.none -^samba.unittests.auth.sam.test_update_bad_reread_locked_out.none -^samba.unittests.auth.sam.test_update_bad_start_txn_failed.none -^samba.unittests.auth.sam.test_update_bad_txn_cancel_failed.none -^samba.unittests.auth.sam.test_update_bad_update_count_failed.none diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc index e8e9258bef8..deb84e63a19 100644 --- a/selftest/knownfail_heimdal_kdc +++ b/selftest/knownfail_heimdal_kdc @@ -57,8 +57,4 @@ # # Lockout tests # -^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_kdc.ad_dc:local -^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_ntlm.ad_dc:local -^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_samr.ad_dc:local ^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_transaction_kdc.ad_dc:local -^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_transaction_ntlm.ad_dc:local diff --git a/selftest/knownfail_mit_kdc b/selftest/knownfail_mit_kdc index 9260ff0c096..e843f687de0 100644 --- a/selftest/knownfail_mit_kdc +++ b/selftest/knownfail_mit_kdc @@ -437,7 +437,4 @@ samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_ # Lockout tests # ^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_kdc.ad_dc:local -^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_ntlm.ad_dc:local -^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_samr.ad_dc:local ^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_transaction_kdc.ad_dc:local -^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_transaction_ntlm.ad_dc:local diff --git a/source4/auth/sam.c b/source4/auth/sam.c index 85a23075c4e..cd81efe81c4 100644 --- a/source4/auth/sam.c +++ b/source4/auth/sam.c @@ -1073,7 +1073,9 @@ NTSTATUS authsam_update_bad_pwd_count(struct ldb_context *sam_ctx, NTSTATUS status; struct ldb_result *domain_res; struct ldb_message *msg_mod = NULL; + struct ldb_message *current = NULL; struct ldb_message *pso_msg = NULL; + bool txn_active = false; TALLOC_CTX *mem_ctx; mem_ctx = talloc_new(msg); @@ -1098,14 +1100,65 @@ NTSTATUS authsam_update_bad_pwd_count(struct ldb_context *sam_ctx, ret, ldb_dn_get_linearized(msg->dn)); } - status = dsdb_update_bad_pwd_count(mem_ctx, sam_ctx, - msg, domain_res->msgs[0], pso_msg, - &msg_mod); + /* + * To ensure that the bad password count is updated atomically, + * we need to: + * begin a transaction + * re-read the account details, + * using the msgs[0], + pso_msg, + &msg_mod); + if (!NT_STATUS_IS_OK(status)) { + status = NT_STATUS_INTERNAL_ERROR; + goto error; + } + + /* + * Write the data back to disk if required. + */ if (msg_mod != NULL) { struct ldb_request *req; @@ -1116,7 +1169,9 @@ NTSTATUS authsam_update_bad_pwd_count(struct ldb_context *sam_ctx, ldb_op_default_callback, NULL); if (ret != LDB_SUCCESS) { - goto done; + TALLOC_FREE(msg_mod); + status = NT_STATUS_INTERNAL_ERROR; + goto error; } ret = ldb_request_add_control(req, @@ -1124,31 +1179,72 @@ NTSTATUS authsam_update_bad_pwd_count(struct ldb_context *sam_ctx, false, NULL); if (ret != LDB_SUCCESS) { talloc_free(req); - goto done; + status = NT_STATUS_INTERNAL_ERROR; + goto error; } - ret = dsdb_autotransaction_request(sam_ctx, req); + /* + * As we're in a transaction, make the ldb request directly + * to avoid the nested transaction that would result if we + * called dsdb_autotransaction_request + */ + ret = ldb_request(sam_ctx, req); + if (ret == LDB_SUCCESS) { + ret = ldb_wait(req->handle, LDB_WAIT_ALL); + } talloc_free(req); - + if (ret != LDB_SUCCESS) { + status = NT_STATUS_INTERNAL_ERROR; + goto error; + } status = authsam_set_bad_password_indicator( sam_ctx, mem_ctx, msg); - /* Failure is ignored for now */ + if (!NT_STATUS_IS_OK(status)) { + goto error; + } } - -done: + /* + * Note that we may not have updated the user record, but + * committing the transaction in that case is still the correct + * thing to do. + * If the transaction was cancelled, this would be logged by + * the dsdb audit log as a failure. When in fact it is expected + * behaviour. + */ +exit: + TALLOC_FREE(mem_ctx); + ret = ldb_transaction_commit(sam_ctx); if (ret != LDB_SUCCESS) { - DBG_ERR("Failed to update badPwdCount, badPasswordTime or " - "set lockoutTime on %s: %s\n", - ldb_dn_get_linearized(msg->dn), - ldb_errstring(sam_ctx)); - TALLOC_FREE(mem_ctx); + DBG_ERR("Error (%d) %s, committing transaction," + " while updating bad password count" + " for (%s)\n", + ret, + ldb_errstring(sam_ctx), + ldb_dn_get_linearized(msg->dn)); return NT_STATUS_INTERNAL_ERROR; } + return status; +error: + DBG_ERR("Failed to update badPwdCount, badPasswordTime or " + "set lockoutTime on %s: %s\n", + ldb_dn_get_linearized(msg->dn), + ldb_errstring(sam_ctx) != NULL ? + ldb_errstring(sam_ctx) :nt_errstr(status)); + if (txn_active) { + ret = ldb_transaction_cancel(sam_ctx); + if (ret != LDB_SUCCESS) { + DBG_ERR("Error rolling back transaction," + " while updating bad password count" + " on %s: %s\n", + ldb_dn_get_linearized(msg->dn), + ldb_errstring(sam_ctx)); + } + } TALLOC_FREE(mem_ctx); - return NT_STATUS_OK; -} + return status; +} static NTSTATUS authsam_update_lastlogon_timestamp(struct ldb_context *sam_ctx, struct ldb_message *msg_mod, @@ -1296,6 +1392,7 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx, NTTIME now; NTTIME lastLogonTimestamp; bool am_rodc = false; + bool txn_active = false; bool need_db_reread; mem_ctx = talloc_new(msg); @@ -1303,15 +1400,41 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx, return NT_STATUS_NO_MEMORY; } + /* + * Any update of the last logon data, needs to be done inside a + * transaction. + * And the user data needs to be re-read, and the account re-checked + * for lockout. + * + * Howevver we have long-running transactions like replication + * that could otherwise grind the system to a halt so we first + * determine if *this* account has seen a bad password, + * otherwise we only start a transaction if there was a need + * (because a change was to be made). + */ + status = authsam_check_bad_password_indicator( sam_ctx, mem_ctx, &need_db_reread, msg); if (!NT_STATUS_IS_OK(status)) { return status; } +get_transaction: + if (need_db_reread) { struct ldb_message *current = NULL; + /* + * Start a new transaction + */ + ret = ldb_transaction_start(sam_ctx); + if (ret != LDB_SUCCESS) { + status = NT_STATUS_INTERNAL_ERROR; + goto error; + } + + txn_active = true; + /* * Re-read the account details, using the GUID * embedded in DN so this is safe against a race where @@ -1324,7 +1447,15 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx, * The re-read can return account locked out, as well * as an internal error */ - return status; + if (NT_STATUS_EQUAL(status, NT_STATUS_ACCOUNT_LOCKED_OUT)) { + /* + * For NT_STATUS_ACCOUNT_LOCKED_OUT we want to commit + * the transaction. Again to avoid cluttering the + * audit logs with spurious errors + */ + goto exit; + } + goto error; } msg = current; } @@ -1345,9 +1476,15 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx, msg_mod = ldb_msg_new(mem_ctx); if (msg_mod == NULL) { - TALLOC_FREE(mem_ctx); - return NT_STATUS_NO_MEMORY; + status = NT_STATUS_NO_MEMORY; + goto error; } + + /* + * By using the DN from msg->dn directly, we allow LDB to + * prefer the embedded GUID form, so this is actually quite + * safe even in the case where DN has been changed + */ msg_mod->dn = msg->dn; if (lockoutTime != 0) { @@ -1356,14 +1493,14 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx, */ ret = samdb_msg_add_int(sam_ctx, msg_mod, msg_mod, "lockoutTime", 0); if (ret != LDB_SUCCESS) { - TALLOC_FREE(mem_ctx); - return NT_STATUS_NO_MEMORY; + status = NT_STATUS_NO_MEMORY; + goto error; } } else if (badPwdCount != 0) { ret = samdb_msg_add_int(sam_ctx, msg_mod, msg_mod, "badPwdCount", 0); if (ret != LDB_SUCCESS) { - TALLOC_FREE(mem_ctx); - return NT_STATUS_NO_MEMORY; + status = NT_STATUS_NO_MEMORY; + goto error; } } @@ -1375,8 +1512,8 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx, ret = samdb_msg_add_int64(sam_ctx, msg_mod, msg_mod, "lastLogon", now); if (ret != LDB_SUCCESS) { - TALLOC_FREE(mem_ctx); - return NT_STATUS_NO_MEMORY; + status = NT_STATUS_NO_MEMORY; + goto error; } } @@ -1390,8 +1527,8 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx, ret = samdb_msg_add_int(sam_ctx, msg_mod, msg_mod, "logonCount", logonCount); if (ret != LDB_SUCCESS) { - TALLOC_FREE(mem_ctx); - return NT_STATUS_NO_MEMORY; + status = NT_STATUS_NO_MEMORY; + goto error; } } else { /* Set an unset logonCount to 0 on first successful login */ @@ -1407,16 +1544,16 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx, ret = samdb_rodc(sam_ctx, &am_rodc); if (ret != LDB_SUCCESS) { - TALLOC_FREE(mem_ctx); - return NT_STATUS_INTERNAL_ERROR; + status = NT_STATUS_INTERNAL_ERROR; + goto error; } if (!am_rodc) { status = authsam_update_lastlogon_timestamp(sam_ctx, msg_mod, domain_dn, lastLogonTimestamp, now); if (!NT_STATUS_IS_OK(status)) { - TALLOC_FREE(mem_ctx); - return NT_STATUS_NO_MEMORY; + status = NT_STATUS_NO_MEMORY; + goto error; } } else { /* Perform the (async) SendToSAM calls for MS-SAMS */ @@ -1436,6 +1573,16 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx, unsigned int i; struct ldb_request *req; + /* + * If it turns out we are going to update the DB, go + * back to the start, get a transaction and the + * current DB state and try again + */ + if (txn_active == false) { + need_db_reread = true; + goto get_transaction; + } + /* mark all the message elements as LDB_FLAG_MOD_REPLACE */ for (i=0;inum_elements;i++) { msg_mod->elements[i].flags = LDB_FLAG_MOD_REPLACE; @@ -1448,35 +1595,84 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx, ldb_op_default_callback, NULL); if (ret != LDB_SUCCESS) { - goto done; + status = NT_STATUS_INTERNAL_ERROR; + goto error; } ret = ldb_request_add_control(req, DSDB_CONTROL_FORCE_RODC_LOCAL_CHANGE, false, NULL); if (ret != LDB_SUCCESS) { - talloc_free(req); - goto done; + TALLOC_FREE(req); + status = NT_STATUS_INTERNAL_ERROR; + goto error; } - - ret = dsdb_autotransaction_request(sam_ctx, req); - talloc_free(req); + /* + * As we're in a transaction, make the ldb request directly + * to avoid the nested transaction that would result if we + * called dsdb_autotransaction_request + */ + ret = ldb_request(sam_ctx, req); + if (ret == LDB_SUCCESS) { + ret = ldb_wait(req->handle, LDB_WAIT_ALL); + } + TALLOC_FREE(req); + if (ret != LDB_SUCCESS) { + status = NT_STATUS_INTERNAL_ERROR; + goto error; + } + } + status = authsam_clear_bad_password_indicator(sam_ctx, mem_ctx, msg); + if (!NT_STATUS_IS_OK(status)) { + goto error; } - status = authsam_clear_bad_password_indicator(sam_ctx, mem_ctx, msg); - /* Failure is ignored for now */ + /* + * Note that we may not have updated the user record, but + * committing the transaction in that case is still the correct + * thing to do. + * If the transaction was cancelled, this would be logged by + * the dsdb audit log as a failure. When in fact it is expected + * behaviour. + * + * Thankfully both TDB and LMDB seem to optimise for the empty + * transaction case + */ +exit: + TALLOC_FREE(mem_ctx); -done: + if (txn_active == false) { + return status; + } + + ret = ldb_transaction_commit(sam_ctx); if (ret != LDB_SUCCESS) { - DEBUG(0, ("Failed to set badPwdCount and lockoutTime " - "to 0 and/or lastlogon to now (%lld) " - "%s: %s\n", (long long int)now, - ldb_dn_get_linearized(msg_mod->dn), - ldb_errstring(sam_ctx))); - TALLOC_FREE(mem_ctx); + DBG_ERR("Error (%d) %s, committing transaction," + " while updating successful logon accounting" + " for (%s)\n", + ret, + ldb_errstring(sam_ctx), + ldb_dn_get_linearized(msg->dn)); return NT_STATUS_INTERNAL_ERROR; } + return status; +error: + DBG_ERR("Failed to update badPwdCount, badPasswordTime or " + "set lockoutTime on %s: %s\n", + ldb_dn_get_linearized(msg->dn), + ldb_errstring(sam_ctx) != NULL ? + ldb_errstring(sam_ctx) :nt_errstr(status)); + if (txn_active) { + ret = ldb_transaction_cancel(sam_ctx); + if (ret != LDB_SUCCESS) { + DBG_ERR("Error rolling back transaction," + " while updating bad password count" + " on %s: %s\n", + ldb_dn_get_linearized(msg->dn), + ldb_errstring(sam_ctx)); + } + } TALLOC_FREE(mem_ctx); - return NT_STATUS_OK; + return status; } -- 2.35.0 From 923a56a79f71179737bf5fd6f4a016d4d4c2af47 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 30 Mar 2021 16:35:44 +1300 Subject: [PATCH 11/21] CVE-2021-20251 auth4: Add missing newline to debug message on PSO read failure BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Andrew Bartlett Reviewed-by: Joseph Sutton --- source4/auth/sam.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source4/auth/sam.c b/source4/auth/sam.c index cd81efe81c4..d672706d6c6 100644 --- a/source4/auth/sam.c +++ b/source4/auth/sam.c @@ -1096,7 +1096,7 @@ NTSTATUS authsam_update_bad_pwd_count(struct ldb_context *sam_ctx, * fallback to using the domain defaults so that we still * record the bad password attempt */ - DBG_ERR("Error (%d) checking PSO for %s", + DBG_ERR("Error (%d) checking PSO for %s\n", ret, ldb_dn_get_linearized(msg->dn)); } -- 2.35.0 From 56df3a37413f64cccca9a297de3cc3e85ddb3607 Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Thu, 25 Mar 2021 11:30:59 +1300 Subject: [PATCH 12/21] CVE-2021-20251 auth4: Return only the result message and free the surrounding result BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Andrew Bartlett Reviewed-by: Joseph Sutton --- selftest/knownfail.d/auth-sam | 1 - source4/auth/sam.c | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/selftest/knownfail.d/auth-sam b/selftest/knownfail.d/auth-sam index aecca4ae088..8f7c915289c 100644 --- a/selftest/knownfail.d/auth-sam +++ b/selftest/knownfail.d/auth-sam @@ -1,4 +1,3 @@ -^samba.unittests.auth.sam.test_reread_account_not_locked.none ^samba.unittests.auth.sam.test_success_accounting_add_control_failed.none ^samba.unittests.auth.sam.test_success_accounting_build_mod_req_failed.none ^samba.unittests.auth.sam.test_success_accounting_commit_failed.none diff --git a/source4/auth/sam.c b/source4/auth/sam.c index d672706d6c6..e4e88405c40 100644 --- a/source4/auth/sam.c +++ b/source4/auth/sam.c @@ -885,7 +885,8 @@ NTSTATUS authsam_reread_user_logon_data( TALLOC_FREE(res); return NT_STATUS_ACCOUNT_LOCKED_OUT; } - *current = res->msgs[0]; + *current = talloc_steal(mem_ctx, res->msgs[0]); + TALLOC_FREE(res); return NT_STATUS_OK; } -- 2.35.0 From 90cd902c8020aaeb37d453851896d20541a1d1f1 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 25 Mar 2021 14:42:39 +1300 Subject: [PATCH 13/21] CVE-2021-20251 auth4: Split authsam_calculate_lastlogon_sync_interval() out authsam_calculate_lastlogon_sync_interval() is split out of authsam_update_lastlogon_timestamp() Based on work by Gary Lockyer BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Andrew Bartlett Reviewed-by: Joseph Sutton --- source4/auth/sam.c | 115 ++++++++++++++++++++++++++++----------------- 1 file changed, 73 insertions(+), 42 deletions(-) diff --git a/source4/auth/sam.c b/source4/auth/sam.c index e4e88405c40..eb22cd8d2a3 100644 --- a/source4/auth/sam.c +++ b/source4/auth/sam.c @@ -1247,40 +1247,36 @@ error: } -static NTSTATUS authsam_update_lastlogon_timestamp(struct ldb_context *sam_ctx, - struct ldb_message *msg_mod, - struct ldb_dn *domain_dn, - NTTIME old_timestamp, - NTTIME now) +/* + * msDS-LogonTimeSyncInterval is an int32_t number of days. + * + * The docs say: "the initial update, after the domain functional + * level is raised to DS_BEHAVIOR_WIN2003 or higher, is calculated as + * 14 days minus a random percentage of 5 days", but we aren't doing + * that. The blogosphere seems to think that this randomised update + * happens everytime, but [MS-ADA1] doesn't agree. + * + * Dochelp referred us to the following blog post: + * http://blogs.technet.com/b/askds/archive/2009/04/15/the-lastlogontimestamp-attribute-what-it-was-designed-for-and-how-it-works.aspx + * + * when msDS-LogonTimeSyncInterval is zero, the lastLogonTimestamp is + * not changed. + */ + +static NTSTATUS authsam_calculate_lastlogon_sync_interval( + struct ldb_context *sam_ctx, + TALLOC_CTX *ctx, + struct ldb_dn *domain_dn, + NTTIME *sync_interval_nt) { - /* - * We only set lastLogonTimestamp if the current value is older than - * now - msDS-LogonTimeSyncInterval days. - * - * msDS-LogonTimeSyncInterval is an int32_t number of days, while - * lastLogonTimestamp is in the 64 bit 100ns NTTIME format. - * - * The docs say: "the initial update, after the domain functional - * level is raised to DS_BEHAVIOR_WIN2003 or higher, is calculated as - * 14 days minus a random percentage of 5 days", but we aren't doing - * that. The blogosphere seems to think that this randomised update - * happens everytime, but [MS-ADA1] doesn't agree. - * - * Dochelp referred us to the following blog post: - * http://blogs.technet.com/b/askds/archive/2009/04/15/the-lastlogontimestamp-attribute-what-it-was-designed-for-and-how-it-works.aspx - * - * en msDS-LogonTimeSyncInterval is zero, the lastLogonTimestamp is - * not changed. - */ static const char *attrs[] = { "msDS-LogonTimeSyncInterval", NULL }; int ret; struct ldb_result *domain_res = NULL; TALLOC_CTX *mem_ctx = NULL; - int32_t sync_interval; - NTTIME sync_interval_nt; + uint32_t sync_interval; - mem_ctx = talloc_new(msg_mod); + mem_ctx = talloc_new(ctx); if (mem_ctx == NULL) { return NT_STATUS_NO_MEMORY; } @@ -1296,15 +1292,7 @@ static NTSTATUS authsam_update_lastlogon_timestamp(struct ldb_context *sam_ctx, "msDS-LogonTimeSyncInterval", 14); DEBUG(5, ("sync interval is %d\n", sync_interval)); - if (sync_interval == 0){ - /* - * Setting msDS-LogonTimeSyncInterval to zero is how you ask - * that nothing happens here. - */ - TALLOC_FREE(mem_ctx); - return NT_STATUS_OK; - } - else if (sync_interval >= 5){ + if (sync_interval >= 5){ /* * Subtract "a random percentage of 5" days. Presumably this * percentage is between 0 and 100, and modulus is accurate @@ -1312,17 +1300,47 @@ static NTSTATUS authsam_update_lastlogon_timestamp(struct ldb_context *sam_ctx, */ uint32_t r = generate_random() % 6; sync_interval -= r; - DEBUG(5, ("randomised sync interval is %d (-%d)\n", sync_interval, r)); + DBG_INFO("randomised sync interval is %d (-%d)\n", sync_interval, r); } /* In the case where sync_interval < 5 there is no randomisation */ - sync_interval_nt = sync_interval * 24LL * 3600LL * 10000000LL; + /* + * msDS-LogonTimeSyncInterval is an int32_t number of days, + * while lastLogonTimestamp (to be updated) is in the 64 bit + * 100ns NTTIME format so we must convert. + */ + *sync_interval_nt = sync_interval * 24LL * 3600LL * 10000000LL; + TALLOC_FREE(mem_ctx); + return NT_STATUS_OK; +} + +/* + * We only set lastLogonTimestamp if the current value is older than + * now - msDS-LogonTimeSyncInterval days. + * + * lastLogonTimestamp is in the 64 bit 100ns NTTIME format + */ +static NTSTATUS authsam_update_lastlogon_timestamp(struct ldb_context *sam_ctx, + struct ldb_message *msg_mod, + struct ldb_dn *domain_dn, + NTTIME old_timestamp, + NTTIME now, + NTTIME sync_interval_nt) +{ + int ret; DEBUG(5, ("old timestamp is %lld, threshold %lld, diff %lld\n", (long long int)old_timestamp, (long long int)(now - sync_interval_nt), (long long int)(old_timestamp - now + sync_interval_nt))); + if (sync_interval_nt == 0){ + /* + * Setting msDS-LogonTimeSyncInterval to zero is how you ask + * that nothing happens here. + */ + return NT_STATUS_OK; + } if (old_timestamp > now){ DEBUG(0, ("lastLogonTimestamp is in the future! (%lld > %lld)\n", (long long int)old_timestamp, (long long int)now)); @@ -1337,11 +1355,9 @@ static NTSTATUS authsam_update_lastlogon_timestamp(struct ldb_context *sam_ctx, "lastLogonTimestamp", now); if (ret != LDB_SUCCESS) { - TALLOC_FREE(mem_ctx); return NT_STATUS_NO_MEMORY; } } - TALLOC_FREE(mem_ctx); return NT_STATUS_OK; } @@ -1550,8 +1566,23 @@ get_transaction: } if (!am_rodc) { - status = authsam_update_lastlogon_timestamp(sam_ctx, msg_mod, domain_dn, - lastLogonTimestamp, now); + NTTIME sync_interval_nt; + + status = authsam_calculate_lastlogon_sync_interval( + sam_ctx, mem_ctx, domain_dn, &sync_interval_nt); + + if (!NT_STATUS_IS_OK(status)) { + status = NT_STATUS_INTERNAL_ERROR; + goto error; + } + + status = authsam_update_lastlogon_timestamp( + sam_ctx, + msg_mod, + domain_dn, + lastLogonTimestamp, + now, + sync_interval_nt); if (!NT_STATUS_IS_OK(status)) { status = NT_STATUS_NO_MEMORY; goto error; -- 2.35.0 From 002e68e7db1f12fdd561ab7d003fef3d49979d73 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 25 Mar 2021 15:33:08 +1300 Subject: [PATCH 14/21] CVE-2021-20251 auth4: Inline samdb_result_effective_badPwdCount() in authsam_logon_success_accounting() By bringing this function inline it can then be split out in a subsequent commit. Based on work by Gary Lockyer BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Andrew Bartlett Reviewed-by: Joseph Sutton --- source4/auth/sam.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/source4/auth/sam.c b/source4/auth/sam.c index eb22cd8d2a3..1551638f09a 100644 --- a/source4/auth/sam.c +++ b/source4/auth/sam.c @@ -1479,11 +1479,17 @@ get_transaction: lockoutTime = ldb_msg_find_attr_as_int64(msg, "lockoutTime", 0); dbBadPwdCount = ldb_msg_find_attr_as_int(msg, "badPwdCount", 0); + tv_now = timeval_current(); + now = timeval_to_nttime(&tv_now); + if (interactive_or_kerberos) { badPwdCount = dbBadPwdCount; } else { - badPwdCount = samdb_result_effective_badPwdCount(sam_ctx, mem_ctx, - domain_dn, msg); + int64_t lockOutObservationWindow = + samdb_result_msds_LockoutObservationWindow( + sam_ctx, mem_ctx, domain_dn, msg); + badPwdCount = dsdb_effective_badPwdCount( + msg, lockOutObservationWindow, now); } lastLogonTimestamp = ldb_msg_find_attr_as_int64(msg, "lastLogonTimestamp", 0); @@ -1521,9 +1527,6 @@ get_transaction: } } - tv_now = timeval_current(); - now = timeval_to_nttime(&tv_now); - if (interactive_or_kerberos || (badPwdCount != 0 && lockoutTime == 0)) { ret = samdb_msg_add_int64(sam_ctx, msg_mod, msg_mod, -- 2.35.0 From 72a657d0bed72e7befbb41cef4f7c762290a1b51 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 30 Mar 2021 16:48:31 +1300 Subject: [PATCH 15/21] CVE-2021-20251 auth4: Avoid reading the database twice by precaculating some variables These variables are not important to protect against a race with and a double-read can easily be avoided by moving them up the file a little. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Andrew Bartlett Reviewed-by: Joseph Sutton --- selftest/knownfail.d/auth-sam | 11 ------- source4/auth/sam.c | 55 +++++++++++++++++++++++------------ 2 files changed, 36 insertions(+), 30 deletions(-) delete mode 100644 selftest/knownfail.d/auth-sam diff --git a/selftest/knownfail.d/auth-sam b/selftest/knownfail.d/auth-sam deleted file mode 100644 index 8f7c915289c..00000000000 --- a/selftest/knownfail.d/auth-sam +++ /dev/null @@ -1,11 +0,0 @@ -^samba.unittests.auth.sam.test_success_accounting_add_control_failed.none -^samba.unittests.auth.sam.test_success_accounting_build_mod_req_failed.none -^samba.unittests.auth.sam.test_success_accounting_commit_failed.none -^samba.unittests.auth.sam.test_success_accounting_ldb_msg_new_failed.none -^samba.unittests.auth.sam.test_success_accounting_ldb_request_failed.none -^samba.unittests.auth.sam.test_success_accounting_ldb_wait_failed.none -^samba.unittests.auth.sam.test_success_accounting_reread_failed.none -^samba.unittests.auth.sam.test_success_accounting_rollback_failed.none -^samba.unittests.auth.sam.test_success_accounting_samdb_rodc_failed.none -^samba.unittests.auth.sam.test_success_accounting_start_txn_failed.none -^samba.unittests.auth.sam.test_success_accounting_update_lastlogon_failed.none diff --git a/source4/auth/sam.c b/source4/auth/sam.c index 1551638f09a..7bb59d225a4 100644 --- a/source4/auth/sam.c +++ b/source4/auth/sam.c @@ -1408,6 +1408,8 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx, struct timeval tv_now; NTTIME now; NTTIME lastLogonTimestamp; + int64_t lockOutObservationWindow; + NTTIME sync_interval_nt = 0; bool am_rodc = false; bool txn_active = false; bool need_db_reread; @@ -1436,6 +1438,36 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx, return status; } + if (interactive_or_kerberos == false) { + /* + * Avoid calculating this twice, it reads the PSO. A + * race on this is unimportant. + */ + lockOutObservationWindow + = samdb_result_msds_LockoutObservationWindow( + sam_ctx, mem_ctx, domain_dn, msg); + } + + ret = samdb_rodc(sam_ctx, &am_rodc); + if (ret != LDB_SUCCESS) { + status = NT_STATUS_INTERNAL_ERROR; + goto error; + } + + if (!am_rodc) { + /* + * Avoid reading the main domain DN twice. A race on + * this is unimportant. + */ + status = authsam_calculate_lastlogon_sync_interval( + sam_ctx, mem_ctx, domain_dn, &sync_interval_nt); + + if (!NT_STATUS_IS_OK(status)) { + status = NT_STATUS_INTERNAL_ERROR; + goto error; + } + } + get_transaction: if (need_db_reread) { @@ -1485,9 +1517,10 @@ get_transaction: if (interactive_or_kerberos) { badPwdCount = dbBadPwdCount; } else { - int64_t lockOutObservationWindow = - samdb_result_msds_LockoutObservationWindow( - sam_ctx, mem_ctx, domain_dn, msg); + /* + * We get lockOutObservationWindow above, before the + * transaction + */ badPwdCount = dsdb_effective_badPwdCount( msg, lockOutObservationWindow, now); } @@ -1562,23 +1595,7 @@ get_transaction: } } - ret = samdb_rodc(sam_ctx, &am_rodc); - if (ret != LDB_SUCCESS) { - status = NT_STATUS_INTERNAL_ERROR; - goto error; - } - if (!am_rodc) { - NTTIME sync_interval_nt; - - status = authsam_calculate_lastlogon_sync_interval( - sam_ctx, mem_ctx, domain_dn, &sync_interval_nt); - - if (!NT_STATUS_IS_OK(status)) { - status = NT_STATUS_INTERNAL_ERROR; - goto error; - } - status = authsam_update_lastlogon_timestamp( sam_ctx, msg_mod, -- 2.35.0 From 8b4b5d2a44d936eb53a84615c221910b5b8eea89 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 6 Jul 2022 11:11:43 +1200 Subject: [PATCH 16/21] CVE-2021-20251 s4:kdc: Move logon success accounting code into existing branch This simplifies the code for the following commit. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Joseph Sutton --- source4/kdc/hdb-samba4.c | 33 +++++++++++++-------------------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/source4/kdc/hdb-samba4.c b/source4/kdc/hdb-samba4.c index 480d2c06e5e..b84750c2fba 100644 --- a/source4/kdc/hdb-samba4.c +++ b/source4/kdc/hdb-samba4.c @@ -552,26 +552,6 @@ static krb5_error_code hdb_samba4_audit(krb5_context context, } switch (hdb_auth_status) { - case KDC_AUTH_EVENT_CLIENT_AUTHORIZED: - { - TALLOC_CTX *frame = talloc_stackframe(); - struct samba_kdc_entry *p = talloc_get_type(entry->context, - struct samba_kdc_entry); - struct netr_SendToSamBase *send_to_sam = NULL; - - /* - * TODO: We could log the AS-REQ authorization success here as - * well. However before we do that, we need to pass - * in the PAC here or re-calculate it. - */ - authsam_logon_success_accounting(kdc_db_ctx->samdb, p->msg, - domain_dn, true, &send_to_sam); - if (kdc_db_ctx->rodc && send_to_sam != NULL) { - reset_bad_password_netlogon(frame, kdc_db_ctx, send_to_sam); - } - talloc_free(frame); - } - FALL_THROUGH; default: { TALLOC_CTX *frame = talloc_stackframe(); @@ -613,6 +593,19 @@ static krb5_error_code hdb_samba4_audit(krb5_context context, ui.auth_description = auth_description; if (hdb_auth_status == KDC_AUTH_EVENT_CLIENT_AUTHORIZED) { + struct netr_SendToSamBase *send_to_sam = NULL; + + /* + * TODO: We could log the AS-REQ authorization success here as + * well. However before we do that, we need to pass + * in the PAC here or re-calculate it. + */ + authsam_logon_success_accounting(kdc_db_ctx->samdb, p->msg, + domain_dn, true, &send_to_sam); + if (kdc_db_ctx->rodc && send_to_sam != NULL) { + reset_bad_password_netlogon(frame, kdc_db_ctx, send_to_sam); + } + /* This is the final sucess */ status = NT_STATUS_OK; } else if (hdb_auth_status == KDC_AUTH_EVENT_VALIDATED_LONG_TERM_KEY) { -- 2.35.0 From 4090f0ae88e150279d2a4b6876a8d4727ce0ef46 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Fri, 1 Jul 2022 15:04:41 +1200 Subject: [PATCH 17/21] CVE-2021-20251 s4:kdc: Check return status of authsam_logon_success_accounting() If we find that the user has been locked out sometime during the request (due to a race), we will now return an error code. Note that we cannot avoid the MIT KDC aspect of the issue by checking the return status of mit_samba_zero_bad_password_count(), because kdb_vftabl::audit_as_req() returning void means we cannot pass on the result. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Joseph Sutton --- selftest/knownfail_heimdal_kdc | 4 ---- source4/kdc/hdb-samba4.c | 15 +++++++++++---- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc index deb84e63a19..4ae27eacb09 100644 --- a/selftest/knownfail_heimdal_kdc +++ b/selftest/knownfail_heimdal_kdc @@ -54,7 +54,3 @@ ^samba.tests.krb5.protected_users_tests.samba.tests.krb5.protected_users_tests.ProtectedUsersTests.test_proxiable_as_protected.ad_dc # ^samba.tests.krb5.protected_users_tests.samba.tests.krb5.protected_users_tests.ProtectedUsersTests.test_samr_change_password_protected.ad_dc -# -# Lockout tests -# -^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_transaction_kdc.ad_dc:local diff --git a/source4/kdc/hdb-samba4.c b/source4/kdc/hdb-samba4.c index b84750c2fba..22922a23834 100644 --- a/source4/kdc/hdb-samba4.c +++ b/source4/kdc/hdb-samba4.c @@ -600,14 +600,21 @@ static krb5_error_code hdb_samba4_audit(krb5_context context, * well. However before we do that, we need to pass * in the PAC here or re-calculate it. */ - authsam_logon_success_accounting(kdc_db_ctx->samdb, p->msg, - domain_dn, true, &send_to_sam); - if (kdc_db_ctx->rodc && send_to_sam != NULL) { + status = authsam_logon_success_accounting(kdc_db_ctx->samdb, p->msg, + domain_dn, true, &send_to_sam); + if (NT_STATUS_EQUAL(status, NT_STATUS_ACCOUNT_LOCKED_OUT)) { + final_ret = KRB5KDC_ERR_CLIENT_REVOKED; + r->error_code = final_ret; + rwdc_fallback = kdc_db_ctx->rodc; + } else if (!NT_STATUS_IS_OK(status)) { + final_ret = KRB5KRB_ERR_GENERIC; + r->error_code = final_ret; + rwdc_fallback = kdc_db_ctx->rodc; + } else if (kdc_db_ctx->rodc && send_to_sam != NULL) { reset_bad_password_netlogon(frame, kdc_db_ctx, send_to_sam); } /* This is the final sucess */ - status = NT_STATUS_OK; } else if (hdb_auth_status == KDC_AUTH_EVENT_VALIDATED_LONG_TERM_KEY) { /* * This was only a pre-authentication success, -- 2.35.0 From 044354332443f779162f968e53d1bc2ae8899bc8 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Mon, 4 Jul 2022 20:51:38 +1200 Subject: [PATCH 18/21] CVE-2021-20251 s4:auth_winbind: Check return status of authsam_logon_success_accounting() This may return an error if we find the account is locked out. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Joseph Sutton --- source4/auth/ntlm/auth_winbind.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/source4/auth/ntlm/auth_winbind.c b/source4/auth/ntlm/auth_winbind.c index d7879966603..6381f866667 100644 --- a/source4/auth/ntlm/auth_winbind.c +++ b/source4/auth/ntlm/auth_winbind.c @@ -252,11 +252,14 @@ static void winbind_check_password_done(struct tevent_req *subreq) status = authsam_search_account(state, ctx->auth_ctx->sam_ctx, nt4_account, domain_dn, &msg); if (NT_STATUS_IS_OK(status)) { - authsam_logon_success_accounting( + status = authsam_logon_success_accounting( ctx->auth_ctx->sam_ctx, msg, domain_dn, user_info->flags & USER_INFO_INTERACTIVE_LOGON, NULL); + if (tevent_req_nterror(req, status)) { + return; + } } } -- 2.35.0 From 797d6ab80182da894d98ae9f2f3b4a6d2447af52 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 11 Jan 2021 12:11:35 -0800 Subject: [PATCH 19/21] CVE-2021-20251 s3: ensure bad password count atomic updates The bad password count is supposed to limit the number of failed login attempt a user can make before being temporarily locked out, but race conditions between processes have allowed determined attackers to make many more than the specified number of attempts. This is especially bad on constrained or overcommitted hardware. To fix this, once a bad password is detected, we reload the sam account information under a user-specific mutex, ensuring we have an up to date bad password count. Discovered by Nathaniel W. Turner. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Jeremy Allison --- source3/auth/check_samsec.c | 77 +++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/source3/auth/check_samsec.c b/source3/auth/check_samsec.c index b9563c958a9..bd8ca8de2f0 100644 --- a/source3/auth/check_samsec.c +++ b/source3/auth/check_samsec.c @@ -379,6 +379,8 @@ NTSTATUS check_sam_security(const DATA_BLOB *challenge, const uint8_t *nt_pw; const uint8_t *lm_pw; uint32_t acct_ctrl; + char *mutex_name_by_user = NULL; + struct named_mutex *mtx = NULL; /* the returned struct gets kept on the server_info, by means of a steal further down */ @@ -418,6 +420,79 @@ NTSTATUS check_sam_security(const DATA_BLOB *challenge, challenge, lm_pw, nt_pw, user_info, &user_sess_key, &lm_sess_key); + /* + * We must re-load the sam acount information under a mutex + * lock to ensure we don't miss any concurrent account lockout + * changes. + */ + + /* Clear out old sampass info. */ + TALLOC_FREE(sampass); + acct_ctrl = 0; + username = NULL; + nt_pw = NULL; + lm_pw = NULL; + + sampass = samu_new(mem_ctx); + if (sampass == NULL) { + return NT_STATUS_NO_MEMORY; + } + + mutex_name_by_user = talloc_asprintf(mem_ctx, + "check_sam_security_mutex_%s", + user_info->mapped.account_name); + if (mutex_name_by_user == NULL) { + nt_status = NT_STATUS_NO_MEMORY; + goto done; + } + + /* Grab the named mutex under root with 30 second timeout. */ + become_root(); + mtx = grab_named_mutex(mem_ctx, mutex_name_by_user, 30); + if (mtx != NULL) { + /* Re-load the account information if we got the mutex. */ + ret = pdb_getsampwnam(sampass, user_info->mapped.account_name); + } + unbecome_root(); + + /* Everything from here on until mtx is freed is done under the mutex.*/ + + if (mtx == NULL) { + DBG_ERR("Acquisition of mutex %s failed " + "for user %s\n", + mutex_name_by_user, + user_info->mapped.account_name); + nt_status = NT_STATUS_INTERNAL_ERROR; + goto done; + } + + if (!ret) { + /* + * Re-load of account failed. This could only happen if the + * user was deleted in the meantime. + */ + DBG_NOTICE("reload of user '%s' in passdb failed.\n", + user_info->mapped.account_name); + nt_status = NT_STATUS_NO_SUCH_USER; + goto done; + } + + /* Re-load the account control info. */ + acct_ctrl = pdb_get_acct_ctrl(sampass); + username = pdb_get_username(sampass); + + /* + * Check if the account is now locked out - now under the mutex. + * This can happen if the server is under + * a password guess attack and the ACB_AUTOLOCK is set by + * another process. + */ + if (acct_ctrl & ACB_AUTOLOCK) { + DBG_NOTICE("Account for user %s was locked out.\n", username); + nt_status = NT_STATUS_ACCOUNT_LOCKED_OUT; + goto done; + } + /* Notify passdb backend of login success/failure. If not NT_STATUS_OK the backend doesn't like the login */ @@ -510,6 +585,8 @@ done: TALLOC_FREE(sampass); data_blob_free(&user_sess_key); data_blob_free(&lm_sess_key); + TALLOC_FREE(mutex_name_by_user); + TALLOC_FREE(mtx); return nt_status; } -- 2.35.0 From c98a6f8d608a7ffc285461048b676de07bdb0f4c Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 5 Jul 2022 20:17:33 +1200 Subject: [PATCH 20/21] CVE-2021-20251 s3: Ensure bad password count atomic updates for SAMR password change The bad password count is supposed to limit the number of failed login attempt a user can make before being temporarily locked out, but race conditions between processes have allowed determined attackers to make many more than the specified number of attempts. This is especially bad on constrained or overcommitted hardware. To fix this, once a bad password is detected, we reload the sam account information under a user-specific mutex, ensuring we have an up to date bad password count. Derived from a similar patch to source3/auth/check_samsec.c by Jeremy Allison BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Joseph Sutton --- source3/rpc_server/samr/srv_samr_chgpasswd.c | 81 ++++++++++++++++++-- 1 file changed, 75 insertions(+), 6 deletions(-) diff --git a/source3/rpc_server/samr/srv_samr_chgpasswd.c b/source3/rpc_server/samr/srv_samr_chgpasswd.c index b5cd308a119..6e99c0f0a9c 100644 --- a/source3/rpc_server/samr/srv_samr_chgpasswd.c +++ b/source3/rpc_server/samr/srv_samr_chgpasswd.c @@ -1201,6 +1201,8 @@ NTSTATUS pass_oem_change(char *user, const char *rhost, bool ret = false; bool updated_badpw = false; NTSTATUS update_login_attempts_status; + char *mutex_name_by_user = NULL; + struct named_mutex *mtx = NULL; if (!(sampass = samu_new(NULL))) { return NT_STATUS_NO_MEMORY; @@ -1212,15 +1214,15 @@ NTSTATUS pass_oem_change(char *user, const char *rhost, if (ret == false) { DEBUG(0,("pass_oem_change: getsmbpwnam returned NULL\n")); - TALLOC_FREE(sampass); - return NT_STATUS_NO_SUCH_USER; + nt_status = NT_STATUS_NO_SUCH_USER; + goto done; } /* Quit if the account was locked out. */ if (pdb_get_acct_ctrl(sampass) & ACB_AUTOLOCK) { DEBUG(3,("check_sam_security: Account for user %s was locked out.\n", user)); - TALLOC_FREE(sampass); - return NT_STATUS_ACCOUNT_LOCKED_OUT; + nt_status = NT_STATUS_ACCOUNT_LOCKED_OUT; + goto done; } nt_status = check_oem_password(user, @@ -1231,6 +1233,71 @@ NTSTATUS pass_oem_change(char *user, const char *rhost, sampass, &new_passwd); + /* + * We must re-load the sam acount information under a mutex + * lock to ensure we don't miss any concurrent account lockout + * changes. + */ + + /* Clear out old sampass info. */ + TALLOC_FREE(sampass); + + sampass = samu_new(NULL); + if (sampass == NULL) { + return NT_STATUS_NO_MEMORY; + } + + mutex_name_by_user = talloc_asprintf(NULL, + "check_sam_security_mutex_%s", + user); + if (mutex_name_by_user == NULL) { + nt_status = NT_STATUS_NO_MEMORY; + goto done; + } + + /* Grab the named mutex under root with 30 second timeout. */ + become_root(); + mtx = grab_named_mutex(NULL, mutex_name_by_user, 30); + if (mtx != NULL) { + /* Re-load the account information if we got the mutex. */ + ret = pdb_getsampwnam(sampass, user); + } + unbecome_root(); + + /* Everything from here on until mtx is freed is done under the mutex.*/ + + if (mtx == NULL) { + DBG_ERR("Acquisition of mutex %s failed " + "for user %s\n", + mutex_name_by_user, + user); + nt_status = NT_STATUS_INTERNAL_ERROR; + goto done; + } + + if (!ret) { + /* + * Re-load of account failed. This could only happen if the + * user was deleted in the meantime. + */ + DBG_NOTICE("reload of user '%s' in passdb failed.\n", + user); + nt_status = NT_STATUS_NO_SUCH_USER; + goto done; + } + + /* + * Check if the account is now locked out - now under the mutex. + * This can happen if the server is under + * a password guess attack and the ACB_AUTOLOCK is set by + * another process. + */ + if (pdb_get_acct_ctrl(sampass) & ACB_AUTOLOCK) { + DBG_NOTICE("Account for user %s was locked out.\n", user); + nt_status = NT_STATUS_ACCOUNT_LOCKED_OUT; + goto done; + } + /* * Notify passdb backend of login success/failure. If not * NT_STATUS_OK the backend doesn't like the login @@ -1278,8 +1345,7 @@ NTSTATUS pass_oem_change(char *user, const char *rhost, } if (!NT_STATUS_IS_OK(nt_status)) { - TALLOC_FREE(sampass); - return nt_status; + goto done; } /* We've already checked the old password here.... */ @@ -1290,7 +1356,10 @@ NTSTATUS pass_oem_change(char *user, const char *rhost, memset(new_passwd, 0, strlen(new_passwd)); +done: TALLOC_FREE(sampass); + TALLOC_FREE(mutex_name_by_user); + TALLOC_FREE(mtx); return nt_status; } -- 2.35.0 From 3d301049d1ea0465e7e2d920eca7fc1bdd4523cc Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 5 Jul 2022 20:18:01 +1200 Subject: [PATCH 21/21] CVE-2021-20251 s4-rpc_server: Remove transaction logic from SAMR password change samdb_set_password() also starts a transaction, so doing it here is unecessary and only results in a nested transaction. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Joseph Sutton --- source4/rpc_server/samr/samr_password.c | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/source4/rpc_server/samr/samr_password.c b/source4/rpc_server/samr/samr_password.c index 70f969ffdf0..daccb02d00d 100644 --- a/source4/rpc_server/samr/samr_password.c +++ b/source4/rpc_server/samr/samr_password.c @@ -123,7 +123,6 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, DATA_BLOB new_password; struct ldb_context *sam_ctx = NULL; struct ldb_dn *user_dn = NULL; - int ret; struct ldb_message *msg = NULL; struct samr_Password *nt_pwd; struct samr_DomInfo1 *dominfo = NULL; @@ -245,12 +244,6 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, return NT_STATUS_INVALID_SYSTEM_SERVICE; } - ret = ldb_transaction_start(sam_ctx); - if (ret != LDB_SUCCESS) { - DEBUG(1, ("Failed to start transaction: %s\n", ldb_errstring(sam_ctx))); - return NT_STATUS_TRANSACTION_ABORTED; - } - /* Performs the password modification. We pass the old hashes read out * from the database since they were already checked against the user- * provided ones. */ @@ -262,23 +255,6 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, &reason, &dominfo); - if (!NT_STATUS_IS_OK(status)) { - ldb_transaction_cancel(sam_ctx); - goto failed; - } - - /* And this confirms it in a transaction commit */ - ret = ldb_transaction_commit(sam_ctx); - if (ret != LDB_SUCCESS) { - DEBUG(1,("Failed to commit transaction to change password on %s: %s\n", - ldb_dn_get_linearized(user_dn), - ldb_errstring(sam_ctx))); - status = NT_STATUS_TRANSACTION_ABORTED; - goto failed; - } - - status = NT_STATUS_OK; - failed: log_password_change_event(imsg_ctx, -- 2.35.0