* Re: [PATCH V31 25/25] debugfs: Disable open() when kernel is locked down
From: Andy Lutomirski @ 2019-03-27 4:29 UTC (permalink / raw)
To: Greg KH
Cc: Andy Lutomirski, Matthew Garrett, James Morris, LSM List, LKML,
David Howells, Linux API, Matthew Garrett
In-Reply-To: <20190327003057.GA27311@kroah.com>
On Tue, Mar 26, 2019 at 5:31 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Tue, Mar 26, 2019 at 12:20:24PM -0700, Andy Lutomirski wrote:
> > On Tue, Mar 26, 2019 at 11:28 AM Matthew Garrett
> > <matthewgarrett@google.com> wrote:
> > >
> > > From: Matthew Garrett <mjg59@google.com>
> > >
> > > debugfs has not been meaningfully audited in terms of ensuring that
> > > userland cannot trample over the kernel. At Greg's request, disable
> > > access to it entirely when the kernel is locked down. This is done at
> > > open() time rather than init time as the kernel lockdown status may be
> > > made stricter at runtime.
> >
> > Ugh. Some of those files are very useful. Could this perhaps still
> > allow O_RDONLY if we're in INTEGRITY mode?
>
> Useful for what? Debugging, sure, but for "normal operation", no kernel
> functionality should ever require debugfs. If it does, that's a bug and
> should be fixed.
>
I semi-regularly read files in debugfs to diagnose things, and I think
it would be good for this to work on distro kernels.
^ permalink raw reply
* Re: [PATCH V31 25/25] debugfs: Disable open() when kernel is locked down
From: Greg KH @ 2019-03-27 2:35 UTC (permalink / raw)
To: Matthew Garrett
Cc: James Morris, LSM List, Linux Kernel Mailing List, David Howells,
Linux API, Andy Lutomirski
In-Reply-To: <CACdnJuvBJmCRnzT=4qh9cmNkRNHi3Xfha5xqUqYu8cgCRrtsqQ@mail.gmail.com>
On Tue, Mar 26, 2019 at 07:06:36PM -0700, Matthew Garrett wrote:
> On Tue, Mar 26, 2019 at 5:31 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Tue, Mar 26, 2019 at 11:27:41AM -0700, Matthew Garrett wrote:
> > > From: Matthew Garrett <mjg59@google.com>
> > >
> > > debugfs has not been meaningfully audited in terms of ensuring that
> > > userland cannot trample over the kernel. At Greg's request, disable
> > > access to it entirely when the kernel is locked down. This is done at
> > > open() time rather than init time as the kernel lockdown status may be
> > > made stricter at runtime.
>
> (snip)
>
> > Why allow all this, why not just abort the registering of the filesystem
> > with the vfs core so it can't even be mounted?
>
> As mentioned in the commit message, because the lockdown state can be
> made stricter at runtime - blocking at mount time would be
> inconsistent if the machine is locked down afterwards. We could
> potentially assert that it's the admin's responsibility to ensure that
> debugfs isn't mounted at the point of policy being made stricter?
Ugh, I can not read, sorry, neverind.
The patch is fine as-is.
greg k-h
^ permalink raw reply
* Re: [PATCH v2 00/11] LSM documentation update
From: James Morris @ 2019-03-27 2:22 UTC (permalink / raw)
To: Denis Efremov
Cc: Casey Schaufler, Kees Cook, Eric W. Biederman, Eric Paris,
John Johansen, Serge E. Hallyn, Paul Moore, Kentaro Takeda,
linux-security-module, linux-kernel
In-Reply-To: <cover.1551213058.git.efremov@ispras.ru>
On Tue, 26 Feb 2019, Denis Efremov wrote:
> Recent "New LSM Hooks" discussion has led me to the
> thought that it might be a good idea to slightly
> update the current documentation. The patchset adds
> nothing new to the documentation, only fixes the old
> description of hooks to reflect their current state.
>
> V2 adds the clarification on arguments for some hooks.
> The format of the documentation is also slightly updated
> for better html. However, there are still 10 hooks without
> documentation at all. I think that this should be fixed
> separatedly.
Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next-general
Thanks!
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH V31 25/25] debugfs: Disable open() when kernel is locked down
From: Matthew Garrett @ 2019-03-27 2:06 UTC (permalink / raw)
To: Greg KH
Cc: James Morris, LSM List, Linux Kernel Mailing List, David Howells,
Linux API, Andy Lutomirski
In-Reply-To: <20190327003130.GB27311@kroah.com>
On Tue, Mar 26, 2019 at 5:31 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> On Tue, Mar 26, 2019 at 11:27:41AM -0700, Matthew Garrett wrote:
> > From: Matthew Garrett <mjg59@google.com>
> >
> > debugfs has not been meaningfully audited in terms of ensuring that
> > userland cannot trample over the kernel. At Greg's request, disable
> > access to it entirely when the kernel is locked down. This is done at
> > open() time rather than init time as the kernel lockdown status may be
> > made stricter at runtime.
(snip)
> Why allow all this, why not just abort the registering of the filesystem
> with the vfs core so it can't even be mounted?
As mentioned in the commit message, because the lockdown state can be
made stricter at runtime - blocking at mount time would be
inconsistent if the machine is locked down afterwards. We could
potentially assert that it's the admin's responsibility to ensure that
debugfs isn't mounted at the point of policy being made stricter?
^ permalink raw reply
* Re: [PATCH V31 25/25] debugfs: Disable open() when kernel is locked down
From: Greg KH @ 2019-03-27 0:31 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, dhowells, linux-api,
luto, Matthew Garrett
In-Reply-To: <20190326182742.16950-26-matthewgarrett@google.com>
On Tue, Mar 26, 2019 at 11:27:41AM -0700, Matthew Garrett wrote:
> From: Matthew Garrett <mjg59@google.com>
>
> debugfs has not been meaningfully audited in terms of ensuring that
> userland cannot trample over the kernel. At Greg's request, disable
> access to it entirely when the kernel is locked down. This is done at
> open() time rather than init time as the kernel lockdown status may be
> made stricter at runtime.
>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> Cc: gregkh@linuxfoundation.org
> ---
> fs/debugfs/file.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
> index 4fce1da7db23..9ae12ef29ba0 100644
> --- a/fs/debugfs/file.c
> +++ b/fs/debugfs/file.c
> @@ -142,6 +142,9 @@ static int open_proxy_open(struct inode *inode, struct file *filp)
> const struct file_operations *real_fops = NULL;
> int r;
>
> + if (kernel_is_locked_down("debugfs", LOCKDOWN_INTEGRITY))
> + return -EPERM;
Why allow all this, why not just abort the registering of the filesystem
with the vfs core so it can't even be mounted?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH V31 25/25] debugfs: Disable open() when kernel is locked down
From: Greg KH @ 2019-03-27 0:30 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Matthew Garrett, James Morris, LSM List, LKML, David Howells,
Linux API, Matthew Garrett
In-Reply-To: <CALCETrXDF2c2-gnOyNTnEn-uPAZp0AaSB_PB8e2y8jf7zo=Cyg@mail.gmail.com>
On Tue, Mar 26, 2019 at 12:20:24PM -0700, Andy Lutomirski wrote:
> On Tue, Mar 26, 2019 at 11:28 AM Matthew Garrett
> <matthewgarrett@google.com> wrote:
> >
> > From: Matthew Garrett <mjg59@google.com>
> >
> > debugfs has not been meaningfully audited in terms of ensuring that
> > userland cannot trample over the kernel. At Greg's request, disable
> > access to it entirely when the kernel is locked down. This is done at
> > open() time rather than init time as the kernel lockdown status may be
> > made stricter at runtime.
>
> Ugh. Some of those files are very useful. Could this perhaps still
> allow O_RDONLY if we're in INTEGRITY mode?
Useful for what? Debugging, sure, but for "normal operation", no kernel
functionality should ever require debugfs. If it does, that's a bug and
should be fixed.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v19 17/27] x86/sgx: Add provisioning
From: Sean Christopherson @ 2019-03-27 0:14 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Andy Lutomirski, X86 ML, linux-sgx, Andrew Morton, Dave Hansen,
nhorman, npmccallum, Ayoun, Serge, Katz-zamir, Shay,
Huang, Haitao, Andy Shevchenko, Thomas Gleixner, Svahn, Kai,
Borislav Petkov, Josh Triplett, Huang, Kai, David Rientjes,
James Morris, Serge E . Hallyn, LSM List
In-Reply-To: <20190325145503.GB29989@linux.intel.com>
On Mon, Mar 25, 2019 at 04:55:03PM +0200, Jarkko Sakkinen wrote:
> On Fri, Mar 22, 2019 at 11:20:30AM -0700, Andy Lutomirski wrote:
> > On Fri, Mar 22, 2019 at 4:43 AM Jarkko Sakkinen
> > <jarkko.sakkinen@linux.intel.com> wrote:
> > >
> > > On Fri, Mar 22, 2019 at 01:29:38PM +0200, Jarkko Sakkinen wrote:
> > > > On Thu, Mar 21, 2019 at 09:50:41AM -0700, Andy Lutomirski wrote:
> > > > > On Sun, Mar 17, 2019 at 2:18 PM Jarkko Sakkinen
> > > > > <jarkko.sakkinen@linux.intel.com> wrote:
> > > > > >
> > > > > > In order to provide a mechanism for devilering provisoning rights:
> > > > > >
> > > > > > 1. Add a new file to the securityfs file called sgx/provision that works
> > > > > > as a token for allowing an enclave to have the provisioning privileges.
> > > > > > 2. Add a new ioctl called SGX_IOC_ENCLAVE_SET_ATTRIBUTE that accepts the
> > > > > > following data structure:
> > > > > >
> > > > > > struct sgx_enclave_set_attribute {
> > > > > > __u64 addr;
> > > > > > __u64 token_fd;
> > > > > > };
> > > > >
> > > > > Here's a potential issue:
> > > > >
> > > > > For container use, is it reasonable for a container manager to
> > > > > bind-mount a file into securityfs? Or would something in /dev make
> > > > > this easier?
> > > >
> > > > I guess that is a valid point given that the securityfs contains the LSM
> > > > (e.g. SELinux or AppArmor) policy. So yeah, I think your are right what
> > > > you say.
> > > >
> > > > I propose that we create /dev/sgx/enclave to act as the enclave manager
> > > > and /dev/sgx/provision for provisioning. Is this sustainable for you?
> > >
> > > Hmm.. on 2nd thought the LSM policy or even DAC policy would restrict
> > > that the container manager can only access specific files inside
> > > securityfs. With this conclusion I still think it is probably the best
> > > place for seurity policy like things even for SGX. It is meant for that
> > > anyway.
> > >
> >
> > LSM or DAC policy can certainly *restrict* it, but I suspect that most
> > container runtimes don't mount securityfs at all. OTOH, the runtime
> > definitely needs to have a way to pass /dev/sgx/enclave (or whatever
> > it's called) through, so using another device node will definitely
> > work.
>
> OK, I can cope with this argument. I go with the device names above for
> v20.
Regardless of where the file ends up living, I think it should be owned
by the "core" SGX subsystem and not the driver. At some point in the
past Andy requested that KVM intercept EINIT as needed to prevent
unauthorized access to the PROVISION_KEY (by running a KVM guest),
I.e. userspace may want to access /dev/sgx/provision or whatever even
if the driver is not loaded. I don't see any reason to defer that change
to the KVM series.
If the ACPI-based autoprobing is removed, then sgx_init() can remove the
file if probing the driver fails, i.e. the kernel won't end up with a
file that userspace can't use for anything.
^ permalink raw reply
* Re: [PATCH ghak109 V2] audit: link integrity evm_write_xattrs record to syscall event
From: Paul Moore @ 2019-03-26 23:58 UTC (permalink / raw)
To: Mimi Zohar
Cc: Richard Guy Briggs, linux-integrity, linux-security-module,
Linux-Audit Mailing List, LKML, sgrubb, omosnace, Eric Paris,
Serge Hallyn, mjg59
In-Reply-To: <1553632830.4233.3.camel@linux.ibm.com>
On Tue, Mar 26, 2019 at 4:40 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> Hi Richard, Paul,
>
> On Tue, 2019-03-26 at 14:49 -0400, Richard Guy Briggs wrote:
> > In commit fa516b66a1bf ("EVM: Allow runtime modification of the set of
> > verified xattrs"), the call to audit_log_start() is missing a context to
> > link it to an audit event. Since this event is in user context, add
> > the process' syscall context to the record.
> >
> > In addition, the orphaned keyword "locked" appears in the record.
> > Normalize this by changing it to logging the locking string "." as any
> > other user input in the "xattr=" field.
> >
> > Please see the github issue
> > https://github.com/linux-audit/audit-kernel/issues/109
> >
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
>
> Acked-by: Mimi Zohar <zohar@linux.ibm.com>
>
> Paul, were you planning on upstreaming this patch?
Yep, unless you would rather do it? If you pull it into the IMA tree,
please add my ACK; otherwise let me know and I'll merge it into
audit/next.
Acked-by: Paul Moore <paul@paul-moore.com>
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: Linux security subsystem tree sync to v5.1-rc2
From: Casey Schaufler @ 2019-03-26 23:55 UTC (permalink / raw)
To: James Morris, linux-security-module
In-Reply-To: <alpine.LRH.2.21.1903271036300.9951@namei.org>
On 3/26/2019 4:37 PM, James Morris wrote:
> FYI,
>
> The next-general branch has been updated to v5.1-rc2
>
> git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git
Thank you. This is always a big help.
^ permalink raw reply
* Linux security subsystem tree sync to v5.1-rc2
From: James Morris @ 2019-03-26 23:37 UTC (permalink / raw)
To: linux-security-module
FYI,
The next-general branch has been updated to v5.1-rc2
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* [PATCH] Yama: mark local symbols as static
From: Jann Horn @ 2019-03-26 23:08 UTC (permalink / raw)
To: Kees Cook, jannh
Cc: linux-kernel, James Morris, Serge E. Hallyn,
linux-security-module
sparse complains that Yama defines functions and a variable as non-static
even though they don't exist in any header. Fix it by making them static.
Signed-off-by: Jann Horn <jannh@google.com>
---
security/yama/yama_lsm.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c
index 57cc60722dd3..06b14a57b0a4 100644
--- a/security/yama/yama_lsm.c
+++ b/security/yama/yama_lsm.c
@@ -206,7 +206,7 @@ static void yama_ptracer_del(struct task_struct *tracer,
* yama_task_free - check for task_pid to remove from exception list
* @task: task being removed
*/
-void yama_task_free(struct task_struct *task)
+static void yama_task_free(struct task_struct *task)
{
yama_ptracer_del(task, task);
}
@@ -401,7 +401,7 @@ static int yama_ptrace_access_check(struct task_struct *child,
*
* Returns 0 if following the ptrace is allowed, -ve on error.
*/
-int yama_ptrace_traceme(struct task_struct *parent)
+static int yama_ptrace_traceme(struct task_struct *parent)
{
int rc = 0;
@@ -452,7 +452,7 @@ static int yama_dointvec_minmax(struct ctl_table *table, int write,
static int zero;
static int max_scope = YAMA_SCOPE_NO_ATTACH;
-struct ctl_path yama_sysctl_path[] = {
+static struct ctl_path yama_sysctl_path[] = {
{ .procname = "kernel", },
{ .procname = "yama", },
{ }
--
2.21.0.392.gf8f6787159e-goog
^ permalink raw reply related
* Re: [PATCH 22/27] Lock down kprobes
From: Masami Hiramatsu @ 2019-03-26 22:47 UTC (permalink / raw)
To: Matthew Garrett
Cc: James Morris, LSM List, Linux Kernel Mailing List, David Howells,
Alexei Starovoitov, Naveen N . Rao, Anil S Keshavamurthy, davem
In-Reply-To: <CACdnJuss=NmcxS0UoTeopYzipt53ba5GTBwQ+YtJQXLj7_Q10w@mail.gmail.com>
On Tue, 26 Mar 2019 10:41:23 -0700
Matthew Garrett <mjg59@google.com> wrote:
> On Tue, Mar 26, 2019 at 5:30 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> >
> > On Mon, 25 Mar 2019 15:09:49 -0700
> > Matthew Garrett <matthewgarrett@google.com> wrote:
> >
> > > From: David Howells <dhowells@redhat.com>
> > >
> > > Disallow the creation of kprobes when the kernel is locked down by
> > > preventing their registration. This prevents kprobes from being used to
> > > access kernel memory, either to make modifications or to steal crypto data.
> >
> > Hmm, if you enforce signature check of modules, those modules
> > should be allowed to use kprobes?
> > I think we should introduce some kind of trust inheritance from
> > signed (trusted) modules.
>
> Is there any way to install a kprobe /without/ it coming from a
> module? The presumption in lockdown mode is that module signing is
> enforced, so I'll admit to not being entirely clear on why this patch
> is needed in that case.
Yes, there are 2 paths, ftrace and perf(bpf). If you want to disable ftrace
path (which start from user's input via tracefs), this should be done in
trace_kprobe_create()@kernel/trace/trace_kprobe.c.
If you want to disable both, __register_trace_kprobe()@kernel/trace/trace_kprobe.c
is the best place.
Thank you,
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH V31 10/25] PCI: Lock down BAR access when the kernel is locked down
From: Alex Williamson @ 2019-03-26 21:19 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Matthew Garrett, James Morris, LSM List, LKML, David Howells,
Linux API, Matthew Garrett, Matthew Garrett, Bjorn Helgaas,
linux-pci
In-Reply-To: <CALCETrW43Syz-deM-fGUCbWfA8tcsoCJJ5XqjFhpg+d0sNYDRA@mail.gmail.com>
On Tue, 26 Mar 2019 13:55:39 -0700
Andy Lutomirski <luto@kernel.org> wrote:
> On Tue, Mar 26, 2019 at 11:28 AM Matthew Garrett
> <matthewgarrett@google.com> wrote:
> >
> > From: Matthew Garrett <mjg59@srcf.ucam.org>
> >
> > Any hardware that can potentially generate DMA has to be locked down in
> > order to avoid it being possible for an attacker to modify kernel code,
> > allowing them to circumvent disabled module loading or module signing.
> > Default to paranoid - in future we can potentially relax this for
> > sufficiently IOMMU-isolated devices.
>
> Does this break vfio?
No, vfio provides its own interface to pci config space. Thanks,
Alex
^ permalink raw reply
* Re: [PATCH V31 11/25] x86: Lock down IO port access when the kernel is locked down
From: Andy Lutomirski @ 2019-03-26 20:56 UTC (permalink / raw)
To: Matthew Garrett
Cc: James Morris, LSM List, LKML, David Howells, Linux API,
Andrew Lutomirski, Matthew Garrett, Matthew Garrett, X86 ML
In-Reply-To: <20190326182742.16950-12-matthewgarrett@google.com>
On Tue, Mar 26, 2019 at 11:28 AM Matthew Garrett
<matthewgarrett@google.com> wrote:
>
> From: Matthew Garrett <mjg59@srcf.ucam.org>
>
> IO port access would permit users to gain access to PCI configuration
> registers, which in turn (on a lot of hardware) give access to MMIO
> register space. This would potentially permit root to trigger arbitrary
> DMA, so lock it down by default.
>
> This also implicitly locks down the KDADDIO, KDDELIO, KDENABIO and
> KDDISABIO console ioctls.
>
Reviewed-by: Andy Lutomirski <luto@kernel.org>
^ permalink raw reply
* Re: [PATCH V31 10/25] PCI: Lock down BAR access when the kernel is locked down
From: Andy Lutomirski @ 2019-03-26 20:55 UTC (permalink / raw)
To: Matthew Garrett
Cc: James Morris, LSM List, LKML, David Howells, Linux API,
Andrew Lutomirski, Matthew Garrett, Matthew Garrett,
Bjorn Helgaas, linux-pci
In-Reply-To: <20190326182742.16950-11-matthewgarrett@google.com>
On Tue, Mar 26, 2019 at 11:28 AM Matthew Garrett
<matthewgarrett@google.com> wrote:
>
> From: Matthew Garrett <mjg59@srcf.ucam.org>
>
> Any hardware that can potentially generate DMA has to be locked down in
> order to avoid it being possible for an attacker to modify kernel code,
> allowing them to circumvent disabled module loading or module signing.
> Default to paranoid - in future we can potentially relax this for
> sufficiently IOMMU-isolated devices.
Does this break vfio?
--Andy
^ permalink raw reply
* Re: [PATCH ghak109 V2] audit: link integrity evm_write_xattrs record to syscall event
From: Mimi Zohar @ 2019-03-26 20:40 UTC (permalink / raw)
To: Richard Guy Briggs, linux-integrity, linux-security-module,
Linux-Audit Mailing List, LKML
Cc: Paul Moore, sgrubb, omosnace, eparis, serge, mjg59
In-Reply-To: <087489b21e50bcda65c6af3e038394d5bfe09e00.1553626080.git.rgb@redhat.com>
Hi Richard, Paul,
On Tue, 2019-03-26 at 14:49 -0400, Richard Guy Briggs wrote:
> In commit fa516b66a1bf ("EVM: Allow runtime modification of the set of
> verified xattrs"), the call to audit_log_start() is missing a context to
> link it to an audit event. Since this event is in user context, add
> the process' syscall context to the record.
>
> In addition, the orphaned keyword "locked" appears in the record.
> Normalize this by changing it to logging the locking string "." as any
> other user input in the "xattr=" field.
>
> Please see the github issue
> https://github.com/linux-audit/audit-kernel/issues/109
>
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
Acked-by: Mimi Zohar <zohar@linux.ibm.com>
Paul, were you planning on upstreaming this patch?
Mimi
^ permalink raw reply
* Re: [PATCH 23/27] bpf: Restrict kernel image access functions when the kernel is locked down
From: Matthew Garrett @ 2019-03-26 20:19 UTC (permalink / raw)
To: James Morris
Cc: Andy Lutomirski, Stephen Hemminger, Linux API, LSM List, LKML,
David Howells, Alexei Starovoitov, Network Development,
Chun-Yi Lee, Daniel Borkmann, Kees Cook, Will Drewry
In-Reply-To: <alpine.LRH.2.21.1903270523240.5235@namei.org>
On Tue, Mar 26, 2019 at 11:57 AM James Morris <jmorris@namei.org> wrote:
> - Assign an ID to each lockdown point
> - Implement a policy mechanism where each ID is mapped to 0 or 1
> - Allow this policy to be specified statically or dynamically
One of the problems with this approach is what the default behaviour
should be when a new feature is added. If an admin fails to notice
that there's now a new policy element, they run the risk of kernel
integrity being compromised via the new feature even if the rest of
the kernel is locked down.
^ permalink raw reply
* Re: [PATCH 23/27] bpf: Restrict kernel image access functions when the kernel is locked down
From: Andy Lutomirski @ 2019-03-26 19:22 UTC (permalink / raw)
To: James Morris
Cc: Andy Lutomirski, Stephen Hemminger, Linux API, Matthew Garrett,
LSM List, LKML, David Howells, Alexei Starovoitov,
Network Development, Chun-Yi Lee, Daniel Borkmann, Kees Cook,
Will Drewry
In-Reply-To: <alpine.LRH.2.21.1903270523240.5235@namei.org>
On Tue, Mar 26, 2019 at 11:57 AM James Morris <jmorris@namei.org> wrote:
>
> On Mon, 25 Mar 2019, Andy Lutomirski wrote:
>
> > A while back, I suggested an approach to actually make this stuff
> > mergeable: submit a patch series that adds lockdown mode, enables it
> > by command line option (and maybe sysctl) *only* and has either no
> > effect or only a token effect. Then we can add actual features to
> > lockdown mode one at a time and review them separately.
>
> This makes sense to me.
>
> >
> > And I'm going to complain loudly unless two things change about this
> > whole thing:
> >
> > 1. Lockdown mode becomes three states, not a boolean. The states are:
> > no lockdown, best-effort-to-protect-kernel-integrity, and
> > best-effort-to-protect-kernel-secrecy-and-integrity. And this BPF
> > mess illustrates why: most users will really strongly object to
> > turning off BPF when they actually just want to protect kernel
> > integrity. And as far as I know, things like Secure Boot policy will
> > mostly care about integrity, not secrecy, and tracing and such should
> > work on a normal locked-down kernel. So I think we need this knob.
>
> Another approach would be to make this entirely policy based:
>
> - Assign an ID to each lockdown point
> - Implement a policy mechanism where each ID is mapped to 0 or 1
> - Allow this policy to be specified statically or dynamically
>
> So,
>
> kernel_is_locked_down("ioperm")
>
> becomes
>
> kernel_is_locked_down(LOCKDOWN_IOPERM)
>
> and this function checks e.g.
>
> if (lockdown_polcy[id]) {
> fail or warn;
> }
>
> Thoughts?
I'm concerned that this gives too much useless flexibility to
administrators and user code in general. If you can break kernel
integrity, you can break kernel integrity -- it shouldn't really
matter *how* you break it.
--Andy
^ permalink raw reply
* Re: [PATCH V31 25/25] debugfs: Disable open() when kernel is locked down
From: Matthew Garrett @ 2019-03-26 19:21 UTC (permalink / raw)
To: Andy Lutomirski
Cc: James Morris, LSM List, LKML, David Howells, Linux API, Greg KH
In-Reply-To: <CALCETrXDF2c2-gnOyNTnEn-uPAZp0AaSB_PB8e2y8jf7zo=Cyg@mail.gmail.com>
On Tue, Mar 26, 2019 at 12:20 PM Andy Lutomirski <luto@kernel.org> wrote:
> Ugh. Some of those files are very useful. Could this perhaps still
> allow O_RDONLY if we're in INTEGRITY mode?
The previous implementation did, but Greg wanted it to go away entirely.
^ permalink raw reply
* Re: [PATCH V31 22/25] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Andy Lutomirski @ 2019-03-26 19:21 UTC (permalink / raw)
To: Matthew Garrett
Cc: James Morris, LSM List, LKML, David Howells, Linux API,
Andrew Lutomirski, Alexei Starovoitov, Matthew Garrett,
Network Development, Chun-Yi Lee, Daniel Borkmann
In-Reply-To: <20190326182742.16950-23-matthewgarrett@google.com>
On Tue, Mar 26, 2019 at 11:28 AM Matthew Garrett
<matthewgarrett@google.com> wrote:
>
> From: David Howells <dhowells@redhat.com>
>
> There are some bpf functions can be used to read kernel memory:
> bpf_probe_read, bpf_probe_write_user and bpf_trace_printk. These allow
> private keys in kernel memory (e.g. the hibernation image signing key) to
> be read by an eBPF program and kernel memory to be altered without
> restriction. Disable them if the kernel has been locked down in
> confidentiality mode.
>
:)
This is yet another reason to get the new improved bpf_probe_user_read
stuff landed!
^ permalink raw reply
* Re: [PATCH V31 25/25] debugfs: Disable open() when kernel is locked down
From: Andy Lutomirski @ 2019-03-26 19:20 UTC (permalink / raw)
To: Matthew Garrett
Cc: James Morris, LSM List, LKML, David Howells, Linux API,
Andrew Lutomirski, Matthew Garrett, Greg KH
In-Reply-To: <20190326182742.16950-26-matthewgarrett@google.com>
On Tue, Mar 26, 2019 at 11:28 AM Matthew Garrett
<matthewgarrett@google.com> wrote:
>
> From: Matthew Garrett <mjg59@google.com>
>
> debugfs has not been meaningfully audited in terms of ensuring that
> userland cannot trample over the kernel. At Greg's request, disable
> access to it entirely when the kernel is locked down. This is done at
> open() time rather than init time as the kernel lockdown status may be
> made stricter at runtime.
Ugh. Some of those files are very useful. Could this perhaps still
allow O_RDONLY if we're in INTEGRITY mode?
^ permalink raw reply
* Re: [PATCH 23/27] bpf: Restrict kernel image access functions when the kernel is locked down
From: James Morris @ 2019-03-26 18:57 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Stephen Hemminger, Linux API, Matthew Garrett, LSM List, LKML,
David Howells, Alexei Starovoitov, Network Development,
Chun-Yi Lee, Daniel Borkmann, Kees Cook, Will Drewry
In-Reply-To: <CALCETrU=EqqRwyQGBT0=_N9-E_pBM53=z+WJrd9OzV7g6xJa0A@mail.gmail.com>
On Mon, 25 Mar 2019, Andy Lutomirski wrote:
> A while back, I suggested an approach to actually make this stuff
> mergeable: submit a patch series that adds lockdown mode, enables it
> by command line option (and maybe sysctl) *only* and has either no
> effect or only a token effect. Then we can add actual features to
> lockdown mode one at a time and review them separately.
This makes sense to me.
>
> And I'm going to complain loudly unless two things change about this
> whole thing:
>
> 1. Lockdown mode becomes three states, not a boolean. The states are:
> no lockdown, best-effort-to-protect-kernel-integrity, and
> best-effort-to-protect-kernel-secrecy-and-integrity. And this BPF
> mess illustrates why: most users will really strongly object to
> turning off BPF when they actually just want to protect kernel
> integrity. And as far as I know, things like Secure Boot policy will
> mostly care about integrity, not secrecy, and tracing and such should
> work on a normal locked-down kernel. So I think we need this knob.
Another approach would be to make this entirely policy based:
- Assign an ID to each lockdown point
- Implement a policy mechanism where each ID is mapped to 0 or 1
- Allow this policy to be specified statically or dynamically
So,
kernel_is_locked_down("ioperm")
becomes
kernel_is_locked_down(LOCKDOWN_IOPERM)
and this function checks e.g.
if (lockdown_polcy[id]) {
fail or warn;
}
Thoughts?
> 2. All the proponents of this series, and the documentation, needs to
> document that it's best effort. There will always be security bugs,
> and there will always be things we miss.
Right. Maintaining this feature will be an ongoing effort, and if its not
actively maintained, it will bitrot and become useless.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* [PATCH ghak109 V2] audit: link integrity evm_write_xattrs record to syscall event
From: Richard Guy Briggs @ 2019-03-26 18:49 UTC (permalink / raw)
To: linux-integrity, linux-security-module, Linux-Audit Mailing List,
LKML
Cc: Paul Moore, sgrubb, omosnace, eparis, serge, zohar, mjg59,
Richard Guy Briggs
In commit fa516b66a1bf ("EVM: Allow runtime modification of the set of
verified xattrs"), the call to audit_log_start() is missing a context to
link it to an audit event. Since this event is in user context, add
the process' syscall context to the record.
In addition, the orphaned keyword "locked" appears in the record.
Normalize this by changing it to logging the locking string "." as any
other user input in the "xattr=" field.
Please see the github issue
https://github.com/linux-audit/audit-kernel/issues/109
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
Changelog:
v2
- switch from "(locked)" to printing the "." verbatim, untrusted.
security/integrity/evm/evm_secfs.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c
index 015aea8fdf1e..3f7cbb238923 100644
--- a/security/integrity/evm/evm_secfs.c
+++ b/security/integrity/evm/evm_secfs.c
@@ -192,7 +192,8 @@ static ssize_t evm_write_xattrs(struct file *file, const char __user *buf,
if (count > XATTR_NAME_MAX)
return -E2BIG;
- ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_INTEGRITY_EVM_XATTR);
+ ab = audit_log_start(audit_context(), GFP_KERNEL,
+ AUDIT_INTEGRITY_EVM_XATTR);
if (!ab)
return -ENOMEM;
@@ -214,6 +215,9 @@ static ssize_t evm_write_xattrs(struct file *file, const char __user *buf,
if (len && xattr->name[len-1] == '\n')
xattr->name[len-1] = '\0';
+ audit_log_format(ab, "xattr=");
+ audit_log_untrustedstring(ab, xattr->name);
+
if (strcmp(xattr->name, ".") == 0) {
evm_xattrs_locked = 1;
newattrs.ia_mode = S_IFREG | 0440;
@@ -222,15 +226,11 @@ static ssize_t evm_write_xattrs(struct file *file, const char __user *buf,
inode_lock(inode);
err = simple_setattr(evm_xattrs, &newattrs);
inode_unlock(inode);
- audit_log_format(ab, "locked");
if (!err)
err = count;
goto out;
}
- audit_log_format(ab, "xattr=");
- audit_log_untrustedstring(ab, xattr->name);
-
if (strncmp(xattr->name, XATTR_SECURITY_PREFIX,
XATTR_SECURITY_PREFIX_LEN) != 0) {
err = -EINVAL;
--
1.8.3.1
^ permalink raw reply related
* [PATCH V31 01/25] Add the ability to lock down access to the running kernel image
From: Matthew Garrett @ 2019-03-26 18:27 UTC (permalink / raw)
To: jmorris
Cc: linux-security-module, linux-kernel, dhowells, linux-api, luto,
Matthew Garrett
In-Reply-To: <20190326182742.16950-1-matthewgarrett@google.com>
From: David Howells <dhowells@redhat.com>
Provide a single call to allow kernel code to determine whether the system
should be locked down, thereby disallowing various accesses that might
allow the running kernel image to be changed including the loading of
modules that aren't validly signed with a key we recognise, fiddling with
MSR registers and disallowing hibernation.
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Matthew Garrett <matthewgarrett@google.com>
---
Documentation/ABI/testing/lockdown | 19 +++
.../admin-guide/kernel-parameters.txt | 9 ++
include/linux/kernel.h | 28 ++++
include/linux/security.h | 9 +-
init/main.c | 1 +
security/Kconfig | 39 +++++
security/Makefile | 3 +
security/lock_down.c | 147 ++++++++++++++++++
8 files changed, 254 insertions(+), 1 deletion(-)
create mode 100644 Documentation/ABI/testing/lockdown
create mode 100644 security/lock_down.c
diff --git a/Documentation/ABI/testing/lockdown b/Documentation/ABI/testing/lockdown
new file mode 100644
index 000000000000..5bd51e20917a
--- /dev/null
+++ b/Documentation/ABI/testing/lockdown
@@ -0,0 +1,19 @@
+What: security/lockdown
+Date: March 2019
+Contact: Matthew Garrett <mjg59@google.com>
+Description:
+ If CONFIG_LOCK_DOWN_KERNEL is enabled, the kernel can be
+ moved to a more locked down state at runtime by writing to
+ this attribute. Valid values are:
+
+ integrity:
+ The kernel will disable functionality that allows
+ userland to modify the running kernel image, other
+ than through the loading or execution of appropriately
+ signed objects.
+
+ confidentiality:
+ The kernel will disable all functionality disabled by
+ the integrity mode, but additionally will disable
+ features that potentially permit userland to obtain
+ confidential information stored within the kernel.
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 91c0251fdb86..594d268d92ba 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2213,6 +2213,15 @@
lockd.nlm_udpport=M [NFS] Assign UDP port.
Format: <integer>
+ lockdown= [SECURITY]
+ { integrity | confidentiality }
+ Enable the kernel lockdown feature. If set to
+ integrity, kernel features that allow userland to
+ modify the running kernel are disabled. If set to
+ confidentiality, kernel features that allow userland
+ to extract confidential information from the kernel
+ are also disabled.
+
locktorture.nreaders_stress= [KNL]
Set the number of locking read-acquisition kthreads.
Defaults to being automatically set based on the
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 8f0e68e250a7..30cf695719d5 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -340,6 +340,34 @@ static inline void refcount_error_report(struct pt_regs *regs, const char *err)
{ }
#endif
+enum lockdown_level {
+ LOCKDOWN_NONE,
+ LOCKDOWN_INTEGRITY,
+ LOCKDOWN_CONFIDENTIALITY,
+ LOCKDOWN_MAX,
+};
+
+#ifdef CONFIG_LOCK_DOWN_KERNEL
+extern bool __kernel_is_locked_down(const char *what,
+ enum lockdown_level level,
+ bool first);
+#else
+static inline bool __kernel_is_locked_down(const char *what,
+ enum lockdown_level level,
+ bool first)
+{
+ return false;
+}
+#endif
+
+#define kernel_is_locked_down(what, level) \
+ ({ \
+ static bool message_given; \
+ bool locked_down = __kernel_is_locked_down(what, level, !message_given); \
+ message_given = true; \
+ locked_down; \
+ })
+
/* Internal, do not use. */
int __must_check _kstrtoul(const char *s, unsigned int base, unsigned long *res);
int __must_check _kstrtol(const char *s, unsigned int base, long *res);
diff --git a/include/linux/security.h b/include/linux/security.h
index 13537a49ae97..b290946341a4 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1798,5 +1798,12 @@ static inline void security_bpf_prog_free(struct bpf_prog_aux *aux)
#endif /* CONFIG_SECURITY */
#endif /* CONFIG_BPF_SYSCALL */
-#endif /* ! __LINUX_SECURITY_H */
+#ifdef CONFIG_LOCK_DOWN_KERNEL
+extern void __init init_lockdown(void);
+#else
+static inline void __init init_lockdown(void)
+{
+}
+#endif
+#endif /* ! __LINUX_SECURITY_H */
diff --git a/init/main.c b/init/main.c
index e2e80ca3165a..4c6cca9681c7 100644
--- a/init/main.c
+++ b/init/main.c
@@ -555,6 +555,7 @@ asmlinkage __visible void __init start_kernel(void)
boot_cpu_init();
page_address_init();
pr_notice("%s", linux_banner);
+ init_lockdown();
setup_arch(&command_line);
/*
* Set up the the initial canary and entropy after arch
diff --git a/security/Kconfig b/security/Kconfig
index 1d6463fb1450..593ff231eac6 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -229,6 +229,45 @@ config STATIC_USERMODEHELPER_PATH
If you wish for all usermode helper programs to be disabled,
specify an empty string here (i.e. "").
+config LOCK_DOWN_KERNEL
+ bool "Allow the kernel to be 'locked down'"
+ help
+ Allow the kernel to be locked down. If lockdown support is enabled
+ and activated, the kernel will impose additional restrictions
+ intended to prevent uid 0 from being able to modify the running
+ kernel. This may break userland applications that rely on low-level
+ access to hardware.
+
+choice
+ prompt "Kernel default lockdown mode"
+ default LOCK_DOWN_KERNEL_FORCE_NONE
+ depends on LOCK_DOWN_KERNEL
+ help
+ The kernel can be configured to default to differing levels of
+ lockdown.
+
+config LOCK_DOWN_KERNEL_FORCE_NONE
+ bool "None"
+ help
+ No lockdown functionality is enabled by default. Lockdown may be
+ enabled via the kernel commandline or /sys/kernel/security/lockdown.
+
+config LOCK_DOWN_KERNEL_FORCE_INTEGRITY
+ bool "Integrity"
+ help
+ The kernel runs in integrity mode by default. Features that allow
+ the kernel to be modified at runtime are disabled.
+
+config LOCK_DOWN_KERNEL_FORCE_CONFIDENTIALITY
+ bool "Confidentiality"
+ help
+ The kernel runs in confidentiality mode by default. Features that
+ allow the kernel to be modified at runtime or that permit userland
+ code to read confidential material held inside the kernel are
+ disabled.
+
+endchoice
+
source "security/selinux/Kconfig"
source "security/smack/Kconfig"
source "security/tomoyo/Kconfig"
diff --git a/security/Makefile b/security/Makefile
index c598b904938f..5ff090149c88 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -32,3 +32,6 @@ obj-$(CONFIG_CGROUP_DEVICE) += device_cgroup.o
# Object integrity file lists
subdir-$(CONFIG_INTEGRITY) += integrity
obj-$(CONFIG_INTEGRITY) += integrity/
+
+# Allow the kernel to be locked down
+obj-$(CONFIG_LOCK_DOWN_KERNEL) += lock_down.o
diff --git a/security/lock_down.c b/security/lock_down.c
new file mode 100644
index 000000000000..0f9ef4c30aa8
--- /dev/null
+++ b/security/lock_down.c
@@ -0,0 +1,147 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Lock down the kernel
+ *
+ * Copyright (C) 2016 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#include <linux/security.h>
+#include <linux/export.h>
+
+static enum lockdown_level kernel_locked_down;
+
+char *lockdown_levels[LOCKDOWN_MAX] = {"none", "integrity", "confidentiality"};
+
+/*
+ * Put the kernel into lock-down mode.
+ */
+static int lock_kernel_down(const char *where, enum lockdown_level level)
+{
+ if (kernel_locked_down >= level)
+ return -EPERM;
+
+ kernel_locked_down = level;
+ pr_notice("Kernel is locked down from %s; see man kernel_lockdown.7\n",
+ where);
+ return 0;
+}
+
+static int __init lockdown_param(char *level)
+{
+ if (!level)
+ return -EINVAL;
+
+ if (strcmp(level, "integrity") == 0)
+ lock_kernel_down("command line", LOCKDOWN_INTEGRITY);
+ else if (strcmp(level, "confidentiality") == 0)
+ lock_kernel_down("command line", LOCKDOWN_CONFIDENTIALITY);
+ else
+ return -EINVAL;
+
+ return 0;
+}
+
+early_param("lockdown", lockdown_param);
+
+/*
+ * This must be called before arch setup code in order to ensure that the
+ * appropriate default can be applied without being overridden by the command
+ * line option.
+ */
+void __init init_lockdown(void)
+{
+#if defined(CONFIG_LOCK_DOWN_KERNEL_FORCE_INTEGRITY)
+ lock_kernel_down("Kernel configuration", LOCKDOWN_INTEGRITY);
+#elif defined(CONFIG_LOCK_DOWN_KERNEL_FORCE_CONFIDENTIALITY)
+ lock_kernel_down("Kernel configuration", LOCKDOWN_CONFIDENTIALITY);
+#endif
+}
+
+/**
+ * kernel_is_locked_down - Find out if the kernel is locked down
+ * @what: Tag to use in notice generated if lockdown is in effect
+ */
+bool __kernel_is_locked_down(const char *what, enum lockdown_level level,
+ bool first)
+{
+ if ((kernel_locked_down >= level) && what && first)
+ pr_notice("Lockdown: %s is restricted; see man kernel_lockdown.7\n",
+ what);
+ return (kernel_locked_down >= level);
+}
+EXPORT_SYMBOL(__kernel_is_locked_down);
+
+static ssize_t lockdown_read(struct file *filp, char __user *buf, size_t count,
+ loff_t *ppos)
+{
+ char temp[80];
+ int i, offset=0;
+
+ for (i = LOCKDOWN_NONE; i < LOCKDOWN_MAX; i++) {
+ if (lockdown_levels[i]) {
+ const char *label = lockdown_levels[i];
+
+ if (kernel_locked_down == i)
+ offset += sprintf(temp+offset, "[%s] ", label);
+ else
+ offset += sprintf(temp+offset, "%s ", label);
+ }
+ }
+
+ /* Convert the last space to a newline if needed. */
+ if (offset > 0)
+ temp[offset-1] = '\n';
+
+ return simple_read_from_buffer(buf, count, ppos, temp, strlen(temp));
+}
+
+static ssize_t lockdown_write(struct file *file, const char __user *buf,
+ size_t n, loff_t *ppos)
+{
+ char *state;
+ int i, len, err = 0;
+
+ state = memdup_user_nul(buf, n);
+ if (IS_ERR(state))
+ return PTR_ERR(state);
+
+ len = strlen(state);
+ if (state[len-1] == '\n') {
+ state[len-1] = '\0';
+ len--;
+ }
+
+ for (i = 0; i < LOCKDOWN_MAX; i++) {
+ const char *label = lockdown_levels[i];
+
+ if (label && len == strlen(label) && !strncmp(state, label, len))
+ err = lock_kernel_down("securityfs", i);
+ }
+
+ kfree(state);
+ return err ? err : n;
+}
+
+static const struct file_operations lockdown_ops = {
+ .read = lockdown_read,
+ .write = lockdown_write,
+};
+
+static int __init lockdown_secfs_init(void)
+{
+ struct dentry *dentry;
+
+ dentry = securityfs_create_file("lockdown", 0660, NULL, NULL,
+ &lockdown_ops);
+ if (IS_ERR(dentry))
+ return PTR_ERR(dentry);
+
+ return 0;
+}
+
+core_initcall(lockdown_secfs_init);
--
2.21.0.392.gf8f6787159e-goog
^ permalink raw reply related
* [PATCH V31 03/25] Restrict /dev/{mem,kmem,port} when the kernel is locked down
From: Matthew Garrett @ 2019-03-26 18:27 UTC (permalink / raw)
To: jmorris
Cc: linux-security-module, linux-kernel, dhowells, linux-api, luto,
Matthew Garrett, Matthew Garrett, x86
In-Reply-To: <20190326182742.16950-1-matthewgarrett@google.com>
From: Matthew Garrett <mjg59@srcf.ucam.org>
Allowing users to read and write to core kernel memory makes it possible
for the kernel to be subverted, avoiding module loading restrictions, and
also to steal cryptographic information.
Disallow /dev/mem and /dev/kmem from being opened this when the kernel has
been locked down to prevent this.
Also disallow /dev/port from being opened to prevent raw ioport access and
thus DMA from being used to accomplish the same thing.
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Matthew Garrett <mjg59@google.com>
Cc: x86@kernel.org
---
drivers/char/mem.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index b08dc50f9f26..67b85939b1bd 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -786,6 +786,8 @@ static loff_t memory_lseek(struct file *file, loff_t offset, int orig)
static int open_port(struct inode *inode, struct file *filp)
{
+ if (kernel_is_locked_down("/dev/mem,kmem,port", LOCKDOWN_INTEGRITY))
+ return -EPERM;
return capable(CAP_SYS_RAWIO) ? 0 : -EPERM;
}
--
2.21.0.392.gf8f6787159e-goog
^ 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