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. --- src/conf_mode/service_https.py | 237 ++++++++------------- .../system/nginx.service.d/10-override.conf | 3 + src/migration-scripts/https/5-to-6 | 76 +++++-- src/services/vyos-http-api-server | 10 +- 4 files changed, 159 insertions(+), 167 deletions(-) create mode 100644 src/etc/systemd/system/nginx.service.d/10-override.conf (limited to 'src') diff --git a/src/conf_mode/service_https.py b/src/conf_mode/service_https.py index 2e7ebda5a..46efc3c93 100755 --- a/src/conf_mode/service_https.py +++ b/src/conf_mode/service_https.py @@ -15,51 +15,41 @@ # along with this program. If not, see . import os +import socket import sys import json -from copy import deepcopy from time import sleep -import vyos.defaults - from vyos.base import Warning from vyos.config import Config +from vyos.config import config_dict_merge from vyos.configdiff import get_config_diff from vyos.configverify import verify_vrf -from vyos import ConfigError +from vyos.defaults import api_config_state from vyos.pki import wrap_certificate from vyos.pki import wrap_private_key +from vyos.pki import wrap_dh_parameters +from vyos.pki import load_dh_parameters from vyos.template import render +from vyos.utils.dict import dict_search from vyos.utils.process import call +from vyos.utils.process import is_systemd_service_active from vyos.utils.network import check_port_availability from vyos.utils.network import is_listen_port_bind_service from vyos.utils.file import write_file - +from vyos import ConfigError from vyos import airbag airbag.enable() -config_file = '/etc/nginx/sites-available/default' +config_file = '/etc/nginx/sites-enabled/default' systemd_override = r'/run/systemd/system/nginx.service.d/override.conf' -cert_dir = '/etc/ssl/certs' -key_dir = '/etc/ssl/private' - -api_config_state = '/run/http-api-state' -systemd_service = '/run/systemd/system/vyos-http-api.service' - -# https config needs to coordinate several subsystems: api, -# self-signed certificate, as well as the virtual hosts defined within the -# https config definition itself. Consequently, one needs a general dict, -# encompassing the https and other configs, and a list of such virtual hosts -# (server blocks in nginx terminology) to pass to the jinja2 template. -default_server_block = { - 'id' : '', - 'address' : '*', - 'port' : '443', - 'name' : ['_'], - 'api' : False, - 'vyos_cert' : {}, -} +cert_dir = '/run/nginx/certs' + +user = 'www-data' +group = 'www-data' + +systemd_service_api = '/run/systemd/system/vyos-http-api.service' def get_config(config=None): if config: @@ -71,83 +61,70 @@ def get_config(config=None): if not conf.exists(base): return None - diff = get_config_diff(conf) - - https = conf.get_config_dict(base, get_first_key=True, with_pki=True) + https = conf.get_config_dict(base, get_first_key=True, + key_mangling=('-', '_'), + with_pki=True) - https['api_add_or_delete'] = diff.node_changed_presence(base + ['api']) + # store path to API config file for later use in templates + https['api_config_state'] = api_config_state + # get fully qualified system hsotname + https['hostname'] = socket.getfqdn() - if 'api' not in https: - return https + # We have gathered the dict representation of the CLI, but there are default + # options which we need to update into the dictionary retrived. + default_values = conf.get_config_defaults(**https.kwargs, recursive=True) + if 'api' not in https or 'graphql' not in https['api']: + del default_values['api'] - http_api = conf.get_config_dict(base + ['api'], key_mangling=('-', '_'), - no_tag_node_value_mangle=True, - get_first_key=True, - with_recursive_defaults=True) - - if http_api.from_defaults(['graphql']): - del http_api['graphql'] - - # Do we run inside a VRF context? - vrf_path = ['service', 'https', 'vrf'] - if conf.exists(vrf_path): - http_api['vrf'] = conf.return_value(vrf_path) - - https['api'] = http_api + # merge CLI and default dictionary + https = config_dict_merge(default_values, https) return https def verify(https): - from vyos.utils.dict import dict_search - if https is None: return None - if 'certificates' in https: - certificates = https['certificates'] + if 'certificates' in https and 'certificate' in https['certificates']: + cert_name = https['certificates']['certificate'] + if 'pki' not in https: + raise ConfigError('PKI is not configured!') - if 'certificate' in certificates: - if not https['pki']: - raise ConfigError('PKI is not configured') + if cert_name not in https['pki']['certificate']: + raise ConfigError('Invalid certificate in configuration!') - cert_name = certificates['certificate'] + pki_cert = https['pki']['certificate'][cert_name] - if cert_name not in https['pki']['certificate']: - raise ConfigError('Invalid certificate on https configuration') + if 'certificate' not in pki_cert: + raise ConfigError('Missing certificate in configuration!') - pki_cert = https['pki']['certificate'][cert_name] + if 'private' not in pki_cert or 'key' not in pki_cert['private']: + raise ConfigError('Missing certificate private key in configuration!') - if 'certificate' not in pki_cert: - raise ConfigError('Missing certificate on https configuration') + if 'dh_params' in https['certificates']: + dh_name = https['certificates']['dh_params'] + if dh_name not in https['pki']['dh']: + raise ConfigError('Invalid DH parameter in configuration!') - if 'private' not in pki_cert or 'key' not in pki_cert['private']: - raise ConfigError("Missing certificate private key on https configuration") - else: - Warning('No certificate specified, using buildin self-signed certificates!') + pki_dh = https['pki']['dh'][dh_name] + dh_params = load_dh_parameters(pki_dh['parameters']) + dh_numbers = dh_params.parameter_numbers() + dh_bits = dh_numbers.p.bit_length() + if dh_bits < 2048: + raise ConfigError(f'Minimum DH key-size is 2048 bits') - server_block_list = [] + else: + Warning('No certificate specified, using build-in self-signed certificates. '\ + 'Do not use them in a production environment!') - # organize by vhosts - vhost_dict = https.get('virtual-host', {}) + # Check if server port is already in use by a different appliaction + listen_address = ['0.0.0.0'] + port = int(https['port']) + if 'listen_address' in https: + listen_address = https['listen_address'] - if not vhost_dict: - # no specified virtual hosts (server blocks); use default - server_block_list.append(default_server_block) - else: - for vhost in list(vhost_dict): - server_block = deepcopy(default_server_block) - data = vhost_dict.get(vhost, {}) - server_block['address'] = data.get('listen-address', '*') - server_block['port'] = data.get('port', '443') - server_block_list.append(server_block) - - for entry in server_block_list: - _address = entry.get('address') - _address = '0.0.0.0' if _address == '*' else _address - _port = entry.get('port') - proto = 'tcp' - if check_port_availability(_address, int(_port), proto) is not True and \ - not is_listen_port_bind_service(int(_port), 'nginx'): - raise ConfigError(f'"{proto}" port "{_port}" is used by another service') + for address in listen_address: + if not check_port_availability(address, port, 'tcp') and not is_listen_port_bind_service(port, 'nginx'): + raise ConfigError(f'TCP port "{port}" is used by another service!') verify_vrf(https) @@ -172,89 +149,61 @@ def verify(https): # If only key-based methods are enabled, # fail the commit if no valid key configurations are found if (not valid_keys_exist) and (not jwt_auth): - raise ConfigError('At least one HTTPS API key is required unless GraphQL token authentication is enabled') + raise ConfigError('At least one HTTPS API key is required unless GraphQL token authentication is enabled!') if (not valid_keys_exist) and jwt_auth: - Warning(f'API keys are not configured: the classic (non-GraphQL) API will be unavailable.') + Warning(f'API keys are not configured: classic (non-GraphQL) API will be unavailable!') return None def generate(https): if https is None: + for file in [systemd_service_api, config_file, systemd_override]: + if os.path.exists(file): + os.unlink(file) return None - if 'api' not in https: - if os.path.exists(systemd_service): - os.unlink(systemd_service) - else: - render(systemd_service, 'https/vyos-http-api.service.j2', https['api']) + if 'api' in https: + render(systemd_service_api, 'https/vyos-http-api.service.j2', https) with open(api_config_state, 'w') as f: json.dump(https['api'], f, indent=2) - - server_block_list = [] - - # organize by vhosts - - vhost_dict = https.get('virtual-host', {}) - - if not vhost_dict: - # no specified virtual hosts (server blocks); use default - server_block_list.append(default_server_block) else: - for vhost in list(vhost_dict): - server_block = deepcopy(default_server_block) - server_block['id'] = vhost - data = vhost_dict.get(vhost, {}) - server_block['address'] = data.get('listen-address', '*') - server_block['port'] = data.get('port', '443') - name = data.get('server-name', ['_']) - server_block['name'] = name - allow_client = data.get('allow-client', {}) - server_block['allow_client'] = allow_client.get('address', []) - server_block_list.append(server_block) + if os.path.exists(systemd_service_api): + os.unlink(systemd_service_api) # get certificate data - - cert_dict = https.get('certificates', {}) - - if 'certificate' in cert_dict: - cert_name = cert_dict['certificate'] + if 'certificates' in https and 'certificate' in https['certificates']: + cert_name = https['certificates']['certificate'] pki_cert = https['pki']['certificate'][cert_name] - cert_path = os.path.join(cert_dir, f'{cert_name}.pem') - key_path = os.path.join(key_dir, f'{cert_name}.pem') + cert_path = os.path.join(cert_dir, f'{cert_name}_cert.pem') + key_path = os.path.join(cert_dir, f'{cert_name}_key.pem') server_cert = str(wrap_certificate(pki_cert['certificate'])) - if 'ca-certificate' in cert_dict: - ca_cert = cert_dict['ca-certificate'] - server_cert += '\n' + str(wrap_certificate(https['pki']['ca'][ca_cert]['certificate'])) - write_file(cert_path, server_cert) - write_file(key_path, wrap_private_key(pki_cert['private']['key'])) + # Append CA certificate if specified to form a full chain + if 'ca_certificate' in https['certificates']: + ca_cert = https['certificates']['ca_certificate'] + server_cert += '\n' + str(wrap_certificate(https['pki']['ca'][ca_cert]['certificate'])) - vyos_cert_data = { - 'crt': cert_path, - 'key': key_path - } + write_file(cert_path, server_cert, user=user, group=group, mode=0o644) + write_file(key_path, wrap_private_key(pki_cert['private']['key']), + user=user, group=group, mode=0o600) - for block in server_block_list: - block['vyos_cert'] = vyos_cert_data + tmp_path = {'cert_path': cert_path, 'key_path': key_path} - if 'api' in list(https): - vhost_list = https.get('api-restrict', {}).get('virtual-host', []) - if not vhost_list: - for block in server_block_list: - block['api'] = True - else: - for block in server_block_list: - if block['id'] in vhost_list: - block['api'] = True + if 'dh_params' in https['certificates']: + dh_name = https['certificates']['dh_params'] + pki_dh = https['pki']['dh'][dh_name] + if 'parameters' in pki_dh: + dh_path = os.path.join(cert_dir, f'{dh_name}_dh.pem') + write_file(dh_path, wrap_dh_parameters(pki_dh['parameters']), + user=user, group=group, mode=0o600) + tmp_path.update({'dh_file' : dh_path}) - data = { - 'server_block_list': server_block_list, - } + https['certificates'].update(tmp_path) - render(config_file, 'https/nginx.default.j2', data) + render(config_file, 'https/nginx.default.j2', https) render(systemd_override, 'https/override.conf.j2', https) return None @@ -273,7 +222,7 @@ def apply(https): call(f'systemctl reload-or-restart {http_api_service_name}') # Let uvicorn settle before (possibly) restarting nginx sleep(1) - else: + elif is_systemd_service_active(http_api_service_name): call(f'systemctl stop {http_api_service_name}') call(f'systemctl reload-or-restart {https_service_name}') diff --git a/src/etc/systemd/system/nginx.service.d/10-override.conf b/src/etc/systemd/system/nginx.service.d/10-override.conf new file mode 100644 index 000000000..1be5cec81 --- /dev/null +++ b/src/etc/systemd/system/nginx.service.d/10-override.conf @@ -0,0 +1,3 @@ +[Unit] +After= +After=vyos-router.service diff --git a/src/migration-scripts/https/5-to-6 b/src/migration-scripts/https/5-to-6 index b4159f02f..6d6efd32c 100755 --- a/src/migration-scripts/https/5-to-6 +++ b/src/migration-scripts/https/5-to-6 @@ -16,12 +16,14 @@ # T5886: Add support for ACME protocol (LetsEncrypt), migrate https certbot # to new "pki certificate" CLI tree +# T5902: Remove virtual-host import os import sys from vyos.configtree import ConfigTree from vyos.defaults import directories +from vyos.utils.process import cmd vyos_certbot_dir = directories['certbot'] @@ -36,30 +38,68 @@ with open(file_name, 'r') as f: config = ConfigTree(config_file) -base = ['service', 'https', 'certificates'] +base = ['service', 'https'] if not config.exists(base): # Nothing to do sys.exit(0) -# both domain-name and email must be set on CLI - ensured by previous verify() -domain_names = config.return_values(base + ['certbot', 'domain-name']) -email = config.return_value(base + ['certbot', 'email']) -config.delete(base) - -# Set default certname based on domain-name -cert_name = 'https-' + domain_names[0].split('.')[0] -# Overwrite certname from previous certbot calls if available -if os.path.exists(f'{vyos_certbot_dir}/live'): - for cert in [f.path.split('/')[-1] for f in os.scandir(f'{vyos_certbot_dir}/live') if f.is_dir()]: - cert_name = cert - break - -for domain in domain_names: - config.set(['pki', 'certificate', cert_name, 'acme', 'domain-name'], value=domain, replace=False) +if config.exists(base + ['certificates']): + # both domain-name and email must be set on CLI - ensured by previous verify() + domain_names = config.return_values(base + ['certificates', 'certbot', 'domain-name']) + email = config.return_value(base + ['certificates', 'certbot', 'email']) + config.delete(base + ['certificates']) + + # Set default certname based on domain-name + cert_name = 'https-' + domain_names[0].split('.')[0] + # Overwrite certname from previous certbot calls if available + # We can not use python code like os.scandir due to filesystem permissions. + # This must be run as root + certbot_live = f'{vyos_certbot_dir}/live/' # we need the trailing / + if os.path.exists(certbot_live): + tmp = cmd(f'sudo find {certbot_live} -maxdepth 1 -type d') + tmp = tmp.split() # tmp = ['/config/auth/letsencrypt/live', '/config/auth/letsencrypt/live/router.vyos.net'] + tmp.remove(certbot_live) + cert_name = tmp[0].replace(certbot_live, '') + config.set(['pki', 'certificate', cert_name, 'acme', 'email'], value=email) + config.set_tag(['pki', 'certificate']) + for domain in domain_names: + config.set(['pki', 'certificate', cert_name, 'acme', 'domain-name'], value=domain, replace=False) + + # Update Webserver certificate + config.set(base + ['certificates', 'certificate'], value=cert_name) + +if config.exists(base + ['virtual-host']): + allow_client = [] + listen_port = [] + listen_address = [] + for virtual_host in config.list_nodes(base + ['virtual-host']): + allow_path = base + ['virtual-host', virtual_host, 'allow-client', 'address'] + if config.exists(allow_path): + tmp = config.return_values(allow_path) + allow_client.extend(tmp) + + port_path = base + ['virtual-host', virtual_host, 'listen-port'] + if config.exists(port_path): + tmp = config.return_value(port_path) + listen_port.append(tmp) + + listen_address_path = base + ['virtual-host', virtual_host, 'listen-address'] + if config.exists(listen_address_path): + tmp = config.return_value(listen_address_path) + listen_address.append(tmp) + + config.delete(base + ['virtual-host']) + for client in allow_client: + config.set(base + ['allow-client', 'address'], value=client, replace=False) + + # clear listen-address if "all" were specified + if '*' in listen_address: + listen_address = [] + for address in listen_address: + config.set(base + ['listen-address'], value=address, replace=False) + -# Update Webserver certificate -config.set(base + ['certificate'], value=cert_name) try: with open(file_name, 'w') as f: diff --git a/src/services/vyos-http-api-server b/src/services/vyos-http-api-server index b64e58132..40d442e30 100755 --- a/src/services/vyos-http-api-server +++ b/src/services/vyos-http-api-server @@ -1,6 +1,6 @@ #!/usr/share/vyos-http-api-tools/bin/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 @@ -13,8 +13,6 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -# -# import os import sys @@ -25,6 +23,7 @@ import logging import signal import traceback import threading + from time import sleep from typing import List, Union, Callable, Dict @@ -46,11 +45,12 @@ from ariadne.asgi import GraphQL from vyos.config import Config from vyos.configtree import ConfigTree from vyos.configdiff import get_config_diff -from vyos.configsession import ConfigSession, ConfigSessionError +from vyos.configsession import ConfigSession +from vyos.configsession import ConfigSessionError +from vyos.defaults import api_config_state import api.graphql.state -api_config_state = '/run/http-api-state' CFG_GROUP = 'vyattacfg' debug = True -- 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 'src') 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 From 07e802a2d3f98cdf29928bf321cc8b89cb41766c Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Tue, 9 Jan 2024 07:32:41 +0100 Subject: boot-config-loader: T1622: add missing groups to failsafe user This extends commit 86d1291ec5 ("[boot-config-loader] T1622: Add failsafe and back trace") and adds missing groups to the vyos user. Without this change the vyos user will only have operator (vyos@vyos>) privileges, even if this level is discontinued. One could hack himself up as the user has sudo rights, but rather place the user in the right groups from the beginning. NOTE: This user is only added if booted with "vyos-config-debug" and an error when the configuration can not be loaded at all. --- src/helpers/vyos-boot-config-loader.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/helpers/vyos-boot-config-loader.py b/src/helpers/vyos-boot-config-loader.py index 01b06526d..42de696ce 100755 --- a/src/helpers/vyos-boot-config-loader.py +++ b/src/helpers/vyos-boot-config-loader.py @@ -102,7 +102,8 @@ def failsafe(config_file_name): 'authentication', 'encrypted-password']) - cmd(f"useradd -s /bin/bash -G 'users,sudo' -m -N -p '{passwd}' vyos") + cmd(f"useradd --create-home --no-user-group --shell /bin/vbash --password '{passwd}' "\ + "--groups frr,frrvty,vyattacfg,sudo,adm,dip,disk vyos") if __name__ == '__main__': if len(sys.argv) < 2: -- cgit v1.2.3