[Pal/Linux-SGX] Verify enclave attributes during remote attestation

Previously, ATTRIBUTES were not verified during SGX remote attestation in RA-TLS flows. However, MRENCLAVE doesn't reflect ATTRIBUTES of the enclave. This could lead to situations where the verifier doesn't notice ATTRIBUTES.DEBUG = 1 (debug enclave) and shares secrets with this debug enclave. This commit fixes this by verifying all relevant SGX enclave attributes, including DEBUG, INIT, etc. In particular, DEBUG=1 is only allowed if the verifier has set the environment variable `RA_TLS_ALLOW_DEBUG_ENCLAVE_INSECURE=1`. Signed-off-by: Dmitrii Kuvaiskii <dmitrii.kuvaiskii@intel.com>
gramineproject · Oct 1, 2021 · 0e61d7e · 0e61d7e
1 parent 580d025
commit 0e61d7e
Show file tree

Hide file tree

Showing 15 changed files with 186 additions and 131 deletions.
diff --git a/.ci/lib/config.jenkinsfile b/.ci/lib/config.jenkinsfile
@@ -5,6 +5,7 @@ python_platlib = sh(returnStdout: true, script: 'python3 Scripts/get-python-plat
 env.PYTHONPATH = python_platlib + ':' + env.WORKSPACE + '/Scripts'
 
 env.RA_TLS_ALLOW_OUTDATED_TCB_INSECURE = '1'
+env.RA_TLS_ALLOW_DEBUG_ENCLAVE_INSECURE = '1'
 
 env.LC_ALL = 'C.UTF-8'
 env.LANG = env.LC_ALL
diff --git a/CI-Examples/ra-tls-mbedtls/README.md b/CI-Examples/ra-tls-mbedtls/README.md
@@ -52,6 +52,11 @@ RA-TLS verification callback that compares `MRENCLAVE`, `MRSIGNER`,
 variables. To run the client with its own verification callback, execute it with
 four additional command-line arguments (see the source code for details).
 
+Also, because this example builds and uses debug SGX enclaves
+(`SIGSTRUCT.ATTRIBUTES.DEBUG` bit is set to one), we use environment variable
+`RA_TLS_ALLOW_DEBUG_ENCLAVE_INSECURE=1`. Note that in production environments,
+you must *not* use this option!
+
 
 # Quick Start
 
@@ -74,8 +79,9 @@ RA_CLIENT_SPID=12345678901234567890123456789012 RA_CLIENT_LINKABLE=0 make app ep
 
 gramine-sgx ./server epid &
 
-RA_TLS_EPID_API_KEY=12345678901234567890123456789012 \
+RA_TLS_ALLOW_DEBUG_ENCLAVE_INSECURE=1 \
 RA_TLS_ALLOW_OUTDATED_TCB_INSECURE=1 \
+RA_TLS_EPID_API_KEY=12345678901234567890123456789012 \
 RA_TLS_MRENCLAVE=1234567890123456789012345678901234567890123456789012345678901234 \
 RA_TLS_MRSIGNER=1234567890123456789012345678901234567890123456789012345678901234 \
 RA_TLS_ISV_PROD_ID=0 RA_TLS_ISV_SVN=0 \
@@ -94,6 +100,7 @@ make app dcap
 
 gramine-sgx ./server dcap &
 
+RA_TLS_ALLOW_DEBUG_ENCLAVE_INSECURE=1 \
 RA_TLS_ALLOW_OUTDATED_TCB_INSECURE=1 \
 RA_TLS_MRENCLAVE=1234567890123456789012345678901234567890123456789012345678901234 \
 RA_TLS_MRSIGNER=1234567890123456789012345678901234567890123456789012345678901234 \
@@ -114,7 +121,7 @@ make app dcap
 gramine-sgx ./server dcap &
 
 # arguments are: MRENCLAVE in hex, MRSIGNER in hex, ISV_PROD_ID as dec, ISV_SVN as dec
-RA_TLS_ALLOW_OUTDATED_TCB_INSECURE=1 ./client dcap \
+RA_TLS_ALLOW_DEBUG_ENCLAVE_INSECURE=1 RA_TLS_ALLOW_OUTDATED_TCB_INSECURE=1 ./client dcap \
     1234567890123456789012345678901234567890123456789012345678901234 \
     1234567890123456789012345678901234567890123456789012345678901234 \
     0 0
@@ -139,8 +146,9 @@ kill %%
 - RA-TLS flows with SGX and with Gramine, running EPID client in SGX:
 
 Note: you may also add environment variables to `client.manifest.template`, such
-as `RA_TLS_ALLOW_OUTDATED_TCB_INSECURE`, `RA_TLS_MRENCLAVE`, `RA_TLS_MRSIGNER`,
-`RA_TLS_ISV_PROD_ID` and `RA_TLS_ISV_SVN`.
+as `RA_TLS_ALLOW_DEBUG_ENCLAVE_INSECURE`, `RA_TLS_ALLOW_OUTDATED_TCB_INSECURE`,
+`RA_TLS_MRENCLAVE`, `RA_TLS_MRSIGNER`, `RA_TLS_ISV_PROD_ID` and
+`RA_TLS_ISV_SVN`.
 
 ```sh
 make clean

diff --git a/CI-Examples/ra-tls-secret-prov/README.md b/CI-Examples/ra-tls-secret-prov/README.md
@@ -27,6 +27,11 @@ There are two versions of the server: the EPID one and the DCAP one. Each of
 them links against the corresponding EPID/DCAP secret-provisioning library at
 build time.
 
+Because this example builds and uses debug SGX enclaves
+(`SIGSTRUCT.ATTRIBUTES.DEBUG` bit is set to one), we use environment variable
+`RA_TLS_ALLOW_DEBUG_ENCLAVE_INSECURE=1`. Note that in production environments,
+you must *not* use this option!
+
 
 ## Secret Provisioning clients
 
@@ -59,8 +64,10 @@ build time.
 ```sh
 RA_CLIENT_SPID=12345678901234567890123456789012 RA_CLIENT_LINKABLE=0 make app epid files/input.txt
 
+RA_TLS_ALLOW_DEBUG_ENCLAVE_INSECURE=1 \
+RA_TLS_ALLOW_OUTDATED_TCB_INSECURE=1 \
 RA_TLS_EPID_API_KEY=12345678901234567890123456789012 \
-RA_TLS_ALLOW_OUTDATED_TCB_INSECURE=1 ./secret_prov_server_epid &
+./secret_prov_server_epid &
 
 # test minimal client
 gramine-sgx ./secret_prov_min_client
@@ -79,7 +86,9 @@ kill %%
 ```sh
 make app dcap files/input.txt
 
-RA_TLS_ALLOW_OUTDATED_TCB_INSECURE=1 ./secret_prov_server_dcap &
+RA_TLS_ALLOW_DEBUG_ENCLAVE_INSECURE=1 \
+RA_TLS_ALLOW_OUTDATED_TCB_INSECURE=1 \
+./secret_prov_server_dcap &
 
 # test minimal client
 gramine-sgx ./secret_prov_min_client

diff --git a/Documentation/attestation.rst b/Documentation/attestation.rst
@@ -298,13 +298,19 @@ quote are passed as four arguments. It is up to the user to implement the
 correct verification of SGX measurements in this callback (e.g., by comparing
 against expected values stored in a central database).
 
-The library also uses the following SGX-specific environment variable:
+The library also uses the following SGX-specific environment variables:
 
 - ``RA_TLS_ALLOW_OUTDATED_TCB_INSECURE`` (optional) -- whether to allow outdated
   TCB as returned in the IAS attestation report or returned by the DCAP
-  verification library. Values ``1/true/TRUE`` mean "allow outdated TCB". Note
-  that allowing outdated TCB is **insecure** and should be used only for
-  debugging and testing. Outdated TCB is not allowed by default.
+  verification library. Value ``1`` means "allow outdated TCB". Note that
+  allowing outdated TCB is **insecure** and should be used only for debugging
+  and testing. Outdated TCB is not allowed by default.
+
+- ``RA_TLS_ALLOW_DEBUG_ENCLAVE_INSECURE`` (optional) -- whether to allow debug
+  enclaves (enclaves with ``SECS.ATTRIBUTES.DEBUG`` bit set to one). Value ``1``
+  means "allow debug enclaves". Note that allowing debug enclaves is
+  **insecure** and should be used only for debugging and testing. Debug enclaves
+  are not allowed by default.
 
 The library uses the following EPID-specific environment variables if available:
 

diff --git a/Pal/src/host/Linux-SGX/db_process.c b/Pal/src/host/Linux-SGX/db_process.c
@@ -126,6 +126,8 @@ bool is_peer_enclave_ok(sgx_report_body_t* peer_enclave_info,
     /* all Gramine enclaves with same config (manifest) should have same enclave information
      * (in the future, we may exclude `config_svn` and `config_id` from this check -- they are set
      * by the app enclave after EINIT and are thus not security-critical) */
+    static_assert(offsetof(sgx_report_body_t, report_data) + sizeof(*expected_data) ==
+                      sizeof(sgx_report_body_t), "wrong sgx_report_body_t::report_data offset");
     if (memcmp(peer_enclave_info, &g_pal_sec.enclave_info,
                offsetof(sgx_report_body_t, report_data)))
         return false;

diff --git a/Pal/src/host/Linux-SGX/sgx_attest.h b/Pal/src/host/Linux-SGX/sgx_attest.h
@@ -44,6 +44,10 @@ typedef struct _sgx_basename_t {
     uint8_t name[32];
 } sgx_basename_t;
 
+/* TODO: IAS returns an IAS report object that contains a truncated SGX quote inside, without
+ * `signature_len` and `signature` fields. This is called "SGX quote body". We must split the
+ * current `sgx_quote_t` struct into two structs, and use the "SGX quote body" in all relevant
+ * functions instead of the full `sgx_quote_t` (otherwise it will blow up someday). */
 typedef struct _sgx_quote_t {
     uint16_t version;
     uint16_t sign_type;

diff --git a/Pal/src/host/Linux-SGX/tools/README.rst b/Pal/src/host/Linux-SGX/tools/README.rst
@@ -133,6 +133,7 @@ that the quote from the report contains expected values::
       --report-path, -r PATH    Path to the IAS report
       --sig-path, -s PATH       Path to the IAS report's signature
       --allow-outdated-tcb, -o  Treat IAS status GROUP_OUT_OF_DATE as OK
+      --allow-debug-enclave, -d Allow debug enclave (SGXREPORT.ATTRIBUTES.DEBUG = 1)
       --nonce, -n STRING        Nonce that's expected in the report (optional)
       --mr-signer, -S STRING    Expected mr_signer field (hex string, optional)
       --mr-enclave, -E STRING   Expected mr_enclave field (hex string, optional)
@@ -143,7 +144,7 @@ that the quote from the report contains expected values::
 
 Example report verification with all options enabled::
 
-    $ verify_ias_report -v -m -r rp -s sp -i ias.pem -o -n thisisnonce -S 14b284525c45c4f526bf1535d05bd88aa73b9e184464f2d97be3dabc0d187b57 -E 4d69102c40401f40a54eb156601be73fb7605db0601845580f036fd284b7b303 -R 0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004ba476e321e12c720000000000000001 -P 0 -V 0
+    $ verify_ias_report -v -m -r rp -s sp -i ias.pem -o -d -n thisisnonce -S 14b284525c45c4f526bf1535d05bd88aa73b9e184464f2d97be3dabc0d187b57 -E 4d69102c40401f40a54eb156601be73fb7605db0601845580f036fd284b7b303 -R 0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004ba476e321e12c720000000000000001 -P 0 -V 0
     Verbose output enabled
     Endianness set to MSB
     Using IAS public key from file 'ias.pem'
@@ -159,6 +160,7 @@ Example report verification with all options enabled::
     Quote: isv_prod_id OK
     Quote: isv_svn OK
     Quote: report_data OK
+    Quote: enclave attributes OK
 
 
 RA-TLS Libraries

diff --git a/Pal/src/host/Linux-SGX/tools/common/attestation.c b/Pal/src/host/Linux-SGX/tools/common/attestation.c
@@ -84,7 +84,7 @@ void display_report_body(const sgx_report_body_t* body) {
 }
 
 void display_quote(const void* quote_data, size_t quote_size) {
-    if (quote_size < offsetof(sgx_quote_t, signature_len)) {
+    if (quote_size < SGX_QUOTE_BODY_SIZE) {
         ERROR("Quote size too small\n");
         return;
     }
@@ -315,31 +315,6 @@ int verify_ias_report_extract_quote(const uint8_t* ias_report, size_t ias_report
     return ret ? -1 : 0;
 }
 
-int verify_ias_report(const uint8_t* ias_report, size_t ias_report_size, uint8_t* ias_sig_b64,
-                      size_t ias_sig_b64_size, bool allow_outdated_tcb, const char* nonce,
-                      const char* mrsigner, const char* mrenclave, const char* isv_prod_id,
-                      const char* isv_svn, const char* report_data, const char* ias_pub_key_pem,
-                      bool expected_as_str) {
-    int ret;
-
-    uint8_t* report_quote = NULL;
-    size_t quote_size     = 0;
-
-    ret = verify_ias_report_extract_quote(ias_report, ias_report_size, ias_sig_b64,
-                                          ias_sig_b64_size, allow_outdated_tcb, nonce,
-                                          ias_pub_key_pem, &report_quote, &quote_size);
-    if (ret) {
-        goto out;
-    }
-
-    ret = verify_quote(report_quote, quote_size, mrsigner, mrenclave, isv_prod_id, isv_svn,
-                       report_data, expected_as_str);
-
-out:
-    free(report_quote);
-    return ret ? -1 : 0;
-}
-
 int verify_quote(const void* quote_data, size_t quote_size, const char* mr_signer,
                  const char* mr_enclave, const char* isv_prod_id, const char* isv_svn,
                  const char* report_data, bool expected_as_str) {
@@ -350,9 +325,8 @@ int verify_quote(const void* quote_data, size_t quote_size, const char* mr_signe
 
     // Quote contained in the IAS report doesn't contain signature_len and signature fields
     // Reject any smaller quotes as invalid
-    size_t ias_quote_size = offsetof(sgx_quote_t, signature_len);
-    if (quote_size < ias_quote_size) {
-        ERROR("Quote: Bad size %zu < %zu\n", quote_size, ias_quote_size);
+    if (quote_size < SGX_QUOTE_BODY_SIZE) {
+        ERROR("Quote: Bad size %zu < %zu\n", quote_size, SGX_QUOTE_BODY_SIZE);
         goto out;
     }
 
@@ -470,3 +444,32 @@ int verify_quote(const void* quote_data, size_t quote_size, const char* mr_signe
 out:
     return ret;
 }
+
+int verify_quote_enclave_attributes(sgx_quote_t* quote, bool allow_debug_enclave) {
+    if (!allow_debug_enclave && (quote->report_body.attributes.flags & SGX_FLAGS_DEBUG)) {
+        ERROR("Quote: DEBUG bit in enclave attributes is set\n");
+        return -1;
+    }
+
+    /* sanity check: enclave must be initialized */
+    if (!(quote->report_body.attributes.flags & SGX_FLAGS_INITIALIZED)) {
+        ERROR("Quote: INIT bit in enclave attributes is not set\n");
+        return -1;
+    }
+
+    /* sanity check: enclave must not have provision/EINIT token key */
+    if ((quote->report_body.attributes.flags & SGX_FLAGS_PROVISION_KEY) ||
+            (quote->report_body.attributes.flags & SGX_FLAGS_LICENSE_KEY)) {
+        ERROR("Quote: PROVISION_KEY or LICENSE_KEY bit in enclave attributes is set\n");
+        return -1;
+    }
+
+    /* currently only support 64-bit enclaves */
+    if (!(quote->report_body.attributes.flags & SGX_FLAGS_MODE64BIT)) {
+        ERROR("Quote: MODE64 bit in enclave attributes is not set\n");
+        return -1;
+    }
+
+    DBG("Quote: enclave attributes OK\n");
+    return 0;
+}
diff --git a/Pal/src/host/Linux-SGX/tools/common/attestation.h b/Pal/src/host/Linux-SGX/tools/common/attestation.h
@@ -10,6 +10,7 @@
 #include <stdint.h>
 
 #include "sgx_arch.h"
+#include "sgx_attest.h"
 
 /*!
  *  \brief Display internal SGX quote structure.
@@ -29,41 +30,8 @@ void display_quote(const void* quote_data, size_t quote_size);
 void display_report_body(const sgx_report_body_t* body);
 
 /*!
- *  \brief Verify IAS attestation report. Also verify that the quote contained in IAS report
- *         is valid and matches expected values.
- *
- *  \param[in] ias_report         IAS attestation verification report.
- *  \param[in] ias_report_size    Size of \a ias_report in bytes.
- *  \param[in] ias_sig_b64        IAS report signature (base64-encoded as returned by IAS).
- *  \param[in] ias_sig_b64_size   Size of \a ias_sig_b64 in bytes.
- *  \param[in] allow_outdated_tcb Treat IAS status codes: GROUP_OUT_OF_DATE, CONFIGURATION_NEEDED,
- *                                SW_HARDENING_NEEDED, CONFIGURATION_AND_SW_HARDENING_NEEDED as OK.
- *  \param[in] nonce              (Optional) Nonce that's expected in the report.
- *  \param[in] mr_signer          (Optional) Expected mr_signer quote field.
- *  \param[in] mr_enclave         (Optional) Expected mr_enclave quote field.
- *  \param[in] isv_prod_id        (Optional) Expected isv_prod_id quote field.
- *  \param[in] isv_svn            (Optional) Expected isv_svn quote field.
- *  \param[in] report_data        (Optional) Expected report_data quote field.
- *  \param[in] ias_pub_key_pem    (Optional) IAS public RSA key (PEM format, NULL-terminated).
- *                                If not specified, a hardcoded Intel's key is used.
- *  \param[in] expected_as_str    If true, then all expected SGX fields are treated as hex and
- *                                decimal strings. Otherwise, they are treated as raw bytes.
- *
- *  If \a expected_as_str is true, then \a mr_signer, \a mr_enclave and \a report_data are treated
- *  as hex strings, and \a isv_prod_id and a isv_svn are treated as decimal strings. This is
- *  convenient for command-line utilities.
- *
- *  \return 0 on successful verification, negative value on error.
- */
-int verify_ias_report(const uint8_t* ias_report, size_t ias_report_size, uint8_t* ias_sig_b64,
-                      size_t ias_sig_b64_size, bool allow_outdated_tcb, const char* nonce,
-                      const char* mrsigner, const char* mrenclave, const char* isv_prod_id,
-                      const char* isv_svn, const char* report_data, const char* ias_pub_key_pem,
-                      bool expected_as_str);
-
-/*!
- *  \brief Same as verify_ias_report() but instead of verifying the quote contained in IAS report,
- *         this function allocates enough memory to hold the quote and passes it to the user.
+ *  \brief Verify IAS attestation report. Also extract the SGX quote contained in IAS report:
+ *         allocate enough memory to hold the quote and pass it to the user.
  *
  *  \param[in] ias_report         IAS attestation verification report.
  *  \param[in] ias_report_size    Size of \a ias_report in bytes.
@@ -108,4 +76,14 @@ int verify_quote(const void* quote_data, size_t quote_size, const char* mr_signe
                  const char* mr_enclave, const char* isv_prod_id, const char* isv_svn,
                  const char* report_data, bool expected_as_str);
 
+/*!
+ *  \brief Verify enclave attributes of the provided SGX quote.
+ *
+ *  \param[in] quote                Quote to verify.
+ *  \param[in] allow_debug_enclave  If true, then SGXREPORT.ATTRIBUTES.DEBUG can be 1.
+ *
+ *  \return 0 on successful verification, negative value on error.
+ */
+int verify_quote_enclave_attributes(sgx_quote_t* quote, bool allow_debug_enclave);
+
 #endif /* ATTESTATION_H */
diff --git a/Pal/src/host/Linux-SGX/tools/ra-tls/ra_tls.h b/Pal/src/host/Linux-SGX/tools/ra-tls/ra_tls.h
@@ -9,7 +9,8 @@
 
 #define RA_TLS_EPID_API_KEY "RA_TLS_EPID_API_KEY"
 
-#define RA_TLS_ALLOW_OUTDATED_TCB_INSECURE "RA_TLS_ALLOW_OUTDATED_TCB_INSECURE"
+#define RA_TLS_ALLOW_OUTDATED_TCB_INSECURE  "RA_TLS_ALLOW_OUTDATED_TCB_INSECURE"
+#define RA_TLS_ALLOW_DEBUG_ENCLAVE_INSECURE "RA_TLS_ALLOW_DEBUG_ENCLAVE_INSECURE"
 
 #define RA_TLS_MRSIGNER    "RA_TLS_MRSIGNER"
 #define RA_TLS_MRENCLAVE   "RA_TLS_MRENCLAVE"
@@ -34,14 +35,16 @@
     { 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF8, 0x4D, 0x8A, 0x39, (N) }
 static const uint8_t quote_oid[] = OID(0x06);
 static const size_t quote_oid_len = sizeof(quote_oid);
-#define QUOTE_MAX_SIZE 8192
 
 typedef int (*verify_measurements_cb_t)(const char* mrenclave, const char* mrsigner,
                                         const char* isv_prod_id, const char* isv_svn);
 
 /* internally used functions, not exported */
 __attribute__ ((visibility("hidden")))
-int getenv_allow_outdated_tcb(bool* allow_outdated_tcb);
+bool getenv_allow_outdated_tcb(void);
+
+__attribute__ ((visibility("hidden")))
+bool getenv_allow_debug_enclave(void);
 
 __attribute__ ((visibility("hidden")))
 int find_oid(const uint8_t* exts, size_t exts_len, const uint8_t* oid, size_t oid_len,

diff --git a/Pal/src/host/Linux-SGX/tools/ra-tls/ra_tls_attest.c b/Pal/src/host/Linux-SGX/tools/ra-tls/ra_tls_attest.c
@@ -33,6 +33,7 @@
 
 #include "ra_tls.h"
 #include "sgx_arch.h"
+#include "sgx_attest.h"
 
 #define CERT_SUBJECT_NAME_VALUES  "CN=RATLS,O=GramineDevelopers,C=US"
 #define CERT_TIMESTAMP_NOT_BEFORE_DEFAULT "20010101000000"
@@ -175,11 +176,11 @@ static int create_x509(mbedtls_pk_context* pk, mbedtls_x509write_cert* writecrt)
     if (written != sizeof(user_report_data))
         return MBEDTLS_ERR_X509_FILE_IO_ERROR;
 
-    uint8_t* quote = malloc(QUOTE_MAX_SIZE);
+    uint8_t* quote = malloc(SGX_QUOTE_MAX_SIZE);
     if (!quote)
         return MBEDTLS_ERR_X509_ALLOC_FAILED;
 
-    ssize_t quote_size = rw_file("/dev/attestation/quote", quote, QUOTE_MAX_SIZE,
+    ssize_t quote_size = rw_file("/dev/attestation/quote", quote, SGX_QUOTE_MAX_SIZE,
                                  /*do_write=*/false);
     if (quote_size < 0) {
         free(quote);