X-Forwarded-Proto / X_FORWARDED_PROTO

X-Forwarded-Proto / X_FORWARDED_PROTO

[skaar]

Eric,

think I came across an issue with the parser in unicorn, with a request
(due to 2 layers of nginx proxying) coming across with both a
X_FORWARDED_PROTO and a X-Forwarded-Proto header. From the socket (in
HttpRequest) - we get:

  X_FORWARDED_PROTO: http
  X-Forwarded-Proto: https

which is parsed to 

  HTTP_X_FORWARDED_PROTO"=>"http,https

There was a passenger ticket that describes that from nginx's point of
view - X-Forwarded-Proto is the "correct" form. 

-- 
/skaar

skaar@waste.org
where in the W.A.S.T.E is the wisdom
s_u_b_s_t_r_u_c_t_i_o_n
_______________________________________________
Unicorn mailing list - mongrel-unicorn@rubyforge.org
http://rubyforge.org/mailman/listinfo/mongrel-unicorn
Do not quote signatures (like this one) or top post when replying

Re: X-Forwarded-Proto / X_FORWARDED_PROTO

[Iñaki Baz Castillo]

El Sábado, 9 de Enero de 2010, skaar escribió:
> Eric,
> 
> think I came across an issue with the parser in unicorn, with a request
> (due to 2 layers of nginx proxying) coming across with both a
> X_FORWARDED_PROTO and a X-Forwarded-Proto header. From the socket (in
> HttpRequest) - we get:
> 
>   X_FORWARDED_PROTO: http
>   X-Forwarded-Proto: https
> 
> which is parsed to
> 
>   HTTP_X_FORWARDED_PROTO"=>"http,https
> 
> There was a passenger ticket that describes that from nginx's point of
> view - X-Forwarded-Proto is the "correct" form.

Rack specs require "-" being converted to "_" in header names, so both header 
X_FORWARDED_PROTO and X-Forwarded-Proto become the same header.

Then we have two headers with same name and different value. Some headers are 
defined as "multivalue" which means that they would have the same meaning if 
an unique header exists containing both values separated by comma (the top one 
first). So:

  X_FORWARDED_PROTO: http
  X-Forwarded-Proto: https

is equivalent to:

  X_FORWARDED_PROTO: http, https
 
However it's not true that *any* header behaves in this way, but just headers 
defined in such way (by the standards). For example two "Authorization" 
headers cannot be joined in a single header.

So it's not totally safe to assume that any header appearing various times 
could be a single header with their values separated by comma, however I think 
it's a good approach.

More correct would be that Unicorn parses uknown headers appearing more than 
one time by setting a Array as value:

  X_FORWARDED_PROTO: ["http", "https"]

Unfortunately this is not allowed in Rack specifications.


-- 
Iñaki Baz Castillo <ibc@aliax.net>
_______________________________________________
Unicorn mailing list - mongrel-unicorn@rubyforge.org
http://rubyforge.org/mailman/listinfo/mongrel-unicorn
Do not quote signatures (like this one) or top post when replying

Re: X-Forwarded-Proto / X_FORWARDED_PROTO

[Eric Wong]

skaar <skaar@waste.org> wrote:
> Eric,
> 
> think I came across an issue with the parser in unicorn, with a request
> (due to 2 layers of nginx proxying) coming across with both a
> X_FORWARDED_PROTO and a X-Forwarded-Proto header. From the socket (in
> HttpRequest) - we get:
> 
>   X_FORWARDED_PROTO: http
>   X-Forwarded-Proto: https
> 
> which is parsed to 
> 
>   HTTP_X_FORWARDED_PROTO"=>"http,https
> 
> There was a passenger ticket that describes that from nginx's point of
> view - X-Forwarded-Proto is the "correct" form. 

Hi skaar,

The Rack (and CGI) specs require that '-' be replaced with '_' for
HTTP header names, so Unicorn is doing the correct thing and treating
it as the same header.

Even though '_' appears to be allowed in header names by rfc 2616, it's
use is questionable as there are parsers that reject it.  Any chance of
fixing whatever sends the "X_FORWARDED_PROTO" header?  It's completely
pointless to set to "http"

-- 
Eric Wong
_______________________________________________
Unicorn mailing list - mongrel-unicorn@rubyforge.org
http://rubyforge.org/mailman/listinfo/mongrel-unicorn
Do not quote signatures (like this one) or top post when replying

Re: X-Forwarded-Proto / X_FORWARDED_PROTO

[skaar]

