We leak

src/client/gpm_accept_sec_context.c

@@ -21,7 +21,6 @@ OM_uint32 gpm_accept_sec_context(OM_uint32 *minor_status,
     gssx_res_accept_sec_context *res = &ures.accept_sec_context;
     gssx_ctx *ctx = NULL;
     gssx_name *name = NULL;
-    gss_OID_desc *mech = NULL;
     gss_buffer_t outbuf = NULL;
     uint32_t ret_maj;
     int ret;
@@ -70,15 +69,6 @@ OM_uint32 gpm_accept_sec_context(OM_uint32 *minor_status,
         goto done;
     }
 
-    if (mech_type) {
-        if (res->status.mech.octet_string_len) {
-            ret = gp_conv_gssx_to_oid_alloc(&res->status.mech, &mech);
-            if (ret) {
-                goto done;
-            }
-        }
-    }
-
     ctx = res->context_handle;
     /* we are stealing the delegated creds on success, so we do not want
      * it to be freed by xdr_free */
@@ -101,8 +91,14 @@ OM_uint32 gpm_accept_sec_context(OM_uint32 *minor_status,
     }
 
     if (mech_type) {
-        *mech_type = mech;
+        gss_OID_desc mech;
+        gp_conv_gssx_to_oid(&res->status.mech, &mech);
+        ret = gpm_mech_to_static(&mech, mech_type);
+        if (ret) {
+            goto done;
+        }
     }
+
     if (src_name) {
         *src_name = name;
     }
@@ -145,10 +141,6 @@ OM_uint32 gpm_accept_sec_context(OM_uint32 *minor_status,
             xdr_free((xdrproc_t)xdr_gssx_name, (char *)name);
             free(name);
         }
-        if (mech) {
-            free(mech->elements);
-            free(mech);
-        }
         if (outbuf) {
             free(outbuf->value);
             free(outbuf);

src/client/gpm_common.c

@@ -799,4 +799,3 @@ void gpm_free_xdrs(int proc, union gp_rpc_arg *arg, union gp_rpc_res *res)
     xdr_free(gpm_xdr_set[proc].arg_fn, (char *)arg);
     xdr_free(gpm_xdr_set[proc].res_fn, (char *)res);
 }
-

src/client/gpm_import_and_canon_name.c

@@ -2,6 +2,26 @@
 
 #include "gssapi_gpm.h"
 
+static int gpm_name_oid_to_static(gss_OID name_type, gss_OID *name_static)
+{
+#define ret_static(b) \
+    if (gss_oid_equal(name_type, b)) { \
+        *name_static = b; \
+        return 0; \
+    }
+    ret_static(GSS_C_NT_USER_NAME);
+    ret_static(GSS_C_NT_MACHINE_UID_NAME);
+    ret_static(GSS_C_NT_STRING_UID_NAME);
+    ret_static(GSS_C_NT_HOSTBASED_SERVICE_X);
+    ret_static(GSS_C_NT_HOSTBASED_SERVICE);
+    ret_static(GSS_C_NT_ANONYMOUS);
+    ret_static(GSS_C_NT_EXPORT_NAME);
+    ret_static(GSS_C_NT_COMPOSITE_EXPORT);
+    ret_static(GSS_KRB5_NT_PRINCIPAL_NAME);
+    ret_static(gss_nt_krb5_name);
+    return ENOENT;
+}
+
 OM_uint32 gpm_display_name(OM_uint32 *minor_status,
                            gssx_name *in_name,
                            gss_buffer_t output_name_buffer,
@@ -57,7 +77,9 @@ OM_uint32 gpm_display_name(OM_uint32 *minor_status,
     }
 
     if (output_name_type) {
-        ret = gp_conv_gssx_to_oid_alloc(&in_name->name_type, output_name_type);
+        gss_OID_desc oid;
+        gp_conv_gssx_to_oid(&in_name->name_type, &oid);
+        ret = gpm_name_oid_to_static(&oid, output_name_type);
         if (ret) {
             gss_release_buffer(&discard, output_name_buffer);
             ret_min = ret;
@@ -285,7 +307,9 @@ OM_uint32 gpm_inquire_name(OM_uint32 *minor_status,
     }
 
     if (MN_mech != NULL) {
-        ret = gp_conv_gssx_to_oid_alloc(&name->name_type, MN_mech);
+        gss_OID_desc oid;
+        gp_conv_gssx_to_oid(&name->name_type, &oid);
+        ret = gpm_name_oid_to_static(&oid, MN_mech);
         if (ret) {
             *minor_status = ret;
             return GSS_S_FAILURE;

src/client/gpm_indicate_mechs.c

@@ -95,20 +95,6 @@ static uint32_t gpm_copy_gss_buffer(uint32_t *minor_status,
     return GSS_S_COMPLETE;
 }
 
-static bool gpm_equal_oids(gss_const_OID a, gss_const_OID b)
-{
-    int ret;
-
-    if (a->length == b->length) {
-        ret = memcmp(a->elements, b->elements, a->length);
-        if (ret == 0) {
-            return true;
-        }
-    }
-
-    return false;
-}
-
 static void gpmint_indicate_mechs(void)
 {
     union gp_rpc_arg uarg;
@@ -300,6 +286,40 @@ static int gpmint_init_global_mechs(void)
     return 0;
 }
 
+/* GSSAPI requires some APIs to return "static" mechs that callers do not need
+ * to free. So match a radom mech and return from our global "static" array */
+int gpm_mech_to_static(gss_OID mech_type, gss_OID *mech_static)
+{
+    int ret;
+
+    ret = gpmint_init_global_mechs();
+    if (ret) {
+        return ret;
+    }
+
+    *mech_static = GSS_C_NO_OID;
+    for (size_t i = 0; i < global_mechs.mech_set->count; i++) {
+        if (gss_oid_equal(&global_mechs.mech_set->elements[i], mech_type)) {
+            *mech_static = &global_mechs.mech_set->elements[i];
+            return 0;
+        }
+    }
+    /* TODO: potentially in future add the mech to the list if missing */
+    return ENOENT;
+}
+
+bool gpm_mech_is_static(gss_OID mech_type)
+{
+    if (global_mechs.mech_set) {
+        for (size_t i = 0; i < global_mechs.mech_set->count; i++) {
+            if (&global_mechs.mech_set->elements[i] == mech_type) {
+                return true;
+            }
+        }
+    }
+    return false;
+}
+
 OM_uint32 gpm_indicate_mechs(OM_uint32 *minor_status, gss_OID_set *mech_set)
 {
     uint32_t ret_min;
@@ -349,7 +369,7 @@ OM_uint32 gpm_inquire_names_for_mech(OM_uint32 *minor_status,
     }
 
     for (unsigned i = 0; i < global_mechs.info_len; i++) {
-        if (!gpm_equal_oids(global_mechs.info[i].mech, mech_type)) {
+        if (!gss_oid_equal(global_mechs.info[i].mech, mech_type)) {
             continue;
         }
         ret_maj = gpm_copy_gss_OID_set(&ret_min,
@@ -370,7 +390,7 @@ OM_uint32 gpm_inquire_mechs_for_name(OM_uint32 *minor_status,
     uint32_t ret_min;
     uint32_t ret_maj;
     uint32_t discard;
-    gss_OID name_type = GSS_C_NO_OID;
+    gss_OID_desc name_type;
     int present;
 
     if (!minor_status) {
@@ -387,19 +407,14 @@ OM_uint32 gpm_inquire_mechs_for_name(OM_uint32 *minor_status,
         return GSS_S_FAILURE;
     }
 
-    ret_min = gp_conv_gssx_to_oid_alloc(&input_name->name_type, &name_type);
-    if (ret_min) {
-        ret_maj = GSS_S_FAILURE;
-        goto done;
-    }
-
     ret_maj = gss_create_empty_oid_set(&ret_min, mech_types);
     if (ret_maj) {
         goto done;
     }
 
+    gp_conv_gssx_to_oid(&input_name->name_type, &name_type);
     for (unsigned i = 0; i < global_mechs.info_len; i++) {
-        ret_maj = gss_test_oid_set_member(&ret_min, name_type,
+        ret_maj = gss_test_oid_set_member(&ret_min, &name_type,
                                           global_mechs.info[i].name_types,
                                           &present);
         if (ret_maj) {
@@ -417,7 +432,6 @@ OM_uint32 gpm_inquire_mechs_for_name(OM_uint32 *minor_status,
     }
 
 done:
-    gss_release_oid(&discard, &name_type);
     if (ret_maj) {
         gss_release_oid_set(&discard, mech_types);
         *minor_status = ret_min;
@@ -447,7 +461,7 @@ OM_uint32 gpm_inquire_attrs_for_mech(OM_uint32 *minor_status,
     }
 
     for (unsigned i = 0; i < global_mechs.info_len; i++) {
-        if (!gpm_equal_oids(global_mechs.info[i].mech, mech)) {
+        if (!gss_oid_equal(global_mechs.info[i].mech, mech)) {
             continue;
         }
 
@@ -506,7 +520,7 @@ OM_uint32 gpm_inquire_saslname_for_mech(OM_uint32 *minor_status,
     }
 
     for (unsigned i = 0; i < global_mechs.info_len; i++) {
-        if (!gpm_equal_oids(global_mechs.info[i].mech, desired_mech)) {
+        if (!gss_oid_equal(global_mechs.info[i].mech, desired_mech)) {
             continue;
         }
         ret_maj = gpm_copy_gss_buffer(&ret_min,
@@ -564,7 +578,7 @@ OM_uint32 gpm_display_mech_attr(OM_uint32 *minor_status,
     }
 
     for (unsigned i = 0; i < global_mechs.desc_len; i++) {
-        if (!gpm_equal_oids(global_mechs.desc[i].attr, mech_attr)) {
+        if (!gss_oid_equal(global_mechs.desc[i].attr, mech_attr)) {
             continue;
         }
         ret_maj = gpm_copy_gss_buffer(&ret_min,

src/client/gpm_init_sec_context.c

@@ -43,7 +43,6 @@ OM_uint32 gpm_init_sec_context(OM_uint32 *minor_status,
     gssx_arg_init_sec_context *arg = &uarg.init_sec_context;
     gssx_res_init_sec_context *res = &ures.init_sec_context;
     gssx_ctx *ctx = NULL;
-    gss_OID_desc *mech = NULL;
     gss_buffer_t outbuf = NULL;
     uint32_t ret_maj = GSS_S_COMPLETE;
     uint32_t ret_min = 0;
@@ -100,11 +99,12 @@ OM_uint32 gpm_init_sec_context(OM_uint32 *minor_status,
 
     /* return values */
     if (actual_mech_type) {
-        if (res->status.mech.octet_string_len) {
-            ret = gp_conv_gssx_to_oid_alloc(&res->status.mech, &mech);
-            if (ret) {
-                goto done;
-            }
+        gss_OID_desc mech;
+        gp_conv_gssx_to_oid(&res->status.mech, &mech);
+        ret = gpm_mech_to_static(&mech, actual_mech_type);
+        if (ret) {
+            gpm_save_internal_status(ret, gp_strerror(ret));
+            goto done;
         }
     }
 
@@ -151,9 +151,6 @@ OM_uint32 gpm_init_sec_context(OM_uint32 *minor_status,
     gpm_free_xdrs(GSSX_INIT_SEC_CONTEXT, &uarg, &ures);
 
     if (ret_maj == GSS_S_COMPLETE || ret_maj == GSS_S_CONTINUE_NEEDED) {
-        if (actual_mech_type) {
-            *actual_mech_type = mech;
-        }
         if (outbuf) {
             *output_token = *outbuf;
             free(outbuf);
@@ -170,10 +167,6 @@ OM_uint32 gpm_init_sec_context(OM_uint32 *minor_status,
             free(ctx);
             ctx = NULL;
         }
-        if (mech) {
-            free(mech->elements);
-            free(mech);
-        }
         if (outbuf) {
             free(outbuf->value);
             free(outbuf);

src/client/gpm_inquire_context.c

@@ -51,7 +51,9 @@ OM_uint32 gpm_inquire_context(OM_uint32 *minor_status,
     }
 
     if (mech_type) {
-        ret = gp_conv_gssx_to_oid_alloc(&context_handle->mech, mech_type);
+        gss_OID_desc mech;
+        gp_conv_gssx_to_oid(&context_handle->mech, &mech);
+        ret = gpm_mech_to_static(&mech, mech_type);
         if (ret) {
             if (src_name) {
                 (void)gpm_release_name(&tmp_min, src_name);

src/client/gpm_release_handle.c

@@ -106,5 +106,7 @@ OM_uint32 gpm_delete_sec_context(OM_uint32 *minor_status,
     gpm_free_xdrs(GSSX_RELEASE_HANDLE, &uarg, &ures);
 done:
     xdr_free((xdrproc_t)xdr_gssx_ctx, (char *)r);
+    free(r);
+    *context_handle = NULL;
     return ret;
 }

src/client/gssapi_gpm.h

@@ -10,6 +10,7 @@
 #include <string.h>
 #include <gssapi/gssapi.h>
 #include <gssapi/gssapi_ext.h>
+#include <gssapi/gssapi_krb5.h>
 #include "rpcgen/gp_rpc.h"
 #include "rpcgen/gss_proxy.h"
 #include "src/gp_common.h"
@@ -27,6 +28,9 @@ void gpm_display_status_init_once(void);
 void gpm_save_status(gssx_status *status);
 void gpm_save_internal_status(uint32_t err, char *err_str);
 
+int gpm_mech_to_static(gss_OID mech_type, gss_OID *mech_static);
+bool gpm_mech_is_static(gss_OID mech_type);
+
 OM_uint32 gpm_display_status(OM_uint32 *minor_status,
                              OM_uint32 status_value,
                              int status_type,

src/gp_creds.c

@@ -775,6 +775,7 @@ static uint32_t get_impersonator_fallback(uint32_t *min, gss_cred_id_t cred,
     uint32_t ret_maj = 0;
     uint32_t ret_min = 0;
     char *memcache = NULL;
+    char **ptr = &memcache;
     krb5_context context = NULL;
     krb5_ccache ccache = NULL;
     krb5_data config;
@@ -791,7 +792,7 @@ static uint32_t get_impersonator_fallback(uint32_t *min, gss_cred_id_t cred,
     gss_key_value_element_desc ccelement = { "ccache", NULL };
     gss_key_value_set_desc cred_store = { 1, &ccelement };
 
-    err = asprintf(&memcache, "MEMORY:cred_allowed_%p", &memcache);
+    err = asprintf(&memcache, "MEMORY:cred_allowed_%p", ptr);
     if (err == -1) {
         memcache = NULL;
         ret_min = ENOMEM;
@@ -991,6 +992,7 @@ uint32_t gp_count_tickets(uint32_t *min, gss_cred_id_t cred, uint32_t *ccsum)
     uint32_t ret_maj = 0;
     uint32_t ret_min = 0;
     char *memcache = NULL;
+    char **ptr = &memcache;
     krb5_context context = NULL;
     krb5_ccache ccache = NULL;
     krb5_cc_cursor cursor = NULL;
@@ -1008,7 +1010,7 @@ uint32_t gp_count_tickets(uint32_t *min, gss_cred_id_t cred, uint32_t *ccsum)
     gss_key_value_element_desc ccelement = { "ccache", NULL };
     gss_key_value_set_desc cred_store = { 1, &ccelement };
 
-    err = asprintf(&memcache, "MEMORY:cred_allowed_%p", &memcache);
+    err = asprintf(&memcache, "MEMORY:cred_allowed_%p", ptr);
     if (err == -1) {
         memcache = NULL;
         ret_min = ENOMEM;

src/gp_export.c

@@ -308,10 +308,9 @@ static int gp_encrypt_buffer(krb5_context context, krb5_keyblock *key,
     ret = gp_conv_octet_string(enc_handle.ciphertext.length,
                                enc_handle.ciphertext.data,
                                out);
-    if (ret) {
-        free(enc_handle.ciphertext.data);
-        goto done;
-    }
+    /* the conversion function copies the data, so free our copy
+     * unconditionally, or we leak */
+    free(enc_handle.ciphertext.data);
 
 done:
     free(padded);