diff options
| -rw-r--r-- | CHANGELOG.md | 2 | ||||
| -rw-r--r-- | Cargo.lock | 90 | ||||
| -rw-r--r-- | Cargo.toml | 3 | ||||
| -rw-r--r-- | doc/nitrocli.1 | 30 | ||||
| -rw-r--r-- | src/args.rs | 5 | ||||
| -rw-r--r-- | src/commands.rs | 94 | ||||
| -rw-r--r-- | src/error.rs | 9 | ||||
| -rw-r--r-- | src/main.rs | 7 | ||||
| -rw-r--r-- | src/tests/extension_model_test.py | 52 | ||||
| -rw-r--r-- | src/tests/extensions.rs | 65 | ||||
| -rw-r--r-- | src/tests/mod.rs | 13 | ||||
| -rw-r--r-- | src/tests/run.rs | 112 |
12 files changed, 480 insertions, 2 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index b336f4c..3171b43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ Unreleased ---------- +- Added support for user provided extensions through lookup via the + `PATH` environment variable - Changed default OTP format from `hex` to `base32` - Bumped `structopt` dependency to `0.3.17` @@ -93,6 +93,7 @@ dependencies = [ "nitrokey-test-state", "regex", "structopt", + "tempfile", ] [[package]] @@ -134,6 +135,12 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a59b732ed6d5212424ed31ec9649f05652bcbc38f45f2292b27a6044e7098803" [[package]] +name = "ppv-lite86" +version = "0.2.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c36fa947111f5c62a733b652544dd0016a43ce89619538a8ef92724a6f501a20" + +[[package]] name = "proc-macro-error" version = "1.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -178,6 +185,29 @@ dependencies = [ ] [[package]] +name = "rand" +version = "0.7.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6a6b1679d49b24bbfe0c803429aa1874472f50d9b363131f0e89fc356b544d03" +dependencies = [ + "getrandom", + "libc", + "rand_chacha", + "rand_core", + "rand_hc", +] + +[[package]] +name = "rand_chacha" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f4c8ed856279c9737206bf725bf36935d8666ead7aa69b52be55af369d193402" +dependencies = [ + "ppv-lite86", + "rand_core", +] + +[[package]] name = "rand_core" version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -187,6 +217,21 @@ dependencies = [ ] [[package]] +name = "rand_hc" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ca3129af7b92a17112d59ad498c6f81eaf463253766b90396d39ea7a39d6613c" +dependencies = [ + "rand_core", +] + +[[package]] +name = "redox_syscall" +version = "0.1.57" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "41cc0f7e4d5d4544e8861606a285bb08d3e70712ccc7d2b84d7c0ccfaf4b05ce" + +[[package]] name = "regex" version = "1.3.7" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -205,6 +250,15 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7fe5bd57d1d7414c6b5ed48563a2c855d995ff777729dcd91c369ec7fea395ae" [[package]] +name = "remove_dir_all" +version = "0.5.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3acd125665422973a33ac9d3dd2df85edad0f4ae9b00dafb1a05e43a9f5ef8e7" +dependencies = [ + "winapi", +] + +[[package]] name = "structopt" version = "0.3.17" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -251,6 +305,20 @@ dependencies = [ ] [[package]] +name = "tempfile" +version = "3.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7a6e24d9338a0a5be79593e2fa15a648add6138caa803e2d5bc782c371732ca9" +dependencies = [ + "cfg-if", + "libc", + "rand", + "redox_syscall", + "remove_dir_all", + "winapi", +] + +[[package]] name = "textwrap" version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -297,3 +365,25 @@ name = "wasi" version = "0.9.0+wasi-snapshot-preview1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cccddf32554fecc6acb585f82a32a72e28b48f8c4c1883ddfeeeaa96f7d8e519" + +[[package]] +name = "winapi" +version = "0.3.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c839a674fcd7a98952e593242ea400abe93992746761e38641405d28b00f419" +dependencies = [ + "winapi-i686-pc-windows-gnu", + "winapi-x86_64-pc-windows-gnu", +] + +[[package]] +name = "winapi-i686-pc-windows-gnu" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" + +[[package]] +name = "winapi-x86_64-pc-windows-gnu" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" @@ -67,3 +67,6 @@ version = "0.1" [dev-dependencies.regex] version = "1" + +[dev-dependencies.tempfile] +version = "3.1" diff --git a/doc/nitrocli.1 b/doc/nitrocli.1 index 0d33cd6..30a57a8 100644 --- a/doc/nitrocli.1 +++ b/doc/nitrocli.1 @@ -14,8 +14,8 @@ and the password safe. .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. +If this option is not set, \fBnitrocli\fR will connect to any connected Nitrokey +Pro or Nitrokey Storage device. .TP \fB\-v\fR, \fB\-\-verbose\fR Enable additional logging and control its verbosity. Logging enabled through @@ -271,6 +271,32 @@ The admin PIN cannot be unblocked. This operation is equivalent to the unblock PIN option provided by \fBgpg\fR(1) (using the \fB\-\-change\-pin\fR option). +.SS Extensions +In addition to the above built-in commands, \fBnitrocli\fR supports +user-provided functionality in the form of extensions. An extension can be any +executable file whose filename starts with "nitrocli-" and that is discoverable +through lookup via the \fBPATH\fR environment variable. + +An extension should honor the following set of options, which are supplied to +the extension by \fBnitrocli\fR itself: +.TP +\fB\-\-nitrocli\fR \fIpath\fR +The path to the \fBnitrocli\fR binary. This path can be used to recursively +invoke \fBnitrocli\fR to implement certain functionality. This option is +guaranteed to be supplied. +.TP +\fB\-\-model pro\fR|\fBstorage\fR +Restrict connections to the given device model (see the Options section for more +details). This option is supplied only if it was provided by the user to the +invocation of \fBnitrocli\fR itself. +.TP +\fB\-\-verbosity\fR \fIlevel\fR +Control the logging verbosity by setting the log level to \fIlevel\fR. The +default level is 0, which corresponds to an invocation of \fBnitrocli\fR +without additional logging related options. Each additional occurrence of +\fB\-v\fR/\fB\-\-verbose\fR increments the log level accordingly. This option is +guaranteed to be supplied. + .SH ENVIRONMENT The program honors a set of environment variables that can be used to suppress interactive PIN entry through \fBpinentry\fR(1). The following diff --git a/src/args.rs b/src/args.rs index 56a10b4..744245c 100644 --- a/src/args.rs +++ b/src/args.rs @@ -17,6 +17,8 @@ // * along with this program. If not, see <http://www.gnu.org/licenses/>. * // ************************************************************************* +use std::ffi; + /// Provides access to a Nitrokey device #[derive(structopt::StructOpt)] #[structopt(name = "nitrocli")] @@ -82,6 +84,9 @@ Command! { Status => crate::commands::status, /// Interacts with the device's unencrypted volume Unencrypted(UnencryptedArgs) => |ctx, args: UnencryptedArgs| args.subcmd.execute(ctx), + /// An extension and its arguments. + #[structopt(external_subcommand)] + Extension(Vec<ffi::OsString>) => crate::commands::extension, ] } diff --git a/src/commands.rs b/src/commands.rs index a2b6004..5e2af91 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -17,8 +17,14 @@ // * along with this program. If not, see <http://www.gnu.org/licenses/>. * // ************************************************************************* +use std::borrow; +use std::env; +use std::ffi; use std::fmt; +use std::io; use std::mem; +use std::path; +use std::process; use std::result; use std::thread; use std::time; @@ -980,6 +986,94 @@ pub fn pws_status(ctx: &mut ExecCtx<'_>, all: bool) -> Result<()> { }) } +/// Resolve an extension provided by name to an actual path. +/// +/// Extensions are (executable) files that have the "nitrocli-" prefix +/// and are discoverable via the `PATH` environment variable. +pub(crate) fn resolve_extension( + path_var: &ffi::OsStr, + ext_name: &ffi::OsStr, +) -> Result<path::PathBuf> { + let mut bin_name = ffi::OsString::from("nitrocli-"); + bin_name.push(ext_name); + + // The std::env module has several references to the PATH environment + // variable, indicating that this name is considered platform + // independent from their perspective. We do the same. + for dir in env::split_paths(path_var) { + let mut bin_path = dir.clone(); + bin_path.push(&bin_name); + // Note that we deliberately do not check whether the file we found + // is executable. If it is not we will just fail later on with a + // permission denied error. The reasons for this behavior are two + // fold: + // 1) Checking whether a file is executable in Rust is painful (as + // of 1.37 there exists the PermissionsExt trait but it is + // available only for Unix based systems). + // 2) It is considered a better user experience to show an extension + // that we found (we list them in the help text) even if it later + // turned out to be not usable over not showing it and silently + // doing nothing -- mostly because anything residing in PATH + // should be executable anyway and given that its name also + // starts with nitrocli- we are pretty sure that's a bug on the + // user's side. + if bin_path.is_file() { + return Ok(bin_path); + } + } + + let err = if let Some(name) = bin_name.to_str() { + format!("extension {} not found", name).into() + } else { + borrow::Cow::from("extension not found") + }; + Err(io::Error::new(io::ErrorKind::NotFound, err).into()) +} + +/// Run an extension. +pub fn extension(ctx: &mut ExecCtx<'_>, args: Vec<ffi::OsString>) -> Result<()> { + // Note that while `Command` would actually honor PATH by itself, we + // do not want that behavior because it would circumvent the execution + // context we use for testing. As such, we need to do our own search. + let mut args = args.into_iter(); + let ext_name = args + .next() + .ok_or_else(|| Error::from("no extension specified"))?; + let path_var = ctx + .path + .as_ref() + .ok_or_else(|| Error::from("PATH variable not present"))?; + let ext_path = resolve_extension(&path_var, &ext_name)?; + + // Note that theoretically we could just exec the extension and be + // done. However, the problem with that approach is that it makes + // testing extension support much more nasty, because the test process + // would be overwritten in the process, requiring us to essentially + // fork & exec nitrocli beforehand -- which is much more involved from + // a cargo test context. + let mut cmd = process::Command::new(&ext_path); + + if let Some(model) = ctx.model { + let _ = cmd.args(&["--model", model.as_ref()]); + }; + + let out = cmd + // TODO: We may want to take this path from the command execution + // context. + .args(&["--nitrocli", &env::current_exe()?.to_string_lossy()]) + .args(&["--verbosity", &ctx.verbosity.to_string()]) + .args(args) + .output()?; + ctx.stdout.write_all(&out.stdout)?; + ctx.stderr.write_all(&out.stderr)?; + + if out.status.success() { + Ok(()) + } else { + Err(Error::ExtensionFailed(ext_path, out.status.code())) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/error.rs b/src/error.rs index e891da2..839c37a 100644 --- a/src/error.rs +++ b/src/error.rs @@ -19,6 +19,7 @@ use std::fmt; use std::io; +use std::path; use std::str; use std::string; @@ -47,6 +48,7 @@ pub enum Error { NitrokeyError(Option<&'static str>, nitrokey::Error), Utf8Error(str::Utf8Error), Error(String), + ExtensionFailed(path::PathBuf, Option<i32>), } impl TryInto<nitrokey::Error> for Error { @@ -113,6 +115,13 @@ impl fmt::Display for Error { Error::Utf8Error(_) => write!(f, "Encountered UTF-8 conversion error"), Error::IoError(ref e) => write!(f, "IO error: {}", e), Error::Error(ref e) => write!(f, "{}", e), + Error::ExtensionFailed(ref path, rc) => { + write!(f, "Extension {} failed", path.to_string_lossy())?; + if let Some(rc) = rc { + write!(f, " with exit code {}", rc)?; + } + Ok(()) + } } } } diff --git a/src/main.rs b/src/main.rs index 27097c9..5d4ea1d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -80,6 +80,7 @@ use std::ffi; use std::io; use std::process; use std::result; +use std::str; use crate::error::Error; @@ -121,6 +122,8 @@ pub struct ExecCtx<'io> { pub stdout: &'io mut dyn io::Write, /// See `RunCtx::stderr`. pub stderr: &'io mut dyn io::Write, + /// See `RunCtx::path`. + pub path: Option<ffi::OsString>, /// See `RunCtx::admin_pin`. pub admin_pin: Option<ffi::OsString>, /// See `RunCtx::user_pin`. @@ -153,6 +156,7 @@ fn handle_arguments(ctx: &mut RunCtx<'_>, args: Vec<String>) -> Result<()> { model: args.model, stdout: ctx.stdout, stderr: ctx.stderr, + path: ctx.path.take(), admin_pin: ctx.admin_pin.take(), user_pin: ctx.user_pin.take(), new_admin_pin: ctx.new_admin_pin.take(), @@ -180,6 +184,8 @@ pub(crate) struct RunCtx<'io> { pub stdout: &'io mut dyn io::Write, /// The `Write` object used as standard error throughout the program. pub stderr: &'io mut dyn io::Write, + /// The content of the `PATH` environment variable. + pub path: Option<ffi::OsString>, /// The admin PIN, if provided through an environment variable. pub admin_pin: Option<ffi::OsString>, /// The user PIN, if provided through an environment variable. @@ -217,6 +223,7 @@ fn main() { let ctx = &mut RunCtx { stdout: &mut stdout, stderr: &mut stderr, + path: env::var_os("PATH"), admin_pin: env::var_os(NITROCLI_ADMIN_PIN), user_pin: env::var_os(NITROCLI_USER_PIN), new_admin_pin: env::var_os(NITROCLI_NEW_ADMIN_PIN), diff --git a/src/tests/extension_model_test.py b/src/tests/extension_model_test.py new file mode 100644 index 0000000..651c8e7 --- /dev/null +++ b/src/tests/extension_model_test.py @@ -0,0 +1,52 @@ +#!/usr/bin/env python + +from argparse import ( + ArgumentParser, +) +from enum import ( + Enum, +) +from sys import ( + argv, + exit, +) + + +class Action(Enum): + """An action to perform.""" + NITROCLI = "nitrocli" + MODEL = "model" + VERBOSITY = "verbosity" + + @classmethod + def all(cls): + """Return the list of all the enum members' values.""" + return [x.value for x in cls.__members__.values()] + + +def main(args): + """The extension's main function.""" + parser = ArgumentParser() + parser.add_argument(choices=Action.all(), dest="what") + parser.add_argument("--nitrocli", action="store", default=None) + parser.add_argument("--model", action="store", default=None) + # We deliberately store the argument to this option as a string + # because we can differentiate between None and a valid value, in + # order to verify that it really is supplied. + parser.add_argument("--verbosity", action="store", default=None) + + namespace = parser.parse_args(args[1:]) + if namespace.what == Action.NITROCLI.value: + print(namespace.nitrocli) + elif namespace.what == Action.MODEL.value: + print(namespace.model) + elif namespace.what == Action.VERBOSITY.value: + print(namespace.verbosity) + else: + return 1 + + return 0 + + +if __name__ == "__main__": + exit(main(argv)) diff --git a/src/tests/extensions.rs b/src/tests/extensions.rs new file mode 100644 index 0000000..9e70181 --- /dev/null +++ b/src/tests/extensions.rs @@ -0,0 +1,65 @@ +// extensions.rs + +// ************************************************************************* +// * Copyright (C) 2020 Daniel Mueller (deso@posteo.net) * +// * * +// * This program is free software: you can redistribute it and/or modify * +// * it under the terms of the GNU General Public License as published by * +// * the Free Software Foundation, either version 3 of the License, or * +// * (at your option) any later version. * +// * * +// * This program is distributed in the hope that it will be useful, * +// * but WITHOUT ANY WARRANTY; without even the implied warranty of * +// * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * +// * GNU General Public License for more details. * +// * * +// * You should have received a copy of the GNU General Public License * +// * along with this program. If not, see <http://www.gnu.org/licenses/>. * +// ************************************************************************* + +use std::env; +use std::fs; +use std::io; + +use super::*; + +use crate::commands::resolve_extension; +use crate::Error; + +#[test] +fn discover_extensions() -> crate::Result<()> { + let dir1 = tempfile::tempdir()?; + let dir2 = tempfile::tempdir()?; + + { + let ext1_path = dir1.path().join("nitrocli-ext1"); + let ext2_path = dir1.path().join("nitrocli-ext2"); + let ext3_path = dir2.path().join("nitrocli-super-1337-extensions111one"); + let _ext1 = fs::File::create(&ext1_path)?; + let _ext2 = fs::File::create(&ext2_path)?; + let _ext3 = fs::File::create(&ext3_path)?; + + let path = env::join_paths(&[dir1.path(), dir2.path()]).map_err(|err| err.to_string())?; + assert_eq!( + resolve_extension(&path, ffi::OsStr::new("ext1"))?, + ext1_path + ); + assert_eq!( + resolve_extension(&path, ffi::OsStr::new("ext2"))?, + ext2_path + ); + assert_eq!( + resolve_extension(&path, ffi::OsStr::new("super-1337-extensions111one"))?, + ext3_path + ); + + match resolve_extension(&ffi::OsStr::new(""), ffi::OsStr::new("ext1")) { + Err(Error::IoError(err)) if err.kind() == io::ErrorKind::NotFound => { + let expected = io::Error::new(io::ErrorKind::NotFound, "extension nitrocli-ext1 not found"); + assert_eq!(err.to_string(), expected.to_string()) + } + r => panic!("Unexpected variant found: {:?}", r), + } + } + Ok(()) +} diff --git a/src/tests/mod.rs b/src/tests/mod.rs index e86f42f..e0ee876 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -24,6 +24,7 @@ use nitrokey_test::test as test_device; mod config; mod encrypted; +mod extensions; mod hidden; mod list; mod lock; @@ -93,6 +94,15 @@ impl Builder { self } + /// Set the `PATH` used for looking up extensions. + fn path<P>(mut self, path: P) -> Self + where + P: Into<ffi::OsString>, + { + self.0.path = Some(path.into()); + self + } + /// Set the password to use for certain operations. fn password<P>(mut self, password: P) -> Self where @@ -110,6 +120,7 @@ impl Builder { struct Nitrocli { model: Option<nitrokey::Model>, + path: Option<ffi::OsString>, admin_pin: Option<ffi::OsString>, user_pin: Option<ffi::OsString>, new_admin_pin: Option<ffi::OsString>, @@ -121,6 +132,7 @@ impl Nitrocli { pub fn new() -> Self { Self { model: None, + path: None, admin_pin: Some(nitrokey::DEFAULT_ADMIN_PIN.into()), user_pin: Some(nitrokey::DEFAULT_USER_PIN.into()), new_admin_pin: None, @@ -174,6 +186,7 @@ impl Nitrocli { let ctx = &mut crate::RunCtx { stdout: &mut stdout, stderr: &mut stderr, + path: self.path.clone(), admin_pin: self.admin_pin.clone(), user_pin: self.user_pin.clone(), new_admin_pin: self.new_admin_pin.clone(), diff --git a/src/tests/run.rs b/src/tests/run.rs index 22e7004..f8470ad 100644 --- a/src/tests/run.rs +++ b/src/tests/run.rs @@ -17,6 +17,11 @@ // * along with this program. If not, see <http://www.gnu.org/licenses/>. * // ************************************************************************* +use std::fs; +use std::io::Write; +use std::os::unix::fs::OpenOptionsExt; +use std::path; + use super::*; #[test] @@ -108,3 +113,110 @@ fn version_option() { test(&re, "--version"); test(&re, "-V"); } + +#[test] +fn extension() -> crate::Result<()> { + let ext_dir = tempfile::tempdir()?; + { + let mut ext = fs::OpenOptions::new() + .create(true) + .mode(0o755) + .write(true) + .open(ext_dir.path().join("nitrocli-ext"))?; + + ext.write_all( + br#"#!/usr/bin/env python +print("success") +"#, + )?; + } + + let path = ext_dir.path().as_os_str().to_os_string(); + let out = Nitrocli::make().path(path).build().handle(&["ext"])?; + assert_eq!(out, "success\n"); + Ok(()) +} + +#[test] +fn extension_failure() -> crate::Result<()> { + let ext_dir = tempfile::tempdir()?; + { + let mut ext = fs::OpenOptions::new() + .create(true) + .mode(0o755) + .write(true) + .open(ext_dir.path().join("nitrocli-ext"))?; + + ext.write_all( + br#"#!/usr/bin/env python +import sys +sys.exit(42); +"#, + )?; + } + + let path = ext_dir.path().as_os_str().to_os_string(); + let err = Nitrocli::make() + .path(path) + .build() + .handle(&["ext"]) + .unwrap_err(); + + match err { + crate::Error::ExtensionFailed(ext, rc) => { + assert_eq!(ext, ext_dir.path().join("nitrocli-ext")); + assert_eq!(rc, Some(42)); + } + _ => panic!("Unexpected error variant found: {:?}", err), + }; + Ok(()) +} + +#[test_device] +fn extension_arguments(model: nitrokey::Model) -> crate::Result<()> { + fn test<F>(model: nitrokey::Model, what: &str, args: &[&str], check: F) -> crate::Result<()> + where + F: FnOnce(&str) -> bool, + { + let ext_dir = tempfile::tempdir()?; + { + let mut ext = fs::OpenOptions::new() + .create(true) + .mode(0o755) + .write(true) + .open(ext_dir.path().join("nitrocli-ext"))?; + + ext.write_all(include_bytes!("extension_model_test.py"))?; + } + + let mut args = args.to_vec(); + args.append(&mut vec!["ext", what]); + + let path = ext_dir.path().as_os_str().to_os_string(); + let out = Nitrocli::make() + .model(model) + .path(path) + .build() + .handle(&args)?; + + assert!(check(&out), out); + Ok(()) + } + + test(model, "model", &[], |out| { + out == model.to_string().to_lowercase() + "\n" + })?; + test(model, "nitrocli", &[], |out| { + path::Path::new(out) + .file_stem() + .unwrap() + .to_str() + .unwrap() + .trim() + .contains("nitrocli") + })?; + test(model, "verbosity", &[], |out| out == "0\n")?; + test(model, "verbosity", &["-v"], |out| out == "1\n")?; + test(model, "verbosity", &["-v", "--verbose"], |out| out == "2\n")?; + Ok(()) +} |