Clamp position precision on public / known-keys (#10665)

* Clamp position precision on public / known-keys (Compliance)

* Fix review comments: bounds check, all-channel precision clamp, disabled channel guard

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Refactor position precision comments for clarity and update channel configuration handling in AdminModule

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

src/mesh/Channels.cpp

@@ -404,6 +404,42 @@ bool Channels::isDefaultChannel(ChannelIndex chIndex)
     return false;
 }
 
+bool cryptoKeyIsPublic(const CryptoKey &key)
+{
+    if (key.length == 0)
+        return true; // encryption disabled
+    // Match the defaultpsk family ignoring its last byte (getKey() bumps only that byte per 1-byte index).
+    if (key.length == (int)sizeof(defaultpsk) && memcmp(key.bytes, defaultpsk, sizeof(defaultpsk) - 1) == 0)
+        return true;
+    return false;
+}
+
+bool Channels::usesPublicKey(ChannelIndex chIndex)
+{
+    const meshtastic_Channel &ch = getByIndex(chIndex);
+    if (!ch.has_settings || ch.role == meshtastic_Channel_Role_DISABLED)
+        return false;
+
+    const auto &psk = ch.settings.psk;
+    if (psk.size == 0) {
+        // Secondary channels inherit the primary key when unset; primary size==0 means encryption disabled.
+        if (ch.role == meshtastic_Channel_Role_SECONDARY) {
+            // Guard against malformed configs with no PRIMARY channel (primaryIndex could point back to us).
+            if (primaryIndex == chIndex)
+                return true; // fail closed: treat as public
+            return usesPublicKey(primaryIndex);
+        }
+        return true;
+    }
+
+    if (psk.size == 1) {
+        // Short PSK aliases: 0 disables encryption; 1..255 are the public defaultpsk family.
+        return true;
+    }
+
+    return (psk.size == sizeof(defaultpsk) && memcmp(psk.bytes, defaultpsk, sizeof(defaultpsk) - 1) == 0);
+}
+
 bool Channels::hasDefaultChannel()
 {
     // If we don't use a preset or the default frequency slot, or we override the frequency, we don't have a default channel

src/mesh/Channels.h

@@ -86,6 +86,9 @@ class Channels
     // Returns true if the channel has the default name and PSK
     bool isDefaultChannel(ChannelIndex chIndex);
 
+    // Returns true if this channel's effective key is publicly decryptable (open or well-known/default PSK).
+    bool usesPublicKey(ChannelIndex chIndex);
+
     // Returns true if we can be reached via a channel with the default settings given a region and modem preset
     bool hasDefaultChannel();
 
@@ -144,6 +147,9 @@ extern Channels channels;
 static const uint8_t defaultpsk[] = {0xd4, 0xf1, 0xbb, 0x3a, 0x20, 0x29, 0x07, 0x59,
                                      0xf0, 0xbc, 0xff, 0xab, 0xcf, 0x4e, 0x69, 0x01};
 
+/// True if a getKey()-resolved key offers no privacy: length 0 (off) or the public defaultpsk family. Pure; for tests.
+bool cryptoKeyIsPublic(const CryptoKey &key);
+
 static const uint8_t eventpsk[] = {0x38, 0x4b, 0xbc, 0xc0, 0x1d, 0xc0, 0x22, 0xd1, 0x81, 0xbf, 0x36,
                                    0xb8, 0x61, 0x21, 0xe1, 0xfb, 0x96, 0xb7, 0x2e, 0x55, 0xbf, 0x74,
                                    0x22, 0x7e, 0x9d, 0x6a, 0xfb, 0x48, 0xd6, 0x4c, 0xb1, 0xa1};

src/mesh/PositionPrecision.cpp

@@ -16,7 +16,16 @@ uint32_t getPositionPrecisionForChannel(const meshtastic_Channel &channel)
 
 uint32_t getPositionPrecisionForChannel(uint8_t channelIndex)
 {
-    return getPositionPrecisionForChannel(channels.getByIndex(channelIndex));
+    const meshtastic_Channel &ch = channels.getByIndex(channelIndex);
+    if (ch.role == meshtastic_Channel_Role_DISABLED)
+        return 0;
+    uint32_t precision = getPositionPrecisionForChannel(ch);
+
+    // Never send a precise position on a publicly-decryptable channel (key check is gated on > ceiling).
+    if (precision > MAX_POSITION_PRECISION_PUBLIC_KEY && channels.usesPublicKey(channelIndex)) {
+        precision = MAX_POSITION_PRECISION_PUBLIC_KEY;
+    }
+    return precision;
 }
 
 static int32_t truncateCoordinate(int32_t coordinate, uint32_t precision)

src/mesh/PositionPrecision.h

@@ -4,7 +4,16 @@
 #include "meshtastic/mesh.pb.h"
 #include <stdint.h>
 
+// Max precision on a publicly-decryptable channel. CCPA "precise geolocation" = within a ~564m (1,850ft) radius.
+// Precision is bit-truncation of latitude_i/longitude_i: the latitude cell stays ~constant in meters worldwide
+// (~700m at 15 bits), while only the longitude cell varies — widest at the equator, narrowing toward the poles.
+// 15 also matches the MQTT map-report public precision ceiling.
+#define MAX_POSITION_PRECISION_PUBLIC_KEY 15
+
+// Configured precision as-is; does NOT apply the public-key clamp -- use the channelIndex overload for the on-wire value.
 uint32_t getPositionPrecisionForChannel(const meshtastic_Channel &channel);
+
+// Configured precision, clamped to MAX_POSITION_PRECISION_PUBLIC_KEY when the channel's effective key is publicly decryptable.
 uint32_t getPositionPrecisionForChannel(uint8_t channelIndex);
 void applyPositionPrecision(meshtastic_Position &position, uint32_t precision);
 bool applyPositionPrecision(meshtastic_MeshPacket &packet, uint32_t precision);

src/modules/AdminModule.cpp

@@ -2,6 +2,7 @@
 #include "Channels.h"
 #include "MeshService.h"
 #include "NodeDB.h"
+#include "PositionPrecision.h"
 #include "PowerFSM.h"
 #include "RTC.h"
 #include "SPILock.h"
@@ -1157,7 +1158,26 @@ void AdminModule::handleSetChannel(const meshtastic_Channel &cc)
     if (channels.ensureLicensedOperation()) {
         sendWarning(licensedModeMessage);
     }
+    // Refresh derived state (primaryIndex in particular) BEFORE the precision clamp below. usesPublicKey()
+    // resolves a secondary channel's key against the primary, so it must see the post-update primaryIndex;
+    // running the clamp first could evaluate secondaries against the previous primary and skip the clamp/warning.
     channels.onConfigChanged(); // tell the radios about this change
+
+    // Persist the public-key precision clamp for all channels that may be affected (e.g. secondaries
+    // that inherit a now-public primary key) and warn the client once if anything was coarsened.
+    bool clamped = false;
+    for (uint8_t i = 0; i < channels.getNumChannels(); i++) {
+        meshtastic_Channel &ch = channels.getByIndex(i);
+        if (ch.role == meshtastic_Channel_Role_DISABLED || !ch.settings.has_module_settings)
+            continue;
+        uint32_t allowed = getPositionPrecisionForChannel(i);
+        if (allowed != ch.settings.module_settings.position_precision) {
+            ch.settings.module_settings.position_precision = allowed;
+            clamped = true;
+        }
+    }
+    if (clamped)
+        sendWarning(publicChannelPrecisionMessage);
     saveChanges(SEGMENT_CHANNELS, false);
 }
 

