Uplink DMs not to us if MQTT encryption enabled (#5025)

* Uplink DMs not to us if MQTT encryption enabled * Only really need to try uplinking encrypted packet if MQTT encryption is enabled * Add log about publishing nothing when packet is not decrypted and encryption_enabled is false * Improve comment
meshtastic · Oct 10, 2024 · d55c08d · d55c08d
1 parent e8f287a
commit d55c08d
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 16 deletions.
diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp
@@ -613,8 +613,12 @@ void Router::handleReceived(meshtastic_MeshPacket *p, RxSource src)
         MeshModule::callModules(*p, src);
 
 #if !MESHTASTIC_EXCLUDE_MQTT
+        // Mark as pki_encrypted if it is not yet decoded and MQTT encryption is also enabled, hash matches and it's a DM not to
+        // us (because we would be able to decrypt it)
+        if (!decoded && moduleConfig.mqtt.encryption_enabled && p->channel == 0x00 && p->to != NODENUM_BROADCAST && !isToUs(p))
+            p_encrypted->pki_encrypted = true;
         // After potentially altering it, publish received message to MQTT if we're not the original transmitter of the packet
-        if ((decoded || (p->channel == 0x00 && p->to != NODENUM_BROADCAST)) && moduleConfig.mqtt.enabled && !isFromUs(p) && mqtt)
+        if ((decoded || p_encrypted->pki_encrypted) && moduleConfig.mqtt.enabled && !isFromUs(p) && mqtt)
             mqtt->onSend(*p_encrypted, *p, p->channel);
 #endif
     }

diff --git a/src/mqtt/MQTT.cpp b/src/mqtt/MQTT.cpp
@@ -524,9 +524,9 @@ void MQTT::publishQueuedMessages()
     }
 }
 
-void MQTT::onSend(const meshtastic_MeshPacket &mp, const meshtastic_MeshPacket &mp_decoded, ChannelIndex chIndex)
+void MQTT::onSend(const meshtastic_MeshPacket &mp_encrypted, const meshtastic_MeshPacket &mp_decoded, ChannelIndex chIndex)
 {
-    if (mp.via_mqtt)
+    if (mp_encrypted.via_mqtt)
         return; // Don't send messages that came from MQTT back into MQTT
     bool uplinkEnabled = false;
     for (int i = 0; i <= 7; i++) {
@@ -537,12 +537,8 @@ void MQTT::onSend(const meshtastic_MeshPacket &mp, const meshtastic_MeshPacket &
         return; // no channels have an uplink enabled
     auto &ch = channels.getByIndex(chIndex);
 
-    if (!mp.pki_encrypted) {
-        if (mp_decoded.which_payload_variant != meshtastic_MeshPacket_decoded_tag) {
-            LOG_CRIT("MQTT::onSend(): mp_decoded isn't actually decoded\n");
-            return;
-        }
-
+    // mp_decoded will not be decoded when it's PKI encrypted and not directed to us
+    if (mp_decoded.which_payload_variant == meshtastic_MeshPacket_decoded_tag) {
         // check for the lowest bit of the data bitfield set false, and the use of one of the default keys.
         if (!isFromUs(&mp_decoded) && strcmp(moduleConfig.mqtt.address, "127.0.0.1") != 0 && mp_decoded.decoded.has_bitfield &&
             !(mp_decoded.decoded.bitfield & BITFIELD_OK_TO_MQTT_MASK) &&
@@ -559,21 +555,26 @@ void MQTT::onSend(const meshtastic_MeshPacket &mp, const meshtastic_MeshPacket &
             return;
         }
     }
-    if (mp.pki_encrypted || ch.settings.uplink_enabled) {
-        const char *channelId = mp.pki_encrypted ? "PKI" : channels.getGlobalId(chIndex);
+    // Either encrypted packet (we couldn't decrypt) is marked as pki_encrypted, or we could decode the PKI encrypted packet
+    bool isPKIEncrypted = mp_encrypted.pki_encrypted || mp_decoded.pki_encrypted;
+    // If it was to a channel, check uplink enabled, else must be pki_encrypted
+    if ((ch.settings.uplink_enabled && !isPKIEncrypted) || isPKIEncrypted) {
+        const char *channelId = isPKIEncrypted ? "PKI" : channels.getGlobalId(chIndex);
 
         meshtastic_ServiceEnvelope *env = mqttPool.allocZeroed();
         env->channel_id = (char *)channelId;
         env->gateway_id = owner.id;
 
         LOG_DEBUG("MQTT onSend - Publishing ");
         if (moduleConfig.mqtt.encryption_enabled) {
-            env->packet = (meshtastic_MeshPacket *)&mp;
+            env->packet = (meshtastic_MeshPacket *)&mp_encrypted;
             LOG_DEBUG("encrypted message\n");
-        } else if (mp_decoded.which_payload_variant ==
-                   meshtastic_MeshPacket_decoded_tag) { // Don't upload a still-encrypted PKI packet
+        } else if (mp_decoded.which_payload_variant == meshtastic_MeshPacket_decoded_tag) {
             env->packet = (meshtastic_MeshPacket *)&mp_decoded;
             LOG_DEBUG("portnum %i message\n", env->packet->decoded.portnum);
+        } else {
+            LOG_DEBUG("nothing, pkt not decrypted\n");
+            return; // Don't upload a still-encrypted PKI packet if not encryption_enabled
         }
 
         if (moduleConfig.mqtt.proxy_to_client_enabled || this->isConnectedDirectly()) {

diff --git a/src/mqtt/MQTT.h b/src/mqtt/MQTT.h
@@ -52,14 +52,14 @@ class MQTT : private concurrency::OSThread
 
     /**
      * Publish a packet on the global MQTT server.
-     * @param mp the encrypted packet to publish
+     * @param mp_encrypted the encrypted packet to publish
      * @param mp_decoded the decrypted packet to publish
      * @param chIndex the index of the channel for this message
      *
      * Note: for messages we are forwarding on the mesh that we can't find the channel for (because we don't have the keys), we
      * can not forward those messages to the cloud - because no way to find a global channel ID.
      */
-    void onSend(const meshtastic_MeshPacket &mp, const meshtastic_MeshPacket &mp_decoded, ChannelIndex chIndex);
+    void onSend(const meshtastic_MeshPacket &mp_encrypted, const meshtastic_MeshPacket &mp_decoded, ChannelIndex chIndex);
 
     /** Attempt to connect to server if necessary
      */