Repository navigation
client: harden password and prompt handling - #1339
ejohnstown wants to merge 3 commits into
Conversation
97a8dad to
2a69602
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1339
Scan targets checked: wolfssh-src, wolfssh-bugs
Not reviewed (no scan target covers them): apps/wolfssh/common.h, examples/client/common.h, scripts/sshclient.test
Coverage: 5 of 8 in-scope changed file(s) opened by the reviewer; not opened: examples/scpclient/scpclient.c, examples/sftpclient/sftpclient.c, tests/auth.c
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Review tier: Lite
ClientUserAuth() hands the library its static password buffer, and the library sends from it after the callback returns. The clients now wipe it, and the example's keyboard-interactive responses, as soon as connect returns instead of holding them for the whole session. - ClientClearPassword() in both common.c copies, called after connect in the wolfssh app, the example client and the sftp client - the password arm wipes before each prompt and on failure, so a retry keeps no tail of a longer password - the keyboard arm wipes its scratch copy after each response - tests: regress and auth cases for the retry tail, the wipe and a failed read Issue: F-8846
The password prompt reads stdin with fgets(), and a buffered stdin keeps its own copy of the line in libc for the life of the process. The clients make stdin unbuffered before any stdin I/O, so the typed password lands only in the buffer ClientUserAuth() wipes. - the wolfssh app, and the example client, sftp client and scp client whether or not -P was given, as keyboard-interactive reads stdin too - ClientUnbufferStdin() acts once per process and reports a failure - scripts/sshclient.test sends the session data right after the password, with no sleep Issue: F-15669
The example client prints the keyboard-interactive name, instruction and prompts through ClientPrintSafe(), which prints a '.' in place of each control character other than tab and newline, DEL, and each byte 0x80 and up, per RFC 4256 section 3.3 and RFC 4251 section 9.2. - tests: auth case capturing the printed strings Issue: F-15534
2a69602 to
effb203
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1339
Scan targets checked: none
Unchanged since last review (not re-run): wolfssh-src, wolfssh-bugs
Not reviewed (no scan target covers them): apps/wolfssh/common.h, examples/client/common.h, scripts/sshclient.test
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Review tier: Lite
The example clients and the wolfssh app wipe the password and the
keyboard-interactive responses once connect returns, read them from
an unbuffered stdin, and mask control bytes in server prompts.
wiped before each prompt.
keyboard-interactive strings.
publickey, as OpenSSH does. The method order is left for a later
change.
Issue: F-8846, F-15669, F-15534