Skip to content

Commit c800fc8

Browse files
caveman99thebentern
authored andcommitted
Signing and ingress hardening (#11282)
* Include warm-tier signers in the identity update gate * Fail the send when PKI encryption fails * Require signatures on licensed unicasts * Include warm-tier signers in the NodeInfo downgrade drop * Clamp hop fields on UDP multicast ingress * Address review comments on signing hardening Condense the updateUser rationale to two lines and stop calling the Balanced-mode drop a broadcast now that licensed unicasts reach it. (cherry picked from commit df6e67f)
1 parent 9e21ed1 commit c800fc8

4 files changed

Lines changed: 21 additions & 19 deletions

File tree

src/mesh/NodeDB.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3499,10 +3499,9 @@ void NodeDB::addFromContact(meshtastic_SharedContact contact)
34993499
*/
35003500
bool NodeDB::updateUser(uint32_t nodeId, meshtastic_User &p, uint8_t channelIndex, bool xeddsaSigned)
35013501
{
3502-
// Only a signed update may change the identity of a node that has proven it signs; our own record is
3503-
// exempt. Checked before getOrCreateMeshNode so a refused update cannot evict or write the warm tier.
3504-
const meshtastic_NodeInfoLite *existing = getMeshNode(nodeId);
3505-
if (nodeId != getNodeNum() && existing && nodeInfoLiteHasXeddsaSigned(existing) && !xeddsaSigned) {
3502+
// Only a signed update may change the identity of a proven signer; our own record is exempt.
3503+
// Checked before getOrCreateMeshNode so a refusal cannot evict; isKnownXeddsaSigner covers the warm tier.
3504+
if (nodeId != getNodeNum() && isKnownXeddsaSigner(nodeId) && !xeddsaSigned) {
35063505
LOG_WARN("Refusing unsigned identity update for node 0x%08x that previously signed", nodeId);
35073506
return false;
35083507
}

src/mesh/Router.cpp

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -667,20 +667,14 @@ bool checkXeddsaReceivePolicy(meshtastic_MeshPacket *p)
667667
if (compatible)
668668
return true;
669669

670-
// In Balanced, preserve legacy unsigned-unicast compatibility and only reject the class a
671-
// signing node always signs: a non-PKI broadcast whose signed encoding would still fit the
672-
// LoRa frame. Canonical sizing removes unknown protobuf fields before mirroring the
673-
// sender-side signedDataFits() gate, so this counts the same fields that gate counted.
674-
// Unicast packets and broadcasts too big to carry a signature are never signed, so they
675-
// must not be hard-failed here even for a known signer (PKI already returned above).
676-
// isKnownXeddsaSigner consults the warm tier too: a signer evicted from the hot store
677-
// must not become impersonatable via unsigned broadcasts until it is re-heard.
678-
if (nodeDB->isKnownXeddsaSigner(p->from) && isBroadcast(p->to)) {
670+
// Balanced rejects only what a signer always signs: non-PKI broadcasts whose signed encoding
671+
// would have fit, plus unicasts on ham where licensed senders sign too. Mirrors perhapsEncode.
672+
if (nodeDB->isKnownXeddsaSigner(p->from) && (isBroadcast(p->to) || owner.is_licensed)) {
679673
size_t canonicalSize;
680674
if (!canonicalSignableSize(&p->decoded, &canonicalSize))
681675
return true; // can't size it; never drop on a sizing failure
682676
if (canonicalSize + XEDDSA_SIGNATURE_FIELD_BYTES + MESHTASTIC_HEADER_LENGTH <= MAX_LORA_PAYLOAD_LEN) {
683-
LOG_WARN("Dropping unsigned broadcast from 0x%08x that previously signed", p->from);
677+
LOG_WARN("Dropping unsigned packet from 0x%08x that previously signed", p->from);
684678
return false;
685679
}
686680
}
@@ -1158,7 +1152,12 @@ meshtastic_Routing_Error perhapsEncode(meshtastic_MeshPacket *p)
11581152
*destKey.bytes);
11591153
return meshtastic_Routing_Error_PKI_FAILED;
11601154
}
1161-
crypto->encryptCurve25519(p->to, getFrom(p), destKey, p->id, numbytes, bytes, p->encrypted.bytes);
1155+
// On failure encrypted.bytes holds no ciphertext, so continuing would put the plaintext
1156+
// on the air labelled pki_encrypted.
1157+
if (!crypto->encryptCurve25519(p->to, getFrom(p), destKey, p->id, numbytes, bytes, p->encrypted.bytes)) {
1158+
LOG_WARN("PKI encryption failed for destination node 0x%08x", p->to);
1159+
return meshtastic_Routing_Error_PKI_FAILED;
1160+
}
11621161
numbytes += MESHTASTIC_PKC_OVERHEAD;
11631162
p->channel = 0;
11641163
p->pki_encrypted = true;

src/mesh/udp/UdpMulticastHandler.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,11 @@ class UdpMulticastHandler final
7979
LOG_WARN("UDP packet with spoofed local from=0x%08x, dropping", mp.from);
8080
return;
8181
}
82+
// Same clamp the MQTT ingress applies: an out-of-range hop count is not relayable.
83+
if (mp.hop_limit > HOP_MAX || mp.hop_start > HOP_MAX) {
84+
LOG_WARN("UDP packet with invalid hop_limit(%u) or hop_start(%u), dropping", mp.hop_limit, mp.hop_start);
85+
return;
86+
}
8287
mp.transport_mechanism = meshtastic_MeshPacket_TransportMechanism_TRANSPORT_MULTICAST_UDP;
8388
// Authentication metadata is local-only; Router re-establishes it after successful PKI decryption.
8489
mp.pki_encrypted = false;

src/modules/NodeInfoModule.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,9 @@ bool NodeInfoModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, mes
5050
return true;
5151
}
5252
NodeNum sourceNum = getFrom(&mp);
53-
const meshtastic_NodeInfoLite *node = nodeDB->getMeshNode(sourceNum);
54-
// Broadcasts only: senders never sign unicast NodeInfo, so dropping it would break exchanges
55-
// with signer nodes. Backstops ingress that skips Router's downgrade drop (e.g. decoded MQTT).
56-
if (node && nodeInfoLiteHasXeddsaSigned(node) && !mp.xeddsa_signed && isBroadcast(mp.to)) {
53+
// Broadcasts only: unicast NodeInfo is unsigned off ham, so updateUser refuses the identity
54+
// write instead. isKnownXeddsaSigner also covers the warm tier.
55+
if (nodeDB->isKnownXeddsaSigner(sourceNum) && !mp.xeddsa_signed && isBroadcast(mp.to)) {
5756
LOG_WARN("Dropping unsigned NodeInfo broadcast from node 0x%08x that previously signed", sourceNum);
5857
return true;
5958
}

0 commit comments

Comments
 (0)