feat: Better error reporting when features are disabled (#2972)

In case support for e.g. RSA keys is disabled at compile-time, we will now print a better error message. For example:

> Failed to dial Some(PeerId("QmcZf59bWwK5XFi76CZX8cbJ4BhTzzA3gU1ZjYZcYW3dwt")): Failed to negotiate transport protocol(s): [(/dnsaddr/bootstrap.libp2p.io/p2p/QmcZf59bWwK5XFi76CZX8cbJ4BhTzzA3gU1ZjYZcYW3dwt): : Handshake failed: Handshake failed: Invalid public key: Key decoding error: RSA keys are unsupported)]

Fixes #2971.

core/CHANGELOG.md

@@ -10,10 +10,13 @@
 
 - Update `multistream-select` to `v0.12.1`. See [PR 3090].
 
+- Improve error messages in case keys cannot be decoded because of missing feature flags. See [PR 2972].
+
 [PR 3031]: https://github.com/libp2p/rust-libp2p/pull/3031
 [PR 3058]: https://github.com/libp2p/rust-libp2p/pull/3058
 [PR 3097]: https://github.com/libp2p/rust-libp2p/pull/3097
 [PR 3090]: https://github.com/libp2p/rust-libp2p/pull/3090
+[PR 2972]: https://github.com/libp2p/rust-libp2p/pull/2972
 
 # 0.37.0
 

core/Cargo.toml

@@ -24,20 +24,21 @@ log = "0.4"
 multiaddr = { version = "0.16.0" }
 multihash = { version = "0.16", default-features = false, features = ["std", "multihash-impl", "identity", "sha2"] }
 multistream-select = { version = "0.12.1", path = "../misc/multistream-select" }
-p256 = { version = "0.11.1", default-features = false, features = ["ecdsa"], optional = true }
+p256 = { version = "0.11.1", default-features = false, features = ["ecdsa", "std"], optional = true }
 parking_lot = "0.12.0"
 pin-project = "1.0.0"
 prost = "0.11"
 once_cell = "1.16.0"
 rand = "0.8"
 rw-stream-sink = { version = "0.3.0", path = "../misc/rw-stream-sink" }
+sec1 = { version = "0.3.0", features = ["std"] } # Activate `std` feature until https://github.com/RustCrypto/traits/pull/1131 is released.
+serde = { version = "1", optional = true, features = ["derive"] }
 sha2 = "0.10.0"
 smallvec = "1.6.1"
 thiserror = "1.0"
 unsigned-varint = "0.7"
 void = "1"
 zeroize = "1"
-serde = { version = "1", optional = true, features = ["derive"] }
 
 [target.'cfg(not(target_arch = "wasm32"))'.dependencies]
 ring = { version = "0.16.9", features = ["alloc", "std"], default-features = false, optional = true}

core/src/identity.rs

@@ -155,23 +155,11 @@ impl Keypair {
                 data: data.encode().into(),
             },
             #[cfg(all(feature = "rsa", not(target_arch = "wasm32")))]
-            Self::Rsa(_) => {
+            Self::Rsa(_) => return Err(DecodingError::encoding_unsupported("RSA")),
-                return Err(DecodingError::new(
-                    "Encoding RSA key into Protobuf is unsupported",
-                ))
-            }
             #[cfg(feature = "secp256k1")]
-            Self::Secp256k1(_) => {
+            Self::Secp256k1(_) => return Err(DecodingError::encoding_unsupported("secp256k1")),
-                return Err(DecodingError::new(
-                    "Encoding Secp256k1 key into Protobuf is unsupported",
-                ))
-            }
             #[cfg(feature = "ecdsa")]
-            Self::Ecdsa(_) => {
+            Self::Ecdsa(_) => return Err(DecodingError::encoding_unsupported("ECDSA")),
-                return Err(DecodingError::new(
-                    "Encoding ECDSA key into Protobuf is unsupported",
-                ))
-            }
         };
 
         Ok(pk.encode_to_vec())
