From 674e2e93c5dc1811928fc0fcea540509c3f44d93 Mon Sep 17 00:00:00 2001 From: ryan Date: Mon, 24 Feb 2025 17:44:21 -0500 Subject: [PATCH] keyfork: restructure CLI commands to act more like the other commands --- Cargo.lock | 49 ++- Cargo.toml | 1 + crates/daemon/keyforkd/Cargo.toml | 2 +- crates/keyfork/Cargo.toml | 2 + crates/keyfork/src/clap_ext.rs | 70 +++- crates/keyfork/src/cli/derive.rs | 217 +++++++++-- crates/keyfork/src/cli/mnemonic.rs | 393 ++++++++++++-------- crates/keyfork/src/cli/provision/mod.rs | 40 +- crates/keyfork/src/cli/provision/openpgp.rs | 41 +- crates/keyfork/src/config.rs | 6 +- crates/keyfork/src/openpgp_card.rs | 67 +++- 11 files changed, 624 insertions(+), 264 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a17b4dd..1176fe7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1381,10 +1381,22 @@ dependencies = [ "cfg-if", "js-sys", "libc", - "wasi", + "wasi 0.11.0+wasi-snapshot-preview1", "wasm-bindgen", ] +[[package]] +name = "getrandom" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "43a49c392881ce6d5c3b8cb70f98717b7c07aabbdff06687b9030dbfbe2725f8" +dependencies = [ + "cfg-if", + "libc", + "wasi 0.13.3+wasi-0.2.2", + "windows-targets", +] + [[package]] name = "ghash" version = "0.5.1" @@ -1808,7 +1820,9 @@ dependencies = [ "openpgp-card-sequoia", "sequoia-openpgp", "serde", + "shlex", "smex", + "tempfile", "thiserror", "tokio", ] @@ -2244,7 +2258,7 @@ dependencies = [ "hermit-abi 0.3.9", "libc", "log", - "wasi", + "wasi 0.11.0+wasi-snapshot-preview1", "windows-sys 0.52.0", ] @@ -2254,7 +2268,7 @@ version = "7.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "44e6ff4a94e5d34a1fd5abbd39418074646e2fa51b257198701330f22fcd6936" dependencies = [ - "getrandom", + "getrandom 0.2.15", "libc", "nettle-sys", "thiserror", @@ -2820,7 +2834,7 @@ version = "0.6.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ec0be4795e2f6a28069bec0b5ff3e2ac9bafc99e6a9a7dc3547996c5c816922c" dependencies = [ - "getrandom", + "getrandom 0.2.15", ] [[package]] @@ -2838,7 +2852,7 @@ version = "0.4.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ba009ff324d1fc1b900bd1fdb31564febe58a8ccc8a6fdbb93b543d33b13ca43" dependencies = [ - "getrandom", + "getrandom 0.2.15", "libredox", "thiserror", ] @@ -3067,7 +3081,7 @@ dependencies = [ "ed25519", "ed25519-dalek", "flate2", - "getrandom", + "getrandom 0.2.15", "idea", "idna", "lalrpop", @@ -3355,12 +3369,13 @@ dependencies = [ [[package]] name = "tempfile" -version = "3.14.0" +version = "3.17.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "28cce251fcbc87fac86a866eeb0d6c2d536fc16d06f184bb61aeae11aa4cee0c" +checksum = "22e5a0acb1f3f55f65cc4a866c361b2fb2a0ff6366785ae6fbb5f85df07ba230" dependencies = [ "cfg-if", "fastrand", + "getrandom 0.3.1", "once_cell", "rustix", "windows-sys 0.59.0", @@ -3726,6 +3741,15 @@ version = "0.11.0+wasi-snapshot-preview1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9c8d87e72b64a3b4db28d11ce29237c246188f4f51057d65a7eab63b7987e423" +[[package]] +name = "wasi" +version = "0.13.3+wasi-0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "26816d2e1a4a36a2940b96c5296ce403917633dff8f3440e9b236ed6f6bacad2" +dependencies = [ + "wit-bindgen-rt", +] + [[package]] name = "wasm-bindgen" version = "0.2.95" @@ -3937,6 +3961,15 @@ version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" +[[package]] +name = "wit-bindgen-rt" +version = "0.33.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3268f3d866458b787f390cf61f4bbb563b922d091359f9608842999eaee3943c" +dependencies = [ + "bitflags 2.6.0", +] + [[package]] name = "write16" version = "1.0.0" diff --git a/Cargo.toml b/Cargo.toml index b30e5da..2682a35 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -76,6 +76,7 @@ thiserror = "1.0.56" tokio = "1.35.1" v4l = "0.14.0" base64 = "0.22.1" +tempfile = "3.17.1" [profile.release] debug = true diff --git a/crates/daemon/keyforkd/Cargo.toml b/crates/daemon/keyforkd/Cargo.toml index b4f78d6..3893370 100644 --- a/crates/daemon/keyforkd/Cargo.toml +++ b/crates/daemon/keyforkd/Cargo.toml @@ -32,7 +32,7 @@ tower = { version = "0.4.13", features = ["tokio", "util"] } # Personally audited thiserror = { workspace = true } serde = { workspace = true } -tempfile = { version = "3.10.0", default-features = false } +tempfile = { workspace = true } [dev-dependencies] hex-literal = { workspace = true } diff --git a/crates/keyfork/Cargo.toml b/crates/keyfork/Cargo.toml index 836e284..eda5c26 100644 --- a/crates/keyfork/Cargo.toml +++ b/crates/keyfork/Cargo.toml @@ -48,3 +48,5 @@ sequoia-openpgp = { workspace = true } keyforkd-models.workspace = true base64.workspace = true nix = { version = "0.29.0", default-features = false, features = ["process"] } +shlex = "1.3.0" +tempfile.workspace = true diff --git a/crates/keyfork/src/clap_ext.rs b/crates/keyfork/src/clap_ext.rs index 4cb0eb7..664f0eb 100644 --- a/crates/keyfork/src/clap_ext.rs +++ b/crates/keyfork/src/clap_ext.rs @@ -2,20 +2,6 @@ use std::{collections::HashMap, str::FromStr}; -/// A helper struct for clap arguments that can contain additional arguments. For example: -/// `keyfork mnemonic generate --encrypt-to cert.asc,output=encrypted.asc`. -#[derive(Clone, Debug)] -pub struct ValueWithOptions -where - T::Err: std::error::Error, -{ - /// A mapping between keys and values. - pub values: HashMap, - - /// The first variable for the argument, such as a [`PathBuf`]. - pub inner: T, -} - /// An error that occurred while parsing a base value or its #[derive(Debug, thiserror::Error)] pub enum ValueParseError { @@ -32,6 +18,62 @@ pub enum ValueParseError { BadKeyValue, } +/// A helper struct to parse key-value arguments, without any prior argument. +#[derive(Clone, Debug, Default)] +pub struct Options { + /// The values provided. + pub values: HashMap, +} + +impl std::fmt::Display for Options { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let mut iter = self.values.iter().peekable(); + while let Some((key, value)) = iter.next() { + write!(f, "{key}={value}")?; + if iter.peek().is_some() { + write!(f, ",")?; + } + } + Ok(()) + } +} + +impl FromStr for Options { + type Err = ValueParseError; + + fn from_str(s: &str) -> Result { + if s.is_empty() { + return Ok(Default::default()) + } + let values = s + .split(',') + .map(|value| { + let [k, v] = value + .splitn(2, '=') + .collect::>() + .try_into() + .map_err(|_| ValueParseError::BadKeyValue)?; + Ok((k.to_string(), v.to_string())) + }) + .collect::, ValueParseError>>()?; + Ok(Self { values }) + } +} + +/// A helper struct for clap arguments that can contain additional arguments. For example: +/// `keyfork mnemonic generate --encrypt-to cert.asc,output=encrypted.asc`. +#[derive(Clone, Debug)] +pub struct ValueWithOptions +where + T::Err: std::error::Error, +{ + /// A mapping between keys and values. + pub values: HashMap, + + /// The first variable for the argument, such as a [`PathBuf`]. + pub inner: T, +} + impl FromStr for ValueWithOptions where ::Err: std::error::Error, diff --git a/crates/keyfork/src/cli/derive.rs b/crates/keyfork/src/cli/derive.rs index 2811856..76d095c 100644 --- a/crates/keyfork/src/cli/derive.rs +++ b/crates/keyfork/src/cli/derive.rs @@ -1,25 +1,41 @@ use super::Keyfork; use clap::{Args, Parser, Subcommand, ValueEnum}; +use std::{fmt::Display, io::Write, path::PathBuf}; -use keyfork_derive_openpgp::{ - openpgp::{ - armor::{Kind, Writer}, - packet::UserID, - serialize::Marshal, - types::KeyFlags, - }, - XPrvKey, +use keyfork_derive_openpgp::openpgp::{ + armor::{Kind, Writer}, + packet::UserID, + serialize::Marshal, + types::KeyFlags, + Cert, }; use keyfork_derive_path_data::paths; use keyfork_derive_util::{ - request::{DerivationAlgorithm, DerivationRequest, DerivationResponse}, - DerivationIndex, DerivationPath, IndexError, + request::DerivationAlgorithm, DerivationIndex, DerivationPath, ExtendedPrivateKey as XPrv, + IndexError, PrivateKey, }; use keyforkd_client::Client; -use keyforkd_models::Request; + +type OptWrite = Option>; type Result> = std::result::Result; +fn create(path: &std::path::Path) -> std::io::Result { + eprintln!("Writing derived key to: {path}", path=path.display()); + std::fs::File::create(path) +} + +pub trait Deriver { + type Prv: PrivateKey + Clone; + const DERIVATION_ALGORITHM: DerivationAlgorithm; + + fn derivation_path(&self) -> DerivationPath; + + fn derive_with_xprv(&self, writer: OptWrite, xprv: XPrv) -> Result<()>; + + fn derive_public_with_xprv(&self, writer: OptWrite, xprv: XPrv) -> Result<()>; +} + #[derive(Subcommand, Clone, Debug)] pub enum DeriveSubcommands { /// Derive an OpenPGP Transferable Secret Key (private key). The key is encoded using OpenPGP @@ -34,7 +50,7 @@ pub enum DeriveSubcommands { #[command(name = "openpgp")] OpenPGP(OpenPGP), - /// Derive a bare key for a specific algorithm, in a given format. + /// Derive an Ed25519 key for a specific algorithm, in a given format. Key(Key), } @@ -83,6 +99,18 @@ impl std::str::FromStr for Slug { } } +impl Display for Slug { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match (self.0.inner() & (0b1 << 31)).to_be_bytes().as_slice() { + [0, 0, 0, 0] => Ok(()), + [0, 0, 0, bytes @ ..] | [0, 0, bytes @ ..] | [0, bytes @ ..] | [bytes @ ..] => f + .write_str( + std::str::from_utf8(&bytes[..]).expect("slug constructed from non-utf8"), + ), + } + } +} + #[derive(Args, Clone, Debug)] pub struct Key { /// The derivation algorithm to derive a key for. @@ -98,18 +126,34 @@ pub struct Key { } impl DeriveSubcommands { - fn handle(&self, account: DerivationIndex) -> Result<()> { + fn handle(&self, account: DerivationIndex, is_public: bool, writer: OptWrite) -> Result<()> { match self { - DeriveSubcommands::OpenPGP(opgp) => opgp.handle(account), - DeriveSubcommands::Key(key) => key.handle(account), + DeriveSubcommands::OpenPGP(opgp) => { + let path = opgp.derivation_path(); + let xprv = Client::discover_socket()? + .request_xprv::<::Prv>(&path.chain_push(account))?; + if is_public { + opgp.derive_public_with_xprv(writer, xprv) + } else { + opgp.derive_with_xprv(writer, xprv) + } + } + DeriveSubcommands::Key(key) => { + let path = key.derivation_path(); + let xprv = Client::discover_socket()? + .request_xprv::<::Prv>(&path.chain_push(account))?; + if is_public { + key.derive_public_with_xprv(writer, xprv) + } else { + key.derive_with_xprv(writer, xprv) + } + } } } } impl OpenPGP { - pub fn handle(&self, account: DerivationIndex) -> Result<()> { - let path = paths::OPENPGP.clone().chain_push(account); - // TODO: should this be customizable? + fn cert_from_xprv(&self, xprv: keyfork_derive_openpgp::XPrv) -> Result { let subkeys = vec![ KeyFlags::empty().set_certification(), KeyFlags::empty().set_signing(), @@ -118,40 +162,100 @@ impl OpenPGP { .set_storage_encryption(), KeyFlags::empty().set_authentication(), ]; - let xprv = Client::discover_socket()?.request_xprv::(&path)?; - let default_userid = UserID::from(self.user_id.as_str()); - let cert = keyfork_derive_openpgp::derive(xprv, &subkeys, &default_userid)?; - let mut w = Writer::new(std::io::stdout(), Kind::SecretKey)?; + let userid = UserID::from(&*self.user_id); + keyfork_derive_openpgp::derive(xprv, &subkeys, &userid).map_err(Into::into) + } +} +impl Deriver for OpenPGP { + type Prv = keyfork_derive_openpgp::XPrvKey; + const DERIVATION_ALGORITHM: DerivationAlgorithm = DerivationAlgorithm::Ed25519; + + fn derivation_path(&self) -> DerivationPath { + paths::OPENPGP.clone() + } + + fn derive_with_xprv(&self, writer: OptWrite, xprv: XPrv) -> Result<()> { + let cert = self.cert_from_xprv(xprv)?; + let writer = match writer { + Some(w) => w, + None => { + let path = PathBuf::from(cert.fingerprint().to_string()).with_extension("asc"); + let file = create(&path)?; + Box::new(file) + } + }; + let mut writer = Writer::new(writer, Kind::SecretKey)?; for packet in cert.as_tsk().into_packets() { - packet.serialize(&mut w)?; + packet.serialize(&mut writer)?; } + writer.finalize()?; + Ok(()) + } - w.finalize()?; + fn derive_public_with_xprv(&self, writer: OptWrite, xprv: XPrv) -> Result<()> { + let cert = self.cert_from_xprv(xprv)?; + let writer = match writer { + Some(w) => w, + None => { + let path = PathBuf::from(cert.fingerprint().to_string()).with_extension("asc"); + let file = create(&path)?; + Box::new(file) + } + }; + let mut writer = Writer::new(writer, Kind::PublicKey)?; + for packet in cert.into_packets2() { + packet.serialize(&mut writer)?; + } + writer.finalize()?; Ok(()) } } -impl Key { - pub fn handle(&self, account: DerivationIndex) -> Result<()> { - let mut client = keyforkd_client::Client::discover_socket()?; - let path = DerivationPath::default() - .chain_push(self.slug.0.clone()) - .chain_push(account); - let request = DerivationRequest::new(self.derivation_algorithm.clone(), &path); - let request = Request::Derivation(request); - let derived_key: DerivationResponse = client.request(&request)?.try_into()?; +impl Deriver for Key { + // HACK: We're abusing that we use the same key as OpenPGP. Maybe we should use ed25519_dalek. + type Prv = keyfork_derive_openpgp::XPrvKey; + const DERIVATION_ALGORITHM: DerivationAlgorithm = DerivationAlgorithm::Ed25519; - let formatted = match self.format { - KeyFormat::Hex => smex::encode(derived_key.data), + fn derivation_path(&self) -> DerivationPath { + DerivationPath::default().chain_push(self.slug.0.clone()) + } + + fn derive_with_xprv(&self, writer: OptWrite, xprv: XPrv) -> Result<()> { + let (formatted, ext) = match self.format { + KeyFormat::Hex => (smex::encode(xprv.private_key().to_bytes()), "hex"), KeyFormat::Base64 => { use base64::prelude::*; - BASE64_STANDARD.encode(derived_key.data) + (BASE64_STANDARD.encode(xprv.private_key().to_bytes()), "b64") } }; + let filename = + PathBuf::from(smex::encode(xprv.public_key().to_bytes())).with_extension(ext); + if let Some(mut writer) = writer { + writeln!(writer, "{formatted}")?; + } else { + std::fs::write(&filename, formatted)?; + } - eprintln!("{formatted}"); + Ok(()) + } + + fn derive_public_with_xprv(&self, writer: OptWrite, xprv: XPrv) -> Result<()> { + let (formatted, ext) = match self.format { + KeyFormat::Hex => (smex::encode(xprv.public_key().to_bytes()), "hex"), + KeyFormat::Base64 => { + use base64::prelude::*; + (BASE64_STANDARD.encode(xprv.public_key().to_bytes()), "b64") + } + }; + let filename = + PathBuf::from(smex::encode(xprv.public_key().to_bytes())).with_extension(ext); + if let Some(mut writer) = writer { + writeln!(writer, "{formatted}")?; + } else { + std::fs::write(&filename, formatted)?; + } Ok(()) } } @@ -159,7 +263,7 @@ impl Key { #[derive(Parser, Debug, Clone)] pub struct Derive { #[command(subcommand)] - command: DeriveSubcommands, + pub(crate) command: DeriveSubcommands, /// Account ID. Required for all derivations. /// @@ -167,12 +271,45 @@ pub struct Derive { /// account ID can often come as a hindrance in the future. As such, it is always required. If /// the account ID is not relevant, it is assumed to be `0`. #[arg(long, global = true, default_value = "0")] - account_id: u32, + pub(crate) account_id: u32, + + /// Whether derivation should return the public key or a private key. + #[arg(long, global = true)] + pub(crate) public: bool, + + /// Whether the file should be written to standard output, or to a filename generated by the + /// derivation system. + #[arg(long, global = true, default_value = "false")] + pub to_stdout: bool, + + /// The file to write the derived public key to, if not standard output. If omitted, a filename + /// will be generated by the relevant deriver. + #[arg(long, global = true, conflicts_with = "to_stdout")] + pub output: Option, } impl Derive { pub fn handle(&self, _k: &Keyfork) -> Result<()> { let account = DerivationIndex::new(self.account_id, true)?; - self.command.handle(account) + let writer = if let Some(output) = self.output.as_deref() { + Some(Box::new(std::fs::File::create(output)?) as Box) + } else if self.to_stdout { + Some(Box::new(std::io::stdout()) as Box) + } else { + None + }; + self.command.handle(account, self.public, writer) + } +} + +impl std::str::FromStr for Derive { + type Err = clap::Error; + + fn from_str(s: &str) -> std::result::Result { + Derive::try_parse_from( + [String::from("derive")] + .into_iter() + .chain(shlex::Shlex::new(s)), + ) } } diff --git a/crates/keyfork/src/cli/mnemonic.rs b/crates/keyfork/src/cli/mnemonic.rs index d680dbe..89a68f6 100644 --- a/crates/keyfork/src/cli/mnemonic.rs +++ b/crates/keyfork/src/cli/mnemonic.rs @@ -1,12 +1,14 @@ -use super::provision; -use super::Keyfork; +use super::{ + derive::{self, Deriver}, + provision, Keyfork, +}; use crate::{clap_ext::*, config}; use clap::{builder::PossibleValue, Parser, Subcommand, ValueEnum}; use std::{ collections::HashMap, fmt::Display, fs::File, - io::Write, + io::{IsTerminal, Write}, path::{Path, PathBuf}, str::FromStr, }; @@ -25,6 +27,7 @@ use keyfork_derive_openpgp::{ }, XPrv, }; +use keyfork_derive_util::DerivationIndex; use keyfork_prompt::default_handler; use keyfork_shard::{openpgp::OpenPGP, Format}; @@ -179,6 +182,19 @@ pub enum MnemonicSubcommands { #[arg(long, default_value_t = Default::default())] size: SeedSize, + /// Derive a key. By default, a private key is derived. Unlike other arguments in this + /// file, arguments must be passed using the format similar to the CLI. For example: + /// `--derive='openpgp --public "Ryan Heywood "'` would be synonymous + /// with starting the Keyfork daemon with the provided mnemonic, then running + /// `keyfork derive openpgp --public "Ryan Heywood "`. + /// + /// The output of the derived key is written to a filename based on the content of the key; + /// for instance, OpenPGP keys are written to a file identifiable by the certificate's + /// fingerprint. This behavior can be changed by using the `--to-stdout` or `--output` + /// modifiers to the `--derive` command. + #[arg(long)] + derive: Option, + /// Encrypt the mnemonic to an OpenPGP certificate in the provided path. /// /// When given arguments in the format `--encrypt-to input.asc,output=output.asc`, the @@ -191,7 +207,7 @@ pub enum MnemonicSubcommands { /// Shard the mnemonic to the certificates in the given Shardfile. Requires a decrypt /// operation on the Shardfile to access the metadata and certificates. /// - /// When given arguments in the format `--encrypt-to input.asc,output=output.asc`, the + /// When given arguments in the format `--shard-to input.asc,output=output.asc`, the /// output of the encryption will be written to `output.asc`. Otherwise, the default /// behavior is to write the output to `input.new.asc`. If the output file already exists, /// it will not be overwritten, and the command will exit unsuccessfully. @@ -216,48 +232,39 @@ pub enum MnemonicSubcommands { /// Encrypt the mnemonic to an OpenPGP certificate derived from the mnemonic, writing the /// output to the provided path. This command must be run in combination with - /// `--provision openpgp-card` or another relevant provisioner, to ensure the newly - /// generated mnemonic would be decryptable by some form of provisioned hardware. + /// `--provision openpgp-card`, `--derive openpgp`, or another OpenPGP key derivation + /// mechanism, to ensure the generated mnemonic would be decryptable. /// - /// When given arguments in the format `--encrypt-to-self encrypted.asc,output=cert.asc`, - /// the output of the OpenPGP certificate will be written to `cert.asc`, while the output - /// of the encryption will be written to `encrypted.asc`. Otherwise, the - /// default behavior is to write the certificate to a file named after the certificate's - /// fingerprint. If either output file already exists, it will not be overwritten, and the - /// command will exit unsuccessfully. This functionality must happen regardless if a - /// provisioner output is specified, as the certificate is then used to encrypt the - /// mnemonic. - /// - /// Additionally, when given the `account=` option (which must match the `account=` option - /// of the relevant provisioner), the given account will be used instead of the default - /// account of 0. - /// - /// Because a new OpenPGP cert needs to be created, a User ID can also be supplied, using - /// the option `userid=`. It can contain any characters that are not a comma. - /// If any other operation generating an OpenPGP key has a `userid=` field, and this - /// operation doesn't, that User ID will be used instead. + /// When used in combination with `--derive` or `--provision` with OpenPGP configurations, + /// the default behavior is to encrypt the mnemonic to all derived and provisioned + /// accounts. By default, the account `0` is used. #[arg(long)] - encrypt_to_self: Option>, + encrypt_to_self: Option, /// Provision a key derived from the mnemonic to a piece of hardware such as an OpenPGP /// smartcard. This argument is required when used with `--encrypt-to-self`. /// - /// Additional arguments, such as the amount of hardware to provision and the - /// account to use when deriving, can be specified by using (for example) - /// `--provision openpgp-card,count=2,account=1`. - /// - /// Provisioners may output their public key, if necessary. The file path may be chosen - /// based on the provided `output` field, or automatically determined based on the content - /// of the key, such as an OpenPGP fingerprint or a public key hash. If automatically - /// generated, the filename will be printed. - /// - /// If the OpenPGP Card provisioner is selected, because a new OpenPGP cert needs to be - /// created, a User ID can also be supplied, using the option `userid=`. It - /// can contain any characters that are not a comma. If any other operation generating an - /// OpenPGP key has a `userid=` field, and this operation doesn't, that User ID will be - /// used instead. + /// Provisioners may choose to output a public key to the current directory by default, but + /// this functionality may be altered on a by-provisioner basis by providing the `output=` + /// option to `--provisioner-config`. Additionally, Keyfork may choose to disable + /// provisioner output if a matching public key has been derived using `--derive`, which + /// may allow for controlling additional metadata that is not relevant to the provisioned + /// keys, such as an OpenPGP User ID. #[arg(long)] - provision: Option>, + provision: Option, + + /// The amount of times the provisioner should be run. If provisioning multiple devices at + /// once, this number should be specified to the number of devices, and all devices should + /// be plugged into the system at the same time. + #[arg(long, requires = "provision", default_value = "1")] + provision_count: usize, + + /// The configuration to pass to the provisioner. These values are specific to each + /// provisioner, and should be provided in a `key=value,key=value` format. Most + /// provisioners only expect an `output=` option, to be used in place of the default output + /// path, if the provisioner needs to write data to a file, such as an OpenPGP certificate. + #[arg(long, requires = "provision", default_value_t = Options::default())] + provision_config: Options, }, } @@ -324,7 +331,7 @@ fn do_encrypt_to( literal_message.write_all(b"\n")?; literal_message.finalize()?; - let mut file = File::create_new(&output_file).map_err(context_stub(&output_file))?; + let mut file = File::create(&output_file).map_err(context_stub(&output_file))?; if is_armored { let mut writer = Writer::new(file, Kind::Message)?; writer.write_all(&output)?; @@ -339,64 +346,53 @@ fn do_encrypt_to( fn do_encrypt_to_self( mnemonic: &keyfork_mnemonic::Mnemonic, path: &Path, - options: &StringMap, + accounts: &[keyfork_derive_util::DerivationIndex], ) -> Result<(), Box> { - let account = options - .get("account") - .map(|account| u32::from_str(account)) - .transpose()? - .unwrap_or(0); - let account_index = keyfork_derive_util::DerivationIndex::new(account, true)?; + let mut certs = vec![]; - let userid = options - .get("userid") - .map(|userid| UserID::from(userid.as_str())); + for account in accounts.iter().cloned() { + let userid = UserID::from("Keyfork Temporary Key"); - let subkeys = [ - KeyFlags::empty().set_certification(), - KeyFlags::empty().set_signing(), - KeyFlags::empty() - .set_transport_encryption() - .set_storage_encryption(), - KeyFlags::empty().set_authentication(), - ]; + let subkeys = [ + KeyFlags::empty().set_certification(), + KeyFlags::empty().set_signing(), + KeyFlags::empty() + .set_transport_encryption() + .set_storage_encryption(), + KeyFlags::empty().set_authentication(), + ]; - let seed = mnemonic.generate_seed(None); - let xprv = XPrv::new(seed)?; - let derivation_path = keyfork_derive_path_data::paths::OPENPGP - .clone() - .chain_push(account_index); + let seed = mnemonic.generate_seed(None); + let xprv = XPrv::new(seed)?; + let derivation_path = keyfork_derive_path_data::paths::OPENPGP + .clone() + .chain_push(account); - let cert = keyfork_derive_openpgp::derive( - xprv.derive_path(&derivation_path)?, - &subkeys, - &userid.unwrap_or(UserID::from("Keyfork-Generated Key")), - )?; + let cert = + keyfork_derive_openpgp::derive(xprv.derive_path(&derivation_path)?, &subkeys, &userid)?; - let cert_path = match options.get("output") { - Some(path) => PathBuf::from(path), - None => { - let path = PathBuf::from(cert.fingerprint().to_string()).with_extension("asc"); - eprintln!( - "Writing OpenPGP certificate to default path: {path}", - path = path.display() - ); - path - } - }; + certs.push(cert); + } - let file = File::create_new(&cert_path).map_err(context_stub(&cert_path))?; - let mut writer = Writer::new(file, Kind::PublicKey)?; - cert.serialize(&mut writer)?; + let mut file = tempfile::NamedTempFile::new()?; + + let mut writer = Writer::new(&mut file, Kind::PublicKey)?; + for cert in certs { + cert.serialize(&mut writer)?; + } writer.finalize()?; + let temp_path = file.into_temp_path(); + // a sneaky bit of DRY do_encrypt_to( mnemonic, - &cert_path, + &temp_path, &StringMap::from([(String::from("output"), path.to_string_lossy().to_string())]), )?; + temp_path.close()?; + Ok(()) } @@ -449,7 +445,7 @@ fn do_shard( let mut output = vec![]; openpgp.shard_and_encrypt(threshold, max, mnemonic.as_bytes(), &certs[..], &mut output)?; - let mut file = File::create_new(&output_file).map_err(context_stub(&output_file))?; + let mut file = File::create(&output_file).map_err(context_stub(&output_file))?; if is_armored { file.write_all(&output)?; } else { @@ -494,7 +490,7 @@ fn do_shard_to( &mut output, )?; - let mut file = File::create_new(&output_file).map_err(context_stub(&output_file))?; + let mut file = File::create(&output_file).map_err(context_stub(&output_file))?; if is_armored { file.write_all(&output)?; } else { @@ -512,51 +508,105 @@ fn do_shard_to( fn do_provision( mnemonic: &keyfork_mnemonic::Mnemonic, - provisioner: &provision::Provisioner, - options: &StringMap, + provision: &provision::Provision, + count: usize, + config: &HashMap, ) -> Result<(), Box> { - let mut options = options.clone(); - let account = options - .remove("account") - .map(|account| u32::from_str(&account)) - .transpose()? - .unwrap_or(0); - let identifier = options - .remove("identifier") - .map(|s| s.split('.').map(String::from).collect::>()) - .map(Result::<_, Box>::Ok) - .unwrap_or_else(|| { - Ok(provisioner - .discover()? - .into_iter() - .map(|(identifier, _)| identifier) - .collect()) - })?; - let count = options - .remove("count") - .map(|count| usize::from_str(&count)) - .transpose()? - .unwrap_or(identifier.len()); - - assert_eq!( - count, - identifier.len(), - "amount of identifiers discovered or provided did not match provisioner count" + assert!( + provision.subcommand.is_none(), + "provisioner was given a subcommand; this functionality is not supported" ); - for (_, identifier) in (0..count).zip(identifier.into_iter()) { - let provisioner_config = config::Provisioner { - account, - identifier, - metadata: Some(options.clone()), - }; + let identifiers = match &provision.identifier { + Some(identifier) => { + vec![identifier.clone()] + } + None => provision + .provisioner_name + .discover()? + .into_iter() + .map(|(name, _ctx)| name) + .collect(), + }; - provisioner.provision_with_mnemonic(mnemonic, provisioner_config.clone())?; + assert_eq!( + identifiers.len(), + count, + "amount of provisionable devices discovered did not match provisioner count" + ); + + for identifier in identifiers { + let provisioner_with_identifier = provision::Provision { + identifier: Some(identifier), + ..provision.clone() + }; + let mut provisioner = config::Provisioner::try_from(provisioner_with_identifier)?; + match &mut provisioner.metadata { + Some(metadata) => { + metadata.extend(config.clone().into_iter()); + } + metadata @ None => { + *metadata = Some(config.clone()); + } + }; + provision + .provisioner_name + .provision_with_mnemonic(mnemonic, provisioner)?; } Ok(()) } +fn do_derive( + mnemonic: &keyfork_mnemonic::MnemonicBase, + deriver: &derive::Derive, +) -> Result<(), Box> { + let writer = if let Some(output) = deriver.output.as_deref() { + Some(Box::new(std::fs::File::create(output)?) as Box) + } else if deriver.to_stdout { + Some(Box::new(std::io::stdout()) as Box) + } else { + None + }; + match deriver { + derive::Derive { + command: derive::DeriveSubcommands::OpenPGP(opgp), + account_id, + public, + .. + } => { + use keyfork_derive_openpgp::XPrv; + let root_xprv = XPrv::new(mnemonic.generate_seed(None))?; + let account = DerivationIndex::new(*account_id, true)?; + let derived = root_xprv.derive_path(&opgp.derivation_path().chain_push(account))?; + if *public { + opgp.derive_public_with_xprv(writer, derived)?; + } else { + opgp.derive_with_xprv(writer, derived)?; + } + } + derive::Derive { + command: derive::DeriveSubcommands::Key(key), + account_id, + public, + .. + } => { + // HACK: We're abusing that we use the same key as OpenPGP. Maybe + // we should use ed25519_dalek. + use keyfork_derive_openpgp::XPrv; + let root_xprv = XPrv::new(mnemonic.generate_seed(None))?; + let account = DerivationIndex::new(*account_id, true)?; + let derived = root_xprv.derive_path(&key.derivation_path().chain_push(account))?; + if *public { + key.derive_public_with_xprv(writer, derived)?; + } else { + key.derive_with_xprv(writer, derived)?; + } + } + } + Ok(()) +} + impl MnemonicSubcommands { pub fn handle( &self, @@ -567,11 +617,14 @@ impl MnemonicSubcommands { MnemonicSubcommands::Generate { source, size, + derive, encrypt_to, shard_to, shard, encrypt_to_self, provision, + provision_count, + provision_config, } => { // NOTE: We should never have a case where there's Some() of empty vec, but // we will make sure to check it just in case. @@ -586,6 +639,16 @@ impl MnemonicSubcommands { let mnemonic = source.handle(size)?; + if let Some(derive) = derive { + let stdout = std::io::stdout(); + if will_print_mnemonic && !stdout.is_terminal() { + eprintln!( + "Writing plaintext mnemonic and derivation output to standard output" + ); + } + do_derive(&mnemonic, derive)?; + } + if let Some(encrypt_to) = encrypt_to { for entry in encrypt_to { do_encrypt_to(&mnemonic, &entry.inner, &entry.values)?; @@ -593,46 +656,66 @@ impl MnemonicSubcommands { } if let Some(encrypt_to_self) = encrypt_to_self { - let mut values = encrypt_to_self.values.clone(); - // If we have a userid from `provision` but not one here, use that one. - if let Some(provision) = provision { - if matches!(&provision.inner, provision::Provisioner::OpenPGPCard(_)) - && !values.contains_key("userid") - { - if let Some(userid) = provision.values.get("userid") { - values.insert(String::from("userid"), userid.clone()); - } - } + let mut accounts: std::collections::HashSet = Default::default(); + if let Some(provision::Provision { + provisioner_name: provision::Provisioner::OpenPGPCard(_), + account_id, + .. + }) = provision + { + accounts.insert(*account_id); } - do_encrypt_to_self(&mnemonic, &encrypt_to_self.inner, &values)?; + if let Some(derive::Derive { + command: derive::DeriveSubcommands::OpenPGP(_), + account_id, + .. + }) = derive + { + accounts.insert(*account_id); + } + let indices = accounts + .into_iter() + .map(|i| DerivationIndex::new(i, true)) + .collect::, _>>()?; + assert!( + !indices.is_empty(), + "neither derived nor provisioned accounts were found" + ); + do_encrypt_to_self(&mnemonic, &encrypt_to_self, &indices)?; } if let Some(provisioner) = provision { - // NOTE: If we have encrypt_to_self, we likely also have the certificate - // already generated. Therefore, we can skip generating it in the provisioner. - // However, if we don't have encrypt_to_self, we might not have the - // certificate, therefore the provisioner - by default - generates the public - // key output. - // - // We use the atypical `_skip_cert_output` field here to denote an automatic - // marking to skip the cert output. However, the `output` field will take - // priority, since it can only be manually set by the user. - let mut values = provisioner.values.clone(); - if let Some(encrypt_to_self) = encrypt_to_self { - if !values.contains_key("output") { - values.insert(String::from("_skip_cert_output"), String::from("1")); - } - // If we have a userid from `encrypt_to_self` but not one here, use that - // one. - if matches!(&provisioner.inner, provision::Provisioner::OpenPGPCard(_)) - && !values.contains_key("userid") - { - if let Some(userid) = encrypt_to_self.values.get("userid") { - values.insert(String::from("userid"), userid.clone()); - } + // determine if we should write to standard output based on whether we have a + // matching pair of provisioner and public derivation output. + let mut will_output_public_key = true; + + if let Some(derive) = derive { + let matches = match (provisioner, derive) { + ( + provision::Provision { + provisioner_name: provision::Provisioner::OpenPGPCard(_), + account_id: p_id, + .. + }, + derive::Derive { + command: derive::DeriveSubcommands::OpenPGP(_), + account_id: d_id, + .. + }, + ) => p_id == d_id, + _ => false, + }; + if matches && derive.public { + will_output_public_key = false; } } - do_provision(&mnemonic, &provisioner.inner, &values)?; + + let mut values = provision_config.values.clone(); + if !will_output_public_key && !values.contains_key("output") { + values.insert(String::from("_skip_cert_output"), String::from("1")); + } + + do_provision(&mnemonic, provisioner, *provision_count, &values)?; } if let Some(shard_to) = shard_to { diff --git a/crates/keyfork/src/cli/provision/mod.rs b/crates/keyfork/src/cli/provision/mod.rs index 544b788..480faea 100644 --- a/crates/keyfork/src/cli/provision/mod.rs +++ b/crates/keyfork/src/cli/provision/mod.rs @@ -124,15 +124,27 @@ pub struct Provision { #[command(subcommand)] pub subcommand: Option, - provisioner_name: Provisioner, + pub provisioner_name: Provisioner, /// Account ID. - #[arg(long, required(true))] - account_id: Option, + #[arg(long, default_value = "0")] + pub account_id: u32, /// Identifier of the hardware to deploy to, listable by running the `discover` subcommand. - #[arg(long, required(true))] - identifier: Option, + #[arg(long)] + pub identifier: Option, +} + +impl std::str::FromStr for Provision { + type Err = clap::Error; + + fn from_str(s: &str) -> std::result::Result { + Provision::try_parse_from( + [String::from("provision")] + .into_iter() + .chain(shlex::Shlex::new(s)), + ) + } } // NOTE: In the future, this impl will be used by `keyfork recover` to reprovision hardware from @@ -148,7 +160,7 @@ impl TryFrom for config::Provisioner { fn try_from(value: Provision) -> Result { Ok(Self { - account: value.account_id.ok_or(MissingField("account_id"))?, + account: value.account_id, identifier: value.identifier.ok_or(MissingField("identifier"))?, metadata: Default::default(), }) @@ -171,7 +183,21 @@ impl Provision { } } None => { - self.provisioner_name.provision(self.clone().try_into()?)?; + let provisioner_with_identifier = match self.identifier { + Some(_) => self.clone(), + None => { + let identifiers = self.provisioner_name.discover()?; + let [id] = &identifiers[..] else { + panic!("invalid amount of identifiers; pass --identifier"); + }; + Self { + identifier: Some(id.0.clone()), + ..self.clone() + } + } + }; + let config = config::Provisioner::try_from(provisioner_with_identifier)?; + self.provisioner_name.provision(config)?; } } Ok(()) diff --git a/crates/keyfork/src/cli/provision/openpgp.rs b/crates/keyfork/src/cli/provision/openpgp.rs index a3c6d26..f639726 100644 --- a/crates/keyfork/src/cli/provision/openpgp.rs +++ b/crates/keyfork/src/cli/provision/openpgp.rs @@ -1,5 +1,8 @@ use super::ProvisionExec; -use crate::{config, openpgp_card::factory_reset_current_card}; +use crate::{ + config, + openpgp_card::{factory_reset_current_card, get_new_pins}, +}; use card_backend_pcsc::PcscBackend; use keyfork_derive_openpgp::{ @@ -11,10 +14,7 @@ use keyfork_derive_openpgp::{ }, XPrv, }; -use keyfork_prompt::{ - default_handler, prompt_validated_passphrase, - validators::{SecurePinValidator, Validator}, -}; +use keyfork_prompt::default_handler; use openpgp_card_sequoia::{state::Open, Card}; use std::path::PathBuf; @@ -52,29 +52,8 @@ impl ProvisionExec for OpenPGPCard { provisioner: config::Provisioner, ) -> Result<(), Box> { let mut pm = default_handler()?; - let user_pin_validator = SecurePinValidator { - min_length: Some(6), - ..Default::default() - } - .to_fn(); - let admin_pin_validator = SecurePinValidator { - min_length: Some(8), - ..Default::default() - } - .to_fn(); - let user_pin = prompt_validated_passphrase( - &mut *pm, - "Please enter the new smartcard User PIN: ", - 3, - &user_pin_validator, - )?; - let admin_pin = prompt_validated_passphrase( - &mut *pm, - "Please enter the new smartcard Admin PIN: ", - 3, - &admin_pin_validator, - )?; + let (user_pin, admin_pin) = get_new_pins(&mut *pm)?; let subkeys = vec![ KeyFlags::empty().set_certification(), @@ -96,11 +75,7 @@ impl ProvisionExec for OpenPGPCard { .as_ref() .is_some_and(|m| m.contains_key("_skip_cert_output")) { - let cert_output = match provisioner - .metadata - .as_ref() - .and_then(|m| m.get("output")) - { + let cert_output = match provisioner.metadata.as_ref().and_then(|m| m.get("output")) { Some(cert_output) => PathBuf::from(cert_output), None => { let path = PathBuf::from(cert.fingerprint().to_string()).with_extension("asc"); @@ -112,7 +87,7 @@ impl ProvisionExec for OpenPGPCard { } }; - let cert_output_file = std::fs::File::create_new(cert_output)?; + let cert_output_file = std::fs::File::create(cert_output)?; let mut writer = Writer::new(cert_output_file, Kind::PublicKey)?; cert.serialize(&mut writer)?; writer.finalize()?; diff --git a/crates/keyfork/src/config.rs b/crates/keyfork/src/config.rs index f670c0a..3d6a1b2 100644 --- a/crates/keyfork/src/config.rs +++ b/crates/keyfork/src/config.rs @@ -2,19 +2,19 @@ use std::collections::HashMap; use serde::{Deserialize, Serialize}; -#[derive(Serialize, Deserialize, Clone)] +#[derive(Serialize, Deserialize, Clone, Debug)] pub struct Mnemonic { pub hash: String, } -#[derive(Serialize, Deserialize, Clone)] +#[derive(Serialize, Deserialize, Clone, Debug)] pub struct Provisioner { pub account: u32, pub identifier: String, pub metadata: Option>, } -#[derive(Serialize, Deserialize, Clone)] +#[derive(Serialize, Deserialize, Clone, Debug)] pub struct Config { pub mnemonic: Mnemonic, pub provisioner: Vec, diff --git a/crates/keyfork/src/openpgp_card.rs b/crates/keyfork/src/openpgp_card.rs index f9510a3..2811985 100644 --- a/crates/keyfork/src/openpgp_card.rs +++ b/crates/keyfork/src/openpgp_card.rs @@ -1,6 +1,68 @@ use card_backend_pcsc::PcscBackend; -use openpgp_card_sequoia::{state::Open, types::KeyType, Card, types::TouchPolicy}; -use keyfork_derive_openpgp::openpgp::{Cert, policy::Policy}; +use keyfork_derive_openpgp::openpgp::{policy::Policy, Cert}; +use keyfork_prompt::{ + prompt_validated_passphrase, + validators::{SecurePinValidator, Validator}, + Message, PromptHandler, +}; +use openpgp_card_sequoia::{state::Open, types::KeyType, types::TouchPolicy, Card}; + +pub fn get_new_pins( + pm: &mut dyn PromptHandler, +) -> Result<(String, String), Box> { + let user_pin_validator = SecurePinValidator { + min_length: Some(6), + ..Default::default() + } + .to_fn(); + let admin_pin_validator = SecurePinValidator { + min_length: Some(8), + ..Default::default() + } + .to_fn(); + + let user_pin = loop { + let user_pin = prompt_validated_passphrase( + &mut *pm, + "Please enter the new smartcard User PIN: ", + 3, + &user_pin_validator, + )?; + let validated_user_pin = prompt_validated_passphrase( + &mut *pm, + "Please verify the new smartcard User PIN: ", + 3, + &user_pin_validator, + )?; + if user_pin != validated_user_pin { + pm.prompt_message(Message::Text("User PINs did not match. Retrying.".into()))?; + } else { + break user_pin; + } + }; + + let admin_pin = loop { + let admin_pin = prompt_validated_passphrase( + &mut *pm, + "Please enter the new smartcard Admin PIN: ", + 3, + &admin_pin_validator, + )?; + let validated_admin_pin = prompt_validated_passphrase( + &mut *pm, + "Please verify the new smartcard Admin PIN: ", + 3, + &admin_pin_validator, + )?; + if admin_pin != validated_admin_pin { + pm.prompt_message(Message::Text("Admin PINs did not match. Retrying.".into()))?; + } else { + break admin_pin; + } + }; + + Ok((user_pin, admin_pin)) +} /// Factory reset the current card so long as it does not match the last-used backend. /// @@ -51,4 +113,3 @@ pub fn factory_reset_current_card( transaction.change_admin_pin("12345678", admin_pin)?; Ok(true) } -