AdminModule: only accept admin responses to requests we sent

src/mesh/MeshService.cpp

@@ -17,6 +17,7 @@
 #include "main.h"
 #include "mesh-pb-constants.h"
 #include "meshUtils.h"
+#include "modules/AdminModule.h"
 #include "modules/NodeInfoModule.h"
 #include "modules/PositionModule.h"
 #include "modules/RoutingModule.h"
@@ -259,6 +260,14 @@ void MeshService::handleToRadio(meshtastic_MeshPacket &p)
                   const StoredMessage &sm = messageStore.addFromPacket(p);
                   graphics::MessageRenderer::handleNewMessage(nullptr, sm, p); // notify UI
               })
+#if !MESHTASTIC_EXCLUDE_ADMIN
+    // Note admin requests on their way out: AdminModule only accepts a response from a remote we
+    // actually asked. Runs before encryption, while the payload is still readable.
+    if (adminModule && p.which_payload_variant == meshtastic_MeshPacket_decoded_tag &&
+        p.decoded.portnum == meshtastic_PortNum_ADMIN_APP)
+        adminModule->noteOutgoingAdminRequest(p);
+#endif
+
     // Send the packet into the mesh
     DEBUG_HEAP_BEFORE;
     auto a = packetPool.allocCopy(p);

src/modules/AdminModule.cpp

@@ -10,6 +10,7 @@
 #include "input/InputBroker.h"
 #include "meshUtils.h"
 #include <FSCommon.h>
+#include <Throttle.h>
 #include <ctype.h> // for better whitespace handling
 #if defined(ARCH_ESP32) && !MESHTASTIC_EXCLUDE_WIFI
 #include "MeshtasticOTA.h"
@@ -125,9 +126,13 @@ bool AdminModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, meshta
     }
 #endif
     meshtastic_Channel *ch = &channels.getByIndex(mp.channel);
