p2p: add more bad ports

jlopp commented at 5:16 pm on June 29, 2025: contributor

Add a few more ports used by extremely well adopted services that require authentication and really ought not be used by bitcoin nodes for p2p traffic.

DrahtBot commented at 5:16 pm on June 29, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32826.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	Sjors, l0rinc, glozow
Stale ACK	kevkevinpal

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

jlopp renamed this:
~~add more bad p2p ports~~
p2p: add more bad ports
on Jun 29, 2025

DrahtBot added the label P2P on Jun 29, 2025

in src/test/netbase_tests.cpp:607 in 23a7807ec6 outdated

602@@ -603,14 +603,14 @@ BOOST_AUTO_TEST_CASE(isbadport)
603     BOOST_CHECK(!IsBadPort(443));
604     BOOST_CHECK(!IsBadPort(8333));
605 
606-    // Check all ports, there must be 80 bad ports in total.
607+    // Check all ports, there must be 85 bad ports in total.

l0rinc commented at 6:39 pm on June 29, 2025:

instead of updating the comment to match the code (which is the ultimate source of truth anyway), seems like a good opportunity to remove the duplication of the code below - which is already trivial, no need for a dead comment that doesn’t add any new info.

in src/netbase.cpp:924 in 23a7807ec6 outdated

920@@ -921,10 +921,14 @@ bool IsBadPort(uint16_t port)
921     case 1720:  // h323hostcall
922     case 1723:  // pptp
923     case 2049:  // nfs
924+    case 3306:  // MySQL

l0rinc commented at 6:41 pm on June 29, 2025:

👍 https://dev.mysql.com/doc/mysql-port-reference/en/mysql-port-reference-tables.html#GUID-65C1FF7E-5357-4E58-8D68-A0C3D24C0832__MYSQL-CLIENT-SERVER-PORTS

in src/netbase.cpp:925 in 23a7807ec6 outdated

920@@ -921,10 +921,14 @@ bool IsBadPort(uint16_t port)
921     case 1720:  // h323hostcall
922     case 1723:  // pptp
923     case 2049:  // nfs
924+    case 3306:  // MySQL
925+    case 3389:  // RDP / Windows Remote Desktop

l0rinc commented at 6:42 pm on June 29, 2025:

👍 https://www.datacentreplus.co.uk/support/knowledge-base/changing-the-rdp-port-on-a-windows-server

in src/netbase.cpp:930 in 23a7807ec6 outdated

925+    case 3389:  // RDP / Windows Remote Desktop
926     case 3659:  // apple-sasl / PasswordServer
927     case 4045:  // lockd
928     case 5060:  // sip
929     case 5061:  // sips
930+    case 5432:  // PostgreSQL

l0rinc commented at 6:43 pm on June 29, 2025:

👍 https://www.postgresql.org/docs/current/app-postgres.html

in src/netbase.cpp:931 in 23a7807ec6 outdated

926     case 3659:  // apple-sasl / PasswordServer
927     case 4045:  // lockd
928     case 5060:  // sip
929     case 5061:  // sips
930+    case 5432:  // PostgreSQL
931+    case 5900:  // VNC

l0rinc commented at 6:45 pm on June 29, 2025:

👍 https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/6/html/deployment_guide/s1-connecting-to-vnc-server#sec-Configuring_the_Firewall_for_VNC

in src/netbase.cpp:941 in 23a7807ec6 outdated

937@@ -934,6 +938,7 @@ bool IsBadPort(uint16_t port)
938     case 6669:  // Alternate IRC
939     case 6697:  // IRC + TLS
940     case 10080: // Amanda
941+    case 27017: // MongoDB

l0rinc commented at 6:45 pm on June 29, 2025:

👍 https://www.mongodb.com/docs/manual/reference/default-mongodb-port

in src/test/netbase_tests.cpp:613 in 23a7807ec6 outdated

610         if (IsBadPort(port)) {
611             ++total_bad_ports;
612         }
613     }
614-    BOOST_CHECK_EQUAL(total_bad_ports, 80);
615+    BOOST_CHECK_EQUAL(total_bad_ports, 85);

l0rinc commented at 6:59 pm on June 29, 2025:

I was wondering if we could do a more functional approach here, since we already have a few count_if instances - not sure it’s better or not, will let you decide, please resolve if you don’t think it’s a good alternative (assuming integer promotion for max + 1 here):

0    using namespace std::ranges;
1    auto ports{views::iota(1, std::numeric_limits<uint16_t>::max() + 1)};
2    BOOST_CHECK_EQUAL(count_if(ports, IsBadPort), 85);

jlopp commented at 7:32 pm on June 29, 2025:

implemented

l0rinc approved

l0rinc commented at 7:05 pm on June 29, 2025: contributor

ACK 23a7807ec6d35d3ef5b65e4abc3b3841e84b702e

Verified that these are indeed the default ports and that the docs match the code. Have a miner test suggestion, LGTM otherwise.

in src/test/netbase_tests.cpp:611 in 8716fd2f4a outdated

614-    BOOST_CHECK_EQUAL(total_bad_ports, 80);
615+    // Check all ports, there must be 85 bad ports in total.
616+    using namespace std::ranges;
617+    std::list<int> ports(std::numeric_limits<uint16_t>::max());
618+    std::iota(ports.begin(), ports.end(), 1);
619+    BOOST_CHECK_EQUAL(count_if(ports, IsBadPort), 85);

l0rinc commented at 7:35 pm on June 29, 2025:

might as well inline it now that it’s only used once

0    BOOST_CHECK_EQUAL(std::ranges::count_if(ports, IsBadPort), 85);

jlopp commented at 8:05 pm on June 29, 2025:

done

in src/test/netbase_tests.cpp:608 in 8716fd2f4a outdated

612-        }
613-    }
614-    BOOST_CHECK_EQUAL(total_bad_ports, 80);
615+    // Check all ports, there must be 85 bad ports in total.
616+    using namespace std::ranges;
617+    std::list<int> ports(std::numeric_limits<uint16_t>::max());

l0rinc commented at 7:41 pm on June 29, 2025:

👍, looks like this contains the end as well, no +1 needed. Checked with BOOST_CHECK_EQUAL(std::ranges::count_if(ports, [&](const int i) { return i == std::numeric_limits<uint16_t>::max(); }), 1); to make sure.

Note though that this is a substantial memory allocation, the previous suggestion was lazy. For the record here’s the complete lazy solution I suggested (with fixed imports):

 0diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp
 1index e3bf482873..0404a45c79 100644
 2--- a/src/test/netbase_tests.cpp
 3+++ b/src/test/netbase_tests.cpp
 4@@ -2,6 +2,12 @@
 5 // Distributed under the MIT software license, see the accompanying
 6 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 7 
 8+#include <algorithm>
 9+#include <limits>
10+#include <numeric>
11+#include <ranges>
12+#include <string>
13+
14 #include <net_permissions.h>
15 #include <netaddress.h>
16 #include <netbase.h>
17@@ -13,12 +19,10 @@
18 #include <util/strencodings.h>
19 #include <util/translation.h>
20 
21-#include <string>
22-#include <numeric>
23-
24 #include <boost/test/unit_test.hpp>
25 
26 using namespace std::literals;
27+using namespace std::ranges;
28 
29 BOOST_FIXTURE_TEST_SUITE(netbase_tests, BasicTestingSetup)
30 
31@@ -604,10 +608,7 @@ BOOST_AUTO_TEST_CASE(isbadport)
32     BOOST_CHECK(!IsBadPort(443));
33     BOOST_CHECK(!IsBadPort(8333));
34 
35-    // Check all ports, there must be 85 bad ports in total.
36-    using namespace std::ranges;
37-    std::list<int> ports(std::numeric_limits<uint16_t>::max());
38-    std::iota(ports.begin(), ports.end(), 1);
39+    constexpr auto ports{views::iota(1, std::numeric_limits<uint16_t>::max() + 1)};
40     BOOST_CHECK_EQUAL(count_if(ports, IsBadPort), 85);
41 }

hodlinator commented at 10:21 pm on August 9, 2025:

Got nerd-sniped by this commit using list for no reason and was about to suggest vector<uint16_t> as a better default. But a view is nicer conceptually and slightly nicer on the cache (memory usage difference seems negligible).

 0₿ poop "./build/bin/test_bitcoin_list -t netbase_tests/isbadport" "./build/bin/test_bitcoin_vector16 -t netbase_tests/isbadport"
 1Benchmark 1 (43 runs): ./build/bin/test_bitcoin_list -t netbase_tests/isbadport
 2  measurement          mean ± σ            min … max           outliers         delta
 3  wall_time           117ms ± 1.45ms     117ms …  125ms          2 ( 5%)        0%
 4  peak_rss           22.6MB ±  154KB    22.3MB … 22.9MB          0 ( 0%)        0%
 5  cpu_cycles          521M  ± 3.63M      510M  …  527M           1 ( 2%)        0%
 6  instructions       1.81G  ± 14.3M     1.77G  … 1.83G           0 ( 0%)        0%
 7  cache_references    957K  ± 28.7K      911K  … 1.01M           0 ( 0%)        0%
 8  cache_misses        184K  ± 8.55K      172K  …  214K           1 ( 2%)        0%
 9  branch_misses       164K  ±  702       163K  …  166K           1 ( 2%)        0%
10Benchmark 2 (44 runs): ./build/bin/test_bitcoin_vector16 -t netbase_tests/isbadport
11  measurement          mean ± σ            min … max           outliers         delta
12  wall_time           115ms ±  181us     114ms …  115ms          0 ( 0%)        ⚡-  2.3% ±  0.4%
13  peak_rss           22.5MB ±  171KB    22.3MB … 23.1MB          1 ( 2%)          -  0.2% ±  0.3%
14  cpu_cycles          513M  ± 2.87M      505M  …  517M           2 ( 5%)        ⚡-  1.6% ±  0.3%
15  instructions       1.80G  ± 15.7M     1.74G  … 1.82G           3 ( 7%)          -  0.6% ±  0.4%
16  cache_references    684K  ± 35.5K      631K  …  771K           0 ( 0%)        ⚡- 28.5% ±  1.4%
17  cache_misses        178K  ± 3.97K      172K  …  190K           1 ( 2%)        ⚡-  3.1% ±  1.5%
18  branch_misses       165K  ±  523       163K  …  166K           1 ( 2%)          +  0.4% ±  0.2%
19
20~/bitcoin
21₿ poop "./build/bin/test_bitcoin_vector16 -t netbase_tests/isbadport" "./build/bin/test_bitcoin_view -t netbase_tests/isbadport"
22Benchmark 1 (44 runs): ./build/bin/test_bitcoin_vector16 -t netbase_tests/isbadport
23  measurement          mean ± σ            min … max           outliers         delta
24  wall_time           115ms ± 2.52ms     114ms …  131ms          3 ( 7%)        0%
25  peak_rss           22.5MB ±  209KB    22.2MB … 23.1MB          4 ( 9%)        0%
26  cpu_cycles          512M  ± 3.19M      503M  …  518M           1 ( 2%)        0%
27  instructions       1.80G  ± 13.7M     1.74G  … 1.81G           2 ( 5%)        0%
28  cache_references    687K  ± 31.7K      633K  …  805K           1 ( 2%)        0%
29  cache_misses        179K  ± 3.83K      174K  …  191K           2 ( 5%)        0%
30  branch_misses       165K  ±  585       164K  …  166K           1 ( 2%)        0%
31Benchmark 2 (44 runs): ./build/bin/test_bitcoin_view -t netbase_tests/isbadport
32  measurement          mean ± σ            min … max           outliers         delta
33  wall_time           115ms ± 1.19ms     114ms …  122ms          4 ( 9%)          -  0.2% ±  0.7%
34  peak_rss           22.5MB ±  241KB    22.1MB … 23.0MB          0 ( 0%)          -  0.1% ±  0.4%
35  cpu_cycles          512M  ± 4.47M      492M  …  518M           5 (11%)          +  0.0% ±  0.3%
36  instructions       1.78G  ± 47.8M     1.49G  … 1.80G           6 (14%)          -  1.0% ±  0.8%
37  cache_references    668K  ± 26.4K      632K  …  726K           0 ( 0%)        ⚡-  2.8% ±  1.8%
38  cache_misses        177K  ± 4.16K      167K  …  187K           3 ( 7%)          -  1.0% ±  0.9%
39  branch_misses       165K  ±  901       164K  …  168K           4 ( 9%)          +  0.1% ±  0.2%

I’ll see myself out. 🚪🏃

l0rinc commented at 7:55 pm on June 29, 2025: contributor

ACK 8716fd2f4a00b07426e54a1491af421d6345f708

kevkevinpal commented at 7:58 pm on June 29, 2025: contributor

ACK 8716fd2

It makes sense to me to add these ports since they’re popular and used. I double-checked the links @l0rinc provided and also did a quick search myself to make sure they were correct

maflcko commented at 6:25 am on June 30, 2025: member

Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

in doc/p2p-bad-ports.md:91 in 8f36e7e5cf outdated

86@@ -87,10 +87,14 @@ incoming connections.
87     1720:  h323hostcall
88     1723:  pptp
89     2049:  nfs
90+    3306:  MySQL
91+    3389:  RDP / Windows Remote Desktop

Sjors commented at 8:43 am on June 30, 2025:

Trying to connect to (or open( that port sounds like a great way to be flagged as malware / attacker.

jlopp force-pushed on Jun 30, 2025

add more bad p2p ports 6967e8e8ab

jlopp force-pushed on Jun 30, 2025

Sjors commented at 10:41 am on June 30, 2025: member

utACK 6967e8e8abbc35ac98e8e3745a8bbed56e77526f

I checked that the ports make sense.

DrahtBot requested review from l0rinc on Jun 30, 2025

l0rinc approved

l0rinc commented at 11:09 am on June 30, 2025: contributor

ACK 6967e8e8abbc35ac98e8e3745a8bbed56e77526f

Changes since my last review are comment updates and namespace inlines.

My only remaining concern is the unnecessary heavy memory usage of the test - but it’s not a blocker.

bitcoin deleted a comment on Jun 30, 2025

glozow commented at 5:21 pm on June 30, 2025: member

ACK 6967e8e8abbc35ac98e8e3745a8bbed56e77526f

glozow merged this on Jun 30, 2025

glozow closed this on Jun 30, 2025

jlopp deleted the branch on Jun 30, 2025

fanquake commented at 3:39 pm on July 4, 2025: member

Backported to 29.x in #32863.

fanquake referenced this in commit 58b1a65ab0 on Jul 4, 2025

luke-jr referenced this in commit 2b9b8b8684 on Jul 17, 2025

glozow referenced this in commit ef380a454c on Jul 18, 2025

stringintech referenced this in commit f19e7819e9 on Jul 19, 2025

alexanderwiederin referenced this in commit 9997c3d993 on Jul 25, 2025

alexanderwiederin referenced this in commit 8d6ebb9c4e on Jul 28, 2025

alexanderwiederin referenced this in commit 533a4585b5 on Jul 28, 2025

p2p: add more bad ports #32826

Code Coverage & Benchmarks

Reviews