src/modules/AdminModule.h

@@ -88,6 +88,9 @@ class AdminModule : public ProtobufModule<meshtastic_AdminMessage>, public Obser
 static constexpr const char *licensedModeMessage =
     "Licensed mode activated, removing admin channel and encryption from all channels";
 
+static constexpr const char *publicChannelPrecisionMessage =
+    "Precise position is not allowed on a public (open / known-key) channel; reduced to coarse precision";
+
 extern AdminModule *adminModule;
 
 void disableBluetooth();

test/test_position_precision/test_main.cpp

@@ -1,6 +1,8 @@
+#include "Channels.h"
 #include "PositionPrecision.h"
 #include "TestUtil.h"
 #include "mesh-pb-constants.h"
+#include <cstring>
 #include <unity.h>
 
 static meshtastic_Position makePosition()
@@ -119,6 +121,92 @@ static void test_getPositionPrecisionForChannel_secondaryWithoutModuleSettingsFa
     TEST_ASSERT_EQUAL_UINT32(0, getPositionPrecisionForChannel(channel));
 }
 
+// End-to-end via the channelIndex overload + live channels singleton, exercising getKey()'s 1-byte->16-byte expansion.
+static void test_getPositionPrecisionForChannel_clampsPreciseOnDefaultKeyChannel()
+{
+    channels.initDefaults(); // channel 0: primary, default key (psk {0x01}) -> publicly decryptable
+    uint8_t idx = 0;
+    meshtastic_Channel &ch = channels.getByIndex(idx);
+    ch.settings.has_module_settings = true;
+    ch.settings.module_settings.position_precision = 32; // user requests "Precise" on a public channel
+
+    TEST_ASSERT_EQUAL_UINT32(MAX_POSITION_PRECISION_PUBLIC_KEY, getPositionPrecisionForChannel(idx));
+}
+
+static void test_getPositionPrecisionForChannel_keepsPreciseOnStrongKeyChannel()
+{
+    channels.initDefaults();
+    uint8_t idx = 0;
+    meshtastic_Channel &ch = channels.getByIndex(idx);
+    memset(ch.settings.psk.bytes, 0xAB, 16); // a private 128-bit key, not the defaultpsk family
+    ch.settings.psk.size = 16;
+    ch.settings.has_module_settings = true;
+    ch.settings.module_settings.position_precision = 32;
+
+    TEST_ASSERT_EQUAL_UINT32(32, getPositionPrecisionForChannel(idx));
+}
+
+static CryptoKey makeCryptoKey(const uint8_t *bytes, int length)
+{
+    CryptoKey k;
+    memset(k.bytes, 0, sizeof(k.bytes));
+
+    // CryptoKey::length is int8_t and CryptoKey::bytes is 32 bytes; keep the helper consistent and overflow-safe.
+    int cappedLen = length;
+    if (cappedLen < 0)
+        cappedLen = -1;
+    else if (cappedLen > static_cast<int>(sizeof(k.bytes)))
+        cappedLen = static_cast<int>(sizeof(k.bytes));
+
+    if (cappedLen > 0 && bytes != nullptr) {
+        memcpy(k.bytes, bytes, static_cast<size_t>(cappedLen));
+    }
+
+    k.length = static_cast<int8_t>(cappedLen);
+    return k;
+}
+
+static void test_cryptoKeyIsPublic_openKeyIsPublic()
+{
+    // length 0 == encryption disabled.
+    TEST_ASSERT_TRUE(cryptoKeyIsPublic(makeCryptoKey(nullptr, 0)));
+}
+
+static void test_cryptoKeyIsPublic_defaultKeyIsPublic()
+{
+    // The expanded default PSK (the 16-byte defaultpsk) -- the case a key-length check misses.
+    TEST_ASSERT_TRUE(cryptoKeyIsPublic(makeCryptoKey(defaultpsk, sizeof(defaultpsk))));
+}
+
+static void test_cryptoKeyIsPublic_defaultKeyFamilyVariesLastByte()
+{
+    // Higher indices (e.g. {0x02}) expand to defaultpsk with only the last byte bumped -- still public.
+    uint8_t key[sizeof(defaultpsk)];
+    memcpy(key, defaultpsk, sizeof(defaultpsk));
+    key[sizeof(defaultpsk) - 1] = static_cast<uint8_t>(key[sizeof(defaultpsk) - 1] + 1);
+    TEST_ASSERT_TRUE(cryptoKeyIsPublic(makeCryptoKey(key, sizeof(key))));
+}
+
+static void test_cryptoKeyIsPublic_strongKeyIsPrivate()
+{
+    uint8_t key[16];
+    memset(key, 0xAB, sizeof(key)); // not the defaultpsk family
+    TEST_ASSERT_FALSE(cryptoKeyIsPublic(makeCryptoKey(key, sizeof(key))));
+}
+
+static void test_cryptoKeyIsPublic_aes256KeyIsPrivate()
+{
+    uint8_t key[32];
+    memset(key, 0x11, sizeof(key));
+    TEST_ASSERT_FALSE(cryptoKeyIsPublic(makeCryptoKey(key, sizeof(key))));
+}
+
+static void test_cryptoKeyIsPublic_invalidKeyIsNotPublic()
+{
+    // length < 0 == no/invalid key (e.g. a disabled channel); it carries no traffic to leak.
+    TEST_ASSERT_FALSE(cryptoKeyIsPublic(makeCryptoKey(nullptr, -1)));
+}
+
 void setUp(void) {}
 
 void tearDown(void) {}
@@ -136,6 +224,14 @@ void setup()
     RUN_TEST(test_getPositionPrecisionForChannel_explicitZeroDisablesPrimary);
     RUN_TEST(test_getPositionPrecisionForChannel_primaryWithoutModuleSettingsFailsClosed);
     RUN_TEST(test_getPositionPrecisionForChannel_secondaryWithoutModuleSettingsFailsClosed);
+    RUN_TEST(test_getPositionPrecisionForChannel_clampsPreciseOnDefaultKeyChannel);
+    RUN_TEST(test_getPositionPrecisionForChannel_keepsPreciseOnStrongKeyChannel);
+    RUN_TEST(test_cryptoKeyIsPublic_openKeyIsPublic);
+    RUN_TEST(test_cryptoKeyIsPublic_defaultKeyIsPublic);
+    RUN_TEST(test_cryptoKeyIsPublic_defaultKeyFamilyVariesLastByte);
+    RUN_TEST(test_cryptoKeyIsPublic_strongKeyIsPrivate);
+    RUN_TEST(test_cryptoKeyIsPublic_aes256KeyIsPrivate);
+    RUN_TEST(test_cryptoKeyIsPublic_invalidKeyIsNotPublic);
     exit(UNITY_END());
 }