Fix http_get_userid() returning garbage on CGI-invoked requests#110
Merged
Conversation
http_get_userid() decrypted httpc->cred->id via credid_dec(), whose blowfish key (credkey()) lives in per-GRT WSA and is initialized only in httpd's own GRT by cred_init(). A CGI reached through the HTTPX vector (e.g. mvsMF) runs under its own C runtime / GRT, where that key is still zero, so credid_dec() decrypted with an all-zero key and returned garbage -- but only for CGI-invoked requests. httpd's own credid_dec() callers run in httpd's GRT and were unaffected, which is why http_get_acee() and the SMF/console userid stayed correct. Read the userid straight from the RACF ACEE (aceeuser is length-prefixed) -- the RACF-canonical identity, no key needed, correct from any execution context, and the same field http_get_acee() consumers already read. The CREDID encryption (which protects the password held in the in-memory credential cache) is untouched. Verified on MVS: the mvsMF /zosmf/test?fn=userid probe now returns IBMUSER (fresh and cache-hit); a wrong password still gets 401. Tests (MVS-only): TSTGUID drives the real http_get_userid() over a fabricated-ACEE CRED and a real cred_login()+cache-hit path; TSTCRED covers the credid_enc()/credid_dec() round-trip and the CREDID_USER_ENCRYPT flag invariant. Fixes #109
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 #109
Root cause (confirmed live on MVS)
http_get_userid()decryptedhttpc->cred->idviacredid_dec(), whoseblowfish key (
credkey()) lives in per-GRT WSA and is initialized only inhttpd's own GRT by
cred_init(). A CGI reached through the HTTPX vector (mvsMF)runs under its own C runtime / GRT, where that key is still zero — so
credid_dec()decrypted with an all-zero key and returned garbage.Proven with temporary instrumentation on the live server (encrypt vs decrypt
context):
Same ciphertext, flag set, but a different, uninitialized key in the CGI's
GRT (the GRT varied per request). This is why
http_get_acee()(raw pointer)and httpd's own
credid_dec()callers (SMF, console, login WTO — all in httpd'sGRT) were unaffected, and why an in-module unit test could not reproduce it.
Fix
Read the userid straight from the RACF ACEE (
aceeuser, length-prefixed) —the RACF-canonical identity, no key needed, correct from any execution
context, and the same field
http_get_acee()consumers already read. TheCREDID encryption (which protects the password held in the in-memory credential
cache) is untouched.
Verification
curl -u ibmuser:sys1 .../zosmf/test?fn=userid→IBMUSER(fresh andcache-hit); wrong password still
401.make test-mvs: TSTGUID (realhttp_get_userid()over a fabricated-ACEECRED + a real
cred_login()+cache-hit path) and TSTCRED (credidencrypt/decrypt round-trip + flag invariant) both pass.
Note
http_get_userid()stays in the HTTPX vector (append-only ABI; already consumedby mvsMF
jobsapi.c) — it is now a thin, layout-independent wrapper over theACEE userid. The per-GRT
credkey()limitation is inherent to the credentialspackage and only ever affected this one CGI-context decrypt path, which this
change removes.