bugfix: verifying signed information of legacy nodes (#5029)

* Added new legacy variant of HostInformation

* fixed 'option_bs58_x25519_pubkey' for empty string

* 'Debug' impl for x25519 and ed25519 to use human-readable representation

* HttpClient to use explicit 'serde_json' conversion for better errors

* additional 'Debug' derives

common/crypto/src/asymmetric/encryption/mod.rs

@@ -2,7 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 use nym_pemstore::traits::{PemStorableKey, PemStorableKeyPair};
-use std::fmt::{self, Display, Formatter};
+use std::fmt::{self, Debug, Display, Formatter};
 use std::str::FromStr;
 use thiserror::Error;
 use zeroize::{Zeroize, ZeroizeOnDrop};
@@ -112,12 +112,18 @@ impl PemStorableKeyPair for KeyPair {
     }
 }
 
-#[derive(PartialEq, Eq, Hash, Copy, Clone, Debug)]
+#[derive(PartialEq, Eq, Hash, Copy, Clone)]
 pub struct PublicKey(x25519_dalek::PublicKey);
 
 impl Display for PublicKey {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
-        write!(f, "{}", self.to_base58_string())
+        Display::fmt(&self.to_base58_string(), f)
+    }
+}
+
+impl Debug for PublicKey {
+    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+        Debug::fmt(&self.to_base58_string(), f)
     }
 }
 

common/crypto/src/asymmetric/encryption/serde_helpers.rs

@@ -31,8 +31,16 @@ pub mod option_bs58_x25519_pubkey {
     pub fn deserialize<'de, D: Deserializer<'de>>(
         deserializer: D,
     ) -> Result<Option<PublicKey>, D::Error> {
-        let s = Option::<String>::deserialize(deserializer)?;
-        s.map(|s| PublicKey::from_base58_string(&s).map_err(serde::de::Error::custom))
-            .transpose()
+        match Option::<String>::deserialize(deserializer)? {
+            None => Ok(None),
+            Some(s) => {
+                if s.is_empty() {
+                    Ok(None)
+                } else {
+                    Some(PublicKey::from_base58_string(&s).map_err(serde::de::Error::custom))
+                        .transpose()
+                }
+            }
+        }
     }
 }

common/crypto/src/asymmetric/identity/mod.rs

@@ -5,7 +5,7 @@ pub use ed25519_dalek::SignatureError;
 use ed25519_dalek::{Signer, SigningKey};
 pub use ed25519_dalek::{Verifier, PUBLIC_KEY_LENGTH, SECRET_KEY_LENGTH, SIGNATURE_LENGTH};
 use nym_pemstore::traits::{PemStorableKey, PemStorableKeyPair};
-use std::fmt::{self, Display, Formatter};
+use std::fmt::{self, Debug, Display, Formatter};
 use std::str::FromStr;
 use thiserror::Error;
 use zeroize::{Zeroize, ZeroizeOnDrop};
@@ -119,12 +119,18 @@ impl PemStorableKeyPair for KeyPair {
 }
 
 /// ed25519 EdDSA Public Key
-#[derive(Debug, Copy, Clone, Eq, PartialEq)]
+#[derive(Copy, Clone, Eq, PartialEq)]
 pub struct PublicKey(ed25519_dalek::VerifyingKey);
 
 impl Display for PublicKey {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
-        write!(f, "{}", self.to_base58_string())
+        Display::fmt(&self.to_base58_string(), f)
+    }
+}
+
+impl Debug for PublicKey {
+    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+        Debug::fmt(&self.to_base58_string(), f)
     }
 }
 

common/http-api-client/src/lib.rs

@@ -35,6 +35,9 @@ pub enum HttpClientError<E: Display = String> {
         source: reqwest::Error,
     },
 
+    #[error("failed to deserialise received response: {source}")]
+    ResponseDeserialisationFailure { source: serde_json::Error },
+
     #[error("provided url is malformed: {source}")]
     MalformedUrl {
         #[from]
@@ -526,7 +529,11 @@ where
     }
 
     if res.status().is_success() {
-        Ok(res.json().await?)
+        let text = res.text().await?;
+        match serde_json::from_str(&text) {
+            Ok(res) => Ok(res),
+            Err(source) => Err(HttpClientError::ResponseDeserialisationFailure { source }),
+        }
     } else if res.status() == StatusCode::NOT_FOUND {
         Err(HttpClientError::NotFound)
     } else {

nym-api/src/node_describe_cache/mod.rs

@@ -145,6 +145,7 @@ impl NodeDescriptionTopologyExt for NymNodeDescription {
     }
 }
 
