-
Notifications
You must be signed in to change notification settings - Fork 107
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Auto merge of #375 - aneeshusa:add-basic-cors-headers, r=<try>
Use Nginx to make Buildbot logs available via CORS Buildbot itself (neither the current version eight nor the upcoming verison nine) does not seem to have built-in CORS support, so use Nginx to supply CORS headers while reverse proxying. This is only needed to download log files, so start with a very conservative set of CORS headers. In particular: - Only support basic CORS requests (those that don't require preflight requests via OPTIONS). This means supporting only GET requests. - Do not allow sending cookies or exposing any headers. In return, whitelist all domains for CORS, and send the Allow-Origin header unconditionally, as the actual value of the Origin header does not matter. Because we are using proxy_pass to reverse proxy Buildbot, we need to tell Nginx to always add the CORS headers, instead of only adding them for successful responses. (I.e. add CORS headers even for 404s and other erroneous status codes). This features is not available in the Nginx packaged for Ubuntu Trusty, so replace it with a newer Nginx from upstream Nginx's respositories. Because Debian/Ubuntu modify the default configuration for Nginx, use Salt to ensure that the installed nginx.conf properly looks into the sites-enabled directory (which is a Debian-ism) and that the default handler in /etc/nginx/conf.d/ is purged. In order to only support CORS on GET requests, use the 'if' directive. However, because Nginx's if is evil, (https://www.nginx.com/resources/wiki/start/topics/depth/ifisevil/) add a bunch of tests to ensure that the configuration is working. Also, update the Nginx states to follow Salt best practices. Fixes #354. Blocked on #374 because I encountered various failures while upgrading from the Ubuntu Nginx to upstream Nginx, and I'd like to make sure I caught them all. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/375) <!-- Reviewable:end -->
- Loading branch information
Showing
9 changed files
with
255 additions
and
25 deletions.
There are no files selected for viewing
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
server { | ||
listen 80 default_server; | ||
server_name build.servo.org; | ||
|
||
|
||
location / { | ||
proxy_pass http://localhost:8010/; | ||
|
||
# Enable CORS for basic (GET-only without OPTIONS preflight) requests | ||
if ($request_method = 'GET') { | ||
add_header 'Access-Control-Allow-Origin' '*' always; | ||
# Do not set Access-Control-Allow-Credentials | ||
# Do not set Access-Control-Expose-Headers | ||
} | ||
} | ||
|
||
location /homu/ { | ||
proxy_pass http://localhost:54856/; | ||
} | ||
} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
user {{ nginx.user }}; | ||
worker_processes 4; | ||
pid /run/nginx.pid; | ||
|
||
events { | ||
worker_connections 768; | ||
# multi_accept on; | ||
} | ||
|
||
http { | ||
|
||
## | ||
# Basic Settings | ||
## | ||
|
||
sendfile on; | ||
tcp_nopush on; | ||
tcp_nodelay on; | ||
keepalive_timeout 65; | ||
types_hash_max_size 2048; | ||
# server_tokens off; | ||
|
||
# server_names_hash_bucket_size 64; | ||
# server_name_in_redirect off; | ||
|
||
include /etc/nginx/mime.types; | ||
default_type application/octet-stream; | ||
|
||
## | ||
# Logging Settings | ||
## | ||
|
||
access_log /var/log/nginx/access.log; | ||
error_log /var/log/nginx/error.log; | ||
|
||
## | ||
# Gzip Settings | ||
## | ||
|
||
gzip on; | ||
gzip_disable "msie6"; | ||
|
||
# gzip_vary on; | ||
# gzip_proxied any; | ||
# gzip_comp_level 6; | ||
# gzip_buffers 16 8k; | ||
# gzip_http_version 1.1; | ||
# gzip_types text/plain text/css application/json application/x-javascript text/xml application/xml application/xml+rss text/javascript; | ||
|
||
## | ||
# Virtual Host Configs | ||
## | ||
|
||
include /etc/nginx/conf.d/*.conf; | ||
include /etc/nginx/sites-enabled/*; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
-----BEGIN PGP PUBLIC KEY BLOCK----- | ||
Version: GnuPG v1.4.11 (FreeBSD) | ||
|
||
mQENBE5OMmIBCAD+FPYKGriGGf7NqwKfWC83cBV01gabgVWQmZbMcFzeW+hMsgxH | ||
W6iimD0RsfZ9oEbfJCPG0CRSZ7ppq5pKamYs2+EJ8Q2ysOFHHwpGrA2C8zyNAs4I | ||
QxnZZIbETgcSwFtDun0XiqPwPZgyuXVm9PAbLZRbfBzm8wR/3SWygqZBBLdQk5TE | ||
fDR+Eny/M1RVR4xClECONF9UBB2ejFdI1LD45APbP2hsN/piFByU1t7yK2gpFyRt | ||
97WzGHn9MV5/TL7AmRPM4pcr3JacmtCnxXeCZ8nLqedoSuHFuhwyDnlAbu8I16O5 | ||
XRrfzhrHRJFM1JnIiGmzZi6zBvH0ItfyX6ttABEBAAG0KW5naW54IHNpZ25pbmcg | ||
a2V5IDxzaWduaW5nLWtleUBuZ2lueC5jb20+iQE+BBMBAgAoBQJOTjJiAhsDBQkJ | ||
ZgGABgsJCAcDAgYVCAIJCgsEFgIDAQIeAQIXgAAKCRCr9b2Ce9m/YpvjB/98uV4t | ||
94d0oEh5XlqEZzVMrcTgPQ3BZt05N5xVuYaglv7OQtdlErMXmRWaFZEqDaMHdniC | ||
sF63jWMd29vC4xpzIfmsLK3ce9oYo4t9o4WWqBUdf0Ff1LMz1dfLG2HDtKPfYg3C | ||
8NESud09zuP5NohaE8Qzj/4p6rWDiRpuZ++4fnL3Dt3N6jXILwr/TM/Ma7jvaXGP | ||
DO3kzm4dNKp5b5bn2nT2QWLPnEKxvOg5Zoej8l9+KFsUnXoWoYCkMQ2QTpZQFNwF | ||
xwJGoAz8K3PwVPUrIL6b1lsiNovDgcgP0eDgzvwLynWKBPkRRjtgmWLoeaS9FAZV | ||
ccXJMmANXJFuCf26iQEcBBABAgAGBQJOTkelAAoJEKZP1bF62zmo79oH/1XDb29S | ||
YtWp+MTJTPFEwlWRiyRuDXy3wBd/BpwBRIWfWzMs1gnCjNjk0EVBVGa2grvy9Jtx | ||
JKMd6l/PWXVucSt+U/+GO8rBkw14SdhqxaS2l14v6gyMeUrSbY3XfToGfwHC4sa/ | ||
Thn8X4jFaQ2XN5dAIzJGU1s5JA0tjEzUwCnmrKmyMlXZaoQVrmORGjCuH0I0aAFk | ||
RS0UtnB9HPpxhGVbs24xXZQnZDNbUQeulFxS4uP3OLDBAeCHl+v4t/uotIad8v6J | ||
SO93vc1evIje6lguE81HHmJn9noxPItvOvSMb2yPsE8mH4cJHRTFNSEhPW6ghmlf | ||
Wa9ZwiVX5igxcvaIRgQQEQIABgUCTk5b0gAKCRDs8OkLLBcgg1G+AKCnacLb/+W6 | ||
cflirUIExgZdUJqoogCeNPVwXiHEIVqithAM1pdY/gcaQZmIRgQQEQIABgUCTk5f | ||
YQAKCRCpN2E5pSTFPnNWAJ9gUozyiS+9jf2rJvqmJSeWuCgVRwCcCUFhXRCpQO2Y | ||
Va3l3WuB+rgKjsQ= | ||
=A015 | ||
-----END PGP PUBLIC KEY BLOCK----- |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,20 +1,74 @@ | ||
{% from tpldir ~ '/map.jinja' import nginx %} | ||
# Fix conflicts if Nginx from Ubuntu is previously installed | ||
nginx-distro-pkgs: | ||
pkg.purged: | ||
- pkgs: | ||
- nginx-common | ||
- nginx-core | ||
nginx: | ||
pkg.installed: [] | ||
pkgrepo.managed: | ||
- name: 'deb http://nginx.org/packages/ubuntu/ trusty nginx' | ||
- file: /etc/apt/sources.list.d/nginx.list | ||
# Available online but not via HTTPS, so serve locally instead | ||
- key_url: salt://{{ tpldir }}/files/nginx_signing.key | ||
pkg.installed: | ||
- name: {{ nginx.pkg }} | ||
- version: {{ nginx.version }} | ||
- require: | ||
- pkg: nginx-distro-pkgs | ||
- pkgrepo: nginx | ||
service.running: | ||
- enable: True | ||
- watch: | ||
- pkg: nginx | ||
- file: /etc/nginx/sites-enabled/default | ||
- file: /etc/nginx/conf.d | ||
- file: /etc/nginx/nginx.conf | ||
/etc/apt/sources.list.d/nginx.list: | ||
file.exists: | ||
- require: | ||
- pkgrepo: nginx | ||
- require_in: | ||
- file: /etc/apt/sources.list.d | ||
/etc/nginx/sites-available/default: | ||
file.managed: | ||
- source: salt://nginx/default | ||
- source: salt://{{ tpldir }}/files/default | ||
- user: root | ||
- group: root | ||
- mode: 644 | ||
- watch_in: | ||
- service: nginx | ||
- makedirs: True | ||
- require: | ||
- pkg: nginx | ||
/etc/nginx/sites-enabled/default: | ||
file.symlink: | ||
- target: /etc/nginx/sites-available/default | ||
- makedirs: True | ||
- require: | ||
- file: /etc/nginx/sites-available/default | ||
- pkg: nginx | ||
/etc/nginx/conf.d: | ||
file.directory: | ||
- user: root | ||
- group: root | ||
- clean: True | ||
- file_mode: 644 | ||
- dir_mode: 755 | ||
- require: | ||
- pkg: nginx | ||
/etc/nginx/nginx.conf: | ||
file.managed: | ||
- user: root | ||
- group: root | ||
- source: salt://{{ tpldir }}/files/nginx.conf | ||
- template: jinja | ||
- context: | ||
nginx: {{ nginx }} | ||
- require: | ||
- pkg: nginx |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
{% | ||
set nginx = salt.grains.filter_by({ | ||
'Ubuntu': { | ||
'pkg': 'nginx', | ||
'version': '1.10.0-1~trusty', | ||
'user': 'www-data', | ||
}, | ||
}, | ||
base='defaults', | ||
merge=salt.pillar.get('nginx', {}), | ||
grain='os' | ||
) | ||
%} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
import urllib.request | ||
import urllib.error | ||
|
||
from tests.util import Failure, Success, format_nested_failure | ||
|
||
|
||
def format_header(response, header): | ||
""" | ||
Precondition: header exists in response.headers | ||
""" | ||
return "{}: Access-Control-{}".format(header, response.headers[header]) | ||
|
||
|
||
def make_cors_request(url, **kwargs): | ||
try: | ||
cors_request = urllib.request.Request(url, headers={ | ||
'Origin': 'http://example.com', | ||
}, **kwargs) | ||
return urllib.request.urlopen(cors_request) | ||
except urllib.error.URLError as e: | ||
# Can read e.headers() if there was a response but the HTTP status code | ||
# indicated error; the method is unavailable if a connection could not | ||
# be made. Nginx is reverse proxying Homu and Buildbot, which will be | ||
# dead on Travis because the test pillar credentials are not valid. | ||
return e | ||
|
||
# No need to catch HTTPError or ContentTooShortError specially here | ||
|
||
|
||
def run(): | ||
url = 'http://localhost/' | ||
get = make_cors_request(url) | ||
post = make_cors_request(url, method='POST') | ||
|
||
if 'Access-Control-Allow-Origin' not in get.headers: | ||
return Failure("Nginx is not serving CORS headers:", str(get.headers)) | ||
|
||
failures = [] | ||
|
||
if 'Access-Control-Allow-Origin' in post.headers: | ||
failures += [Failure("CORS headers are served on non-GET requests", | ||
"")] | ||
|
||
if get.headers['Access-Control-Allow-Origin'] != '*': | ||
failures += [Failure("All origins are not whitelisted:", | ||
format_header(get, 'Allow-Origin'))] | ||
|
||
if 'Access-Control-Allow-Credentials' in get.headers: | ||
failures += [Failure("Cookies are allowed for CORS requests:", | ||
format_header(get, 'Allow-Credentials'))] | ||
|
||
if 'Access-Control-Expose-Headers' in get.headers: | ||
failures += [Failure("Headers are exposed on CORS requests:", | ||
format_header(get, 'Expose-Headers'))] | ||
|
||
if len(failures) > 0: | ||
return Failure("Nginx is serving incorrect CORS headers:", | ||
'\n'.join([format_nested_failure(f) for f in failures])) | ||
|
||
return Success("Nginx is serving CORS headers properly") |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters