From 7c438caa2c21101cbefc2eec21935ab55af19c46 Mon Sep 17 00:00:00 2001 From: Viacheslav Hletenko Date: Tue, 14 May 2024 16:47:29 +0000 Subject: T3420: Remove service upnp Remove `service upnp` as it never worked as expected, nft rules do not integrated and custom patches do not seem like a suitable solution for now. Security: UPnP has been historically associated with security risks due to its automatic and potentially unauthenticated nature. UPnP devices might be vulnerable to unauthorized access or exploitation. --- data/configd-include.json | 1 - data/templates/firewall/upnpd.conf.j2 | 227 ---------------------------- debian/control | 3 - interface-definitions/service_upnp.xml.in | 229 ----------------------------- smoketest/scripts/cli/test_service_upnp.py | 103 ------------- src/conf_mode/service_upnp.py | 157 -------------------- src/systemd/miniupnpd.service | 13 -- 7 files changed, 733 deletions(-) delete mode 100644 data/templates/firewall/upnpd.conf.j2 delete mode 100644 interface-definitions/service_upnp.xml.in delete mode 100755 smoketest/scripts/cli/test_service_upnp.py delete mode 100755 src/conf_mode/service_upnp.py delete mode 100644 src/systemd/miniupnpd.service diff --git a/data/configd-include.json b/data/configd-include.json index 0c767f987..dcee50306 100644 --- a/data/configd-include.json +++ b/data/configd-include.json @@ -81,7 +81,6 @@ "service_sla.py", "service_ssh.py", "service_tftp-server.py", -"service_upnp.py", "service_webproxy.py", "system_acceleration.py", "system_config-management.py", diff --git a/data/templates/firewall/upnpd.conf.j2 b/data/templates/firewall/upnpd.conf.j2 deleted file mode 100644 index 616e8869f..000000000 --- a/data/templates/firewall/upnpd.conf.j2 +++ /dev/null @@ -1,227 +0,0 @@ -# This is the UPNP configuration file - -# WAN network interface -ext_ifname={{ wan_interface }} -{% if wan_ip is vyos_defined %} - -# if the WAN network interface for IPv6 is different than for IPv4, -# set ext_ifname6 -#ext_ifname6=eth2 - -# If the WAN interface has several IP addresses, you -# can specify the one to use below. -# Setting ext_ip is also useful in double NAT setup, you can declare here -# the public IP address. -{% for addr in wan_ip %} -ext_ip={{ addr }} -{% endfor %} -{% endif %} - -{% if stun is vyos_defined %} -# WAN interface must have public IP address. Otherwise it is behind NAT -# and port forwarding is impossible. In some cases WAN interface can be -# behind unrestricted full-cone NAT 1:1 when all incoming traffic is NAT-ed and -# routed to WAN interfaces without any filtering. In this cases miniupnpd -# needs to know public IP address and it can be learnt by asking external -# server via STUN protocol. Following option enable retrieving external -# public IP address from STUN server and detection of NAT type. You need -# to specify also external STUN server in stun_host option below. -# This option is disabled by default. -ext_perform_stun=yes -# Specify STUN server, either hostname or IP address -# Some public STUN servers: -# stun.stunprotocol.org -# stun.sipgate.net -# stun.xten.com -# stun.l.google.com (on non standard port 19302) -ext_stun_host={{ stun.host }} -# Specify STUN UDP port, by default it is standard port 3478. -ext_stun_port={{ stun.port }} -{% endif %} - -# LAN network interfaces IPs / networks -{% if listen is vyos_defined %} -# There can be multiple listening IPs for SSDP traffic, in that case -# use multiple 'listening_ip=...' lines, one for each network interface. -# It can be IP address or network interface name (ie. "eth0") -# It is mandatory to use the network interface name in order to enable IPv6 -# HTTP is available on all interfaces. -# When MULTIPLE_EXTERNAL_IP is enabled, the external IP -# address associated with the subnet follows. For example: -# listening_ip=192.168.0.1/24 88.22.44.13 -# When MULTIPLE_EXTERNAL_IP is disabled, you can list associated network -# interfaces (for bridges) -# listening_ip=bridge0 em0 wlan0 -{% for addr in listen %} -{% if addr | is_ipv4 %} -listening_ip={{ addr }} -{% elif addr | is_ipv6 %} -ipv6_listening_ip={{ addr }} -{% else %} -listening_ip={{ addr }} -{% endif %} -{% endfor %} -{% endif %} - -# CAUTION: mixing up WAN and LAN interfaces may introduce security risks! -# Be sure to assign the correct interfaces to LAN and WAN and consider -# implementing UPnP permission rules at the bottom of this configuration file - -# Port for HTTP (descriptions and SOAP) traffic. Set to 0 for autoselect. -#http_port=0 -# Port for HTTPS. Set to 0 for autoselect (default) -#https_port=0 - -# Path to the UNIX socket used to communicate with MiniSSDPd -# If running, MiniSSDPd will manage M-SEARCH answering. -# default is /var/run/minissdpd.sock -#minissdpdsocket=/var/run/minissdpd.sock - -{% if nat_pmp is vyos_defined %} -# Enable NAT-PMP support (default is no) -enable_natpmp=yes -{% endif %} - -# Enable UPNP support (default is yes) -enable_upnp=yes - -{% if pcp_lifetime is vyos_defined %} -# PCP -# Configure the minimum and maximum lifetime of a port mapping in seconds -# 120s and 86400s (24h) are suggested values from PCP-base -{% if pcp_lifetime.max is vyos_defined %} -max_lifetime={{ pcp_lifetime.max }} -{% endif %} -{% if pcp_lifetime.min is vyos_defined %} -min_lifetime={{ pcp_lifetime.min }} -{% endif %} -{% endif %} - -# table names for netfilter nft. Default is "filter" for both -#upnp_table_name= -#upnp_nat_table_name= -# chain names for netfilter and netfilter nft -# netfilter : default are MINIUPNPD, MINIUPNPD, MINIUPNPD-POSTROUTING -# netfilter nft : default are miniupnpd, prerouting_miniupnpd, postrouting_miniupnpd -#upnp_forward_chain=forwardUPnP -#upnp_nat_chain=UPnP -#upnp_nat_postrouting_chain=UPnP-Postrouting - -# Lease file location -lease_file=/config/upnp.leases - -# To enable the next few runtime options, see compile time -# ENABLE_MANUFACTURER_INFO_CONFIGURATION (config.h) - -{% if friendly_name is vyos_defined %} -# Name of this service, default is "`uname -s` router" -friendly_name={{ friendly_name }} -{% endif %} - -# Manufacturer name, default is "`uname -s`" -manufacturer_name=VyOS - -# Manufacturer URL, default is URL of OS vendor -manufacturer_url=https://vyos.io/ - -# Model name, default is "`uname -s` router" -model_name=VyOS Router Model - -# Model description, default is "`uname -s` router" -model_description=Vyos open source enterprise router/firewall operating system - -# Model URL, default is URL of OS vendor -model_url=https://vyos.io/ - -# Bitrates reported by daemon in bits per second -# by default miniupnpd tries to get WAN interface speed -#bitrate_up=1000000 -#bitrate_down=10000000 - -{% if secure_mode is vyos_defined %} -# Secure Mode, UPnP clients can only add mappings to their own IP -secure_mode=yes -{% else %} -# Secure Mode, UPnP clients can only add mappings to their own IP -secure_mode=no -{% endif %} - -{% if presentation_url is vyos_defined %} -# Default presentation URL is HTTP address on port 80 -# If set to an empty string, no presentationURL element will appear -# in the XML description of the device, which prevents MS Windows -# from displaying an icon in the "Network Connections" panel. -#presentation_url= {{ presentation_url }} -{% endif %} - -# Report system uptime instead of daemon uptime -system_uptime=yes - -# Notify interval in seconds. default is 30 seconds. -#notify_interval=240 -notify_interval=60 - -# Unused rules cleaning. -# never remove any rule before this threshold for the number -# of redirections is exceeded. default to 20 -clean_ruleset_threshold=10 -# Clean process work interval in seconds. default to 0 (disabled). -# a 600 seconds (10 minutes) interval makes sense -clean_ruleset_interval=600 - -############################################################################ -## The next 5 config parameters (packet_log, anchor, queue, tag, quickrules) -## are specific to BSD's pf(4) packet filter and hence cannot be enabled in -## VyOS. -# Log packets in pf (default is no) -#packet_log=no - -# Anchor name in pf (default is miniupnpd) -#anchor=miniupnpd - -# ALTQ queue in pf -# Filter rules must be used for this to be used. -# compile with PF_ENABLE_FILTER_RULES (see config.h file) -#queue=queue_name1 - -# Tag name in pf -#tag=tag_name1 - -# Make filter rules in pf quick or not. default is yes -# active when compiled with PF_ENABLE_FILTER_RULES (see config.h file) -#quickrules=no -## -## End of pf(4)-specific configuration not to be set in VyOS. -############################################################################ - -# UUID, generate your own UUID with "make genuuid" -uuid={{ uuid }} - -# Daemon's serial and model number when reporting to clients -# (in XML description) -#serial=12345678 -#model_number=1 - -# If compiled with IGD_V2 defined, force reporting IGDv1 in rootDesc (default -# is no) -#force_igd_desc_v1=no - -{% if rule is vyos_defined %} -# UPnP permission rules (also enforced for NAT-PMP and PCP) -# (allow|deny) (external port range) IP/mask (internal port range) (optional regex filter) -# A port range is - or if there is only -# one port in the range. -# IP/mask format must be nnn.nnn.nnn.nnn/nn -# It is advised to only allow redirection of port >= 1024 -# and end the rule set with "deny 0-65535 0.0.0.0/0 0-65535" -# The following default ruleset allows specific LAN side IP addresses -# to request only ephemeral ports. It is recommended that users -# modify the IP ranges to match their own internal networks, and -# also consider implementing network-specific restrictions -# CAUTION: failure to enforce any rules may permit insecure requests to be made! -{% for rule, config in rule.items() %} -{% if config.disable is not vyos_defined %} -{{ config.action }} {{ config.external_port_range }} {{ config.ip }}{{ '/32' if '/' not in config.ip else '' }} {{ config.internal_port_range }} -{% endif %} -{% endfor %} -{% endif %} diff --git a/debian/control b/debian/control index 594e9e8d8..329ed09eb 100644 --- a/debian/control +++ b/debian/control @@ -196,9 +196,6 @@ Depends: snmp, snmpd, # End "service snmp" -# For "service upnp" - miniupnpd-nftables, -# End "service upnp" # For "service webproxy" squid, squidclient, diff --git a/interface-definitions/service_upnp.xml.in b/interface-definitions/service_upnp.xml.in deleted file mode 100644 index 064386ee5..000000000 --- a/interface-definitions/service_upnp.xml.in +++ /dev/null @@ -1,229 +0,0 @@ - - - - - - - Universal Plug and Play (UPnP) service - 900 - - - - - Name of this service - - txt - Friendly name - - - - - - WAN network interface - - - - - #include - - - - - - WAN network IP - - ipv4 - IPv4 address - - - ipv6 - IPv6 address - - - - - - - - - - - Enable NAT-PMP support - - - - - - Enable Secure Mode - - - - - - Presentation Url - - txt - Presentation Url - - - - - - PCP-base lifetime Option - - - - - Max lifetime time - - - - - - - - Min lifetime time - - - - - - - - - - Local IP addresses for service to listen on - - - - - - <interface> - Monitor interface address - - - ipv4 - IPv4 address to listen for incoming connections - - - ipv4net - IPv4 prefix to listen for incoming connections - - - ipv6 - IPv6 address to listen for incoming connections - - - ipv6net - IPv6 prefix to listen for incoming connections - - - - #include - - - - - - - - - Enable STUN probe support (can be used with NAT 1:1 support for WAN interfaces) - - - - - The STUN server address - - txt - The STUN server host address - - - - - - - #include - - - - - UPnP Rule - - u32:0-65535 - Rule number - - - - - - - #include - - - Port range (REQUIRE) - - <port> - single port - - - <portN>-<portM> - Port range (use '-' as delimiter) - - - - - - - - - Port range (REQUIRE) - - <port> - single port - - - <portN>-<portM> - Port range (use '-' as delimiter) - - - - - - - - - The IP to which this rule applies (REQUIRE) - - ipv4 - The IPv4 address to which this rule applies - - - ipv4net - The IPv4 to which this rule applies - - - - - - - - - - - Actions against the rule (REQUIRE) - - allow deny - - - (allow|deny) - - - - - - - - - - diff --git a/smoketest/scripts/cli/test_service_upnp.py b/smoketest/scripts/cli/test_service_upnp.py deleted file mode 100755 index fd67b0ced..000000000 --- a/smoketest/scripts/cli/test_service_upnp.py +++ /dev/null @@ -1,103 +0,0 @@ -#!/usr/bin/env python3 -# -# Copyright (C) 2021-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 -# 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 . - -import unittest - -from base_vyostest_shim import VyOSUnitTestSHIM - -from vyos.configsession import ConfigSessionError -from vyos.template import ip_from_cidr -from vyos.utils.file import read_file -from vyos.utils.process import process_named_running - -UPNP_CONF = '/run/upnp/miniupnp.conf' -DAEMON = 'miniupnpd' -interface = 'eth0' -base_path = ['service', 'upnp'] -address_base = ['interfaces', 'ethernet', interface, 'address'] - -ipv4_addr = '100.64.0.1/24' -ipv6_addr = '2001:db8::1/64' - -class TestServiceUPnP(VyOSUnitTestSHIM.TestCase): - @classmethod - def setUpClass(cls): - super(TestServiceUPnP, cls).setUpClass() - - # ensure we can also run this test on a live system - so lets clean - # out the current configuration :) - cls.cli_delete(cls, base_path) - - cls.cli_set(cls, address_base + [ipv4_addr]) - cls.cli_set(cls, address_base + [ipv6_addr]) - - @classmethod - def tearDownClass(cls): - cls.cli_delete(cls, address_base) - cls._session.commit() - - super(TestServiceUPnP, cls).tearDownClass() - - def tearDown(self): - # Check for running process - self.assertTrue(process_named_running(DAEMON)) - - self.cli_delete(base_path) - self.cli_commit() - - # Check for running process - self.assertFalse(process_named_running(DAEMON)) - - def test_ipv4_base(self): - self.cli_set(base_path + ['nat-pmp']) - self.cli_set(base_path + ['listen', interface]) - - # check validate() - WAN interface is mandatory - with self.assertRaises(ConfigSessionError): - self.cli_commit() - self.cli_set(base_path + ['wan-interface', interface]) - - self.cli_commit() - - config = read_file(UPNP_CONF) - self.assertIn(f'ext_ifname={interface}', config) - self.assertIn(f'listening_ip={interface}', config) - self.assertIn(f'enable_natpmp=yes', config) - self.assertIn(f'enable_upnp=yes', config) - - def test_ipv6_base(self): - v6_addr = ip_from_cidr(ipv6_addr) - - self.cli_set(base_path + ['nat-pmp']) - self.cli_set(base_path + ['listen', interface]) - self.cli_set(base_path + ['listen', v6_addr]) - - # check validate() - WAN interface is mandatory - with self.assertRaises(ConfigSessionError): - self.cli_commit() - self.cli_set(base_path + ['wan-interface', interface]) - - self.cli_commit() - - config = read_file(UPNP_CONF) - self.assertIn(f'ext_ifname={interface}', config) - self.assertIn(f'listening_ip={interface}', config) - self.assertIn(f'ipv6_listening_ip={v6_addr}', config) - self.assertIn(f'enable_natpmp=yes', config) - self.assertIn(f'enable_upnp=yes', config) - -if __name__ == '__main__': - unittest.main(verbosity=2) diff --git a/src/conf_mode/service_upnp.py b/src/conf_mode/service_upnp.py deleted file mode 100755 index 0df8dc09e..000000000 --- a/src/conf_mode/service_upnp.py +++ /dev/null @@ -1,157 +0,0 @@ -#!/usr/bin/env python3 -# -# Copyright (C) 2021-2022 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 . - -import os - -from sys import exit -import uuid -import netifaces -from ipaddress import IPv4Network -from ipaddress import IPv6Network - -from vyos.config import Config -from vyos.utils.process import call -from vyos.template import render -from vyos.template import is_ipv4 -from vyos.template import is_ipv6 -from vyos import ConfigError -from vyos import airbag -airbag.enable() - -config_file = r'/run/upnp/miniupnp.conf' - -def get_config(config=None): - if config: - conf = config - else: - conf = Config() - - base = ['service', 'upnp'] - upnpd = conf.get_config_dict(base, key_mangling=('-', '_'), get_first_key=True) - - if not upnpd: - return None - - upnpd = conf.merge_defaults(upnpd, recursive=True) - - uuidgen = uuid.uuid1() - upnpd.update({'uuid': uuidgen}) - - return upnpd - -def get_all_interface_addr(prefix, filter_dev, filter_family): - list_addr = [] - for interface in netifaces.interfaces(): - if filter_dev and interface in filter_dev: - continue - addrs = netifaces.ifaddresses(interface) - if netifaces.AF_INET in addrs.keys(): - if netifaces.AF_INET in filter_family: - for addr in addrs[netifaces.AF_INET]: - if prefix: - # we need to manually assemble a list of IPv4 address/prefix - prefix = '/' + \ - str(IPv4Network('0.0.0.0/' + addr['netmask']).prefixlen) - list_addr.append(addr['addr'] + prefix) - else: - list_addr.append(addr['addr']) - if netifaces.AF_INET6 in addrs.keys(): - if netifaces.AF_INET6 in filter_family: - for addr in addrs[netifaces.AF_INET6]: - if prefix: - # we need to manually assemble a list of IPv4 address/prefix - bits = bin(int(addr['netmask'].replace(':', '').split('/')[0], 16)).count('1') - prefix = '/' + str(bits) - list_addr.append(addr['addr'] + prefix) - else: - list_addr.append(addr['addr']) - - return list_addr - -def verify(upnpd): - if not upnpd: - return None - - if 'wan_interface' not in upnpd: - raise ConfigError('To enable UPNP, you must have the "wan-interface" option!') - - if 'rule' in upnpd: - for rule, rule_config in upnpd['rule'].items(): - for option in ['external_port_range', 'internal_port_range', 'ip', 'action']: - if option not in rule_config: - tmp = option.replace('_', '-') - raise ConfigError(f'Every UPNP rule requires "{tmp}" to be set!') - - if 'stun' in upnpd: - for option in ['host', 'port']: - if option not in upnpd['stun']: - raise ConfigError(f'A UPNP stun support must have an "{option}" option!') - - # Check the validity of the IP address - listen_dev = [] - system_addrs_cidr = get_all_interface_addr(True, [], [netifaces.AF_INET, netifaces.AF_INET6]) - system_addrs = get_all_interface_addr(False, [], [netifaces.AF_INET, netifaces.AF_INET6]) - if 'listen' not in upnpd: - raise ConfigError(f'Listen address or interface is required!') - for listen_if_or_addr in upnpd['listen']: - if listen_if_or_addr not in netifaces.interfaces(): - listen_dev.append(listen_if_or_addr) - if (listen_if_or_addr not in system_addrs) and (listen_if_or_addr not in system_addrs_cidr) and \ - (listen_if_or_addr not in netifaces.interfaces()): - if is_ipv4(listen_if_or_addr) and IPv4Network(listen_if_or_addr).is_multicast: - raise ConfigError(f'The address "{listen_if_or_addr}" is an address that is not allowed' - f'to listen on. It is not an interface address nor a multicast address!') - if is_ipv6(listen_if_or_addr) and IPv6Network(listen_if_or_addr).is_multicast: - raise ConfigError(f'The address "{listen_if_or_addr}" is an address that is not allowed' - f'to listen on. It is not an interface address nor a multicast address!') - - system_listening_dev_addrs_cidr = get_all_interface_addr(True, listen_dev, [netifaces.AF_INET6]) - system_listening_dev_addrs = get_all_interface_addr(False, listen_dev, [netifaces.AF_INET6]) - for listen_if_or_addr in upnpd['listen']: - if listen_if_or_addr not in netifaces.interfaces() and \ - (listen_if_or_addr not in system_listening_dev_addrs_cidr) and \ - (listen_if_or_addr not in system_listening_dev_addrs) and \ - is_ipv6(listen_if_or_addr) and \ - (not IPv6Network(listen_if_or_addr).is_multicast): - raise ConfigError(f'{listen_if_or_addr} must listen on the interface of the network card') - -def generate(upnpd): - if not upnpd: - return None - - if os.path.isfile(config_file): - os.unlink(config_file) - - render(config_file, 'firewall/upnpd.conf.j2', upnpd) - -def apply(upnpd): - systemd_service_name = 'miniupnpd.service' - if not upnpd: - # Stop the UPNP service - call(f'systemctl stop {systemd_service_name}') - else: - # Start the UPNP service - call(f'systemctl restart {systemd_service_name}') - -if __name__ == '__main__': - try: - c = get_config() - verify(c) - generate(c) - apply(c) - except ConfigError as e: - print(e) - exit(1) diff --git a/src/systemd/miniupnpd.service b/src/systemd/miniupnpd.service deleted file mode 100644 index 51cb2eed8..000000000 --- a/src/systemd/miniupnpd.service +++ /dev/null @@ -1,13 +0,0 @@ -[Unit] -Description=UPnP service -ConditionPathExists=/run/upnp/miniupnp.conf -After=vyos-router.service -StartLimitIntervalSec=0 - -[Service] -WorkingDirectory=/run/upnp -Type=simple -ExecStart=/usr/sbin/miniupnpd -d -f /run/upnp/miniupnp.conf -PrivateTmp=yes -PIDFile=/run/miniupnpd.pid -Restart=on-failure -- cgit v1.2.3