+#[derive(Debug, Clone)]
 pub struct DescribedNodes {
     nodes: HashMap<NodeId, NymNodeDescription>,
 }
@@ -290,6 +291,7 @@ async fn try_get_description(
     })
 }
 
+#[derive(Debug)]
 struct RefreshData {
     host: String,
     node_id: NodeId,

nym-api/src/node_describe_cache/query_helpers.rs

@@ -210,6 +210,7 @@ impl ResolvedNodeDescribedInfo {
     }
 }
 
+#[derive(Debug)]
 pub(crate) struct UnwrappedResolvedNodeDescribedInfo {
     pub(crate) build_info: BinaryBuildInformationOwned,
     pub(crate) roles: DeclaredRoles,

nym-node/nym-node-requests/src/api/mod.rs

@@ -1,7 +1,9 @@
 // Copyright 2023 - Nym Technologies SA <contact@nymtech.net>
 // SPDX-License-Identifier: Apache-2.0
 
-use crate::api::v1::node::models::{HostInformation, LegacyHostInformation};
+use crate::api::v1::node::models::{
+    HostInformation, LegacyHostInformation, LegacyHostInformationV2,
+};
 use crate::error::Error;
 use nym_crypto::asymmetric::identity;
 use schemars::JsonSchema;
@@ -61,9 +63,18 @@ impl SignedHostInformation {
             return true;
         }
 
-        // attempt to verify legacy signature
+        // attempt to verify legacy signatures
+        let legacy_v2 = SignedData {
+            data: LegacyHostInformationV2::from(self.data.clone()),
+            signature: self.signature.clone(),
+        };
+
+        if legacy_v2.verify(&self.keys.ed25519_identity) {
+            return true;
+        }
+
         SignedData {
-            data: LegacyHostInformation::from(self.data.clone()),
+            data: LegacyHostInformation::from(legacy_v2.data),
             signature: self.signature.clone(),
         }
         .verify(&self.keys.ed25519_identity)
@@ -133,6 +144,81 @@ mod tests {
         assert!(signed_info.verify_host_information());
     }
 
+    #[test]
+    fn dummy_legacy_v2_signed_host_verification() {
+        let mut rng = rand_chacha::ChaCha20Rng::from_seed([0u8; 32]);
+        let ed22519 = ed25519::KeyPair::new(&mut rng);
+        let x25519_sphinx = x25519::KeyPair::new(&mut rng);
+        let x25519_noise = x25519::KeyPair::new(&mut rng);
+
+        let legacy_info_no_noise = crate::api::v1::node::models::LegacyHostInformationV2 {
+            ip_address: vec!["1.1.1.1".parse().unwrap()],
+            hostname: Some("foomp.com".to_string()),
+            keys: crate::api::v1::node::models::LegacyHostKeysV2 {
+                ed25519_identity: ed22519.public_key().to_base58_string(),
+                x25519_sphinx: x25519_sphinx.public_key().to_base58_string(),
+                x25519_noise: "".to_string(),
+            },
+        };
+
+        let legacy_info_noise = crate::api::v1::node::models::LegacyHostInformationV2 {
+            ip_address: vec!["1.1.1.1".parse().unwrap()],
+            hostname: Some("foomp.com".to_string()),
+            keys: crate::api::v1::node::models::LegacyHostKeysV2 {
+                ed25519_identity: ed22519.public_key().to_base58_string(),
+                x25519_sphinx: x25519_sphinx.public_key().to_base58_string(),
+                x25519_noise: x25519_noise.public_key().to_base58_string(),
+            },
+        };
+
+        let host_info_no_noise = crate::api::v1::node::models::HostInformation {
+            ip_address: legacy_info_no_noise.ip_address.clone(),
+            hostname: legacy_info_no_noise.hostname.clone(),
+            keys: crate::api::v1::node::models::HostKeys {
+                ed25519_identity: legacy_info_no_noise.keys.ed25519_identity.parse().unwrap(),
+                x25519_sphinx: legacy_info_no_noise.keys.x25519_sphinx.parse().unwrap(),
+                x25519_noise: None,
+            },
+        };
+
+        let host_info_noise = crate::api::v1::node::models::HostInformation {
+            ip_address: legacy_info_noise.ip_address.clone(),
+            hostname: legacy_info_noise.hostname.clone(),
+            keys: crate::api::v1::node::models::HostKeys {
+                ed25519_identity: legacy_info_noise.keys.ed25519_identity.parse().unwrap(),
+                x25519_sphinx: legacy_info_noise.keys.x25519_sphinx.parse().unwrap(),
+                x25519_noise: Some(legacy_info_noise.keys.x25519_noise.parse().unwrap()),
+            },
+        };
+
+        // signature on legacy data
+        let signature_no_noise = SignedData::new(legacy_info_no_noise, ed22519.private_key())
+            .unwrap()
+            .signature;
+
+        let signature_noise = SignedData::new(legacy_info_noise, ed22519.private_key())
+            .unwrap()
+            .signature;
+
+        // signed blob with the 'current' structure
+        let current_struct_no_noise = SignedData {
+            data: host_info_no_noise,
+            signature: signature_no_noise,
+        };
+
+        let current_struct_noise = SignedData {
+            data: host_info_noise,
+            signature: signature_noise,
+        };
+
+        assert!(!current_struct_no_noise.verify(ed22519.public_key()));
+        assert!(current_struct_no_noise.verify_host_information());
+
+        // if noise key is present, the signature is actually valid
+        assert!(current_struct_noise.verify(ed22519.public_key()));
+        assert!(current_struct_noise.verify_host_information())
+    }
+
     #[test]
     fn dummy_legacy_signed_host_verification() {
         let mut rng = rand_chacha::ChaCha20Rng::from_seed([0u8; 32]);

nym-node/nym-node-requests/src/api/v1/node/models.rs

@@ -59,6 +59,13 @@ pub struct HostInformation {
     pub keys: HostKeys,
 }
 
+#[derive(Serialize)]
+pub struct LegacyHostInformationV2 {
+    pub ip_address: Vec<IpAddr>,
+    pub hostname: Option<String>,
+    pub keys: LegacyHostKeysV2,
+}
+
 #[derive(Serialize)]
 pub struct LegacyHostInformation {
     pub ip_address: Vec<IpAddr>,
@@ -66,8 +73,18 @@ pub struct LegacyHostInformation {
     pub keys: LegacyHostKeys,
 }
 
-impl From<HostInformation> for LegacyHostInformation {
+impl From<HostInformation> for LegacyHostInformationV2 {
     fn from(value: HostInformation) -> Self {
+        LegacyHostInformationV2 {
+            ip_address: value.ip_address,
+            hostname: value.hostname,
+            keys: value.keys.into(),
+        }
+    }
+}
+
+impl From<LegacyHostInformationV2> for LegacyHostInformation {
+    fn from(value: LegacyHostInformationV2) -> Self {
         LegacyHostInformation {
             ip_address: value.ip_address,
             hostname: value.hostname,
@@ -99,13 +116,11 @@ pub struct HostKeys {
     pub x25519_noise: Option<x25519::PublicKey>,
 }
 
-impl From<HostKeys> for LegacyHostKeys {
-    fn from(value: HostKeys) -> Self {
-        LegacyHostKeys {
-            ed25519: value.ed25519_identity.to_base58_string(),
-            x25519: value.x25519_sphinx.to_base58_string(),
-        }
-    }
+#[derive(Serialize)]
+pub struct LegacyHostKeysV2 {
+    pub ed25519_identity: String,
+    pub x25519_sphinx: String,
+    pub x25519_noise: String,
 }
 
 #[derive(Serialize)]
@@ -114,6 +129,28 @@ pub struct LegacyHostKeys {
     pub x25519: String,
 }
 
+impl From<HostKeys> for LegacyHostKeysV2 {
+    fn from(value: HostKeys) -> Self {
+        LegacyHostKeysV2 {
+            ed25519_identity: value.ed25519_identity.to_base58_string(),
+            x25519_sphinx: value.x25519_sphinx.to_base58_string(),
+            x25519_noise: value
+                .x25519_noise
+                .map(|k| k.to_base58_string())
+                .unwrap_or_default(),
+        }
+    }
+}
+
+impl From<LegacyHostKeysV2> for LegacyHostKeys {
+    fn from(value: LegacyHostKeysV2) -> Self {
+        LegacyHostKeys {
+            ed25519: value.ed25519_identity,
+            x25519: value.x25519_sphinx,
+        }
+    }
+}
+
 #[derive(Clone, Default, Debug, Serialize, Deserialize, JsonSchema)]
 #[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))]
 pub struct HostSystem {