kdc: Add ret to common svc req elements

We're logging SUCCESS even when the KDC sends error replies.  That's
because we're returning success to process_request() even when we send
errors to clients.  The error we want to send to the client, and that we
succeed or fail to send it, are different statuses.

Also, further move things into `r` and out of function arguments.
This commit is contained in:
Nicolas Williams
2021-12-23 23:06:59 -06:00
committed by Luke Howard
parent 5e3c0a7ce0
commit fb3ea5b943
6 changed files with 34 additions and 22 deletions

View File

@@ -126,7 +126,6 @@ typedef struct bx509_request_desc {
char *ccname;
char *freeme1;
krb5_addresses tgt_addresses; /* For /get-tgt */
krb5_error_code ret;
char frombuf[128];
} *bx509_request_desc;

View File

@@ -328,7 +328,6 @@ _kdc_fast_mk_error(astgs_request_t r,
krb5_crypto armor_crypto,
const KDC_REQ_BODY *req_body,
krb5_error_code outer_error,
const char *e_text,
krb5_principal error_client,
krb5_principal error_server,
time_t *csec, int *cusec,
@@ -369,7 +368,7 @@ _kdc_fast_mk_error(astgs_request_t r,
ret = krb5_mk_error(r->context,
outer_error,
e_text,
r->e_text,
NULL,
error_client,
error_server,
@@ -393,7 +392,7 @@ _kdc_fast_mk_error(astgs_request_t r,
}
outer_error = KRB5_KDC_ERR_MORE_PREAUTH_DATA_REQUIRED;
e_text = NULL;
r->e_text = NULL;
if (r->fast.flags.requested_hidden_names) {
error_client = NULL;
error_server = NULL;
@@ -435,7 +434,7 @@ _kdc_fast_mk_error(astgs_request_t r,
ret = krb5_mk_error(r->context,
outer_error,
e_text,
r->e_text,
(e_data.length ? &e_data : NULL),
error_client,
error_server,

View File

@@ -119,7 +119,6 @@ typedef struct kadmin_request_desc {
HEIM_SVC_REQUEST_DESC_COMMON_ELEMENTS;
struct MHD_Connection *connection;
krb5_error_code ret;
krb5_times token_times;
/*
* FIXME

View File

@@ -418,6 +418,21 @@ _kdc_r_log(astgs_request_t r, int level, const char *fmt, ...)
va_end(ap);
}
void
_kdc_set_const_e_text(astgs_request_t r, const char *e_text)
{
/* We should never see this */
if (r->e_text) {
kdc_log(r->context, r->config, 1,
"trying to replace e-text \"%s\" with \"%s\"\n",
r->e_text, e_text);
return;
}
r->e_text = e_text;
kdc_log(r->context, r->config, 4, "%s", e_text);
}
void
_kdc_set_e_text(astgs_request_t r, const char *fmt, ...)
__attribute__ ((__format__ (__printf__, 2, 3)))
@@ -430,9 +445,12 @@ _kdc_set_e_text(astgs_request_t r, const char *fmt, ...)
vasprintf_ret = vasprintf(&e_text, fmt, ap);
va_end(ap);
if (vasprintf_ret < 0 || !e_text)
if (vasprintf_ret < 0 || !e_text) {
/* not much else to do... */
kdc_log(r->context, r->config, 1,
"Could not set e_text: %s (out of memory)", fmt);
return;
}
/* We should never see this */
if (r->e_text) {
@@ -2190,9 +2208,10 @@ _kdc_as_rep(astgs_request_t r)
r->cname, fixed_client_name);
free(fixed_client_name);
r->e_text = NULL;
ret = _kdc_fast_mk_error(r, r->rep.padata, r->armor_crypto,
&req->req_body, KRB5_KDC_ERR_WRONG_REALM,
NULL,
&req->req_body,
r->ret = KRB5_KDC_ERR_WRONG_REALM,
r->client->entry.principal, r->server_princ,
NULL, NULL, r->reply);
goto out;
@@ -2776,7 +2795,7 @@ out:
r->rep.padata,
r->armor_crypto,
&req->req_body,
ret, r->e_text,
r->ret = ret,
r->client_princ,
r->server_princ,
NULL, NULL,

View File

@@ -859,7 +859,6 @@ tgs_check_authenticator(krb5_context context,
krb5_kdc_configuration *config,
krb5_auth_context ac,
KDC_REQ_BODY *b,
const char **e_text,
krb5_keyblock *key)
{
krb5_authenticator auth;
@@ -982,7 +981,6 @@ tgs_parse_request(astgs_request_t r,
hdb_entry_ex **krbtgt,
krb5_enctype *krbtgt_etype,
krb5_ticket **ticket,
const char **e_text,
const char *from,
const struct sockaddr *from_addr,
time_t **csec,
@@ -1183,8 +1181,8 @@ next_kvno:
}
}
ret = tgs_check_authenticator(r->context, config,
ac, b, e_text, &(*ticket)->ticket.key);
ret = tgs_check_authenticator(r->context, config, ac, b,
&(*ticket)->ticket.key);
if (ret) {
krb5_auth_con_free(r->context, ac);
goto out;
@@ -1426,7 +1424,6 @@ tgs_build_reply(astgs_request_t priv,
hdb_entry_ex *krbtgt,
krb5_enctype krbtgt_etype,
krb5_ticket *ticket,
const char **e_text,
AuthorizationData **auth_data,
const struct sockaddr *from_addr)
{
@@ -1490,7 +1487,7 @@ tgs_build_reply(astgs_request_t priv,
if (s == NULL) {
ret = KRB5KDC_ERR_S_PRINCIPAL_UNKNOWN;
_kdc_set_e_text(priv, "No server in request");
_kdc_set_const_e_text(priv, "No server in request");
goto out;
}
@@ -2498,12 +2495,13 @@ _kdc_tgs_rep(astgs_request_t r)
hdb_entry_ex *krbtgt = NULL;
krb5_ticket *ticket = NULL;
const char *e_text = NULL;
krb5_enctype krbtgt_etype = ETYPE_NULL;
time_t *csec = NULL;
int *cusec = NULL;
r->e_text = NULL;
if(req->padata == NULL){
ret = KRB5KDC_ERR_PREAUTH_REQUIRED; /* XXX ??? */
kdc_log(r->context, config, 4,
@@ -2532,7 +2530,6 @@ _kdc_tgs_rep(astgs_request_t r)
&krbtgt,
&krbtgt_etype,
&ticket,
&e_text,
from, from_addr,
&csec, &cusec,
&auth_data);
@@ -2561,7 +2558,6 @@ _kdc_tgs_rep(astgs_request_t r)
krbtgt,
krbtgt_etype,
ticket,
&e_text,
&auth_data,
from_addr);
if (ret) {
@@ -2574,7 +2570,7 @@ _kdc_tgs_rep(astgs_request_t r)
if (datagram_reply && data->length > config->max_datagram_reply_length) {
krb5_data_free(data);
ret = KRB5KRB_ERR_RESPONSE_TOO_BIG;
e_text = "Reply packet too large";
_kdc_set_const_e_text(r, "Reply packet too large");
}
out:
@@ -2586,7 +2582,7 @@ out:
&error_method,
r->armor_crypto,
&req->req_body,
ret, r->e_text,
r->ret = ret,
ticket != NULL ? ticket->client : NULL,
ticket != NULL ? ticket->server : NULL,
csec, cusec,

View File

@@ -125,7 +125,7 @@ _kdc_audit_trail(kdc_request_t r, krb5_error_code ret)
/* Get a symbolic name for some error codes */
#define CASE(x) case x : retname = #x; break
switch (ret) {
switch (ret ? ret : r->ret) {
CASE(ENOMEM);
CASE(EACCES);
CASE(HDB_ERR_NOT_FOUND_HERE);