All of lore.kernel.org
 help / color / mirror / Atom feed
* [cifs-utils PATCH 0/3] cifs-utils: overhaul of cifs.upcall krb5 handling
@ 2016-08-22 12:29 Jeff Layton
       [not found] ` <1471868962-7312-1-git-send-email-jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Jeff Layton @ 2016-08-22 12:29 UTC (permalink / raw)
  To: linux-cifs-u79uwXL29TY76Z2rM5mHXA; +Cc: Dey, John F

The handling of krb5 in cifs.upcall has always been pretty klunky. It
rolls through /tmp, trying to find the latest credcache and has some
hacks to allow it to use DIR: caches as well, but none of that really
works for KEYRING:, which is pretty common these days.

In practice, I doubt anyone relies on that behavior. What most people
want is for cifs.upcall to find the default credcache for a user given
krb5.conf -- full stop.

This patchset rips out most of the unneeded machinery in cifs.upcall,
and just has it find the default credcache and verify that it has a
valid TGT. If not then we'll try to init it from the keytab as before.

I think there's some more opportunity to clean up this code in the
future as well. Currently we pass around strings that represent the
credcache, and that could be made more efficient.  It might also be
good to just reimplement the whole thing with gssapi calls instead.

Still, this is a good step in that direction I think.

Jeff Layton (3):
  aclocal: fix typo in idmap.m4
  cifs.upcall: use krb5 routines to get default ccname
  cifs.upcall: make the krb5_context a static global variable

 aclocal/idmap.m4 |   2 +-
 cifs.upcall.c    | 185 ++++++++++---------------------------------------------
 2 files changed, 32 insertions(+), 155 deletions(-)

-- 
2.7.4

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

* [cifs-utils PATCH 1/3] aclocal: fix typo in idmap.m4
       [not found] ` <1471868962-7312-1-git-send-email-jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
@ 2016-08-22 12:29   ` Jeff Layton
  2016-08-22 12:29   ` [cifs-utils PATCH 2/3] cifs.upcall: use krb5 routines to get default ccname Jeff Layton
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Jeff Layton @ 2016-08-22 12:29 UTC (permalink / raw)
  To: linux-cifs-u79uwXL29TY76Z2rM5mHXA; +Cc: Dey, John F

We really don't want to do the same check twice.

Signed-off-by: Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
---
 aclocal/idmap.m4 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/aclocal/idmap.m4 b/aclocal/idmap.m4
index 3ccdae3ab968..4e16a46568a1 100644
--- a/aclocal/idmap.m4
+++ b/aclocal/idmap.m4
@@ -19,7 +19,7 @@ if test $enable_cifsidmap != "no" -o $enable_cifsacl != "no"; then
 			])
 fi
 
-if test $enable_cifsacl != "no" -o $enable_cifsacl != "no"; then
+if test $enable_cifsidmap != "no" -o $enable_cifsacl != "no"; then
 	ac_wbc_save_LDFLAGS="$LDFLAGS"
 	ac_wbc_save_LIBS="$LIBS"
 	LDFLAGS="$LDFLAGS $WBCLIENT_LIBS"
-- 
2.7.4

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

* [cifs-utils PATCH 2/3] cifs.upcall: use krb5 routines to get default ccname
       [not found] ` <1471868962-7312-1-git-send-email-jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
  2016-08-22 12:29   ` [cifs-utils PATCH 1/3] aclocal: fix typo in idmap.m4 Jeff Layton
@ 2016-08-22 12:29   ` Jeff Layton
       [not found]     ` <1471868962-7312-3-git-send-email-jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
  2016-08-22 12:29   ` [cifs-utils PATCH 3/3] cifs.upcall: make the krb5_context a static global variable Jeff Layton
  2016-08-23 19:21   ` [cifs-utils PATCH 0/3] cifs-utils: overhaul of cifs.upcall krb5 handling Dey, John F
  3 siblings, 1 reply; 8+ messages in thread
From: Jeff Layton @ 2016-08-22 12:29 UTC (permalink / raw)
  To: linux-cifs-u79uwXL29TY76Z2rM5mHXA; +Cc: Dey, John F

Currently we end up groveling around in /tmp, trying to guess what the
credcache will be. Instead, just get the default ccname for the user,
and then see if it has a valid tgt. If it doesn't then we try to use
the keytab to init the credcache before proceeding.

Signed-off-by: Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
---
 cifs.upcall.c | 148 +++++++++++-----------------------------------------------
 1 file changed, 27 insertions(+), 121 deletions(-)

diff --git a/cifs.upcall.c b/cifs.upcall.c
index e8544c2b68ad..d0f6d089d8e1 100644
--- a/cifs.upcall.c
+++ b/cifs.upcall.c
@@ -52,12 +52,6 @@
 #include "spnego.h"
 #include "cifs_spnego.h"
 
-#define	CIFS_DEFAULT_KRB5_DIR		"/tmp"
-#define	CIFS_DEFAULT_KRB5_USER_DIR	"/run/user/%U"
-#define	CIFS_DEFAULT_KRB5_PREFIX	"krb5cc"
-
-#define	MAX_CCNAME_LEN			PATH_MAX + 5
-
 static const char *prog = "cifs.upcall";
 typedef enum _sectype {
 	NONE = 0,
@@ -178,13 +172,34 @@ err_cache:
 	return credtime;
 }
 
