Skip to content

Commit 9337fb6

Browse files
lexnvdmitry-markin
andauthored
identify: Improve identify message decoding (#379)
This PR improves the decoding of the identify messages by checking the following: - if provided, the public key must decode properly and point towards the same peer ID - on failure the identify message is not propagated to the higher levels - empty addresses or addresses corresponding to different peers are removed from the message - added extra debug logs to the identify component ### Testing Done - discovered the whole network of polkadot and kusama using https://github.com/lexnv/subp2p-explorer - all the provided identify messages decoded correctly (including all addresses and pKs) cc @paritytech/networking --------- Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> Co-authored-by: Dmitry Markin <dmitry@markin.tech>
1 parent 2f74c6e commit 9337fb6

File tree

1 file changed

+68
-4
lines changed

1 file changed

+68
-4
lines changed

src/protocol/libp2p/identify.rs

Lines changed: 68 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,9 @@ pub(crate) struct Identify {
172172
// Public key of the local node, filled by `Litep2p`.
173173
public: PublicKey,
174174

175+
/// Local peer ID.
176+
local_peer_id: PeerId,
177+
175178
/// Protocol version.
176179
protocol_version: String,
177180

@@ -191,11 +194,18 @@ pub(crate) struct Identify {
191194
impl Identify {
192195
/// Create new [`Identify`] protocol.
193196
pub(crate) fn new(service: TransportService, config: Config) -> Self {
197+
// The public key is always supplied by litep2p and is the one
198+
// used to identify the local peer. This is a similar story to the
199+
// supported protocols.
200+
let public = config.public.expect("public key to always be supplied by litep2p; qed");
201+
let local_peer_id = public.to_peer_id();
202+
194203
Self {
195204
service,
196205
tx: config.tx_event,
197206
peers: HashMap::new(),
198-
public: config.public.expect("public key to be supplied"),
207+
public,
208+
local_peer_id,
199209
protocol_version: config.protocol_version,
200210
user_agent: config.user_agent.unwrap_or(DEFAULT_AGENT.to_string()),
201211
pending_inbound: FuturesStream::new(),
@@ -313,6 +323,8 @@ impl Identify {
313323
"outbound substream opened"
314324
);
315325

326+
let local_peer_id = self.local_peer_id.clone();
327+
316328
self.pending_outbound.push(Box::pin(async move {
317329
let payload =
318330
match tokio::time::timeout(Duration::from_secs(10), substream.next()).await {
@@ -325,17 +337,69 @@ impl Identify {
325337
Ok(Some(Ok(payload))) => payload,
326338
};
327339

328-
let info = identify_schema::Identify::decode(payload.to_vec().as_slice())?;
340+
let info = identify_schema::Identify::decode(payload.to_vec().as_slice()).map_err(
341+
|err| {
342+
tracing::debug!(target: LOG_TARGET, ?peer, ?err, "peer identified provided undecodable identify response");
343+
err
344+
})?;
329345

330346
tracing::trace!(target: LOG_TARGET, ?peer, ?info, "peer identified");
331347

348+
// The check ensures the provided public key is the same one as the
349+
// one exchanged during the connection establishment.
350+
if let Some(public_key) = &info.public_key {
351+
let public_key = PublicKey::from_protobuf_encoding(&public_key).map_err(|err| {
352+
tracing::debug!(target: LOG_TARGET, ?peer, ?err, "peer identified provided undecodable public key");
353+
err
354+
})?;
355+
356+
if public_key.to_peer_id() != peer {
357+
tracing::debug!(target: LOG_TARGET, ?peer, "peer identified provided invalid public key");
358+
return Err(Error::InvalidData);
359+
}
360+
}
361+
332362
let listen_addresses = info
333363
.listen_addrs
334364
.iter()
335-
.filter_map(|address| Multiaddr::try_from(address.clone()).ok())
365+
.filter_map(|address| {
366+
let address = Multiaddr::try_from(address.clone()).ok()?;
367+
368+
// Ensure the address ends with the provided peer ID and is not empty.
369+
if address.is_empty() {
370+
tracing::debug!(target: LOG_TARGET, ?peer, ?address, "peer identified provided empty listen address");
371+
return None;
372+
}
373+
if let Some(multiaddr::Protocol::P2p(peer_id)) = address.iter().last() {
374+
if peer_id != peer.into() {
375+
tracing::debug!(target: LOG_TARGET, ?peer, ?address, "peer identified provided listen address with incorrect peer ID; discarding the address");
376+
return None;
377+
}
378+
}
379+
380+
Some(address)
381+
})
336382
.collect();
383+
337384
let observed_address =
338-
info.observed_addr.and_then(|address| Multiaddr::try_from(address).ok());
385+
info.observed_addr.and_then(|address| {
386+
let address = Multiaddr::try_from(address).ok()?;
387+
388+
if address.is_empty() {
389+
tracing::debug!(target: LOG_TARGET, ?peer, ?address, "peer identified provided empty observed address");
390+
return None;
391+
}
392+
393+
if let Some(multiaddr::Protocol::P2p(peer_id)) = address.iter().last() {
394+
if peer_id != local_peer_id.into() {
395+
tracing::debug!(target: LOG_TARGET, ?peer, ?address, "peer identified provided observed address with peer ID not matching our peer ID; discarding address");
396+
return None;
397+
}
398+
}
399+
400+
Some(address)
401+
});
402+
339403
let protocol_version = info.protocol_version;
340404
let user_agent = info.agent_version;
341405

0 commit comments

Comments
 (0)