[RFC PATCH 0/4] ima: filesystems not mounted with i_version

[RFC PATCH 0/4] ima: filesystems not mounted with i_version

[Mimi Zohar]

IMA measures a file, verifies a file's integrity, and caches the
results.  On filesystems with MS_I_VERSION enabled, IMA can detect
file changes and cause them to be re-measured and verified.  On
filesystems without MS_I_VERSION enabled, files are measured and
verified just once.

Currently users either have to look at the source code or test 
to determine if the file system supports i_version.  Even if the
file system supports i_version, there is no guarantee that the
filesystem was actually mounted with the i_version flag.

This patch set emits warning messages when filesystems are not
mounted with i_version support.  This patch set defines two new post
LSM hooks named security_sb_post_new_mount and
security_sb_post_remount, with their corresponding IMA functions.

Questions:
- IMA can call out directly to the IMA functions, without having to
  define these LSM hooks.  Is there a need for these LSM hooks?

- do_new_mount() creates a new vfsmount. If there is a way of accessing
  this new vfsmount from the caller do_mount(), we would only need one
  new LSM hook and corresponding IMA hook.

Mimi

Mimi Zohar (4):
  security: define new LSM sb_post_new_mount hook
  ima: define new ima_sb_post_new_mount hook
  security: define a new LSM sb_post_remount hook
  ima: define a new ima_sb_post_remount hook

 fs/namespace.c                    |  5 ++++
 include/linux/ima.h               | 14 +++++++++
 include/linux/lsm_hooks.h         | 16 ++++++++++
 include/linux/security.h          | 10 +++++++
 security/integrity/ima/ima_main.c | 62 +++++++++++++++++++++++++++++++++++++++
 security/security.c               | 15 ++++++++++
 6 files changed, 122 insertions(+)

-- 
2.7.4

[RFC PATCH 0/4] ima: filesystems not mounted with i_version

[Mimi Zohar]

IMA measures a file, verifies a file's integrity, and caches the
results.  On filesystems with MS_I_VERSION enabled, IMA can detect
file changes and cause them to be re-measured and verified.  On
filesystems without MS_I_VERSION enabled, files are measured and
verified just once.

Currently users either have to look at the source code or test 
to determine if the file system supports i_version.  Even if the
file system supports i_version, there is no guarantee that the
filesystem was actually mounted with the i_version flag.

This patch set emits warning messages when filesystems are not
mounted with i_version support.  This patch set defines two new post
LSM hooks named security_sb_post_new_mount and
security_sb_post_remount, with their corresponding IMA functions.

Questions:
- IMA can call out directly to the IMA functions, without having to
  define these LSM hooks.  Is there a need for these LSM hooks?

- do_new_mount() creates a new vfsmount. If there is a way of accessing
  this new vfsmount from the caller do_mount(), we would only need one
  new LSM hook and corresponding IMA hook.

Mimi

Mimi Zohar (4):
  security: define new LSM sb_post_new_mount hook
  ima: define new ima_sb_post_new_mount hook
  security: define a new LSM sb_post_remount hook
  ima: define a new ima_sb_post_remount hook

 fs/namespace.c                    |  5 ++++
 include/linux/ima.h               | 14 +++++++++
 include/linux/lsm_hooks.h         | 16 ++++++++++
 include/linux/security.h          | 10 +++++++
 security/integrity/ima/ima_main.c | 62 +++++++++++++++++++++++++++++++++++++++
 security/security.c               | 15 ++++++++++
 6 files changed, 122 insertions(+)

-- 
2.7.4

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

[RFC PATCH 1/4] security: define new LSM sb_post_new_mount hook

[Mimi Zohar]

Different filesystems enable different flags automatically, while
others require the mount flag to be supplied as a mount option (eg.
i_version).  Although this hook is post mount, permit logging or
auditing missing flags.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 fs/namespace.c            | 2 ++
 include/linux/lsm_hooks.h | 7 +++++++
 include/linux/security.h  | 2 ++
 security/security.c       | 6 ++++++
 4 files changed, 17 insertions(+)

diff --git a/fs/namespace.c b/fs/namespace.c
index f8893dc6a989..a7fa13f422ad 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2468,6 +2468,8 @@ static int do_new_mount(struct path *path, const char *fstype, int flags,
 	err = do_add_mount(real_mount(mnt), path, mnt_flags);
 	if (err)
 		mntput(mnt);
+	else
+		security_sb_post_new_mount(mnt, path);
 	return err;
 }
 
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index ce02f76a6188..c3ecea0d0dca 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -128,6 +128,10 @@
  *	@mnt contains the mounted file system.
  *	@flags contains the unmount flags, e.g. MNT_FORCE.
  *	Return 0 if permission is granted.
+ * @sb_post_new_mount:
+ *	Check mounted options conform to expectations
+ *	@newmnt contains the newly mounted file system.
+ *	@path contains the path for mount point object.
  * @sb_pivotroot:
  *	Check permission before pivoting the root filesystem.
  *	@old_path contains the path for the new location of the
