Skip to content
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

Make our enclave's deps testable with cargo test. #272

Closed
clangenb opened this issue Jun 21, 2021 · 4 comments
Closed

Make our enclave's deps testable with cargo test. #272

clangenb opened this issue Jun 21, 2021 · 4 comments

Comments

@clangenb
Copy link
Contributor

This is currently not possible. The cargo system is (in my opinion) abused for the enclave's conditional compilation. We simply copied the compilation methodology from Teaclave's repository.

How the current build mechanism works:

target_env = "sgx" is used by Xargo only. It implies that Xargo should build the sysroot of the SGX target when given the corresponding cli arg.

Hence, #[cfg(not(target_env = "sgx"))] checks if the enclave is being built with the sysroot of the SGX target.

Now comes the hack:

#![cfg_attr(not(target_env = "sgx"), no_std)] enforces no std if the enclave is not being built with the SGX sysroot - which is always the case if compiled with cargo. This might make sense for the enclave itself, but it inherently limits all other crates to only be usable/testable within an enclave. As we don't use Xargo, I don't see any reason to use this pattern. It neither makes sense to switch to xargo, as it is in maintenance mode since three years. Integration of Xargo's features into Cargo has been requested, but it will take some time until this is ready.

Hence, I suggest changing the approach. I currently believe that it would be the best to change the conditional compilation to something like this:

  • Define conditional sgx deps like: [target.'cfg(not(feature = "std"))'.dependencies]
  • Be no_std if the std feature's disabled: #![cfg_attr(not(features= "std"), no_std)]
  • #[cfg(not(target_env = "sgx"))] extern crate sgx_tstd as std;

Potential problems:

  • Some sgx libs are not identically mirrored in std. -> Solution: make that specific part no_std only.
  • We implicitly assume sgx, if std is disabled. Does that matter? - Don't think so. If we need pure no_std for some parts, it's probably better to isolate that into a separate crate anyhow.

I currently don't see bigger problems, but have not spent much time thinking about caveats.

@brenzi
Copy link
Collaborator

brenzi commented Jun 23, 2021

it may be worth trying. The current setup isn't satisfactory, agreed. But expect a hell lot of trouble on the way!

@clangenb
Copy link
Contributor Author

clangenb commented Aug 9, 2021

A first very minimal proof of concept of cargo test with an enclave-compatible crate is here: #342. The crate that is tested itself is no_std compatible to it is nothing special. However, it uses the mocks from the test-utils crate in #341. The mocks compile to both std and enclave-compatible code.

We can use the mocks' pattern for enclave crates that need sgx_tstd to cargo test them.

@brenzi
Copy link
Collaborator

brenzi commented Sep 7, 2021

can we close this after recent improvements?

@clangenb
Copy link
Contributor Author

clangenb commented Sep 7, 2021

Agreed. We still need to extract some more crates out of the main code. But at this point, making them testable with cargo test, not a big issue.

@clangenb clangenb closed this as completed Sep 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants