On Mon, May 23, 2011 at 8:08 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> 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>
>
I think I am OK with that. So basically we are ruling out assigning
nobody/nogroup as uid/gid.
--
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
