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.
This commit is contained in:
Nicolas Williams
2021-10-10 21:36:28 -05:00
parent 7e17db9f04
commit 7672ad31db
3 changed files with 15 additions and 29 deletions

View File

@@ -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 */

View File

@@ -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;

View File

@@ -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;
}