From 99a35b8a2c655075c6462fed8eb7c9c6f5757899 Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Mon, 7 Jun 2021 10:13:44 +0900 Subject: [PATCH] apkdmverity: build for Android.bp ... and some parts of the source code were revised to satisfy the stricter lint checks for Android. Bug: 189785765 Test: cargo test Test: m apkdmverity Change-Id: Ic3d80922396fb8e7cba29b092d6f74d17e936f7a --- apkverity/Android.bp | 42 +++++++++++++++++++++++++++++++++ apkverity/src/apksigv4.rs | 6 ++--- apkverity/src/dm.rs | 25 +++++++++++--------- apkverity/src/dm/verity.rs | 1 + apkverity/src/loopdevice.rs | 5 +++- apkverity/src/loopdevice/sys.rs | 1 + apkverity/src/main.rs | 9 ++++++- apkverity/src/util.rs | 3 +-- 8 files changed, 74 insertions(+), 18 deletions(-) create mode 100644 apkverity/Android.bp diff --git a/apkverity/Android.bp b/apkverity/Android.bp new file mode 100644 index 00000000..3d0dab51 --- /dev/null +++ b/apkverity/Android.bp @@ -0,0 +1,42 @@ +package { + default_applicable_licenses: ["Android-Apache-2.0"], +} + +rust_defaults { + name: "apkdmverity.defaults", + crate_name: "apkdmverity", + srcs: ["src/main.rs"], + edition: "2018", + prefer_rlib: true, + rustlibs: [ + "libanyhow", + "libbitflags", + "libclap", + "liblibc", + "libnix", + "libnum_traits", + "libscopeguard", + "libuuid", + ], + proc_macros: ["libnum_derive"], + multilib: { + lib32: { + enabled: false, + }, + }, +} + +rust_binary { + name: "apkdmverity", + defaults: ["apkdmverity.defaults"], +} + +rust_test { + name: "apkdmverity.test", + defaults: ["apkdmverity.defaults"], + test_suites: ["general-tests"], + compile_multilib: "first", + rustlibs: [ + "libtempfile", + ], +} diff --git a/apkverity/src/apksigv4.rs b/apkverity/src/apksigv4.rs index f1ee0a4f..7d8f318b 100644 --- a/apkverity/src/apksigv4.rs +++ b/apkverity/src/apksigv4.rs @@ -57,7 +57,7 @@ pub enum Version { impl Version { fn from(val: u32) -> Result { - Self::from_u32(val).ok_or(anyhow!("{} is an unsupported version", val)) + Self::from_u32(val).ok_or_else(|| anyhow!("{} is an unsupported version", val)) } } @@ -69,7 +69,7 @@ pub enum HashAlgorithm { impl HashAlgorithm { fn from(val: u32) -> Result { - Self::from_u32(val).ok_or(anyhow!("{} is an unsupported hash algorithm", val)) + Self::from_u32(val).ok_or_else(|| anyhow!("{} is an unsupported hash algorithm", val)) } } @@ -157,7 +157,7 @@ mod tests { use std::io::Cursor; fn hexstring_from(s: &[u8]) -> String { - s.iter().map(|byte| format!("{:02x}", byte)).reduce(|i, j| i + &j).unwrap_or(String::new()) + s.iter().map(|byte| format!("{:02x}", byte)).reduce(|i, j| i + &j).unwrap_or_default() } #[test] diff --git a/apkverity/src/dm.rs b/apkverity/src/dm.rs index ec4248c4..8828b0a2 100644 --- a/apkverity/src/dm.rs +++ b/apkverity/src/dm.rs @@ -41,9 +41,10 @@ use sys::*; pub use verity::*; nix::ioctl_readwrite!(_dm_dev_create, DM_IOCTL, Cmd::DM_DEV_CREATE, DmIoctl); -nix::ioctl_readwrite!(_dm_dev_remove, DM_IOCTL, Cmd::DM_DEV_REMOVE, DmIoctl); nix::ioctl_readwrite!(_dm_dev_suspend, DM_IOCTL, Cmd::DM_DEV_SUSPEND, DmIoctl); nix::ioctl_readwrite!(_dm_table_load, DM_IOCTL, Cmd::DM_TABLE_LOAD, DmIoctl); +#[cfg(test)] +nix::ioctl_readwrite!(_dm_dev_remove, DM_IOCTL, Cmd::DM_DEV_REMOVE, DmIoctl); fn dm_dev_create(dm: &DeviceMapper, ioctl: *mut DmIoctl) -> Result { // SAFETY: `ioctl` is copied into the kernel. It modifies the state in the kernel, not the @@ -51,12 +52,6 @@ fn dm_dev_create(dm: &DeviceMapper, ioctl: *mut DmIoctl) -> Result { Ok(unsafe { _dm_dev_create(dm.0.as_raw_fd(), ioctl) }?) } -fn dm_dev_remove(dm: &DeviceMapper, ioctl: *mut DmIoctl) -> Result { - // SAFETY: `ioctl` is copied into the kernel. It modifies the state in the kernel, not the - // state of this process in any way. - Ok(unsafe { _dm_dev_remove(dm.0.as_raw_fd(), ioctl) }?) -} - fn dm_dev_suspend(dm: &DeviceMapper, ioctl: *mut DmIoctl) -> Result { // SAFETY: `ioctl` is copied into the kernel. It modifies the state in the kernel, not the // state of this process in any way. @@ -69,6 +64,13 @@ fn dm_table_load(dm: &DeviceMapper, ioctl: *mut DmIoctl) -> Result { Ok(unsafe { _dm_table_load(dm.0.as_raw_fd(), ioctl) }?) } +#[cfg(test)] +fn dm_dev_remove(dm: &DeviceMapper, ioctl: *mut DmIoctl) -> Result { + // SAFETY: `ioctl` is copied into the kernel. It modifies the state in the kernel, not the + // state of this process in any way. + Ok(unsafe { _dm_dev_remove(dm.0.as_raw_fd(), ioctl) }?) +} + // `DmTargetSpec` is the header of the data structure for a device-mapper target. When doing the // ioctl, one of more `DmTargetSpec` (and its body) are appened to the `DmIoctl` struct. #[repr(C)] @@ -90,7 +92,7 @@ impl DmTargetSpec { fn as_u8_slice(&self) -> &[u8; size_of::()] { // SAFETY: lifetime of the output reference isn't changed. - unsafe { std::mem::transmute::<&Self, &[u8; size_of::()]>(&self) } + unsafe { &*(&self as *const &Self as *const [u8; size_of::()]) } } } @@ -116,7 +118,7 @@ impl DmIoctl { fn as_u8_slice(&self) -> &[u8; size_of::()] { // SAFETY: lifetime of the output reference isn't changed. - unsafe { std::mem::transmute::<&Self, &[u8; size_of::()]>(&self) } + unsafe { &*(&self as *const &Self as *const [u8; size_of::()]) } } } @@ -150,8 +152,8 @@ impl DeviceMapper { data.flags |= Flag::DM_READONLY_FLAG; let mut payload = Vec::with_capacity(payload_size); - payload.extend_from_slice(&data.as_u8_slice()[..]); - payload.extend_from_slice(&target.as_u8_slice()[..]); + payload.extend_from_slice(data.as_u8_slice()); + payload.extend_from_slice(target.as_u8_slice()); dm_table_load(&self, payload.as_mut_ptr() as *mut DmIoctl)?; // Step 3: activate the device (note: the term 'suspend' might be misleading, but it @@ -166,6 +168,7 @@ impl DeviceMapper { } /// Removes a mapper device + #[cfg(test)] pub fn delete_device_deferred(&self, name: &str) -> Result<()> { let mut data = DmIoctl::new(&name)?; data.flags |= Flag::DM_DEFERRED_REMOVE; diff --git a/apkverity/src/dm/verity.rs b/apkverity/src/dm/verity.rs index cfc9504a..950b26e8 100644 --- a/apkverity/src/dm/verity.rs +++ b/apkverity/src/dm/verity.rs @@ -35,6 +35,7 @@ pub enum DmVerityVersion { } /// The hash algorithm to use. SHA256 and SHA512 are supported. +#[allow(dead_code)] pub enum DmVerityHashAlgorithm { SHA256, SHA512, diff --git a/apkverity/src/loopdevice.rs b/apkverity/src/loopdevice.rs index bb0e767b..519e3bd3 100644 --- a/apkverity/src/loopdevice.rs +++ b/apkverity/src/loopdevice.rs @@ -36,6 +36,7 @@ use crate::util::*; // These are old-style ioctls, thus *_bad. nix::ioctl_none_bad!(_loop_ctl_get_free, LOOP_CTL_GET_FREE); nix::ioctl_write_ptr_bad!(_loop_configure, LOOP_CONFIGURE, loop_config); +#[cfg(test)] nix::ioctl_none_bad!(_loop_clr_fd, LOOP_CLR_FD); fn loop_ctl_get_free(ctrl_file: &File) -> Result { @@ -50,6 +51,7 @@ fn loop_configure(device_file: &File, config: &loop_config) -> Result { Ok(unsafe { _loop_configure(device_file.as_raw_fd(), config) }?) } +#[cfg(test)] fn loop_clr_fd(device_file: &File) -> Result { // SAFETY: this ioctl disassociates the loop device with `device_file`, where the FD will // remain opened afterward. The association itself is kept for open FDs. @@ -122,13 +124,14 @@ fn try_attach>(path: P, offset: u64, size_limit: u64) -> Result

>(path: P) -> Result<()> { let device_file = OpenOptions::new().read(true).write(true).open(&path)?; loop_clr_fd(&device_file)?; diff --git a/apkverity/src/loopdevice/sys.rs b/apkverity/src/loopdevice/sys.rs index 2d4977b0..3f10f22c 100644 --- a/apkverity/src/loopdevice/sys.rs +++ b/apkverity/src/loopdevice/sys.rs @@ -24,6 +24,7 @@ pub const LOOP_CONTROL: &str = "/dev/loop-control"; pub const LOOP_CTL_GET_FREE: libc::c_ulong = 0x4C82; pub const LOOP_CONFIGURE: libc::c_ulong = 0x4C0A; +#[cfg(test)] pub const LOOP_CLR_FD: libc::c_ulong = 0x4C01; #[repr(C)] diff --git a/apkverity/src/main.rs b/apkverity/src/main.rs index 6fe12a03..5094c50d 100644 --- a/apkverity/src/main.rs +++ b/apkverity/src/main.rs @@ -57,12 +57,19 @@ fn main() -> Result<()> { ) .required(true), ) + .arg(Arg::with_name("verbose").short("v").long("verbose").help("Shows verbose output")) .get_matches(); let apk = matches.value_of("apk").unwrap(); let idsig = matches.value_of("idsig").unwrap(); let name = matches.value_of("name").unwrap(); - enable_verity(apk, idsig, name)?; + let ret = enable_verity(apk, idsig, name)?; + if matches.is_present("verbose") { + println!( + "data_device: {:?}, hash_device: {:?}, mapper_device: {:?}", + ret.data_device, ret.hash_device, ret.mapper_device + ); + } Ok(()) } diff --git a/apkverity/src/util.rs b/apkverity/src/util.rs index 415e99ba..d2bc7996 100644 --- a/apkverity/src/util.rs +++ b/apkverity/src/util.rs @@ -16,7 +16,6 @@ use anyhow::{bail, Result}; use nix::sys::stat::FileStat; -use std::fs; use std::fs::File; use std::os::unix::fs::FileTypeExt; use std::os::unix::io::AsRawFd; @@ -40,7 +39,7 @@ pub fn wait_for_path>(path: P) -> Result<()> { /// Returns hexadecimal reprentation of a given byte array. pub fn hexstring_from(s: &[u8]) -> String { - s.iter().map(|byte| format!("{:02x}", byte)).reduce(|i, j| i + &j).unwrap_or(String::new()) + s.iter().map(|byte| format!("{:02x}", byte)).reduce(|i, j| i + &j).unwrap_or_default() } /// fstat that accepts a path rather than FD