| Message ID | 20200601075218.65618-13-jarkko.sakkinen@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Intel SGX foundations | expand |
Hi Jarkko, Just a couple of nits below... On Monday, 2020-06-01 at 10:52:09 +03, Jarkko Sakkinen wrote: > In order to provide a mechanism for devilering provisoning rights: TYPO: s/devilering/delivering/? > > 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 > Acked-by: Jethro Beekman <jethro@fortanix.com> > 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 757cb9a4ae70..713bce437659 100644 > --- a/arch/x86/kernel/cpu/sgx/ioctl.c > +++ b/arch/x86/kernel/cpu/sgx/ioctl.c > @@ -669,6 +669,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". Maybe this should be simply a double-quote rather than the Unicode left quote? Thanks, Darren. > + * > + * 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) > { > @@ -694,6 +738,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;
On Fri, Jun 05, 2020 at 09:09:49PM +0100, Darren Kenny wrote: > TYPO: s/devilering/delivering/? I decided to rewrite the whole thing: x86/sgx: Allow a limited use of ATTRIBUTE.PROVISIONKEY for attestation Provisioning Certification Enclave (PCE), the root of trust for other enclaves, generates a signing key from a fused key called Provisioning Certification Key. PCE can then use this key to certify an attestation key of a QE, e.g. we get the chain of trust down to the hardware if the Intel signed PCE is used. To use the needed keys, ATTRIBUTE.PROVISIONKEY is required but should be only allowed for those who actually need it so that only the trusted parties can certify QE's. Obviously the attestation service should know the public key of the used PCE and that way detect illegit attestation, but whitelisting the legit users still adds an additional layer of defence. Add new device file called /dev/sgx/provision. The sole purpose of this file is to provide file descriptors that act as privilege tokens to allow to build enclaves with ATTRIBUTE.PROVISIONKEY set. A new ioctl called SGX_IOC_ENCLAVE_SET_ATTRIBUTE is used to assign this token to an enclave. > > + * 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". > > Maybe this should be simply a double-quote rather than the Unicode left > quote? Not sure how that got there but it is a mistake. > Thanks, > > Darren. Thanks for the remarks. /Jarkko
On Tue, Jun 16, 2020 at 10:55:11PM +0300, Jarkko Sakkinen wrote: > On Fri, Jun 05, 2020 at 09:09:49PM +0100, Darren Kenny wrote: > > TYPO: s/devilering/delivering/? > > I decided to rewrite the whole thing: > > x86/sgx: Allow a limited use of ATTRIBUTE.PROVISIONKEY for attestation > > Provisioning Certification Enclave (PCE), the root of trust for other > enclaves, generates a signing key from a fused key called Provisioning > Certification Key. PCE can then use this key to certify an attestation key > of a QE, e.g. we get the chain of trust down to the hardware if the Intel > signed PCE is used. > > To use the needed keys, ATTRIBUTE.PROVISIONKEY is required but should be > only allowed for those who actually need it so that only the trusted > parties can certify QE's. > > Obviously the attestation service should know the public key of the used > PCE and that way detect illegit attestation, but whitelisting the legit > users still adds an additional layer of defence. > > Add new device file called /dev/sgx/provision. The sole purpose of this > file is to provide file descriptors that act as privilege tokens to allow > to build enclaves with ATTRIBUTE.PROVISIONKEY set. A new ioctl called > SGX_IOC_ENCLAVE_SET_ATTRIBUTE is used to assign this token to an enclave. I also refined Documentation/x86/sgx.rst based on this in my tree [*]. [*] https://github.com/jsakkine-intel/linux-sgx.git /Jarkko
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 757cb9a4ae70..713bce437659 100644 --- a/arch/x86/kernel/cpu/sgx/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/ioctl.c @@ -669,6 +669,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) { @@ -694,6 +738,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;
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 Acked-by: Jethro Beekman <jethro@fortanix.com> 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(+)