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 cf57e44fe4d747cad573679b447eec227f4452c5..ef8fafb3a2bcc00cbfbdfc034a58f831d5678b12 100644 (file)
--- 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 4d871059f07701ee9c99a8ace140bc80cbd73bf2..be36b40940de2e1f0817d8f89f62dff9ed0e96e8 100644 (file)
--- 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<http_status>(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;