-static int krb5cc_filter(const struct dirent *dirent)
+static char *
+get_default_cc(void)
 {
-	/* subtract 1 for the null terminator */
-	return !strncmp(dirent->d_name, CIFS_DEFAULT_KRB5_PREFIX,
-			sizeof(CIFS_DEFAULT_KRB5_PREFIX) - 1);
+	krb5_error_code ret;
+	const char *ccname;
+	char *rcc = NULL;
+	krb5_context context = NULL;
+
+	ret = krb5_init_context(&context);
+	if (ret) {
+		syslog(LOG_DEBUG, "krb5_init_context: %d", (int)ret);
+		return NULL;
+	}
+
+	ccname = krb5_cc_default_name(context);
+	if (!ccname) {
+		syslog(LOG_DEBUG, "krb5_cc_default returned NULL.");
+		goto out_free_context;
+	}
+
+	if (get_tgt_time(ccname))
+		rcc = strdup(ccname);
+out_free_context:
+	krb5_free_context(context);
+	return rcc;
 }
 
+
 static char *
 init_cc_from_keytab(const char *keytab_name, const char *user)
 {
@@ -263,109 +278,6 @@ icfk_cleanup:
 	return ccname;
 }
 
-/* resolve a pattern to an actual directory path */
-static char *resolve_krb5_dir(const char *pattern, uid_t uid)
-{
-	char name[MAX_CCNAME_LEN];
-	int i;
-	size_t j;
-	for (i = 0, j = 0; (pattern[i] != '\0') && (j < sizeof(name)); i++) {
-		switch (pattern[i]) {
-		case '%':
-			switch (pattern[i + 1]) {
-			case '%':
-				name[j++] = pattern[i];
-				i++;
-				break;
-			case 'U':
-				j += snprintf(name + j, sizeof(name) - j,
-					      "%lu", (unsigned long) uid);
-				i++;
-				break;
-			}
-			break;
-		default:
-			name[j++] = pattern[i];
-			break;
-		}
-	}
-	if ((j > 0) && (j < sizeof(name)))
-		return strndup(name, MAX_CCNAME_LEN);
-	else
-		return NULL;
-}
-
-/* search for a credcache that looks like a likely candidate */
-static char *find_krb5_cc(const char *dirname, uid_t uid,
-			  char **best_cache, time_t *best_time)
-{
-	struct dirent **namelist;
-	struct stat sbuf;
-	char ccname[MAX_CCNAME_LEN], *credpath;
-	int i, n;
-	time_t cred_time;
-
-	n = scandir(dirname, &namelist, krb5cc_filter, NULL);
-	if (n < 0) {
-		syslog(LOG_DEBUG, "%s: scandir error on directory '%s': %s",
-		       __func__, dirname, strerror(errno));
-		return NULL;
-	}
-
-	for (i = 0; i < n; i++) {
-		snprintf(ccname, sizeof(ccname), "FILE:%s/%s", dirname,
-			 namelist[i]->d_name);
-		credpath = ccname + 5;
-		syslog(LOG_DEBUG, "%s: considering %s", __func__, credpath);
-
-		if (lstat(credpath, &sbuf)) {
-			syslog(LOG_DEBUG, "%s: stat error on '%s': %s",
-			       __func__, credpath, strerror(errno));
-			free(namelist[i]);
-			continue;
-		}
-		if (sbuf.st_uid != uid) {
-			syslog(LOG_DEBUG, "%s: %s is owned by %u, not %u",
-			       __func__, credpath, sbuf.st_uid, uid);
-			free(namelist[i]);
-			continue;
-		}
-		if (S_ISDIR(sbuf.st_mode)) {
-			snprintf(ccname, sizeof(ccname), "DIR:%s/%s", dirname,
-				 namelist[i]->d_name);
-			credpath = ccname + 4;
-		} else
-		if (!S_ISREG(sbuf.st_mode)) {
-			syslog(LOG_DEBUG, "%s: %s is not a regular file",
-			       __func__, credpath);
-			free(namelist[i]);
-			continue;
-		}
-		if (!(cred_time = get_tgt_time(ccname))) {
-			syslog(LOG_DEBUG, "%s: %s is not a valid credcache.",
-			       __func__, ccname);
-			free(namelist[i]);
-			continue;
-		}
-
-		if (cred_time <= *best_time) {
-			syslog(LOG_DEBUG, "%s: %s expires sooner than current "
-			       "best.", __func__, ccname);
-			free(namelist[i]);
-			continue;
-		}
-
-		syslog(LOG_DEBUG, "%s: %s is valid ccache", __func__, ccname);
-		free(*best_cache);
-		*best_cache = strndup(ccname, MAX_CCNAME_LEN);
-		*best_time = cred_time;
-		free(namelist[i]);
-	}
-	free(namelist);
-
-	return *best_cache;
-}
-
 static int
 cifs_krb5_get_req(const char *host, const char *ccname,
 		  DATA_BLOB * mechtoken, DATA_BLOB * sess_key)
@@ -841,13 +753,12 @@ int main(const int argc, char *const argv[])
 	unsigned int have;
 	long rc = 1;
 	int c, try_dns = 0, legacy_uid = 0;
-	char *buf, *ccdir = NULL, *ccname = NULL, *best_cache = NULL;
+	char *buf, *ccname = NULL;
 	char hostbuf[NI_MAXHOST], *host;
 	struct decoded_args arg;
 	const char *oid;
 	uid_t uid;
 	char *keytab_name = NULL;
-	time_t best_time = 0;
 
 	hostbuf[0] = '\0';
 	memset(&arg, 0, sizeof(arg));
@@ -954,13 +865,8 @@ int main(const int argc, char *const argv[])
 		syslog(LOG_ERR, "setuid: %s", strerror(errno));
 		goto out;
 	}
-	ccdir = resolve_krb5_dir(CIFS_DEFAULT_KRB5_USER_DIR, uid);
-	if (ccdir != NULL)
-		find_krb5_cc(ccdir, uid, &best_cache, &best_time);
-	ccname = find_krb5_cc(CIFS_DEFAULT_KRB5_DIR, uid, &best_cache,
-			      &best_time);
-	SAFE_FREE(ccdir);
 
