feat: Better error reporting when features are disabled

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(_) => {
-                return Err(DecodingError::new(
-                    "Encoding RSA key into Protobuf is unsupported",
-                ))
-            }
+            Self::Rsa(_) => return Err(DecodingError::encoding_unsupported("RSA")),
             #[cfg(feature = "secp256k1")]
-            Self::Secp256k1(_) => {
-                return Err(DecodingError::new(
-                    "Encoding Secp256k1 key into Protobuf is unsupported",
-                ))
-            }
+            Self::Secp256k1(_) => return Err(DecodingError::encoding_unsupported("secp256k1")),
             #[cfg(feature = "ecdsa")]
-            Self::Ecdsa(_) => {
-                return Err(DecodingError::new(
-                    "Encoding ECDSA key into Protobuf is unsupported",
-                ))
-            }
+            Self::Ecdsa(_) => return Err(DecodingError::encoding_unsupported("ECDSA")),
         };
 
         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(|| {
-            DecodingError::new(format!("unknown key type: {}", private_key.r#type))
-        })?;
+        let key_type = keys_proto::KeyType::from_i32(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(
-                "Decoding RSA key from Protobuf is unsupported.",
-            )),
-            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.",
-            )),
+            keys_proto::KeyType::Rsa => Err(DecodingError::decoding_unsupported("RSA")),
+            keys_proto::KeyType::Secp256k1 => Err(DecodingError::decoding_unsupported("secp256k1")),
+            keys_proto::KeyType::Ecdsa => Err(DecodingError::decoding_unsupported("ECDSA")),
         }
     }
 }
@@ -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(|_| {
-            DecodingError::new("failed to parse ecdsa p256 public key, bad point encoding")
-        })?;
+        let enc_pt = EncodedPoint::from_bytes(k)
+            .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 sk_obj = obj
-            .get(1)
-            .map_err(|e| DecodingError::new("Not enough elements in DER").source(e))?;
-        let mut sk_bytes: Vec<u8> =
-            asn1_der::typed::DerDecodable::load(sk_obj).map_err(DecodingError::new)?;
+
+        let mut sk_bytes = Sequence::decode(der_obj)
+            .and_then(|seq| seq.get(1))
+            .and_then(Vec::load)
+            .map_err(|e| DecodingError::failed_to_parse("secp256k1 SecretKey bytes", e))?;
+
         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::Apply(e) => write!(f, "upgrade apply error: {}", e),
+            UpgradeError::Select(_) => write!(f, "Multistream select failed"),
+            UpgradeError::Apply(_) => write!(f, "Handshake failed"),
         }
     }
 }