On Thu, Jul 19, 2012 at 11:27:04AM -0400, Jeff Trawick wrote:
> What changes are needed to httpd trunk so that you can build mpm-itk
> with apxs and enable it via LoadModule, such that mpm-itk is fully
> functional? As I'm sure you're aware, prefork, worker, and event are
> all untied from core enough to support that in httpd >= 2.4.
We'd need:
1. A hook right after merging the perdir config.
2. Fixes to get Apache to drop the connection if it detects
(during .htaccess lookup) that it would need to change the uid.
Both patches are simple, although for #2 to be truly generic (ie. be usable
by mod_privileges as well) we'd need some sort of signalling mechanism saying
“we have switched uids and cannot switch back”, which then both
mod_privileges (in secure mode) and mpm-itk could set.
I've attached the current versions of both patches from my current Apache 2.4
patch set; you can see the “ap_running_under_mpm_itk” variable which would
probably need to be replaced by ap_mpm_query() or similar.
/* Steinar */
--
Homepage: http://www.sesse.net/
Add an extra hook right after merging per-directory configuration.
This makes sure we are able to setuid() as early as possible (that
is, as soon as know what uid/gid to use for this location), so we
won't run all sorts of subrequests and other stuff as root.
Index: httpd-2.4.1/server/request.c
===================================================================
--- httpd-2.4.1.orig/server/request.c
+++ httpd-2.4.1/server/request.c
@@ -69,6 +69,7 @@ APR_HOOK_STRUCT(
APR_HOOK_LINK(auth_checker)
APR_HOOK_LINK(insert_filter)
APR_HOOK_LINK(create_request)
+ APR_HOOK_LINK(post_perdir_config)
)
AP_IMPLEMENT_HOOK_RUN_FIRST(int,translate_name,
@@ -91,6 +92,21 @@ AP_IMPLEMENT_HOOK_VOID(insert_filter, (r
AP_IMPLEMENT_HOOK_RUN_ALL(int, create_request,
(request_rec *r), (r), OK, DECLINED)
+/**
+ * This hook allows modules to affect the request immediately after the
+ * per-directory configuration for the request has been generated. This allows
+ * modules to make decisions based upon the current directory configuration
+ *
+ * This hook is private to mpm-itk, so it is not exposed in http_request.h.
+ *
+ * @param r The current request
+ * @return OK or DECLINED
+ */
+AP_DECLARE_HOOK(int,post_perdir_config,(request_rec *r))
+
+AP_IMPLEMENT_HOOK_RUN_ALL(int,post_perdir_config,
+ (request_rec *r), (r), OK, DECLINED)
+
static int auth_internal_per_conf = 0;
static int auth_internal_per_conf_hooks = 0;
static int auth_internal_per_conf_providers = 0;
@@ -191,6 +207,13 @@ AP_DECLARE(int) ap_process_request_inter
r->log = d->log;
}
+ /* First chance to handle the request after per-directory configuration is
+ * generated
+ */
+ if ((access_status = ap_run_post_perdir_config(r))) {
+ return access_status;
+ }
+
/* Only on the main request! */
if (r->main == NULL) {
if ((access_status = ap_run_header_parser(r))) {
Fix an issue where users can sometimes get spurious 403s on persistent
connections (the description in the comments explains the logic).
This would particularly hit people with reverse proxies, since these
have a higher tendency of accessing things from different vhosts in
the same connection.
Index: httpd-2.4.2/server/config.c
===================================================================
--- httpd-2.4.2.orig/server/config.c
+++ httpd-2.4.2/server/config.c
@@ -69,6 +69,8 @@ AP_DECLARE_DATA apr_array_header_t *ap_s
AP_DECLARE_DATA ap_directive_t *ap_conftree = NULL;
+AP_DECLARE_DATA int ap_running_under_mpm_itk = 0;
+
APR_HOOK_STRUCT(
APR_HOOK_LINK(header_parser)
APR_HOOK_LINK(pre_config)
@@ -2129,6 +2131,32 @@ AP_CORE_DECLARE(int) ap_parse_htaccess(a
else {
if (!APR_STATUS_IS_ENOENT(status)
&& !APR_STATUS_IS_ENOTDIR(status)) {
+ /*
+ * If we are in a persistent connection, we might end up in a state
+ * where we can no longer read .htaccess files because we have already
+ * setuid(). This can either be because the previous request was for
+ * another vhost (basically the same problem as when setuid() fails in
+ * itk.c), or it can be because a .htaccess file is readable only by
+ * root.
+ *
+ * In any case, we don't want to give out a 403, since the request has
+ * a very real chance of succeeding on a fresh connection (where
+ * presumably uid=0). Thus, we give up serving the request on this
+ * TCP connection, and do a hard close of the socket. As long as we're
+ * in a persistent connection (and there _should_ not be a way this
+ * would happen on the first request in a connection, save for subrequests,
+ * which we special-case), this is allowed, as it is what happens on
+ * a timeout. The browser will simply open a new connection and try
+ * again (there's of course a performance hit, though, both due to
+ * the new connection setup and the fork() of a new server child).
+ */
+ if (ap_running_under_mpm_itk && r->main == NULL && getuid() != 0) {
+ ap_log_rerror(APLOG_MARK, APLOG_WARNING, status, r,
+ "Couldn't read %s, closing connection.",
+ filename);
+ ap_lingering_close(r->connection);
+ exit(0);
+ }
ap_log_rerror(APLOG_MARK, APLOG_CRIT, status, r, APLOGNO(00529)
"%s pcfg_openfile: unable to check htaccess file, "
"ensure it is readable and that '%s' "
Index: httpd-2.4.2/server/mpm/itk/itk.c
===================================================================
--- httpd-2.4.2.orig/server/mpm/itk/itk.c
+++ httpd-2.4.2/server/mpm/itk/itk.c
@@ -181,6 +181,7 @@ typedef struct
} itk_server_conf;
module AP_MODULE_DECLARE_DATA mpm_itk_module;
+extern AP_DECLARE_DATA int ap_running_under_mpm_itk;
#ifdef GPROF
/*
@@ -543,6 +544,7 @@ static void child_main(int child_num_arg
* child initializes
*/
+ ap_running_under_mpm_itk = 1;
my_child_num = child_num_arg;
ap_my_pid = getpid();
requests_this_child = 0;
Index: httpd-2.4.2/server/request.c
===================================================================
--- httpd-2.4.2.orig/server/request.c
+++ httpd-2.4.2/server/request.c
@@ -111,6 +111,7 @@ static int auth_internal_per_conf = 0;
static int auth_internal_per_conf_hooks = 0;
static int auth_internal_per_conf_providers = 0;
+extern AP_DECLARE_DATA int ap_running_under_mpm_itk;
static int decl_die(int status, const char *phase, request_rec *r)
{
@@ -1116,6 +1117,13 @@ AP_DECLARE(int) ap_directory_walk(reques
break;
}
else if (APR_STATUS_IS_EACCES(rv)) {
+ if (ap_running_under_mpm_itk && r->main == NULL && getuid() != 0) {
+ ap_log_rerror(APLOG_MARK, APLOG_WARNING, rv, r,
+ "Access to %s denied, closing connection.",
+ r->filename);
+ ap_lingering_close(r->connection);
+ exit(0);
+ }
ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(00035)
"access to %s denied (filesystem path '%s') "
"because search permissions are missing on a "
