From fca763e71b19bbdee29219961214fee73d4ab519 Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Sun, 26 Jul 2026 19:56:38 -0700 Subject: [PATCH 1/3] fix: keep telemetry on channel encryption by default --- src/mesh/Router.cpp | 3 +++ test/test_admin_session_repro/test_main.cpp | 22 +++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 66bc1d3b3b8..4997426f014 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -1035,6 +1035,9 @@ bool wouldEncryptWithPKC(const meshtastic_MeshPacket *p, ChannelIndex chIndex, b // Some portnums either make no sense to send with PKC p->decoded.portnum != meshtastic_PortNum_TRACEROUTE_APP && p->decoded.portnum != meshtastic_PortNum_NODEINFO_APP && p->decoded.portnum != meshtastic_PortNum_ROUTING_APP && p->decoded.portnum != meshtastic_PortNum_POSITION_APP && + // Telemetry is shared-channel encrypted unless a caller explicitly requests PKC. + // The latter is used by the restricted LocalStats policy. + (p->decoded.portnum != meshtastic_PortNum_TELEMETRY_APP || p->pki_encrypted) && // We allow Key Verification messages to be sent without a known destination key, since the point of those messages is // to exchange keys. The first exchange (no usable key yet) falls through to channel encryption; the follow-on packet // uses the pending key resolved into haveDestKey/destKey above. diff --git a/test/test_admin_session_repro/test_main.cpp b/test/test_admin_session_repro/test_main.cpp index c93c1fd9472..3de2145d57a 100644 --- a/test/test_admin_session_repro/test_main.cpp +++ b/test/test_admin_session_repro/test_main.cpp @@ -15,6 +15,7 @@ #include "mesh/Channels.h" #include "mesh/NodeDB.h" +#include "mesh/Router.h" #include "mesh/mesh-pb-constants.h" #include "modules/AdminModule.h" #include "support/AdminModuleTestShim.h" @@ -556,6 +557,26 @@ void test_ham_mode_request_is_not_pinned(void) "a request that could not have gone out over PKC must not be pinned"); } +// Telemetry requests and replies need normal shared-channel encryption so every node with the +// channel key can decode the LocalStats payload. They must not be auto-upgraded to PKC merely +// because both endpoints have identity keys. +void test_telemetry_uses_channel_encryption_not_pkc(void) +{ + meshtastic_MeshPacket telemetry = meshtastic_MeshPacket_init_zero; + telemetry.from = LOCAL_NODE; + telemetry.to = QUERIED_NODE; + telemetry.channel = channels.getPrimaryIndex(); + telemetry.which_payload_variant = meshtastic_MeshPacket_decoded_tag; + telemetry.decoded.portnum = meshtastic_PortNum_TELEMETRY_APP; + + TEST_ASSERT_FALSE_MESSAGE(wouldEncryptWithPKC(&telemetry, telemetry.channel, true), + "LocalStats telemetry must use normal channel encryption"); + + telemetry.pki_encrypted = true; + TEST_ASSERT_TRUE_MESSAGE(wouldEncryptWithPKC(&telemetry, telemetry.channel, true), + "an explicitly PKI LocalStats request must retain PKC encryption"); +} + // The response must echo our request's packet id, so an injector cannot answer a request it did // not see just by naming the right node and variant. void test_response_with_wrong_request_id_is_rejected(void) @@ -690,6 +711,7 @@ void setup() RUN_TEST(test_pinned_request_keeps_its_key_after_an_unpinned_request); RUN_TEST(test_request_to_keyed_node_pins_the_stored_key); RUN_TEST(test_ham_mode_request_is_not_pinned); + RUN_TEST(test_telemetry_uses_channel_encryption_not_pkc); RUN_TEST(test_response_with_wrong_request_id_is_rejected); RUN_TEST(test_request_without_an_id_admits_nothing); RUN_TEST(test_module_config_subtype_must_match); From 04062df64b8d636518b410173fa56bd86bf17bcf Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Sun, 26 Jul 2026 20:20:31 -0700 Subject: [PATCH 2/3] test: preserve PKI for non-LocalStats telemetry --- test/test_admin_session_repro/test_main.cpp | 22 ++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/test/test_admin_session_repro/test_main.cpp b/test/test_admin_session_repro/test_main.cpp index 3de2145d57a..18063735a96 100644 --- a/test/test_admin_session_repro/test_main.cpp +++ b/test/test_admin_session_repro/test_main.cpp @@ -557,10 +557,10 @@ void test_ham_mode_request_is_not_pinned(void) "a request that could not have gone out over PKC must not be pinned"); } -// Telemetry requests and replies need normal shared-channel encryption so every node with the -// channel key can decode the LocalStats payload. They must not be auto-upgraded to PKC merely -// because both endpoints have identity keys. -void test_telemetry_uses_channel_encryption_not_pkc(void) +// LocalStats requests and replies need normal shared-channel encryption so every node with the +// channel key can decode the payload. They must not be auto-upgraded to PKC merely because both +// endpoints have identity keys. Other telemetry retains its normal PKC behavior. +void test_local_stats_uses_channel_encryption_not_pkc(void) { meshtastic_MeshPacket telemetry = meshtastic_MeshPacket_init_zero; telemetry.from = LOCAL_NODE; @@ -568,6 +568,10 @@ void test_telemetry_uses_channel_encryption_not_pkc(void) telemetry.channel = channels.getPrimaryIndex(); telemetry.which_payload_variant = meshtastic_MeshPacket_decoded_tag; telemetry.decoded.portnum = meshtastic_PortNum_TELEMETRY_APP; + meshtastic_Telemetry localStats = meshtastic_Telemetry_init_zero; + localStats.which_variant = meshtastic_Telemetry_local_stats_tag; + telemetry.decoded.payload.size = pb_encode_to_bytes(telemetry.decoded.payload.bytes, sizeof(telemetry.decoded.payload.bytes), + &meshtastic_Telemetry_msg, &localStats); TEST_ASSERT_FALSE_MESSAGE(wouldEncryptWithPKC(&telemetry, telemetry.channel, true), "LocalStats telemetry must use normal channel encryption"); @@ -575,6 +579,14 @@ void test_telemetry_uses_channel_encryption_not_pkc(void) telemetry.pki_encrypted = true; TEST_ASSERT_TRUE_MESSAGE(wouldEncryptWithPKC(&telemetry, telemetry.channel, true), "an explicitly PKI LocalStats request must retain PKC encryption"); + + telemetry.pki_encrypted = false; + meshtastic_Telemetry deviceMetrics = meshtastic_Telemetry_init_zero; + deviceMetrics.which_variant = meshtastic_Telemetry_device_metrics_tag; + telemetry.decoded.payload.size = pb_encode_to_bytes(telemetry.decoded.payload.bytes, sizeof(telemetry.decoded.payload.bytes), + &meshtastic_Telemetry_msg, &deviceMetrics); + TEST_ASSERT_TRUE_MESSAGE(wouldEncryptWithPKC(&telemetry, telemetry.channel, true), + "non-LocalStats telemetry must retain its existing PKC behavior"); } // The response must echo our request's packet id, so an injector cannot answer a request it did @@ -711,7 +723,7 @@ void setup() RUN_TEST(test_pinned_request_keeps_its_key_after_an_unpinned_request); RUN_TEST(test_request_to_keyed_node_pins_the_stored_key); RUN_TEST(test_ham_mode_request_is_not_pinned); - RUN_TEST(test_telemetry_uses_channel_encryption_not_pkc); + RUN_TEST(test_local_stats_uses_channel_encryption_not_pkc); RUN_TEST(test_response_with_wrong_request_id_is_rejected); RUN_TEST(test_request_without_an_id_admits_nothing); RUN_TEST(test_module_config_subtype_must_match); From f5f0259f562b2dfc1886722e50534986c18b9fe4 Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Sun, 26 Jul 2026 20:21:43 -0700 Subject: [PATCH 3/3] fix: limit channel encryption to LocalStats --- src/mesh/Router.cpp | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 4997426f014..17efaa475fa 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -1017,6 +1017,19 @@ static bool signedDataFits(meshtastic_Data *d) #endif #if !(MESHTASTIC_EXCLUDE_PKI) +static bool isLocalStatsTelemetry(const meshtastic_MeshPacket *p) +{ + if (p->decoded.portnum != meshtastic_PortNum_TELEMETRY_APP) + return false; + + // Telemetry is large on smaller targets, so do not place this decode scratch on the stack. + // perhapsEncode() holds cryptLock while calling wouldEncryptWithPKC(), which serializes its use. + static meshtastic_Telemetry telemetry; + memset(&telemetry, 0, sizeof(telemetry)); + return pb_decode_from_bytes(p->decoded.payload.bytes, p->decoded.payload.size, &meshtastic_Telemetry_msg, &telemetry) && + telemetry.which_variant == meshtastic_Telemetry_local_stats_tag; +} + bool wouldEncryptWithPKC(const meshtastic_MeshPacket *p, ChannelIndex chIndex, bool haveDestKey) { // First, only PKC encrypt packets we are originating @@ -1035,9 +1048,9 @@ bool wouldEncryptWithPKC(const meshtastic_MeshPacket *p, ChannelIndex chIndex, b // Some portnums either make no sense to send with PKC p->decoded.portnum != meshtastic_PortNum_TRACEROUTE_APP && p->decoded.portnum != meshtastic_PortNum_NODEINFO_APP && p->decoded.portnum != meshtastic_PortNum_ROUTING_APP && p->decoded.portnum != meshtastic_PortNum_POSITION_APP && - // Telemetry is shared-channel encrypted unless a caller explicitly requests PKC. - // The latter is used by the restricted LocalStats policy. - (p->decoded.portnum != meshtastic_PortNum_TELEMETRY_APP || p->pki_encrypted) && + // LocalStats is shared-channel encrypted unless a caller explicitly requests PKC. + // Other telemetry keeps its existing PKC behavior. + (p->pki_encrypted || !isLocalStatsTelemetry(p)) && // We allow Key Verification messages to be sent without a known destination key, since the point of those messages is // to exchange keys. The first exchange (no usable key yet) falls through to channel encryption; the follow-on packet // uses the pending key resolved into haveDestKey/destKey above.