From 4f511b633e2dcbed851a739c37464ead9ab6d72a Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Mon, 20 Nov 2023 10:13:21 +0100 Subject: [PATCH] http: T5762: api: make API socket backend communication the one and only default Why: Smoketests fail as they can not establish IPv6 connection to uvicorn backend server. https://github.com/vyos/vyos-1x/pull/2481 added a bunch of new smoketests. While debugging those failing, it was uncovered, that uvicorn only listens on IPv4 connections vyos@vyos# netstat -tulnp | grep 8080 (Not all processes could be identified, non-owned process info will not be shown, you would have to be root to see it all.) tcp 0 0 127.0.0.1:8080 0.0.0.0:* LISTEN - As the CLI already has an option to move the API communication from an IP to a UNIX domain socket, the best idea is to make this the default way of communication, as we never directly talk to the API server but rather use the NGINX reverse proxy. --- data/templates/https/nginx.default.j2 | 4 -- interface-definitions/https.xml.in | 7 -- .../include/version/https-version.xml.i | 2 +- smoketest/configs/http-api-basic | 66 +++++++++++++++++++ smoketest/scripts/cli/test_service_https.py | 31 ++++----- src/conf_mode/https.py | 5 -- src/etc/sysctl.d/30-vyos-router.conf | 8 +++ src/migration-scripts/https/4-to-5 | 56 ++++++++++++++++ src/services/vyos-http-api-server | 10 +-- 9 files changed, 146 insertions(+), 43 deletions(-) create mode 100644 smoketest/configs/http-api-basic create mode 100755 src/migration-scripts/https/4-to-5 diff --git a/data/templates/https/nginx.default.j2 b/data/templates/https/nginx.default.j2 index b541ff30974..468640b4b66 100644 --- a/data/templates/https/nginx.default.j2 +++ b/data/templates/https/nginx.default.j2 @@ -38,11 +38,7 @@ server { # proxy settings for HTTP API, if enabled; 503, if not location ~ ^/(retrieve|configure|config-file|image|container-image|generate|show|reset|docs|openapi.json|redoc|graphql) { {% if server.api %} -{% if server.api.socket %} proxy_pass http://unix:/run/api.sock; -{% else %} - proxy_pass http://localhost:{{ server.api.port }}; -{% endif %} proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Proto $scheme; proxy_read_timeout 600; diff --git a/interface-definitions/https.xml.in b/interface-definitions/https.xml.in index 5430193b538..448075b5b47 100644 --- a/interface-definitions/https.xml.in +++ b/interface-definitions/https.xml.in @@ -68,7 +68,6 @@ 1002 - #include HTTP API keys @@ -101,12 +100,6 @@ - - - Run server on Unix domain socket - - - GraphQL support diff --git a/interface-definitions/include/version/https-version.xml.i b/interface-definitions/include/version/https-version.xml.i index 11107697421..fa18278f38b 100644 --- a/interface-definitions/include/version/https-version.xml.i +++ b/interface-definitions/include/version/https-version.xml.i @@ -1,3 +1,3 @@ - + diff --git a/smoketest/configs/http-api-basic b/smoketest/configs/http-api-basic new file mode 100644 index 00000000000..98b2ebcf894 --- /dev/null +++ b/smoketest/configs/http-api-basic @@ -0,0 +1,66 @@ +interfaces { + ethernet eth0 { + address 192.0.2.1/31 + address 2001:db8::1234/64 + } + ethernet eth1 { + } + loopback lo { + } +} +service { + https { + api { + keys { + id 1 { + key S3cur3 + } + } + socket + } + } + ssh { + } +} +system { + config-management { + commit-revisions 100 + } + console { + device ttyS0 { + speed 115200 + } + } + host-name vyos + login { + user vyos { + authentication { + encrypted-password $6$2Ta6TWHd/U$NmrX0x9kexCimeOcYK1MfhMpITF9ELxHcaBU/znBq.X2ukQOj61fVI2UYP/xBzP4QtiTcdkgs7WOQMHWsRymO/ + plaintext-password "" + } + } + } + ntp { + server time1.vyos.net { + } + server time2.vyos.net { + } + server time3.vyos.net { + } + } + syslog { + global { + facility all { + level info + } + facility protocols { + level debug + } + } + } +} + + +// Warning: Do not remove the following line. +// vyos-config-version: "broadcast-relay@1:cluster@1:config-management@1:conntrack@1:conntrack-sync@1:dhcp-relay@2:dhcp-server@5:dhcpv6-server@1:dns-forwarding@3:firewall@5:https@2:interfaces@13:ipoe-server@1:ipsec@5:l2tp@3:lldp@1:mdns@1:nat@5:ntp@1:pppoe-server@5:pptp@2:qos@1:quagga@6:salt@1:snmp@2:ssh@2:sstp@3:system@19:vrrp@2:vyos-accel-ppp@2:wanloadbalance@3:webgui@1:webproxy@2:zone-policy@1" +// Release version: 1.3-rolling-202010241631 diff --git a/smoketest/scripts/cli/test_service_https.py b/smoketest/scripts/cli/test_service_https.py index a18e7dfacd8..4da85fadfb7 100755 --- a/smoketest/scripts/cli/test_service_https.py +++ b/smoketest/scripts/cli/test_service_https.py @@ -23,7 +23,7 @@ from base_vyostest_shim import VyOSUnitTestSHIM from base_vyostest_shim import ignore_warning from vyos.utils.file import read_file -from vyos.utils.process import run +from vyos.utils.process import process_named_running base_path = ['service', 'https'] pki_base = ['pki'] @@ -49,24 +49,28 @@ u8/3jHMM7sDwL3aWzW/zp54/LhCWUoLMjDdDEEigK4fal4ZF9aA9F0Ww """ +PROCESS_NAME = 'nginx' + class TestHTTPSService(VyOSUnitTestSHIM.TestCase): - def setUp(self): + @classmethod + def setUpClass(cls): + super(TestHTTPSService, cls).setUpClass() + # ensure we can also run this test on a live system - so lets clean # out the current configuration :) - self.cli_delete(base_path) - self.cli_delete(pki_base) + cls.cli_delete(cls, base_path) + cls.cli_delete(cls, pki_base) def tearDown(self): + # Check for running process + self.assertTrue(process_named_running(PROCESS_NAME)) + self.cli_delete(base_path) self.cli_delete(pki_base) self.cli_commit() - def test_default(self): - self.cli_set(base_path) - self.cli_commit() - - ret = run('sudo /usr/sbin/nginx -t') - self.assertEqual(ret, 0) + # Check for stopped process + self.assertFalse(process_named_running(PROCESS_NAME)) def test_server_block(self): vhost_id = 'example' @@ -82,9 +86,6 @@ def test_server_block(self): self.cli_commit() - ret = run('sudo /usr/sbin/nginx -t') - self.assertEqual(ret, 0) - nginx_config = read_file('/etc/nginx/sites-enabled/default') self.assertIn(f'listen {address}:{port} ssl;', nginx_config) self.assertIn(f'ssl_protocols TLSv1.2 TLSv1.3;', nginx_config) @@ -97,9 +98,6 @@ def test_certificate(self): self.cli_commit() - ret = run('sudo /usr/sbin/nginx -t') - self.assertEqual(ret, 0) - @ignore_warning(InsecureRequestWarning) def test_api_auth(self): vhost_id = 'example' @@ -107,7 +105,6 @@ def test_api_auth(self): port = '443' name = 'localhost' - self.cli_set(base_path + ['api', 'socket']) key = 'MySuperSecretVyOS' self.cli_set(base_path + ['api', 'keys', 'id', 'key-01', 'key', key]) diff --git a/src/conf_mode/https.py b/src/conf_mode/https.py index 010490c7eea..028a5007a85 100755 --- a/src/conf_mode/https.py +++ b/src/conf_mode/https.py @@ -215,14 +215,9 @@ def generate(https): api_data = vyos.defaults.api_data api_settings = https.get('api', {}) if api_settings: - port = api_settings.get('port', '') - if port: - api_data['port'] = port vhosts = https.get('api-restrict', {}).get('virtual-host', []) if vhosts: api_data['vhost'] = vhosts[:] - if 'socket' in list(api_settings): - api_data['socket'] = True if api_data: vhost_list = api_data.get('vhost', []) diff --git a/src/etc/sysctl.d/30-vyos-router.conf b/src/etc/sysctl.d/30-vyos-router.conf index 1c9b8999f8d..67d96969e0f 100644 --- a/src/etc/sysctl.d/30-vyos-router.conf +++ b/src/etc/sysctl.d/30-vyos-router.conf @@ -105,3 +105,11 @@ net.core.rps_sock_flow_entries = 32768 net.core.default_qdisc=fq_codel net.ipv4.tcp_congestion_control=bbr +# VRF - Virtual routing and forwarding +# When net.vrf.strict_mode=0 (default) it is possible to associate multiple +# VRF devices to the same table. Conversely, when net.vrf.strict_mode=1 a +# table can be associated to a single VRF device. +# +# A VRF table can be used by the VyOS CLI only once (ensured by verify()), +# this simply adds an additional Kernel safety net +net.vrf.strict_mode=1 diff --git a/src/migration-scripts/https/4-to-5 b/src/migration-scripts/https/4-to-5 new file mode 100755 index 00000000000..a503e0cb76f --- /dev/null +++ b/src/migration-scripts/https/4-to-5 @@ -0,0 +1,56 @@ +#!/usr/bin/env python3 +# +# Copyright (C) 2023 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 +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# T5762: http: api: smoketests fail as they can not establish IPv6 connection +# to uvicorn backend server, always make the UNIX domain socket the +# default way of communication + +import sys + +from vyos.configtree import ConfigTree + +if len(sys.argv) < 2: + print("Must specify file name!") + sys.exit(1) + +file_name = sys.argv[1] + +with open(file_name, 'r') as f: + config_file = f.read() + +config = ConfigTree(config_file) + +base = ['service', 'https'] +if not config.exists(base): + # Nothing to do + sys.exit(0) + +# Delete "socket" CLI option - we always use UNIX domain sockets for +# NGINX <-> API server communication +if config.exists(base + ['api', 'socket']): + config.delete(base + ['api', 'socket']) + +# There is no need for an API service port, as UNIX domain sockets +# are used +if config.exists(base + ['api', 'port']): + config.delete(base + ['api', 'port']) + +try: + with open(file_name, 'w') as f: + f.write(config.to_string()) +except OSError as e: + print("Failed to save the modified config: {}".format(e)) + sys.exit(1) diff --git a/src/services/vyos-http-api-server b/src/services/vyos-http-api-server index 3a9efb73ef7..daee2425796 100755 --- a/src/services/vyos-http-api-server +++ b/src/services/vyos-http-api-server @@ -825,15 +825,7 @@ def initialization(session: ConfigSession, app: FastAPI = app): if app.state.vyos_graphql: graphql_init(app) - if not server_config['socket']: - config = ApiServerConfig(app, - host=server_config["listen_address"], - port=int(server_config["port"]), - proxy_headers=True) - else: - config = ApiServerConfig(app, - uds="/run/api.sock", - proxy_headers=True) + config = ApiServerConfig(app, uds="/run/api.sock", proxy_headers=True) server = ApiServer(config) def run_server():