From patchwork Fri Mar 26 14:25:31 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Layton X-Patchwork-Id: 48640 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.samba.org (fn.samba.org [216.83.154.106]) by ozlabs.org (Postfix) with ESMTP id 324B4B7CCD for ; Sat, 27 Mar 2010 01:26:05 +1100 (EST) Received: from fn.samba.org (localhost [127.0.0.1]) by lists.samba.org (Postfix) with ESMTP id 2AEA64664F; Fri, 26 Mar 2010 08:26:05 -0600 (MDT) X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on fn.samba.org X-Spam-Level: X-Spam-Status: No, score=-9.9 required=3.8 tests=BAYES_00, RCVD_IN_DNSWL_HI, SPF_HELO_PASS,SPF_NEUTRAL autolearn=ham version=3.2.5 X-Original-To: linux-cifs-client@lists.samba.org Delivered-To: linux-cifs-client@lists.samba.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by lists.samba.org (Postfix) with ESMTP id B026946657 for ; Fri, 26 Mar 2010 08:25:46 -0600 (MDT) Received: from int-mx05.intmail.prod.int.phx2.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.18]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o2QEPjEs005345 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 26 Mar 2010 10:25:45 -0400 Received: from localhost.localdomain (vpn-10-105.rdu.redhat.com [10.11.10.105]) by int-mx05.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o2QEPdCd026868 for ; Fri, 26 Mar 2010 10:25:45 -0400 From: Jeff Layton To: linux-cifs-client@lists.samba.org Date: Fri, 26 Mar 2010 10:25:31 -0400 Message-Id: <1269613542-6402-9-git-send-email-jlayton@samba.org> In-Reply-To: <1269613542-6402-1-git-send-email-jlayton@samba.org> References: <1269613542-6402-1-git-send-email-jlayton@samba.org> X-Scanned-By: MIMEDefang 2.67 on 10.5.11.18 Subject: [linux-cifs-client] [PATCH 08/19] mount.cifs: eliminate "legacy" setuid behavior X-BeenThere: linux-cifs-client@lists.samba.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: The Linux CIFS VFS client List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: linux-cifs-client-bounces@lists.samba.org Errors-To: linux-cifs-client-bounces@lists.samba.org From: Jeff Layton This behavior is demonstrably unsafe and not something we want to support going forward. Signed-off-by: Jeff Layton --- mount.cifs.c | 90 ++++++---------------------------------------------------- 1 files changed, 9 insertions(+), 81 deletions(-) diff --git a/mount.cifs.c b/mount.cifs.c index a86812c..46a8161 100644 --- a/mount.cifs.c +++ b/mount.cifs.c @@ -101,21 +101,6 @@ #define CIFS_DISABLE_SETUID_CHECK 0 /* - * By default, mount.cifs follows the conventions set forth by /bin/mount - * for user mounts. That is, it requires that the mount be listed in - * /etc/fstab with the "user" option when run as an unprivileged user and - * mount.cifs is setuid root. - * - * Older versions of mount.cifs however were "looser" in this regard. When - * made setuid root, a user could run mount.cifs directly and mount any share - * on a directory owned by that user. - * - * The legacy behavior is now disabled by default. To reenable it, set the - * following #define to true. - */ -#define CIFS_LEGACY_SETUID_CHECK 0 - -/* * When an unprivileged user runs a setuid mount.cifs, we set certain mount * flags by default. These defaults can be changed here. */ @@ -142,36 +127,6 @@ const char *cifs_fstype = "cifs"; static int parse_unc(char *unc_name, struct parsed_mount_info *parsed_info); -#if CIFS_LEGACY_SETUID_CHECK -static int -check_mountpoint(const char *progname, char *mountpoint) -{ - /* do extra checks on mountpoint for legacy setuid behavior */ - if (!getuid() || geteuid()) - return 0; - - if (statbuf.st_uid != getuid()) { - fprintf(stderr, "%s: %s is not owned by user\n", progname, - mountpoint); - return EX_USAGE; - } - - if ((statbuf.st_mode & S_IRWXU) != S_IRWXU) { - fprintf(stderr, "%s: invalid permissions on %s\n", progname, - mountpoint); - return EX_USAGE; - } - - return 0; -} -#else /* CIFS_LEGACY_SETUID_CHECK */ -static int -check_mountpoint(const char *progname, char *mountpoint) -{ - return 0; -} -#endif /* CIFS_LEGACY_SETUID_CHECK */ - #if CIFS_DISABLE_SETUID_CHECK static int check_setuid(void) @@ -195,14 +150,6 @@ check_setuid(void) } #endif /* CIFS_DISABLE_SETUID_CHECK */ -#if CIFS_LEGACY_SETUID_CHECK -static int -check_fstab(const char *progname, char *mountpoint, char *devname, - char **options) -{ - return 0; -} -#else /* CIFS_LEGACY_SETUID_CHECK */ static int check_fstab(const char *progname, char *mountpoint, char *devname, char **options) @@ -241,7 +188,6 @@ check_fstab(const char *progname, char *mountpoint, char *devname, *options = strdup(mnt->mnt_opts); return 0; } -#endif /* CIFS_LEGACY_SETUID_CHECK */ /* BB finish BB @@ -1079,7 +1025,6 @@ int main(int argc, char ** argv) char * orgoptions = NULL; char * mountpoint = NULL; char * options = NULL; - char * resolved_path = NULL; char * dev_name; char *currentaddress, *nextaddress; int rc = 0; @@ -1256,7 +1201,7 @@ int main(int argc, char ** argv) dev_name = argv[optind]; mountpoint = argv[optind + 1]; - /* make sure mountpoint is legit */ + /* chdir into mountpoint as soon as possible */ rc = chdir(mountpoint); if (rc) { fprintf(stderr, "Couldn't chdir to %s: %s\n", mountpoint, @@ -1265,9 +1210,13 @@ int main(int argc, char ** argv) goto mount_exit; } - rc = check_mountpoint(thisprogram, mountpoint); - if (rc) + mountpoint = realpath(".", NULL); + if(!mountpoint) { + fprintf(stderr, "Unable to resolve %s to canonical path: %s\n", + mountpoint, strerror(errno)); + rc = EX_SYSERR; goto mount_exit; + } /* sanity check for unprivileged mounts */ if (getuid()) { @@ -1291,13 +1240,11 @@ int main(int argc, char ** argv) } if (getuid()) { -#if !CIFS_LEGACY_SETUID_CHECK if (!(parsed_info->flags & (MS_USERS|MS_USER))) { fprintf(stderr, "%s: permission denied\n", thisprogram); rc = EX_USAGE; goto mount_exit; } -#endif /* !CIFS_LEGACY_SETUID_CHECK */ if (geteuid()) { fprintf(stderr, "%s: not installed setuid - \"user\" " @@ -1318,25 +1265,6 @@ int main(int argc, char ** argv) if (rc) goto mount_exit; - /* BB save off path and pop after mount returns? */ - resolved_path = (char *)malloc(PATH_MAX+1); - if (!resolved_path) { - fprintf(stderr, "Unable to allocate memory.\n"); - rc = EX_SYSERR; - goto mount_exit; - } - - /* Note that if we can not canonicalize the name, we get - another chance to see if it is valid when we chdir to it */ - if(!realpath(".", resolved_path)) { - fprintf(stderr, "Unable to resolve %s to canonical path: %s\n", - mountpoint, strerror(errno)); - rc = EX_SYSERR; - goto mount_exit; - } - - mountpoint = resolved_path; - if (!parsed_info->got_user) { /* * Note that the password will not be retrieved from the @@ -1420,7 +1348,7 @@ mount_retry: } if(verboseflag) - fprintf(stderr, "\nmount.cifs kernel mount options: %s", options); + fprintf(stderr, "mount.cifs kernel mount options: %s\n", options); if (parsed_info->got_password) { /* @@ -1486,6 +1414,7 @@ mount_retry: rc = EX_FILEIO; goto mount_exit; } + mountent.mnt_fsname = dev_name; mountent.mnt_dir = mountpoint; mountent.mnt_type = (char *)(void *)cifs_fstype; @@ -1529,6 +1458,5 @@ mount_exit: SAFE_FREE(parsed_info); SAFE_FREE(options); SAFE_FREE(orgoptions); - SAFE_FREE(resolved_path); return rc; }