2013-10-22 14:22:43

by Andy Adamson

[permalink] [raw]
Subject: [PATCH Version 2 0/3] GSSD: Use gss-ctx keys and gsskeyd to sync Kerberos credentials and kernel gss_contexts.

From: Andy Adamson <[email protected]>

This is an RFC patchset, which will be used for testing.

This patchset requires the "SUNRPC: destroy gss_cred and context on Kerberos credential destruction" kernel patchset.

We need to do a lot of testing to ensure that once kdestroy and gss-ctx
gss_user_destroy is called, all existing buffered
writes using the 'destroyed gss credential + context' are serviced.

Differences from version 1:

- moved from nfstgt_login and nfstgt_logout to gsskeyd.
- gsskeyd automatically creates gss-ctx key on kinit and destroys the gss-ctx
key on kdestroy.

gsskeyd will need to act differently for different krb5 credential caches.
For example, some versions of gssd store FILE credentials in FILE:/tmp/krb5cc_<UID>
while this code, written for fedora 19 uses FILE:/run/user/<UID>/krb5cc/tgt.

As Trond suggested, if we keep gsskeyd separate from gssd, we could set up a
configuration file along the lines of the keytools' request-key.conf file to
allow both NFS and CIFS (and other filesystems) to install plugin handlers
for kinit/kdestroy events.

Else, we could have gssd be the process to poll inotify (given
that it already polls rpc_pipefs) and then just have it fork off the
subprocess if and when it sees an interesting event.

We need to investigate how this works when the kernel keyring is used for
Kerberos credentials. I believe that in this case gsskeyd can add the gss-ctx
key to the kerberos keyring, and it will get destroyed along with all other
keys at kdestroy.

Andy Adamson (2):
GSSD add cc_name to upcall
ANDROS: update gsskeyd to use new /run/user/UID/krb5cc/tgt cache file

Weston Andros Adamson (1):
WIP: Add gsskeyd

configure.ac | 1 +
utils/Makefile.am | 2 +-
utils/gssd/gssd_proc.c | 37 ++++-
utils/gssd/krb5_util.c | 2 +-
utils/gssd/krb5_util.h | 1 +
utils/gsskeyd/gsskeyd.c | 371 ++++++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 408 insertions(+), 6 deletions(-)
create mode 100644 utils/gsskeyd/gsskeyd.c

--
1.8.3.1



2013-10-22 14:22:45

by Andy Adamson

[permalink] [raw]
Subject: [PATCH Version 2 2/3] WIP: Add gsskeyd

From: Weston Andros Adamson <[email protected]>

this is an experimental daemon that watches for krb credential cache files
being created and deleted and creates or removes the mapping in the user
keyring.

this replaces the wrappers for kinit and kdestroy as they are no longer needed -
users can go ahead using kinit/kdestroy and existing pam modules.

if this is the way we want to go, this should probably end up as a processes
forked from gssd and not its own daemon.

Signed-off-by: Weston Andros Adamson <[email protected]>
---
configure.ac | 1 +
utils/Makefile.am | 2 +-
utils/gsskeyd/gsskeyd.c | 328 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 330 insertions(+), 1 deletion(-)
create mode 100644 utils/gsskeyd/gsskeyd.c

diff --git a/configure.ac b/configure.ac
index d3ad854..a8e75ac 100644
--- a/configure.ac
+++ b/configure.ac
@@ -497,6 +497,7 @@ AC_CONFIG_FILES([
utils/nfsdcltrack/Makefile
utils/exportfs/Makefile
utils/gssd/Makefile
+ utils/gsskeyd/Makefile
utils/idmapd/Makefile
utils/mount/Makefile
utils/mountd/Makefile
diff --git a/utils/Makefile.am b/utils/Makefile.am
index b892dc8..943ae2d 100644
--- a/utils/Makefile.am
+++ b/utils/Makefile.am
@@ -14,7 +14,7 @@ OPTDIRS += blkmapd
endif

if CONFIG_GSS
-OPTDIRS += gssd
+OPTDIRS += gssd gsskeyd
endif

if CONFIG_MOUNT
diff --git a/utils/gsskeyd/gsskeyd.c b/utils/gsskeyd/gsskeyd.c
new file mode 100644
index 0000000..6d598fa
--- /dev/null
+++ b/utils/gsskeyd/gsskeyd.c
@@ -0,0 +1,328 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <limits.h>
+#include <assert.h>
+#include <keyutils.h>
+#include <err.h>
+#include <errno.h>
+#include <string.h>
+
+#include <sys/inotify.h>
+#include <sys/errno.h>
+#include <sys/select.h>
+#include <sys/param.h>
+#include <sys/time.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <sys/queue.h>
+#include <sys/uio.h>
+#include <unistd.h>
+
+#define MAX_EVENT_SIZE (sizeof(struct inotify_event) + NAME_MAX + 1)
+
+//#define DEBUG_TRACE
+
+#ifdef DEBUG_TRACE
+#define TRACE(X...) do { fprintf(stderr, "_trace_: " X); } while (0)
+#else
+#define TRACE(X...)
+#endif
+
+/* .25 second timeouts to select */
+#define SELECT_INTERVAL_TV { 0, 250000 }
+
+/* .5 seconds to collect list of unique file names that had events */
+#define COLLECT_INTERVAL_TV { 0, 500000 }
+
+struct watchdir {
+ char dir[MAXPATHLEN];
+ int wd;
+ LIST_ENTRY(watchdir) entry;
+};
+
+LIST_HEAD(watchdir_list, watchdir);
+
+struct update {
+ char dir[MAXPATHLEN];
+ char name[MAXPATHLEN];
+ int is_remove;
+ LIST_ENTRY(update) entry;
+};
+
+LIST_HEAD(update_list, update);
+
+struct updates {
+ struct update_list list;
+ struct timeval first;
+};
+
+void
+watchdir_add(struct watchdir_list *watchdirs, int notify, char *dir)
+{
+ struct watchdir *w;
+
+ w = calloc(1, sizeof(struct watchdir));
+ assert(w != NULL);
+
+ strncpy(w->dir, dir, MAXPATHLEN);
+
+ w->wd = inotify_add_watch(notify, dir,
+ IN_CREATE | IN_DELETE | IN_MOVED_FROM | IN_MOVED_TO );
+ //| IN_MODIFY
+
+ if (w->wd < 0)
+ perror("inotify_add_watch");
+
+ LIST_INSERT_HEAD(watchdirs, w, entry);
+
+ TRACE("WATCH(%d): %s\n", w->wd, w->dir);
+}
+
+/*
+ * XXX maybe use hash when there is more than one watchdir?
+ */
+struct watchdir *
+watchdir_lookup(struct watchdir_list *watchdirs, int wd)
+{
+ struct watchdir *w;
+
+ LIST_FOREACH(w, watchdirs, entry) {
+ if (wd == w->wd)
+ return w;
+ }
+
+ return NULL;
+}
+
+static void
+update_set_type(struct update *u, struct inotify_event *e)
+{
+ if (e->mask & (IN_DELETE | IN_MOVED_FROM))
+ u->is_remove = 1;
+ else
+ u->is_remove = 0;
+}
+
+void
+updates_add(struct updates *updates, struct watchdir_list *watchdirs, struct inotify_event *e)
+{
+ struct update *u;
+ struct watchdir *w;
+
+
+ w = watchdir_lookup(watchdirs, e->wd);
+ assert(w != NULL);
+
+ TRACE("updates_add: %s/%s\n", w->dir, e->name);
+
+ LIST_FOREACH(u, &updates->list, entry) {
+ if (strncmp(w->dir, u->dir, MAXPATHLEN) == 0 &&
+ strncmp(e->name, u->name, MAXPATHLEN) == 0) {
+ update_set_type(u, e);
+ return;
+ }
+ }
+
+ u = calloc(1, sizeof(struct update));
+ strncpy(u->dir, w->dir, MAXPATHLEN);
+ strncpy(u->name, e->name, MAXPATHLEN);
+ update_set_type(u, e);
+
+ LIST_INSERT_HEAD(&updates->list, u, entry);
+
+ if (!timerisset(&updates->first)) {
+ if (gettimeofday(&updates->first, NULL) < 0)
+ perror("gettimeofday");
+ }
+}
+
+void
+print_event_mask(FILE *fp, uint32_t mask)
+{
+#define _print_if_found(X, S) do { \
+ if (mask & (X)) { \
+ fprintf(fp, S); \
+ } \
+ } while(0)
+
+ _print_if_found(IN_ACCESS, "access");
+ _print_if_found(IN_ATTRIB, "attrib");
+ _print_if_found(IN_CLOSE_WRITE, "close write");
+ _print_if_found(IN_CLOSE_NOWRITE, "close nowrite");
+ _print_if_found(IN_CREATE, "create");
+ _print_if_found(IN_DELETE, "delete");
+ _print_if_found(IN_DELETE_SELF, "delete self");
+ _print_if_found(IN_MODIFY, "modify");
+ _print_if_found(IN_MOVE_SELF, "move self");
+ _print_if_found(IN_MOVED_FROM, "moved from");
+ _print_if_found(IN_MOVED_TO, "moved to");
+ _print_if_found(IN_OPEN, "open");
+}
+
+static int
+manage_gss_ctx_keyring_mapping(struct update *u)
+{
+ char buf[MAXPATHLEN];
+ int status;
+ uid_t uid;
+ pid_t pid;
+ key_serial_t key;
+ long res;
+
+ snprintf(buf, MAXPATHLEN, "%s/%s", u->dir, u->name);
+
+ if (sscanf(u->name, "krb5cc_%u", &uid) <= 0)
+ perror("parsing krb5cc uid");
+
+ if ((pid = fork()) < 0)
+ perror("fork");
+
+ if (pid == 0) {
+ if (setuid(uid) < 0)
+ perror("setuid");
+
+ if (u->is_remove) {
+ fprintf(stderr, "remove gss ctx mapping for uid %u\n",
+ uid);
+ key = request_key("gss-ctx", "_nfstgt_", NULL,
+ KEY_SPEC_USER_KEYRING);
+ if (key > 0) {
+ res = keyctl_unlink(key, KEY_SPEC_USER_KEYRING);
+ if (res < 0)
+ warn("keyctl_unlink failed");
+ } else
+ warn("request_key failed");
+ } else {
+ fprintf(stderr, "add gss ctx mapping for uid=%u\n",
+ uid);
+ snprintf(buf, MAXPATHLEN, "FILE:%s/%s",
+ u->dir, u->name);
+ key = add_key("gss-ctx", "_nfstgt_", buf,
+ MAXPATHLEN, KEY_SPEC_USER_KEYRING);
+
+ if (key < 0)
+ warn("add_key failed");
+ }
+
+ exit(0);
+ } else {
+ waitpid(pid, &status, 0);
+
+ if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
+ return WEXITSTATUS(status);
+ }
+ }
+
+ return 0;
+}
+
+void
+parse_events(int notify, struct watchdir_list *watchdirs, struct updates *updates)
+{
+ ssize_t nread, pos;
+ unsigned char buf[MAX_EVENT_SIZE * 100];
+ struct inotify_event *e;
+
+ for (;;) {
+ nread = read(notify, buf, sizeof(buf));
+
+ if (nread <= 0) {
+ TRACE("read returns %d, errno %u\n", nread, errno);
+ break;
+ }
+
+ TRACE("Read %d bytes\n", nread);
+
+ pos = 0;
+ while (pos < nread) {
+ assert((nread - pos) >=
+ (ssize_t)sizeof(struct inotify_event));
+ e = (struct inotify_event *)(buf + pos);
+ pos += sizeof(struct inotify_event) + e->len;
+
+#ifdef DEBUG_TRACE
+ TRACE("EVENT on %s: ", e->name);
+ print_event_mask(stderr, e->mask);
+ fprintf(stderr, "\n");
+#endif
+
+ if (strlen(e->name) >= strlen("krb5cc_") &&
+ strncmp("krb5cc_", e->name, strlen("krb5cc_")) != 0)
+ TRACE("skip file: %s\n", e->name);
+ else
+ updates_add(updates, watchdirs, e);
+ }
+ assert(pos == nread);
+ }
+}
+
+void
+handle_events(struct updates *updates)
+{
+ struct update *u;
+ struct timeval now, diff, collection_interval = COLLECT_INTERVAL_TV;
+
+ if (!timerisset(&updates->first))
+ return;
+
+ if (gettimeofday(&now, NULL) < 0)
+ perror("gettimeofday");
+
+ timersub(&now, &updates->first, &diff);
+
+ if (!(diff.tv_sec >= collection_interval.tv_sec &&
+ diff.tv_usec >= collection_interval.tv_usec)) {
+ TRACE("it's been %u s %u ms - waiting longer...\n",
+ diff.tv_sec, diff.tv_usec);
+ return;
+ }
+
+ while (!LIST_EMPTY(&updates->list)) {
+ u = updates->list.lh_first;
+
+ TRACE("handle krb5 cc file: %s\n", u->name);
+ manage_gss_ctx_keyring_mapping(u);
+ LIST_REMOVE(u, entry);
+ }
+
+ timerclear(&updates->first);
+}
+
+
+int
+main(int argc, char **argv)
+{
+ fd_set readfds;
+ int notify;
+ int res;
+ struct timeval timeo, select_interval = SELECT_INTERVAL_TV;
+ struct watchdir_list watchdirs;
+ struct updates updates;
+
+ LIST_INIT(&watchdirs);
+
+ notify = inotify_init1(IN_NONBLOCK);
+ if (notify < 0)
+ perror("inotify_init1");
+
+ watchdir_add(&watchdirs, notify, "/tmp");
+
+ LIST_INIT(&updates.list);
+ timerclear(&updates.first);
+
+ for (;;) {
+ FD_ZERO(&readfds);
+ FD_SET(notify, &readfds);
+ memcpy(&timeo, &select_interval, sizeof(struct timeval));
+
+ res = select(notify + 1, &readfds, NULL, NULL, &timeo);
+
+ if (res < 0)
+ perror("error calling select");
+
+ if (FD_ISSET(notify, &readfds))
+ parse_events(notify, &watchdirs, &updates);
+
+ handle_events(&updates);
+ }
+}
--
1.8.3.1


2013-10-22 17:00:02

by Adamson, Andy

[permalink] [raw]
Subject: Re: [PATCH Version 2 0/3] GSSD: Use gss-ctx keys and gsskeyd to sync Kerberos credentials and kernel gss_contexts.


On Oct 22, 2013, at 12:09 PM, Simo Sorce <[email protected]>
wrote:

> On Tue, 2013-10-22 at 15:32 +0000, Adamson, Andy wrote:
>> On Oct 22, 2013, at 11:02 AM, Simo Sorce <[email protected]> wrote:
>>
>>> On Tue, 2013-10-22 at 10:22 -0400, [email protected] wrote:
>>>> From: Andy Adamson <[email protected]>
>>>>
>>>> This is an RFC patchset, which will be used for testing.
>>>>
>>>> This patchset requires the "SUNRPC: destroy gss_cred and context on Kerberos credential destruction" kernel patchset.
>>>>
>>>> We need to do a lot of testing to ensure that once kdestroy and gss-ctx
>>>> gss_user_destroy is called, all existing buffered
>>>> writes using the 'destroyed gss credential + context' are serviced.
>>>>
>>>> Differences from version 1:
>>>>
>>>> - moved from nfstgt_login and nfstgt_logout to gsskeyd.
>>>> - gsskeyd automatically creates gss-ctx key on kinit and destroys the gss-ctx
>>>> key on kdestroy.
>>>>
>>>> gsskeyd will need to act differently for different krb5 credential caches.
>>>
>>> Why ?
>>
>> Just need different parsing to know which directory to poll.
>>>
>>>> For example, some versions of gssd store FILE credentials in FILE:/tmp/krb5cc_<UID>
>>>> while this code, written for fedora 19 uses FILE:/run/user/<UID>/krb5cc/tgt.
>>>
>>> This is incorrect, Fedora 19 use DIR type ccaches, so you should
>>> reference a cache withing the dir type, which will notmally be something
>>> like: DIR:/run/user/<uidnumber>/krb5cc for the whole collection or
>>> DIR::/run/user/<uidnumber>/krb5cc/txtXXXXXX for the sepcific cache.
>>>
>>> However note that due to issues with DIR type caches we are moving to a
>>> new KEYRING based type of cache in F20, so assuming any specific type of
>>> cache is a dead start.
>>
>> But I'm not assuming any kind of kerberos credential cache! I just coded to one type for this RFC.
>> Didn't you see my KEYRING based ccache comment below?
>
> Ah sorry, I had not noticed.
>
>>>> As Trond suggested, if we keep gsskeyd separate from gssd, we could set up a
>>>> configuration file along the lines of the keytools' request-key.conf file to
>>>> allow both NFS and CIFS (and other filesystems) to install plugin handlers
>>>> for kinit/kdestroy events.
>>>
>>> Given in many cases (the desirable ones at least) kerberos credentials
>>> are not inited via kinit, but rather by things like pam_krb5, sssd, or
>>> directly imported via sshd, I am trying to understand how you are going
>>> to address the majority of these cases.
>>
>> All of these cases use a kerberos credential cache which if is of type
>> FILE the gsskeyd can poll inotify and if it is of type KEY: then
>> gsskeyd adds the gss-ctx key to the Kerberos keyring (that is the
>> theory :)) so the same code services kinit/pam_krb5
>
> You wouldn't be able to see it if I set a custom ccache file though
> export KRB5CCNAME=FILE:/random/sir/foofile
>
>>> Should users put gsskeyd in their .profile/.bashrc files or something ?
>>>
>>>> Else, we could have gssd be the process to poll inotify (given
>>>> that it already polls rpc_pipefs) and then just have it fork off the
>>>> subprocess if and when it sees an interesting event.
>>>
>>> What is an 'interesting' event ?
>>
>> TGT (used for NFS TGS) creation/destruction
>
> Shouldn't you care for the specific NFS ticket only ?
> Although kdestroy is quite blunt, in theory users can simply destroy the
> specific NFS ticket and keep their TGT.

