Skip to content

Commit

Permalink
refactor(iroh-base): remove automatic key caching (#3051)
Browse files Browse the repository at this point in the history 
## Description

- remove automatic caching of public key material
- move `crypto_box` based shared secret into `iroh` behind a private API

## Breaking Changes

- remove `iroh_base::SharedSecret`
- remove `iroh_base::DecryptionError`, `iroh::DecryptionError`
- remove `iroh_base::SecretKey::shared`

## Notes

I removed the benchmark for this, because it would require keeping this
API public.
This needs a follow up PR adding an explicit key cache for the relay
protocol, which needs to check keys on a per packet basis.
  • Loading branch information
@dignifiedquire
dignifiedquire authored Dec 16, 2024
1 parent a6f502c commit 58df1d8
Show file tree
Hide file tree
Showing 10 changed files with 99 additions and 300 deletions.
9 changes: 5 additions & 4 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

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

[dependencies]
aead = { version = "0.5.2", features = ["bytes"], optional = true }
crypto_box = { version = "0.9.1", features = ["serde", "chacha20"], optional = true }
curve25519-dalek = { version = "4.1.3", features = ["serde", "rand_core", "zeroize"], 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 }
Expand All @@ -25,7 +24,6 @@ postcard = { version = "1", default-features = false, features = ["alloc", "use-
rand_core = { version = "0.6.4", optional = true }
serde = { version = "1", features = ["derive"] }
thiserror = { version = "2", optional = true }
ttl_cache = { version = "0.5.1", optional = true }

# wasm
getrandom = { version = "0.2", default-features = false, optional = true }
Expand All @@ -43,16 +41,14 @@ serde_test = "1"
default = ["ticket", "relay"]
ticket = ["key", "dep:postcard", "dep:data-encoding"]
key = [
"dep:curve25519-dalek",
"dep:ed25519-dalek",
"dep:rand_core",
"dep:ttl_cache",
"dep:aead",
"dep:crypto_box",
"dep:url",
"dep:derive_more",
"dep:getrandom",
"dep:thiserror",
"dep:data-encoding",
"dep:rand_core",
"relay",
]
wasm = ["getrandom?/js"]
Expand Down
184 changes: 34 additions & 150 deletions iroh-base/src/key.rs
View file
Original file line number Diff line number Diff line change
@@ -1,94 +1,38 @@
//! Cryptographic key handling for `iroh`.
mod encryption;

use std::{
cmp::{Ord, PartialOrd},
fmt::{Debug, Display},
hash::Hash,
str::FromStr,
sync::{Mutex, OnceLock},
time::Duration,
};

use curve25519_dalek::edwards::CompressedEdwardsY;
pub use ed25519_dalek::Signature;
use ed25519_dalek::{SignatureError, SigningKey, VerifyingKey};
use rand_core::CryptoRngCore;
use serde::{Deserialize, Serialize};
use ttl_cache::TtlCache;

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

#[derive(Debug)]
struct CryptoKeys {
verifying_key: VerifyingKey,
crypto_box: crypto_box::PublicKey,
}

impl CryptoKeys {
fn new(verifying_key: VerifyingKey) -> Self {
let crypto_box = public_ed_box(&verifying_key);
Self {
verifying_key,
crypto_box,
}
}
}

/// Expiry time for the crypto key cache.
///
/// Basically, if no crypto operations have been performed with a key for this
/// duration, the crypto keys will be removed from the cache and need to be
/// re-created when they are used again.
const KEY_CACHE_TTL: Duration = Duration::from_secs(60);
/// Maximum number of keys in the crypto key cache. CryptoKeys are 224 bytes,
/// keys are 32 bytes, so each entry is 256 bytes plus some overhead.
/// A public key.
///
/// So that is about 4MB of max memory for the cache.
const KEY_CACHE_CAPACITY: usize = 1024 * 16;
static KEY_CACHE: OnceLock<Mutex<TtlCache<[u8; 32], CryptoKeys>>> = OnceLock::new();
/// The key itself is stored as the `CompressedEdwards` y coordinate of the public key
/// It is verified to decompress into a valid key when created.
#[derive(Clone, Copy, PartialEq, Eq)]
#[repr(transparent)]
pub struct PublicKey(CompressedEdwardsY);

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)));
mutex.lock().expect("not poisoned")
impl PartialOrd for PublicKey {
fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
Some(self.cmp(other))
}
}

