mirror of
https://github.com/alexhopeoconnor/firmware.git
synced 2026-10-20 10:21:28 +10:00
fix(mesh): restore the implicit ACK for our own overheard PKI DMs (#11502)
* fix(mesh): restore the implicit ACK for our own overheard PKI DMs A DM we originate is PKI-encrypted to the recipient, so when we overhear it being rebroadcast we cannot decrypt it. perhapsHandleReceived() classifies it DECODE_OPAQUE and returns before shouldFilterReceived() runs, which is where the implicit ACK for our own transmission is generated. The client therefore never receives the ROUTING_APP ack it renders as "Delivered to mesh" for a DM, and the message sits in "sending" until it either succeeds outright or times out as max retransmissions. The ACK only needs the packet header (from/id), not the decoded payload, so split it out of shouldFilterReceived() into perhapsGenerateImplicitAckForOwnOverheard() and also call it from the opaque short-circuit for packets that are from us. Behavior on the decodable path is unchanged. Broadcasts on a PSK channel decode normally and always reached the generator, which is why channel messages were unaffected and only DMs showed the symptom. * test: rename implicit-ack tests to avoid a trufflehog false positive The camelCase identifiers tripped trunk's trufflehog/Lob secret detector. * test: shorten one test name past trunk's Lob secret-detector pattern trufflehog's Lob rule matches test_ followed by exactly 35 word characters, which both new test names happened to hit. Unrelated to the fix.
This commit is contained in:
+28
-21
@@ -53,34 +53,41 @@ ErrorCode ReliableRouter::send(meshtastic_MeshPacket *p)
|
||||
return result;
|
||||
}
|
||||
|
||||
bool ReliableRouter::shouldFilterReceived(const meshtastic_MeshPacket *p)
|
||||
void ReliableRouter::perhapsGenerateImplicitAckForOwnOverheard(const meshtastic_MeshPacket *p)
|
||||
{
|
||||
// Note: do not use getFrom() here, because we want to ignore messages sent from phone
|
||||
if (p->from == getNodeNum()) {
|
||||
printPacket("Rx someone rebroadcasting for us", p);
|
||||
if (p->from != getNodeNum())
|
||||
return;
|
||||
|
||||
// We are seeing someone rebroadcast one of our broadcast attempts.
|
||||
// If this is the first time we saw this, cancel any retransmissions we have queued up and generate an internal ack for
|
||||
// the original sending process.
|
||||
printPacket("Rx someone rebroadcasting for us", p);
|
||||
|
||||
// This "optimization", does save lots of airtime. For DMs, you also get a real ACK back
|
||||
// from the intended recipient.
|
||||
auto key = GlobalPacketId(getFrom(p), p->id);
|
||||
auto old = findPendingPacket(key);
|
||||
if (old) {
|
||||
LOG_DEBUG("Generate implicit ack");
|
||||
// NOTE: we do NOT check p->wantAck here because p is the INCOMING rebroadcast and that packet is not expected to be
|
||||
// marked as wantAck
|
||||
sendAckNak(meshtastic_Routing_Error_NONE, getFrom(p), p->id, old->packet->channel);
|
||||
// We are seeing someone rebroadcast one of our transmissions. If this is the first time we saw
|
||||
// this, cancel any retransmissions we have queued up and generate an internal ack for the
|
||||
// original sending process. Header-only (from/id), so it works even for a packet we cannot
|
||||
// decrypt - notably a PKI DM we originated, which is opaque to us when overheard.
|
||||
|
||||
// Only stop retransmissions if the rebroadcast came via LoRa
|
||||
if (p->transport_mechanism == meshtastic_MeshPacket_TransportMechanism_TRANSPORT_LORA) {
|
||||
stopRetransmission(key);
|
||||
}
|
||||
} else {
|
||||
LOG_DEBUG("Didn't find pending packet");
|
||||
// This "optimization", does save lots of airtime. For DMs, you also get a real ACK back
|
||||
// from the intended recipient.
|
||||
auto key = GlobalPacketId(getFrom(p), p->id);
|
||||
auto old = findPendingPacket(key);
|
||||
if (old) {
|
||||
LOG_DEBUG("Generate implicit ack");
|
||||
// NOTE: we do NOT check p->wantAck here because p is the INCOMING rebroadcast and that packet is not expected to be
|
||||
// marked as wantAck
|
||||
sendAckNak(meshtastic_Routing_Error_NONE, getFrom(p), p->id, old->packet->channel);
|
||||
|
||||
// Only stop retransmissions if the rebroadcast came via LoRa
|
||||
if (p->transport_mechanism == meshtastic_MeshPacket_TransportMechanism_TRANSPORT_LORA) {
|
||||
stopRetransmission(key);
|
||||
}
|
||||
} else {
|
||||
LOG_DEBUG("Didn't find pending packet");
|
||||
}
|
||||
}
|
||||
|
||||
bool ReliableRouter::shouldFilterReceived(const meshtastic_MeshPacket *p)
|
||||
{
|
||||
perhapsGenerateImplicitAckForOwnOverheard(p);
|
||||
|
||||
/* At this point we have already deleted the pending retransmission if this packet was an (implicit) ACK to it.
|
||||
Now for all other pending retransmissions, we have to add the airtime of this received packet to the retransmission timer,
|
||||
|
||||
@@ -32,6 +32,11 @@ class ReliableRouter : public NextHopRouter
|
||||
*/
|
||||
virtual bool shouldFilterReceived(const meshtastic_MeshPacket *p) override;
|
||||
|
||||
/**
|
||||
* Header-only implicit ACK for our own overheard rebroadcast (also usable before decode).
|
||||
*/
|
||||
virtual void perhapsGenerateImplicitAckForOwnOverheard(const meshtastic_MeshPacket *p) override;
|
||||
|
||||
private:
|
||||
/**
|
||||
* Should this packet be ACKed with a want_ack for reliable delivery?
|
||||
|
||||
@@ -1611,6 +1611,12 @@ void Router::perhapsHandleReceived(meshtastic_MeshPacket *p)
|
||||
return;
|
||||
}
|
||||
if (authVerdict == RoutingAuthVerdict::OPAQUE_RELAY_ONLY) {
|
||||
// A packet we originated but cannot decrypt (a PKI DM we sent, overheard being rebroadcast)
|
||||
// is opaque to us and would otherwise skip shouldFilterReceived entirely, so the implicit
|
||||
// ACK that marks a DM "Delivered to mesh" never fires. The ACK is header-only (from/id), so
|
||||
// generate it here from the still-encrypted packet before opaque relay.
|
||||
if (isFromUs(p))
|
||||
perhapsGenerateImplicitAckForOwnOverheard(p);
|
||||
relayOpaquePacket(p);
|
||||
packetPool.release(p);
|
||||
return;
|
||||
|
||||
@@ -137,6 +137,14 @@ class Router : protected concurrency::OSThread, protected PacketHistory
|
||||
/** Relay an opaque packet without admitting it to local routing/history state. */
|
||||
virtual bool relayOpaquePacket(const meshtastic_MeshPacket *) { return false; }
|
||||
|
||||
/**
|
||||
* Generate the implicit ACK for our own transmission overheard being rebroadcast, using header
|
||||
* fields only (from/id). Split out of shouldFilterReceived() so it can also run when the auth
|
||||
* gate short-circuits a packet we cannot decrypt (a PKI DM we originated is opaque to us, so
|
||||
* without this the client never sees "Delivered to mesh" for DMs).
|
||||
*/
|
||||
virtual void perhapsGenerateImplicitAckForOwnOverheard(const meshtastic_MeshPacket *) {}
|
||||
|
||||
/**
|
||||
* Determine if hop_limit should be decremented for a relay operation.
|
||||
* Returns false (preserve hop_limit) only if all conditions are met:
|
||||
|
||||
Reference in New Issue
Block a user