From f21cfb829996db24dc39600d523a7cb6eeefb1a6 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Thu, 16 Jun 2022 15:32:49 +1200 Subject: [PATCH 01/43] CVE-2022-2031 third_party/heimdal: Check generate_pac() return code If the function fails, we should not issue a ticket missing the PAC. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 Signed-off-by: Joseph Sutton --- third_party/heimdal/kdc/kerberos5.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/third_party/heimdal/kdc/kerberos5.c b/third_party/heimdal/kdc/kerberos5.c index e95bdad0a64..0a7934310cc 100644 --- a/third_party/heimdal/kdc/kerberos5.c +++ b/third_party/heimdal/kdc/kerberos5.c @@ -2668,7 +2668,9 @@ _kdc_as_rep(astgs_request_t r) /* Add the PAC */ if (!r->et.flags.anonymous) { - generate_pac(r, skey, krbtgt_key, is_tgs); + ret = generate_pac(r, skey, krbtgt_key, is_tgs); + if (ret) + goto out; } if (r->client->flags.synthetic) { -- 2.35.0 From 428bad4be314ed639327ea95044a1462a0388dfc Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Fri, 27 May 2022 19:17:02 +1200 Subject: [PATCH 02/43] CVE-2022-2031 s4:kpasswd: Account for missing target principal This field is supposed to be optional. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- source4/kdc/kpasswd-service-mit.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/source4/kdc/kpasswd-service-mit.c b/source4/kdc/kpasswd-service-mit.c index 1dbe5adad6d..670210819a1 100644 --- a/source4/kdc/kpasswd-service-mit.c +++ b/source4/kdc/kpasswd-service-mit.c @@ -142,16 +142,18 @@ static krb5_error_code kpasswd_set_password(struct kdc_server *kdc, return KRB5_KPASSWD_HARDERROR; } - target_realm = smb_krb5_principal_get_realm( - mem_ctx, context, target_principal); - code = krb5_unparse_name_flags(context, - target_principal, - KRB5_PRINCIPAL_UNPARSE_NO_REALM, - &target_name); - if (code != 0) { - DBG_WARNING("Failed to parse principal\n"); - *error_string = "String conversion failed"; - return KRB5_KPASSWD_HARDERROR; + if (target_principal != NULL) { + target_realm = smb_krb5_principal_get_realm( + mem_ctx, context, target_principal); + code = krb5_unparse_name_flags(context, + target_principal, + KRB5_PRINCIPAL_UNPARSE_NO_REALM, + &target_name); + if (code != 0) { + DBG_WARNING("Failed to parse principal\n"); + *error_string = "String conversion failed"; + return KRB5_KPASSWD_HARDERROR; + } } if ((target_name != NULL && target_realm == NULL) || -- 2.35.0 From 9533a4e5cef693a0d92e97d0fd7a2ca88147b096 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Mon, 30 May 2022 19:17:41 +1200 Subject: [PATCH 03/43] CVE-2022-2031 s4:kpasswd: Add MIT fallback for decoding setpw structure The target principal and realm fields of the setpw structure are supposed to be optional, but in MIT Kerberos they are mandatory. For better compatibility and ease of testing, fall back to parsing the simpler (containing only the new password) structure if the MIT function fails to decode it. Although the target principal and realm fields should be optional, one is not supposed to specified without the other, so we don't have to deal with the case where only one is specified. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- source4/kdc/kpasswd-service-mit.c | 94 ++++++++++++++++++++++++++----- 1 file changed, 79 insertions(+), 15 deletions(-) diff --git a/source4/kdc/kpasswd-service-mit.c b/source4/kdc/kpasswd-service-mit.c index 670210819a1..26b62162e68 100644 --- a/source4/kdc/kpasswd-service-mit.c +++ b/source4/kdc/kpasswd-service-mit.c @@ -28,6 +28,7 @@ #include "kdc/kpasswd_glue.h" #include "kdc/kpasswd-service.h" #include "kdc/kpasswd-helper.h" +#include "../lib/util/asn1.h" #define RFC3244_VERSION 0xff80 @@ -35,6 +36,52 @@ krb5_error_code decode_krb5_setpw_req(const krb5_data *code, krb5_data **password_out, krb5_principal *target_out); +/* + * A fallback for when MIT refuses to parse a setpw structure without the + * (optional) target principal and realm + */ +static bool decode_krb5_setpw_req_simple(TALLOC_CTX *mem_ctx, + const DATA_BLOB *decoded_data, + DATA_BLOB *clear_data) +{ + struct asn1_data *asn1 = NULL; + bool ret; + + asn1 = asn1_init(mem_ctx, 3); + if (asn1 == NULL) { + return false; + } + + ret = asn1_load(asn1, *decoded_data); + if (!ret) { + goto out; + } + + ret = asn1_start_tag(asn1, ASN1_SEQUENCE(0)); + if (!ret) { + goto out; + } + ret = asn1_start_tag(asn1, ASN1_CONTEXT(0)); + if (!ret) { + goto out; + } + ret = asn1_read_OctetString(asn1, mem_ctx, clear_data); + if (!ret) { + goto out; + } + + ret = asn1_end_tag(asn1); + if (!ret) { + goto out; + } + ret = asn1_end_tag(asn1); + +out: + asn1_free(asn1); + + return ret; +} + static krb5_error_code kpasswd_change_password(struct kdc_server *kdc, TALLOC_CTX *mem_ctx, struct auth_session_info *session_info, @@ -92,9 +139,10 @@ static krb5_error_code kpasswd_set_password(struct kdc_server *kdc, const char **error_string) { krb5_context context = kdc->smb_krb5_context->krb5_context; + DATA_BLOB clear_data; krb5_data k_dec_data; - krb5_data *k_clear_data; - krb5_principal target_principal; + krb5_data *k_clear_data = NULL; + krb5_principal target_principal = NULL; krb5_error_code code; DATA_BLOB password; char *target_realm = NULL; @@ -113,29 +161,45 @@ static krb5_error_code kpasswd_set_password(struct kdc_server *kdc, code = decode_krb5_setpw_req(&k_dec_data, &k_clear_data, &target_principal); - if (code != 0) { - DBG_WARNING("decode_krb5_setpw_req failed: %s\n", - error_message(code)); - ok = kpasswd_make_error_reply(mem_ctx, - KRB5_KPASSWD_MALFORMED, - "Failed to decode packet", - kpasswd_reply); + if (code == 0) { + clear_data.data = (uint8_t *)k_clear_data->data; + clear_data.length = k_clear_data->length; + } else { + target_principal = NULL; + + /* + * The MIT decode failed, so fall back to trying the simple + * case, without target_principal. + */ + ok = decode_krb5_setpw_req_simple(mem_ctx, + decoded_data, + &clear_data); if (!ok) { - *error_string = "Failed to create reply"; - return KRB5_KPASSWD_HARDERROR; + DBG_WARNING("decode_krb5_setpw_req failed: %s\n", + error_message(code)); + ok = kpasswd_make_error_reply(mem_ctx, + KRB5_KPASSWD_MALFORMED, + "Failed to decode packet", + kpasswd_reply); + if (!ok) { + *error_string = "Failed to create reply"; + return KRB5_KPASSWD_HARDERROR; + } + return 0; } - return 0; } ok = convert_string_talloc_handle(mem_ctx, lpcfg_iconv_handle(kdc->task->lp_ctx), CH_UTF8, CH_UTF16, - (const char *)k_clear_data->data, - k_clear_data->length, + clear_data.data, + clear_data.length, (void **)&password.data, &password.length); - krb5_free_data(context, k_clear_data); + if (k_clear_data != NULL) { + krb5_free_data(context, k_clear_data); + } if (!ok) { DBG_WARNING("String conversion failed\n"); *error_string = "String conversion failed"; -- 2.35.0 From fef9599cd70889484367cd2eb7195d4706503c1b Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Thu, 26 May 2022 16:34:01 +1200 Subject: [PATCH 04/43] CVE-2022-32744 tests/krb5: Correctly handle specifying account kvno The environment variable is a string, but we expect an integer. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- python/samba/tests/krb5/raw_testcase.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/samba/tests/krb5/raw_testcase.py b/python/samba/tests/krb5/raw_testcase.py index 7f9d9d17640..d0c28fb2002 100644 --- a/python/samba/tests/krb5/raw_testcase.py +++ b/python/samba/tests/krb5/raw_testcase.py @@ -759,7 +759,7 @@ class RawKerberosTest(TestCaseInTempDir): fallback_default=False, allow_missing=kvno_allow_missing) if kvno is not None: - c.set_kvno(kvno) + c.set_kvno(int(kvno)) aes256_key = self.env_get_var('AES256_KEY_HEX', prefix, fallback_default=False, allow_missing=aes256_allow_missing) -- 2.35.0 From 83c0c3de12f2c9bea8996a62526c09b90ed3f32a Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Thu, 26 May 2022 20:52:04 +1200 Subject: [PATCH 05/43] CVE-2022-2031 tests/krb5: Split out _make_tgs_request() This allows us to make use of it in other tests. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- python/samba/tests/krb5/kdc_base_test.py | 85 ++++++++++++++++++++++++ python/samba/tests/krb5/kdc_tgs_tests.py | 84 ----------------------- 2 files changed, 85 insertions(+), 84 deletions(-) diff --git a/python/samba/tests/krb5/kdc_base_test.py b/python/samba/tests/krb5/kdc_base_test.py index d9efde8273a..cc3de5e2bc1 100644 --- a/python/samba/tests/krb5/kdc_base_test.py +++ b/python/samba/tests/krb5/kdc_base_test.py @@ -72,6 +72,7 @@ from samba.tests.krb5.rfc4120_constants import ( AES256_CTS_HMAC_SHA1_96, ARCFOUR_HMAC_MD5, KDC_ERR_PREAUTH_REQUIRED, + KDC_ERR_TGT_REVOKED, KRB_AS_REP, KRB_TGS_REP, KRB_ERROR, @@ -1663,6 +1664,90 @@ class KDCBaseTest(RawKerberosTest): return ticket_creds + def _make_tgs_request(self, client_creds, service_creds, tgt, + client_account=None, + client_name_type=NT_PRINCIPAL, + kdc_options=None, + pac_request=None, expect_pac=True, + expect_error=False, + expected_cname=None, + expected_account_name=None, + expected_upn_name=None, + expected_sid=None): + if client_account is None: + client_account = client_creds.get_username() + cname = self.PrincipalName_create(name_type=client_name_type, + names=client_account.split('/')) + + service_account = service_creds.get_username() + sname = self.PrincipalName_create(name_type=NT_PRINCIPAL, + names=[service_account]) + + realm = service_creds.get_realm() + + expected_crealm = realm + if expected_cname is None: + expected_cname = cname + expected_srealm = realm + expected_sname = sname + + expected_supported_etypes = service_creds.tgs_supported_enctypes + + etypes = (AES256_CTS_HMAC_SHA1_96, ARCFOUR_HMAC_MD5) + + if kdc_options is None: + kdc_options = 'canonicalize' + kdc_options = str(krb5_asn1.KDCOptions(kdc_options)) + + target_decryption_key = self.TicketDecryptionKey_from_creds( + service_creds) + + authenticator_subkey = self.RandomKey(kcrypto.Enctype.AES256) + + if expect_error: + expected_error_mode = KDC_ERR_TGT_REVOKED + check_error_fn = self.generic_check_kdc_error + check_rep_fn = None + else: + expected_error_mode = 0 + check_error_fn = None + check_rep_fn = self.generic_check_kdc_rep + + kdc_exchange_dict = self.tgs_exchange_dict( + expected_crealm=expected_crealm, + expected_cname=expected_cname, + expected_srealm=expected_srealm, + expected_sname=expected_sname, + expected_account_name=expected_account_name, + expected_upn_name=expected_upn_name, + expected_sid=expected_sid, + expected_supported_etypes=expected_supported_etypes, + ticket_decryption_key=target_decryption_key, + 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_mode, + tgt=tgt, + authenticator_subkey=authenticator_subkey, + kdc_options=kdc_options, + pac_request=pac_request, + expect_pac=expect_pac, + expect_edata=False) + + rep = self._generic_kdc_exchange(kdc_exchange_dict, + cname=cname, + realm=realm, + sname=sname, + etypes=etypes) + if expect_error: + self.check_error_rep(rep, expected_error_mode) + + return None + else: + self.check_reply(rep, KRB_TGS_REP) + + return kdc_exchange_dict['rep_ticket_creds'] + # Named tuple to contain values of interest when the PAC is decoded. PacData = namedtuple( "PacData", diff --git a/python/samba/tests/krb5/kdc_tgs_tests.py b/python/samba/tests/krb5/kdc_tgs_tests.py index 1f16d05e2db..83315f6879f 100755 --- a/python/samba/tests/krb5/kdc_tgs_tests.py +++ b/python/samba/tests/krb5/kdc_tgs_tests.py @@ -231,90 +231,6 @@ class KdcTgsTests(KDCBaseTest): pac_data.account_sid, "rep = {%s},%s" % (rep, pac_data)) - def _make_tgs_request(self, client_creds, service_creds, tgt, - client_account=None, - client_name_type=NT_PRINCIPAL, - kdc_options=None, - pac_request=None, expect_pac=True, - expect_error=False, - expected_cname=None, - expected_account_name=None, - expected_upn_name=None, - expected_sid=None): - if client_account is None: - client_account = client_creds.get_username() - cname = self.PrincipalName_create(name_type=client_name_type, - names=client_account.split('/')) - - service_account = service_creds.get_username() - sname = self.PrincipalName_create(name_type=NT_PRINCIPAL, - names=[service_account]) - - realm = service_creds.get_realm() - - expected_crealm = realm - if expected_cname is None: - expected_cname = cname - expected_srealm = realm - expected_sname = sname - - expected_supported_etypes = service_creds.tgs_supported_enctypes - - etypes = (AES256_CTS_HMAC_SHA1_96, ARCFOUR_HMAC_MD5) - - if kdc_options is None: - kdc_options = 'canonicalize' - kdc_options = str(krb5_asn1.KDCOptions(kdc_options)) - - target_decryption_key = self.TicketDecryptionKey_from_creds( - service_creds) - - authenticator_subkey = self.RandomKey(kcrypto.Enctype.AES256) - - if expect_error: - expected_error_mode = KDC_ERR_TGT_REVOKED - check_error_fn = self.generic_check_kdc_error - check_rep_fn = None - else: - expected_error_mode = 0 - check_error_fn = None - check_rep_fn = self.generic_check_kdc_rep - - kdc_exchange_dict = self.tgs_exchange_dict( - expected_crealm=expected_crealm, - expected_cname=expected_cname, - expected_srealm=expected_srealm, - expected_sname=expected_sname, - expected_account_name=expected_account_name, - expected_upn_name=expected_upn_name, - expected_sid=expected_sid, - expected_supported_etypes=expected_supported_etypes, - ticket_decryption_key=target_decryption_key, - 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_mode, - tgt=tgt, - authenticator_subkey=authenticator_subkey, - kdc_options=kdc_options, - pac_request=pac_request, - expect_pac=expect_pac, - expect_edata=False) - - rep = self._generic_kdc_exchange(kdc_exchange_dict, - cname=cname, - realm=realm, - sname=sname, - etypes=etypes) - if expect_error: - self.check_error_rep(rep, expected_error_mode) - - return None - else: - self.check_reply(rep, KRB_TGS_REP) - - return kdc_exchange_dict['rep_ticket_creds'] - def test_request(self): client_creds = self.get_client_creds() service_creds = self.get_service_creds() -- 2.35.0 From ce6fb389b208c83250131527919b17cf58d0c19e Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 19:06:53 +1200 Subject: [PATCH 06/43] CVE-2022-32744 tests/krb5: Correctly calculate salt for pre-existing accounts BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- python/samba/tests/krb5/kdc_base_test.py | 1 + python/samba/tests/krb5/raw_testcase.py | 1 + 2 files changed, 2 insertions(+) diff --git a/python/samba/tests/krb5/kdc_base_test.py b/python/samba/tests/krb5/kdc_base_test.py index cc3de5e2bc1..f0dfddff02f 100644 --- a/python/samba/tests/krb5/kdc_base_test.py +++ b/python/samba/tests/krb5/kdc_base_test.py @@ -1155,6 +1155,7 @@ class KDCBaseTest(RawKerberosTest): kvno = int(res[0]['msDS-KeyVersionNumber'][0]) creds.set_kvno(kvno) + creds.set_workstation(username[:-1]) creds.set_dn(dn) keys = self.get_keys(samdb, dn) diff --git a/python/samba/tests/krb5/raw_testcase.py b/python/samba/tests/krb5/raw_testcase.py index d0c28fb2002..8d2b84c9d7f 100644 --- a/python/samba/tests/krb5/raw_testcase.py +++ b/python/samba/tests/krb5/raw_testcase.py @@ -869,6 +869,7 @@ class RawKerberosTest(TestCaseInTempDir): allow_missing_password=allow_missing_password, allow_missing_keys=allow_missing_keys) c.set_gensec_features(c.get_gensec_features() | FEATURE_SEAL) + c.set_workstation('') return c def get_rodc_krbtgt_creds(self, -- 2.35.0 From 7423f72ebab4bd59ebc327f6dc0897f1d8bfd858 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 19:13:54 +1200 Subject: [PATCH 07/43] CVE-2022-2031 tests/krb5: Add new definitions for kpasswd BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- python/samba/tests/krb5/rfc4120.asn1 | 6 ++++++ python/samba/tests/krb5/rfc4120_constants.py | 13 +++++++++++++ python/samba/tests/krb5/rfc4120_pyasn1.py | 13 ++++++++++++- 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/python/samba/tests/krb5/rfc4120.asn1 b/python/samba/tests/krb5/rfc4120.asn1 index 7b146015548..e2f96829370 100644 --- a/python/samba/tests/krb5/rfc4120.asn1 +++ b/python/samba/tests/krb5/rfc4120.asn1 @@ -568,6 +568,12 @@ PA-FX-FAST-REPLY ::= CHOICE { ... } +ChangePasswdDataMS ::= SEQUENCE { + newpasswd [0] OCTET STRING, + targname [1] PrincipalName OPTIONAL, + targrealm [2] Realm OPTIONAL +} + -- MS-KILE End -- -- diff --git a/python/samba/tests/krb5/rfc4120_constants.py b/python/samba/tests/krb5/rfc4120_constants.py index 28d83407ac5..7d20093f97d 100644 --- a/python/samba/tests/krb5/rfc4120_constants.py +++ b/python/samba/tests/krb5/rfc4120_constants.py @@ -35,11 +35,13 @@ DES3_CBC_SHA1 = int( # Message types KRB_ERROR = int(krb5_asn1.MessageTypeValues('krb-error')) +KRB_AP_REP = int(krb5_asn1.MessageTypeValues('krb-ap-rep')) KRB_AP_REQ = int(krb5_asn1.MessageTypeValues('krb-ap-req')) KRB_AS_REP = int(krb5_asn1.MessageTypeValues('krb-as-rep')) KRB_AS_REQ = int(krb5_asn1.MessageTypeValues('krb-as-req')) KRB_TGS_REP = int(krb5_asn1.MessageTypeValues('krb-tgs-rep')) KRB_TGS_REQ = int(krb5_asn1.MessageTypeValues('krb-tgs-req')) +KRB_PRIV = int(krb5_asn1.MessageTypeValues('krb-priv')) # PAData types PADATA_ENC_TIMESTAMP = int( @@ -90,6 +92,7 @@ KDC_ERR_TGT_REVOKED = 20 KDC_ERR_PREAUTH_FAILED = 24 KDC_ERR_PREAUTH_REQUIRED = 25 KDC_ERR_BAD_INTEGRITY = 31 +KDC_ERR_TKT_EXPIRED = 32 KRB_ERR_TKT_NYV = 33 KDC_ERR_NOT_US = 35 KDC_ERR_BADMATCH = 36 @@ -101,6 +104,16 @@ KDC_ERR_WRONG_REALM = 68 KDC_ERR_CLIENT_NAME_MISMATCH = 75 KDC_ERR_UNKNOWN_CRITICAL_FAST_OPTIONS = 93 +# Kpasswd error codes +KPASSWD_SUCCESS = 0 +KPASSWD_MALFORMED = 1 +KPASSWD_HARDERROR = 2 +KPASSWD_AUTHERROR = 3 +KPASSWD_SOFTERROR = 4 +KPASSWD_ACCESSDENIED = 5 +KPASSWD_BAD_VERSION = 6 +KPASSWD_INITIAL_FLAG_NEEDED = 7 + # Extended error types KERB_AP_ERR_TYPE_SKEW_RECOVERY = int( krb5_asn1.KerbErrorDataTypeValues('kERB-AP-ERR-TYPE-SKEW-RECOVERY')) diff --git a/python/samba/tests/krb5/rfc4120_pyasn1.py b/python/samba/tests/krb5/rfc4120_pyasn1.py index d789ab96b43..ef77ac19ce3 100644 --- a/python/samba/tests/krb5/rfc4120_pyasn1.py +++ b/python/samba/tests/krb5/rfc4120_pyasn1.py @@ -1,5 +1,5 @@ # Auto-generated by asn1ate v.0.6.1.dev0 from rfc4120.asn1 -# (last modified on 2021-06-25 12:10:34.484667) +# (last modified on 2022-05-13 20:03:06.039817) # KerberosV5Spec2 from pyasn1.type import univ, char, namedtype, namedval, tag, constraint, useful @@ -364,6 +364,17 @@ Authenticator.componentType = namedtype.NamedTypes( ) +class ChangePasswdDataMS(univ.Sequence): + pass + + +ChangePasswdDataMS.componentType = namedtype.NamedTypes( + namedtype.NamedType('newpasswd', univ.OctetString().subtype(explicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 0))), + namedtype.OptionalNamedType('targname', PrincipalName().subtype(explicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatConstructed, 1))), + namedtype.OptionalNamedType('targrealm', Realm().subtype(explicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 2))) +) + + class ChecksumTypeValues(univ.Integer): pass -- 2.35.0 From bb68cf53c32beabd7344c63449d3369ecdd7d6b4 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 19:17:45 +1200 Subject: [PATCH 08/43] CVE-2022-2031 tests/krb5: Add methods to create ASN1 kpasswd structures BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- python/samba/tests/krb5/raw_testcase.py | 95 +++++++++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/python/samba/tests/krb5/raw_testcase.py b/python/samba/tests/krb5/raw_testcase.py index 8d2b84c9d7f..2a67e6d991c 100644 --- a/python/samba/tests/krb5/raw_testcase.py +++ b/python/samba/tests/krb5/raw_testcase.py @@ -56,6 +56,7 @@ from samba.tests.krb5.rfc4120_constants import ( KRB_AS_REP, KRB_AS_REQ, KRB_ERROR, + KRB_PRIV, KRB_TGS_REP, KRB_TGS_REQ, KU_AP_REQ_AUTH, @@ -66,6 +67,7 @@ from samba.tests.krb5.rfc4120_constants import ( KU_FAST_FINISHED, KU_FAST_REP, KU_FAST_REQ_CHKSUM, + KU_KRB_PRIV, KU_NON_KERB_CKSUM_SALT, KU_TGS_REP_ENC_PART_SESSION, KU_TGS_REP_ENC_PART_SUB_KEY, @@ -1825,6 +1827,99 @@ class RawKerberosTest(TestCaseInTempDir): PA_S4U2Self_obj, asn1Spec=krb5_asn1.PA_S4U2Self()) return self.PA_DATA_create(PADATA_FOR_USER, pa_s4u2self) + def ChangePasswdDataMS_create(self, + new_password, + target_princ=None, + target_realm=None): + ChangePasswdDataMS_obj = { + 'newpasswd': new_password, + } + if target_princ is not None: + ChangePasswdDataMS_obj['targname'] = target_princ + if target_realm is not None: + ChangePasswdDataMS_obj['targrealm'] = target_realm + + change_password_data = self.der_encode( + ChangePasswdDataMS_obj, asn1Spec=krb5_asn1.ChangePasswdDataMS()) + + return change_password_data + + def KRB_PRIV_create(self, + subkey, + user_data, + s_address, + timestamp=None, + usec=None, + seq_number=None, + r_address=None): + EncKrbPrivPart_obj = { + 'user-data': user_data, + 's-address': s_address, + } + if timestamp is not None: + EncKrbPrivPart_obj['timestamp'] = timestamp + if usec is not None: + EncKrbPrivPart_obj['usec'] = usec + if seq_number is not None: + EncKrbPrivPart_obj['seq-number'] = seq_number + if r_address is not None: + EncKrbPrivPart_obj['r-address'] = r_address + + enc_krb_priv_part = self.der_encode( + EncKrbPrivPart_obj, asn1Spec=krb5_asn1.EncKrbPrivPart()) + + enc_data = self.EncryptedData_create(subkey, + KU_KRB_PRIV, + enc_krb_priv_part) + + KRB_PRIV_obj = { + 'pvno': 5, + 'msg-type': KRB_PRIV, + 'enc-part': enc_data, + } + + krb_priv = self.der_encode( + KRB_PRIV_obj, asn1Spec=krb5_asn1.KRB_PRIV()) + + return krb_priv + + def kpasswd_create(self, + subkey, + user_data, + version, + seq_number, + ap_req, + local_address, + remote_address): + self.assertIsNotNone(self.s, 'call self.connect() first') + + timestamp, usec = self.get_KerberosTimeWithUsec() + + krb_priv = self.KRB_PRIV_create(subkey, + user_data, + s_address=local_address, + timestamp=timestamp, + usec=usec, + seq_number=seq_number, + r_address=remote_address) + + size = 6 + len(ap_req) + len(krb_priv) + self.assertLess(size, 0x10000) + + msg = bytearray() + msg.append(size >> 8) + msg.append(size & 0xff) + msg.append(version >> 8) + msg.append(version & 0xff) + msg.append(len(ap_req) >> 8) + msg.append(len(ap_req) & 0xff) + # Note: for sets, there could be a little-endian four-byte length here. + + msg.extend(ap_req) + msg.extend(krb_priv) + + return msg + def _generic_kdc_exchange(self, kdc_exchange_dict, # required cname=None, # optional -- 2.35.0 From c12850c3d9d09d36d08ef48fc9b2d6f8af32580d Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 19:21:37 +1200 Subject: [PATCH 09/43] CVE-2022-2031 tests/krb5: Add 'port' parameter to connect() This allows us to use the kpasswd port, 464. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- python/samba/tests/krb5/raw_testcase.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/python/samba/tests/krb5/raw_testcase.py b/python/samba/tests/krb5/raw_testcase.py index 2a67e6d991c..3374beecd1e 100644 --- a/python/samba/tests/krb5/raw_testcase.py +++ b/python/samba/tests/krb5/raw_testcase.py @@ -673,10 +673,11 @@ class RawKerberosTest(TestCaseInTempDir): if self.do_hexdump: sys.stderr.write("disconnect[%s]\n" % reason) - def _connect_tcp(self, host): - tcp_port = 88 + def _connect_tcp(self, host, port=None): + if port is None: + port = 88 try: - self.a = socket.getaddrinfo(host, tcp_port, socket.AF_UNSPEC, + self.a = socket.getaddrinfo(host, port, socket.AF_UNSPEC, socket.SOCK_STREAM, socket.SOL_TCP, 0) self.s = socket.socket(self.a[0][0], self.a[0][1], self.a[0][2]) @@ -689,9 +690,9 @@ class RawKerberosTest(TestCaseInTempDir): self.s.close() raise - def connect(self, host): + def connect(self, host, port=None): self.assertNotConnected() - self._connect_tcp(host) + self._connect_tcp(host, port) if self.do_hexdump: sys.stderr.write("connected[%s]\n" % host) -- 2.35.0 From f289a4ae6bb7c2cfcf3ae9b5eaf9b328d3dd7afc Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 19:20:28 +1200 Subject: [PATCH 10/43] CVE-2022-2031 tests/krb5: Add methods to send and receive generic messages This allows us to send and receive kpasswd messages, while avoiding the existing logic for encoding and decoding other Kerberos message types. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- python/samba/tests/krb5/raw_testcase.py | 44 +++++++++++++++---------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/python/samba/tests/krb5/raw_testcase.py b/python/samba/tests/krb5/raw_testcase.py index 3374beecd1e..b16b78ebf56 100644 --- a/python/samba/tests/krb5/raw_testcase.py +++ b/python/samba/tests/krb5/raw_testcase.py @@ -955,24 +955,28 @@ class RawKerberosTest(TestCaseInTempDir): return blob def send_pdu(self, req, asn1_print=None, hexdump=None): + k5_pdu = self.der_encode( + req, native_decode=False, asn1_print=asn1_print, hexdump=False) + self.send_msg(k5_pdu, hexdump=hexdump) + + def send_msg(self, msg, hexdump=None): + header = struct.pack('>I', len(msg)) + req_pdu = header + req_pdu += msg + self.hex_dump("send_msg", header, hexdump=hexdump) + self.hex_dump("send_msg", msg, hexdump=hexdump) + try: - k5_pdu = self.der_encode( - req, native_decode=False, asn1_print=asn1_print, hexdump=False) - header = struct.pack('>I', len(k5_pdu)) - req_pdu = header - req_pdu += k5_pdu - self.hex_dump("send_pdu", header, hexdump=hexdump) - self.hex_dump("send_pdu", k5_pdu, hexdump=hexdump) while True: sent = self.s.send(req_pdu, 0) if sent == len(req_pdu): - break + return req_pdu = req_pdu[sent:] except socket.error as e: - self._disconnect("send_pdu: %s" % e) + self._disconnect("send_msg: %s" % e) raise except IOError as e: - self._disconnect("send_pdu: %s" % e) + self._disconnect("send_msg: %s" % e) raise def recv_raw(self, num_recv=0xffff, hexdump=None, timeout=None): @@ -998,16 +1002,14 @@ class RawKerberosTest(TestCaseInTempDir): return rep_pdu def recv_pdu_raw(self, asn1_print=None, hexdump=None, timeout=None): - rep_pdu = None - rep = None raw_pdu = self.recv_raw( num_recv=4, hexdump=hexdump, timeout=timeout) if raw_pdu is None: - return (None, None) + return None header = struct.unpack(">I", raw_pdu[0:4]) k5_len = header[0] if k5_len == 0: - return (None, "") + return "" missing = k5_len rep_pdu = b'' while missing > 0: @@ -1016,6 +1018,14 @@ class RawKerberosTest(TestCaseInTempDir): self.assertGreaterEqual(len(raw_pdu), 1) rep_pdu += raw_pdu missing = k5_len - len(rep_pdu) + return rep_pdu + + def recv_reply(self, asn1_print=None, hexdump=None, timeout=None): + rep_pdu = self.recv_pdu_raw(asn1_print=asn1_print, + hexdump=hexdump, + timeout=timeout) + if not rep_pdu: + return None, rep_pdu k5_raw = self.der_decode( rep_pdu, asn1Spec=None, @@ -1037,9 +1047,9 @@ class RawKerberosTest(TestCaseInTempDir): return (rep, rep_pdu) def recv_pdu(self, asn1_print=None, hexdump=None, timeout=None): - (rep, rep_pdu) = self.recv_pdu_raw(asn1_print=asn1_print, - hexdump=hexdump, - timeout=timeout) + (rep, rep_pdu) = self.recv_reply(asn1_print=asn1_print, + hexdump=hexdump, + timeout=timeout) return rep def assertIsConnected(self): -- 2.35.0 From 688339238e84527281bc92ee53e5dd08b60d6853 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 19:26:56 +1200 Subject: [PATCH 11/43] tests/krb5: Fix enum typo Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- python/samba/tests/krb5/kdc_base_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/samba/tests/krb5/kdc_base_test.py b/python/samba/tests/krb5/kdc_base_test.py index f0dfddff02f..a1cc1b1e794 100644 --- a/python/samba/tests/krb5/kdc_base_test.py +++ b/python/samba/tests/krb5/kdc_base_test.py @@ -276,9 +276,9 @@ class KDCBaseTest(RawKerberosTest): which is used by tearDownClass to clean up the created accounts. ''' if ou is None: - if account_type is account_type.COMPUTER: + if account_type is self.AccountType.COMPUTER: guid = DS_GUID_COMPUTERS_CONTAINER - elif account_type is account_type.SERVER: + elif account_type is self.AccountType.SERVER: guid = DS_GUID_DOMAIN_CONTROLLERS_CONTAINER else: guid = DS_GUID_USERS_CONTAINER -- 2.35.0 From f71db277539577762cfffb9d5f399aca5a83fd76 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 19:30:12 +1200 Subject: [PATCH 12/43] tests/krb5: Add option for creating accounts with expired passwords Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- python/samba/tests/krb5/kdc_base_test.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/python/samba/tests/krb5/kdc_base_test.py b/python/samba/tests/krb5/kdc_base_test.py index a1cc1b1e794..6b3d38649f4 100644 --- a/python/samba/tests/krb5/kdc_base_test.py +++ b/python/samba/tests/krb5/kdc_base_test.py @@ -270,7 +270,8 @@ class KDCBaseTest(RawKerberosTest): def create_account(self, samdb, name, account_type=AccountType.USER, spn=None, upn=None, additional_details=None, - ou=None, account_control=0, add_dollar=True): + ou=None, account_control=0, add_dollar=True, + expired_password=False): '''Create an account for testing. The dn of the created account is added to self.accounts, which is used by tearDownClass to clean up the created accounts. @@ -324,6 +325,8 @@ class KDCBaseTest(RawKerberosTest): details["servicePrincipalName"] = spn if upn is not None: details["userPrincipalName"] = upn + if expired_password: + details["pwdLastSet"] = "0" if additional_details is not None: details.update(additional_details) # Save the account name so it can be deleted in tearDownClass @@ -730,6 +733,7 @@ class KDCBaseTest(RawKerberosTest): 'revealed_to_rodc': False, 'revealed_to_mock_rodc': False, 'no_auth_data_required': False, + 'expired_password': False, 'supported_enctypes': None, 'not_delegated': False, 'delegation_to_spn': None, @@ -776,6 +780,7 @@ class KDCBaseTest(RawKerberosTest): revealed_to_rodc, revealed_to_mock_rodc, no_auth_data_required, + expired_password, supported_enctypes, not_delegated, delegation_to_spn, @@ -841,7 +846,8 @@ class KDCBaseTest(RawKerberosTest): spn=spn, additional_details=details, account_control=user_account_control, - add_dollar=add_dollar) + add_dollar=add_dollar, + expired_password=expired_password) keys = self.get_keys(samdb, dn) self.creds_set_keys(creds, keys) -- 2.35.0 From 82ee26261d60edb68cea5cb985907e4e38efce03 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 19:34:59 +1200 Subject: [PATCH 13/43] CVE-2022-2031 tests/krb5: Allow requesting a TGT to a different sname and realm BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- python/samba/tests/krb5/kdc_base_test.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/python/samba/tests/krb5/kdc_base_test.py b/python/samba/tests/krb5/kdc_base_test.py index 6b3d38649f4..acd35a2db14 100644 --- a/python/samba/tests/krb5/kdc_base_test.py +++ b/python/samba/tests/krb5/kdc_base_test.py @@ -1451,11 +1451,13 @@ class KDCBaseTest(RawKerberosTest): expected_flags=None, unexpected_flags=None, pac_request=True, expect_pac=True, fresh=False): user_name = tgt.cname['name-string'][0] + ticket_sname = tgt.sname if target_name is None: target_name = target_creds.get_username()[:-1] cache_key = (user_name, target_name, service, to_rodc, kdc_options, pac_request, str(expected_flags), str(unexpected_flags), till, rc4_support, + str(ticket_sname), expect_pac) if not fresh: @@ -1526,6 +1528,7 @@ class KDCBaseTest(RawKerberosTest): expected_account_name=None, expected_upn_name=None, expected_cname=None, expected_sid=None, + sname=None, realm=None, pac_request=True, expect_pac=True, expect_pac_attrs=None, expect_pac_attrs_pac_request=None, expect_requester_sid=None, @@ -1540,6 +1543,7 @@ class KDCBaseTest(RawKerberosTest): client_name_type, str(expected_flags), str(unexpected_flags), expected_account_name, expected_upn_name, expected_sid, + str(sname), str(realm), str(expected_cname), rc4_support, expect_pac, expect_pac_attrs, @@ -1551,15 +1555,21 @@ class KDCBaseTest(RawKerberosTest): if tgt is not None: return tgt - realm = creds.get_realm() + if realm is None: + realm = creds.get_realm() salt = creds.get_salt() etype = (AES256_CTS_HMAC_SHA1_96, ARCFOUR_HMAC_MD5) cname = self.PrincipalName_create(name_type=client_name_type, names=user_name.split('/')) - sname = self.PrincipalName_create(name_type=NT_SRV_INST, - names=['krbtgt', realm]) + if sname is None: + sname = self.PrincipalName_create(name_type=NT_SRV_INST, + names=['krbtgt', realm]) + expected_sname = self.PrincipalName_create( + name_type=NT_SRV_INST, names=['krbtgt', realm.upper()]) + else: + expected_sname = sname if expected_cname is None: expected_cname = cname @@ -1629,9 +1639,6 @@ class KDCBaseTest(RawKerberosTest): expected_realm = realm.upper() - expected_sname = self.PrincipalName_create( - name_type=NT_SRV_INST, names=['krbtgt', realm.upper()]) - rep, kdc_exchange_dict = self._test_as_exchange( cname=cname, realm=realm, -- 2.35.0 From aafedf8d99fb2417a21af66c9de05ee2415fec5c Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 19:57:57 +1200 Subject: [PATCH 14/43] CVE-2022-2031 tests/krb5: Add kpasswd_exchange() method Now we can test the kpasswd service from Python. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- python/samba/tests/krb5/raw_testcase.py | 264 ++++++++++++++++++++++-- 1 file changed, 251 insertions(+), 13 deletions(-) diff --git a/python/samba/tests/krb5/raw_testcase.py b/python/samba/tests/krb5/raw_testcase.py index b16b78ebf56..b22617c3882 100644 --- a/python/samba/tests/krb5/raw_testcase.py +++ b/python/samba/tests/krb5/raw_testcase.py @@ -26,6 +26,8 @@ import binascii import itertools import collections +from enum import Enum + from pyasn1.codec.der.decoder import decode as pyasn1_der_decode from pyasn1.codec.der.encoder import encode as pyasn1_der_encode from pyasn1.codec.native.decoder import decode as pyasn1_native_decode @@ -33,6 +35,8 @@ from pyasn1.codec.native.encoder import encode as pyasn1_native_encode from pyasn1.codec.ber.encoder import BitStringEncoder +from pyasn1.error import PyAsn1Error + from samba.credentials import Credentials from samba.dcerpc import krb5pac, security from samba.gensec import FEATURE_SEAL @@ -52,6 +56,7 @@ from samba.tests.krb5.rfc4120_constants import ( KDC_ERR_SKEW, KDC_ERR_UNKNOWN_CRITICAL_FAST_OPTIONS, KERB_ERR_TYPE_EXTENDED, + KRB_AP_REP, KRB_AP_REQ, KRB_AS_REP, KRB_AS_REQ, @@ -61,6 +66,7 @@ from samba.tests.krb5.rfc4120_constants import ( KRB_TGS_REQ, KU_AP_REQ_AUTH, KU_AS_REP_ENC_PART, + KU_AP_REQ_ENC_PART, KU_AS_REQ, KU_ENC_CHALLENGE_KDC, KU_FAST_ENC, @@ -76,6 +82,7 @@ from samba.tests.krb5.rfc4120_constants import ( KU_TGS_REQ_AUTH_DAT_SESSION, KU_TGS_REQ_AUTH_DAT_SUBKEY, KU_TICKET, + NT_PRINCIPAL, NT_SRV_INST, NT_WELLKNOWN, PADATA_ENCRYPTED_CHALLENGE, @@ -525,6 +532,10 @@ class KerberosTicketCreds: class RawKerberosTest(TestCaseInTempDir): """A raw Kerberos Test case.""" + class KpasswdMode(Enum): + SET = object() + CHANGE = object() + pac_checksum_types = {krb5pac.PAC_TYPE_SRV_CHECKSUM, krb5pac.PAC_TYPE_KDC_CHECKSUM, krb5pac.PAC_TYPE_TICKET_CHECKSUM} @@ -1931,6 +1942,224 @@ class RawKerberosTest(TestCaseInTempDir): return msg + def get_enc_part(self, obj, key, usage): + self.assertElementEqual(obj, 'pvno', 5) + + enc_part = obj['enc-part'] + self.assertElementEqual(enc_part, 'etype', key.etype) + self.assertElementKVNO(enc_part, 'kvno', key.kvno) + + enc_part = key.decrypt(usage, enc_part['cipher']) + + return enc_part + + def kpasswd_exchange(self, + ticket, + new_password, + expected_code, + expected_msg, + mode, + target_princ=None, + target_realm=None, + ap_options=None, + send_seq_number=True): + if mode is self.KpasswdMode.SET: + version = 0xff80 + user_data = self.ChangePasswdDataMS_create(new_password, + target_princ, + target_realm) + elif mode is self.KpasswdMode.CHANGE: + self.assertIsNone(target_princ, + 'target_princ only valid for pw set') + self.assertIsNone(target_realm, + 'target_realm only valid for pw set') + + version = 1 + user_data = new_password.encode('utf-8') + else: + self.fail(f'invalid mode {mode}') + + subkey = self.RandomKey(kcrypto.Enctype.AES256) + + if ap_options is None: + ap_options = '0' + ap_options = str(krb5_asn1.APOptions(ap_options)) + + kdc_exchange_dict = { + 'tgt': ticket, + 'authenticator_subkey': subkey, + 'auth_data': None, + 'ap_options': ap_options, + } + + if send_seq_number: + seq_number = random.randint(0, 0xfffffffe) + else: + seq_number = None + + ap_req = self.generate_ap_req(kdc_exchange_dict, + None, + req_body=None, + armor=False, + usage=KU_AP_REQ_AUTH, + seq_number=seq_number) + + self.connect(self.host, port=464) + self.assertIsNotNone(self.s) + + family = self.s.family + + if family == socket.AF_INET: + addr_type = 2 # IPv4 + elif family == socket.AF_INET6: + addr_type = 24 # IPv6 + else: + self.fail(f'unknown family {family}') + + def create_address(ip): + return { + 'addr-type': addr_type, + 'address': socket.inet_pton(family, ip), + } + + local_ip = self.s.getsockname()[0] + local_address = create_address(local_ip) + + # remote_ip = self.s.getpeername()[0] + # remote_address = create_address(remote_ip) + + # TODO: due to a bug (?), MIT Kerberos will not accept the request + # unless r-address is set to our _local_ address. Heimdal, on the other + # hand, requires the r-address is set to the remote address (as + # expected). To avoid problems, avoid sending r-address for now. + remote_address = None + + msg = self.kpasswd_create(subkey, + user_data, + version, + seq_number, + ap_req, + local_address, + remote_address) + + self.send_msg(msg) + rep_pdu = self.recv_pdu_raw() + + self._disconnect('transaction done') + + self.assertIsNotNone(rep_pdu) + + header = rep_pdu[:6] + reply = rep_pdu[6:] + + reply_len = (header[0] << 8) | header[1] + reply_version = (header[2] << 8) | header[3] + ap_rep_len = (header[4] << 8) | header[5] + + self.assertEqual(reply_len, len(rep_pdu)) + self.assertEqual(1, reply_version) # KRB5_KPASSWD_VERS_CHANGEPW + self.assertLess(ap_rep_len, reply_len) + + self.assertNotEqual(0x7e, rep_pdu[1]) + self.assertNotEqual(0x5e, rep_pdu[1]) + + if ap_rep_len: + # We received an AP-REQ and KRB-PRIV as a response. This may or may + # not indicate an error, depending on the status code. + ap_rep = reply[:ap_rep_len] + krb_priv = reply[ap_rep_len:] + + key = ticket.session_key + + ap_rep = self.der_decode(ap_rep, asn1Spec=krb5_asn1.AP_REP()) + self.assertElementEqual(ap_rep, 'msg-type', KRB_AP_REP) + enc_part = self.get_enc_part(ap_rep, key, KU_AP_REQ_ENC_PART) + enc_part = self.der_decode( + enc_part, asn1Spec=krb5_asn1.EncAPRepPart()) + + self.assertElementPresent(enc_part, 'ctime') + self.assertElementPresent(enc_part, 'cusec') + # self.assertElementMissing(enc_part, 'subkey') # TODO + # self.assertElementPresent(enc_part, 'seq-number') # TODO + + try: + krb_priv = self.der_decode(krb_priv, asn1Spec=krb5_asn1.KRB_PRIV()) + except PyAsn1Error: + self.fail() + + self.assertElementEqual(krb_priv, 'msg-type', KRB_PRIV) + priv_enc_part = self.get_enc_part(krb_priv, subkey, KU_KRB_PRIV) + priv_enc_part = self.der_decode( + priv_enc_part, asn1Spec=krb5_asn1.EncKrbPrivPart()) + + self.assertElementMissing(priv_enc_part, 'timestamp') + self.assertElementMissing(priv_enc_part, 'usec') + # self.assertElementPresent(priv_enc_part, 'seq-number') # TODO + # self.assertElementEqual(priv_enc_part, 's-address', remote_address) # TODO + # self.assertElementMissing(priv_enc_part, 'r-address') # TODO + + result_data = priv_enc_part['user-data'] + else: + # We received a KRB-ERROR as a response, indicating an error. + krb_error = self.der_decode(reply, asn1Spec=krb5_asn1.KRB_ERROR()) + + sname = self.PrincipalName_create( + name_type=NT_PRINCIPAL, + names=['kadmin', 'changepw']) + realm = self.get_krbtgt_creds().get_realm().upper() + + self.assertElementEqual(krb_error, 'pvno', 5) + self.assertElementEqual(krb_error, 'msg-type', KRB_ERROR) + self.assertElementMissing(krb_error, 'ctime') + self.assertElementMissing(krb_error, 'usec') + self.assertElementPresent(krb_error, 'stime') + self.assertElementPresent(krb_error, 'susec') + + error_code = krb_error['error-code'] + if isinstance(expected_code, int): + self.assertEqual(error_code, expected_code) + else: + self.assertIn(error_code, expected_code) + + self.assertElementMissing(krb_error, 'crealm') + self.assertElementMissing(krb_error, 'cname') + self.assertElementEqual(krb_error, 'realm', realm.encode('utf-8')) + self.assertElementEqualPrincipal(krb_error, 'sname', sname) + self.assertElementMissing(krb_error, 'e-text') + + result_data = krb_error['e-data'] + + status = result_data[:2] + message = result_data[2:] + + status_code = (status[0] << 8) | status[1] + if isinstance(expected_code, int): + self.assertEqual(status_code, expected_code) + else: + self.assertIn(status_code, expected_code) + + if not message: + self.assertEqual(0, status_code, + 'got an error result, but no message') + return + + # Check the first character of the message. + if message[0]: + if isinstance(expected_msg, bytes): + self.assertEqual(message, expected_msg) + else: + self.assertIn(message, expected_msg) + else: + # We got AD password policy information. + self.assertEqual(30, len(message)) + + (empty_bytes, + min_length, + history_length, + properties, + expire_time, + min_age) = struct.unpack('>HIIIQQ', message) + def _generic_kdc_exchange(self, kdc_exchange_dict, # required cname=None, # optional @@ -2041,7 +2270,7 @@ class RawKerberosTest(TestCaseInTempDir): self.assertIsNotNone(generate_fast_fn) fast_ap_req = generate_fast_armor_fn(kdc_exchange_dict, callback_dict, - req_body, + None, armor=True) fast_armor_type = kdc_exchange_dict['fast_armor_type'] @@ -3438,31 +3667,39 @@ class RawKerberosTest(TestCaseInTempDir): kdc_exchange_dict, _callback_dict, req_body, - armor): + armor, + usage=None, + seq_number=None): + req_body_checksum = None + if armor: + self.assertIsNone(req_body) + tgt = kdc_exchange_dict['armor_tgt'] authenticator_subkey = kdc_exchange_dict['armor_subkey'] - - req_body_checksum = None else: tgt = kdc_exchange_dict['tgt'] authenticator_subkey = kdc_exchange_dict['authenticator_subkey'] - body_checksum_type = kdc_exchange_dict['body_checksum_type'] - req_body_blob = self.der_encode(req_body, - asn1Spec=krb5_asn1.KDC_REQ_BODY()) + if req_body is not None: + body_checksum_type = kdc_exchange_dict['body_checksum_type'] - req_body_checksum = self.Checksum_create(tgt.session_key, - KU_TGS_REQ_AUTH_CKSUM, - req_body_blob, - ctype=body_checksum_type) + req_body_blob = self.der_encode( + req_body, asn1Spec=krb5_asn1.KDC_REQ_BODY()) + + req_body_checksum = self.Checksum_create( + tgt.session_key, + KU_TGS_REQ_AUTH_CKSUM, + req_body_blob, + ctype=body_checksum_type) auth_data = kdc_exchange_dict['auth_data'] subkey_obj = None if authenticator_subkey is not None: subkey_obj = authenticator_subkey.export_obj() - seq_number = random.randint(0, 0xfffffffe) + if seq_number is None: + seq_number = random.randint(0, 0xfffffffe) (ctime, cusec) = self.get_KerberosTimeWithUsec() authenticator_obj = self.Authenticator_create( crealm=tgt.crealm, @@ -3477,7 +3714,8 @@ class RawKerberosTest(TestCaseInTempDir): authenticator_obj, asn1Spec=krb5_asn1.Authenticator()) - usage = KU_AP_REQ_AUTH if armor else KU_TGS_REQ_AUTH + if usage is None: + usage = KU_AP_REQ_AUTH if armor else KU_TGS_REQ_AUTH authenticator = self.EncryptedData_create(tgt.session_key, usage, authenticator_blob) -- 2.35.0 From 9287f86251ebebe621db71e1454a354e23c5aa35 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Thu, 26 May 2022 16:35:03 +1200 Subject: [PATCH 15/43] CVE-2022-32744 selftest: Specify Administrator kvno for Python krb5 tests BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- source4/selftest/tests.py | 1 + 1 file changed, 1 insertion(+) diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index 32de26a9d23..fe0b041fdbc 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -996,6 +996,7 @@ krb5_environ = { 'SERVICE_USERNAME': '$SERVER', 'ADMIN_USERNAME': '$DC_USERNAME', 'ADMIN_PASSWORD': '$DC_PASSWORD', + 'ADMIN_KVNO': '1', 'FOR_USER': '$DC_USERNAME', 'STRICT_CHECKING':'0', 'FAST_SUPPORT': have_fast_support, -- 2.35.0 From 5c1648d5b47d9b7e8e569c06bdce62bb82d613a9 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 31 May 2022 19:23:06 +1200 Subject: [PATCH 16/43] CVE-2022-2031 tests/krb5: Consider kadmin/* principals as TGS for MIT KRB5 >= 1.20 With MIT Kerberos >= 1.20, we should not expect a ticket checksum in tickets to principals such as kpasswd/changepw, as they are encrypted with the krbtgt's key. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- python/samba/tests/krb5/kdc_base_test.py | 5 ++++- python/samba/tests/krb5/raw_testcase.py | 28 ++++++++++++++++++++---- source4/selftest/tests.py | 6 +++++ 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/python/samba/tests/krb5/kdc_base_test.py b/python/samba/tests/krb5/kdc_base_test.py index acd35a2db14..c3a8fb764d4 100644 --- a/python/samba/tests/krb5/kdc_base_test.py +++ b/python/samba/tests/krb5/kdc_base_test.py @@ -1514,9 +1514,12 @@ class KDCBaseTest(RawKerberosTest): else: krbtgt_creds = self.get_krbtgt_creds() krbtgt_key = self.TicketDecryptionKey_from_creds(krbtgt_creds) + + expect_ticket_checksum = (self.tkt_sig_support + and not self.is_tgs_principal(sname)) self.verify_ticket(service_ticket_creds, krbtgt_key, service_ticket=True, expect_pac=expect_pac, - expect_ticket_checksum=self.tkt_sig_support) + expect_ticket_checksum=expect_ticket_checksum) self.tkt_cache[cache_key] = service_ticket_creds diff --git a/python/samba/tests/krb5/raw_testcase.py b/python/samba/tests/krb5/raw_testcase.py index b22617c3882..4ef37c51222 100644 --- a/python/samba/tests/krb5/raw_testcase.py +++ b/python/samba/tests/krb5/raw_testcase.py @@ -657,6 +657,12 @@ class RawKerberosTest(TestCaseInTempDir): padata_checking = '1' cls.padata_checking = bool(int(padata_checking)) + kadmin_is_tgs = samba.tests.env_get_var_value('KADMIN_IS_TGS', + allow_missing=True) + if kadmin_is_tgs is None: + kadmin_is_tgs = '0' + cls.kadmin_is_tgs = bool(int(kadmin_is_tgs)) + def setUp(self): super().setUp() self.do_asn1_print = False @@ -3057,8 +3063,8 @@ class RawKerberosTest(TestCaseInTempDir): self.assertIsNotNone(ticket_decryption_key) if ticket_decryption_key is not None: - service_ticket = (not self.is_tgs(expected_sname) - and rep_msg_type == KRB_TGS_REP) + service_ticket = (rep_msg_type == KRB_TGS_REP + and not self.is_tgs_principal(expected_sname)) self.verify_ticket(ticket_creds, krbtgt_keys, service_ticket=service_ticket, expect_pac=expect_pac, @@ -3098,8 +3104,9 @@ class RawKerberosTest(TestCaseInTempDir): expected_types.append(krb5pac.PAC_TYPE_DEVICE_INFO) expected_types.append(krb5pac.PAC_TYPE_DEVICE_CLAIMS_INFO) - if not self.is_tgs(expected_sname) and rep_msg_type == KRB_TGS_REP: - expected_types.append(krb5pac.PAC_TYPE_TICKET_CHECKSUM) + if rep_msg_type == KRB_TGS_REP: + if not self.is_tgs_principal(expected_sname): + expected_types.append(krb5pac.PAC_TYPE_TICKET_CHECKSUM) require_strict = {krb5pac.PAC_TYPE_CLIENT_CLAIMS_INFO, krb5pac.PAC_TYPE_DEVICE_INFO, @@ -4244,6 +4251,19 @@ class RawKerberosTest(TestCaseInTempDir): krb5pac.PAC_TYPE_KDC_CHECKSUM: krbtgt_key } + def is_tgs_principal(self, principal): + if self.is_tgs(principal): + return True + + if self.kadmin_is_tgs and self.is_kadmin(principal): + return True + + return False + + def is_kadmin(self, principal): + name = principal['name-string'][0] + return name in ('kadmin', b'kadmin') + def is_tgs(self, principal): name = principal['name-string'][0] return name in ('krbtgt', b'krbtgt') diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index fe0b041fdbc..bb4566779c2 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -988,6 +988,11 @@ if ('SAMBA4_USES_HEIMDAL' in config_hash or else: tkt_sig_support = 0 +if 'HAVE_MIT_KRB5_1_20' in config_hash: + kadmin_is_tgs = 1 +else: + kadmin_is_tgs = 0 + expect_pac = int('SAMBA4_USES_HEIMDAL' in config_hash) extra_pac_buffers = int('SAMBA4_USES_HEIMDAL' in config_hash) check_cname = int('SAMBA4_USES_HEIMDAL' in config_hash) @@ -1007,6 +1012,7 @@ krb5_environ = { 'EXPECT_EXTRA_PAC_BUFFERS': extra_pac_buffers, 'CHECK_CNAME': check_cname, 'CHECK_PADATA': check_padata, + 'KADMIN_IS_TGS': kadmin_is_tgs, } planoldpythontestsuite("none", "samba.tests.krb5.kcrypto") planoldpythontestsuite("ad_dc_default", "samba.tests.krb5.simple_tests", -- 2.35.0 From e91880c72c8727eeed7e51dd108d3661ffea1bab Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 19:59:16 +1200 Subject: [PATCH 17/43] CVE-2022-2031 tests/krb5: Add tests for kpasswd service BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- python/samba/tests/krb5/kdc_base_test.py | 4 +- python/samba/tests/krb5/kpasswd_tests.py | 1021 ++++++++++++++++++++++ python/samba/tests/krb5/raw_testcase.py | 8 + python/samba/tests/usage.py | 1 + selftest/knownfail_heimdal_kdc | 26 + selftest/knownfail_mit_kdc | 26 + selftest/knownfail_mit_kdc_1_20 | 5 + source4/selftest/tests.py | 4 + 8 files changed, 1094 insertions(+), 1 deletion(-) create mode 100755 python/samba/tests/krb5/kpasswd_tests.py diff --git a/python/samba/tests/krb5/kdc_base_test.py b/python/samba/tests/krb5/kdc_base_test.py index c3a8fb764d4..2254b1daae4 100644 --- a/python/samba/tests/krb5/kdc_base_test.py +++ b/python/samba/tests/krb5/kdc_base_test.py @@ -1722,7 +1722,9 @@ class KDCBaseTest(RawKerberosTest): authenticator_subkey = self.RandomKey(kcrypto.Enctype.AES256) if expect_error: - expected_error_mode = KDC_ERR_TGT_REVOKED + expected_error_mode = expect_error + if expected_error_mode is True: + expected_error_mode = KDC_ERR_TGT_REVOKED check_error_fn = self.generic_check_kdc_error check_rep_fn = None else: diff --git a/python/samba/tests/krb5/kpasswd_tests.py b/python/samba/tests/krb5/kpasswd_tests.py new file mode 100755 index 00000000000..3a6c7d818dc --- /dev/null +++ b/python/samba/tests/krb5/kpasswd_tests.py @@ -0,0 +1,1021 @@ +#!/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 . +# + +import os +import sys + +from functools import partial + +from samba import generate_random_password, unix2nttime +from samba.dcerpc import krb5pac, security +from samba.sd_utils import SDUtils + +from samba.tests.krb5.kdc_base_test import KDCBaseTest +from samba.tests.krb5.rfc4120_constants import ( + KDC_ERR_TGT_REVOKED, + KDC_ERR_TKT_EXPIRED, + KPASSWD_ACCESSDENIED, + KPASSWD_HARDERROR, + KPASSWD_INITIAL_FLAG_NEEDED, + KPASSWD_MALFORMED, + KPASSWD_SOFTERROR, + KPASSWD_SUCCESS, + NT_PRINCIPAL, + NT_SRV_INST, +) + +sys.path.insert(0, 'bin/python') +os.environ['PYTHONUNBUFFERED'] = '1' + +global_asn1_print = False +global_hexdump = False + + +# Note: these tests do not pass on Windows, which returns different error codes +# to the ones we have chosen, and does not always return additional error data. +class KpasswdTests(KDCBaseTest): + + def setUp(self): + super().setUp() + self.do_asn1_print = global_asn1_print + self.do_hexdump = global_hexdump + + samdb = self.get_samdb() + + # 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 _get_creds(self, expired=False): + opts = { + 'expired_password': expired + } + + # Create the account. + creds = self.get_cached_creds(account_type=self.AccountType.USER, + opts=opts, + use_cache=False) + + return creds + + def issued_by_rodc(self, ticket): + krbtgt_creds = self.get_mock_rodc_krbtgt_creds() + + krbtgt_key = self.TicketDecryptionKey_from_creds(krbtgt_creds) + checksum_keys = { + krb5pac.PAC_TYPE_KDC_CHECKSUM: krbtgt_key, + } + + return self.modified_ticket( + ticket, + new_ticket_key=krbtgt_key, + checksum_keys=checksum_keys) + + def get_kpasswd_sname(self): + return self.PrincipalName_create(name_type=NT_PRINCIPAL, + names=['kadmin', 'changepw']) + + def get_ticket_lifetime(self, ticket): + enc_part = ticket.ticket_private + + authtime = enc_part['authtime'] + starttime = enc_part.get('starttime', authtime) + endtime = enc_part['endtime'] + + starttime = self.get_EpochFromKerberosTime(starttime) + endtime = self.get_EpochFromKerberosTime(endtime) + + return endtime - starttime + + def add_requester_sid(self, pac, sid): + pac_buffers = pac.buffers + + buffer_types = [pac_buffer.type for pac_buffer in pac_buffers] + self.assertNotIn(krb5pac.PAC_TYPE_REQUESTER_SID, buffer_types) + + requester_sid = krb5pac.PAC_REQUESTER_SID() + requester_sid.sid = security.dom_sid(sid) + + requester_sid_buffer = krb5pac.PAC_BUFFER() + requester_sid_buffer.type = krb5pac.PAC_TYPE_REQUESTER_SID + requester_sid_buffer.info = requester_sid + + pac_buffers.append(requester_sid_buffer) + + pac.buffers = pac_buffers + pac.num_buffers += 1 + + return pac + + # Test setting a password with kpasswd. + def test_kpasswd_set(self): + # Create an account for testing. + creds = self._get_creds() + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + expected_code = KPASSWD_SUCCESS + expected_msg = b'Password changed' + + # Set the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET) + + # Test the newly set password. + creds.update_password(new_password) + self.get_tgt(creds, fresh=True) + + # Test changing a password with kpasswd. + def test_kpasswd_change(self): + # Create an account for testing. + creds = self._get_creds() + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + expected_code = KPASSWD_SUCCESS + expected_msg = b'Password changed' + + # Change the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.CHANGE) + + # Test the newly set password. + creds.update_password(new_password) + self.get_tgt(creds, fresh=True) + + # Test kpasswd without setting the canonicalize option. + def test_kpasswd_no_canonicalize(self): + # Create an account for testing. + creds = self._get_creds() + + sname = self.get_kpasswd_sname() + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=sname, + kdc_options='0') + + expected_code = KPASSWD_SUCCESS + expected_msg = b'Password changed' + + # Set the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET) + + creds.update_password(new_password) + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=sname, + kdc_options='0') + + # Change the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.CHANGE) + + # Test kpasswd with the canonicalize option reset and a non-canonical + # (by conversion to title case) realm. + def test_kpasswd_no_canonicalize_realm_case(self): + # Create an account for testing. + creds = self._get_creds() + + sname = self.get_kpasswd_sname() + realm = creds.get_realm().capitalize() # We use a title-cased realm. + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=sname, + realm=realm, + kdc_options='0') + + expected_code = KPASSWD_SUCCESS + expected_msg = b'Password changed' + + # Set the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET) + + creds.update_password(new_password) + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=sname, + realm=realm, + kdc_options='0') + + # Change the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.CHANGE) + + # Test kpasswd with the canonicalize option set. + def test_kpasswd_canonicalize(self): + # Create an account for testing. + creds = self._get_creds() + + # Get an initial ticket to kpasswd. We set the canonicalize flag here. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='canonicalize') + + expected_code = KPASSWD_SUCCESS + expected_msg = b'Password changed' + + # Set the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET) + + creds.update_password(new_password) + + # Get an initial ticket to kpasswd. We set the canonicalize flag here. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='canonicalize') + + # Change the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.CHANGE) + + # Test kpasswd with the canonicalize option set and a non-canonical (by + # conversion to title case) realm. + def test_kpasswd_canonicalize_realm_case(self): + # Create an account for testing. + creds = self._get_creds() + + sname = self.get_kpasswd_sname() + realm = creds.get_realm().capitalize() # We use a title-cased realm. + + # Get an initial ticket to kpasswd. We set the canonicalize flag here. + ticket = self.get_tgt(creds, sname=sname, + realm=realm, + kdc_options='canonicalize') + + expected_code = KPASSWD_SUCCESS + expected_msg = b'Password changed' + + # Set the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET) + + creds.update_password(new_password) + + # Get an initial ticket to kpasswd. We set the canonicalize flag here. + ticket = self.get_tgt(creds, sname=sname, + realm=realm, + kdc_options='canonicalize') + + # Change the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.CHANGE) + + # Test kpasswd rejects a password that does not meet complexity + # requirements. + def test_kpasswd_too_weak(self): + # Create an account for testing. + creds = self._get_creds() + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + expected_code = KPASSWD_SOFTERROR + expected_msg = b'Password does not meet complexity requirements' + + # Set the password. + new_password = 'password' + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET) + + # Change the password. + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.CHANGE) + + # Test kpasswd rejects an empty new password. + def test_kpasswd_empty(self): + # Create an account for testing. + creds = self._get_creds() + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + expected_code = KPASSWD_SOFTERROR, KPASSWD_HARDERROR + expected_msg = (b'Password too short, password must be at least 7 ' + b'characters long.', + b'String conversion failed!') + + # Set the password. + new_password = '' + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET) + + expected_code = KPASSWD_HARDERROR + expected_msg = b'String conversion failed!' + + # Change the password. + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.CHANGE) + + # Test kpasswd rejects a request that does not include a random sequence + # number. + def test_kpasswd_no_seq_number(self): + # Create an account for testing. + creds = self._get_creds() + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + expected_code = KPASSWD_HARDERROR + expected_msg = b'gensec_unwrap failed - NT_STATUS_ACCESS_DENIED\n' + + # Set the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET, + send_seq_number=False) + + # Change the password. + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.CHANGE, + send_seq_number=False) + + # Test kpasswd rejects a ticket issued by an RODC. + def test_kpasswd_from_rodc(self): + # Create an account for testing. + creds = self._get_creds() + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + # Have the ticket be issued by the RODC. + ticket = self.issued_by_rodc(ticket) + + expected_code = KPASSWD_HARDERROR + expected_msg = b'gensec_update failed - NT_STATUS_LOGON_FAILURE\n' + + # Set the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET) + + # Change the password. + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.CHANGE) + + # Test setting a password, specifying the principal of the target user. + def test_kpasswd_set_target_princ_only(self): + # Create an account for testing. + creds = self._get_creds() + username = creds.get_username() + + cname = self.PrincipalName_create(name_type=NT_PRINCIPAL, + names=username.split('/')) + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + expected_code = KPASSWD_MALFORMED + expected_msg = (b'Realm and principal must be both present, or ' + b'neither present', + b'Failed to decode packet') + + # Change the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET, + target_princ=cname) + + # Test that kpasswd rejects a password set specifying only the realm of the + # target user. + def test_kpasswd_set_target_realm_only(self): + # Create an account for testing. + creds = self._get_creds() + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + expected_code = KPASSWD_MALFORMED, KPASSWD_ACCESSDENIED + expected_msg = (b'Realm and principal must be both present, or ' + b'neither present', + b'Failed to decode packet', + b'No such user when changing password') + + # Change the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET, + target_realm=creds.get_realm()) + + # Show that a user cannot set a password, specifying both principal and + # realm of the target user, without having control access. + def test_kpasswd_set_target_princ_and_realm_no_access(self): + # Create an account for testing. + creds = self._get_creds() + username = creds.get_username() + + cname = self.PrincipalName_create(name_type=NT_PRINCIPAL, + names=username.split('/')) + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + expected_code = KPASSWD_ACCESSDENIED + expected_msg = b'Not permitted to change password' + + # Change the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET, + target_princ=cname, + target_realm=creds.get_realm()) + + # Test setting a password, specifying both principal and realm of the + # target user, whem the user has control access on their account. + def test_kpasswd_set_target_princ_and_realm_access(self): + # Create an account for testing. + creds = self._get_creds() + username = creds.get_username() + tgt = self.get_tgt(creds) + + cname = self.PrincipalName_create(name_type=NT_PRINCIPAL, + names=username.split('/')) + + samdb = self.get_samdb() + sd_utils = SDUtils(samdb) + + user_dn = creds.get_dn() + user_sid = self.get_objectSid(samdb, user_dn) + + # Give the user control access on their account. + ace = f'(A;;CR;;;{user_sid})' + sd_utils.dacl_add_ace(user_dn, ace) + + # Get a non-initial ticket to kpasswd. Since we have the right to + # change the account's password, we don't need an initial ticket. + krbtgt_creds = self.get_krbtgt_creds() + ticket = self.get_service_ticket(tgt, + krbtgt_creds, + service='kadmin', + target_name='changepw', + kdc_options='0') + + expected_code = KPASSWD_SUCCESS + expected_msg = b'Password changed' + + # Change the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET, + target_princ=cname, + target_realm=creds.get_realm()) + + # Test setting a password when the existing password has expired. + def test_kpasswd_set_expired_password(self): + # Create an account for testing, with an expired password. + creds = self._get_creds(expired=True) + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + expected_code = KPASSWD_SUCCESS + expected_msg = b'Password changed' + + # Set the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET) + + # Test changing a password when the existing password has expired. + def test_kpasswd_change_expired_password(self): + # Create an account for testing, with an expired password. + creds = self._get_creds(expired=True) + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + expected_code = KPASSWD_SUCCESS + expected_msg = b'Password changed' + + # Change the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.CHANGE) + + # Check the lifetime of a kpasswd ticket is not more than two minutes. + def test_kpasswd_ticket_lifetime(self): + # Create an account for testing. + creds = self._get_creds() + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + # Check the lifetime of the ticket is equal to two minutes. + lifetime = self.get_ticket_lifetime(ticket) + self.assertEqual(2 * 60, lifetime) + + # Ensure we cannot perform a TGS-REQ with a kpasswd ticket. + def test_kpasswd_ticket_tgs(self): + creds = self.get_client_creds() + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + # Change the sname of the ticket to match that of a TGT. + realm = creds.get_realm() + krbtgt_sname = self.PrincipalName_create(name_type=NT_SRV_INST, + names=['krbtgt', realm]) + ticket.set_sname(krbtgt_sname) + + # Try to use that ticket to get a service ticket. + service_creds = self.get_service_creds() + + # This fails due to missing REQUESTER_SID buffer. + self._make_tgs_request(creds, service_creds, ticket, + expect_error=(KDC_ERR_TGT_REVOKED, + KDC_ERR_TKT_EXPIRED)) + + def modify_requester_sid_time(self, ticket, sid, lifetime): + # Get the krbtgt key. + krbtgt_creds = self.get_krbtgt_creds() + + krbtgt_key = self.TicketDecryptionKey_from_creds(krbtgt_creds) + checksum_keys = { + krb5pac.PAC_TYPE_KDC_CHECKSUM: krbtgt_key, + } + + # Set authtime and starttime to an hour in the past, to show that they + # do not affect ticket rejection. + start_time = self.get_KerberosTime(offset=-60 * 60) + + # Set the endtime of the ticket relative to our current time, so that + # the ticket has 'lifetime' seconds remaining to live. + end_time = self.get_KerberosTime(offset=lifetime) + + # Modify the times in the ticket. + def modify_ticket_times(enc_part): + enc_part['authtime'] = start_time + if 'starttime' in enc_part: + enc_part['starttime'] = start_time + + enc_part['endtime'] = end_time + + return enc_part + + # We have to set the times in both the ticket and the PAC, otherwise + # Heimdal will complain. + def modify_pac_time(pac): + pac_buffers = pac.buffers + + for pac_buffer in pac_buffers: + if pac_buffer.type == krb5pac.PAC_TYPE_LOGON_NAME: + logon_time = self.get_EpochFromKerberosTime(start_time) + pac_buffer.info.logon_time = unix2nttime(logon_time) + break + else: + self.fail('failed to find LOGON_NAME PAC buffer') + + pac.buffers = pac_buffers + + return pac + + # Add a requester SID to show that the KDC will then accept this + # kpasswd ticket as if it were a TGT. + def modify_pac_fn(pac): + pac = self.add_requester_sid(pac, sid=sid) + pac = modify_pac_time(pac) + return pac + + # Do the actual modification. + return self.modified_ticket(ticket, + new_ticket_key=krbtgt_key, + modify_fn=modify_ticket_times, + modify_pac_fn=modify_pac_fn, + checksum_keys=checksum_keys) + + # Ensure we cannot perform a TGS-REQ with a kpasswd ticket containing a + # requester SID and having a remaining lifetime of two minutes. + def test_kpasswd_ticket_requester_sid_tgs(self): + creds = self.get_client_creds() + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + # Change the sname of the ticket to match that of a TGT. + realm = creds.get_realm() + krbtgt_sname = self.PrincipalName_create(name_type=NT_SRV_INST, + names=['krbtgt', realm]) + ticket.set_sname(krbtgt_sname) + + # Get the user's SID. + samdb = self.get_samdb() + + user_dn = creds.get_dn() + user_sid = self.get_objectSid(samdb, user_dn) + + # Modify the ticket to add a requester SID and give it two minutes to + # live. + ticket = self.modify_requester_sid_time(ticket, + sid=user_sid, + lifetime=2 * 60) + + # Try to use that ticket to get a service ticket. + service_creds = self.get_service_creds() + + # This fails due to the lifetime being too short. + self._make_tgs_request(creds, service_creds, ticket, + expect_error=KDC_ERR_TKT_EXPIRED) + + # Show we can perform a TGS-REQ with a kpasswd ticket containing a + # requester SID if the remaining lifetime exceeds two minutes. + def test_kpasswd_ticket_requester_sid_lifetime_tgs(self): + creds = self.get_client_creds() + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + # Change the sname of the ticket to match that of a TGT. + realm = creds.get_realm() + krbtgt_sname = self.PrincipalName_create(name_type=NT_SRV_INST, + names=['krbtgt', realm]) + ticket.set_sname(krbtgt_sname) + + # Get the user's SID. + samdb = self.get_samdb() + + user_dn = creds.get_dn() + user_sid = self.get_objectSid(samdb, user_dn) + + # Modify the ticket to add a requester SID and give it two minutes and + # ten seconds to live. + ticket = self.modify_requester_sid_time(ticket, + sid=user_sid, + lifetime=2 * 60 + 10) + + # Try to use that ticket to get a service ticket. + service_creds = self.get_service_creds() + + # This succeeds. + self._make_tgs_request(creds, service_creds, ticket, + expect_error=False) + + # Test that kpasswd rejects requests with a service ticket. + def test_kpasswd_non_initial(self): + # Create an account for testing, and get a TGT. + creds = self._get_creds() + tgt = self.get_tgt(creds) + + # Get a non-initial ticket to kpasswd. + krbtgt_creds = self.get_krbtgt_creds() + ticket = self.get_service_ticket(tgt, + krbtgt_creds, + service='kadmin', + target_name='changepw', + kdc_options='0') + + expected_code = KPASSWD_INITIAL_FLAG_NEEDED + expected_msg = b'Expected an initial ticket' + + # Set the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET) + + # Change the password. + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.CHANGE) + + # Show that kpasswd accepts requests with a service ticket modified to set + # the 'initial' flag. + def test_kpasswd_initial(self): + # Create an account for testing, and get a TGT. + creds = self._get_creds() + + krbtgt_creds = self.get_krbtgt_creds() + + # Get a service ticket, and modify it to set the 'initial' flag. + def get_ticket(): + tgt = self.get_tgt(creds, fresh=True) + + # Get a non-initial ticket to kpasswd. + ticket = self.get_service_ticket(tgt, + krbtgt_creds, + service='kadmin', + target_name='changepw', + kdc_options='0', + fresh=True) + + set_initial_flag = partial(self.modify_ticket_flag, flag='initial', + value=True) + + checksum_keys = self.get_krbtgt_checksum_key() + return self.modified_ticket(ticket, + modify_fn=set_initial_flag, + checksum_keys=checksum_keys) + + expected_code = KPASSWD_SUCCESS + expected_msg = b'Password changed' + + ticket = get_ticket() + + # Set the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET) + + creds.update_password(new_password) + ticket = get_ticket() + + # Change the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.CHANGE) + + # Test that kpasswd rejects requests where the ticket is encrypted with a + # key other than the krbtgt's. + def test_kpasswd_wrong_key(self): + # Create an account for testing. + creds = self._get_creds() + + sname = self.get_kpasswd_sname() + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=sname, + kdc_options='0') + + # Get a key belonging to the Administrator account. + admin_creds = self.get_admin_creds() + admin_key = self.TicketDecryptionKey_from_creds(admin_creds) + self.assertIsNotNone(admin_key.kvno, + 'a kvno is required to tell the DB ' + 'which key to look up.') + checksum_keys = { + krb5pac.PAC_TYPE_KDC_CHECKSUM: admin_key, + } + + # Re-encrypt the ticket using the Administrator's key. + ticket = self.modified_ticket(ticket, + new_ticket_key=admin_key, + checksum_keys=checksum_keys) + + # Set the sname of the ticket to that of the Administrator account. + admin_sname = self.PrincipalName_create(name_type=NT_PRINCIPAL, + names=['Administrator']) + ticket.set_sname(admin_sname) + + expected_code = KPASSWD_HARDERROR + expected_msg = b'gensec_update failed - NT_STATUS_LOGON_FAILURE\n' + + # Set the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET) + + # Change the password. + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.CHANGE) + + def test_kpasswd_wrong_key_service(self): + # Create an account for testing. + creds = self.get_cached_creds(account_type=self.AccountType.COMPUTER, + use_cache=False) + + sname = self.get_kpasswd_sname() + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=sname, + kdc_options='0') + + # Get a key belonging to our account. + our_key = self.TicketDecryptionKey_from_creds(creds) + self.assertIsNotNone(our_key.kvno, + 'a kvno is required to tell the DB ' + 'which key to look up.') + checksum_keys = { + krb5pac.PAC_TYPE_KDC_CHECKSUM: our_key, + } + + # Re-encrypt the ticket using our key. + ticket = self.modified_ticket(ticket, + new_ticket_key=our_key, + checksum_keys=checksum_keys) + + # Set the sname of the ticket to that of our account. + username = creds.get_username() + sname = self.PrincipalName_create(name_type=NT_PRINCIPAL, + names=username.split('/')) + ticket.set_sname(sname) + + expected_code = KPASSWD_HARDERROR + expected_msg = b'gensec_update failed - NT_STATUS_LOGON_FAILURE\n' + + # Set the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET) + + # Change the password. + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.CHANGE) + + # Test that kpasswd rejects requests where the ticket is encrypted with a + # key belonging to a server account other than the krbtgt. + def test_kpasswd_wrong_key_server(self): + # Create an account for testing. + creds = self._get_creds() + + sname = self.get_kpasswd_sname() + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=sname, + kdc_options='0') + + # Get a key belonging to the DC's account. + dc_creds = self.get_dc_creds() + dc_key = self.TicketDecryptionKey_from_creds(dc_creds) + self.assertIsNotNone(dc_key.kvno, + 'a kvno is required to tell the DB ' + 'which key to look up.') + checksum_keys = { + krb5pac.PAC_TYPE_KDC_CHECKSUM: dc_key, + } + + # Re-encrypt the ticket using the DC's key. + ticket = self.modified_ticket(ticket, + new_ticket_key=dc_key, + checksum_keys=checksum_keys) + + # Set the sname of the ticket to that of the DC's account. + dc_username = dc_creds.get_username() + dc_sname = self.PrincipalName_create(name_type=NT_PRINCIPAL, + names=dc_username.split('/')) + ticket.set_sname(dc_sname) + + expected_code = KPASSWD_HARDERROR + expected_msg = b'gensec_update failed - NT_STATUS_LOGON_FAILURE\n' + + # Set the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET) + + # Change the password. + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.CHANGE) + + +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 4ef37c51222..a7e3add60b7 100644 --- a/python/samba/tests/krb5/raw_testcase.py +++ b/python/samba/tests/krb5/raw_testcase.py @@ -510,6 +510,10 @@ class KerberosCredentials(Credentials): def get_upn(self): return self.upn + def update_password(self, password): + self.set_password(password) + self.set_kvno(self.get_kvno() + 1) + class KerberosTicketCreds: def __init__(self, ticket, session_key, @@ -528,6 +532,10 @@ class KerberosTicketCreds: self.ticket_private = ticket_private self.encpart_private = encpart_private + def set_sname(self, sname): + self.ticket['sname'] = sname + self.sname = sname + class RawKerberosTest(TestCaseInTempDir): """A raw Kerberos Test case.""" diff --git a/python/samba/tests/usage.py b/python/samba/tests/usage.py index f8aed2c67dd..346d57d2f20 100644 --- a/python/samba/tests/usage.py +++ b/python/samba/tests/usage.py @@ -111,6 +111,7 @@ EXCLUDE_USAGE = { 'python/samba/tests/krb5/test_idmap_nss.py', 'python/samba/tests/krb5/pac_align_tests.py', 'python/samba/tests/krb5/protected_users_tests.py', + 'python/samba/tests/krb5/kpasswd_tests.py', } EXCLUDE_HELP = { diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc index 4ae27eacb09..90da53ada61 100644 --- a/selftest/knownfail_heimdal_kdc +++ b/selftest/knownfail_heimdal_kdc @@ -54,3 +54,29 @@ ^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 +# +# Kpasswd tests +# +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_canonicalize.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_canonicalize_realm_case.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_change.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_change_expired_password.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_empty.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_from_rodc.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_initial.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_canonicalize.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_canonicalize_realm_case.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_seq_number.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_non_initial.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_expired_password.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_target_princ_and_realm_access.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_target_princ_and_realm_no_access.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_target_princ_only.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_target_realm_only.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_lifetime.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_requester_sid_tgs.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_too_weak.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_server.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_service.ad_dc diff --git a/selftest/knownfail_mit_kdc b/selftest/knownfail_mit_kdc index 9b55627bbc8..fcf96b68597 100644 --- a/selftest/knownfail_mit_kdc +++ b/selftest/knownfail_mit_kdc @@ -433,3 +433,29 @@ 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 +# +# Kpasswd tests +# +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_canonicalize.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_canonicalize_realm_case.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_change.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_change_expired_password.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_empty.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_from_rodc.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_initial.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_canonicalize.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_canonicalize_realm_case.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_seq_number.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_non_initial.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_expired_password.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_target_princ_and_realm_access.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_target_princ_and_realm_no_access.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_target_princ_only.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_target_realm_only.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_lifetime.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_requester_sid_tgs.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_too_weak.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_server.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_service.ad_dc diff --git a/selftest/knownfail_mit_kdc_1_20 b/selftest/knownfail_mit_kdc_1_20 index 4a47ab974ae..fe6b526b9ac 100644 --- a/selftest/knownfail_mit_kdc_1_20 +++ b/selftest/knownfail_mit_kdc_1_20 @@ -7,3 +7,8 @@ ^samba.tests.krb5.fast_tests.samba.tests.krb5.fast_tests.FAST_Tests.test_simple_as_req_self\( ^samba.tests.krb5.fast_tests.samba.tests.krb5.fast_tests.FAST_Tests.test_simple_as_req_self_pac_request_none\( ^samba.tests.krb5.fast_tests.samba.tests.krb5.fast_tests.FAST_Tests.test_simple_as_req_self_pac_request_true\( +# +# Kpasswd tests +# +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_requester_sid_lifetime_tgs.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_tgs.ad_dc diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index bb4566779c2..cf5db4babae 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -1700,6 +1700,10 @@ planoldpythontestsuite( 'ad_dc:local', 'samba.tests.krb5.protected_users_tests', environ=krb5_environ) +planoldpythontestsuite( + 'ad_dc', + 'samba.tests.krb5.kpasswd_tests', + environ=krb5_environ) for env in [ 'vampire_dc', -- 2.35.0 From a788b84a6dae1c2d63b845b59be0462d88ff1421 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Fri, 27 May 2022 19:21:06 +1200 Subject: [PATCH 18/43] CVE-2022-2031 s4:kpasswd: Correctly generate error strings The error_data we create already has an explicit length, and should not be zero-terminated, so we omit the trailing null byte. Previously, Heimdal builds would leave a superfluous trailing null byte on error strings, while MIT builds would omit the final character. The two bytes added to the string's length are for the prepended error code. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- selftest/knownfail_heimdal_kdc | 12 ------------ selftest/knownfail_mit_kdc | 15 --------------- selftest/knownfail_mit_kdc_1_20 | 15 +++++++++++++++ source4/kdc/kpasswd-helper.c | 13 ++++++------- 4 files changed, 21 insertions(+), 34 deletions(-) diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc index 90da53ada61..22d5656e5fb 100644 --- a/selftest/knownfail_heimdal_kdc +++ b/selftest/knownfail_heimdal_kdc @@ -59,24 +59,12 @@ # ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_canonicalize.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_canonicalize_realm_case.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_change.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_change_expired_password.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_empty.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_from_rodc.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_initial.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_canonicalize.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_canonicalize_realm_case.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_seq_number.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_non_initial.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_expired_password.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_target_princ_and_realm_access.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_target_princ_and_realm_no_access.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_target_princ_only.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_target_realm_only.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_lifetime.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_requester_sid_tgs.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_too_weak.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_server.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_service.ad_dc diff --git a/selftest/knownfail_mit_kdc b/selftest/knownfail_mit_kdc index fcf96b68597..dfe9a5c212e 100644 --- a/selftest/knownfail_mit_kdc +++ b/selftest/knownfail_mit_kdc @@ -436,26 +436,11 @@ samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_ # # Kpasswd tests # -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_canonicalize.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_canonicalize_realm_case.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_change.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_change_expired_password.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_empty.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_from_rodc.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_initial.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_canonicalize.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_canonicalize_realm_case.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_seq_number.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_non_initial.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_expired_password.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_target_princ_and_realm_access.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_target_princ_and_realm_no_access.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_target_princ_only.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_target_realm_only.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_lifetime.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_requester_sid_tgs.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_too_weak.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_server.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_service.ad_dc diff --git a/selftest/knownfail_mit_kdc_1_20 b/selftest/knownfail_mit_kdc_1_20 index fe6b526b9ac..f886c360381 100644 --- a/selftest/knownfail_mit_kdc_1_20 +++ b/selftest/knownfail_mit_kdc_1_20 @@ -10,5 +10,20 @@ # # Kpasswd tests # +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_canonicalize.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_change.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_change_expired_password.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_from_rodc.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_initial.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_canonicalize.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_seq_number.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_expired_password.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_target_princ_and_realm_access.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_target_princ_and_realm_no_access.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_target_princ_only.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_target_realm_only.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_requester_sid_lifetime_tgs.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_tgs.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_too_weak.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key.ad_dc diff --git a/source4/kdc/kpasswd-helper.c b/source4/kdc/kpasswd-helper.c index ffd0a1ef060..83d2dabad1f 100644 --- a/source4/kdc/kpasswd-helper.c +++ b/source4/kdc/kpasswd-helper.c @@ -48,17 +48,16 @@ bool kpasswd_make_error_reply(TALLOC_CTX *mem_ctx, } /* - * The string 's' has two terminating nul-bytes which are also - * reflected by 'slen'. Normally Kerberos doesn't expect that strings - * are nul-terminated, but Heimdal does! + * The string 's' has one terminating nul-byte which is also + * reflected by 'slen'. We subtract it from the length. */ -#ifndef SAMBA4_USES_HEIMDAL - if (slen < 2) { + if (slen < 1) { talloc_free(s); return false; } - slen -= 2; -#endif + slen--; + + /* Two bytes are added to the length to account for the error code. */ if (2 + slen < slen) { talloc_free(s); return false; -- 2.35.0 From 965d12ec87f03314168b352eb52248498f0d8697 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 18 May 2022 16:48:59 +1200 Subject: [PATCH 19/43] CVE-2022-2031 s4:kpasswd: Don't return AP-REP on failure BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- selftest/knownfail_mit_kdc | 1 - selftest/knownfail_mit_kdc_1_20 | 1 + source4/kdc/kpasswd-service.c | 2 ++ 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/selftest/knownfail_mit_kdc b/selftest/knownfail_mit_kdc index dfe9a5c212e..a1059a54b81 100644 --- a/selftest/knownfail_mit_kdc +++ b/selftest/knownfail_mit_kdc @@ -437,7 +437,6 @@ samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_ # Kpasswd tests # ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_canonicalize_realm_case.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_empty.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_canonicalize_realm_case.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_non_initial.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_lifetime.ad_dc diff --git a/selftest/knownfail_mit_kdc_1_20 b/selftest/knownfail_mit_kdc_1_20 index f886c360381..c4f2ea2def7 100644 --- a/selftest/knownfail_mit_kdc_1_20 +++ b/selftest/knownfail_mit_kdc_1_20 @@ -13,6 +13,7 @@ ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_canonicalize.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_change.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_change_expired_password.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_empty.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_from_rodc.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_initial.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_canonicalize.ad_dc diff --git a/source4/kdc/kpasswd-service.c b/source4/kdc/kpasswd-service.c index 061aedc80e5..22e1295c11e 100644 --- a/source4/kdc/kpasswd-service.c +++ b/source4/kdc/kpasswd-service.c @@ -256,6 +256,7 @@ kdc_code kpasswd_process(struct kdc_server *kdc, &kpasswd_dec_reply, &error_string); if (code != 0) { + ap_rep_blob = data_blob_null; error_code = code; goto reply; } @@ -265,6 +266,7 @@ kdc_code kpasswd_process(struct kdc_server *kdc, &kpasswd_dec_reply, &enc_data_blob); if (!NT_STATUS_IS_OK(status)) { + ap_rep_blob = data_blob_null; error_code = KRB5_KPASSWD_HARDERROR; error_string = talloc_asprintf(tmp_ctx, "gensec_wrap failed - %s\n", -- 2.35.0 From 72b084955b7a2a3fe69a46df039f727ec7174ef5 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Fri, 27 May 2022 19:29:34 +1200 Subject: [PATCH 20/43] CVE-2022-2031 lib:krb5_wrap: Generate valid error codes in smb_krb5_mk_error() The error code passed in will be an offset from ERROR_TABLE_BASE_krb5, so we need to subtract that before creating the error. Heimdal does this internally, so it isn't needed there. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- lib/krb5_wrap/krb5_samba.c | 2 +- selftest/knownfail_mit_kdc | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/krb5_wrap/krb5_samba.c b/lib/krb5_wrap/krb5_samba.c index 57ffdc72780..fed8fd73e5a 100644 --- a/lib/krb5_wrap/krb5_samba.c +++ b/lib/krb5_wrap/krb5_samba.c @@ -237,7 +237,7 @@ krb5_error_code smb_krb5_mk_error(krb5_context context, return code; } - errpkt.error = error_code; + errpkt.error = error_code - ERROR_TABLE_BASE_krb5; errpkt.text.length = 0; if (e_text != NULL) { diff --git a/selftest/knownfail_mit_kdc b/selftest/knownfail_mit_kdc index a1059a54b81..866d6f6df5e 100644 --- a/selftest/knownfail_mit_kdc +++ b/selftest/knownfail_mit_kdc @@ -437,9 +437,13 @@ samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_ # Kpasswd tests # ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_canonicalize_realm_case.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_empty.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_from_rodc.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_canonicalize_realm_case.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_seq_number.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_non_initial.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_lifetime.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_requester_sid_tgs.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_server.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_service.ad_dc -- 2.35.0 From 7860f0af01b4bfd35de3b56ac642ba598ba3ce44 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 18 May 2022 16:49:43 +1200 Subject: [PATCH 21/43] CVE-2022-2031 s4:kpasswd: Return a kpasswd error code in KRB-ERROR If we attempt to return an error code outside of Heimdal's allowed range [KRB5KDC_ERR_NONE, KRB5_ERR_RCSID), it will be replaced with a GENERIC error, and the error text will be set to the meaningless result of krb5_get_error_message(). Avoid this by ensuring the error code is in the correct range. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- selftest/knownfail_heimdal_kdc | 2 -- selftest/knownfail_mit_kdc | 4 ---- source4/kdc/kpasswd-service.c | 2 +- 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc index 22d5656e5fb..3a45c91320a 100644 --- a/selftest/knownfail_heimdal_kdc +++ b/selftest/knownfail_heimdal_kdc @@ -59,9 +59,7 @@ # ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_canonicalize.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_canonicalize_realm_case.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_empty.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_from_rodc.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_seq_number.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_non_initial.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_lifetime.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_requester_sid_tgs.ad_dc diff --git a/selftest/knownfail_mit_kdc b/selftest/knownfail_mit_kdc index 866d6f6df5e..a1059a54b81 100644 --- a/selftest/knownfail_mit_kdc +++ b/selftest/knownfail_mit_kdc @@ -437,13 +437,9 @@ samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_ # Kpasswd tests # ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_canonicalize_realm_case.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_empty.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_from_rodc.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_canonicalize_realm_case.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_seq_number.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_non_initial.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_lifetime.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_requester_sid_tgs.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_server.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_service.ad_dc diff --git a/source4/kdc/kpasswd-service.c b/source4/kdc/kpasswd-service.c index 22e1295c11e..379ddebf3ad 100644 --- a/source4/kdc/kpasswd-service.c +++ b/source4/kdc/kpasswd-service.c @@ -315,7 +315,7 @@ reply: } code = smb_krb5_mk_error(kdc->smb_krb5_context->krb5_context, - error_code, + KRB5KDC_ERR_NONE + error_code, NULL, /* e_text */ &k_dec_data, NULL, /* client */ -- 2.35.0 From b5e13c6c5d447dc10e5d8bd6359e7a22aa9cbc63 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 18 May 2022 16:06:31 +1200 Subject: [PATCH 22/43] CVE-2022-2031 gensec_krb5: Add helper function to check if client sent an initial ticket This will be used in the kpasswd service to ensure that the client has an initial ticket to kadmin/changepw, and not a service ticket. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- source4/auth/gensec/gensec_krb5.c | 20 +----- source4/auth/gensec/gensec_krb5_helpers.c | 72 ++++++++++++++++++++++ source4/auth/gensec/gensec_krb5_helpers.h | 32 ++++++++++ source4/auth/gensec/gensec_krb5_internal.h | 47 ++++++++++++++ source4/auth/gensec/wscript_build | 4 ++ 5 files changed, 157 insertions(+), 18 deletions(-) create mode 100644 source4/auth/gensec/gensec_krb5_helpers.c create mode 100644 source4/auth/gensec/gensec_krb5_helpers.h create mode 100644 source4/auth/gensec/gensec_krb5_internal.h diff --git a/source4/auth/gensec/gensec_krb5.c b/source4/auth/gensec/gensec_krb5.c index 7d87b3ac6b9..104e4639c44 100644 --- a/source4/auth/gensec/gensec_krb5.c +++ b/source4/auth/gensec/gensec_krb5.c @@ -44,27 +44,11 @@ #include "../lib/util/asn1.h" #include "auth/kerberos/pac_utils.h" #include "gensec_krb5.h" +#include "gensec_krb5_internal.h" +#include "gensec_krb5_helpers.h" _PUBLIC_ NTSTATUS gensec_krb5_init(TALLOC_CTX *); -enum GENSEC_KRB5_STATE { - GENSEC_KRB5_SERVER_START, - GENSEC_KRB5_CLIENT_START, - GENSEC_KRB5_CLIENT_MUTUAL_AUTH, - GENSEC_KRB5_DONE -}; - -struct gensec_krb5_state { - enum GENSEC_KRB5_STATE state_position; - struct smb_krb5_context *smb_krb5_context; - krb5_auth_context auth_context; - krb5_data enc_ticket; - krb5_keyblock *keyblock; - krb5_ticket *ticket; - bool gssapi; - krb5_flags ap_req_options; -}; - static int gensec_krb5_destroy(struct gensec_krb5_state *gensec_krb5_state) { if (!gensec_krb5_state->smb_krb5_context) { diff --git a/source4/auth/gensec/gensec_krb5_helpers.c b/source4/auth/gensec/gensec_krb5_helpers.c new file mode 100644 index 00000000000..21f2f1e884e --- /dev/null +++ b/source4/auth/gensec/gensec_krb5_helpers.c @@ -0,0 +1,72 @@ +/* + Unix SMB/CIFS implementation. + + Kerberos backend for GENSEC + + Copyright (C) Andrew Bartlett 2004 + Copyright (C) Andrew Tridgell 2001 + Copyright (C) Luke Howard 2002-2003 + Copyright (C) Stefan Metzmacher 2004-2005 + + 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 . +*/ + +#include "includes.h" +#include "auth/auth.h" +#include "auth/gensec/gensec.h" +#include "auth/gensec/gensec_internal.h" +#include "gensec_krb5_internal.h" +#include "gensec_krb5_helpers.h" +#include "system/kerberos.h" +#include "auth/kerberos/kerberos.h" + +static struct gensec_krb5_state *get_private_state(const struct gensec_security *gensec_security) +{ + struct gensec_krb5_state *gensec_krb5_state = NULL; + + if (strcmp(gensec_security->ops->name, "krb5") != 0) { + /* We require that the krb5 mechanism is being used. */ + return NULL; + } + + gensec_krb5_state = talloc_get_type(gensec_security->private_data, + struct gensec_krb5_state); + return gensec_krb5_state; +} + +/* + * Returns 1 if our ticket has the initial flag set, 0 if not, and -1 in case of + * error. + */ +int gensec_krb5_initial_ticket(const struct gensec_security *gensec_security) +{ + struct gensec_krb5_state *gensec_krb5_state = NULL; + + gensec_krb5_state = get_private_state(gensec_security); + if (gensec_krb5_state == NULL) { + return -1; + } + + if (gensec_krb5_state->ticket == NULL) { + /* We don't have a ticket */ + return -1; + } + +#ifdef SAMBA4_USES_HEIMDAL + return gensec_krb5_state->ticket->ticket.flags.initial; +#else /* MIT KERBEROS */ + return (gensec_krb5_state->ticket->enc_part2->flags & TKT_FLG_INITIAL) ? 1 : 0; +#endif /* SAMBA4_USES_HEIMDAL */ +} diff --git a/source4/auth/gensec/gensec_krb5_helpers.h b/source4/auth/gensec/gensec_krb5_helpers.h new file mode 100644 index 00000000000..d7b694dad0c --- /dev/null +++ b/source4/auth/gensec/gensec_krb5_helpers.h @@ -0,0 +1,32 @@ +/* + Unix SMB/CIFS implementation. + + Kerberos backend for GENSEC + + Copyright (C) Andrew Bartlett 2004 + Copyright (C) Andrew Tridgell 2001 + Copyright (C) Luke Howard 2002-2003 + Copyright (C) Stefan Metzmacher 2004-2005 + + 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 . +*/ + +struct gensec_security; + +/* + * Returns 1 if our ticket has the initial flag set, 0 if not, and -1 in case of + * error. + */ +int gensec_krb5_initial_ticket(const struct gensec_security *gensec_security); diff --git a/source4/auth/gensec/gensec_krb5_internal.h b/source4/auth/gensec/gensec_krb5_internal.h new file mode 100644 index 00000000000..0bb796f1b2a --- /dev/null +++ b/source4/auth/gensec/gensec_krb5_internal.h @@ -0,0 +1,47 @@ +/* + Unix SMB/CIFS implementation. + + Kerberos backend for GENSEC + + Copyright (C) Andrew Bartlett 2004 + Copyright (C) Andrew Tridgell 2001 + Copyright (C) Luke Howard 2002-2003 + Copyright (C) Stefan Metzmacher 2004-2005 + + 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 . +*/ + +#include "includes.h" +#include "auth/gensec/gensec.h" +#include "system/kerberos.h" +#include "auth/kerberos/kerberos.h" + +enum GENSEC_KRB5_STATE { + GENSEC_KRB5_SERVER_START, + GENSEC_KRB5_CLIENT_START, + GENSEC_KRB5_CLIENT_MUTUAL_AUTH, + GENSEC_KRB5_DONE +}; + +struct gensec_krb5_state { + enum GENSEC_KRB5_STATE state_position; + struct smb_krb5_context *smb_krb5_context; + krb5_auth_context auth_context; + krb5_data enc_ticket; + krb5_keyblock *keyblock; + krb5_ticket *ticket; + bool gssapi; + krb5_flags ap_req_options; +}; diff --git a/source4/auth/gensec/wscript_build b/source4/auth/gensec/wscript_build index d14a50ff273..20271f1665b 100644 --- a/source4/auth/gensec/wscript_build +++ b/source4/auth/gensec/wscript_build @@ -18,6 +18,10 @@ bld.SAMBA_MODULE('gensec_krb5', enabled=bld.AD_DC_BUILD_IS_ENABLED() ) +bld.SAMBA_SUBSYSTEM('gensec_krb5_helpers', + source='gensec_krb5_helpers.c', + deps='gensec_krb5', + enabled=bld.AD_DC_BUILD_IS_ENABLED()) bld.SAMBA_MODULE('gensec_gssapi', source='gensec_gssapi.c', -- 2.35.0 From 6d4839cd5d440edd0015b38b1418c2fd119c774e Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 18 May 2022 16:52:41 +1200 Subject: [PATCH 23/43] CVE-2022-2031 s4:kpasswd: Require an initial ticket Ensure that for password changes the client uses an AS-REQ to get the ticket to kpasswd, and not a TGS-REQ. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- selftest/knownfail_heimdal_kdc | 1 - selftest/knownfail_mit_kdc | 1 - selftest/knownfail_mit_kdc_1_20 | 1 + source4/kdc/kpasswd-service-heimdal.c | 17 +++++++++++++++++ source4/kdc/kpasswd-service-mit.c | 17 +++++++++++++++++ source4/kdc/wscript_build | 1 + 6 files changed, 36 insertions(+), 2 deletions(-) diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc index 3a45c91320a..ee22fcf688e 100644 --- a/selftest/knownfail_heimdal_kdc +++ b/selftest/knownfail_heimdal_kdc @@ -60,7 +60,6 @@ ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_canonicalize.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_canonicalize_realm_case.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_from_rodc.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_non_initial.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_lifetime.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_requester_sid_tgs.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key.ad_dc diff --git a/selftest/knownfail_mit_kdc b/selftest/knownfail_mit_kdc index a1059a54b81..4059f2210f4 100644 --- a/selftest/knownfail_mit_kdc +++ b/selftest/knownfail_mit_kdc @@ -438,7 +438,6 @@ samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_ # ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_canonicalize_realm_case.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_canonicalize_realm_case.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_non_initial.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_lifetime.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_requester_sid_tgs.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_server.ad_dc diff --git a/selftest/knownfail_mit_kdc_1_20 b/selftest/knownfail_mit_kdc_1_20 index c4f2ea2def7..ea9de50ff52 100644 --- a/selftest/knownfail_mit_kdc_1_20 +++ b/selftest/knownfail_mit_kdc_1_20 @@ -18,6 +18,7 @@ ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_initial.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_canonicalize.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_seq_number.ad_dc +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_non_initial.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_expired_password.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_target_princ_and_realm_access.ad_dc diff --git a/source4/kdc/kpasswd-service-heimdal.c b/source4/kdc/kpasswd-service-heimdal.c index 21596d8d8a4..7ddf1af148e 100644 --- a/source4/kdc/kpasswd-service-heimdal.c +++ b/source4/kdc/kpasswd-service-heimdal.c @@ -24,6 +24,7 @@ #include "param/param.h" #include "auth/auth.h" #include "auth/gensec/gensec.h" +#include "gensec_krb5_helpers.h" #include "kdc/kdc-server.h" #include "kdc/kpasswd_glue.h" #include "kdc/kpasswd-service.h" @@ -31,6 +32,7 @@ static krb5_error_code kpasswd_change_password(struct kdc_server *kdc, TALLOC_CTX *mem_ctx, + const struct gensec_security *gensec_security, struct auth_session_info *session_info, DATA_BLOB *password, DATA_BLOB *kpasswd_reply, @@ -42,6 +44,17 @@ static krb5_error_code kpasswd_change_password(struct kdc_server *kdc, const char *reject_string = NULL; struct samr_DomInfo1 *dominfo; bool ok; + int ret; + + /* + * We're doing a password change (rather than a password set), so check + * that we were given an initial ticket. + */ + ret = gensec_krb5_initial_ticket(gensec_security); + if (ret != 1) { + *error_string = "Expected an initial ticket"; + return KRB5_KPASSWD_INITIAL_FLAG_NEEDED; + } status = samdb_kpasswd_change_password(mem_ctx, kdc->task->lp_ctx, @@ -80,6 +93,7 @@ static krb5_error_code kpasswd_change_password(struct kdc_server *kdc, static krb5_error_code kpasswd_set_password(struct kdc_server *kdc, TALLOC_CTX *mem_ctx, + const struct gensec_security *gensec_security, struct auth_session_info *session_info, DATA_BLOB *decoded_data, DATA_BLOB *kpasswd_reply, @@ -172,6 +186,7 @@ static krb5_error_code kpasswd_set_password(struct kdc_server *kdc, free_ChangePasswdDataMS(&chpw); return kpasswd_change_password(kdc, mem_ctx, + gensec_security, session_info, &password, kpasswd_reply, @@ -271,6 +286,7 @@ krb5_error_code kpasswd_handle_request(struct kdc_server *kdc, return kpasswd_change_password(kdc, mem_ctx, + gensec_security, session_info, &password, kpasswd_reply, @@ -279,6 +295,7 @@ krb5_error_code kpasswd_handle_request(struct kdc_server *kdc, case KRB5_KPASSWD_VERS_SETPW: { return kpasswd_set_password(kdc, mem_ctx, + gensec_security, session_info, decoded_data, kpasswd_reply, diff --git a/source4/kdc/kpasswd-service-mit.c b/source4/kdc/kpasswd-service-mit.c index 26b62162e68..fe24693139c 100644 --- a/source4/kdc/kpasswd-service-mit.c +++ b/source4/kdc/kpasswd-service-mit.c @@ -24,6 +24,7 @@ #include "param/param.h" #include "auth/auth.h" #include "auth/gensec/gensec.h" +#include "gensec_krb5_helpers.h" #include "kdc/kdc-server.h" #include "kdc/kpasswd_glue.h" #include "kdc/kpasswd-service.h" @@ -84,6 +85,7 @@ out: static krb5_error_code kpasswd_change_password(struct kdc_server *kdc, TALLOC_CTX *mem_ctx, + const struct gensec_security *gensec_security, struct auth_session_info *session_info, DATA_BLOB *password, DATA_BLOB *kpasswd_reply, @@ -95,6 +97,17 @@ static krb5_error_code kpasswd_change_password(struct kdc_server *kdc, const char *reject_string = NULL; struct samr_DomInfo1 *dominfo; bool ok; + int ret; + + /* + * We're doing a password change (rather than a password set), so check + * that we were given an initial ticket. + */ + ret = gensec_krb5_initial_ticket(gensec_security); + if (ret != 1) { + *error_string = "Expected an initial ticket"; + return KRB5_KPASSWD_INITIAL_FLAG_NEEDED; + } status = samdb_kpasswd_change_password(mem_ctx, kdc->task->lp_ctx, @@ -133,6 +146,7 @@ static krb5_error_code kpasswd_change_password(struct kdc_server *kdc, static krb5_error_code kpasswd_set_password(struct kdc_server *kdc, TALLOC_CTX *mem_ctx, + const struct gensec_security *gensec_security, struct auth_session_info *session_info, DATA_BLOB *decoded_data, DATA_BLOB *kpasswd_reply, @@ -249,6 +263,7 @@ static krb5_error_code kpasswd_set_password(struct kdc_server *kdc, return kpasswd_change_password(kdc, mem_ctx, + gensec_security, session_info, &password, kpasswd_reply, @@ -349,6 +364,7 @@ krb5_error_code kpasswd_handle_request(struct kdc_server *kdc, return kpasswd_change_password(kdc, mem_ctx, + gensec_security, session_info, &password, kpasswd_reply, @@ -357,6 +373,7 @@ krb5_error_code kpasswd_handle_request(struct kdc_server *kdc, case RFC3244_VERSION: { return kpasswd_set_password(kdc, mem_ctx, + gensec_security, session_info, decoded_data, kpasswd_reply, diff --git a/source4/kdc/wscript_build b/source4/kdc/wscript_build index 5c16e68ee0a..0c902f50534 100644 --- a/source4/kdc/wscript_build +++ b/source4/kdc/wscript_build @@ -85,6 +85,7 @@ bld.SAMBA_SUBSYSTEM('KPASSWD-SERVICE', krb5samba samba_server_gensec KPASSWD_GLUE + gensec_krb5_helpers ''') bld.SAMBA_SUBSYSTEM('KDC-GLUE', -- 2.35.0 From 535e8b2e62658717366a2cbfe60249a12a60b6a8 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 18 May 2022 17:11:49 +1200 Subject: [PATCH 24/43] s4:kpasswd: Restructure code for clarity View with 'git show -b'. Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- source4/kdc/kpasswd-service-heimdal.c | 46 +++++++++++++-------------- 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/source4/kdc/kpasswd-service-heimdal.c b/source4/kdc/kpasswd-service-heimdal.c index 7ddf1af148e..00470af7cf2 100644 --- a/source4/kdc/kpasswd-service-heimdal.c +++ b/source4/kdc/kpasswd-service-heimdal.c @@ -159,30 +159,7 @@ static krb5_error_code kpasswd_set_password(struct kdc_server *kdc, return 0; } - if (chpw.targname != NULL && chpw.targrealm != NULL) { - code = krb5_build_principal_ext(context, - &target_principal, - strlen(*chpw.targrealm), - *chpw.targrealm, - 0); - if (code != 0) { - free_ChangePasswdDataMS(&chpw); - return kpasswd_make_error_reply(mem_ctx, - KRB5_KPASSWD_MALFORMED, - "Failed to parse principal", - kpasswd_reply); - } - code = copy_PrincipalName(chpw.targname, - &target_principal->name); - if (code != 0) { - free_ChangePasswdDataMS(&chpw); - krb5_free_principal(context, target_principal); - return kpasswd_make_error_reply(mem_ctx, - KRB5_KPASSWD_MALFORMED, - "Failed to parse principal", - kpasswd_reply); - } - } else { + if (chpw.targname == NULL || chpw.targrealm == NULL) { free_ChangePasswdDataMS(&chpw); return kpasswd_change_password(kdc, mem_ctx, @@ -192,7 +169,28 @@ static krb5_error_code kpasswd_set_password(struct kdc_server *kdc, kpasswd_reply, error_string); } + code = krb5_build_principal_ext(context, + &target_principal, + strlen(*chpw.targrealm), + *chpw.targrealm, + 0); + if (code != 0) { + free_ChangePasswdDataMS(&chpw); + return kpasswd_make_error_reply(mem_ctx, + KRB5_KPASSWD_MALFORMED, + "Failed to parse principal", + kpasswd_reply); + } + code = copy_PrincipalName(chpw.targname, + &target_principal->name); free_ChangePasswdDataMS(&chpw); + if (code != 0) { + krb5_free_principal(context, target_principal); + return kpasswd_make_error_reply(mem_ctx, + KRB5_KPASSWD_MALFORMED, + "Failed to parse principal", + kpasswd_reply); + } if (target_principal->name.name_string.len >= 2) { is_service_principal = true; -- 2.35.0 From 89519d603e2830f49b0e7ba58ab618b9170032ed Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Tue, 24 May 2022 10:17:00 +0200 Subject: [PATCH 25/43] CVE-2022-2031 testprogs: Fix auth with smbclient and krb5 ccache BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 Signed-off-by: Andreas Schneider Reviewed-by: Joseph Sutton --- testprogs/blackbox/test_kpasswd_heimdal.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testprogs/blackbox/test_kpasswd_heimdal.sh b/testprogs/blackbox/test_kpasswd_heimdal.sh index 43f38b09de2..a73c6665a18 100755 --- a/testprogs/blackbox/test_kpasswd_heimdal.sh +++ b/testprogs/blackbox/test_kpasswd_heimdal.sh @@ -71,7 +71,7 @@ testit "kinit with user password" \ do_kinit $TEST_PRINCIPAL $TEST_PASSWORD || failed=`expr $failed + 1` test_smbclient "Test login with user kerberos ccache" \ - "ls" "$SMB_UNC" --use-kerberos=required || failed=`expr $failed + 1` + "ls" "$SMB_UNC" --use-krb5-ccache=${KRB5CCNAME} || failed=`expr $failed + 1` testit "change user password with 'samba-tool user password' (unforced)" \ $VALGRIND $PYTHON $samba_tool user password -W$DOMAIN -U$TEST_USERNAME%$TEST_PASSWORD --use-kerberos=off --newpassword=$TEST_PASSWORD_NEW || failed=`expr $failed + 1` @@ -84,7 +84,7 @@ testit "kinit with user password" \ do_kinit $TEST_PRINCIPAL $TEST_PASSWORD || failed=`expr $failed + 1` test_smbclient "Test login with user kerberos ccache" \ - "ls" "$SMB_UNC" --use-kerberos=required || failed=`expr $failed + 1` + "ls" "$SMB_UNC" --use-krb5-ccache=${KRB5CCNAME} || failed=`expr $failed + 1` ########################################################### ### check that a short password is rejected -- 2.35.0 From dcecdc42956f843816e0c3b42171aafea2d99b49 Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Thu, 19 May 2022 16:35:28 +0200 Subject: [PATCH 26/43] CVE-2022-2031 testprogs: Add kadmin/changepw canonicalization test with MIT kpasswd BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 Signed-off-by: Andreas Schneider Reviewed-by: Joseph Sutton --- selftest/knownfail.d/kadmin_changepw | 1 + testprogs/blackbox/test_kpasswd_heimdal.sh | 35 +++++++++++++++++++++- 2 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 selftest/knownfail.d/kadmin_changepw diff --git a/selftest/knownfail.d/kadmin_changepw b/selftest/knownfail.d/kadmin_changepw new file mode 100644 index 00000000000..97c14793ea5 --- /dev/null +++ b/selftest/knownfail.d/kadmin_changepw @@ -0,0 +1 @@ +^samba4.blackbox.kpasswd.MIT kpasswd.change.user.password diff --git a/testprogs/blackbox/test_kpasswd_heimdal.sh b/testprogs/blackbox/test_kpasswd_heimdal.sh index a73c6665a18..698044a3fd3 100755 --- a/testprogs/blackbox/test_kpasswd_heimdal.sh +++ b/testprogs/blackbox/test_kpasswd_heimdal.sh @@ -7,7 +7,7 @@ if [ $# -lt 6 ]; then cat < "${PREFIX}/tmpkpasswdscript" < "${KRB5_CONFIG}" + testit "MIT kpasswd change user password" \ + "${texpect}" "${PREFIX}/tmpkpasswdscript" "${mit_kpasswd}" \ + "${TEST_PRINCIPAL}" || + failed=$((failed + 1)) + KRB5_CONFIG="${SAVE_KRB5_CONFIG}" + export KRB5_CONFIG +fi + +TEST_PASSWORD="${TEST_PASSWORD_NEW}" +TEST_PASSWORD_NEW="testPaSS@03force%" + ########################################################### ### Force password change at login ########################################################### -- 2.35.0 From ccff6e8ac35c2df59e1100f9b8a1493138b63703 Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Tue, 24 May 2022 09:54:18 +0200 Subject: [PATCH 27/43] CVE-2022-2031 s4:kdc: Implement is_kadmin_changepw() helper function BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 Signed-off-by: Andreas Schneider Reviewed-by: Joseph Sutton --- source4/kdc/db-glue.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c index 172a34194c6..c95102737b1 100644 --- a/source4/kdc/db-glue.c +++ b/source4/kdc/db-glue.c @@ -917,6 +917,14 @@ static int principal_comp_strcmp(krb5_context context, component, string, false); } +static bool is_kadmin_changepw(krb5_context context, + krb5_const_principal principal) +{ + return krb5_princ_size(context, principal) == 2 && + (principal_comp_strcmp(context, principal, 0, "kadmin") == 0) && + (principal_comp_strcmp(context, principal, 1, "changepw") == 0); +} + /* * Construct an hdb_entry from a directory entry. */ @@ -1221,11 +1229,9 @@ static krb5_error_code samba_kdc_message2entry(krb5_context context, * 'change password', as otherwise we could get into * trouble, and not enforce the password expirty. * Instead, only do it when request is for the kpasswd service */ - if (ent_type == SAMBA_KDC_ENT_TYPE_SERVER - && krb5_princ_size(context, principal) == 2 - && (principal_comp_strcmp(context, principal, 0, "kadmin") == 0) - && (principal_comp_strcmp(context, principal, 1, "changepw") == 0) - && lpcfg_is_my_domain_or_realm(lp_ctx, realm)) { + if (ent_type == SAMBA_KDC_ENT_TYPE_SERVER && + is_kadmin_changepw(context, principal) && + lpcfg_is_my_domain_or_realm(lp_ctx, realm)) { entry->flags.change_pw = 1; } -- 2.35.0 From eb5fe23dfd9b6326b79ae33991cb5430a0b44fdd Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 18 May 2022 16:56:01 +1200 Subject: [PATCH 28/43] CVE-2022-2031 s4:kdc: Split out a samba_kdc_get_entry_principal() function BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- source4/kdc/db-glue.c | 192 +++++++++++++++++++++++------------------- 1 file changed, 107 insertions(+), 85 deletions(-) diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c index c95102737b1..8a2f868bcc0 100644 --- a/source4/kdc/db-glue.c +++ b/source4/kdc/db-glue.c @@ -925,6 +925,101 @@ static bool is_kadmin_changepw(krb5_context context, (principal_comp_strcmp(context, principal, 1, "changepw") == 0); } +static krb5_error_code samba_kdc_get_entry_principal( + krb5_context context, + struct samba_kdc_db_context *kdc_db_ctx, + const char *samAccountName, + enum samba_kdc_ent_type ent_type, + unsigned flags, + krb5_const_principal in_princ, + krb5_principal *out_princ) +{ + struct loadparm_context *lp_ctx = kdc_db_ctx->lp_ctx; + krb5_error_code ret = 0; + + /* + * If we are set to canonicalize, we get back the fixed UPPER + * case realm, and the real username (ie matching LDAP + * samAccountName) + * + * Otherwise, if we are set to enterprise, we + * get back the whole principal as-sent + * + * Finally, if we are not set to canonicalize, we get back the + * fixed UPPER case realm, but the as-sent username + */ + + if (ent_type == SAMBA_KDC_ENT_TYPE_KRBTGT) { + if (flags & (SDB_F_CANON|SDB_F_FORCE_CANON)) { + /* + * When requested to do so, ensure that the + * both realm values in the principal are set + * to the upper case, canonical realm + */ + ret = smb_krb5_make_principal(context, out_princ, + lpcfg_realm(lp_ctx), "krbtgt", + lpcfg_realm(lp_ctx), NULL); + if (ret) { + return ret; + } + smb_krb5_principal_set_type(context, *out_princ, KRB5_NT_SRV_INST); + } else { + ret = krb5_copy_principal(context, in_princ, out_princ); + if (ret) { + return ret; + } + /* + * this appears to be required regardless of + * the canonicalize flag from the client + */ + ret = smb_krb5_principal_set_realm(context, *out_princ, lpcfg_realm(lp_ctx)); + if (ret) { + return ret; + } + } + + } else if (ent_type == SAMBA_KDC_ENT_TYPE_ANY && in_princ == NULL) { + ret = smb_krb5_make_principal(context, out_princ, lpcfg_realm(lp_ctx), samAccountName, NULL); + if (ret) { + return ret; + } + } else if ((flags & SDB_F_FORCE_CANON) || + ((flags & SDB_F_CANON) && (flags & SDB_F_FOR_AS_REQ))) { + /* + * SDB_F_CANON maps from the canonicalize flag in the + * packet, and has a different meaning between AS-REQ + * and TGS-REQ. We only change the principal in the AS-REQ case + * + * The SDB_F_FORCE_CANON if for new MIT KDC code that wants + * the canonical name in all lookups, and takes care to + * canonicalize only when appropriate. + */ + ret = smb_krb5_make_principal(context, out_princ, lpcfg_realm(lp_ctx), samAccountName, NULL); + if (ret) { + return ret; + } + } else { + ret = krb5_copy_principal(context, in_princ, out_princ); + if (ret) { + return ret; + } + + /* While we have copied the client principal, tests + * show that Win2k3 returns the 'corrected' realm, not + * the client-specified realm. This code attempts to + * replace the client principal's realm with the one + * we determine from our records */ + + /* this has to be with malloc() */ + ret = smb_krb5_principal_set_realm(context, *out_princ, lpcfg_realm(lp_ctx)); + if (ret) { + return ret; + } + } + + return 0; +} + /* * Construct an hdb_entry from a directory entry. */ @@ -1017,93 +1112,8 @@ static krb5_error_code samba_kdc_message2entry(krb5_context context, userAccountControl |= msDS_User_Account_Control_Computed; } - /* - * If we are set to canonicalize, we get back the fixed UPPER - * case realm, and the real username (ie matching LDAP - * samAccountName) - * - * Otherwise, if we are set to enterprise, we - * get back the whole principal as-sent - * - * Finally, if we are not set to canonicalize, we get back the - * fixed UPPER case realm, but the as-sent username - */ - if (ent_type == SAMBA_KDC_ENT_TYPE_KRBTGT) { p->is_krbtgt = true; - - if (flags & (SDB_F_CANON|SDB_F_FORCE_CANON)) { - /* - * When requested to do so, ensure that the - * both realm values in the principal are set - * to the upper case, canonical realm - */ - ret = smb_krb5_make_principal(context, &entry->principal, - lpcfg_realm(lp_ctx), "krbtgt", - lpcfg_realm(lp_ctx), NULL); - if (ret) { - krb5_clear_error_message(context); - goto out; - } - smb_krb5_principal_set_type(context, entry->principal, KRB5_NT_SRV_INST); - } else { - ret = krb5_copy_principal(context, principal, &entry->principal); - if (ret) { - krb5_clear_error_message(context); - goto out; - } - /* - * this appears to be required regardless of - * the canonicalize flag from the client - */ - ret = smb_krb5_principal_set_realm(context, entry->principal, lpcfg_realm(lp_ctx)); - if (ret) { - krb5_clear_error_message(context); - goto out; - } - } - - } else if (ent_type == SAMBA_KDC_ENT_TYPE_ANY && principal == NULL) { - ret = smb_krb5_make_principal(context, &entry->principal, lpcfg_realm(lp_ctx), samAccountName, NULL); - if (ret) { - krb5_clear_error_message(context); - goto out; - } - } else if ((flags & SDB_F_FORCE_CANON) || - ((flags & SDB_F_CANON) && (flags & SDB_F_FOR_AS_REQ))) { - /* - * SDB_F_CANON maps from the canonicalize flag in the - * packet, and has a different meaning between AS-REQ - * and TGS-REQ. We only change the principal in the AS-REQ case - * - * The SDB_F_FORCE_CANON if for new MIT KDC code that wants - * the canonical name in all lookups, and takes care to - * canonicalize only when appropriate. - */ - ret = smb_krb5_make_principal(context, &entry->principal, lpcfg_realm(lp_ctx), samAccountName, NULL); - if (ret) { - krb5_clear_error_message(context); - goto out; - } - } else { - ret = krb5_copy_principal(context, principal, &entry->principal); - if (ret) { - krb5_clear_error_message(context); - goto out; - } - - /* While we have copied the client principal, tests - * show that Win2k3 returns the 'corrected' realm, not - * the client-specified realm. This code attempts to - * replace the client principal's realm with the one - * we determine from our records */ - - /* this has to be with malloc() */ - ret = smb_krb5_principal_set_realm(context, entry->principal, lpcfg_realm(lp_ctx)); - if (ret) { - krb5_clear_error_message(context); - goto out; - } } /* First try and figure out the flags based on the userAccountControl */ @@ -1296,6 +1306,18 @@ static krb5_error_code samba_kdc_message2entry(krb5_context context, } } + ret = samba_kdc_get_entry_principal(context, + kdc_db_ctx, + samAccountName, + ent_type, + flags, + principal, + &entry->principal); + if (ret != 0) { + krb5_clear_error_message(context); + goto out; + } + entry->valid_start = NULL; entry->max_life = malloc(sizeof(*entry->max_life)); -- 2.35.0 From 7fbd6f8de022c50f9909c23ae7ae0a2302ac0759 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 25 May 2022 17:19:58 +1200 Subject: [PATCH 29/43] CVE-2022-2031 s4:kdc: Refactor samba_kdc_get_entry_principal() This eliminates some duplicate branches. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 Signed-off-by: Joseph Sutton Pair-Programmed-With: Andreas Schneider Reviewed-by: Andreas Schneider --- source4/kdc/db-glue.c | 116 ++++++++++++++++++++---------------------- 1 file changed, 55 insertions(+), 61 deletions(-) diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c index 8a2f868bcc0..70af9293f6d 100644 --- a/source4/kdc/db-glue.c +++ b/source4/kdc/db-glue.c @@ -935,7 +935,8 @@ static krb5_error_code samba_kdc_get_entry_principal( krb5_principal *out_princ) { struct loadparm_context *lp_ctx = kdc_db_ctx->lp_ctx; - krb5_error_code ret = 0; + krb5_error_code code = 0; + bool canon = flags & (SDB_F_CANON|SDB_F_FORCE_CANON); /* * If we are set to canonicalize, we get back the fixed UPPER @@ -949,75 +950,68 @@ static krb5_error_code samba_kdc_get_entry_principal( * fixed UPPER case realm, but the as-sent username */ - if (ent_type == SAMBA_KDC_ENT_TYPE_KRBTGT) { - if (flags & (SDB_F_CANON|SDB_F_FORCE_CANON)) { - /* - * When requested to do so, ensure that the - * both realm values in the principal are set - * to the upper case, canonical realm - */ - ret = smb_krb5_make_principal(context, out_princ, - lpcfg_realm(lp_ctx), "krbtgt", - lpcfg_realm(lp_ctx), NULL); - if (ret) { - return ret; - } - smb_krb5_principal_set_type(context, *out_princ, KRB5_NT_SRV_INST); - } else { - ret = krb5_copy_principal(context, in_princ, out_princ); - if (ret) { - return ret; - } - /* - * this appears to be required regardless of - * the canonicalize flag from the client - */ - ret = smb_krb5_principal_set_realm(context, *out_princ, lpcfg_realm(lp_ctx)); - if (ret) { - return ret; - } + if (ent_type == SAMBA_KDC_ENT_TYPE_KRBTGT && canon) { + /* + * When requested to do so, ensure that the + * both realm values in the principal are set + * to the upper case, canonical realm + */ + code = smb_krb5_make_principal(context, + out_princ, + lpcfg_realm(lp_ctx), + "krbtgt", + lpcfg_realm(lp_ctx), + NULL); + if (code != 0) { + return code; } + smb_krb5_principal_set_type(context, + *out_princ, + KRB5_NT_SRV_INST); - } else if (ent_type == SAMBA_KDC_ENT_TYPE_ANY && in_princ == NULL) { - ret = smb_krb5_make_principal(context, out_princ, lpcfg_realm(lp_ctx), samAccountName, NULL); - if (ret) { - return ret; - } - } else if ((flags & SDB_F_FORCE_CANON) || - ((flags & SDB_F_CANON) && (flags & SDB_F_FOR_AS_REQ))) { + return 0; + } + + if ((canon && flags & (SDB_F_FORCE_CANON|SDB_F_FOR_AS_REQ)) || + (ent_type == SAMBA_KDC_ENT_TYPE_ANY && in_princ == NULL)) { /* * SDB_F_CANON maps from the canonicalize flag in the * packet, and has a different meaning between AS-REQ - * and TGS-REQ. We only change the principal in the AS-REQ case + * and TGS-REQ. We only change the principal in the + * AS-REQ case. * - * The SDB_F_FORCE_CANON if for new MIT KDC code that wants - * the canonical name in all lookups, and takes care to - * canonicalize only when appropriate. + * The SDB_F_FORCE_CANON if for new MIT KDC code that + * wants the canonical name in all lookups, and takes + * care to canonicalize only when appropriate. */ - ret = smb_krb5_make_principal(context, out_princ, lpcfg_realm(lp_ctx), samAccountName, NULL); - if (ret) { - return ret; - } - } else { - ret = krb5_copy_principal(context, in_princ, out_princ); - if (ret) { - return ret; - } - - /* While we have copied the client principal, tests - * show that Win2k3 returns the 'corrected' realm, not - * the client-specified realm. This code attempts to - * replace the client principal's realm with the one - * we determine from our records */ - - /* this has to be with malloc() */ - ret = smb_krb5_principal_set_realm(context, *out_princ, lpcfg_realm(lp_ctx)); - if (ret) { - return ret; - } + code = smb_krb5_make_principal(context, + out_princ, + lpcfg_realm(lp_ctx), + samAccountName, + NULL); + return code; } - return 0; + /* + * For a krbtgt entry, this appears to be required regardless of the + * canonicalize flag from the client. + */ + code = krb5_copy_principal(context, in_princ, out_princ); + if (code != 0) { + return code; + } + + /* + * While we have copied the client principal, tests show that Win2k3 + * returns the 'corrected' realm, not the client-specified realm. This + * code attempts to replace the client principal's realm with the one + * we determine from our records + */ + code = smb_krb5_principal_set_realm(context, + *out_princ, + lpcfg_realm(lp_ctx)); + + return code; } /* -- 2.35.0 From fcd404ffecd3d5e1a67ed1fe25259c7dd64b15f6 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 18 May 2022 16:56:01 +1200 Subject: [PATCH 30/43] CVE-2022-2031 s4:kdc: Fix canonicalisation of kadmin/changepw principal Since this principal goes through the samba_kdc_fetch_server() path, setting the canonicalisation flag would cause the principal to be replaced with the sAMAccountName; this meant requests to kadmin/changepw@REALM would result in a ticket to krbtgt@REALM. Now we properly handle canonicalisation for the kadmin/changepw principal. View with 'git show -b'. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 Pair-Programmed-With: Andreas Schneider Signed-off-by: Andreas Schneider Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- selftest/knownfail.d/kadmin_changepw | 1 - selftest/knownfail_heimdal_kdc | 2 - selftest/knownfail_mit_kdc_1_20 | 22 -------- source4/kdc/db-glue.c | 84 +++++++++++++++------------- 4 files changed, 46 insertions(+), 63 deletions(-) delete mode 100644 selftest/knownfail.d/kadmin_changepw diff --git a/selftest/knownfail.d/kadmin_changepw b/selftest/knownfail.d/kadmin_changepw deleted file mode 100644 index 97c14793ea5..00000000000 --- a/selftest/knownfail.d/kadmin_changepw +++ /dev/null @@ -1 +0,0 @@ -^samba4.blackbox.kpasswd.MIT kpasswd.change.user.password diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc index ee22fcf688e..2e523ec6483 100644 --- a/selftest/knownfail_heimdal_kdc +++ b/selftest/knownfail_heimdal_kdc @@ -57,8 +57,6 @@ # # Kpasswd tests # -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_canonicalize.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_canonicalize_realm_case.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_from_rodc.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_lifetime.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_requester_sid_tgs.ad_dc diff --git a/selftest/knownfail_mit_kdc_1_20 b/selftest/knownfail_mit_kdc_1_20 index ea9de50ff52..4a47ab974ae 100644 --- a/selftest/knownfail_mit_kdc_1_20 +++ b/selftest/knownfail_mit_kdc_1_20 @@ -7,25 +7,3 @@ ^samba.tests.krb5.fast_tests.samba.tests.krb5.fast_tests.FAST_Tests.test_simple_as_req_self\( ^samba.tests.krb5.fast_tests.samba.tests.krb5.fast_tests.FAST_Tests.test_simple_as_req_self_pac_request_none\( ^samba.tests.krb5.fast_tests.samba.tests.krb5.fast_tests.FAST_Tests.test_simple_as_req_self_pac_request_true\( -# -# Kpasswd tests -# -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_canonicalize.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_change.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_change_expired_password.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_empty.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_from_rodc.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_initial.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_canonicalize.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_seq_number.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_non_initial.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_expired_password.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_target_princ_and_realm_access.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_target_princ_and_realm_no_access.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_target_princ_only.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_set_target_realm_only.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_requester_sid_lifetime_tgs.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_tgs.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_too_weak.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key.ad_dc diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c index 70af9293f6d..9572263a539 100644 --- a/source4/kdc/db-glue.c +++ b/source4/kdc/db-glue.c @@ -931,6 +931,7 @@ static krb5_error_code samba_kdc_get_entry_principal( const char *samAccountName, enum samba_kdc_ent_type ent_type, unsigned flags, + bool is_kadmin_changepw, krb5_const_principal in_princ, krb5_principal *out_princ) { @@ -950,46 +951,52 @@ static krb5_error_code samba_kdc_get_entry_principal( * fixed UPPER case realm, but the as-sent username */ - if (ent_type == SAMBA_KDC_ENT_TYPE_KRBTGT && canon) { - /* - * When requested to do so, ensure that the - * both realm values in the principal are set - * to the upper case, canonical realm - */ - code = smb_krb5_make_principal(context, - out_princ, - lpcfg_realm(lp_ctx), - "krbtgt", - lpcfg_realm(lp_ctx), - NULL); - if (code != 0) { + /* + * We need to ensure that the kadmin/changepw principal isn't able to + * issue krbtgt tickets, even if canonicalization is turned on. + */ + if (!is_kadmin_changepw) { + if (ent_type == SAMBA_KDC_ENT_TYPE_KRBTGT && canon) { + /* + * When requested to do so, ensure that the + * both realm values in the principal are set + * to the upper case, canonical realm + */ + code = smb_krb5_make_principal(context, + out_princ, + lpcfg_realm(lp_ctx), + "krbtgt", + lpcfg_realm(lp_ctx), + NULL); + if (code != 0) { + return code; + } + smb_krb5_principal_set_type(context, + *out_princ, + KRB5_NT_SRV_INST); + + return 0; + } + + if ((canon && flags & (SDB_F_FORCE_CANON|SDB_F_FOR_AS_REQ)) || + (ent_type == SAMBA_KDC_ENT_TYPE_ANY && in_princ == NULL)) { + /* + * SDB_F_CANON maps from the canonicalize flag in the + * packet, and has a different meaning between AS-REQ + * and TGS-REQ. We only change the principal in the + * AS-REQ case. + * + * The SDB_F_FORCE_CANON if for new MIT KDC code that + * wants the canonical name in all lookups, and takes + * care to canonicalize only when appropriate. + */ + code = smb_krb5_make_principal(context, + out_princ, + lpcfg_realm(lp_ctx), + samAccountName, + NULL); return code; } - smb_krb5_principal_set_type(context, - *out_princ, - KRB5_NT_SRV_INST); - - return 0; - } - - if ((canon && flags & (SDB_F_FORCE_CANON|SDB_F_FOR_AS_REQ)) || - (ent_type == SAMBA_KDC_ENT_TYPE_ANY && in_princ == NULL)) { - /* - * SDB_F_CANON maps from the canonicalize flag in the - * packet, and has a different meaning between AS-REQ - * and TGS-REQ. We only change the principal in the - * AS-REQ case. - * - * The SDB_F_FORCE_CANON if for new MIT KDC code that - * wants the canonical name in all lookups, and takes - * care to canonicalize only when appropriate. - */ - code = smb_krb5_make_principal(context, - out_princ, - lpcfg_realm(lp_ctx), - samAccountName, - NULL); - return code; } /* @@ -1305,6 +1312,7 @@ static krb5_error_code samba_kdc_message2entry(krb5_context context, samAccountName, ent_type, flags, + entry->flags.change_pw, principal, &entry->principal); if (ret != 0) { -- 2.35.0 From 4936ff13e1d6eea5f76274a6976c52efa2f51d10 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 17:53:49 +1200 Subject: [PATCH 31/43] CVE-2022-2031 s4:kdc: Limit kpasswd ticket lifetime to two minutes or less This matches the behaviour of Windows. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- selftest/knownfail_heimdal_kdc | 1 - selftest/knownfail_mit_kdc | 1 - source4/kdc/db-glue.c | 5 +++++ source4/kdc/mit-kdb/kdb_samba_principals.c | 1 - source4/kdc/samba_kdc.h | 2 ++ 5 files changed, 7 insertions(+), 3 deletions(-) diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc index 2e523ec6483..ec0dd106f0f 100644 --- a/selftest/knownfail_heimdal_kdc +++ b/selftest/knownfail_heimdal_kdc @@ -58,7 +58,6 @@ # Kpasswd tests # ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_from_rodc.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_lifetime.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_requester_sid_tgs.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_server.ad_dc diff --git a/selftest/knownfail_mit_kdc b/selftest/knownfail_mit_kdc index 4059f2210f4..e1315ed71ac 100644 --- a/selftest/knownfail_mit_kdc +++ b/selftest/knownfail_mit_kdc @@ -438,7 +438,6 @@ samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_ # ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_canonicalize_realm_case.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_canonicalize_realm_case.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_lifetime.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_requester_sid_tgs.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_server.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_service.ad_dc diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c index 9572263a539..dcf2e257447 100644 --- a/source4/kdc/db-glue.c +++ b/source4/kdc/db-glue.c @@ -1337,6 +1337,11 @@ static krb5_error_code samba_kdc_message2entry(krb5_context context, kdc_db_ctx->policy.usr_tkt_lifetime); } + if (entry->flags.change_pw) { + /* Limit lifetime of kpasswd tickets to two minutes or less. */ + *entry->max_life = MIN(*entry->max_life, CHANGEPW_LIFETIME); + } + entry->max_renew = malloc(sizeof(*entry->max_renew)); if (entry->max_renew == NULL) { ret = ENOMEM; diff --git a/source4/kdc/mit-kdb/kdb_samba_principals.c b/source4/kdc/mit-kdb/kdb_samba_principals.c index 31983a7da6c..27260186f0a 100644 --- a/source4/kdc/mit-kdb/kdb_samba_principals.c +++ b/source4/kdc/mit-kdb/kdb_samba_principals.c @@ -35,7 +35,6 @@ #define DBGC_CLASS DBGC_KERBEROS #define ADMIN_LIFETIME 60*60*3 /* 3 hours */ -#define CHANGEPW_LIFETIME 60*5 /* 5 minutes */ krb5_error_code ks_get_principal(krb5_context context, krb5_const_principal principal, diff --git a/source4/kdc/samba_kdc.h b/source4/kdc/samba_kdc.h index 2caefd58ae9..5d73c5bbb9d 100644 --- a/source4/kdc/samba_kdc.h +++ b/source4/kdc/samba_kdc.h @@ -68,4 +68,6 @@ struct samba_kdc_entry { extern struct hdb_method hdb_samba4_interface; +#define CHANGEPW_LIFETIME 60*2 /* 2 minutes */ + #endif /* _SAMBA_KDC_H_ */ -- 2.35.0 From 13243ece31d98060e0893342824b22856ca9fe69 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Mon, 30 May 2022 19:18:17 +1200 Subject: [PATCH 32/43] CVE-2022-2031 s4:kdc: Reject tickets during the last two minutes of their life For Heimdal, this now matches the behaviour of Windows. The object of this requirement is to ensure we don't allow kpasswd tickets, not having a lifetime of more than two minutes, to be passed off as TGTs. An existing requirement for TGTs to contain a REQUESTER_SID PAC buffer suffices to prevent kpasswd ticket misuse, so this is just an additional precaution on top. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- selftest/knownfail_heimdal_kdc | 1 - source4/kdc/wdc-samba4.c | 36 ++++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc index ec0dd106f0f..5321b109fc6 100644 --- a/selftest/knownfail_heimdal_kdc +++ b/selftest/knownfail_heimdal_kdc @@ -58,7 +58,6 @@ # Kpasswd tests # ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_from_rodc.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_requester_sid_tgs.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_server.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_service.ad_dc diff --git a/source4/kdc/wdc-samba4.c b/source4/kdc/wdc-samba4.c index d1825c2902c..9118febdb2d 100644 --- a/source4/kdc/wdc-samba4.c +++ b/source4/kdc/wdc-samba4.c @@ -415,6 +415,42 @@ static krb5_error_code samba_wdc_reget_pac(void *priv, astgs_request_t r, krbtgt = &signing_krbtgt_hdb; } } + } else { + /* + * We expect to have received a TGT, so check that we haven't + * been given a kpasswd ticket instead. + */ + krb5_ticket *tgt = kdc_request_get_ticket(r); + + struct timeval now; + KerberosTime lifetime; + + /* + * We use the same function to get the current time as Heimdal + * does (see krb5_kdc_update_time()), to ensure our time is + * consistent. Otherwise tickets could slip through if our time + * value comes before Heimdal's chronologically. + */ + ret = gettimeofday(&now, NULL); + if (ret != 0) { + return errno; + } + + /* + * Check if the ticket is in the last two minutes of its + * life. + */ + lifetime = rk_time_sub(tgt->ticket.endtime, now.tv_sec); + if (lifetime <= CHANGEPW_LIFETIME) { + /* + * This ticket has at most two minutes left to live. It + * may be a kpasswd ticket rather than a TGT, so don't + * accept it. + */ + kdc_audit_addreason((kdc_request_t)r, + "Ticket is not a ticket-granting ticket"); + return KRB5KRB_AP_ERR_TKT_EXPIRED; + } } ret = samba_wdc_reget_pac2(r, -- 2.35.0 From 1f0fd86110618dfd1944951dbe69280b3ac9fafb Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 17:52:05 +1200 Subject: [PATCH 33/43] CVE-2022-32744 s4:kdc: Don't allow HDB keytab iteration A fallback in krb5_rd_req_ctx() means that Samba's kpasswd service will try many inappropriate keys to decrypt the ticket supplied to it. For example, it will accept a ticket encrypted with the Administrator's key, when it should rather accept only tickets encrypted with the krbtgt's key (and not an RODC krbtgt). To fix this, declare the HDB keytab using the HDBGET ops, which do not support iteration. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- selftest/knownfail_heimdal_kdc | 1 - source4/kdc/kdc-heimdal.c | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc index 5321b109fc6..341cdfb21c1 100644 --- a/selftest/knownfail_heimdal_kdc +++ b/selftest/knownfail_heimdal_kdc @@ -57,7 +57,6 @@ # # Kpasswd tests # -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_from_rodc.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_server.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_service.ad_dc diff --git a/source4/kdc/kdc-heimdal.c b/source4/kdc/kdc-heimdal.c index 0d2a410fc3b..542986c5ad3 100644 --- a/source4/kdc/kdc-heimdal.c +++ b/source4/kdc/kdc-heimdal.c @@ -463,7 +463,7 @@ static void kdc_post_fork(struct task_server *task, struct process_details *pd) return; } - kdc->keytab_name = talloc_asprintf(kdc, "HDB:samba4:&%p", kdc->base_ctx); + kdc->keytab_name = talloc_asprintf(kdc, "HDBGET:samba4:&%p", kdc->base_ctx); if (kdc->keytab_name == NULL) { task_server_terminate(task, "kdc: Failed to set keytab name", @@ -471,7 +471,7 @@ static void kdc_post_fork(struct task_server *task, struct process_details *pd) return; } - ret = krb5_kt_register(kdc->smb_krb5_context->krb5_context, &hdb_kt_ops); + ret = krb5_kt_register(kdc->smb_krb5_context->krb5_context, &hdb_get_kt_ops); if(ret) { task_server_terminate(task, "kdc: failed to register keytab plugin", true); return; -- 2.35.0 From 09a5930a80e4cadaddf9756b0cf135040d387e40 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 14 Jun 2022 15:23:55 +1200 Subject: [PATCH 34/43] CVE-2022-2031 tests/krb5: Test truncated forms of server principals We should not be able to use krb@REALM instead of krbtgt@REALM. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- python/samba/tests/krb5/as_req_tests.py | 30 ++++++++++++++++++++++--- selftest/knownfail_heimdal_kdc | 4 ++++ selftest/knownfail_mit_kdc | 4 ++++ 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/python/samba/tests/krb5/as_req_tests.py b/python/samba/tests/krb5/as_req_tests.py index b52937530e6..6a573947067 100755 --- a/python/samba/tests/krb5/as_req_tests.py +++ b/python/samba/tests/krb5/as_req_tests.py @@ -28,6 +28,7 @@ import samba.tests.krb5.kcrypto as kcrypto import samba.tests.krb5.rfc4120_pyasn1 as krb5_asn1 from samba.tests.krb5.rfc4120_constants import ( KDC_ERR_C_PRINCIPAL_UNKNOWN, + KDC_ERR_S_PRINCIPAL_UNKNOWN, KDC_ERR_ETYPE_NOSUPP, KDC_ERR_PREAUTH_REQUIRED, KU_PA_ENC_TIMESTAMP, @@ -43,7 +44,7 @@ global_hexdump = False class AsReqBaseTest(KDCBaseTest): def _run_as_req_enc_timestamp(self, client_creds, client_account=None, - expected_cname=None, + expected_cname=None, sname=None, name_type=NT_PRINCIPAL, etypes=None, expected_error=None, expect_edata=None, kdc_options=None): @@ -59,8 +60,9 @@ class AsReqBaseTest(KDCBaseTest): cname = self.PrincipalName_create(name_type=name_type, names=client_account.split('/')) - sname = self.PrincipalName_create(name_type=NT_SRV_INST, - names=[krbtgt_account, realm]) + if sname is None: + sname = self.PrincipalName_create(name_type=NT_SRV_INST, + names=[krbtgt_account, realm]) expected_crealm = realm if expected_cname is None: @@ -492,6 +494,28 @@ class AsReqKerberosTests(AsReqBaseTest): name_type=NT_ENTERPRISE_PRINCIPAL, kdc_options=0) + # Ensure we can't use truncated well-known principals such as krb@REALM + # instead of krbtgt@REALM. + def test_krbtgt_wrong_principal(self): + client_creds = self.get_client_creds() + + krbtgt_creds = self.get_krbtgt_creds() + + krbtgt_account = krbtgt_creds.get_username() + realm = krbtgt_creds.get_realm() + + # Truncate the name of the krbtgt principal. + krbtgt_account = krbtgt_account[:3] + + wrong_krbtgt_princ = self.PrincipalName_create( + name_type=NT_SRV_INST, + names=[krbtgt_account, realm]) + + self._run_as_req_enc_timestamp( + client_creds, + sname=wrong_krbtgt_princ, + expected_error=KDC_ERR_S_PRINCIPAL_UNKNOWN) + if __name__ == "__main__": global_asn1_print = False diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc index 341cdfb21c1..403735d09b9 100644 --- a/selftest/knownfail_heimdal_kdc +++ b/selftest/knownfail_heimdal_kdc @@ -60,3 +60,7 @@ ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_server.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_service.ad_dc +# +# AS-REQ tests +# +^samba.tests.krb5.as_req_tests.samba.tests.krb5.as_req_tests.AsReqKerberosTests.test_krbtgt_wrong_principal\( diff --git a/selftest/knownfail_mit_kdc b/selftest/knownfail_mit_kdc index e1315ed71ac..d1413f10692 100644 --- a/selftest/knownfail_mit_kdc +++ b/selftest/knownfail_mit_kdc @@ -441,3 +441,7 @@ samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_ ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_requester_sid_tgs.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_server.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_service.ad_dc +# +# AS-REQ tests +# +^samba.tests.krb5.as_req_tests.samba.tests.krb5.as_req_tests.AsReqKerberosTests.test_krbtgt_wrong_principal\( -- 2.35.0 From 759363990b1997332ce2d0f7742f886c7721d89c Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 25 May 2022 20:00:55 +1200 Subject: [PATCH 35/43] CVE-2022-2031 s4:kdc: Don't use strncmp to compare principal components We would only compare the first 'n' characters, where 'n' is the length of the principal component string, so 'k@REALM' would erroneously be considered equal to 'krbtgt@REALM'. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- selftest/knownfail_heimdal_kdc | 4 ---- selftest/knownfail_mit_kdc | 4 ---- source4/kdc/db-glue.c | 27 ++++++++++++++++++++++----- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc index 403735d09b9..341cdfb21c1 100644 --- a/selftest/knownfail_heimdal_kdc +++ b/selftest/knownfail_heimdal_kdc @@ -60,7 +60,3 @@ ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_server.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_service.ad_dc -# -# AS-REQ tests -# -^samba.tests.krb5.as_req_tests.samba.tests.krb5.as_req_tests.AsReqKerberosTests.test_krbtgt_wrong_principal\( diff --git a/selftest/knownfail_mit_kdc b/selftest/knownfail_mit_kdc index d1413f10692..e1315ed71ac 100644 --- a/selftest/knownfail_mit_kdc +++ b/selftest/knownfail_mit_kdc @@ -441,7 +441,3 @@ samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_ ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_requester_sid_tgs.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_server.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_service.ad_dc -# -# AS-REQ tests -# -^samba.tests.krb5.as_req_tests.samba.tests.krb5.as_req_tests.AsReqKerberosTests.test_krbtgt_wrong_principal\( diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c index dcf2e257447..ba46161fac1 100644 --- a/source4/kdc/db-glue.c +++ b/source4/kdc/db-glue.c @@ -870,15 +870,19 @@ static int principal_comp_strcmp_int(krb5_context context, bool do_strcasecmp) { const char *p; - size_t len; #if defined(HAVE_KRB5_PRINCIPAL_GET_COMP_STRING) p = krb5_principal_get_comp_string(context, principal, component); if (p == NULL) { return -1; } - len = strlen(p); + if (do_strcasecmp) { + return strcasecmp(p, string); + } else { + return strcmp(p, string); + } #else + size_t len; krb5_data *d; if (component >= krb5_princ_size(context, principal)) { return -1; @@ -890,13 +894,26 @@ static int principal_comp_strcmp_int(krb5_context context, } p = d->data; - len = d->length; -#endif + + len = strlen(string); + + /* + * We explicitly return -1 or 1. Subtracting of the two lengths might + * give the wrong result if the result overflows or loses data when + * narrowed to int. + */ + if (d->length < len) { + return -1; + } else if (d->length > len) { + return 1; + } + if (do_strcasecmp) { return strncasecmp(p, string, len); } else { - return strncmp(p, string, len); + return memcmp(p, string, len); } +#endif } static int principal_comp_strcasecmp(krb5_context context, -- 2.35.0 From 5f92c5bd6fdc19ae09be822978ad33c9bf153886 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Thu, 26 May 2022 16:36:30 +1200 Subject: [PATCH 36/43] CVE-2022-32744 s4:kdc: Rename keytab_name -> kpasswd_keytab_name This makes explicitly clear the purpose of this keytab. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- source4/kdc/kdc-heimdal.c | 4 ++-- source4/kdc/kdc-server.h | 2 +- source4/kdc/kdc-service-mit.c | 4 ++-- source4/kdc/kpasswd-service.c | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/source4/kdc/kdc-heimdal.c b/source4/kdc/kdc-heimdal.c index 542986c5ad3..5b2b3e36652 100644 --- a/source4/kdc/kdc-heimdal.c +++ b/source4/kdc/kdc-heimdal.c @@ -463,8 +463,8 @@ static void kdc_post_fork(struct task_server *task, struct process_details *pd) return; } - kdc->keytab_name = talloc_asprintf(kdc, "HDBGET:samba4:&%p", kdc->base_ctx); - if (kdc->keytab_name == NULL) { + kdc->kpasswd_keytab_name = talloc_asprintf(kdc, "HDBGET:samba4:&%p", kdc->base_ctx); + if (kdc->kpasswd_keytab_name == NULL) { task_server_terminate(task, "kdc: Failed to set keytab name", true); diff --git a/source4/kdc/kdc-server.h b/source4/kdc/kdc-server.h index fd883c2e4b4..89b30f122f5 100644 --- a/source4/kdc/kdc-server.h +++ b/source4/kdc/kdc-server.h @@ -40,7 +40,7 @@ struct kdc_server { struct ldb_context *samdb; bool am_rodc; uint32_t proxy_timeout; - const char *keytab_name; + const char *kpasswd_keytab_name; void *private_data; }; diff --git a/source4/kdc/kdc-service-mit.c b/source4/kdc/kdc-service-mit.c index f9aaedefc23..31cfef2eae1 100644 --- a/source4/kdc/kdc-service-mit.c +++ b/source4/kdc/kdc-service-mit.c @@ -307,8 +307,8 @@ NTSTATUS mitkdc_task_init(struct task_server *task) return NT_STATUS_INTERNAL_ERROR; } - kdc->keytab_name = talloc_asprintf(kdc, "KDB:"); - if (kdc->keytab_name == NULL) { + kdc->kpasswd_keytab_name = talloc_asprintf(kdc, "KDB:"); + if (kdc->kpasswd_keytab_name == NULL) { task_server_terminate(task, "KDC: Out of memory", true); diff --git a/source4/kdc/kpasswd-service.c b/source4/kdc/kpasswd-service.c index 379ddebf3ad..aec30850173 100644 --- a/source4/kdc/kpasswd-service.c +++ b/source4/kdc/kpasswd-service.c @@ -170,7 +170,7 @@ kdc_code kpasswd_process(struct kdc_server *kdc, rv = cli_credentials_set_keytab_name(server_credentials, kdc->task->lp_ctx, - kdc->keytab_name, + kdc->kpasswd_keytab_name, CRED_SPECIFIED); if (rv != 0) { DBG_ERR("Failed to set credentials keytab name\n"); -- 2.35.0 From d36bc93b4cdfbb1f2b18a494f0c47a86c2268802 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 8 Jun 2022 13:53:29 +1200 Subject: [PATCH 37/43] s4:kdc: Remove kadmin mode from HDB plugin It appears we no longer require it. Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- source4/kdc/hdb-samba4-plugin.c | 33 ++++++--------------------------- 1 file changed, 6 insertions(+), 27 deletions(-) diff --git a/source4/kdc/hdb-samba4-plugin.c b/source4/kdc/hdb-samba4-plugin.c index 9dc4784f379..7a5a9485e05 100644 --- a/source4/kdc/hdb-samba4-plugin.c +++ b/source4/kdc/hdb-samba4-plugin.c @@ -21,40 +21,20 @@ #include "includes.h" #include "kdc/kdc-glue.h" -#include "kdc/db-glue.h" -#include "lib/util/samba_util.h" #include "lib/param/param.h" -#include "source4/lib/events/events.h" static krb5_error_code hdb_samba4_create(krb5_context context, struct HDB **db, const char *arg) { NTSTATUS nt_status; - void *ptr; - struct samba_kdc_base_context *base_ctx; - - if (sscanf(arg, "&%p", &ptr) == 1) { - base_ctx = talloc_get_type_abort(ptr, struct samba_kdc_base_context); - } else if (arg[0] == '\0' || file_exist(arg)) { - /* This mode for use in kadmin, rather than in Samba */ - - setup_logging("hdb_samba4", DEBUG_DEFAULT_STDERR); + void *ptr = NULL; + struct samba_kdc_base_context *base_ctx = NULL; - base_ctx = talloc_zero(NULL, struct samba_kdc_base_context); - if (!base_ctx) { - return ENOMEM; - } - - base_ctx->ev_ctx = s4_event_context_init(base_ctx); - base_ctx->lp_ctx = loadparm_init_global(false); - if (arg[0]) { - lpcfg_load(base_ctx->lp_ctx, arg); - } else { - lpcfg_load_default(base_ctx->lp_ctx); - } - } else { + if (sscanf(arg, "&%p", &ptr) != 1) { return EINVAL; } + base_ctx = talloc_get_type_abort(ptr, struct samba_kdc_base_context); + /* The global kdc_mem_ctx and kdc_lp_ctx, Disgusting, ugly hack, but it means one less private hook */ nt_status = hdb_samba4_create_kdc(base_ctx, context, db); @@ -90,8 +70,7 @@ static void hdb_samba4_fini(void *ctx) /* Only used in the hdb-backed keytab code * for a keytab of 'samba4&
' or samba4, to find - * kpasswd's key in the main DB, and to - * copy all the keys into a file (libnet_keytab_export) + * kpasswd's key in the main DB * * The
is the string form of a pointer to a talloced struct hdb_samba_context */ -- 2.35.0 From 145e3683d4ddfce73ae5d9c5051072f483819c41 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Thu, 26 May 2022 16:39:20 +1200 Subject: [PATCH 38/43] CVE-2022-32744 s4:kdc: Modify HDB plugin to only look up kpasswd principal This plugin is now only used by the kpasswd service. Thus, ensuring we only look up the kadmin/changepw principal means we can't be fooled into accepting tickets for other service principals. We make sure not to specify a specific kvno, to ensure that we do not accept RODC-issued tickets. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- selftest/knownfail_heimdal_kdc | 6 ---- source4/kdc/hdb-samba4-plugin.c | 2 +- source4/kdc/hdb-samba4.c | 58 +++++++++++++++++++++++++++++++++ source4/kdc/kdc-glue.h | 3 ++ 4 files changed, 62 insertions(+), 7 deletions(-) diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc index 341cdfb21c1..4ae27eacb09 100644 --- a/selftest/knownfail_heimdal_kdc +++ b/selftest/knownfail_heimdal_kdc @@ -54,9 +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 -# -# Kpasswd tests -# -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_server.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_service.ad_dc diff --git a/source4/kdc/hdb-samba4-plugin.c b/source4/kdc/hdb-samba4-plugin.c index 7a5a9485e05..be6d2437d0e 100644 --- a/source4/kdc/hdb-samba4-plugin.c +++ b/source4/kdc/hdb-samba4-plugin.c @@ -36,7 +36,7 @@ static krb5_error_code hdb_samba4_create(krb5_context context, struct HDB **db, base_ctx = talloc_get_type_abort(ptr, struct samba_kdc_base_context); /* The global kdc_mem_ctx and kdc_lp_ctx, Disgusting, ugly hack, but it means one less private hook */ - nt_status = hdb_samba4_create_kdc(base_ctx, context, db); + nt_status = hdb_samba4_kpasswd_create_kdc(base_ctx, context, db); if (NT_STATUS_IS_OK(nt_status)) { return 0; diff --git a/source4/kdc/hdb-samba4.c b/source4/kdc/hdb-samba4.c index 480d2c06e5e..fbd44f47d3b 100644 --- a/source4/kdc/hdb-samba4.c +++ b/source4/kdc/hdb-samba4.c @@ -293,6 +293,47 @@ static krb5_error_code hdb_samba4_fetch_kvno(krb5_context context, HDB *db, return code; } +static krb5_error_code hdb_samba4_kpasswd_fetch_kvno(krb5_context context, HDB *db, + krb5_const_principal _principal, + unsigned flags, + krb5_kvno _kvno, + hdb_entry *entry) +{ + struct samba_kdc_db_context *kdc_db_ctx = NULL; + krb5_error_code ret; + krb5_principal kpasswd_principal = NULL; + + kdc_db_ctx = talloc_get_type_abort(db->hdb_db, + struct samba_kdc_db_context); + + ret = smb_krb5_make_principal(context, &kpasswd_principal, + lpcfg_realm(kdc_db_ctx->lp_ctx), + "kadmin", "changepw", + NULL); + if (ret) { + return ret; + } + smb_krb5_principal_set_type(context, kpasswd_principal, KRB5_NT_SRV_INST); + + /* + * For the kpasswd service, always ensure we get the latest kvno. This + * also means we (correctly) refuse RODC-issued tickets. + */ + flags &= ~HDB_F_KVNO_SPECIFIED; + + /* Don't bother looking up a client or krbtgt. */ + flags &= ~(SDB_F_GET_CLIENT|SDB_F_GET_KRBTGT); + + ret = hdb_samba4_fetch_kvno(context, db, + kpasswd_principal, + flags, + 0, + entry); + + krb5_free_principal(context, kpasswd_principal); + return ret; +} + static krb5_error_code hdb_samba4_firstkey(krb5_context context, HDB *db, unsigned flags, hdb_entry *entry) { @@ -813,3 +854,20 @@ NTSTATUS hdb_samba4_create_kdc(struct samba_kdc_base_context *base_ctx, return NT_STATUS_OK; } + +NTSTATUS hdb_samba4_kpasswd_create_kdc(struct samba_kdc_base_context *base_ctx, + krb5_context context, struct HDB **db) +{ + NTSTATUS nt_status; + + nt_status = hdb_samba4_create_kdc(base_ctx, context, db); + if (!NT_STATUS_IS_OK(nt_status)) { + return nt_status; + } + + (*db)->hdb_fetch_kvno = hdb_samba4_kpasswd_fetch_kvno; + (*db)->hdb_firstkey = NULL; + (*db)->hdb_nextkey = NULL; + + return NT_STATUS_OK; +} diff --git a/source4/kdc/kdc-glue.h b/source4/kdc/kdc-glue.h index 47642e12432..7a0184c4021 100644 --- a/source4/kdc/kdc-glue.h +++ b/source4/kdc/kdc-glue.h @@ -46,6 +46,9 @@ kdc_code kpasswdd_process(struct kdc_server *kdc, NTSTATUS hdb_samba4_create_kdc(struct samba_kdc_base_context *base_ctx, krb5_context context, struct HDB **db); +NTSTATUS hdb_samba4_kpasswd_create_kdc(struct samba_kdc_base_context *base_ctx, + krb5_context context, struct HDB **db); + /* from kdc-glue.c */ int kdc_check_pac(krb5_context krb5_context, DATA_BLOB server_sig, -- 2.35.0 From 7159581278002673e5b6468be86a5a6d77fd108e Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Mon, 30 May 2022 19:16:02 +1200 Subject: [PATCH 39/43] CVE-2022-32744 s4:kpasswd: Ensure we pass the kpasswd server principal into krb5_rd_req_ctx() To ensure that, when decrypting the kpasswd ticket, we look up the correct principal and don't trust the sname from the ticket, we should pass the principal name of the kpasswd service into krb5_rd_req_ctx(). However, gensec_krb5_update_internal() will pass in NULL unless the principal in our credentials is CRED_SPECIFIED. At present, our principal will be considered obtained as CRED_SMB_CONF (from the cli_credentials_set_conf() a few lines up), so we explicitly set the realm again, but this time as CRED_SPECIFIED. Now the value of server_in_keytab that we provide to smb_krb5_rd_req_decoded() will not be NULL. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- selftest/knownfail_mit_kdc | 2 -- source4/kdc/kpasswd-service.c | 30 ++++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/selftest/knownfail_mit_kdc b/selftest/knownfail_mit_kdc index e1315ed71ac..8601da3d79a 100644 --- a/selftest/knownfail_mit_kdc +++ b/selftest/knownfail_mit_kdc @@ -439,5 +439,3 @@ samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_ ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_canonicalize_realm_case.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_canonicalize_realm_case.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_requester_sid_tgs.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_server.ad_dc -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_service.ad_dc diff --git a/source4/kdc/kpasswd-service.c b/source4/kdc/kpasswd-service.c index aec30850173..d2f1bb02906 100644 --- a/source4/kdc/kpasswd-service.c +++ b/source4/kdc/kpasswd-service.c @@ -29,6 +29,7 @@ #include "kdc/kdc-server.h" #include "kdc/kpasswd-service.h" #include "kdc/kpasswd-helper.h" +#include "param/param.h" #define HEADER_LEN 6 #ifndef RFC3244_VERSION @@ -161,6 +162,20 @@ kdc_code kpasswd_process(struct kdc_server *kdc, goto done; } + /* + * After calling cli_credentials_set_conf(), explicitly set the realm + * with CRED_SPECIFIED. We need to do this so the result of + * principal_from_credentials() called from the gensec layer is + * CRED_SPECIFIED rather than CRED_SMB_CONF, avoiding a fallback to + * match-by-key (very undesirable in this case). + */ + ok = cli_credentials_set_realm(server_credentials, + lpcfg_realm(kdc->task->lp_ctx), + CRED_SPECIFIED); + if (!ok) { + goto done; + } + ok = cli_credentials_set_username(server_credentials, "kadmin/changepw", CRED_SPECIFIED); @@ -168,6 +183,21 @@ kdc_code kpasswd_process(struct kdc_server *kdc, goto done; } + /* Check that the server principal is indeed CRED_SPECIFIED. */ + { + char *principal = NULL; + enum credentials_obtained obtained; + + principal = cli_credentials_get_principal_and_obtained(server_credentials, + tmp_ctx, + &obtained); + if (obtained < CRED_SPECIFIED) { + goto done; + } + + TALLOC_FREE(principal); + } + rv = cli_credentials_set_keytab_name(server_credentials, kdc->task->lp_ctx, kdc->kpasswd_keytab_name, -- 2.35.0 From 95c27d73386dd05c0c11b7f5f4856d493009b2cb Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Fri, 10 Jun 2022 19:17:11 +1200 Subject: [PATCH 40/43] CVE-2022-2031 tests/krb5: Add test that we cannot provide a TGT to kpasswd The kpasswd service should require a kpasswd service ticket, and disallow TGTs. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- python/samba/tests/krb5/kpasswd_tests.py | 28 ++++++++++++++++++++++++ selftest/knownfail_heimdal_kdc | 4 ++++ selftest/knownfail_mit_kdc | 4 ++++ 3 files changed, 36 insertions(+) diff --git a/python/samba/tests/krb5/kpasswd_tests.py b/python/samba/tests/krb5/kpasswd_tests.py index 3a6c7d818dc..0db857f7bbd 100755 --- a/python/samba/tests/krb5/kpasswd_tests.py +++ b/python/samba/tests/krb5/kpasswd_tests.py @@ -31,6 +31,7 @@ from samba.tests.krb5.rfc4120_constants import ( KDC_ERR_TGT_REVOKED, KDC_ERR_TKT_EXPIRED, KPASSWD_ACCESSDENIED, + KPASSWD_AUTHERROR, KPASSWD_HARDERROR, KPASSWD_INITIAL_FLAG_NEEDED, KPASSWD_MALFORMED, @@ -779,6 +780,33 @@ class KpasswdTests(KDCBaseTest): self._make_tgs_request(creds, service_creds, ticket, expect_error=False) + # Show that we cannot provide a TGT to kpasswd to change the password. + def test_kpasswd_tgt(self): + # Create an account for testing, and get a TGT. + creds = self._get_creds() + tgt = self.get_tgt(creds) + + # Change the sname of the ticket to match that of kadmin/changepw. + tgt.set_sname(self.get_kpasswd_sname()) + + expected_code = KPASSWD_AUTHERROR + expected_msg = b'A TGT may not be used as a ticket to kpasswd' + + # Set the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(tgt, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET) + + # Change the password. + self.kpasswd_exchange(tgt, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.CHANGE) + # Test that kpasswd rejects requests with a service ticket. def test_kpasswd_non_initial(self): # Create an account for testing, and get a TGT. diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc index 4ae27eacb09..0097862aea8 100644 --- a/selftest/knownfail_heimdal_kdc +++ b/selftest/knownfail_heimdal_kdc @@ -54,3 +54,7 @@ ^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 +# +# Kpasswd tests +# +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_tgt.ad_dc diff --git a/selftest/knownfail_mit_kdc b/selftest/knownfail_mit_kdc index 8601da3d79a..b0a1a1c35e0 100644 --- a/selftest/knownfail_mit_kdc +++ b/selftest/knownfail_mit_kdc @@ -439,3 +439,7 @@ samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_ ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_canonicalize_realm_case.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_canonicalize_realm_case.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_requester_sid_tgs.ad_dc +# +# Kpasswd tests +# +samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_tgt.ad_dc -- 2.35.0 From 1454d16884e59ccd47e094f79cc78ba2d90a44e3 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Fri, 10 Jun 2022 19:18:07 +1200 Subject: [PATCH 41/43] CVE-2022-2031 auth: Add ticket type field to auth_user_info_dc and auth_session_info This field may be used to convey whether we were provided with a TGT or a non-TGT. We ensure both structures are zeroed out to avoid incorrect results being produced by an uninitialised field. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- auth/auth_sam_reply.c | 2 +- auth/auth_util.c | 2 +- librpc/idl/auth.idl | 23 +++++++++++++++++++++++ source4/auth/ntlm/auth_developer.c | 2 +- source4/auth/sam.c | 2 +- source4/auth/session.c | 2 ++ source4/auth/system_session.c | 6 +++--- 7 files changed, 32 insertions(+), 7 deletions(-) diff --git a/auth/auth_sam_reply.c b/auth/auth_sam_reply.c index fda014c87d5..173a5132964 100644 --- a/auth/auth_sam_reply.c +++ b/auth/auth_sam_reply.c @@ -416,7 +416,7 @@ NTSTATUS make_user_info_dc_netlogon_validation(TALLOC_CTX *mem_ctx, return NT_STATUS_INVALID_LEVEL; } - user_info_dc = talloc(mem_ctx, struct auth_user_info_dc); + user_info_dc = talloc_zero(mem_ctx, struct auth_user_info_dc); NT_STATUS_HAVE_NO_MEMORY(user_info_dc); /* diff --git a/auth/auth_util.c b/auth/auth_util.c index fe01babd107..ec9094d0f15 100644 --- a/auth/auth_util.c +++ b/auth/auth_util.c @@ -44,7 +44,7 @@ struct auth_session_info *copy_session_info(TALLOC_CTX *mem_ctx, return NULL; } - dst = talloc(mem_ctx, struct auth_session_info); + dst = talloc_zero(mem_ctx, struct auth_session_info); if (dst == NULL) { DBG_ERR("talloc failed\n"); TALLOC_FREE(frame); diff --git a/librpc/idl/auth.idl b/librpc/idl/auth.idl index 7de3d4c6bfb..59ed2c3c5ea 100644 --- a/librpc/idl/auth.idl +++ b/librpc/idl/auth.idl @@ -75,6 +75,26 @@ interface auth [unique,charset(UTF8),string] char *sanitized_username; } auth_user_info_unix; + /* + * If the user was authenticated with a Kerberos ticket, this indicates + * the type of the ticket; TGT, or non-TGT (i.e. service ticket). If + * unset, the type is unknown. This indicator is useful for the KDC and + * the kpasswd service, which share the same account and keys. By + * ensuring it is provided with the appopriate ticket type, each service + * avoids accepting a ticket meant for the other. + * + * The heuristic used to determine the type is the presence or absence + * of a REQUESTER_SID buffer in the PAC; we use its presence to assume + * we have a TGT. This heuristic will fail for older Samba versions and + * Windows prior to Nov. 2021 updates, which lack support for this + * buffer. + */ + typedef enum { + TICKET_TYPE_UNKNOWN = 0, + TICKET_TYPE_TGT = 1, + TICKET_TYPE_NON_TGT = 2 + } ticket_type; + /* This is the interim product of the auth subsystem, before * privileges and local groups are handled */ typedef [public] struct { @@ -83,6 +103,7 @@ interface auth auth_user_info *info; [noprint] DATA_BLOB user_session_key; [noprint] DATA_BLOB lm_session_key; + ticket_type ticket_type; } auth_user_info_dc; typedef [public] struct { @@ -112,6 +133,8 @@ interface auth * We generate this in auth_generate_session_info() */ GUID unique_session_token; + + ticket_type ticket_type; } auth_session_info; typedef [public] struct { diff --git a/source4/auth/ntlm/auth_developer.c b/source4/auth/ntlm/auth_developer.c index 1823989c68d..6e92252d5c5 100644 --- a/source4/auth/ntlm/auth_developer.c +++ b/source4/auth/ntlm/auth_developer.c @@ -76,7 +76,7 @@ static NTSTATUS name_to_ntstatus_check_password(struct auth_method_context *ctx, } NT_STATUS_NOT_OK_RETURN(nt_status); - user_info_dc = talloc(mem_ctx, struct auth_user_info_dc); + user_info_dc = talloc_zero(mem_ctx, struct auth_user_info_dc); NT_STATUS_HAVE_NO_MEMORY(user_info_dc); /* This returns a pointer to a struct dom_sid, which is the diff --git a/source4/auth/sam.c b/source4/auth/sam.c index 4bbf9aadc81..420b165446a 100644 --- a/source4/auth/sam.c +++ b/source4/auth/sam.c @@ -363,7 +363,7 @@ _PUBLIC_ NTSTATUS authsam_make_user_info_dc(TALLOC_CTX *mem_ctx, TALLOC_CTX *tmp_ctx; struct ldb_message_element *el; - user_info_dc = talloc(mem_ctx, struct auth_user_info_dc); + user_info_dc = talloc_zero(mem_ctx, struct auth_user_info_dc); NT_STATUS_HAVE_NO_MEMORY(user_info_dc); tmp_ctx = talloc_new(user_info_dc); diff --git a/source4/auth/session.c b/source4/auth/session.c index 8cf8670d848..34ad557eebb 100644 --- a/source4/auth/session.c +++ b/source4/auth/session.c @@ -222,6 +222,8 @@ _PUBLIC_ NTSTATUS auth_generate_session_info(TALLOC_CTX *mem_ctx, session_info->credentials = NULL; + session_info->ticket_type = user_info_dc->ticket_type; + talloc_steal(mem_ctx, session_info); *_session_info = session_info; talloc_free(tmp_ctx); diff --git a/source4/auth/system_session.c b/source4/auth/system_session.c index e46b4584817..17cfc4bab8b 100644 --- a/source4/auth/system_session.c +++ b/source4/auth/system_session.c @@ -119,7 +119,7 @@ NTSTATUS auth_system_user_info_dc(TALLOC_CTX *mem_ctx, const char *netbios_name, struct auth_user_info_dc *user_info_dc; struct auth_user_info *info; - user_info_dc = talloc(mem_ctx, struct auth_user_info_dc); + user_info_dc = talloc_zero(mem_ctx, struct auth_user_info_dc); NT_STATUS_HAVE_NO_MEMORY(user_info_dc); /* This returns a pointer to a struct dom_sid, which is the @@ -195,7 +195,7 @@ static NTSTATUS auth_domain_admin_user_info_dc(TALLOC_CTX *mem_ctx, struct auth_user_info_dc *user_info_dc; struct auth_user_info *info; - user_info_dc = talloc(mem_ctx, struct auth_user_info_dc); + user_info_dc = talloc_zero(mem_ctx, struct auth_user_info_dc); NT_STATUS_HAVE_NO_MEMORY(user_info_dc); user_info_dc->num_sids = 7; @@ -364,7 +364,7 @@ _PUBLIC_ NTSTATUS auth_anonymous_user_info_dc(TALLOC_CTX *mem_ctx, { struct auth_user_info_dc *user_info_dc; struct auth_user_info *info; - user_info_dc = talloc(mem_ctx, struct auth_user_info_dc); + user_info_dc = talloc_zero(mem_ctx, struct auth_user_info_dc); NT_STATUS_HAVE_NO_MEMORY(user_info_dc); /* This returns a pointer to a struct dom_sid, which is the -- 2.35.0 From fead884b51d9c8ca695254f92f6002e3870542cd Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Fri, 10 Jun 2022 19:18:35 +1200 Subject: [PATCH 42/43] CVE-2022-2031 s4:auth: Use PAC to determine whether ticket is a TGT We use the presence or absence of a REQUESTER_SID PAC buffer to determine whether the ticket is a TGT. We will later use this to reject TGTs where a service ticket is expected. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- source4/auth/kerberos/kerberos_pac.c | 44 ++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/source4/auth/kerberos/kerberos_pac.c b/source4/auth/kerberos/kerberos_pac.c index dccf165384e..156a140b1de 100644 --- a/source4/auth/kerberos/kerberos_pac.c +++ b/source4/auth/kerberos/kerberos_pac.c @@ -282,6 +282,28 @@ return ret; } +static krb5_error_code kerberos_pac_buffer_present(krb5_context context, + const krb5_pac pac, + uint32_t type) +{ +#ifdef SAMBA4_USES_HEIMDAL + return krb5_pac_get_buffer(context, pac, type, NULL); +#else /* MIT */ + krb5_error_code ret; + krb5_data data; + + /* + * MIT won't let us pass NULL for the data parameter, so we are forced + * to allocate a new buffer and then immediately free it. + */ + ret = krb5_pac_get_buffer(context, pac, type, &data); + if (ret == 0) { + krb5_free_data_contents(context, &data); + } + return ret; +#endif /* SAMBA4_USES_HEIMDAL */ +} + krb5_error_code kerberos_pac_to_user_info_dc(TALLOC_CTX *mem_ctx, krb5_pac pac, krb5_context context, @@ -420,6 +442,28 @@ krb5_error_code kerberos_pac_to_user_info_dc(TALLOC_CTX *mem_ctx, return EINVAL; } } + + /* + * Based on the presence of a REQUESTER_SID PAC buffer, ascertain + * whether the ticket is a TGT. This helps the KDC and kpasswd service + * ensure they do not accept tickets meant for the other. + * + * This heuristic will fail for older Samba versions and Windows prior + * to Nov. 2021 updates, which lack support for the REQUESTER_SID PAC + * buffer. + */ + ret = kerberos_pac_buffer_present(context, pac, PAC_TYPE_REQUESTER_SID); + if (ret == ENOENT) { + /* This probably isn't a TGT. */ + user_info_dc_out->ticket_type = TICKET_TYPE_NON_TGT; + } else if (ret != 0) { + talloc_free(tmp_ctx); + return ret; + } else { + /* This probably is a TGT. */ + user_info_dc_out->ticket_type = TICKET_TYPE_TGT; + } + *user_info_dc = user_info_dc_out; return 0; -- 2.35.0 From 403f9e86893cb645be04a06806b19bae5b5189c5 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Fri, 10 Jun 2022 19:18:53 +1200 Subject: [PATCH 43/43] CVE-2022-2031 s4:kpasswd: Do not accept TGTs as kpasswd tickets If TGTs can be used as kpasswd tickets, the two-minute lifetime of a authentic kpasswd ticket may be bypassed. Furthermore, kpasswd tickets are not supposed to be cached, but using this flaw, a stolen credentials cache containing a TGT may be used to change that account's password, and thus is made more valuable to an attacker. Since all TGTs should be issued with a REQUESTER_SID PAC buffer, and service tickets without it, we assert the absence of this buffer to ensure we're not accepting a TGT. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- selftest/knownfail_heimdal_kdc | 4 ---- selftest/knownfail_mit_kdc | 4 ---- source4/kdc/kpasswd-helper.c | 20 ++++++++++++++++++++ source4/kdc/kpasswd-helper.h | 2 ++ source4/kdc/kpasswd-service-heimdal.c | 13 +++++++++++++ source4/kdc/kpasswd-service-mit.c | 13 +++++++++++++ 6 files changed, 48 insertions(+), 8 deletions(-) diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc index 0097862aea8..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 -# -# Kpasswd tests -# -^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_tgt.ad_dc diff --git a/selftest/knownfail_mit_kdc b/selftest/knownfail_mit_kdc index b0a1a1c35e0..8601da3d79a 100644 --- a/selftest/knownfail_mit_kdc +++ b/selftest/knownfail_mit_kdc @@ -439,7 +439,3 @@ samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_ ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_canonicalize_realm_case.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_canonicalize_realm_case.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_requester_sid_tgs.ad_dc -# -# Kpasswd tests -# -samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_tgt.ad_dc diff --git a/source4/kdc/kpasswd-helper.c b/source4/kdc/kpasswd-helper.c index 83d2dabad1f..645f9c96989 100644 --- a/source4/kdc/kpasswd-helper.c +++ b/source4/kdc/kpasswd-helper.c @@ -239,3 +239,23 @@ NTSTATUS kpasswd_samdb_set_password(TALLOC_CTX *mem_ctx, return status; } + +krb5_error_code kpasswd_check_non_tgt(struct auth_session_info *session_info, + const char **error_string) +{ + switch(session_info->ticket_type) { + case TICKET_TYPE_TGT: + /* TGTs are disallowed here. */ + *error_string = "A TGT may not be used as a ticket to kpasswd"; + return KRB5_KPASSWD_AUTHERROR; + case TICKET_TYPE_NON_TGT: + /* Non-TGTs are permitted, and expected. */ + break; + default: + /* In case we forgot to set the type. */ + *error_string = "Failed to ascertain that ticket to kpasswd is not a TGT"; + return KRB5_KPASSWD_HARDERROR; + } + + return 0; +} diff --git a/source4/kdc/kpasswd-helper.h b/source4/kdc/kpasswd-helper.h index 8fad81e0a5d..94a6e2acfdd 100644 --- a/source4/kdc/kpasswd-helper.h +++ b/source4/kdc/kpasswd-helper.h @@ -43,4 +43,6 @@ NTSTATUS kpasswd_samdb_set_password(TALLOC_CTX *mem_ctx, enum samPwdChangeReason *reject_reason, struct samr_DomInfo1 **dominfo); +krb5_error_code kpasswd_check_non_tgt(struct auth_session_info *session_info, + const char **error_string); #endif /* _KPASSWD_HELPER_H */ diff --git a/source4/kdc/kpasswd-service-heimdal.c b/source4/kdc/kpasswd-service-heimdal.c index 00470af7cf2..c92b13def93 100644 --- a/source4/kdc/kpasswd-service-heimdal.c +++ b/source4/kdc/kpasswd-service-heimdal.c @@ -252,6 +252,7 @@ krb5_error_code kpasswd_handle_request(struct kdc_server *kdc, { struct auth_session_info *session_info; NTSTATUS status; + krb5_error_code code; status = gensec_session_info(gensec_security, mem_ctx, @@ -263,6 +264,18 @@ krb5_error_code kpasswd_handle_request(struct kdc_server *kdc, return KRB5_KPASSWD_HARDERROR; } + /* + * Since the kpasswd service shares its keys with the krbtgt, we might + * have received a TGT rather than a kpasswd ticket. We need to check + * the ticket type to ensure that TGTs cannot be misused in this manner. + */ + code = kpasswd_check_non_tgt(session_info, + error_string); + if (code != 0) { + DBG_WARNING("%s\n", *error_string); + return code; + } + switch(verno) { case KRB5_KPASSWD_VERS_CHANGEPW: { DATA_BLOB password = data_blob_null; diff --git a/source4/kdc/kpasswd-service-mit.c b/source4/kdc/kpasswd-service-mit.c index fe24693139c..053b1f2bb6f 100644 --- a/source4/kdc/kpasswd-service-mit.c +++ b/source4/kdc/kpasswd-service-mit.c @@ -331,6 +331,7 @@ krb5_error_code kpasswd_handle_request(struct kdc_server *kdc, { struct auth_session_info *session_info; NTSTATUS status; + krb5_error_code code; status = gensec_session_info(gensec_security, mem_ctx, @@ -343,6 +344,18 @@ krb5_error_code kpasswd_handle_request(struct kdc_server *kdc, return KRB5_KPASSWD_HARDERROR; } + /* + * Since the kpasswd service shares its keys with the krbtgt, we might + * have received a TGT rather than a kpasswd ticket. We need to check + * the ticket type to ensure that TGTs cannot be misused in this manner. + */ + code = kpasswd_check_non_tgt(session_info, + error_string); + if (code != 0) { + DBG_WARNING("%s\n", *error_string); + return code; + } + switch(verno) { case 1: { DATA_BLOB password; -- 2.35.0