From 93d07bf6e418b1a349228e8e60e6dbaa456e5b83 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Fri, 29 Sep 2023 13:13:01 +1300 Subject: [PATCH 1/8] =?UTF-8?q?tests/krb5:=20Add=20method=20to=20perform?= =?UTF-8?q?=20an=20armored=20AS=E2=80=90REQ?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Joseph Sutton Reviewed-by: Andrew Bartlett (cherry picked from commit 849ee959845832b206ae315ab5911c623ea61148) --- python/samba/tests/krb5/kdc_tgs_tests.py | 117 +++++++++++++++++++++++ python/samba/tests/krb5/raw_testcase.py | 2 + 2 files changed, 119 insertions(+) diff --git a/python/samba/tests/krb5/kdc_tgs_tests.py b/python/samba/tests/krb5/kdc_tgs_tests.py index 27c7ee38cc640..aa2d132289c88 100755 --- a/python/samba/tests/krb5/kdc_tgs_tests.py +++ b/python/samba/tests/krb5/kdc_tgs_tests.py @@ -36,6 +36,7 @@ from samba.tests.krb5.raw_testcase import Krb5EncryptionKey from samba.tests.krb5.rfc4120_constants import ( AES256_CTS_HMAC_SHA1_96, ARCFOUR_HMAC_MD5, + FX_FAST_ARMOR_AP_REQUEST, KRB_ERROR, KDC_ERR_BADKEYVER, KDC_ERR_BADMATCH, @@ -169,6 +170,122 @@ class KdcTgsBaseTests(KDCBaseTest): self.check_as_reply(rep) return kdc_exchange_dict['rep_ticket_creds'] + def _armored_as_req(self, + client_creds, + target_creds, + armor_tgt, + *, + expected_error=0, + expected_sname=None, + expect_edata=None, + expect_status=None, + expected_status=None, + expected_groups=None, + expect_device_info=None, + expected_device_groups=None, + expect_device_claims=None, + expected_device_claims=None): + client_username = client_creds.get_username() + client_realm = client_creds.get_realm() + client_cname = self.PrincipalName_create(name_type=NT_PRINCIPAL, + names=[client_username]) + + target_name = target_creds.get_username() + target_sname = self.PrincipalName_create( + name_type=NT_PRINCIPAL, names=[target_name]) + target_realm = target_creds.get_realm() + target_decryption_key = self.TicketDecryptionKey_from_creds( + target_creds) + target_etypes = target_creds.tgs_supported_enctypes + + authenticator_subkey = self.RandomKey(kcrypto.Enctype.AES256) + armor_key = self.generate_armor_key(authenticator_subkey, + armor_tgt.session_key) + + preauth_key = self.PasswordKey_from_creds(client_creds, + kcrypto.Enctype.AES256) + + client_challenge_key = ( + self.generate_client_challenge_key(armor_key, preauth_key)) + fast_padata = [self.get_challenge_pa_data(client_challenge_key)] + + def _generate_fast_padata(kdc_exchange_dict, + _callback_dict, + req_body): + return list(fast_padata), req_body + + etypes = kcrypto.Enctype.AES256, kcrypto.Enctype.RC4 + + if expected_error: + check_error_fn = self.generic_check_kdc_error + check_rep_fn = None + else: + check_error_fn = None + check_rep_fn = self.generic_check_kdc_rep + + pac_options = '1' # claims support + + samdb = self.get_samdb() + domain_sid_str = samdb.get_domain_sid() + + if expected_groups is not None: + expected_groups = self.map_sids(expected_groups, None, domain_sid_str) + + if expected_device_groups is not None: + expected_device_groups = self.map_sids(expected_device_groups, None, domain_sid_str) + + if expected_sname is None: + expected_sname = target_sname + + kdc_exchange_dict = self.as_exchange_dict( + creds=client_creds, + expected_crealm=client_realm, + expected_cname=client_cname, + expected_srealm=target_realm, + expected_sname=expected_sname, + expected_supported_etypes=target_etypes, + ticket_decryption_key=target_decryption_key, + generate_fast_fn=self.generate_simple_fast, + generate_fast_armor_fn=self.generate_ap_req, + generate_fast_padata_fn=_generate_fast_padata, + fast_armor_type=FX_FAST_ARMOR_AP_REQUEST, + check_error_fn=check_error_fn, + check_rep_fn=check_rep_fn, + check_kdc_private_fn=self.generic_check_kdc_private, + expected_error_mode=expected_error, + expected_salt=client_creds.get_salt(), + expect_edata=expect_edata, + expect_status=expect_status, + expected_status=expected_status, + expected_groups=expected_groups, + expect_device_info=expect_device_info, + expected_device_domain_sid=domain_sid_str, + expected_device_groups=expected_device_groups, + expect_device_claims=expect_device_claims, + expected_device_claims=expected_device_claims, + authenticator_subkey=authenticator_subkey, + preauth_key=preauth_key, + armor_key=armor_key, + armor_tgt=armor_tgt, + armor_subkey=authenticator_subkey, + kdc_options='0', + pac_options=pac_options, + # PA-DATA types are not important for these tests. + check_patypes=False) + + rep = self._generic_kdc_exchange( + kdc_exchange_dict, + cname=client_cname, + realm=client_realm, + sname=target_sname, + etypes=etypes) + if expected_error: + self.check_error_rep(rep, expected_error) + return None + else: + self.check_as_reply(rep) + return kdc_exchange_dict['rep_ticket_creds'] + def _tgs_req(self, tgt, expected_error, creds, target_creds, *, armor_tgt=None, kdc_options='0', diff --git a/python/samba/tests/krb5/raw_testcase.py b/python/samba/tests/krb5/raw_testcase.py index 17a0fe906ac0a..1507e4a9c5adf 100644 --- a/python/samba/tests/krb5/raw_testcase.py +++ b/python/samba/tests/krb5/raw_testcase.py @@ -2992,6 +2992,7 @@ class RawKerberosTest(TestCase): expected_sid=None, expected_requester_sid=None, expected_domain_sid=None, + expected_device_domain_sid=None, expected_supported_etypes=None, expected_flags=None, unexpected_flags=None, @@ -3070,6 +3071,7 @@ class RawKerberosTest(TestCase): 'expected_sid': expected_sid, 'expected_requester_sid': expected_requester_sid, 'expected_domain_sid': expected_domain_sid, + 'expected_device_domain_sid': expected_device_domain_sid, 'expected_supported_etypes': expected_supported_etypes, 'expected_flags': expected_flags, 'unexpected_flags': unexpected_flags, -- 2.39.1 From 1561b504ce596a9166bc26a5070ee68408160ddd Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Mon, 30 Oct 2023 14:05:17 +1300 Subject: [PATCH 2/8] tests/krb5: Use __slots__ to indicate which attributes are used by classes These should help to catch mistaken attempts to set invalid attributes. Signed-off-by: Joseph Sutton Reviewed-by: Andrew Bartlett (cherry picked from commit 2b69e1e7c316e634090aad1d97ecadf8cdf529f3) --- python/samba/tests/krb5/kdc_base_test.py | 2 ++ python/samba/tests/krb5/raw_testcase.py | 37 ++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/python/samba/tests/krb5/kdc_base_test.py b/python/samba/tests/krb5/kdc_base_test.py index 96ae43a0937bd..76cea241d30f4 100644 --- a/python/samba/tests/krb5/kdc_base_test.py +++ b/python/samba/tests/krb5/kdc_base_test.py @@ -137,6 +137,8 @@ class GroupType(Enum): # This simple class encapsulates the DN and SID of a Principal. class Principal: + __slots__ = ['dn', 'sid'] + def __init__(self, dn, sid): if dn is not None and not isinstance(dn, ldb.Dn): raise AssertionError(f'expected {dn} to be an ldb.Dn') diff --git a/python/samba/tests/krb5/raw_testcase.py b/python/samba/tests/krb5/raw_testcase.py index 1507e4a9c5adf..db4db9c9e8573 100644 --- a/python/samba/tests/krb5/raw_testcase.py +++ b/python/samba/tests/krb5/raw_testcase.py @@ -248,6 +248,13 @@ krb5_asn1.KerbErrorDataType.prettyPrint =\ class Krb5EncryptionKey: + __slots__ = [ + 'ctype', + 'etype', + 'key', + 'kvno', + ] + def __init__(self, key, kvno): EncTypeChecksum = { kcrypto.Enctype.AES256: kcrypto.Cksumtype.SHA1_AES256, @@ -304,6 +311,8 @@ class Krb5EncryptionKey: class RodcPacEncryptionKey(Krb5EncryptionKey): + __slots__ = ['rodc_id'] + def __init__(self, key, kvno, rodc_id=None): super().__init__(key, kvno) @@ -351,6 +360,8 @@ class ZeroedChecksumKey(RodcPacEncryptionKey): class WrongLengthChecksumKey(RodcPacEncryptionKey): + __slots__ = ['_length'] + def __init__(self, key, kvno, length): super().__init__(key, kvno) @@ -382,6 +393,20 @@ class WrongLengthChecksumKey(RodcPacEncryptionKey): class KerberosCredentials(Credentials): + __slots__ = [ + '_private_key', + 'account_type', + 'ap_supported_enctypes', + 'as_supported_enctypes', + 'dn', + 'forced_keys', + 'forced_salt', + 'kvno', + 'sid', + 'spn', + 'tgs_supported_enctypes', + 'upn', + ] non_etype_bits = ( security.KERB_ENCTYPE_FAST_SUPPORTED) | ( @@ -577,6 +602,18 @@ class KerberosCredentials(Credentials): class KerberosTicketCreds: + __slots__ = [ + 'cname', + 'crealm', + 'decryption_key', + 'encpart_private', + 'session_key', + 'sname', + 'srealm', + 'ticket_private', + 'ticket', + ] + def __init__(self, ticket, session_key, crealm=None, cname=None, srealm=None, sname=None, -- 2.39.1 From b40950d7d33b7451735f7d37e00b9bf7ba105848 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Fri, 17 May 2024 14:19:31 +1200 Subject: [PATCH 3/8] dsdb: Reduce minimum maxPwdAge from 1 day to nil This allows us to have tests, which pass on Windows, that use a very short maxPwdAge. Signed-off-by: Andrew Bartlett Reviewed-by: Jo Sutton (cherry picked from commit 3669479f22f2109a64250ffabd1f6453882d29f1) --- source4/dsdb/samdb/ldb_modules/operational.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/operational.c b/source4/dsdb/samdb/ldb_modules/operational.c index 8c8bfa89ddef1..9ca96ac27de5b 100644 --- a/source4/dsdb/samdb/ldb_modules/operational.c +++ b/source4/dsdb/samdb/ldb_modules/operational.c @@ -786,13 +786,13 @@ static NTTIME get_msds_user_password_expiry_time_computed(struct ldb_module *mod * * Possible values are in the range of: * - * maxPwdAge: -864000000001 + * maxPwdAge: -2 * to * maxPwdAge: -9223372036854775808 (-0x8000000000000000ULL) * */ maxPwdAge = get_user_max_pwd_age(module, msg, parent, domain_dn); - if (maxPwdAge >= -864000000000) { + if (maxPwdAge >= -1) { /* * This is not really possible... */ -- 2.39.1 From 037ab35811827a948781f035b5f64f6bcd8d0537 Mon Sep 17 00:00:00 2001 From: Jo Sutton Date: Fri, 22 Mar 2024 12:58:19 +1300 Subject: [PATCH 4/8] tests/krb5: Fix PK-INIT test framework to allow expired password keys Signed-off-by: Jo Sutton Reviewed-by: Andrew Bartlett (cherry picked from commit 7cc8f455191faacf32efc474c27e99d45ef2e024) --- python/samba/tests/krb5/raw_testcase.py | 3 +++ python/samba/tests/krb5/rfc4120_constants.py | 1 + 2 files changed, 4 insertions(+) diff --git a/python/samba/tests/krb5/raw_testcase.py b/python/samba/tests/krb5/raw_testcase.py index db4db9c9e8573..f3f7778c8415b 100644 --- a/python/samba/tests/krb5/raw_testcase.py +++ b/python/samba/tests/krb5/raw_testcase.py @@ -65,6 +65,7 @@ from samba.tests.krb5.rfc4120_constants import ( FX_FAST_ARMOR_AP_REQUEST, KDC_ERR_CLIENT_REVOKED, KDC_ERR_GENERIC, + KDC_ERR_KEY_EXPIRED, KDC_ERR_POLICY, KDC_ERR_PREAUTH_FAILED, KDC_ERR_SKEW, @@ -5031,6 +5032,8 @@ class RawKerberosTest(TestCase): if ('1' in sent_pac_options and error_code not in (0, KDC_ERR_GENERIC)): expected_patypes += (PADATA_PAC_OPTIONS,) + elif error_code == KDC_ERR_KEY_EXPIRED: + expected_patypes += (PADATA_PK_AS_REP,) elif error_code != KDC_ERR_GENERIC: if expect_etype_info: expected_patypes += (PADATA_ETYPE_INFO,) diff --git a/python/samba/tests/krb5/rfc4120_constants.py b/python/samba/tests/krb5/rfc4120_constants.py index 583ffbaf6af5b..a5dc83db7ab15 100644 --- a/python/samba/tests/krb5/rfc4120_constants.py +++ b/python/samba/tests/krb5/rfc4120_constants.py @@ -99,6 +99,7 @@ KDC_ERR_ETYPE_NOSUPP = 14 KDC_ERR_SUMTYPE_NOSUPP = 15 KDC_ERR_CLIENT_REVOKED = 18 KDC_ERR_TGT_REVOKED = 20 +KDC_ERR_KEY_EXPIRED = 23 KDC_ERR_PREAUTH_FAILED = 24 KDC_ERR_PREAUTH_REQUIRED = 25 KDC_ERR_BAD_INTEGRITY = 31 -- 2.39.1 From d034116767c9004fb7bee8a4aed6a7e9f0458c29 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 19 Mar 2024 14:37:24 +1300 Subject: [PATCH 5/8] python/tests/krb5: Prepare for PKINIT tests with UF_SMARTCARD_REQUIRED Signed-off-by: Andrew Bartlett Reviewed-by: Jo Sutton (backported from commit b2fe1ea1c6aba116b31a1c803b4e0d36ac1a32ee) [jsutton@samba.org Fixed conflicting import statements in python/samba/tests/krb5/pkinit_tests.py] [jsutton@samba.org Fixed conflicting import statements in python/samba/tests/krb5/kdc_base_test.py] --- python/samba/tests/krb5/kdc_base_test.py | 21 ++++++++++++++++++--- python/samba/tests/krb5/pkinit_tests.py | 15 ++++++++++++--- python/samba/tests/krb5/raw_testcase.py | 15 +++++++++++++-- 3 files changed, 43 insertions(+), 8 deletions(-) diff --git a/python/samba/tests/krb5/kdc_base_test.py b/python/samba/tests/krb5/kdc_base_test.py index 76cea241d30f4..7d6cad9a78507 100644 --- a/python/samba/tests/krb5/kdc_base_test.py +++ b/python/samba/tests/krb5/kdc_base_test.py @@ -80,7 +80,8 @@ from samba.dsdb import ( UF_NOT_DELEGATED, UF_PARTIAL_SECRETS_ACCOUNT, UF_SERVER_TRUST_ACCOUNT, - UF_TRUSTED_TO_AUTHENTICATE_FOR_DELEGATION + UF_TRUSTED_TO_AUTHENTICATE_FOR_DELEGATION, + UF_SMARTCARD_REQUIRED ) from samba.dcerpc.misc import ( SEC_CHAN_NULL, @@ -907,6 +908,7 @@ class KDCBaseTest(TestCaseInTempDir, RawKerberosTest): creds.set_upn(upn) creds.set_spn(spn) creds.set_type(account_type) + creds.set_user_account_control(account_control) self.creds_set_enctypes(creds) @@ -1712,6 +1714,7 @@ class KDCBaseTest(TestCaseInTempDir, RawKerberosTest): 'assigned_policy': None, 'assigned_silo': None, 'logon_hours': None, + 'smartcard_required': False } account_opts = { @@ -1764,7 +1767,8 @@ class KDCBaseTest(TestCaseInTempDir, RawKerberosTest): force_nt4_hash, assigned_policy, assigned_silo, - logon_hours): + logon_hours, + smartcard_required): if account_type is self.AccountType.USER: self.assertIsNone(delegation_to_spn) self.assertIsNone(delegation_from_dn) @@ -1787,6 +1791,8 @@ class KDCBaseTest(TestCaseInTempDir, RawKerberosTest): user_account_control |= UF_NOT_DELEGATED if no_auth_data_required: user_account_control |= UF_NO_AUTH_DATA_REQUIRED + if smartcard_required: + user_account_control |= UF_SMARTCARD_REQUIRED if additional_details: details = {k: v for k, v in additional_details} @@ -1844,7 +1850,16 @@ class KDCBaseTest(TestCaseInTempDir, RawKerberosTest): preserve=use_cache) expected_etypes = None - if force_nt4_hash: + + # We don't force fetching the keys other than the NT hash as + # how the server stores the unused KDC keys for the + # smartcard_required case is not important and makes unrelated + # tests break because of differences between Samba and + # Windows. + # + # The NT hash is different, as it is returned to the client in + # the PAC so is visible in the network behaviour. + if force_nt4_hash or smartcard_required: expected_etypes = {kcrypto.Enctype.RC4} keys = self.get_keys(creds, expected_etypes=expected_etypes) self.creds_set_keys(creds, keys) diff --git a/python/samba/tests/krb5/pkinit_tests.py b/python/samba/tests/krb5/pkinit_tests.py index 3d47c799f8680..afbbb45bf73c0 100755 --- a/python/samba/tests/krb5/pkinit_tests.py +++ b/python/samba/tests/krb5/pkinit_tests.py @@ -35,6 +35,8 @@ from cryptography.hazmat.primitives.asymmetric import dh, padding from cryptography.x509.oid import NameOID import samba.tests +from samba import credentials, generate_random_password, ntstatus +from samba.dcerpc import security, netlogon from samba.tests.krb5 import kcrypto from samba.tests.krb5.kdc_base_test import KDCBaseTest from samba.tests.krb5.raw_testcase import PkInit @@ -43,6 +45,7 @@ from samba.tests.krb5.rfc4120_constants import ( KDC_ERR_CLIENT_NOT_TRUSTED, KDC_ERR_ETYPE_NOSUPP, KDC_ERR_MODIFIED, + KDC_ERR_POLICY, KDC_ERR_PREAUTH_EXPIRED, KDC_ERR_PREAUTH_FAILED, KDC_ERR_PREAUTH_REQUIRED, @@ -69,7 +72,7 @@ class PkInitTests(KDCBaseTest): self.do_asn1_print = global_asn1_print self.do_hexdump = global_hexdump - def _get_creds(self, account_type=KDCBaseTest.AccountType.USER): + def _get_creds(self, account_type=KDCBaseTest.AccountType.USER, use_cache=False, smartcard_required=False): """Return credentials with an account having a UPN for performing PK-INIT.""" samdb = self.get_samdb() @@ -77,7 +80,9 @@ class PkInitTests(KDCBaseTest): return self.get_cached_creds( account_type=account_type, - opts={'upn': f'{{account}}.{realm}@{realm}'}) + opts={'upn': f'{{account}}.{realm}@{realm}', + 'smartcard_required': smartcard_required}, + use_cache=use_cache) def test_pkinit_no_des3(self): """Test public-key PK-INIT without specifying the DES3 encryption @@ -568,6 +573,8 @@ class PkInitTests(KDCBaseTest): target_creds, *, expect_error=0, + expect_status=False, + expected_status=None, expect_edata=False, etypes=None, freshness=None, @@ -656,7 +663,9 @@ class PkInitTests(KDCBaseTest): expected_salt=creds.get_salt(), preauth_key=preauth_key, kdc_options=str(kdc_options), - expect_edata=expect_edata) + expect_edata=expect_edata, + expect_status=expect_status, + expected_status=expected_status) till = self.get_KerberosTime(offset=36000) diff --git a/python/samba/tests/krb5/raw_testcase.py b/python/samba/tests/krb5/raw_testcase.py index f3f7778c8415b..a80e62a29b40c 100644 --- a/python/samba/tests/krb5/raw_testcase.py +++ b/python/samba/tests/krb5/raw_testcase.py @@ -54,7 +54,9 @@ from samba.dcerpc.misc import ( SEC_CHAN_WKSTA, SEC_CHAN_BDC, ) - +from samba.dsdb import ( + UF_SMARTCARD_REQUIRED +) import samba.tests from samba.tests import TestCase @@ -407,6 +409,7 @@ class KerberosCredentials(Credentials): 'spn', 'tgs_supported_enctypes', 'upn', + 'user_account_control' ] non_etype_bits = ( @@ -438,6 +441,8 @@ class KerberosCredentials(Credentials): self.sid = None self.account_type = None + self.user_account_control = None + self._private_key = None def set_as_supported_enctypes(self, value): @@ -449,6 +454,9 @@ class KerberosCredentials(Credentials): def set_ap_supported_enctypes(self, value): self.ap_supported_enctypes = int(value) + def set_user_account_control(self, value): + self.user_account_control = int(value) + etype_map = collections.OrderedDict([ (kcrypto.Enctype.AES256, security.KERB_ENCTYPE_AES256_CTS_HMAC_SHA1_96), @@ -4745,7 +4753,10 @@ class RawKerberosTest(TestCase): creds = kdc_exchange_dict['creds'] nt_password = bytes(ntlm_package.nt_password.hash) - self.assertEqual(creds.get_nt_hash(), nt_password) + if creds.user_account_control & UF_SMARTCARD_REQUIRED: + self.assertNotEqual(creds.get_nt_hash(), nt_password) + else: + self.assertEqual(creds.get_nt_hash(), nt_password) lm_password = bytes(ntlm_package.lm_password.hash) self.assertEqual(bytes(16), lm_password) -- 2.39.1 From 2ce0242698cb5944b048b747f3f7566bb076b8d2 Mon Sep 17 00:00:00 2001 From: Jo Sutton Date: Tue, 25 Jun 2024 12:51:48 +1200 Subject: [PATCH 6/8] tests/krb5: Allow creation of disabled accounts for testing BUG: https://bugzilla.samba.org/show_bug.cgi?id=15655 Signed-off-by: Jo Sutton Reviewed-by: Douglas Bagnall (backported from commit 6dc6168719cf232ac2c1d747f10aad9b13300c02) [jsutton@samba.org Fixed conflicting import statements in python/samba/tests/krb5/kdc_base_test.py] [jsutton@samba.org Fixed conflicting import statements in python/samba/tests/krb5/kdc_base_test.py] --- python/samba/tests/krb5/kdc_base_test.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/python/samba/tests/krb5/kdc_base_test.py b/python/samba/tests/krb5/kdc_base_test.py index 7d6cad9a78507..6e8549be11b47 100644 --- a/python/samba/tests/krb5/kdc_base_test.py +++ b/python/samba/tests/krb5/kdc_base_test.py @@ -74,6 +74,7 @@ from samba.dsdb import ( GTYPE_SECURITY_DOMAIN_LOCAL_GROUP, GTYPE_SECURITY_GLOBAL_GROUP, GTYPE_SECURITY_UNIVERSAL_GROUP, + UF_ACCOUNTDISABLE, UF_WORKSTATION_TRUST_ACCOUNT, UF_NO_AUTH_DATA_REQUIRED, UF_NORMAL_ACCOUNT, @@ -1714,7 +1715,8 @@ class KDCBaseTest(TestCaseInTempDir, RawKerberosTest): 'assigned_policy': None, 'assigned_silo': None, 'logon_hours': None, - 'smartcard_required': False + 'smartcard_required': False, + 'enabled': True, } account_opts = { @@ -1768,7 +1770,8 @@ class KDCBaseTest(TestCaseInTempDir, RawKerberosTest): assigned_policy, assigned_silo, logon_hours, - smartcard_required): + smartcard_required, + enabled): if account_type is self.AccountType.USER: self.assertIsNone(delegation_to_spn) self.assertIsNone(delegation_from_dn) @@ -1793,6 +1796,8 @@ class KDCBaseTest(TestCaseInTempDir, RawKerberosTest): user_account_control |= UF_NO_AUTH_DATA_REQUIRED if smartcard_required: user_account_control |= UF_SMARTCARD_REQUIRED + if not enabled: + user_account_control |= UF_ACCOUNTDISABLE if additional_details: details = {k: v for k, v in additional_details} -- 2.39.1 From 0744ce79c3c96a0b43b94cf41ecafdf40d56514d Mon Sep 17 00:00:00 2001 From: Jo Sutton Date: Thu, 27 Jun 2024 12:29:52 +1200 Subject: [PATCH 7/8] tests/krb5: Add tests for errors produced when logging in with unusable accounts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Heimdal matches Windows in the no‐FAST case, but produces NTSTATUS codes when it shouldn’t in the FAST case. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15655 Signed-off-by: Jo Sutton Reviewed-by: Douglas Bagnall (backported from commit c5ee0b60b20011aeaa60c2f549c2a78269c97c8f) [jsutton@samba.org Fixed conflicts in selftest/knownfail_heimdal_kdc] --- python/samba/tests/krb5/lockout_tests.py | 210 ++++++++++++++++++++++- selftest/knownfail_heimdal_kdc | 8 + selftest/knownfail_mit_kdc | 5 + 3 files changed, 221 insertions(+), 2 deletions(-) diff --git a/python/samba/tests/krb5/lockout_tests.py b/python/samba/tests/krb5/lockout_tests.py index 3fe098a662d85..cfae57132ee87 100755 --- a/python/samba/tests/krb5/lockout_tests.py +++ b/python/samba/tests/krb5/lockout_tests.py @@ -58,11 +58,12 @@ 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.kdc_tgs_tests import KdcTgsBaseTests 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_KEY_EXPIRED, KDC_ERR_PREAUTH_FAILED, KRB_AS_REP, KRB_ERROR, @@ -518,7 +519,7 @@ def ldap_pwd_change(pipe, return ConnectionResult.SUCCESS -class LockoutTests(KDCBaseTest): +class LockoutTests(KdcTgsBaseTests): def setUp(self): super().setUp() @@ -611,6 +612,211 @@ class LockoutTests(KDCBaseTest): def test_lockout_transaction_kdc_ntstatus(self): self.do_lockout_transaction(partial(connect_kdc, expect_status=True)) + # Test that performing AS‐REQs with accounts in various states of + # unusability results in appropriate NTSTATUS and Kerberos error codes. + + def test_lockout_status_disabled(self): + self._run_lockout_status( + self._get_creds_disabled(), + expected_status=ntstatus.NT_STATUS_ACCOUNT_DISABLED, + expected_error=KDC_ERR_CLIENT_REVOKED, + ) + + def test_lockout_status_locked_out(self): + self._run_lockout_status( + self._get_creds_locked_out(), + expected_status=ntstatus.NT_STATUS_ACCOUNT_LOCKED_OUT, + expected_error=KDC_ERR_CLIENT_REVOKED, + ) + + def test_lockout_status_expired(self): + self._run_lockout_status( + self._get_creds_expired(), + expected_status=ntstatus.NT_STATUS_ACCOUNT_EXPIRED, + expected_error=KDC_ERR_CLIENT_REVOKED, + ) + + def test_lockout_status_must_change(self): + self._run_lockout_status( + self._get_creds_must_change(), + expected_status=ntstatus.NT_STATUS_PASSWORD_MUST_CHANGE, + expected_error=KDC_ERR_KEY_EXPIRED, + ) + + def test_lockout_status_password_expired(self): + self._run_lockout_status( + self._get_creds_password_expired(), + expected_status=ntstatus.NT_STATUS_PASSWORD_EXPIRED, + expected_error=KDC_ERR_KEY_EXPIRED, + ) + + # Test that performing the same AS‐REQs, this time with FAST, does not + # result in NTSTATUS codes. + + def test_lockout_status_disabled_fast(self): + self._run_lockout_status_fast( + self._get_creds_disabled(), expected_error=KDC_ERR_CLIENT_REVOKED + ) + + def test_lockout_status_locked_out_fast(self): + self._run_lockout_status_fast( + self._get_creds_locked_out(), expected_error=KDC_ERR_CLIENT_REVOKED + ) + + def test_lockout_status_expired_fast(self): + self._run_lockout_status_fast( + self._get_creds_expired(), expected_error=KDC_ERR_CLIENT_REVOKED + ) + + def test_lockout_status_must_change_fast(self): + self._run_lockout_status_fast( + self._get_creds_must_change(), expected_error=KDC_ERR_KEY_EXPIRED + ) + + def test_lockout_status_password_expired_fast(self): + self._run_lockout_status_fast( + self._get_creds_password_expired(), expected_error=KDC_ERR_KEY_EXPIRED + ) + + def _get_creds_disabled(self): + return self.get_cached_creds( + account_type=self.AccountType.USER, opts={"enabled": False} + ) + + def _get_creds_locked_out(self) -> KerberosCredentials: + samdb = self.get_samdb() + + user_creds = self.get_cached_creds( + account_type=self.AccountType.USER, use_cache=False + ) + user_dn = user_creds.get_dn() + + # Lock out the account. + + old_utf16pw = '"Secret007"'.encode("utf-16le") # invalid pwd + new_utf16pw = '"Secret008"'.encode("utf-16le") + + msg = ldb.Message(user_dn) + msg["0"] = ldb.MessageElement(old_utf16pw, ldb.FLAG_MOD_DELETE, "unicodePwd") + msg["1"] = ldb.MessageElement(new_utf16pw, ldb.FLAG_MOD_ADD, "unicodePwd") + + for _ in range(self.lockout_threshold): + try: + samdb.modify(msg) + except ldb.LdbError as err: + num, _ = err.args + + # We get an error, but the bad password count should + # still be updated. + self.assertEqual(num, ldb.ERR_CONSTRAINT_VIOLATION) + else: + self.fail("pwd change should have failed") + + # 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) + + return user_creds + + def _get_creds_expired(self) -> KerberosCredentials: + return self.get_cached_creds( + account_type=self.AccountType.USER, + opts={"additional_details": self.freeze({"accountExpires": "1"})}, + ) + + def _get_creds_must_change(self) -> KerberosCredentials: + return self.get_cached_creds( + account_type=self.AccountType.USER, + opts={"additional_details": self.freeze({"pwdLastSet": "0"})}, + ) + + def _get_creds_password_expired(self) -> KerberosCredentials: + samdb = self.get_samdb() + self.addCleanup(samdb.set_maxPwdAge, samdb.get_maxPwdAge()) + low_pwd_age = -2 + samdb.set_maxPwdAge(low_pwd_age) + + return self.get_cached_creds(account_type=self.AccountType.USER) + + def _run_lockout_status( + self, + user_creds: KerberosCredentials, + *, + expected_status: int, + expected_error: int, + ) -> None: + user_name = user_creds.get_username() + cname = self.PrincipalName_create( + name_type=NT_PRINCIPAL, names=user_name.split("/") + ) + + krbtgt_creds = self.get_krbtgt_creds() + realm = krbtgt_creds.get_realm() + + sname = self.get_krbtgt_sname() + + preauth_key = self.PasswordKey_from_creds(user_creds, kcrypto.Enctype.AES256) + + ts_enc_padata = self.get_enc_timestamp_pa_data_from_key(preauth_key) + padata = [ts_enc_padata] + + def _generate_padata_copy(_kdc_exchange_dict, _callback_dict, req_body): + return padata, req_body + + kdc_exchange_dict = self.as_exchange_dict( + creds=user_creds, + expected_crealm=realm, + expected_cname=cname, + expected_srealm=realm, + expected_sname=sname, + expected_account_name=user_name, + expected_supported_etypes=krbtgt_creds.tgs_supported_enctypes, + expect_edata=True, + expect_status=True, + expected_status=expected_status, + ticket_decryption_key=self.TicketDecryptionKey_from_creds(krbtgt_creds), + generate_padata_fn=_generate_padata_copy, + check_error_fn=self.generic_check_kdc_error, + check_rep_fn=None, + check_kdc_private_fn=self.generic_check_kdc_private, + expected_error_mode=expected_error, + expected_salt=user_creds.get_salt(), + preauth_key=preauth_key, + kdc_options=str(krb5_asn1.KDCOptions("postdated")), + pac_request=True, + ) + + # Try making a Kerberos AS-REQ to the KDC. This might fail, either due + # to the user's account being locked out or due to using the wrong + # password. + self._generic_kdc_exchange( + kdc_exchange_dict, + cname=cname, + realm=realm, + sname=sname, + till_time=self.get_KerberosTime(offset=36000), + etypes=self.get_default_enctypes(user_creds), + ) + + def _run_lockout_status_fast( + self, user_creds: KerberosCredentials, *, expected_error: int + ) -> None: + self._armored_as_req( + user_creds, + self.get_krbtgt_creds(), + self.get_tgt(self.get_mach_creds()), + expected_error=expected_error, + expect_edata=self.expect_padata_outer, + # FAST‐armored responses never contain an NTSTATUS code. + expect_status=False, + ) + def test_lockout_transaction_ntlm(self): self.do_lockout_transaction(connect_ntlm) diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc index 8c4c7f73ff5a7..a3bca02695b44 100644 --- a/selftest/knownfail_heimdal_kdc +++ b/selftest/knownfail_heimdal_kdc @@ -75,3 +75,11 @@ ^samba.tests.krb5.pkinit_tests.samba.tests.krb5.pkinit_tests.PkInitTests.test_pkinit_sha256_certificate_signature_win2k.ad_dc ^samba.tests.krb5.pkinit_tests.samba.tests.krb5.pkinit_tests.PkInitTests.test_pkinit_sha256_signature_win2k.ad_dc ^samba.tests.krb5.pkinit_tests.samba.tests.krb5.pkinit_tests.PkInitTests.test_pkinit_win2k.ad_dc +# +# Lockout tests +# +^samba\.tests\.krb5\.lockout_tests\.samba\.tests\.krb5\.lockout_tests\.LockoutTests\.test_lockout_status_disabled_fast\(ad_dc:local\)$ +^samba\.tests\.krb5\.lockout_tests\.samba\.tests\.krb5\.lockout_tests\.LockoutTests\.test_lockout_status_expired_fast\(ad_dc:local\)$ +^samba\.tests\.krb5\.lockout_tests\.samba\.tests\.krb5\.lockout_tests\.LockoutTests\.test_lockout_status_locked_out_fast\(ad_dc:local\)$ +^samba\.tests\.krb5\.lockout_tests\.samba\.tests\.krb5\.lockout_tests\.LockoutTests\.test_lockout_status_must_change_fast\(ad_dc:local\)$ +^samba\.tests\.krb5\.lockout_tests\.samba\.tests\.krb5\.lockout_tests\.LockoutTests\.test_lockout_status_password_expired_fast\(ad_dc:local\)$ diff --git a/selftest/knownfail_mit_kdc b/selftest/knownfail_mit_kdc index 6f4df9771ca82..c9f58df8e3949 100644 --- a/selftest/knownfail_mit_kdc +++ b/selftest/knownfail_mit_kdc @@ -506,6 +506,11 @@ samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_ ^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_transaction_kdc_ntstatus.ad_dc:local ^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_transaction_rename_kdc.ad_dc:local ^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_transaction_rename_kdc_ntstatus.ad_dc:local +^samba\.tests\.krb5\.lockout_tests\.samba\.tests\.krb5\.lockout_tests\.LockoutTests\.test_lockout_status_disabled\(ad_dc:local\)$ +^samba\.tests\.krb5\.lockout_tests\.samba\.tests\.krb5\.lockout_tests\.LockoutTests\.test_lockout_status_expired\(ad_dc:local\)$ +^samba\.tests\.krb5\.lockout_tests\.samba\.tests\.krb5\.lockout_tests\.LockoutTests\.test_lockout_status_locked_out\(ad_dc:local\)$ +^samba\.tests\.krb5\.lockout_tests\.samba\.tests\.krb5\.lockout_tests\.LockoutTests\.test_lockout_status_must_change\(ad_dc:local\)$ +^samba\.tests\.krb5\.lockout_tests\.samba\.tests\.krb5\.lockout_tests\.LockoutTests\.test_lockout_status_password_expired\(ad_dc:local\)$ # # Encryption type tests # -- 2.39.1 From 3316ee58b4f7867ee25db59f2c811ab7478909b2 Mon Sep 17 00:00:00 2001 From: Jo Sutton Date: Wed, 12 Jun 2024 14:42:38 +1200 Subject: [PATCH 8/8] third_party/heimdal: Import lorikeet-heimdal-202406240121 (commit 4315286377278234be2f3b6d52225a17b6116d54) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This lets us match the Windows FAST reply when the password is expired. Windows clients were upset by the NTSTATUS field in the edata, apparently interpreting it to mean “insufficient resource”. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15655 Signed-off-by: Jo Sutton Reviewed-by: Douglas Bagnall (backported from commit fe90576871b5d644b9e888fd7a0b0351feaba750) [jsutton@samba.org Fixed conflicts in knownfails and third_party/heimdal/kdc/fast.c] --- selftest/knownfail_heimdal_kdc | 8 -------- third_party/heimdal/kdc/fast.c | 13 ++++++++++++- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc index a3bca02695b44..8c4c7f73ff5a7 100644 --- a/selftest/knownfail_heimdal_kdc +++ b/selftest/knownfail_heimdal_kdc @@ -75,11 +75,3 @@ ^samba.tests.krb5.pkinit_tests.samba.tests.krb5.pkinit_tests.PkInitTests.test_pkinit_sha256_certificate_signature_win2k.ad_dc ^samba.tests.krb5.pkinit_tests.samba.tests.krb5.pkinit_tests.PkInitTests.test_pkinit_sha256_signature_win2k.ad_dc ^samba.tests.krb5.pkinit_tests.samba.tests.krb5.pkinit_tests.PkInitTests.test_pkinit_win2k.ad_dc -# -# Lockout tests -# -^samba\.tests\.krb5\.lockout_tests\.samba\.tests\.krb5\.lockout_tests\.LockoutTests\.test_lockout_status_disabled_fast\(ad_dc:local\)$ -^samba\.tests\.krb5\.lockout_tests\.samba\.tests\.krb5\.lockout_tests\.LockoutTests\.test_lockout_status_expired_fast\(ad_dc:local\)$ -^samba\.tests\.krb5\.lockout_tests\.samba\.tests\.krb5\.lockout_tests\.LockoutTests\.test_lockout_status_locked_out_fast\(ad_dc:local\)$ -^samba\.tests\.krb5\.lockout_tests\.samba\.tests\.krb5\.lockout_tests\.LockoutTests\.test_lockout_status_must_change_fast\(ad_dc:local\)$ -^samba\.tests\.krb5\.lockout_tests\.samba\.tests\.krb5\.lockout_tests\.LockoutTests\.test_lockout_status_password_expired_fast\(ad_dc:local\)$ diff --git a/third_party/heimdal/kdc/fast.c b/third_party/heimdal/kdc/fast.c index b63d0b16a9d14..b6dfab849feed 100644 --- a/third_party/heimdal/kdc/fast.c +++ b/third_party/heimdal/kdc/fast.c @@ -488,7 +488,18 @@ _kdc_fast_mk_error(astgs_request_t r, heim_assert(r != NULL, "invalid request in _kdc_fast_mk_error"); - if (r->e_data != NULL) { + if (!armor_crypto && r->e_data != NULL) { + /* + * If we’re not armoring the response with FAST, r->e_data + * takes precedence over the e‐data that would normally be + * generated. r->e_data typically contains a + * Microsoft‐specific NTSTATUS code. + * + * But if FAST is in use, Windows Server suppresses the + * NTSTATUS code in favour of an armored response + * encapsulating an ordinary KRB‐ERROR. So we ignore r->e_data + * in that case. + */ e_data = r->e_data; } else { ret = _kdc_fast_mk_e_data(r, -- 2.39.1