Ticket History #7802: KDC sometimes fails to log principal names

-----Original Message-----
From: Richard Basch [mailto:basch@alum.mit.edu]
Sent: Monday, December 16, 2013 2:49 AM
To: 'rt-comment@krbdev.mit.edu'; ''AdminCc of krbdev.mit.edu Ticket #7802':'
Cc: 'Richard Basch'; 'richard.basch@gs.com'
Subject: RE: [krbdev.mit.edu #7802] krb5-1.11 & krb5-1.12: incomplete
logging

I also noticed one other anomaly... For TGS_REQ server princ lookup
failures, it logs twice...
i.e. two messages stating "Server not found in Kerberos database".

(It only occurs with TGS_REQ, not AS_REQ.)

Aside from the double logging (which is misleading), here is a quick patch
which I think fixes the issue.

diff --git a/src/kdc/do_as_req.c b/src/kdc/do_as_req.c
index 268d4f4..95c3e23 100644
--- a/src/kdc/do_as_req.c
+++ b/src/kdc/do_as_req.c
@@ -555,6 +555,19 @@ process_as_req(krb5_kdc_req *request, krb5_data
*req_pkt,
}
limit_string(state->cname);

+ if (!state->request->server) {
+ state->status = "NULL_SERVER";
+ errcode = KRB5KDC_ERR_S_PRINCIPAL_UNKNOWN;
+ goto errout;
+ }
+ if ((errcode = krb5_unparse_name(kdc_context,
+ state->request->server,
+ &state->sname))) {
+ state->status = "UNPARSING_SERVER";
+ goto errout;
+ }
+ limit_string(state->sname);
+
/*
* We set KRB5_KDB_FLAG_CLIENT_REFERRALS_ONLY as a hint
* to the backend to return naming information in lieu
@@ -604,18 +617,6 @@ process_as_req(krb5_kdc_req *request, krb5_data
*req_pkt,

au_state->stage = SRVC_PRINC;

- if (!state->request->server) {
- state->status = "NULL_SERVER";
- errcode = KRB5KDC_ERR_S_PRINCIPAL_UNKNOWN;
- goto errout;
- }
- if ((errcode = krb5_unparse_name(kdc_context,
- state->request->server,
- &state->sname))) {
- state->status = "UNPARSING_SERVER";
- goto errout;
- }
- limit_string(state->sname);
s_flags = 0;
setflag(s_flags, KRB5_KDB_FLAG_ALIAS_OK);
if (isflagset(state->request->kdc_options, KDC_OPT_CANONICALIZE)) {
diff --git a/src/kdc/do_tgs_req.c b/src/kdc/do_tgs_req.c
index c12de2b..76dca73 100644
--- a/src/kdc/do_tgs_req.c
+++ b/src/kdc/do_tgs_req.c
@@ -237,6 +237,7 @@ process_tgs_req(struct server_handle *handle, krb5_data
*pkt

errcode = search_sprinc(kdc_active_realm, request, s_flags, &server,
&status);
+ sprinc = request->server;
if (errcode != 0)
goto cleanup;
sprinc = server->princ;

> If a user attempts to authenticate with an unknown client or to an unknown service, the service name is not depicted in the Kerberos logs.
>
> This makes anomaly detection harder to perform as well as impede diagnostics.
>
> How to reproduce:
>
> 1. For AS_REQ, simply use kinit with an unknown client name (krbtgt/REALM@REALM will not be logged as the service name).

> 2. For TGS_REQ, simply use kvno to query an unknown service name.

-----Original Message-----
From: Richard Basch [mailto:basch@alum.mit.edu]
Sent: Tuesday, December 31, 2013 6:58 PM
To: rt-comment@krbdev.mit.edu; ''AdminCc of krbdev.mit.edu Ticket #7802':';
tlyu@mit.edu; krbdev@mit.edu
Subject: RE: [krbdev.mit.edu #7802] git commit

BTW, I am not sure this part of the patch is correct...

retval = decode_krb5_tgs_req(pkt, &request);
+ /* Save pointer to client-requested service principal, in case of
errors
+ * before a successful call to search_sprinc(). */
+ sprinc = request->server;
if (retval)
return retval;

If decode_krb5_tgs_req fails, I foresee a potential dereference of a null
pointer. You might only want to set sprinc after confirming retval = 0.