From 7672ad31db115ca998d1e410533964a07cf6678f Mon Sep 17 00:00:00 2001 From: Nicolas Williams Date: Sun, 10 Oct 2021 21:36:28 -0500 Subject: [PATCH] kdc: Fix leak and loss of kdc_check_flags() reason We were losing and leaking the reason for which kdc_check_flags() was rejecting any S4U requests, yielding incomplete error messages. The issue is that kdc_check_flags() wants to check the client and server principals in the input state structure, but doesn't know about impersonated principal name, and so we want to pass it a state structure that has the impersonated instead of the impersonator client name. This is a bad design, but I'm ignoring that for now and just fixing this one leak. --- kdc/kerberos5.c | 9 +++++---- kdc/krb5tgs.c | 17 +++-------------- kdc/windc.c | 18 +++++++----------- 3 files changed, 15 insertions(+), 29 deletions(-) diff --git a/kdc/kerberos5.c b/kdc/kerberos5.c index 473600609..755fc91c5 100644 --- a/kdc/kerberos5.c +++ b/kdc/kerberos5.c @@ -1597,13 +1597,14 @@ _log_astgs_req(astgs_request_t r, krb5_enctype setype) */ krb5_error_code -kdc_check_flags(astgs_request_t r, krb5_boolean is_as_req) +kdc_check_flags(astgs_request_t r, + krb5_boolean is_as_req, + hdb_entry_ex *client_ex, + hdb_entry_ex *server_ex) { krb5_context context = r->context; - hdb_entry_ex *client_ex = r->client; - hdb_entry_ex *server_ex = r->server; - if(client_ex != NULL) { + if (client_ex != NULL) { hdb_entry *client = &client_ex->entry; /* check client */ diff --git a/kdc/krb5tgs.c b/kdc/krb5tgs.c index b4ae35f79..a9e246c2a 100644 --- a/kdc/krb5tgs.c +++ b/kdc/krb5tgs.c @@ -1805,7 +1805,6 @@ server_lookup: sdata = _kdc_find_padata(req, &i, KRB5_PADATA_FOR_USER); if (sdata) { - struct astgs_request_desc imp_req; krb5_crypto crypto; krb5_data datack; PA_S4U2Self self; @@ -1930,11 +1929,7 @@ server_lookup: free(s4u2self_impersonated_client->entry.pw_end); s4u2self_impersonated_client->entry.pw_end = NULL; - imp_req = *priv; - imp_req.client = s4u2self_impersonated_client; - imp_req.client_princ = tp; - - ret = kdc_check_flags(&imp_req, FALSE); + ret = kdc_check_flags(priv, FALSE, s4u2self_impersonated_client, priv->server); if (ret) goto out; /* kdc_check_flags() calls _kdc_audit_addreason() */ @@ -2093,13 +2088,7 @@ server_lookup: goto out; if (adclient != NULL) { - struct astgs_request_desc deleg_req; - - deleg_req = *priv; - deleg_req.client = adclient; - deleg_req.client_princ = tp; - - ret = kdc_check_flags(&deleg_req, FALSE); + ret = kdc_check_flags(priv, FALSE, adclient, priv->server); if (ret) { _kdc_free_ent(context, adclient); goto out; @@ -2145,7 +2134,7 @@ server_lookup: * Check flags */ - ret = kdc_check_flags(priv, FALSE); + ret = kdc_check_flags(priv, FALSE, priv->client, priv->server); if(ret) goto out; diff --git a/kdc/windc.c b/kdc/windc.c index 18dfb9556..fa5f7ccd6 100644 --- a/kdc/windc.c +++ b/kdc/windc.c @@ -199,20 +199,15 @@ krb5_error_code _kdc_check_access(astgs_request_t r, KDC_REQ *req, METHOD_DATA *method_data) { krb5_context context = r->context; - krb5_kdc_configuration *config = r->config; - hdb_entry_ex *client_ex = r->client; - const char *client_name = r->cname; - hdb_entry_ex *server_ex = r->server; - const char *server_name = r->sname; krb5_error_code ret = KRB5_PLUGIN_NO_HANDLE; struct check_uc uc; if (have_plugin) { - uc.config = config; - uc.client_ex = client_ex; - uc.client_name = client_name; - uc.server_ex = server_ex; - uc.server_name = server_name; + uc.config = r->config; + uc.client_ex = r->client; + uc.client_name = r->cname; + uc.server_ex = r->server; + uc.server_name = r->sname; uc.req = req; uc.method_data = method_data; @@ -221,7 +216,8 @@ _kdc_check_access(astgs_request_t r, KDC_REQ *req, METHOD_DATA *method_data) } if (ret == KRB5_PLUGIN_NO_HANDLE) - return kdc_check_flags(r, req->msg_type == krb_as_req); + return kdc_check_flags(r, req->msg_type == krb_as_req, + r->client, r->server); return ret; }