From e761313fef0848d2c32adad3cd2caf851c22a92a Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Wed, 7 Sep 2022 14:21:37 +0200 Subject: [PATCH 1/5] Update the experimental multiaddr WebRTC support --- src/libp2p/multiaddr.rs | 60 ++++++++++++++++++++++++----------------- 1 file changed, 35 insertions(+), 25 deletions(-) diff --git a/src/libp2p/multiaddr.rs b/src/libp2p/multiaddr.rs index 4bd63e36a7..b398c14a28 100644 --- a/src/libp2p/multiaddr.rs +++ b/src/libp2p/multiaddr.rs @@ -197,7 +197,7 @@ pub enum ProtocolRef<'a> { DnsAddr(DomainNameRef<'a>), Ip4([u8; 4]), Ip6([u8; 16]), - P2p(Cow<'a, [u8]>), // TODO: a bit hacky + P2p(Cow<'a, [u8]>), // TODO: a bit hacky because there's no "owned" equivalent to MultihashRef Quic, Tcp(u16), Tls, @@ -207,11 +207,9 @@ pub enum ProtocolRef<'a> { Wss, // TODO: unclear what the payload is; see https://github.com/multiformats/multiaddr/issues/127 Memory(u64), - // TODO: these protocols are inventions for prototyping, see https://github.com/libp2p/specs/pull/412 - ExperimentalWebRtc, - // TODO: these protocols are inventions for prototyping, see https://github.com/libp2p/specs/pull/412 - /// Contains the SHA-256 hash of the TLS certificate. - ExperimentalCertHash([u8; 32]), + WebRtc, + /// Contains the multihash of the TLS certificate. + CertHash(Cow<'a, [u8]>), // TODO: a bit hacky because there's no "owned" equivalent to MultihashRef } impl<'a> ProtocolRef<'a> { @@ -279,13 +277,16 @@ impl<'a> ProtocolRef<'a> { .map_err(|_| ParseError::InvalidMemoryPayload)?, )) } - "experimental-webrtc" => Ok(ProtocolRef::ExperimentalWebRtc), - "experimental-certhash" => { - let string_hash = iter.next().ok_or(ParseError::UnexpectedEof)?; - let parsed = hex::decode(string_hash).map_err(|_| ParseError::InvalidDomainName)?; // TODO: wrong error; proper errors should be added to ParseError when it's no longer experimental - let correct_len = - <[u8; 32]>::try_from(&parsed[..]).map_err(|_| ParseError::InvalidDomainName)?; // TODO: wrong error - Ok(ProtocolRef::ExperimentalCertHash(correct_len)) + "webrtc" => Ok(ProtocolRef::WebRtc), + "certhash" => { + let s = iter.next().ok_or(ParseError::UnexpectedEof)?; + let decoded = bs58::decode(s) + .into_vec() + .map_err(|_| ParseError::NotBase58)?; + if let Err(err) = multihash::MultihashRef::from_bytes(&decoded) { + return Err(ParseError::InvalidMultihash(err)); + } + Ok(ProtocolRef::CertHash(Cow::Owned(decoded))) } _ => Err(ParseError::UnrecognizedProtocol), } @@ -309,8 +310,8 @@ impl<'a> ProtocolRef<'a> { ProtocolRef::Ws => 477, ProtocolRef::Wss => 478, ProtocolRef::Memory(_) => 777, - ProtocolRef::ExperimentalWebRtc => 991234, // TODO: these numbers are complete invention - ProtocolRef::ExperimentalCertHash(_) => 991235, // TODO: these numbers are complete invention + ProtocolRef::WebRtc => 280, + ProtocolRef::CertHash(_) => 466, }; // TODO: optimize by not allocating a Vec @@ -335,7 +336,13 @@ impl<'a> ProtocolRef<'a> { } ProtocolRef::Tcp(port) | ProtocolRef::Udp(port) => port.to_be_bytes().to_vec(), ProtocolRef::Memory(payload) => payload.to_be_bytes().to_vec(), - ProtocolRef::ExperimentalCertHash(hash) => hash.to_vec(), + ProtocolRef::CertHash(multihash) => { + // TODO: what if not a valid multihash? the enum variant can be constructed by the user + let mut out = Vec::with_capacity(multihash.len() + 4); + out.extend(crate::util::leb128::encode_usize(multihash.len())); + out.extend_from_slice(multihash); + out + } _ => Vec::new(), }; @@ -368,9 +375,10 @@ impl<'a> fmt::Display for ProtocolRef<'a> { ProtocolRef::Ws => write!(f, "/ws"), ProtocolRef::Wss => write!(f, "/wss"), ProtocolRef::Memory(payload) => write!(f, "/memory/{}", payload), - ProtocolRef::ExperimentalWebRtc => write!(f, "/experimental-webrtc"), - ProtocolRef::ExperimentalCertHash(hash) => { - write!(f, "/experimental-certhash/{}", hex::encode(hash)) + ProtocolRef::WebRtc => write!(f, "/webrtc"), + ProtocolRef::CertHash(multihash) => { + // Base58 encoding doesn't have `/` in its characters set. + write!(f, "/certhash/{}", bs58::encode(multihash).into_string()) } } } @@ -507,12 +515,14 @@ fn protocol<'a, E: nom::error::ParseError<&'a [u8]>>( 478 => Ok((bytes, ProtocolRef::Wss)), // TODO: unclear what the /memory payload is, see https://github.com/multiformats/multiaddr/issues/127 777 => nom::combinator::map(nom::number::complete::be_u64, ProtocolRef::Memory)(bytes), - 991234 => Ok((bytes, ProtocolRef::ExperimentalWebRtc)), // TODO: these numbers are complete invention - 991235 => { - nom::combinator::map(nom::bytes::complete::take(32_u32), |hash: &'a [u8]| { - ProtocolRef::ExperimentalCertHash(<[u8; 32]>::try_from(hash).unwrap()) - })(bytes) - } // TODO: these numbers are complete invention + 280 => Ok((bytes, ProtocolRef::WebRtc)), + 466 => nom::combinator::map( + nom::combinator::verify( + nom::multi::length_data(crate::util::leb128::nom_leb128_usize), + |s| multihash::MultihashRef::from_bytes(s).is_ok(), + ), + |b| ProtocolRef::CertHash(Cow::Borrowed(b)), + )(bytes), _ => Err(nom::Err::Error(nom::error::make_error( bytes, nom::error::ErrorKind::Tag, From a0fd5315e0890c57a2374bdefd293484946155ad Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Thu, 8 Sep 2022 10:45:51 +0200 Subject: [PATCH 2/5] Renames --- src/libp2p/multiaddr.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/libp2p/multiaddr.rs b/src/libp2p/multiaddr.rs index b398c14a28..44430c8942 100644 --- a/src/libp2p/multiaddr.rs +++ b/src/libp2p/multiaddr.rs @@ -207,9 +207,9 @@ pub enum ProtocolRef<'a> { Wss, // TODO: unclear what the payload is; see https://github.com/multiformats/multiaddr/issues/127 Memory(u64), - WebRtc, + WebRTC, /// Contains the multihash of the TLS certificate. - CertHash(Cow<'a, [u8]>), // TODO: a bit hacky because there's no "owned" equivalent to MultihashRef + Certhash(Cow<'a, [u8]>), // TODO: a bit hacky because there's no "owned" equivalent to MultihashRef } impl<'a> ProtocolRef<'a> { @@ -277,7 +277,7 @@ impl<'a> ProtocolRef<'a> { .map_err(|_| ParseError::InvalidMemoryPayload)?, )) } - "webrtc" => Ok(ProtocolRef::WebRtc), + "webrtc" => Ok(ProtocolRef::WebRTC), "certhash" => { let s = iter.next().ok_or(ParseError::UnexpectedEof)?; let decoded = bs58::decode(s) @@ -286,7 +286,7 @@ impl<'a> ProtocolRef<'a> { if let Err(err) = multihash::MultihashRef::from_bytes(&decoded) { return Err(ParseError::InvalidMultihash(err)); } - Ok(ProtocolRef::CertHash(Cow::Owned(decoded))) + Ok(ProtocolRef::Certhash(Cow::Owned(decoded))) } _ => Err(ParseError::UnrecognizedProtocol), } @@ -310,8 +310,8 @@ impl<'a> ProtocolRef<'a> { ProtocolRef::Ws => 477, ProtocolRef::Wss => 478, ProtocolRef::Memory(_) => 777, - ProtocolRef::WebRtc => 280, - ProtocolRef::CertHash(_) => 466, + ProtocolRef::WebRTC => 280, + ProtocolRef::Certhash(_) => 466, }; // TODO: optimize by not allocating a Vec @@ -336,7 +336,7 @@ impl<'a> ProtocolRef<'a> { } ProtocolRef::Tcp(port) | ProtocolRef::Udp(port) => port.to_be_bytes().to_vec(), ProtocolRef::Memory(payload) => payload.to_be_bytes().to_vec(), - ProtocolRef::CertHash(multihash) => { + ProtocolRef::Certhash(multihash) => { // TODO: what if not a valid multihash? the enum variant can be constructed by the user let mut out = Vec::with_capacity(multihash.len() + 4); out.extend(crate::util::leb128::encode_usize(multihash.len())); @@ -375,8 +375,8 @@ impl<'a> fmt::Display for ProtocolRef<'a> { ProtocolRef::Ws => write!(f, "/ws"), ProtocolRef::Wss => write!(f, "/wss"), ProtocolRef::Memory(payload) => write!(f, "/memory/{}", payload), - ProtocolRef::WebRtc => write!(f, "/webrtc"), - ProtocolRef::CertHash(multihash) => { + ProtocolRef::WebRTC => write!(f, "/webrtc"), + ProtocolRef::Certhash(multihash) => { // Base58 encoding doesn't have `/` in its characters set. write!(f, "/certhash/{}", bs58::encode(multihash).into_string()) } @@ -515,13 +515,13 @@ fn protocol<'a, E: nom::error::ParseError<&'a [u8]>>( 478 => Ok((bytes, ProtocolRef::Wss)), // TODO: unclear what the /memory payload is, see https://github.com/multiformats/multiaddr/issues/127 777 => nom::combinator::map(nom::number::complete::be_u64, ProtocolRef::Memory)(bytes), - 280 => Ok((bytes, ProtocolRef::WebRtc)), + 280 => Ok((bytes, ProtocolRef::WebRTC)), 466 => nom::combinator::map( nom::combinator::verify( nom::multi::length_data(crate::util::leb128::nom_leb128_usize), |s| multihash::MultihashRef::from_bytes(s).is_ok(), ), - |b| ProtocolRef::CertHash(Cow::Borrowed(b)), + |b| ProtocolRef::Certhash(Cow::Borrowed(b)), )(bytes), _ => Err(nom::Err::Error(nom::error::make_error( bytes, From db995095f076ba18258a1d356e016c683b8bf97e Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Thu, 8 Sep 2022 10:56:12 +0200 Subject: [PATCH 3/5] Fix the certhash format --- Cargo.lock | 1 + Cargo.toml | 1 + src/libp2p/multiaddr.rs | 22 +++++++++++++++++----- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9818f4a35b..2d50334dbd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2262,6 +2262,7 @@ dependencies = [ "arrayvec 0.7.2", "async-std", "atomic", + "base64", "bip39", "blake2-rfc", "bs58", diff --git a/Cargo.toml b/Cargo.toml index 463a4e20c8..0ee79fb88a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -45,6 +45,7 @@ std = [ # not, or if things are sketchy, please leave a comment next to it. arrayvec = { version = "0.7.2", default-features = false } atomic = { version = "0.5.1", default-features = false } +base64 = { version = "0.13.0", default-features = false } bip39 = { version = "1.0.1", default-features = false } blake2-rfc = { version = "0.2.18", default-features = false } bs58 = { version = "0.4.0", default-features = false, features = ["alloc"] } diff --git a/src/libp2p/multiaddr.rs b/src/libp2p/multiaddr.rs index 44430c8942..557f5ff28c 100644 --- a/src/libp2p/multiaddr.rs +++ b/src/libp2p/multiaddr.rs @@ -187,6 +187,8 @@ pub enum ParseError { InvalidDomainName, InvalidMultihash(multihash::FromBytesError), InvalidMemoryPayload, + InvalidMultibase, + InvalidBase64, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -280,9 +282,16 @@ impl<'a> ProtocolRef<'a> { "webrtc" => Ok(ProtocolRef::WebRTC), "certhash" => { let s = iter.next().ok_or(ParseError::UnexpectedEof)?; - let decoded = bs58::decode(s) - .into_vec() - .map_err(|_| ParseError::NotBase58)?; + // See + let base64_flavor = match s.as_bytes().first() { + Some(b'm') => base64::STANDARD_NO_PAD, + Some(b'M') => base64::STANDARD, + Some(b'u') => base64::URL_SAFE_NO_PAD, + Some(b'U') => base64::URL_SAFE, + _ => return Err(ParseError::InvalidMultibase), + }; + let decoded = base64::decode_config(&s[1..], base64_flavor) + .map_err(|_| ParseError::InvalidBase64)?; if let Err(err) = multihash::MultihashRef::from_bytes(&decoded) { return Err(ParseError::InvalidMultihash(err)); } @@ -377,8 +386,11 @@ impl<'a> fmt::Display for ProtocolRef<'a> { ProtocolRef::Memory(payload) => write!(f, "/memory/{}", payload), ProtocolRef::WebRTC => write!(f, "/webrtc"), ProtocolRef::Certhash(multihash) => { - // Base58 encoding doesn't have `/` in its characters set. - write!(f, "/certhash/{}", bs58::encode(multihash).into_string()) + write!( + f, + "/certhash/u{}", + base64::encode_config(multihash, base64::URL_SAFE_NO_PAD) + ) } } } From 85f80a5cd6a8e62a797d9c1cb0dd3e30002d01de Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Thu, 8 Sep 2022 10:58:35 +0200 Subject: [PATCH 4/5] Add some tests --- src/libp2p/multiaddr.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/libp2p/multiaddr.rs b/src/libp2p/multiaddr.rs index 557f5ff28c..6ffb8aaa8d 100644 --- a/src/libp2p/multiaddr.rs +++ b/src/libp2p/multiaddr.rs @@ -578,6 +578,8 @@ mod tests { check_valid("/dns6//tcp/55"); check_valid("/dnsaddr/./tcp/55"); check_valid("/memory/1234567890"); + check_valid("/webrtc"); + // TODO: example valid /certhash check_invalid("/"); check_invalid("ip4/1.2.3.4"); @@ -587,5 +589,8 @@ mod tests { check_invalid("/ws/1.2.3.4"); check_invalid("/tcp/65536"); check_invalid("/p2p/blablabla"); + check_invalid("/webrtc/2"); + check_invalid("/certhash"); + check_invalid("/certhash/12D3KooWDpJ7As7BWAwRMfu1VU2WCqNjvq387JEYKDBj4kx6nXTN"); } } From ed128d20516e177ca5c4ff8fa4351312b3d06b1a Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Thu, 8 Sep 2022 11:51:59 +0200 Subject: [PATCH 5/5] Fix missing base64 feature --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 0ee79fb88a..0237914af3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -45,7 +45,7 @@ std = [ # not, or if things are sketchy, please leave a comment next to it. arrayvec = { version = "0.7.2", default-features = false } atomic = { version = "0.5.1", default-features = false } -base64 = { version = "0.13.0", default-features = false } +base64 = { version = "0.13.0", default-features = false, features = ["alloc"] } bip39 = { version = "1.0.1", default-features = false } blake2-rfc = { version = "0.2.18", default-features = false } bs58 = { version = "0.4.0", default-features = false, features = ["alloc"] }