Merge branch 'main' into ramfox/flakey-test-simple-node-id

Author: ramfox

Cargo.lock

@@ -26,6 +26,8 @@ serde = { version = "1", features = ["derive", "rc"] }
 snafu = { version = "0.8.5", features = ["rust_1_81"], optional = true }
 n0-snafu = "0.2.2"
 nested_enum_utils = "0.2.0"
+zeroize = { version = "1.8.2", optional = true, features = ["derive"] }
+zeroize_derive = { version = "1.4.2", optional = true } # needed for minimal versions
 
 [dev-dependencies]
 postcard = { version = "1", features = ["use-std"] }
@@ -47,6 +49,8 @@ key = [
   "dep:snafu",
   "dep:data-encoding",
   "dep:rand_core",
+  "dep:zeroize",
+  "dep:zeroize_derive",
   "relay",
 ]
 relay = [

iroh-base/Cargo.toml

@@ -3,14 +3,13 @@
 use std::{
     borrow::Borrow,
     cmp::{Ord, PartialOrd},
-    fmt::{Debug, Display},
+    fmt::{self, Debug, Display},
     hash::Hash,
     ops::Deref,
     str::FromStr,
 };
 
 use curve25519_dalek::edwards::CompressedEdwardsY;
-pub use ed25519_dalek::{Signature, SignatureError};
 use ed25519_dalek::{SigningKey, VerifyingKey};
 use nested_enum_utils::common_fields;
 use rand_core::CryptoRng;
@@ -100,24 +99,22 @@ impl<'de> Deserialize<'de> for PublicKey {
 }
 
 impl PublicKey {
+    /// The length of an ed25519 `PublicKey`, in bytes.
+    pub const LENGTH: usize = ed25519_dalek::PUBLIC_KEY_LENGTH;
+
     /// Get this public key as a byte array.
     pub fn as_bytes(&self) -> &[u8; 32] {
         self.0.as_bytes()
     }
 
-    /// 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.
     ///
     /// # Warning
     ///
     /// This will return a [`SignatureError`] if the bytes passed into this method do not represent
     /// 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> {
+    pub fn from_bytes(bytes: &[u8; 32]) -> Result<Self, KeyParsingError> {
         let key = VerifyingKey::from_bytes(bytes)?;
         let y = CompressedEdwardsY(key.to_bytes());
         Ok(Self(y))
@@ -129,7 +126,9 @@ impl PublicKey {
     ///
     /// Returns `Ok(())` if the signature is valid, and `Err` otherwise.
     pub fn verify(&self, message: &[u8], signature: &Signature) -> Result<(), SignatureError> {
-        self.public().verify_strict(message, signature)
+        self.as_verifying_key()
+            .verify_strict(message, &signature.0)
+            .map_err(|_| SignatureSnafu.build())
     }
 
     /// Convert to a hex string limited to the first 5 bytes for a friendly string
@@ -142,8 +141,17 @@ impl PublicKey {
         )
     }
 
-    /// The length of an ed25519 `PublicKey`, in bytes.
-    pub const LENGTH: usize = ed25519_dalek::PUBLIC_KEY_LENGTH;
+    /// Needed for internal conversions, not part of the stable API.
+    #[doc(hidden)]
+    pub fn as_verifying_key(&self) -> VerifyingKey {
+        VerifyingKey::from_bytes(self.0.as_bytes()).expect("already verified")
+    }
+
+    /// Needed for internal conversions, not part of the stable API.
+    #[doc(hidden)]
+    pub fn from_verifying_key(key: VerifyingKey) -> Self {
+        Self(CompressedEdwardsY(key.to_bytes()))
+    }
 }
 
 struct PublicKeyShort([u8; 5]);
@@ -155,7 +163,7 @@ impl Display for PublicKeyShort {
 }
 
 impl TryFrom<&[u8]> for PublicKey {
-    type Error = SignatureError;
+    type Error = KeyParsingError;
 
     #[inline]
     fn try_from(bytes: &[u8]) -> Result<Self, Self::Error> {
@@ -165,7 +173,7 @@ impl TryFrom<&[u8]> for PublicKey {
 }
 
 impl TryFrom<&[u8; 32]> for PublicKey {
-    type Error = SignatureError;
+    type Error = KeyParsingError;
 
     #[inline]
     fn try_from(bytes: &[u8; 32]) -> Result<Self, Self::Error> {
@@ -179,13 +187,6 @@ impl AsRef<[u8]> for PublicKey {
     }
 }
 
-impl From<VerifyingKey> for PublicKey {
-    fn from(verifying_key: VerifyingKey) -> Self {
-        let key = verifying_key.to_bytes();
-        PublicKey(CompressedEdwardsY(key))
-    }
-}
-
 impl Debug for PublicKey {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         write!(
@@ -234,15 +235,13 @@ impl FromStr for PublicKey {
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         let bytes = decode_base32_hex(s)?;
 
-        Ok(Self::from_bytes(&bytes)?)
+        Self::from_bytes(&bytes)
     }
 }
 
 /// A secret key.
-#[derive(Clone)]
-pub struct SecretKey {
-    secret: SigningKey,
-}
+#[derive(Clone, zeroize::ZeroizeOnDrop)]
+pub struct SecretKey(SigningKey);
 
 impl Debug for SecretKey {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
@@ -264,7 +263,7 @@ impl Serialize for SecretKey {
     where
         S: serde::Serializer,
     {
-        self.secret.serialize(serializer)
+        self.0.serialize(serializer)
     }
 }
 
@@ -274,14 +273,15 @@ impl<'de> Deserialize<'de> for SecretKey {
         D: serde::Deserializer<'de>,
     {
         let secret = SigningKey::deserialize(deserializer)?;
-        Ok(secret.into())
+        Ok(Self(secret))
     }
 }
 
 impl SecretKey {
     /// The public key of this [`SecretKey`].
     pub fn public(&self) -> PublicKey {
-        self.secret.verifying_key().into()
+        let key = self.0.verifying_key().to_bytes();
+        PublicKey(CompressedEdwardsY(key))
     }
 
     /// Generate a new [`SecretKey`] with a randomness generator.
@@ -292,38 +292,33 @@ impl SecretKey {
     /// ```
     pub fn generate<R: CryptoRng + ?Sized>(csprng: &mut R) -> Self {
         let secret = SigningKey::generate(csprng);
-
-        Self { secret }
+        Self(secret)
     }
 
     /// Sign the given message and return a digital signature
     pub fn sign(&self, msg: &[u8]) -> Signature {
         use ed25519_dalek::Signer;
 
-        self.secret.sign(msg)
+        let sig = self.0.sign(msg);
+        Signature(sig)
     }
 
     /// Convert this to the bytes representing the secret part.
     /// The public part can always be recovered.
     pub fn to_bytes(&self) -> [u8; 32] {
-        self.secret.to_bytes()
+        self.0.to_bytes()
     }
 
     /// Create a secret key from its byte representation.
     pub fn from_bytes(bytes: &[u8; 32]) -> Self {
         let secret = SigningKey::from_bytes(bytes);
-        secret.into()
+        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 }
+    /// Needed for internal conversions, not part of the stable API.
+    #[doc(hidden)]
+    pub fn as_signing_key(&self) -> &SigningKey {
+        &self.0
     }
 }
 
@@ -334,15 +329,51 @@ impl From<[u8; 32]> for SecretKey {
 }
 
 impl TryFrom<&[u8]> for SecretKey {
-    type Error = SignatureError;
+    type Error = KeyParsingError;
 
     #[inline]
     fn try_from(bytes: &[u8]) -> Result<Self, Self::Error> {
         let secret = SigningKey::try_from(bytes)?;
-        Ok(secret.into())
+        Ok(Self(secret))
+    }
+}
+
+/// Ed25519 signature.
+#[derive(Copy, Clone, Eq, PartialEq)]
+pub struct Signature(ed25519_dalek::Signature);
+
+impl Debug for Signature {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{:?}", self.0)
+    }
+}
+
+impl Display for Signature {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{}", self.0)
     }
 }
 
+impl Signature {
+    /// The length of an ed25519 `Signature`, in bytes.
+    pub const LENGTH: usize = ed25519_dalek::Signature::BYTE_SIZE;
+
+    /// Return the inner byte array.
+    pub fn to_bytes(&self) -> [u8; Self::LENGTH] {
+        self.0.to_bytes()
+    }
+
+    /// Parse an Ed25519 signature from a byte slice.
+    pub fn from_bytes(bytes: &[u8; Self::LENGTH]) -> Self {
+        Self(ed25519_dalek::Signature::from_bytes(bytes))
+    }
+}
+
+/// Verification of a signature failed.
+#[derive(Debug, Snafu)]
+#[snafu(display("Invalid signature"))]
+pub struct SignatureError;
+
 fn decode_base32_hex(s: &str) -> Result<[u8; 32], KeyParsingError> {
     let mut bytes = [0u8; 32];
 

iroh-base/src/key.rs

@@ -40,7 +40,7 @@ use std::{
     str::{FromStr, Utf8Error},
 };
 
-use iroh_base::{NodeAddr, NodeId, RelayUrl, SecretKey, SignatureError};
+use iroh_base::{KeyParsingError, NodeAddr, NodeId, RelayUrl, SecretKey};
 use nested_enum_utils::common_fields;
 use snafu::{Backtrace, ResultExt, Snafu};
 use url::Url;
@@ -81,7 +81,10 @@ pub enum DecodingError {
     #[snafu(display("length must be 32 bytes, but got {len} byte(s)"))]
     InvalidLength { len: usize },
     #[snafu(display("node id is not a valid public key"))]
-    InvalidSignature { source: SignatureError },
+    InvalidKey {
+        #[snafu(source(from(KeyParsingError, Box::new)))]
+        source: Box<KeyParsingError>,
+    },
 }
 
 /// Extension methods for [`NodeId`] to encode to and decode from [`z32`],
@@ -108,7 +111,7 @@ impl NodeIdExt for NodeId {
         let bytes: &[u8; 32] = &bytes
             .try_into()
             .map_err(|_| InvalidLengthSnafu { len: s.len() }.build())?;
-        let node_id = NodeId::from_bytes(bytes).context(InvalidSignatureSnafu)?;
+        let node_id = NodeId::from_bytes(bytes).context(InvalidKeySnafu)?;
         Ok(node_id)
     }
 }
@@ -561,7 +564,7 @@ impl<T: FromStr + Display + Hash + Ord> TxtAttrs<T> {
         };
         let pubkey = packet.public_key();
         let pubkey_z32 = pubkey.to_z32();
-        let node_id = NodeId::from(*pubkey.verifying_key());
+        let node_id = NodeId::from_bytes(&pubkey.verifying_key().to_bytes()).expect("valid key");
         let zone = dns::Name::new(&pubkey_z32).expect("z32 encoding is valid");
         let txt_data = packet
             .all_resource_records()

iroh-relay/src/node_info.rs

@@ -10,7 +10,7 @@
 use std::num::NonZeroU16;
 
 use bytes::{Buf, BufMut, Bytes, BytesMut};
-use iroh_base::{NodeId, SignatureError};
+use iroh_base::{KeyParsingError, NodeId};
 use n0_future::time::Duration;
 use nested_enum_utils::common_fields;
 use snafu::{Backtrace, ResultExt, Snafu};
@@ -59,7 +59,7 @@ pub enum Error {
     #[snafu(transparent)]
     FrameTypeError { source: FrameTypeError },
     #[snafu(display("Invalid public key"))]
-    InvalidPublicKey { source: SignatureError },
+    InvalidPublicKey { source: KeyParsingError },
     #[snafu(display("Invalid frame encoding"))]
     InvalidFrame {},
     #[snafu(display("Invalid frame type: {frame_type:?}"))]
@@ -216,7 +216,7 @@ impl Datagrams {
         + self.contents.len()
     }
 
-    #[allow(clippy::len_zero)]
+    #[allow(clippy::len_zero, clippy::result_large_err)]
     fn from_bytes(mut bytes: Bytes, is_batch: bool) -> Result<Self, Error> {
         if is_batch {
             // 1 bytes ECN, 2 bytes segment size

iroh-relay/src/protos/relay.rs

@@ -521,8 +521,8 @@ mod tests {
             node_key: sender_key.public(),
         });
 
-        let sender_secret = secret_ed_box(sender_key.secret());
-        let shared = SharedSecret::new(&sender_secret, &public_ed_box(&recv_key.public().public()));
+        let sender_secret = secret_ed_box(&sender_key);
+        let shared = SharedSecret::new(&sender_secret, &public_ed_box(&recv_key.public()));
         let mut seal = msg.as_bytes();
         shared.seal(&mut seal);
 
@@ -535,9 +535,8 @@ mod tests {
         assert_eq!(raw_key, sender_key.public());
         assert_eq!(seal_back, seal);
 
-        let recv_secret = secret_ed_box(recv_key.secret());
-        let shared_recv =
-            SharedSecret::new(&recv_secret, &public_ed_box(&sender_key.public().public()));
+        let recv_secret = secret_ed_box(&recv_key);
+        let shared_recv = SharedSecret::new(&recv_secret, &public_ed_box(&sender_key.public()));
         let mut open_seal = seal_back.to_vec();
         shared_recv
             .open(&mut open_seal)