Skip to content

Commit 63b13aa

Browse files
kwvgkwvg
committed
merge bitcoin#28525: Drop v2 garbage authentication packet
1 parent 662394c commit 63b13aa

File tree

3 files changed

+70
-103
lines changed

3 files changed

+70
-103
lines changed

‎src/net.cpp‎

Lines changed: 25 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1098,8 +1098,7 @@ void V2Transport::StartSendingHandshake() noexcept
10981098
m_send_buffer.resize(EllSwiftPubKey::size() + m_send_garbage.size());
10991099
std::copy(std::begin(m_cipher.GetOurPubKey()), std::end(m_cipher.GetOurPubKey()), MakeWritableByteSpan(m_send_buffer).begin());
11001100
std::copy(m_send_garbage.begin(), m_send_garbage.end(), m_send_buffer.begin() + EllSwiftPubKey::size());
1101-
// We cannot wipe m_send_garbage as it will still be used to construct the garbage
1102-
// authentication packet.
1101+
// We cannot wipe m_send_garbage as it will still be used as AAD later in the handshake.
11031102
}
11041103

11051104
V2Transport::V2Transport(NodeId nodeid, bool initiating, int type_in, int version_in, const CKey& key, Span<const std::byte> ent32, std::vector<uint8_t> garbage) noexcept :
@@ -1133,9 +1132,6 @@ void V2Transport::SetReceiveState(RecvState recv_state) noexcept
11331132
Assume(recv_state == RecvState::GARB_GARBTERM);
11341133
break;
11351134
case RecvState::GARB_GARBTERM:
1136-
Assume(recv_state == RecvState::GARBAUTH);
1137-
break;
1138-
case RecvState::GARBAUTH:
11391135
Assume(recv_state == RecvState::VERSION);
11401136
break;
11411137
case RecvState::VERSION:
@@ -1267,25 +1263,16 @@ bool V2Transport::ProcessReceivedKeyBytes() noexcept
12671263
m_cipher.GetSendGarbageTerminator().end(),
12681264
MakeWritableByteSpan(m_send_buffer).last(BIP324Cipher::GARBAGE_TERMINATOR_LEN).begin());
12691265

1270-
// Construct garbage authentication packet in the send buffer (using the garbage data which
1271-
// is still there).
1272-
m_send_buffer.resize(m_send_buffer.size() + BIP324Cipher::EXPANSION);
1266+
// Construct version packet in the send buffer, with the sent garbage data as AAD.
1267+
m_send_buffer.resize(m_send_buffer.size() + BIP324Cipher::EXPANSION + VERSION_CONTENTS.size());
12731268
m_cipher.Encrypt(
1274-
/*contents=*/{},
1269+
/*contents=*/VERSION_CONTENTS,
12751270
/*aad=*/MakeByteSpan(m_send_garbage),
12761271
/*ignore=*/false,
1277-
/*output=*/MakeWritableByteSpan(m_send_buffer).last(BIP324Cipher::EXPANSION));
1272+
/*output=*/MakeWritableByteSpan(m_send_buffer).last(BIP324Cipher::EXPANSION + VERSION_CONTENTS.size()));
12781273
// We no longer need the garbage.
12791274
m_send_garbage.clear();
12801275
m_send_garbage.shrink_to_fit();
1281-
1282-
// Construct version packet in the send buffer.
1283-
m_send_buffer.resize(m_send_buffer.size() + BIP324Cipher::EXPANSION + VERSION_CONTENTS.size());
1284-
m_cipher.Encrypt(
1285-
/*contents=*/VERSION_CONTENTS,
1286-
/*aad=*/{},
1287-
/*ignore=*/false,
1288-
/*output=*/MakeWritableByteSpan(m_send_buffer).last(BIP324Cipher::EXPANSION + VERSION_CONTENTS.size()));
12891276
} else {
12901277
// We still have to receive more key bytes.
12911278
}
@@ -1299,11 +1286,11 @@ bool V2Transport::ProcessReceivedGarbageBytes() noexcept
12991286
Assume(m_recv_buffer.size() <= MAX_GARBAGE_LEN + BIP324Cipher::GARBAGE_TERMINATOR_LEN);
13001287
if (m_recv_buffer.size() >= BIP324Cipher::GARBAGE_TERMINATOR_LEN) {
13011288
if (MakeByteSpan(m_recv_buffer).last(BIP324Cipher::GARBAGE_TERMINATOR_LEN) == m_cipher.GetReceiveGarbageTerminator()) {
1302-
// Garbage terminator received. Switch to receiving garbage authentication packet.
1303-
m_recv_garbage = std::move(m_recv_buffer);
1304-
m_recv_garbage.resize(m_recv_garbage.size() - BIP324Cipher::GARBAGE_TERMINATOR_LEN);
1289+
// Garbage terminator received. Store garbage to authenticate it as AAD later.
1290+
m_recv_aad = std::move(m_recv_buffer);
1291+
m_recv_aad.resize(m_recv_aad.size() - BIP324Cipher::GARBAGE_TERMINATOR_LEN);
13051292
m_recv_buffer.clear();
1306-
SetReceiveState(RecvState::GARBAUTH);
1293+
SetReceiveState(RecvState::VERSION);
13071294
} else if (m_recv_buffer.size() == MAX_GARBAGE_LEN + BIP324Cipher::GARBAGE_TERMINATOR_LEN) {
13081295
// We've reached the maximum length for garbage + garbage terminator, and the
13091296
// terminator still does not match. Abort.
@@ -1322,8 +1309,7 @@ bool V2Transport::ProcessReceivedGarbageBytes() noexcept
13221309
bool V2Transport::ProcessReceivedPacketBytes() noexcept
13231310
{
13241311
AssertLockHeld(m_recv_mutex);
1325-
Assume(m_recv_state == RecvState::GARBAUTH || m_recv_state == RecvState::VERSION ||
1326-
m_recv_state == RecvState::APP);
1312+
Assume(m_recv_state == RecvState::VERSION || m_recv_state == RecvState::APP);
13271313

13281314
// The maximum permitted contents length for a packet, consisting of:
13291315
// - 0x00 byte: indicating long message type encoding
@@ -1346,45 +1332,38 @@ bool V2Transport::ProcessReceivedPacketBytes() noexcept
13461332
// as GetMaxBytesToProcess only allows up to LENGTH_LEN into the buffer before that point.
13471333
m_recv_decode_buffer.resize(m_recv_len);
13481334
bool ignore{false};
1349-
Span<const std::byte> aad;
1350-
if (m_recv_state == RecvState::GARBAUTH) aad = MakeByteSpan(m_recv_garbage);
13511335
bool ret = m_cipher.Decrypt(
13521336
/*input=*/MakeByteSpan(m_recv_buffer).subspan(BIP324Cipher::LENGTH_LEN),
1353-
/*aad=*/aad,
1337+
/*aad=*/MakeByteSpan(m_recv_aad),
13541338
/*ignore=*/ignore,
13551339
/*contents=*/MakeWritableByteSpan(m_recv_decode_buffer));
13561340
if (!ret) {
13571341
LogPrint(BCLog::NET, "V2 transport error: packet decryption failure (%u bytes), peer=%d\n", m_recv_len, m_nodeid);
13581342
return false;
13591343
}
1344+
// We have decrypted a valid packet with the AAD we expected, so clear the expected AAD.
1345+
m_recv_aad.clear();
1346+
m_recv_aad.shrink_to_fit();
13601347
// Feed the last 4 bytes of the Poly1305 authentication tag (and its timing) into our RNG.
13611348
RandAddEvent(ReadLE32(m_recv_buffer.data() + m_recv_buffer.size() - 4));
13621349

1363-
// At this point we have a valid packet decrypted into m_recv_decode_buffer. Depending on
1364-
// the current state, decide what to do with it.
1365-
switch (m_recv_state) {
1366-
case RecvState::GARBAUTH:
1367-
// Ignore flag does not matter for garbage authentication. Any valid packet functions
1368-
// as authentication. Receive and process the version packet next.
1369-
SetReceiveState(RecvState::VERSION);
1370-
m_recv_garbage = {};
1371-
break;
1372-
case RecvState::VERSION:
1373-
if (!ignore) {
1350+
// At this point we have a valid packet decrypted into m_recv_decode_buffer. If it's not a
1351+
// decoy, which we simply ignore, use the current state to decide what to do with it.
1352+
if (!ignore) {
1353+
switch (m_recv_state) {
1354+
case RecvState::VERSION:
13741355
// Version message received; transition to application phase. The contents is
13751356
// ignored, but can be used for future extensions.
13761357
SetReceiveState(RecvState::APP);
1377-
}
1378-
break;
1379-
case RecvState::APP:
1380-
if (!ignore) {
1358+
break;
1359+
case RecvState::APP:
13811360
// Application message decrypted correctly. It can be extracted using GetMessage().
13821361
SetReceiveState(RecvState::APP_READY);
1362+
break;
1363+
default:
1364+
// Any other state is invalid (this function should not have been called).
1365+
Assume(false);
13831366
}
1384-
break;
1385-
default:
1386-
// Any other state is invalid (this function should not have been called).
1387-
Assume(false);
13881367
}
13891368
// Wipe the receive buffer where the next packet will be received into.
13901369
m_recv_buffer = {};
@@ -1420,7 +1399,6 @@ size_t V2Transport::GetMaxBytesToProcess() noexcept
14201399
case RecvState::GARB_GARBTERM:
14211400
// Process garbage bytes one by one (because terminator may appear anywhere).
14221401
return 1;
1423-
case RecvState::GARBAUTH:
14241402
case RecvState::VERSION:
14251403
case RecvState::APP:
14261404
// These three states all involve decoding a packet. Process the length descriptor first,
@@ -1474,7 +1452,6 @@ bool V2Transport::ReceivedBytes(Span<const uint8_t>& msg_bytes) noexcept
14741452
// bytes).
14751453
m_recv_buffer.reserve(MAX_GARBAGE_LEN + BIP324Cipher::GARBAGE_TERMINATOR_LEN);
14761454
break;
1477-
case RecvState::GARBAUTH:
14781455
case RecvState::VERSION:
14791456
case RecvState::APP: {
14801457
// During states where a packet is being received, as much as is expected but never
@@ -1518,7 +1495,6 @@ bool V2Transport::ReceivedBytes(Span<const uint8_t>& msg_bytes) noexcept
15181495
if (!ProcessReceivedGarbageBytes()) return false;
15191496
break;
15201497

1521-
case RecvState::GARBAUTH:
15221498
case RecvState::VERSION:
15231499
case RecvState::APP:
15241500
if (!ProcessReceivedPacketBytes()) return false;

‎src/net.h‎

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -496,10 +496,10 @@ class V2Transport final : public Transport
496496
*
497497
* start(responder)
498498
* |
499-
* | start(initiator) /---------\
500-
* | | | |
501-
* v v v |
502-
* KEY_MAYBE_V1 -> KEY -> GARB_GARBTERM -> GARBAUTH -> VERSION -> APP -> APP_READY
499+
* | start(initiator) /---------\
500+
* | | | |
501+
* v v v |
502+
* KEY_MAYBE_V1 -> KEY -> GARB_GARBTERM -> VERSION -> APP -> APP_READY
503503
* |
504504
* \-------> V1
505505
*/
@@ -521,24 +521,19 @@ class V2Transport final : public Transport
521521
/** Garbage and garbage terminator.
522522
*
523523
* Whenever a byte is received, the last 16 bytes are compared with the expected garbage
524-
* terminator. When that happens, the state becomes GARBAUTH. If no matching terminator is
524+
* terminator. When that happens, the state becomes VERSION. If no matching terminator is
525525
* received in 4111 bytes (4095 for the maximum garbage length, and 16 bytes for the
526526
* terminator), the connection aborts. */
527527
GARB_GARBTERM,
528528

529-
/** Garbage authentication packet.
530-
*
531-
* A packet is received, and decrypted/verified with AAD set to the garbage received during
532-
* the GARB_GARBTERM state. If that succeeds, the state becomes VERSION. If it fails the
533-
* connection aborts. */
534-
GARBAUTH,
535-
536529
/** Version packet.
537530
*
538-
* A packet is received, and decrypted/verified. If that succeeds, the state becomes APP,
539-
* and the decrypted contents is interpreted as version negotiation (currently, that means
540-
* ignoring it, but it can be used for negotiating future extensions). If it fails, the
541-
* connection aborts. */
531+
* A packet is received, and decrypted/verified. If that fails, the connection aborts. The
532+
* first received packet in this state (whether it's a decoy or not) is expected to
533+
* authenticate the garbage received during the GARB_GARBTERM state as associated
534+
* authenticated data (AAD). The first non-decoy packet in this state is interpreted as
535+
* version negotiation (currently, that means ignoring the contents, but it can be used for
536+
* negotiating future extensions), and afterwards the state becomes APP. */
542537
VERSION,
543538

544539
/** Application packet.
@@ -592,9 +587,9 @@ class V2Transport final : public Transport
592587
/** Normal sending state.
593588
*
594589
* In this state, the ciphers are initialized, so packets can be sent. When this state is
595-
* entered, the garbage terminator, garbage authentication packet, and version
596-
* packet are appended to the send buffer (in addition to the key and garbage which may
597-
* still be there). In this state a message can be provided if the send buffer is empty. */
590+
* entered, the garbage terminator and version packet are appended to the send buffer (in
591+
* addition to the key and garbage which may still be there). In this state a message can be
592+
* provided if the send buffer is empty. */
598593
READY,
599594

600595
/** This transport is using v1 fallback.
@@ -614,13 +609,13 @@ class V2Transport final : public Transport
614609

615610
/** Lock for receiver-side fields. */
616611
mutable Mutex m_recv_mutex ACQUIRED_BEFORE(m_send_mutex);
617-
/** In {GARBAUTH, VERSION, APP}, the decrypted packet length, if m_recv_buffer.size() >=
612+
/** In {VERSION, APP}, the decrypted packet length, if m_recv_buffer.size() >=
618613
* BIP324Cipher::LENGTH_LEN. Unspecified otherwise. */
619614
uint32_t m_recv_len GUARDED_BY(m_recv_mutex) {0};
620615
/** Receive buffer; meaning is determined by m_recv_state. */
621616
std::vector<uint8_t> m_recv_buffer GUARDED_BY(m_recv_mutex);
622-
/** During GARBAUTH, the garbage received during GARB_GARBTERM. */
623-
std::vector<uint8_t> m_recv_garbage GUARDED_BY(m_recv_mutex);
617+
/** AAD expected in next received packet (currently used only for garbage). */
618+
std::vector<uint8_t> m_recv_aad GUARDED_BY(m_recv_mutex);
624619
/** Buffer to put decrypted contents in, for converting to CNetMessage. */
625620
std::vector<uint8_t> m_recv_decode_buffer GUARDED_BY(m_recv_mutex);
626621
/** Deserialization type. */
@@ -660,7 +655,7 @@ class V2Transport final : public Transport
660655
bool ProcessReceivedKeyBytes() noexcept EXCLUSIVE_LOCKS_REQUIRED(m_recv_mutex, !m_send_mutex);
661656
/** Process bytes in m_recv_buffer, while in GARB_GARBTERM state. */
662657
bool ProcessReceivedGarbageBytes() noexcept EXCLUSIVE_LOCKS_REQUIRED(m_recv_mutex);
663-
/** Process bytes in m_recv_buffer, while in GARBAUTH/VERSION/APP state. */
658+
/** Process bytes in m_recv_buffer, while in VERSION/APP state. */
664659
bool ProcessReceivedPacketBytes() noexcept EXCLUSIVE_LOCKS_REQUIRED(m_recv_mutex);
665660

666661
public:

0 commit comments

Comments
 (0)