diff options
-rw-r--r-- | nitrocli/CHANGELOG.md | 2 | ||||
-rw-r--r-- | nitrocli/doc/nitrocli.1 | 6 | ||||
-rw-r--r-- | nitrocli/doc/nitrocli.1.pdf | bin | 14794 -> 15579 bytes | |||
-rw-r--r-- | nitrocli/src/args.rs | 2 | ||||
-rw-r--r-- | nitrocli/src/commands.rs | 88 | ||||
-rw-r--r-- | nitrocli/src/pinentry.rs | 86 |
6 files changed, 118 insertions, 66 deletions
diff --git a/nitrocli/CHANGELOG.md b/nitrocli/CHANGELOG.md index 77841dd..52ff45c 100644 --- a/nitrocli/CHANGELOG.md +++ b/nitrocli/CHANGELOG.md @@ -1,5 +1,7 @@ Unreleased ---------- +- Store cached PINs on a per-device basis to better support multi-device + scenarios - Further decrease binary size by using system allocator diff --git a/nitrocli/doc/nitrocli.1 b/nitrocli/doc/nitrocli.1 index 9537439..dbbaf2b 100644 --- a/nitrocli/doc/nitrocli.1 +++ b/nitrocli/doc/nitrocli.1 @@ -1,4 +1,4 @@ -.TH NITROCLI 1 2019-01-10 +.TH NITROCLI 1 2019-01-11 .SH NAME nitrocli \- access Nitrokey devices .SH SYNOPSIS @@ -201,7 +201,9 @@ reset using \fBgpg\fR(1). Use the \fBstatus\fR command to check the retry counters. .TP .B nitrocli pin clear -Clear the PINs cached by the other commands. +Clear the PINs cached by the other commands. Note that cached PINs are +associated with the device they belong to and the \fBclear\fR command will only +clear the PIN for the currently used device, not all others. .TP \fBnitrocli pin set \fItype\fR Change a PIN. diff --git a/nitrocli/doc/nitrocli.1.pdf b/nitrocli/doc/nitrocli.1.pdf Binary files differindex f93e3b7..a823db0 100644 --- a/nitrocli/doc/nitrocli.1.pdf +++ b/nitrocli/doc/nitrocli.1.pdf diff --git a/nitrocli/src/args.rs b/nitrocli/src/args.rs index f4e0758..b15ca65 100644 --- a/nitrocli/src/args.rs +++ b/nitrocli/src/args.rs @@ -631,7 +631,7 @@ fn pin_clear(ctx: &mut ExecCtx<'_>, args: Vec<String>) -> Result<()> { parser.set_description("Clears the cached PINs"); parse(ctx, &parser, args)?; - commands::pin_clear() + commands::pin_clear(ctx) } /// Change a PIN. diff --git a/nitrocli/src/commands.rs b/nitrocli/src/commands.rs index 209fb8f..00a594f 100644 --- a/nitrocli/src/commands.rs +++ b/nitrocli/src/commands.rs @@ -81,13 +81,18 @@ fn get_storage_device(ctx: &mut args::ExecCtx<'_>) -> Result<nitrokey::Storage> } /// Open the password safe on the given device. -fn get_password_safe<'dev>( +fn get_password_safe<'dev, D>( ctx: &mut args::ExecCtx<'_>, - device: &'dev dyn Device, -) -> Result<nitrokey::PasswordSafe<'dev>> { + device: &'dev D, +) -> Result<nitrokey::PasswordSafe<'dev>> +where + D: Device, +{ + let pin_entry = pinentry::PinEntry::from(pinentry::PinType::User, device)?; + try_with_pin_and_data( ctx, - pinentry::PinType::User, + &pin_entry, "Could not access the password safe", (), |_, pin| device.get_password_safe(pin).map_err(|err| ((), err)), @@ -108,7 +113,9 @@ where D: Device, F: Fn(D, &str) -> result::Result<A, (D, nitrokey::CommandError)>, { - try_with_pin_and_data(ctx, pin_type, msg, device, op) + let pin_entry = pinentry::PinEntry::from(pin_type, &device)?; + + try_with_pin_and_data(ctx, &pin_entry, msg, device, op) } /// Authenticate the given device with the user PIN. @@ -167,7 +174,7 @@ fn get_volume_status(status: &nitrokey::VolumeStatus) -> &'static str { /// second or third try, it will call `op` with the data returned by the /// previous call to `op`. fn try_with_pin_and_data_with_pinentry<D, F, R>( - pin_type: pinentry::PinType, + pin_entry: &pinentry::PinEntry, msg: &'static str, data: D, op: F, @@ -179,12 +186,12 @@ where let mut retry = 3; let mut error_msg = None; loop { - let pin = pinentry::inquire_pin(pin_type, pinentry::Mode::Query, error_msg)?; + let pin = pinentry::inquire_pin(pin_entry, pinentry::Mode::Query, error_msg)?; match op(data, &pin) { Ok(result) => return Ok(result), Err((new_data, err)) => match err { nitrokey::CommandError::WrongPassword => { - pinentry::clear_pin(pin_type)?; + pinentry::clear_pin(pin_entry)?; retry -= 1; if retry > 0 { @@ -203,7 +210,7 @@ where /// Try to execute the given function with a PIN. fn try_with_pin_and_data<D, F, R>( ctx: &mut args::ExecCtx<'_>, - pin_type: pinentry::PinType, + pin_entry: &pinentry::PinEntry, msg: &'static str, data: D, op: F, @@ -211,7 +218,7 @@ fn try_with_pin_and_data<D, F, R>( where F: Fn(D, &str) -> result::Result<R, (D, nitrokey::CommandError)>, { - let pin = match pin_type { + let pin = match pin_entry.pin_type() { pinentry::PinType::Admin => &ctx.admin_pin, pinentry::PinType::User => &ctx.user_pin, }; @@ -225,7 +232,7 @@ where })?; op(data, &pin).map_err(|(_, err)| get_error(msg, err)) } else { - try_with_pin_and_data_with_pinentry(pin_type, msg, data, op) + try_with_pin_and_data_with_pinentry(pin_entry, msg, data, op) } } @@ -235,14 +242,14 @@ where /// it refrains from passing any data to it. fn try_with_pin<F>( ctx: &mut args::ExecCtx<'_>, - pin_type: pinentry::PinType, + pin_entry: &pinentry::PinEntry, msg: &'static str, op: F, ) -> Result<()> where F: Fn(&str) -> result::Result<(), nitrokey::CommandError>, { - try_with_pin_and_data(ctx, pin_type, msg, (), |data, pin| { + try_with_pin_and_data(ctx, pin_entry, msg, (), |data, pin| { op(pin).map_err(|err| (data, err)) }) } @@ -287,12 +294,11 @@ pub fn status(ctx: &mut args::ExecCtx<'_>) -> Result<()> { /// Open the encrypted volume on the nitrokey. pub fn storage_open(ctx: &mut args::ExecCtx<'_>) -> Result<()> { let device = get_storage_device(ctx)?; - try_with_pin( - ctx, - pinentry::PinType::User, - "Opening encrypted volume failed", - |pin| device.enable_encrypted_volume(&pin), - ) + let pin_entry = pinentry::PinEntry::from(pinentry::PinType::User, &device)?; + + try_with_pin(ctx, &pin_entry, "Opening encrypted volume failed", |pin| { + device.enable_encrypted_volume(&pin) + }) } /// Close the previously opened encrypted volume. @@ -573,9 +579,14 @@ pub fn otp_status(ctx: &mut args::ExecCtx<'_>, all: bool) -> Result<()> { } /// Clear the PIN stored by various operations. -pub fn pin_clear() -> Result<()> { - pinentry::clear_pin(pinentry::PinType::Admin)?; - pinentry::clear_pin(pinentry::PinType::User)?; +pub fn pin_clear(ctx: &mut args::ExecCtx<'_>) -> Result<()> { + let device = get_device(ctx)?; + + pinentry::clear_pin(&pinentry::PinEntry::from( + pinentry::PinType::Admin, + &device, + )?)?; + pinentry::clear_pin(&pinentry::PinEntry::from(pinentry::PinType::User, &device)?)?; Ok(()) } @@ -594,14 +605,14 @@ fn check_pin(pin_type: pinentry::PinType, pin: &str) -> Result<()> { } } -fn choose_pin_with_pinentry(pin_type: pinentry::PinType) -> Result<String> { - pinentry::clear_pin(pin_type)?; - let new_pin = pinentry::inquire_pin(pin_type, pinentry::Mode::Choose, None)?; - pinentry::clear_pin(pin_type)?; - check_pin(pin_type, &new_pin)?; +fn choose_pin_with_pinentry(pin_entry: &pinentry::PinEntry) -> Result<String> { + pinentry::clear_pin(pin_entry)?; + let new_pin = pinentry::inquire_pin(pin_entry, pinentry::Mode::Choose, None)?; + pinentry::clear_pin(pin_entry)?; + check_pin(pin_entry.pin_type(), &new_pin)?; - let confirm_pin = pinentry::inquire_pin(pin_type, pinentry::Mode::Confirm, None)?; - pinentry::clear_pin(pin_type)?; + let confirm_pin = pinentry::inquire_pin(pin_entry, pinentry::Mode::Confirm, None)?; + pinentry::clear_pin(pin_entry)?; if new_pin != confirm_pin { Err(Error::from("Entered PINs do not match")) @@ -616,10 +627,10 @@ fn choose_pin_with_pinentry(pin_type: pinentry::PinType) -> Result<String> { /// given PIN type, it will be used. fn choose_pin( ctx: &mut args::ExecCtx<'_>, - pin_type: pinentry::PinType, + pin_entry: &pinentry::PinEntry, new: bool, ) -> Result<String> { - let new_pin = match pin_type { + let new_pin = match pin_entry.pin_type() { pinentry::PinType::Admin => { if new { &ctx.new_admin_pin @@ -642,17 +653,19 @@ fn choose_pin( .ok_or_else(|| Error::from("Failed to read PIN: invalid Unicode data found")) .map(ToOwned::to_owned) } else { - choose_pin_with_pinentry(pin_type) + choose_pin_with_pinentry(pin_entry) } } /// Change a PIN. pub fn pin_set(ctx: &mut args::ExecCtx<'_>, pin_type: pinentry::PinType) -> Result<()> { let device = get_device(ctx)?; - let new_pin = choose_pin(ctx, pin_type, true)?; + let pin_entry = pinentry::PinEntry::from(pin_type, &device)?; + let new_pin = choose_pin(ctx, &pin_entry, true)?; + try_with_pin( ctx, - pin_type, + &pin_entry, "Could not change the PIN", |current_pin| match pin_type { pinentry::PinType::Admin => device.change_admin_pin(¤t_pin, &new_pin), @@ -664,10 +677,13 @@ pub fn pin_set(ctx: &mut args::ExecCtx<'_>, pin_type: pinentry::PinType) -> Resu /// Unblock and reset the user PIN. pub fn pin_unblock(ctx: &mut args::ExecCtx<'_>) -> Result<()> { let device = get_device(ctx)?; - let user_pin = choose_pin(ctx, pinentry::PinType::User, false)?; + let pin_entry = pinentry::PinEntry::from(pinentry::PinType::User, &device)?; + let user_pin = choose_pin(ctx, &pin_entry, false)?; + let pin_entry = pinentry::PinEntry::from(pinentry::PinType::Admin, &device)?; + try_with_pin( ctx, - pinentry::PinType::Admin, + &pin_entry, "Could not unblock the user PIN", |admin_pin| device.unlock_user_pin(&admin_pin, &user_pin), ) diff --git a/nitrocli/src/pinentry.rs b/nitrocli/src/pinentry.rs index d54a182..1eecdd0 100644 --- a/nitrocli/src/pinentry.rs +++ b/nitrocli/src/pinentry.rs @@ -46,34 +46,66 @@ impl str::FromStr for PinType { } } -impl PinType { - fn cache_id(self) -> &'static str { - match self { - PinType::Admin => "nitrocli:admin", - PinType::User => "nitrocli:user", +#[derive(Debug)] +pub struct PinEntry { + pin_type: PinType, + model: nitrokey::Model, + serial: String, +} + +impl PinEntry { + pub fn from<D>(pin_type: PinType, device: &D) -> crate::Result<Self> + where + D: nitrokey::Device, + { + let model = device.get_model(); + let serial = device.get_serial_number()?; + Ok(Self { + pin_type, + model, + serial, + }) + } + + fn cache_id(&self) -> String { + let model = self.model.to_string().to_lowercase(); + let suffix = format!("{}:{}", model, self.serial); + + match self.pin_type { + PinType::Admin => format!("nitrocli:admin:{}", suffix), + PinType::User => format!("nitrocli:user:{}", suffix), } } - fn prompt(self) -> &'static str { - match self { + fn prompt(&self) -> &'static str { + match self.pin_type { PinType::Admin => "Admin PIN", PinType::User => "User PIN", } } - fn description(self, mode: Mode) -> &'static str { - match self { - PinType::Admin => match mode { - Mode::Choose => "Please enter a new admin PIN", - Mode::Confirm => "Please confirm the new admin PIN", - Mode::Query => "Please enter the admin PIN", + fn description(&self, mode: Mode) -> String { + format!( + "{} for\rNitrokey {} {}", + match self.pin_type { + PinType::Admin => match mode { + Mode::Choose => "Please enter a new admin PIN", + Mode::Confirm => "Please confirm the new admin PIN", + Mode::Query => "Please enter the admin PIN", + }, + PinType::User => match mode { + Mode::Choose => "Please enter a new user PIN", + Mode::Confirm => "Please confirm the new user PIN", + Mode::Query => "Please enter the user PIN", + }, }, - PinType::User => match mode { - Mode::Choose => "Please enter a new user PIN", - Mode::Confirm => "Please confirm the new user PIN", - Mode::Query => "Please enter the user PIN", - }, - } + self.model, + self.serial, + ) + } + + pub fn pin_type(&self) -> PinType { + self.pin_type } } @@ -134,18 +166,18 @@ where /// description and to decide whether a quality bar is shown in the /// dialog. pub fn inquire_pin( - pin_type: PinType, + pin_entry: &PinEntry, mode: Mode, error_msg: Option<&str>, ) -> Result<String, Error> { - let cache_id = pin_type.cache_id(); + let cache_id = pin_entry.cache_id(); let error_msg = error_msg .map(|msg| msg.replace(" ", "+")) .unwrap_or_else(|| String::from("+")); - let prompt = pin_type.prompt().replace(" ", "+"); - let description = pin_type.description(mode).replace(" ", "+"); + let prompt = pin_entry.prompt().replace(" ", "+"); + let description = pin_entry.description(mode).replace(" ", "+"); - let args = vec![cache_id, &error_msg, &prompt, &description].join(" "); + let args = vec![cache_id, error_msg, prompt, description].join(" "); let mut command = "GET_PASSPHRASE --data ".to_string(); if mode.show_quality_bar() { command += "--qualitybar "; @@ -178,9 +210,9 @@ where Err(Error::Error(format!("Unexpected response: {}", string))) } -/// Clear the cached pin of the given type. -pub fn clear_pin(pin_type: PinType) -> Result<(), Error> { - let command = "CLEAR_PASSPHRASE ".to_string() + pin_type.cache_id(); +/// Clear the cached pin represented by the given entry. +pub fn clear_pin(pin_entry: &PinEntry) -> Result<(), Error> { + let command = format!("CLEAR_PASSPHRASE {}", pin_entry.cache_id()); let output = process::Command::new("gpg-connect-agent") .arg(command) .arg("/bye") |