diff mbox

Making shares unaccessible at root level mountable (aka solving bsc#8950 ...again)

Message ID 20160527194346.08416d79@aaptelpc (mailing list archive)
State New, archived
Headers show

Commit Message

Aurélien Aptel May 27, 2016, 5:43 p.m. UTC
Hi all,

I've come up with a new solution for the problem (attached). Between
the long, old and irrelevant comments on the bug report and the various
mailing-list threads it was getting hard to understand. So I've written
a summary which explains the problem in details and the 2 proposed
solutions.

http://diobla.info/stuff/bugs/bsc799133/

The new solution is basically switching to the old prefixpath system
when the normal method of querying intermediary path fails. It's much
more simpler.

*Any* feedback would be nice. I haven't noticed any problems so far but
I haven't run the xfs test suite (I still have to figure out how
to make it play nice with cifs...). I'm sending to samba-tech too in
hopes of reviews/comments.

Comments

Aurélien Aptel June 9, 2016, 4:50 p.m. UTC | #1
Small update: I've written a powershell script to reproduce the problem
(attached). If you're wondering I'm not using samba see my notes
about it [1].

On the window server:
- Edit $Dir (script will create parent dirs)
- Edit $LimitedUser/$AdminUser to an existing one
- Run the script as admin

On the linux client:
- Mount the share sub dir with the limited user credentials:
  mount //lutze/bug8950/sub/dir' /mnt \
        -o 'domain=LURCH,ip=10.160.5.42,username=bill,password=*****,rw'

My second solution fails for the case when the dir *containing* the
shared dir restricts the limited user. See "HARD MODE" at the end
of the script.

1: http://diobla.info/stuff/bugs/bsc799133/#sec-4
Marcus Hoffmann June 9, 2016, 7:27 p.m. UTC | #2
Hey Aurélien,
with your script I can reproduce the bug locally now.

I can mount the share (which is on a Windows 8.1 vm) with a Windows 7 PC
with the restricted user account. (Even in hard mode.)
I can mount the share from Linux-cifs using the admin user but not the
restricted user.

(I noticed though that no user has access to the file in the shared dir.
But this doesn't really matter for the test.)

Marcus

On 06/09/2016 06:50 PM, Aurélien Aptel wrote:
> Small update: I've written a powershell script to reproduce the problem
> (attached). If you're wondering I'm not using samba see my notes
> about it [1].
>
> On the window server:
> - Edit $Dir (script will create parent dirs)
> - Edit $LimitedUser/$AdminUser to an existing one
> - Run the script as admin
>
> On the linux client:
> - Mount the share sub dir with the limited user credentials:
>   mount //lutze/bug8950/sub/dir' /mnt \
>         -o 'domain=LURCH,ip=10.160.5.42,username=bill,password=*****,rw'
>
> My second solution fails for the case when the dir *containing* the
> shared dir restricts the limited user. See "HARD MODE" at the end
> of the script.
>
> 1: http://diobla.info/stuff/bugs/bsc799133/#sec-4
>

--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

From 3e9af75bea055bf88a4700fced50a7ba38b39b6f Mon Sep 17 00:00:00 2001
From: Aurelien Aptel <aaptel@suse.com>
Date: Wed, 25 May 2016 19:59:09 +0200
Subject: [PATCH] fs/cifs: make share unaccessible at root level mountable

if, when mounting //HOST/share/sub/dir/foo we can query /sub/dir/foo but
not any of the path components above:

- store the /sub/dir/foo prefix in the cifs super_block info
- in the superblock, set root dentry to the subpath dentry (instead of
  the share root)
- set a flag in the superblock to remember it
- use prefixpath when building path from a dentry

fixes bso#8950

Signed-off-by: Aurelien Aptel <aaptel@suse.com>
---
 fs/cifs/cifs_fs_sb.h |  2 ++
 fs/cifs/cifsfs.c     | 19 +++++++++++++++++++
 fs/cifs/connect.c    |  3 +++
 fs/cifs/dir.c        | 19 +++++++++++++++++--
 4 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/fs/cifs/cifs_fs_sb.h b/fs/cifs/cifs_fs_sb.h
index 3182273..02b9ac3 100644
--- a/fs/cifs/cifs_fs_sb.h
+++ b/fs/cifs/cifs_fs_sb.h
@@ -46,6 +46,7 @@ 
 #define CIFS_MOUNT_CIFS_BACKUPUID 0x200000 /* backup intent bit for a user */
 #define CIFS_MOUNT_CIFS_BACKUPGID 0x400000 /* backup intent bit for a group */
 #define CIFS_MOUNT_MAP_SFM_CHR	0x800000 /* SFM/MAC mapping for illegal chars */
+#define CIFS_MOUNT_USE_PREFIX_PATH 0x1000000 /* make subpath with unaccessible root mountable */
 
 struct cifs_sb_info {
 	struct rb_root tlink_tree;
@@ -67,5 +68,6 @@  struct cifs_sb_info {
 	struct backing_dev_info bdi;
 	struct delayed_work prune_tlinks;
 	struct rcu_head rcu;
+	char *prepath;
 };
 #endif				/* _CIFS_FS_SB_H */
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 5d8b7ed..d1fc593 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -649,6 +649,17 @@  cifs_get_root(struct smb_vol *vol, struct super_block *sb)
 		dentry = child;
 	} while (!IS_ERR(dentry));
 	kfree(full_path);
+
+	if (IS_ERR(dentry) /* && path accessible */) {
+		/* we know the path is accessible (it is tested
+		 * earlier in cifs_do_mount()) so there must be a perm
+		 * problem */
+		cifs_dbg(VFS, "cannot query directories between root and final path, "
+			 "enabling CIFS_MOUNT_USE_PREFIX_PATH\n");
+		cifs_sb->mnt_cifs_flags |= CIFS_MOUNT_USE_PREFIX_PATH;
+		dentry = dget(sb->s_root);
+	}
+
 	return dentry;
 }
 