> 
> The Rack (and CGI) specs require that '-' be replaced with '_' for
> HTTP header names, so Unicorn is doing the correct thing and treating
> it as the same header.

but should it aggregate the values?
 
> Even though '_' appears to be allowed in header names by rfc 2616, it's
> use is questionable as there are parsers that reject it.  Any chance of
> fixing whatever sends the "X_FORWARDED_PROTO" header?  It's completely
> pointless to set to "http"

in our case we can probably fix the proxy chain - we have a set of
dedicated SSL systems (which sets X-Forwarded-Proto) which forward to
the main nginx front end (which sets X_FORWARDED_PROTO). I'll also look
at removing the proxy-set-header cases that sets it to to 'http'.

Just for reference: we ran into this issue when replacing nginx to
mongrels over a TCP socket, with nginx to unicorn over a domain socket.
With mongrel we don't have this problem (although I see from
e26ebc985b882c38da50fb0104791a5f2c0f8522 - that you added support for it
in http11)

/skaar






> 
> -- 
> Eric Wong
> _______________________________________________
> Unicorn mailing list - mongrel-unicorn@rubyforge.org
> http://rubyforge.org/mailman/listinfo/mongrel-unicorn
> Do not quote signatures (like this one) or top post when replying

-- 
/skaar

skaar@waste.org
where in the W.A.S.T.E is the wisdom
s_u_b_s_t_r_u_c_t_i_o_n
_______________________________________________
Unicorn mailing list - mongrel-unicorn@rubyforge.org
http://rubyforge.org/mailman/listinfo/mongrel-unicorn
Do not quote signatures (like this one) or top post when replying

Re: X-Forwarded-Proto / X_FORWARDED_PROTO

[Iñaki Baz Castillo]

El Domingo, 10 de Enero de 2010, skaar escribió:
> > The Rack (and CGI) specs require that '-' be replaced with '_' for
> > HTTP header names, so Unicorn is doing the correct thing and treating
> > it as the same header.
> 
> but should it aggregate the values?

Hi, review my other response in this thread.

This is undefined. Some headers do allow multiple values separated by comma in 
the same header with the same meaning as varios headers with same name and 
single values. But this depends on each header specification.

In your case it seems valid for me (just an opinnion) as 
"HTTP_X_FORWARDED_PROTO: http,https" could mean that the request has been sent 
using HTTPS and an intermediary proxy has forwarded it using HTTP. Of course 
the final destination (Unicorn application) must be ready to support such 
syntax.


-- 
Iñaki Baz Castillo <ibc@aliax.net>
_______________________________________________
Unicorn mailing list - mongrel-unicorn@rubyforge.org
http://rubyforge.org/mailman/listinfo/mongrel-unicorn
Do not quote signatures (like this one) or top post when replying

Re: X-Forwarded-Proto / X_FORWARDED_PROTO

[Eric Wong]

Iñaki Baz Castillo <ibc@aliax.net> wrote:
> El Domingo, 10 de Enero de 2010, skaar escribió:
> > > The Rack (and CGI) specs require that '-' be replaced with '_' for
> > > HTTP header names, so Unicorn is doing the correct thing and treating
> > > it as the same header.
> > 
> > but should it aggregate the values?
> 
> Hi, review my other response in this thread.
> 
> This is undefined. Some headers do allow multiple values separated by comma in 
> the same header with the same meaning as varios headers with same name and 
> single values. But this depends on each header specification.

>From reading rfc 2616, section 4.2 that all multi-value headers can be
combined with commas without changing semantics of the message[1].
There's also no other way (e.g. with an Array) to represent them for
Rack...

[1] In the Real-World(TM), this is not true for Set-Cookie headers in
    HTTP responses.

> In your case it seems valid for me (just an opinnion) as 
> "HTTP_X_FORWARDED_PROTO: http,https" could mean that the request has been sent 
> using HTTPS and an intermediary proxy has forwarded it using HTTP. Of course 
> the final destination (Unicorn application) must be ready to support such 
> syntax.

Is it safe to say that if there's an "https" *anywhere* in the
X-Forwarded-Proto chain, that "rack.url_scheme" should be set to
"https"?   Because I suppose most of the time there's only one
(client-facing) proxy using https.

-- 
Eric Wong
_______________________________________________
Unicorn mailing list - mongrel-unicorn@rubyforge.org
http://rubyforge.org/mailman/listinfo/mongrel-unicorn
Do not quote signatures (like this one) or top post when replying

Re: X-Forwarded-Proto / X_FORWARDED_PROTO

