Refactor connection handling for multiple devices

CHANGELOG.md

@@ -8,6 +8,12 @@ Unreleased
   - Added `envy` dependency in version `0.4.1`
   - Added `merge` dependency in version `0.1.0`
   - Added `directories` dependency in version `3.0.1`
+- Refactored connection handling to improve support for multiple attached
+  Nitrokey devices:
+  - Fail if multiple attached devices match the filter options (or no filter
+    options are set)
+  - Added `--serial-number` option that restricts the serial number of the
+    device to connect to
 
 
 0.3.4
@@ -50,8 +56,6 @@ Unreleased
 - Removed vendored dependencies and moved source code into repository
   root
 - Bumped `nitrokey` dependency to `0.6.0`
-- Bumped `quote` dependency to `1.0.3`
-- Bumped `syn` dependency to `1.0.14`
 
 
 0.3.1

doc/config.example.toml

@@ -4,6 +4,9 @@
 
 # The model to connect to (string, "pro" or "storage", default: not set).
 model = "pro"
+# The serial number of the device to connect to (list of strings, default:
+# empty).
+serial_numbers = ["0xf00baa", "deadbeef"]
 # Do not cache secrets (boolean, default: false).
 no_cache = true
 # The log level (integer, default: 0).

doc/nitrocli.1

@@ -1,4 +1,4 @@
-.TH NITROCLI 1 2020-09-01
+.TH NITROCLI 1 2020-09-07
 .SH NAME
 nitrocli \- access Nitrokey devices
 .SH SYNOPSIS
@@ -10,12 +10,26 @@ nitrocli \- access Nitrokey devices
 It supports the Nitrokey Pro and the Nitrokey Storage.
 It can be used to access the encrypted volume, the one-time password generator,
 and the password safe.
+.SS Device selection
+Per default, \fBnitrocli\fR connects to any attached Nitrokey device.
+You can use the \fB\-\-model\fR and \fB\-\-serial-number\fR options to select
+the device to connect to.
+\fBnitrocli\fR fails if more than one attached Nitrokey device matches this
+filter, or if multiple Nitrokey devices are attached and none of the filter
+options is set.
 .SH OPTIONS
 .TP
 \fB\-m\fR, \fB\-\-model pro\fR|\fBstorage\fR
-Restrict connections to the given device model.
-If this option is not set, nitrocli will connect to any connected Nitrokey Pro
-or Nitrokey Storage device.
+Restrict connections to the given device model, see the Device selection
+section.
+.TP
+\fB\-\-serial-number \fIserial-number\fR
+Restrict connections to the given serial number, see the Device selection
+section.
+\fIserial-number\fR must be a hex string with an optional 0x prefix.
+This option can be set multiple times to allow any of the given serial numbers.
+Nitrokey Storage devices never match this restriction as they do not expose
+their serial number in the USB device descriptor.
 .TP
 \fB\-\-no\-cache\fR
 If this option is set, nitrocli will not cache any inquired secrets using
@@ -297,6 +311,10 @@ The following values can be set in the configuration file:
 Restrict connections to the given device model (string, default: not set, see
 \fB\-\-model\fR).
 .TP
+.B serial_numbers
+Restrict connections to the given serial numbers (list of strings, default:
+empty, see \fB\-\-serial-number\fR).
+.TP
 .B no_cache
 If set to true, do not cache any inquired secrets (boolean, default: false,
 see \fB\-\-no\-cache\fR).
@@ -306,6 +324,7 @@ Set the log level (integer, default: 0, see \fB\-\-verbose\fR).
 .P
 The configuration file must use the TOML format, for example:
     model = "pro"
+    serial_numbers = ["0xf00baa", "deadbeef"]
     no_cache = false
     verbosity = 0
 
@@ -338,6 +357,10 @@ configuration (see the Config file section):
 Restrict connections to the given device model (string, default: not set, see
 \fB\-\-model\fR).
 .TP
+.B NITROCLI_SERIAL_NUMBERS
+Restrict connections to the given list of serial numbers (comma-separated list
+of strings, default: empty, see \fB\-\-serial-number\fR).
+.TP
 .B NITROCLI_NO_CACHE
 If set to true, do not cache any inquired secrets (boolean, default: false,
 see \fB\-\-no\-cache\fR).

src/args.rs

@@ -13,6 +13,16 @@ pub struct Args {
   /// Selects the device model to connect to
   #[structopt(short, long, global = true, possible_values = &DeviceModel::all_str())]
   pub model: Option<DeviceModel>,
+  /// Sets the serial number of the device to connect to. Can be set multiple times to allow
+  /// mulitple serial numbers
+  // TODO: add short options (avoid collisions)
+  #[structopt(
+    long = "serial-number",
+    global = true,
+    multiple = true,
+    number_of_values = 1
+  )]
+  pub serial_numbers: Vec<nitrokey::SerialNumber>,
   /// Disables the cache for all secrets.
   #[structopt(long, global = true)]
   pub no_cache: bool,

src/commands.rs

@@ -39,6 +39,74 @@ fn set_log_level(ctx: &mut Context<'_>) {
   nitrokey::set_log_level(log_lvl);
 }
 
+/// Find a Nitrokey device that matches the given requirements
+fn find_device(
+  model: Option<args::DeviceModel>,
+  serial_numbers: &[nitrokey::SerialNumber],
+) -> anyhow::Result<nitrokey::DeviceInfo> {
+  let devices = nitrokey::list_devices().context("Failed to enumerate Nitrokey devices")?;
+  let nkmodel: Option<nitrokey::Model> = model.map(From::from);
+  let mut iter = devices
+    .into_iter()
+    .filter(|device| nkmodel.is_none() || device.model == nkmodel)
+    .filter(|device| {
+      serial_numbers.is_empty()
+        || device
+          .serial_number
+          .map(|sn| serial_numbers.contains(&sn))
+          .unwrap_or_default()
+    });
+
+  let get_filter = || {
+    let mut filters = Vec::new();
+    if let Some(model) = model {
+      filters.push(format!("model = {}", model.as_user_facing_str()));
+    }
+    if !serial_numbers.is_empty() {
+      let serial_numbers: Vec<_> = serial_numbers
+        .as_ref()
+        .iter()
+        .map(ToString::to_string)
+        .collect();
+      filters.push(format!("serial number in [{}]", serial_numbers.join(", ")));
+    }
+    if filters.is_empty() {
+      String::new()
+    } else {
+      format!(" (filter: {})", filters.join(", "))
+    }
+  };
+  let device = iter
+    .next()
+    .with_context(|| format!("Nitrokey device not found{}", get_filter()))?;
+  anyhow::ensure!(
+    iter.next().is_none(),
+    "Multiple Nitrokey devices found{}.  Use the --model and --serial-number options to \
+    select one",
+    get_filter()
+  );
+  Ok(device)
+}
+
+/// Connect to a Nitrokey device that matches the given requirements
+fn connect<'mgr>(
+  manager: &'mgr mut nitrokey::Manager,
+  model: Option<args::DeviceModel>,
+  serial_numbers: &[nitrokey::SerialNumber],
+) -> anyhow::Result<nitrokey::DeviceWrapper<'mgr>> {
+  use std::ops::Deref as _;
+
+  let device_info = find_device(model, serial_numbers)?;
+  manager
+    .connect_path(device_info.path.deref())
+    .with_context(|| {
+      format!(
+        "Failed to connect to Nitrokey device at path {}",
+        device_info.path
+      )
+    })
+}
+
 /// Connect to any Nitrokey device and do something with it.
 fn with_device<F>(ctx: &mut Context<'_>, op: F) -> anyhow::Result<()>
 where
@@ -49,13 +117,7 @@ where
 
   set_log_level(ctx);
 
-  let device = match ctx.config.model {
-    Some(model) => manager.connect_model(model.into()).with_context(|| {
-      anyhow::anyhow!("Nitrokey {} device not found", model.as_user_facing_str())
-    })?,
-    None => manager.connect().context("Nitrokey device not found")?,
-  };
-
+  let device = connect(&mut manager, ctx.config.model, &ctx.config.serial_numbers)?;
   op(ctx, device)
 }
 
@@ -75,10 +137,18 @@ where
     }
   }
 
-  let device = manager
-    .connect_storage()
-    .context("Nitrokey Storage device not found")?;
-  op(ctx, device)
+  let device = connect(
+    &mut manager,
+    Some(args::DeviceModel::Storage),
+    &ctx.config.serial_numbers,
+  )?;
+  if let nitrokey::DeviceWrapper::Storage(storage) = device {
+    op(ctx, storage)
+  } else {
+    Err(anyhow::anyhow!(
+      "Programming error: connect returned a wrong model"
+    ))
+  }
 }
 
 /// Connect to any Nitrokey device, retrieve a password safe handle, and

src/config.rs

@@ -20,10 +20,14 @@ const CONFIG_FILE: &str = "config.toml";
 
 /// The configuration for nitrocli, usually read from configuration
 /// files and environment variables.
-#[derive(Clone, Copy, Debug, Default, PartialEq, merge::Merge, serde::Deserialize)]
+#[derive(Clone, Debug, Default, PartialEq, merge::Merge, serde::Deserialize)]
 pub struct Config {
   /// The model to connect to.
   pub model: Option<args::DeviceModel>,
+  /// The serial numbers of the device to connect to.
+  #[merge(strategy = merge::vec::overwrite_empty)]
+  #[serde(default, deserialize_with = "deserialize_serial_number_vec")]
+  pub serial_numbers: Vec<nitrokey::SerialNumber>,
   /// Whether to bypass the cache for all secrets or not.
   #[merge(strategy = merge::bool::overwrite_false)]
   #[serde(default)]
@@ -34,6 +38,21 @@ pub struct Config {
   pub verbosity: u8,
 }
 
+fn deserialize_serial_number_vec<'de, D>(d: D) -> Result<Vec<nitrokey::SerialNumber>, D::Error>
+where
+  D: serde::Deserializer<'de>,
+{
+  use serde::de::Error as _;
+  use std::str::FromStr as _;
+
+  let strings: Vec<String> = serde::Deserialize::deserialize(d).map_err(D::Error::custom)?;
+  let result: Result<Vec<_>, _> = strings
+    .iter()
+    .map(|s| nitrokey::SerialNumber::from_str(s))
+    .collect();
+  result.map_err(D::Error::custom)
+}
+
 impl Config {
   pub fn load() -> anyhow::Result<Self> {
     use merge::Merge as _;
@@ -51,6 +70,10 @@ impl Config {
     if args.model.is_some() {
       self.model = args.model;
     }
+    if !args.serial_numbers.is_empty() {
+      // TODO: don’t clone
+      self.serial_numbers = args.serial_numbers.clone();
+    }
     if args.no_cache {
       self.no_cache = true;
     }

src/tests/status.rs

@@ -11,11 +11,7 @@ fn not_found_raw() {
 
   assert_ne!(rc, 0);
   assert_eq!(out, b"");
-  let expected = r#"Nitrokey device not found
-
-Caused by:
-    Communication error: Could not connect to a Nitrokey device
-"#;
+  let expected = "Nitrokey device not found\n";
   assert_eq!(err, expected.as_bytes());
 }