mirror of
https://github.com/apache/httpd.git
synced 2026-10-23 22:49:01 +03:00
mod_auth_digest: Fix handling of a request with no per-request record:
The per-request record is created in a post_read_request hook, which does not run for a request rejected earlier (e.g. no Host on HTTP/1.1). An ErrorDocument directing such a request into a Digest-protected location then reached the auth hooks, which walked ->prev to the original request and used its record without checking it had been created. * modules/aaa/mod_auth_digest.c (make_digest_rec): New, split from init_digest_request, storing the record on a given request. (get_digest_rec): New; walk to the shared record and create it if the originating request never ran init_digest_request. (authenticate_digest_user, hook_note_digest_auth_failure): Use it in place of the open-coded walk, which did not check for a missing record. * test/modules/aaa/conftest.py: Add an ErrorDocument 400 into a Digest-protected location. * test/modules/aaa/test_012_errordoc.py: New test suite. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> GitHub: PR #787 git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1939032 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
@@ -1082,6 +1082,21 @@ static enum hdr_sts parse_digest_header(request_rec *r,
|
||||
}
|
||||
|
||||
|
||||
/* Create the per-request Digest record describing request r, store it on
|
||||
* store's request_config, and parse r's Authorization header into it. */
|
||||
static digest_header_rec *make_digest_rec(request_rec *store, request_rec *r)
|
||||
{
|
||||
digest_header_rec *resp = apr_pcalloc(store->pool, sizeof(*resp));
|
||||
|
||||
resp->raw_request_uri = r->unparsed_uri;
|
||||
resp->psd_request_uri = &r->parsed_uri;
|
||||
resp->method = r->method;
|
||||
ap_set_module_config(store->request_config, &auth_digest_module, resp);
|
||||
|
||||
resp->auth_hdr_sts = parse_digest_header(r, resp);
|
||||
return resp;
|
||||
}
|
||||
|
||||
/* Set up the per-request record: this is the place to get the request-uri
|
||||
* (before any subrequests etc are initiated), to initialize the
|
||||
* request_config, and to parse the Authorization header.
|
||||
@@ -1097,24 +1112,38 @@ static enum hdr_sts parse_digest_header(request_rec *r,
|
||||
*/
|
||||
static int init_digest_request(request_rec *r)
|
||||
{
|
||||
digest_header_rec *resp;
|
||||
|
||||
if (!ap_is_initial_req(r)) {
|
||||
return DECLINED;
|
||||
}
|
||||
|
||||
resp = apr_pcalloc(r->pool, sizeof(digest_header_rec));
|
||||
resp->raw_request_uri = r->unparsed_uri;
|
||||
resp->psd_request_uri = &r->parsed_uri;
|
||||
resp->needed_auth = 0;
|
||||
resp->method = r->method;
|
||||
ap_set_module_config(r->request_config, &auth_digest_module, resp);
|
||||
|
||||
resp->auth_hdr_sts = parse_digest_header(r, resp);
|
||||
|
||||
make_digest_rec(r, r);
|
||||
return DECLINED;
|
||||
}
|
||||
|
||||
/* Return the Digest record shared across this request tree, which lives on
|
||||
* the initial request. init_digest_request creates it in post_read_request;
|
||||
* a request which never ran that hook (one rejected before it and brought
|
||||
* here by an ErrorDocument or other internal redirect) has none, so create
|
||||
* it now rather than dereferencing NULL. */
|
||||
static digest_header_rec *get_digest_rec(request_rec *r)
|
||||
{
|
||||
request_rec *mainreq = r;
|
||||
digest_header_rec *resp;
|
||||
|
||||
while (mainreq->main != NULL) {
|
||||
mainreq = mainreq->main;
|
||||
}
|
||||
while (mainreq->prev != NULL) {
|
||||
mainreq = mainreq->prev;
|
||||
}
|
||||
|
||||
resp = ap_get_module_config(mainreq->request_config, &auth_digest_module);
|
||||
if (resp == NULL) {
|
||||
resp = make_digest_rec(mainreq, r);
|
||||
}
|
||||
return resp;
|
||||
}
|
||||
|
||||
|
||||
/* Writes the hash part of the server nonce to hash, which must be of
|
||||
* minimum size (NONCE_HASH_LEN+1). */
|
||||
@@ -1288,29 +1317,15 @@ static int note_digest_auth_failure(request_rec *r,
|
||||
|
||||
static int hook_note_digest_auth_failure(request_rec *r, const char *auth_type)
|
||||
{
|
||||
request_rec *mainreq;
|
||||
digest_header_rec *resp;
|
||||
digest_config_rec *conf;
|
||||
|
||||
if (ap_cstr_casecmp(auth_type, "Digest"))
|
||||
return DECLINED;
|
||||
|
||||
/* get the client response and mark */
|
||||
|
||||
mainreq = r;
|
||||
while (mainreq->main != NULL) {
|
||||
mainreq = mainreq->main;
|
||||
}
|
||||
while (mainreq->prev != NULL) {
|
||||
mainreq = mainreq->prev;
|
||||
}
|
||||
resp = (digest_header_rec *) ap_get_module_config(mainreq->request_config,
|
||||
&auth_digest_module);
|
||||
resp = get_digest_rec(r);
|
||||
resp->needed_auth = 1;
|
||||
|
||||
|
||||
/* get our conf */
|
||||
|
||||
conf = (digest_config_rec *) ap_get_module_config(r->per_dir_config,
|
||||
&auth_digest_module);
|
||||
|
||||
@@ -1556,7 +1571,6 @@ static int authenticate_digest_user(request_rec *r)
|
||||
{
|
||||
digest_config_rec *conf;
|
||||
digest_header_rec *resp;
|
||||
request_rec *mainreq;
|
||||
const char *t;
|
||||
int res;
|
||||
authn_status return_code;
|
||||
@@ -1574,24 +1588,11 @@ static int authenticate_digest_user(request_rec *r)
|
||||
return HTTP_INTERNAL_SERVER_ERROR;
|
||||
}
|
||||
|
||||
|
||||
/* get the client response and mark */
|
||||
|
||||
mainreq = r;
|
||||
while (mainreq->main != NULL) {
|
||||
mainreq = mainreq->main;
|
||||
}
|
||||
while (mainreq->prev != NULL) {
|
||||
mainreq = mainreq->prev;
|
||||
}
|
||||
resp = (digest_header_rec *) ap_get_module_config(mainreq->request_config,
|
||||
&auth_digest_module);
|
||||
resp = get_digest_rec(r);
|
||||
resp->needed_auth = 1;
|
||||
|
||||
realm = ap_auth_name(r);
|
||||
|
||||
/* get our conf */
|
||||
|
||||
conf = (digest_config_rec *) ap_get_module_config(r->per_dir_config,
|
||||
&auth_digest_module);
|
||||
|
||||
|
||||
@@ -94,6 +94,11 @@ def env(pytestconfig) -> AAATestEnv:
|
||||
# AuthDigestProvider intentionally omitted: falls back to "file".
|
||||
f'AuthUserFile "{pwfile}"',
|
||||
]))
|
||||
# A request which fails header validation (e.g. no Host on HTTP/1.1)
|
||||
# is answered before the Digest module's post_read_request runs, so it
|
||||
# has no per-request Digest record; an ErrorDocument redirecting it into
|
||||
# a Digest-protected location must not then dereference that record.
|
||||
conf.add('ErrorDocument 400 /digest/default/secret.txt')
|
||||
conf.install()
|
||||
assert env.apache_restart() == 0
|
||||
return env
|
||||
|
||||
37
test/modules/aaa/test_012_errordoc.py
Normal file
37
test/modules/aaa/test_012_errordoc.py
Normal file
@@ -0,0 +1,37 @@
|
||||
"""An ErrorDocument redirecting into a Digest-protected location.
|
||||
|
||||
A request which fails header validation -- e.g. an HTTP/1.1 request with no
|
||||
Host header -- is rejected in ap_check_request_header, before
|
||||
ap_post_read_request runs. The Digest module sets up its per-request record
|
||||
in a post_read_request hook, so that record was never created for such a
|
||||
request.
|
||||
|
||||
If ErrorDocument then redirects the failure into a Digest-protected
|
||||
location, the auth hook runs for the redirect and walks back along ->prev
|
||||
to the original request to find the shared record -- which is NULL here.
|
||||
Dereferencing it crashes the child, so an unauthenticated client can take
|
||||
down a worker with a single malformed request.
|
||||
"""
|
||||
|
||||
from .env import AAATestEnv
|
||||
|
||||
|
||||
class TestDigestErrorDocument:
|
||||
|
||||
def test_digest_125_errordoc_into_digest_without_prr(self, env):
|
||||
# Send an HTTP/1.1 request with the Host header removed; the server
|
||||
# answers 400 before the Digest post_read_request hook runs, and the
|
||||
# ErrorDocument points into a Digest-protected location.
|
||||
url = env.mkurl("http", "aaa", "/anything")
|
||||
r = env.curl_get(url, options=["--http1.1", "-H", "Host:"])
|
||||
|
||||
# The request must get an HTTP response, not a reset connection from
|
||||
# a crashed child.
|
||||
assert r.exit_code == 0, \
|
||||
f"no HTTP response (curl exit {r.exit_code}): the worker likely " \
|
||||
f"crashed handling the ErrorDocument"
|
||||
assert r.response["status"] in (400, 401)
|
||||
|
||||
# The server is still serving afterwards.
|
||||
r = env.curl_get(env.mkurl("http", "aaa", "/digest/default/secret.txt"))
|
||||
assert r.response["status"] == 401
|
||||
Reference in New Issue
Block a user