linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/6] gssd: add the GSSAPI acceptor name to the info passed in downcall
@ 2014-04-15 15:19 Jeff Layton
  2014-04-15 15:19 ` [PATCH v2 1/6] gssd: handle malloc failure appropriately in do_downcall Jeff Layton
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Jeff Layton @ 2014-04-15 15:19 UTC (permalink / raw)
  To: steved; +Cc: linux-nfs

v2:
- add patch to reset lifetime_rec if gss_inquire_context fails
- ensure that we always send the length in the downcall, even if
  there is no acceptor string.
- comment and error handling fixups (primarily in last patch)

Recently, I started a mailing list thread about some authentication
failures that I was seeing on the callback channel when krb5 was in use.

After a bit of discussion we determined that the right way to fix it
was to save off the GSSAPI acceptor name used in the SETCLIENT call,
and then ensure that the same principal is used in callback requests.

This patchset is the userland portion of that change. It basically
just adds the acceptor name to the downcall, immediately following
the context token. Older kernel will just ignore this data, so this
should be safe.

There is also a companion kernel patchset that will allow the kernel
to save off this info for later usage.

Jeff Layton (6):
  gssd: handle malloc failure appropriately in do_downcall
  gssd: make do_downcall a void return
  gssd: move hostbased name routines into separate file
  gssd: add new routine for generating a hostbased principal in a
    gss_buffer_t
  gssd: explicitly set lifetime_rec to 0 when gss_inquire_context fails
  gssd: scrape the acceptor name out of the context

 utils/gssd/Makefile.am    |   2 +
 utils/gssd/gss_names.c    | 138 ++++++++++++++++++++++++++++++++++++++++++++++
 utils/gssd/gss_names.h    |  36 ++++++++++++
 utils/gssd/gssd_proc.c    |  53 ++++++++++++------
 utils/gssd/svcgssd_proc.c |  66 +---------------------
 5 files changed, 214 insertions(+), 81 deletions(-)
 create mode 100644 utils/gssd/gss_names.c
 create mode 100644 utils/gssd/gss_names.h

-- 
1.9.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/6] gssd: handle malloc failure appropriately in do_downcall
  2014-04-15 15:19 [PATCH v2 0/6] gssd: add the GSSAPI acceptor name to the info passed in downcall Jeff Layton
@ 2014-04-15 15:19 ` Jeff Layton
  2014-04-15 15:19 ` [PATCH v2 2/6] gssd: make do_downcall a void return Jeff Layton
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Jeff Layton @ 2014-04-15 15:19 UTC (permalink / raw)
  To: steved; +Cc: linux-nfs

...and get rid of some pointless NULL ptr checks.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 utils/gssd/gssd_proc.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index 33cfeb2afd2e..5f7fb32c41b5 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -694,6 +694,9 @@ do_downcall(int k5_fd, uid_t uid, struct authgss_private_data *pd,
 		sizeof(pd->pd_ctx_hndl.length) + pd->pd_ctx_hndl.length +
 		sizeof(context_token->length) + context_token->length;
 	p = buf = malloc(buf_size);
+	if (!buf)
+		goto out_err;
+
 	end = buf + buf_size;
 
 	/* context_timeout set by -t option overrides context lifetime */
@@ -706,10 +709,10 @@ do_downcall(int k5_fd, uid_t uid, struct authgss_private_data *pd,
 	if (write_buffer(&p, end, context_token)) goto out_err;
 
 	if (write(k5_fd, buf, p - buf) < p - buf) goto out_err;
-	if (buf) free(buf);
+	free(buf);
 	return 0;
 out_err:
-	if (buf) free(buf);
+	free(buf);
 	printerr(1, "Failed to write downcall!\n");
 	return -1;
 }
-- 
1.9.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/6] gssd: make do_downcall a void return
  2014-04-15 15:19 [PATCH v2 0/6] gssd: add the GSSAPI acceptor name to the info passed in downcall Jeff Layton
  2014-04-15 15:19 ` [PATCH v2 1/6] gssd: handle malloc failure appropriately in do_downcall Jeff Layton
@ 2014-04-15 15:19 ` Jeff Layton
  2014-04-15 15:19 ` [PATCH v2 3/6] gssd: move hostbased name routines into separate file Jeff Layton
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Jeff Layton @ 2014-04-15 15:19 UTC (permalink / raw)
  To: steved; +Cc: linux-nfs