+	ccname = get_default_cc();
 	/* Couldn't find credcache? Try to use keytab */
 	if (ccname == NULL && arg.username != NULL)
 		ccname = init_cc_from_keytab(keytab_name, arg.username);
-- 
2.7.4

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

* [cifs-utils PATCH 3/3] cifs.upcall: make the krb5_context a static global variable
       [not found] ` <1471868962-7312-1-git-send-email-jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
  2016-08-22 12:29   ` [cifs-utils PATCH 1/3] aclocal: fix typo in idmap.m4 Jeff Layton
  2016-08-22 12:29   ` [cifs-utils PATCH 2/3] cifs.upcall: use krb5 routines to get default ccname Jeff Layton
@ 2016-08-22 12:29   ` Jeff Layton
  2016-08-23 19:21   ` [cifs-utils PATCH 0/3] cifs-utils: overhaul of cifs.upcall krb5 handling Dey, John F
  3 siblings, 0 replies; 8+ messages in thread
From: Jeff Layton @ 2016-08-22 12:29 UTC (permalink / raw)
  To: linux-cifs-u79uwXL29TY76Z2rM5mHXA; +Cc: Dey, John F

There's no need to keep initing a new context for every function. Just
do it once and reuse as needed.

Signed-off-by: Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
---
 cifs.upcall.c | 61 ++++++++++++++++-------------------------------------------
 1 file changed, 16 insertions(+), 45 deletions(-)

diff --git a/cifs.upcall.c b/cifs.upcall.c
index d0f6d089d8e1..8448d00f6061 100644
--- a/cifs.upcall.c
+++ b/cifs.upcall.c
@@ -52,7 +52,9 @@
 #include "spnego.h"
 #include "cifs_spnego.h"
 
-static const char *prog = "cifs.upcall";
+static krb5_context	context;
+static const char	*prog = "cifs.upcall";
+
 typedef enum _sectype {
 	NONE = 0,
 	KRB5,
@@ -69,9 +71,7 @@ typedef enum _sectype {
  * @return pointer to the realm
  *
  */
-
-static char *cifs_krb5_principal_get_realm(krb5_context context __attribute__ ((unused)),
-					   krb5_principal principal)
+static char *cifs_krb5_principal_get_realm(krb5_principal principal)
 {
 #ifdef HAVE_KRB5_PRINCIPAL_GET_REALM	/* Heimdal */
 	return krb5_principal_get_realm(context, principal);
@@ -104,7 +104,6 @@ krb5_auth_con_getsendsubkey(krb5_context context,
 /* does the ccache have a valid TGT? */
 static time_t get_tgt_time(const char *ccname)
 {
-	krb5_context context;
 	krb5_ccache ccache;
 	krb5_cc_cursor cur;
 	krb5_creds creds;
@@ -112,11 +111,6 @@ static time_t get_tgt_time(const char *ccname)
 	time_t credtime = 0;
 	char *realm = NULL;
 
-	if (krb5_init_context(&context)) {
-		syslog(LOG_DEBUG, "%s: unable to init krb5 context", __func__);
-		return 0;
-	}
-
 	if (krb5_cc_resolve(context, ccname, &ccache)) {
 		syslog(LOG_DEBUG, "%s: unable to resolve krb5 cache", __func__);
 		goto err_cache;
@@ -137,7 +131,7 @@ static time_t get_tgt_time(const char *ccname)
 		goto err_ccstart;
 	}
 
-	if ((realm = cifs_krb5_principal_get_realm(context, principal)) == NULL) {
+	if ((realm = cifs_krb5_principal_get_realm(principal)) == NULL) {
 		syslog(LOG_DEBUG, "%s: unable to get realm", __func__);
 		goto err_ccstart;
 	}
@@ -168,34 +162,23 @@ err_princ:
 #endif
 	krb5_cc_close(context, ccache);
 err_cache:
-	krb5_free_context(context);
 	return credtime;
 }
 
 static char *
 get_default_cc(void)
 {
-	krb5_error_code ret;
 	const char *ccname;
 	char *rcc = NULL;
-	krb5_context context = NULL;
-
-	ret = krb5_init_context(&context);
-	if (ret) {
-		syslog(LOG_DEBUG, "krb5_init_context: %d", (int)ret);
-		return NULL;
-	}
 
 	ccname = krb5_cc_default_name(context);
 	if (!ccname) {
 		syslog(LOG_DEBUG, "krb5_cc_default returned NULL.");
-		goto out_free_context;
+		return NULL;
 	}
 
 	if (get_tgt_time(ccname))
 		rcc = strdup(ccname);
-out_free_context:
-	krb5_free_context(context);
 	return rcc;
 }
 
@@ -203,7 +186,6 @@ out_free_context:
 static char *
 init_cc_from_keytab(const char *keytab_name, const char *user)
 {
-	krb5_context context = NULL;
 	krb5_error_code ret;
 	krb5_creds my_creds;
 	krb5_keytab keytab = NULL;
@@ -213,12 +195,6 @@ init_cc_from_keytab(const char *keytab_name, const char *user)
 
 	memset((char *) &my_creds, 0, sizeof(my_creds));
 
-	ret = krb5_init_context(&context);
-	if (ret) {
-		syslog(LOG_DEBUG, "krb5_init_context: %d", (int)ret);
-		goto icfk_cleanup;
-	}
-
 	if (keytab_name)
 		ret = krb5_kt_resolve(context, keytab_name, &keytab);
 	else
@@ -273,8 +249,6 @@ icfk_cleanup:
 		krb5_cc_close(context, cc);
 	if (keytab)
 		krb5_kt_close(context, keytab);
-	if (context)
-		krb5_free_context(context);
 	return ccname;
 }
 
@@ -284,7 +258,6 @@ cifs_krb5_get_req(const char *host, const char *ccname,
 {
 	krb5_error_code ret;
 	krb5_keyblock *tokb;
-	krb5_context context;
 	krb5_ccache ccache;
 	krb5_creds in_creds, *out_creds;
 	krb5_data apreq_pkt, in_data;
@@ -292,26 +265,19 @@ cifs_krb5_get_req(const char *host, const char *ccname,
 #if defined(HAVE_KRB5_AUTH_CON_SETADDRS) && defined(HAVE_KRB5_AUTH_CON_SET_REQ_CKSUMTYPE)
 	static const uint8_t gss_cksum[24] = { 0x10, 0x00, /* ... */};
 #endif
-
-	ret = krb5_init_context(&context);
-	if (ret) {
-		syslog(LOG_DEBUG, "%s: unable to init krb5 context", __func__);
-		return ret;
-	}
-
 	if (ccname) {
 		ret = krb5_cc_resolve(context, ccname, &ccache);
 		if (ret) {
 			syslog(LOG_DEBUG, "%s: unable to resolve %s to ccache\n",
 			       __func__, ccname);
-			goto out_free_context;
+			return ret;
 		}
 	} else {
 		ret = krb5_cc_default(context, &ccache);
 		if (ret) {
 			syslog(LOG_DEBUG, "%s: krb5_cc_default: %d",
 				__func__, (int)ret);
-			goto out_free_context;
+			return ret;
 		}
 	}
 
@@ -383,7 +349,6 @@ cifs_krb5_get_req(const char *host, const char *ccname,
 	/* MIT krb5 < 1.7 is missing the prototype, but still has the symbol */
 #if !HAVE_DECL_KRB5_AUTH_CON_SET_REQ_CKSUMTYPE
 	krb5_error_code krb5_auth_con_set_req_cksumtype(
-		krb5_context      context,
 		krb5_auth_context auth_context,
 		krb5_cksumtype    cksumtype);
 #endif
@@ -427,8 +392,6 @@ out_free_ccache:
 	krb5_cc_set_flags(context, ccache, KRB5_TC_OPENCLOSE);
 #endif
 	krb5_cc_close(context, ccache);
-out_free_context:
-	krb5_free_context(context);
 	return ret;
 }
 
@@ -866,6 +829,12 @@ int main(const int argc, char *const argv[])
 		goto out;
 	}
 
+	rc = krb5_init_context(&context);
+	if (rc) {
+		syslog(LOG_ERR, "unable to init krb5 context: %ld", rc);
+		goto out;
+	}
+
 	ccname = get_default_cc();
 	/* Couldn't find credcache? Try to use keytab */
 	if (ccname == NULL && arg.username != NULL)
@@ -1006,6 +975,8 @@ out:
 	}
 	data_blob_free(&secblob);
 	data_blob_free(&sess_key);
+	if (context)
+		krb5_free_context(context);
 	SAFE_FREE(ccname);
 	SAFE_FREE(arg.hostname);
 	SAFE_FREE(arg.ip);
-- 
2.7.4

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

* Re: [cifs-utils PATCH 0/3] cifs-utils: overhaul of cifs.upcall krb5 handling
       [not found] ` <1471868962-7312-1-git-send-email-jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
                     ` (2 preceding siblings ...)
  2016-08-22 12:29   ` [cifs-utils PATCH 3/3] cifs.upcall: make the krb5_context a static global variable Jeff Layton