AFAICS MIT kdestroy does not let you specifiy which credentials to destroy. It simply destroys the whole specified ccache, TGT and all.

I think Heimdal kdestroy lets you specify...

WRT NFS: As long as the TGT has not expired, the kernel will upcall and GSSD will get a new TGS. So, the kernel gss_context lifetime == TGT lifetime.

The solution with the current MIT kdestroy is to have a NFS-only kerberos credentail cache - with it's own TGT and only NFS TGS's.

-->Andy



>
> Also I see a problem with this approach. In order to make user life
> easier we reinit ccaches in sssd when a user for example unlocks the
> screen. If you track a specific TGT or NFS ticket, this would cause the
> immediate destruction of credentials in the nfs client which will also
> kill access to the NFS server. I think this would cause quite some grief
> to users.
>
>>>> We need to investigate how this works when the kernel keyring is used for
>>>> Kerberos credentials. I believe that in this case gsskeyd can add the gss-ctx
>>>> key to the kerberos keyring, and it will get destroyed along with all other
>>>> keys at kdestroy.
>>>
>>> Is there any reason why you are doing this work with an utility that is
>>> separate from gssd or libkrb5 ?
>>
>> Yes - to allow both NFS and CIFS (and other filesystems) to install
>> plugin handlers for kerberos credential cache creation/destruction
>> events.
>>
>> But, having gssd do it is fine, or having a client plugin in libkrb5
>> is also fine.
>
>>>
>>> I think the only sensible way to handle something like this is by adding
>>> support directly in libkrb5, I do not see something external ever
>>> working reliably.
>>
>> I agree that I should persue a libkrb5 approach.
>
> Keep in mind the point above though.
> The issues here is that you do not know what the intention was if you
> try a completely automated approach. you should probably interact with
> the login service and act conditionally in any way.
>
> I think it may be safer for you to provide a hook that the system can
> call when a user fully logs out. And use that hook to kill any
> credential associated to a particular UID.
>
> Done this way you do not need to poll anything nor change libkrb5. And
> you also allow the creation of a tool that the user can run to force the
> destruction of his credentials if he is not using a compliant login
> manager. Or if he wants to explicitly kill NFS creds in the kernel for
> whatever reason.
>
>>> I am not against it for testing purposes, but then does it need to be
>>> committed to nfs-utils ?
>>
>> Did you not see the "This is an RFC patchset"? I'm not asking for a commit to nfs-utils, but "request for comments" which you just did! Thanks :)
>
> Yes I was providing feedback :-)
>
> Simo.
>
> --
> Simo Sorce * Red Hat, Inc * New York
>


2013-10-22 15:07:42

by Simo Sorce

[permalink] [raw]
Subject: Re: [PATCH Version 2 1/3] GSSD add cc_name to upcall

On Tue, 2013-10-22 at 10:22 -0400, [email protected] wrote:
> From: Andy Adamson <[email protected]>
>
> Signed-off-by: Andy Adamson <[email protected]>
> ---
> utils/gssd/gssd_proc.c | 37 +++++++++++++++++++++++++++++++++----
> utils/gssd/krb5_util.c | 2 +-
> utils/gssd/krb5_util.h | 1 +
> 3 files changed, 35 insertions(+), 5 deletions(-)
>
> diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
> index 2d3dbec..8df61a4 100644
> --- a/utils/gssd/gssd_proc.c
> +++ b/utils/gssd/gssd_proc.c
> @@ -966,7 +966,7 @@ create_auth_rpc_client(struct clnt_info *clp,
> */
> static void
> process_krb5_upcall(struct clnt_info *clp, uid_t uid, int fd, char *tgtname,
> - char *service)
> + char *service, char *cc_name)
> {
> CLIENT *rpc_clnt = NULL;
> AUTH *auth = NULL;
> @@ -980,7 +980,8 @@ process_krb5_upcall(struct clnt_info *clp, uid_t uid, int fd, char *tgtname,
> gss_cred_id_t gss_cred;
> OM_uint32 maj_stat, min_stat, lifetime_rec;
>
> - printerr(1, "handling krb5 upcall (%s)\n", clp->dirname);
> + printerr(1, "handling krb5 upcall (%s) cc_name %p\n", clp->dirname,
> + cc_name);
>
> token.length = 0;
> token.value = NULL;
> @@ -1011,6 +1012,18 @@ process_krb5_upcall(struct clnt_info *clp, uid_t uid, int fd, char *tgtname,
> service ? service : "<null>");
> if (uid != 0 || (uid == 0 && root_uses_machine_creds == 0 &&
> service == NULL)) {
> + /* Use the ccache name from the upcall */
> + if (cc_name != NULL) {
> + printerr(2, "using %s as credentials cache for client "
> + "with uid %u for server %s\n", cc_name,
> + uid, clp->servername);
> + gssd_set_krb5_ccache_name(cc_name);
> + create_resp = create_auth_rpc_client(clp,
> + &rpc_clnt, &auth, uid,
> + AUTHTYPE_KRB5, gss_cred);
> + if (create_resp == 0)
> + goto resp_found;
> + }

Please don't do this it will break gss-proxy and the impersonation
feature. The next call *must* be the first and not direct krb5 calls
must happen before it.

Simo.

> /* Tell krb5 gss which credentials cache to use */
> /* Try first to acquire credentials directly via GSSAPI */
> err = gssd_acquire_user_cred(uid, &gss_cred);
> @@ -1083,6 +1096,7 @@ process_krb5_upcall(struct clnt_info *clp, uid_t uid, int fd, char *tgtname,
> }
> }
>
> +resp_found:
> if (!authgss_get_private_data(auth, &pd)) {
> printerr(1, "WARNING: Failed to obtain authentication "
> "data for user with uid %d for server %s\n",
> @@ -1137,7 +1151,7 @@ handle_krb5_upcall(struct clnt_info *clp)
> return;
> }
>
> - process_krb5_upcall(clp, uid, clp->krb5_fd, NULL, NULL);
> + process_krb5_upcall(clp, uid, clp->krb5_fd, NULL, NULL, NULL);
> }
>
> void
> @@ -1151,6 +1165,7 @@ handle_gssd_upcall(struct clnt_info *clp)
> char *target = NULL;
> char *service = NULL;
> char *enctypes = NULL;
> + char *cc_name = NULL;
>
> printerr(1, "handling gssd upcall (%s)\n", clp->dirname);
>
> @@ -1245,9 +1260,23 @@ handle_gssd_upcall(struct clnt_info *clp)
> goto out;
> }
> }
> + /* read the ccache name. */
> + if ((p = strstr(lbuf, "ccache=")) != NULL) {
> + printerr(2, "CC_NAME to parse\n");
> + cc_name = malloc(lbuflen);
> + if (!cc_name)
> + goto out;
> + if (sscanf(p, "ccache=%s", cc_name) != 1) {
> + printerr(2, "WARNING: handle_gssd_upcall: "
> + "failed to parse cc_name "
> + "in upcall string '%s'\n", lbuf);
> + goto out;
> + }
> + }
>
> if (strcmp(mech, "krb5") == 0)
> - process_krb5_upcall(clp, uid, clp->gssd_fd, target, service);
> + process_krb5_upcall(clp, uid, clp->gssd_fd, target, service,
> + cc_name);
> else
> printerr(0, "WARNING: handle_gssd_upcall: "
> "received unknown gss mech '%s'\n", mech);
> diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
> index 83b9651..1bb0da6 100644
> --- a/utils/gssd/krb5_util.c
> +++ b/utils/gssd/krb5_util.c
> @@ -471,7 +471,7 @@ gssd_get_single_krb5_cred(krb5_context context,
> * Depending on the version of Kerberos, we either need to use
> * a private function, or simply set the environment variable.
> */
> -static void
> +void
> gssd_set_krb5_ccache_name(char *ccname)
> {
> #ifdef USE_GSS_KRB5_CCACHE_NAME
> diff --git a/utils/gssd/krb5_util.h b/utils/gssd/krb5_util.h
> index eed1294..16119a8 100644
> --- a/utils/gssd/krb5_util.h
> +++ b/utils/gssd/krb5_util.h
> @@ -23,6 +23,7 @@ struct gssd_k5_kt_princ {
> };
>
>
> +void gssd_set_krb5_ccache_name(char *ccname);
> int gssd_setup_krb5_user_gss_ccache(uid_t uid, char *servername,
> char *dirname);
> int gssd_get_krb5_machine_cred_list(char ***list);


--
Simo Sorce * Red Hat, Inc * New York


2013-10-22 17:25:23

by Simo Sorce

[permalink] [raw]
Subject: Re: [PATCH Version 2 0/3] GSSD: Use gss-ctx keys and gsskeyd to sync Kerberos credentials and kernel gss_contexts.

On Tue, 2013-10-22 at 17:00 +0000, Adamson, Andy wrote:
> On Oct 22, 2013, at 12:09 PM, Simo Sorce <[email protected]>
> wrote:
>
> > On Tue, 2013-10-22 at 15:32 +0000, Adamson, Andy wrote:
> >> On Oct 22, 2013, at 11:02 AM, Simo Sorce <[email protected]> wrote:
> >>
> >>> On Tue, 2013-10-22 at 10:22 -0400, [email protected] wrote:
> >>>> From: Andy Adamson <[email protected]>
> >>>>
> >>>> This is an RFC patchset, which will be used for testing.
> >>>>
> >>>> This patchset requires the "SUNRPC: destroy gss_cred and context on Kerberos credential destruction" kernel patchset.
> >>>>
> >>>> We need to do a lot of testing to ensure that once kdestroy and gss-ctx
> >>>> gss_user_destroy is called, all existing buffered
> >>>> writes using the 'destroyed gss credential + context' are serviced.
> >>>>
> >>>> Differences from version 1:
> >>>>
> >>>> - moved from nfstgt_login and nfstgt_logout to gsskeyd.
> >>>> - gsskeyd automatically creates gss-ctx key on kinit and destroys the gss-ctx
> >>>> key on kdestroy.
> >>>>
> >>>> gsskeyd will need to act differently for different krb5 credential caches.
> >>>
> >>> Why ?
> >>
> >> Just need different parsing to know which directory to poll.
> >>>
> >>>> For example, some versions of gssd store FILE credentials in FILE:/tmp/krb5cc_<UID>
> >>>> while this code, written for fedora 19 uses FILE:/run/user/<UID>/krb5cc/tgt.
> >>>
> >>> This is incorrect, Fedora 19 use DIR type ccaches, so you should
> >>> reference a cache withing the dir type, which will notmally be something
> >>> like: DIR:/run/user/<uidnumber>/krb5cc for the whole collection or
> >>> DIR::/run/user/<uidnumber>/krb5cc/txtXXXXXX for the sepcific cache.
> >>>
> >>> However note that due to issues with DIR type caches we are moving to a
> >>> new KEYRING based type of cache in F20, so assuming any specific type of
> >>> cache is a dead start.
> >>
> >> But I'm not assuming any kind of kerberos credential cache! I just coded to one type for this RFC.
> >> Didn't you see my KEYRING based ccache comment below?
> >
> > Ah sorry, I had not noticed.
> >
> >>>> As Trond suggested, if we keep gsskeyd separate from gssd, we could set up a
> >>>> configuration file along the lines of the keytools' request-key.conf file to
> >>>> allow both NFS and CIFS (and other filesystems) to install plugin handlers
> >>>> for kinit/kdestroy events.
> >>>
> >>> Given in many cases (the desirable ones at least) kerberos credentials
> >>> are not inited via kinit, but rather by things like pam_krb5, sssd, or
> >>> directly imported via sshd, I am trying to understand how you are going
> >>> to address the majority of these cases.
> >>
> >> All of these cases use a kerberos credential cache which if is of type
> >> FILE the gsskeyd can poll inotify and if it is of type KEY: then
> >> gsskeyd adds the gss-ctx key to the Kerberos keyring (that is the
> >> theory :)) so the same code services kinit/pam_krb5
> >
> > You wouldn't be able to see it if I set a custom ccache file though
> > export KRB5CCNAME=FILE:/random/sir/foofile
> >
> >>> Should users put gsskeyd in their .profile/.bashrc files or something ?
> >>>
> >>>> Else, we could have gssd be the process to poll inotify (given
> >>>> that it already polls rpc_pipefs) and then just have it fork off the
> >>>> subprocess if and when it sees an interesting event.
> >>>
> >>> What is an 'interesting' event ?
> >>
> >> TGT (used for NFS TGS) creation/destruction
> >
> > Shouldn't you care for the specific NFS ticket only ?
> > Although kdestroy is quite blunt, in theory users can simply destroy the
> > specific NFS ticket and keep their TGT.
>
> AFAICS MIT kdestroy does not let you specifiy which credentials to
> destroy. It simply destroys the whole specified ccache, TGT and all.
>
> I think Heimdal kdestroy lets you specify...
>
> WRT NFS: As long as the TGT has not expired, the kernel will upcall
> and GSSD will get a new TGS. So, the kernel gss_context lifetime ==
> TGT lifetime.
>
> The solution with the current MIT kdestroy is to have a NFS-only
> kerberos credentail cache - with it's own TGT and only NFS TGS's.

There isn't just kdestroy, applications can use libkrb5 to manipulate
the ccache if needed. I am just pointing out that there are many ways
ccaches are manipulated in real life.

I think it is valuable to be able to invalidate the credentials in the
kernel when the user really wants it, I just think that guessing when
the user/admin wants you to drop the creds based on ccaches interactions
is probably not going to work very well.

So the way I see it you probably want to keep the tracking in whatever
tool you want to use to experiment (say gsskeyd) and only provide a
downcall to the kernel that will tell it: destroy any cache for 'uid
number (optionally pass in a session id too ?)'.

This way you can replace the logic of how to keep track of what is going
on completely in user space, where it can easily be adjusted and adapted
as experience is gained.

IE: track it completely in userspace for now and only provide a syscall
to kill creds per UID, no tracking on the kernel side.

Simo.

--
Simo Sorce * Red Hat, Inc * New York


2013-10-22 15:46:27

by Weston Andros Adamson

[permalink] [raw]
Subject: Re: [PATCH Version 2 0/3] GSSD: Use gss-ctx keys and gsskeyd to sync Kerberos credentials and kernel gss_contexts.


> On Oct 22, 2013, at 11:02 AM, "Simo Sorce" <[email protected]> wrote:
>
>> On Tue, 2013-10-22 at 10:22 -0400, [email protected] wrote:
>> From: Andy Adamson <[email protected]>
>>
>> This is an RFC patchset, which will be used for testing.
>>
>> This patchset requires the "SUNRPC: destroy gss_cred and context on Kerberos credential destruction" kernel patchset.
>>
>> We need to do a lot of testing to ensure that once kdestroy and gss-ctx
>> gss_user_destroy is called, all existing buffered
>> writes using the 'destroyed gss credential + context' are serviced.
>>
>> Differences from version 1:
>>
>> - moved from nfstgt_login and nfstgt_logout to gsskeyd.
>> - gsskeyd automatically creates gss-ctx key on kinit and destroys the gss-ctx
>> key on kdestroy.
>>
>> gsskeyd will need to act differently for different krb5 credential caches.
>
> Why ?
>
>> For example, some versions of gssd store FILE credentials in FILE:/tmp/krb5cc_<UID>
>> while this code, written for fedora 19 uses FILE:/run/user/<UID>/krb5cc/tgt.
>
> This is incorrect, Fedora 19 use DIR type ccaches, so you should
> reference a cache withing the dir type, which will notmally be something
> like: DIR:/run/user/<uidnumber>/krb5cc for the whole collection or
> DIR::/run/user/<uidnumber>/krb5cc/txtXXXXXX for the sepcific cache.
>
> However note that due to issues with DIR type caches we are moving to a
> new KEYRING based type of cache in F20, so assuming any specific type of
> cache is a dead start.
>
>> As Trond suggested, if we keep gsskeyd separate from gssd, we could set up a
>> configuration file along the lines of the keytools' request-key.conf file to
>> allow both NFS and CIFS (and other filesystems) to install plugin handlers
>> for kinit/kdestroy events.
>
> Given in many cases (the desirable ones at least) kerberos credentials
> are not inited via kinit, but rather by things like pam_krb5, sssd, or
> directly imported via sshd, I am trying to understand how you are going
> to address the majority of these cases.
>
> Should users put gsskeyd in their .profile/.bashrc files or something ?
>
>> Else, we could have gssd be the process to poll inotify (given
>> that it already polls rpc_pipefs) and then just have it fork off the
>> subprocess if and when it sees an interesting event.
>
> What is an 'interesting' event ?
>
>> We need to investigate how this works when the kernel keyring is used for
>> Kerberos credentials. I believe that in this case gsskeyd can add the gss-ctx
>> key to the kerberos keyring, and it will get destroyed along with all other
>> keys at kdestroy.
>
> Is there any reason why you are doing this work with an utility that is
> separate from gssd or libkrb5 ?
>

gsskeyd is a separate daemon only for proof of concept. In the commit message it makes it clear that if this is the way we want to go, it should be incorporated into gssd.

-dros

> I think the only sensible way to handle something like this is by adding
> support directly in libkrb5, I do not see something external ever
> working reliably.
> I am not against it for testing purposes, but then does it need to be
> committed to nfs-utils ?
>
> Simo.
>
>> Andy Adamson (2):
>> GSSD add cc_name to upcall
>> ANDROS: update gsskeyd to use new /run/user/UID/krb5cc/tgt cache file
>>
>> Weston Andros Adamson (1):
>> WIP: Add gsskeyd
>>
>> configure.ac | 1 +
>> utils/Makefile.am | 2 +-
>> utils/gssd/gssd_proc.c | 37 ++++-
>> utils/gssd/krb5_util.c | 2 +-
>> utils/gssd/krb5_util.h | 1 +
>> utils/gsskeyd/gsskeyd.c | 371 ++++++++++++++++++++++++++++++++++++++++++++++++
>> 6 files changed, 408 insertions(+), 6 deletions(-)
>> create mode 100644 utils/gsskeyd/gsskeyd.c
>
>
> --
> Simo Sorce * Red Hat, Inc * New York
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2013-10-22 15:32:18

by Adamson, Andy

[permalink] [raw]
Subject: Re: [PATCH Version 2 0/3] GSSD: Use gss-ctx keys and gsskeyd to sync Kerberos credentials and kernel gss_contexts.


On Oct 22, 2013, at 11:02 AM, Simo Sorce <[email protected]> wrote:

> On Tue, 2013-10-22 at 10:22 -0400, [email protected] wrote:
>> From: Andy Adamson <[email protected]>
>>
>> This is an RFC patchset, which will be used for testing.
>>
>> This patchset requires the "SUNRPC: destroy gss_cred and context on Kerberos credential destruction" kernel patchset.
>>
>> We need to do a lot of testing to ensure that once kdestroy and gss-ctx
>> gss_user_destroy is called, all existing buffered
>> writes using the 'destroyed gss credential + context' are serviced.
>>
>> Differences from version 1:
>>
>> - moved from nfstgt_login and nfstgt_logout to gsskeyd.
>> - gsskeyd automatically creates gss-ctx key on kinit and destroys the gss-ctx
>> key on kdestroy.
>>
>> gsskeyd will need to act differently for different krb5 credential caches.
>
> Why ?

Just need different parsing to know which directory to poll.
>
>> For example, some versions of gssd store FILE credentials in FILE:/tmp/krb5cc_<UID>
>> while this code, written for fedora 19 uses FILE:/run/user/<UID>/krb5cc/tgt.
>
> This is incorrect, Fedora 19 use DIR type ccaches, so you should
> reference a cache withing the dir type, which will notmally be something
> like: DIR:/run/user/<uidnumber>/krb5cc for the whole collection or
> DIR::/run/user/<uidnumber>/krb5cc/txtXXXXXX for the sepcific cache.
>
> However note that due to issues with DIR type caches we are moving to a
> new KEYRING based type of cache in F20, so assuming any specific type of
> cache is a dead start.

But I'm not assuming any kind of kerberos credential cache! I just coded to one type for this RFC.
Didn't you see my KEYRING based ccache comment below?

>
>> As Trond suggested, if we keep gsskeyd separate from gssd, we could set up a
>> configuration file along the lines of the keytools' request-key.conf file to
>> allow both NFS and CIFS (and other filesystems) to install plugin handlers
>> for kinit/kdestroy events.
>
> Given in many cases (the desirable ones at least) kerberos credentials
> are not inited via kinit, but rather by things like pam_krb5, sssd, or
> directly imported via sshd, I am trying to understand how you are going
> to address the majority of these cases.

All of these cases use a kerberos credential cache which if is of type FILE the gsskeyd can poll inotify and if it is of type KEY: then gsskeyd adds the gss-ctx key to the Kerberos keyring (that is the theory :)) so the same code services kinit/pam_krb5

