[core] Don't require peer_ids to match expectations

If a connection has already been made to a particular ip/port and
the peer_id is known, or if a non-compact response is returned by
the tracker so the peer_id for a given remote ip/port is 'known',
do *not* close the connection if the actual client at that ip/port
returns a different peer_id.

While the torrent spec requires that these connections be closed,
there are a few reasons which make this fairly pointless.

1) libtorrent randomises these by default. Other clients probably
randomise these too.

2) It's normal for these to change when a client restarts anyway.
Clients may be restarted for any number of reasons.

3) They're not transmitted by DHT, and are not transmitted by
the tracker by default.

4) The onus is on the *connecting* peer to close the connection,
which means the check is only 50% effective anyway. A client
could still join the swarm by making outgoing connections to
other peers.

If someone really wants this enabled, it can be enabled on a
per-torrent basis using the new setting in TorrentSettings.

Probably address #627

src/MonoTorrent.Client/MonoTorrent.Client.Modes/Mode.cs

@@ -221,11 +221,11 @@ protected virtual void HandleHandshakeMessage (PeerId id, HandshakeMessage messa
 
             // If the peer id's don't match, dump the connection. This is due to peers faking usually
             if (!id.Peer.Info.PeerId.Equals (message.PeerId)) {
-                if (Manager.HasMetadata && Manager.Torrent!.IsPrivate) {
-                    // If this is a private torrent we should be careful about peerids. If they don't
-                    // match we should close the connection. I *think* uTorrent doesn't randomise peerids
-                    // for private torrents. It's not documented very well. We may need to relax this check
-                    // if other clients randomize for private torrents.
+                if (Manager.Settings.RequirePeerIdToMatch) {
+                    // Several prominent clients randomise peer ids (at the least, everything based on libtorrent)
+                    // so closing connections when the peer id does not match risks blocking compatibility with many
+                    // clients. Additionally, MonoTorrent has long been configured to default to compact tracker responses
+                    // so the odds of having the peer ID are slim.
                     logger.InfoFormatted (id.Connection, "HandShake.Handle - Invalid peerid. Expected '{0}' but received '{1}'", id.Peer.Info.PeerId, message.PeerId);
                     throw new TorrentException ("Supplied PeerID didn't match the one the tracker gave us");
                 } else {

src/MonoTorrent.Client/MonoTorrent.Client/Settings/TorrentSettings.cs

@@ -74,6 +74,15 @@ public sealed class TorrentSettings : IEquatable<TorrentSettings>
         /// </summary>
         public int MaximumUploadRate { get; }
 
+        /// <summary>
+        /// The BitTorrent specification requires that clients which initiate an outgoing connection to
+        /// a remote peer must close that connection if the remote peer reports a different 'peer_id' than
+        /// it previously reported to the tracker. Several prominant BitTorrent clients/libraries, such as
+        /// libtorrent, randomise their peer id. Additionally, if the announce request requests a compact
+        /// response, the peer id will not be known anyway. Defaults to <see langword="false"/>.
+        /// </summary>
+        public bool RequirePeerIdToMatch { get; }
+
         /// <summary>
         /// The number of peers which can be uploaded to concurrently for this torrent. A value of 0 means unlimited. defaults to 8.
         /// </summary>
@@ -102,7 +111,7 @@ public TorrentSettings ()
 
         }
 
-        internal TorrentSettings (bool allowDht, bool allowInitialSeeding, bool allowPeerExchange, int maximumConnections, int maximumDownloadRate, int maximumUploadRate, int uploadSlots, bool createContainingDirectory)
+        internal TorrentSettings (bool allowDht, bool allowInitialSeeding, bool allowPeerExchange, int maximumConnections, int maximumDownloadRate, int maximumUploadRate, int uploadSlots, bool createContainingDirectory, bool requirePeerIdToMatch)
         {
             AllowDht = allowDht;
             AllowInitialSeeding = allowInitialSeeding;
@@ -111,6 +120,7 @@ internal TorrentSettings (bool allowDht, bool allowInitialSeeding, bool allowPee
             MaximumConnections = maximumConnections;
             MaximumDownloadRate = maximumDownloadRate;
             MaximumUploadRate = maximumUploadRate;
+            RequirePeerIdToMatch = requirePeerIdToMatch;
             UploadSlots = uploadSlots;
         }
 
@@ -127,6 +137,7 @@ public bool Equals (TorrentSettings? other)
                 && MaximumConnections == other.MaximumConnections
                 && MaximumDownloadRate == other.MaximumDownloadRate
                 && MaximumUploadRate == other.MaximumUploadRate
+                && RequirePeerIdToMatch == other.RequirePeerIdToMatch
                 && UploadSlots == other.UploadSlots;
         }
 

src/MonoTorrent.Client/MonoTorrent.Client/Settings/TorrentSettingsBuilder.cs

@@ -88,6 +88,15 @@ public int MaximumUploadRate {
             set => maximumUploadRate = CheckZeroOrPositive (value);
         }
 
+        /// <summary>
+        /// The BitTorrent specification requires that clients which initiate an outgoing connection to
+        /// a remote peer must close that connection if the remote peer reports a different 'peer_id' than
+        /// it previously reported to the tracker. Several prominant BitTorrent clients/libraries, such as
+        /// libtorrent, randomise their peer id. Additionally, if the announce request requests a compact
+        /// response, the peer id will not be known anyway. Defaults to <see langword="false"/>.
+        /// </summary>
+        public bool RequirePeerIdToMatch { get; set; }
+
         /// <summary>
         /// The number of peers which can be uploaded to concurrently for this torrent. A value of 0 means unlimited. defaults to 8.
         /// </summary>
@@ -111,6 +120,7 @@ public TorrentSettingsBuilder (TorrentSettings settings)
             MaximumConnections = settings.MaximumConnections;
             MaximumDownloadRate = settings.MaximumDownloadRate;
             MaximumUploadRate = settings.MaximumUploadRate;
+            RequirePeerIdToMatch = settings.RequirePeerIdToMatch;
             UploadSlots = settings.UploadSlots;
         }
 
@@ -124,6 +134,7 @@ public TorrentSettings ToSettings ()
                 maximumConnections: MaximumConnections,
                 maximumDownloadRate: MaximumDownloadRate,
                 maximumUploadRate: MaximumUploadRate,
+                requirePeerIdToMatch: RequirePeerIdToMatch,
                 uploadSlots: UploadSlots
             );
         }

src/Tests/Tests.MonoTorrent.Client/MonoTorrent.Client.Modes/DownloadModeTests.cs

@@ -311,11 +311,15 @@ public void EmptyPeerId_PublicTorrent ()
         }
 
         [Test]
-        public async Task MismatchedPeerId_PrivateTorrent ()
+        public async Task MismatchedPeerId_RequiredToMatch ()
         {
             var torrent = TestRig.CreatePrivate ();
             using var engine = new ClientEngine (EngineSettingsBuilder.CreateForTests ());
-            var manager = await engine.AddAsync (torrent, "");
+
+            var settings = new TorrentSettingsBuilder {
+                RequirePeerIdToMatch = true,
+            }.ToSettings ();
+            var manager = await engine.AddAsync (torrent, "", settings);
 
             manager.Mode = new DownloadMode (manager, DiskManager, ConnectionManager, Settings);
             var peer = PeerId.CreateNull (manager.Bitfield.Length, Manager.InfoHashes.V1OrV2);
@@ -324,6 +328,23 @@ public async Task MismatchedPeerId_PrivateTorrent ()
             Assert.Throws<TorrentException> (() => manager.Mode.HandleMessage (peer, handshake, default));
         }
 
+        [Test]
+        public async Task MismatchedPeerId_PrivateTorrent ()
+        {
+            var torrent = TestRig.CreatePrivate ();
+            using var engine = new ClientEngine (EngineSettingsBuilder.CreateForTests ());
+            Manager = await engine.AddAsync (torrent, "");
+
+            Manager.Mode = new DownloadMode (Manager, DiskManager, ConnectionManager, Settings);
+            var peer = PeerId.CreateNull (Manager.Bitfield.Length, Manager.InfoHashes.V1OrV2);
+
+            var actualPeerId = new BEncodedString (Enumerable.Repeat ('c', 20).ToArray ());
+            var handshake = new HandshakeMessage (Manager.InfoHashes.V1OrV2, actualPeerId, Constants.ProtocolStringV100, false);
+
+            Assert.DoesNotThrow (() => Manager.Mode.HandleMessage (peer, handshake, default));
+            Assert.AreEqual (peer.PeerID, handshake.PeerId);
+        }
+
         [Test]
         public void MismatchedPeerId_PublicTorrent ()
         {