diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index cba2d9868..032f0a851 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -577,7 +577,14 @@ DecodeState perhapsDecode(meshtastic_MeshPacket *p) // Resolve the sender's public key: prefer the one stored in NodeDB (hot store or warm tier), else // fall back to a not-yet-committed key held during an in-progress key-verification handshake. meshtastic_NodeInfoLite_public_key_t remotePublic = {0, {0}}; - bool haveRemoteKey = nodeDB->copyPublicKey(p->from, remotePublic) || crypto->getPendingPublicKey(p->from, remotePublic); + bool haveRemoteKey = nodeDB->copyPublicKey(p->from, remotePublic); + // A pending key is an unverified identity claim supplied by whoever opened the handshake, so it is + // accepted only for the exchange itself (checked after decode). perhapsEncode applies the same rule. + bool havePendingKey = false; + if (!haveRemoteKey) { + havePendingKey = crypto->getPendingPublicKey(p->from, remotePublic); + haveRemoteKey = havePendingKey; + } meshtastic_NodeInfoLite *ourNode = nullptr; if (p->channel == 0 && isToUs(p) && p->to > 0 && !isBroadcast(p->to) && rawSize > MESHTASTIC_PKC_OVERHEAD && @@ -585,8 +592,10 @@ DecodeState perhapsDecode(meshtastic_MeshPacket *p) // Try the sender's known key first, then each configured admin key so an authorized admin can // reach a node that has not yet learned their key. AES-CCM AEAD rejects wrong candidates. bool viaAdminKey = false; + bool viaPendingKey = false; if (haveRemoteKey && crypto->decryptCurve25519(p->from, remotePublic, p->id, rawSize, p->encrypted.bytes, bytes)) { decrypted = true; + viaPendingKey = havePendingKey; } for (int i = 0; i < 3 && !decrypted; i++) { if (config.security.admin_key[i].size != 32) @@ -607,6 +616,12 @@ DecodeState perhapsDecode(meshtastic_MeshPacket *p) size_t payloadSize = rawSize - MESHTASTIC_PKC_OVERHEAD; if (pb_decode_from_bytes(bytes, payloadSize, &meshtastic_Data_msg, &decodedtmp) && decodedtmp.portnum != meshtastic_PortNum_UNKNOWN_APP) { + if (viaPendingKey && decodedtmp.portnum != meshtastic_PortNum_KEY_VERIFICATION_APP) { + // The pending key only proves the handshake initiator holds it, not that they are + // p->from. Beyond the exchange it would let them send DMs that look authenticated. + LOG_WARN("Refusing pending-key decrypt of port %u from 0x%08x", (unsigned)decodedtmp.portnum, p->from); + return DecodeState::DECODE_FAILURE; + } decrypted = true; rawSize = payloadSize; // commit the overhead subtraction only on full success LOG_INFO("Packet decrypted using PKI!"); diff --git a/test/test_pki_admin_fallback/test_main.cpp b/test/test_pki_admin_fallback/test_main.cpp index c03652244..18e5e2260 100644 --- a/test/test_pki_admin_fallback/test_main.cpp +++ b/test/test_pki_admin_fallback/test_main.cpp @@ -202,6 +202,33 @@ void test_wrong_admin_key_does_not_decode(void) TEST_ASSERT_NULL(mockNodeDB->getMeshNode(ADMIN_NODE)); } +// A pending key is an unverified identity claim from whoever opened a key-verification handshake, so it +// must decrypt only the exchange itself. Otherwise they could send DMs that look PKI-authenticated as a +// node they never proved they are. +void test_pending_key_decrypts_only_key_verification(void) +{ + // PEER is unknown to us; the handshake stashed its claimed key as pending. + static constexpr NodeNum PEER = 0x0C0C0C0C; + uint8_t peerPub[32], peerPriv[32]; + crypto->generateKeyPair(peerPub, peerPriv); + crypto->setDHPrivateKey(ourPriv); // generateKeyPair changed it + crypto->setPendingPublicKey(PEER, peerPub); + + // A DM on any other port must not decode, even though the pending key would decrypt it. + meshtastic_MeshPacket spoofed = makePkiPacket(PEER, meshtastic_PortNum_TEXT_MESSAGE_APP, 16, peerPriv); + TEST_ASSERT_NOT_EQUAL_MESSAGE(DECODE_SUCCESS, perhapsDecode(&spoofed), "pending key must not decrypt a text DM"); + TEST_ASSERT_FALSE_MESSAGE(spoofed.pki_encrypted, "spoofed DM must not be marked PKI-authenticated"); + + // The key-verification exchange itself still works, so bootstrapping is unaffected. + meshtastic_MeshPacket handshake = makePkiPacket(PEER, meshtastic_PortNum_KEY_VERIFICATION_APP, 16, peerPriv); + TEST_ASSERT_EQUAL_MESSAGE(DECODE_SUCCESS, perhapsDecode(&handshake), "key verification must still decrypt"); + TEST_ASSERT_TRUE(handshake.pki_encrypted); + + // A pending key is never persisted, so the peer stays unknown until verification commits it. + TEST_ASSERT_NULL_MESSAGE(mockNodeDB->getMeshNode(PEER), "pending key must not be learned into NodeDB"); + crypto->clearPendingPublicKey(); +} + #endif // !(MESHTASTIC_EXCLUDE_PKI) void setup() @@ -216,6 +243,7 @@ void setup() RUN_TEST(test_admin_key_slot2_only_decrypts); RUN_TEST(test_no_admin_key_unknown_sender_not_decoded); RUN_TEST(test_wrong_admin_key_does_not_decode); + RUN_TEST(test_pending_key_decrypts_only_key_verification); #endif exit(UNITY_END()); }