krb5 commit: Make etype names in KDC logs human-readable

Greg Hudson <[email protected]>
Newsgroups gmane.comp.encryption.kerberos.cvs
Message-ID <[email protected]>
https://github.com/krb5/krb5/commit/a649279727490687d54becad91fde8cf7429d951
commit a649279727490687d54becad91fde8cf7429d951
Author: Robbie Harwood <[email protected]>
Date:   Tue Jan 8 17:42:35 2019 -0500

    Make etype names in KDC logs human-readable
    
    Introduce enctype_name() as a wrapper over krb5_enctype_to_name for
    converting between registered constants and names.  Adjust signatures
    and rewrite ktypes2str() and rep_etypes2str() to operate on dynamic
    buffers.
    
    ticket: 8772 (new)

 src/kdc/kdc_log.c  |   42 +++++++++---------
 src/kdc/kdc_util.c |  125 ++++++++++++++++++++++++++-------------------------
 src/kdc/kdc_util.h |    6 +--
 3 files changed, 87 insertions(+), 86 deletions(-)

diff --git a/src/kdc/kdc_log.c b/src/kdc/kdc_log.c
index 4eec503..b160ba2 100644
--- a/src/kdc/kdc_log.c
+++ b/src/kdc/kdc_log.c
@@ -65,7 +65,7 @@ log_as_req(krb5_context context,
 {
     const char *fromstring = 0;
     char fromstringbuf[70];
-    char ktypestr[128];
+    char *ktypestr = NULL;
     const char *cname2 = cname ? cname : "<unknown client>";
     const char *sname2 = sname ? sname : "<unknown server>";
 
@@ -74,26 +74,29 @@ log_as_req(krb5_context context,
                            fromstringbuf, sizeof(fromstringbuf));
     if (!fromstring)
         fromstring = "<unknown>";
-    ktypes2str(ktypestr, sizeof(ktypestr),
-               request->nktypes, request->ktype);
+
+    ktypestr = ktypes2str(request->ktype, request->nktypes);
 
     if (status == NULL) {
         /* success */
-        char rep_etypestr[128];
-        rep_etypes2str(rep_etypestr, sizeof(rep_etypestr), reply);
+        char *rep_etypestr = rep_etypes2str(reply);
         krb5_klog_syslog(LOG_INFO, _("AS_REQ (%s) %s: ISSUE: authtime %u, %s, "
                                      "%s for %s"),
-                         ktypestr, fromstring, (unsigned int)authtime,
-                         rep_etypestr, cname2, sname2);
+                         ktypestr ? ktypestr : "", fromstring,
+                         (unsigned int)authtime,
+                         rep_etypestr ? rep_etypestr : "", cname2, sname2);
+        free(rep_etypestr);
     } else {
         /* fail */
         krb5_klog_syslog(LOG_INFO, _("AS_REQ (%s) %s: %s: %s for %s%s%s"),
-                         ktypestr, fromstring, status,
-                         cname2, sname2, emsg ? ", " : "", emsg ? emsg : "");
+                         ktypestr ? ktypestr : "", fromstring, status, cname2,
+                         sname2, emsg ? ", " : "", emsg ? emsg : "");
     }
     krb5_db_audit_as_req(context, request,
                          local_addr->address, remote_addr->address,
                          client, server, authtime, errcode);
