[PATCH v3 0/7] kexec/firmware: support system wide policy requiring signatures

[PATCH v3 0/7] kexec/firmware: support system wide policy requiring signatures

[Mimi Zohar]

IMA-appraisal is mostly being used in the embedded or single purpose
closed system environments.  In these environments, both the Kconfig
options and the userspace tools can be modified appropriately to limit
syscalls.  For stock kernels, userspace applications need to continue to
work with older kernels as well as with newer kernels.

In this environment, the customer needs the ability to define a system
wide IMA policy, such as requiring all kexec'ed images, firmware, kernel
modules to be signed, without being dependent on either the Kconfig
options or the userspace tools.[1]

This patch set allows the customer to define a policy which requires
the kexec'ed kernel images, firmware, and/or kernel modules to be
signed.

New to this patch set is the ability to configure a build time IMA policy,
which is automatically loaded at run time without needing to specify it
on the boot command line.  The build time policy rules persist after
loading a custom kernel policy.

[1] kexec-tools suupports the new syscall based on a flag (-s).

Changelog v3:
Based on James' feedback:
- Renamed security_kernel_read_file() to security_kernel_read_data().
- Cleaned up ima_read_data(), replacing if's with switch.

Changelog v2:
- combined "kexec: limit kexec_load syscall" and "firmware: kernel
signature verification" patch sets.
- add support for build time policy.
- defined generic security_kernel_read_blob() wrapper for
  security_kernel_read_file(). Suggested by Luis.
- removed the CONFIG_CFG80211_REQUIRE_SIGNED_REGDB ifdef.  If both REGDB
  and an IMA-appraisal policy require signed firmware, for now require
  both signatures.  Subsequent patches might change this.
- Still unclear if the pre-allocated firmware buffer can be accessed

Mimi Zohar (7):
  security: rename security_kernel_read_file() hook
  kexec: add call to LSM hook in original kexec_load syscall
  ima: based on policy require signed kexec kernel images
  firmware: add call to LSM hook before firmware sysfs fallback
  ima: based on policy require signed firmware (sysfs fallback)
  ima: add build time policy
  ima: based on policy prevent loading firmware (pre-allocated buffer)

 drivers/base/firmware_loader/fallback.c |  7 ++++
 fs/exec.c                               |  2 +-
 include/linux/fs.h                      |  1 +
 include/linux/ima.h                     |  4 +--
 include/linux/security.h                |  4 +--
 kernel/kexec.c                          |  8 +++++
 kernel/module.c                         |  2 +-
 security/integrity/ima/Kconfig          | 58 +++++++++++++++++++++++++++++++++
 security/integrity/ima/ima.h            |  1 +
 security/integrity/ima/ima_main.c       | 39 ++++++++++++++++++----
 security/integrity/ima/ima_policy.c     | 48 +++++++++++++++++++++++++--
 security/loadpin/loadpin.c              |  2 +-
 security/security.c                     |  6 ++--
 13 files changed, 162 insertions(+), 20 deletions(-)

-- 
2.7.5

[PATCH v3 0/7] kexec/firmware: support system wide policy requiring signatures

[Mimi Zohar]

IMA-appraisal is mostly being used in the embedded or single purpose
closed system environments.  In these environments, both the Kconfig
options and the userspace tools can be modified appropriately to limit
syscalls.  For stock kernels, userspace applications need to continue to
work with older kernels as well as with newer kernels.

In this environment, the customer needs the ability to define a system
wide IMA policy, such as requiring all kexec'ed images, firmware, kernel
modules to be signed, without being dependent on either the Kconfig
options or the userspace tools.[1]

This patch set allows the customer to define a policy which requires
the kexec'ed kernel images, firmware, and/or kernel modules to be
signed.

New to this patch set is the ability to configure a build time IMA policy,
which is automatically loaded at run time without needing to specify it
on the boot command line.  The build time policy rules persist after
loading a custom kernel policy.

[1] kexec-tools suupports the new syscall based on a flag (-s).

Changelog v3:
Based on James' feedback:
- Renamed security_kernel_read_file() to security_kernel_read_data().
- Cleaned up ima_read_data(), replacing if's with switch.

Changelog v2:
- combined "kexec: limit kexec_load syscall" and "firmware: kernel
signature verification" patch sets.
- add support for build time policy.
- defined generic security_kernel_read_blob() wrapper for
  security_kernel_read_file(). Suggested by Luis.
- removed the CONFIG_CFG80211_REQUIRE_SIGNED_REGDB ifdef.  If both REGDB
  and an IMA-appraisal policy require signed firmware, for now require
  both signatures.  Subsequent patches might change this.
- Still unclear if the pre-allocated firmware buffer can be accessed

Mimi Zohar (7):
  security: rename security_kernel_read_file() hook
  kexec: add call to LSM hook in original kexec_load syscall
  ima: based on policy require signed kexec kernel images
  firmware: add call to LSM hook before firmware sysfs fallback
  ima: based on policy require signed firmware (sysfs fallback)
  ima: add build time policy
  ima: based on policy prevent loading firmware (pre-allocated buffer)

 drivers/base/firmware_loader/fallback.c |  7 ++++
 fs/exec.c                               |  2 +-
 include/linux/fs.h                      |  1 +
 include/linux/ima.h                     |  4 +--
 include/linux/security.h                |  4 +--
 kernel/kexec.c                          |  8 +++++
 kernel/module.c                         |  2 +-
 security/integrity/ima/Kconfig          | 58 +++++++++++++++++++++++++++++++++
 security/integrity/ima/ima.h            |  1 +
 security/integrity/ima/ima_main.c       | 39 ++++++++++++++++++----
 security/integrity/ima/ima_policy.c     | 48 +++++++++++++++++++++++++--
 security/loadpin/loadpin.c              |  2 +-
 security/security.c                     |  6 ++--
 13 files changed, 162 insertions(+), 20 deletions(-)

-- 
2.7.5

--
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

[PATCH v3 0/7] kexec/firmware: support system wide policy requiring signatures

[Mimi Zohar]

IMA-appraisal is mostly being used in the embedded or single purpose
closed system environments.  In these environments, both the Kconfig
options and the userspace tools can be modified appropriately to limit
syscalls.  For stock kernels, userspace applications need to continue to
work with older kernels as well as with newer kernels.

In this environment, the customer needs the ability to define a system
wide IMA policy, such as requiring all kexec'ed images, firmware, kernel
modules to be signed, without being dependent on either the Kconfig
options or the userspace tools.[1]

This patch set allows the customer to define a policy which requires
the kexec'ed kernel images, firmware, and/or kernel modules to be
signed.

New to this patch set is the ability to configure a build time IMA policy,
which is automatically loaded at run time without needing to specify it
on the boot command line.  The build time policy rules persist after
loading a custom kernel policy.

[1] kexec-tools suupports the new syscall based on a flag (-s).

Changelog v3:
Based on James' feedback:
- Renamed security_kernel_read_file() to security_kernel_read_data().
- Cleaned up ima_read_data(), replacing if's with switch.

Changelog v2:
- combined "kexec: limit kexec_load syscall" and "firmware: kernel
signature verification" patch sets.
- add support for build time policy.
- defined generic security_kernel_read_blob() wrapper for
  security_kernel_read_file(). Suggested by Luis.
- removed the CONFIG_CFG80211_REQUIRE_SIGNED_REGDB ifdef.  If both REGDB
  and an IMA-appraisal policy require signed firmware, for now require
  both signatures.  Subsequent patches might change this.
- Still unclear if the pre-allocated firmware buffer can be accessed

Mimi Zohar (7):
  security: rename security_kernel_read_file() hook
  kexec: add call to LSM hook in original kexec_load syscall
  ima: based on policy require signed kexec kernel images
  firmware: add call to LSM hook before firmware sysfs fallback
  ima: based on policy require signed firmware (sysfs fallback)
  ima: add build time policy
  ima: based on policy prevent loading firmware (pre-allocated buffer)

 drivers/base/firmware_loader/fallback.c |  7 ++++
 fs/exec.c                               |  2 +-
 include/linux/fs.h                      |  1 +
 include/linux/ima.h                     |  4 +--
 include/linux/security.h                |  4 +--
 kernel/kexec.c                          |  8 +++++
 kernel/module.c                         |  2 +-
 security/integrity/ima/Kconfig          | 58 +++++++++++++++++++++++++++++++++
 security/integrity/ima/ima.h            |  1 +
 security/integrity/ima/ima_main.c       | 39 ++++++++++++++++++----
 security/integrity/ima/ima_policy.c     | 48 +++++++++++++++++++++++++--
 security/loadpin/loadpin.c              |  2 +-
 security/security.c                     |  6 ++--
 13 files changed, 162 insertions(+), 20 deletions(-)

-- 
2.7.5


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v3 1/7] security: rename security_kernel_read_file() hook

[Mimi Zohar]

In order for LSMs and IMA-appraisal to differentiate between the original
and new syscalls (eg. kexec, kernel modules, firmware), both the original
and new syscalls must call an LSM hook.