@@ -688,6 +699,14 @@  cifs_do_mount(struct file_system_type *fs_type,
 		goto out_cifs_sb;
 	}
 
+	if (volume_info->prepath) {
+		cifs_sb->prepath = kstrdup(volume_info->prepath, GFP_KERNEL);
+		if (cifs_sb->prepath == NULL) {
+			root = ERR_PTR(-ENOMEM);
+			goto out_cifs_sb;
+		}
+	}
+
 	cifs_setup_cifs_sb(volume_info, cifs_sb);
 
 	rc = cifs_mount(cifs_sb, volume_info);
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 66736f5..90e57ee 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -3887,6 +3887,9 @@  cifs_umount(struct cifs_sb_info *cifs_sb)
 
 	bdi_destroy(&cifs_sb->bdi);
 	kfree(cifs_sb->mountdata);
+	if (cifs_sb->prepath) {
+		kfree(cifs_sb->prepath);
+	}
 	call_rcu(&cifs_sb->rcu, delayed_free);
 }
 
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index c3eb998..5374253 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -84,6 +84,7 @@  build_path_from_dentry(struct dentry *direntry)
 	struct dentry *temp;
 	int namelen;
 	int dfsplen;
+	int pplen = 0;
 	char *full_path;
 	char dirsep;
 	struct cifs_sb_info *cifs_sb = CIFS_SB(direntry->d_sb);
@@ -95,8 +96,12 @@  build_path_from_dentry(struct dentry *direntry)
 		dfsplen = strnlen(tcon->treeName, MAX_TREE_SIZE + 1);
 	else
 		dfsplen = 0;
+
+	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_USE_PREFIX_PATH)
+		pplen = cifs_sb->prepath ? strlen(cifs_sb->prepath) + 1 : 0;
+
 cifs_bp_rename_retry:
-	namelen = dfsplen;
+	namelen = dfsplen + pplen;
 	seq = read_seqbegin(&rename_lock);
 	rcu_read_lock();
 	for (temp = direntry; !IS_ROOT(temp);) {
@@ -137,7 +142,7 @@  cifs_bp_rename_retry:
 		}
 	}
 	rcu_read_unlock();
-	if (namelen != dfsplen || read_seqretry(&rename_lock, seq)) {
+	if (namelen != dfsplen + pplen || read_seqretry(&rename_lock, seq)) {
 		cifs_dbg(FYI, "did not end path lookup where expected. namelen=%ddfsplen=%d\n",
 			 namelen, dfsplen);
 		/* presumably this is only possible if racing with a rename
@@ -153,6 +158,16 @@  cifs_bp_rename_retry:
 	   those safely to '/' if any are found in the middle of the prepath */
 	/* BB test paths to Windows with '/' in the midst of prepath */
 
+	if (pplen) {
+		int i;
+		cifs_dbg(FYI, "using cifs_sb prepath <%s>\n", cifs_sb->prepath);
+		memcpy(full_path+dfsplen+1, cifs_sb->prepath, pplen-1);
+		full_path[dfsplen] = '\\';
+		for (i = 0; i < pplen-1; i++)
+			if (full_path[dfsplen+1+i] == '/')
+				full_path[dfsplen+1+i] = CIFS_DIR_SEP(cifs_sb);
+	}
+
 	if (dfsplen) {
 		strncpy(full_path, tcon->treeName, dfsplen);
 		if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_POSIX_PATHS) {
-- 
2.1.4