Mitigate timing oracle using dummy hash cache - #1116
Conversation
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1116
Scan targets checked: wolfssh-bugs, wolfssh-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
6be2e53 to
49470f2
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1116
Scan targets checked: wolfssh-bugs, wolfssh-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
49470f2 to
f1ec2b3
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1116
Scan targets checked: wolfssh-bugs, wolfssh-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
f1ec2b3 to
4c429fa
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1116
Scan targets checked: wolfssh-bugs, wolfssh-src
No new issues found in the changed files. ✅
4c429fa to
101b87a
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1116
Scan targets checked: wolfssh-bugs, wolfssh-src
No new issues found in the changed files. ✅
101b87a to
be56e50
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1116
Scan targets checked: wolfssh-bugs, wolfssh-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
be56e50 to
568fea0
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1116
Scan targets checked: wolfssh-bugs, wolfssh-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
568fea0 to
b1ea836
Compare
b1ea836 to
dc37d33
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1116
Scan targets checked: wolfssh-bugs, wolfssh-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
dc37d33 to
5732e90
Compare
5732e90 to
2dd784c
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1116
Scan targets checked: wolfssh-bugs, wolfssh-src
Findings: 3
3 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
2dd784c to
da0f70b
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1116
Scan targets checked: wolfssh-bugs, wolfssh-src
No new issues found in the changed files. ✅
da0f70b to
1b6b43a
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1116
Scan targets checked: wolfssh-bugs, wolfssh-src
Findings: 3
3 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
| savedGetspnam = wsshd_getspnam_cb; | ||
| wsshd_getspnam_cb = stub_getspnam_null; | ||
|
|
||
| wolfSSHD_SetCachedFakeHashForTest("!$6$wolfsshtestsalt$priorFakeHash$"); |
There was a problem hiding this comment.
🟠 [Medium] test_AuthInit_degradedMode never exercises the getspnam-failure path · Dead error handling
wolfSSHD_SetCachedFakeHashForTest(...) sets numCachedFakeHashes=1 before calling wolfSSHD_AuthInit(), but AuthInit's getspnam("root") fallback only runs if (numCachedFakeHashes == 0), so the stubbed getspnam failure is never actually invoked; the test passes vacuously.
Fix: Reset the cache to empty (not seed it) before calling AuthInit, then separately assert degraded-mode output, to actually exercise the getspnam-NULL branch.
| savedGetspnam = wsshd_getspnam_cb; | ||
| wsshd_getspnam_cb = stub_getspnam_nullPassword; | ||
|
|
||
| wolfSSHD_SetCachedFakeHashForTest("!$6$wolfsshtestsalt$priorFakeHash$"); |
There was a problem hiding this comment.
🟠 [Medium] test_AuthInit_nullPasswordField never exercises the null sp_pwdp path · Dead error handling
Seeding cachedFakeHashes via wolfSSHD_SetCachedFakeHashForTest before wolfSSHD_AuthInit() makes numCachedFakeHashes non-zero, so AuthInit's root-shadow lookup (and thus stub_getspnam_nullPassword) is skipped entirely; the intended NULL-sp_pwdp handling is never run.
Fix: Clear the cache (numCachedFakeHashes==0) before calling AuthInit so the stubbed getspnam is actually reached.
|
|
||
| #ifndef WOLFSSHD_UNIT_TEST | ||
| WFILE* f = NULL; | ||
| char line[512]; |
There was a problem hiding this comment.
🔵 [Low] Real shadow password hashes left unzeroed in stack buffer · Missing ForceZero
line (and the nested dump) hold the full real password hash for every parsed /etc/shadow entry, but unlike every other password-hash buffer in this file (storedHashCpy, hashBuf, pwStr), they are never WS_FORCEZERO'd before the stack frame is reused or returned.
Fix: WS_FORCEZERO the line and dump buffers (or at least the password field) before each loop iteration ends and before returning.
4 participants
Does a fake crypt() operation when an authentication attempt is rejected early and caches hash type at SSHD startup. CI testing is added for this change.