@@ -182,26 +170,19 @@ impl Keypair {
         use prost::Message;
 
         let mut private_key = keys_proto::PrivateKey::decode(bytes)
-            .map_err(|e| DecodingError::new("Protobuf").source(e))
+            .map_err(|e| DecodingError::bad_protobuf("private key bytes", e))
             .map(zeroize::Zeroizing::new)?;
 
-        let key_type = keys_proto::KeyType::from_i32(private_key.r#type).ok_or_else(|| {
+        let key_type = keys_proto::KeyType::from_i32(private_key.r#type)
-            DecodingError::new(format!("unknown key type: {}", private_key.r#type))
+            .ok_or_else(|| DecodingError::unknown_key_type(private_key.r#type))?;
-        })?;
 
         match key_type {
             keys_proto::KeyType::Ed25519 => {
                 ed25519::Keypair::decode(&mut private_key.data).map(Keypair::Ed25519)
             }
-            keys_proto::KeyType::Rsa => Err(DecodingError::new(
+            keys_proto::KeyType::Rsa => Err(DecodingError::decoding_unsupported("RSA")),
-                "Decoding RSA key from Protobuf is unsupported.",
+            keys_proto::KeyType::Secp256k1 => Err(DecodingError::decoding_unsupported("secp256k1")),
-            )),
+            keys_proto::KeyType::Ecdsa => Err(DecodingError::decoding_unsupported("ECDSA")),
-            keys_proto::KeyType::Secp256k1 => Err(DecodingError::new(
-                "Decoding Secp256k1 key from Protobuf is unsupported.",
-            )),
-            keys_proto::KeyType::Ecdsa => Err(DecodingError::new(
-                "Decoding ECDSA key from Protobuf is unsupported.",
-            )),
         }
     }
 }
@@ -268,7 +249,7 @@ impl PublicKey {
         use prost::Message;
 
         let pubkey = keys_proto::PublicKey::decode(bytes)
-            .map_err(|e| DecodingError::new("Protobuf").source(e))?;
+            .map_err(|e| DecodingError::bad_protobuf("public key bytes", e))?;
 
         pubkey.try_into()
     }
@@ -310,7 +291,7 @@ impl TryFrom<keys_proto::PublicKey> for PublicKey {
 
     fn try_from(pubkey: keys_proto::PublicKey) -> Result<Self, Self::Error> {
         let key_type = keys_proto::KeyType::from_i32(pubkey.r#type)
-            .ok_or_else(|| DecodingError::new(format!("unknown key type: {}", pubkey.r#type)))?;
+            .ok_or_else(|| DecodingError::unknown_key_type(pubkey.r#type))?;
 
         match key_type {
             keys_proto::KeyType::Ed25519 => {
@@ -323,7 +304,7 @@ impl TryFrom<keys_proto::PublicKey> for PublicKey {
             #[cfg(any(not(feature = "rsa"), target_arch = "wasm32"))]
             keys_proto::KeyType::Rsa => {
                 log::debug!("support for RSA was disabled at compile-time");
-                Err(DecodingError::new("Unsupported"))
+                Err(DecodingError::missing_feature("rsa"))
             }
             #[cfg(feature = "secp256k1")]
             keys_proto::KeyType::Secp256k1 => {
@@ -332,7 +313,7 @@ impl TryFrom<keys_proto::PublicKey> for PublicKey {
             #[cfg(not(feature = "secp256k1"))]
             keys_proto::KeyType::Secp256k1 => {
                 log::debug!("support for secp256k1 was disabled at compile-time");
-                Err(DecodingError::new("Unsupported"))
+                Err(DecodingError::missing_feature("secp256k1"))
             }
             #[cfg(feature = "ecdsa")]
             keys_proto::KeyType::Ecdsa => {
@@ -341,7 +322,7 @@ impl TryFrom<keys_proto::PublicKey> for PublicKey {
             #[cfg(not(feature = "ecdsa"))]
             keys_proto::KeyType::Ecdsa => {
                 log::debug!("support for ECDSA was disabled at compile-time");
-                Err(DecodingError::new("Unsupported"))
+                Err(DecodingError::missing_feature("ecdsa"))
             }
         }
     }

core/src/identity/ecdsa.rs

@@ -31,6 +31,7 @@ use p256::{
     },
     EncodedPoint,
 };
+use void::Void;
 
 /// An ECDSA keypair.
 #[derive(Clone)]
@@ -107,7 +108,7 @@ impl SecretKey {
     /// Decode a secret key from a byte buffer.
     pub fn from_bytes(buf: &[u8]) -> Result<Self, DecodingError> {
         SigningKey::from_bytes(buf)
-            .map_err(|err| DecodingError::new("failed to parse ecdsa p256 secret key").source(err))
+            .map_err(|err| DecodingError::failed_to_parse("ecdsa p256 secret key", err))
             .map(SecretKey)
     }
 }
@@ -134,12 +135,11 @@ impl PublicKey {
 
     /// Decode a public key from a byte buffer without compression.
     pub fn from_bytes(k: &[u8]) -> Result<PublicKey, DecodingError> {
-        let enc_pt = EncodedPoint::from_bytes(k).map_err(|_| {
+        let enc_pt = EncodedPoint::from_bytes(k)
-            DecodingError::new("failed to parse ecdsa p256 public key, bad point encoding")
+            .map_err(|e| DecodingError::failed_to_parse("ecdsa p256 encoded point", e))?;
-        })?;
 
         VerifyingKey::from_encoded_point(&enc_pt)
-            .map_err(|err| DecodingError::new("failed to parse ecdsa p256 public key").source(err))
+            .map_err(|err| DecodingError::failed_to_parse("ecdsa p256 public key", err))
             .map(PublicKey)
     }
 
@@ -157,7 +157,7 @@ impl PublicKey {
     /// Decode a public key into a DER encoded byte buffer as defined by SEC1 standard.
     pub fn decode_der(k: &[u8]) -> Result<PublicKey, DecodingError> {
         let buf = Self::del_asn1_header(k).ok_or_else(|| {
-            DecodingError::new("failed to parse asn.1 encoded ecdsa p256 public key")
+            DecodingError::failed_to_parse::<Void, _>("ASN.1-encoded ecdsa p256 public key", None)
         })?;
         Self::from_bytes(buf)
     }

core/src/identity/ed25519.rs

@@ -55,7 +55,7 @@ impl Keypair {
                 kp.zeroize();
                 Keypair(k)
             })
-            .map_err(|e| DecodingError::new("Ed25519 keypair").source(e))
+            .map_err(|e| DecodingError::failed_to_parse("Ed25519 keypair", e))
     }
 
     /// Sign a message using the private key of this keypair.
@@ -169,7 +169,7 @@ impl PublicKey {
     /// Decode a public key from a byte array as produced by `encode`.
     pub fn decode(k: &[u8]) -> Result<PublicKey, DecodingError> {
         ed25519::PublicKey::from_bytes(k)
-            .map_err(|e| DecodingError::new("Ed25519 public key").source(e))
+            .map_err(|e| DecodingError::failed_to_parse("Ed25519 public key", e))
             .map(PublicKey)
     }
 }
@@ -215,7 +215,7 @@ impl SecretKey {
     pub fn from_bytes(mut sk_bytes: impl AsMut<[u8]>) -> Result<SecretKey, DecodingError> {
         let sk_bytes = sk_bytes.as_mut();
         let secret = ed25519::SecretKey::from_bytes(&*sk_bytes)
-            .map_err(|e| DecodingError::new("Ed25519 secret key").source(e))?;
+            .map_err(|e| DecodingError::failed_to_parse("Ed25519 secret key", e))?;
         sk_bytes.zeroize();
         Ok(SecretKey(secret))
     }

core/src/identity/error.rs

@@ -31,17 +31,61 @@ pub struct DecodingError {
 }
 
 impl DecodingError {
-    pub(crate) fn new<S: ToString>(msg: S) -> Self {
+    #[cfg(not(all(
+        feature = "ecdsa",
+        feature = "rsa",
+        feature = "secp256k1",
+        not(target_arch = "wasm32")
+    )))]
+    pub(crate) fn missing_feature(feature_name: &'static str) -> Self {
         Self {
-            msg: msg.to_string(),
+            msg: format!("cargo feature `{feature_name}` is not enabled"),
             source: None,
         }
     }
 
-    pub(crate) fn source(self, source: impl Error + Send + Sync + 'static) -> Self {
+    pub(crate) fn failed_to_parse<E, S>(what: &'static str, source: S) -> Self
+    where
+        E: Error + Send + Sync + 'static,
+        S: Into<Option<E>>,
+    {
         Self {
+            msg: format!("failed to parse {what}"),
+            source: match source.into() {
+                None => None,
+                Some(e) => Some(Box::new(e)),
+            },
+        }
+    }
+
+    pub(crate) fn bad_protobuf(
+        what: &'static str,
+        source: impl Error + Send + Sync + 'static,
+    ) -> Self {
+        Self {
+            msg: format!("failed to decode {what} from protobuf"),
             source: Some(Box::new(source)),
-            ..self
+        }
+    }
+
+    pub(crate) fn unknown_key_type(key_type: i32) -> Self {
+        Self {
+            msg: format!("unknown key-type {key_type}"),
+            source: None,
+        }
+    }
+
+    pub(crate) fn decoding_unsupported(key_type: &'static str) -> Self {
+        Self {
+            msg: format!("decoding {key_type} key from Protobuf is unsupported"),
+            source: None,
+        }
+    }
+
+    pub(crate) fn encoding_unsupported(key_type: &'static str) -> Self {
+        Self {
+            msg: format!("encoding {key_type} key to Protobuf is unsupported"),
+            source: None,
         }
     }
 }

core/src/identity/rsa.rs

@@ -48,7 +48,7 @@ impl Keypair {
     /// [RFC5208]: https://tools.ietf.org/html/rfc5208#section-5
     pub fn from_pkcs8(der: &mut [u8]) -> Result<Keypair, DecodingError> {
         let kp = RsaKeyPair::from_pkcs8(der)
-            .map_err(|e| DecodingError::new("RSA PKCS#8 PrivateKeyInfo").source(e))?;
+            .map_err(|e| DecodingError::failed_to_parse("RSA PKCS#8 PrivateKeyInfo", e))?;
         der.zeroize();
         Ok(Keypair(Arc::new(kp)))
     }
@@ -111,7 +111,7 @@ impl PublicKey {
     /// structure. See also `encode_x509`.
     pub fn decode_x509(pk: &[u8]) -> Result<PublicKey, DecodingError> {
         Asn1SubjectPublicKeyInfo::decode(pk)
-            .map_err(|e| DecodingError::new("RSA X.509").source(e))
+            .map_err(|e| DecodingError::failed_to_parse("RSA X.509", e))
             .map(|spki| spki.subjectPublicKey.0)
     }
 }

core/src/identity/secp256k1.rs

@@ -100,7 +100,7 @@ impl SecretKey {
     pub fn from_bytes(mut sk: impl AsMut<[u8]>) -> Result<SecretKey, DecodingError> {
         let sk_bytes = sk.as_mut();
         let secret = libsecp256k1::SecretKey::parse_slice(&*sk_bytes)
-            .map_err(|_| DecodingError::new("failed to parse secp256k1 secret key"))?;
+            .map_err(|e| DecodingError::failed_to_parse("parse secp256k1 secret key", e))?;
         sk_bytes.zeroize();
         Ok(SecretKey(secret))
     }
@@ -112,13 +112,12 @@ impl SecretKey {
     pub fn from_der(mut der: impl AsMut<[u8]>) -> Result<SecretKey, DecodingError> {
         // TODO: Stricter parsing.
         let der_obj = der.as_mut();
-        let obj: Sequence = DerDecodable::decode(der_obj)
+
-            .map_err(|e| DecodingError::new("Secp256k1 DER ECPrivateKey").source(e))?;
+        let mut sk_bytes = Sequence::decode(der_obj)
-        let sk_obj = obj
+            .and_then(|seq| seq.get(1))
-            .get(1)
+            .and_then(Vec::load)
-            .map_err(|e| DecodingError::new("Not enough elements in DER").source(e))?;
+            .map_err(|e| DecodingError::failed_to_parse("secp256k1 SecretKey bytes", e))?;
-        let mut sk_bytes: Vec<u8> =
+
-            asn1_der::typed::DerDecodable::load(sk_obj).map_err(DecodingError::new)?;
         let sk = SecretKey::from_bytes(&mut sk_bytes)?;
         sk_bytes.zeroize();
         der_obj.zeroize();
@@ -217,7 +216,7 @@ impl PublicKey {
     /// by `encode`.
     pub fn decode(k: &[u8]) -> Result<PublicKey, DecodingError> {
         libsecp256k1::PublicKey::parse_slice(k, Some(libsecp256k1::PublicKeyFormat::Compressed))
-            .map_err(|_| DecodingError::new("failed to parse secp256k1 public key"))
+            .map_err(|e| DecodingError::failed_to_parse("secp256k1 public key", e))
             .map(PublicKey)
     }
 }

core/src/upgrade/error.rs

@@ -55,8 +55,8 @@ where
 {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
-            UpgradeError::Select(e) => write!(f, "select error: {}", e),
+            UpgradeError::Select(_) => write!(f, "Multistream select failed"),
-            UpgradeError::Apply(e) => write!(f, "upgrade apply error: {}", e),
+            UpgradeError::Apply(_) => write!(f, "Handshake failed"),
         }
     }
 }

examples/ipfs-kad.rs

@@ -23,16 +23,15 @@
 //! You can pass as parameter a base58 peer ID to search for. If you don't pass any parameter, a
 //! peer ID will be generated randomly.
 
-use async_std::task;
 use futures::StreamExt;
 use libp2p::kad::record::store::MemoryStore;
 use libp2p::kad::{GetClosestPeersError, Kademlia, KademliaConfig, KademliaEvent, QueryResult};
 use libp2p::{
     development_transport, identity,
     swarm::{Swarm, SwarmEvent},
-    Multiaddr, PeerId,
+    PeerId,
 };
-use std::{env, error::Error, str::FromStr, time::Duration};
+use std::{env, error::Error, time::Duration};
 
 const BOOTNODES: [&str; 4] = [
     "QmNnooDu7bfjPFoTZYxMNLWUQJyrVwtbZg5gBMjTezGAJN",
@@ -63,58 +62,54 @@ async fn main() -> Result<(), Box<dyn Error>> {
         // Add the bootnodes to the local routing table. `libp2p-dns` built
         // into the `transport` resolves the `dnsaddr` when Kademlia tries
         // to dial these nodes.
-        let bootaddr = Multiaddr::from_str("/dnsaddr/bootstrap.libp2p.io")?;
         for peer in &BOOTNODES {
-            behaviour.add_address(&PeerId::from_str(peer)?, bootaddr.clone());
+            behaviour.add_address(&peer.parse()?, "/dnsaddr/bootstrap.libp2p.io".parse()?);
         }
 
         Swarm::with_async_std_executor(transport, behaviour, local_peer_id)
     };
 
     // Order Kademlia to search for a peer.
-    let to_search: PeerId = if let Some(peer_id) = env::args().nth(1) {
+    let to_search = env::args()
-        peer_id.parse()?
+        .nth(1)
-    } else {
+        .map(|p| p.parse())
-        identity::Keypair::generate_ed25519().public().into()
+        .transpose()?
-    };
+        .unwrap_or_else(PeerId::random);
 
-    println!("Searching for the closest peers to {to_search:?}");
+    println!("Searching for the closest peers to {to_search}");
     swarm.behaviour_mut().get_closest_peers(to_search);
 
-    // Kick it off!
+    loop {
-    task::block_on(async move {
+        let event = swarm.select_next_some().await;
-        loop {
+        if let SwarmEvent::Behaviour(KademliaEvent::OutboundQueryCompleted {
-            let event = swarm.select_next_some().await;
+            result: QueryResult::GetClosestPeers(result),
-            if let SwarmEvent::Behaviour(KademliaEvent::OutboundQueryCompleted {
+            ..
-                result: QueryResult::GetClosestPeers(result),
+        }) = event
-                ..
+        {
-            }) = event
+            match result {
-            {
+                Ok(ok) => {
-                match result {
+                    if !ok.peers.is_empty() {
-                    Ok(ok) => {
+                        println!("Query finished with closest peers: {:#?}", ok.peers)
-                        if !ok.peers.is_empty() {
+                    } else {
-                            println!("Query finished with closest peers: {:#?}", ok.peers)
+                        // The example is considered failed as there
-                        } else {
+                        // should always be at least 1 reachable peer.
-                            // The example is considered failed as there
+                        println!("Query finished with no closest peers.")
-                            // should always be at least 1 reachable peer.
-                            println!("Query finished with no closest peers.")
-                        }
                     }
-                    Err(GetClosestPeersError::Timeout { peers, .. }) => {
+                }
-                        if !peers.is_empty() {
+                Err(GetClosestPeersError::Timeout { peers, .. }) => {
-                            println!("Query timed out with closest peers: {peers:#?}")
+                    if !peers.is_empty() {
-                        } else {
+                        println!("Query timed out with closest peers: {peers:#?}")
-                            // The example is considered failed as there
+                    } else {
-                            // should always be at least 1 reachable peer.
+                        // The example is considered failed as there
-                            println!("Query timed out with no closest peers.");
+                        // should always be at least 1 reachable peer.
-                        }
+                        println!("Query timed out with no closest peers.");
                     }
-                };
+                }
+            };
 
-                break;
+            break;
-            }
         }
+    }
 
-        Ok(())
+    Ok(())
-    })
 }

swarm/src/lib.rs

@@ -1660,17 +1660,45 @@ impl fmt::Display for DialError {
                 f,
                 "Dial error: Pending connection attempt has been aborted."
             ),
-            DialError::InvalidPeerId(multihash) => write!(f, "Dial error: multihash {:?} is not a PeerId", multihash),
+            DialError::InvalidPeerId(multihash) => {
-            DialError::WrongPeerId { obtained, endpoint} => write!(f, "Dial error: Unexpected peer ID {} at {:?}.", obtained, endpoint),
+                write!(f, "Dial error: multihash {:?} is not a PeerId", multihash)
+            }
+            DialError::WrongPeerId { obtained, endpoint } => write!(
+                f,
+                "Dial error: Unexpected peer ID {} at {:?}.",
+                obtained, endpoint
+            ),
             DialError::ConnectionIo(e) => write!(
                 f,
-                "Dial error: An I/O error occurred on the connection: {:?}.", e
+                "Dial error: An I/O error occurred on the connection: {:?}.",
+                e
             ),
-            DialError::Transport(e) => write!(f, "An error occurred while negotiating the transport protocol(s) on a connection: {:?}.", e),
+            DialError::Transport(errors) => {
+                write!(f, "Failed to negotiate transport protocol(s): [")?;
+
+                for (addr, error) in errors {
+                    write!(f, "({addr}")?;
+                    print_error_chain(f, error)?;
+                    write!(f, ")")?;
+                }
+                write!(f, "]")?;
+
+                Ok(())
+            }
         }
     }
 }
 
+fn print_error_chain(f: &mut fmt::Formatter<'_>, e: &dyn error::Error) -> fmt::Result {
+    write!(f, ": {e}")?;
+
+    if let Some(source) = e.source() {
+        print_error_chain(f, source)?;
+    }
+
+    Ok(())
+}
+
 impl error::Error for DialError {
     fn source(&self) -> Option<&(dyn error::Error + 'static)> {
         match self {
@@ -1745,13 +1773,16 @@ mod tests {
     use futures::future::poll_fn;
     use futures::future::Either;
     use futures::{executor, future, ready};
+    use libp2p_core::either::EitherError;
     use libp2p_core::multiaddr::multiaddr;
+    use libp2p_core::transport::memory::MemoryTransportError;
     use libp2p_core::transport::TransportEvent;
-    use libp2p_core::Endpoint;
     use libp2p_core::{identity, multiaddr, transport, upgrade};
+    use libp2p_core::{Endpoint, UpgradeError};
     use libp2p_plaintext as plaintext;
     use libp2p_yamux as yamux;
     use quickcheck::*;
+    use void::Void;
 
     // Test execution state.
     // Connection => Disconnecting => Connecting.
@@ -2610,4 +2641,23 @@ mod tests {
             e => panic!("Unexpected swarm event {:?}.", e),
         }
     }
+
+    #[test]
+    fn dial_error_prints_sources() {
+        // This constitutes a fairly typical error for chained transports.
+        let error = DialError::Transport(vec![(
+            "/ip4/127.0.0.1/tcp/80".parse().unwrap(),
+            TransportError::Other(io::Error::new(
+                io::ErrorKind::Other,
+                EitherError::<_, Void>::A(EitherError::<Void, _>::B(UpgradeError::Apply(
+                    MemoryTransportError::Unreachable,
+                ))),
+            )),
+        )]);
+
+        let string = format!("{error}");
+
+        // Unfortunately, we have some "empty" errors that lead to multiple colons without text but that is the best we can do.
+        assert_eq!("Failed to negotiate transport protocol(s): [(/ip4/127.0.0.1/tcp/80: : Handshake failed: No listener on the given port.)]", string)
+    }
 }

transports/noise/CHANGELOG.md

@@ -6,8 +6,11 @@
 
 - Update `rust-version` to reflect the actual MSRV: 1.60.0. See [PR 3090].
 
+- Introduce more variants to `NoiseError` to better differentiate between failure cases during authentication. See [PR 2972].
+
 [PR 3058]: https://github.com/libp2p/rust-libp2p/pull/3058
 [PR 3090]: https://github.com/libp2p/rust-libp2p/pull/3090
+[PR 2972]: https://github.com/libp2p/rust-libp2p/pull/2972
 
 # 0.40.0
 

transports/noise/Cargo.toml

@@ -19,6 +19,7 @@ prost = "0.11"
 rand = "0.8.3"
 sha2 = "0.10.0"
 static_assertions = "1"
+thiserror = "1.0.37"
 x25519-dalek = "1.1.0"
 zeroize = "1"
 

transports/noise/src/error.rs

@@ -1,107 +0,0 @@
-// Copyright 2019 Parity Technologies (UK) Ltd.
-//
-// Permission is hereby granted, free of charge, to any person obtaining a
-// copy of this software and associated documentation files (the "Software"),
-// to deal in the Software without restriction, including without limitation
-// the rights to use, copy, modify, merge, publish, distribute, sublicense,
-// and/or sell copies of the Software, and to permit persons to whom the
-// Software is furnished to do so, subject to the following conditions:
-//
-// The above copyright notice and this permission notice shall be included in
-// all copies or substantial portions of the Software.
-//
-// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
-// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
-// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
-// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
-// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
-// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
-// DEALINGS IN THE SOFTWARE.
-
-use libp2p_core::identity;
-use snow::error::Error as SnowError;
-use std::{error::Error, fmt, io};
-
-/// libp2p_noise error type.
-#[derive(Debug)]
-#[non_exhaustive]
-pub enum NoiseError {
-    /// An I/O error has been encountered.
-    Io(io::Error),
-    /// An noise framework error has been encountered.
-    Noise(SnowError),
-    /// A public key is invalid.
-    InvalidKey,
-    /// Authentication in a [`NoiseAuthenticated`](crate::NoiseAuthenticated)
-    /// upgrade failed.
-    AuthenticationFailed,
-    /// A handshake payload is invalid.
-    InvalidPayload(DecodeError),
-    /// A signature was required and could not be created.
-    SigningError(identity::error::SigningError),
-}
-
-#[derive(Debug)]
-pub struct DecodeError(prost::DecodeError);
-
-impl fmt::Display for DecodeError {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        self.0.fmt(f)
-    }
-}
-
-impl Error for DecodeError {
-    fn source(&self) -> Option<&(dyn Error + 'static)> {
-        self.0.source()
-    }
-}
-
-impl fmt::Display for NoiseError {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match self {
-            NoiseError::Io(e) => write!(f, "{}", e),
-            NoiseError::Noise(e) => write!(f, "{}", e),
-            NoiseError::InvalidKey => f.write_str("invalid public key"),
-            NoiseError::InvalidPayload(e) => write!(f, "{}", e),
-            NoiseError::AuthenticationFailed => f.write_str("Authentication failed"),
-            NoiseError::SigningError(e) => write!(f, "{}", e),
-        }
-    }
-}
-
-impl Error for NoiseError {
-    fn source(&self) -> Option<&(dyn Error + 'static)> {
-        match self {
-            NoiseError::Io(e) => Some(e),
-            NoiseError::Noise(_) => None, // TODO: `SnowError` should implement `Error`.
-            NoiseError::InvalidKey => None,
-            NoiseError::AuthenticationFailed => None,
-            NoiseError::InvalidPayload(e) => Some(e),
-            NoiseError::SigningError(e) => Some(e),
-        }
-    }
-}
-
-impl From<io::Error> for NoiseError {
-    fn from(e: io::Error) -> Self {
-        NoiseError::Io(e)
-    }
-}
-
-impl From<SnowError> for NoiseError {
-    fn from(e: SnowError) -> Self {
-        NoiseError::Noise(e)
-    }
-}
-
-impl From<prost::DecodeError> for NoiseError {
-    fn from(e: prost::DecodeError) -> Self {
-        NoiseError::InvalidPayload(DecodeError(e))
-    }
-}
-
-impl From<identity::error::SigningError> for NoiseError {
-    fn from(e: identity::error::SigningError) -> Self {
-        NoiseError::SigningError(e)
-    }
-}

transports/noise/src/io/framed.rs

@@ -94,28 +94,23 @@ impl<T> NoiseFramed<T, snow::HandshakeState> {
     where
         C: Protocol<C> + AsRef<[u8]>,
     {
-        let dh_remote_pubkey = match self.session.get_remote_static() {
+        let dh_remote_pubkey = self
-            None => None,
+            .session
-            Some(k) => match C::public_from_bytes(k) {
+            .get_remote_static()
-                Err(e) => return Err(e),
+            .map(C::public_from_bytes)
-                Ok(dh_pk) => Some(dh_pk),
+            .transpose()?;
-            },
+
+        let io = NoiseFramed {
+            session: self.session.into_transport_mode()?,
+            io: self.io,
+            read_state: ReadState::Ready,
+            write_state: WriteState::Ready,
+            read_buffer: self.read_buffer,
+            write_buffer: self.write_buffer,
+            decrypt_buffer: self.decrypt_buffer,
         };
-        match self.session.into_transport_mode() {
+
-            Err(e) => Err(e.into()),
+        Ok((dh_remote_pubkey, NoiseOutput::new(io)))
-            Ok(s) => {
-                let io = NoiseFramed {
-                    session: s,
-                    io: self.io,
-                    read_state: ReadState::Ready,
-                    write_state: WriteState::Ready,
-                    read_buffer: self.read_buffer,
-                    write_buffer: self.write_buffer,
-                    decrypt_buffer: self.decrypt_buffer,
-                };
-                Ok((dh_remote_pubkey, NoiseOutput::new(io)))
-            }
-        }
     }
 }
 

transports/noise/src/io/handshake.rs

@@ -25,10 +25,10 @@ mod payload_proto {
     include!(concat!(env!("OUT_DIR"), "/payload.proto.rs"));
 }
 
-use crate::error::NoiseError;
 use crate::io::{framed::NoiseFramed, NoiseOutput};
 use crate::protocol::{KeypairIdentity, Protocol, PublicKey};
 use crate::LegacyConfig;
+use crate::NoiseError;
 use bytes::Bytes;
 use futures::prelude::*;
 use libp2p_core::identity;
@@ -118,7 +118,7 @@ impl<T> State<T> {
                 if C::verify(&id_pk, &dh_pk, &self.dh_remote_pubkey_sig) {
                     RemoteIdentity::IdentityKey(id_pk)
                 } else {
-                    return Err(NoiseError::InvalidKey);
+                    return Err(NoiseError::BadSignature);
                 }
             }
         };
@@ -208,11 +208,10 @@ where
     let pb = pb_result?;
 
     if !pb.identity_key.is_empty() {
-        let pk = identity::PublicKey::from_protobuf_encoding(&pb.identity_key)
+        let pk = identity::PublicKey::from_protobuf_encoding(&pb.identity_key)?;
-            .map_err(|_| NoiseError::InvalidKey)?;
         if let Some(ref k) = state.id_remote_pubkey {
             if k != &pk {
-                return Err(NoiseError::InvalidKey);
+                return Err(NoiseError::UnexpectedKey);
             }
         }
         state.id_remote_pubkey = Some(pk);

transports/noise/src/lib.rs

@@ -55,11 +55,9 @@
 
 #![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))]
 
-mod error;
 mod io;
 mod protocol;
 
-pub use error::NoiseError;
 pub use io::handshake::RemoteIdentity;
 pub use io::NoiseOutput;
 pub use protocol::{x25519::X25519, x25519_spec::X25519Spec};
@@ -241,6 +239,42 @@ where
     }
 }
 
+/// libp2p_noise error type.
+#[derive(Debug, thiserror::Error)]
+#[non_exhaustive]
+pub enum NoiseError {
+    #[error(transparent)]
+    Io(#[from] std::io::Error),
+    #[error(transparent)]
+    Noise(#[from] snow::Error),
+    #[error("Invalid public key")]
+    InvalidKey(#[from] identity::error::DecodingError),
+    #[error("Only keys of length 32 bytes are supported")]
+    InvalidLength,
+    #[error("Remote authenticated with an unexpected public key")]
+    UnexpectedKey,
+    #[error("The signature of the remote identity's public key does not verify")]
+    BadSignature,
+    #[error("Authentication failed")]
+    AuthenticationFailed,
+    #[error(transparent)]
+    InvalidPayload(DecodeError),
+    #[error(transparent)]
+    SigningError(#[from] identity::error::SigningError),
+}
+
+#[derive(Debug, thiserror::Error)]
+#[error(transparent)]
+pub struct DecodeError(prost::DecodeError);
+
+impl From<prost::DecodeError> for NoiseError {
+    fn from(e: prost::DecodeError) -> Self {
+        NoiseError::InvalidPayload(DecodeError(e))
+    }
+}
+
+// Handshake pattern IX /////////////////////////////////////////////////////
+
 /// Implements the responder part of the `IX` noise handshake pattern.
 ///
 /// `IX` is a single round-trip (2 messages) handshake in which each party sends their identity over to the other party.

transports/noise/src/protocol/x25519.rs

@@ -117,7 +117,7 @@ impl Protocol<X25519> for X25519 {
 
     fn public_from_bytes(bytes: &[u8]) -> Result<PublicKey<X25519>, NoiseError> {
         if bytes.len() != 32 {
-            return Err(NoiseError::InvalidKey);
+            return Err(NoiseError::InvalidLength);
         }
         let mut pk = [0u8; 32];
         pk.copy_from_slice(bytes);

transports/noise/src/protocol/x25519_spec.rs

@@ -123,7 +123,7 @@ impl Protocol<X25519Spec> for X25519Spec {
 
     fn public_from_bytes(bytes: &[u8]) -> Result<PublicKey<X25519Spec>, NoiseError> {
         if bytes.len() != 32 {
-            return Err(NoiseError::InvalidKey);
+            return Err(NoiseError::InvalidLength);
         }
         let mut pk = [0u8; 32];
         pk.copy_from_slice(bytes);

transports/noise/tests/smoke.rs

@@ -27,10 +27,10 @@ use libp2p::core::identity;
 use libp2p::core::transport::{self, Transport};
 use libp2p::core::upgrade::{self, apply_inbound, apply_outbound, Negotiated};
 use libp2p::noise::{
-    Keypair, NoiseAuthenticated, NoiseConfig, NoiseError, NoiseOutput, RemoteIdentity, X25519Spec,
+    Keypair, NoiseAuthenticated, NoiseConfig, NoiseOutput, RemoteIdentity, X25519Spec, X25519,
-    X25519,
 };
 use libp2p::tcp;
+use libp2p_noise::NoiseError;
 use log::info;
 use quickcheck::*;
 use std::{convert::TryInto, io, net::TcpStream};