From 4a9722694adfcdbf2ef438dcaaa936da553d7fd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Gro=C3=9Fmann?= Date: Mon, 6 Jul 2026 11:29:04 +0200 Subject: [PATCH] Fix http_logout() no-op from a CGI: reach httpd's store, not the CGI'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 --- credentials/include/cred.h | 13 ++++- credentials/src/credtokl.c | 30 ++++++---- include/httpd.h | 3 +- project.toml | 9 +++ src/httpd.c | 7 +++ src/httpxauth.c | 18 +++++- test/mvs/tstlout.c | 109 +++++++++++++++++++++++++++++++++++++ 7 files changed, 173 insertions(+), 16 deletions(-) create mode 100644 test/mvs/tstlout.c diff --git a/credentials/include/cred.h b/credentials/include/cred.h index 601747a..3f23696 100644 --- a/credentials/include/cred.h +++ b/credentials/include/cred.h @@ -93,10 +93,19 @@ credid_enc(CREDID *in, CREDID *out) asm("CREDIDEN"); extern CREDTOK credtok_gen(CREDID *id) asm("CREDTOKG"); -/* logout CRED for this CREDTOK */ -extern int +/* logout CRED for this CREDTOK (uses cred_array() -- caller must run in the + GRT where the credential store was initialized, i.e. httpd's own core) */ +extern int credtok_logout(CREDTOK *token) asm("CREDTOKL"); +/* credtok_logout_arr() - like credtok_logout() but operates on the caller- + supplied credential array instead of cred_array(). cred_array() reads the + current GRT's WSA, so it is empty/foreign from a CGI's GRT; passing httpd's + own array pointer (cached at cred_init() time) makes logout work from any + execution context. See issue #113 (same GRT/WSA class as #109/#111). */ +extern int +credtok_logout_arr(CRED ***array, CREDTOK *token) asm("CREDTKLA"); + /* cred_new() - allocate a new CRED struct. all parms can be NULL */ extern CRED * cred_new(CREDID *id, CREDTOK *token, ACEE *acee, unsigned char flags) asm("CREDNEW"); diff --git a/credentials/src/credtokl.c b/credentials/src/credtokl.c index e3c2bd0..04a5047 100644 --- a/credentials/src/credtokl.c +++ b/credentials/src/credtokl.c @@ -1,25 +1,28 @@ #include "cred.h" -/* logout CRED for this CREDTOK */ -int -credtok_logout(CREDTOK *token) +/* logout the CRED matching this CREDTOK in the supplied array. Split out from + credtok_logout() so a CGI can pass httpd's own array pointer: cred_array() + reads the current GRT's WSA, which is empty/foreign from a CGI's GRT, so the + scan there finds nothing and the session is never invalidated (issue #113, + same GRT/WSA class as #109/#111). */ +int +credtok_logout_arr(CRED ***array, CREDTOK *token) { int rc = 1; int lockrc; - CRED ***array = cred_array(); unsigned count, n; - + if (!token) goto quit; if (!array) goto quit; - + lockrc = lock(array, LOCK_EXC); - + count = array_count(array); for (n=1; n <= count; n++) { CRED *cred = array_get(array, n); if (!cred) continue; - + if (memcmp(&cred->token, token, sizeof(CREDTOK))==0) { cred = array_del(array, n); if (cred) { @@ -28,9 +31,16 @@ credtok_logout(CREDTOK *token) break; } } - + if (lockrc==0) unlock(array, LOCK_EXC); - + quit: return rc; } + +/* logout CRED for this CREDTOK, using this GRT's credential array */ +int +credtok_logout(CREDTOK *token) +{ + return credtok_logout_arr(cred_array(), token); +} diff --git a/include/httpd.h b/include/httpd.h index ff48923..1ab1cb8 100644 --- a/include/httpd.h +++ b/include/httpd.h @@ -152,7 +152,8 @@ struct httpd { UCHAR cfg_keepalive_max; /* 135 max reqs per connection */ USHRT cfg_session_timeout; /* 136 credential idle TTL (min), 0=off */ CREDKEY *credkey; /* 138 blowfish key ptr (issue #111) */ -}; /* 13C */ + CRED ***credarr; /* 13C credential array ptr (issue #113) */ +}; /* 140 */ /* HTTP variables */ struct httpv { diff --git a/project.toml b/project.toml index eb8b892..c7ff80a 100644 --- a/project.toml +++ b/project.toml @@ -133,6 +133,15 @@ name = "TSTGPWD" sources = ["test/mvs/tstgpwd.c"] host = false # blowfish_* + RACF SVCs are MVS-only -> test-mvs +# credtok_logout_arr() + wrapper (issue #113): logout must find, unlink and free +# the CRED in the supplied array so the token stops resolving. Guards the split +# that lets http_logout() reach httpd's store from a CGI's GRT. MVS-only (SHA256 +# token + array/lock SVCs); cred_/credtok_ resolve from the [internal] archive. +[[test]] +name = "TSTLOUT" +sources = ["test/mvs/tstlout.c"] +host = false # crent370 SVCs are MVS-only -> test-mvs + # http_new_env() / httpnenv() env-block allocation + layout (guards the M9 # offsetof sizing). Needs the HTTPV struct via httpd.h, so MVS-only; httpnenv # (HTTPNENV) resolves from the [internal] autocall archive. diff --git a/src/httpd.c b/src/httpd.c index 2b685a3..4b7f36e 100644 --- a/src/httpd.c +++ b/src/httpd.c @@ -182,6 +182,13 @@ initialize(int argc, char **argv) copy) is enough for a CGI running under its own GRT (issue #111). */ httpd->credkey = credkey(); + /* Likewise cache the credential array pointer so http_logout() can reach + httpd's real store from a CGI's GRT. cred_array() is GRT-relative too, + so a CGI calling it sees an empty/foreign array and logout invalidates + nothing; caching httpd's array pointer here (in httpd's GRT) fixes it + (issue #113, same class as #111). */ + httpd->credarr = cred_array(); + /* unlock the httpd */ unlock(httpd,0); diff --git a/src/httpxauth.c b/src/httpxauth.c index 7dc4078..33c1e53 100644 --- a/src/httpxauth.c +++ b/src/httpxauth.c @@ -97,7 +97,19 @@ http_check_auth(HTTPC *httpc, const char *classname, const char *resource, int a /* http_logout() - end the client's session: drop the CRED (and its ACEE) from ** the credential store by token and clear httpc->cred. Returns 0 on success, -** -1 if there was no session. */ +** -1 if there was no session (or no cached store). +** +** We must NOT reach the store via credtok_logout()/cred_array() here: +** cred_array() reads the credential array out of the *current* GRT's WSA, which +** is only populated in httpd's own GRT. A CGI reached through the HTTPX vector +** runs under its own GRT, where that slot is empty -- so the token scan finds +** nothing, the CRED stays in httpd's real store, and the (deterministic) token +** keeps resolving: logout would be a silent no-op (issue #113). Instead we use +** the array pointer httpd cached in the HTTPD block at cred_init() time +** (httpc->httpd->credarr), the same GRT-independent pattern http_get_password() +** uses for the blowfish key (#111). The array lives in the shared address +** space (address-keyed ENQ lock, subpool-0 storage), so mutating/freeing it +** from the CGI's GRT is safe. */ __asm__("\n&FUNC SETC 'HTTPLOUT'"); #undef http_logout int @@ -105,9 +117,9 @@ http_logout(HTTPC *httpc) { int rc; - if (!httpc || !httpc->cred) return -1; + if (!httpc || !httpc->cred || !httpc->httpd) return -1; - rc = credtok_logout(&httpc->cred->token); + rc = credtok_logout_arr(httpc->httpd->credarr, &httpc->cred->token); httpc->cred = NULL; /* the CRED it pointed at may now be freed */ return rc; } diff --git a/test/mvs/tstlout.c b/test/mvs/tstlout.c new file mode 100644 index 0000000..c996df4 --- /dev/null +++ b/test/mvs/tstlout.c @@ -0,0 +1,109 @@ +/* TSTLOUT.C +** Test for credtok_logout_arr() (CREDTKLA, credentials/src/credtokl.c) and its +** credtok_logout() wrapper -- the store side of http_logout() (HTTPLOUT). +** +** Issue #113: http_logout() was a silent no-op when called from a CGI. It +** reached the store via credtok_logout() -> cred_array(), which reads the +** credential array out of the *current* GRT's WSA. A CGI 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 -- +** logout returned "success" but never invalidated the session. The fix splits +** out credtok_logout_arr(array, token), which operates on a caller-supplied +** array pointer (httpd caches its own in HTTPD.credarr at cred_init() time), so +** logout reaches httpd's real store from any GRT. Same class as #109/#111. +** +** This test runs single-GRT (a unit test cannot cross the CGI GRT boundary -- +** that is verified live via mvsMF #168), so it guards the *refactor*: given an +** explicit array, credtok_logout_arr() must find, unlink and free the matching +** CRED so the token stops resolving; the wrapper must still work via +** cred_array(); and the NULL/absent cases must be no-ops. It needs no RACF and +** no APF: the CREDs carry no ACEE, so cred_free() does no racf_logout(). +** +** MVS-only (SHA256 token + array/lock SVCs live in crent370), like tstcred. +*/ +#include "cred.h" +#include + +/* build + store a CRED for (userid,addr) and return it; *tok gets its token */ +static CRED * +add_cred(unsigned addr, const char *user, CREDTOK *tok) +{ + CREDID id; + CRED *cred; + + credid_init(&id); + credid_update(&id, &addr, (unsigned char *)user, (unsigned char *)"SYS1"); + *tok = credtok_gen(&id); + + cred = cred_new(&id, tok, NULL, 0); /* no ACEE -> cred_free needs no RACF */ + if (cred) cred_add(cred); + return cred; +} + +int main(void) +{ + CRED ***array; + CRED *credA; + CRED *credB; + CREDTOK tokA; + CREDTOK tokB; + CREDTOK tokX; + CREDID idX; + unsigned before; + int rc; + + printf("=== HTTPD credtok_logout (issue #113) tests ===\n"); + + rc = cred_init(NULL, 0); + CHECK(rc == 0, "cred_init() succeeds"); + if (rc) return mbt_test_summary("TSTLOUT"); + + array = cred_array(); + CHECK(array != NULL, "cred_array() returns the store"); + if (!array) return mbt_test_summary("TSTLOUT"); + + /* --- credtok_logout_arr(): the array-explicit path http_logout() uses --- */ + credA = add_cred(0xC0A80101, "USERA", &tokA); + CHECK(credA != NULL, "cred_new/cred_add (USERA) succeeds"); + if (credA) { + CHECK(cred_find_by_token(&tokA) == credA, + "token resolves to the CRED before logout"); + + before = array_count(array); + rc = credtok_logout_arr(array, &tokA); + CHECK(rc == 0, "credtok_logout_arr returns 0 (found + freed)"); + CHECK(cred_find_by_token(&tokA) == NULL, + "token no longer resolves after logout (the #113 fix)"); + CHECK(array_count(array) == before - 1, + "the CRED was unlinked from the store"); + } + + /* logging the same (now absent) token out again finds nothing -> rc 1 */ + rc = credtok_logout_arr(array, &tokA); + CHECK(rc == 1, "credtok_logout_arr on an absent token is a no-op (rc 1)"); + + /* a token that was never added -> rc 1 */ + credid_init(&idX); + credid_update(&idX, NULL, (unsigned char *)"NOBODY", (unsigned char *)"X"); + tokX = credtok_gen(&idX); + rc = credtok_logout_arr(array, &tokX); + CHECK(rc == 1, "credtok_logout_arr on an unknown token is a no-op (rc 1)"); + + /* NULL guards must not crash and must report "nothing done" (rc 1) */ + CHECK(credtok_logout_arr(NULL, &tokA) == 1, "NULL array -> rc 1"); + CHECK(credtok_logout_arr(array, NULL) == 1, "NULL token -> rc 1"); + + /* --- the credtok_logout() wrapper still works via cred_array() --------- */ + credB = add_cred(0xC0A80102, "USERB", &tokB); + CHECK(credB != NULL, "cred_new/cred_add (USERB) succeeds"); + if (credB) { + CHECK(cred_find_by_token(&tokB) == credB, + "wrapper: token resolves before logout"); + rc = credtok_logout(&tokB); + CHECK(rc == 0, "credtok_logout() wrapper returns 0"); + CHECK(cred_find_by_token(&tokB) == NULL, + "wrapper: token no longer resolves after logout"); + } + + return mbt_test_summary("TSTLOUT"); +}