* Re: [PATCH v1 4/5] LSM: Define SELinux function to measure security state
From: Stephen Smalley @ 2020-07-15 18:04 UTC (permalink / raw)
To: Lakshmi Ramasubramanian
Cc: Mimi Zohar, Casey Schaufler, James Morris, linux-integrity,
SElinux list, LSM List, linux-kernel
In-Reply-To: <20200715154853.23374-5-nramas@linux.microsoft.com>
On Wed, Jul 15, 2020 at 11:48 AM Lakshmi Ramasubramanian
<nramas@linux.microsoft.com> wrote:
>
> SELinux configuration and policy are some of the critical data for this
> security module that needs to be measured. To enable this measurement
> SELinux needs to implement the interface function, security_state(), that
> the LSM can call.
>
> Define the security_state() function in SELinux to measure SELinux
> configuration and policy. Call this function to measure SELinux data
> when there is a change in the security module's state.
>
> Sample measurement of SELinux state and hash of the policy:
>
> 10 e32e...5ac3 ima-buf sha256:86e8...4594 selinux-state 656e61626c65643d313b656e666f7263696e673d303b636865636b72657170726f743d313b6e6574706565723d313b6f70656e7065726d3d313b657874736f636b636c6173733d313b616c776179736e6574776f726b3d303b6367726f75707365636c6162656c3d313b6e6e706e6f737569647472616e736974696f6e3d313b67656e66737365636c6162656c73796d6c696e6b3d303b
> 10 f4a7...9408 ima-buf sha256:4941...68fc selinux-policy-hash 8d1d...1834
>
> The data for selinux-state in the above measurement is:
> enabled=1;enforcing=0;checkreqprot=1;netpeer=1;openperm=1;extsockclass=1;alwaysnetwork=0;cgroupseclabel=1;nnpnosuidtransition=1;genfsseclabelsymlink=0;
>
> The data for selinux-policy-hash in the above measurement is
> the SHA256 hash of the SELinux policy.
>
> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> Suggested-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> ---
> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> index b0e02cfe3ce1..691c7e35f82a 100644
> --- a/security/selinux/include/security.h
> +++ b/security/selinux/include/security.h
> @@ -222,16 +222,35 @@ static inline bool selinux_policycap_genfs_seclabel_symlinks(void)
> return state->policycap[POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS];
> }
>
> +static inline bool selinux_checkreqprot(void)
> +{
> + struct selinux_state *state = &selinux_state;
> +
> + return state->checkreqprot;
> +}
Probably should use READ_ONCE().
> diff --git a/security/selinux/measure.c b/security/selinux/measure.c
> new file mode 100644
> index 000000000000..b909e8e61542
> --- /dev/null
> +++ b/security/selinux/measure.c
> @@ -0,0 +1,122 @@
> +int selinux_security_state(void)
Let's call this selinux_measure_state() or similar. Needs a verb. And
pass it a struct selinux_state * pointer argument to be measured, even
though initially it will always be passed &selinux_state. The
encapsulation of selinux state within selinux_state was to support
multiple selinux namespaces in the future, each with their own state.
> +{
> + int rc = 0;
> + int count;
> + size_t buflen;
> + int policy_hash_len;
> + char *state = NULL;
> + void *policy = NULL;
> + void *policy_hash = NULL;
> + static char *security_state_string =
> + "enabled=%d;enforcing=%d;checkreqprot=%d;" \
> + "netpeer=%d;openperm=%d;extsockclass=%d;" \
> + "alwaysnetwork=%d;cgroupseclabel=%d;" \
> + "nnpnosuidtransition=%d;genfsseclabelsymlink=%d;";
Rather than hardcoding policy capability names here, I would recommend
using the selinux_policycap_names[] array for the names and the
selinux_state.policycap[] array for the values. Also recommend
passing in a struct selinux_state * here to allow for future case
where there are multiple selinux states, one per selinux namespace.
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index ef0afd878bfc..0c289d13ef6a 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -3724,10 +3724,11 @@ int security_netlbl_sid_to_secattr(struct selinux_state *state,
> * security_read_policy - read the policy.
> * @data: binary policy data
> * @len: length of data in bytes
> - *
> + * @alloc_kernel_memory: flag to indicate memory allocation
> */
> -int security_read_policy(struct selinux_state *state,
> - void **data, size_t *len)
> +int security_read_selinux_policy(struct selinux_state *state,
> + void **data, size_t *len,
> + bool alloc_kernel_memory)
Instead of passing in a boolean to control how the memory is
allocated, split this into a helper function that takes an
already-allocated buffer and two
different front-end wrappers, one for kernel-internal use and one for
userspace use.
> @@ -3738,7 +3739,10 @@ int security_read_policy(struct selinux_state *state,
>
> *len = security_policydb_len(state);
>
> - *data = vmalloc_user(*len);
> + if (alloc_kernel_memory)
> + *data = kzalloc(*len, GFP_KERNEL);
You need vmalloc() since policy image size may exceed kmalloc max (or
at least that used to be the case).
^ permalink raw reply
* Re: [PATCH 7/7] exec: Implement kernel_execve
From: Christoph Hellwig @ 2020-07-15 18:20 UTC (permalink / raw)
To: Kees Cook
Cc: Christoph Hellwig, Eric W. Biederman, linux-kernel,
Linus Torvalds, Andy Lutomirski, H. Peter Anvin, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Al Viro, Luis Chamberlain,
linux-fsdevel, Tetsuo Handa, linux-security-module,
Serge E. Hallyn, James Morris, Kentaro Takeda, Casey Schaufler,
John Johansen
In-Reply-To: <202007150758.3D1597C6D@keescook>
On Wed, Jul 15, 2020 at 08:00:16AM -0700, Kees Cook wrote:
> Heh, yes please. :) (Which branch is this from?
http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/exec-cleanup
> Are yours and Eric's
> tree going to collide?)
Yes, badly.
^ permalink raw reply
* Re: [PATCH 7/7] exec: Implement kernel_execve
From: Eric W. Biederman @ 2020-07-15 18:23 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-kernel, Linus Torvalds, Kees Cook, Andy Lutomirski,
H. Peter Anvin, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Al Viro, Luis Chamberlain, linux-fsdevel, Tetsuo Handa,
linux-security-module, Serge E. Hallyn, James Morris,
Kentaro Takeda, Casey Schaufler, John Johansen
In-Reply-To: <20200715064220.GG32470@infradead.org>
Christoph Hellwig <hch@infradead.org> writes:
>> +static int count_strings_kernel(const char *const *argv)
>> +{
>> + int i;
>> +
>> + if (!argv)
>> + return 0;
>> +
>> + for (i = 0; argv[i]; ++i) {
>> + if (i >= MAX_ARG_STRINGS)
>> + return -E2BIG;
>> + if (fatal_signal_pending(current))
>> + return -ERESTARTNOHAND;
>> + cond_resched();
>
> I don't think we need a fatal_signal_pending and cond_resched() is
> needed in each step given that we don't actually do anything.
If we have a MAX_ARG_STRINGS sized argv passed in, that is 2^31
iterations of the loop. A processor at 2Ghz performs roughly 2^31
cycles per second. So this loop has the potential to run for an entire
second. That is long enough to need fatal_signal_pending() and
cond_resched checks.
In practice I don't think we have any argv arrays anywhere near that big
passed in from the kernel. However removing the logic that accounts for
long running loops is best handled as a separate change so that people
will analyze the patch based on that criterian, and so that in the
highly unlikely even something goes wrong people have a nice commit
to bisect things to.
Eric
^ permalink raw reply
* Re: [PATCH v1 4/5] LSM: Define SELinux function to measure security state
From: Lakshmi Ramasubramanian @ 2020-07-15 18:34 UTC (permalink / raw)
To: Stephen Smalley
Cc: Mimi Zohar, Casey Schaufler, James Morris, linux-integrity,
SElinux list, LSM List, linux-kernel
In-Reply-To: <CAEjxPJ6UsK9QqFTpMKjgSin2Q6D-5NCNDS0enuRNuixVP9H2wQ@mail.gmail.com>
On 7/15/20 11:04 AM, Stephen Smalley wrote:
>> +static inline bool selinux_checkreqprot(void)
>> +{
>> + struct selinux_state *state = &selinux_state;
>> +
>> + return state->checkreqprot;
>> +}
>
> Probably should use READ_ONCE().
Will do.
>
>> diff --git a/security/selinux/measure.c b/security/selinux/measure.c
>> new file mode 100644
>> index 000000000000..b909e8e61542
>> --- /dev/null
>> +++ b/security/selinux/measure.c
>> @@ -0,0 +1,122 @@
>> +int selinux_security_state(void)
>
> Let's call this selinux_measure_state() or similar. Needs a verb. And
> pass it a struct selinux_state * pointer argument to be measured, even
> though initially it will always be passed &selinux_state. The
> encapsulation of selinux state within selinux_state was to support
> multiple selinux namespaces in the future, each with their own state.
Will do.
>> + static char *security_state_string =
>> + "enabled=%d;enforcing=%d;checkreqprot=%d;" \
>> + "netpeer=%d;openperm=%d;extsockclass=%d;" \
>> + "alwaysnetwork=%d;cgroupseclabel=%d;" \
>> + "nnpnosuidtransition=%d;genfsseclabelsymlink=%d;";
>
> Rather than hardcoding policy capability names here, I would recommend
> using the selinux_policycap_names[] array for the names and the
> selinux_state.policycap[] array for the values. Also recommend
> passing in a struct selinux_state * here to allow for future case
> where there are multiple selinux states, one per selinux namespace.
Will do.
>
>> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
>> index ef0afd878bfc..0c289d13ef6a 100644
>> --- a/security/selinux/ss/services.c
>> +++ b/security/selinux/ss/services.c
>> @@ -3724,10 +3724,11 @@ int security_netlbl_sid_to_secattr(struct selinux_state *state,
>> * security_read_policy - read the policy.
>> * @data: binary policy data
>> * @len: length of data in bytes
>> - *
>> + * @alloc_kernel_memory: flag to indicate memory allocation
>> */
>> -int security_read_policy(struct selinux_state *state,
>> - void **data, size_t *len)
>> +int security_read_selinux_policy(struct selinux_state *state,
>> + void **data, size_t *len,
>> + bool alloc_kernel_memory)
>
> Instead of passing in a boolean to control how the memory is
> allocated, split this into a helper function that takes an
> already-allocated buffer and two
> different front-end wrappers, one for kernel-internal use and one for
> userspace use.
Will do.
>
>> @@ -3738,7 +3739,10 @@ int security_read_policy(struct selinux_state *state,
>>
>> *len = security_policydb_len(state);
>>
>> - *data = vmalloc_user(*len);
>> + if (alloc_kernel_memory)
>> + *data = kzalloc(*len, GFP_KERNEL);
>
> You need vmalloc() since policy image size may exceed kmalloc max (or
> at least that used to be the case).
Will do.
thanks,
-lakshmi
^ permalink raw reply
* Re: [RFC PATCH v3 03/12] security: add ipe lsm policy parser and policy loading
From: Tyler Hicks @ 2020-07-15 19:16 UTC (permalink / raw)
To: deven.desai
Cc: agk, axboe, snitzer, jmorris, serge, zohar, linux-integrity,
linux-security-module, dm-devel, linux-block, jannh,
pasha.tatashin, sashal, jaskarankhurana, nramas, mdsakib,
linux-kernel, corbet
In-Reply-To: <20200415162550.2324-4-deven.desai@linux.microsoft.com>
On 2020-04-15 09:25:41, deven.desai@linux.microsoft.com wrote:
> From: Deven Bowers <deven.desai@linux.microsoft.com>
>
> Adds the policy parser and the policy loading to IPE, along with the
> related sysfs, securityfs entries, and audit events.
>
> Signed-off-by: Deven Bowers <deven.desai@linux.microsoft.com>
> ---
...
> diff --git a/security/ipe/ipe-sysfs.c b/security/ipe/ipe-sysfs.c
> index 1c65185c531d..a250da29c3b5 100644
> --- a/security/ipe/ipe-sysfs.c
> +++ b/security/ipe/ipe-sysfs.c
> @@ -5,6 +5,7 @@
>
> #include "ipe.h"
> #include "ipe-audit.h"
> +#include "ipe-secfs.h"
>
> #include <linux/sysctl.h>
> #include <linux/fs.h>
> @@ -45,6 +46,79 @@ static int ipe_switch_mode(struct ctl_table *table, int write,
>
> #endif /* CONFIG_SECURITY_IPE_PERMISSIVE_SWITCH */
>
> +#ifdef CONFIG_SECURITYFS
> +
> +/**
> + * ipe_switch_active_policy: Handler to switch the policy IPE is enforcing.
> + * @table: Sysctl table entry from the variable, sysctl_table.
> + * @write: Integer indicating whether this is a write or a read.
> + * @buffer: Data passed to sysctl. This is the policy id to activate,
> + * for this function.
> + * @lenp: Pointer to the size of @buffer.
> + * @ppos: Offset into @buffer.
> + *
> + * This wraps proc_dointvec_minmax, and if there's a change, emits an
> + * audit event.
> + *
> + * Return:
> + * 0 - OK
> + * -ENOMEM - Out of memory
> + * -ENOENT - Policy identified by @id does not exist
> + * Other - See proc_dostring and retrieve_backed_dentry
> + */
> +static int ipe_switch_active_policy(struct ctl_table *table, int write,
> + void __user *buffer, size_t *lenp,
> + loff_t *ppos)
> +{
> + int rc = 0;
> + char *id = NULL;
> + size_t size = 0;
> +
> + if (write) {
I see that the policy files in securityfs, such as new_policy, are
checking for CAP_MAC_ADMIN but there's no check here for CAP_MAC_ADMIN
when switching the active policy. I think we should enforce that cap
here, too.
Thinking about it some more, I find it a little odd that we're spreading
the files necessary to update a policy across both procfs (sysctl) and
securityfs. It looks like procfs will have different semantics than
securityfs after looking at proc_sys_permission(). I suggest moving
strict_parse and active_policy under securityfs for a unified experience
and common location when updating policy.
Tyler
> + id = kzalloc((*lenp) + 1, GFP_KERNEL);
> + if (!id)
> + return -ENOMEM;
> +
> + table->data = id;
> + table->maxlen = (*lenp) + 1;
> +
> + rc = proc_dostring(table, write, buffer, lenp, ppos);
> + if (rc != 0)
> + goto out;
> +
> + rc = ipe_set_active_policy(id, strlen(id));
> + } else {
> + if (!rcu_access_pointer(ipe_active_policy)) {
> + table->data = "";
> + table->maxlen = 1;
> + return proc_dostring(table, write, buffer, lenp, ppos);
> + }
> +
> + rcu_read_lock();
> + size = strlen(rcu_dereference(ipe_active_policy)->policy_name);
> + rcu_read_unlock();
> +
> + id = kzalloc(size + 1, GFP_KERNEL);
> + if (!id)
> + return -ENOMEM;
> +
> + rcu_read_lock();
> + strncpy(id, rcu_dereference(ipe_active_policy)->policy_name,
> + size);
> + rcu_read_unlock();
> +
> + table->data = id;
> + table->maxlen = size;
> +
> + rc = proc_dostring(table, write, buffer, lenp, ppos);
> + }
> +out:
> + kfree(id);
> + return rc;
> +}
> +
> +#endif /* CONFIG_SECURITYFS */
> +
> static struct ctl_table_header *sysctl_header;
>
> static const struct ctl_path sysctl_path[] = {
> @@ -75,6 +149,24 @@ static struct ctl_table sysctl_table[] = {
> .extra1 = SYSCTL_ZERO,
> .extra2 = SYSCTL_ONE,
> },
> +#ifdef CONFIG_SECURITYFS
> + {
> + .procname = "strict_parse",
> + .data = &ipe_strict_parse,
> + .maxlen = sizeof(int),
> + .mode = 0644,
> + .proc_handler = proc_dointvec_minmax,
> + .extra1 = SYSCTL_ZERO,
> + .extra2 = SYSCTL_ONE,
> + },
> + {
> + .procname = "active_policy",
> + .data = NULL,
> + .maxlen = 0,
> + .mode = 0644,
> + .proc_handler = ipe_switch_active_policy,
> + },
> +#endif /* CONFIG_SECURITYFS */
> {}
> };
>
> diff --git a/security/ipe/ipe.c b/security/ipe/ipe.c
> index b6553e370f98..07f855ffb79a 100644
> --- a/security/ipe/ipe.c
> +++ b/security/ipe/ipe.c
> @@ -6,6 +6,7 @@
> #include "ipe.h"
> #include "ipe-policy.h"
> #include "ipe-hooks.h"
> +#include "ipe-secfs.h"
> #include "ipe-sysfs.h"
>
> #include <linux/module.h>
> @@ -60,3 +61,4 @@ DEFINE_LSM(ipe) = {
>
> int ipe_enforce = 1;
> int ipe_success_audit;
> +int ipe_strict_parse;
> diff --git a/security/ipe/ipe.h b/security/ipe/ipe.h
> index 6a47f55b05d9..bf6cf7744b0e 100644
> --- a/security/ipe/ipe.h
> +++ b/security/ipe/ipe.h
> @@ -16,5 +16,6 @@
>
> extern int ipe_enforce;
> extern int ipe_success_audit;
> +extern int ipe_strict_parse;
>
> #endif /* IPE_H */
> --
> 2.26.0
^ permalink raw reply
* Re: [PATCH v6 4/7] fs: Introduce O_MAYEXEC flag for openat2(2)
From: Kees Cook @ 2020-07-15 20:06 UTC (permalink / raw)
To: Mickaël Salaün
Cc: linux-kernel, Aleksa Sarai, Alexei Starovoitov, Al Viro,
Andrew Morton, Andy Lutomirski, Christian Brauner,
Christian Heimes, Daniel Borkmann, Deven Bowers, Dmitry Vyukov,
Eric Biggers, Eric Chiang, Florian Weimer, James Morris, Jan Kara,
Jann Horn, Jonathan Corbet, Lakshmi Ramasubramanian,
Matthew Garrett, Matthew Wilcox, Michael Kerrisk,
Mickaël Salaün, Mimi Zohar, Philippe Trébuchet,
Scott Shell, Sean Christopherson, Shuah Khan, Steve Dower,
Steve Grubb, Tetsuo Handa, Thibaut Sautereau, Vincent Strubel,
kernel-hardening, linux-api, linux-integrity,
linux-security-module, linux-fsdevel
In-Reply-To: <20200714181638.45751-5-mic@digikod.net>
On Tue, Jul 14, 2020 at 08:16:35PM +0200, Mickaël Salaün wrote:
> When the O_MAYEXEC flag is passed, openat2(2) may be subject to
> additional restrictions depending on a security policy managed by the
> kernel through a sysctl or implemented by an LSM thanks to the
> inode_permission hook. This new flag is ignored by open(2) and
> openat(2) because of their unspecified flags handling.
>
> The underlying idea is to be able to restrict scripts interpretation
> according to a policy defined by the system administrator. For this to
> be possible, script interpreters must use the O_MAYEXEC flag
> appropriately. To be fully effective, these interpreters also need to
> handle the other ways to execute code: command line parameters (e.g.,
> option -e for Perl), module loading (e.g., option -m for Python), stdin,
> file sourcing, environment variables, configuration files, etc.
> According to the threat model, it may be acceptable to allow some script
> interpreters (e.g. Bash) to interpret commands from stdin, may it be a
> TTY or a pipe, because it may not be enough to (directly) perform
> syscalls. Further documentation can be found in a following patch.
>
> Even without enforced security policy, userland interpreters can set it
> to enforce the system policy at their level, knowing that it will not
> break anything on running systems which do not care about this feature.
> However, on systems which want this feature enforced, there will be
> knowledgeable people (i.e. sysadmins who enforced O_MAYEXEC
> deliberately) to manage it. A simple security policy implementation,
> configured through a dedicated sysctl, is available in a following
> patch.
>
> O_MAYEXEC should not be confused with the O_EXEC flag which is intended
> for execute-only, which obviously doesn't work for scripts. However, a
> similar behavior could be implemented in userland with O_PATH:
> https://lore.kernel.org/lkml/1e2f6913-42f2-3578-28ed-567f6a4bdda1@digikod.net/
>
> The implementation of O_MAYEXEC almost duplicates what execve(2) and
> uselib(2) are already doing: setting MAY_OPENEXEC in acc_mode (which can
> then be checked as MAY_EXEC, if enforced), and propagating FMODE_EXEC to
> _fmode via __FMODE_EXEC flag (which can then trigger a
> fanotify/FAN_OPEN_EXEC event).
>
> This is an updated subset of the patch initially written by Vincent
> Strubel for CLIP OS 4:
> https://github.com/clipos-archive/src_platform_clip-patches/blob/f5cb330d6b684752e403b4e41b39f7004d88e561/1901_open_mayexec.patch
> This patch has been used for more than 12 years with customized script
> interpreters. Some examples (with the original name O_MAYEXEC) can be
> found here:
> https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYEXEC
>
> Co-developed-by: Vincent Strubel <vincent.strubel@ssi.gouv.fr>
> Signed-off-by: Vincent Strubel <vincent.strubel@ssi.gouv.fr>
> Co-developed-by: Thibaut Sautereau <thibaut.sautereau@ssi.gouv.fr>
> Signed-off-by: Thibaut Sautereau <thibaut.sautereau@ssi.gouv.fr>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Reviewed-by: Deven Bowers <deven.desai@linux.microsoft.com>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Cc: Aleksa Sarai <cyphar@cyphar.com>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> ---
>
> Changes since v5:
> * Update commit message.
>
> Changes since v3:
> * Switch back to O_MAYEXEC, but only handle it with openat2(2) which
> checks unknown flags (suggested by Aleksa Sarai). Cf.
> https://lore.kernel.org/lkml/20200430015429.wuob7m5ofdewubui@yavin.dot.cyphar.com/
>
> Changes since v2:
> * Replace O_MAYEXEC with RESOLVE_MAYEXEC from openat2(2). This change
> enables to not break existing application using bogus O_* flags that
> may be ignored by current kernels by using a new dedicated flag, only
> usable through openat2(2) (suggested by Jeff Layton). Using this flag
> will results in an error if the running kernel does not support it.
> User space needs to manage this case, as with other RESOLVE_* flags.
> The best effort approach to security (for most common distros) will
> simply consists of ignoring such an error and retry without
> RESOLVE_MAYEXEC. However, a fully controlled system may which to
> error out if such an inconsistency is detected.
>
> Changes since v1:
> * Set __FMODE_EXEC when using O_MAYEXEC to make this information
> available through the new fanotify/FAN_OPEN_EXEC event (suggested by
> Jan Kara and Matthew Bobrowski):
> https://lore.kernel.org/lkml/20181213094658.GA996@lithium.mbobrowski.org/
> ---
> fs/fcntl.c | 2 +-
> fs/open.c | 8 ++++++++
> include/linux/fcntl.h | 2 +-
> include/linux/fs.h | 2 ++
> include/uapi/asm-generic/fcntl.h | 7 +++++++
> 5 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/fs/fcntl.c b/fs/fcntl.c
> index 2e4c0fa2074b..0357ad667563 100644
> --- a/fs/fcntl.c
> +++ b/fs/fcntl.c
> @@ -1033,7 +1033,7 @@ static int __init fcntl_init(void)
> * Exceptions: O_NONBLOCK is a two bit define on parisc; O_NDELAY
> * is defined as O_NONBLOCK on some platforms and not on others.
> */
> - BUILD_BUG_ON(21 - 1 /* for O_RDONLY being 0 */ !=
> + BUILD_BUG_ON(22 - 1 /* for O_RDONLY being 0 */ !=
> HWEIGHT32(
> (VALID_OPEN_FLAGS & ~(O_NONBLOCK | O_NDELAY)) |
> __FMODE_EXEC | __FMODE_NONOTIFY));
> diff --git a/fs/open.c b/fs/open.c
> index 623b7506a6db..38e434bdbbb6 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -987,6 +987,8 @@ inline struct open_how build_open_how(int flags, umode_t mode)
> .mode = mode & S_IALLUGO,
> };
>
> + /* O_MAYEXEC is ignored by syscalls relying on build_open_how(). */
> + how.flags &= ~O_MAYEXEC;
> /* O_PATH beats everything else. */
> if (how.flags & O_PATH)
> how.flags &= O_PATH_FLAGS;
> @@ -1054,6 +1056,12 @@ inline int build_open_flags(const struct open_how *how, struct open_flags *op)
> if (flags & __O_SYNC)
> flags |= O_DSYNC;
>
> + /* Checks execution permissions on open. */
> + if (flags & O_MAYEXEC) {
> + acc_mode |= MAY_OPENEXEC;
> + flags |= __FMODE_EXEC;
> + }
Adding __FMODE_EXEC here will immediately change the behaviors of NFS
and fsnotify. If that's going to happen, I think it needs to be under
the control of the later patches doing the behavioral controls.
(specifically, NFS looks like it completely changes its access control
test when this is set and ignores the read/write checks entirely, which
is not what's wanted).
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v6 5/7] fs,doc: Enable to enforce noexec mounts or file exec through O_MAYEXEC
From: Kees Cook @ 2020-07-15 20:37 UTC (permalink / raw)
To: Mickaël Salaün
Cc: linux-kernel, Aleksa Sarai, Alexei Starovoitov, Al Viro,
Andrew Morton, Andy Lutomirski, Christian Brauner,
Christian Heimes, Daniel Borkmann, Deven Bowers, Dmitry Vyukov,
Eric Biggers, Eric Chiang, Florian Weimer, James Morris, Jan Kara,
Jann Horn, Jonathan Corbet, Lakshmi Ramasubramanian,
Matthew Garrett, Matthew Wilcox, Michael Kerrisk,
Mickaël Salaün, Mimi Zohar, Philippe Trébuchet,
Scott Shell, Sean Christopherson, Shuah Khan, Steve Dower,
Steve Grubb, Tetsuo Handa, Thibaut Sautereau, Vincent Strubel,
kernel-hardening, linux-api, linux-integrity,
linux-security-module, linux-fsdevel
In-Reply-To: <20200714181638.45751-6-mic@digikod.net>
On Tue, Jul 14, 2020 at 08:16:36PM +0200, Mickaël Salaün wrote:
> @@ -2849,7 +2855,7 @@ static int may_open(const struct path *path, int acc_mode, int flag)
> case S_IFLNK:
> return -ELOOP;
> case S_IFDIR:
> - if (acc_mode & (MAY_WRITE | MAY_EXEC))
> + if (acc_mode & (MAY_WRITE | MAY_EXEC | MAY_OPENEXEC))
> return -EISDIR;
> break;
(I need to figure out where "open for reading" rejects S_IFDIR, since
it's clearly not here...)
> case S_IFBLK:
> @@ -2859,13 +2865,26 @@ static int may_open(const struct path *path, int acc_mode, int flag)
> fallthrough;
> case S_IFIFO:
> case S_IFSOCK:
> - if (acc_mode & MAY_EXEC)
> + if (acc_mode & (MAY_EXEC | MAY_OPENEXEC))
> return -EACCES;
> flag &= ~O_TRUNC;
> break;
This will immediately break a system that runs code with MAY_OPENEXEC
set but reads from a block, char, fifo, or socket, even in the case of
a sysadmin leaving the "file" sysctl disabled.
> case S_IFREG:
> - if ((acc_mode & MAY_EXEC) && path_noexec(path))
> - return -EACCES;
> + if (path_noexec(path)) {
> + if (acc_mode & MAY_EXEC)
> + return -EACCES;
> + if ((acc_mode & MAY_OPENEXEC) &&
> + (sysctl_open_mayexec_enforce & OPEN_MAYEXEC_ENFORCE_MOUNT))
> + return -EACCES;
> + }
> + if ((acc_mode & MAY_OPENEXEC) &&
> + (sysctl_open_mayexec_enforce & OPEN_MAYEXEC_ENFORCE_FILE))
> + /*
> + * Because acc_mode may change here, the next and only
> + * use of acc_mode should then be by the following call
> + * to inode_permission().
> + */
> + acc_mode |= MAY_EXEC;
> break;
> }
Likely very minor, but I'd like to avoid the path_noexec() call in the
fast-path (it dereferences a couple pointers where as doing bit tests on
acc_mode is fast).
Given that and the above observations, I think that may_open() likely
needs to start with:
if (acc_mode & MAY_OPENEXEC) {
/* Reject all file types when mount enforcement set. */
if ((sysctl_open_mayexec_enforce & OPEN_MAYEXEC_ENFORCE_MOUNT) &&
path_noexec(path))
return -EACCES;
/* Treat the same as MAY_EXEC. */
if (sysctl_open_mayexec_enforce & OPEN_MAYEXEC_ENFORCE_FILE))
acc_mode |= MAY_EXEC;
}
(Though I'm not 100% sure that path_noexec() is safe to be called for
all file types: i.e. path->mnt and path->-mnt->mnt_sb *always* non-NULL?)
This change would also imply that OPEN_MAYEXEC_ENFORCE_FILE *includes*
OPEN_MAYEXEC_ENFORCE_MOUNT (i.e. the sysctl should not be a bitfield),
since path_noexec() would get checked for S_ISREG. I can't come up with
a rationale where one would want OPEN_MAYEXEC_ENFORCE_FILE but _not_
OPEN_MAYEXEC_ENFORCE_MOUNT?
(I can absolutely see wanting only OPEN_MAYEXEC_ENFORCE_MOUNT, or
suddenly one has to go mark every loaded thing with the exec bit and
most distros haven't done this to, for example, shared libraries. But
setting the exec bit and then NOT wanting to enforce the mount check
seems... not sensible?)
Outside of this change, yes, I like this now -- it's much cleaner
because we have all the checks in the same place where they belong. :)
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index db1ce7af2563..5008a2566e79 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -113,6 +113,7 @@ static int sixty = 60;
>
> static int __maybe_unused neg_one = -1;
> static int __maybe_unused two = 2;
> +static int __maybe_unused three = 3;
> static int __maybe_unused four = 4;
> static unsigned long zero_ul;
> static unsigned long one_ul = 1;
Oh, are these still here? I thought they got removed (or at least made
const). Where did that series go? Hmpf, see sysctl_vals, but yes, for
now, this is fine.
> @@ -888,7 +889,6 @@ static int proc_taint(struct ctl_table *table, int write,
> return err;
> }
>
> -#ifdef CONFIG_PRINTK
> static int proc_dointvec_minmax_sysadmin(struct ctl_table *table, int write,
> void *buffer, size_t *lenp, loff_t *ppos)
> {
> @@ -897,7 +897,6 @@ static int proc_dointvec_minmax_sysadmin(struct ctl_table *table, int write,
>
> return proc_dointvec_minmax(table, write, buffer, lenp, ppos);
> }
> -#endif
>
> /**
> * struct do_proc_dointvec_minmax_conv_param - proc_dointvec_minmax() range checking structure
> @@ -3264,6 +3263,15 @@ static struct ctl_table fs_table[] = {
> .extra1 = SYSCTL_ZERO,
> .extra2 = &two,
> },
> + {
> + .procname = "open_mayexec_enforce",
> + .data = &sysctl_open_mayexec_enforce,
> + .maxlen = sizeof(int),
> + .mode = 0600,
> + .proc_handler = proc_dointvec_minmax_sysadmin,
> + .extra1 = SYSCTL_ZERO,
> + .extra2 = &three,
> + },
> #if defined(CONFIG_BINFMT_MISC) || defined(CONFIG_BINFMT_MISC_MODULE)
> {
> .procname = "binfmt_misc",
> --
> 2.27.0
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v6 6/7] selftest/openat2: Add tests for O_MAYEXEC enforcing
From: Kees Cook @ 2020-07-15 20:38 UTC (permalink / raw)
To: Mickaël Salaün
Cc: linux-kernel, Aleksa Sarai, Alexei Starovoitov, Al Viro,
Andrew Morton, Andy Lutomirski, Christian Brauner,
Christian Heimes, Daniel Borkmann, Deven Bowers, Dmitry Vyukov,
Eric Biggers, Eric Chiang, Florian Weimer, James Morris, Jan Kara,
Jann Horn, Jonathan Corbet, Lakshmi Ramasubramanian,
Matthew Garrett, Matthew Wilcox, Michael Kerrisk,
Mickaël Salaün, Mimi Zohar, Philippe Trébuchet,
Scott Shell, Sean Christopherson, Shuah Khan, Steve Dower,
Steve Grubb, Tetsuo Handa, Thibaut Sautereau, Vincent Strubel,
kernel-hardening, linux-api, linux-integrity,
linux-security-module, linux-fsdevel
In-Reply-To: <20200714181638.45751-7-mic@digikod.net>
On Tue, Jul 14, 2020 at 08:16:37PM +0200, Mickaël Salaün wrote:
> Test propagation of noexec mount points or file executability through
> files open with or without O_MAYEXEC, thanks to the
> fs.open_mayexec_enforce sysctl.
>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Reviewed-by: Thibaut Sautereau <thibaut.sautereau@ssi.gouv.fr>
> Cc: Aleksa Sarai <cyphar@cyphar.com>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Shuah Khan <shuah@kernel.org>
Yay variants! :)
Reviewed-by: Kees Cook <keescook@chromium.org>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v6 7/7] ima: add policy support for the new file open MAY_OPENEXEC flag
From: Kees Cook @ 2020-07-15 20:40 UTC (permalink / raw)
To: Mickaël Salaün
Cc: linux-kernel, Aleksa Sarai, Alexei Starovoitov, Al Viro,
Andrew Morton, Andy Lutomirski, Christian Brauner,
Christian Heimes, Daniel Borkmann, Deven Bowers, Dmitry Vyukov,
Eric Biggers, Eric Chiang, Florian Weimer, James Morris, Jan Kara,
Jann Horn, Jonathan Corbet, Lakshmi Ramasubramanian,
Matthew Garrett, Matthew Wilcox, Michael Kerrisk,
Mickaël Salaün, Mimi Zohar, Philippe Trébuchet,
Scott Shell, Sean Christopherson, Shuah Khan, Steve Dower,
Steve Grubb, Tetsuo Handa, Thibaut Sautereau, Vincent Strubel,
kernel-hardening, linux-api, linux-integrity,
linux-security-module, linux-fsdevel
In-Reply-To: <20200714181638.45751-8-mic@digikod.net>
On Tue, Jul 14, 2020 at 08:16:38PM +0200, Mickaël Salaün wrote:
> From: Mimi Zohar <zohar@linux.ibm.com>
>
> The kernel has no way of differentiating between a file containing data
> or code being opened by an interpreter. The proposed O_MAYEXEC
> openat2(2) flag bridges this gap by defining and enabling the
> MAY_OPENEXEC flag.
>
> This patch adds IMA policy support for the new MAY_OPENEXEC flag.
>
> Example:
> measure func=FILE_CHECK mask=^MAY_OPENEXEC
> appraise func=FILE_CHECK appraise_type=imasig mask=^MAY_OPENEXEC
>
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> Reviewed-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> Acked-by: Mickaël Salaün <mic@digikod.net>
(Process nit: if you're sending this on behalf of another author, then
this should be Signed-off-by rather than Acked-by.)
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v5 4/6] proc: allow access in init userns for map_files with CAP_CHECKPOINT_RESTORE
From: Cyrill Gorcunov @ 2020-07-15 21:17 UTC (permalink / raw)
To: Adrian Reber
Cc: Christian Brauner, Eric Biederman, Pavel Emelyanov, Oleg Nesterov,
Dmitry Safonov, Andrei Vagin, Nicolas Viennot,
Michał Cłapiński, Kamil Yurtsever, Dirk Petersen,
Christine Flood, Casey Schaufler, Mike Rapoport,
Radostin Stoyanov, Serge Hallyn, Stephen Smalley, Sargun Dhillon,
Arnd Bergmann, linux-security-module, linux-kernel, selinux,
Eric Paris, Jann Horn, linux-fsdevel
In-Reply-To: <20200715144954.1387760-5-areber@redhat.com>
On Wed, Jul 15, 2020 at 04:49:52PM +0200, Adrian Reber wrote:
> Opening files in /proc/pid/map_files when the current user is
> CAP_CHECKPOINT_RESTORE capable in the root namespace is useful for
> checkpointing and restoring to recover files that are unreachable via
> the file system such as deleted files, or memfd files.
>
> Signed-off-by: Adrian Reber <areber@redhat.com>
> Signed-off-by: Nicolas Viennot <Nicolas.Viennot@twosigma.com>
I still have a plan to make this code been usable without
capabilities requirements but due to lack of spare time
for deep investigation this won't happen anytime soon.
Thus the patch looks OK to me, fwiw
Reviewed-by: Cyrill Gorcunov <gorcunov@gmail.com>
^ permalink raw reply
* Re: [PATCH bpf-next v4 2/4] bpf: Implement bpf_local_storage for inodes
From: Martin KaFai Lau @ 2020-07-15 21:57 UTC (permalink / raw)
To: KP Singh
Cc: linux-kernel, bpf, linux-security-module, Alexei Starovoitov,
Daniel Borkmann, Paul Turner, Jann Horn, Florent Revest
In-Reply-To: <20200709101239.3829793-3-kpsingh@chromium.org>
On Thu, Jul 09, 2020 at 12:12:37PM +0200, KP Singh wrote:
> From: KP Singh <kpsingh@google.com>
>
> Similar to bpf_local_storage for sockets, add local storage for inodes.
> The life-cycle of storage is managed with the life-cycle of the inode.
> i.e. the storage is destroyed along with the owning inode.
>
> The BPF LSM allocates an __rcu pointer to the bpf_local_storage in the
> security blob which are now stackable and can co-exist with other LSMs.
>
> Signed-off-by: KP Singh <kpsingh@google.com>
[ ... ]
> +static void *bpf_inode_storage_lookup_elem(struct bpf_map *map, void *key)
> +{
> + struct bpf_local_storage_data *sdata;
> + struct inode *inode;
> + int err = -EINVAL;
> +
> + if (key) {
> + inode = *(struct inode **)(key);
The bpf_inode_storage_lookup_elem() here and the (update|delete)_elem() below
are called from the userspace syscall. How the userspace may provide this key?
> + sdata = inode_storage_lookup(inode, map, true);
> + return sdata ? sdata->data : NULL;
> + }
> +
> + return ERR_PTR(err);
> +}
> +
> +static int bpf_inode_storage_update_elem(struct bpf_map *map, void *key,
> + void *value, u64 map_flags)
> +{
> + struct bpf_local_storage_data *sdata;
> + struct inode *inode;
> + int err = -EINVAL;
> +
> + if (key) {
> + inode = *(struct inode **)(key);
> + sdata = map->ops->map_local_storage_update(inode, map, value,
> + map_flags);
> + return PTR_ERR_OR_ZERO(sdata);
> + }
> + return err;
> +}
> +
> +static int inode_storage_delete(struct inode *inode, struct bpf_map *map)
> +{
> + struct bpf_local_storage_data *sdata;
> +
> + sdata = inode_storage_lookup(inode, map, false);
> + if (!sdata)
> + return -ENOENT;
> +
> + bpf_selem_unlink_map_elem(SELEM(sdata));
> +
> + return 0;
> +}
> +
> +static int bpf_inode_storage_delete_elem(struct bpf_map *map, void *key)
> +{
> + struct inode *inode;
> + int err = -EINVAL;
> +
> + if (key) {
> + inode = *(struct inode **)(key);
> + err = inode_storage_delete(inode, map);
> + }
> +
> + return err;
> +}
> +
[ ... ]
> +static int inode_storage_map_btf_id;
> +const struct bpf_map_ops inode_storage_map_ops = {
> + .map_alloc_check = bpf_local_storage_map_alloc_check,
> + .map_alloc = inode_storage_map_alloc,
> + .map_free = inode_storage_map_free,
> + .map_get_next_key = notsupp_get_next_key,
> + .map_lookup_elem = bpf_inode_storage_lookup_elem,
> + .map_update_elem = bpf_inode_storage_update_elem,
> + .map_delete_elem = bpf_inode_storage_delete_elem,
> + .map_check_btf = bpf_local_storage_map_check_btf,
> + .map_btf_name = "bpf_local_storage_map",
> + .map_btf_id = &inode_storage_map_btf_id,
> + .map_local_storage_alloc = inode_storage_alloc,
> + .map_selem_alloc = inode_selem_alloc,
> + .map_local_storage_update = inode_storage_update,
> + .map_local_storage_unlink = unlink_inode_storage,
> +};
> +
^ permalink raw reply
* [PATCH] keys: asymmetric: fix error return code in software_key_query()
From: David Howells @ 2020-07-15 22:28 UTC (permalink / raw)
To: torvalds
Cc: Wei Yongjun, dhowells, jarkko.sakkinen, keyrings,
linux-security-module, linux-kernel
From: Wei Yongjun <weiyongjun1@huawei.com>
Fix to return negative error code -ENOMEM from kmalloc() error handling
case instead of 0, as done elsewhere in this function.
Fixes: f1774cb8956a ("X.509: parse public key parameters from x509 for akcipher")
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
Signed-off-by: David Howells <dhowells@redhat.com>
---
crypto/asymmetric_keys/public_key.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
index d7f43d4ea925..e5fae4e838c0 100644
--- a/crypto/asymmetric_keys/public_key.c
+++ b/crypto/asymmetric_keys/public_key.c
@@ -119,6 +119,7 @@ static int software_key_query(const struct kernel_pkey_params *params,
if (IS_ERR(tfm))
return PTR_ERR(tfm);
+ ret = -ENOMEM;
key = kmalloc(pkey->keylen + sizeof(u32) * 2 + pkey->paramlen,
GFP_KERNEL);
if (!key)
^ permalink raw reply related
* Re: [PATCH bpf-next v4 2/4] bpf: Implement bpf_local_storage for inodes
From: KP Singh @ 2020-07-15 22:59 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: KP Singh, linux-kernel, bpf, linux-security-module,
Alexei Starovoitov, Daniel Borkmann, Paul Turner, Jann Horn,
Florent Revest
In-Reply-To: <20200715215751.6llgungzff66iwxh@kafai-mbp>
On 15-Jul 14:57, Martin KaFai Lau wrote:
> On Thu, Jul 09, 2020 at 12:12:37PM +0200, KP Singh wrote:
> > From: KP Singh <kpsingh@google.com>
> >
> > Similar to bpf_local_storage for sockets, add local storage for inodes.
> > The life-cycle of storage is managed with the life-cycle of the inode.
> > i.e. the storage is destroyed along with the owning inode.
> >
> > The BPF LSM allocates an __rcu pointer to the bpf_local_storage in the
> > security blob which are now stackable and can co-exist with other LSMs.
> >
> > Signed-off-by: KP Singh <kpsingh@google.com>
>
> [ ... ]
>
>
> > +static void *bpf_inode_storage_lookup_elem(struct bpf_map *map, void *key)
> > +{
> > + struct bpf_local_storage_data *sdata;
> > + struct inode *inode;
> > + int err = -EINVAL;
> > +
> > + if (key) {
> > + inode = *(struct inode **)(key);
> The bpf_inode_storage_lookup_elem() here and the (update|delete)_elem() below
> are called from the userspace syscall. How the userspace may provide this key?
I realized this when I replied about the _fd_ name in the sk helpers.
I am going to mark them as unsupported for now for inodes.
We could, probably and separately, use a combination of the device
and inode number as a key from userspace.
- KP
>
> > + sdata = inode_storage_lookup(inode, map, true);
> > + return sdata ? sdata->data : NULL;
> > + }
> > +
> > + return ERR_PTR(err);
> > +}
> > +
> > +static int bpf_inode_storage_update_elem(struct bpf_map *map, void *key,
> > + void *value, u64 map_flags)
> > +{
> > + struct bpf_local_storage_data *sdata;
> > + struct inode *inode;
> > + int err = -EINVAL;
> > +
> > + if (key) {
> > + inode = *(struct inode **)(key);
> > + sdata = map->ops->map_local_storage_update(inode, map, value,
> > + map_flags);
> > + return PTR_ERR_OR_ZERO(sdata);
> > + }
> > + return err;
> > +}
> > +
> > +static int inode_storage_delete(struct inode *inode, struct bpf_map *map)
> > +{
> > + struct bpf_local_storage_data *sdata;
> > +
> > + sdata = inode_storage_lookup(inode, map, false);
> > + if (!sdata)
> > + return -ENOENT;
> > +
> > + bpf_selem_unlink_map_elem(SELEM(sdata));
> > +
> > + return 0;
> > +}
> > +
> > +static int bpf_inode_storage_delete_elem(struct bpf_map *map, void *key)
> > +{
> > + struct inode *inode;
> > + int err = -EINVAL;
> > +
> > + if (key) {
> > + inode = *(struct inode **)(key);
> > + err = inode_storage_delete(inode, map);
> > + }
> > +
> > + return err;
> > +}
> > +
>
> [ ... ]
>
> > +static int inode_storage_map_btf_id;
> > +const struct bpf_map_ops inode_storage_map_ops = {
> > + .map_alloc_check = bpf_local_storage_map_alloc_check,
> > + .map_alloc = inode_storage_map_alloc,
> > + .map_free = inode_storage_map_free,
> > + .map_get_next_key = notsupp_get_next_key,
> > + .map_lookup_elem = bpf_inode_storage_lookup_elem,
> > + .map_update_elem = bpf_inode_storage_update_elem,
> > + .map_delete_elem = bpf_inode_storage_delete_elem,
> > + .map_check_btf = bpf_local_storage_map_check_btf,
> > + .map_btf_name = "bpf_local_storage_map",
> > + .map_btf_id = &inode_storage_map_btf_id,
> > + .map_local_storage_alloc = inode_storage_alloc,
> > + .map_selem_alloc = inode_selem_alloc,
> > + .map_local_storage_update = inode_storage_update,
> > + .map_local_storage_unlink = unlink_inode_storage,
> > +};
> > +
^ permalink raw reply
* Re: [PATCH] Smack: fix use-after-free in smk_write_relabel_self()
From: Sasha Levin @ 2020-07-16 0:27 UTC (permalink / raw)
To: Sasha Levin, Eric Biggers, Eric Biggers, linux-security-module
Cc: syzkaller-bugs, linux-kernel, stable
In-Reply-To: <20200708201520.140376-1-ebiggers@kernel.org>
Hi
[This is an automated email]
This commit has been processed because it contains a "Fixes:" tag
fixing commit: 38416e53936e ("Smack: limited capability for changing process label").
The bot has tested the following trees: v5.7.8, v5.4.51, v4.19.132, v4.14.188, v4.9.230, v4.4.230.
v5.7.8: Build OK!
v5.4.51: Build OK!
v4.19.132: Failed to apply! Possible dependencies:
b17103a8b8ae9 ("Smack: Abstract use of cred security blob")
v4.14.188: Failed to apply! Possible dependencies:
03450e271a160 ("fs: add ksys_fchmod() and do_fchmodat() helpers and ksys_chmod() wrapper; remove in-kernel calls to syscall")
312db1aa1dc7b ("fs: add ksys_mount() helper; remove in-kernel calls to sys_mount()")
3a18ef5c1b393 ("fs: add ksys_umount() helper; remove in-kernel call to sys_umount()")
447016e968196 ("fs: add ksys_chdir() helper; remove in-kernel calls to sys_chdir()")
819671ff849b0 ("syscalls: define and explain goal to not call syscalls in the kernel")
9481769208b5e ("->file_open(): lose cred argument")
a16fe33ab5572 ("fs: add ksys_chroot() helper; remove-in kernel calls to sys_chroot()")
ae2bb293a3e8a ("get rid of cred argument of vfs_open() and do_dentry_open()")
af04fadcaa932 ("Revert "fs: fold open_check_o_direct into do_dentry_open"")
b17103a8b8ae9 ("Smack: Abstract use of cred security blob")
c7248321a3d42 ("fs: add ksys_dup{,3}() helper; remove in-kernel calls to sys_dup{,3}()")
d19dfe58b7ecb ("Smack: Privilege check on key operations")
dcb569cf6ac99 ("Smack: ptrace capability use fixes")
e3f20ae21079e ("security_file_open(): lose cred argument")
e7a3e8b2edf54 ("fs: add ksys_write() helper; remove in-kernel calls to sys_write()")
v4.9.230: Failed to apply! Possible dependencies:
078c73c63fb28 ("apparmor: add profile and ns params to aa_may_manage_policy()")
121d4a91e3c12 ("apparmor: rename sid to secid")
12557dcba21b0 ("apparmor: move lib definitions into separate lib include")
2bd8dbbf22fe9 ("apparmor: add ns being viewed as a param to policy_view_capable()")
5ac8c355ae001 ("apparmor: allow introspecting the loaded policy pre internal transform")
637f688dc3dc3 ("apparmor: switch from profiles to using labels on contexts")
73688d1ed0b8f ("apparmor: refactor prepare_ns() and make usable from different views")
9481769208b5e ("->file_open(): lose cred argument")
98849dff90e27 ("apparmor: rename namespace to ns to improve code line lengths")
9a2d40c12d00e ("apparmor: add strn version of aa_find_ns")
a6f233003b1af ("apparmor: allow specifying the profile doing the management")
b17103a8b8ae9 ("Smack: Abstract use of cred security blob")
cff281f6861e7 ("apparmor: split apparmor policy namespaces code into its own file")
d19dfe58b7ecb ("Smack: Privilege check on key operations")
dcb569cf6ac99 ("Smack: ptrace capability use fixes")
f28e783ff668c ("Smack: Use cap_capable in privilege check")
fd2a80438d736 ("apparmor: add ns being viewed as a param to policy_admin_capable()")
fe6bb31f590c9 ("apparmor: split out shared policy_XXX fns to lib")
v4.4.230: Failed to apply! Possible dependencies:
078c73c63fb28 ("apparmor: add profile and ns params to aa_may_manage_policy()")
121d4a91e3c12 ("apparmor: rename sid to secid")
12557dcba21b0 ("apparmor: move lib definitions into separate lib include")
2bd8dbbf22fe9 ("apparmor: add ns being viewed as a param to policy_view_capable()")
5ac8c355ae001 ("apparmor: allow introspecting the loaded policy pre internal transform")
637f688dc3dc3 ("apparmor: switch from profiles to using labels on contexts")
73688d1ed0b8f ("apparmor: refactor prepare_ns() and make usable from different views")
79be093500791 ("Smack: File receive for sockets")
9481769208b5e ("->file_open(): lose cred argument")
98849dff90e27 ("apparmor: rename namespace to ns to improve code line lengths")
9a2d40c12d00e ("apparmor: add strn version of aa_find_ns")
a6f233003b1af ("apparmor: allow specifying the profile doing the management")
b17103a8b8ae9 ("Smack: Abstract use of cred security blob")
c60b906673eeb ("Smack: Signal delivery as an append operation")
cff281f6861e7 ("apparmor: split apparmor policy namespaces code into its own file")
d19dfe58b7ecb ("Smack: Privilege check on key operations")
dcb569cf6ac99 ("Smack: ptrace capability use fixes")
f28e783ff668c ("Smack: Use cap_capable in privilege check")
fd2a80438d736 ("apparmor: add ns being viewed as a param to policy_admin_capable()")
fe6bb31f590c9 ("apparmor: split out shared policy_XXX fns to lib")
NOTE: The patch will not be queued to stable trees until it is upstream.
How should we proceed with this patch?
--
Thanks
Sasha
^ permalink raw reply
* Re: [PATCH v5 4/6] proc: allow access in init userns for map_files with CAP_CHECKPOINT_RESTORE
From: Christian Brauner @ 2020-07-16 8:51 UTC (permalink / raw)
To: Adrian Reber
Cc: Eric Biederman, Pavel Emelyanov, Oleg Nesterov, Dmitry Safonov,
Andrei Vagin, Nicolas Viennot, Michał Cłapiński,
Kamil Yurtsever, Dirk Petersen, Christine Flood, Casey Schaufler,
Mike Rapoport, Radostin Stoyanov, Cyrill Gorcunov, Serge Hallyn,
Stephen Smalley, Sargun Dhillon, Arnd Bergmann,
linux-security-module, linux-kernel, selinux, Eric Paris,
Jann Horn, linux-fsdevel
In-Reply-To: <20200715144954.1387760-5-areber@redhat.com>
On Wed, Jul 15, 2020 at 04:49:52PM +0200, Adrian Reber wrote:
> Opening files in /proc/pid/map_files when the current user is
> CAP_CHECKPOINT_RESTORE capable in the root namespace is useful for
> checkpointing and restoring to recover files that are unreachable via
> the file system such as deleted files, or memfd files.
>
> Signed-off-by: Adrian Reber <areber@redhat.com>
> Signed-off-by: Nicolas Viennot <Nicolas.Viennot@twosigma.com>
> ---
> fs/proc/base.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index 65893686d1f1..cada783f229e 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -2194,16 +2194,16 @@ struct map_files_info {
> };
>
> /*
> - * Only allow CAP_SYS_ADMIN to follow the links, due to concerns about how the
> - * symlinks may be used to bypass permissions on ancestor directories in the
> - * path to the file in question.
> + * Only allow CAP_SYS_ADMIN and CAP_CHECKPOINT_RESTORE to follow the links, due
> + * to concerns about how the symlinks may be used to bypass permissions on
> + * ancestor directories in the path to the file in question.
> */
> static const char *
> proc_map_files_get_link(struct dentry *dentry,
> struct inode *inode,
> struct delayed_call *done)
> {
> - if (!capable(CAP_SYS_ADMIN))
> + if (!capable(CAP_SYS_ADMIN) || !capable(CAP_CHECKPOINT_RESTORE))
So right now, when I'd do
git grep checkpoint_restore_ns_capable
I'd not hit that codepath which isn't great. So I'd suggest to use:
if (!checkpoint_restore_ns_capable(&init_user_ns))
at the end of the day, capable(<cap>) just calls
ns_capable(&init_user_ns, <cap>) anyway.
Thanks!
Christian
^ permalink raw reply
* [PATCH 01/16] Manual pages: getcap.8, getpcaps.8, setcap.8: SEE ALSO: add capabilities(7)
From: Michael Kerrisk (man-pages) @ 2020-07-16 10:18 UTC (permalink / raw)
To: mtk.manpages, Andrew G . Morgan; +Cc: linux-security-module
Signed-off-by: Michael Kerrisk (man-pages) <mtk.manpages@gmail.com>
---
doc/getcap.8 | 1 +
doc/getpcaps.8 | 1 +
doc/setcap.8 | 1 +
3 files changed, 3 insertions(+)
diff --git a/doc/getcap.8 b/doc/getcap.8
index 05ee9bb..0547ff9 100644
--- a/doc/getcap.8
+++ b/doc/getcap.8
@@ -31,4 +31,5 @@ https://bugzilla.kernel.org/buglist.cgi?component=libcap&list_id=1047723&product
.SH "SEE ALSO"
.BR cap_get_file (3),
.BR cap_to_text (3),
+.BR capabilities (7),
.BR setcap (8)
diff --git a/doc/getpcaps.8 b/doc/getpcaps.8
index b070a3c..53d342e 100644
--- a/doc/getpcaps.8
+++ b/doc/getpcaps.8
@@ -34,6 +34,7 @@ Displays output in a somewhat ugly legacy format.
Displays usage in a legacy-like format but not quite so ugly in modern
default terminal fonts.
.SH SEE ALSO
+.BR capabilities (7),
.BR capsh "(8), " setcap "(8) and " getcap (8).
.br
.SH AUTHOR
diff --git a/doc/setcap.8 b/doc/setcap.8
index 445ed03..da95afb 100644
--- a/doc/setcap.8
+++ b/doc/setcap.8
@@ -58,4 +58,5 @@ https://bugzilla.kernel.org/buglist.cgi?component=libcap&list_id=1047723&product
.SH "SEE ALSO"
.BR cap_from_text (3),
.BR cap_get_file (3),
+.BR capabilities (7),
.BR getcap (8)
--
2.26.2
^ permalink raw reply related
* [PATCH 02/16] Manual pages: cap_get_file.3, getcap.8, setcap.8: SEE ALSO: add user_namespaces(7)
From: Michael Kerrisk (man-pages) @ 2020-07-16 10:18 UTC (permalink / raw)
To: mtk.manpages, Andrew G . Morgan; +Cc: linux-security-module
In-Reply-To: <20200716101827.162793-1-mtk.manpages@gmail.com>
Since namespaces are mentioned in this page, it's wise also to have
a reference to the relevant page that explains the concept.
Signed-off-by: Michael Kerrisk (man-pages) <mtk.manpages@gmail.com>
---
doc/cap_get_file.3 | 3 ++-
doc/getcap.8 | 1 +
doc/setcap.8 | 1 +
3 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/doc/cap_get_file.3 b/doc/cap_get_file.3
index a99ab40..9c115a6 100644
--- a/doc/cap_get_file.3
+++ b/doc/cap_get_file.3
@@ -136,4 +136,5 @@ Permitted or Inheritable flag enabled.
.BR cap_from_text (3),
.BR cap_get_proc (3),
.BR cap_init (3),
-.BR capabilities (7)
+.BR capabilities (7),
+.BR user_namespaces (7)
diff --git a/doc/getcap.8 b/doc/getcap.8
index 0547ff9..497699c 100644
--- a/doc/getcap.8
+++ b/doc/getcap.8
@@ -32,4 +32,5 @@ https://bugzilla.kernel.org/buglist.cgi?component=libcap&list_id=1047723&product
.BR cap_get_file (3),
.BR cap_to_text (3),
.BR capabilities (7),
+.BR user_namespaces (7),
.BR setcap (8)
diff --git a/doc/setcap.8 b/doc/setcap.8
index da95afb..99e3c36 100644
--- a/doc/setcap.8
+++ b/doc/setcap.8
@@ -59,4 +59,5 @@ https://bugzilla.kernel.org/buglist.cgi?component=libcap&list_id=1047723&product
.BR cap_from_text (3),
.BR cap_get_file (3),
.BR capabilities (7),
+.BR user_namespaces (7),
.BR getcap (8)
--
2.26.2
^ permalink raw reply related
* [PATCH 03/16] Manual pages: setcap.8: Formatting fix: use bold for function name
From: Michael Kerrisk (man-pages) @ 2020-07-16 10:18 UTC (permalink / raw)
To: mtk.manpages, Andrew G . Morgan; +Cc: linux-security-module
In-Reply-To: <20200716101827.162793-1-mtk.manpages@gmail.com>
Signed-off-by: Michael Kerrisk (man-pages) <mtk.manpages@gmail.com>
---
doc/setcap.8 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/setcap.8 b/doc/setcap.8
index 99e3c36..90aa13f 100644
--- a/doc/setcap.8
+++ b/doc/setcap.8
@@ -25,7 +25,7 @@ argument is also verified.
The
.I capabilities
are specified in the form described in
-.IR cap_from_text (3).
+.BR cap_from_text (3).
.PP
The special capability string,
.BR '\-' ,
--
2.26.2
^ permalink raw reply related
* [PATCH 04/16] Manual pages: cap_from_text.3: typo fix
From: Michael Kerrisk (man-pages) @ 2020-07-16 10:18 UTC (permalink / raw)
To: mtk.manpages, Andrew G . Morgan; +Cc: linux-security-module
In-Reply-To: <20200716101827.162793-1-mtk.manpages@gmail.com>
Signed-off-by: Michael Kerrisk (man-pages) <mtk.manpages@gmail.com>
---
doc/cap_from_text.3 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/cap_from_text.3 b/doc/cap_from_text.3
index 455a52d..e59ca59 100644
--- a/doc/cap_from_text.3
+++ b/doc/cap_from_text.3
@@ -172,7 +172,7 @@ The example program below demonstrates the use of
.BR cap_from_text ()
and
.BR cap_to_text ().
-The following shell session shows a some example runs:
+The following shell session shows some example runs:
.nf
$ ./a.out "cap_chown=p cap_chown+e"
--
2.26.2
^ permalink raw reply related
* [PATCH 05/16] Manual pages: cap_get_file.3, getcap.8, setcap.8: clarify "namespace"
From: Michael Kerrisk (man-pages) @ 2020-07-16 10:18 UTC (permalink / raw)
To: mtk.manpages, Andrew G . Morgan; +Cc: linux-security-module
In-Reply-To: <20200716101827.162793-1-mtk.manpages@gmail.com>
In these pages, the "namespace" that is being mentioned is the
"user namespace". Make this clearer by adding the word "user".
Signed-off-by: Michael Kerrisk (man-pages) <mtk.manpages@gmail.com>
---
doc/cap_get_file.3 | 2 +-
doc/getcap.8 | 2 +-
doc/setcap.8 | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/doc/cap_get_file.3 b/doc/cap_get_file.3
index 9c115a6..42255ee 100644
--- a/doc/cap_get_file.3
+++ b/doc/cap_get_file.3
@@ -69,7 +69,7 @@ capability state to any file type other than a regular file are
undefined.
.PP
A capability set held in memory can be associated with the rootid in
-use in a specific namespace. It is possible to get and set this value
+use in a specific user namespace. It is possible to get and set this value
(in the memory copy) with
.BR cap_get_nsowner ()
and
diff --git a/doc/getcap.8 b/doc/getcap.8
index 497699c..d867203 100644
--- a/doc/getcap.8
+++ b/doc/getcap.8
@@ -13,7 +13,7 @@ displays the name and capabilities of each specified
prints quick usage.
.TP 4
.B \-n
-prints any non-zero namespace rootid value found to be associated with
+prints any non-zero user namespace rootid value found to be associated with
a file's capabilities.
.TP 4
.B \-r
diff --git a/doc/setcap.8 b/doc/setcap.8
index 90aa13f..ae044aa 100644
--- a/doc/setcap.8
+++ b/doc/setcap.8
@@ -15,7 +15,7 @@ to the
specified. The optional
.B \-n <rootid>
argument can be used to set the file capability for use only in a
-namespace with this rootid owner. The
+user namespace with this rootid owner. The
.B \-v
option is used to verify that the specified capabilities are currently
associated with the file. If \-v and \-n are supplied, the
--
2.26.2
^ permalink raw reply related
* [PATCH 06/16] Manual pages: cap_get_file.3: Remove stray macros that have no effect
From: Michael Kerrisk (man-pages) @ 2020-07-16 10:18 UTC (permalink / raw)
To: mtk.manpages, Andrew G . Morgan; +Cc: linux-security-module
In-Reply-To: <20200716101827.162793-1-mtk.manpages@gmail.com>
These macros give warnings from 'mandoc -T lint'.
Signed-off-by: Michael Kerrisk (man-pages) <mtk.manpages@gmail.com>
---
doc/cap_get_file.3 | 2 --
1 file changed, 2 deletions(-)
diff --git a/doc/cap_get_file.3 b/doc/cap_get_file.3
index 42255ee..c028148 100644
--- a/doc/cap_get_file.3
+++ b/doc/cap_get_file.3
@@ -6,8 +6,6 @@
cap_get_file, cap_set_file, cap_get_fd, cap_set_fd \- capability
manipulation on files
.SH SYNOPSIS
-.B
-.sp
.B #include <sys/capability.h>
.sp
.BI "cap_t cap_get_file(const char *" path_p );
--
2.26.2
^ permalink raw reply related
* [PATCH 07/16] Manual pages: cap_get_proc.3: s/UNCERTAIN/CAP_MODE_UNCERTAIN/
From: Michael Kerrisk (man-pages) @ 2020-07-16 10:18 UTC (permalink / raw)
To: mtk.manpages, Andrew G . Morgan; +Cc: linux-security-module
In-Reply-To: <20200716101827.162793-1-mtk.manpages@gmail.com>
Use the proper name of the constant in DESCRIPTION.
Signed-off-by: Michael Kerrisk (man-pages) <mtk.manpages@gmail.com>
---
doc/cap_get_proc.3 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/cap_get_proc.3 b/doc/cap_get_proc.3
index b0a61d1..ded1491 100644
--- a/doc/cap_get_proc.3
+++ b/doc/cap_get_proc.3
@@ -168,7 +168,7 @@ returns
which
.BR cap_get_name ()
declares as
-.IR "UNCERTAIN" .
+.BR CAP_MODE_UNCERTAIN .
Supported modes are:
.BR CAP_MODE_NOPRIV ", " CAP_MODE_PURE1E_INIT " and " CAP_MODE_PURE1E .
.PP
--
2.26.2
^ permalink raw reply related
* [PATCH 08/16] Manual pages: cap_get_proc.3: formatting fix
From: Michael Kerrisk (man-pages) @ 2020-07-16 10:18 UTC (permalink / raw)
To: mtk.manpages, Andrew G . Morgan; +Cc: linux-security-module
In-Reply-To: <20200716101827.162793-1-mtk.manpages@gmail.com>
Signed-off-by: Michael Kerrisk (man-pages) <mtk.manpages@gmail.com>
---
doc/cap_get_proc.3 | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/doc/cap_get_proc.3 b/doc/cap_get_proc.3
index ded1491..f90c4f5 100644
--- a/doc/cap_get_proc.3
+++ b/doc/cap_get_proc.3
@@ -261,7 +261,8 @@ The library also supports the deprecated functions:
.BR capgetp ()
attempts to obtain the capabilities of some other process; storing the
capabilities in a pre-allocated
-.IR cap_d . See
+.IR cap_d .
+See
.BR cap_init ()
for information on allocating an empty capability set. This function,
.BR capgetp (),
--
2.26.2
^ permalink raw reply related
* [PATCH 09/16] Manual pages: capsh.1: spelling fixes
From: Michael Kerrisk (man-pages) @ 2020-07-16 10:18 UTC (permalink / raw)
To: mtk.manpages, Andrew G . Morgan; +Cc: linux-security-module
In-Reply-To: <20200716101827.162793-1-mtk.manpages@gmail.com>
Signed-off-by: Michael Kerrisk (man-pages) <mtk.manpages@gmail.com>
---
doc/capsh.1 | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/doc/capsh.1 b/doc/capsh.1
index cd30dc3..24e80b7 100644
--- a/doc/capsh.1
+++ b/doc/capsh.1
@@ -103,7 +103,7 @@ effective set.
use the
.BR cap_setuid (3)
function to set the uid of the current process. This performs all
-prepations for setting the uid without dropping capabilities in the
+preparations for setting the uid without dropping capabilities in the
process. Following this command the prevailing effective capabilities
will be lowered.
.TP
@@ -242,7 +242,7 @@ vector has capability
raised.
.TP
.BI \-\-addamb= xxx
-Adds the specificed ambient capability to the running process.
+Adds the specified ambient capability to the running process.
.TP
.BI \-\-delamb= xxx
Removes the specified ambient capability from the running process.
--
2.26.2
^ permalink raw reply related
* [PATCH 10/16] Manual pages: capsh.1: Remove stray .TP macro
From: Michael Kerrisk (man-pages) @ 2020-07-16 10:18 UTC (permalink / raw)
To: mtk.manpages, Andrew G . Morgan; +Cc: linux-security-module
In-Reply-To: <20200716101827.162793-1-mtk.manpages@gmail.com>
Signed-off-by: Michael Kerrisk (man-pages) <mtk.manpages@gmail.com>
---
doc/capsh.1 | 2 --
1 file changed, 2 deletions(-)
diff --git a/doc/capsh.1 b/doc/capsh.1
index 24e80b7..522e719 100644
--- a/doc/capsh.1
+++ b/doc/capsh.1
@@ -249,8 +249,6 @@ Removes the specified ambient capability from the running process.
.TP
.B \-\-noamb
Drops all ambient capabilities from the running process.
-.TP
-
.SH "EXIT STATUS"
Following successful execution the tool exits with status 0. Following
an error, the tool immediately exits with status 1.
--
2.26.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox