1. Patchew
  2. Patchew
  3. View series

[patchew-devel] [PATCH] use X-Forwarded-Host header in HttpRequest.get_host()/get_port()

Paolo Bonzini posted 1 patch 6 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/patchew-ci tags/patchew/20180228124135.586-1-pbonzini@redhat.com
patchew/settings.py               |  2 ++
scripts/patchew-server.nginx.conf |  1 +
tests/test_rest.py                | 10 ++++++++++
3 files changed, 13 insertions(+)
[patchew-devel] [PATCH] use X-Forwarded-Host header in HttpRequest.get_host()/get_port()
Posted by Paolo Bonzini 6 years, 1 month ago 
This fixes absolute URIs in the REST API.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
	Fam, can you please apply this patch and re-deploy next.patchew.org?

 patchew/settings.py               |  2 ++
 scripts/patchew-server.nginx.conf |  1 +
 tests/test_rest.py                | 10 ++++++++++
 3 files changed, 13 insertions(+)

diff --git a/patchew/settings.py b/patchew/settings.py
index cca6c79..b5b8019 100644
--- a/patchew/settings.py
+++ b/patchew/settings.py
@@ -113,6 +113,8 @@ def env_detect():
         raise Exception("Unknown running environment")
 
 DEBUG, DATA_DIR = env_detect()
+
+USE_X_FORWARDED_HOST = True
 if DEBUG:
     ALLOWED_HOSTS = ["*"]
 else:
