Content Cache Charm

Merge ~hloeung/content-cache-charm:nginx-config into content-cache-charm:master

  1. Git
  2. lp:~hloeung/content-cache-charm
  3. nginx-config
  4. Merge into master
Proposed by Haw Loeung
Status: Merged
Approved by: Haw Loeung
Approved revision: ff185866ce85b9c99da9712c502eea59311dbdfa
Merged at revision: 9f064cb7f4b29edf9f2a344574288cd7b8dd703a
Proposed branch: ~hloeung/content-cache-charm:nginx-config
Merge into: content-cache-charm:master
Diff against target: 230 lines (+84/-0)
12 files modified
templates/nginx_cfg.tmpl (+6/-0)
tests/unit/files/nginx_config_rendered_test_output-basic_site.txt (+6/-0)
tests/unit/files/nginx_config_rendered_test_output-site1.local-secrets.txt (+6/-0)
tests/unit/files/nginx_config_rendered_test_output-site1.local.txt (+6/-0)
tests/unit/files/nginx_config_rendered_test_output-site2.local.txt (+6/-0)
tests/unit/files/nginx_config_rendered_test_output-site3.local.txt (+6/-0)
tests/unit/files/nginx_config_rendered_test_output-site5.txt (+12/-0)
tests/unit/files/nginx_config_rendered_test_output-site6.local.txt (+6/-0)
tests/unit/files/nginx_config_rendered_test_output-site7.local.txt (+6/-0)
tests/unit/files/nginx_config_rendered_test_output-site8.local.txt (+12/-0)
tests/unit/files/nginx_config_rendered_test_output-site9.local.txt (+6/-0)
tests/unit/files/nginx_config_rendered_test_output-token_site.txt (+6/-0)
Reviewer Review Type Date Requested Status
Junien F Approve
Haw Loeung Needs Resubmitting
Joel Sing (community) +1 Approve
Review via email: mp+383187@code.launchpad.net

Commit message

Strip or remove 'Forwarded' header to avoid cache poisoning

Description of the change

This can happen when backends uses the forward headers (X-Forward-For
and Forwarded) and prefer Forwarded over X-Forward-For.
e.g. Kubernetes with use-forwarded-headers.

Also blacklist X-Forwarded-Host, X-Forwarded-Port, and
X-Forwarded-Scheme just to be super safe.

To post a comment you must log in.
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

Revision history for this message
Junien F (axino) wrote :

+1 - we should ALWAYS either set it or hide it. Hiding it is good.

review: Approve
Revision history for this message
Joel Sing (jsing) wrote :

We probably should eventually set the Forwarded header, but removing it is preferable to the current situation.

review: Approve (+1)
Revision history for this message
Joel Sing (jsing) wrote :

Turns out proxy_hide_header is in the other direction (server back to client) - we need 'proxy_set_header Forwarded "";' instead. We probably want to consider X-Forwarded-{Host,Port,Proto,Scheme} as well.

Revision history for this message
Junien F (axino) wrote :

Actually this is not the appropriate stanza - we need to use proxy_set_header

review: Needs Fixing
Revision history for this message
Haw Loeung (hloeung) :
review: Needs Resubmitting
Revision history for this message
Haw Loeung (hloeung) wrote :

Had a look and we can also strip these headers on HAProxy's side:

| backend backend-cached-archive-ubuntu-com
| option forwardfor
| http-request del-header Forwarded

Revision history for this message
Junien F (axino) wrote :

+1

review: Approve
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