[Eric Wong]

Eric Wong <normalperson@yhbt.net> wrote:
> Iñaki Baz Castillo <ibc@aliax.net> wrote:
> > In your case it seems valid for me (just an opinnion) as 
> > "HTTP_X_FORWARDED_PROTO: http,https" could mean that the request has been sent 
> > using HTTPS and an intermediary proxy has forwarded it using HTTP. Of course 
> > the final destination (Unicorn application) must be ready to support such 
> > syntax.
> 
> Is it safe to say that if there's an "https" *anywhere* in the
> X-Forwarded-Proto chain, that "rack.url_scheme" should be set to
> "https"?   Because I suppose most of the time there's only one
> (client-facing) proxy using https.

Maybe this will work...

diff --git a/ext/unicorn_http/global_variables.h b/ext/unicorn_http/global_variables.h
index e593cf6..6705851 100644
--- a/ext/unicorn_http/global_variables.h
+++ b/ext/unicorn_http/global_variables.h
@@ -74,7 +74,6 @@ void init_globals(void)
   DEF_GLOBAL(server_name, "SERVER_NAME");
   DEF_GLOBAL(server_port, "SERVER_PORT");
   DEF_GLOBAL(server_protocol, "SERVER_PROTOCOL");
-  DEF_GLOBAL(http_x_forwarded_proto, "HTTP_X_FORWARDED_PROTO");
   DEF_GLOBAL(port_80, "80");
   DEF_GLOBAL(port_443, "443");
   DEF_GLOBAL(localhost, "localhost");
diff --git a/ext/unicorn_http/unicorn_http.rl b/ext/unicorn_http/unicorn_http.rl
index 6232e2c..f3945b2 100644
--- a/ext/unicorn_http/unicorn_http.rl
+++ b/ext/unicorn_http/unicorn_http.rl
@@ -197,6 +197,14 @@ static void write_value(VALUE req, struct http_parser *hp,
     assert_frozen(f);
   }
 
+  /*
+   * any X-Forwarded-Proto: https means there's an https server in the
+   * proxy chain, and that server is most likely the one that actually
+   * sees the client, so help Rack apps generate URLs with "https"
+   */
+  if (f == g_http_x_forwarded_proto && STR_CSTR_EQ(v, "https"))
+    rb_hash_aset(req, g_rack_url_scheme, v);
+
   e = rb_hash_aref(req, f);
   if (NIL_P(e)) {
     hp->cont = rb_hash_aset(req, f, v);
@@ -393,12 +401,7 @@ static void finalize_header(struct http_parser *hp, VALUE req)
 
   /* set rack.url_scheme to "https" or "http", no others are allowed by Rack */
   if (NIL_P(temp)) {
-    temp = rb_hash_aref(req, g_http_x_forwarded_proto);
-    if (!NIL_P(temp) && STR_CSTR_EQ(temp, "https"))
-      server_port = g_port_443;
-    else
-      temp = g_http;
-    rb_hash_aset(req, g_rack_url_scheme, temp);
+    rb_hash_aset(req, g_rack_url_scheme, g_http);
   } else if (STR_CSTR_EQ(temp, "https")) {
     server_port = g_port_443;
   } else {
@@ -712,5 +715,6 @@ void Init_unicorn_http(void)
   SET_GLOBAL(g_http_transfer_encoding, "TRANSFER_ENCODING");
   SET_GLOBAL(g_content_length, "CONTENT_LENGTH");
   SET_GLOBAL(g_http_connection, "CONNECTION");
+  SET_GLOBAL(g_http_x_forwarded_proto, "X_FORWARDED_PROTO");
 }
 #undef SET_GLOBAL
-- 
Eric Wong
_______________________________________________
Unicorn mailing list - mongrel-unicorn@rubyforge.org
http://rubyforge.org/mailman/listinfo/mongrel-unicorn
Do not quote signatures (like this one) or top post when replying

Re: X-Forwarded-Proto / X_FORWARDED_PROTO

[John-Paul Bader]

Hey Guys,

I posted my setup before but for this thread I'll do it again.

For http I have: varnish(80) -> nginx(8080) -> unicorn

And for https: nginx(443) -> varnish(80) -> nginx(8080) -> unicorn

Nginx has these proxy settings for the 443 vhost: 

location / {
      …
      access_log         off;
      proxy_redirect     off;
      proxy_pass         http://www.ccc.de;
      proxy_set_header   X-Real-IP  $remote_addr;
      proxy_set_header   X-FORWARDED-PROTO https;
    }
…

So I explicitly set this header. This way varnish(80), nginx(8080), unicorn and ultimately rails will get this Header and know that this was a https request. This works for me perfectly and maybe that is helpful for you ? If not never mind ;)