Commit 2e72d51b4ac3 ("security: introduce kernel_module_from_file hook")
introduced calling security_kernel_module_from_file() in both the original
and new syscalls.  Commit a1db74209483 ("module: replace
copy_module_from_fd with kernel version") replaced these LSM calls with
security_kernel_read_file().

Commit e40ba6d56b41 ("firmware: replace call to fw_read_file_contents()
with kernel version") and commit b804defe4297  ("kexec: replace call to
copy_file_from_fd() with kernel version") replaced their own version of
reading a file from the kernel with the generic
kernel_read_file_from_path/fd() versions, which call the pre and post
security_kernel_read_file LSM hooks.

Missing are LSM calls in the original kexec syscall and firmware sysfs
fallback method.  Instead of defining a new LSM hook or wrapper for
security_kernel_read_file(), this patch renames the original
security_kernel_read_file() hook to security_kernel_read_data(), and
updates LSM usage of the hook (eg. loadpin, init_module, IMA).

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Eric Biederman <ebiederm@xmission.com>
Cc: Luis R. Rodriguez <mcgrof@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Casey Schaufler <casey@schaufler-ca.com>

Changelog v3:
- Rename security_kernel_read_file to security_kernel_read_data().

Changelog v2:
- Define a generic wrapper named security_kernel_read_blob() for
security_kernel_read_file().

Changelog v1:
- Define and call security_kexec_load(), a wrapper for
security_kernel_read_file().
---
 fs/exec.c                         | 2 +-
 include/linux/ima.h               | 4 ++--
 include/linux/security.h          | 4 ++--
 kernel/module.c                   | 2 +-
 security/integrity/ima/ima_main.c | 4 ++--
 security/loadpin/loadpin.c        | 2 +-
 security/security.c               | 6 +++---
 7 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 183059c427b9..0c832b4c6a22 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -904,7 +904,7 @@ int kernel_read_file(struct file *file, void **buf, loff_t *size,
 	if (ret)
 		return ret;
 
-	ret = security_kernel_read_file(file, id);
+	ret = security_kernel_read_data(file, id);
 	if (ret)
 		goto out;
 
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 0e4647e0eb60..423aaf88f8c6 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -19,7 +19,7 @@ extern int ima_bprm_check(struct linux_binprm *bprm);
 extern int ima_file_check(struct file *file, int mask, int opened);
 extern void ima_file_free(struct file *file);
 extern int ima_file_mmap(struct file *file, unsigned long prot);
-extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
+extern int ima_read_data(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);
@@ -49,7 +49,7 @@ static inline int ima_file_mmap(struct file *file, unsigned long prot)
 	return 0;
 }
 
-static inline int ima_read_file(struct file *file, enum kernel_read_file_id id)
+static inline int ima_read_data(struct file *file, enum kernel_read_file_id id)
 {
 	return 0;
 }
diff --git a/include/linux/security.h b/include/linux/security.h
index 63030c85ee19..836a9081b2f3 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -320,7 +320,7 @@ void security_cred_getsecid(const struct cred *c, u32 *secid);
 int security_kernel_act_as(struct cred *new, u32 secid);
 int security_kernel_create_files_as(struct cred *new, struct inode *inode);
 int security_kernel_module_request(char *kmod_name);
-int security_kernel_read_file(struct file *file, enum kernel_read_file_id id);
+int security_kernel_read_data(struct file *file, enum kernel_read_file_id id);
 int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
 				   enum kernel_read_file_id id);
 int security_task_fix_setuid(struct cred *new, const struct cred *old,
@@ -909,7 +909,7 @@ static inline int security_kernel_module_request(char *kmod_name)
 	return 0;
 }
 
-static inline int security_kernel_read_file(struct file *file,
+static inline int security_kernel_read_data(struct file *file,
 					    enum kernel_read_file_id id)
 {
 	return 0;
diff --git a/kernel/module.c b/kernel/module.c
index ce8066b88178..cb84a0b7fbe9 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2879,7 +2879,7 @@ static int copy_module_from_user(const void __user *umod, unsigned long len,
 	if (info->len < sizeof(*(info->hdr)))
 		return -ENOEXEC;
 
-	err = security_kernel_read_file(NULL, READING_MODULE);
+	err = security_kernel_read_data(NULL, READING_MODULE);
 	if (err)
 		return err;
 
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 83f84928ad76..eeb7075868db 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -420,7 +420,7 @@ void ima_post_path_mknod(struct dentry *dentry)
 }
 
 /**
- * ima_read_file - pre-measure/appraise hook decision based on policy
+ * ima_read_data - pre-measure/appraise hook decision based on policy
  * @file: pointer to the file to be measured/appraised/audit
  * @read_id: caller identifier
  *
@@ -430,7 +430,7 @@ void ima_post_path_mknod(struct dentry *dentry)
  *
  * For permission return 0, otherwise return -EACCES.
  */
-int ima_read_file(struct file *file, enum kernel_read_file_id read_id)
+int ima_read_data(struct file *file, enum kernel_read_file_id read_id)
 {
 	bool sig_enforce = is_module_sig_enforced();
 
diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
index 5fa191252c8f..8d7db638fdeb 100644
--- a/security/loadpin/loadpin.c
+++ b/security/loadpin/loadpin.c
@@ -175,7 +175,7 @@ static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
 
 static struct security_hook_list loadpin_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(sb_free_security, loadpin_sb_free_security),
-	LSM_HOOK_INIT(kernel_read_file, loadpin_read_file),
+	LSM_HOOK_INIT(kernel_read_data, loadpin_read_file),
 };
 
 void __init loadpin_add_hooks(void)
diff --git a/security/security.c b/security/security.c
index 68f46d849abe..fc7a2bcf3177 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1033,16 +1033,16 @@ int security_kernel_module_request(char *kmod_name)
 	return call_int_hook(kernel_module_request, 0, kmod_name);
 }
 
-int security_kernel_read_file(struct file *file, enum kernel_read_file_id id)
+int security_kernel_read_data(struct file *file, enum kernel_read_file_id id)
 {
 	int ret;
 
 	ret = call_int_hook(kernel_read_file, 0, file, id);
 	if (ret)
 		return ret;
-	return ima_read_file(file, id);
+	return ima_read_data(file, id);
 }
-EXPORT_SYMBOL_GPL(security_kernel_read_file);
+EXPORT_SYMBOL_GPL(security_kernel_read_data);
 
 int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
 				   enum kernel_read_file_id id)
-- 
2.7.5

[PATCH v3 1/7] security: rename security_kernel_read_file() hook

[Mimi Zohar]

In order for LSMs and IMA-appraisal to differentiate between the original
and new syscalls (eg. kexec, kernel modules, firmware), both the original
and new syscalls must call an LSM hook.

Commit 2e72d51b4ac3 ("security: introduce kernel_module_from_file hook")
introduced calling security_kernel_module_from_file() in both the original
and new syscalls.  Commit a1db74209483 ("module: replace
copy_module_from_fd with kernel version") replaced these LSM calls with
security_kernel_read_file().

Commit e40ba6d56b41 ("firmware: replace call to fw_read_file_contents()
with kernel version") and commit b804defe4297  ("kexec: replace call to
copy_file_from_fd() with kernel version") replaced their own version of
reading a file from the kernel with the generic
kernel_read_file_from_path/fd() versions, which call the pre and post
security_kernel_read_file LSM hooks.

Missing are LSM calls in the original kexec syscall and firmware sysfs
fallback method.  Instead of defining a new LSM hook or wrapper for
security_kernel_read_file(), this patch renames the original
security_kernel_read_file() hook to security_kernel_read_data(), and
updates LSM usage of the hook (eg. loadpin, init_module, IMA).

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Eric Biederman <ebiederm@xmission.com>
Cc: Luis R. Rodriguez <mcgrof@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Casey Schaufler <casey@schaufler-ca.com>

Changelog v3:
- Rename security_kernel_read_file to security_kernel_read_data().

Changelog v2:
- Define a generic wrapper named security_kernel_read_blob() for
security_kernel_read_file().

Changelog v1:
- Define and call security_kexec_load(), a wrapper for
security_kernel_read_file().
---
 fs/exec.c                         | 2 +-
 include/linux/ima.h               | 4 ++--
 include/linux/security.h          | 4 ++--
 kernel/module.c                   | 2 +-
 security/integrity/ima/ima_main.c | 4 ++--
 security/loadpin/loadpin.c        | 2 +-
 security/security.c               | 6 +++---
 7 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 183059c427b9..0c832b4c6a22 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -904,7 +904,7 @@ int kernel_read_file(struct file *file, void **buf, loff_t *size,
 	if (ret)
 		return ret;
 
-	ret = security_kernel_read_file(file, id);
+	ret = security_kernel_read_data(file, id);
 	if (ret)
 		goto out;
 
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 0e4647e0eb60..423aaf88f8c6 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -19,7 +19,7 @@ extern int ima_bprm_check(struct linux_binprm *bprm);
 extern int ima_file_check(struct file *file, int mask, int opened);
 extern void ima_file_free(struct file *file);
 extern int ima_file_mmap(struct file *file, unsigned long prot);
-extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
+extern int ima_read_data(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);
@@ -49,7 +49,7 @@ static inline int ima_file_mmap(struct file *file, unsigned long prot)
 	return 0;
 }
 
-static inline int ima_read_file(struct file *file, enum kernel_read_file_id id)
+static inline int ima_read_data(struct file *file, enum kernel_read_file_id id)
 {
 	return 0;
 }
diff --git a/include/linux/security.h b/include/linux/security.h
index 63030c85ee19..836a9081b2f3 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -320,7 +320,7 @@ void security_cred_getsecid(const struct cred *c, u32 *secid);
 int security_kernel_act_as(struct cred *new, u32 secid);
 int security_kernel_create_files_as(struct cred *new, struct inode *inode);
 int security_kernel_module_request(char *kmod_name);
-int security_kernel_read_file(struct file *file, enum kernel_read_file_id id);
+int security_kernel_read_data(struct file *file, enum kernel_read_file_id id);
 int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
 				   enum kernel_read_file_id id);
 int security_task_fix_setuid(struct cred *new, const struct cred *old,
@@ -909,7 +909,7 @@ static inline int security_kernel_module_request(char *kmod_name)
 	return 0;
 }
 
-static inline int security_kernel_read_file(struct file *file,
+static inline int security_kernel_read_data(struct file *file,
 					    enum kernel_read_file_id id)
 {
 	return 0;
diff --git a/kernel/module.c b/kernel/module.c
index ce8066b88178..cb84a0b7fbe9 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2879,7 +2879,7 @@ static int copy_module_from_user(const void __user *umod, unsigned long len,
 	if (info->len < sizeof(*(info->hdr)))
 		return -ENOEXEC;
 
-	err = security_kernel_read_file(NULL, READING_MODULE);
+	err = security_kernel_read_data(NULL, READING_MODULE);
 	if (err)
 		return err;
 
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 83f84928ad76..eeb7075868db 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -420,7 +420,7 @@ void ima_post_path_mknod(struct dentry *dentry)
 }
 
 /**
- * ima_read_file - pre-measure/appraise hook decision based on policy
+ * ima_read_data - pre-measure/appraise hook decision based on policy
  * @file: pointer to the file to be measured/appraised/audit
  * @read_id: caller identifier
  *
@@ -430,7 +430,7 @@ void ima_post_path_mknod(struct dentry *dentry)
  *
  * For permission return 0, otherwise return -EACCES.
  */
-int ima_read_file(struct file *file, enum kernel_read_file_id read_id)
+int ima_read_data(struct file *file, enum kernel_read_file_id read_id)
 {
 	bool sig_enforce = is_module_sig_enforced();
 
diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
index 5fa191252c8f..8d7db638fdeb 100644
--- a/security/loadpin/loadpin.c
+++ b/security/loadpin/loadpin.c
@@ -175,7 +175,7 @@ static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
 
 static struct security_hook_list loadpin_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(sb_free_security, loadpin_sb_free_security),
-	LSM_HOOK_INIT(kernel_read_file, loadpin_read_file),
+	LSM_HOOK_INIT(kernel_read_data, loadpin_read_file),
 };
 
 void __init loadpin_add_hooks(void)
diff --git a/security/security.c b/security/security.c
index 68f46d849abe..fc7a2bcf3177 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1033,16 +1033,16 @@ int security_kernel_module_request(char *kmod_name)
 	return call_int_hook(kernel_module_request, 0, kmod_name);
 }
 
-int security_kernel_read_file(struct file *file, enum kernel_read_file_id id)
+int security_kernel_read_data(struct file *file, enum kernel_read_file_id id)
 {
 	int ret;
 
 	ret = call_int_hook(kernel_read_file, 0, file, id);
 	if (ret)
 		return ret;
-	return ima_read_file(file, id);
+	return ima_read_data(file, id);
 }
-EXPORT_SYMBOL_GPL(security_kernel_read_file);
+EXPORT_SYMBOL_GPL(security_kernel_read_data);
 
 int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
 				   enum kernel_read_file_id id)
-- 
2.7.5

--
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

[PATCH v3 1/7] security: rename security_kernel_read_file() hook

[Mimi Zohar]

In order for LSMs and IMA-appraisal to differentiate between the original
and new syscalls (eg. kexec, kernel modules, firmware), both the original
and new syscalls must call an LSM hook.

Commit 2e72d51b4ac3 ("security: introduce kernel_module_from_file hook")
introduced calling security_kernel_module_from_file() in both the original
and new syscalls.  Commit a1db74209483 ("module: replace
copy_module_from_fd with kernel version") replaced these LSM calls with
security_kernel_read_file().

Commit e40ba6d56b41 ("firmware: replace call to fw_read_file_contents()
with kernel version") and commit b804defe4297  ("kexec: replace call to
copy_file_from_fd() with kernel version") replaced their own version of
reading a file from the kernel with the generic
kernel_read_file_from_path/fd() versions, which call the pre and post
security_kernel_read_file LSM hooks.

Missing are LSM calls in the original kexec syscall and firmware sysfs
fallback method.  Instead of defining a new LSM hook or wrapper for
security_kernel_read_file(), this patch renames the original
security_kernel_read_file() hook to security_kernel_read_data(), and
updates LSM usage of the hook (eg. loadpin, init_module, IMA).

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Eric Biederman <ebiederm@xmission.com>
Cc: Luis R. Rodriguez <mcgrof@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Casey Schaufler <casey@schaufler-ca.com>

Changelog v3:
- Rename security_kernel_read_file to security_kernel_read_data().

Changelog v2:
- Define a generic wrapper named security_kernel_read_blob() for
security_kernel_read_file().

Changelog v1:
- Define and call security_kexec_load(), a wrapper for
security_kernel_read_file().
---
 fs/exec.c                         | 2 +-
 include/linux/ima.h               | 4 ++--
 include/linux/security.h          | 4 ++--
 kernel/module.c                   | 2 +-
 security/integrity/ima/ima_main.c | 4 ++--
 security/loadpin/loadpin.c        | 2 +-
 security/security.c               | 6 +++---
 7 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 183059c427b9..0c832b4c6a22 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -904,7 +904,7 @@ int kernel_read_file(struct file *file, void **buf, loff_t *size,
 	if (ret)
 		return ret;
 
-	ret = security_kernel_read_file(file, id);
+	ret = security_kernel_read_data(file, id);
 	if (ret)
 		goto out;
 
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 0e4647e0eb60..423aaf88f8c6 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -19,7 +19,7 @@ extern int ima_bprm_check(struct linux_binprm *bprm);
 extern int ima_file_check(struct file *file, int mask, int opened);
 extern void ima_file_free(struct file *file);
 extern int ima_file_mmap(struct file *file, unsigned long prot);
-extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
+extern int ima_read_data(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);
@@ -49,7 +49,7 @@ static inline int ima_file_mmap(struct file *file, unsigned long prot)
 	return 0;
 }
 
-static inline int ima_read_file(struct file *file, enum kernel_read_file_id id)
+static inline int ima_read_data(struct file *file, enum kernel_read_file_id id)
 {
 	return 0;
 }
diff --git a/include/linux/security.h b/include/linux/security.h
index 63030c85ee19..836a9081b2f3 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -320,7 +320,7 @@ void security_cred_getsecid(const struct cred *c, u32 *secid);
 int security_kernel_act_as(struct cred *new, u32 secid);
 int security_kernel_create_files_as(struct cred *new, struct inode *inode);
 int security_kernel_module_request(char *kmod_name);
-int security_kernel_read_file(struct file *file, enum kernel_read_file_id id);
+int security_kernel_read_data(struct file *file, enum kernel_read_file_id id);
 int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
 				   enum kernel_read_file_id id);
 int security_task_fix_setuid(struct cred *new, const struct cred *old,
@@ -909,7 +909,7 @@ static inline int security_kernel_module_request(char *kmod_name)
 	return 0;
 }
 
-static inline int security_kernel_read_file(struct file *file,
+static inline int security_kernel_read_data(struct file *file,
 					    enum kernel_read_file_id id)
 {
 	return 0;
diff --git a/kernel/module.c b/kernel/module.c
index ce8066b88178..cb84a0b7fbe9 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2879,7 +2879,7 @@ static int copy_module_from_user(const void __user *umod, unsigned long len,
 	if (info->len < sizeof(*(info->hdr)))
 		return -ENOEXEC;
 
-	err = security_kernel_read_file(NULL, READING_MODULE);
+	err = security_kernel_read_data(NULL, READING_MODULE);
 	if (err)
 		return err;
 
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 83f84928ad76..eeb7075868db 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -420,7 +420,7 @@ void ima_post_path_mknod(struct dentry *dentry)
 }
 
 /**
- * ima_read_file - pre-measure/appraise hook decision based on policy
+ * ima_read_data - pre-measure/appraise hook decision based on policy
  * @file: pointer to the file to be measured/appraised/audit
  * @read_id: caller identifier
  *
@@ -430,7 +430,7 @@ void ima_post_path_mknod(struct dentry *dentry)
  *
  * For permission return 0, otherwise return -EACCES.
  */
-int ima_read_file(struct file *file, enum kernel_read_file_id read_id)
+int ima_read_data(struct file *file, enum kernel_read_file_id read_id)
 {
 	bool sig_enforce = is_module_sig_enforced();
 
diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
index 5fa191252c8f..8d7db638fdeb 100644
--- a/security/loadpin/loadpin.c
+++ b/security/loadpin/loadpin.c
@@ -175,7 +175,7 @@ static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
 
 static struct security_hook_list loadpin_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(sb_free_security, loadpin_sb_free_security),
-	LSM_HOOK_INIT(kernel_read_file, loadpin_read_file),
+	LSM_HOOK_INIT(kernel_read_data, loadpin_read_file),
 };
 
 void __init loadpin_add_hooks(void)
diff --git a/security/security.c b/security/security.c
index 68f46d849abe..fc7a2bcf3177 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1033,16 +1033,16 @@ int security_kernel_module_request(char *kmod_name)
 	return call_int_hook(kernel_module_request, 0, kmod_name);
 }
 
-int security_kernel_read_file(struct file *file, enum kernel_read_file_id id)
+int security_kernel_read_data(struct file *file, enum kernel_read_file_id id)
 {
 	int ret;
 
 	ret = call_int_hook(kernel_read_file, 0, file, id);
 	if (ret)
 		return ret;
-	return ima_read_file(file, id);
+	return ima_read_data(file, id);
 }
-EXPORT_SYMBOL_GPL(security_kernel_read_file);
+EXPORT_SYMBOL_GPL(security_kernel_read_data);
 
 int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
 				   enum kernel_read_file_id id)
-- 
2.7.5


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v3 2/7] kexec: add call to LSM hook in original kexec_load syscall

[Mimi Zohar]

In order for LSMs and IMA-appraisal to differentiate between the
original and new syscalls, both the original and new syscalls must call
an LSM hook.  This patch adds a call to security_kernel_read_data() in
the original kexec syscall.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Eric Biederman <ebiederm@xmission.com>
Cc: Luis R. Rodriguez <mcgrof@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: David Howells <dhowells@redhat.com>
---
 kernel/kexec.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/kernel/kexec.c b/kernel/kexec.c
index aed8fb2564b3..061ada41c18c 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -11,6 +11,7 @@
 #include <linux/capability.h>
 #include <linux/mm.h>
 #include <linux/file.h>
+#include <linux/security.h>
 #include <linux/kexec.h>
 #include <linux/mutex.h>
 #include <linux/list.h>
@@ -195,10 +196,17 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
 static inline int kexec_load_check(unsigned long nr_segments,
 				   unsigned long flags)
 {
+	int result;
+
 	/* We only trust the superuser with rebooting the system. */
 	if (!capable(CAP_SYS_BOOT) || kexec_load_disabled)
 		return -EPERM;
 
+	/* Permit LSMs and IMA to fail the kexec */
+	result = security_kernel_read_data(NULL, READING_KEXEC_IMAGE);
+	if (result < 0)
+		return result;
+
 	/*
 	 * Verify we have a legal set of flags
 	 * This leaves us room for future extensions.
-- 
2.7.5

[PATCH v3 2/7] kexec: add call to LSM hook in original kexec_load syscall

[Mimi Zohar]

In order for LSMs and IMA-appraisal to differentiate between the
original and new syscalls, both the original and new syscalls must call
an LSM hook.  This patch adds a call to security_kernel_read_data() in
the original kexec syscall.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Eric Biederman <ebiederm@xmission.com>
Cc: Luis R. Rodriguez <mcgrof@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: David Howells <dhowells@redhat.com>
---
 kernel/kexec.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/kernel/kexec.c b/kernel/kexec.c
index aed8fb2564b3..061ada41c18c 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -11,6 +11,7 @@
 #include <linux/capability.h>
 #include <linux/mm.h>
 #include <linux/file.h>
+#include <linux/security.h>
 #include <linux/kexec.h>
 #include <linux/mutex.h>
 #include <linux/list.h>
@@ -195,10 +196,17 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
 static inline int kexec_load_check(unsigned long nr_segments,
 				   unsigned long flags)
 {
+	int result;
+
 	/* We only trust the superuser with rebooting the system. */
 	if (!capable(CAP_SYS_BOOT) || kexec_load_disabled)
 		return -EPERM;
 
+	/* Permit LSMs and IMA to fail the kexec */
+	result = security_kernel_read_data(NULL, READING_KEXEC_IMAGE);
+	if (result < 0)
+		return result;
+
 	/*
 	 * Verify we have a legal set of flags
 	 * This leaves us room for future extensions.
-- 
2.7.5

--
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@ http://vger.kernel.org/majordomo-info.html

[PATCH v3 2/7] kexec: add call to LSM hook in original kexec_load syscall

[Mimi Zohar]

In order for LSMs and IMA-appraisal to differentiate between the
original and new syscalls, both the original and new syscalls must call
an LSM hook.  This patch adds a call to security_kernel_read_data() in
the original kexec syscall.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Eric Biederman <ebiederm@xmission.com>
Cc: Luis R. Rodriguez <mcgrof@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: David Howells <dhowells@redhat.com>
---
 kernel/kexec.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/kernel/kexec.c b/kernel/kexec.c
index aed8fb2564b3..061ada41c18c 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -11,6 +11,7 @@
 #include <linux/capability.h>
 #include <linux/mm.h>
 #include <linux/file.h>
+#include <linux/security.h>
 #include <linux/kexec.h>
 #include <linux/mutex.h>
 #include <linux/list.h>
@@ -195,10 +196,17 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
 static inline int kexec_load_check(unsigned long nr_segments,
 				   unsigned long flags)
 {
+	int result;
+
 	/* We only trust the superuser with rebooting the system. */
 	if (!capable(CAP_SYS_BOOT) || kexec_load_disabled)
 		return -EPERM;
 
+	/* Permit LSMs and IMA to fail the kexec */
+	result = security_kernel_read_data(NULL, READING_KEXEC_IMAGE);
+	if (result < 0)
+		return result;
+
 	/*
 	 * Verify we have a legal set of flags
 	 * This leaves us room for future extensions.
-- 
2.7.5


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v3 3/7] ima: based on policy require signed kexec kernel images

[Mimi Zohar]

The original kexec_load syscall can not verify file signatures.  This
patch differentiates between the kexec_load and kexec_file_load
syscalls.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Eric Biederman <ebiederm@xmission.com>
Cc: Luis R. Rodriguez <mcgrof@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: David Howells <dhowells@redhat.com>

Changelog v3:
- use switch/case
---
 security/integrity/ima/ima.h        |  1 +
 security/integrity/ima/ima_main.c   | 22 +++++++++++++++++-----
 security/integrity/ima/ima_policy.c |  2 ++
 3 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 354bb5716ce3..78c15264b17b 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -232,6 +232,7 @@ int ima_policy_show(struct seq_file *m, void *v);
 #define IMA_APPRAISE_MODULES	0x08
 #define IMA_APPRAISE_FIRMWARE	0x10
 #define IMA_APPRAISE_POLICY	0x20
+#define IMA_APPRAISE_KEXEC	0x40
 
 #ifdef CONFIG_IMA_APPRAISE
 int ima_appraise_measurement(enum ima_hooks func,
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index eeb7075868db..fbbcc02a1380 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -432,15 +432,27 @@ void ima_post_path_mknod(struct dentry *dentry)
  */
 int ima_read_data(struct file *file, enum kernel_read_file_id read_id)
 {
-	bool sig_enforce = is_module_sig_enforced();
+	bool sig_enforce;
 
-	if (!file && read_id == READING_MODULE) {
-		if (!sig_enforce && (ima_appraise & IMA_APPRAISE_MODULES) &&
-		    (ima_appraise & IMA_APPRAISE_ENFORCE)) {
+	if ((ima_appraise & IMA_APPRAISE_ENFORCE) != IMA_APPRAISE_ENFORCE)
+		return 0;
+
+	switch (read_id) {
+	case READING_MODULE:
+		sig_enforce = is_module_sig_enforced(); /* appended sig */
+		if (!file && !sig_enforce &&
+		    (ima_appraise & IMA_APPRAISE_MODULES)) {
 			pr_err("impossible to appraise a module without a file descriptor. sig_enforce kernel parameter might help\n");
 			return -EACCES;	/* INTEGRITY_UNKNOWN */
 		}
-		return 0;	/* We rely on module signature checking */
+		break;
+	case READING_KEXEC_IMAGE:
+		if (!file && (ima_appraise & IMA_APPRAISE_KEXEC)) {
+			pr_err("impossible to appraise a kernel image without a file descriptor; try using kexec_file syscall.\n");
+			return -EACCES;	/* INTEGRITY_UNKNOWN */
+		}
+	default:
+		break;
 	}
 	return 0;
 }
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 8bbc18eb07eb..c27f6993b07a 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -448,6 +448,8 @@ static int ima_appraise_flag(enum ima_hooks func)
 		return IMA_APPRAISE_FIRMWARE;
 	else if (func == POLICY_CHECK)
 		return IMA_APPRAISE_POLICY;
+	else if (func == KEXEC_KERNEL_CHECK)
+		return IMA_APPRAISE_KEXEC;
 	return 0;
 }
 
-- 
2.7.5

[PATCH v3 3/7] ima: based on policy require signed kexec kernel images

[Mimi Zohar]

The original kexec_load syscall can not verify file signatures.  This
patch differentiates between the kexec_load and kexec_file_load
syscalls.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Eric Biederman <ebiederm@xmission.com>
Cc: Luis R. Rodriguez <mcgrof@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: David Howells <dhowells@redhat.com>

Changelog v3:
- use switch/case
---
 security/integrity/ima/ima.h        |  1 +
 security/integrity/ima/ima_main.c   | 22 +++++++++++++++++-----
 security/integrity/ima/ima_policy.c |  2 ++
 3 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 354bb5716ce3..78c15264b17b 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -232,6 +232,7 @@ int ima_policy_show(struct seq_file *m, void *v);
 #define IMA_APPRAISE_MODULES	0x08
 #define IMA_APPRAISE_FIRMWARE	0x10
 #define IMA_APPRAISE_POLICY	0x20
+#define IMA_APPRAISE_KEXEC	0x40
 
 #ifdef CONFIG_IMA_APPRAISE
 int ima_appraise_measurement(enum ima_hooks func,
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index eeb7075868db..fbbcc02a1380 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -432,15 +432,27 @@ void ima_post_path_mknod(struct dentry *dentry)
  */
 int ima_read_data(struct file *file, enum kernel_read_file_id read_id)
 {
-	bool sig_enforce = is_module_sig_enforced();
+	bool sig_enforce;
 
-	if (!file && read_id == READING_MODULE) {
-		if (!sig_enforce && (ima_appraise & IMA_APPRAISE_MODULES) &&
-		    (ima_appraise & IMA_APPRAISE_ENFORCE)) {
+	if ((ima_appraise & IMA_APPRAISE_ENFORCE) != IMA_APPRAISE_ENFORCE)
+		return 0;
+
+	switch (read_id) {
+	case READING_MODULE:
+		sig_enforce = is_module_sig_enforced(); /* appended sig */
+		if (!file && !sig_enforce &&
+		    (ima_appraise & IMA_APPRAISE_MODULES)) {
 			pr_err("impossible to appraise a module without a file descriptor. sig_enforce kernel parameter might help\n");
 			return -EACCES;	/* INTEGRITY_UNKNOWN */
 		}
-		return 0;	/* We rely on module signature checking */
+		break;
+	case READING_KEXEC_IMAGE:
+		if (!file && (ima_appraise & IMA_APPRAISE_KEXEC)) {
+			pr_err("impossible to appraise a kernel image without a file descriptor; try using kexec_file syscall.\n");
+			return -EACCES;	/* INTEGRITY_UNKNOWN */
+		}
+	default:
+		break;
 	}
 	return 0;
 }
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 8bbc18eb07eb..c27f6993b07a 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -448,6 +448,8 @@ static int ima_appraise_flag(enum ima_hooks func)
 		return IMA_APPRAISE_FIRMWARE;
 	else if (func == POLICY_CHECK)
 		return IMA_APPRAISE_POLICY;
+	else if (func == KEXEC_KERNEL_CHECK)
+		return IMA_APPRAISE_KEXEC;
 	return 0;
 }
 
-- 
2.7.5

--
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

[PATCH v3 3/7] ima: based on policy require signed kexec kernel images

[Mimi Zohar]

The original kexec_load syscall can not verify file signatures.  This
patch differentiates between the kexec_load and kexec_file_load
syscalls.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Eric Biederman <ebiederm@xmission.com>
Cc: Luis R. Rodriguez <mcgrof@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: David Howells <dhowells@redhat.com>

Changelog v3:
- use switch/case
---
 security/integrity/ima/ima.h        |  1 +
 security/integrity/ima/ima_main.c   | 22 +++++++++++++++++-----
 security/integrity/ima/ima_policy.c |  2 ++
 3 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 354bb5716ce3..78c15264b17b 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -232,6 +232,7 @@ int ima_policy_show(struct seq_file *m, void *v);
 #define IMA_APPRAISE_MODULES	0x08
 #define IMA_APPRAISE_FIRMWARE	0x10
 #define IMA_APPRAISE_POLICY	0x20
+#define IMA_APPRAISE_KEXEC	0x40
 
 #ifdef CONFIG_IMA_APPRAISE
 int ima_appraise_measurement(enum ima_hooks func,
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index eeb7075868db..fbbcc02a1380 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -432,15 +432,27 @@ void ima_post_path_mknod(struct dentry *dentry)
  */
 int ima_read_data(struct file *file, enum kernel_read_file_id read_id)
 {
-	bool sig_enforce = is_module_sig_enforced();
+	bool sig_enforce;
 
-	if (!file && read_id == READING_MODULE) {
-		if (!sig_enforce && (ima_appraise & IMA_APPRAISE_MODULES) &&
-		    (ima_appraise & IMA_APPRAISE_ENFORCE)) {
+	if ((ima_appraise & IMA_APPRAISE_ENFORCE) != IMA_APPRAISE_ENFORCE)
+		return 0;
+
+	switch (read_id) {
+	case READING_MODULE:
+		sig_enforce = is_module_sig_enforced(); /* appended sig */
+		if (!file && !sig_enforce &&
+		    (ima_appraise & IMA_APPRAISE_MODULES)) {
 			pr_err("impossible to appraise a module without a file descriptor. sig_enforce kernel parameter might help\n");
 			return -EACCES;	/* INTEGRITY_UNKNOWN */
 		}
-		return 0;	/* We rely on module signature checking */
+		break;
+	case READING_KEXEC_IMAGE:
+		if (!file && (ima_appraise & IMA_APPRAISE_KEXEC)) {
+			pr_err("impossible to appraise a kernel image without a file descriptor; try using kexec_file syscall.\n");
+			return -EACCES;	/* INTEGRITY_UNKNOWN */
+		}
+	default:
+		break;
 	}
 	return 0;
 }
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 8bbc18eb07eb..c27f6993b07a 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -448,6 +448,8 @@ static int ima_appraise_flag(enum ima_hooks func)
 		return IMA_APPRAISE_FIRMWARE;
 	else if (func == POLICY_CHECK)
 		return IMA_APPRAISE_POLICY;
+	else if (func == KEXEC_KERNEL_CHECK)
+		return IMA_APPRAISE_KEXEC;
 	return 0;
 }
 
-- 
2.7.5


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v3 4/7] firmware: add call to LSM hook before firmware sysfs fallback

[Mimi Zohar]

Add an LSM hook prior to allowing firmware sysfs fallback loading.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Luis R. Rodriguez <mcgrof@suse.com>
Cc: David Howells <dhowells@redhat.com>
Cc: Kees Cook <keescook@chromium.org>

Changelog:
- call security_kernel_read_blob()
- rename the READING_FIRMWARE_FALLBACK kernel_read_file_id enumeration to
READING_FIRMWARE_FALLBACK_SYSFS.
---
 drivers/base/firmware_loader/fallback.c | 7 +++++++
 include/linux/fs.h                      | 1 +
 2 files changed, 8 insertions(+)

diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index 358354148dec..ae102efcc9f0 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -651,6 +651,8 @@ static bool fw_force_sysfs_fallback(unsigned int opt_flags)
 
 static bool fw_run_sysfs_fallback(unsigned int opt_flags)
 {
+	int ret;
+
 	if (fw_fallback_config.ignore_sysfs_fallback) {
 		pr_info_once("Ignoring firmware sysfs fallback due to sysctl knob\n");
 		return false;
@@ -659,6 +661,11 @@ static bool fw_run_sysfs_fallback(unsigned int opt_flags)
 	if ((opt_flags & FW_OPT_NOFALLBACK))
 		return false;
 
+	/* Also permit LSMs and IMA to fail firmware sysfs fallback */
+	ret = security_kernel_read_data(NULL, READING_FIRMWARE_FALLBACK_SYSFS);
+	if (ret < 0)
+		return ret;
+
 	return fw_force_sysfs_fallback(opt_flags);
 }
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 760d8da1b6c7..6e31d9207435 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2810,6 +2810,7 @@ extern int do_pipe_flags(int *, int);
 	id(UNKNOWN, unknown)		\
 	id(FIRMWARE, firmware)		\
 	id(FIRMWARE_PREALLOC_BUFFER, firmware)	\
+	id(FIRMWARE_FALLBACK_SYSFS, firmware)	\
 	id(MODULE, kernel-module)		\
 	id(KEXEC_IMAGE, kexec-image)		\
 	id(KEXEC_INITRAMFS, kexec-initramfs)	\
-- 
2.7.5

[PATCH v3 4/7] firmware: add call to LSM hook before firmware sysfs fallback

[Mimi Zohar]

Add an LSM hook prior to allowing firmware sysfs fallback loading.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Luis R. Rodriguez <mcgrof@suse.com>
Cc: David Howells <dhowells@redhat.com>
Cc: Kees Cook <keescook@chromium.org>

Changelog:
- call security_kernel_read_blob()
- rename the READING_FIRMWARE_FALLBACK kernel_read_file_id enumeration to
READING_FIRMWARE_FALLBACK_SYSFS.
---
 drivers/base/firmware_loader/fallback.c | 7 +++++++
 include/linux/fs.h                      | 1 +
 2 files changed, 8 insertions(+)

diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index 358354148dec..ae102efcc9f0 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -651,6 +651,8 @@ static bool fw_force_sysfs_fallback(unsigned int opt_flags)
 
 static bool fw_run_sysfs_fallback(unsigned int opt_flags)
 {
+	int ret;
+
 	if (fw_fallback_config.ignore_sysfs_fallback) {
 		pr_info_once("Ignoring firmware sysfs fallback due to sysctl knob\n");
 		return false;
@@ -659,6 +661,11 @@ static bool fw_run_sysfs_fallback(unsigned int opt_flags)
 	if ((opt_flags & FW_OPT_NOFALLBACK))
 		return false;
 
+	/* Also permit LSMs and IMA to fail firmware sysfs fallback */
+	ret = security_kernel_read_data(NULL, READING_FIRMWARE_FALLBACK_SYSFS);
+	if (ret < 0)
+		return ret;
+
 	return fw_force_sysfs_fallback(opt_flags);
 }
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 760d8da1b6c7..6e31d9207435 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2810,6 +2810,7 @@ extern int do_pipe_flags(int *, int);
 	id(UNKNOWN, unknown)		\
 	id(FIRMWARE, firmware)		\
 	id(FIRMWARE_PREALLOC_BUFFER, firmware)	\
+	id(FIRMWARE_FALLBACK_SYSFS, firmware)	\
 	id(MODULE, kernel-module)		\
 	id(KEXEC_IMAGE, kexec-image)		\
 	id(KEXEC_INITRAMFS, kexec-initramfs)	\
-- 
2.7.5

--
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@ http://vger.kernel.org/majordomo-info.html

[PATCH v3 4/7] firmware: add call to LSM hook before firmware sysfs fallback

[Mimi Zohar]

Add an LSM hook prior to allowing firmware sysfs fallback loading.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Luis R. Rodriguez <mcgrof@suse.com>
Cc: David Howells <dhowells@redhat.com>
Cc: Kees Cook <keescook@chromium.org>

Changelog:
- call security_kernel_read_blob()
- rename the READING_FIRMWARE_FALLBACK kernel_read_file_id enumeration to
READING_FIRMWARE_FALLBACK_SYSFS.
---
 drivers/base/firmware_loader/fallback.c | 7 +++++++
 include/linux/fs.h                      | 1 +
 2 files changed, 8 insertions(+)

diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index 358354148dec..ae102efcc9f0 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -651,6 +651,8 @@ static bool fw_force_sysfs_fallback(unsigned int opt_flags)
 
 static bool fw_run_sysfs_fallback(unsigned int opt_flags)
 {
+	int ret;
+
 	if (fw_fallback_config.ignore_sysfs_fallback) {
 		pr_info_once("Ignoring firmware sysfs fallback due to sysctl knob\n");
 		return false;
@@ -659,6 +661,11 @@ static bool fw_run_sysfs_fallback(unsigned int opt_flags)
 	if ((opt_flags & FW_OPT_NOFALLBACK))
 		return false;
 
+	/* Also permit LSMs and IMA to fail firmware sysfs fallback */
+	ret = security_kernel_read_data(NULL, READING_FIRMWARE_FALLBACK_SYSFS);
+	if (ret < 0)
+		return ret;
+
 	return fw_force_sysfs_fallback(opt_flags);
 }
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 760d8da1b6c7..6e31d9207435 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2810,6 +2810,7 @@ extern int do_pipe_flags(int *, int);
 	id(UNKNOWN, unknown)		\
 	id(FIRMWARE, firmware)		\
 	id(FIRMWARE_PREALLOC_BUFFER, firmware)	\
+	id(FIRMWARE_FALLBACK_SYSFS, firmware)	\
 	id(MODULE, kernel-module)		\
 	id(KEXEC_IMAGE, kexec-image)		\
 	id(KEXEC_INITRAMFS, kexec-initramfs)	\
-- 
2.7.5


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v3 5/7] ima: based on policy require signed firmware (sysfs fallback)

[Mimi Zohar]

With an IMA policy requiring signed firmware, this patch prevents
the sysfs fallback method of loading firmware.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Luis R. Rodriguez <mcgrof@suse.com>
Cc: David Howells <dhowells@redhat.com>
Cc: Matthew Garrett <mjg59@google.com>
---
 security/integrity/ima/ima_main.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index fbbcc02a1380..dd1f263f950a 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -451,10 +451,17 @@ int ima_read_data(struct file *file, enum kernel_read_file_id read_id)
 			pr_err("impossible to appraise a kernel image without a file descriptor; try using kexec_file syscall.\n");
 			return -EACCES;	/* INTEGRITY_UNKNOWN */
 		}
+		break;
+	case READING_FIRMWARE_FALLBACK_SYSFS:
+		if (ima_appraise & IMA_APPRAISE_FIRMWARE) {
+			pr_err("Prevent firmware sysfs fallback loading.\n");
+			return -EACCES;	/* INTEGRITY_UNKNOWN */
+		}
 	default:
 		break;
 	}
 	return 0;
+
 }
 
 static int read_idmap[READING_MAX_ID] = {
-- 
2.7.5

[PATCH v3 5/7] ima: based on policy require signed firmware (sysfs fallback)

[Mimi Zohar]

With an IMA policy requiring signed firmware, this patch prevents
the sysfs fallback method of loading firmware.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Luis R. Rodriguez <mcgrof@suse.com>
Cc: David Howells <dhowells@redhat.com>
Cc: Matthew Garrett <mjg59@google.com>
---
 security/integrity/ima/ima_main.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index fbbcc02a1380..dd1f263f950a 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -451,10 +451,17 @@ int ima_read_data(struct file *file, enum kernel_read_file_id read_id)
 			pr_err("impossible to appraise a kernel image without a file descriptor; try using kexec_file syscall.\n");
 			return -EACCES;	/* INTEGRITY_UNKNOWN */
 		}
+		break;
+	case READING_FIRMWARE_FALLBACK_SYSFS:
+		if (ima_appraise & IMA_APPRAISE_FIRMWARE) {
+			pr_err("Prevent firmware sysfs fallback loading.\n");
+			return -EACCES;	/* INTEGRITY_UNKNOWN */
+		}
 	default:
 		break;
 	}
 	return 0;
+
 }
 
 static int read_idmap[READING_MAX_ID] = {
-- 
2.7.5

--
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

[PATCH v3 5/7] ima: based on policy require signed firmware (sysfs fallback)

[Mimi Zohar]

With an IMA policy requiring signed firmware, this patch prevents
the sysfs fallback method of loading firmware.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Luis R. Rodriguez <mcgrof@suse.com>
Cc: David Howells <dhowells@redhat.com>
Cc: Matthew Garrett <mjg59@google.com>
---
 security/integrity/ima/ima_main.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index fbbcc02a1380..dd1f263f950a 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -451,10 +451,17 @@ int ima_read_data(struct file *file, enum kernel_read_file_id read_id)
 			pr_err("impossible to appraise a kernel image without a file descriptor; try using kexec_file syscall.\n");
 			return -EACCES;	/* INTEGRITY_UNKNOWN */
 		}
+		break;
+	case READING_FIRMWARE_FALLBACK_SYSFS:
+		if (ima_appraise & IMA_APPRAISE_FIRMWARE) {
+			pr_err("Prevent firmware sysfs fallback loading.\n");
+			return -EACCES;	/* INTEGRITY_UNKNOWN */
+		}
 	default:
 		break;
 	}
 	return 0;
+
 }
 
 static int read_idmap[READING_MAX_ID] = {
-- 
2.7.5


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v3 6/7] ima: add build time policy

[Mimi Zohar]

IMA by default does not measure, appraise or audit files, but can be
enabled at runtime by specifying a builtin policy on the boot command line
or by loading a custom policy.

This patch defines a build time policy, which verifies kernel modules,
firmware, kexec image, and/or the IMA policy signatures.  This build time
policy is automatically enabled at runtime.  The build time policy rules
persist after loading a custom policy.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 security/integrity/ima/Kconfig      | 58 +++++++++++++++++++++++++++++++++++++
 security/integrity/ima/ima_policy.c | 46 +++++++++++++++++++++++++++--
 2 files changed, 101 insertions(+), 3 deletions(-)

diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 6a8f67714c83..004919d9bf09 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -156,6 +156,64 @@ config IMA_APPRAISE
 	  <http://linux-ima.sourceforge.net>
 	  If unsure, say N.
 
+config IMA_APPRAISE_BUILD_POLICY
+	bool "IMA build time configured policy rules"
+	depends on IMA_APPRAISE && INTEGRITY_ASYMMETRIC_KEYS
+	default n
+	help
+	  This option defines an IMA appraisal policy at build time, which
+	  is enforced at run time without having to specify a builtin
+	  policy name on the boot command line.  The build time appraisal
+	  policy rules persist after loading a custom policy.
+
+	  Depending on the rules configured, this policy may require kernel
+	  modules, firmware, the kexec kernel image, and/or the IMA policy
+	  to be signed.  Unsigned files might prevent the system from
+	  booting or applications from working properly.
+
+config IMA_APPRAISE_REQUIRE_FIRMWARE_SIGS
+	bool "Appraise firmware signatures"
+	depends on IMA_APPRAISE_BUILD_POLICY
+	default n
+	help
+	  This option defines a policy requiring all firmware to be signed,
+	  including the regulatory.db.  If both this option and
+	  CFG80211_REQUIRE_SIGNED_REGDB are enabled, then both signature
+	  verification methods are necessary.
+
+config IMA_APPRAISE_REQUIRE_KEXEC_SIGS
+	bool "Appraise kexec kernel image signatures"
+	depends on IMA_APPRAISE_BUILD_POLICY
+	default n
+	help
+	  Enabling this rule will require all kexec'ed kernel images to
+	  be signed and verified by a public key on the trusted IMA
+	  keyring.
+
+	  Kernel image signatures can not be verified by the original
+	  kexec_load syscall.  Enabling this rule will prevent its
+	  usage.
+
+config IMA_APPRAISE_REQUIRE_MODULE_SIGS
+	bool "Appraise kernel modules signatures"
+	depends on IMA_APPRAISE_BUILD_POLICY
+	default n
+	help
+	  Enabling this rule will require all kernel modules to be signed
+	  and verified by a public key on the trusted IMA keyring.
+
+	  Kernel module signatures can only be verified by IMA-appraisal,
+	  via the finit_module syscall. Enabling this rule will prevent
+	  the usage of the init_module syscall.
+
+config IMA_APPRAISE_REQUIRE_POLICY_SIGS
+	bool "Appraise IMA policy signature"
+	depends on IMA_APPRAISE_BUILD_POLICY
+	default n
+	help
+	  Enabling this rule will require the IMA policy to be signed and
+	  and verified by a key on the trusted IMA keyring.
+
 config IMA_APPRAISE_BOOTPARAM
 	bool "ima_appraise boot parameter"
 	depends on IMA_APPRAISE
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index c27f6993b07a..3c0bc8a1a88e 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -49,6 +49,7 @@
 
 int ima_policy_flag;
 static int temp_ima_appraise;
+static int build_ima_appraise __ro_after_init;
 
 #define MAX_LSM_RULES 6
 enum lsm_rule_types { LSM_OBJ_USER, LSM_OBJ_ROLE, LSM_OBJ_TYPE,
@@ -162,6 +163,25 @@ static struct ima_rule_entry default_appraise_rules[] __ro_after_init = {
 #endif
 };
 
+static struct ima_rule_entry build_appraise_rules[] __ro_after_init = {
+#ifdef CONFIG_IMA_APPRAISE_REQUIRE_MODULE_SIGS
+	{.action = APPRAISE, .func = MODULE_CHECK,
+	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
+#endif
+#ifdef CONFIG_IMA_APPRAISE_REQUIRE_FIRMWARE_SIGS
+	{.action = APPRAISE, .func = FIRMWARE_CHECK,
+	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
+#endif
+#ifdef CONFIG_IMA_APPRAISE_REQUIRE_KEXEC_SIGS
+	{.action = APPRAISE, .func = KEXEC_KERNEL_CHECK,
+	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
+#endif
+#ifdef CONFIG_IMA_APPRAISE_REQUIRE_POLICY_SIGS
+	{.action = APPRAISE, .func = POLICY_CHECK,
+	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
+#endif
+};
+
 static struct ima_rule_entry secure_boot_rules[] __ro_after_init = {
 	{.action = APPRAISE, .func = MODULE_CHECK,
 	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
@@ -435,7 +455,7 @@ void ima_update_policy_flag(void)
 			ima_policy_flag |= entry->action;
 	}
 
-	ima_appraise |= temp_ima_appraise;
+	ima_appraise |= (build_ima_appraise | temp_ima_appraise);
 	if (!ima_appraise)
 		ima_policy_flag &= ~IMA_APPRAISE;
 }
@@ -488,8 +508,8 @@ void __init ima_init_policy(void)
 	}
 
 	/*
-	 * Insert the appraise rules requiring file signatures, prior to
-	 * any other appraise rules.
+	 * Insert the builtin "secure_boot" policy rules requiring file
+	 * signatures, prior to any other appraise rules.
 	 */
 	for (i = 0; i < secure_boot_entries; i++) {
 		list_add_tail(&secure_boot_rules[i].list, &ima_default_rules);
@@ -497,6 +517,26 @@ void __init ima_init_policy(void)
 		    ima_appraise_flag(secure_boot_rules[i].func);
 	}
 
+	/*
+	 * Insert the build time appraise rules requiring file signatures
+	 * for both the initial and custom policies, prior to other appraise
+	 * rules.
+	 */
+	for (i = 0; i < ARRAY_SIZE(build_appraise_rules); i++) {
+		struct ima_rule_entry *entry;
+
+		if (!secure_boot_entries)
+			list_add_tail(&build_appraise_rules[i].list,
+				      &ima_default_rules);
+
+		entry = kmemdup(&build_appraise_rules[i], sizeof(*entry),
+				GFP_KERNEL);
+		if (entry)
+			list_add_tail(&entry->list, &ima_policy_rules);
+		build_ima_appraise |=
+			ima_appraise_flag(build_appraise_rules[i].func);
+	}
+
 	for (i = 0; i < appraise_entries; i++) {
 		list_add_tail(&default_appraise_rules[i].list,
 			      &ima_default_rules);
-- 
2.7.5

[PATCH v3 6/7] ima: add build time policy

[Mimi Zohar]

IMA by default does not measure, appraise or audit files, but can be
enabled at runtime by specifying a builtin policy on the boot command line
or by loading a custom policy.

This patch defines a build time policy, which verifies kernel modules,
firmware, kexec image, and/or the IMA policy signatures.  This build time
policy is automatically enabled at runtime.  The build time policy rules
persist after loading a custom policy.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 security/integrity/ima/Kconfig      | 58 +++++++++++++++++++++++++++++++++++++
 security/integrity/ima/ima_policy.c | 46 +++++++++++++++++++++++++++--
 2 files changed, 101 insertions(+), 3 deletions(-)

diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 6a8f67714c83..004919d9bf09 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -156,6 +156,64 @@ config IMA_APPRAISE
 	  <http://linux-ima.sourceforge.net>
 	  If unsure, say N.
 
+config IMA_APPRAISE_BUILD_POLICY
+	bool "IMA build time configured policy rules"
+	depends on IMA_APPRAISE && INTEGRITY_ASYMMETRIC_KEYS
+	default n
+	help
+	  This option defines an IMA appraisal policy at build time, which
+	  is enforced at run time without having to specify a builtin
+	  policy name on the boot command line.  The build time appraisal
+	  policy rules persist after loading a custom policy.
+
+	  Depending on the rules configured, this policy may require kernel
+	  modules, firmware, the kexec kernel image, and/or the IMA policy
+	  to be signed.  Unsigned files might prevent the system from
+	  booting or applications from working properly.
+
+config IMA_APPRAISE_REQUIRE_FIRMWARE_SIGS
+	bool "Appraise firmware signatures"
+	depends on IMA_APPRAISE_BUILD_POLICY
+	default n
+	help
+	  This option defines a policy requiring all firmware to be signed,
+	  including the regulatory.db.  If both this option and
+	  CFG80211_REQUIRE_SIGNED_REGDB are enabled, then both signature
+	  verification methods are necessary.
+
+config IMA_APPRAISE_REQUIRE_KEXEC_SIGS
+	bool "Appraise kexec kernel image signatures"
+	depends on IMA_APPRAISE_BUILD_POLICY
+	default n
+	help
+	  Enabling this rule will require all kexec'ed kernel images to
+	  be signed and verified by a public key on the trusted IMA
+	  keyring.
+
+	  Kernel image signatures can not be verified by the original
+	  kexec_load syscall.  Enabling this rule will prevent its
+	  usage.
+
+config IMA_APPRAISE_REQUIRE_MODULE_SIGS
+	bool "Appraise kernel modules signatures"
+	depends on IMA_APPRAISE_BUILD_POLICY
+	default n
+	help
+	  Enabling this rule will require all kernel modules to be signed
+	  and verified by a public key on the trusted IMA keyring.
+
+	  Kernel module signatures can only be verified by IMA-appraisal,
+	  via the finit_module syscall. Enabling this rule will prevent
+	  the usage of the init_module syscall.
+
+config IMA_APPRAISE_REQUIRE_POLICY_SIGS
+	bool "Appraise IMA policy signature"
+	depends on IMA_APPRAISE_BUILD_POLICY
+	default n
+	help
+	  Enabling this rule will require the IMA policy to be signed and
+	  and verified by a key on the trusted IMA keyring.
+
 config IMA_APPRAISE_BOOTPARAM
 	bool "ima_appraise boot parameter"
 	depends on IMA_APPRAISE
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index c27f6993b07a..3c0bc8a1a88e 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -49,6 +49,7 @@
 
 int ima_policy_flag;
 static int temp_ima_appraise;
+static int build_ima_appraise __ro_after_init;
 
 #define MAX_LSM_RULES 6
 enum lsm_rule_types { LSM_OBJ_USER, LSM_OBJ_ROLE, LSM_OBJ_TYPE,
@@ -162,6 +163,25 @@ static struct ima_rule_entry default_appraise_rules[] __ro_after_init = {
 #endif
 };
 
+static struct ima_rule_entry build_appraise_rules[] __ro_after_init = {
+#ifdef CONFIG_IMA_APPRAISE_REQUIRE_MODULE_SIGS
+	{.action = APPRAISE, .func = MODULE_CHECK,
+	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
+#endif
+#ifdef CONFIG_IMA_APPRAISE_REQUIRE_FIRMWARE_SIGS
+	{.action = APPRAISE, .func = FIRMWARE_CHECK,
+	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
+#endif
+#ifdef CONFIG_IMA_APPRAISE_REQUIRE_KEXEC_SIGS
+	{.action = APPRAISE, .func = KEXEC_KERNEL_CHECK,
+	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
+#endif
+#ifdef CONFIG_IMA_APPRAISE_REQUIRE_POLICY_SIGS
+	{.action = APPRAISE, .func = POLICY_CHECK,
+	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
+#endif
+};
+
 static struct ima_rule_entry secure_boot_rules[] __ro_after_init = {
 	{.action = APPRAISE, .func = MODULE_CHECK,
 	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
@@ -435,7 +455,7 @@ void ima_update_policy_flag(void)
 			ima_policy_flag |= entry->action;
 	}
 
-	ima_appraise |= temp_ima_appraise;
+	ima_appraise |= (build_ima_appraise | temp_ima_appraise);
 	if (!ima_appraise)
 		ima_policy_flag &= ~IMA_APPRAISE;
 }
@@ -488,8 +508,8 @@ void __init ima_init_policy(void)
 	}
 
 	/*
-	 * Insert the appraise rules requiring file signatures, prior to
-	 * any other appraise rules.
+	 * Insert the builtin "secure_boot" policy rules requiring file
+	 * signatures, prior to any other appraise rules.
 	 */
 	for (i = 0; i < secure_boot_entries; i++) {
 		list_add_tail(&secure_boot_rules[i].list, &ima_default_rules);
@@ -497,6 +517,26 @@ void __init ima_init_policy(void)
 		    ima_appraise_flag(secure_boot_rules[i].func);
 	}
 
+	/*
+	 * Insert the build time appraise rules requiring file signatures
+	 * for both the initial and custom policies, prior to other appraise
+	 * rules.
+	 */
+	for (i = 0; i < ARRAY_SIZE(build_appraise_rules); i++) {
+		struct ima_rule_entry *entry;
+
+		if (!secure_boot_entries)
+			list_add_tail(&build_appraise_rules[i].list,
+				      &ima_default_rules);
+
+		entry = kmemdup(&build_appraise_rules[i], sizeof(*entry),
+				GFP_KERNEL);
+		if (entry)
+			list_add_tail(&entry->list, &ima_policy_rules);
+		build_ima_appraise |=
+			ima_appraise_flag(build_appraise_rules[i].func);
+	}
+
 	for (i = 0; i < appraise_entries; i++) {
 		list_add_tail(&default_appraise_rules[i].list,
 			      &ima_default_rules);
-- 
2.7.5

--
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@ http://vger.kernel.org/majordomo-info.html

[PATCH v3 6/7] ima: add build time policy

[Mimi Zohar]

IMA by default does not measure, appraise or audit files, but can be
enabled at runtime by specifying a builtin policy on the boot command line
or by loading a custom policy.

This patch defines a build time policy, which verifies kernel modules,
firmware, kexec image, and/or the IMA policy signatures.  This build time
policy is automatically enabled at runtime.  The build time policy rules
persist after loading a custom policy.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 security/integrity/ima/Kconfig      | 58 +++++++++++++++++++++++++++++++++++++
 security/integrity/ima/ima_policy.c | 46 +++++++++++++++++++++++++++--
 2 files changed, 101 insertions(+), 3 deletions(-)

diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 6a8f67714c83..004919d9bf09 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -156,6 +156,64 @@ config IMA_APPRAISE
 	  <http://linux-ima.sourceforge.net>
 	  If unsure, say N.
 
+config IMA_APPRAISE_BUILD_POLICY
+	bool "IMA build time configured policy rules"
+	depends on IMA_APPRAISE && INTEGRITY_ASYMMETRIC_KEYS
+	default n
+	help
+	  This option defines an IMA appraisal policy at build time, which
+	  is enforced at run time without having to specify a builtin
+	  policy name on the boot command line.  The build time appraisal
+	  policy rules persist after loading a custom policy.
+
+	  Depending on the rules configured, this policy may require kernel
+	  modules, firmware, the kexec kernel image, and/or the IMA policy
+	  to be signed.  Unsigned files might prevent the system from
+	  booting or applications from working properly.
+
+config IMA_APPRAISE_REQUIRE_FIRMWARE_SIGS
+	bool "Appraise firmware signatures"
+	depends on IMA_APPRAISE_BUILD_POLICY
+	default n
+	help
+	  This option defines a policy requiring all firmware to be signed,
+	  including the regulatory.db.  If both this option and
+	  CFG80211_REQUIRE_SIGNED_REGDB are enabled, then both signature
+	  verification methods are necessary.
+
+config IMA_APPRAISE_REQUIRE_KEXEC_SIGS
+	bool "Appraise kexec kernel image signatures"
+	depends on IMA_APPRAISE_BUILD_POLICY
+	default n
+	help
+	  Enabling this rule will require all kexec'ed kernel images to
+	  be signed and verified by a public key on the trusted IMA
+	  keyring.
+
+	  Kernel image signatures can not be verified by the original
+	  kexec_load syscall.  Enabling this rule will prevent its
+	  usage.
+
+config IMA_APPRAISE_REQUIRE_MODULE_SIGS
+	bool "Appraise kernel modules signatures"
+	depends on IMA_APPRAISE_BUILD_POLICY
+	default n
+	help
+	  Enabling this rule will require all kernel modules to be signed
+	  and verified by a public key on the trusted IMA keyring.
+
+	  Kernel module signatures can only be verified by IMA-appraisal,
+	  via the finit_module syscall. Enabling this rule will prevent
+	  the usage of the init_module syscall.
+
+config IMA_APPRAISE_REQUIRE_POLICY_SIGS
+	bool "Appraise IMA policy signature"
+	depends on IMA_APPRAISE_BUILD_POLICY
+	default n
+	help
+	  Enabling this rule will require the IMA policy to be signed and
+	  and verified by a key on the trusted IMA keyring.
+
 config IMA_APPRAISE_BOOTPARAM
 	bool "ima_appraise boot parameter"
 	depends on IMA_APPRAISE
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index c27f6993b07a..3c0bc8a1a88e 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -49,6 +49,7 @@
 
 int ima_policy_flag;
 static int temp_ima_appraise;
+static int build_ima_appraise __ro_after_init;
 
 #define MAX_LSM_RULES 6
 enum lsm_rule_types { LSM_OBJ_USER, LSM_OBJ_ROLE, LSM_OBJ_TYPE,
@@ -162,6 +163,25 @@ static struct ima_rule_entry default_appraise_rules[] __ro_after_init = {
 #endif
 };
 
+static struct ima_rule_entry build_appraise_rules[] __ro_after_init = {
+#ifdef CONFIG_IMA_APPRAISE_REQUIRE_MODULE_SIGS
+	{.action = APPRAISE, .func = MODULE_CHECK,
+	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
+#endif
+#ifdef CONFIG_IMA_APPRAISE_REQUIRE_FIRMWARE_SIGS
+	{.action = APPRAISE, .func = FIRMWARE_CHECK,
+	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
+#endif
+#ifdef CONFIG_IMA_APPRAISE_REQUIRE_KEXEC_SIGS
+	{.action = APPRAISE, .func = KEXEC_KERNEL_CHECK,
+	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
+#endif
+#ifdef CONFIG_IMA_APPRAISE_REQUIRE_POLICY_SIGS
+	{.action = APPRAISE, .func = POLICY_CHECK,
+	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
+#endif
+};
+
 static struct ima_rule_entry secure_boot_rules[] __ro_after_init = {
 	{.action = APPRAISE, .func = MODULE_CHECK,
 	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
@@ -435,7 +455,7 @@ void ima_update_policy_flag(void)
 			ima_policy_flag |= entry->action;
 	}
 
-	ima_appraise |= temp_ima_appraise;
+	ima_appraise |= (build_ima_appraise | temp_ima_appraise);
 	if (!ima_appraise)
 		ima_policy_flag &= ~IMA_APPRAISE;
 }
@@ -488,8 +508,8 @@ void __init ima_init_policy(void)
 	}
 
 	/*
-	 * Insert the appraise rules requiring file signatures, prior to
-	 * any other appraise rules.
+	 * Insert the builtin "secure_boot" policy rules requiring file
+	 * signatures, prior to any other appraise rules.
 	 */
 	for (i = 0; i < secure_boot_entries; i++) {
 		list_add_tail(&secure_boot_rules[i].list, &ima_default_rules);
@@ -497,6 +517,26 @@ void __init ima_init_policy(void)
 		    ima_appraise_flag(secure_boot_rules[i].func);
 	}
 
+	/*
+	 * Insert the build time appraise rules requiring file signatures
+	 * for both the initial and custom policies, prior to other appraise
+	 * rules.
+	 */
+	for (i = 0; i < ARRAY_SIZE(build_appraise_rules); i++) {
+		struct ima_rule_entry *entry;
+
+		if (!secure_boot_entries)
+			list_add_tail(&build_appraise_rules[i].list,
+				      &ima_default_rules);
+
+		entry = kmemdup(&build_appraise_rules[i], sizeof(*entry),
+				GFP_KERNEL);
+		if (entry)
+			list_add_tail(&entry->list, &ima_policy_rules);
+		build_ima_appraise |=
+			ima_appraise_flag(build_appraise_rules[i].func);
+	}
+
 	for (i = 0; i < appraise_entries; i++) {
 		list_add_tail(&default_appraise_rules[i].list,
 			      &ima_default_rules);
-- 
2.7.5


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[RFC PATCH v3 7/7] ima: based on policy prevent loading firmware (pre-allocated buffer)

[Mimi Zohar]

Question: can the device access the pre-allocated buffer at any time?
(Still waiting to hear from Qualcomm...)

By allowing devices to request firmware be loaded directly into a
pre-allocated buffer, will this allow the device access to the firmware
before the kernel has verified the firmware signature?

Is it dependent on the type of buffer allocated (eg. DMA)?  For example,
qcom_mdt_load() -> qcom_scm_pas_init_image() -> dma_alloc_coherent().

With an IMA policy requiring signed firmware, this patch would prevent
loading firmware into a pre-allocated buffer.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Luis R. Rodriguez <mcgrof@suse.com>
Cc: David Howells <dhowells@redhat.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Serge E. Hallyn <serge@hallyn.com>
Cc: Stephen Boyd <sboyd@kernel.org>
---
 security/integrity/ima/ima_main.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index dd1f263f950a..d114b7ad2c86 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -457,6 +457,12 @@ int ima_read_data(struct file *file, enum kernel_read_file_id read_id)
 			pr_err("Prevent firmware sysfs fallback loading.\n");
 			return -EACCES;	/* INTEGRITY_UNKNOWN */
 		}
+		break;
+	case READING_FIRMWARE_PREALLOC_BUFFER:
+		if (ima_appraise & IMA_APPRAISE_FIRMWARE) {
+			pr_err("Prevent device from accessing firmware prior to verifying the firmware signature.\n");
+			return -EACCES;
+		}
 	default:
 		break;
 	}
-- 
2.7.5

[RFC PATCH v3 7/7] ima: based on policy prevent loading firmware (pre-allocated buffer)

[Mimi Zohar]

Question: can the device access the pre-allocated buffer at any time?
(Still waiting to hear from Qualcomm...)

By allowing devices to request firmware be loaded directly into a
pre-allocated buffer, will this allow the device access to the firmware
before the kernel has verified the firmware signature?

Is it dependent on the type of buffer allocated (eg. DMA)?  For example,
qcom_mdt_load() -> qcom_scm_pas_init_image() -> dma_alloc_coherent().

With an IMA policy requiring signed firmware, this patch would prevent
loading firmware into a pre-allocated buffer.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Luis R. Rodriguez <mcgrof@suse.com>
Cc: David Howells <dhowells@redhat.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Serge E. Hallyn <serge@hallyn.com>
Cc: Stephen Boyd <sboyd@kernel.org>
---
 security/integrity/ima/ima_main.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index dd1f263f950a..d114b7ad2c86 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -457,6 +457,12 @@ int ima_read_data(struct file *file, enum kernel_read_file_id read_id)
 			pr_err("Prevent firmware sysfs fallback loading.\n");
 			return -EACCES;	/* INTEGRITY_UNKNOWN */
 		}
+		break;
+	case READING_FIRMWARE_PREALLOC_BUFFER:
+		if (ima_appraise & IMA_APPRAISE_FIRMWARE) {
+			pr_err("Prevent device from accessing firmware prior to verifying the firmware signature.\n");
+			return -EACCES;
+		}
 	default:
 		break;
 	}
-- 
2.7.5

--
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 v3 7/7] ima: based on policy prevent loading firmware (pre-allocated buffer)

[Mimi Zohar]

Question: can the device access the pre-allocated buffer at any time?
(Still waiting to hear from Qualcomm...)

By allowing devices to request firmware be loaded directly into a
pre-allocated buffer, will this allow the device access to the firmware
before the kernel has verified the firmware signature?

Is it dependent on the type of buffer allocated (eg. DMA)?  For example,
qcom_mdt_load() -> qcom_scm_pas_init_image() -> dma_alloc_coherent().

With an IMA policy requiring signed firmware, this patch would prevent
loading firmware into a pre-allocated buffer.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Luis R. Rodriguez <mcgrof@suse.com>
Cc: David Howells <dhowells@redhat.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Serge E. Hallyn <serge@hallyn.com>
Cc: Stephen Boyd <sboyd@kernel.org>
---
 security/integrity/ima/ima_main.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index dd1f263f950a..d114b7ad2c86 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -457,6 +457,12 @@ int ima_read_data(struct file *file, enum kernel_read_file_id read_id)
 			pr_err("Prevent firmware sysfs fallback loading.\n");
 			return -EACCES;	/* INTEGRITY_UNKNOWN */
 		}
+		break;
+	case READING_FIRMWARE_PREALLOC_BUFFER:
+		if (ima_appraise & IMA_APPRAISE_FIRMWARE) {
+			pr_err("Prevent device from accessing firmware prior to verifying the firmware signature.\n");
+			return -EACCES;
+		}
 	default:
 		break;
 	}
-- 
2.7.5


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v3 1/7] security: rename security_kernel_read_file() hook

[Eric W. Biederman]

I already nacked this approach because the two cases don't
share a bit of code.  When I looked closer it was even crazier.

The way ima uses this hook and the post_load hook today is a travesty.

The way the security_kernel_file_read and security_kernel_file_post_read
are called today and are used by ima don't make the least little bit of
sense.

Abusing security_kernel_file_read in the module loader and then abusing
security_kernel_file_post_read in the firmware loader is insane.  The
loadpin lsm could not even figure this out and so it failed to work
because of these shenanighans.

Only implementing kernel_file_read to handle the !file case is pretty
much insane.   There is no way this should be expanded to cover kexec
until the code actually makes sense.  We need a maintainable kernel.

Below is where I suggest you start on sorting out these security hooks.
- Adding a security_kernel_arg to catch when you want to allow/deny the
  use of an argument to a syscall.  What security_kernel_file_read and
  security_kernel_file_post_read have been abused for.

- Removing ima_file_read because it is completely subsumed by the new
  call.

- Please note with adding this new hook there is no code shared between
  the cases, and the lsm code becomes simpler shorter when it can assume
  security_kernel_file_post_read always takes a struct file.  (Even with
  the addition of a new security hook).

Eric

diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index 358354148dec..04536ff81bd2 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -294,9 +294,7 @@ static ssize_t firmware_loading_store(struct device *dev,
 				dev_err(dev, "%s: map pages failed\n",
 					__func__);
 			else
-				rc = security_kernel_post_read_file(NULL,
-						fw_priv->data, fw_priv->size,
-						READING_FIRMWARE);
+				rc = security_kernel_arg(KARG_FIRMWARE);
 
 			/*
 			 * Same logic as fw_load_abort, only the DONE bit
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 0e4647e0eb60..9fb42736ba29 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -11,6 +11,7 @@
 #define _LINUX_IMA_H
 
 #include <linux/fs.h>
+#include <linux/security.h>
 #include <linux/kexec.h>
 struct linux_binprm;
 
@@ -19,7 +20,7 @@ extern int ima_bprm_check(struct linux_binprm *bprm);
 extern int ima_file_check(struct file *file, int mask, int opened);
 extern void ima_file_free(struct file *file);
 extern int ima_file_mmap(struct file *file, unsigned long prot);
-extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
+extern int ima_kernel_arg(enum kernel_arg_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);
@@ -49,7 +50,7 @@ static inline int ima_file_mmap(struct file *file, unsigned long prot)
 	return 0;
 }
 
-static inline int ima_read_file(struct file *file, enum kernel_read_file_id id)
+static inline int ima_kernel_arg(enum kernel_arg_id id)
 {
 	return 0;
 }
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 9d0b286f3dba..7f8bc3030784 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -576,6 +576,10 @@
  *	userspace to load a kernel module with the given name.
  *	@kmod_name name of the module requested by the kernel
  *	Return 0 if successful.
+ * @kernel_arg:
+ *	Use a syscall argument
+ *	@id kernel argument identifier
+ *	Return 0 if permission is granted.
  * @kernel_read_file:
  *	Read a file specified by userspace.
  *	@file contains the file structure pointing to the file being read
@@ -1577,6 +1581,7 @@ union security_list_options {
 	int (*kernel_act_as)(struct cred *new, u32 secid);
 	int (*kernel_create_files_as)(struct cred *new, struct inode *inode);
 	int (*kernel_module_request)(char *kmod_name);
+	int (*kernel_arg)(enum kernel_arg_id id);
 	int (*kernel_read_file)(struct file *file, enum kernel_read_file_id id);
 	int (*kernel_post_read_file)(struct file *file, char *buf, loff_t size,
 				     enum kernel_read_file_id id);
@@ -1866,6 +1871,7 @@ struct security_hook_heads {
 	struct hlist_head cred_getsecid;
 	struct hlist_head kernel_act_as;
 	struct hlist_head kernel_create_files_as;
+	struct hlist_head kernel_arg;
 	struct hlist_head kernel_read_file;
 	struct hlist_head kernel_post_read_file;
 	struct hlist_head kernel_module_request;
diff --git a/include/linux/security.h b/include/linux/security.h
index 200920f521a1..6cf1bd87f041 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -159,6 +159,32 @@ extern int mmap_min_addr_handler(struct ctl_table *table, int write,
 typedef int (*initxattrs) (struct inode *inode,
 			   const struct xattr *xattr_array, void *fs_data);
 
+#define __kernel_arg_id(id) \
+	id(UNKNOWN, unknown)		\
+	id(FIRMWARE, firmware)		\
+	id(MODULE, kernel-module)	\
+	id(MAX_ID, )
+
+#define __karg_enumify(ENUM, dummy) KARG_ ## ENUM,
+#define __karg_stringify(dummy, str) #str,
+
+enum kernel_arg_id {
+	__kernel_arg_id(__karg_enumify)
+};
+
+static const char * const kernel_arg_str[] = {
+	__kernel_arg_id(__karg_stringify)
+};
+
+static inline const char *kernel_arg_id_str(enum kernel_arg_id id)
+{
+	if ((unsigned)id >= KARG_MAX_ID)
+		return kernel_arg_str[KARG_UNKNOWN];
+
+	return kernel_arg_str[id];
+}
+
+
 #ifdef CONFIG_SECURITY
 
 struct security_mnt_opts {
@@ -326,6 +352,7 @@ void security_cred_getsecid(const struct cred *c, u32 *secid);
 int security_kernel_act_as(struct cred *new, u32 secid);
 int security_kernel_create_files_as(struct cred *new, struct inode *inode);
 int security_kernel_module_request(char *kmod_name);
+int security_kernel_arg(enum kernel_arg_id id);
 int security_kernel_read_file(struct file *file, enum kernel_read_file_id id);
 int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
 				   enum kernel_read_file_id id);
@@ -923,6 +950,11 @@ static inline int security_kernel_module_request(char *kmod_name)
 	return 0;
 }
 
+static inline int security_kernel_arg(enum kernel_arg_id id)
+{
+	return 0;
+}
+
 static inline int security_kernel_read_file(struct file *file,
 					    enum kernel_read_file_id id)
 {
diff --git a/kernel/module.c b/kernel/module.c
index ce8066b88178..03a1dd21ad4a 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2879,7 +2879,7 @@ static int copy_module_from_user(const void __user *umod, unsigned long len,
 	if (info->len < sizeof(*(info->hdr)))
 		return -ENOEXEC;
 
-	err = security_kernel_read_file(NULL, READING_MODULE);
+	err = security_kernel_arg(KARG_MODULE);
 	if (err)
 		return err;
 
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 74d0bd7e76d7..d51a8ca97238 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -421,32 +421,6 @@ void ima_post_path_mknod(struct dentry *dentry)
 		iint->flags |= IMA_NEW_FILE;
 }
 
-/**
- * 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
- *
- * Permit reading a file based on policy. The policy rules are written
- * in terms of the policy identifier.  Appraising the integrity of
- * a file requires a file descriptor.
- *
- * For permission return 0, otherwise return -EACCES.
- */
-int ima_read_file(struct file *file, enum kernel_read_file_id read_id)
-{
-	bool sig_enforce = is_module_sig_enforced();
-
-	if (!file && read_id == READING_MODULE) {
-		if (!sig_enforce && (ima_appraise & IMA_APPRAISE_MODULES) &&
-		    (ima_appraise & IMA_APPRAISE_ENFORCE)) {
-			pr_err("impossible to appraise a module without a file descriptor. sig_enforce kernel parameter might help\n");
-			return -EACCES;	/* INTEGRITY_UNKNOWN */
-		}
-		return 0;	/* We rely on module signature checking */
-	}
-	return 0;
-}
-
 static int read_idmap[READING_MAX_ID] = {
 	[READING_FIRMWARE] = FIRMWARE_CHECK,
 	[READING_MODULE] = MODULE_CHECK,
@@ -474,21 +448,7 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
 	enum ima_hooks func;
 	u32 secid;
 
-	if (!file && read_id == READING_FIRMWARE) {
-		if ((ima_appraise & IMA_APPRAISE_FIRMWARE) &&
-		    (ima_appraise & IMA_APPRAISE_ENFORCE))
-			return -EACCES;	/* INTEGRITY_UNKNOWN */
-		return 0;
-	}
-
-	if (!file && read_id == READING_MODULE) /* MODULE_SIG_FORCE enabled */
-		return 0;
-
-	/* permit signed certs */
-	if (!file && read_id == READING_X509_CERTIFICATE)
-		return 0;
-
-	if (!file || !buf || size == 0) { /* should never happen */
+	if (!buf || size == 0) { /* should never happen */
 		if (ima_appraise & IMA_APPRAISE_ENFORCE)
 			return -EACCES;
 		return 0;
@@ -500,6 +460,40 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
 				   MAY_READ, func, 0);
 }
 
+/**
+ * ima_kernel_arg - pre-measure/appraise hook decision based on policy
+ * @id: caller identifier
+ *
+ * Permit using an argument to a syscall based on policy. The policy
+ * rules are written in terms of the policy identifier.
+ *
+ * For permission return 0, otherwise return -EACCES.
+ */
+int ima_kernel_arg(enum kernel_arg_id id)
+{
+	if (id == KARG_MODULE) {
+		bool sig_enforce = is_module_sig_enforced();
+		if (!sig_enforce && (ima_appraise & IMA_APPRAISE_MODULES) &&
+		    (ima_appraise & IMA_APPRAISE_ENFORCE)) {
+			pr_err("impossible to appraise a module without a file descriptor. sig_enforce kernel parameter might help\n");
+			return -EACCES;	/* INTEGRITY_UNKNOWN */
+		}
+		return 0;	/* We rely on module signature checking */
+	}
+	else if (id == KARG_FIRMWARE) {
+		if ((ima_appraise & IMA_APPRAISE_FIRMWARE) &&
+		    (ima_appraise & IMA_APPRAISE_ENFORCE))
+			return -EACCES;	/* INTEGRITY_UNKNOWN */
+		return 0;
+	}
+	else {
+		if (ima_appraise & IMA_APPRAISE_ENFORCE)
+			return -EACCES;
+		return 0;
+	}
+	return 0;
+}
+
 static int __init init_ima(void)
 {
 	int error;
diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
index 5fa191252c8f..f5333e5abac9 100644
--- a/security/loadpin/loadpin.c
+++ b/security/loadpin/loadpin.c
@@ -121,23 +121,24 @@ static void loadpin_sb_free_security(struct super_block *mnt_sb)
 	}
 }
 
-static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
+static int loadpin_read_data(enum kernel_read_data_id id)
 {
-	struct super_block *load_root;
-	const char *origin = kernel_read_file_id_str(id);
+	const char *origin = kernel_arg_id_str(id);
 
 	/* This handles the older init_module API that has a NULL file. */
-	if (!file) {
-		if (!enabled) {
-			report_load(origin, NULL, "old-api-pinning-ignored");
-			return 0;
-		}
-
-		report_load(origin, NULL, "old-api-denied");
-		return -EPERM;
+	if (!enabled) {
+		report_load(origin, NULL, "old-api-pinning-ignored");
+		return 0;
 	}
 
-	load_root = file->f_path.mnt->mnt_sb;
+	report_load(origin, NULL, "old-api-denied");
+	return -EPERM;
+}
+
+static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
+{
+	struct super_block *load_root;
+	const char *origin = kernel_read_file_id_str(id);
 
 	/* First loaded module/firmware defines the root for all others. */
 	spin_lock(&pinned_root_spinlock);
@@ -175,6 +176,7 @@ static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
 
 static struct security_hook_list loadpin_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(sb_free_security, loadpin_sb_free_security),
+	LSM_HOOK_INIT(kernel_read_data, loadpin_read_data),
 	LSM_HOOK_INIT(kernel_read_file, loadpin_read_file),
 };
 
diff --git a/security/security.c b/security/security.c
index 7bc2fde023a7..9b5f43c24ee2 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1033,14 +1033,19 @@ int security_kernel_module_request(char *kmod_name)
 	return call_int_hook(kernel_module_request, 0, kmod_name);
 }
 
-int security_kernel_read_file(struct file *file, enum kernel_read_file_id id)
+int security_kernel_arg(enum kernel_arg_id id)
 {
 	int ret;
 
-	ret = call_int_hook(kernel_read_file, 0, file, id);
-	if (ret)
-		return ret;
-	return ima_read_file(file, id);
+	ret = call_int_hook(kernel_arg, 0, id);
+	if (!ret)
+		ret = ima_kernel_arg(id);
+	return ret;
+}
+
+int security_kernel_read_file(struct file *file, enum kernel_read_file_id id)
+{
+	return call_int_hook(kernel_read_file, 0, file, id);
 }
 EXPORT_SYMBOL_GPL(security_kernel_read_file);
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 4cafe6a19167..76843099fed6 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4010,6 +4010,15 @@ static int selinux_kernel_module_request(char *kmod_name)
 			    SYSTEM__MODULE_REQUEST, &ad);
 }
 
+static int selinux_kernel_module_arg(void)
+{
+	/* init_module */
+	u32 sid = current_sid();
+	return avc_has_perm(&selinux_state,
+			    sid, sid, SECCLASS_SYSTEM,
+			    SYSTEM__MODULE_LOAD, NULL);
+}
+
 static int selinux_kernel_module_from_file(struct file *file)
 {
 	struct common_audit_data ad;
@@ -4018,12 +4027,6 @@ static int selinux_kernel_module_from_file(struct file *file)
 	u32 sid = current_sid();
 	int rc;
 
-	/* init_module */
-	if (file == NULL)
-		return avc_has_perm(&selinux_state,
-				    sid, sid, SECCLASS_SYSTEM,
-					SYSTEM__MODULE_LOAD, NULL);
-
 	/* finit_module */
 
 	ad.type = LSM_AUDIT_DATA_FILE;
@@ -4043,6 +4046,20 @@ static int selinux_kernel_module_from_file(struct file *file)
 				SYSTEM__MODULE_LOAD, &ad);
 }
 
+static int selinux_kernel_arg(enum kernel_arg_id id)
+{
+	int rc = 0;
+
+	switch (id) {
+	case KARG_MODULE:
+		rc = selinux_kernel_module_arg();
+		break;
+	default:
+		break;
+	}
+	return rc;
+}
+
 static int selinux_kernel_read_file(struct file *file,
 				    enum kernel_read_file_id id)
 {
@@ -6938,6 +6955,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(kernel_act_as, selinux_kernel_act_as),
 	LSM_HOOK_INIT(kernel_create_files_as, selinux_kernel_create_files_as),
 	LSM_HOOK_INIT(kernel_module_request, selinux_kernel_module_request),
+	LSM_HOOK_INIT(kernel_arg, selinux_kernel_arg),
 	LSM_HOOK_INIT(kernel_read_file, selinux_kernel_read_file),
 	LSM_HOOK_INIT(task_setpgid, selinux_task_setpgid),
 	LSM_HOOK_INIT(task_getpgid, selinux_task_getpgid),

[PATCH v3 1/7] security: rename security_kernel_read_file() hook

[Eric W. Biederman]

I already nacked this approach because the two cases don't
share a bit of code.  When I looked closer it was even crazier.

The way ima uses this hook and the post_load hook today is a travesty.

The way the security_kernel_file_read and security_kernel_file_post_read
are called today and are used by ima don't make the least little bit of
sense.

Abusing security_kernel_file_read in the module loader and then abusing
security_kernel_file_post_read in the firmware loader is insane.  The
loadpin lsm could not even figure this out and so it failed to work
because of these shenanighans.

Only implementing kernel_file_read to handle the !file case is pretty
much insane.   There is no way this should be expanded to cover kexec
until the code actually makes sense.  We need a maintainable kernel.

Below is where I suggest you start on sorting out these security hooks.
- Adding a security_kernel_arg to catch when you want to allow/deny the
  use of an argument to a syscall.  What security_kernel_file_read and
  security_kernel_file_post_read have been abused for.

- Removing ima_file_read because it is completely subsumed by the new
  call.

- Please note with adding this new hook there is no code shared between
  the cases, and the lsm code becomes simpler shorter when it can assume
  security_kernel_file_post_read always takes a struct file.  (Even with
  the addition of a new security hook).

Eric

diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index 358354148dec..04536ff81bd2 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -294,9 +294,7 @@ static ssize_t firmware_loading_store(struct device *dev,
 				dev_err(dev, "%s: map pages failed\n",
 					__func__);
 			else
-				rc = security_kernel_post_read_file(NULL,
-						fw_priv->data, fw_priv->size,
-						READING_FIRMWARE);
+				rc = security_kernel_arg(KARG_FIRMWARE);
 
 			/*
 			 * Same logic as fw_load_abort, only the DONE bit
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 0e4647e0eb60..9fb42736ba29 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -11,6 +11,7 @@
 #define _LINUX_IMA_H
 
 #include <linux/fs.h>
+#include <linux/security.h>
 #include <linux/kexec.h>
 struct linux_binprm;
 
@@ -19,7 +20,7 @@ extern int ima_bprm_check(struct linux_binprm *bprm);
 extern int ima_file_check(struct file *file, int mask, int opened);
 extern void ima_file_free(struct file *file);
 extern int ima_file_mmap(struct file *file, unsigned long prot);
-extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
+extern int ima_kernel_arg(enum kernel_arg_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);
@@ -49,7 +50,7 @@ static inline int ima_file_mmap(struct file *file, unsigned long prot)
 	return 0;
 }
 
-static inline int ima_read_file(struct file *file, enum kernel_read_file_id id)
+static inline int ima_kernel_arg(enum kernel_arg_id id)
 {
 	return 0;
 }
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 9d0b286f3dba..7f8bc3030784 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -576,6 +576,10 @@
  *	userspace to load a kernel module with the given name.
  *	@kmod_name name of the module requested by the kernel
  *	Return 0 if successful.
+ * @kernel_arg:
+ *	Use a syscall argument
+ *	@id kernel argument identifier
+ *	Return 0 if permission is granted.
  * @kernel_read_file:
  *	Read a file specified by userspace.
  *	@file contains the file structure pointing to the file being read
@@ -1577,6 +1581,7 @@ union security_list_options {
 	int (*kernel_act_as)(struct cred *new, u32 secid);
 	int (*kernel_create_files_as)(struct cred *new, struct inode *inode);
 	int (*kernel_module_request)(char *kmod_name);
+	int (*kernel_arg)(enum kernel_arg_id id);
 	int (*kernel_read_file)(struct file *file, enum kernel_read_file_id id);
 	int (*kernel_post_read_file)(struct file *file, char *buf, loff_t size,
 				     enum kernel_read_file_id id);
@@ -1866,6 +1871,7 @@ struct security_hook_heads {
 	struct hlist_head cred_getsecid;
 	struct hlist_head kernel_act_as;
 	struct hlist_head kernel_create_files_as;
+	struct hlist_head kernel_arg;
 	struct hlist_head kernel_read_file;
 	struct hlist_head kernel_post_read_file;
 	struct hlist_head kernel_module_request;
diff --git a/include/linux/security.h b/include/linux/security.h
index 200920f521a1..6cf1bd87f041 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -159,6 +159,32 @@ extern int mmap_min_addr_handler(struct ctl_table *table, int write,
 typedef int (*initxattrs) (struct inode *inode,
 			   const struct xattr *xattr_array, void *fs_data);
 
+#define __kernel_arg_id(id) \
+	id(UNKNOWN, unknown)		\
+	id(FIRMWARE, firmware)		\
+	id(MODULE, kernel-module)	\
+	id(MAX_ID, )
+
+#define __karg_enumify(ENUM, dummy) KARG_ ## ENUM,
+#define __karg_stringify(dummy, str) #str,
+
+enum kernel_arg_id {
+	__kernel_arg_id(__karg_enumify)
+};
+
+static const char * const kernel_arg_str[] = {
+	__kernel_arg_id(__karg_stringify)
+};
+
+static inline const char *kernel_arg_id_str(enum kernel_arg_id id)
+{
+	if ((unsigned)id >= KARG_MAX_ID)
+		return kernel_arg_str[KARG_UNKNOWN];
+
+	return kernel_arg_str[id];
+}
+
+
 #ifdef CONFIG_SECURITY
 
 struct security_mnt_opts {
@@ -326,6 +352,7 @@ void security_cred_getsecid(const struct cred *c, u32 *secid);
 int security_kernel_act_as(struct cred *new, u32 secid);
 int security_kernel_create_files_as(struct cred *new, struct inode *inode);
 int security_kernel_module_request(char *kmod_name);
+int security_kernel_arg(enum kernel_arg_id id);
 int security_kernel_read_file(struct file *file, enum kernel_read_file_id id);
 int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
 				   enum kernel_read_file_id id);
@@ -923,6 +950,11 @@ static inline int security_kernel_module_request(char *kmod_name)
 	return 0;
 }
 
+static inline int security_kernel_arg(enum kernel_arg_id id)
+{
+	return 0;
+}
+
 static inline int security_kernel_read_file(struct file *file,
 					    enum kernel_read_file_id id)
 {
diff --git a/kernel/module.c b/kernel/module.c
index ce8066b88178..03a1dd21ad4a 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2879,7 +2879,7 @@ static int copy_module_from_user(const void __user *umod, unsigned long len,
 	if (info->len < sizeof(*(info->hdr)))
 		return -ENOEXEC;
 
-	err = security_kernel_read_file(NULL, READING_MODULE);
+	err = security_kernel_arg(KARG_MODULE);
 	if (err)
 		return err;
 
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 74d0bd7e76d7..d51a8ca97238 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -421,32 +421,6 @@ void ima_post_path_mknod(struct dentry *dentry)
 		iint->flags |= IMA_NEW_FILE;
 }
 
-/**
- * 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
- *
- * Permit reading a file based on policy. The policy rules are written
- * in terms of the policy identifier.  Appraising the integrity of
- * a file requires a file descriptor.
- *
- * For permission return 0, otherwise return -EACCES.
- */
-int ima_read_file(struct file *file, enum kernel_read_file_id read_id)
-{
-	bool sig_enforce = is_module_sig_enforced();
-
-	if (!file && read_id == READING_MODULE) {
-		if (!sig_enforce && (ima_appraise & IMA_APPRAISE_MODULES) &&
-		    (ima_appraise & IMA_APPRAISE_ENFORCE)) {
-			pr_err("impossible to appraise a module without a file descriptor. sig_enforce kernel parameter might help\n");
-			return -EACCES;	/* INTEGRITY_UNKNOWN */
-		}
-		return 0;	/* We rely on module signature checking */
-	}
-	return 0;
-}
-
 static int read_idmap[READING_MAX_ID] = {
 	[READING_FIRMWARE] = FIRMWARE_CHECK,
 	[READING_MODULE] = MODULE_CHECK,
@@ -474,21 +448,7 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
 	enum ima_hooks func;
 	u32 secid;
 
-	if (!file && read_id == READING_FIRMWARE) {
-		if ((ima_appraise & IMA_APPRAISE_FIRMWARE) &&
-		    (ima_appraise & IMA_APPRAISE_ENFORCE))
-			return -EACCES;	/* INTEGRITY_UNKNOWN */
-		return 0;
-	}
-
-	if (!file && read_id == READING_MODULE) /* MODULE_SIG_FORCE enabled */
-		return 0;
-
-	/* permit signed certs */
-	if (!file && read_id == READING_X509_CERTIFICATE)
-		return 0;
-
-	if (!file || !buf || size == 0) { /* should never happen */
+	if (!buf || size == 0) { /* should never happen */
 		if (ima_appraise & IMA_APPRAISE_ENFORCE)
 			return -EACCES;
 		return 0;
@@ -500,6 +460,40 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
 				   MAY_READ, func, 0);
 }
 
+/**
+ * ima_kernel_arg - pre-measure/appraise hook decision based on policy
+ * @id: caller identifier
+ *
+ * Permit using an argument to a syscall based on policy. The policy
+ * rules are written in terms of the policy identifier.
+ *
+ * For permission return 0, otherwise return -EACCES.
+ */
+int ima_kernel_arg(enum kernel_arg_id id)
+{
+	if (id == KARG_MODULE) {
+		bool sig_enforce = is_module_sig_enforced();
+		if (!sig_enforce && (ima_appraise & IMA_APPRAISE_MODULES) &&
+		    (ima_appraise & IMA_APPRAISE_ENFORCE)) {
+			pr_err("impossible to appraise a module without a file descriptor. sig_enforce kernel parameter might help\n");
+			return -EACCES;	/* INTEGRITY_UNKNOWN */
+		}
+		return 0;	/* We rely on module signature checking */
+	}
+	else if (id == KARG_FIRMWARE) {
+		if ((ima_appraise & IMA_APPRAISE_FIRMWARE) &&
+		    (ima_appraise & IMA_APPRAISE_ENFORCE))
+			return -EACCES;	/* INTEGRITY_UNKNOWN */
+		return 0;
+	}
+	else {
+		if (ima_appraise & IMA_APPRAISE_ENFORCE)
+			return -EACCES;
+		return 0;
+	}
+	return 0;
+}
+
 static int __init init_ima(void)
 {
 	int error;
diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
index 5fa191252c8f..f5333e5abac9 100644
--- a/security/loadpin/loadpin.c
+++ b/security/loadpin/loadpin.c
@@ -121,23 +121,24 @@ static void loadpin_sb_free_security(struct super_block *mnt_sb)
 	}
 }
 
-static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
+static int loadpin_read_data(enum kernel_read_data_id id)
 {
-	struct super_block *load_root;
-	const char *origin = kernel_read_file_id_str(id);
+	const char *origin = kernel_arg_id_str(id);
 
 	/* This handles the older init_module API that has a NULL file. */
-	if (!file) {
-		if (!enabled) {
-			report_load(origin, NULL, "old-api-pinning-ignored");
-			return 0;
-		}
-
-		report_load(origin, NULL, "old-api-denied");
-		return -EPERM;
+	if (!enabled) {
+		report_load(origin, NULL, "old-api-pinning-ignored");
+		return 0;
 	}
 
-	load_root = file->f_path.mnt->mnt_sb;
+	report_load(origin, NULL, "old-api-denied");
+	return -EPERM;
+}
+
+static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
+{
+	struct super_block *load_root;
+	const char *origin = kernel_read_file_id_str(id);
 
 	/* First loaded module/firmware defines the root for all others. */
 	spin_lock(&pinned_root_spinlock);
@@ -175,6 +176,7 @@ static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
 
 static struct security_hook_list loadpin_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(sb_free_security, loadpin_sb_free_security),
+	LSM_HOOK_INIT(kernel_read_data, loadpin_read_data),
 	LSM_HOOK_INIT(kernel_read_file, loadpin_read_file),
 };
 
diff --git a/security/security.c b/security/security.c
index 7bc2fde023a7..9b5f43c24ee2 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1033,14 +1033,19 @@ int security_kernel_module_request(char *kmod_name)
 	return call_int_hook(kernel_module_request, 0, kmod_name);
 }
 
-int security_kernel_read_file(struct file *file, enum kernel_read_file_id id)
+int security_kernel_arg(enum kernel_arg_id id)
 {
 	int ret;
 
-	ret = call_int_hook(kernel_read_file, 0, file, id);
-	if (ret)
-		return ret;
-	return ima_read_file(file, id);
+	ret = call_int_hook(kernel_arg, 0, id);
+	if (!ret)
+		ret = ima_kernel_arg(id);
+	return ret;
+}
+
+int security_kernel_read_file(struct file *file, enum kernel_read_file_id id)
+{
+	return call_int_hook(kernel_read_file, 0, file, id);
 }
 EXPORT_SYMBOL_GPL(security_kernel_read_file);
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 4cafe6a19167..76843099fed6 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4010,6 +4010,15 @@ static int selinux_kernel_module_request(char *kmod_name)
 			    SYSTEM__MODULE_REQUEST, &ad);
 }
 
+static int selinux_kernel_module_arg(void)
+{
+	/* init_module */
+	u32 sid = current_sid();
+	return avc_has_perm(&selinux_state,
+			    sid, sid, SECCLASS_SYSTEM,
+			    SYSTEM__MODULE_LOAD, NULL);
+}
+
 static int selinux_kernel_module_from_file(struct file *file)
 {
 	struct common_audit_data ad;
@@ -4018,12 +4027,6 @@ static int selinux_kernel_module_from_file(struct file *file)
 	u32 sid = current_sid();
 	int rc;
 
-	/* init_module */
-	if (file == NULL)
-		return avc_has_perm(&selinux_state,
-				    sid, sid, SECCLASS_SYSTEM,
-					SYSTEM__MODULE_LOAD, NULL);
-
 	/* finit_module */
 
 	ad.type = LSM_AUDIT_DATA_FILE;
@@ -4043,6 +4046,20 @@ static int selinux_kernel_module_from_file(struct file *file)
 				SYSTEM__MODULE_LOAD, &ad);
 }
 
+static int selinux_kernel_arg(enum kernel_arg_id id)
+{
+	int rc = 0;
+
+	switch (id) {
+	case KARG_MODULE:
+		rc = selinux_kernel_module_arg();
+		break;
+	default:
+		break;
+	}
+	return rc;
+}
+
 static int selinux_kernel_read_file(struct file *file,
 				    enum kernel_read_file_id id)
 {
@@ -6938,6 +6955,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(kernel_act_as, selinux_kernel_act_as),
 	LSM_HOOK_INIT(kernel_create_files_as, selinux_kernel_create_files_as),
 	LSM_HOOK_INIT(kernel_module_request, selinux_kernel_module_request),
+	LSM_HOOK_INIT(kernel_arg, selinux_kernel_arg),
 	LSM_HOOK_INIT(kernel_read_file, selinux_kernel_read_file),
 	LSM_HOOK_INIT(task_setpgid, selinux_task_setpgid),
 	LSM_HOOK_INIT(task_getpgid, selinux_task_getpgid),

--
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: [PATCH v3 1/7] security: rename security_kernel_read_file() hook

[Eric W. Biederman]

I already nacked this approach because the two cases don't
share a bit of code.  When I looked closer it was even crazier.

The way ima uses this hook and the post_load hook today is a travesty.

The way the security_kernel_file_read and security_kernel_file_post_read
are called today and are used by ima don't make the least little bit of
sense.

Abusing security_kernel_file_read in the module loader and then abusing
security_kernel_file_post_read in the firmware loader is insane.  The
loadpin lsm could not even figure this out and so it failed to work
because of these shenanighans.

Only implementing kernel_file_read to handle the !file case is pretty
much insane.   There is no way this should be expanded to cover kexec
until the code actually makes sense.  We need a maintainable kernel.

Below is where I suggest you start on sorting out these security hooks.
- Adding a security_kernel_arg to catch when you want to allow/deny the
  use of an argument to a syscall.  What security_kernel_file_read and
  security_kernel_file_post_read have been abused for.

- Removing ima_file_read because it is completely subsumed by the new
  call.

- Please note with adding this new hook there is no code shared between
  the cases, and the lsm code becomes simpler shorter when it can assume
  security_kernel_file_post_read always takes a struct file.  (Even with
  the addition of a new security hook).

Eric

diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index 358354148dec..04536ff81bd2 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -294,9 +294,7 @@ static ssize_t firmware_loading_store(struct device *dev,
 				dev_err(dev, "%s: map pages failed\n",
 					__func__);
 			else
-				rc = security_kernel_post_read_file(NULL,
-						fw_priv->data, fw_priv->size,
-						READING_FIRMWARE);
+				rc = security_kernel_arg(KARG_FIRMWARE);
 
 			/*
 			 * Same logic as fw_load_abort, only the DONE bit
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 0e4647e0eb60..9fb42736ba29 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -11,6 +11,7 @@
 #define _LINUX_IMA_H
 
 #include <linux/fs.h>
+#include <linux/security.h>
 #include <linux/kexec.h>
 struct linux_binprm;
 
@@ -19,7 +20,7 @@ extern int ima_bprm_check(struct linux_binprm *bprm);
 extern int ima_file_check(struct file *file, int mask, int opened);
 extern void ima_file_free(struct file *file);
 extern int ima_file_mmap(struct file *file, unsigned long prot);
-extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
+extern int ima_kernel_arg(enum kernel_arg_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);
@@ -49,7 +50,7 @@ static inline int ima_file_mmap(struct file *file, unsigned long prot)
 	return 0;
 }
 
-static inline int ima_read_file(struct file *file, enum kernel_read_file_id id)
+static inline int ima_kernel_arg(enum kernel_arg_id id)
 {
 	return 0;
 }
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 9d0b286f3dba..7f8bc3030784 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -576,6 +576,10 @@
  *	userspace to load a kernel module with the given name.
  *	@kmod_name name of the module requested by the kernel
  *	Return 0 if successful.
+ * @kernel_arg:
+ *	Use a syscall argument
+ *	@id kernel argument identifier
+ *	Return 0 if permission is granted.
  * @kernel_read_file:
  *	Read a file specified by userspace.
  *	@file contains the file structure pointing to the file being read
@@ -1577,6 +1581,7 @@ union security_list_options {
 	int (*kernel_act_as)(struct cred *new, u32 secid);
 	int (*kernel_create_files_as)(struct cred *new, struct inode *inode);
 	int (*kernel_module_request)(char *kmod_name);
+	int (*kernel_arg)(enum kernel_arg_id id);
 	int (*kernel_read_file)(struct file *file, enum kernel_read_file_id id);
 	int (*kernel_post_read_file)(struct file *file, char *buf, loff_t size,
 				     enum kernel_read_file_id id);
@@ -1866,6 +1871,7 @@ struct security_hook_heads {
 	struct hlist_head cred_getsecid;
 	struct hlist_head kernel_act_as;
 	struct hlist_head kernel_create_files_as;
+	struct hlist_head kernel_arg;
 	struct hlist_head kernel_read_file;
 	struct hlist_head kernel_post_read_file;
 	struct hlist_head kernel_module_request;
diff --git a/include/linux/security.h b/include/linux/security.h
index 200920f521a1..6cf1bd87f041 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -159,6 +159,32 @@ extern int mmap_min_addr_handler(struct ctl_table *table, int write,
 typedef int (*initxattrs) (struct inode *inode,
 			   const struct xattr *xattr_array, void *fs_data);
 
+#define __kernel_arg_id(id) \
+	id(UNKNOWN, unknown)		\
+	id(FIRMWARE, firmware)		\
+	id(MODULE, kernel-module)	\
+	id(MAX_ID, )
+
+#define __karg_enumify(ENUM, dummy) KARG_ ## ENUM,
+#define __karg_stringify(dummy, str) #str,
+
+enum kernel_arg_id {
+	__kernel_arg_id(__karg_enumify)
+};
+
+static const char * const kernel_arg_str[] = {
+	__kernel_arg_id(__karg_stringify)
+};
+
+static inline const char *kernel_arg_id_str(enum kernel_arg_id id)
+{
+	if ((unsigned)id >= KARG_MAX_ID)
+		return kernel_arg_str[KARG_UNKNOWN];
+
+	return kernel_arg_str[id];
+}
+
+
 #ifdef CONFIG_SECURITY
 
 struct security_mnt_opts {
@@ -326,6 +352,7 @@ void security_cred_getsecid(const struct cred *c, u32 *secid);
 int security_kernel_act_as(struct cred *new, u32 secid);
 int security_kernel_create_files_as(struct cred *new, struct inode *inode);
 int security_kernel_module_request(char *kmod_name);
+int security_kernel_arg(enum kernel_arg_id id);
 int security_kernel_read_file(struct file *file, enum kernel_read_file_id id);
 int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
 				   enum kernel_read_file_id id);
@@ -923,6 +950,11 @@ static inline int security_kernel_module_request(char *kmod_name)
 	return 0;
 }
 
+static inline int security_kernel_arg(enum kernel_arg_id id)
+{
+	return 0;
+}
+
 static inline int security_kernel_read_file(struct file *file,
 					    enum kernel_read_file_id id)
 {
diff --git a/kernel/module.c b/kernel/module.c
index ce8066b88178..03a1dd21ad4a 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2879,7 +2879,7 @@ static int copy_module_from_user(const void __user *umod, unsigned long len,
 	if (info->len < sizeof(*(info->hdr)))
 		return -ENOEXEC;
 
-	err = security_kernel_read_file(NULL, READING_MODULE);
+	err = security_kernel_arg(KARG_MODULE);
 	if (err)
 		return err;
 
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 74d0bd7e76d7..d51a8ca97238 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -421,32 +421,6 @@ void ima_post_path_mknod(struct dentry *dentry)
 		iint->flags |= IMA_NEW_FILE;
 }
 
-/**
- * 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
- *
- * Permit reading a file based on policy. The policy rules are written
- * in terms of the policy identifier.  Appraising the integrity of
- * a file requires a file descriptor.
- *
- * For permission return 0, otherwise return -EACCES.
- */
-int ima_read_file(struct file *file, enum kernel_read_file_id read_id)
-{
-	bool sig_enforce = is_module_sig_enforced();
-
-	if (!file && read_id == READING_MODULE) {
-		if (!sig_enforce && (ima_appraise & IMA_APPRAISE_MODULES) &&
-		    (ima_appraise & IMA_APPRAISE_ENFORCE)) {
-			pr_err("impossible to appraise a module without a file descriptor. sig_enforce kernel parameter might help\n");
-			return -EACCES;	/* INTEGRITY_UNKNOWN */
-		}
-		return 0;	/* We rely on module signature checking */
-	}
-	return 0;
-}
-
 static int read_idmap[READING_MAX_ID] = {
 	[READING_FIRMWARE] = FIRMWARE_CHECK,
 	[READING_MODULE] = MODULE_CHECK,
@@ -474,21 +448,7 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
 	enum ima_hooks func;
 	u32 secid;
 
-	if (!file && read_id == READING_FIRMWARE) {
-		if ((ima_appraise & IMA_APPRAISE_FIRMWARE) &&
-		    (ima_appraise & IMA_APPRAISE_ENFORCE))
-			return -EACCES;	/* INTEGRITY_UNKNOWN */
-		return 0;
-	}
-
-	if (!file && read_id == READING_MODULE) /* MODULE_SIG_FORCE enabled */
-		return 0;
-
-	/* permit signed certs */
-	if (!file && read_id == READING_X509_CERTIFICATE)
-		return 0;
-
-	if (!file || !buf || size == 0) { /* should never happen */
+	if (!buf || size == 0) { /* should never happen */
 		if (ima_appraise & IMA_APPRAISE_ENFORCE)
 			return -EACCES;
 		return 0;
@@ -500,6 +460,40 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
 				   MAY_READ, func, 0);
 }
 
+/**
+ * ima_kernel_arg - pre-measure/appraise hook decision based on policy
+ * @id: caller identifier
+ *
+ * Permit using an argument to a syscall based on policy. The policy
+ * rules are written in terms of the policy identifier.
+ *
+ * For permission return 0, otherwise return -EACCES.
+ */
+int ima_kernel_arg(enum kernel_arg_id id)
+{
+	if (id == KARG_MODULE) {
+		bool sig_enforce = is_module_sig_enforced();
+		if (!sig_enforce && (ima_appraise & IMA_APPRAISE_MODULES) &&
+		    (ima_appraise & IMA_APPRAISE_ENFORCE)) {
+			pr_err("impossible to appraise a module without a file descriptor. sig_enforce kernel parameter might help\n");
+			return -EACCES;	/* INTEGRITY_UNKNOWN */
+		}
+		return 0;	/* We rely on module signature checking */
+	}
+	else if (id == KARG_FIRMWARE) {
+		if ((ima_appraise & IMA_APPRAISE_FIRMWARE) &&
+		    (ima_appraise & IMA_APPRAISE_ENFORCE))
+			return -EACCES;	/* INTEGRITY_UNKNOWN */
+		return 0;
+	}
+	else {
+		if (ima_appraise & IMA_APPRAISE_ENFORCE)
+			return -EACCES;
+		return 0;
+	}
+	return 0;
+}
+
 static int __init init_ima(void)
 {
 	int error;
diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
index 5fa191252c8f..f5333e5abac9 100644
--- a/security/loadpin/loadpin.c
+++ b/security/loadpin/loadpin.c
@@ -121,23 +121,24 @@ static void loadpin_sb_free_security(struct super_block *mnt_sb)
 	}
 }
 
-static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
+static int loadpin_read_data(enum kernel_read_data_id id)
 {
-	struct super_block *load_root;
-	const char *origin = kernel_read_file_id_str(id);
+	const char *origin = kernel_arg_id_str(id);
 
 	/* This handles the older init_module API that has a NULL file. */
-	if (!file) {
-		if (!enabled) {
-			report_load(origin, NULL, "old-api-pinning-ignored");
-			return 0;
-		}
-
-		report_load(origin, NULL, "old-api-denied");
-		return -EPERM;
+	if (!enabled) {
+		report_load(origin, NULL, "old-api-pinning-ignored");
+		return 0;
 	}
 
-	load_root = file->f_path.mnt->mnt_sb;
+	report_load(origin, NULL, "old-api-denied");
+	return -EPERM;
+}
+
+static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
+{
+	struct super_block *load_root;
+	const char *origin = kernel_read_file_id_str(id);
 
 	/* First loaded module/firmware defines the root for all others. */
 	spin_lock(&pinned_root_spinlock);
@@ -175,6 +176,7 @@ static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
 
 static struct security_hook_list loadpin_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(sb_free_security, loadpin_sb_free_security),
+	LSM_HOOK_INIT(kernel_read_data, loadpin_read_data),
 	LSM_HOOK_INIT(kernel_read_file, loadpin_read_file),
 };
 
diff --git a/security/security.c b/security/security.c
index 7bc2fde023a7..9b5f43c24ee2 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1033,14 +1033,19 @@ int security_kernel_module_request(char *kmod_name)
 	return call_int_hook(kernel_module_request, 0, kmod_name);
 }
 
-int security_kernel_read_file(struct file *file, enum kernel_read_file_id id)
+int security_kernel_arg(enum kernel_arg_id id)
 {
 	int ret;
 
-	ret = call_int_hook(kernel_read_file, 0, file, id);
-	if (ret)
-		return ret;
-	return ima_read_file(file, id);
+	ret = call_int_hook(kernel_arg, 0, id);
+	if (!ret)
+		ret = ima_kernel_arg(id);
+	return ret;
+}
+
+int security_kernel_read_file(struct file *file, enum kernel_read_file_id id)
+{
+	return call_int_hook(kernel_read_file, 0, file, id);
 }
 EXPORT_SYMBOL_GPL(security_kernel_read_file);
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 4cafe6a19167..76843099fed6 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4010,6 +4010,15 @@ static int selinux_kernel_module_request(char *kmod_name)
 			    SYSTEM__MODULE_REQUEST, &ad);
 }
 
+static int selinux_kernel_module_arg(void)
+{
+	/* init_module */
+	u32 sid = current_sid();
+	return avc_has_perm(&selinux_state,
+			    sid, sid, SECCLASS_SYSTEM,
+			    SYSTEM__MODULE_LOAD, NULL);
+}
+
 static int selinux_kernel_module_from_file(struct file *file)
 {
 	struct common_audit_data ad;
@@ -4018,12 +4027,6 @@ static int selinux_kernel_module_from_file(struct file *file)
 	u32 sid = current_sid();
 	int rc;
 
-	/* init_module */
-	if (file == NULL)
-		return avc_has_perm(&selinux_state,
-				    sid, sid, SECCLASS_SYSTEM,
-					SYSTEM__MODULE_LOAD, NULL);
-
 	/* finit_module */
 
 	ad.type = LSM_AUDIT_DATA_FILE;
@@ -4043,6 +4046,20 @@ static int selinux_kernel_module_from_file(struct file *file)
 				SYSTEM__MODULE_LOAD, &ad);
 }
 
+static int selinux_kernel_arg(enum kernel_arg_id id)
+{
+	int rc = 0;
+
+	switch (id) {
+	case KARG_MODULE:
+		rc = selinux_kernel_module_arg();
+		break;
+	default:
+		break;
+	}
+	return rc;
+}
+
 static int selinux_kernel_read_file(struct file *file,
 				    enum kernel_read_file_id id)
 {
@@ -6938,6 +6955,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(kernel_act_as, selinux_kernel_act_as),
 	LSM_HOOK_INIT(kernel_create_files_as, selinux_kernel_create_files_as),
 	LSM_HOOK_INIT(kernel_module_request, selinux_kernel_module_request),
+	LSM_HOOK_INIT(kernel_arg, selinux_kernel_arg),
 	LSM_HOOK_INIT(kernel_read_file, selinux_kernel_read_file),
 	LSM_HOOK_INIT(task_setpgid, selinux_task_setpgid),
 	LSM_HOOK_INIT(task_getpgid, selinux_task_getpgid),


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v3 2/7] kexec: add call to LSM hook in original kexec_load syscall

[Eric W. Biederman]

Mimi Zohar <zohar@linux.vnet.ibm.com> writes:

> In order for LSMs and IMA-appraisal to differentiate between the
> original and new syscalls, both the original and new syscalls must call
> an LSM hook.  This patch adds a call to security_kernel_read_data() in
> the original kexec syscall.

Until the lsm hook mess gets cleaned up.

Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>

>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> Cc: Eric Biederman <ebiederm@xmission.com>
> Cc: Luis R. Rodriguez <mcgrof@kernel.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: David Howells <dhowells@redhat.com>
> ---
>  kernel/kexec.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/kernel/kexec.c b/kernel/kexec.c
> index aed8fb2564b3..061ada41c18c 100644
> --- a/kernel/kexec.c
> +++ b/kernel/kexec.c
> @@ -11,6 +11,7 @@
>  #include <linux/capability.h>
>  #include <linux/mm.h>
>  #include <linux/file.h>
> +#include <linux/security.h>
>  #include <linux/kexec.h>
>  #include <linux/mutex.h>
>  #include <linux/list.h>
> @@ -195,10 +196,17 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
>  static inline int kexec_load_check(unsigned long nr_segments,
>  				   unsigned long flags)
>  {
> +	int result;
> +
>  	/* We only trust the superuser with rebooting the system. */
>  	if (!capable(CAP_SYS_BOOT) || kexec_load_disabled)
>  		return -EPERM;
>  
> +	/* Permit LSMs and IMA to fail the kexec */
> +	result = security_kernel_read_data(NULL, READING_KEXEC_IMAGE);
> +	if (result < 0)
> +		return result;
> +
>  	/*
>  	 * Verify we have a legal set of flags
>  	 * This leaves us room for future extensions.

[PATCH v3 2/7] kexec: add call to LSM hook in original kexec_load syscall

[Eric W. Biederman]

Mimi Zohar <zohar@linux.vnet.ibm.com> writes:

> In order for LSMs and IMA-appraisal to differentiate between the
> original and new syscalls, both the original and new syscalls must call
> an LSM hook.  This patch adds a call to security_kernel_read_data() in
> the original kexec syscall.

Until the lsm hook mess gets cleaned up.

Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>

>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> Cc: Eric Biederman <ebiederm@xmission.com>
> Cc: Luis R. Rodriguez <mcgrof@kernel.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: David Howells <dhowells@redhat.com>
> ---
>  kernel/kexec.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/kernel/kexec.c b/kernel/kexec.c
> index aed8fb2564b3..061ada41c18c 100644
> --- a/kernel/kexec.c
> +++ b/kernel/kexec.c
> @@ -11,6 +11,7 @@
>  #include <linux/capability.h>
>  #include <linux/mm.h>
>  #include <linux/file.h>
> +#include <linux/security.h>
>  #include <linux/kexec.h>
>  #include <linux/mutex.h>
>  #include <linux/list.h>
> @@ -195,10 +196,17 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
>  static inline int kexec_load_check(unsigned long nr_segments,
>  				   unsigned long flags)
>  {
> +	int result;
> +
>  	/* We only trust the superuser with rebooting the system. */
>  	if (!capable(CAP_SYS_BOOT) || kexec_load_disabled)
>  		return -EPERM;
>  
> +	/* Permit LSMs and IMA to fail the kexec */
> +	result = security_kernel_read_data(NULL, READING_KEXEC_IMAGE);
> +	if (result < 0)
> +		return result;
> +
>  	/*
>  	 * Verify we have a legal set of flags
>  	 * This leaves us room for future extensions.
--
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: [PATCH v3 2/7] kexec: add call to LSM hook in original kexec_load syscall

[Eric W. Biederman]

Mimi Zohar <zohar@linux.vnet.ibm.com> writes:

> In order for LSMs and IMA-appraisal to differentiate between the
> original and new syscalls, both the original and new syscalls must call
> an LSM hook.  This patch adds a call to security_kernel_read_data() in
> the original kexec syscall.

Until the lsm hook mess gets cleaned up.

Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>

>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> Cc: Eric Biederman <ebiederm@xmission.com>
> Cc: Luis R. Rodriguez <mcgrof@kernel.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: David Howells <dhowells@redhat.com>
> ---
>  kernel/kexec.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/kernel/kexec.c b/kernel/kexec.c
> index aed8fb2564b3..061ada41c18c 100644
> --- a/kernel/kexec.c
> +++ b/kernel/kexec.c
> @@ -11,6 +11,7 @@
>  #include <linux/capability.h>
>  #include <linux/mm.h>
>  #include <linux/file.h>
> +#include <linux/security.h>
>  #include <linux/kexec.h>
>  #include <linux/mutex.h>
>  #include <linux/list.h>
> @@ -195,10 +196,17 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
>  static inline int kexec_load_check(unsigned long nr_segments,
>  				   unsigned long flags)
>  {
> +	int result;
> +
>  	/* We only trust the superuser with rebooting the system. */
>  	if (!capable(CAP_SYS_BOOT) || kexec_load_disabled)
>  		return -EPERM;
>  
> +	/* Permit LSMs and IMA to fail the kexec */
> +	result = security_kernel_read_data(NULL, READING_KEXEC_IMAGE);
> +	if (result < 0)
> +		return result;
> +
>  	/*
>  	 * Verify we have a legal set of flags
>  	 * This leaves us room for future extensions.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v3 1/7] security: rename security_kernel_read_file() hook

[Mimi Zohar]

On Thu, 2018-05-24 at 15:49 -0500, Eric W. Biederman wrote:
> I already nacked this approach because the two cases don't
> share a bit of code.  When I looked closer it was even crazier.

It hasn't been clear what you meant by "the two cases don't share a
bit of code".  The first attempt called
security_kernel_read_file().  As per your comments, kexec_load doesn't
load a file.  Thinking it was a naming issue the second attempt
defined a wrapper named security_kernel_read_blob() for
security_kernel_read_file().  Still thinking it was a naming issue,
this attempt renamed the security_kernel_read_file() to
security_kernel_read_data().

> 
> The way ima uses this hook and the post_load hook today is a travesty.

Instead of having multiple functions, each a bit different, for
reading a file from the kernel, kernel_read_file() is a generic
implementation with both pre and post security calls.

 I think the pre and post security kernel_read_file() LSM hooks are
quite well thought out.  The security_kernel_read_file is called
before the kernel reads the file.  The security_kernel_post_read_file
is called after the kernel reads the file.

> The way the security_kernel_file_read and security_kernel_file_post_read
> are called today and are used by ima don't make the least little bit of
> sense.
> 
> Abusing security_kernel_file_read in the module loader and then abusing
> security_kernel_file_post_read in the firmware loader is insane.  The
> loadpin lsm could not even figure this out and so it failed to work
> because of these shenanighans.
> 
> Only implementing kernel_file_read to handle the !file case is pretty
> much insane.   There is no way this should be expanded to cover kexec
> until the code actually makes sense.  We need a maintainable kernel.

It wasn't implemented *only* for the IMA !file case, but as a generic
mechanism.  True, IMA is only using the security_kernel_read_file hook
for detecting !file, but the security_kernel_post_read_file hook is
used for verifying the file's integrity.

> Below is where I suggest you start on sorting out these security hooks.
> - Adding a security_kernel_arg to catch when you want to allow/deny the
>   use of an argument to a syscall.  What security_kernel_file_read and
>   security_kernel_file_post_read have been abused for.

Assuming we define a new LSM hook named "security_kernel_arg", would
we also define a new enumeration or could we still use the existing
kernel_read_file_id?

> 
> - Removing ima_file_read because it is completely subsumed by the new
>   call.

The existing IMA function wouldn't be removed, but renamed to whatever
the new LSM hook is named.

> 
> - Please note with adding this new hook there is no code shared between
>   the cases, and the lsm code becomes simpler shorter when it can assume
>   security_kernel_file_post_read always takes a struct file.  (Even with
>   the addition of a new security hook).

We would be defining a new LSM hook, not removing the existing
security_kernel_read_file hook, and only renaming the IMA usage of the
hook.

If defining a new LSM hook named security_kernel_arg makes you happy,
I don't have a problem with implementing it.

James, Casey, are you Ok with this?

Mimi

[PATCH v3 1/7] security: rename security_kernel_read_file() hook

[Mimi Zohar]

On Thu, 2018-05-24 at 15:49 -0500, Eric W. Biederman wrote:
> I already nacked this approach because the two cases don't
> share a bit of code.  When I looked closer it was even crazier.

It hasn't been clear what you meant by "the two cases don't share a
bit of code".??The first attempt called
security_kernel_read_file().??As per your comments, kexec_load doesn't
load a file.??Thinking it was a naming issue the second attempt
defined a wrapper named security_kernel_read_blob() for
security_kernel_read_file().??Still thinking it was a naming issue,
this attempt renamed the security_kernel_read_file() to
security_kernel_read_data().

> 
> The way ima uses this hook and the post_load hook today is a travesty.

Instead of having multiple functions, each a bit different, for
reading a file from the kernel, kernel_read_file() is a generic
implementation with both pre and post security calls.

 I think the pre and post security kernel_read_file() LSM hooks are
quite well thought out. ?The security_kernel_read_file is called
before the kernel reads the file. ?The security_kernel_post_read_file
is called after the kernel reads the file.

> The way the security_kernel_file_read and security_kernel_file_post_read
> are called today and are used by ima don't make the least little bit of
> sense.
> 
> Abusing security_kernel_file_read in the module loader and then abusing
> security_kernel_file_post_read in the firmware loader is insane.  The
> loadpin lsm could not even figure this out and so it failed to work
> because of these shenanighans.
> 
> Only implementing kernel_file_read to handle the !file case is pretty
> much insane.   There is no way this should be expanded to cover kexec
> until the code actually makes sense.  We need a maintainable kernel.

It wasn't implemented *only* for the IMA !file case, but as a generic
mechanism.??True, IMA is only using the security_kernel_read_file hook
for detecting !file, but the security_kernel_post_read_file hook is
used for verifying the file's integrity.

> Below is where I suggest you start on sorting out these security hooks.
> - Adding a security_kernel_arg to catch when you want to allow/deny the
>   use of an argument to a syscall.  What security_kernel_file_read and
>   security_kernel_file_post_read have been abused for.

Assuming we define a new LSM hook named "security_kernel_arg", would
we also define a new enumeration or could we still use the existing
kernel_read_file_id?

> 
> - Removing ima_file_read because it is completely subsumed by the new
>   call.

The existing IMA function wouldn't be removed, but renamed to whatever
the new LSM hook is named.

> 
> - Please note with adding this new hook there is no code shared between
>   the cases, and the lsm code becomes simpler shorter when it can assume
>   security_kernel_file_post_read always takes a struct file.  (Even with
>   the addition of a new security hook).

We would be defining a new LSM hook, not removing the existing
security_kernel_read_file hook, and only renaming the IMA usage of the
hook.

If defining a new LSM hook named security_kernel_arg makes you happy,
I don't have a problem with implementing it.

James, Casey, are you Ok with this?

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: [PATCH v3 1/7] security: rename security_kernel_read_file() hook

[Mimi Zohar]

On Thu, 2018-05-24 at 15:49 -0500, Eric W. Biederman wrote:
> I already nacked this approach because the two cases don't
> share a bit of code.  When I looked closer it was even crazier.

It hasn't been clear what you meant by "the two cases don't share a
bit of code".  The first attempt called
security_kernel_read_file().  As per your comments, kexec_load doesn't
load a file.  Thinking it was a naming issue the second attempt
defined a wrapper named security_kernel_read_blob() for
security_kernel_read_file().  Still thinking it was a naming issue,
this attempt renamed the security_kernel_read_file() to
security_kernel_read_data().

> 
> The way ima uses this hook and the post_load hook today is a travesty.

Instead of having multiple functions, each a bit different, for
reading a file from the kernel, kernel_read_file() is a generic
implementation with both pre and post security calls.

 I think the pre and post security kernel_read_file() LSM hooks are
quite well thought out.  The security_kernel_read_file is called
before the kernel reads the file.  The security_kernel_post_read_file
is called after the kernel reads the file.

> The way the security_kernel_file_read and security_kernel_file_post_read
> are called today and are used by ima don't make the least little bit of
> sense.
> 
> Abusing security_kernel_file_read in the module loader and then abusing
> security_kernel_file_post_read in the firmware loader is insane.  The
> loadpin lsm could not even figure this out and so it failed to work
> because of these shenanighans.
> 
> Only implementing kernel_file_read to handle the !file case is pretty
> much insane.   There is no way this should be expanded to cover kexec
> until the code actually makes sense.  We need a maintainable kernel.

It wasn't implemented *only* for the IMA !file case, but as a generic
mechanism.  True, IMA is only using the security_kernel_read_file hook
for detecting !file, but the security_kernel_post_read_file hook is
used for verifying the file's integrity.

> Below is where I suggest you start on sorting out these security hooks.
> - Adding a security_kernel_arg to catch when you want to allow/deny the
>   use of an argument to a syscall.  What security_kernel_file_read and
>   security_kernel_file_post_read have been abused for.

Assuming we define a new LSM hook named "security_kernel_arg", would
we also define a new enumeration or could we still use the existing
kernel_read_file_id?

> 
> - Removing ima_file_read because it is completely subsumed by the new
>   call.

The existing IMA function wouldn't be removed, but renamed to whatever
the new LSM hook is named.

> 
> - Please note with adding this new hook there is no code shared between
>   the cases, and the lsm code becomes simpler shorter when it can assume
>   security_kernel_file_post_read always takes a struct file.  (Even with
>   the addition of a new security hook).

We would be defining a new LSM hook, not removing the existing
security_kernel_read_file hook, and only renaming the IMA usage of the
hook.

If defining a new LSM hook named security_kernel_arg makes you happy,
I don't have a problem with implementing it.

James, Casey, are you Ok with this?

Mimi

Re: [PATCH v3 1/7] security: rename security_kernel_read_file() hook

[Mimi Zohar]

On Thu, 2018-05-24 at 15:49 -0500, Eric W. Biederman wrote:
> I already nacked this approach because the two cases don't
> share a bit of code.  When I looked closer it was even crazier.

It hasn't been clear what you meant by "the two cases don't share a
bit of code".  The first attempt called
security_kernel_read_file().  As per your comments, kexec_load doesn't
load a file.  Thinking it was a naming issue the second attempt
defined a wrapper named security_kernel_read_blob() for
security_kernel_read_file().  Still thinking it was a naming issue,
this attempt renamed the security_kernel_read_file() to
security_kernel_read_data().

> 
> The way ima uses this hook and the post_load hook today is a travesty.

Instead of having multiple functions, each a bit different, for
reading a file from the kernel, kernel_read_file() is a generic
implementation with both pre and post security calls.

 I think the pre and post security kernel_read_file() LSM hooks are
quite well thought out.  The security_kernel_read_file is called
before the kernel reads the file.  The security_kernel_post_read_file
is called after the kernel reads the file.

> The way the security_kernel_file_read and security_kernel_file_post_read
> are called today and are used by ima don't make the least little bit of
> sense.
> 
> Abusing security_kernel_file_read in the module loader and then abusing
> security_kernel_file_post_read in the firmware loader is insane.  The
> loadpin lsm could not even figure this out and so it failed to work
> because of these shenanighans.
> 
> Only implementing kernel_file_read to handle the !file case is pretty
> much insane.   There is no way this should be expanded to cover kexec
> until the code actually makes sense.  We need a maintainable kernel.

It wasn't implemented *only* for the IMA !file case, but as a generic
mechanism.  True, IMA is only using the security_kernel_read_file hook
for detecting !file, but the security_kernel_post_read_file hook is
used for verifying the file's integrity.

> Below is where I suggest you start on sorting out these security hooks.
> - Adding a security_kernel_arg to catch when you want to allow/deny the
>   use of an argument to a syscall.  What security_kernel_file_read and
>   security_kernel_file_post_read have been abused for.

Assuming we define a new LSM hook named "security_kernel_arg", would
we also define a new enumeration or could we still use the existing
kernel_read_file_id?

> 
> - Removing ima_file_read because it is completely subsumed by the new
>   call.

The existing IMA function wouldn't be removed, but renamed to whatever
the new LSM hook is named.

> 
> - Please note with adding this new hook there is no code shared between
>   the cases, and the lsm code becomes simpler shorter when it can assume
>   security_kernel_file_post_read always takes a struct file.  (Even with
>   the addition of a new security hook).

We would be defining a new LSM hook, not removing the existing
security_kernel_read_file hook, and only renaming the IMA usage of the
hook.

If defining a new LSM hook named security_kernel_arg makes you happy,
I don't have a problem with implementing it.

James, Casey, are you Ok with this?

Mimi


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v3 1/7] security: rename security_kernel_read_file() hook

[Mimi Zohar]

On Thu, 2018-05-24 at 15:49 -0500, Eric W. Biederman wrote:

Thank you for the sample code below.  It needs to be broken up into
proper patches, with some changes, but it is a good start.

Mimi 

> diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
> index 358354148dec..04536ff81bd2 100644
> --- a/drivers/base/firmware_loader/fallback.c
> +++ b/drivers/base/firmware_loader/fallback.c
> @@ -294,9 +294,7 @@ static ssize_t firmware_loading_store(struct device *dev,
>  				dev_err(dev, "%s: map pages failed\n",
>  					__func__);
>  			else
> -				rc = security_kernel_post_read_file(NULL,
> -						fw_priv->data, fw_priv->size,
> -						READING_FIRMWARE);
> +				rc = security_kernel_arg(KARG_FIRMWARE);
> 
>  			/*
>  			 * Same logic as fw_load_abort, only the DONE bit
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 0e4647e0eb60..9fb42736ba29 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -11,6 +11,7 @@
>  #define _LINUX_IMA_H
> 
>  #include <linux/fs.h>
> +#include <linux/security.h>
>  #include <linux/kexec.h>
>  struct linux_binprm;
> 
> @@ -19,7 +20,7 @@ extern int ima_bprm_check(struct linux_binprm *bprm);
>  extern int ima_file_check(struct file *file, int mask, int opened);
>  extern void ima_file_free(struct file *file);
>  extern int ima_file_mmap(struct file *file, unsigned long prot);
> -extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
> +extern int ima_kernel_arg(enum kernel_arg_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);
> @@ -49,7 +50,7 @@ static inline int ima_file_mmap(struct file *file, unsigned long prot)
>  	return 0;
>  }
> 
> -static inline int ima_read_file(struct file *file, enum kernel_read_file_id id)
> +static inline int ima_kernel_arg(enum kernel_arg_id id)
>  {
>  	return 0;
>  }
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 9d0b286f3dba..7f8bc3030784 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -576,6 +576,10 @@
>   *	userspace to load a kernel module with the given name.
>   *	@kmod_name name of the module requested by the kernel
>   *	Return 0 if successful.
> + * @kernel_arg:
> + *	Use a syscall argument
> + *	@id kernel argument identifier
> + *	Return 0 if permission is granted.
>   * @kernel_read_file:
>   *	Read a file specified by userspace.
>   *	@file contains the file structure pointing to the file being read
> @@ -1577,6 +1581,7 @@ union security_list_options {
>  	int (*kernel_act_as)(struct cred *new, u32 secid);
>  	int (*kernel_create_files_as)(struct cred *new, struct inode *inode);
>  	int (*kernel_module_request)(char *kmod_name);
> +	int (*kernel_arg)(enum kernel_arg_id id);
>  	int (*kernel_read_file)(struct file *file, enum kernel_read_file_id id);
>  	int (*kernel_post_read_file)(struct file *file, char *buf, loff_t size,
>  				     enum kernel_read_file_id id);
> @@ -1866,6 +1871,7 @@ struct security_hook_heads {
>  	struct hlist_head cred_getsecid;
>  	struct hlist_head kernel_act_as;
>  	struct hlist_head kernel_create_files_as;
> +	struct hlist_head kernel_arg;
>  	struct hlist_head kernel_read_file;
>  	struct hlist_head kernel_post_read_file;
>  	struct hlist_head kernel_module_request;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 200920f521a1..6cf1bd87f041 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -159,6 +159,32 @@ extern int mmap_min_addr_handler(struct ctl_table *table, int write,
>  typedef int (*initxattrs) (struct inode *inode,
>  			   const struct xattr *xattr_array, void *fs_data);
> 
> +#define __kernel_arg_id(id) \
> +	id(UNKNOWN, unknown)		\
> +	id(FIRMWARE, firmware)		\
> +	id(MODULE, kernel-module)	\
> +	id(MAX_ID, )
> +
> +#define __karg_enumify(ENUM, dummy) KARG_ ## ENUM,
> +#define __karg_stringify(dummy, str) #str,
> +
> +enum kernel_arg_id {
> +	__kernel_arg_id(__karg_enumify)
> +};
> +
> +static const char * const kernel_arg_str[] = {
> +	__kernel_arg_id(__karg_stringify)
> +};
> +
> +static inline const char *kernel_arg_id_str(enum kernel_arg_id id)
> +{
> +	if ((unsigned)id >= KARG_MAX_ID)
> +		return kernel_arg_str[KARG_UNKNOWN];
> +
> +	return kernel_arg_str[id];
> +}
> +
> +
>  #ifdef CONFIG_SECURITY
> 
>  struct security_mnt_opts {
> @@ -326,6 +352,7 @@ void security_cred_getsecid(const struct cred *c, u32 *secid);
>  int security_kernel_act_as(struct cred *new, u32 secid);
>  int security_kernel_create_files_as(struct cred *new, struct inode *inode);
>  int security_kernel_module_request(char *kmod_name);
> +int security_kernel_arg(enum kernel_arg_id id);
>  int security_kernel_read_file(struct file *file, enum kernel_read_file_id id);
>  int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
>  				   enum kernel_read_file_id id);
> @@ -923,6 +950,11 @@ static inline int security_kernel_module_request(char *kmod_name)
>  	return 0;
>  }
> 
> +static inline int security_kernel_arg(enum kernel_arg_id id)
> +{
> +	return 0;
> +}
> +
>  static inline int security_kernel_read_file(struct file *file,
>  					    enum kernel_read_file_id id)
>  {
> diff --git a/kernel/module.c b/kernel/module.c
> index ce8066b88178..03a1dd21ad4a 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2879,7 +2879,7 @@ static int copy_module_from_user(const void __user *umod, unsigned long len,
>  	if (info->len < sizeof(*(info->hdr)))
>  		return -ENOEXEC;
> 
> -	err = security_kernel_read_file(NULL, READING_MODULE);
> +	err = security_kernel_arg(KARG_MODULE);
>  	if (err)
>  		return err;
> 
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 74d0bd7e76d7..d51a8ca97238 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -421,32 +421,6 @@ void ima_post_path_mknod(struct dentry *dentry)
>  		iint->flags |= IMA_NEW_FILE;
>  }
> 
> -/**
> - * 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
> - *
> - * Permit reading a file based on policy. The policy rules are written
> - * in terms of the policy identifier.  Appraising the integrity of
> - * a file requires a file descriptor.
> - *
> - * For permission return 0, otherwise return -EACCES.
> - */
> -int ima_read_file(struct file *file, enum kernel_read_file_id read_id)
> -{
> -	bool sig_enforce = is_module_sig_enforced();
> -
> -	if (!file && read_id == READING_MODULE) {
> -		if (!sig_enforce && (ima_appraise & IMA_APPRAISE_MODULES) &&
> -		    (ima_appraise & IMA_APPRAISE_ENFORCE)) {
> -			pr_err("impossible to appraise a module without a file descriptor. sig_enforce kernel parameter might help\n");
> -			return -EACCES;	/* INTEGRITY_UNKNOWN */
> -		}
> -		return 0;	/* We rely on module signature checking */
> -	}
> -	return 0;
> -}
> -
>  static int read_idmap[READING_MAX_ID] = {
>  	[READING_FIRMWARE] = FIRMWARE_CHECK,
>  	[READING_MODULE] = MODULE_CHECK,
> @@ -474,21 +448,7 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
>  	enum ima_hooks func;
>  	u32 secid;
> 
> -	if (!file && read_id == READING_FIRMWARE) {
> -		if ((ima_appraise & IMA_APPRAISE_FIRMWARE) &&
> -		    (ima_appraise & IMA_APPRAISE_ENFORCE))
> -			return -EACCES;	/* INTEGRITY_UNKNOWN */
> -		return 0;
> -	}
> -
> -	if (!file && read_id == READING_MODULE) /* MODULE_SIG_FORCE enabled */
> -		return 0;
> -
> -	/* permit signed certs */
> -	if (!file && read_id == READING_X509_CERTIFICATE)
> -		return 0;
> -
> -	if (!file || !buf || size == 0) { /* should never happen */
> +	if (!buf || size == 0) { /* should never happen */
>  		if (ima_appraise & IMA_APPRAISE_ENFORCE)
>  			return -EACCES;
>  		return 0;
> @@ -500,6 +460,40 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
>  				   MAY_READ, func, 0);
>  }
> 
> +/**
> + * ima_kernel_arg - pre-measure/appraise hook decision based on policy
> + * @id: caller identifier
> + *
> + * Permit using an argument to a syscall based on policy. The policy
> + * rules are written in terms of the policy identifier.
> + *
> + * For permission return 0, otherwise return -EACCES.
> + */
> +int ima_kernel_arg(enum kernel_arg_id id)
> +{
> +	if (id == KARG_MODULE) {
> +		bool sig_enforce = is_module_sig_enforced();
> +		if (!sig_enforce && (ima_appraise & IMA_APPRAISE_MODULES) &&
> +		    (ima_appraise & IMA_APPRAISE_ENFORCE)) {
> +			pr_err("impossible to appraise a module without a file descriptor. sig_enforce kernel parameter might help\n");
> +			return -EACCES;	/* INTEGRITY_UNKNOWN */
> +		}
> +		return 0;	/* We rely on module signature checking */
> +	}
> +	else if (id == KARG_FIRMWARE) {
> +		if ((ima_appraise & IMA_APPRAISE_FIRMWARE) &&
> +		    (ima_appraise & IMA_APPRAISE_ENFORCE))
> +			return -EACCES;	/* INTEGRITY_UNKNOWN */
> +		return 0;
> +	}
> +	else {
> +		if (ima_appraise & IMA_APPRAISE_ENFORCE)
> +			return -EACCES;
> +		return 0;
> +	}
> +	return 0;
> +}
> +
>  static int __init init_ima(void)
>  {
>  	int error;
> diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
> index 5fa191252c8f..f5333e5abac9 100644
> --- a/security/loadpin/loadpin.c
> +++ b/security/loadpin/loadpin.c
> @@ -121,23 +121,24 @@ static void loadpin_sb_free_security(struct super_block *mnt_sb)
>  	}
>  }
> 
> -static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
> +static int loadpin_read_data(enum kernel_read_data_id id)
>  {
> -	struct super_block *load_root;
> -	const char *origin = kernel_read_file_id_str(id);
> +	const char *origin = kernel_arg_id_str(id);
> 
>  	/* This handles the older init_module API that has a NULL file. */
> -	if (!file) {
> -		if (!enabled) {
> -			report_load(origin, NULL, "old-api-pinning-ignored");
> -			return 0;
> -		}
> -
> -		report_load(origin, NULL, "old-api-denied");
> -		return -EPERM;
> +	if (!enabled) {
> +		report_load(origin, NULL, "old-api-pinning-ignored");
> +		return 0;
>  	}
> 
> -	load_root = file->f_path.mnt->mnt_sb;
> +	report_load(origin, NULL, "old-api-denied");
> +	return -EPERM;
> +}
> +
> +static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
> +{
> +	struct super_block *load_root;
> +	const char *origin = kernel_read_file_id_str(id);
> 
>  	/* First loaded module/firmware defines the root for all others. */
>  	spin_lock(&pinned_root_spinlock);
> @@ -175,6 +176,7 @@ static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
> 
>  static struct security_hook_list loadpin_hooks[] __lsm_ro_after_init = {
>  	LSM_HOOK_INIT(sb_free_security, loadpin_sb_free_security),
> +	LSM_HOOK_INIT(kernel_read_data, loadpin_read_data),
>  	LSM_HOOK_INIT(kernel_read_file, loadpin_read_file),
>  };
> 
> diff --git a/security/security.c b/security/security.c
> index 7bc2fde023a7..9b5f43c24ee2 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1033,14 +1033,19 @@ int security_kernel_module_request(char *kmod_name)
>  	return call_int_hook(kernel_module_request, 0, kmod_name);
>  }
> 
> -int security_kernel_read_file(struct file *file, enum kernel_read_file_id id)
> +int security_kernel_arg(enum kernel_arg_id id)
>  {
>  	int ret;
> 
> -	ret = call_int_hook(kernel_read_file, 0, file, id);
> -	if (ret)
> -		return ret;
> -	return ima_read_file(file, id);
> +	ret = call_int_hook(kernel_arg, 0, id);
> +	if (!ret)
> +		ret = ima_kernel_arg(id);
> +	return ret;
> +}
> +
> +int security_kernel_read_file(struct file *file, enum kernel_read_file_id id)
> +{
> +	return call_int_hook(kernel_read_file, 0, file, id);
>  }
>  EXPORT_SYMBOL_GPL(security_kernel_read_file);
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 4cafe6a19167..76843099fed6 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4010,6 +4010,15 @@ static int selinux_kernel_module_request(char *kmod_name)
>  			    SYSTEM__MODULE_REQUEST, &ad);
>  }
> 
> +static int selinux_kernel_module_arg(void)
> +{
> +	/* init_module */
> +	u32 sid = current_sid();
> +	return avc_has_perm(&selinux_state,
> +			    sid, sid, SECCLASS_SYSTEM,
> +			    SYSTEM__MODULE_LOAD, NULL);
> +}
> +
>  static int selinux_kernel_module_from_file(struct file *file)
>  {
>  	struct common_audit_data ad;
> @@ -4018,12 +4027,6 @@ static int selinux_kernel_module_from_file(struct file *file)
>  	u32 sid = current_sid();
>  	int rc;
> 
> -	/* init_module */
> -	if (file == NULL)
> -		return avc_has_perm(&selinux_state,
> -				    sid, sid, SECCLASS_SYSTEM,
> -					SYSTEM__MODULE_LOAD, NULL);
> -
>  	/* finit_module */
> 
>  	ad.type = LSM_AUDIT_DATA_FILE;
> @@ -4043,6 +4046,20 @@ static int selinux_kernel_module_from_file(struct file *file)
>  				SYSTEM__MODULE_LOAD, &ad);
>  }
> 
> +static int selinux_kernel_arg(enum kernel_arg_id id)
> +{
> +	int rc = 0;
> +
> +	switch (id) {
> +	case KARG_MODULE:
> +		rc = selinux_kernel_module_arg();
> +		break;
> +	default:
> +		break;
> +	}
> +	return rc;
> +}
> +
>  static int selinux_kernel_read_file(struct file *file,
>  				    enum kernel_read_file_id id)
>  {
> @@ -6938,6 +6955,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
>  	LSM_HOOK_INIT(kernel_act_as, selinux_kernel_act_as),
>  	LSM_HOOK_INIT(kernel_create_files_as, selinux_kernel_create_files_as),
>  	LSM_HOOK_INIT(kernel_module_request, selinux_kernel_module_request),
> +	LSM_HOOK_INIT(kernel_arg, selinux_kernel_arg),
>  	LSM_HOOK_INIT(kernel_read_file, selinux_kernel_read_file),
>  	LSM_HOOK_INIT(task_setpgid, selinux_task_setpgid),
>  	LSM_HOOK_INIT(task_getpgid, selinux_task_getpgid),
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH v3 1/7] security: rename security_kernel_read_file() hook

[Mimi Zohar]

On Thu, 2018-05-24 at 15:49 -0500, Eric W. Biederman wrote:

Thank you for the sample code below. ?It needs to be broken up into
proper patches, with some changes, but it is a good start.

Mimi?

> diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
> index 358354148dec..04536ff81bd2 100644
> --- a/drivers/base/firmware_loader/fallback.c
> +++ b/drivers/base/firmware_loader/fallback.c
> @@ -294,9 +294,7 @@ static ssize_t firmware_loading_store(struct device *dev,
>  				dev_err(dev, "%s: map pages failed\n",
>  					__func__);
>  			else
> -				rc = security_kernel_post_read_file(NULL,
> -						fw_priv->data, fw_priv->size,
> -						READING_FIRMWARE);
> +				rc = security_kernel_arg(KARG_FIRMWARE);
> 
>  			/*
>  			 * Same logic as fw_load_abort, only the DONE bit
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 0e4647e0eb60..9fb42736ba29 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -11,6 +11,7 @@
>  #define _LINUX_IMA_H
> 
>  #include <linux/fs.h>
> +#include <linux/security.h>
>  #include <linux/kexec.h>
>  struct linux_binprm;
> 
> @@ -19,7 +20,7 @@ extern int ima_bprm_check(struct linux_binprm *bprm);
>  extern int ima_file_check(struct file *file, int mask, int opened);
>  extern void ima_file_free(struct file *file);
>  extern int ima_file_mmap(struct file *file, unsigned long prot);
> -extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
> +extern int ima_kernel_arg(enum kernel_arg_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);
> @@ -49,7 +50,7 @@ static inline int ima_file_mmap(struct file *file, unsigned long prot)
>  	return 0;
>  }
> 
> -static inline int ima_read_file(struct file *file, enum kernel_read_file_id id)
> +static inline int ima_kernel_arg(enum kernel_arg_id id)
>  {
>  	return 0;
>  }
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 9d0b286f3dba..7f8bc3030784 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -576,6 +576,10 @@
>   *	userspace to load a kernel module with the given name.
>   *	@kmod_name name of the module requested by the kernel
>   *	Return 0 if successful.
> + * @kernel_arg:
> + *	Use a syscall argument
> + *	@id kernel argument identifier
> + *	Return 0 if permission is granted.
>   * @kernel_read_file:
>   *	Read a file specified by userspace.
>   *	@file contains the file structure pointing to the file being read
> @@ -1577,6 +1581,7 @@ union security_list_options {
>  	int (*kernel_act_as)(struct cred *new, u32 secid);
>  	int (*kernel_create_files_as)(struct cred *new, struct inode *inode);
>  	int (*kernel_module_request)(char *kmod_name);
> +	int (*kernel_arg)(enum kernel_arg_id id);
>  	int (*kernel_read_file)(struct file *file, enum kernel_read_file_id id);
>  	int (*kernel_post_read_file)(struct file *file, char *buf, loff_t size,
>  				     enum kernel_read_file_id id);
> @@ -1866,6 +1871,7 @@ struct security_hook_heads {
>  	struct hlist_head cred_getsecid;
>  	struct hlist_head kernel_act_as;
>  	struct hlist_head kernel_create_files_as;
> +	struct hlist_head kernel_arg;
>  	struct hlist_head kernel_read_file;
>  	struct hlist_head kernel_post_read_file;
>  	struct hlist_head kernel_module_request;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 200920f521a1..6cf1bd87f041 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -159,6 +159,32 @@ extern int mmap_min_addr_handler(struct ctl_table *table, int write,
>  typedef int (*initxattrs) (struct inode *inode,
>  			   const struct xattr *xattr_array, void *fs_data);
> 
> +#define __kernel_arg_id(id) \
> +	id(UNKNOWN, unknown)		\
> +	id(FIRMWARE, firmware)		\
> +	id(MODULE, kernel-module)	\
> +	id(MAX_ID, )
> +
> +#define __karg_enumify(ENUM, dummy) KARG_ ## ENUM,
> +#define __karg_stringify(dummy, str) #str,
> +
> +enum kernel_arg_id {
> +	__kernel_arg_id(__karg_enumify)
> +};
> +
> +static const char * const kernel_arg_str[] = {
> +	__kernel_arg_id(__karg_stringify)
> +};
> +
> +static inline const char *kernel_arg_id_str(enum kernel_arg_id id)
> +{
> +	if ((unsigned)id >= KARG_MAX_ID)
> +		return kernel_arg_str[KARG_UNKNOWN];
> +
> +	return kernel_arg_str[id];
> +}
> +
> +
>  #ifdef CONFIG_SECURITY
> 
>  struct security_mnt_opts {
> @@ -326,6 +352,7 @@ void security_cred_getsecid(const struct cred *c, u32 *secid);
>  int security_kernel_act_as(struct cred *new, u32 secid);
>  int security_kernel_create_files_as(struct cred *new, struct inode *inode);
>  int security_kernel_module_request(char *kmod_name);
> +int security_kernel_arg(enum kernel_arg_id id);
>  int security_kernel_read_file(struct file *file, enum kernel_read_file_id id);
>  int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
>  				   enum kernel_read_file_id id);
> @@ -923,6 +950,11 @@ static inline int security_kernel_module_request(char *kmod_name)
>  	return 0;
>  }
> 
> +static inline int security_kernel_arg(enum kernel_arg_id id)
> +{
> +	return 0;
> +}
> +
>  static inline int security_kernel_read_file(struct file *file,
>  					    enum kernel_read_file_id id)
>  {
> diff --git a/kernel/module.c b/kernel/module.c
> index ce8066b88178..03a1dd21ad4a 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2879,7 +2879,7 @@ static int copy_module_from_user(const void __user *umod, unsigned long len,
>  	if (info->len < sizeof(*(info->hdr)))
>  		return -ENOEXEC;
> 
> -	err = security_kernel_read_file(NULL, READING_MODULE);
> +	err = security_kernel_arg(KARG_MODULE);
>  	if (err)
>  		return err;
> 
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 74d0bd7e76d7..d51a8ca97238 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -421,32 +421,6 @@ void ima_post_path_mknod(struct dentry *dentry)
>  		iint->flags |= IMA_NEW_FILE;
>  }
> 
> -/**
> - * 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
> - *
> - * Permit reading a file based on policy. The policy rules are written
> - * in terms of the policy identifier.  Appraising the integrity of
> - * a file requires a file descriptor.
> - *
> - * For permission return 0, otherwise return -EACCES.
> - */
> -int ima_read_file(struct file *file, enum kernel_read_file_id read_id)
> -{
> -	bool sig_enforce = is_module_sig_enforced();
> -
> -	if (!file && read_id == READING_MODULE) {
> -		if (!sig_enforce && (ima_appraise & IMA_APPRAISE_MODULES) &&
> -		    (ima_appraise & IMA_APPRAISE_ENFORCE)) {
> -			pr_err("impossible to appraise a module without a file descriptor. sig_enforce kernel parameter might help\n");
> -			return -EACCES;	/* INTEGRITY_UNKNOWN */
> -		}
> -		return 0;	/* We rely on module signature checking */
> -	}
> -	return 0;
> -}
> -
>  static int read_idmap[READING_MAX_ID] = {
>  	[READING_FIRMWARE] = FIRMWARE_CHECK,
>  	[READING_MODULE] = MODULE_CHECK,
> @@ -474,21 +448,7 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
>  	enum ima_hooks func;
>  	u32 secid;
> 
> -	if (!file && read_id == READING_FIRMWARE) {
> -		if ((ima_appraise & IMA_APPRAISE_FIRMWARE) &&
> -		    (ima_appraise & IMA_APPRAISE_ENFORCE))
> -			return -EACCES;	/* INTEGRITY_UNKNOWN */
> -		return 0;
> -	}
> -
> -	if (!file && read_id == READING_MODULE) /* MODULE_SIG_FORCE enabled */
> -		return 0;
> -
> -	/* permit signed certs */
> -	if (!file && read_id == READING_X509_CERTIFICATE)
> -		return 0;
> -
> -	if (!file || !buf || size == 0) { /* should never happen */
> +	if (!buf || size == 0) { /* should never happen */
>  		if (ima_appraise & IMA_APPRAISE_ENFORCE)
>  			return -EACCES;
>  		return 0;
> @@ -500,6 +460,40 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
>  				   MAY_READ, func, 0);
>  }
> 
> +/**
> + * ima_kernel_arg - pre-measure/appraise hook decision based on policy
> + * @id: caller identifier
> + *
> + * Permit using an argument to a syscall based on policy. The policy
> + * rules are written in terms of the policy identifier.
> + *
> + * For permission return 0, otherwise return -EACCES.
> + */
> +int ima_kernel_arg(enum kernel_arg_id id)
> +{
> +	if (id == KARG_MODULE) {
> +		bool sig_enforce = is_module_sig_enforced();
> +		if (!sig_enforce && (ima_appraise & IMA_APPRAISE_MODULES) &&
> +		    (ima_appraise & IMA_APPRAISE_ENFORCE)) {
> +			pr_err("impossible to appraise a module without a file descriptor. sig_enforce kernel parameter might help\n");
> +			return -EACCES;	/* INTEGRITY_UNKNOWN */
> +		}
> +		return 0;	/* We rely on module signature checking */
> +	}
> +	else if (id == KARG_FIRMWARE) {
> +		if ((ima_appraise & IMA_APPRAISE_FIRMWARE) &&
> +		    (ima_appraise & IMA_APPRAISE_ENFORCE))
> +			return -EACCES;	/* INTEGRITY_UNKNOWN */
> +		return 0;
> +	}
> +	else {
> +		if (ima_appraise & IMA_APPRAISE_ENFORCE)
> +			return -EACCES;
> +		return 0;
> +	}
> +	return 0;
> +}
> +
>  static int __init init_ima(void)
>  {
>  	int error;
> diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
> index 5fa191252c8f..f5333e5abac9 100644
> --- a/security/loadpin/loadpin.c
> +++ b/security/loadpin/loadpin.c
> @@ -121,23 +121,24 @@ static void loadpin_sb_free_security(struct super_block *mnt_sb)
>  	}
>  }
> 
> -static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
> +static int loadpin_read_data(enum kernel_read_data_id id)
>  {
> -	struct super_block *load_root;
> -	const char *origin = kernel_read_file_id_str(id);
> +	const char *origin = kernel_arg_id_str(id);
> 
>  	/* This handles the older init_module API that has a NULL file. */
> -	if (!file) {
> -		if (!enabled) {
> -			report_load(origin, NULL, "old-api-pinning-ignored");
> -			return 0;
> -		}
> -
> -		report_load(origin, NULL, "old-api-denied");
> -		return -EPERM;
> +	if (!enabled) {
> +		report_load(origin, NULL, "old-api-pinning-ignored");
> +		return 0;
>  	}
> 
> -	load_root = file->f_path.mnt->mnt_sb;
> +	report_load(origin, NULL, "old-api-denied");
> +	return -EPERM;
> +}
> +
> +static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
> +{
> +	struct super_block *load_root;
> +	const char *origin = kernel_read_file_id_str(id);
> 
>  	/* First loaded module/firmware defines the root for all others. */
>  	spin_lock(&pinned_root_spinlock);
> @@ -175,6 +176,7 @@ static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
> 
>  static struct security_hook_list loadpin_hooks[] __lsm_ro_after_init = {
>  	LSM_HOOK_INIT(sb_free_security, loadpin_sb_free_security),
> +	LSM_HOOK_INIT(kernel_read_data, loadpin_read_data),
>  	LSM_HOOK_INIT(kernel_read_file, loadpin_read_file),
>  };
> 
> diff --git a/security/security.c b/security/security.c
> index 7bc2fde023a7..9b5f43c24ee2 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1033,14 +1033,19 @@ int security_kernel_module_request(char *kmod_name)
>  	return call_int_hook(kernel_module_request, 0, kmod_name);
>  }
> 
> -int security_kernel_read_file(struct file *file, enum kernel_read_file_id id)
> +int security_kernel_arg(enum kernel_arg_id id)
>  {
>  	int ret;
> 
> -	ret = call_int_hook(kernel_read_file, 0, file, id);
> -	if (ret)
> -		return ret;
> -	return ima_read_file(file, id);
> +	ret = call_int_hook(kernel_arg, 0, id);
> +	if (!ret)
> +		ret = ima_kernel_arg(id);
> +	return ret;
> +}
> +
> +int security_kernel_read_file(struct file *file, enum kernel_read_file_id id)
> +{
> +	return call_int_hook(kernel_read_file, 0, file, id);
>  }
>  EXPORT_SYMBOL_GPL(security_kernel_read_file);
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 4cafe6a19167..76843099fed6 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4010,6 +4010,15 @@ static int selinux_kernel_module_request(char *kmod_name)
>  			    SYSTEM__MODULE_REQUEST, &ad);
>  }
> 
> +static int selinux_kernel_module_arg(void)
> +{
> +	/* init_module */
> +	u32 sid = current_sid();
> +	return avc_has_perm(&selinux_state,
> +			    sid, sid, SECCLASS_SYSTEM,
> +			    SYSTEM__MODULE_LOAD, NULL);
> +}
> +
>  static int selinux_kernel_module_from_file(struct file *file)
>  {
>  	struct common_audit_data ad;
> @@ -4018,12 +4027,6 @@ static int selinux_kernel_module_from_file(struct file *file)
>  	u32 sid = current_sid();
>  	int rc;
> 
> -	/* init_module */
> -	if (file == NULL)
> -		return avc_has_perm(&selinux_state,
> -				    sid, sid, SECCLASS_SYSTEM,
> -					SYSTEM__MODULE_LOAD, NULL);
> -
>  	/* finit_module */
> 
>  	ad.type = LSM_AUDIT_DATA_FILE;
> @@ -4043,6 +4046,20 @@ static int selinux_kernel_module_from_file(struct file *file)
>  				SYSTEM__MODULE_LOAD, &ad);
>  }
> 
> +static int selinux_kernel_arg(enum kernel_arg_id id)
> +{
> +	int rc = 0;
> +
> +	switch (id) {
> +	case KARG_MODULE:
> +		rc = selinux_kernel_module_arg();
> +		break;
> +	default:
> +		break;
> +	}
> +	return rc;
> +}
> +
>  static int selinux_kernel_read_file(struct file *file,
>  				    enum kernel_read_file_id id)
>  {
> @@ -6938,6 +6955,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
>  	LSM_HOOK_INIT(kernel_act_as, selinux_kernel_act_as),
>  	LSM_HOOK_INIT(kernel_create_files_as, selinux_kernel_create_files_as),
>  	LSM_HOOK_INIT(kernel_module_request, selinux_kernel_module_request),
> +	LSM_HOOK_INIT(kernel_arg, selinux_kernel_arg),
>  	LSM_HOOK_INIT(kernel_read_file, selinux_kernel_read_file),
>  	LSM_HOOK_INIT(task_setpgid, selinux_task_setpgid),
>  	LSM_HOOK_INIT(task_getpgid, selinux_task_getpgid),
> 
> --
> 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
> 

--
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: [PATCH v3 1/7] security: rename security_kernel_read_file() hook

[Mimi Zohar]

On Thu, 2018-05-24 at 15:49 -0500, Eric W. Biederman wrote:

Thank you for the sample code below.  It needs to be broken up into
proper patches, with some changes, but it is a good start.

Mimi 

> diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
> index 358354148dec..04536ff81bd2 100644
> --- a/drivers/base/firmware_loader/fallback.c
> +++ b/drivers/base/firmware_loader/fallback.c
> @@ -294,9 +294,7 @@ static ssize_t firmware_loading_store(struct device *dev,
>  				dev_err(dev, "%s: map pages failed\n",
>  					__func__);
>  			else
> -				rc = security_kernel_post_read_file(NULL,
> -						fw_priv->data, fw_priv->size,
> -						READING_FIRMWARE);
> +				rc = security_kernel_arg(KARG_FIRMWARE);
> 
>  			/*
>  			 * Same logic as fw_load_abort, only the DONE bit
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 0e4647e0eb60..9fb42736ba29 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -11,6 +11,7 @@
>  #define _LINUX_IMA_H
> 
>  #include <linux/fs.h>
> +#include <linux/security.h>
>  #include <linux/kexec.h>
>  struct linux_binprm;
> 
> @@ -19,7 +20,7 @@ extern int ima_bprm_check(struct linux_binprm *bprm);
>  extern int ima_file_check(struct file *file, int mask, int opened);
>  extern void ima_file_free(struct file *file);
>  extern int ima_file_mmap(struct file *file, unsigned long prot);
> -extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
> +extern int ima_kernel_arg(enum kernel_arg_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);
> @@ -49,7 +50,7 @@ static inline int ima_file_mmap(struct file *file, unsigned long prot)
>  	return 0;
>  }
> 
> -static inline int ima_read_file(struct file *file, enum kernel_read_file_id id)
> +static inline int ima_kernel_arg(enum kernel_arg_id id)
>  {
>  	return 0;
>  }
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 9d0b286f3dba..7f8bc3030784 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -576,6 +576,10 @@
>   *	userspace to load a kernel module with the given name.
>   *	@kmod_name name of the module requested by the kernel
>   *	Return 0 if successful.
> + * @kernel_arg:
> + *	Use a syscall argument
> + *	@id kernel argument identifier
> + *	Return 0 if permission is granted.
>   * @kernel_read_file:
>   *	Read a file specified by userspace.
>   *	@file contains the file structure pointing to the file being read
> @@ -1577,6 +1581,7 @@ union security_list_options {
>  	int (*kernel_act_as)(struct cred *new, u32 secid);
>  	int (*kernel_create_files_as)(struct cred *new, struct inode *inode);
>  	int (*kernel_module_request)(char *kmod_name);
> +	int (*kernel_arg)(enum kernel_arg_id id);
>  	int (*kernel_read_file)(struct file *file, enum kernel_read_file_id id);
>  	int (*kernel_post_read_file)(struct file *file, char *buf, loff_t size,
>  				     enum kernel_read_file_id id);
> @@ -1866,6 +1871,7 @@ struct security_hook_heads {
>  	struct hlist_head cred_getsecid;
>  	struct hlist_head kernel_act_as;
>  	struct hlist_head kernel_create_files_as;
> +	struct hlist_head kernel_arg;
>  	struct hlist_head kernel_read_file;
>  	struct hlist_head kernel_post_read_file;
>  	struct hlist_head kernel_module_request;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 200920f521a1..6cf1bd87f041 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -159,6 +159,32 @@ extern int mmap_min_addr_handler(struct ctl_table *table, int write,
>  typedef int (*initxattrs) (struct inode *inode,
>  			   const struct xattr *xattr_array, void *fs_data);
> 
> +#define __kernel_arg_id(id) \
> +	id(UNKNOWN, unknown)		\
> +	id(FIRMWARE, firmware)		\
> +	id(MODULE, kernel-module)	\
> +	id(MAX_ID, )
> +
> +#define __karg_enumify(ENUM, dummy) KARG_ ## ENUM,
> +#define __karg_stringify(dummy, str) #str,
> +
> +enum kernel_arg_id {
> +	__kernel_arg_id(__karg_enumify)
> +};
> +
> +static const char * const kernel_arg_str[] = {
> +	__kernel_arg_id(__karg_stringify)
> +};
> +
> +static inline const char *kernel_arg_id_str(enum kernel_arg_id id)
> +{
> +	if ((unsigned)id >= KARG_MAX_ID)
> +		return kernel_arg_str[KARG_UNKNOWN];
> +
> +	return kernel_arg_str[id];
> +}
> +
> +
>  #ifdef CONFIG_SECURITY
> 
>  struct security_mnt_opts {
> @@ -326,6 +352,7 @@ void security_cred_getsecid(const struct cred *c, u32 *secid);
>  int security_kernel_act_as(struct cred *new, u32 secid);
>  int security_kernel_create_files_as(struct cred *new, struct inode *inode);
>  int security_kernel_module_request(char *kmod_name);
> +int security_kernel_arg(enum kernel_arg_id id);
>  int security_kernel_read_file(struct file *file, enum kernel_read_file_id id);
>  int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
>  				   enum kernel_read_file_id id);
> @@ -923,6 +950,11 @@ static inline int security_kernel_module_request(char *kmod_name)
>  	return 0;
>  }
> 
> +static inline int security_kernel_arg(enum kernel_arg_id id)
> +{
> +	return 0;
> +}
> +
>  static inline int security_kernel_read_file(struct file *file,
>  					    enum kernel_read_file_id id)
>  {
> diff --git a/kernel/module.c b/kernel/module.c
> index ce8066b88178..03a1dd21ad4a 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2879,7 +2879,7 @@ static int copy_module_from_user(const void __user *umod, unsigned long len,
>  	if (info->len < sizeof(*(info->hdr)))
>  		return -ENOEXEC;
> 
> -	err = security_kernel_read_file(NULL, READING_MODULE);
> +	err = security_kernel_arg(KARG_MODULE);
>  	if (err)
>  		return err;
> 
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 74d0bd7e76d7..d51a8ca97238 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -421,32 +421,6 @@ void ima_post_path_mknod(struct dentry *dentry)
>  		iint->flags |= IMA_NEW_FILE;
>  }
> 
> -/**
> - * 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
> - *
> - * Permit reading a file based on policy. The policy rules are written
> - * in terms of the policy identifier.  Appraising the integrity of
> - * a file requires a file descriptor.
> - *
> - * For permission return 0, otherwise return -EACCES.
> - */
> -int ima_read_file(struct file *file, enum kernel_read_file_id read_id)
> -{
> -	bool sig_enforce = is_module_sig_enforced();
> -
> -	if (!file && read_id == READING_MODULE) {
> -		if (!sig_enforce && (ima_appraise & IMA_APPRAISE_MODULES) &&
> -		    (ima_appraise & IMA_APPRAISE_ENFORCE)) {
> -			pr_err("impossible to appraise a module without a file descriptor. sig_enforce kernel parameter might help\n");
> -			return -EACCES;	/* INTEGRITY_UNKNOWN */
> -		}
> -		return 0;	/* We rely on module signature checking */
> -	}
> -	return 0;
> -}
> -
>  static int read_idmap[READING_MAX_ID] = {
>  	[READING_FIRMWARE] = FIRMWARE_CHECK,
>  	[READING_MODULE] = MODULE_CHECK,
> @@ -474,21 +448,7 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
>  	enum ima_hooks func;
>  	u32 secid;
> 
> -	if (!file && read_id == READING_FIRMWARE) {
> -		if ((ima_appraise & IMA_APPRAISE_FIRMWARE) &&
> -		    (ima_appraise & IMA_APPRAISE_ENFORCE))
> -			return -EACCES;	/* INTEGRITY_UNKNOWN */
> -		return 0;
> -	}
> -
> -	if (!file && read_id == READING_MODULE) /* MODULE_SIG_FORCE enabled */
> -		return 0;
> -
> -	/* permit signed certs */
> -	if (!file && read_id == READING_X509_CERTIFICATE)
> -		return 0;
> -
> -	if (!file || !buf || size == 0) { /* should never happen */
> +	if (!buf || size == 0) { /* should never happen */
>  		if (ima_appraise & IMA_APPRAISE_ENFORCE)
>  			return -EACCES;
>  		return 0;
> @@ -500,6 +460,40 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
>  				   MAY_READ, func, 0);
>  }
> 
> +/**
> + * ima_kernel_arg - pre-measure/appraise hook decision based on policy
> + * @id: caller identifier
> + *
> + * Permit using an argument to a syscall based on policy. The policy
> + * rules are written in terms of the policy identifier.
> + *
> + * For permission return 0, otherwise return -EACCES.
> + */
> +int ima_kernel_arg(enum kernel_arg_id id)
> +{
> +	if (id == KARG_MODULE) {
> +		bool sig_enforce = is_module_sig_enforced();
> +		if (!sig_enforce && (ima_appraise & IMA_APPRAISE_MODULES) &&
> +		    (ima_appraise & IMA_APPRAISE_ENFORCE)) {
> +			pr_err("impossible to appraise a module without a file descriptor. sig_enforce kernel parameter might help\n");
> +			return -EACCES;	/* INTEGRITY_UNKNOWN */
> +		}
> +		return 0;	/* We rely on module signature checking */
> +	}
> +	else if (id == KARG_FIRMWARE) {
> +		if ((ima_appraise & IMA_APPRAISE_FIRMWARE) &&
> +		    (ima_appraise & IMA_APPRAISE_ENFORCE))
> +			return -EACCES;	/* INTEGRITY_UNKNOWN */
> +		return 0;
> +	}
> +	else {
> +		if (ima_appraise & IMA_APPRAISE_ENFORCE)
> +			return -EACCES;
> +		return 0;
> +	}
> +	return 0;
> +}
> +
>  static int __init init_ima(void)
>  {
>  	int error;
> diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
> index 5fa191252c8f..f5333e5abac9 100644
> --- a/security/loadpin/loadpin.c
> +++ b/security/loadpin/loadpin.c
> @@ -121,23 +121,24 @@ static void loadpin_sb_free_security(struct super_block *mnt_sb)
>  	}
>  }
> 
> -static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
> +static int loadpin_read_data(enum kernel_read_data_id id)
>  {
> -	struct super_block *load_root;
> -	const char *origin = kernel_read_file_id_str(id);
> +	const char *origin = kernel_arg_id_str(id);
> 
>  	/* This handles the older init_module API that has a NULL file. */
> -	if (!file) {
> -		if (!enabled) {
> -			report_load(origin, NULL, "old-api-pinning-ignored");
> -			return 0;
> -		}
> -
> -		report_load(origin, NULL, "old-api-denied");
> -		return -EPERM;
> +	if (!enabled) {
> +		report_load(origin, NULL, "old-api-pinning-ignored");
> +		return 0;
>  	}
> 
> -	load_root = file->f_path.mnt->mnt_sb;
> +	report_load(origin, NULL, "old-api-denied");
> +	return -EPERM;
> +}
> +
> +static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
> +{
> +	struct super_block *load_root;
> +	const char *origin = kernel_read_file_id_str(id);
> 
>  	/* First loaded module/firmware defines the root for all others. */
>  	spin_lock(&pinned_root_spinlock);
> @@ -175,6 +176,7 @@ static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
> 
>  static struct security_hook_list loadpin_hooks[] __lsm_ro_after_init = {
>  	LSM_HOOK_INIT(sb_free_security, loadpin_sb_free_security),
> +	LSM_HOOK_INIT(kernel_read_data, loadpin_read_data),
>  	LSM_HOOK_INIT(kernel_read_file, loadpin_read_file),
>  };
> 
> diff --git a/security/security.c b/security/security.c
> index 7bc2fde023a7..9b5f43c24ee2 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1033,14 +1033,19 @@ int security_kernel_module_request(char *kmod_name)
>  	return call_int_hook(kernel_module_request, 0, kmod_name);
>  }
> 
> -int security_kernel_read_file(struct file *file, enum kernel_read_file_id id)
> +int security_kernel_arg(enum kernel_arg_id id)
>  {
>  	int ret;
> 
> -	ret = call_int_hook(kernel_read_file, 0, file, id);
> -	if (ret)
> -		return ret;
> -	return ima_read_file(file, id);
> +	ret = call_int_hook(kernel_arg, 0, id);
> +	if (!ret)
> +		ret = ima_kernel_arg(id);
> +	return ret;
> +}
> +
> +int security_kernel_read_file(struct file *file, enum kernel_read_file_id id)
> +{
> +	return call_int_hook(kernel_read_file, 0, file, id);
>  }
>  EXPORT_SYMBOL_GPL(security_kernel_read_file);
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 4cafe6a19167..76843099fed6 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4010,6 +4010,15 @@ static int selinux_kernel_module_request(char *kmod_name)
>  			    SYSTEM__MODULE_REQUEST, &ad);
>  }
> 
> +static int selinux_kernel_module_arg(void)
> +{
> +	/* init_module */
> +	u32 sid = current_sid();
> +	return avc_has_perm(&selinux_state,
> +			    sid, sid, SECCLASS_SYSTEM,
> +			    SYSTEM__MODULE_LOAD, NULL);
> +}
> +
>  static int selinux_kernel_module_from_file(struct file *file)
>  {
>  	struct common_audit_data ad;
> @@ -4018,12 +4027,6 @@ static int selinux_kernel_module_from_file(struct file *file)
>  	u32 sid = current_sid();
>  	int rc;
> 
> -	/* init_module */
> -	if (file == NULL)
> -		return avc_has_perm(&selinux_state,
> -				    sid, sid, SECCLASS_SYSTEM,
> -					SYSTEM__MODULE_LOAD, NULL);
> -
>  	/* finit_module */
> 
>  	ad.type = LSM_AUDIT_DATA_FILE;
> @@ -4043,6 +4046,20 @@ static int selinux_kernel_module_from_file(struct file *file)
>  				SYSTEM__MODULE_LOAD, &ad);
>  }
> 
> +static int selinux_kernel_arg(enum kernel_arg_id id)
> +{
> +	int rc = 0;
> +
> +	switch (id) {
> +	case KARG_MODULE:
> +		rc = selinux_kernel_module_arg();
> +		break;
> +	default:
> +		break;
> +	}
> +	return rc;
> +}
> +
>  static int selinux_kernel_read_file(struct file *file,
>  				    enum kernel_read_file_id id)
>  {
> @@ -6938,6 +6955,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
>  	LSM_HOOK_INIT(kernel_act_as, selinux_kernel_act_as),
>  	LSM_HOOK_INIT(kernel_create_files_as, selinux_kernel_create_files_as),
>  	LSM_HOOK_INIT(kernel_module_request, selinux_kernel_module_request),
> +	LSM_HOOK_INIT(kernel_arg, selinux_kernel_arg),
>  	LSM_HOOK_INIT(kernel_read_file, selinux_kernel_read_file),
>  	LSM_HOOK_INIT(task_setpgid, selinux_task_setpgid),
>  	LSM_HOOK_INIT(task_getpgid, selinux_task_getpgid),
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH v3 1/7] security: rename security_kernel_read_file() hook

[Mimi Zohar]

On Thu, 2018-05-24 at 15:49 -0500, Eric W. Biederman wrote:

Thank you for the sample code below.  It needs to be broken up into
proper patches, with some changes, but it is a good start.

Mimi 

> diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
> index 358354148dec..04536ff81bd2 100644
> --- a/drivers/base/firmware_loader/fallback.c
> +++ b/drivers/base/firmware_loader/fallback.c
> @@ -294,9 +294,7 @@ static ssize_t firmware_loading_store(struct device *dev,
>  				dev_err(dev, "%s: map pages failed\n",
>  					__func__);
>  			else
> -				rc = security_kernel_post_read_file(NULL,
> -						fw_priv->data, fw_priv->size,
> -						READING_FIRMWARE);
> +				rc = security_kernel_arg(KARG_FIRMWARE);
> 
>  			/*
>  			 * Same logic as fw_load_abort, only the DONE bit
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 0e4647e0eb60..9fb42736ba29 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -11,6 +11,7 @@
>  #define _LINUX_IMA_H
> 
>  #include <linux/fs.h>
> +#include <linux/security.h>
>  #include <linux/kexec.h>
>  struct linux_binprm;
> 
> @@ -19,7 +20,7 @@ extern int ima_bprm_check(struct linux_binprm *bprm);
>  extern int ima_file_check(struct file *file, int mask, int opened);
>  extern void ima_file_free(struct file *file);
>  extern int ima_file_mmap(struct file *file, unsigned long prot);
> -extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
> +extern int ima_kernel_arg(enum kernel_arg_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);
> @@ -49,7 +50,7 @@ static inline int ima_file_mmap(struct file *file, unsigned long prot)
>  	return 0;
>  }
> 
> -static inline int ima_read_file(struct file *file, enum kernel_read_file_id id)
> +static inline int ima_kernel_arg(enum kernel_arg_id id)
>  {
>  	return 0;
>  }
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 9d0b286f3dba..7f8bc3030784 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -576,6 +576,10 @@
>   *	userspace to load a kernel module with the given name.
>   *	@kmod_name name of the module requested by the kernel
>   *	Return 0 if successful.
> + * @kernel_arg:
> + *	Use a syscall argument
> + *	@id kernel argument identifier
> + *	Return 0 if permission is granted.
>   * @kernel_read_file:
>   *	Read a file specified by userspace.
>   *	@file contains the file structure pointing to the file being read
> @@ -1577,6 +1581,7 @@ union security_list_options {
>  	int (*kernel_act_as)(struct cred *new, u32 secid);
>  	int (*kernel_create_files_as)(struct cred *new, struct inode *inode);
>  	int (*kernel_module_request)(char *kmod_name);
> +	int (*kernel_arg)(enum kernel_arg_id id);
>  	int (*kernel_read_file)(struct file *file, enum kernel_read_file_id id);
>  	int (*kernel_post_read_file)(struct file *file, char *buf, loff_t size,
>  				     enum kernel_read_file_id id);
> @@ -1866,6 +1871,7 @@ struct security_hook_heads {
>  	struct hlist_head cred_getsecid;
>  	struct hlist_head kernel_act_as;
>  	struct hlist_head kernel_create_files_as;
> +	struct hlist_head kernel_arg;
>  	struct hlist_head kernel_read_file;
>  	struct hlist_head kernel_post_read_file;
>  	struct hlist_head kernel_module_request;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 200920f521a1..6cf1bd87f041 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -159,6 +159,32 @@ extern int mmap_min_addr_handler(struct ctl_table *table, int write,
>  typedef int (*initxattrs) (struct inode *inode,
>  			   const struct xattr *xattr_array, void *fs_data);
> 
> +#define __kernel_arg_id(id) \
> +	id(UNKNOWN, unknown)		\
> +	id(FIRMWARE, firmware)		\
> +	id(MODULE, kernel-module)	\
> +	id(MAX_ID, )
> +
> +#define __karg_enumify(ENUM, dummy) KARG_ ## ENUM,
> +#define __karg_stringify(dummy, str) #str,
> +
> +enum kernel_arg_id {
> +	__kernel_arg_id(__karg_enumify)
> +};
> +
> +static const char * const kernel_arg_str[] = {
> +	__kernel_arg_id(__karg_stringify)
> +};
> +
> +static inline const char *kernel_arg_id_str(enum kernel_arg_id id)
> +{
> +	if ((unsigned)id >= KARG_MAX_ID)
> +		return kernel_arg_str[KARG_UNKNOWN];
> +
> +	return kernel_arg_str[id];
> +}
> +
> +
>  #ifdef CONFIG_SECURITY
> 
>  struct security_mnt_opts {
> @@ -326,6 +352,7 @@ void security_cred_getsecid(const struct cred *c, u32 *secid);
>  int security_kernel_act_as(struct cred *new, u32 secid);
>  int security_kernel_create_files_as(struct cred *new, struct inode *inode);
>  int security_kernel_module_request(char *kmod_name);
> +int security_kernel_arg(enum kernel_arg_id id);
>  int security_kernel_read_file(struct file *file, enum kernel_read_file_id id);
>  int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
>  				   enum kernel_read_file_id id);
> @@ -923,6 +950,11 @@ static inline int security_kernel_module_request(char *kmod_name)
>  	return 0;
>  }
> 
> +static inline int security_kernel_arg(enum kernel_arg_id id)
> +{
> +	return 0;
> +}
> +
>  static inline int security_kernel_read_file(struct file *file,
>  					    enum kernel_read_file_id id)
>  {
> diff --git a/kernel/module.c b/kernel/module.c
> index ce8066b88178..03a1dd21ad4a 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2879,7 +2879,7 @@ static int copy_module_from_user(const void __user *umod, unsigned long len,
>  	if (info->len < sizeof(*(info->hdr)))
>  		return -ENOEXEC;
> 
> -	err = security_kernel_read_file(NULL, READING_MODULE);
> +	err = security_kernel_arg(KARG_MODULE);
>  	if (err)
>  		return err;
> 
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 74d0bd7e76d7..d51a8ca97238 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -421,32 +421,6 @@ void ima_post_path_mknod(struct dentry *dentry)
>  		iint->flags |= IMA_NEW_FILE;
>  }
> 
> -/**
> - * 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
> - *
> - * Permit reading a file based on policy. The policy rules are written
> - * in terms of the policy identifier.  Appraising the integrity of
> - * a file requires a file descriptor.
> - *
> - * For permission return 0, otherwise return -EACCES.
> - */
> -int ima_read_file(struct file *file, enum kernel_read_file_id read_id)
> -{
> -	bool sig_enforce = is_module_sig_enforced();
> -
> -	if (!file && read_id == READING_MODULE) {
> -		if (!sig_enforce && (ima_appraise & IMA_APPRAISE_MODULES) &&
> -		    (ima_appraise & IMA_APPRAISE_ENFORCE)) {
> -			pr_err("impossible to appraise a module without a file descriptor. sig_enforce kernel parameter might help\n");
> -			return -EACCES;	/* INTEGRITY_UNKNOWN */
> -		}
> -		return 0;	/* We rely on module signature checking */
> -	}
> -	return 0;
> -}
> -
>  static int read_idmap[READING_MAX_ID] = {
>  	[READING_FIRMWARE] = FIRMWARE_CHECK,
>  	[READING_MODULE] = MODULE_CHECK,
> @@ -474,21 +448,7 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
>  	enum ima_hooks func;
>  	u32 secid;
> 
> -	if (!file && read_id == READING_FIRMWARE) {
> -		if ((ima_appraise & IMA_APPRAISE_FIRMWARE) &&
> -		    (ima_appraise & IMA_APPRAISE_ENFORCE))
> -			return -EACCES;	/* INTEGRITY_UNKNOWN */
> -		return 0;
> -	}
> -
> -	if (!file && read_id == READING_MODULE) /* MODULE_SIG_FORCE enabled */
> -		return 0;
> -
> -	/* permit signed certs */
> -	if (!file && read_id == READING_X509_CERTIFICATE)
> -		return 0;
> -
> -	if (!file || !buf || size == 0) { /* should never happen */
> +	if (!buf || size == 0) { /* should never happen */
>  		if (ima_appraise & IMA_APPRAISE_ENFORCE)
>  			return -EACCES;
>  		return 0;
> @@ -500,6 +460,40 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
>  				   MAY_READ, func, 0);
>  }
> 
> +/**
> + * ima_kernel_arg - pre-measure/appraise hook decision based on policy
> + * @id: caller identifier
> + *
> + * Permit using an argument to a syscall based on policy. The policy
> + * rules are written in terms of the policy identifier.
> + *
> + * For permission return 0, otherwise return -EACCES.
> + */
> +int ima_kernel_arg(enum kernel_arg_id id)
> +{
> +	if (id == KARG_MODULE) {
> +		bool sig_enforce = is_module_sig_enforced();
> +		if (!sig_enforce && (ima_appraise & IMA_APPRAISE_MODULES) &&
> +		    (ima_appraise & IMA_APPRAISE_ENFORCE)) {
> +			pr_err("impossible to appraise a module without a file descriptor. sig_enforce kernel parameter might help\n");
> +			return -EACCES;	/* INTEGRITY_UNKNOWN */
> +		}
> +		return 0;	/* We rely on module signature checking */
> +	}
> +	else if (id == KARG_FIRMWARE) {
> +		if ((ima_appraise & IMA_APPRAISE_FIRMWARE) &&
> +		    (ima_appraise & IMA_APPRAISE_ENFORCE))
> +			return -EACCES;	/* INTEGRITY_UNKNOWN */
> +		return 0;
> +	}
> +	else {
> +		if (ima_appraise & IMA_APPRAISE_ENFORCE)
> +			return -EACCES;
> +		return 0;
> +	}
> +	return 0;
> +}
> +
>  static int __init init_ima(void)
>  {
>  	int error;
> diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
> index 5fa191252c8f..f5333e5abac9 100644
> --- a/security/loadpin/loadpin.c
> +++ b/security/loadpin/loadpin.c
> @@ -121,23 +121,24 @@ static void loadpin_sb_free_security(struct super_block *mnt_sb)
>  	}
>  }
> 
> -static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
> +static int loadpin_read_data(enum kernel_read_data_id id)
>  {
> -	struct super_block *load_root;
> -	const char *origin = kernel_read_file_id_str(id);
> +	const char *origin = kernel_arg_id_str(id);
> 
>  	/* This handles the older init_module API that has a NULL file. */
> -	if (!file) {
> -		if (!enabled) {
> -			report_load(origin, NULL, "old-api-pinning-ignored");
> -			return 0;
> -		}
> -
> -		report_load(origin, NULL, "old-api-denied");
> -		return -EPERM;
> +	if (!enabled) {
> +		report_load(origin, NULL, "old-api-pinning-ignored");
> +		return 0;
>  	}
> 
> -	load_root = file->f_path.mnt->mnt_sb;
> +	report_load(origin, NULL, "old-api-denied");
> +	return -EPERM;
> +}
> +
> +static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
> +{
> +	struct super_block *load_root;
> +	const char *origin = kernel_read_file_id_str(id);
> 
>  	/* First loaded module/firmware defines the root for all others. */
>  	spin_lock(&pinned_root_spinlock);
> @@ -175,6 +176,7 @@ static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
> 
>  static struct security_hook_list loadpin_hooks[] __lsm_ro_after_init = {
>  	LSM_HOOK_INIT(sb_free_security, loadpin_sb_free_security),
> +	LSM_HOOK_INIT(kernel_read_data, loadpin_read_data),
>  	LSM_HOOK_INIT(kernel_read_file, loadpin_read_file),
>  };
> 
> diff --git a/security/security.c b/security/security.c
> index 7bc2fde023a7..9b5f43c24ee2 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1033,14 +1033,19 @@ int security_kernel_module_request(char *kmod_name)
>  	return call_int_hook(kernel_module_request, 0, kmod_name);
>  }
> 
> -int security_kernel_read_file(struct file *file, enum kernel_read_file_id id)
> +int security_kernel_arg(enum kernel_arg_id id)
>  {
>  	int ret;
> 
> -	ret = call_int_hook(kernel_read_file, 0, file, id);
> -	if (ret)
> -		return ret;
> -	return ima_read_file(file, id);
> +	ret = call_int_hook(kernel_arg, 0, id);
> +	if (!ret)
> +		ret = ima_kernel_arg(id);
> +	return ret;
> +}
> +
> +int security_kernel_read_file(struct file *file, enum kernel_read_file_id id)
> +{
> +	return call_int_hook(kernel_read_file, 0, file, id);
>  }
>  EXPORT_SYMBOL_GPL(security_kernel_read_file);
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 4cafe6a19167..76843099fed6 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4010,6 +4010,15 @@ static int selinux_kernel_module_request(char *kmod_name)
>  			    SYSTEM__MODULE_REQUEST, &ad);
>  }
> 
> +static int selinux_kernel_module_arg(void)
> +{
> +	/* init_module */
> +	u32 sid = current_sid();
> +	return avc_has_perm(&selinux_state,
> +			    sid, sid, SECCLASS_SYSTEM,
> +			    SYSTEM__MODULE_LOAD, NULL);
> +}
> +
>  static int selinux_kernel_module_from_file(struct file *file)
>  {
>  	struct common_audit_data ad;
> @@ -4018,12 +4027,6 @@ static int selinux_kernel_module_from_file(struct file *file)
>  	u32 sid = current_sid();
>  	int rc;
> 
> -	/* init_module */
> -	if (file == NULL)
> -		return avc_has_perm(&selinux_state,
> -				    sid, sid, SECCLASS_SYSTEM,
> -					SYSTEM__MODULE_LOAD, NULL);
> -
>  	/* finit_module */
> 
>  	ad.type = LSM_AUDIT_DATA_FILE;
> @@ -4043,6 +4046,20 @@ static int selinux_kernel_module_from_file(struct file *file)
>  				SYSTEM__MODULE_LOAD, &ad);
>  }
> 
> +static int selinux_kernel_arg(enum kernel_arg_id id)
> +{
> +	int rc = 0;
> +
> +	switch (id) {
> +	case KARG_MODULE:
> +		rc = selinux_kernel_module_arg();
> +		break;
> +	default:
> +		break;
> +	}
> +	return rc;
> +}
> +
>  static int selinux_kernel_read_file(struct file *file,
>  				    enum kernel_read_file_id id)
>  {
> @@ -6938,6 +6955,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
>  	LSM_HOOK_INIT(kernel_act_as, selinux_kernel_act_as),
>  	LSM_HOOK_INIT(kernel_create_files_as, selinux_kernel_create_files_as),
>  	LSM_HOOK_INIT(kernel_module_request, selinux_kernel_module_request),
> +	LSM_HOOK_INIT(kernel_arg, selinux_kernel_arg),
>  	LSM_HOOK_INIT(kernel_read_file, selinux_kernel_read_file),
>  	LSM_HOOK_INIT(task_setpgid, selinux_task_setpgid),
>  	LSM_HOOK_INIT(task_getpgid, selinux_task_getpgid),
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v3 1/7] security: rename security_kernel_read_file() hook

[James Morris]

On Thu, 24 May 2018, Eric W. Biederman wrote:

> Below is where I suggest you start on sorting out these security hooks.
> - Adding a security_kernel_arg to catch when you want to allow/deny the
>   use of an argument to a syscall.  What security_kernel_file_read and
>   security_kernel_file_post_read have been abused for.

NAK. This abstraction is too semantically weak.

LSM hooks need to map to stronger semantics so we can reason about what 
the hook and the policy is supposed to be mediating.

-- 
James Morris
<jmorris@namei.org>

[PATCH v3 1/7] security: rename security_kernel_read_file() hook

[James Morris]

On Thu, 24 May 2018, Eric W. Biederman wrote:

> Below is where I suggest you start on sorting out these security hooks.
> - Adding a security_kernel_arg to catch when you want to allow/deny the
>   use of an argument to a syscall.  What security_kernel_file_read and
>   security_kernel_file_post_read have been abused for.

NAK. This abstraction is too semantically weak.

LSM hooks need to map to stronger semantics so we can reason about what 
the hook and the policy is supposed to be mediating.

-- 
James Morris
<jmorris@namei.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

Re: [PATCH v3 1/7] security: rename security_kernel_read_file() hook

[James Morris]

On Thu, 24 May 2018, Eric W. Biederman wrote:

> Below is where I suggest you start on sorting out these security hooks.
> - Adding a security_kernel_arg to catch when you want to allow/deny the
>   use of an argument to a syscall.  What security_kernel_file_read and
>   security_kernel_file_post_read have been abused for.

NAK. This abstraction is too semantically weak.

LSM hooks need to map to stronger semantics so we can reason about what 
the hook and the policy is supposed to be mediating.

-- 
James Morris
<jmorris@namei.org>


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v3 1/7] security: rename security_kernel_read_file() hook

[Eric W. Biederman]

James Morris <jmorris@namei.org> writes:

> On Thu, 24 May 2018, Eric W. Biederman wrote:
>
>> Below is where I suggest you start on sorting out these security hooks.
>> - Adding a security_kernel_arg to catch when you want to allow/deny the
>>   use of an argument to a syscall.  What security_kernel_file_read and
>>   security_kernel_file_post_read have been abused for.
>
> NAK. This abstraction is too semantically weak.
>
> LSM hooks need to map to stronger semantics so we can reason about what 
> the hook and the policy is supposed to be mediating.

I will take that as an extremely weak nack as all I did was expose the
existing code and what the code is currently doing.  I don't see how you
can NAK what is already being merged and used.

I will be happy to see a better proposal.

The best I can see is to take each and every syscall that my patch
is calling syscall_kernel_arg and make it it's own hook without an
enumeration.  I did not see any real duplication between the cases in my
enumeration so I don't think that will be a problem.  Maybe a bit of a
challenge for loadpin but otherwise not.

Thank you in this for understanding why I am having problems with the
current hook.

Eric

[PATCH v3 1/7] security: rename security_kernel_read_file() hook

[Eric W. Biederman]

James Morris <jmorris@namei.org> writes:

> On Thu, 24 May 2018, Eric W. Biederman wrote:
>
>> Below is where I suggest you start on sorting out these security hooks.
>> - Adding a security_kernel_arg to catch when you want to allow/deny the
>>   use of an argument to a syscall.  What security_kernel_file_read and
>>   security_kernel_file_post_read have been abused for.
>
> NAK. This abstraction is too semantically weak.
>
> LSM hooks need to map to stronger semantics so we can reason about what 
> the hook and the policy is supposed to be mediating.

I will take that as an extremely weak nack as all I did was expose the
existing code and what the code is currently doing.  I don't see how you
can NAK what is already being merged and used.

I will be happy to see a better proposal.

The best I can see is to take each and every syscall that my patch
is calling syscall_kernel_arg and make it it's own hook without an
enumeration.  I did not see any real duplication between the cases in my
enumeration so I don't think that will be a problem.  Maybe a bit of a
challenge for loadpin but otherwise not.

Thank you in this for understanding why I am having problems with the
current hook.

Eric

--
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: [PATCH v3 1/7] security: rename security_kernel_read_file() hook

[Eric W. Biederman]

James Morris <jmorris@namei.org> writes:

> On Thu, 24 May 2018, Eric W. Biederman wrote:
>
>> Below is where I suggest you start on sorting out these security hooks.
>> - Adding a security_kernel_arg to catch when you want to allow/deny the
>>   use of an argument to a syscall.  What security_kernel_file_read and
>>   security_kernel_file_post_read have been abused for.
>
> NAK. This abstraction is too semantically weak.
>
> LSM hooks need to map to stronger semantics so we can reason about what 
> the hook and the policy is supposed to be mediating.

I will take that as an extremely weak nack as all I did was expose the
existing code and what the code is currently doing.  I don't see how you
can NAK what is already being merged and used.

I will be happy to see a better proposal.

The best I can see is to take each and every syscall that my patch
is calling syscall_kernel_arg and make it it's own hook without an
enumeration.  I did not see any real duplication between the cases in my
enumeration so I don't think that will be a problem.  Maybe a bit of a
challenge for loadpin but otherwise not.

Thank you in this for understanding why I am having problems with the
current hook.

Eric


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v3 1/7] security: rename security_kernel_read_file() hook

