kdc: add _nocopy setter for use by mssfu
authorLuke Howard <lukeh@padl.com>
Thu, 20 Jan 2022 04:26:50 +0000 (15:26 +1100)
committerLuke Howard <lukeh@padl.com>
Thu, 20 Jan 2022 06:23:24 +0000 (17:23 +1100)
Add an internal-use setter accessor for use by mssfu.c when principal names are
replaced. This also fixes a leak where r->client_princ was not freed before
being replaced with the impersonated client name.

kdc/kdc-plugin.c
kdc/kdc_locl.h
kdc/mssfu.c

index e2a114f57c31c4b00ae39af93788b133ab6eb364..80a155529f323f582aa37641d80441f1f28a4f5f 100644 (file)
@@ -493,3 +493,35 @@ free_keyblock(EncryptionKey *key)
 
 #undef HEIMDAL_KDC_KDC_ACCESSORS_H
 #include "kdc-accessors.h"
+
+#undef _KDC_REQUEST_GET_ACCESSOR
+#undef _KDC_REQUEST_SET_ACCESSOR
+
+#undef _KDC_REQUEST_GET_ACCESSOR_PTR
+#undef _KDC_REQUEST_SET_ACCESSOR_PTR
+#define _KDC_REQUEST_SET_ACCESSOR_PTR(R, T, t, f)          \
+    void                                                   \
+    _kdc_request_set_ ## f ## _nocopy(R r, T *v)           \
+    {                                                      \
+       if (*v != r->f) {                                   \
+           free_##t(r->f);                                 \
+           r->f = *v;                                      \
+       }                                                   \
+       *v = NULL;                                          \
+    }
+
+#undef _KDC_REQUEST_GET_ACCESSOR_STRUCT
+#undef _KDC_REQUEST_SET_ACCESSOR_STRUCT
+#define _KDC_REQUEST_SET_ACCESSOR_STRUCT(R, T, t, f)       \
+    void                                                   \
+    _kdc_request_set_ ## f ## _nocopy(R r, T *v)           \
+    {                                                      \
+       if (v != &r->f) {                                   \
+           free_##t(&r->f);                                \
+           r->f = *v;                                      \
+       }                                                   \
+       memset(v, 0, sizeof(*v));                           \
+    }
+
+#undef HEIMDAL_KDC_KDC_ACCESSORS_H
+#include "kdc-accessors.h"
index 43af59c8cca91e3ab228de1e1d1cd84b2ced3a35..86e6a712f452a1b891b71516fa938e071117bd62 100644 (file)
@@ -244,4 +244,24 @@ configure(krb5_context context, int argc, char **argv, int *optidx);
 void bonjour_announce(krb5_context, krb5_kdc_configuration *);
 #endif
 
+/* no-copy setters */
+
+#undef _KDC_REQUEST_GET_ACCESSOR
+#undef _KDC_REQUEST_SET_ACCESSOR
+
+#undef _KDC_REQUEST_GET_ACCESSOR_PTR
+#undef _KDC_REQUEST_SET_ACCESSOR_PTR
+#define _KDC_REQUEST_SET_ACCESSOR_PTR(R, T, t, f)          \
+    void                                                   \
+    _kdc_request_set_ ## f ## _nocopy(R r, T *v);
+
+#undef _KDC_REQUEST_GET_ACCESSOR_STRUCT
+#undef _KDC_REQUEST_SET_ACCESSOR_STRUCT
+#define _KDC_REQUEST_SET_ACCESSOR_STRUCT(R, T, t, f)       \
+    void                                                   \
+    _kdc_request_set_ ## f ## _nocopy(R r, T *v);
+
+#undef HEIMDAL_KDC_KDC_ACCESSORS_H
+#include "kdc-accessors.h"
+
 #endif /* __KDC_LOCL_H__ */
index dada92e0eb89f260feb22ba0c78e908fdfca2195..9e67aad33193aa3375ca05485c7aa1deac29b406 100644 (file)
@@ -96,37 +96,6 @@ check_constrained_delegation(krb5_context context,
     return ret;
 }
 
-static void
-update_client_names(astgs_request_t r,
-                   char **s4ucname,
-                   krb5_principal *s4u_client_name,
-                   HDB **s4u_clientdb,
-                   hdb_entry **s4u_client,
-                   krb5_principal *s4u_canon_client_name,
-                   krb5_pac *s4u_pac)
-{
-    krb5_xfree(r->cname);
-    r->cname = *s4ucname;
-    *s4ucname = NULL;
-
-    r->client_princ = *s4u_client_name;
-    *s4u_client_name = NULL;
-
-    _kdc_free_ent(r->context, r->clientdb, r->client);
-    r->client = *s4u_client;
-    *s4u_client = NULL;
-    r->clientdb = *s4u_clientdb;
-    *s4u_clientdb = NULL;
-
-    krb5_free_principal(r->context, r->canon_client_princ);
-    r->canon_client_princ = *s4u_canon_client_name;
-    *s4u_canon_client_name = NULL;
-
-    krb5_pac_free(r->context, r->pac);
-    r->pac = *s4u_pac;
-    *s4u_pac = NULL;
-}
-
 /*
  * Validate a protocol transition (S4U2Self) request. If present and
  * successfully validated then the client in the request structure
@@ -338,9 +307,17 @@ validate_protocol_transition(astgs_request_t r)
      * impersonated client. (The audit entry containing the original
      * client name will have been created before this point.)
      */
-    update_client_names(r, &s4ucname, &s4u_client_name,
-                       &s4u_clientdb, &s4u_client,
-                       &s4u_canon_client_name, &s4u_pac);
+    _kdc_request_set_cname_nocopy((kdc_request_t)r, &s4ucname);
+    _kdc_request_set_client_princ_nocopy(r, &s4u_client_name);
+
+    _kdc_free_ent(r->context, r->clientdb, r->client);
+    r->client = s4u_client;
+    s4u_client = NULL;
+    r->clientdb = s4u_clientdb;
+    s4u_clientdb = NULL;
+
+    _kdc_request_set_canon_client_princ_nocopy(r, &s4u_canon_client_name);
+    _kdc_request_set_pac_nocopy(r, &s4u_pac);
 
 out:
     if (s4u_client)
@@ -545,9 +522,18 @@ validate_constrained_delegation(astgs_request_t r)
      * impersonated client. (The audit entry containing the original
      * client name will have been created before this point.)
      */
-    update_client_names(r, &s4ucname, &s4u_client_name,
-                       &s4u_clientdb, &s4u_client,
-                       &s4u_canon_client_name, &s4u_pac);
+    _kdc_request_set_cname_nocopy((kdc_request_t)r, &s4ucname);
+    _kdc_request_set_client_princ_nocopy(r, &s4u_client_name);
+
+    _kdc_free_ent(r->context, r->clientdb, r->client);
+    r->client = s4u_client;
+    s4u_client = NULL;
+    r->clientdb = s4u_clientdb;
+    s4u_clientdb = NULL;
+
+    _kdc_request_set_canon_client_princ_nocopy(r, &s4u_canon_client_name);
+    _kdc_request_set_pac_nocopy(r, &s4u_pac);
+
     r->pac_attributes = s4u_pac_attributes;
 
 out: