From 19484fa1421042d9bfb7eb3c4eece080054c67f9 Mon Sep 17 00:00:00 2001 From: Rich Megginson Date: Tue, 22 Oct 2013 13:49:01 -0600 Subject: [PATCH] Ticket #47478 No groups file? error restarting Admin server https://fedorahosted.org/389/ticket/47478 Reviewed by: ??? Branch: master Fix Description: When Apache runs the CGI, it has to run through all of the hooks/callbacks again, including the authz callbacks, via a sub-request. For the sub-request, admserv_check_authz was just returning DECLINED, which means "I decline to answer this request for authz, go to the next handler". It was then doing regular Apache authz handling, including looking at the groups file. The fix is to remember what the original authz return value was, and just return that if being called as part of a sub-request. Platforms tested: RHEL6 x86_64 Flag Day: no Doc impact: no --- mod_admserv/mod_admserv.c | 78 ++++++++++++++++++++++++++++++-------------- 1 files changed, 53 insertions(+), 25 deletions(-) diff --git a/mod_admserv/mod_admserv.c b/mod_admserv/mod_admserv.c index d104538..420e36b 100644 --- a/mod_admserv/mod_admserv.c +++ b/mod_admserv/mod_admserv.c @@ -1589,6 +1589,7 @@ populate_task_cache_entries(const char *userDN, LDAP *server) #define IS_START_CONFIG_DS_REQ(uri, serverid, userdn, user) \ uri && !STRNCASECMP(uri, STARTDS_IDENTIFIER, strlen(STARTDS_IDENTIFIER)) && \ serverid && !STRCMP(serverid, "admin-serv") && /* is local superuser */ !userdn && user +#define ADMSERV_AUTHZ_RETVAL "admserv_authz_retval" static int admserv_check_authz(request_rec *r) @@ -1608,19 +1609,25 @@ admserv_check_authz(request_rec *r) long now; int is_start_config_ds_req = 0; int tries = 0; + int retval = DECLINED; + char rvbuf[20]; + const char *rvstr; admserv_config *cf = ap_get_module_config(r->per_dir_config, &admserv_module); ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, "admserv_check_authz: request for uri [%s]", r->uri); - /* for some reason, we get sub requests for our tasks which we can ignore */ - if (r->main) { + /* sub requests need to reparse the uri, but do not need to go through the whole authz again + * just return the result from the original authz request + */ + if (r->main && (rvstr = apr_table_get(r->main->notes, ADMSERV_AUTHZ_RETVAL))) { ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, - "admserv_check_authz: skipping sub request [%s] - filename [%s] user [%s]", + "admserv_check_authz: sub request [%s] - filename [%s] user [%s] retval [%s]", r->uri, r->main->filename ? r->main->filename : "(null)", - r->main->user ? r->main->user : "(null)"); - return DECLINED; + r->main->user ? r->main->user : "(null)", rvstr); + retval = atoi(rvstr); + goto done; } uri = apr_pstrdup(r->pool, r->uri); /* might need unparsed_uri here? */ @@ -1628,7 +1635,8 @@ admserv_check_authz(request_rec *r) if (!(p = strchr(uri+1, '/'))) { ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, "admserv_check_authz(): Skipping invalid URI [%s]", uri); - return DECLINED; + retval = DECLINED; + goto done; } serverid = uri+1; @@ -1655,7 +1663,8 @@ admserv_check_authz(request_rec *r) ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "admserv_check_authz(): unable to find registered server (%s)", serverid); - return admserv_error(r, HTTP_BAD_REQUEST, "server not registered"); /*i18n*/ + retval = admserv_error(r, HTTP_BAD_REQUEST, "server not registered"); /*i18n*/ + goto done; } } } @@ -1665,7 +1674,8 @@ admserv_check_authz(request_rec *r) ap_log_rerror(APLOG_MARK, APLOG_NOTICE, 0, r, "admserv_check_authz(): passing [%s] to the userauth handler", r->uri); - return OK; + retval = OK; + goto done; } entryDN[0] = '\0'; @@ -1673,7 +1683,8 @@ admserv_check_authz(request_rec *r) ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "admserv_check_authz(): unable to build DN from URL - bad URL [%s]", uri); - return admserv_error_std(r, "server not registered"); /*i18n*/ + retval = admserv_error_std(r, "server not registered"); /*i18n*/ + goto done; } convert_to_lower_case(entryDN); @@ -1693,12 +1704,14 @@ admserv_check_authz(request_rec *r) "admserv_check_authz: task [%s] not cached for local superuser", entryDN); if (configdsdown) { - return OK; /* let the admserv_config_ds_down handler handle this situation */ + retval = OK; /* let the admserv_config_ds_down handler handle this situation */ + goto done; } if (!send_response) { - return admserv_error_std(r, retmsg); + retval = admserv_error_std(r, retmsg); + goto done; } - return retval; + goto done; } goto found; } @@ -1706,7 +1719,8 @@ admserv_check_authz(request_rec *r) if (!userdn || !pw) { ap_log_rerror(APLOG_MARK, APLOG_CRIT, 0, r, "admserv_check_authz(): invalid userdn/pw"); - return admserv_error_std(r, "invalid user/password"); /*i18n*/ + retval = admserv_error_std(r, "invalid user/password"); /*i18n*/ + goto done; } /* DT 3/10/98 @@ -1725,7 +1739,8 @@ admserv_check_authz(request_rec *r) ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "admserv_check_authz(): unable to open LDAP connection to %s:%d", registryServer.host, registryServer.port); - return admserv_error_std(r, "unable to open LDAPConnection"); /*i18n*/ + retval = admserv_error_std(r, "unable to open LDAPConnection"); /*i18n*/ + goto done; } tries = 0; @@ -1743,7 +1758,8 @@ admserv_check_authz(request_rec *r) ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "admserv_check_authz(): unable to LDAP BIND as [%s] to %s:%d", userdn ? userdn : "(anon)", registryServer.host, registryServer.port); - return admserv_error_std(r, "unable to open LDAPConnection"); /*i18n*/ + retval = admserv_error_std(r, "unable to open LDAPConnection"); /*i18n*/ + goto done; } } while (server != NULL && ++tries < 2); @@ -1756,7 +1772,7 @@ admserv_check_authz(request_rec *r) ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, "admserv_check_authz: could not find cached task [%s] for [%s]", entryDN, userdn ? userdn : "(anon)"); - return retval; + goto done; } goto found; } @@ -1764,7 +1780,8 @@ admserv_check_authz(request_rec *r) ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "admserv_check_authz(): got LDAP error %d talking to %s:%d - possible user [%s] not authorized", ldapError, registryServer.host, registryServer.port, userdn ? userdn : "(anon)"); - return admserv_error(r, HTTP_UNAUTHORIZED, "invalid user credentials"); /*i18n*/ + retval = admserv_error(r, HTTP_UNAUTHORIZED, "invalid user credentials"); /*i18n*/ + goto done; } /* Attempt to read the entry data -- involves DS eval of entry ACI for userDN. @@ -1781,7 +1798,8 @@ admserv_check_authz(request_rec *r) "admserv_check_authz(): Task [%s] not found for user [%s] - either" " the task was not registered or the user was not authorized", entryDN, userdn ? userdn : "(anon)"); - return admserv_error(r, HTTP_UNAUTHORIZED, "task not found or unauthorized"); /*i18n*/ + retval = admserv_error(r, HTTP_UNAUTHORIZED, "task not found or unauthorized"); /*i18n*/ + goto done; /* This is remapping - converting the requested URI to the actual CGI filename We can't do this in a translate_name hook because that happens _before_ authenticating @@ -1800,9 +1818,10 @@ found: ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "admserv_check_authz(): No ADMCgiBinDir was specified for location [%s]", r->filename); - return admserv_error_std(r, apr_psprintf(r->pool, + retval = admserv_error_std(r, apr_psprintf(r->pool, "No ADMCgiBinDir specified for location [%s]", r->filename)); + goto done; } r->filename = apr_psprintf(r->pool, "%s%c%s", @@ -1819,9 +1838,10 @@ found: ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "admserv_check_authz(): start config ds CGI is a directory [%s]", r->filename); - return admserv_error_std(r, apr_psprintf(r->pool, + retval = admserv_error_std(r, apr_psprintf(r->pool, "Invalid URL [%s] is a directory", r->filename)); + goto done; } } @@ -1829,7 +1849,8 @@ found: ap_log_rerror(APLOG_MARK, APLOG_NOTICE, 0, r, "admserv_check_authz(): StartConfigDS requested by the local superuser"); - return OK; + retval = OK; + goto done; } /* DT 3/15/98 Runtime command support. */ @@ -1842,7 +1863,8 @@ found: ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, "admserv_check_authz: mapped uri [%s] to command [%s]", r->uri, name); - return OK; + retval = OK; + goto done; } else { ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, "admserv_check_authz: uri [%s] did not begin with [%s] - not a command", @@ -1859,9 +1881,10 @@ found: ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "admserv_check_authz(): No ADMCgiBinDir was specified for location [%s]", r->filename); - return admserv_error_std(r, apr_psprintf(r->pool, + retval = admserv_error_std(r, apr_psprintf(r->pool, "No ADMCgiBinDir specified for location [%s]", r->filename)); + goto done; } /* set cgibindir in the Directory/Location section for this server */ @@ -1880,9 +1903,10 @@ found: ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "admserv_check_authz(): CGI is a directory [%s]", r->filename); - return admserv_error_std(r, apr_psprintf(r->pool, + retval = admserv_error_std(r, apr_psprintf(r->pool, "Invalid URL [%s] is a directory", r->filename)); + goto done; } } @@ -1901,7 +1925,11 @@ found: "admserv_check_authz: execute CGI [%s] args [%s]", r->filename, r->args); - return OK; /* FIXME, does this matter? */ + retval = OK; +done: + snprintf(rvbuf, sizeof(rvbuf), "%d", retval); + apr_table_set(r->notes, ADMSERV_AUTHZ_RETVAL, rvbuf); /* remember authz result for sub-requests */ + return retval; } static int -- 1.7.1