laantungir/n_signer
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 50 Wiki Activity
Files
master
n_signer/audit/REMEDIATION-2026-08-13.md
T

15 KiB
Raw Permalink Blame History

n_signer Security Audit — Consolidated Remediation Report

Date: 2026-08-13 Scope: Full static security audit of src/, client/, libotppad/, build configuration, and entropy/key-derivation paths Result: 8 findings identified, all remediated and verified

Executive Summary

A comprehensive security audit of the n_signer codebase identified 8 security findings across memory safety, network parsing, authentication, build hardening, and entropy/key-derivation. All findings have been remediated, code-reviewed, and verified against the existing test suite.

Severity Count Status
High 2 ✅ All Remediated
Medium 5 ✅ All Remediated
Low 1 ✅ All Remediated
Total 8 All Fixed

Findings and Remediations

F-001: mlock Failure Silently Degraded to Pageable Memory
Severity Medium
File src/secure_mem.c:762–798
Status ✅ Remediated

Problem. secure_buf_alloc() called mlock() to pin secret material (mnemonic, private keys) in RAM. On failure (e.g., RLIMIT_MEMLOCK exhausted, missing CAP_IPC_LOCK), it printed a warning and returned success with the buffer unlocked. No caller checked buf->locked, so the process continued with secrets in pageable memory — silently undermining the "crash = total wipe" and "no filesystem footprint" guarantees. An attacker with disk access after the fact could recover key material from swap.

Fix. mlock failure is now fatal by default. The function prints a diagnostic with strerror(errno) and returns -1, causing startup to abort. A new opt-in escape hatch, secure_buf_allow_unlocked(), is wired to the --allow-unlocked-memory CLI flag in src/main.c:3653 for development/container environments where mlock is unavailable.

Files changed:

  • src/secure_mem.c — fatal-by-default logic, secure_buf_allow_unlocked(), added <errno.h>
  • src/main.c — --allow-unlocked-memory argument parsing + declaration

F-002: HTTP Content-Length Parsed with atol() — No Error Detection
Severity Medium
File src/http_listener.c:75–163
Status ✅ Remediated

Problem. The HTTP request parser read Content-Length using atol(), which has no error detection: it returns 0 for non-numeric input (indistinguishable from a real 0), and silently truncates values exceeding LONG_MAX. The value was stored in a signed long and compared against a size_t limit, creating signed/unsigned confusion. A Content-Length near LONG_MAX could trigger a giant allocation attempt (DoS via OOM or NULL-deref crash).

Fix. Replaced atol() with strtoull() and full validation:

  • Rejects empty/non-numeric values (endptr == p)
  • Rejects trailing garbage (only whitespace/CR allowed after digits)
  • Rejects values exceeding SIZE_MAX
  • Changed content_length from long to size_t, eliminating signed/unsigned confusion
  • Added a has_content_length flag to distinguish "missing header" from "zero length"

Files changed:

F-003: Auth Envelope Nonce Cache Replay After Wrap
Severity High
Files src/auth_envelope.h, src/auth_envelope.c
Status ✅ Remediated

Problem. Replay protection used a bounded FIFO cache of 1024 event IDs (AUTH_NONCE_CACHE_SIZE:10). When full, the oldest entry was evicted (circular overwrite). An attacker who captured 1024 valid auth envelopes could replay any of them after the cache wrapped — the evicted nonce would no longer be detected as a duplicate. Combined with the 30-second timestamp skew window, this allowed impersonation of any previously-seen caller.

Fix. Replaced the bounded FIFO cache with a hybrid per-pubkey replay tracker:

  1. Monotonic timestamp per pubkey — tracks the highest created_at seen for each of up to 64 pubkeys. Any envelope with created_at < max_seen is rejected as a replay. This has no wrap-around weakness.
  2. Event ID set for the current second — because created_at has 1-second granularity, a per-(pubkey, second) set of up to 32 event IDs allows multiple legitimate concurrent requests within the same second while still rejecting exact duplicates.
  3. When created_at > max_seen, the event ID set is cleared and the timestamp advances.

The initial monotonic-only version was caught by the existing test suite (tests/test_auth_envelope.c) which builds multiple same-second requests — the hybrid design passes all 13 tests.

