krb5 commit: Add KDC support for X.509 S4U2Self requests

https://github.com/krb5/krb5/commit/0fbfffbef2c266fedac557e00108b944e31e8d50
commit 0fbfffbef2c266fedac557e00108b944e31e8d50
Author: Isaac Boukris <[email protected]>
Date:   Thu Jan 17 00:23:25 2019 +0200

    Add KDC support for X.509 S4U2Self requests
    
    Add a KDB function krb5_db_get_s4u_x509_principal() and an associated
    method in the DAL, bumping the minor version and cleaning up a
    leftover comment in the table from major version 6.
    
    When processing an AS-REQ, look up the client principal by certificate
    if the request contains a non-empty PA-S4U-X509-USER value.  When
    processing an S4U2Self TGS-REQ, allow requests with certificates, and
    look up the client principal by certificate if one is presented.
    
    [[email protected]: factored out lookup_client() in AS code; rewrote
    commit message and some comments; adjusted flow control changes in
    kdc_process_s4u_x509_user()]
    
    ticket: 8781 (new)

 src/include/kdb.h           |   31 ++++++++++++++++++++++++++++---
 src/kdc/do_as_req.c         |   23 +++++++++++++++++++++--
 src/kdc/kdc_util.c          |   34 +++++++++++++++++++++++++++-------
 src/lib/kdb/kdb5.c          |   32 ++++++++++++++++++++++++++++++++
 src/lib/kdb/libkdb5.exports |    1 +
 5 files changed, 109 insertions(+), 12 deletions(-)

diff --git a/src/include/kdb.h b/src/include/kdb.h
index 9812a35..92f8458 100644
--- a/src/include/kdb.h
+++ b/src/include/kdb.h
@@ -707,6 +707,12 @@ krb5_error_code krb5_db_check_allowed_to_delegate(krb5_context kcontext,
                                                   const krb5_db_entry *server,
                                                   krb5_const_principal proxy);
 
+krb5_error_code krb5_db_get_s4u_x509_principal(krb5_context kcontext,
+                                               const krb5_data *client_cert,
+                                               krb5_const_principal in_princ,
+                                               unsigned int flags,
+                                               krb5_db_entry **entry);
+
 /**
  * Sort an array of @a krb5_key_data keys in descending order by their kvno.
  * Key data order within a kvno is preserved.
@@ -1389,8 +1395,6 @@ typedef struct _kdb_vftabl {
                                                  const krb5_db_entry *server,
                                                  krb5_const_principal proxy);
 
-    /* End of minor version 0. */
-
     /*
      * Optional: Free the e_data pointer of a database entry.  If this method
      * is not implemented, the e_data pointer in principal entries will be
@@ -1398,7 +1402,28 @@ typedef struct _kdb_vftabl {
      */
     void (*free_principal_e_data)(krb5_context kcontext, krb5_octet *e_data);
 
-    /* End of minor version 1 for major version 6. */
+    /* End of minor version 0. */
+
+    /*
+     * Optional: get a principal entry for S4U2Self based on X509 certificate.
+     *
+     * If flags include KRB5_KDB_FLAG_CLIENT_REFERRALS_ONLY, princ->realm
+     * indicates the request realm, but the data components should be ignored.
+     * The module can return an out-of-realm client referral as it would for
+     * get_principal().
+     *
+     * If flags does not include KRB5_KDB_FLAG_CLIENT_REFERRALS_ONLY, princ is
+     * from PA-S4U-X509-USER.  If it contains data components (and not just a
+     * realm), the module should verify that it is the same as the lookup
+     * result for client_cert.  The module should not return a referral.
+     */
+    krb5_error_code (*get_s4u_x509_principal)(krb5_context kcontext,
+                                              const krb5_data *client_cert,
+                                              krb5_const_principal princ,
+                                              unsigned int flags,
+                                              krb5_db_entry **entry_out);
+
+    /* End of minor version 1 for major version 7. */
 } kdb_vftabl;
 
 #endif /* !defined(_WIN32) */
diff --git a/src/kdc/do_as_req.c b/src/kdc/do_as_req.c
index 8a96c12..b49df6a 100644
--- a/src/kdc/do_as_req.c
+++ b/src/kdc/do_as_req.c
@@ -130,6 +130,25 @@ select_client_key(krb5_context context, krb5_db_entry *client,
     return 0;
 }
 
