diff --git a/c-bindings/src/api/blend.rs b/c-bindings/src/api/blend.rs index 5f67eb39f..9b2d445eb 100644 --- a/c-bindings/src/api/blend.rs +++ b/c-bindings/src/api/blend.rs @@ -6,7 +6,6 @@ use lb_core::{ }; use lb_groth16::fr_from_bytes; use lb_key_management_system_keys::keys::ZkPublicKey; -use multiaddr::Multiaddr; use crate::{ LogosBlockchainNode, @@ -67,11 +66,11 @@ unsafe fn parse_locators(ptrs: *const *const c_char, len: usize) -> StatusResult log::error!("[blend_join_as_core_node] `locators[{i}]` is not valid UTF-8."); return Err(OperationStatus::ValidationError); }; - let Ok(addr) = s.parse::() else { - log::error!("[blend_join_as_core_node] `locators[{i}]` is not a valid multiaddr."); + let Ok(addr) = s.parse::() else { + log::error!("[blend_join_as_core_node] `locators[{i}]` is not a valid locator."); return Err(OperationStatus::ValidationError); }; - parsed.push(Locator(addr)); + parsed.push(addr); } Ok(parsed) } @@ -86,7 +85,7 @@ unsafe fn parse_locators(ptrs: *const *const c_char, len: usize) -> StatusResult /// - `zk_id`: A non-null pointer to 32 bytes representing the ZK public key. /// - `locked_note_id`: A non-null pointer to 32 bytes representing the locked /// note ID. -/// - `locators`: A pointer to an array of multiaddr C strings. May be null if +/// - `locators`: A pointer to an array of locator C strings. May be null if /// `locators_len` is 0. /// - `locators_len`: Number of entries in the `locators` array. /// diff --git a/core/src/mantle/encoding.rs b/core/src/mantle/encoding.rs index 49a749b17..b0d144827 100644 --- a/core/src/mantle/encoding.rs +++ b/core/src/mantle/encoding.rs @@ -1,5 +1,6 @@ use lb_groth16::{CompressedGroth16Proof, Fr, fr_from_bytes}; use lb_key_management_system_keys::keys::{Ed25519Signature, ZkPublicKey, ZkSignature}; +use multiaddr::Multiaddr; use nom::{ IResult, Parser as _, bytes::complete::take, @@ -190,7 +191,11 @@ fn decode_sdp_declare(input: &[u8]) -> IResult<&[u8], SDPDeclareOp> { let (input, locator_count) = decode_byte(input)?; let (input, multiaddrs) = count(decode_locator, locator_count as usize).parse(input)?; - let locators = multiaddrs.into_iter().map(Locator::new).collect(); + let locators = multiaddrs + .into_iter() + .map(Locator::try_from) + .collect::, _>>() + .map_err(|_| nom::Err::Error(Error::new(input, ErrorKind::Fail)))?; let (input, provider_key) = decode_ed25519_public_key(input)?; let provider_id = ProviderId(provider_key); let (input, zk_fr) = decode_field_element(input)?; @@ -209,7 +214,7 @@ fn decode_sdp_declare(input: &[u8]) -> IResult<&[u8], SDPDeclareOp> { )) } -fn decode_locator(input: &[u8]) -> IResult<&[u8], multiaddr::Multiaddr> { +fn decode_locator(input: &[u8]) -> IResult<&[u8], Multiaddr> { // Locator = 2Byte *BYTE let (input, len_bytes) = take(2usize).parse(input)?; let len = u16::from_le_bytes([len_bytes[0], len_bytes[1]]) as usize; @@ -217,8 +222,7 @@ fn decode_locator(input: &[u8]) -> IResult<&[u8], multiaddr::Multiaddr> { return Err(nom::Err::Error(Error::new(input, ErrorKind::LengthValue))); } map_res(take(len), |bytes: &[u8]| { - multiaddr::Multiaddr::try_from(bytes.to_vec()) - .map_err(|_| Error::new(bytes, ErrorKind::Fail)) + Multiaddr::try_from(bytes.to_vec()).map_err(|_| Error::new(bytes, ErrorKind::Fail)) }) .parse(input) } @@ -664,7 +668,7 @@ pub fn encode_channel_withdraw(op: &ChannelWithdrawOp) -> Vec { } /// Encode SDP operations -fn encode_locator(locator: &multiaddr::Multiaddr) -> Vec { +fn encode_locator(locator: &Multiaddr) -> Vec { let locator_bytes = locator.to_vec(); assert!( locator_bytes.len() <= LOCATOR_BYTES_SIZE_LIMIT, @@ -1322,8 +1326,8 @@ mod tests { let signing_key = Ed25519Key::from_bytes(&[1; 32]); let zk_sk = ZkKey::zero(); - let locator1: multiaddr::Multiaddr = "/ip4/127.0.0.1/tcp/8080".parse().unwrap(); - let locator2: multiaddr::Multiaddr = "/ip6/::1/tcp/9090".parse().unwrap(); + let locator1: Multiaddr = "/ip4/127.0.0.1/tcp/8080".parse().unwrap(); + let locator2: Multiaddr = "/ip6/::1/tcp/9090".parse().unwrap(); let locked_note_sk = ZkKey::from(BigUint::from(1u64)); let locked_note = crate::mantle::Utxo { @@ -1336,7 +1340,10 @@ mod tests { }; let sdp_declare_op = SDPDeclareOp { service_type: ServiceType::BlendNetwork, - locators: vec![Locator::new(locator1), Locator::new(locator2)], + locators: vec![ + Locator::new_unchecked(locator1), + Locator::new_unchecked(locator2), + ], provider_id: ProviderId(signing_key.public_key()), zk_id: zk_sk.to_public_key(), locked_note_id: locked_note.id(), @@ -1557,11 +1564,11 @@ mod tests { outputs: Outputs::new(vec![Note::new(5000, locked_note_sk.to_public_key())]), }; - let locator: multiaddr::Multiaddr = "/dns4/example.com/tcp/443".parse().unwrap(); + let locator: Multiaddr = "/dns4/example.com/tcp/443".parse().unwrap(); let zk_sk = ZkKey::zero(); let sdp_declare_op = SDPDeclareOp { service_type: ServiceType::BlendNetwork, - locators: vec![Locator::new(locator)], + locators: vec![Locator::new_unchecked(locator)], provider_id: ProviderId(signing_key1.public_key()), zk_id: zk_sk.to_public_key(), locked_note_id: transfer_op @@ -1925,10 +1932,10 @@ mod tests { #[test] fn test_encode_reject_excessive_sdp_declare() { - let locator: multiaddr::Multiaddr = "/dns4/example.com/tcp/443".parse().unwrap(); + let locator: Multiaddr = "/dns4/example.com/tcp/443".parse().unwrap(); let sdp_declare_op = SDPDeclareOp { service_type: ServiceType::BlendNetwork, - locators: vec![Locator::new(locator); u8::MAX as usize + 1], // excessive locator count + locators: vec![Locator::new_unchecked(locator); u8::MAX as usize + 1], /* excessive locator count */ provider_id: ProviderId(Ed25519Key::from_bytes(&[1; 32]).public_key()), zk_id: ZkKey::zero().to_public_key(), locked_note_id: NoteId(BigUint::from(111u64).into()), @@ -2046,6 +2053,28 @@ mod tests { assert!(result.is_err(), "Should reject invalid declaration"); } + #[test] + fn decode_reject_invalid_locator() { + let invalid_locator: Multiaddr = "/ip4/0.0.0.0/udp/3000/quic-v1" + .parse() + .expect("locator should parse as multiaddr"); + let op = SDPDeclareOp { + service_type: ServiceType::BlendNetwork, + locators: vec![Locator::new_unchecked(invalid_locator)], + provider_id: ProviderId(Ed25519Key::from_bytes(&[1; 32]).public_key()), + zk_id: ZkKey::zero().to_public_key(), + locked_note_id: NoteId(BigUint::from(111u64).into()), + }; + + let encoded = encode_sdp_declare(&op); + let result = decode_sdp_declare(&encoded); + + match result { + Err(nom::Err::Error(e)) => assert_eq!(e.code, ErrorKind::Fail), + _ => panic!("Expected Verify error for invalid locator"), + } + } + #[test] fn test_encode_decode_max_inputs() { let note_id = NoteId(BigUint::from(111u64).into()); diff --git a/core/src/sdp/mod.rs b/core/src/sdp/mod.rs index 9e9b1e3eb..f59e1dfeb 100644 --- a/core/src/sdp/mod.rs +++ b/core/src/sdp/mod.rs @@ -1,11 +1,12 @@ pub mod blend; pub mod locked_notes; +use core::str::FromStr; use std::hash::Hash; use blake2::{Blake2b, Digest as _}; use lb_key_management_system_keys::keys::ZkPublicKey; -use multiaddr::Multiaddr; +use multiaddr::{Multiaddr, Protocol}; use nom::{IResult, Parser as _, bytes::complete::take}; use serde::{Deserialize, Serialize}; use strum::EnumIter; @@ -44,14 +45,19 @@ impl ServiceParameters { } #[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord, Serialize, Deserialize)] -#[serde(transparent)] -pub struct Locator(pub Multiaddr); +#[serde(try_from = "Multiaddr")] +pub struct Locator(Multiaddr); impl Locator { #[must_use] - pub const fn new(addr: Multiaddr) -> Self { + pub const fn new_unchecked(addr: Multiaddr) -> Self { Self(addr) } + + #[must_use] + pub fn into_inner(self) -> Multiaddr { + self.0 + } } impl AsRef for Locator { @@ -60,6 +66,47 @@ impl AsRef for Locator { } } +impl TryFrom for Locator { + type Error = String; + + fn try_from(value: Multiaddr) -> Result { + for protocol in &value { + match protocol { + Protocol::Ip4(ip) if ip.is_unspecified() => { + return Err(format!( + "Locator multiaddr must not contain an unspecified IPv4 address: {value}" + )); + } + Protocol::Ip6(ip) if ip.is_unspecified() => { + return Err(format!( + "Locator multiaddr must not contain an unspecified IPv6 address: {value}" + )); + } + Protocol::P2p(_) => { + return Err(format!( + "Locator multiaddr must not contain a peer ID: {value}" + )); + } + _ => {} + } + } + + Ok(Self(value)) + } +} + +impl FromStr for Locator { + type Err = String; + + fn from_str(s: &str) -> Result { + let multiaddr = s + .parse::() + .map_err(|e| format!("Invalid multiaddr: {e}"))?; + println!("Multiaddr: {multiaddr}"); + Self::try_from(multiaddr) + } +} + #[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, Serialize, Deserialize, EnumIter)] pub enum ServiceType { #[serde(rename = "BN")] @@ -266,4 +313,38 @@ mod tests { .contains("Unknown metadata type") ); } + + #[test] + fn locator_rejects_multiaddr_with_peer_id() { + assert!("/ip4/65.109.51.37/udp/3000/quic-v1/p2p/12D3KooWL7a8LBbLRYnabptHPFBCmAs49Y7cVMqvzuSdd43tAJk8".parse::().unwrap_err().contains("must not contain a peer ID")); + } + + #[test] + fn locator_rejects_multiaddr_with_unspecified_ipv4() { + assert!( + "/ip4/0.0.0.0/udp/3000/quic-v1" + .parse::() + .unwrap_err() + .contains("must not contain an unspecified IPv4 address") + ); + } + + #[test] + fn locator_rejects_multiaddr_with_unspecified_ipv6() { + assert!( + "/ip6/::/udp/3000/quic-v1" + .parse::() + .unwrap_err() + .contains("must not contain an unspecified IPv6 address") + ); + } + + #[test] + fn locator_accepts_specific_ip_without_peer_id() { + let addr: Multiaddr = "/ip4/127.0.0.1/udp/3000/quic-v1".parse().unwrap(); + + let result = Locator::try_from(addr.clone()).unwrap(); + + assert_eq!(result.into_inner(), addr); + } } diff --git a/services/blend/src/membership/service.rs b/services/blend/src/membership/service.rs index 409d1612e..ed0f8b2df 100644 --- a/services/blend/src/membership/service.rs +++ b/services/blend/src/membership/service.rs @@ -157,7 +157,7 @@ where NodeId: node_id::TryFrom, { let provider_id = provider_id.0.as_bytes(); - let address = locators.first()?.0.clone(); + let address = locators.first()?.clone(); let id = NodeId::try_from_provider_id(provider_id) .map_err(|e| { warn!("Failed to decode provider_id to node ID: {e:?}"); @@ -171,7 +171,7 @@ where Some(ZkNode { node: Node { id, - address, + address: address.into_inner(), public_key, }, zk_key: *zk_id, diff --git a/testnet/cfgsync/src/config/mod.rs b/testnet/cfgsync/src/config/mod.rs index eca8e5563..15e4a847a 100644 --- a/testnet/cfgsync/src/config/mod.rs +++ b/testnet/cfgsync/src/config/mod.rs @@ -234,7 +234,7 @@ fn create_providers( service_type: ServiceType::BlendNetwork, provider_sk: private_key.clone(), zk_sk: secret_zk_key.clone(), - locator: Locator( + locator: Locator::new_unchecked( Multiaddr::from_str(&format!( "/ip4/{}/udp/{}{}", hosts[i].ip, hosts[i].blend_port, LIBP2P_QUIC_PROTOCOL_SUFFIX diff --git a/tests/src/tests/mantle/sdp/ops.rs b/tests/src/tests/mantle/sdp/ops.rs index 9a3c08a05..d8eea653b 100644 --- a/tests/src/tests/mantle/sdp/ops.rs +++ b/tests/src/tests/mantle/sdp/ops.rs @@ -13,7 +13,7 @@ use lb_core::{ tx::{GasPrices, MantleTxGasContext}, tx_builder::MantleTxBuilder, }, - sdp::{Declaration, DeclarationMessage, Locator, ServiceType, WithdrawMessage}, + sdp::{Declaration, DeclarationMessage, ServiceType, WithdrawMessage}, }; use lb_key_management_system_service::keys::{Ed25519Key, Ed25519Signature, ZkKey}; use lb_node::config::RunConfig; @@ -85,11 +85,9 @@ async fn sdp_ops_e2e() { let provider_signing_key = Ed25519Key::from_bytes(&[7u8; 32]); let provider_zk_key = ZkKey::from(BigUint::from(7u64)); let zk_id = provider_zk_key.to_public_key(); - let locator = Locator( - "/ip4/127.0.0.1/tcp/9100" - .parse() - .expect("Valid locator multiaddr"), - ); + let locator = "/ip4/127.0.0.1/tcp/9100" + .parse() + .expect("Valid locator multiaddr"); let declaration = DeclarationMessage { service_type: ServiceType::BlendNetwork, diff --git a/tests/src/tests/zone_sdk/e2e.rs b/tests/src/tests/zone_sdk/e2e.rs index 6a8cf4ab7..dffef6036 100644 --- a/tests/src/tests/zone_sdk/e2e.rs +++ b/tests/src/tests/zone_sdk/e2e.rs @@ -1705,7 +1705,9 @@ fn add_extra_funding_notes_to_genesis( service_type: ServiceType::BlendNetwork, provider_sk: provider_sk.clone(), zk_sk: zk_sk.clone(), - locator: Locator(blend_config.core.backend.listening_address.clone()), + locator: Locator::new_unchecked( + blend_config.core.backend.listening_address.clone(), + ), note: config.consensus_config.blend_note.clone(), } }) diff --git a/tests/src/topology/mod.rs b/tests/src/topology/mod.rs index 37041d401..8ae5eb5e1 100644 --- a/tests/src/topology/mod.rs +++ b/tests/src/topology/mod.rs @@ -163,7 +163,9 @@ impl Topology { service_type: ServiceType::BlendNetwork, provider_sk: private_key.clone(), zk_sk: zk_secret_key.clone(), - locator: Locator(blend_conf.core.backend.listening_address.clone()), + locator: Locator::new_unchecked( + blend_conf.core.backend.listening_address.clone(), + ), note: consensus_configs[i].blend_note.clone(), }, ) diff --git a/tests/testing_framework/src/framework/deployment_artifacts.rs b/tests/testing_framework/src/framework/deployment_artifacts.rs index 10f25b649..7dbc831a0 100644 --- a/tests/testing_framework/src/framework/deployment_artifacts.rs +++ b/tests/testing_framework/src/framework/deployment_artifacts.rs @@ -161,5 +161,5 @@ fn runtime_blend_locator(hostname: &str, port: u16) -> Locator { multiaddr.push(Protocol::Udp(port)); multiaddr.push(Protocol::QuicV1); - Locator::new(multiaddr) + Locator::new_unchecked(multiaddr) } diff --git a/tests/testing_framework/src/node/configs/postprocess.rs b/tests/testing_framework/src/node/configs/postprocess.rs index fdccfc6ad..1b37aafbe 100644 --- a/tests/testing_framework/src/node/configs/postprocess.rs +++ b/tests/testing_framework/src/node/configs/postprocess.rs @@ -67,7 +67,7 @@ pub fn apply_wallet_genesis_overrides( service_type: ServiceType::BlendNetwork, provider_sk: private_key.clone(), zk_sk: secret_zk_key.clone(), - locator: Locator(blend_conf.core.backend.listening_address.clone()), + locator: Locator::new_unchecked(blend_conf.core.backend.listening_address.clone()), note: general_configs[idx].consensus_config.blend_note.clone(), }); } diff --git a/tools/config/src/lib.rs b/tools/config/src/lib.rs index 441a9b535..8ae21d25c 100644 --- a/tools/config/src/lib.rs +++ b/tools/config/src/lib.rs @@ -142,7 +142,7 @@ pub fn create_general_configs_from_ids( service_type: ServiceType::BlendNetwork, provider_sk: private_key.clone(), zk_sk: secret_zk_key.clone(), - locator: Locator(blend_conf.core.backend.listening_address.clone()), + locator: Locator::new_unchecked(blend_conf.core.backend.listening_address.clone()), note: consensus_configs[i].blend_note.clone(), }, )