>
> Should users put gsskeyd in their .profile/.bashrc files or something ?
>
>> Else, we could have gssd be the process to poll inotify (given
>> that it already polls rpc_pipefs) and then just have it fork off the
>> subprocess if and when it sees an interesting event.
>
> What is an 'interesting' event ?

TGT (used for NFS TGS) creation/destruction

>
>> We need to investigate how this works when the kernel keyring is used for
>> Kerberos credentials. I believe that in this case gsskeyd can add the gss-ctx
>> key to the kerberos keyring, and it will get destroyed along with all other
>> keys at kdestroy.
>
> Is there any reason why you are doing this work with an utility that is
> separate from gssd or libkrb5 ?

Yes - to allow both NFS and CIFS (and other filesystems) to install plugin handlers for kerberos credential cache creation/destruction events.

But, having gssd do it is fine, or having a client plugin in libkrb5 is also fine.

>
> I think the only sensible way to handle something like this is by adding
> support directly in libkrb5, I do not see something external ever
> working reliably.

I agree that I should persue a libkrb5 approach.

> I am not against it for testing purposes, but then does it need to be
> committed to nfs-utils ?

Did you not see the "This is an RFC patchset"? I'm not asking for a commit to nfs-utils, but "request for comments" which you just did! Thanks :)

-->Andy

>
> Simo.
>
>> Andy Adamson (2):
>> GSSD add cc_name to upcall
>> ANDROS: update gsskeyd to use new /run/user/UID/krb5cc/tgt cache file
>>
>> Weston Andros Adamson (1):
>> WIP: Add gsskeyd
>>
>> configure.ac | 1 +
>> utils/Makefile.am | 2 +-
>> utils/gssd/gssd_proc.c | 37 ++++-
>> utils/gssd/krb5_util.c | 2 +-
>> utils/gssd/krb5_util.h | 1 +
>> utils/gsskeyd/gsskeyd.c | 371 ++++++++++++++++++++++++++++++++++++++++++++++++
>> 6 files changed, 408 insertions(+), 6 deletions(-)
>> create mode 100644 utils/gsskeyd/gsskeyd.c
>>
>
>
> --
> Simo Sorce * Red Hat, Inc * New York
>


2013-10-22 14:22:45

by Andy Adamson

[permalink] [raw]
Subject: [PATCH Version 2 3/3] ANDROS: update gsskeyd to use new /run/user/UID/krb5cc/tgt cache file

From: Andy Adamson <[email protected]>

Signed-off-by: Andy Adamson <[email protected]>
---
utils/gsskeyd/gsskeyd.c | 87 ++++++++++++++++++++++++++++++++++++-------------
1 file changed, 65 insertions(+), 22 deletions(-)

diff --git a/utils/gsskeyd/gsskeyd.c b/utils/gsskeyd/gsskeyd.c
index 6d598fa..722f893 100644
--- a/utils/gsskeyd/gsskeyd.c
+++ b/utils/gsskeyd/gsskeyd.c
@@ -17,10 +17,11 @@
#include <sys/queue.h>
#include <sys/uio.h>
#include <unistd.h>
+#include <dirent.h>

#define MAX_EVENT_SIZE (sizeof(struct inotify_event) + NAME_MAX + 1)

-//#define DEBUG_TRACE
+#define DEBUG_TRACE

