Bug#607418: nginx: $host variable mis-parses IPv6 literal addresses from HTTP request Host header

2010-12-18 Thread Kartik Mistry
On Sat, Dec 18, 2010 at 1:22 PM, Steven Chamberlain ste...@pyro.eu.org wrote:
 I'm attaching a patch, tested, that is the simplest solution I can think
 of.  Colons found between square brackets are now preserved.

 If a closing square bracket is not found, such as in '[::', this now
 returns 400 Bad Request, in line with the behaviour observed in Apache.

 At this time, the IPv6 address is not sanitised in any way, I just did
 the minimal change possible to fix the bug I've been seeing.

Many thanks from all nginx maintainers! We'll add patch with next upload soon.

-- 
Kartik Mistry
Debian GNU/Linux Developer
IRC: kart_ | Identica: @kartikm



--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#607418: nginx: $host variable mis-parses IPv6 literal addresses from HTTP request Host header

2010-12-17 Thread Steven Chamberlain
Package: nginx
Version: 0.7.67-3
Tags: ipv6

Hello,

I've noticed that nginx incorrectly parses IPv6 addresses from the HTTP
requests Host header into its '$host' configuration variable.  An HTTP
request (via IPv4 or IPv5) in the following form:

GET / HTTP/1.0
Host: [::1]

results in a value of only '[::' in $host (excluding my quotes).

However, the following request:

GET / HTTP/1.0
Host: [::1]:80

results in the correct value of '[::1]' in $host.

For 'longer' IPv6 addresses the result is that a full two octets can be
missing from the $host variable, e.g.:

GET / HTTP/1.0
Host: [fdf2:9468:665c:eaf6:2af6:c9ca:f24e:ae62]

results in only '[fdf2:9468:665c:eaf6:2af6:c9ca:f24e:' in $host.

After some experimentation it seems that the last colon and all
characters following it are omitted.  This may be the way the 'port'
portion is normally stripped from IPv4 literal addresses.

This leads to a number of potential issues:

1. Proxying

Consider the following configuration, which may be used to proxy traffic
to an upstream server that serves multiple, name-based virtual hosts:

 location / {
 proxy_pass  http://localhost:8000;
 proxy_set_headerHost $host;
 }

Due to the incorrect parsing of the Host header in the request, a
malformed Host header is sent in the HTTP request to the upstream
server, resulting in '400 Bad Request' from Apache at least.

If a website had an nginx reverse proxy server configured in this way, a
visitor to http://[fc00::1]/ would likely receive this error.

2. Logs

Consider the following log format which is the same as the default
format, but prepended with '$host '.  This may be used in virtual
hosting environments or by awstats to distinguish traffic for different
customers or websites:

 log_format  vhost '$host $remote_addr - $remote_user [$time_local]  '
   '$request $status $body_bytes_sent '
   '$http_referer $http_user_agent';
 access_log  /var/log/nginx/localhost.access.log vhost;

The resulting log entry is ambiguous, and may confused parsers:

 [: ::1 - - [18/Dec/2010:06:29:42 +]  GET / HTTP/1.0 404 169 - -

3. Rewrites, access control, other uses of the $host variable

Obviously, any configuration that matches on the contents of the $host
variable can fail to work as expected due to this bug.

The value of $host may sometimes be used as a 'key' for caching (such as
the documented example for proxy_cache_key).  In this case, it is
possible that two different sites (eg. on [fc00::1234] and [fc00::5678])
could be given the same key due to the incorrect parsing.  In the most
extreme case this may cause one website's content to appear on the other
(if the upstream server and path part of the URI were the same).


As well as fixing the parsing of the IPv6 literal address, I suggest
nginx also sanitises it (e.g. removing unnecessary zeroes), otherwise a
malicious user could try to influence the behaviour of proxy_cache by
visiting [fc00::1], [fc00::01], [fc00::000:1], etc.

Regards,
-- 
Steven Chamberlain
ste...@pyro.eu.org



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#607418: nginx: $host variable mis-parses IPv6 literal addresses from HTTP request Host header

2010-12-17 Thread Steven Chamberlain
Tags: +patch

Hi,

I'm attaching a patch, tested, that is the simplest solution I can think
of.  Colons found between square brackets are now preserved.

If a closing square bracket is not found, such as in '[::', this now
returns 400 Bad Request, in line with the behaviour observed in Apache.

At this time, the IPv6 address is not sanitised in any way, I just did
the minimal change possible to fix the bug I've been seeing.

Regards,
-- 
Steven Chamberlain
ste...@pyro.eu.org
diff -Nru a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c
--- a/src/http/ngx_http_request.c	2010-06-07 11:14:11.0 +0100
+++ b/src/http/ngx_http_request.c	2010-12-18 07:42:29.0 +
@@ -1645,11 +1645,12 @@
 {
 u_char  *h, ch;
 size_t   i, last;
-ngx_uint_t   dot;
+ngx_uint_t   dot, in_brackets;
 
 last = len;
 h = *host;
 dot = 0;
+in_brackets = 0;
 
 for (i = 0; i  len; i++) {
 ch = h[i];
@@ -1665,11 +1666,27 @@
 
 dot = 0;
 
-if (ch == ':') {
+if (ch == '['  i == 0) {
+/* start of literal IPv6 address */
+in_brackets = 1;
+continue;
+}
+
+/*
+ * Inside square brackets, the colon is a delimeter for an IPv6 address.
+ * Otherwise it comes before the port number, so remove it.
+ */
+if (ch == ':'  !in_brackets) {
 last = i;
 continue;
 }
 
+if (ch == ']') {
+/* end of literal IPv6 address */
+in_brackets = 0;
+continue;
+}
+
 if (ngx_path_separator(ch) || ch == '\0') {
 return 0;
 }
@@ -1679,6 +1696,11 @@
 }
 }
 
+if (in_brackets) {
+/* missing the closing square bracket for IPv6 address */
+return 0;
+}
+
 if (dot) {
 last--;
 }