Fail closed when a route's auth policy cannot be built - #163
Conversation
RES=class:resource strdup'd both strings unchecked. A NULL resclass made both RACF gates in auth_gate() false, so the route's resource check silently vanished and the request fell back to the global LOGIN policy; a NULL resname alone still forced authentication but never authorized the request against the resource. Neither said anything. Skipping the route is not a safe fallback either: an unregistered route does not disappear, its requests are served under the global LOGIN default, and for a LOC= prefix under LOGIN NONE that is the whole protected subtree served to anyone. The same applies to the two arms next to it -- http_add_cgi() returning NULL, and the strdup() of the line itself -- which dropped a policy-bearing route just as quietly. Now: RES= keeps resclass/resname both-or-neither and marks the policy failed if either strdup fails (HTTPD418E); a route that carried a binding policy (RES=, or AUTH=FORM/BASIC) and could not be registered issues HTTPD419E and sets HTTPD_FLAG_CFGERR; http_config() checks that flag after the Parmlib read and returns 8 with HTTPD420E, before do_bind(), so the listener is never opened. AUTH=NONE and policy-less routes stay warnings -- the fallback can only be stricter. A line that could not be tokenized is treated as policy-bearing, since the parser cannot tell. Fixes #161
|
Adjacent fail-open found while reading |
Live verification (mvsdev, MVS 3.8j)Deployed, activated via New module is really the one running — Success path intact. All six configured routes register exactly as before; startup reaches RES= acceptance path. The live Parmlib carries no
Both halves of the pair present. The gate then behaved as configured: unauthenticated Probe line removed, |
HTTPD418E/419E can carry raw Parmlib text: 419E is passed the whole untokenized line when strdup() of it fails, and 418E the RES= operand. Truncate both, matching the %.40s parse_line() already uses for HTTPD020W. HTTPDSRV's httpd->flag decode lists every bit defined in httpd.h, so add CFGERR to it -- a server that can answer the request never has it set (it aborts before the listener is bound), but a bit missing from that table is how #146 and #155 started.
|
Scope note on what was verified where: the live run above was on d33fdf1. 7248061 came after it and is bounds + display only — |
HTTPD404E printed the CONFIG= parm, which http_config() has ignored since the Parmlib migration -- and which the shipped STC procedure does not pass at all, so the one message naming what failed to process always read "processing (null)". After #163 and #165 gave the parser real reasons to refuse a startup, that message got a lot more visible. The configuration's real identity is the HTTPPRM DD, and the PROC allocates it as &D(&M), so which member is in effect is a startup choice an operator can genuinely need to confirm. parmlib_name() resolves it from the DSAB -> TIOT entry -> JFCB, read-only pointer chasing with no SVC and no allocation, falling back to the DD name if any link is missing. It is reported before parsing, so a later error is already attributed: HTTPD022I Configuration from SYS2.PARMLIB(HTTPPRM0) HTTPD404E drops the operand accordingly. CONFIG= is now answered rather than swallowed. Silently ignoring it is worse than not accepting it -- an operator who passes it believes it took effect, and migration.md still tells 3.x users to remove theirs, so a leftover is exactly what this catches. HTTPD024W says it did nothing and names the mechanism that does. Fixes #166
1 participant
Fixes #161
The defect
parse_kv_tail()strdup'd bothRES=class:resourcestrings unchecked. A NULLresclassmakes both RACF gates inauth_gate()(src/httppc.c:376,:394)false, so the route's resource check silently vanishes and the request falls
back to the global
LOGINpolicy. A NULLresnamealone still forcesauthentication at
:376but the authorize call at:394needs both — therequest authenticates and is then never checked against the resource. Neither
case said anything.
Why "skip the route" is not the fix
An unregistered route does not disappear — its requests are served under the
global
LOGINdefault. For aMOD=route that lands on 404 (closed), but for aLOC=prefix underLOGIN NONEit hands out the whole subtree theRES=wasmeant to protect. So skipping is the fail-open the issue describes, and the
only strictly closed answer is to refuse to start.
httpd.c:94alreadyimplements that path (HTTPD404E → close listener → quit).
The change
RES=keepsresclass/resnameboth-or-neither: if eitherstrdup()fails, both are dropped,
resattris cleared, the policy is marked failed(
HTTPD418E), and the loop breaks so a laterRES=on the same line cannotclear the failure.
apply_policy()is the only writer of those two fields, sothe invariant holds for every registered route.
HTTPD419Eand setsHTTPD_FLAG_CFGERR(new,0x08, a free bit — no HTTPDoffset moves).
http_config()checks that flag right after the Parmlib read and returns 8with
HTTPD420E— beforedo_bind(), so the listener is never opened andhttpd.c's cleanup is a no-op.
RES=, orAUTH=FORM/BASIC.AUTH=NONEand policy-less routesstay warnings: the fallback can only be stricter than what they asked for.
Scope: two arms wider than the issue
The issue names the
strduppair. Two neighbours in the same functions are theidentical defect and are fixed with it — flagging them rather than leaving them
in the diff for a reviewer to find:
http_add_cgi()returning NULL (calloc/strdup/array_add OOM insidehttpacgi.c). Reported as HTTPD035W/HTTPD416W with startup continuing —for a policy-bearing
LOC=route that is the same fail-open.strdup(value)at the top ofparse_mod/parse_loc. The line cannot betokenized, so whether it carried a policy is unknowable. Treated as if it
did, which means any undippable route line is now fatal, including a
policy-less
MOD=HELLO /hello. That is a startup-behaviour change beyond theissue; the reasoning is that the parser genuinely cannot tell, and a region
that cannot dup 40 bytes at startup is finished anyway.
Testing
No new test. The OOM path is not reproducible in a host test — there is no
allocator interposition on this toolchain, and a
strduplink-override isfragile on macOS. Building either an injectable allocator or a symbol override
into production code for this would be worse than the honest note.
Verified instead by:
make modules/make testclean under-Wall -Werror(cc370)make test-host— 4 suites, 63 assertions, 0 failin
/.dsrv?target=MOD,mgr->funcmoved so it is genuinely the new module,and a temporary
RES=probe route registered with both halves of the pairintact. Full evidence in the verification comment below.
The success path was restructured in both parsers (
parse_locwent from anunconditional to a conditional
http_add_cgi, and both message blocks wereinverted), which the compiler cannot check — that live route-table check is what
covers it.
Docs
docs/configuration.mdgains a paragraph under theAUTH=/RES=sectionstating the rule: a route carrying an auth policy is registered or the server
does not start.