From 4d314264a897c474c12e626a2be36b75dc57f5c9 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Tue, 11 Dec 2018 23:51:01 +0100 Subject: Port the open and close commands to libnitrokey This patch removes the raw hidapi implementations of the Enable Encrypted Volume and Disable Encrypted Volume commands and replaces them with the methods enable_encrypted_volume and disable_encrypted_volume of the Storage struct provided by the nitrokey trait. To provide some context to the error messages, the errors are wrapped using the map_err method of the Result enum and the get_error function that combines a nitrokey error code and a string into a nitrocli error. It would be more idiomatic to define a conversion from a nitrokey error to a nitrocli error, but then we would lose information about the context of the error. --- nitrocli/CHANGELOG.md | 7 ++- nitrocli/src/main.rs | 130 ++++++++++++++++++----------------------------- nitrocli/src/nitrokey.rs | 83 ------------------------------ 3 files changed, 54 insertions(+), 166 deletions(-) diff --git a/nitrocli/CHANGELOG.md b/nitrocli/CHANGELOG.md index bfe1ffa..7ff193a 100644 --- a/nitrocli/CHANGELOG.md +++ b/nitrocli/CHANGELOG.md @@ -1,7 +1,10 @@ Unreleased ---------- -- Added `nitrokey` version `0.2.1` as a direct dependency and `nitrokey-sys` - version `3.4.1` as well as `rand` version `0.4.3` as indirect dependencies +- Use the `nitrokey` crate for the `open` and `close` commands instead + of directly communicating with the Nitrokey device + - Added `nitrokey` version `0.2.1` as a direct dependency and + `nitrokey-sys` version `3.4.1` as well as `rand` version `0.4.3` as + indirect dependencies 0.1.3 diff --git a/nitrocli/src/main.rs b/nitrocli/src/main.rs index 23370ee..714c573 100644 --- a/nitrocli/src/main.rs +++ b/nitrocli/src/main.rs @@ -79,6 +79,8 @@ use std::result; use std::thread; use std::time; +use libnitrokey; + use crate::error::Error; type Result = result::Result; @@ -91,6 +93,19 @@ const RECV_TRY_COUNT: i8 = 40; const SEND_RECV_DELAY_MS: u64 = 200; +/// Create an `error::Error` with an error message of the format `msg: err`. +fn get_error(msg: &str, err: &libnitrokey::CommandError) -> Error { + Error::Error(format!("{}: {:?}", msg, err)) +} + + +/// Connect to a Nitrokey Storage device and return it. +fn get_storage_device() -> Result { + libnitrokey::Storage::connect() + .or_else(|_| Err(Error::Error("Nitrokey device not found".to_string()))) +} + + /// Send a HID feature report to the device represented by the given handle. fn send