+
+    free(ktypestr);
 }
 
 /*
@@ -122,10 +125,9 @@ log_tgs_req(krb5_context ctx, const krb5_fulladdr *from,
             unsigned int c_flags,
             const char *status, krb5_error_code errcode, const char *emsg)
 {
-    char ktypestr[128];
+    char *ktypestr = NULL, *rep_etypestr = NULL;
     const char *fromstring = 0;
     char fromstringbuf[70];
-    char rep_etypestr[128];
     char *cname = NULL, *sname = NULL, *altcname = NULL;
     char *logcname = NULL, *logsname = NULL, *logaltcname = NULL;
 
@@ -134,11 +136,6 @@ log_tgs_req(krb5_context ctx, const krb5_fulladdr *from,
                            fromstringbuf, sizeof(fromstringbuf));
     if (!fromstring)
         fromstring = "<unknown>";
-    ktypes2str(ktypestr, sizeof(ktypestr), request->nktypes, request->ktype);
-    if (!errcode)
-        rep_etypes2str(rep_etypestr, sizeof(rep_etypestr), reply);
-    else
-        rep_etypestr[0] = 0;
 
     unparse_and_limit(ctx, cprinc, &cname);
     logcname = (cname != NULL) ? cname : "<unknown client>";
@@ -151,10 +148,14 @@ log_tgs_req(krb5_context ctx, const krb5_fulladdr *from,
        name (useful), and doesn't log ktypestr (probably not
        important).  */
     if (errcode != KRB5KDC_ERR_SERVER_NOMATCH) {
+        ktypestr = ktypes2str(request->ktype, request->nktypes);
+        rep_etypestr = rep_etypes2str(reply);
         krb5_klog_syslog(LOG_INFO, _("TGS_REQ (%s) %s: %s: authtime %u, %s%s "
                                      "%s for %s%s%s"),
-                         ktypestr, fromstring, status, (unsigned int)authtime,
-                         rep_etypestr, !errcode ? "," : "", logcname, logsname,
+                         ktypestr ? ktypestr : "", fromstring, status,
+                         (unsigned int)authtime,
+                         rep_etypestr ? rep_etypestr : "",
+                         !errcode ? "," : "", logcname, logsname,
                          errcode ? ", " : "", errcode ? emsg : "");
         if (isflagset(c_flags, KRB5_KDB_FLAG_PROTOCOL_TRANSITION))
             krb5_klog_syslog(LOG_INFO,
@@ -171,9 +172,8 @@ log_tgs_req(krb5_context ctx, const krb5_fulladdr *from,
                          fromstring, status, (unsigned int)authtime,
                          logcname, logsname, logaltcname);
 
-    /* OpenSolaris: audit_krb5kdc_tgs_req(...)  or
-       audit_krb5kdc_tgs_req_2ndtktmm(...) */
-
+    free(rep_etypestr);
+    free(ktypestr);
     krb5_free_unparsed_name(ctx, cname);
     krb5_free_unparsed_name(ctx, sname);
     krb5_free_unparsed_name(ctx, altcname);
diff --git a/src/kdc/kdc_util.c b/src/kdc/kdc_util.c
index 6517a21..43dba15 100644
--- a/src/kdc/kdc_util.c
+++ b/src/kdc/kdc_util.c
@@ -1055,84 +1055,87 @@ void limit_string(char *name)
     return;
 }
 
-/*
- * L10_2 = log10(2**x), rounded up; log10(2) ~= 0.301.
- */
-#define L10_2(x) ((int)(((x * 301) + 999) / 1000))
+/* Wrapper of krb5_enctype_to_name() to include the PKINIT types. */
+static krb5_error_code
+enctype_name(krb5_enctype ktype, char *buf, size_t buflen)
+{
+    char *name;
+
+    if (buflen == 0)
+        return EINVAL;
+    *buf = '\0'; /* ensure these are always valid C-strings */
+
+    /* rfc4556 recommends that clients wishing to indicate support for these
+     * pkinit algorithms include them in the etype field of the AS-REQ. */
+    if (ktype == ENCTYPE_DSA_SHA1_CMS)
+        name = "id-dsa-with-sha1-CmsOID";
+    else if (ktype == ENCTYPE_MD5_RSA_CMS)
+        name = "md5WithRSAEncryption-CmsOID";
+    else if (ktype == ENCTYPE_SHA1_RSA_CMS)
+        name = "sha-1WithRSAEncryption-CmsOID";
+    else if (ktype == ENCTYPE_RC2_CBC_ENV)
+        name = "rc2-cbc-EnvOID";
+    else if (ktype == ENCTYPE_RSA_ENV)
+        name = "rsaEncryption-EnvOID";
+    else if (ktype == ENCTYPE_RSA_ES_OAEP_ENV)
+        name = "id-RSAES-OAEP-EnvOID";
+    else if (ktype == ENCTYPE_DES3_CBC_ENV)
+        name = "des-ede3-cbc-EnvOID";
+    else
+        return krb5_enctype_to_name(ktype, FALSE, buf, buflen);
 
-/*
- * Max length of sprintf("%ld") for an int of type T; includes leading
- * minus sign and terminating NUL.
- */
-#define D_LEN(t) (L10_2(sizeof(t) * CHAR_BIT) + 2)
+    if (strlcpy(name, buf, buflen) >= buflen)
+        return ENOMEM;
+    return 0;
+}
 
-void
-ktypes2str(char *s, size_t len, int nktypes, krb5_enctype *ktype)
+char *
+ktypes2str(krb5_enctype *ktype, int nktypes)
 {
+    struct k5buf buf;
     int i;
-    char stmp[D_LEN(krb5_enctype) + 1];
-    char *p;
+    char name[64];
 
-    if (nktypes < 0
-        || len < (sizeof(" etypes {...}") + D_LEN(int))) {
-        *s = '\0';
-        return;
-    }
+    if (nktypes < 0)
+        return NULL;
 
-    snprintf(s, len, "%d etypes {", nktypes);
+    k5_buf_init_dynamic(&buf);
+    k5_buf_add_fmt(&buf, "%d etypes {", nktypes);
     for (i = 0; i < nktypes; i++) {
-        snprintf(stmp, sizeof(stmp), "%s%ld", i ? " " : "", (long)ktype[i]);
-        if (strlen(s) + strlen(stmp) + sizeof("}") > len)
-            break;
-        strlcat(s, stmp, len);
+        enctype_name(ktype[i], name, sizeof(name));
+        k5_buf_add_fmt(&buf, "%s%s(%ld)", i ? ", " : "", name, (long)ktype[i]);
     }
-    if (i < nktypes) {
-        /*
-         * We broke out of the loop. Try to truncate the list.
-         */
-        p = s + strlen(s);
-        while (p - s + sizeof("...}") > len) {
-            while (p > s && *p != ' ' && *p != '{')
-                *p-- = '\0';
-            if (p > s && *p == ' ') {
-                *p-- = '\0';
-                continue;
-            }
-        }
-        strlcat(s, "...", len);
-    }
-    strlcat(s, "}", len);
-    return;
+    k5_buf_add(&buf, "}");
+    return buf.data;
 }
 
-void
-rep_etypes2str(char *s, size_t len, krb5_kdc_rep *rep)
+char *
+rep_etypes2str(krb5_kdc_rep *rep)
 {
-    char stmp[sizeof("ses=") + D_LEN(krb5_enctype)];
-
-    if (len < (3 * D_LEN(krb5_enctype)
-               + sizeof("etypes {rep= tkt= ses=}"))) {
-        *s = '\0';
-        return;
-    }
+    struct k5buf buf;
+    char name[64];
+    krb5_enctype etype;
 
-    snprintf(s, len, "etypes {rep=%ld", (long)rep->enc_part.enctype);
+    k5_buf_init_dynamic(&buf);
+    k5_buf_add(&buf, "etypes {rep=");
+    enctype_name(rep->enc_part.enctype, name, sizeof(name));
+    k5_buf_add_fmt(&buf, "%s(%ld)", name, (long)rep->enc_part.enctype);
 
     if (rep->ticket != NULL) {
-        snprintf(stmp, sizeof(stmp),
-                 " tkt=%ld", (long)rep->ticket->enc_part.enctype);
-        strlcat(s, stmp, len);
+        etype = rep->ticket->enc_part.enctype;
+        enctype_name(etype, name, sizeof(name));
+        k5_buf_add_fmt(&buf, ", tkt=%s(%ld)", name, (long)etype);
     }
 
-    if (rep->ticket != NULL
-        && rep->ticket->enc_part2 != NULL
-        && rep->ticket->enc_part2->session != NULL) {
-        snprintf(stmp, sizeof(stmp), " ses=%ld",
-                 (long)rep->ticket->enc_part2->session->enctype);
-        strlcat(s, stmp, len);
+    if (rep->ticket != NULL && rep->ticket->enc_part2 != NULL &&
+        rep->ticket->enc_part2->session != NULL) {
+        etype = rep->ticket->enc_part2->session->enctype;
+        enctype_name(etype, name, sizeof(name));
+        k5_buf_add_fmt(&buf, ", ses=%s(%ld)", name, (long)etype);
     }
-    strlcat(s, "}", len);
-    return;
+
+    k5_buf_add(&buf, "}");
+    return buf.data;
 }
 
 static krb5_error_code
diff --git a/src/kdc/kdc_util.h b/src/kdc/kdc_util.h
index b89a3de..1314bdd 100644
--- a/src/kdc/kdc_util.h
+++ b/src/kdc/kdc_util.h
@@ -109,11 +109,9 @@ select_session_keytype (kdc_realm_t *kdc_active_realm,
 
 void limit_string (char *name);
 
-void
-ktypes2str(char *s, size_t len, int nktypes, krb5_enctype *ktype);
+char *ktypes2str(krb5_enctype *ktype, int nktypes);
 
-void
-rep_etypes2str(char *s, size_t len, krb5_kdc_rep *rep);
+char *rep_etypes2str(krb5_kdc_rep *rep);
 
 /* authind.c */
 krb5_boolean
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.