-
Notifications
You must be signed in to change notification settings - Fork 70
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remote attestation for SGX enclaves #621
Conversation
239c204
to
b5259c5
Compare
b5259c5
to
ab6b7b6
Compare
@@ -0,0 +1,38 @@ | |||
FROM ubuntu:18.04 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, I have not been able to build this image from ubuntu:20.04
.
46a8479
to
4a21512
Compare
@@ -78,6 +82,10 @@ ExternalProject_Add(aws_ext | |||
-DAUTORUN_UNIT_TESTS=OFF | |||
-DENABLE_TESTING=OFF | |||
-DCMAKE_BUILD_TYPE=Release | |||
LOG_CONFIGURE ON |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This supresses some verbosity from the AWS-SDK build. I included it while supressing the SGX related warnigns for cleaner builds.
@@ -31,6 +31,8 @@ endif() | |||
conan_cmake_configure( | |||
REQUIRES | |||
"catch2/2.13.7@#31c8cd08e3c957a9eac8cb1377cf5863" | |||
"jwt-cpp/0.6.0@#cd6b5c1318b29f4becaf807b23f7bb44" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The jwt-cpp
dependency is needed to validate the JWT returned by the Azure Attestation Service. The picojson
dependency should be installed as a dependency of jwt-cpp
, but there seems to be a problem in their conan recipe. I have opened a PR in the jwt-cpp
repo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a comment above here saying that these dependencies are only needed for the Azure attestation service?
#include <sgx_urts.h> | ||
|
||
// What follows are the definitions of the enclave-entry calls. This is, the | ||
// way we have to interact with a running enclave. These calls are shared among | ||
// all function invocations (i.e. module instantiation). | ||
extern "C" | ||
{ | ||
extern faasm_sgx_status_t faasm_sgx_get_sgx_support(void); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was not used anymore, and escaped removal in a previous PR.
@@ -60,36 +60,6 @@ set(WAMR_BUILD_LIB_PTHREAD 0) | |||
# Let WAMR do the including and importing of the sources | |||
include(${WAMR_ROOT_DIR}/build-scripts/runtime_lib.cmake) | |||
|
|||
# WAMR Trusted lib |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These have been moved to inside
and outside
respectively.
6093952
to
05f7ac7
Compare
@@ -0,0 +1,250 @@ | |||
#include <enclave/outside/attestation/AzureAttestationServiceClient.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To read this file it may be helpful to keep in mind that validateQuote
in src/enclave/outside/attestation/validateQuote.cpp
calls the methods in this file in this order:
AzureAttestationServiceClient client(attestationProviderUrl);
// Send enclave quote to remote attestation service for validation
std::string jwtResponse = client.attestEnclave(enclaveInfo);
// Validate JWT response token
client.validateJwtToken(jwtResponse);
05f7ac7
to
a1cca8a
Compare
@@ -0,0 +1,91 @@ | |||
#include <enclave/outside/attestation/attestation.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, this method can only run in Hardware
mode, which means it lies outside our test coverage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just noticed these file names and the header structure. We only use Pascal case in file names when they hold a class. We also usually have .h
and .cpp
files with the same names where possible, as it makes the code easier to navigate. For functions like this we'd put them in a file together with a snake case name. In this case I think you could combine generateQuote.cpp
and validateQuote.cpp
into attestation.cpp
as they're defined in attestation.h
. Is there a reason they're spread out?
84c96b8
to
7be2575
Compare
faasmcli/requirements.txt
Outdated
@@ -1,4 +1,4 @@ | |||
black==21.9b0 | |||
black==22.3.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated to this PR, but fixes the python formatting checks that had broken as a result of psf/black#2964.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM, but I'd like to get the main build working properly again before merging, just to reduce moving parts. That means the cpp, python and faabric updates in this PR shouldn't be necessary, and we'll need to bump it to 0.8.9.
@csegarragonz I've now made the following conflicting changes upstream, but it seems that they've fixed the build.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes to file structure.
@@ -0,0 +1,91 @@ | |||
#include <enclave/outside/attestation/attestation.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just noticed these file names and the header structure. We only use Pascal case in file names when they hold a class. We also usually have .h
and .cpp
files with the same names where possible, as it makes the code easier to navigate. For functions like this we'd put them in a file together with a snake case name. In this case I think you could combine generateQuote.cpp
and validateQuote.cpp
into attestation.cpp
as they're defined in attestation.h
. Is there a reason they're spread out?
…h the exception of enclave held data
dd6a4a8
to
446e1ef
Compare
In this PR I introduce support to remotely attest SGX enclaves using the Azure Attestation Service.
Attesting an SGX enclave consists of two steps: (1) generating a quote for the attested enclave, (2) having the remote attestation service validate said quote.
Quote Generation:
An enclave quote is an enclave's report signed by the platform's Quoting Enclave (QE). An enclave report is a measure of the enclave's memory content signed to the enclave's identity. The QE is a special-purpose Intel-provisioned singleton enclave running in all SGX-enabled and SGX-capable platforms. This means we can only generate a quote in hardware mode.
Additionally, quote generation can happen in two modes
in-proc
andout-of-proc
. The latter is the preferred one for performance and memory consumption, and it is also the required mode for integration with AKS. To generate a quote inout-of-proc
mode, we need to run a service that wraps the QE and signs reports on demand. This service is called AESM, and it listens for incoming requests in a UNIX socket. In a development cluster, we deploy this service as part of our container mesh. In an AKS cluster this service is provided by the AKS runtime.As an artifact of the quote generation process we have a byte array with the enclave's quote. We persist useful information, together with the quote, in a helper class:
sgx::EnclaveInfo
.Quote Validation:
To validate an enclave's quote, we need to send a request to attest an SGX enclave to an Azure Attestation Provider, link to the API. If the request goes through (i.e. the enclave's quote passes the policy loaded in the attestation provider) we receive a JWT. Before deeming the attestation as valid, we check the integrity of the actual JWT.
We run our own Attestation Provider, where we could potentially modify the attestation policy, but so far run the default policy. This service is free.
As part of the PR I include correct and rogue versions of both an enclave's quote and JWT, so that we can also test it in simulation mode.
There are two unrelated issues that make the tests fail. I have reported them in #634 and #635.