gss_krb5_import_creds can't work with memory keytab

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

gss_krb5_import_creds can't work with memory keytab

Andrew Bartlett
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.

However, we make extensive use of in-memory keytabs, and currently this
routine fails to 'reference' an existing in-memory keytab.  Instead,
these steps create a new, blank in-memory keytab:

        kret = krb5_kt_get_full_name(gssapi_krb5_context, keytab, &str);
        if (kret)
            goto out;

        kret = krb5_kt_resolve(gssapi_krb5_context, str, &handle->keytab);
        free(str);
        if (kret)
            goto out;

I see a few solutions:  We could copy the contents of the keytab (as
being 'unlikely to change', we could add a new function to 'reference' a
keytab (other than by get/resolve name), or 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.

Andrew Bartlett
--
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
Student Network Administrator, Hawker College  http://hawkerc.net

signature.asc (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: gss_krb5_import_creds can't work with memory keytab

Love Hörnquist Åstrand

Andrew Bartlett <[hidden email]> 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, "[hidden email]", &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);
 

attachment0 (487 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: gss_krb5_import_creds can't work with memory keytab

Andrew Bartlett
On Thu, 2005-12-01 at 09:20 +0100, Love Hörnquist Åstrand wrote:

> Andrew Bartlett <[hidden email]> 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 ?
It's my preferred option.  I'll test this out and let you know.

The fact that this was broken got me to implement the FILE keytab
support into the Samba4 credentials, which I needed anyway, so this
hasn't been to bad a thing :-)

Andrew Bartlett

--
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
Student Network Administrator, Hawker College  http://hawkerc.net

signature.asc (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: gss_krb5_import_creds can't work with memory keytab

Andrew Bartlett
On Thu, 2005-12-01 at 20:49 +1100, Andrew Bartlett wrote:

> On Thu, 2005-12-01 at 09:20 +0100, Love Hörnquist Åstrand wrote:
> > Andrew Bartlett <[hidden email]> 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 ?
>
> It's my preferred option.  I'll test this out and let you know.
This works, except that the reference count 'works' on a keytab of name
'MEMORY:'.  Perhaps this should be a special case that always makes a
new keytab?  I'm thinking an application may have relied on the previous
behaviour.  (early Samba3 I know used 'MEMORY:', but only ever had one
keytab).

Andrew Bartlett

--
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
Student Network Administrator, Hawker College  http://hawkerc.net

signature.asc (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: gss_krb5_import_creds can't work with memory keytab

Andrew Bartlett
On Thu, 2005-12-01 at 21:12 +1100, Andrew Bartlett wrote:

> On Thu, 2005-12-01 at 20:49 +1100, Andrew Bartlett wrote:
> > On Thu, 2005-12-01 at 09:20 +0100, Love Hörnquist Åstrand wrote:
> > > Andrew Bartlett <[hidden email]> 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 ?
> >
> > It's my preferred option.  I'll test this out and let you know.
>
> This works, except that the reference count 'works' on a keytab of name
> 'MEMORY:'.  Perhaps this should be a special case that always makes a
> new keytab?  I'm thinking an application may have relied on the previous
> behaviour.  (early Samba3 I know used 'MEMORY:', but only ever had one
> keytab).
One bug.  d->refcount is uninitialised (must be initialised to 1).

Andrew Bartlett

--
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
Student Network Administrator, Hawker College  http://hawkerc.net

keytab_memory2.patch (3K) Download Attachment
signature.asc (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: gss_krb5_import_creds can't work with memory keytab

Love Hörnquist Åstrand

Andrew Bartlett <[hidden email]> writes:

>> This works, except that the reference count 'works' on a keytab of name
>> 'MEMORY:'.  Perhaps this should be a special case that always makes a
>> new keytab?  I'm thinking an application may have relied on the previous
>> behaviour.  (early Samba3 I know used 'MEMORY:', but only ever had one
>> keytab).
>
> One bug.  d->refcount is uninitialised (must be initialised to 1).

There was some more bugs I found after lunch when I added more more
assertions and expanded the test-suite. I commited the code.

Love


attachment0 (487 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: gss_krb5_import_creds can't work with memory keytab

Love Hörnquist Åstrand
In reply to this post by Andrew Bartlett

Andrew Bartlett <[hidden email]> writes:

> This works, except that the reference count 'works' on a keytab of name
> 'MEMORY:'.  Perhaps this should be a special case that always makes a
> new keytab?  I'm thinking an application may have relied on the previous
> behaviour.  (early Samba3 I know used 'MEMORY:', but only ever had one
> keytab).

I think its ok that this changes. The only thing that happen is that two
keys can be added to the keytab, but if the consumer add two entries with
the same name, enctype and kvno, but the not same key.

Samba could be the only consumer I know about, and since it doesn't break
for you guys, I'll ignore it.

Love


attachment0 (487 bytes) Download Attachment