Thomas GöttgensandGitHub
8147970957
* AdminModule: only accept admin responses to requests we sent An admin *_response short-circuited the auth and session-passkey checks that gate every other admin message, so any node could deliver one. On a channel the module listens to unauthenticated, a get_module_config_response drives the remote-hardware pin handler with attacker-supplied values. Track the destination of outgoing admin requests (per remote, with the pinned PKC key when there is one) and accept a response only from a node with a matching outstanding request, inside the same window as the session passkey. Local (from == 0) admin is unchanged; PhoneAPI already gates it. Also fix the response dispatch: get_module_config_response.which_payload_variant is a ModuleConfig oneof tag, but it was compared against the AdminMessage ModuleConfigType enum (different numbering), so the handler never ran. Compare against the oneof tag. * AdminModule: rollover-safe request window, bind response to request type Two review refinements to the request/response pairing: Use Throttle::isWithinTimespanMs for the outstanding-request expiry instead of comparing millis()/1000 sums, which mis-expired across the millis() rollover. Bind each accepted response to a request type actually sent to that node. Each outstanding record now carries a bitmask of the response variants its requests authorize, so a get_owner request no longer admits a get_module_config response. The mask accumulates per remote, so a client may still pipeline several request types to one node and have every answer accepted. * AdminModule: track admin requests per-request, not per-node Reworks the outstanding-request table so each request is its own entry with its own expiry window and pinned key, replacing the per-node bitmask that shared one timestamp and one key across every response variant. That sharing let a later request to the same node extend an earlier one's window and, worse, clear its PKC pin: an unpinned request cleared keyValid, so a plaintext response to an earlier PKC-pinned request was then accepted. Per-request entries keep each pin intact. Identical requests are de-duplicated (a client may fetch several config subtypes, all answered by one response variant) and eviction compares elapsed time, which is rollover-safe. Test: a pinned request's response still requires its key after an unpinned request to the same node. * AdminModule: match module-config subtype and consume answered requests Two refinements to the request/response pairing: Only remote_hardware get_module_config_response mutates state (the pin table), so it must answer a request for that exact ModuleConfigType, not just any module-config request. Each entry records the requested subtype and the gate checks it. A matched request is now consumed on accept, so a node cannot replay a state-mutating response within the window. Because one request yields one response, request de-dup is dropped (a client's N indexed get_channel requests are N entries, each consumed once). Tests: a non-remote-hardware request does not admit a remote_hardware response, and a second copy of an answered response is rejected.
33 lines
1.2 KiB
C++
33 lines
1.2 KiB
C++
#pragma once
|
|
// The one class AdminModule.h befriends (test seam): exposes the protected/private handlers to the
|
|
// native suites and defers persistence so setters exercise in-RAM logic without disk/reboot effects.
|
|
#include "MeshTypes.h" // packetPool
|
|
#include "modules/AdminModule.h"
|
|
|
|
class AdminModuleTestShim : public AdminModule
|
|
{
|
|
public:
|
|
using AdminModule::checkPassKey; // session-key gate seam (see test_admin_session_repro)
|
|
using AdminModule::handleGetConfig;
|
|
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.
|
|
meshtastic_MeshPacket *reply() { return myReply; }
|
|
|
|
// With an "open edit transaction" saveChanges() is a pure no-op: no reloadConfig/saveToDisk/reboot.
|
|
void deferSaves() { hasOpenEditTransaction = true; }
|
|
|
|
// Setters may allocate an error reply from packetPool; drain it each iteration or the pool leaks.
|
|
void drainReply()
|
|
{
|
|
if (myReply) {
|
|
packetPool.release(myReply);
|
|
myReply = nullptr;
|
|
}
|
|
}
|
|
};
|