Add Phase 9C security hardening
Generate exact-hash SDK source overrides without modifying dependencies. Harden SSH allocation and algorithm policy, tighten web authentication cleanup, and add focused host contract tests and documentation.
This commit is contained in:
@@ -14,8 +14,9 @@ preprocess and execution subprocesses have 30/30/10-second limits.
|
||||
|
||||
The runner prefers the sole `.pio/build/*/compile_commands.json`, otherwise the
|
||||
root database. Select another existing database with `--compile-commands PATH`.
|
||||
It preprocesses the actual wolfSSH `internal.c` compile command (`-E -dM`) and
|
||||
checks this reviewed profile:
|
||||
It requires the actual generated wolfSSH `internal.c` compilation input to equal
|
||||
`tools/security_overrides.py`'s rendering of the pinned original, preprocesses
|
||||
that compile command (`-E -dM`), and checks this reviewed profile:
|
||||
|
||||
- `LIBWOLFSSH_VERSION_HEX == 0x01004020` (1.4.20).
|
||||
- RSA disabled; ECDSA and Ed25519 not disabled.
|
||||
@@ -23,8 +24,8 @@ checks this reviewed profile:
|
||||
|
||||
For hosts without the ESP compiler/database, explicitly use `--host-only`.
|
||||
This prints a **SKIP** for production feature verification; it still checks the
|
||||
source/version/pin and executes the host contract with the reviewed feature
|
||||
profile. A stale compilation database is not proof of the next firmware build's
|
||||
source/version/pin and executes the rendered host contract with the reviewed
|
||||
feature profile. A stale compilation database is not proof of the next firmware build's
|
||||
configuration.
|
||||
|
||||
## What executes
|
||||
@@ -39,8 +40,9 @@ reviewed SHA-256 of `managed_components/wolfssl__wolfssh/src/internal.c`:
|
||||
Any same-version source change fails before compilation. **Re-audit before
|
||||
updating this hash**; do not automatically bless a dependency update.
|
||||
|
||||
The runner extracts actual function definitions by balancing braces after
|
||||
masking comments/string literals. It does not rewrite their bodies:
|
||||
The runner extracts actual **overridden production** function definitions by
|
||||
balancing braces after masking comments/string literals. Extraction does not
|
||||
rewrite their bodies; the separately verified build overlay does:
|
||||
|
||||
- `GetBoolean`, `GetUint32`, `GetSize`, `GetStringRef`
|
||||
- `DoUserAuthRequestPassword`, `DoUserAuthRequestPublicKey`, `DoUserAuthRequest`
|
||||
@@ -53,7 +55,7 @@ models, name/algorithm lookup, crypto and packet-output doubles. Binary request
|
||||
fixtures execute the extracted parsers; ordered event traces assert callback,
|
||||
hashing/signature and response order, rather than inspecting source substrings.
|
||||
|
||||
The 35 cases cover:
|
||||
The 35 baseline cases cover (with the stricter malformed-password contract):
|
||||
|
||||
- Ed25519 and ECDSA: signed authorization rejection (`INVALID_PUBLICKEY`,
|
||||
`FAILURE`, `REJECTED`, `INVALID_USER`, `INVALID_AUTHTYPE`) never hashes,
|
||||
@@ -62,8 +64,8 @@ The 35 cases cover:
|
||||
probe sends PK_OK but does not complete authentication.
|
||||
- Bad signatures, good signatures, success-result veto, ignored failure-result
|
||||
callback return, and auth `WOULD_BLOCK`.
|
||||
- Password success/failure, rejected password change, and the installed parser's
|
||||
callback on a truncated new-password-length field. No password result callback.
|
||||
- Password success/failure, rejected password change, and rejection **before the
|
||||
callback** for a truncated new-password-length field. No password result callback.
|
||||
- Disabled `none`, unknown methods/key algorithms and truncated signed framing.
|
||||
- Direct keyboard-interactive dispatch invokes a **registered non-NULL rejecting
|
||||
prompt callback**, returns error and purges without preparing/building/sending
|
||||
@@ -76,6 +78,15 @@ The 35 cases cover:
|
||||
prefix leaves the library copy intact. A blocked flush of earlier data returns
|
||||
a negative code without consuming new data.
|
||||
|
||||
A further **100 generated-parser cases** test short/missing flags and lengths,
|
||||
truncated/oversized/`UINT32_MAX` password and replacement-password lengths,
|
||||
checked initial offsets and canaries, no callback on malformed fields, preserved
|
||||
username/service/method prefixes, and suffix wiping before response emission.
|
||||
They include success, invalid/backend/rejected outcomes, password changes, no
|
||||
callback, callback-modified credential pointers/lengths, and pending retry.
|
||||
`WS_AUTH_PENDING` deliberately preserves bytes; the project's synchronous
|
||||
callbacks do not use it. This is not an unconditional async secret-wipe promise.
|
||||
|
||||
## Limits / ownership
|
||||
|
||||
This is a library parser/control-flow regression, **not application callback
|
||||
|
||||
@@ -67,7 +67,8 @@ static const word32 cannedKeyAlgoClientSz = sizeof(cannedKeyAlgoClient);
|
||||
static char events[64];
|
||||
static unsigned event_count, groups;
|
||||
static int auth_return, crypto_return, result_return, send_return;
|
||||
static int expect_new_password;
|
||||
static int expect_new_password, poison_password_pointers;
|
||||
static void inspect_password_wipe(void);
|
||||
static void event(char c) { assert(event_count + 1 < sizeof(events)); events[event_count++] = c; }
|
||||
static void ato32(const byte *b, word32 *v) {
|
||||
*v = (word32)b[0] << 24 | (word32)b[1] << 16 | (word32)b[2] << 8 | b[3];
|
||||
@@ -89,8 +90,13 @@ static byte MatchIdLists(int side, const byte *id, word32 count, const byte *lis
|
||||
for (word32 i = 0; i < n; ++i) if (*id == list[i]) return *id;
|
||||
return ID_UNKNOWN;
|
||||
}
|
||||
static int wolfSSH_SetUsernameRaw(WOLFSSH *s, const byte *u, word32 n) { return WS_SUCCESS; }
|
||||
static int SendUserAuthFailure(WOLFSSH *s, byte partial) { event('F'); return WS_SUCCESS; }
|
||||
static int wolfSSH_SetUsernameRaw(WOLFSSH *s, const byte *u, word32 n) {
|
||||
/* Called again after the method parser: prefix must still be valid. */
|
||||
assert(n == 4 && !memcmp(u, "test", 4)); return WS_SUCCESS;
|
||||
}
|
||||
static int SendUserAuthFailure(WOLFSSH *s, byte partial) {
|
||||
inspect_password_wipe(); event('F'); return send_return;
|
||||
}
|
||||
static int SendUserAuthPkOk(WOLFSSH *s, const byte *a, word32 an, const byte *k, word32 kn) {
|
||||
event('P'); return WS_SUCCESS;
|
||||
}
|
||||
@@ -129,6 +135,17 @@ static int authorize(byte method, WS_UserAuthData *a, void *ctx) {
|
||||
assert(a->sf.password.hasNewPassword == expect_new_password);
|
||||
assert(a->sf.password.passwordSz == 5);
|
||||
assert(!memcmp(a->sf.password.password, "dummy", 5));
|
||||
if (expect_new_password) {
|
||||
assert(a->sf.password.newPasswordSz == 9);
|
||||
assert(!memcmp(a->sf.password.newPassword, "new-dummy", 9));
|
||||
}
|
||||
if (poison_password_pointers) {
|
||||
/* Cleanup must use checked packet bounds, not mutable authData. */
|
||||
a->sf.password.password = (const byte *)(uintptr_t)1;
|
||||
a->sf.password.passwordSz = UINT32_MAX;
|
||||
a->sf.password.newPassword = (const byte *)(uintptr_t)1;
|
||||
a->sf.password.newPasswordSz = UINT32_MAX;
|
||||
}
|
||||
}
|
||||
return auth_return;
|
||||
}
|
||||
@@ -144,8 +161,23 @@ static int reject_keyboard(WS_UserAuthData_Keyboard *k, void *ctx) {
|
||||
static int allowed(WOLFSSH *s, void *ctx) {
|
||||
return WOLFSSH_USERAUTH_PASSWORD | WOLFSSH_USERAUTH_PUBLICKEY;
|
||||
}
|
||||
static byte output[1024], packet[1024];
|
||||
static word32 length;
|
||||
static byte output[1024];
|
||||
static struct { byte before[16], bytes[1024], after[16]; } storage, saved;
|
||||
#define packet storage.bytes
|
||||
static word32 length, suffix_start, suffix_end;
|
||||
static int watch_password;
|
||||
static void inspect_password_wipe(void) {
|
||||
if (!watch_password) return;
|
||||
assert(!memcmp(storage.before, saved.before, sizeof(storage.before)));
|
||||
assert(!memcmp(storage.after, saved.after, sizeof(storage.after)));
|
||||
assert(!memcmp(packet, saved.bytes, suffix_start));
|
||||
for (word32 i = suffix_start; i < suffix_end; ++i) assert(packet[i] == 0);
|
||||
assert(!memcmp(packet + suffix_end, saved.bytes + suffix_end,
|
||||
sizeof(packet) - suffix_end));
|
||||
}
|
||||
static void watch_suffix(word32 start) {
|
||||
suffix_start = start; suffix_end = length; saved = storage; watch_password = 1;
|
||||
}
|
||||
static WOLFSSH_CTX context = { authorize, result, reject_keyboard, allowed };
|
||||
static WOLFSSH ssh;
|
||||
static void reset(void) {
|
||||
@@ -154,7 +186,9 @@ static void reset(void) {
|
||||
ssh.ctx = &context; ssh.outputBuffer.buffer = output; ssh.sessionIdSz = 32;
|
||||
auth_return = WOLFSSH_USERAUTH_SUCCESS; crypto_return = WS_SUCCESS;
|
||||
result_return = WS_SUCCESS; send_return = WS_SUCCESS; expect_new_password = 0;
|
||||
length = 0;
|
||||
length = 0; watch_password = 0; poison_password_pointers = 0;
|
||||
memset(&storage, 0xa5, sizeof(storage));
|
||||
context.userAuthCb = authorize;
|
||||
}
|
||||
static void blob(const void *s, word32 n) {
|
||||
assert(length + 4 + n <= sizeof(packet));
|
||||
@@ -178,6 +212,95 @@ static void check(const char *trace, int done) {
|
||||
assert(!strcmp(events, trace));
|
||||
assert((ssh.clientState == CLIENT_USERAUTH_DONE) == done); ++groups;
|
||||
}
|
||||
static void password_cleanup_tests(void) {
|
||||
/* Every truncation of both encodings, including flag and length fields.
|
||||
* No malformed input may reach the auth/database double, even if absent. */
|
||||
for (int change = 0; change < 2; ++change) {
|
||||
word32 payload_size = change ? 23 : 10;
|
||||
for (word32 cut = 0; cut < payload_size; ++cut) {
|
||||
for (int no_callback = 0; no_callback < 2; ++no_callback) {
|
||||
reset(); request("password"); word32 start = length;
|
||||
packet[length++] = change; string("dummy");
|
||||
if (change) string("new-dummy");
|
||||
length = start + cut; watch_suffix(start);
|
||||
if (no_callback) context.userAuthCb = NULL;
|
||||
assert(dispatch() == WS_BUFFER_E); check("", 0);
|
||||
inspect_password_wipe();
|
||||
}
|
||||
}
|
||||
}
|
||||
const word32 oversized[] = { 6, 1024, UINT32_MAX };
|
||||
for (unsigned i = 0; i < sizeof(oversized)/sizeof(oversized[0]); ++i) {
|
||||
for (int change = 0; change < 2; ++change) {
|
||||
reset(); request("password"); word32 start = length;
|
||||
packet[length++] = change; string("dummy");
|
||||
word32 field = start + 1;
|
||||
if (change) { field = length; string("new-dummy"); }
|
||||
c32toa(change && oversized[i] == 6 ? 10 : oversized[i], packet + field);
|
||||
watch_suffix(start);
|
||||
assert(dispatch() == WS_BUFFER_E); check("", 0); inspect_password_wipe();
|
||||
}
|
||||
}
|
||||
/* Application bad-password/backend failure/admission denial all retain the
|
||||
* library's ordinary result mapping. Include partial success and no callback. */
|
||||
const int outcomes[] = { WOLFSSH_USERAUTH_SUCCESS, WOLFSSH_USERAUTH_INVALID_PASSWORD,
|
||||
WOLFSSH_USERAUTH_FAILURE, WOLFSSH_USERAUTH_REJECTED,
|
||||
WOLFSSH_USERAUTH_INVALID_USER, WOLFSSH_USERAUTH_INVALID_AUTHTYPE,
|
||||
WOLFSSH_USERAUTH_PARTIAL_SUCCESS };
|
||||
for (unsigned i = 0; i < sizeof(outcomes)/sizeof(outcomes[0]); ++i) {
|
||||
for (int change = 0; change < 2; ++change) {
|
||||
reset(); request("password"); word32 start = length;
|
||||
packet[length++] = change; string("dummy");
|
||||
if (change) string("new-dummy");
|
||||
expect_new_password = change; auth_return = outcomes[i];
|
||||
poison_password_pointers = 1;
|
||||
/* Trailing payload is also wiped but not included in parsed idx. */
|
||||
word32 parsed_end = length; packet[length++] = 0x71;
|
||||
watch_suffix(start);
|
||||
WS_UserAuthData data = {0}; data.username = packet + 4; data.usernameSz = 4;
|
||||
word32 idx = start;
|
||||
assert(DoUserAuthRequestPassword(&ssh, &data, packet, length, &idx) == WS_SUCCESS);
|
||||
assert(idx == (outcomes[i] == WOLFSSH_USERAUTH_REJECTED ? start : parsed_end));
|
||||
check(i == 0 ? "A" : "AF", i == 0); inspect_password_wipe();
|
||||
}
|
||||
}
|
||||
reset(); request("password"); word32 start = length;
|
||||
packet[length++] = 0; string("dummy"); watch_suffix(start);
|
||||
context.userAuthCb = NULL;
|
||||
assert(dispatch() == WS_SUCCESS); check("F", 0); inspect_password_wipe();
|
||||
|
||||
reset(); request("password"); start = length;
|
||||
packet[length++] = 0; string("dummy"); watch_suffix(start);
|
||||
auth_return = WOLFSSH_USERAUTH_FAILURE; send_return = WS_WANT_WRITE;
|
||||
assert(dispatch() == WS_WANT_WRITE); check("AF", 0); inspect_password_wipe();
|
||||
|
||||
for (int change = 0; change < 2; ++change) {
|
||||
reset(); request("password"); start = length;
|
||||
packet[length++] = change; string("dummy");
|
||||
if (change) string("new-dummy");
|
||||
expect_new_password = change; watch_suffix(start);
|
||||
auth_return = WOLFSSH_USERAUTH_WOULD_BLOCK;
|
||||
assert(dispatch() == WS_AUTH_PENDING); check("A", 0);
|
||||
assert(!memcmp(&storage, &saved, sizeof(storage)));
|
||||
auth_return = WOLFSSH_USERAUTH_SUCCESS;
|
||||
assert(dispatch() == WS_SUCCESS); check("AA", 1); inspect_password_wipe();
|
||||
}
|
||||
/* Invalid argument paths must neither dereference idx nor guess wipe bounds. */
|
||||
for (int bad = 0; bad < 8; ++bad) {
|
||||
reset(); request("password"); start = length;
|
||||
packet[length++] = 0; string("dummy"); saved = storage;
|
||||
WS_UserAuthData data = {0}; word32 idx = start;
|
||||
if (bad == 5) idx = length + 1;
|
||||
if (bad == 6) idx = UINT32_MAX;
|
||||
if (bad == 7) ssh.ctx = NULL;
|
||||
int ret = DoUserAuthRequestPassword(bad == 0 ? NULL : &ssh,
|
||||
bad == 1 ? NULL : &data, bad == 2 ? NULL : packet,
|
||||
bad == 3 ? 0 : length, bad == 4 ? NULL : &idx);
|
||||
assert(ret == (bad == 5 || bad == 6 ? WS_BUFFER_E : WS_BAD_ARGUMENT));
|
||||
assert(!memcmp(&storage, &saved, sizeof(storage))); check("", 0);
|
||||
}
|
||||
}
|
||||
|
||||
int main(void) {
|
||||
const int rejected[] = { WOLFSSH_USERAUTH_INVALID_PUBLICKEY,
|
||||
WOLFSSH_USERAUTH_FAILURE, WOLFSSH_USERAUTH_REJECTED,
|
||||
@@ -209,10 +332,10 @@ int main(void) {
|
||||
reset(); request("password"); packet[length++] = 1; string("dummy"); string("new-dummy");
|
||||
expect_new_password = 1; auth_return = WOLFSSH_USERAUTH_INVALID_AUTHTYPE;
|
||||
assert(dispatch() == WS_SUCCESS); check("AF", 0);
|
||||
/* Actual parser still calls auth when the new-password length is truncated. */
|
||||
/* The generated parser rejects a truncated new-password length before auth. */
|
||||
reset(); request("password"); packet[length++] = 1; string("dummy");
|
||||
expect_new_password = 1; auth_return = WOLFSSH_USERAUTH_INVALID_AUTHTYPE;
|
||||
assert(dispatch() == WS_SUCCESS); check("AF", 0);
|
||||
assert(dispatch() == WS_BUFFER_E); check("", 0);
|
||||
const char *unsupported[] = { "none", "unrecognized" };
|
||||
for (unsigned i = 0; i < 2; ++i) {
|
||||
reset(); request(unsupported[i]); assert(dispatch() == WS_SUCCESS); check("F", 0);
|
||||
@@ -241,6 +364,9 @@ int main(void) {
|
||||
reset(); ssh.outputBuffer.plainSz = 2; send_return = WS_WANT_WRITE;
|
||||
byte data[] = { 1, 2 }; assert(SendChannelData(&ssh, 1, data, 2) == WS_WANT_WRITE);
|
||||
assert(output[9] == 0); check("S", 0);
|
||||
printf("PASS: %u actual wolfSSH parser/control-flow cases\n", groups);
|
||||
assert(groups == 35);
|
||||
printf("PASS: %u original wolfSSH parser/control-flow cases (stricter malformed password contract)\n", groups);
|
||||
password_cleanup_tests();
|
||||
printf("PASS: %u additional generated password parser/cleanup cases\n", groups - 35);
|
||||
return 0;
|
||||
}
|
||||
|
||||
@@ -8,12 +8,16 @@ from pathlib import Path
|
||||
import re
|
||||
import shlex
|
||||
import subprocess
|
||||
import sys
|
||||
import tempfile
|
||||
|
||||
ROOT = Path(__file__).resolve().parents[2]
|
||||
HERE = Path(__file__).resolve().parent
|
||||
VENDOR = ROOT / "managed_components/wolfssl__wolfssh"
|
||||
REVIEWED_SHA256 = "81ff1f9166708abd5c2911e9fe57c0aee01c88b5d3f68c909ee8a856d37f36a9"
|
||||
sys.dont_write_bytecode = True
|
||||
sys.path.insert(0, str(ROOT / "tools"))
|
||||
from security_overrides import ENTRIES, render_entry
|
||||
|
||||
|
||||
def extract(source, name):
|
||||
@@ -34,10 +38,24 @@ def extract(source, name):
|
||||
return source[start:end] + "\n"
|
||||
|
||||
|
||||
def check_build_profile(database):
|
||||
def check_build_profile(database, override, expected):
|
||||
entries = json.loads(database.read_text())
|
||||
entry = next(e for e in entries if Path(e["file"]).resolve() ==
|
||||
(VENDOR / "src/internal.c").resolve())
|
||||
|
||||
def source_path(entry):
|
||||
path = Path(entry["file"])
|
||||
return (Path(entry["directory"]) / path).resolve()
|
||||
|
||||
original = (VENDOR / "src/internal.c").resolve()
|
||||
if any(source_path(e) == original for e in entries):
|
||||
raise RuntimeError("Production still compiles original internal.c; reconfigure the build")
|
||||
suffix = ("security_overrides", override.name, "internal.c")
|
||||
matches = [e for e in entries if source_path(e).parts[-3:] == suffix]
|
||||
if len(matches) != 1:
|
||||
raise RuntimeError(f"Expected one generated wolfSSH compile entry, found {len(matches)}")
|
||||
entry = matches[0]
|
||||
actual = source_path(entry).read_bytes()
|
||||
if actual != expected:
|
||||
raise RuntimeError("Generated wolfSSH source differs from render_entry; reconfigure the build")
|
||||
args = entry.get("arguments") or shlex.split(entry["command"])
|
||||
# Strip output/dependency-writing flags: this must only preprocess to stdout.
|
||||
clean = []
|
||||
@@ -59,7 +77,8 @@ def check_build_profile(database):
|
||||
"WOLFSSH_NO_ED25519", "NO_FAILURE_ON_REJECTED")
|
||||
if "WOLFSSH_NO_RSA" not in macros or any(m in macros for m in absent):
|
||||
raise RuntimeError("Resolved wolfSSH auth feature profile changed; re-audit")
|
||||
print("PASS: actual compiler preprocessing matches reviewed auth feature profile", flush=True)
|
||||
print("PASS: generated source equals render_entry; actual compiler preprocessing matches reviewed auth feature profile", flush=True)
|
||||
return actual.decode()
|
||||
|
||||
|
||||
def main():
|
||||
@@ -83,12 +102,17 @@ def main():
|
||||
if not re.search(r'^\s*wolfssl/wolfssh:\s*"1\.4\.20"\s*$', manifest, re.M):
|
||||
raise RuntimeError("Application must pin wolfSSH exactly to 1.4.20")
|
||||
|
||||
overrides = [e for e in ENTRIES if e.component == "wolfssl__wolfssh" and
|
||||
e.source == "managed_components/wolfssl__wolfssh/src/internal.c"]
|
||||
if len(overrides) != 1 or overrides[0].root != "project" or overrides[0].sha256 != REVIEWED_SHA256:
|
||||
raise RuntimeError("Expected one independently pinned project wolfSSH override")
|
||||
override = overrides[0]
|
||||
_, expected = render_entry(override, {"project": ROOT})
|
||||
if options.host_only:
|
||||
print("SKIP: production feature verification (--host-only)", flush=True)
|
||||
print("SKIP: production generated-source/feature verification (--host-only); testing render_entry output", flush=True)
|
||||
source = expected.decode()
|
||||
else:
|
||||
check_build_profile(options.compile_commands)
|
||||
|
||||
source = raw.decode()
|
||||
source = check_build_profile(options.compile_commands, override, expected)
|
||||
# Use the installed public callback data layouts, not hand-maintained copies.
|
||||
header = (VENDOR / "wolfssh/ssh.h").read_text()
|
||||
types = header[header.index("typedef struct WS_UserAuthData_Password {"):
|
||||
@@ -101,14 +125,19 @@ def main():
|
||||
"DoUserAuthRequestPassword", "DoUserAuthRequestPublicKey",
|
||||
"SendUserAuthKeyboardRequest", "DoUserAuthRequest", "GetAllowedAuth",
|
||||
"SendChannelData"]
|
||||
extracted = "\n".join(extract(source, name) for name in names)
|
||||
# Exercise the installed nonoptimizable wipe, not a memset replacement.
|
||||
misc = (VENDOR / "src/misc.c").read_text()
|
||||
wipe = extract(misc.replace("STATIC INLINE void ForceZero", "static void ForceZero"), "ForceZero")
|
||||
if "volatile byte*" not in wipe:
|
||||
raise RuntimeError("ForceZero implementation changed; re-audit")
|
||||
extracted = wipe + "\n" + "\n".join(extract(source, name) for name in names)
|
||||
with tempfile.TemporaryDirectory(prefix="wolfssh-auth-contract-") as temp:
|
||||
temp = Path(temp)
|
||||
(temp / "auth_types.h").write_text(types)
|
||||
(temp / "actual.c").write_text(extracted)
|
||||
binary = temp / "contract"
|
||||
cc = shlex.split(os.environ.get("CC", "cc"))
|
||||
subprocess.run(cc + ["-std=c99", "-Wall", "-Wextra", "-Werror",
|
||||
subprocess.run(cc + ["-std=c99", "-O2", "-Wall", "-Wextra", "-Werror",
|
||||
"-Wno-unused-parameter", "-I", str(temp),
|
||||
str(HERE / "contract.c"), "-o", str(binary)],
|
||||
check=True, timeout=30, env={**os.environ, "CCACHE_DISABLE": "1"})
|
||||
|
||||
Reference in New Issue
Block a user