Merge pull request #2773 from c-po/https-rework-t5766

https: T5902: remove virtual-host configuration

6 files changed, 166 insertions, 174 deletions

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})
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 <http://www.gnu.org/licenses/>.
 
 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/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:
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 <http://www.gnu.org/licenses/>.
-#
-#
 
 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