Re: gss_krb5_import_creds can't work with memory keytab

[Love Hörnquist Åstrand <lha@kth.se>]

Andrew Bartlett <abartlet@samba.org> writes:

> I've been trying to move Samba4 across to using the new
> gss_krb5_import_creds function.  This should reduce our custom hacks
> significantly, and I thought it provided the correct semantics.
[...]
> the code in keytab_memory.c
> could be changed to record the list of keytabs (with reference counting
> etc), much as the in-memory ccache code does.

I think I like reference counting better, how about this ?

Love

--- lib/krb5/keytab_memory.c	2005/05/18 04:44:40	1.6
+++ lib/krb5/keytab_memory.c	2005/12/01 08:18:03
@@ -40,19 +40,66 @@ RCSID("$Id: keytab_memory.c,v 1.6 2005/0
 struct mkt_data {
     krb5_keytab_entry *entries;
     int num_entries;
+    char *name;
+    int refcount;
+    struct mkt_data *next;
 };
 
+/* this mutex protects mkt_head, ->refcount, and ->next 
+ * content is not protected (name is static and need no protection)
+ */
+static HEIMDAL_MUTEX mkt_mutex = HEIMDAL_MUTEX_INITIALIZER;
+static struct mkt_data *mkt_head;
+
+
+static struct mkt_data *
+find_name(const char *name)
+{
+    struct mkt_data *d;
+
+    for (d = mkt_head; d != NULL; d = d->next)
+	if (strcmp(d->name, name) == 0)
+	    return d;
+    return NULL;
+}
+
+
 static krb5_error_code
 mkt_resolve(krb5_context context, const char *name, krb5_keytab id)
 {
     struct mkt_data *d;
+
+    HEIMDAL_MUTEX_lock(&mkt_mutex);
+
+    d = find_name(name);
+    if (d) {
+	if (d->refcount < 1)
+	    krb5_abortx(context, "krb5 internal error, "
+			"memory keytab refcount < 1");
+	d->refcount++;
+	id->data = d;
+	HEIMDAL_MUTEX_unlock(&mkt_mutex);
+	return 0;
+    }
+
     d = malloc(sizeof(*d));
     if(d == NULL) {
+	HEIMDAL_MUTEX_unlock(&mkt_mutex);
 	krb5_set_error_string (context, "malloc: out of memory");
 	return ENOMEM;
     }
+    d->name = strdup(name);
+    if (d->name == NULL) {
+	HEIMDAL_MUTEX_unlock(&mkt_mutex);
+	free(d);
+	krb5_set_error_string (context, "malloc: out of memory");
+	return ENOMEM;
+    }
     d->entries = NULL;
     d->num_entries = 0;
+    d->next = mkt_head;
+    mkt_head = d;
+    HEIMDAL_MUTEX_unlock(&mkt_mutex);
     id->data = d;
     return 0;
 }
@@ -60,8 +107,23 @@ mkt_resolve(krb5_context context, const 
 static krb5_error_code
 mkt_close(krb5_context context, krb5_keytab id)
 {
-    struct mkt_data *d = id->data;
+    struct mkt_data *d = id->data, **dp;
     int i;
+
+    HEIMDAL_MUTEX_lock(&mkt_mutex);
+    if (d->refcount-- > 0) {
+	HEIMDAL_MUTEX_unlock(&mkt_mutex);
+	return 0;
+    }
+    for (dp = &mkt_head; *dp != NULL; dp = &(*dp)->next) {
+	if (*dp == d) {
+	    *dp = d->next;
+	    break;
+	}
+    }
+    HEIMDAL_MUTEX_unlock(&mkt_mutex);
+
+    free(d->name);
     for(i = 0; i < d->num_entries; i++)
 	krb5_kt_free_entry(context, &d->entries[i]);
     free(d->entries);
@@ -75,7 +137,8 @@ mkt_get_name(krb5_context context, 
 	     char *name, 
 	     size_t namesize)
 {
-    strlcpy(name, "", namesize);
+    struct mkt_data *d = id->data;
+    strlcpy(name, d->name, namesize);
     return 0;
 }
 
--- lib/krb5/krb5_keytab.3	2005/08/12 13:32:11	1.20
+++ lib/krb5/krb5_keytab.3	2005/12/01 08:18:05
@@ -230,10 +230,11 @@ The residual part is a filename.
 The keytab is stored in a memory segment. This allows sensitive and/or
 temporary data not to be stored on disk. The type's name is
 .Li MEMORY .
-There are no residual part, the only pointer back to the keytab is the
-.Fa id
-returned by
-.Fn krb5_kt_resolve .
+Each
+.Li MEMORY
+keytab is referenced counted by and open by the residual name, so two
+handles can point to the same memory area.
+When the last user closes the entry, it disappears.
 .El
 .Pp
 .Nm krb5_keytab_entry
--- lib/krb5/test_keytab.c	2005/05/20 09:01:29	1.2
+++ lib/krb5/test_keytab.c	2005/12/01 08:18:07
@@ -60,6 +60,85 @@ test_empty_keytab(krb5_context context, 
 	krb5_err(context, 1, ret, "krb5_kt_close");
 }
 
+/*
+ * Test that memory keytab are refcounted.
+ */
+
+static void
+test_memory_keytab(krb5_context context, const char *keytab)
+{
+    krb5_error_code ret;
+    krb5_keytab id, id2;
+    krb5_keytab_entry entry, entry2;
+
+    ret = krb5_kt_resolve(context, keytab, &id);
+    if (ret)
+	krb5_err(context, 1, ret, "krb5_kt_resolve");
+
+    memset(&entry, 0, sizeof(entry));
+    ret = krb5_parse_name(context, "lha@SU.SE", &entry.principal);
+    if (ret)
+	krb5_err(context, 1, ret, "krb5_parse_name");
+    entry.vno = 1;
+    ret = krb5_generate_random_keyblock(context,
+					ETYPE_AES256_CTS_HMAC_SHA1_96,
+					&entry.keyblock);
+    if (ret)
+	krb5_err(context, 1, ret, "krb5_generate_random_keyblock");
+
+    krb5_kt_add_entry(context, id, &entry);
+
+    ret = krb5_kt_resolve(context, keytab, &id2);
+    if (ret)
+	krb5_err(context, 1, ret, "krb5_kt_resolve");
+
+    ret = krb5_kt_get_entry(context, id,
+			    entry.principal,
+			    0,
+			    ETYPE_AES256_CTS_HMAC_SHA1_96,
+			    &entry2);
+    if (ret)
+	krb5_err(context, 1, ret, "krb5_kt_get_entry");
+    krb5_kt_free_entry(context, &entry2);
+
+    ret = krb5_kt_close(context, id);
+    if (ret)
+	krb5_err(context, 1, ret, "krb5_kt_close");
+
+    ret = krb5_kt_get_entry(context, id2,
+			    entry.principal,
+			    0,
+			    ETYPE_AES256_CTS_HMAC_SHA1_96,
+			    &entry2);
+    if (ret)
+	krb5_err(context, 1, ret, "krb5_kt_get_entry");
+    krb5_kt_free_entry(context, &entry2);
+
+    ret = krb5_kt_close(context, id2);
+    if (ret)
+	krb5_err(context, 1, ret, "krb5_kt_close");
+
+
+
+    ret = krb5_kt_resolve(context, keytab, &id);
+    if (ret)
+	krb5_err(context, 1, ret, "krb5_kt_resolve");
+
+    ret = krb5_kt_close(context, id2);
+    if (ret)
+	krb5_err(context, 1, ret, "krb5_kt_close");
+
+    ret = krb5_kt_get_entry(context, id,
+			    entry.principal,
+			    0,
+			    ETYPE_AES256_CTS_HMAC_SHA1_96,
+			    &entry2);
+    if (ret == 0)
+	krb5_errx(context, 1, "krb5_kt_get_entry when if should fail");
+
+    krb5_kt_free_entry(context, &entry);
+}
+
 int
 main(int argc, char **argv)
 {
@@ -75,6 +154,8 @@ main(int argc, char **argv)
     test_empty_keytab(context, "MEMORY:foo");
     test_empty_keytab(context, "FILE:foo");
     test_empty_keytab(context, "KRB4:foo");
+
+    test_memory_keytab(context, "MEMORY:foo");
 
     krb5_free_context(context);

PGP signature