Merge pull request #85 from registreerocks/he-local-attestation-2

Add local attestation support - follow-up
ntls-io · Jun 1, 2021 · 4e090a2 · 4e090a2
2 parents dcc86a5 + 1013f69
commit 4e090a2
Show file tree

Hide file tree

Showing 11 changed files with 73 additions and 17 deletions.
diff --git a/HACKING.md b/HACKING.md
@@ -59,3 +59,34 @@ Rust SGX SDK:
 * [v1.1.0 release notes](https://github.com/apache/incubator-teaclave-sgx-sdk/blob/v1.1.0/release_notes.md#rust-sgx-sdk-v110)
 * [Mitigation of Intel SA 00219 in Rust SGX](https://github.com/apache/incubator-teaclave-sgx-sdk/wiki/Mitigation-of-Intel-SA-00219-in-Rust-SGX)
 
+
+## ECALL enclave name prefixing and --use-prefix
+
+See "[Avoiding Name Collisions]" in the Intel SGX Developer Reference.
+
+[Avoiding Name Collisions]: https://download.01.org/intel-sgx/sgx-linux/2.13/docs/Intel_SGX_Developer_Reference_Linux_2.13_Open_Source.pdf#Avoiding%20Name%20Collisions
+
+When linking more than one enclave library into an application,
+all ECALL and OCALL function names must be unique to avoid linking collisions.
+The `sgx_edger8r` tool automatically prevents OCALL name collisions by
+prepending the enclave name to all bridge functions, but does not do the same
+for ECALL names by default.
+
+This means that when more than one enclave library uses a shared library with
+its own ECALLs, like we do with `rtc_tenclave`, the ECALL function names of
+the different instances of shared library will collide, by default.
+
+To avoid this, we pass the `--use-prefix` option to `sgx_edger8r` to prepend
+the enclave name to all untrusted proxy function names, so that the shared
+library ECALLs will have a unique interface for each enclave library they're
+exposed from.
+
+This means that the shared library's trusted code and EDL will refer to a function
+like `session_request`, but the untrusted code will refer to different per-enclave
+instances of it, like `rtc_auth_session_request`, `rtc_data_session_request`,
+and so on.
+
+However, this means that all other references to the enclave's non-library ECALLs
+will also become prefixed in the same way: the function names in the EDL will use
+the unprefixed form, while the references in the untrusted code must use the
+prefixed from.
diff --git a/edl/rtc_tenclave.edl b/edl/rtc_tenclave.edl
@@ -7,15 +7,16 @@ enclave  {
     include "sgx_eid.h"
     include "sgx_dh.h"
 
-    trusted{
+    // See "ECALL enclave name prefixing and --use-prefix" in HACKING.md
+    trusted {
         public SessionRequestResult session_request(sgx_enclave_id_t src_enclave_id);
         public ExchangeReportResult exchange_report(sgx_enclave_id_t src_enclave_id, [in]const sgx_dh_msg2_t *dh_msg2);
         public sgx_status_t end_session(sgx_enclave_id_t src_enclave_id);
     };
 
-    untrusted{
+    untrusted {
         SessionRequestResult rtc_session_request_u(sgx_enclave_id_t src_enclave_id, sgx_enclave_id_t dest_enclave_id);
         ExchangeReportResult rtc_exchange_report_u(sgx_enclave_id_t src_enclave_id, sgx_enclave_id_t dest_enclave_id, [in]sgx_dh_msg2_t *dh_msg2);
         sgx_status_t rtc_end_session_u(sgx_enclave_id_t src_enclave_id, sgx_enclave_id_t dest_enclave_id);
     };
-};
+};
diff --git a/rtc_auth_enclave/Makefile b/rtc_auth_enclave/Makefile
@@ -98,6 +98,7 @@ $(Out_StaticLib) $(Out_Bindings): $(Crate_Files)
 	@rm -f $(Out_StaticLib) $(Out_Bindings)
 	cargo build --release
 
+# See "ECALL enclave name prefixing and --use-prefix" in HACKING.md
 $(Out_CodegenFiles): $(SGX_EDGER8R) ./$(ENCLAVE_NAME).edl $(Out_Bindings) | $(CODEGEN_PATH)
 	$(SGX_EDGER8R) --use-prefix ./$(ENCLAVE_NAME).edl --search-path $(SGX_SDK)/include --search-path $(RUST_EDL_PATH) --search-path $(RTC_EDL_PATH) --trusted-dir $(CODEGEN_PATH) --untrusted-dir $(CODEGEN_PATH)
 	@echo "GEN  =>  $(Enclave_EDL_Files)"

diff --git a/rtc_auth_enclave/rtc_auth.edl b/rtc_auth_enclave/rtc_auth.edl
@@ -7,6 +7,7 @@ enclave {
     include "sgx_dh.h"
     include "bindings.h"
 
+    // See "ECALL enclave name prefixing and --use-prefix" in HACKING.md
     trusted {
         public CreateReportResult enclave_create_report([in]const sgx_target_info_t* p_qe3_target,
                                               [out, isary]EnclaveHeldData enclave_data,

diff --git a/rtc_data_enclave/Makefile b/rtc_data_enclave/Makefile
@@ -98,6 +98,7 @@ $(Out_StaticLib) $(Out_Bindings): $(Crate_Files)
 	@rm -f $(Out_StaticLib) $(Out_Bindings)
 	cargo build --release
 
+# See "ECALL enclave name prefixing and --use-prefix" in HACKING.md
 $(Out_CodegenFiles): $(SGX_EDGER8R) ./$(ENCLAVE_NAME).edl $(Out_Bindings) | $(CODEGEN_PATH)
 	$(SGX_EDGER8R) --use-prefix ./$(ENCLAVE_NAME).edl --search-path $(SGX_SDK)/include --search-path $(RUST_EDL_PATH) --search-path $(RTC_EDL_PATH) --trusted-dir $(CODEGEN_PATH) --untrusted-dir $(CODEGEN_PATH)
 	@echo "GEN  =>  $(Enclave_EDL_Files)"

diff --git a/rtc_data_enclave/rtc_data.edl b/rtc_data_enclave/rtc_data.edl
@@ -13,6 +13,7 @@ enclave {
     include "sgx_dh.h"
     include "bindings.h"
 
+    // See "ECALL enclave name prefixing and --use-prefix" in HACKING.md
     trusted {
         /* define ECALLs here. */
         public CreateReportResult enclave_create_report([in]const sgx_target_info_t* p_qe3_target,

diff --git a/rtc_types/src/dh.rs b/rtc_types/src/dh.rs
@@ -4,14 +4,16 @@ use sgx_types::*;
 pub type SessionRequestResult = EcallResult<sgx_dh_msg1_t, sgx_status_t>;
 pub type ExchangeReportResult = EcallResult<sgx_dh_msg3_t, sgx_status_t>;
 
+// Note: The following Default implementations are intended for allocating out-parameters only.
+
 impl Default for SessionRequestResult {
     fn default() -> Self {
-        Self::Err(sgx_status_t::SGX_SUCCESS)
+        Self::Ok(sgx_dh_msg1_t::default())
     }
 }
 
 impl Default for ExchangeReportResult {
     fn default() -> Self {
-        Self::Err(sgx_status_t::SGX_SUCCESS)
+        Self::Ok(sgx_dh_msg3_t::default())
     }
 }
diff --git a/rtc_udh/src/lib.rs b/rtc_udh/src/lib.rs
@@ -13,6 +13,7 @@ use rtc_types::{
     EcallResult,
 };
 use sgx_types::*;
+use std::collections::hash_map::Entry;
 
 type SyncSendResponder = Arc<Mutex<Responder>>;
 
@@ -24,16 +25,29 @@ fn dh_responders() -> &'static RwLock<DhResponders> {
 }
 
 /// Register enclave as a DH responder.
+///
+/// # Panics
+///
+/// If `enclave_id` already has a registered responder.
 pub fn set_responder(
     enclave_id: sgx_enclave_id_t,
     responder: Box<(dyn ResponderSys + 'static)>,
 ) -> Result<(), sgx_status_t> {
     match dh_responders().write() {
         Ok(mut resp_map) => {
-            resp_map.insert(
-                enclave_id,
-                Arc::new(Mutex::new(Responder::new(enclave_id, responder))),
-            );
+            let value = Arc::new(Mutex::new(Responder::new(enclave_id, responder)));
+
+            // TODO: Use [`HashMap::try_insert`] once stable.
+            // Unstable tracking issue: <https://github.com/rust-lang/rust/issues/82766>
+            match resp_map.entry(enclave_id) {
+                // TODO: Is there any way to report more useful debug information about
+                //       the new and existing responders?
+                Entry::Occupied(_entry) => panic!(
+                    "set_responder: enclave_id {:?} already has a registered responder",
+                    enclave_id,
+                ),
+                Entry::Vacant(entry) => entry.insert(value),
+            };
             Ok(())
         }
         Err(_) => Err(sgx_status_t::SGX_ERROR_UNEXPECTED),

diff --git a/rtc_uenclave/auth-sys/build.rs b/rtc_uenclave/auth-sys/build.rs
@@ -17,8 +17,10 @@ fn main() {
 
     println!("cargo:rustc-link-search=native={}/lib64", sdk_dir);
 
+    let filename_u = "rtc_auth_u";
+
     let mut base_u = cc::Build::new()
-        .file(enclave_gen.join("rtc_auth_u.c"))
+        .file(enclave_gen.join(filename_u).with_extension("c"))
         .no_default_flags(true)
         .includes(&includes)
         .flag("-fstack-protector")
@@ -30,9 +32,9 @@ fn main() {
         .to_owned();
 
     if profile == "release" {
-        base_u.flag("-O2").compile("rtc_auth_u");
+        base_u.flag("-O2").compile(filename_u);
     } else {
-        base_u.flag("-O0").flag("-g").compile("rtc_auth_u");
+        base_u.flag("-O0").flag("-g").compile(filename_u);
     }
 
     println!("cargo:rerun-if-changed=wrapper.h");

diff --git a/rtc_uenclave/data-sys/Cargo.toml b/rtc_uenclave/data-sys/Cargo.toml
@@ -3,7 +3,7 @@ name = "data-sys"
 version = "0.1.0"
 authors = ["Herman <[email protected]>"]
 edition = "2018"
-links = "Enclave_data_u"
+links = "rtc_data_u"
 
 [lib]
 crate-type = ["rlib"]

diff --git a/rtc_uenclave/data-sys/build.rs b/rtc_uenclave/data-sys/build.rs
@@ -17,8 +17,10 @@ fn main() {
 
     println!("cargo:rustc-link-search=native={}/lib64", sdk_dir);
 
+    let filename_u = "rtc_data_u";
+
     let mut base_u = cc::Build::new()
-        .file(enclave_gen.join("rtc_data_u.c"))
+        .file(enclave_gen.join(filename_u).with_extension("c"))
         .no_default_flags(true)
         .includes(&includes)
         .flag("-fstack-protector")
@@ -30,9 +32,9 @@ fn main() {
         .to_owned();
 
     if profile == "release" {
-        base_u.flag("-O2").compile("rtc_data_u");
+        base_u.flag("-O2").compile(filename_u);
     } else {
-        base_u.flag("-O0").flag("-g").compile("rtc_data_u");
+        base_u.flag("-O0").flag("-g").compile(filename_u);
     }
 
     println!("cargo:rerun-if-changed=wrapper.h");
@@ -57,5 +59,5 @@ fn main() {
         .generate()
         .expect("Unable to generate bindings")
         .write_to_file(out_path.join("bindings.rs"))
-        .expect("Failed to wirte bindings to file");
+        .expect("Failed to write bindings to file");
 }