@ 2016-08-23 19:21   ` Dey, John F
       [not found]     ` <9A891926-AE7D-4916-AB71-A49D4A1588BC-q9hIisBwmLrYtjvyW6yDsg@public.gmane.org>
  3 siblings, 1 reply; 8+ messages in thread
From: Dey, John F @ 2016-08-23 19:21 UTC (permalink / raw)
  To: linux-cifs-u79uwXL29TY76Z2rM5mHXA

I have a newer version of the cifs-upcall patch which follows the linux coding standards.  I have taken a look at the use of krb5_context through out all the code and reduced the usage of krb5_context to one call.
 

John Dey







On 8/22/16, 5:29 AM, "Jeff Layton" <jlayton@samba.org> wrote:

>The handling of krb5 in cifs.upcall has always been pretty klunky. It
>rolls through /tmp, trying to find the latest credcache and has some
>hacks to allow it to use DIR: caches as well, but none of that really
>works for KEYRING:, which is pretty common these days.
>
>In practice, I doubt anyone relies on that behavior. What most people
>want is for cifs.upcall to find the default credcache for a user given
>krb5.conf -- full stop.
>
>This patchset rips out most of the unneeded machinery in cifs.upcall,
>and just has it find the default credcache and verify that it has a
>valid TGT. If not then we'll try to init it from the keytab as before.
>
>I think there's some more opportunity to clean up this code in the
>future as well. Currently we pass around strings that represent the
>credcache, and that could be made more efficient.  It might also be
>good to just reimplement the whole thing with gssapi calls instead.
>
>Still, this is a good step in that direction I think.
>
>Jeff Layton (3):
>  aclocal: fix typo in idmap.m4
>  cifs.upcall: use krb5 routines to get default ccname
>  cifs.upcall: make the krb5_context a static global variable
>
> aclocal/idmap.m4 |   2 +-
> cifs.upcall.c    | 185 ++++++++++---------------------------------------------
> 2 files changed, 32 insertions(+), 155 deletions(-)
>
>-- 
>2.7.4
>

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

