The branch, master has been updated via ad98643fbd9 s4:kdc: Replace FAST cookie with dummy string via fc4740426d2 third_party/heimdal: Import lorikeet-heimdal-202306112240 (commit c7f4ffe1a6e8dafc86ec3357c498d31c97ece386) via 53caae00b82 tests/krb5: Test that FX-COOKIE matches cookie returned by Windows from c4e27ae4f69 smbd: Don't set security_descriptor_hash_v4->time
https://git.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit ad98643fbd914b7fb28d43a36bd51eeb1f8e2e06 Author: Joseph Sutton <josephsut...@catalyst.net.nz> Date: Fri Jun 9 15:46:33 2023 +1200 s4:kdc: Replace FAST cookie with dummy string All that uses the FAST cookie is the gss-preauth authentication mechanism, which is untested in Samba, and disabled by default. Disabling the FAST cookie code (and sending a dummy string instead) relieves us of the maintenance and testing burden of this untested code. Signed-off-by: Joseph Sutton <josephsut...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> Reviewed-by: Stefan Metzmacher <me...@samba.org> Autobuild-User(master): Stefan Metzmacher <me...@samba.org> Autobuild-Date(master): Wed Jun 21 13:19:17 UTC 2023 on atb-devel-224 commit fc4740426d2f43ca7703e3e4e6ef71c902ce5cd3 Author: Joseph Sutton <josephsut...@catalyst.net.nz> Date: Mon Jun 12 12:12:06 2023 +1200 third_party/heimdal: Import lorikeet-heimdal-202306112240 (commit c7f4ffe1a6e8dafc86ec3357c498d31c97ece386) Signed-off-by: Joseph Sutton <josephsut...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> Reviewed-by: Stefan Metzmacher <me...@samba.org> commit 53caae00b824e1fe67a67978a5ad604964f10c7a Author: Joseph Sutton <josephsut...@catalyst.net.nz> Date: Mon Jun 12 13:06:21 2023 +1200 tests/krb5: Test that FX-COOKIE matches cookie returned by Windows The cookie produced by Windows differs depending on whether FAST was used. Signed-off-by: Joseph Sutton <josephsut...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> Reviewed-by: Stefan Metzmacher <me...@samba.org> ----------------------------------------------------------------------- Summary of changes: python/samba/tests/krb5/fast_tests.py | 87 +++++++++++++++++++++++ selftest/knownfail_heimdal_kdc | 1 + selftest/knownfail_mit_kdc | 3 + source4/kdc/db-glue.c | 19 ----- source4/kdc/hdb-samba4.c | 117 +------------------------------ source4/kdc/kdc-heimdal.c | 29 ++++++++ source4/kdc/samba_kdc.h | 2 - third_party/heimdal/kdc/default_config.c | 9 +++ third_party/heimdal/kdc/fast.c | 72 ++++++++++++++----- third_party/heimdal/kdc/kdc.h | 7 ++ third_party/heimdal/kdc/kerberos5.c | 7 +- third_party/heimdal/lib/krb5/krb5.conf.5 | 3 + 12 files changed, 203 insertions(+), 153 deletions(-) Changeset truncated at 500 lines: diff --git a/python/samba/tests/krb5/fast_tests.py b/python/samba/tests/krb5/fast_tests.py index e57ea5e1c4b..1c4b5256cef 100755 --- a/python/samba/tests/krb5/fast_tests.py +++ b/python/samba/tests/krb5/fast_tests.py @@ -1418,6 +1418,86 @@ class FAST_Tests(KDCBaseTest): } ]) + def test_fx_cookie_fast(self): + """Test that the FAST cookie is present and that its value is as + expected when FAST is used.""" + kdc_exchange_dict = self._run_test_sequence([ + { + 'rep_type': KRB_AS_REP, + 'expected_error_mode': KDC_ERR_PREAUTH_REQUIRED, + 'use_fast': True, + 'fast_armor': FX_FAST_ARMOR_AP_REQUEST, + 'gen_armor_tgt_fn': self.get_mach_tgt + }, + ]) + + cookie = kdc_exchange_dict.get('fast_cookie') + self.assertEqual(b'Microsoft', cookie) + + def test_fx_cookie_no_fast(self): + """Test that the FAST cookie is present and that its value is as + expected when FAST is not used.""" + kdc_exchange_dict = self._run_test_sequence([ + { + 'rep_type': KRB_AS_REP, + 'expected_error_mode': KDC_ERR_PREAUTH_REQUIRED, + 'use_fast': False + }, + ]) + + cookie = kdc_exchange_dict.get('fast_cookie') + self.assertEqual(b'Microsof\x00', cookie) + + def test_unsolicited_fx_cookie_preauth(self): + """Test sending an unsolicited FX-COOKIE in an AS-REQ without + pre-authentication data.""" + + # Include a FAST cookie. + fast_cookie = self.create_fast_cookie('Samba-Test') + + kdc_exchange_dict = self._run_test_sequence([ + { + 'rep_type': KRB_AS_REP, + 'expected_error_mode': KDC_ERR_PREAUTH_REQUIRED, + 'use_fast': True, + 'fast_armor': FX_FAST_ARMOR_AP_REQUEST, + 'gen_armor_tgt_fn': self.get_mach_tgt, + 'fast_cookie': fast_cookie, + }, + ]) + + got_cookie = kdc_exchange_dict.get('fast_cookie') + self.assertEqual(b'Microsoft', got_cookie) + + def test_unsolicited_fx_cookie_fast(self): + """Test sending an unsolicited FX-COOKIE in an AS-REQ with + pre-authentication data.""" + + # Include a FAST cookie. + fast_cookie = self.create_fast_cookie('Samba-Test') + + kdc_exchange_dict = self._run_test_sequence([ + { + 'rep_type': KRB_AS_REP, + 'expected_error_mode': KDC_ERR_PREAUTH_REQUIRED, + 'use_fast': True, + 'fast_armor': FX_FAST_ARMOR_AP_REQUEST, + 'gen_armor_tgt_fn': self.get_mach_tgt, + }, + { + 'rep_type': KRB_AS_REP, + 'expected_error_mode': 0, + 'use_fast': True, + 'gen_padata_fn': self.generate_enc_challenge_padata, + 'fast_armor': FX_FAST_ARMOR_AP_REQUEST, + 'gen_armor_tgt_fn': self.get_mach_tgt, + 'fast_cookie': fast_cookie, + } + ]) + + got_cookie = kdc_exchange_dict.get('fast_cookie') + self.assertIsNone(got_cookie) + def generate_enc_timestamp_padata(self, kdc_exchange_dict, callback_dict, @@ -1697,6 +1777,11 @@ class FAST_Tests(KDCBaseTest): preauth_key = None if use_fast: + try: + fast_cookie = kdc_dict.pop('fast_cookie') + except KeyError: + pass + generate_fast_padata_fn = gen_padata_fn generate_padata_fn = (functools.partial(_generate_padata_copy, padata=[fast_cookie]) @@ -1869,6 +1954,8 @@ class FAST_Tests(KDCBaseTest): # Ensure we used all the parameters given to us. self.assertEqual({}, kdc_dict) + return kdc_exchange_dict + def generate_enc_pa_rep_padata(self, kdc_exchange_dict, callback_dict, diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc index 4dcd20107ba..ea2537530f8 100644 --- a/selftest/knownfail_heimdal_kdc +++ b/selftest/knownfail_heimdal_kdc @@ -17,6 +17,7 @@ ^samba.tests.krb5.fast_tests.samba.tests.krb5.fast_tests.FAST_Tests.test_fast_hide_client_names.ad_dc ^samba.tests.krb5.fast_tests.samba.tests.krb5.fast_tests.FAST_Tests.test_fast_tgs_armor_enc_pa_rep.ad_dc ^samba.tests.krb5.fast_tests.samba.tests.krb5.fast_tests.FAST_Tests.test_fast_tgs_enc_pa_rep.ad_dc +^samba.tests.krb5.fast_tests.samba.tests.krb5.fast_tests.FAST_Tests.test_fx_cookie_no_fast.ad_dc ^samba.tests.krb5.fast_tests.samba.tests.krb5.fast_tests.FAST_Tests.test_simple_tgs_enc_pa_rep.ad_dc # # S4U tests diff --git a/selftest/knownfail_mit_kdc b/selftest/knownfail_mit_kdc index 6b6482f3295..9c5b76cac5a 100644 --- a/selftest/knownfail_mit_kdc +++ b/selftest/knownfail_mit_kdc @@ -262,10 +262,13 @@ samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_ ^samba.tests.krb5.fast_tests.samba.tests.krb5.fast_tests.FAST_Tests.test_fast_tgs_enc_pa_rep.ad_dc ^samba.tests.krb5.fast_tests.samba.tests.krb5.fast_tests.FAST_Tests.test_fast_tgs_no_sname.ad_dc ^samba.tests.krb5.fast_tests.samba.tests.krb5.fast_tests.FAST_Tests.test_fast_tgs_rodc_issued_armor.ad_dc +^samba.tests.krb5.fast_tests.samba.tests.krb5.fast_tests.FAST_Tests.test_fx_cookie_fast.ad_dc +^samba.tests.krb5.fast_tests.samba.tests.krb5.fast_tests.FAST_Tests.test_fx_cookie_no_fast.ad_dc ^samba.tests.krb5.fast_tests.samba.tests.krb5.fast_tests.FAST_Tests.test_simple_as_req_self_no_auth_data.ad_dc ^samba.tests.krb5.fast_tests.samba.tests.krb5.fast_tests.FAST_Tests.test_simple_no_sname.ad_dc ^samba.tests.krb5.fast_tests.samba.tests.krb5.fast_tests.FAST_Tests.test_simple_tgs_enc_pa_rep.ad_dc ^samba.tests.krb5.fast_tests.samba.tests.krb5.fast_tests.FAST_Tests.test_simple_tgs_no_sname.ad_dc +^samba.tests.krb5.fast_tests.samba.tests.krb5.fast_tests.FAST_Tests.test_unsolicited_fx_cookie_preauth.ad_dc ^samba.tests.krb5.fast_tests.samba.tests.krb5.fast_tests.FAST_Tests.test_fast_inner_no_sname.ad_dc ^samba.tests.krb5.fast_tests.samba.tests.krb5.fast_tests.FAST_Tests.test_fast_tgs_inner_no_sname.ad_dc # diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c index 112c9556e93..ae90896d572 100644 --- a/source4/kdc/db-glue.c +++ b/source4/kdc/db-glue.c @@ -3536,25 +3536,6 @@ NTSTATUS samba_kdc_setup_db_ctx(TALLOC_CTX *mem_ctx, struct samba_kdc_base_conte return NT_STATUS_INTERNAL_ERROR; } - /* Setup the link to secrets.ldb */ - - kdc_db_ctx->secrets_db = secrets_db_connect(kdc_db_ctx, - base_ctx->lp_ctx); - if (kdc_db_ctx->secrets_db == NULL) { - DEBUG(1, ("samba_kdc_setup_db_ctx: " - "Cannot open secrets.ldb for KDC backend!")); - talloc_free(kdc_db_ctx); - return NT_STATUS_CANT_ACCESS_DOMAIN_INFO; - } - - kdc_db_ctx->fx_cookie_dn = ldb_dn_new(kdc_db_ctx, - kdc_db_ctx->secrets_db, - "CN=FX Cookie"); - if (kdc_db_ctx->fx_cookie_dn == NULL) { - talloc_free(kdc_db_ctx); - return NT_STATUS_NO_MEMORY; - } - /* Setup the link to LDB */ kdc_db_ctx->samdb = samdb_connect(kdc_db_ctx, base_ctx->ev_ctx, diff --git a/source4/kdc/hdb-samba4.c b/source4/kdc/hdb-samba4.c index 96c1ee5e072..90e52f60cef 100644 --- a/source4/kdc/hdb-samba4.c +++ b/source4/kdc/hdb-samba4.c @@ -119,125 +119,12 @@ static void hdb_samba4_free_entry_context(krb5_context context, struct HDB *db, } } -static int hdb_samba4_fill_fast_cookie(krb5_context context, - struct samba_kdc_db_context *kdc_db_ctx) -{ - struct ldb_message *msg = ldb_msg_new(kdc_db_ctx); - int ldb_ret; - - uint8_t secretbuffer[32]; - struct ldb_val val = data_blob_const(secretbuffer, - sizeof(secretbuffer)); - - if (msg == NULL) { - DBG_ERR("Failed to allocate msg for new fast cookie\n"); - return LDB_ERR_OPERATIONS_ERROR; - } - - /* Fill in all the keys with the same secret */ - generate_secret_buffer(secretbuffer, - sizeof(secretbuffer)); - - msg->dn = kdc_db_ctx->fx_cookie_dn; - - ldb_ret = ldb_msg_add_value(msg, "secret", &val, NULL); - - if (ldb_ret != LDB_SUCCESS) { - return ldb_ret; - } - - ldb_ret = ldb_add(kdc_db_ctx->secrets_db, - msg); - if (ldb_ret != LDB_SUCCESS) { - DBG_ERR("Failed to add fast cookie to ldb: %s\n", - ldb_errstring(kdc_db_ctx->secrets_db)); - } - return ldb_ret; -} - static krb5_error_code hdb_samba4_fetch_fast_cookie(krb5_context context, struct samba_kdc_db_context *kdc_db_ctx, hdb_entry *entry) { - krb5_error_code ret = SDB_ERR_NOENTRY; - TALLOC_CTX *mem_ctx; - struct ldb_result *res; - int ldb_ret; - struct sdb_entry sentry = {}; - const char *attrs[] = { - "secret", - NULL - }; - const struct ldb_val *val; - - mem_ctx = talloc_named(kdc_db_ctx, 0, "hdb_samba4_fetch_fast_cookie context"); - if (!mem_ctx) { - ret = ENOMEM; - krb5_set_error_message(context, ret, "hdb_samba4_fetch_fast_cookie: talloc_named() failed!"); - return ret; - } - - /* search for CN=FX-COOKIE */ - ldb_ret = ldb_search(kdc_db_ctx->secrets_db, - mem_ctx, - &res, - kdc_db_ctx->fx_cookie_dn, - LDB_SCOPE_BASE, - attrs, NULL); - - if (ldb_ret == LDB_ERR_NO_SUCH_OBJECT || res->count == 0) { - - ldb_ret = hdb_samba4_fill_fast_cookie(context, - kdc_db_ctx); - - if (ldb_ret != LDB_SUCCESS) { - TALLOC_FREE(mem_ctx); - return HDB_ERR_NO_WRITE_SUPPORT; - } - - /* search for CN=FX-COOKIE */ - ldb_ret = ldb_search(kdc_db_ctx->secrets_db, - mem_ctx, - &res, - kdc_db_ctx->fx_cookie_dn, - LDB_SCOPE_BASE, - attrs, NULL); - - if (ldb_ret != LDB_SUCCESS || res->count != 1) { - TALLOC_FREE(mem_ctx); - return HDB_ERR_NOENTRY; - } - } - - val = ldb_msg_find_ldb_val(res->msgs[0], - "secret"); - if (val == NULL || val->length != 32) { - TALLOC_FREE(mem_ctx); - return HDB_ERR_NOENTRY; - } - - - ret = krb5_make_principal(context, - &sentry.principal, - KRB5_WELLKNOWN_ORG_H5L_REALM, - KRB5_WELLKNOWN_NAME, "org.h5l.fast-cookie", - NULL); - if (ret) { - TALLOC_FREE(mem_ctx); - return ret; - } - - ret = samba_kdc_set_fixed_keys(context, val, ENC_ALL_TYPES, - &sentry.keys); - if (ret != 0) { - return ret; - } - - ret = sdb_entry_to_hdb_entry(context, &sentry, entry); - sdb_entry_free(&sentry); - TALLOC_FREE(mem_ctx); - - return ret; + DBG_ERR("Looked up HDB entry for unsupported FX-COOKIE.\n"); + return HDB_ERR_NOENTRY; } static krb5_error_code hdb_samba4_fetch_kvno(krb5_context context, HDB *db, diff --git a/source4/kdc/kdc-heimdal.c b/source4/kdc/kdc-heimdal.c index 766f7ba917d..da1c2118bd2 100644 --- a/source4/kdc/kdc-heimdal.c +++ b/source4/kdc/kdc-heimdal.c @@ -425,6 +425,35 @@ static void kdc_post_fork(struct task_server *task, struct process_details *pd) */ kdc_config->enable_fast = lpcfg_kdc_enable_fast(task->lp_ctx); + { + static const char *dummy_string = "Microsoft"; + + /* + * The FAST cookie is not cryptographically required, + * provided that the non-AD gss-preauth authentication + * method is removed (as this is the only multi-step + * authentication method). + * + * gss-preauth has been disabled both by not being + * configured and by being made dependent + * configuration for a "real" fast cookie. + * + * The hide_client_names feature in Heimdal is the + * only other state that is persisted in the cookie, + * and this does not need to be in the cookie for + * single-shot authentication protocols such as ENC-TS + * and ENC-CHAL, the standard password protocols in + * AD. + * + * Furthermore, the Heimdal KDC does not fail if the + * client does not supply a FAST cookie, showing that + * the presence of the cookie is not required. + */ + kdc_config->enable_fast_cookie = false; + kdc_config->dummy_fast_cookie = smb_krb5_make_data(discard_const_p(char, dummy_string), + strlen(dummy_string)); + } + /* * Match Windows and RFC6113 and Windows but break older * Heimdal clients. diff --git a/source4/kdc/samba_kdc.h b/source4/kdc/samba_kdc.h index 8410a5bebac..8b3f072b362 100644 --- a/source4/kdc/samba_kdc.h +++ b/source4/kdc/samba_kdc.h @@ -52,8 +52,6 @@ struct samba_kdc_db_context { unsigned int my_krbtgt_number; struct ldb_dn *krbtgt_dn; struct samba_kdc_policy policy; - struct ldb_dn *fx_cookie_dn; - struct ldb_context *secrets_db; }; struct samba_kdc_entry { diff --git a/third_party/heimdal/kdc/default_config.c b/third_party/heimdal/kdc/default_config.c index 83c73504ce7..ce29dcc4b5a 100644 --- a/third_party/heimdal/kdc/default_config.c +++ b/third_party/heimdal/kdc/default_config.c @@ -102,6 +102,7 @@ krb5_kdc_get_config(krb5_context context, krb5_kdc_configuration **config) c->trpolicy = TRPOLICY_ALWAYS_CHECK; c->require_pac = FALSE; c->enable_fast = TRUE; + c->enable_fast_cookie = TRUE; c->enable_armored_pa_enc_timestamp = TRUE; c->enable_unarmored_pa_enc_timestamp = TRUE; c->enable_pkinit = FALSE; @@ -271,6 +272,14 @@ krb5_kdc_get_config(krb5_context context, krb5_kdc_configuration **config) "enable_fast", NULL); + c->enable_fast_cookie = + krb5_config_get_bool_default(context, + NULL, + c->enable_fast_cookie, + "kdc", + "enable_fast_cookie", + NULL); + c->enable_armored_pa_enc_timestamp = krb5_config_get_bool_default(context, NULL, diff --git a/third_party/heimdal/kdc/fast.c b/third_party/heimdal/kdc/fast.c index 969b5d2f8da..1352a10fe01 100644 --- a/third_party/heimdal/kdc/fast.c +++ b/third_party/heimdal/kdc/fast.c @@ -266,6 +266,33 @@ fast_add_cookie(astgs_request_t r, return ret; } +static krb5_error_code +fast_add_dummy_cookie(astgs_request_t r, + METHOD_DATA *method_data) +{ + krb5_error_code ret; + krb5_data data; + const krb5_data *dummy_fast_cookie = &r->config->dummy_fast_cookie; + + if (dummy_fast_cookie->data == NULL) + return 0; + + ret = krb5_data_copy(&data, + dummy_fast_cookie->data, + dummy_fast_cookie->length); + if (ret) + return ret; + + ret = krb5_padata_add(r->context, method_data, + KRB5_PADATA_FX_COOKIE, + data.data, data.length); + if (ret) { + krb5_data_free(&data); + } + + return ret; +} + krb5_error_code _kdc_fast_mk_response(krb5_context context, krb5_crypto armor_crypto, @@ -341,13 +368,24 @@ _kdc_fast_mk_e_data(astgs_request_t r, * FX-COOKIE can be used outside of FAST, e.g. SRP or GSS. */ if (armor_crypto || r->fast.fast_state.len) { - kdc_log(r->context, r->config, 5, "Adding FAST cookie for KRB-ERROR"); - ret = fast_add_cookie(r, error_client, error_method); - if (ret) { - kdc_log(r->context, r->config, 1, - "Failed to add FAST cookie: %d", ret); - free_METHOD_DATA(error_method); - return ret; + if (r->config->enable_fast_cookie) { + kdc_log(r->context, r->config, 5, "Adding FAST cookie for KRB-ERROR"); + ret = fast_add_cookie(r, error_client, error_method); + if (ret) { + kdc_log(r->context, r->config, 1, + "Failed to add FAST cookie: %d", ret); + free_METHOD_DATA(error_method); + return ret; + } + } else { + kdc_log(r->context, r->config, 5, "Adding dummy FAST cookie for KRB-ERROR"); + ret = fast_add_dummy_cookie(r, error_method); + if (ret) { + kdc_log(r->context, r->config, 1, + "Failed to add dummy FAST cookie: %d", ret); + free_METHOD_DATA(error_method); + return ret; + } } } @@ -803,17 +841,19 @@ _kdc_fast_unwrap_request(astgs_request_t r, if (ret) return ret; - /* - * FX-COOKIE can be used outside of FAST, e.g. SRP or GSS. - */ - pa = _kdc_find_padata(&r->req, &i, KRB5_PADATA_FX_COOKIE); - if (pa) { - krb5_const_principal ticket_client = NULL; + if (r->config->enable_fast_cookie) { + /* + * FX-COOKIE can be used outside of FAST, e.g. SRP or GSS. + */ + pa = _kdc_find_padata(&r->req, &i, KRB5_PADATA_FX_COOKIE); + if (pa) { + krb5_const_principal ticket_client = NULL; - if (tgs_ticket) - ticket_client = tgs_ticket->client; + if (tgs_ticket) + ticket_client = tgs_ticket->client; - ret = fast_parse_cookie(r, ticket_client, pa); + ret = fast_parse_cookie(r, ticket_client, pa); + } } return ret; diff --git a/third_party/heimdal/kdc/kdc.h b/third_party/heimdal/kdc/kdc.h index 31e54325452..057d29a02a1 100644 --- a/third_party/heimdal/kdc/kdc.h +++ b/third_party/heimdal/kdc/kdc.h @@ -92,6 +92,12 @@ struct krb5_kdc_service { size_t num_db; \ const char *app; \ \ + /* + * If non-null, contains static dummy data to include in + * place of the FAST cookie when it is disabled. + */ \ + krb5_data dummy_fast_cookie; \ + \ /* \ * Windows 2019 (and earlier versions) always sends the salt\ * and Samba has testsuites that check this behaviour, so a \ -- Samba Shared Repository