On Mon, 23 May 2011 08:04:09 -0500
Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> wrote:
> On Mon, May 23, 2011 at 7:42 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> > On Mon, 23 May 2011 07:26:49 -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   Â|  11 +++-
> >> Âaclocal/idmap.m4 | Â 45 +++++++++++
> >> Âcifs.idmap.c   | Â221 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >> Âcifs.upcall.c  Â|  Â1 -
> >> Âconfigure.ac   |  20 +++++-
> >> Â5 files changed, 294 insertions(+), 4 deletions(-)
> >> Âcreate mode 100644 aclocal/idmap.m4
> >> Âcreate mode 100644 cifs.idmap.c
> >>
> >> diff --git a/Makefile.am b/Makefile.am
> >> index 67a0190..c6eeb22 100644
> >> --- a/Makefile.am
> >> +++ b/Makefile.am
> >> @@ -8,8 +8,10 @@ mount_cifs_LDADD = $(LIBCAP) $(CAPNG_LDADD)
> >>
> >> Âman_MANS = mount.cifs.8
> >>
> >> +sbin_PROGRAMS =
> >> +
> >> Âif CONFIG_CIFSUPCALL
> >> -sbin_PROGRAMS = cifs.upcall
> >> +sbin_PROGRAMS += cifs.upcall
> >> Âcifs_upcall_SOURCES = cifs.upcall.c data_blob.c asn1.c spnego.c util.c
> >> Âcifs_upcall_LDADD = -ltalloc -lkeyutils $(KRB5_LDADD)
> >> Âman_MANS += cifs.upcall.8
> >> @@ -30,3 +32,10 @@ bin_PROGRAMS = cifscreds
> >> Âcifscreds_SOURCES = cifscreds.c resolve_host.c util.c
> >> Âcifscreds_LDADD = -lkeyutils
> >> Âendif
> >> +
> >> +if CONFIG_CIFSIDMAP
> >> +sbin_PROGRAMS += cifs.idmap
> >> +cifs_idmap_SOURCES = cifs.idmap.c
> >> +cifs_idmap_LDADD = -lkeyutils $(WINB_LDADD)
> >> +endif
> >> +
> >> diff --git a/aclocal/idmap.m4 b/aclocal/idmap.m4
> >> new file mode 100644
> >> index 0000000..211d372
> >> --- /dev/null
> >> +++ b/aclocal/idmap.m4
> >> @@ -0,0 +1,45 @@
> >> +dnl Headers needed by wbclient.h
> >> +dnl
> >> +AC_DEFUN([AC_WBCH_COMPL],[
> >> +[
> >> +#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
> >> +]])
> >> +
> >> +dnl Check for wbclient.h header and libwbclietn.so
> >> +dnl
> >> +AC_DEFUN([AC_TEST_WBCHL],[
> >> +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([wbclient.h not found, consider installing libwbclient-devel. Disabling cifs.idmap.])
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â enable_cifsidmap="no"
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â fi
> >> + Â Â Â Â Â Â Â Â Â Â ], [ AC_WBCH_COMPL ])
> >> +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
> >> +])
> >> diff --git a/cifs.idmap.c b/cifs.idmap.c
> >> new file mode 100644
> >> index 0000000..7788bf2
> >> --- /dev/null
> >> +++ b/cifs.idmap.c
> >> @@ -0,0 +1,221 @@
> >> +/*
> >> +* 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>
> >> +#include <keyutils.h>
> >> +#include <stdint.h>
> >> +#include <stdbool.h>
> >> +#include <stdio.h>
> >> +#include <stdlib.h>
> >> +#include <errno.h>
> >> +#include <limits.h>
> >> +#include <wbclient.h>
> >> +
> >> +static const char *prog = "cifs.idmap";
> >> +
> >> +static void usage(void)
> >> +{
> >> + Â Â fprintf(stderr, "Usage: %s key_serial\n", prog);
> >> +}
> >> +
> >> +char *strget(const char *str, char *substr)
> >> +{
> >> + Â Â int len, sublen, retlen;
> >> + Â Â char *retstr, *substrptr;
> >> +
> >> + Â Â sublen = strlen(substr);
> >> + Â Â substrptr = strstr(str, substr);
> >> + Â Â if (substrptr) {
> >> + Â Â Â Â Â Â len = strlen(substrptr);
> >> + Â Â Â Â Â Â substrptr += sublen;
> >> +
> >> + Â Â Â Â Â Â retlen = len - sublen;
> >> + Â Â Â Â Â Â if (retlen > 0) {
> >> + Â Â Â Â Â Â Â Â Â Â retstr = malloc(retlen + 1);
> >> + Â Â Â Â Â Â Â Â Â Â if (retstr) {
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â strncpy(retstr, substrptr, retlen);
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â return retstr;
> >> + Â Â Â Â Â Â Â Â Â Â }
> >> + Â Â Â Â Â Â }
> >> + Â Â }
> >> +
> >> + Â Â return NULL;
> >> +}
> >> +
> >
> > I still worry a bit about bounds checking here, but I think we can trust
> > keyctl_describe_alloc to give us a NULL terminated string, so let's not
> > sweat it for now.
> >
> >> +static int
> >> +cifs_idmap(const key_serial_t key, const char *key_descr)
> >> +{
> >> + Â Â uid_t uid = 0;
> >> + Â Â gid_t gid = 0;;
> >> + Â Â wbcErr rc = 1;
> >> + Â Â char *sidstr = NULL;
> >> + Â Â struct wbcDomainSid sid;
> >> + Â Â struct passwd *pw;
> >> + Â Â struct group *gr;
> >> +
> >> + Â Â /*
> >> + Â Â Â* 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.
> >> + Â Â Â*/
> >> + Â Â sidstr = strget(key_descr, "os:");
> >> + Â Â if (sidstr) {
> >> + Â Â Â Â Â Â rc = wbcStringToSid(sidstr, &sid);
> >> + Â Â Â Â Â Â if (rc)
> >> + Â Â Â Â Â Â Â Â Â Â syslog(LOG_DEBUG, "Invalid owner string: %s, rc: %d",
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â key_descr, rc);
> >> + Â Â Â Â Â Â else {
> >> + Â Â Â Â Â Â Â Â Â Â rc = wbcSidToUid(&sid, &uid);
> >> + Â Â Â Â Â Â Â Â Â Â if (rc)
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â syslog(LOG_DEBUG, "SID %s to uid wbc error: %d",
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â key_descr, 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",
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â key_descr, rc);
> >> + Â Â Â Â Â Â Â Â Â Â else {
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â uid = pw->pw_uid;
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â rc = 0;
> >> + Â Â Â Â Â Â Â Â Â Â }
> > Â Â Â Â Â Â Â Â Â Â Â Â^^^^^^^^^^^
> > Â Â Â Â Â Â Â Â Â Â Â ÂIs it ever possible that the error from the wb
> > Â Â Â Â Â Â Â Â Â Â Â Âlibs will be transient? If so, then mapping
> > Â Â Â Â Â Â Â Â Â Â Â Âthis to nobody could be problematic, right?
> >
>
> We can't say. It is possible that the error from winbind is
> transiet and it is also possible that winbind is not running
> at all.
>
> > Â Â Â Â Â Â Â Â Â Â Â ÂMight it be better to not map it to anything
> > Â Â Â Â Â Â Â Â Â Â Â Âand let the kernel turn this into the mount
> > Â Â Â Â Â Â Â Â Â Â Â Âuid/gid? If "nobody" really is better then you
> > Â Â Â Â Â Â Â Â Â Â Â Âprobably ought to set a short timeout on this
> > Â Â Â Â Â Â Â Â Â Â Â Âkey.
>
> Even if the key is changed to a transiet, the mapping in cache in kernel
> is "permanent" i.e. at least till that cache entry is evicted by shrinker,
> if at all.
>
> What we can do is perhaps return different codes to kernel to only
> mark an entry as mapped when key handling code returns 0, say
> if it returns 1, then it is nobody/nogroup and do not mark an entry
> as mapped. If it is error (negative ret code), do not mark an entry
> as mapped but assign sb uid/gid.
>
That sounds a bit complicated. Why not just exit without instantiating
the key if a successful mapping can't be made? With this scheme, you'll
get different behavior depending on whether cifs.idmap couldn't be
called, or if it got called and the mapping didn't get made.
I think it might be best to have the user get the same behavior in the
event of any failure. Thoughts?
--
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
