Merge branch 'develop' into codex/packet-auth-policy
This commit is contained in:
+29
-22
@@ -1013,6 +1013,34 @@ static bool signedDataFits(meshtastic_Data *d)
|
||||
}
|
||||
#endif
|
||||
|
||||
#if !(MESHTASTIC_EXCLUDE_PKI)
|
||||
bool wouldEncryptWithPKC(const meshtastic_MeshPacket *p, ChannelIndex chIndex, bool haveDestKey)
|
||||
{
|
||||
// First, only PKC encrypt packets we are originating
|
||||
return isFromUs(p) &&
|
||||
#if ARCH_PORTDUINO
|
||||
// Sim radio via the cli flag skips PKC
|
||||
!portduino_config.force_simradio &&
|
||||
#endif
|
||||
// Don't use PKC with Ham mode
|
||||
!owner.is_licensed &&
|
||||
// Don't use PKC on 'serial' or 'gpio' channels unless explicitly requested
|
||||
!(p->pki_encrypted != true && (strcasecmp(channels.getName(chIndex), Channels::serialChannel) == 0 ||
|
||||
strcasecmp(channels.getName(chIndex), Channels::gpioChannel) == 0)) &&
|
||||
// Check for valid keys and single node destination
|
||||
config.security.private_key.size == 32 && !isBroadcast(p->to) &&
|
||||
// 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 &&
|
||||
// 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.
|
||||
// Though possible the first packet each direction should go non-pkc
|
||||
// to handle the case where the remote node has our key, but we don't have theirs.
|
||||
!(p->decoded.portnum == meshtastic_PortNum_KEY_VERIFICATION_APP && !haveDestKey);
|
||||
}
|
||||
#endif
|
||||
|
||||
/** Return 0 for success or a Routing_Error code for failure
|
||||
*/
|
||||
meshtastic_Routing_Error perhapsEncode(meshtastic_MeshPacket *p)
|
||||
@@ -1106,28 +1134,7 @@ meshtastic_Routing_Error perhapsEncode(meshtastic_MeshPacket *p)
|
||||
}
|
||||
// We may want to retool things so we can send a PKC packet when the client specifies a key and nodenum, even if the node
|
||||
// is not in the local nodedb
|
||||
// First, only PKC encrypt packets we are originating
|
||||
if (isFromUs(p) &&
|
||||
#if ARCH_PORTDUINO
|
||||
// Sim radio via the cli flag skips PKC
|
||||
!portduino_config.force_simradio &&
|
||||
#endif
|
||||
// Don't use PKC with Ham mode
|
||||
!owner.is_licensed &&
|
||||
// Don't use PKC on 'serial' or 'gpio' channels unless explicitly requested
|
||||
!(p->pki_encrypted != true && (strcasecmp(channels.getName(chIndex), Channels::serialChannel) == 0 ||
|
||||
strcasecmp(channels.getName(chIndex), Channels::gpioChannel) == 0)) &&
|
||||
// Check for valid keys and single node destination
|
||||
config.security.private_key.size == 32 && !isBroadcast(p->to) &&
|
||||
// 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 &&
|
||||
// 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.
|
||||
// Though possible the first packet each direction should go non-pkc
|
||||
// to handle the case where the remote node has our key, but we don't have theirs.
|
||||
!(p->decoded.portnum == meshtastic_PortNum_KEY_VERIFICATION_APP && !haveDestKey)) {
|
||||
if (wouldEncryptWithPKC(p, chIndex, haveDestKey)) {
|
||||
LOG_DEBUG("Use PKI!");
|
||||
if (numbytes + MESHTASTIC_HEADER_LENGTH + MESHTASTIC_PKC_OVERHEAD > MAX_LORA_PAYLOAD_LEN)
|
||||
return meshtastic_Routing_Error_TOO_LARGE;
|
||||
|
||||
@@ -191,6 +191,18 @@ meshtastic_Routing_Error perhapsEncode(meshtastic_MeshPacket *p);
|
||||
bool checkXeddsaReceivePolicy(meshtastic_MeshPacket *p);
|
||||
#endif
|
||||
|
||||
#if !(MESHTASTIC_EXCLUDE_PKI)
|
||||
/**
|
||||
* Would perhapsEncode() PKC-encrypt this outgoing packet? Callers that must know the encryption a
|
||||
* packet will get before it is encoded (e.g. pinning a peer key at request time) have to ask this
|
||||
* rather than inspect p, whose pki_encrypted/public_key fields are only populated on the RX path.
|
||||
*
|
||||
* @param chIndex the channel index p carries before encoding rewrites it to a hash.
|
||||
* @param haveDestKey whether a public key for p->to was resolvable.
|
||||
*/
|
||||
bool wouldEncryptWithPKC(const meshtastic_MeshPacket *p, ChannelIndex chIndex, bool haveDestKey);
|
||||
#endif
|
||||
|
||||
extern Router *router;
|
||||
|
||||
/// Generate a unique packet id
|
||||
|
||||
@@ -1985,7 +1985,15 @@ void AdminModule::noteOutgoingAdminRequest(const meshtastic_MeshPacket &p)
|
||||
if (!responseVariant)
|
||||
return; // not a getter whose response we can pair
|
||||
|
||||
const bool keyValid = p.pki_encrypted && p.public_key.size == 32;
|
||||
// Pin the key perhapsEncode will actually encrypt to, resolved the same way it resolves it.
|
||||
// p.public_key is NOT it: nothing populates that field on the outgoing path (only perhapsDecode
|
||||
// sets it, on RX), so reading it here pinned nothing and left `from` as the sole check.
|
||||
bool keyValid = false;
|
||||
meshtastic_NodeInfoLite_public_key_t destKey = {0, {0}};
|
||||
#if !(MESHTASTIC_EXCLUDE_PKI)
|
||||
const bool haveDestKey = nodeDB->copyPublicKey(p.to, destKey);
|
||||
keyValid = haveDestKey && wouldEncryptWithPKC(&p, p.channel, haveDestKey);
|
||||
#endif
|
||||
|
||||
// One entry per request (a client sends N indexed get_channel requests, each answered once, so
|
||||
// entries must not merge). Free slot, else evict the oldest by rollover-safe elapsed time.
|
||||
@@ -2004,6 +2012,7 @@ void AdminModule::noteOutgoingAdminRequest(const meshtastic_MeshPacket &p)
|
||||
}
|
||||
|
||||
slot->to = p.to;
|
||||
slot->requestId = p.id;
|
||||
slot->sentAtMs = millis();
|
||||
slot->expectedResponse = responseVariant;
|
||||
slot->moduleConfigType = admin.which_payload_variant == meshtastic_AdminMessage_get_module_config_request_tag
|
||||
@@ -2011,7 +2020,7 @@ void AdminModule::noteOutgoingAdminRequest(const meshtastic_MeshPacket &p)
|
||||
: 0;
|
||||
slot->keyValid = keyValid;
|
||||
if (keyValid)
|
||||
memcpy(slot->key, p.public_key.bytes, 32);
|
||||
memcpy(slot->key, destKey.bytes, 32);
|
||||
else
|
||||
memset(slot->key, 0, 32);
|
||||
LOG_DEBUG("Admin request sent to 0x%08x, expecting its response", p.to);
|
||||
@@ -2024,6 +2033,11 @@ bool AdminModule::responseIsSolicited(const meshtastic_MeshPacket &mp, pb_size_t
|
||||
for (auto &o : outstandingAdminRequests) {
|
||||
if (o.to != mp.from || o.expectedResponse != responseVariant)
|
||||
continue;
|
||||
// mp.from is unauthenticated, so also require the response to echo our request's packet id
|
||||
// (setReplyTo puts it in decoded.request_id). A blind injector must now guess it. Id 0 is
|
||||
// no token at all - an omitted request_id decodes to 0 - so such a slot never matches.
|
||||
if (o.requestId == 0 || mp.decoded.request_id != o.requestId)
|
||||
continue;
|
||||
if (!Throttle::isWithinTimespanMs(o.sentAtMs, kOutstandingAdminRequestMs)) {
|
||||
o.to = 0; // lapsed; free the slot and keep looking for another live match
|
||||
continue;
|
||||
|
||||
@@ -90,10 +90,11 @@ class AdminModule : public ProtobufModule<meshtastic_AdminMessage>, public Obser
|
||||
static constexpr uint32_t kOutstandingAdminRequestMs = 300 * 1000; // same window as the session passkey
|
||||
struct OutstandingAdminRequest {
|
||||
NodeNum to; // 0 = free slot
|
||||
uint32_t requestId; // our request's packet id; the response must echo it as request_id
|
||||
uint32_t sentAtMs; // millis() when this request went out
|
||||
pb_size_t expectedResponse; // the one response variant this request authorizes
|
||||
uint8_t moduleConfigType; // for get_module_config_request: which ModuleConfigType we asked
|
||||
uint8_t key[32]; // pinned destination key when the request went out over PKC
|
||||
uint8_t key[32]; // pinned destination key when the request goes out over PKC
|
||||
bool keyValid;
|
||||
};
|
||||
OutstandingAdminRequest outstandingAdminRequests[kOutstandingAdminRequests] = {};
|
||||
|
||||
@@ -29,8 +29,43 @@ static const uint8_t ADMIN_KEY[32] = {0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77,
|
||||
0xcc, 0xdd, 0xee, 0xff, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
|
||||
0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x20};
|
||||
|
||||
// MeshService assigns every outgoing packet an id before noteOutgoingAdminRequest sees it, and
|
||||
// setReplyTo echoes it back as decoded.request_id, so the pairing is keyed on it.
|
||||
static constexpr uint32_t REQUEST_ID = 0x5EED0001;
|
||||
static const uint8_t QUERIED_KEY[32] = {0xC1, 0xC2, 0xC3, 0xC4, 0xC5, 0xC6, 0xC7, 0xC8, 0xC9, 0xCa, 0xCb,
|
||||
0xCc, 0xCd, 0xCe, 0xCf, 0xD0, 0xD1, 0xD2, 0xD3, 0xD4, 0xD5, 0xD6,
|
||||
0xD7, 0xD8, 0xD9, 0xDa, 0xDb, 0xDc, 0xDd, 0xDe, 0xDf, 0xE0};
|
||||
|
||||
// NodeDB with injectable nodes, so a destination can have a stored public key to pin.
|
||||
class MockNodeDB : public NodeDB
|
||||
{
|
||||
public:
|
||||
void clearTestNodes()
|
||||
{
|
||||
testNodes.clear();
|
||||
meshNodes = &testNodes;
|
||||
numMeshNodes = 0;
|
||||
}
|
||||
|
||||
void addNodeWithKey(NodeNum num, const uint8_t *key)
|
||||
{
|
||||
meshtastic_NodeInfoLite n = meshtastic_NodeInfoLite_init_zero;
|
||||
n.num = num;
|
||||
if (key) {
|
||||
n.public_key.size = 32;
|
||||
memcpy(n.public_key.bytes, key, 32);
|
||||
}
|
||||
testNodes.push_back(n);
|
||||
meshNodes = &testNodes;
|
||||
numMeshNodes = testNodes.size();
|
||||
}
|
||||
|
||||
std::vector<meshtastic_NodeInfoLite> testNodes;
|
||||
};
|
||||
|
||||
static MockMeshService *mockService = nullptr;
|
||||
static AdminModuleTestShim *admin = nullptr;
|
||||
static MockNodeDB *mockNodeDB = nullptr;
|
||||
|
||||
// A remote, PKC-authorized set_owner. `session` (if non-empty) is the session_passkey the client presents.
|
||||
static meshtastic_MeshPacket makeRemoteSetOwner(const char *newLongName, const uint8_t *session, size_t sessionLen,
|
||||
@@ -71,6 +106,7 @@ static meshtastic_MeshPacket makeModuleConfigResponse(NodeNum from, meshtastic_A
|
||||
mp.to = LOCAL_NODE;
|
||||
mp.channel = 0;
|
||||
mp.which_payload_variant = meshtastic_MeshPacket_decoded_tag;
|
||||
mp.decoded.request_id = REQUEST_ID; // setReplyTo echoes the request's packet id
|
||||
return mp;
|
||||
}
|
||||
|
||||
@@ -93,6 +129,7 @@ static meshtastic_MeshPacket makeOutgoingRequest(NodeNum to, const meshtastic_Ad
|
||||
p.to = to;
|
||||
p.which_payload_variant = meshtastic_MeshPacket_decoded_tag;
|
||||
p.decoded.portnum = meshtastic_PortNum_ADMIN_APP;
|
||||
p.id = REQUEST_ID;
|
||||
p.decoded.payload.size =
|
||||
pb_encode_to_bytes(p.decoded.payload.bytes, sizeof(p.decoded.payload.bytes), &meshtastic_AdminMessage_msg, &req);
|
||||
return p;
|
||||
@@ -114,11 +151,16 @@ void setUp(void)
|
||||
admin = new AdminModuleTestShim();
|
||||
admin->deferSaves(); // no disk/reboot side effects when a setter is accepted
|
||||
|
||||
if (!nodeDB)
|
||||
nodeDB = new NodeDB();
|
||||
if (!mockNodeDB)
|
||||
mockNodeDB = new MockNodeDB();
|
||||
mockNodeDB->clearTestNodes();
|
||||
nodeDB = mockNodeDB;
|
||||
myNodeInfo.my_node_num = LOCAL_NODE;
|
||||
|
||||
config = meshtastic_LocalConfig_init_zero;
|
||||
// A real device always holds a private key; without one perhapsEncode never picks PKC.
|
||||
config.security.private_key.size = 32;
|
||||
memset(config.security.private_key.bytes, 0xA5, 32);
|
||||
// Authorize ADMIN_NODE's key as an admin key so the PKC path accepts it and we reach the session gate.
|
||||
config.security.admin_key[0].size = 32;
|
||||
memcpy(config.security.admin_key[0].bytes, ADMIN_KEY, 32);
|
||||
@@ -411,38 +453,104 @@ void test_response_variant_must_match_request(void)
|
||||
// must not relax the PKC pin of an earlier one (the old shared-slot model cleared it).
|
||||
void test_pinned_request_keeps_its_key_after_an_unpinned_request(void)
|
||||
{
|
||||
uint8_t key[32];
|
||||
memset(key, 0xC1, 32);
|
||||
// QUERIED has a stored key, so a request to it will be PKC-encrypted and pins that key.
|
||||
// STRANGER has none, so a request to it cannot be pinned.
|
||||
mockNodeDB->addNodeWithKey(QUERIED_NODE, QUERIED_KEY);
|
||||
mockNodeDB->addNodeWithKey(STRANGER_NODE, nullptr);
|
||||
|
||||
// A PKC-pinned get_config request to STRANGER (pins `key`).
|
||||
meshtastic_AdminMessage cfg = meshtastic_AdminMessage_init_zero;
|
||||
cfg.which_payload_variant = meshtastic_AdminMessage_get_config_request_tag;
|
||||
meshtastic_MeshPacket pinned = makeOutgoingRequest(STRANGER_NODE, cfg);
|
||||
pinned.pki_encrypted = true;
|
||||
pinned.public_key.size = 32;
|
||||
memcpy(pinned.public_key.bytes, key, 32);
|
||||
admin->noteOutgoingAdminRequest(pinned);
|
||||
admin->noteOutgoingAdminRequest(makeOutgoingRequest(QUERIED_NODE, cfg));
|
||||
|
||||
// A later, unpinned get_owner request to the same node.
|
||||
meshtastic_AdminMessage own = meshtastic_AdminMessage_init_zero;
|
||||
own.which_payload_variant = meshtastic_AdminMessage_get_owner_request_tag;
|
||||
admin->noteOutgoingAdminRequest(makeOutgoingRequest(STRANGER_NODE, own));
|
||||
|
||||
meshtastic_AdminMessage m;
|
||||
meshtastic_MeshPacket resp = makeModuleConfigResponse(STRANGER_NODE, m); // from set; pki off by default
|
||||
meshtastic_MeshPacket resp = makeModuleConfigResponse(QUERIED_NODE, m); // pki off by default
|
||||
|
||||
// A plaintext get_config_response is still rejected: the config request was pinned.
|
||||
TEST_ASSERT_FALSE_MESSAGE(admin->responseIsSolicited(resp, meshtastic_AdminMessage_get_config_response_tag, 0),
|
||||
"an unpinned request must not relax an earlier request's key pin");
|
||||
// Over PKC with the pinned key it is accepted.
|
||||
|
||||
resp.pki_encrypted = true;
|
||||
resp.public_key.size = 32;
|
||||
memcpy(resp.public_key.bytes, key, 32);
|
||||
memcpy(resp.public_key.bytes, QUERIED_KEY, 32);
|
||||
TEST_ASSERT_TRUE(admin->responseIsSolicited(resp, meshtastic_AdminMessage_get_config_response_tag, 0));
|
||||
// The unpinned get_owner_response is accepted in plaintext (its request carried no pin).
|
||||
resp.pki_encrypted = false;
|
||||
resp.public_key.size = 0;
|
||||
TEST_ASSERT_TRUE(admin->responseIsSolicited(resp, meshtastic_AdminMessage_get_owner_response_tag, 0));
|
||||
}
|
||||
|
||||
// The pin is taken from the destination's stored NodeDB key - the key perhapsEncode will encrypt
|
||||
// to. Reading the outgoing packet's public_key instead pinned nothing: nothing populates that
|
||||
// field before encryption, so every real request was unpinned and `from` alone admitted responses.
|
||||
void test_request_to_keyed_node_pins_the_stored_key(void)
|
||||
{
|
||||
mockNodeDB->addNodeWithKey(QUERIED_NODE, QUERIED_KEY);
|
||||
admin->noteOutgoingAdminRequest(makeOutgoingModuleConfigRequest(QUERIED_NODE));
|
||||
|
||||
meshtastic_AdminMessage m;
|
||||
meshtastic_MeshPacket plain = makeModuleConfigResponse(QUERIED_NODE, m);
|
||||
TEST_ASSERT_FALSE_MESSAGE(admin->responseIsSolicited(plain, MODULE_CONFIG_RESPONSE, REMOTE_HW_TAG),
|
||||
"a plaintext response must not answer a PKC-pinned request");
|
||||
|
||||
meshtastic_MeshPacket wrongKey = makeModuleConfigResponse(QUERIED_NODE, m);
|
||||
wrongKey.pki_encrypted = true;
|
||||
wrongKey.public_key.size = 32;
|
||||
memset(wrongKey.public_key.bytes, 0x77, 32);
|
||||
TEST_ASSERT_FALSE_MESSAGE(admin->responseIsSolicited(wrongKey, MODULE_CONFIG_RESPONSE, REMOTE_HW_TAG),
|
||||
"a response under a different key must not be accepted");
|
||||
|
||||
meshtastic_MeshPacket good = makeModuleConfigResponse(QUERIED_NODE, m);
|
||||
good.pki_encrypted = true;
|
||||
good.public_key.size = 32;
|
||||
memcpy(good.public_key.bytes, QUERIED_KEY, 32);
|
||||
TEST_ASSERT_TRUE_MESSAGE(admin->responseIsSolicited(good, MODULE_CONFIG_RESPONSE, REMOTE_HW_TAG),
|
||||
"the pinned key must still admit the genuine response");
|
||||
}
|
||||
|
||||
// Ham mode never uses PKC, so pinning a key there would reject the legitimate plaintext response.
|
||||
void test_ham_mode_request_is_not_pinned(void)
|
||||
{
|
||||
owner.is_licensed = true;
|
||||
mockNodeDB->addNodeWithKey(QUERIED_NODE, QUERIED_KEY);
|
||||
admin->noteOutgoingAdminRequest(makeOutgoingModuleConfigRequest(QUERIED_NODE));
|
||||
|
||||
meshtastic_AdminMessage m;
|
||||
meshtastic_MeshPacket mp = makeModuleConfigResponse(QUERIED_NODE, m);
|
||||
TEST_ASSERT_TRUE_MESSAGE(admin->responseIsSolicited(mp, MODULE_CONFIG_RESPONSE, REMOTE_HW_TAG),
|
||||
"a request that could not have gone out over PKC must not be pinned");
|
||||
}
|
||||
|
||||
// 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)
|
||||
{
|
||||
admin->noteOutgoingAdminRequest(makeOutgoingModuleConfigRequest(STRANGER_NODE));
|
||||
|
||||
meshtastic_AdminMessage m;
|
||||
meshtastic_MeshPacket mp = makeModuleConfigResponse(STRANGER_NODE, m);
|
||||
mp.decoded.request_id = REQUEST_ID ^ 0xFFFF; // answers some other request
|
||||
|
||||
TEST_ASSERT_FALSE_MESSAGE(admin->responseIsSolicited(mp, MODULE_CONFIG_RESPONSE, REMOTE_HW_TAG),
|
||||
"a response that does not echo our request id must be rejected");
|
||||
|
||||
mp.decoded.request_id = REQUEST_ID;
|
||||
TEST_ASSERT_TRUE_MESSAGE(admin->responseIsSolicited(mp, MODULE_CONFIG_RESPONSE, REMOTE_HW_TAG),
|
||||
"the matching request id must still be accepted");
|
||||
}
|
||||
|
||||
// A request we could not bind to an id must not be answerable by a response that simply omits
|
||||
// request_id, which decodes to 0.
|
||||
void test_request_without_an_id_admits_nothing(void)
|
||||
{
|
||||
meshtastic_MeshPacket req = makeOutgoingModuleConfigRequest(STRANGER_NODE);
|
||||
req.id = 0;
|
||||
admin->noteOutgoingAdminRequest(req);
|
||||
|
||||
meshtastic_AdminMessage m;
|
||||
meshtastic_MeshPacket mp = makeModuleConfigResponse(STRANGER_NODE, m);
|
||||
mp.decoded.request_id = 0;
|
||||
|
||||
TEST_ASSERT_FALSE_MESSAGE(admin->responseIsSolicited(mp, MODULE_CONFIG_RESPONSE, REMOTE_HW_TAG),
|
||||
"a zero request id must not act as a matching token");
|
||||
}
|
||||
|
||||
// A remote_hardware response must answer a request for that exact subtype, not just any module
|
||||
@@ -542,6 +650,10 @@ void setup()
|
||||
RUN_TEST(test_request_to_one_node_does_not_admit_another);
|
||||
RUN_TEST(test_response_variant_must_match_request);
|
||||
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_response_with_wrong_request_id_is_rejected);
|
||||
RUN_TEST(test_request_without_an_id_admits_nothing);
|
||||
RUN_TEST(test_module_config_subtype_must_match);
|
||||
RUN_TEST(test_response_is_consumed_no_replay);
|
||||
RUN_TEST(test_outgoing_setter_does_not_admit_responses);
|
||||
|
||||
Reference in New Issue
Block a user