Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Audit, clean up, and annotate Apache configs #5797

Merged
merged 7 commits into from
Feb 23, 2021
Merged

Audit, clean up, and annotate Apache configs #5797

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f7844a9
Audit, clean up, and annotate Apache configs
Feb 8, 2021
1a23782
Updates Apache config tests
Feb 22, 2021
a508f3d
Allows access to the mod_wsgi files for source and journalist app
kushaldas Feb 23, 2021
3c698b4
Ensures JI have DELETE capability
Feb 23, 2021
70de127
Trims down focal cipher list
Feb 23, 2021
ae28d9e
Forces headers in Apache config
Feb 23, 2021
c3d4626
Makes apache header tests distro-specific
Feb 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 6 additions & 18 deletions install_files/ansible-base/roles/app/defaults/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,29 +30,17 @@ securedrop_app_https_certificate_cert_src: securedrop_source_onion.crt
securedrop_app_https_certificate_key_src: securedrop_source_onion.key
securedrop_app_https_certificate_chain_src: DigiCertCA.crt

# List of SSL ciphers honored by Source Interface vhost. Order matters!
# The `SSLHonorCipherOrder` option is set to true, so ciphers below are
# listed in order of preference.
securedrop_app_https_ssl_ciphers:
- ECDHE-ECDSA-AES256-GCM-SHA384
- ECDHE-RSA-AES256-GCM-SHA384
- ECDHE-ECDSA-CHACHA20-POLY1305
- ECDHE-RSA-CHACHA20-POLY1305
- ECDHE-ECDSA-AES128-GCM-SHA256
- ECDHE-RSA-AES128-GCM-SHA256
- ECDHE-ECDSA-AES256-SHA384
- ECDHE-RSA-AES256-SHA384
- ECDHE-ECDSA-AES128-SHA256
- ECDHE-RSA-AES128-SHA256

apache_packages:
- apache2
- libapache2-mod-xsendfile

apache_templates:
- ports.conf
- sites-available/journalist.conf
- sites-available/source.conf
- src: ports.conf
dest: /etc/apache2/ports.conf
- src: sites-available/{{ ansible_distribution_release }}/journalist.conf
dest: /etc/apache2/sites-available/journalist.conf
- src: sites-available/{{ ansible_distribution_release }}/source.conf
dest: /etc/apache2/sites-available/source.conf

# Apache modules required for the SecureDrop application. Will be enabled.
apache_modules:
Expand Down
6 changes: 3 additions & 3 deletions install_files/ansible-base/roles/app/tasks/install_and_harden_apache.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
- name: Install apache packages.
apt:
pkg: "{{ apache_packages }}"
state: latest
state: present
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed this to state=present to avoid yet another apt-get update. Earlier in the playbook run, all packages are updated to via apt safe-upgrade (dist-upgrade, as of #5793), so there's no need to update the lists again.

update_cache: yes
cache_valid_time: 3600
tags:
Expand Down Expand Up @@ -32,8 +32,8 @@

- name: Copy Apache ports and site configs.
template:
src: "{{ item }}"
dest: /etc/apache2/{{ item }}
src: "{{ item.src }}"
dest: "{{ item.dest }}"
owner: root
mode: "0644"
with_items: "{{ apache_templates }}"
Expand Down
2 changes: 2 additions & 0 deletions install_files/ansible-base/roles/app/tasks/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
---
- include_vars: "{{ ansible_distribution }}_{{ ansible_distribution_release }}.yml"

- include: app_install_fpf_deb_pkgs.yml
when: securedrop_app_install_from_repo

Expand Down
88 changes: 88 additions & 0 deletions install_files/ansible-base/roles/app/templates/sites-available/focal/journalist.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
ServerName {{ securedrop_app_apache_listening_address }}
<VirtualHost {{ securedrop_app_apache_listening_address }}:8080>

# WSGI settings for Flask app for Source Interface
WSGIDaemonProcess journalist processes=2 threads=30 display-name=%{GROUP} python-path=/var/www/securedrop
WSGIScriptAlias / /var/www/journalist.wsgi process-group=journalist application-group=journalist
WSGIPassAuthorization On

# Tell the browser not to cache HTML responses in order to minimize the chance
# of the inadvertent release or retention of sensitive data. For more, see
# https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.9.2.
Header set Cache-Control "no-store"

# Configure X-Sendfile for more efficient large file downloads.
# Modern versions of WSGI wrapper may make this obsolete, more
# research required.
XSendFile On
XSendFilePath /var/lib/securedrop/store/
XSendFilePath /var/lib/securedrop/tmp/
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a comment saying that XSendFile may not actually be required these days. Would prefer to punt on functionality changes now, pending further testing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More recently than this comment, I feel pretty strongly we should keep XSendFile, at least as long as we're using Apache. So, will clarify in the comment here.


Header edit Set-Cookie ^(.*)$ $1;HttpOnly
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flask will handle this in it's default configuration[1], and this setting can be set/overrided in the Flask config as well. It might be worth removing here to simplify, but I also see no harm in applying this here.

[1] https://flask.palletsprojects.com/en/1.1.x/api/?#flask.Flask.default_config

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I'd prefer to leverage flask as much as possible (static config issues aside), this strikes me as useful defense. Besides, if we ever want to change it, it's simple enough to snip out in postinst when updating the Flask config.


Header onsuccess unset X-Frame-Options
Header always set X-Frame-Options "DENY"
Header onsuccess unset Referrer-Policy
Header always set Referrer-Policy "no-referrer"
Header onsuccess unset X-XSS-Protection
Header always set X-XSS-Protection "1; mode=block"

Header onsuccess unset X-Content-Type-Options
Header always set X-Content-Type-Options "nosniff"
Header onsuccess unset X-Download-Options
Header always set X-Download-Options "noopen"
Header onsuccess unset Content-Security-Policy
Header always set Content-Security-Policy "default-src 'none'; script-src 'self'; style-src 'self'; img-src 'self'; font-src 'self';"

# Limit the max submitted size of requests.
LimitRequestBody 524288000

# Set default to deny all access from all filepaths.
<Directory />
Options None
AllowOverride None
Require all denied
emkll marked this conversation as resolved.
Show resolved Hide resolved
</Directory>

# Permit limited access specifically to the SecureDrop wsgi files.
<Directory /var/www/>
Options None
AllowOverride None
<Limit GET POST HEAD DELETE>
Require ip {{ securedrop_app_apache_allow_from }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if a Require all denied is needed here too. In the absence of RequireAny/RequierAll tags, it will default to RequireAny[1] which should be the behavior that we want

[1] https://httpd.apache.org/docs/2.4/upgrading.html

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's just weird to not see a deny-by-default rule here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Relevant docs here:

Worth being explicit about the default states, will add.

</Limit>
<LimitExcept GET POST HEAD DELETE>
Require all denied
</LimitExcept>
</Directory>

# Permit limited access specifically to the SecureDrop application directory.
<Directory /var/www/securedrop>
Options None
AllowOverride None
<Limit GET POST HEAD DELETE>
Require ip {{ securedrop_app_apache_allow_from }}
</Limit>
<LimitExcept GET POST HEAD DELETE>
Require all denied
</LimitExcept>
</Directory>

Alias /static /var/www/securedrop/static
<Directory /var/www/securedrop/static>
Require all granted
# Cache static resources for 1 hour
Header set Cache-Control "max-age=3600"
</Directory>

# Deny all non-HTTP traffic, as a precaution
RewriteEngine On
RewriteCond %{THE_REQUEST} !HTTP/1\.1$
RewriteRule .* - [F]

# Configure logging for Journalist Interface
ErrorLog /var/log/apache2/journalist-error.log
CustomLog /var/log/apache2/journalist-access.log combined
LogLevel info

</VirtualHost>
116 changes: 116 additions & 0 deletions install_files/ansible-base/roles/app/templates/sites-available/focal/source.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
ServerName {{ securedrop_app_apache_listening_address }}
<VirtualHost {{ securedrop_app_apache_listening_address }}:80>
{% if securedrop_app_https_on_source_interface %}
# Optional HTTPS settings for Source Interface. Requires opt-in
# via 'securedrop-admin sdconfig', as well as an active certificate.
# If enabled, all HTTP connections will be redirected to HTTPS.
RewriteEngine On
RewriteCond %{HTTPS} off
RewriteRule (.*) https://%{HTTP_HOST}%{REQUEST_URI}
</VirtualHost>

<VirtualHost 127.0.0.1:443>
# HTTPS config
SSLEngine on
SSLCertificateFile /var/lib/ssl/{{ securedrop_app_https_certificate_cert_src|basename }}
SSLCertificateKeyFile /var/lib/ssl/{{ securedrop_app_https_certificate_key_src|basename }}
SSLCertificateChainFile /var/lib/ssl/{{ securedrop_app_https_certificate_chain_src|basename }}

# Evaluate support for TLSv1.3 in Tor Browser for Onions, conservatively
# we'll continue to support TLSv1.2 for now.
SSLProtocol all -SSLv3 -TLSv1 -TLSv1.1
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TLSv1.3 support deserves a spike, but I don't see it as a release-blocker. Probably worth its own issue, though. cc @emkll

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, a good topic for follow-up. But right now, we only support TLS 1.2 which remains safe. TLS1.3 only would vastly improve cipher suite configuration however, see below.

SSLCipherSuite {{ securedrop_app_https_ssl_ciphers|join(':') }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should note here that Tor Onion Services provide transport layer encryption and server authentication, similar to TLS. HTTPS is used as defense-in-depth here. However:

A modern Cipher suite is very simply to allow TLS 1.3 only. In the absence of TLS 1.3 support (which will be tracked per the above), Mozilla recommends[1]:
In other words, we should consider removing the following cipher suites:

  - ECDHE-ECDSA-AES256-SHA384
  - ECDHE-RSA-AES256-SHA384
  - ECDHE-ECDSA-AES128-SHA256
  - ECDHE-RSA-AES128-SHA256

[1] https://ssl-config.mozilla.org/#server=apache&version=2.4.41&config=intermediate&openssl=1.1.1d&guideline=5.6

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can make the ciphers distro-specific, will add

SSLHonorCipherOrder on
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All our cipher suites should be strong, so this directive should not be required, but also fine to keep as-is

SSLCompression off
{% endif %}

# WSGI settings for Flask app for Source Interface
WSGIDaemonProcess source processes=2 threads=30 display-name=%{GROUP} python-path=/var/www/securedrop
WSGIProcessGroup source
WSGIScriptAlias / /var/www/source.wsgi

# Tell the browser not to cache HTML responses in order to minimize the chance
# of the inadvertent release or retention of sensitive data. For more, see
# https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.9.2.
Header set Cache-Control "no-store"

XSendFile Off

# Prevent cookies from being accessed from Javascript. XSS mitigation.
Header edit Set-Cookie ^(.*)$ $1;HttpOnly
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as JI, this is flask controlled/default


# Don't allow SecureDrop to be framed. Clickjacking mitigation.
Header onsuccess unset X-Frame-Options
Header always set X-Frame-Options "DENY"
Header onsuccess unset Referrer-Policy
Header always set Referrer-Policy "same-origin"
Header onsuccess unset X-XSS-Protection
Header always set X-XSS-Protection "1; mode=block"

# Set a strict CSP; "default-src 'self'" prevents 3rd party subresources from
# loading and prevents inline script from executing.
Header onsuccess unset Content-Security-Policy
Header always set Content-Security-Policy "default-src 'none'; script-src 'self'; style-src 'self'; img-src 'self'; font-src 'self';"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can also set frame-ancestors to none here: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/frame-ancestors

Header onsuccess unset X-Download-Options
Header always set X-Download-Options "noopen"
Header onsuccess unset X-Content-Type-Options
Header always set X-Content-Type-Options "nosniff"

Header unset Etag

# Limit the max submitted size of requests to help prevent DoS.
LimitRequestBody 524288000

# Set default to deny all access from all filepaths.
<Directory />
Options None
AllowOverride None
Require all denied
</Directory>

# Permit limited access specifically to the SecureDrop wsgi files.
<Directory /var/www/>
Options None
AllowOverride None
<Limit GET POST HEAD>
Require ip {{ securedrop_app_apache_allow_from }}
</Limit>
<LimitExcept GET POST HEAD>
Require all denied
</LimitExcept>
</Directory>

# Permit limited access specifically to the SecureDrop application directory.
<Directory /var/www/securedrop>
Options None
AllowOverride None
<Limit GET POST HEAD>
Require ip {{ securedrop_app_apache_allow_from }}
emkll marked this conversation as resolved.
Show resolved Hide resolved
</Limit>
<LimitExcept GET POST HEAD>
Require all denied
</LimitExcept>
</Directory>

# Allow dropping txt files here for onion server validation
Alias /.well-known/pki-validation /var/www/securedrop/.well-known/pki-validation
<Directory /var/www/securedrop/.well-known/pki-validation>
Require all granted
</Directory>

Alias /static /var/www/securedrop/static
<Directory /var/www/securedrop/static>
Require all granted
# Cache static resources for 1 hour
Header set Cache-Control "max-age=3600"
</Directory>

# Deny all non-HTTP traffic, as a precaution
RewriteEngine On
RewriteCond %{THE_REQUEST} !HTTP/1\.1$
RewriteRule .* - [F]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't actually test these lines, but I added a comment describing their intent. It would be nice to test these types of mitigations functionally, but fell back to providing comments to clarify.


ErrorLog {{ source_apache_log_location | default('/dev/null') }}
LogLevel {{ apache_logging_level | default('crit') }}

</VirtualHost>
0 ...templates/sites-available/journalist.conf → ...es/sites-available/xenial/journalist.conf
View file
Open in desktop
File renamed without changes.
0 ...app/templates/sites-available/source.conf → ...plates/sites-available/xenial/source.conf
View file
Open in desktop
File renamed without changes.
9 changes: 9 additions & 0 deletions install_files/ansible-base/roles/app/vars/Ubuntu_focal.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
# List of SSL ciphers honored by Source Interface vhost. Order matters!
# The `SSLHonorCipherOrder` option is set to true, so ciphers below are
# listed in order of preference.
securedrop_app_https_ssl_ciphers:
- ECDHE-ECDSA-AES256-GCM-SHA384
- ECDHE-RSA-AES256-GCM-SHA384
- ECDHE-ECDSA-AES128-GCM-SHA256
- ECDHE-RSA-AES128-GCM-SHA256
15 changes: 15 additions & 0 deletions install_files/ansible-base/roles/app/vars/Ubuntu_xenial.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
# List of SSL ciphers honored by Source Interface vhost. Order matters!
# The `SSLHonorCipherOrder` option is set to true, so ciphers below are
# listed in order of preference.
securedrop_app_https_ssl_ciphers:
- ECDHE-ECDSA-AES256-GCM-SHA384
- ECDHE-RSA-AES256-GCM-SHA384
- ECDHE-ECDSA-CHACHA20-POLY1305
- ECDHE-RSA-CHACHA20-POLY1305
- ECDHE-ECDSA-AES128-GCM-SHA256
- ECDHE-RSA-AES128-GCM-SHA256
- ECDHE-ECDSA-AES256-SHA384
- ECDHE-RSA-AES256-SHA384
- ECDHE-ECDSA-AES128-SHA256
- ECDHE-RSA-AES128-SHA256
Loading