* Re: [RFC PATCH 0/2] ima: uncompressed module appraisal support
From: Mimi Zohar @ 2020-02-07 18:54 UTC (permalink / raw)
To: Eric Snowberg
Cc: Nayna, dmitry.kasatkin, jmorris, serge, dhowells, geert, gregkh,
nayna, tglx, bauerman, mpe, linux-integrity,
linux-security-module, linux-kernel
In-Reply-To: <992E95D5-D4B9-4913-A36F-BB47631DFE0A@oracle.com>
On Fri, 2020-02-07 at 11:45 -0700, Eric Snowberg wrote:
>
> > On Feb 7, 2020, at 11:28 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Fri, 2020-02-07 at 10:49 -0700, Eric Snowberg wrote:
> >>
> >>> On Feb 7, 2020, at 10:40 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> >>>
> >>>> $ insmod ./foo.ko
> >>>> insmod: ERROR: could not insert module ./foo.ko: Permission denied
> >>>>
> >>>> last entry from audit log:
> >>>> type=INTEGRITY_DATA msg=audit(1581089373.076:83): pid=2874 uid=0
> >>>> auid=0 ses=1 subj=unconfined_u:unconfined_r:unconfined_t:s0-
> >>>> s0:c0.c1023 op=appraise_data cause=invalid-signature comm="insmod"
> >>>> name="/root/keys/modules/foo.ko" dev="dm-0" ino=10918365
> >>>> res=0^]UID="root" AUID=“root"
> >>>>
> >>>> This is because modsig_verify() will be called from within
> >>>> ima_appraise_measurement(),
> >>>> since try_modsig is true. Then modsig_verify() will return
> >>>> INTEGRITY_FAIL.
> >>>
> >>> Why is it an "invalid signature"? For that you need to look at the
> >>> kernel messages. Most likely it can't find the public key on the .ima
> >>> keyring to verify the signature.
> >>
> >> It is invalid because the module has not been ima signed.
> >
> > With the IMA policy rule "appraise func=MODULE_CHECK
> > appraise_type=imasig|modsig", IMA first tries to verify the IMA
> > signature stored as an xattr and on failure then attempts to verify
> > the appended signatures.
> >
> > The audit message above indicates that there was a signature, but the
> > signature validation failed.
> >
>
> I do have CONFIG_IMA_APPRAISE_MODSIG enabled. I believe the audit message above
> is coming from modsig_verify in security/integrity/ima/ima_appraise.c.
Right, and it's calling:
rc = integrity_modsig_verify(INTEGRITY_KEYRING_IMA, modsig);
It's failing because it is trying to find the public key on the .ima
keyring. Make sure that the public needed to validate the kernel
module is on the IMA keyring (eg. keyctl show %keyring:.ima).
Mimi
^ permalink raw reply
* Re: [RFC PATCH 0/2] ima: uncompressed module appraisal support
From: Eric Snowberg @ 2020-02-07 21:38 UTC (permalink / raw)
To: Mimi Zohar
Cc: Nayna, dmitry.kasatkin, jmorris, serge, dhowells, geert, gregkh,
nayna, tglx, bauerman, mpe, linux-integrity,
linux-security-module, linux-kernel
In-Reply-To: <1581101672.5585.628.camel@linux.ibm.com>
> On Feb 7, 2020, at 11:54 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Fri, 2020-02-07 at 11:45 -0700, Eric Snowberg wrote:
>>
>>> On Feb 7, 2020, at 11:28 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>>>
>>> On Fri, 2020-02-07 at 10:49 -0700, Eric Snowberg wrote:
>>>>
>>>>> On Feb 7, 2020, at 10:40 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>>>>>
>>>>>> $ insmod ./foo.ko
>>>>>> insmod: ERROR: could not insert module ./foo.ko: Permission denied
>>>>>>
>>>>>> last entry from audit log:
>>>>>> type=INTEGRITY_DATA msg=audit(1581089373.076:83): pid=2874 uid=0
>>>>>> auid=0 ses=1 subj=unconfined_u:unconfined_r:unconfined_t:s0-
>>>>>> s0:c0.c1023 op=appraise_data cause=invalid-signature comm="insmod"
>>>>>> name="/root/keys/modules/foo.ko" dev="dm-0" ino=10918365
>>>>>> res=0^]UID="root" AUID=“root"
>>>>>>
>>>>>> This is because modsig_verify() will be called from within
>>>>>> ima_appraise_measurement(),
>>>>>> since try_modsig is true. Then modsig_verify() will return
>>>>>> INTEGRITY_FAIL.
>>>>>
>>>>> Why is it an "invalid signature"? For that you need to look at the
>>>>> kernel messages. Most likely it can't find the public key on the .ima
>>>>> keyring to verify the signature.
>>>>
>>>> It is invalid because the module has not been ima signed.
>>>
>>> With the IMA policy rule "appraise func=MODULE_CHECK
>>> appraise_type=imasig|modsig", IMA first tries to verify the IMA
>>> signature stored as an xattr and on failure then attempts to verify
>>> the appended signatures.
>>>
>>> The audit message above indicates that there was a signature, but the
>>> signature validation failed.
>>>
>>
>> I do have CONFIG_IMA_APPRAISE_MODSIG enabled. I believe the audit message above
>> is coming from modsig_verify in security/integrity/ima/ima_appraise.c.
>
> Right, and it's calling:
>
> rc = integrity_modsig_verify(INTEGRITY_KEYRING_IMA, modsig);
>
> It's failing because it is trying to find the public key on the .ima
> keyring. Make sure that the public needed to validate the kernel
> module is on the IMA keyring (eg. keyctl show %keyring:.ima).
>
I know that will validate the module properly, but that is not what I’m
trying to solve here. I thought the point of adding “|modsig” to the
ima policy was to tell ima it can either validate against an ima keyring OR
default back to the kernel keyring. This is what happens with the compressed
module. There isn’t anything in the ima keyring to validate the compressed
modules and it loads when I add “|modsig”.
The use case I’m trying to solve is when someone boots with ima_policy=secure_boot.
If their initramfs contains compressed modules with appended signatures the
system boots. If they use the same ima policy with an initramfs that contains
uncompressed modules, it doesn't boot. I thought the point of adding “|modsig”
was to help with the initramfs problem, since it is difficult to ima sign
things within it.
^ permalink raw reply
* Re: [RFC PATCH 0/2] ima: uncompressed module appraisal support
From: Mimi Zohar @ 2020-02-08 23:43 UTC (permalink / raw)
To: Eric Snowberg
Cc: Nayna, dmitry.kasatkin, jmorris, serge, dhowells, geert, gregkh,
nayna, tglx, bauerman, mpe, linux-integrity,
linux-security-module, linux-kernel, Roberto Sassu
In-Reply-To: <C25E5885-F00B-48C0-AEF1-FA3014B2FDA6@oracle.com>
On Fri, 2020-02-07 at 14:38 -0700, Eric Snowberg wrote:
> > On Feb 7, 2020, at 11:54 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Fri, 2020-02-07 at 11:45 -0700, Eric Snowberg wrote:
> >>
> >>> On Feb 7, 2020, at 11:28 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> >>>
> >>> On Fri, 2020-02-07 at 10:49 -0700, Eric Snowberg wrote:
> >>>>
> >>>>> On Feb 7, 2020, at 10:40 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> >>>>>
> >>>>>> $ insmod ./foo.ko
> >>>>>> insmod: ERROR: could not insert module ./foo.ko: Permission denied
> >>>>>>
> >>>>>> last entry from audit log:
> >>>>>> type=INTEGRITY_DATA msg=audit(1581089373.076:83): pid=2874 uid=0
> >>>>>> auid=0 ses=1 subj=unconfined_u:unconfined_r:unconfined_t:s0-
> >>>>>> s0:c0.c1023 op=appraise_data cause=invalid-signature comm="insmod"
> >>>>>> name="/root/keys/modules/foo.ko" dev="dm-0" ino=10918365
> >>>>>> res=0^]UID="root" AUID=“root"
> >>>>>>
> >>>>>> This is because modsig_verify() will be called from within
> >>>>>> ima_appraise_measurement(),
> >>>>>> since try_modsig is true. Then modsig_verify() will return
> >>>>>> INTEGRITY_FAIL.
> >>>>>
> >>>>> Why is it an "invalid signature"? For that you need to look at the
> >>>>> kernel messages. Most likely it can't find the public key on the .ima
> >>>>> keyring to verify the signature.
> >>>>
> >>>> It is invalid because the module has not been ima signed.
> >>>
> >>> With the IMA policy rule "appraise func=MODULE_CHECK
> >>> appraise_type=imasig|modsig", IMA first tries to verify the IMA
> >>> signature stored as an xattr and on failure then attempts to verify
> >>> the appended signatures.
> >>>
> >>> The audit message above indicates that there was a signature, but the
> >>> signature validation failed.
> >>>
> >>
> >> I do have CONFIG_IMA_APPRAISE_MODSIG enabled. I believe the audit message above
> >> is coming from modsig_verify in security/integrity/ima/ima_appraise.c.
> >
> > Right, and it's calling:
> >
> > rc = integrity_modsig_verify(INTEGRITY_KEYRING_IMA, modsig);
> >
> > It's failing because it is trying to find the public key on the .ima
> > keyring. Make sure that the public needed to validate the kernel
> > module is on the IMA keyring (eg. keyctl show %keyring:.ima).
> >
>
> I know that will validate the module properly, but that is not what I’m
> trying to solve here. I thought the point of adding “|modsig” to the
> ima policy was to tell ima it can either validate against an ima keyring OR
> default back to the kernel keyring. This is what happens with the compressed
> module. There isn’t anything in the ima keyring to validate the compressed
> modules and it loads when I add “|modsig”.
"modsig" has nothing to do with keyrings. The term "modsig" is
juxtaposed to "imasig". "modsig" refers to kernel module appended
signature.
>
> The use case I’m trying to solve is when someone boots with ima_policy=secure_boot.
As the secure_boot policy rules are replaced once a custom policy is
loaded, the "secure_boot" policy should probably be deprecated. I
highly recommend using the more recent build time and architecture
specific run time policy rules, which persist after loading a custom
policy.
> If their initramfs contains compressed modules with appended signatures the
> system boots. If they use the same ima policy with an initramfs that contains
> uncompressed modules, it doesn't boot. I thought the point of adding “|modsig”
> was to help with the initramfs problem, since it is difficult to ima sign
> things within it.
There have been a number of attempts to address the CPIO problem of
not being able to include security extended attributes in the
initramfs. If you're interested in solving that problem, then review
and comment on Roberto Sassu's patches[1].
Mimi
[1] https://lkml.org/lkml/2019/5/23/415
^ permalink raw reply
* [PATCH v26 10/22] x86/sgx: Linux Enclave Driver
From: Jarkko Sakkinen @ 2020-02-09 21:25 UTC (permalink / raw)
To: linux-kernel, x86, linux-sgx
Cc: akpm, dave.hansen, sean.j.christopherson, nhorman, npmccallum,
haitao.huang, andriy.shevchenko, tglx, kai.svahn, bp, josh, luto,
kai.huang, rientjes, cedric.xing, puiterwijk, Jarkko Sakkinen,
linux-security-module, Suresh Siddha, Haitao Huang
In-Reply-To: <20200209212609.7928-1-jarkko.sakkinen@linux.intel.com>
Intel Software Guard eXtensions (SGX) is a set of CPU instructions that
can be used by applications to set aside private regions of code and
data. The code outside the SGX hosted software entity is disallowed to
access the memory inside the enclave enforced by the CPU. We call these
entities as enclaves.
This commit implements a driver that provides an ioctl API to construct
and run enclaves. Enclaves are constructed from pages residing in
reserved physical memory areas. The contents of these pages can only be
accessed when they are mapped as part of an enclave, by a hardware
thread running inside the enclave.
The starting state of an enclave consists of a fixed measured set of
pages that are copied to the EPC during the construction process by
using ENCLS leaf functions and Software Enclave Control Structure (SECS)
that defines the enclave properties.
Enclave are constructed by using ENCLS leaf functions ECREATE, EADD and
EINIT. ECREATE initializes SECS, EADD copies pages from system memory to
the EPC and EINIT check a given signed measurement and moves the enclave
into a state ready for execution.
An initialized enclave can only be accessed through special Thread Control
Structure (TCS) pages by using ENCLU (ring-3 only) leaf EENTER. This leaf
function converts a thread into enclave mode and continues the execution in
the offset defined by the TCS provided to EENTER. An enclave is exited
through syscall, exception, interrupts or by explicitly calling another
ENCLU leaf EEXIT.
The permissions, which enclave page is added will set the limit for maximum
permissions that can be set for mmap() and mprotect(). This will
effectively allow to build different security schemes between producers and
consumers of enclaves. Later on we can increase granularity with LSM hooks
for page addition (i.e. for producers) and mapping of the enclave (i.e. for
consumers)
Cc: linux-security-module@vger.kernel.org
Cc: Nathaniel McCallum <npmccallum@redhat.com>
Co-developed-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Co-developed-by: Suresh Siddha <suresh.b.siddha@intel.com>
Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Tested-by: Haitao Huang <haitao.huang@linux.intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
.../userspace-api/ioctl/ioctl-number.rst | 1 +
arch/x86/include/uapi/asm/sgx.h | 66 ++
arch/x86/kernel/cpu/sgx/Makefile | 3 +
arch/x86/kernel/cpu/sgx/driver.c | 194 +++++
arch/x86/kernel/cpu/sgx/driver.h | 30 +
arch/x86/kernel/cpu/sgx/encl.c | 329 +++++++++
arch/x86/kernel/cpu/sgx/encl.h | 87 +++
arch/x86/kernel/cpu/sgx/ioctl.c | 697 ++++++++++++++++++
arch/x86/kernel/cpu/sgx/main.c | 12 +-
arch/x86/kernel/cpu/sgx/reclaim.c | 1 +
10 files changed, 1419 insertions(+), 1 deletion(-)
create mode 100644 arch/x86/include/uapi/asm/sgx.h
create mode 100644 arch/x86/kernel/cpu/sgx/driver.c
create mode 100644 arch/x86/kernel/cpu/sgx/driver.h
create mode 100644 arch/x86/kernel/cpu/sgx/encl.c
create mode 100644 arch/x86/kernel/cpu/sgx/encl.h
create mode 100644 arch/x86/kernel/cpu/sgx/ioctl.c
diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
index 2e91370dc159..1c54dd2704db 100644
--- a/Documentation/userspace-api/ioctl/ioctl-number.rst
+++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
@@ -321,6 +321,7 @@ Code Seq# Include File Comments
<mailto:tlewis@mindspring.com>
0xA3 90-9F linux/dtlk.h
0xA4 00-1F uapi/linux/tee.h Generic TEE subsystem
+0xA4 00-1F uapi/asm/sgx.h Intel SGX subsystem (a legit conflict as TEE and SGX do not co-exist)
0xAA 00-3F linux/uapi/linux/userfaultfd.h
0xAB 00-1F linux/nbd.h
0xAC 00-1F linux/raw.h
diff --git a/arch/x86/include/uapi/asm/sgx.h b/arch/x86/include/uapi/asm/sgx.h
new file mode 100644
index 000000000000..5edb08ab8fd0
--- /dev/null
+++ b/arch/x86/include/uapi/asm/sgx.h
@@ -0,0 +1,66 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) WITH Linux-syscall-note */
+/*
+ * Copyright(c) 2016-19 Intel Corporation.
+ */
+#ifndef _UAPI_ASM_X86_SGX_H
+#define _UAPI_ASM_X86_SGX_H
+
+#include <linux/types.h>
+#include <linux/ioctl.h>
+
+/**
+ * enum sgx_epage_flags - page control flags
+ * %SGX_PAGE_MEASURE: Measure the page contents with a sequence of
+ * ENCLS[EEXTEND] operations.
+ */
+enum sgx_page_flags {
+ SGX_PAGE_MEASURE = 0x01,
+};
+
+#define SGX_MAGIC 0xA4
+
+#define SGX_IOC_ENCLAVE_CREATE \
+ _IOW(SGX_MAGIC, 0x00, struct sgx_enclave_create)
+#define SGX_IOC_ENCLAVE_ADD_PAGES \
+ _IOWR(SGX_MAGIC, 0x01, struct sgx_enclave_add_pages)
+#define SGX_IOC_ENCLAVE_INIT \
+ _IOW(SGX_MAGIC, 0x02, struct sgx_enclave_init)
+
+/**
+ * struct sgx_enclave_create - parameter structure for the
+ * %SGX_IOC_ENCLAVE_CREATE ioctl
+ * @src: address for the SECS page data
+ */
+struct sgx_enclave_create {
+ __u64 src;
+};
+
+/**
+ * struct sgx_enclave_add_pages - parameter structure for the
+ * %SGX_IOC_ENCLAVE_ADD_PAGE ioctl
+ * @src: start address for the page data
+ * @offset: starting page offset
+ * @length: length of the data (multiple of the page size)
+ * @secinfo: address for the SECINFO data
+ * @flags: page control flags
+ * @count: number of bytes added (multiple of the page size)
+ */
+struct sgx_enclave_add_pages {
+ __u64 src;
+ __u64 offset;
+ __u64 length;
+ __u64 secinfo;
+ __u64 flags;
+ __u64 count;
+};
+
+/**
+ * struct sgx_enclave_init - parameter structure for the
+ * %SGX_IOC_ENCLAVE_INIT ioctl
+ * @sigstruct: address for the SIGSTRUCT data
+ */
+struct sgx_enclave_init {
+ __u64 sigstruct;
+};
+
+#endif /* _UAPI_ASM_X86_SGX_H */
diff --git a/arch/x86/kernel/cpu/sgx/Makefile b/arch/x86/kernel/cpu/sgx/Makefile
index 2dec75916a5e..f8d32da3a67a 100644
--- a/arch/x86/kernel/cpu/sgx/Makefile
+++ b/arch/x86/kernel/cpu/sgx/Makefile
@@ -1,3 +1,6 @@
obj-y += \
+ driver.o \
+ encl.o \
+ ioctl.o \
main.o \
reclaim.o
diff --git a/arch/x86/kernel/cpu/sgx/driver.c b/arch/x86/kernel/cpu/sgx/driver.c
new file mode 100644
index 000000000000..b4aa7b9f8376
--- /dev/null
+++ b/arch/x86/kernel/cpu/sgx/driver.c
@@ -0,0 +1,194 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+// Copyright(c) 2016-18 Intel Corporation.
+
+#include <linux/acpi.h>
+#include <linux/miscdevice.h>
+#include <linux/mman.h>
+#include <linux/security.h>
+#include <linux/suspend.h>
+#include <asm/traps.h>
+#include "driver.h"
+#include "encl.h"
+
+MODULE_DESCRIPTION("Intel SGX Enclave Driver");
+MODULE_AUTHOR("Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>");
+MODULE_LICENSE("Dual BSD/GPL");
+
+u64 sgx_encl_size_max_32;
+u64 sgx_encl_size_max_64;
+u32 sgx_misc_reserved_mask;
+u64 sgx_attributes_reserved_mask;
+u64 sgx_xfrm_reserved_mask = ~0x3;
+u32 sgx_xsave_size_tbl[64];
+
+static int sgx_open(struct inode *inode, struct file *file)
+{
+ struct sgx_encl *encl;
+ int ret;
+
+ encl = kzalloc(sizeof(*encl), GFP_KERNEL);
+ if (!encl)
+ return -ENOMEM;
+
+ atomic_set(&encl->flags, 0);
+ kref_init(&encl->refcount);
+ INIT_RADIX_TREE(&encl->page_tree, GFP_KERNEL);
+ mutex_init(&encl->lock);
+ INIT_LIST_HEAD(&encl->mm_list);
+ spin_lock_init(&encl->mm_lock);
+
+ ret = init_srcu_struct(&encl->srcu);
+ if (ret) {
+ kfree(encl);
+ return ret;
+ }
+
+ file->private_data = encl;
+
+ return 0;
+}
+
+static int sgx_release(struct inode *inode, struct file *file)
+{
+ struct sgx_encl *encl = file->private_data;
+ struct sgx_encl_mm *encl_mm;
+
+ for ( ; ; ) {
+ spin_lock(&encl->mm_lock);
+
+ if (list_empty(&encl->mm_list)) {
+ encl_mm = NULL;
+ } else {
+ encl_mm = list_first_entry(&encl->mm_list,
+ struct sgx_encl_mm, list);
+ list_del_rcu(&encl_mm->list);
+ }
+
+ spin_unlock(&encl->mm_lock);
+
+ /* The list is empty, ready to go. */
+ if (!encl_mm)
+ break;
+
+ synchronize_srcu(&encl->srcu);
+ mmu_notifier_unregister(&encl_mm->mmu_notifier, encl_mm->mm);
+ kfree(encl_mm);
+ };
+
+ mutex_lock(&encl->lock);
+ atomic_or(SGX_ENCL_DEAD, &encl->flags);
+ mutex_unlock(&encl->lock);
+
+ kref_put(&encl->refcount, sgx_encl_release);
+ return 0;
+}
+
+#ifdef CONFIG_COMPAT
+static long sgx_compat_ioctl(struct file *filep, unsigned int cmd,
+ unsigned long arg)
+{
+ return sgx_ioctl(filep, cmd, arg);
+}
+#endif
+
+static int sgx_mmap(struct file *file, struct vm_area_struct *vma)
+{
+ struct sgx_encl *encl = file->private_data;
+ int ret;
+
+ ret = sgx_encl_may_map(encl, vma->vm_start, vma->vm_end,
+ vma->vm_flags & (VM_READ | VM_WRITE | VM_EXEC));
+ if (ret)
+ return ret;
+
+ ret = sgx_encl_mm_add(encl, vma->vm_mm);
+ if (ret)
+ return ret;
+
+ vma->vm_ops = &sgx_vm_ops;
+ vma->vm_flags |= VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP | VM_IO;
+ vma->vm_private_data = encl;
+
+ return 0;
+}
+
+static unsigned long sgx_get_unmapped_area(struct file *file,
+ unsigned long addr,
+ unsigned long len,
+ unsigned long pgoff,
+ unsigned long flags)
+{
+ if (flags & MAP_PRIVATE)
+ return -EINVAL;
+
+ if (flags & MAP_FIXED)
+ return addr;
+
+ return current->mm->get_unmapped_area(file, addr, len, pgoff, flags);
+}
+
+static const struct file_operations sgx_encl_fops = {
+ .owner = THIS_MODULE,
+ .open = sgx_open,
+ .release = sgx_release,
+ .unlocked_ioctl = sgx_ioctl,
+#ifdef CONFIG_COMPAT
+ .compat_ioctl = sgx_compat_ioctl,
+#endif
+ .mmap = sgx_mmap,
+ .get_unmapped_area = sgx_get_unmapped_area,
+};
+
+const struct file_operations sgx_provision_fops = {
+ .owner = THIS_MODULE,
+};
+
+static struct miscdevice sgx_dev_enclave = {
+ .minor = MISC_DYNAMIC_MINOR,
+ .name = "enclave",
+ .nodename = "sgx/enclave",
+ .fops = &sgx_encl_fops,
+};
+
+int __init sgx_drv_init(void)
+{
+ unsigned int eax, ebx, ecx, edx;
+ u64 attr_mask, xfrm_mask;
+ int ret;
+ int i;
+
+ if (!boot_cpu_has(X86_FEATURE_SGX_LC)) {
+ pr_info("The public key MSRs are not writable.\n");
+ return -ENODEV;
+ }
+
+ cpuid_count(SGX_CPUID, 0, &eax, &ebx, &ecx, &edx);
+ sgx_misc_reserved_mask = ~ebx | SGX_MISC_RESERVED_MASK;
+ sgx_encl_size_max_64 = 1ULL << ((edx >> 8) & 0xFF);
+ sgx_encl_size_max_32 = 1ULL << (edx & 0xFF);
+
+ cpuid_count(SGX_CPUID, 1, &eax, &ebx, &ecx, &edx);
+
+ attr_mask = (((u64)ebx) << 32) + (u64)eax;
+ sgx_attributes_reserved_mask = ~attr_mask | SGX_ATTR_RESERVED_MASK;
+
+ if (boot_cpu_has(X86_FEATURE_OSXSAVE)) {
+ xfrm_mask = (((u64)edx) << 32) + (u64)ecx;
+
+ for (i = 2; i < 64; i++) {
+ cpuid_count(0x0D, i, &eax, &ebx, &ecx, &edx);
+ if ((1 << i) & xfrm_mask)
+ sgx_xsave_size_tbl[i] = eax + ebx;
+ }
+
+ sgx_xfrm_reserved_mask = ~xfrm_mask;
+ }
+
+ ret = misc_register(&sgx_dev_enclave);
+ if (ret) {
+ pr_err("Creating /dev/sgx/enclave failed with %d.\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
diff --git a/arch/x86/kernel/cpu/sgx/driver.h b/arch/x86/kernel/cpu/sgx/driver.h
new file mode 100644
index 000000000000..e4063923115b
--- /dev/null
+++ b/arch/x86/kernel/cpu/sgx/driver.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */
+#ifndef __ARCH_SGX_DRIVER_H__
+#define __ARCH_SGX_DRIVER_H__
+
+#include <crypto/hash.h>
+#include <linux/kref.h>
+#include <linux/mmu_notifier.h>
+#include <linux/radix-tree.h>
+#include <linux/rwsem.h>
+#include <linux/sched.h>
+#include <linux/workqueue.h>
+#include <uapi/asm/sgx.h>
+#include "sgx.h"
+
+#define SGX_EINIT_SPIN_COUNT 20
+#define SGX_EINIT_SLEEP_COUNT 50
+#define SGX_EINIT_SLEEP_TIME 20
+
+extern u64 sgx_encl_size_max_32;
+extern u64 sgx_encl_size_max_64;
+extern u32 sgx_misc_reserved_mask;
+extern u64 sgx_attributes_reserved_mask;
+extern u64 sgx_xfrm_reserved_mask;
+extern u32 sgx_xsave_size_tbl[64];
+
+long sgx_ioctl(struct file *filep, unsigned int cmd, unsigned long arg);
+
+int sgx_drv_init(void);
+
+#endif /* __ARCH_X86_SGX_DRIVER_H__ */
diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c
new file mode 100644
index 000000000000..cd2b8dbb0eca
--- /dev/null
+++ b/arch/x86/kernel/cpu/sgx/encl.c
@@ -0,0 +1,329 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+// Copyright(c) 2016-18 Intel Corporation.
+
+#include <linux/lockdep.h>
+#include <linux/mm.h>
+#include <linux/mman.h>
+#include <linux/shmem_fs.h>
+#include <linux/suspend.h>
+#include <linux/sched/mm.h>
+#include "arch.h"
+#include "encl.h"
+#include "sgx.h"
+
+static struct sgx_encl_page *sgx_encl_load_page(struct sgx_encl *encl,
+ unsigned long addr)
+{
+ struct sgx_encl_page *entry;
+ unsigned int flags;
+
+ /* If process was forked, VMA is still there but vm_private_data is set
+ * to NULL.
+ */
+ if (!encl)
+ return ERR_PTR(-EFAULT);
+
+ flags = atomic_read(&encl->flags);
+
+ if ((flags & SGX_ENCL_DEAD) || !(flags & SGX_ENCL_INITIALIZED))
+ return ERR_PTR(-EFAULT);
+
+ entry = radix_tree_lookup(&encl->page_tree, addr >> PAGE_SHIFT);
+ if (!entry)
+ return ERR_PTR(-EFAULT);
+
+ /* Page is already resident in the EPC. */
+ if (entry->epc_page)
+ return entry;
+
+ return ERR_PTR(-EFAULT);
+}
+
+static void sgx_mmu_notifier_release(struct mmu_notifier *mn,
+ struct mm_struct *mm)
+{
+ struct sgx_encl_mm *encl_mm =
+ container_of(mn, struct sgx_encl_mm, mmu_notifier);
+ struct sgx_encl_mm *tmp = NULL;
+
+ /*
+ * The enclave itself can remove encl_mm. Note, objects can't be moved
+ * off an RCU protected list, but deletion is ok.
+ */
+ spin_lock(&encl_mm->encl->mm_lock);
+ list_for_each_entry(tmp, &encl_mm->encl->mm_list, list) {
+ if (tmp == encl_mm) {
+ list_del_rcu(&encl_mm->list);
+ break;
+ }
+ }
+ spin_unlock(&encl_mm->encl->mm_lock);
+
+ if (tmp == encl_mm) {
+ synchronize_srcu(&encl_mm->encl->srcu);
+ mmu_notifier_put(mn);
+ }
+}
+
+static void sgx_mmu_notifier_free(struct mmu_notifier *mn)
+{
+ struct sgx_encl_mm *encl_mm =
+ container_of(mn, struct sgx_encl_mm, mmu_notifier);
+
+ kfree(encl_mm);
+}
+
+static const struct mmu_notifier_ops sgx_mmu_notifier_ops = {
+ .release = sgx_mmu_notifier_release,
+ .free_notifier = sgx_mmu_notifier_free,
+};
+
+static struct sgx_encl_mm *sgx_encl_find_mm(struct sgx_encl *encl,
+ struct mm_struct *mm)
+{
+ struct sgx_encl_mm *encl_mm = NULL;
+ struct sgx_encl_mm *tmp;
+ int idx;
+
+ idx = srcu_read_lock(&encl->srcu);
+
+ list_for_each_entry_rcu(tmp, &encl->mm_list, list) {
+ if (tmp->mm == mm) {
+ encl_mm = tmp;
+ break;
+ }
+ }
+
+ srcu_read_unlock(&encl->srcu, idx);
+
+ return encl_mm;
+}
+
+int sgx_encl_mm_add(struct sgx_encl *encl, struct mm_struct *mm)
+{
+ struct sgx_encl_mm *encl_mm;
+ int ret;
+
+ if (atomic_read(&encl->flags) & SGX_ENCL_DEAD)
+ return -EINVAL;
+
+ /*
+ * mm_structs are kept on mm_list until the mm or the enclave dies,
+ * i.e. once an mm is off the list, it's gone for good, therefore it's
+ * impossible to get a false positive on @mm due to a stale mm_list.
+ */
+ if (sgx_encl_find_mm(encl, mm))
+ return 0;
+
+ encl_mm = kzalloc(sizeof(*encl_mm), GFP_KERNEL);
+ if (!encl_mm)
+ return -ENOMEM;
+
+ encl_mm->encl = encl;
+ encl_mm->mm = mm;
+ encl_mm->mmu_notifier.ops = &sgx_mmu_notifier_ops;
+
+ ret = __mmu_notifier_register(&encl_mm->mmu_notifier, mm);
+ if (ret) {
+ kfree(encl_mm);
+ return ret;
+ }
+
+ spin_lock(&encl->mm_lock);
+ list_add_rcu(&encl_mm->list, &encl->mm_list);
+ spin_unlock(&encl->mm_lock);
+
+ synchronize_srcu(&encl->srcu);
+
+ return 0;
+}
+
+static void sgx_vma_open(struct vm_area_struct *vma)
+{
+ struct sgx_encl *encl = vma->vm_private_data;
+
+ if (!encl)
+ return;
+
+ if (sgx_encl_mm_add(encl, vma->vm_mm))
+ vma->vm_private_data = NULL;
+}
+
+static unsigned int sgx_vma_fault(struct vm_fault *vmf)
+{
+ unsigned long addr = (unsigned long)vmf->address;
+ struct vm_area_struct *vma = vmf->vma;
+ struct sgx_encl *encl = vma->vm_private_data;
+ struct sgx_encl_page *entry;
+ int ret = VM_FAULT_NOPAGE;
+ unsigned long pfn;
+
+ if (!encl)
+ return VM_FAULT_SIGBUS;
+
+ mutex_lock(&encl->lock);
+
+ entry = sgx_encl_load_page(encl, addr);
+ if (IS_ERR(entry)) {
+ if (unlikely(PTR_ERR(entry) != -EBUSY))
+ ret = VM_FAULT_SIGBUS;
+
+ goto out;
+ }
+
+ if (!follow_pfn(vma, addr, &pfn))
+ goto out;
+
+ ret = vmf_insert_pfn(vma, addr, PFN_DOWN(entry->epc_page->desc));
+ if (ret != VM_FAULT_NOPAGE) {
+ ret = VM_FAULT_SIGBUS;
+ goto out;
+ }
+
+out:
+ mutex_unlock(&encl->lock);
+ return ret;
+}
+
+/**
+ * sgx_encl_may_map() - Check if a requested VMA mapping is allowed
+ * @encl: an enclave
+ * @start: lower bound of the address range, inclusive
+ * @end: upper bound of the address range, exclusive
+ * @vm_prot_bits: requested protections of the address range
+ *
+ * Iterate through the enclave pages contained within [@start, @end) to verify
+ * the permissions requested by @vm_prot_bits do not exceed that of any enclave
+ * page to be mapped. Page addresses that do not have an associated enclave
+ * page are interpreted to zero permissions.
+ *
+ * Return:
+ * 0 on success,
+ * -EACCES if VMA permissions exceed enclave page permissions
+ */
+int sgx_encl_may_map(struct sgx_encl *encl, unsigned long start,
+ unsigned long end, unsigned long vm_prot_bits)
+{
+ unsigned long idx, idx_start, idx_end;
+ struct sgx_encl_page *page;
+
+ /* PROT_NONE always succeeds. */
+ if (!vm_prot_bits)
+ return 0;
+
+ idx_start = PFN_DOWN(start);
+ idx_end = PFN_DOWN(end - 1);
+
+ for (idx = idx_start; idx <= idx_end; ++idx) {
+ mutex_lock(&encl->lock);
+ page = radix_tree_lookup(&encl->page_tree, idx);
+ mutex_unlock(&encl->lock);
+
+ if (!page || (~page->vm_max_prot_bits & vm_prot_bits))
+ return -EACCES;
+ }
+
+ return 0;
+}
+
+static int sgx_vma_mprotect(struct vm_area_struct *vma, unsigned long start,
+ unsigned long end, unsigned long prot)
+{
+ return sgx_encl_may_map(vma->vm_private_data, start, end,
+ calc_vm_prot_bits(prot, 0));
+}
+
+const struct vm_operations_struct sgx_vm_ops = {
+ .open = sgx_vma_open,
+ .fault = sgx_vma_fault,
+ .may_mprotect = sgx_vma_mprotect,
+};
+
+/**
+ * sgx_encl_find - find an enclave
+ * @mm: mm struct of the current process
+ * @addr: address in the ELRANGE
+ * @vma: the resulting VMA
+ *
+ * Find an enclave identified by the given address. Give back a VMA that is
+ * part of the enclave and located in that address. The VMA is given back if it
+ * is a proper enclave VMA even if an &sgx_encl instance does not exist yet
+ * (enclave creation has not been performed).
+ *
+ * Return:
+ * 0 on success,
+ * -EINVAL if an enclave was not found,
+ * -ENOENT if the enclave has not been created yet
+ */
+int sgx_encl_find(struct mm_struct *mm, unsigned long addr,
+ struct vm_area_struct **vma)
+{
+ struct vm_area_struct *result;
+ struct sgx_encl *encl;
+
+ result = find_vma(mm, addr);
+ if (!result || result->vm_ops != &sgx_vm_ops || addr < result->vm_start)
+ return -EINVAL;
+
+ encl = result->vm_private_data;
+ *vma = result;
+
+ return encl ? 0 : -ENOENT;
+}
+
+/**
+ * sgx_encl_destroy() - destroy enclave resources
+ * @encl: an &sgx_encl instance
+ */
+void sgx_encl_destroy(struct sgx_encl *encl)
+{
+ struct sgx_encl_page *entry;
+ struct radix_tree_iter iter;
+ void **slot;
+
+ atomic_or(SGX_ENCL_DEAD, &encl->flags);
+
+ radix_tree_for_each_slot(slot, &encl->page_tree, &iter, 0) {
+ entry = *slot;
+
+ if (entry->epc_page) {
+ sgx_free_page(entry->epc_page);
+ encl->secs_child_cnt--;
+ entry->epc_page = NULL;
+ }
+
+ radix_tree_delete(&entry->encl->page_tree,
+ PFN_DOWN(entry->desc));
+ kfree(entry);
+ }
+
+ if (!encl->secs_child_cnt && encl->secs.epc_page) {
+ sgx_free_page(encl->secs.epc_page);
+ encl->secs.epc_page = NULL;
+ }
+}
+
+/**
+ * sgx_encl_release - Destroy an enclave instance
+ * @kref: address of a kref inside &sgx_encl
+ *
+ * Used together with kref_put(). Frees all the resources associated with the
+ * enclave and the instance itself.
+ */
+void sgx_encl_release(struct kref *ref)
+{
+ struct sgx_encl *encl = container_of(ref, struct sgx_encl, refcount);
+
+ sgx_encl_destroy(encl);
+
+ if (encl->backing)
+ fput(encl->backing);
+
+ WARN_ON_ONCE(!list_empty(&encl->mm_list));
+
+ /* Detect EPC page leak's. */
+ WARN_ON_ONCE(encl->secs_child_cnt);
+ WARN_ON_ONCE(encl->secs.epc_page);
+
+ kfree(encl);
+}
diff --git a/arch/x86/kernel/cpu/sgx/encl.h b/arch/x86/kernel/cpu/sgx/encl.h
new file mode 100644
index 000000000000..1d1bc5d590ee
--- /dev/null
+++ b/arch/x86/kernel/cpu/sgx/encl.h
@@ -0,0 +1,87 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */
+/**
+ * Copyright(c) 2016-19 Intel Corporation.
+ */
+#ifndef _X86_ENCL_H
+#define _X86_ENCL_H
+
+#include <linux/cpumask.h>
+#include <linux/kref.h>
+#include <linux/list.h>
+#include <linux/mm_types.h>
+#include <linux/mmu_notifier.h>
+#include <linux/mutex.h>
+#include <linux/notifier.h>
+#include <linux/radix-tree.h>
+#include <linux/srcu.h>
+#include <linux/workqueue.h>
+#include "sgx.h"
+
+/**
+ * enum sgx_encl_page_desc - defines bits for an enclave page's descriptor
+ * %SGX_ENCL_PAGE_ADDR_MASK: Holds the virtual address of the page.
+ *
+ * The page address for SECS is zero and is used by the subsystem to recognize
+ * the SECS page.
+ */
+enum sgx_encl_page_desc {
+ /* Bits 11:3 are available when the page is not swapped. */
+ SGX_ENCL_PAGE_ADDR_MASK = PAGE_MASK,
+};
+
+#define SGX_ENCL_PAGE_ADDR(page) \
+ ((page)->desc & SGX_ENCL_PAGE_ADDR_MASK)
+
+struct sgx_encl_page {
+ unsigned long desc;
+ unsigned long vm_max_prot_bits;
+ struct sgx_epc_page *epc_page;
+ struct sgx_encl *encl;
+};
+
+enum sgx_encl_flags {
+ SGX_ENCL_CREATED = BIT(0),
+ SGX_ENCL_INITIALIZED = BIT(1),
+ SGX_ENCL_DEBUG = BIT(2),
+ SGX_ENCL_DEAD = BIT(3),
+ SGX_ENCL_IOCTL = BIT(4),
+};
+
+struct sgx_encl_mm {
+ struct sgx_encl *encl;
+ struct mm_struct *mm;
+ struct list_head list;
+ struct mmu_notifier mmu_notifier;
+};
+
+struct sgx_encl {
+ atomic_t flags;
+ u64 secs_attributes;
+ u64 allowed_attributes;
+ unsigned int page_cnt;
+ unsigned int secs_child_cnt;
+ struct mutex lock;
+ struct list_head mm_list;
+ spinlock_t mm_lock;
+ struct file *backing;
+ struct kref refcount;
+ struct srcu_struct srcu;
+ unsigned long base;
+ unsigned long size;
+ unsigned long ssaframesize;
+ struct radix_tree_root page_tree;
+ struct sgx_encl_page secs;
+ cpumask_t cpumask;
+};
+
+extern const struct vm_operations_struct sgx_vm_ops;
+
+int sgx_encl_find(struct mm_struct *mm, unsigned long addr,
+ struct vm_area_struct **vma);
+void sgx_encl_destroy(struct sgx_encl *encl);
+void sgx_encl_release(struct kref *ref);
+int sgx_encl_mm_add(struct sgx_encl *encl, struct mm_struct *mm);
+int sgx_encl_may_map(struct sgx_encl *encl, unsigned long start,
+ unsigned long end, unsigned long vm_prot_bits);
+
+#endif /* _X86_ENCL_H */
diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
new file mode 100644
index 000000000000..83513cdfd1c0
--- /dev/null
+++ b/arch/x86/kernel/cpu/sgx/ioctl.c
@@ -0,0 +1,697 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+// Copyright(c) 2016-19 Intel Corporation.
+
+#include <asm/mman.h>
+#include <linux/mman.h>
+#include <linux/delay.h>
+#include <linux/file.h>
+#include <linux/hashtable.h>
+#include <linux/highmem.h>
+#include <linux/ratelimit.h>
+#include <linux/sched/signal.h>
+#include <linux/shmem_fs.h>
+#include <linux/slab.h>
+#include <linux/suspend.h>
+#include "driver.h"
+#include "encl.h"
+#include "encls.h"
+
+/* A per-cpu cache for the last known values of IA32_SGXLEPUBKEYHASHx MSRs. */
+static DEFINE_PER_CPU(u64 [4], sgx_lepubkeyhash_cache);
+
+static u32 sgx_calc_ssaframesize(u32 miscselect, u64 xfrm)
+{
+ u32 size_max = PAGE_SIZE;
+ u32 size;
+ int i;
+
+ for (i = 2; i < 64; i++) {
+ if (!((1 << i) & xfrm))
+ continue;
+
+ size = SGX_SSA_GPRS_SIZE + sgx_xsave_size_tbl[i];
+ if (miscselect & SGX_MISC_EXINFO)
+ size += SGX_SSA_MISC_EXINFO_SIZE;
+
+ if (size > size_max)
+ size_max = size;
+ }
+
+ return PFN_UP(size_max);
+}
+
+static int sgx_validate_secs(const struct sgx_secs *secs,
+ unsigned long ssaframesize)
+{
+ if (secs->size < (2 * PAGE_SIZE) || !is_power_of_2(secs->size))
+ return -EINVAL;
+
+ if (secs->base & (secs->size - 1))
+ return -EINVAL;
+
+ if (secs->miscselect & sgx_misc_reserved_mask ||
+ secs->attributes & sgx_attributes_reserved_mask ||
+ secs->xfrm & sgx_xfrm_reserved_mask)
+ return -EINVAL;
+
+ if (secs->attributes & SGX_ATTR_MODE64BIT) {
+ if (secs->size > sgx_encl_size_max_64)
+ return -EINVAL;
+ } else if (secs->size > sgx_encl_size_max_32)
+ return -EINVAL;
+
+ if (!(secs->xfrm & XFEATURE_MASK_FP) ||
+ !(secs->xfrm & XFEATURE_MASK_SSE) ||
+ (((secs->xfrm >> XFEATURE_BNDREGS) & 1) !=
+ ((secs->xfrm >> XFEATURE_BNDCSR) & 1)))
+ return -EINVAL;
+
+ if (!secs->ssa_frame_size || ssaframesize > secs->ssa_frame_size)
+ return -EINVAL;
+
+ if (memchr_inv(secs->reserved1, 0, sizeof(secs->reserved1)) ||
+ memchr_inv(secs->reserved2, 0, sizeof(secs->reserved2)) ||
+ memchr_inv(secs->reserved3, 0, sizeof(secs->reserved3)) ||
+ memchr_inv(secs->reserved4, 0, sizeof(secs->reserved4)))
+ return -EINVAL;
+
+ return 0;
+}
+
+static struct sgx_encl_page *sgx_encl_page_alloc(struct sgx_encl *encl,
+ unsigned long offset,
+ u64 secinfo_flags)
+{
+ struct sgx_encl_page *encl_page;
+ unsigned long prot;
+
+ encl_page = kzalloc(sizeof(*encl_page), GFP_KERNEL);
+ if (!encl_page)
+ return ERR_PTR(-ENOMEM);
+
+ encl_page->desc = encl->base + offset;
+ encl_page->encl = encl;
+
+ prot = _calc_vm_trans(secinfo_flags, SGX_SECINFO_R, PROT_READ) |
+ _calc_vm_trans(secinfo_flags, SGX_SECINFO_W, PROT_WRITE) |
+ _calc_vm_trans(secinfo_flags, SGX_SECINFO_X, PROT_EXEC);
+
+ /*
+ * TCS pages must always RW set for CPU access while the SECINFO
+ * permissions are *always* zero - the CPU ignores the user provided
+ * values and silently overwrites them with zero permissions.
+ */
+ if ((secinfo_flags & SGX_SECINFO_PAGE_TYPE_MASK) == SGX_SECINFO_TCS)
+ prot |= PROT_READ | PROT_WRITE;
+
+ /* Calculate maximum of the VM flags for the page. */
+ encl_page->vm_max_prot_bits = calc_vm_prot_bits(prot, 0);
+
+ return encl_page;
+}
+
+static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs)
+{
+ unsigned long encl_size = secs->size + PAGE_SIZE;
+ struct sgx_epc_page *secs_epc;
+ unsigned long ssaframesize;
+ struct sgx_pageinfo pginfo;
+ struct sgx_secinfo secinfo;
+ struct file *backing;
+ long ret;
+
+ if (atomic_read(&encl->flags) & SGX_ENCL_CREATED)
+ return -EINVAL;
+
+ ssaframesize = sgx_calc_ssaframesize(secs->miscselect, secs->xfrm);
+ if (sgx_validate_secs(secs, ssaframesize)) {
+ pr_debug("invalid SECS\n");
+ return -EINVAL;
+ }
+
+ backing = shmem_file_setup("SGX backing", encl_size + (encl_size >> 5),
+ VM_NORESERVE);
+ if (IS_ERR(backing))
+ return PTR_ERR(backing);
+
+ encl->backing = backing;
+
+ secs_epc = sgx_try_alloc_page();
+ if (IS_ERR(secs_epc)) {
+ ret = PTR_ERR(secs_epc);
+ goto err_out_backing;
+ }
+
+ encl->secs.epc_page = secs_epc;
+
+ pginfo.addr = 0;
+ pginfo.contents = (unsigned long)secs;
+ pginfo.metadata = (unsigned long)&secinfo;
+ pginfo.secs = 0;
+ memset(&secinfo, 0, sizeof(secinfo));
+
+ ret = __ecreate((void *)&pginfo, sgx_epc_addr(secs_epc));
+ if (ret) {
+ pr_debug("ECREATE returned %ld\n", ret);
+ goto err_out;
+ }
+
+ if (secs->attributes & SGX_ATTR_DEBUG)
+ atomic_or(SGX_ENCL_DEBUG, &encl->flags);
+
+ encl->secs.encl = encl;
+ encl->secs_attributes = secs->attributes;
+ encl->allowed_attributes |= SGX_ATTR_ALLOWED_MASK;
+ encl->base = secs->base;
+ encl->size = secs->size;
+ encl->ssaframesize = secs->ssa_frame_size;
+
+ /*
+ * Set SGX_ENCL_CREATED only after the enclave is fully prepped. This
+ * allows setting and checking enclave creation without having to take
+ * encl->lock.
+ */
+ atomic_or(SGX_ENCL_CREATED, &encl->flags);
+
+ return 0;
+
+err_out:
+ sgx_free_page(encl->secs.epc_page);
+ encl->secs.epc_page = NULL;
+
+err_out_backing:
+ fput(encl->backing);
+ encl->backing = NULL;
+
+ return ret;
+}
+
+/**
+ * sgx_ioc_enclave_create - handler for %SGX_IOC_ENCLAVE_CREATE
+ * @filep: open file to /dev/sgx
+ * @arg: userspace pointer to a struct sgx_enclave_create instance
+ *
+ * Allocate kernel data structures for a new enclave and execute ECREATE after
+ * verifying the correctness of the provided SECS.
+ *
+ * Note, enforcement of restricted and disallowed attributes is deferred until
+ * sgx_ioc_enclave_init(), only the architectural correctness of the SECS is
+ * checked by sgx_ioc_enclave_create().
+ *
+ * Return:
+ * 0 on success,
+ * -errno otherwise
+ */
+static long sgx_ioc_enclave_create(struct sgx_encl *encl, void __user *arg)
+{
+ struct sgx_enclave_create ecreate;
+ struct page *secs_page;
+ struct sgx_secs *secs;
+ int ret;
+
+ if (copy_from_user(&ecreate, arg, sizeof(ecreate)))
+ return -EFAULT;
+
+ secs_page = alloc_page(GFP_KERNEL);
+ if (!secs_page)
+ return -ENOMEM;
+
+ secs = kmap(secs_page);
+ if (copy_from_user(secs, (void __user *)ecreate.src, sizeof(*secs))) {
+ ret = -EFAULT;
+ goto out;
+ }
+
+ ret = sgx_encl_create(encl, secs);
+
+out:
+ kunmap(secs_page);
+ __free_page(secs_page);
+ return ret;
+}
+
+static int sgx_validate_secinfo(struct sgx_secinfo *secinfo)
+{
+ u64 perm = secinfo->flags & SGX_SECINFO_PERMISSION_MASK;
+ u64 pt = secinfo->flags & SGX_SECINFO_PAGE_TYPE_MASK;
+
+ if (pt != SGX_SECINFO_REG && pt != SGX_SECINFO_TCS)
+ return -EINVAL;
+
+ if ((perm & SGX_SECINFO_W) && !(perm & SGX_SECINFO_R))
+ return -EINVAL;
+
+ /*
+ * CPU will silently overwrite the permissions as zero, which means
+ * that we need to validate it ourselves.
+ */
+ if (pt == SGX_SECINFO_TCS && perm)
+ return -EINVAL;
+
+ if (secinfo->flags & SGX_SECINFO_RESERVED_MASK)
+ return -EINVAL;
+
+ if (memchr_inv(secinfo->reserved, 0, sizeof(secinfo->reserved)))
+ return -EINVAL;
+
+ return 0;
+}
+
+static int __sgx_encl_add_page(struct sgx_encl *encl,
+ struct sgx_encl_page *encl_page,
+ struct sgx_epc_page *epc_page,
+ struct sgx_secinfo *secinfo, unsigned long src)
+{
+ struct sgx_pageinfo pginfo;
+ struct vm_area_struct *vma;
+ struct page *src_page;
+ int ret;
+
+ /* Query vma's VM_MAYEXEC as an indirect path_noexec() check. */
+ if (encl_page->vm_max_prot_bits & VM_EXEC) {
+ vma = find_vma(current->mm, src);
+ if (!vma)
+ return -EFAULT;
+
+ if (!(vma->vm_flags & VM_MAYEXEC))
+ return -EACCES;
+ }
+
+ ret = get_user_pages(src, 1, 0, &src_page, NULL);
+ if (ret < 1)
+ return ret;
+
+ pginfo.secs = (unsigned long)sgx_epc_addr(encl->secs.epc_page);
+ pginfo.addr = SGX_ENCL_PAGE_ADDR(encl_page);
+ pginfo.metadata = (unsigned long)secinfo;
+ pginfo.contents = (unsigned long)kmap_atomic(src_page);
+
+ ret = __eadd(&pginfo, sgx_epc_addr(epc_page));
+
+ kunmap_atomic((void *)pginfo.contents);
+ put_page(src_page);
+
+ return ret ? -EIO : 0;
+}
+
+static int __sgx_encl_extend(struct sgx_encl *encl,
+ struct sgx_epc_page *epc_page)
+{
+ int ret;
+ int i;
+
+ for (i = 0; i < 16; i++) {
+ ret = __eextend(sgx_epc_addr(encl->secs.epc_page),
+ sgx_epc_addr(epc_page) + (i * 0x100));
+ if (ret) {
+ if (encls_failed(ret))
+ ENCLS_WARN(ret, "EEXTEND");
+ return -EIO;
+ }
+ }
+
+ return 0;
+}
+
+static int sgx_encl_add_page(struct sgx_encl *encl, unsigned long src,
+ unsigned long offset, unsigned long length,
+ struct sgx_secinfo *secinfo, unsigned long flags)
+{
+ struct sgx_encl_page *encl_page;
+ struct sgx_epc_page *epc_page;
+ int ret;
+
+ encl_page = sgx_encl_page_alloc(encl, offset, secinfo->flags);
+ if (IS_ERR(encl_page))
+ return PTR_ERR(encl_page);
+
+ epc_page = sgx_try_alloc_page();
+ if (IS_ERR(epc_page)) {
+ kfree(encl_page);
+ return PTR_ERR(epc_page);
+ }
+
+ if (atomic_read(&encl->flags) &
+ (SGX_ENCL_INITIALIZED | SGX_ENCL_DEAD)) {
+ ret = -EFAULT;
+ goto err_out_free;
+ }
+
+ down_read(¤t->mm->mmap_sem);
+ mutex_lock(&encl->lock);
+
+ /*
+ * Insert prior to EADD in case of OOM. EADD modifies MRENCLAVE, i.e.
+ * can't be gracefully unwound, while failure on EADD/EXTEND is limited
+ * to userspace errors (or kernel/hardware bugs).
+ */
+ ret = radix_tree_insert(&encl->page_tree, PFN_DOWN(encl_page->desc),
+ encl_page);
+ if (ret)
+ goto err_out_unlock;
+
+ ret = __sgx_encl_add_page(encl, encl_page, epc_page, secinfo,
+ src);
+ if (ret)
+ goto err_out;
+
+ /*
+ * Complete the "add" before doing the "extend" so that the "add"
+ * isn't in a half-baked state in the extremely unlikely scenario the
+ * the enclave will be destroyed in response to EEXTEND failure.
+ */
+ encl_page->encl = encl;
+ encl_page->epc_page = epc_page;
+ encl->secs_child_cnt++;
+
+ if (flags & SGX_PAGE_MEASURE) {
+ ret = __sgx_encl_extend(encl, epc_page);
+ if (ret)
+ goto err_out;
+ }
+
+ mutex_unlock(&encl->lock);
+ up_read(¤t->mm->mmap_sem);
+ return ret;
+
+err_out:
+ radix_tree_delete(&encl_page->encl->page_tree,
+ PFN_DOWN(encl_page->desc));
+
+err_out_unlock:
+ mutex_unlock(&encl->lock);
+ up_read(¤t->mm->mmap_sem);
+
+err_out_free:
+ sgx_free_page(epc_page);
+ kfree(encl_page);
+
+ /*
+ * Destroy enclave on ENCLS failure as this means that EPC has been
+ * invalidated.
+ */
+ if (ret == -EIO)
+ sgx_encl_destroy(encl);
+
+ return ret;
+}
+
+/**
+ * sgx_ioc_enclave_add_pages() - The handler for %SGX_IOC_ENCLAVE_ADD_PAGES
+ * @encl: pointer to an enclave instance (via ioctl() file pointer)
+ * @arg: a user pointer to a struct sgx_enclave_add_pages instance
+ *
+ * Add one or more pages to an uninitialized enclave, and optionally extend the
+ * measurement with the contents of the page. The address range of pages must
+ * be contiguous. The SECINFO and measurement mask are applied to all pages.
+ *
+ * A SECINFO for a TCS is required to always contain zero permissions because
+ * CPU silently zeros them. Allowing anything else would cause a mismatch in
+ * the measurement.
+ *
+ * mmap()'s protection bits are capped by the page permissions. For each page
+ * address, the maximum protection bits are computed with the following
+ * heuristics:
+ *
+ * 1. A regular page: PROT_R, PROT_W and PROT_X match the SECINFO permissions.
+ * 2. A TCS page: PROT_R | PROT_W.
+ * 3. No page: PROT_NONE.
+ *
+ * mmap() is not allowed to surpass the minimum of the maximum protection bits
+ * within the given address range.
+ *
+ * As stated above, a non-existent page is interpreted as a page with no
+ * permissions. In effect, this allows mmap() with PROT_NONE to be used to seek
+ * an address range for the enclave that can be then populated into SECS.
+ *
+ * If ENCLS opcode fails, that effectively means that EPC has been invalidated.
+ * When this happens the enclave is destroyed and -EIO is returned to the
+ * caller.
+ *
+ * Return:
+ * 0 on success,
+ * -EACCES if an executable source page is located in a noexec partition,
+ * -EIO if either ENCLS[EADD] or ENCLS[EEXTEND] fails
+ * -errno otherwise
+ */
+static long sgx_ioc_enclave_add_pages(struct sgx_encl *encl, void __user *arg)
+{
+ struct sgx_enclave_add_pages addp;
+ struct sgx_secinfo secinfo;
+ unsigned long c;
+ int ret;
+
+ if (!(atomic_read(&encl->flags) & SGX_ENCL_CREATED))
+ return -EINVAL;
+
+ if (copy_from_user(&addp, arg, sizeof(addp)))
+ return -EFAULT;
+
+ if (!IS_ALIGNED(addp.offset, PAGE_SIZE) ||
+ !IS_ALIGNED(addp.src, PAGE_SIZE))
+ return -EINVAL;
+
+ if (!(access_ok(addp.src, PAGE_SIZE)))
+ return -EFAULT;
+
+ if (addp.length & (PAGE_SIZE - 1))
+ return -EINVAL;
+
+ if (addp.offset + addp.length - PAGE_SIZE >= encl->size)
+ return -EINVAL;
+
+ if (copy_from_user(&secinfo, (void __user *)addp.secinfo,
+ sizeof(secinfo)))
+ return -EFAULT;
+
+ if (sgx_validate_secinfo(&secinfo))
+ return -EINVAL;
+
+ for (c = 0 ; c < addp.length; c += PAGE_SIZE) {
+ if (signal_pending(current)) {
+ ret = -EINTR;
+ break;
+ }
+
+ if (need_resched())
+ cond_resched();
+
+ ret = sgx_encl_add_page(encl, addp.src + c, addp.offset + c,
+ addp.length - c, &secinfo, addp.flags);
+ if (ret)
+ break;
+ }
+
+ addp.count = c;
+
+ if (copy_to_user(arg, &addp, sizeof(addp)))
+ return -EFAULT;
+
+ return ret;
+}
+
+static int __sgx_get_key_hash(struct crypto_shash *tfm, const void *modulus,
+ void *hash)
+{
+ SHASH_DESC_ON_STACK(shash, tfm);
+
+ shash->tfm = tfm;
+
+ return crypto_shash_digest(shash, modulus, SGX_MODULUS_SIZE, hash);
+}
+
+static int sgx_get_key_hash(const void *modulus, void *hash)
+{
+ struct crypto_shash *tfm;
+ int ret;
+
+ tfm = crypto_alloc_shash("sha256", 0, CRYPTO_ALG_ASYNC);
+ if (IS_ERR(tfm))
+ return PTR_ERR(tfm);
+
+ ret = __sgx_get_key_hash(tfm, modulus, hash);
+
+ crypto_free_shash(tfm);
+ return ret;
+}
+
+static void sgx_update_lepubkeyhash_msrs(u64 *lepubkeyhash, bool enforce)
+{
+ u64 *cache;
+ int i;
+
+ cache = per_cpu(sgx_lepubkeyhash_cache, smp_processor_id());
+ for (i = 0; i < 4; i++) {
+ if (enforce || (lepubkeyhash[i] != cache[i])) {
+ wrmsrl(MSR_IA32_SGXLEPUBKEYHASH0 + i, lepubkeyhash[i]);
+ cache[i] = lepubkeyhash[i];
+ }
+ }
+}
+
+static int sgx_einit(struct sgx_sigstruct *sigstruct,
+ struct sgx_einittoken *token,
+ struct sgx_epc_page *secs, u64 *lepubkeyhash)
+{
+ int ret;
+
+ if (!boot_cpu_has(X86_FEATURE_SGX_LC))
+ return __einit(sigstruct, token, sgx_epc_addr(secs));
+
+ preempt_disable();
+ sgx_update_lepubkeyhash_msrs(lepubkeyhash, false);
+ ret = __einit(sigstruct, token, sgx_epc_addr(secs));
+ if (ret == SGX_INVALID_EINITTOKEN) {
+ sgx_update_lepubkeyhash_msrs(lepubkeyhash, true);
+ ret = __einit(sigstruct, token, sgx_epc_addr(secs));
+ }
+ preempt_enable();
+ return ret;
+}
+
+static int sgx_encl_init(struct sgx_encl *encl, struct sgx_sigstruct *sigstruct,
+ struct sgx_einittoken *token)
+{
+ u64 mrsigner[4];
+ int ret;
+ int i;
+ int j;
+
+ /* Check that the required attributes have been authorized. */
+ if (encl->secs_attributes & ~encl->allowed_attributes)
+ return -EACCES;
+
+ ret = sgx_get_key_hash(sigstruct->modulus, mrsigner);
+ if (ret)
+ return ret;
+
+ mutex_lock(&encl->lock);
+
+ if (atomic_read(&encl->flags) & SGX_ENCL_INITIALIZED) {
+ ret = -EFAULT;
+ goto err_out;
+ }
+
+ for (i = 0; i < SGX_EINIT_SLEEP_COUNT; i++) {
+ for (j = 0; j < SGX_EINIT_SPIN_COUNT; j++) {
+ ret = sgx_einit(sigstruct, token, encl->secs.epc_page,
+ mrsigner);
+ if (ret == SGX_UNMASKED_EVENT)
+ continue;
+ else
+ break;
+ }
+
+ if (ret != SGX_UNMASKED_EVENT)
+ break;
+
+ msleep_interruptible(SGX_EINIT_SLEEP_TIME);
+
+ if (signal_pending(current)) {
+ ret = -ERESTARTSYS;
+ goto err_out;
+ }
+ }
+
+ if (ret & ENCLS_FAULT_FLAG) {
+ if (encls_failed(ret))
+ ENCLS_WARN(ret, "EINIT");
+
+ sgx_encl_destroy(encl);
+ ret = -EFAULT;
+ } else if (ret) {
+ pr_debug("EINIT returned %d\n", ret);
+ ret = -EPERM;
+ } else {
+ atomic_or(SGX_ENCL_INITIALIZED, &encl->flags);
+ }
+
+err_out:
+ mutex_unlock(&encl->lock);
+ return ret;
+}
+
+/**
+ * sgx_ioc_enclave_init - handler for %SGX_IOC_ENCLAVE_INIT
+ *
+ * @filep: open file to /dev/sgx
+ * @arg: userspace pointer to a struct sgx_enclave_init instance
+ *
+ * Flush any outstanding enqueued EADD operations and perform EINIT. The
+ * Launch Enclave Public Key Hash MSRs are rewritten as necessary to match
+ * the enclave's MRSIGNER, which is caculated from the provided sigstruct.
+ *
+ * Return:
+ * 0 on success,
+ * SGX error code on EINIT failure,
+ * -errno otherwise
+ */
+static long sgx_ioc_enclave_init(struct sgx_encl *encl, void __user *arg)
+{
+ struct sgx_einittoken *einittoken;
+ struct sgx_sigstruct *sigstruct;
+ struct sgx_enclave_init einit;
+ struct page *initp_page;
+ int ret;
+
+ if (!(atomic_read(&encl->flags) & SGX_ENCL_CREATED))
+ return -EINVAL;
+
+ if (copy_from_user(&einit, arg, sizeof(einit)))
+ return -EFAULT;
+
+ initp_page = alloc_page(GFP_KERNEL);
+ if (!initp_page)
+ return -ENOMEM;
+
+ sigstruct = kmap(initp_page);
+ einittoken = (struct sgx_einittoken *)
+ ((unsigned long)sigstruct + PAGE_SIZE / 2);
+ memset(einittoken, 0, sizeof(*einittoken));
+
+ if (copy_from_user(sigstruct, (void __user *)einit.sigstruct,
+ sizeof(*sigstruct))) {
+ ret = -EFAULT;
+ goto out;
+ }
+
+ ret = sgx_encl_init(encl, sigstruct, einittoken);
+
+out:
+ kunmap(initp_page);
+ __free_page(initp_page);
+ return ret;
+}
+
+
+long sgx_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
+{
+ struct sgx_encl *encl = filep->private_data;
+ int ret, encl_flags;
+
+ encl_flags = atomic_fetch_or(SGX_ENCL_IOCTL, &encl->flags);
+ if (encl_flags & SGX_ENCL_IOCTL)
+ return -EBUSY;
+
+ if (encl_flags & SGX_ENCL_DEAD)
+ return -EFAULT;
+
+ switch (cmd) {
+ case SGX_IOC_ENCLAVE_CREATE:
+ ret = sgx_ioc_enclave_create(encl, (void __user *)arg);
+ break;
+ case SGX_IOC_ENCLAVE_ADD_PAGES:
+ ret = sgx_ioc_enclave_add_pages(encl, (void __user *)arg);
+ break;
+ case SGX_IOC_ENCLAVE_INIT:
+ ret = sgx_ioc_enclave_init(encl, (void __user *)arg);
+ break;
+ default:
+ ret = -ENOIOCTLCMD;
+ break;
+ }
+
+ atomic_andnot(SGX_ENCL_IOCTL, &encl->flags);
+
+ return ret;
+}
diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
index 60d82e7537c8..842f9abba1c0 100644
--- a/arch/x86/kernel/cpu/sgx/main.c
+++ b/arch/x86/kernel/cpu/sgx/main.c
@@ -8,6 +8,7 @@
#include <linux/ratelimit.h>
#include <linux/sched/signal.h>
#include <linux/slab.h>
+#include "driver.h"
#include "encls.h"
struct sgx_epc_section sgx_epc_sections[SGX_MAX_EPC_SECTIONS];
@@ -193,6 +194,8 @@ static bool __init sgx_page_cache_init(void)
static void __init sgx_init(void)
{
+ int ret;
+
if (!boot_cpu_has(X86_FEATURE_SGX))
return;
@@ -202,10 +205,17 @@ static void __init sgx_init(void)
if (!sgx_page_reclaimer_init())
goto err_page_cache;
+ ret = sgx_drv_init();
+ if (ret)
+ goto err_kthread;
+
return;
+err_kthread:
+ kthread_stop(ksgxswapd_tsk);
+
err_page_cache:
sgx_page_cache_teardown();
}
-arch_initcall(sgx_init);
+device_initcall(sgx_init);
diff --git a/arch/x86/kernel/cpu/sgx/reclaim.c b/arch/x86/kernel/cpu/sgx/reclaim.c
index 215371588a25..9e6d3e147aa2 100644
--- a/arch/x86/kernel/cpu/sgx/reclaim.c
+++ b/arch/x86/kernel/cpu/sgx/reclaim.c
@@ -10,6 +10,7 @@
#include <linux/sched/mm.h>
#include <linux/sched/signal.h>
#include "encls.h"
+#include "driver.h"
struct task_struct *ksgxswapd_tsk;
--
2.20.1
^ permalink raw reply related
* [PATCH v26 13/22] x86/sgx: Add provisioning
From: Jarkko Sakkinen @ 2020-02-09 21:26 UTC (permalink / raw)
To: linux-kernel, x86, linux-sgx
Cc: akpm, dave.hansen, sean.j.christopherson, nhorman, npmccallum,
haitao.huang, andriy.shevchenko, tglx, kai.svahn, bp, josh, luto,
kai.huang, rientjes, cedric.xing, puiterwijk, Jarkko Sakkinen,
linux-security-module
In-Reply-To: <20200209212609.7928-1-jarkko.sakkinen@linux.intel.com>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=a, Size: 5616 bytes --]
In order to provide a mechanism for devilering provisoning rights:
1. Add a new device file /dev/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 attribute_fd;
};
A daemon could sit on top of /dev/sgx/provision and send a file
descriptor of this file to a process that needs to be able to provision
enclaves.
The way this API is used is straight-forward. Lets assume that dev_fd is
a handle to /dev/sgx/enclave and prov_fd is a handle to
/dev/sgx/provision. You would allow SGX_IOC_ENCLAVE_CREATE to
initialize an enclave with the PROVISIONKEY attribute by
params.addr = <enclave address>;
params.token_fd = prov_fd;
ioctl(dev_fd, SGX_IOC_ENCLAVE_SET_ATTRIBUTE, ¶ms);
Cc: linux-security-module@vger.kernel.org
Suggested-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
arch/x86/include/uapi/asm/sgx.h | 11 ++++++++
arch/x86/kernel/cpu/sgx/driver.c | 14 ++++++++++
arch/x86/kernel/cpu/sgx/driver.h | 2 ++
arch/x86/kernel/cpu/sgx/ioctl.c | 47 ++++++++++++++++++++++++++++++++
4 files changed, 74 insertions(+)
diff --git a/arch/x86/include/uapi/asm/sgx.h b/arch/x86/include/uapi/asm/sgx.h
index 5edb08ab8fd0..57d0d30c79b3 100644
--- a/arch/x86/include/uapi/asm/sgx.h
+++ b/arch/x86/include/uapi/asm/sgx.h
@@ -25,6 +25,8 @@ enum sgx_page_flags {
_IOWR(SGX_MAGIC, 0x01, struct sgx_enclave_add_pages)
#define SGX_IOC_ENCLAVE_INIT \
_IOW(SGX_MAGIC, 0x02, struct sgx_enclave_init)
+#define SGX_IOC_ENCLAVE_SET_ATTRIBUTE \
+ _IOW(SGX_MAGIC, 0x03, struct sgx_enclave_set_attribute)
/**
* struct sgx_enclave_create - parameter structure for the
@@ -63,4 +65,13 @@ struct sgx_enclave_init {
__u64 sigstruct;
};
+/**
+ * struct sgx_enclave_set_attribute - parameter structure for the
+ * %SGX_IOC_ENCLAVE_SET_ATTRIBUTE ioctl
+ * @attribute_fd: file handle of the attribute file in the securityfs
+ */
+struct sgx_enclave_set_attribute {
+ __u64 attribute_fd;
+};
+
#endif /* _UAPI_ASM_X86_SGX_H */
diff --git a/arch/x86/kernel/cpu/sgx/driver.c b/arch/x86/kernel/cpu/sgx/driver.c
index b4aa7b9f8376..d90114cec1c3 100644
--- a/arch/x86/kernel/cpu/sgx/driver.c
+++ b/arch/x86/kernel/cpu/sgx/driver.c
@@ -150,6 +150,13 @@ static struct miscdevice sgx_dev_enclave = {
.fops = &sgx_encl_fops,
};
+static struct miscdevice sgx_dev_provision = {
+ .minor = MISC_DYNAMIC_MINOR,
+ .name = "provision",
+ .nodename = "sgx/provision",
+ .fops = &sgx_provision_fops,
+};
+
int __init sgx_drv_init(void)
{
unsigned int eax, ebx, ecx, edx;
@@ -190,5 +197,12 @@ int __init sgx_drv_init(void)
return ret;
}
+ ret = misc_register(&sgx_dev_provision);
+ if (ret) {
+ pr_err("Creating /dev/sgx/provision failed with %d.\n", ret);
+ misc_deregister(&sgx_dev_enclave);
+ return ret;
+ }
+
return 0;
}
diff --git a/arch/x86/kernel/cpu/sgx/driver.h b/arch/x86/kernel/cpu/sgx/driver.h
index e4063923115b..72747d01c046 100644
--- a/arch/x86/kernel/cpu/sgx/driver.h
+++ b/arch/x86/kernel/cpu/sgx/driver.h
@@ -23,6 +23,8 @@ extern u64 sgx_attributes_reserved_mask;
extern u64 sgx_xfrm_reserved_mask;
extern u32 sgx_xsave_size_tbl[64];
+extern const struct file_operations sgx_provision_fops;
+
long sgx_ioctl(struct file *filep, unsigned int cmd, unsigned long arg);
int sgx_drv_init(void);
diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
index 83513cdfd1c0..262001df3ae4 100644
--- a/arch/x86/kernel/cpu/sgx/ioctl.c
+++ b/arch/x86/kernel/cpu/sgx/ioctl.c
@@ -663,6 +663,50 @@ static long sgx_ioc_enclave_init(struct sgx_encl *encl, void __user *arg)
return ret;
}
+/**
+ * sgx_ioc_enclave_set_attribute - handler for %SGX_IOC_ENCLAVE_SET_ATTRIBUTE
+ * @filep: open file to /dev/sgx
+ * @arg: userspace pointer to a struct sgx_enclave_set_attribute instance
+ *
+ * Mark the enclave as being allowed to access a restricted attribute bit.
+ * The requested attribute is specified via the attribute_fd field in the
+ * provided struct sgx_enclave_set_attribute. The attribute_fd must be a
+ * handle to an SGX attribute file, e.g. “/dev/sgx/provision".
+ *
+ * Failure to explicitly request access to a restricted attribute will cause
+ * sgx_ioc_enclave_init() to fail. Currently, the only restricted attribute
+ * is access to the PROVISION_KEY.
+ *
+ * Note, access to the EINITTOKEN_KEY is disallowed entirely.
+ *
+ * Return: 0 on success, -errno otherwise
+ */
+static long sgx_ioc_enclave_set_attribute(struct sgx_encl *encl,
+ void __user *arg)
+{
+ struct sgx_enclave_set_attribute params;
+ struct file *attribute_file;
+ int ret;
+
+ if (copy_from_user(¶ms, arg, sizeof(params)))
+ return -EFAULT;
+
+ attribute_file = fget(params.attribute_fd);
+ if (!attribute_file)
+ return -EINVAL;
+
+ if (attribute_file->f_op != &sgx_provision_fops) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ encl->allowed_attributes |= SGX_ATTR_PROVISIONKEY;
+ ret = 0;
+
+out:
+ fput(attribute_file);
+ return ret;
+}
long sgx_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
{
@@ -686,6 +730,9 @@ long sgx_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
case SGX_IOC_ENCLAVE_INIT:
ret = sgx_ioc_enclave_init(encl, (void __user *)arg);
break;
+ case SGX_IOC_ENCLAVE_SET_ATTRIBUTE:
+ ret = sgx_ioc_enclave_set_attribute(encl, (void __user *)arg);
+ break;
default:
ret = -ENOIOCTLCMD;
break;
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 1/2] crypto: sm3 - add a new alias name sm3-256
From: Eric Biggers @ 2020-02-10 3:17 UTC (permalink / raw)
To: Tianjia Zhang
Cc: herbert, davem, zohar, dmitry.kasatkin, jmorris, serge,
linux-crypto, linux-integrity, linux-security-module,
linux-kernel
In-Reply-To: <20200207092219.115056-2-tianjia.zhang@linux.alibaba.com>
On Fri, Feb 07, 2020 at 05:22:18PM +0800, Tianjia Zhang wrote:
> The name sm3-256 is defined in hash_algo_name in hash_info, but the
> algorithm name implemented in sm3_generic.c is sm3, which will cause
> the sm3-256 algorithm to be not found in some application scenarios of
> the hash algorithm, and an ENOENT error will occur. For example,
> IMA, keys, and other subsystems that reference hash_algo_name cannot use
> the hash algorithm of sm3. This patch adds an alias name sm3-256 to sm3,
> which can better solve the above problems.
>
> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
> ---
> crypto/sm3_generic.c | 21 +++++++++++++++++----
> 1 file changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/crypto/sm3_generic.c b/crypto/sm3_generic.c
> index 3468975215ca..ded41031bd5f 100644
> --- a/crypto/sm3_generic.c
> +++ b/crypto/sm3_generic.c
> @@ -163,7 +163,7 @@ int crypto_sm3_finup(struct shash_desc *desc, const u8 *data,
> }
> EXPORT_SYMBOL(crypto_sm3_finup);
>
> -static struct shash_alg sm3_alg = {
> +static struct shash_alg sm3_algs[2] = { {
> .digestsize = SM3_DIGEST_SIZE,
> .init = sm3_base_init,
> .update = crypto_sm3_update,
> @@ -176,16 +176,28 @@ static struct shash_alg sm3_alg = {
> .cra_blocksize = SM3_BLOCK_SIZE,
> .cra_module = THIS_MODULE,
> }
> -};
> +}, {
> + .digestsize = SM3_DIGEST_SIZE,
> + .init = sm3_base_init,
> + .update = crypto_sm3_update,
> + .final = sm3_final,
> + .finup = crypto_sm3_finup,
> + .descsize = sizeof(struct sm3_state),
> + .base = {
> + .cra_name = "sm3-256",
> + .cra_blocksize = SM3_BLOCK_SIZE,
> + .cra_module = THIS_MODULE,
> + }
> +} };
According to https://tools.ietf.org/id/draft-oscca-cfrg-sm3-01.html,
SM3 always produces a 256-bit hash value. E.g., it says:
"SM3 produces an output hash value of 256 bits long"
and
"SM3 is a hash function that generates a 256-bit hash value."
I don't see any mention of "SM3-256".
So why not just keep it as "sm3" and change hash_info.c instead?
Since the name there is currently wrong, no one can be using it yet.
- Eric
^ permalink raw reply
* Re: [PATCH v2] ima: export the measurement list when needed
From: Janne Karhunen @ 2020-02-10 8:04 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-integrity, linux-security-module, Ken Goldman,
david.safford, Wiseman, Monty (GE Global Research, US),
Amir Goldstein, linux-fsdevel
In-Reply-To: <1580998432.5585.411.camel@linux.ibm.com>
On Thu, Feb 6, 2020 at 4:14 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> The implications of exporting and removing records from the IMA-
> measurement list needs to be considered carefully. Verifying a TPM
> quote will become dependent on knowing where the measurements are
> stored. The existing measurement list is stored in kernel memory and,
> barring a kernel memory attack, is protected from modification.
> Before upstreaming this or a similar patch, there needs to be a
> discussion as to how the measurement list will be protected once is it
> exported to userspace.
>
> This patch now attempts to address two very different scenarios. The
> first scenario is where userspace is requesting exporting and removing
> of the measurement list records. The other scenario is the kernel
> exporting and removing of the measurement list records. Conflating
> these two different use cases might not be the right solution, as we
> originally thought.
>
> The kernel already exports the IMA measurement list to userspace via a
> securityfs file. From a userspace perspective, missing is the ability
> of removing N number of records. In this scenario, userspace would be
> responsible for safely storing the measurements (e.g. blockchain).
> The kernel would only be responsible for limiting permission, perhaps
> based on a capability, before removing records from the measurement
> list.
This is a good point. I will adapt the patch to this.
> In the kernel usecase, somehow the kernel would need to safely export
> the measurement list, or some portion of the measurement list, to a
> file and then delete that portion. What protects the exported records
> stored in a file from modification?
Are we looking at protecting this file from a root exploit and the
potential DOS it might cause? In the original patch the file was root
writable only. As far as further limitations go, the easiest would
probably be to use the file immutable bit. If the kernel opens the
file and sets the immutable bit, it is the only entity that can ever
write to it - not even another root task could directly write to it.
The kernel could, as long as it keeps the file open.
> Instead of exporting the measurement records, one option as suggested
> by Amir Goldstein, would be to use a vfs_tmpfile() to get an anonymous
> file for backing store. The existing securityfs measurement lists
> would then read from this private copy of the anonymous file.
>
> I've Cc'ed fsdevel for additional comments/suggestions.
I didn't quickly see what the actual problem is that the vfs_tmpfile
solves in this context, will check.
--
Janne
^ permalink raw reply
* [PATCH v3 0/8] ima: support stronger algorithms for attestation
From: Roberto Sassu @ 2020-02-10 10:00 UTC (permalink / raw)
To: zohar, James.Bottomley, jarkko.sakkinen
Cc: linux-integrity, linux-security-module, linux-kernel,
silviu.vlasceanu, Roberto Sassu
IMA extends Platform Configuration Registers (PCRs) of the TPM to give a
proof to a remote verifier that the measurement list contains all
measurements done by the kernel and that the list was not maliciously
modified by an attacker.
IMA was originally designed to extend PCRs with a SHA1 digest, provided
with the measurement list, and was subsequently updated to extend all PCR
banks in case a TPM 2.0 is used. Non-SHA1 PCR banks are not supposed to be
used for remote attestation, as they are extended with a SHA1 digest padded
with zeros, which does not increase the strength.
This patch set addresses this issue by extending PCRs with the digest of
the measurement entry calculated with the crypto subsystem. The list of
algorithms used to calculate the digest are taken from
ima_tpm_chip->allocated_banks, returned by the TPM driver. The SHA1 digest
is always calculated, as SHA1 still remains the default algorithm for the
template digest in the measurement list.
This patch set also makes two additional modifications related to the usage
of hash algorithms. First, since now the template digest for the default
IMA algorithm is always calculated, this is used for hash collision
detection, to check if there are duplicate measurement entries.
Second, it uses the default IMA hash algorithm to calculate the boot
aggregate, assuming that the corresponding PCR bank is currently allocated.
Otherwise, it finds the SHA256 PCR bank (mandatory for TPM 2.0 in TCG PC
Client specification). Lastly, if that bank was not found, it selects the
first PCR bank. If the TPM algorithm ID of the first PCR bank is not mapped
to a crypto ID, boot_aggregate is set to zero.
This patch set does not yet modify the format of the measurement list to
provide the digests passed to the TPM. However, reconstructing the value of
the quoted PCR is still possible for the verifier by calculating the digest
on measurement data found in binary_runtime_measurements.
attest-tools (https://github.com/euleros/attest-tools, branch 0.2-devel)
has the ability to parse the BIOS and IMA event logs, and to compare
boot_aggregate with the digest of final PCR values obtained by performing
in software the PCR extend operation with digests in the BIOS event log.
To perform the test, it is necessary to have a complete BIOS event log and
to apply the boot_aggregate patches. It would be possible to use qemu and
swtpm from Stefan Berger, but at the moment it is necessary to change the
ACPI parser in drivers/char/tpm/event_log/acpi.c to accept TPM 2.0 and to
return EFI_TCG2_EVENT_LOG_FORMAT_TCG_2.
Create req.json with this content:
---
{
"reqs":{
"dummy|verify":"",
"ima_boot_aggregate|verify":""
}
}
---
With the requirements above, attest-tools verifies boot_aggregate and
accepts any other entry in the event logs.
On server side run:
# attest_ra_server -p 10 -r req.json -s -i
-s disables TPM signature verification
-i allows IMA violations
To enable TPM signature verification it is necessary to have a valid AK
certificate. It can be obtained by following the instructions at:
https://github.com/euleros/attest-tools/blob/0.2-devel/README
On client side run:
# echo test > aik_cert.pem
# echo aik_cert.pem > list_privacy_ca
# attest_ra_client -A
The commands above generate an AK and tell attest-tools to use a dummy AK
certificate.
# attest_ra_client -s <server IP> -q -p 10 -P <PCR algo> -b -i
The command above sends the TPM quote and the event logs to the RA server
and gets the response (successful/failed verification).
-b includes the BIOS event log from securityfs
-i includes the IMA event log from securityfs
To check that IMA extends non-SHA1 PCR banks with an appropriate digest,
use -P sha256, so that attest_ra_client selects the SHA256 PCR bank. To
check that boot_aggregate is calculated properly, set ima_hash=sha256 in
the kernel command line.
Changelog
v2:
- add NR_BANKS macro to return zero if ima_tpm_chip is NULL
- replace ima_num_template_digests with
NR_BANKS(ima_tpm_chip) + ima_extra_slots (suggested by Mimi)
- add __ro_after_init to declaration of ima_sha1_idx ima_hash_algo_idx and
ima_extra_slots (suggested by Mimi)
- declare ima_init_ima_crypto() as static (reported by kbuild test robot)
- use ima_sha1_idx and ima_hash_algo_idx to access ima_algo_array elements
in ima_init_crypto()
v1:
- move ima_sha1_idx and ima_hash_algo_idx to ima_crypto.c
- introduce ima_num_template_digests (suggested by Mimi)
- determine ima_num_template_digests before allocating ima_algo_array
(suggested by Mimi)
- replace kmalloc_array() with kcalloc() in ima_init_crypto() (suggested by
Mimi)
- check if ima_tpm_chip is NULL
Roberto Sassu (8):
tpm: Initialize crypto_id of allocated_banks to HASH_ALGO__LAST
ima: Switch to ima_hash_algo for boot aggregate
ima: Evaluate error in init_ima()
ima: Store template digest directly in ima_template_entry
ima: Switch to dynamically allocated buffer for template digests
ima: Allocate and initialize tfm for each PCR bank
ima: Calculate and extend PCR with digests in ima_template_entry
ima: Use ima_hash_algo for collision detection in the measurement list
drivers/char/tpm/tpm2-cmd.c | 2 +
security/integrity/ima/ima.h | 10 +-
security/integrity/ima/ima_api.c | 20 +--
security/integrity/ima/ima_crypto.c | 244 ++++++++++++++++++++++----
security/integrity/ima/ima_fs.c | 4 +-
security/integrity/ima/ima_init.c | 22 ++-
security/integrity/ima/ima_main.c | 3 +
security/integrity/ima/ima_queue.c | 36 ++--
security/integrity/ima/ima_template.c | 22 ++-
9 files changed, 290 insertions(+), 73 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH v3 1/8] tpm: Initialize crypto_id of allocated_banks to HASH_ALGO__LAST
From: Roberto Sassu @ 2020-02-10 10:00 UTC (permalink / raw)
To: zohar, James.Bottomley, jarkko.sakkinen
Cc: linux-integrity, linux-security-module, linux-kernel,
silviu.vlasceanu, Roberto Sassu, stable
In-Reply-To: <20200210100048.21448-1-roberto.sassu@huawei.com>
chip->allocated_banks, an array of tpm_bank_info structures, contains the
list of TPM algorithm IDs of allocated PCR banks. It also contains the
corresponding ID of the crypto subsystem, so that users of the TPM driver
can calculate a digest for a PCR extend operation.
However, if there is no mapping between TPM algorithm ID and crypto ID, the
crypto_id field of tpm_bank_info remains set to zero (the array is
allocated and initialized with kcalloc() in tpm2_get_pcr_allocation()).
Zero should not be used as value for unknown mappings, as it is a valid
crypto ID (HASH_ALGO_MD4).
Thus, initialize crypto_id to HASH_ALGO__LAST.
Cc: stable@vger.kernel.org # 5.1.x
Fixes: 879b589210a9 ("tpm: retrieve digest size of unknown algorithms with PCR read")
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
---
drivers/char/tpm/tpm2-cmd.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 13696deceae8..760329598b99 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -525,6 +525,8 @@ static int tpm2_init_bank_info(struct tpm_chip *chip, u32 bank_index)
return 0;
}
+ bank->crypto_id = HASH_ALGO__LAST;
+
return tpm2_pcr_read(chip, 0, &digest, &bank->digest_size);
}
--
2.17.1
^ permalink raw reply related
* [PATCH v3 2/8] ima: Switch to ima_hash_algo for boot aggregate
From: Roberto Sassu @ 2020-02-10 10:00 UTC (permalink / raw)
To: zohar, James.Bottomley, jarkko.sakkinen
Cc: linux-integrity, linux-security-module, linux-kernel,
silviu.vlasceanu, Roberto Sassu, stable
In-Reply-To: <20200210100048.21448-1-roberto.sassu@huawei.com>
boot_aggregate is the first entry of IMA measurement list. Its purpose is
to link pre-boot measurements to IMA measurements. As IMA was designed to
work with a TPM 1.2, the SHA1 PCR bank was always selected.
Currently, even if a TPM 2.0 is used, the SHA1 PCR bank is selected.
However, the assumption that the SHA1 PCR bank is always available is not
correct, as PCR banks can be selected with the PCR_Allocate() TPM command.
This patch tries to use ima_hash_algo as hash algorithm for boot_aggregate.
If no PCR bank uses that algorithm, the patch tries to find the SHA256 PCR
bank (which is mandatory in the TCG PC Client specification). If also this
bank is not found, the patch selects the first one. If the TPM algorithm
of that bank is not mapped to a crypto ID, boot_aggregate is set to zero.
Changelog
v1:
- add Mimi's comments
- if there is no PCR bank for the IMA default algorithm use SHA256
(suggested by James Bottomley)
Cc: stable@vger.kernel.org # 5.1.x
Fixes: 879b589210a9 ("tpm: retrieve digest size of unknown algorithms with PCR read")
Reported-by: Jerry Snitselaar <jsnitsel@redhat.com>
Suggested-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
security/integrity/ima/ima_crypto.c | 45 +++++++++++++++++++++++++----
security/integrity/ima/ima_init.c | 22 ++++++++++----
2 files changed, 56 insertions(+), 11 deletions(-)
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 73044fc6a952..f2f41a2bc3d4 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -655,18 +655,29 @@ static void __init ima_pcrread(u32 idx, struct tpm_digest *d)
}
/*
- * Calculate the boot aggregate hash
+ * The boot_aggregate is a cumulative hash over TPM registers 0 - 7. With
+ * TPM 1.2 the boot_aggregate was based on reading the SHA1 PCRs, but with
+ * TPM 2.0 hash agility, TPM chips could support multiple TPM PCR banks,
+ * allowing firmware to configure and enable different banks.
+ *
+ * Knowing which TPM bank is read to calculate the boot_aggregate digest
+ * needs to be conveyed to a verifier. For this reason, use the same
+ * hash algorithm for reading the TPM PCRs as for calculating the boot
+ * aggregate digest as stored in the measurement list.
*/
-static int __init ima_calc_boot_aggregate_tfm(char *digest,
+static int __init ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id,
struct crypto_shash *tfm)
{
- struct tpm_digest d = { .alg_id = TPM_ALG_SHA1, .digest = {0} };
+ struct tpm_digest d = { .alg_id = alg_id, .digest = {0} };
int rc;
u32 i;
SHASH_DESC_ON_STACK(shash, tfm);
shash->tfm = tfm;
+ pr_devel("calculating the boot-aggregate based on TPM bank: %04x\n",
+ d.alg_id);
+
rc = crypto_shash_init(shash);
if (rc != 0)
return rc;
@@ -675,7 +686,8 @@ static int __init ima_calc_boot_aggregate_tfm(char *digest,
for (i = TPM_PCR0; i < TPM_PCR8; i++) {
ima_pcrread(i, &d);
/* now accumulate with current aggregate */
- rc = crypto_shash_update(shash, d.digest, TPM_DIGEST_SIZE);
+ rc = crypto_shash_update(shash, d.digest,
+ crypto_shash_digestsize(tfm));
}
if (!rc)
crypto_shash_final(shash, digest);
@@ -685,14 +697,35 @@ static int __init ima_calc_boot_aggregate_tfm(char *digest,
int __init ima_calc_boot_aggregate(struct ima_digest_data *hash)
{
struct crypto_shash *tfm;
- int rc;
+ u16 crypto_id, alg_id;
+ int rc, i, bank_idx = 0;
+
+ for (i = 0; i < ima_tpm_chip->nr_allocated_banks; i++) {
+ crypto_id = ima_tpm_chip->allocated_banks[i].crypto_id;
+ if (crypto_id == hash->algo) {
+ bank_idx = i;
+ break;
+ }
+
+ if (crypto_id == HASH_ALGO_SHA256)
+ bank_idx = i;
+ }
+
+ if (bank_idx == 0 &&
+ ima_tpm_chip->allocated_banks[0].crypto_id == HASH_ALGO__LAST) {
+ pr_err("No suitable TPM algorithm for boot aggregate\n");
+ return 0;
+ }
+
+ hash->algo = ima_tpm_chip->allocated_banks[bank_idx].crypto_id;
tfm = ima_alloc_tfm(hash->algo);
if (IS_ERR(tfm))
return PTR_ERR(tfm);
hash->length = crypto_shash_digestsize(tfm);
- rc = ima_calc_boot_aggregate_tfm(hash->digest, tfm);
+ alg_id = ima_tpm_chip->allocated_banks[bank_idx].alg_id;
+ rc = ima_calc_boot_aggregate_tfm(hash->digest, alg_id, tfm);
ima_free_tfm(tfm);
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 5d55ade5f3b9..fbd7a8e28a6b 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -27,7 +27,7 @@ struct tpm_chip *ima_tpm_chip;
/* Add the boot aggregate to the IMA measurement list and extend
* the PCR register.
*
- * Calculate the boot aggregate, a SHA1 over tpm registers 0-7,
+ * Calculate the boot aggregate, a hash over tpm registers 0-7,
* assuming a TPM chip exists, and zeroes if the TPM chip does not
* exist. Add the boot aggregate measurement to the measurement
* list and extend the PCR register.
@@ -51,15 +51,27 @@ static int __init ima_add_boot_aggregate(void)
int violation = 0;
struct {
struct ima_digest_data hdr;
- char digest[TPM_DIGEST_SIZE];
+ char digest[TPM_MAX_DIGEST_SIZE];
} hash;
memset(iint, 0, sizeof(*iint));
memset(&hash, 0, sizeof(hash));
iint->ima_hash = &hash.hdr;
- iint->ima_hash->algo = HASH_ALGO_SHA1;
- iint->ima_hash->length = SHA1_DIGEST_SIZE;
-
+ iint->ima_hash->algo = ima_hash_algo;
+ iint->ima_hash->length = hash_digest_size[ima_hash_algo];
+
+ /*
+ * With TPM 2.0 hash agility, TPM chips could support multiple TPM
+ * PCR banks, allowing firmware to configure and enable different
+ * banks. The SHA1 bank is not necessarily enabled.
+ *
+ * Use the same hash algorithm for reading the TPM PCRs as for
+ * calculating the boot aggregate digest. Preference is given to
+ * the configured IMA default hash algorithm. Otherwise, use the
+ * TPM required banks - SHA256 for TPM 2.0, SHA1 for TPM 1.2. If
+ * SHA256 is not available, use the first PCR bank and if that is
+ * not mapped to a crypto ID, set boot_aggregate to zero.
+ */
if (ima_tpm_chip) {
result = ima_calc_boot_aggregate(&hash.hdr);
if (result < 0) {
--
2.17.1
^ permalink raw reply related
* [PATCH v3 3/8] ima: Evaluate error in init_ima()
From: Roberto Sassu @ 2020-02-10 10:00 UTC (permalink / raw)
To: zohar, James.Bottomley, jarkko.sakkinen
Cc: linux-integrity, linux-security-module, linux-kernel,
silviu.vlasceanu, Roberto Sassu, stable
In-Reply-To: <20200210100048.21448-1-roberto.sassu@huawei.com>
Evaluate error in init_ima() before register_blocking_lsm_notifier() and
return if not zero.
Cc: stable@vger.kernel.org # 5.3.x
Fixes: b16942455193 ("ima: use the lsm policy update notifier")
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
security/integrity/ima/ima_main.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index d7e987baf127..a16c148ed90d 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -738,6 +738,9 @@ static int __init init_ima(void)
error = ima_init();
}
+ if (error)
+ return error;
+
error = register_blocking_lsm_notifier(&ima_lsm_policy_notifier);
if (error)
pr_warn("Couldn't register LSM notifier, error %d\n", error);
--
2.17.1
^ permalink raw reply related
* [PATCH v3 4/8] ima: Store template digest directly in ima_template_entry
From: Roberto Sassu @ 2020-02-10 10:00 UTC (permalink / raw)
To: zohar, James.Bottomley, jarkko.sakkinen
Cc: linux-integrity, linux-security-module, linux-kernel,
silviu.vlasceanu, Roberto Sassu
In-Reply-To: <20200210100048.21448-1-roberto.sassu@huawei.com>
In preparation for the patch that calculates a digest for each allocated
PCR bank, this patch passes to ima_calc_field_array_hash() the
ima_template_entry structure, so that digests can be directly stored in
that structure instead of ima_digest_data.
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
security/integrity/ima/ima.h | 3 +--
security/integrity/ima/ima_api.c | 12 +-----------
security/integrity/ima/ima_crypto.c | 18 +++++++-----------
3 files changed, 9 insertions(+), 24 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index df4ca482fb53..2f380fb92a7a 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -138,8 +138,7 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash);
int ima_calc_buffer_hash(const void *buf, loff_t len,
struct ima_digest_data *hash);
int ima_calc_field_array_hash(struct ima_field_data *field_data,
- struct ima_template_desc *desc, int num_fields,
- struct ima_digest_data *hash);
+ struct ima_template_entry *entry);
int __init ima_calc_boot_aggregate(struct ima_digest_data *hash);
void ima_add_violation(struct file *file, const unsigned char *filename,
struct integrity_iint_cache *iint,
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 610759fe63b8..51f562111864 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -96,26 +96,16 @@ int ima_store_template(struct ima_template_entry *entry,
static const char audit_cause[] = "hashing_error";
char *template_name = entry->template_desc->name;
int result;
- struct {
- struct ima_digest_data hdr;
- char digest[TPM_DIGEST_SIZE];
- } hash;
if (!violation) {
- int num_fields = entry->template_desc->num_fields;
-
- /* this function uses default algo */
- hash.hdr.algo = HASH_ALGO_SHA1;
result = ima_calc_field_array_hash(&entry->template_data[0],
- entry->template_desc,
- num_fields, &hash.hdr);
+ entry);
if (result < 0) {
integrity_audit_msg(AUDIT_INTEGRITY_PCR, inode,
template_name, op,
audit_cause, result, 0);
return result;
}
- memcpy(entry->digest, hash.hdr.digest, hash.hdr.length);
}
entry->pcr = pcr;
result = ima_add_template_entry(entry, violation, op, inode, filename);
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index f2f41a2bc3d4..2d356ae8e823 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -464,18 +464,16 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
* Calculate the hash of template data
*/
static int ima_calc_field_array_hash_tfm(struct ima_field_data *field_data,
- struct ima_template_desc *td,
- int num_fields,
- struct ima_digest_data *hash,
+ struct ima_template_entry *entry,
struct crypto_shash *tfm)
{
SHASH_DESC_ON_STACK(shash, tfm);
+ struct ima_template_desc *td = entry->template_desc;
+ int num_fields = entry->template_desc->num_fields;
int rc, i;
shash->tfm = tfm;
- hash->length = crypto_shash_digestsize(tfm);
-
rc = crypto_shash_init(shash);
if (rc != 0)
return rc;
@@ -504,24 +502,22 @@ static int ima_calc_field_array_hash_tfm(struct ima_field_data *field_data,
}
if (!rc)
- rc = crypto_shash_final(shash, hash->digest);
+ rc = crypto_shash_final(shash, entry->digest);
return rc;
}
int ima_calc_field_array_hash(struct ima_field_data *field_data,
- struct ima_template_desc *desc, int num_fields,
- struct ima_digest_data *hash)
+ struct ima_template_entry *entry)
{
struct crypto_shash *tfm;
int rc;
- tfm = ima_alloc_tfm(hash->algo);
+ tfm = ima_alloc_tfm(HASH_ALGO_SHA1);
if (IS_ERR(tfm))
return PTR_ERR(tfm);
- rc = ima_calc_field_array_hash_tfm(field_data, desc, num_fields,
- hash, tfm);
+ rc = ima_calc_field_array_hash_tfm(field_data, entry, tfm);
ima_free_tfm(tfm);
--
2.17.1
^ permalink raw reply related
* [PATCH v3 5/8] ima: Switch to dynamically allocated buffer for template digests
From: Roberto Sassu @ 2020-02-10 10:02 UTC (permalink / raw)
To: zohar, James.Bottomley, jarkko.sakkinen
Cc: linux-integrity, linux-security-module, linux-kernel,
silviu.vlasceanu, Roberto Sassu
This patch dynamically allocates the array of tpm_digest structures in
ima_alloc_init_template() and ima_restore_template_data(). The size of the
array is equal to the number of PCR banks plus ima_extra_slots, to make
room for SHA1 and the IMA default algorithm, when PCR banks with those
algorithms are not allocated.
Calculating the SHA1 digest is mandatory, as SHA1 still remains the default
hash algorithm for the measurement list. When IMA will support the Crypto
Agile format, remaining digests will be also provided.
The position in the measurement entry array of the SHA1 digest is stored in
the ima_sha1_idx global variable and is determined at IMA initialization
time.
Changelog
v2:
- add NR_BANKS macro to return zero if ima_tpm_chip is NULL
- replace ima_num_template_digests with
NR_BANKS(ima_tpm_chip) + ima_extra_slots (suggested by Mimi)
- add __ro_after_init to declaration of ima_sha1_idx and ima_extra_slots
(suggested by Mimi)
v1:
- move ima_sha1_idx to ima_crypto.c
- introduce ima_num_template_digests (suggested by Mimi)
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
security/integrity/ima/ima.h | 6 +++++-
security/integrity/ima/ima_api.c | 8 ++++++++
security/integrity/ima/ima_crypto.c | 7 ++++++-
security/integrity/ima/ima_fs.c | 4 ++--
security/integrity/ima/ima_queue.c | 10 ++++++----
security/integrity/ima/ima_template.c | 12 ++++++++++--
6 files changed, 37 insertions(+), 10 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 2f380fb92a7a..f04bec2ab83f 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -45,11 +45,15 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8 };
#define IMA_TEMPLATE_IMA_NAME "ima"
#define IMA_TEMPLATE_IMA_FMT "d|n"
+#define NR_BANKS(chip) ((chip != NULL) ? chip->nr_allocated_banks : 0)
+
/* current content of the policy */
extern int ima_policy_flag;
/* set during initialization */
extern int ima_hash_algo;
+extern int ima_sha1_idx __ro_after_init;
+extern int ima_extra_slots __ro_after_init;
extern int ima_appraise;
extern struct tpm_chip *ima_tpm_chip;
@@ -92,7 +96,7 @@ struct ima_template_desc {
struct ima_template_entry {
int pcr;
- u8 digest[TPM_DIGEST_SIZE]; /* sha1 or md5 measurement hash */
+ struct tpm_digest *digests;
struct ima_template_desc *template_desc; /* template descriptor */
u32 template_data_len;
struct ima_field_data template_data[0]; /* template related data */
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 51f562111864..ea6d834a5fab 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -27,6 +27,7 @@ void ima_free_template_entry(struct ima_template_entry *entry)
for (i = 0; i < entry->template_desc->num_fields; i++)
kfree(entry->template_data[i].data);
+ kfree(entry->digests);
kfree(entry);
}
@@ -50,6 +51,13 @@ int ima_alloc_init_template(struct ima_event_data *event_data,
if (!*entry)
return -ENOMEM;
+ (*entry)->digests = kcalloc(NR_BANKS(ima_tpm_chip) + ima_extra_slots,
+ sizeof(*(*entry)->digests), GFP_NOFS);
+ if (!(*entry)->digests) {
+ result = -ENOMEM;
+ goto out;
+ }
+
(*entry)->template_desc = template_desc;
for (i = 0; i < template_desc->num_fields; i++) {
const struct ima_template_field *field =
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 2d356ae8e823..ea24d2f6b513 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -59,6 +59,10 @@ MODULE_PARM_DESC(ahash_bufsize, "Maximum ahash buffer size");
static struct crypto_shash *ima_shash_tfm;
static struct crypto_ahash *ima_ahash_tfm;
+int ima_sha1_idx __ro_after_init;
+/* Additional number of slots to be reserved for SHA1 and IMA default algo */
+int ima_extra_slots __ro_after_init = 1;
+
int __init ima_init_crypto(void)
{
long rc;
@@ -502,7 +506,8 @@ static int ima_calc_field_array_hash_tfm(struct ima_field_data *field_data,
}
if (!rc)
- rc = crypto_shash_final(shash, entry->digest);
+ rc = crypto_shash_final(shash,
+ entry->digests[ima_sha1_idx].digest);
return rc;
}
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 2000e8df0301..2b79581d0e25 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -152,7 +152,7 @@ int ima_measurements_show(struct seq_file *m, void *v)
ima_putc(m, &pcr, sizeof(e->pcr));
/* 2nd: template digest */
- ima_putc(m, e->digest, TPM_DIGEST_SIZE);
+ ima_putc(m, e->digests[ima_sha1_idx].digest, TPM_DIGEST_SIZE);
/* 3rd: template name size */
namelen = !ima_canonical_fmt ? strlen(template_name) :
@@ -235,7 +235,7 @@ static int ima_ascii_measurements_show(struct seq_file *m, void *v)
seq_printf(m, "%2d ", e->pcr);
/* 2nd: SHA1 template hash */
- ima_print_digest(m, e->digest, TPM_DIGEST_SIZE);
+ ima_print_digest(m, e->digests[ima_sha1_idx].digest, TPM_DIGEST_SIZE);
/* 3th: template name */
seq_printf(m, " %s", template_name);
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 1ce8b1701566..bcd99db9722c 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -57,7 +57,8 @@ static struct ima_queue_entry *ima_lookup_digest_entry(u8 *digest_value,
key = ima_hash_key(digest_value);
rcu_read_lock();
hlist_for_each_entry_rcu(qe, &ima_htable.queue[key], hnext) {
- rc = memcmp(qe->entry->digest, digest_value, TPM_DIGEST_SIZE);
+ rc = memcmp(qe->entry->digests[ima_sha1_idx].digest,
+ digest_value, TPM_DIGEST_SIZE);
if ((rc == 0) && (qe->entry->pcr == pcr)) {
ret = qe;
break;
@@ -77,7 +78,7 @@ static int get_binary_runtime_size(struct ima_template_entry *entry)
int size = 0;
size += sizeof(u32); /* pcr */
- size += sizeof(entry->digest);
+ size += TPM_DIGEST_SIZE;
size += sizeof(int); /* template name size field */
size += strlen(entry->template_desc->name);
size += sizeof(entry->template_data_len);
@@ -109,7 +110,7 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
atomic_long_inc(&ima_htable.len);
if (update_htable) {
- key = ima_hash_key(entry->digest);
+ key = ima_hash_key(entry->digests[ima_sha1_idx].digest);
hlist_add_head_rcu(&qe->hnext, &ima_htable.queue[key]);
}
@@ -173,7 +174,8 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
mutex_lock(&ima_extend_list_mutex);
if (!violation) {
- memcpy(digest, entry->digest, sizeof(digest));
+ memcpy(digest, entry->digests[ima_sha1_idx].digest,
+ sizeof(digest));
if (ima_lookup_digest_entry(digest, entry->pcr)) {
audit_cause = "hash_exists";
result = -EEXIST;
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index 6aa6408603e3..586e9fd9fc12 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -311,11 +311,19 @@ static int ima_restore_template_data(struct ima_template_desc *template_desc,
if (!*entry)
return -ENOMEM;
+ (*entry)->digests = kcalloc(NR_BANKS(ima_tpm_chip) + ima_extra_slots,
+ sizeof(*(*entry)->digests), GFP_NOFS);
+ if (!(*entry)->digests) {
+ kfree(*entry);
+ return -ENOMEM;
+ }
+
ret = ima_parse_buf(template_data, template_data + template_data_size,
NULL, template_desc->num_fields,
(*entry)->template_data, NULL, NULL,
ENFORCE_FIELDS | ENFORCE_BUFEND, "template data");
if (ret < 0) {
+ kfree((*entry)->digests);
kfree(*entry);
return ret;
}
@@ -447,8 +455,8 @@ int ima_restore_measurement_list(loff_t size, void *buf)
if (ret < 0)
break;
- memcpy(entry->digest, hdr[HDR_DIGEST].data,
- hdr[HDR_DIGEST].len);
+ memcpy(entry->digests[ima_sha1_idx].digest,
+ hdr[HDR_DIGEST].data, hdr[HDR_DIGEST].len);
entry->pcr = !ima_canonical_fmt ? *(hdr[HDR_PCR].data) :
le32_to_cpu(*(hdr[HDR_PCR].data));
ret = ima_restore_measurement_entry(entry);
--
2.17.1
^ permalink raw reply related
* [PATCH v3 6/8] ima: Allocate and initialize tfm for each PCR bank
From: Roberto Sassu @ 2020-02-10 10:03 UTC (permalink / raw)
To: zohar, James.Bottomley, jarkko.sakkinen
Cc: linux-integrity, linux-security-module, linux-kernel,
silviu.vlasceanu, Roberto Sassu
This patch creates a crypto_shash structure for each allocated PCR bank and
for SHA1 if a bank with that algorithm is not currently allocated.
Changelog
v2:
- declare ima_init_ima_crypto() as static
- replace ima_num_template_digests with
NR_BANKS(ima_tpm_chip) + ima_extra_slots (suggested by Mimi)
- use ima_sha1_idx to access ima_algo_array elements in ima_init_crypto()
v1:
- determine ima_num_template_digests before allocating ima_algo_array
(suggested by Mimi)
- replace kmalloc_array() with kcalloc() in ima_init_crypto() (suggested by
Mimi)
- check tfm first in ima_alloc_tfm()
- check if ima_tpm_chip is NULL
Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
security/integrity/ima/ima_crypto.c | 146 +++++++++++++++++++++++-----
1 file changed, 120 insertions(+), 26 deletions(-)
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index ea24d2f6b513..d57d3c95c4f9 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -59,11 +59,18 @@ MODULE_PARM_DESC(ahash_bufsize, "Maximum ahash buffer size");
static struct crypto_shash *ima_shash_tfm;
static struct crypto_ahash *ima_ahash_tfm;
+struct ima_algo_desc {
+ struct crypto_shash *tfm;
+ enum hash_algo algo;
+};
+
int ima_sha1_idx __ro_after_init;
-/* Additional number of slots to be reserved for SHA1 and IMA default algo */
-int ima_extra_slots __ro_after_init = 1;
+/* Additional number of slots in ima_algo_array for SHA1 and default IMA algo */
+int ima_extra_slots __ro_after_init;
-int __init ima_init_crypto(void)
+static struct ima_algo_desc *ima_algo_array;
+
+static int __init ima_init_ima_crypto(void)
{
long rc;
@@ -82,26 +89,121 @@ int __init ima_init_crypto(void)
static struct crypto_shash *ima_alloc_tfm(enum hash_algo algo)
{
struct crypto_shash *tfm = ima_shash_tfm;
- int rc;
+ int rc, i;
if (algo < 0 || algo >= HASH_ALGO__LAST)
algo = ima_hash_algo;
- if (algo != ima_hash_algo) {
- tfm = crypto_alloc_shash(hash_algo_name[algo], 0, 0);
- if (IS_ERR(tfm)) {
- rc = PTR_ERR(tfm);
- pr_err("Can not allocate %s (reason: %d)\n",
- hash_algo_name[algo], rc);
- }
+ if (algo == ima_hash_algo)
+ return tfm;
+
+ for (i = 0; i < NR_BANKS(ima_tpm_chip) + ima_extra_slots; i++)
+ if (ima_algo_array[i].tfm && ima_algo_array[i].algo == algo)
+ return ima_algo_array[i].tfm;
+
+ tfm = crypto_alloc_shash(hash_algo_name[algo], 0, 0);
+ if (IS_ERR(tfm)) {
+ rc = PTR_ERR(tfm);
+ pr_err("Can not allocate %s (reason: %d)\n",
+ hash_algo_name[algo], rc);
}
return tfm;
}
+int __init ima_init_crypto(void)
+{
+ enum hash_algo algo;
+ long rc;
+ int i;
+
+ rc = ima_init_ima_crypto();
+ if (rc)
+ return rc;
+
+ ima_sha1_idx = -1;
+
+ for (i = 0; i < NR_BANKS(ima_tpm_chip); i++) {
+ algo = ima_tpm_chip->allocated_banks[i].crypto_id;
+ if (algo == HASH_ALGO_SHA1)
+ ima_sha1_idx = i;
+ }
+
+ if (ima_sha1_idx < 0)
+ ima_sha1_idx = NR_BANKS(ima_tpm_chip) + ima_extra_slots++;
+
+ ima_algo_array = kcalloc(NR_BANKS(ima_tpm_chip) + ima_extra_slots,
+ sizeof(*ima_algo_array), GFP_KERNEL);
+ if (!ima_algo_array) {
+ rc = -ENOMEM;
+ goto out;
+ }
+
+ for (i = 0; i < NR_BANKS(ima_tpm_chip); i++) {
+ algo = ima_tpm_chip->allocated_banks[i].crypto_id;
+ ima_algo_array[i].algo = algo;
+
+ /* unknown TPM algorithm */
+ if (algo == HASH_ALGO__LAST)
+ continue;
+
+ if (algo == ima_hash_algo) {
+ ima_algo_array[i].tfm = ima_shash_tfm;
+ continue;
+ }
+
+ ima_algo_array[i].tfm = ima_alloc_tfm(algo);
+ if (IS_ERR(ima_algo_array[i].tfm)) {
+ if (algo == HASH_ALGO_SHA1) {
+ rc = PTR_ERR(ima_algo_array[i].tfm);
+ ima_algo_array[i].tfm = NULL;
+ goto out_array;
+ }
+
+ ima_algo_array[i].tfm = NULL;
+ }
+ }
+
+ if (ima_sha1_idx >= NR_BANKS(ima_tpm_chip)) {
+ if (ima_hash_algo == HASH_ALGO_SHA1) {
+ ima_algo_array[ima_sha1_idx].tfm = ima_shash_tfm;
+ } else {
+ ima_algo_array[ima_sha1_idx].tfm =
+ ima_alloc_tfm(HASH_ALGO_SHA1);
+ if (IS_ERR(ima_algo_array[ima_sha1_idx].tfm)) {
+ rc = PTR_ERR(ima_algo_array[ima_sha1_idx].tfm);
+ goto out_array;
+ }
+ }
+
+ ima_algo_array[ima_sha1_idx].algo = HASH_ALGO_SHA1;
+ }
+
+ return 0;
+out_array:
+ for (i = 0; i < NR_BANKS(ima_tpm_chip) + ima_extra_slots; i++) {
+ if (!ima_algo_array[i].tfm ||
+ ima_algo_array[i].tfm == ima_shash_tfm)
+ continue;
+
+ crypto_free_shash(ima_algo_array[i].tfm);
+ }
+out:
+ crypto_free_shash(ima_shash_tfm);
+ return rc;
+}
+
static void ima_free_tfm(struct crypto_shash *tfm)
{
- if (tfm != ima_shash_tfm)
- crypto_free_shash(tfm);
+ int i;
+
+ if (tfm == ima_shash_tfm)
+ return;
+
+ for (i = 0; i < NR_BANKS(ima_tpm_chip) + ima_extra_slots; i++)
+ if (ima_algo_array[i].tfm == tfm)
+ return;
+
+ crypto_free_shash(tfm);
}
/**
@@ -469,14 +571,14 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
*/
static int ima_calc_field_array_hash_tfm(struct ima_field_data *field_data,
struct ima_template_entry *entry,
- struct crypto_shash *tfm)
+ int tfm_idx)
{
- SHASH_DESC_ON_STACK(shash, tfm);
+ SHASH_DESC_ON_STACK(shash, ima_algo_array[tfm_idx].tfm);
struct ima_template_desc *td = entry->template_desc;
int num_fields = entry->template_desc->num_fields;
int rc, i;
- shash->tfm = tfm;
+ shash->tfm = ima_algo_array[tfm_idx].tfm;
rc = crypto_shash_init(shash);
if (rc != 0)
@@ -507,7 +609,7 @@ static int ima_calc_field_array_hash_tfm(struct ima_field_data *field_data,
if (!rc)
rc = crypto_shash_final(shash,
- entry->digests[ima_sha1_idx].digest);
+ entry->digests[tfm_idx].digest);
return rc;
}
@@ -515,17 +617,9 @@ static int ima_calc_field_array_hash_tfm(struct ima_field_data *field_data,
int ima_calc_field_array_hash(struct ima_field_data *field_data,
struct ima_template_entry *entry)
{
- struct crypto_shash *tfm;
int rc;
- tfm = ima_alloc_tfm(HASH_ALGO_SHA1);
- if (IS_ERR(tfm))
- return PTR_ERR(tfm);
-
- rc = ima_calc_field_array_hash_tfm(field_data, entry, tfm);
-
- ima_free_tfm(tfm);
-
+ rc = ima_calc_field_array_hash_tfm(field_data, entry, ima_sha1_idx);
return rc;
}
--
2.17.1
^ permalink raw reply related
* [PATCH v3 7/8] ima: Calculate and extend PCR with digests in ima_template_entry
From: Roberto Sassu @ 2020-02-10 10:04 UTC (permalink / raw)
To: zohar, James.Bottomley, jarkko.sakkinen
Cc: linux-integrity, linux-security-module, linux-kernel,
silviu.vlasceanu, Roberto Sassu
This patch modifies ima_calc_field_array_hash() to calculate a template
digest for each allocated PCR bank and SHA1. It also passes the tpm_digest
array of the template entry to ima_pcr_extend() or in case of a violation,
the pre-initialized digests array filled with 0xff.
Padding with zeros is still done if the mapping between TPM algorithm ID
and crypto ID is unknown.
This patch calculates again the template digest when a measurement list is
restored. Copying only the SHA1 digest (due to the limitation of the
current measurement list format) is not sufficient, as hash collision
detection will be done on the digest calculated with the default IMA hash
algorithm.
Changelog
v2:
- replace ima_num_template_digests with
NR_BANKS(ima_tpm_chip) + ima_extra_slots (suggested by Mimi)
v1:
- replace ima_tpm_chip->nr_allocated_banks with ima_num_template_digests
(suggested by Mimi)
- retrieve alg_id only if i < ima_tpm_chip->nr_allocated_banks
- check if ima_tpm_chip is NULL
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
security/integrity/ima/ima_crypto.c | 29 +++++++++++++++++++++++++-
security/integrity/ima/ima_queue.c | 30 ++++++++++++++++-----------
security/integrity/ima/ima_template.c | 14 +++++++++++--
3 files changed, 58 insertions(+), 15 deletions(-)
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index d57d3c95c4f9..5ebe31576f7b 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -617,9 +617,36 @@ static int ima_calc_field_array_hash_tfm(struct ima_field_data *field_data,
int ima_calc_field_array_hash(struct ima_field_data *field_data,
struct ima_template_entry *entry)
{
- int rc;
+ u16 alg_id;
+ int rc, i;
rc = ima_calc_field_array_hash_tfm(field_data, entry, ima_sha1_idx);
+ if (rc)
+ return rc;
+
+ entry->digests[ima_sha1_idx].alg_id = TPM_ALG_SHA1;
+
+ for (i = 0; i < NR_BANKS(ima_tpm_chip) + ima_extra_slots; i++) {
+ if (i == ima_sha1_idx)
+ continue;
+
+ if (i < NR_BANKS(ima_tpm_chip)) {
+ alg_id = ima_tpm_chip->allocated_banks[i].alg_id;
+ entry->digests[i].alg_id = alg_id;
+ }
+
+ /* for unmapped TPM algorithms digest is still a padded SHA1 */
+ if (!ima_algo_array[i].tfm) {
+ memcpy(entry->digests[i].digest,
+ entry->digests[ima_sha1_idx].digest,
+ TPM_DIGEST_SIZE);
+ continue;
+ }
+
+ rc = ima_calc_field_array_hash_tfm(field_data, entry, i);
+ if (rc)
+ return rc;
+ }
return rc;
}
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index bcd99db9722c..7f7509774b85 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -137,18 +137,14 @@ unsigned long ima_get_binary_runtime_size(void)
return binary_runtime_size + sizeof(struct ima_kexec_hdr);
};
-static int ima_pcr_extend(const u8 *hash, int pcr)
+static int ima_pcr_extend(struct tpm_digest *digests_arg, int pcr)
{
int result = 0;
- int i;
if (!ima_tpm_chip)
return result;
- for (i = 0; i < ima_tpm_chip->nr_allocated_banks; i++)
- memcpy(digests[i].digest, hash, TPM_DIGEST_SIZE);
-
- result = tpm_pcr_extend(ima_tpm_chip, pcr, digests);
+ result = tpm_pcr_extend(ima_tpm_chip, pcr, digests_arg);
if (result != 0)
pr_err("Error Communicating to TPM chip, result: %d\n", result);
return result;
@@ -166,7 +162,8 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
const char *op, struct inode *inode,
const unsigned char *filename)
{
- u8 digest[TPM_DIGEST_SIZE];
+ u8 *digest = entry->digests[ima_sha1_idx].digest;
+ struct tpm_digest *digests_arg = entry->digests;
const char *audit_cause = "hash_added";
char tpm_audit_cause[AUDIT_CAUSE_LEN_MAX];
int audit_info = 1;
@@ -174,8 +171,6 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
mutex_lock(&ima_extend_list_mutex);
if (!violation) {
- memcpy(digest, entry->digests[ima_sha1_idx].digest,
- sizeof(digest));
if (ima_lookup_digest_entry(digest, entry->pcr)) {
audit_cause = "hash_exists";
result = -EEXIST;
@@ -191,9 +186,9 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
}
if (violation) /* invalidate pcr */
- memset(digest, 0xff, sizeof(digest));
+ digests_arg = digests;
- tpmresult = ima_pcr_extend(digest, entry->pcr);
+ tpmresult = ima_pcr_extend(digests_arg, entry->pcr);
if (tpmresult != 0) {
snprintf(tpm_audit_cause, AUDIT_CAUSE_LEN_MAX, "TPM_error(%d)",
tpmresult);
@@ -219,6 +214,8 @@ int ima_restore_measurement_entry(struct ima_template_entry *entry)
int __init ima_init_digests(void)
{
+ u16 digest_size;
+ u16 crypto_id;
int i;
if (!ima_tpm_chip)
@@ -229,8 +226,17 @@ int __init ima_init_digests(void)
if (!digests)
return -ENOMEM;
- for (i = 0; i < ima_tpm_chip->nr_allocated_banks; i++)
+ for (i = 0; i < ima_tpm_chip->nr_allocated_banks; i++) {
digests[i].alg_id = ima_tpm_chip->allocated_banks[i].alg_id;
+ digest_size = ima_tpm_chip->allocated_banks[i].digest_size;
+ crypto_id = ima_tpm_chip->allocated_banks[i].crypto_id;
+
+ /* for unmapped TPM algorithms digest is still a padded SHA1 */
+ if (crypto_id == HASH_ALGO__LAST)
+ digest_size = SHA1_DIGEST_SIZE;
+
+ memset(digests[i].digest, 0xff, digest_size);
+ }
return 0;
}
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index 586e9fd9fc12..db984313ba1e 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -356,6 +356,7 @@ static int ima_restore_template_data(struct ima_template_desc *template_desc,
int ima_restore_measurement_list(loff_t size, void *buf)
{
char template_name[MAX_TEMPLATE_NAME_LEN];
+ unsigned char zero[TPM_DIGEST_SIZE] = { 0 };
struct ima_kexec_hdr *khdr = buf;
struct ima_field_data hdr[HDR__LAST] = {
@@ -455,8 +456,17 @@ int ima_restore_measurement_list(loff_t size, void *buf)
if (ret < 0)
break;
- memcpy(entry->digests[ima_sha1_idx].digest,
- hdr[HDR_DIGEST].data, hdr[HDR_DIGEST].len);
+ if (memcmp(hdr[HDR_DIGEST].data, zero, sizeof(zero))) {
+ ret = ima_calc_field_array_hash(
+ &entry->template_data[0],
+ entry);
+ if (ret < 0) {
+ pr_err("cannot calculate template digest\n");
+ ret = -EINVAL;
+ break;
+ }
+ }
+
entry->pcr = !ima_canonical_fmt ? *(hdr[HDR_PCR].data) :
le32_to_cpu(*(hdr[HDR_PCR].data));
ret = ima_restore_measurement_entry(entry);
--
2.17.1
^ permalink raw reply related
* [PATCH v3 8/8] ima: Use ima_hash_algo for collision detection in the measurement list
From: Roberto Sassu @ 2020-02-10 10:04 UTC (permalink / raw)
To: zohar, James.Bottomley, jarkko.sakkinen
Cc: linux-integrity, linux-security-module, linux-kernel,
silviu.vlasceanu, Roberto Sassu
Before calculating a digest for each PCR bank, collisions were detected
with a SHA1 digest. This patch includes ima_hash_algo among the algorithms
used to calculate the template digest and checks collisions on that digest.
The position in the measurement entry array of the template digest
calculated with the default IMA algorithm is stored in the
ima_hash_algo_idx global variable and is determined at IMA initialization
time.
Changelog
v2:
- add __ro_after_init to declaration of ima_hash_algo_idx (suggested by
Mimi)
- replace ima_num_template_digests with
NR_BANKS(ima_tpm_chip) + ima_extra_slots(suggested by Mimi)
- use ima_hash_algo_idx to access ima_algo_array elements in
ima_init_crypto()
v1:
- increment ima_num_template_digests before kcalloc() (suggested by Mimi)
- check if ima_tpm_chip is NULL
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
security/integrity/ima/ima.h | 1 +
security/integrity/ima/ima_crypto.c | 19 ++++++++++++++++++-
security/integrity/ima/ima_queue.c | 8 ++++----
3 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index f04bec2ab83f..15663711d0ec 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -53,6 +53,7 @@ extern int ima_policy_flag;
/* set during initialization */
extern int ima_hash_algo;
extern int ima_sha1_idx __ro_after_init;
+extern int ima_hash_algo_idx __ro_after_init;
extern int ima_extra_slots __ro_after_init;
extern int ima_appraise;
extern struct tpm_chip *ima_tpm_chip;
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 5ebe31576f7b..84e2b239d31c 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -65,6 +65,7 @@ struct ima_algo_desc {
};
int ima_sha1_idx __ro_after_init;
+int ima_hash_algo_idx __ro_after_init;
/* Additional number of slots in ima_algo_array for SHA1 and default IMA algo */
int ima_extra_slots __ro_after_init;
@@ -121,15 +122,25 @@ int __init ima_init_crypto(void)
return rc;
ima_sha1_idx = -1;
+ ima_hash_algo_idx = -1;
for (i = 0; i < NR_BANKS(ima_tpm_chip); i++) {
algo = ima_tpm_chip->allocated_banks[i].crypto_id;
if (algo == HASH_ALGO_SHA1)
ima_sha1_idx = i;
+
+ if (algo == ima_hash_algo)
+ ima_hash_algo_idx = i;
}
- if (ima_sha1_idx < 0)
+ if (ima_sha1_idx < 0) {
ima_sha1_idx = NR_BANKS(ima_tpm_chip) + ima_extra_slots++;
+ if (ima_hash_algo == HASH_ALGO_SHA1)
+ ima_hash_algo_idx = ima_sha1_idx;
+ }
+
+ if (ima_hash_algo_idx < 0)
+ ima_hash_algo_idx = NR_BANKS(ima_tpm_chip) + ima_extra_slots++;
ima_algo_array = kcalloc(NR_BANKS(ima_tpm_chip) + ima_extra_slots,
sizeof(*ima_algo_array), GFP_KERNEL);
@@ -178,6 +189,12 @@ int __init ima_init_crypto(void)
ima_algo_array[ima_sha1_idx].algo = HASH_ALGO_SHA1;
}
+ if (ima_hash_algo_idx >= NR_BANKS(ima_tpm_chip) &&
+ ima_hash_algo_idx != ima_sha1_idx) {
+ ima_algo_array[ima_hash_algo_idx].tfm = ima_shash_tfm;
+ ima_algo_array[ima_hash_algo_idx].algo = ima_hash_algo;
+ }
+
return 0;
out_array:
for (i = 0; i < NR_BANKS(ima_tpm_chip) + ima_extra_slots; i++) {
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 7f7509774b85..58983d0f0214 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -57,8 +57,8 @@ static struct ima_queue_entry *ima_lookup_digest_entry(u8 *digest_value,
key = ima_hash_key(digest_value);
rcu_read_lock();
hlist_for_each_entry_rcu(qe, &ima_htable.queue[key], hnext) {
- rc = memcmp(qe->entry->digests[ima_sha1_idx].digest,
- digest_value, TPM_DIGEST_SIZE);
+ rc = memcmp(qe->entry->digests[ima_hash_algo_idx].digest,
+ digest_value, hash_digest_size[ima_hash_algo]);
if ((rc == 0) && (qe->entry->pcr == pcr)) {
ret = qe;
break;
@@ -110,7 +110,7 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
atomic_long_inc(&ima_htable.len);
if (update_htable) {
- key = ima_hash_key(entry->digests[ima_sha1_idx].digest);
+ key = ima_hash_key(entry->digests[ima_hash_algo_idx].digest);
hlist_add_head_rcu(&qe->hnext, &ima_htable.queue[key]);
}
@@ -162,7 +162,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
const char *op, struct inode *inode,
const unsigned char *filename)
{
- u8 *digest = entry->digests[ima_sha1_idx].digest;
+ u8 *digest = entry->digests[ima_hash_algo_idx].digest;
struct tpm_digest *digests_arg = entry->digests;
const char *audit_cause = "hash_added";
char tpm_audit_cause[AUDIT_CAUSE_LEN_MAX];
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v14 22/23] LSM: Add /proc attr entry for full LSM context
From: Simon McVittie @ 2020-02-10 11:56 UTC (permalink / raw)
To: Stephen Smalley
Cc: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
selinux, keescook, john.johansen, penguin-kernel, paul
In-Reply-To: <09d96236-715a-344a-38bc-c05208698125@tycho.nsa.gov>
On Mon, 03 Feb 2020 at 13:54:45 -0500, Stephen Smalley wrote:
> The printable ASCII bit is based on what the dbus maintainer requested in
> previous discussions.
I thought in previous discussions, we had come to the conclusion that
I can't assume it's 7-bit ASCII. (If I *can* assume that for this new
API, that's even better.)
To be clear, when I say ASCII I mean a sequence of bytes != '\0' with
their high bit unset (x & 0x7f == x) and the obvious mapping to/from
Unicode (bytes '\1' to '\x7f' represent codepoints U+0001 to U+007F). Is
that the same thing you mean?
I thought the conclusion we had come to in previous conversations was
that the LSM context is what GLib calls a "bytestring", the same as
filenames and environment variables - an opaque sequence of bytes != '\0',
with no further guarantees, and no specified encoding or mapping to/from
Unicode (most likely some superset of ASCII like UTF-8 or Latin-1,
but nobody knows which one, and they could equally well be some binary
encoding with no Unicode meaning, as long as it avoids '\0').
If I can safely assume that a new kernel <-> user-space API is constrained
to UTF-8 or a UTF-8 subset like ASCII, then I can provide more friendly
APIs for user-space features built over it. If that isn't possible, the
next best thing is a "bytestring" like filenames, environment variables,
and most kernel <-> user-space strings in general.
smcv
^ permalink raw reply
* 回复:[PATCH 1/2] crypto: sm3 - add a new alias name sm3-256
From: Tianjia Zhang @ 2020-02-10 12:06 UTC (permalink / raw)
To: linux-crypto-owner, ebiggers
Cc: herbert, davem, zohar, dmitry.kasatkin, jmorris, serge,
linux-crypto, linux-integrity, linux-security-module,
linux-kernel
In-Reply-To: <b7ce247b-ede1-4b01-bb11-894c042679e1.tianjia.zhang@linux.alibaba.com>
It is also a good idea to change "sm3-256" to "sm3" in hash_info, I will submit a new patch to fix this problem.
^ permalink raw reply
* [PATCH 2/2] ima: add sm3 algorithm to hash algorithm configuration list
From: Tianjia Zhang @ 2020-02-10 12:44 UTC (permalink / raw)
To: herbert, davem, zohar, dmitry.kasatkin, jmorris, serge, ebiggers
Cc: linux-crypto, linux-integrity, linux-security-module,
linux-kernel
In-Reply-To: <20200210124440.23929-1-tianjia.zhang@linux.alibaba.com>
sm3 has been supported by the ima hash algorithm, but it is not
yet in the Kconfig configuration list. After adding, both ima and tpm2
can support sm3 well.
Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
---
security/integrity/ima/Kconfig | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 838476d780e5..ace6762c1cf1 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -112,6 +112,10 @@ choice
config IMA_DEFAULT_HASH_WP512
bool "WP512"
depends on CRYPTO_WP512=y && !IMA_TEMPLATE
+
+ config IMA_DEFAULT_HASH_SM3
+ bool "SM3"
+ depends on CRYPTO_SM3=y && !IMA_TEMPLATE
endchoice
config IMA_DEFAULT_HASH
@@ -121,6 +125,7 @@ config IMA_DEFAULT_HASH
default "sha256" if IMA_DEFAULT_HASH_SHA256
default "sha512" if IMA_DEFAULT_HASH_SHA512
default "wp512" if IMA_DEFAULT_HASH_WP512
+ default "sm3" if IMA_DEFAULT_HASH_SM3
config IMA_WRITE_POLICY
bool "Enable multiple writes to the IMA policy"
--
2.17.1
^ permalink raw reply related
* [PATCH 1/2] crypto: rename sm3-256 to sm3 in hash_algo_name
From: Tianjia Zhang @ 2020-02-10 12:44 UTC (permalink / raw)
To: herbert, davem, zohar, dmitry.kasatkin, jmorris, serge, ebiggers
Cc: linux-crypto, linux-integrity, linux-security-module,
linux-kernel
In-Reply-To: <20200210124440.23929-1-tianjia.zhang@linux.alibaba.com>
The name sm3-256 is defined in hash_algo_name in hash_info, but the
algorithm name implemented in sm3_generic.c is sm3, which will cause
the sm3-256 algorithm to be not found in some application scenarios of
the hash algorithm, and an ENOENT error will occur. For example,
IMA, keys, and other subsystems that reference hash_algo_name all use
the hash algorithm of sm3.
Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
---
crypto/hash_info.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/crypto/hash_info.c b/crypto/hash_info.c
index c754cb75dd1a..a49ff96bde77 100644
--- a/crypto/hash_info.c
+++ b/crypto/hash_info.c
@@ -26,7 +26,7 @@ const char *const hash_algo_name[HASH_ALGO__LAST] = {
[HASH_ALGO_TGR_128] = "tgr128",
[HASH_ALGO_TGR_160] = "tgr160",
[HASH_ALGO_TGR_192] = "tgr192",
- [HASH_ALGO_SM3_256] = "sm3-256",
+ [HASH_ALGO_SM3_256] = "sm3",
[HASH_ALGO_STREEBOG_256] = "streebog256",
[HASH_ALGO_STREEBOG_512] = "streebog512",
};
--
2.17.1
^ permalink raw reply related
* [PATCH] IMA hash algorithm supports sm3
From: Tianjia Zhang @ 2020-02-10 12:44 UTC (permalink / raw)
To: herbert, davem, zohar, dmitry.kasatkin, jmorris, serge, ebiggers
Cc: linux-crypto, linux-integrity, linux-security-module,
linux-kernel
Fixed an issue where the sm3 algorithm name mismatch in the crypto subsystem hash_algo_name.
Make IMA support sm3 hash algorithm, added support for sm3 in IMA algorithm Kconfig configuration.
^ permalink raw reply
* Re: [PATCH v14 22/23] LSM: Add /proc attr entry for full LSM context
From: Stephen Smalley @ 2020-02-10 13:25 UTC (permalink / raw)
To: Simon McVittie
Cc: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
selinux, keescook, john.johansen, penguin-kernel, paul
In-Reply-To: <20200210115611.GA13930@horizon>
On 2/10/20 6:56 AM, Simon McVittie wrote:
> On Mon, 03 Feb 2020 at 13:54:45 -0500, Stephen Smalley wrote:
>> The printable ASCII bit is based on what the dbus maintainer requested in
>> previous discussions.
>
> I thought in previous discussions, we had come to the conclusion that
> I can't assume it's 7-bit ASCII. (If I *can* assume that for this new
> API, that's even better.)
>
> To be clear, when I say ASCII I mean a sequence of bytes != '\0' with
> their high bit unset (x & 0x7f == x) and the obvious mapping to/from
> Unicode (bytes '\1' to '\x7f' represent codepoints U+0001 to U+007F). Is
> that the same thing you mean?
I mean the subset of 7-bit ASCII that satisfies isprint() using the "C"
locale. That is already true for SELinux with the existing interfaces.
I can't necessarily speak for the others.
> I thought the conclusion we had come to in previous conversations was
> that the LSM context is what GLib calls a "bytestring", the same as
> filenames and environment variables - an opaque sequence of bytes != '\0',
> with no further guarantees, and no specified encoding or mapping to/from
> Unicode (most likely some superset of ASCII like UTF-8 or Latin-1,
> but nobody knows which one, and they could equally well be some binary
> encoding with no Unicode meaning, as long as it avoids '\0').
>
> If I can safely assume that a new kernel <-> user-space API is constrained
> to UTF-8 or a UTF-8 subset like ASCII, then I can provide more friendly
> APIs for user-space features built over it. If that isn't possible, the
> next best thing is a "bytestring" like filenames, environment variables,
> and most kernel <-> user-space strings in general.
>
> smcv
>
^ permalink raw reply
* Re: [PATCH v14 22/23] LSM: Add /proc attr entry for full LSM context
From: Stephen Smalley @ 2020-02-10 14:55 UTC (permalink / raw)
To: Simon McVittie
Cc: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
selinux, keescook, john.johansen, penguin-kernel, paul
In-Reply-To: <94aaf6c8-cc69-5804-2d45-3b8c96689331@tycho.nsa.gov>
On 2/10/20 8:25 AM, Stephen Smalley wrote:
> On 2/10/20 6:56 AM, Simon McVittie wrote:
>> On Mon, 03 Feb 2020 at 13:54:45 -0500, Stephen Smalley wrote:
>>> The printable ASCII bit is based on what the dbus maintainer
>>> requested in
>>> previous discussions.
>>
>> I thought in previous discussions, we had come to the conclusion that
>> I can't assume it's 7-bit ASCII. (If I *can* assume that for this new
>> API, that's even better.)
>>
>> To be clear, when I say ASCII I mean a sequence of bytes != '\0' with
>> their high bit unset (x & 0x7f == x) and the obvious mapping to/from
>> Unicode (bytes '\1' to '\x7f' represent codepoints U+0001 to U+007F). Is
>> that the same thing you mean?
>
> I mean the subset of 7-bit ASCII that satisfies isprint() using the "C"
> locale. That is already true for SELinux with the existing interfaces.
> I can't necessarily speak for the others.
Looks like Smack labels are similarly restricted, per
Documentation/admin-guide/LSM/Smack.rst. So I guess the only one that
is perhaps unclear is AppArmor, since its labels are typically derived
from pathnames? Can an AppArmor label returned via its getprocattr()
hook be any legal pathname?
>> I thought the conclusion we had come to in previous conversations was
>> that the LSM context is what GLib calls a "bytestring", the same as
>> filenames and environment variables - an opaque sequence of bytes !=
>> '\0',
>> with no further guarantees, and no specified encoding or mapping to/from
>> Unicode (most likely some superset of ASCII like UTF-8 or Latin-1,
>> but nobody knows which one, and they could equally well be some binary
>> encoding with no Unicode meaning, as long as it avoids '\0').
>>
>> If I can safely assume that a new kernel <-> user-space API is
>> constrained
>> to UTF-8 or a UTF-8 subset like ASCII, then I can provide more friendly
>> APIs for user-space features built over it. If that isn't possible, the
>> next best thing is a "bytestring" like filenames, environment variables,
>> and most kernel <-> user-space strings in general.
>>
>> smcv
>>
>
^ permalink raw reply
* [PATCH v8 00/11] proc: modernize proc to support multiple private instances
From: Alexey Gladkov @ 2020-02-10 15:05 UTC (permalink / raw)
To: LKML, Kernel Hardening, Linux API, Linux FS Devel,
Linux Security Module
Cc: Akinobu Mita, Alexander Viro, Alexey Dobriyan, Alexey Gladkov,
Andrew Morton, Andy Lutomirski, Daniel Micay, Djalal Harouni,
Dmitry V . Levin, Eric W . Biederman, Greg Kroah-Hartman,
Ingo Molnar, J . Bruce Fields, Jeff Layton, Jonathan Corbet,
Kees Cook, Linus Torvalds, Oleg Nesterov, Solar Designer
Greetings!
Preface:
--------
This is patchset v8 to modernize procfs and make it able to support multiple
private instances per the same pid namespace.
This patchset can be applied on top of v5.4-rc7-49-g0e3f1ad80fc8
Procfs modernization:
---------------------
Historically procfs was always tied to pid namespaces, during pid
namespace creation we internally create a procfs mount for it. However,
this has the effect that all new procfs mounts are just a mirror of the
internal one, any change, any mount option update, any new future
introduction will propagate to all other procfs mounts that are in the
same pid namespace.
This may have solved several use cases in that time. However today we
face new requirements, and making procfs able to support new private
instances inside same pid namespace seems a major point. If we want to
to introduce new features and security mechanisms we have to make sure
first that we do not break existing usecases. Supporting private procfs
instances will allow to support new features and behaviour without
propagating it to all other procfs mounts.
Today procfs is more of a burden especially to some Embedded, IoT,
sandbox, container use cases. In user space we are over-mounting null
or inaccessible files on top to hide files and information. If we want
to hide pids we have to create PID namespaces otherwise mount options
propagate to all other proc mounts, changing a mount option value in one
mount will propagate to all other proc mounts. If we want to introduce
new features, then they will propagate to all other mounts too, resulting
either maybe new useful functionality or maybe breaking stuff. We have
also to note that userspace should not workaround procfs, the kernel
should just provide a sane simple interface.
In this regard several developers and maintainers pointed out that
there are problems with procfs and it has to be modernized:
"Here's another one: split up and modernize /proc." by Andy Lutomirski [1]
Discussion about kernel pointer leaks:
"And yes, as Kees and Daniel mentioned, it's definitely not just dmesg.
In fact, the primary things tend to be /proc and /sys, not dmesg
itself." By Linus Torvalds [2]
Lot of other areas in the kernel and filesystems have been updated to be
able to support private instances, devpts is one major example [3].
Which will be used for:
1) Embedded systems and IoT: usually we have one supervisor for
apps, we have some lightweight sandbox support, however if we create
pid namespaces we have to manage all the processes inside too,
where our goal is to be able to run a bunch of apps each one inside
its own mount namespace, maybe use network namespaces for vlans
setups, but right now we only want mount namespaces, without all the
other complexity. We want procfs to behave more like a real file system,
and block access to inodes that belong to other users. The 'hidepid=' will
not work since it is a shared mount option.
2) Containers, sandboxes and Private instances of file systems - devpts case
Historically, lot of file systems inside Linux kernel view when instantiated
were just a mirror of an already created and mounted filesystem. This was the
case of devpts filesystem, it seems at that time the requirements were to
optimize things and reuse the same memory, etc. This design used to work but not
anymore with today's containers, IoT, hostile environments and all the privacy
challenges that Linux faces.
In that regards, devpts was updated so that each new mounts is a total
independent file system by the following patches:
"devpts: Make each mount of devpts an independent filesystem" by
Eric W. Biederman [3] [4]
3) Linux Security Modules have multiple ptrace paths inside some
subsystems, however inside procfs, the implementation does not guarantee
that the ptrace() check which triggers the security_ptrace_check() hook
will always run. We have the 'hidepid' mount option that can be used to
force the ptrace_may_access() check inside has_pid_permissions() to run.
The problem is that 'hidepid' is per pid namespace and not attached to
the mount point, any remount or modification of 'hidepid' will propagate
to all other procfs mounts.
This also does not allow to support Yama LSM easily in desktop and user
sessions. Yama ptrace scope which restricts ptrace and some other
syscalls to be allowed only on inferiors, can be updated to have a
per-task context, where the context will be inherited during fork(),
clone() and preserved across execve(). If we support multiple private
procfs instances, then we may force the ptrace_may_access() on
/proc/<pids>/ to always run inside that new procfs instances. This will
allow to specifiy on user sessions if we should populate procfs with
pids that the user can ptrace or not.
By using Yama ptrace scope, some restricted users will only be able to see
inferiors inside /proc, they won't even be able to see their other
processes. Some software like Chromium, Firefox's crash handler, Wine
and others are already using Yama to restrict which processes can be
ptracable. With this change this will give the possibility to restrict
/proc/<pids>/ but more importantly this will give desktop users a
generic and usuable way to specifiy which users should see all processes
and which user can not.
Side notes:
* This covers the lack of seccomp where it is not able to parse
arguments, it is easy to install a seccomp filter on direct syscalls
that operate on pids, however /proc/<pid>/ is a Linux ABI using
filesystem syscalls. With this change all LSMs should be able to analyze
open/read/write/close... on /proc/<pid>/
4) This will allow to implement new features either in kernel or
userspace without having to worry about procfs.
In containers, sandboxes, etc we have workarounds to hide some /proc
inodes, this should be supported natively without doing extra complex
work, the kernel should be able to support sane options that work with
today and future Linux use cases.
5) Creation of new superblock with all procfs options for each procfs
mount will fix the ignoring of mount options. The problem is that the
second mount of procfs in the same pid namespace ignores the mount
options. The mount options are ignored without error until procfs is
remounted.
Before:
# grep ^proc /proc/mounts
proc /proc proc rw,relatime,hidepid=2 0 0
# strace -e mount mount -o hidepid=1 -t proc proc /tmp/proc
mount("proc", "/tmp/proc", "proc", 0, "hidepid=1") = 0
+++ exited with 0 +++
# grep ^proc /proc/mounts
proc /proc proc rw,relatime,hidepid=2 0 0
proc /tmp/proc proc rw,relatime,hidepid=2 0 0
# mount -o remount,hidepid=1 -t proc proc /tmp/proc
# grep ^proc /proc/mounts
proc /proc proc rw,relatime,hidepid=1 0 0
proc /tmp/proc proc rw,relatime,hidepid=1 0 0
After:
# grep ^proc /proc/mounts
proc /proc proc rw,relatime,hidepid=2 0 0
# mount -o hidepid=1 -t proc proc /tmp/proc
# grep ^proc /proc/mounts
proc /proc proc rw,relatime,hidepid=2 0 0
proc /tmp/proc proc rw,relatime,hidepid=1 0 0
Introduced changes:
-------------------
Each mount of procfs creates a separate procfs instance with its own
mount options.
This series adds few new mount options:
* New 'hidepid=4' mount option to show only ptraceable processes in the procfs.
This allows to support lightweight sandboxes in Embedded Linux, also
solves the case for LSM where now with this mount option, we make sure
that they have a ptrace path in procfs.
* 'subset=pidfs' that allows to hide non-pid inodes from procfs. It can be used
in containers and sandboxes, as these are already trying to hide and block
access to procfs inodes anyway.
ChangeLog:
----------
# v8:
* Started using RCU lock to clean dcache entries as Linus Torvalds suggested.
# v7:
* 'pidonly=1' renamed to 'subset=pidfs' as Alexey Dobriyan suggested.
* HIDEPID_* moved to uapi/ as they are user interface to mount().
Suggested-by Alexey Dobriyan <adobriyan@gmail.com>
# v6:
* 'hidepid=' and 'gid=' mount options are moved from pid namespace to superblock.
* 'newinstance' mount option removed as Eric W. Biederman suggested.
Mount of procfs always creates a new instance.
* 'limit_pids' renamed to 'hidepid=3'.
* I took into account the comment of Linus Torvalds [7].
* Documentation added.
# v5:
* Fixed a bug that caused a problem with the Fedora boot.
* The 'pidonly' option is visible among the mount options.
# v2:
* Renamed mount options to 'newinstance' and 'pids='
Suggested-by: Andy Lutomirski <luto@kernel.org>
* Fixed order of commit, Suggested-by: Andy Lutomirski <luto@kernel.org>
* Many bug fixes.
# v1:
* Removed 'unshared' mount option and replaced it with 'limit_pids'
which is attached to the current procfs mount.
Suggested-by Andy Lutomirski <luto@kernel.org>
* Do not fill dcache with pid entries that we can not ptrace.
* Many bug fixes.
References:
-----------
[1] https://lists.linuxfoundation.org/pipermail/ksummit-discuss/2017-January/004215.html
[2] http://www.openwall.com/lists/kernel-hardening/2017/10/05/5
[3] https://lwn.net/Articles/689539/
[4] http://lxr.free-electrons.com/source/Documentation/filesystems/devpts.txt?v=3.14
[5] https://lkml.org/lkml/2017/5/2/407
[6] https://lkml.org/lkml/2017/5/3/357
[7] https://lkml.org/lkml/2018/5/11/505
Alexey Gladkov (11):
proc: Rename struct proc_fs_info to proc_fs_opts
proc: add proc_fs_info struct to store proc information
proc: move /proc/{self|thread-self} dentries to proc_fs_info
proc: move hide_pid, pid_gid from pid_namespace to proc_fs_info
proc: add helpers to set and get proc hidepid and gid mount options
proc: support mounting procfs instances inside same pid namespace
proc: flush task dcache entries from all procfs instances
proc: instantiate only pids that we can ptrace on 'hidepid=4' mount
option
proc: add option to mount only a pids subset
docs: proc: add documentation for "hidepid=4" and "subset=pidfs"
options and new mount behavior
proc: Move hidepid values to uapi as they are user interface to mount
Documentation/filesystems/proc.txt | 53 +++++++++++
fs/locks.c | 6 +-
fs/proc/base.c | 66 ++++++++++----
fs/proc/generic.c | 9 ++
fs/proc/inode.c | 21 +++--
fs/proc/internal.h | 30 ++++++
fs/proc/root.c | 141 +++++++++++++++++++++++------
fs/proc/self.c | 4 +-
fs/proc/thread_self.c | 6 +-
fs/proc_namespace.c | 14 +--
include/linux/pid_namespace.h | 14 +--
include/linux/proc_fs.h | 25 ++++-
include/uapi/linux/proc_fs.h | 13 +++
13 files changed, 324 insertions(+), 78 deletions(-)
create mode 100644 include/uapi/linux/proc_fs.h
--
2.24.1
^ permalink raw reply
* [PATCH v8 01/11] proc: Rename struct proc_fs_info to proc_fs_opts
From: Alexey Gladkov @ 2020-02-10 15:05 UTC (permalink / raw)
To: LKML, Kernel Hardening, Linux API, Linux FS Devel,
Linux Security Module
Cc: Akinobu Mita, Alexander Viro, Alexey Dobriyan, Alexey Gladkov,
Andrew Morton, Andy Lutomirski, Daniel Micay, Djalal Harouni,
Dmitry V . Levin, Eric W . Biederman, Greg Kroah-Hartman,
Ingo Molnar, J . Bruce Fields, Jeff Layton, Jonathan Corbet,
Kees Cook, Linus Torvalds, Oleg Nesterov, Solar Designer
In-Reply-To: <20200210150519.538333-1-gladkov.alexey@gmail.com>
Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
---
fs/proc_namespace.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/fs/proc_namespace.c b/fs/proc_namespace.c
index 273ee82d8aa9..9a8b624bc3db 100644
--- a/fs/proc_namespace.c
+++ b/fs/proc_namespace.c
@@ -37,23 +37,23 @@ static __poll_t mounts_poll(struct file *file, poll_table *wait)
return res;
}
-struct proc_fs_info {
+struct proc_fs_opts {
int flag;
const char *str;
};
static int show_sb_opts(struct seq_file *m, struct super_block *sb)
{
- static const struct proc_fs_info fs_info[] = {
+ static const struct proc_fs_opts fs_opts[] = {
{ SB_SYNCHRONOUS, ",sync" },
{ SB_DIRSYNC, ",dirsync" },
{ SB_MANDLOCK, ",mand" },
{ SB_LAZYTIME, ",lazytime" },
{ 0, NULL }
};
- const struct proc_fs_info *fs_infop;
+ const struct proc_fs_opts *fs_infop;
- for (fs_infop = fs_info; fs_infop->flag; fs_infop++) {
+ for (fs_infop = fs_opts; fs_infop->flag; fs_infop++) {
if (sb->s_flags & fs_infop->flag)
seq_puts(m, fs_infop->str);
}
@@ -63,7 +63,7 @@ static int show_sb_opts(struct seq_file *m, struct super_block *sb)
static void show_mnt_opts(struct seq_file *m, struct vfsmount *mnt)
{
- static const struct proc_fs_info mnt_info[] = {
+ static const struct proc_fs_opts mnt_opts[] = {
{ MNT_NOSUID, ",nosuid" },
{ MNT_NODEV, ",nodev" },
{ MNT_NOEXEC, ",noexec" },
@@ -72,9 +72,9 @@ static void show_mnt_opts(struct seq_file *m, struct vfsmount *mnt)
{ MNT_RELATIME, ",relatime" },
{ 0, NULL }
};
- const struct proc_fs_info *fs_infop;
+ const struct proc_fs_opts *fs_infop;
- for (fs_infop = mnt_info; fs_infop->flag; fs_infop++) {
+ for (fs_infop = mnt_opts; fs_infop->flag; fs_infop++) {
if (mnt->mnt_flags & fs_infop->flag)
seq_puts(m, fs_infop->str);
}
--
2.24.1
^ 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