Fix signedPeerRecord validation in IdentifyMessageProcessor to prevent address injection attacks (Issue #332)

Author: alienx5499

include/libp2p/protocol/identify/identify_msg_processor.hpp

@@ -130,6 +130,18 @@ namespace libp2p::protocol {
     void consumeListenAddresses(std::span<const std::string> 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<multi::Multiaddress> 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_;

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<std::string> 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<multi::Multiaddress>
+  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

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;