+static krb5_error_code
+lookup_client(krb5_context context, krb5_kdc_req *req, unsigned int flags,
+              krb5_db_entry **entry_out)
+{
+    krb5_pa_data *pa;
+    krb5_data cert;
+
+    *entry_out = NULL;
+    pa = krb5int_find_pa_data(context, req->padata, KRB5_PADATA_S4U_X509_USER);
+    if (pa != NULL && pa->length != 0 &&
+        req->client->type == KRB5_NT_X500_PRINCIPAL) {
+        cert = make_data(pa->contents, pa->length);
+        return krb5_db_get_s4u_x509_principal(context, &cert, req->client,
+                                              flags, entry_out);
+    } else {
+        return krb5_db_get_principal(context, req->client, flags, entry_out);
+    }
+}
+
 struct as_req_state {
     loop_respond_fn respond;
     void *arg;
@@ -597,8 +616,8 @@ process_as_req(krb5_kdc_req *request, krb5_data *req_pkt,
     if (include_pac_p(kdc_context, state->request)) {
         setflag(state->c_flags, KRB5_KDB_FLAG_INCLUDE_PAC);
     }
-    errcode = krb5_db_get_principal(kdc_context, state->request->client,
-                                    state->c_flags, &state->client);
+    errcode = lookup_client(kdc_context, state->request, state->c_flags,
+                            &state->client);
     if (errcode == KRB5_KDB_CANTLOCK_DB)
         errcode = KRB5KDC_ERR_SVC_UNAVAILABLE;
     if (errcode == KRB5_KDB_NOENTRY) {
diff --git a/src/kdc/kdc_util.c b/src/kdc/kdc_util.c
index 0dcc0c3..1c77cc1 100644
--- a/src/kdc/kdc_util.c
+++ b/src/kdc/kdc_util.c
@@ -1364,8 +1364,8 @@ kdc_process_s4u_x509_user(krb5_context context,
         return code;
     }
 
-    if (krb5_princ_size(context, (*s4u_x509_user)->user_id.user) == 0 ||
-        (*s4u_x509_user)->user_id.subject_cert.length != 0) {
+    if (krb5_princ_size(context, (*s4u_x509_user)->user_id.user) == 0 &&
+        (*s4u_x509_user)->user_id.subject_cert.length == 0) {
         *status = "INVALID_S4U2SELF_REQUEST";
         krb5_free_pa_s4u_x509_user(context, *s4u_x509_user);
         *s4u_x509_user = NULL;
@@ -1487,6 +1487,7 @@ kdc_process_s4u2self_req(kdc_realm_t *kdc_active_realm,
     krb5_pa_data                *pa_data;
     int                         flags;
     krb5_db_entry               *princ;
+    krb5_s4u_userid             *id;
 
     *princ_ptr = NULL;
 
@@ -1516,6 +1517,7 @@ kdc_process_s4u2self_req(kdc_realm_t *kdc_active_realm,
         } else
             return 0;
     }
+    id = &(*s4u_x509_user)->user_id;
 
     /*
      * We need to compare the client name in the TGT with the requested
@@ -1601,8 +1603,7 @@ kdc_process_s4u2self_req(kdc_realm_t *kdc_active_realm,
     /*
      * Do not attempt to lookup principals in foreign realms.
      */
-    if (is_local_principal(kdc_active_realm,
-                           (*s4u_x509_user)->user_id.user)) {
+    if (is_local_principal(kdc_active_realm, id->user)) {
         krb5_db_entry no_server;
         krb5_pa_data **e_data = NULL;
 
@@ -1613,9 +1614,20 @@ kdc_process_s4u2self_req(kdc_realm_t *kdc_active_realm,
             return KRB5KDC_ERR_C_PRINCIPAL_UNKNOWN; /* match Windows error */
         }
 
-        code = krb5_db_get_principal(kdc_context,
-                                     (*s4u_x509_user)->user_id.user,
-                                     KRB5_KDB_FLAG_INCLUDE_PAC, &princ);
+        if (id->subject_cert.length != 0) {
+            code = krb5_db_get_s4u_x509_principal(kdc_context,
+                                                  &id->subject_cert, id->user,
+                                                  KRB5_KDB_FLAG_INCLUDE_PAC,
+                                                  &princ);
+            if (code == 0 && id->user->length == 0) {
+                krb5_free_principal(kdc_context, id->user);
+                code = krb5_copy_principal(kdc_context, princ->princ,
+                                           &id->user);
+            }
+        } else {
+            code = krb5_db_get_principal(kdc_context, id->user,
+                                         KRB5_KDB_FLAG_INCLUDE_PAC, &princ);
+        }
         if (code == KRB5_KDB_NOENTRY) {
             *status = "UNKNOWN_S4U2SELF_PRINCIPAL";
             return KRB5KDC_ERR_C_PRINCIPAL_UNKNOWN;
@@ -1648,6 +1660,14 @@ kdc_process_s4u2self_req(kdc_realm_t *kdc_active_realm,
          */
         *status = "S4U2SELF_CLIENT_NOT_OURS";
         return KRB5KDC_ERR_POLICY; /* match Windows error */
+    } else if (id->user->length == 0) {
+        /*
+         * Only a KDC in the client realm can handle a certificate-only
+         * S4U2Self request.  Other KDCs require a principal name and ignore
+         * the subject-certificate field.
+         */
+        *status = "INVALID_XREALM_S4U2SELF_REQUEST";
+        return KRB5KDC_ERR_POLICY; /* match Windows error */
     }
 
     return 0;
diff --git a/src/lib/kdb/kdb5.c b/src/lib/kdb/kdb5.c
index da53322..0c25bf2 100644
--- a/src/lib/kdb/kdb5.c
+++ b/src/lib/kdb/kdb5.c
@@ -324,6 +324,12 @@ copy_vtable(const kdb_vftabl *in, kdb_vftabl *out)
     out->check_allowed_to_delegate = in->check_allowed_to_delegate;
     out->free_principal_e_data = in->free_principal_e_data;
 
+    /* Copy fields for minor version 1 (major version 7). */
+    assert(KRB5_KDB_DAL_MAJOR_VERSION == 7);
+    out->get_s4u_x509_principal = NULL;
+    if (in->min_ver >= 1)
+        out->get_s4u_x509_principal = in->get_s4u_x509_principal;
+
     /* Set defaults for optional fields. */
     if (out->fetch_master_key == NULL)
         out->fetch_master_key = krb5_db_def_fetch_mkey;
@@ -2717,6 +2723,32 @@ krb5_db_check_allowed_to_delegate(krb5_context kcontext,
     return v->check_allowed_to_delegate(kcontext, client, server, proxy);
 }
 
+krb5_error_code
+krb5_db_get_s4u_x509_principal(krb5_context kcontext,
+                               const krb5_data *client_cert,
+                               krb5_const_principal in_princ,
+                               unsigned int flags, krb5_db_entry **entry)
+{
+    krb5_error_code ret;
+    kdb_vftabl *v;
+
+    ret = get_vftabl(kcontext, &v);
+    if (ret)
+        return ret;
+    if (v->get_s4u_x509_principal == NULL)
+        return KRB5_PLUGIN_OP_NOTSUPP;
+    ret = v->get_s4u_x509_principal(kcontext, client_cert, in_princ, flags,
+                                    entry);
+    if (ret)
+        return ret;
+
+    /* Sort the keys in the db entry, same as get_principal(). */
+    if ((*entry)->key_data != NULL)
+        krb5_dbe_sort_key_data((*entry)->key_data, (*entry)->n_key_data);
+
+    return 0;
+}
+
 void
 krb5_dbe_sort_key_data(krb5_key_data *key_data, size_t key_data_length)
 {
diff --git a/src/lib/kdb/libkdb5.exports b/src/lib/kdb/libkdb5.exports
index 5df596a..f9f602e 100644
--- a/src/lib/kdb/libkdb5.exports
+++ b/src/lib/kdb/libkdb5.exports
@@ -5,6 +5,7 @@ krb5_db_alloc
 krb5_db_free
 krb5_db_audit_as_req
 krb5_db_check_allowed_to_delegate
+krb5_db_get_s4u_x509_principal
 krb5_db_check_policy_as
 krb5_db_check_policy_tgs
 krb5_db_check_transited_realms