From 947661b89a8fc6bc717bc2b4895ed930b3c5a1dd Mon Sep 17 00:00:00 2001 From: xphoniex Date: Fri, 7 Oct 2022 16:41:02 +0000 Subject: [PATCH] make ssh operations sync Signed-off-by: xphoniex --- cli/linkd-lib/Cargo.toml | 4 -- cli/lnk-clib/Cargo.toml | 14 +--- cli/lnk-clib/src/keys/ssh/unix.rs | 55 +++++++-------- cli/lnk-clib/src/lib.rs | 1 - cli/lnk-clib/src/runtime.rs | 112 ------------------------------ cli/lnk-clib/src/seed.rs | 24 ++++--- cli/lnk-exe/Cargo.toml | 4 -- cli/lnk-identities/Cargo.toml | 4 -- cli/lnk-profile/Cargo.toml | 2 +- cli/lnk-profile/src/cli/main.rs | 14 +--- cli/lnk-profile/src/lib.rs | 7 +- cli/lnk-sync/src/cli/main.rs | 5 +- librad/src/git/refs.rs | 6 +- librad/src/net/x509.rs | 19 +---- link-crypto/Cargo.toml | 1 - link-crypto/src/keys.rs | 10 ++- link-crypto/src/signer.rs | 14 ++-- link-identities/Cargo.toml | 1 - link-identities/src/git.rs | 3 +- 19 files changed, 64 insertions(+), 236 deletions(-) delete mode 100644 cli/lnk-clib/src/runtime.rs diff --git a/cli/linkd-lib/Cargo.toml b/cli/linkd-lib/Cargo.toml index f0c5a0f89..aaece8a3a 100644 --- a/cli/linkd-lib/Cargo.toml +++ b/cli/linkd-lib/Cargo.toml @@ -48,10 +48,6 @@ features = ["std", "derive"] path = "../lnk-clib" version = "0.1.0" -[dependencies.lnk-thrussh-agent] -version = "0.1.0" -features = [ "tokio-agent" ] - [dependencies.radicle-macros] path = "../../macros" diff --git a/cli/lnk-clib/Cargo.toml b/cli/lnk-clib/Cargo.toml index 94b63402c..e29ceb5dc 100644 --- a/cli/lnk-clib/Cargo.toml +++ b/cli/lnk-clib/Cargo.toml @@ -13,29 +13,19 @@ test = false unsafe = [] [dependencies] -async-trait = "0.1" -futures = "0.3" itertools = "0.10.0" nix = "0.23.1" -once_cell = "1.10" serde = "1.0" serde_json = "1.0" socket2 = "0.4.4" thiserror = "1.0" tracing = "0.1" +agent = { version = "0.1.0", git = "https://github.com/radicle-dev/radicle-ssh" } +dns-lookup = "1.0.8" [dependencies.librad] path = "../../librad" -[dependencies.lnk-thrussh-agent] -version = "0.1.0" -features = [ "tokio-agent" ] - [dependencies.minicbor] version = "0.13" features = ["std"] - -[dependencies.tokio] -version = "1.17" -default-features = false -features = [ "fs", "io-std", "macros", "process", "rt-multi-thread", "signal" ] diff --git a/cli/lnk-clib/src/keys/ssh/unix.rs b/cli/lnk-clib/src/keys/ssh/unix.rs index 62b020609..2f3c22343 100644 --- a/cli/lnk-clib/src/keys/ssh/unix.rs +++ b/cli/lnk-clib/src/keys/ssh/unix.rs @@ -3,10 +3,11 @@ // This file is part of radicle-link, distributed under the GPLv3 with Radicle // Linking Exception. For full terms see the included LICENSE file. -use std::{fmt, sync::Arc}; +use std::fmt; +use std::os::unix::net::UnixStream; +use std::sync::Arc; -use async_trait::async_trait; -use lnk_thrussh_agent::{client::tokio::UnixStream, Constraint}; +use agent::Constraint; use serde::{de::DeserializeOwned, Serialize}; use librad::{ @@ -19,9 +20,7 @@ use librad::{ }, Keystore as _, }, - BoxedSignError, - BoxedSigner, - Signer as _, + BoxedSignError, BoxedSigner, Signer as _, }, git::storage::ReadOnly, keystore::sign::Signer, @@ -29,7 +28,7 @@ use librad::{ Signature, }; -use crate::{keys, runtime}; +use crate::keys; use super::{with_socket, SshAuthSock}; @@ -38,7 +37,6 @@ pub struct SshSigner { signer: Arc + Send + Sync>, } -#[async_trait] impl Signer for SshSigner { type Error = BoxedSignError; @@ -46,10 +44,9 @@ impl Signer for SshSigner { self.signer.public_key() } - async fn sign(&self, data: &[u8]) -> Result { + fn sign(&self, data: &[u8]) -> Result { self.signer .sign(data) - .await .map_err(BoxedSignError::from_std_error) } } @@ -57,8 +54,7 @@ impl Signer for SshSigner { impl librad::Signer for SshSigner { fn sign_blocking(&self, data: &[u8]) -> Result::Error> { let data = data.to_vec(); - let signer = self.clone(); - runtime::block_on(async move { signer.sign(&data).await }) + self.sign(&data) } } @@ -70,18 +66,16 @@ pub fn signer(profile: &Profile, sock: SshAuthSock) -> Result(&agent).await?; - if keys.contains(&pk) { - let signer = agent.connect::().await?; - let signer = SshSigner { - signer: Arc::new(signer), - }; - Ok(BoxedSigner::new(signer)) - } else { - Err(super::Error::NoSuchKey(peer_id)) - } - }) + let keys = ssh::list_keys::(&agent)?; + if keys.contains(&pk) { + let signer = agent.connect::()?; + let signer = SshSigner { + signer: Arc::new(signer), + }; + Ok(BoxedSigner::new(signer)) + } else { + Err(super::Error::NoSuchKey(peer_id)) + } } /// Add the signing key associated with this `profile` to the `ssh-agent`. @@ -106,9 +100,7 @@ where .get_key() .map_err(|err| super::Error::GetKey(err.into()))?; let agent = with_socket(SshAgent::new(key.public_key.into()), sock); - runtime::block_on(async move { - ssh::add_key::(&agent, key.secret_key.into(), &constraints).await - })?; + ssh::add_key::(&agent, key.secret_key.into(), &constraints)?; Ok(()) } @@ -129,9 +121,10 @@ where .get_key() .map_err(|err| super::Error::GetKey(err.into()))?; let agent = with_socket(SshAgent::new(key.public_key.into()), sock); - Ok(runtime::block_on(async move { - ssh::remove_key::(&agent, &key.public_key.into()).await - })?) + Ok(ssh::remove_key::( + &agent, + &key.public_key.into(), + )?) } /// Test whether the signing key associated with this `profile` is present on @@ -144,7 +137,7 @@ pub fn is_signer_present(profile: &Profile, sock: SshAuthSock) -> Result(&agent).await })?; + let keys = ssh::list_keys::(&agent)?; Ok(keys.contains(&pk)) } diff --git a/cli/lnk-clib/src/lib.rs b/cli/lnk-clib/src/lib.rs index fcaa7f645..05da8ebe3 100644 --- a/cli/lnk-clib/src/lib.rs +++ b/cli/lnk-clib/src/lib.rs @@ -4,7 +4,6 @@ // Linking Exception. For full terms see the included LICENSE file. pub mod keys; -pub mod runtime; pub mod seed; pub mod ser; #[cfg(unix)] diff --git a/cli/lnk-clib/src/runtime.rs b/cli/lnk-clib/src/runtime.rs deleted file mode 100644 index 2af4c58fb..000000000 --- a/cli/lnk-clib/src/runtime.rs +++ /dev/null @@ -1,112 +0,0 @@ -// Copyright © 2021 The Radicle Link Contributors -// -// This file is part of radicle-link, distributed under the GPLv3 with Radicle -// Linking Exception. For full terms see the included LICENSE file. - -//! The purpose of this module is to provide a way to block on futures which -//! does not block the current tokio runtime in -//! `link_crypto::Signer::sign_blocking`. In order to do this we start a -//! static runtime and send tasks to it via a `std::sync::mpsc::Channel`. Tasks -//! are any future which is `Send + 'static`. -use std::{pin::Pin, sync::Arc}; - -use once_cell::sync::Lazy; - -static RUNTIME: Lazy = Lazy::new(Runtime::new); - -/// Submit a task to the static runtime and wait for its output. The task is run -/// within the context of a separate multi threaded tokio runtime, which means -/// that spawned sub-tasks will execute correctly. If the thread waiting for -/// this task panics then the running task will be cancelled at the next await -/// point. -pub(crate) fn block_on(future: F) -> F::Output -where - F: futures::Future + Send + 'static, - F::Output: Send + 'static, -{ - let job = RUNTIME.spawn(future); - job.wait().unwrap() -} - -pub(crate) struct Runtime { - requests: tokio::sync::mpsc::UnboundedSender, -} - -impl Runtime { - pub fn new() -> Self { - let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel::(); - - std::thread::spawn(|| { - let runtime = tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() - .expect("unable to create tokio runtime"); - let rt_handle = runtime.handle(); - runtime.block_on(async move { - // Dropping the tokio runtime only waits for tasks to yield not to complete - // - // We therefore use a RwLock to wait for tasks to complete - let join = Arc::new(tokio::sync::RwLock::new(())); - - while let Some(task) = rx.recv().await { - let join = Arc::clone(&join); - let handle = join.read_owned().await; - - rt_handle.spawn(async move { - task.run().await; - std::mem::drop(handle); - }); - } - join.write().await; - }); - }); - - Runtime { requests: tx } - } - - pub fn spawn(&self, task: T) -> Job - where - T: futures::Future + Send + 'static, - T::Output: Send + 'static, - { - let (tx, rx) = std::sync::mpsc::channel::(); - - let fut = Box::pin(async move { - let task_output = task.await; - if tx.send(task_output).is_err() { - tracing::warn!("spawned task output ignored, receiver dropped"); - } - }); - - let task = Task { fut }; - self.requests.send(task).ok(); - - Job { rx } - } -} - -struct Task { - fut: Pin + Send>>, -} - -impl Task { - async fn run(self) { - self.fut.await - } -} - -#[derive(Debug, thiserror::Error)] -pub(crate) enum Error { - #[error(transparent)] - Recv(#[from] std::sync::mpsc::RecvError), -} - -pub(crate) struct Job { - rx: std::sync::mpsc::Receiver, -} - -impl Job { - pub(crate) fn wait(self) -> Result { - self.rx.recv().map_err(Error::from) - } -} diff --git a/cli/lnk-clib/src/seed.rs b/cli/lnk-clib/src/seed.rs index 188ab5dfe..0a7789f9b 100644 --- a/cli/lnk-clib/src/seed.rs +++ b/cli/lnk-clib/src/seed.rs @@ -3,12 +3,13 @@ // This file is part of radicle-link, distributed under the GPLv3 with Radicle // Linking Exception. For full terms see the included LICENSE file. -use std::{convert::TryFrom, fmt, io, net::SocketAddr, str::FromStr}; +use std::net::{SocketAddr, ToSocketAddrs}; +use std::{convert::TryFrom, fmt, io, str::FromStr}; +use dns_lookup::lookup_host; use serde::Serialize; use librad::{net::discovery, PeerId}; -use tokio::net::{lookup_host, ToSocketAddrs}; pub mod store; pub use store::Store; @@ -72,17 +73,20 @@ where } impl Seed { - /// Resolve the `Seed`'s address by calling [`tokio::net::lookup_host`]. + /// Resolve the `Seed`'s address by calling [`dns_lookup::lookup_host`]. /// /// # Errors /// /// If the addresses returned by `lookup_host` are empty, this will result /// in an [`error::Resolve::DnsLookupFailed`]. - pub async fn resolve(&self) -> Result>, error::Resolve> + pub fn resolve(&self) -> Result>, error::Resolve> where T: Clone + ToSocketAddrs + fmt::Display, { - let addrs = lookup_host(self.addrs.clone()).await?.collect::>(); + let addrs = lookup_host(&self.addrs.clone().to_string())? + .into_iter() + .map(|e| SocketAddr::new(e, 0)) + .collect::>(); if !addrs.is_empty() { Ok(Seed { peer: self.peer, @@ -118,7 +122,7 @@ impl Seeds { /// /// If any seeds failed to be resolved they will be returned alongside the /// successful seeds. - pub async fn load( + pub fn load( store: &S, cutoff: impl Into>, ) -> Result<(Seeds, Vec), S::Scan> @@ -135,7 +139,7 @@ impl Seeds { for seed in store.scan()? { match seed { Err(err) => failures.push(error::Load::MalformedSeed(Box::new(err))), - Ok(seed) => match seed.resolve().await { + Ok(seed) => match seed.resolve() { Ok(r) => { resolved.push(r); if Some(resolved.len()) == cutoff { @@ -154,14 +158,14 @@ impl Seeds { /// /// If any seeds failed to be resolved they will be returned alongside the /// successful seeds. - pub async fn resolve( - seeds: impl ExactSizeIterator>, + pub fn resolve<'a>( + seeds: impl ExactSizeIterator>, ) -> (Self, Vec) { let mut resolved = Vec::with_capacity(seeds.len()); let mut failures = Vec::new(); for seed in seeds { - match seed.resolve().await { + match seed.resolve() { Ok(r) => resolved.push(r), Err(err) => failures.push(err), } diff --git a/cli/lnk-exe/Cargo.toml b/cli/lnk-exe/Cargo.toml index 768492935..5d792b2ba 100644 --- a/cli/lnk-exe/Cargo.toml +++ b/cli/lnk-exe/Cargo.toml @@ -31,10 +31,6 @@ path = "../lnk-profile" [dependencies.lnk-sync] path = "../lnk-sync" -[dependencies.lnk-thrussh-agent] -version = "0.1.0" -default-features = false - [dependencies.tokio] version = "1.17" features = ["rt"] diff --git a/cli/lnk-identities/Cargo.toml b/cli/lnk-identities/Cargo.toml index b6a427367..788491c64 100644 --- a/cli/lnk-identities/Cargo.toml +++ b/cli/lnk-identities/Cargo.toml @@ -38,10 +38,6 @@ features = ["vendored"] [dependencies.librad] path = "../../librad" -[dependencies.lnk-thrussh-agent] -version = "0.1.0" -default-features = false - [dependencies.radicle-git-ext] path = "../../git-ext" diff --git a/cli/lnk-profile/Cargo.toml b/cli/lnk-profile/Cargo.toml index 2211dcad0..a58a7ff8e 100644 --- a/cli/lnk-profile/Cargo.toml +++ b/cli/lnk-profile/Cargo.toml @@ -12,7 +12,7 @@ test = false [dependencies] anyhow = "1" futures-lite = "1.12.0" -lnk-thrussh-agent = "0.1.0" +agent = { version = "0.1.0", git = "https://github.com/radicle-dev/radicle-ssh" } thiserror = "1" serde = "1" diff --git a/cli/lnk-profile/src/cli/main.rs b/cli/lnk-profile/src/cli/main.rs index 050a2900b..1a06c3b89 100644 --- a/cli/lnk-profile/src/cli/main.rs +++ b/cli/lnk-profile/src/cli/main.rs @@ -5,23 +5,13 @@ use std::{convert::TryInto as _, process::exit}; -use lnk_thrussh_agent::Constraint; +use agent::Constraint; use librad::crypto::keystore::sign; use lnk_clib::keys::{self, ssh::SshAuthSock}; use crate::{ - create, - get, - list, - paths, - peer_id, - set, - ssh_add, - ssh_ready, - ssh_remove, - ssh_sign, - ssh_verify, + create, get, list, paths, peer_id, set, ssh_add, ssh_ready, ssh_remove, ssh_sign, ssh_verify, }; use super::args::*; diff --git a/cli/lnk-profile/src/lib.rs b/cli/lnk-profile/src/lib.rs index c7c0d376b..a0bbb3db1 100644 --- a/cli/lnk-profile/src/lib.rs +++ b/cli/lnk-profile/src/lib.rs @@ -5,17 +5,14 @@ use std::{error, fmt}; -use lnk_thrussh_agent::Constraint; +use agent::Constraint; use serde::{de::DeserializeOwned, Serialize}; use thiserror::Error; use librad::{ crypto::{ keystore::{crypto::Crypto, file, FileStorage, Keystore as _}, - IntoSecretKeyError, - PeerId, - PublicKey, - SecretKey, + IntoSecretKeyError, PeerId, PublicKey, SecretKey, }, git::storage::{self, read, ReadOnly, Storage}, paths::Paths, diff --git a/cli/lnk-sync/src/cli/main.rs b/cli/lnk-sync/src/cli/main.rs index 3e0c6c7e4..387ebbafa 100644 --- a/cli/lnk-sync/src/cli/main.rs +++ b/cli/lnk-sync/src/cli/main.rs @@ -11,8 +11,7 @@ use librad::{ net::{ self, peer::{client, Client}, - quic, - Network, + quic, Network, }, profile::{LnkHome, Profile, ProfileId}, }; @@ -55,7 +54,7 @@ pub fn main( let seeds = { let seeds_file = profile.paths().seeds_file(); let store = seed::store::FileStore::::new(seeds_file)?; - let (seeds, errors) = Seeds::load(&store, None).await?; + let (seeds, errors) = Seeds::load(&store, None)?; for error in errors { eprintln!("failed to load seed: {}", error); diff --git a/librad/src/git/refs.rs b/librad/src/git/refs.rs index a96d7fb54..1cffb62f5 100644 --- a/librad/src/git/refs.rs +++ b/librad/src/git/refs.rs @@ -19,8 +19,7 @@ use link_canonical::{Cjson, CjsonError}; use serde::{ de, ser::{self, SerializeStruct}, - Deserialize, - Serialize, + Deserialize, Serialize, }; use std_ext::result::ResultExt as _; use thiserror::Error; @@ -406,7 +405,8 @@ impl Refs { where S: Signer, { - let signature = futures::executor::block_on(signer.sign(&self.canonical_form()?)) + let signature = signer + .sign(&self.canonical_form()?) .map_err(|err| signing::Error::Sign(Box::new(err)))?; Ok(Signed { refs: self, diff --git a/librad/src/net/x509.rs b/librad/src/net/x509.rs index 4a61de292..bdbf2bacf 100644 --- a/librad/src/net/x509.rs +++ b/librad/src/net/x509.rs @@ -5,7 +5,6 @@ use std::{convert::TryFrom as _, ops::Deref, time::Duration}; -use futures::executor::block_on; use picky_asn1::{ bit_string::BitString, date::{GeneralizedTime, UTCTime}, @@ -14,20 +13,8 @@ use picky_asn1::{ }; use picky_asn1_der::Asn1DerError; use picky_asn1_x509::{ - self as x509, - oids, - validity, - AlgorithmIdentifier, - DirectoryName, - ExtendedKeyUsage, - Extension, - Extensions, - GeneralName, - PublicKey, - SubjectPublicKeyInfo, - TbsCertificate, - Validity, - Version, + self as x509, oids, validity, AlgorithmIdentifier, DirectoryName, ExtendedKeyUsage, Extension, + Extensions, GeneralName, PublicKey, SubjectPublicKeyInfo, TbsCertificate, Validity, Version, }; use thiserror::Error; use time::{Date, OffsetDateTime}; @@ -106,7 +93,7 @@ impl Certificate { let signature_value = { let tbs_der = picky_asn1_der::to_vec(&tbs_certificate).unwrap(); - let signature = block_on(signer.sign(&tbs_der))?; + let signature = signer.sign(&tbs_der)?; BitString::with_bytes(signature.as_ref()).into() }; diff --git a/link-crypto/Cargo.toml b/link-crypto/Cargo.toml index 0e66d88e7..b29460d58 100644 --- a/link-crypto/Cargo.toml +++ b/link-crypto/Cargo.toml @@ -13,7 +13,6 @@ test = false async-trait = "0.1" dyn-clone = "1.0" ed25519-zebra = "3.0" -futures-lite = "1.12.0" multibase = "0.9" rand = "0.8" rustls = "0.19" diff --git a/link-crypto/src/keys.rs b/link-crypto/src/keys.rs index 5ab63a8e1..e40d1910f 100644 --- a/link-crypto/src/keys.rs +++ b/link-crypto/src/keys.rs @@ -5,9 +5,7 @@ use std::{ convert::{Infallible, TryFrom}, - error, - fmt, - iter, + error, fmt, iter, ops::Deref, }; @@ -132,8 +130,8 @@ impl sign::Signer for SecretKey { sign::Signer::public_key(&self) } - async fn sign(&self, data: &[u8]) -> Result { - sign::Signer::sign(&self, data).await + fn sign(&self, data: &[u8]) -> Result { + sign::Signer::sign(&self, data) } } @@ -145,7 +143,7 @@ impl<'a> sign::Signer for &'a SecretKey { sign::PublicKey(ed25519::VerificationKey::from(&self.0).into()) } - async fn sign(&self, data: &[u8]) -> Result { + fn sign(&self, data: &[u8]) -> Result { let signature = (*self).sign(data).0; Ok(sign::Signature(signature.into())) } diff --git a/link-crypto/src/signer.rs b/link-crypto/src/signer.rs index 705199c5f..b54c07de8 100644 --- a/link-crypto/src/signer.rs +++ b/link-crypto/src/signer.rs @@ -7,16 +7,16 @@ use std::error::Error; -use futures_lite::future::block_on; use keystore::sign; use crate::{keys, peer::PeerId}; /// A blanket trait over [`sign::Signer`] that can be shared safely among /// threads. +/// NOTE: might be redundant since `.sign` became sync pub trait Signer: sign::Signer + Send + Sync + dyn_clone::DynClone + 'static { fn sign_blocking(&self, data: &[u8]) -> Result::Error> { - block_on(self.sign(data)) + self.sign(data) } } @@ -107,8 +107,8 @@ impl sign::Signer for BoxedSigner { self.signer.public_key() } - async fn sign(&self, data: &[u8]) -> Result { - self.signer.sign(data).await + fn sign(&self, data: &[u8]) -> Result { + self.signer.sign(data) } } @@ -198,9 +198,7 @@ where self.signer.public_key() } - async fn sign(&self, data: &[u8]) -> Result { - sign::Signer::sign(&self.signer, data) - .await - .map_err(BoxedSignError::from_std_error) + fn sign(&self, data: &[u8]) -> Result { + sign::Signer::sign(&self.signer, data).map_err(BoxedSignError::from_std_error) } } diff --git a/link-identities/Cargo.toml b/link-identities/Cargo.toml index 295d6eaee..10c24d2e1 100644 --- a/link-identities/Cargo.toml +++ b/link-identities/Cargo.toml @@ -13,7 +13,6 @@ doctest = false test = false [dependencies] -futures-lite = "1.12.0" lazy_static = "1.4" multibase = "0.9" multihash = "0.11" diff --git a/link-identities/src/git.rs b/link-identities/src/git.rs index 765eb360a..8fa795f44 100644 --- a/link-identities/src/git.rs +++ b/link-identities/src/git.rs @@ -8,7 +8,6 @@ use std::{convert::TryFrom, fmt::Debug, marker::PhantomData}; use canonical::Cjson; use crypto::{PublicKey, Signer}; use either::*; -use futures_lite::future::block_on; use git_ext as ext; use multihash::Multihash; @@ -850,6 +849,6 @@ pub fn sign(signer: &S, rev: git_ext::Oid) -> Result where S: Signer, { - let sig = block_on(signer.sign(rev.as_bytes()))?; + let sig = signer.sign(rev.as_bytes())?; Ok(Signature::from((signer.public_key().into(), sig.into()))) }