Skip to content

Commit

Permalink
refactor(iroh-base)!: reduce dependencies (#3046)
Browse files Browse the repository at this point in the history
## Description

- ensure `zeroize` is used for `SecretKey`
- remove unused dependencies and features from `iroh-base`

## Breaking Changes

- `iroh_base::SecretKey::generate` now takes an rng
- removed `iroh_base::SecretKey::generate_with_rng`, use `generate`
directly
- removed `iroh_base::SecretKey::to_openssh`
- removed `iroh_base::SecretKey::from_openssh`
- `anyhow::Error` is replaced with explicit errors for
`RelayUrl::from_str`
- `anyhow::Error` is replaced with explicit errors for
`SharedSecret::open`

## Notes & open questions

As a follow up, all usage of `SecretKey::generate` in tests should be
changed to use a seeded rng, like chacha8, instead of `thread_rng`

## Change checklist

- [ ] Self-review.
- [ ] Documentation updates following the [style
guide](https://rust-lang.github.io/rfcs/1574-more-api-documentation-conventions.html#appendix-a-full-conventions-text),
if relevant.
- [ ] Tests if relevant.
- [ ] All breaking changes documented.
  • Loading branch information
dignifiedquire authored Dec 16, 2024
1 parent 8ec0d73 commit 4a774f1
Show file tree
Hide file tree
Showing 39 changed files with 186 additions and 441 deletions.
259 changes: 2 additions & 257 deletions Cargo.lock

Large diffs are not rendered by default.

30 changes: 10 additions & 20 deletions iroh-base/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,48 +15,39 @@ rust-version = "1.81"
workspace = true

[dependencies]
anyhow = { version = "1", optional = true }
aead = { version = "0.5.2", features = ["bytes"], optional = true }
crypto_box = { version = "0.9.1", features = ["serde", "chacha20"], optional = true }
data-encoding = { version = "2.3.3", optional = true }
ed25519-dalek = { version = "2.0.0", features = ["serde", "rand_core", "zeroize"], optional = true }
derive_more = { version = "1.0.0", features = ["display"], optional = true }
url = { version = "2.5", features = ["serde"], optional = true }
postcard = { version = "1", default-features = false, features = ["alloc", "use-std", "experimental-derive"], optional = true }
rand_core = { version = "0.6.4", optional = true }
serde = { version = "1", features = ["derive"] }
thiserror = { version = "2", optional = true }

# key module
aead = { version = "0.5.2", features = ["bytes"], optional = true }
derive_more = { version = "1.0.0", features = ["debug", "display", "from_str"], optional = true }
ed25519-dalek = { version = "2.0.0", features = ["serde", "rand_core"], optional = true }
once_cell = { version = "1.18.0", optional = true }
rand = { version = "0.8", optional = true }
rand_core = { version = "0.6.4", optional = true }
ssh-key = { version = "0.6.0", features = ["ed25519", "std", "rand_core"], optional = true }
ttl_cache = { version = "0.5.1", optional = true }
crypto_box = { version = "0.9.1", features = ["serde", "chacha20"], optional = true }
zeroize = { version = "1.5", optional = true }
url = { version = "2.5", features = ["serde"], optional = true }

# wasm
getrandom = { version = "0.2", default-features = false, optional = true }

[dev-dependencies]
iroh-test = "0.28.0"
postcard = { version = "1", features = ["use-std"] }
proptest = "1.0.0"
rand = "0.8"
serde_json = "1"
serde_test = "1"
postcard = { version = "1", features = ["use-std"] }


[features]
default = ["ticket", "relay"]
ticket = ["key", "dep:postcard", "dep:data-encoding"]
key = [
"dep:anyhow",
"dep:ed25519-dalek",
"dep:once_cell",
"dep:rand",
"dep:rand_core",
"dep:ssh-key",
"dep:ttl_cache",
"dep:aead",
"dep:crypto_box",
"dep:zeroize",
"dep:url",
"dep:derive_more",
"dep:getrandom",
Expand All @@ -68,7 +59,6 @@ wasm = ["getrandom?/js"]
relay = [
"dep:url",
"dep:derive_more",
"dep:anyhow",
"dep:thiserror",
]

Expand Down
63 changes: 17 additions & 46 deletions iroh-base/src/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,18 @@ use std::{
fmt::{Debug, Display},
hash::Hash,
str::FromStr,
sync::Mutex,
sync::{Mutex, OnceLock},
time::Duration,
};

pub use ed25519_dalek::Signature;
use ed25519_dalek::{SignatureError, SigningKey, VerifyingKey};
use once_cell::sync::OnceCell;
use rand_core::CryptoRngCore;
use serde::{Deserialize, Serialize};
use ssh_key::LineEnding;
use ttl_cache::TtlCache;

pub use self::encryption::SharedSecret;
use self::encryption::{public_ed_box, secret_ed_box};
pub use self::encryption::{DecryptionError, SharedSecret};

#[derive(Debug)]
struct CryptoKeys {
Expand Down Expand Up @@ -48,7 +46,7 @@ const KEY_CACHE_TTL: Duration = Duration::from_secs(60);
///
/// So that is about 4MB of max memory for the cache.
const KEY_CACHE_CAPACITY: usize = 1024 * 16;
static KEY_CACHE: OnceCell<Mutex<TtlCache<[u8; 32], CryptoKeys>>> = OnceCell::new();
static KEY_CACHE: OnceLock<Mutex<TtlCache<[u8; 32], CryptoKeys>>> = OnceLock::new();

fn lock_key_cache() -> std::sync::MutexGuard<'static, TtlCache<[u8; 32], CryptoKeys>> {
let mutex = KEY_CACHE.get_or_init(|| Mutex::new(TtlCache::new(KEY_CACHE_CAPACITY)));
Expand Down Expand Up @@ -181,6 +179,7 @@ impl PublicKey {
data_encoding::HEXLOWER.encode(&self.as_bytes()[..5])
}

/// The length of an ed25519 `PublicKey`, in bytes.
pub const LENGTH: usize = ed25519_dalek::PUBLIC_KEY_LENGTH;
}

Expand Down Expand Up @@ -259,6 +258,7 @@ pub enum KeyParsingError {
/// Error when decoding the public key.
#[error("key: {0}")]
Key(#[from] ed25519_dalek::SignatureError),
/// The encoded information had the wrong length.
#[error("invalid length")]
DecodeInvalidLength,
}
Expand All @@ -280,7 +280,7 @@ impl FromStr for PublicKey {
#[derive(Clone)]
pub struct SecretKey {
secret: SigningKey,
secret_crypto_box: OnceCell<crypto_box::SecretKey>,
secret_crypto_box: OnceLock<crypto_box::SecretKey>,
}

impl Debug for SecretKey {
Expand Down Expand Up @@ -334,40 +334,19 @@ impl SecretKey {
self.secret.verifying_key().into()
}

/// Generate a new [`SecretKey`] with the default randomness generator.
pub fn generate() -> Self {
let mut rng = rand::rngs::OsRng;
Self::generate_with_rng(&mut rng)
}

/// Generate a new [`SecretKey`] with a randomness generator.
pub fn generate_with_rng<R: CryptoRngCore + ?Sized>(csprng: &mut R) -> Self {
let secret = SigningKey::generate(csprng);
///
/// ```rust
/// // use the OsRng option for OS depedndent most secure RNG.
/// let mut rng = rand::rngs::OsRng;
/// let _key = iroh_base::SecretKey::generate(&mut rng);
/// ```
pub fn generate<R: CryptoRngCore>(mut csprng: R) -> Self {
let secret = SigningKey::generate(&mut csprng);

Self {
secret,
secret_crypto_box: OnceCell::default(),
}
}

/// Serialise this key to OpenSSH format.
pub fn to_openssh(&self) -> ssh_key::Result<zeroize::Zeroizing<String>> {
let ckey = ssh_key::private::Ed25519Keypair {
public: self.secret.verifying_key().into(),
private: self.secret.clone().into(),
};
ssh_key::private::PrivateKey::from(ckey).to_openssh(LineEnding::default())
}

/// Deserialise this key from OpenSSH format.
pub fn try_from_openssh<T: AsRef<[u8]>>(data: T) -> anyhow::Result<Self> {
let ser_key = ssh_key::private::PrivateKey::from_openssh(data)?;
match ser_key.key_data() {
ssh_key::private::KeypairData::Ed25519(kp) => Ok(SecretKey {
secret: kp.private.clone().into(),
secret_crypto_box: OnceCell::default(),
}),
_ => anyhow::bail!("invalid key format"),
secret_crypto_box: Default::default(),
}
}

Expand Down Expand Up @@ -400,7 +379,7 @@ impl From<SigningKey> for SecretKey {
fn from(secret: SigningKey) -> Self {
SecretKey {
secret,
secret_crypto_box: OnceCell::default(),
secret_crypto_box: Default::default(),
}
}
}
Expand Down Expand Up @@ -459,14 +438,6 @@ mod tests {
assert_eq_hex!(bytes, expected);
}

#[test]
fn test_secret_key_openssh_roundtrip() {
let kp = SecretKey::generate();
let ser = kp.to_openssh().unwrap();
let de = SecretKey::try_from_openssh(&ser).unwrap();
assert_eq!(kp.to_bytes(), de.to_bytes());
}

#[test]
fn public_key_postcard() {
let key = PublicKey::from_bytes(&[0; 32]).unwrap();
Expand All @@ -485,7 +456,7 @@ mod tests {

#[test]
fn test_display_from_str() {
let key = SecretKey::generate();
let key = SecretKey::generate(&mut rand::thread_rng());
assert_eq!(
SecretKey::from_str(&key.to_string()).unwrap().to_bytes(),
key.to_bytes()
Expand Down
28 changes: 20 additions & 8 deletions iroh-base/src/key/encryption.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
use std::fmt::Debug;

use aead::Buffer;
use anyhow::{anyhow, ensure, Context, Result};

pub(crate) const NONCE_LEN: usize = 24;

Expand All @@ -18,6 +17,17 @@ pub(super) fn secret_ed_box(key: &ed25519_dalek::SigningKey) -> crypto_box::Secr
/// Shared Secret.
pub struct SharedSecret(crypto_box::ChaChaBox);

/// Errors that can occur during [`SharedSecret::open`].
#[derive(Debug, thiserror::Error)]
pub enum DecryptionError {
/// The nonce had the wrong size.
#[error("Invalid nonce")]
InvalidNonce,
/// AEAD decryption failed.
#[error("Aead error")]
Aead(aead::Error),
}

impl Debug for SharedSecret {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "SharedSecret(crypto_box::ChaChaBox)")
Expand All @@ -42,19 +52,21 @@ impl SharedSecret {
}

/// Opens the ciphertext, which must have been created using `Self::seal`, and places the clear text into the provided buffer.
pub fn open(&self, buffer: &mut dyn Buffer) -> Result<()> {
pub fn open(&self, buffer: &mut dyn Buffer) -> Result<(), DecryptionError> {
use aead::AeadInPlace;
ensure!(buffer.len() > NONCE_LEN, "too short");
if buffer.len() < NONCE_LEN {
return Err(DecryptionError::InvalidNonce);
}

let offset = buffer.len() - NONCE_LEN;
let nonce: [u8; NONCE_LEN] = buffer.as_ref()[offset..]
.try_into()
.context("nonce wrong length")?;
.map_err(|_| DecryptionError::InvalidNonce)?;

buffer.truncate(offset);
self.0
.decrypt_in_place(&nonce.into(), &[], buffer)
.map_err(|e| anyhow!("decryption failed: {:?}", e))?;
.map_err(DecryptionError::Aead)?;

Ok(())
}
Expand All @@ -76,8 +88,8 @@ mod tests {

#[test]
fn test_seal_open_roundtrip() {
let key_a = crate::key::SecretKey::generate();
let key_b = crate::key::SecretKey::generate();
let key_a = crate::key::SecretKey::generate(&mut rand::thread_rng());
let key_b = crate::key::SecretKey::generate(&mut rand::thread_rng());

seal_open_roundtrip(&key_a, &key_b);
seal_open_roundtrip(&key_b, &key_a);
Expand Down Expand Up @@ -105,7 +117,7 @@ mod tests {

#[test]
fn test_same_public_key_api() {
let key = crate::key::SecretKey::generate();
let key = crate::key::SecretKey::generate(&mut rand::thread_rng());
let public_key1: crypto_box::PublicKey = public_ed_box(&key.public().public());
let public_key2: crypto_box::PublicKey = secret_ed_box(&key.secret).public_key();

Expand Down
8 changes: 6 additions & 2 deletions iroh-base/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! Base types and utilities for Iroh
#![cfg_attr(iroh_docsrs, feature(doc_auto_cfg))]
#![deny(missing_docs, rustdoc::broken_intra_doc_links)]
#![cfg_attr(not(test), deny(clippy::unwrap_used))]

// TODO: move to own crate
#[cfg(feature = "ticket")]
Expand All @@ -13,8 +15,10 @@ mod node_addr;
mod relay_url;

#[cfg(feature = "key")]
pub use self::key::{KeyParsingError, NodeId, PublicKey, SecretKey, SharedSecret, Signature};
pub use self::key::{
DecryptionError, KeyParsingError, NodeId, PublicKey, SecretKey, SharedSecret, Signature,
};
#[cfg(feature = "key")]
pub use self::node_addr::NodeAddr;
#[cfg(feature = "relay")]
pub use self::relay_url::RelayUrl;
pub use self::relay_url::{RelayUrl, RelayUrlParseError};
10 changes: 7 additions & 3 deletions iroh-base/src/relay_url.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::{fmt, ops::Deref, str::FromStr};

use anyhow::Context;
use serde::{Deserialize, Serialize};
use url::Url;
/// A URL identifying a relay server.
Expand Down Expand Up @@ -36,15 +35,20 @@ impl From<Url> for RelayUrl {
}
}

/// Can occur when parsing a string into a [`RelayUrl`].
#[derive(Debug, thiserror::Error)]
#[error("Failed to parse: {0}")]
pub struct RelayUrlParseError(#[from] url::ParseError);

/// Support for parsing strings directly.
///
/// If you need more control over the error first create a [`Url`] and use [`RelayUrl::from`]
/// instead.
impl FromStr for RelayUrl {
type Err = anyhow::Error;
type Err = RelayUrlParseError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
let inner = Url::from_str(s).context("invalid URL")?;
let inner = Url::from_str(s)?;
Ok(RelayUrl::from(inner))
}
}
Expand Down
14 changes: 9 additions & 5 deletions iroh-base/src/ticket.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
//! Tickets is a serializable object combining information required for an operation.
//! Typically tickets contain all information required for an operation, e.g. an iroh blob
//! ticket would contain the hash of the data as well as information about how to reach the
//! provider.
use std::{collections::BTreeSet, net::SocketAddr};

use serde::{Deserialize, Serialize};
Expand All @@ -10,10 +15,6 @@ pub use self::node::NodeTicket;

/// A ticket is a serializable object combining information required for an operation.
///
/// Typically tickets contain all information required for an operation, e.g. an iroh blob
/// ticket would contain the hash of the data as well as information about how to reach the
/// provider.
///
/// Tickets support serialization to a string using base32 encoding. The kind of
/// ticket will be prepended to the string to make it somewhat self describing.
///
Expand Down Expand Up @@ -60,7 +61,10 @@ pub trait Ticket: Sized {
pub enum Error {
/// Found a ticket of with the wrong prefix, indicating the wrong kind.
#[error("wrong prefix, expected {expected}")]
Kind { expected: &'static str },
Kind {
/// The expected prefix.
expected: &'static str,
},
/// This looks like a ticket, but postcard deserialization failed.
#[error("deserialization failed: {_0}")]
Postcard(#[from] postcard::Error),
Expand Down
2 changes: 1 addition & 1 deletion iroh-base/src/ticket/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ mod tests {
use crate::key::{PublicKey, SecretKey};

fn make_ticket() -> NodeTicket {
let peer = SecretKey::generate().public();
let peer = SecretKey::generate(&mut rand::thread_rng()).public();
let addr = SocketAddr::from((Ipv4Addr::LOCALHOST, 1234));
let relay_url = None;
NodeTicket {
Expand Down
2 changes: 2 additions & 0 deletions iroh-dns-server/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ hickory-resolver = "=0.25.0-alpha.4"
iroh = { version = "0.29.0", path = "../iroh" }
iroh-test = { version = "0.29.0", path = "../iroh-test" }
pkarr = { version = "2.2.0", features = ["rand"] }
rand = "0.8"
rand_chacha = "0.3.1"
testresult = "0.4.1"

[[bench]]
Expand Down
4 changes: 3 additions & 1 deletion iroh-dns-server/benches/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use anyhow::Result;
use criterion::{criterion_group, criterion_main, BenchmarkId, Criterion, Throughput};
use iroh::{discovery::pkarr::PkarrRelayClient, dns::node_info::NodeInfo, SecretKey};
use iroh_dns_server::{config::Config, server::Server, ZoneStore};
use rand_chacha::rand_core::SeedableRng;
use tokio::runtime::Runtime;

const LOCALHOST_PKARR: &str = "http://localhost:8080/pkarr";
Expand All @@ -23,7 +24,8 @@ fn benchmark_dns_server(c: &mut Criterion) {
let config = Config::load("./config.dev.toml").await.unwrap();
let server = start_dns_server(config).await.unwrap();

let secret_key = SecretKey::generate();
let mut rng = rand_chacha::ChaCha8Rng::seed_from_u64(42);
let secret_key = SecretKey::generate(&mut rng);
let node_id = secret_key.public();

let pkarr_relay = LOCALHOST_PKARR.parse().expect("valid url");
Expand Down
Loading

0 comments on commit 4a774f1

Please sign in to comment.