...since its return code is ignored anyway.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 utils/gssd/gssd_proc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index 5f7fb32c41b5..7387cce010cf 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -681,7 +681,7 @@ parse_enctypes(char *enctypes)
 	return 0;
 }
 
-static int
+static void
 do_downcall(int k5_fd, uid_t uid, struct authgss_private_data *pd,
 	    gss_buffer_desc *context_token, OM_uint32 lifetime_rec)
 {
@@ -710,11 +710,11 @@ do_downcall(int k5_fd, uid_t uid, struct authgss_private_data *pd,
 
 	if (write(k5_fd, buf, p - buf) < p - buf) goto out_err;
 	free(buf);
-	return 0;
+	return;
 out_err:
 	free(buf);
 	printerr(1, "Failed to write downcall!\n");
-	return -1;
+	return;
 }
 
 static int
-- 
1.9.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 3/6] gssd: move hostbased name routines into separate file
  2014-04-15 15:19 [PATCH v2 0/6] gssd: add the GSSAPI acceptor name to the info passed in downcall Jeff Layton
  2014-04-15 15:19 ` [PATCH v2 1/6] gssd: handle malloc failure appropriately in do_downcall Jeff Layton
  2014-04-15 15:19 ` [PATCH v2 2/6] gssd: make do_downcall a void return Jeff Layton
@ 2014-04-15 15:19 ` Jeff Layton
  2014-04-15 15:19 ` [PATCH v2 4/6] gssd: add new routine for generating a hostbased principal in a gss_buffer_t Jeff Layton
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Jeff Layton @ 2014-04-15 15:19 UTC (permalink / raw)
  To: steved; +Cc: linux-nfs

In a later patch, we'll need gssd to call into this code as well as
svcgssd. Move it into a common file that both can link in.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 utils/gssd/Makefile.am    |   2 +
 utils/gssd/gss_names.c    | 123 ++++++++++++++++++++++++++++++++++++++++++++++
 utils/gssd/gss_names.h    |  34 +++++++++++++
 utils/gssd/svcgssd_proc.c |  66 +------------------------
 4 files changed, 160 insertions(+), 65 deletions(-)
 create mode 100644 utils/gssd/gss_names.c
 create mode 100644 utils/gssd/gss_names.h

diff --git a/utils/gssd/Makefile.am b/utils/gssd/Makefile.am
index a9a3e42de19d..af597918e8e0 100644
--- a/utils/gssd/Makefile.am
+++ b/utils/gssd/Makefile.am
@@ -18,11 +18,13 @@ COMMON_SRCS = \
 	context_lucid.c \
 	gss_util.c \
 	gss_oids.c \
+	gss_names.c \
 	err_util.c \
 	\
 	context.h \
 	err_util.h \
 	gss_oids.h \
+	gss_names.h \
 	gss_util.h
 
 gssd_SOURCES = \
