[RFC PATCH 0/9][V3] Overlayfs SELinux Support

[RFC PATCH 0/9][V3] Overlayfs SELinux Support

[Vivek Goyal]

Hi All,

Please find attached the V3 of patches. Changes since V2 are as follows.

- Fixed the build issue with CONFIG_SECURITY=n.

- Dan Walsh was writing more tests for selinux-testsuite and noted couple
  of issues. I have fixed those issues and added two more patches in series.

  1. We are resetting MAY_WRITE check for lower inode assuming file will
     be coiped up. But this is not true for special_file() as these files
     are not copied up. So checks should not be reset in case of special
     file.

  2. We are resetting MAY_WRITE check for lower inode assuming file will
     be copied up. But this also should mean that mounter has permission
     to MAY_READ lower file for copy up to succeed. So add MAY_READ
     check while resetting MAY_WRITE.

Original description of patches follows.

Following are RFC patches to support SELinux with overlayfs. I started
with David Howells's latest posting on this topic and started modifying
patches. These patches apply on top of overlayfs-next branch of miklos
vfs git tree.

git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git overlayfs-next

These patches can be pulled from my branch too.

https://github.com/rhvgoyal/linux/commits/overlayfs-selinux-mounter-next

Thanks to Dan Walsh, Stephen Smalley and Miklos Szeredi for numerous
conversation and ideas in helping figuring out what one reasonable
implementation might look like.

Dan Walsh has been writing tests for selinux overlayfs in selinux-testsuite.
These patches pass those tests now

https://github.com/rhatdan/selinux-testsuite/commits/master

Posting these patches for review and comments.

These patches introduce 3 new security hooks.

- security_inode_copy_up(), is called when a file is copied up. This hook
  prepares a new set of cred which is used for copy up operation. And
  new set of creds are prepared so that ->create_sid can be set appropriately
  and newly created file is labeled properly. 

  When a file is copied up, label of lower file is retained except for the
  case of context= mount where new file gets the label from context= option.

- security_inode_copy_up_xattr(), is called when xattrs of a file are
  being copied up. Before this we already called security_inode_copy_up()
  and created new file and copied up data. That means file already got
  labeled properly and there is no need to take SELINUX xattr of lower
  file and overwrite the upper file xattr. So this hook is used to avoid
  copying up of SELINUX xattr.

- dentry_create_files_as(), is called when a new file is about to be created.
  This hook determines what the label of the file should be if task had
  created that file in upper/ and sets create_sid accordingly in the passed
  in creds.

  Normal transition rules don't work for the case of context mounts as
  underlying file system is not aware of context option which only overlay
  layer is aware of. For non-context mounts, creation can happen in work/
  dir first and then file might be renamed into upper/, and it might get
  label based on work/ dir. So this hooks helps avoiding all these issues.

  When a new file is created in upper/, it gets its label based on transition
  rules. For the case of context mount, it gets the label from context=
  option.

Any feedback is welcome.


Vivek Goyal (9):
  security, overlayfs: provide copy up security hook for unioned files
  selinux: Implementation for inode_copy_up() hook
  security,overlayfs: Provide security hook for copy up of xattrs for
    overlay file
  selinux: Implementation for inode_copy_up_xattr() hook
  selinux: Pass security pointer to determine_inode_label()
  security, overlayfs: Provide hook to correctly label newly created
    files
  selinux: Implement dentry_create_files_as() hook
  overlayfs: Dilute permission checks on lower only if not special file
  overlayfs: Append MAY_READ when diluting write checks

 fs/overlayfs/copy_up.c    | 25 +++++++++++++++
 fs/overlayfs/dir.c        | 10 ++++++
 fs/overlayfs/inode.c      |  9 ++++--
 include/linux/lsm_hooks.h | 36 ++++++++++++++++++++++
 include/linux/security.h  | 24 +++++++++++++++
 security/security.c       | 27 ++++++++++++++++
 security/selinux/hooks.c  | 78 +++++++++++++++++++++++++++++++++++++++++------
 7 files changed, 198 insertions(+), 11 deletions(-)

-- 
2.7.4

[PATCH 1/9] security, overlayfs: provide copy up security hook for unioned files

[Vivek Goyal]

Provide a security hook to label new file correctly when a file is copied
up from lower layer to upper layer of a overlay/union mount.

This hook can prepare a new set of creds which are suitable for new file
creation during copy up. Caller will use new creds to create file and then
revert back to old creds and release new creds.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 fs/overlayfs/copy_up.c    | 18 ++++++++++++++++++
 include/linux/lsm_hooks.h | 11 +++++++++++
 include/linux/security.h  |  6 ++++++
 security/security.c       |  8 ++++++++
 4 files changed, 43 insertions(+)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 80aa6f1..8ebea18 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -246,6 +246,8 @@ static int ovl_copy_up_locked(struct dentry *workdir, struct dentry *upperdir,
 	struct dentry *upper = NULL;
 	umode_t mode = stat->mode;
 	int err;
+	const struct cred *old_creds = NULL;
+	struct cred *new_creds = NULL;
 
 	newdentry = ovl_lookup_temp(workdir, dentry);
 	err = PTR_ERR(newdentry);
@@ -258,10 +260,26 @@ static int ovl_copy_up_locked(struct dentry *workdir, struct dentry *upperdir,
 	if (IS_ERR(upper))
 		goto out1;
 
+	err = security_inode_copy_up(dentry, &new_creds);
+	if (err < 0) {
+		if (new_creds)
+			put_cred(new_creds);
+		goto out2;
+	}
+
+	if (new_creds)
+		old_creds = override_creds(new_creds);
+
 	/* Can't properly set mode on creation because of the umask */
 	stat->mode &= S_IFMT;
 	err = ovl_create_real(wdir, newdentry, stat, link, NULL, true);
 	stat->mode = mode;
+
+	if (new_creds) {
+		revert_creds(old_creds);
+		put_cred(new_creds);
+	}
+
 	if (err)
 		goto out2;
 
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 7ae3976..c1f95be 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -401,6 +401,15 @@
  *	@inode contains a pointer to the inode.
  *	@secid contains a pointer to the location where result will be saved.
  *	In case of failure, @secid will be set to zero.
+ * @inode_copy_up:
+ *	A file is about to be copied up from lower layer to upper layer of
+ *	overlay filesystem. Security module can prepare a set of new creds
+ *	and modify as need be and return new creds. Caller will switch to
+ *	new creds temporarily to create new file and release newly allocated
+ *	creds.
+ *	@src indicates the union dentry of file that is being copied up.
+ *	@new pointer to pointer to return newly allocated creds.
+ *	Returns 0 on success or a negative error code on error.
  *
  * Security hooks for file operations
  *
@@ -1425,6 +1434,7 @@ union security_list_options {
 	int (*inode_listsecurity)(struct inode *inode, char *buffer,
 					size_t buffer_size);
 	void (*inode_getsecid)(struct inode *inode, u32 *secid);
+	int (*inode_copy_up) (struct dentry *src, struct cred **new);
 
 	int (*file_permission)(struct file *file, int mask);
 	int (*file_alloc_security)(struct file *file);
@@ -1696,6 +1706,7 @@ struct security_hook_heads {
 	struct list_head inode_setsecurity;
 	struct list_head inode_listsecurity;
 	struct list_head inode_getsecid;
+	struct list_head inode_copy_up;
 	struct list_head file_permission;
 	struct list_head file_alloc_security;
 	struct list_head file_free_security;
diff --git a/include/linux/security.h b/include/linux/security.h
index 14df373..c976d79 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -282,6 +282,7 @@ int security_inode_getsecurity(struct inode *inode, const char *name, void **buf
 int security_inode_setsecurity(struct inode *inode, const char *name, const void *value, size_t size, int flags);
 int security_inode_listsecurity(struct inode *inode, char *buffer, size_t buffer_size);
 void security_inode_getsecid(struct inode *inode, u32 *secid);
+int security_inode_copy_up(struct dentry *src, struct cred **new);
 int security_file_permission(struct file *file, int mask);
 int security_file_alloc(struct file *file);
 void security_file_free(struct file *file);
@@ -758,6 +759,11 @@ static inline void security_inode_getsecid(struct inode *inode, u32 *secid)
 	*secid = 0;
 }
 
+static inline int security_inode_copy_up(struct dentry *src, struct cred **new)
+{
+	return 0;
+}
+
 static inline int security_file_permission(struct file *file, int mask)
 {
 	return 0;
diff --git a/security/security.c b/security/security.c
index 7095693..3d142aa 100644
--- a/security/security.c
+++ b/security/security.c
@@ -727,6 +727,12 @@ void security_inode_getsecid(struct inode *inode, u32 *secid)
 	call_void_hook(inode_getsecid, inode, secid);
 }
 
+int security_inode_copy_up(struct dentry *src, struct cred **new)
+{
+	return call_int_hook(inode_copy_up, 0, src, new);
+}
+EXPORT_SYMBOL(security_inode_copy_up);
+
 int security_file_permission(struct file *file, int mask)
 {
 	int ret;
@@ -1663,6 +1669,8 @@ struct security_hook_heads security_hook_heads = {
 		LIST_HEAD_INIT(security_hook_heads.inode_listsecurity),
 	.inode_getsecid =
 		LIST_HEAD_INIT(security_hook_heads.inode_getsecid),
+	.inode_copy_up =
+		LIST_HEAD_INIT(security_hook_heads.inode_copy_up),
 	.file_permission =
 		LIST_HEAD_INIT(security_hook_heads.file_permission),
 	.file_alloc_security =
-- 
2.7.4

[PATCH 2/9] selinux: Implementation for inode_copy_up() hook

[Vivek Goyal]

A file is being copied up for overlay file system. Prepare a new set of
creds and set create_sid appropriately so that new file is created with
appropriate label.

Overlay inode has right label for both context and non-context mount
cases. In case of non-context mount, overlay inode will have the label
of lower file and in case of context mount, overlay inode will have
the label from context= mount option.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 security/selinux/hooks.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index a86d537..c82ee54 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3270,6 +3270,26 @@ static void selinux_inode_getsecid(struct inode *inode, u32 *secid)
 	*secid = isec->sid;
 }
 
+static int selinux_inode_copy_up(struct dentry *src, struct cred **new)
+{
+	u32 sid;
+	struct task_security_struct *tsec;
+	struct cred *new_creds = *new;
+
+	if (new_creds == NULL) {
+		new_creds = prepare_creds();
+		if (!new_creds)
+			return -ENOMEM;
+	}
+
+	tsec = new_creds->security;
+	/* Get label from overlay inode and set it in create_sid */
+	selinux_inode_getsecid(d_inode(src), &sid);
+	tsec->create_sid = sid;
+	*new = new_creds;
+	return 0;
+}
+
 /* file security operations */
 
 static int selinux_revalidate_file_permission(struct file *file, int mask)
@@ -6056,6 +6076,7 @@ static struct security_hook_list selinux_hooks[] = {
 	LSM_HOOK_INIT(inode_setsecurity, selinux_inode_setsecurity),
 	LSM_HOOK_INIT(inode_listsecurity, selinux_inode_listsecurity),
 	LSM_HOOK_INIT(inode_getsecid, selinux_inode_getsecid),
+	LSM_HOOK_INIT(inode_copy_up, selinux_inode_copy_up),
 
 	LSM_HOOK_INIT(file_permission, selinux_file_permission),
 	LSM_HOOK_INIT(file_alloc_security, selinux_file_alloc_security),
-- 
2.7.4

[PATCH 3/9] security,overlayfs: Provide security hook for copy up of xattrs for overlay file

[Vivek Goyal]

Provide a security hook which is called when xattrs of a file are being
copied up. This hook is called once for each xattr and LSM can return
0 if the security module wants the xattr to be copied up, 1 if the
security module wants the xattr to be discarded on the copy, -EOPNOTSUPP
if the security module does not handle/manage the xattr, or a -errno
upon an error.

Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 fs/overlayfs/copy_up.c    |  7 +++++++
 include/linux/lsm_hooks.h | 10 ++++++++++
 include/linux/security.h  |  6 ++++++
 security/security.c       |  8 ++++++++
 4 files changed, 31 insertions(+)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 8ebea18..68cefb2 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -103,6 +103,13 @@ retry:
 			goto retry;
 		}
 
+		error = security_inode_copy_up_xattr(name);
+		if (error < 0 && error != -EOPNOTSUPP)
+			break;
+		if (error == 1) {
+			error = 0;
+			continue; /* Discard */
+		}
 		error = vfs_setxattr(new, name, value, size, 0);
 		if (error)
 			break;
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index c1f95be..84caead 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -410,6 +410,14 @@
  *	@src indicates the union dentry of file that is being copied up.
  *	@new pointer to pointer to return newly allocated creds.
  *	Returns 0 on success or a negative error code on error.
+ * @inode_copy_up_xattr:
+ *	Filter the xattrs being copied up when a unioned file is copied
+ *	up from a lower layer to the union/overlay layer.
+ *	@name indicates the name of the xattr.
+ *	Returns 0 to accept the xattr, 1 to discard the xattr, -EOPNOTSUPP if
+ *	security module does not know about attribute or a negative error code
+ *	to abort the copy up. Note that the caller is responsible for reading
+ *	and writing the xattrs as this hook is merely a filter.
  *
  * Security hooks for file operations
  *
@@ -1435,6 +1443,7 @@ union security_list_options {
 					size_t buffer_size);
 	void (*inode_getsecid)(struct inode *inode, u32 *secid);
 	int (*inode_copy_up) (struct dentry *src, struct cred **new);
+	int (*inode_copy_up_xattr) (const char *name);
 
 	int (*file_permission)(struct file *file, int mask);
 	int (*file_alloc_security)(struct file *file);
@@ -1707,6 +1716,7 @@ struct security_hook_heads {
 	struct list_head inode_listsecurity;
 	struct list_head inode_getsecid;
 	struct list_head inode_copy_up;
+	struct list_head inode_copy_up_xattr;
 	struct list_head file_permission;
 	struct list_head file_alloc_security;
 	struct list_head file_free_security;
diff --git a/include/linux/security.h b/include/linux/security.h
index c976d79..4a3b8bc 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -283,6 +283,7 @@ int security_inode_setsecurity(struct inode *inode, const char *name, const void
 int security_inode_listsecurity(struct inode *inode, char *buffer, size_t buffer_size);
 void security_inode_getsecid(struct inode *inode, u32 *secid);
 int security_inode_copy_up(struct dentry *src, struct cred **new);
+int security_inode_copy_up_xattr(const char *name);
 int security_file_permission(struct file *file, int mask);
 int security_file_alloc(struct file *file);
 void security_file_free(struct file *file);
@@ -764,6 +765,11 @@ static inline int security_inode_copy_up(struct dentry *src, struct cred **new)
 	return 0;
 }
 
+static inline int security_inode_copy_up_xattr(const char *name)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline int security_file_permission(struct file *file, int mask)
 {
 	return 0;
diff --git a/security/security.c b/security/security.c
index 3d142aa..3321e31 100644
--- a/security/security.c
+++ b/security/security.c
@@ -733,6 +733,12 @@ int security_inode_copy_up(struct dentry *src, struct cred **new)
 }
 EXPORT_SYMBOL(security_inode_copy_up);
 
+int security_inode_copy_up_xattr(const char *name)
+{
+	return call_int_hook(inode_copy_up_xattr, -EOPNOTSUPP, name);
+}
+EXPORT_SYMBOL(security_inode_copy_up_xattr);
+
 int security_file_permission(struct file *file, int mask)
 {
 	int ret;
@@ -1671,6 +1677,8 @@ struct security_hook_heads security_hook_heads = {
 		LIST_HEAD_INIT(security_hook_heads.inode_getsecid),
 	.inode_copy_up =
 		LIST_HEAD_INIT(security_hook_heads.inode_copy_up),
+	.inode_copy_up_xattr =
+		LIST_HEAD_INIT(security_hook_heads.inode_copy_up_xattr),
 	.file_permission =
 		LIST_HEAD_INIT(security_hook_heads.file_permission),
 	.file_alloc_security =
-- 
2.7.4

[PATCH 4/9] selinux: Implementation for inode_copy_up_xattr() hook

[Vivek Goyal]

When a file is copied up in overlay, we have already created file on upper/
with right label and there is no need to copy up selinux label/xattr from
lower file to upper file. In fact in case of context mount, we don't want
to copy up label as newly created file got its label from context= option.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 security/selinux/hooks.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index c82ee54..4fda548 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3290,6 +3290,21 @@ static int selinux_inode_copy_up(struct dentry *src, struct cred **new)
 	return 0;
 }
 
+static int selinux_inode_copy_up_xattr(const char *name)
+{
+	/* The copy_up hook above sets the initial context on an inode, but we
+	 * don't then want to overwrite it by blindly copying all the lower
+	 * xattrs up.  Instead, we have to filter out SELinux-related xattrs.
+	 */
+	if (strcmp(name, XATTR_NAME_SELINUX) == 0)
+		return 1; /* Discard */
+	/*
+	 * Any other attribute apart from SELINUX is not claimed, supported
+	 * by selinux.
+	 */
+	return -EOPNOTSUPP;
+}
+
 /* file security operations */
 
 static int selinux_revalidate_file_permission(struct file *file, int mask)
@@ -6077,6 +6092,7 @@ static struct security_hook_list selinux_hooks[] = {
 	LSM_HOOK_INIT(inode_listsecurity, selinux_inode_listsecurity),
 	LSM_HOOK_INIT(inode_getsecid, selinux_inode_getsecid),
 	LSM_HOOK_INIT(inode_copy_up, selinux_inode_copy_up),
+	LSM_HOOK_INIT(inode_copy_up_xattr, selinux_inode_copy_up_xattr),
 
 	LSM_HOOK_INIT(file_permission, selinux_file_permission),
 	LSM_HOOK_INIT(file_alloc_security, selinux_file_alloc_security),
-- 
2.7.4

[PATCH 5/9] selinux: Pass security pointer to determine_inode_label()

[Vivek Goyal]

Right now selinux_determine_inode_label() works on security pointer of
current task. Soon I need this to work on a security pointer retrieved
from a set of creds. So start passing in a pointer and caller can decide
where to fetch security pointer from.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 security/selinux/hooks.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 4fda548..ae11fd9 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1785,13 +1785,13 @@ out:
 /*
  * Determine the label for an inode that might be unioned.
  */
-static int selinux_determine_inode_label(struct inode *dir,
-					 const struct qstr *name,
-					 u16 tclass,
-					 u32 *_new_isid)
+static int
+selinux_determine_inode_label(const struct task_security_struct *tsec,
+				 struct inode *dir,
+				 const struct qstr *name, u16 tclass,
+				 u32 *_new_isid)
 {
 	const struct superblock_security_struct *sbsec = dir->i_sb->s_security;
-	const struct task_security_struct *tsec = current_security();
 
 	if ((sbsec->flags & SE_SBINITIALIZED) &&
 	    (sbsec->behavior == SECURITY_FS_USE_MNTPOINT)) {
@@ -1834,8 +1834,8 @@ static int may_create(struct inode *dir,
 	if (rc)
 		return rc;
 
-	rc = selinux_determine_inode_label(dir, &dentry->d_name, tclass,
-					   &newsid);
+	rc = selinux_determine_inode_label(current_security(), dir,
+					   &dentry->d_name, tclass, &newsid);
 	if (rc)
 		return rc;
 
@@ -2815,7 +2815,8 @@ static int selinux_dentry_init_security(struct dentry *dentry, int mode,
 	u32 newsid;
 	int rc;
 
-	rc = selinux_determine_inode_label(d_inode(dentry->d_parent), name,
+	rc = selinux_determine_inode_label(current_security(),
+					   d_inode(dentry->d_parent), name,
 					   inode_mode_to_security_class(mode),
 					   &newsid);
 	if (rc)
@@ -2840,7 +2841,7 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
 	sid = tsec->sid;
 	newsid = tsec->create_sid;
 
-	rc = selinux_determine_inode_label(
+	rc = selinux_determine_inode_label(current_security(),
 		dir, qstr,
 		inode_mode_to_security_class(inode->i_mode),
 		&newsid);
-- 
2.7.4

[PATCH 6/9] security, overlayfs: Provide hook to correctly label newly created files

[Vivek Goyal]

During a new file creation we need to make sure new file is created with the
right label. New file is created in upper/ so effectively file should get
label as if task had created file in upper/.

We switched to mounter's creds for actual file creation. Also if there is a
whiteout present, then file will be created in work/ dir first and then
renamed in upper. In none of the cases file will be labeled as we want it to
be.

This patch introduces a new hook dentry_create_files_as(), which determines
the label/context dentry will get if it had been created by task in upper
and modify passed set of creds appropriately. Caller makes use of these new
creds for file creation.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 fs/overlayfs/dir.c        | 10 ++++++++++
 include/linux/lsm_hooks.h | 15 +++++++++++++++
 include/linux/security.h  | 12 ++++++++++++
 security/security.c       | 11 +++++++++++
 4 files changed, 48 insertions(+)

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 4cdeb74..f94872f 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -433,6 +433,15 @@ static int ovl_create_or_link(struct dentry *dentry, int mode, dev_t rdev,
 	if (override_cred) {
 		override_cred->fsuid = inode->i_uid;
 		override_cred->fsgid = inode->i_gid;
+		if (!hardlink) {
+			err = security_dentry_create_files_as(dentry,
+					mode, &dentry->d_name, old_cred,
+					override_cred);
+			if (err) {
+				put_cred(override_cred);
+				goto out_revert_creds;
+			}
+		}
 		put_cred(override_creds(override_cred));
 		put_cred(override_cred);
 
@@ -443,6 +452,7 @@ static int ovl_create_or_link(struct dentry *dentry, int mode, dev_t rdev,
 			err = ovl_create_over_whiteout(dentry, inode, &stat,
 							link, hardlink);
 	}
+out_revert_creds:
 	revert_creds(old_cred);
 	if (!err) {
 		struct inode *realinode = d_inode(ovl_dentry_upper(dentry));
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 84caead..95745fe 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -151,6 +151,16 @@
  *	@name name of the last path component used to create file
  *	@ctx pointer to place the pointer to the resulting context in.
  *	@ctxlen point to place the length of the resulting context.
+ * @dentry_create_files_as:
+ *	Compute a context for a dentry as the inode is not yet available
+ *	and set that context in passed in creds so that new files are
+ *	created using that context. Context is calculated using the
+ *	passed in creds and not the creds of the caller.
+ *	@dentry dentry to use in calculating the context.
+ *	@mode mode used to determine resource type.
+ *	@name name of the last path component used to create file
+ * 	@old creds which should be used for context calculation
+ * 	@new creds to modify
  *
  *
  * Security hooks for inode operations.
@@ -1375,6 +1385,10 @@ union security_list_options {
 	int (*dentry_init_security)(struct dentry *dentry, int mode,
 					struct qstr *name, void **ctx,
 					u32 *ctxlen);
+	int (*dentry_create_files_as)(struct dentry *dentry, int mode,
+					struct qstr *name,
+					const struct cred *old,
+					struct cred *new);
 
 
 #ifdef CONFIG_SECURITY_PATH
@@ -1675,6 +1689,7 @@ struct security_hook_heads {
 	struct list_head sb_clone_mnt_opts;
 	struct list_head sb_parse_opts_str;
 	struct list_head dentry_init_security;
+	struct list_head dentry_create_files_as;
 #ifdef CONFIG_SECURITY_PATH
 	struct list_head path_unlink;
 	struct list_head path_mkdir;
diff --git a/include/linux/security.h b/include/linux/security.h
index 4a3b8bc..1eb03dc 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -242,6 +242,10 @@ int security_sb_parse_opts_str(char *options, struct security_mnt_opts *opts);
 int security_dentry_init_security(struct dentry *dentry, int mode,
 					struct qstr *name, void **ctx,
 					u32 *ctxlen);
+int security_dentry_create_files_as(struct dentry *dentry, int mode,
+					struct qstr *name,
+					const struct cred *old,
+					struct cred *new);
 
 int security_inode_alloc(struct inode *inode);
 void security_inode_free(struct inode *inode);
@@ -600,6 +604,14 @@ static inline int security_dentry_init_security(struct dentry *dentry,
 	return -EOPNOTSUPP;
 }
 
+static inline int security_dentry_create_files_as(struct dentry *dentry,
+						  int mode, struct qstr *name,
+						  const struct cred *old,
+						  struct cred *new)
+{
+	return 0;
+}
+
 
 static inline int security_inode_init_security(struct inode *inode,
 						struct inode *dir,
diff --git a/security/security.c b/security/security.c
index 3321e31..38747d1 100644
--- a/security/security.c
+++ b/security/security.c
@@ -364,6 +364,15 @@ int security_dentry_init_security(struct dentry *dentry, int mode,
 }
 EXPORT_SYMBOL(security_dentry_init_security);
 
+int security_dentry_create_files_as(struct dentry *dentry, int mode,
+					struct qstr *name,
+					const struct cred *old, struct cred *new)
+{
+	return call_int_hook(dentry_create_files_as, 0, dentry, mode,
+				name, old, new);
+}
+EXPORT_SYMBOL(security_dentry_create_files_as);
+
 int security_inode_init_security(struct inode *inode, struct inode *dir,
 				 const struct qstr *qstr,
 				 const initxattrs initxattrs, void *fs_data)
@@ -1614,6 +1623,8 @@ struct security_hook_heads security_hook_heads = {
 		LIST_HEAD_INIT(security_hook_heads.sb_parse_opts_str),
 	.dentry_init_security =
 		LIST_HEAD_INIT(security_hook_heads.dentry_init_security),
+	.dentry_create_files_as =
+		LIST_HEAD_INIT(security_hook_heads.dentry_create_files_as),
 #ifdef CONFIG_SECURITY_PATH
 	.path_unlink =	LIST_HEAD_INIT(security_hook_heads.path_unlink),
 	.path_mkdir =	LIST_HEAD_INIT(security_hook_heads.path_mkdir),
-- 
2.7.4

[PATCH 7/9] selinux: Implement dentry_create_files_as() hook

[Vivek Goyal]

Calculate what would be the label of newly created file and set that secid
in the passed creds.

Context of the task which is actually creating file is retrieved from
set of creds passed in. (old->security).

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 security/selinux/hooks.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index ae11fd9..77eb5a8 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2825,6 +2825,27 @@ static int selinux_dentry_init_security(struct dentry *dentry, int mode,
 	return security_sid_to_context(newsid, (char **)ctx, ctxlen);
 }
 
+static int selinux_dentry_create_files_as(struct dentry *dentry, int mode,
+					  struct qstr *name,
+					  const struct cred *old,
+					  struct cred *new)
+{
+	u32 newsid;
+	int rc;
+	struct task_security_struct *tsec;
+
+	rc = selinux_determine_inode_label(old->security,
+					   d_inode(dentry->d_parent), name,
+					   inode_mode_to_security_class(mode),
+					   &newsid);
+	if (rc)
+		return rc;
+
+	tsec = new->security;
+	tsec->create_sid = newsid;
+	return 0;
+}
+
 static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
 				       const struct qstr *qstr,
 				       const char **name,
@@ -6066,6 +6087,7 @@ static struct security_hook_list selinux_hooks[] = {
 	LSM_HOOK_INIT(sb_parse_opts_str, selinux_parse_opts_str),
 
 	LSM_HOOK_INIT(dentry_init_security, selinux_dentry_init_security),
+	LSM_HOOK_INIT(dentry_create_files_as, selinux_dentry_create_files_as),
 
 	LSM_HOOK_INIT(inode_alloc_security, selinux_inode_alloc_security),
 	LSM_HOOK_INIT(inode_free_security, selinux_inode_free_security),
-- 
2.7.4

[PATCH 8/9] overlayfs: Dilute permission checks on lower only if not special file

[Vivek Goyal]

Right now if file is on lower/, we remove MAY_WRITE/MAY_APPEND bits from
mask as lower/ will never be written and file will be copied up. But this
is not true for special files. These files are not copied up and are
opened in place. So don't dilute the checks for these types of files.

Reported-by: Dan Walsh <dwalsh@redhat.com>
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 fs/overlayfs/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 66f42f5..6d9d86e 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -140,7 +140,7 @@ int ovl_permission(struct inode *inode, int mask)
 		return err;
 
 	old_cred = ovl_override_creds(inode->i_sb);
-	if (!is_upper)
+	if (!is_upper && !special_file(realinode->i_mode))
 		mask &= ~(MAY_WRITE | MAY_APPEND);
 	err = inode_permission(realinode, mask);
 	revert_creds(old_cred);
-- 
2.7.4

[PATCH 9/9] overlayfs: Append MAY_READ when diluting write checks

[Vivek Goyal]

Right now we remove MAY_WRITE/MAY_APPEND bits from mask if realfile is
on lower/. This is done as files on lower will never be written and will
be copied up. But to copy up a file, mounter should have MAY_READ permission
otherwise copy up will fail. So set MAY_READ in mask when MAY_WRITE is
reset.

Dan Walsh noticed this when he did access(lowerfile, W_OK) and it returned
True (context mounts) but when he tried to actually write to file, it failed
as mounter did not have permission on lower file.

Reported-by: Dan Walsh <dwalsh@redhat.com>
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 fs/overlayfs/inode.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 6d9d86e..83b2422 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -140,8 +140,13 @@ int ovl_permission(struct inode *inode, int mask)
 		return err;
 
 	old_cred = ovl_override_creds(inode->i_sb);
-	if (!is_upper && !special_file(realinode->i_mode))
-		mask &= ~(MAY_WRITE | MAY_APPEND);
+	if (!is_upper && !special_file(realinode->i_mode)) {
+		if (mask & MAY_WRITE || mask & MAY_APPEND) {
+			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);
 
-- 
2.7.4

Re: [PATCH 1/9] security, overlayfs: provide copy up security hook for unioned files

[Stephen Smalley]

On 07/13/2016 10:44 AM, Vivek Goyal wrote:
> Provide a security hook to label new file correctly when a file is copied
> up from lower layer to upper layer of a overlay/union mount.
> 
> This hook can prepare a new set of creds which are suitable for new file
> creation during copy up. Caller will use new creds to create file and then
> revert back to old creds and release new creds.
> 
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> ---
>  fs/overlayfs/copy_up.c    | 18 ++++++++++++++++++
>  include/linux/lsm_hooks.h | 11 +++++++++++
>  include/linux/security.h  |  6 ++++++
>  security/security.c       |  8 ++++++++
>  4 files changed, 43 insertions(+)
> 
> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> index 80aa6f1..8ebea18 100644
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -246,6 +246,8 @@ static int ovl_copy_up_locked(struct dentry *workdir, struct dentry *upperdir,
>  	struct dentry *upper = NULL;
>  	umode_t mode = stat->mode;
>  	int err;
> +	const struct cred *old_creds = NULL;
> +	struct cred *new_creds = NULL;
>  
>  	newdentry = ovl_lookup_temp(workdir, dentry);
>  	err = PTR_ERR(newdentry);
> @@ -258,10 +260,26 @@ static int ovl_copy_up_locked(struct dentry *workdir, struct dentry *upperdir,
>  	if (IS_ERR(upper))
>  		goto out1;
>  
> +	err = security_inode_copy_up(dentry, &new_creds);
> +	if (err < 0) {
> +		if (new_creds)
> +			put_cred(new_creds);

I think this is a mistake, diverges from how other hooks handle error
conditions (if the hook allocates, the hook or the security
infrastructure is responsible for freeing on error return, not the
caller), and will be prone to double free errors.

> +		goto out2;
> +	}
> +
> +	if (new_creds)
> +		old_creds = override_creds(new_creds);
> +
>  	/* Can't properly set mode on creation because of the umask */
>  	stat->mode &= S_IFMT;
>  	err = ovl_create_real(wdir, newdentry, stat, link, NULL, true);
>  	stat->mode = mode;
> +
> +	if (new_creds) {
> +		revert_creds(old_creds);
> +		put_cred(new_creds);
> +	}
> +
>  	if (err)
>  		goto out2;
>  
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 7ae3976..c1f95be 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -401,6 +401,15 @@
>   *	@inode contains a pointer to the inode.
>   *	@secid contains a pointer to the location where result will be saved.
>   *	In case of failure, @secid will be set to zero.
> + * @inode_copy_up:
> + *	A file is about to be copied up from lower layer to upper layer of
> + *	overlay filesystem. Security module can prepare a set of new creds
> + *	and modify as need be and return new creds. Caller will switch to
> + *	new creds temporarily to create new file and release newly allocated
> + *	creds.
> + *	@src indicates the union dentry of file that is being copied up.
> + *	@new pointer to pointer to return newly allocated creds.
> + *	Returns 0 on success or a negative error code on error.
>   *
>   * Security hooks for file operations
>   *
> @@ -1425,6 +1434,7 @@ union security_list_options {
>  	int (*inode_listsecurity)(struct inode *inode, char *buffer,
>  					size_t buffer_size);
>  	void (*inode_getsecid)(struct inode *inode, u32 *secid);
> +	int (*inode_copy_up) (struct dentry *src, struct cred **new);
>  
>  	int (*file_permission)(struct file *file, int mask);
>  	int (*file_alloc_security)(struct file *file);
> @@ -1696,6 +1706,7 @@ struct security_hook_heads {
>  	struct list_head inode_setsecurity;
>  	struct list_head inode_listsecurity;
>  	struct list_head inode_getsecid;
> +	struct list_head inode_copy_up;
>  	struct list_head file_permission;
>  	struct list_head file_alloc_security;
>  	struct list_head file_free_security;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 14df373..c976d79 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -282,6 +282,7 @@ int security_inode_getsecurity(struct inode *inode, const char *name, void **buf
>  int security_inode_setsecurity(struct inode *inode, const char *name, const void *value, size_t size, int flags);
>  int security_inode_listsecurity(struct inode *inode, char *buffer, size_t buffer_size);
>  void security_inode_getsecid(struct inode *inode, u32 *secid);
> +int security_inode_copy_up(struct dentry *src, struct cred **new);
>  int security_file_permission(struct file *file, int mask);
>  int security_file_alloc(struct file *file);
>  void security_file_free(struct file *file);
> @@ -758,6 +759,11 @@ static inline void security_inode_getsecid(struct inode *inode, u32 *secid)
>  	*secid = 0;
>  }
>  
> +static inline int security_inode_copy_up(struct dentry *src, struct cred **new)
> +{
> +	return 0;
> +}
> +
>  static inline int security_file_permission(struct file *file, int mask)
>  {
>  	return 0;
> diff --git a/security/security.c b/security/security.c
> index 7095693..3d142aa 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -727,6 +727,12 @@ void security_inode_getsecid(struct inode *inode, u32 *secid)
>  	call_void_hook(inode_getsecid, inode, secid);
>  }
>  
> +int security_inode_copy_up(struct dentry *src, struct cred **new)
> +{
> +	return call_int_hook(inode_copy_up, 0, src, new);
> +}
> +EXPORT_SYMBOL(security_inode_copy_up);
> +
>  int security_file_permission(struct file *file, int mask)
>  {
>  	int ret;
> @@ -1663,6 +1669,8 @@ struct security_hook_heads security_hook_heads = {
>  		LIST_HEAD_INIT(security_hook_heads.inode_listsecurity),
>  	.inode_getsecid =
>  		LIST_HEAD_INIT(security_hook_heads.inode_getsecid),
> +	.inode_copy_up =
> +		LIST_HEAD_INIT(security_hook_heads.inode_copy_up),
>  	.file_permission =
>  		LIST_HEAD_INIT(security_hook_heads.file_permission),
>  	.file_alloc_security =
> 

Re: [PATCH 2/9] selinux: Implementation for inode_copy_up() hook

[Stephen Smalley]

On 07/13/2016 10:44 AM, Vivek Goyal wrote:
> A file is being copied up for overlay file system. Prepare a new set of
> creds and set create_sid appropriately so that new file is created with
> appropriate label.
> 
> Overlay inode has right label for both context and non-context mount
> cases. In case of non-context mount, overlay inode will have the label
> of lower file and in case of context mount, overlay inode will have
> the label from context= mount option.
> 
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>

Acked-by: Stephen Smalley <sds@tycho.nsa.gov>

> ---
>  security/selinux/hooks.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index a86d537..c82ee54 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -3270,6 +3270,26 @@ static void selinux_inode_getsecid(struct inode *inode, u32 *secid)
>  	*secid = isec->sid;
>  }
>  
> +static int selinux_inode_copy_up(struct dentry *src, struct cred **new)
> +{
> +	u32 sid;
> +	struct task_security_struct *tsec;
> +	struct cred *new_creds = *new;
> +
> +	if (new_creds == NULL) {
> +		new_creds = prepare_creds();
> +		if (!new_creds)
> +			return -ENOMEM;
> +	}
> +
> +	tsec = new_creds->security;
> +	/* Get label from overlay inode and set it in create_sid */
> +	selinux_inode_getsecid(d_inode(src), &sid);
> +	tsec->create_sid = sid;
> +	*new = new_creds;
> +	return 0;
> +}
> +
>  /* file security operations */
>  
>  static int selinux_revalidate_file_permission(struct file *file, int mask)
> @@ -6056,6 +6076,7 @@ static struct security_hook_list selinux_hooks[] = {
>  	LSM_HOOK_INIT(inode_setsecurity, selinux_inode_setsecurity),
>  	LSM_HOOK_INIT(inode_listsecurity, selinux_inode_listsecurity),
>  	LSM_HOOK_INIT(inode_getsecid, selinux_inode_getsecid),
> +	LSM_HOOK_INIT(inode_copy_up, selinux_inode_copy_up),
>  
>  	LSM_HOOK_INIT(file_permission, selinux_file_permission),
>  	LSM_HOOK_INIT(file_alloc_security, selinux_file_alloc_security),
> 

Re: [PATCH 4/9] selinux: Implementation for inode_copy_up_xattr() hook

[Stephen Smalley]

On 07/13/2016 10:44 AM, Vivek Goyal wrote:
> When a file is copied up in overlay, we have already created file on upper/
> with right label and there is no need to copy up selinux label/xattr from
> lower file to upper file. In fact in case of context mount, we don't want
> to copy up label as newly created file got its label from context= option.
> 
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>

Acked-by: Stephen Smalley <sds@tycho.nsa.gov>

> ---
>  security/selinux/hooks.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index c82ee54..4fda548 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -3290,6 +3290,21 @@ static int selinux_inode_copy_up(struct dentry *src, struct cred **new)
>  	return 0;
>  }
>  
> +static int selinux_inode_copy_up_xattr(const char *name)
> +{
> +	/* The copy_up hook above sets the initial context on an inode, but we
> +	 * don't then want to overwrite it by blindly copying all the lower
> +	 * xattrs up.  Instead, we have to filter out SELinux-related xattrs.
> +	 */
> +	if (strcmp(name, XATTR_NAME_SELINUX) == 0)
> +		return 1; /* Discard */
> +	/*
> +	 * Any other attribute apart from SELINUX is not claimed, supported
> +	 * by selinux.
> +	 */
> +	return -EOPNOTSUPP;
> +}
> +
>  /* file security operations */
>  
>  static int selinux_revalidate_file_permission(struct file *file, int mask)
> @@ -6077,6 +6092,7 @@ static struct security_hook_list selinux_hooks[] = {
>  	LSM_HOOK_INIT(inode_listsecurity, selinux_inode_listsecurity),
>  	LSM_HOOK_INIT(inode_getsecid, selinux_inode_getsecid),
>  	LSM_HOOK_INIT(inode_copy_up, selinux_inode_copy_up),
> +	LSM_HOOK_INIT(inode_copy_up_xattr, selinux_inode_copy_up_xattr),
>  
>  	LSM_HOOK_INIT(file_permission, selinux_file_permission),
>  	LSM_HOOK_INIT(file_alloc_security, selinux_file_alloc_security),
> 

Re: [PATCH 1/9] security, overlayfs: provide copy up security hook for unioned files

[Vivek Goyal]

On Wed, Jul 13, 2016 at 10:52:34AM -0400, Stephen Smalley wrote:
> On 07/13/2016 10:44 AM, Vivek Goyal wrote:
> > Provide a security hook to label new file correctly when a file is copied
> > up from lower layer to upper layer of a overlay/union mount.
> > 
> > This hook can prepare a new set of creds which are suitable for new file
> > creation during copy up. Caller will use new creds to create file and then
> > revert back to old creds and release new creds.
> > 
> > Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> > ---
> >  fs/overlayfs/copy_up.c    | 18 ++++++++++++++++++
> >  include/linux/lsm_hooks.h | 11 +++++++++++
> >  include/linux/security.h  |  6 ++++++
> >  security/security.c       |  8 ++++++++
> >  4 files changed, 43 insertions(+)
> > 
> > diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> > index 80aa6f1..8ebea18 100644
> > --- a/fs/overlayfs/copy_up.c
> > +++ b/fs/overlayfs/copy_up.c
> > @@ -246,6 +246,8 @@ static int ovl_copy_up_locked(struct dentry *workdir, struct dentry *upperdir,
> >  	struct dentry *upper = NULL;
> >  	umode_t mode = stat->mode;
> >  	int err;
> > +	const struct cred *old_creds = NULL;
> > +	struct cred *new_creds = NULL;
> >  
> >  	newdentry = ovl_lookup_temp(workdir, dentry);
> >  	err = PTR_ERR(newdentry);
> > @@ -258,10 +260,26 @@ static int ovl_copy_up_locked(struct dentry *workdir, struct dentry *upperdir,
> >  	if (IS_ERR(upper))
> >  		goto out1;
> >  
> > +	err = security_inode_copy_up(dentry, &new_creds);
> > +	if (err < 0) {
> > +		if (new_creds)
> > +			put_cred(new_creds);
> 
> I think this is a mistake, diverges from how other hooks handle error
> conditions (if the hook allocates, the hook or the security
> infrastructure is responsible for freeing on error return, not the
> caller), and will be prone to double free errors.

Ok, I will get rid of it and assume LSM infrastructure will be responsible
for freeing this in case of error.

Vivek

Re: [PATCH 5/9] selinux: Pass security pointer to determine_inode_label()

[Stephen Smalley]

On 07/13/2016 10:44 AM, Vivek Goyal wrote:
> Right now selinux_determine_inode_label() works on security pointer of
> current task. Soon I need this to work on a security pointer retrieved
> from a set of creds. So start passing in a pointer and caller can decide
> where to fetch security pointer from.
> 
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>

Acked-by: Stephen Smalley <sds@tycho.nsa.gov>

> ---
>  security/selinux/hooks.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 4fda548..ae11fd9 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -1785,13 +1785,13 @@ out:
>  /*
>   * Determine the label for an inode that might be unioned.
>   */
> -static int selinux_determine_inode_label(struct inode *dir,
> -					 const struct qstr *name,
> -					 u16 tclass,
> -					 u32 *_new_isid)
> +static int
> +selinux_determine_inode_label(const struct task_security_struct *tsec,
> +				 struct inode *dir,
> +				 const struct qstr *name, u16 tclass,
> +				 u32 *_new_isid)
>  {
>  	const struct superblock_security_struct *sbsec = dir->i_sb->s_security;
> -	const struct task_security_struct *tsec = current_security();
>  
>  	if ((sbsec->flags & SE_SBINITIALIZED) &&
>  	    (sbsec->behavior == SECURITY_FS_USE_MNTPOINT)) {
> @@ -1834,8 +1834,8 @@ static int may_create(struct inode *dir,
>  	if (rc)
>  		return rc;
>  
> -	rc = selinux_determine_inode_label(dir, &dentry->d_name, tclass,
> -					   &newsid);
> +	rc = selinux_determine_inode_label(current_security(), dir,
> +					   &dentry->d_name, tclass, &newsid);
>  	if (rc)
>  		return rc;
>  
> @@ -2815,7 +2815,8 @@ static int selinux_dentry_init_security(struct dentry *dentry, int mode,
>  	u32 newsid;
>  	int rc;
>  
> -	rc = selinux_determine_inode_label(d_inode(dentry->d_parent), name,
> +	rc = selinux_determine_inode_label(current_security(),
> +					   d_inode(dentry->d_parent), name,
>  					   inode_mode_to_security_class(mode),
>  					   &newsid);
>  	if (rc)
> @@ -2840,7 +2841,7 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
>  	sid = tsec->sid;
>  	newsid = tsec->create_sid;
>  
> -	rc = selinux_determine_inode_label(
> +	rc = selinux_determine_inode_label(current_security(),
>  		dir, qstr,
>  		inode_mode_to_security_class(inode->i_mode),
>  		&newsid);
> 

Re: [PATCH 6/9] security, overlayfs: Provide hook to correctly label newly created files

[Stephen Smalley]

On 07/13/2016 10:44 AM, Vivek Goyal wrote:
> During a new file creation we need to make sure new file is created with the
> right label. New file is created in upper/ so effectively file should get
> label as if task had created file in upper/.
> 
> We switched to mounter's creds for actual file creation. Also if there is a
> whiteout present, then file will be created in work/ dir first and then
> renamed in upper. In none of the cases file will be labeled as we want it to
> be.
> 
> This patch introduces a new hook dentry_create_files_as(), which determines
> the label/context dentry will get if it had been created by task in upper
> and modify passed set of creds appropriately. Caller makes use of these new
> creds for file creation.
> 
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> ---
>  fs/overlayfs/dir.c        | 10 ++++++++++
>  include/linux/lsm_hooks.h | 15 +++++++++++++++
>  include/linux/security.h  | 12 ++++++++++++
>  security/security.c       | 11 +++++++++++
>  4 files changed, 48 insertions(+)
> 
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index 4cdeb74..f94872f 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -433,6 +433,15 @@ static int ovl_create_or_link(struct dentry *dentry, int mode, dev_t rdev,
>  	if (override_cred) {
>  		override_cred->fsuid = inode->i_uid;
>  		override_cred->fsgid = inode->i_gid;
> +		if (!hardlink) {
> +			err = security_dentry_create_files_as(dentry,
> +					mode, &dentry->d_name, old_cred,
> +					override_cred);
> +			if (err) {
> +				put_cred(override_cred);

Same principle here; on error the caller should do nothing with
override_cred.

> +				goto out_revert_creds;
> +			}
> +		}
>  		put_cred(override_creds(override_cred));
>  		put_cred(override_cred);
>  
> @@ -443,6 +452,7 @@ static int ovl_create_or_link(struct dentry *dentry, int mode, dev_t rdev,
>  			err = ovl_create_over_whiteout(dentry, inode, &stat,
>  							link, hardlink);
>  	}
> +out_revert_creds:
>  	revert_creds(old_cred);
>  	if (!err) {
>  		struct inode *realinode = d_inode(ovl_dentry_upper(dentry));
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 84caead..95745fe 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -151,6 +151,16 @@
>   *	@name name of the last path component used to create file
>   *	@ctx pointer to place the pointer to the resulting context in.
>   *	@ctxlen point to place the length of the resulting context.
> + * @dentry_create_files_as:
> + *	Compute a context for a dentry as the inode is not yet available
> + *	and set that context in passed in creds so that new files are
> + *	created using that context. Context is calculated using the
> + *	passed in creds and not the creds of the caller.
> + *	@dentry dentry to use in calculating the context.
> + *	@mode mode used to determine resource type.
> + *	@name name of the last path component used to create file
> + * 	@old creds which should be used for context calculation
> + * 	@new creds to modify
>   *
>   *
>   * Security hooks for inode operations.
> @@ -1375,6 +1385,10 @@ union security_list_options {
>  	int (*dentry_init_security)(struct dentry *dentry, int mode,
>  					struct qstr *name, void **ctx,
>  					u32 *ctxlen);
> +	int (*dentry_create_files_as)(struct dentry *dentry, int mode,
> +					struct qstr *name,
> +					const struct cred *old,
> +					struct cred *new);
>  
>  
>  #ifdef CONFIG_SECURITY_PATH
> @@ -1675,6 +1689,7 @@ struct security_hook_heads {
>  	struct list_head sb_clone_mnt_opts;
>  	struct list_head sb_parse_opts_str;
>  	struct list_head dentry_init_security;
> +	struct list_head dentry_create_files_as;
>  #ifdef CONFIG_SECURITY_PATH
>  	struct list_head path_unlink;
>  	struct list_head path_mkdir;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 4a3b8bc..1eb03dc 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -242,6 +242,10 @@ int security_sb_parse_opts_str(char *options, struct security_mnt_opts *opts);
>  int security_dentry_init_security(struct dentry *dentry, int mode,
>  					struct qstr *name, void **ctx,
>  					u32 *ctxlen);
> +int security_dentry_create_files_as(struct dentry *dentry, int mode,
> +					struct qstr *name,
> +					const struct cred *old,
> +					struct cred *new);
>  
>  int security_inode_alloc(struct inode *inode);
>  void security_inode_free(struct inode *inode);
> @@ -600,6 +604,14 @@ static inline int security_dentry_init_security(struct dentry *dentry,
>  	return -EOPNOTSUPP;
>  }
>  
> +static inline int security_dentry_create_files_as(struct dentry *dentry,
> +						  int mode, struct qstr *name,
> +						  const struct cred *old,
> +						  struct cred *new)
> +{
> +	return 0;
> +}
> +
>  
>  static inline int security_inode_init_security(struct inode *inode,
>  						struct inode *dir,
> diff --git a/security/security.c b/security/security.c
> index 3321e31..38747d1 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -364,6 +364,15 @@ int security_dentry_init_security(struct dentry *dentry, int mode,
>  }
>  EXPORT_SYMBOL(security_dentry_init_security);
>  
> +int security_dentry_create_files_as(struct dentry *dentry, int mode,
> +					struct qstr *name,
> +					const struct cred *old, struct cred *new)
> +{
> +	return call_int_hook(dentry_create_files_as, 0, dentry, mode,
> +				name, old, new);
> +}
> +EXPORT_SYMBOL(security_dentry_create_files_as);
> +
>  int security_inode_init_security(struct inode *inode, struct inode *dir,
>  				 const struct qstr *qstr,
>  				 const initxattrs initxattrs, void *fs_data)
> @@ -1614,6 +1623,8 @@ struct security_hook_heads security_hook_heads = {
>  		LIST_HEAD_INIT(security_hook_heads.sb_parse_opts_str),
>  	.dentry_init_security =
>  		LIST_HEAD_INIT(security_hook_heads.dentry_init_security),
> +	.dentry_create_files_as =
> +		LIST_HEAD_INIT(security_hook_heads.dentry_create_files_as),
>  #ifdef CONFIG_SECURITY_PATH
>  	.path_unlink =	LIST_HEAD_INIT(security_hook_heads.path_unlink),
>  	.path_mkdir =	LIST_HEAD_INIT(security_hook_heads.path_mkdir),
> 

Re: [PATCH 6/9] security, overlayfs: Provide hook to correctly label newly created files

[Stephen Smalley]

On 07/13/2016 10:57 AM, Stephen Smalley wrote:
> On 07/13/2016 10:44 AM, Vivek Goyal wrote:
>> During a new file creation we need to make sure new file is created with the
>> right label. New file is created in upper/ so effectively file should get
>> label as if task had created file in upper/.
>>
>> We switched to mounter's creds for actual file creation. Also if there is a
>> whiteout present, then file will be created in work/ dir first and then
>> renamed in upper. In none of the cases file will be labeled as we want it to
>> be.
>>
>> This patch introduces a new hook dentry_create_files_as(), which determines
>> the label/context dentry will get if it had been created by task in upper
>> and modify passed set of creds appropriately. Caller makes use of these new
>> creds for file creation.
>>
>> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
>> ---
>>  fs/overlayfs/dir.c        | 10 ++++++++++
>>  include/linux/lsm_hooks.h | 15 +++++++++++++++
>>  include/linux/security.h  | 12 ++++++++++++
>>  security/security.c       | 11 +++++++++++
>>  4 files changed, 48 insertions(+)
>>
>> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
>> index 4cdeb74..f94872f 100644
>> --- a/fs/overlayfs/dir.c
>> +++ b/fs/overlayfs/dir.c
>> @@ -433,6 +433,15 @@ static int ovl_create_or_link(struct dentry *dentry, int mode, dev_t rdev,
>>  	if (override_cred) {
>>  		override_cred->fsuid = inode->i_uid;
>>  		override_cred->fsgid = inode->i_gid;
>> +		if (!hardlink) {
>> +			err = security_dentry_create_files_as(dentry,
>> +					mode, &dentry->d_name, old_cred,
>> +					override_cred);
>> +			if (err) {
>> +				put_cred(override_cred);
> 
> Same principle here; on error the caller should do nothing with
> override_cred.

Sorry, never mind - not allocated by the hook so properly handled by the
caller.

> 
>> +				goto out_revert_creds;
>> +			}
>> +		}
>>  		put_cred(override_creds(override_cred));
>>  		put_cred(override_cred);
>>  
>> @@ -443,6 +452,7 @@ static int ovl_create_or_link(struct dentry *dentry, int mode, dev_t rdev,
>>  			err = ovl_create_over_whiteout(dentry, inode, &stat,
>>  							link, hardlink);
>>  	}
>> +out_revert_creds:
>>  	revert_creds(old_cred);
>>  	if (!err) {
>>  		struct inode *realinode = d_inode(ovl_dentry_upper(dentry));
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index 84caead..95745fe 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -151,6 +151,16 @@
>>   *	@name name of the last path component used to create file
>>   *	@ctx pointer to place the pointer to the resulting context in.
>>   *	@ctxlen point to place the length of the resulting context.
>> + * @dentry_create_files_as:
>> + *	Compute a context for a dentry as the inode is not yet available
>> + *	and set that context in passed in creds so that new files are
>> + *	created using that context. Context is calculated using the
>> + *	passed in creds and not the creds of the caller.
>> + *	@dentry dentry to use in calculating the context.
>> + *	@mode mode used to determine resource type.
>> + *	@name name of the last path component used to create file
>> + * 	@old creds which should be used for context calculation
>> + * 	@new creds to modify
>>   *
>>   *
>>   * Security hooks for inode operations.
>> @@ -1375,6 +1385,10 @@ union security_list_options {
>>  	int (*dentry_init_security)(struct dentry *dentry, int mode,
>>  					struct qstr *name, void **ctx,
>>  					u32 *ctxlen);
>> +	int (*dentry_create_files_as)(struct dentry *dentry, int mode,
>> +					struct qstr *name,
>> +					const struct cred *old,
>> +					struct cred *new);
>>  
>>  
>>  #ifdef CONFIG_SECURITY_PATH
>> @@ -1675,6 +1689,7 @@ struct security_hook_heads {
>>  	struct list_head sb_clone_mnt_opts;
>>  	struct list_head sb_parse_opts_str;
>>  	struct list_head dentry_init_security;
>> +	struct list_head dentry_create_files_as;
>>  #ifdef CONFIG_SECURITY_PATH
>>  	struct list_head path_unlink;
>>  	struct list_head path_mkdir;
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index 4a3b8bc..1eb03dc 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -242,6 +242,10 @@ int security_sb_parse_opts_str(char *options, struct security_mnt_opts *opts);
>>  int security_dentry_init_security(struct dentry *dentry, int mode,
>>  					struct qstr *name, void **ctx,
>>  					u32 *ctxlen);
>> +int security_dentry_create_files_as(struct dentry *dentry, int mode,
>> +					struct qstr *name,
>> +					const struct cred *old,
>> +					struct cred *new);
>>  
>>  int security_inode_alloc(struct inode *inode);
>>  void security_inode_free(struct inode *inode);
>> @@ -600,6 +604,14 @@ static inline int security_dentry_init_security(struct dentry *dentry,
>>  	return -EOPNOTSUPP;
>>  }
>>  
>> +static inline int security_dentry_create_files_as(struct dentry *dentry,
>> +						  int mode, struct qstr *name,
>> +						  const struct cred *old,
>> +						  struct cred *new)
>> +{
>> +	return 0;
>> +}
>> +
>>  
>>  static inline int security_inode_init_security(struct inode *inode,
>>  						struct inode *dir,
>> diff --git a/security/security.c b/security/security.c
>> index 3321e31..38747d1 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -364,6 +364,15 @@ int security_dentry_init_security(struct dentry *dentry, int mode,
>>  }
>>  EXPORT_SYMBOL(security_dentry_init_security);
>>  
>> +int security_dentry_create_files_as(struct dentry *dentry, int mode,
>> +					struct qstr *name,
>> +					const struct cred *old, struct cred *new)
>> +{
>> +	return call_int_hook(dentry_create_files_as, 0, dentry, mode,
>> +				name, old, new);
>> +}
>> +EXPORT_SYMBOL(security_dentry_create_files_as);
>> +
>>  int security_inode_init_security(struct inode *inode, struct inode *dir,
>>  				 const struct qstr *qstr,
>>  				 const initxattrs initxattrs, void *fs_data)
>> @@ -1614,6 +1623,8 @@ struct security_hook_heads security_hook_heads = {
>>  		LIST_HEAD_INIT(security_hook_heads.sb_parse_opts_str),
>>  	.dentry_init_security =
>>  		LIST_HEAD_INIT(security_hook_heads.dentry_init_security),
>> +	.dentry_create_files_as =
>> +		LIST_HEAD_INIT(security_hook_heads.dentry_create_files_as),
>>  #ifdef CONFIG_SECURITY_PATH
>>  	.path_unlink =	LIST_HEAD_INIT(security_hook_heads.path_unlink),
>>  	.path_mkdir =	LIST_HEAD_INIT(security_hook_heads.path_mkdir),
>>
> 

Re: [PATCH 7/9] selinux: Implement dentry_create_files_as() hook

[Stephen Smalley]

On 07/13/2016 10:44 AM, Vivek Goyal wrote:
> Calculate what would be the label of newly created file and set that secid
> in the passed creds.
> 
> Context of the task which is actually creating file is retrieved from
> set of creds passed in. (old->security).
> 
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>

Acked-by: Stephen Smalley <sds@tycho.nsa.gov>

> ---
>  security/selinux/hooks.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index ae11fd9..77eb5a8 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2825,6 +2825,27 @@ static int selinux_dentry_init_security(struct dentry *dentry, int mode,
>  	return security_sid_to_context(newsid, (char **)ctx, ctxlen);
>  }
>  
> +static int selinux_dentry_create_files_as(struct dentry *dentry, int mode,
> +					  struct qstr *name,
> +					  const struct cred *old,
> +					  struct cred *new)
> +{
> +	u32 newsid;
> +	int rc;
> +	struct task_security_struct *tsec;
> +
> +	rc = selinux_determine_inode_label(old->security,
> +					   d_inode(dentry->d_parent), name,
> +					   inode_mode_to_security_class(mode),
> +					   &newsid);
> +	if (rc)
> +		return rc;
> +
> +	tsec = new->security;
> +	tsec->create_sid = newsid;
> +	return 0;
> +}
> +
>  static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
>  				       const struct qstr *qstr,
>  				       const char **name,
> @@ -6066,6 +6087,7 @@ static struct security_hook_list selinux_hooks[] = {
>  	LSM_HOOK_INIT(sb_parse_opts_str, selinux_parse_opts_str),
>  
>  	LSM_HOOK_INIT(dentry_init_security, selinux_dentry_init_security),
> +	LSM_HOOK_INIT(dentry_create_files_as, selinux_dentry_create_files_as),
>  
>  	LSM_HOOK_INIT(inode_alloc_security, selinux_inode_alloc_security),
>  	LSM_HOOK_INIT(inode_free_security, selinux_inode_free_security),
> 

Re: [PATCH 1/9] security, overlayfs: provide copy up security hook for unioned files

[Vivek Goyal]

Updated patch as per Stephen's feedback.

Provide a security hook to label new file correctly when a file is copied
up from lower layer to upper layer of a overlay/union mount.

This hook can prepare a new set of creds which are suitable for new file
creation during copy up. Caller will use new creds to create file and then
revert back to old creds and release new creds.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 fs/overlayfs/copy_up.c    | 15 +++++++++++++++
 include/linux/lsm_hooks.h | 11 +++++++++++
 include/linux/security.h  |  6 ++++++
 security/security.c       |  8 ++++++++
 4 files changed, 40 insertions(+)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 80aa6f1..e5e3557 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -246,6 +246,8 @@ static int ovl_copy_up_locked(struct dentry *workdir, struct dentry *upperdir,
 	struct dentry *upper = NULL;
 	umode_t mode = stat->mode;
 	int err;
+	const struct cred *old_creds = NULL;
+	struct cred *new_creds = NULL;
 
 	newdentry = ovl_lookup_temp(workdir, dentry);
 	err = PTR_ERR(newdentry);
@@ -258,10 +260,23 @@ static int ovl_copy_up_locked(struct dentry *workdir, struct dentry *upperdir,
 	if (IS_ERR(upper))
 		goto out1;
 
+	err = security_inode_copy_up(dentry, &new_creds);
+	if (err < 0)
+		goto out2;
+
+	if (new_creds)
+		old_creds = override_creds(new_creds);
+
 	/* Can't properly set mode on creation because of the umask */
 	stat->mode &= S_IFMT;
 	err = ovl_create_real(wdir, newdentry, stat, link, NULL, true);
 	stat->mode = mode;
+
+	if (new_creds) {
+		revert_creds(old_creds);
+		put_cred(new_creds);
+	}
+
 	if (err)
 		goto out2;
 
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 7ae3976..c1f95be 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -401,6 +401,15 @@
  *	@inode contains a pointer to the inode.
  *	@secid contains a pointer to the location where result will be saved.
  *	In case of failure, @secid will be set to zero.
+ * @inode_copy_up:
+ *	A file is about to be copied up from lower layer to upper layer of
+ *	overlay filesystem. Security module can prepare a set of new creds
+ *	and modify as need be and return new creds. Caller will switch to
+ *	new creds temporarily to create new file and release newly allocated
+ *	creds.
+ *	@src indicates the union dentry of file that is being copied up.
+ *	@new pointer to pointer to return newly allocated creds.
+ *	Returns 0 on success or a negative error code on error.
  *
  * Security hooks for file operations
  *
@@ -1425,6 +1434,7 @@ union security_list_options {
 	int (*inode_listsecurity)(struct inode *inode, char *buffer,
 					size_t buffer_size);
 	void (*inode_getsecid)(struct inode *inode, u32 *secid);
+	int (*inode_copy_up) (struct dentry *src, struct cred **new);
 
 	int (*file_permission)(struct file *file, int mask);
 	int (*file_alloc_security)(struct file *file);
@@ -1696,6 +1706,7 @@ struct security_hook_heads {
 	struct list_head inode_setsecurity;
 	struct list_head inode_listsecurity;
 	struct list_head inode_getsecid;
+	struct list_head inode_copy_up;
 	struct list_head file_permission;
 	struct list_head file_alloc_security;
 	struct list_head file_free_security;
diff --git a/include/linux/security.h b/include/linux/security.h
index 14df373..c976d79 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -282,6 +282,7 @@ int security_inode_getsecurity(struct inode *inode, const char *name, void **buf
 int security_inode_setsecurity(struct inode *inode, const char *name, const void *value, size_t size, int flags);
 int security_inode_listsecurity(struct inode *inode, char *buffer, size_t buffer_size);
 void security_inode_getsecid(struct inode *inode, u32 *secid);
+int security_inode_copy_up(struct dentry *src, struct cred **new);
 int security_file_permission(struct file *file, int mask);
 int security_file_alloc(struct file *file);
 void security_file_free(struct file *file);
@@ -758,6 +759,11 @@ static inline void security_inode_getsecid(struct inode *inode, u32 *secid)
 	*secid = 0;
 }
 
+static inline int security_inode_copy_up(struct dentry *src, struct cred **new)
+{
+	return 0;
+}
+
 static inline int security_file_permission(struct file *file, int mask)
 {
 	return 0;
diff --git a/security/security.c b/security/security.c
index 7095693..3d142aa 100644
--- a/security/security.c
+++ b/security/security.c
@@ -727,6 +727,12 @@ void security_inode_getsecid(struct inode *inode, u32 *secid)
 	call_void_hook(inode_getsecid, inode, secid);
 }
 
+int security_inode_copy_up(struct dentry *src, struct cred **new)
+{
+	return call_int_hook(inode_copy_up, 0, src, new);
+}
+EXPORT_SYMBOL(security_inode_copy_up);
+
 int security_file_permission(struct file *file, int mask)
 {
 	int ret;
@@ -1663,6 +1669,8 @@ struct security_hook_heads security_hook_heads = {
 		LIST_HEAD_INIT(security_hook_heads.inode_listsecurity),
 	.inode_getsecid =
 		LIST_HEAD_INIT(security_hook_heads.inode_getsecid),
+	.inode_copy_up =
+		LIST_HEAD_INIT(security_hook_heads.inode_copy_up),
 	.file_permission =
 		LIST_HEAD_INIT(security_hook_heads.file_permission),
 	.file_alloc_security =
-- 
2.7.4

Re: [PATCH 9/9] overlayfs: Append MAY_READ when diluting write checks

[Miklos Szeredi]

On Wed, Jul 13, 2016 at 4:44 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> Right now we remove MAY_WRITE/MAY_APPEND bits from mask if realfile is
> on lower/. This is done as files on lower will never be written and will
> be copied up. But to copy up a file, mounter should have MAY_READ permission
> otherwise copy up will fail. So set MAY_READ in mask when MAY_WRITE is
> reset.
>
> Dan Walsh noticed this when he did access(lowerfile, W_OK) and it returned
> True (context mounts) but when he tried to actually write to file, it failed
> as mounter did not have permission on lower file.
>
> Reported-by: Dan Walsh <dwalsh@redhat.com>
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> ---
>  fs/overlayfs/inode.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
> index 6d9d86e..83b2422 100644
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -140,8 +140,13 @@ int ovl_permission(struct inode *inode, int mask)
>                 return err;
>
>         old_cred = ovl_override_creds(inode->i_sb);
> -       if (!is_upper && !special_file(realinode->i_mode))
> -               mask &= ~(MAY_WRITE | MAY_APPEND);
> +       if (!is_upper && !special_file(realinode->i_mode)) {
> +               if (mask & MAY_WRITE || mask & MAY_APPEND) {

Hmm.   From what I can tell O_APPEND with O_RDONLY is equivalent to
O_RDONLY, hence MAY_APPEND without MAY_WRITE should not behave as if
MAY_WRITE was given.  I.e. it should not trigger a copy up.  Not sure
what MAC checks do with MAY_APPEND, though.

Pushed amended commit to overlayfs-next.

Thanks,
Miklos


> +                       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);
>
> --
> 2.7.4
>

Re: [PATCH 8/9] overlayfs: Dilute permission checks on lower only if not special file

[Miklos Szeredi]

On Wed, Jul 13, 2016 at 4:44 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> Right now if file is on lower/, we remove MAY_WRITE/MAY_APPEND bits from
> mask as lower/ will never be written and file will be copied up. But this
> is not true for special files. These files are not copied up and are
> opened in place. So don't dilute the checks for these types of files.

Pushed this one as well.

Miklos

>
> Reported-by: Dan Walsh <dwalsh@redhat.com>
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> ---
>  fs/overlayfs/inode.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
> index 66f42f5..6d9d86e 100644
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -140,7 +140,7 @@ int ovl_permission(struct inode *inode, int mask)
>                 return err;
>
>         old_cred = ovl_override_creds(inode->i_sb);
> -       if (!is_upper)
> +       if (!is_upper && !special_file(realinode->i_mode))
>                 mask &= ~(MAY_WRITE | MAY_APPEND);
>         err = inode_permission(realinode, mask);
>         revert_creds(old_cred);
> --
> 2.7.4
>

Re: [PATCH 3/9] security,overlayfs: Provide security hook for copy up of xattrs for overlay file

[Stephen Smalley]

On 07/13/2016 10:44 AM, Vivek Goyal wrote:
> Provide a security hook which is called when xattrs of a file are being
> copied up. This hook is called once for each xattr and LSM can return
> 0 if the security module wants the xattr to be copied up, 1 if the
> security module wants the xattr to be discarded on the copy, -EOPNOTSUPP
> if the security module does not handle/manage the xattr, or a -errno
> upon an error.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>

Acked-by: Stephen Smalley <sds@tycho.nsa.gov>

> ---
>  fs/overlayfs/copy_up.c    |  7 +++++++
>  include/linux/lsm_hooks.h | 10 ++++++++++
>  include/linux/security.h  |  6 ++++++
>  security/security.c       |  8 ++++++++
>  4 files changed, 31 insertions(+)
> 
> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> index 8ebea18..68cefb2 100644
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -103,6 +103,13 @@ retry:
>  			goto retry;
>  		}
>  
> +		error = security_inode_copy_up_xattr(name);
> +		if (error < 0 && error != -EOPNOTSUPP)
> +			break;
> +		if (error == 1) {
> +			error = 0;
> +			continue; /* Discard */
> +		}
>  		error = vfs_setxattr(new, name, value, size, 0);
>  		if (error)
>  			break;
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index c1f95be..84caead 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -410,6 +410,14 @@
>   *	@src indicates the union dentry of file that is being copied up.
>   *	@new pointer to pointer to return newly allocated creds.
>   *	Returns 0 on success or a negative error code on error.
> + * @inode_copy_up_xattr:
> + *	Filter the xattrs being copied up when a unioned file is copied
> + *	up from a lower layer to the union/overlay layer.
> + *	@name indicates the name of the xattr.
> + *	Returns 0 to accept the xattr, 1 to discard the xattr, -EOPNOTSUPP if
> + *	security module does not know about attribute or a negative error code
> + *	to abort the copy up. Note that the caller is responsible for reading
> + *	and writing the xattrs as this hook is merely a filter.
>   *
>   * Security hooks for file operations
>   *
> @@ -1435,6 +1443,7 @@ union security_list_options {
>  					size_t buffer_size);
>  	void (*inode_getsecid)(struct inode *inode, u32 *secid);
>  	int (*inode_copy_up) (struct dentry *src, struct cred **new);
> +	int (*inode_copy_up_xattr) (const char *name);
>  
>  	int (*file_permission)(struct file *file, int mask);
>  	int (*file_alloc_security)(struct file *file);
> @@ -1707,6 +1716,7 @@ struct security_hook_heads {
>  	struct list_head inode_listsecurity;
>  	struct list_head inode_getsecid;
>  	struct list_head inode_copy_up;
> +	struct list_head inode_copy_up_xattr;
>  	struct list_head file_permission;
>  	struct list_head file_alloc_security;
>  	struct list_head file_free_security;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index c976d79..4a3b8bc 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -283,6 +283,7 @@ int security_inode_setsecurity(struct inode *inode, const char *name, const void
>  int security_inode_listsecurity(struct inode *inode, char *buffer, size_t buffer_size);
>  void security_inode_getsecid(struct inode *inode, u32 *secid);
>  int security_inode_copy_up(struct dentry *src, struct cred **new);
> +int security_inode_copy_up_xattr(const char *name);
>  int security_file_permission(struct file *file, int mask);
>  int security_file_alloc(struct file *file);
>  void security_file_free(struct file *file);
> @@ -764,6 +765,11 @@ static inline int security_inode_copy_up(struct dentry *src, struct cred **new)
>  	return 0;
>  }
>  
> +static inline int security_inode_copy_up_xattr(const char *name)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
>  static inline int security_file_permission(struct file *file, int mask)
>  {
>  	return 0;
> diff --git a/security/security.c b/security/security.c
> index 3d142aa..3321e31 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -733,6 +733,12 @@ int security_inode_copy_up(struct dentry *src, struct cred **new)
>  }
>  EXPORT_SYMBOL(security_inode_copy_up);
>  
> +int security_inode_copy_up_xattr(const char *name)
> +{
> +	return call_int_hook(inode_copy_up_xattr, -EOPNOTSUPP, name);
> +}
> +EXPORT_SYMBOL(security_inode_copy_up_xattr);
> +
>  int security_file_permission(struct file *file, int mask)
>  {
>  	int ret;
> @@ -1671,6 +1677,8 @@ struct security_hook_heads security_hook_heads = {
>  		LIST_HEAD_INIT(security_hook_heads.inode_getsecid),
>  	.inode_copy_up =
>  		LIST_HEAD_INIT(security_hook_heads.inode_copy_up),
> +	.inode_copy_up_xattr =
> +		LIST_HEAD_INIT(security_hook_heads.inode_copy_up_xattr),
>  	.file_permission =
>  		LIST_HEAD_INIT(security_hook_heads.file_permission),
>  	.file_alloc_security =
> 

Re: [PATCH 6/9] security, overlayfs: Provide hook to correctly label newly created files

[Stephen Smalley]

On 07/13/2016 10:44 AM, Vivek Goyal wrote:
> During a new file creation we need to make sure new file is created with the
> right label. New file is created in upper/ so effectively file should get
> label as if task had created file in upper/.
> 
> We switched to mounter's creds for actual file creation. Also if there is a
> whiteout present, then file will be created in work/ dir first and then
> renamed in upper. In none of the cases file will be labeled as we want it to
> be.
> 
> This patch introduces a new hook dentry_create_files_as(), which determines
> the label/context dentry will get if it had been created by task in upper
> and modify passed set of creds appropriately. Caller makes use of these new
> creds for file creation.
> 
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>

Acked-by: Stephen Smalley <sds@tycho.nsa.gov>

> ---
>  fs/overlayfs/dir.c        | 10 ++++++++++
>  include/linux/lsm_hooks.h | 15 +++++++++++++++
>  include/linux/security.h  | 12 ++++++++++++
>  security/security.c       | 11 +++++++++++
>  4 files changed, 48 insertions(+)
> 
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index 4cdeb74..f94872f 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -433,6 +433,15 @@ static int ovl_create_or_link(struct dentry *dentry, int mode, dev_t rdev,
>  	if (override_cred) {
>  		override_cred->fsuid = inode->i_uid;
>  		override_cred->fsgid = inode->i_gid;
> +		if (!hardlink) {
> +			err = security_dentry_create_files_as(dentry,
> +					mode, &dentry->d_name, old_cred,
> +					override_cred);
> +			if (err) {
> +				put_cred(override_cred);
> +				goto out_revert_creds;
> +			}
> +		}
>  		put_cred(override_creds(override_cred));
>  		put_cred(override_cred);
>  
> @@ -443,6 +452,7 @@ static int ovl_create_or_link(struct dentry *dentry, int mode, dev_t rdev,
>  			err = ovl_create_over_whiteout(dentry, inode, &stat,
>  							link, hardlink);
>  	}
> +out_revert_creds:
>  	revert_creds(old_cred);
>  	if (!err) {
>  		struct inode *realinode = d_inode(ovl_dentry_upper(dentry));
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 84caead..95745fe 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -151,6 +151,16 @@
>   *	@name name of the last path component used to create file
>   *	@ctx pointer to place the pointer to the resulting context in.
>   *	@ctxlen point to place the length of the resulting context.
> + * @dentry_create_files_as:
> + *	Compute a context for a dentry as the inode is not yet available
> + *	and set that context in passed in creds so that new files are
> + *	created using that context. Context is calculated using the
> + *	passed in creds and not the creds of the caller.
> + *	@dentry dentry to use in calculating the context.
> + *	@mode mode used to determine resource type.
> + *	@name name of the last path component used to create file
> + * 	@old creds which should be used for context calculation
> + * 	@new creds to modify
>   *
>   *
>   * Security hooks for inode operations.
> @@ -1375,6 +1385,10 @@ union security_list_options {
>  	int (*dentry_init_security)(struct dentry *dentry, int mode,
>  					struct qstr *name, void **ctx,
>  					u32 *ctxlen);
> +	int (*dentry_create_files_as)(struct dentry *dentry, int mode,
> +					struct qstr *name,
> +					const struct cred *old,
> +					struct cred *new);
>  
>  
>  #ifdef CONFIG_SECURITY_PATH
> @@ -1675,6 +1689,7 @@ struct security_hook_heads {
>  	struct list_head sb_clone_mnt_opts;
>  	struct list_head sb_parse_opts_str;
>  	struct list_head dentry_init_security;
> +	struct list_head dentry_create_files_as;
>  #ifdef CONFIG_SECURITY_PATH
>  	struct list_head path_unlink;
>  	struct list_head path_mkdir;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 4a3b8bc..1eb03dc 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -242,6 +242,10 @@ int security_sb_parse_opts_str(char *options, struct security_mnt_opts *opts);
>  int security_dentry_init_security(struct dentry *dentry, int mode,
>  					struct qstr *name, void **ctx,
>  					u32 *ctxlen);
> +int security_dentry_create_files_as(struct dentry *dentry, int mode,
> +					struct qstr *name,
> +					const struct cred *old,
> +					struct cred *new);
>  
>  int security_inode_alloc(struct inode *inode);
>  void security_inode_free(struct inode *inode);
> @@ -600,6 +604,14 @@ static inline int security_dentry_init_security(struct dentry *dentry,
>  	return -EOPNOTSUPP;
>  }
>  
> +static inline int security_dentry_create_files_as(struct dentry *dentry,
> +						  int mode, struct qstr *name,
> +						  const struct cred *old,
> +						  struct cred *new)
> +{
> +	return 0;
> +}
> +
>  
>  static inline int security_inode_init_security(struct inode *inode,
>  						struct inode *dir,
> diff --git a/security/security.c b/security/security.c
> index 3321e31..38747d1 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -364,6 +364,15 @@ int security_dentry_init_security(struct dentry *dentry, int mode,
>  }
>  EXPORT_SYMBOL(security_dentry_init_security);
>  
> +int security_dentry_create_files_as(struct dentry *dentry, int mode,
> +					struct qstr *name,
> +					const struct cred *old, struct cred *new)
> +{
> +	return call_int_hook(dentry_create_files_as, 0, dentry, mode,
> +				name, old, new);
> +}
> +EXPORT_SYMBOL(security_dentry_create_files_as);
> +
>  int security_inode_init_security(struct inode *inode, struct inode *dir,
>  				 const struct qstr *qstr,
>  				 const initxattrs initxattrs, void *fs_data)
> @@ -1614,6 +1623,8 @@ struct security_hook_heads security_hook_heads = {
>  		LIST_HEAD_INIT(security_hook_heads.sb_parse_opts_str),
>  	.dentry_init_security =
>  		LIST_HEAD_INIT(security_hook_heads.dentry_init_security),
> +	.dentry_create_files_as =
> +		LIST_HEAD_INIT(security_hook_heads.dentry_create_files_as),
>  #ifdef CONFIG_SECURITY_PATH
>  	.path_unlink =	LIST_HEAD_INIT(security_hook_heads.path_unlink),
>  	.path_mkdir =	LIST_HEAD_INIT(security_hook_heads.path_mkdir),
> 

Re: [PATCH 1/9] security, overlayfs: provide copy up security hook for unioned files

[Stephen Smalley]

On 07/13/2016 11:13 AM, Vivek Goyal wrote:
> Updated patch as per Stephen's feedback.
> 
> Provide a security hook to label new file correctly when a file is copied
> up from lower layer to upper layer of a overlay/union mount.
> 
> This hook can prepare a new set of creds which are suitable for new file
> creation during copy up. Caller will use new creds to create file and then
> revert back to old creds and release new creds.
> 
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>

Acked-by: Stephen Smalley <sds@tycho.nsa.gov>

> ---
>  fs/overlayfs/copy_up.c    | 15 +++++++++++++++
>  include/linux/lsm_hooks.h | 11 +++++++++++
>  include/linux/security.h  |  6 ++++++
>  security/security.c       |  8 ++++++++
>  4 files changed, 40 insertions(+)
> 
> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> index 80aa6f1..e5e3557 100644
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -246,6 +246,8 @@ static int ovl_copy_up_locked(struct dentry *workdir, struct dentry *upperdir,
>  	struct dentry *upper = NULL;
>  	umode_t mode = stat->mode;
>  	int err;
> +	const struct cred *old_creds = NULL;
> +	struct cred *new_creds = NULL;
>  
>  	newdentry = ovl_lookup_temp(workdir, dentry);
>  	err = PTR_ERR(newdentry);
> @@ -258,10 +260,23 @@ static int ovl_copy_up_locked(struct dentry *workdir, struct dentry *upperdir,
>  	if (IS_ERR(upper))
>  		goto out1;
>  
> +	err = security_inode_copy_up(dentry, &new_creds);
> +	if (err < 0)
> +		goto out2;
> +
> +	if (new_creds)
> +		old_creds = override_creds(new_creds);
> +
>  	/* Can't properly set mode on creation because of the umask */
>  	stat->mode &= S_IFMT;
>  	err = ovl_create_real(wdir, newdentry, stat, link, NULL, true);
>  	stat->mode = mode;
> +
> +	if (new_creds) {
> +		revert_creds(old_creds);
> +		put_cred(new_creds);
> +	}
> +
>  	if (err)
>  		goto out2;
>  
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 7ae3976..c1f95be 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -401,6 +401,15 @@
>   *	@inode contains a pointer to the inode.
>   *	@secid contains a pointer to the location where result will be saved.
>   *	In case of failure, @secid will be set to zero.
> + * @inode_copy_up:
> + *	A file is about to be copied up from lower layer to upper layer of
> + *	overlay filesystem. Security module can prepare a set of new creds
> + *	and modify as need be and return new creds. Caller will switch to
> + *	new creds temporarily to create new file and release newly allocated
> + *	creds.
> + *	@src indicates the union dentry of file that is being copied up.
> + *	@new pointer to pointer to return newly allocated creds.
> + *	Returns 0 on success or a negative error code on error.
>   *
>   * Security hooks for file operations
>   *
> @@ -1425,6 +1434,7 @@ union security_list_options {
>  	int (*inode_listsecurity)(struct inode *inode, char *buffer,
>  					size_t buffer_size);
>  	void (*inode_getsecid)(struct inode *inode, u32 *secid);
> +	int (*inode_copy_up) (struct dentry *src, struct cred **new);
>  
>  	int (*file_permission)(struct file *file, int mask);
>  	int (*file_alloc_security)(struct file *file);
> @@ -1696,6 +1706,7 @@ struct security_hook_heads {
>  	struct list_head inode_setsecurity;
>  	struct list_head inode_listsecurity;
>  	struct list_head inode_getsecid;
> +	struct list_head inode_copy_up;
>  	struct list_head file_permission;
>  	struct list_head file_alloc_security;
>  	struct list_head file_free_security;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 14df373..c976d79 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -282,6 +282,7 @@ int security_inode_getsecurity(struct inode *inode, const char *name, void **buf
>  int security_inode_setsecurity(struct inode *inode, const char *name, const void *value, size_t size, int flags);
>  int security_inode_listsecurity(struct inode *inode, char *buffer, size_t buffer_size);
>  void security_inode_getsecid(struct inode *inode, u32 *secid);
> +int security_inode_copy_up(struct dentry *src, struct cred **new);
>  int security_file_permission(struct file *file, int mask);
>  int security_file_alloc(struct file *file);
>  void security_file_free(struct file *file);
> @@ -758,6 +759,11 @@ static inline void security_inode_getsecid(struct inode *inode, u32 *secid)
>  	*secid = 0;
>  }
>  
> +static inline int security_inode_copy_up(struct dentry *src, struct cred **new)
> +{
> +	return 0;
> +}
> +
>  static inline int security_file_permission(struct file *file, int mask)
>  {
>  	return 0;
> diff --git a/security/security.c b/security/security.c
> index 7095693..3d142aa 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -727,6 +727,12 @@ void security_inode_getsecid(struct inode *inode, u32 *secid)
>  	call_void_hook(inode_getsecid, inode, secid);
>  }
>  
> +int security_inode_copy_up(struct dentry *src, struct cred **new)
> +{
> +	return call_int_hook(inode_copy_up, 0, src, new);
> +}
> +EXPORT_SYMBOL(security_inode_copy_up);
> +
>  int security_file_permission(struct file *file, int mask)
>  {
>  	int ret;
> @@ -1663,6 +1669,8 @@ struct security_hook_heads security_hook_heads = {
>  		LIST_HEAD_INIT(security_hook_heads.inode_listsecurity),
>  	.inode_getsecid =
>  		LIST_HEAD_INIT(security_hook_heads.inode_getsecid),
> +	.inode_copy_up =
> +		LIST_HEAD_INIT(security_hook_heads.inode_copy_up),
>  	.file_permission =
>  		LIST_HEAD_INIT(security_hook_heads.file_permission),
>  	.file_alloc_security =
> 

Re: [RFC PATCH 0/9][V3] Overlayfs SELinux Support

[Paul Moore]

On Wed, Jul 13, 2016 at 10:44 AM, Vivek Goyal <vgoyal@redhat.com> wrote:
> Hi All,
>
> Please find attached the V3 of patches. Changes since V2 are as follows.
>
> - Fixed the build issue with CONFIG_SECURITY=n.
>
> - Dan Walsh was writing more tests for selinux-testsuite and noted couple
>   of issues. I have fixed those issues and added two more patches in series.
>
>   1. We are resetting MAY_WRITE check for lower inode assuming file will
>      be coiped up. But this is not true for special_file() as these files
>      are not copied up. So checks should not be reset in case of special
>      file.
>
>   2. We are resetting MAY_WRITE check for lower inode assuming file will
>      be copied up. But this also should mean that mounter has permission
>      to MAY_READ lower file for copy up to succeed. So add MAY_READ
>      check while resetting MAY_WRITE.
>
> Original description of patches follows.
>
> Following are RFC patches to support SELinux with overlayfs. I started
> with David Howells's latest posting on this topic and started modifying
> patches. These patches apply on top of overlayfs-next branch of miklos
> vfs git tree.
>
> git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git overlayfs-next
>
> These patches can be pulled from my branch too.
>
> https://github.com/rhvgoyal/linux/commits/overlayfs-selinux-mounter-next
>
> Thanks to Dan Walsh, Stephen Smalley and Miklos Szeredi for numerous
> conversation and ideas in helping figuring out what one reasonable
> implementation might look like.
>
> Dan Walsh has been writing tests for selinux overlayfs in selinux-testsuite.
> These patches pass those tests now
>
> https://github.com/rhatdan/selinux-testsuite/commits/master
>
> Posting these patches for review and comments.
>
> These patches introduce 3 new security hooks.
>
> - security_inode_copy_up(), is called when a file is copied up. This hook
>   prepares a new set of cred which is used for copy up operation. And
>   new set of creds are prepared so that ->create_sid can be set appropriately
>   and newly created file is labeled properly.
>
>   When a file is copied up, label of lower file is retained except for the
>   case of context= mount where new file gets the label from context= option.
>
> - security_inode_copy_up_xattr(), is called when xattrs of a file are
>   being copied up. Before this we already called security_inode_copy_up()
>   and created new file and copied up data. That means file already got
>   labeled properly and there is no need to take SELINUX xattr of lower
>   file and overwrite the upper file xattr. So this hook is used to avoid
>   copying up of SELINUX xattr.
>
> - dentry_create_files_as(), is called when a new file is about to be created.
>   This hook determines what the label of the file should be if task had
>   created that file in upper/ and sets create_sid accordingly in the passed
>   in creds.
>
>   Normal transition rules don't work for the case of context mounts as
>   underlying file system is not aware of context option which only overlay
>   layer is aware of. For non-context mounts, creation can happen in work/
>   dir first and then file might be renamed into upper/, and it might get
>   label based on work/ dir. So this hooks helps avoiding all these issues.
>
>   When a new file is created in upper/, it gets its label based on transition
>   rules. For the case of context mount, it gets the label from context=
>   option.
>
> Any feedback is welcome.

Hi Vivek,

These patches look fine to me, thanks for all your hard work and to
everyone who helped review and provide feedback.  I have tagged these
patches for merging into the SELinux next branch after this merge
window.

Miklos, this patchset depends on patches in your overlayfs-next
branch, I assume you're pushing that branch during the upcoming merge
window?

James, assuming the overlayfs-next branch is pulled during the merge
window, can you rebase your linux-security#next branch to v4.8-rc1
once Linus tags it?

-- 
paul moore
www.paul-moore.com

Re: [RFC PATCH 0/9][V3] Overlayfs SELinux Support

[James Morris]

On Thu, 21 Jul 2016, Paul Moore wrote:

> James, assuming the overlayfs-next branch is pulled during the merge
> window, can you rebase your linux-security#next branch to v4.8-rc1
> once Linus tags it?

Yep.

-- 
James Morris
<jmorris@namei.org>

Re: [RFC PATCH 0/9][V3] Overlayfs SELinux Support

[Miklos Szeredi]

On Thu, Jul 21, 2016 at 11:16 PM, Paul Moore <paul@paul-moore.com> wrote:

> Hi Vivek,
>
> These patches look fine to me, thanks for all your hard work and to
> everyone who helped review and provide feedback.  I have tagged these
> patches for merging into the SELinux next branch after this merge
> window.
>
> Miklos, this patchset depends on patches in your overlayfs-next
> branch, I assume you're pushing that branch during the upcoming merge
> window?

Yes, that's the plan.

Thanks,
Miklos

Re: [RFC PATCH 0/9][V3] Overlayfs SELinux Support

[Paul Moore]

On Thu, Jul 21, 2016 at 7:09 PM, James Morris <jmorris@namei.org> wrote:
> On Thu, 21 Jul 2016, Paul Moore wrote:
>> James, assuming the overlayfs-next branch is pulled during the merge
>> window, can you rebase your linux-security#next branch to v4.8-rc1
>> once Linus tags it?
>
> Yep.

On Fri, Jul 22, 2016 at 3:05 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> On Thu, Jul 21, 2016 at 11:16 PM, Paul Moore <paul@paul-moore.com> wrote:
>> Hi Vivek,
>>
>> These patches look fine to me, thanks for all your hard work and to
>> everyone who helped review and provide feedback.  I have tagged these
>> patches for merging into the SELinux next branch after this merge
>> window.
>>
>> Miklos, this patchset depends on patches in your overlayfs-next
>> branch, I assume you're pushing that branch during the upcoming merge
>> window?
>
> Yes, that's the plan.

Great, thanks.

-- 
paul moore
www.paul-moore.com

Re: [RFC PATCH 0/9][V3] Overlayfs SELinux Support

[Miklos Szeredi]

On Fri, Jul 22, 2016 at 5:33 PM, Paul Moore <paul@paul-moore.com> wrote:
> On Thu, Jul 21, 2016 at 7:09 PM, James Morris <jmorris@namei.org> wrote:
>> On Thu, 21 Jul 2016, Paul Moore wrote:
>>> James, assuming the overlayfs-next branch is pulled during the merge
>>> window, can you rebase your linux-security#next branch to v4.8-rc1
>>> once Linus tags it?
>>
>> Yep.
>
> On Fri, Jul 22, 2016 at 3:05 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>> On Thu, Jul 21, 2016 at 11:16 PM, Paul Moore <paul@paul-moore.com> wrote:
>>> Hi Vivek,
>>>
>>> These patches look fine to me, thanks for all your hard work and to
>>> everyone who helped review and provide feedback.  I have tagged these
>>> patches for merging into the SELinux next branch after this merge
>>> window.
>>>
>>> Miklos, this patchset depends on patches in your overlayfs-next
>>> branch, I assume you're pushing that branch during the upcoming merge
>>> window?
>>
>> Yes, that's the plan.
>
> Great, thanks.

Base patches are now in v4.8-rc1.

Thanks,
Miklos

Re: [RFC PATCH 0/9][V3] Overlayfs SELinux Support

[Paul Moore]

On Mon, Aug 8, 2016 at 8:46 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> On Fri, Jul 22, 2016 at 5:33 PM, Paul Moore <paul@paul-moore.com> wrote:
>> On Thu, Jul 21, 2016 at 7:09 PM, James Morris <jmorris@namei.org> wrote:
>>> On Thu, 21 Jul 2016, Paul Moore wrote:
>>>> James, assuming the overlayfs-next branch is pulled during the merge
>>>> window, can you rebase your linux-security#next branch to v4.8-rc1
>>>> once Linus tags it?
>>>
>>> Yep.
>>
>> On Fri, Jul 22, 2016 at 3:05 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>>> On Thu, Jul 21, 2016 at 11:16 PM, Paul Moore <paul@paul-moore.com> wrote:
>>>> Hi Vivek,
>>>>
>>>> These patches look fine to me, thanks for all your hard work and to
>>>> everyone who helped review and provide feedback.  I have tagged these
>>>> patches for merging into the SELinux next branch after this merge
>>>> window.
>>>>
>>>> Miklos, this patchset depends on patches in your overlayfs-next
>>>> branch, I assume you're pushing that branch during the upcoming merge
>>>> window?
>>>
>>> Yes, that's the plan.
>>
>> Great, thanks.
>
> Base patches are now in v4.8-rc1.

Thanks, I've already pinged James to rebase the linux-security tree,
once he does I'll merge the SELinux/overlayfs patches into the SELinux
next branch.

-- 
paul moore
www.paul-moore.com

Re: [RFC PATCH 0/9][V3] Overlayfs SELinux Support

[Paul Moore]

On Thu, Jul 21, 2016 at 5:16 PM, Paul Moore <paul@paul-moore.com> wrote:
> On Wed, Jul 13, 2016 at 10:44 AM, Vivek Goyal <vgoyal@redhat.com> wrote:
>> Hi All,
>>
>> Please find attached the V3 of patches. Changes since V2 are as follows.
>>
>> - Fixed the build issue with CONFIG_SECURITY=n.
>>
>> - Dan Walsh was writing more tests for selinux-testsuite and noted couple
>>   of issues. I have fixed those issues and added two more patches in series.
>>
>>   1. We are resetting MAY_WRITE check for lower inode assuming file will
>>      be coiped up. But this is not true for special_file() as these files
>>      are not copied up. So checks should not be reset in case of special
>>      file.
>>
>>   2. We are resetting MAY_WRITE check for lower inode assuming file will
>>      be copied up. But this also should mean that mounter has permission
>>      to MAY_READ lower file for copy up to succeed. So add MAY_READ
>>      check while resetting MAY_WRITE.
>>
>> Original description of patches follows.
>>
>> Following are RFC patches to support SELinux with overlayfs. I started
>> with David Howells's latest posting on this topic and started modifying
>> patches. These patches apply on top of overlayfs-next branch of miklos
>> vfs git tree.
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git overlayfs-next
>>
>> These patches can be pulled from my branch too.
>>
>> https://github.com/rhvgoyal/linux/commits/overlayfs-selinux-mounter-next
>>
>> Thanks to Dan Walsh, Stephen Smalley and Miklos Szeredi for numerous
>> conversation and ideas in helping figuring out what one reasonable
>> implementation might look like.
>>
>> Dan Walsh has been writing tests for selinux overlayfs in selinux-testsuite.
>> These patches pass those tests now
>>
>> https://github.com/rhatdan/selinux-testsuite/commits/master
>>
>> Posting these patches for review and comments.
>>
>> These patches introduce 3 new security hooks.
>>
>> - security_inode_copy_up(), is called when a file is copied up. This hook
>>   prepares a new set of cred which is used for copy up operation. And
>>   new set of creds are prepared so that ->create_sid can be set appropriately
>>   and newly created file is labeled properly.
>>
>>   When a file is copied up, label of lower file is retained except for the
>>   case of context= mount where new file gets the label from context= option.
>>
>> - security_inode_copy_up_xattr(), is called when xattrs of a file are
>>   being copied up. Before this we already called security_inode_copy_up()
>>   and created new file and copied up data. That means file already got
>>   labeled properly and there is no need to take SELINUX xattr of lower
>>   file and overwrite the upper file xattr. So this hook is used to avoid
>>   copying up of SELINUX xattr.
>>
>> - dentry_create_files_as(), is called when a new file is about to be created.
>>   This hook determines what the label of the file should be if task had
>>   created that file in upper/ and sets create_sid accordingly in the passed
>>   in creds.
>>
>>   Normal transition rules don't work for the case of context mounts as
>>   underlying file system is not aware of context option which only overlay
>>   layer is aware of. For non-context mounts, creation can happen in work/
>>   dir first and then file might be renamed into upper/, and it might get
>>   label based on work/ dir. So this hooks helps avoiding all these issues.
>>
>>   When a new file is created in upper/, it gets its label based on transition
>>   rules. For the case of context mount, it gets the label from context=
>>   option.
>>
>> Any feedback is welcome.
>
> Hi Vivek,
>
> These patches look fine to me, thanks for all your hard work and to
> everyone who helped review and provide feedback.  I have tagged these
> patches for merging into the SELinux next branch after this merge
> window.

Okay, I just merged these patches into selinux#next.  With the
exception of some changes to restore the mode argument to
ovl_create_or_link() and to fix some whitespace damage the patches
were merged cleanly.

-- 
paul moore
www.paul-moore.com

Re: [RFC PATCH 0/9][V3] Overlayfs SELinux Support

[Miklos Szeredi]

On Tue, Aug 9, 2016 at 3:19 AM, Paul Moore <paul@paul-moore.com> wrote:
> On Thu, Jul 21, 2016 at 5:16 PM, Paul Moore <paul@paul-moore.com> wrote:
>> On Wed, Jul 13, 2016 at 10:44 AM, Vivek Goyal <vgoyal@redhat.com> wrote:
>>> Hi All,
>>>
>>> Please find attached the V3 of patches. Changes since V2 are as follows.
>>>
>>> - Fixed the build issue with CONFIG_SECURITY=n.
>>>
>>> - Dan Walsh was writing more tests for selinux-testsuite and noted couple
>>>   of issues. I have fixed those issues and added two more patches in series.
>>>
>>>   1. We are resetting MAY_WRITE check for lower inode assuming file will
>>>      be coiped up. But this is not true for special_file() as these files
>>>      are not copied up. So checks should not be reset in case of special
>>>      file.
>>>
>>>   2. We are resetting MAY_WRITE check for lower inode assuming file will
>>>      be copied up. But this also should mean that mounter has permission
>>>      to MAY_READ lower file for copy up to succeed. So add MAY_READ
>>>      check while resetting MAY_WRITE.
>>>
>>> Original description of patches follows.
>>>
>>> Following are RFC patches to support SELinux with overlayfs. I started
>>> with David Howells's latest posting on this topic and started modifying
>>> patches. These patches apply on top of overlayfs-next branch of miklos
>>> vfs git tree.
>>>
>>> git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git overlayfs-next
>>>
>>> These patches can be pulled from my branch too.
>>>
>>> https://github.com/rhvgoyal/linux/commits/overlayfs-selinux-mounter-next
>>>
>>> Thanks to Dan Walsh, Stephen Smalley and Miklos Szeredi for numerous
>>> conversation and ideas in helping figuring out what one reasonable
>>> implementation might look like.
>>>
>>> Dan Walsh has been writing tests for selinux overlayfs in selinux-testsuite.
>>> These patches pass those tests now
>>>
>>> https://github.com/rhatdan/selinux-testsuite/commits/master
>>>
>>> Posting these patches for review and comments.
>>>
>>> These patches introduce 3 new security hooks.
>>>
>>> - security_inode_copy_up(), is called when a file is copied up. This hook
>>>   prepares a new set of cred which is used for copy up operation. And
>>>   new set of creds are prepared so that ->create_sid can be set appropriately
>>>   and newly created file is labeled properly.
>>>
>>>   When a file is copied up, label of lower file is retained except for the
>>>   case of context= mount where new file gets the label from context= option.
>>>
>>> - security_inode_copy_up_xattr(), is called when xattrs of a file are
>>>   being copied up. Before this we already called security_inode_copy_up()
>>>   and created new file and copied up data. That means file already got
>>>   labeled properly and there is no need to take SELINUX xattr of lower
>>>   file and overwrite the upper file xattr. So this hook is used to avoid
>>>   copying up of SELINUX xattr.
>>>
>>> - dentry_create_files_as(), is called when a new file is about to be created.
>>>   This hook determines what the label of the file should be if task had
>>>   created that file in upper/ and sets create_sid accordingly in the passed
>>>   in creds.
>>>
>>>   Normal transition rules don't work for the case of context mounts as
>>>   underlying file system is not aware of context option which only overlay
>>>   layer is aware of. For non-context mounts, creation can happen in work/
>>>   dir first and then file might be renamed into upper/, and it might get
>>>   label based on work/ dir. So this hooks helps avoiding all these issues.
>>>
>>>   When a new file is created in upper/, it gets its label based on transition
>>>   rules. For the case of context mount, it gets the label from context=
>>>   option.
>>>
>>> Any feedback is welcome.
>>
>> Hi Vivek,
>>
>> These patches look fine to me, thanks for all your hard work and to
>> everyone who helped review and provide feedback.  I have tagged these
>> patches for merging into the SELinux next branch after this merge
>> window.
>
> Okay, I just merged these patches into selinux#next.  With the
> exception of some changes to restore the mode argument to
> ovl_create_or_link() and to fix some whitespace damage the patches
> were merged cleanly.

Don't need to add the back the mode argument, just use stat->mode.

Thanks,
Miklos

Re: [RFC PATCH 0/9][V3] Overlayfs SELinux Support

[Paul Moore]

On Wed, Aug 10, 2016 at 5:11 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> On Tue, Aug 9, 2016 at 3:19 AM, Paul Moore <paul@paul-moore.com> wrote:
>> Okay, I just merged these patches into selinux#next.  With the
>> exception of some changes to restore the mode argument to
>> ovl_create_or_link() and to fix some whitespace damage the patches
>> were merged cleanly.
>
> Don't need to add the back the mode argument, just use stat->mode.

Thanks for the pointer (I'm on vacation at the moment and trying to do
this quickly).  Since it was a merge issue, and the branch hasn't been
pulled by Linus, I didn't bother with a new patch, I simply updated
the existing patch from Vivek and re-pushed to selinux#next.  If you
see anything else, please let me know.

For Fedora folks, I'm currently rebuilding the COPR
pcmoore/kernel-secnext kernel packages with this update; assuming
there are no problems with the COPR infrastructure the kernel should
be ready in a couple of hours.

* https://copr.fedorainfracloud.org/coprs/pcmoore/kernel-secnext

-- 
paul moore
www.paul-moore.com

Re: [RFC PATCH 0/9][V3] Overlayfs SELinux Support

[Daniel J Walsh]

On 08/10/2016 08:32 AM, Paul Moore wrote:
> On Wed, Aug 10, 2016 at 5:11 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>> On Tue, Aug 9, 2016 at 3:19 AM, Paul Moore <paul@paul-moore.com> wrote:
>>> Okay, I just merged these patches into selinux#next.  With the
>>> exception of some changes to restore the mode argument to
>>> ovl_create_or_link() and to fix some whitespace damage the patches
>>> were merged cleanly.
>> Don't need to add the back the mode argument, just use stat->mode.
> Thanks for the pointer (I'm on vacation at the moment and trying to do
> this quickly).  Since it was a merge issue, and the branch hasn't been
> pulled by Linus, I didn't bother with a new patch, I simply updated
> the existing patch from Vivek and re-pushed to selinux#next.  If you
> see anything else, please let me know.
>
> For Fedora folks, I'm currently rebuilding the COPR
> pcmoore/kernel-secnext kernel packages with this update; assuming
> there are no problems with the COPR infrastructure the kernel should
> be ready in a couple of hours.
>
> * https://copr.fedorainfracloud.org/coprs/pcmoore/kernel-secnext
>
Cool once there is a kernel with ovlerlay fs/selinux support I will run
it on my laptop and attempt
the selinux test suite, if everything goes well I will start running
docker on overlay with selinux enforcing mode.

Re: [RFC PATCH 0/9][V3] Overlayfs SELinux Support

[Paul Moore]

On Wed, Aug 10, 2016 at 8:52 AM, Daniel J Walsh <dwalsh@redhat.com> wrote:
> On 08/10/2016 08:32 AM, Paul Moore wrote:
>> On Wed, Aug 10, 2016 at 5:11 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>>> On Tue, Aug 9, 2016 at 3:19 AM, Paul Moore <paul@paul-moore.com> wrote:
>>>> Okay, I just merged these patches into selinux#next.  With the
>>>> exception of some changes to restore the mode argument to
>>>> ovl_create_or_link() and to fix some whitespace damage the patches
>>>> were merged cleanly.
>>> Don't need to add the back the mode argument, just use stat->mode.
>> Thanks for the pointer (I'm on vacation at the moment and trying to do
>> this quickly).  Since it was a merge issue, and the branch hasn't been
>> pulled by Linus, I didn't bother with a new patch, I simply updated
>> the existing patch from Vivek and re-pushed to selinux#next.  If you
>> see anything else, please let me know.
>>
>> For Fedora folks, I'm currently rebuilding the COPR
>> pcmoore/kernel-secnext kernel packages with this update; assuming
>> there are no problems with the COPR infrastructure the kernel should
>> be ready in a couple of hours.
>>
>> * https://copr.fedorainfracloud.org/coprs/pcmoore/kernel-secnext
>>
> Cool once there is a kernel with ovlerlay fs/selinux support I will run
> it on my laptop and attempt
> the selinux test suite, if everything goes well I will start running
> docker on overlay with selinux enforcing mode.

Okay, the build finished and passes the base SELinux/audit regressions
tests (I didn't test the SELinux/overlayfs integration yet);
kernel-4.8.0-0.rc1.git1.1.1.secnext.fc26 or greater will have the
SELinux/overlayfs patches included.  The link above has instructions
on enabling the COPR repo on your system.

-- 
paul moore
www.paul-moore.com

Re: [RFC PATCH 0/9][V3] Overlayfs SELinux Support

[Daniel J Walsh]

On 08/11/2016 08:36 AM, Paul Moore wrote:
> On Wed, Aug 10, 2016 at 8:52 AM, Daniel J Walsh <dwalsh@redhat.com> wrote:
>> On 08/10/2016 08:32 AM, Paul Moore wrote:
>>> On Wed, Aug 10, 2016 at 5:11 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>>>> On Tue, Aug 9, 2016 at 3:19 AM, Paul Moore <paul@paul-moore.com> wrote:
>>>>> Okay, I just merged these patches into selinux#next.  With the
>>>>> exception of some changes to restore the mode argument to
>>>>> ovl_create_or_link() and to fix some whitespace damage the patches
>>>>> were merged cleanly.
>>>> Don't need to add the back the mode argument, just use stat->mode.
>>> Thanks for the pointer (I'm on vacation at the moment and trying to do
>>> this quickly).  Since it was a merge issue, and the branch hasn't been
>>> pulled by Linus, I didn't bother with a new patch, I simply updated
>>> the existing patch from Vivek and re-pushed to selinux#next.  If you
>>> see anything else, please let me know.
>>>
>>> For Fedora folks, I'm currently rebuilding the COPR
>>> pcmoore/kernel-secnext kernel packages with this update; assuming
>>> there are no problems with the COPR infrastructure the kernel should
>>> be ready in a couple of hours.
>>>
>>> * https://copr.fedorainfracloud.org/coprs/pcmoore/kernel-secnext
>>>
>> Cool once there is a kernel with ovlerlay fs/selinux support I will run
>> it on my laptop and attempt
>> the selinux test suite, if everything goes well I will start running
>> docker on overlay with selinux enforcing mode.
> Okay, the build finished and passes the base SELinux/audit regressions
> tests (I didn't test the SELinux/overlayfs integration yet);
> kernel-4.8.0-0.rc1.git1.1.1.secnext.fc26 or greater will have the
> SELinux/overlayfs patches included.  The link above has instructions
> on enabling the COPR repo on your system.
>
Awesome,  I will try this out in Rawhide now.

Re: [RFC PATCH 0/9][V3] Overlayfs SELinux Support

[Daniel J Walsh]

On 08/11/2016 08:36 AM, Paul Moore wrote:
> On Wed, Aug 10, 2016 at 8:52 AM, Daniel J Walsh <dwalsh@redhat.com> wrote:
>> On 08/10/2016 08:32 AM, Paul Moore wrote:
>>> On Wed, Aug 10, 2016 at 5:11 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>>>> On Tue, Aug 9, 2016 at 3:19 AM, Paul Moore <paul@paul-moore.com> wrote:
>>>>> Okay, I just merged these patches into selinux#next.  With the
>>>>> exception of some changes to restore the mode argument to
>>>>> ovl_create_or_link() and to fix some whitespace damage the patches
>>>>> were merged cleanly.
>>>> Don't need to add the back the mode argument, just use stat->mode.
>>> Thanks for the pointer (I'm on vacation at the moment and trying to do
>>> this quickly).  Since it was a merge issue, and the branch hasn't been
>>> pulled by Linus, I didn't bother with a new patch, I simply updated
>>> the existing patch from Vivek and re-pushed to selinux#next.  If you
>>> see anything else, please let me know.
>>>
>>> For Fedora folks, I'm currently rebuilding the COPR
>>> pcmoore/kernel-secnext kernel packages with this update; assuming
>>> there are no problems with the COPR infrastructure the kernel should
>>> be ready in a couple of hours.
>>>
>>> * https://copr.fedorainfracloud.org/coprs/pcmoore/kernel-secnext
>>>
>> Cool once there is a kernel with ovlerlay fs/selinux support I will run
>> it on my laptop and attempt
>> the selinux test suite, if everything goes well I will start running
>> docker on overlay with selinux enforcing mode.
> Okay, the build finished and passes the base SELinux/audit regressions
> tests (I didn't test the SELinux/overlayfs integration yet);
> kernel-4.8.0-0.rc1.git1.1.1.secnext.fc26 or greater will have the
> SELinux/overlayfs patches included.  The link above has instructions
> on enabling the COPR repo on your system.
>
Looks great.  Passes the test suite, and I have a patched version of docker
now running with overlay backend.