lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20221013223654.659758-7-keescook@chromium.org>
Date:   Thu, 13 Oct 2022 15:36:52 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Mimi Zohar <zohar@...ux.ibm.com>
Cc:     Kees Cook <keescook@...omium.org>,
        Dmitry Kasatkin <dmitry.kasatkin@...il.com>,
        Paul Moore <paul@...l-moore.com>,
        James Morris <jmorris@...ei.org>,
        "Serge E. Hallyn" <serge@...lyn.com>,
        Jonathan McDowell <noodles@...com>,
        linux-integrity@...r.kernel.org,
        linux-security-module@...r.kernel.org,
        Mickaël Salaün <mic@...ikod.net>,
        KP Singh <kpsingh@...nel.org>,
        Casey Schaufler <casey@...aufler-ca.com>,
        John Johansen <john.johansen@...onical.com>,
        linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: [PATCH 7/9] ima: Move ima_file_check() into LSM

The "file_open" hook in the LSM is the correct place to add the
ima_file_check() callback. Rename it to ima_file_open(), and use the
newly created helper to construct the permissions mask from the file
flags and fmode.

For reference, the LSM hooks across an open are:

    do_filp_open(dfd, filename, open_flags)
        path_openat(nameidata, open_flags, flags)
            file = alloc_empty_file(open_flags, current_cred());
            do_open(nameidata, file, open_flags)
                may_open(path, acc_mode, open_flag)
                    inode_permission(inode, MAY_OPEN | acc_mode)
---->                   security_inode_permission(inode, acc_mode)
                vfs_open(path, file)
                    do_dentry_open(file, path->dentry->d_inode, open)
---->                   security_file_open(f)
                        open()

The open-coded hook in the VFS and NFS are removed, as they are fully
covered by the security_file_open() hook.

Cc: Mimi Zohar <zohar@...ux.ibm.com>
Cc: Dmitry Kasatkin <dmitry.kasatkin@...il.com>
Cc: Paul Moore <paul@...l-moore.com>
Cc: James Morris <jmorris@...ei.org>
Cc: "Serge E. Hallyn" <serge@...lyn.com>
Cc: Jonathan McDowell <noodles@...com>
Cc: linux-integrity@...r.kernel.org
Cc: linux-security-module@...r.kernel.org
Signed-off-by: Kees Cook <keescook@...omium.org>
---
 fs/namei.c                        |  2 --
 fs/nfsd/vfs.c                     |  6 ------
 include/linux/ima.h               |  6 ------
 security/integrity/ima/ima_main.c | 14 +++++++-------
 4 files changed, 7 insertions(+), 21 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 53b4bc094db2..d9bd3887e823 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3555,8 +3555,6 @@ static int do_open(struct nameidata *nd,
 	error = may_open(mnt_userns, &nd->path, acc_mode, open_flag);
 	if (!error && !(file->f_mode & FMODE_OPENED))
 		error = vfs_open(&nd->path, file);
-	if (!error)
-		error = ima_file_check(file, op->acc_mode);
 	if (!error && do_truncate)
 		error = handle_truncate(mnt_userns, file);
 	if (unlikely(error > 0)) {
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 9f486b788ed0..33fe326272df 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -762,12 +762,6 @@ __nfsd_open(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type,
 		goto out_nfserr;
 	}
 
-	host_err = ima_file_check(file, may_flags);
-	if (host_err) {
-		fput(file);
-		goto out_nfserr;
-	}
-
 	if (may_flags & NFSD_MAY_64BIT_COOKIE)
 		file->f_mode |= FMODE_64BITHASH;
 	else
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 70180b9bd974..cf1e48a2d97d 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -16,7 +16,6 @@ struct linux_binprm;
 
 #ifdef CONFIG_IMA
 extern enum hash_algo ima_get_current_hash_algo(void);
-extern int ima_file_check(struct file *file, int mask);
 extern void ima_post_create_tmpfile(struct user_namespace *mnt_userns,
 				    struct inode *inode);
 extern void ima_post_path_mknod(struct user_namespace *mnt_userns,
@@ -45,11 +44,6 @@ static inline enum hash_algo ima_get_current_hash_algo(void)
 	return HASH_ALGO__LAST;
 }
 
-static inline int ima_file_check(struct file *file, int mask)
-{
-	return 0;
-}
-
 static inline void ima_post_create_tmpfile(struct user_namespace *mnt_userns,
 					   struct inode *inode)
 {
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index ffebd3236f24..823d660b53ec 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -12,7 +12,7 @@
  *
  * File: ima_main.c
  *	implements the IMA hooks: ima_bprm_check, ima_file_mmap,
- *	and ima_file_check.
+ *	and ima_file_open.
  */
 
 #include <linux/module.h>
@@ -504,25 +504,24 @@ static int ima_bprm_check(struct linux_binprm *bprm)
 }
 
 /**
- * ima_file_check - based on policy, collect/store measurement.
+ * ima_file_open - based on policy, collect/store measurement.
  * @file: pointer to the file to be measured
- * @mask: contains MAY_READ, MAY_WRITE, MAY_EXEC or MAY_APPEND
  *
  * Measure files based on the ima_must_measure() policy decision.
  *
  * On success return 0.  On integrity appraisal error, assuming the file
  * is in policy and IMA-appraisal is in enforcing mode, return -EACCES.
  */
-int ima_file_check(struct file *file, int mask)
+static int ima_file_open(struct file *file)
 {
+	u32 perms = file_to_perms(file);
 	u32 secid;
 
 	security_current_getsecid_subj(&secid);
+
 	return process_measurement(file, current_cred(), secid, NULL, 0,
-				   mask & (MAY_READ | MAY_WRITE | MAY_EXEC |
-					   MAY_APPEND), FILE_CHECK);
+				   perms, FILE_CHECK);
 }
-EXPORT_SYMBOL_GPL(ima_file_check);
 
 static int __ima_inode_hash(struct inode *inode, struct file *file, char *buf,
 			    size_t buf_size)
@@ -1085,6 +1084,7 @@ static struct security_hook_list ima_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(bprm_check_security, ima_bprm_check),
 	LSM_HOOK_INIT(mmap_file, ima_file_mmap),
 	LSM_HOOK_INIT(file_mprotect, ima_file_mprotect),
+	LSM_HOOK_INIT(file_open, ima_file_open),
 	LSM_HOOK_INIT(file_free_security, ima_file_free),
 	LSM_HOOK_INIT(kernel_read_file, ima_read_file),
 	LSM_HOOK_INIT(kernel_post_read_file, ima_post_read_file),
-- 
2.34.1

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