[James Morris]

On Fri, 25 May 2018, Eric W. Biederman wrote:

> James Morris <jmorris@namei.org> writes:
> 
> > On Thu, 24 May 2018, Eric W. Biederman wrote:
> >
> >> Below is where I suggest you start on sorting out these security hooks.
> >> - Adding a security_kernel_arg to catch when you want to allow/deny the
> >>   use of an argument to a syscall.  What security_kernel_file_read and
> >>   security_kernel_file_post_read have been abused for.
> >
> > NAK. This abstraction is too semantically weak.
> >
> > LSM hooks need to map to stronger semantics so we can reason about what 
> > the hook and the policy is supposed to be mediating.
> 
> I will take that as an extremely weak nack as all I did was expose the
> existing code and what the code is currently doing.  I don't see how you
> can NAK what is already being merged and used.

It's a strong NAK.

LSM is a logical API, it provides an abstraction layer for security 
policies to mediate kernel security behaviors.

Adding an argument to a syscall is not a security behavior.

Loading a firmware file is.

=
-- 
James Morris
<jmorris@namei.org>

[PATCH v3 1/7] security: rename security_kernel_read_file() hook

[James Morris]

On Fri, 25 May 2018, Eric W. Biederman wrote:

> James Morris <jmorris@namei.org> writes:
> 
> > On Thu, 24 May 2018, Eric W. Biederman wrote:
> >
> >> Below is where I suggest you start on sorting out these security hooks.
> >> - Adding a security_kernel_arg to catch when you want to allow/deny the
> >>   use of an argument to a syscall.  What security_kernel_file_read and
> >>   security_kernel_file_post_read have been abused for.
> >
> > NAK. This abstraction is too semantically weak.
> >
> > LSM hooks need to map to stronger semantics so we can reason about what 
> > the hook and the policy is supposed to be mediating.
> 
> I will take that as an extremely weak nack as all I did was expose the
> existing code and what the code is currently doing.  I don't see how you
> can NAK what is already being merged and used.

