From 29d712b081240743924367b329b58b3c170101db Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Sun, 22 May 2022 21:31:50 -0700 Subject: [PATCH 01/25] CI checks for build, test, lint, and dep vulnerabilities --- .github/workflows/audit.yml | 21 ++++++++++ .github/workflows/pr.yml | 81 +++++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+) create mode 100644 .github/workflows/audit.yml create mode 100644 .github/workflows/pr.yml diff --git a/.github/workflows/audit.yml b/.github/workflows/audit.yml new file mode 100644 index 00000000..54480faf --- /dev/null +++ b/.github/workflows/audit.yml @@ -0,0 +1,21 @@ +# Perform a audit any time a change is dependencies is detected in Cargo.toml or Cargo.lock + +name: Security audit +on: + push: + paths: + - '**/Cargo.toml' + - '**/Cargo.lock' + pull_request: + paths: + - '**/Cargo.toml' + - '**/Cargo.lock' + +jobs: + security_audit: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - uses: actions-rs/audit-check@v1 + with: + token: ${{ secrets.GITHUB_TOKEN }} \ No newline at end of file diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml new file mode 100644 index 00000000..4e0e1ded --- /dev/null +++ b/.github/workflows/pr.yml @@ -0,0 +1,81 @@ +# Checks for PRs + +on: [push, pull_request] + +name: PR + +jobs: + build: + name: Build + runs-on: ubuntu-latest + strategy: + matrix: + target: + - ubuntu-latest + - macOS-latest + steps: + - uses: actions/checkout@v2 + - uses: actions-rs/toolchain@v1 + with: + toolchain: stable + target: ${{ matrix.target }} + override: true + - uses: actions-rs/cargo@v1 + with: + use-cross: true + command: build + args: --release --target=${{ matrix.target }} + + test: + name: Test Suite + runs-on: ubuntu-latest + strategy: + matrix: + target: + - ubuntu-latest + - macOS-latest + steps: + - name: Checkout sources + uses: actions/checkout@v2 + + - name: Install stable toolchain + uses: actions-rs/toolchain@v1 + with: + profile: minimal + toolchain: stable + target: ${{ matrix.target }} + override: true + + - name: Run cargo test + uses: actions-rs/cargo@v1 + with: + use-cross: true + command: test + args: --target=${{ matrix.target }} + + lints: + name: Lints + runs-on: ubuntu-latest + steps: + - name: Checkout sources + uses: actions/checkout@v2 + + - name: Install stable toolchain + uses: actions-rs/toolchain@v1 + with: + profile: minimal + toolchain: stable + override: true + components: rustfmt, clippy + + - name: Run cargo fmt + uses: actions-rs/cargo@v1 + with: + command: fmt + args: --all -- --check + + - name: Run cargo clippy + uses: actions-rs/cargo@v1 + with: + command: clippy + args: -- -D warnings \ No newline at end of file From a02b8ea183cc41b44bf2e38ef5025e71a72718e2 Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Sun, 22 May 2022 21:32:48 -0700 Subject: [PATCH 02/25] fmt --- .github/workflows/audit.yml | 1 + .github/workflows/pr.yml | 1 + qos-core/src/io/stream.rs | 2 +- qos-core/src/protocol/provisioner.rs | 4 ++-- qos-crypto/src/shamir.rs | 2 +- qos-host/src/cli.rs | 2 +- qos-host/src/lib.rs | 2 +- rustfmt.toml | 7 +++---- 8 files changed, 11 insertions(+), 10 deletions(-) diff --git a/.github/workflows/audit.yml b/.github/workflows/audit.yml index 54480faf..3cd17528 100644 --- a/.github/workflows/audit.yml +++ b/.github/workflows/audit.yml @@ -1,4 +1,5 @@ # Perform a audit any time a change is dependencies is detected in Cargo.toml or Cargo.lock +# See: https://github.com/actions-rs/audit-check name: Security audit on: diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 4e0e1ded..70be8cd7 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -1,4 +1,5 @@ # Checks for PRs +# See: https://github.com/actions-rs/example/blob/master/.github/workflows/quickstart.yml on: [push, pull_request] diff --git a/qos-core/src/io/stream.rs b/qos-core/src/io/stream.rs index afa92481..cf1328cc 100644 --- a/qos-core/src/io/stream.rs +++ b/qos-core/src/io/stream.rs @@ -306,7 +306,7 @@ mod test { while let Some(stream) = listener.next() { let req = stream.recv().unwrap(); stream.send(&req).unwrap(); - break + break; } }); diff --git a/qos-core/src/protocol/provisioner.rs b/qos-core/src/protocol/provisioner.rs index 32304fb2..923c0bb6 100644 --- a/qos-core/src/protocol/provisioner.rs +++ b/qos-core/src/protocol/provisioner.rs @@ -20,7 +20,7 @@ impl SecretProvisioner { pub fn add_share(&mut self, share: Share) -> Result<(), ProtocolError> { if share.len() == 0 { - return Err(ProtocolError::InvalidShare) + return Err(ProtocolError::InvalidShare); } self.shares.push(share); @@ -32,7 +32,7 @@ impl SecretProvisioner { // TODO: Add better validation... if secret.len() == 0 { - return Err(ProtocolError::ReconstructionError) + return Err(ProtocolError::ReconstructionError); } // TODO: Make errors more specific... diff --git a/qos-crypto/src/shamir.rs b/qos-crypto/src/shamir.rs index f5d4ab30..efae2e24 100644 --- a/qos-crypto/src/shamir.rs +++ b/qos-crypto/src/shamir.rs @@ -192,7 +192,7 @@ pub fn shares_reconstruct>(shares: &[S]) -> Vec { // This matches the behavior of bad shares (random output) and simplifies // consumers. if len == 0 { - return vec![] + return vec![]; } let mut secret = vec![]; diff --git a/qos-host/src/cli.rs b/qos-host/src/cli.rs index c442ef06..582c028c 100644 --- a/qos-host/src/cli.rs +++ b/qos-host/src/cli.rs @@ -37,7 +37,7 @@ impl HostOptions { return format!( "http://{}.{}.{}.{}:{}", ip[0], ip[1], ip[2], ip[3], port - ) + ); } else { panic!("Couldn't parse URL from options.") } diff --git a/qos-host/src/lib.rs b/qos-host/src/lib.rs index 4e903f66..6f141e13 100644 --- a/qos-host/src/lib.rs +++ b/qos-host/src/lib.rs @@ -94,7 +94,7 @@ impl HostServer { StatusCode::BAD_REQUEST, serde_cbor::to_vec(&ProtocolMsg::ErrorResponse) .expect("ProtocolMsg can always serialize. qed."), - ) + ); } match serde_cbor::from_slice(&body) { diff --git a/rustfmt.toml b/rustfmt.toml index 66c428e7..55bbfe73 100644 --- a/rustfmt.toml +++ b/rustfmt.toml @@ -7,15 +7,14 @@ tab_spaces = 4 # Line wrapping use_small_heuristics = "Max" -wrap_comments = true +# wrap_comments = true max_width = 80 # Imports -imports_granularity = "Crate" +# imports_granularity = "Crate" reorder_imports = true -group_imports = "StdExternalCrate" +# group_imports = "StdExternalCrate" # Random use_field_init_shorthand = true newline_style = "Unix" -trailing_semicolon = false From e61ebd22883e984263c4f899ac3f55bdb097a5b3 Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Sun, 22 May 2022 21:36:31 -0700 Subject: [PATCH 03/25] Try and simplify PR checks --- .github/workflows/pr.yml | 48 +++++++++++++++++----------------------- 1 file changed, 20 insertions(+), 28 deletions(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 70be8cd7..4db83edb 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -6,35 +6,30 @@ on: [push, pull_request] name: PR jobs: - build: - name: Build - runs-on: ubuntu-latest - strategy: - matrix: - target: - - ubuntu-latest - - macOS-latest - steps: - - uses: actions/checkout@v2 - - uses: actions-rs/toolchain@v1 - with: - toolchain: stable - target: ${{ matrix.target }} - override: true - - uses: actions-rs/cargo@v1 - with: - use-cross: true - command: build - args: --release --target=${{ matrix.target }} + # build: + # name: Build + # runs-on: ubuntu-latest + # strategy: + # matrix: + # target: + # - ubuntu-latest + # - macOS-latest + # steps: + # - uses: actions/checkout@v2 + # - uses: actions-rs/toolchain@v1 + # with: + # toolchain: stable + # target: ${{ matrix.target }} + # override: true + # - uses: actions-rs/cargo@v1 + # with: + # use-cross: true + # command: build + # args: --release --target=${{ matrix.target }} test: name: Test Suite runs-on: ubuntu-latest - strategy: - matrix: - target: - - ubuntu-latest - - macOS-latest steps: - name: Checkout sources uses: actions/checkout@v2 @@ -44,15 +39,12 @@ jobs: with: profile: minimal toolchain: stable - target: ${{ matrix.target }} override: true - name: Run cargo test uses: actions-rs/cargo@v1 with: - use-cross: true command: test - args: --target=${{ matrix.target }} lints: name: Lints From 60badb1622a8b6d3c471348a72e516f96c427a22 Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Sun, 22 May 2022 21:37:34 -0700 Subject: [PATCH 04/25] Only run on PR --- .github/workflows/pr.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 4db83edb..ab293ae1 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -1,7 +1,7 @@ # Checks for PRs # See: https://github.com/actions-rs/example/blob/master/.github/workflows/quickstart.yml -on: [push, pull_request] +on: [pull_request] name: PR From e4aa83b276aac7d549287488116b520a75cac7a2 Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Sun, 22 May 2022 22:07:03 -0700 Subject: [PATCH 05/25] Address clippy lint warnings --- qos-client/src/attest.rs | 18 ++++---- qos-client/src/cli.rs | 20 ++++----- qos-client/src/lib.rs | 2 +- qos-core/src/cli.rs | 52 ++++++++++------------ qos-core/src/client.rs | 4 +- qos-core/src/io/stream.rs | 7 +-- qos-core/src/protocol/attestor.rs | 2 +- qos-core/src/protocol/mod.rs | 4 +- qos-core/src/protocol/msg.rs | 4 -- qos-core/src/protocol/provisioner.rs | 4 +- qos-core/src/server.rs | 4 +- qos-crypto/src/shamir.rs | 8 ++-- qos-host/src/cli.rs | 64 +++++++++++++--------------- qos-host/src/lib.rs | 12 +++--- qos-test/tests/end_to_end.rs | 7 +-- 15 files changed, 93 insertions(+), 119 deletions(-) diff --git a/qos-client/src/attest.rs b/qos-client/src/attest.rs index 31bde9a3..c19f186a 100644 --- a/qos-client/src/attest.rs +++ b/qos-client/src/attest.rs @@ -56,7 +56,7 @@ pub mod nitro { /// AWS Nitro root CA certificate. /// This should be validated against the below checksum: /// `8cf60e2b2efca96c6a9e71e851d00c1b6991cc09eadbe64a6a1d1b1eb9faff7c` - pub(crate) const AWS_ROOT_CERT: &'static [u8] = + pub(crate) const AWS_ROOT_CERT: &[u8] = std::include_bytes!("./aws_root_cert.pem"); /// Extract a DER encoded certificate from bytes representing a PEM encoded @@ -126,7 +126,7 @@ pub mod nitro { /// Verify the certificate chain against the root & end entity certificates. fn verify_certificate_chain( - cabundle: &Vec, + cabundle: &[ByteBuf], root_cert: &[u8], end_entity_certificate: &[u8], validation_time: u64, @@ -136,7 +136,7 @@ pub mod nitro { // (first element). Ordering is: root cert .. intermediate certs .. // end entity cert. let intermediate_certs: Vec<_> = - cabundle[1..].into_iter().map(|x| x.as_slice()).collect(); + cabundle[1..].iter().map(|x| x.as_slice()).collect(); let anchor = vec![webpki::TrustAnchor::try_from_cert_der(root_cert)?]; let anchors = webpki::TlsServerTrustAnchors(&anchor); @@ -148,7 +148,7 @@ pub mod nitro { &intermediate_certs, webpki::Time::from_seconds_since_unix_epoch(validation_time), ) - .map_err(|e| AttestError::InvalidCertChain(e))?; + .map_err(AttestError::InvalidCertChain)?; Ok(()) } @@ -187,8 +187,8 @@ pub mod nitro { use super::*; /// Mandatory field - pub(super) fn module_id(id: &String) -> Result<(), AttestError> { - if id.len() < 1 { + pub(super) fn module_id(id: &str) -> Result<(), AttestError> { + if id.is_empty() { Err(AttestError::InvalidModuleId) } else { Ok(()) @@ -198,7 +198,7 @@ pub mod nitro { pub(super) fn pcrs( pcrs: &BTreeMap, ) -> Result<(), AttestError> { - let is_valid_pcr_count = pcrs.len() > 0 && pcrs.len() <= 32; + let is_valid_pcr_count = !pcrs.is_empty() && pcrs.len() <= 32; let is_valid_index_and_len = pcrs.iter().all(|(idx, pcr)| { let is_valid_idx = *idx <= 32; @@ -214,9 +214,9 @@ pub mod nitro { } /// Mandatory field pub(super) fn cabundle( - cabundle: &Vec, + cabundle: &[ByteBuf], ) -> Result<(), AttestError> { - let is_valid_len = cabundle.len() > 0; + let is_valid_len = !cabundle.is_empty(); let is_valid_entries = cabundle .iter() .all(|cert| cert.len() >= 1 || cert.len() <= 1024); diff --git a/qos-client/src/cli.rs b/qos-client/src/cli.rs index b44ee31a..93793839 100644 --- a/qos-client/src/cli.rs +++ b/qos-client/src/cli.rs @@ -28,9 +28,10 @@ impl Command { } } } -impl Into for &str { - fn into(self) -> Command { - match self { + +impl From<&str > for Command { + fn from(s: &str) -> Command { + match s { "health" => Command::Health, "echo" => Command::Echo, "describe-nsm" => Command::DescribeNsm, @@ -41,12 +42,12 @@ impl Into for &str { } } + #[derive(Clone, PartialEq, Debug)] struct ClientOptions { cmd: Command, host: HostOptions, echo: EchoOptions, - // ... other options } impl ClientOptions { /// Create `ClientOptions` from the command line arguments. @@ -59,14 +60,14 @@ impl ClientOptions { }; let mut chunks = args.chunks_exact(2); - if chunks.remainder().len() > 0 { + if !chunks.remainder().is_empty() { panic!("Unexpected number of arguments"); } while let Some([cmd, arg]) = chunks.next() { - options.host.parse(&cmd, &arg); + options.host.parse(cmd, arg); match options.cmd { - Command::Echo => options.echo.parse(&cmd, arg), + Command::Echo => options.echo.parse(cmd, arg), Command::Health => {} Command::DescribeNsm => {} Command::MockAttestation => {} @@ -104,9 +105,8 @@ impl EchoOptions { Self { data: None } } fn parse(&mut self, cmd: &str, arg: &str) { - match cmd { - "--data" => self.data = Some(arg.to_string()), - _ => {} + if cmd == "--data" { + self.data = Some(arg.to_string()) }; } fn data(&self) -> String { diff --git a/qos-client/src/lib.rs b/qos-client/src/lib.rs index 7200f860..0500d423 100644 --- a/qos-client/src/lib.rs +++ b/qos-client/src/lib.rs @@ -24,7 +24,7 @@ pub mod request { .read_to_end(&mut buf) .map_err(|_| "send response error".to_string())?; - let pr = serde_cbor::from_slice(&mut buf) + let pr = serde_cbor::from_slice(&buf) .map_err(|_| "send response error".to_string())?; Ok(pr) diff --git a/qos-core/src/cli.rs b/qos-core/src/cli.rs index 7a17d2bf..b0ccb2c4 100644 --- a/qos-core/src/cli.rs +++ b/qos-core/src/cli.rs @@ -6,7 +6,7 @@ use crate::{ server::SocketServer, }; -#[derive(Clone, Debug, PartialEq)] +#[derive(Default, Clone, Debug, PartialEq)] pub struct EnclaveOptions { cid: Option, port: Option, @@ -16,14 +16,14 @@ pub struct EnclaveOptions { impl EnclaveOptions { pub fn new() -> Self { - Self { cid: None, port: None, usock: None, mock: false } + Default::default() } fn from(args: Vec) -> EnclaveOptions { let mut options = EnclaveOptions::new(); let mut chunks = args.chunks_exact(2); - if chunks.remainder().len() > 0 { + if !chunks.remainder().is_empty() { panic!("Unexepected number of arguments") } @@ -42,45 +42,37 @@ impl EnclaveOptions { } pub fn parse_cid(&mut self, cmd: &str, arg: &str) { - match cmd { - "--cid" => { - self.cid = arg - .parse::() - .map_err(|_| { - panic!("Could not parse provided value for `--cid`") - }) - .ok(); - } - _ => {} + if cmd == "--cid" { + self.cid = arg + .parse::() + .map_err(|_| { + panic!("Could not parse provided value for `--cid`") + }) + .ok(); } } pub fn parse_port(&mut self, cmd: &str, arg: &str) { - match cmd { - "--port" => { - self.port = arg - .parse::() - .map_err(|_| { - panic!("Could not parse provided value for `--port`") - }) - .ok(); - } - _ => {} + if cmd == "--port" { + self.port = arg + .parse::() + .map_err(|_| { + panic!("Could not parse provided value for `--port`") + }) + .ok(); } } pub fn parse_usock(&mut self, cmd: &str, arg: &str) { - match cmd { - "--usock" => self.usock = Some(arg.to_string()), - _ => {} + if cmd == "--usock" { + self.usock = Some(arg.to_string()) } } pub fn parse_mock(&mut self, cmd: &str, arg: &str) { - match cmd { - "--mock" => self.mock = arg == "true", - _ => {} - } + if cmd == "--mock" { + self.mock = arg == "true" + }; } pub fn addr(&self) -> SocketAddress { diff --git a/qos-core/src/client.rs b/qos-core/src/client.rs index 145033c4..fdc22e0f 100644 --- a/qos-core/src/client.rs +++ b/qos-core/src/client.rs @@ -51,7 +51,7 @@ impl Client { &serde_cbor::to_vec(&request) .expect("ProtocolMsg can be serialized. qed."), )?; - let mut response = stream.recv()?; - serde_cbor::from_slice(&mut response).map_err(Into::into) + let response = stream.recv()?; + serde_cbor::from_slice(&response).map_err(Into::into) } } diff --git a/qos-core/src/io/stream.rs b/qos-core/src/io/stream.rs index cf1328cc..d6ddc1c9 100644 --- a/qos-core/src/io/stream.rs +++ b/qos-core/src/io/stream.rs @@ -64,7 +64,6 @@ pub struct Stream { } impl Stream { - #[must_use] pub(crate) fn connect(addr: &SocketAddress) -> Result { let mut err = IOError::UnknownError; @@ -88,8 +87,7 @@ impl Stream { Err(err) } - #[must_use] - pub(crate) fn send(&self, buf: &Vec) -> Result<(), IOError> { + pub(crate) fn send(&self, buf: &[u8]) -> Result<(), IOError> { let len = buf.len(); // First, send the length of the buffer @@ -130,7 +128,6 @@ impl Stream { Ok(()) } - #[must_use] pub(crate) fn recv(&self) -> Result, IOError> { // First, read the length let length: usize = { @@ -264,7 +261,7 @@ fn socket_fd(addr: &SocketAddress) -> Result { // is both a type and protocol. None, ) - .map_err(|e| IOError::NixError(e)) + .map_err(IOError::NixError) } #[cfg(test)] diff --git a/qos-core/src/protocol/attestor.rs b/qos-core/src/protocol/attestor.rs index 66ed82e1..df81ac02 100644 --- a/qos-core/src/protocol/attestor.rs +++ b/qos-core/src/protocol/attestor.rs @@ -16,7 +16,7 @@ pub trait NsmProvider { pub struct Nsm; impl NsmProvider for Nsm { fn nsm_process_request(&self, fd: i32, request: NsmRequest) -> NsmResponse { - nsm::driver::nsm_process_request(fd, request.into()).into() + nsm::driver::nsm_process_request(fd, request) } fn nsm_init(&self) -> i32 { diff --git a/qos-core/src/protocol/mod.rs b/qos-core/src/protocol/mod.rs index 6538b18b..8e40bb2d 100644 --- a/qos-core/src/protocol/mod.rs +++ b/qos-core/src/protocol/mod.rs @@ -52,13 +52,13 @@ impl Executor { } impl server::Routable for Executor { - fn process(&mut self, mut req_bytes: Vec) -> Vec { + fn process(&mut self, req_bytes: Vec) -> Vec { if req_bytes.len() > MAX_ENCODED_MSG_LEN { return serde_cbor::to_vec(&ProtocolMsg::ErrorResponse) .expect("ProtocolMsg can always be serialized. qed."); } - let msg_req = match serde_cbor::from_slice(&mut req_bytes) { + let msg_req = match serde_cbor::from_slice(&req_bytes) { Ok(req) => req, Err(_) => { return serde_cbor::to_vec(&ProtocolMsg::ErrorResponse) diff --git a/qos-core/src/protocol/msg.rs b/qos-core/src/protocol/msg.rs index 4ff86e89..74f3d205 100644 --- a/qos-core/src/protocol/msg.rs +++ b/qos-core/src/protocol/msg.rs @@ -31,10 +31,6 @@ impl PartialEq for ProtocolMsg { serde_cbor::to_vec(self).expect("ProtocolMsg serializes. qed.") == serde_cbor::to_vec(other).expect("ProtocolMsg serializes. qed.") } - - fn ne(&self, other: &Self) -> bool { - !self.eq(other) - } } #[derive(PartialEq, Debug, Clone, serde::Serialize, serde::Deserialize)] diff --git a/qos-core/src/protocol/provisioner.rs b/qos-core/src/protocol/provisioner.rs index 923c0bb6..7e9b8a83 100644 --- a/qos-core/src/protocol/provisioner.rs +++ b/qos-core/src/protocol/provisioner.rs @@ -19,7 +19,7 @@ impl SecretProvisioner { } pub fn add_share(&mut self, share: Share) -> Result<(), ProtocolError> { - if share.len() == 0 { + if share.is_empty() { return Err(ProtocolError::InvalidShare); } @@ -31,7 +31,7 @@ impl SecretProvisioner { let secret = qos_crypto::shares_reconstruct(&self.shares); // TODO: Add better validation... - if secret.len() == 0 { + if secret.is_empty() { return Err(ProtocolError::ReconstructionError); } diff --git a/qos-core/src/server.rs b/qos-core/src/server.rs index 64254eea..60a2b52a 100644 --- a/qos-core/src/server.rs +++ b/qos-core/src/server.rs @@ -28,9 +28,9 @@ impl SocketServer { ) -> Result<(), SocketServerError> { println!("SocketServer listening on {:?}", addr); - let mut listener = Listener::listen(addr)?; + let listener = Listener::listen(addr)?; - while let Some(stream) = listener.next() { + for stream in listener { match stream.recv() { Ok(payload) => { let response = processor.process(payload); diff --git a/qos-crypto/src/shamir.rs b/qos-crypto/src/shamir.rs index efae2e24..264d8965 100644 --- a/qos-crypto/src/shamir.rs +++ b/qos-crypto/src/shamir.rs @@ -165,8 +165,8 @@ pub fn shares_generate(secret: &[u8], n: usize, k: usize) -> Vec> { // we need to store x for each point somewhere, so just prepend // each array with it - for i in 0..n { - shares[i].push(u8::try_from(i + 1).expect("exceeded 255 shares")); + for (i, share) in shares.iter_mut().enumerate().take(n) { + share.push(u8::try_from(i + 1).expect("exceeded 255 shares")); } for x in secret { @@ -174,8 +174,8 @@ pub fn shares_generate(secret: &[u8], n: usize, k: usize) -> Vec> { let f = gf256_generate(*x, k - 1); // assign each share a point at f(i) - for i in 0..n { - shares[i].push(gf256_eval( + for (i, share) in shares.iter_mut().enumerate().take(n) { + share.push(gf256_eval( &f, u8::try_from(i + 1).expect("exceeeded 255 shares"), )); diff --git a/qos-host/src/cli.rs b/qos-host/src/cli.rs index 582c028c..18c28eed 100644 --- a/qos-host/src/cli.rs +++ b/qos-host/src/cli.rs @@ -7,7 +7,7 @@ use regex::Regex; use crate::HostServer; -const IP_REGEX: &'static str = r"^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$"; +const IP_REGEX: &str = r"^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$"; #[derive(Clone, Debug, PartialEq)] pub struct HostServerOptions { @@ -21,7 +21,7 @@ impl HostServerOptions { } } -#[derive(Clone, Debug, PartialEq)] +#[derive(Default, Clone, Debug, PartialEq)] pub struct HostOptions { ip: Option<[u8; 4]>, port: Option, @@ -29,7 +29,7 @@ pub struct HostOptions { impl HostOptions { pub fn new() -> Self { - Self { ip: None, port: None } + Default::default() } pub fn url(&self) -> String { @@ -54,42 +54,36 @@ impl HostOptions { } pub fn parse_ip(&mut self, cmd: &str, arg: &str) { - match cmd { - "--host-ip" => { - let re = Regex::new(IP_REGEX) - .expect("Could not parse value from `--host-ip`"); - let mut iter = re.captures_iter(arg); - - let parse = |string: &str| { - string - .to_string() - .parse::() - .expect("Could not parse value from `--host-ip`") - }; - - if let Some(cap) = iter.next() { - let ip1 = parse(&cap[1]); - let ip2 = parse(&cap[2]); - let ip3 = parse(&cap[3]); - let ip4 = parse(&cap[4]); - self.ip = Some([ip1, ip2, ip3, ip4]); - } + if cmd == "--host-ip" { + let re = Regex::new(IP_REGEX) + .expect("Could not parse value from `--host-ip`"); + let mut iter = re.captures_iter(arg); + + let parse = |string: &str| { + string + .to_string() + .parse::() + .expect("Could not parse value from `--host-ip`") + }; + + if let Some(cap) = iter.next() { + let ip1 = parse(&cap[1]); + let ip2 = parse(&cap[2]); + let ip3 = parse(&cap[3]); + let ip4 = parse(&cap[4]); + self.ip = Some([ip1, ip2, ip3, ip4]); } - _ => {} } } pub fn parse_port(&mut self, cmd: &str, arg: &str) { - match cmd { - "--host-port" => { - self.port = arg - .parse::() - .map_err(|_| { - panic!("Could not parse provided value for `--port`") - }) - .ok(); - } - _ => {} + if cmd == "--host-port" { + self.port = arg + .parse::() + .map_err(|_| { + panic!("Could not parse provided value for `--port`") + }) + .ok(); } } } @@ -124,7 +118,7 @@ pub fn parse_args(args: Vec) -> HostServerOptions { let mut options = HostServerOptions::new(); let mut chunks = args.chunks_exact(2); - if chunks.remainder().len() > 0 { + if !chunks.remainder().is_empty() { panic!("Unexepected number of arguments") } while let Some([cmd, arg]) = chunks.next() { diff --git a/qos-host/src/lib.rs b/qos-host/src/lib.rs index 6f141e13..2b8ccf5d 100644 --- a/qos-host/src/lib.rs +++ b/qos-host/src/lib.rs @@ -98,13 +98,11 @@ impl HostServer { } match serde_cbor::from_slice(&body) { - Err(_) => { - return ( - StatusCode::BAD_REQUEST, - serde_cbor::to_vec(&ProtocolMsg::ErrorResponse) - .expect("ProtocolMsg can always serialize. qed."), - ) - } + Err(_) => ( + StatusCode::BAD_REQUEST, + serde_cbor::to_vec(&ProtocolMsg::ErrorResponse) + .expect("ProtocolMsg can always serialize. qed."), + ), Ok(request) => match state.enclave_client.send(request) { Err(_) => ( StatusCode::INTERNAL_SERVER_ERROR, diff --git a/qos-test/tests/end_to_end.rs b/qos-test/tests/end_to_end.rs index 8ceadf78..707c8dbe 100644 --- a/qos-test/tests/end_to_end.rs +++ b/qos-test/tests/end_to_end.rs @@ -1,12 +1,9 @@ -use std::{collections::BTreeSet, fs::File, io::Read, path::Path}; +use std::{fs::File, io::Read, path::Path}; use qos_client; use qos_core::{ io::SocketAddress, - protocol::{ - Echo, Executor, MockNsm, NsmDigest, NsmRequest, NsmResponse, - ProtocolMsg, ProvisionRequest, - }, + protocol::{Echo, Executor, MockNsm, ProtocolMsg, ProvisionRequest}, server::SocketServer, }; use qos_crypto; From acf10b455d540c6129b97ca91e9518959a42ec42 Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Sun, 22 May 2022 22:09:40 -0700 Subject: [PATCH 06/25] fmt --- qos-client/src/cli.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/qos-client/src/cli.rs b/qos-client/src/cli.rs index 93793839..388e728f 100644 --- a/qos-client/src/cli.rs +++ b/qos-client/src/cli.rs @@ -29,7 +29,7 @@ impl Command { } } -impl From<&str > for Command { +impl From<&str> for Command { fn from(s: &str) -> Command { match s { "health" => Command::Health, @@ -42,7 +42,6 @@ impl From<&str > for Command { } } - #[derive(Clone, PartialEq, Debug)] struct ClientOptions { cmd: Command, From 576b3b8fb6d20aa327416c41e50849d13af74bae Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Sun, 22 May 2022 22:14:37 -0700 Subject: [PATCH 07/25] newlines --- .github/workflows/audit.yml | 2 +- .github/workflows/pr.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/audit.yml b/.github/workflows/audit.yml index 3cd17528..ca2daec5 100644 --- a/.github/workflows/audit.yml +++ b/.github/workflows/audit.yml @@ -19,4 +19,4 @@ jobs: - uses: actions/checkout@v1 - uses: actions-rs/audit-check@v1 with: - token: ${{ secrets.GITHUB_TOKEN }} \ No newline at end of file + token: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index ab293ae1..dd477466 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -71,4 +71,4 @@ jobs: uses: actions-rs/cargo@v1 with: command: clippy - args: -- -D warnings \ No newline at end of file + args: -- -D warnings From 5e9325ed1763fb05e2baeae611dc15dea37dcd27 Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Sun, 22 May 2022 22:15:54 -0700 Subject: [PATCH 08/25] Try adding cross target builds --- .github/workflows/pr.yml | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index dd477466..19cb86f1 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -6,26 +6,26 @@ on: [pull_request] name: PR jobs: - # build: - # name: Build - # runs-on: ubuntu-latest - # strategy: - # matrix: - # target: - # - ubuntu-latest - # - macOS-latest - # steps: - # - uses: actions/checkout@v2 - # - uses: actions-rs/toolchain@v1 - # with: - # toolchain: stable - # target: ${{ matrix.target }} - # override: true - # - uses: actions-rs/cargo@v1 - # with: - # use-cross: true - # command: build - # args: --release --target=${{ matrix.target }} + build: + name: Build + runs-on: ubuntu-latest + strategy: + matrix: + target: + - ubuntu-latest + - macOS-latest + steps: + - uses: actions/checkout@v2 + - uses: actions-rs/toolchain@v1 + with: + toolchain: stable + target: ${{ matrix.target }} + override: true + - uses: actions-rs/cargo@v1 + with: + use-cross: true + command: build + args: --release --target=${{ matrix.target }} test: name: Test Suite From abefa7d2d7a5ac46da6b2483c7faf0adf7a38c16 Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Sun, 22 May 2022 22:24:24 -0700 Subject: [PATCH 09/25] Try cross target again --- .github/workflows/pr.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 19cb86f1..0240ce44 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -25,7 +25,6 @@ jobs: with: use-cross: true command: build - args: --release --target=${{ matrix.target }} test: name: Test Suite From 42848f3eed42793bc467342e51f7184a99bda9ac Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Sun, 22 May 2022 22:29:07 -0700 Subject: [PATCH 10/25] Try runs on target os --- .github/workflows/pr.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 0240ce44..e30ca9de 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -8,7 +8,7 @@ name: PR jobs: build: name: Build - runs-on: ubuntu-latest + runs-on: ${{ matrix.target }} strategy: matrix: target: @@ -25,6 +25,7 @@ jobs: with: use-cross: true command: build + args: --target=${{ matrix.target }} test: name: Test Suite From a5694093c5265eb6d024e5ae1563e43cf2b7ccfe Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Sun, 22 May 2022 22:31:41 -0700 Subject: [PATCH 11/25] Remove build --- .github/workflows/pr.yml | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index e30ca9de..5a5480e4 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -6,27 +6,6 @@ on: [pull_request] name: PR jobs: - build: - name: Build - runs-on: ${{ matrix.target }} - strategy: - matrix: - target: - - ubuntu-latest - - macOS-latest - steps: - - uses: actions/checkout@v2 - - uses: actions-rs/toolchain@v1 - with: - toolchain: stable - target: ${{ matrix.target }} - override: true - - uses: actions-rs/cargo@v1 - with: - use-cross: true - command: build - args: --target=${{ matrix.target }} - test: name: Test Suite runs-on: ubuntu-latest From b9d5c505bca4969a5d4eda4362872c5083b75022 Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Mon, 23 May 2022 10:00:28 -0700 Subject: [PATCH 12/25] fmt --- qos-client/src/attest/nitro/mod.rs | 2 +- qos-core/src/protocol/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/qos-client/src/attest/nitro/mod.rs b/qos-client/src/attest/nitro/mod.rs index f40d19a3..184fedcd 100644 --- a/qos-client/src/attest/nitro/mod.rs +++ b/qos-client/src/attest/nitro/mod.rs @@ -135,7 +135,7 @@ fn verify_cose_sign1_sig( // Expect v3 if ee_cert.version() != X509_V3 { - return Err(AttestError::InvalidEndEntityCert) + return Err(AttestError::InvalidEndEntityCert); } let ee_cert_pub_key = ee_cert.public_key()?; diff --git a/qos-core/src/protocol/mod.rs b/qos-core/src/protocol/mod.rs index 166e7122..f047a8f2 100644 --- a/qos-core/src/protocol/mod.rs +++ b/qos-core/src/protocol/mod.rs @@ -55,7 +55,7 @@ impl server::Routable for Executor { fn process(&mut self, req_bytes: Vec) -> Vec { if req_bytes.len() > MAX_ENCODED_MSG_LEN { return serde_cbor::to_vec(&ProtocolMsg::ErrorResponse) - .expect("ProtocolMsg can always be serialized. qed.") + .expect("ProtocolMsg can always be serialized. qed."); } let msg_req = match serde_cbor::from_slice(&req_bytes) { From 698d39d54d0894bedd672aeb04f4bc1c619ac28d Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Tue, 7 Jun 2022 10:03:34 -0700 Subject: [PATCH 13/25] FMT --- qos-core/src/coordinator.rs | 12 +++++++----- qos-test/tests/coordinator.rs | 10 ++++++---- rustfmt.toml | 1 - 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/qos-core/src/coordinator.rs b/qos-core/src/coordinator.rs index 0ea1f686..1f691561 100644 --- a/qos-core/src/coordinator.rs +++ b/qos-core/src/coordinator.rs @@ -30,7 +30,7 @@ impl Coordinator { let pivot_file_exists = is_file(&pivot_file); if secret_file_exists && pivot_file_exists { - break + break; } std::thread::sleep(std::time::Duration::from_secs(1)); @@ -43,14 +43,16 @@ impl Coordinator { pivot.spawn().expect("Process failed to execute..."); // Wait for the child process to finish - let status = child_process - .wait() - .expect("Pivot executable never started..."); + let status = + child_process.wait().expect("Pivot executable never started..."); // and log some information about the exit status if status.success() { println!("Pivot executable exited successfully: {}", status); } else { - println!("Pivot executable exited with a non zero status: {}", status) + println!( + "Pivot executable exited with a non zero status: {}", + status + ) } println!("Coordinator exiting ..."); diff --git a/qos-test/tests/coordinator.rs b/qos-test/tests/coordinator.rs index 12c6fd67..0ab9a874 100644 --- a/qos-test/tests/coordinator.rs +++ b/qos-test/tests/coordinator.rs @@ -117,12 +117,13 @@ async fn coordinator_e2e() { fn coordinator_works() { let secret_path = "./coordinator_exits_cleanly_with_non_panicking_executable.secret"; - let usock = "./coordinator_exits_cleanly_with_non_panicking_executable.sock"; + let usock = + "./coordinator_exits_cleanly_with_non_panicking_executable.sock"; // For our sanity, ensure the secret does not yet exist let _ = std::fs::remove_file(secret_path); assert!(File::open(PIVOT_OK_PATH).is_ok(),); - let opts: Vec<_> = [ + let opts: Vec<_> = [ "--usock", usock, "--mock", @@ -162,7 +163,8 @@ fn coordinator_works() { fn coordinator_handles_non_zero_exits() { let secret_path = "./coordinator_keeps_re_spawning_pivot_executable_that_panics.secret"; - let usock = "./coordinator_keeps_re_spawning_pivot_executable_that_panics.sock"; + let usock = + "./coordinator_keeps_re_spawning_pivot_executable_that_panics.sock"; // For our sanity, ensure the secret does not yet exist let _ = std::fs::remove_file(secret_path); assert!(File::open(PIVOT_ABORT_PATH).is_ok(),); @@ -213,7 +215,7 @@ fn coordinator_handles_panic() { let _ = std::fs::remove_file(secret_path); assert!(File::open(PIVOT_PANIC_PATH).is_ok(),); - let opts: Vec<_> = [ + let opts: Vec<_> = [ "--usock", usock, "--mock", diff --git a/rustfmt.toml b/rustfmt.toml index 55bbfe73..473873ae 100644 --- a/rustfmt.toml +++ b/rustfmt.toml @@ -1,4 +1,3 @@ -# Let there be async! edition = "2021" # Tabs From 2628fdc84ef9a156be6e0a6bc46b95fba9263aa4 Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Tue, 7 Jun 2022 14:46:26 -0700 Subject: [PATCH 14/25] Remove security audit --- .github/workflows/audit.yml | 22 ---------------------- 1 file changed, 22 deletions(-) delete mode 100644 .github/workflows/audit.yml diff --git a/.github/workflows/audit.yml b/.github/workflows/audit.yml deleted file mode 100644 index ca2daec5..00000000 --- a/.github/workflows/audit.yml +++ /dev/null @@ -1,22 +0,0 @@ -# Perform a audit any time a change is dependencies is detected in Cargo.toml or Cargo.lock -# See: https://github.com/actions-rs/audit-check - -name: Security audit -on: - push: - paths: - - '**/Cargo.toml' - - '**/Cargo.lock' - pull_request: - paths: - - '**/Cargo.toml' - - '**/Cargo.lock' - -jobs: - security_audit: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v1 - - uses: actions-rs/audit-check@v1 - with: - token: ${{ secrets.GITHUB_TOKEN }} From 666f7e172f886ce0a7243fc2fe135525b70708a8 Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Tue, 14 Jun 2022 18:40:53 -0700 Subject: [PATCH 15/25] Update the readme --- .github/workflows/pr.yml | 2 +- README.md | 52 +++++++++---------- qos-client/src/attest/nitro/mod.rs | 4 +- .../src/attest/nitro/syntactic_validation.rs | 20 +++---- qos-client/src/cli.rs | 12 ++--- qos-core/src/io/stream.rs | 2 +- qos-core/src/protocol/attestor/types.rs | 2 +- qos-core/src/protocol/boot.rs | 45 +++++++--------- qos-core/src/protocol/mod.rs | 4 +- qos-crypto/src/lib.rs | 2 +- qos-host/src/cli.rs | 2 +- qos-test/tests/load.rs | 15 ++---- qos-test/tests/provision.rs | 6 +-- rustfmt.toml | 6 +-- 14 files changed, 77 insertions(+), 97 deletions(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 5a5480e4..065a20ae 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -36,7 +36,7 @@ jobs: uses: actions-rs/toolchain@v1 with: profile: minimal - toolchain: stable + toolchain: nightly override: true components: rustfmt, clippy diff --git a/README.md b/README.md index 6d649e92..7eceab98 100644 --- a/README.md +++ b/README.md @@ -1,47 +1,47 @@ -Quick start +## Submitting a PR +Before a PR can be merged it must: + +Be formatted + +```bash +cargo +nightly +``` + +Pass the linter + +```bash +cargo clippy + +# to fix some types of lints you can run +cargo clippy --fix ``` -# make sure you have the latest rustc stable -rustup update stable -# run tests -cargo test --all +And pass all tests -# format code -cargo +nightly fmt +```bash +cargo test ``` -# System requirements +## System requirements - openssl >= 1.1.0 -# Key parts +## Key parts -## Enclave +### Enclave - houses nitro server +- see `qos-core` ## Host - EC2 instance where the nitro enclave lives inside - has client for talking to nitro enclave - has server for incoming request from outside world +- see `qos-host` ## End user -- Anything making request to host - -# Decisions / Things to Revisit: - -- Use Serde in `qos-core`. We've decided to do this right now for agility; but we should probably make our own simple macro or find a secure serialization lib (look into borsch?) - -# TODO: - -- Build crypto - all public key + hashing logic. High level so we can swap. Bring in OpenSSL -- Pivot logic -- Cli for posting shards, nsm attestation flow -- Research flow for attestation - with nsm / nitro enclave docs - -- Sanity check vsock - aws or qemu -- Run deployed aws attestation flow (save nsm responses for stubbing) -- Smart shamir logic in enclave, don't randomly reconstruct \ No newline at end of file +- anything making request to host +- see `qos-client` diff --git a/qos-client/src/attest/nitro/mod.rs b/qos-client/src/attest/nitro/mod.rs index 811cf0d9..de7c0a2e 100644 --- a/qos-client/src/attest/nitro/mod.rs +++ b/qos-client/src/attest/nitro/mod.rs @@ -29,7 +29,7 @@ pub const MOCK_SECONDS_SINCE_EPOCH: u64 = 1652756400; /// `8cf60e2b2efca96c6a9e71e851d00c1b6991cc09eadbe64a6a1d1b1eb9faff7c`. This /// checksum and the certificate should be manually verified against /// https://docs.aws.amazon.com/enclaves/latest/user/verify-root.html. -pub const AWS_ROOT_CERT_PEM: &'static [u8] = +pub const AWS_ROOT_CERT_PEM: &[u8] = std::include_bytes!("./static/aws_root_cert.pem"); /// Extract a DER encoded certificate from bytes representing a PEM encoded @@ -290,7 +290,7 @@ mod test { } { - let valid = attestation_doc.clone(); + let valid = attestation_doc; // Don't pop anything, just want to sanity check that we get a // corrupt signature on the cose sign1 structure. diff --git a/qos-client/src/attest/nitro/syntactic_validation.rs b/qos-client/src/attest/nitro/syntactic_validation.rs index b15d71c7..abbb5c67 100644 --- a/qos-client/src/attest/nitro/syntactic_validation.rs +++ b/qos-client/src/attest/nitro/syntactic_validation.rs @@ -149,15 +149,15 @@ mod test { #[test] fn cabundle_works() { let valid_cert = ByteBuf::from(vec![42]); - assert!(cabundle(&vec![valid_cert]).is_ok()); + assert!(cabundle(&[valid_cert]).is_ok()); - assert!(cabundle(&vec![]).is_err()); + assert!(cabundle(&[]).is_err()); let short_cert = ByteBuf::new(); - assert!(cabundle(&vec![short_cert]).is_err()); + assert!(cabundle(&[short_cert]).is_err()); let long_cert = ByteBuf::from((0..1025).map(|_| 3).collect::>()); - assert!(cabundle(&vec![long_cert]).is_err()); + assert!(cabundle(&[long_cert]).is_err()); } #[test] @@ -182,19 +182,15 @@ mod test { assert!(pcrs(&BTreeMap::from([(33, pcr32.clone())])).is_err()); // Valid - assert!(pcrs(&BTreeMap::from([ - (0, pcr48.clone()), - (32, pcr32.clone()), - (5, pcr64.clone()) - ])) - .is_ok()); + assert!(pcrs(&BTreeMap::from([(0, pcr48), (32, pcr32), (5, pcr64)])) + .is_ok()); assert!(pcrs(&BTreeMap::from([(5, pcr_invalid)])).is_err()); } #[test] fn module_id_works() { - assert!(module_id(&"".to_string()).is_err()); - assert!(module_id(&"1".to_string()).is_ok()); + assert!(module_id("").is_err()); + assert!(module_id("1").is_ok()); } } diff --git a/qos-client/src/cli.rs b/qos-client/src/cli.rs index 6dcd7001..09298d05 100644 --- a/qos-client/src/cli.rs +++ b/qos-client/src/cli.rs @@ -76,15 +76,15 @@ impl ClientOptions { while let Some([cmd, arg]) = chunks.next() { options.host.parse(cmd, arg); match options.cmd { - Command::Echo => options.echo.parse(&cmd, arg), + Command::Echo => options.echo.parse(cmd, arg), Command::GenerateSetupKey => { - options.generate_setup_key.parse(&cmd, arg) + options.generate_setup_key.parse(cmd, arg) } Command::Health => {} Command::DescribeNsm => {} Command::MockAttestation => {} Command::Attestation => {} - Command::BootGenesis => options.boot_genesis.parse(&cmd, arg), + Command::BootGenesis => options.boot_genesis.parse(cmd, arg), } } @@ -186,7 +186,7 @@ impl BootGenesisOptions { self.key_dir.clone().expect("No `--key-dir` provided") } fn threshold(&self) -> u32 { - self.threshold.clone().expect("No `--threshold` provided") + self.threshold.expect("No `--threshold` provided") } } @@ -472,13 +472,13 @@ mod handlers { .filter_map(|key_path| { let file_name = key_path.file_name().map(|f| f.to_string_lossy()).unwrap(); - let split: Vec<_> = file_name.split(".").collect(); + let split: Vec<_> = file_name.split('.').collect(); // TODO: do we want to dissallow having anything in this folder // that is not a public key for the quorum set? if *split.last().unwrap() != "pub" { println!("A non `.pub` file was found in the setup key directory - skipping."); - return None + return None; } let public_key = RsaPub::from_pem_file(key_path.clone()) diff --git a/qos-core/src/io/stream.rs b/qos-core/src/io/stream.rs index d6ddc1c9..8f20c8a9 100644 --- a/qos-core/src/io/stream.rs +++ b/qos-core/src/io/stream.rs @@ -300,7 +300,7 @@ mod test { let mut listener = Listener::listen(addr.clone()).unwrap(); let handler = std::thread::spawn(move || { - while let Some(stream) = listener.next() { + for stream in listener.by_ref() { let req = stream.recv().unwrap(); stream.send(&req).unwrap(); break; diff --git a/qos-core/src/protocol/attestor/types.rs b/qos-core/src/protocol/attestor/types.rs index 5e7bfd97..6809d369 100644 --- a/qos-core/src/protocol/attestor/types.rs +++ b/qos-core/src/protocol/attestor/types.rs @@ -312,7 +312,7 @@ impl From for nsm::api::Response { R::Attestation { document } => Self::Attestation { document }, R::GetRandom { random } => Self::GetRandom { random }, R::Error(e) => Self::Error(e.into()), - _ => Self::Error(ErrorCode::InternalError.into()), + _ => Self::Error(ErrorCode::InternalError), } } } diff --git a/qos-core/src/protocol/boot.rs b/qos-core/src/protocol/boot.rs index 38c64d89..4946d697 100644 --- a/qos-core/src/protocol/boot.rs +++ b/qos-core/src/protocol/boot.rs @@ -107,12 +107,12 @@ impl ManifestEnvelope { if !is_valid_signature { return Err(ProtocolError::InvalidManifestApproval( approval.clone(), - )) + )); } } if self.approvals.len() < self.manifest.quorum_set.threshold as usize { - return Err(ProtocolError::NotEnoughApprovals) + return Err(ProtocolError::NotEnoughApprovals); } Ok(()) @@ -135,7 +135,7 @@ pub fn boot_standard( )?; if sha_256(pivot) != manifest_envelope.manifest.pivot.hash { - return Err(ProtocolError::InvalidPivotHash) + return Err(ProtocolError::InvalidPivotHash); }; std::fs::write(&state.pivot_file, pivot)?; @@ -215,10 +215,7 @@ mod test { restart: RestartPolicy::Always, }, quorum_key: quorum_pair.public_key_to_der().unwrap(), - quorum_set: QuorumSet { - threshold: 2, - members: quorum_members.clone(), - }, + quorum_set: QuorumSet { threshold: 2, members: quorum_members }, }; (manifest, member_with_keys, pivot) @@ -241,11 +238,9 @@ mod test { let manifest_hash = manifest.hash(); let approvals = members .into_iter() - .map(|(pair, member)| { - return Approval { - signature: pair.sign_sha256(&manifest_hash).unwrap(), - member: member.clone(), - } + .map(|(pair, member)| Approval { + signature: pair.sign_sha256(&manifest_hash).unwrap(), + member, }) .collect(); @@ -284,12 +279,10 @@ mod test { let manifest_hash = manifest.hash(); let approvals = members [0usize..manifest.quorum_set.threshold as usize - 1] - .into_iter() - .map(|(pair, member)| { - return Approval { - signature: pair.sign_sha256(&manifest_hash).unwrap(), - member: member.clone(), - } + .iter() + .map(|(pair, member)| Approval { + signature: pair.sign_sha256(&manifest_hash).unwrap(), + member: member.clone(), }) .collect(); @@ -301,8 +294,8 @@ mod test { let mut protocol_state = ProtocolState::new( Box::new(MockNsm), "secret".to_string(), - pivot_file.clone(), - ephemeral_file.clone(), + pivot_file, + ephemeral_file, ); let nsm_resposne = @@ -318,11 +311,9 @@ mod test { let manifest_envelope = { let approvals = members .into_iter() - .map(|(_pair, member)| { - return Approval { - signature: vec![0, 0], - member: member.clone(), - } + .map(|(_pair, member)| Approval { + signature: vec![0, 0], + member, }) .collect(); @@ -334,8 +325,8 @@ mod test { let mut protocol_state = ProtocolState::new( Box::new(MockNsm), "secret".to_string(), - pivot_file.clone(), - ephemeral_file.clone(), + pivot_file, + ephemeral_file, ); let nsm_resposne = diff --git a/qos-core/src/protocol/mod.rs b/qos-core/src/protocol/mod.rs index b54efac4..fa48eed8 100644 --- a/qos-core/src/protocol/mod.rs +++ b/qos-core/src/protocol/mod.rs @@ -157,7 +157,7 @@ impl server::Routable for Executor { }; if req_bytes.len() > MAX_ENCODED_MSG_LEN { - return err_resp() + return err_resp(); } let msg_req = match ProtocolMsg::try_from_slice(&req_bytes) { @@ -326,7 +326,7 @@ mod handlers { Ok(r) => r, Err(e) => { state.phase = ProtocolPhase::UnrecoverableError; - return Some(ProtocolMsg::ProtocolErrorResponse(e)) + return Some(ProtocolMsg::ProtocolErrorResponse(e)); } } }; diff --git a/qos-crypto/src/lib.rs b/qos-crypto/src/lib.rs index 0dce607a..dbb8945f 100644 --- a/qos-crypto/src/lib.rs +++ b/qos-crypto/src/lib.rs @@ -211,7 +211,7 @@ impl RsaPub { let public_key_size = self.public_key.size() as usize; // TODO: WTF? if data.len() > public_key_size - 42 { - return Err(CryptoError::EncryptionPayloadTooBig) + return Err(CryptoError::EncryptionPayloadTooBig); } let mut to = vec![0; public_key_size]; diff --git a/qos-host/src/cli.rs b/qos-host/src/cli.rs index 18c28eed..cd6c70d1 100644 --- a/qos-host/src/cli.rs +++ b/qos-host/src/cli.rs @@ -157,7 +157,7 @@ mod test { fn expect_ip(arg: &str, expected: [u8; 4]) { let mut options = HostOptions::new(); - options.parse_ip(&"--host-ip", arg); + options.parse_ip("--host-ip", arg); if let Some(ip) = options.ip { assert_eq!(ip[0], expected[0]); diff --git a/qos-test/tests/load.rs b/qos-test/tests/load.rs index cbc90b60..62db739e 100644 --- a/qos-test/tests/load.rs +++ b/qos-test/tests/load.rs @@ -15,7 +15,7 @@ fn load_e2e() { let make_path = |n: usize| { let mut path = base_path.clone(); - path.push(format!("rsa_public_{}.mock.pem", n.to_string())); + path.push(format!("rsa_public_{}.mock.pem", n)); path }; @@ -24,13 +24,8 @@ fn load_e2e() { // let executable = b"vape nation".to_vec(); let key_range = 0..5; - let pairs: Vec<_> = key_range - .clone() - .map(|_| { - let pair = RsaPair::generate().unwrap(); - pair - }) - .collect(); + let pairs: Vec<_> = + key_range.map(|_| RsaPair::generate().unwrap()).collect(); let paths: Vec<_> = pairs .iter() @@ -71,8 +66,8 @@ fn load_e2e() { // Spawn enclave let pivot_file2 = pivot_file.clone(); - let secret_file2 = secret_file.clone(); - let ephemeral_file2 = ephemeral_file.clone(); + let secret_file2 = secret_file; + let ephemeral_file2 = ephemeral_file; std::thread::spawn(move || { let attestor = MockNsm {}; let executor = Executor::new( diff --git a/qos-test/tests/provision.rs b/qos-test/tests/provision.rs index f6ee6a80..7fa918b0 100644 --- a/qos-test/tests/provision.rs +++ b/qos-test/tests/provision.rs @@ -1,12 +1,10 @@ use std::{fs::File, io::Read, path::Path}; -use qos_client; use qos_core::{ io::SocketAddress, protocol::{Echo, Executor, MockNsm, ProtocolMsg, Provision}, server::SocketServer, }; -use qos_crypto; use qos_host::HostServer; #[tokio::test] @@ -26,9 +24,9 @@ async fn provision_e2e() { let ephemeral_file = "./end-to-end.ephemeral".to_string(); // Spawn enclave - let pivot_file2 = pivot_file.clone(); + let pivot_file2 = pivot_file; let secret_file2 = secret_file.clone(); - let ephemeral_file2 = ephemeral_file.clone(); + let ephemeral_file2 = ephemeral_file; std::thread::spawn(move || { let attestor = MockNsm {}; let executor = Executor::new( diff --git a/rustfmt.toml b/rustfmt.toml index 473873ae..a416aea0 100644 --- a/rustfmt.toml +++ b/rustfmt.toml @@ -6,13 +6,13 @@ tab_spaces = 4 # Line wrapping use_small_heuristics = "Max" -# wrap_comments = true +wrap_comments = true max_width = 80 # Imports -# imports_granularity = "Crate" +imports_granularity = "Crate" reorder_imports = true -# group_imports = "StdExternalCrate" +group_imports = "StdExternalCrate" # Random use_field_init_shorthand = true From 1b2c80d6f1ad33c434745cd3418dc3c89d3017e4 Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Tue, 14 Jun 2022 18:53:51 -0700 Subject: [PATCH 16/25] Don't use nightly for clippy --- .github/workflows/pr.yml | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 065a20ae..3a538b63 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -25,8 +25,8 @@ jobs: with: command: test - lints: - name: Lints + rustfmt: + name: rustfmt runs-on: ubuntu-latest steps: - name: Checkout sources @@ -46,8 +46,23 @@ jobs: command: fmt args: --all -- --check + clippy: + name: clippy + runs-on: ubuntu-latest + steps: + - name: Checkout sources + uses: actions/checkout@v2 + + - name: Install stable toolchain + uses: actions-rs/toolchain@v1 + with: + profile: minimal + toolchain: stable + override: true + components: rustfmt, clippy + - name: Run cargo clippy uses: actions-rs/cargo@v1 with: command: clippy - args: -- -D warnings + args: --all-targets --all-features -- -D warnings From c42d16b55c06cd19104f4f24431928c65422f687 Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Tue, 14 Jun 2022 18:55:56 -0700 Subject: [PATCH 17/25] Some changes to stream --- qos-core/src/io/stream.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/qos-core/src/io/stream.rs b/qos-core/src/io/stream.rs index 8f20c8a9..4a44adf4 100644 --- a/qos-core/src/io/stream.rs +++ b/qos-core/src/io/stream.rs @@ -297,10 +297,10 @@ mod test { .unwrap(); let addr = SocketAddress::Unix(unix_addr); - let mut listener = Listener::listen(addr.clone()).unwrap(); + let listener = Listener::listen(addr.clone()).unwrap(); let handler = std::thread::spawn(move || { - for stream in listener.by_ref() { + for stream in listener { let req = stream.recv().unwrap(); stream.send(&req).unwrap(); break; @@ -312,7 +312,6 @@ mod test { let data = vec![1, 2, 3, 4, 5, 6, 6, 6]; let _ = client.send(&data).unwrap(); let resp = client.recv().unwrap(); - assert_eq!(data, resp); handler.join().unwrap(); From 035630c0750b60504ebecda406805ad0119427b8 Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Tue, 14 Jun 2022 22:22:56 -0700 Subject: [PATCH 18/25] Add missing_docs to some crates --- .github/workflows/pr.yml | 2 +- README.md | 4 ++ qos-core/src/cli.rs | 21 ++++++--- qos-core/src/client.rs | 4 ++ qos-core/src/coordinator.rs | 2 + qos-core/src/io/mod.rs | 4 ++ qos-core/src/io/stream.rs | 14 ++++-- qos-core/src/lib.rs | 25 +++++++++-- qos-core/src/protocol/attestor/mock.rs | 1 + qos-core/src/protocol/attestor/mod.rs | 15 +++++++ qos-core/src/protocol/attestor/types.rs | 9 +++- qos-core/src/protocol/boot.rs | 41 +++++++++++++++-- qos-core/src/protocol/genesis.rs | 19 ++++++-- qos-core/src/protocol/mod.rs | 49 ++++++++++++++++----- qos-core/src/protocol/msg.rs | 58 ++++++++++++++++++++----- qos-core/src/protocol/provisioner.rs | 8 +++- qos-core/src/server.rs | 14 ++++-- qos-crypto/src/lib.rs | 39 +++++++++++++---- qos-crypto/src/shamir.rs | 15 +++---- qos-test/src/bin/core_cli.rs | 2 +- qos-test/src/lib.rs | 1 + qos-test/tests/load.rs | 2 +- 22 files changed, 284 insertions(+), 65 deletions(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 3a538b63..00a6f7c1 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -65,4 +65,4 @@ jobs: uses: actions-rs/cargo@v1 with: command: clippy - args: --all-targets --all-features -- -D warnings + args: --all-targets -- -D warnings diff --git a/README.md b/README.md index 7eceab98..412e883a 100644 --- a/README.md +++ b/README.md @@ -1,3 +1,7 @@ +# QuorumOS + +This is a WIP. + ## Submitting a PR Before a PR can be merged it must: diff --git a/qos-core/src/cli.rs b/qos-core/src/cli.rs index 24443e7e..544b9078 100644 --- a/qos-core/src/cli.rs +++ b/qos-core/src/cli.rs @@ -7,6 +7,7 @@ use crate::{ EPHEMERAL_KEY_FILE, PIVOT_FILE, SECRET_FILE, }; +/// CLI options for starting up the enclave server. #[derive(Default, Clone, Debug, PartialEq)] pub struct EnclaveOptions { cid: Option, @@ -19,6 +20,7 @@ pub struct EnclaveOptions { } impl EnclaveOptions { + /// Create a new instance of [`Self`] with some defaults. pub fn new() -> Self { Self { cid: None, @@ -46,6 +48,7 @@ impl EnclaveOptions { options } + /// Parse a set of command and argument. pub fn parse(&mut self, cmd: &str, arg: &str) { self.parse_cid(cmd, arg); self.parse_port(cmd, arg); @@ -56,7 +59,7 @@ impl EnclaveOptions { self.parse_ephemeral_key_file(cmd, arg); } - pub fn parse_cid(&mut self, cmd: &str, arg: &str) { + fn parse_cid(&mut self, cmd: &str, arg: &str) { if cmd == "--cid" { self.cid = arg .parse::() @@ -67,7 +70,7 @@ impl EnclaveOptions { } } - pub fn parse_port(&mut self, cmd: &str, arg: &str) { + fn parse_port(&mut self, cmd: &str, arg: &str) { if cmd == "--port" { self.port = arg .parse::() @@ -78,36 +81,37 @@ impl EnclaveOptions { } } - pub fn parse_usock(&mut self, cmd: &str, arg: &str) { + fn parse_usock(&mut self, cmd: &str, arg: &str) { if cmd == "--usock" { self.usock = Some(arg.to_string()) } } - pub fn parse_mock(&mut self, cmd: &str, arg: &str) { + fn parse_mock(&mut self, cmd: &str, arg: &str) { if cmd == "--mock" { self.mock = arg == "true" }; } - pub fn parse_secret_file(&mut self, cmd: &str, arg: &str) { + fn parse_secret_file(&mut self, cmd: &str, arg: &str) { if cmd == "--secret-file" { self.secret_file = arg.to_owned() } } - pub fn parse_pivot_file(&mut self, cmd: &str, arg: &str) { + fn parse_pivot_file(&mut self, cmd: &str, arg: &str) { if cmd == "--pivot-file" { self.pivot_file = arg.to_owned() } } - pub fn parse_ephemeral_key_file(&mut self, cmd: &str, arg: &str) { + fn parse_ephemeral_key_file(&mut self, cmd: &str, arg: &str) { if cmd == "--ephemeral-key-file" { self.ephemeral_key_file = arg.to_owned() } } + /// Get the `SocketAddress` for the enclave server. pub fn addr(&self) -> SocketAddress { match self.clone() { #[cfg(feature = "vm")] @@ -121,6 +125,7 @@ impl EnclaveOptions { } } + /// Get the [`NsmProvider`] pub fn nsm(&self) -> Box { if self.mock { Box::new(MockNsm) @@ -151,8 +156,10 @@ impl From> for EnclaveOptions { } } +/// Enclave server CLI. pub struct CLI {} impl CLI { + /// Execute the enclave server CLI with the environment args. pub fn execute() { let mut args: Vec = env::args().collect(); args.remove(0); diff --git a/qos-core/src/client.rs b/qos-core/src/client.rs index eb9e63e0..791d0a0c 100644 --- a/qos-core/src/client.rs +++ b/qos-core/src/client.rs @@ -6,10 +6,14 @@ use crate::{ protocol::{ProtocolError, ProtocolMsg}, }; +/// Enclave client error. #[derive(Debug)] pub enum ClientError { + /// [`io::IOError`] wrapper. IOError(io::IOError), + /// `ProtocolError` error wrapper. ProtocolError(ProtocolError), + /// `borsh::maybestd::io::Error` wrapper. BorshError(borsh::maybestd::io::Error), } diff --git a/qos-core/src/coordinator.rs b/qos-core/src/coordinator.rs index 828c8d95..94ee5c6b 100644 --- a/qos-core/src/coordinator.rs +++ b/qos-core/src/coordinator.rs @@ -8,6 +8,8 @@ use std::{path::Path, process::Command}; use crate::{cli::EnclaveOptions, protocol::Executor, server::SocketServer}; +/// Primary entry point for running the enclave. Coordinates spawning the server +/// and pivot binary. pub struct Coordinator; impl Coordinator { /// Run the coordinator. diff --git a/qos-core/src/io/mod.rs b/qos-core/src/io/mod.rs index 13665f9b..e810e0b7 100644 --- a/qos-core/src/io/mod.rs +++ b/qos-core/src/io/mod.rs @@ -8,10 +8,14 @@ mod stream; pub use stream::SocketAddress; pub(crate) use stream::{Listener, Stream}; +/// QOS I/O error #[derive(Debug)] pub enum IOError { + /// `nix::Error` wrapper. NixError(nix::Error), + /// Arithmetic operation saturated. ArithmeticSaturation, + /// Unknown error. UnknownError, } diff --git a/qos-core/src/io/stream.rs b/qos-core/src/io/stream.rs index 4a44adf4..87ed326a 100644 --- a/qos-core/src/io/stream.rs +++ b/qos-core/src/io/stream.rs @@ -19,26 +19,32 @@ use super::IOError; const MAX_RETRY: usize = 8; const BACKLOG: usize = 128; +/// Socket address. #[derive(Clone, Debug)] pub enum SocketAddress { + /// VSOCK address. #[cfg(feature = "vm")] Vsock(VsockAddr), + /// Unix address. #[cfg(feature = "local")] Unix(UnixAddr), } impl SocketAddress { + /// Create a new Unix socket. pub fn new_unix(path: &str) -> Self { let addr = UnixAddr::new(path).unwrap(); Self::Unix(addr) } + /// Create a new Vsock socket. #[cfg(feature = "vm")] pub fn new_vsock(cid: u32, port: u32) -> Self { let addr = VsockAddr::new(cid, port); Self::Vsock(addr) } + /// Get the `AddressFamily` of the socket. fn family(&self) -> AddressFamily { match *self { #[cfg(feature = "vm")] @@ -59,7 +65,8 @@ impl SocketAddress { } } -pub struct Stream { +/// Handle on a stream +pub(crate) struct Stream { fd: RawFd, } @@ -297,13 +304,12 @@ mod test { .unwrap(); let addr = SocketAddress::Unix(unix_addr); - let listener = Listener::listen(addr.clone()).unwrap(); + let mut listener = Listener::listen(addr.clone()).unwrap(); let handler = std::thread::spawn(move || { - for stream in listener { + if let Some(stream) = listener.next() { let req = stream.recv().unwrap(); stream.send(&req).unwrap(); - break; } }); diff --git a/qos-core/src/lib.rs b/qos-core/src/lib.rs index 0ff46345..f5c57dfc 100644 --- a/qos-core/src/lib.rs +++ b/qos-core/src/lib.rs @@ -1,25 +1,44 @@ +//! Core components of QOS. +//! +//! Any code that runs in the enclave should be contained here. +//! +//! This crate should have as minimal dependencies as possible to decrease +//! supply chain attack vectors and audit burden. + #![forbid(unsafe_code)] +#![deny(clippy::all)] +#![warn(missing_docs)] +/// Command line interface pub mod cli; -pub use cli::CLI; - +/// Client for communicating with the enclave server pub mod client; +/// Entry point for starting up enclave pub mod coordinator; +/// Basic IO capabilities pub mod io; +/// QuorumOS protocol details pub mod protocol; -pub mod server; +/// Basic socket server +mod server; +/// Path to Quorum Key secret. #[cfg(not(feature = "vm"))] pub const SECRET_FILE: &str = "./qos.secret"; +/// Path to Quorum Key secret. #[cfg(feature = "vm")] pub const SECRET_FILE: &str = "/qos.secret"; +/// Path to Pivot binary. #[cfg(not(feature = "vm"))] pub const PIVOT_FILE: &str = "../target/debug/pivot_ok"; +/// Path to Pivot binary. #[cfg(feature = "vm")] pub const PIVOT_FILE: &str = "/qos.pivot"; +/// Path to Ephemeral Key. #[cfg(not(feature = "vm"))] pub const EPHEMERAL_KEY_FILE: &str = "./qos.ephemeral.key"; +/// Path to Ephemeral Key. #[cfg(feature = "vm")] pub const EPHEMERAL_KEY_FILE: &str = "/qos.ephemeral.key"; diff --git a/qos-core/src/protocol/attestor/mock.rs b/qos-core/src/protocol/attestor/mock.rs index 5d4bbdcb..9a87d9ca 100644 --- a/qos-core/src/protocol/attestor/mock.rs +++ b/qos-core/src/protocol/attestor/mock.rs @@ -7,6 +7,7 @@ use super::{ NsmProvider, }; +/// Mock Nitro Secure Module endpoint that should only ever be used for testing. pub struct MockNsm; impl NsmProvider for MockNsm { fn nsm_process_request( diff --git a/qos-core/src/protocol/attestor/mod.rs b/qos-core/src/protocol/attestor/mod.rs index 461e27fc..11e20e34 100644 --- a/qos-core/src/protocol/attestor/mod.rs +++ b/qos-core/src/protocol/attestor/mod.rs @@ -4,19 +4,34 @@ mod mock; pub mod types; pub use mock::{MockNsm, MOCK_NSM_ATTESTATION_DOCUMENT}; +/// Something that implements the Nitro Secure Module endpoints. This is made +/// generic so mock providers can be subbed in for testing. In production use +/// [`Nsm`]. // https://github.com/aws/aws-nitro-enclaves-nsm-api/blob/main/docs/attestation_process.md pub trait NsmProvider { + /// Create a message with input data and output capacity from a given + /// request, then send it to the NSM driver via `ioctl()` and wait + /// for the driver's response. + /// *Argument 1 (input)*: The descriptor to the NSM device file. + /// *Argument 2 (input)*: The NSM request. + /// *Returns*: The corresponding NSM response from the driver. fn nsm_process_request( &self, fd: i32, request: types::NsmRequest, ) -> types::NsmResponse; + /// NSM library initialization function. + /// *Returns*: A descriptor for the opened device file. fn nsm_init(&self) -> i32; + /// NSM library exit function. + /// *Argument 1 (input)*: The descriptor for the opened device file, as + /// obtained from `nsm_init()`. fn nsm_exit(&self, fd: i32); } +/// Nitro Secure Module endpoints. pub struct Nsm; impl NsmProvider for Nsm { fn nsm_process_request( diff --git a/qos-core/src/protocol/attestor/types.rs b/qos-core/src/protocol/attestor/types.rs index 6809d369..83dc9e3d 100644 --- a/qos-core/src/protocol/attestor/types.rs +++ b/qos-core/src/protocol/attestor/types.rs @@ -1,9 +1,13 @@ -//! Types specific to AWS nitro enclave protocol implementation. +//! Types specific to AWS nitro enclave protocol implementation. We have types +//! that map 1 to 1 with the types we use from `ws_nitro_enclaves_nsm_api::api` +//! so we can derive borsh, among other things. + use std::collections::BTreeSet; use aws_nitro_enclaves_nsm_api as nsm; use nsm::api::{Digest, ErrorCode, Request, Response}; +/// Possible error codes from the Nitro Secure Module API. #[derive( Debug, borsh::BorshSerialize, borsh::BorshDeserialize, PartialEq, Clone, )] @@ -63,6 +67,7 @@ impl From for ErrorCode { } } +/// Possible hash digest for the Nitro Secure Module API. #[derive( Debug, borsh::BorshSerialize, @@ -102,6 +107,7 @@ impl From for Digest { } } +/// Request type for the Nitro Secure Module API. #[derive( Debug, borsh::BorshSerialize, borsh::BorshDeserialize, PartialEq, Clone, )] @@ -193,6 +199,7 @@ impl From for Request { } } +/// Response type for the Nitro Secure Module API. #[derive( Debug, borsh::BorshSerialize, borsh::BorshDeserialize, PartialEq, Clone, )] diff --git a/qos-core/src/protocol/boot.rs b/qos-core/src/protocol/boot.rs index 4946d697..c95edf5d 100644 --- a/qos-core/src/protocol/boot.rs +++ b/qos-core/src/protocol/boot.rs @@ -34,11 +34,14 @@ pub enum RestartPolicy { Always, } +/// Pivot binary configuration #[derive( PartialEq, Debug, Clone, borsh::BorshSerialize, borsh::BorshDeserialize, )] pub struct PivotConfig { + /// Hash of the pivot binary, taken from the binary as a `Vec`. pub hash: Hash256, + /// Restart policy for running the pivot binary. pub restart: RestartPolicy, } @@ -46,32 +49,57 @@ pub struct PivotConfig { PartialEq, Debug, Clone, borsh::BorshSerialize, borsh::BorshDeserialize, )] pub struct QuorumMember { + /// A human readable alias to identify the member. Must be unique to the + /// Quorum Set. pub alias: String, /// DER encoded RSA public key pub pub_key: Vec, } +/// The Quorum Set. #[derive( PartialEq, Debug, Clone, borsh::BorshSerialize, borsh::BorshDeserialize, )] pub struct QuorumSet { + /// The threshold, K, of signatures necessary to have quorum. pub threshold: u32, + /// Members composing the set. The length of this, N, must be gte to the + /// `threshold`, K. pub members: Vec, } +#[derive( + PartialEq, Debug, Clone, borsh::BorshSerialize, borsh::BorshDeserialize, +)] +pub struct Namespace { + /// The namespace. This should be unique relative to other namespaces the + /// organization running QuorumOs has. + name: String, + /// A monotonically increasing value, used to identify the order in which + /// manifests for this namespace have been created. This is used to prevent + /// downgrade attacks - quorum members should only approve a manifest that + /// has the highest nonce. + nonce: u32, +} + #[derive( PartialEq, Debug, Clone, borsh::BorshSerialize, borsh::BorshDeserialize, )] pub struct Manifest { - pub nonce: u32, - pub namespace: String, + /// Namespace this manifest belongs too. + pub namespace: Namespace, + /// Configuration and verifiable values for the enclave hardware. pub enclave: NitroConfig, + /// Pivot binary configuration and verifiable values. pub pivot: PivotConfig, + /// Quorum Key as a DER encoded RSA public key. pub quorum_key: Vec, + /// Quorum Set members and threshold. pub quorum_set: QuorumSet, } impl Manifest { + /// Canonical hash for the manifest. pub fn hash(&self) -> Hash256 { qos_crypto::sha_256( &self.try_to_vec().expect("`Manifest` serializes with cbor"), @@ -79,23 +107,30 @@ impl Manifest { } } +/// An approval by a Quorum Member. #[derive( PartialEq, Debug, Clone, borsh::BorshSerialize, borsh::BorshDeserialize, )] pub struct Approval { + /// Quorum Member's signature. pub signature: Vec, + /// Description of the Quorum Member pub member: QuorumMember, } +/// [`Manifest`] with accompanying [`Approval`]s. #[derive( PartialEq, Debug, Clone, borsh::BorshSerialize, borsh::BorshDeserialize, )] pub struct ManifestEnvelope { + /// Encapsulated manifest. pub manifest: Manifest, + /// Approvals for [`Self::manifest`]. pub approvals: Vec, } impl ManifestEnvelope { + /// Check if the encapsulated manifest has K valid approvals. pub fn check_approvals(&self) -> Result<(), ProtocolError> { for approval in self.approvals.iter() { let pub_key = RsaPub::from_der(&approval.member.pub_key) @@ -119,7 +154,7 @@ impl ManifestEnvelope { } } -pub fn boot_standard( +pub(super) fn boot_standard( state: &mut ProtocolState, manifest_envelope: ManifestEnvelope, pivot: &Vec, diff --git a/qos-core/src/protocol/genesis.rs b/qos-core/src/protocol/genesis.rs index 1dbfc134..162da42a 100644 --- a/qos-core/src/protocol/genesis.rs +++ b/qos-core/src/protocol/genesis.rs @@ -5,6 +5,7 @@ use qos_crypto::{RsaPair, RsaPub}; use super::{Hash256, NsmRequest, NsmResponse, ProtocolError, ProtocolState}; +/// Member of the [`SetupSet`]. #[derive( PartialEq, Debug, Clone, borsh::BorshSerialize, borsh::BorshDeserialize, )] @@ -34,39 +35,49 @@ pub struct GenesisSet { )] struct MemberShard { // TODO: is this taking up too much unnecessary space? + /// Member of the Setup Set. member: SetupMember, + /// Shard of the generated Quorum Key, encrypted to the `member`s Setup + /// Key. shard: Vec, } +/// A set of member shards used to succesfully recover the quorum key during the +/// genesis ceremony. #[derive( PartialEq, Debug, Clone, borsh::BorshSerialize, borsh::BorshDeserialize, )] pub struct RecoveredPermutation(Vec); +/// Genesis output per Setup Member. #[derive( PartialEq, Debug, Clone, borsh::BorshSerialize, borsh::BorshDeserialize, )] pub struct GenesisMemberOutput { /// The Quorum Member whom's Setup Key was used. pub setup_member: SetupMember, - /// Quorum Key Share encrypted to the Personal Key. + /// Quorum Key Share encrypted to the `setup_member`'s Personal Key. pub encrypted_quorum_key_share: Vec, - /// Personal Key encrypted to the Quorum Member's Setup Key. + /// Personal Key encrypted to the `setup_member`'s Setup Key. pub encrypted_personal_key: Vec, } +/// Output from running Genesis Boot. #[derive( PartialEq, Debug, Clone, borsh::BorshSerialize, borsh::BorshDeserialize, )] pub struct GenesisOutput { - /// Quorum Key - DER encoded RSA public key + /// Public Quorum Key, DER encoded. pub quorum_key: Vec, /// Quorum Member specific outputs from the genesis ceremony. pub member_outputs: Vec, + /// All successfully `RecoveredPermutation`s completed during the genesis + /// process. pub recovery_permutations: Vec, } impl GenesisOutput { + /// Canonical hash of [`Self`] pub fn hash(&self) -> Hash256 { qos_crypto::sha_256( &self.try_to_vec().expect("`Manifest` serializes with cbor"), @@ -80,7 +91,7 @@ impl GenesisOutput { // // TODO: Disaster recovery logic! // Maybe we can just accept 2 set configs, and one is the recovery set?`` -pub fn boot_genesis( +pub(super) fn boot_genesis( state: &mut ProtocolState, genesis_set: &GenesisSet, ) -> Result<(GenesisOutput, NsmResponse), ProtocolError> { diff --git a/qos-core/src/protocol/mod.rs b/qos-core/src/protocol/mod.rs index fa48eed8..2a69f165 100644 --- a/qos-core/src/protocol/mod.rs +++ b/qos-core/src/protocol/mod.rs @@ -32,17 +32,29 @@ const MAX_ENCODED_MSG_LEN: usize = 10 * MEGABYTE; type ProtocolHandler = dyn Fn(&ProtocolMsg, &mut ProtocolState) -> Option; +/// 256bit hash pub type Hash256 = [u8; 32]; +/// A error from protocol execution. #[derive(Debug, PartialEq, borsh::BorshSerialize, borsh::BorshDeserialize)] pub enum ProtocolError { + /// TODO InvalidShare, + /// TODO ReconstructionError, + /// Filesystem error IOError, + /// Cryptography error CryptoError, + /// Approval was not valid for a manifest. InvalidManifestApproval(Approval), + /// [`ManifestEnvelope`] did not have approvals NotEnoughApprovals, + /// Protocol Message could not be matched against an available route. + /// Ensure the executor is in the correct phase. NoMatchingRoute(ProtocolPhase), + /// Hash of the Pivot binary does not match the pivot configuration in the + /// manifest. InvalidPivotHash, } @@ -64,15 +76,22 @@ impl From for ProtocolError { } } +/// Protocol executor state. #[derive( Debug, PartialEq, Clone, borsh::BorshSerialize, borsh::BorshDeserialize, )] pub enum ProtocolPhase { + /// The state machine cannot recover. The enclave must be rebooted. UnrecoverableError, + /// Waiting to receive a boot instruction. WaitingForBootInstruction, + /// Waiting to receive K quorum shards WaitingForQuorumShards, } +/// Enclave executor state +// TODO only include mutables in here, all else should be written to file as +// read only pub struct ProtocolState { provisioner: SecretProvisioner, attestor: Box, @@ -101,11 +120,14 @@ impl ProtocolState { } } +/// Maybe rename state machine? +/// Enclave state machine that executes when given a `ProtocolMsg`. pub struct Executor { state: ProtocolState, } impl Executor { + /// Create a new `Self`. pub fn new( attestor: Box, secret_file: String, @@ -200,7 +222,8 @@ mod handlers { } // TODO: Add tests for this in the middle of some integration tests - pub fn status( + /// Status of the enclave. + pub(super) fn status( req: &ProtocolMsg, state: &mut ProtocolState, ) -> Option { @@ -211,7 +234,8 @@ mod handlers { } } - pub fn empty( + /// TODO: remove + pub(super) fn empty( req: &ProtocolMsg, _state: &mut ProtocolState, ) -> Option { @@ -222,7 +246,8 @@ mod handlers { } } - pub fn echo( + /// TODO: remove + pub(super) fn echo( req: &ProtocolMsg, _state: &mut ProtocolState, ) -> Option { @@ -233,7 +258,7 @@ mod handlers { } } - pub fn provision( + pub(super) fn provision( req: &ProtocolMsg, state: &mut ProtocolState, ) -> Option { @@ -247,14 +272,15 @@ mod handlers { } } - pub fn reconstruct( + /// TODO: remove + pub(super) fn reconstruct( req: &ProtocolMsg, state: &mut ProtocolState, ) -> Option { if let ProtocolMsg::ReconstructRequest = req { match state.provisioner.reconstruct() { - Ok(secret) => { - state.provisioner.secret = Some(secret); + Ok(_secret) => { + // state.provisioner.secret = Some(secret); Some(ProtocolMsg::SuccessResponse) } Err(_) => Some(ProtocolMsg::ErrorResponse), @@ -264,7 +290,8 @@ mod handlers { } } - pub fn nsm_attestation( + /// TODO: remove + pub(super) fn nsm_attestation( req: &ProtocolMsg, state: &mut ProtocolState, ) -> Option { @@ -284,7 +311,8 @@ mod handlers { } } - pub fn load( + /// TODO: remove + pub(super) fn load( req: &ProtocolMsg, state: &mut ProtocolState, ) -> Option { @@ -316,7 +344,8 @@ mod handlers { } } - pub fn boot_instruction( + /// TODO: remove + pub(super) fn boot_instruction( req: &ProtocolMsg, state: &mut ProtocolState, ) -> Option { diff --git a/qos-core/src/protocol/msg.rs b/qos-core/src/protocol/msg.rs index 6095545f..638381a4 100644 --- a/qos-core/src/protocol/msg.rs +++ b/qos-core/src/protocol/msg.rs @@ -1,4 +1,4 @@ -//! Enclave I/O message format. +//! Enclave executor message types. use super::{ boot::ManifestEnvelope, @@ -6,52 +6,78 @@ use super::{ NsmRequest, NsmResponse, ProtocolError, }; +/// Message types for communicating with protocol executor. #[derive(Debug, PartialEq, borsh::BorshSerialize, borsh::BorshDeserialize)] pub enum ProtocolMsg { - SuccessResponse, - // TODO: Error response should hold a protocol error - ErrorResponse, + /// The executor encountered an unrecoverable error. UnrecoverableErrorResponse, + /// Could not process response because in unrecoverable phase. TODO: maybe + /// remove InUnrecoverablePhaseResponse, + + /// A error from executing the protocol. + ProtocolErrorResponse(ProtocolError), + + /// TODO: remove EmptyRequest, + /// TODO: remove EmptyResponse, + /// TODO: remove EchoRequest(Echo), + /// TODO: remove EchoResponse(Echo), - ProvisionRequest(Provision), + /// TODO: remove ReconstructRequest, + /// TODO: remove NsmRequest(NsmRequest), + /// TODO: remove NsmResponse(NsmResponse), + /// TODO: remove LoadRequest(Load), + /// Request was succesful. TODO: remove + SuccessResponse, + /// TODO: Error response should hold a protocol error, Remove + ErrorResponse, + /// Request the status of the enclave. StatusRequest, + /// Response for [`StatusRequest`] StatusResponse(super::ProtocolPhase), + /// Send the boot instruction. BootRequest(BootInstruction), + /// Response for Standard Boot. BootStandardResponse(NsmResponse), + /// Response for Genesis Boot. BootGenesisResponse { /// COSE SIGN1 structure with Attestation Doc nsm_response: NsmResponse, /// Output from the Genesis flow. genesis_output: GenesisOutput, }, - - ProtocolErrorResponse(ProtocolError), + /// Post a quorum key shard + ProvisionRequest(Provision), } +/// TODO: remove #[derive( PartialEq, Debug, Clone, borsh::BorshSerialize, borsh::BorshDeserialize, )] pub struct Echo { + /// TODO: remove pub data: Vec, } +/// TODO: replace with provision service etc #[derive( PartialEq, Debug, Clone, borsh::BorshSerialize, borsh::BorshDeserialize, )] pub struct Provision { + /// TODO: remove pub share: Vec, } +/// TODO: remove #[derive( Debug, PartialEq, Clone, borsh::BorshSerialize, borsh::BorshDeserialize, )] @@ -63,22 +89,34 @@ pub struct SignatureWithPubKey { pub path: String, } +/// TODO: remove #[derive( Debug, PartialEq, Clone, borsh::BorshSerialize, borsh::BorshDeserialize, )] pub struct Load { /// The executable to pivot to pub executable: Vec, - //// Signatures of the data + /// Signatures of the data pub signatures: Vec, } +/// Instruction for initiate the enclave boot interactive process. #[derive( PartialEq, Debug, Clone, borsh::BorshSerialize, borsh::BorshDeserialize, )] pub enum BootInstruction { - Standard { manifest_envelope: Box, pivot: Vec }, - Genesis { set: GenesisSet }, + /// Execute Standard Boot. + Standard { + /// Manifest with approvals + manifest_envelope: Box, + /// Pivot binary + pivot: Vec, + }, + /// Execute Genesis Boot. + Genesis { + /// Parameters for creating a Quorum Set + set: GenesisSet, + }, } #[cfg(test)] diff --git a/qos-core/src/protocol/provisioner.rs b/qos-core/src/protocol/provisioner.rs index fd6fc65d..7d80146b 100644 --- a/qos-core/src/protocol/provisioner.rs +++ b/qos-core/src/protocol/provisioner.rs @@ -6,17 +6,21 @@ type Secret = Vec; type Share = Vec; type Shares = Vec; +/// Shamir Secret provisioner. pub struct SecretProvisioner { shares: Shares, - pub secret: Option, + // TODO: maybe don't store secret and just return it on reconstruct + secret: Option, secret_file: String, } impl SecretProvisioner { + /// Create a instance of [`Self`]. pub fn new(secret_file: String) -> Self { Self { shares: Vec::new(), secret: None, secret_file } } + /// Add a share to later be used to reconstruct. pub fn add_share(&mut self, share: Share) -> Result<(), ProtocolError> { if share.is_empty() { return Err(ProtocolError::InvalidShare); @@ -26,6 +30,7 @@ impl SecretProvisioner { Ok(()) } + /// Attempt to reconstruct the secret from the pub fn reconstruct(&mut self) -> Result { let secret = qos_crypto::shares_reconstruct(&self.shares); @@ -41,6 +46,7 @@ impl SecretProvisioner { file.write_all(&secret) .map_err(|_e| ProtocolError::ReconstructionError)?; + self.secret = Some(secret.clone()); Ok(secret) } } diff --git a/qos-core/src/server.rs b/qos-core/src/server.rs index 60a2b52a..39d84908 100644 --- a/qos-core/src/server.rs +++ b/qos-core/src/server.rs @@ -5,10 +5,11 @@ use std::marker::PhantomData; use crate::io::{self, Listener, SocketAddress}; +/// Error variants for [`SocketServer`] #[derive(Debug)] pub enum SocketServerError { + /// `io::IOError` wrapper. IOError(io::IOError), - NotFound, } impl From for SocketServerError { @@ -17,16 +18,18 @@ impl From for SocketServerError { } } +/// A bare bare bones, socket based server. pub struct SocketServer { _phantom: PhantomData, } impl SocketServer { + /// Listen and respond to incoming requests with the given `processor`. pub fn listen( addr: SocketAddress, mut processor: R, ) -> Result<(), SocketServerError> { - println!("SocketServer listening on {:?}", addr); + println!("`SocketServer` listening on {:?}", addr); let listener = Listener::listen(addr)?; @@ -45,5 +48,10 @@ impl SocketServer { } pub trait Routable { - fn process(&mut self, req: Vec) -> Vec; + /// Process an incoming request and return a response. + /// + /// The request and response are raw bytes. Likely this should be encoded + /// data and logic inside of this function should take care of decoding the + /// request and encoding a response. + fn process(&mut self, request: Vec) -> Vec; } diff --git a/qos-crypto/src/lib.rs b/qos-crypto/src/lib.rs index dbb8945f..db8b845f 100644 --- a/qos-crypto/src/lib.rs +++ b/qos-crypto/src/lib.rs @@ -1,3 +1,9 @@ +//! Cryptographic primitves for use with QuorumOS. + +#![forbid(unsafe_code)] +#![deny(clippy::all)] +#![warn(missing_docs)] + // TODO: Audit encryption strategy // This file implements an envelope encryption strategy using RSA and AES 256 // CBC Ensure that this is a sensible approach. @@ -24,12 +30,18 @@ pub use shamir::*; /// Standard length for QuorumOS RSA keys, specified in bits. pub const RSA_KEY_LEN: u32 = 4096; +/// Errors for this crate. #[derive(Debug)] pub enum CryptoError { + /// Wrapper for `std::io::Error`. IOError(std::io::Error), + /// Wrapper for `openssl::error::ErrorStack`. OpenSSLError(openssl::error::ErrorStack), + /// Error while trying to decrypt. DecryptError(openssl::error::ErrorStack), + /// An `Envelope` could not be deserialized. InvalidEnvelope, + /// Cannot encrypt a payload because it is too big. EncryptionPayloadTooBig, } @@ -45,6 +57,13 @@ impl From for CryptoError { } } +/// Create a SHA256 hash digest of `buf`. +pub fn sha_256(buf: &[u8]) -> [u8; 32] { + let mut hasher = openssl::sha::Sha256::new(); + hasher.update(buf); + hasher.finish() +} + /// RSA Private key pair. #[derive(Clone)] pub struct RsaPair { @@ -90,6 +109,7 @@ impl RsaPair { signer.sign_to_vec().map_err(Into::into) } + /// Get the PEM encoded private key. pub fn public_key_pem(&self) -> Result, CryptoError> { self.private_key.public_key_to_pem().map_err(Into::into) } @@ -107,6 +127,7 @@ impl RsaPair { Ok(to[0..size].to_vec()) } + /// Decrypt envelope encrypted `data`. Also see [`Self::envelope_encrypt`]. pub fn envelope_decrypt( &self, data: &[u8], @@ -159,25 +180,30 @@ impl Deref for RsaPair { } } +/// RSA public key. #[derive(Debug, Clone)] pub struct RsaPub { public_key: Rsa, } impl RsaPub { + /// Create [`Self`] from a PEM encoded RSA public key from a file. pub fn from_pem_file>(path: P) -> Result { let content = std::fs::read(path)?; Self::from_pem(&content[..]) } + /// Create [`Self`] from a PEM encoded RSA public key. pub fn from_pem(pem: &[u8]) -> Result { Ok(Self { public_key: Rsa::public_key_from_pem(pem)? }) } + /// Create [`Self`] from a DER encoded RSA public key. pub fn from_der(der: &[u8]) -> Result { Ok(Self { public_key: Rsa::public_key_from_der(der)? }) } + /// Write the PEM encoded public key to file. pub fn write_pem_file>( &self, path: P, @@ -188,6 +214,7 @@ impl RsaPub { Ok(()) } + /// Verify the signature over the SHA-256 digest of `msg`. pub fn verify_sha256( &self, signature: &[u8], @@ -225,6 +252,9 @@ impl RsaPub { Ok(to[0..size].to_vec()) } + /// Encrypt `data` using envelope encryption. The data is encrypted with AES + /// 256 CBC, a symmetric encryption key. The AES 256 CBC key is encrypted + /// with this public RSA key. pub fn envelope_encrypt( &self, data: &[u8], @@ -287,7 +317,7 @@ impl PartialEq for RsaPub { } #[derive(PartialEq, Debug, Clone, BorshSerialize, BorshDeserialize)] -pub struct Envelope { +struct Envelope { pub encrypted_symm_key: Vec, pub encrypted_data: Vec, pub iv: Vec, @@ -302,13 +332,6 @@ impl TryFrom> for RsaPub { } } -/// Create a SHA256 hash digest of `buf`. -pub fn sha_256(buf: &[u8]) -> [u8; 32] { - let mut hasher = openssl::sha::Sha256::new(); - hasher.update(buf); - hasher.finish() -} - #[cfg(test)] mod test { use super::*; diff --git a/qos-crypto/src/shamir.rs b/qos-crypto/src/shamir.rs index 4543624b..0549c8a5 100644 --- a/qos-crypto/src/shamir.rs +++ b/qos-crypto/src/shamir.rs @@ -111,7 +111,7 @@ const GF256_EXP: [u8; 2*255] = [ 0x1c, 0x24, 0x6c, 0xb4, 0xc7, 0x52, 0xf6, ]; -/// multiply in GF(256) +/// Multiply in GF(256). fn gf256_mul(a: u8, b: u8) -> u8 { if a == 0 || b == 0 { 0 @@ -121,18 +121,18 @@ fn gf256_mul(a: u8, b: u8) -> u8 { } } -/// divide in GF(256) +/// Divide in GF(256)/ fn gf256_div(a: u8, b: u8) -> u8 { // multiply a against inverse b gf256_mul(a, GF256_EXP[usize::from(255 - GF256_LOG[usize::from(b)])]) } -/// evaluate a polynomial at x over GF(256) using Horner's method +/// Evaluate a polynomial at x over GF(256) using Horner's method. fn gf256_eval(f: &[u8], x: u8) -> u8 { f.iter().rev().fold(0, |acc, c| gf256_mul(acc, x) ^ c) } -/// generate a random polynomial of given degree, fixing f(0) = secret +/// Generate a random polynomial of given degree, fixing f(0) = secret. fn gf256_generate(secret: u8, degree: usize) -> Vec { // TODO: improve randomness let mut rng = rand::thread_rng(); @@ -141,7 +141,7 @@ fn gf256_generate(secret: u8, degree: usize) -> Vec { .collect() } -/// find f(0) using Lagrange interpolation +/// Find f(0) using Lagrange interpolation. fn gf256_interpolate(xs: &[u8], ys: &[u8]) -> u8 { assert!(xs.len() == ys.len()); let mut y = 0u8; @@ -159,8 +159,7 @@ fn gf256_interpolate(xs: &[u8], ys: &[u8]) -> u8 { y } -/// generate n shares requiring k shares to reconstruct -// #[allow(clippy::needless_range_loop)] +/// Generate n shares requiring k shares to reconstruct. pub fn shares_generate(secret: &[u8], n: usize, k: usize) -> Vec> { let mut shares = vec![vec![]; n]; @@ -186,7 +185,7 @@ pub fn shares_generate(secret: &[u8], n: usize, k: usize) -> Vec> { shares } -/// reconstruct our secret +/// Reconstruct our secret from the given `shares`. pub fn shares_reconstruct>(shares: &[S]) -> Vec { let len = shares.iter().map(|s| s.as_ref().len()).min().unwrap_or(0); // rather than erroring, return empty secrets if input is malformed. diff --git a/qos-test/src/bin/core_cli.rs b/qos-test/src/bin/core_cli.rs index b6127e80..6f9e487b 100644 --- a/qos-test/src/bin/core_cli.rs +++ b/qos-test/src/bin/core_cli.rs @@ -1,3 +1,3 @@ fn main() { - qos_core::CLI::execute() + qos_core::cli::CLI::execute() } diff --git a/qos-test/src/lib.rs b/qos-test/src/lib.rs index 5a78e964..aed3c262 100644 --- a/qos-test/src/lib.rs +++ b/qos-test/src/lib.rs @@ -1,3 +1,4 @@ #![forbid(unsafe_code)] +#![deny(clippy::all)] pub const PIVOT_OK_SUCCESS_FILE: &str = "./pivot_ok_works"; diff --git a/qos-test/tests/load.rs b/qos-test/tests/load.rs index 62db739e..66330148 100644 --- a/qos-test/tests/load.rs +++ b/qos-test/tests/load.rs @@ -45,7 +45,7 @@ fn load_e2e() { .iter() .enumerate() .map(|(i, pair)| SignatureWithPubKey { - signature: pair.sign_sha256(&mut executable.clone()[..]).unwrap(), + signature: pair.sign_sha256(&executable.clone()[..]).unwrap(), path: paths[i].to_string_lossy().into_owned(), }) .collect(); From 39264b34c12b868dcd4b3ac23989e5544c7dc547 Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Tue, 14 Jun 2022 22:46:54 -0700 Subject: [PATCH 19/25] Add missing_docs to the remaining crates --- qos-client/src/attest/mod.rs | 19 ++++++++++++++++++- qos-client/src/cli.rs | 4 ++++ qos-client/src/lib.rs | 7 +++++++ qos-host/src/cli.rs | 20 ++++++++++++++------ qos-host/src/lib.rs | 3 +++ qos-test/src/lib.rs | 4 ++++ 6 files changed, 50 insertions(+), 7 deletions(-) diff --git a/qos-client/src/attest/mod.rs b/qos-client/src/attest/mod.rs index 9121fa72..10cb717b 100644 --- a/qos-client/src/attest/mod.rs +++ b/qos-client/src/attest/mod.rs @@ -1,22 +1,39 @@ -//! Attestation verification logic +//! Attestation specific logic pub mod nitro; +/// Attestation error. #[derive(Debug)] pub enum AttestError { + /// `webpki::Error` wrapper. WebPki(webpki::Error), + /// Invalid certificate chain. InvalidCertChain(webpki::Error), + /// `openssl::error::ErrorStack` wrapper. OpenSSLError(openssl::error::ErrorStack), + /// `aws_nitro_enclaves_nsm_api::api::Error` wrapper. Nsm(aws_nitro_enclaves_nsm_api::api::Error), + /// Invalid end entity certificate. In the case of Nitro this means the + /// NSM's certificate was invalid. InvalidEndEntityCert, + /// Invalid COSE Sign1 structure signature. In the case of Nitro this means + /// the end entitys signature of the attestation doc was invalid. InvalidCOSESign1Signature, + /// Invalid COSE Sign1 structure. InvalidCOSESign1Structure, + /// Invalid hash digest. InvalidDigest, + /// Invalid NSM module id. InvalidModuleId, + /// Invalid PCR. InvalidPcr, + /// Invalid certificate authority bundle. InvalidCABundle, + /// Invalid time. InvalidTimeStamp, + /// Invalid public key. InvalidPubKey, + /// Invalid bytes. InvalidBytes, } diff --git a/qos-client/src/cli.rs b/qos-client/src/cli.rs index 09298d05..87b6221a 100644 --- a/qos-client/src/cli.rs +++ b/qos-client/src/cli.rs @@ -1,3 +1,5 @@ +//! QuorumOS client command line interface. + use std::env; use borsh::BorshSerialize; @@ -190,8 +192,10 @@ impl BootGenesisOptions { } } +/// Client command line interface pub struct CLI; impl CLI { + /// Execute this command line interface. pub fn execute() { let args: Vec = env::args().collect(); let options = ClientOptions::from(args); diff --git a/qos-client/src/lib.rs b/qos-client/src/lib.rs index 9d4cb8eb..ea7ecea9 100644 --- a/qos-client/src/lib.rs +++ b/qos-client/src/lib.rs @@ -1,8 +1,13 @@ +//! CLI Client for interacting with QuorumOS enclave and host. + #![forbid(unsafe_code)] +#![deny(clippy::all)] +#![warn(missing_docs)] pub mod attest; pub mod cli; +/// Host HTTP request helpers. pub mod request { use std::io::Read; @@ -11,6 +16,7 @@ pub mod request { const MAX_SIZE: u64 = u32::MAX as u64; + /// Post a [`qos_core::protocol::ProtocolMsg`] to the given host `url`. pub fn post(url: &str, msg: ProtocolMsg) -> Result { let mut buf: Vec = vec![]; @@ -33,6 +39,7 @@ pub mod request { Ok(pr) } + /// Get the resource at the given host `url`. pub fn get(url: &str) -> Result { ureq::get(url) .call() diff --git a/qos-host/src/cli.rs b/qos-host/src/cli.rs index cd6c70d1..0d93657d 100644 --- a/qos-host/src/cli.rs +++ b/qos-host/src/cli.rs @@ -1,5 +1,6 @@ -// Enclave socket address -// Port/Host bindings +//! Command line interface for creating a host server and helpers for parsing +//! host specific command line arguments. + use std::{env, net::SocketAddr}; use qos_core::cli::EnclaveOptions; @@ -9,6 +10,7 @@ use crate::HostServer; const IP_REGEX: &str = r"^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$"; +/// CLI options for starting a host server #[derive(Clone, Debug, PartialEq)] pub struct HostServerOptions { enclave: EnclaveOptions, @@ -21,6 +23,7 @@ impl HostServerOptions { } } +/// CLI options for host IP address and Port. #[derive(Default, Clone, Debug, PartialEq)] pub struct HostOptions { ip: Option<[u8; 4]>, @@ -28,10 +31,12 @@ pub struct HostOptions { } impl HostOptions { + /// Create a new instance of [`self`]. pub fn new() -> Self { Default::default() } + /// Get the host server url. pub fn url(&self) -> String { if let Self { ip: Some(ip), port: Some(port) } = self.clone() { return format!( @@ -43,17 +48,19 @@ impl HostOptions { } } + /// Get the resource path. pub fn path(&self, path: &str) -> String { let url = self.url(); format!("{}/{}", url, path) } + /// Parse host options. pub fn parse(&mut self, cmd: &str, arg: &str) { self.parse_ip(cmd, arg); self.parse_port(cmd, arg); } - pub fn parse_ip(&mut self, cmd: &str, arg: &str) { + fn parse_ip(&mut self, cmd: &str, arg: &str) { if cmd == "--host-ip" { let re = Regex::new(IP_REGEX) .expect("Could not parse value from `--host-ip`"); @@ -76,7 +83,7 @@ impl HostOptions { } } - pub fn parse_port(&mut self, cmd: &str, arg: &str) { + fn parse_port(&mut self, cmd: &str, arg: &str) { if cmd == "--host-port" { self.port = arg .parse::() @@ -101,6 +108,7 @@ impl HostOptions { /// sockets) pub struct CLI; impl CLI { + /// Execute the command line interface. pub async fn execute() { let mut args: Vec = env::args().collect(); args.remove(0); @@ -114,7 +122,7 @@ impl CLI { } } -pub fn parse_args(args: Vec) -> HostServerOptions { +fn parse_args(args: Vec) -> HostServerOptions { let mut options = HostServerOptions::new(); let mut chunks = args.chunks_exact(2); @@ -129,7 +137,7 @@ pub fn parse_args(args: Vec) -> HostServerOptions { options } -pub fn host_addr_from_options(options: HostOptions) -> SocketAddr { +fn host_addr_from_options(options: HostOptions) -> SocketAddr { if let HostOptions { ip: Some(ip), port: Some(port), .. } = options { SocketAddr::from((ip, port)) } else { diff --git a/qos-host/src/lib.rs b/qos-host/src/lib.rs index d5e95c98..51142ebf 100644 --- a/qos-host/src/lib.rs +++ b/qos-host/src/lib.rs @@ -13,6 +13,8 @@ //! * Response: //! * Responding with error: #![forbid(unsafe_code)] +#![deny(clippy::all)] +#![warn(missing_docs)] use std::{net::SocketAddr, sync::Arc}; @@ -51,6 +53,7 @@ impl HostServer { Self { addr: SocketAddr::from((ip, port)), enclave_addr } } + /// Create a new [`HostServer`]. pub fn new_with_socket_addr( enclave_addr: SocketAddress, addr: SocketAddr, diff --git a/qos-test/src/lib.rs b/qos-test/src/lib.rs index aed3c262..9e96471b 100644 --- a/qos-test/src/lib.rs +++ b/qos-test/src/lib.rs @@ -1,4 +1,8 @@ +//! Integration tests. + #![forbid(unsafe_code)] #![deny(clippy::all)] +#![warn(missing_docs)] +/// Path to the file `pivot_ok` writes on success. pub const PIVOT_OK_SUCCESS_FILE: &str = "./pivot_ok_works"; From c986b006fbd3f2cf5fc6e146cbbda3b39356aaa7 Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Tue, 14 Jun 2022 22:53:53 -0700 Subject: [PATCH 20/25] Add clippy::pedantic to host --- qos-host/src/cli.rs | 18 +++++++++++------- qos-host/src/lib.rs | 15 ++++++++++----- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/qos-host/src/cli.rs b/qos-host/src/cli.rs index 0d93657d..0965515c 100644 --- a/qos-host/src/cli.rs +++ b/qos-host/src/cli.rs @@ -32,24 +32,29 @@ pub struct HostOptions { impl HostOptions { /// Create a new instance of [`self`]. + #[must_use] pub fn new() -> Self { - Default::default() + Self::default() } /// Get the host server url. - pub fn url(&self) -> String { + /// + /// # Panics + /// + /// Panics if the url cannot be parsed from options + #[must_use] pub fn url(&self) -> String { if let Self { ip: Some(ip), port: Some(port) } = self.clone() { return format!( "http://{}.{}.{}.{}:{}", ip[0], ip[1], ip[2], ip[3], port ); - } else { - panic!("Couldn't parse URL from options.") } + + panic!("Couldn't parse URL from options.") } /// Get the resource path. - pub fn path(&self, path: &str) -> String { + #[must_use] pub fn path(&self, path: &str) -> String { let url = self.url(); format!("{}/{}", url, path) } @@ -117,8 +122,7 @@ impl CLI { let enclave_addr = options.enclave.addr(); HostServer::new_with_socket_addr(enclave_addr, addr) .serve() - .await - .unwrap(); + .await; } } diff --git a/qos-host/src/lib.rs b/qos-host/src/lib.rs index 51142ebf..a7f217f7 100644 --- a/qos-host/src/lib.rs +++ b/qos-host/src/lib.rs @@ -14,7 +14,7 @@ //! * Responding with error: #![forbid(unsafe_code)] #![deny(clippy::all)] -#![warn(missing_docs)] +#![warn(missing_docs, clippy::pedantic)] use std::{net::SocketAddr, sync::Arc}; @@ -54,15 +54,20 @@ impl HostServer { } /// Create a new [`HostServer`]. - pub fn new_with_socket_addr( + #[must_use] pub fn new_with_socket_addr( enclave_addr: SocketAddress, addr: SocketAddr, ) -> Self { - Self { addr, enclave_addr } + Self { enclave_addr, addr } } /// Start the server, running indefinitely. - pub async fn serve(&self) -> Result<(), String> { + /// + /// # Panics + /// + /// Panics if there is an issue starting the server. + // pub async fn serve(&self) -> Result<(), String> { + pub async fn serve(&self) { let state = Arc::new(State { enclave_client: Client::new(self.enclave_addr.clone()), }); @@ -79,7 +84,7 @@ impl HostServer { .await .unwrap(); - Ok(()) + // Ok(()) } /// Health route handler. From 5d193e21c04ce846a8900b78ecfa261ad5a5f09f Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Tue, 14 Jun 2022 22:57:01 -0700 Subject: [PATCH 21/25] Finish pedantic lints for host --- qos-crypto/src/lib.rs | 2 +- qos-host/src/cli.rs | 17 +++++++---------- qos-host/src/lib.rs | 2 +- 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/qos-crypto/src/lib.rs b/qos-crypto/src/lib.rs index db8b845f..4fb92709 100644 --- a/qos-crypto/src/lib.rs +++ b/qos-crypto/src/lib.rs @@ -2,7 +2,7 @@ #![forbid(unsafe_code)] #![deny(clippy::all)] -#![warn(missing_docs)] +#![warn(missing_docs, clippy::pedantic)] // TODO: Audit encryption strategy // This file implements an envelope encryption strategy using RSA and AES 256 diff --git a/qos-host/src/cli.rs b/qos-host/src/cli.rs index 0965515c..635644f7 100644 --- a/qos-host/src/cli.rs +++ b/qos-host/src/cli.rs @@ -24,7 +24,7 @@ impl HostServerOptions { } /// CLI options for host IP address and Port. -#[derive(Default, Clone, Debug, PartialEq)] +#[derive(Default, Clone, Copy, Debug, PartialEq)] pub struct HostOptions { ip: Option<[u8; 4]>, port: Option, @@ -32,8 +32,7 @@ pub struct HostOptions { impl HostOptions { /// Create a new instance of [`self`]. - #[must_use] - pub fn new() -> Self { + #[must_use] pub fn new() -> Self { Self::default() } @@ -43,7 +42,7 @@ impl HostOptions { /// /// Panics if the url cannot be parsed from options #[must_use] pub fn url(&self) -> String { - if let Self { ip: Some(ip), port: Some(port) } = self.clone() { + if let Self { ip: Some(ip), port: Some(port) } = self { return format!( "http://{}.{}.{}.{}:{}", ip[0], ip[1], ip[2], ip[3], port @@ -117,8 +116,8 @@ impl CLI { pub async fn execute() { let mut args: Vec = env::args().collect(); args.remove(0); - let options = parse_args(args); - let addr = host_addr_from_options(options.host.clone()); + let options = parse_args(&args); + let addr = host_addr_from_options(options.host); let enclave_addr = options.enclave.addr(); HostServer::new_with_socket_addr(enclave_addr, addr) .serve() @@ -126,13 +125,11 @@ impl CLI { } } -fn parse_args(args: Vec) -> HostServerOptions { +fn parse_args(args: &[String]) -> HostServerOptions { let mut options = HostServerOptions::new(); let mut chunks = args.chunks_exact(2); - if !chunks.remainder().is_empty() { - panic!("Unexepected number of arguments") - } + assert!(chunks.remainder().is_empty(), "Unexepected number of arguments"); while let Some([cmd, arg]) = chunks.next() { options.host.parse(cmd, arg); options.enclave.parse(cmd, arg); diff --git a/qos-host/src/lib.rs b/qos-host/src/lib.rs index a7f217f7..793ec680 100644 --- a/qos-host/src/lib.rs +++ b/qos-host/src/lib.rs @@ -49,7 +49,7 @@ pub struct HostServer { impl HostServer { /// Create a new [`HostServer`]. - pub fn new(enclave_addr: SocketAddress, ip: [u8; 4], port: u16) -> Self { + #[must_use] pub fn new(enclave_addr: SocketAddress, ip: [u8; 4], port: u16) -> Self { Self { addr: SocketAddr::from((ip, port)), enclave_addr } } From 61258a9f2ff973f8ccb4c4b603529ff508a2248b Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Tue, 14 Jun 2022 23:02:54 -0700 Subject: [PATCH 22/25] Add clippy::pedantic to crypto --- qos-core/src/lib.rs | 2 +- qos-core/src/protocol/boot.rs | 6 ++++-- qos-core/src/server.rs | 1 + qos-crypto/src/lib.rs | 13 +++++++------ qos-crypto/src/shamir.rs | 2 +- qos-host/src/lib.rs | 1 + 6 files changed, 15 insertions(+), 10 deletions(-) diff --git a/qos-core/src/lib.rs b/qos-core/src/lib.rs index f5c57dfc..27c21ca4 100644 --- a/qos-core/src/lib.rs +++ b/qos-core/src/lib.rs @@ -20,7 +20,7 @@ pub mod io; /// QuorumOS protocol details pub mod protocol; /// Basic socket server -mod server; +pub mod server; /// Path to Quorum Key secret. #[cfg(not(feature = "vm"))] diff --git a/qos-core/src/protocol/boot.rs b/qos-core/src/protocol/boot.rs index c95edf5d..fc1efe87 100644 --- a/qos-core/src/protocol/boot.rs +++ b/qos-core/src/protocol/boot.rs @@ -235,8 +235,10 @@ mod test { ]; let manifest = Manifest { - nonce: 420, - namespace: "vape lord".to_string(), + namespace: Namespace { + nonce: 420, + name: "vape lord".to_string(), + }, enclave: NitroConfig { vsock_cid: 69, vsock_port: 42069, diff --git a/qos-core/src/server.rs b/qos-core/src/server.rs index 39d84908..4696da4e 100644 --- a/qos-core/src/server.rs +++ b/qos-core/src/server.rs @@ -47,6 +47,7 @@ impl SocketServer { } } +/// Something that can process requests. pub trait Routable { /// Process an incoming request and return a response. /// diff --git a/qos-crypto/src/lib.rs b/qos-crypto/src/lib.rs index 4fb92709..c682df1d 100644 --- a/qos-crypto/src/lib.rs +++ b/qos-crypto/src/lib.rs @@ -1,8 +1,9 @@ -//! Cryptographic primitves for use with QuorumOS. +//! Cryptographic primitves for use with `QuorumOS`. #![forbid(unsafe_code)] #![deny(clippy::all)] #![warn(missing_docs, clippy::pedantic)] +#![allow(clippy::missing_errors_doc)] // TODO: Audit encryption strategy // This file implements an envelope encryption strategy using RSA and AES 256 @@ -27,7 +28,7 @@ mod shamir; pub use shamir::*; -/// Standard length for QuorumOS RSA keys, specified in bits. +/// Standard length for `QuorumOS` RSA keys, specified in bits. pub const RSA_KEY_LEN: u32 = 4096; /// Errors for this crate. @@ -58,7 +59,7 @@ impl From for CryptoError { } /// Create a SHA256 hash digest of `buf`. -pub fn sha_256(buf: &[u8]) -> [u8; 32] { +#[must_use] pub fn sha_256(buf: &[u8]) -> [u8; 32] { let mut hasher = openssl::sha::Sha256::new(); hasher.update(buf); hasher.finish() @@ -73,7 +74,7 @@ pub struct RsaPair { impl RsaPair { /// Get the public key of this pair. - pub fn public_key(&self) -> &RsaPub { + #[must_use] pub fn public_key(&self) -> &RsaPub { &self.public_key } @@ -146,7 +147,7 @@ impl RsaPair { .map_err(CryptoError::from) } - /// Envelope encrypt using the RsaPair's associated RsaPub + /// Envelope encrypt using the `RsaPair`'s associated `RsaPub` pub fn envelope_encrypt( &self, data: &[u8], @@ -277,7 +278,7 @@ impl RsaPub { let encrypted_data = symm::encrypt(cipher, &key, Some(&iv), data)?; let encrypted_symm_key = self.encrypt(&key)?; - let envelope = Envelope { encrypted_data, encrypted_symm_key, iv }; + let envelope = Envelope { encrypted_symm_key, encrypted_data, iv }; Ok(envelope.try_to_vec().expect("`Envelope` impls serialization")) } } diff --git a/qos-crypto/src/shamir.rs b/qos-crypto/src/shamir.rs index 0549c8a5..dd30e142 100644 --- a/qos-crypto/src/shamir.rs +++ b/qos-crypto/src/shamir.rs @@ -160,7 +160,7 @@ fn gf256_interpolate(xs: &[u8], ys: &[u8]) -> u8 { } /// Generate n shares requiring k shares to reconstruct. -pub fn shares_generate(secret: &[u8], n: usize, k: usize) -> Vec> { +#[must_use] pub fn shares_generate(secret: &[u8], n: usize, k: usize) -> Vec> { let mut shares = vec![vec![]; n]; // we need to store x for each point somewhere, so just prepend diff --git a/qos-host/src/lib.rs b/qos-host/src/lib.rs index 793ec680..06a922bb 100644 --- a/qos-host/src/lib.rs +++ b/qos-host/src/lib.rs @@ -15,6 +15,7 @@ #![forbid(unsafe_code)] #![deny(clippy::all)] #![warn(missing_docs, clippy::pedantic)] +#![allow(clippy::missing_errors_doc)] use std::{net::SocketAddr, sync::Arc}; From 89d77305037e51e74a386c4c379dc46e6a689243 Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Tue, 14 Jun 2022 23:21:20 -0700 Subject: [PATCH 23/25] Add clippy::pedantic to core --- qos-core/src/cli.rs | 70 +++++++++++++------------ qos-core/src/client.rs | 4 +- qos-core/src/coordinator.rs | 8 ++- qos-core/src/io/stream.rs | 12 +++-- qos-core/src/lib.rs | 5 +- qos-core/src/protocol/attestor/mod.rs | 2 +- qos-core/src/protocol/attestor/types.rs | 3 +- qos-core/src/protocol/boot.rs | 10 ++-- qos-core/src/protocol/genesis.rs | 8 +-- qos-core/src/protocol/mod.rs | 10 ++-- qos-host/src/lib.rs | 2 +- 11 files changed, 72 insertions(+), 62 deletions(-) diff --git a/qos-core/src/cli.rs b/qos-core/src/cli.rs index 544b9078..c19ea1ba 100644 --- a/qos-core/src/cli.rs +++ b/qos-core/src/cli.rs @@ -21,7 +21,7 @@ pub struct EnclaveOptions { impl EnclaveOptions { /// Create a new instance of [`Self`] with some defaults. - pub fn new() -> Self { + #[must_use] pub fn new() -> Self { Self { cid: None, port: None, @@ -33,13 +33,11 @@ impl EnclaveOptions { } } - fn from_args(args: Vec) -> EnclaveOptions { + fn from_args(args: &[String]) -> EnclaveOptions { let mut options = EnclaveOptions::new(); let mut chunks = args.chunks_exact(2); - if !chunks.remainder().is_empty() { - panic!("Unexepected number of arguments") - } + assert!(chunks.remainder().is_empty(), "Unexepected number of arguments"); while let Some([cmd, arg]) = chunks.next() { options.parse(cmd, arg); @@ -83,36 +81,40 @@ impl EnclaveOptions { fn parse_usock(&mut self, cmd: &str, arg: &str) { if cmd == "--usock" { - self.usock = Some(arg.to_string()) + self.usock = Some(arg.to_string()); } } fn parse_mock(&mut self, cmd: &str, arg: &str) { if cmd == "--mock" { - self.mock = arg == "true" + self.mock = arg == "true"; }; } fn parse_secret_file(&mut self, cmd: &str, arg: &str) { if cmd == "--secret-file" { - self.secret_file = arg.to_owned() + self.secret_file = arg.to_owned(); } } fn parse_pivot_file(&mut self, cmd: &str, arg: &str) { if cmd == "--pivot-file" { - self.pivot_file = arg.to_owned() + self.pivot_file = arg.to_owned(); } } fn parse_ephemeral_key_file(&mut self, cmd: &str, arg: &str) { if cmd == "--ephemeral-key-file" { - self.ephemeral_key_file = arg.to_owned() + self.ephemeral_key_file = arg.to_owned(); } } /// Get the `SocketAddress` for the enclave server. - pub fn addr(&self) -> SocketAddress { + /// + /// # Panics + /// + /// Panics if the options are not valid for exactly one of unix or vsock. + #[must_use] pub fn addr(&self) -> SocketAddress { match self.clone() { #[cfg(feature = "vm")] EnclaveOptions { @@ -126,7 +128,7 @@ impl EnclaveOptions { } /// Get the [`NsmProvider`] - pub fn nsm(&self) -> Box { + #[must_use] pub fn nsm(&self) -> Box { if self.mock { Box::new(MockNsm) } else { @@ -135,24 +137,24 @@ impl EnclaveOptions { } /// Defaults to [`SECRET_FILE`] if not explicitly specified - pub fn secret_file(&self) -> String { + #[must_use] pub fn secret_file(&self) -> String { self.secret_file.clone() } /// Defaults to [`PIVOT_FILE`] if not explicitly specified - pub fn pivot_file(&self) -> String { + #[must_use] pub fn pivot_file(&self) -> String { self.pivot_file.clone() } /// Defaults to [`EPHEMERAL_KEY_FILE`] if not explicitly specified - pub fn ephemeral_key_file(&self) -> String { + #[must_use] pub fn ephemeral_key_file(&self) -> String { self.ephemeral_key_file.clone() } } impl From> for EnclaveOptions { fn from(args: Vec) -> Self { - Self::from_args(args) + Self::from_args(&args) } } @@ -164,7 +166,7 @@ impl CLI { let mut args: Vec = env::args().collect(); args.remove(0); - let options = EnclaveOptions::from_args(args); + let options = EnclaveOptions::from_args(&args); Coordinator::execute(options); } @@ -179,11 +181,11 @@ mod test { #[test] fn parse_cid_and_port() { - let args = vec!["--cid", "6", "--port", "3999"] + let args: Vec<_> = vec!["--cid", "6", "--port", "3999"] .into_iter() .map(String::from) .collect(); - let options = EnclaveOptions::from_args(args); + let options = EnclaveOptions::from_args(&args); assert_eq!( options, @@ -196,7 +198,7 @@ mod test { secret_file: SECRET_FILE.to_string(), ephemeral_key_file: EPHEMERAL_KEY_FILE.to_string(), } - ) + ); } #[test] @@ -204,7 +206,7 @@ mod test { let pivot = "pivot.file"; let secret = "secret.file"; let ephemeral = "ephemeral.file"; - let args = vec![ + let args: Vec<_> = vec![ "--cid", "6", "--port", @@ -219,7 +221,7 @@ mod test { .into_iter() .map(String::from) .collect(); - let options = EnclaveOptions::from_args(args); + let options = EnclaveOptions::from_args(&args); assert_eq!( options, @@ -232,16 +234,16 @@ mod test { secret_file: secret.to_string(), ephemeral_key_file: ephemeral.to_string() } - ) + ); } #[test] fn parse_usock() { - let args = vec!["--usock", "./test.sock"] + let args: Vec<_> = vec!["--usock", "./test.sock"] .into_iter() .map(String::from) .collect(); - let options = EnclaveOptions::from_args(args); + let options = EnclaveOptions::from_args(&args); assert_eq!( options, @@ -254,7 +256,7 @@ mod test { secret_file: SECRET_FILE.to_string(), ephemeral_key_file: EPHEMERAL_KEY_FILE.to_string() } - ) + ); } #[test] @@ -269,7 +271,7 @@ mod test { secret_file: SECRET_FILE.to_string(), ephemeral_key_file: EPHEMERAL_KEY_FILE.to_string(), }; - options.addr(); + let _ = options.addr(); } #[test] @@ -284,7 +286,7 @@ mod test { secret_file: SECRET_FILE.to_string(), ephemeral_key_file: EPHEMERAL_KEY_FILE.to_string(), }; - options.addr(); + let _ = options.addr(); } #[test] @@ -303,7 +305,7 @@ mod test { _ => { panic!("Can't build SocketAddress:Vsock from options") } - } + }; } #[test] @@ -323,26 +325,26 @@ mod test { _ => { panic!("Can't build SocketAddress:Unix from options") } - } + }; } #[test] #[should_panic] fn panic_when_mistyped_cid() { - let args = vec!["--cid", "notanint", "--port", "3999"] + let args: Vec<_> = vec!["--cid", "notanint", "--port", "3999"] .into_iter() .map(String::from) .collect(); - let _options = EnclaveOptions::from_args(args); + let _options = EnclaveOptions::from_args(&args); } #[test] #[should_panic] fn panic_when_mistyped_port() { - let args = vec!["--cid", "123", "--port", "notanint"] + let args: Vec<_> = vec!["--cid", "123", "--port", "notanint"] .into_iter() .map(String::from) .collect(); - let _options = EnclaveOptions::from_args(args); + let _options = EnclaveOptions::from_args(&args); } } diff --git a/qos-core/src/client.rs b/qos-core/src/client.rs index 791d0a0c..e64085ee 100644 --- a/qos-core/src/client.rs +++ b/qos-core/src/client.rs @@ -43,14 +43,14 @@ pub struct Client { impl Client { /// Create a new client. - pub fn new(addr: SocketAddress) -> Self { + #[must_use] pub fn new(addr: SocketAddress) -> Self { Self { addr } } /// Send a [`ProtocolMsg`] and wait for the response. pub fn send( &self, - request: ProtocolMsg, + request: &ProtocolMsg, ) -> Result { let stream = Stream::connect(&self.addr)?; diff --git a/qos-core/src/coordinator.rs b/qos-core/src/coordinator.rs index 94ee5c6b..045bd7e6 100644 --- a/qos-core/src/coordinator.rs +++ b/qos-core/src/coordinator.rs @@ -13,6 +13,12 @@ use crate::{cli::EnclaveOptions, protocol::Executor, server::SocketServer}; pub struct Coordinator; impl Coordinator { /// Run the coordinator. + /// + /// # Panics + /// + /// - If any enclave options are incorrect + /// - If spawning the pivot error. + /// - If waiting for the pivot errors. pub fn execute(opts: EnclaveOptions) { let secret_file = opts.secret_file(); let pivot_file = opts.pivot_file(); @@ -55,7 +61,7 @@ impl Coordinator { println!( "Pivot executable exited with a non zero status: {}", status - ) + ); } println!("Coordinator exiting ..."); diff --git a/qos-core/src/io/stream.rs b/qos-core/src/io/stream.rs index 87ed326a..4c35f910 100644 --- a/qos-core/src/io/stream.rs +++ b/qos-core/src/io/stream.rs @@ -32,7 +32,11 @@ pub enum SocketAddress { impl SocketAddress { /// Create a new Unix socket. - pub fn new_unix(path: &str) -> Self { + /// + /// # Panics + /// + /// Panics if `nix::sys::socket::UnixAddr::new` panics. + #[must_use] pub fn new_unix(path: &str) -> Self { let addr = UnixAddr::new(path).unwrap(); Self::Unix(addr) } @@ -231,7 +235,7 @@ impl Listener { if let SocketAddress::Unix(addr) = addr { if let Some(path) = addr.path() { if path.exists() { - let _ = remove_file(path); + drop(remove_file(path)); } } } @@ -252,7 +256,7 @@ impl Drop for Listener { // connection has been shutdown let _ = shutdown(self.fd, Shutdown::Both); let _ = close(self.fd); - Self::clean(&self.addr) + Self::clean(&self.addr); } } @@ -316,7 +320,7 @@ mod test { let client = Stream::connect(&addr).unwrap(); let data = vec![1, 2, 3, 4, 5, 6, 6, 6]; - let _ = client.send(&data).unwrap(); + client.send(&data).unwrap(); let resp = client.recv().unwrap(); assert_eq!(data, resp); diff --git a/qos-core/src/lib.rs b/qos-core/src/lib.rs index 27c21ca4..f7b8e979 100644 --- a/qos-core/src/lib.rs +++ b/qos-core/src/lib.rs @@ -7,7 +7,8 @@ #![forbid(unsafe_code)] #![deny(clippy::all)] -#![warn(missing_docs)] +#![warn(missing_docs, clippy::pedantic)] +#![allow(clippy::missing_errors_doc, clippy::module_name_repetitions)] /// Command line interface pub mod cli; @@ -17,7 +18,7 @@ pub mod client; pub mod coordinator; /// Basic IO capabilities pub mod io; -/// QuorumOS protocol details +/// `QuorumOS` protocol details pub mod protocol; /// Basic socket server pub mod server; diff --git a/qos-core/src/protocol/attestor/mod.rs b/qos-core/src/protocol/attestor/mod.rs index 11e20e34..7700634d 100644 --- a/qos-core/src/protocol/attestor/mod.rs +++ b/qos-core/src/protocol/attestor/mod.rs @@ -47,6 +47,6 @@ impl NsmProvider for Nsm { } fn nsm_exit(&self, fd: i32) { - nsm::driver::nsm_exit(fd) + nsm::driver::nsm_exit(fd); } } diff --git a/qos-core/src/protocol/attestor/types.rs b/qos-core/src/protocol/attestor/types.rs index 83dc9e3d..19681cb5 100644 --- a/qos-core/src/protocol/attestor/types.rs +++ b/qos-core/src/protocol/attestor/types.rs @@ -267,6 +267,7 @@ impl From for NsmResponse { R::DescribePCR { lock, data } => Self::DescribePCR { lock, data }, R::ExtendPCR { data } => Self::ExtendPCR { data }, R::LockPCR => Self::LockPCR, + R::LockPCRs => Self::LockPCRs, R::DescribeNSM { version_major, version_minor, @@ -299,6 +300,7 @@ impl From for nsm::api::Response { R::DescribePCR { lock, data } => Self::DescribePCR { lock, data }, R::ExtendPCR { data } => Self::ExtendPCR { data }, R::LockPCR => Self::LockPCR, + R::LockPCRs => Self::LockPCRs, R::DescribeNSM { version_major, version_minor, @@ -319,7 +321,6 @@ impl From for nsm::api::Response { R::Attestation { document } => Self::Attestation { document }, R::GetRandom { random } => Self::GetRandom { random }, R::Error(e) => Self::Error(e.into()), - _ => Self::Error(ErrorCode::InternalError), } } } diff --git a/qos-core/src/protocol/boot.rs b/qos-core/src/protocol/boot.rs index fc1efe87..97a348ef 100644 --- a/qos-core/src/protocol/boot.rs +++ b/qos-core/src/protocol/boot.rs @@ -132,7 +132,7 @@ pub struct ManifestEnvelope { impl ManifestEnvelope { /// Check if the encapsulated manifest has K valid approvals. pub fn check_approvals(&self) -> Result<(), ProtocolError> { - for approval in self.approvals.iter() { + for approval in &self.approvals { let pub_key = RsaPub::from_der(&approval.member.pub_key) .map_err(|_| ProtocolError::CryptoError)?; @@ -156,7 +156,7 @@ impl ManifestEnvelope { pub(super) fn boot_standard( state: &mut ProtocolState, - manifest_envelope: ManifestEnvelope, + manifest_envelope: &ManifestEnvelope, pivot: &Vec, ) -> Result { use std::os::unix::fs::PermissionsExt as _; @@ -296,7 +296,7 @@ mod test { ); let _nsm_resposne = - boot_standard(&mut protocol_state, manifest_envelope, &pivot) + boot_standard(&mut protocol_state, &manifest_envelope, &pivot) .unwrap(); assert!(Path::new(&pivot_file).exists()); @@ -336,7 +336,7 @@ mod test { ); let nsm_resposne = - boot_standard(&mut protocol_state, manifest_envelope, &pivot); + boot_standard(&mut protocol_state, &manifest_envelope, &pivot); assert!(nsm_resposne.is_err()); } @@ -367,7 +367,7 @@ mod test { ); let nsm_resposne = - boot_standard(&mut protocol_state, manifest_envelope, &pivot); + boot_standard(&mut protocol_state, &manifest_envelope, &pivot); assert!(nsm_resposne.is_err()); } diff --git a/qos-core/src/protocol/genesis.rs b/qos-core/src/protocol/genesis.rs index 162da42a..2cc1d90c 100644 --- a/qos-core/src/protocol/genesis.rs +++ b/qos-core/src/protocol/genesis.rs @@ -78,7 +78,7 @@ pub struct GenesisOutput { impl GenesisOutput { /// Canonical hash of [`Self`] - pub fn hash(&self) -> Hash256 { + #[must_use] pub fn hash(&self) -> Hash256 { qos_crypto::sha_256( &self.try_to_vec().expect("`Manifest` serializes with cbor"), ) @@ -122,11 +122,7 @@ pub(super) fn boot_genesis( let encrypted_quorum_key_share = personal_pair.envelope_encrypt(&share)?; - member_outputs.push(GenesisMemberOutput { - setup_member, - encrypted_personal_key, - encrypted_quorum_key_share, - }); + member_outputs.push(GenesisMemberOutput { setup_member, encrypted_quorum_key_share, encrypted_personal_key }); } let genesis_output = GenesisOutput { diff --git a/qos-core/src/protocol/mod.rs b/qos-core/src/protocol/mod.rs index 2a69f165..cb7db6fc 100644 --- a/qos-core/src/protocol/mod.rs +++ b/qos-core/src/protocol/mod.rs @@ -22,7 +22,7 @@ pub use genesis::{ GenesisMemberOutput, GenesisOutput, GenesisSet, SetupMember, }; pub use msg::*; -use provisioner::*; +use provisioner::SecretProvisioner; use crate::server; @@ -128,7 +128,7 @@ pub struct Executor { impl Executor { /// Create a new `Self`. - pub fn new( + #[must_use] pub fn new( attestor: Box, secret_file: String, pivot_file: String, @@ -187,7 +187,7 @@ impl server::Routable for Executor { Err(_) => return err_resp(), }; - for handler in self.routes().iter() { + for handler in &self.routes() { match handler(&msg_req, &mut self.state) { Some(msg_resp) => { return msg_resp @@ -208,7 +208,7 @@ impl server::Routable for Executor { mod handlers { use qos_crypto::RsaPair; - use super::*; + use super::{BootInstruction, Load, NsmRequest, Permissions, PermissionsExt, ProtocolMsg, ProtocolPhase, ProtocolState, boot, genesis, set_permissions}; /// Unwrap an ok or return early with a generic error. /// TODO: this try and pass through the returned error @@ -370,7 +370,7 @@ mod handlers { }) => { let nsm_response = ok_or_err!(boot::boot_standard( state, - *manifest_envelope.clone(), + manifest_envelope, pivot )); Some(ProtocolMsg::BootStandardResponse(nsm_response)) diff --git a/qos-host/src/lib.rs b/qos-host/src/lib.rs index 06a922bb..fc4ae328 100644 --- a/qos-host/src/lib.rs +++ b/qos-host/src/lib.rs @@ -119,7 +119,7 @@ impl HostServer { .expect("ProtocolMsg can always serialize. qed."), ) } - Ok(request) => state.enclave_client.send(request), + Ok(request) => state.enclave_client.send(&request), }; match response { From 154072949757124090b589753b9e41e0ae7e1f7d Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Tue, 14 Jun 2022 23:35:02 -0700 Subject: [PATCH 24/25] Add clippy::pedantic to client --- qos-client/src/attest/nitro/mod.rs | 10 +-- .../src/attest/nitro/syntactic_validation.rs | 12 ++-- qos-client/src/cli.rs | 66 +++++++++---------- qos-client/src/lib.rs | 11 +++- qos-test/tests/coordinator.rs | 6 +- qos-test/tests/load.rs | 2 +- qos-test/tests/provision.rs | 10 +-- 7 files changed, 58 insertions(+), 59 deletions(-) diff --git a/qos-client/src/attest/nitro/mod.rs b/qos-client/src/attest/nitro/mod.rs index de7c0a2e..fc8aafc9 100644 --- a/qos-client/src/attest/nitro/mod.rs +++ b/qos-client/src/attest/nitro/mod.rs @@ -21,14 +21,14 @@ 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. #[cfg(any(feature = "mock", test))] -pub const MOCK_SECONDS_SINCE_EPOCH: u64 = 1652756400; +pub const MOCK_SECONDS_SINCE_EPOCH: u64 = 1_652_756_400; /// 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 const AWS_ROOT_CERT_PEM: &[u8] = std::include_bytes!("./static/aws_root_cert.pem"); @@ -145,10 +145,10 @@ fn verify_cose_sign1_sig( let is_valid_sig = cose_sign1 .verify_signature(&ee_cert_pub_key) .map_err(|_| AttestError::InvalidCOSESign1Signature)?; - if !is_valid_sig { - Err(AttestError::InvalidCOSESign1Signature) - } else { + if is_valid_sig { Ok(()) + } else { + Err(AttestError::InvalidCOSESign1Signature) } } diff --git a/qos-client/src/attest/nitro/syntactic_validation.rs b/qos-client/src/attest/nitro/syntactic_validation.rs index abbb5c67..48fe5a6e 100644 --- a/qos-client/src/attest/nitro/syntactic_validation.rs +++ b/qos-client/src/attest/nitro/syntactic_validation.rs @@ -4,7 +4,7 @@ use std::collections::BTreeMap; use aws_nitro_enclaves_nsm_api::api::Digest; -use super::*; +use super::{AttestError, ByteBuf}; const MIN_PCR_COUNT: usize = 1; const MAX_PRC_COUNT: usize = 32; @@ -58,10 +58,10 @@ pub(super) fn cabundle(cabundle: &[ByteBuf]) -> Result<(), AttestError> { } /// Mandatory field pub(super) fn digest(d: Digest) -> Result<(), AttestError> { - if d != Digest::SHA384 { - Err(AttestError::InvalidDigest) - } else { + if d == Digest::SHA384 { Ok(()) + } else { + Err(AttestError::InvalidDigest) } } /// Mandatory field @@ -77,7 +77,7 @@ pub(super) fn public_key(pub_key: &Option) -> Result<(), AttestError> { if let Some(key) = pub_key { (key.len() >= MIN_PUB_KEY_LEN && key.len() <= MAX_PUB_KEY_LEN) .then(|| ()) - .ok_or(AttestError::InvalidPubKey)? + .ok_or(AttestError::InvalidPubKey)?; } Ok(()) @@ -93,7 +93,7 @@ pub(super) fn nonce(n: &Option) -> Result<(), AttestError> { fn bytes_512(val: &Option) -> Result<(), AttestError> { if let Some(val) = val { - (val.len() <= 512).then(|| ()).ok_or(AttestError::InvalidBytes)? + (val.len() <= 512).then(|| ()).ok_or(AttestError::InvalidBytes)?; } Ok(()) diff --git a/qos-client/src/cli.rs b/qos-client/src/cli.rs index 87b6221a..3f3ee87a 100644 --- a/qos-client/src/cli.rs +++ b/qos-client/src/cli.rs @@ -1,4 +1,4 @@ -//! QuorumOS client command line interface. +//! `QuorumOS` client command line interface. use std::env; @@ -22,7 +22,7 @@ enum Command { BootGenesis, } impl Command { - fn run(&self, options: ClientOptions) { + fn run(&self, options: &ClientOptions) { match self { Self::Health => handlers::health(options), Self::Echo => handlers::echo(options), @@ -71,20 +71,18 @@ impl ClientOptions { }; let mut chunks = args.chunks_exact(2); - if !chunks.remainder().is_empty() { - panic!("Unexpected number of arguments"); - } + assert!(chunks.remainder().is_empty(), "Unexpected number of arguments"); while let Some([cmd, arg]) = chunks.next() { options.host.parse(cmd, arg); match options.cmd { Command::Echo => options.echo.parse(cmd, arg), Command::GenerateSetupKey => { - options.generate_setup_key.parse(cmd, arg) + options.generate_setup_key.parse(cmd, arg); } - Command::Health => {} - Command::DescribeNsm => {} - Command::MockAttestation => {} + Command::Health | + Command::DescribeNsm | + Command::MockAttestation | Command::Attestation => {} Command::BootGenesis => options.boot_genesis.parse(cmd, arg), } @@ -95,7 +93,7 @@ impl ClientOptions { /// Run the given given command. pub fn run(self) { - self.cmd.clone().run(self) + self.cmd.run(&self); } /// Helper function to extract the command from arguments. @@ -121,7 +119,7 @@ impl EchoOptions { } fn parse(&mut self, cmd: &str, arg: &str) { if cmd == "--data" { - self.data = Some(arg.to_string()) + self.data = Some(arg.to_string()); }; } fn data(&self) -> String { @@ -175,7 +173,7 @@ impl BootGenesisOptions { self.threshold = Some(arg.parse::().expect( "Could not parse provided value for `--threshold`", - )) + )); } "--out-dir" => self.out_dir = Some(arg.to_string()), _ => {} @@ -211,10 +209,10 @@ mod handlers { }; use qos_crypto::RsaPub; - use super::*; + use super::{AWS_ROOT_CERT_PEM, BorshSerialize, ClientOptions, Echo, ProtocolMsg, RsaPair, attestation_doc_from_der, cert_from_pem}; use crate::{attest, request}; - pub(super) fn health(options: ClientOptions) { + pub(super) fn health(options: &ClientOptions) { let path = &options.host.path("health"); if let Ok(response) = request::get(path) { println!("{}", response); @@ -223,11 +221,11 @@ mod handlers { } } - pub(super) fn echo(options: ClientOptions) { + pub(super) fn echo(options: &ClientOptions) { let path = &options.host.path("message"); let msg = options.echo.data().into_bytes(); let response = - request::post(path, ProtocolMsg::EchoRequest(Echo { data: msg })) + request::post(path, &ProtocolMsg::EchoRequest(Echo { data: msg })) .map_err(|e| println!("{:?}", e)) .expect("Echo message failed"); @@ -243,27 +241,27 @@ mod handlers { }; } - pub(super) fn describe_nsm(options: ClientOptions) { + pub(super) fn describe_nsm(options: &ClientOptions) { let path = &options.host.path("message"); match request::post( path, - ProtocolMsg::NsmRequest(NsmRequest::DescribeNSM), + &ProtocolMsg::NsmRequest(NsmRequest::DescribeNSM), ) .map_err(|e| println!("{:?}", e)) .expect("Attestation request failed") { ProtocolMsg::NsmResponse(description) => { - println!("{:#?}", description) + println!("{:#?}", description); } other => panic!("Unexpected response {:?}", other), } } - pub(super) fn attestation(options: ClientOptions) { + pub(super) fn attestation(options: &ClientOptions) { let path = &options.host.path("message"); let response = request::post( path, - ProtocolMsg::NsmRequest(NsmRequest::Attestation { + &ProtocolMsg::NsmRequest(NsmRequest::Attestation { user_data: None, nonce: None, public_key: None, @@ -294,12 +292,12 @@ mod handlers { } } - pub(super) fn mock_attestation(options: ClientOptions) { + pub(super) fn mock_attestation(options: &ClientOptions) { let path = &options.host.path("message"); let response = request::post( path, - ProtocolMsg::NsmRequest(NsmRequest::Attestation { + &ProtocolMsg::NsmRequest(NsmRequest::Attestation { user_data: None, nonce: None, public_key: Some( @@ -330,15 +328,13 @@ mod handlers { } } - pub(super) fn generate_setup_key(options: ClientOptions) { + pub(super) fn generate_setup_key(options: &ClientOptions) { let alias = options.generate_setup_key.alias(); let namespace = options.generate_setup_key.namespace(); let key_dir = options.generate_setup_key.key_dir(); let key_dir_path = std::path::Path::new(&key_dir); - if !key_dir_path.is_dir() { - panic!("Provided `--key-dir` does not exist is not valid"); - } + assert!(key_dir_path.is_dir(), "Provided `--key-dir` does not exist is not valid"); let setup_key = RsaPair::generate().expect("RSA key generation failed"); // Write the setup key secret @@ -367,10 +363,10 @@ mod handlers { // TODO: verify AWS_ROOT_CERT_PEM against a checksum // TODO: verify PCRs - pub(super) fn boot_genesis(options: ClientOptions) { + pub(super) fn boot_genesis(options: &ClientOptions) { let uri = &options.host.path("message"); - let genesis_set = create_genesis_set(&options); + let genesis_set = create_genesis_set(options); let output_dir = options.boot_genesis.out_dir(); let output_dir = Path::new(&output_dir); @@ -379,7 +375,7 @@ mod handlers { }); let (nsm_response, genesis_output) = - match request::post(uri, req).unwrap() { + match request::post(uri, &req).unwrap() { ProtocolMsg::BootGenesisResponse { nsm_response, genesis_output, @@ -410,7 +406,7 @@ mod handlers { .duration_since(std::time::UNIX_EPOCH) .unwrap() .as_secs(); - let _ = attestation_doc_from_der( + attestation_doc_from_der( &cose_sign1_der, &cert_from_pem(AWS_ROOT_CERT_PEM) .expect("AWS ROOT CERT is not valid PEM"), @@ -462,9 +458,7 @@ mod handlers { // Get all the files in the key directory let key_files = { let key_dir_path = std::path::Path::new(&key_dir); - if !key_dir_path.is_dir() { - panic!("Provided path is not a valid directory"); - }; + assert!(key_dir_path.is_dir(), "Provided path is not a valid directory"); std::fs::read_dir(key_dir_path) .expect("Failed to read key directory") }; @@ -475,7 +469,7 @@ mod handlers { .map(|maybe_key_path| maybe_key_path.unwrap().path()) .filter_map(|key_path| { let file_name = - key_path.file_name().map(|f| f.to_string_lossy()).unwrap(); + key_path.file_name().map(std::ffi::OsStr::to_string_lossy).unwrap(); let split: Vec<_> = file_name.split('.').collect(); // TODO: do we want to dissallow having anything in this folder @@ -489,7 +483,7 @@ mod handlers { .expect("Failed to read in rsa pub key."); Some(SetupMember { - alias: split.get(0).unwrap().to_string(), + alias: (*split.get(0).unwrap()).to_string(), pub_key: public_key.public_key_to_der().unwrap(), }) }) diff --git a/qos-client/src/lib.rs b/qos-client/src/lib.rs index ea7ecea9..c6441d2c 100644 --- a/qos-client/src/lib.rs +++ b/qos-client/src/lib.rs @@ -1,8 +1,9 @@ -//! CLI Client for interacting with QuorumOS enclave and host. +//! CLI Client for interacting with `QuorumOS` enclave and host. #![forbid(unsafe_code)] #![deny(clippy::all)] -#![warn(missing_docs)] +#![warn(missing_docs, clippy::pedantic)] +#![allow(clippy::missing_errors_doc, clippy::module_name_repetitions)] pub mod attest; pub mod cli; @@ -17,7 +18,7 @@ pub mod request { const MAX_SIZE: u64 = u32::MAX as u64; /// Post a [`qos_core::protocol::ProtocolMsg`] to the given host `url`. - pub fn post(url: &str, msg: ProtocolMsg) -> Result { + pub fn post(url: &str, msg: &ProtocolMsg) -> Result { let mut buf: Vec = vec![]; let response = ureq::post(url) @@ -40,6 +41,10 @@ pub mod request { } /// Get the resource at the given host `url`. + /// + /// # Panics + /// + /// Panics if the http request fails. pub fn get(url: &str) -> Result { ureq::get(url) .call() diff --git a/qos-test/tests/coordinator.rs b/qos-test/tests/coordinator.rs index 09369160..3133248a 100644 --- a/qos-test/tests/coordinator.rs +++ b/qos-test/tests/coordinator.rs @@ -67,7 +67,7 @@ async fn coordinator_e2e() { executable: pivot_bytes, signatures: vec![], }); - let response = request::post(&message_url, load_msg).unwrap(); + let response = request::post(&message_url, &load_msg).unwrap(); assert_eq!(response, ProtocolMsg::SuccessResponse); // -- Check that the executable got written as a file @@ -84,13 +84,13 @@ async fn coordinator_e2e() { // -- For each shard send it and expect a success response for share in all_shares.into_iter().take(k) { let provision_msg = ProtocolMsg::ProvisionRequest(Provision { share }); - let response = request::post(&message_url, provision_msg).unwrap(); + let response = request::post(&message_url, &provision_msg).unwrap(); assert_eq!(response, ProtocolMsg::SuccessResponse); } // -- Send reconstruct request to create secret file from shards let response = - request::post(&message_url, ProtocolMsg::ReconstructRequest).unwrap(); + request::post(&message_url, &ProtocolMsg::ReconstructRequest).unwrap(); assert_eq!(response, ProtocolMsg::SuccessResponse); assert!(Path::new(secret_path).exists()); diff --git a/qos-test/tests/load.rs b/qos-test/tests/load.rs index 66330148..ab74f921 100644 --- a/qos-test/tests/load.rs +++ b/qos-test/tests/load.rs @@ -100,7 +100,7 @@ fn load_e2e() { let load_request = ProtocolMsg::LoadRequest(Load { executable, signatures }); let response = - qos_client::request::post(&message_url, load_request).unwrap(); + qos_client::request::post(&message_url, &load_request).unwrap(); assert_eq!(response, ProtocolMsg::SuccessResponse); // diff --git a/qos-test/tests/provision.rs b/qos-test/tests/provision.rs index 7fa918b0..a304abf8 100644 --- a/qos-test/tests/provision.rs +++ b/qos-test/tests/provision.rs @@ -61,7 +61,7 @@ async fn provision_e2e() { // Test message endpoint let data = b"Hello, world!".to_vec(); let request = ProtocolMsg::EchoRequest(Echo { data: data.clone() }); - let response = qos_client::request::post(&message_url, request).unwrap(); + let response = qos_client::request::post(&message_url, &request).unwrap(); let expected = ProtocolMsg::EchoResponse(Echo { data }); assert_eq!(expected, response); @@ -73,19 +73,19 @@ async fn provision_e2e() { let s1 = all_shares[0].clone(); let r1 = ProtocolMsg::ProvisionRequest(Provision { share: s1 }); - let response = qos_client::request::post(&message_url, r1).unwrap(); + let response = qos_client::request::post(&message_url, &r1).unwrap(); let expected = ProtocolMsg::SuccessResponse; assert_eq!(expected, response); let s2 = all_shares[1].clone(); let r2 = ProtocolMsg::ProvisionRequest(Provision { share: s2 }); - let response = qos_client::request::post(&message_url, r2).unwrap(); + let response = qos_client::request::post(&message_url, &r2).unwrap(); let expected = ProtocolMsg::SuccessResponse; assert_eq!(expected, response); let s3 = all_shares[2].clone(); let r3 = ProtocolMsg::ProvisionRequest(Provision { share: s3 }); - let response = qos_client::request::post(&message_url, r3).unwrap(); + let response = qos_client::request::post(&message_url, &r3).unwrap(); let expected = ProtocolMsg::SuccessResponse; assert_eq!(expected, response); @@ -93,7 +93,7 @@ async fn provision_e2e() { assert!(!path.exists()); let rr = ProtocolMsg::ReconstructRequest; - let response = qos_client::request::post(&message_url, rr).unwrap(); + let response = qos_client::request::post(&message_url, &rr).unwrap(); let expected = ProtocolMsg::SuccessResponse; assert_eq!(expected, response); From ac0f61c5ca3271cd7a72a8681ad043b0498b9e42 Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Tue, 14 Jun 2022 23:35:38 -0700 Subject: [PATCH 25/25] fmt --- qos-client/src/cli.rs | 41 +++++++++++++++++++++----------- qos-core/src/cli.rs | 23 ++++++++++++------ qos-core/src/client.rs | 3 ++- qos-core/src/io/stream.rs | 3 ++- qos-core/src/protocol/boot.rs | 5 +--- qos-core/src/protocol/genesis.rs | 9 +++++-- qos-core/src/protocol/mod.rs | 8 +++++-- qos-crypto/src/lib.rs | 6 +++-- qos-crypto/src/shamir.rs | 3 ++- qos-host/src/cli.rs | 13 +++++----- qos-host/src/lib.rs | 6 +++-- 11 files changed, 78 insertions(+), 42 deletions(-) diff --git a/qos-client/src/cli.rs b/qos-client/src/cli.rs index 3f3ee87a..f33074a0 100644 --- a/qos-client/src/cli.rs +++ b/qos-client/src/cli.rs @@ -71,7 +71,10 @@ impl ClientOptions { }; let mut chunks = args.chunks_exact(2); - assert!(chunks.remainder().is_empty(), "Unexpected number of arguments"); + assert!( + chunks.remainder().is_empty(), + "Unexpected number of arguments" + ); while let Some([cmd, arg]) = chunks.next() { options.host.parse(cmd, arg); @@ -80,10 +83,10 @@ impl ClientOptions { Command::GenerateSetupKey => { options.generate_setup_key.parse(cmd, arg); } - Command::Health | - Command::DescribeNsm | - Command::MockAttestation | - Command::Attestation => {} + Command::Health + | Command::DescribeNsm + | Command::MockAttestation + | Command::Attestation => {} Command::BootGenesis => options.boot_genesis.parse(cmd, arg), } } @@ -170,10 +173,9 @@ impl BootGenesisOptions { match cmd { "--key-dir" => self.key_dir = Some(arg.to_string()), "--threshold" => { - self.threshold = - Some(arg.parse::().expect( - "Could not parse provided value for `--threshold`", - )); + self.threshold = Some(arg.parse::().expect( + "Could not parse provided value for `--threshold`", + )); } "--out-dir" => self.out_dir = Some(arg.to_string()), _ => {} @@ -209,7 +211,10 @@ mod handlers { }; use qos_crypto::RsaPub; - use super::{AWS_ROOT_CERT_PEM, BorshSerialize, ClientOptions, Echo, ProtocolMsg, RsaPair, attestation_doc_from_der, cert_from_pem}; + use super::{ + attestation_doc_from_der, cert_from_pem, BorshSerialize, ClientOptions, + Echo, ProtocolMsg, RsaPair, AWS_ROOT_CERT_PEM, + }; use crate::{attest, request}; pub(super) fn health(options: &ClientOptions) { @@ -334,7 +339,10 @@ mod handlers { let key_dir = options.generate_setup_key.key_dir(); let key_dir_path = std::path::Path::new(&key_dir); - assert!(key_dir_path.is_dir(), "Provided `--key-dir` does not exist is not valid"); + assert!( + key_dir_path.is_dir(), + "Provided `--key-dir` does not exist is not valid" + ); let setup_key = RsaPair::generate().expect("RSA key generation failed"); // Write the setup key secret @@ -458,7 +466,10 @@ mod handlers { // Get all the files in the key directory let key_files = { let key_dir_path = std::path::Path::new(&key_dir); - assert!(key_dir_path.is_dir(), "Provided path is not a valid directory"); + assert!( + key_dir_path.is_dir(), + "Provided path is not a valid directory" + ); std::fs::read_dir(key_dir_path) .expect("Failed to read key directory") }; @@ -468,8 +479,10 @@ mod handlers { let members: Vec<_> = key_files .map(|maybe_key_path| maybe_key_path.unwrap().path()) .filter_map(|key_path| { - let file_name = - key_path.file_name().map(std::ffi::OsStr::to_string_lossy).unwrap(); + let file_name = key_path + .file_name() + .map(std::ffi::OsStr::to_string_lossy) + .unwrap(); let split: Vec<_> = file_name.split('.').collect(); // TODO: do we want to dissallow having anything in this folder diff --git a/qos-core/src/cli.rs b/qos-core/src/cli.rs index c19ea1ba..ff4948a5 100644 --- a/qos-core/src/cli.rs +++ b/qos-core/src/cli.rs @@ -21,7 +21,8 @@ pub struct EnclaveOptions { impl EnclaveOptions { /// Create a new instance of [`Self`] with some defaults. - #[must_use] pub fn new() -> Self { + #[must_use] + pub fn new() -> Self { Self { cid: None, port: None, @@ -37,7 +38,10 @@ impl EnclaveOptions { let mut options = EnclaveOptions::new(); let mut chunks = args.chunks_exact(2); - assert!(chunks.remainder().is_empty(), "Unexepected number of arguments"); + assert!( + chunks.remainder().is_empty(), + "Unexepected number of arguments" + ); while let Some([cmd, arg]) = chunks.next() { options.parse(cmd, arg); @@ -114,7 +118,8 @@ impl EnclaveOptions { /// # Panics /// /// Panics if the options are not valid for exactly one of unix or vsock. - #[must_use] pub fn addr(&self) -> SocketAddress { + #[must_use] + pub fn addr(&self) -> SocketAddress { match self.clone() { #[cfg(feature = "vm")] EnclaveOptions { @@ -128,7 +133,8 @@ impl EnclaveOptions { } /// Get the [`NsmProvider`] - #[must_use] pub fn nsm(&self) -> Box { + #[must_use] + pub fn nsm(&self) -> Box { if self.mock { Box::new(MockNsm) } else { @@ -137,17 +143,20 @@ impl EnclaveOptions { } /// Defaults to [`SECRET_FILE`] if not explicitly specified - #[must_use] pub fn secret_file(&self) -> String { + #[must_use] + pub fn secret_file(&self) -> String { self.secret_file.clone() } /// Defaults to [`PIVOT_FILE`] if not explicitly specified - #[must_use] pub fn pivot_file(&self) -> String { + #[must_use] + pub fn pivot_file(&self) -> String { self.pivot_file.clone() } /// Defaults to [`EPHEMERAL_KEY_FILE`] if not explicitly specified - #[must_use] pub fn ephemeral_key_file(&self) -> String { + #[must_use] + pub fn ephemeral_key_file(&self) -> String { self.ephemeral_key_file.clone() } } diff --git a/qos-core/src/client.rs b/qos-core/src/client.rs index e64085ee..d1ac254e 100644 --- a/qos-core/src/client.rs +++ b/qos-core/src/client.rs @@ -43,7 +43,8 @@ pub struct Client { impl Client { /// Create a new client. - #[must_use] pub fn new(addr: SocketAddress) -> Self { + #[must_use] + pub fn new(addr: SocketAddress) -> Self { Self { addr } } diff --git a/qos-core/src/io/stream.rs b/qos-core/src/io/stream.rs index 4c35f910..dd0c2b12 100644 --- a/qos-core/src/io/stream.rs +++ b/qos-core/src/io/stream.rs @@ -36,7 +36,8 @@ impl SocketAddress { /// # Panics /// /// Panics if `nix::sys::socket::UnixAddr::new` panics. - #[must_use] pub fn new_unix(path: &str) -> Self { + #[must_use] + pub fn new_unix(path: &str) -> Self { let addr = UnixAddr::new(path).unwrap(); Self::Unix(addr) } diff --git a/qos-core/src/protocol/boot.rs b/qos-core/src/protocol/boot.rs index 97a348ef..9f0aa61e 100644 --- a/qos-core/src/protocol/boot.rs +++ b/qos-core/src/protocol/boot.rs @@ -235,10 +235,7 @@ mod test { ]; let manifest = Manifest { - namespace: Namespace { - nonce: 420, - name: "vape lord".to_string(), - }, + namespace: Namespace { nonce: 420, name: "vape lord".to_string() }, enclave: NitroConfig { vsock_cid: 69, vsock_port: 42069, diff --git a/qos-core/src/protocol/genesis.rs b/qos-core/src/protocol/genesis.rs index 2cc1d90c..b7432f1d 100644 --- a/qos-core/src/protocol/genesis.rs +++ b/qos-core/src/protocol/genesis.rs @@ -78,7 +78,8 @@ pub struct GenesisOutput { impl GenesisOutput { /// Canonical hash of [`Self`] - #[must_use] pub fn hash(&self) -> Hash256 { + #[must_use] + pub fn hash(&self) -> Hash256 { qos_crypto::sha_256( &self.try_to_vec().expect("`Manifest` serializes with cbor"), ) @@ -122,7 +123,11 @@ pub(super) fn boot_genesis( let encrypted_quorum_key_share = personal_pair.envelope_encrypt(&share)?; - member_outputs.push(GenesisMemberOutput { setup_member, encrypted_quorum_key_share, encrypted_personal_key }); + member_outputs.push(GenesisMemberOutput { + setup_member, + encrypted_quorum_key_share, + encrypted_personal_key, + }); } let genesis_output = GenesisOutput { diff --git a/qos-core/src/protocol/mod.rs b/qos-core/src/protocol/mod.rs index cb7db6fc..7d395dc6 100644 --- a/qos-core/src/protocol/mod.rs +++ b/qos-core/src/protocol/mod.rs @@ -128,7 +128,8 @@ pub struct Executor { impl Executor { /// Create a new `Self`. - #[must_use] pub fn new( + #[must_use] + pub fn new( attestor: Box, secret_file: String, pivot_file: String, @@ -208,7 +209,10 @@ impl server::Routable for Executor { mod handlers { use qos_crypto::RsaPair; - use super::{BootInstruction, Load, NsmRequest, Permissions, PermissionsExt, ProtocolMsg, ProtocolPhase, ProtocolState, boot, genesis, set_permissions}; + use super::{ + boot, genesis, set_permissions, BootInstruction, Load, NsmRequest, + Permissions, PermissionsExt, ProtocolMsg, ProtocolPhase, ProtocolState, + }; /// Unwrap an ok or return early with a generic error. /// TODO: this try and pass through the returned error diff --git a/qos-crypto/src/lib.rs b/qos-crypto/src/lib.rs index c682df1d..fc86917f 100644 --- a/qos-crypto/src/lib.rs +++ b/qos-crypto/src/lib.rs @@ -59,7 +59,8 @@ impl From for CryptoError { } /// Create a SHA256 hash digest of `buf`. -#[must_use] pub fn sha_256(buf: &[u8]) -> [u8; 32] { +#[must_use] +pub fn sha_256(buf: &[u8]) -> [u8; 32] { let mut hasher = openssl::sha::Sha256::new(); hasher.update(buf); hasher.finish() @@ -74,7 +75,8 @@ pub struct RsaPair { impl RsaPair { /// Get the public key of this pair. - #[must_use] pub fn public_key(&self) -> &RsaPub { + #[must_use] + pub fn public_key(&self) -> &RsaPub { &self.public_key } diff --git a/qos-crypto/src/shamir.rs b/qos-crypto/src/shamir.rs index dd30e142..3a15550f 100644 --- a/qos-crypto/src/shamir.rs +++ b/qos-crypto/src/shamir.rs @@ -160,7 +160,8 @@ fn gf256_interpolate(xs: &[u8], ys: &[u8]) -> u8 { } /// Generate n shares requiring k shares to reconstruct. -#[must_use] pub fn shares_generate(secret: &[u8], n: usize, k: usize) -> Vec> { +#[must_use] +pub fn shares_generate(secret: &[u8], n: usize, k: usize) -> Vec> { let mut shares = vec![vec![]; n]; // we need to store x for each point somewhere, so just prepend diff --git a/qos-host/src/cli.rs b/qos-host/src/cli.rs index 635644f7..64255940 100644 --- a/qos-host/src/cli.rs +++ b/qos-host/src/cli.rs @@ -32,7 +32,8 @@ pub struct HostOptions { impl HostOptions { /// Create a new instance of [`self`]. - #[must_use] pub fn new() -> Self { + #[must_use] + pub fn new() -> Self { Self::default() } @@ -41,7 +42,8 @@ impl HostOptions { /// # Panics /// /// Panics if the url cannot be parsed from options - #[must_use] pub fn url(&self) -> String { + #[must_use] + pub fn url(&self) -> String { if let Self { ip: Some(ip), port: Some(port) } = self { return format!( "http://{}.{}.{}.{}:{}", @@ -53,7 +55,8 @@ impl HostOptions { } /// Get the resource path. - #[must_use] pub fn path(&self, path: &str) -> String { + #[must_use] + pub fn path(&self, path: &str) -> String { let url = self.url(); format!("{}/{}", url, path) } @@ -119,9 +122,7 @@ impl CLI { let options = parse_args(&args); let addr = host_addr_from_options(options.host); let enclave_addr = options.enclave.addr(); - HostServer::new_with_socket_addr(enclave_addr, addr) - .serve() - .await; + HostServer::new_with_socket_addr(enclave_addr, addr).serve().await; } } diff --git a/qos-host/src/lib.rs b/qos-host/src/lib.rs index fc4ae328..60e08099 100644 --- a/qos-host/src/lib.rs +++ b/qos-host/src/lib.rs @@ -50,12 +50,14 @@ pub struct HostServer { impl HostServer { /// Create a new [`HostServer`]. - #[must_use] pub fn new(enclave_addr: SocketAddress, ip: [u8; 4], port: u16) -> Self { + #[must_use] + pub fn new(enclave_addr: SocketAddress, ip: [u8; 4], port: u16) -> Self { Self { addr: SocketAddr::from((ip, port)), enclave_addr } } /// Create a new [`HostServer`]. - #[must_use] pub fn new_with_socket_addr( + #[must_use] + pub fn new_with_socket_addr( enclave_addr: SocketAddress, addr: SocketAddr, ) -> Self {