From 5d93b5435637ca9cf5866e7ba9a3ebe4c984b28b Mon Sep 17 00:00:00 2001 From: Aneesh Agrawal Date: Sun, 15 May 2016 22:05:51 -0400 Subject: [PATCH] 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. --- nginx/default | 14 -------- nginx/files/default | 21 +++++++++++ nginx/files/nginx.conf | 56 +++++++++++++++++++++++++++++ nginx/files/nginx_signing.key | 28 +++++++++++++++ nginx/init.sls | 62 +++++++++++++++++++++++++++++--- nginx/map.jinja | 13 +++++++ test.py | 11 +++--- tests/sls/nginx/buildbot_cors.py | 60 +++++++++++++++++++++++++++++++ tests/util.py | 15 ++++++++ 9 files changed, 255 insertions(+), 25 deletions(-) delete mode 100644 nginx/default create mode 100644 nginx/files/default create mode 100644 nginx/files/nginx.conf create mode 100644 nginx/files/nginx_signing.key create mode 100644 nginx/map.jinja create mode 100644 tests/sls/nginx/buildbot_cors.py diff --git a/nginx/default b/nginx/default deleted file mode 100644 index 6cace48b..00000000 --- a/nginx/default +++ /dev/null @@ -1,14 +0,0 @@ -server { - listen 80 default_server; - server_name build.servo.org; - - - location / { - proxy_pass http://localhost:8010/; - } - - location /homu/ { - proxy_pass http://localhost:54856/; - } -} - diff --git a/nginx/files/default b/nginx/files/default new file mode 100644 index 00000000..aa2b625c --- /dev/null +++ b/nginx/files/default @@ -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/; + } +} + diff --git a/nginx/files/nginx.conf b/nginx/files/nginx.conf new file mode 100644 index 00000000..0a803593 --- /dev/null +++ b/nginx/files/nginx.conf @@ -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/*; +} diff --git a/nginx/files/nginx_signing.key b/nginx/files/nginx_signing.key new file mode 100644 index 00000000..7cb07689 --- /dev/null +++ b/nginx/files/nginx_signing.key @@ -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----- diff --git a/nginx/init.sls b/nginx/init.sls index 94ede988..1bc58ddb 100644 --- a/nginx/init.sls +++ b/nginx/init.sls @@ -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 diff --git a/nginx/map.jinja b/nginx/map.jinja new file mode 100644 index 00000000..2cb3fd79 --- /dev/null +++ b/nginx/map.jinja @@ -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' + ) +%} diff --git a/test.py b/test.py index 0a48f6f7..bd10d9af 100755 --- a/test.py +++ b/test.py @@ -4,7 +4,7 @@ import os import sys -from tests.util import color, GREEN, RED, Failure, project_path +from tests.util import Failure, project_path def is_python_script(dir_entry): @@ -32,13 +32,10 @@ def run_tests(tests): message = 'Test \'{}\' raised an exception:'.format(test) result = Failure(message, str(e)) - if result.is_success(): - print('[ {} ] {}'.format(color(GREEN, 'PASS'), result.message)) - else: + if result.is_failure(): any_failures = True - print('[ {} ] {}'.format(color(RED, 'FAIL'), result.message)) - for line in result.output.splitlines(): - print(' {}'.format(line)) + + print(result) return 1 if any_failures else 0 diff --git a/tests/sls/nginx/buildbot_cors.py b/tests/sls/nginx/buildbot_cors.py new file mode 100644 index 00000000..cdbae420 --- /dev/null +++ b/tests/sls/nginx/buildbot_cors.py @@ -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") diff --git a/tests/util.py b/tests/util.py index f726ba59..3d0111de 100644 --- a/tests/util.py +++ b/tests/util.py @@ -52,6 +52,9 @@ def is_success(self): def is_failure(self): return False + def __str__(self): + return '[ {} ] {}'.format(color(GREEN, 'PASS'), self.message) + class Failure(TestResult): def __init__(self, message, output): @@ -63,3 +66,15 @@ def is_success(self): def is_failure(self): return True + + def __str__(self): + output = ['[ {} ] {}'.format(color(RED, 'FAIL'), self.message)] + for line in self.output.splitlines(): + output.append(' {}'.format(line)) + return '\n'.join(output) + + +def format_nested_failure(failure): + output = ['- {}'.format(failure.message)] + output += [' {}'.format(line) for line in failure.output.splitlines()] + return '\n'.join(output)