It's a strong NAK.

LSM is a logical API, it provides an abstraction layer for security 
policies to mediate kernel security behaviors.

Adding an argument to a syscall is not a security behavior.

Loading a firmware file is.

=
-- 
James Morris
<jmorris@namei.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

Re: [PATCH v3 1/7] security: rename security_kernel_read_file() hook

[James Morris]

On Fri, 25 May 2018, Eric W. Biederman wrote:

> James Morris <jmorris@namei.org> writes:
> 
> > On Thu, 24 May 2018, Eric W. Biederman wrote:
> >
> >> Below is where I suggest you start on sorting out these security hooks.
> >> - Adding a security_kernel_arg to catch when you want to allow/deny the
> >>   use of an argument to a syscall.  What security_kernel_file_read and
> >>   security_kernel_file_post_read have been abused for.
> >
> > NAK. This abstraction is too semantically weak.
> >
> > LSM hooks need to map to stronger semantics so we can reason about what 
> > the hook and the policy is supposed to be mediating.
> 
> I will take that as an extremely weak nack as all I did was expose the
> existing code and what the code is currently doing.  I don't see how you
> can NAK what is already being merged and used.

It's a strong NAK.

LSM is a logical API, it provides an abstraction layer for security 
policies to mediate kernel security behaviors.