#ifdef DEBUG_TRACE
#define TRACE(X...) do { fprintf(stderr, "_trace_: " X); } while (0)
@@ -162,46 +163,57 @@ print_event_mask(FILE *fp, uint32_t mask)
static int
manage_gss_ctx_keyring_mapping(struct update *u)
{
- char buf[MAXPATHLEN];
int status;
uid_t uid;
pid_t pid;
- key_serial_t key;
+ key_serial_t serial;
long res;

- snprintf(buf, MAXPATHLEN, "%s/%s", u->dir, u->name);
-
+#if CONFIG_OLD
if (sscanf(u->name, "krb5cc_%u", &uid) <= 0)
perror("parsing krb5cc uid");
+#endif /* CONFIG_OLD */
+
+ if (sscanf(u->dir, "/run/user/%u/krb5cc", &uid) <= 0)
+ perror("parsing krb5cc uid");

if ((pid = fork()) < 0)
perror("fork");

if (pid == 0) {
+ char description[16];
+
if (setuid(uid) < 0)
perror("setuid");

+ memset(description, 0, 16);
+ snprintf(description, 16, "gss-ctx_%d", uid);
+
if (u->is_remove) {
- fprintf(stderr, "remove gss ctx mapping for uid %u\n",
- uid);
- key = request_key("gss-ctx", "_nfstgt_", NULL,
- KEY_SPEC_USER_KEYRING);
- if (key > 0) {
- res = keyctl_unlink(key, KEY_SPEC_USER_KEYRING);
+
+ serial = keyctl_search(KEY_SPEC_USER_SESSION_KEYRING,
+ "gss-ctx", description, 0);
+ if (serial > 0) {
+ res = keyctl_unlink(serial,
+ KEY_SPEC_USER_SESSION_KEYRING);
if (res < 0)
warn("keyctl_unlink failed");
- } else
- warn("request_key failed");
+ } else
+ warn("keyctl_search failed");
+
+ TRACE("keyclt_unlink: serial %u for %d\n",
+ serial, uid);
} else {
- fprintf(stderr, "add gss ctx mapping for uid=%u\n",
- uid);
- snprintf(buf, MAXPATHLEN, "FILE:%s/%s",
+ char ccache[MAXPATHLEN];
+
+ snprintf(ccache, MAXPATHLEN, "FILE:%s/%s",
u->dir, u->name);
- key = add_key("gss-ctx", "_nfstgt_", buf,
- MAXPATHLEN, KEY_SPEC_USER_KEYRING);
+ serial = add_key("gss-ctx", description, ccache,
+ MAXPATHLEN, KEY_SPEC_USER_SESSION_KEYRING);

- if (key < 0)
+ if (serial < 0)
warn("add_key failed");
+ TRACE("add_key: serial %u for %d\n", serial, uid);
}

exit(0);
@@ -246,8 +258,7 @@ parse_events(int notify, struct watchdir_list *watchdirs, struct updates *update
fprintf(stderr, "\n");
#endif

- if (strlen(e->name) >= strlen("krb5cc_") &&
- strncmp("krb5cc_", e->name, strlen("krb5cc_")) != 0)
+ if (strncmp("tkt", e->name, strlen("tkt")) != 0)
TRACE("skip file: %s\n", e->name);
else
updates_add(updates, watchdirs, e);
@@ -288,6 +299,38 @@ handle_events(struct updates *updates)
timerclear(&updates->first);
}

+/**
+ * /run/user/<UID>/krb5cc are the directories that we want to watch.
+ * Note: assume only UIDs in the /run/user directory.
+ */
+void get_notify_dirs(struct watchdir_list *watchdirs, char *dirpath, int notify)
+{
+ DIR *dp;
+ struct dirent *ep;
+ char buf[MAXPATHLEN];
+
+ dp = opendir(dirpath);
+ if (dp == NULL) {
+ perror ("Couldn't open the notify directory");
+ return;
+ }
+
+ while (ep = readdir (dp)) {
+ if (strncmp(ep->d_name, ".", 1) == 0)
+ continue;
+ else if (strncmp(ep->d_name, "..", 2) == 0)
+ continue;
+ else if (ep->d_type == DT_DIR) {
+ memset(buf, 0, MAXPATHLEN);
+ snprintf(buf, MAXPATHLEN, "%s/%s/%s", dirpath,
+ ep->d_name, "krb5cc");
+ fprintf(stderr, "Added watch dir %s\n", buf);
+ watchdir_add(watchdirs, notify, buf);
+ }
+ }
+ (void) closedir (dp);
+ return;
+}

int
main(int argc, char **argv)
@@ -305,7 +348,7 @@ main(int argc, char **argv)
if (notify < 0)
perror("inotify_init1");

- watchdir_add(&watchdirs, notify, "/tmp");
+ get_notify_dirs(&watchdirs, "/run/user", notify);

LIST_INIT(&updates.list);
timerclear(&updates.first);
--
1.8.3.1


2013-10-22 15:02:30

by Simo Sorce

[permalink] [raw]
Subject: Re: [PATCH Version 2 0/3] GSSD: Use gss-ctx keys and gsskeyd to sync Kerberos credentials and kernel gss_contexts.

On Tue, 2013-10-22 at 10:22 -0400, [email protected] wrote:
> From: Andy Adamson <[email protected]>
>
> This is an RFC patchset, which will be used for testing.
>
> This patchset requires the "SUNRPC: destroy gss_cred and context on Kerberos credential destruction" kernel patchset.
>
> We need to do a lot of testing to ensure that once kdestroy and gss-ctx
> gss_user_destroy is called, all existing buffered
> writes using the 'destroyed gss credential + context' are serviced.
>
> Differences from version 1:
>
> - moved from nfstgt_login and nfstgt_logout to gsskeyd.
> - gsskeyd automatically creates gss-ctx key on kinit and destroys the gss-ctx
> key on kdestroy.
>
> gsskeyd will need to act differently for different krb5 credential caches.

Why ?

> For example, some versions of gssd store FILE credentials in FILE:/tmp/krb5cc_<UID>
> while this code, written for fedora 19 uses FILE:/run/user/<UID>/krb5cc/tgt.

This is incorrect, Fedora 19 use DIR type ccaches, so you should
reference a cache withing the dir type, which will notmally be something
like: DIR:/run/user/<uidnumber>/krb5cc for the whole collection or
DIR::/run/user/<uidnumber>/krb5cc/txtXXXXXX for the sepcific cache.

However note that due to issues with DIR type caches we are moving to a
new KEYRING based type of cache in F20, so assuming any specific type of
cache is a dead start.

> As Trond suggested, if we keep gsskeyd separate from gssd, we could set up a
> configuration file along the lines of the keytools' request-key.conf file to
> allow both NFS and CIFS (and other filesystems) to install plugin handlers
> for kinit/kdestroy events.

Given in many cases (the desirable ones at least) kerberos credentials
are not inited via kinit, but rather by things like pam_krb5, sssd, or
directly imported via sshd, I am trying to understand how you are going
to address the majority of these cases.

Should users put gsskeyd in their .profile/.bashrc files or something ?

> Else, we could have gssd be the process to poll inotify (given
> that it already polls rpc_pipefs) and then just have it fork off the
> subprocess if and when it sees an interesting event.

What is an 'interesting' event ?

> We need to investigate how this works when the kernel keyring is used for
> Kerberos credentials. I believe that in this case gsskeyd can add the gss-ctx
> key to the kerberos keyring, and it will get destroyed along with all other
> keys at kdestroy.

Is there any reason why you are doing this work with an utility that is
separate from gssd or libkrb5 ?

I think the only sensible way to handle something like this is by adding
support directly in libkrb5, I do not see something external ever
working reliably.
I am not against it for testing purposes, but then does it need to be
committed to nfs-utils ?

Simo.

> Andy Adamson (2):
> GSSD add cc_name to upcall
> ANDROS: update gsskeyd to use new /run/user/UID/krb5cc/tgt cache file
>
> Weston Andros Adamson (1):
> WIP: Add gsskeyd
>
> configure.ac | 1 +
> utils/Makefile.am | 2 +-
> utils/gssd/gssd_proc.c | 37 ++++-
> utils/gssd/krb5_util.c | 2 +-
> utils/gssd/krb5_util.h | 1 +
> utils/gsskeyd/gsskeyd.c | 371 ++++++++++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 408 insertions(+), 6 deletions(-)
> create mode 100644 utils/gsskeyd/gsskeyd.c
>


--
Simo Sorce * Red Hat, Inc * New York


2013-10-23 15:02:31

by Adamson, Andy

[permalink] [raw]
Subject: Re: [PATCH Version 2 2/3] WIP: Add gsskeyd

Sorry for all of the confusion. Next time I'll put WIP in the patch subject and make it a lot clearer as to what is going on.

Basically, the daemon is a way to automatically create/destroy a kernel keyring key on Kerberos credential cache creation and destruction which in turn triggers the Kernel code to locate and do the correct thing with the associated gss_cred and gss_contexts.

The kernel code is what I need tested.

We also, of course, need the correct way to notify the kernel GSS layer that the credentials have been destroyed. I like the suggestion of a libkrb5 client plugin or call out. I also think that when the Kerberos credentials are stored in the kernel keyring, there is an opportunity to have that work cleanly without any mucking with libkrb5.

-->Andy

On Oct 23, 2013, at 10:40 AM, Weston Andros Adamson <[email protected]>
wrote:

> Hey Steve,
>
> This patch is simply for testing. It is not anywhere near something we?d ask for inclusion in nfs-utils.
> If and when we want something like this, we?ll post something appropriate.
>
> -dros
>
> On Oct 23, 2013, at 10:30 AM, Steve Dickson <[email protected]> wrote:
>
>> Hello.
>>
>> After read this these recent threads about this, we may or may not end up with this
>> type of daemon, personally I hope we don't... ;-) But if we do...
>>
>> On 22/10/13 10:22, [email protected] wrote:
>>> From: Weston Andros Adamson <[email protected]>
>>>
>>> this is an experimental daemon that watches for krb credential cache files
>>> being created and deleted and creates or removes the mapping in the user
>>> keyring.
>>>
>>> this replaces the wrappers for kinit and kdestroy as they are no longer needed -
>>> users can go ahead using kinit/kdestroy and existing pam modules.
>>>
>>> if this is the way we want to go, this should probably end up as a processes
>>> forked from gssd and not its own daemon.
>>>
>>> Signed-off-by: Weston Andros Adamson <[email protected]>
>>> ---
>>> configure.ac | 1 +
>>> utils/Makefile.am | 2 +-
>>> utils/gsskeyd/gsskeyd.c | 328 ++++++++++++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 330 insertions(+), 1 deletion(-)
>>> create mode 100644 utils/gsskeyd/gsskeyd.c
>>>
>>> diff --git a/configure.ac b/configure.ac
>>> index d3ad854..a8e75ac 100644
>>> --- a/configure.ac
>>> +++ b/configure.ac
>>> @@ -497,6 +497,7 @@ AC_CONFIG_FILES([
>>> utils/nfsdcltrack/Makefile
>>> utils/exportfs/Makefile
>>> utils/gssd/Makefile
>>> + utils/gsskeyd/Makefile
>>> utils/idmapd/Makefile
>>> utils/mount/Makefile
>>> utils/mountd/Makefile
>>> diff --git a/utils/Makefile.am b/utils/Makefile.am
>>> index b892dc8..943ae2d 100644
>>> --- a/utils/Makefile.am
>>> +++ b/utils/Makefile.am
>>> @@ -14,7 +14,7 @@ OPTDIRS += blkmapd
>>> endif
>>>
>>> if CONFIG_GSS
>>> -OPTDIRS += gssd
>>> +OPTDIRS += gssd gsskeyd
>>> endif
>>>
>>> if CONFIG_MOUNT
>>> diff --git a/utils/gsskeyd/gsskeyd.c b/utils/gsskeyd/gsskeyd.c
>>> new file mode 100644
>>> index 0000000..6d598fa
>>> --- /dev/null
>>> +++ b/utils/gsskeyd/gsskeyd.c
>>> @@ -0,0 +1,328 @@
>>> +#include <stdio.h>
>>> +#include <stdlib.h>
>>> +#include <limits.h>
>>> +#include <assert.h>
>>> +#include <keyutils.h>
>>> +#include <err.h>
>>> +#include <errno.h>
>>> +#include <string.h>
>>> +
>>> +#include <sys/inotify.h>
>>> +#include <sys/errno.h>
>>> +#include <sys/select.h>
>>> +#include <sys/param.h>
>>> +#include <sys/time.h>
>>> +#include <sys/types.h>
>>> +#include <sys/wait.h>
>>> +#include <sys/queue.h>
>>> +#include <sys/uio.h>
>>> +#include <unistd.h>
>>> +
>>> +#define MAX_EVENT_SIZE (sizeof(struct inotify_event) + NAME_MAX + 1)
>>> +
>>> +//#define DEBUG_TRACE
>>> +
>>> +#ifdef DEBUG_TRACE
>>> +#define TRACE(X...) do { fprintf(stderr, "_trace_: " X); } while (0)
>>> +#else
>>> +#define TRACE(X...)
>>> +#endif
>> Please do not make debugging a compile decision. Go ahead and
>> use a '-d' or '-vvvv' (which each 'v' add more logging) flags to
>> make it a run time decision... Having to recompile to debug is
>> something that never made much sense to me...
>>
>> Secondly, you please use the existing xlog() interface to do your
>> error/warning/debugging logging... Having all logging going through
>> on interface is a good thing... IMHO...
>>
>> steved.
>>> +
>>> +/* .25 second timeouts to select */
>>> +#define SELECT_INTERVAL_TV { 0, 250000 }
>>> +
>>> +/* .5 seconds to collect list of unique file names that had events */
>>> +#define COLLECT_INTERVAL_TV { 0, 500000 }
>>> +
>>> +struct watchdir {
>>> + char dir[MAXPATHLEN];
>>> + int wd;
>>> + LIST_ENTRY(watchdir) entry;
>>> +};
>>> +
>>> +LIST_HEAD(watchdir_list, watchdir);
>>> +
>>> +struct update {
>>> + char dir[MAXPATHLEN];
>>> + char name[MAXPATHLEN];
>>> + int is_remove;
>>> + LIST_ENTRY(update) entry;
>>> +};
>>> +
>>> +LIST_HEAD(update_list, update);
>>> +
>>> +struct updates {
>>> + struct update_list list;
>>> + struct timeval first;
>>> +};
>>> +
>>> +void
>>> +watchdir_add(struct watchdir_list *watchdirs, int notify, char *dir)
>>> +{
>>> + struct watchdir *w;
>>> +
>>> + w = calloc(1, sizeof(struct watchdir));
>>> + assert(w != NULL);
>>> +
>>> + strncpy(w->dir, dir, MAXPATHLEN);
>>> +
>>> + w->wd = inotify_add_watch(notify, dir,
>>> + IN_CREATE | IN_DELETE | IN_MOVED_FROM | IN_MOVED_TO );
>>> + //| IN_MODIFY
>>> +
>>> + if (w->wd < 0)
>>> + perror("inotify_add_watch");
>>> +
>>> + LIST_INSERT_HEAD(watchdirs, w, entry);
>>> +
>>> + TRACE("WATCH(%d): %s\n", w->wd, w->dir);
>>> +}
>>> +
>>> +/*
>>> + * XXX maybe use hash when there is more than one watchdir?
>>> + */
>>> +struct watchdir *
>>> +watchdir_lookup(struct watchdir_list *watchdirs, int wd)
>>> +{
>>> + struct watchdir *w;
>>> +
>>> + LIST_FOREACH(w, watchdirs, entry) {
>>> + if (wd == w->wd)
>>> + return w;
>>> + }
>>> +
>>> + return NULL;
>>> +}
>>> +
>>> +static void
>>> +update_set_type(struct update *u, struct inotify_event *e)
>>> +{
>>> + if (e->mask & (IN_DELETE | IN_MOVED_FROM))
>>> + u->is_remove = 1;
>>> + else
>>> + u->is_remove = 0;
>>> +}
>>> +
>>> +void
>>> +updates_add(struct updates *updates, struct watchdir_list *watchdirs, struct inotify_event *e)
>>> +{
>>> + struct update *u;
>>> + struct watchdir *w;
>>> +
>>> +
>>> + w = watchdir_lookup(watchdirs, e->wd);
>>> + assert(w != NULL);
>>> +
>>> + TRACE("updates_add: %s/%s\n", w->dir, e->name);
>>> +
>>> + LIST_FOREACH(u, &updates->list, entry) {
>>> + if (strncmp(w->dir, u->dir, MAXPATHLEN) == 0 &&
>>> + strncmp(e->name, u->name, MAXPATHLEN) == 0) {
>>> + update_set_type(u, e);
>>> + return;
>>> + }
>>> + }
>>> +
>>> + u = calloc(1, sizeof(struct update));
>>> + strncpy(u->dir, w->dir, MAXPATHLEN);
>>> + strncpy(u->name, e->name, MAXPATHLEN);
>>> + update_set_type(u, e);
>>> +
>>> + LIST_INSERT_HEAD(&updates->list, u, entry);
>>> +
>>> + if (!timerisset(&updates->first)) {
>>> + if (gettimeofday(&updates->first, NULL) < 0)
>>> + perror("gettimeofday");
>>> + }
>>> +}
>>> +
>>> +void
>>> +print_event_mask(FILE *fp, uint32_t mask)
>>> +{
>>> +#define _print_if_found(X, S) do { \
>>> + if (mask & (X)) { \
>>> + fprintf(fp, S); \
>>> + } \
>>> + } while(0)
>>> +
>>> + _print_if_found(IN_ACCESS, "access");
>>> + _print_if_found(IN_ATTRIB, "attrib");
>>> + _print_if_found(IN_CLOSE_WRITE, "close write");
>>> + _print_if_found(IN_CLOSE_NOWRITE, "close nowrite");
>>> + _print_if_found(IN_CREATE, "create");
>>> + _print_if_found(IN_DELETE, "delete");
>>> + _print_if_found(IN_DELETE_SELF, "delete self");
>>> + _print_if_found(IN_MODIFY, "modify");
>>> + _print_if_found(IN_MOVE_SELF, "move self");
>>> + _print_if_found(IN_MOVED_FROM, "moved from");
>>> + _print_if_found(IN_MOVED_TO, "moved to");
>>> + _print_if_found(IN_OPEN, "open");
>>> +}
>>> +
>>> +static int
>>> +manage_gss_ctx_keyring_mapping(struct update *u)
>>> +{
>>> + char buf[MAXPATHLEN];
>>> + int status;
>>> + uid_t uid;
>>> + pid_t pid;
>>> + key_serial_t key;
>>> + long res;
>>> +
>>> + snprintf(buf, MAXPATHLEN, "%s/%s", u->dir, u->name);
>>> +
>>> + if (sscanf(u->name, "krb5cc_%u", &uid) <= 0)
>>> + perror("parsing krb5cc uid");
>>> +
>>> + if ((pid = fork()) < 0)
>>> + perror("fork");
>>> +
>>> + if (pid == 0) {
>>> + if (setuid(uid) < 0)
>>> + perror("setuid");
>>> +
>>> + if (u->is_remove) {
>>> + fprintf(stderr, "remove gss ctx mapping for uid %u\n",
>>> + uid);
>>> + key = request_key("gss-ctx", "_nfstgt_", NULL,
>>> + KEY_SPEC_USER_KEYRING);
>>> + if (key > 0) {
>>> + res = keyctl_unlink(key, KEY_SPEC_USER_KEYRING);
>>> + if (res < 0)
>>> + warn("keyctl_unlink failed");
>>> + } else
>>> + warn("request_key failed");
>>> + } else {
>>> + fprintf(stderr, "add gss ctx mapping for uid=%u\n",
>>> + uid);
>>> + snprintf(buf, MAXPATHLEN, "FILE:%s/%s",
>>> + u->dir, u->name);
>>> + key = add_key("gss-ctx", "_nfstgt_", buf,
>>> + MAXPATHLEN, KEY_SPEC_USER_KEYRING);
>>> +
>>> + if (key < 0)
>>> + warn("add_key failed");
>>> + }
>>> +
>>> + exit(0);
>>> + } else {
>>> + waitpid(pid, &status, 0);
>>> +
>>> + if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
>>> + return WEXITSTATUS(status);
>>> + }
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +void
>>> +parse_events(int notify, struct watchdir_list *watchdirs, struct updates *updates)
>>> +{
>>> + ssize_t nread, pos;
>>> + unsigned char buf[MAX_EVENT_SIZE * 100];
>>> + struct inotify_event *e;
>>> +
>>> + for (;;) {
>>> + nread = read(notify, buf, sizeof(buf));
>>> +
>>> + if (nread <= 0) {
>>> + TRACE("read returns %d, errno %u\n", nread, errno);
>>> + break;
>>> + }
>>> +
>>> + TRACE("Read %d bytes\n", nread);
>>> +
>>> + pos = 0;
>>> + while (pos < nread) {
>>> + assert((nread - pos) >=
>>> + (ssize_t)sizeof(struct inotify_event));
>>> + e = (struct inotify_event *)(buf + pos);
>>> + pos += sizeof(struct inotify_event) + e->len;
>>> +
>>> +#ifdef DEBUG_TRACE
>>> + TRACE("EVENT on %s: ", e->name);
>>> + print_event_mask(stderr, e->mask);
>>> + fprintf(stderr, "\n");
>>> +#endif
>>> +
>>> + if (strlen(e->name) >= strlen("krb5cc_") &&
>>> + strncmp("krb5cc_", e->name, strlen("krb5cc_")) != 0)
>>> + TRACE("skip file: %s\n", e->name);
>>> + else
>>> + updates_add(updates, watchdirs, e);
>>> + }
>>> + assert(pos == nread);
>>> + }
>>> +}
>>> +
>>> +void
>>> +handle_events(struct updates *updates)
>>> +{
>>> + struct update *u;
>>> + struct timeval now, diff, collection_interval = COLLECT_INTERVAL_TV;
>>> +
>>> + if (!timerisset(&updates->first))
>>> + return;
>>> +
>>> + if (gettimeofday(&now, NULL) < 0)
>>> + perror("gettimeofday");
>>> +
>>> + timersub(&now, &updates->first, &diff);
>>> +
>>> + if (!(diff.tv_sec >= collection_interval.tv_sec &&
>>> + diff.tv_usec >= collection_interval.tv_usec)) {
>>> + TRACE("it's been %u s %u ms - waiting longer...\n",
>>> + diff.tv_sec, diff.tv_usec);
>>> + return;
>>> + }
>>> +
>>> + while (!LIST_EMPTY(&updates->list)) {
>>> + u = updates->list.lh_first;
>>> +
>>> + TRACE("handle krb5 cc file: %s\n", u->name);
>>> + manage_gss_ctx_keyring_mapping(u);
>>> + LIST_REMOVE(u, entry);
>>> + }
>>> +
>>> + timerclear(&updates->first);
>>> +}
>>> +
>>> +
>>> +int
>>> +main(int argc, char **argv)
>>> +{
>>> + fd_set readfds;
>>> + int notify;
>>> + int res;
>>> + struct timeval timeo, select_interval = SELECT_INTERVAL_TV;
>>> + struct watchdir_list watchdirs;
>>> + struct updates updates;
>>> +
>>> + LIST_INIT(&watchdirs);
>>> +
>>> + notify = inotify_init1(IN_NONBLOCK);
>>> + if (notify < 0)
>>> + perror("inotify_init1");
>>> +
>>> + watchdir_add(&watchdirs, notify, "/tmp");
>>> +
>>> + LIST_INIT(&updates.list);
>>> + timerclear(&updates.first);
>>> +
>>> + for (;;) {
>>> + FD_ZERO(&readfds);
>>> + FD_SET(notify, &readfds);
>>> + memcpy(&timeo, &select_interval, sizeof(struct timeval));
>>> +
>>> + res = select(notify + 1, &readfds, NULL, NULL, &timeo);
>>> +
>>> + if (res < 0)
>>> + perror("error calling select");
>>> +
>>> + if (FD_ISSET(notify, &readfds))
>>> + parse_events(notify, &watchdirs, &updates);
>>> +
>>> + handle_events(&updates);
>>> + }
>>> +}
>


2013-10-22 16:11:03

by Simo Sorce

[permalink] [raw]
Subject: Re: [PATCH Version 2 0/3] GSSD: Use gss-ctx keys and gsskeyd to sync Kerberos credentials and kernel gss_contexts.

On Tue, 2013-10-22 at 15:46 +0000, Weston Andros Adamson wrote:
>
> gsskeyd is a separate daemon only for proof of concept. In the commit
> message it makes it clear that if this is the way we want to go, it
> should be incorporated into gssd.
>
The more I think of the idea the more I think you'd not get what you
want using a daemon that tries to poll files and second guess user
intentions by the way libkrb5 actually operates, you'd probably be
subject to way too many false positives to be useful.

Simo.

--
Simo Sorce * Red Hat, Inc * New York


2013-10-22 16:39:40

by Adamson, Andy

[permalink] [raw]
Subject: Re: [PATCH Version 2 0/3] GSSD: Use gss-ctx keys and gsskeyd to sync Kerberos credentials and kernel gss_contexts.


On Oct 22, 2013, at 12:14 PM, Weston Andros Adamson <[email protected]>
wrote:

> On Oct 22, 2013, at 12:11 PM, Simo Sorce <[email protected]>
> wrote:
>
>> On Tue, 2013-10-22 at 15:46 +0000, Weston Andros Adamson wrote:
>>>
>>> gsskeyd is a separate daemon only for proof of concept. In the commit
>>> message it makes it clear that if this is the way we want to go, it
>>> should be incorporated into gssd.
>>>
>> The more I think of the idea the more I think you'd not get what you
>> want using a daemon that tries to poll files and second guess user
>> intentions by the way libkrb5 actually operates, you'd probably be
>> subject to way too many false positives to be useful.

I will approach MIT about the possibility of client-side plugins.

-->Andy

>>
>
> I think you're probably right, but this method works well for testing NFS code under near-to-expiry and expired cred conditions.
>
> -dros


2013-10-22 16:14:38

by Weston Andros Adamson

[permalink] [raw]
Subject: Re: [PATCH Version 2 0/3] GSSD: Use gss-ctx keys and gsskeyd to sync Kerberos credentials and kernel gss_contexts.

On Oct 22, 2013, at 12:11 PM, Simo Sorce <[email protected]>
wrote:

> On Tue, 2013-10-22 at 15:46 +0000, Weston Andros Adamson wrote:
>>
>> gsskeyd is a separate daemon only for proof of concept. In the commit
>> message it makes it clear that if this is the way we want to go, it
>> should be incorporated into gssd.
>>
> The more I think of the idea the more I think you'd not get what you
> want using a daemon that tries to poll files and second guess user
> intentions by the way libkrb5 actually operates, you'd probably be
> subject to way too many false positives to be useful.
>

I think you're probably right, but this method works well for testing NFS code under near-to-expiry and expired cred conditions.

-dros

2013-10-22 16:09:26

by Simo Sorce

[permalink] [raw]
Subject: Re: [PATCH Version 2 0/3] GSSD: Use gss-ctx keys and gsskeyd to sync Kerberos credentials and kernel gss_contexts.

On Tue, 2013-10-22 at 15:32 +0000, Adamson, Andy wrote:
> On Oct 22, 2013, at 11:02 AM, Simo Sorce <[email protected]> wrote:
>
> > On Tue, 2013-10-22 at 10:22 -0400, [email protected] wrote:
> >> From: Andy Adamson <[email protected]>
> >>
> >> This is an RFC patchset, which will be used for testing.
> >>
> >> This patchset requires the "SUNRPC: destroy gss_cred and context on Kerberos credential destruction" kernel patchset.
> >>
> >> We need to do a lot of testing to ensure that once kdestroy and gss-ctx
> >> gss_user_destroy is called, all existing buffered
> >> writes using the 'destroyed gss credential + context' are serviced.
> >>
> >> Differences from version 1:
> >>
> >> - moved from nfstgt_login and nfstgt_logout to gsskeyd.
> >> - gsskeyd automatically creates gss-ctx key on kinit and destroys the gss-ctx
> >> key on kdestroy.
> >>
> >> gsskeyd will need to act differently for different krb5 credential caches.
> >
> > Why ?
>
> Just need different parsing to know which directory to poll.
> >
> >> For example, some versions of gssd store FILE credentials in FILE:/tmp/krb5cc_<UID>
> >> while this code, written for fedora 19 uses FILE:/run/user/<UID>/krb5cc/tgt.
> >
> > This is incorrect, Fedora 19 use DIR type ccaches, so you should
> > reference a cache withing the dir type, which will notmally be something
> > like: DIR:/run/user/<uidnumber>/krb5cc for the whole collection or
> > DIR::/run/user/<uidnumber>/krb5cc/txtXXXXXX for the sepcific cache.
> >
> > However note that due to issues with DIR type caches we are moving to a
> > new KEYRING based type of cache in F20, so assuming any specific type of
> > cache is a dead start.
>
> But I'm not assuming any kind of kerberos credential cache! I just coded to one type for this RFC.
> Didn't you see my KEYRING based ccache comment below?

Ah sorry, I had not noticed.

> >> As Trond suggested, if we keep gsskeyd separate from gssd, we could set up a
> >> configuration file along the lines of the keytools' request-key.conf file to
> >> allow both NFS and CIFS (and other filesystems) to install plugin handlers
> >> for kinit/kdestroy events.
> >
> > Given in many cases (the desirable ones at least) kerberos credentials
> > are not inited via kinit, but rather by things like pam_krb5, sssd, or
> > directly imported via sshd, I am trying to understand how you are going
> > to address the majority of these cases.
>
> All of these cases use a kerberos credential cache which if is of type
> FILE the gsskeyd can poll inotify and if it is of type KEY: then
> gsskeyd adds the gss-ctx key to the Kerberos keyring (that is the
> theory :)) so the same code services kinit/pam_krb5

You wouldn't be able to see it if I set a custom ccache file though
export KRB5CCNAME=FILE:/random/sir/foofile

> > Should users put gsskeyd in their .profile/.bashrc files or something ?
> >
> >> Else, we could have gssd be the process to poll inotify (given
> >> that it already polls rpc_pipefs) and then just have it fork off the
> >> subprocess if and when it sees an interesting event.
> >
> > What is an 'interesting' event ?
>
> TGT (used for NFS TGS) creation/destruction

Shouldn't you care for the specific NFS ticket only ?
Although kdestroy is quite blunt, in theory users can simply destroy the
specific NFS ticket and keep their TGT.

Also I see a problem with this approach. In order to make user life
easier we reinit ccaches in sssd when a user for example unlocks the
screen. If you track a specific TGT or NFS ticket, this would cause the
immediate destruction of credentials in the nfs client which will also
kill access to the NFS server. I think this would cause quite some grief
to users.

> >> We need to investigate how this works when the kernel keyring is used for
> >> Kerberos credentials. I believe that in this case gsskeyd can add the gss-ctx
> >> key to the kerberos keyring, and it will get destroyed along with all other
> >> keys at kdestroy.
> >
> > Is there any reason why you are doing this work with an utility that is
> > separate from gssd or libkrb5 ?
>
> Yes - to allow both NFS and CIFS (and other filesystems) to install
> plugin handlers for kerberos credential cache creation/destruction
> events.
>
> But, having gssd do it is fine, or having a client plugin in libkrb5
> is also fine.

> >
> > I think the only sensible way to handle something like this is by adding
> > support directly in libkrb5, I do not see something external ever
> > working reliably.
>
> I agree that I should persue a libkrb5 approach.

Keep in mind the point above though.
The issues here is that you do not know what the intention was if you
try a completely automated approach. you should probably interact with
the login service and act conditionally in any way.

I think it may be safer for you to provide a hook that the system can
call when a user fully logs out. And use that hook to kill any
credential associated to a particular UID.

Done this way you do not need to poll anything nor change libkrb5. And
you also allow the creation of a tool that the user can run to force the
destruction of his credentials if he is not using a compliant login
manager. Or if he wants to explicitly kill NFS creds in the kernel for
whatever reason.

> > I am not against it for testing purposes, but then does it need to be
> > committed to nfs-utils ?
>
> Did you not see the "This is an RFC patchset"? I'm not asking for a commit to nfs-utils, but "request for comments" which you just did! Thanks :)

Yes I was providing feedback :-)

Simo.

--
Simo Sorce * Red Hat, Inc * New York


2013-10-22 14:22:45

by Andy Adamson

[permalink] [raw]
Subject: [PATCH Version 2 1/3] GSSD add cc_name to upcall

From: Andy Adamson <[email protected]>

Signed-off-by: Andy Adamson <[email protected]>
---
utils/gssd/gssd_proc.c | 37 +++++++++++++++++++++++++++++++++----
utils/gssd/krb5_util.c | 2 +-
utils/gssd/krb5_util.h | 1 +
3 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index 2d3dbec..8df61a4 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -966,7 +966,7 @@ create_auth_rpc_client(struct clnt_info *clp,
*/
static void
process_krb5_upcall(struct clnt_info *clp, uid_t uid, int fd, char *tgtname,
- char *service)
+ char *service, char *cc_name)
{
CLIENT *rpc_clnt = NULL;
AUTH *auth = NULL;
@@ -980,7 +980,8 @@ process_krb5_upcall(struct clnt_info *clp, uid_t uid, int fd, char *tgtname,
gss_cred_id_t gss_cred;
OM_uint32 maj_stat, min_stat, lifetime_rec;

- printerr(1, "handling krb5 upcall (%s)\n", clp->dirname);
+ printerr(1, "handling krb5 upcall (%s) cc_name %p\n", clp->dirname,
+ cc_name);

token.length = 0;
token.value = NULL;
@@ -1011,6 +1012,18 @@ process_krb5_upcall(struct clnt_info *clp, uid_t uid, int fd, char *tgtname,
service ? service : "<null>");
if (uid != 0 || (uid == 0 && root_uses_machine_creds == 0 &&
service == NULL)) {
+ /* Use the ccache name from the upcall */
+ if (cc_name != NULL) {
+ printerr(2, "using %s as credentials cache for client "
+ "with uid %u for server %s\n", cc_name,
+ uid, clp->servername);
+ gssd_set_krb5_ccache_name(cc_name);
+ create_resp = create_auth_rpc_client(clp,
+ &rpc_clnt, &auth, uid,
+ AUTHTYPE_KRB5, gss_cred);
+ if (create_resp == 0)
+ goto resp_found;
+ }
/* Tell krb5 gss which credentials cache to use */
/* Try first to acquire credentials directly via GSSAPI */
err = gssd_acquire_user_cred(uid, &gss_cred);
@@ -1083,6 +1096,7 @@ process_krb5_upcall(struct clnt_info *clp, uid_t uid, int fd, char *tgtname,
}
}

+resp_found:
if (!authgss_get_private_data(auth, &pd)) {
printerr(1, "WARNING: Failed to obtain authentication "
"data for user with uid %d for server %s\n",
@@ -1137,7 +1151,7 @@ handle_krb5_upcall(struct clnt_info *clp)
return;
}

- process_krb5_upcall(clp, uid, clp->krb5_fd, NULL, NULL);
+ process_krb5_upcall(clp, uid, clp->krb5_fd, NULL, NULL, NULL);
}

void
@@ -1151,6 +1165,7 @@ handle_gssd_upcall(struct clnt_info *clp)
char *target = NULL;
char *service = NULL;
char *enctypes = NULL;
+ char *cc_name = NULL;

printerr(1, "handling gssd upcall (%s)\n", clp->dirname);

@@ -1245,9 +1260,23 @@ handle_gssd_upcall(struct clnt_info *clp)
goto out;
}
}
+ /* read the ccache name. */
+ if ((p = strstr(lbuf, "ccache=")) != NULL) {
+ printerr(2, "CC_NAME to parse\n");
+ cc_name = malloc(lbuflen);
+ if (!cc_name)
+ goto out;
+ if (sscanf(p, "ccache=%s", cc_name) != 1) {
+ printerr(2, "WARNING: handle_gssd_upcall: "
+ "failed to parse cc_name "
+ "in upcall string '%s'\n", lbuf);
+ goto out;
+ }
+ }

if (strcmp(mech, "krb5") == 0)
- process_krb5_upcall(clp, uid, clp->gssd_fd, target, service);
+ process_krb5_upcall(clp, uid, clp->gssd_fd, target, service,
+ cc_name);
else
printerr(0, "WARNING: handle_gssd_upcall: "
"received unknown gss mech '%s'\n", mech);
diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
index 83b9651..1bb0da6 100644
--- a/utils/gssd/krb5_util.c
+++ b/utils/gssd/krb5_util.c
@@ -471,7 +471,7 @@ gssd_get_single_krb5_cred(krb5_context context,
* Depending on the version of Kerberos, we either need to use
* a private function, or simply set the environment variable.
*/
-static void
+void
gssd_set_krb5_ccache_name(char *ccname)
{
#ifdef USE_GSS_KRB5_CCACHE_NAME
diff --git a/utils/gssd/krb5_util.h b/utils/gssd/krb5_util.h
index eed1294..16119a8 100644
--- a/utils/gssd/krb5_util.h
+++ b/utils/gssd/krb5_util.h
@@ -23,6 +23,7 @@ struct gssd_k5_kt_princ {
};


+void gssd_set_krb5_ccache_name(char *ccname);
int gssd_setup_krb5_user_gss_ccache(uid_t uid, char *servername,
char *dirname);
int gssd_get_krb5_machine_cred_list(char ***list);
--
1.8.3.1


2013-10-23 14:40:49

by Weston Andros Adamson

[permalink] [raw]
Subject: Re: [PATCH Version 2 2/3] WIP: Add gsskeyd

Hey Steve,

This patch is simply for testing. It is not anywhere near something we?d ask for inclusion in nfs-utils.
If and when we want something like this, we?ll post something appropriate.

-dros

On Oct 23, 2013, at 10:30 AM, Steve Dickson <[email protected]> wrote:

> Hello.
>
> After read this these recent threads about this, we may or may not end up with this
> type of daemon, personally I hope we don't... ;-) But if we do...
>
> On 22/10/13 10:22, [email protected] wrote:
>> From: Weston Andros Adamson <[email protected]>
>>
>> this is an experimental daemon that watches for krb credential cache files
>> being created and deleted and creates or removes the mapping in the user
>> keyring.
>>
>> this replaces the wrappers for kinit and kdestroy as they are no longer needed -
>> users can go ahead using kinit/kdestroy and existing pam modules.
>>
>> if this is the way we want to go, this should probably end up as a processes
>> forked from gssd and not its own daemon.
>>
>> Signed-off-by: Weston Andros Adamson <[email protected]>
>> ---
>> configure.ac | 1 +
>> utils/Makefile.am | 2 +-
>> utils/gsskeyd/gsskeyd.c | 328 ++++++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 330 insertions(+), 1 deletion(-)
>> create mode 100644 utils/gsskeyd/gsskeyd.c
>>
>> diff --git a/configure.ac b/configure.ac
>> index d3ad854..a8e75ac 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -497,6 +497,7 @@ AC_CONFIG_FILES([
>> utils/nfsdcltrack/Makefile
>> utils/exportfs/Makefile
>> utils/gssd/Makefile
>> + utils/gsskeyd/Makefile
>> utils/idmapd/Makefile
>> utils/mount/Makefile
>> utils/mountd/Makefile
>> diff --git a/utils/Makefile.am b/utils/Makefile.am
>> index b892dc8..943ae2d 100644
>> --- a/utils/Makefile.am
>> +++ b/utils/Makefile.am
>> @@ -14,7 +14,7 @@ OPTDIRS += blkmapd
>> endif
>>
>> if CONFIG_GSS
>> -OPTDIRS += gssd
>> +OPTDIRS += gssd gsskeyd
>> endif
>>
>> if CONFIG_MOUNT
>> diff --git a/utils/gsskeyd/gsskeyd.c b/utils/gsskeyd/gsskeyd.c
>> new file mode 100644
>> index 0000000..6d598fa
>> --- /dev/null
>> +++ b/utils/gsskeyd/gsskeyd.c
>> @@ -0,0 +1,328 @@
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <limits.h>
>> +#include <assert.h>
>> +#include <keyutils.h>
>> +#include <err.h>
>> +#include <errno.h>
>> +#include <string.h>
>> +
>> +#include <sys/inotify.h>
>> +#include <sys/errno.h>
>> +#include <sys/select.h>
>> +#include <sys/param.h>
>> +#include <sys/time.h>
>> +#include <sys/types.h>
>> +#include <sys/wait.h>
>> +#include <sys/queue.h>
>> +#include <sys/uio.h>
>> +#include <unistd.h>
>> +
>> +#define MAX_EVENT_SIZE (sizeof(struct inotify_event) + NAME_MAX + 1)
>> +
>> +//#define DEBUG_TRACE
>> +
>> +#ifdef DEBUG_TRACE
>> +#define TRACE(X...) do { fprintf(stderr, "_trace_: " X); } while (0)
>> +#else
>> +#define TRACE(X...)
>> +#endif
> Please do not make debugging a compile decision. Go ahead and
> use a '-d' or '-vvvv' (which each 'v' add more logging) flags to
> make it a run time decision... Having to recompile to debug is
> something that never made much sense to me...
>
> Secondly, you please use the existing xlog() interface to do your
> error/warning/debugging logging... Having all logging going through
> on interface is a good thing... IMHO...
>
> steved.
>> +
>> +/* .25 second timeouts to select */
>> +#define SELECT_INTERVAL_TV { 0, 250000 }
>> +
>> +/* .5 seconds to collect list of unique file names that had events */
>> +#define COLLECT_INTERVAL_TV { 0, 500000 }
>> +
>> +struct watchdir {
>> + char dir[MAXPATHLEN];
>> + int wd;
>> + LIST_ENTRY(watchdir) entry;
>> +};
>> +
>> +LIST_HEAD(watchdir_list, watchdir);
>> +
>> +struct update {
>> + char dir[MAXPATHLEN];
>> + char name[MAXPATHLEN];
>> + int is_remove;
>> + LIST_ENTRY(update) entry;
>> +};
>> +
>> +LIST_HEAD(update_list, update);
>> +
>> +struct updates {
>> + struct update_list list;
>> + struct timeval first;
>> +};
>> +
>> +void
>> +watchdir_add(struct watchdir_list *watchdirs, int notify, char *dir)
>> +{
>> + struct watchdir *w;
>> +
>> + w = calloc(1, sizeof(struct watchdir));
>> + assert(w != NULL);
>> +
>> + strncpy(w->dir, dir, MAXPATHLEN);
>> +
>> + w->wd = inotify_add_watch(notify, dir,
>> + IN_CREATE | IN_DELETE | IN_MOVED_FROM | IN_MOVED_TO );
>> + //| IN_MODIFY
>> +
>> + if (w->wd < 0)
>> + perror("inotify_add_watch");
>> +
>> + LIST_INSERT_HEAD(watchdirs, w, entry);
>> +
>> + TRACE("WATCH(%d): %s\n", w->wd, w->dir);
>> +}
>> +
>> +/*
>> + * XXX maybe use hash when there is more than one watchdir?
>> + */
>> +struct watchdir *
>> +watchdir_lookup(struct watchdir_list *watchdirs, int wd)
>> +{
>> + struct watchdir *w;
>> +
>> + LIST_FOREACH(w, watchdirs, entry) {
>> + if (wd == w->wd)
>> + return w;
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +static void
>> +update_set_type(struct update *u, struct inotify_event *e)
>> +{
>> + if (e->mask & (IN_DELETE | IN_MOVED_FROM))
>> + u->is_remove = 1;
>> + else
>> + u->is_remove = 0;
>> +}
>> +
>> +void
>> +updates_add(struct updates *updates, struct watchdir_list *watchdirs, struct inotify_event *e)
>> +{
>> + struct update *u;
>> + struct watchdir *w;
>> +
>> +
>> + w = watchdir_lookup(watchdirs, e->wd);
>> + assert(w != NULL);
>> +
>> + TRACE("updates_add: %s/%s\n", w->dir, e->name);
>> +
>> + LIST_FOREACH(u, &updates->list, entry) {
>> + if (strncmp(w->dir, u->dir, MAXPATHLEN) == 0 &&
>> + strncmp(e->name, u->name, MAXPATHLEN) == 0) {
>> + update_set_type(u, e);
>> + return;
>> + }
>> + }
>> +
>> + u = calloc(1, sizeof(struct update));
>> + strncpy(u->dir, w->dir, MAXPATHLEN);
>> + strncpy(u->name, e->name, MAXPATHLEN);
>> + update_set_type(u, e);
>> +
>> + LIST_INSERT_HEAD(&updates->list, u, entry);
>> +
>> + if (!timerisset(&updates->first)) {
>> + if (gettimeofday(&updates->first, NULL) < 0)
>> + perror("gettimeofday");
>> + }
>> +}
>> +
>> +void
>> +print_event_mask(FILE *fp, uint32_t mask)
>> +{
>> +#define _print_if_found(X, S) do { \
>> + if (mask & (X)) { \
>> + fprintf(fp, S); \
>> + } \
>> + } while(0)
>> +
>> + _print_if_found(IN_ACCESS, "access");
>> + _print_if_found(IN_ATTRIB, "attrib");
>> + _print_if_found(IN_CLOSE_WRITE, "close write");
>> + _print_if_found(IN_CLOSE_NOWRITE, "close nowrite");
>> + _print_if_found(IN_CREATE, "create");
>> + _print_if_found(IN_DELETE, "delete");
>> + _print_if_found(IN_DELETE_SELF, "delete self");
>> + _print_if_found(IN_MODIFY, "modify");
>> + _print_if_found(IN_MOVE_SELF, "move self");
>> + _print_if_found(IN_MOVED_FROM, "moved from");
>> + _print_if_found(IN_MOVED_TO, "moved to");
>> + _print_if_found(IN_OPEN, "open");
>> +}
>> +
>> +static int
>> +manage_gss_ctx_keyring_mapping(struct update *u)
>> +{
>> + char buf[MAXPATHLEN];
>> + int status;
>> + uid_t uid;
>> + pid_t pid;
>> + key_serial_t key;
>> + long res;
>> +
>> + snprintf(buf, MAXPATHLEN, "%s/%s", u->dir, u->name);
>> +
>> + if (sscanf(u->name, "krb5cc_%u", &uid) <= 0)
>> + perror("parsing krb5cc uid");
>> +
>> + if ((pid = fork()) < 0)
>> + perror("fork");
>> +
>> + if (pid == 0) {
>> + if (setuid(uid) < 0)
>> + perror("setuid");
>> +
>> + if (u->is_remove) {
>> + fprintf(stderr, "remove gss ctx mapping for uid %u\n",
>> + uid);
>> + key = request_key("gss-ctx", "_nfstgt_", NULL,
>> + KEY_SPEC_USER_KEYRING);
>> + if (key > 0) {
>> + res = keyctl_unlink(key, KEY_SPEC_USER_KEYRING);
>> + if (res < 0)
>> + warn("keyctl_unlink failed");
>> + } else
>> + warn("request_key failed");
>> + } else {
>> + fprintf(stderr, "add gss ctx mapping for uid=%u\n",
>> + uid);
>> + snprintf(buf, MAXPATHLEN, "FILE:%s/%s",
>> + u->dir, u->name);
>> + key = add_key("gss-ctx", "_nfstgt_", buf,
>> + MAXPATHLEN, KEY_SPEC_USER_KEYRING);
>> +
>> + if (key < 0)
>> + warn("add_key failed");
>> + }
>> +
>> + exit(0);
>> + } else {
>> + waitpid(pid, &status, 0);
>> +
>> + if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
>> + return WEXITSTATUS(status);
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +void
>> +parse_events(int notify, struct watchdir_list *watchdirs, struct updates *updates)
>> +{
>> + ssize_t nread, pos;
>> + unsigned char buf[MAX_EVENT_SIZE * 100];
>> + struct inotify_event *e;
>> +
>> + for (;;) {
>> + nread = read(notify, buf, sizeof(buf));
>> +
>> + if (nread <= 0) {
>> + TRACE("read returns %d, errno %u\n", nread, errno);
>> + break;
>> + }
>> +
>> + TRACE("Read %d bytes\n", nread);
>> +
>> + pos = 0;
>> + while (pos < nread) {
>> + assert((nread - pos) >=
>> + (ssize_t)sizeof(struct inotify_event));
>> + e = (struct inotify_event *)(buf + pos);
>> + pos += sizeof(struct inotify_event) + e->len;
>> +
>> +#ifdef DEBUG_TRACE
>> + TRACE("EVENT on %s: ", e->name);
>> + print_event_mask(stderr, e->mask);
>> + fprintf(stderr, "\n");
>> +#endif
>> +
>> + if (strlen(e->name) >= strlen("krb5cc_") &&
>> + strncmp("krb5cc_", e->name, strlen("krb5cc_")) != 0)
>> + TRACE("skip file: %s\n", e->name);
>> + else
>> + updates_add(updates, watchdirs, e);
>> + }
>> + assert(pos == nread);
>> + }
>> +}
>> +
>> +void
>> +handle_events(struct updates *updates)
>> +{
>> + struct update *u;
>> + struct timeval now, diff, collection_interval = COLLECT_INTERVAL_TV;
>> +
>> + if (!timerisset(&updates->first))
>> + return;
>> +
>> + if (gettimeofday(&now, NULL) < 0)
>> + perror("gettimeofday");
>> +
>> + timersub(&now, &updates->first, &diff);
>> +
>> + if (!(diff.tv_sec >= collection_interval.tv_sec &&
>> + diff.tv_usec >= collection_interval.tv_usec)) {
>> + TRACE("it's been %u s %u ms - waiting longer...\n",
>> + diff.tv_sec, diff.tv_usec);
>> + return;
>> + }
>> +
>> + while (!LIST_EMPTY(&updates->list)) {
>> + u = updates->list.lh_first;
>> +
>> + TRACE("handle krb5 cc file: %s\n", u->name);
>> + manage_gss_ctx_keyring_mapping(u);
>> + LIST_REMOVE(u, entry);
>> + }
>> +
>> + timerclear(&updates->first);
>> +}
>> +
>> +
>> +int
>> +main(int argc, char **argv)
>> +{
>> + fd_set readfds;
>> + int notify;
>> + int res;
>> + struct timeval timeo, select_interval = SELECT_INTERVAL_TV;
>> + struct watchdir_list watchdirs;
>> + struct updates updates;
>> +
>> + LIST_INIT(&watchdirs);
>> +
>> + notify = inotify_init1(IN_NONBLOCK);
>> + if (notify < 0)
>> + perror("inotify_init1");
>> +
>> + watchdir_add(&watchdirs, notify, "/tmp");
>> +
>> + LIST_INIT(&updates.list);
>> + timerclear(&updates.first);
>> +
>> + for (;;) {
>> + FD_ZERO(&readfds);
>> + FD_SET(notify, &readfds);
>> + memcpy(&timeo, &select_interval, sizeof(struct timeval));
>> +
>> + res = select(notify + 1, &readfds, NULL, NULL, &timeo);
>> +
>> + if (res < 0)
>> + perror("error calling select");
>> +
>> + if (FD_ISSET(notify, &readfds))
>> + parse_events(notify, &watchdirs, &updates);
>> +
>> + handle_events(&updates);
>> + }
>> +}


2013-10-23 14:29:29

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH Version 2 2/3] WIP: Add gsskeyd

Hello.

After read this these recent threads about this, we may or may not end up with this
type of daemon, personally I hope we don't... ;-) But if we do...

On 22/10/13 10:22, [email protected] wrote:
> From: Weston Andros Adamson <[email protected]>
>
> this is an experimental daemon that watches for krb credential cache files
> being created and deleted and creates or removes the mapping in the user
> keyring.
>
> this replaces the wrappers for kinit and kdestroy as they are no longer needed -
> users can go ahead using kinit/kdestroy and existing pam modules.
>
> if this is the way we want to go, this should probably end up as a processes
> forked from gssd and not its own daemon.
>
> Signed-off-by: Weston Andros Adamson <[email protected]>
> ---
> configure.ac | 1 +
> utils/Makefile.am | 2 +-
> utils/gsskeyd/gsskeyd.c | 328 ++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 330 insertions(+), 1 deletion(-)
> create mode 100644 utils/gsskeyd/gsskeyd.c
>
> diff --git a/configure.ac b/configure.ac
> index d3ad854..a8e75ac 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -497,6 +497,7 @@ AC_CONFIG_FILES([
> utils/nfsdcltrack/Makefile
> utils/exportfs/Makefile
> utils/gssd/Makefile
> + utils/gsskeyd/Makefile
> utils/idmapd/Makefile
> utils/mount/Makefile
> utils/mountd/Makefile
> diff --git a/utils/Makefile.am b/utils/Makefile.am
> index b892dc8..943ae2d 100644
> --- a/utils/Makefile.am
> +++ b/utils/Makefile.am
> @@ -14,7 +14,7 @@ OPTDIRS += blkmapd
> endif
>
> if CONFIG_GSS
> -OPTDIRS += gssd
> +OPTDIRS += gssd gsskeyd
> endif
>
> if CONFIG_MOUNT
> diff --git a/utils/gsskeyd/gsskeyd.c b/utils/gsskeyd/gsskeyd.c
> new file mode 100644
> index 0000000..6d598fa
> --- /dev/null
> +++ b/utils/gsskeyd/gsskeyd.c
> @@ -0,0 +1,328 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <limits.h>
> +#include <assert.h>
> +#include <keyutils.h>
> +#include <err.h>
> +#include <errno.h>
> +#include <string.h>
> +
> +#include <sys/inotify.h>
> +#include <sys/errno.h>
> +#include <sys/select.h>
> +#include <sys/param.h>
> +#include <sys/time.h>
> +#include <sys/types.h>
> +#include <sys/wait.h>
> +#include <sys/queue.h>
> +#include <sys/uio.h>
> +#include <unistd.h>
> +
> +#define MAX_EVENT_SIZE (sizeof(struct inotify_event) + NAME_MAX + 1)
> +
> +//#define DEBUG_TRACE
> +
> +#ifdef DEBUG_TRACE
> +#define TRACE(X...) do { fprintf(stderr, "_trace_: " X); } while (0)
> +#else
> +#define TRACE(X...)
> +#endif
Please do not make debugging a compile decision. Go ahead and
use a '-d' or '-vvvv' (which each 'v' add more logging) flags to
make it a run time decision... Having to recompile to debug is
something that never made much sense to me...

Secondly, you please use the existing xlog() interface to do your
error/warning/debugging logging... Having all logging going through
on interface is a good thing... IMHO...

steved.
> +
> +/* .25 second timeouts to select */
> +#define SELECT_INTERVAL_TV { 0, 250000 }
> +
> +/* .5 seconds to collect list of unique file names that had events */
> +#define COLLECT_INTERVAL_TV { 0, 500000 }
> +
> +struct watchdir {
> + char dir[MAXPATHLEN];
> + int wd;
> + LIST_ENTRY(watchdir) entry;
> +};
> +
> +LIST_HEAD(watchdir_list, watchdir);
> +
> +struct update {
> + char dir[MAXPATHLEN];
> + char name[MAXPATHLEN];
> + int is_remove;
> + LIST_ENTRY(update) entry;
> +};
> +
> +LIST_HEAD(update_list, update);
> +
> +struct updates {
> + struct update_list list;
> + struct timeval first;
> +};
> +
> +void
> +watchdir_add(struct watchdir_list *watchdirs, int notify, char *dir)
> +{
> + struct watchdir *w;
> +
> + w = calloc(1, sizeof(struct watchdir));
> + assert(w != NULL);
> +
> + strncpy(w->dir, dir, MAXPATHLEN);
> +
> + w->wd = inotify_add_watch(notify, dir,
> + IN_CREATE | IN_DELETE | IN_MOVED_FROM | IN_MOVED_TO );
> + //| IN_MODIFY
> +
> + if (w->wd < 0)
> + perror("inotify_add_watch");
> +
> + LIST_INSERT_HEAD(watchdirs, w, entry);
> +
> + TRACE("WATCH(%d): %s\n", w->wd, w->dir);
> +}
> +
> +/*
> + * XXX maybe use hash when there is more than one watchdir?
> + */
> +struct watchdir *
> +watchdir_lookup(struct watchdir_list *watchdirs, int wd)
> +{
> + struct watchdir *w;
> +
> + LIST_FOREACH(w, watchdirs, entry) {
> + if (wd == w->wd)
> + return w;
> + }
> +
> + return NULL;
> +}
> +
> +static void
> +update_set_type(struct update *u, struct inotify_event *e)
> +{
> + if (e->mask & (IN_DELETE | IN_MOVED_FROM))
> + u->is_remove = 1;
> + else
> + u->is_remove = 0;
> +}
> +
> +void
> +updates_add(struct updates *updates, struct watchdir_list *watchdirs, struct inotify_event *e)
> +{
> + struct update *u;
> + struct watchdir *w;
> +
> +
> + w = watchdir_lookup(watchdirs, e->wd);
> + assert(w != NULL);
> +
> + TRACE("updates_add: %s/%s\n", w->dir, e->name);
> +
> + LIST_FOREACH(u, &updates->list, entry) {
> + if (strncmp(w->dir, u->dir, MAXPATHLEN) == 0 &&
> + strncmp(e->name, u->name, MAXPATHLEN) == 0) {
> + update_set_type(u, e);
> + return;
> + }
> + }
> +
> + u = calloc(1, sizeof(struct update));
> + strncpy(u->dir, w->dir, MAXPATHLEN);
> + strncpy(u->name, e->name, MAXPATHLEN);
> + update_set_type(u, e);
> +
> + LIST_INSERT_HEAD(&updates->list, u, entry);
> +
> + if (!timerisset(&updates->first)) {
> + if (gettimeofday(&updates->first, NULL) < 0)
> + perror("gettimeofday");
> + }
> +}
> +
> +void
> +print_event_mask(FILE *fp, uint32_t mask)
> +{
> +#define _print_if_found(X, S) do { \
> + if (mask & (X)) { \
> + fprintf(fp, S); \
> + } \
> + } while(0)
> +
> + _print_if_found(IN_ACCESS, "access");
> + _print_if_found(IN_ATTRIB, "attrib");
> + _print_if_found(IN_CLOSE_WRITE, "close write");
> + _print_if_found(IN_CLOSE_NOWRITE, "close nowrite");
> + _print_if_found(IN_CREATE, "create");
> + _print_if_found(IN_DELETE, "delete");
> + _print_if_found(IN_DELETE_SELF, "delete self");
> + _print_if_found(IN_MODIFY, "modify");
> + _print_if_found(IN_MOVE_SELF, "move self");
> + _print_if_found(IN_MOVED_FROM, "moved from");
> + _print_if_found(IN_MOVED_TO, "moved to");
> + _print_if_found(IN_OPEN, "open");
> +}
> +
> +static int
> +manage_gss_ctx_keyring_mapping(struct update *u)
> +{
> + char buf[MAXPATHLEN];
> + int status;
> + uid_t uid;
> + pid_t pid;
> + key_serial_t key;
> + long res;
> +
> + snprintf(buf, MAXPATHLEN, "%s/%s", u->dir, u->name);
> +
> + if (sscanf(u->name, "krb5cc_%u", &uid) <= 0)
> + perror("parsing krb5cc uid");
> +
> + if ((pid = fork()) < 0)
> + perror("fork");
> +
> + if (pid == 0) {
> + if (setuid(uid) < 0)
> + perror("setuid");
> +
> + if (u->is_remove) {
> + fprintf(stderr, "remove gss ctx mapping for uid %u\n",
> + uid);
> + key = request_key("gss-ctx", "_nfstgt_", NULL,
> + KEY_SPEC_USER_KEYRING);
> + if (key > 0) {
> + res = keyctl_unlink(key, KEY_SPEC_USER_KEYRING);
> + if (res < 0)
> + warn("keyctl_unlink failed");
> + } else
> + warn("request_key failed");
> + } else {
> + fprintf(stderr, "add gss ctx mapping for uid=%u\n",
> + uid);
> + snprintf(buf, MAXPATHLEN, "FILE:%s/%s",
> + u->dir, u->name);
> + key = add_key("gss-ctx", "_nfstgt_", buf,
> + MAXPATHLEN, KEY_SPEC_USER_KEYRING);
> +
> + if (key < 0)
> + warn("add_key failed");
> + }
> +
> + exit(0);
> + } else {
> + waitpid(pid, &status, 0);
> +
> + if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
> + return WEXITSTATUS(status);
> + }
> + }
> +
> + return 0;
> +}
> +
> +void
> +parse_events(int notify, struct watchdir_list *watchdirs, struct updates *updates)
> +{
> + ssize_t nread, pos;
> + unsigned char buf[MAX_EVENT_SIZE * 100];
> + struct inotify_event *e;
> +
> + for (;;) {
> + nread = read(notify, buf, sizeof(buf));
> +
> + if (nread <= 0) {
> + TRACE("read returns %d, errno %u\n", nread, errno);
> + break;
> + }
> +
> + TRACE("Read %d bytes\n", nread);
> +
> + pos = 0;
> + while (pos < nread) {
> + assert((nread - pos) >=
> + (ssize_t)sizeof(struct inotify_event));
> + e = (struct inotify_event *)(buf + pos);
> + pos += sizeof(struct inotify_event) + e->len;
> +
> +#ifdef DEBUG_TRACE
> + TRACE("EVENT on %s: ", e->name);
> + print_event_mask(stderr, e->mask);
> + fprintf(stderr, "\n");
> +#endif
> +
> + if (strlen(e->name) >= strlen("krb5cc_") &&
> + strncmp("krb5cc_", e->name, strlen("krb5cc_")) != 0)
> + TRACE("skip file: %s\n", e->name);
> + else
> + updates_add(updates, watchdirs, e);
> + }
> + assert(pos == nread);
> + }
> +}
> +
> +void
> +handle_events(struct updates *updates)
> +{
> + struct update *u;
> + struct timeval now, diff, collection_interval = COLLECT_INTERVAL_TV;
> +
> + if (!timerisset(&updates->first))
> + return;
> +
> + if (gettimeofday(&now, NULL) < 0)
> + perror("gettimeofday");
> +
> + timersub(&now, &updates->first, &diff);
> +
> + if (!(diff.tv_sec >= collection_interval.tv_sec &&
> + diff.tv_usec >= collection_interval.tv_usec)) {
> + TRACE("it's been %u s %u ms - waiting longer...\n",
> + diff.tv_sec, diff.tv_usec);
> + return;
> + }
> +
> + while (!LIST_EMPTY(&updates->list)) {
> + u = updates->list.lh_first;
> +
> + TRACE("handle krb5 cc file: %s\n", u->name);
> + manage_gss_ctx_keyring_mapping(u);
> + LIST_REMOVE(u, entry);
> + }
> +
> + timerclear(&updates->first);
> +}
> +
> +
> +int
> +main(int argc, char **argv)
> +{
> + fd_set readfds;
> + int notify;
> + int res;
> + struct timeval timeo, select_interval = SELECT_INTERVAL_TV;
> + struct watchdir_list watchdirs;
> + struct updates updates;
> +
> + LIST_INIT(&watchdirs);
> +
> + notify = inotify_init1(IN_NONBLOCK);
> + if (notify < 0)
> + perror("inotify_init1");
> +
> + watchdir_add(&watchdirs, notify, "/tmp");
> +
> + LIST_INIT(&updates.list);
> + timerclear(&updates.first);
> +
> + for (;;) {
> + FD_ZERO(&readfds);
> + FD_SET(notify, &readfds);
> + memcpy(&timeo, &select_interval, sizeof(struct timeval));
> +
> + res = select(notify + 1, &readfds, NULL, NULL, &timeo);
> +
> + if (res < 0)
> + perror("error calling select");
> +
> + if (FD_ISSET(notify, &readfds))
> + parse_events(notify, &watchdirs, &updates);
> +
> + handle_events(&updates);
> + }
> +}
>

2013-11-20 21:21:09

by Adamson, Andy

[permalink] [raw]
Subject: Re: [PATCH Version 2 0/3] GSSD: Use gss-ctx keys and gsskeyd to sync Kerberos credentials and kernel gss_contexts.


On Nov 20, 2013, at 3:49 PM, Simo Sorce <[email protected]>
wrote:

> On Wed, 2013-11-20 at 20:35 +0000, Adamson, Andy wrote:
>> Hi
>>
>> As suggested, I approached the [email protected] about the possibility
>> of a plugin architecture for libkrb5 credential cache manipulation
>> functions so that we could trigger the kernel GSS_context management
>> functions.
>
> [..]
>>
>> It appears that Solution 4: [plugin architecture to libkrb for
>> callouts on functions that manipulate kerberos credentials.] is a
>> no-go.
>>
>> I agree that Solution 1: [inotify on FILE credentials] is clunky and
>> won't work well.
>>
>> Solution 2: [integrate with KEYRING credentials] could work if we
>> insist that all NFS Kerberos credentials use the KEYRING: - e.g. the
>> proposed new 'big-key' type. Note there is no backporting of this
>> solution.
>
> Note that solution 2 is semantically identical to solution 4, you are
> going to try to guess what user space is doing, based on how it
> manipulates caches, and would have the same side effects.
>
>> I think Solution 3: [nfslog/nfslogout interfaces invoked from PAM or
>> other login system facility] is a good way to go. Note that a PAM
>> based solution where in the PAM would get us most of the way towards
>> providing users with a way to login and logout of NFS kerberized
>> shares.
>>
>> Comments on an NFS PAM that will destroy GSS context for a UID upon
>> logout?
>
> I prefer 3 too, let it to the login system (whether PAM based or other)
> to determine when it is time to destroy credentials, that's the only
> component that have a chance of guessing right.
> Of course you could also provide a user utility to force a purge.

Good.

>
>> Simo - I answered your latest comments below in-line.
>
> [..]
>
>> I think a PAM based solution will get us most of the way there.
>
> Agree.
>
>
>>> So the way I see it you probably want to keep the tracking in
>>> whatever tool you want to use to experiment (say gsskeyd) and only
>>> provide a downcall to the kernel that will tell it: destroy any
>>> cache for 'uid number (optionally pass in a session id too ?)'.
>>
>> This is what the gss-ctx keyring destroy method is - a downcall to the
>> kernel telling it to destroy all GSS_contexts for a UID.
>
> Why do you need to abuse the keyring interface to implement a syscall ?

Do you think this job requires a system call?

I felt the keyring interface gives us what we want: a way to pass some information and trigger a behavior. The gss-ctx key caches the relationship between the principals Kerberos credentials and the associated RPC layer gss_cred (which in turn has the gss_context). When a gss-ctx key exists, the principal associated with the UID has kerberos credentials (located in the Kerberos ccache stored in the key) that are used for NFS, and the key serial is stored in the gss_cred.

>From kernel documentation: Documentation/security/keys.txt

"This service allows cryptographic keys, authentication tokens, cross-domain
user mappings, and similar to be cached in the kernel for the use of
filesystems and other kernel services."

I think the gss-ctx key fits into the above description (under "similar"), so I don't think of this use as an abuse.

> Or did I misunderstand what you mean by "telling the kernel to do X" ?
>
>>> This way you can replace the logic of how to keep track of what is
>>> going on completely in user space, where it can easily be adjusted
>>> and adapted as experience is gained.
>>>
>>> IE: track it completely in userspace for now and only provide a
>>> syscall to kill creds per UID, no tracking on the kernel side.
>>
>> Yes - I believe that is what the gss-ctx keyring code I wrote does.
>
> A keyring is not a syscall.

Yes, but it does as you suggested "provide a downcall to the kernel that will tell it: destroy any
cache for 'uid number"

What is the disadvantage of using the keyring?

--Andy

>
> Simo.
>
> --
> Simo Sorce * Red Hat, Inc * New York
>


2013-11-21 13:36:07

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH Version 2 0/3] GSSD: Use gss-ctx keys and gsskeyd to sync Kerberos credentials and kernel gss_contexts.



On 20/11/13 15:49, Simo Sorce wrote:
>> I think Solution 3: [nfslog/nfslogout interfaces invoked from PAM or
>> > other login system facility] is a good way to go. Note that a PAM
>> > based solution where in the PAM would get us most of the way towards
>> > providing users with a way to login and logout of NFS kerberized
>> > shares.
>> >
>> > Comments on an NFS PAM that will destroy GSS context for a UID upon
>> > logout?
> I prefer 3 too, let it to the login system (whether PAM based or other)
> to determine when it is time to destroy credentials, that's the only
> component that have a chance of guessing right.
> Of course you could also provide a user utility to force a purge.
>
+1 for me on this options as well...

But how is it known when somebody logs out? Is that PAM-able as well?

steved.

2013-11-22 21:28:48

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH Version 2 0/3] GSSD: Use gss-ctx keys and gsskeyd to sync Kerberos credentials and kernel gss_contexts.


On Nov 22, 2013, at 14:11, Simo Sorce <[email protected]> wrote:

> On Thu, 2013-11-21 at 08:37 -0500, Steve Dickson wrote:
>>
>> On 20/11/13 15:49, Simo Sorce wrote:
>>>> I think Solution 3: [nfslog/nfslogout interfaces invoked from PAM or
>>>>> other login system facility] is a good way to go. Note that a PAM
>>>>> based solution where in the PAM would get us most of the way towards
>>>>> providing users with a way to login and logout of NFS kerberized
>>>>> shares.
>>>>>
>>>>> Comments on an NFS PAM that will destroy GSS context for a UID upon
>>>>> logout?
>>> I prefer 3 too, let it to the login system (whether PAM based or other)
>>> to determine when it is time to destroy credentials, that's the only
>>> component that have a chance of guessing right.

Really? How do you deal with backgrounded tasks?

>>> Of course you could also provide a user utility to force a purge.
>>>
>> +1 for me on this options as well...
>>
>> But how is it known when somebody logs out? Is that PAM-able as well?
>
> I would say "login process" more than pam, but the basic idea is the
> same, a user space program that knows when the user is logging out for
> good and is privileged enough to go an tell the kernel to nuke creds.

What?s such a process going to use as an indicator that the user is ?logging out for good??

Trond

2013-11-20 21:24:48

by Adamson, Andy

[permalink] [raw]
Subject: Re: [PATCH Version 2 0/3] GSSD: Use gss-ctx keys and gsskeyd to sync Kerberos credentials and kernel gss_contexts.


On Nov 20, 2013, at 4:21 PM, "Adamson, Andy" <[email protected]>
wrote:

>
> On Nov 20, 2013, at 3:49 PM, Simo Sorce <[email protected]>
> wrote:
>
>> On Wed, 2013-11-20 at 20:35 +0000, Adamson, Andy wrote:
>>> Hi
>>>
>>> As suggested, I approached the [email protected] about the possibility
>>> of a plugin architecture for libkrb5 credential cache manipulation
>>> functions so that we could trigger the kernel GSS_context management
>>> functions.
>>
>> [..]
>>>
>>> It appears that Solution 4: [plugin architecture to libkrb for
>>> callouts on functions that manipulate kerberos credentials.] is a
>>> no-go.
>>>
>>> I agree that Solution 1: [inotify on FILE credentials] is clunky and
>>> won't work well.
>>>
>>> Solution 2: [integrate with KEYRING credentials] could work if we
>>> insist that all NFS Kerberos credentials use the KEYRING: - e.g. the
>>> proposed new 'big-key' type. Note there is no backporting of this
>>> solution.
>>
>> Note that solution 2 is semantically identical to solution 4, you are
>> going to try to guess what user space is doing, based on how it
>> manipulates caches, and would have the same side effects.
>>
>>> I think Solution 3: [nfslog/nfslogout interfaces invoked from PAM or
>>> other login system facility] is a good way to go. Note that a PAM
>>> based solution where in the PAM would get us most of the way towards
>>> providing users with a way to login and logout of NFS kerberized
>>> shares.
>>>
>>> Comments on an NFS PAM that will destroy GSS context for a UID upon
>>> logout?
>>
>> I prefer 3 too, let it to the login system (whether PAM based or other)
>> to determine when it is time to destroy credentials, that's the only
>> component that have a chance of guessing right.
>> Of course you could also provide a user utility to force a purge.
>
> Good.
>
>>
>>> Simo - I answered your latest comments below in-line.
>>
>> [..]
>>
>>> I think a PAM based solution will get us most of the way there.
>>
>> Agree.
>>
>>
>>>> So the way I see it you probably want to keep the tracking in
>>>> whatever tool you want to use to experiment (say gsskeyd) and only
>>>> provide a downcall to the kernel that will tell it: destroy any
>>>> cache for 'uid number (optionally pass in a session id too ?)'.
>>>
>>> This is what the gss-ctx keyring destroy method is - a downcall to the
>>> kernel telling it to destroy all GSS_contexts for a UID.
>>
>> Why do you need to abuse the keyring interface to implement a syscall ?
>
> Do you think this job requires a system call?
>
> I felt the keyring interface gives us what we want: a way to pass some information and trigger a behavior. The gss-ctx key caches the relationship between the principals Kerberos credentials and the associated RPC layer gss_cred (which in turn has the gss_context). When a gss-ctx key exists, the principal associated with the UID has kerberos credentials (located in the Kerberos ccache

this is confusing: the _location_ of the KRB5 ccache is stored in the gss-ctxkey, not the actual creds...

> stored in the key) that are used for NFS, and the key serial is stored in the gss_cred.
>
> From kernel documentation: Documentation/security/keys.txt
>
> "This service allows cryptographic keys, authentication tokens, cross-domain
> user mappings, and similar to be cached in the kernel for the use of
> filesystems and other kernel services."
>
> I think the gss-ctx key fits into the above description (under "similar"), so I don't think of this use as an abuse.
>
>> Or did I misunderstand what you mean by "telling the kernel to do X" ?
>>
>>>> This way you can replace the logic of how to keep track of what is
>>>> going on completely in user space, where it can easily be adjusted
>>>> and adapted as experience is gained.
>>>>
>>>> IE: track it completely in userspace for now and only provide a
>>>> syscall to kill creds per UID, no tracking on the kernel side.
>>>
>>> Yes - I believe that is what the gss-ctx keyring code I wrote does.
>>
>> A keyring is not a syscall.
>
> Yes, but it does as you suggested "provide a downcall to the kernel that will tell it: destroy any
> cache for 'uid number"
>
> What is the disadvantage of using the keyring?
>
> --Andy
>
>>
>> Simo.
>>
>> --
>> Simo Sorce * Red Hat, Inc * New York
>>
>


2013-11-22 20:44:59

by Adamson, Andy

[permalink] [raw]
Subject: Re: [PATCH Version 2 0/3] GSSD: Use gss-ctx keys and gsskeyd to sync Kerberos credentials and kernel gss_contexts.


On Nov 22, 2013, at 2:09 PM, Simo Sorce <[email protected]>
wrote:

> On Wed, 2013-11-20 at 21:21 +0000, Adamson, Andy wrote:
>
>>> Why do you need to abuse the keyring interface to implement a syscall ?
>>
>> Do you think this job requires a system call?
>>
>> I felt the keyring interface gives us what we want: a way to pass some
>> information and trigger a behavior. The gss-ctx key caches the
>> relationship between the principals Kerberos credentials and the
>> associated RPC layer gss_cred (which in turn has the gss_context).
>> When a gss-ctx key exists, the principal associated with the UID has
>> kerberos credentials (located in the Kerberos ccache stored in the
>> key) that are used for NFS, and the key serial is stored in the
>> gss_cred.
>>
>> From kernel documentation: Documentation/security/keys.txt
>>
>> "This service allows cryptographic keys, authentication tokens, cross-domain
>> user mappings, and similar to be cached in the kernel for the use of
>> filesystems and other kernel services."
>>
>> I think the gss-ctx key fits into the above description (under
>> "similar"), so I don't think of this use as an abuse.
>
> You walk a fine line here.
>
> IMHO the kernel keyring is a place where you store information. It is
> true that based on the information you store there you can take
> additional actions (in the user space usually). But I think that the
> gss-ctx usage is overstepping the boundary and using the presence of a
> key as a trigger to perform actions in the kernel. This is something
> that is traditionally done via a syscall.
>
> My main concern is around things like access control, how is that
> performed ?
>
> Also I think it introduces a lot more code than is needed for the
> simplest implementation.
> The simplest implementation in my mind is that the login system can call
> the syscall no matter what the current status in the kernel and ask the
> nfs client subsystem to nuke any credentials associated to uid 1234
> whether that uid has any credentials on the system or not. So it allows
> to ignore completely any real tracking and just simply be sure each and
> all creds associated to that user id are nuked.
> this could actually be extended beyond NFS, ideally any kernel driver
> that stores some form of credential or data associated to a specific UID
> may hook to the syscall to be notified when it is time to blast away the
> data.
>
> Your model seem more fragile in the sense that you need to set up the
> whole tracking part to, so if you have a bug there you'll end up not
> removing creds from the kernel. It is also very specific to the NFS
> client, but that may or may not be an issue.

OK - I'll look into a syscall.

-->Andy

>
>>> Or did I misunderstand what you mean by "telling the kernel to do X" ?
>>>
>>>>> This way you can replace the logic of how to keep track of what is
>>>>> going on completely in user space, where it can easily be adjusted
>>>>> and adapted as experience is gained.
>>>>>
>>>>> IE: track it completely in userspace for now and only provide a
>>>>> syscall to kill creds per UID, no tracking on the kernel side.
>>>>
>>>> Yes - I believe that is what the gss-ctx keyring code I wrote does.
>>>
>>> A keyring is not a syscall.
>>
>> Yes, but it does as you suggested "provide a downcall to the kernel that will tell it: destroy any
>> cache for 'uid number"
>>
>> What is the disadvantage of using the keyring?
>
> Hopefully I answered above.
>
> Simo.
>
> --
> Simo Sorce * Red Hat, Inc * New York
>


2013-11-20 20:49:50

by Simo Sorce

[permalink] [raw]
Subject: Re: [PATCH Version 2 0/3] GSSD: Use gss-ctx keys and gsskeyd to sync Kerberos credentials and kernel gss_contexts.

On Wed, 2013-11-20 at 20:35 +0000, Adamson, Andy wrote:
> Hi
>
> As suggested, I approached the [email protected] about the possibility
> of a plugin architecture for libkrb5 credential cache manipulation
> functions so that we could trigger the kernel GSS_context management
> functions.

[..]
>
> It appears that Solution 4: [plugin architecture to libkrb for
> callouts on functions that manipulate kerberos credentials.] is a
> no-go.
>
> I agree that Solution 1: [inotify on FILE credentials] is clunky and
> won't work well.
>
> Solution 2: [integrate with KEYRING credentials] could work if we
> insist that all NFS Kerberos credentials use the KEYRING: - e.g. the
> proposed new 'big-key' type. Note there is no backporting of this
> solution.

Note that solution 2 is semantically identical to solution 4, you are
going to try to guess what user space is doing, based on how it
manipulates caches, and would have the same side effects.

> I think Solution 3: [nfslog/nfslogout interfaces invoked from PAM or
> other login system facility] is a good way to go. Note that a PAM
> based solution where in the PAM would get us most of the way towards
> providing users with a way to login and logout of NFS kerberized
> shares.
>
> Comments on an NFS PAM that will destroy GSS context for a UID upon
> logout?

I prefer 3 too, let it to the login system (whether PAM based or other)
to determine when it is time to destroy credentials, that's the only
component that have a chance of guessing right.
Of course you could also provide a user utility to force a purge.

> Simo - I answered your latest comments below in-line.

[..]

> I think a PAM based solution will get us most of the way there.

Agree.


> > So the way I see it you probably want to keep the tracking in
> > whatever tool you want to use to experiment (say gsskeyd) and only
> > provide a downcall to the kernel that will tell it: destroy any
> > cache for 'uid number (optionally pass in a session id too ?)'.
>
> This is what the gss-ctx keyring destroy method is - a downcall to the
> kernel telling it to destroy all GSS_contexts for a UID.

Why do you need to abuse the keyring interface to implement a syscall ?
Or did I misunderstand what you mean by "telling the kernel to do X" ?

> > This way you can replace the logic of how to keep track of what is
> > going on completely in user space, where it can easily be adjusted
> > and adapted as experience is gained.
> >
> > IE: track it completely in userspace for now and only provide a
> > syscall to kill creds per UID, no tracking on the kernel side.
>
> Yes - I believe that is what the gss-ctx keyring code I wrote does.

A keyring is not a syscall.

Simo.

--
Simo Sorce * Red Hat, Inc * New York


2013-11-20 20:35:35

by Adamson, Andy

[permalink] [raw]
Subject: Re: [PATCH Version 2 0/3] GSSD: Use gss-ctx keys and gsskeyd to sync Kerberos credentials and kernel gss_contexts.

Hi

As suggested, I approached the [email protected] about the possibility of a plugin architecture for libkrb5 credential cache manipulation functions so that we could trigger the kernel GSS_context management functions.

On Nov 19, 2013, at 2:46 PM, Greg Hudson <[email protected]> wrote:

> On 11/15/2013 12:12 PM, Adamson, Andy wrote:
>> Solution 1: [inotify on FILE credentials]
>> Solution 2: [integrate with KEYRING credentials]
>> Solution 3: [nfslog/nfslogout interfaces invoked from PAM or other login system facility]
>> Solution 4: AFAICS the most versatile solution is to add a plugin architecture to libkrb for call
>> outs on functions that manipulate kerberos credentials.
>
> I agree that solution 1 isn't great.
>
> I have concerns about solution 4. Kerberos credential caches can be
> used for several different purposes; they aren't only used to store
> login credentials. For instance, a user could run a server process
> which receives delegated credentials from a client, or could run kadmin
> and get credentials for username/admin to administer the realm's KDB.
> Notifying the kernel any time any credential cache is destroyed would
> create a lot of false positives.
>
> I would be happy to have a pluggable interface which allows for
> implementations of new ccache types, but I don't think I would welcome a
> hook-style interface which causes ccache operations to have arbitrary
> side effects beyond changing the ccache.
>
> I don't know enough about the Linux kernel to comment on whether
> solution 2 is viable. Solution 3 is obviously viable in the sense that
> we're used to it with AFS.
>


It appears that Solution 4: [plugin architecture to libkrb for callouts on functions that manipulate kerberos credentials.] is a no-go.

I agree that Solution 1: [inotify on FILE credentials] is clunky and won't work well.

Solution 2: [integrate with KEYRING credentials] could work if we insist that all NFS Kerberos credentials use the KEYRING: - e.g. the proposed new 'big-key' type. Note there is no backporting of this solution.

I think Solution 3: [nfslog/nfslogout interfaces invoked from PAM or other login system facility] is a good way to go. Note that a PAM based solution where in the PAM would get us most of the way towards providing users with a way to login and logout of NFS kerberized shares.

Comments on an NFS PAM that will destroy GSS context for a UID upon logout?

Simo - I answered your latest comments below in-line.

On Oct 22, 2013, at 1:25 PM, Simo Sorce <[email protected]> wrote:

> On Tue, 2013-10-22 at 17:00 +0000, Adamson, Andy wrote:
>> On Oct 22, 2013, at 12:09 PM, Simo Sorce <[email protected]>
>> wrote:
>>
>>> On Tue, 2013-10-22 at 15:32 +0000, Adamson, Andy wrote:
>>>> On Oct 22, 2013, at 11:02 AM, Simo Sorce <[email protected]> wrote:
>>>>
>>>>> On Tue, 2013-10-22 at 10:22 -0400, [email protected] wrote:
>>>>>> From: Andy Adamson <[email protected]>
>>>>>>
>>>>>> This is an RFC patchset, which will be used for testing.
>>>>>>
>>>>>> This patchset requires the "SUNRPC: destroy gss_cred and context on Kerberos credential destruction" kernel patchset.
>>>>>>
>>>>>> We need to do a lot of testing to ensure that once kdestroy and gss-ctx
>>>>>> gss_user_destroy is called, all existing buffered
>>>>>> writes using the 'destroyed gss credential + context' are serviced.
>>>>>>
>>>>>> Differences from version 1:
>>>>>>
>>>>>> - moved from nfstgt_login and nfstgt_logout to gsskeyd.
>>>>>> - gsskeyd automatically creates gss-ctx key on kinit and destroys the gss-ctx
>>>>>> key on kdestroy.
>>>>>>
>>>>>> gsskeyd will need to act differently for different krb5 credential caches.
>>>>>
>>>>> Why ?
>>>>
>>>> Just need different parsing to know which directory to poll.
>>>>>
>>>>>> For example, some versions of gssd store FILE credentials in FILE:/tmp/krb5cc_<UID>
>>>>>> while this code, written for fedora 19 uses FILE:/run/user/<UID>/krb5cc/tgt.
>>>>>
>>>>> This is incorrect, Fedora 19 use DIR type ccaches, so you should
>>>>> reference a cache withing the dir type, which will notmally be something
>>>>> like: DIR:/run/user/<uidnumber>/krb5cc for the whole collection or
>>>>> DIR::/run/user/<uidnumber>/krb5cc/txtXXXXXX for the sepcific cache.
>>>>>
>>>>> However note that due to issues with DIR type caches we are moving to a
>>>>> new KEYRING based type of cache in F20, so assuming any specific type of
>>>>> cache is a dead start.
>>>>
>>>> But I'm not assuming any kind of kerberos credential cache! I just coded to one type for this RFC.
>>>> Didn't you see my KEYRING based ccache comment below?
>>>
>>> Ah sorry, I had not noticed.
>>>
>>>>>> As Trond suggested, if we keep gsskeyd separate from gssd, we could set up a
>>>>>> configuration file along the lines of the keytools' request-key.conf file to
>>>>>> allow both NFS and CIFS (and other filesystems) to install plugin handlers
>>>>>> for kinit/kdestroy events.
>>>>>
>>>>> Given in many cases (the desirable ones at least) kerberos credentials
>>>>> are not inited via kinit, but rather by things like pam_krb5, sssd, or
>>>>> directly imported via sshd, I am trying to understand how you are going
>>>>> to address the majority of these cases.
>>>>
>>>> All of these cases use a kerberos credential cache which if is of type
>>>> FILE the gsskeyd can poll inotify and if it is of type KEY: then
>>>> gsskeyd adds the gss-ctx key to the Kerberos keyring (that is the
>>>> theory :)) so the same code services kinit/pam_krb5
>>>
>>> You wouldn't be able to see it if I set a custom ccache file though
>>> export KRB5CCNAME=FILE:/random/sir/foofile
>>>
>>>>> Should users put gsskeyd in their .profile/.bashrc files or something ?
>>>>>
>>>>>> Else, we could have gssd be the process to poll inotify (given
>>>>>> that it already polls rpc_pipefs) and then just have it fork off the
>>>>>> subprocess if and when it sees an interesting event.
>>>>>
>>>>> What is an 'interesting' event ?
>>>>
>>>> TGT (used for NFS TGS) creation/destruction
>>>
>>> Shouldn't you care for the specific NFS ticket only ?
>>> Although kdestroy is quite blunt, in theory users can simply destroy the
>>> specific NFS ticket and keep their TGT.
>>
>> AFAICS MIT kdestroy does not let you specifiy which credentials to
>> destroy. It simply destroys the whole specified ccache, TGT and all.
>>
>> I think Heimdal kdestroy lets you specify...
>>
>> WRT NFS: As long as the TGT has not expired, the kernel will upcall
>> and GSSD will get a new TGS. So, the kernel gss_context lifetime ==
>> TGT lifetime.
>>
>> The solution with the current MIT kdestroy is to have a NFS-only
>> kerberos credentail cache - with it's own TGT and only NFS TGS's.
>
> There isn't just kdestroy, applications can use libkrb5 to manipulate
> the ccache if needed. I am just pointing out that there are many ways
> ccaches are manipulated in real life.
>
> I think it is valuable to be able to invalidate the credentials in the
> kernel when the user really wants it, I just think that guessing when
> the user/admin wants you to drop the creds based on ccaches interactions
> is probably not going to work very well.

I think a PAM based solution will get us most of the way there.

>
> So the way I see it you probably want to keep the tracking in whatever
> tool you want to use to experiment (say gsskeyd) and only provide a
> downcall to the kernel that will tell it: destroy any cache for 'uid
> number (optionally pass in a session id too ?)'.

This is what the gss-ctx keyring destroy method is - a downcall to the kernel telling it to destroy all GSS_contexts for a UID.

>
> This way you can replace the logic of how to keep track of what is going
> on completely in user space, where it can easily be adjusted and adapted
> as experience is gained.
>
> IE: track it completely in userspace for now and only provide a syscall
> to kill creds per UID, no tracking on the kernel side.

Yes - I believe that is what the gss-ctx keyring code I wrote does.

-->Andy

>
> Simo.
>
> --
> Simo Sorce * Red Hat, Inc * New York
>


2013-11-22 21:39:35

by Simo Sorce

[permalink] [raw]
Subject: Re: [PATCH Version 2 0/3] GSSD: Use gss-ctx keys and gsskeyd to sync Kerberos credentials and kernel gss_contexts.

On Fri, 2013-11-22 at 16:28 -0500, Trond Myklebust wrote:
> On Nov 22, 2013, at 14:11, Simo Sorce <[email protected]> wrote:
>
> > On Thu, 2013-11-21 at 08:37 -0500, Steve Dickson wrote:
> >>
> >> On 20/11/13 15:49, Simo Sorce wrote:
> >>>> I think Solution 3: [nfslog/nfslogout interfaces invoked from PAM or
> >>>>> other login system facility] is a good way to go. Note that a PAM
> >>>>> based solution where in the PAM would get us most of the way towards
> >>>>> providing users with a way to login and logout of NFS kerberized
> >>>>> shares.
> >>>>>
> >>>>> Comments on an NFS PAM that will destroy GSS context for a UID upon
> >>>>> logout?
> >>> I prefer 3 too, let it to the login system (whether PAM based or other)
> >>> to determine when it is time to destroy credentials, that's the only
> >>> component that have a chance of guessing right.
>
> Really? How do you deal with backgrounded tasks?

for background tasks you still need to keep your kerberos credentials
around somehow, if you are doing that then the kernel will be able to
get a new session key if needed.
If you instruct your system to destroy everything then background tasks
will fail. In any case it needs to be a user space decision, the kernel
can't guess.

> >>> Of course you could also provide a user utility to force a purge.
> >>>
> >> +1 for me on this options as well...
> >>
> >> But how is it known when somebody logs out? Is that PAM-able as well?
> >
> > I would say "login process" more than pam, but the basic idea is the
> > same, a user space program that knows when the user is logging out for
> > good and is privileged enough to go an tell the kernel to nuke creds.
>
> What’s such a process going to use as an indicator that the user is “logging out for good”?

If you do logout from a graphical login manager that's a pretty good
indication I would say. Whether that will work for all use cases is
debatable of course, I am sure cases can be found where that will not
the right option.

On a system where it is too difficult to properly assess automatically
when a user is logged out for good the only option is to disable any
automatism and let the user call a cmdline tool.

Simo.

--
Simo Sorce * Red Hat, Inc * New York


2013-11-22 19:09:41

by Simo Sorce

[permalink] [raw]
Subject: Re: [PATCH Version 2 0/3] GSSD: Use gss-ctx keys and gsskeyd to sync Kerberos credentials and kernel gss_contexts.

On Wed, 2013-11-20 at 21:21 +0000, Adamson, Andy wrote:

> > Why do you need to abuse the keyring interface to implement a syscall ?
>
> Do you think this job requires a system call?
>
> I felt the keyring interface gives us what we want: a way to pass some
> information and trigger a behavior. The gss-ctx key caches the
> relationship between the principals Kerberos credentials and the
> associated RPC layer gss_cred (which in turn has the gss_context).
> When a gss-ctx key exists, the principal associated with the UID has
> kerberos credentials (located in the Kerberos ccache stored in the
> key) that are used for NFS, and the key serial is stored in the
> gss_cred.
>
> From kernel documentation: Documentation/security/keys.txt
>
> "This service allows cryptographic keys, authentication tokens, cross-domain
> user mappings, and similar to be cached in the kernel for the use of
> filesystems and other kernel services."
>
> I think the gss-ctx key fits into the above description (under
> "similar"), so I don't think of this use as an abuse.

You walk a fine line here.

IMHO the kernel keyring is a place where you store information. It is
true that based on the information you store there you can take
additional actions (in the user space usually). But I think that the
gss-ctx usage is overstepping the boundary and using the presence of a
key as a trigger to perform actions in the kernel. This is something
that is traditionally done via a syscall.

My main concern is around things like access control, how is that
performed ?

Also I think it introduces a lot more code than is needed for the
simplest implementation.
The simplest implementation in my mind is that the login system can call
the syscall no matter what the current status in the kernel and ask the
nfs client subsystem to nuke any credentials associated to uid 1234
whether that uid has any credentials on the system or not. So it allows
to ignore completely any real tracking and just simply be sure each and
all creds associated to that user id are nuked.
this could actually be extended beyond NFS, ideally any kernel driver
that stores some form of credential or data associated to a specific UID
may hook to the syscall to be notified when it is time to blast away the
data.

Your model seem more fragile in the sense that you need to set up the
whole tracking part to, so if you have a bug there you'll end up not
removing creds from the kernel. It is also very specific to the NFS
client, but that may or may not be an issue.

> > Or did I misunderstand what you mean by "telling the kernel to do X" ?
> >
> >>> This way you can replace the logic of how to keep track of what is
> >>> going on completely in user space, where it can easily be adjusted
> >>> and adapted as experience is gained.
> >>>
> >>> IE: track it completely in userspace for now and only provide a
> >>> syscall to kill creds per UID, no tracking on the kernel side.
> >>
> >> Yes - I believe that is what the gss-ctx keyring code I wrote does.
> >
> > A keyring is not a syscall.
>
> Yes, but it does as you suggested "provide a downcall to the kernel that will tell it: destroy any
> cache for 'uid number"
>
> What is the disadvantage of using the keyring?

Hopefully I answered above.

Simo.

--
Simo Sorce * Red Hat, Inc * New York


2013-11-22 19:11:42

by Simo Sorce

[permalink] [raw]
Subject: Re: [PATCH Version 2 0/3] GSSD: Use gss-ctx keys and gsskeyd to sync Kerberos credentials and kernel gss_contexts.

On Thu, 2013-11-21 at 08:37 -0500, Steve Dickson wrote:
>
> On 20/11/13 15:49, Simo Sorce wrote:
> >> I think Solution 3: [nfslog/nfslogout interfaces invoked from PAM or
> >> > other login system facility] is a good way to go. Note that a PAM
> >> > based solution where in the PAM would get us most of the way towards
> >> > providing users with a way to login and logout of NFS kerberized
> >> > shares.
> >> >
> >> > Comments on an NFS PAM that will destroy GSS context for a UID upon
> >> > logout?
> > I prefer 3 too, let it to the login system (whether PAM based or other)
> > to determine when it is time to destroy credentials, that's the only
> > component that have a chance of guessing right.
> > Of course you could also provide a user utility to force a purge.
> >
> +1 for me on this options as well...
>
> But how is it known when somebody logs out? Is that PAM-able as well?

I would say "login process" more than pam, but the basic idea is the
same, a user space program that knows when the user is logging out for
good and is privileged enough to go an tell the kernel to nuke creds.

Simo.

--
Simo Sorce * Red Hat, Inc * New York