* [PATCH 06/11] capabilities: Allow privileged user in s_user_ns to set security.* xattrs
[not found] <cover.1512741134.git.dongsu@kinvolk.io>
@ 2017-12-22 14:32 ` Dongsu Park
2017-12-23 3:33 ` Serge E. Hallyn
2017-12-22 14:32 ` [PATCH 11/11] evm: Don't update hmacs in user ns mounts Dongsu Park
1 sibling, 1 reply; 6+ messages in thread
From: Dongsu Park @ 2017-12-22 14:32 UTC (permalink / raw)
To: linux-security-module
From: Seth Forshee <seth.forshee@canonical.com>
A privileged user in s_user_ns will generally have the ability to
manipulate the backing store and insert security.* xattrs into
the filesystem directly. Therefore the kernel must be prepared to
handle these xattrs from unprivileged mounts, and it makes little
sense for commoncap to prevent writing these xattrs to the
filesystem. The capability and LSM code have already been updated
to appropriately handle xattrs from unprivileged mounts, so it
is safe to loosen this restriction on setting xattrs.
The exception to this logic is that writing xattrs to a mounted
filesystem may also cause the LSM inode_post_setxattr or
inode_setsecurity callbacks to be invoked. SELinux will deny the
xattr update by virtue of applying mountpoint labeling to
unprivileged userns mounts, and Smack will deny the writes for
any user without global CAP_MAC_ADMIN, so loosening the
capability check in commoncap is safe in this respect as well.
Patch v4 is available: https://patchwork.kernel.org/patch/8944641/
Cc: linux-security-module at vger.kernel.org
Cc: linux-kernel at vger.kernel.org
Cc: James Morris <james.l.morris@oracle.com>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
Signed-off-by: Dongsu Park <dongsu@kinvolk.io>
---
security/commoncap.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/security/commoncap.c b/security/commoncap.c
index 4f8e0934..dd0afef9 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -920,6 +920,8 @@ int cap_bprm_set_creds(struct linux_binprm *bprm)
int cap_inode_setxattr(struct dentry *dentry, const char *name,
const void *value, size_t size, int flags)
{
+ struct user_namespace *user_ns = dentry->d_sb->s_user_ns;
+
/* Ignore non-security xattrs */
if (strncmp(name, XATTR_SECURITY_PREFIX,
sizeof(XATTR_SECURITY_PREFIX) - 1) != 0)
@@ -932,7 +934,7 @@ int cap_inode_setxattr(struct dentry *dentry, const char *name,
if (strcmp(name, XATTR_NAME_CAPS) == 0)
return 0;
- if (!capable(CAP_SYS_ADMIN))
+ if (!ns_capable(user_ns, CAP_SYS_ADMIN))
return -EPERM;
return 0;
}
@@ -950,6 +952,8 @@ int cap_inode_setxattr(struct dentry *dentry, const char *name,
*/
int cap_inode_removexattr(struct dentry *dentry, const char *name)
{
+ struct user_namespace *user_ns = dentry->d_sb->s_user_ns;
+
/* Ignore non-security xattrs */
if (strncmp(name, XATTR_SECURITY_PREFIX,
sizeof(XATTR_SECURITY_PREFIX) - 1) != 0)
@@ -965,7 +969,7 @@ int cap_inode_removexattr(struct dentry *dentry, const char *name)
return 0;
}
- if (!capable(CAP_SYS_ADMIN))
+ if (!ns_capable(user_ns, CAP_SYS_ADMIN))
return -EPERM;
return 0;
}
--
2.13.6
--
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 11/11] evm: Don't update hmacs in user ns mounts
[not found] <cover.1512741134.git.dongsu@kinvolk.io>
2017-12-22 14:32 ` [PATCH 06/11] capabilities: Allow privileged user in s_user_ns to set security.* xattrs Dongsu Park
@ 2017-12-22 14:32 ` Dongsu Park
2017-12-23 4:03 ` Serge E. Hallyn
1 sibling, 1 reply; 6+ messages in thread
From: Dongsu Park @ 2017-12-22 14:32 UTC (permalink / raw)
To: linux-security-module
From: Seth Forshee <seth.forshee@canonical.com>
The kernel should not calculate new hmacs for mounts done by
non-root users. Update evm_calc_hmac_or_hash() to refuse to
calculate new hmacs for mounts for non-init user namespaces.
Cc: linux-integrity at vger.kernel.org
Cc: linux-security-module at vger.kernel.org
Cc: linux-kernel at vger.kernel.org
Cc: James Morris <james.l.morris@oracle.com>
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
Signed-off-by: Dongsu Park <dongsu@kinvolk.io>
---
security/integrity/evm/evm_crypto.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index bcd64baf..729f4545 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -190,7 +190,8 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
int error;
int size;
- if (!(inode->i_opflags & IOP_XATTR))
+ if (!(inode->i_opflags & IOP_XATTR) ||
+ inode->i_sb->s_user_ns != &init_user_ns)
return -EOPNOTSUPP;
desc = init_desc(type);
--
2.13.6
--
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 06/11] capabilities: Allow privileged user in s_user_ns to set security.* xattrs
2017-12-22 14:32 ` [PATCH 06/11] capabilities: Allow privileged user in s_user_ns to set security.* xattrs Dongsu Park
@ 2017-12-23 3:33 ` Serge E. Hallyn
0 siblings, 0 replies; 6+ messages in thread
From: Serge E. Hallyn @ 2017-12-23 3:33 UTC (permalink / raw)
To: linux-security-module
On Fri, Dec 22, 2017 at 03:32:30PM +0100, Dongsu Park wrote:
> From: Seth Forshee <seth.forshee@canonical.com>
>
> A privileged user in s_user_ns will generally have the ability to
> manipulate the backing store and insert security.* xattrs into
> the filesystem directly. Therefore the kernel must be prepared to
> handle these xattrs from unprivileged mounts, and it makes little
> sense for commoncap to prevent writing these xattrs to the
> filesystem. The capability and LSM code have already been updated
> to appropriately handle xattrs from unprivileged mounts, so it
> is safe to loosen this restriction on setting xattrs.
>
> The exception to this logic is that writing xattrs to a mounted
> filesystem may also cause the LSM inode_post_setxattr or
> inode_setsecurity callbacks to be invoked. SELinux will deny the
> xattr update by virtue of applying mountpoint labeling to
> unprivileged userns mounts, and Smack will deny the writes for
> any user without global CAP_MAC_ADMIN, so loosening the
> capability check in commoncap is safe in this respect as well.
>
> Patch v4 is available: https://patchwork.kernel.org/patch/8944641/
>
> Cc: linux-security-module at vger.kernel.org
> Cc: linux-kernel at vger.kernel.org
> Cc: James Morris <james.l.morris@oracle.com>
> Cc: Serge Hallyn <serge@hallyn.com>
Reviewed-by: Serge Hallyn <serge@hallyn.com>
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> Signed-off-by: Dongsu Park <dongsu@kinvolk.io>
> ---
> security/commoncap.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 4f8e0934..dd0afef9 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -920,6 +920,8 @@ int cap_bprm_set_creds(struct linux_binprm *bprm)
> int cap_inode_setxattr(struct dentry *dentry, const char *name,
> const void *value, size_t size, int flags)
> {
> + struct user_namespace *user_ns = dentry->d_sb->s_user_ns;
> +
> /* Ignore non-security xattrs */
> if (strncmp(name, XATTR_SECURITY_PREFIX,
> sizeof(XATTR_SECURITY_PREFIX) - 1) != 0)
> @@ -932,7 +934,7 @@ int cap_inode_setxattr(struct dentry *dentry, const char *name,
> if (strcmp(name, XATTR_NAME_CAPS) == 0)
> return 0;
>
> - if (!capable(CAP_SYS_ADMIN))
> + if (!ns_capable(user_ns, CAP_SYS_ADMIN))
> return -EPERM;
> return 0;
> }
> @@ -950,6 +952,8 @@ int cap_inode_setxattr(struct dentry *dentry, const char *name,
> */
> int cap_inode_removexattr(struct dentry *dentry, const char *name)
> {
> + struct user_namespace *user_ns = dentry->d_sb->s_user_ns;
> +
> /* Ignore non-security xattrs */
> if (strncmp(name, XATTR_SECURITY_PREFIX,
> sizeof(XATTR_SECURITY_PREFIX) - 1) != 0)
> @@ -965,7 +969,7 @@ int cap_inode_removexattr(struct dentry *dentry, const char *name)
> return 0;
> }
>
> - if (!capable(CAP_SYS_ADMIN))
> + if (!ns_capable(user_ns, CAP_SYS_ADMIN))
> return -EPERM;
> return 0;
> }
> --
> 2.13.6
--
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 11/11] evm: Don't update hmacs in user ns mounts
2017-12-22 14:32 ` [PATCH 11/11] evm: Don't update hmacs in user ns mounts Dongsu Park
@ 2017-12-23 4:03 ` Serge E. Hallyn
2017-12-24 5:12 ` Mimi Zohar
0 siblings, 1 reply; 6+ messages in thread
From: Serge E. Hallyn @ 2017-12-23 4:03 UTC (permalink / raw)
To: linux-security-module
On Fri, Dec 22, 2017 at 03:32:35PM +0100, Dongsu Park wrote:
> From: Seth Forshee <seth.forshee@canonical.com>
>
> The kernel should not calculate new hmacs for mounts done by
> non-root users. Update evm_calc_hmac_or_hash() to refuse to
> calculate new hmacs for mounts for non-init user namespaces.
>
> Cc: linux-integrity at vger.kernel.org
> Cc: linux-security-module at vger.kernel.org
> Cc: linux-kernel at vger.kernel.org
> Cc: James Morris <james.l.morris@oracle.com>
> Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Hi Mimi,
does this change seem sufficient to you?
> Cc: "Serge E. Hallyn" <serge@hallyn.com>
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> Signed-off-by: Dongsu Park <dongsu@kinvolk.io>
> ---
> security/integrity/evm/evm_crypto.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> index bcd64baf..729f4545 100644
> --- a/security/integrity/evm/evm_crypto.c
> +++ b/security/integrity/evm/evm_crypto.c
> @@ -190,7 +190,8 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
> int error;
> int size;
>
> - if (!(inode->i_opflags & IOP_XATTR))
> + if (!(inode->i_opflags & IOP_XATTR) ||
> + inode->i_sb->s_user_ns != &init_user_ns)
> return -EOPNOTSUPP;
>
> desc = init_desc(type);
> --
> 2.13.6
--
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 11/11] evm: Don't update hmacs in user ns mounts
2017-12-23 4:03 ` Serge E. Hallyn
@ 2017-12-24 5:12 ` Mimi Zohar
2017-12-24 5:56 ` Mimi Zohar
0 siblings, 1 reply; 6+ messages in thread
From: Mimi Zohar @ 2017-12-24 5:12 UTC (permalink / raw)
To: linux-security-module
Hi Serge,
On Fri, 2017-12-22 at 22:03 -0600, Serge E. Hallyn wrote:
> On Fri, Dec 22, 2017 at 03:32:35PM +0100, Dongsu Park wrote:
> > From: Seth Forshee <seth.forshee@canonical.com>
> >
> > The kernel should not calculate new hmacs for mounts done by
> > non-root users. Update evm_calc_hmac_or_hash() to refuse to
> > calculate new hmacs for mounts for non-init user namespaces.
> >
> > Cc: linux-integrity at vger.kernel.org
> > Cc: linux-security-module at vger.kernel.org
> > Cc: linux-kernel at vger.kernel.org
> > Cc: James Morris <james.l.morris@oracle.com>
> > Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
>
> Hi Mimi,
>
> does this change seem sufficient to you?
I think this is the correct behavior in the context of fuse file
systems. ?This patch, the "ima: define a new policy option named
force" patch, and an updated IMA policy should be upstreamed together.
?The cover letter should provide the motivation for these patches.
Mimi
>
> > Cc: "Serge E. Hallyn" <serge@hallyn.com>
> > Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> > Signed-off-by: Dongsu Park <dongsu@kinvolk.io>
> > ---
> > security/integrity/evm/evm_crypto.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> > index bcd64baf..729f4545 100644
> > --- a/security/integrity/evm/evm_crypto.c
> > +++ b/security/integrity/evm/evm_crypto.c
> > @@ -190,7 +190,8 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
> > int error;
> > int size;
> >
> > - if (!(inode->i_opflags & IOP_XATTR))
> > + if (!(inode->i_opflags & IOP_XATTR) ||
> > + inode->i_sb->s_user_ns != &init_user_ns)
> > return -EOPNOTSUPP;
> >
> > desc = init_desc(type);
> > --
> > 2.13.6
>
--
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 11/11] evm: Don't update hmacs in user ns mounts
2017-12-24 5:12 ` Mimi Zohar
@ 2017-12-24 5:56 ` Mimi Zohar
0 siblings, 0 replies; 6+ messages in thread
From: Mimi Zohar @ 2017-12-24 5:56 UTC (permalink / raw)
To: linux-security-module
On Sun, 2017-12-24 at 00:12 -0500, Mimi Zohar wrote:
> Hi Serge,
>
> On Fri, 2017-12-22 at 22:03 -0600, Serge E. Hallyn wrote:
> > On Fri, Dec 22, 2017 at 03:32:35PM +0100, Dongsu Park wrote:
> > > From: Seth Forshee <seth.forshee@canonical.com>
> > >
> > > The kernel should not calculate new hmacs for mounts done by
> > > non-root users. Update evm_calc_hmac_or_hash() to refuse to
> > > calculate new hmacs for mounts for non-init user namespaces.
> > >
> > > Cc: linux-integrity at vger.kernel.org
> > > Cc: linux-security-module at vger.kernel.org
> > > Cc: linux-kernel at vger.kernel.org
> > > Cc: James Morris <james.l.morris@oracle.com>
> > > Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
> >
> > Hi Mimi,
> >
> > does this change seem sufficient to you?
>
> I think this is the correct behavior in the context of fuse file
> systems. ?This patch, the "ima: define a new policy option named
> force" patch, and an updated IMA policy should be upstreamed together.
> ?The cover letter should provide the motivation for these patches.
Ah, this patch is being upstreamed with the fuse mounts patches. ?I
guess Seth is planning on posting the IMA policy changes for fuse
separately.
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-12-24 5:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <cover.1512741134.git.dongsu@kinvolk.io>
2017-12-22 14:32 ` [PATCH 06/11] capabilities: Allow privileged user in s_user_ns to set security.* xattrs Dongsu Park
2017-12-23 3:33 ` Serge E. Hallyn
2017-12-22 14:32 ` [PATCH 11/11] evm: Don't update hmacs in user ns mounts Dongsu Park
2017-12-23 4:03 ` Serge E. Hallyn
2017-12-24 5:12 ` Mimi Zohar
2017-12-24 5:56 ` Mimi Zohar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).