Re: [PATCH] cifs-utils: Create new binary cifs.idmap for sid to uid/gid mapping

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 17 May 2011 21:06:53 -0500
Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> wrote:

> On Tue, May 17, 2011 at 11:09 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> > On Fri, Â6 May 2011 02:33:59 -0500
> > shirishpargaonkar@xxxxxxxxx wrote:
> >
> >> From: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
> >>
> >> Handle cifs.idmap type of key. Extract a SID string from the description
> >> and map it to either an uid or gid using winbind APIs.
> >> If that fails (e.g. because winbind is not installed/running or winbind returns
> >> an error), try to obtain uid of 'nobody' and gid of 'nogroup'.
> >> And if that fails, kernel assigns uid and gid (from mount superblock).
> >>
> >> Enable including winbind header files and idmapping code conditional
> >> to winbind devel rpms (header and library).
> >>
> >> An entry such as this
> >>
> >> create Âcifs.idmap  *    *        /usr/sbin/cifs.idmap %k
> >>
> >> is needed in the file /etc/request-key.conf.
> >>
> >>
> >> Signed-off-by: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
> >> ---
> >> ÂMakefile.am Â| Â Â8 ++
> >> Âcifs.idmap.c | Â215 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >> Âconfigure.ac | Â 58 +++++++++++++++-
> >> Â3 files changed, 280 insertions(+), 1 deletions(-)
> >> Âcreate mode 100644 cifs.idmap.c
> >>
> >> diff --git a/Makefile.am b/Makefile.am
> >> index 67a0190..ad33914 100644
> >> --- a/Makefile.am
> >> +++ b/Makefile.am
> >> @@ -30,3 +30,11 @@ bin_PROGRAMS = cifscreds
> >> Âcifscreds_SOURCES = cifscreds.c resolve_host.c util.c
> >> Âcifscreds_LDADD = -lkeyutils
> >> Âendif
> >> +
> >> +if CONFIG_CIFSIDMAP
> >> +cifs_idmap_sbindir = /usr/local/sbin
> >
> > Â Â Â Â^^^^^^^^^
> > That looks wrong. If I configure with --prefix=/usr then this is still
> > going to go in /usr/local/sbin, right? I think you instead want to
> > append cifs.idmap to sbin_PROGRAMS.
> >
> >> +cifs_idmap_sbin_PROGRAMS = cifs.idmap
> >> +cifs_idmap_SOURCES = cifs.idmap.c
> >> +cifs_idmap_LDADD = -lkeyutils $(WINB_LDADD)
> >> +endif
> >> +
> >> diff --git a/cifs.idmap.c b/cifs.idmap.c
> >> new file mode 100644
> >> index 0000000..0f76873
> >> --- /dev/null
> >> +++ b/cifs.idmap.c
> >> @@ -0,0 +1,215 @@
> >> +/*
> >> +* CIFS idmap helper.
> >> +* Copyright (C) Shirish Pargaonkar (shirishp@xxxxxxxxxx) 2011
> >> +*
> >> +* Used by /sbin/request-key.conf for handling
> >> +* cifs upcall for SID to uig/gid and uid/gid to SID mapping.
> >> +* You should have keyutils installed and add
> >> +* this lines to /etc/request-key.conf file:
> >> +
> >> + Â Âcreate cifs.idmap * * /usr/local/sbin/cifs.idmap %k
> >> +
> >> +* This program is free software; you can redistribute it and/or modify
> >> +* it under the terms of the GNU General Public License as published by
> >> +* the Free Software Foundation; either version 2 of the License, or
> >> +* (at your option) any later version.
> >> +* This program is distributed in the hope that it will be useful,
> >> +* but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >> +* GNU General Public License for more details.
> >> +* You should have received a copy of the GNU General Public License
> >> +* along with this program; if not, write to the Free Software
> >> +* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> >> +*/
> >> +
> >> +#ifdef HAVE_CONFIG_H
> >> +#include "config.h"
> >> +#endif /* HAVE_CONFIG_H */
> >> +
> >> +#include <string.h>
> >> +#include <getopt.h>
> >> +#include <syslog.h>
> >> +#include <dirent.h>
> >> +#include <sys/types.h>
> >> +#include <sys/stat.h>
> >> +#include <unistd.h>
> >> +#ifdef HAVE_KEYUTILS_H
> >> +#include <keyutils.h>
> >> +#endif /* HAVE_KEYUTILS_H */
> > Â Â Â Â^^^^^^^^^^
> > This can't be built w/o keyutils, right? So I don't think we need the
> > #ifdefs.
> >
> >> +#include <stdint.h>
> >> +#include <stdbool.h>
> >> +#include <stdio.h>
> >> +#include <stdlib.h>
> >> +#include <errno.h>
> >> +#include <limits.h>
> >> +#ifdef HAVE_WBCLIENT_H
> >> +#include <wbclient.h>
> >> +#endif /* HAVE_WBCLIENT_H */
> >> +
> >> +static const char *prog = "cifs.idmap";
> >> +
> >> +static void usage(void)
> >> +{
> >> + Â Â syslog(LOG_INFO, "Usage: %s [-t] [-v] [-l] key_serial", prog);
> >> + Â Â fprintf(stderr, "Usage: %s [-t] [-v] [-l] key_serial\n", prog);
> >> +}
> >> +
> >> +#ifdef HAVE_LIBWBCLIENT
> >> +static int
> >> +cifs_idmap(const key_serial_t key, const char *key_descr)
> >> +{
> >> + Â Â int i;
> >> + Â Â uid_t uid = 0;
> >> + Â Â gid_t gid = 0;;
> >> + Â Â wbcErr rc = 1;
> >> + Â Â const char *keyend = key_descr;
> >> + Â Â struct wbcDomainSid sid;
> >> + Â Â struct passwd *pw;
> >> + Â Â struct group *gr;
> >> +
> >> + Â Â /* skip next 4 ';' delimiters to get to description */
> >> + Â Â for (i = 1; i <= 4; ++i) {
> >> + Â Â Â Â Â Â keyend = index(keyend + 1, ';');
> >> + Â Â Â Â Â Â if (!keyend) {
> >> + Â Â Â Â Â Â Â Â Â Â syslog(LOG_ERR, "invalid key description: %s",
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Âkey_descr);
> >> + Â Â Â Â Â Â Â Â Â Â return 1;
> >> + Â Â Â Â Â Â }
> >> + Â Â }
> >> + Â Â keyend++;
> >> +
> >
> > No real bounds checking here. It seems unlikely, but a buggy kernel
> > could send a garbled string and you could walk off the end of it here.
> >
> >> + Â Â /*
> >> + Â Â Â* Use winbind to convert received string to a SID and lookup
> >> + Â Â Â* name and map that SID to an uid. ÂIf either of these
> >> + Â Â Â* function calls return with an error, Âuse system calls to obtain
> >> + Â Â Â* uid of user "nobody". If winbind fails to map a SID to an UID
> >> + Â Â Â* and there is no user named "nobody", return error to the
> >> + Â Â Â* upcall caller. Otherwise instanticate a key using that uid.
> >> + Â Â Â*
> >> + Â Â Â* The same applies to SID and gid mapping. ÂInstead of a
> >> + Â Â Â* user "nobody", user "nogroup" is looked up if winbind
> >> + Â Â Â* fails to map a SID to a gid.
> >> + Â Â Â*/
> >> + Â Â if (strncmp(keyend, "os", 2) == 0) {
> >> + Â Â Â Â Â Â keyend = index(keyend + 1, ':');
> >> + Â Â Â Â Â Â keyend++;
> > Â Â Â Â Â Â Â Â^^^^^^^^^
> > Â Â Â Â Â Â Â Âditto on the bounds checks
> >
> >> + Â Â Â Â Â Â rc = wbcStringToSid(keyend, &sid);
> >> + Â Â Â Â Â Â if (rc)
> >> + Â Â Â Â Â Â Â Â Â Â syslog(LOG_DEBUG, "O strtosid: %s, rc: %d", keyend, rc);
> >> + Â Â Â Â Â Â else {
> >> + Â Â Â Â Â Â Â Â Â Â rc = wbcSidToUid(&sid, &uid);
> >> + Â Â Â Â Â Â Â Â Â Â if (rc)
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â syslog(LOG_DEBUG, "SID %s to uid wbc error: %d",
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â keyend, rc);
> >> + Â Â Â Â Â Â }
> >> + Â Â Â Â Â Â if (rc) { /* either of the two wbcSid functions failed */
> >> + Â Â Â Â Â Â Â Â Â Â pw = getpwnam("nobody");
> >> + Â Â Â Â Â Â Â Â Â Â if (!pw)
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â syslog(LOG_DEBUG, "SID %s to uid pw error: %d",
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â keyend, rc);
> >> + Â Â Â Â Â Â Â Â Â Â else {
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â uid = pw->pw_uid;
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â rc = 0;
> >> + Â Â Â Â Â Â Â Â Â Â }
> >> + Â Â Â Â Â Â }
> >> + Â Â Â Â Â Â if (!rc) { /* SID has been mapped to a uid */
> >> + Â Â Â Â Â Â Â Â Â Â rc = keyctl_instantiate(key, &uid, sizeof(uid_t), 0);
> >> + Â Â Â Â Â Â Â Â Â Â if (rc)
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â syslog(LOG_ERR, "%s: key inst: %s",
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â __func__, strerror(errno));
> >> + Â Â Â Â Â Â }
> >> + Â Â } else if (strncmp(keyend, "gs", 2) == 0) {
> >> + Â Â Â Â Â Â keyend = index(keyend + 1, ':');
> >> + Â Â Â Â Â Â keyend++;
> >> + Â Â Â Â Â Â rc = wbcStringToSid(keyend, &sid);
> >> + Â Â Â Â Â Â if (rc)
> >> + Â Â Â Â Â Â Â Â Â Â syslog(LOG_DEBUG, "O strtosid: %s, rc: %d", keyend, rc);
> >> + Â Â Â Â Â Â else {
> >> + Â Â Â Â Â Â Â Â Â Â rc = wbcSidToGid(&sid, &gid);
> >> + Â Â Â Â Â Â Â Â Â Â if (rc)
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â syslog(LOG_DEBUG, "SID %s to gid wbc error: %d",
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â keyend, rc);
> >> + Â Â Â Â Â Â }
> >> + Â Â Â Â Â Â if (rc) { /* either of the two wbcSid functions failed */
> >> + Â Â Â Â Â Â Â Â Â Â gr = getgrnam("nogroup");
> >> + Â Â Â Â Â Â Â Â Â Â if (!gr)
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â syslog(LOG_DEBUG, "SID %s to gid pw error: %d",
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â keyend, rc);
> >> + Â Â Â Â Â Â Â Â Â Â else {
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â gid = gr->gr_gid;
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â rc = 0;
> >> + Â Â Â Â Â Â Â Â Â Â }
> >> + Â Â Â Â Â Â }
> >> + Â Â Â Â Â Â if (!rc) { /* SID has been mapped to a gid */
> >> + Â Â Â Â Â Â Â Â Â Â rc = keyctl_instantiate(key, &gid, sizeof(gid_t), 0);
> >> + Â Â Â Â Â Â Â Â Â Â if (rc)
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â syslog(LOG_ERR, "%s: key inst: %s",
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â __func__, strerror(errno));
> >> + Â Â Â Â Â Â }
> >> + Â Â } else
> >> + Â Â Â Â Â Â syslog(LOG_DEBUG, "Invalid SID: %s", keyend);
> >> +
> >> + Â Â return rc;
> >> +}
> >> +#endif /* HAVE_LIBWBCLIENT */
> >> +
> >> +int main(const int argc, char *const argv[])
> >> +{
> >> + Â Â int c;
> >> + Â Â long rc = 1;
> >> + Â Â key_serial_t key = 0;
> >> + Â Â char *buf;
> >> +
> >> + Â Â openlog(prog, 0, LOG_DAEMON);
> >> +
> >> + Â Â while ((c = getopt_long(argc, argv, "v", NULL, NULL)) != -1) {
> >> + Â Â Â Â Â Â switch (c) {
> >> + Â Â Â Â Â Â case 'v':
> >> + Â Â Â Â Â Â Â Â Â Â printf("version: %s\n", VERSION);
> >> + Â Â Â Â Â Â Â Â Â Â goto out;
> >> + Â Â Â Â Â Â default:
> >> + Â Â Â Â Â Â Â Â Â Â syslog(LOG_ERR, "unknown option: %c", c);
> >> + Â Â Â Â Â Â Â Â Â Â goto out;
> >> + Â Â Â Â Â Â }
> >> + Â Â }
> >> +
> >> + Â Â /* is there a key? */
> >> + Â Â if (argc <= optind) {
> >> + Â Â Â Â Â Â usage();
> >> + Â Â Â Â Â Â goto out;
> >> + Â Â }
> >> +
> >> + Â Â /* get key and keyring values */
> >> + Â Â errno = 0;
> >> + Â Â key = strtol(argv[optind], NULL, 10);
> >> + Â Â if (errno != 0) {
> >> + Â Â Â Â Â Â key = 0;
> >> + Â Â Â Â Â Â syslog(LOG_ERR, "Invalid key format: %s", strerror(errno));
> >> + Â Â Â Â Â Â goto out;
> >> + Â Â }
> >> +
> >> + Â Â rc = keyctl_describe_alloc(key, &buf);
> >> + Â Â if (rc == -1) {
> >> + Â Â Â Â Â Â syslog(LOG_ERR, "keyctl_describe_alloc failed: %s",
> >> + Â Â Â Â Â Â Â Â Â Âstrerror(errno));
> >> + Â Â Â Â Â Â rc = 1;
> >> + Â Â Â Â Â Â goto out;
> >> + Â Â }
> >> +
> >> + Â Â syslog(LOG_DEBUG, "key description: %s", buf);
> >> +
> >> +#ifdef HAVE_LIBWBCLIENT
> >> + Â Â if ((strncmp(buf, "cifs.idmap", sizeof("cifs.idmap") - 1) == 0))
> >> + Â Â Â Â Â Â rc = cifs_idmap(key, buf);
> >> +#endif /* HAVE_LIBWBCLIENT */
> >> +
> >
> > Doesn't this program require libwbclient? If so, then why the #ifdef's?
> >
> >> +out:
> >> + Â Â /*
> >> + Â Â Â* on error, negatively instantiate the key ourselves so that we can
> >> + Â Â Â* make sure the kernel doesn't hang it off of a searchable keyring
> >> + Â Â Â* and interfere with the next attempt to instantiate the key.
> >> + Â Â Â*/
> >> + Â Â if (rc != 0 && key == 0)
> >> + Â Â Â Â Â Â keyctl_negate(key, 1, KEY_REQKEY_DEFL_DEFAULT);
> >
> > Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â^^^^^^^
> > Is a 1s timeout appropriate here?
> 
> Not sure about the value?  Would 30s or 60s timeout be appropriate?
> Once a key request was fulfilled successfully, cifs module is not likely
> to re-request that at key since it caches for an hour or so.
> And if key request was unsuccessful, cifs module will not re-request
> that key for another five minutes.
> 
> Perhaps we can negatively instantiate it for 60 seconds and be ready
> to handle any requests after that?
> I think any value such as 1, 30, 60 seconds would work?
> 

If that's the case, then there's probably no reason to negatively
instantiate it at all. The kernel will do that on its own with a 60s
timeout (I think).

> >
> >> + Â Â return rc;
> >> +}
> >> diff --git a/configure.ac b/configure.ac
> >> index e0e2a60..d9eaead 100644
> >> --- a/configure.ac
> >> +++ b/configure.ac
> >> @@ -22,13 +22,19 @@ AC_ARG_ENABLE(cifscreds,
> >> Â Â Â enable_cifscreds=$enableval,
> >> Â Â Â enable_cifscreds="no")
> >>
> >> +AC_ARG_ENABLE(cifsidmap,
> >> + Â Â [AC_HELP_STRING([--enable-cifsidmap],
> >> + Â Â Â Â Â Â Â Â Â Â [Create cifs.idmap binary @<:@default=yes@:>@])],
> >> + Â Â enable_cifsidmap=$enableval,
> >> + Â Â enable_cifsidmap="maybe")
> >> +
> >> Â# Checks for programs.
> >> ÂAC_PROG_CC
> >> ÂAC_PROG_SED
> >> ÂAC_GNU_SOURCE
> >>
> >> Â# Checks for header files.
> >> -AC_CHECK_HEADERS([arpa/inet.h ctype.h fcntl.h inttypes.h limits.h mntent.h netdb.h stddef.h stdint.h stdlib.h string.h strings.h sys/mount.h sys/param.h sys/socket.h sys/time.h syslog.h unistd.h], , [AC_MSG_ERROR([necessary header(s) not found])])
> >> +AC_CHECK_HEADERS([arpa/inet.h ctype.h fcntl.h inttypes.h limits.h mntent.h netdb.h stddef.h stdint.h stdbool.h stdlib.h stdio.h errno.h string.h strings.h sys/mount.h sys/param.h sys/socket.h sys/time.h syslog.h unistd.h], , [AC_MSG_ERROR([necessary header(s) not found])])
> >>
> >> Âif test $enable_cifsupcall != "no"; then
> >> Â Â Â AC_CHECK_HEADERS([krb5.h krb5/krb5.h])
> >> @@ -89,6 +95,55 @@ if test $enable_cifsupcall != "no"; then
> >> Â Â Â AC_SUBST(KRB5_LDADD)
> >> Âfi
> >>
> >> +if test $enable_cifsidmap != "no"; then
> >> + Â Â AC_CHECK_HEADERS([keyutils.h], , [
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â if test "$enable_cifsidmap" = "yes"; then
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â AC_MSG_ERROR([keyutils.h not found, consider installing keyutils-libs-devel.])
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â else
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â AC_MSG_WARN([keyutils.h not found, consider installing keyutils-libs-devel. Disabling cifs.idmap.])
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â enable_cifsidmap="no"
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â fi
> >> + Â Â Â Â Â Â Â Â Â Â ])
> >> +fi
> >> +
> >
> > We're already doing a AC_CHECK_HEADERS for keyutils.h -- I don't think
> > we want to do it again. Just use the same check for both cifs.upcall
> > and cifs.idmap.
> >
> >> +if test $enable_cifsidmap != "no"; then
> >> + Â Â AC_CHECK_HEADERS([wbclient.h], , [
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â if test "$enable_cifsidmap" = "yes"; then
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â AC_MSG_ERROR([wbclient.h not found, consider installing libwbclient-devel..])
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â else
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â AC_MSG_WARN([keyutils.h not found, consider installing libwbclient-devel. Disabling cifs.idmap.])
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â enable_cifsidmap="no"
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â fi
> >> + Â Â Â Â Â Â Â Â Â Â ],
> >> +[#ifdef HAVE_STDINT_H
> >> +#include <stdint.h>
> >> +#endif
> >> +]
> >> +[#ifdef HAVE_STDBOOL_H
> >> +#include <stdbool.h>
> >> +#endif
> >> +]
> >> +[#ifdef HAVE_STDIO_H
> >> +#include <stdio.h>
> >> +#endif
> >> +]
> >> +[#ifdef HAVE_STDLIB_H
> >> +#include <stdlib.h>
> >> +#endif
> >> +]
> >> +[#ifdef HAVE_ERRNO_H
> >> +#include <errno.h>
> >> +#endif
> >> +]
> >> +)
> >> +fi
> >> +
> >> +if test $enable_cifsidmap != "no"; then
> >> + Â Â AC_CHECK_LIB([wbclient], [wbcStringToSid],
> >> + Â Â Â Â Â Â [ WINB_LDADD='-lwbclient' ] [ AC_DEFINE(HAVE_LIBWBCLIENT, 1, ["define var have_libwbclient"]) ], [AC_MSG_ERROR([no functioning wbclient library found!])])
> >> + Â Â AC_SUBST(WINB_LDADD)
> >> +fi
> >> +
> >
> >
> > Bleh, that's pretty messy. Maybe this should go into a separate
> > file/function in aclocal/ ?
> >
> >
> >> Âif test $enable_cifscreds = "yes"; then
> >> Â Â Â AC_CHECK_HEADERS([keyutils.h], , [AC_MSG_ERROR([keyutils.h not found, consider installing keyutils-libs-devel.])])
> >> Âfi
> >> @@ -140,6 +195,7 @@ LIBS=$cu_saved_libs
> >>
> >> ÂAM_CONDITIONAL(CONFIG_CIFSUPCALL, [test "$enable_cifsupcall" != "no"])
> >> ÂAM_CONDITIONAL(CONFIG_CIFSCREDS, [test "$enable_cifscreds" = "yes"])
> >> +AM_CONDITIONAL(CONFIG_CIFSIDMAP, [test "$enable_cifsidmap" != "no"])
> >>
> >> ÂLIBCAP_NG_PATH
> >>
> >
> >
> > --
> > Jeff Layton <jlayton@xxxxxxxxxx>
> >
> --
> 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


-- 
Jeff Layton <jlayton@xxxxxxxxxx>
--
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


[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux