From fe1b1e11c3704e00b8023a44af542bbf07a084a2 Mon Sep 17 00:00:00 2001 From: ericbsd Date: Tue, 6 Oct 2026 18:03:54 -0300 Subject: [PATCH 1/2] Validate configuration window entries before writing them The configuration window wrote whatever was typed in the address, netmask, gateway, DNS and search fields to rc.conf and resolv.conf. sysrc does not escape values, and rc.conf is run by the shell as root at boot, so a value holding a quote, $(...) or a backtick ran as root. A newline in a DNS or search field also added lines to resolv.conf. Add NetworkMgr/validate.py, which checks interface names, IPv4 and IPv6 addresses, netmasks, prefix lengths and search domains, and returns them normalized. Save now validates every entry first and shows an error dialog naming the bad field, writing nothing. The update code only uses the checked values. set_rc_conf() also refuses any name or value with characters outside a small safe set, as a last guard. An empty IPv4 gateway now removes defaultrouter instead of writing it empty. --- NetworkMgr/configuration.py | 145 +++++++++++++++++++++++---- NetworkMgr/validate.py | 195 ++++++++++++++++++++++++++++++++++++ 2 files changed, 319 insertions(+), 21 deletions(-) create mode 100644 NetworkMgr/validate.py diff --git a/NetworkMgr/configuration.py b/NetworkMgr/configuration.py index 20e37da..ce6785a 100755 --- a/NetworkMgr/configuration.py +++ b/NetworkMgr/configuration.py @@ -5,7 +5,8 @@ Two tabs, IPv4 and IPv6, each offering an automatic mode (DHCP or SLAAC) or a manual address, mask, gateway, DNS and search domain. Saving writes rc.conf through sysrc and /etc/resolv.conf directly, then applies the same -settings to the running system through NetworkMgr.net_api. +settings to the running system through NetworkMgr.net_api. Every value is +checked by NetworkMgr.validate first, since rc.conf runs as shell code. """ import re @@ -30,6 +31,15 @@ wait_for_address ) from NetworkMgr.query import get_interface_settings, get_interface_settings_ipv6 +from NetworkMgr import validate + +# What set_rc_conf lets into rc.conf: the characters an interface name, +# address, netmask or keyword such as "WPA DHCP" can hold. No quote, $, +# backtick, backslash or newline can get through. +RC_CONF_SAFE_REGEX = re.compile(r'[A-Za-z0-9_.:%/ -]*') +# An rc.conf variable name. A "." from a vlan name such as em0.10 is let +# through, sysrc refuses it on its own. +RC_CONF_NAME_REGEX = re.compile(r'[A-Za-z_][A-Za-z0-9_.]*') class NetCardConfigWindow(Gtk.Window): @@ -122,6 +132,8 @@ def __init__(self, selected_nic=None): Gtk.Window.__init__(self, title="Network Configuration") self.set_default_size(475, 400) self.nics = nics_list() + # The checked values Save hands to update_system(). + self.pending = {} default_nic = selected_nic if selected_nic else default_card() # Build Tab 1 Content # Interface Drop Down Combo Box @@ -521,14 +533,87 @@ def update_interface_settings_ipv6(self): self.search_entry6.set_sensitive(False) def commit_pending_changes(self, _widget): - """Hide the window and apply the settings on the GTK idle handler. + """Check the entries, then apply them on the GTK idle handler. + + When an entry is invalid, an error dialog names it and the window + stays open so it can be corrected. Nothing is written. Args: _widget (Gtk.Widget): the Save button, unused. """ + try: + self.pending = self.validated_settings() + except ValueError as error: + self.show_invalid_entry(str(error)) + return self.hide_window() GLib.idle_add(self.update_system) + def validated_settings(self): + """Read the entries the chosen methods use and check each of them. + + Returns: + dict[str, str]: the interface and the normalized values, keyed + nic, inet, netmask, defaultrouter, dns1, dns2, search, + inet6, prefixlen, gateway6 and dns6. Optional entries left + blank are empty strings. + + Raises: + ValueError: naming the first entry that is not valid. + """ + nic = validate.interface(self.current_settings["Active Interface"]) + if nic not in self.nics: + raise ValueError(f'"{nic}" is not one of this system\'s interfaces.') + settings = {'nic': nic} + if self.method == 'Manual': + gateway = self.ip_input_gateway_entry.get_text().strip() + dns2 = self.secondary_dns_entry.get_text().strip() + settings.update({ + 'inet': validate.ipv4_address( + self.ip_input_address_entry.get_text(), 'Address'), + 'netmask': validate.ipv4_netmask( + self.ip_input_mask_entry.get_text()), + 'defaultrouter': validate.ipv4_address(gateway, 'Gateway') + if gateway else '', + 'dns1': validate.ip_address( + self.prymary_dns_entry.get_text(), 'Primary DNS Server'), + 'dns2': validate.ip_address(dns2, 'Secondary DNS Server') + if dns2 else '', + 'search': validate.search_domains(self.search_entry.get_text()), + }) + if self.method6 == 'Manual': + prefixlen = self.ip_input_mask_entry6.get_text().strip() or '64' + gateway6 = self.ip_input_gateway_entry6.get_text().strip() + dns6 = self.prymary_dns_entry6.get_text().strip() + settings.update({ + 'inet6': validate.ipv6_address( + self.ip_input_address_entry6.get_text(), 'IPv6 Address'), + 'prefixlen': validate.ipv6_prefixlen(prefixlen), + 'gateway6': validate.ipv6_address( + gateway6, 'IPv6 Gateway', allow_scope=True) + if gateway6 else '', + 'dns6': validate.ipv6_address(dns6, 'IPv6 DNS Server') + if dns6 else '', + }) + return settings + + def show_invalid_entry(self, message): + """Tell the user which entry was refused. + + Args: + message (str): the ValueError text from NetworkMgr.validate. + """ + dialog = Gtk.MessageDialog( + transient_for=self, + modal=True, + message_type=Gtk.MessageType.ERROR, + buttons=Gtk.ButtonsType.OK, + text="Invalid network setting", + ) + dialog.format_secondary_text(message) + dialog.run() + dialog.destroy() + def update_system(self): """Write the chosen IPv4 and IPv6 settings and apply them. @@ -536,28 +621,31 @@ def update_system(self): a manual configuration, and the interface is restarted so the new settings take effect straight away. """ - nic = self.current_settings["Active Interface"] - inet = self.ip_input_address_entry.get_text() - netmask = self.ip_input_mask_entry.get_text() - defaultrouter = self.ip_input_gateway_entry.get_text() + settings = self.pending + nic = settings['nic'] if self.method == 'Manual': + inet = settings['inet'] + netmask = settings['netmask'] if 'wlan' in nic: ifconfig_value = f'WPA inet {inet} netmask {netmask}' else: ifconfig_value = f'inet {inet} netmask {netmask}' self.set_rc_conf(f'ifconfig_{nic}', ifconfig_value) - self.set_rc_conf('defaultrouter', defaultrouter) + if settings['defaultrouter']: + self.set_rc_conf('defaultrouter', settings['defaultrouter']) + else: + self.remove_rc_conf_var('defaultrouter') start_static_network(nic, inet, netmask) with open('/etc/resolv.conf', 'w', encoding='utf-8') as resolv_conf: resolv_conf.writelines('# Generated by NetworkMgr\n') - search = self.search_entry.get_text() + search = settings['search'] if search: search_line = f'search {search}\n' resolv_conf.writelines(search_line) - dns1 = self.prymary_dns_entry.get_text() + dns1 = settings['dns1'] nameserver1_line = f'nameserver {dns1}\n' resolv_conf.writelines(nameserver1_line) - dns2 = self.secondary_dns_entry.get_text() + dns2 = settings['dns2'] if dns2: nameserver2_line = f'nameserver {dns2}\n' resolv_conf.writelines(nameserver2_line) @@ -581,18 +669,24 @@ def update_system(self): restart_routing_and_dhcp(nic) # Apply IPv6 configuration - self.update_system_ipv6(nic) + self.update_system_ipv6(settings) self.destroy() - def update_system_ipv6(self, nic): - """Apply IPv6 configuration changes.""" - inet6 = self.ip_input_address_entry6.get_text() - prefixlen = self.ip_input_mask_entry6.get_text() or "64" - gateway6 = self.ip_input_gateway_entry6.get_text() - dns6 = self.prymary_dns_entry6.get_text() + def update_system_ipv6(self, settings): + """Apply IPv6 configuration changes. + + Args: + settings (dict[str, str]): the checked values from + validated_settings(). + """ + nic = settings['nic'] if self.method6 == 'Manual': + inet6 = settings['inet6'] + prefixlen = settings['prefixlen'] + gateway6 = settings['gateway6'] + dns6 = settings['dns6'] # Static IPv6 configuration self.set_rc_conf(f'ifconfig_{nic}_ipv6', f'inet6 {inet6} prefixlen {prefixlen}') @@ -679,16 +773,25 @@ def discard_pending_changes(self, _widget): def set_rc_conf(self, name, value): """Set one rc.conf variable through sysrc. - The name and the value are passed as a single argument rather than - built into a shell command, so a value holding spaces needs no - quoting and a value holding a quote or a semicolon cannot run - anything. sysrc adds the quotes when it writes the file. + sysrc writes the value between double quotes without escaping it, + and rc.conf is run by the shell as root at boot, so a value holding + a quote, $(...) or a backtick would run as root. The entries are + checked by NetworkMgr.validate before they get here. As a last + guard, a name outside RC_CONF_NAME_REGEX or a value with any + character outside RC_CONF_SAFE_REGEX is refused rather than written. Args: name (str): the rc.conf variable, for instance ifconfig_em0. value (str): its value, for instance "inet 10.0.0.2 netmask 255.255.255.0". Spaces are fine. + + Raises: + ValueError: when the name or the value holds a character that + is not safe in rc.conf. """ + if not RC_CONF_NAME_REGEX.fullmatch(name) \ + or not RC_CONF_SAFE_REGEX.fullmatch(value): + raise ValueError(f'Refusing to write {name!r}={value!r} to rc.conf.') run(['sysrc', f'{name}={value}'], check=False) diff --git a/NetworkMgr/validate.py b/NetworkMgr/validate.py new file mode 100644 index 0000000..caf1e48 --- /dev/null +++ b/NetworkMgr/validate.py @@ -0,0 +1,195 @@ +"""Validation for the values the configuration window writes to the system. + +Everything typed in the configuration window ends up in rc.conf or +/etc/resolv.conf. sysrc writes a value between double quotes without +escaping it, and rc.conf is a shell script that runs as root at boot, so a +value holding a quote, $(...) or a backtick would run as root. These checks +only let through what each field can legitimately hold and return it in a +normalized form. Anything else raises ValueError with a message for the user. +""" + +import re +from ipaddress import IPv4Address, IPv4Network, IPv6Address + +# FreeBSD interface names such as em0, wlan0, epair0a or em0.10. Renamed +# interfaces can be almost anything, so this only keeps to the characters +# that are safe in rc.conf and to IFNAMSIZ, 15 characters plus the NUL. +INTERFACE_REGEX = re.compile(r'[A-Za-z][A-Za-z0-9_.]{0,14}') +# One DNS label: letters, digits and inner hyphens, at most 63 characters. +LABEL_REGEX = re.compile(r'(?!-)[A-Za-z0-9-]{1,63}(? 32: + raise error + return str(IPv4Network(f'0.0.0.0/{value}').netmask) + try: + mask = IPv4Address(value) + # IPv4Network also takes host masks such as 0.0.0.255, so check + # that the netmask it derives is the one that was typed. + if IPv4Network(f'0.0.0.0/{value}').netmask != mask: + raise error + except ValueError: + raise error from None + return str(mask) + + +def ipv6_address(text, field, allow_scope=False): + """ + Check an IPv6 address. + + Args: + text (str): what the user typed. + field (str): the field's label, used in the error message. + allow_scope (bool): accept a "%interface" zone, as a link-local + gateway needs. + + Returns: + str: the address in compressed form, with its zone if it had one. + + Raises: + ValueError: when the text is not an IPv6 address, or carries a zone + that is not allowed or is not an interface name. + """ + value = text.strip() + error = ValueError(f'{field}: "{text}" is not a valid IPv6 address.') + address, _, scope = value.partition('%') + if scope or value.endswith('%'): + if not allow_scope or not INTERFACE_REGEX.fullmatch(scope): + raise error + try: + normalized = str(IPv6Address(address)) + except ValueError: + raise error from None + return f'{normalized}%{scope}' if scope else normalized + + +def ip_address(text, field): + """ + Check an IPv4 or IPv6 address, as a nameserver line accepts either. + + Args: + text (str): what the user typed. + field (str): the field's label, used in the error message. + + Returns: + str: the normalized address. + + Raises: + ValueError: when the text is neither. + """ + try: + return ipv4_address(text, field) + except ValueError: + pass + try: + return ipv6_address(text, field) + except ValueError: + raise ValueError(f'{field}: "{text}" is not a valid IP address.') from None + + +def ipv6_prefixlen(text): + """ + Check an IPv6 prefix length. + + Args: + text (str): what the user typed, for instance "64". + + Returns: + str: the prefix length without leading zeros. + + Raises: + ValueError: when it is not a whole number from 1 to 128. + """ + value = text.strip() + if not PREFIX_REGEX.fullmatch(value) or not 1 <= int(value) <= 128: + raise ValueError(f'Prefix Length: "{text}" must be a number from 1 to 128.') + return str(int(value)) + + +def search_domains(text): + """ + Check a space separated list of search domains. + + Args: + text (str): what the user typed, for instance "example.org lan". + + Returns: + str: the domains joined by single spaces, empty if none were given. + + Raises: + ValueError: when a domain is not a valid DNS name, or the list is + longer than resolv.conf accepts. + """ + domains = text.split() + for domain in domains: + labels = domain.rstrip('.').split('.') + if len(domain) > 253 or not all(LABEL_REGEX.fullmatch(label) for label in labels): + raise ValueError(f'Search domains: "{domain}" is not a valid domain name.') + joined = ' '.join(domains) + if len(domains) > MAX_SEARCH_DOMAINS or len(joined) > MAX_SEARCH_LENGTH: + raise ValueError( + f'Search domains: at most {MAX_SEARCH_DOMAINS} domains ' + f'and {MAX_SEARCH_LENGTH} characters are allowed.' + ) + return joined From b24ff6f00a77d2e1fed74e205994de15dc26e947 Mon Sep 17 00:00:00 2001 From: ericbsd Date: Tue, 6 Oct 2026 19:11:02 -0300 Subject: [PATCH 2/2] Fix DHCP save, dotted interfaces and entry checks from review Switching an interface to DHCP raised UnboundLocalError partway through the save, because the interim start_static_network call used locals that only the manual path set. The greyed out address entries are now checked in DHCP mode too, and the call is skipped when they hold no valid address. rc.subr reads the settings of em0.10 from ifconfig_em0_10, and sysrc refuses a name with a dot, so settings for dotted interfaces were silently not saved. Add validate.rc_conf_interface(), which maps ".-/+" to "_" like get_if_var in network.subr, and use it for every ifconfig_* variable written or read. An IPv6 gateway zone must now name the interface being configured. Search domains with more than one trailing dot are refused, and so is a zero IPv4 netmask. --- NetworkMgr/configuration.py | 40 +++++++++++++++++++++++++++---------- NetworkMgr/query.py | 7 +++++-- NetworkMgr/validate.py | 31 ++++++++++++++++++++++++---- 3 files changed, 62 insertions(+), 16 deletions(-) diff --git a/NetworkMgr/configuration.py b/NetworkMgr/configuration.py index ce6785a..2762d4e 100755 --- a/NetworkMgr/configuration.py +++ b/NetworkMgr/configuration.py @@ -37,9 +37,9 @@ # address, netmask or keyword such as "WPA DHCP" can hold. No quote, $, # backtick, backslash or newline can get through. RC_CONF_SAFE_REGEX = re.compile(r'[A-Za-z0-9_.:%/ -]*') -# An rc.conf variable name. A "." from a vlan name such as em0.10 is let -# through, sysrc refuses it on its own. -RC_CONF_NAME_REGEX = re.compile(r'[A-Za-z_][A-Za-z0-9_.]*') +# An rc.conf variable name. Interface names go through +# validate.rc_conf_interface first, so em0.10 arrives as em0_10. +RC_CONF_NAME_REGEX = re.compile(r'[A-Za-z_][A-Za-z0-9_]*') class NetCardConfigWindow(Gtk.Window): @@ -564,7 +564,18 @@ def validated_settings(self): nic = validate.interface(self.current_settings["Active Interface"]) if nic not in self.nics: raise ValueError(f'"{nic}" is not one of this system\'s interfaces.') - settings = {'nic': nic} + settings = {'nic': nic, 'inet': '', 'netmask': ''} + if self.method == 'DHCP': + # Saving DHCP briefly puts the current address back while + # dhclient starts. The entries are greyed out and hold what + # ifconfig reported, so an empty or odd value just skips that. + try: + settings['inet'] = validate.ipv4_address( + self.ip_input_address_entry.get_text(), 'Address') + settings['netmask'] = validate.ipv4_netmask( + self.ip_input_mask_entry.get_text()) + except ValueError: + settings['inet'] = settings['netmask'] = '' if self.method == 'Manual': gateway = self.ip_input_gateway_entry.get_text().strip() dns2 = self.secondary_dns_entry.get_text().strip() @@ -595,6 +606,11 @@ def validated_settings(self): 'dns6': validate.ipv6_address(dns6, 'IPv6 DNS Server') if dns6 else '', }) + scope = settings.get('gateway6', '').partition('%')[2] + if scope and scope != nic: + raise ValueError( + f'IPv6 Gateway: the "%{scope}" zone does not match the ' + f'interface being configured, {nic}.') return settings def show_invalid_entry(self, message): @@ -623,6 +639,7 @@ def update_system(self): """ settings = self.pending nic = settings['nic'] + rc_nic = validate.rc_conf_interface(nic) if self.method == 'Manual': inet = settings['inet'] netmask = settings['netmask'] @@ -630,7 +647,7 @@ def update_system(self): ifconfig_value = f'WPA inet {inet} netmask {netmask}' else: ifconfig_value = f'inet {inet} netmask {netmask}' - self.set_rc_conf(f'ifconfig_{nic}', ifconfig_value) + self.set_rc_conf(f'ifconfig_{rc_nic}', ifconfig_value) if settings['defaultrouter']: self.set_rc_conf('defaultrouter', settings['defaultrouter']) else: @@ -650,13 +667,14 @@ def update_system(self): nameserver2_line = f'nameserver {dns2}\n' resolv_conf.writelines(nameserver2_line) else: - self.set_rc_conf(f'ifconfig_{nic}', + self.set_rc_conf(f'ifconfig_{rc_nic}', 'WPA DHCP' if 'wlan' in nic else 'DHCP') with open('/etc/rc.conf', 'r', encoding='utf-8') as rc_conf_file: rc_conf = rc_conf_file.read() for nic_search in self.nics: - if re.search(f'^ifconfig_{nic_search}=".*inet', rc_conf, re.MULTILINE): + rc_search = validate.rc_conf_interface(nic_search) + if re.search(f'^ifconfig_{rc_search}=".*inet', rc_conf, re.MULTILINE): break else: # Nothing static left, so dhclient takes the default @@ -664,7 +682,8 @@ def update_system(self): self.remove_rc_conf_var('defaultrouter') restart_card_network(nic) # sometimes the inet address isn't available immediately after dhcp is enabled. - start_static_network(nic, inet, netmask) + if settings['inet']: + start_static_network(nic, settings['inet'], settings['netmask']) wait_for_address(nic) restart_routing_and_dhcp(nic) @@ -681,6 +700,7 @@ def update_system_ipv6(self, settings): validated_settings(). """ nic = settings['nic'] + rc_nic = validate.rc_conf_interface(nic) if self.method6 == 'Manual': inet6 = settings['inet6'] @@ -688,7 +708,7 @@ def update_system_ipv6(self, settings): gateway6 = settings['gateway6'] dns6 = settings['dns6'] # Static IPv6 configuration - self.set_rc_conf(f'ifconfig_{nic}_ipv6', + self.set_rc_conf(f'ifconfig_{rc_nic}_ipv6', f'inet6 {inet6} prefixlen {prefixlen}') # Disable rtsold for static configuration @@ -719,7 +739,7 @@ def update_system_ipv6(self, settings): self.add_ipv6_dns(dns6) else: # SLAAC configuration - self.set_rc_conf(f'ifconfig_{nic}_ipv6', 'inet6 accept_rtadv') + self.set_rc_conf(f'ifconfig_{rc_nic}_ipv6', 'inet6 accept_rtadv') # Enable rtsold for SLAAC self.set_rc_conf('rtsold_enable', 'YES') diff --git a/NetworkMgr/query.py b/NetworkMgr/query.py index 245c56d..2c135f1 100644 --- a/NetworkMgr/query.py +++ b/NetworkMgr/query.py @@ -12,6 +12,8 @@ import re import os +from NetworkMgr.validate import rc_conf_interface + IP_REGEX = r'[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+' @@ -70,7 +72,8 @@ def get_interface_settings_ipv6(active_nic): empty strings rather than absent keys. """ ipv6_settings = {} - ifconfig_ipv6 = _rc_conf_value(f'ifconfig_{active_nic}_ipv6') + ifconfig_ipv6 = _rc_conf_value( + f'ifconfig_{rc_conf_interface(active_nic)}_ipv6') # Check if SLAAC is enabled (accept_rtadv in rc.conf) slaac_search = re.search( @@ -186,7 +189,7 @@ def get_interface_settings(active_nic): 2 are always present, empty when unset. """ interface_settings = {} - if 'DHCP' in _rc_conf_value(f'ifconfig_{active_nic}'): + if 'DHCP' in _rc_conf_value(f'ifconfig_{rc_conf_interface(active_nic)}'): dhcp_status_output = "DHCP" else: dhcp_status_output = "Manual" diff --git a/NetworkMgr/validate.py b/NetworkMgr/validate.py index caf1e48..40c4fbc 100644 --- a/NetworkMgr/validate.py +++ b/NetworkMgr/validate.py @@ -22,6 +22,9 @@ MAX_SEARCH_LENGTH = 256 # A prefix length. str.isdigit() would also take digits such as "²". PREFIX_REGEX = re.compile(r'[0-9]{1,3}') +# rc.subr turns these characters of an interface name into "_" when it +# looks up ifconfig_ and friends. See get_if_var in network.subr. +RC_CONF_PUNCTUATION = str.maketrans('.-/+', '____') def interface(name): @@ -42,6 +45,22 @@ def interface(name): return name +def rc_conf_interface(name): + """ + Give the form of an interface name that rc.conf variables use. + + rc.subr reads the settings of em0.10 from ifconfig_em0_10, and sysrc + refuses a variable name with a dot in it. + + Args: + name (str): the interface, for instance "em0.10". + + Returns: + str: the name with ".", "-", "/" and "+" replaced by "_". + """ + return name.translate(RC_CONF_PUNCTUATION) + + def ipv4_address(text, field): """ Check an IPv4 address. @@ -75,19 +94,21 @@ def ipv4_netmask(text): Raises: ValueError: when the text is neither a contiguous dotted netmask - nor a prefix length from 0 to 32. + nor a prefix length from 1 to 32. A zero netmask would put the + whole internet on the link, so it is refused. """ value = text.strip() error = ValueError(f'Subnet Mask: "{text}" is not a valid netmask.') if PREFIX_REGEX.fullmatch(value): - if int(value) > 32: + if not 1 <= int(value) <= 32: raise error return str(IPv4Network(f'0.0.0.0/{value}').netmask) try: mask = IPv4Address(value) # IPv4Network also takes host masks such as 0.0.0.255, so check # that the netmask it derives is the one that was typed. - if IPv4Network(f'0.0.0.0/{value}').netmask != mask: + if IPv4Network(f'0.0.0.0/{value}').netmask != mask \ + or mask == IPv4Address('0.0.0.0'): raise error except ValueError: raise error from None @@ -183,7 +204,9 @@ def search_domains(text): """ domains = text.split() for domain in domains: - labels = domain.rstrip('.').split('.') + # One trailing dot marks a fully qualified name. Only that one is + # dropped, so "example.com.." leaves an empty label and is refused. + labels = domain.removesuffix('.').split('.') if len(domain) > 253 or not all(LABEL_REGEX.fullmatch(label) for label in labels): raise ValueError(f'Search domains: "{domain}" is not a valid domain name.') joined = ' '.join(domains)