ElixirForge/firmware
1
0
Fork 0
mirror of https://github.com/alexhopeoconnor/firmware.git synced 2026-10-03 19:12:00 +10:00
Code Releases Activity

fix(mesh): relay foreign packets whose channel hash collides with a local channel (#11544)

Browse Source 
* fix(mesh): relay foreign packets whose channel hash collides with a local channel

The channel hash is one byte, so a foreign channel's name/PSK can fold to the
same hash as a local channel (~1/256 per local channel held). Since d6b12ea3f,
perhapsDecode returns DECODE_FAILURE whenever any local channel matched the
hash, and passesRoutingAuthGate turned that into REJECT - silently blackholing
legitimate foreign traffic that master and 2.7.x relay. A node with a wrong PSK
for a channel name stopped relaying the real channel entirely.

Channel crypto (AES-CTR) has no authentication tag, so "wrong key, foreign
channel" and "our channel, tampered payload" are indistinguishable at this
decision point. The strict drop bought nothing: an attacker picks a hash
matching no local channel and gets DECODE_OPAQUE relay anyway (test_C6), so
the rule only suppressed honest colliding traffic.

Return OPAQUE_RELAY_ONLY on DECODE_FAILURE unless the packet is addressed to
us or claims to be from us. isFromUs stays REJECT because OPAQUE_RELAY_ONLY
reaches perhapsGenerateImplicitAckForOwnOverheard, which matches pending sends
on header bytes alone - a forged sender with a colliding hash and matching id
could otherwise fake-ACK a DM and cancel its retransmissions. Other
DECODE_FAILURE sources are unaffected: legacy-DM rejection, pending-key
refusal, and failed PKI candidates are all isToUs, and the KNOWN_ONLY early
return is re-gated by relayOpaquePacket's own mode check. Opaque frames still
never touch PacketHistory, NodeDB, modules, MQTT, ACKs, or the phone.

test_C12's collision leg now expects OPAQUE_RELAY_ONLY (its tampered packet is
a broadcast - byte-identical to the foreign case); it still pins per-exact-byte
cache reevaluation. test_C9 renamed to match what it now verifies. New test_C17
covers the colliding-hash foreign broadcast and the spoofed-sender REJECT.

* style(mesh): trim collision-relay comment to two lines
This commit is contained in:
Ixitxachitl
2026-08-18 21:12:44 +00:00
committed by GitHub
parent 48699a7a48
commit 8fe246e250
2 changed files with 46 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/mesh/Router.cpp
+6
View File
@@ -789,6 +789,12 @@ RoutingAuthVerdict passesRoutingAuthGate(meshtastic_MeshPacket *p)
return RoutingAuthVerdict::REJECT;
}
if (state == DecodeState::DECODE_FAILURE) {
// One-byte hash collisions are indistinguishable from tampering, so relay opaquely
// instead of blackholing; isFromUs stays REJECT to keep forged senders off the ACK path.
if (!isToUs(p) && !isFromUs(p)) {
LOG_WARN("Decryptable packet failed decoding, relay opaquely");
return RoutingAuthVerdict::OPAQUE_RELAY_ONLY;
}
LOG_WARN("Decryptable packet failed decoding, drop");
return RoutingAuthVerdict::REJECT;
}
test/test_packet_signing/test_main.cpp
+40 -3
View File
@@ -1391,7 +1391,7 @@ void test_C8_trusted_local_decoded_delivery_is_not_filtered(void)
packetPool.release(local);
}
void test_C9_known_channel_malformed_plaintext_is_not_relayed_as_opaque(void)
void test_C9_known_channel_malformed_plaintext_has_no_pipeline_effects(void)
{
meshtastic_MeshPacket malformed = meshtastic_MeshPacket_init_zero;
malformed.from = REMOTE_NODE;
@@ -1404,6 +1404,12 @@ void test_C9_known_channel_malformed_plaintext_is_not_relayed_as_opaque(void)
malformed.encrypted.bytes[2] = 0xFF;
malformed.channel = channels.setActiveByIndex(0);
crypto->encryptPacket(malformed.from, malformed.id, malformed.encrypted.size, malformed.encrypted.bytes);
// Verdict is opaque-relay-eligible now (see test_C17); hop_limit 0 is what keeps this a no-op.
meshtastic_MeshPacket verdictCopy = malformed;
TEST_ASSERT_EQUAL(static_cast<int>(RoutingAuthVerdict::OPAQUE_RELAY_ONLY),
static_cast<int>(passesRoutingAuthGate(&verdictCopy)));
mockNodeDB->addNode(REMOTE_NODE);
const uint32_t lastHeard = mockNodeDB->getMeshNode(REMOTE_NODE)->last_heard;
runPipelineIngress(malformed);
@@ -1468,9 +1474,12 @@ void test_C12_exact_authenticated_replay_reuses_verdict_without_collision_bypass
runPipelineIngress(valid);
TEST_ASSERT_EQUAL_MESSAGE(2, routingAuthEvaluationCount(), "consumed verdict must not authenticate a later replay");
// Broadcast, so isToUs() is false like any colliding-hash foreign broadcast (see test_C17);
// this still guards that the cache is reevaluated per exact bytes, not reused for a same-ID replay.
meshtastic_MeshPacket collision = valid;
collision.encrypted.bytes[0] ^= 0x80;
TEST_ASSERT_EQUAL(static_cast<int>(RoutingAuthVerdict::REJECT), static_cast<int>(passesRoutingAuthGate(&collision)));
TEST_ASSERT_EQUAL(static_cast<int>(RoutingAuthVerdict::OPAQUE_RELAY_ONLY),
static_cast<int>(passesRoutingAuthGate(&collision)));
TEST_ASSERT_EQUAL_MESSAGE(3, routingAuthEvaluationCount(), "same packet ID with different bytes must be reevaluated");
}
@@ -1573,6 +1582,33 @@ void test_C16_reliable_broadcast_keeps_three_total_attempts(void)
TEST_ASSERT_EQUAL_UINT8(3, pipelineRouter->pendingTotalAttempts(LOCAL_NODE, p.id));
}
void test_C17_colliding_channel_hash_foreign_broadcast_is_relay_only(void)
{
// Foreign channel whose PSK collides with our channel 0's one-byte hash (see test_C9/test_C12
// for the paired tradeoff): indistinguishable from tampering, so it must relay opaquely.
setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_STRICT);
meshtastic_MeshPacket foreign = meshtastic_MeshPacket_init_zero;
foreign.from = REMOTE_NODE;
foreign.to = NODENUM_BROADCAST;
foreign.id = 0xC1700017;
foreign.hop_limit = 1;
foreign.hop_start = 2;
foreign.which_payload_variant = meshtastic_MeshPacket_encrypted_tag;
const int16_t hash = channels.setActiveByIndex(0);
TEST_ASSERT_GREATER_OR_EQUAL_MESSAGE(0, hash, "no usable primary channel");
foreign.channel = (uint8_t)hash; // collides with our channel 0, but the ciphertext below is not ours
foreign.encrypted.size = 16;
memset(foreign.encrypted.bytes, 0xA5, foreign.encrypted.size);
TEST_ASSERT_EQUAL(static_cast<int>(RoutingAuthVerdict::OPAQUE_RELAY_ONLY), static_cast<int>(passesRoutingAuthGate(&foreign)));
// Same undecodable frame claiming to be from us must still be dropped: OPAQUE_RELAY_ONLY would
// reach perhapsGenerateImplicitAckForOwnOverheard, which acts on header bytes alone.
meshtastic_MeshPacket spoofed = foreign;
spoofed.from = LOCAL_NODE;
TEST_ASSERT_EQUAL(static_cast<int>(RoutingAuthVerdict::REJECT), static_cast<int>(passesRoutingAuthGate(&spoofed)));
}
// C5: the packet survives (C4) but the identity claim inside it must not land - the pubkey guard
// can't tell a signer from an impersonator replaying its (public) key. Only the write is refused.
void test_N5_unsigned_unicast_nodeinfo_from_signer_does_not_change_name(void)
@@ -2142,7 +2178,7 @@ void setup()
RUN_TEST(test_C6_opaque_unknown_channel_is_relay_only);
RUN_TEST(test_C7_strict_rejects_unsigned_decoded_simradio_ingress);
RUN_TEST(test_C8_trusted_local_decoded_delivery_is_not_filtered);
RUN_TEST(test_C9_known_channel_malformed_plaintext_is_not_relayed_as_opaque);
RUN_TEST(test_C9_known_channel_malformed_plaintext_has_no_pipeline_effects);
RUN_TEST(test_C10_legacy_channel_dm_failure_has_no_pipeline_effects);
RUN_TEST(test_C11_malformed_pki_plaintext_has_no_pipeline_effects);
RUN_TEST(test_C12_exact_authenticated_replay_reuses_verdict_without_collision_bypass);
@@ -2150,6 +2186,7 @@ void setup()
RUN_TEST(test_C14_duty_cycle_limited_reliable_send_remains_pending);
RUN_TEST(test_C15_reliable_unicast_tracks_five_total_attempts);
RUN_TEST(test_C16_reliable_broadcast_keeps_three_total_attempts);
RUN_TEST(test_C17_colliding_channel_hash_foreign_broadcast_is_relay_only);
printf("\n=== Group N: NodeInfoModule authentication ===\n");
RUN_TEST(test_N1_unsigned_nodeinfo_from_signer_dropped);
RUN_TEST(test_N2_signed_nodeinfo_from_signer_not_dropped);