On Mon, 23 May 2011 08:04:09 -0500 Shirish Pargaonkar <[email protected]> wrote:
> On Mon, May 23, 2011 at 7:42 AM, Jeff Layton <[email protected]> wrote: > > On Mon, 23 May 2011 07:26:49 -0500 > > [email protected] wrote: > > > >> From: Shirish Pargaonkar <[email protected]> > >> > >> 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 <[email protected]> > >> --- > >> 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 ([email protected]) 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 <[email protected]> -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
