On Sat, 2017-02-11 at 08:41 -0500, Jeff Layton wrote:
> Chad reported that he was seeing a regression in cifs-utils-6.6. Prior
> to that, cifs.upcall was able to find credcaches in non-default FILE:
> locations, but with the rework of that code, that ability was lost.
>
> Unfortunately, the krb5 library design doesn't really take into account
> the fact that we might need to find a credcache in a process that isn't
> descended from the session.
>
> When the kernel does an upcall, it passes several bits of info about the
> task that initiated the upcall. One of those things is the PID (the
> tgid, in particular). We can use that info to reach into the
> /proc/<pid>/environ file for the process, and grab whatever value of
> KRB5CCNAME is there. This patch adds this ability to cifs.upcall.
>
> I'm not 100% convinced that this is a good idea however, so for now,
> this is disabled unless the command line has a '-e' switch. Anyone
> wishing to play with this should edit their /etc/request-key.conf files
> accordingly.
>
Meant to put a Reported-by: here for Chad. I'll do that before merging.
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxx>
> ---
> cifs.upcall.8.in | 10 ++++
> cifs.upcall.c | 148 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
> 2 files changed, 152 insertions(+), 6 deletions(-)
>
FWIW, this patch works as expected in cursory, light testing. This
should be superior to the earlier method where we scanned in /tmp as
well -- more efficient, and also able to find non-FILE: credcaches, as
well as ones that lie in other places besides /tmp.
My main concern is that we have to do the environ file scraping while
privileged, and that we end up trusting some info from a userland
process that triggered the upcall in KRB5CCNAME. Does this open any
potential attack vectors that I'm not considering?
diff --git a/cifs.upcall.8.in b/cifs.upcall.8.in
> index 50f79d18331d..06c6d3a02c2f 100644
> --- a/cifs.upcall.8.in
> +++ b/cifs.upcall.8.in
> @@ -38,6 +38,16 @@ for a particular key type\&. While it can be run directly from the command\-line
> This option is deprecated and is currently ignored\&.
> .RE
> .PP
> +\-\-env-probe|\-e
> +.RS 4
> +Allow cifs.upcall to do an environment probe to help find the credential
> +cache. This can assist the program with finding credential caches in
> +non-default locations. If this is set, then the program will attempt to
> +retrieve the KRB5CCNAME environment variable from the process that initiated
> +the upcall, and then set that in the upcall process for use when searching for
> +a credcache.
> +.RE
> +.PP
> \--krb5conf=/path/to/krb5.conf|-k /path/to/krb5.conf
> .RS 4
> This option allows administrators to set an alternate location for the
> diff --git a/cifs.upcall.c b/cifs.upcall.c
> index 8f146c92b4a5..1b2c6c19e39e 100644
> --- a/cifs.upcall.c
> +++ b/cifs.upcall.c
> @@ -40,6 +40,7 @@
> #include <dirent.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> +#include <fcntl.h>
> #include <unistd.h>
> #include <keyutils.h>
> #include <time.h>
> @@ -154,11 +155,126 @@ err_cache:
> return credtime;
> }
>
> +#define ENV_PATH_FMT "/proc/%d/environ"
> +#define ENV_PATH_MAXLEN (6 + 10 + 8 + 1)
> +
> +#define ENV_NAME "KRB5CCNAME"
> +#define ENV_PREFIX "KRB5CCNAME="
> +#define ENV_PREFIX_LEN 11
> +
> +#define ENV_BUF_START (4096)
> +#define ENV_BUF_MAX (131072)
> +
> +/**
> + * get_cachename_from_process_env - scrape value of $KRB5CCNAME out of the
> + * initiating process' environment.
> + * @pid: initiating pid value from the upcall string
> + *
> + * Open the /proc/<pid>/environ file for the given pid, and scrape it for
> + * KRB5CCNAME entries.
> + *
> + * We start with a page-size buffer, and then progressively double it until
> + * we can slurp in the whole thing.
> + *
> + * Note that this is not entirely reliable. If the process is sitting in a
> + * container or something, then this is almost certainly not going to point
> + * where you expect.
> + *
> + * Probably it just won't work, but could a user use this to trick cifs.upcall
> + * into reading a file outside the container, by setting KRB5CCNAME in a
> + * crafty way?
> + *
> + * YMMV here.
> + */
> +static char *
> +get_cachename_from_process_env(pid_t pid)
> +{
> + int fd, ret;
> + ssize_t buflen;
> + ssize_t bufsize = ENV_BUF_START;
> + char pathname[ENV_PATH_MAXLEN];
> + char *cachename = NULL;
> + char *buf = NULL, *pos;
> +
> + if (!pid) {
> + syslog(LOG_DEBUG, "%s: pid == 0\n", __func__);
> + return NULL;
> + }
> +
> + pathname[ENV_PATH_MAXLEN - 1] = '\0';
> + ret = snprintf(pathname, ENV_PATH_MAXLEN, ENV_PATH_FMT, pid);
> + if (ret >= ENV_PATH_MAXLEN) {
> + syslog(LOG_DEBUG, "%s: unterminated path!\n", __func__);
> + return NULL;
> + }
> +
> + syslog(LOG_DEBUG, "%s: pathname=%s\n", __func__, pathname);
> + fd = open(pathname, O_RDONLY);
> + if (fd < 0) {
> + syslog(LOG_DEBUG, "%s: open failed: %d\n", __func__, errno);
> + return NULL;
> + }
> +retry:
> + if (bufsize > ENV_BUF_MAX) {
> + syslog(LOG_DEBUG, "%s: buffer too big: %d\n", __func__, errno);
> + goto out_close;
> + }
> +
> + buf = malloc(bufsize);
> + if (!buf) {
> + syslog(LOG_DEBUG, "%s: malloc failure\n", __func__);
> + goto out_close;
> + }
> +
> + buflen = read(fd, buf, bufsize);
> + if (buflen < 0) {
> + syslog(LOG_DEBUG, "%s: read failed: %d\n", __func__, errno);
> + goto out_close;
> + }
> +
> + if (buflen == bufsize) {
> + /* We read to the end of the buffer, double and try again */
> + syslog(LOG_DEBUG, "%s: read to end of buffer (%zu bytes)\n",
> + __func__, bufsize);
> + free(buf);
> + bufsize *= 2;
> + if (lseek(fd, 0, SEEK_SET) < 0)
> + goto out_close;
> + goto retry;
> + }
> +
> + pos = buf;
> + while (buflen > 0) {
> + size_t len = strnlen(pos, buflen);
> +
> + if (len > ENV_PREFIX_LEN &&
> + !memcmp(pos, ENV_PREFIX, ENV_PREFIX_LEN)) {
> + cachename = strndup(pos + ENV_PREFIX_LEN,
> + len - ENV_PREFIX_LEN);
> + syslog(LOG_DEBUG, "%s: cachename = %s\n",
> + __func__, cachename);
> + break;
> + }
> + buflen -= (len + 1);
> + pos += (len + 1);
> + }
> + free(buf);
> +out_close:
> + close(fd);
> + return cachename;
> +}
> +
> static krb5_ccache
> -get_default_cc(void)
> +get_existing_cc(const char *env_cachename)
> {
> krb5_error_code ret;
> krb5_ccache cc;
> + char *cachename;
> +
> + if (env_cachename) {
> + if (setenv(ENV_NAME, env_cachename, 1))
> + syslog(LOG_DEBUG, "%s: failed to setenv %d\n", __func__, errno);
> + }
>
> ret = krb5_cc_default(context, &cc);
> if (ret) {
> @@ -166,6 +282,14 @@ get_default_cc(void)
> return NULL;
> }
>
> + ret = krb5_cc_get_full_name(context, cc, &cachename);
> + if (ret) {
> + syslog(LOG_DEBUG, "%s: krb5_cc_get_full_name failed: %d\n", __func__, ret);
> + } else {
> + syslog(LOG_DEBUG, "%s: default ccache is %s\n", __func__, cachename);
> + krb5_free_string(context, cachename);
> + }
> +
> if (!get_tgt_time(cc)) {
> krb5_cc_close(context, cc);
> cc = NULL;
> @@ -173,7 +297,6 @@ get_default_cc(void)
> return cc;
> }
>
> -
> static krb5_ccache
> init_cc_from_keytab(const char *keytab_name, const char *user)
> {
> @@ -664,10 +787,11 @@ lowercase_string(char *c)
>
> static void usage(void)
> {
> - fprintf(stderr, "Usage: %s [ -K /path/to/keytab] [-k /path/to/krb5.conf] [-t] [-v] [-l] key_serial\n", prog);
> + fprintf(stderr, "Usage: %s [ -K /path/to/keytab] [-k /path/to/krb5.conf] [-e] [-t] [-v] [-l] key_serial\n", prog);
> }
>
> static const struct option long_options[] = {
> + {"env-probe", 0, NULL, 'e'},
> {"krb5conf", 1, NULL, 'k'},
> {"legacy-uid", 0, NULL, 'l'},
> {"trust-dns", 0, NULL, 't'},
> @@ -685,13 +809,14 @@ int main(const int argc, char *const argv[])
> size_t datalen;
> unsigned int have;
> long rc = 1;
> - int c, try_dns = 0, legacy_uid = 0;
> + int c, try_dns = 0, legacy_uid = 0, env_probe = 0;
> char *buf;
> char hostbuf[NI_MAXHOST], *host;
> struct decoded_args arg;
> const char *oid;
> uid_t uid;
> char *keytab_name = NULL;
> + char *env_cachename = NULL;
> krb5_ccache ccache = NULL;
>
> hostbuf[0] = '\0';
> @@ -699,11 +824,15 @@ int main(const int argc, char *const argv[])
>
> openlog(prog, 0, LOG_DAEMON);
>
> - while ((c = getopt_long(argc, argv, "ck:K:ltv", long_options, NULL)) != -1) {
> + while ((c = getopt_long(argc, argv, "cek:K:ltv", long_options, NULL)) != -1) {
> switch (c) {
> case 'c':
> /* legacy option -- skip it */
> break;
> + case 'e':
> + /* do an environmental probe to get the credcache */
> + env_probe++;
> + break;
> case 't':
> try_dns++;
> break;
> @@ -794,6 +923,12 @@ int main(const int argc, char *const argv[])
> goto out;
> }
>
> + /*
> + * Must do this before setuid, as we need ptrace perms to look at
> + * environ file.
> + */
> + env_cachename = get_cachename_from_process_env(env_probe ? arg.pid : 0);
> +
> rc = setuid(uid);
> if (rc == -1) {
> syslog(LOG_ERR, "setuid: %s", strerror(errno));
> @@ -806,7 +941,7 @@ int main(const int argc, char *const argv[])
> goto out;
> }
>
> - ccache = get_default_cc();
> + ccache = get_existing_cc(env_cachename);
> /* Couldn't find credcache? Try to use keytab */
> if (ccache == NULL && arg.username != NULL)
> ccache = init_cc_from_keytab(keytab_name, arg.username);
> @@ -959,6 +1094,7 @@ out:
> SAFE_FREE(arg.ip);
> SAFE_FREE(arg.username);
> SAFE_FREE(keydata);
> + SAFE_FREE(env_cachename);
> syslog(LOG_DEBUG, "Exit status %ld", rc);
> return rc;
> }
--
Jeff Layton <jlayton@xxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
