Re: Chroot bug

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Kyle Moffett napsal(a):
On Sep 26, 2007, at 06:27:38, David Newall wrote:
Kyle Moffett wrote:
David, please do tell myself and Adrian how "locking down" chroot() the way you want will avoid letting root break out through any of the above ways?

As has been said, there are thousands of ways to break out of a chroot. It's just that one of them should not be that chroot lets you walk out. I can't explain it clearer than that. If you don't see it now you probably never will.

Let me put it this way: You *CANNOT* enforce chroot() the way you want to without a completely unacceptable performance penalty. Let's start with the simplest example of:

fd = open("/", O_DIRECTORY);
chroot("/foo");
fchdir(fd);
chroot(".");

If you had ever actually looked at the Linux VFS, it is completely *impossible* to tell whether "fd" at the time of the chroot is inside or outside of "/foo" without tracking an enormous amount of extra state.
so there *is* solution. It is possible. I solved it. I have patch and it is working. So if you find some way how to break it I woud glad if you tell me it.
Even then, any such determination may not be valid since an FD may be opened to an inode which is hardlinked at multiple locations in the directory tree. It could also be bind-mounted at multiple locations, or it may not even be mounted at all in this namespace (CDROM that was lazy-unmounted). That FD may be later passed over an open UNIX-domain socket from another process. Moreover, arbitrarily closing FDs would break a huge number of programs. Furthermore, since you can't fix the "trivial" case of 'fchdir()', then there's no point in even *attempting* to fix the "cwd is outside of chroot" problem, although that is basically equivalent in difficulty to fixing the "dir-fd is outside of chroot" problem.

As for the nested-chroot() bit, the root user inside of a chroot is always allowed to chroot(). This is necessary for test-suites for various distro installers, chroot once to enter the installer playpen, installer chroots again to configure the test-installed-system. Once you allow a second chroot, you're back at the "can't reliably and efficiently track directory sub-tree members" problem.

So if you think it can and should be fixed, then PROVIDE THE CODE.
Miloslav Semler
diff -Nrp linux-2.6.16.53/fs/namei.c linux-2.6.16.53-new/fs/namei.c
*** linux-2.6.16.53/fs/namei.c	2007-07-25 23:05:45.000000000 +0200
--- linux-2.6.16.53-new/fs/namei.c	2007-09-15 16:13:50.000000000 +0200
*************** static __always_inline void follow_dotdo
*** 728,733 ****
--- 728,772 ----
  	}
  	follow_mount(&nd->mnt, &nd->dentry);
  }
+ long directory_is_out(struct vfsmount *wdmnt, struct dentry *wdentry,
+ 		struct vfsmount *rootmnt, struct dentry *root)
+ {
+ 	struct nameidata oldentry, newentry;
+ 	long ret = 1;
+ 	
+         read_lock(&current->fs->lock);
+ 	oldentry.dentry = dget(wdentry);
+ 	oldentry.mnt = mntget(wdmnt);
+         read_unlock(&current->fs->lock);
+ 	newentry.dentry = oldentry.dentry;
+ 	newentry.mnt = oldentry.mnt;
+ 	
+ 	follow_dotdot(&newentry);
+ 	/* check it */
+ 	if(newentry.dentry == root && 
+ 		newentry.mnt == rootmnt){
+ 		ret = 0;
+ 		goto out;
+ 	}
+ 	
+ 	while(oldentry.mnt != newentry.mnt ||
+ 		oldentry.dentry != newentry.dentry){
+ 		
+ 		memcpy(&oldentry, &newentry, sizeof(struct nameidata));
+ 		follow_dotdot(&newentry);
+ 		
+ 		/* check it */
+ 		if(newentry.dentry == root && 
+ 			newentry.mnt == rootmnt){
+ 			ret = 0;
+ 			goto out;
+ 		}
+ 	}
+ out:
+ 	dput(newentry.dentry);
+ 	mntput(newentry.mnt);
+ 	return ret;
+ }
  
  /*
   *  It's more convoluted than I'd like it to be, but... it's still fairly
diff -Nrp linux-2.6.16.53/fs/open.c linux-2.6.16.53-new/fs/open.c
*** linux-2.6.16.53/fs/open.c	2007-07-25 23:05:45.000000000 +0200
--- linux-2.6.16.53-new/fs/open.c	2007-09-15 16:14:52.000000000 +0200
*************** dput_and_out:
*** 560,565 ****
--- 560,567 ----
  out:
  	return error;
  }
+ long directory_is_out(struct vfsmount *, struct dentry*, 
+ 		struct vfsmount *, struct dentry *);
  
  asmlinkage long sys_fchdir(unsigned int fd)
  {
*************** asmlinkage long sys_fchdir(unsigned int 
*** 581,586 ****
--- 583,591 ----
  	error = -ENOTDIR;
  	if (!S_ISDIR(inode->i_mode))
  		goto out_putf;
+ 	if (directory_is_out(mnt, dentry, current->fs->rootmnt, 
+ 				current->fs->root))
+ 		goto out_putf;
  
  	error = file_permission(file, MAY_EXEC);
  	if (!error)
*************** out:
*** 594,600 ****
  asmlinkage long sys_chroot(const char __user * filename)
  {
  	struct nameidata nd;
! 	int error;
  
  	error = __user_walk(filename, LOOKUP_FOLLOW | LOOKUP_DIRECTORY | LOOKUP_NOALT, &nd);
  	if (error)
--- 599,605 ----
  asmlinkage long sys_chroot(const char __user * filename)
  {
  	struct nameidata nd;
! 	int error, set_wd = 0;
  
  	error = __user_walk(filename, LOOKUP_FOLLOW | LOOKUP_DIRECTORY | LOOKUP_NOALT, &nd);
  	if (error)
*************** asmlinkage long sys_chroot(const char __
*** 607,615 ****
--- 612,631 ----
  	error = -EPERM;
  	if (!capable(CAP_SYS_CHROOT))
  		goto dput_and_out;
+ 	error = -ENOTDIR;
+ 	/*
+ 	if (directory_is_out(nd.mnt, nd.dentry, current->fs->rootmnt,
+ 				current->fs->root))
+ 		goto dput_and_out;
+ 		*/
+ 	set_wd = directory_is_out(current->fs->pwdmnt, current->fs->pwd, 
+ 				nd.mnt, nd.dentry);
  
  	set_fs_root(current->fs, nd.mnt, nd.dentry);
  	set_fs_altroot();
+ 	/* if wd is out, reset it to . */
+ 	if(set_wd)
+ 		set_fs_pwd(current->fs, nd.mnt, nd.dentry);
  	error = 0;
  dput_and_out:
  	path_release(&nd);

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux