Repository navigation
wolfsshd: honor none keywords, refuse Match Group - #1348
ejohnstown wants to merge 3 commits into
Conversation
|
@claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline finding, I checked whether relaxing the two SetupCTX TrustedSystemCAKeys gates for AuthorizedKeysFile none nodes could let a certificate log in on CN or UPN alone. It can't: with none set, X.509 certs skip the CA-only success branch in RequestAuthentication. On both the Unix and Windows check-key paths they then reach SearchForPubKey, which refuses them before any path is resolved.
Extended reasoning...
This PR changes wolfsshd authentication config handling: the AuthorizedKeysFile/ChrootDirectory none keyword, the X.509 CA-only fallback, the SystemCA setup gates, and refusing Match Group on Windows. That is security-sensitive auth code. I traced the none flag through ConfigCopy, ConfigComposeFrom, RequestAuthentication, and both callers of SearchForPubKey (CheckPublicKeyUnix and CheckPublicKeyWIN) to confirm that the relaxed SetupCTX gates do not widen certificate access.
e3b44e9 to
a5e63cf
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1348
Scan targets checked: wolfssh-src, wolfssh-bugs
Not reviewed (no scan target covers them): .github/workflows/windows-check.yml, README.md, apps/wolfsshd/configuration.h, apps/wolfsshd/test/run_all_sshd_tests.sh, apps/wolfsshd/test/sshd_authkeys_none_test.sh, apps/wolfsshd/test/sshd_chroot_none_test.sh, apps/wolfsshd/test/sshd_match_group_win_test.ps1
Coverage: 3 of 4 in-scope changed file(s) opened by the reviewer; not opened: apps/wolfsshd/test/test_configuration.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
"AuthorizedKeysFile none" now turns off the authorized keys file, as in OpenSSH, instead of naming ~/none, a file the user controls. The value stays "none" through Match composition, and SearchForPubKey refuses it before resolving any path. - match "none" case-insensitively, as OpenSSH does - OpenSSH certificates still check against TrustedUserCAKeys - X.509 certificates do not fall back to CA-only under "none" - a "none" node passes the wolfSSH_TrustedSystemCAKeys startup gates - add sshd_authkeys_none_test.sh: key in ~/none, X.509 CA-only and system CA gate cases, each set globally and in a Match block Issue: F-15558
"ChrootDirectory none" now means no chroot, as in OpenSSH, instead of a relative chdir() target. The keyword still counts as set, so a Match block using it exempts a user from a global ChrootDirectory. - match "none" case-insensitively, as OpenSSH does - add sshd_chroot_none_test.sh: global and Match cases against an empty global chroot Issue: F-16811
Windows wolfsshd does not look up a user's groups, so a Match Group block could never apply and its restrictions were skipped. The config load now rejects a Group selector there, as it does other unsupported Match selectors. - WOLFSSHD_NO_MATCH_GROUP gates the rejection, set for _WIN32 - add sshd_match_group_win_test.ps1 to the Windows check workflow Issue: F-15559
a5e63cf to
3b51d0f
Compare
AuthorizedKeysFile none and ChrootDirectory none now mean what they do
in OpenSSH, and Windows wolfsshd refuses Match Group, which it could
never apply.
~/none; X.509 does not fall back to CA-only under it
user from a global one
Issue: F-15558, F-16811, F-15559