From d0d3071e99eb65edb888c26ef2fdc9e038438887 Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Sat, 6 Jan 2024 10:55:42 +0100 Subject: https: T5902: remove virtual-host configuration We have not seen the adoption of the https virtual-host CLI option. What it did? * Create multiple webservers each listening on a different IP/port (but in the same VRF) * All webservers shared one common document root * All webservers shared the same SSL certificates * All webservers could have had individual allow-client configurations * API could be enabled for a particular virtual-host but was always enabled on the default host This configuration tried to provide a full webserver via the CLI but VyOS is a router and the Webserver is there for an API or to serve files for a local-ui. Changes Remove support for virtual-hosts as it's an incomplete and thus mostly useless "thing". Migrate all allow-client statements to one top-level allow statement. --- smoketest/scripts/cli/test_pki.py | 52 +++++++++-------- smoketest/scripts/cli/test_service_https.py | 87 +++++++++++++++++------------ 2 files changed, 79 insertions(+), 60 deletions(-) (limited to 'smoketest/scripts/cli') diff --git a/smoketest/scripts/cli/test_pki.py b/smoketest/scripts/cli/test_pki.py index 2ccc63b2c..940ff9ec0 100755 --- a/smoketest/scripts/cli/test_pki.py +++ b/smoketest/scripts/cli/test_pki.py @@ -19,6 +19,8 @@ import unittest from base_vyostest_shim import VyOSUnitTestSHIM from vyos.configsession import ConfigSessionError +from vyos.utils.file import read_file + base_path = ['pki'] valid_ca_cert = """ @@ -153,10 +155,10 @@ class TestPKI(VyOSUnitTestSHIM.TestCase): @classmethod def setUpClass(cls): super(TestPKI, cls).setUpClass() - # ensure we can also run this test on a live system - so lets clean # out the current configuration :) cls.cli_delete(cls, base_path) + cls.cli_delete(cls, ['service', 'https']) def tearDown(self): self.cli_delete(base_path) @@ -181,68 +183,72 @@ class TestPKI(VyOSUnitTestSHIM.TestCase): self.cli_commit() def test_invalid_ca_valid_certificate(self): - self.cli_set(base_path + ['ca', 'smoketest', 'certificate', valid_cert.replace('\n','')]) + self.cli_set(base_path + ['ca', 'invalid-ca', 'certificate', valid_cert.replace('\n','')]) with self.assertRaises(ConfigSessionError): self.cli_commit() def test_certificate_in_use(self): - self.cli_set(base_path + ['certificate', 'smoketest', 'certificate', valid_ca_cert.replace('\n','')]) - self.cli_set(base_path + ['certificate', 'smoketest', 'private', 'key', valid_ca_private_key.replace('\n','')]) + cert_name = 'smoketest' + + self.cli_set(base_path + ['certificate', cert_name, 'certificate', valid_ca_cert.replace('\n','')]) + self.cli_set(base_path + ['certificate', cert_name, 'private', 'key', valid_ca_private_key.replace('\n','')]) self.cli_commit() - self.cli_set(['service', 'https', 'certificates', 'certificate', 'smoketest']) + self.cli_set(['service', 'https', 'certificates', 'certificate', cert_name]) self.cli_commit() - self.cli_delete(base_path + ['certificate', 'smoketest']) + self.cli_delete(base_path + ['certificate', cert_name]) with self.assertRaises(ConfigSessionError): self.cli_commit() self.cli_delete(['service', 'https', 'certificates', 'certificate']) def test_certificate_https_update(self): - self.cli_set(base_path + ['certificate', 'smoketest', 'certificate', valid_ca_cert.replace('\n','')]) - self.cli_set(base_path + ['certificate', 'smoketest', 'private', 'key', valid_ca_private_key.replace('\n','')]) + cert_name = 'smoketest' + cert_path = f'/run/nginx/certs/{cert_name}_cert.pem' + self.cli_set(base_path + ['certificate', cert_name, 'certificate', valid_ca_cert.replace('\n','')]) + self.cli_set(base_path + ['certificate', cert_name, 'private', 'key', valid_ca_private_key.replace('\n','')]) self.cli_commit() - self.cli_set(['service', 'https', 'certificates', 'certificate', 'smoketest']) + self.cli_set(['service', 'https', 'certificates', 'certificate', cert_name]) self.cli_commit() cert_data = None - with open('/etc/ssl/certs/smoketest.pem') as f: - cert_data = f.read() + cert_data = read_file(cert_path) - self.cli_set(base_path + ['certificate', 'smoketest', 'certificate', valid_update_cert.replace('\n','')]) - self.cli_set(base_path + ['certificate', 'smoketest', 'private', 'key', valid_update_private_key.replace('\n','')]) + self.cli_set(base_path + ['certificate', cert_name, 'certificate', valid_update_cert.replace('\n','')]) + self.cli_set(base_path + ['certificate', cert_name, 'private', 'key', valid_update_private_key.replace('\n','')]) self.cli_commit() - with open('/etc/ssl/certs/smoketest.pem') as f: - self.assertNotEqual(cert_data, f.read()) + self.assertNotEqual(cert_data, read_file(cert_path)) self.cli_delete(['service', 'https', 'certificates', 'certificate']) def test_certificate_eapol_update(self): - self.cli_set(base_path + ['certificate', 'smoketest', 'certificate', valid_ca_cert.replace('\n','')]) - self.cli_set(base_path + ['certificate', 'smoketest', 'private', 'key', valid_ca_private_key.replace('\n','')]) + cert_name = 'eapol' + interface = 'eth1' + self.cli_set(base_path + ['certificate', cert_name, 'certificate', valid_ca_cert.replace('\n','')]) + self.cli_set(base_path + ['certificate', cert_name, 'private', 'key', valid_ca_private_key.replace('\n','')]) self.cli_commit() - self.cli_set(['interfaces', 'ethernet', 'eth1', 'eapol', 'certificate', 'smoketest']) + self.cli_set(['interfaces', 'ethernet', interface, 'eapol', 'certificate', cert_name]) self.cli_commit() cert_data = None - with open('/run/wpa_supplicant/eth1_cert.pem') as f: + with open(f'/run/wpa_supplicant/{interface}_cert.pem') as f: cert_data = f.read() - self.cli_set(base_path + ['certificate', 'smoketest', 'certificate', valid_update_cert.replace('\n','')]) - self.cli_set(base_path + ['certificate', 'smoketest', 'private', 'key', valid_update_private_key.replace('\n','')]) + self.cli_set(base_path + ['certificate', cert_name, 'certificate', valid_update_cert.replace('\n','')]) + self.cli_set(base_path + ['certificate', cert_name, 'private', 'key', valid_update_private_key.replace('\n','')]) self.cli_commit() - with open('/run/wpa_supplicant/eth1_cert.pem') as f: + with open(f'/run/wpa_supplicant/{interface}_cert.pem') as f: self.assertNotEqual(cert_data, f.read()) - self.cli_delete(['interfaces', 'ethernet', 'eth1', 'eapol']) + self.cli_delete(['interfaces', 'ethernet', interface, 'eapol']) if __name__ == '__main__': unittest.main(verbosity=2) diff --git a/smoketest/scripts/cli/test_service_https.py b/smoketest/scripts/cli/test_service_https.py index 280932fd7..8d9b8459e 100755 --- a/smoketest/scripts/cli/test_service_https.py +++ b/smoketest/scripts/cli/test_service_https.py @@ -1,6 +1,6 @@ #!/usr/bin/env python3 # -# Copyright (C) 2019-2023 VyOS maintainers and contributors +# Copyright (C) 2019-2024 VyOS maintainers and contributors # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License version 2 or later as @@ -23,6 +23,7 @@ from urllib3.exceptions import InsecureRequestWarning from base_vyostest_shim import VyOSUnitTestSHIM from base_vyostest_shim import ignore_warning from vyos.utils.file import read_file +from vyos.utils.file import write_file from vyos.utils.process import call from vyos.utils.process import process_named_running @@ -52,7 +53,22 @@ MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgPLpD0Ohhoq0g4nhx u8/3jHMM7sDwL3aWzW/zp54/LhCWUoLMjDdDEEigK4fal4ZF9aA9F0Ww """ +dh_1024 = """ +MIGHAoGBAM3nvMkHGi/xmRs8cYg4pcl5sAanxel9EM+1XobVhUViXw8JvlmSEVOj +n2aXUifc4SEs3WDzVPRC8O8qQWjvErpTq/HOgt3aqBCabMgvflmt706XP0KiqnpW +EyvNiI27J3wBUzEXLIS110MxPAX5Tcug974PecFcOxn1RWrbWcx/AgEC +""" + +dh_2048 = """ +MIIBCAKCAQEA1mld/V7WnxxRinkOlhx/BoZkRELtIUQFYxyARBqYk4C5G3YnZNNu +zjaGyPnfIKHu8SIUH85OecM+5/co9nYlcUJuph2tbR6qNgPw7LOKIhf27u7WhvJk +iVsJhwZiWmvvMV4jTParNEI2svoooMyhHXzeweYsg6YtgLVmwiwKj3XP3gRH2i3B +Mq8CDS7X6xaKvjfeMPZBFqOM5nb6HhsbaAUyiZxrfipLvXxtnbzd/eJUQVfVdxM3 +pn0i+QrO2tuNAzX7GoPc9pefrbb5xJmGS50G0uqsR59+7LhYmyZSBASA0lxTEW9t +kv/0LPvaYTY57WL7hBeqqHy/WPZHPzDI3wIBAg== +""" # to test load config via HTTP URL +nginx_tmp_site = '/etc/nginx/sites-enabled/smoketest' nginx_conf_smoketest = """ server { listen 8000; @@ -81,6 +97,11 @@ class TestHTTPSService(VyOSUnitTestSHIM.TestCase): cls.cli_delete(cls, base_path) cls.cli_delete(cls, pki_base) + @classmethod + def tearDownClass(cls): + super(TestHTTPSService, cls).tearDownClass() + call(f'sudo rm -f {nginx_tmp_site}') + def tearDown(self): self.cli_delete(base_path) self.cli_delete(pki_base) @@ -89,33 +110,31 @@ class TestHTTPSService(VyOSUnitTestSHIM.TestCase): # Check for stopped process self.assertFalse(process_named_running(PROCESS_NAME)) - def test_server_block(self): - vhost_id = 'example' - address = '0.0.0.0' - port = '8443' - name = 'example.org' - - test_path = base_path + ['virtual-host', vhost_id] - - self.cli_set(test_path + ['listen-address', address]) - self.cli_set(test_path + ['port', port]) - self.cli_set(test_path + ['server-name', name]) - - self.cli_commit() - - nginx_config = read_file('/etc/nginx/sites-enabled/default') - self.assertIn(f'listen {address}:{port} ssl;', nginx_config) - self.assertIn(f'ssl_protocols TLSv1.2 TLSv1.3;', nginx_config) - self.assertTrue(process_named_running(PROCESS_NAME)) - def test_certificate(self): - self.cli_set(pki_base + ['certificate', 'test_https', 'certificate', cert_data.replace('\n','')]) - self.cli_set(pki_base + ['certificate', 'test_https', 'private', 'key', key_data.replace('\n','')]) - - self.cli_set(base_path + ['certificates', 'certificate', 'test_https']) + cert_name = 'test_https' + dh_name = 'dh-test' + + self.cli_set(base_path + ['certificates', 'certificate', cert_name]) + # verify() - certificates do not exist (yet) + with self.assertRaises(ConfigSessionError): + self.cli_commit() + self.cli_set(pki_base + ['certificate', cert_name, 'certificate', cert_data.replace('\n','')]) + self.cli_set(pki_base + ['certificate', cert_name, 'private', 'key', key_data.replace('\n','')]) + + self.cli_set(base_path + ['certificates', 'dh-params', dh_name]) + # verify() - dh-params do not exist (yet) + with self.assertRaises(ConfigSessionError): + self.cli_commit() + + self.cli_set(pki_base + ['dh', dh_name, 'parameters', dh_1024.replace('\n','')]) + # verify() - dh-param minimum length is 2048 bit + with self.assertRaises(ConfigSessionError): + self.cli_commit() + self.cli_set(pki_base + ['dh', dh_name, 'parameters', dh_2048.replace('\n','')]) self.cli_commit() self.assertTrue(process_named_running(PROCESS_NAME)) + self.debug = False def test_api_missing_keys(self): self.cli_set(base_path + ['api']) @@ -135,15 +154,13 @@ class TestHTTPSService(VyOSUnitTestSHIM.TestCase): key = 'MySuperSecretVyOS' self.cli_set(base_path + ['api', 'keys', 'id', 'key-01', 'key', key]) - test_path = base_path + ['virtual-host', vhost_id] - self.cli_set(test_path + ['listen-address', address]) - self.cli_set(test_path + ['server-name', name]) + self.cli_set(base_path + ['listen-address', address]) self.cli_commit() nginx_config = read_file('/etc/nginx/sites-enabled/default') self.assertIn(f'listen {address}:{port} ssl;', nginx_config) - self.assertIn(f'ssl_protocols TLSv1.2 TLSv1.3;', nginx_config) + self.assertIn(f'ssl_protocols TLSv1.2 TLSv1.3;', nginx_config) # default url = f'https://{address}/retrieve' payload = {'data': '{"op": "showConfig", "path": []}', 'key': f'{key}'} @@ -402,19 +419,15 @@ class TestHTTPSService(VyOSUnitTestSHIM.TestCase): url_config = f'https://{address}/configure' headers = {} tmp_file = 'tmp-config.boot' - nginx_tmp_site = '/etc/nginx/sites-enabled/smoketest' self.cli_set(base_path + ['api', 'keys', 'id', 'key-01', 'key', key]) self.cli_commit() # load config via HTTP requires nginx config call(f'sudo touch {nginx_tmp_site}') - call(f'sudo chown vyos:vyattacfg {nginx_tmp_site}') - call(f'sudo chmod +w {nginx_tmp_site}') - - with open(nginx_tmp_site, 'w') as f: - f.write(nginx_conf_smoketest) - call('sudo nginx -s reload') + call(f'sudo chmod 666 {nginx_tmp_site}') + write_file(nginx_tmp_site, nginx_conf_smoketest) + call('sudo systemctl reload nginx') # save config payload = { @@ -441,8 +454,8 @@ class TestHTTPSService(VyOSUnitTestSHIM.TestCase): self.assertEqual(r.status_code, 200) # cleanup tmp nginx conf - call(f'sudo rm -rf {nginx_tmp_site}') - call('sudo nginx -s reload') + call(f'sudo rm -f {nginx_tmp_site}') + call('sudo systemctl reload nginx') if __name__ == '__main__': unittest.main(verbosity=5) -- cgit v1.2.3 From fc6926fdf32a7bdf9f943c7818ee6ea4a8131fba Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Mon, 8 Jan 2024 20:42:17 +0100 Subject: pki: T5911: fix service update algorithm if certificate name contains a hyphen (-) When testing for changed PKI certificates using node_changed(), we should not use key_mangling=('-', '_'), as this will make certificate updates with a hypen not possible. --- smoketest/scripts/cli/test_pki.py | 2 +- src/conf_mode/pki.py | 11 +++++------ 2 files changed, 6 insertions(+), 7 deletions(-) (limited to 'smoketest/scripts/cli') diff --git a/smoketest/scripts/cli/test_pki.py b/smoketest/scripts/cli/test_pki.py index 940ff9ec0..02beafb26 100755 --- a/smoketest/scripts/cli/test_pki.py +++ b/smoketest/scripts/cli/test_pki.py @@ -205,7 +205,7 @@ class TestPKI(VyOSUnitTestSHIM.TestCase): self.cli_delete(['service', 'https', 'certificates', 'certificate']) def test_certificate_https_update(self): - cert_name = 'smoketest' + cert_name = 'smoke-test_foo' cert_path = f'/run/nginx/certs/{cert_name}_cert.pem' self.cli_set(base_path + ['certificate', cert_name, 'certificate', valid_ca_cert.replace('\n','')]) self.cli_set(base_path + ['certificate', cert_name, 'private', 'key', valid_ca_private_key.replace('\n','')]) diff --git a/src/conf_mode/pki.py b/src/conf_mode/pki.py index 239e44c3b..4be40e99e 100755 --- a/src/conf_mode/pki.py +++ b/src/conf_mode/pki.py @@ -130,28 +130,27 @@ def get_config(config=None): if len(argv) > 1 and argv[1] == 'certbot_renew': pki['certbot_renew'] = {} - tmp = node_changed(conf, base + ['ca'], key_mangling=('-', '_'), recursive=True) + tmp = node_changed(conf, base + ['ca'], recursive=True) if tmp: if 'changed' not in pki: pki.update({'changed':{}}) pki['changed'].update({'ca' : tmp}) - tmp = node_changed(conf, base + ['certificate'], key_mangling=('-', '_'), recursive=True) + tmp = node_changed(conf, base + ['certificate'], recursive=True) if tmp: if 'changed' not in pki: pki.update({'changed':{}}) pki['changed'].update({'certificate' : tmp}) - tmp = node_changed(conf, base + ['dh'], key_mangling=('-', '_'), recursive=True) + tmp = node_changed(conf, base + ['dh'], recursive=True) if tmp: if 'changed' not in pki: pki.update({'changed':{}}) pki['changed'].update({'dh' : tmp}) - tmp = node_changed(conf, base + ['key-pair'], key_mangling=('-', '_'), recursive=True) + tmp = node_changed(conf, base + ['key-pair'], recursive=True) if tmp: if 'changed' not in pki: pki.update({'changed':{}}) pki['changed'].update({'key_pair' : tmp}) - tmp = node_changed(conf, base + ['openvpn', 'shared-secret'], key_mangling=('-', '_'), - recursive=True) + tmp = node_changed(conf, base + ['openvpn', 'shared-secret'], recursive=True) if tmp: if 'changed' not in pki: pki.update({'changed':{}}) pki['changed'].update({'openvpn' : tmp}) -- cgit v1.2.3