From cbb6c944fea616547cec43f7f1ed6ea3cc4beb54 Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Tue, 22 Apr 2025 16:31:09 +0200 Subject: vyos.utils: T7122: fix IPv6 support in check_port_availability() Commit 4523e9c897b3 ("wireguard: T3763: Added check for listening port availability") added a function to check if a port is free to use or already occupied by a different running service. This has been done by trying to bind a socket to said given port. Unfortunately there is no support for IPv6 address-fdamily in both socketserver.TCPServer or socketserver.UDPServer. This must be done manually by deriving TCPServer and setting self.address_family for IPv6. The new implementation gets rid of both TCPServer and UDPServer and replaces it with a simple socket binding to a given IPv4/IPv6 address or any interface/ address if unspecified. In addition build time tests are added for the function to check for proper behavior during build time of vyos-1x. --- src/conf_mode/interfaces_wireguard.py | 2 +- src/conf_mode/load-balancing_haproxy.py | 2 +- src/tests/test_utils_network.py | 11 ++++++++++- 3 files changed, 12 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/conf_mode/interfaces_wireguard.py b/src/conf_mode/interfaces_wireguard.py index 192937dba..3ca6ecdca 100755 --- a/src/conf_mode/interfaces_wireguard.py +++ b/src/conf_mode/interfaces_wireguard.py @@ -97,7 +97,7 @@ def verify(wireguard): if 'port' in wireguard and 'port_changed' in wireguard: listen_port = int(wireguard['port']) - if check_port_availability('0.0.0.0', listen_port, 'udp') is not True: + if check_port_availability(None, listen_port, protocol='udp') is not True: raise ConfigError(f'UDP port {listen_port} is busy or unavailable and ' 'cannot be used for the interface!') diff --git a/src/conf_mode/load-balancing_haproxy.py b/src/conf_mode/load-balancing_haproxy.py index 5fd1beec9..16c9300c2 100644 --- a/src/conf_mode/load-balancing_haproxy.py +++ b/src/conf_mode/load-balancing_haproxy.py @@ -72,7 +72,7 @@ def verify(lb): raise ConfigError(f'"{front} service port" must be configured!') # Check if bind address:port are used by another service - tmp_address = front_config.get('address', '0.0.0.0') + tmp_address = front_config.get('address', None) tmp_port = front_config['port'] if check_port_availability(tmp_address, int(tmp_port), 'tcp') is not True and \ not is_listen_port_bind_service(int(tmp_port), 'haproxy'): diff --git a/src/tests/test_utils_network.py b/src/tests/test_utils_network.py index d68dec16f..92fde447d 100644 --- a/src/tests/test_utils_network.py +++ b/src/tests/test_utils_network.py @@ -1,4 +1,4 @@ -# Copyright (C) 2020-2024 VyOS maintainers and contributors +# Copyright (C) 2020-2025 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 @@ -43,3 +43,12 @@ class TestVyOSUtilsNetwork(TestCase): self.assertFalse(vyos.utils.network.is_loopback_addr('::2')) self.assertFalse(vyos.utils.network.is_loopback_addr('192.0.2.1')) + + def test_check_port_availability(self): + self.assertTrue(vyos.utils.network.check_port_availability('::1', 8080)) + self.assertTrue(vyos.utils.network.check_port_availability('127.0.0.1', 8080)) + self.assertTrue(vyos.utils.network.check_port_availability(None, 8080, protocol='udp')) + # We do not have 192.0.2.1 configured on this system + self.assertFalse(vyos.utils.network.check_port_availability('192.0.2.1', 443)) + # We do not have 2001:db8::1 configured on this system + self.assertFalse(vyos.utils.network.check_port_availability('2001:db8::1', 80, protocol='udp')) -- cgit v1.2.3 From b433f9d48141496926f9499808cb57067352e432 Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Mon, 28 Apr 2025 21:55:17 +0200 Subject: pki: T7122: remove duplicate list definition - can be auto generated changed_keys had the same content as the values inside the sync_translate dictionary. Infact they were both used together do defined changed CLI keys. The list for changed_keys is a list of all unique values inside the sync_translate dict. --- src/conf_mode/pki.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/conf_mode/pki.py b/src/conf_mode/pki.py index 724f97555..521af61df 100755 --- a/src/conf_mode/pki.py +++ b/src/conf_mode/pki.py @@ -1,6 +1,6 @@ #!/usr/bin/env python3 # -# Copyright (C) 2021-2024 VyOS maintainers and contributors +# Copyright (C) 2021-2025 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 @@ -150,14 +150,18 @@ def get_config(config=None): if len(argv) > 1 and argv[1] == 'certbot_renew': pki['certbot_renew'] = {} - changed_keys = ['ca', 'certificate', 'dh', 'key-pair', 'openssh', 'openvpn'] + # Walk through the list of sync_translate mapping and build a list + # which is later used to check if the node was changed in the CLI config + changed_keys = [] + for value in sync_translate.values(): + if value not in changed_keys: + changed_keys.append(value) + # Check for changes to said given keys in the CLI config for key in changed_keys: tmp = node_changed(conf, base + [key], recursive=True, expand_nodes=Diff.DELETE | Diff.ADD) - if 'changed' not in pki: pki.update({'changed':{}}) - pki['changed'].update({key.replace('-', '_') : tmp}) # We only merge on the defaults of there is a configuration at all -- cgit v1.2.3 From b93427874a0e502f83c3cc450663e079af214ea9 Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Mon, 28 Apr 2025 22:06:40 +0200 Subject: pki: T7122: place certbot behind reverse-proxy if cert used by haproxy If we detect that an ACME issued certificate is consumed by haproxy service, we will move the certbot webserver to localhost and a highport, to proxy the request via haproxy which is already using port 80. --- python/vyos/defaults.py | 4 ++++ src/conf_mode/pki.py | 37 +++++++++++++++++++++++++++++++++++-- 2 files changed, 39 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/python/vyos/defaults.py b/python/vyos/defaults.py index 7efccded6..1e6be6241 100644 --- a/python/vyos/defaults.py +++ b/python/vyos/defaults.py @@ -47,6 +47,10 @@ systemd_services = { 'snmpd' : 'snmpd.service', } +internal_ports = { + 'certbot_haproxy' : 65080, # Certbot running behing haproxy +} + config_status = '/tmp/vyos-config-status' api_config_state = '/run/http-api-state' frr_debug_enable = '/tmp/vyos.frr.debug' diff --git a/src/conf_mode/pki.py b/src/conf_mode/pki.py index 521af61df..c1ff80d8a 100755 --- a/src/conf_mode/pki.py +++ b/src/conf_mode/pki.py @@ -27,6 +27,7 @@ from vyos.configdict import node_changed from vyos.configdiff import Diff from vyos.configdiff import get_config_diff from vyos.defaults import directories +from vyos.defaults import internal_ports from vyos.pki import encode_certificate from vyos.pki import is_ca_certificate from vyos.pki import load_certificate @@ -42,6 +43,7 @@ from vyos.utils.dict import dict_search from vyos.utils.dict import dict_search_args from vyos.utils.dict import dict_search_recursive from vyos.utils.file import read_file +from vyos.utils.network import check_port_availability from vyos.utils.process import call from vyos.utils.process import cmd from vyos.utils.process import is_systemd_service_active @@ -128,8 +130,13 @@ def certbot_request(name: str, config: dict, dry_run: bool=True): f'--standalone --agree-tos --no-eff-email --expand --server {config["url"]} '\ f'--email {config["email"]} --key-type rsa --rsa-key-size {config["rsa_key_size"]} '\ f'{domains}' - if 'listen_address' in config: + # When ACME is used behind a reverse proxy, we always bind to localhost + # whatever the CLI listen-address is configured for. + if 'used_by' in config and 'haproxy' in config['used_by']: + tmp += f' --http-01-address 127.0.0.1 --http-01-port {internal_ports["certbot_haproxy"]}' + elif 'listen_address' in config: tmp += f' --http-01-address {config["listen_address"]}' + # verify() does not need to actually request a cert but only test for plausability if dry_run: tmp += ' --dry-run' @@ -223,7 +230,7 @@ def get_config(config=None): continue path = search['path'] - path_str = ' '.join(path + found_path) + path_str = ' '.join(path + found_path).replace('_','-') #print(f'PKI: Updating config: {path_str} {item_name}') if path[0] == 'interfaces': @@ -234,6 +241,24 @@ def get_config(config=None): if not D.node_changed_presence(path): set_dependents(path[1], conf) + # Check PKI certificates if they are generated by ACME. If they are, traverse + # the current configutration and determine the service where the certificate + # is used by. This is needed to check if we might need to start ACME behing + # a reverse proxy. + if 'certificate' in pki: + for name, cert_config in pki['certificate'].items(): + if 'acme' not in cert_config: + continue + if not dict_search('system.load_balancing.haproxy', pki): + continue + used_by = [] + for cert_list, cli_path in dict_search_recursive( + pki['system']['load_balancing']['haproxy'], 'certificate'): + if name in cert_list: + used_by.append('haproxy') + if used_by: + pki['certificate'][name]['acme'].update({'used_by': used_by}) + return pki def is_valid_certificate(raw_data): @@ -325,6 +350,14 @@ def verify(pki): raise ConfigError(f'An email address is required to request '\ f'certificate for "{name}" via ACME!') + listen_address = None + if 'listen_address' in cert_conf['acme']: + listen_address = cert_conf['acme']['listen_address'] + + if 'used_by' not in cert_conf['acme']: + if not check_port_availability(listen_address, 80): + raise ConfigError(f'Port 80 is not available for ACME challenge for certificate "{name}"!') + if 'certbot_renew' not in pki: # Only run the ACME command if something on this entity changed, # as this is time intensive -- cgit v1.2.3 From 6abf68da33aa71913872730e24396f366c4dc9fa Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Mon, 28 Apr 2025 22:08:08 +0200 Subject: haproxy: T7122: do not use f'ormat strings without variable --- src/conf_mode/load-balancing_haproxy.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/conf_mode/load-balancing_haproxy.py b/src/conf_mode/load-balancing_haproxy.py index 16c9300c2..92bf818dc 100644 --- a/src/conf_mode/load-balancing_haproxy.py +++ b/src/conf_mode/load-balancing_haproxy.py @@ -1,6 +1,6 @@ #!/usr/bin/env python3 # -# Copyright (C) 2023-2024 VyOS maintainers and contributors +# Copyright (C) 2023-2025 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 @@ -65,7 +65,7 @@ def verify(lb): return None if 'backend' not in lb or 'service' not in lb: - raise ConfigError(f'"service" and "backend" must be configured!') + raise ConfigError('Both "service" and "backend" must be configured!') for front, front_config in lb['service'].items(): if 'port' not in front_config: @@ -76,7 +76,7 @@ def verify(lb): tmp_port = front_config['port'] if check_port_availability(tmp_address, int(tmp_port), 'tcp') is not True and \ not is_listen_port_bind_service(int(tmp_port), 'haproxy'): - raise ConfigError(f'"TCP" port "{tmp_port}" is used by another service') + raise ConfigError(f'TCP port "{tmp_port}" is used by another service') if 'http_compression' in front_config: if front_config['mode'] != 'http': @@ -89,12 +89,12 @@ def verify(lb): if 'http_check' in back_config: http_check = back_config['http_check'] if 'expect' in http_check and 'status' in http_check['expect'] and 'string' in http_check['expect']: - raise ConfigError(f'"expect status" and "expect string" can not be configured together!') + raise ConfigError('"expect status" and "expect string" can not be configured together!') if 'health_check' in back_config: if back_config['mode'] != 'tcp': raise ConfigError(f'backend "{back}" can only be configured with {back_config["health_check"]} ' + - f'health-check whilst in TCP mode!') + 'health-check whilst in TCP mode!') if 'http_check' in back_config: raise ConfigError(f'backend "{back}" cannot be configured with both http-check and health-check!') @@ -198,7 +198,6 @@ def apply(lb): call(f'systemctl stop {systemd_service}') else: call(f'systemctl reload-or-restart {systemd_service}') - return None -- cgit v1.2.3 From f8b0d74eecabdd16cb0cd6239c8095ed6d2321e3 Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Mon, 28 Apr 2025 22:08:46 +0200 Subject: haproxy: T7122: automatically reverse-proxy to certbot Automatically render HaProxy rules to reverse-proxy ACME challanges when the requested certificate was issued using ACME. --- data/templates/load-balancing/haproxy.cfg.j2 | 13 +++++++++++-- src/conf_mode/load-balancing_haproxy.py | 23 +++++++++++++++-------- 2 files changed, 26 insertions(+), 10 deletions(-) (limited to 'src') diff --git a/data/templates/load-balancing/haproxy.cfg.j2 b/data/templates/load-balancing/haproxy.cfg.j2 index 2287cb815..400936474 100644 --- a/data/templates/load-balancing/haproxy.cfg.j2 +++ b/data/templates/load-balancing/haproxy.cfg.j2 @@ -64,9 +64,19 @@ frontend {{ front }}-http {% else %} bind [::]:80 v4v6 {% endif %} - redirect scheme https code 301 if !{ ssl_fc } +{% if front_config.ssl.acme_certificate is vyos_defined %} + acl acme_acl path_beg /.well-known/acme-challenge/ + use_backend certbot_{{ front }}_backend if acme_acl +{% endif %} + redirect scheme https code 301 if !acme_acl +{% endif %} + +{% if front_config.ssl.acme_certificate is vyos_defined %} +backend certbot_{{ front }}_backend + server fe_{{ front }}_acme 127.0.0.1:65080 {% endif %} + frontend {{ front }} {% set ssl_front = [] %} {% if front_config.ssl.certificate is vyos_defined and front_config.ssl.certificate is iterable %} @@ -248,6 +258,5 @@ backend {{ back }} {% if back_config.timeout.server is vyos_defined %} timeout server {{ back_config.timeout.server }}s {% endif %} - {% endfor %} {% endif %} diff --git a/src/conf_mode/load-balancing_haproxy.py b/src/conf_mode/load-balancing_haproxy.py index 92bf818dc..f176009a0 100644 --- a/src/conf_mode/load-balancing_haproxy.py +++ b/src/conf_mode/load-balancing_haproxy.py @@ -22,6 +22,7 @@ from shutil import rmtree from vyos.config import Config from vyos.configverify import verify_pki_certificate from vyos.configverify import verify_pki_ca_certificate +from vyos.defaults import internal_ports from vyos.utils.dict import dict_search from vyos.utils.process import call from vyos.utils.network import check_port_availability @@ -58,6 +59,14 @@ def get_config(config=None): with_recursive_defaults=True, with_pki=True) + lb['certbot_port'] = internal_ports['certbot_haproxy'] + + if 'service' in lb: + for front, front_config in lb['service'].items(): + for cert in dict_search('ssl.certificate', front_config) or []: + if dict_search(f'pki.certificate.{cert}.acme', lb): + lb['service'][front]['ssl'].update({'acme_certificate': {}}) + return lb def verify(lb): @@ -85,6 +94,9 @@ def verify(lb): raise ConfigError(f'service {front} must have at least one mime-type configured to use' f'http_compression!') + for cert in dict_search('ssl.certificate', front_config) or []: + verify_pki_certificate(lb, cert) + for back, back_config in lb['backend'].items(): if 'http_check' in back_config: http_check = back_config['http_check'] @@ -112,20 +124,15 @@ def verify(lb): if {'no_verify', 'ca_certificate'} <= set(back_config['ssl']): raise ConfigError(f'backend {back} cannot have both ssl options no-verify and ca-certificate set!') + tmp = dict_search('ssl.ca_certificate', back_config) + if tmp: verify_pki_ca_certificate(lb, tmp) + # Check if http-response-headers are configured in any frontend/backend where mode != http for group in ['service', 'backend']: for config_name, config in lb[group].items(): if 'http_response_headers' in config and config['mode'] != 'http': raise ConfigError(f'{group} {config_name} must be set to http mode to use http_response_headers!') - for front, front_config in lb['service'].items(): - for cert in dict_search('ssl.certificate', front_config) or []: - verify_pki_certificate(lb, cert) - - for back, back_config in lb['backend'].items(): - tmp = dict_search('ssl.ca_certificate', back_config) - if tmp: verify_pki_ca_certificate(lb, tmp) - def generate(lb): if not lb: -- cgit v1.2.3 From aff2835d7b6226e4b89f51e3f6133da26f3a07bf Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Sun, 4 May 2025 11:23:54 +0200 Subject: vyos.template: T7122: add Jinja2 clever function helper to read vyos.defaults Add a new category if Jinja2 operands. We already have filters and tests, but sometimes we would like to call a Python function without and data "|" piped to it - that's what they call a clever-function. {{ get_default_port(NAME) }} can be used to retrieve the value from vyos.defaults.internal_ports[NAME] within Jinja2. We no longer need to extend the dictionary with arbitrary data retrieved from vyos.defaults, we can now simply register another clever-function to the Jinja2 backend. --- python/vyos/template.py | 44 ++++++++++++++++++++++++++++++++++++++++++-- src/tests/test_template.py | 9 +++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/python/vyos/template.py b/python/vyos/template.py index 513490963..11e1cc50f 100755 --- a/python/vyos/template.py +++ b/python/vyos/template.py @@ -36,6 +36,7 @@ DEFAULT_TEMPLATE_DIR = directories["templates"] # Holds template filters registered via register_filter() _FILTERS = {} _TESTS = {} +_CLEVER_FUNCTIONS = {} # reuse Environments with identical settings to improve performance @functools.lru_cache(maxsize=2) @@ -58,6 +59,7 @@ def _get_environment(location=None): ) env.filters.update(_FILTERS) env.tests.update(_TESTS) + env.globals.update(_CLEVER_FUNCTIONS) return env @@ -77,7 +79,7 @@ def register_filter(name, func=None): "Filters can only be registered before rendering the first template" ) if name in _FILTERS: - raise ValueError(f"A filter with name {name!r} was registered already") + raise ValueError(f"A filter with name {name!r} was already registered") _FILTERS[name] = func return func @@ -97,10 +99,30 @@ def register_test(name, func=None): "Tests can only be registered before rendering the first template" ) if name in _TESTS: - raise ValueError(f"A test with name {name!r} was registered already") + raise ValueError(f"A test with name {name!r} was already registered") _TESTS[name] = func return func +def register_clever_function(name, func=None): + """Register a function to be available as test in templates under given name. + + It can also be used as a decorator, see below in this module for examples. + + :raise RuntimeError: + when trying to register a test after a template has been rendered already + :raise ValueError: when trying to register a name which was taken already + """ + if func is None: + return functools.partial(register_clever_function, name) + if _get_environment.cache_info().currsize: + raise RuntimeError( + "Clever functions can only be registered before rendering the" \ + "first template") + if name in _CLEVER_FUNCTIONS: + raise ValueError(f"A clever function with name {name!r} was already "\ + "registered") + _CLEVER_FUNCTIONS[name] = func + return func def render_to_string(template, content, formater=None, location=None): """Render a template from the template directory, raise on any errors. @@ -1052,3 +1074,21 @@ def vyos_defined(value, test_value=None, var_type=None): else: # Valid value and is matching optional argument if provided - return true return True + +@register_clever_function('get_default_port') +def get_default_port(service): + """ + Jinja2 plugin to retrieve common service port number from vyos.defaults + class form a Jinja2 template. This removes the need to hardcode, or pass in + the data using the general dictionary. + + Added to remove code complexity and make it easier to read. + + Example: + {{ get_default_port('certbot_haproxy') }} + """ + from vyos.defaults import internal_ports + if service not in internal_ports: + raise RuntimeError(f'Service "{service}" not found in internal ' \ + 'vyos.defaults.internal_ports dict!') + return internal_ports[service] diff --git a/src/tests/test_template.py b/src/tests/test_template.py index 6377f6da5..7cae867a0 100644 --- a/src/tests/test_template.py +++ b/src/tests/test_template.py @@ -190,3 +190,12 @@ class TestVyOSTemplate(TestCase): for group_name, group_config in data['ike_group'].items(): ciphers = vyos.template.get_esp_ike_cipher(group_config) self.assertIn(IKEv2_DEFAULT, ','.join(ciphers)) + + def test_get_default_port(self): + from vyos.defaults import internal_ports + + with self.assertRaises(RuntimeError): + vyos.template.get_default_port('UNKNOWN') + + self.assertEqual(vyos.template.get_default_port('certbot_haproxy'), + internal_ports['certbot_haproxy']) -- cgit v1.2.3 From 59957ad694043f41a7b1e9ee740b19c87f297867 Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Sun, 4 May 2025 11:35:33 +0200 Subject: haproxy: T7122: always reverse-proxy ACL for certbot Always enable the ACL entry to reverse-proxy requests to the path "/.well-known/acme-challenge/" when "redirect-http-to-https" is configured for a given HAProxy frontend service. This is an intentional design decision to simplify the implementation and reduce overall code complexity. It poses no risk: a missing path returns a 404, and an unavailable backend yields an error 503. This approach avoids a chicken-and-egg problem where certbot might try to request a certificate via reverse-proxy before the proxy config is actually generated and active. By always routing through HAProxy, we also eliminate downtime as port 80 does not need to be freed for certbot's standalone mode. --- data/templates/load-balancing/haproxy.cfg.j2 | 13 +- .../scripts/cli/test_load-balancing_haproxy.py | 142 +++++++++++++-------- src/conf_mode/load-balancing_haproxy.py | 9 -- src/conf_mode/pki.py | 15 ++- 4 files changed, 104 insertions(+), 75 deletions(-) (limited to 'src') diff --git a/data/templates/load-balancing/haproxy.cfg.j2 b/data/templates/load-balancing/haproxy.cfg.j2 index 400936474..7a6b86c10 100644 --- a/data/templates/load-balancing/haproxy.cfg.j2 +++ b/data/templates/load-balancing/haproxy.cfg.j2 @@ -53,8 +53,8 @@ defaults # Frontend {% if service is vyos_defined %} {% for front, front_config in service.items() %} - {% if front_config.redirect_http_to_https is vyos_defined %} +{% set certbot_backend_name = 'certbot_' ~ front ~ '_backend' %} frontend {{ front }}-http mode http {% if front_config.listen_address is vyos_defined %} @@ -64,19 +64,14 @@ frontend {{ front }}-http {% else %} bind [::]:80 v4v6 {% endif %} -{% if front_config.ssl.acme_certificate is vyos_defined %} acl acme_acl path_beg /.well-known/acme-challenge/ - use_backend certbot_{{ front }}_backend if acme_acl -{% endif %} + use_backend {{ certbot_backend_name }} if acme_acl redirect scheme https code 301 if !acme_acl -{% endif %} -{% if front_config.ssl.acme_certificate is vyos_defined %} -backend certbot_{{ front }}_backend - server fe_{{ front }}_acme 127.0.0.1:65080 +backend {{ certbot_backend_name }} + server acme_{{ front }} 127.0.0.1:{{ get_default_port('certbot_haproxy') }} {% endif %} - frontend {{ front }} {% set ssl_front = [] %} {% if front_config.ssl.certificate is vyos_defined and front_config.ssl.certificate is iterable %} diff --git a/smoketest/scripts/cli/test_load-balancing_haproxy.py b/smoketest/scripts/cli/test_load-balancing_haproxy.py index 077f1974f..6a410ffde 100755 --- a/smoketest/scripts/cli/test_load-balancing_haproxy.py +++ b/smoketest/scripts/cli/test_load-balancing_haproxy.py @@ -14,11 +14,14 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . +import re +import textwrap import unittest from base_vyostest_shim import VyOSUnitTestSHIM from vyos.configsession import ConfigSessionError +from vyos.template import get_default_port from vyos.utils.process import process_named_running from vyos.utils.file import read_file @@ -131,7 +134,25 @@ ZXLrtgVJR9W020qTurO2f91qfU8646n11hR9ObBB1IYbagOU0Pw1Nrq/FRp/u2tx 7i7xFz2WEiQeSCPaKYOiqM3t """ +haproxy_service_name = 'https_front' +haproxy_backend_name = 'bk-01' +def parse_haproxy_config() -> dict: + config_str = read_file(HAPROXY_CONF) + section_pattern = re.compile(r'^(global|defaults|frontend\s+\S+|backend\s+\S+)', re.MULTILINE) + sections = {} + + matches = list(section_pattern.finditer(config_str)) + + for i, match in enumerate(matches): + section_name = match.group(1).strip() + start = match.end() + end = matches[i + 1].start() if i + 1 < len(matches) else len(config_str) + section_body = config_str[start:end] + dedented_body = textwrap.dedent(section_body).strip() + sections[section_name] = dedented_body + + return sections class TestLoadBalancingReverseProxy(VyOSUnitTestSHIM.TestCase): def tearDown(self): # Check for running process @@ -146,14 +167,14 @@ class TestLoadBalancingReverseProxy(VyOSUnitTestSHIM.TestCase): self.assertFalse(process_named_running(PROCESS_NAME)) def base_config(self): - self.cli_set(base_path + ['service', 'https_front', 'mode', 'http']) - self.cli_set(base_path + ['service', 'https_front', 'port', '4433']) - self.cli_set(base_path + ['service', 'https_front', 'backend', 'bk-01']) + self.cli_set(base_path + ['service', haproxy_service_name, 'mode', 'http']) + self.cli_set(base_path + ['service', haproxy_service_name, 'port', '4433']) + self.cli_set(base_path + ['service', haproxy_service_name, 'backend', haproxy_backend_name]) - self.cli_set(base_path + ['backend', 'bk-01', 'mode', 'http']) - self.cli_set(base_path + ['backend', 'bk-01', 'server', 'bk-01', 'address', '192.0.2.11']) - self.cli_set(base_path + ['backend', 'bk-01', 'server', 'bk-01', 'port', '9090']) - self.cli_set(base_path + ['backend', 'bk-01', 'server', 'bk-01', 'send-proxy']) + self.cli_set(base_path + ['backend', haproxy_backend_name, 'mode', 'http']) + self.cli_set(base_path + ['backend', haproxy_backend_name, 'server', haproxy_backend_name, 'address', '192.0.2.11']) + self.cli_set(base_path + ['backend', haproxy_backend_name, 'server', haproxy_backend_name, 'port', '9090']) + self.cli_set(base_path + ['backend', haproxy_backend_name, 'server', haproxy_backend_name, 'send-proxy']) self.cli_set(base_path + ['global-parameters', 'max-connections', '1000']) @@ -167,15 +188,15 @@ class TestLoadBalancingReverseProxy(VyOSUnitTestSHIM.TestCase): self.cli_set(['pki', 'certificate', 'smoketest', 'certificate', valid_cert.replace('\n','')]) self.cli_set(['pki', 'certificate', 'smoketest', 'private', 'key', valid_cert_private_key.replace('\n','')]) - def test_01_lb_reverse_proxy_domain(self): + def test_reverse_proxy_domain(self): domains_bk_first = ['n1.example.com', 'n2.example.com', 'n3.example.com'] domain_bk_second = 'n5.example.com' - frontend = 'https_front' + frontend = 'vyos_smoketest' front_port = '4433' bk_server_first = '192.0.2.11' bk_server_second = '192.0.2.12' - bk_first_name = 'bk-01' - bk_second_name = 'bk-02' + bk_first_name = 'vyosbk-01' + bk_second_name = 'vyosbk-02' bk_server_port = '9090' mode = 'http' rule_ten = '10' @@ -241,9 +262,9 @@ class TestLoadBalancingReverseProxy(VyOSUnitTestSHIM.TestCase): self.assertIn(f'server {bk_second_name} {bk_server_second}:{bk_server_port}', config) self.assertIn(f'server {bk_second_name} {bk_server_second}:{bk_server_port} backup', config) - def test_02_lb_reverse_proxy_cert_not_exists(self): + def test_reverse_proxy_cert_not_exists(self): self.base_config() - self.cli_set(base_path + ['service', 'https_front', 'ssl', 'certificate', 'cert']) + self.cli_set(base_path + ['service', haproxy_service_name, 'ssl', 'certificate', 'cert']) with self.assertRaises(ConfigSessionError) as e: self.cli_commit() @@ -253,19 +274,19 @@ class TestLoadBalancingReverseProxy(VyOSUnitTestSHIM.TestCase): self.configure_pki() self.base_config() - self.cli_set(base_path + ['service', 'https_front', 'ssl', 'certificate', 'cert']) + self.cli_set(base_path + ['service', haproxy_service_name, 'ssl', 'certificate', 'cert']) with self.assertRaises(ConfigSessionError) as e: self.cli_commit() # self.assertIn('\nCertificate "cert" does not exist\n', str(e.exception)) - self.cli_delete(base_path + ['service', 'https_front', 'ssl', 'certificate', 'cert']) - self.cli_set(base_path + ['service', 'https_front', 'ssl', 'certificate', 'smoketest']) + self.cli_delete(base_path + ['service', haproxy_service_name, 'ssl', 'certificate', 'cert']) + self.cli_set(base_path + ['service', haproxy_service_name, 'ssl', 'certificate', 'smoketest']) self.cli_commit() - def test_03_lb_reverse_proxy_ca_not_exists(self): + def test_reverse_proxy_ca_not_exists(self): self.base_config() - self.cli_set(base_path + ['backend', 'bk-01', 'ssl', 'ca-certificate', 'ca-test']) + self.cli_set(base_path + ['backend', haproxy_backend_name, 'ssl', 'ca-certificate', 'ca-test']) with self.assertRaises(ConfigSessionError) as e: self.cli_commit() @@ -275,40 +296,40 @@ class TestLoadBalancingReverseProxy(VyOSUnitTestSHIM.TestCase): self.configure_pki() self.base_config() - self.cli_set(base_path + ['backend', 'bk-01', 'ssl', 'ca-certificate', 'ca-test']) + self.cli_set(base_path + ['backend', haproxy_backend_name, 'ssl', 'ca-certificate', 'ca-test']) with self.assertRaises(ConfigSessionError) as e: self.cli_commit() # self.assertIn('\nCA certificate "ca-test" does not exist\n', str(e.exception)) - self.cli_delete(base_path + ['backend', 'bk-01', 'ssl', 'ca-certificate', 'ca-test']) - self.cli_set(base_path + ['backend', 'bk-01', 'ssl', 'ca-certificate', 'smoketest']) + self.cli_delete(base_path + ['backend', haproxy_backend_name, 'ssl', 'ca-certificate', 'ca-test']) + self.cli_set(base_path + ['backend', haproxy_backend_name, 'ssl', 'ca-certificate', 'smoketest']) self.cli_commit() - def test_04_lb_reverse_proxy_backend_ssl_no_verify(self): + def test_reverse_proxy_backend_ssl_no_verify(self): # Setup base self.configure_pki() self.base_config() # Set no-verify option - self.cli_set(base_path + ['backend', 'bk-01', 'ssl', 'no-verify']) + self.cli_set(base_path + ['backend', haproxy_backend_name, 'ssl', 'no-verify']) self.cli_commit() # Test no-verify option config = read_file(HAPROXY_CONF) - self.assertIn('server bk-01 192.0.2.11:9090 send-proxy ssl verify none', config) + self.assertIn(f'server {haproxy_backend_name} 192.0.2.11:9090 send-proxy ssl verify none', config) # Test setting ca-certificate alongside no-verify option fails, to test config validation - self.cli_set(base_path + ['backend', 'bk-01', 'ssl', 'ca-certificate', 'smoketest']) + self.cli_set(base_path + ['backend', haproxy_backend_name, 'ssl', 'ca-certificate', 'smoketest']) with self.assertRaises(ConfigSessionError) as e: self.cli_commit() - def test_05_lb_reverse_proxy_backend_http_check(self): + def test_reverse_proxy_backend_http_check(self): # Setup base self.base_config() # Set http-check - self.cli_set(base_path + ['backend', 'bk-01', 'http-check', 'method', 'get']) + self.cli_set(base_path + ['backend', haproxy_backend_name, 'http-check', 'method', 'get']) self.cli_commit() # Test http-check @@ -317,8 +338,8 @@ class TestLoadBalancingReverseProxy(VyOSUnitTestSHIM.TestCase): self.assertIn('http-check send meth GET', config) # Set http-check with uri and status - self.cli_set(base_path + ['backend', 'bk-01', 'http-check', 'uri', '/health']) - self.cli_set(base_path + ['backend', 'bk-01', 'http-check', 'expect', 'status', '200']) + self.cli_set(base_path + ['backend', haproxy_backend_name, 'http-check', 'uri', '/health']) + self.cli_set(base_path + ['backend', haproxy_backend_name, 'http-check', 'expect', 'status', '200']) self.cli_commit() # Test http-check with uri and status @@ -328,8 +349,8 @@ class TestLoadBalancingReverseProxy(VyOSUnitTestSHIM.TestCase): self.assertIn('http-check expect status 200', config) # Set http-check with string - self.cli_delete(base_path + ['backend', 'bk-01', 'http-check', 'expect', 'status', '200']) - self.cli_set(base_path + ['backend', 'bk-01', 'http-check', 'expect', 'string', 'success']) + self.cli_delete(base_path + ['backend', haproxy_backend_name, 'http-check', 'expect', 'status', '200']) + self.cli_set(base_path + ['backend', haproxy_backend_name, 'http-check', 'expect', 'string', 'success']) self.cli_commit() # Test http-check with string @@ -339,11 +360,11 @@ class TestLoadBalancingReverseProxy(VyOSUnitTestSHIM.TestCase): self.assertIn('http-check expect string success', config) # Test configuring both http-check & health-check fails validation script - self.cli_set(base_path + ['backend', 'bk-01', 'health-check', 'ldap']) + self.cli_set(base_path + ['backend', haproxy_backend_name, 'health-check', 'ldap']) with self.assertRaises(ConfigSessionError) as e: self.cli_commit() - def test_06_lb_reverse_proxy_tcp_mode(self): + def test_reverse_proxy_tcp_mode(self): frontend = 'tcp_8443' mode = 'tcp' front_port = '8433' @@ -390,27 +411,27 @@ class TestLoadBalancingReverseProxy(VyOSUnitTestSHIM.TestCase): self.assertIn(f'mode {mode}', config) self.assertIn(f'server {bk_name} {bk_server}:{bk_server_port}', config) - def test_07_lb_reverse_proxy_http_response_headers(self): + def test_reverse_proxy_http_response_headers(self): # Setup base self.configure_pki() self.base_config() # Set example headers in both frontend and backend - self.cli_set(base_path + ['service', 'https_front', 'http-response-headers', 'Cache-Control', 'value', 'max-age=604800']) - self.cli_set(base_path + ['backend', 'bk-01', 'http-response-headers', 'Proxy-Backend-ID', 'value', 'bk-01']) + self.cli_set(base_path + ['service', haproxy_service_name, 'http-response-headers', 'Cache-Control', 'value', 'max-age=604800']) + self.cli_set(base_path + ['backend', haproxy_backend_name, 'http-response-headers', 'Proxy-Backend-ID', 'value', haproxy_backend_name]) self.cli_commit() # Test headers are present in generated configuration file config = read_file(HAPROXY_CONF) self.assertIn('http-response set-header Cache-Control \'max-age=604800\'', config) - self.assertIn('http-response set-header Proxy-Backend-ID \'bk-01\'', config) + self.assertIn(f'http-response set-header Proxy-Backend-ID \'{haproxy_backend_name}\'', config) # Test setting alongside modes other than http is blocked by validation conditions - self.cli_set(base_path + ['service', 'https_front', 'mode', 'tcp']) + self.cli_set(base_path + ['service', haproxy_service_name, 'mode', 'tcp']) with self.assertRaises(ConfigSessionError) as e: self.cli_commit() - def test_08_lb_reverse_proxy_tcp_health_checks(self): + def test_reverse_proxy_tcp_health_checks(self): # Setup PKI self.configure_pki() @@ -458,7 +479,7 @@ class TestLoadBalancingReverseProxy(VyOSUnitTestSHIM.TestCase): config = read_file(HAPROXY_CONF) self.assertIn(f'option smtpchk', config) - def test_09_lb_reverse_proxy_logging(self): + def test_reverse_proxy_logging(self): # Setup base self.base_config() self.cli_commit() @@ -477,7 +498,7 @@ class TestLoadBalancingReverseProxy(VyOSUnitTestSHIM.TestCase): self.assertIn('log /dev/log local2 warning', config) # Test backend logging options - backend_path = base_path + ['backend', 'bk-01'] + backend_path = base_path + ['backend', haproxy_backend_name] self.cli_set(backend_path + ['logging', 'facility', 'local3', 'level', 'debug']) self.cli_set(backend_path + ['logging', 'facility', 'local4', 'level', 'info']) self.cli_commit() @@ -488,7 +509,7 @@ class TestLoadBalancingReverseProxy(VyOSUnitTestSHIM.TestCase): self.assertIn('log /dev/log local4 info', config) # Test service logging options - service_path = base_path + ['service', 'https_front'] + service_path = base_path + ['service', haproxy_service_name] self.cli_set(service_path + ['logging', 'facility', 'local5', 'level', 'notice']) self.cli_set(service_path + ['logging', 'facility', 'local6', 'level', 'crit']) self.cli_commit() @@ -498,16 +519,17 @@ class TestLoadBalancingReverseProxy(VyOSUnitTestSHIM.TestCase): self.assertIn('log /dev/log local5 notice', config) self.assertIn('log /dev/log local6 crit', config) - def test_10_lb_reverse_proxy_http_compression(self): + def test_reverse_proxy_http_compression(self): # Setup base self.configure_pki() self.base_config() # Configure compression in frontend - self.cli_set(base_path + ['service', 'https_front', 'http-compression', 'algorithm', 'gzip']) - self.cli_set(base_path + ['service', 'https_front', 'http-compression', 'mime-type', 'text/html']) - self.cli_set(base_path + ['service', 'https_front', 'http-compression', 'mime-type', 'text/javascript']) - self.cli_set(base_path + ['service', 'https_front', 'http-compression', 'mime-type', 'text/plain']) + http_comp_path = base_path + ['service', haproxy_service_name, 'http-compression'] + self.cli_set(http_comp_path + ['algorithm', 'gzip']) + self.cli_set(http_comp_path + ['mime-type', 'text/html']) + self.cli_set(http_comp_path + ['mime-type', 'text/javascript']) + self.cli_set(http_comp_path + ['mime-type', 'text/plain']) self.cli_commit() # Test compression is present in generated configuration file @@ -517,11 +539,11 @@ class TestLoadBalancingReverseProxy(VyOSUnitTestSHIM.TestCase): self.assertIn('compression type text/html text/javascript text/plain', config) # Test setting compression without specifying any mime-types fails verification - self.cli_delete(base_path + ['service', 'https_front', 'http-compression', 'mime-type']) + self.cli_delete(base_path + ['service', haproxy_service_name, 'http-compression', 'mime-type']) with self.assertRaises(ConfigSessionError) as e: self.cli_commit() - def test_11_lb_haproxy_timeout(self): + def test_reverse_proxy_timeout(self): t_default_check = '5' t_default_client = '50' t_default_connect = '10' @@ -551,7 +573,7 @@ class TestLoadBalancingReverseProxy(VyOSUnitTestSHIM.TestCase): self.cli_set(base_path + ['timeout', 'client', t_client]) self.cli_set(base_path + ['timeout', 'connect', t_connect]) self.cli_set(base_path + ['timeout', 'server', t_server]) - self.cli_set(base_path + ['service', 'https_front', 'timeout', 'client', t_front_client]) + self.cli_set(base_path + ['service', haproxy_service_name, 'timeout', 'client', t_front_client]) self.cli_commit() @@ -569,5 +591,25 @@ class TestLoadBalancingReverseProxy(VyOSUnitTestSHIM.TestCase): for config_entry in config_entries: self.assertIn(config_entry, config) + def test_reverse_proxy_http_redirect(self): + self.base_config() + self.cli_set(base_path + ['service', haproxy_service_name, 'redirect-http-to-https']) + + self.cli_commit() + + config = parse_haproxy_config() + frontend_name = f'frontend {haproxy_service_name}-http' + self.assertIn(frontend_name, config.keys()) + self.assertIn('mode http', config[frontend_name]) + self.assertIn('bind [::]:80 v4v6', config[frontend_name]) + self.assertIn('acl acme_acl path_beg /.well-known/acme-challenge/', config[frontend_name]) + self.assertIn(f'use_backend certbot_{haproxy_service_name}_backend if acme_acl', config[frontend_name]) + self.assertIn('redirect scheme https code 301 if !acme_acl', config[frontend_name]) + + backend_name = f'backend certbot_{haproxy_service_name}_backend' + self.assertIn(backend_name, config.keys()) + port = get_default_port('certbot_haproxy') + self.assertIn(f'server acme_https_front 127.0.0.1:{port}', config[backend_name]) + if __name__ == '__main__': unittest.main(verbosity=2) diff --git a/src/conf_mode/load-balancing_haproxy.py b/src/conf_mode/load-balancing_haproxy.py index f176009a0..0e959480c 100644 --- a/src/conf_mode/load-balancing_haproxy.py +++ b/src/conf_mode/load-balancing_haproxy.py @@ -22,7 +22,6 @@ from shutil import rmtree from vyos.config import Config from vyos.configverify import verify_pki_certificate from vyos.configverify import verify_pki_ca_certificate -from vyos.defaults import internal_ports from vyos.utils.dict import dict_search from vyos.utils.process import call from vyos.utils.network import check_port_availability @@ -59,14 +58,6 @@ def get_config(config=None): with_recursive_defaults=True, with_pki=True) - lb['certbot_port'] = internal_ports['certbot_haproxy'] - - if 'service' in lb: - for front, front_config in lb['service'].items(): - for cert in dict_search('ssl.certificate', front_config) or []: - if dict_search(f'pki.certificate.{cert}.acme', lb): - lb['service'][front]['ssl'].update({'acme_certificate': {}}) - return lb def verify(lb): diff --git a/src/conf_mode/pki.py b/src/conf_mode/pki.py index c1ff80d8a..98922595c 100755 --- a/src/conf_mode/pki.py +++ b/src/conf_mode/pki.py @@ -231,7 +231,7 @@ def get_config(config=None): path = search['path'] path_str = ' '.join(path + found_path).replace('_','-') - #print(f'PKI: Updating config: {path_str} {item_name}') + print(f'PKI: Updating config: {path_str} {item_name}') if path[0] == 'interfaces': ifname = found_path[0] @@ -241,10 +241,10 @@ def get_config(config=None): if not D.node_changed_presence(path): set_dependents(path[1], conf) - # Check PKI certificates if they are generated by ACME. If they are, traverse - # the current configutration and determine the service where the certificate - # is used by. This is needed to check if we might need to start ACME behing - # a reverse proxy. + # Check PKI certificates if they are auto-generated by ACME. If they are, + # traverse the current configuration and determine the service where the + # certificate is used by. + # Required to check if we might need to run certbot behing a reverse proxy. if 'certificate' in pki: for name, cert_config in pki['certificate'].items(): if 'acme' not in cert_config: @@ -252,7 +252,7 @@ def get_config(config=None): if not dict_search('system.load_balancing.haproxy', pki): continue used_by = [] - for cert_list, cli_path in dict_search_recursive( + for cert_list, _ in dict_search_recursive( pki['system']['load_balancing']['haproxy'], 'certificate'): if name in cert_list: used_by.append('haproxy') @@ -356,7 +356,8 @@ def verify(pki): if 'used_by' not in cert_conf['acme']: if not check_port_availability(listen_address, 80): - raise ConfigError(f'Port 80 is not available for ACME challenge for certificate "{name}"!') + raise ConfigError('Port 80 is already in use and not available '\ + f'to provide ACME challenge for "{name}"!') if 'certbot_renew' not in pki: # Only run the ACME command if something on this entity changed, -- cgit v1.2.3 From 40a99b1d00d822ef6f1a3772768919d14c3500d9 Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Sun, 4 May 2025 12:17:06 +0200 Subject: vyos.base: T7122: add new Message() helper wrapper for print() This will wrap the messages at 72 characters in the same way as Warning() and DeprecationWarning() would do. We now have simple wrappers for it! Example: vyos@vyos# commit [ pki ] Updating configuration: "load-balancing haproxy service frontend ssl certificate LE_cloud" Add/replace automatically imported CA certificate for "LE_cloud" --- python/vyos/base.py | 21 +++++++++++++-------- src/conf_mode/pki.py | 5 +++-- 2 files changed, 16 insertions(+), 10 deletions(-) (limited to 'src') diff --git a/python/vyos/base.py b/python/vyos/base.py index ca96d96ce..3173ddc20 100644 --- a/python/vyos/base.py +++ b/python/vyos/base.py @@ -1,4 +1,4 @@ -# Copyright 2018-2022 VyOS maintainers and contributors +# Copyright 2018-2025 VyOS maintainers and contributors # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -15,8 +15,7 @@ from textwrap import fill - -class BaseWarning: +class UserMessage: def __init__(self, header, message, **kwargs): self.message = message self.kwargs = kwargs @@ -33,7 +32,6 @@ class BaseWarning: messages = self.message.split('\n') isfirstmessage = True initial_indent = self.textinitindent - print('') for mes in messages: mes = fill(mes, initial_indent=initial_indent, subsequent_indent=self.standardindent, **self.kwargs) @@ -44,17 +42,24 @@ class BaseWarning: print('', flush=True) +class Message(): + def __init__(self, message, **kwargs): + self.Message = UserMessage('', message, **kwargs) + self.Message.print() + class Warning(): def __init__(self, message, **kwargs): - self.BaseWarn = BaseWarning('WARNING: ', message, **kwargs) - self.BaseWarn.print() + print('') + self.UserMessage = UserMessage('WARNING: ', message, **kwargs) + self.UserMessage.print() class DeprecationWarning(): def __init__(self, message, **kwargs): # Reformat the message and trim it to 72 characters in length - self.BaseWarn = BaseWarning('DEPRECATION WARNING: ', message, **kwargs) - self.BaseWarn.print() + print('') + self.UserMessage = UserMessage('DEPRECATION WARNING: ', message, **kwargs) + self.UserMessage.print() class ConfigError(Exception): diff --git a/src/conf_mode/pki.py b/src/conf_mode/pki.py index 98922595c..6957248d2 100755 --- a/src/conf_mode/pki.py +++ b/src/conf_mode/pki.py @@ -19,6 +19,7 @@ import os from sys import argv from sys import exit +from vyos.base import Message from vyos.config import Config from vyos.config import config_dict_merge from vyos.configdep import set_dependents @@ -231,7 +232,7 @@ def get_config(config=None): path = search['path'] path_str = ' '.join(path + found_path).replace('_','-') - print(f'PKI: Updating config: {path_str} {item_name}') + Message(f'Updating configuration: "{path_str} {item_name}"') if path[0] == 'interfaces': ifname = found_path[0] @@ -528,7 +529,7 @@ def generate(pki): if not ca_cert_present: tmp = dict_search_args(pki, 'ca', f'{autochain_prefix}{cert}', 'certificate') if not bool(tmp) or tmp != cert_chain_base64: - print(f'Adding/replacing automatically imported CA certificate for "{cert}" ...') + Message(f'Add/replace automatically imported CA certificate for "{cert}"...') add_cli_node(['pki', 'ca', f'{autochain_prefix}{cert}', 'certificate'], value=cert_chain_base64) return None -- cgit v1.2.3 From c05edd62cf1120fb14b66ca0377061a59a9d00db Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Sun, 4 May 2025 21:48:32 +0200 Subject: pki: T7122: extend ca/certificate removal check to lists Some VyOS CLI nodes support defining multiple certificates. The previous check when removing a certificate from the CLI only performed a string comparison, which failed in cases where the underlying data was a list (CLI node). This update extends the check to handle both cases: - If the datum is a string, perform a string comparison. - If the datum is a list, check whether the target certificate is part of the list. This ensures proper removal behavior regardless of the data type used in the CLI node. --- src/conf_mode/pki.py | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) (limited to 'src') diff --git a/src/conf_mode/pki.py b/src/conf_mode/pki.py index 6957248d2..f53e5db8b 100755 --- a/src/conf_mode/pki.py +++ b/src/conf_mode/pki.py @@ -413,27 +413,35 @@ def verify(pki): for search in sync_search: for key in search['keys']: changed_key = sync_translate[key] - if changed_key not in pki['changed']: continue - for item_name in pki['changed'][changed_key]: node_present = False if changed_key == 'openvpn': node_present = dict_search_args(pki, 'openvpn', 'shared_secret', item_name) else: node_present = dict_search_args(pki, changed_key, item_name) + # If the node is still present, we can skip the check + # as we are not deleting it + if node_present: + continue - if not node_present: - search_dict = dict_search_args(pki['system'], *search['path']) - - if not search_dict: - continue + search_dict = dict_search_args(pki['system'], *search['path']) + if not search_dict: + continue - for found_name, found_path in dict_search_recursive(search_dict, key): - if found_name == item_name: - path_str = " ".join(search['path'] + found_path) - raise ConfigError(f'PKI object "{item_name}" still in use by "{path_str}"') + for found_name, found_path in dict_search_recursive(search_dict, key): + # Check if the name matches either by string compare, or beeing + # part of a list + if ((isinstance(found_name, str) and found_name == item_name) or + (isinstance(found_name, list) and item_name in found_name)): + # We do not support _ in CLI paths - this is only a convenience + # as we mangle all - to _, now it's time to reverse this! + path_str = ' '.join(search['path'] + found_path).replace('_','-') + object = changed_key.replace('_','-') + tmp = f'Embedded PKI {object} with name "{item_name}" is still '\ + f'in use by CLI path "{path_str}"' + raise ConfigError(tmp) return None -- cgit v1.2.3 From 59d86826a2ffb2df6a0ce603c879e541a4fe88ba Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Sun, 4 May 2025 22:08:13 +0200 Subject: haproxy: T7122: add ACME/certbot bootstrap support When both the CLI PKI node for an ACME-issued certificate and HAProxy are configured during initial setup, the certbot challenge cannot be served via the reverse proxy because HAProxy has not yet been configured at all. This commit introduces a special case to handle this bootstrap scenario, ensuring that the certbot challenge can still be served correctly in standalone mode on port 80 despite initial config dependencies/priorities between PKI and HAProxy. --- python/vyos/defaults.py | 1 + src/conf_mode/load-balancing_haproxy.py | 10 +++++----- src/conf_mode/pki.py | 5 ++++- 3 files changed, 10 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/python/vyos/defaults.py b/python/vyos/defaults.py index 1e6be6241..c1e5ddc04 100644 --- a/python/vyos/defaults.py +++ b/python/vyos/defaults.py @@ -43,6 +43,7 @@ directories = { } systemd_services = { + 'haproxy' : 'haproxy.service', 'syslog' : 'syslog.service', 'snmpd' : 'snmpd.service', } diff --git a/src/conf_mode/load-balancing_haproxy.py b/src/conf_mode/load-balancing_haproxy.py index 0e959480c..504a90596 100644 --- a/src/conf_mode/load-balancing_haproxy.py +++ b/src/conf_mode/load-balancing_haproxy.py @@ -19,6 +19,7 @@ import os from sys import exit from shutil import rmtree +from vyos.defaults import systemd_services from vyos.config import Config from vyos.configverify import verify_pki_certificate from vyos.configverify import verify_pki_ca_certificate @@ -39,7 +40,6 @@ airbag.enable() load_balancing_dir = '/run/haproxy' load_balancing_conf_file = f'{load_balancing_dir}/haproxy.cfg' -systemd_service = 'haproxy.service' systemd_override = '/run/systemd/system/haproxy.service.d/10-override.conf' def get_config(config=None): @@ -191,11 +191,11 @@ def generate(lb): return None def apply(lb): + action = 'stop' + if lb: + action = 'reload-or-restart' call('systemctl daemon-reload') - if not lb: - call(f'systemctl stop {systemd_service}') - else: - call(f'systemctl reload-or-restart {systemd_service}') + call(f'systemctl {action} {systemd_services["haproxy"]}') return None diff --git a/src/conf_mode/pki.py b/src/conf_mode/pki.py index f53e5db8b..7ee1705c0 100755 --- a/src/conf_mode/pki.py +++ b/src/conf_mode/pki.py @@ -29,6 +29,7 @@ from vyos.configdiff import Diff from vyos.configdiff import get_config_diff from vyos.defaults import directories from vyos.defaults import internal_ports +from vyos.defaults import systemd_services from vyos.pki import encode_certificate from vyos.pki import is_ca_certificate from vyos.pki import load_certificate @@ -48,6 +49,7 @@ from vyos.utils.network import check_port_availability from vyos.utils.process import call from vyos.utils.process import cmd from vyos.utils.process import is_systemd_service_active +from vyos.utils.process import is_systemd_service_running from vyos import ConfigError from vyos import airbag airbag.enable() @@ -133,7 +135,8 @@ def certbot_request(name: str, config: dict, dry_run: bool=True): f'{domains}' # When ACME is used behind a reverse proxy, we always bind to localhost # whatever the CLI listen-address is configured for. - if 'used_by' in config and 'haproxy' in config['used_by']: + if ('haproxy' in dict_search('used_by', config) and + is_systemd_service_running(systemd_services['haproxy'])): tmp += f' --http-01-address 127.0.0.1 --http-01-port {internal_ports["certbot_haproxy"]}' elif 'listen_address' in config: tmp += f' --http-01-address {config["listen_address"]}' -- cgit v1.2.3 From f40cf6064a02fbb6baae924e94b9183d6bd87474 Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Mon, 5 May 2025 17:20:44 +0200 Subject: pki: T7122: when ACME listen-address is used - check if port is available When instructing certbot to listen on a given address, check if the address is free to use. Also take this into account when spawning certbot behind HAProxy. If the address is not (yet) bound - the request must be done in standalone mode and not via the reverse-proxy. --- data/templates/load-balancing/haproxy.cfg.j2 | 9 +++++---- smoketest/scripts/cli/test_load-balancing_haproxy.py | 6 +++--- src/conf_mode/pki.py | 12 +++++++++--- 3 files changed, 17 insertions(+), 10 deletions(-) (limited to 'src') diff --git a/data/templates/load-balancing/haproxy.cfg.j2 b/data/templates/load-balancing/haproxy.cfg.j2 index 7a6b86c10..62934c612 100644 --- a/data/templates/load-balancing/haproxy.cfg.j2 +++ b/data/templates/load-balancing/haproxy.cfg.j2 @@ -50,6 +50,10 @@ defaults errorfile 503 /etc/haproxy/errors/503.http errorfile 504 /etc/haproxy/errors/504.http +# Default ACME backend +backend buildin_acme_certbot + server localhost 127.0.0.1:{{ get_default_port('certbot_haproxy') }} + # Frontend {% if service is vyos_defined %} {% for front, front_config in service.items() %} @@ -65,11 +69,8 @@ frontend {{ front }}-http bind [::]:80 v4v6 {% endif %} acl acme_acl path_beg /.well-known/acme-challenge/ - use_backend {{ certbot_backend_name }} if acme_acl + use_backend buildin_acme_certbot if acme_acl redirect scheme https code 301 if !acme_acl - -backend {{ certbot_backend_name }} - server acme_{{ front }} 127.0.0.1:{{ get_default_port('certbot_haproxy') }} {% endif %} frontend {{ front }} diff --git a/smoketest/scripts/cli/test_load-balancing_haproxy.py b/smoketest/scripts/cli/test_load-balancing_haproxy.py index 6a410ffde..833e0a92b 100755 --- a/smoketest/scripts/cli/test_load-balancing_haproxy.py +++ b/smoketest/scripts/cli/test_load-balancing_haproxy.py @@ -603,13 +603,13 @@ class TestLoadBalancingReverseProxy(VyOSUnitTestSHIM.TestCase): self.assertIn('mode http', config[frontend_name]) self.assertIn('bind [::]:80 v4v6', config[frontend_name]) self.assertIn('acl acme_acl path_beg /.well-known/acme-challenge/', config[frontend_name]) - self.assertIn(f'use_backend certbot_{haproxy_service_name}_backend if acme_acl', config[frontend_name]) + self.assertIn('use_backend buildin_acme_certbot if acme_acl', config[frontend_name]) self.assertIn('redirect scheme https code 301 if !acme_acl', config[frontend_name]) - backend_name = f'backend certbot_{haproxy_service_name}_backend' + backend_name = 'backend buildin_acme_certbot' self.assertIn(backend_name, config.keys()) port = get_default_port('certbot_haproxy') - self.assertIn(f'server acme_https_front 127.0.0.1:{port}', config[backend_name]) + self.assertIn(f'server localhost 127.0.0.1:{port}', config[backend_name]) if __name__ == '__main__': unittest.main(verbosity=2) diff --git a/src/conf_mode/pki.py b/src/conf_mode/pki.py index 7ee1705c0..869518dd9 100755 --- a/src/conf_mode/pki.py +++ b/src/conf_mode/pki.py @@ -133,13 +133,19 @@ def certbot_request(name: str, config: dict, dry_run: bool=True): f'--standalone --agree-tos --no-eff-email --expand --server {config["url"]} '\ f'--email {config["email"]} --key-type rsa --rsa-key-size {config["rsa_key_size"]} '\ f'{domains}' + + listen_address = None + if 'listen_address' in config: + listen_address = config['listen_address'] + # When ACME is used behind a reverse proxy, we always bind to localhost # whatever the CLI listen-address is configured for. if ('haproxy' in dict_search('used_by', config) and - is_systemd_service_running(systemd_services['haproxy'])): + is_systemd_service_running(systemd_services['haproxy']) and + not check_port_availability(listen_address, 80)): tmp += f' --http-01-address 127.0.0.1 --http-01-port {internal_ports["certbot_haproxy"]}' - elif 'listen_address' in config: - tmp += f' --http-01-address {config["listen_address"]}' + elif listen_address: + tmp += f' --http-01-address {listen_address}' # verify() does not need to actually request a cert but only test for plausability if dry_run: -- cgit v1.2.3