Change successfully merged at revision 9f064cb7f4b29edf9f2a344574288cd7b8dd703a

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/templates/nginx_cfg.tmpl b/templates/nginx_cfg.tmpl
index 70be061..c583365 100644
--- a/templates/nginx_cfg.tmpl
+++ b/templates/nginx_cfg.tmpl
@@ -13,6 +13,12 @@ server {
13{%- if conf['backend'] %}13{%- if conf['backend'] %}
14 proxy_pass {{conf['backend']}};14 proxy_pass {{conf['backend']}};
15 proxy_set_header Host "{{site_name}}";15 proxy_set_header Host "{{site_name}}";
16 # Removed the following headers to avoid cache poisoning.
17 proxy_set_header Forwarded "";
18 proxy_set_header X-Forwarded-Host "";
19 proxy_set_header X-Forwarded-Port "";
20 proxy_set_header X-Forwarded-Scheme "";
21
16 add_header X-Cache-Status "$upstream_cache_status from {{juju_unit}}";22 add_header X-Cache-Status "$upstream_cache_status from {{juju_unit}}";
17 proxy_force_ranges {{conf['force_ranges']}};23 proxy_force_ranges {{conf['force_ranges']}};
18 proxy_cache {{keys_zone}};24 proxy_cache {{keys_zone}};
diff --git a/tests/unit/files/nginx_config_rendered_test_output-basic_site.txt b/tests/unit/files/nginx_config_rendered_test_output-basic_site.txt
index 7b2cac5..c4c29c7 100644
--- a/tests/unit/files/nginx_config_rendered_test_output-basic_site.txt
+++ b/tests/unit/files/nginx_config_rendered_test_output-basic_site.txt
@@ -10,6 +10,12 @@ server {
10 location / {10 location / {
11 proxy_pass http://localhost:8080;11 proxy_pass http://localhost:8080;
12 proxy_set_header Host "basic_site";12 proxy_set_header Host "basic_site";
13 # Removed the following headers to avoid cache poisoning.
14 proxy_set_header Forwarded "";
15 proxy_set_header X-Forwarded-Host "";
16 proxy_set_header X-Forwarded-Port "";
17 proxy_set_header X-Forwarded-Scheme "";
18
13 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";19 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";
14 proxy_force_ranges on;20 proxy_force_ranges on;
15 proxy_cache e176ab9d1f16-cache;21 proxy_cache e176ab9d1f16-cache;
diff --git a/tests/unit/files/nginx_config_rendered_test_output-site1.local-secrets.txt b/tests/unit/files/nginx_config_rendered_test_output-site1.local-secrets.txt
index adc4c47..17f7fba 100644
--- a/tests/unit/files/nginx_config_rendered_test_output-site1.local-secrets.txt
+++ b/tests/unit/files/nginx_config_rendered_test_output-site1.local-secrets.txt
@@ -10,6 +10,12 @@ server {
10 location / {10 location / {
11 proxy_pass http://localhost:8080;11 proxy_pass http://localhost:8080;
12 proxy_set_header Host "site1.local";12 proxy_set_header Host "site1.local";
13 # Removed the following headers to avoid cache poisoning.
14 proxy_set_header Forwarded "";
15 proxy_set_header X-Forwarded-Host "";
16 proxy_set_header X-Forwarded-Port "";
17 proxy_set_header X-Forwarded-Scheme "";
18
13 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";19 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";
14 proxy_force_ranges on;20 proxy_force_ranges on;
15 proxy_cache a5586980e57a-cache;21 proxy_cache a5586980e57a-cache;
diff --git a/tests/unit/files/nginx_config_rendered_test_output-site1.local.txt b/tests/unit/files/nginx_config_rendered_test_output-site1.local.txt
index 59ebf86..cc34e05 100644
--- a/tests/unit/files/nginx_config_rendered_test_output-site1.local.txt
+++ b/tests/unit/files/nginx_config_rendered_test_output-site1.local.txt
@@ -10,6 +10,12 @@ server {
10 location / {10 location / {
11 proxy_pass http://localhost:8080;11 proxy_pass http://localhost:8080;
12 proxy_set_header Host "site1.local";12 proxy_set_header Host "site1.local";
13 # Removed the following headers to avoid cache poisoning.
14 proxy_set_header Forwarded "";
15 proxy_set_header X-Forwarded-Host "";
16 proxy_set_header X-Forwarded-Port "";
17 proxy_set_header X-Forwarded-Scheme "";
18
13 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";19 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";
14 proxy_force_ranges on;20 proxy_force_ranges on;
15 proxy_cache a5586980e57a-cache;21 proxy_cache a5586980e57a-cache;
diff --git a/tests/unit/files/nginx_config_rendered_test_output-site2.local.txt b/tests/unit/files/nginx_config_rendered_test_output-site2.local.txt
index 3695d49..a2823f5 100644
--- a/tests/unit/files/nginx_config_rendered_test_output-site2.local.txt
+++ b/tests/unit/files/nginx_config_rendered_test_output-site2.local.txt
@@ -10,6 +10,12 @@ server {
10 location / {10 location / {
11 proxy_pass http://localhost:8081;11 proxy_pass http://localhost:8081;
12 proxy_set_header Host "site2.local";12 proxy_set_header Host "site2.local";
13 # Removed the following headers to avoid cache poisoning.
14 proxy_set_header Forwarded "";
15 proxy_set_header X-Forwarded-Host "";
16 proxy_set_header X-Forwarded-Port "";
17 proxy_set_header X-Forwarded-Scheme "";
18
13 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";19 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";
14 proxy_force_ranges on;20 proxy_force_ranges on;
15 proxy_cache 9813f9fe7826-cache;21 proxy_cache 9813f9fe7826-cache;
diff --git a/tests/unit/files/nginx_config_rendered_test_output-site3.local.txt b/tests/unit/files/nginx_config_rendered_test_output-site3.local.txt
index 2a1b852..40da73c 100644
--- a/tests/unit/files/nginx_config_rendered_test_output-site3.local.txt
+++ b/tests/unit/files/nginx_config_rendered_test_output-site3.local.txt
@@ -10,6 +10,12 @@ server {
10 location / {10 location / {
11 proxy_pass http://localhost:8082;11 proxy_pass http://localhost:8082;
12 proxy_set_header Host "site3.local";12 proxy_set_header Host "site3.local";
13 # Removed the following headers to avoid cache poisoning.
14 proxy_set_header Forwarded "";
15 proxy_set_header X-Forwarded-Host "";
16 proxy_set_header X-Forwarded-Port "";
17 proxy_set_header X-Forwarded-Scheme "";
18
13 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";19 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";
14 proxy_force_ranges on;20 proxy_force_ranges on;
15 proxy_cache d515cd6e60b1-cache;21 proxy_cache d515cd6e60b1-cache;
diff --git a/tests/unit/files/nginx_config_rendered_test_output-site5.txt b/tests/unit/files/nginx_config_rendered_test_output-site5.txt
index 8f1cc3d..1dd3549 100644
--- a/tests/unit/files/nginx_config_rendered_test_output-site5.txt
+++ b/tests/unit/files/nginx_config_rendered_test_output-site5.txt
@@ -10,6 +10,12 @@ server {
10 location / {10 location / {
11 proxy_pass http://localhost:8083;11 proxy_pass http://localhost:8083;
12 proxy_set_header Host "site5.local";12 proxy_set_header Host "site5.local";
13 # Removed the following headers to avoid cache poisoning.
14 proxy_set_header Forwarded "";
15 proxy_set_header X-Forwarded-Host "";
16 proxy_set_header X-Forwarded-Port "";
17 proxy_set_header X-Forwarded-Scheme "";
18
13 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";19 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";
14 proxy_force_ranges on;20 proxy_force_ranges on;
15 proxy_cache a5c1b7c73d09-cache;21 proxy_cache a5c1b7c73d09-cache;
@@ -25,6 +31,12 @@ server {
25 location = /auth {31 location = /auth {
26 proxy_pass http://localhost:8084/auth-check/;32 proxy_pass http://localhost:8084/auth-check/;
27 proxy_set_header Host "site5.local";33 proxy_set_header Host "site5.local";
34 # Removed the following headers to avoid cache poisoning.
35 proxy_set_header Forwarded "";
36 proxy_set_header X-Forwarded-Host "";
37 proxy_set_header X-Forwarded-Port "";
38 proxy_set_header X-Forwarded-Scheme "";
39
28 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";40 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";
29 proxy_force_ranges on;41 proxy_force_ranges on;
30 proxy_cache a5c1b7c73d09-cache;42 proxy_cache a5c1b7c73d09-cache;
diff --git a/tests/unit/files/nginx_config_rendered_test_output-site6.local.txt b/tests/unit/files/nginx_config_rendered_test_output-site6.local.txt
index 2f359c6..0663432 100644
--- a/tests/unit/files/nginx_config_rendered_test_output-site6.local.txt
+++ b/tests/unit/files/nginx_config_rendered_test_output-site6.local.txt
@@ -10,6 +10,12 @@ server {
10 location / {10 location / {
11 proxy_pass http://localhost:8085;11 proxy_pass http://localhost:8085;
12 proxy_set_header Host "site6.local";12 proxy_set_header Host "site6.local";
13 # Removed the following headers to avoid cache poisoning.
14 proxy_set_header Forwarded "";
15 proxy_set_header X-Forwarded-Host "";
16 proxy_set_header X-Forwarded-Port "";
17 proxy_set_header X-Forwarded-Scheme "";
18
13 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";19 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";
14 proxy_force_ranges on;20 proxy_force_ranges on;
15 proxy_cache 48e3ce186287-cache;21 proxy_cache 48e3ce186287-cache;
diff --git a/tests/unit/files/nginx_config_rendered_test_output-site7.local.txt b/tests/unit/files/nginx_config_rendered_test_output-site7.local.txt
index a223624..b87d1ee 100644
--- a/tests/unit/files/nginx_config_rendered_test_output-site7.local.txt
+++ b/tests/unit/files/nginx_config_rendered_test_output-site7.local.txt
@@ -10,6 +10,12 @@ server {
10 location / {10 location / {
11 proxy_pass http://localhost:8086;11 proxy_pass http://localhost:8086;
12 proxy_set_header Host "site7.local";12 proxy_set_header Host "site7.local";
13 # Removed the following headers to avoid cache poisoning.
14 proxy_set_header Forwarded "";
15 proxy_set_header X-Forwarded-Host "";
16 proxy_set_header X-Forwarded-Port "";
17 proxy_set_header X-Forwarded-Scheme "";
18
13 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";19 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";
14 proxy_force_ranges on;20 proxy_force_ranges on;
15 proxy_cache 1d780969d5c7-cache;21 proxy_cache 1d780969d5c7-cache;
diff --git a/tests/unit/files/nginx_config_rendered_test_output-site8.local.txt b/tests/unit/files/nginx_config_rendered_test_output-site8.local.txt
index 5e324e8..e21326b 100644
--- a/tests/unit/files/nginx_config_rendered_test_output-site8.local.txt
+++ b/tests/unit/files/nginx_config_rendered_test_output-site8.local.txt
@@ -10,6 +10,12 @@ server {
10 location / {10 location / {
11 proxy_pass http://localhost:8087;11 proxy_pass http://localhost:8087;
12 proxy_set_header Host "site8.local";12 proxy_set_header Host "site8.local";
13 # Removed the following headers to avoid cache poisoning.
14 proxy_set_header Forwarded "";
15 proxy_set_header X-Forwarded-Host "";
16 proxy_set_header X-Forwarded-Port "";
17 proxy_set_header X-Forwarded-Scheme "";
18
13 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";19 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";
14 proxy_force_ranges on;20 proxy_force_ranges on;
15 proxy_cache dcd95df02bf2-cache;21 proxy_cache dcd95df02bf2-cache;
@@ -25,6 +31,12 @@ server {
25 location /auth {31 location /auth {
26 proxy_pass http://localhost:8088;32 proxy_pass http://localhost:8088;
27 proxy_set_header Host "site8.local";33 proxy_set_header Host "site8.local";
34 # Removed the following headers to avoid cache poisoning.
35 proxy_set_header Forwarded "";
36 proxy_set_header X-Forwarded-Host "";
37 proxy_set_header X-Forwarded-Port "";
38 proxy_set_header X-Forwarded-Scheme "";
39
28 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";40 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";
29 proxy_force_ranges on;41 proxy_force_ranges on;
30 proxy_cache dcd95df02bf2-cache;42 proxy_cache dcd95df02bf2-cache;
diff --git a/tests/unit/files/nginx_config_rendered_test_output-site9.local.txt b/tests/unit/files/nginx_config_rendered_test_output-site9.local.txt
index 80dbdfa..44d8b57 100644
--- a/tests/unit/files/nginx_config_rendered_test_output-site9.local.txt
+++ b/tests/unit/files/nginx_config_rendered_test_output-site9.local.txt
@@ -10,6 +10,12 @@ server {
10 location / {10 location / {
11 proxy_pass http://localhost:8089;11 proxy_pass http://localhost:8089;
12 proxy_set_header Host "site9.local";12 proxy_set_header Host "site9.local";
13 # Removed the following headers to avoid cache poisoning.
14 proxy_set_header Forwarded "";
15 proxy_set_header X-Forwarded-Host "";
16 proxy_set_header X-Forwarded-Port "";
17 proxy_set_header X-Forwarded-Scheme "";
18
13 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";19 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";
14 proxy_force_ranges off;20 proxy_force_ranges off;
15 proxy_cache 8facdb232d5b-cache;21 proxy_cache 8facdb232d5b-cache;
diff --git a/tests/unit/files/nginx_config_rendered_test_output-token_site.txt b/tests/unit/files/nginx_config_rendered_test_output-token_site.txt
index 94fbf13..ffd7b0b 100644
--- a/tests/unit/files/nginx_config_rendered_test_output-token_site.txt
+++ b/tests/unit/files/nginx_config_rendered_test_output-token_site.txt
@@ -10,6 +10,12 @@ server {
10 location / {10 location / {
11 proxy_pass http://localhost:8080;11 proxy_pass http://localhost:8080;
12 proxy_set_header Host "token_site";12 proxy_set_header Host "token_site";
13 # Removed the following headers to avoid cache poisoning.
14 proxy_set_header Forwarded "";
15 proxy_set_header X-Forwarded-Host "";
16 proxy_set_header X-Forwarded-Port "";
17 proxy_set_header X-Forwarded-Scheme "";
18
13 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";19 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";
14 proxy_force_ranges on;20 proxy_force_ranges on;
15 proxy_cache c27ba6753fee-cache;21 proxy_cache c27ba6753fee-cache;

Subscribers

People subscribed via source and target branches