| Message ID | 20220125224645.79319-13-stefanb@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ima: Namespace IMA with audit support in IMA-ns | expand |
On Tue, Jan 25, 2022 at 05:46:34PM -0500, Stefan Berger wrote: > From: Stefan Berger <stefanb@linux.ibm.com> > > Define mac_admin_ns_capable() as a wrapper for the combined ns_capable() > checks on CAP_MAC_ADMIN and CAP_SYS_ADMIN in a user namespace. Return > true on the check if either capability or both are available. > > Use mac_admin_ns_capable() in place of capable(SYS_ADMIN). This will allow > an IMA namespace to read the policy with only CAP_MAC_ADMIN, which has > less privileges than CAP_SYS_ADMIN. > > Signed-off-by: Denis Semakin <denis.semakin@huawei.com> > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > --- It's unfortunate that the securityfs namespacing patch with FS_USERNS_MOUNT set comes before this patch. The check below with ima_open_policy() operates on an unprivileged userns if securityfs is mounted in there. But the ima namespace is the hardcoded as the initial ima namespace. I'd rather have the securityfs namespacing patch moved very late in the series. Even if might not yet possible to open an ima policy in a non-init userns at this point in the patch series, it still is confusing in terms of ordering. Especially when considering this were to land in an upstream tree and one would later in a few years have to read through the history of this code again. > static int ima_open_policy(struct inode *inode, struct file *filp) > { > +#ifdef CONFIG_IMA_READ_POLICY > + struct user_namespace *user_ns = ima_user_ns_from_file(filp); > +#endif > include/linux/capability.h | 6 ++++++ > security/integrity/ima/ima.h | 6 ++++++ > security/integrity/ima/ima_fs.c | 5 ++++- > 3 files changed, 16 insertions(+), 1 deletion(-) > > diff --git a/include/linux/capability.h b/include/linux/capability.h > index 65efb74c3585..991579178f32 100644 > --- a/include/linux/capability.h > +++ b/include/linux/capability.h > @@ -270,6 +270,12 @@ static inline bool checkpoint_restore_ns_capable(struct user_namespace *ns) > ns_capable(ns, CAP_SYS_ADMIN); > } > > +static inline bool mac_admin_ns_capable(struct user_namespace *ns) > +{ > + return ns_capable(ns, CAP_MAC_ADMIN) || > + ns_capable(ns, CAP_SYS_ADMIN); > +} > + > /* audit system wants to get cap info from files as well */ > int get_vfs_caps_from_disk(struct user_namespace *mnt_userns, > const struct dentry *dentry, > diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h > index fb6bd054d899..0057b1fd6c18 100644 > --- a/security/integrity/ima/ima.h > +++ b/security/integrity/ima/ima.h > @@ -487,4 +487,10 @@ static inline int ima_filter_rule_match(u32 secid, u32 field, u32 op, > #define POLICY_FILE_FLAGS S_IWUSR > #endif /* CONFIG_IMA_READ_POLICY */ > > +static inline > +struct user_namespace *ima_user_ns_from_file(const struct file *filp) > +{ > + return file_inode(filp)->i_sb->s_user_ns; > +} > + > #endif /* __LINUX_IMA_H */ > diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c > index 3afb7a74d2cf..9162f06d182f 100644 > --- a/security/integrity/ima/ima_fs.c > +++ b/security/integrity/ima/ima_fs.c > @@ -377,6 +377,9 @@ static const struct seq_operations ima_policy_seqops = { > */ > static int ima_open_policy(struct inode *inode, struct file *filp) > { > +#ifdef CONFIG_IMA_READ_POLICY > + struct user_namespace *user_ns = ima_user_ns_from_file(filp); > +#endif > struct ima_namespace *ns = &init_ima_ns; > > if (!(filp->f_flags & O_WRONLY)) { > @@ -385,7 +388,7 @@ static int ima_open_policy(struct inode *inode, struct file *filp) > #else > if ((filp->f_flags & O_ACCMODE) != O_RDONLY) > return -EACCES; > - if (!capable(CAP_SYS_ADMIN)) > + if (!mac_admin_ns_capable(user_ns)) > return -EPERM; > return seq_open(filp, &ima_policy_seqops); > #endif > -- > 2.31.1 > >
diff --git a/include/linux/capability.h b/include/linux/capability.h index 65efb74c3585..991579178f32 100644 --- a/include/linux/capability.h +++ b/include/linux/capability.h @@ -270,6 +270,12 @@ static inline bool checkpoint_restore_ns_capable(struct user_namespace *ns) ns_capable(ns, CAP_SYS_ADMIN); } +static inline bool mac_admin_ns_capable(struct user_namespace *ns) +{ + return ns_capable(ns, CAP_MAC_ADMIN) || + ns_capable(ns, CAP_SYS_ADMIN); +} + /* audit system wants to get cap info from files as well */ int get_vfs_caps_from_disk(struct user_namespace *mnt_userns, const struct dentry *dentry, diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index fb6bd054d899..0057b1fd6c18 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -487,4 +487,10 @@ static inline int ima_filter_rule_match(u32 secid, u32 field, u32 op, #define POLICY_FILE_FLAGS S_IWUSR #endif /* CONFIG_IMA_READ_POLICY */ +static inline +struct user_namespace *ima_user_ns_from_file(const struct file *filp) +{ + return file_inode(filp)->i_sb->s_user_ns; +} + #endif /* __LINUX_IMA_H */ diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index 3afb7a74d2cf..9162f06d182f 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -377,6 +377,9 @@ static const struct seq_operations ima_policy_seqops = { */ static int ima_open_policy(struct inode *inode, struct file *filp) { +#ifdef CONFIG_IMA_READ_POLICY + struct user_namespace *user_ns = ima_user_ns_from_file(filp); +#endif struct ima_namespace *ns = &init_ima_ns; if (!(filp->f_flags & O_WRONLY)) { @@ -385,7 +388,7 @@ static int ima_open_policy(struct inode *inode, struct file *filp) #else if ((filp->f_flags & O_ACCMODE) != O_RDONLY) return -EACCES; - if (!capable(CAP_SYS_ADMIN)) + if (!mac_admin_ns_capable(user_ns)) return -EPERM; return seq_open(filp, &ima_policy_seqops); #endif