* Re: [cifs-utils PATCH 0/3] cifs-utils: overhaul of cifs.upcall krb5 handling
       [not found]     ` <9A891926-AE7D-4916-AB71-A49D4A1588BC-q9hIisBwmLrYtjvyW6yDsg@public.gmane.org>
@ 2016-08-24 11:06       ` Jeff Layton
  0 siblings, 0 replies; 8+ messages in thread
From: Jeff Layton @ 2016-08-24 11:06 UTC (permalink / raw)
  To: Dey, John F, linux-cifs-u79uwXL29TY76Z2rM5mHXA

On Tue, 2016-08-23 at 19:21 +0000, Dey, John F wrote:
> I have a newer version of the cifs-upcall patch which follows the
> linux coding standards.  I have taken a look at the use of
> krb5_context through out all the code and reduced the usage of
> krb5_context to one call.
>  
> 
> John Dey
> 
> 
> 

Ok. With this set, those patches may no longer be needed. You may want
to try building cifs-utils with them and see whether it fixes the
problem you were having.

The idea here is to get cifs-utils out of the business of looking for a
credcache in likely places at all, and to instead rely on the krb5 libs
to tell us where it thinks the credcache will be. That should make
cifs.upcall a lot more efficient, and better able to work with non
FILE: credcaches.

