Crypt module

[KenVanHoeylandt]

• Moved cryptography code out of Tactility project and into a kernel module.
• Converted C++ code to C style interface
• Hardened security

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

This PR adds a new crypt-module with C-linkage crypto and hash headers, implementation files, and a module symbol. Build files, startup wiring, callers, tests, and generated build artifacts are updated to include the module and its test target. ChatSettings.cpp and WifiApSettings.cpp switch to the new crypt_* APIs. The crypto implementation updates key handling, IV generation, and zeroization behavior.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title names the new crypt module, which matches the core change in the PR.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch crypt-module

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

Modules/crypt-module/source/crypt.cpp (1)

126-133: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win

Fix IV derivation here — %= leaves every byte at 0, so CBC always uses an all-zero IV and encryptions become deterministic for a given plaintext/key. If any input byte is 0, this also becomes undefined behavior. This should be ^= (or another non-zero mixing step). Modules/crypt-module/source/crypt.cpp:126

Modules/crypt-module/source/hash.cpp (1)

14-25: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Out-of-bounds read in djb2_data.

The loop unconditionally reads *data_bytes++ before checking length (OOB read for length == 0), and reads one extra byte past the buffer at the end of every call (c = *data_bytes++ on the last iteration reads data[length]). The hash result is unaffected since the extra byte is discarded, but this is undefined behavior that can fault on guarded/boundary memory, e.g. hashing arbitrary key/config buffers from crypt.cpp.

🐛 Proposed fix

 uint32_t djb2_data(const void* data, size_t length) {
     uint32_t hash = 5381;
     auto* data_bytes = static_cast<const uint8_t*>(data);
-    uint8_t c = *data_bytes++;
-    size_t index = 0;
-    while (index < length) {
-        hash = ((hash << 5) + hash) + (uint32_t)c; // hash * 33 + c
-        c = *data_bytes++;
-        index++;
-    }
+    for (size_t index = 0; index < length; index++) {
+        hash = ((hash << 5) + hash) + (uint32_t)data_bytes[index]; // hash * 33 + c
+    }
     return hash;
 }

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e8f36b5c-98c4-4eb9-a59a-ff8af1751a32

📥 Commits

Reviewing files that changed from the base of the PR and between 9d59939 and 757115a.

📒 Files selected for processing (15)

• CMakeLists.txt
• Firmware/CMakeLists.txt
• Modules/crypt-module/CMakeLists.txt
• Modules/crypt-module/include/tactility/crypt.h
• Modules/crypt-module/include/tactility/crypt_module.h
• Modules/crypt-module/include/tactility/hash.h
• Modules/crypt-module/source/crypt.cpp
• Modules/crypt-module/source/hash.cpp
• Modules/crypt-module/source/module.cpp
• Tactility/CMakeLists.txt
• Tactility/Source/Tactility.cpp
• Tactility/Source/app/chat/ChatSettings.cpp
• Tactility/Source/service/wifi/WifiApSettings.cpp
• TactilityCore/CMakeLists.txt
• Tests/Tactility/CMakeLists.txt

[coderabbitai]

Actionable comments posted: 3

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 42e0f1b6-4015-43e0-b501-7b8e5ac9063d

📥 Commits

Reviewing files that changed from the base of the PR and between 757115a and 93984b5.

📒 Files selected for processing (27)

• .github/workflows/tests.yml
• Tests/CMakeLists.txt
• Tests/crypt-module/CMakeLists.txt
• Tests/crypt-module/Source/CryptTest.cpp
• Tests/crypt-module/Source/HashTest.cpp
• Tests/crypt-module/Source/Main.cpp
• build-tests/Libraries/FreeRTOS-Kernel/libfreertos_kernel.a
• build-tests/Libraries/FreeRTOS-Kernel/portable/libfreertos_kernel_port.a
• build-tests/Libraries/mbedtls/3rdparty/everest/libeverest.a
• build-tests/Libraries/mbedtls/3rdparty/p256-m/libp256m.a
• build-tests/Libraries/mbedtls/library/libmbedcrypto.a
• build-tests/Libraries/mbedtls/library/libmbedtls.a
• build-tests/Libraries/mbedtls/library/libmbedx509.a
• build-tests/Makefile
• build-tests/Modules/crypt-module/Makefile
• build-tests/Modules/crypt-module/cmake_install.cmake
• build-tests/Tactility/Makefile
• build-tests/TactilityCore/Makefile
• build-tests/Tests/CTestTestfile.cmake
• build-tests/Tests/Tactility/Makefile
• build-tests/Tests/TactilityCore/TactilityCoreTests
• build-tests/Tests/cmake_install.cmake
• build-tests/Tests/crypt-module/CTestTestfile.cmake
• build-tests/Tests/crypt-module/CryptModuleTests
• build-tests/Tests/crypt-module/Makefile
• build-tests/Tests/crypt-module/cmake_install.cmake
• build-tests/cmake_install.cmake

💤 Files with no reviewable changes (1)

• build-tests/TactilityCore/Makefile

✅ Files skipped from review due to trivial changes (9)

• build-tests/Tests/cmake_install.cmake
• build-tests/Tests/crypt-module/CTestTestfile.cmake
• .github/workflows/tests.yml
• build-tests/Tests/CTestTestfile.cmake
• build-tests/Tests/crypt-module/cmake_install.cmake
• build-tests/Modules/crypt-module/cmake_install.cmake
• build-tests/Tactility/Makefile
• build-tests/Tests/Tactility/Makefile
• build-tests/Makefile

[coderabbitai]

Actionable comments posted: 2

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ff39a4b4-f9ca-4ba1-b864-e0ba5b85876c

📥 Commits

Reviewing files that changed from the base of the PR and between e280505 and 719abe8.

📒 Files selected for processing (5)

• .gitignore
• Modules/crypt-module/include/tactility/crypt.h
• Modules/crypt-module/source/crypt.cpp
• Tactility/Source/service/wifi/WifiApSettings.cpp
• Tests/crypt-module/Source/CryptTest.cpp

✅ Files skipped from review due to trivial changes (1)

• .gitignore

🚧 Files skipped from review as they are similar to previous changes (1)

• Tests/crypt-module/Source/CryptTest.cpp

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a9940a46-a59f-4dbe-ad9a-24603d9f54f3

📥 Commits

Reviewing files that changed from the base of the PR and between 719abe8 and 2fb4bbb.

📒 Files selected for processing (3)

• Buildscripts/release-sdk.py
• Modules/crypt-module/source/module.cpp
• Tactility/Source/service/wifi/WifiApSettings.cpp