-
Notifications
You must be signed in to change notification settings - Fork 7
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
Genesis Boot core logic and CLI #31
Conversation
} | ||
|
||
fn routes(&self) -> Vec<Box<ProtocolHandler>> { | ||
match self.state.phase { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We gate the available routes based on phase
provisioner, | ||
pivot_file, | ||
ephemeral_key_file, | ||
phase: ProtocolPhase::WaitingForBootInstruction, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can view/update the protocol phase from state
pivot_file, | ||
ephemeral_key_file, | ||
phase: ProtocolPhase::WaitingForBootInstruction, | ||
manifest: None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will always have access to the manifest from state
IOError, | ||
CryptoError, | ||
InvalidManifestApproval(Approval), | ||
NoMatchingRoute(ProtocolPhase), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We add some error types that we can propagate back to qos clients in protocol messages - hopefully we can get rid of all generic errors in protocol message responses
qos-core/src/protocol/genesis.rs
Outdated
// to reconstruct the original Quorum Key? | ||
// | ||
// TODO: Disaster recovery logic! | ||
// Maybe we can just accept 2 set configs, and one is the recovery set?`` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note
qos-core/src/protocol/genesis.rs
Outdated
member_outputs, | ||
quorum_key: quorum_pair.public_key_to_der()?, | ||
// TODO: generate N choose K recovery permutations | ||
recovery_permutations: vec![], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note I put this place holder in for the recovery permutations
fn routes(&self) -> Vec<Box<ProtocolHandler>> { | ||
match self.state.phase { | ||
ProtocolPhase::UnrecoverableError => { | ||
vec![Box::new(handlers::status)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Always include the status handler so we can at least get some insight.
In order for this status endpoint to always be useful, (and not blocked by some other ongoing actions,) we should consider making the enclave server async. I actually don't think it would be too hard. Main thing is we would need to make ProtocolState thread safe by adding a Mutex
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this is a really interesting topic. On the one hand, before pivoting I don't think the operations of the enclave will ever be complicated enough to warrant introducing this complexity. On the other hand, after pivoting we will almost certainly need the secure application to be async.
Let's discuss today
qos-crypto/Cargo.toml
Outdated
openssl = "0.10.40" | ||
serde = { version = "1.0", default-features = false, features = ["derive", "std"] } | ||
serde_cbor = { version = "0.11", default-features = false, features = ["std"] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We needed both of these to have std enabled. I think there was something in the other crate implicitly requiring serde with std enabled farther down the dep tree, so it was building fine. But when we built this crate in isolation we got issues because we didn't implicitly have the std feature
} | ||
|
||
impl RsaPair { | ||
pub fn generate() -> Result<Self, CryptoError> { | ||
Rsa::generate(RSA_KEY_LEN)?.try_into() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrapped this up and replaced it everywhere so we always get the same key len. But need to look into how to add entropy
impl Deref for RsaPair { | ||
type Target = Rsa<Private>; | ||
|
||
fn deref(&self) -> &Self::Target { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good to know about and understand. Basically if you go like *rsa_pair
, or long form rsa_pair.deref()
, you will get a Rsa<Private>
. Kind of magic, but super convenient since we get access to the underlying openssl methods defined on Rsa<Private>
pub fn encrypt(&self, data: &[u8]) -> Result<Vec<u8>, CryptoError> { | ||
let public_key_size = self.public_key.size() as usize; | ||
// TODO: WTF? | ||
if data.len() > public_key_size - 42 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah - no idea why i found the data len had to be 42 less bytes then key size. I just found this from writing tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting! Definitely worth investigating I think :)
@@ -20,20 +20,21 @@ static AWS_NITRO_CERT_SIG_ALG: &[&webpki::SignatureAlgorithm] = | |||
|
|||
/// Corresponds to `MockNsm` attestation document response. This time is | |||
/// valid for the mock and should only be used for testing. | |||
pub(crate) const MOCK_SECONDS_SINCE_EPOCH: u64 = 1652756400; | |||
#[cfg(any(feature = "mock", test))] | |||
pub const MOCK_SECONDS_SINCE_EPOCH: u64 = 1652756400; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feature gated so it doesn't slip into production
|
||
/// AWS Nitro root CA certificate. | ||
/// | ||
/// This should be validated against the checksum: | ||
/// `8cf60e2b2efca96c6a9e71e851d00c1b6991cc09eadbe64a6a1d1b1eb9faff7c`. This | ||
/// checksum and the certificate should be manually verified against | ||
/// https://docs.aws.amazon.com/enclaves/latest/user/verify-root.html. | ||
pub(crate) const AWS_ROOT_CERT: &'static [u8] = | ||
pub const AWS_ROOT_CERT_PEM: &'static [u8] = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is public now, but we should at least check the checksum via CLI
@@ -0,0 +1,318 @@ | |||
//! Types specific to AWS nitro enclave protocol implementation. | |||
use std::collections::BTreeSet; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All these types map 1 to 1 with aws_nitro_enclaves_nsm_api::api
types. We do this so we can derive borsh methods
|
||
// TODO: verify AWS_ROOT_CERT_PEM against a checksum | ||
// TODO: verify PCRs | ||
pub(super) fn boot_genesis(options: ClientOptions) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that boot genesis implicitly creates the genesis set. This is opposed to the initial design of creating the genesis set and writing it to file in one command, and then reading+broadcasting the genesis set in another command.
Now the genesis set is an implementation detail the CLI user doesn't need to know about. Instead they just need to have a folder with the appropriate keys ready.
|
||
// -- CLIENT Create 3 setup keys | ||
// Make sure the directory keys are getting written to already exist. | ||
let _ = std::fs::create_dir(key_dir); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is a good description of flow for genesis ceremony
Also includes
Primary follow up work