Skip to content

Commit db9888f

Browse files
committed
net: detect wrong-network V1 talking to V2Transport
1 parent 91e1ef8 commit db9888f

File tree

3 files changed

+46
-3
lines changed

3 files changed

+46
-3
lines changed

src/net.cpp

+20-2
Original file line numberDiff line numberDiff line change
@@ -1109,12 +1109,29 @@ void V2Transport::ProcessReceivedMaybeV1Bytes() noexcept
11091109
}
11101110
}
11111111

1112-
void V2Transport::ProcessReceivedKeyBytes() noexcept
1112+
bool V2Transport::ProcessReceivedKeyBytes() noexcept
11131113
{
11141114
AssertLockHeld(m_recv_mutex);
11151115
AssertLockNotHeld(m_send_mutex);
11161116
Assume(m_recv_state == RecvState::KEY);
11171117
Assume(m_recv_buffer.size() <= EllSwiftPubKey::size());
1118+
1119+
// As a special exception, if bytes 4-16 of the key on a responder connection match the
1120+
// corresponding bytes of a V1 version message, but bytes 0-4 don't match the network magic
1121+
// (if they did, we'd have switched to V1 state already), assume this is a peer from
1122+
// another network, and disconnect them. They will almost certainly disconnect us too when
1123+
// they receive our uniformly random key and garbage, but detecting this case specially
1124+
// means we can log it.
1125+
static constexpr std::array<uint8_t, 12> MATCH = {'v', 'e', 'r', 's', 'i', 'o', 'n', 0, 0, 0, 0, 0};
1126+
static constexpr size_t OFFSET = sizeof(CMessageHeader::MessageStartChars);
1127+
if (!m_initiating && m_recv_buffer.size() >= OFFSET + MATCH.size()) {
1128+
if (std::equal(MATCH.begin(), MATCH.end(), m_recv_buffer.begin() + OFFSET)) {
1129+
LogPrint(BCLog::NET, "V2 transport error: V1 peer with wrong MessageStart %s\n",
1130+
HexStr(Span(m_recv_buffer).first(OFFSET)));
1131+
return false;
1132+
}
1133+
}
1134+
11181135
if (m_recv_buffer.size() == EllSwiftPubKey::size()) {
11191136
// Other side's key has been fully received, and can now be Diffie-Hellman combined with
11201137
// our key to initialize the encryption ciphers.
@@ -1157,6 +1174,7 @@ void V2Transport::ProcessReceivedKeyBytes() noexcept
11571174
} else {
11581175
// We still have to receive more key bytes.
11591176
}
1177+
return true;
11601178
}
11611179

11621180
bool V2Transport::ProcessReceivedGarbageBytes() noexcept
@@ -1378,7 +1396,7 @@ bool V2Transport::ReceivedBytes(Span<const uint8_t>& msg_bytes) noexcept
13781396
break;
13791397

13801398
case RecvState::KEY:
1381-
ProcessReceivedKeyBytes();
1399+
if (!ProcessReceivedKeyBytes()) return false;
13821400
break;
13831401

13841402
case RecvState::GARB_GARBTERM:

src/net.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -617,7 +617,7 @@ class V2Transport final : public Transport
617617
/** Process bytes in m_recv_buffer, while in KEY_MAYBE_V1 state. */
618618
void ProcessReceivedMaybeV1Bytes() noexcept EXCLUSIVE_LOCKS_REQUIRED(m_recv_mutex, !m_send_mutex);
619619
/** Process bytes in m_recv_buffer, while in KEY state. */
620-
void ProcessReceivedKeyBytes() noexcept EXCLUSIVE_LOCKS_REQUIRED(m_recv_mutex, !m_send_mutex);
620+
bool ProcessReceivedKeyBytes() noexcept EXCLUSIVE_LOCKS_REQUIRED(m_recv_mutex, !m_send_mutex);
621621
/** Process bytes in m_recv_buffer, while in GARB_GARBTERM state. */
622622
bool ProcessReceivedGarbageBytes() noexcept EXCLUSIVE_LOCKS_REQUIRED(m_recv_mutex);
623623
/** Process bytes in m_recv_buffer, while in GARBAUTH/VERSION/APP state. */

src/test/net_tests.cpp

+25
Original file line numberDiff line numberDiff line change
@@ -1104,6 +1104,15 @@ class V2TransportTester
11041104
m_to_send.insert(m_to_send.end(), data.begin(), data.end());
11051105
}
11061106

1107+
/** Send V1 version message header to the transport. */
1108+
void SendV1Version(const CMessageHeader::MessageStartChars& magic)
1109+
{
1110+
CMessageHeader hdr(magic, "version", 126 + InsecureRandRange(11));
1111+
CDataStream ser(SER_NETWORK, CLIENT_VERSION);
1112+
ser << hdr;
1113+
m_to_send.insert(m_to_send.end(), UCharCast(ser.data()), UCharCast(ser.data() + ser.size()));
1114+
}
1115+
11071116
/** Schedule bytes to be sent to the transport. */
11081117
void Send(Span<const std::byte> data) { Send(MakeUCharSpan(data)); }
11091118

@@ -1505,6 +1514,22 @@ BOOST_AUTO_TEST_CASE(v2transport_test)
15051514
BOOST_CHECK((*ret)[1] && (*ret)[1]->m_type == "block" && Span{(*ret)[1]->m_recv} == MakeByteSpan(msg_data_1));
15061515
tester.ReceiveMessage(uint8_t(3), msg_data_2); // "blocktxn" short id
15071516
}
1517+
1518+
// Send correct network's V1 header
1519+
{
1520+
V2TransportTester tester(false);
1521+
tester.SendV1Version(Params().MessageStart());
1522+
auto ret = tester.Interact();
1523+
BOOST_CHECK(ret);
1524+
}
1525+
1526+
// Send wrong network's V1 header
1527+
{
1528+
V2TransportTester tester(false);
1529+
tester.SendV1Version(CChainParams::Main()->MessageStart());
1530+
auto ret = tester.Interact();
1531+
BOOST_CHECK(!ret);
1532+
}
15081533
}
15091534

15101535
BOOST_AUTO_TEST_SUITE_END()

0 commit comments

Comments
 (0)