-    // Could tighten this up further by tracking the last public_key we went an AdminMessage request to
-    // and only allowing responses from that remote.
     if (messageIsResponse(r)) {
+        // Only accept a response from a remote we sent the matching request to. from == 0 is a
+        // local client, which PhoneAPI has already gated.
+        if (mp.from != 0 && !responseIsSolicited(mp, r->which_payload_variant)) {
+            LOG_INFO("Ignore admin response from 0x%08x, no outstanding request", mp.from);
+            return handled;
+        }
         LOG_DEBUG("Allow admin response message");
     } else if (mp.from == 0) {
         // Local admin from a BLE/USB/TCP client. from == 0 cannot arrive from the
@@ -472,8 +477,9 @@ bool AdminModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, meshta
     }
     case meshtastic_AdminMessage_get_module_config_response_tag: {
         LOG_INFO("Client received a get_module_config response");
-        if (fromOthers && r->get_module_config_response.which_payload_variant ==
-                              meshtastic_AdminMessage_ModuleConfigType_REMOTEHARDWARE_CONFIG) {
+        // which_payload_variant is the ModuleConfig oneof tag, so compare against that tag, not the
+        // AdminMessage ModuleConfigType enum (whose REMOTEHARDWARE value is a different number).
+        if (fromOthers && r->get_module_config_response.which_payload_variant == meshtastic_ModuleConfig_remote_hardware_tag) {
             handleGetModuleConfigResponse(mp, r);
         }
         break;
@@ -1821,6 +1827,130 @@ bool AdminModule::messageIsResponse(const meshtastic_AdminMessage *r)
         return false;
 }
 
+// The response variant that answers a getter request, or 0 if the request has none.
+static pb_size_t adminResponseForRequest(pb_size_t requestVariant)
+{
+    switch (requestVariant) {
+    case meshtastic_AdminMessage_get_channel_request_tag:
+        return meshtastic_AdminMessage_get_channel_response_tag;
+    case meshtastic_AdminMessage_get_owner_request_tag:
+        return meshtastic_AdminMessage_get_owner_response_tag;
+    case meshtastic_AdminMessage_get_config_request_tag:
+        return meshtastic_AdminMessage_get_config_response_tag;
+    case meshtastic_AdminMessage_get_module_config_request_tag:
+        return meshtastic_AdminMessage_get_module_config_response_tag;
+    case meshtastic_AdminMessage_get_canned_message_module_messages_request_tag:
+        return meshtastic_AdminMessage_get_canned_message_module_messages_response_tag;
+    case meshtastic_AdminMessage_get_device_metadata_request_tag:
+        return meshtastic_AdminMessage_get_device_metadata_response_tag;
+    case meshtastic_AdminMessage_get_ringtone_request_tag:
+        return meshtastic_AdminMessage_get_ringtone_response_tag;
+    case meshtastic_AdminMessage_get_device_connection_status_request_tag:
+        return meshtastic_AdminMessage_get_device_connection_status_response_tag;
+    case meshtastic_AdminMessage_get_node_remote_hardware_pins_request_tag:
+        return meshtastic_AdminMessage_get_node_remote_hardware_pins_response_tag;
+    case meshtastic_AdminMessage_get_ui_config_request_tag:
+        return meshtastic_AdminMessage_get_ui_config_response_tag;
+    default:
+        return 0;
+    }
+}
+
+// A one-bit identifier for a response variant, so several expected responses pack into a mask.
+static uint32_t adminResponseBit(pb_size_t responseVariant)
+{
+    switch (responseVariant) {
+    case meshtastic_AdminMessage_get_channel_response_tag:
+        return 1u << 0;
+    case meshtastic_AdminMessage_get_owner_response_tag:
+        return 1u << 1;
+    case meshtastic_AdminMessage_get_config_response_tag:
+        return 1u << 2;
+    case meshtastic_AdminMessage_get_module_config_response_tag:
+        return 1u << 3;
+    case meshtastic_AdminMessage_get_canned_message_module_messages_response_tag:
+        return 1u << 4;
+    case meshtastic_AdminMessage_get_device_metadata_response_tag:
+        return 1u << 5;
+    case meshtastic_AdminMessage_get_ringtone_response_tag:
+        return 1u << 6;
+    case meshtastic_AdminMessage_get_device_connection_status_response_tag:
+        return 1u << 7;
+    case meshtastic_AdminMessage_get_node_remote_hardware_pins_response_tag:
+        return 1u << 8;
+    case meshtastic_AdminMessage_get_ui_config_response_tag:
+        return 1u << 9;
+    default:
+        return 0;
+    }
+}
+
+void AdminModule::noteOutgoingAdminRequest(const meshtastic_MeshPacket &p)
+{
+    if (p.which_payload_variant != meshtastic_MeshPacket_decoded_tag || p.decoded.portnum != meshtastic_PortNum_ADMIN_APP)
+        return;
+    // Local admin is answered in-process, and a broadcast is never a request to one remote.
+    if (p.to == 0 || isBroadcast(p.to) || p.to == nodeDB->getNodeNum())
+        return;
+
+    meshtastic_AdminMessage admin = meshtastic_AdminMessage_init_zero;
+    if (!pb_decode_from_bytes(p.decoded.payload.bytes, p.decoded.payload.size, &meshtastic_AdminMessage_msg, &admin))
+        return;
+    const uint32_t bit = adminResponseBit(adminResponseForRequest(admin.which_payload_variant));
+    if (!bit)
+        return; // not a getter whose response we can pair
+
+    // Reuse this remote's slot if it has one, else a free slot, else the oldest.
+    OutstandingAdminRequest *slot = nullptr;
+    for (auto &o : outstandingAdminRequests)
+        if (o.to == p.to)
+            slot = &o;
+    const bool sameNode = slot != nullptr;
+    if (!slot)
+        for (auto &o : outstandingAdminRequests)
+            if (o.to == 0) {
+                slot = &o;
+                break;
+            }
+    if (!slot) {
+        slot = &outstandingAdminRequests[0];
+        for (auto &o : outstandingAdminRequests)
+            if (o.sentAtMs < slot->sentAtMs)
+                slot = &o;
+    }
+
+    slot->to = p.to;
+    slot->sentAtMs = millis();
+    // Keep the types already pending for this node so a client may pipeline several to it.
+    slot->expectedResponses = sameNode ? (slot->expectedResponses | bit) : bit;
+    slot->keyValid = p.pki_encrypted && p.public_key.size == 32;
+    if (slot->keyValid)
+        memcpy(slot->key, p.public_key.bytes, 32);
+    else
+        memset(slot->key, 0, 32);
+    LOG_DEBUG("Admin request sent to 0x%08x, expecting its response", p.to);
+}
+
+bool AdminModule::responseIsSolicited(const meshtastic_MeshPacket &mp, pb_size_t responseVariant)
+{
+    const uint32_t bit = adminResponseBit(responseVariant);
+    for (auto &o : outstandingAdminRequests) {
+        if (o.to == 0 || o.to != mp.from)
+            continue;
+        if (!Throttle::isWithinTimespanMs(o.sentAtMs, kOutstandingAdminRequestMs)) {
+            o.to = 0; // lapsed; free the slot
+            return false;
+        }
+        if (!(o.expectedResponses & bit))
+            return false; // we queried this node, but not for this
+        // A request the client pinned to a key must be answered over PKC by that same key.
+        if (o.keyValid && (!mp.pki_encrypted || mp.public_key.size != 32 || memcmp(mp.public_key.bytes, o.key, 32) != 0))
+            return false;
+        return true;
+    }
+    return false;
+}
+
 bool AdminModule::messageIsRequest(const meshtastic_AdminMessage *r)
 {
     if (r->which_payload_variant == meshtastic_AdminMessage_get_channel_request_tag ||

src/modules/AdminModule.h

@@ -77,7 +77,27 @@ class AdminModule : public ProtobufModule<meshtastic_AdminMessage>, public Obser
   public:
     void handleSetHamMode(const meshtastic_HamParameters &req);
 
+    /// Note an admin request leaving this node for a remote, so that remote's response is
+    /// accepted. Called from the client-to-mesh path (MeshService::handleToRadio).
+    void noteOutgoingAdminRequest(const meshtastic_MeshPacket &p);
+
   private:
+    // An admin response carries no session passkey and its sender is not an admin-key holder, so
+    // the only thing vouching for one is a request we sent. Track those per remote: a client may
+    // have several nodes in flight, and each keeps answering until the window lapses.
+    static constexpr size_t kOutstandingAdminRequests = 3;
+    static constexpr uint32_t kOutstandingAdminRequestMs = 300 * 1000; // same window as the session passkey
+    struct OutstandingAdminRequest {
+        NodeNum to;                 // 0 = free slot
+        uint32_t sentAtMs;          // millis() when the most recent request to this node went out
+        uint32_t expectedResponses; // bitmask of response variants these requests authorize
+        uint8_t key[32];            // destination key, when the client pinned one
+        bool keyValid;
+    };
+    OutstandingAdminRequest outstandingAdminRequests[kOutstandingAdminRequests] = {};
+
+    /// Whether a response of variant responseVariant from mp.from answers a request we sent it.
+    bool responseIsSolicited(const meshtastic_MeshPacket &mp, pb_size_t responseVariant);
     void handleStoreDeviceUIConfig(const meshtastic_DeviceUIConfig &uicfg);
     void handleSendInputEvent(const meshtastic_AdminMessage_InputEvent &inputEvent);
     void reboot(int32_t seconds);

test/support/AdminModuleTestShim.h

@@ -12,6 +12,7 @@ class AdminModuleTestShim : public AdminModule
     using AdminModule::handleReceivedProtobuf;
     using AdminModule::handleSetConfig;
     using AdminModule::handleSetModuleConfig;
+    using AdminModule::responseIsSolicited; // request/response pairing gate
     using AdminModule::setPassKey;
 
     // Peek at the reply a handler queued, before drainReply() releases it.