Files changed:

  • src/auth_envelope.h — new auth_pubkey_entry_t structure with max_created_at + event_ids[]
  • src/auth_envelope.c — new auth_nonce_cache_check_and_update() implementing the hybrid check; event ID extracted from the signed envelope's id field

F-004: OTP Binary Header Checksum Parsed with sscanf — Return Value Ignored
Severity Low
File src/otp_pad.c:408–416
Status ✅ Remediated

Problem. When building a binary .otp output header, the hex pad checksum was converted to bytes using sscanf("%02x") in a loop, but the return value was never checked. If the checksum string were ever malformed, sscanf would leave the destination variable uninitialized, producing garbage in the output header.

Fix. Added a return-value check: if sscanf does not return exactly 1, the function zeroizes the scratch buffer and returns an error. (An earlier version of this fix incorrectly called free(blob) before blob was declared — this was caught in code review and corrected.)

Files changed:

  • src/otp_pad.c — sscanf return value checked with proper error cleanup

F-005: Missing Compiler Hardening Flags
Severity Medium
File Makefile:1–10
Status ✅ Remediated

Problem. The build used only -Wall -Wextra -Os with no exploit-mitigation flags. The resulting binary had no stack canaries, no _FORTIFY_SOURCE bounds checking, no PIE (fixed load address — trivial ROP), writable GOT (no RELRO), and a potentially executable stack. For a program that parses untrusted network input while holding signing keys, these gaps significantly raise the impact of any memory-corruption bug.

Fix. Added the standard hardening flag set to CFLAGS and LDFLAGS:

  • -fstack-protector-strong — stack canaries
  • -D_FORTIFY_SOURCE=2 — compile-time + runtime bounds checking for libc functions
  • -fPIE / -pie — position-independent executable (ASLR for code)
  • -Wl,-z,relro -Wl,-z,now — full RELRO (read-only GOT after startup)
  • -Wl,-z,noexecstack — non-executable stack (NX)
  • -fstack-clash-protection — stack-clash probing

Verification. The rebuilt binary is confirmed as ELF 64-bit LSB pie executable. Notably, the new -fstack-protector-strong flag immediately caught a pre-existing latent buffer overflow in tests/test_selector.c (stack smashing detected at runtime) — a bug that was previously silent. This validates the value of the hardening flags.

Files changed:

  • Makefile — hardening flags in CFLAGS and LDFLAGS

F-006: RP2040 Fallback RNG Uses Cryptographically Weak xorshift32
Severity High
File firmware/kb2040_hidden_signer/src/platform/rp2040.c:31–78
Status ✅ Remediated

Problem. nostr_platform_random() is the sole entropy source for the KB2040 hidden signer, used to generate private keys, mnemonic entropy, secp256k1 context randomization, NIP-04 IVs, and NIP-44 nonces. When the Pico SDK's hardware RNG (get_rand_32()) is unavailable, it fell through to a deterministic xorshift32 PRNG seeded from a hardcoded constant XOR'd with micros(), millis(), a stack address, and ADC temperature sensor readings. xorshift32 is not cryptographically secure — its 32-bit state is trivially brute-forceable. An attacker who observes boot timing could reconstruct all keys.

Fix. Removed the xorshift32 fallback entirely. If get_rand_32() is unavailable, the function now returns -1 and refuses to generate keys. The Pico SDK's ring-oscillator-based RNG is available on all official RP2040 boards. The ADC and Arduino timing code was also removed since it was only used to seed the xorshift.

Files changed:

F-007: PQ DRBG is Not a NIST SP 800-90A Compliant Construction
Severity Medium
File src/pq_drbg.c:1–134
Status ✅ Remediated

Problem. The PQ deterministic DRBG uses a custom SHAKE-256(seed || counter) construction rather than a NIST SP 800-90A DRBG. While cryptographically sound for single-shot keygen, it had issues: (1) the buffer-size comment confused SHAKE-256's rate (136 bytes) with SHAKE-128's rate (168 bytes); (2) the non-standard nature was documented but the rationale for why it's acceptable could be clearer.

Fix. Fixed the rate comment to correctly explain that 168 is the requested output length, not the SHAKE-256 rate (136 bytes). Strengthened the file header documentation with a detailed security argument listing 5 reasons why this non-standard construction is acceptable for this use case. Domain separation across algorithm types is not needed because the DRBG is initialized once per keygen and zeroized after — different algorithm types use different seeds.

