[PATCH 0/1][C/D] CVE-2018-6559 - Filename information disclosure in overlayfs

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/1][C/D] CVE-2018-6559 - Filename information disclosure in overlayfs

Tyler Hicks-2
https://people.canonical.com/~ubuntu-security/cve/2018/CVE-2018-6559.html
https://launchpad.net/bugs/1793458/

 The overlayfs implementation in the linux (aka Linux kernel) package in Ubuntu
 did not properly check permissions for read operations on directories in the
 lower filesystem directory, which allows local users to obtain names of files
 in which they would not normally be able to access by performing an overlayfs
 mount inside of a user namespace.

I've tested this change with a QRT regression test that I wrote as well as the
unionmount-testsuite:

https://github.com/amir73il/unionmount-testsuite.git

This issue is related to a portion of CVE-2015-1328 that was reintroduced into
the Ubuntu kernel. This bug comment describes the situation:

https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1793458/comments/4

As mentioned above, I wrote a QRT test for this issue so that we don't
accidentally drop our SAUCE patch in the future.

Tyler


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/1][C/D] UBUNTU: SAUCE: overlayfs: ensure mounter privileges when reading directories

Tyler Hicks-2
From: Andy Whitcroft <[hidden email]>

BugLink: https://launchpad.net/bugs/1793458

When reading directory contents ensure the mounter has permissions for
the operation over the constituent parts (lower and upper). Where we are
in a namespace this ensures that the mounter (root in that namespace)
has permissions over the files and directories, preventing exposure of
protected files and directory contents.

CVE-2018-6559

Signed-off-by: Andy Whitcroft <[hidden email]>
[tyhicks: make use of new upstream check in ovl_permission() for copy-ups]
[tyhicks: make use of creator (mounter) creds hanging off the super block]
Signed-off-by: Tyler Hicks <[hidden email]>
---
 fs/overlayfs/inode.c     |  5 +----
 fs/overlayfs/overlayfs.h |  2 ++
 fs/overlayfs/readdir.c   | 12 ++++++++++++
 fs/overlayfs/util.c      | 13 +++++++++++++
 4 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index ed16a898caeb..988f7bdde3de 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -217,7 +217,6 @@ int ovl_permission(struct inode *inode, int mask)
 {
  struct inode *upperinode = ovl_inode_upper(inode);
  struct inode *realinode = upperinode ?: ovl_inode_lower(inode);
- const struct cred *old_cred;
  int err;
 
  /* Careful in RCU walk mode */
@@ -234,15 +233,13 @@ int ovl_permission(struct inode *inode, int mask)
  if (err)
  return err;
 
- old_cred = ovl_override_creds(inode->i_sb);
  if (!upperinode &&
     !special_file(realinode->i_mode) && mask & MAY_WRITE) {
  mask &= ~(MAY_WRITE | MAY_APPEND);
  /* Make sure mounter can read file for copy up later */
  mask |= MAY_READ;
  }
- err = inode_permission(realinode, mask);
- revert_creds(old_cred);
+ err = ovl_creator_permission(inode->i_sb, realinode, mask);
 
  return err;
 }
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 31aebb429d02..62c3c080b185 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -208,6 +208,8 @@ void ovl_drop_write(struct dentry *dentry);
 struct dentry *ovl_workdir(struct dentry *dentry);
 const struct cred *ovl_override_creds(struct super_block *sb);
 struct super_block *ovl_same_sb(struct super_block *sb);
+int ovl_creator_permission(struct super_block *sb, struct inode *inode,
+   int mode);
 int ovl_can_decode_fh(struct super_block *sb);
 struct dentry *ovl_indexdir(struct super_block *sb);
 bool ovl_index_all(struct super_block *sb);
diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
index cc8303a806b4..d8ec95a4069f 100644
--- a/fs/overlayfs/readdir.c
+++ b/fs/overlayfs/readdir.c
@@ -373,6 +373,12 @@ static int ovl_dir_read_merged(struct dentry *dentry, struct list_head *list,
  next = ovl_path_next(idx, dentry, &realpath);
  rdd.is_upper = ovl_dentry_upper(dentry) == realpath.dentry;
 
+ err = ovl_creator_permission(dentry->d_sb,
+     d_inode(realpath.dentry),
+     MAY_READ);
+ if (err)
+ break;
+
  if (next != -1) {
  err = ovl_dir_read(&realpath, &rdd);
  if (err)
@@ -735,6 +741,12 @@ static int ovl_iterate(struct file *file, struct dir_context *ctx)
  ovl_dir_reset(file);
 
  if (od->is_real) {
+ err = ovl_creator_permission(dentry->d_sb,
+     file_inode(od->realfile),
+     MAY_READ);
+ if (err)
+ return err;
+
  /*
  * If parent is merge, then need to adjust d_ino for '..', if
  * dir is impure then need to adjust d_ino for copied up
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 6f1078028c66..2ca86dbb55b6 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -55,6 +55,19 @@ struct super_block *ovl_same_sb(struct super_block *sb)
  return NULL;
 }
 
+int ovl_creator_permission(struct super_block *sb, struct inode *inode,
+   int mode)
+{
+ const struct cred *old_cred;
+ int err = 0;
+
+ old_cred = ovl_override_creds(sb);
+ err = inode_permission(inode, mode);
+ revert_creds(old_cred);
+
+ return err;
+}
+
 /*
  * Check if underlying fs supports file handles and try to determine encoding
  * type, in order to deduce maximum inode number used by fs.
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK / APPLIED[Unstable]: [PATCH 1/1][C/D] UBUNTU: SAUCE: overlayfs: ensure mounter privileges when reading directories

Seth Forshee
On Fri, Oct 19, 2018 at 04:44:53PM +0000, Tyler Hicks wrote:

> From: Andy Whitcroft <[hidden email]>
>
> BugLink: https://launchpad.net/bugs/1793458
>
> When reading directory contents ensure the mounter has permissions for
> the operation over the constituent parts (lower and upper). Where we are
> in a namespace this ensures that the mounter (root in that namespace)
> has permissions over the files and directories, preventing exposure of
> protected files and directory contents.
>
> CVE-2018-6559
>
> Signed-off-by: Andy Whitcroft <[hidden email]>
> [tyhicks: make use of new upstream check in ovl_permission() for copy-ups]
> [tyhicks: make use of creator (mounter) creds hanging off the super block]
> Signed-off-by: Tyler Hicks <[hidden email]>

Acked-by: Seth Forshee <[hidden email]>

Applied to unstable/master, thanks!

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK/Cmnt: [PATCH 1/1][C/D] UBUNTU: SAUCE: overlayfs: ensure mounter privileges when reading directories

Stefan Bader-2
In reply to this post by Tyler Hicks-2
On 19.10.18 18:44, Tyler Hicks wrote:

> From: Andy Whitcroft <[hidden email]>
>
> BugLink: https://launchpad.net/bugs/1793458
>
> When reading directory contents ensure the mounter has permissions for
> the operation over the constituent parts (lower and upper). Where we are
> in a namespace this ensures that the mounter (root in that namespace)
> has permissions over the files and directories, preventing exposure of
> protected files and directory contents.
>
> CVE-2018-6559
>
> Signed-off-by: Andy Whitcroft <[hidden email]>
> [tyhicks: make use of new upstream check in ovl_permission() for copy-ups]
> [tyhicks: make use of creator (mounter) creds hanging off the super block]
> Signed-off-by: Tyler Hicks <[hidden email]>
Acked-by: Stefan Bader <[hidden email]>
> ---

From the bug report I take this was taken from Xenial and forward ported to
Bionic and Cosmic. I wonder whether we could make this more obvious in the s-o-b
area something like

(fwd-ported from commit <sha1> xenial)

-Stefan

>  fs/overlayfs/inode.c     |  5 +----
>  fs/overlayfs/overlayfs.h |  2 ++
>  fs/overlayfs/readdir.c   | 12 ++++++++++++
>  fs/overlayfs/util.c      | 13 +++++++++++++
>  4 files changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
> index ed16a898caeb..988f7bdde3de 100644
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -217,7 +217,6 @@ int ovl_permission(struct inode *inode, int mask)
>  {
>   struct inode *upperinode = ovl_inode_upper(inode);
>   struct inode *realinode = upperinode ?: ovl_inode_lower(inode);
> - const struct cred *old_cred;
>   int err;
>  
>   /* Careful in RCU walk mode */
> @@ -234,15 +233,13 @@ int ovl_permission(struct inode *inode, int mask)
>   if (err)
>   return err;
>  
> - old_cred = ovl_override_creds(inode->i_sb);
>   if (!upperinode &&
>      !special_file(realinode->i_mode) && mask & MAY_WRITE) {
>   mask &= ~(MAY_WRITE | MAY_APPEND);
>   /* Make sure mounter can read file for copy up later */
>   mask |= MAY_READ;
>   }
> - err = inode_permission(realinode, mask);
> - revert_creds(old_cred);
> + err = ovl_creator_permission(inode->i_sb, realinode, mask);
>  
>   return err;
>  }
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index 31aebb429d02..62c3c080b185 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -208,6 +208,8 @@ void ovl_drop_write(struct dentry *dentry);
>  struct dentry *ovl_workdir(struct dentry *dentry);
>  const struct cred *ovl_override_creds(struct super_block *sb);
>  struct super_block *ovl_same_sb(struct super_block *sb);
> +int ovl_creator_permission(struct super_block *sb, struct inode *inode,
> +   int mode);
>  int ovl_can_decode_fh(struct super_block *sb);
>  struct dentry *ovl_indexdir(struct super_block *sb);
>  bool ovl_index_all(struct super_block *sb);
> diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
> index cc8303a806b4..d8ec95a4069f 100644
> --- a/fs/overlayfs/readdir.c
> +++ b/fs/overlayfs/readdir.c
> @@ -373,6 +373,12 @@ static int ovl_dir_read_merged(struct dentry *dentry, struct list_head *list,
>   next = ovl_path_next(idx, dentry, &realpath);
>   rdd.is_upper = ovl_dentry_upper(dentry) == realpath.dentry;
>  
> + err = ovl_creator_permission(dentry->d_sb,
> +     d_inode(realpath.dentry),
> +     MAY_READ);
> + if (err)
> + break;
> +
>   if (next != -1) {
>   err = ovl_dir_read(&realpath, &rdd);
>   if (err)
> @@ -735,6 +741,12 @@ static int ovl_iterate(struct file *file, struct dir_context *ctx)
>   ovl_dir_reset(file);
>  
>   if (od->is_real) {
> + err = ovl_creator_permission(dentry->d_sb,
> +     file_inode(od->realfile),
> +     MAY_READ);
> + if (err)
> + return err;
> +
>   /*
>   * If parent is merge, then need to adjust d_ino for '..', if
>   * dir is impure then need to adjust d_ino for copied up
> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
> index 6f1078028c66..2ca86dbb55b6 100644
> --- a/fs/overlayfs/util.c
> +++ b/fs/overlayfs/util.c
> @@ -55,6 +55,19 @@ struct super_block *ovl_same_sb(struct super_block *sb)
>   return NULL;
>  }
>  
> +int ovl_creator_permission(struct super_block *sb, struct inode *inode,
> +   int mode)
> +{
> + const struct cred *old_cred;
> + int err = 0;
> +
> + old_cred = ovl_override_creds(sb);
> + err = inode_permission(inode, mode);
> + revert_creds(old_cred);
> +
> + return err;
> +}
> +
>  /*
>   * Check if underlying fs supports file handles and try to determine encoding
>   * type, in order to deduce maximum inode number used by fs.
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

APPLIED: [PATCH 0/1][C/D] CVE-2018-6559 - Filename information disclosure in overlayfs

Stefan Bader-2
In reply to this post by Tyler Hicks-2
On 19.10.18 18:44, Tyler Hicks wrote:

> https://people.canonical.com/~ubuntu-security/cve/2018/CVE-2018-6559.html
> https://launchpad.net/bugs/1793458/
>
>  The overlayfs implementation in the linux (aka Linux kernel) package in Ubuntu
>  did not properly check permissions for read operations on directories in the
>  lower filesystem directory, which allows local users to obtain names of files
>  in which they would not normally be able to access by performing an overlayfs
>  mount inside of a user namespace.
>
> I've tested this change with a QRT regression test that I wrote as well as the
> unionmount-testsuite:
>
> https://github.com/amir73il/unionmount-testsuite.git
>
> This issue is related to a portion of CVE-2015-1328 that was reintroduced into
> the Ubuntu kernel. This bug comment describes the situation:
>
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1793458/comments/4
>
> As mentioned above, I wrote a QRT test for this issue so that we don't
> accidentally drop our SAUCE patch in the future.
>
> Tyler
>
>
Applied to cosmic/master-next. Thanks.

-Stefan


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment