Repository navigation
wolfsshd: config file parser fixes - #1347
Conversation
c4d31e3 to
cb1501d
Compare
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1347
Scan targets checked: wolfssh-src, wolfssh-bugs
Not reviewed (no scan target covers them): apps/wolfsshd/configuration.h
Coverage: 1 of 2 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
1470a3d to
15209d0
Compare
|
@claude review |
GetConfigInt() now requires the whole value to be decimal digits, so "Port 22abc" fails to load instead of reading 22. LoginGraceTime takes the sshd_config(5) time format, number and unit groups summed as seconds, so "1h30m" is 5400 and "2junk" is rejected. - accept the s, m, h, d and w units in either case, zero-padded groups, and a bare trailing number as seconds, as OpenSSH does - reject values above 0x7FFFFFFF, per group and in the sum - write the bad Include fixture as "LoginGraceTime bad", since a zero-padded time is now valid - add Port, Protocol and LoginGraceTime cases to the parser test Issue: F-10549, F-17100
HandleInclude() now checks the first character of the pattern for a '*', so a pattern with no directory part, such as "Include *.conf", is expanded rather than opened as a literal file. The '*' may match an empty sequence, but the fixed prefix and postfix may not overlap. - skip names with a leading '.' unless the pattern starts with '.', as glob(3) does - add tests for foo*, *foo, f*o, a*a and the overlapping ab*ba - add a leading-wildcard pattern that matches nothing Issue: F-10550, F-14632
Match User and Match Group now take a comma separated list of exact names and apply when any entry equals the user or one of the user's groups. - reject '*', '?', '!' and empty entries in a Match name list - add parser cases and a Match list lookup test Issue: F-11661
ConfigLoad() reads lines of up to 1023 bytes and drains the rest of a longer one, so its tail is never parsed as a line of its own. - skip an over-long comment whole and fail the load on an over-long directive, also one pushed out by leading whitespace - ignore whitespace past the end of the buffer - drop the one-byte peek past a full buffer, which the drain replaces; it failed every over-long comment - add a long-line load test Issue: F-14395
15209d0 to
443b1c9
Compare
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1347
Scan targets checked: wolfssh-src, wolfssh-bugs
Coverage: 2 of 2 in-scope changed file(s) opened by the reviewer
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: Max
Four fixes to how wolfsshd parses its config file.
This overlaps the long-line change in #1344; whichever merges second
will be rebased onto the other.
Issue: F-10549, F-10550, F-14632, F-11661, F-14395