Accept a pending key only for the key-verification exchange

src/mesh/Router.cpp

@@ -575,16 +575,25 @@ 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 &&
         (ourNode = nodeDB->getMeshNode(p->to)) != nullptr && ourNode->public_key.size > 0) {
         // 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)
@@ -605,6 +614,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!");

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());
 }