From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE, SPF_PASS,T_DKIMWL_WL_HIGH,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id ABCA1C282CE for ; Tue, 4 Jun 2019 20:37:24 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 7BBCD20717 for ; Tue, 4 Jun 2019 20:37:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1559680644; bh=cFvISWqZtenk/itmER7f+dDnKlLEKMgxPk2A4pPepQo=; h=References:In-Reply-To:From:Date:Subject:To:Cc:List-ID:From; b=DitiABaO2ktiEzZuXmTqugTcfajxKpz25NvF/msbNdoH5SOGl+eXJfLYwOAs1+AB7 CzGMlCxz48oFHpvI3FmpbeWW3mh70iv8aFfUlc0NCQfw0rzgVYbUV7P7pPXXfNS9ah xMGvZ4oAo3iTLu5ZD52VeOnSs+EwCBkUfonnGyk8= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726033AbfFDUhY (ORCPT ); Tue, 4 Jun 2019 16:37:24 -0400 Received: from mail.kernel.org ([198.145.29.99]:33722 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726568AbfFDUhX (ORCPT ); Tue, 4 Jun 2019 16:37:23 -0400 Received: from mail-wr1-f43.google.com (mail-wr1-f43.google.com [209.85.221.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 1443D21670 for ; Tue, 4 Jun 2019 20:29:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1559680163; bh=cFvISWqZtenk/itmER7f+dDnKlLEKMgxPk2A4pPepQo=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=2HRcWilzppDkhdZc4TaBjrV5UIkT55QL43VjaSXEiVplRVpcAxBwNkDcNug+37B+Y t9fbgeUJZIdsTchPZypFGwlhJLgpyuR/aNqVnzXAOqtuoX8oEoU5j1y8GrjbLYqYJn ogGs6TSf8PXi12+IZT5rOfJH6kFTruNBQ6p9qQWM= Received: by mail-wr1-f43.google.com with SMTP id n4so14160573wrs.3 for ; Tue, 04 Jun 2019 13:29:23 -0700 (PDT) X-Gm-Message-State: APjAAAWd6y7URMhkD1WdYalAhZ2y/Q4d0iynaEOGzRlrIuf0Wik0J9wD jy1fdFhtOUcvMnEgJO7EeZ5CUp09TcAGKnQTB3INZA== X-Google-Smtp-Source: APXvYqzggKbEmevf1hAGqsqkg6nyK2jhLYRBA68/pK6t43ULfTO2FFzNf9MvZev2j4Pu6QQWtmJouUP2ZUsabFVGcpo= X-Received: by 2002:adf:f2c8:: with SMTP id d8mr92723wrp.221.1559680161582; Tue, 04 Jun 2019 13:29:21 -0700 (PDT) MIME-Version: 1.0 References: <20190531233159.30992-1-sean.j.christopherson@intel.com> <20190531233159.30992-9-sean.j.christopherson@intel.com> In-Reply-To: <20190531233159.30992-9-sean.j.christopherson@intel.com> From: Andy Lutomirski Date: Tue, 4 Jun 2019 13:29:10 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [RFC PATCH 8/9] LSM: x86/sgx: Introduce ->enclave_load() hook for Intel SGX To: Sean Christopherson Cc: Jarkko Sakkinen , Andy Lutomirski , Cedric Xing , Stephen Smalley , James Morris , "Serge E . Hallyn" , LSM List , Paul Moore , Eric Paris , selinux@vger.kernel.org, Jethro Beekman , Dave Hansen , Thomas Gleixner , Linus Torvalds , LKML , X86 ML , linux-sgx@vger.kernel.org, Andrew Morton , nhorman@redhat.com, npmccallum@redhat.com, Serge Ayoun , Shay Katz-zamir , Haitao Huang , Andy Shevchenko , Kai Svahn , Borislav Petkov , Josh Triplett , Kai Huang , David Rientjes , William Roberts , Philip Tricca Content-Type: text/plain; charset="UTF-8" Sender: selinux-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: selinux@vger.kernel.org On Fri, May 31, 2019 at 4:32 PM Sean Christopherson wrote: > > enclave_load() is roughly analogous to the existing file_mprotect(). > > Due to the nature of SGX and its Enclave Page Cache (EPC), all enclave > VMAs are backed by a single file, i.e. /dev/sgx/enclave, that must be > MAP_SHARED. Furthermore, all enclaves need read, write and execute > VMAs. As a result, file_mprotect() does not provide any meaningful > security for enclaves since an LSM can only deny/grant access to the > EPC as a whole. > > security_enclave_load() is called when SGX is first loading an enclave > page, i.e. copying a page from normal memory into the EPC. The notable > difference from file_mprotect() is the allowed_prot parameter, which > is essentially an SGX-specific version of a VMA's MAY_{READ,WRITE,EXEC} > flags. The purpose of allowed_prot is to enable checks such as > SELinux's FILE__EXECMOD permission without having to track and update > VMAs across multiple mm structs, i.e. SGX can ensure userspace doesn't > overstep its bounds simply by restricting an enclave VMA's protections > by vetting what is maximally allowed during build time. > > An alternative to the allowed_prot approach would be to use an enclave's > SIGSTRUCT (a smallish structure that can uniquely identify an enclave) > as a proxy for the enclave. For example, SGX could take and hold a > reference to the file containing the SIGSTRUCT (if it's in a file) and > call security_enclave_load() during mprotect(). While the SIGSTRUCT > approach would provide better precision, the actual value added was > deemed to be negligible. On the other hand, pinning a file for the > lifetime of the enclave is ugly, and essentially caching LSM policies > in each page's allowed_prot avoids having to make an extra LSM upcall > during mprotect(). > > Note, extensive discussion yielded no sane alternative to some form of > SGX specific LSM hook[1]. > > [1] https://lkml.kernel.org/r/CALCETrXf8mSK45h7sTK5Wf+pXLVn=Bjsc_RLpgO-h-qdzBRo5Q@mail.gmail.com > > Signed-off-by: Sean Christopherson > --- > arch/x86/kernel/cpu/sgx/driver/ioctl.c | 14 +++++++++----- > include/linux/lsm_hooks.h | 16 ++++++++++++++++ > include/linux/security.h | 2 ++ > security/security.c | 8 ++++++++ > 4 files changed, 35 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/kernel/cpu/sgx/driver/ioctl.c b/arch/x86/kernel/cpu/sgx/driver/ioctl.c > index 5f71be7cbb01..260417ecbcff 100644 > --- a/arch/x86/kernel/cpu/sgx/driver/ioctl.c > +++ b/arch/x86/kernel/cpu/sgx/driver/ioctl.c > @@ -8,6 +8,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -580,21 +581,24 @@ static int sgx_encl_page_protect(unsigned long src, unsigned long prot, > unsigned long *allowed_prot) > { > struct vm_area_struct *vma; > + int ret = 0; > > - if (!(*allowed_prot & VM_EXEC)) > + if (!(*allowed_prot & VM_EXEC) && !IS_ENABLED(CONFIG_SECURITY)) > goto do_check; > > down_read(¤t->mm->mmap_sem); > vma = find_vma(current->mm, src); > if (!vma || (vma->vm_file && path_noexec(&vma->vm_file->f_path))) > *allowed_prot &= ~VM_EXEC; > +#ifdef CONFIG_SECURITY > + ret = security_enclave_load(vma, prot, allowed_prot); > +#endif > up_read(¤t->mm->mmap_sem); > > do_check: > - if (prot & ~*allowed_prot) > - return -EACCES; > - > - return 0; > + if (!ret && (prot & ~*allowed_prot)) > + ret = -EACCES; > + return ret; > } > > static int sgx_encl_add_page(struct sgx_encl *encl, unsigned long addr, > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h > index 47f58cfb6a19..0562775424a0 100644 > --- a/include/linux/lsm_hooks.h > +++ b/include/linux/lsm_hooks.h > @@ -1446,6 +1446,14 @@ > * @bpf_prog_free_security: > * Clean up the security information stored inside bpf prog. > * > + * Security hooks for Intel SGX enclaves. > + * > + * @enclave_load: > + * On success, returns 0 and optionally adjusts @allowed_prot > + * @vma: the source memory region of the enclave page being loaded. > + * @prot: the initial protection of the enclave page. What do you mean "initial"? The page is always mapped PROT_NONE when this is called, right? I feel like I must be missing something here.