Skip to content
This repository was archived by the owner on Jun 21, 2020. It is now read-only.

Temporary Increasing TCS amount to 3 #138

Merged
merged 1 commit into from
May 5, 2019
Merged

Temporary Increasing TCS amount to 3 #138

merged 1 commit into from
May 5, 2019

Conversation

elichai
Copy link
Contributor

@elichai elichai commented May 2, 2019

Introduction

The way threads inside the enclave works is by multiple threads trying the access the enclave from the untrusted.

The amount of threads that can do that in parallel is decided by the value of TCSNum in the xml As written here: https://download.01.org/intel-sgx/linux-2.5/docs/Intel_SGX_Developer_Reference_Linux_2.5_Open_Source.pdf page 65.

We don't know enough what's the security implications of launching multiple threads in the enclave, because they share the same memory.

The problem

Right now there are 2 problems:

  1. Both get_state_keys and watch_blocks try to access the enclave while running in separate threads.

  2. The current JSON-RPC server initiates 2 threads even though it should only initiate 1. (Somehow the server always spawn at least 2 threads. paritytech/jsonrpc#425)

Temporary Solution

This PR will temporarly increase the number of allowed threads to 3 to answer the 3 threads I stated above.
This solution cannot go into production without seriously understanding and evaluating the risks.

Future Solution
I think the right solution for production is as follow:

Only 1 entity would control the enclave itself. the eid won’t be passed around and only that entity will make ecalls.

we will try to minimize the amount of threads to the minimum required (it seems that 2 unless we can try and make everything async which isn’t too easy in rust)

The enclave object itself will be guarded by a Mutex so that no 2 threads can make an ecall at the same time. that way they won’t be able to interfere with each other.

CC @fredfortier @lacabra @moriaab @AvishaiW #137

@elichai
Copy link
Contributor Author

elichai commented May 2, 2019

The build is failing due to unrelated issues. I'll fix them in a new PR

*This should be reverted before production*
@elichai elichai force-pushed the elichai-patch-1 branch from ebc7bd0 to d0ec414 Compare May 2, 2019 13:03
@moriaab moriaab assigned moriaab and unassigned moriaab May 2, 2019
@moriaab moriaab self-requested a review May 2, 2019 21:01
@fredfortier fredfortier mentioned this pull request May 2, 2019
@elichai elichai merged commit 296c39e into develop May 5, 2019
@elichai elichai deleted the elichai-patch-1 branch May 5, 2019 09:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants