From 8f672cd49b753ea0889e24fff6614b07d68a2dee Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Thu, 29 Sep 2022 10:57:17 -0700 Subject: [PATCH] openvpn: Drop RSA to ECC migration code and two-step setup - RSA to ECC migration was introduced in October 2020 is available to Buster (via backports) and to Bullseye users. Dropping the code will make it easy to test regular maintenance code updates. - A two step setup process of first installing and then setting up the certificates is no longer necessary. (New installs already don't use this). The certificate generation process does not take hours but minutes. We also have a good progress indication during install+setup process. Tests: - Functional tests pass. - Initial setup completes successfully and does not take very long time. - Profiles can be downloaded successfully and imported. - A client an use them to connect. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- plinth/modules/openvpn/__init__.py | 26 ------- plinth/modules/openvpn/privileged.py | 60 ++--------------- .../openvpn/templates/migrate_to_ecc.html | 39 ----------- plinth/modules/openvpn/templates/openvpn.html | 58 ++++++---------- .../openvpn/tests/test_configuration.py | 67 ------------------- plinth/modules/openvpn/urls.py | 6 +- plinth/modules/openvpn/views.py | 34 +--------- 7 files changed, 27 insertions(+), 263 deletions(-) delete mode 100644 plinth/modules/openvpn/templates/migrate_to_ecc.html delete mode 100644 plinth/modules/openvpn/tests/test_configuration.py diff --git a/plinth/modules/openvpn/__init__.py b/plinth/modules/openvpn/__init__.py index 93b09dd3e..832723573 100644 --- a/plinth/modules/openvpn/__init__.py +++ b/plinth/modules/openvpn/__init__.py @@ -1,8 +1,6 @@ # SPDX-License-Identifier: AGPL-3.0-or-later """FreedomBox app to configure OpenVPN server.""" -import os - from django.urls import reverse_lazy from django.utils.translation import gettext_lazy as _ @@ -38,11 +36,6 @@ class OpenVPNApp(app_module.App): _version = 4 - @property - def can_be_disabled(self): - """Return whether the app can be disabled.""" - return privileged.is_setup() - def __init__(self): """Create components for the app.""" super().__init__() @@ -93,27 +86,8 @@ class OpenVPNApp(app_module.App): **manifest.backup) self.add(backup_restore) - def is_enabled(self): - """Return whether all the leader components are enabled. - - Return True when there are no leader components and OpenVPN setup - is done. - """ - return super().is_enabled() and privileged.is_setup() - def setup(self, old_version): """Install and configure the app.""" super().setup(old_version) privileged.setup() self.enable() - - -def is_using_ecc(): - """Return whether the service is using ECC.""" - if os.path.exists(SERVER_CONFIGURATION_FILE): - with open(SERVER_CONFIGURATION_FILE, 'r', - encoding='utf-8') as file_handle: - for line in file_handle: - if line.strip() == 'dh none': - return True - return False diff --git a/plinth/modules/openvpn/privileged.py b/plinth/modules/openvpn/privileged.py index 85682ae32..e33c7c541 100644 --- a/plinth/modules/openvpn/privileged.py +++ b/plinth/modules/openvpn/privileged.py @@ -50,26 +50,7 @@ cipher AES-256-CBC script-security 2 ''' -CLIENT_CONFIGURATION_RSA = ''' -client -remote {remote} 1194 -proto udp -proto udp6 -dev tun -nobind -remote-cert-tls server -cipher AES-256-CBC -comp-lzo -redirect-gateway -verb 3 - -{ca} - -{cert} - -{key}''' - -CLIENT_CONFIGURATION_ECC = ''' +CLIENT_CONFIGURATION = ''' client remote {remote} 1194 proto udp @@ -88,6 +69,7 @@ verb 3 {key}''' CERTIFICATE_CONFIGURATION = { + 'EASYRSA_ALGO': 'ec', 'EASYRSA_BATCH': '1', 'EASYRSA_DIGEST': 'sha512', 'KEY_CONFIG': '/usr/share/easy-rsa/openssl-easyrsa.cnf', @@ -104,34 +86,7 @@ CERTIFICATE_CONFIGURATION = { 'EASYRSA_REQ_NAME': 'FreedomBox' } -CERTIFICATE_CONFIGURATION_RSA = { - 'EASYRSA_KEY_SIZE': '4096', - **CERTIFICATE_CONFIGURATION -} - -CERTIFICATE_CONFIGURATION_ECC = { - 'EASYRSA_ALGO': 'ec', - **CERTIFICATE_CONFIGURATION -} - -COMMON_ARGS = {'env': CERTIFICATE_CONFIGURATION_ECC, 'cwd': KEYS_DIRECTORY} - - -def _is_using_ecc(): - """Return whether the service is using ECC.""" - if os.path.exists(SERVER_CONFIGURATION_PATH): - with open(SERVER_CONFIGURATION_PATH, 'r', - encoding='utf-8') as file_handle: - for line in file_handle: - if line.strip() == 'dh none': - return True - return False - - -@privileged -def is_setup() -> bool: - """Return whether setup is complete.""" - return _is_non_empty_file(DH_PARAMS) or os.path.exists(EC_PARAMS_DIR) +COMMON_ARGS = {'env': CERTIFICATE_CONFIGURATION, 'cwd': KEYS_DIRECTORY} @privileged @@ -227,18 +182,13 @@ def get_profile(username: str, remote_server: str) -> str: subprocess.check_call([ '/usr/share/easy-rsa/easyrsa', 'build-client-full', username, 'nopass' - ], env=CERTIFICATE_CONFIGURATION_ECC if _is_using_ecc() else - CERTIFICATE_CONFIGURATION_RSA, - cwd=KEYS_DIRECTORY) + ], env=CERTIFICATE_CONFIGURATION, cwd=KEYS_DIRECTORY) user_certificate_string = _read_file(user_certificate) user_key_string = _read_file(user_key) ca_string = _read_file(CA_CERTIFICATE_PATH) - client_configuration = CLIENT_CONFIGURATION_ECC if _is_using_ecc( - ) else CLIENT_CONFIGURATION_RSA - - return client_configuration.format(ca=ca_string, + return CLIENT_CONFIGURATION.format(ca=ca_string, cert=user_certificate_string, key=user_key_string, remote=remote_server) diff --git a/plinth/modules/openvpn/templates/migrate_to_ecc.html b/plinth/modules/openvpn/templates/migrate_to_ecc.html deleted file mode 100644 index 8f22afab1..000000000 --- a/plinth/modules/openvpn/templates/migrate_to_ecc.html +++ /dev/null @@ -1,39 +0,0 @@ -{% comment %} -# SPDX-License-Identifier: AGPL-3.0-or-later -{% endcomment %} - -{% load i18n %} - -

{% trans "Migrate to ECC" %}

- -

- {% blocktrans trimmed %} - Your OpenVPN installation is currently using RSA. Switching to the - modern Elliptic Curve Cryptography improves speed of establishing a - connection and security. This operation is irreversible. It should only take - a few minutes on most single board computers. - {% endblocktrans %} -

- -

- {% blocktrans trimmed %} - All new installations of OpenVPN on {{ box_name }} will - use ECC by default. We recommend migrating as soon as possible. - {% endblocktrans %} -

- -

- {% blocktrans trimmed %} - Warning: Existing client profiles will be invalidated by this operation. All - OpenVPN users on {{ box_name }} must download their new profiles. OpenVPN - clients compatible with ECC should be used to connect to this server. - {% endblocktrans %} -

- -
- {% csrf_token %} - - -
diff --git a/plinth/modules/openvpn/templates/openvpn.html b/plinth/modules/openvpn/templates/openvpn.html index 62a42c3e8..6c2d8a4cb 100644 --- a/plinth/modules/openvpn/templates/openvpn.html +++ b/plinth/modules/openvpn/templates/openvpn.html @@ -7,49 +7,31 @@ {% load i18n %} {% load static %} -{% block status %} - - {% if status.is_setup %} - {{ block.super }} - {% endif %} - -{% endblock %} - {% block configuration %} - {% if status.is_setup %} +

{% trans "Profile" %}

-

{% trans "Profile" %}

+

+ {% blocktrans trimmed %} + To connect to {{ box_name }}'s VPN, you need to download a profile and + feed it to an OpenVPN client on your mobile or desktop machine. OpenVPN + Clients are available for most platforms. Click "Learn more..." above for + recommended clients and instructions on how to configure them. + {% endblocktrans %} +

-

- {% blocktrans trimmed %} - To connect to {{ box_name }}'s VPN, you need to download a - profile and feed it to an OpenVPN client on your mobile or - desktop machine. OpenVPN Clients are available for most - platforms. Click "Learn more..." above for recommended clients - and instructions on how to configure them. - {% endblocktrans %} -

+

+ {% blocktrans trimmed %} + Profile is specific to each user of {{ box_name }}. Keep it a secret. + {% endblocktrans %} +

-

- {% blocktrans trimmed %} - Profile is specific to each user of {{ box_name }}. Keep it a - secret. - {% endblocktrans %} -

+
+ {% csrf_token %} - - {% csrf_token %} - - -
- - {% if not using_ecc %} - {% include "migrate_to_ecc.html" %} - {% endif %} - - {% endif %} + + {% endblock %} diff --git a/plinth/modules/openvpn/tests/test_configuration.py b/plinth/modules/openvpn/tests/test_configuration.py deleted file mode 100644 index 11f8e8cee..000000000 --- a/plinth/modules/openvpn/tests/test_configuration.py +++ /dev/null @@ -1,67 +0,0 @@ -# SPDX-License-Identifier: AGPL-3.0-or-later -""" -Test module for OpenVPN configuration. -""" - -import os -from unittest.mock import patch - -import pytest - -from plinth.modules import openvpn -from plinth.modules.openvpn import privileged - -pytestmark = pytest.mark.usefixtures('mock_privileged') -privileged_modules_to_mock = ['plinth.modules.openvpn.privileged'] - - -@pytest.fixture(name='keys_directory') -def fixture_keys_directory(tmp_path): - return tmp_path - - -@pytest.fixture(autouse=True) -def fixture_set_keys_directory(keys_directory): - """Set the keys directory in the actions module.""" - privileged.DH_PARAMS = f'{keys_directory}/pki/dh.pem' - privileged.EC_PARAMS_DIR = f'{keys_directory}/pki/ecparams' - - -@pytest.fixture(name='conf_file') -def fixture_conf_file(tmp_path): - """Fixture that returns an empty configuration file.""" - return str(tmp_path / 'freedombox.conf') - - -def test_identify_rsa_configuration(conf_file): - """Identify RSA configuration based on configuration file.""" - with patch('plinth.modules.openvpn.SERVER_CONFIGURATION_FILE', conf_file): - with open(conf_file, 'w', encoding='utf-8') as file_handle: - file_handle.write('dh /etc/openvpn/freedombox-keys/pki/dh.pem') - assert not openvpn.is_using_ecc() - - -def test_identify_ecc_configuration(conf_file): - """Identify ECC configuration based on configuration file.""" - with patch('plinth.modules.openvpn.SERVER_CONFIGURATION_FILE', conf_file): - with open(conf_file, 'w', encoding='utf-8') as file_handle: - file_handle.write('dh none') - assert openvpn.is_using_ecc() - - -def test_is_setup_with_rsa(keys_directory): - """is_setup should work with RSA configuration.""" - (keys_directory / 'pki').mkdir() - dh_params_file = keys_directory / 'pki' / 'dh.pem' - dh_params_file.write_text('some content') - assert privileged.is_setup() - os.remove(dh_params_file) - - -def test_is_setup_with_ecc(keys_directory): - """is_setup should work with RSA configuration.""" - (keys_directory / 'pki' / 'ecparams').mkdir(parents=True) - ec_params_file = keys_directory / 'pki' / 'ecparams' / 'somecurve.pem' - ec_params_file.write_text('some content') - assert privileged.is_setup() - os.remove(ec_params_file) diff --git a/plinth/modules/openvpn/urls.py b/plinth/modules/openvpn/urls.py index 77b24d3e1..e9382644b 100644 --- a/plinth/modules/openvpn/urls.py +++ b/plinth/modules/openvpn/urls.py @@ -1,7 +1,5 @@ # SPDX-License-Identifier: AGPL-3.0-or-later -""" -URLs for the OpenVPN module. -""" +"""URLs for the OpenVPN module.""" from django.urls import re_path @@ -11,8 +9,6 @@ from . import views urlpatterns = [ re_path(r'^apps/openvpn/$', views.OpenVPNAppView.as_view(), name='index'), - re_path(r'^apps/openvpn/setup/$', views.setup, name='setup'), - re_path(r'^apps/openvpn/ecc/$', views.ecc, name='ecc'), re_path(r'^apps/openvpn/profile/$', user_group_view(views.profile, 'vpn'), name='profile'), ] diff --git a/plinth/modules/openvpn/views.py b/plinth/modules/openvpn/views.py index f64ff29c1..3dad16fc2 100644 --- a/plinth/modules/openvpn/views.py +++ b/plinth/modules/openvpn/views.py @@ -4,11 +4,8 @@ import logging from django.http import HttpResponse -from django.shortcuts import redirect -from django.views.decorators.http import require_POST -from plinth import app as app_module -from plinth.modules import config, openvpn +from plinth.modules import config from plinth.views import AppView from . import privileged @@ -22,26 +19,6 @@ class OpenVPNAppView(AppView): app_id = 'openvpn' template_name = 'openvpn.html' - def get_context_data(self, *args, **kwargs): - """Add additional context data for template.""" - context = super().get_context_data(*args, **kwargs) - context['status'] = { - 'is_setup': privileged.is_setup(), - } - context['using_ecc'] = openvpn.is_using_ecc() - return context - - -@require_POST -def setup(_): - """Start the setup process.""" - if not privileged.is_setup(): - privileged.setup() - - app_module.App.get('openvpn').enable() - - return redirect('openvpn:index') - def profile(request): """Provide the user's profile for download.""" @@ -58,12 +35,3 @@ def profile(request): 'attachment; filename={username}.ovpn'.format(username=username) return response - - -@require_POST -def ecc(_): - """Migrate from RSA to ECC.""" - if privileged.is_setup(): - privileged.setup() - - return redirect('openvpn:index')