Skip to content

Draft: Restore XEdDSA material in keygen-excluded identity loads #10984

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
RCGV1 wants to merge 3 commits into
base: develop
Choose a base branch
from
+50 −1
Draft

Draft: Restore XEdDSA material in keygen-excluded identity loads #10984

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
65af8eb
fix: initialize XEdDSA keys when loading identity
RCGV1 Jul 10, 2026
521c620
test: cover persisted XEdDSA key loading
RCGV1 Jul 10, 2026
f68b0b0
fix(crypto): reject blank persisted signing keys
RCGV1 Jul 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/mesh/CryptoEngine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,14 @@ bool CryptoEngine::decryptCurve25519(uint32_t fromNode, meshtastic_NodeInfoLite_
void CryptoEngine::setDHPrivateKey(uint8_t *_private_key)
{
memcpy(private_key, _private_key, 32);
#if !(MESHTASTIC_EXCLUDE_XEDDSA)
if (memfll(private_key, 0, sizeof(private_key))) {
memset(xeddsa_private_key, 0, sizeof(xeddsa_private_key));
memset(xeddsa_public_key, 0, sizeof(xeddsa_public_key));
return;
}
XEdDSA::priv_curve_to_ed_keys(private_key, xeddsa_private_key, xeddsa_public_key);
#endif
}
Comment on lines 289 to 300

@coderabbitai coderabbitai Bot Jul 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find all callers of setDHPrivateKey and check whether any can pass a blank/unvalidated key
rg -n -B5 -A2 '\bsetDHPrivateKey\s*\(' --type=cpp

# Inspect priv_curve_to_ed_keys implementation to confirm derivation behavior on zero input
ast-grep run --pattern 'void priv_curve_to_ed_keys($$$) { $$$ }' --lang cpp
rg -n 'priv_curve_to_ed_keys' -g '*.cpp' -g '*.h'

Repository: meshtastic/firmware

Length of output: 157

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map relevant symbols in CryptoEngine.cpp
ast-grep outline src/mesh/CryptoEngine.cpp --view expanded

# Find callers/usages of setDHPrivateKey and related key-setting flows
rg -n -C 3 '\bsetDHPrivateKey\s*\(' src
rg -n -C 3 '\bregeneratePublicKey\s*\(' src

# Find blank-key checks around XEdDSA material
rg -n -C 3 'memfll\s*\(\s*xeddsa_private_key|priv_curve_to_ed_keys|xeddsa_sign|memfll\s*\(\s*private_key' src/mesh/CryptoEngine.cpp src

Repository: meshtastic/firmware

Length of output: 25164

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the implementation of the key-generation helpers and call paths
wc -l src/mesh/CryptoEngine.cpp
sed -n '1,220p' src/mesh/CryptoEngine.cpp
sed -n '220,360p' src/mesh/CryptoEngine.cpp
sed -n '360,520p' src/mesh/CryptoEngine.cpp

Repository: meshtastic/firmware

Length of output: 15250

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map CryptoEngine.cpp and inspect key-related paths
ast-grep outline src/mesh/CryptoEngine.cpp --view expanded
printf '\n--- callers ---\n'
rg -n -C 3 '\bsetDHPrivateKey\s*\(' src
printf '\n--- regeneratePublicKey ---\n'
rg -n -C 3 '\bregeneratePublicKey\s*\(' src/mesh/CryptoEngine.cpp
printf '\n--- xeddsa guard / derivation ---\n'
rg -n -C 2 'memfll\s*\(\s*xeddsa_private_key|priv_curve_to_ed_keys|xeddsa_sign|memfll\s*\(\s*private_key' src/mesh/CryptoEngine.cpp
printf '\n--- file slice ---\n'
wc -l src/mesh/CryptoEngine.cpp
sed -n '1,220p' src/mesh/CryptoEngine.cpp
sed -n '220,360p' src/mesh/CryptoEngine.cpp
sed -n '360,520p' src/mesh/CryptoEngine.cpp

Repository: meshtastic/firmware

Length of output: 20910

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the NodeDB boot/config flow around the setDHPrivateKey call sites
sed -n '3840,3945p' src/mesh/NodeDB.cpp
sed -n '460,510p' src/mesh/NodeDB.cpp

# Find places that populate or reset config.security.private_key
rg -n -C 3 'private_key\.size|private_key\.bytes|public_key\.size|public_key\.bytes|ensurePkiKeys|createNewIdentity' src/mesh/NodeDB.cpp src/mesh/CryptoEngine.cpp

Repository: meshtastic/firmware

Length of output: 25559

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Look for validation/sanitization of the stored security private key
rg -n -C 3 'memfll\s*\(\s*config\.security\.private_key|private_key\.size == 32|private_key\.size > 0|setDHPrivateKey\(' src/mesh/NodeDB.cpp src/mesh/CryptoEngine.cpp

# Inspect the exact context around the PKI-only branch that calls setDHPrivateKey directly
sed -n '480,500p' src/mesh/NodeDB.cpp

# Find all direct calls to setDHPrivateKey outside NodeDB::generateCryptoKeyPair
rg -n -C 2 '\bsetDHPrivateKey\s*\(' src | sed -n '1,120p'

Repository: meshtastic/firmware

Length of output: 7511

Guard XEdDSA derivation against all-zero private keys setDHPrivateKey() is still reached from the direct boot path in src/mesh/NodeDB.cpp for any persisted 32-byte key pair, so a stored all-zero private key would now populate xeddsa_private_key and let xeddsa_sign() start succeeding on an invalid identity. Add the blank-key check here.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/mesh/CryptoEngine.cpp` around lines 289 - 295, Add an all-zero 32-byte
private-key check in CryptoEngine::setDHPrivateKey before calling
XEdDSA::priv_curve_to_ed_keys; skip XEdDSA key derivation for blank keys while
preserving the existing key copy behavior.

Source: Coding guidelines


/**
Expand Down
43 changes: 42 additions & 1 deletion test/test_crypto/test_main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,45 @@ void test_XEdDSA_repeated_sign_is_randomized(void)
TEST_ASSERT_TRUE(crypto->xeddsa_verify(pub, fromNode, packetId, portnum, message, sizeof(message), sig2));
}

void test_XEdDSA_persisted_private_key_reload(void)
{
uint8_t curvePublic[32], persistedPrivate[32];
uint8_t expectedEdPrivate[32], expectedEdPublic[32];
crypto->generateKeyPair(curvePublic, persistedPrivate);
XEdDSA::priv_curve_to_ed_keys(persistedPrivate, expectedEdPrivate, expectedEdPublic);

CryptoEngine firstLoad;
CryptoEngine secondLoad;
firstLoad.setDHPrivateKey(persistedPrivate);
secondLoad.setDHPrivateKey(persistedPrivate);

TEST_ASSERT_EQUAL_MEMORY(expectedEdPrivate, firstLoad.xeddsa_private_key, sizeof(expectedEdPrivate));
TEST_ASSERT_EQUAL_MEMORY(expectedEdPublic, firstLoad.xeddsa_public_key, sizeof(expectedEdPublic));
TEST_ASSERT_EQUAL_MEMORY(firstLoad.xeddsa_private_key, secondLoad.xeddsa_private_key, sizeof(expectedEdPrivate));
TEST_ASSERT_EQUAL_MEMORY(firstLoad.xeddsa_public_key, secondLoad.xeddsa_public_key, sizeof(expectedEdPublic));

const uint8_t payload[] = "persisted identity";
uint8_t signature[XEDDSA_SIGNATURE_SIZE];
TEST_ASSERT_TRUE(firstLoad.xeddsa_sign(0x12345678, 0xABCDEF01, 1, payload, sizeof(payload), signature));
TEST_ASSERT_TRUE(secondLoad.xeddsa_verify(curvePublic, 0x12345678, 0xABCDEF01, 1, payload, sizeof(payload), signature));
}

void test_XEdDSA_zero_private_key_reload_clears_signing_material(void)
{
uint8_t curvePublic[32], privateKey[32], zeroKey[32] = {0};
uint8_t signature[XEDDSA_SIGNATURE_SIZE];
const uint8_t payload[] = "zero persisted identity";

crypto->generateKeyPair(curvePublic, privateKey);
crypto->setDHPrivateKey(privateKey);
TEST_ASSERT_TRUE(crypto->xeddsa_sign(0x1, 0x2, 1, payload, sizeof(payload), signature));

crypto->setDHPrivateKey(zeroKey);
TEST_ASSERT_EQUAL_MEMORY(zeroKey, crypto->xeddsa_private_key, sizeof(zeroKey));
TEST_ASSERT_EQUAL_MEMORY(zeroKey, crypto->xeddsa_public_key, sizeof(zeroKey));
TEST_ASSERT_FALSE(crypto->xeddsa_sign(0x1, 0x2, 1, payload, sizeof(payload), signature));
}

void test_AES_CTR(void)
{
uint8_t expected[32];
Expand Down Expand Up @@ -330,7 +369,9 @@ void setup()
RUN_TEST(test_XEdDSA_curve_to_ed_cache);
RUN_TEST(test_XEdDSA_max_payload);
RUN_TEST(test_XEdDSA_repeated_sign_is_randomized);
RUN_TEST(test_XEdDSA_persisted_private_key_reload);
RUN_TEST(test_XEdDSA_zero_private_key_reload_clears_signing_material);
exit(UNITY_END()); // stop unit testing
}

void loop() {}
void loop() {}
Loading