rust: setup clippy lints and fix existing lint issues

.changelog/4170.internal.md

@@ -0,0 +1 @@
+Setup rust code lints

.github/workflows/ci-lint.yml

@@ -42,6 +42,14 @@ jobs:
         uses: actions/setup-go@v2.1.5
         with:
           go-version: "1.17.x"
+      - name: Set up Rust
+        uses: actions-rs/toolchain@v1
+        with:
+          components: clippy
+      - name: Install dependencies
+        run: |
+          sudo apt-get update
+          sudo apt-get install make libseccomp-dev protobuf-compiler
       - name: Install gitlint
         run: |
           python -m pip install gitlint
@@ -91,3 +99,8 @@ jobs:
           make lint-go-mod-tidy
         # Always run this step so that all linting errors can be seen at once.
         if: always()
+      - name: Lint rust
+        run: |
+          make lint-rust
+        # Always run this step so that all linting errors can be seen at once.
+        if: always()

Makefile

@@ -62,7 +62,16 @@ fmt-go:
 fmt: $(fmt-targets)
 
 # Lint code, commits and documentation.
-lint-targets := lint-go lint-git lint-md lint-changelog lint-docs lint-go-mod-tidy
+lint-targets := lint-rust lint-go lint-git lint-md lint-changelog lint-docs lint-go-mod-tidy
+
+lint-rust:
+	@$(ECHO) "$(CYAN)*** Running cargo clippy linters...$(OFF)"
+	@cargo clippy --all-features -- -D warnings \
+		-A clippy::upper-case-acronyms \
+		-A clippy::borrowed-box \
+		-A clippy::ptr-arg  \
+		-A clippy::large_enum_variant \
+		-A clippy::field-reassign-with-default
 
 lint-go:
 	@$(MAKE) -C go lint

client/src/enclave_rpc/client.rs

