From b49e18441c459449548efffef3591ae02ec9e084 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 8 Mar 2023 15:22:29 +1300 Subject: [PATCH] kdc-plugin: Split updating a PAC out of PAC verification Up to now krb5plugin_kdc_pac_verify() has served to both verified and updated a PAC. There are cases, however, when we only want to retrieve and verify a PAC, but don't want to modify it. This is the case with the PAC from a FAST armor ticket. Therefore, add a new plugin function, pac_update(), that will update a PAC obtained using pac_verify(). pac_verify() now only deals with verifying a PAC, while pac_update() handles any necessary updates to it. Signed-off-by: Joseph Sutton --- kdc/kdc-plugin.c | 60 ++++++++++++++++++++++++++++++++-- kdc/kdc-plugin.h | 25 +++++++++++--- kdc/krb5tgs.c | 25 +++++++++++++- kdc/mssfu.c | 22 ++++++++++++- tests/plugin/kdc_test_plugin.c | 12 ++++--- 5 files changed, 130 insertions(+), 14 deletions(-) diff --git a/kdc/kdc-plugin.c b/kdc/kdc-plugin.c index 925c250597..887f179e00 100644 --- a/kdc/kdc-plugin.c +++ b/kdc/kdc-plugin.c @@ -51,7 +51,7 @@ static const char *kdc_plugin_deps[] = { static struct heim_plugin_data kdc_plugin_data = { "krb5", "kdc", - KRB5_PLUGIN_KDC_VERSION_10, + KRB5_PLUGIN_KDC_VERSION_11, kdc_plugin_deps, kdc_get_instance }; @@ -145,7 +145,7 @@ struct verify_uc { hdb_entry *client; hdb_entry *server; hdb_entry *krbtgt; - krb5_pac *pac; + krb5_pac pac; }; static krb5_error_code KRB5_LIB_CALL @@ -173,7 +173,7 @@ _kdc_pac_verify(astgs_request_t r, hdb_entry *client, hdb_entry *server, hdb_entry *krbtgt, - krb5_pac *pac) + krb5_pac pac) { struct verify_uc uc; @@ -192,6 +192,60 @@ _kdc_pac_verify(astgs_request_t r, 0, &uc, verify); } +struct update_uc { + astgs_request_t r; + krb5_principal client_principal; + krb5_principal delegated_proxy_principal; + hdb_entry *client; + hdb_entry *server; + hdb_entry *krbtgt; + krb5_pac *pac; +}; + +static krb5_error_code KRB5_LIB_CALL +update(krb5_context context, const void *plug, void *plugctx, void *userctx) +{ + const krb5plugin_kdc_ftable *ft = (const krb5plugin_kdc_ftable *)plug; + struct update_uc *uc = (struct update_uc *)userctx; + krb5_error_code ret; + + if (ft->pac_update == NULL) + return KRB5_PLUGIN_NO_HANDLE; + + ret = ft->pac_update((void *)plug, + uc->r, + uc->client_principal, + uc->delegated_proxy_principal, + uc->client, uc->server, uc->krbtgt, uc->pac); + return ret; +} + +krb5_error_code +_kdc_pac_update(astgs_request_t r, + const krb5_principal client_principal, + const krb5_principal delegated_proxy_principal, + hdb_entry *client, + hdb_entry *server, + hdb_entry *krbtgt, + krb5_pac *pac) +{ + struct update_uc uc; + + if (!have_plugin) + return KRB5_PLUGIN_NO_HANDLE; + + uc.r = r; + uc.client_principal = client_principal; + uc.delegated_proxy_principal = delegated_proxy_principal; + uc.client = client; + uc.server = server; + uc.krbtgt = krbtgt; + uc.pac = pac; + + return _krb5_plugin_run_f(r->context, &kdc_plugin_data, + 0, &uc, update); +} + static krb5_error_code KRB5_LIB_CALL check(krb5_context context, const void *plug, void *plugctx, void *userctx) { diff --git a/kdc/kdc-plugin.h b/kdc/kdc-plugin.h index 05286257bf..f667184e60 100644 --- a/kdc/kdc-plugin.h +++ b/kdc/kdc-plugin.h @@ -57,8 +57,7 @@ typedef krb5_error_code /* * Verify the PAC KDC signatures by fetching the appropriate TGS key - * and calling krb5_pac_verify() with that key. Optionally update the - * PAC buffers on success. + * and calling krb5_pac_verify() with that key. */ typedef krb5_error_code @@ -69,7 +68,24 @@ typedef krb5_error_code hdb_entry *,/* client */ hdb_entry *,/* server */ hdb_entry *,/* krbtgt */ - krb5_pac *); + krb5_pac); /* pac */ + +/* + * Update the KDC PAC buffers. This function may be used after verifying the PAC + * with a call to krb5plugin_kdc_pac_verify(), and it resembles the latter + * function in the parameters it takes. The 'pac' parameter always points to a + * non-NULL PAC. + */ + +typedef krb5_error_code +(KRB5_CALLCONV *krb5plugin_kdc_pac_update)(void *, + astgs_request_t, + const krb5_principal, /* new ticket client */ + const krb5_principal, /* delegation proxy */ + hdb_entry *,/* client */ + hdb_entry *,/* server */ + hdb_entry *,/* krbtgt */ + krb5_pac *); /* pac */ /* * Authorize the client principal's access to the Authentication Service (AS). @@ -117,12 +133,13 @@ typedef krb5_error_code * Plugins should carefully check API contract notes for changes * between plugin API versions. */ -#define KRB5_PLUGIN_KDC_VERSION_10 10 +#define KRB5_PLUGIN_KDC_VERSION_11 11 typedef struct krb5plugin_kdc_ftable { HEIM_PLUGIN_FTABLE_COMMON_ELEMENTS(krb5_context); krb5plugin_kdc_pac_generate pac_generate; krb5plugin_kdc_pac_verify pac_verify; + krb5plugin_kdc_pac_update pac_update; krb5plugin_kdc_client_access client_access; krb5plugin_kdc_referral_policy referral_policy; krb5plugin_kdc_finalize_reply finalize_reply; diff --git a/kdc/krb5tgs.c b/kdc/krb5tgs.c index af3a394262..f87c9c5791 100644 --- a/kdc/krb5tgs.c +++ b/kdc/krb5tgs.c @@ -125,7 +125,7 @@ _kdc_check_pac(astgs_request_t r, /* Verify the KDC signatures. */ ret = _kdc_pac_verify(r, client_principal, delegated_proxy_principal, - client, server, krbtgt, &pac); + client, server, krbtgt, pac); if (ret == 0) { if (pac_canon_name) { ret = _krb5_pac_get_canon_principal(context, pac, pac_canon_name); @@ -1887,6 +1887,29 @@ server_lookup: goto out; } + if (priv->pac != NULL) { + ret = _kdc_pac_update(priv, priv->client_princ, NULL, + priv->client, priv->server, priv->krbtgt, + &priv->pac); + if (ret == KRB5_PLUGIN_NO_HANDLE) { + ret = 0; + } + if (ret) { + const char *msg = krb5_get_error_message(context, ret); + kdc_audit_addreason((kdc_request_t)priv, "PAC update failed"); + kdc_log(context, config, 4, + "Update PAC failed for %s (%s) from %s with %s", + spn, cpn, from, msg); + krb5_free_error_message(context, msg); + goto out; + } + + if (priv->pac == NULL) { + /* the plugin may indicate no PAC should be generated */ + priv->pac_attributes = 0; + } + } + /* * Process request */ diff --git a/kdc/mssfu.c b/kdc/mssfu.c index ebdeec96b5..8232523c0c 100644 --- a/kdc/mssfu.c +++ b/kdc/mssfu.c @@ -482,7 +482,7 @@ validate_constrained_delegation(astgs_request_t r) kdc_audit_addreason((kdc_request_t)r, "Constrained delegation ticket PAC check failed"); kdc_log(r->context, r->config, 4, - "Verify delegated PAC failed to %s for client" + "Verify delegated PAC failed to %s for client " "%s (%s) as %s from %s with %s", r->sname, r->cname, s4usname, s4ucname, r->from, msg); krb5_free_error_message(r->context, msg); @@ -501,6 +501,26 @@ validate_constrained_delegation(astgs_request_t r) goto out; } + heim_assert(s4u_pac != NULL, "ad_kdc_issued implies the PAC is non-NULL"); + + ret = _kdc_pac_update(r, s4u_client_name, s4u_server_name, + s4u_client, r->server, r->krbtgt, + &s4u_pac); + if (ret == KRB5_PLUGIN_NO_HANDLE) { + ret = 0; + } + if (ret) { + const char *msg = krb5_get_error_message(r->context, ret); + kdc_audit_addreason((kdc_request_t)r, + "Constrained delegation ticket PAC update failed"); + kdc_log(r->context, r->config, 4, + "Update delegated PAC failed to %s for client " + "%s (%s) as %s from %s with %s", + r->sname, r->cname, s4usname, s4ucname, r->from, msg); + krb5_free_error_message(r->context, msg); + goto out; + } + /* * If the evidence ticket PAC didn't include PAC_UPN_DNS_INFO with * the canonical client name, but the user is local to our KDC, we diff --git a/tests/plugin/kdc_test_plugin.c b/tests/plugin/kdc_test_plugin.c index ff33b5f726..6df40a2b72 100644 --- a/tests/plugin/kdc_test_plugin.c +++ b/tests/plugin/kdc_test_plugin.c @@ -61,7 +61,8 @@ pac_verify(void *ctx, hdb_entry * client, hdb_entry * server, hdb_entry * krbtgt, - krb5_pac *pac) + krb5_pac pac, + krb5_boolean *is_trusted) { krb5_context context = kdc_request_get_context((kdc_request_t)r); krb5_error_code ret; @@ -73,12 +74,12 @@ pac_verify(void *ctx, krb5_warnx(context, "pac_verify"); - ret = krb5_pac_get_buffer(context, *pac, 1, &data); + ret = krb5_pac_get_buffer(context, pac, 1, &data); if (ret) return ret; krb5_data_free(&data); - ret = krb5_pac_get_kdc_checksum_info(context, *pac, &cstype, &rodc_id); + ret = krb5_pac_get_kdc_checksum_info(context, pac, &cstype, &rodc_id); if (ret) return ret; @@ -95,7 +96,7 @@ pac_verify(void *ctx, if (ret) return ret; - return krb5_pac_verify(context, *pac, 0, NULL, NULL, &key->key); + return krb5_pac_verify(context, pac, 0, NULL, NULL, &key->key); } static void logit(const char *what, astgs_request_t r) @@ -161,11 +162,12 @@ audit(void *ctx, astgs_request_t r) } static krb5plugin_kdc_ftable kdc_plugin = { - KRB5_PLUGIN_KDC_VERSION_10, + KRB5_PLUGIN_KDC_VERSION_11, init, fini, pac_generate, pac_verify, + NULL, /* pac_update */ client_access, NULL, /* referral_policy */ finalize_reply, -- 2.34.1