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
1diff --git a/templates/nginx_cfg.tmpl b/templates/nginx_cfg.tmpl
2index 70be061..c583365 100644
3--- a/templates/nginx_cfg.tmpl
4+++ b/templates/nginx_cfg.tmpl
5@@ -13,6 +13,12 @@ server {
6 {%- if conf['backend'] %}
7 proxy_pass {{conf['backend']}};
8 proxy_set_header Host "{{site_name}}";
9+ # Removed the following headers to avoid cache poisoning.
10+ proxy_set_header Forwarded "";
11+ proxy_set_header X-Forwarded-Host "";
12+ proxy_set_header X-Forwarded-Port "";
13+ proxy_set_header X-Forwarded-Scheme "";
14+
15 add_header X-Cache-Status "$upstream_cache_status from {{juju_unit}}";
16 proxy_force_ranges {{conf['force_ranges']}};
17 proxy_cache {{keys_zone}};
18diff --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
19index 7b2cac5..c4c29c7 100644
20--- a/tests/unit/files/nginx_config_rendered_test_output-basic_site.txt
21+++ b/tests/unit/files/nginx_config_rendered_test_output-basic_site.txt
22@@ -10,6 +10,12 @@ server {
23 location / {
24 proxy_pass http://localhost:8080;
25 proxy_set_header Host "basic_site";
26+ # Removed the following headers to avoid cache poisoning.
27+ proxy_set_header Forwarded "";
28+ proxy_set_header X-Forwarded-Host "";
29+ proxy_set_header X-Forwarded-Port "";
30+ proxy_set_header X-Forwarded-Scheme "";
31+
32 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";
33 proxy_force_ranges on;
34 proxy_cache e176ab9d1f16-cache;
35diff --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
36index adc4c47..17f7fba 100644
37--- a/tests/unit/files/nginx_config_rendered_test_output-site1.local-secrets.txt
38+++ b/tests/unit/files/nginx_config_rendered_test_output-site1.local-secrets.txt
39@@ -10,6 +10,12 @@ server {
40 location / {
41 proxy_pass http://localhost:8080;
42 proxy_set_header Host "site1.local";
43+ # Removed the following headers to avoid cache poisoning.
44+ proxy_set_header Forwarded "";
45+ proxy_set_header X-Forwarded-Host "";
46+ proxy_set_header X-Forwarded-Port "";
47+ proxy_set_header X-Forwarded-Scheme "";
48+
49 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";
50 proxy_force_ranges on;
51 proxy_cache a5586980e57a-cache;
52diff --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
53index 59ebf86..cc34e05 100644
54--- a/tests/unit/files/nginx_config_rendered_test_output-site1.local.txt
55+++ b/tests/unit/files/nginx_config_rendered_test_output-site1.local.txt
56@@ -10,6 +10,12 @@ server {
57 location / {
58 proxy_pass http://localhost:8080;
59 proxy_set_header Host "site1.local";
60+ # Removed the following headers to avoid cache poisoning.
61+ proxy_set_header Forwarded "";
62+ proxy_set_header X-Forwarded-Host "";
63+ proxy_set_header X-Forwarded-Port "";
64+ proxy_set_header X-Forwarded-Scheme "";
65+
66 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";
67 proxy_force_ranges on;
68 proxy_cache a5586980e57a-cache;
69diff --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
70index 3695d49..a2823f5 100644
71--- a/tests/unit/files/nginx_config_rendered_test_output-site2.local.txt
72+++ b/tests/unit/files/nginx_config_rendered_test_output-site2.local.txt
73@@ -10,6 +10,12 @@ server {
74 location / {
75 proxy_pass http://localhost:8081;
76 proxy_set_header Host "site2.local";
77+ # Removed the following headers to avoid cache poisoning.
78+ proxy_set_header Forwarded "";
79+ proxy_set_header X-Forwarded-Host "";
80+ proxy_set_header X-Forwarded-Port "";
81+ proxy_set_header X-Forwarded-Scheme "";
82+
83 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";
84 proxy_force_ranges on;
85 proxy_cache 9813f9fe7826-cache;
86diff --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
87index 2a1b852..40da73c 100644
88--- a/tests/unit/files/nginx_config_rendered_test_output-site3.local.txt
89+++ b/tests/unit/files/nginx_config_rendered_test_output-site3.local.txt
90@@ -10,6 +10,12 @@ server {
91 location / {
92 proxy_pass http://localhost:8082;
93 proxy_set_header Host "site3.local";
94+ # Removed the following headers to avoid cache poisoning.
95+ proxy_set_header Forwarded "";
96+ proxy_set_header X-Forwarded-Host "";
97+ proxy_set_header X-Forwarded-Port "";
98+ proxy_set_header X-Forwarded-Scheme "";
99+
100 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";
101 proxy_force_ranges on;
102 proxy_cache d515cd6e60b1-cache;
103diff --git a/tests/unit/files/nginx_config_rendered_test_output-site5.txt b/tests/unit/files/nginx_config_rendered_test_output-site5.txt
104index 8f1cc3d..1dd3549 100644
105--- a/tests/unit/files/nginx_config_rendered_test_output-site5.txt
106+++ b/tests/unit/files/nginx_config_rendered_test_output-site5.txt
107@@ -10,6 +10,12 @@ server {
108 location / {
109 proxy_pass http://localhost:8083;
110 proxy_set_header Host "site5.local";
111+ # Removed the following headers to avoid cache poisoning.
112+ proxy_set_header Forwarded "";
113+ proxy_set_header X-Forwarded-Host "";
114+ proxy_set_header X-Forwarded-Port "";
115+ proxy_set_header X-Forwarded-Scheme "";
116+
117 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";
118 proxy_force_ranges on;
119 proxy_cache a5c1b7c73d09-cache;
120@@ -25,6 +31,12 @@ server {
121 location = /auth {
122 proxy_pass http://localhost:8084/auth-check/;
123 proxy_set_header Host "site5.local";
124+ # Removed the following headers to avoid cache poisoning.
125+ proxy_set_header Forwarded "";
126+ proxy_set_header X-Forwarded-Host "";
127+ proxy_set_header X-Forwarded-Port "";
128+ proxy_set_header X-Forwarded-Scheme "";
129+
130 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";
131 proxy_force_ranges on;
132 proxy_cache a5c1b7c73d09-cache;
133diff --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
134index 2f359c6..0663432 100644
135--- a/tests/unit/files/nginx_config_rendered_test_output-site6.local.txt
136+++ b/tests/unit/files/nginx_config_rendered_test_output-site6.local.txt
137@@ -10,6 +10,12 @@ server {
138 location / {
139 proxy_pass http://localhost:8085;
140 proxy_set_header Host "site6.local";
141+ # Removed the following headers to avoid cache poisoning.
142+ proxy_set_header Forwarded "";
143+ proxy_set_header X-Forwarded-Host "";
144+ proxy_set_header X-Forwarded-Port "";
145+ proxy_set_header X-Forwarded-Scheme "";
146+
147 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";
148 proxy_force_ranges on;
149 proxy_cache 48e3ce186287-cache;
150diff --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
151index a223624..b87d1ee 100644
152--- a/tests/unit/files/nginx_config_rendered_test_output-site7.local.txt
153+++ b/tests/unit/files/nginx_config_rendered_test_output-site7.local.txt
154@@ -10,6 +10,12 @@ server {
155 location / {
156 proxy_pass http://localhost:8086;
157 proxy_set_header Host "site7.local";
158+ # Removed the following headers to avoid cache poisoning.
159+ proxy_set_header Forwarded "";
160+ proxy_set_header X-Forwarded-Host "";
161+ proxy_set_header X-Forwarded-Port "";
162+ proxy_set_header X-Forwarded-Scheme "";
163+
164 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";
165 proxy_force_ranges on;
166 proxy_cache 1d780969d5c7-cache;
167diff --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
168index 5e324e8..e21326b 100644
169--- a/tests/unit/files/nginx_config_rendered_test_output-site8.local.txt
170+++ b/tests/unit/files/nginx_config_rendered_test_output-site8.local.txt
171@@ -10,6 +10,12 @@ server {
172 location / {
173 proxy_pass http://localhost:8087;
174 proxy_set_header Host "site8.local";
175+ # Removed the following headers to avoid cache poisoning.
176+ proxy_set_header Forwarded "";
177+ proxy_set_header X-Forwarded-Host "";
178+ proxy_set_header X-Forwarded-Port "";
179+ proxy_set_header X-Forwarded-Scheme "";
180+
181 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";
182 proxy_force_ranges on;
183 proxy_cache dcd95df02bf2-cache;
184@@ -25,6 +31,12 @@ server {
185 location /auth {
186 proxy_pass http://localhost:8088;
187 proxy_set_header Host "site8.local";
188+ # Removed the following headers to avoid cache poisoning.
189+ proxy_set_header Forwarded "";
190+ proxy_set_header X-Forwarded-Host "";
191+ proxy_set_header X-Forwarded-Port "";
192+ proxy_set_header X-Forwarded-Scheme "";
193+
194 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";
195 proxy_force_ranges on;
196 proxy_cache dcd95df02bf2-cache;
197diff --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
198index 80dbdfa..44d8b57 100644
199--- a/tests/unit/files/nginx_config_rendered_test_output-site9.local.txt
200+++ b/tests/unit/files/nginx_config_rendered_test_output-site9.local.txt
201@@ -10,6 +10,12 @@ server {
202 location / {
203 proxy_pass http://localhost:8089;
204 proxy_set_header Host "site9.local";
205+ # Removed the following headers to avoid cache poisoning.
206+ proxy_set_header Forwarded "";
207+ proxy_set_header X-Forwarded-Host "";
208+ proxy_set_header X-Forwarded-Port "";
209+ proxy_set_header X-Forwarded-Scheme "";
210+
211 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";
212 proxy_force_ranges off;
213 proxy_cache 8facdb232d5b-cache;
214diff --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
215index 94fbf13..ffd7b0b 100644
216--- a/tests/unit/files/nginx_config_rendered_test_output-token_site.txt
217+++ b/tests/unit/files/nginx_config_rendered_test_output-token_site.txt
218@@ -10,6 +10,12 @@ server {
219 location / {
220 proxy_pass http://localhost:8080;
221 proxy_set_header Host "token_site";
222+ # Removed the following headers to avoid cache poisoning.
223+ proxy_set_header Forwarded "";
224+ proxy_set_header X-Forwarded-Host "";
225+ proxy_set_header X-Forwarded-Port "";
226+ proxy_set_header X-Forwarded-Scheme "";
227+
228 add_header X-Cache-Status "$upstream_cache_status from mock-content-cache/0";
229 proxy_force_ranges on;
230 proxy_cache c27ba6753fee-cache;

Subscribers

People subscribed via source and target branches