Adding an argument to a syscall is not a security behavior.

Loading a firmware file is.

=
-- 
James Morris
<jmorris@namei.org>


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v3 1/7] security: rename security_kernel_read_file() hook

[Eric W. Biederman]

James Morris <jmorris@namei.org> writes:

> On Fri, 25 May 2018, Eric W. Biederman wrote:
>
>> James Morris <jmorris@namei.org> writes:
>> 
>> > On Thu, 24 May 2018, Eric W. Biederman wrote:
>> >
>> >> Below is where I suggest you start on sorting out these security hooks.
>> >> - Adding a security_kernel_arg to catch when you want to allow/deny the
>> >>   use of an argument to a syscall.  What security_kernel_file_read and
>> >>   security_kernel_file_post_read have been abused for.
>> >
>> > NAK. This abstraction is too semantically weak.
>> >
>> > LSM hooks need to map to stronger semantics so we can reason about what 
>> > the hook and the policy is supposed to be mediating.
>> 
>> I will take that as an extremely weak nack as all I did was expose the
>> existing code and what the code is currently doing.  I don't see how you
>> can NAK what is already being merged and used.
>
> It's a strong NAK.

We are either not understading each other or you have just strong NAK'd
part of the existing LSM api.  Not my proposal.

> LSM is a logical API, it provides an abstraction layer for security 
> policies to mediate kernel security behaviors.