After all this header is a non standard extension introduced by squid. Theres a nice wikipedia page about it: http://en.wikipedia.org/wiki/X-Forwarded-For

Kind regards, John


On 10.01.2010, at 07:32, Eric Wong wrote:

> Eric Wong <normalperson@yhbt.net> wrote:
>> Iñaki Baz Castillo <ibc@aliax.net> wrote:
>>> In your case it seems valid for me (just an opinnion) as 
>>> "HTTP_X_FORWARDED_PROTO: http,https" could mean that the request has been sent 
>>> using HTTPS and an intermediary proxy has forwarded it using HTTP. Of course 
>>> the final destination (Unicorn application) must be ready to support such 
>>> syntax.
>> 
>> Is it safe to say that if there's an "https" *anywhere* in the
>> X-Forwarded-Proto chain, that "rack.url_scheme" should be set to
>> "https"?   Because I suppose most of the time there's only one
>> (client-facing) proxy using https.
> 
> Maybe this will work...
> 
> diff --git a/ext/unicorn_http/global_variables.h b/ext/unicorn_http/global_variables.h
> index e593cf6..6705851 100644
> --- a/ext/unicorn_http/global_variables.h
> +++ b/ext/unicorn_http/global_variables.h
> @@ -74,7 +74,6 @@ void init_globals(void)
>   DEF_GLOBAL(server_name, "SERVER_NAME");
>   DEF_GLOBAL(server_port, "SERVER_PORT");
>   DEF_GLOBAL(server_protocol, "SERVER_PROTOCOL");
> -  DEF_GLOBAL(http_x_forwarded_proto, "HTTP_X_FORWARDED_PROTO");
>   DEF_GLOBAL(port_80, "80");
>   DEF_GLOBAL(port_443, "443");
>   DEF_GLOBAL(localhost, "localhost");
> diff --git a/ext/unicorn_http/unicorn_http.rl b/ext/unicorn_http/unicorn_http.rl
> index 6232e2c..f3945b2 100644
> --- a/ext/unicorn_http/unicorn_http.rl
> +++ b/ext/unicorn_http/unicorn_http.rl
> @@ -197,6 +197,14 @@ static void write_value(VALUE req, struct http_parser *hp,
>     assert_frozen(f);
>   }
> 
> +  /*
> +   * any X-Forwarded-Proto: https means there's an https server in the
> +   * proxy chain, and that server is most likely the one that actually
> +   * sees the client, so help Rack apps generate URLs with "https"
> +   */
> +  if (f == g_http_x_forwarded_proto && STR_CSTR_EQ(v, "https"))
> +    rb_hash_aset(req, g_rack_url_scheme, v);
> +
>   e = rb_hash_aref(req, f);
>   if (NIL_P(e)) {
>     hp->cont = rb_hash_aset(req, f, v);
> @@ -393,12 +401,7 @@ static void finalize_header(struct http_parser *hp, VALUE req)
> 
>   /* set rack.url_scheme to "https" or "http", no others are allowed by Rack */
>   if (NIL_P(temp)) {
> -    temp = rb_hash_aref(req, g_http_x_forwarded_proto);
> -    if (!NIL_P(temp) && STR_CSTR_EQ(temp, "https"))
> -      server_port = g_port_443;
> -    else
> -      temp = g_http;
> -    rb_hash_aset(req, g_rack_url_scheme, temp);
> +    rb_hash_aset(req, g_rack_url_scheme, g_http);
>   } else if (STR_CSTR_EQ(temp, "https")) {
>     server_port = g_port_443;
>   } else {
> @@ -712,5 +715,6 @@ void Init_unicorn_http(void)
>   SET_GLOBAL(g_http_transfer_encoding, "TRANSFER_ENCODING");
>   SET_GLOBAL(g_content_length, "CONTENT_LENGTH");
>   SET_GLOBAL(g_http_connection, "CONNECTION");
> +  SET_GLOBAL(g_http_x_forwarded_proto, "X_FORWARDED_PROTO");
> }
> #undef SET_GLOBAL
> -- 
> Eric Wong
> _______________________________________________
> Unicorn mailing list - mongrel-unicorn@rubyforge.org
> http://rubyforge.org/mailman/listinfo/mongrel-unicorn
> Do not quote signatures (like this one) or top post when replying
> 

_______________________________________________
Unicorn mailing list - mongrel-unicorn@rubyforge.org
http://rubyforge.org/mailman/listinfo/mongrel-unicorn
Do not quote signatures (like this one) or top post when replying

Re: X-Forwarded-Proto / X_FORWARDED_PROTO

[Eric Wong]

Eric Wong <normalperson@yhbt.net> wrote:
> Eric Wong <normalperson@yhbt.net> wrote:
> > Iñaki Baz Castillo <ibc@aliax.net> wrote:
> > > In your case it seems valid for me (just an opinnion) as 
> > > "HTTP_X_FORWARDED_PROTO: http,https" could mean that the request has been sent 
> > > using HTTPS and an intermediary proxy has forwarded it using HTTP. Of course 
> > > the final destination (Unicorn application) must be ready to support such 
> > > syntax.
> > 
> > Is it safe to say that if there's an "https" *anywhere* in the
> > X-Forwarded-Proto chain, that "rack.url_scheme" should be set to
> > "https"?   Because I suppose most of the time there's only one
> > (client-facing) proxy using https.
> 
> Maybe this will work...

Any feedback on this patch would be appreciated, thanks.

> diff --git a/ext/unicorn_http/global_variables.h b/ext/unicorn_http/global_variables.h
> index e593cf6..6705851 100644
> --- a/ext/unicorn_http/global_variables.h
> +++ b/ext/unicorn_http/global_variables.h
> @@ -74,7 +74,6 @@ void init_globals(void)
>    DEF_GLOBAL(server_name, "SERVER_NAME");
>    DEF_GLOBAL(server_port, "SERVER_PORT");
>    DEF_GLOBAL(server_protocol, "SERVER_PROTOCOL");
> -  DEF_GLOBAL(http_x_forwarded_proto, "HTTP_X_FORWARDED_PROTO");
>    DEF_GLOBAL(port_80, "80");
>    DEF_GLOBAL(port_443, "443");
>    DEF_GLOBAL(localhost, "localhost");
> diff --git a/ext/unicorn_http/unicorn_http.rl b/ext/unicorn_http/unicorn_http.rl
> index 6232e2c..f3945b2 100644
> --- a/ext/unicorn_http/unicorn_http.rl
> +++ b/ext/unicorn_http/unicorn_http.rl
> @@ -197,6 +197,14 @@ static void write_value(VALUE req, struct http_parser *hp,
>      assert_frozen(f);
>    }
>  
> +  /*
> +   * any X-Forwarded-Proto: https means there's an https server in the
> +   * proxy chain, and that server is most likely the one that actually
> +   * sees the client, so help Rack apps generate URLs with "https"
> +   */
> +  if (f == g_http_x_forwarded_proto && STR_CSTR_EQ(v, "https"))
> +    rb_hash_aset(req, g_rack_url_scheme, v);
> +
>    e = rb_hash_aref(req, f);
>    if (NIL_P(e)) {
>      hp->cont = rb_hash_aset(req, f, v);
> @@ -393,12 +401,7 @@ static void finalize_header(struct http_parser *hp, VALUE req)
>  
>    /* set rack.url_scheme to "https" or "http", no others are allowed by Rack */
>    if (NIL_P(temp)) {
> -    temp = rb_hash_aref(req, g_http_x_forwarded_proto);
> -    if (!NIL_P(temp) && STR_CSTR_EQ(temp, "https"))
> -      server_port = g_port_443;
> -    else
> -      temp = g_http;
> -    rb_hash_aset(req, g_rack_url_scheme, temp);
> +    rb_hash_aset(req, g_rack_url_scheme, g_http);
>    } else if (STR_CSTR_EQ(temp, "https")) {
>      server_port = g_port_443;
>    } else {
> @@ -712,5 +715,6 @@ void Init_unicorn_http(void)
>    SET_GLOBAL(g_http_transfer_encoding, "TRANSFER_ENCODING");
>    SET_GLOBAL(g_content_length, "CONTENT_LENGTH");
>    SET_GLOBAL(g_http_connection, "CONNECTION");
> +  SET_GLOBAL(g_http_x_forwarded_proto, "X_FORWARDED_PROTO");
>  }
>  #undef SET_GLOBAL
-- 
Eric Wong
_______________________________________________
Unicorn mailing list - mongrel-unicorn@rubyforge.org
http://rubyforge.org/mailman/listinfo/mongrel-unicorn
Do not quote signatures (like this one) or top post when replying