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

Rewrite ekiden runtime implementation (aka Fortanix SGX) #1531

Merged
merged 2 commits into from
Apr 8, 2019

Conversation

kostko
Copy link
Member

@kostko kostko commented Mar 14, 2019

Closes #1364
Closes #96
Closes #1022
Closes #1235
Closes #167
Closes #1260
Closes #1222
Closes #52
See #1000 (comment)
See #1318

TODO

  • Verify RAK binding in RPC sessions.
  • Extract MRENCLAVE from sgxs files (use sgxs crate).
  • Persist key manager root hash (currently into a file in node's datadir).
  • Make E2E tests pass.
  • Add cargo-elf2sgxs based on ftxsgx-runner-cargo from fortanix-sgx-tools (that one almost works, but runs the binary as well which is annoying).
  • Update README.
  • Check in Cargo.lock for better reproducibility.
  • Rebase on master.
  • Port over runtime-ethereum.

@kostko kostko mentioned this pull request Mar 14, 2019
@kostko kostko force-pushed the kostko/feature/fortanix-sgx branch 3 times, most recently from 610e937 to f8f1cf6 Compare March 18, 2019 14:14
Copy link
Contributor

@pro-wh pro-wh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dumping comments from older version

/// Dispatch a raw call to the node.
pub fn submit_tx_raw<C>(&self, call: C) -> BoxFuture<Vec<u8>>
where
C: Serialize,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we were to serialize call outside this, either in call or an additional layer, would it save the compiler the trouble of emitting several specializations of this relatively large function?

kind of unbalanced that serializing call happens inside but deserializing the output happens outside

@pro-wh
Copy link
Contributor

pro-wh commented Mar 18, 2019

this PR also rearranges a lot of code. can you post a guide of what new code (roughly) corresponds to what old code?

@kostko
Copy link
Member Author

kostko commented Mar 18, 2019

this PR also rearranges a lot of code. can you post a guide of what new code (roughly) corresponds to what old code?

Sure. So roughly everything from common, storage, db, rpc, runtime (and maybe more) is now in runtime (the part that is used in the runtime implementation) and client (the client part for calling into runtimes). The worker is now runtime-loader. The key manager is now in keymanager-runtime (runtime) and keymanager-client (RPC client wrapper for the key manager).

@kostko kostko force-pushed the kostko/feature/fortanix-sgx branch from f8f1cf6 to baf8f27 Compare March 18, 2019 17:02
@kostko
Copy link
Member Author

kostko commented Mar 18, 2019

The bigger change is that the IPC protocol now goes directly into the runtime instead of only to the worker. This means that some of the attestation stuff (AESM interaction) is now handled at the worker host.

@kostko kostko force-pushed the kostko/feature/fortanix-sgx branch from baf8f27 to ab84ced Compare March 18, 2019 17:37
Copy link
Contributor

@pro-wh pro-wh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dumping the day's comments

ekiden-runtime = { path = "../runtime" }
serde = "1.0.71"
# TODO: Change to released version when 0.10.0 is released.
serde_cbor = { git = "https://github.com/pyfisch/cbor", rev = "114ecaeac53799d0bf81ca8d1b980c7c419d76fe" }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This revision is just the master at a specific point in time. The important PR that we need is pyfisch/cbor#67.

pub mod transaction;

/// Boxed future type.
pub type BoxFuture<T> = Box<futures::Future<Item = T, Error = failure::Error> + Send>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(me) check if we do the same thing elsewhere

runtime/src/rpc/session.rs Outdated Show resolved Hide resolved
client/src/rpc/client.rs Outdated Show resolved Hide resolved
client/src/rpc/client.rs Show resolved Hide resolved
runtime/src/rpc/session.rs Show resolved Hide resolved
client/src/rpc/client.rs Outdated Show resolved Hide resolved
@pro-wh
Copy link
Contributor

pro-wh commented Mar 19, 2019

fortanix-rpc

@kostko kostko force-pushed the kostko/feature/fortanix-sgx branch 2 times, most recently from 59b6547 to ed07194 Compare March 19, 2019 17:26
Copy link
Contributor

@Yawning Yawning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First pass over the Go, I'll look over the rust changes in a bit.

"github.com/oasislabs/ekiden/go/common/sgx/ias"
)

//go:generate protoc --go_out=. aesm_proto.proto
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I understand why this lives here, I'd rather it lived with all the other protobuf files.

go/common/sgx/aesm/aesm.go Outdated Show resolved Hide resolved
go/common/sgx/aesm/aesm.go Outdated Show resolved Hide resolved
go/common/sgx/aesm/aesm.go Outdated Show resolved Hide resolved
go/common/sgx/aesm/aesm.go Outdated Show resolved Hide resolved
go/common/sgx/aesm/aesm.go Outdated Show resolved Hide resolved
go/ekiden/cmd/node/node.go Outdated Show resolved Hide resolved
@kostko kostko force-pushed the kostko/feature/fortanix-sgx branch 5 times, most recently from 5c97bb5 to e5449b2 Compare March 20, 2019 16:09
Copy link
Contributor

@pro-wh pro-wh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dumping today's comments

client/src/rpc/client.rs Show resolved Hide resolved
@kostko kostko force-pushed the kostko/feature/fortanix-sgx branch 3 times, most recently from c41ad76 to 792af83 Compare March 21, 2019 13:50
@kostko kostko force-pushed the kostko/feature/fortanix-sgx branch from 843122c to 5128aa6 Compare April 2, 2019 12:24
@Yawning
Copy link
Contributor

Yawning commented Apr 2, 2019

The OpenBSD project has songs for each release, this is the ekiden 0.3 one.

https://www.youtube.com/watch?v=xCGu5Z_vaps

@kostko kostko force-pushed the kostko/feature/fortanix-sgx branch 4 times, most recently from c881a7c to ed15809 Compare April 3, 2019 17:36
@kostko kostko force-pushed the kostko/feature/fortanix-sgx branch 3 times, most recently from 23b15c9 to c9ddbdc Compare April 4, 2019 11:09
Copy link
Contributor

@Yawning Yawning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yolo.

Copy link
Contributor

@pro-wh pro-wh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also committed to maintaining this

None => return Err(RAKError::NotConfigured.into()),
};
let _authenticated_avr = avr::verify(&avr)?;
// TODO: Verify that the AVR has H(RAK) in report body.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ downstream consumers will check our AVR. this would be a convenience sanity check

@kostko kostko force-pushed the kostko/feature/fortanix-sgx branch 2 times, most recently from c4cc89a to 3e1a08f Compare April 5, 2019 13:18
README.md Outdated Show resolved Hide resolved
@kostko kostko force-pushed the kostko/feature/fortanix-sgx branch 4 times, most recently from 6a04d1c to 2e374db Compare April 8, 2019 09:04
@kostko kostko force-pushed the kostko/feature/fortanix-sgx branch from 2e374db to 3ff39a5 Compare April 8, 2019 09:18
@kostko kostko merged commit c0df8ca into master Apr 8, 2019
@kostko kostko deleted the kostko/feature/fortanix-sgx branch May 9, 2019 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c:breaking/consensus Category: breaking consensus changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants