kevin/meshtastic_firmware
1
0
Fork 0
Code Issues Pull Requests Actions 4 Packages Projects Releases Wiki Activity

Router: size the fit check from the decoded Data (#11018)

Browse Source
This commit is contained in:
Thomas Göttgens
2026-07-16 08:51:10 -05:00
committed by GitHub
co-authored by GitHub
parent f7fd058308
commit f52d7753ad
3 changed files with 111 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/mesh/Router.cpp
+15 -15
View File
@@ -452,7 +452,7 @@ void Router::sniffReceived(const meshtastic_MeshPacket *p, const meshtastic_Rout
}
#if !(MESHTASTIC_EXCLUDE_PKI) && !(MESHTASTIC_EXCLUDE_XEDDSA)
bool checkXeddsaReceivePolicy(meshtastic_MeshPacket *p, size_t encodedDataSize)
bool checkXeddsaReceivePolicy(meshtastic_MeshPacket *p)
{
// Only a signature we verify below may mark this packet signed; never trust an inbound flag.
p->xeddsa_signed = false;
@@ -483,17 +483,17 @@ bool checkXeddsaReceivePolicy(meshtastic_MeshPacket *p, size_t encodedDataSize)
return false;
} else {
// Truly unsigned (signature size 0) - only reject the class a signing node always signs: a
// non-PKI broadcast whose signed encoding would still fit the LoRa frame. encodedDataSize is
// the size of the encoded Data exactly as the sender built it (or 0 to size p->decoded
// canonically); with no signature field present it is the unsigned base, and adding
// XEDDSA_SIGNATURE_FIELD_BYTES mirrors the sender-side signedDataFits() gate per packet,
// whatever fields the Data carried. Unicast/PKI packets and broadcasts too big to carry a
// signature are never signed, so they must not be hard-failed here even for a known signer.
// non-PKI broadcast whose signed encoding would still fit the LoRa frame. Size p->decoded
// canonically so this counts the same fields the sender's signedDataFits() gate counted;
// adding XEDDSA_SIGNATURE_FIELD_BYTES to that unsigned base mirrors it exactly, whatever
// fields the Data carried. Unicast/PKI packets and broadcasts too big to carry a signature
// are never signed, so they must not be hard-failed here even for a known signer.
const meshtastic_NodeInfoLite *node = nodeDB->getMeshNode(p->from);
if (node && nodeInfoLiteHasXeddsaSigned(node) && !p->pki_encrypted && isBroadcast(p->to)) {
if (encodedDataSize == 0 && !pb_get_encoded_size(&encodedDataSize, &meshtastic_Data_msg, &p->decoded))
size_t canonicalSize;
if (!pb_get_encoded_size(&canonicalSize, &meshtastic_Data_msg, &p->decoded))
return true; // can't size it; never drop on a sizing failure
if (encodedDataSize + XEDDSA_SIGNATURE_FIELD_BYTES + MESHTASTIC_HEADER_LENGTH <= MAX_LORA_PAYLOAD_LEN) {
if (canonicalSize + XEDDSA_SIGNATURE_FIELD_BYTES + MESHTASTIC_HEADER_LENGTH <= MAX_LORA_PAYLOAD_LEN) {
LOG_WARN("Dropping unsigned broadcast from 0x%08x that previously signed", p->from);
return false;
}
@@ -621,17 +621,17 @@ DecodeState perhapsDecode(meshtastic_MeshPacket *p)
if (decrypted) {
// parsing was successful
p->channel = chIndex; // change to store the index instead of the hash
if (p->decoded.has_bitfield)
p->decoded.want_response |= p->decoded.bitfield & BITFIELD_WANT_RESPONSE_MASK;
#if !(MESHTASTIC_EXCLUDE_PKI) && !(MESHTASTIC_EXCLUDE_XEDDSA)
// rawSize is the size of the encoded Data exactly as the sender built it (the PKI branch's
// MESHTASTIC_PKC_OVERHEAD subtraction preserves that, and PKI packets are unicast so the
// downgrade predicate ignores them anyway).
if (!checkXeddsaReceivePolicy(p, rawSize))
// Runs before the bitfield merge below: that merge can set want_response, adding wire bytes
// the sender never encoded and skewing the policy's sizing of p->decoded.
if (!checkXeddsaReceivePolicy(p))
return DecodeState::DECODE_FAILURE;
#endif
if (p->decoded.has_bitfield)
p->decoded.want_response |= p->decoded.bitfield & BITFIELD_WANT_RESPONSE_MASK;
/* Not actually ever used.
// Decompress if needed. jm
if (p->decoded.portnum == meshtastic_PortNum_TEXT_MESSAGE_COMPRESSED_APP) {
src/mesh/Router.h
+3 -4
View File
@@ -183,16 +183,15 @@ meshtastic_Routing_Error perhapsEncode(meshtastic_MeshPacket *p);
* downgrade protection: drop a non-PKI broadcast from a known signer whose signed encoding would
* still fit a LoRa frame (unicast, PKI, and oversized broadcasts always pass).
*
* encodedDataSize is the wire size of the encoded Data as the sender built it; pass 0 to size
* p->decoded canonically instead (for already-decoded ingress such as plaintext-MQTT downlink,
* which bypasses perhapsDecode's crypto path).
* The fit test sizes p->decoded with the real encoder, so it measures the fields the sender
* encoded rather than any raw wire length.
*
* The caller MUST hold cryptLock: verification runs through the shared CryptoEngine key cache.
* (perhapsDecode already holds it; other call sites must take it themselves.)
*
* @return false if the packet must be dropped.
*/
bool checkXeddsaReceivePolicy(meshtastic_MeshPacket *p, size_t encodedDataSize = 0);
bool checkXeddsaReceivePolicy(meshtastic_MeshPacket *p);
#endif
extern Router *router;
test/test_packet_signing/test_main.cpp
+93 -4
View File
@@ -148,6 +148,58 @@ static bool signedEncodingFits(const meshtastic_Data *d)
return encodedDataSize(&copy) + MESHTASTIC_HEADER_LENGTH <= MAX_LORA_PAYLOAD_LEN;
}
// Append a length-delimited field whose tag this build's Data schema does not define, as a sender
// on a newer schema would emit. nanopb skips unknown fields at decode, so these bytes count toward
// the raw wire size but not the decoded struct. Returns the number of bytes appended.
static size_t appendUnknownField(uint8_t *dst, size_t dstLen, size_t contentLen)
{
constexpr uint32_t UNKNOWN_FIELD_NUMBER = 100; // not a field of meshtastic_Data
std::vector<uint8_t> content(contentLen, 0x77);
pb_ostream_t stream = pb_ostream_from_buffer(dst, dstLen);
TEST_ASSERT_TRUE(pb_encode_tag(&stream, PB_WT_STRING, UNKNOWN_FIELD_NUMBER));
TEST_ASSERT_TRUE(pb_encode_string(&stream, content.data(), content.size()));
return stream.bytes_written;
}
// Channel-encrypt raw Data bytes into a packet, exactly as perhapsEncode's non-PKI path does.
// Used to inject wire bytes perhapsEncode would never produce (it only encodes p->decoded).
static void encryptAsChannelPacket(meshtastic_MeshPacket *p, uint8_t *wire, size_t size)
{
const int16_t hash = channels.setActiveByIndex(0);
TEST_ASSERT_GREATER_OR_EQUAL_MESSAGE(0, hash, "no usable primary channel");
crypto->encryptPacket(getFrom(p), p->id, size, wire);
memcpy(p->encrypted.bytes, wire, size);
p->encrypted.size = size;
p->channel = hash; // on the wire the channel field carries the hash, not the index
p->which_payload_variant = meshtastic_MeshPacket_encrypted_tag;
}
// Build A10's frame: an unsigned broadcast carrying a POSITION payload plus unknown fields, sized
// so the raw wire length exceeds the signature-fit threshold while the decoded fields stay under
// it. Channel-encrypted like a normal sender. The asserts pin that split, which is what makes A10
// and A11 meaningful.
static meshtastic_MeshPacket makeBroadcastWithUnknownFields()
{
meshtastic_MeshPacket p = makeDecoded(REMOTE_NODE, NODENUM_BROADCAST, meshtastic_PortNum_POSITION_APP, SMALL_PAYLOAD);
uint8_t wire[MAX_LORA_PAYLOAD_LEN + 1];
const size_t base = pb_encode_to_bytes(wire, sizeof(wire), &meshtastic_Data_msg, &p.decoded);
TEST_ASSERT_GREATER_THAN_MESSAGE(0, base, "failed to encode the base Data");
const size_t raw = base + appendUnknownField(wire + base, sizeof(wire) - base, 160);
// The decoded fields fit a signature, so a sender that signs would have signed this Data.
TEST_ASSERT_LESS_OR_EQUAL_MESSAGE(MAX_LORA_PAYLOAD_LEN, base + XEDDSA_SIGNATURE_FIELD_BYTES + MESHTASTIC_HEADER_LENGTH,
"decoded fields must fit a signature, else the test is vacuous");
// The unknown fields put the raw size over that threshold, so the two sizings disagree here.
TEST_ASSERT_GREATER_THAN_MESSAGE(MAX_LORA_PAYLOAD_LEN, raw + XEDDSA_SIGNATURE_FIELD_BYTES + MESHTASTIC_HEADER_LENGTH,
"unknown fields must push the raw size past the fit threshold");
// The frame is still one a radio could actually send.
TEST_ASSERT_LESS_OR_EQUAL_MESSAGE(MAX_LORA_PAYLOAD_LEN, raw + MESHTASTIC_HEADER_LENGTH, "frame must still fit a LoRa frame");
encryptAsChannelPacket(&p, wire, raw);
return p;
}
// ---------------------------------------------------------------------------
// Unity lifecycle
// ---------------------------------------------------------------------------
@@ -324,6 +376,41 @@ void test_A9_unsigned_boundary_broadcast_from_signer_still_dropped(void)
TEST_ASSERT_EQUAL(DECODE_FAILURE, roundTrip(&p));
}
// A10: unknown fields must not sway the downgrade decision. A sender on a newer schema can include
// Data fields this build doesn't define; nanopb skips them, so they grow the frame without changing
// what decodes. The decision sizes p->decoded, keeping it on the fields the sender encoded, so an
// unsigned broadcast from a known signer is dropped whether or not unknown fields came along.
void test_A10_unsigned_broadcast_from_signer_with_unknown_fields_dropped(void)
{
mockNodeDB->addNode(REMOTE_NODE);
mockNodeDB->setSignerBit(REMOTE_NODE, true); // we've seen this node sign before
meshtastic_MeshPacket p = makeBroadcastWithUnknownFields();
TEST_ASSERT_EQUAL_MESSAGE(DECODE_FAILURE, perhapsDecode(&p),
"unsigned broadcast from a signer must be dropped despite unknown fields");
}
// A11: A10's control - the same frame from a node we've never seen sign is accepted and still
// delivers its payload, so A10's DECODE_FAILURE is the downgrade drop and not a decode failure
// caused by the unknown fields.
void test_A11_unsigned_broadcast_from_nonsigner_with_unknown_fields_accepted(void)
{
mockNodeDB->addNode(REMOTE_NODE); // signer bit clear
meshtastic_MeshPacket p = makeBroadcastWithUnknownFields();
const size_t rawSize = p.encrypted.size; // read before decode: encrypted/decoded share a union
TEST_ASSERT_EQUAL_MESSAGE(DECODE_SUCCESS, perhapsDecode(&p), "frame from a non-signer must still decode");
TEST_ASSERT_EQUAL_MESSAGE(meshtastic_PortNum_POSITION_APP, p.decoded.portnum, "unknown fields must not disturb the portnum");
TEST_ASSERT_EQUAL_MESSAGE(SMALL_PAYLOAD, p.decoded.payload.size, "payload must survive the unknown fields");
TEST_ASSERT_FALSE(p.xeddsa_signed);
// The unknown fields are gone from the decoded struct: the gap the sizing basis has to account for.
TEST_ASSERT_LESS_THAN_MESSAGE(rawSize, encodedDataSize(&p.decoded),
"unknown fields must drop at decode, leaving decoded size < raw");
}
// ===========================================================================
// Group B - send-side signing policy (perhapsEncode)
// ===========================================================================
@@ -418,7 +505,7 @@ void test_B5_preset_signature_on_local_packet_cleared(void)
// B6: the exact-fit gate tracks Data *shape*, not just payload size. A tapback-style broadcast
// (want_response + reply_id + emoji) carries extra wire bytes that shift the fit boundary; the
// sweep proves no dead band exists for that shape either, and - once the signer bit is learned -
// that the receiver's rawSize-driven downgrade predicate stays symmetric for it too. Window
// that the receiver's downgrade predicate stays symmetric for it too. Window
// straddles this shape's boundary; capped at 200 so even the unsigned rich encoding stays well
// inside the frame (at n=221 it first hits the pre-existing, signing-unrelated TOO_LARGE).
void test_B6_rich_shape_sweep_no_deadband(void)
@@ -558,7 +645,7 @@ void test_D1_signature_field_overhead_exact(void)
// ===========================================================================
// Already-decoded packets never reach perhapsDecode's crypto path (it early-returns), so
// plaintext-MQTT downlink applies this policy function directly at ingress (MQTT.cpp). These
// tests drive it the same way: decoded packets, encodedDataSize = 0 (canonical sizing).
// tests drive it the same way: decoded packets, sized from p->decoded exactly as the RF path is.
// End-to-end MQTT wiring is covered in test_mqtt.
// E1: unsigned small broadcast from a known signer -> dropped (downgrade protection holds on
@@ -616,8 +703,8 @@ void test_E4_decoded_bad_signature_dropped(void)
TEST_ASSERT_FALSE(p.xeddsa_signed);
}
// E5: unsigned oversized broadcast from a signer -> accepted (canonical sizing exempts packets
// whose signed encoding wouldn't fit, mirroring the RF-path rawSize rule).
// E5: unsigned oversized broadcast from a signer -> accepted (packets whose signed encoding
// wouldn't fit are exempt, identically to the RF path: both size p->decoded).
void test_E5_decoded_unsigned_oversized_broadcast_from_signer_accepted(void)
{
mockNodeDB->addNode(REMOTE_NODE);
@@ -700,6 +787,8 @@ void setup()
RUN_TEST(test_A7_unsigned_oversized_broadcast_from_signer_accepted);
RUN_TEST(test_A8_unsigned_deadband_broadcast_from_signer_accepted);
RUN_TEST(test_A9_unsigned_boundary_broadcast_from_signer_still_dropped);
RUN_TEST(test_A10_unsigned_broadcast_from_signer_with_unknown_fields_dropped);
RUN_TEST(test_A11_unsigned_broadcast_from_nonsigner_with_unknown_fields_accepted);
printf("\n=== Group B: send-side signing policy ===\n");
RUN_TEST(test_B1_local_broadcast_is_signed);