* Re: [PATCH 23/27] bpf: Restrict kernel image access functions when the kernel is locked down
From: Daniel Borkmann @ 2019-03-26 0:00 UTC (permalink / raw)
To: Stephen Hemminger, Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, dhowells,
Alexei Starovoitov, netdev, Chun-Yi Lee, Kees Cook,
Andy Lutomirski, Will Drewry
In-Reply-To: <20190325164221.5d8687bd@shemminger-XPS-13-9360>
On 03/26/2019 12:42 AM, Stephen Hemminger wrote:
> On Mon, 25 Mar 2019 15:09:50 -0700
> 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.
I'm not sure where 'kernel memory to be altered without restriction' comes
from, but it's definitely a wrong statement.
>> Completely prohibit the use of BPF when the kernel is locked down.
In which scenarios will the lock-down mode be used? Mostly niche? I'm asking
as this would otherwise break a lot of existing stuff ... I'd prefer you find
a better solution to this than this straight -EPERM rejection.
>> Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
>> Signed-off-by: David Howells <dhowells@redhat.com>
>> cc: netdev@vger.kernel.org
>> cc: Chun-Yi Lee <jlee@suse.com>
>> cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
>> Cc: Daniel Borkmann <daniel@iogearbox.net>
>> Signed-off-by: Matthew Garrett <matthewgarrett@google.com>
>
> Wouldn't this mean that Seccomp won't work in locked down mode?
>
^ 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 0:10 UTC (permalink / raw)
To: Stephen Hemminger, Linux API
Cc: Matthew Garrett, James Morris, LSM List, LKML, David Howells,
Alexei Starovoitov, Network Development, Chun-Yi Lee,
Daniel Borkmann, Kees Cook, Will Drewry
In-Reply-To: <20190325164221.5d8687bd@shemminger-XPS-13-9360>
On Mon, Mar 25, 2019 at 4:42 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Mon, 25 Mar 2019 15:09:50 -0700
> 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.
> >
> > Completely prohibit the use of BPF when the kernel is locked down.
> >
> > Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> > Signed-off-by: David Howells <dhowells@redhat.com>
> > cc: netdev@vger.kernel.org
> > cc: Chun-Yi Lee <jlee@suse.com>
> > cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> > Cc: Daniel Borkmann <daniel@iogearbox.net>
> > Signed-off-by: Matthew Garrett <matthewgarrett@google.com>
>
> Wouldn't this mean that Seccomp won't work in locked down mode?
I wasn't cc'd on this series, nor was linux-api, so it's awkward to review.
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.
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.
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.
--Andy
^ permalink raw reply
* [PATCH 1/1] RFC: security: add SECURE_KEEP_FSUID to preserve fsuid/fsgid across execve
From: Igor Lubashev @ 2019-03-25 23:07 UTC (permalink / raw)
To: Serge Hallyn, James Morris, Igor Lubashev, open list,
open list:CAPABILITIES
In-Reply-To: <1553555278-19153-1-git-send-email-ilubashe@akamai.com>
Many kernel interfaces require real and/or effective root uid instead
of relying solely of capabilities. An executable that uses such
interfaces has to be set-uid-root or be executed by a thread with
effective root uid. Presently, fsuid and saved uid will reset to the
effective uid during execve. As a result, it is not possible to
execute a binary such that it has effective root uid but retains
fsuid/fsgid of the actual user. Retaining fsuid/fsgid of the actual
user could be required if the executable needs to access the
filesystem. It may also be desired from the security perspective of
delegating the minimal set of privileges.
Setting SECURE_KEEP_FSUID bit ensures that the current fsuid/fsgiud is
retained by execve.
Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
---
include/uapi/linux/securebits.h | 10 +++++++++-
security/commoncap.c | 9 +++++++--
2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/include/uapi/linux/securebits.h b/include/uapi/linux/securebits.h
index d6d9887..6c20b22 100644
--- a/include/uapi/linux/securebits.h
+++ b/include/uapi/linux/securebits.h
@@ -52,10 +52,18 @@
#define SECBIT_NO_CAP_AMBIENT_RAISE_LOCKED \
(issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE_LOCKED))
+/* When set, execve does not reset fsuid/fsgid to euid/egid */
+#define SECURE_KEEP_FSUID 8
+#define SECURE_KEEP_FSUID_LOCKED 9 /* make bit-8 immutable */
+
+#define SECBIT_KEEP_FSUID (issecure_mask(SECURE_KEEP_FSUID))
+#define SECBIT_KEEP_FSUID_LOCKED (issecure_mask(SECURE_KEEP_FSUID_LOCKED))
+
#define SECURE_ALL_BITS (issecure_mask(SECURE_NOROOT) | \
issecure_mask(SECURE_NO_SETUID_FIXUP) | \
issecure_mask(SECURE_KEEP_CAPS) | \
- issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE))
+ issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE) | \
+ issecure_mask(SECURE_KEEP_FSUID))
#define SECURE_ALL_LOCKS (SECURE_ALL_BITS << 1)
#endif /* _UAPI_LINUX_SECUREBITS_H */
diff --git a/security/commoncap.c b/security/commoncap.c
index c477fb6..490a7b4 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -852,8 +852,13 @@ int cap_bprm_set_creds(struct linux_binprm *bprm)
old->cap_permitted);
}
- new->suid = new->fsuid = new->euid;
- new->sgid = new->fsgid = new->egid;
+ new->suid = new->euid;
+ new->sgid = new->egid;
+
+ if (!issecure(SECURE_KEEP_FSUID)) {
+ new->fsuid = new->euid;
+ new->fsgid = new->egid;
+ }
/* File caps or setid cancels ambient. */
if (has_fcap || is_setid)
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 25/27] debugfs: Restrict debugfs when the kernel is locked down
From: Greg Kroah-Hartman @ 2019-03-26 0:31 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, dhowells,
Andy Shevchenko, acpi4asus-user, platform-driver-x86,
Matthew Garrett, Thomas Gleixner
In-Reply-To: <20190325220954.29054-26-matthewgarrett@google.com>
On Mon, Mar 25, 2019 at 03:09:52PM -0700, Matthew Garrett wrote:
> From: David Howells <dhowells@redhat.com>
>
> Disallow opening of debugfs files that might be used to muck around when
> the kernel is locked down as various drivers give raw access to hardware
> through debugfs. Given the effort of auditing all 2000 or so files and
> manually fixing each one as necessary, I've chosen to apply a heuristic
> instead. The following changes are made:
>
> (1) chmod and chown are disallowed on debugfs objects (though the root dir
> can be modified by mount and remount, but I'm not worried about that).
>
> (2) When the kernel is locked down, only files with the following criteria
> are permitted to be opened:
>
> - The file must have mode 00444
> - The file must not have ioctl methods
> - The file must not have mmap
>
> (3) When the kernel is locked down, files may only be opened for reading.
>
> Normal device interaction should be done through configfs, sysfs or a
> miscdev, not debugfs.
Then why not just not allow debugfs at all if it is such a "big
problem"?
Actually, it is a big problem, and I am pushing for systems to not mount
debugfs at all these days, but that's a different story...
Just "disabling" it entirely for this crazy dream of a "locked down"
kernel is fine for me.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 25/27] debugfs: Restrict debugfs when the kernel is locked down
From: Matthew Garrett @ 2019-03-26 0:38 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: James Morris, LSM List, Linux Kernel Mailing List, David Howells,
Andy Shevchenko, acpi4asus-user, platform-driver-x86,
Matthew Garrett, Thomas Gleixner
In-Reply-To: <20190326003137.GB5112@kroah.com>
On Mon, Mar 25, 2019 at 5:35 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Mon, Mar 25, 2019 at 03:09:52PM -0700, Matthew Garrett wrote:
> > Normal device interaction should be done through configfs, sysfs or a
> > miscdev, not debugfs.
>
> Then why not just not allow debugfs at all if it is such a "big
> problem"?
Previous attempts to do so have resulted in strong pushback from
various maintainers. If you're happy just having any complaints
reassigned to you then I'm more than happy to turn it off entirely.
^ permalink raw reply
* Re: [PATCH 25/27] debugfs: Restrict debugfs when the kernel is locked down
From: Greg Kroah-Hartman @ 2019-03-26 0:43 UTC (permalink / raw)
To: Matthew Garrett
Cc: James Morris, LSM List, Linux Kernel Mailing List, David Howells,
Andy Shevchenko, acpi4asus-user, platform-driver-x86,
Matthew Garrett, Thomas Gleixner
In-Reply-To: <CACdnJus6a_7VUR27K7dJE037b3TPwDNMh1AyAOptHepB18vjxw@mail.gmail.com>
On Mon, Mar 25, 2019 at 05:38:09PM -0700, Matthew Garrett wrote:
> On Mon, Mar 25, 2019 at 5:35 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Mon, Mar 25, 2019 at 03:09:52PM -0700, Matthew Garrett wrote:
> > > Normal device interaction should be done through configfs, sysfs or a
> > > miscdev, not debugfs.
> >
> > Then why not just not allow debugfs at all if it is such a "big
> > problem"?
>
> Previous attempts to do so have resulted in strong pushback from
> various maintainers. If you're happy just having any complaints
> reassigned to you then I'm more than happy to turn it off entirely.
Sure, send them my way :)
^ permalink raw reply
* Re: [PATCH 01/27] Add the ability to lock down access to the running kernel image
From: Matthew Garrett @ 2019-03-26 5:30 UTC (permalink / raw)
To: James Morris
Cc: LSM List, Linux Kernel Mailing List, David Howells, James Morris
In-Reply-To: <20190325220954.29054-2-matthewgarrett@google.com>
Based on Andy's feedback, I'm going to rework this slightly and
re-send tomorrow.
^ permalink raw reply
* Re: [PATCH] KEYS: trusted: allow trusted.ko to initialize w/o a TPM
From: Jarkko Sakkinen @ 2019-03-26 11:50 UTC (permalink / raw)
To: Dan Williams
Cc: linux-integrity, linux-security-module, stable, James Bottomley,
Mimi Zohar, David Howells, James Morris, Serge E. Hallyn,
open list:KEYS-TRUSTED, open list
In-Reply-To: <CAPcyv4gCvqkAUSb2sC1+hoHF=bu5X8AXvJuRoqcEt4OQGvWk5A@mail.gmail.com>
On Mon, Mar 25, 2019 at 02:33:38PM -0700, Dan Williams wrote:
> On Mon, Mar 25, 2019 at 7:48 AM Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> >
> > Allow trusted.ko to initialize w/o a TPM. This commit adds checks to the
> > key type callbacks and exported functions to fail when a TPM is not
> > available.
> >
> > Cc: Dan Williams <dan.j.williams@intel.com>
>
> Reported-and-tested-by: Dan Williams <dan.j.williams@intel.com>
Thanks Dan!
/Jarkko
^ permalink raw reply
* Re: [PATCH] KEYS: trusted: allow trusted.ko to initialize w/o a TPM
From: Jarkko Sakkinen @ 2019-03-26 11:52 UTC (permalink / raw)
To: linux-integrity
Cc: linux-security-module, Dan Williams, stable, James Bottomley,
Mimi Zohar, David Howells, James Morris, Serge E. Hallyn,
open list:KEYS-TRUSTED, open list
In-Reply-To: <20190325144735.30443-1-jarkko.sakkinen@linux.intel.com>
On Mon, Mar 25, 2019 at 04:47:35PM +0200, Jarkko Sakkinen wrote:
> Allow trusted.ko to initialize w/o a TPM. This commit adds checks to the
> key type callbacks and exported functions to fail when a TPM is not
> available.
>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: stable@vger.kernel.org
> Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...")
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Will send a revised version: since now the key type is not registered,
none of the callbacks needs the check for the chip instance. Only the
exported functions should have it.
/Jarkko
^ permalink raw reply
* [PATCH v2] KEYS: trusted: allow trusted.ko to initialize w/o a TPM
From: Jarkko Sakkinen @ 2019-03-26 12:09 UTC (permalink / raw)
To: linux-integrity
Cc: linux-security-module, Jarkko Sakkinen, James Morris,
James Bottomley, Mimi Zohar, David Howells, Serge E. Hallyn,
open list:KEYS-TRUSTED, open list
Allow trusted.ko to initialize w/o a TPM. This commit adds checks to the
key type callbacks and exported functions to fail when a TPM is not
available.
Cc: James Morris <jmorris@namei.org>
Reported-by: Dan Williams <dan.j.williams@intel.com>
Tested-by: Dan Williams <dan.j.williams@intel.com>
Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...")
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
v2:
- Remove checks from the key type callbacks because the key type will
not be registered.
- Cc to James Morris because hopefully we can land this 5.1.0.
security/keys/trusted.c | 31 +++++++++++++++++++++++++------
1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index ecec672d3a77..67c0e19a2125 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -135,6 +135,9 @@ int TSS_authhmac(unsigned char *digest, const unsigned char *key,
int ret;
va_list argp;
+ if (!chip)
+ return -ENODEV;
+
sdesc = init_sdesc(hashalg);
if (IS_ERR(sdesc)) {
pr_info("trusted_key: can't alloc %s\n", hash_alg);
@@ -196,6 +199,9 @@ int TSS_checkhmac1(unsigned char *buffer,
va_list argp;
int ret;
+ if (!chip)
+ return -ENODEV;
+
bufsize = LOAD32(buffer, TPM_SIZE_OFFSET);
tag = LOAD16(buffer, 0);
ordinal = command;
@@ -363,6 +369,9 @@ int trusted_tpm_send(unsigned char *cmd, size_t buflen)
{
int rc;
+ if (!chip)
+ return -ENODEV;
+
dump_tpm_buf(cmd);
rc = tpm_send(chip, cmd, buflen);
dump_tpm_buf(cmd);
@@ -429,6 +438,9 @@ int oiap(struct tpm_buf *tb, uint32_t *handle, unsigned char *nonce)
{
int ret;
+ if (!chip)
+ return -ENODEV;
+
INIT_BUF(tb);
store16(tb, TPM_TAG_RQU_COMMAND);
store32(tb, TPM_OIAP_SIZE);
@@ -1245,9 +1257,13 @@ static int __init init_trusted(void)
{
int ret;
+ /* encrypted_keys.ko depends on successful load of this module even if
+ * TPM is not used.
+ */
chip = tpm_default_chip();
if (!chip)
- return -ENOENT;
+ return 0;
+
ret = init_digests();
if (ret < 0)
goto err_put;
@@ -1263,16 +1279,19 @@ static int __init init_trusted(void)
err_free:
kfree(digests);
err_put:
- put_device(&chip->dev);
+ if (chip)
+ put_device(&chip->dev);
return ret;
}
static void __exit cleanup_trusted(void)
{
- put_device(&chip->dev);
- kfree(digests);
- trusted_shash_release();
- unregister_key_type(&key_type_trusted);
+ if (chip) {
+ put_device(&chip->dev);
+ kfree(digests);
+ trusted_shash_release();
+ unregister_key_type(&key_type_trusted);
+ }
}
late_initcall(init_trusted);
--
2.19.1
^ permalink raw reply related
* [PATCH v3] KEYS: trusted: allow trusted.ko to initialize w/o a TPM
From: Jarkko Sakkinen @ 2019-03-26 12:11 UTC (permalink / raw)
To: linux-integrity
Cc: linux-security-module, Jarkko Sakkinen, James Morris,
James Bottomley, Mimi Zohar, David Howells, Serge E. Hallyn,
open list:KEYS-TRUSTED, open list
Allow trusted.ko to initialize w/o a TPM. This commit adds checks to the
key type callbacks and exported functions to fail when a TPM is not
available.
Cc: James Morris <jmorris@namei.org>
Reported-by: Dan Williams <dan.j.williams@intel.com>
Tested-by: Dan Williams <dan.j.williams@intel.com>
Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...")
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
v3:
- remove unnecessary check for chip in init_trusted()
v2:
- Remove checks from the key type callbacks because the key type will
not be registered.
- Cc to James Morris because hopefully we can land this 5.1.0.
security/keys/trusted.c | 28 +++++++++++++++++++++++-----
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index ecec672d3a77..efdbf17f3915 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -135,6 +135,9 @@ int TSS_authhmac(unsigned char *digest, const unsigned char *key,
int ret;
va_list argp;
+ if (!chip)
+ return -ENODEV;
+
sdesc = init_sdesc(hashalg);
if (IS_ERR(sdesc)) {
pr_info("trusted_key: can't alloc %s\n", hash_alg);
@@ -196,6 +199,9 @@ int TSS_checkhmac1(unsigned char *buffer,
va_list argp;
int ret;
+ if (!chip)
+ return -ENODEV;
+
bufsize = LOAD32(buffer, TPM_SIZE_OFFSET);
tag = LOAD16(buffer, 0);
ordinal = command;
@@ -363,6 +369,9 @@ int trusted_tpm_send(unsigned char *cmd, size_t buflen)
{
int rc;
+ if (!chip)
+ return -ENODEV;
+
dump_tpm_buf(cmd);
rc = tpm_send(chip, cmd, buflen);
dump_tpm_buf(cmd);
@@ -429,6 +438,9 @@ int oiap(struct tpm_buf *tb, uint32_t *handle, unsigned char *nonce)
{
int ret;
+ if (!chip)
+ return -ENODEV;
+
INIT_BUF(tb);
store16(tb, TPM_TAG_RQU_COMMAND);
store32(tb, TPM_OIAP_SIZE);
@@ -1245,9 +1257,13 @@ static int __init init_trusted(void)
{
int ret;
+ /* encrypted_keys.ko depends on successful load of this module even if
+ * TPM is not used.
+ */
chip = tpm_default_chip();
if (!chip)
- return -ENOENT;
+ return 0;
+
ret = init_digests();
if (ret < 0)
goto err_put;
@@ -1269,10 +1285,12 @@ static int __init init_trusted(void)
static void __exit cleanup_trusted(void)
{
- put_device(&chip->dev);
- kfree(digests);
- trusted_shash_release();
- unregister_key_type(&key_type_trusted);
+ if (chip) {
+ put_device(&chip->dev);
+ kfree(digests);
+ trusted_shash_release();
+ unregister_key_type(&key_type_trusted);
+ }
}
late_initcall(init_trusted);
--
2.19.1
^ permalink raw reply related
* Re: [PATCH 22/27] Lock down kprobes
From: Masami Hiramatsu @ 2019-03-26 12:29 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, dhowells,
Alexei Starovoitov, Naveen N . Rao, Anil S Keshavamurthy, davem,
Masami Hiramatsu
In-Reply-To: <20190325220954.29054-23-matthewgarrett@google.com>
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.
Thank you,
>
> Reported-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Matthew Garrett <matthewgarrett@google.com>
> Cc: Naveen N. Rao <naveen.n.rao@linux.ibm.com>
> Cc: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
> Cc: davem@davemloft.net
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> ---
> kernel/kprobes.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index f4ddfdd2d07e..6f66cca8e2c6 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1552,6 +1552,9 @@ int register_kprobe(struct kprobe *p)
> struct module *probed_mod;
> kprobe_opcode_t *addr;
>
> + if (kernel_is_locked_down("Use of kprobes"))
> + return -EPERM;
> +
> /* Adjust probe address from symbol */
> addr = kprobe_addr(p);
> if (IS_ERR(addr))
> --
> 2.21.0.392.gf8f6787159e-goog
>
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH 23/27] bpf: Restrict kernel image access functions when the kernel is locked down
From: Jordan Glover @ 2019-03-26 13:54 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Stephen Hemminger, Matthew Garrett, jmorris@namei.org,
linux-security-module@vger.kernel.org,
linux-kernel@vger.kernel.org, dhowells@redhat.com,
Alexei Starovoitov, netdev@vger.kernel.org, Chun-Yi Lee,
Kees Cook, Andy Lutomirski, Will Drewry
In-Reply-To: <1cfa7345-c807-db76-f50a-ea3ba70f07b2@iogearbox.net>
On Tuesday, March 26, 2019 12:00 AM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 03/26/2019 12:42 AM, Stephen Hemminger wrote:
>
> > On Mon, 25 Mar 2019 15:09:50 -0700
> > 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.
>
> I'm not sure where 'kernel memory to be altered without restriction' comes
> from, but it's definitely a wrong statement.
>
> > > Completely prohibit the use of BPF when the kernel is locked down.
>
> In which scenarios will the lock-down mode be used? Mostly niche? I'm asking
> as this would otherwise break a lot of existing stuff ... I'd prefer you find
> a better solution to this than this straight -EPERM rejection.
AFAIK this change breaks IPAddressAllow/IPAddressDeny usage in systemd services
which makes them LESS secure.
https://www.freedesktop.org/software/systemd/man/systemd.resource-control.html
https://github.com/systemd/systemd/blob/04d7ca022843913fba5170c40be07acf2ab5902b/README#L96
Jordan
^ permalink raw reply
* Re: [PATCH v2 00/11] LSM documentation update
From: Denis Efremov @ 2019-03-26 14:38 UTC (permalink / raw)
To: Jonathan Corbet
Cc: Casey Schaufler, Kees Cook, Eric W. Biederman, Eric Paris,
John Johansen, James Morris, Serge E. Hallyn, Paul Moore,
Kentaro Takeda, linux-security-module, linux-kernel,
open list:DOCUMENTATION
In-Reply-To: <cover.1551213058.git.efremov@ispras.ru>
Is there something more I could do so that these changes are accepted?
At least this patchset fixes the documentation format for better html
generation.
Denis
26.02.2019 21:49, Denis Efremov пишет:
> 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.
>
> Denis Efremov (11):
> LSM: fix documentation for sb_copy_data hook
> LSM: fix documentation for the syslog hook
> LSM: fix documentation for the socket_post_create hook
> LSM: fix documentation for the task_setscheduler hook
> LSM: fix documentation for the socket_getpeersec_dgram hook
> LSM: fix documentation for the path_chmod hook
> LSM: fix documentation for the audit_* hooks
> LSM: fix documentation for the msg_queue_* hooks
> LSM: fix documentation for the sem_* hooks
> LSM: fix documentation for the shm_* hooks
> LSM: lsm_hooks.h: fix documentation format
>
> include/linux/lsm_hooks.h | 170 ++++++++++++++++++--------------------
> 1 file changed, 81 insertions(+), 89 deletions(-)
>
^ permalink raw reply
* Re: [PATCH v2 00/11] LSM documentation update
From: Jonathan Corbet @ 2019-03-26 14:53 UTC (permalink / raw)
To: Denis Efremov
Cc: Casey Schaufler, Kees Cook, Eric W. Biederman, Eric Paris,
John Johansen, James Morris, Serge E. Hallyn, Paul Moore,
Kentaro Takeda, linux-security-module, linux-kernel,
open list:DOCUMENTATION
In-Reply-To: <f89f1335-a76a-b364-d426-b0fe3423ad0a@ispras.ru>
On Tue, 26 Mar 2019 15:38:21 +0100
Denis Efremov <efremov@ispras.ru> wrote:
> Is there something more I could do so that these changes are accepted?
> At least this patchset fixes the documentation format for better html
> generation.
I had assumed that this would go through the security tree, but can
certainly pick it up if that works better.
jon
^ permalink raw reply
* Re: [PATCH ghak109 V1] audit: link integrity evm_write_xattrs record to syscall event
From: Mimi Zohar @ 2019-03-26 15:11 UTC (permalink / raw)
To: Richard Guy Briggs, Paul Moore
Cc: linux-integrity, linux-security-module, Linux-Audit Mailing List,
LKML, sgrubb, omosnace, mjg59
In-Reply-To: <20190321005008.wfz3bk7q262km5fz@madcap2.tricolour.ca>
On Wed, 2019-03-20 at 20:50 -0400, Richard Guy Briggs wrote:
> On 2019-03-20 19:48, Paul Moore wrote:
> > On Sat, Mar 16, 2019 at 8:10 AM Richard Guy Briggs <rgb@redhat.com> 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 "xattr=(locked)".
> > >
> > > Please see the github issue
> > > https://github.com/linux-audit/audit-kernel/issues/109
> > >
> > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > > ---
> > > security/integrity/evm/evm_secfs.c | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c
> > > index 015aea8fdf1e..4171d174e9da 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);
> >
> > This part is fine.
> >
> > > if (!ab)
> > > return -ENOMEM;
> > >
> > > @@ -222,7 +223,7 @@ 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");
> > > + audit_log_format(ab, "xattr=(locked)");
> >
> > Two things come to mind:
> >
> > * While we can clearly trust the string above, should we be logging
> > the xattr field value as an untrusted string so it is consistent with
> > how we record other xattr names?
>
> That would be a question for Steve.
>
> > * I'm not sure you can ever have parens in a xattr (I would hope not),
> > but if we are going to use the xattr field, perhaps we should simply
> > stick with the name as provided (".") so we don't ever run afoul of
> > xattr names? I'm curious to hear what the IMA/EVM folks think of
> > this.
>
> The legal xaddr names start with XATTR_SECURITY_PREFIX which is
> "security." so there is no danger of collision with legal names, but I
> suppose someone could try to use "(locked)" as a name which would look
> identical but fail with a different res= number. I think I prefer your
> idea of printing the given value verbatim.
I really don't have a preference - "locked", "(locked)", "." or "(.)".
Any of them is fine.
Thanks!
Mimi
^ permalink raw reply
* Re: [PATCH ghak109 V1] audit: link integrity evm_write_xattrs record to syscall event
From: Steve Grubb @ 2019-03-26 15:22 UTC (permalink / raw)
To: Richard Guy Briggs
Cc: Paul Moore, linux-integrity, linux-security-module,
Linux-Audit Mailing List, LKML, omosnace, Eric Paris,
Serge Hallyn, zohar, mjg59
In-Reply-To: <20190321005008.wfz3bk7q262km5fz@madcap2.tricolour.ca>
On Wednesday, March 20, 2019 8:50:08 PM EDT Richard Guy Briggs wrote:
> On 2019-03-20 19:48, Paul Moore wrote:
> > On Sat, Mar 16, 2019 at 8:10 AM Richard Guy Briggs <rgb@redhat.com>
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 "xattr=(locked)".
> > >
> > > Please see the github issue
> > > https://github.com/linux-audit/audit-kernel/issues/109
> > >
> > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > > ---
> > >
> > > security/integrity/evm/evm_secfs.c | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/security/integrity/evm/evm_secfs.c
> > > b/security/integrity/evm/evm_secfs.c index 015aea8fdf1e..4171d174e9da
> > > 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);
> >
> > This part is fine.
> >
> > > if (!ab)
> > >
> > > return -ENOMEM;
> > >
> > > @@ -222,7 +223,7 @@ 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");
> > > + audit_log_format(ab, "xattr=(locked)");
> >
> > Two things come to mind:
> >
> > * While we can clearly trust the string above, should we be logging
> > the xattr field value as an untrusted string so it is consistent with
> > how we record other xattr names?
>
> That would be a question for Steve.
All fields with the same name must be represented the same way. If one
instance is untrusted, every instance of the same keyword must be untrusted.
-Steve
> > * I'm not sure you can ever have parens in a xattr (I would hope not),
> > but if we are going to use the xattr field, perhaps we should simply
> > stick with the name as provided (".") so we don't ever run afoul of
> > xattr names? I'm curious to hear what the IMA/EVM folks think of
> > this.
>
> The legal xaddr names start with XATTR_SECURITY_PREFIX which is
> "security." so there is no danger of collision with legal names, but I
> suppose someone could try to use "(locked)" as a name which would look
> identical but fail with a different res= number. I think I prefer your
> idea of printing the given value verbatim.
>
> > paul moore
>
> - RGB
>
> --
> Richard Guy Briggs <rgb@redhat.com>
> Sr. S/W Engineer, Kernel Security, Base Operating Systems
> Remote, Ottawa, Red Hat Canada
> IRC: rgb, SunRaycer
> Voice: +1.647.777.2635, Internal: (81) 32635
^ permalink raw reply
* Re: [PATCH ghak109 V1] audit: link integrity evm_write_xattrs record to syscall event
From: Mimi Zohar @ 2019-03-26 15:29 UTC (permalink / raw)
To: Steve Grubb, Richard Guy Briggs
Cc: Paul Moore, linux-integrity, linux-security-module,
Linux-Audit Mailing List, LKML, omosnace, Eric Paris,
Serge Hallyn, mjg59
In-Reply-To: <2006016.NXIvICiRTL@x2>
On Tue, 2019-03-26 at 11:22 -0400, Steve Grubb wrote:
> > > > --- 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);
> > >
> > > This part is fine.
> > >
> > > > if (!ab)
> > > >
> > > > return -ENOMEM;
> > > >
> > > > @@ -222,7 +223,7 @@ 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");
> > > > + audit_log_format(ab, "xattr=(locked)");
> > >
> > > Two things come to mind:
> > >
> > > * While we can clearly trust the string above, should we be logging
> > > the xattr field value as an untrusted string so it is consistent with
> > > how we record other xattr names?
> >
> > That would be a question for Steve.
>
> All fields with the same name must be represented the same way. If one
> instance is untrusted, every instance of the same keyword must be untrusted.
Normal case:
audit_log_format(ab, "xattr=");
audit_log_untrustedstring(ab, xattr->name);
Ok, so the above audit_log_format() call should be replaced with
audit_log_untrustedstring().
Mimi
^ permalink raw reply
* Re: [PATCH 27/27] kexec: Allow kexec_file() with appropriate IMA policy when locked down
From: Mimi Zohar @ 2019-03-26 15:33 UTC (permalink / raw)
To: Matthew Garrett, jmorris
Cc: linux-security-module, linux-kernel, dhowells, Matthew Garrett,
Dmitry Kasatkin, linux-integrity
In-Reply-To: <20190325220954.29054-28-matthewgarrett@google.com>
On Mon, 2019-03-25 at 15:09 -0700, Matthew Garrett wrote:
> Systems in lockdown mode should block the kexec of untrusted kernels.
> For x86 and ARM we can ensure that a kernel is trustworthy by validating
> a PE signature, but this isn't possible on other architectures. On those
> platforms we can use IMA digital signatures instead. Add a function to
> determine whether IMA has or will verify signatures for a given event type,
> and if so permit kexec_file() even if the kernel is otherwise locked down.
> This is restricted to cases where CONFIG_INTEGRITY_TRUSTED_KEYRING is set
> in order to prevent an attacker from loading additional keys at runtime.
>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> Cc: Mimi Zohar <zohar@linux.ibm.com>
> Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
> Cc: linux-integrity@vger.kernel.org
Acked-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
> include/linux/ima.h | 9 ++++++
> kernel/kexec_file.c | 7 +++-
> security/integrity/ima/ima.h | 2 ++
> security/integrity/ima/ima_main.c | 2 +-
> security/integrity/ima/ima_policy.c | 50 +++++++++++++++++++++++++++++
> 5 files changed, 68 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index b5e16b8c50b7..05921227d700 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -127,4 +127,13 @@ static inline int ima_inode_removexattr(struct dentry *dentry,
> return 0;
> }
> #endif /* CONFIG_IMA_APPRAISE */
> +
> +#if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING)
> +extern bool ima_appraise_signature(enum kernel_read_file_id func);
> +#else
> +static inline bool ima_appraise_kexec_signature(enum kernel_read_file_id func)
> +{
> + return false;
> +}
> +#endif /* CONFIG_IMA_APPRAISE && CONFIG_INTEGRITY_TRUSTED_KEYRING */
> #endif /* _LINUX_IMA_H */
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 0cfe4f6f7f85..8ffa4b75c620 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -240,7 +240,12 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
>
> ret = 0;
>
> - if (kernel_is_locked_down(reason)) {
> + /* If IMA is guaranteed to appraise a signature on the kexec
> + * image, permit it even if the kernel is otherwise locked
> + * down.
> + */
> + if (!ima_appraise_signature(READING_KEXEC_IMAGE) &&
> + kernel_is_locked_down(reason)) {
> ret = -EPERM;
> goto out;
> }
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index cc12f3449a72..fe03cc6f1ca4 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -115,6 +115,8 @@ struct ima_kexec_hdr {
> u64 count;
> };
>
> +extern const int read_idmap[];
> +
> #ifdef CONFIG_HAVE_IMA_KEXEC
> void ima_load_kexec_buffer(void);
> #else
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 4ffac4f5c647..106f06dee9d1 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -442,7 +442,7 @@ int ima_read_file(struct file *file, enum kernel_read_file_id read_id)
> return 0;
> }
>
> -static const int read_idmap[READING_MAX_ID] = {
> +const int read_idmap[READING_MAX_ID] = {
> [READING_FIRMWARE] = FIRMWARE_CHECK,
> [READING_FIRMWARE_PREALLOC_BUFFER] = FIRMWARE_CHECK,
> [READING_MODULE] = MODULE_CHECK,
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index 122797023bdb..f8f1cdb74a4f 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -1341,3 +1341,53 @@ int ima_policy_show(struct seq_file *m, void *v)
> return 0;
> }
> #endif /* CONFIG_IMA_READ_POLICY */
> +
> +#if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING)
> +/*
> + * ima_appraise_signature: whether IMA will appraise a given function using
> + * an IMA digital signature. This is restricted to cases where the kernel
> + * has a set of built-in trusted keys in order to avoid an attacker simply
> + * loading additional keys.
> + */
> +bool ima_appraise_signature(enum kernel_read_file_id id)
> +{
> + struct ima_rule_entry *entry;
> + bool found = false;
> + enum ima_hooks func;
> +
> + if (id >= READING_MAX_ID)
> + return false;
> +
> + func = read_idmap[id] ?: FILE_CHECK;
> +
> + rcu_read_lock();
> + list_for_each_entry_rcu(entry, ima_rules, list) {
> + if (entry->action != APPRAISE)
> + continue;
> +
> + /*
> + * A generic entry will match, but otherwise require that it
> + * match the func we're looking for
> + */
> + if (entry->func && entry->func != func)
> + continue;
> +
> + /*
> + * We require this to be a digital signature, not a raw IMA
> + * hash.
> + */
> + if (entry->flags & IMA_DIGSIG_REQUIRED)
> + found = true;
> +
> + /*
> + * We've found a rule that matches, so break now even if it
> + * didn't require a digital signature - a later rule that does
> + * won't override it, so would be a false positive.
> + */
> + break;
> + }
> +
> + rcu_read_unlock();
> + return found;
> +}
> +#endif /* CONFIG_IMA_APPRAISE && CONFIG_INTEGRITY_TRUSTED_KEYRING */
^ permalink raw reply
* Re: [PATCH ghak109 V1] audit: link integrity evm_write_xattrs record to syscall event
From: Richard Guy Briggs @ 2019-03-26 16:14 UTC (permalink / raw)
To: Mimi Zohar
Cc: Steve Grubb, Paul Moore, linux-integrity, linux-security-module,
Linux-Audit Mailing List, LKML, omosnace, Eric Paris,
Serge Hallyn, mjg59
In-Reply-To: <1553614157.4115.17.camel@linux.ibm.com>
On 2019-03-26 11:29, Mimi Zohar wrote:
> On Tue, 2019-03-26 at 11:22 -0400, Steve Grubb wrote:
>
> > > > > --- 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);
> > > >
> > > > This part is fine.
> > > >
> > > > > if (!ab)
> > > > >
> > > > > return -ENOMEM;
> > > > >
> > > > > @@ -222,7 +223,7 @@ 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");
> > > > > + audit_log_format(ab, "xattr=(locked)");
> > > >
> > > > Two things come to mind:
> > > >
> > > > * While we can clearly trust the string above, should we be logging
> > > > the xattr field value as an untrusted string so it is consistent with
> > > > how we record other xattr names?
> > >
> > > That would be a question for Steve.
> >
> > All fields with the same name must be represented the same way. If one
> > instance is untrusted, every instance of the same keyword must be untrusted.
>
> Normal case:
> audit_log_format(ab, "xattr=");
> audit_log_untrustedstring(ab, xattr->name);
>
> Ok, so the above audit_log_format() call should be replaced with
> audit_log_untrustedstring().
Ok, so I think we can agree on "audit_log_untrustedstring(ab,
"xattr=.");" and simpler yet just print the contents regardless and not
special case this print. V2 coming...
> Mimi
- RGB
--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
^ permalink raw reply
* Re: [PATCH 22/27] Lock down kprobes
From: Matthew Garrett @ 2019-03-26 17:41 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: James Morris, LSM List, Linux Kernel Mailing List, David Howells,
Alexei Starovoitov, Naveen N . Rao, Anil S Keshavamurthy, davem
In-Reply-To: <20190326212957.f5b518990c14cf21262bfdcc@kernel.org>
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.
^ permalink raw reply
* Re: [PATCH ghak109 V1] audit: link integrity evm_write_xattrs record to syscall event
From: Richard Guy Briggs @ 2019-03-26 17:42 UTC (permalink / raw)
To: Mimi Zohar
Cc: Steve Grubb, Paul Moore, linux-integrity, linux-security-module,
Linux-Audit Mailing List, LKML, omosnace, Eric Paris,
Serge Hallyn, mjg59
In-Reply-To: <20190326161409.3wej7hcwnqalf4or@madcap2.tricolour.ca>
On 2019-03-26 12:14, Richard Guy Briggs wrote:
> On 2019-03-26 11:29, Mimi Zohar wrote:
> > On Tue, 2019-03-26 at 11:22 -0400, Steve Grubb wrote:
> >
> > > > > > --- 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);
> > > > >
> > > > > This part is fine.
> > > > >
> > > > > > if (!ab)
> > > > > >
> > > > > > return -ENOMEM;
> > > > > >
> > > > > > @@ -222,7 +223,7 @@ 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");
> > > > > > + audit_log_format(ab, "xattr=(locked)");
> > > > >
> > > > > Two things come to mind:
> > > > >
> > > > > * While we can clearly trust the string above, should we be logging
> > > > > the xattr field value as an untrusted string so it is consistent with
> > > > > how we record other xattr names?
> > > >
> > > > That would be a question for Steve.
> > >
> > > All fields with the same name must be represented the same way. If one
> > > instance is untrusted, every instance of the same keyword must be untrusted.
> >
> > Normal case:
> > audit_log_format(ab, "xattr=");
> > audit_log_untrustedstring(ab, xattr->name);
> >
> > Ok, so the above audit_log_format() call should be replaced with
> > audit_log_untrustedstring().
>
> Ok, so I think we can agree on "audit_log_untrustedstring(ab,
> "xattr=.");" and simpler yet just print the contents regardless and not
> special case this print. V2 coming...
Ok, what I typed above wasn't quite what I intended... This is what I
meant:
audit_log_format(ab, "xattr=");
audit_log_untrustedstring(ab, ".");
But, I'll just move the normal case above the "." locking detection and
log all cases the same way.
> > Mimi
>
> - RGB
- RGB
--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
^ permalink raw reply
* Re: [PATCH ghak109 V1] audit: link integrity evm_write_xattrs record to syscall event
From: Matthew Garrett @ 2019-03-26 17:55 UTC (permalink / raw)
To: Richard Guy Briggs
Cc: Mimi Zohar, Steve Grubb, Paul Moore, linux-integrity, LSM List,
Linux-Audit Mailing List, LKML, omosnace, Eric Paris,
Serge Hallyn
In-Reply-To: <20190326174252.irqrldtcy5bwho6k@madcap2.tricolour.ca>
On Tue, Mar 26, 2019 at 10:43 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> Ok, what I typed above wasn't quite what I intended... This is what I
> meant:
>
> audit_log_format(ab, "xattr=");
> audit_log_untrustedstring(ab, ".");
>
> But, I'll just move the normal case above the "." locking detection and
> log all cases the same way.
Sounds good to me.
^ permalink raw reply
* Re: [PATCH v2 00/11] LSM documentation update
From: James Morris @ 2019-03-26 18:16 UTC (permalink / raw)
To: Jonathan Corbet
Cc: Denis Efremov, Casey Schaufler, Kees Cook, Eric W. Biederman,
Eric Paris, John Johansen, Serge E. Hallyn, Paul Moore,
Kentaro Takeda, linux-security-module, linux-kernel,
open list:DOCUMENTATION
In-Reply-To: <20190326085350.4de2c68d@lwn.net>
On Tue, 26 Mar 2019, Jonathan Corbet wrote:
> On Tue, 26 Mar 2019 15:38:21 +0100
> Denis Efremov <efremov@ispras.ru> wrote:
>
> > Is there something more I could do so that these changes are accepted?
> > At least this patchset fixes the documentation format for better html
> > generation.
>
> I had assumed that this would go through the security tree, but can
> certainly pick it up if that works better.
I'll take them for 5.2.
>
> jon
>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* [PATCH V31 00/25] Add support for kernel lockdown
From: Matthew Garrett @ 2019-03-26 18:27 UTC (permalink / raw)
To: jmorris; +Cc: linux-security-module, linux-kernel, dhowells, linux-api, luto
Updates: Based on Andy's feedback, lockdown is now a tristate and can be
made stricter at runtime. The states are "none", "integrity" and
"confidentiality". "none" results in no behavioural change, "integrity"
enables features that prevent untrusted code from being run in ring 0,
and "confidentiality" is a superset of "integrity" that also disables
features that may be used to extract secret information from the kernel
at runtime. I've also modified the bpf patch so that only the calls
documented as giving the ability to read in-kernel data are locked down,
rather than all functionality being disabled - I'm not a bpf expert so
would gladly go for further review here. Long term, it'd be preferable
to be able to tag secrets held by the kernel and grant access to
everything else, but I'm open to further feedback here. And at Greg's
request, debugfs is now largely disabled once the system is locked down.
In the general case, I'd expect distributions to opt for nothing
stricter than "integrity" - "confidentiality" seems more suitable for
more special-case scenarios.
^ permalink raw reply
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