diff --git a/utils/gssd/gss_names.c b/utils/gssd/gss_names.c
new file mode 100644
index 000000000000..aa61e4d7a851
--- /dev/null
+++ b/utils/gssd/gss_names.c
@@ -0,0 +1,123 @@
+/*
+  Copyright (c) 2000 The Regents of the University of Michigan.
+  All rights reserved.
+
+  Copyright (c) 2002 Bruce Fields <bfields@UMICH.EDU>
+
+  Redistribution and use in source and binary forms, with or without
+  modification, are permitted provided that the following conditions
+  are met:
+
+  1. Redistributions of source code must retain the above copyright
+     notice, this list of conditions and the following disclaimer.
+  2. Redistributions in binary form must reproduce the above copyright
+     notice, this list of conditions and the following disclaimer in the
+     documentation and/or other materials provided with the distribution.
+  3. Neither the name of the University nor the names of its
+     contributors may be used to endorse or promote products derived
+     from this software without specific prior written permission.
+
+  THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
+  WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+  MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+  DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
+  FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+  CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+  SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+  BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+  LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+  NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+  SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+*/
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif	/* HAVE_CONFIG_H */
+
+#include <sys/param.h>
+#include <sys/stat.h>
+#include <rpc/rpc.h>
+
+#include <pwd.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <ctype.h>
+#include <string.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <nfsidmap.h>
+#include <nfslib.h>
+#include <time.h>
+
+#include "svcgssd.h"
+#include "gss_util.h"
+#include "err_util.h"
+#include "context.h"
+#include "misc.h"
+#include "gss_oids.h"
+#include "svcgssd_krb5.h"
+
+static int
+get_krb5_hostbased_name(gss_buffer_desc *name, char **hostbased_name)
+{
+	char *p, *sname = NULL;
+	if (strchr(name->value, '@') && strchr(name->value, '/')) {
+		if ((sname = calloc(name->length, 1)) == NULL) {
+			printerr(0, "ERROR: get_krb5_hostbased_name failed "
+				 "to allocate %d bytes\n", name->length);
+			return -1;
+		}
+		/* read in name and instance and replace '/' with '@' */
+		sscanf(name->value, "%[^@]", sname);
+		p = strrchr(sname, '/');
+		if (p == NULL) {    /* The '@' preceeded the '/' */
+			free(sname);
+			return -1;
+		}
+		*p = '@';
+	}
+	*hostbased_name = sname;
+	return 0;
+}
+
+int
+get_hostbased_client_name(gss_name_t client_name, gss_OID mech,
+			  char **hostbased_name)
+{
+	u_int32_t	maj_stat, min_stat;
+	gss_buffer_desc	name;
+	gss_OID		name_type = GSS_C_NO_OID;
+	char		*cname;
+	int		res = -1;
+
+	*hostbased_name = NULL;	    /* preset in case we fail */
+
+	/* Get the client's gss authenticated name */
+	maj_stat = gss_display_name(&min_stat, client_name, &name, &name_type);
+	if (maj_stat != GSS_S_COMPLETE) {
+		pgsserr("get_hostbased_client_name: gss_display_name",
+			maj_stat, min_stat, mech);
+		goto out_err;
+	}
+	if (name.length >= 0xffff) {	    /* don't overflow */
+		printerr(0, "ERROR: get_hostbased_client_name: "
+			 "received gss_name is too long (%d bytes)\n",
+			 name.length);
+		goto out_rel_buf;
+	}
+
+	/* For Kerberos, transform the NT_KRB5_PRINCIPAL name to
+	 * an NT_HOSTBASED_SERVICE name */
+	if (g_OID_equal(&krb5oid, mech)) {
+		if (get_krb5_hostbased_name(&name, &cname) == 0)
+			*hostbased_name = cname;
+	} else {
+		printerr(1, "WARNING: unknown/unsupport mech OID\n");
+	}
+
+	res = 0;
+out_rel_buf:
+	gss_release_buffer(&min_stat, &name);
+out_err:
+	return res;
+}
diff --git a/utils/gssd/gss_names.h b/utils/gssd/gss_names.h
new file mode 100644
index 000000000000..1d5f49c1c1d2
--- /dev/null
+++ b/utils/gssd/gss_names.h
@@ -0,0 +1,34 @@
+/*
+  Copyright (c) 2000 The Regents of the University of Michigan.
+  All rights reserved.
+
+  Copyright (c) 2002 Bruce Fields <bfields@UMICH.EDU>
+
+  Redistribution and use in source and binary forms, with or without
+  modification, are permitted provided that the following conditions
+  are met:
+
+  1. Redistributions of source code must retain the above copyright
+     notice, this list of conditions and the following disclaimer.
+  2. Redistributions in binary form must reproduce the above copyright
+     notice, this list of conditions and the following disclaimer in the
+     documentation and/or other materials provided with the distribution.
+  3. Neither the name of the University nor the names of its
+     contributors may be used to endorse or promote products derived
+     from this software without specific prior written permission.
+
+  THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
+  WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+  MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+  DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
+  FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+  CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+  SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+  BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+  LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+  NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+  SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+*/
+
+extern int get_hostbased_client_name(gss_name_t client_name, gss_OID mech,
+					char **hostbased_name);
diff --git a/utils/gssd/svcgssd_proc.c b/utils/gssd/svcgssd_proc.c
index 3757d5191041..5bdb438650d4 100644
--- a/utils/gssd/svcgssd_proc.c
+++ b/utils/gssd/svcgssd_proc.c
@@ -59,6 +59,7 @@
 #include "misc.h"
 #include "gss_oids.h"
 #include "svcgssd_krb5.h"
