| Message ID | 20181221231154.6120-20-jarkko.sakkinen@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On 2018-12-22 04:41, Jarkko Sakkinen wrote: > In order to provide a mechanism for devilering provisoning rights: > > 1. Add a new file to the securityfs file called sgx/provision that works > as a token for allowing an enclave to have the provisioning privileges. > 2. Add a new ioctl called SGX_IOC_ENCLAVE_SET_ATTRIBUTE that accepts the > following data structure: > > struct sgx_enclave_set_attribute { > __u64 addr; > __u64 token_fd; > }; > > A daemon could sit on top of 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 more or less straight-forward. Lets assume that > dev_fd is a handle to /dev/sgx and prov_fd is a handle to 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); I think the attribute handling in this and the previous patch doesn't work. How am I supposed to call this ioctl between sgx_encl_alloc and sgx_encl_create in sgx_ioc_enclave_create? Even if I somehow manage to call this, how is secs->attributes getting set? How do I unset SGX_ATTR_DEBUG? -- Jethro Beekman | Fortanix
On Mon, Dec 24, 2018 at 05:36:36AM +0000, Jethro Beekman wrote: > On 2018-12-22 04:41, Jarkko Sakkinen wrote: > > In order to provide a mechanism for devilering provisoning rights: > > > > 1. Add a new file to the securityfs file called sgx/provision that works > > as a token for allowing an enclave to have the provisioning privileges. > > 2. Add a new ioctl called SGX_IOC_ENCLAVE_SET_ATTRIBUTE that accepts the > > following data structure: > > > > struct sgx_enclave_set_attribute { > > __u64 addr; > > __u64 token_fd; > > }; > > > > A daemon could sit on top of 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 more or less straight-forward. Lets assume that > > dev_fd is a handle to /dev/sgx and prov_fd is a handle to 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); > > I think the attribute handling in this and the previous patch doesn't work. > How am I supposed to call this ioctl between sgx_encl_alloc and > sgx_encl_create in sgx_ioc_enclave_create? Even if I somehow manage to call > this, how is secs->attributes getting set? How do I unset SGX_ATTR_DEBUG? encl->attributes was previously unused (noticed when I wrote this patch). It is actually a mask for allowed attributes that is checked on EINIT. Should rename the variable, yes. /Jarkko
diff --git a/arch/x86/include/uapi/asm/sgx.h b/arch/x86/include/uapi/asm/sgx.h index aadf9c76e360..150a784db395 100644 --- a/arch/x86/include/uapi/asm/sgx.h +++ b/arch/x86/include/uapi/asm/sgx.h @@ -16,6 +16,8 @@ _IOW(SGX_MAGIC, 0x01, struct sgx_enclave_add_page) #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) /* IOCTL return values */ #define SGX_POWER_LOST_ENCLAVE 0x40000000 @@ -56,4 +58,15 @@ struct sgx_enclave_init { __u64 sigstruct; }; +/** + * struct sgx_enclave_set_attribute - parameter structure for the + * %SGX_IOC_ENCLAVE_INIT ioctl + * @addr: address within the ELRANGE + * @attribute_fd: file handle of the attribute file in the securityfs + */ +struct sgx_enclave_set_attribute { + __u64 addr; + __u64 attribute_fd; +}; + #endif /* _UAPI_ASM_X86_SGX_H */ diff --git a/arch/x86/kernel/cpu/sgx/driver/Makefile b/arch/x86/kernel/cpu/sgx/driver/Makefile index 2bd952cc8a65..0325de93d605 100644 --- a/arch/x86/kernel/cpu/sgx/driver/Makefile +++ b/arch/x86/kernel/cpu/sgx/driver/Makefile @@ -1,2 +1,2 @@ obj-$(CONFIG_INTEL_SGX) += sgx.o -sgx-$(CONFIG_INTEL_SGX) += encl.o ioctl.o main.o pte.o vma.o +sgx-$(CONFIG_INTEL_SGX) += encl.o fs.o ioctl.o main.o pte.o vma.o diff --git a/arch/x86/kernel/cpu/sgx/driver/driver.h b/arch/x86/kernel/cpu/sgx/driver/driver.h index c9e40e090c58..e3487a077b6c 100644 --- a/arch/x86/kernel/cpu/sgx/driver/driver.h +++ b/arch/x86/kernel/cpu/sgx/driver/driver.h @@ -167,4 +167,9 @@ struct sgx_encl_page *sgx_fault_page(struct vm_area_struct *vma, int sgx_test_and_clear_young(struct sgx_encl_page *page); void sgx_flush_cpus(struct sgx_encl *encl); +extern const struct file_operations sgx_fs_provision_fops; + +int sgx_fs_init(struct device *dev); +void sgx_fs_remove(void); + #endif /* __ARCH_X86_INTEL_SGX_H__ */ diff --git a/arch/x86/kernel/cpu/sgx/driver/fs.c b/arch/x86/kernel/cpu/sgx/driver/fs.c new file mode 100644 index 000000000000..b36b62a9cafb --- /dev/null +++ b/arch/x86/kernel/cpu/sgx/driver/fs.c @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) +// Copyright(c) 2016-18 Intel Corporation. + +#include <linux/acpi.h> +#include <linux/cdev.h> +#include <linux/platform_device.h> +#include <linux/security.h> +#include <linux/suspend.h> +#include <asm/traps.h> +#include "driver.h" + +const struct file_operations sgx_fs_provision_fops; + +static struct dentry *sgx_fs; +static struct dentry *sgx_fs_provision; + +int sgx_fs_init(struct device *dev) +{ + int ret; + + sgx_fs = securityfs_create_dir(dev_name(dev), NULL); + if (IS_ERR(sgx_fs)) { + ret = PTR_ERR(sgx_fs); + goto err_sgx_fs; + } + + sgx_fs_provision = securityfs_create_file("provision", 0600, sgx_fs, + NULL, &sgx_fs_provision_fops); + if (IS_ERR(sgx_fs)) { + ret = PTR_ERR(sgx_fs_provision); + goto err_sgx_fs_provision; + } + + return 0; + +err_sgx_fs_provision: + securityfs_remove(sgx_fs); + sgx_fs_provision = NULL; + +err_sgx_fs: + sgx_fs = NULL; + return ret; +} + +void sgx_fs_remove(void) +{ + securityfs_remove(sgx_fs_provision); + securityfs_remove(sgx_fs); +} diff --git a/arch/x86/kernel/cpu/sgx/driver/ioctl.c b/arch/x86/kernel/cpu/sgx/driver/ioctl.c index 3a712950e116..d249a4329116 100644 --- a/arch/x86/kernel/cpu/sgx/driver/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/driver/ioctl.c @@ -193,6 +193,46 @@ static long sgx_ioc_enclave_init(struct file *filep, unsigned int cmd, return ret; } +/** + * sgx_ioc_enclave_set_attribute - handler for %SGX_IOC_ENCLAVE_SET_ATTRIBUTE + * @filep: open file to /dev/sgx + * @cmd: the command value + * @arg: pointer to a struct sgx_enclave_set_attribute instance + * + * Sets an attribute matching the attribute file that is pointed by the + * parameter structure field attribute_fd. + * + * Return: 0 on success, -errno otherwise + */ +static long sgx_ioc_enclave_set_attribute(struct file *filep, unsigned int cmd, + unsigned long arg) +{ + struct sgx_enclave_set_attribute *params = (void *)arg; + struct file *attribute_file; + struct sgx_encl *encl; + int ret; + + attribute_file = fget(params->attribute_fd); + if (!attribute_file->f_op) + return -EINVAL; + + if (attribute_file->f_op != &sgx_fs_provision_fops) { + ret = -EINVAL; + goto out; + } + + ret = sgx_encl_get(params->addr, &encl); + if (ret) + goto out; + + encl->attributes |= SGX_ATTR_PROVISIONKEY; + kref_put(&encl->refcount, sgx_encl_release); + +out: + fput(attribute_file); + return ret; +} + typedef long (*sgx_ioc_t)(struct file *filep, unsigned int cmd, unsigned long arg); @@ -212,6 +252,9 @@ long sgx_ioctl(struct file *filep, unsigned int cmd, unsigned long arg) case SGX_IOC_ENCLAVE_INIT: handler = sgx_ioc_enclave_init; break; + case SGX_IOC_ENCLAVE_SET_ATTRIBUTE: + handler = sgx_ioc_enclave_set_attribute; + break; default: return -ENOIOCTLCMD; } diff --git a/arch/x86/kernel/cpu/sgx/driver/main.c b/arch/x86/kernel/cpu/sgx/driver/main.c index 19280863d703..ba125365a0d3 100644 --- a/arch/x86/kernel/cpu/sgx/driver/main.c +++ b/arch/x86/kernel/cpu/sgx/driver/main.c @@ -172,6 +172,10 @@ static int sgx_drv_init(struct device *parent) if (!sgx_encl_wq) return -ENOMEM; + ret = sgx_fs_init(&sgx_dev->ctrl_dev); + if (ret) + goto err_fs_init; + ret = cdev_device_add(&sgx_dev->ctrl_cdev, &sgx_dev->ctrl_dev); if (ret) goto err_device_add; @@ -179,6 +183,9 @@ static int sgx_drv_init(struct device *parent) return 0; err_device_add: + sgx_fs_remove(); + +err_fs_init: destroy_workqueue(sgx_encl_wq); return ret; }
In order to provide a mechanism for devilering provisoning rights: 1. Add a new file to the securityfs file called sgx/provision that works as a token for allowing an enclave to have the provisioning privileges. 2. Add a new ioctl called SGX_IOC_ENCLAVE_SET_ATTRIBUTE that accepts the following data structure: struct sgx_enclave_set_attribute { __u64 addr; __u64 token_fd; }; A daemon could sit on top of 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 more or less straight-forward. Lets assume that dev_fd is a handle to /dev/sgx and prov_fd is a handle to 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: James Morris <jmorris@namei.org> Cc: Serge E. Hallyn <serge@hallyn.com> 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 | 13 +++++++ arch/x86/kernel/cpu/sgx/driver/Makefile | 2 +- arch/x86/kernel/cpu/sgx/driver/driver.h | 5 +++ arch/x86/kernel/cpu/sgx/driver/fs.c | 49 +++++++++++++++++++++++++ arch/x86/kernel/cpu/sgx/driver/ioctl.c | 43 ++++++++++++++++++++++ arch/x86/kernel/cpu/sgx/driver/main.c | 7 ++++ 6 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 arch/x86/kernel/cpu/sgx/driver/fs.c