Fix http_logout() no-op from a CGI (GRT/WSA credential-store context)#115
Merged
Conversation
…s GRT http_logout() invalidated nothing when called from a CGI: the session token kept resolving after a "successful" logout. It reached the store via credtok_logout() -> cred_array(), which reads the credential array out of the current GRT's WSA. A CGI reached through the HTTPX vector runs under its own GRT, where that slot is empty, so the token scan found nothing, the CRED stayed in httpd's real store, and the deterministic CREDTOK kept resolving. Same GRT/WSA class as #109 (userid) and #111 (password), but the logout path was never corrected. Cache httpd's credential array pointer in the HTTPD control block (httpd->credarr = cred_array()) at cred_init() time, in httpd's own GRT where it is valid -- the same GRT-independent pattern http_get_password() uses for the blowfish key. Split credtok_logout() into an array-explicit credtok_logout_arr(array, token); the token/wrapper keeps using cred_array() for httpd-core callers, while http_logout() passes httpc->httpd->credarr so it operates on httpd's real store from any GRT. Cross-GRT free is safe here: the sysroot libc is built without USE_MEMMGR (no per-runtime free-list -- @@getm/@@freem keep the length in a prefix on the block itself), so free() is a pure function of (block, subpool). httpd already frees CREDs across TCB boundaries (the reaper frees CREDs that cred_login allocated on worker threads); this adds no new cross-TCB free pattern. The array lock is an address-keyed ENQ (address-space-wide), so locking httpd's real array address from the CGI coordinates with the core. Adds TSTLOUT covering the array-explicit logout and the wrapper. Fixes #113
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #113
Problem
http_logout()(HTTPX export) invalidated nothing when called from a CGI: after a "successful" logout the session token kept resolving. It reached the store viacredtok_logout()→cred_array(), which reads the credential array out of the current GRT's WSA. A CGI reached through the HTTPX vector runs under its own GRT, where that slot is empty — so the token scan found nothing, the CRED stayed in httpd's real store, and the deterministicCREDTOKkept resolving. Same GRT/WSA class as #109 (userid) and #111 (password); the logout path was just never corrected.Fix (issue's Option 2 — the #111 pattern)
httpd->credarr = cred_array()) atcred_init()time, in httpd's own GRT where it is valid.credtok_logout(token)into an array-explicitcredtok_logout_arr(array, token); the wrapper keeps usingcred_array()for httpd-core callers (httpcred.cform logout), whilehttp_logout()passeshttpc->httpd->credarrso it operates on httpd's real store from any GRT.LOCK.%08X) → address-space-wide, so locking httpd's real array address from the CGI coordinates with the core.Option 1 (make
cred_array()AS-wide via theOURKEYhigh-byte pattern) is not viable — #111 already established that pattern is still per-GRT.Why cross-GRT
free()is safe (this is the crux, and it's proven, not assumed)cred_free()runs from the CGI's runtime but frees a CRED that httpd's runtimemalloc'd. That is safe here:libc.ais built withoutUSE_MEMMGR(zeromemmgrsymbols in the built archive;getmainis what's linked).@@getm.asmstores the block length in an 8-byte prefix on the block itself;@@freem.asmreads it back from the block and FREEMAINs. Sofree()is a pure function of (block, subpool) — any runtime can free any block.cred_loginallocated on worker threads, and HTTPC is alloc'd/freed across threads. This change introduces no new cross-TCB free pattern.Note on
cred_freercInherited behavior: if
cred_freehits itstrylock==4 path it returns 4 and the CRED is unlinked-but-not-freed (a transient leak reclaimed later).array_delruns before the free, so the token stops resolving regardless — logout still succeeds even in that edge; a reviewer shouldn't read rc=4 as "logout failed".Verification
make test-mvs: 114 assertions PASS, 0 FAIL (batch + TSO) across TSTLOUT + TSTGPWD/TSTGUID/TSTCRED — no regression from thecredtok_logoutsplit or the new HTTPD field.credtok_logout_arr()finds, unlinks and frees the matching CRED so the token stops resolving; the wrapper still works; NULL/absent cases are no-ops.f12ff9balready callshttp_logout(it just ignores the rc), so with this httpd deployed the final GET flips 200 → 401 with no mvsMF change. That's runnable on live MVS (mvsMF: implement /zosmf/services/authenticate (Authentication Service) mvsmf#162 / PR #168) once deployed.No httpcgi.h change
http_logout's signature is unchanged andHTTPDis opaque in the publichttpcgi.h(CGIs only rely onhttpxat offset 0x08, which the appendedcredarrfield does not disturb) — so, likecredkeyfrom #111, the new field is not mirrored there.