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 at 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
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 at aliax.net>
skaar <skaar at 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
> > 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 at rubyforge.org > http://rubyforge.org/mailman/listinfo/mongrel-unicorn > Do not quote signatures (like this one) or top post when replying-- /skaar skaar at 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
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 at aliax.net>
I?aki Baz Castillo <ibc at 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 becombined 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
Eric Wong <normalperson at yhbt.net> wrote:> I?aki Baz Castillo <ibc at 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
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 at yhbt.net> wrote:
>> I?aki Baz Castillo <ibc at 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 at rubyforge.org
> http://rubyforge.org/mailman/listinfo/mongrel-unicorn
> Do not quote signatures (like this one) or top post when replying
>
Eric Wong <normalperson at yhbt.net> wrote:> Eric Wong <normalperson at yhbt.net> wrote: > > I?aki Baz Castillo <ibc at 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