LSM: Add "contents" flag to kernel_read_file hook

As with the kernel_load_data LSM hook, add a "contents" flag to the
kernel_read_file LSM hook that indicates whether the LSM can expect
a matching call to the kernel_post_read_file LSM hook with the full
contents of the file. With the coming addition of partial file read
support for kernel_read_file*() API, the LSM will no longer be able
to always see the entire contents of a file during the read calls.

For cases where the LSM must read examine the complete file contents,
it will need to do so on its own every time the kernel_read_file
hook is called with contents=false (or reject such cases). Adjust all
existing LSMs to retain existing behavior.

Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
Link: https://lore.kernel.org/r/20201002173828.2099543-12-keescook@chromium.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/fs/kernel_read_file.c b/fs/kernel_read_file.c
index 2e29c38..d73bc3f 100644 (file)
--- a/fs/kernel_read_file.c
+++ b/fs/kernel_read_file.c
@@ -39,7 +39,7 @@ int kernel_read_file(struct file *file, void **buf,
        if (ret)
                return ret;
 
-       ret = security_kernel_read_file(file, id);
+       ret = security_kernel_read_file(file, id, true);
        if (ret)
                goto out;
 

diff --git a/include/linux/ima.h b/include/linux/ima.h
index af9fb8c..8fa7bcf 100644 (file)
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -23,7 +23,8 @@ extern int ima_file_mprotect(struct vm_area_struct *vma, unsigned long prot);
 extern int ima_load_data(enum kernel_load_data_id id, bool contents);
 extern int ima_post_load_data(char *buf, loff_t size,
                              enum kernel_load_data_id id, char *description);
-extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
+extern int ima_read_file(struct file *file, enum kernel_read_file_id id,
+                        bool contents);
 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);
@@ -92,7 +93,8 @@ static inline int ima_post_load_data(char *buf, loff_t size,
        return 0;
 }
 
-static inline int ima_read_file(struct file *file, enum kernel_read_file_id id)
+static inline int ima_read_file(struct file *file, enum kernel_read_file_id id,
+                               bool contents)
 {
        return 0;
 }

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 83c6f1f..d67cb35 100644 (file)
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -188,7 +188,7 @@ LSM_HOOK(int, 0, kernel_load_data, enum kernel_load_data_id id, bool contents)
 LSM_HOOK(int, 0, kernel_post_load_data, char *buf, loff_t size,
         enum kernel_read_file_id id, char *description)
 LSM_HOOK(int, 0, kernel_read_file, struct file *file,
-        enum kernel_read_file_id id)
+        enum kernel_read_file_id id, bool contents)
 LSM_HOOK(int, 0, kernel_post_read_file, struct file *file, char *buf,
         loff_t size, enum kernel_read_file_id id)
 LSM_HOOK(int, 0, task_fix_setuid, struct cred *new, const struct cred *old,

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 6bb4f1a..8814e3d 100644 (file)
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -651,6 +651,7 @@
  *     @file contains the file structure pointing to the file being read
  *     by the kernel.
  *     @id kernel read file identifier
+ *     @contents if a subsequent @kernel_post_read_file will be called.
  *     Return 0 if permission is granted.
  * @kernel_post_read_file:
  *     Read a file specified by userspace.
@@ -659,6 +660,8 @@
  *     @buf pointer to buffer containing the file contents.
  *     @size length of the file contents.
  *     @id kernel read file identifier
+ *     This must be paired with a prior @kernel_read_file call that had
+ *     @contents set to true.
  *     Return 0 if permission is granted.
  * @task_fix_setuid:
  *     Update the module's state after setting one or more of the user

diff --git a/include/linux/security.h b/include/linux/security.h
index 51c8e4e..bc27254 100644 (file)
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -391,7 +391,8 @@ int security_kernel_load_data(enum kernel_load_data_id id, bool contents);
 int security_kernel_post_load_data(char *buf, loff_t size,
                                   enum kernel_load_data_id id,
                                   char *description);
-int security_kernel_read_file(struct file *file, enum kernel_read_file_id id);
+int security_kernel_read_file(struct file *file, enum kernel_read_file_id id,
+                             bool contents);
 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,
@@ -1030,7 +1031,8 @@ static inline int security_kernel_post_load_data(char *buf, loff_t size,
 }
 
 static inline int security_kernel_read_file(struct file *file,
-                                           enum kernel_read_file_id id)
+                                           enum kernel_read_file_id id,
+                                           bool contents)
 {
        return 0;
 }

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 6f2b835..939f53d 100644 (file)
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -602,6 +602,7 @@ void ima_post_path_mknod(struct dentry *dentry)
  * 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
