integritee-network / worker

Integritee off-chain worker and sidechain validateer
Apache License 2.0
90 stars 47 forks source link

enclave secrets are protected with MRSIGNER instead of MRENCLAVE key policy #88

Open brenzi opened 4 years ago

brenzi commented 4 years ago

Currently, we're using SgxFs for persisting the enclave's secrets: https://github.com/scs/substraTEE-worker/blob/15f83244c1d7904f4b95f3fbbc63530b7831b2f2/enclave/src/io.rs#L58

This, however, uses MRSIGNER to auto-derive an encryption key, as explained here: https://www.intel.com/content/www/us/en/developer/articles/technical/overview-of-intel-protected-file-system-library-using-software-guard-extensions.html

What we would like to have is key_policy SGX_KEYPOLICY_MRENCLAVE, as explained here: https://software.intel.com/en-us/blogs/2016/05/04/introduction-to-intel-sgx-sealing

This means we should not use SgxFs, but rather this here: https://github.com/apache/incubator-teaclave-sgx-sdk/blob/15b8470dd50e9628e1cb7a0fdb08989f0b5ecc35/sgx_tseal/src/seal.rs#L276

Reasoning: As SusbtraTEE is an unpermissioned open source project, there can be no SIGNER authority. Everyone can build their own enclave and sign it with an ephemeral key

brenzi commented 4 years ago

It would be possible to supply a key to SgxFs: https://github.com/apache/incubator-teaclave-sgx-sdk/blob/8d9f05abd2ebe0a3f1db901f2280513c36325553/sgx_tstd/src/sgxfs.rs#L121 however, it seems more elegant to use SgxSealedData::seal_data_ex() instead and then just writing it to standard Fs: https://github.com/apache/incubator-teaclave-sgx-sdk/blob/15b8470dd50e9628e1cb7a0fdb08989f0b5ecc35/sgx_tseal/src/seal.rs#L276

brenzi commented 4 years ago

Tests: In order to test that this bug is fixed, do:

  1. make all mrsigner mrenclave remember output for MRSIGNER and MRENCLAVE
  2. run substraTEE-worker getsignkey
  3. remember output of xxd ed25519_key_sealed.bin |head -2
  4. remember the output of xxd ecc_pubkey.bin
  5. now change the enclave signer key
    1. cd enclave
    2. `openssl genrsa -3 -out Enclave_private.pem 3072
  6. make all mrsigner mrenclave. MRENCLAVE should stay the same as above. MRSIGNER should've changed
  7. run substraTEE-worker getsignkey (it would be better if this fails, but it will just overwrite the key if it can't read it)
  8. xxd ecc_pubkey.bin
  9. now, change the binary by modification of code
  10. rerun the above

Here's a dump of what happens todaxy, with MRSIGNER policy:


Succeed.
MRSIGNER: d11b71385c1225466236e6c6539d4b2a27a48121f4bfb48a69810191841af53c
Succeed.
MRENCLAVE: 7921acf80f13619ba553f8678b151c25c91552fdc61ae611518dfd33e763da83

xxd ed25519_key_sealed.bin |head -2
00000000: 454c 4946 5f58 4753 0100 f553 a266 0f95  ELIF_XGS...S.f..
00000010: a823 4560 0cf6 5d0d aa87 84ae 0f90 7c5e  .#E`..].......|^
xxd ecc_pubkey.bin 
00000000: 310c 5c47 60e5 418b 7c6c 564a 9520 6e16  1.\G`.A.|lVJ. n.
00000010: b049 ba32 6f2b 0369 0dcb 7223 2568 739c  .I.2o+.i..r#%hs.

openssl genrsa -3 -out ../enclave/Enclave_private.pem 3072

make all mrsigner mrenclave

Succeed.
MRSIGNER: eb01b00b426a371758effbd62205381537cf453452437d2fd7cbb17e60ffdcc8
Succeed.
MRENCLAVE: 7921acf80f13619ba553f8678b151c25c91552fdc61ae611518dfd33e763da83

xxd ed25519_key_sealed.bin |head -2
00000000: 454c 4946 5f58 4753 0100 fb69 3ca3 c491  ELIF_XGS...i<...
00000010: e046 bd9b fd29 d59e aed1 14b2 44d4 f9d3  .F...)......D...

-> has been overwritten! (now fixed, it would panic now

With MRSIGNER policy, the key can't be retrieved because MRSIGNER changed
With MRENCLAVE policy, the key can be retrieved because MRENCLAVE didn't change

change binary

Enclave is in Development Mode
Succeed.
MRSIGNER: eb01b00b426a371758effbd62205381537cf453452437d2fd7cbb17e60ffdcc8
Succeed.
MRENCLAVE: 04bee684d3813bed654e37ae90b15b8601f01bae518b081620f1e467bd2b2c66

xxd ed25519_key_sealed.bin |head -2
00000000: 454c 4946 5f58 4753 0100 fb69 3ca3 c491  ELIF_XGS...i<...
00000010: e046 bd9b fd29 d59e aed1 14b2 44d4 f9d3  .F...)......D...
abrenzikofer@devsgx02:~/substraTEE-worker/bin$ xxd ecc_pubkey.bin 
00000000: 7a0f 7cb8 feb3 9ff4 9df5 2335 27e9 023b  z.|.......#5'..;
00000010: 6682 8003 d759 d805 68cc 3618 a8ac 225e  f....Y..h.6..."^

this should not work if MRSIGNER key policy. it should panic:
thread '<unnamed>' panicked at '[Enclave] Keyfile ed25519_key_sealed.bin exists but can't be opened. has it been written by the same enclave?', src/ed25519.rs:44:4
note: Call backtrace::enable_backtrace with 'PrintFormat::Short' for a backtrace.
Illegal instruction (core dumped)

With MRSIGNER policy, the key can be retrieved because MRSIGNER didn't change
With MRENCLAVE policy, the key can't be retrieved because MRENCLAVE changed
brenzi commented 4 years ago

We should wait with fixing this until we can build the enclave deterministically. Until then, SCS shall act as the vendor and distribute enclave builds through github releases

Watch this: https://github.com/apache/incubator-teaclave-sgx-sdk/issues/52