The way it deals with firmware blobs and module loading is not logical.
It is some random pass a NULL pointer into some other security hook.

> Adding an argument to a syscall is not a security behavior.
>
> Loading a firmware file is.

It is a firmware blob not a file.  Perhaps the blob is stored as a file
on-disk, perhaps it is not.

The similar case with kexec never stores all of the data in a file.

Why module_init (which does not take a file) is calling a file based lsm
hook is also bizarre.


Perhaps that means all 3 of these cases should have their own void
security hooks.  Perhaps it means something else.  I just know the name
on the security hook, how it is getting called, and how it is getting
used simply do not agree.

Eric

[PATCH v3 1/7] security: rename security_kernel_read_file() hook

[Eric W. Biederman]

James Morris <jmorris@namei.org> writes:

> On Fri, 25 May 2018, Eric W. Biederman wrote:
>
>> James Morris <jmorris@namei.org> writes:
>> 
>> > On Thu, 24 May 2018, Eric W. Biederman wrote:
>> >
>> >> Below is where I suggest you start on sorting out these security hooks.
>> >> - Adding a security_kernel_arg to catch when you want to allow/deny the
>> >>   use of an argument to a syscall.  What security_kernel_file_read and
>> >>   security_kernel_file_post_read have been abused for.
>> >
>> > NAK. This abstraction is too semantically weak.
>> >
>> > LSM hooks need to map to stronger semantics so we can reason about what 
>> > the hook and the policy is supposed to be mediating.
>> 
>> I will take that as an extremely weak nack as all I did was expose the
>> existing code and what the code is currently doing.  I don't see how you
>> can NAK what is already being merged and used.
>
> It's a strong NAK.