+ * @contents: whether a subsequent call will be made to ima_post_read_file()
  *
  * Permit reading a file based on policy. The policy rules are written
  * in terms of the policy identifier.  Appraising the integrity of
@@ -609,8 +610,15 @@ 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_file(struct file *file, enum kernel_read_file_id read_id,
+                 bool contents)
 {
+       /* Reject all partial reads during appraisal. */
+       if (!contents) {
+               if (ima_appraise & IMA_APPRAISE_ENFORCE)
+                       return -EACCES;
+       }
+
        /*
         * Do devices using pre-allocated memory run the risk of the
         * firmware being accessible to the device prior to the completion

diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
index 2878241..b12f7d9 100644 (file)
--- a/security/loadpin/loadpin.c
+++ b/security/loadpin/loadpin.c
@@ -118,11 +118,21 @@ 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_file(struct file *file, enum kernel_read_file_id id,
+                            bool contents)
 {
        struct super_block *load_root;
        const char *origin = kernel_read_file_id_str(id);
 
+       /*
+        * If we will not know that we'll be seeing the full contents
+        * then we cannot trust a load will be complete and unchanged
+        * off disk. Treat all contents=false hooks as if there were
+        * no associated file struct.
+        */
+       if (!contents)
+               file = NULL;
+
        /* If the file id is excluded, ignore the pinning. */
        if ((unsigned int)id < ARRAY_SIZE(ignore_read_file_id) &&
            ignore_read_file_id[id]) {
@@ -179,7 +189,7 @@ static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
 
 static int loadpin_load_data(enum kernel_load_data_id id, bool contents)
 {
-       return loadpin_read_file(NULL, (enum kernel_read_file_id) id);
+       return loadpin_read_file(NULL, (enum kernel_read_file_id) id, contents);
 }
 
 static struct security_hook_list loadpin_hooks[] __lsm_ro_after_init = {

diff --git a/security/security.c b/security/security.c
index 531b855..a28045d 100644 (file)
--- a/security/security.c
+++ b/security/security.c
@@ -1672,14 +1672,15 @@ int security_kernel_module_request(char *kmod_name)
        return integrity_kernel_module_request(kmod_name);
 }
 
-int security_kernel_read_file(struct file *file, enum kernel_read_file_id id)
+int security_kernel_read_file(struct file *file, enum kernel_read_file_id id,
+                             bool contents)
 {
        int ret;
 
-       ret = call_int_hook(kernel_read_file, 0, file, id);
+       ret = call_int_hook(kernel_read_file, 0, file, id, contents);
        if (ret)
                return ret;
-       return ima_read_file(file, id);
+       return ima_read_file(file, id, contents);
 }
 EXPORT_SYMBOL_GPL(security_kernel_read_file);
 

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 558beee..dec654d 100644 (file)
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4003,13 +4003,14 @@ static int selinux_kernel_module_from_file(struct file *file)
 }
 
 static int selinux_kernel_read_file(struct file *file,
-                                   enum kernel_read_file_id id)
+                                   enum kernel_read_file_id id,
+                                   bool contents)
 {
        int rc = 0;
 
        switch (id) {
        case READING_MODULE:
-               rc = selinux_kernel_module_from_file(file);
+               rc = selinux_kernel_module_from_file(contents ? file : NULL);
                break;
        default:
                break;