Merge crypto-com#883

883: Problem (crypto-com#870): There are unfixed clippy suggestions on enclave code r=tomtau a=yihuang

Solution:
- Fix existing clippy suggestions.
- Can't add clippy on enclave into travis ci script yet due to lack of sgx sdk

The `cast_ptr_alignment` warning looks suspicious, the correctness dependents on the alignment of `Vec<u8>` pointer, I think normal allocator will produce aligned pointer, so it should be fine normally.

Run on linux:
```
$ cargo clippy -p tx-validation-app -p tx-query-app
$ cargo clippy -p tx-validation-enclave -p tx-query-enclave
```

Co-authored-by: yihuang <[email protected]>

chain-tx-enclave/enclave-t-common/src/lib.rs

@@ -60,6 +60,8 @@ where
             return None;
         }
         let opt = unsafe {
+            // TODO check alignment correctness
+            #[allow(clippy::cast_ptr_alignment)]
             SgxSealedData::<[u8]>::from_raw_sealed_data_t(
                 sealed_log.as_mut_ptr() as *mut sgx_sealed_data_t,
                 sealed_log.len() as u32,

chain-tx-enclave/tx-query/app/src/enclave_u/mod.rs

@@ -50,7 +50,7 @@ pub extern "C" fn ocall_get_txs(
 ) -> sgx_status_t {
     let req = unsafe { std::slice::from_raw_parts(txids, txids_len as usize) };
 
-    let r = ZMQ_SOCKET.with(|socket| {
+    ZMQ_SOCKET.with(|socket| {
         let send_r = socket.send(req, FLAGS);
         if send_r.is_err() {
             error!("failed to send a request for obtaining sealed data");
@@ -59,19 +59,18 @@ pub extern "C" fn ocall_get_txs(
         if let Ok(msg) = socket.recv_bytes(FLAGS) {
             if msg.len() > (txs_len as usize) {
                 error!("Not enough allocated space to return the sealed tx data");
-                return sgx_status_t::SGX_ERROR_UNEXPECTED;
+                sgx_status_t::SGX_ERROR_UNEXPECTED
             } else {
                 unsafe {
                     std::ptr::copy(msg.as_ptr(), txs, msg.len());
                 }
-                return sgx_status_t::SGX_SUCCESS;
+                sgx_status_t::SGX_SUCCESS
             }
         } else {
             error!("failed to receive a response for obtaining sealed data");
-            return sgx_status_t::SGX_ERROR_UNEXPECTED;
+            sgx_status_t::SGX_ERROR_UNEXPECTED
         }
-    });
-    r
+    })
 }
 
 /// Untrusted function called from the enclave -- sends a ZMQ message to
@@ -93,16 +92,16 @@ pub extern "C" fn ocall_encrypt_request(
         if let Ok(msg) = socket.recv_bytes(FLAGS) {
             if msg.len() > (result_len as usize) {
                 error!("Not enough allocated space to return the sealed tx data");
-                return sgx_status_t::SGX_ERROR_UNEXPECTED;
+                sgx_status_t::SGX_ERROR_UNEXPECTED
             } else {
                 unsafe {
                     std::ptr::copy(msg.as_ptr(), result, msg.len());
                 }
-                return sgx_status_t::SGX_SUCCESS;
+                sgx_status_t::SGX_SUCCESS
             }
         } else {
             error!("failed to send a request for obtaining obfuscated tx");
-            return sgx_status_t::SGX_ERROR_UNEXPECTED;
+            sgx_status_t::SGX_ERROR_UNEXPECTED
         }
     })
 }
@@ -170,11 +169,11 @@ pub extern "C" fn ocall_get_ias_socket(ret_fd: *mut c_int) -> sgx_status_t {
     sgx_status_t::SGX_SUCCESS
 }
 
-fn decode_hex_digit(digit: char) -> u8 {
+fn decode_hex_digit(digit: u8) -> u8 {
     match digit {
-        '0'..='9' => digit as u8 - '0' as u8,
-        'a'..='f' => digit as u8 - 'a' as u8 + 10,
-        'A'..='F' => digit as u8 - 'A' as u8 + 10,
+        b'0'..=b'9' => digit - b'0',
+        b'a'..=b'f' => digit - b'a' + 10,
+        b'A'..=b'F' => digit - b'A' + 10,
         _ => panic!(),
     }
 }
@@ -197,13 +196,13 @@ fn get_spid() -> sgx_spid_t {
 
 fn decode_hex(hex: &str) -> Vec<u8> {
     let mut r: Vec<u8> = Vec::new();
-    let mut chars = hex.chars().enumerate();
+    let mut chars = hex.as_bytes().iter().copied().enumerate();
     loop {
         let (pos, first) = match chars.next() {
             None => break,
             Some(elt) => elt,
         };
-        if first == ' ' {
+        if b' ' == first {
             continue;
         }
         let (_, second) = match chars.next() {

chain-tx-enclave/tx-query/enclave/src/attest.rs

@@ -26,13 +26,13 @@ use std::untrusted::time::SystemTimeEx;
 use std::vec::Vec;
 use zeroize::Zeroize;
 
-pub const IAS_HOSTNAME: &'static str = "api.trustedservices.intel.com";
+pub const IAS_HOSTNAME: &str = "api.trustedservices.intel.com";
 #[cfg(not(feature = "production"))]
-pub const API_SUFFIX: &'static str = "/sgx/dev";
+pub const API_SUFFIX: &str = "/sgx/dev";
 #[cfg(feature = "production")]
-pub const API_SUFFIX: &'static str = "/sgx";
-pub const SIGRL_SUFFIX: &'static str = "/attestation/v3/sigrl/";
-pub const REPORT_SUFFIX: &'static str = "/attestation/v3/report";
+pub const API_SUFFIX: &str = "/sgx";
+pub const SIGRL_SUFFIX: &str = "/attestation/v3/sigrl/";
+pub const REPORT_SUFFIX: &str = "/attestation/v3/report";
 
 extern "C" {
     pub fn ocall_sgx_init_quote(
@@ -382,9 +382,9 @@ fn create_attestation_report(
     // (2) Generate the report
     // Fill ecc256 public key into report_data
     let mut report_data: sgx_report_data_t = sgx_report_data_t::default();
-    let mut pub_k_gx = pub_k.gx.clone();
+    let mut pub_k_gx = pub_k.gx;
     pub_k_gx.reverse();
-    let mut pub_k_gy = pub_k.gy.clone();
+    let mut pub_k_gy = pub_k.gy;
     pub_k_gy.reverse();
     report_data.d[..32].clone_from_slice(&pub_k_gx);
     report_data.d[32..].clone_from_slice(&pub_k_gy);
@@ -420,7 +420,7 @@ fn create_attestation_report(
     //       7. [out]p_qe_report need further check
     //       8. [out]p_quote
     //       9. quote_size
-    let (p_sigrl, sigrl_len) = if sigrl_vec.len() == 0 {
+    let (p_sigrl, sigrl_len) = if sigrl_vec.is_empty() {
         (ptr::null(), 0)
     } else {
         (sigrl_vec.as_ptr(), sigrl_vec.len() as u32)

chain-tx-enclave/tx-query/enclave/src/cert.rs

@@ -64,9 +64,9 @@ pub fn gen_ecc_cert(
 ) -> Result<CertKeyPair, sgx_status_t> {
     // Generate public key bytes since both DER will use it
     let mut pub_key_bytes: Vec<u8> = vec![4];
-    let mut pk_gx = pub_k.gx.clone();
+    let mut pk_gx = pub_k.gx;
     pk_gx.reverse();
-    let mut pk_gy = pub_k.gy.clone();
+    let mut pk_gy = pub_k.gy;
     pk_gy.reverse();
     pub_key_bytes.extend_from_slice(&pk_gx);
     pub_key_bytes.extend_from_slice(&pk_gy);
@@ -148,7 +148,7 @@ pub fn gen_ecc_cert(
                         writer.write_sequence(|writer| {
                             writer.next().write_sequence(|writer| {
                                 writer.next().write_oid(&ObjectIdentifier::from_slice(&[
-                                    2, 16, 840, 1, 113730, 1, 13,
+                                    2, 16, 840, 1, 113_730, 1, 13,
                                 ]));
                                 writer.next().write_bytes(&payload.into_bytes());
                             });
@@ -168,9 +168,9 @@ pub fn gen_ecc_cert(
             };
             let sig_der = yasna::construct_der(|writer| {
                 writer.write_sequence(|writer| {
-                    let mut sig_x = sig.x.clone();
+                    let mut sig_x = sig.x;
                     sig_x.reverse();
-                    let mut sig_y = sig.y.clone();
+                    let mut sig_y = sig.y;
                     sig_y.reverse();
                     writer.next().write_biguint(&BigUint::from_slice(&sig_x));
                     writer.next().write_biguint(&BigUint::from_slice(&sig_y));
@@ -195,7 +195,7 @@ pub fn gen_ecc_cert(
             let inner_key_der = yasna::construct_der(|writer| {
                 writer.write_sequence(|writer| {
                     writer.next().write_u8(1);
-                    let mut prv_k_r = prv_k.r.clone();
+                    let mut prv_k_r = prv_k.r;
                     prv_k_r.reverse();
                     writer.next().write_bytes(&prv_k_r);
                     prv_k_r.zeroize();

chain-tx-enclave/tx-query/enclave/src/lib.rs

@@ -75,13 +75,10 @@ fn process_decryption_request(body: &DecryptionRequestBody) -> Option<Decryption
         return None;
     }
     match EnclaveResponse::decode(&mut inputs_buf.as_slice()) {
-        Ok(EnclaveResponse::GetSealedTxData(Some(inputs))) => check_unseal(
-            Some(body.view_key),
-            true,
-            body.txs.iter().map(|x| *x),
-            inputs,
-        )
-        .map(|txs| DecryptionResponse { txs }),
+        Ok(EnclaveResponse::GetSealedTxData(Some(inputs))) => {
+            check_unseal(Some(body.view_key), true, body.txs.iter().copied(), inputs)
+                .map(|txs| DecryptionResponse { txs })
+        }
         _ => {
             // println!("failed to decode a response for obtaining sealed data");
             None
@@ -96,7 +93,10 @@ fn handle_decryption_request(
     let mut challenge = [0u8; 32];
     let mut os_rng = os::SgxRng::new().unwrap();
     os_rng.fill_bytes(&mut challenge);
-    if let Err(_) = tls.write(&TxQueryInitResponse::DecryptChallenge(challenge).encode()[..]) {
+    if tls
+        .write(&TxQueryInitResponse::DecryptChallenge(challenge).encode()[..])
+        .is_err()
+    {
         let _ = tls.sock.shutdown(Shutdown::Both);
         return sgx_status_t::SGX_ERROR_UNEXPECTED;
     }
@@ -149,13 +149,12 @@ fn get_sealed_request(req: &EncryptionRequest, txid: &TxId) -> Option<Vec<u8>> {
     let mut sealed_log: Vec<u8> = vec![0u8; sealed_log_size];
 
     unsafe {
-        let sealed_r = sealed_data.to_raw_sealed_data_t(
+        // TODO check alignment correctness
+        #[allow(clippy::cast_ptr_alignment)]
+        sealed_data.to_raw_sealed_data_t(
             sealed_log.as_mut_ptr() as *mut sgx_sealed_data_t,
             sealed_log_size as u32,
-        );
-        if sealed_r.is_none() {
-            return None;
-        }
+        )?;
     }
     Some(sealed_log)
 }
@@ -195,7 +194,7 @@ fn handle_encryption_request(
     match request {
         None => {
             let _ = tls.sock.shutdown(Shutdown::Both);
-            return sgx_status_t::SGX_ERROR_UNEXPECTED;
+            sgx_status_t::SGX_ERROR_UNEXPECTED
         }
         Some(qreq) => {
             let req_enc = EnclaveRequest::EncryptTx(Box::new(qreq)).encode();
@@ -251,9 +250,7 @@ fn handle_encryption_request(
                     let _ = tls.flush();
                     sgx_status_t::SGX_SUCCESS
                 }
-                _ => {
-                    return sgx_status_t::SGX_ERROR_INVALID_PARAMETER;
-                }
+                _ => sgx_status_t::SGX_ERROR_INVALID_PARAMETER,
             }
         }
     }

chain-tx-enclave/tx-validation/app/src/server/mod.rs

@@ -130,7 +130,7 @@ impl TxValidationServer {
                     Ok(EnclaveRequest::CommitBlock { app_hash, info }) => {
                         let _ = self.metadb.insert(LAST_APP_HASH_KEY, &app_hash);
                         let _ = self.metadb.insert(LAST_CHAIN_INFO_KEY, &info.encode()[..]);
-                        if let Ok(_) = self.flush_all() {
+                        if self.flush_all().is_ok() {
                             self.info = Some(info);
                             EnclaveResponse::CommitBlock(Ok(()))
                         } else {
@@ -156,9 +156,7 @@ impl TxValidationServer {
                         }
                     }
                     Ok(EnclaveRequest::GetSealedTxData { txids }) => {
-                        EnclaveResponse::GetSealedTxData(
-                            self.lookup_txids(txids.iter().map(|x| *x)),
-                        )
+                        EnclaveResponse::GetSealedTxData(self.lookup_txids(txids.iter().copied()))
                     }
                     Ok(EnclaveRequest::EncryptTx(req)) => {
                         let result = match self.info {

chain-tx-enclave/tx-validation/enclave/src/lib.rs

@@ -30,14 +30,17 @@ pub extern "C" fn ecall_initchain(chain_hex_id: u8) -> sgx_status_t {
     }
 }
 
+/// # Safety
+///
+/// This function should not be called with null pointer.
 #[no_mangle]
-pub extern "C" fn ecall_check_tx(
+pub unsafe extern "C" fn ecall_check_tx(
     tx_request: *const u8,
     tx_request_len: usize,
     response_buf: *mut u8,
     response_len: u32,
 ) -> sgx_status_t {
-    let mut tx_request_slice = unsafe { slice::from_raw_parts(tx_request, tx_request_len) };
+    let mut tx_request_slice = slice::from_raw_parts(tx_request, tx_request_len);
     match IntraEnclaveRequest::decode(&mut tx_request_slice) {
         Ok(IntraEnclaveRequest::ValidateTx { request, tx_inputs }) => {
             validate::handle_validate_tx(request, tx_inputs, response_buf, response_len)
@@ -48,7 +51,7 @@ pub extern "C" fn ecall_check_tx(
         }
         Err(e) => {
             log::error!("ecall check tx failed: {:?}", e);
-            return sgx_status_t::SGX_ERROR_INVALID_PARAMETER;
+            sgx_status_t::SGX_ERROR_INVALID_PARAMETER
         }
     }
 }

chain-tx-enclave/tx-validation/enclave/src/obfuscate.rs

@@ -109,6 +109,8 @@ pub(crate) fn decrypt(tx: &TxObfuscated) -> Result<PlainTxAux, ()> {
 #[inline]
 fn unseal_request(request: &mut IntraEncryptRequest) -> Option<EncryptionRequest> {
     let opt = unsafe {
+        // TODO check alignment correctness
+        #[allow(clippy::cast_ptr_alignment)]
         SgxSealedData::<[u8]>::from_raw_sealed_data_t(
             request.sealed_enc_request.as_mut_ptr() as *mut sgx_sealed_data_t,
             request.sealed_enc_request.len() as u32,
@@ -176,7 +178,7 @@ pub(crate) fn handle_encrypt_request(
                 );
                 write_back_response(response, response_buf, response_len)
             } else {
-                return sgx_status_t::SGX_ERROR_INVALID_PARAMETER;
+                sgx_status_t::SGX_ERROR_INVALID_PARAMETER
             }
         }
         (Some(EncryptionRequest::DepositStake(tx, witness)), Some(sealed_inputs)) => {
@@ -192,7 +194,7 @@ pub(crate) fn handle_encrypt_request(
                 );
                 write_back_response(response, response_buf, response_len)
             } else {
-                return sgx_status_t::SGX_ERROR_INVALID_PARAMETER;
+                sgx_status_t::SGX_ERROR_INVALID_PARAMETER
             }
         }
         (Some(EncryptionRequest::WithdrawStake(tx, account, witness)), None) => {
@@ -208,13 +210,9 @@ pub(crate) fn handle_encrypt_request(
                     );
                     write_back_response(response, response_buf, response_len)
                 }
-                _ => {
-                    return sgx_status_t::SGX_ERROR_INVALID_PARAMETER;
-                }
+                _ => sgx_status_t::SGX_ERROR_INVALID_PARAMETER,
             }
         }
-        (_, _) => {
-            return sgx_status_t::SGX_ERROR_INVALID_PARAMETER;
-        }
+        (_, _) => sgx_status_t::SGX_ERROR_INVALID_PARAMETER,
     }
 }

chain-tx-enclave/tx-validation/enclave/src/validate.rs

@@ -85,6 +85,8 @@ fn construct_sealed_response(
             let mut sealed_log: Vec<u8> = vec![0u8; sealed_log_size];
 
             unsafe {
+                // TODO check alignment correctness
+                #[allow(clippy::cast_ptr_alignment)]
                 let sealed_r = sealed_data.to_raw_sealed_data_t(
                     sealed_log.as_mut_ptr() as *mut sgx_sealed_data_t,
                     sealed_log_size as u32,
@@ -183,7 +185,7 @@ pub(crate) fn handle_validate_tx(
                 }
                 _ => {
                     log::error!("can not find plain transfer transaction or unsealed inputs");
-                    return sgx_status_t::SGX_ERROR_INVALID_PARAMETER;
+                    sgx_status_t::SGX_ERROR_INVALID_PARAMETER
                 }
             }
         }
@@ -198,7 +200,7 @@ pub(crate) fn handle_validate_tx(
                 }
                 _ => {
                     log::error!("can not get plain deposit stake transaction or unsealed inputs");
-                    return sgx_status_t::SGX_ERROR_INVALID_PARAMETER;
+                    sgx_status_t::SGX_ERROR_INVALID_PARAMETER
                 }
             }
         }
@@ -234,13 +236,13 @@ pub(crate) fn handle_validate_tx(
                 }
                 _ => {
                     log::error!("invalid parameter");
-                    return sgx_status_t::SGX_ERROR_INVALID_PARAMETER;
+                    sgx_status_t::SGX_ERROR_INVALID_PARAMETER
                 }
             }
         }
         (_, _) => {
             log::error!("invalid parameter");
-            return sgx_status_t::SGX_ERROR_INVALID_PARAMETER;
+            sgx_status_t::SGX_ERROR_INVALID_PARAMETER
         }
     }
 }