> 
> 
> 
> On 8/22/16, 5:29 AM, "Jeff Layton" <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> 
> > 
> > The handling of krb5 in cifs.upcall has always been pretty klunky.
> > It
> > rolls through /tmp, trying to find the latest credcache and has
> > some
> > hacks to allow it to use DIR: caches as well, but none of that
> > really
> > works for KEYRING:, which is pretty common these days.
> > 
> > In practice, I doubt anyone relies on that behavior. What most
> > people
> > want is for cifs.upcall to find the default credcache for a user
> > given
> > krb5.conf -- full stop.
> > 
> > This patchset rips out most of the unneeded machinery in
> > cifs.upcall,
> > and just has it find the default credcache and verify that it has a
> > valid TGT. If not then we'll try to init it from the keytab as
> > before.
> > 
> > I think there's some more opportunity to clean up this code in the
> > future as well. Currently we pass around strings that represent the
> > credcache, and that could be made more efficient.  It might also be
> > good to just reimplement the whole thing with gssapi calls instead.
> > 
> > Still, this is a good step in that direction I think.
> > 
> > Jeff Layton (3):
> >  aclocal: fix typo in idmap.m4
> >  cifs.upcall: use krb5 routines to get default ccname
> >  cifs.upcall: make the krb5_context a static global variable
> > 
> > aclocal/idmap.m4 |   2 +-
> > cifs.upcall.c    | 185 ++++++++++--------------------------------
> > -------------
> > 2 files changed, 32 insertions(+), 155 deletions(-)
> > 
> > -- 
> > 2.7.4
> > 
> NrybXǧv^)޺{.n+{r'{ay\x1d
ʇڙ,j\afhz\x1e
w\f
j:+vwjm\azZ+ݢj"!

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

* Re: [cifs-utils PATCH 2/3] cifs.upcall: use krb5 routines to get default ccname
       [not found]     ` <1471868962-7312-3-git-send-email-jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
@ 2016-10-14 18:09       ` Dey, John F
       [not found]         ` <039A665D-75B4-4008-85F9-6CAE8BFB2E15-q9hIisBwmLrYtjvyW6yDsg@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Dey, John F @ 2016-10-14 18:09 UTC (permalink / raw)
  To: Jeff Layton, linux-cifs-u79uwXL29TY76Z2rM5mHXA

Are the default_ccache_name patches making it upstream? 

John Dey






On 8/22/16, 5:29 AM, "Jeff Layton" <jlayton@samba.org> wrote:

>Currently we end up groveling around in /tmp, trying to guess what the
>credcache will be. Instead, just get the default ccname for the user,
>and then see if it has a valid tgt. If it doesn't then we try to use
>the keytab to init the credcache before proceeding.
>
>Signed-off-by: Jeff Layton <jlayton@samba.org>
>---
> cifs.upcall.c | 148 +++++++++++-----------------------------------------------
> 1 file changed, 27 insertions(+), 121 deletions(-)
>
>diff --git a/cifs.upcall.c b/cifs.upcall.c
>index e8544c2b68ad..d0f6d089d8e1 100644
>--- a/cifs.upcall.c
>+++ b/cifs.upcall.c
>@@ -52,12 +52,6 @@
> #include "spnego.h"
> #include "cifs_spnego.h"
> 
>-#define	CIFS_DEFAULT_KRB5_DIR		"/tmp"
>-#define	CIFS_DEFAULT_KRB5_USER_DIR	"/run/user/%U"
>-#define	CIFS_DEFAULT_KRB5_PREFIX	"krb5cc"
>-
>-#define	MAX_CCNAME_LEN			PATH_MAX + 5
>-
> static const char *prog = "cifs.upcall";
> typedef enum _sectype {
> 	NONE = 0,
>@@ -178,13 +172,34 @@ err_cache:
> 	return credtime;
> }
> 
>-static int krb5cc_filter(const struct dirent *dirent)
>+static char *
>+get_default_cc(void)
> {
>-	/* subtract 1 for the null terminator */
>-	return !strncmp(dirent->d_name, CIFS_DEFAULT_KRB5_PREFIX,
>-			sizeof(CIFS_DEFAULT_KRB5_PREFIX) - 1);
>+	krb5_error_code ret;
>+	const char *ccname;
>+	char *rcc = NULL;
>+	krb5_context context = NULL;
>+
>+	ret = krb5_init_context(&context);
>+	if (ret) {
>+		syslog(LOG_DEBUG, "krb5_init_context: %d", (int)ret);
>+		return NULL;
>+	}
>+
>+	ccname = krb5_cc_default_name(context);
>+	if (!ccname) {
>+		syslog(LOG_DEBUG, "krb5_cc_default returned NULL.");
>+		goto out_free_context;
>+	}
>+
>+	if (get_tgt_time(ccname))
>+		rcc = strdup(ccname);
>+out_free_context:
>+	krb5_free_context(context);
>+	return rcc;
> }
> 
>+
> static char *
> init_cc_from_keytab(const char *keytab_name, const char *user)
> {
>@@ -263,109 +278,6 @@ icfk_cleanup:
> 	return ccname;
> }
> 
>-/* resolve a pattern to an actual directory path */
>-static char *resolve_krb5_dir(const char *pattern, uid_t uid)
>-{
>-	char name[MAX_CCNAME_LEN];
>-	int i;
>-	size_t j;
>-	for (i = 0, j = 0; (pattern[i] != '\0') && (j < sizeof(name)); i++) {
>-		switch (pattern[i]) {
>-		case '%':
>-			switch (pattern[i + 1]) {
>-			case '%':
>-				name[j++] = pattern[i];
>-				i++;
>-				break;
>-			case 'U':
>-				j += snprintf(name + j, sizeof(name) - j,
>-					      "%lu", (unsigned long) uid);
>-				i++;
>-				break;
>-			}
>-			break;
>-		default:
>-			name[j++] = pattern[i];
>-			break;
>-		}
>-	}
>-	if ((j > 0) && (j < sizeof(name)))
>-		return strndup(name, MAX_CCNAME_LEN);
>-	else
>-		return NULL;
>-}
>-
>-/* search for a credcache that looks like a likely candidate */
>-static char *find_krb5_cc(const char *dirname, uid_t uid,
>-			  char **best_cache, time_t *best_time)
>-{
>-	struct dirent **namelist;
>-	struct stat sbuf;
>-	char ccname[MAX_CCNAME_LEN], *credpath;
>-	int i, n;
>-	time_t cred_time;
>-
>-	n = scandir(dirname, &namelist, krb5cc_filter, NULL);
>-	if (n < 0) {
>-		syslog(LOG_DEBUG, "%s: scandir error on directory '%s': %s",
>-		       __func__, dirname, strerror(errno));
>-		return NULL;
>-	}
>-
>-	for (i = 0; i < n; i++) {
>-		snprintf(ccname, sizeof(ccname), "FILE:%s/%s", dirname,
>-			 namelist[i]->d_name);
>-		credpath = ccname + 5;
>-		syslog(LOG_DEBUG, "%s: considering %s", __func__, credpath);
>-
>-		if (lstat(credpath, &sbuf)) {
>-			syslog(LOG_DEBUG, "%s: stat error on '%s': %s",
>-			       __func__, credpath, strerror(errno));
>-			free(namelist[i]);
>-			continue;
>-		}
>-		if (sbuf.st_uid != uid) {
>-			syslog(LOG_DEBUG, "%s: %s is owned by %u, not %u",
>-			       __func__, credpath, sbuf.st_uid, uid);
>-			free(namelist[i]);
>-			continue;
>-		}
>-		if (S_ISDIR(sbuf.st_mode)) {
>-			snprintf(ccname, sizeof(ccname), "DIR:%s/%s", dirname,
>-				 namelist[i]->d_name);
>-			credpath = ccname + 4;
>-		} else
>-		if (!S_ISREG(sbuf.st_mode)) {
>-			syslog(LOG_DEBUG, "%s: %s is not a regular file",
>-			       __func__, credpath);
>-			free(namelist[i]);
>-			continue;
>-		}
>-		if (!(cred_time = get_tgt_time(ccname))) {
>-			syslog(LOG_DEBUG, "%s: %s is not a valid credcache.",
>-			       __func__, ccname);
>-			free(namelist[i]);
>-			continue;
>-		}
>-
>-		if (cred_time <= *best_time) {
>-			syslog(LOG_DEBUG, "%s: %s expires sooner than current "
>-			       "best.", __func__, ccname);
>-			free(namelist[i]);
>-			continue;
>-		}
>-
>-		syslog(LOG_DEBUG, "%s: %s is valid ccache", __func__, ccname);
>-		free(*best_cache);
>-		*best_cache = strndup(ccname, MAX_CCNAME_LEN);
>-		*best_time = cred_time;
>-		free(namelist[i]);
>-	}
>-	free(namelist);
>-
>-	return *best_cache;
>-}
>-
> static int
> cifs_krb5_get_req(const char *host, const char *ccname,
> 		  DATA_BLOB * mechtoken, DATA_BLOB * sess_key)
>@@ -841,13 +753,12 @@ int main(const int argc, char *const argv[])
> 	unsigned int have;
> 	long rc = 1;
> 	int c, try_dns = 0, legacy_uid = 0;
>-	char *buf, *ccdir = NULL, *ccname = NULL, *best_cache = NULL;
>+	char *buf, *ccname = NULL;
> 	char hostbuf[NI_MAXHOST], *host;
> 	struct decoded_args arg;
> 	const char *oid;
> 	uid_t uid;
> 	char *keytab_name = NULL;
>-	time_t best_time = 0;
> 
> 	hostbuf[0] = '\0';
> 	memset(&arg, 0, sizeof(arg));
>@@ -954,13 +865,8 @@ int main(const int argc, char *const argv[])
> 		syslog(LOG_ERR, "setuid: %s", strerror(errno));
> 		goto out;
> 	}
>-	ccdir = resolve_krb5_dir(CIFS_DEFAULT_KRB5_USER_DIR, uid);
>-	if (ccdir != NULL)
>-		find_krb5_cc(ccdir, uid, &best_cache, &best_time);
>-	ccname = find_krb5_cc(CIFS_DEFAULT_KRB5_DIR, uid, &best_cache,
>-			      &best_time);
>-	SAFE_FREE(ccdir);
> 
>+	ccname = get_default_cc();
> 	/* Couldn't find credcache? Try to use keytab */
> 	if (ccname == NULL && arg.username != NULL)
> 		ccname = init_cc_from_keytab(keytab_name, arg.username);
>-- 
>2.7.4
>

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

* Re: [cifs-utils PATCH 2/3] cifs.upcall: use krb5 routines to get default ccname
       [not found]         ` <039A665D-75B4-4008-85F9-6CAE8BFB2E15-q9hIisBwmLrYtjvyW6yDsg@public.gmane.org>
@ 2016-10-14 18:31           ` Jeff Layton
  0 siblings, 0 replies; 8+ messages in thread
From: Jeff Layton @ 2016-10-14 18:31 UTC (permalink / raw)
  To: Dey, John F, linux-cifs-u79uwXL29TY76Z2rM5mHXA

If you mean the set that I proposed in late August, then yes...that
went into cifs-utils v6.6.

Thanks,
Jeff

On Fri, 2016-10-14 at 18:09 +0000, Dey, John F wrote:
> Are the default_ccache_name patches making it upstream? 
> 
> John Dey
> 
> 
> 
> 
> 
> 
> On 8/22/16, 5:29 AM, "Jeff Layton" <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> 
> > 
> > Currently we end up groveling around in /tmp, trying to guess what the
> > credcache will be. Instead, just get the default ccname for the user,
> > and then see if it has a valid tgt. If it doesn't then we try to use
> > the keytab to init the credcache before proceeding.
> > 
> > Signed-off-by: Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
> > ---
> > cifs.upcall.c | 148 +++++++++++-----------------------------------------------
> > 1 file changed, 27 insertions(+), 121 deletions(-)
> > 
> > diff --git a/cifs.upcall.c b/cifs.upcall.c
> > index e8544c2b68ad..d0f6d089d8e1 100644
> > --- a/cifs.upcall.c
> > +++ b/cifs.upcall.c
> > @@ -52,12 +52,6 @@
> > #include "spnego.h"
> > #include "cifs_spnego.h"
> > 
> > -#define	CIFS_DEFAULT_KRB5_DIR		"/tmp"
> > -#define	CIFS_DEFAULT_KRB5_USER_DIR	"/run/user/%U"
> > -#define	CIFS_DEFAULT_KRB5_PREFIX	"krb5cc"
> > -
> > -#define	MAX_CCNAME_LEN			PATH_MAX + 5
> > -
> > static const char *prog = "cifs.upcall";
> > typedef enum _sectype {
> > 	NONE = 0,
> > @@ -178,13 +172,34 @@ err_cache:
> > 	return credtime;
> > }
> > 
> > -static int krb5cc_filter(const struct dirent *dirent)
> > +static char *
> > +get_default_cc(void)
> > {
> > -	/* subtract 1 for the null terminator */
> > -	return !strncmp(dirent->d_name, CIFS_DEFAULT_KRB5_PREFIX,
> > -			sizeof(CIFS_DEFAULT_KRB5_PREFIX) - 1);
> > +	krb5_error_code ret;
> > +	const char *ccname;
> > +	char *rcc = NULL;
> > +	krb5_context context = NULL;
> > +
> > +	ret = krb5_init_context(&context);
> > +	if (ret) {
> > +		syslog(LOG_DEBUG, "krb5_init_context: %d", (int)ret);
> > +		return NULL;
> > +	}
> > +
> > +	ccname = krb5_cc_default_name(context);
> > +	if (!ccname) {
> > +		syslog(LOG_DEBUG, "krb5_cc_default returned NULL.");
> > +		goto out_free_context;
> > +	}
> > +
> > +	if (get_tgt_time(ccname))
> > +		rcc = strdup(ccname);
> > +out_free_context:
> > +	krb5_free_context(context);
> > +	return rcc;
> > }
> > 
> > +
> > static char *
> > init_cc_from_keytab(const char *keytab_name, const char *user)
> > {
> > @@ -263,109 +278,6 @@ icfk_cleanup:
> > 	return ccname;
> > }
> > 
> > -/* resolve a pattern to an actual directory path */
> > -static char *resolve_krb5_dir(const char *pattern, uid_t uid)
> > -{
> > -	char name[MAX_CCNAME_LEN];
> > -	int i;
> > -	size_t j;
> > -	for (i = 0, j = 0; (pattern[i] != '\0') && (j < sizeof(name)); i++) {
> > -		switch (pattern[i]) {
> > -		case '%':
> > -			switch (pattern[i + 1]) {
> > -			case '%':
> > -				name[j++] = pattern[i];
> > -				i++;
> > -				break;
> > -			case 'U':
> > -				j += snprintf(name + j, sizeof(name) - j,
> > -					      "%lu", (unsigned long) uid);
> > -				i++;
> > -				break;
> > -			}
> > -			break;
> > -		default:
> > -			name[j++] = pattern[i];
> > -			break;
> > -		}
> > -	}
> > -	if ((j > 0) && (j < sizeof(name)))
> > -		return strndup(name, MAX_CCNAME_LEN);
> > -	else
> > -		return NULL;
> > -}
> > -
> > -/* search for a credcache that looks like a likely candidate */
> > -static char *find_krb5_cc(const char *dirname, uid_t uid,
> > -			  char **best_cache, time_t *best_time)
> > -{
> > -	struct dirent **namelist;
> > -	struct stat sbuf;
> > -	char ccname[MAX_CCNAME_LEN], *credpath;
> > -	int i, n;
> > -	time_t cred_time;
> > -
> > -	n = scandir(dirname, &namelist, krb5cc_filter, NULL);
> > -	if (n < 0) {
> > -		syslog(LOG_DEBUG, "%s: scandir error on directory '%s': %s",
> > -		       __func__, dirname, strerror(errno));
> > -		return NULL;
> > -	}
> > -
> > -	for (i = 0; i < n; i++) {
> > -		snprintf(ccname, sizeof(ccname), "FILE:%s/%s", dirname,
> > -			 namelist[i]->d_name);
> > -		credpath = ccname + 5;
> > -		syslog(LOG_DEBUG, "%s: considering %s", __func__, credpath);
> > -
> > -		if (lstat(credpath, &sbuf)) {
> > -			syslog(LOG_DEBUG, "%s: stat error on '%s': %s",
> > -			       __func__, credpath, strerror(errno));
> > -			free(namelist[i]);
> > -			continue;
> > -		}
> > -		if (sbuf.st_uid != uid) {
> > -			syslog(LOG_DEBUG, "%s: %s is owned by %u, not %u",
> > -			       __func__, credpath, sbuf.st_uid, uid);
> > -			free(namelist[i]);
> > -			continue;
> > -		}
> > -		if (S_ISDIR(sbuf.st_mode)) {
> > -			snprintf(ccname, sizeof(ccname), "DIR:%s/%s", dirname,
> > -				 namelist[i]->d_name);
> > -			credpath = ccname + 4;
> > -		} else
> > -		if (!S_ISREG(sbuf.st_mode)) {
> > -			syslog(LOG_DEBUG, "%s: %s is not a regular file",
> > -			       __func__, credpath);
> > -			free(namelist[i]);
> > -			continue;
> > -		}
> > -		if (!(cred_time = get_tgt_time(ccname))) {
> > -			syslog(LOG_DEBUG, "%s: %s is not a valid credcache.",
> > -			       __func__, ccname);
> > -			free(namelist[i]);
> > -			continue;
> > -		}
> > -
> > -		if (cred_time <= *best_time) {
> > -			syslog(LOG_DEBUG, "%s: %s expires sooner than current "
> > -			       "best.", __func__, ccname);
> > -			free(namelist[i]);
> > -			continue;
> > -		}
> > -
> > -		syslog(LOG_DEBUG, "%s: %s is valid ccache", __func__, ccname);
> > -		free(*best_cache);
> > -		*best_cache = strndup(ccname, MAX_CCNAME_LEN);
> > -		*best_time = cred_time;
> > -		free(namelist[i]);
> > -	}
> > -	free(namelist);
> > -
> > -	return *best_cache;
> > -}
> > -
> > static int
> > cifs_krb5_get_req(const char *host, const char *ccname,
> > 		  DATA_BLOB * mechtoken, DATA_BLOB * sess_key)
> > @@ -841,13 +753,12 @@ int main(const int argc, char *const argv[])
> > 	unsigned int have;
> > 	long rc = 1;
> > 	int c, try_dns = 0, legacy_uid = 0;
> > -	char *buf, *ccdir = NULL, *ccname = NULL, *best_cache = NULL;
> > +	char *buf, *ccname = NULL;
> > 	char hostbuf[NI_MAXHOST], *host;
> > 	struct decoded_args arg;
> > 	const char *oid;
> > 	uid_t uid;
> > 	char *keytab_name = NULL;
> > -	time_t best_time = 0;
> > 
> > 	hostbuf[0] = '\0';
> > 	memset(&arg, 0, sizeof(arg));
> > @@ -954,13 +865,8 @@ int main(const int argc, char *const argv[])
> > 		syslog(LOG_ERR, "setuid: %s", strerror(errno));
> > 		goto out;
> > 	}
> > -	ccdir = resolve_krb5_dir(CIFS_DEFAULT_KRB5_USER_DIR, uid);
> > -	if (ccdir != NULL)
> > -		find_krb5_cc(ccdir, uid, &best_cache, &best_time);
> > -	ccname = find_krb5_cc(CIFS_DEFAULT_KRB5_DIR, uid, &best_cache,
> > -			      &best_time);
> > -	SAFE_FREE(ccdir);
> > 
> > +	ccname = get_default_cc();
> > 	/* Couldn't find credcache? Try to use keytab */
> > 	if (ccname == NULL && arg.username != NULL)
> > 		ccname = init_cc_from_keytab(keytab_name, arg.username);
> > -- 
> > 2.7.4
> > 

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

end of thread, other threads:[~2016-10-14 18:31 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-22 12:29 [cifs-utils PATCH 0/3] cifs-utils: overhaul of cifs.upcall krb5 handling Jeff Layton
     [not found] ` <1471868962-7312-1-git-send-email-jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
2016-08-22 12:29   ` [cifs-utils PATCH 1/3] aclocal: fix typo in idmap.m4 Jeff Layton
2016-08-22 12:29   ` [cifs-utils PATCH 2/3] cifs.upcall: use krb5 routines to get default ccname Jeff Layton
     [not found]     ` <1471868962-7312-3-git-send-email-jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
2016-10-14 18:09       ` Dey, John F
     [not found]         ` <039A665D-75B4-4008-85F9-6CAE8BFB2E15-q9hIisBwmLrYtjvyW6yDsg@public.gmane.org>
2016-10-14 18:31           ` Jeff Layton
2016-08-22 12:29   ` [cifs-utils PATCH 3/3] cifs.upcall: make the krb5_context a static global variable Jeff Layton
2016-08-23 19:21   ` [cifs-utils PATCH 0/3] cifs-utils: overhaul of cifs.upcall krb5 handling Dey, John F
     [not found]     ` <9A891926-AE7D-4916-AB71-A49D4A1588BC-q9hIisBwmLrYtjvyW6yDsg@public.gmane.org>
2016-08-24 11:06       ` Jeff Layton

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.