From 54ff1ee513989fbd00b0aa7d4fe94baaae1ebbb1 Mon Sep 17 00:00:00 2001 From: gggeek Date: Wed, 17 Feb 2016 12:28:42 +0000 Subject: [PATCH 1/3] Fix EZP-25479: better support for nginx in front of varnish --- doc/varnish/vcl/varnish3.vcl | 20 +++++++++++++++++--- doc/varnish/vcl/varnish4.vcl | 20 +++++++++++++++++--- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/doc/varnish/vcl/varnish3.vcl b/doc/varnish/vcl/varnish3.vcl index 718c26ea75..ecbc031874 100644 --- a/doc/varnish/vcl/varnish3.vcl +++ b/doc/varnish/vcl/varnish3.vcl @@ -20,6 +20,12 @@ acl debuggers { "192.168.0.0"/16; } +// ACL for the proxies in front of Varnish (e.g. Nginx terminating https) +acl proxies { + "127.0.0.1"; + "192.168.0.0"/16; +} + // Called at the beginning of a request, after the complete request has been received sub vcl_recv { @@ -32,10 +38,18 @@ sub vcl_recv { // Add a unique header containing the client address (only for master request) // Please note that /_fragment URI can change in Symfony configuration if (!req.url ~ "^/_fragment") { - if (req.http.x-forwarded-for) { - set req.http.X-Forwarded-For = req.http.X-Forwarded-For + ", " + client.ip; + // only accept the x-forwarded-for header if the remote-proxy is trusted + // also we add our ip to the list of forwarders, as it is logged by apache by default + if (req.http.x-forwarded-for && client.ip ~ proxies) { + // funnily enough, it seems that this bit is executed twice, so we do a bit of dirty coding + if (req.http.X-Added-Forwarded-For) { + unset req.http.X-Added-Forwarded-For; + } else { + set req.http.X-Forwarded-For = req.http.X-Forwarded-For + ", " + server.ip; + set req.http.X-Added-Forwarded-For = "1"; + } } else { - set req.http.X-Forwarded-For = client.ip; + set req.http.X-Forwarded-For = "" + client.ip + ", " + server.ip; } } diff --git a/doc/varnish/vcl/varnish4.vcl b/doc/varnish/vcl/varnish4.vcl index 8d743b6563..10e3e88b78 100644 --- a/doc/varnish/vcl/varnish4.vcl +++ b/doc/varnish/vcl/varnish4.vcl @@ -22,6 +22,12 @@ acl debuggers { "192.168.0.0"/16; } +// ACL for the proxies in front of Varnish (e.g. Nginx terminating https) +acl proxies { + "127.0.0.1"; + "192.168.0.0"/16; +} + // Called at the beginning of a request, after the complete request has been received sub vcl_recv { @@ -34,10 +40,18 @@ sub vcl_recv { // Add a unique header containing the client address (only for master request) // Please note that /_fragment URI can change in Symfony configuration if (!req.url ~ "^/_fragment") { - if (req.http.x-forwarded-for) { - set req.http.X-Forwarded-For = req.http.X-Forwarded-For + ", " + client.ip; + // only accept the x-forwarded-for header if the remote-proxy is trusted + // also we add our ip to the list of forwarders, as it is logged by apache by default + if (req.http.x-forwarded-for && client.ip ~ proxies) { + // funnily enough, it seems that this bit is executed twice, so we do a bit of dirty coding + if (req.http.X-Added-Forwarded-For) { + unset req.http.X-Added-Forwarded-For; + } else { + set req.http.X-Forwarded-For = req.http.X-Forwarded-For + ", " + server.ip; + set req.http.X-Added-Forwarded-For = "1"; + } } else { - set req.http.X-Forwarded-For = client.ip; + set req.http.X-Forwarded-For = "" + client.ip + ", " + server.ip; } } From e996c96bf5f293c85a6ab5cc8499cb67a8c5f38b Mon Sep 17 00:00:00 2001 From: gggeek Date: Mon, 28 Mar 2016 20:08:43 +0100 Subject: [PATCH 2/3] Simplify management of X-Forwarded-for header; take into account the presence of a reverse proxy when sending debug headers --- doc/varnish/vcl/varnish4.vcl | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/doc/varnish/vcl/varnish4.vcl b/doc/varnish/vcl/varnish4.vcl index 10e3e88b78..ff6fd3fe3f 100644 --- a/doc/varnish/vcl/varnish4.vcl +++ b/doc/varnish/vcl/varnish4.vcl @@ -39,17 +39,12 @@ sub vcl_recv { // Add a unique header containing the client address (only for master request) // Please note that /_fragment URI can change in Symfony configuration - if (!req.url ~ "^/_fragment") { + // Take care of requests getting 'restarted' because of the user-hash lookup + if (req.url !~ "^/_fragment" && req.restarts == 0) { // only accept the x-forwarded-for header if the remote-proxy is trusted // also we add our ip to the list of forwarders, as it is logged by apache by default if (req.http.x-forwarded-for && client.ip ~ proxies) { - // funnily enough, it seems that this bit is executed twice, so we do a bit of dirty coding - if (req.http.X-Added-Forwarded-For) { - unset req.http.X-Added-Forwarded-For; - } else { - set req.http.X-Forwarded-For = req.http.X-Forwarded-For + ", " + server.ip; - set req.http.X-Added-Forwarded-For = "1"; - } + set req.http.X-Forwarded-For = req.http.X-Forwarded-For + ", " + server.ip; } else { set req.http.X-Forwarded-For = "" + client.ip + ", " + server.ip; } @@ -223,7 +218,7 @@ sub vcl_deliver { // Sanity check to prevent ever exposing the hash to a client. unset resp.http.x-user-hash; - if (client.ip ~ debuggers) { + if ((client.ip ~ debuggers) || (client.ip ~ proxies && std.ip(regsub(req.http.X-Forwarded-For, "^(([0-9]{1,3}\.){3}[0-9]{1,3}),(.*)$", "\1"), "0.0.0.0") ~ debuggers)) { if (obj.hits > 0) { set resp.http.X-Cache = "HIT"; set resp.http.X-Cache-Hits = obj.hits; From 29019f2131b441a79c8d4d78fb0f2b889852a8eb Mon Sep 17 00:00:00 2001 From: gggeek Date: Mon, 28 Mar 2016 20:18:35 +0100 Subject: [PATCH 3/3] Port latest changes to vcl v3 and improve docs --- doc/varnish/vcl/varnish3.vcl | 18 +++++++----------- doc/varnish/vcl/varnish4.vcl | 5 +++-- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/doc/varnish/vcl/varnish3.vcl b/doc/varnish/vcl/varnish3.vcl index ecbc031874..b7579bfe78 100644 --- a/doc/varnish/vcl/varnish3.vcl +++ b/doc/varnish/vcl/varnish3.vcl @@ -37,17 +37,13 @@ sub vcl_recv { // Add a unique header containing the client address (only for master request) // Please note that /_fragment URI can change in Symfony configuration - if (!req.url ~ "^/_fragment") { - // only accept the x-forwarded-for header if the remote-proxy is trusted - // also we add our ip to the list of forwarders, as it is logged by apache by default + // Take care of requests getting 'restarted' because of the user-hash lookup + if (req.url !~ "^/_fragment" && req.restarts == 0) { + // Only accept the x-forwarded-for header if the remote-proxy is trusted + // Also we add our ip to the list of forwarders, as it is logged by Apache by default, e.g. + // ip-client, ip-nginx, ip-varnish - - [17/Feb/2016:10:55:08 +0000] "GET /setup/info/php HTTP/1.1" 200 ... if (req.http.x-forwarded-for && client.ip ~ proxies) { - // funnily enough, it seems that this bit is executed twice, so we do a bit of dirty coding - if (req.http.X-Added-Forwarded-For) { - unset req.http.X-Added-Forwarded-For; - } else { - set req.http.X-Forwarded-For = req.http.X-Forwarded-For + ", " + server.ip; - set req.http.X-Added-Forwarded-For = "1"; - } + set req.http.X-Forwarded-For = req.http.X-Forwarded-For + ", " + server.ip; } else { set req.http.X-Forwarded-For = "" + client.ip + ", " + server.ip; } @@ -227,7 +223,7 @@ sub vcl_deliver { // Sanity check to prevent ever exposing the hash to a client. unset resp.http.x-user-hash; - if (client.ip ~ debuggers) { + if ((client.ip ~ debuggers) || (client.ip ~ proxies && std.ip(regsub(req.http.X-Forwarded-For, "^(([0-9]{1,3}\.){3}[0-9]{1,3}),(.*)$", "\1"), "0.0.0.0") ~ debuggers)) { if (obj.hits > 0) { set resp.http.X-Cache = "HIT"; set resp.http.X-Cache-Hits = obj.hits; diff --git a/doc/varnish/vcl/varnish4.vcl b/doc/varnish/vcl/varnish4.vcl index ff6fd3fe3f..1c01c80937 100644 --- a/doc/varnish/vcl/varnish4.vcl +++ b/doc/varnish/vcl/varnish4.vcl @@ -41,8 +41,9 @@ sub vcl_recv { // Please note that /_fragment URI can change in Symfony configuration // Take care of requests getting 'restarted' because of the user-hash lookup if (req.url !~ "^/_fragment" && req.restarts == 0) { - // only accept the x-forwarded-for header if the remote-proxy is trusted - // also we add our ip to the list of forwarders, as it is logged by apache by default + // Only accept the x-forwarded-for header if the remote-proxy is trusted + // Also we add our ip to the list of forwarders, as it is logged by Apache by default, e.g. + // ip-client, ip-nginx, ip-varnish - - [17/Feb/2016:10:55:08 +0000] "GET /setup/info/php HTTP/1.1" 200 ... if (req.http.x-forwarded-for && client.ip ~ proxies) { set req.http.X-Forwarded-For = req.http.X-Forwarded-For + ", " + server.ip; } else {