diff --git a/scripts/patchew-server.nginx.conf b/scripts/patchew-server.nginx.conf
index d87d8b9..890a747 100644
--- a/scripts/patchew-server.nginx.conf
+++ b/scripts/patchew-server.nginx.conf
@@ -64,6 +64,7 @@ http {
 
     location @proxy_to_app {
       proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
+      proxy_set_header X-Forwarded-Host $host;
       # enable this if and only if you use HTTPS
       # proxy_set_header X-Forwarded-Proto https;
       proxy_set_header Host localhost;
diff --git a/tests/test_rest.py b/tests/test_rest.py
index 0e43797..018a97f 100755
--- a/tests/test_rest.py
+++ b/tests/test_rest.py
@@ -36,6 +36,16 @@ class RestTest(PatchewTestCase):
         self.admin = User.objects.get(username='admin')
         self.USER_BASE = '%susers/%d/' % (self.REST_BASE, self.admin.id)
 
+    def test_root(self):
+        resp = self.api_client.get(self.REST_BASE)
+        self.assertEquals(resp.data['users'], self.REST_BASE + 'users/')
+        self.assertEquals(resp.data['projects'], self.REST_BASE + 'projects/')
+        self.assertEquals(resp.data['series'], self.REST_BASE + 'series/')
+        resp = self.api_client.get(self.REST_BASE, HTTP_X_FORWARDED_HOST='patchew.org')
+        self.assertEquals(resp.data['users'], 'http://patchew.org/api/v1/users/')
+        self.assertEquals(resp.data['projects'], 'http://patchew.org/api/v1/projects/')
+        self.assertEquals(resp.data['series'], 'http://patchew.org/api/v1/series/')
+
     def test_users(self):
         resp = self.api_client.get(self.REST_BASE + 'users/')
         self.assertEquals(resp.data['count'], 1)
-- 
2.14.3
[patchew-devel] Re: [PATCH] use X-Forwarded-Host header in HttpRequest.get_host()/get_port()
Posted by Fam Zheng 6 years, 1 month ago 
On Wed, 02/28 13:41, Paolo Bonzini wrote:
> This fixes absolute URIs in the REST API.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> 	Fam, can you please apply this patch and re-deploy next.patchew.org?

With the other two changes on top, next.patchew.org is now working.

Fam
[patchew-devel] Re: [PATCH] use X-Forwarded-Host header in HttpRequest.get_host()/get_port()
Posted by Fam Zheng 6 years, 1 month ago 
On Wed, 02/28 13:41, Paolo Bonzini wrote:
> This fixes absolute URIs in the REST API.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> 	Fam, can you please apply this patch and re-deploy next.patchew.org?
> 
>  patchew/settings.py               |  2 ++
>  scripts/patchew-server.nginx.conf |  1 +
>  tests/test_rest.py                | 10 ++++++++++
>  3 files changed, 13 insertions(+)
> 
> diff --git a/patchew/settings.py b/patchew/settings.py
> index cca6c79..b5b8019 100644
> --- a/patchew/settings.py
> +++ b/patchew/settings.py
> @@ -113,6 +113,8 @@ def env_detect():
>          raise Exception("Unknown running environment")
>  
>  DEBUG, DATA_DIR = env_detect()
> +
> +USE_X_FORWARDED_HOST = True
>  if DEBUG:
>      ALLOWED_HOSTS = ["*"]
>  else:

I find two more things need to touch in addition. One is ALLOWED_HOSTS: what we
expect from X-Forwarded-Host must be listed there; the other is the host nginx
(the one that faces internet) must set Host header.

For the former, I wonder what are the risks to unconditionally do

    ALLOWED_HOSTS = ["*"]

since we run in a container behind a proxy that is associated to only one domain
name.

> diff --git a/scripts/patchew-server.nginx.conf b/scripts/patchew-server.nginx.conf
> index d87d8b9..890a747 100644
> --- a/scripts/patchew-server.nginx.conf
> +++ b/scripts/patchew-server.nginx.conf
> @@ -64,6 +64,7 @@ http {
>  
>      location @proxy_to_app {
>        proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
> +      proxy_set_header X-Forwarded-Host $host;
>        # enable this if and only if you use HTTPS
>        # proxy_set_header X-Forwarded-Proto https;
>        proxy_set_header Host localhost;

Actually, why not just change this to:

         proxy_set_header Host            $host;
?

> diff --git a/tests/test_rest.py b/tests/test_rest.py
> index 0e43797..018a97f 100755
> --- a/tests/test_rest.py
> +++ b/tests/test_rest.py
> @@ -36,6 +36,16 @@ class RestTest(PatchewTestCase):
>          self.admin = User.objects.get(username='admin')
>          self.USER_BASE = '%susers/%d/' % (self.REST_BASE, self.admin.id)
>  
> +    def test_root(self):
> +        resp = self.api_client.get(self.REST_BASE)
> +        self.assertEquals(resp.data['users'], self.REST_BASE + 'users/')
> +        self.assertEquals(resp.data['projects'], self.REST_BASE + 'projects/')
> +        self.assertEquals(resp.data['series'], self.REST_BASE + 'series/')
> +        resp = self.api_client.get(self.REST_BASE, HTTP_X_FORWARDED_HOST='patchew.org')
> +        self.assertEquals(resp.data['users'], 'http://patchew.org/api/v1/users/')
> +        self.assertEquals(resp.data['projects'], 'http://patchew.org/api/v1/projects/')
> +        self.assertEquals(resp.data['series'], 'http://patchew.org/api/v1/series/')
> +
>      def test_users(self):
>          resp = self.api_client.get(self.REST_BASE + 'users/')
>          self.assertEquals(resp.data['count'], 1)
> -- 
> 2.14.3
>
[patchew-devel] Re: [PATCH] use X-Forwarded-Host header in HttpRequest.get_host()/get_port()
Posted by Paolo Bonzini 6 years, 1 month ago 
On 28/02/2018 16:13, Fam Zheng wrote:
> For the former, I wonder what are the risks to unconditionally do
> 
>     ALLOWED_HOSTS = ["*"]

Django documentation says that "a fake Host value can be used for
Cross-Site Request Forgery, cache poisoning attacks, and poisoning links
in emails".

But I agree that it's okay for Patchew to set ALLOWED_HOSTS = ["*"],
with a comment that points to server_name in patchew-server.nginx.conf
as the right place for configuring patchew.

>>        proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
>> +      proxy_set_header X-Forwarded-Host $host;
>>        # enable this if and only if you use HTTPS
>>        # proxy_set_header X-Forwarded-Proto https;
>>        proxy_set_header Host localhost;
> Actually, why not just change this to:
> 
>          proxy_set_header Host            $host;

That probably works too.

Paolo

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.