From 86566f9431dde61890ef8a77da6373155de070a0 Mon Sep 17 00:00:00 2001 From: alienx5499 Date: Fri, 28 Nov 2025 23:54:08 +0530 Subject: [PATCH 1/3] Fix signedPeerRecord validation in IdentifyMessageProcessor to prevent address injection attacks (Issue #332) --- .../identify/identify_msg_processor.hpp | 12 +++ .../identify/identify_msg_processor.cpp | 73 ++++++++++++++++++- src/protocol/identify/protobuf/identify.proto | 4 + 3 files changed, 87 insertions(+), 2 deletions(-) diff --git a/include/libp2p/protocol/identify/identify_msg_processor.hpp b/include/libp2p/protocol/identify/identify_msg_processor.hpp index c9689b936..307fc36ec 100644 --- a/include/libp2p/protocol/identify/identify_msg_processor.hpp +++ b/include/libp2p/protocol/identify/identify_msg_processor.hpp @@ -130,6 +130,18 @@ namespace libp2p::protocol { void consumeListenAddresses(std::span addresses_strings, const peer::PeerId &peer_id); + /** + * Validate and consume signed peer record from Identify message + * @param signed_peer_record_bytes - raw bytes of the signed peer record envelope + * @param peer_id - ID of the peer that sent the message + * @param stream - stream over which the message was received + * @return validated addresses from the signed peer record, or empty if validation fails + */ + std::vector consumeSignedPeerRecord( + const std::string &signed_peer_record_bytes, + const peer::PeerId &peer_id, + const StreamSPtr &stream); + Host &host_; network::ConnectionManager &conn_manager_; peer::IdentityManager &identity_manager_; diff --git a/src/protocol/identify/identify_msg_processor.cpp b/src/protocol/identify/identify_msg_processor.cpp index 64a141ab2..bdf6808f0 100644 --- a/src/protocol/identify/identify_msg_processor.cpp +++ b/src/protocol/identify/identify_msg_processor.cpp @@ -201,10 +201,38 @@ namespace libp2p::protocol { consumeObservedAddresses(msg.observedaddr(), peer_id, stream); } + // Check for signedPeerRecord first - if present and valid, use its addresses + // Otherwise fall back to listenAddrs std::vector addresses; - for (const auto &addr : msg.listenaddrs()) { - addresses.push_back(addr); + bool has_valid_signed_record = false; + + if (msg.has_signedpeerrecord()) { + auto signed_record_addresses = + consumeSignedPeerRecord(msg.signedpeerrecord(), peer_id, stream); + if (!signed_record_addresses.empty()) { + // Valid signed peer record found - use its addresses + has_valid_signed_record = true; + for (const auto &addr : signed_record_addresses) { + addresses.push_back(fromMultiaddrToString(addr)); + } + } else { + // signedPeerRecord was present but validation failed + log_->warn("peer {} sent invalid signedPeerRecord, rejecting addresses", + peer_id.toBase58()); + // Don't accept addresses from listenAddrs either if signedPeerRecord + // validation failed - this prevents address injection attacks + signal_identify_received_(peer_id); + return; + } + } + + // If no signedPeerRecord or it was missing, use listenAddrs + if (!has_valid_signed_record) { + for (const auto &addr : msg.listenaddrs()) { + addresses.push_back(addr); + } } + consumeListenAddresses(addresses, peer_id); signal_identify_received_(peer_id); @@ -397,4 +425,45 @@ namespace libp2p::protocol { upsert_res.error()); } } + + std::vector + IdentifyMessageProcessor::consumeSignedPeerRecord( + const std::string &signed_peer_record_bytes, + const peer::PeerId &peer_id, + const StreamSPtr &stream) { + // Security fix: Validate signedPeerRecord to prevent address injection + // attacks. According to libp2p spec, a signed peer record envelope must: + // 1. Have a valid envelope signature + // 2. Contain a public key that derives a PeerId equal to remotePeerId + // 3. Contain a PeerRecord with peerId matching the derived PeerId + // Only if all checks pass should we accept the certified addresses. + + if (signed_peer_record_bytes.empty()) { + log_->warn("peer {} sent empty signedPeerRecord", peer_id.toBase58()); + return {}; + } + + auto stream_peer_id_res = stream->remotePeerId(); + if (!stream_peer_id_res) { + log_->warn("cannot validate signedPeerRecord: no remote peer ID in stream"); + return {}; + } + auto remote_peer_id = stream_peer_id_res.value(); + + // TODO: Implement full peer record envelope parsing and validation + // according to libp2p peer record specification. + // For now, we reject all signedPeerRecords to prevent the vulnerability + // where malicious peers could inject third-party signed records. + // This is a security fix to prevent address poisoning attacks. + + log_->warn( + "peer {} sent signedPeerRecord, but full validation is not yet " + "implemented. Rejecting to prevent address injection attacks. " + "Full peer record envelope parsing needs to be implemented.", + peer_id.toBase58()); + + // Return empty vector to indicate validation failed + // The caller will reject addresses if validation fails + return {}; + } } // namespace libp2p::protocol diff --git a/src/protocol/identify/protobuf/identify.proto b/src/protocol/identify/protobuf/identify.proto index 5b3116a0b..beb28722d 100644 --- a/src/protocol/identify/protobuf/identify.proto +++ b/src/protocol/identify/protobuf/identify.proto @@ -35,6 +35,10 @@ message Identify { // protocols are the services this node is running repeated string protocols = 3; + // signedPeerRecord is a signed peer record envelope containing certified + // addresses. If present, must be validated before accepting addresses. + optional bytes signedPeerRecord = 8; + // a delta update is incompatible with everything else. If this field is // included, none of the others can appear. optional Delta delta = 7; From 90f5d6bdc13ce54e13aa4499ca8aa37f2af98227 Mon Sep 17 00:00:00 2001 From: alienx5499 Date: Mon, 1 Dec 2025 11:48:26 +0530 Subject: [PATCH 2/3] Fix clang-tidy unused variable warning in consumeSignedPeerRecord --- src/protocol/identify/identify_msg_processor.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/protocol/identify/identify_msg_processor.cpp b/src/protocol/identify/identify_msg_processor.cpp index bdf6808f0..cd31e62f8 100644 --- a/src/protocol/identify/identify_msg_processor.cpp +++ b/src/protocol/identify/identify_msg_processor.cpp @@ -448,13 +448,14 @@ namespace libp2p::protocol { log_->warn("cannot validate signedPeerRecord: no remote peer ID in stream"); return {}; } - auto remote_peer_id = stream_peer_id_res.value(); + [[maybe_unused]] auto remote_peer_id = stream_peer_id_res.value(); // TODO: Implement full peer record envelope parsing and validation // according to libp2p peer record specification. // For now, we reject all signedPeerRecords to prevent the vulnerability // where malicious peers could inject third-party signed records. // This is a security fix to prevent address poisoning attacks. + // remote_peer_id will be used when full validation is implemented. log_->warn( "peer {} sent signedPeerRecord, but full validation is not yet " From 8ce5b18a14a0c5d3ec208b3a85558a670cc39b23 Mon Sep 17 00:00:00 2001 From: alienx5499 Date: Mon, 1 Dec 2025 13:17:29 +0530 Subject: [PATCH 3/3] Fix clang-tidy TODO style for signed peer record validation --- src/protocol/identify/identify_msg_processor.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/protocol/identify/identify_msg_processor.cpp b/src/protocol/identify/identify_msg_processor.cpp index cd31e62f8..b9fbcb7b2 100644 --- a/src/protocol/identify/identify_msg_processor.cpp +++ b/src/protocol/identify/identify_msg_processor.cpp @@ -450,8 +450,8 @@ namespace libp2p::protocol { } [[maybe_unused]] auto remote_peer_id = stream_peer_id_res.value(); - // TODO: Implement full peer record envelope parsing and validation - // according to libp2p peer record specification. + // TODO(identify-signed-peer-record): Implement full peer record envelope + // parsing and validation according to libp2p peer record specification. // For now, we reject all signedPeerRecords to prevent the vulnerability // where malicious peers could inject third-party signed records. // This is a security fix to prevent address poisoning attacks.