From ada6cf150b96fec0c5bb1e0bdcd07f7f0a11e1f1 Mon Sep 17 00:00:00 2001 From: ryan Date: Sun, 5 Nov 2023 22:51:40 -0600 Subject: [PATCH] keyforkd: split into enum based request --- Cargo.lock | 19 ++++-- Cargo.toml | 1 + keyfork-derive-key/src/main.rs | 6 +- keyfork-derive-openpgp/src/main.rs | 6 +- keyforkd-client/Cargo.toml | 1 + keyforkd-client/src/lib.rs | 4 +- keyforkd-client/src/tests.rs | 6 +- keyforkd-models/Cargo.toml | 11 ++++ keyforkd-models/src/lib.rs | 84 ++++++++++++++++++++++++++ keyforkd/Cargo.toml | 1 + keyforkd/src/service.rs | 95 +++++++++++++++++++----------- 11 files changed, 188 insertions(+), 46 deletions(-) create mode 100644 keyforkd-models/Cargo.toml create mode 100644 keyforkd-models/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 57691de..19d4286 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1103,6 +1103,7 @@ dependencies = [ "keyfork-frame", "keyfork-mnemonic-util", "keyfork-slip10-test-data", + "keyforkd-models", "serde", "thiserror", "tokio", @@ -1121,11 +1122,21 @@ dependencies = [ "keyfork-frame", "keyfork-slip10-test-data", "keyforkd", + "keyforkd-models", "tempdir", "thiserror", "tokio", ] +[[package]] +name = "keyforkd-models" +version = "0.1.0" +dependencies = [ + "keyfork-derive-util", + "serde", + "thiserror", +] + [[package]] name = "lalrpop" version = "0.20.0" @@ -1944,18 +1955,18 @@ dependencies = [ [[package]] name = "serde" -version = "1.0.188" +version = "1.0.190" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cf9e0fcba69a370eed61bcf2b728575f726b50b55cba78064753d708ddc7549e" +checksum = "91d3c334ca1ee894a2c6f6ad698fe8c435b76d504b13d436f0685d648d6d96f7" dependencies = [ "serde_derive", ] [[package]] name = "serde_derive" -version = "1.0.188" +version = "1.0.190" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4eca7ac642d82aa35b60049a6eccb4be6be75e599bd2e9adb5f875a737654af2" +checksum = "67c5609f394e5c2bd7fc51efda478004ea80ef42fee983d5c67a65e34f32c0e3" dependencies = [ "proc-macro2", "quote", diff --git a/Cargo.toml b/Cargo.toml index c431567..49bfed1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,5 +15,6 @@ members = [ "keyfork-slip10-test-data", "keyforkd", "keyforkd-client", + "keyforkd-models", "smex", ] diff --git a/keyfork-derive-key/src/main.rs b/keyfork-derive-key/src/main.rs index d6456b5..b6429c7 100644 --- a/keyfork-derive-key/src/main.rs +++ b/keyfork-derive-key/src/main.rs @@ -1,7 +1,7 @@ use std::{env, process::ExitCode, str::FromStr}; use keyfork_derive_util::{ - request::{DerivationAlgorithm, DerivationError, DerivationRequest}, + request::{DerivationAlgorithm, DerivationError, DerivationRequest, DerivationResponse}, DerivationPath, }; use keyforkd_client::Client; @@ -38,8 +38,8 @@ fn run() -> Result<(), Box> { let mut client = Client::discover_socket()?; let request = DerivationRequest::new(algo, &path); - let response = client.request(&request)?; - println!("{}", smex::encode(&response.data)); + let response = client.request(&request.into())?; + println!("{}", smex::encode(&DerivationResponse::try_from(response)?.data)); Ok(()) } diff --git a/keyfork-derive-openpgp/src/main.rs b/keyfork-derive-openpgp/src/main.rs index 6c514bf..2171486 100644 --- a/keyfork-derive-openpgp/src/main.rs +++ b/keyfork-derive-openpgp/src/main.rs @@ -1,7 +1,7 @@ use std::{env, process::ExitCode, str::FromStr}; use keyfork_derive_util::{ - request::{DerivationAlgorithm, DerivationRequest}, + request::{DerivationAlgorithm, DerivationRequest, DerivationResponse}, DerivationIndex, DerivationPath, }; use keyforkd_client::Client; @@ -107,7 +107,9 @@ fn run() -> Result<(), Box> { }; let request = DerivationRequest::new(DerivationAlgorithm::Ed25519, &path); - let derived_data = Client::discover_socket()?.request(&request)?; + let derived_data: DerivationResponse = Client::discover_socket()? + .request(&request.into())? + .try_into()?; let subkeys = subkey_format .iter() .map(|kt| kt.inner().clone()) diff --git a/keyforkd-client/Cargo.toml b/keyforkd-client/Cargo.toml index 35da19f..1418b86 100644 --- a/keyforkd-client/Cargo.toml +++ b/keyforkd-client/Cargo.toml @@ -14,6 +14,7 @@ secp256k1 = ["keyfork-derive-util/secp256k1"] bincode = "1.3.3" keyfork-derive-util = { version = "0.1.0", path = "../keyfork-derive-util", default-features = false } keyfork-frame = { version = "0.1.0", path = "../keyfork-frame" } +keyforkd-models = { version = "0.1.0", path = "../keyforkd-models" } thiserror = "1.0.49" [dev-dependencies] diff --git a/keyforkd-client/src/lib.rs b/keyforkd-client/src/lib.rs index 7dcae33..3ac56a1 100644 --- a/keyforkd-client/src/lib.rs +++ b/keyforkd-client/src/lib.rs @@ -1,7 +1,7 @@ use std::{collections::HashMap, os::unix::net::UnixStream, path::PathBuf}; -use keyfork_derive_util::request::{DerivationRequest, DerivationResponse}; use keyfork_frame::{try_decode_from, try_encode_to, DecodeError, EncodeError}; +use keyforkd_models::{Request, Response /* Error as KeyforkdError */}; #[cfg(test)] mod tests; @@ -65,7 +65,7 @@ impl Client { get_socket().map(|socket| Self { socket }) } - pub fn request(&mut self, req: &DerivationRequest) -> Result { + pub fn request(&mut self, req: &Request) -> Result { try_encode_to(&bincode::serialize(&req)?, &mut self.socket)?; let resp = try_decode_from(&mut self.socket)?; bincode::deserialize(&resp).map_err(From::from) diff --git a/keyforkd-client/src/tests.rs b/keyforkd-client/src/tests.rs index 154558f..6c3a760 100644 --- a/keyforkd-client/src/tests.rs +++ b/keyforkd-client/src/tests.rs @@ -46,7 +46,8 @@ fn secp256k1() { DerivationAlgorithm::Secp256k1, &DerivationPath::from_str(test.chain).unwrap(), ); - let response = client.request(&req).unwrap(); + let response = + DerivationResponse::try_from(client.request(&req.into()).unwrap()).unwrap(); assert_eq!(response.data, test.private_key); } @@ -90,7 +91,8 @@ fn ed25519() { DerivationAlgorithm::Ed25519, &DerivationPath::from_str(test.chain).unwrap(), ); - let response = client.request(&req).unwrap(); + let response = + DerivationResponse::try_from(client.request(&req.into()).unwrap()).unwrap(); assert_eq!(response.data, test.private_key); } diff --git a/keyforkd-models/Cargo.toml b/keyforkd-models/Cargo.toml new file mode 100644 index 0000000..485e6b2 --- /dev/null +++ b/keyforkd-models/Cargo.toml @@ -0,0 +1,11 @@ +[package] +name = "keyforkd-models" +version = "0.1.0" +edition = "2021" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +keyfork-derive-util = { version = "0.1.0", path = "../keyfork-derive-util", default-features = false } +serde = { version = "1.0.190", features = ["derive"] } +thiserror = "1.0.50" diff --git a/keyforkd-models/src/lib.rs b/keyforkd-models/src/lib.rs new file mode 100644 index 0000000..ee5e10e --- /dev/null +++ b/keyforkd-models/src/lib.rs @@ -0,0 +1,84 @@ +//! All values that can be sent to and returned from Keyforkd. +//! +//! All Error values are stored as enum unit values. + +use keyfork_derive_util::request::{DerivationRequest, DerivationResponse}; + +use serde::{Deserialize, Serialize}; + +/// A request made to Keyforkd. +#[derive(Serialize, Deserialize, Clone, Debug)] +pub enum Request { + /// A derivation request. + Derivation(DerivationRequest), + + /// A derivation request, with a TTY provided for pinentry if necessary. + DerivationWithTTY(DerivationRequest, String), +} + +impl From for Request { + fn from(value: DerivationRequest) -> Self { + Self::Derivation(value) + } +} + +impl From<(DerivationRequest, String)> for Request { + fn from((request, tty): (DerivationRequest, String)) -> Self { + Self::DerivationWithTTY(request, tty) + } +} + +#[derive(thiserror::Error, Clone, Debug, Serialize, Deserialize)] +pub enum DerivationError { + #[error("The provided TTY was not valid")] + InvalidTTY, + + #[error("A TTY was required by the pinentry program but was not provided")] + NoTTY, + + #[error("Invalid derivation length: Expected 2, actual: {0}")] + InvalidDerivationLength(usize), + + #[error("Derivation error: {0}")] + Derivation(String), +} + +#[derive(thiserror::Error, Clone, Debug, Serialize, Deserialize)] +pub enum Error { + #[error(transparent)] + Derivation(#[from] DerivationError), +} + +/// Any response from a Keyforkd request. +/// +/// Responses can be converted to their inner values without matching the Response type by +/// using a type annotation and [`TryInto::try_into`]. +#[derive(Serialize, Deserialize, Clone, Debug)] +#[non_exhaustive] +pub enum Response { + /// A derivation response. + /// + /// From: + /// * [`Request::Derivation`] + /// * [`Request::DerivationWithTTY`] + Derivation(DerivationResponse), +} + +#[derive(thiserror::Error, Debug)] +#[error("Unable to downcast to {0}")] +pub struct Downcast(&'static str); + +use std::any::type_name; + +impl TryFrom for DerivationResponse { + type Error = Downcast; + + fn try_from(value: Response) -> Result { + #[allow(irrefutable_let_patterns)] + if let Response::Derivation(response) = value { + Ok(response) + } else { + Err(Downcast(type_name::())) + } + } +} diff --git a/keyforkd/Cargo.toml b/keyforkd/Cargo.toml index 005414d..35fe3f8 100644 --- a/keyforkd/Cargo.toml +++ b/keyforkd/Cargo.toml @@ -29,6 +29,7 @@ tower = { version = "0.4.13", features = ["tokio", "util"] } thiserror = "1.0.47" serde = { version = "1.0.186", features = ["derive"] } keyfork-derive-path-data = { version = "0.1.0", path = "../keyfork-derive-path-data" } +keyforkd-models = { version = "0.1.0", path = "../keyforkd-models" } [dev-dependencies] hex-literal = "0.4.1" diff --git a/keyforkd/src/service.rs b/keyforkd/src/service.rs index 491d1b5..7ac7343 100644 --- a/keyforkd/src/service.rs +++ b/keyforkd/src/service.rs @@ -1,22 +1,14 @@ use std::{future::Future, pin::Pin, sync::Arc, task::Poll}; use keyfork_derive_path_data::guess_target; -use keyfork_derive_util::request::{DerivationError, DerivationRequest, DerivationResponse}; +// use keyfork_derive_util::request::{DerivationError, DerivationRequest, DerivationResponse}; +use keyforkd_models::{DerivationError, Error, Request, Response}; use tower::Service; use tracing::info; // NOTE: All values implemented in Keyforkd must implement Clone with low overhead, either by // using an Arc or by having a small signature. This is because Service takes &mut self. - -#[derive(thiserror::Error, Debug)] -pub enum KeyforkdRequestError { - #[error("Invalid derivation length: Expected: 2, actual: {0}")] - InvalidDerivationLength(usize), - - #[error("Derivation error: {0}")] - Derivation(#[from] DerivationError), -} - +// #[derive(Clone, Debug)] pub struct Keyforkd { seed: Arc>, @@ -30,10 +22,11 @@ impl Keyforkd { } } -impl Service for Keyforkd { - type Response = DerivationResponse; +impl Service for Keyforkd { + type Response = Response; - type Error = KeyforkdRequestError; + // TODO: indicate serialize in BincodeLayer + type Error = Error; type Future = Pin> + Send>>; @@ -45,24 +38,28 @@ impl Service for Keyforkd { } #[cfg_attr(feature = "tracing", tracing::instrument(skip(self)))] - fn call(&mut self, req: DerivationRequest) -> Self::Future { + fn call(&mut self, req: Request) -> Self::Future { let seed = self.seed.clone(); - Box::pin(async move { - let len = req.path().len(); - if len < 2 { - return Err(KeyforkdRequestError::InvalidDerivationLength(len)); - } + match req { + Request::Derivation(req) => Box::pin(async move { + let len = req.path().len(); + if len < 2 { + return Err(DerivationError::InvalidDerivationLength(len).into()); + } - #[cfg(feature = "tracing")] - if let Some(target) = guess_target(req.path()) { - info!("Deriving path: {target}"); - } else { - info!("Deriving path: {}", req.path()); - } + #[cfg(feature = "tracing")] + if let Some(target) = guess_target(req.path()) { + info!("Deriving path: {target}"); + } else { + info!("Deriving path: {}", req.path()); + } - req.derive_with_master_seed((*seed).clone()) - .map_err(KeyforkdRequestError::from) - }) + req.derive_with_master_seed((*seed).clone()) + .map(Response::Derivation) + .map_err(|e| DerivationError::Derivation(e.to_string()).into()) + }), + Request::DerivationWithTTY(_, _) => todo!(), + } } } @@ -92,7 +89,15 @@ mod tests { continue; } let req = DerivationRequest::new(DerivationAlgorithm::Secp256k1, &chain); - let response = keyforkd.ready().await.unwrap().call(req).await.unwrap(); + let response: DerivationResponse = keyforkd + .ready() + .await + .unwrap() + .call(Request::Derivation(req)) + .await + .unwrap() + .try_into() + .unwrap(); assert_eq!(response.data, test.private_key); assert_eq!(response.chain_code.as_slice(), test.chain_code); } @@ -114,7 +119,15 @@ mod tests { continue; } let req = DerivationRequest::new(DerivationAlgorithm::Ed25519, &chain); - let response = keyforkd.ready().await.unwrap().call(req).await.unwrap(); + let response: DerivationResponse = keyforkd + .ready() + .await + .unwrap() + .call(Request::Derivation(req)) + .await + .unwrap() + .try_into() + .unwrap(); assert_eq!(response.data, test.private_key); assert_eq!(response.chain_code.as_slice(), test.chain_code); } @@ -134,7 +147,15 @@ mod tests { for (seed, path, _, private_key, _) in tests { let req = DerivationRequest::new(DerivationAlgorithm::Ed25519, &path); let mut keyforkd = Keyforkd::new(seed.to_vec()); - let response = keyforkd.ready().await.unwrap().call(req).await.unwrap(); + let response: DerivationResponse = keyforkd + .ready() + .await + .unwrap() + .call(Request::Derivation(req)) + .await + .unwrap() + .try_into() + .unwrap(); assert_eq!(response.data, private_key) } } @@ -152,7 +173,15 @@ mod tests { for (seed, path, _, private_key, _) in tests { let req = DerivationRequest::new(DerivationAlgorithm::Ed25519, &path); let mut keyforkd = Keyforkd::new(seed.to_vec()); - let response = keyforkd.ready().await.unwrap().call(req).await.unwrap(); + let response: DerivationResponse = keyforkd + .ready() + .await + .unwrap() + .call(Request::Derivation(req)) + .await + .unwrap() + .try_into() + .unwrap(); assert_eq!(response.data, private_key) } }