(handle: &mut libhid::Handle, report: &nitrokey::Report

) -> Result<()> where P: AsRef<[u8]>, @@ -272,64 +287,35 @@ fn status() -> Result<()> { } -/// Poll the nitrokey until it reports to no longer be busy. -fn wait(handle: &mut libhid::Handle) -> Result { - type Response = nitrokey::Response; - - loop { - thread::sleep(time::Duration::from_millis(SEND_RECV_DELAY_MS)); - - let report = receive::(handle)?; - let response = AsRef::::as_ref(&report.data); - let status = response.data.storage_status; - - if status != nitrokey::StorageStatus::Busy && - status != nitrokey::StorageStatus::BusyProgressbar { - return Ok(status); - } - } -} - - /// Open the encrypted volume on the nitrokey. fn open() -> Result<()> { - type Response = nitrokey::Response; - - nitrokey_do(&|handle| { - let mut retry = 3; - let mut error_msg: Option<&str> = None; - loop { - let passphrase = pinentry::inquire_passphrase(PIN_TYPE, error_msg)?; - let payload = nitrokey::EnableEncryptedVolumeCommand::new(&passphrase); - let report = nitrokey::Report::from(payload); + let device = get_storage_device()?; - let report = transmit::<_, nitrokey::EmptyPayload>(handle, &report)?; - let response = AsRef::::as_ref(&report.data); - let mut status = response.data.storage_status; - - if status == nitrokey::StorageStatus::WrongPassword { - pinentry::clear_passphrase(PIN_TYPE)?; - retry -= 1; + let mut retry = 3; + let mut error_msg: Option<&str> = None; + loop { + // TODO: Rethink the usage of String::from_utf8_lossy here. We may + // not want to silently modify the password! + let passphrase = pinentry::inquire_passphrase(PIN_TYPE, error_msg)?; + let passphrase = String::from_utf8_lossy(&passphrase); + match device.enable_encrypted_volume(&passphrase) { + Ok(()) => return Ok(()), + Err(err) => match err { + libnitrokey::CommandError::WrongPassword => { + pinentry::clear_passphrase(PIN_TYPE)?; + retry -= 1; - if retry > 0 { - error_msg = Some("Wrong password, please reenter"); - continue; - } - let error = "Opening encrypted volume failed: Wrong password"; - return Err(Error::Error(error.to_string())); - } - if status == nitrokey::StorageStatus::Busy || - status == nitrokey::StorageStatus::BusyProgressbar { - status = wait(handle)?; - } - if status != nitrokey::StorageStatus::Okay && status != nitrokey::StorageStatus::Idle { - let status = format!("{:?}", status); - let error = format!("Opening encrypted volume failed: {}", status); - return Err(Error::Error(error)); - } - return Ok(()); - } - }) + if retry > 0 { + error_msg = Some("Wrong password, please reenter"); + continue; + } + let error = "Opening encrypted volume failed: Wrong password"; + return Err(Error::Error(error.to_string())); + }, + err => return Err(get_error("Opening encrypted volume failed", &err)), + }, + }; + } } @@ -340,33 +326,15 @@ extern "C" { /// Close the previously opened encrypted volume. fn close() -> Result<()> { - type Response = nitrokey::Response; - - nitrokey_do(&|handle| { - // Flush all filesystem caches to disk. We are mostly interested in - // making sure that the encrypted volume on the nitrokey we are - // about to close is not closed while not all data was written to - // it. - unsafe { sync() }; - - let payload = nitrokey::DisableEncryptedVolumeCommand::new(); - let report = nitrokey::Report::from(payload); - - let report = transmit::<_, nitrokey::EmptyPayload>(handle, &report)?; - let response = AsRef::::as_ref(&report.data); - let mut status = response.data.storage_status; - - if status == nitrokey::StorageStatus::Busy || - status == nitrokey::StorageStatus::BusyProgressbar { - status = wait(handle)?; - } - if status != nitrokey::StorageStatus::Okay && status != nitrokey::StorageStatus::Idle { - let status = format!("{:?}", status); - let error = format!("Closing encrypted volume failed: {}", status); - return Err(Error::Error(error)); - } - Ok(()) - }) + // Flush all filesystem caches to disk. We are mostly interested in + // making sure that the encrypted volume on the nitrokey we are + // about to close is not closed while not all data was written to + // it. + unsafe { sync() }; + + get_storage_device()? + .disable_encrypted_volume() + .map_err(|err| get_error("Closing encrypted volume failed", &err)) } diff --git a/nitrocli/src/nitrokey.rs b/nitrocli/src/nitrokey.rs index 64685e2..9f767d6 100644 --- a/nitrocli/src/nitrokey.rs +++ b/nitrocli/src/nitrokey.rs @@ -17,7 +17,6 @@ // * along with this program. If not, see . * // ************************************************************************* -use std::cmp; use std::mem; use crate::crc32::crc; @@ -41,10 +40,6 @@ pub const VOLUME_ACTIVE_HIDDEN: u8 = 0b100; #[derive(PartialEq)] #[repr(u8)] pub enum Command { - // The command to enable the encrypted volume. - EnableEncryptedVolume = 0x20, - // The command to disable the encrypted volume. - DisableEncryptedVolume = 0x21, // Retrieve the device status. GetDeviceStatus = 0x2E, } @@ -183,39 +178,6 @@ macro_rules! defaultCommand { } -#[allow(dead_code)] -#[repr(packed)] -pub struct EnableEncryptedVolumeCommand { - command: Command, - // The kind of password. Unconditionally 'P' because the User PIN is - // used to enable the encrypted volume. - kind: u8, - // The password has a maximum length of twenty characters. - password: [u8; 20], - padding: [u8; 38], -} - - -impl EnableEncryptedVolumeCommand { - pub fn new(password: &[u8]) -> EnableEncryptedVolumeCommand { - let mut report = EnableEncryptedVolumeCommand { - command: Command::EnableEncryptedVolume, - kind: b'P', - password: [0; 20], - padding: [0; 38], - }; - - debug_assert!(password.len() <= report.password.len()); - - let len = cmp::min(report.password.len(), password.len()); - report.password[..len].copy_from_slice(&password[..len]); - report - } -} - -defaultPayloadAsRef!(EnableEncryptedVolumeCommand); - -defaultCommand!(DisableEncryptedVolumeCommand, DisableEncryptedVolume); defaultCommand!(DeviceStatusCommand, GetDeviceStatus); @@ -270,17 +232,6 @@ impl

AsRef<[u8]> for Response

{ } -#[repr(packed)] -pub struct StorageResponse { - pub padding1: [u8; 13], - pub command_counter: u8, - pub last_storage_command: Command, - pub storage_status: StorageStatus, - pub progress: u8, - pub padding2: [u8; 2], -} - - #[repr(packed)] pub struct DeviceStatusResponse { pub padding0: [u8; 22], @@ -303,37 +254,3 @@ pub struct DeviceStatusResponse { pub active_smartcard_id: u32, pub storage_keys_missing: u8, } - - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn encrypted_volume_report() { - let password = "test42".to_string().into_bytes(); - let report = EnableEncryptedVolumeCommand::new(&password); - let expected = ['t' as u8, 'e' as u8, 's' as u8, 't' as u8, '4' as u8, '2' as u8, 0u8, 0u8, - 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8]; - assert_eq!(report.password, expected); - } - - #[test] - #[cfg(debug)] - #[should_panic(expected = "assertion failed")] - fn overly_long_password() { - let password = "012345678912345678901".to_string().into_bytes(); - EnableEncryptedVolumeCommand::new(&password); - } - - #[test] - fn report_crc() { - let password = "passphrase".to_string().into_bytes(); - let payload = EnableEncryptedVolumeCommand::new(&password); - let report = Report::from(payload); - - // The expected checksum was computed using the original - // functionality. - assert_eq!(report.crc, 0xeeb583c); - } -} -- cgit v1.2.3