+#include "gss_names.h"
 
 extern char * mech2file(gss_OID mech);
 #define SVCGSSD_CONTEXT_CHANNEL "/proc/net/rpc/auth.rpcsec.context/channel"
@@ -315,71 +316,6 @@ print_hexl(const char *description, unsigned char *cp, int length)
 }
 #endif
 
-static int
-get_krb5_hostbased_name (gss_buffer_desc *name, char **hostbased_name)
-{
-	char *p, *sname = NULL;
-	if (strchr(name->value, '@') && strchr(name->value, '/')) {
-		if ((sname = calloc(name->length, 1)) == NULL) {
-			printerr(0, "ERROR: get_krb5_hostbased_name failed "
-				 "to allocate %d bytes\n", name->length);
-			return -1;
-		}
-		/* read in name and instance and replace '/' with '@' */
-		sscanf(name->value, "%[^@]", sname);
-		p = strrchr(sname, '/');
-		if (p == NULL) {    /* The '@' preceeded the '/' */
-			free(sname);
-			return -1;
-		}
-		*p = '@';
-	}
-	*hostbased_name = sname;
-	return 0;
-}
-
-static int
-get_hostbased_client_name(gss_name_t client_name, gss_OID mech,
-			  char **hostbased_name)
-{
-	u_int32_t	maj_stat, min_stat;
-	gss_buffer_desc	name;
-	gss_OID		name_type = GSS_C_NO_OID;
-	char		*cname;
-	int		res = -1;
-
-	*hostbased_name = NULL;	    /* preset in case we fail */
-
-	/* Get the client's gss authenticated name */
-	maj_stat = gss_display_name(&min_stat, client_name, &name, &name_type);
-	if (maj_stat != GSS_S_COMPLETE) {
-		pgsserr("get_hostbased_client_name: gss_display_name",
-			maj_stat, min_stat, mech);
-		goto out_err;
-	}
-	if (name.length >= 0xffff) {	    /* don't overflow */
-		printerr(0, "ERROR: get_hostbased_client_name: "
-			 "received gss_name is too long (%d bytes)\n",
-			 name.length);
-		goto out_rel_buf;
-	}
-
-	/* For Kerberos, transform the NT_KRB5_PRINCIPAL name to
-	 * an NT_HOSTBASED_SERVICE name */
-	if (g_OID_equal(&krb5oid, mech)) {
-		if (get_krb5_hostbased_name(&name, &cname) == 0)
-			*hostbased_name = cname;
-	} else {
-		printerr(1, "WARNING: unknown/unsupport mech OID\n");
-	}
-
-	res = 0;
-out_rel_buf:
-	gss_release_buffer(&min_stat, &name);
-out_err:
-	return res;
-}
-
 void
 handle_nullreq(FILE *f) {
 	/* XXX initialize to a random integer to reduce chances of unnecessary
-- 
1.9.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 4/6] gssd: add new routine for generating a hostbased principal in a gss_buffer_t
  2014-04-15 15:19 [PATCH v2 0/6] gssd: add the GSSAPI acceptor name to the info passed in downcall Jeff Layton
                   ` (2 preceding siblings ...)
  2014-04-15 15:19 ` [PATCH v2 3/6] gssd: move hostbased name routines into separate file Jeff Layton
@ 2014-04-15 15:19 ` Jeff Layton
  2014-04-15 15:19 ` [PATCH v2 5/6] gssd: explicitly set lifetime_rec to 0 when gss_inquire_context fails Jeff Layton
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Jeff Layton @ 2014-04-15 15:19 UTC (permalink / raw)
  To: steved; +Cc: linux-nfs

We'll need a gss_buffer_t to pass to the downcall marshalling code.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 utils/gssd/gss_names.c | 15 +++++++++++++++
 utils/gssd/gss_names.h |  2 ++
 2 files changed, 17 insertions(+)

diff --git a/utils/gssd/gss_names.c b/utils/gssd/gss_names.c
index aa61e4d7a851..047069de1755 100644
--- a/utils/gssd/gss_names.c
+++ b/utils/gssd/gss_names.c
@@ -121,3 +121,18 @@ out_rel_buf:
 out_err:
 	return res;
 }
+
+void
+get_hostbased_client_buffer(gss_name_t client_name, gss_OID mech,
+			    gss_buffer_t buf)
+{
+	char *hname;
+
+	if (!get_hostbased_client_name(client_name, mech, &hname)) {
+		buf->length = strlen(hname) + 1;
+		buf->value = hname;
+	} else {
+		buf->length = 0;
+		buf->value = NULL;
+	}
+}
diff --git a/utils/gssd/gss_names.h b/utils/gssd/gss_names.h
index 1d5f49c1c1d2..ce182f7985f3 100644
--- a/utils/gssd/gss_names.h
+++ b/utils/gssd/gss_names.h
@@ -32,3 +32,5 @@
 
 extern int get_hostbased_client_name(gss_name_t client_name, gss_OID mech,
 					char **hostbased_name);
+extern void get_hostbased_client_buffer(gss_name_t client_name,
+			gss_OID mech, gss_buffer_t buf);
-- 
1.9.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 5/6] gssd: explicitly set lifetime_rec to 0 when gss_inquire_context fails
  2014-04-15 15:19 [PATCH v2 0/6] gssd: add the GSSAPI acceptor name to the info passed in downcall Jeff Layton
                   ` (3 preceding siblings ...)
  2014-04-15 15:19 ` [PATCH v2 4/6] gssd: add new routine for generating a hostbased principal in a gss_buffer_t Jeff Layton
@ 2014-04-15 15:19 ` Jeff Layton
  2014-04-15 15:19 ` [PATCH v2 6/6] gssd: scrape the acceptor name out of the context Jeff Layton
  2014-04-30 16:30 ` [PATCH v2 0/6] gssd: add the GSSAPI acceptor name to the info passed in downcall Steve Dickson
  6 siblings, 0 replies; 8+ messages in thread
From: Jeff Layton @ 2014-04-15 15:19 UTC (permalink / raw)
  To: steved; +Cc: linux-nfs, Andy Adamson

Contrary to the comment here, the lifetime_rec is not necessarily set
to zero on failure. That's only guaranteed to be the case if the context
has expired.

Cc: Andy Adamson <androsadamson@gmail.com>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 utils/gssd/gssd_proc.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index 7387cce010cf..e26935dcfc8f 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -1174,14 +1174,15 @@ process_krb5_upcall(struct clnt_info *clp, uid_t uid, int fd, char *tgtname,
 		goto out_return_error;
 	}
 
-	/* Grab the context lifetime to pass to the kernel. lifetime_rec
-	 * is set to zero on error */
+	/* Grab the context lifetime to pass to the kernel. */
 	maj_stat = gss_inquire_context(&min_stat, pd.pd_ctx, NULL, NULL,
 				       &lifetime_rec, NULL, NULL, NULL, NULL);
 
-	if (maj_stat)
+	if (maj_stat) {
 		printerr(1, "WARNING: Failed to inquire context for lifetme "
 			    "maj_stat %u\n", maj_stat);
+		lifetime_rec = 0;
+	}
 
 	if (serialize_context_for_kernel(&pd.pd_ctx, &token, &krb5oid, NULL)) {
 		printerr(0, "WARNING: Failed to serialize krb5 context for "
-- 
1.9.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 6/6] gssd: scrape the acceptor name out of the context
  2014-04-15 15:19 [PATCH v2 0/6] gssd: add the GSSAPI acceptor name to the info passed in downcall Jeff Layton
                   ` (4 preceding siblings ...)
  2014-04-15 15:19 ` [PATCH v2 5/6] gssd: explicitly set lifetime_rec to 0 when gss_inquire_context fails Jeff Layton
@ 2014-04-15 15:19 ` Jeff Layton
  2014-04-30 16:30 ` [PATCH v2 0/6] gssd: add the GSSAPI acceptor name to the info passed in downcall Steve Dickson
  6 siblings, 0 replies; 8+ messages in thread
From: Jeff Layton @ 2014-04-15 15:19 UTC (permalink / raw)
  To: steved; +Cc: linux-nfs

...and pass it to the kernel in the downcall. Legacy kernels will just
ignore the extra data, but with a proposed kernel patch the kernel will
grab this info and use it to verify requests on the v4.0 callback
channel.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 utils/gssd/gssd_proc.c | 37 +++++++++++++++++++++++++++----------
 1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index e26935dcfc8f..69bb3c6f11b3 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -77,6 +77,7 @@
 #include "context.h"
 #include "nfsrpc.h"
 #include "nfslib.h"
+#include "gss_names.h"
 
 /*
  * pollarray:
@@ -683,16 +684,19 @@ parse_enctypes(char *enctypes)
 
 static void
 do_downcall(int k5_fd, uid_t uid, struct authgss_private_data *pd,
-	    gss_buffer_desc *context_token, OM_uint32 lifetime_rec)
+	    gss_buffer_desc *context_token, OM_uint32 lifetime_rec,
+	    gss_buffer_desc *acceptor)
 {
 	char    *buf = NULL, *p = NULL, *end = NULL;
 	unsigned int timeout = context_timeout;
 	unsigned int buf_size = 0;
 
-	printerr(1, "doing downcall lifetime_rec %u\n", lifetime_rec);
+	printerr(1, "doing downcall: lifetime_rec=%u acceptor=%.*s\n",
+		lifetime_rec, acceptor->length, acceptor->value);
 	buf_size = sizeof(uid) + sizeof(timeout) + sizeof(pd->pd_seq_win) +
 		sizeof(pd->pd_ctx_hndl.length) + pd->pd_ctx_hndl.length +
-		sizeof(context_token->length) + context_token->length;
+		sizeof(context_token->length) + context_token->length +
+		acceptor->length;
 	p = buf = malloc(buf_size);
 	if (!buf)
 		goto out_err;
@@ -707,6 +711,7 @@ do_downcall(int k5_fd, uid_t uid, struct authgss_private_data *pd,
 	if (WRITE_BYTES(&p, end, pd->pd_seq_win)) goto out_err;
 	if (write_buffer(&p, end, &pd->pd_ctx_hndl)) goto out_err;
 	if (write_buffer(&p, end, context_token)) goto out_err;
+	if (write_buffer(&p, end, acceptor)) goto out_err;
 
 	if (write(k5_fd, buf, p - buf) < p - buf) goto out_err;
 	free(buf);
@@ -1034,6 +1039,9 @@ 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;
 	pid_t			pid;
+	gss_name_t		gacceptor = GSS_C_NO_NAME;
+	gss_OID			mech;
+	gss_buffer_desc		acceptor  = {0};
 
 	pid = fork();
 	switch(pid) {
@@ -1174,16 +1182,24 @@ process_krb5_upcall(struct clnt_info *clp, uid_t uid, int fd, char *tgtname,
 		goto out_return_error;
 	}
 
-	/* Grab the context lifetime to pass to the kernel. */
-	maj_stat = gss_inquire_context(&min_stat, pd.pd_ctx, NULL, NULL,
-				       &lifetime_rec, NULL, NULL, NULL, NULL);
+	/* Grab the context lifetime and acceptor name out of the ctx. */
+	maj_stat = gss_inquire_context(&min_stat, pd.pd_ctx, NULL, &gacceptor,
+				       &lifetime_rec, &mech, NULL, NULL, NULL);
 
-	if (maj_stat) {
-		printerr(1, "WARNING: Failed to inquire context for lifetme "
-			    "maj_stat %u\n", maj_stat);
+	if (maj_stat != GSS_S_COMPLETE) {
+		printerr(1, "WARNING: Failed to inquire context "
+			    "maj_stat (0x%x)\n", maj_stat);
 		lifetime_rec = 0;
+	} else {
+		get_hostbased_client_buffer(gacceptor, mech, &acceptor);
+		gss_release_name(&min_stat, &gacceptor);
 	}
 
+	/*
+	 * The serialization can mean turning pd.pd_ctx into a lucid context. If
+	 * that happens then the pd.pd_ctx will be unusable, so we must never
+	 * try to use it after this point.
+	 */
 	if (serialize_context_for_kernel(&pd.pd_ctx, &token, &krb5oid, NULL)) {
 		printerr(0, "WARNING: Failed to serialize krb5 context for "
 			    "user with uid %d for server %s\n",
@@ -1191,9 +1207,10 @@ process_krb5_upcall(struct clnt_info *clp, uid_t uid, int fd, char *tgtname,
 		goto out_return_error;
 	}
 
-	do_downcall(fd, uid, &pd, &token, lifetime_rec);
+	do_downcall(fd, uid, &pd, &token, lifetime_rec, &acceptor);
 
 out:
+	gss_release_buffer(&min_stat, &acceptor);
 	if (token.value)
 		free(token.value);
 #ifdef HAVE_AUTHGSS_FREE_PRIVATE_DATA
-- 
1.9.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 0/6] gssd: add the GSSAPI acceptor name to the info passed in downcall
  2014-04-15 15:19 [PATCH v2 0/6] gssd: add the GSSAPI acceptor name to the info passed in downcall Jeff Layton
                   ` (5 preceding siblings ...)
  2014-04-15 15:19 ` [PATCH v2 6/6] gssd: scrape the acceptor name out of the context Jeff Layton
@ 2014-04-30 16:30 ` Steve Dickson
  6 siblings, 0 replies; 8+ messages in thread
From: Steve Dickson @ 2014-04-30 16:30 UTC (permalink / raw)
  To: Jeff Layton; +Cc: linux-nfs



On 04/15/2014 11:19 AM, Jeff Layton wrote:
> v2:
> - add patch to reset lifetime_rec if gss_inquire_context fails
> - ensure that we always send the length in the downcall, even if
>   there is no acceptor string.
> - comment and error handling fixups (primarily in last patch)
> 
> Recently, I started a mailing list thread about some authentication
> failures that I was seeing on the callback channel when krb5 was in use.
> 
> After a bit of discussion we determined that the right way to fix it
> was to save off the GSSAPI acceptor name used in the SETCLIENT call,
> and then ensure that the same principal is used in callback requests.
> 
> This patchset is the userland portion of that change. It basically
> just adds the acceptor name to the downcall, immediately following
> the context token. Older kernel will just ignore this data, so this
> should be safe.
> 
> There is also a companion kernel patchset that will allow the kernel
> to save off this info for later usage.
> 
> Jeff Layton (6):
>   gssd: handle malloc failure appropriately in do_downcall
>   gssd: make do_downcall a void return
>   gssd: move hostbased name routines into separate file
>   gssd: add new routine for generating a hostbased principal in a
>     gss_buffer_t
>   gssd: explicitly set lifetime_rec to 0 when gss_inquire_context fails
>   gssd: scrape the acceptor name out of the context
> 
>  utils/gssd/Makefile.am    |   2 +
>  utils/gssd/gss_names.c    | 138 ++++++++++++++++++++++++++++++++++++++++++++++
>  utils/gssd/gss_names.h    |  36 ++++++++++++
>  utils/gssd/gssd_proc.c    |  53 ++++++++++++------
>  utils/gssd/svcgssd_proc.c |  66 +---------------------
>  5 files changed, 214 insertions(+), 81 deletions(-)
>  create mode 100644 utils/gssd/gss_names.c
>  create mode 100644 utils/gssd/gss_names.h
> 
Committed... All six patches...

steved.


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-04-30 17:21 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-04-15 15:19 [PATCH v2 0/6] gssd: add the GSSAPI acceptor name to the info passed in downcall Jeff Layton
2014-04-15 15:19 ` [PATCH v2 1/6] gssd: handle malloc failure appropriately in do_downcall Jeff Layton
2014-04-15 15:19 ` [PATCH v2 2/6] gssd: make do_downcall a void return Jeff Layton
2014-04-15 15:19 ` [PATCH v2 3/6] gssd: move hostbased name routines into separate file Jeff Layton
2014-04-15 15:19 ` [PATCH v2 4/6] gssd: add new routine for generating a hostbased principal in a gss_buffer_t Jeff Layton
2014-04-15 15:19 ` [PATCH v2 5/6] gssd: explicitly set lifetime_rec to 0 when gss_inquire_context fails Jeff Layton
2014-04-15 15:19 ` [PATCH v2 6/6] gssd: scrape the acceptor name out of the context Jeff Layton
2014-04-30 16:30 ` [PATCH v2 0/6] gssd: add the GSSAPI acceptor name to the info passed in downcall Steve Dickson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).