/// Get or create the crypto keys, and project something out of them.
///
/// If the key has been verified before, this will not fail.
fn get_or_create_crypto_keys<T>(
key: &[u8; 32],
f: impl Fn(&CryptoKeys) -> T,
) -> std::result::Result<T, SignatureError> {
let mut state = lock_key_cache();
Ok(match state.entry(*key) {
ttl_cache::Entry::Occupied(entry) => {
// cache hit
f(entry.get())
}
ttl_cache::Entry::Vacant(entry) => {
// cache miss, create. This might fail if the key is invalid.
let vk = VerifyingKey::from_bytes(key)?;
let item = CryptoKeys::new(vk);
let item = entry.insert(item, KEY_CACHE_TTL);
f(item)
}
})
impl Ord for PublicKey {
fn cmp(&self, other: &Self) -> std::cmp::Ordering {
self.0.as_bytes().cmp(other.0.as_bytes())
}
}

/// A public key.
///
/// The key itself is just a 32 byte array, but a key has associated crypto
/// information that is cached for performance reasons.
///
/// The cache item will be refreshed every time a crypto operation is performed,
/// or when a key is deserialised or created from a byte array.
///
/// Serialisation or creation from a byte array is cheap if the key is already
/// in the cache, but expensive if it is not.
#[derive(Clone, Copy, PartialEq, Eq, Ord, PartialOrd)]
pub struct PublicKey([u8; 32]);

/// The identifier for a node in the (iroh) network.
///
/// Each node in iroh has a unique identifier created as a cryptographic key. This can be
Expand Down Expand Up @@ -117,7 +61,7 @@ impl Serialize for PublicKey {
if serializer.is_human_readable() {
serializer.serialize_str(&self.to_string())
} else {
self.0.serialize(serializer)
self.0.as_bytes().serialize(serializer)
}
}
}
Expand All @@ -140,16 +84,12 @@ impl<'de> Deserialize<'de> for PublicKey {
impl PublicKey {
/// Get this public key as a byte array.
pub fn as_bytes(&self) -> &[u8; 32] {
&self.0
}

fn public(&self) -> VerifyingKey {
get_or_create_crypto_keys(&self.0, |item| item.verifying_key).expect("key has been checked")
self.0.as_bytes()
}

fn public_crypto_box(&self) -> crypto_box::PublicKey {
get_or_create_crypto_keys(&self.0, |item| item.crypto_box.clone())
.expect("key has been checked")
/// Returns the [`VerifyingKey`] for this `PublicKey`.
pub fn public(&self) -> VerifyingKey {
VerifyingKey::from_bytes(self.0.as_bytes()).expect("already verified")
}

/// Construct a `PublicKey` from a slice of bytes.
Expand All @@ -160,8 +100,9 @@ impl PublicKey {
/// a valid `ed25519_dalek` curve point. Will never fail for bytes return from [`Self::as_bytes`].
/// See [`VerifyingKey::from_bytes`] for details.
pub fn from_bytes(bytes: &[u8; 32]) -> Result<Self, SignatureError> {
get_or_create_crypto_keys(bytes, |item| item.verifying_key)?;
Ok(Self(*bytes))
let key = VerifyingKey::from_bytes(bytes)?;
let y = CompressedEdwardsY(key.to_bytes());
Ok(Self(y))
}

/// Verify a signature on a message with this secret key's public key.
Expand All @@ -188,21 +129,8 @@ impl TryFrom<&[u8]> for PublicKey {

#[inline]
fn try_from(bytes: &[u8]) -> Result<Self, Self::Error> {
Ok(match <[u8; 32]>::try_from(bytes) {
Ok(bytes) => {
// using from_bytes is faster than going via the verifying
// key in case the key is already in the cache, which should
// be quite common.
Self::from_bytes(&bytes)?
}
Err(_) => {
// this will always fail since the size is wrong.
// but there is no public constructor for SignatureError,
// so ¯\_(ツ)_/¯...
let vk = VerifyingKey::try_from(bytes)?;
vk.into()
}
})
let vk = VerifyingKey::try_from(bytes)?;
Ok(Self(CompressedEdwardsY(vk.to_bytes())))
}
}

Expand All @@ -223,13 +151,8 @@ impl AsRef<[u8]> for PublicKey {

impl From<VerifyingKey> for PublicKey {
fn from(verifying_key: VerifyingKey) -> Self {
let item = CryptoKeys::new(verifying_key);
let key = *verifying_key.as_bytes();
let mut table = lock_key_cache();
// we already have performed the crypto operation, so no need for
// get_or_create_crypto_keys. Just insert in any case.
table.insert(key, item, KEY_CACHE_TTL);
PublicKey(key)
let key = verifying_key.to_bytes();
PublicKey(CompressedEdwardsY(key))
}
}

Expand Down Expand Up @@ -272,15 +195,14 @@ impl FromStr for PublicKey {
fn from_str(s: &str) -> Result<Self, Self::Err> {
let bytes = decode_base32_hex(s)?;

Ok(Self::try_from(&bytes)?)
Ok(Self::from_bytes(&bytes)?)
}
}

/// A secret key.
#[derive(Clone)]
pub struct SecretKey {
secret: SigningKey,
secret_crypto_box: OnceLock<crypto_box::SecretKey>,
}

impl Debug for SecretKey {
Expand Down Expand Up @@ -344,10 +266,7 @@ impl SecretKey {
pub fn generate<R: CryptoRngCore>(mut csprng: R) -> Self {
let secret = SigningKey::generate(&mut csprng);

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

/// Sign the given message and return a digital signature
Expand All @@ -369,18 +288,15 @@ impl SecretKey {
secret.into()
}

fn secret_crypto_box(&self) -> &crypto_box::SecretKey {
self.secret_crypto_box
.get_or_init(|| secret_ed_box(&self.secret))
/// Returns the [`SigningKey`] for this `SecretKey`.
pub fn secret(&self) -> &SigningKey {
&self.secret
}
}

impl From<SigningKey> for SecretKey {
fn from(secret: SigningKey) -> Self {
SecretKey {
secret,
secret_crypto_box: Default::default(),
}
SecretKey { secret }
}
}

Expand Down Expand Up @@ -467,36 +383,4 @@ mod tests {
key.public()
);
}

/// Test the different ways a key can come into existence, and that they
/// all populate the key cache.
#[test]
fn test_key_creation_cache() {
let random_verifying_key = || {
let sk = SigningKey::generate(&mut rand::thread_rng());
sk.verifying_key()
};
let random_public_key = || random_verifying_key().to_bytes();
let k1 = random_public_key();
let _key = PublicKey::from_bytes(&k1).unwrap();
assert!(lock_key_cache().contains_key(&k1));

let k2 = random_public_key();
let _key = PublicKey::try_from(&k2).unwrap();
assert!(lock_key_cache().contains_key(&k2));

let k3 = random_public_key();
let _key = PublicKey::try_from(k3.as_slice()).unwrap();
assert!(lock_key_cache().contains_key(&k3));

let k4 = random_verifying_key();
let _key = PublicKey::from(k4);
assert!(lock_key_cache().contains_key(k4.as_bytes()));

let k5 = random_verifying_key();
let bytes = postcard::to_stdvec(&k5).unwrap();
// VerifyingKey serialises with a length prefix, PublicKey does not.
let _key: PublicKey = postcard::from_bytes(&bytes[1..]).unwrap();
assert!(lock_key_cache().contains_key(k5.as_bytes()));
}
}
4 changes: 1 addition & 3 deletions iroh-base/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@ mod node_addr;
mod relay_url;

#[cfg(feature = "key")]
pub use self::key::{
DecryptionError, KeyParsingError, NodeId, PublicKey, SecretKey, SharedSecret, Signature,
};
pub use self::key::{KeyParsingError, NodeId, PublicKey, SecretKey, Signature};
#[cfg(feature = "key")]
pub use self::node_addr::NodeAddr;
#[cfg(feature = "relay")]
Expand Down
Loading

0 comments on commit 58df1d8

Please sign in to comment.