You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by fi...@hyperreal.org on 1998/08/09 08:37:19 UTC

cvs commit: apache-1.3/src/main http_config.c http_protocol.c

fielding    98/08/08 23:37:19

  Modified:    src      CHANGES
               src/include http_config.h httpd.h
               src/main http_config.c http_protocol.c
  Log:
  Added default limits for various aspects of reading a
  client request to avoid some simple denial of service attacks,
  including limits on maximum request-line size, number of header
  fields, size of any one header field, and size of the request
  message body.
  
  Bumped MMN for addition of limit_req_line, limit_req_fields,
  limit_req_fieldsize and limit_req_body variables to server_rec.
  
  Revision  Changes    Path
  1.1012    +6 -0      apache-1.3/src/CHANGES
  
  Index: CHANGES
  ===================================================================
  RCS file: /home/cvs/apache-1.3/src/CHANGES,v
  retrieving revision 1.1011
  retrieving revision 1.1012
  diff -u -r1.1011 -r1.1012
  --- CHANGES	1998/08/08 13:26:04	1.1011
  +++ CHANGES	1998/08/09 06:37:12	1.1012
  @@ -1,5 +1,11 @@
   Changes with Apache 1.3.2
   
  +  *) SECURITY: Added default limits for various aspects of reading a
  +     client request to avoid some simple denial of service attacks,
  +     including limits on maximum request-line size, number of header
  +     fields, size of any one header field, and size of the request
  +     message body.  [Roy Fielding]
  +
     *) Make status module aware of DNS and logging states, even if
        STATUS not defined.  [Jim Jagielski]
   
  
  
  
  1.92      +1 -1      apache-1.3/src/include/http_config.h
  
  Index: http_config.h
  ===================================================================
  RCS file: /home/cvs/apache-1.3/src/include/http_config.h,v
  retrieving revision 1.91
  retrieving revision 1.92
  diff -u -r1.91 -r1.92
  --- http_config.h	1998/08/06 17:30:23	1.91
  +++ http_config.h	1998/08/09 06:37:15	1.92
  @@ -275,7 +275,7 @@
    * handle it back-compatibly, or at least signal an error).
    */
   
  -#define MODULE_MAGIC_NUMBER 19980806
  +#define MODULE_MAGIC_NUMBER 19980808
   #define STANDARD_MODULE_STUFF MODULE_MAGIC_NUMBER, -1, __FILE__, NULL, NULL
   
   /* Generic accessors for other modules to get at their own module-specific
  
  
  
  1.231     +29 -2     apache-1.3/src/include/httpd.h
  
  Index: httpd.h
  ===================================================================
  RCS file: /home/cvs/apache-1.3/src/include/httpd.h,v
  retrieving revision 1.230
  retrieving revision 1.231
  diff -u -r1.230 -r1.231
  --- httpd.h	1998/08/06 19:13:52	1.230
  +++ httpd.h	1998/08/09 06:37:16	1.231
  @@ -541,6 +541,28 @@
   #define REQUEST_CHUNKED_DECHUNK  2
   #define REQUEST_CHUNKED_PASS     3
   
  +/* Limits on the size of various request items.  These limits primarily
  + * exist to prevent simple denial-of-service attacks on a server based
  + * on misuse of the protocol.  The recommended values will depend on the
  + * nature of the server resources -- CGI scripts and database backends
  + * might require large values, but most servers could get by with much
  + * smaller limits than we use below.  These limits can be reset on a
  + * per-server basis using the LimitRequestLine, LimitRequestFields,
  + * LimitRequestFieldSize, and LimitRequestBody configuration directives.
  + */
  +#ifndef DEFAULT_LIMIT_REQUEST_LINE
  +#define DEFAULT_LIMIT_REQUEST_LINE 8190
  +#endif /* default limit on bytes in Request-Line (Method+URI+HTTP-version) */
  +#ifndef DEFAULT_LIMIT_REQUEST_FIELDS
  +#define DEFAULT_LIMIT_REQUEST_FIELDS 100
  +#endif /* default limit on number of header fields */
  +#ifndef DEFAULT_LIMIT_REQUEST_FIELDSIZE
  +#define DEFAULT_LIMIT_REQUEST_FIELDSIZE 8190
  +#endif /* default limit on bytes in any one field  */
  +#ifndef DEFAULT_LIMIT_REQUEST_BODY
  +#define DEFAULT_LIMIT_REQUEST_BODY 33554432ul
  +#endif /* default limit on bytes in request body   */
  +
   /* Things which may vary per file-lookup WITHIN a request ---
    * e.g., state of MIME config.  Basically, the name of an object, info
    * about the object, and any other info we may ahve which may need to
  @@ -821,9 +843,14 @@
   
       array_header *names;	/* Normal names for ServerAlias servers */
       array_header *wild_names;	/* Wildcarded names for ServerAlias servers */
  +
  +    uid_t server_uid;        /* effective user id when calling exec wrapper */
  +    gid_t server_gid;        /* effective group id when calling exec wrapper */
   
  -    uid_t server_uid;		/* effective user id when calling exec wrapper */
  -    gid_t server_gid;		/* effective group id when calling exec wrapper */
  +    unsigned int  limit_req_line;      /* limit on bytes in Request-Line   */
  +    unsigned int  limit_req_fields;    /* limit on number of header fields */
  +    unsigned long limit_req_fieldsize; /* limit on bytes in any one field  */
  +    unsigned long limit_req_body;      /* limit on bytes in request body   */
   };
   
   /* These are more like real hosts than virtual hosts */
  
  
  
  1.120     +4 -0      apache-1.3/src/main/http_config.c
  
  Index: http_config.c
  ===================================================================
  RCS file: /home/cvs/apache-1.3/src/main/http_config.c,v
  retrieving revision 1.119
  retrieving revision 1.120
  diff -u -r1.119 -r1.120
  --- http_config.c	1998/08/06 17:30:27	1.119
  +++ http_config.c	1998/08/09 06:37:17	1.120
  @@ -1409,6 +1409,10 @@
       s->loglevel = DEFAULT_LOGLEVEL;
       s->srm_confname = RESOURCE_CONFIG_FILE;
       s->access_confname = ACCESS_CONFIG_FILE;
  +    s->limit_req_line      = DEFAULT_LIMIT_REQUEST_LINE;
  +    s->limit_req_fields    = DEFAULT_LIMIT_REQUEST_FIELDS;
  +    s->limit_req_fieldsize = DEFAULT_LIMIT_REQUEST_FIELDSIZE;
  +    s->limit_req_body      = DEFAULT_LIMIT_REQUEST_BODY;
       s->timeout = DEFAULT_TIMEOUT;
       s->keep_alive_timeout = DEFAULT_KEEPALIVE_TIMEOUT;
       s->keep_alive_max = DEFAULT_KEEPALIVE;
  
  
  
  1.230     +46 -19    apache-1.3/src/main/http_protocol.c
  
  Index: http_protocol.c
  ===================================================================
  RCS file: /home/cvs/apache-1.3/src/main/http_protocol.c,v
  retrieving revision 1.229
  retrieving revision 1.230
  diff -u -r1.229 -r1.230
  --- http_protocol.c	1998/08/06 17:30:30	1.229
  +++ http_protocol.c	1998/08/09 06:37:17	1.230
  @@ -626,7 +626,7 @@
   
   static int read_request_line(request_rec *r)
   {
  -    char l[HUGE_STRING_LEN];
  +    char l[r->server->limit_req_line + 2];
       const char *ll = l, *uri;
       conn_rec *conn = r->connection;
       int major = 1, minor = 0;   /* Assume HTTP/1.0 if non-"HTTP" protocol */
  @@ -647,7 +647,7 @@
        * have to block during a read.
        */
       ap_bsetflag(conn->client, B_SAFEREAD, 1);
  -    while ((len = getline(l, HUGE_STRING_LEN, conn->client, 0)) <= 0) {
  +    while ((len = getline(l, sizeof(l), conn->client, 0)) <= 0) {
           if ((len < 0) || ap_bgetflag(conn->client, B_EOF)) {
               ap_bsetflag(conn->client, B_SAFEREAD, 0);
               return 0;
  @@ -689,7 +689,7 @@
   
       ap_parse_uri(r, uri);
   
  -    if (len == (HUGE_STRING_LEN - 1)) {
  +    if (len >= sizeof(l) - 1) {
           r->status    = HTTP_REQUEST_URI_TOO_LARGE;
           r->proto_num = HTTP_VERSION(1,0);
           r->protocol  = ap_pstrdup(r->pool, "HTTP/1.0");
  @@ -711,20 +711,38 @@
   static void get_mime_headers(request_rec *r)
   {
       conn_rec *c = r->connection;
  +    char *value, *copy;
       int len;
  -    char *value;
  -    char field[MAX_STRING_LEN];
  +    unsigned int fields_read = 0;
  +    char field[r->server->limit_req_fieldsize + 2];
   
       /*
        * Read header lines until we get the empty separator line, a read error,
  -     * the connection closes (EOF), or we timeout.
  +     * the connection closes (EOF), reach the server limit, or we timeout.
        */
  -    while ((len = getline(field, MAX_STRING_LEN, c->client, 1)) > 0) {
  -        char *copy = ap_palloc(r->pool, len + 1);
  +    while ((len = getline(field, sizeof(field), c->client, 1)) > 0) {
  +
  +        if (++fields_read > r->server->limit_req_fields) {
  +            r->status = HTTP_BAD_REQUEST;
  +            ap_table_setn(r->notes, "error-notes",
  +                "Number of request header fields exceeds server limit.<P>\n");
  +            return;
  +        }
  +        if (len >= sizeof(field) - 1) { 
  +            r->status = HTTP_BAD_REQUEST;
  +            ap_table_setn(r->notes, "error-notes", ap_pstrcat(r->pool,
  +                "Size of a request header field exceeds server limit.<P>\n"
  +                "<PRE>\n", field, "</PRE>\n", NULL));
  +            return;
  +        }
  +        copy = ap_palloc(r->pool, len + 1);
           memcpy(copy, field, len + 1);
   
           if (!(value = strchr(copy, ':'))) {     /* Find the colon separator */
               r->status = HTTP_BAD_REQUEST;       /* or abort the bad request */
  +            ap_table_setn(r->notes, "error-notes", ap_pstrcat(r->pool,
  +                "Request header field is missing colon separator.<P>\n"
  +                "<PRE>\n", copy, "</PRE>\n", NULL));
               return;
           }
   
  @@ -736,16 +754,6 @@
   	/* XXX: should strip trailing whitespace as well */
   
           ap_table_mergen(r->headers_in, copy, value);
  -
  -	/* the header was too long; at the least we should skip extra data */
  -	if (len >= MAX_STRING_LEN - 1) { 
  -	    while ((len = getline(field, MAX_STRING_LEN, c->client, 1))
  -		    >= MAX_STRING_LEN - 1) {
  -		/* soak up the extra data */
  -	    }
  -	    if (len <= 0) /* time to exit the larger loop as well */
  -		break;
  -	}
       }
   }
   
  @@ -1395,6 +1403,12 @@
                       "%s with body is not allowed for %s", r->method, r->uri);
           return HTTP_REQUEST_ENTITY_TOO_LARGE;
       }
  +    if (r->remaining > r->server->limit_req_body) {
  +        ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, r,
  +          "Request content-length of %s is larger than the configured "
  +          "server limit of %lu", lenp, r->server->limit_req_body);
  +        return HTTP_REQUEST_ENTITY_TOO_LARGE;
  +    }
   
       return OK;
   }
  @@ -1480,6 +1494,19 @@
       if (bufsiz <= 0)
           return -1;              /* Cannot read chunked with a small buffer */
   
  +    /* Check to see if we have already read too much request data.
  +     * For efficiency reasons, we only check this at the top of each
  +     * caller read pass, since the limit exists just to stop infinite
  +     * length requests and nobody cares if it goes over by one buffer.
  +     */
  +    if (r->read_length > r->server->limit_req_body) {
  +        ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, r,
  +          "Chunked request body is larger than the configured "
  +          "server limit of %lu", r->server->limit_req_body);
  +        r->connection->keepalive = -1;
  +        return -1;
  +    }
  +
       if (r->remaining == 0) {    /* Start of new chunk */
   
           chunk_start = getline(buffer, bufsiz, r->connection->client, 0);
  @@ -1593,7 +1620,7 @@
    * Since we return an error status if the request is malformed, this
    * routine should be called at the beginning of a no-body handler, e.g.,
    *
  - *    if ((retval = discard_request_body(r)) != OK)
  + *    if ((retval = ap_discard_request_body(r)) != OK)
    *        return retval;
    */
   API_EXPORT(int) ap_discard_request_body(request_rec *r)
  
  
  

Re: cvs commit: apache-1.3/src/main http_config.c http_protocol.c

Posted by Dean Gaudet <dg...@arctic.org>.
I think a limit of 0 should be considered "unlimited".  For example, I
don't understand the need to limit the request body.

Dean


Re: cvs commit: apache-1.3/src/main http_config.c http_protocol.c

Posted by Ben Laurie <be...@algroup.co.uk>.
Dean Gaudet wrote:
> 
> This is insufficient, as Dirk pointed out.  We're still considering my
> patch, correct?

I certainly am, or the more general version. In fact, I'm +1 on one or
the other, regardless of this limits stuff.

Cheers,

Ben.

-- 
Ben Laurie            |Phone: +44 (181) 735 0686| Apache Group member
Freelance Consultant  |Fax:   +44 (181) 735 0689|http://www.apache.org/
and Technical Director|Email: ben@algroup.co.uk |
A.L. Digital Ltd,     |Apache-SSL author     http://www.apache-ssl.org/
London, England.      |"Apache: TDG" http://www.ora.com/catalog/apache/

WE'RE RECRUITING! http://www.aldigital.co.uk/recruit/

Re: cvs commit: apache-1.3/src/main http_config.c http_protocol.c

Posted by Dean Gaudet <dg...@arctic.org>.
This is insufficient, as Dirk pointed out.  We're still considering my
patch, correct?

Dean

On 9 Aug 1998 fielding@hyperreal.org wrote:

>   Added default limits for various aspects of reading a
>   client request to avoid some simple denial of service attacks,
>   including limits on maximum request-line size, number of header
>   fields, size of any one header field, and size of the request
>   message body.