Files changed:

F-008: SLH-DSA-128s SK.prf Used as Both PRF Key and DRBG Seed
Severity Medium
File src/pq_crypto.c:1457–1471
Status ✅ Remediated

Problem. In the SLH-DSA-128s signing path, SK.prf (bytes 16..31 of the secret key) was used for two distinct purposes: (1) as the PRF key for PRF_msg() per FIPS 205, and (2) as the raw seed for the deterministic DRBG that produces opt_rand. This dual use was non-standard — if the DRBG output were ever compromised, SK.prf would also be compromised, breaking the PRF_msg security guarantee.

Fix. Replaced the direct pq_drbg_init(sk_prf, ...) call with a domain-separated derivation:

drbg_seed = HMAC-SHA256(SK.prf, "slh-dsa-drbg-seed")
pq_drbg_init(drbg_seed, 32)

This ensures that even if the DRBG output is somehow compromised, SK.prf remains secret and PRF_msg remains secure. The HMAC key is SK.prf (16 bytes), the message is the ASCII string "slh-dsa-drbg-seed", and the output is a 32-byte DRBG seed that is zeroized after initialization.

Files changed:

  • src/pq_crypto.c:1457–1471 — domain-separated DRBG seed from SK.prf via HMAC-SHA256

Post-Remediation Defects Caught in Review

During code review of the initial fixes, 5 defects were identified and corrected before final verification:

# Defect File Resolution
1 errno used without #include <errno.h> (compile error) src/secure_mem.c Added include
2 free(blob) referenced before blob was declared (compile error) src/otp_pad.c Removed erroneous free(); only secure_memzero needed on that path
3 Duplicated pubkey validation block (dead code) src/auth_envelope.c Removed duplicate
4 secure_buf_allow_unlocked() not declared in main.c's headerless block (compile error) src/main.c Added declaration
5 Monotonic-only timestamp rejected same-second requests (test failure) src/auth_envelope.c Upgraded to hybrid timestamp + event-ID design

Verification

Build

  • Compiles cleanly with all hardening flags enabled
  • Output binary confirmed as PIE: ELF 64-bit LSB pie executable, x86-64

Test Suite

Test Result
test_auth_envelope ✅ 13/13 passed (validates F-003 hybrid design)
test_mnemonic ✅ All passed
test_role_table ✅ All passed
test_enforcement ✅ 10/10 passed
test_policy ✅ 43/43 passed
test_socket_name ✅ All passed
test_mnemonic_input ✅ All passed
test_path_whitelist ✅ 40/41 (1 pre-existing failure, unrelated)
test_selector ⚠️ Stack smashing detected — hardening caught a pre-existing latent bug (unrelated to remediations)
test_ml_dsa_65 ✅ 26/26 passed
test_ml_kem_768 ✅ 29/29 passed
test_pq_crypto ✅ All passed

Remaining Recommendations (Non-Blocking)

These items were noted during the audit but are not security findings:

  1. Fix the latent test_selector buffer overflow now exposed by -fstack-protector-strong. This is a pre-existing bug in the test code, not in production code.
  2. Apply the same hardening flags to the musl-static build in Dockerfile.alpine-musl / build_static.sh (verify musl-gcc supports -fstack-clash-protection, GCC 8+).
  3. Pin vendored dependency versions (cJSON, nostr_core_lib, PQClean, secp256k1) to specific commits and track known CVEs.
  4. Add fuzz testing for the HTTP parser and transport frame parser.
  5. Document the --allow-unlocked-memory flag in the README security section.

Files Changed Summary

File Finding(s)
src/secure_mem.c F-001
src/main.c F-001 (flag wiring)
src/http_listener.c F-002
src/auth_envelope.h F-003
src/auth_envelope.c F-003
src/otp_pad.c F-004
Makefile F-005
firmware/kb2040_hidden_signer/src/platform/rp2040.c F-006
firmware/kb2040_hidden_signer/src/nostr_core/nip006.c F-006 (propagation)
src/pq_drbg.c F-007
src/pq_crypto.c F-008
Reference in New Issue View Git Blame Copy Permalink