@@ -377,7 +377,7 @@ mod test {
 
     impl MockTransport {
         fn new() -> Self {
-            let rak = Arc::new(RAK::new());
+            let rak = Arc::new(RAK::default());
 
             Self {
                 rak: rak.clone(),
@@ -446,7 +446,7 @@ mod test {
             .build()
             .unwrap();
         let transport = MockTransport::new();
-        let builder = session::Builder::new();
+        let builder = session::Builder::default();
         let client = RpcClient::new(Box::new(transport.clone()), builder);
 
         // Basic call.

client/src/enclave_rpc/transport.rs

@@ -18,7 +18,7 @@ pub trait Transport: Send + Sync {
         // Frame message.
         let frame = types::Frame {
             session: session_id,
-            untrusted_plaintext: untrusted_plaintext,
+            untrusted_plaintext,
             payload: data,
         };
 

keymanager-api-common/src/api.rs

@@ -73,7 +73,7 @@ pub struct InitResponse {
 }
 
 /// Context used for the init response signature.
-pub const INIT_RESPONSE_CONTEXT: &'static [u8] = b"oasis-core/keymanager: init response";
+pub const INIT_RESPONSE_CONTEXT: &[u8] = b"oasis-core/keymanager: init response";
 
 /// Signed InitResponse.
 #[derive(Clone, cbor::Encode, cbor::Decode)]

keymanager-api-common/src/lib.rs

@@ -32,7 +32,7 @@ pub fn set_trusted_policy_signers(signers: TrustedPolicySigners) -> bool {
     true
 }
 
-const POLICY_SIGN_CONTEXT: &'static [u8] = b"oasis-core/keymanager: policy";
+const POLICY_SIGN_CONTEXT: &[u8] = b"oasis-core/keymanager: policy";
 
 impl SignedPolicySGX {
     /// Verify the signatures and return the PolicySGX, if the signatures are correct.
@@ -46,13 +46,10 @@ impl SignedPolicySGX {
                 None => return Err(KeyManagerError::PolicyInvalid),
             };
 
-            if !sig
-                .signature
-                .verify(&public_key, &POLICY_SIGN_CONTEXT, &untrusted_policy_raw)
-                .is_ok()
-            {
-                return Err(KeyManagerError::PolicyInvalidSignature);
-            }
+            sig.signature
+                .verify(&public_key, POLICY_SIGN_CONTEXT, &untrusted_policy_raw)
+                .map_err(|_| KeyManagerError::PolicyInvalidSignature)?;
+
             signers.insert(public_key);
         }
 

keymanager-client/src/client.rs

@@ -21,7 +21,7 @@ use oasis_core_runtime::{
 use super::KeyManagerClient;
 
 /// Key manager RPC endpoint.
-const KEY_MANAGER_ENDPOINT: &'static str = "key-manager";
+const KEY_MANAGER_ENDPOINT: &str = "key-manager";
 
 struct Inner {
     /// Runtime identifier for which we are going to request keys.
@@ -63,7 +63,7 @@ impl RemoteClient {
         Self::new(
             runtime_id,
             RpcClient::new_runtime(
-                session::Builder::new()
+                session::Builder::default()
                     .remote_enclaves(enclaves)
                     .local_rak(rak),
                 protocol,

keymanager-client/src/mock.rs

@@ -12,6 +12,7 @@ use oasis_core_runtime::common::crypto::signature::Signature;
 use super::KeyManagerClient;
 
 /// Mock key manager client which stores everything locally.
+#[derive(Default)]
 pub struct MockClient {
     keys: Mutex<HashMap<KeyPairId, KeyPair>>,
 }

keymanager-lib/src/kdf.rs

@@ -74,9 +74,9 @@ lazy_static! {
 #[cfg(not(target_env = "sgx"))]
 const INSECURE_SIGNING_KEY_SEED: &str = "ekiden test key manager RAK seed";
 
-const MASTER_SECRET_STORAGE_KEY: &'static [u8] = b"keymanager_master_secret";
+const MASTER_SECRET_STORAGE_KEY: &[u8] = b"keymanager_master_secret";
 const MASTER_SECRET_STORAGE_SIZE: usize = 32 + TAG_SIZE + NONCE_SIZE;
-const MASTER_SECRET_SEAL_CONTEXT: &'static [u8] = b"Ekiden Keymanager Seal master secret v0";
+const MASTER_SECRET_SEAL_CONTEXT: &[u8] = b"Ekiden Keymanager Seal master secret v0";
 
 /// Kdf, which derives key manager keys from a master secret.
 pub struct Kdf {
@@ -106,7 +106,7 @@ impl Inner {
         let mut contract_secret = self.derive_contract_secret(req)?;
 
         // Note: The `name` parameter for cSHAKE is reserved for use by NIST.
-        let mut xof = CShake::new_cshake256(&vec![], &RUNTIME_XOF_CUSTOM);
+        let mut xof = CShake::new_cshake256(&[], &RUNTIME_XOF_CUSTOM);
         xof.update(&contract_secret);
         contract_secret.zeroize();
         let mut xof = xof.xof();
@@ -207,13 +207,13 @@ impl Kdf {
             // at least once.
 
             let checksum = inner.get_checksum()?;
-            if req.checksum.len() > 0 && req.checksum != checksum {
+            if !req.checksum.is_empty() && req.checksum != checksum {
                 // The init request provided a checksum and there was a mismatch.
                 // The global key manager state disagrees with the enclave state.
                 inner.reset();
                 return Err(KeyManagerError::StateCorrupted.into());
             }
-        } else if req.checksum.len() > 0 {
+        } else if !req.checksum.is_empty() {
             // A master secret is not set, and there is a checksum in the
             // request.  An enclave somewhere, has initialized at least
             // once.
@@ -323,7 +323,7 @@ impl Kdf {
             .signer
             .as_ref()
             .unwrap()
-            .sign(&INIT_RESPONSE_CONTEXT, &body)?;
+            .sign(INIT_RESPONSE_CONTEXT, &body)?;
 
         Ok(SignedInitResponse {
             init_response,
@@ -337,9 +337,8 @@ impl Kdf {
 
         // Check to see if the cached value exists.
         let mut inner = self.inner.write().unwrap();
-        match inner.cache.get(&cache_key) {
-            Some(keys) => return Ok(keys.clone()),
-            None => {}
+        if let Some(keys) = inner.cache.get(&cache_key) {
+            return Ok(keys.clone());
         };
 
         let contract_key = inner.derive_contract_key(req)?;
@@ -469,7 +468,7 @@ impl Kdf {
     }
 
     fn new_d2() -> DeoxysII {
-        let mut seal_key = egetkey(Keypolicy::MRENCLAVE, &MASTER_SECRET_SEAL_CONTEXT);
+        let mut seal_key = egetkey(Keypolicy::MRENCLAVE, MASTER_SECRET_SEAL_CONTEXT);
         let d2 = DeoxysII::new(&seal_key);
         seal_key.zeroize();
 

keymanager-lib/src/keymanager.rs

@@ -18,7 +18,7 @@ use crate::{context, kdf::Kdf, methods, policy::Policy};
 /// Initialize the Kdf.
 fn init_kdf(req: &InitRequest, ctx: &mut RpcContext) -> Result<SignedInitResponse> {
     let policy_checksum = Policy::global().init(ctx, &req.policy)?;
-    Kdf::global().init(&req, ctx, policy_checksum)
+    Kdf::global().init(req, ctx, policy_checksum)
 }
 
 /// Initialize a keymanager with trusted policy signers.

keymanager-lib/src/methods.rs

@@ -14,7 +14,7 @@ pub fn get_or_create_keys(req: &RequestIds, ctx: &mut RpcContext) -> Result<KeyP
         let si = si.ok_or(KeyManagerError::NotAuthenticated)?;
         let their_id = &si.authenticated_avr.identity;
 
-        Policy::global().may_get_or_create_keys(their_id, &req)?;
+        Policy::global().may_get_or_create_keys(their_id, req)?;
     }
 
     Kdf::global().get_or_create_keys(req)

keymanager-lib/src/policy.rs

@@ -1,5 +1,6 @@
 //! Policy support.
 use std::{
+    cmp::Ordering,
     collections::{HashMap, HashSet},
     sync::RwLock,
 };
@@ -28,8 +29,8 @@ use crate::context::Context as KmContext;
 lazy_static! {
     static ref POLICY: Policy = Policy::new();
 }
-const POLICY_STORAGE_KEY: &'static [u8] = b"keymanager_policy";
-const POLICY_SEAL_CONTEXT: &'static [u8] = b"Ekiden Keymanager Seal policy v0";
+const POLICY_STORAGE_KEY: &[u8] = b"keymanager_policy";
+const POLICY_SEAL_CONTEXT: &[u8] = b"Ekiden Keymanager Seal policy v0";
 
 /// Policy, which manages the key manager policy.
 pub struct Policy {
@@ -57,7 +58,7 @@ impl Policy {
     }
 
     /// Initialize (or update) the policy state.
-    pub fn init(&self, ctx: &mut RpcContext, raw_policy: &Vec<u8>) -> Result<Vec<u8>> {
+    pub fn init(&self, ctx: &mut RpcContext, raw_policy: &[u8]) -> Result<Vec<u8>> {
         // If this is an insecure build, don't bother trying to apply any policy.
         if Self::unsafe_skip() {
             return Ok(vec![]);
@@ -85,25 +86,26 @@ impl Policy {
 
         // Compare the new serial number with the old serial number, ensure
         // it is greater.
-        if old_policy.serial > new_policy.serial {
-            return Err(KeyManagerError::PolicyRollback.into());
-        } else if old_policy.serial == new_policy.serial {
-            // Policy should be identical, ensure nothing has changed
-            // and just return.
-            if old_policy.checksum != new_policy.checksum {
-                return Err(KeyManagerError::PolicyChanged.into());
+        match old_policy.serial.cmp(&new_policy.serial) {
+            Ordering::Greater => Err(KeyManagerError::PolicyRollback.into()),
+            Ordering::Equal if old_policy.checksum != new_policy.checksum => {
+                // Policy should be identical.
+                Err(KeyManagerError::PolicyChanged.into())
+            }
+            Ordering::Equal => {
+                inner.policy = Some(old_policy.clone());
+                Ok(old_policy.checksum)
+            }
+            Ordering::Less => {
+                // Persist then apply the new policy.
+                Self::save_raw_policy(ctx.untrusted_local_storage, raw_policy);
+                let new_checksum = new_policy.checksum.clone();
+                inner.policy = Some(new_policy);
+
+                // Return the checksum of the newly applied policy.
+                Ok(new_checksum)
             }
-            inner.policy = Some(old_policy.clone());
-            return Ok(old_policy.checksum.clone());
         }
-
-        // Persist then apply the new policy.
-        Self::save_raw_policy(ctx.untrusted_local_storage, raw_policy);
-        let new_checksum = new_policy.checksum.clone();
-        inner.policy = Some(new_policy);
-
-        // Return the checksum of the newly applied policy.
-        Ok(new_checksum)
     }
 
     /// Check if the MRSIGNER/MRENCLAVE may query keys for the given
@@ -155,11 +157,8 @@ impl Policy {
             None => HashSet::new(),
         };
 
-        match EnclaveIdentity::current() {
-            Some(id) => {
-                src_set.insert(id);
-            }
-            None => {}
+        if let Some(id) = EnclaveIdentity::current() {
+            src_set.insert(id);
         };
 
         match src_set.is_empty() {
@@ -171,14 +170,14 @@ impl Policy {
     fn load_policy(untrusted_local: &dyn KeyValue) -> Option<CachedPolicy> {
         let ciphertext = untrusted_local.get(POLICY_STORAGE_KEY.to_vec()).unwrap();
 
-        unseal(Keypolicy::MRENCLAVE, &POLICY_SEAL_CONTEXT, &ciphertext).map(|plaintext| {
+        unseal(Keypolicy::MRENCLAVE, POLICY_SEAL_CONTEXT, &ciphertext).map(|plaintext| {
             // Deserialization failures are fatal, because it is state corruption.
             CachedPolicy::parse(&plaintext).expect("failed to deserialize persisted policy")
         })
     }
 
-    fn save_raw_policy(untrusted_local: &dyn KeyValue, raw_policy: &Vec<u8>) {
-        let ciphertext = seal(Keypolicy::MRENCLAVE, &POLICY_SEAL_CONTEXT, &raw_policy);
+    fn save_raw_policy(untrusted_local: &dyn KeyValue, raw_policy: &[u8]) {
+        let ciphertext = seal(Keypolicy::MRENCLAVE, POLICY_SEAL_CONTEXT, raw_policy);
 
         // Persist the encrypted master secret.
         untrusted_local
@@ -198,17 +197,17 @@ struct CachedPolicy {
 }
 
 impl CachedPolicy {
-    fn parse(raw: &Vec<u8>) -> Result<Self> {
+    fn parse(raw: &[u8]) -> Result<Self> {
         // Parse out the signed policy.
-        let untrusted_policy: SignedPolicySGX = cbor::from_slice(&raw)?;
+        let untrusted_policy: SignedPolicySGX = cbor::from_slice(raw)?;
         let policy = untrusted_policy.verify()?;
 
         let mut cached_policy = Self::default();
         cached_policy.serial = policy.serial;
         cached_policy.runtime_id = policy.id;
 
         let mut sha3 = Sha3::v256();
-        sha3.update(&raw);
+        sha3.update(raw);
         let mut k = [0; 32];
         sha3.finalize(&mut k);
         cached_policy.checksum = k.to_vec();

runtime-loader/bin/main.rs

@@ -43,9 +43,11 @@ fn main() {
 
     // Check if passed runtime exists.
     let filename = matches.value_of("runtime").unwrap().to_owned();
-    if !Path::new(&filename).exists() {
-        panic!("Could not find runtime: {}", filename);
-    }
+    assert!(
+        Path::new(&filename).exists(),
+        "Could not find runtime: {}",
+        filename
+    );
 
     // Decode arguments.
     let host_socket = value_t!(matches, "host-socket", String).unwrap_or_else(|e| e.exit());

runtime-loader/src/sgxs.rs

@@ -29,6 +29,7 @@ impl HostService {
     }
 }
 
+#[allow(clippy::type_complexity)]
 impl UsercallExtension for HostService {
     fn connect_stream<'future>(
         &'future self,
@@ -78,6 +79,6 @@ impl Loader for SgxsLoader {
         enclave_builder.usercall_extension(HostService::new(host_socket));
         let enclave = enclave_builder.build(&mut device)?;
 
-        Ok(enclave.run()?)
+        enclave.run()
     }
 }