From 563467962cab39a1ef2c455ffd9df6d1f1f4c81f Mon Sep 17 00:00:00 2001 From: LIAUD Corentin Date: Fri, 18 Oct 2024 12:13:39 +0200 Subject: [PATCH] feat: remove unwraps + clean code --- adb_cli/src/adb_termios.rs | 4 +- adb_client/Cargo.toml | 7 ++-- adb_client/README.md | 4 +- adb_client/src/error.rs | 9 ++++ adb_client/src/models/device_short.rs | 3 +- .../src/server/device_commands/framebuffer.rs | 2 +- .../src/server/server_commands/connect.rs | 7 ++-- .../src/server/server_commands/disconnect.rs | 7 ++-- adb_client/src/server/server_commands/pair.rs | 7 ++-- .../src/transports/tcp_emulator_transport.rs | 2 +- adb_client/src/transports/usb_transport.rs | 14 +------ adb_client/src/usb/adb_rsa_key.rs | 42 ++++++++++++------- adb_client/src/usb/adb_usb_device.rs | 33 +++++++++------ adb_client/src/usb/adb_usb_message.rs | 10 ++--- adb_client/src/usb/usb_commands.rs | 2 +- tests/tests.rs | 12 +++--- 16 files changed, 95 insertions(+), 70 deletions(-) diff --git a/adb_cli/src/adb_termios.rs b/adb_cli/src/adb_termios.rs index 5217f6f..319b027 100644 --- a/adb_cli/src/adb_termios.rs +++ b/adb_cli/src/adb_termios.rs @@ -35,6 +35,8 @@ impl ADBTermios { impl Drop for ADBTermios { fn drop(&mut self) { // Custom drop implementation, restores previous termios structure. - tcsetattr(self.fd, TCSANOW, &self.old_termios).unwrap(); + if let Err(e) = tcsetattr(self.fd, TCSANOW, &self.old_termios) { + log::error!("Error while droping ADBTermios: {e}") + } } } diff --git a/adb_client/Cargo.toml b/adb_client/Cargo.toml index 1d7a9c2..8fef295 100644 --- a/adb_client/Cargo.toml +++ b/adb_client/Cargo.toml @@ -12,12 +12,13 @@ version.workspace = true base64 = "0.22.1" byteorder = { version = "1.5.0" } chrono = { version = "0.4.38" } -homedir = { version = "0.3.3" } -image = { version = "0.25.2" } +homedir = { version = "0.3.4" } +image = { version = "0.25.4" } lazy_static = { version = "1.5.0" } log = { version = "0.4.22" } num-bigint = { version = "0.6", package = "num-bigint-dig" } -regex = { version = "1.10.6", features = ["perf", "std", "unicode"] } +rand = "0.7.0" +regex = { version = "1.11.0", features = ["perf", "std", "unicode"] } rsa = { version = "0.3.0" } rusb = { version = "0.9.4", features = ["vendored"] } thiserror = { version = "1.0.64" } diff --git a/adb_client/README.md b/adb_client/README.md index e3bf01e..a44ed8d 100644 --- a/adb_client/README.md +++ b/adb_client/README.md @@ -20,7 +20,7 @@ adb_client = "*" ### Launch a command on device via ADB server ```rust no_run -use adb_client::ADBServer; +use adb_client::{ADBServer, ADBDeviceExt}; let mut server = ADBServer::default(); let mut device = server.get_device().expect("cannot get device"); @@ -51,6 +51,6 @@ use std::path::Path; let mut server = ADBServer::default(); let mut device = server.get_device().expect("cannot get device"); -let mut input = File::open(Path::new("/tmp")).unwrap(); +let mut input = File::open(Path::new("/tmp")).expect("Cannot open file"); device.send(&mut input, "/data/local/tmp"); ``` diff --git a/adb_client/src/error.rs b/adb_client/src/error.rs index b01f6b7..0836122 100644 --- a/adb_client/src/error.rs +++ b/adb_client/src/error.rs @@ -54,9 +54,15 @@ pub enum RustADBError { /// An error occurred when converting framebuffer content #[error("Cannot convert framebuffer into image")] FramebufferConversionError, + /// Unimplemented framebuffer image version + #[error("Unimplemented framebuffer image version: {0}")] + UnimplementedFramebufferImageVersion(u32), /// An error occurred while getting user's home directory #[error(transparent)] HomeError(#[from] homedir::GetHomeError), + /// Cannot get home directory + #[error("Cannot get home directory")] + NoHomeDirectory, /// Generic USB error #[error(transparent)] UsbError(#[from] rusb::Error), @@ -78,4 +84,7 @@ pub enum RustADBError { /// An error occurred with RSA engine #[error(transparent)] RSAError(#[from] rsa::errors::Error), + /// Cannot convert given data from slice + #[error(transparent)] + TryFromSliceError(#[from] std::array::TryFromSliceError), } diff --git a/adb_client/src/models/device_short.rs b/adb_client/src/models/device_short.rs index bc594b1..cbad7dc 100644 --- a/adb_client/src/models/device_short.rs +++ b/adb_client/src/models/device_short.rs @@ -5,7 +5,8 @@ use std::{fmt::Display, str::FromStr}; use crate::{DeviceState, RustADBError}; lazy_static! { - static ref DEVICES_REGEX: Regex = Regex::new("^(\\S+)\t(\\w+)\n?$").unwrap(); + static ref DEVICES_REGEX: Regex = + Regex::new("^(\\S+)\t(\\w+)\n?$").expect("Cannot build devices regex"); } /// Represents a device connected to the ADB server. diff --git a/adb_client/src/server/device_commands/framebuffer.rs b/adb_client/src/server/device_commands/framebuffer.rs index e533a1e..29cf663 100644 --- a/adb_client/src/server/device_commands/framebuffer.rs +++ b/adb_client/src/server/device_commands/framebuffer.rs @@ -170,7 +170,7 @@ impl ADBServerDevice { .ok_or_else(|| RustADBError::FramebufferConversionError)?, ) } - v => unimplemented!("Version {} not implemented", v), + v => Err(RustADBError::UnimplementedFramebufferImageVersion(v)), } } } diff --git a/adb_client/src/server/server_commands/connect.rs b/adb_client/src/server/server_commands/connect.rs index cb43a1a..bc1201f 100644 --- a/adb_client/src/server/server_commands/connect.rs +++ b/adb_client/src/server/server_commands/connect.rs @@ -8,9 +8,10 @@ impl ADBServer { .connect()? .proxy_connection(AdbServerCommand::Connect(address), true)?; - match String::from_utf8(response).unwrap() { - s if s.starts_with("connected to") => Ok(()), - s => Err(RustADBError::ADBRequestFailed(s)), + match String::from_utf8(response) { + Ok(s) if s.starts_with("connected to") => Ok(()), + Ok(s) => Err(RustADBError::ADBRequestFailed(s)), + Err(e) => Err(e.into()), } } } diff --git a/adb_client/src/server/server_commands/disconnect.rs b/adb_client/src/server/server_commands/disconnect.rs index 715fb21..d570e93 100644 --- a/adb_client/src/server/server_commands/disconnect.rs +++ b/adb_client/src/server/server_commands/disconnect.rs @@ -8,9 +8,10 @@ impl ADBServer { .connect()? .proxy_connection(AdbServerCommand::Disconnect(address), true)?; - match String::from_utf8(response).unwrap() { - s if s.starts_with("disconnected") => Ok(()), - s => Err(RustADBError::ADBRequestFailed(s)), + match String::from_utf8(response) { + Ok(s) if s.starts_with("disconnected") => Ok(()), + Ok(s) => Err(RustADBError::ADBRequestFailed(s)), + Err(e) => Err(e.into()), } } } diff --git a/adb_client/src/server/server_commands/pair.rs b/adb_client/src/server/server_commands/pair.rs index 734cdc5..bafbf7d 100644 --- a/adb_client/src/server/server_commands/pair.rs +++ b/adb_client/src/server/server_commands/pair.rs @@ -9,9 +9,10 @@ impl ADBServer { .connect()? .proxy_connection(AdbServerCommand::Pair(address, code), true)?; - match String::from_utf8(response).unwrap() { - s if s.starts_with("Successfully paired to") => Ok(()), - s => Err(RustADBError::ADBRequestFailed(s)), + match String::from_utf8(response) { + Ok(s) if s.starts_with("Successfully paired to") => Ok(()), + Ok(s) => Err(RustADBError::ADBRequestFailed(s)), + Err(e) => Err(e.into()), } } } diff --git a/adb_client/src/transports/tcp_emulator_transport.rs b/adb_client/src/transports/tcp_emulator_transport.rs index 22b573d..edddb86 100644 --- a/adb_client/src/transports/tcp_emulator_transport.rs +++ b/adb_client/src/transports/tcp_emulator_transport.rs @@ -38,7 +38,7 @@ impl TCPEmulatorTransport { pub fn get_authentication_token(&mut self) -> Result { let home = match my_home()? { Some(home) => home, - None => todo!(), + None => return Err(RustADBError::NoHomeDirectory), }; let mut f = File::open(home.join(".emulator_console_auth_token"))?; diff --git a/adb_client/src/transports/usb_transport.rs b/adb_client/src/transports/usb_transport.rs index b8e6582..fa75ba4 100644 --- a/adb_client/src/transports/usb_transport.rs +++ b/adb_client/src/transports/usb_transport.rs @@ -118,12 +118,7 @@ impl USBTransport { fn find_readable_endpoint(&self) -> Result { let handle = self.get_raw_connection()?; - for n in 0..handle - .device() - .device_descriptor() - .unwrap() - .num_configurations() - { + for n in 0..handle.device().device_descriptor()?.num_configurations() { let config_desc = match handle.device().config_descriptor(n) { Ok(c) => c, Err(_) => continue, @@ -153,12 +148,7 @@ impl USBTransport { fn find_writable_endpoint(&self) -> Result { let handle = self.get_raw_connection()?; - for n in 0..handle - .device() - .device_descriptor() - .unwrap() - .num_configurations() - { + for n in 0..handle.device().device_descriptor()?.num_configurations() { let config_desc = match handle.device().config_descriptor(n) { Ok(c) => c, Err(_) => continue, diff --git a/adb_client/src/usb/adb_rsa_key.rs b/adb_client/src/usb/adb_rsa_key.rs index aa516cc..bfe5e44 100644 --- a/adb_client/src/usb/adb_rsa_key.rs +++ b/adb_client/src/usb/adb_rsa_key.rs @@ -3,16 +3,24 @@ use base64::{engine::general_purpose::STANDARD, Engine}; use byteorder::{LittleEndian, WriteBytesExt}; use num_bigint::traits::ModInverse; use num_bigint::BigUint; +use rand::rngs::OsRng; use rsa::{Hash, PaddingScheme, PublicKeyParts, RSAPrivateKey}; use std::convert::TryInto; // From project: https://github.com/hajifkd/webadb #[derive(Debug)] pub struct ADBRsaKey { - pub private_key: RSAPrivateKey, + private_key: RSAPrivateKey, } impl ADBRsaKey { + pub fn random_with_size(size: usize) -> Result { + let mut rng = OsRng; + Ok(Self { + private_key: RSAPrivateKey::new(&mut rng, size)?, + }) + } + pub fn from_pkcs8(pkcs8_content: &str) -> Result { let der_encoded = pkcs8_content .lines() @@ -36,25 +44,24 @@ impl ADBRsaKey { let mut result = vec![]; result.write_u32::(RSANUMWORDS)?; - let r32 = set_bit(32); + let r32 = set_bit(32)?; let n = self.private_key.n(); - let r = set_bit((32 * RSANUMWORDS) as _); + let r = set_bit((32 * RSANUMWORDS) as _)?; // Well, let rr = set_bit((64 * RSANUMWORDS) as _) % n is also fine, since r \sim n. let rr = r.modpow(&BigUint::from(2u32), n); let rem = n % &r32; let n0inv = rem.mod_inverse(&r32); if let Some(n0inv) = n0inv { - let n0inv = n0inv.to_biguint().unwrap(); - let n0inv_p: u32 = - 1 + !u32::from_le_bytes((&n0inv.to_bytes_le()[..4]).try_into().unwrap()); + let n0inv = n0inv.to_biguint().ok_or(RustADBError::ConversionError)?; + let n0inv_p: u32 = 1 + !u32::from_le_bytes((&n0inv.to_bytes_le()[..4]).try_into()?); result.write_u32::(n0inv_p)?; } else { return Err(RustADBError::ConversionError); } - write_biguint(&mut result, n, RSANUMBYTES as _); - write_biguint(&mut result, &rr, RSANUMBYTES as _); - write_biguint(&mut result, self.private_key.e(), 4); + write_biguint(&mut result, n, RSANUMBYTES as _)?; + write_biguint(&mut result, &rr, RSANUMBYTES as _)?; + write_biguint(&mut result, self.private_key.e(), 4)?; let mut encoded = STANDARD.encode(&result); encoded.push(' '); @@ -70,18 +77,20 @@ impl ADBRsaKey { } } -fn write_biguint(writer: &mut Vec, data: &BigUint, n_bytes: usize) { +fn write_biguint(writer: &mut Vec, data: &BigUint, n_bytes: usize) -> Result<()> { for &v in data .to_bytes_le() .iter() .chain(std::iter::repeat(&0)) .take(n_bytes) { - writer.write_u8(v).unwrap(); + writer.write_u8(v)?; } + + Ok(()) } -fn set_bit(n: usize) -> BigUint { +fn set_bit(n: usize) -> Result { BigUint::parse_bytes( &{ let mut bits = vec![]; @@ -93,7 +102,7 @@ fn set_bit(n: usize) -> BigUint { }[..], 2, ) - .unwrap() + .ok_or(RustADBError::ConversionError) } #[test] @@ -126,8 +135,11 @@ xGDK/moPvzs0CjdPlRcEN+Myy/G0FUrOaC0FcpNoJOdQSYz3F6URA4nX+zj6Ie7G CNkECiepaGyquQaffwR1CAi8dH6biJjlTQWQPFcCLA0hvernWo3eaSfiL7fHyym+ ile69MHFENUePSpuRSiF3Z02 -----END PRIVATE KEY-----"; - let priv_key = ADBRsaKey::from_pkcs8(DEFAULT_PRIV_KEY).unwrap(); - let pub_key = priv_key.encoded_public_key().unwrap(); + let priv_key = + ADBRsaKey::from_pkcs8(DEFAULT_PRIV_KEY).expect("cannot create rsa key from data"); + let pub_key = priv_key + .encoded_public_key() + .expect("cannot encode public key"); let pub_key_adb = "\ QAAAAFH/pU9PVrHRgEjMGnpvOr2QzKYCavSE1fcSwvpS1uPn9GTmuyZr7c9up\ MBpSrrlFYpsjBQ7IfAyZIsVsffr5doEG5StKN8FwaO+sEX9YZX9Sr2m7/eVi0\ diff --git a/adb_client/src/usb/adb_usb_device.rs b/adb_client/src/usb/adb_usb_device.rs index 7391cbd..c87e44b 100644 --- a/adb_client/src/usb/adb_usb_device.rs +++ b/adb_client/src/usb/adb_usb_device.rs @@ -13,25 +13,32 @@ pub struct ADBUSBDevice { pub(crate) transport: USBTransport, } -fn read_adb_private_key(private_key_path: Option) -> Option { - let private_key = private_key_path.or_else(|| { - homedir::my_home() - .ok()? - .map(|home| home.join(".android").join("adbkey")) - })?; +fn read_adb_private_key(private_key_path: Option) -> Result> { + let private_key = private_key_path + .or_else(|| { + homedir::my_home() + .ok()? + .map(|home| home.join(".android").join("adbkey")) + }) + .ok_or(RustADBError::NoHomeDirectory)?; read_to_string(&private_key) .map_err(RustADBError::from) - .map(|pk| ADBRsaKey::from_pkcs8(&pk).unwrap()) - .ok() + .map(|pk| match ADBRsaKey::from_pkcs8(&pk) { + Ok(pk) => Some(pk), + Err(e) => { + log::error!("Error while create RSA private key: {e}"); + None + } + }) } impl ADBUSBDevice { /// Instantiate a new [ADBUSBDevice] pub fn new(vendor_id: u16, product_id: u16, private_key_path: Option) -> Result { - let private_key = match read_adb_private_key(private_key_path) { + let private_key = match read_adb_private_key(private_key_path)? { Some(pk) => pk, - None => unimplemented!(), + None => ADBRsaKey::random_with_size(2048)?, }; Ok(Self { @@ -73,7 +80,7 @@ impl ADBUSBDevice { } }; - let sign = self.private_key.sign(auth_message.into_payload()).unwrap(); + let sign = self.private_key.sign(auth_message.into_payload())?; let message = ADBUsbMessage::new(USBCommand::Auth, AUTH_SIGNATURE, 0, sign); @@ -86,7 +93,7 @@ impl ADBUSBDevice { return Ok(()); } - let mut pubkey = self.private_key.encoded_public_key().unwrap().into_bytes(); + let mut pubkey = self.private_key.encoded_public_key()?.into_bytes(); pubkey.push(b'\0'); let message = ADBUsbMessage::new(USBCommand::Auth, AUTH_RSAPUBLICKEY, 0, pubkey); @@ -100,7 +107,7 @@ impl ADBUSBDevice { match response.command() { USBCommand::Cnxn => log::info!( "Authentication OK, device info {}", - String::from_utf8(response.into_payload().to_vec()).unwrap() + String::from_utf8(response.into_payload().to_vec())? ), _ => { return Err(RustADBError::ADBRequestFailed(format!( diff --git a/adb_client/src/usb/adb_usb_message.rs b/adb_client/src/usb/adb_usb_message.rs index c1f3835..fd24458 100644 --- a/adb_client/src/usb/adb_usb_message.rs +++ b/adb_client/src/usb/adb_usb_message.rs @@ -95,11 +95,11 @@ impl TryFrom<[u8; 24]> for ADBUsbMessage { fn try_from(value: [u8; 24]) -> Result { let message = Self { command: USBCommand::try_from(&value[0..4])?, - arg0: u32::from_le_bytes(value[4..8].try_into().unwrap()), - arg1: u32::from_le_bytes(value[8..12].try_into().unwrap()), - data_length: u32::from_le_bytes(value[12..16].try_into().unwrap()), - data_crc32: u32::from_le_bytes(value[16..20].try_into().unwrap()), - magic: u32::from_le_bytes(value[20..24].try_into().unwrap()), + arg0: u32::from_le_bytes(value[4..8].try_into()?), + arg1: u32::from_le_bytes(value[8..12].try_into()?), + data_length: u32::from_le_bytes(value[12..16].try_into()?), + data_crc32: u32::from_le_bytes(value[16..20].try_into()?), + magic: u32::from_le_bytes(value[20..24].try_into()?), payload: vec![], }; diff --git a/adb_client/src/usb/usb_commands.rs b/adb_client/src/usb/usb_commands.rs index 05efb40..e0a6f42 100644 --- a/adb_client/src/usb/usb_commands.rs +++ b/adb_client/src/usb/usb_commands.rs @@ -50,7 +50,7 @@ impl TryFrom<&[u8]> for USBCommand { type Error = RustADBError; fn try_from(value: &[u8]) -> Result { - match u32::from_le_bytes(value.try_into().unwrap()) { + match u32::from_le_bytes(value.try_into()?) { 0x4e584e43 => Ok(Self::Cnxn), 0x45534c43 => Ok(Self::Clse), 0x48545541 => Ok(Self::Auth), diff --git a/tests/tests.rs b/tests/tests.rs index 3646187..9e22768 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -17,27 +17,27 @@ mod tests { #[test] fn test_version() { let mut adb = new_client(); - adb.version().unwrap(); + adb.version().expect("cannot get adb version"); } #[test] - fn test_shell() { + fn test_shell_commands() { let mut device = new_device(); - device.shell_command(vec!["ls"]).unwrap(); - device.shell_command(vec!["pwd"]).unwrap(); + device.shell_command(["ls"]).expect("error while executing `ls` command"); + device.shell_command(["pwd"]).expect("error while executing `pwd` command"); } #[test] fn test_devices() { let mut adb = new_client(); - adb.devices().unwrap(); + adb.devices().expect("cannot list devices"); } #[test] fn test_devices_long() { let mut adb = new_client(); - adb.devices_long().unwrap(); + adb.devices_long().expect("cannot list devices long"); } #[test]