From: Amos Jeffries Date: Fri, 14 Jan 2011 06:15:23 +0000 (-0700) Subject: Support configurable status codes for deny_info X-Git-Tag: take00~7 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=aed9a15b8fceabd8e4347b4fcca55c7fcafe3335;p=thirdparty%2Fsquid.git Support configurable status codes for deny_info This changes the default behaviour of deny_info redirects. Making Squid automaticaly select 307 or 303 status code where appropriate for HTTP/1.1 clients and 302 for HTTP/1.0 clients or other appropriate cases. For example; deny_info 303:http://example.com/ POST On top of the behaviour change this patch adds capability for admin to configure deny_info with explicit status codes ranging from 200 to 599. There are limits placed on the use of each range of status codes: * 2xx, 4xx and 5xx may only be set when there is a local file or template being used as body content on the response. * 3xx status may only be set when there is a URI being used as a redirect destination. These limitations are enforced with a configuration hard abort due to: 3xx with a named template and 4xx/5xx with a redirect break with a range of horrible results to our file loading and output Location: URLs. My tests ended up with Squid scanning the FS for local files called http://blah, redirecting the browser to 404:ERR_ACCESS_DENIED, or getting past those with zero-sized replies and crashes when err is required to have length. They are going to take something much more major logic re-plumbing and maybe deeper cleanup to get the crossover down to safe enough for just a warning. Given the RFC defined use of each status range I did not think it worth doing to enable something on the fine edge of non-standard. --- diff --git a/src/cf.data.pre b/src/cf.data.pre index cf57e44fe4..ef8fafb3a2 100644 --- a/src/cf.data.pre +++ b/src/cf.data.pre @@ -5957,12 +5957,18 @@ DOC_START you may also specify them by your custom file name: Example: deny_info ERR_CUSTOM_ACCESS_DENIED bad_guys + By defaut Squid will send "403 Forbidden". A different 4xx or 5xx + may be specified by prefixing the file name with the code and a colon. + e.g. 404:ERR_CUSTOM_ACCESS_DENIED + Alternatively you can tell Squid to reset the TCP connection by specifying TCP_RESET. Or you can specify an error URL or URL pattern. The browsers will - get redirected (302) to the specified URL after formatting tags have - been replaced. + get redirected to the specified URL after formatting tags have + been replaced. Redirect will be done with 302 or 307 according to + HTTP/1.1 specs. A different 3xx code may be specified by prefixing + the URL. e.g. 303:http://example.com/ URL FORMAT TAGS: %a - username (if available. Password NOT included) diff --git a/src/errorpage.cc b/src/errorpage.cc index 4d871059f0..be36b40940 100644 --- a/src/errorpage.cc +++ b/src/errorpage.cc @@ -76,6 +76,7 @@ CBDATA_CLASS_INIT(ErrorState); typedef struct { int id; char *page_name; + http_status page_redirect; } ErrorDynamicPageInfo; /* local constant and vars */ @@ -179,9 +180,13 @@ errorInitialize(void) ErrorDynamicPageInfo *info = ErrorDynamicPages.items[i - ERR_MAX]; assert(info && info->id == i && info->page_name); - if (strchr(info->page_name, ':') == NULL) { + const char *pg = info->page_name; + if (info->page_redirect != HTTP_STATUS_NONE) + pg = info->page_name +4; + + if (strchr(pg, ':') == NULL) { /** But only if they are not redirection URL. */ - error_text[i] = errorLoadText(info->page_name); + error_text[i] = errorLoadText(pg); } } } @@ -325,6 +330,40 @@ errorDynamicPageInfoCreate(int id, const char *page_name) ErrorDynamicPageInfo *info = new ErrorDynamicPageInfo; info->id = id; info->page_name = xstrdup(page_name); + info->page_redirect = static_cast(atoi(page_name)); + + /* WARNING on redirection status: + * 2xx are permitted, but not documented officially. + * - might be useful for serving static files (PAC etc) in special cases + * 3xx require a URL suitable for Location: header. + * - the current design does not allow for a Location: URI as well as a local file template + * although this possibility is explicitly permitted in the specs. + * 4xx-5xx require a local file template. + * - sending Location: on these codes with no body is invalid by the specs. + * - current result is Squid crashing or XSS problems as dynamic deny_info load random disk files. + * - a future redesign of the file loading may result in loading remote objects sent inline as local body. + */ + if (info->page_redirect == HTTP_STATUS_NONE) + ; // special case okay. + else if (info->page_redirect < 200 || info->page_redirect > 599) { + // out of range + debugs(0, DBG_CRITICAL, "FATAL: status " << info->page_redirect << " is not valid on '" << page_name << "'"); + self_destruct(); + } else if ( /* >= 200 && */ info->page_redirect < 300 && strchr(&(page_name[4]), ':')) { + // 2xx require a local template file + debugs(0, DBG_CRITICAL, "FATAL: status " << info->page_redirect << " is not valid on '" << page_name << "'"); + self_destruct(); + } else if (/* >= 300 && */ info->page_redirect <= 399 && !strchr(&(page_name[4]), ':')) { + // 3xx require an absolute URL + debugs(0, DBG_CRITICAL, "FATAL: status " << info->page_redirect << " is not valid on '" << page_name << "'"); + self_destruct(); + } else if (info->page_redirect >= 400 /* && <= 599 */ && strchr(&(page_name[4]), ':')) { + // 4xx/5xx require a local template file + debugs(0, DBG_CRITICAL, "FATAL: status " << info->page_redirect << " is not valid on '" << page_name << "'"); + self_destruct(); + } + // else okay. + return info; } @@ -390,6 +429,8 @@ errorCon(err_type type, http_status status, HttpRequest * request) err->err_language = NULL; err->type = type; err->httpStatus = status; + if (err->page_id >= ERR_MAX && ErrorDynamicPages.items[err->page_id - ERR_MAX]->page_redirect != HTTP_STATUS_NONE) + err->httpStatus = ErrorDynamicPages.items[err->page_id - ERR_MAX]->page_redirect; if (request != NULL) { err->request = HTTPMSGLOCK(request); @@ -915,6 +956,9 @@ ErrorState::DenyInfoLocation(const char *name, HttpRequest *aRequest, MemBuf &re char const *p = m; char const *t; + if (m[0] == '3') + m += 4; // skip "3xx:" + while ((p = strchr(m, '%'))) { result.append(m, p - m); /* copy */ t = Convert(*++p, true, true); /* convert */ @@ -935,9 +979,19 @@ ErrorState::BuildHttpReply() const char *name = errorPageName(page_id); /* no LMT for error pages; error pages expire immediately */ - if (strchr(name, ':')) { + if (name[0] == '3' || (name[0] != '4' && name[0] != '5' && strchr(name, ':'))) { /* Redirection */ - rep->setHeaders(HTTP_MOVED_TEMPORARILY, NULL, "text/html", 0, 0, -1); + http_status status = HTTP_MOVED_TEMPORARILY; + // Use configured 3xx reply status if set. + if (name[0] == '3') + status = httpStatus; + else { + // Use 307 for HTTP/1.1 non-GET/HEAD requests. + if (request->method != METHOD_GET && request->method != METHOD_HEAD && request->http_ver >= HttpVersion(1,1)) + status = HTTP_TEMPORARY_REDIRECT; + } + + rep->setHeaders(status, NULL, "text/html", 0, 0, -1); if (request) { MemBuf redirect_location;