From b9a9e06c1d7a370aee87bdca2237eb79c7e7890f Mon Sep 17 00:00:00 2001 From: DjLegolas Date: Sun, 24 Jun 2018 20:39:04 +0300 Subject: [PATCH] [WebUI][Daemon] Enhance TLS Security This applies the following for both WebUI and Daemon: 1. Raised minimal TLS version to TLSv1.2 2. Added specific cipher suite list 3. Added support for ECDSA auth keys 4. Added support for ECDHE key exchange algorithm We disabled the ability to perform TLS/SSL renegotiation and therefore will prevent the clients from renegotiating, which can be exploit for DoS attacks. New security tests now will be skipped when running `pydef` and `trial` testenvs. To run the test, use the testenv `security` or add the environment variable `SECURITY_TESTS` before running the tests. Also should only run when adding to the commit message the string `SECURITY_TEST`. --- .travis.yml | 28 ++++-- ChangeLog | 1 + deluge/core/rpcserver.py | 24 ++--- deluge/crypto_utils.py | 72 +++++++++++++ deluge/tests/test_security.py | 184 ++++++++++++++++++++++++++++++++++ deluge/ui/web/server.py | 26 ++--- tox.ini | 6 +- 7 files changed, 297 insertions(+), 44 deletions(-) create mode 100644 deluge/crypto_utils.py create mode 100644 deluge/tests/test_security.py diff --git a/.travis.yml b/.travis.yml index 45692583e..fc8e8d9f5 100644 --- a/.travis.yml +++ b/.travis.yml @@ -16,22 +16,30 @@ install: - bash -c "echo $APTPACKAGES" - sudo apt-get install $APTPACKAGES - pip install tox + # the next part is for the security tests only. + - wget https://github.com/drwetter/testssl.sh/archive/v2.9.5-5.tar.gz -O testssl.tar.gz + - tar -xvzf testssl.tar.gz + - mv -t deluge/tests/data testssl.sh-2.9.5-5/testssl.sh testssl.sh-2.9.5-5/etc/ env: global: - APTPACKAGES="python-libtorrent" - APTPACKAGES_GTKUI="python-gobject python-glade2" - DISPLAY=:99.0 - matrix: - - TOX_ENV=pydef - - TOX_ENV=flake8 -# - TOX_ENV=flake8-complexity - - TOX_ENV=docs -# - TOX_ENV=todo - - TOX_ENV=trial APTPACKAGES="$APTPACKAGES $APTPACKAGES_GTKUI" - - TOX_ENV=pygtkui APTPACKAGES="$APTPACKAGES $APTPACKAGES_GTKUI" -# - TOX_ENV=testcoverage APTPACKAGES="$APTPACKAGES $APTPACKAGES_GTKUI" - - TOX_ENV=plugins + +matrix: + include: + - env: TOX_ENV=pydef + - if: commit_message =~ SECURITY_TEST + env: TOX_ENV=security + - env: TOX_ENV=flake8 + #- env: TOX_ENV=flake8-complexity + - env: TOX_ENV=docs + #- env: TOX_ENV=todo + - env: TOX_ENV=trial APTPACKAGES="$APTPACKAGES $APTPACKAGES_GTKUI" + - env: TOX_ENV=pygtkui APTPACKAGES="$APTPACKAGES $APTPACKAGES_GTKUI" + #- env: TOX_ENV=testcoverage APTPACKAGES="$APTPACKAGES $APTPACKAGES_GTKUI" + - env: TOX_ENV=plugins virtualenv: system_site_packages: true diff --git a/ChangeLog b/ChangeLog index 3bedfc9d6..66f94e242 100644 --- a/ChangeLog +++ b/ChangeLog @@ -10,6 +10,7 @@ names clashing beetween regular packages and deluge plugins. * Fix potential for host_id collision when creating hostlist entries. * Add Option To Specify Outgoing Connection Interface. + * Updated SSL/TLS Protocol parameters for better security. ==== Core ==== * Make the distinction between adding to the session new unmanaged torrents diff --git a/deluge/core/rpcserver.py b/deluge/core/rpcserver.py index c8532475a..ccbb03d3d 100644 --- a/deluge/core/rpcserver.py +++ b/deluge/core/rpcserver.py @@ -18,13 +18,14 @@ import traceback from collections import namedtuple from types import FunctionType -from OpenSSL import SSL, crypto +from OpenSSL import crypto from twisted.internet import defer, reactor from twisted.internet.protocol import Factory, connectionDone import deluge.component as component import deluge.configmanager from deluge.core.authmanager import AUTH_LEVEL_ADMIN, AUTH_LEVEL_DEFAULT, AUTH_LEVEL_NONE +from deluge.crypto_utils import get_context_factory from deluge.error import DelugeError, IncompatibleClient, NotAuthorizedError, WrappedException, _ClientSideRecreateError from deluge.event import ClientDisconnectedEvent from deluge.transfer import DelugeTransferProtocol @@ -91,22 +92,6 @@ def format_request(call): return s -class ServerContextFactory(object): - def getContext(self): # NOQA: N802 - """ - Create an SSL context. - - This loads the servers cert/private key SSL files for use with the - SSL transport. - """ - ssl_dir = deluge.configmanager.get_config_dir('ssl') - ctx = SSL.Context(SSL.SSLv23_METHOD) - ctx.set_options(SSL.OP_NO_SSLv2 | SSL.OP_NO_SSLv3) - ctx.use_certificate_file(os.path.join(ssl_dir, 'daemon.cert')) - ctx.use_privatekey_file(os.path.join(ssl_dir, 'daemon.pkey')) - return ctx - - class DelugeRPCProtocol(DelugeTransferProtocol): def __init__(self): super(DelugeRPCProtocol, self).__init__() @@ -391,8 +376,11 @@ class RPCServer(component.Component): # Check for SSL keys and generate some if needed check_ssl_keys() + cert = os.path.join(deluge.configmanager.get_config_dir('ssl'), 'daemon.cert') + pkey = os.path.join(deluge.configmanager.get_config_dir('ssl'), 'daemon.pkey') + try: - reactor.listenSSL(port, self.factory, ServerContextFactory(), interface=hostname) + reactor.listenSSL(port, self.factory, get_context_factory(cert, pkey), interface=hostname) except Exception as ex: log.debug('Daemon already running or port not available.: %s', ex) raise diff --git a/deluge/crypto_utils.py b/deluge/crypto_utils.py new file mode 100644 index 000000000..7993fb98f --- /dev/null +++ b/deluge/crypto_utils.py @@ -0,0 +1,72 @@ +# -*- coding: utf-8 -*- +# +# Copyright (C) 2007,2008 Andrew Resch +# +# This file is part of Deluge and is licensed under GNU General Public License 3.0, or later, with +# the additional special exception to link portions of this program with the OpenSSL library. +# See LICENSE for more details. +# + +from __future__ import division, print_function, unicode_literals + +from OpenSSL.crypto import FILETYPE_PEM +from twisted.internet.ssl import AcceptableCiphers, Certificate, CertificateOptions, KeyPair, TLSVersion + +# A TLS ciphers list. +# Sources for more information on TLS ciphers: +# - https://wiki.mozilla.org/Security/Server_Side_TLS +# - https://www.ssllabs.com/projects/best-practices/index.html +# - https://hynek.me/articles/hardening-your-web-servers-ssl-ciphers/ +# +# This list was inspired by the `urllib3` library +# - https://github.com/urllib3/urllib3/blob/master/urllib3/util/ssl_.py#L79 +# +# The general intent is: +# - prefer cipher suites that offer perfect forward secrecy (ECDHE), +# - prefer AES-GCM over ChaCha20 because hardware-accelerated AES is common, +# - disable NULL authentication, MD5 MACs and DSS for security reasons. +TLS_CIPHERS = ':'.join([ + 'ECDH+AESGCM', + 'ECDH+CHACHA20', + 'AES256-GCM-SHA384', + 'AES128-GCM-SHA256', + '!DSS' + '!aNULL', + '!eNULL', + '!MD5' +]) + +# This value tells OpenSSL to disable all SSL/TLS renegotiation. +SSL_OP_NO_RENEGOTIATION = 0x40000000 + + +def get_context_factory(cert_path, pkey_path): + """OpenSSL context factory. + + Generates an OpenSSL context factory using Twisted's CertificateOptions class. + This will keep a server cipher order. + + Args: + cert_path (string): The path to the certificate file + pkey_path (string): The path to the private key file + + Returns: + twisted.internet.ssl.CertificateOptions: An OpenSSL context factory + """ + + with open(cert_path) as cert: + certificate = Certificate.loadPEM(cert.read()).original + with open(pkey_path) as pkey: + private_key = KeyPair.load(pkey.read(), FILETYPE_PEM).original + ciphers = AcceptableCiphers.fromOpenSSLCipherString(TLS_CIPHERS) + cert_options = CertificateOptions( + privateKey=private_key, + certificate=certificate, + raiseMinimumTo=TLSVersion.TLSv1_2, + acceptableCiphers=ciphers, + ) + ctx = cert_options.getContext() + ctx.use_certificate_chain_file(cert_path) + ctx.set_options(SSL_OP_NO_RENEGOTIATION) + + return cert_options diff --git a/deluge/tests/test_security.py b/deluge/tests/test_security.py new file mode 100644 index 000000000..ff6a3e83b --- /dev/null +++ b/deluge/tests/test_security.py @@ -0,0 +1,184 @@ +# -*- coding: utf-8 -*- +# +# This file is part of Deluge and is licensed under GNU General Public License 3.0, or later, with +# the additional special exception to link portions of this program with the OpenSSL library. +# See LICENSE for more details. +# + +from __future__ import print_function, unicode_literals + +import os + +import pytest +from twisted.internet.utils import getProcessOutputAndValue + +import deluge.component as component +import deluge.ui.web.server +from deluge import configmanager +from deluge.common import windows_check +from deluge.ui.web.server import DelugeWeb + +from .basetest import BaseTestCase +from .common import get_test_data_file +from .common_web import WebServerTestBase +from .daemon_base import DaemonBase + +SECURITY_TESTS = bool(os.getenv('SECURITY_TESTS', False)) + + +class SecurityBaseTestCase(object): + if windows_check(): + skip = 'windows can`t run .sh files' + elif not SECURITY_TESTS: + skip = 'Skipping security tests' + + http_err = 'can\'t run http tests on daemon' + + def __init__(self): + self.home_dir = os.path.expanduser('~') + self.port = 8112 + + def _run_test(self, test): + d = getProcessOutputAndValue( + 'bash', + [ + get_test_data_file('testssl.sh'), + '--quiet', + '--nodns', + '--color', + '0', + test, + '127.0.0.1:%d' % self.port, + ]) + + def on_result(results): + + if test == '-e': + results = results[0].split('\n')[7:-6] + self.assertTrue(len(results) > 3) + else: + self.assertIn('OK', results[0]) + self.assertNotIn('NOT ok', results[0]) + + d.addCallback(on_result) + return d + + def test_secured_webserver_protocol(self): + return self._run_test('-p') + + def test_secured_webserver_standard_ciphers(self): + return self._run_test('-s') + + def test_secured_webserver_heartbleed_vulnerability(self): + return self._run_test('-H') + + def test_secured_webserver_css_injection_vulnerability(self): + return self._run_test('-I') + + def test_secured_webserver_ticketbleed_vulnerability(self): + return self._run_test('-T') + + def test_secured_webserver_renegotiation_vulnerabilities(self): + return self._run_test('-R') + + def test_secured_webserver_crime_vulnerability(self): + return self._run_test('-C') + + def test_secured_webserver_breach_vulnerability(self): + return self._run_test('-B') + + def test_secured_webserver_poodle_vulnerability(self): + return self._run_test('-O') + + def test_secured_webserver_tls_fallback_scsv_mitigation_vulnerability(self): + return self._run_test('-Z') + + def test_secured_webserver_sweet32_vulnerability(self): + return self._run_test('-W') + + def test_secured_webserver_beast_vulnerability(self): + return self._run_test('-A') + + def test_secured_webserver_lucky13_vulnerability(self): + return self._run_test('-L') + + def test_secured_webserver_freak_vulnerability(self): + return self._run_test('-F') + + def test_secured_webserver_logjam_vulnerability(self): + return self._run_test('-J') + + def test_secured_webserver_drown_vulnerability(self): + return self._run_test('-D') + + def test_secured_webserver_forward_secrecy_settings(self): + return self._run_test('-f') + + def test_secured_webserver_rc4_ciphers(self): + return self._run_test('-4') + + def test_secured_webserver_preference(self): + return self._run_test('-P') + + def test_secured_webserver_headers(self): + return self._run_test('-h') + + def test_secured_webserver_ciphers(self): + return self._run_test('-e') + + +@pytest.mark.security +class DaemonSecurityTestCase(BaseTestCase, DaemonBase, SecurityBaseTestCase): + + if windows_check(): + skip = 'windows can\'t start_core not enough arguments for format string' + + def __init__(self, testname): + super(DaemonSecurityTestCase, self).__init__(testname) + DaemonBase.__init__(self) + SecurityBaseTestCase.__init__(self) + + def setUp(self): + skip = False + for not_http_test in ('breach', 'headers', 'ticketbleed'): + if not_http_test in self.id().split('.')[-1]: + self.skipTest(SecurityBaseTestCase.http_err) + skip = True + if not skip: + super(DaemonSecurityTestCase, self).setUp() + + def set_up(self): + d = self.common_set_up() + self.port = self.listen_port + d.addCallback(self.start_core) + d.addErrback(self.terminate_core) + return d + + def tear_down(self): + d = component.shutdown() + d.addCallback(self.terminate_core) + return d + + +@pytest.mark.security +class WebUISecurityTestBase(WebServerTestBase, SecurityBaseTestCase): + + def __init__(self, testname): + super(WebUISecurityTestBase, self).__init__(testname) + SecurityBaseTestCase.__init__(self) + + def start_webapi(self, arg): + self.port = self.webserver_listen_port = 8999 + + config_defaults = deluge.ui.web.server.CONFIG_DEFAULTS.copy() + config_defaults['port'] = self.webserver_listen_port + config_defaults['https'] = True + self.config = configmanager.ConfigManager('web.conf', config_defaults) + + self.deluge_web = DelugeWeb(daemon=False) + + host = list(self.deluge_web.web_api.hostlist.config['hosts'][0]) + host[2] = self.listen_port + self.deluge_web.web_api.hostlist.config['hosts'][0] = tuple(host) + self.host_id = host[0] + self.deluge_web.start() diff --git a/deluge/ui/web/server.py b/deluge/ui/web/server.py index 52dc60140..8e84c040d 100644 --- a/deluge/ui/web/server.py +++ b/deluge/ui/web/server.py @@ -16,15 +16,14 @@ import mimetypes import os import tempfile -from OpenSSL.crypto import FILETYPE_PEM from twisted.application import internet, service from twisted.internet import defer, reactor -from twisted.internet.ssl import SSL, Certificate, CertificateOptions, KeyPair, TLSVersion from twisted.web import http, resource, server, static from deluge import common, component, configmanager from deluge.common import is_ipv6 from deluge.core.rpcserver import check_ssl_keys +from deluge.crypto_utils import get_context_factory from deluge.ui.tracker_icons import TrackerIcons from deluge.ui.translations_util import set_language, setup_translations from deluge.ui.web.auth import Auth @@ -623,6 +622,8 @@ class DelugeWeb(component.Component): setup_translations(setup_gettext=True, setup_pygtk=False) + # Remove twisted version number from 'server' http-header for security reasons + server.version = 'TwistedWeb' self.site = server.Site(self.top_level) self.web_api = WebApi() self.web_utils = WebUtils() @@ -684,20 +685,15 @@ class DelugeWeb(component.Component): check_ssl_keys() log.debug('Enabling SSL with PKey: %s, Cert: %s', self.pkey, self.cert) - with open(configmanager.get_config_dir(self.cert)) as cert: - certificate = Certificate.loadPEM(cert.read()).original - with open(configmanager.get_config_dir(self.pkey)) as pkey: - private_key = KeyPair.load(pkey.read(), FILETYPE_PEM).original - options = CertificateOptions( - privateKey=private_key, - certificate=certificate, - raiseMinimumTo=TLSVersion.TLSv1_2, - ) - ctx = options.getContext() - ctx.set_options(SSL.OP_NO_SSLv2 | SSL.OP_NO_SSLv3) - ctx.use_certificate_chain_file(configmanager.get_config_dir(self.cert)) + cert = configmanager.get_config_dir(self.cert) + pkey = configmanager.get_config_dir(self.pkey) - self.socket = reactor.listenSSL(self.port, self.site, options, interface=self.interface) + self.socket = reactor.listenSSL( + self.port, + self.site, + get_context_factory(cert, pkey), + interface=self.interface + ) ip = self.socket.getHost().host ip = '[%s]' % ip if is_ipv6(ip) else ip log.info('Serving at https://%s:%s%s', ip, self.port, self.base) diff --git a/tox.ini b/tox.ini index aa3aa7ce9..176c6e0ab 100644 --- a/tox.ini +++ b/tox.ini @@ -52,7 +52,11 @@ log_cli_level = CRITICAL [testenv:pydef] commands = python -c "import libtorrent as lt; print(lt.__version__)" - pytest -v --basetemp=_pytest_temp -s -m "not (todo or gtkui)" deluge/tests + pytest -v --basetemp=_pytest_temp -s -m "not (todo or gtkui or security)" deluge/tests + +[testenv:security] +setenv = SECURITY_TESTS = True +commands = pytest -v --basetemp=_pytest_temp -s -m "security" deluge/tests/ [testenv:pygtkui] commands = pytest -v --basetemp=_pytest_temp -s -m "gtkui" deluge/tests