@@ -1396,6 +1400,8 @@ union security_list_options {
 	int (*sb_statfs)(struct dentry *dentry);
 	int (*sb_mount)(const char *dev_name, const struct path *path,
 			const char *type, unsigned long flags, void *data);
+	void (*sb_post_new_mount)(const struct vfsmount *newmnt,
+				  const struct path *path);
 	int (*sb_umount)(struct vfsmount *mnt, int flags);
 	int (*sb_pivotroot)(const struct path *old_path, const struct path *new_path);
 	int (*sb_set_mnt_opts)(struct super_block *sb,
@@ -1716,6 +1722,7 @@ struct security_hook_heads {
 	struct list_head sb_statfs;
 	struct list_head sb_mount;
 	struct list_head sb_umount;
+	struct list_head sb_post_new_mount;
 	struct list_head sb_pivotroot;
 	struct list_head sb_set_mnt_opts;
 	struct list_head sb_clone_mnt_opts;
diff --git a/include/linux/security.h b/include/linux/security.h
index 458e24bea2d4..4acdaae7aa04 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -242,6 +242,8 @@ int security_sb_show_options(struct seq_file *m, struct super_block *sb);
 int security_sb_statfs(struct dentry *dentry);
 int security_sb_mount(const char *dev_name, const struct path *path,
 		      const char *type, unsigned long flags, void *data);
+void security_sb_post_new_mount(const struct vfsmount *mnt,
+				const struct path *path);
 int security_sb_umount(struct vfsmount *mnt, int flags);
 int security_sb_pivotroot(const struct path *old_path, const struct path *new_path);
 int security_sb_set_mnt_opts(struct super_block *sb,
diff --git a/security/security.c b/security/security.c
index 55b5997e4b72..592153e8d2b6 100644
--- a/security/security.c
+++ b/security/security.c
@@ -398,6 +398,12 @@ int security_sb_mount(const char *dev_name, const struct path *path,
 	return call_int_hook(sb_mount, 0, dev_name, path, type, flags, data);
 }
 
+void security_sb_post_new_mount(const struct vfsmount *newmnt,
+				const struct path *path)
+{
+	call_void_hook(sb_post_new_mount, newmnt, path);
+}
+
 int security_sb_umount(struct vfsmount *mnt, int flags)
 {
 	return call_int_hook(sb_umount, 0, mnt, flags);
-- 
2.7.4

[RFC PATCH 1/4] security: define new LSM sb_post_new_mount hook

[Mimi Zohar]

Different filesystems enable different flags automatically, while
others require the mount flag to be supplied as a mount option (eg.
i_version).  Although this hook is post mount, permit logging or
auditing missing flags.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 fs/namespace.c            | 2 ++
 include/linux/lsm_hooks.h | 7 +++++++
 include/linux/security.h  | 2 ++
 security/security.c       | 6 ++++++
 4 files changed, 17 insertions(+)

diff --git a/fs/namespace.c b/fs/namespace.c
index f8893dc6a989..a7fa13f422ad 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2468,6 +2468,8 @@ static int do_new_mount(struct path *path, const char *fstype, int flags,
 	err = do_add_mount(real_mount(mnt), path, mnt_flags);
 	if (err)
 		mntput(mnt);
+	else
+		security_sb_post_new_mount(mnt, path);
 	return err;
 }
 
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index ce02f76a6188..c3ecea0d0dca 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -128,6 +128,10 @@
  *	@mnt contains the mounted file system.
  *	@flags contains the unmount flags, e.g. MNT_FORCE.
  *	Return 0 if permission is granted.
+ * @sb_post_new_mount:
+ *	Check mounted options conform to expectations
+ *	@newmnt contains the newly mounted file system.
+ *	@path contains the path for mount point object.
  * @sb_pivotroot:
  *	Check permission before pivoting the root filesystem.
  *	@old_path contains the path for the new location of the
@@ -1396,6 +1400,8 @@ union security_list_options {
 	int (*sb_statfs)(struct dentry *dentry);
 	int (*sb_mount)(const char *dev_name, const struct path *path,
 			const char *type, unsigned long flags, void *data);
+	void (*sb_post_new_mount)(const struct vfsmount *newmnt,
+				  const struct path *path);
 	int (*sb_umount)(struct vfsmount *mnt, int flags);
 	int (*sb_pivotroot)(const struct path *old_path, const struct path *new_path);
 	int (*sb_set_mnt_opts)(struct super_block *sb,
@@ -1716,6 +1722,7 @@ struct security_hook_heads {
 	struct list_head sb_statfs;
 	struct list_head sb_mount;
 	struct list_head sb_umount;
+	struct list_head sb_post_new_mount;
 	struct list_head sb_pivotroot;
 	struct list_head sb_set_mnt_opts;
 	struct list_head sb_clone_mnt_opts;
diff --git a/include/linux/security.h b/include/linux/security.h
index 458e24bea2d4..4acdaae7aa04 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -242,6 +242,8 @@ int security_sb_show_options(struct seq_file *m, struct super_block *sb);
 int security_sb_statfs(struct dentry *dentry);
 int security_sb_mount(const char *dev_name, const struct path *path,
 		      const char *type, unsigned long flags, void *data);
+void security_sb_post_new_mount(const struct vfsmount *mnt,
+				const struct path *path);
 int security_sb_umount(struct vfsmount *mnt, int flags);
 int security_sb_pivotroot(const struct path *old_path, const struct path *new_path);
 int security_sb_set_mnt_opts(struct super_block *sb,
diff --git a/security/security.c b/security/security.c
index 55b5997e4b72..592153e8d2b6 100644
--- a/security/security.c
+++ b/security/security.c
@@ -398,6 +398,12 @@ int security_sb_mount(const char *dev_name, const struct path *path,
 	return call_int_hook(sb_mount, 0, dev_name, path, type, flags, data);
 }
 
+void security_sb_post_new_mount(const struct vfsmount *newmnt,
+				const struct path *path)
+{
+	call_void_hook(sb_post_new_mount, newmnt, path);
+}
+
 int security_sb_umount(struct vfsmount *mnt, int flags)
 {
 	return call_int_hook(sb_umount, 0, mnt, flags);
-- 
2.7.4

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

[RFC PATCH 2/4] ima: define new ima_sb_post_new_mount hook

[Mimi Zohar]

IMA measures a file, verifies a file's integrity, and caches the
results.  On filesystems with MS_I_VERSION enabled, IMA can detect
file changes and cause them to be re-measured and verified.  On
filesystems without MS_I_VERSION enabled, files are measured and
verified just once.

This patch logs filesystems mounted without MS_I_VERSION.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 include/linux/ima.h               |  5 +++++
 security/integrity/ima/ima_main.c | 44 +++++++++++++++++++++++++++++++++++++++
 security/security.c               |  1 +
 3 files changed, 50 insertions(+)

diff --git a/include/linux/ima.h b/include/linux/ima.h
index 0e4647e0eb60..4475cb01149c 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -23,6 +23,8 @@ extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
 extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
 			      enum kernel_read_file_id id);
 extern void ima_post_path_mknod(struct dentry *dentry);
+extern void ima_sb_post_new_mount(const struct vfsmount *newmnt,
+				  const struct path *path);
 
 #ifdef CONFIG_IMA_KEXEC
 extern void ima_add_kexec_buffer(struct kimage *image);
@@ -65,6 +67,9 @@ static inline void ima_post_path_mknod(struct dentry *dentry)
 	return;
 }
 
+static inline void ima_sb_post_new_mount(const struct vfsmount *newmnt,
+					 const struct path *path)
+{ }
 #endif /* CONFIG_IMA */
 
 #ifndef CONFIG_IMA_KEXEC
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index b00186914df8..a0a685189001 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -354,6 +354,50 @@ void ima_post_path_mknod(struct dentry *dentry)
 }
 
 /**
+ * ima_sb_post_new_mount - check filesystem mounted flags
+ *
+ * Indicate that filesystem isn't mounted with i_version enabled.
+ */
+void ima_sb_post_new_mount(const struct vfsmount *newmnt,
+			   const struct path *path)
+{
+	struct super_block *sb;
+	unsigned long pseudo_fs[] = {CGROUP_SUPER_MAGIC, CGROUP2_SUPER_MAGIC,
+		SYSFS_MAGIC, DEVPTS_SUPER_MAGIC, PSTOREFS_MAGIC, EFIVARFS_MAGIC,
+		DEBUGFS_MAGIC, TMPFS_MAGIC};
+	char *pathbuf = NULL;
+	char filename[NAME_MAX];
+	const char *pathname;
+	bool found = 0;
+	int i;
+
+	sb = newmnt ? newmnt->mnt_sb : path->mnt->mnt_sb;
+
+	if ((sb->s_flags & MS_I_VERSION) || (sb->s_flags & MS_RDONLY) ||
+	    (sb->s_flags & MS_KERNMOUNT))
+		return;
+
+	for (i = 0; i < ARRAY_SIZE(pseudo_fs); i++) {
+		if (pseudo_fs[i] != sb->s_magic)
+			continue;
+
+		found = 1;
+		break;
+	}
+	if (found)
+		return;
+
+	pathname = ima_d_path(path, &pathbuf, filename);
+	if (!pathname)
+		return;
+
+	if (newmnt)
+		pr_warn("ima: %s mounted without i_version enabled\n",
+			pathname);
+	 __putname(pathbuf);
+}
+
+/**
  * ima_read_file - pre-measure/appraise hook decision based on policy
  * @file: pointer to the file to be measured/appraised/audit
  * @read_id: caller identifier
diff --git a/security/security.c b/security/security.c
index 592153e8d2b6..79111141b383 100644
--- a/security/security.c
+++ b/security/security.c
@@ -402,6 +402,7 @@ void security_sb_post_new_mount(const struct vfsmount *newmnt,
 				const struct path *path)
 {
 	call_void_hook(sb_post_new_mount, newmnt, path);
+	ima_sb_post_new_mount(newmnt, path);
 }
 
 int security_sb_umount(struct vfsmount *mnt, int flags)
-- 
2.7.4

[RFC PATCH 2/4] ima: define new ima_sb_post_new_mount hook

[Mimi Zohar]

IMA measures a file, verifies a file's integrity, and caches the
results.  On filesystems with MS_I_VERSION enabled, IMA can detect
file changes and cause them to be re-measured and verified.  On
filesystems without MS_I_VERSION enabled, files are measured and
verified just once.

This patch logs filesystems mounted without MS_I_VERSION.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 include/linux/ima.h               |  5 +++++
 security/integrity/ima/ima_main.c | 44 +++++++++++++++++++++++++++++++++++++++
 security/security.c               |  1 +
 3 files changed, 50 insertions(+)

diff --git a/include/linux/ima.h b/include/linux/ima.h
index 0e4647e0eb60..4475cb01149c 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -23,6 +23,8 @@ extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
 extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
 			      enum kernel_read_file_id id);
 extern void ima_post_path_mknod(struct dentry *dentry);
+extern void ima_sb_post_new_mount(const struct vfsmount *newmnt,
+				  const struct path *path);
 
 #ifdef CONFIG_IMA_KEXEC
 extern void ima_add_kexec_buffer(struct kimage *image);
@@ -65,6 +67,9 @@ static inline void ima_post_path_mknod(struct dentry *dentry)
 	return;
 }
 
+static inline void ima_sb_post_new_mount(const struct vfsmount *newmnt,
+					 const struct path *path)
+{ }
 #endif /* CONFIG_IMA */
 
 #ifndef CONFIG_IMA_KEXEC
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index b00186914df8..a0a685189001 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -354,6 +354,50 @@ void ima_post_path_mknod(struct dentry *dentry)
 }
 
 /**
+ * ima_sb_post_new_mount - check filesystem mounted flags
+ *
+ * Indicate that filesystem isn't mounted with i_version enabled.
+ */
+void ima_sb_post_new_mount(const struct vfsmount *newmnt,
+			   const struct path *path)
+{
+	struct super_block *sb;
+	unsigned long pseudo_fs[] = {CGROUP_SUPER_MAGIC, CGROUP2_SUPER_MAGIC,
+		SYSFS_MAGIC, DEVPTS_SUPER_MAGIC, PSTOREFS_MAGIC, EFIVARFS_MAGIC,
+		DEBUGFS_MAGIC, TMPFS_MAGIC};
+	char *pathbuf = NULL;
+	char filename[NAME_MAX];
+	const char *pathname;
+	bool found = 0;
+	int i;
+
+	sb = newmnt ? newmnt->mnt_sb : path->mnt->mnt_sb;
+
+	if ((sb->s_flags & MS_I_VERSION) || (sb->s_flags & MS_RDONLY) ||
+	    (sb->s_flags & MS_KERNMOUNT))
+		return;
+
+	for (i = 0; i < ARRAY_SIZE(pseudo_fs); i++) {
+		if (pseudo_fs[i] != sb->s_magic)
+			continue;
+
+		found = 1;
+		break;
+	}
+	if (found)
+		return;
+
+	pathname = ima_d_path(path, &pathbuf, filename);
+	if (!pathname)
+		return;
+
+	if (newmnt)
+		pr_warn("ima: %s mounted without i_version enabled\n",
+			pathname);
+	 __putname(pathbuf);
+}
+
+/**
  * ima_read_file - pre-measure/appraise hook decision based on policy
  * @file: pointer to the file to be measured/appraised/audit
  * @read_id: caller identifier
diff --git a/security/security.c b/security/security.c
index 592153e8d2b6..79111141b383 100644
--- a/security/security.c
+++ b/security/security.c
@@ -402,6 +402,7 @@ void security_sb_post_new_mount(const struct vfsmount *newmnt,
 				const struct path *path)
 {
 	call_void_hook(sb_post_new_mount, newmnt, path);
+	ima_sb_post_new_mount(newmnt, path);
 }
 
 int security_sb_umount(struct vfsmount *mnt, int flags)
-- 
2.7.4

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

[RFC PATCH 3/4] security: define a new LSM sb_post_remount hook

[Mimi Zohar]

Allow LSMs to compare the previous and new mount flags. This patch
defines a new LSM hook named security_sb_post_remount.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 fs/namespace.c            | 3 +++
 include/linux/lsm_hooks.h | 9 +++++++++
 include/linux/security.h  | 8 ++++++++
 security/security.c       | 7 +++++++
 4 files changed, 27 insertions(+)

diff --git a/fs/namespace.c b/fs/namespace.c
index a7fa13f422ad..e6f4ea9a1008 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2228,6 +2228,7 @@ static int do_remount(struct path *path, int flags, int mnt_flags,
 	int err;
 	struct super_block *sb = path->mnt->mnt_sb;
 	struct mount *mnt = real_mount(path->mnt);
+	unsigned long prev_sb_flags = sb->s_flags;
 
 	if (!check_mnt(mnt))
 		return -EINVAL;
@@ -2279,6 +2280,8 @@ static int do_remount(struct path *path, int flags, int mnt_flags,
 		mnt->mnt.mnt_flags = mnt_flags;
 		touch_mnt_namespace(mnt->mnt_ns);
 		unlock_mount_hash();
+
+		security_sb_post_remount(sb, prev_sb_flags, path);
 	}
 	up_write(&sb->s_umount);
 	return err;
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index c3ecea0d0dca..e940dc43786c 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -123,6 +123,11 @@
  *	@sb superblock being remounted
  *	@data contains the filesystem-specific data.
  *	Return 0 if permission is granted.
+ * @sb_post_remount:
+ *	Allow comparison between new and previous mount flags.
+ *	@sb superblock being remounted
+ *	@prev_sb_flags previous mount flags
+ *	@path contains the path for mount point object.
  * @sb_umount:
  *	Check permission before the @mnt file system is unmounted.
  *	@mnt contains the mounted file system.
@@ -1395,6 +1400,9 @@ union security_list_options {
 	void (*sb_free_security)(struct super_block *sb);
 	int (*sb_copy_data)(char *orig, char *copy);
 	int (*sb_remount)(struct super_block *sb, void *data);
+	void (*sb_post_remount)(const struct super_block *sb,
+				const unsigned long prev_sb_flags,
+				const struct path *path);
 	int (*sb_kern_mount)(struct super_block *sb, int flags, void *data);
 	int (*sb_show_options)(struct seq_file *m, struct super_block *sb);
 	int (*sb_statfs)(struct dentry *dentry);
@@ -1717,6 +1725,7 @@ struct security_hook_heads {
 	struct list_head sb_free_security;
 	struct list_head sb_copy_data;
 	struct list_head sb_remount;
+	struct list_head sb_post_remount;
 	struct list_head sb_kern_mount;
 	struct list_head sb_show_options;
 	struct list_head sb_statfs;
diff --git a/include/linux/security.h b/include/linux/security.h
index 4acdaae7aa04..fa963a870a84 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -237,6 +237,9 @@ int security_sb_alloc(struct super_block *sb);
 void security_sb_free(struct super_block *sb);
 int security_sb_copy_data(char *orig, char *copy);
 int security_sb_remount(struct super_block *sb, void *data);
+void security_sb_post_remount(const struct super_block *sb,
+			      unsigned long prev_sb_flags,
+			      const struct path *path);
 int security_sb_kern_mount(struct super_block *sb, int flags, void *data);
 int security_sb_show_options(struct seq_file *m, struct super_block *sb);
 int security_sb_statfs(struct dentry *dentry);
@@ -565,6 +568,11 @@ static inline int security_sb_remount(struct super_block *sb, void *data)
 	return 0;
 }
 
+static inline void security_sb_post_remount(const struct super_block *sb,
+					    unsigned long prev_sb_flags,
+					    const struct path *path))
+{ }
+
 static inline int security_sb_kern_mount(struct super_block *sb, int flags, void *data)
 {
 	return 0;
diff --git a/security/security.c b/security/security.c
index 79111141b383..7981ad9019c9 100644
--- a/security/security.c
+++ b/security/security.c
@@ -377,6 +377,13 @@ int security_sb_remount(struct super_block *sb, void *data)
 	return call_int_hook(sb_remount, 0, sb, data);
 }
 
+void security_sb_post_remount(const struct super_block *sb,
+			      unsigned long prev_sb_flags,
+			      const struct path *path)
+{
+	call_void_hook(sb_post_remount, sb, prev_sb_flags, path);
+}
+
 int security_sb_kern_mount(struct super_block *sb, int flags, void *data)
 {
 	return call_int_hook(sb_kern_mount, 0, sb, flags, data);
-- 
2.7.4

[RFC PATCH 3/4] security: define a new LSM sb_post_remount hook

[Mimi Zohar]

Allow LSMs to compare the previous and new mount flags. This patch
defines a new LSM hook named security_sb_post_remount.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 fs/namespace.c            | 3 +++
 include/linux/lsm_hooks.h | 9 +++++++++
 include/linux/security.h  | 8 ++++++++
 security/security.c       | 7 +++++++
 4 files changed, 27 insertions(+)

diff --git a/fs/namespace.c b/fs/namespace.c
index a7fa13f422ad..e6f4ea9a1008 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2228,6 +2228,7 @@ static int do_remount(struct path *path, int flags, int mnt_flags,
 	int err;
 	struct super_block *sb = path->mnt->mnt_sb;
 	struct mount *mnt = real_mount(path->mnt);
+	unsigned long prev_sb_flags = sb->s_flags;
 
 	if (!check_mnt(mnt))
 		return -EINVAL;
@@ -2279,6 +2280,8 @@ static int do_remount(struct path *path, int flags, int mnt_flags,
 		mnt->mnt.mnt_flags = mnt_flags;
 		touch_mnt_namespace(mnt->mnt_ns);
 		unlock_mount_hash();
+
+		security_sb_post_remount(sb, prev_sb_flags, path);
 	}
 	up_write(&sb->s_umount);
 	return err;
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index c3ecea0d0dca..e940dc43786c 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -123,6 +123,11 @@
  *	@sb superblock being remounted
  *	@data contains the filesystem-specific data.
  *	Return 0 if permission is granted.
+ * @sb_post_remount:
+ *	Allow comparison between new and previous mount flags.
+ *	@sb superblock being remounted
+ *	@prev_sb_flags previous mount flags
+ *	@path contains the path for mount point object.
  * @sb_umount:
  *	Check permission before the @mnt file system is unmounted.
  *	@mnt contains the mounted file system.
@@ -1395,6 +1400,9 @@ union security_list_options {
 	void (*sb_free_security)(struct super_block *sb);
 	int (*sb_copy_data)(char *orig, char *copy);
 	int (*sb_remount)(struct super_block *sb, void *data);
+	void (*sb_post_remount)(const struct super_block *sb,
+				const unsigned long prev_sb_flags,
+				const struct path *path);
 	int (*sb_kern_mount)(struct super_block *sb, int flags, void *data);
 	int (*sb_show_options)(struct seq_file *m, struct super_block *sb);
 	int (*sb_statfs)(struct dentry *dentry);
@@ -1717,6 +1725,7 @@ struct security_hook_heads {
 	struct list_head sb_free_security;
 	struct list_head sb_copy_data;
 	struct list_head sb_remount;
+	struct list_head sb_post_remount;
 	struct list_head sb_kern_mount;
 	struct list_head sb_show_options;
 	struct list_head sb_statfs;
diff --git a/include/linux/security.h b/include/linux/security.h
index 4acdaae7aa04..fa963a870a84 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -237,6 +237,9 @@ int security_sb_alloc(struct super_block *sb);
 void security_sb_free(struct super_block *sb);
 int security_sb_copy_data(char *orig, char *copy);
 int security_sb_remount(struct super_block *sb, void *data);
+void security_sb_post_remount(const struct super_block *sb,
+			      unsigned long prev_sb_flags,
+			      const struct path *path);
 int security_sb_kern_mount(struct super_block *sb, int flags, void *data);
 int security_sb_show_options(struct seq_file *m, struct super_block *sb);
 int security_sb_statfs(struct dentry *dentry);
@@ -565,6 +568,11 @@ static inline int security_sb_remount(struct super_block *sb, void *data)
 	return 0;
 }
 
+static inline void security_sb_post_remount(const struct super_block *sb,
+					    unsigned long prev_sb_flags,
+					    const struct path *path))
+{ }
+
 static inline int security_sb_kern_mount(struct super_block *sb, int flags, void *data)
 {
 	return 0;
diff --git a/security/security.c b/security/security.c
index 79111141b383..7981ad9019c9 100644
--- a/security/security.c
+++ b/security/security.c
@@ -377,6 +377,13 @@ int security_sb_remount(struct super_block *sb, void *data)
 	return call_int_hook(sb_remount, 0, sb, data);
 }
 
+void security_sb_post_remount(const struct super_block *sb,
+			      unsigned long prev_sb_flags,
+			      const struct path *path)
+{
+	call_void_hook(sb_post_remount, sb, prev_sb_flags, path);
+}
+
 int security_sb_kern_mount(struct super_block *sb, int flags, void *data)
 {
 	return call_int_hook(sb_kern_mount, 0, sb, flags, data);
-- 
2.7.4

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

[RFC PATCH 4/4] ima: define a new ima_sb_post_remount hook

[Mimi Zohar]

Compare the previous i_version flag with the remounted i_version
flag.  Only if there is a change, log change message.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 include/linux/ima.h               |  9 +++++++++
 security/integrity/ima/ima_main.c | 22 ++++++++++++++++++++--
 security/security.c               |  1 +
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/include/linux/ima.h b/include/linux/ima.h
index 4475cb01149c..bd98221c00d5 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -25,6 +25,9 @@ extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
 extern void ima_post_path_mknod(struct dentry *dentry);
 extern void ima_sb_post_new_mount(const struct vfsmount *newmnt,
 				  const struct path *path);
+extern  void ima_sb_post_remount(const struct super_block *sb,
+				 unsigned long prev_sb_flags,
+				 const struct path *path);
 
 #ifdef CONFIG_IMA_KEXEC
 extern void ima_add_kexec_buffer(struct kimage *image);
@@ -70,6 +73,12 @@ static inline void ima_post_path_mknod(struct dentry *dentry)
 static inline void ima_sb_post_new_mount(const struct vfsmount *newmnt,
 					 const struct path *path)
 { }
+
+static inline void ima_sb_post_remount(const struct super_block *sb,
+				       unsigned long prev_sb_flags,
+				       const struct path *path)
+{ }
+
 #endif /* CONFIG_IMA */
 
 #ifndef CONFIG_IMA_KEXEC
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index a0a685189001..1b180f974e8d 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -373,8 +373,10 @@ void ima_sb_post_new_mount(const struct vfsmount *newmnt,
 
 	sb = newmnt ? newmnt->mnt_sb : path->mnt->mnt_sb;
 
-	if ((sb->s_flags & MS_I_VERSION) || (sb->s_flags & MS_RDONLY) ||
-	    (sb->s_flags & MS_KERNMOUNT))
+	if ((sb->s_flags & MS_KERNMOUNT) || (sb->s_flags & MS_RDONLY))
+		return;
+
+	if (newmnt && (sb->s_flags & MS_I_VERSION))
 		return;
 
 	for (i = 0; i < ARRAY_SIZE(pseudo_fs); i++) {
@@ -394,9 +396,25 @@ void ima_sb_post_new_mount(const struct vfsmount *newmnt,
 	if (newmnt)
 		pr_warn("ima: %s mounted without i_version enabled\n",
 			pathname);
+	else if (sb->s_flags & MS_I_VERSION)
+		pr_warn("ima: %s re-mounted with i_version enabled\n",
+			pathname);
+	else
+		pr_warn("ima: %s re-mounted without i_version enabled\n",
+			pathname);
 	 __putname(pathbuf);
 }
 
+void ima_sb_post_remount(const struct super_block *sb,
+			 unsigned long prev_sb_flags,
+			 const struct path *path)
+{
+	if ((sb->s_flags & MS_I_VERSION) == (prev_sb_flags & MS_I_VERSION))
+		return;		/* nothing changed */
+
+	ima_sb_post_new_mount(NULL, path);
+}
+
 /**
  * ima_read_file - pre-measure/appraise hook decision based on policy
  * @file: pointer to the file to be measured/appraised/audit
diff --git a/security/security.c b/security/security.c
index 7981ad9019c9..1e2051a52b9f 100644
--- a/security/security.c
+++ b/security/security.c
@@ -382,6 +382,7 @@ void security_sb_post_remount(const struct super_block *sb,
 			      const struct path *path)
 {
 	call_void_hook(sb_post_remount, sb, prev_sb_flags, path);
+	ima_sb_post_remount(sb, prev_sb_flags, path);
 }
 
 int security_sb_kern_mount(struct super_block *sb, int flags, void *data)
-- 
2.7.4

[RFC PATCH 4/4] ima: define a new ima_sb_post_remount hook

[Mimi Zohar]

Compare the previous i_version flag with the remounted i_version
flag.  Only if there is a change, log change message.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 include/linux/ima.h               |  9 +++++++++
 security/integrity/ima/ima_main.c | 22 ++++++++++++++++++++--
 security/security.c               |  1 +
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/include/linux/ima.h b/include/linux/ima.h
index 4475cb01149c..bd98221c00d5 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -25,6 +25,9 @@ extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
 extern void ima_post_path_mknod(struct dentry *dentry);
 extern void ima_sb_post_new_mount(const struct vfsmount *newmnt,
 				  const struct path *path);
+extern  void ima_sb_post_remount(const struct super_block *sb,
+				 unsigned long prev_sb_flags,
+				 const struct path *path);
 
 #ifdef CONFIG_IMA_KEXEC
 extern void ima_add_kexec_buffer(struct kimage *image);
@@ -70,6 +73,12 @@ static inline void ima_post_path_mknod(struct dentry *dentry)
 static inline void ima_sb_post_new_mount(const struct vfsmount *newmnt,
 					 const struct path *path)
 { }
+
+static inline void ima_sb_post_remount(const struct super_block *sb,
+				       unsigned long prev_sb_flags,
+				       const struct path *path)
+{ }
+
 #endif /* CONFIG_IMA */
 
 #ifndef CONFIG_IMA_KEXEC
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index a0a685189001..1b180f974e8d 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -373,8 +373,10 @@ void ima_sb_post_new_mount(const struct vfsmount *newmnt,
 
 	sb = newmnt ? newmnt->mnt_sb : path->mnt->mnt_sb;
 
-	if ((sb->s_flags & MS_I_VERSION) || (sb->s_flags & MS_RDONLY) ||
-	    (sb->s_flags & MS_KERNMOUNT))
+	if ((sb->s_flags & MS_KERNMOUNT) || (sb->s_flags & MS_RDONLY))
+		return;
+
+	if (newmnt && (sb->s_flags & MS_I_VERSION))
 		return;
 
 	for (i = 0; i < ARRAY_SIZE(pseudo_fs); i++) {
@@ -394,9 +396,25 @@ void ima_sb_post_new_mount(const struct vfsmount *newmnt,
 	if (newmnt)
 		pr_warn("ima: %s mounted without i_version enabled\n",
 			pathname);
+	else if (sb->s_flags & MS_I_VERSION)
+		pr_warn("ima: %s re-mounted with i_version enabled\n",
+			pathname);
+	else
+		pr_warn("ima: %s re-mounted without i_version enabled\n",
+			pathname);
 	 __putname(pathbuf);
 }
 
+void ima_sb_post_remount(const struct super_block *sb,
+			 unsigned long prev_sb_flags,
+			 const struct path *path)
+{
+	if ((sb->s_flags & MS_I_VERSION) == (prev_sb_flags & MS_I_VERSION))
+		return;		/* nothing changed */
+
+	ima_sb_post_new_mount(NULL, path);
+}
+
 /**
  * ima_read_file - pre-measure/appraise hook decision based on policy
  * @file: pointer to the file to be measured/appraised/audit
diff --git a/security/security.c b/security/security.c
index 7981ad9019c9..1e2051a52b9f 100644
--- a/security/security.c
+++ b/security/security.c
@@ -382,6 +382,7 @@ void security_sb_post_remount(const struct super_block *sb,
 			      const struct path *path)
 {
 	call_void_hook(sb_post_remount, sb, prev_sb_flags, path);
+	ima_sb_post_remount(sb, prev_sb_flags, path);
 }
 
 int security_sb_kern_mount(struct super_block *sb, int flags, void *data)
-- 
2.7.4

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

Re: [RFC PATCH 2/4] ima: define new ima_sb_post_new_mount hook

[Casey Schaufler]

On 8/16/2017 10:30 AM, Mimi Zohar wrote:
> IMA measures a file, verifies a file's integrity, and caches the
> results.  On filesystems with MS_I_VERSION enabled, IMA can detect
> file changes and cause them to be re-measured and verified.  On
> filesystems without MS_I_VERSION enabled, files are measured and
> verified just once.
>
> This patch logs filesystems mounted without MS_I_VERSION.
>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> ---
>  include/linux/ima.h               |  5 +++++
>  security/integrity/ima/ima_main.c | 44 +++++++++++++++++++++++++++++++++++++++
>  security/security.c               |  1 +
>  3 files changed, 50 insertions(+)
>
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 0e4647e0eb60..4475cb01149c 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -23,6 +23,8 @@ extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
>  extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
>  			      enum kernel_read_file_id id);
>  extern void ima_post_path_mknod(struct dentry *dentry);
> +extern void ima_sb_post_new_mount(const struct vfsmount *newmnt,
> +				  const struct path *path);
>  
>  #ifdef CONFIG_IMA_KEXEC
>  extern void ima_add_kexec_buffer(struct kimage *image);
> @@ -65,6 +67,9 @@ static inline void ima_post_path_mknod(struct dentry *dentry)
>  	return;
>  }
>  
> +static inline void ima_sb_post_new_mount(const struct vfsmount *newmnt,
> +					 const struct path *path)
> +{ }
>  #endif /* CONFIG_IMA */
>  
>  #ifndef CONFIG_IMA_KEXEC
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index b00186914df8..a0a685189001 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -354,6 +354,50 @@ void ima_post_path_mknod(struct dentry *dentry)
>  }
>  
>  /**
> + * ima_sb_post_new_mount - check filesystem mounted flags
> + *
> + * Indicate that filesystem isn't mounted with i_version enabled.
> + */
> +void ima_sb_post_new_mount(const struct vfsmount *newmnt,
> +			   const struct path *path)
> +{
> +	struct super_block *sb;
> +	unsigned long pseudo_fs[] = {CGROUP_SUPER_MAGIC, CGROUP2_SUPER_MAGIC,
> +		SYSFS_MAGIC, DEVPTS_SUPER_MAGIC, PSTOREFS_MAGIC, EFIVARFS_MAGIC,
> +		DEBUGFS_MAGIC, TMPFS_MAGIC};
> +	char *pathbuf = NULL;
> +	char filename[NAME_MAX];
> +	const char *pathname;
> +	bool found = 0;
> +	int i;
> +
> +	sb = newmnt ? newmnt->mnt_sb : path->mnt->mnt_sb;
> +
> +	if ((sb->s_flags & MS_I_VERSION) || (sb->s_flags & MS_RDONLY) ||
> +	    (sb->s_flags & MS_KERNMOUNT))
> +		return;
> +
> +	for (i = 0; i < ARRAY_SIZE(pseudo_fs); i++) {
> +		if (pseudo_fs[i] != sb->s_magic)
> +			continue;
> +
> +		found = 1;
> +		break;
> +	}
> +	if (found)
> +		return;
> +
> +	pathname = ima_d_path(path, &pathbuf, filename);
> +	if (!pathname)
> +		return;
> +
> +	if (newmnt)
> +		pr_warn("ima: %s mounted without i_version enabled\n",
> +			pathname);
> +	 __putname(pathbuf);
> +}
> +
> +/**
>   * ima_read_file - pre-measure/appraise hook decision based on policy
>   * @file: pointer to the file to be measured/appraised/audit
>   * @read_id: caller identifier
> diff --git a/security/security.c b/security/security.c
> index 592153e8d2b6..79111141b383 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -402,6 +402,7 @@ void security_sb_post_new_mount(const struct vfsmount *newmnt,
>  				const struct path *path)
>  {
>  	call_void_hook(sb_post_new_mount, newmnt, path);
> +	ima_sb_post_new_mount(newmnt, path);

Have you thought about converting the IMA code into
a security module that gets loaded after all the others?
That would make this (and several other similar instances)
bit of special case code go away.


>  }
>  
>  int security_sb_umount(struct vfsmount *mnt, int flags)

[RFC PATCH 2/4] ima: define new ima_sb_post_new_mount hook

[Casey Schaufler]

On 8/16/2017 10:30 AM, Mimi Zohar wrote:
> IMA measures a file, verifies a file's integrity, and caches the
> results.  On filesystems with MS_I_VERSION enabled, IMA can detect
> file changes and cause them to be re-measured and verified.  On
> filesystems without MS_I_VERSION enabled, files are measured and
> verified just once.
>
> This patch logs filesystems mounted without MS_I_VERSION.
>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> ---
>  include/linux/ima.h               |  5 +++++
>  security/integrity/ima/ima_main.c | 44 +++++++++++++++++++++++++++++++++++++++
>  security/security.c               |  1 +
>  3 files changed, 50 insertions(+)
>
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 0e4647e0eb60..4475cb01149c 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -23,6 +23,8 @@ extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
>  extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
>  			      enum kernel_read_file_id id);
>  extern void ima_post_path_mknod(struct dentry *dentry);
> +extern void ima_sb_post_new_mount(const struct vfsmount *newmnt,
> +				  const struct path *path);
>  
>  #ifdef CONFIG_IMA_KEXEC
>  extern void ima_add_kexec_buffer(struct kimage *image);
> @@ -65,6 +67,9 @@ static inline void ima_post_path_mknod(struct dentry *dentry)
>  	return;
>  }
>  
> +static inline void ima_sb_post_new_mount(const struct vfsmount *newmnt,
> +					 const struct path *path)
> +{ }
>  #endif /* CONFIG_IMA */
>  
>  #ifndef CONFIG_IMA_KEXEC
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index b00186914df8..a0a685189001 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -354,6 +354,50 @@ void ima_post_path_mknod(struct dentry *dentry)
>  }
>  
>  /**
> + * ima_sb_post_new_mount - check filesystem mounted flags
> + *
> + * Indicate that filesystem isn't mounted with i_version enabled.
> + */
> +void ima_sb_post_new_mount(const struct vfsmount *newmnt,
> +			   const struct path *path)
> +{
> +	struct super_block *sb;
> +	unsigned long pseudo_fs[] = {CGROUP_SUPER_MAGIC, CGROUP2_SUPER_MAGIC,
> +		SYSFS_MAGIC, DEVPTS_SUPER_MAGIC, PSTOREFS_MAGIC, EFIVARFS_MAGIC,
> +		DEBUGFS_MAGIC, TMPFS_MAGIC};
> +	char *pathbuf = NULL;
> +	char filename[NAME_MAX];
> +	const char *pathname;
> +	bool found = 0;
> +	int i;
> +
> +	sb = newmnt ? newmnt->mnt_sb : path->mnt->mnt_sb;
> +
> +	if ((sb->s_flags & MS_I_VERSION) || (sb->s_flags & MS_RDONLY) ||
> +	    (sb->s_flags & MS_KERNMOUNT))
> +		return;
> +
> +	for (i = 0; i < ARRAY_SIZE(pseudo_fs); i++) {
> +		if (pseudo_fs[i] != sb->s_magic)
> +			continue;
> +
> +		found = 1;
> +		break;
> +	}
> +	if (found)
> +		return;
> +
> +	pathname = ima_d_path(path, &pathbuf, filename);
> +	if (!pathname)
> +		return;
> +
> +	if (newmnt)
> +		pr_warn("ima: %s mounted without i_version enabled\n",
> +			pathname);
> +	 __putname(pathbuf);
> +}
> +
> +/**
>   * ima_read_file - pre-measure/appraise hook decision based on policy
>   * @file: pointer to the file to be measured/appraised/audit
>   * @read_id: caller identifier
> diff --git a/security/security.c b/security/security.c
> index 592153e8d2b6..79111141b383 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -402,6 +402,7 @@ void security_sb_post_new_mount(const struct vfsmount *newmnt,
>  				const struct path *path)
>  {
>  	call_void_hook(sb_post_new_mount, newmnt, path);
> +	ima_sb_post_new_mount(newmnt, path);

Have you thought about converting the IMA code into
a security module that gets loaded after all the others?
That would make this (and several other similar instances)
bit of special case code go away.


>  }
>  
>  int security_sb_umount(struct vfsmount *mnt, int flags)

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

Re: [RFC PATCH 2/4] ima: define new ima_sb_post_new_mount hook

[Mimi Zohar]

On Wed, 2017-08-16 at 12:24 -0700, Casey Schaufler wrote:
> On 8/16/2017 10:30 AM, Mimi Zohar wrote:

> > diff --git a/security/security.c b/security/security.c
> > index 592153e8d2b6..79111141b383 100644
> > --- a/security/security.c
> > +++ b/security/security.c
> > @@ -402,6 +402,7 @@ void security_sb_post_new_mount(const struct vfsmount *newmnt,
> >  				const struct path *path)
> >  {
> >  	call_void_hook(sb_post_new_mount, newmnt, path);
> > +	ima_sb_post_new_mount(newmnt, path);
> 
> Have you thought about converting the IMA code into
> a security module that gets loaded after all the others?
> That would make this (and several other similar instances)
> bit of special case code go away.

Yes, each time I need to add another IMA call, where there isn't an
existing LSM hook, a decision needs to be made as to whether it needs
to be a generic LSM hook or not.

Assuming we made IMA an LSM module, what would we do with these other
calls?  Would they need to be converted to LSM hooks?  (Are all LSMs
visited, even if an earlier LSM fails?  Or does the first LSM failure,
stop the LSM traversal?)

Unlike LSMs which are sharing the i_sec, IMA doesn't have an entry in
the inode, but does an rbtree lookup to access the associated data.
 Having an i_sec would simplify a lot of the code, but making this
sort of change would be a major undertaking.

In this context, I'm not sure what you mean by "loaded".  IMA needs to
be enabled from the very beginning to capture all measurements and
verify the integrity of files, without any gaps.  At some point this
would include other LSM policies.  IMA certainly cannot be loaded late
like kernel modules.

Similarly, we would need to think about EVM.

Mimi

[RFC PATCH 2/4] ima: define new ima_sb_post_new_mount hook

[Mimi Zohar]

On Wed, 2017-08-16 at 12:24 -0700, Casey Schaufler wrote:
> On 8/16/2017 10:30 AM, Mimi Zohar wrote:

> > diff --git a/security/security.c b/security/security.c
> > index 592153e8d2b6..79111141b383 100644
> > --- a/security/security.c
> > +++ b/security/security.c
> > @@ -402,6 +402,7 @@ void security_sb_post_new_mount(const struct vfsmount *newmnt,
> >  				const struct path *path)
> >  {
> >  	call_void_hook(sb_post_new_mount, newmnt, path);
> > +	ima_sb_post_new_mount(newmnt, path);
> 
> Have you thought about converting the IMA code into
> a security module that gets loaded after all the others?
> That would make this (and several other similar instances)
> bit of special case code go away.

Yes, each time I need to add another IMA call, where there isn't an
existing LSM hook, a decision needs to be made as to whether it needs
to be a generic LSM hook or not.

Assuming we made IMA an LSM module, what would we do with these other
calls? ?Would they need to be converted to LSM hooks? ?(Are all LSMs
visited, even if an earlier LSM fails? ?Or does the first LSM failure,
stop the LSM traversal?)

Unlike LSMs which are sharing the i_sec, IMA doesn't have an entry in
the inode, but does an rbtree lookup to access the associated data.
?Having an i_sec would simplify a lot of the code, but making this
sort of change would be a major undertaking.

In this context, I'm not sure what you mean by "loaded".? IMA needs to
be enabled from the very beginning to capture all measurements and
verify the integrity of files, without any gaps. ?At some point this
would include other LSM policies. ?IMA certainly cannot be loaded late
like kernel modules.

Similarly, we would need to think about EVM.

Mimi

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

Re: [Linux-ima-devel] [RFC PATCH 2/4] ima: define new ima_sb_post_new_mount hook

[James Morris]

[-- Attachment #1: Type: text/plain, Size: 576 bytes --]

On Wed, 16 Aug 2017, Mimi Zohar wrote:

> In this context, I'm not sure what you mean by "loaded".  IMA needs to
> be enabled from the very beginning to capture all measurements and
> verify the integrity of files, without any gaps.  At some point this
> would include other LSM policies.

I think it's better to keep IMA orthogonal to LSM for this reason.

The original motivation to implement IMA as a separate API was because LSM 
was at the time considered specific to access control mechanisms, although 
that is not the case now.

-- 
James Morris
<jmorris@namei.org>

[Linux-ima-devel] [RFC PATCH 2/4] ima: define new ima_sb_post_new_mount hook

[James Morris]

On Wed, 16 Aug 2017, Mimi Zohar wrote:

> In this context, I'm not sure what you mean by "loaded".? IMA needs to
> be enabled from the very beginning to capture all measurements and
> verify the integrity of files, without any gaps. ?At some point this
> would include other LSM policies.

I think it's better to keep IMA orthogonal to LSM for this reason.

The original motivation to implement IMA as a separate API was because LSM 
was at the time considered specific to access control mechanisms, although 
that is not the case now.

-- 
James Morris
<jmorris@namei.org>

Re: [RFC PATCH 2/4] ima: define new ima_sb_post_new_mount hook

[Jeff Layton]

On Wed, 2017-08-16 at 13:30 -0400, Mimi Zohar wrote:
> IMA measures a file, verifies a file's integrity, and caches the
> results.  On filesystems with MS_I_VERSION enabled, IMA can detect
> file changes and cause them to be re-measured and verified.  On
> filesystems without MS_I_VERSION enabled, files are measured and
> verified just once.
> 
> This patch logs filesystems mounted without MS_I_VERSION.
> 
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> ---
>  include/linux/ima.h               |  5 +++++
>  security/integrity/ima/ima_main.c | 44 +++++++++++++++++++++++++++++++++++++++
>  security/security.c               |  1 +
>  3 files changed, 50 insertions(+)
> 

Sorry for the late review. I just started dusting off my i_version
rework, and noticed that IMA still has unaddressed problems here.

> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 0e4647e0eb60..4475cb01149c 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -23,6 +23,8 @@ extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
>  extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
>  			      enum kernel_read_file_id id);
>  extern void ima_post_path_mknod(struct dentry *dentry);
> +extern void ima_sb_post_new_mount(const struct vfsmount *newmnt,
> +				  const struct path *path);
>  
>  #ifdef CONFIG_IMA_KEXEC
>  extern void ima_add_kexec_buffer(struct kimage *image);
> @@ -65,6 +67,9 @@ static inline void ima_post_path_mknod(struct dentry *dentry)
>  	return;
>  }
>  
> +static inline void ima_sb_post_new_mount(const struct vfsmount *newmnt,
> +					 const struct path *path)
> +{ }
>  #endif /* CONFIG_IMA */
>  
>  #ifndef CONFIG_IMA_KEXEC
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index b00186914df8..a0a685189001 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -354,6 +354,50 @@ void ima_post_path_mknod(struct dentry *dentry)
>  }
>  
>  /**
> + * ima_sb_post_new_mount - check filesystem mounted flags
> + *
> + * Indicate that filesystem isn't mounted with i_version enabled.
> + */
> +void ima_sb_post_new_mount(const struct vfsmount *newmnt,
> +			   const struct path *path)
> +{
> +	struct super_block *sb;
> +	unsigned long pseudo_fs[] = {CGROUP_SUPER_MAGIC, CGROUP2_SUPER_MAGIC,
> +		SYSFS_MAGIC, DEVPTS_SUPER_MAGIC, PSTOREFS_MAGIC, EFIVARFS_MAGIC,
> +		DEBUGFS_MAGIC, TMPFS_MAGIC};

Barf. What about procfs? This looks like something that will very
subject to bitrot.


> +	char *pathbuf = NULL;
> +	char filename[NAME_MAX];
> +	const char *pathname;
> +	bool found = 0;
> +	int i;
> +
> +	sb = newmnt ? newmnt->mnt_sb : path->mnt->mnt_sb;
> +
> +	if ((sb->s_flags & MS_I_VERSION) || (sb->s_flags & MS_RDONLY) ||
> +	    (sb->s_flags & MS_KERNMOUNT))
> +		return;
> +
> +	for (i = 0; i < ARRAY_SIZE(pseudo_fs); i++) {
> +		if (pseudo_fs[i] != sb->s_magic)
> +			continue;
> +
> +		found = 1;
> +		break;
> +	}
> +	if (found)
> +		return;
> +
> +	pathname = ima_d_path(path, &pathbuf, filename);
> +	if (!pathname)
> +		return;
> +
> +	if (newmnt)
> +		pr_warn("ima: %s mounted without i_version enabled\n",
> +			pathname);
> +	 __putname(pathbuf);
> +}
> +
> +/**
>   * ima_read_file - pre-measure/appraise hook decision based on policy
>   * @file: pointer to the file to be measured/appraised/audit
>   * @read_id: caller identifier
> diff --git a/security/security.c b/security/security.c
> index 592153e8d2b6..79111141b383 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -402,6 +402,7 @@ void security_sb_post_new_mount(const struct vfsmount *newmnt,
>  				const struct path *path)
>  {
>  	call_void_hook(sb_post_new_mount, newmnt, path);
> +	ima_sb_post_new_mount(newmnt, path);
>  }
>  
>  int security_sb_umount(struct vfsmount *mnt, int flags)

Personally, I'm not a huge fan of this scheme. It seems quite invasive,
and doesn't really seem to address the stated problem well.

The warning itself seems ok, but I don't really see what's wrong with
performing remeasurement when the mtime changes on filesystems that
don't have SB_I_VERSION set. Surely that's better than limiting it to an
initial measurement?

Maybe I just don't understand what you're really trying to achieve here.
-- 
Jeff Layton <jlayton@redhat.com>

[RFC PATCH 2/4] ima: define new ima_sb_post_new_mount hook

[Jeff Layton]

On Wed, 2017-08-16 at 13:30 -0400, Mimi Zohar wrote:
> IMA measures a file, verifies a file's integrity, and caches the
> results.  On filesystems with MS_I_VERSION enabled, IMA can detect
> file changes and cause them to be re-measured and verified.  On
> filesystems without MS_I_VERSION enabled, files are measured and
> verified just once.
> 
> This patch logs filesystems mounted without MS_I_VERSION.
> 
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> ---
>  include/linux/ima.h               |  5 +++++
>  security/integrity/ima/ima_main.c | 44 +++++++++++++++++++++++++++++++++++++++
>  security/security.c               |  1 +
>  3 files changed, 50 insertions(+)
> 

Sorry for the late review. I just started dusting off my i_version
rework, and noticed that IMA still has unaddressed problems here.

> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 0e4647e0eb60..4475cb01149c 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -23,6 +23,8 @@ extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
>  extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
>  			      enum kernel_read_file_id id);
>  extern void ima_post_path_mknod(struct dentry *dentry);
> +extern void ima_sb_post_new_mount(const struct vfsmount *newmnt,
> +				  const struct path *path);
>  
>  #ifdef CONFIG_IMA_KEXEC
>  extern void ima_add_kexec_buffer(struct kimage *image);
> @@ -65,6 +67,9 @@ static inline void ima_post_path_mknod(struct dentry *dentry)
>  	return;
>  }
>  
> +static inline void ima_sb_post_new_mount(const struct vfsmount *newmnt,
> +					 const struct path *path)
> +{ }
>  #endif /* CONFIG_IMA */
>  
>  #ifndef CONFIG_IMA_KEXEC
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index b00186914df8..a0a685189001 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -354,6 +354,50 @@ void ima_post_path_mknod(struct dentry *dentry)
>  }
>  
>  /**
> + * ima_sb_post_new_mount - check filesystem mounted flags
> + *
> + * Indicate that filesystem isn't mounted with i_version enabled.
> + */
> +void ima_sb_post_new_mount(const struct vfsmount *newmnt,
> +			   const struct path *path)
> +{
> +	struct super_block *sb;
> +	unsigned long pseudo_fs[] = {CGROUP_SUPER_MAGIC, CGROUP2_SUPER_MAGIC,
> +		SYSFS_MAGIC, DEVPTS_SUPER_MAGIC, PSTOREFS_MAGIC, EFIVARFS_MAGIC,
> +		DEBUGFS_MAGIC, TMPFS_MAGIC};

Barf. What about procfs? This looks like something that will very
subject to bitrot.


> +	char *pathbuf = NULL;
> +	char filename[NAME_MAX];
> +	const char *pathname;
> +	bool found = 0;
> +	int i;
> +
> +	sb = newmnt ? newmnt->mnt_sb : path->mnt->mnt_sb;
> +
> +	if ((sb->s_flags & MS_I_VERSION) || (sb->s_flags & MS_RDONLY) ||
> +	    (sb->s_flags & MS_KERNMOUNT))
> +		return;
> +
> +	for (i = 0; i < ARRAY_SIZE(pseudo_fs); i++) {
> +		if (pseudo_fs[i] != sb->s_magic)
> +			continue;
> +
> +		found = 1;
> +		break;
> +	}
> +	if (found)
> +		return;
> +
> +	pathname = ima_d_path(path, &pathbuf, filename);
> +	if (!pathname)
> +		return;
> +
> +	if (newmnt)
> +		pr_warn("ima: %s mounted without i_version enabled\n",
> +			pathname);
> +	 __putname(pathbuf);
> +}
> +
> +/**
>   * ima_read_file - pre-measure/appraise hook decision based on policy
>   * @file: pointer to the file to be measured/appraised/audit
>   * @read_id: caller identifier
> diff --git a/security/security.c b/security/security.c
> index 592153e8d2b6..79111141b383 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -402,6 +402,7 @@ void security_sb_post_new_mount(const struct vfsmount *newmnt,
>  				const struct path *path)
>  {
>  	call_void_hook(sb_post_new_mount, newmnt, path);
> +	ima_sb_post_new_mount(newmnt, path);
>  }
>  
>  int security_sb_umount(struct vfsmount *mnt, int flags)

Personally, I'm not a huge fan of this scheme. It seems quite invasive,
and doesn't really seem to address the stated problem well.

The warning itself seems ok, but I don't really see what's wrong with
performing remeasurement when the mtime changes on filesystems that
don't have SB_I_VERSION set. Surely that's better than limiting it to an
initial measurement?

Maybe I just don't understand what you're really trying to achieve here.
-- 
Jeff Layton <jlayton@redhat.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 2/4] ima: define new ima_sb_post_new_mount hook

[Mimi Zohar]

Hi Jeff,

[The IMA/EVM and the TPM mailing lists have been combined as a single
linux-integrity mailing list.]

On Thu, 2017-12-07 at 07:26 -0500, Jeff Layton wrote:
> Sorry for the late review. I just started dusting off my i_version
> rework, and noticed that IMA still has unaddressed problems here.

<snip>

> Personally, I'm not a huge fan of this scheme. It seems quite invasive,
> and doesn't really seem to address the stated problem well.

A cleaned up version of this patch set was meant to follow the
introduction of a new integrity_read method, but that patch set was
rejected.  At this point, I have no intentions of upstreaming a
cleaned up version this patch set either.

> The warning itself seems ok, but I don't really see what's wrong with
> performing remeasurement when the mtime changes on filesystems that
> don't have SB_I_VERSION set. Surely that's better than limiting it to an
> initial measurement?
> 
> Maybe I just don't understand what you're really trying to achieve here.

Based on discussions with Sascha Hauer, he convinced me the i_version
test is basically just a performance improvement and posted a patch
that checks the filesystem for i_version support, before relying on it
-  https://www.spinics.net/lists/linux-integrity/msg00033.html.

Mimi

[RFC PATCH 2/4] ima: define new ima_sb_post_new_mount hook

[Mimi Zohar]

Hi Jeff,

[The IMA/EVM and the TPM mailing lists have been combined as a single
linux-integrity mailing list.]

On Thu, 2017-12-07 at 07:26 -0500, Jeff Layton wrote:
> Sorry for the late review. I just started dusting off my i_version
> rework, and noticed that IMA still has unaddressed problems here.

<snip>

> Personally, I'm not a huge fan of this scheme. It seems quite invasive,
> and doesn't really seem to address the stated problem well.

A cleaned up version of this patch set was meant to follow the
introduction of a new integrity_read method, but that patch set was
rejected. ?At this point, I have no intentions of upstreaming a
cleaned up version this patch set either.

> The warning itself seems ok, but I don't really see what's wrong with
> performing remeasurement when the mtime changes on filesystems that
> don't have SB_I_VERSION set. Surely that's better than limiting it to an
> initial measurement?
> 
> Maybe I just don't understand what you're really trying to achieve here.

Based on discussions with Sascha Hauer, he convinced me the i_version
test is basically just a performance improvement and posted a patch
that checks the filesystem for i_version support, before relying on it
- ?https://www.spinics.net/lists/linux-integrity/msg00033.html.

Mimi

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

Re: [RFC PATCH 2/4] ima: define new ima_sb_post_new_mount hook

[Mimi Zohar]

Hi Jeff,

[The IMA/EVM and the TPM mailing lists have been combined as a single
linux-integrity mailing list.]

On Thu, 2017-12-07 at 07:26 -0500, Jeff Layton wrote:
> Sorry for the late review. I just started dusting off my i_version
> rework, and noticed that IMA still has unaddressed problems here.

<snip>

> Personally, I'm not a huge fan of this scheme. It seems quite invasive,
> and doesn't really seem to address the stated problem well.

A cleaned up version of this patch set was meant to follow the
introduction of a new integrity_read method, but that patch set was
rejected.  At this point, I have no intentions of upstreaming a
cleaned up version this patch set either.

> The warning itself seems ok, but I don't really see what's wrong with
> performing remeasurement when the mtime changes on filesystems that
> don't have SB_I_VERSION set. Surely that's better than limiting it to an
> initial measurement?
> 
> Maybe I just don't understand what you're really trying to achieve here.

Based on discussions with Sascha Hauer, he convinced me the i_version
test is basically just a performance improvement and posted a patch
that checks the filesystem for i_version support, before relying on it
-  https://www.spinics.net/lists/linux-integrity/msg00033.html.

Mimi

Re: [RFC PATCH 2/4] ima: define new ima_sb_post_new_mount hook

[Jeff Layton]

On Thu, 2017-12-07 at 09:35 -0500, Mimi Zohar wrote:
> Hi Jeff,
> 
> [The IMA/EVM and the TPM mailing lists have been combined as a single
> linux-integrity mailing list.]
> 
> On Thu, 2017-12-07 at 07:26 -0500, Jeff Layton wrote:
> > Sorry for the late review. I just started dusting off my i_version
> > rework, and noticed that IMA still has unaddressed problems here.
> 
> <snip>
> 
> > Personally, I'm not a huge fan of this scheme. It seems quite invasive,
> > and doesn't really seem to address the stated problem well.
> 
> A cleaned up version of this patch set was meant to follow the
> introduction of a new integrity_read method, but that patch set was
> rejected.  At this point, I have no intentions of upstreaming a
> cleaned up version this patch set either.
> 
> > The warning itself seems ok, but I don't really see what's wrong with
> > performing remeasurement when the mtime changes on filesystems that
> > don't have SB_I_VERSION set. Surely that's better than limiting it to an
> > initial measurement?
> > 
> > Maybe I just don't understand what you're really trying to achieve here.
> 
> Based on discussions with Sascha Hauer, he convinced me the i_version
> test is basically just a performance improvement and posted a patch
> that checks the filesystem for i_version support, before relying on it
> -  https://www.spinics.net/lists/linux-integrity/msg00033.html.
> 
> Mimi
> 

Thanks for the link. That patch looks good to me. Any idea when and if
it will be merged?

Thanks,
-- 
Jeff Layton <jlayton@redhat.com>

[RFC PATCH 2/4] ima: define new ima_sb_post_new_mount hook

[Jeff Layton]

On Thu, 2017-12-07 at 09:35 -0500, Mimi Zohar wrote:
> Hi Jeff,
> 
> [The IMA/EVM and the TPM mailing lists have been combined as a single
> linux-integrity mailing list.]
> 
> On Thu, 2017-12-07 at 07:26 -0500, Jeff Layton wrote:
> > Sorry for the late review. I just started dusting off my i_version
> > rework, and noticed that IMA still has unaddressed problems here.
> 
> <snip>
> 
> > Personally, I'm not a huge fan of this scheme. It seems quite invasive,
> > and doesn't really seem to address the stated problem well.
> 
> A cleaned up version of this patch set was meant to follow the
> introduction of a new integrity_read method, but that patch set was
> rejected.  At this point, I have no intentions of upstreaming a
> cleaned up version this patch set either.
> 
> > The warning itself seems ok, but I don't really see what's wrong with
> > performing remeasurement when the mtime changes on filesystems that
> > don't have SB_I_VERSION set. Surely that's better than limiting it to an
> > initial measurement?
> > 
> > Maybe I just don't understand what you're really trying to achieve here.
> 
> Based on discussions with Sascha Hauer, he convinced me the i_version
> test is basically just a performance improvement and posted a patch
> that checks the filesystem for i_version support, before relying on it
> -  https://www.spinics.net/lists/linux-integrity/msg00033.html.
> 
> Mimi
> 

Thanks for the link. That patch looks good to me. Any idea when and if
it will be merged?

Thanks,
-- 
Jeff Layton <jlayton@redhat.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 2/4] ima: define new ima_sb_post_new_mount hook

[Mimi Zohar]

On Thu, 2017-12-07 at 09:50 -0500, Jeff Layton wrote:
> On Thu, 2017-12-07 at 09:35 -0500, Mimi Zohar wrote:
> > Hi Jeff,
> > 
> > [The IMA/EVM and the TPM mailing lists have been combined as a single
> > linux-integrity mailing list.]
> > 
> > On Thu, 2017-12-07 at 07:26 -0500, Jeff Layton wrote:
> > > Sorry for the late review. I just started dusting off my i_version
> > > rework, and noticed that IMA still has unaddressed problems here.
> > 
> > <snip>
> > 
> > > Personally, I'm not a huge fan of this scheme. It seems quite invasive,
> > > and doesn't really seem to address the stated problem well.
> > 
> > A cleaned up version of this patch set was meant to follow the
> > introduction of a new integrity_read method, but that patch set was
> > rejected.  At this point, I have no intentions of upstreaming a
> > cleaned up version this patch set either.
> > 
> > > The warning itself seems ok, but I don't really see what's wrong with
> > > performing remeasurement when the mtime changes on filesystems that
> > > don't have SB_I_VERSION set. Surely that's better than limiting it to an
> > > initial measurement?
> > > 
> > > Maybe I just don't understand what you're really trying to achieve here.
> > 
> > Based on discussions with Sascha Hauer, he convinced me the i_version
> > test is basically just a performance improvement and posted a patch
> > that checks the filesystem for i_version support, before relying on it
> > -  https://www.spinics.net/lists/linux-integrity/msg00033.html.
> > 
> > Mimi
> > 
> 
> Thanks for the link. That patch looks good to me. Any idea when and if
> it will be merged?

Is that an Ack?  Barring any testing issues, I'll upstream it with
yours in the next open window.

Mimi

[RFC PATCH 2/4] ima: define new ima_sb_post_new_mount hook

[Mimi Zohar]

On Thu, 2017-12-07 at 09:50 -0500, Jeff Layton wrote:
> On Thu, 2017-12-07 at 09:35 -0500, Mimi Zohar wrote:
> > Hi Jeff,
> > 
> > [The IMA/EVM and the TPM mailing lists have been combined as a single
> > linux-integrity mailing list.]
> > 
> > On Thu, 2017-12-07 at 07:26 -0500, Jeff Layton wrote:
> > > Sorry for the late review. I just started dusting off my i_version
> > > rework, and noticed that IMA still has unaddressed problems here.
> > 
> > <snip>
> > 
> > > Personally, I'm not a huge fan of this scheme. It seems quite invasive,
> > > and doesn't really seem to address the stated problem well.
> > 
> > A cleaned up version of this patch set was meant to follow the
> > introduction of a new integrity_read method, but that patch set was
> > rejected.  At this point, I have no intentions of upstreaming a
> > cleaned up version this patch set either.
> > 
> > > The warning itself seems ok, but I don't really see what's wrong with
> > > performing remeasurement when the mtime changes on filesystems that
> > > don't have SB_I_VERSION set. Surely that's better than limiting it to an
> > > initial measurement?
> > > 
> > > Maybe I just don't understand what you're really trying to achieve here.
> > 
> > Based on discussions with Sascha Hauer, he convinced me the i_version
> > test is basically just a performance improvement and posted a patch
> > that checks the filesystem for i_version support, before relying on it
> > -  https://www.spinics.net/lists/linux-integrity/msg00033.html.
> > 
> > Mimi
> > 
> 
> Thanks for the link. That patch looks good to me. Any idea when and if
> it will be merged?

Is that an Ack? ?Barring any testing issues, I'll upstream it with
yours in the next open window.

Mimi

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

Re: [RFC PATCH 2/4] ima: define new ima_sb_post_new_mount hook

[Mimi Zohar]

On Thu, 2017-12-07 at 09:50 -0500, Jeff Layton wrote:
> On Thu, 2017-12-07 at 09:35 -0500, Mimi Zohar wrote:
> > Hi Jeff,
> > 
> > [The IMA/EVM and the TPM mailing lists have been combined as a single
> > linux-integrity mailing list.]
> > 
> > On Thu, 2017-12-07 at 07:26 -0500, Jeff Layton wrote:
> > > Sorry for the late review. I just started dusting off my i_version
> > > rework, and noticed that IMA still has unaddressed problems here.
> > 
> > <snip>
> > 
> > > Personally, I'm not a huge fan of this scheme. It seems quite invasive,
> > > and doesn't really seem to address the stated problem well.
> > 
> > A cleaned up version of this patch set was meant to follow the
> > introduction of a new integrity_read method, but that patch set was
> > rejected.  At this point, I have no intentions of upstreaming a
> > cleaned up version this patch set either.
> > 
> > > The warning itself seems ok, but I don't really see what's wrong with
> > > performing remeasurement when the mtime changes on filesystems that
> > > don't have SB_I_VERSION set. Surely that's better than limiting it to an
> > > initial measurement?
> > > 
> > > Maybe I just don't understand what you're really trying to achieve here.
> > 
> > Based on discussions with Sascha Hauer, he convinced me the i_version
> > test is basically just a performance improvement and posted a patch
> > that checks the filesystem for i_version support, before relying on it
> > -  https://www.spinics.net/lists/linux-integrity/msg00033.html.
> > 
> > Mimi
> > 
> 
> Thanks for the link. That patch looks good to me. Any idea when and if
> it will be merged?

Is that an Ack?  Barring any testing issues, I'll upstream it with
yours in the next open window.

Mimi

Re: [RFC PATCH 2/4] ima: define new ima_sb_post_new_mount hook

[Jeff Layton]

On Thu, 2017-12-07 at 10:08 -0500, Mimi Zohar wrote:
> On Thu, 2017-12-07 at 09:50 -0500, Jeff Layton wrote:
> > On Thu, 2017-12-07 at 09:35 -0500, Mimi Zohar wrote:
> > > Hi Jeff,
> > > 
> > > [The IMA/EVM and the TPM mailing lists have been combined as a single
> > > linux-integrity mailing list.]
> > > 
> > > On Thu, 2017-12-07 at 07:26 -0500, Jeff Layton wrote:
> > > > Sorry for the late review. I just started dusting off my i_version
> > > > rework, and noticed that IMA still has unaddressed problems here.
> > > 
> > > <snip>
> > > 
> > > > Personally, I'm not a huge fan of this scheme. It seems quite invasive,
> > > > and doesn't really seem to address the stated problem well.
> > > 
> > > A cleaned up version of this patch set was meant to follow the
> > > introduction of a new integrity_read method, but that patch set was
> > > rejected.  At this point, I have no intentions of upstreaming a
> > > cleaned up version this patch set either.
> > > 
> > > > The warning itself seems ok, but I don't really see what's wrong with
> > > > performing remeasurement when the mtime changes on filesystems that
> > > > don't have SB_I_VERSION set. Surely that's better than limiting it to an
> > > > initial measurement?
> > > > 
> > > > Maybe I just don't understand what you're really trying to achieve here.
> > > 
> > > Based on discussions with Sascha Hauer, he convinced me the i_version
> > > test is basically just a performance improvement and posted a patch
> > > that checks the filesystem for i_version support, before relying on it
> > > -  https://www.spinics.net/lists/linux-integrity/msg00033.html.
> > > 
> > > Mimi
> > > 
> > 
> > Thanks for the link. That patch looks good to me. Any idea when and if
> > it will be merged?
> 
> Is that an Ack?  Barring any testing issues, I'll upstream it with
> yours in the next open window.
> 
> Mimi
> 

Sure, you can add:

Reviewed-by: Jeff Layton <jlayton@redhat.com>

[RFC PATCH 2/4] ima: define new ima_sb_post_new_mount hook

[Jeff Layton]

On Thu, 2017-12-07 at 10:08 -0500, Mimi Zohar wrote:
> On Thu, 2017-12-07 at 09:50 -0500, Jeff Layton wrote:
> > On Thu, 2017-12-07 at 09:35 -0500, Mimi Zohar wrote:
> > > Hi Jeff,
> > > 
> > > [The IMA/EVM and the TPM mailing lists have been combined as a single
> > > linux-integrity mailing list.]
> > > 
> > > On Thu, 2017-12-07 at 07:26 -0500, Jeff Layton wrote:
> > > > Sorry for the late review. I just started dusting off my i_version
> > > > rework, and noticed that IMA still has unaddressed problems here.
> > > 
> > > <snip>
> > > 
> > > > Personally, I'm not a huge fan of this scheme. It seems quite invasive,
> > > > and doesn't really seem to address the stated problem well.
> > > 
> > > A cleaned up version of this patch set was meant to follow the
> > > introduction of a new integrity_read method, but that patch set was
> > > rejected.  At this point, I have no intentions of upstreaming a
> > > cleaned up version this patch set either.
> > > 
> > > > The warning itself seems ok, but I don't really see what's wrong with
> > > > performing remeasurement when the mtime changes on filesystems that
> > > > don't have SB_I_VERSION set. Surely that's better than limiting it to an
> > > > initial measurement?
> > > > 
> > > > Maybe I just don't understand what you're really trying to achieve here.
> > > 
> > > Based on discussions with Sascha Hauer, he convinced me the i_version
> > > test is basically just a performance improvement and posted a patch
> > > that checks the filesystem for i_version support, before relying on it
> > > -  https://www.spinics.net/lists/linux-integrity/msg00033.html.
> > > 
> > > Mimi
> > > 
> > 
> > Thanks for the link. That patch looks good to me. Any idea when and if
> > it will be merged?
> 
> Is that an Ack?  Barring any testing issues, I'll upstream it with
> yours in the next open window.
> 
> Mimi
> 

Sure, you can add:

Reviewed-by: Jeff Layton <jlayton@redhat.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 2/4] ima: define new ima_sb_post_new_mount hook

[Jeff Layton]

On Thu, 2017-12-07 at 10:09 -0500, Jeff Layton wrote:
> On Thu, 2017-12-07 at 10:08 -0500, Mimi Zohar wrote:
> > On Thu, 2017-12-07 at 09:50 -0500, Jeff Layton wrote:
> > > On Thu, 2017-12-07 at 09:35 -0500, Mimi Zohar wrote:
> > > > Hi Jeff,
> > > > 
> > > > [The IMA/EVM and the TPM mailing lists have been combined as a single
> > > > linux-integrity mailing list.]
> > > > 
> > > > On Thu, 2017-12-07 at 07:26 -0500, Jeff Layton wrote:
> > > > > Sorry for the late review. I just started dusting off my i_version
> > > > > rework, and noticed that IMA still has unaddressed problems here.
> > > > 
> > > > <snip>
> > > > 
> > > > > Personally, I'm not a huge fan of this scheme. It seems quite invasive,
> > > > > and doesn't really seem to address the stated problem well.
> > > > 
> > > > A cleaned up version of this patch set was meant to follow the
> > > > introduction of a new integrity_read method, but that patch set was
> > > > rejected.  At this point, I have no intentions of upstreaming a
> > > > cleaned up version this patch set either.
> > > > 
> > > > > The warning itself seems ok, but I don't really see what's wrong with
> > > > > performing remeasurement when the mtime changes on filesystems that
> > > > > don't have SB_I_VERSION set. Surely that's better than limiting it to an
> > > > > initial measurement?
> > > > > 
> > > > > Maybe I just don't understand what you're really trying to achieve here.
> > > > 
> > > > Based on discussions with Sascha Hauer, he convinced me the i_version
> > > > test is basically just a performance improvement and posted a patch
> > > > that checks the filesystem for i_version support, before relying on it
> > > > -  https://www.spinics.net/lists/linux-integrity/msg00033.html.
> > > > 
> > > > Mimi
> > > > 
> > > 
> > > Thanks for the link. That patch looks good to me. Any idea when and if
> > > it will be merged?
> > 
> > Is that an Ack?  Barring any testing issues, I'll upstream it with
> > yours in the next open window.
> > 
> > Mimi
> > 
> 
> Sure, you can add:
> 
> Reviewed-by: Jeff Layton <jlayton@redhat.com>

BTW, could you get this into linux-next sometime soon? I have a series
of patches to overhaul i_version handling that I want to go in soon and
there could be merge conflicts.

Thanks,
-- 
Jeff Layton <jlayton@kernel.org>

[RFC PATCH 2/4] ima: define new ima_sb_post_new_mount hook

[Jeff Layton]

On Thu, 2017-12-07 at 10:09 -0500, Jeff Layton wrote:
> On Thu, 2017-12-07 at 10:08 -0500, Mimi Zohar wrote:
> > On Thu, 2017-12-07 at 09:50 -0500, Jeff Layton wrote:
> > > On Thu, 2017-12-07 at 09:35 -0500, Mimi Zohar wrote:
> > > > Hi Jeff,
> > > > 
> > > > [The IMA/EVM and the TPM mailing lists have been combined as a single
> > > > linux-integrity mailing list.]
> > > > 
> > > > On Thu, 2017-12-07 at 07:26 -0500, Jeff Layton wrote:
> > > > > Sorry for the late review. I just started dusting off my i_version
> > > > > rework, and noticed that IMA still has unaddressed problems here.
> > > > 
> > > > <snip>
> > > > 
> > > > > Personally, I'm not a huge fan of this scheme. It seems quite invasive,
> > > > > and doesn't really seem to address the stated problem well.
> > > > 
> > > > A cleaned up version of this patch set was meant to follow the
> > > > introduction of a new integrity_read method, but that patch set was
> > > > rejected.  At this point, I have no intentions of upstreaming a
> > > > cleaned up version this patch set either.
> > > > 
> > > > > The warning itself seems ok, but I don't really see what's wrong with
> > > > > performing remeasurement when the mtime changes on filesystems that
> > > > > don't have SB_I_VERSION set. Surely that's better than limiting it to an
> > > > > initial measurement?
> > > > > 
> > > > > Maybe I just don't understand what you're really trying to achieve here.
> > > > 
> > > > Based on discussions with Sascha Hauer, he convinced me the i_version
> > > > test is basically just a performance improvement and posted a patch
> > > > that checks the filesystem for i_version support, before relying on it
> > > > -  https://www.spinics.net/lists/linux-integrity/msg00033.html.
> > > > 
> > > > Mimi
> > > > 
> > > 
> > > Thanks for the link. That patch looks good to me. Any idea when and if
> > > it will be merged?
> > 
> > Is that an Ack?  Barring any testing issues, I'll upstream it with
> > yours in the next open window.
> > 
> > Mimi
> > 
> 
> Sure, you can add:
> 
> Reviewed-by: Jeff Layton <jlayton@redhat.com>

BTW, could you get this into linux-next sometime soon? I have a series
of patches to overhaul i_version handling that I want to go in soon and
there could be merge conflicts.

Thanks,
-- 
Jeff Layton <jlayton@kernel.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html