We are either not understading each other or you have just strong NAK'd
part of the existing LSM api.  Not my proposal.

> LSM is a logical API, it provides an abstraction layer for security 
> policies to mediate kernel security behaviors.

The way it deals with firmware blobs and module loading is not logical.
It is some random pass a NULL pointer into some other security hook.

> Adding an argument to a syscall is not a security behavior.
>
> Loading a firmware file is.

It is a firmware blob not a file.  Perhaps the blob is stored as a file
on-disk, perhaps it is not.

The similar case with kexec never stores all of the data in a file.

Why module_init (which does not take a file) is calling a file based lsm
hook is also bizarre.


Perhaps that means all 3 of these cases should have their own void
security hooks.  Perhaps it means something else.  I just know the name
on the security hook, how it is getting called, and how it is getting
used simply do not agree.

Eric
--
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: [PATCH v3 1/7] security: rename security_kernel_read_file() hook

[Eric W. Biederman]

James Morris <jmorris@namei.org> writes:

> On Fri, 25 May 2018, Eric W. Biederman wrote:
>
>> James Morris <jmorris@namei.org> writes:
>> 
>> > On Thu, 24 May 2018, Eric W. Biederman wrote:
>> >
>> >> Below is where I suggest you start on sorting out these security hooks.
>> >> - Adding a security_kernel_arg to catch when you want to allow/deny the
>> >>   use of an argument to a syscall.  What security_kernel_file_read and
>> >>   security_kernel_file_post_read have been abused for.
>> >
>> > NAK. This abstraction is too semantically weak.
>> >
>> > LSM hooks need to map to stronger semantics so we can reason about what 
>> > the hook and the policy is supposed to be mediating.
>> 
>> I will take that as an extremely weak nack as all I did was expose the
>> existing code and what the code is currently doing.  I don't see how you
>> can NAK what is already being merged and used.
>
> It's a strong NAK.

We are either not understading each other or you have just strong NAK'd
part of the existing LSM api.  Not my proposal.

> LSM is a logical API, it provides an abstraction layer for security 
> policies to mediate kernel security behaviors.

The way it deals with firmware blobs and module loading is not logical.
It is some random pass a NULL pointer into some other security hook.

> Adding an argument to a syscall is not a security behavior.
>
> Loading a firmware file is.

It is a firmware blob not a file.  Perhaps the blob is stored as a file
on-disk, perhaps it is not.

The similar case with kexec never stores all of the data in a file.

Why module_init (which does not take a file) is calling a file based lsm
hook is also bizarre.


Perhaps that means all 3 of these cases should have their own void
security hooks.  Perhaps it means something else.  I just know the name
on the security hook, how it is getting called, and how it is getting
used simply do not agree.

Eric

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec