* [RFC PATCH 2/2] fs: add the possibility to use CAP_SYS_MOUNT to (u)mount a fs
2017-10-21 13:43 [RFC PATCH 1/2] security, capabilities: Add CAP_SYS_MOUNT Nicolas Belouin
@ 2017-10-21 13:43 ` Nicolas Belouin
2017-10-21 17:31 ` [kernel-hardening] [RFC PATCH 1/2] security, capabilities: Add CAP_SYS_MOUNT Casey Schaufler
2017-10-23 12:57 ` Stephen Smalley
2 siblings, 0 replies; 6+ messages in thread
From: Nicolas Belouin @ 2017-10-21 13:43 UTC (permalink / raw)
To: David Howells, Theodore Ts'o, Andreas Dilger, Alexander Viro,
Serge Hallyn, Paul Moore, Stephen Smalley, Eric Paris,
James Morris, linux-cachefs, linux-kernel, linux-ext4,
linux-fsdevel, linux-security-module, selinux, linux-api,
kernel-hardening
Cc: Nicolas Belouin
Fulfill the purpose of CAP_SYS_MOUNT by adding it as a sufficient
capability to mount and unmount filesystems.
Signed-off-by: Nicolas Belouin <nicolas@belouin.fr>
---
fs/cachefiles/daemon.c | 2 +-
fs/ext4/ioctl.c | 2 +-
fs/namespace.c | 3 ++-
fs/super.c | 14 +++++++++-----
4 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/fs/cachefiles/daemon.c b/fs/cachefiles/daemon.c
index 1ee54ffd3a24..fc53bdeacc8a 100644
--- a/fs/cachefiles/daemon.c
+++ b/fs/cachefiles/daemon.c
@@ -91,7 +91,7 @@ static int cachefiles_daemon_open(struct inode *inode, struct file *file)
_enter("");
/* only the superuser may do this */
- if (!capable(CAP_SYS_ADMIN))
+ if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_MOUNT))
return -EPERM;
/* the cachefiles device may only be open once at a time */
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index afb66d4ab5cf..19d838e558e2 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -458,7 +458,7 @@ static int ext4_shutdown(struct super_block *sb, unsigned long arg)
struct ext4_sb_info *sbi = EXT4_SB(sb);
__u32 flags;
- if (!capable(CAP_SYS_ADMIN))
+ if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_MOUNT))
return -EPERM;
if (get_user(flags, (__u32 __user *)arg))
diff --git a/fs/namespace.c b/fs/namespace.c
index 3b601f115b6c..1eaa6a9f1631 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1661,7 +1661,8 @@ void __detach_mounts(struct dentry *dentry)
*/
static inline bool may_mount(void)
{
- return ns_capable(current->nsproxy->mnt_ns->user_ns, CAP_SYS_ADMIN);
+ return ns_capable(current->nsproxy->mnt_ns->user_ns, CAP_SYS_ADMIN) ||
+ ns_capable(current->nsproxy->mnt_ns->user_ns, CAP_SYS_MOUNT);
}
static inline bool may_mandlock(void)
diff --git a/fs/super.c b/fs/super.c
index 166c4ee0d0ed..1d84d8b87216 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -474,7 +474,7 @@ struct super_block *sget_userns(struct file_system_type *type,
if (!(flags & (SB_KERNMOUNT|SB_SUBMOUNT)) &&
!(type->fs_flags & FS_USERNS_MOUNT) &&
- !capable(CAP_SYS_ADMIN))
+ !capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_MOUNT))
return ERR_PTR(-EPERM);
retry:
spin_lock(&sb_lock);
@@ -551,7 +551,9 @@ struct super_block *sget(struct file_system_type *type,
user_ns = &init_user_ns;
/* Ensure the requestor has permissions over the target filesystem */
- if (!(flags & (SB_KERNMOUNT|SB_SUBMOUNT)) && !ns_capable(user_ns, CAP_SYS_ADMIN))
+ if (!(flags & (SB_KERNMOUNT | SB_SUBMOUNT)) &&
+ !ns_capable(user_ns, CAP_SYS_ADMIN) &&
+ !ns_capable(user_ns, CAP_SYS_MOUNT))
return ERR_PTR(-EPERM);
return sget_userns(type, test, set, flags, user_ns, data);
@@ -1020,10 +1022,12 @@ struct dentry *mount_ns(struct file_system_type *fs_type,
{
struct super_block *sb;
- /* Don't allow mounting unless the caller has CAP_SYS_ADMIN
- * over the namespace.
+ /* Don't allow mounting unless the caller has CAP_SYS_ADMIN (deprecated)
+ * or CAP_SYS_MOUNT over the namespace.
*/
- if (!(flags & SB_KERNMOUNT) && !ns_capable(user_ns, CAP_SYS_ADMIN))
+ if (!(flags & SB_KERNMOUNT) &&
+ !ns_capable(user_ns, CAP_SYS_ADMIN) &&
+ !ns_capable(user_ns, CAP_SYS_MOUNT))
return ERR_PTR(-EPERM);
sb = sget_userns(fs_type, ns_test_super, ns_set_super, flags,
--
2.14.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [kernel-hardening] [RFC PATCH 1/2] security, capabilities: Add CAP_SYS_MOUNT
2017-10-21 13:43 [RFC PATCH 1/2] security, capabilities: Add CAP_SYS_MOUNT Nicolas Belouin
2017-10-21 13:43 ` [RFC PATCH 2/2] fs: add the possibility to use CAP_SYS_MOUNT to (u)mount a fs Nicolas Belouin
@ 2017-10-21 17:31 ` Casey Schaufler
2017-10-21 18:41 ` Nicolas Belouin
2017-10-23 12:57 ` Stephen Smalley
2 siblings, 1 reply; 6+ messages in thread
From: Casey Schaufler @ 2017-10-21 17:31 UTC (permalink / raw)
To: Nicolas Belouin, David Howells, Theodore Ts'o, Andreas Dilger,
Alexander Viro, Serge Hallyn, Paul Moore, Stephen Smalley,
Eric Paris, James Morris, linux-cachefs, linux-kernel, linux-ext4,
linux-fsdevel, linux-security-module, selinux, linux-api,
kernel-hardening
On 10/21/2017 6:43 AM, Nicolas Belouin wrote:
> With CAP_SYS_ADMIN being bloated and inapropriate for actions such
> as mounting/unmounting filesystems, the creation of a new capability
> is needed.
> CAP_SYS_MOUNT is meant to give a process the ability to call for mount,
> umount and umount2 syscalls.
This is increased granularity for it's own sake. There is no
compelling reason to break out this capability in particular.
Can you identify existing use cases where you would have
CAP_SYS_MOUNT without also having CAP_SYS_ADMIN? I should think
that all the work that's gone into unprivileged mounts over
the past couple years would make this unnecessary.
> Signed-off-by: Nicolas Belouin <nicolas@belouin.fr>
> ---
> include/uapi/linux/capability.h | 5 ++++-
> security/selinux/include/classmap.h | 4 ++--
> 2 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/include/uapi/linux/capability.h b/include/uapi/linux/capability.h
> index 230e05d35191..ce230aa6d928 100644
> --- a/include/uapi/linux/capability.h
> +++ b/include/uapi/linux/capability.h
> @@ -365,8 +365,11 @@ struct vfs_ns_cap_data {
>
> #define CAP_AUDIT_READ 37
>
> +/* Allow mounting, unmounting filesystems */
>
> -#define CAP_LAST_CAP CAP_AUDIT_READ
> +#define CAP_SYS_MOUNT 38
> +
> +#define CAP_LAST_CAP CAP_SYS_MOUNT
>
> #define cap_valid(x) ((x) >= 0 && (x) <= CAP_LAST_CAP)
>
> diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
> index 35ffb29a69cb..a873dce97fd5 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -24,9 +24,9 @@
> "audit_control", "setfcap"
>
> #define COMMON_CAP2_PERMS "mac_override", "mac_admin", "syslog", \
> - "wake_alarm", "block_suspend", "audit_read"
> + "wake_alarm", "block_suspend", "audit_read", "sys_mount"
>
> -#if CAP_LAST_CAP > CAP_AUDIT_READ
> +#if CAP_LAST_CAP > CAP_SYS_MOUNT
> #error New capability defined, please update COMMON_CAP2_PERMS.
> #endif
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [kernel-hardening] [RFC PATCH 1/2] security, capabilities: Add CAP_SYS_MOUNT
2017-10-21 17:31 ` [kernel-hardening] [RFC PATCH 1/2] security, capabilities: Add CAP_SYS_MOUNT Casey Schaufler
@ 2017-10-21 18:41 ` Nicolas Belouin
2017-10-22 0:54 ` Casey Schaufler
0 siblings, 1 reply; 6+ messages in thread
From: Nicolas Belouin @ 2017-10-21 18:41 UTC (permalink / raw)
To: kernel-hardening, Casey Schaufler, David Howells,
Theodore Ts'o, Andreas Dilger, Alexander Viro, Serge Hallyn,
Paul Moore, Stephen Smalley, Eric Paris, James Morris,
linux-cachefs, linux-kernel, linux-ext4, linux-fsdevel,
linux-security-module, selinux, linux-api
On October 21, 2017 7:31:24 PM GMT+02:00, Casey Schaufler <casey@schaufler-ca.com> wrote:
>On 10/21/2017 6:43 AM, Nicolas Belouin wrote:
>> With CAP_SYS_ADMIN being bloated and inapropriate for actions such
>> as mounting/unmounting filesystems, the creation of a new capability
>> is needed.
>> CAP_SYS_MOUNT is meant to give a process the ability to call for
>mount,
>> umount and umount2 syscalls.
>
>This is increased granularity for it's own sake. There is no
>compelling reason to break out this capability in particular.
Obviously there is a need to break CAP_SYS_ADMIN in pieces, to do so, you have to start somewhere, so I chose to begin with this.
>Can you identify existing use cases where you would have
>CAP_SYS_MOUNT without also having CAP_SYS_ADMIN? I should think
>that all the work that's gone into unprivileged mounts over
>the past couple years would make this unnecessary.
If you look at the udiskd deamon used by most desktop environments, it is launched as root or at least with CAP_SYS_ADMIN. Here, you could use CAP_SYS_MOUNT. There might also be a use within containers as you don't want to give CAP_SYS_ADMIN to a container if it just need to mount/unmount filesystems. If you go even further, it could be used to allow swapon/swapoff (maybe in future patch set).
>
>> Signed-off-by: Nicolas Belouin <nicolas@belouin.fr>
>> ---
>> include/uapi/linux/capability.h | 5 ++++-
>> security/selinux/include/classmap.h | 4 ++--
>> 2 files changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/uapi/linux/capability.h
>b/include/uapi/linux/capability.h
>> index 230e05d35191..ce230aa6d928 100644
>> --- a/include/uapi/linux/capability.h
>> +++ b/include/uapi/linux/capability.h
>> @@ -365,8 +365,11 @@ struct vfs_ns_cap_data {
>>
>> #define CAP_AUDIT_READ 37
>>
>> +/* Allow mounting, unmounting filesystems */
>>
>> -#define CAP_LAST_CAP CAP_AUDIT_READ
>> +#define CAP_SYS_MOUNT 38
>> +
>> +#define CAP_LAST_CAP CAP_SYS_MOUNT
>>
>> #define cap_valid(x) ((x) >= 0 && (x) <= CAP_LAST_CAP)
>>
>> diff --git a/security/selinux/include/classmap.h
>b/security/selinux/include/classmap.h
>> index 35ffb29a69cb..a873dce97fd5 100644
>> --- a/security/selinux/include/classmap.h
>> +++ b/security/selinux/include/classmap.h
>> @@ -24,9 +24,9 @@
>> "audit_control", "setfcap"
>>
>> #define COMMON_CAP2_PERMS "mac_override", "mac_admin", "syslog", \
>> - "wake_alarm", "block_suspend", "audit_read"
>> + "wake_alarm", "block_suspend", "audit_read", "sys_mount"
>>
>> -#if CAP_LAST_CAP > CAP_AUDIT_READ
>> +#if CAP_LAST_CAP > CAP_SYS_MOUNT
>> #error New capability defined, please update COMMON_CAP2_PERMS.
>> #endif
>>
Nicolas
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [kernel-hardening] [RFC PATCH 1/2] security, capabilities: Add CAP_SYS_MOUNT
2017-10-21 18:41 ` Nicolas Belouin
@ 2017-10-22 0:54 ` Casey Schaufler
0 siblings, 0 replies; 6+ messages in thread
From: Casey Schaufler @ 2017-10-22 0:54 UTC (permalink / raw)
To: Nicolas Belouin, kernel-hardening, David Howells,
Theodore Ts'o, Andreas Dilger, Alexander Viro, Serge Hallyn,
Paul Moore, Stephen Smalley, Eric Paris, James Morris,
linux-cachefs, linux-kernel, linux-ext4, linux-fsdevel,
linux-security-module, selinux, linux-api
On 10/21/2017 11:41 AM, Nicolas Belouin wrote:
>
> On October 21, 2017 7:31:24 PM GMT+02:00, Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 10/21/2017 6:43 AM, Nicolas Belouin wrote:
>>> With CAP_SYS_ADMIN being bloated and inapropriate for actions such
>>> as mounting/unmounting filesystems, the creation of a new capability
>>> is needed.
>>> CAP_SYS_MOUNT is meant to give a process the ability to call for
>> mount,
>>> umount and umount2 syscalls.
>> This is increased granularity for it's own sake. There is no
>> compelling reason to break out this capability in particular.
> Obviously there is a need to break CAP_SYS_ADMIN in pieces,
No. This is a baseless assumption. Granularity for the sake of
granularity is bad. Data General (a dead company) followed the
fine grained capability path and ended up with 330 capabilities.
Developers can't handle the granularity we already have (hell,
half of them don't know what mode bits are for) and making it
finer will only make it harder for them to make use of the ones
we have.
> to do so, you have to start somewhere, so I chose to begin with this.
>
>> Can you identify existing use cases where you would have
>> CAP_SYS_MOUNT without also having CAP_SYS_ADMIN? I should think
>> that all the work that's gone into unprivileged mounts over
>> the past couple years would make this unnecessary.
> If you look at the udiskd deamon used by most desktop environments, it is launched as root or at least with CAP_SYS_ADMIN. Here, you could use CAP_SYS_MOUNT.
Does this demon do anything else that uses CAP_SYS_ADMIN?
If it has to have that anyway, it's not an argument for breaking
out the mount capability.
> There might also be a use within containers as you don't want to give CAP_SYS_ADMIN to a container if it just need to mount/unmount filesystems.
There is massive work going on elsewhere to allow containers to
mount without privilege. And I'm not at all interested in "might".
> If you go even further, it could be used to allow swapon/swapoff (maybe in future patch set).
No, you'd be asking for CAP_SWAP for that. Swap control has nothing to do
with mounting filesystems.
>
>>> Signed-off-by: Nicolas Belouin <nicolas@belouin.fr>
>>> ---
>>> include/uapi/linux/capability.h | 5 ++++-
>>> security/selinux/include/classmap.h | 4 ++--
>>> 2 files changed, 6 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/include/uapi/linux/capability.h
>> b/include/uapi/linux/capability.h
>>> index 230e05d35191..ce230aa6d928 100644
>>> --- a/include/uapi/linux/capability.h
>>> +++ b/include/uapi/linux/capability.h
>>> @@ -365,8 +365,11 @@ struct vfs_ns_cap_data {
>>>
>>> #define CAP_AUDIT_READ 37
>>>
>>> +/* Allow mounting, unmounting filesystems */
>>>
>>> -#define CAP_LAST_CAP CAP_AUDIT_READ
>>> +#define CAP_SYS_MOUNT 38
>>> +
>>> +#define CAP_LAST_CAP CAP_SYS_MOUNT
>>>
>>> #define cap_valid(x) ((x) >= 0 && (x) <= CAP_LAST_CAP)
>>>
>>> diff --git a/security/selinux/include/classmap.h
>> b/security/selinux/include/classmap.h
>>> index 35ffb29a69cb..a873dce97fd5 100644
>>> --- a/security/selinux/include/classmap.h
>>> +++ b/security/selinux/include/classmap.h
>>> @@ -24,9 +24,9 @@
>>> "audit_control", "setfcap"
>>>
>>> #define COMMON_CAP2_PERMS "mac_override", "mac_admin", "syslog", \
>>> - "wake_alarm", "block_suspend", "audit_read"
>>> + "wake_alarm", "block_suspend", "audit_read", "sys_mount"
>>>
>>> -#if CAP_LAST_CAP > CAP_AUDIT_READ
>>> +#if CAP_LAST_CAP > CAP_SYS_MOUNT
>>> #error New capability defined, please update COMMON_CAP2_PERMS.
>>> #endif
>>>
> Nicolas
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 1/2] security, capabilities: Add CAP_SYS_MOUNT
2017-10-21 13:43 [RFC PATCH 1/2] security, capabilities: Add CAP_SYS_MOUNT Nicolas Belouin
2017-10-21 13:43 ` [RFC PATCH 2/2] fs: add the possibility to use CAP_SYS_MOUNT to (u)mount a fs Nicolas Belouin
2017-10-21 17:31 ` [kernel-hardening] [RFC PATCH 1/2] security, capabilities: Add CAP_SYS_MOUNT Casey Schaufler
@ 2017-10-23 12:57 ` Stephen Smalley
2 siblings, 0 replies; 6+ messages in thread
From: Stephen Smalley @ 2017-10-23 12:57 UTC (permalink / raw)
To: Nicolas Belouin, David Howells, Theodore Ts'o, Andreas Dilger,
Alexander Viro, Serge Hallyn, Paul Moore, Eric Paris,
James Morris, linux-cachefs, linux-kernel, linux-ext4,
linux-fsdevel, linux-security-module, selinux, linux-api,
kernel-hardening
On Sat, 2017-10-21 at 15:43 +0200, Nicolas Belouin wrote:
> With CAP_SYS_ADMIN being bloated and inapropriate for actions such
> as mounting/unmounting filesystems, the creation of a new capability
> is needed.
> CAP_SYS_MOUNT is meant to give a process the ability to call for
> mount,
> umount and umount2 syscalls.
If adding a new capability isn't deemed acceptable, then another option
would be to introduce LSM hooks where there isn't already coverage and
implement finer-grained permission checks there. In some cases, that
already occurs for mount and umount*. That also offers the possibility
of taking the object of the operation into account, unlike capabilities
which are only subject/process-based.
>
> Signed-off-by: Nicolas Belouin <nicolas@belouin.fr>
> ---
> include/uapi/linux/capability.h | 5 ++++-
> security/selinux/include/classmap.h | 4 ++--
> 2 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/include/uapi/linux/capability.h
> b/include/uapi/linux/capability.h
> index 230e05d35191..ce230aa6d928 100644
> --- a/include/uapi/linux/capability.h
> +++ b/include/uapi/linux/capability.h
> @@ -365,8 +365,11 @@ struct vfs_ns_cap_data {
>
> #define CAP_AUDIT_READ 37
>
> +/* Allow mounting, unmounting filesystems */
>
> -#define CAP_LAST_CAP CAP_AUDIT_READ
> +#define CAP_SYS_MOUNT 38
> +
> +#define CAP_LAST_CAP CAP_SYS_MOUNT
>
> #define cap_valid(x) ((x) >= 0 && (x) <= CAP_LAST_CAP)
>
> diff --git a/security/selinux/include/classmap.h
> b/security/selinux/include/classmap.h
> index 35ffb29a69cb..a873dce97fd5 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -24,9 +24,9 @@
> "audit_control", "setfcap"
>
> #define COMMON_CAP2_PERMS "mac_override", "mac_admin", "syslog", \
> - "wake_alarm", "block_suspend", "audit_read"
> + "wake_alarm", "block_suspend", "audit_read",
> "sys_mount"
>
> -#if CAP_LAST_CAP > CAP_AUDIT_READ
> +#if CAP_LAST_CAP > CAP_SYS_MOUNT
> #error New capability defined, please update COMMON_CAP2_PERMS.
> #endif
>
^ permalink raw reply [flat|nested] 6+ messages in thread