Add ProtocolReqCheck directive, which determines if Apache will

check for a valid protocol string in the request (eg: HTTP/1.1)
and return HTTP_BAD_REQUEST if not valid. Versions of Apache
prior to 1.3.26 would silently ignore bad protocol strings, but
1.3.26 included a more strict check. This makes it runtime
configurable. The default is On. This also removes the requirement
on an ANSI sscanf() implementation.

PR:
Obtained from:
Submitted by:
Reviewed by:

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/1.3.x@95978 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/src/CHANGES b/src/CHANGES
index 649668e7957d7ed42987f228e2ec1772fb75c7f5..36e78d692996175559fbff93e5b957f9c220025c 100644 (file)
--- a/src/CHANGES
+++ b/src/CHANGES
@@ -1,8 +1,16 @@
 Changes with Apache 1.3.27
 
+  *) Add ProtocolReqCheck directive, which determines if Apache will
+     check for a valid protocol string in the request (eg: HTTP/1.1)
+     and return HTTP_BAD_REQUEST if not valid. Versions of Apache
+     prior to 1.3.26 would silently ignore bad protocol strings, but
+     1.3.26 included a more strict check. This makes it runtime
+     configurable. The default is On. This also removes the requirement
+     on an ANSI sscanf() implementation. [Jim Jagielski]
+
   *) NetWare: implemented file locking in mod_rewrite for the NetWare
      CLib platform. This fixes a bug that prevented rewrite logging 
-        from working. [Brad Nicholes]
+     from working. [Brad Nicholes]
 
 Changes with Apache 1.3.26
 

diff --git a/src/include/http_conf_globals.h b/src/include/http_conf_globals.h
index 505567b5d473d0050694744cef6339b5b67ee22c..a33fddc877622bf0af843839f1a7a2406c23c4f5 100644 (file)
--- a/src/include/http_conf_globals.h
+++ b/src/include/http_conf_globals.h
@@ -103,6 +103,8 @@ extern API_VAR_EXPORT char *ap_server_argv0;
 
 extern enum server_token_type ap_server_tokens;
 
+extern int ap_protocol_req_check;
+
 /* Trying to allocate these in the config pool gets us into some *nasty*
  * chicken-and-egg problems in http_main.c --- where do you stick them
  * when pconf gets cleared?  Better to just allocate a little space

diff --git a/src/main/http_core.c b/src/main/http_core.c
index f8090a581cfa3f37c3f433ad1db61d5c8e03877e..b557586eaaa5fe89148c507a732e0b873dd317ff 100644 (file)
--- a/src/main/http_core.c
+++ b/src/main/http_core.c
@@ -2778,6 +2778,18 @@ static const char *set_bs2000_account(cmd_parms *cmd, void *dummy, char *name)
 }
 #endif /*_OSD_POSIX*/
 
+static const char *set_protocol_req_check(cmd_parms *cmd,
+                                              core_dir_config *d, int arg) 
+{
+    const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
+    if (err != NULL) {
+        return err;
+    }
+
+    ap_protocol_req_check = arg != 0;
+    return NULL;
+}
+
 /*
  * Handle a request to include the server's OS platform in the Server
  * response header field (the ServerTokens directive).  Unfortunately
@@ -2785,7 +2797,6 @@ static const char *set_bs2000_account(cmd_parms *cmd, void *dummy, char *name)
  * http_main so it can insert the information in the right place in the
  * string.
  */
-
 static const char *set_serv_tokens(cmd_parms *cmd, void *dummy, char *arg) 
 {
     const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
@@ -3411,6 +3422,8 @@ static const command_rec core_cmds[] = {
   (void*)XtOffsetOf(core_dir_config, limit_req_body),
   OR_ALL, TAKE1,
   "Limit (in bytes) on maximum size of request message body" },
+{ "ProtocolReqCheck", set_protocol_req_check, NULL, RSRC_CONF, FLAG,
+  "Enable strict checking of Protocol type in requests" },
 { "AcceptMutex", set_accept_mutex, NULL, RSRC_CONF, TAKE1,
   "Serialized Accept Mutex; the methods " 
 #ifdef HAVE_USLOCK_SERIALIZED_ACCEPT

diff --git a/src/main/http_main.c b/src/main/http_main.c
index 8559101098599976d0b131ffda3178873fa5b11d..2fa761fa9f865b163c946d2f6e3546d1cf0c7d6f 100644 (file)
--- a/src/main/http_main.c
+++ b/src/main/http_main.c
@@ -398,6 +398,9 @@ static int version_locked = 0;
 /* Global, alas, so http_core can talk to us */
 enum server_token_type ap_server_tokens = SrvTk_FULL;
 
+/* Also global, for http_core and http_protocol */
+int ap_protocol_req_check = 1;
+
 /*
  * This routine is called when the pconf pool is vacuumed.  It resets the
  * server version string to a known value and [re]enables modifications

diff --git a/src/main/http_protocol.c b/src/main/http_protocol.c
index 403d9b17a55055ce1e6bc196c3092c2825857b78..e497bbbaa941fb026a6501ed198535d31ca1fce9 100644 (file)
--- a/src/main/http_protocol.c
+++ b/src/main/http_protocol.c
@@ -991,7 +991,9 @@ static int read_request_line(request_rec *r)
     const char *uri;
     conn_rec *conn = r->connection;
     unsigned int major = 1, minor = 0;   /* Assume HTTP/1.0 if non-"HTTP" protocol */
-    int len, n = 0;
+    int len = 0;
+    int valid_protocol = 1;
+    char *kruft;
 
     /* Read past empty lines until we get a real request line,
      * a read error, the connection closes (EOF), or we timeout.
@@ -1053,26 +1055,41 @@ static int read_request_line(request_rec *r)
     r->assbackwards = (ll[0] == '\0');
     r->protocol = ap_pstrdup(r->pool, ll[0] ? ll : "HTTP/0.9");
 
-    if (2 == sscanf(r->protocol, "HTTP/%u.%u%n", &major, &minor, &n)
-      && minor < HTTP_VERSION(1,0))    /* don't allow HTTP/0.1000 */
-       r->proto_num = HTTP_VERSION(major, minor);
+    /* Avoid sscanf in the common case */
+    if (strlen(r->protocol) == 8
+        && r->protocol[0] == 'H' && r->protocol[1] == 'T'
+       && r->protocol[2] == 'T' && r->protocol[3] == 'P'
+        && r->protocol[4] == '/' && ap_isdigit(r->protocol[5])
+       && r->protocol[6] == '.' && ap_isdigit(r->protocol[7])) {
+        r->proto_num = HTTP_VERSION(r->protocol[5] - '0', r->protocol[7] - '0');
+    }
     else {
-       r->proto_num = HTTP_VERSION(1,0);
-       n = 0;
+       kruft = ap_palloc(r->pool, strlen(r->protocol)+1);
+       if (2 == sscanf(r->protocol, "HTTP/%u.%u%s", &major, &minor, kruft)
+           && minor < HTTP_VERSION(1,0)) /* don't allow HTTP/0.1000 */
+           r->proto_num = HTTP_VERSION(major, minor);
+       else {
+           r->proto_num = HTTP_VERSION(1,0);
+           valid_protocol = 0;
+       }
     }
 
     /* Check for a valid protocol, and disallow everything but whitespace
-     * after the protocol string */
-    while (ap_isspace(r->protocol[n]))
-        ++n;
-    if (r->protocol[n] != '\0') {
-        r->status    = HTTP_BAD_REQUEST;
-        r->proto_num = HTTP_VERSION(1,0);
-        r->protocol  = ap_pstrdup(r->pool, "HTTP/1.0");
-        ap_table_setn(r->notes, "error-notes",
-                      "The request line contained invalid characters "
-                      "following the protocol string.<P>\n");
-        return 0;
+     * after the protocol string. A protocol string of nothing but
+     * whitespace is considered valid */
+    if (ap_protocol_req_check && !valid_protocol) {
+        int n = 0;
+       while (ap_isspace(r->protocol[n]))
+           ++n;
+       if (r->protocol[n] != '\0') {
+           r->status    = HTTP_BAD_REQUEST;
+           r->proto_num = HTTP_VERSION(1,0);
+           r->protocol  = ap_pstrdup(r->pool, "HTTP/1.0");
+           ap_table_setn(r->notes, "error-notes",
+                     "The request line contained invalid characters "
+                     "following the protocol string.<P>\n");
+           return 0;
+       }
     }
 
     return 1;