Snap for 12134224 from 02be88aaaafe70854cfe5a7a4eaf1fe0a38f31df to simpleperf-release
Change-Id: I10cf96465a00c5eae336e558c1d723e05bef6532
diff --git a/app/rules.mk b/app/rules.mk
index d49afbd..f2fd732 100644
--- a/app/rules.mk
+++ b/app/rules.mk
@@ -46,4 +46,6 @@
--cfg 'feature="with_hwwsk_support"'
endif
+MODULE_RUST_USE_CLIPPY := true
+
include make/trusted_app.mk
diff --git a/ffi_bindings.rs b/ffi_bindings.rs
index cd9d4bb..c0538a4 100644
--- a/ffi_bindings.rs
+++ b/ffi_bindings.rs
@@ -41,7 +41,7 @@
}
}
-pub(crate) const KEYBOX_PORT: &'static [u8; 28] = sys::KEYBOX_PORT;
+pub(crate) const KEYBOX_PORT: &[u8; 28] = sys::KEYBOX_PORT;
type KeyboxReqHdr = sys::keybox_req;
type KeyboxUnwrapReqPayloadHdr = sys::keybox_unwrap_req;
diff --git a/ipc_manager.rs b/ipc_manager.rs
index 1ac41a2..702c0c1 100644
--- a/ipc_manager.rs
+++ b/ipc_manager.rs
@@ -83,7 +83,7 @@
&'a self,
serializer: &mut S,
) -> Result<S::Ok, S::Error> {
- serializer.serialize_bytes(&self.0.as_slice())
+ serializer.serialize_bytes(self.0.as_slice())
}
}
@@ -131,7 +131,7 @@
_handle: &Handle,
peer: &Uuid,
) -> tipc::Result<ConnectResult<Self::Connection>> {
- info!("Accepted connection from uuid {:?}.", peer);
+ debug!("Accepted connection from uuid {:?}.", peer);
Ok(ConnectResult::Accept(Context { uuid: peer.clone() }))
}
@@ -185,19 +185,19 @@
/// Indicate whether provisioning is allowed.
fn provisioning_allowed() -> Result<bool, Error> {
- Ok(match get_system_state_provisioning_flag()? {
- ProvisioningAllowedFlagValues::ProvisioningAllowed => true,
- _ => false,
- })
+ Ok(matches!(
+ get_system_state_provisioning_flag()?,
+ ProvisioningAllowedFlagValues::ProvisioningAllowed
+ ))
}
/// Indicate whether provisioning is allowed during boot.
fn provisioning_allowed_at_boot() -> Result<bool, Error> {
- Ok(match get_system_state_provisioning_flag()? {
- ProvisioningAllowedFlagValues::ProvisioningAllowed => true,
- ProvisioningAllowedFlagValues::ProvisioningAllowedAtBoot => true,
- _ => false,
- })
+ Ok(matches!(
+ get_system_state_provisioning_flag()?,
+ ProvisioningAllowedFlagValues::ProvisioningAllowed
+ | ProvisioningAllowedFlagValues::ProvisioningAllowedAtBoot
+ ))
}
/// TIPC service implementation for communication with components outside Trusty (notably the
@@ -398,7 +398,7 @@
_handle: &Handle,
peer: &Uuid,
) -> tipc::Result<ConnectResult<Self::Connection>> {
- info!("Accepted connection from uuid {:?}.", peer);
+ debug!("Accepted connection from uuid {:?}.", peer);
Ok(ConnectResult::Accept(Context { uuid: peer.clone() }))
}
@@ -500,7 +500,7 @@
error!("access policy rejected the uuid: {:?}", peer);
return Ok(ConnectResult::CloseConnection);
}
- info!("Accepted connection from uuid {:?}.", peer);
+ debug!("Accepted connection from uuid {:?}.", peer);
Ok(ConnectResult::Accept(Context { uuid: peer.clone() }))
}
@@ -755,12 +755,10 @@
Ok(req)
}
- #[test]
+ // This test should only be run manually as it writes to the secure storage
+ // of the device under test.
+ // #[test]
fn set_attestation_keys_certs() {
- if !cfg!(kmr_enabled) {
- skip!("KeyMint Rust TA not configured");
- }
-
let port = CString::try_new(KM_NS_LEGACY_TIPC_SRV_PORT).unwrap();
let session = Handle::connect(port.as_c_str()).unwrap();
@@ -807,7 +805,9 @@
session.recv(buf)
}
- #[test]
+ // This test should only be run manually as it writes to the secure storage
+ // of the device under test.
+ // #[test]
fn set_attestation_ids() {
if !cfg!(kmr_enabled) {
skip!("KeyMint Rust TA not configured");
diff --git a/key_wrapper.rs b/key_wrapper.rs
index a1ecc04..9d74ada 100644
--- a/key_wrapper.rs
+++ b/key_wrapper.rs
@@ -16,7 +16,6 @@
//! Trusty implementation of StorageKeyWrapper trait.
use alloc::vec::Vec;
use core::ffi::CStr;
-use hwwsk;
use kmr_common::{
crypto,
crypto::{aes, Aes, KeyMaterial, OpaqueKeyMaterial, OpaqueOr},
@@ -30,7 +29,7 @@
use tipc::Handle;
/// TIPC port used for communication with the `hwwsk` service.
-const HWWSK_PORT: &'static [u8] = b"com.android.trusty.hwwsk\0";
+const HWWSK_PORT: &[u8] = b"com.android.trusty.hwwsk\0";
/// Create a session for `hwwsk` communication.
fn hwwsk_session() -> Result<Handle, Error> {
diff --git a/keys.rs b/keys.rs
index 84197cf..8862a33 100644
--- a/keys.rs
+++ b/keys.rs
@@ -33,11 +33,11 @@
/// Key slot identification; matches the value used in
/// `OpenSSLKeymasterEnforcement::GetKeyAgreementKey` in `openssl_keymaster_enforcement.cpp` for
/// back-compatibility.
-const KM_KAK_SLOT_ID: &'static [u8] = b"com.android.trusty.keymint.kak\0";
+const KM_KAK_SLOT_ID: &[u8] = b"com.android.trusty.keymint.kak\0";
/// Key derivation input data; matches `kMasterKeyDerivationData` in `trusty_keymaster_context.cpp`
/// for back-compatibility.
-const KM_KEY_DERIVATION_DATA: &'static [u8] = b"KeymasterMaster\0";
+const KM_KEY_DERIVATION_DATA: &[u8] = b"KeymasterMaster\0";
/// Size of a `u32` value in bytes.
const U32_SIZE: usize = core::mem::size_of::<u32>();
diff --git a/keys/legacy.rs b/keys/legacy.rs
index ff1fb9a..d4bbbd0 100644
--- a/keys/legacy.rs
+++ b/keys/legacy.rs
@@ -61,7 +61,7 @@
// If the slot is zero, the per-key secret is empty.
let secret: &[u8] = if slot == 0 { &[] } else { &sdd_data.secure_deletion_secret };
info.try_extend_from_slice(&(secret.len() as u32).to_ne_bytes())?;
- info.try_extend_from_slice(&secret)?;
+ info.try_extend_from_slice(secret)?;
info.try_extend_from_slice(&slot.to_ne_bytes())?;
}
@@ -110,7 +110,7 @@
let slot = SecureDeletionSlot(slot_idx);
let sdd_data = sdd_mgr.get_secret(slot)?;
- Some((sdd_data, slot_idx as u32))
+ Some((sdd_data, slot_idx))
}
(true, None, _) => {
return Err(km_err!(
@@ -151,14 +151,14 @@
let rollback_version = match encrypted_keyblob.addl_info {
Some(v) => Some(
- hwkey::OsRollbackVersion::try_from(v as i32)
+ hwkey::OsRollbackVersion::try_from(v)
.map_err(|e| km_err!(InvalidKeyBlob, "unexpected addl_info={} : {:?}", v, e))?,
),
None => None,
};
let kek_context = super::TrustyKekContext::new(
encrypted_keyblob.format.is_versioned(),
- encrypted_keyblob.kdf_version.map(|v| hwkey::KdfVersion::from(v)),
+ encrypted_keyblob.kdf_version.map(hwkey::KdfVersion::from),
rollback_version,
)?
.to_raw()?;
diff --git a/main.rs b/main.rs
index 8033f60..1d5f052 100644
--- a/main.rs
+++ b/main.rs
@@ -30,7 +30,7 @@
hmac::BoringHmac, rsa::BoringRsa, sha256::BoringSha256,
};
use kmr_ta::{HardwareInfo, RpcInfo, RpcInfoV3};
-use log::info;
+use log::debug;
fn log_formatter(record: &log::Record) -> String {
// line number should be present, so keeping it simple by just returning a 0.
@@ -45,7 +45,7 @@
.format(&log_formatter);
trusty_log::init_with_config(config);
- info!("Hello from Keymint Rust!");
+ debug!("Hello from Keymint Rust!");
let hw_info = HardwareInfo {
version_number: 3,
@@ -92,7 +92,7 @@
};
let dev = kmr_ta::device::Implementation {
keys: Box::new(TrustyKeys),
- sign_info: Box::new(CertSignInfo),
+ sign_info: Some(Box::new(CertSignInfo)),
attest_ids: Some(Box::new(AttestationIds)),
sdd_mgr: Some(Box::new(shared_sdd_mgr)),
bootloader: Box::new(kmr_ta::device::BootloaderDone),
diff --git a/monotonic_clock.rs b/monotonic_clock.rs
index 7e71281..5c2db53 100644
--- a/monotonic_clock.rs
+++ b/monotonic_clock.rs
@@ -35,7 +35,7 @@
(secure_time_ns / 1000) / 1000
};
- return MillisecondsSinceEpoch(secure_time_ns);
+ MillisecondsSinceEpoch(secure_time_ns)
}
}
diff --git a/rpc.rs b/rpc.rs
index 8a000d5..44a432c 100644
--- a/rpc.rs
+++ b/rpc.rs
@@ -27,7 +27,7 @@
// This matches the value of kMasterKeyDerivationData in
// trusty/user/app/keymaster/trusty_remote_provisioning_context.cpp
-const HBK_KEY_DERIVATION_DATA: &'static [u8] = b"RemoteKeyProvisioningMasterKey";
+const HBK_KEY_DERIVATION_DATA: &[u8] = b"RemoteKeyProvisioningMasterKey";
pub struct TrustyRpc;
@@ -67,23 +67,23 @@
Ok(dice_info)
}
- fn sign_data<'a>(
+ fn sign_data(
&self,
_ec: &dyn crypto::Ec,
_data: &[u8],
- _rpc_v2: Option<RpcV2Req<'a>>,
+ _rpc_v2: Option<RpcV2Req>,
) -> Result<Vec<u8>, Error> {
// This is marked unimplemented because we override `sign_data_in_cose_sign1` below.
Err(rpc_err!(Failed, "unimplemented"))
}
- fn sign_data_in_cose_sign1<'a>(
+ fn sign_data_in_cose_sign1(
&self,
_ec: &dyn crypto::Ec,
signing_algorithm: &CsrSigningAlgorithm,
payload: &[u8],
aad: &[u8],
- _rpc_v2: Option<RpcV2Req<'a>>,
+ _rpc_v2: Option<RpcV2Req>,
) -> Result<Vec<u8>, Error> {
match signing_algorithm {
CsrSigningAlgorithm::EdDSA => {}
diff --git a/rules.mk b/rules.mk
index ee38d24..0a0b91a 100644
--- a/rules.mk
+++ b/rules.mk
@@ -69,4 +69,6 @@
MODULE_BINDGEN_SRC_HEADER := $(LOCAL_DIR)/bindings.h
+MODULE_RUST_USE_CLIPPY := true
+
include make/library.mk
diff --git a/secure_deletion_secret_manager.rs b/secure_deletion_secret_manager.rs
index e15064f..24df232 100644
--- a/secure_deletion_secret_manager.rs
+++ b/secure_deletion_secret_manager.rs
@@ -24,7 +24,6 @@
};
use log::{debug, error, info, warn};
use storage::{self as storage_session, OpenMode, Port, SecureFile, Session, Transaction};
-use trusty_sys;
use zeroize::{Zeroize, ZeroizeOnDrop};
// Maximum number of attempts to perform a secure storage transaction to read or
@@ -68,7 +67,7 @@
// Name of the file to store secrets. The "_1" suffix is to allow for new file
// formats/versions in the future.
-const SECURE_DELETION_SECRET_FILENAME: &'static str = "SecureDeletionSecrets_1";
+const SECURE_DELETION_SECRET_FILENAME: &str = "SecureDeletionSecrets_1";
// TODO: Add crate static_assertions to trusty to replace these with static_assert!
const _: () = assert!(
@@ -270,7 +269,7 @@
#[derive(Clone, PartialEq, Eq, Zeroize, ZeroizeOnDrop)]
struct FactoryResetSecret([u8; FACTORY_RESET_SECRET_SIZE]);
-#[derive(Clone, PartialEq, Eq)]
+#[derive(Clone, PartialEq, Eq, Default)]
pub struct TrustySecureDeletionSecretManager {
factory_reset_secret: RefCell<Option<FactoryResetSecret>>,
}
@@ -294,7 +293,7 @@
// Checking if we already have a cached secret we can return
if let Some(secret) = self.factory_reset_secret.borrow_mut().deref_mut() {
return Ok(RetrieveSecureDeletionSecretFileData::CachedDataFound(SecureDeletionData {
- factory_reset_secret: secret.0.clone(),
+ factory_reset_secret: secret.0,
secure_deletion_secret: [0; SECRET_SIZE],
}));
}
@@ -315,7 +314,7 @@
})?;
// Found an empty file
- if file_size <= 0 {
+ if file_size == 0 {
return Ok(RetrieveSecureDeletionSecretFileData::EmptyFileFound(sdsf_file));
}
@@ -331,10 +330,7 @@
block.len()
));
}
- self.factory_reset_secret
- .borrow_mut()
- .deref_mut()
- .replace(FactoryResetSecret(buffer.clone()));
+ self.factory_reset_secret.borrow_mut().deref_mut().replace(FactoryResetSecret(buffer));
Ok(RetrieveSecureDeletionSecretFileData::DataFoundOnFile(SecureDeletionData {
factory_reset_secret: buffer,
secure_deletion_secret: [0; SECRET_SIZE],
@@ -368,7 +364,7 @@
self.factory_reset_secret
.borrow_mut()
.deref_mut()
- .replace(FactoryResetSecret(buffer.clone()));
+ .replace(FactoryResetSecret(buffer));
Ok(SecureDeletionData {
factory_reset_secret: buffer,
secure_deletion_secret: [0; SECRET_SIZE],
@@ -525,8 +521,7 @@
error!("Error zeroing space in extended file: {:?}", e);
return Err(e);
}
- let slot_number = original_file_size / SECRET_SIZE;
- slot_number
+ original_file_size / SECRET_SIZE
}
};
@@ -636,7 +631,7 @@
requested_slot
));
}
- if let Err(_) = sdsf_file.zero_entries(key_slot_start, key_slot_end) {
+ if sdsf_file.zero_entries(key_slot_start, key_slot_end).is_err() {
continue;
}
debug!(
@@ -668,6 +663,7 @@
}
}
+#[allow(dead_code)]
#[cfg(test)]
mod tests {
use super::*;
@@ -680,7 +676,8 @@
session.open_file(SECURE_DELETION_SECRET_FILENAME, OpenMode::Open).is_ok()
}
- #[test]
+ // This test should be run manually as it writes to storage.
+ // #[test]
fn secret_data_is_cached() {
let mut sdsf = TrustySecureDeletionSecretManager::new();
sdsf.delete_all();
@@ -730,7 +727,8 @@
expect!(!secret_manager_file_exists(), "Couldn't delete secret manager file");
}
- #[test]
+ // This test should be run manually as it writes to storage.
+ // #[test]
fn new_secret_data_file_is_clean() {
let mut sdsf = TrustySecureDeletionSecretManager::new();
sdsf.delete_all();
@@ -762,8 +760,9 @@
// Not running next test because it takes too long when run on build server, which causes unit
// tests to timeout sometimes. Also not using #[ignore] because it doesn't seem to be supported
// yet.
+ // Also, this test should be run manually as it writes to storage.
- //#[test]
+ // #[test]
#[allow(dead_code)]
fn new_secret_data_file_expands() {
let mut sdsf = TrustySecureDeletionSecretManager::new();
@@ -864,7 +863,8 @@
expect!(!secret_manager_file_exists(), "Couldn't delete secret manager file");
}
- #[test]
+ // This test should be run manually as it writes to storage.
+ // #[test]
fn new_secret_data_dont_affect_neighbors() {
let mut sdsf = TrustySecureDeletionSecretManager::new();
sdsf.delete_all();
diff --git a/secure_storage_manager.rs b/secure_storage_manager.rs
index f04a713..031c91a 100644
--- a/secure_storage_manager.rs
+++ b/secure_storage_manager.rs
@@ -30,18 +30,17 @@
use log::info;
use protobuf::{self, Message};
use storage::{OpenMode, Port, SecureFile, Session};
-use trusty_sys;
#[cfg(feature = "soft_attestation_fallback")]
mod software;
/// Name of file holding attestation device ID information; matches the `kAttestationIdsFileName`
/// value in `secure_storage_manager.cpp` for back-compatibility.
-const KM_ATTESTATION_ID_FILENAME: &'static str = "AttestationIds";
+const KM_ATTESTATION_ID_FILENAME: &str = "AttestationIds";
/// Filename prefix for files holding attestation keys and certificates; matches the
/// `kAttestKeyCertPrefix` value in `secure_storage_manager.cpp` for back-compatibility.
-const KM_ATTESTATION_KEY_CERT_PREFIX: &'static str = "AttestKeyCert";
+const KM_ATTESTATION_KEY_CERT_PREFIX: &str = "AttestKeyCert";
/// Maximum size of each attestation certificate
const MAX_CERT_SIZE: usize = 2048;
@@ -166,7 +165,7 @@
algorithm: SigningAlgorithm,
cert_data: &[u8],
) -> Result<(), Error> {
- if cert_data.len() == 0 {
+ if cert_data.is_empty() {
return Err(km_err!(InvalidInputLength, "received a certificate of length 0"));
}
@@ -200,7 +199,7 @@
/// Tries to read the file containing the attestation key delete only the certificate section.
pub(crate) fn clear_attestation_cert_chain(algorithm: SigningAlgorithm) -> Result<(), Error> {
let mut attestation_key_data = read_attestation_key_content(algorithm)?;
- if attestation_key_data.get_certs().len() == 0 {
+ if attestation_key_data.get_certs().is_empty() {
// No certs found, nothing to delete.
return Ok(());
}
@@ -221,6 +220,7 @@
}
/// Creates a new attestation IDs file and saves the provided data there
+#[allow(clippy::too_many_arguments)]
pub(crate) fn provision_attestation_id_file(
brand: &[u8],
product: &[u8],
@@ -236,32 +236,32 @@
let mut attestation_ids = keymaster_attributes::AttestationIds::new();
- if brand.len() > 0 {
+ if !brand.is_empty() {
attestation_ids.set_brand(try_to_vec(brand)?);
}
- if device.len() > 0 {
+ if !device.is_empty() {
attestation_ids.set_device(try_to_vec(device)?);
}
- if product.len() > 0 {
+ if !product.is_empty() {
attestation_ids.set_product(try_to_vec(product)?);
}
- if serial.len() > 0 {
+ if !serial.is_empty() {
attestation_ids.set_serial(try_to_vec(serial)?);
}
- if imei.len() > 0 {
+ if !imei.is_empty() {
attestation_ids.set_imei(try_to_vec(imei)?);
}
- if meid.len() > 0 {
+ if !meid.is_empty() {
attestation_ids.set_meid(try_to_vec(meid)?);
}
- if manufacturer.len() > 0 {
+ if !manufacturer.is_empty() {
attestation_ids.set_manufacturer(try_to_vec(manufacturer)?);
}
- if model.len() > 0 {
+ if !model.is_empty() {
attestation_ids.set_model(try_to_vec(model)?);
}
match maybe_imei2 {
- Some(imei2) if imei2.len() > 0 => {
+ Some(imei2) if !imei2.is_empty() => {
attestation_ids.set_second_imei(try_to_vec(imei2)?);
}
_ => (),
@@ -471,6 +471,7 @@
}
}
+#[allow(dead_code)]
#[cfg(test)]
mod tests {
use super::*;
@@ -583,7 +584,8 @@
delete_key_file(algorithm);
}
- #[test]
+ // This test should be run manually as it writes to storage.
+ // #[test]
fn read_ec_rsa_certificates() {
read_certificates_test(SigningAlgorithm::Rsa);
read_certificates_test(SigningAlgorithm::Ec);
@@ -616,7 +618,8 @@
Ok(key)
}
- #[test]
+ // This test should be run manually as it writes to storage.
+ // #[test]
fn multiple_attestation_calls_work() {
let ec_test_key =
get_test_attestation_key(SigningAlgorithm::Ec).expect("Couldn't get test key");
@@ -683,7 +686,8 @@
delete_key_file(algorithm);
}
- #[test]
+ // This test should be run manually as it writes to storage.
+ // #[test]
fn read_ec_rsa_key() {
read_key_test(SigningAlgorithm::Rsa);
read_key_test(SigningAlgorithm::Ec);
@@ -701,17 +705,20 @@
delete_key_file(algorithm);
}
- #[test]
+ // This test should be run manually as it writes to storage.
+ // #[test]
fn read_sec1_private_ec_key() {
read_non_pkcs8_key_test(SigningAlgorithm::Ec);
}
- #[test]
+ // This test should be run manually as it writes to storage.
+ // #[test]
fn read_pkcs1_rsa_key() {
read_non_pkcs8_key_test(SigningAlgorithm::Rsa);
}
- #[test]
+ // This test should be run manually as it writes to storage.
+ // #[test]
fn unprovisioned_keys_certs_reads_produces_error() {
if check_key_file_exists(SigningAlgorithm::Ec) {
delete_key_file(SigningAlgorithm::Ec);
@@ -726,7 +733,8 @@
expect!(read_attestation_key(key_type).is_err(), "Shouldn't be able to read a key");
}
- #[test]
+ // This test should be run manually as it writes to storage.
+ // #[test]
fn provision_certs_test() {
provision_certs_test_impl(SigningAlgorithm::Ec, true);
provision_certs_test_impl(SigningAlgorithm::Rsa, true);
@@ -782,7 +790,8 @@
);
}
- #[test]
+ // This test should be run manually as it writes to storage.
+ // #[test]
fn clear_certificate_chain_works_when_unprovisioned() {
clear_certificate_chain_works_when_unprovisioned_impl(SigningAlgorithm::Ec);
clear_certificate_chain_works_when_unprovisioned_impl(SigningAlgorithm::Rsa);
@@ -813,13 +822,15 @@
delete_key_file(algorithm);
}
- #[test]
+ // This test should be run manually as it writes to storage.
+ // #[test]
fn clear_certificate_chain_works() {
clear_certificate_chain_works_impl(SigningAlgorithm::Ec);
clear_certificate_chain_works_impl(SigningAlgorithm::Rsa);
}
- #[test]
+ // This test should be run manually as it writes to storage.
+ // #[test]
fn unprovisioned_attestation_ids_do_not_error() {
if check_attestation_id_file_exists() {
delete_attestation_id_file();
@@ -838,7 +849,8 @@
expect_eq!(attestation_ids.imei2.len(), 0, "imei2 should be empty");
}
- #[test]
+ // This test should be run manually as it writes to storage.
+ // #[test]
fn single_attestation_id_field() {
let mut file = create_attestation_id_file().expect("Couldn't create attestation id file");
@@ -893,7 +905,8 @@
);
}
- #[test]
+ // This test should be run manually as it writes to storage.
+ // #[test]
fn test_provision_attestation_id_file() {
let brand = b"unknown brand";
let product = b"";
@@ -993,7 +1006,8 @@
expect_eq!(attestation_ids_info.imei2, expected_imei2.to_vec(), "imei2 doesn't match");
}
- #[test]
+ // This test should be run manually as it writes to storage.
+ // #[test]
fn all_attestation_id_fields() {
let mut file = create_attestation_id_file().expect("Couldn't create attestation id file");
let mut attestation_ids = keymaster_attributes::AttestationIds::new();
@@ -1076,7 +1090,8 @@
);
}
- #[test]
+ // This test should be run manually as it writes to storage.
+ // #[test]
fn delete_attestation_ids_file() {
let mut file = create_attestation_id_file().expect("Couldn't create attestation id file");
let raw_protobuf = [10, 9, 110, 101, 119, 32, 98, 114, 97, 110, 100];
