From 6a9646ec6f18e7372d9453c36d595b4ab15e7556 Mon Sep 17 00:00:00 2001 From: badrogger Date: Tue, 1 Sep 2026 13:37:48 +0100 Subject: [PATCH 01/16] Update nftables --- node_cli/core/nftables.py | 260 +++++++++++++++++++++++++--- node_cli/core/node.py | 18 ++ tests/cli/node_test.py | 29 +++- tests/core/nftables_test.py | 336 +++++++++++++++++++++++++++++++++++- 4 files changed, 613 insertions(+), 30 deletions(-) diff --git a/node_cli/core/nftables.py b/node_cli/core/nftables.py index e0059fff..f1ae0b5f 100644 --- a/node_cli/core/nftables.py +++ b/node_cli/core/nftables.py @@ -27,14 +27,16 @@ from typing import Optional from node_cli.configs import ( + DEFAULT_NODE_BASE_PORT, ENV, NFTABLES_CHAIN_CONFIG_WILDCARD, NFTABLES_CHAIN_FOLDER_PATH, NFTABLES_MAIN_CONFIG_PATH, NFTABLES_SKALE_BASE_CONFIG_PATH, NFTABLES_USER_CONFIG_PATH, + NODE_CONFIG_PATH, ) -from node_cli.utils.helper import get_ssh_port, run_cmd +from node_cli.utils.helper import get_ssh_port, read_json, run_cmd logger = logging.getLogger(__name__) @@ -66,6 +68,31 @@ class SGXPort: CHAIN_PRIORITY = 1 HOOK = 'input' POLICY = 'accept' +POLICY_DROP = 'drop' + +# Prefix of the dynamic per-sChain chains managed by skale-admin +# in the same inet/firewall table (skale-, skale-network-scope, ...) +DYNAMIC_CHAIN_PREFIX = 'skale-' + +# sChain base ports are allocated as node_base_port + schain_index * 64 +# (PORTS_PER_SCHAIN in skale.py); 128 slots cover every possible allocation +SCHAIN_PORTS_PER_NODE = 128 * 64 +SCHAIN_BASE_PORT_ENV = 'SCHAIN_BASE_PORT' +FIREWALL_DEFAULT_DROP_ENV = 'FIREWALL_DEFAULT_DROP' +MIN_SCHAIN_BASE_PORT = 2000 +MAX_PORT = 65535 + +# Without these a drop policy on an inet chain breaks IPv6 neighbor +# discovery and path MTU discovery +ICMPV6_ACCEPT_TYPES = ( + 'destination-unreachable', + 'packet-too-big', + 'time-exceeded', + 'parameter-problem', + 'nd-router-advert', + 'nd-neighbor-solicit', + 'nd-neighbor-advert', +) try: @@ -180,24 +207,28 @@ def update_chain_policy( family = family or self.family table = table or self.table if self.chain_exists(chain, family=family): - cmd = [ - 'nft', - 'add', - 'chain', - family, - table, - chain, - '{', - 'policy', - POLICY, - ';', - '}', - ] - run_cmd(cmd) + cmd = f'add chain {family} {table} {chain} {{ policy {policy} ; }}' + rc, output, error = self.nft.cmd(cmd) + if rc != 0: + raise NFTablesError(f'Failed to set policy {policy} on {chain}: {error}') logger.info('Updated chain policy: %s %s', chain, policy) else: logger.info('Chain %s does not exist', chain) + def get_chain_policy(self, chain: str) -> Optional[str]: + """Return the policy of a chain in the managed table or None.""" + try: + rc, output, error = self.nft.cmd(f'list chain {self.family} {self.table} {chain}') + if rc != 0: + return None + data = json.loads(output) + for item in data.get('nftables', []): + if 'chain' in item and item['chain'].get('name') == chain: + return item['chain'].get('policy') + except Exception as e: + logger.error('Failed to get policy of chain %s: %s', chain, e) + return None + def table_exists(self) -> bool: try: rc, output, error = self.nft.cmd(f'list table {self.family} {self.table}') @@ -339,7 +370,7 @@ def add_rule(self, rule: Rule) -> None: { 'match': { 'op': '==', - 'left': {'payload': {'protocol': 'tcp', 'field': 'dport'}}, + 'left': {'payload': {'protocol': rule.protocol, 'field': 'dport'}}, 'right': rule.first_port, } } @@ -349,16 +380,16 @@ def add_rule(self, rule: Rule) -> None: { 'match': { 'op': '==', - 'left': {'payload': {'protocol': 'tcp', 'field': 'dport'}}, + 'left': {'payload': {'protocol': rule.protocol, 'field': 'dport'}}, 'right': {'range': [rule.first_port, rule.last_port]}, } } ) - elif rule.protocol == 'icmp' and rule.icmp_type: + elif rule.protocol in ['icmp', 'icmpv6'] and rule.icmp_type: expr.append( { 'match': { - 'left': {'payload': {'protocol': 'icmp', 'field': 'type'}}, + 'left': {'payload': {'protocol': rule.protocol, 'field': 'type'}}, 'op': '==', 'right': rule.icmp_type, } @@ -410,7 +441,7 @@ def remove_rule(self, rule: Rule) -> None: { 'match': { 'op': '==', - 'left': {'payload': {'protocol': 'tcp', 'field': 'dport'}}, + 'left': {'payload': {'protocol': rule.protocol, 'field': 'dport'}}, 'right': rule.first_port, } } @@ -420,7 +451,7 @@ def remove_rule(self, rule: Rule) -> None: { 'match': { 'op': '==', - 'left': {'payload': {'protocol': 'tcp', 'field': 'dport'}}, + 'left': {'payload': {'protocol': rule.protocol, 'field': 'dport'}}, 'right': {'range': [rule.first_port, rule.last_port]}, } } @@ -527,6 +558,129 @@ def add_loopback_rule(self, chain) -> None: else: logger.info('Loopback rule already exists in chain %s', chain) + def get_dynamic_chain_port_ranges(self) -> list[tuple[str, int, int]]: + """Min/max tcp dport covered by each dynamic skale-admin chain.""" + try: + rc, output, error = self.nft.cmd(f'list table {self.family} {self.table}') + if rc != 0: + if error and 'No such file or directory' in error: + return [] + raise NFTablesError(f'Failed to list table {self.table}: {error}') + data = json.loads(output) + except NFTablesError: + raise + except Exception as e: + logger.error('Failed to get dynamic chain ranges: %s', e) + raise NFTablesError(e) + + ports: dict[str, list[int]] = {} + for item in data.get('nftables', []): + rule = item.get('rule') + if not rule or not rule.get('chain', '').startswith(DYNAMIC_CHAIN_PREFIX): + continue + for statement in rule.get('expr', []): + match = statement.get('match', {}) + if match.get('left', {}).get('payload', {}).get('field') != 'dport': + continue + right = match.get('right') + chain_ports = ports.setdefault(rule['chain'], []) + if isinstance(right, dict) and 'range' in right: + chain_ports.extend(right['range']) + elif isinstance(right, int): + chain_ports.append(right) + return [(chain, min(values), max(values)) for chain, values in ports.items() if values] + + def validate_dynamic_ranges(self, envelope: tuple[int, int]) -> None: + """Ensure ports of every dynamic skale-admin chain fit into the envelope.""" + for chain, first_port, last_port in self.get_dynamic_chain_port_ranges(): + if first_port < envelope[0] or last_port > envelope[1]: + raise NFTablesError( + f'Ports {first_port}-{last_port} of dynamic chain {chain} are outside ' + f'of the allowed sChain ports range {envelope[0]}-{envelope[1]}. ' + f'Set {SCHAIN_BASE_PORT_ENV} env variable to the base port the node ' + 'was registered with and rerun the command' + ) + + def verify_critical_accepts(self) -> None: + """Ensure lockout-critical accept rules are in place before setting drop policy.""" + conntrack_expr = [ + { + 'match': { + 'left': {'ct': {'key': 'state'}}, + 'op': 'in', + 'right': ['established', 'related'], + } + }, + {'counter': None}, + {'accept': None}, + ] + ssh_expr = [ + { + 'match': { + 'op': '==', + 'left': {'payload': {'protocol': 'tcp', 'field': 'dport'}}, + 'right': get_ssh_port(), + } + }, + {'counter': None}, + {'accept': None}, + ] + for name, expr in (('conntrack', conntrack_expr), ('ssh', ssh_expr)): + if not self.rule_exists(self.chain, expr): + raise NFTablesError( + f'Refusing to set drop policy: {name} accept rule is missing ' + f'in chain {self.chain}' + ) + + def ensure_default_drop(self, envelope: tuple[int, int]) -> None: + """Switch the skale chain policy to drop after validating the accepts.""" + self.validate_dynamic_ranges(envelope) + self.verify_critical_accepts() + if self.get_chain_policy(self.chain) != POLICY_DROP: + self.update_chain_policy(chain=self.chain, policy=POLICY_DROP) + + def ensure_default_accept(self) -> None: + """Rollback path: switch the skale chain policy back to accept.""" + if self.get_chain_policy(self.chain) == POLICY_DROP: + self.update_chain_policy(chain=self.chain, policy=POLICY) + + def delete_rule_by_handle(self, handle: int) -> None: + cmd = { + 'nftables': [ + { + 'delete': { + 'rule': { + 'family': self.family, + 'table': self.table, + 'chain': self.chain, + 'handle': handle, + } + } + } + ] + } + self.execute_cmd(cmd) + + def remove_stale_envelope_rules(self, envelope: tuple[int, int]) -> None: + """Remove sChain envelope accepts anchored at a different base port.""" + for rule in self.get_rules(self.chain): + expr = rule.get('expr', []) + if {'accept': None} not in expr: + continue + for statement in expr: + match = statement.get('match', {}) + right = match.get('right') + if ( + match.get('left', {}).get('payload', {}).get('field') == 'dport' + and isinstance(right, dict) + and 'range' in right + and right['range'][1] - right['range'][0] == SCHAIN_PORTS_PER_NODE - 1 + and tuple(right['range']) != envelope + and rule.get('handle') is not None + ): + logger.info('Removing stale envelope rule %s', right['range']) + self.delete_rule_by_handle(rule['handle']) + def get_base_ruleset(self) -> str: self.nft.set_json_output(False) try: @@ -538,13 +692,17 @@ def get_base_ruleset(self) -> str: finally: self.nft.set_json_output(True) - def setup_firewall(self, enable_monitoring: bool = False) -> None: """Setup firewall rules.""" logger.info('Configuring firewall rules') + envelope = get_schain_ports_envelope() + default_drop = firewall_default_drop_enabled() try: self.create_table_if_not_exists() + # Fail fast, before any rule is touched, if the envelope does not + # cover the chains skale-admin already created on this node + self.validate_dynamic_ranges(envelope) base_chains_config = {'skale': {'hook': 'input', 'policy': 'accept'}} @@ -575,6 +733,21 @@ def setup_firewall(self, enable_monitoring: bool = False) -> None: for icmp_type in icmp_types: self.add_rule(Rule(chain=self.chain, protocol='icmp', icmp_type=icmp_type)) + for icmpv6_type in ICMPV6_ACCEPT_TYPES: + self.add_rule(Rule(chain=self.chain, protocol='icmpv6', icmp_type=icmpv6_type)) + + # Fine-grained filtering inside the envelope is enforced by the + # dynamic skale-admin chains that run earlier (priority 0) + self.remove_stale_envelope_rules(envelope) + self.add_rule( + Rule( + chain=self.chain, + protocol='tcp', + first_port=envelope[0], + last_port=envelope[1], + ) + ) + self.add_drop_rule( Rule( chain=self.chain, @@ -590,10 +763,18 @@ def setup_firewall(self, enable_monitoring: bool = False) -> None: chain=LEGACY_CHAIN, policy=POLICY, family=LEGACY_FAMILY, table=LEGACY_TABLE ) + if default_drop: + self.ensure_default_drop(envelope) + else: + self.ensure_default_accept() + except Exception as e: logger.error('Failed to setup firewall: %s', e) raise NFTablesError(e) - logger.info('Firewall rules are configured') + logger.info( + 'Firewall rules are configured, default policy: %s', + POLICY_DROP if default_drop else POLICY, + ) def cleanup_legacy_rules(self, ssh: bool = False, dns: bool = False) -> None: """Cleans up all node-cli generated rules.""" @@ -631,6 +812,39 @@ def flush_chain(self, chain: str) -> None: raise NFTablesError('Flushing chain errored') +def firewall_default_drop_enabled() -> bool: + value = os.getenv(FIREWALL_DEFAULT_DROP_ENV, 'True') + return value.lower() not in ('false', '0', 'no', 'off') + + +def get_registered_base_port() -> Optional[int]: + """sChain base port saved to the node config during registration.""" + if not os.path.isfile(NODE_CONFIG_PATH): + return None + try: + node_config = read_json(NODE_CONFIG_PATH) + except Exception as e: + logger.warning('Failed to read node config: %s', e) + return None + base_port = node_config.get('schain_base_port') or 0 + return base_port if base_port > 0 else None + + +def get_schain_ports_envelope() -> tuple[int, int]: + """Range of ports that can be allocated to sChains on this node.""" + env_value = os.getenv(SCHAIN_BASE_PORT_ENV) + if env_value: + try: + base_port = int(env_value) + except ValueError: + raise NFTablesError(f'{SCHAIN_BASE_PORT_ENV} must be an integer, got {env_value}') + else: + base_port = get_registered_base_port() or DEFAULT_NODE_BASE_PORT + if not MIN_SCHAIN_BASE_PORT <= base_port <= MAX_PORT - SCHAIN_PORTS_PER_NODE + 1: + raise NFTablesError(f'Invalid sChain base port {base_port}') + return base_port, base_port + SCHAIN_PORTS_PER_NODE - 1 + + def prepare_directories() -> None: logger.info('Prepare directories for nftables') os.makedirs(NFTABLES_CHAIN_FOLDER_PATH, exist_ok=True) diff --git a/node_cli/core/node.py b/node_cli/core/node.py index b1f22d86..379c069d 100644 --- a/node_cli/core/node.py +++ b/node_cli/core/node.py @@ -34,6 +34,7 @@ CONTAINER_CONFIG_PATH, FILESTORAGE_MAPPING, LOG_PATH, + NODE_CONFIG_PATH, RESTORE_SLEEP_TIMEOUT, SCHAINS_MNT_DIR_REGULAR, SCHAINS_MNT_DIR_SINGLE_CHAIN, @@ -79,6 +80,8 @@ error_exit, get_request, post_request, + read_json, + save_json, ) from node_cli.utils.meta import CliMetaManager from node_cli.utils.node_type import NodeType, NodeMode @@ -145,12 +148,27 @@ def register_node(name, p2p_ip, public_ip, port, domain_name): msg = TEXTS['node']['registered'] logger.info(msg) print(msg) + save_registered_base_port(port) + logger.info('Reconfiguring firewall for the registered base port %d', port) + configure_nftables(enable_monitoring=get_settings().monitoring_containers) else: error_msg = payload logger.error(f'Registration error {error_msg}') error_exit(error_msg, exit_code=CLIExitCodes.BAD_API_RESPONSE) +def save_registered_base_port(port: int) -> None: + """Persist the sChain base port to the node config. + + skale-admin saves it during registration as well - this covers setups + where the admin container predates that behavior. + """ + node_config = read_json(NODE_CONFIG_PATH) if os.path.isfile(NODE_CONFIG_PATH) else {} + if node_config.get('schain_base_port') != port: + node_config['schain_base_port'] = port + save_json(NODE_CONFIG_PATH, node_config) + + @check_not_inited def init(config_file: str, node_type: NodeType) -> None: node_mode = NodeMode.ACTIVE diff --git a/tests/cli/node_test.py b/tests/cli/node_test.py index a0c064f0..52942975 100644 --- a/tests/cli/node_test.py +++ b/tests/cli/node_test.py @@ -58,7 +58,12 @@ def test_register_node(inited_node, resource_alloc, mocked_g_config): resp_mock = response_mock(requests.codes.ok, {'status': 'ok', 'payload': None}) - with mock.patch('node_cli.utils.decorators.is_node_inited', return_value=True): + with ( + mock.patch('node_cli.utils.decorators.is_node_inited', return_value=True), + mock.patch('node_cli.core.node.save_registered_base_port'), + mock.patch('node_cli.core.node.configure_nftables'), + mock.patch('node_cli.core.node.get_settings'), + ): result = run_command_mock( 'node_cli.utils.helper.requests.post', resp_mock, @@ -93,7 +98,12 @@ def test_register_node_with_error(inited_node, resource_alloc, mocked_g_config): def test_register_node_with_prompted_ip(inited_node, resource_alloc, mocked_g_config): resp_mock = response_mock(requests.codes.ok, {'status': 'ok', 'payload': None}) - with mock.patch('node_cli.utils.decorators.is_node_inited', return_value=True): + with ( + mock.patch('node_cli.utils.decorators.is_node_inited', return_value=True), + mock.patch('node_cli.core.node.save_registered_base_port'), + mock.patch('node_cli.core.node.configure_nftables'), + mock.patch('node_cli.core.node.get_settings'), + ): result = run_command_mock( 'node_cli.utils.helper.requests.post', resp_mock, @@ -110,7 +120,12 @@ def test_register_node_with_prompted_ip(inited_node, resource_alloc, mocked_g_co def test_register_node_with_default_port(inited_node, resource_alloc, mocked_g_config): resp_mock = response_mock(requests.codes.ok, {'status': 'ok', 'payload': None}) - with mock.patch('node_cli.utils.decorators.is_node_inited', return_value=True): + with ( + mock.patch('node_cli.utils.decorators.is_node_inited', return_value=True), + mock.patch('node_cli.core.node.save_registered_base_port'), + mock.patch('node_cli.core.node.configure_nftables'), + mock.patch('node_cli.core.node.get_settings'), + ): result = run_command_mock( 'node_cli.utils.helper.requests.post', resp_mock, @@ -384,7 +399,9 @@ def test_maintenance_off(mocked_g_config): ) -def test_turn_off_maintenance_on(mocked_g_config, regular_user_conf, active_node_option, skale_active_settings): +def test_turn_off_maintenance_on( + mocked_g_config, regular_user_conf, active_node_option, skale_active_settings +): resp_mock = response_mock(requests.codes.ok, {'status': 'ok', 'payload': None}) with ( mock.patch('subprocess.run', new=subprocess_run_mock), @@ -415,7 +432,9 @@ def test_turn_off_maintenance_on(mocked_g_config, regular_user_conf, active_node assert result.exit_code == CLIExitCodes.UNSAFE_UPDATE -def test_turn_on_maintenance_off(mocked_g_config, regular_user_conf, active_node_option, skale_active_settings): +def test_turn_on_maintenance_off( + mocked_g_config, regular_user_conf, active_node_option, skale_active_settings +): resp_mock = response_mock(requests.codes.ok, {'status': 'ok', 'payload': None}) with ( mock.patch('subprocess.run', new=subprocess_run_mock), diff --git a/tests/core/nftables_test.py b/tests/core/nftables_test.py index b6490b08..a6d997fa 100644 --- a/tests/core/nftables_test.py +++ b/tests/core/nftables_test.py @@ -4,7 +4,13 @@ import nftables -from node_cli.core.nftables import NFTablesManager, Rule +import node_cli.core.nftables as nftables_core +from node_cli.core.nftables import ( + NFTablesError, + NFTablesManager, + Rule, + get_schain_ports_envelope, +) @pytest.fixture(scope='module') @@ -102,6 +108,42 @@ def test_create_chain_if_not_exists(mock_exists, mock_execute, nft_manager): mock_execute.assert_called_once() +@patch('nftables.Nftables.cmd') +def test_update_chain_policy_uses_given_policy(mock_cmd, nft_manager): + """Test that policy update applies the requested policy.""" + mock_cmd.return_value = (0, '', '') + with patch.object(NFTablesManager, 'chain_exists', return_value=True): + nft_manager.update_chain_policy(chain='skale', policy='drop') + assert mock_cmd.call_args[0][0] == 'add chain inet filter skale { policy drop ; }' + + mock_cmd.return_value = (1, '', 'some error') + with patch.object(NFTablesManager, 'chain_exists', return_value=True): + with pytest.raises(NFTablesError): + nft_manager.update_chain_policy(chain='skale', policy='drop') + + +@patch('nftables.Nftables.cmd') +def test_get_chain_policy(mock_cmd, nft_manager): + listing = { + 'nftables': [ + { + 'chain': { + 'family': 'inet', + 'table': 'filter', + 'name': 'skale', + 'hook': 'input', + 'policy': 'drop', + } + } + ] + } + mock_cmd.return_value = (0, json.dumps(listing), '') + assert nft_manager.get_chain_policy('skale') == 'drop' + + mock_cmd.return_value = (1, '', 'No such file or directory') + assert nft_manager.get_chain_policy('skale') is None + + @pytest.mark.parametrize( 'rule_data', [ @@ -122,17 +164,307 @@ def test_add_rule(mock_exists, mock_execute, nft_manager, rule_data): @patch.object(NFTablesManager, 'execute_cmd') -def test_setup_firewall(mock_execute, nft_manager): +@patch.object(NFTablesManager, 'rule_exists') +def test_add_rule_udp_uses_udp_payload(mock_exists, mock_execute, nft_manager): + """Test that udp rules match udp dport, not tcp.""" + mock_exists.return_value = False + + nft_manager.add_rule(Rule(chain='INPUT', protocol='udp', first_port=53)) + expr = mock_execute.call_args[0][0]['nftables'][0]['add']['rule']['expr'] + assert expr[0]['match']['left']['payload'] == {'protocol': 'udp', 'field': 'dport'} + + +@patch.object(NFTablesManager, 'execute_cmd') +@patch.object(NFTablesManager, 'rule_exists') +def test_add_rule_icmpv6(mock_exists, mock_execute, nft_manager): + """Test icmpv6 rule addition.""" + mock_exists.return_value = False + + nft_manager.add_rule(Rule(chain='INPUT', protocol='icmpv6', icmp_type='nd-neighbor-solicit')) + expr = mock_execute.call_args[0][0]['nftables'][0]['add']['rule']['expr'] + assert expr[0]['match']['left']['payload'] == {'protocol': 'icmpv6', 'field': 'type'} + assert expr[0]['match']['right'] == 'nd-neighbor-solicit' + + +@patch('nftables.Nftables.cmd') +def test_get_dynamic_chain_port_ranges(mock_cmd, nft_manager): + """Test collection of port ranges covered by skale-admin chains.""" + listing = { + 'nftables': [ + {'chain': {'family': 'inet', 'table': 'filter', 'name': 'skale'}}, + {'chain': {'family': 'inet', 'table': 'filter', 'name': 'skale-test'}}, + { + 'rule': { + 'family': 'inet', + 'table': 'filter', + 'chain': 'skale', + 'expr': [ + { + 'match': { + 'op': '==', + 'left': {'payload': {'protocol': 'tcp', 'field': 'dport'}}, + 'right': 22, + } + }, + {'accept': None}, + ], + } + }, + { + 'rule': { + 'family': 'inet', + 'table': 'filter', + 'chain': 'skale-test', + 'expr': [ + { + 'match': { + 'op': '==', + 'left': {'payload': {'protocol': 'ip', 'field': 'saddr'}}, + 'right': '1.2.3.4', + } + }, + { + 'match': { + 'op': '==', + 'left': {'payload': {'protocol': 'tcp', 'field': 'dport'}}, + 'right': 10001, + } + }, + {'accept': None}, + ], + } + }, + { + 'rule': { + 'family': 'inet', + 'table': 'filter', + 'chain': 'skale-test', + 'expr': [ + { + 'match': { + 'op': '==', + 'left': {'payload': {'protocol': 'tcp', 'field': 'dport'}}, + 'right': {'range': [10000, 10063]}, + } + }, + {'drop': None}, + ], + } + }, + ] + } + mock_cmd.return_value = (0, json.dumps(listing), '') + assert nft_manager.get_dynamic_chain_port_ranges() == [('skale-test', 10000, 10063)] + + mock_cmd.return_value = (1, '', 'No such file or directory') + assert nft_manager.get_dynamic_chain_port_ranges() == [] + + +def test_validate_dynamic_ranges(nft_manager): + """Test envelope validation against dynamic chain ranges.""" + with patch.object( + NFTablesManager, + 'get_dynamic_chain_port_ranges', + return_value=[('skale-test', 10064, 10127)], + ): + nft_manager.validate_dynamic_ranges((10000, 18191)) + with pytest.raises(NFTablesError): + nft_manager.validate_dynamic_ranges((10128, 18191)) + + +def test_verify_critical_accepts(nft_manager): + with patch.object(NFTablesManager, 'rule_exists', return_value=True): + nft_manager.verify_critical_accepts() + with patch.object(NFTablesManager, 'rule_exists', return_value=False): + with pytest.raises(NFTablesError): + nft_manager.verify_critical_accepts() + + +def test_ensure_default_drop(nft_manager): + with patch.multiple( + NFTablesManager, + validate_dynamic_ranges=Mock(), + verify_critical_accepts=Mock(), + get_chain_policy=Mock(return_value='accept'), + update_chain_policy=Mock(), + ): + nft_manager.ensure_default_drop((10000, 18191)) + NFTablesManager.validate_dynamic_ranges.assert_called_once_with((10000, 18191)) + NFTablesManager.verify_critical_accepts.assert_called_once() + NFTablesManager.update_chain_policy.assert_called_once_with(chain='skale', policy='drop') + + with patch.multiple( + NFTablesManager, + validate_dynamic_ranges=Mock(), + verify_critical_accepts=Mock(), + get_chain_policy=Mock(return_value='drop'), + update_chain_policy=Mock(), + ): + nft_manager.ensure_default_drop((10000, 18191)) + NFTablesManager.update_chain_policy.assert_not_called() + + +def test_ensure_default_accept(nft_manager): + with patch.multiple( + NFTablesManager, + get_chain_policy=Mock(return_value='drop'), + update_chain_policy=Mock(), + ): + nft_manager.ensure_default_accept() + NFTablesManager.update_chain_policy.assert_called_once_with(chain='skale', policy='accept') + + with patch.multiple( + NFTablesManager, + get_chain_policy=Mock(return_value='accept'), + update_chain_policy=Mock(), + ): + nft_manager.ensure_default_accept() + NFTablesManager.update_chain_policy.assert_not_called() + + +def test_remove_stale_envelope_rules(nft_manager): + stale_rule = { + 'handle': 7, + 'expr': [ + { + 'match': { + 'op': '==', + 'left': {'payload': {'protocol': 'tcp', 'field': 'dport'}}, + 'right': {'range': [10000, 18191]}, + } + }, + {'counter': None}, + {'accept': None}, + ], + } + current_rule = { + 'handle': 8, + 'expr': [ + { + 'match': { + 'op': '==', + 'left': {'payload': {'protocol': 'tcp', 'field': 'dport'}}, + 'right': {'range': [30000, 38191]}, + } + }, + {'counter': None}, + {'accept': None}, + ], + } + sgx_drop_rule = { + 'handle': 9, + 'expr': [ + { + 'match': { + 'op': '==', + 'left': {'payload': {'protocol': 'tcp', 'field': 'dport'}}, + 'right': {'range': [1026, 1031]}, + } + }, + {'counter': None}, + {'drop': None}, + ], + } + with patch.multiple( + NFTablesManager, + get_rules=Mock(return_value=[stale_rule, current_rule, sgx_drop_rule]), + delete_rule_by_handle=Mock(), + ): + nft_manager.remove_stale_envelope_rules((30000, 38191)) + NFTablesManager.delete_rule_by_handle.assert_called_once_with(7) + + +def test_get_schain_ports_envelope_default(monkeypatch, tmp_path): + monkeypatch.setattr(nftables_core, 'NODE_CONFIG_PATH', str(tmp_path / 'nonexistent.json')) + assert get_schain_ports_envelope() == (10000, 18191) + + +def test_get_schain_ports_envelope_env_override(monkeypatch): + monkeypatch.setenv('SCHAIN_BASE_PORT', '30000') + assert get_schain_ports_envelope() == (30000, 38191) + + monkeypatch.setenv('SCHAIN_BASE_PORT', 'not-a-port') + with pytest.raises(NFTablesError): + get_schain_ports_envelope() + + monkeypatch.setenv('SCHAIN_BASE_PORT', '65000') + with pytest.raises(NFTablesError): + get_schain_ports_envelope() + + monkeypatch.setenv('SCHAIN_BASE_PORT', '1000') + with pytest.raises(NFTablesError): + get_schain_ports_envelope() + + +def test_get_schain_ports_envelope_from_node_config(monkeypatch, tmp_path): + config_path = tmp_path / 'node_config.json' + config_path.write_text(json.dumps({'node_id': 1, 'schain_base_port': 20128})) + monkeypatch.setattr(nftables_core, 'NODE_CONFIG_PATH', str(config_path)) + assert get_schain_ports_envelope() == (20128, 28319) + + +@patch.object(NFTablesManager, 'execute_cmd') +def test_setup_firewall(mock_execute, nft_manager, monkeypatch, tmp_path): """Test complete firewall setup.""" + monkeypatch.setattr(nftables_core, 'NODE_CONFIG_PATH', str(tmp_path / 'nonexistent.json')) with patch.multiple( NFTablesManager, table_exists=Mock(return_value=False), chain_exists=Mock(return_value=False), rule_exists=Mock(return_value=False), + verify_critical_accepts=Mock(), + get_dynamic_chain_port_ranges=Mock(return_value=[]), + get_chain_policy=Mock(return_value='accept'), + update_chain_policy=Mock(), ): nft_manager.setup_firewall() assert mock_execute.called + added_exprs = [ + call.args[0]['nftables'][0]['add']['rule']['expr'] + for call in mock_execute.call_args_list + if 'rule' in call.args[0]['nftables'][0].get('add', {}) + ] + envelope_exprs = [ + expr + for expr in added_exprs + if expr[0].get('match', {}).get('right') == {'range': [10000, 18191]} + and {'accept': None} in expr + ] + assert len(envelope_exprs) == 1 + icmpv6_exprs = [ + expr + for expr in added_exprs + if expr[0].get('match', {}).get('left', {}).get('payload', {}).get('protocol') + == 'icmpv6' + ] + assert len(icmpv6_exprs) == len(nftables_core.ICMPV6_ACCEPT_TYPES) + + NFTablesManager.update_chain_policy.assert_any_call( + chain='INPUT', policy='accept', family='ip', table='filter' + ) + NFTablesManager.update_chain_policy.assert_any_call(chain='skale', policy='drop') + + +@patch.object(NFTablesManager, 'execute_cmd') +def test_setup_firewall_default_drop_disabled(mock_execute, nft_manager, monkeypatch, tmp_path): + """Test that FIREWALL_DEFAULT_DROP=False keeps the accept policy.""" + monkeypatch.setenv('FIREWALL_DEFAULT_DROP', 'False') + monkeypatch.setattr(nftables_core, 'NODE_CONFIG_PATH', str(tmp_path / 'nonexistent.json')) + with patch.multiple( + NFTablesManager, + table_exists=Mock(return_value=True), + chain_exists=Mock(return_value=True), + rule_exists=Mock(return_value=True), + get_dynamic_chain_port_ranges=Mock(return_value=[]), + ensure_default_drop=Mock(), + ensure_default_accept=Mock(), + update_chain_policy=Mock(), + ): + nft_manager.setup_firewall() + NFTablesManager.ensure_default_drop.assert_not_called() + NFTablesManager.ensure_default_accept.assert_called_once() + def test_invalid_protocol(nft_manager): """Test adding rule with invalid protocol.""" From 9215f79082fcc150cc199a1039a1a0af7d696690 Mon Sep 17 00:00:00 2001 From: badrogger Date: Tue, 1 Sep 2026 18:15:21 +0100 Subject: [PATCH 02/16] Fix for existing nodes --- docs/firewall.md | 143 ++++++++++++++++++++++++++++++++++++ node_cli/core/nftables.py | 10 ++- node_cli/core/node.py | 12 +-- tests/core/nftables_test.py | 10 ++- 4 files changed, 167 insertions(+), 8 deletions(-) create mode 100644 docs/firewall.md diff --git a/docs/firewall.md b/docs/firewall.md new file mode 100644 index 00000000..3363407d --- /dev/null +++ b/docs/firewall.md @@ -0,0 +1,143 @@ +# Node firewall + +node-cli manages the host firewall with nftables. It owns the base chain +`inet firewall skale` (hook `input`, priority 1), while skale-admin dynamically +manages one base chain per sChain (`skale-`, priority 0) in the same +table with per-peer accept rules and a terminal drop for the sChain port range. + +Since a packet traverses **every** base chain on a hook (only `drop` is +terminal, `accept` in one chain does not skip later chains), the two layers +have to agree on what is reachable. + +## Default-drop policy (phase 1) + +The `skale` chain policy is **drop**. Everything not explicitly accepted is +dropped: + +- conntrack `established/related` and loopback traffic +- ssh (detected via `getservbyname`), DNS (53), HTTP (80), HTTPS (443), + watchdog (3009, 311), and monitoring ports (8080, 9100) when monitoring is + enabled +- ICMP `destination-unreachable`, `source-quench`, `time-exceeded` + (no `echo-request` - the node does not answer ping) +- ICMPv6 neighbor discovery and error types (required for IPv6 to function + under a drop policy on an `inet` chain) +- the **sChain ports envelope**: `tcp dport ..` + (128 sChain slots x 64 ports). Fine-grained filtering inside the envelope + is enforced by the skale-admin chains, which run earlier (priority 0) and + end with a terminal drop for each active sChain range. + +The envelope base port is resolved in this order: + +1. `SCHAIN_BASE_PORT` environment variable +2. `node_base_port` from `node_data/node_config.json` - the node's + registration port, saved by skale-admin at registration and backfilled + from the contracts on every admin restart for existing nodes +3. `schain_base_port` from `node_data/node_config.json` - fallback for + passive and fair nodes, where it holds the single hosted chain's base + port (a valid anchor); on active nodes this field keeps its passive-mode + meaning and is never written by registration +4. `10000` (the default registration port) + +Before flipping the policy to drop, `setup_firewall`: + +1. ensures every accept rule above exists (the flip is the last step, so a + failure mid-way leaves the node reachable - it fails open); +2. validates that the port range of every live `skale-*` chain fits into the + envelope, and aborts with an actionable error otherwise; +3. verifies the ssh and conntrack accept rules are actually present in the + live chain. + +The saved config (`/etc/nft.conf.d/skale/base.conf`) snapshots the chain with +its policy and includes the skale-admin chain configs, so a reboot applies +everything atomically. + +### Operator notes + +- Existing nodes get the new policy on the next `skale node update` (or + `skale node configure-firewall`). The setup is idempotent. +- Ship the skale-admin release (which backfills `node_base_port` on admin + restart) before the node-cli release that flips the policy - existing + custom-port nodes then anchor correctly with no manual step. +- If the node was registered with a non-default base port and + `node_config.json` does not contain `node_base_port` yet (admin has not + restarted on the new version), set `SCHAIN_BASE_PORT` to the registration + port. The validation step catches the mismatch and refuses to flip the + policy until it is fixed. +- Custom services listening on other ports must be allowed in + `/etc/nft.conf.d/skale/user.conf` (included at the top of the `skale` + chain) - plain nftables rule lines, e.g. `tcp dport 5000 counter accept`. +- Rollback: set `FIREWALL_DEFAULT_DROP=False` in the environment and rerun + `skale node configure-firewall`. Emergency manual rollback: + `nft add chain inet firewall skale '{ policy accept ; }'`. +- Ports published by Docker containers over bridge networking bypass the + input hook entirely (they are governed by Docker's forward chains); this + firewall governs host-network services, which includes skaled. + +### Known trade-off of the envelope + +Envelope ports with no active sChain are accepted by the firewall and answer +as *closed* (kernel RST) instead of *filtered*, and a stray listener bound +inside the envelope would be reachable. Ports of active sChains are protected +exactly as before by the skale-admin chains. Removing this trade-off is the +goal of phase 2. + +## Planned: sChain ports set (phase 2) + +Goal: unused sChain ports show as *filtered* - the envelope accept is +replaced by an accept driven by an interval set that skale-admin maintains +in lock-step with its chains. Validated end-to-end by a container prototype +(traffic + reboot persistence) on 2026-09-01. + +Design: + +- node-cli declares an interval set in `base.conf` and swaps the envelope + accept for a set-driven accept in the `skale` chain: + + ``` + set schain_ports { + type inet_service + flags interval + } + ... + tcp dport @schain_ports counter accept + ``` + +- skale-admin keeps its per-sChain priority-0 base chains **exactly as they + are** (per-peer accepts + terminal range drop). The only addition: one set + element added/removed together with the chain lifecycle: + + ``` + add element inet firewall schain_ports { 10064-10127 } + ``` + + Traversal: peer/public traffic accepted at priority 0 continues into + `skale`, matches the set, accepted. Strangers on consensus ports die on + the priority-0 terminal drop. Ports with no element fall to policy drop - + filtered, even if something listens there. + +- persistence: each saved chain config in `/etc/nft.conf.d/skale/chains/` + stays self-contained by re-declaring the set with just its element - nft + merges elements across declarations of the same set, so a reboot restores + chains and elements atomically with no re-sync: + + ``` + chain skale-x { ... } + set schain_ports { type inet_service ; flags interval ; elements = { 10064-10127 } } + ``` + +- rollout, each step safe under version skew: + 1. node-cli release: declare the set, add the `@schain_ports` accept, + seed elements from the live `skale-*` chain ranges at configure time + (`get_dynamic_chain_port_ranges`), **keep the envelope**. + 2. skale-admin release: manage elements on chain create/cleanup, include + the element re-declaration in saved chain configs, extend the firewall + health check to verify the element. + 3. node-cli: remove the envelope accept once every live chain range has a + matching set element (checked at configure time; abort otherwise). + +Alternative considered: a verdict map (`type inet_service : verdict`, +elements `{ range : jump skale-x }`) with per-sChain **regular** chains. +Same filtering result, plus per-chain rules only evaluated for their own +ports - but it requires migrating every existing base chain and changing +skale-admin's chain creation, so the set variant is preferred. diff --git a/node_cli/core/nftables.py b/node_cli/core/nftables.py index f1ae0b5f..c391ce91 100644 --- a/node_cli/core/nftables.py +++ b/node_cli/core/nftables.py @@ -818,7 +818,13 @@ def firewall_default_drop_enabled() -> bool: def get_registered_base_port() -> Optional[int]: - """sChain base port saved to the node config during registration.""" + """Base port for the envelope, taken from the node config. + + node_base_port is the port the node was registered with (saved by + skale-admin at registration and backfilled from the contracts on admin + restarts). schain_base_port is the fallback for passive and fair nodes, + where it holds the single hosted chain's base port - a valid anchor too. + """ if not os.path.isfile(NODE_CONFIG_PATH): return None try: @@ -826,7 +832,7 @@ def get_registered_base_port() -> Optional[int]: except Exception as e: logger.warning('Failed to read node config: %s', e) return None - base_port = node_config.get('schain_base_port') or 0 + base_port = node_config.get('node_base_port') or node_config.get('schain_base_port') or 0 return base_port if base_port > 0 else None diff --git a/node_cli/core/node.py b/node_cli/core/node.py index 379c069d..6898240b 100644 --- a/node_cli/core/node.py +++ b/node_cli/core/node.py @@ -158,14 +158,16 @@ def register_node(name, p2p_ip, public_ip, port, domain_name): def save_registered_base_port(port: int) -> None: - """Persist the sChain base port to the node config. + """Persist the node base port to the node config. - skale-admin saves it during registration as well - this covers setups - where the admin container predates that behavior. + Kept separate from schain_base_port, which holds an already-allocated + sChain port in passive mode. skale-admin saves node_base_port during + registration as well - this covers setups where the admin container + predates that behavior. """ node_config = read_json(NODE_CONFIG_PATH) if os.path.isfile(NODE_CONFIG_PATH) else {} - if node_config.get('schain_base_port') != port: - node_config['schain_base_port'] = port + if node_config.get('node_base_port') != port: + node_config['node_base_port'] = port save_json(NODE_CONFIG_PATH, node_config) diff --git a/tests/core/nftables_test.py b/tests/core/nftables_test.py index a6d997fa..4b3b6f36 100644 --- a/tests/core/nftables_test.py +++ b/tests/core/nftables_test.py @@ -398,8 +398,16 @@ def test_get_schain_ports_envelope_env_override(monkeypatch): def test_get_schain_ports_envelope_from_node_config(monkeypatch, tmp_path): config_path = tmp_path / 'node_config.json' - config_path.write_text(json.dumps({'node_id': 1, 'schain_base_port': 20128})) monkeypatch.setattr(nftables_core, 'NODE_CONFIG_PATH', str(config_path)) + + # active node: node_base_port saved at registration wins + config_path.write_text( + json.dumps({'node_id': 1, 'node_base_port': 20128, 'schain_base_port': 30000}) + ) + assert get_schain_ports_envelope() == (20128, 28319) + + # passive/fair node: only schain_base_port is present + config_path.write_text(json.dumps({'node_id': 1, 'schain_base_port': 20128})) assert get_schain_ports_envelope() == (20128, 28319) From a239ddb29e99c90e77a5cd385ffa3a8921871c78 Mon Sep 17 00:00:00 2001 From: badrogger Date: Tue, 1 Sep 2026 18:16:13 +0100 Subject: [PATCH 03/16] Small improvement --- docs/firewall.md | 143 ----------------------------------------------- 1 file changed, 143 deletions(-) delete mode 100644 docs/firewall.md diff --git a/docs/firewall.md b/docs/firewall.md deleted file mode 100644 index 3363407d..00000000 --- a/docs/firewall.md +++ /dev/null @@ -1,143 +0,0 @@ -# Node firewall - -node-cli manages the host firewall with nftables. It owns the base chain -`inet firewall skale` (hook `input`, priority 1), while skale-admin dynamically -manages one base chain per sChain (`skale-`, priority 0) in the same -table with per-peer accept rules and a terminal drop for the sChain port range. - -Since a packet traverses **every** base chain on a hook (only `drop` is -terminal, `accept` in one chain does not skip later chains), the two layers -have to agree on what is reachable. - -## Default-drop policy (phase 1) - -The `skale` chain policy is **drop**. Everything not explicitly accepted is -dropped: - -- conntrack `established/related` and loopback traffic -- ssh (detected via `getservbyname`), DNS (53), HTTP (80), HTTPS (443), - watchdog (3009, 311), and monitoring ports (8080, 9100) when monitoring is - enabled -- ICMP `destination-unreachable`, `source-quench`, `time-exceeded` - (no `echo-request` - the node does not answer ping) -- ICMPv6 neighbor discovery and error types (required for IPv6 to function - under a drop policy on an `inet` chain) -- the **sChain ports envelope**: `tcp dport ..` - (128 sChain slots x 64 ports). Fine-grained filtering inside the envelope - is enforced by the skale-admin chains, which run earlier (priority 0) and - end with a terminal drop for each active sChain range. - -The envelope base port is resolved in this order: - -1. `SCHAIN_BASE_PORT` environment variable -2. `node_base_port` from `node_data/node_config.json` - the node's - registration port, saved by skale-admin at registration and backfilled - from the contracts on every admin restart for existing nodes -3. `schain_base_port` from `node_data/node_config.json` - fallback for - passive and fair nodes, where it holds the single hosted chain's base - port (a valid anchor); on active nodes this field keeps its passive-mode - meaning and is never written by registration -4. `10000` (the default registration port) - -Before flipping the policy to drop, `setup_firewall`: - -1. ensures every accept rule above exists (the flip is the last step, so a - failure mid-way leaves the node reachable - it fails open); -2. validates that the port range of every live `skale-*` chain fits into the - envelope, and aborts with an actionable error otherwise; -3. verifies the ssh and conntrack accept rules are actually present in the - live chain. - -The saved config (`/etc/nft.conf.d/skale/base.conf`) snapshots the chain with -its policy and includes the skale-admin chain configs, so a reboot applies -everything atomically. - -### Operator notes - -- Existing nodes get the new policy on the next `skale node update` (or - `skale node configure-firewall`). The setup is idempotent. -- Ship the skale-admin release (which backfills `node_base_port` on admin - restart) before the node-cli release that flips the policy - existing - custom-port nodes then anchor correctly with no manual step. -- If the node was registered with a non-default base port and - `node_config.json` does not contain `node_base_port` yet (admin has not - restarted on the new version), set `SCHAIN_BASE_PORT` to the registration - port. The validation step catches the mismatch and refuses to flip the - policy until it is fixed. -- Custom services listening on other ports must be allowed in - `/etc/nft.conf.d/skale/user.conf` (included at the top of the `skale` - chain) - plain nftables rule lines, e.g. `tcp dport 5000 counter accept`. -- Rollback: set `FIREWALL_DEFAULT_DROP=False` in the environment and rerun - `skale node configure-firewall`. Emergency manual rollback: - `nft add chain inet firewall skale '{ policy accept ; }'`. -- Ports published by Docker containers over bridge networking bypass the - input hook entirely (they are governed by Docker's forward chains); this - firewall governs host-network services, which includes skaled. - -### Known trade-off of the envelope - -Envelope ports with no active sChain are accepted by the firewall and answer -as *closed* (kernel RST) instead of *filtered*, and a stray listener bound -inside the envelope would be reachable. Ports of active sChains are protected -exactly as before by the skale-admin chains. Removing this trade-off is the -goal of phase 2. - -## Planned: sChain ports set (phase 2) - -Goal: unused sChain ports show as *filtered* - the envelope accept is -replaced by an accept driven by an interval set that skale-admin maintains -in lock-step with its chains. Validated end-to-end by a container prototype -(traffic + reboot persistence) on 2026-09-01. - -Design: - -- node-cli declares an interval set in `base.conf` and swaps the envelope - accept for a set-driven accept in the `skale` chain: - - ``` - set schain_ports { - type inet_service - flags interval - } - ... - tcp dport @schain_ports counter accept - ``` - -- skale-admin keeps its per-sChain priority-0 base chains **exactly as they - are** (per-peer accepts + terminal range drop). The only addition: one set - element added/removed together with the chain lifecycle: - - ``` - add element inet firewall schain_ports { 10064-10127 } - ``` - - Traversal: peer/public traffic accepted at priority 0 continues into - `skale`, matches the set, accepted. Strangers on consensus ports die on - the priority-0 terminal drop. Ports with no element fall to policy drop - - filtered, even if something listens there. - -- persistence: each saved chain config in `/etc/nft.conf.d/skale/chains/` - stays self-contained by re-declaring the set with just its element - nft - merges elements across declarations of the same set, so a reboot restores - chains and elements atomically with no re-sync: - - ``` - chain skale-x { ... } - set schain_ports { type inet_service ; flags interval ; elements = { 10064-10127 } } - ``` - -- rollout, each step safe under version skew: - 1. node-cli release: declare the set, add the `@schain_ports` accept, - seed elements from the live `skale-*` chain ranges at configure time - (`get_dynamic_chain_port_ranges`), **keep the envelope**. - 2. skale-admin release: manage elements on chain create/cleanup, include - the element re-declaration in saved chain configs, extend the firewall - health check to verify the element. - 3. node-cli: remove the envelope accept once every live chain range has a - matching set element (checked at configure time; abort otherwise). - -Alternative considered: a verdict map (`type inet_service : verdict`, -elements `{ range : jump skale-x }`) with per-sChain **regular** chains. -Same filtering result, plus per-chain rules only evaluated for their own -ports - but it requires migrating every existing base chain and changing -skale-admin's chain creation, so the set variant is preferred. From abc8af9be75dd90cb326eb6ae21a8e4f9b01a119 Mon Sep 17 00:00:00 2001 From: badrogger Date: Tue, 1 Sep 2026 18:26:51 +0100 Subject: [PATCH 04/16] Fix passive node init --- node_cli/core/nftables.py | 102 +++++++++++++++++++++++++++++--- node_cli/core/node.py | 41 ++++++++++++- node_cli/operations/base.py | 4 +- tests/cli/node_test.py | 26 +++++++++ tests/core/nftables_test.py | 112 +++++++++++++++++++++++++++++++++++- 5 files changed, 271 insertions(+), 14 deletions(-) diff --git a/node_cli/core/nftables.py b/node_cli/core/nftables.py index c391ce91..067de148 100644 --- a/node_cli/core/nftables.py +++ b/node_cli/core/nftables.py @@ -681,6 +681,77 @@ def remove_stale_envelope_rules(self, envelope: tuple[int, int]) -> None: logger.info('Removing stale envelope rule %s', right['range']) self.delete_rule_by_handle(rule['handle']) + @staticmethod + def _normalized_expr(expr: list[dict]) -> list[dict]: + return [{'counter': None} if 'counter' in statement else statement for statement in expr] + + def remove_misordered_udp_drop(self) -> None: + """Delete the blanket udp drop when it shadows the udp DNS accept. + + Rulesets created before the udp payload fix have the drop above the + accept; setup re-adds the drop after all accept rules. + """ + udp_drop = [ + { + 'match': { + 'left': {'payload': {'protocol': 'ip', 'field': 'protocol'}}, + 'op': '==', + 'right': 'udp', + } + }, + {'counter': None}, + {'drop': None}, + ] + udp_dns_accept = [ + { + 'match': { + 'op': '==', + 'left': {'payload': {'protocol': 'udp', 'field': 'dport'}}, + 'right': ServicePort.DNS, + } + }, + {'counter': None}, + {'accept': None}, + ] + drop_handle, drop_index, accept_index = None, None, None + for index, rule in enumerate(self.get_rules(self.chain)): + expr = self._normalized_expr(rule.get('expr', [])) + if expr == udp_drop: + drop_handle, drop_index = rule.get('handle'), index + elif expr == udp_dns_accept: + accept_index = index + if drop_handle is not None and (accept_index is None or drop_index < accept_index): + logger.info('Removing misordered udp drop rule') + self.delete_rule_by_handle(drop_handle) + + def apply_user_rules(self) -> None: + """Load user.conf rules into the live chain. + + The file is included into the saved config, but the live chain is + managed through the API - without this, rules added to the file would + apply only after a reboot and would be missing from the live chain + when the policy flips to drop. + """ + if not os.path.isfile(NFTABLES_USER_CONFIG_PATH): + return + with open(NFTABLES_USER_CONFIG_PATH) as user_config: + lines = [line.strip() for line in user_config.readlines()] + lines = [line for line in lines if line and not line.startswith('#')] + if not lines: + return + current_rules = self.get_base_ruleset() + # insert in reverse to keep the file order at the top of the chain, + # mirroring the include position in the saved config + for line in reversed(lines): + if line in current_rules: + continue + rc, output, error = self.nft.cmd( + f'insert rule {self.family} {self.table} {self.chain} {line}' + ) + if rc != 0: + raise NFTablesError(f'Failed to apply user.conf rule "{line}": {error}') + logger.info('Applied user.conf rule: %s', line) + def get_base_ruleset(self) -> str: self.nft.set_json_output(False) try: @@ -692,17 +763,27 @@ def get_base_ruleset(self) -> str: finally: self.nft.set_json_output(True) - def setup_firewall(self, enable_monitoring: bool = False) -> None: - """Setup firewall rules.""" + def setup_firewall( + self, enable_monitoring: bool = False, defer_default_drop: bool = False + ) -> None: + """Setup firewall rules. + + defer_default_drop keeps the accept policy for now - used when the + envelope base port is not known yet (fresh passive init, where + skale-admin computes it only after the containers start). + """ logger.info('Configuring firewall rules') envelope = get_schain_ports_envelope() - default_drop = firewall_default_drop_enabled() + default_drop = firewall_default_drop_enabled() and not defer_default_drop try: self.create_table_if_not_exists() - # Fail fast, before any rule is touched, if the envelope does not - # cover the chains skale-admin already created on this node - self.validate_dynamic_ranges(envelope) + if default_drop: + # Fail fast, before any rule is touched, if the envelope does + # not cover the chains skale-admin already created on this + # node. Skipped on rollback so that a mismatched envelope + # cannot block restoring the accept policy. + self.validate_dynamic_ranges(envelope) base_chains_config = {'skale': {'hook': 'input', 'policy': 'accept'}} @@ -726,6 +807,7 @@ def setup_firewall(self, enable_monitoring: bool = False) -> None: for port in tcp_ports: self.add_rule(Rule(chain=self.chain, protocol='tcp', first_port=port)) + self.remove_misordered_udp_drop() self.add_rule(Rule(chain=self.chain, protocol='udp', first_port=ServicePort.DNS)) self.add_loopback_rule(chain=self.chain) @@ -763,6 +845,8 @@ def setup_firewall(self, enable_monitoring: bool = False) -> None: chain=LEGACY_CHAIN, policy=POLICY, family=LEGACY_FAMILY, table=LEGACY_TABLE ) + self.apply_user_rules() + if default_drop: self.ensure_default_drop(envelope) else: @@ -857,11 +941,13 @@ def prepare_directories() -> None: create_user_config_path() -def configure_nftables(enable_monitoring: bool = False) -> None: +def configure_nftables(enable_monitoring: bool = False, defer_default_drop: bool = False) -> None: prepare_directories() enable_nftables_service() nft_mgr = NFTablesManager() - nft_mgr.setup_firewall(enable_monitoring=enable_monitoring) + nft_mgr.setup_firewall( + enable_monitoring=enable_monitoring, defer_default_drop=defer_default_drop + ) ruleset = nft_mgr.get_base_ruleset() save_nftables_rules(ruleset) remove_legacy_saved_rules() diff --git a/node_cli/core/node.py b/node_cli/core/node.py index 6898240b..da735846 100644 --- a/node_cli/core/node.py +++ b/node_cli/core/node.py @@ -53,6 +53,7 @@ passive_skale, passive_fair, ) +from node_cli.core.nftables import get_registered_base_port from node_cli.migrations.focal_to_jammy import migrate as migrate_2_6 from node_cli.operations import ( cleanup_skale_op, @@ -148,9 +149,17 @@ def register_node(name, p2p_ip, public_ip, port, domain_name): msg = TEXTS['node']['registered'] logger.info(msg) print(msg) - save_registered_base_port(port) - logger.info('Reconfiguring firewall for the registered base port %d', port) - configure_nftables(enable_monitoring=get_settings().monitoring_containers) + try: + save_registered_base_port(port) + logger.info('Reconfiguring firewall for the registered base port %d', port) + configure_nftables(enable_monitoring=get_settings().monitoring_containers) + except Exception: + # registration already succeeded on-chain - do not fail the command + logger.exception('Post-registration firewall reconfiguration failed') + print( + 'Node is registered, but firewall reconfiguration failed. ' + 'Run < skale node configure-firewall > to complete the setup' + ) else: error_msg = payload logger.error(f'Registration error {error_msg}') @@ -237,9 +246,35 @@ def init_passive( time.sleep(TM_INIT_TIMEOUT) if not is_base_containers_alive(node_type=NodeType.SKALE, node_mode=node_mode): error_exit('Containers are not running', exit_code=CLIExitCodes.OPERATION_EXECUTION_ERROR) + enable_firewall_default_drop_when_port_available(settings) logger.info('Passive node initialized successfully') +def enable_firewall_default_drop_when_port_available( + settings, timeout: int = 300, interval: int = 5 +) -> None: + """Flip the firewall to default drop once admin saves the base port. + + Passive init configures nftables before skale-admin computes the mirrored + chain's base port, so the drop policy is deferred until the port is known. + """ + start = time.monotonic() + while time.monotonic() - start < timeout: + if get_registered_base_port() is not None: + configure_nftables(enable_monitoring=settings.monitoring_containers) + return + time.sleep(interval) + logger.warning( + 'Node base port is not available after %d seconds - firewall default ' + 'drop is postponed until the next node update', + timeout, + ) + print( + 'Firewall default drop policy is postponed: the chain base port is not ' + 'known yet. It will be applied on the next < skale node update-passive >' + ) + + @check_inited @check_user def update_passive(config_file: str) -> None: diff --git a/node_cli/operations/base.py b/node_cli/operations/base.py index 8b777ef9..19542fdc 100644 --- a/node_cli/operations/base.py +++ b/node_cli/operations/base.py @@ -223,7 +223,9 @@ def init_passive( if not settings.skip_docker_config: configure_docker() - configure_nftables(enable_monitoring=settings.monitoring_containers) + # The mirrored chain's base port is computed by skale-admin only after + # the containers start - keep accept policy until it is known + configure_nftables(enable_monitoring=settings.monitoring_containers, defer_default_drop=True) prepare_host(env_type=settings.env_type) save_internal_settings(node_type=NodeType.SKALE, node_mode=NodeMode.PASSIVE) diff --git a/tests/cli/node_test.py b/tests/cli/node_test.py index 52942975..164da0ee 100644 --- a/tests/cli/node_test.py +++ b/tests/cli/node_test.py @@ -77,6 +77,32 @@ def test_register_node(inited_node, resource_alloc, mocked_g_config): ) # noqa +def test_register_node_firewall_failure(inited_node, resource_alloc, mocked_g_config): + """Post-registration firewall errors must not fail the command.""" + resp_mock = response_mock(requests.codes.ok, {'status': 'ok', 'payload': None}) + with ( + mock.patch('node_cli.utils.decorators.is_node_inited', return_value=True), + mock.patch( + 'node_cli.core.node.save_registered_base_port', + side_effect=OSError('disk error'), + ), + mock.patch('node_cli.core.node.configure_nftables'), + mock.patch('node_cli.core.node.get_settings'), + ): + result = run_command_mock( + 'node_cli.utils.helper.requests.post', + resp_mock, + register_node, + ['--name', 'test-node', '--ip', '0.0.0.0', '--port', '8080', '-d', 'skale.test'], + ) + assert result.exit_code == 0 + assert result.output == ( + 'Node registered in SKALE manager.\nFor more info run < skale node info >\n' + 'Node is registered, but firewall reconfiguration failed. ' + 'Run < skale node configure-firewall > to complete the setup\n' + ) + + def test_register_node_with_error(inited_node, resource_alloc, mocked_g_config): resp_mock = response_mock( requests.codes.ok, diff --git a/tests/core/nftables_test.py b/tests/core/nftables_test.py index 4b3b6f36..73f05988 100644 --- a/tests/core/nftables_test.py +++ b/tests/core/nftables_test.py @@ -415,6 +415,7 @@ def test_get_schain_ports_envelope_from_node_config(monkeypatch, tmp_path): def test_setup_firewall(mock_execute, nft_manager, monkeypatch, tmp_path): """Test complete firewall setup.""" monkeypatch.setattr(nftables_core, 'NODE_CONFIG_PATH', str(tmp_path / 'nonexistent.json')) + monkeypatch.setattr(nftables_core, 'NFTABLES_USER_CONFIG_PATH', str(tmp_path / 'user.conf')) with patch.multiple( NFTablesManager, table_exists=Mock(return_value=False), @@ -456,24 +457,131 @@ def test_setup_firewall(mock_execute, nft_manager, monkeypatch, tmp_path): @patch.object(NFTablesManager, 'execute_cmd') def test_setup_firewall_default_drop_disabled(mock_execute, nft_manager, monkeypatch, tmp_path): - """Test that FIREWALL_DEFAULT_DROP=False keeps the accept policy.""" + """Test that FIREWALL_DEFAULT_DROP=False keeps the accept policy. + + Rollback must not be blocked by envelope validation. + """ monkeypatch.setenv('FIREWALL_DEFAULT_DROP', 'False') monkeypatch.setattr(nftables_core, 'NODE_CONFIG_PATH', str(tmp_path / 'nonexistent.json')) + monkeypatch.setattr(nftables_core, 'NFTABLES_USER_CONFIG_PATH', str(tmp_path / 'user.conf')) with patch.multiple( NFTablesManager, table_exists=Mock(return_value=True), chain_exists=Mock(return_value=True), rule_exists=Mock(return_value=True), - get_dynamic_chain_port_ranges=Mock(return_value=[]), + validate_dynamic_ranges=Mock(), ensure_default_drop=Mock(), ensure_default_accept=Mock(), update_chain_policy=Mock(), ): nft_manager.setup_firewall() + NFTablesManager.validate_dynamic_ranges.assert_not_called() NFTablesManager.ensure_default_drop.assert_not_called() NFTablesManager.ensure_default_accept.assert_called_once() +@patch.object(NFTablesManager, 'execute_cmd') +def test_setup_firewall_defer_default_drop(mock_execute, nft_manager, monkeypatch, tmp_path): + """Test that defer_default_drop keeps the accept policy (passive init).""" + monkeypatch.setattr(nftables_core, 'NODE_CONFIG_PATH', str(tmp_path / 'nonexistent.json')) + monkeypatch.setattr(nftables_core, 'NFTABLES_USER_CONFIG_PATH', str(tmp_path / 'user.conf')) + with patch.multiple( + NFTablesManager, + table_exists=Mock(return_value=True), + chain_exists=Mock(return_value=True), + rule_exists=Mock(return_value=True), + validate_dynamic_ranges=Mock(), + ensure_default_drop=Mock(), + ensure_default_accept=Mock(), + update_chain_policy=Mock(), + ): + nft_manager.setup_firewall(defer_default_drop=True) + NFTablesManager.ensure_default_drop.assert_not_called() + NFTablesManager.ensure_default_accept.assert_called_once() + + +def test_remove_misordered_udp_drop(nft_manager): + udp_drop = { + 'handle': 5, + 'expr': [ + { + 'match': { + 'left': {'payload': {'protocol': 'ip', 'field': 'protocol'}}, + 'op': '==', + 'right': 'udp', + } + }, + {'counter': {'packets': 0, 'bytes': 0}}, + {'drop': None}, + ], + } + udp_dns_accept = { + 'handle': 6, + 'expr': [ + { + 'match': { + 'op': '==', + 'left': {'payload': {'protocol': 'udp', 'field': 'dport'}}, + 'right': 53, + } + }, + {'counter': {'packets': 0, 'bytes': 0}}, + {'accept': None}, + ], + } + # drop shadows the accept -> removed + with patch.multiple( + NFTablesManager, + get_rules=Mock(return_value=[udp_drop, udp_dns_accept]), + delete_rule_by_handle=Mock(), + ): + nft_manager.remove_misordered_udp_drop() + NFTablesManager.delete_rule_by_handle.assert_called_once_with(5) + + # accept missing -> drop removed so the accept can land above it + with patch.multiple( + NFTablesManager, + get_rules=Mock(return_value=[udp_drop]), + delete_rule_by_handle=Mock(), + ): + nft_manager.remove_misordered_udp_drop() + NFTablesManager.delete_rule_by_handle.assert_called_once_with(5) + + # correct order -> untouched + with patch.multiple( + NFTablesManager, + get_rules=Mock(return_value=[udp_dns_accept, udp_drop]), + delete_rule_by_handle=Mock(), + ): + nft_manager.remove_misordered_udp_drop() + NFTablesManager.delete_rule_by_handle.assert_not_called() + + +@patch('nftables.Nftables.cmd') +def test_apply_user_rules(mock_cmd, nft_manager, monkeypatch, tmp_path): + user_conf = tmp_path / 'user.conf' + user_conf.write_text( + '# custom services\ntcp dport 5000 counter accept\n\ntcp dport 6000 counter accept\n' + ) + monkeypatch.setattr(nftables_core, 'NFTABLES_USER_CONFIG_PATH', str(user_conf)) + + mock_cmd.return_value = (0, '', '') + with patch.object( + NFTablesManager, + 'get_base_ruleset', + return_value='chain skale {\n\t\ttcp dport 5000 counter accept\n}', + ): + nft_manager.apply_user_rules() + + applied = [call.args[0] for call in mock_cmd.call_args_list] + assert applied == ['insert rule inet filter skale tcp dport 6000 counter accept'] + + mock_cmd.return_value = (1, '', 'syntax error') + with patch.object(NFTablesManager, 'get_base_ruleset', return_value=''): + with pytest.raises(NFTablesError): + nft_manager.apply_user_rules() + + def test_invalid_protocol(nft_manager): """Test adding rule with invalid protocol.""" rule = Rule(chain='INPUT', protocol='invalid', first_port=80) From fb77a0cf1bfbc7c7f7837b4efbf76002c8ea724e Mon Sep 17 00:00:00 2001 From: badrogger Date: Wed, 2 Sep 2026 19:57:09 +0100 Subject: [PATCH 05/16] Small improvements --- node_cli/core/nftables.py | 23 ++++++++--------------- node_cli/core/node.py | 11 +++++++---- node_cli/operations/base.py | 4 +--- tests/cli/node_test.py | 13 +++++++++---- tests/core/nftables_test.py | 6 +++--- 5 files changed, 28 insertions(+), 29 deletions(-) diff --git a/node_cli/core/nftables.py b/node_cli/core/nftables.py index 067de148..1846bd6e 100644 --- a/node_cli/core/nftables.py +++ b/node_cli/core/nftables.py @@ -70,8 +70,6 @@ class SGXPort: POLICY = 'accept' POLICY_DROP = 'drop' -# Prefix of the dynamic per-sChain chains managed by skale-admin -# in the same inet/firewall table (skale-, skale-network-scope, ...) DYNAMIC_CHAIN_PREFIX = 'skale-' # sChain base ports are allocated as node_base_port + schain_index * 64 @@ -82,8 +80,6 @@ class SGXPort: MIN_SCHAIN_BASE_PORT = 2000 MAX_PORT = 65535 -# Without these a drop policy on an inet chain breaks IPv6 neighbor -# discovery and path MTU discovery ICMPV6_ACCEPT_TYPES = ( 'destination-unreachable', 'packet-too-big', @@ -764,25 +760,22 @@ def get_base_ruleset(self) -> str: self.nft.set_json_output(True) def setup_firewall( - self, enable_monitoring: bool = False, defer_default_drop: bool = False + self, enable_monitoring: bool = False, keep_accept_policy: bool = False ) -> None: """Setup firewall rules. - defer_default_drop keeps the accept policy for now - used when the - envelope base port is not known yet (fresh passive init, where - skale-admin computes it only after the containers start). + keep_accept_policy leaves the chain on the accept policy for this + run - used when the envelope base port is not known yet (fresh + passive init, where skale-admin computes it only after the + containers start). """ logger.info('Configuring firewall rules') envelope = get_schain_ports_envelope() - default_drop = firewall_default_drop_enabled() and not defer_default_drop + default_drop = firewall_default_drop_enabled() and not keep_accept_policy try: self.create_table_if_not_exists() if default_drop: - # Fail fast, before any rule is touched, if the envelope does - # not cover the chains skale-admin already created on this - # node. Skipped on rollback so that a mismatched envelope - # cannot block restoring the accept policy. self.validate_dynamic_ranges(envelope) base_chains_config = {'skale': {'hook': 'input', 'policy': 'accept'}} @@ -941,12 +934,12 @@ def prepare_directories() -> None: create_user_config_path() -def configure_nftables(enable_monitoring: bool = False, defer_default_drop: bool = False) -> None: +def configure_nftables(enable_monitoring: bool = False, keep_accept_policy: bool = False) -> None: prepare_directories() enable_nftables_service() nft_mgr = NFTablesManager() nft_mgr.setup_firewall( - enable_monitoring=enable_monitoring, defer_default_drop=defer_default_drop + enable_monitoring=enable_monitoring, keep_accept_policy=keep_accept_policy ) ruleset = nft_mgr.get_base_ruleset() save_nftables_rules(ruleset) diff --git a/node_cli/core/node.py b/node_cli/core/node.py index da735846..55952421 100644 --- a/node_cli/core/node.py +++ b/node_cli/core/node.py @@ -154,11 +154,14 @@ def register_node(name, p2p_ip, public_ip, port, domain_name): logger.info('Reconfiguring firewall for the registered base port %d', port) configure_nftables(enable_monitoring=get_settings().monitoring_containers) except Exception: - # registration already succeeded on-chain - do not fail the command + # on-chain registration already succeeded - retrying register + # would fail, so the error must say the node is registered logger.exception('Post-registration firewall reconfiguration failed') - print( - 'Node is registered, but firewall reconfiguration failed. ' - 'Run < skale node configure-firewall > to complete the setup' + error_exit( + 'Node is successfully registered in SKALE manager, but firewall ' + 'reconfiguration failed. Run < skale node configure-firewall > ' + 'to complete the setup', + exit_code=CLIExitCodes.OPERATION_EXECUTION_ERROR, ) else: error_msg = payload diff --git a/node_cli/operations/base.py b/node_cli/operations/base.py index 19542fdc..21a219dd 100644 --- a/node_cli/operations/base.py +++ b/node_cli/operations/base.py @@ -223,9 +223,7 @@ def init_passive( if not settings.skip_docker_config: configure_docker() - # The mirrored chain's base port is computed by skale-admin only after - # the containers start - keep accept policy until it is known - configure_nftables(enable_monitoring=settings.monitoring_containers, defer_default_drop=True) + configure_nftables(enable_monitoring=settings.monitoring_containers, keep_accept_policy=True) prepare_host(env_type=settings.env_type) save_internal_settings(node_type=NodeType.SKALE, node_mode=NodeMode.PASSIVE) diff --git a/tests/cli/node_test.py b/tests/cli/node_test.py index 164da0ee..8e3ac3d8 100644 --- a/tests/cli/node_test.py +++ b/tests/cli/node_test.py @@ -78,7 +78,7 @@ def test_register_node(inited_node, resource_alloc, mocked_g_config): def test_register_node_firewall_failure(inited_node, resource_alloc, mocked_g_config): - """Post-registration firewall errors must not fail the command.""" + """Post-registration firewall errors fail the command but report the registration.""" resp_mock = response_mock(requests.codes.ok, {'status': 'ok', 'payload': None}) with ( mock.patch('node_cli.utils.decorators.is_node_inited', return_value=True), @@ -95,11 +95,16 @@ def test_register_node_firewall_failure(inited_node, resource_alloc, mocked_g_co register_node, ['--name', 'test-node', '--ip', '0.0.0.0', '--port', '8080', '-d', 'skale.test'], ) - assert result.exit_code == 0 + assert result.exit_code == CLIExitCodes.OPERATION_EXECUTION_ERROR.value assert result.output == ( 'Node registered in SKALE manager.\nFor more info run < skale node info >\n' - 'Node is registered, but firewall reconfiguration failed. ' - 'Run < skale node configure-firewall > to complete the setup\n' + 'Command failed with following errors:\n' + '--------------------------------------------------\n' + 'Node is successfully registered in SKALE manager, but firewall ' + 'reconfiguration failed. Run < skale node configure-firewall > ' + 'to complete the setup\n' + '--------------------------------------------------\n' + f'You can find more info in {G_CONF_HOME}.skale/.skale-cli-log/debug-node-cli.log\n' ) diff --git a/tests/core/nftables_test.py b/tests/core/nftables_test.py index 73f05988..313ff35e 100644 --- a/tests/core/nftables_test.py +++ b/tests/core/nftables_test.py @@ -481,8 +481,8 @@ def test_setup_firewall_default_drop_disabled(mock_execute, nft_manager, monkeyp @patch.object(NFTablesManager, 'execute_cmd') -def test_setup_firewall_defer_default_drop(mock_execute, nft_manager, monkeypatch, tmp_path): - """Test that defer_default_drop keeps the accept policy (passive init).""" +def test_setup_firewall_keep_accept_policy(mock_execute, nft_manager, monkeypatch, tmp_path): + """Test that keep_accept_policy skips the drop flip (passive init).""" monkeypatch.setattr(nftables_core, 'NODE_CONFIG_PATH', str(tmp_path / 'nonexistent.json')) monkeypatch.setattr(nftables_core, 'NFTABLES_USER_CONFIG_PATH', str(tmp_path / 'user.conf')) with patch.multiple( @@ -495,7 +495,7 @@ def test_setup_firewall_defer_default_drop(mock_execute, nft_manager, monkeypatc ensure_default_accept=Mock(), update_chain_policy=Mock(), ): - nft_manager.setup_firewall(defer_default_drop=True) + nft_manager.setup_firewall(keep_accept_policy=True) NFTablesManager.ensure_default_drop.assert_not_called() NFTablesManager.ensure_default_accept.assert_called_once() From 200d83be0443fd7908152955fdc9a323a382b2f3 Mon Sep 17 00:00:00 2001 From: badrogger Date: Tue, 8 Sep 2026 17:47:01 +0100 Subject: [PATCH 06/16] Improve exception handling --- node_cli/core/nftables.py | 55 ++++++++++++++++++++---- tests/core/nftables_test.py | 86 +++++++++++++++++++++++++++++++++++++ 2 files changed, 132 insertions(+), 9 deletions(-) diff --git a/node_cli/core/nftables.py b/node_cli/core/nftables.py index 1846bd6e..01efb336 100644 --- a/node_cli/core/nftables.py +++ b/node_cli/core/nftables.py @@ -218,10 +218,15 @@ def get_chain_policy(self, chain: str) -> Optional[str]: if rc != 0: return None data = json.loads(output) + if not isinstance(data, dict): + return None for item in data.get('nftables', []): - if 'chain' in item and item['chain'].get('name') == chain: - return item['chain'].get('policy') - except Exception as e: + if not isinstance(item, dict): + continue + chain_data = item.get('chain') + if isinstance(chain_data, dict) and chain_data.get('name') == chain: + return chain_data.get('policy') + except (TypeError, ValueError) as e: logger.error('Failed to get policy of chain %s: %s', chain, e) return None @@ -636,8 +641,12 @@ def ensure_default_drop(self, envelope: tuple[int, int]) -> None: self.update_chain_policy(chain=self.chain, policy=POLICY_DROP) def ensure_default_accept(self) -> None: - """Rollback path: switch the skale chain policy back to accept.""" - if self.get_chain_policy(self.chain) == POLICY_DROP: + """Rollback path: switch the skale chain policy back to accept. + + Flips whenever the policy cannot be confirmed as accept, so an + unreadable policy does not silently skip the rollback. + """ + if self.get_chain_policy(self.chain) != POLICY: self.update_chain_policy(chain=self.chain, policy=POLICY) def delete_rule_by_handle(self, handle: int) -> None: @@ -720,6 +729,27 @@ def remove_misordered_udp_drop(self) -> None: logger.info('Removing misordered udp drop rule') self.delete_rule_by_handle(drop_handle) + def remove_source_quench_rule(self) -> None: + """Remove the legacy icmp source-quench accept (deprecated by RFC 6633).""" + expr = [ + { + 'match': { + 'left': {'payload': {'protocol': 'icmp', 'field': 'type'}}, + 'op': '==', + 'right': 'source-quench', + } + }, + {'counter': None}, + {'accept': None}, + ] + for rule in self.get_rules(self.chain): + if ( + self._normalized_expr(rule.get('expr', [])) == expr + and rule.get('handle') is not None + ): + logger.info('Removing legacy source-quench rule') + self.delete_rule_by_handle(rule['handle']) + def apply_user_rules(self) -> None: """Load user.conf rules into the live chain. @@ -804,7 +834,8 @@ def setup_firewall( self.add_rule(Rule(chain=self.chain, protocol='udp', first_port=ServicePort.DNS)) self.add_loopback_rule(chain=self.chain) - icmp_types = ['destination-unreachable', 'source-quench', 'time-exceeded'] + self.remove_source_quench_rule() + icmp_types = ['destination-unreachable', 'time-exceeded'] for icmp_type in icmp_types: self.add_rule(Rule(chain=self.chain, protocol='icmp', icmp_type=icmp_type)) @@ -906,11 +937,17 @@ def get_registered_base_port() -> Optional[int]: return None try: node_config = read_json(NODE_CONFIG_PATH) - except Exception as e: + except (OSError, ValueError) as e: logger.warning('Failed to read node config: %s', e) return None - base_port = node_config.get('node_base_port') or node_config.get('schain_base_port') or 0 - return base_port if base_port > 0 else None + if not isinstance(node_config, dict): + logger.warning('Node config is malformed') + return None + for key in ('node_base_port', 'schain_base_port'): + value = node_config.get(key) + if isinstance(value, int) and not isinstance(value, bool) and value > 0: + return value + return None def get_schain_ports_envelope() -> tuple[int, int]: diff --git a/tests/core/nftables_test.py b/tests/core/nftables_test.py index 313ff35e..4c3d6776 100644 --- a/tests/core/nftables_test.py +++ b/tests/core/nftables_test.py @@ -143,6 +143,14 @@ def test_get_chain_policy(mock_cmd, nft_manager): mock_cmd.return_value = (1, '', 'No such file or directory') assert nft_manager.get_chain_policy('skale') is None + mock_cmd.return_value = (0, 'not-json', '') + assert nft_manager.get_chain_policy('skale') is None + + # malformed but valid JSON must not raise - rollback depends on it + for output in ('[]', 'null', '{"nftables": [{"chain": null}]}', '{"nftables": "x"}'): + mock_cmd.return_value = (0, output, '') + assert nft_manager.get_chain_policy('skale') is None + @pytest.mark.parametrize( 'rule_data', @@ -321,6 +329,15 @@ def test_ensure_default_accept(nft_manager): nft_manager.ensure_default_accept() NFTablesManager.update_chain_policy.assert_not_called() + # unreadable policy must not skip the rollback + with patch.multiple( + NFTablesManager, + get_chain_policy=Mock(return_value=None), + update_chain_policy=Mock(), + ): + nft_manager.ensure_default_accept() + NFTablesManager.update_chain_policy.assert_called_once_with(chain='skale', policy='accept') + def test_remove_stale_envelope_rules(nft_manager): stale_rule = { @@ -396,6 +413,26 @@ def test_get_schain_ports_envelope_env_override(monkeypatch): get_schain_ports_envelope() +def test_get_schain_ports_envelope_malformed_node_config(monkeypatch, tmp_path): + config_path = tmp_path / 'node_config.json' + monkeypatch.setattr(nftables_core, 'NODE_CONFIG_PATH', str(config_path)) + + for content in ( + 'not-json', + '[1, 2]', + '{"node_base_port": "not-a-port"}', + '{"node_base_port": true}', + '{"node_base_port": -1}', + ): + config_path.write_text(content) + # falls back to the default base port + assert get_schain_ports_envelope() == (10000, 18191) + + # an invalid primary value must not mask a valid fallback + config_path.write_text(json.dumps({'node_base_port': 'bad', 'schain_base_port': 20128})) + assert get_schain_ports_envelope() == (20128, 28319) + + def test_get_schain_ports_envelope_from_node_config(monkeypatch, tmp_path): config_path = tmp_path / 'node_config.json' monkeypatch.setattr(nftables_core, 'NODE_CONFIG_PATH', str(config_path)) @@ -448,6 +485,9 @@ def test_setup_firewall(mock_execute, nft_manager, monkeypatch, tmp_path): == 'icmpv6' ] assert len(icmpv6_exprs) == len(nftables_core.ICMPV6_ACCEPT_TYPES) + assert not any( + expr[0].get('match', {}).get('right') == 'source-quench' for expr in added_exprs + ) NFTablesManager.update_chain_policy.assert_any_call( chain='INPUT', policy='accept', family='ip', table='filter' @@ -500,6 +540,52 @@ def test_setup_firewall_keep_accept_policy(mock_execute, nft_manager, monkeypatc NFTablesManager.ensure_default_accept.assert_called_once() +def test_remove_source_quench_rule(nft_manager): + source_quench_rule = { + 'handle': 11, + 'expr': [ + { + 'match': { + 'left': {'payload': {'protocol': 'icmp', 'field': 'type'}}, + 'op': '==', + 'right': 'source-quench', + } + }, + {'counter': {'packets': 0, 'bytes': 0}}, + {'accept': None}, + ], + } + other_rule = { + 'handle': 12, + 'expr': [ + { + 'match': { + 'left': {'payload': {'protocol': 'icmp', 'field': 'type'}}, + 'op': '==', + 'right': 'destination-unreachable', + } + }, + {'counter': {'packets': 0, 'bytes': 0}}, + {'accept': None}, + ], + } + with patch.multiple( + NFTablesManager, + get_rules=Mock(return_value=[source_quench_rule, other_rule]), + delete_rule_by_handle=Mock(), + ): + nft_manager.remove_source_quench_rule() + NFTablesManager.delete_rule_by_handle.assert_called_once_with(11) + + with patch.multiple( + NFTablesManager, + get_rules=Mock(return_value=[other_rule]), + delete_rule_by_handle=Mock(), + ): + nft_manager.remove_source_quench_rule() + NFTablesManager.delete_rule_by_handle.assert_not_called() + + def test_remove_misordered_udp_drop(nft_manager): udp_drop = { 'handle': 5, From ef28f02c63a2ae803f4903573c19c57385d435f9 Mon Sep 17 00:00:00 2001 From: badrogger Date: Wed, 9 Sep 2026 20:12:50 +0100 Subject: [PATCH 07/16] Extract user rules to seprate chain --- node_cli/core/nftables.py | 768 ++++++++++---------------- node_cli/core/node.py | 16 +- node_cli/migrations/focal_to_jammy.py | 4 +- tests/cli/passive_node_test.py | 2 + tests/core/nftables_test.py | 127 ++++- 5 files changed, 409 insertions(+), 508 deletions(-) diff --git a/node_cli/core/nftables.py b/node_cli/core/nftables.py index 01efb336..de1e8ef7 100644 --- a/node_cli/core/nftables.py +++ b/node_cli/core/nftables.py @@ -40,6 +40,16 @@ logger = logging.getLogger(__name__) +try: + import nftables +except (FileNotFoundError, AttributeError, ModuleNotFoundError) as err: + if 'pytest' in sys.modules or ENV == 'dev': + from collections import namedtuple # hotfix for tests + + iptc = namedtuple('nftables', ['Chain', 'Rule']) + else: + logger.error(f'Unable to import nftables due to an error {err}') + @dataclass class ServicePort: @@ -67,11 +77,13 @@ class SGXPort: LEGACY_TABLE = 'filter' CHAIN_PRIORITY = 1 HOOK = 'input' -POLICY = 'accept' +POLICY_ACCEPT = 'accept' POLICY_DROP = 'drop' DYNAMIC_CHAIN_PREFIX = 'skale-' +USER_CHAIN = 'skale_user' + # sChain base ports are allocated as node_base_port + schain_index * 64 # (PORTS_PER_SCHAIN in skale.py); 128 slots cover every possible allocation SCHAIN_PORTS_PER_NODE = 128 * 64 @@ -80,6 +92,7 @@ class SGXPort: MIN_SCHAIN_BASE_PORT = 2000 MAX_PORT = 65535 +ICMP_ACCEPT_TYPES = ('destination-unreachable', 'time-exceeded') ICMPV6_ACCEPT_TYPES = ( 'destination-unreachable', 'packet-too-big', @@ -91,19 +104,61 @@ class SGXPort: ) -try: - import nftables -except (FileNotFoundError, AttributeError, ModuleNotFoundError) as err: - if 'pytest' in sys.modules or ENV == 'dev': - from collections import namedtuple # hotfix for tests +class NFTablesError(Exception): + pass - iptc = namedtuple('nftables', ['Chain', 'Rule']) - else: - logger.error(f'Unable to import nftables due to an error {err}') +def dport_match(protocol: str, first_port: int, last_port: int) -> dict: + right = first_port if last_port == first_port else {'range': [first_port, last_port]} + return { + 'match': { + 'op': '==', + 'left': {'payload': {'protocol': protocol, 'field': 'dport'}}, + 'right': right, + } + } + + +def icmp_match(protocol: str, icmp_type: str) -> dict: + return { + 'match': { + 'left': {'payload': {'protocol': protocol, 'field': 'type'}}, + 'op': '==', + 'right': icmp_type, + } + } + + +def ip_protocol_match(protocol: str) -> dict: + return { + 'match': { + 'left': {'payload': {'protocol': 'ip', 'field': 'protocol'}}, + 'op': '==', + 'right': protocol, + } + } + + +def conntrack_accept_expr() -> list[dict]: + return [ + { + 'match': { + 'left': {'ct': {'key': 'state'}}, + 'op': 'in', + 'right': ['established', 'related'], + } + }, + {'counter': None}, + {'accept': None}, + ] -class NFTablesError(Exception): - pass + +def loopback_accept_expr() -> list[dict]: + return [ + {'match': {'left': {'meta': {'key': 'iifname'}}, 'op': '==', 'right': 'lo'}}, + {'counter': None}, + {'accept': None}, + ] @dataclass @@ -118,10 +173,16 @@ class Rule: def __post_init__(self): if self.first_port is not None and self.last_port is None: self.last_port = self.first_port - if all( - val is None for val in (self.first_port, self.last_port, self.protocol, self.icmp_type) - ): - raise NFTablesError('Rule has no meaningful fields') + if self.protocol in ('icmp', 'icmpv6') and not self.icmp_type: + raise NFTablesError(f'{self.protocol} rule requires icmp_type') + + def to_expr(self) -> list[dict]: + matches = [] + if self.protocol in ('tcp', 'udp') and self.first_port: + matches.append(dport_match(self.protocol, self.first_port, self.last_port)) + elif self.protocol in ('icmp', 'icmpv6'): + matches.append(icmp_match(self.protocol, self.icmp_type)) + return [*matches, {'counter': None}, {self.action: None}] class NFTablesManager: @@ -167,7 +228,7 @@ def chain_exists(self, chain: str, family: Optional[str] = None) -> bool: return chain in self.get_chains(family=family) def create_chain_if_not_exists( - self, chain: str, hook: str, priority: int = CHAIN_PRIORITY, policy: str = POLICY + self, chain: str, hook: str, priority: int = CHAIN_PRIORITY, policy: str = POLICY_ACCEPT ) -> None: if not self.chain_exists(chain): cmd = { @@ -195,7 +256,7 @@ def create_chain_if_not_exists( def update_chain_policy( self, chain: str, - policy: str = POLICY, + policy: str = POLICY_ACCEPT, family: Optional[str] = None, table: Optional[str] = None, ) -> None: @@ -262,302 +323,90 @@ def get_rules(self, chain: str) -> list[dict]: logger.error('Failed to get rules: %s', e) return [] + @staticmethod + def _normalized_expr(expr: list[dict]) -> list[dict]: + return [{'counter': None} if 'counter' in statement else statement for statement in expr] + def rule_exists(self, chain: str, new_rule_expr: list[dict]) -> bool: - existing_rules = self.get_rules(chain) - - for rule in existing_rules: - expr = rule.get('expr') - for i, statement in enumerate(expr): - if 'counter' in statement: - expr[i] = {'counter': None} - rule['counter'] = None - if expr == new_rule_expr: - return True - return False + target = self._normalized_expr(new_rule_expr) + return any( + self._normalized_expr(rule.get('expr', [])) == target for rule in self.get_rules(chain) + ) - def add_drop_rule(self, rule: Rule) -> None: - expr = [] + def _execute_rule_with_op( + self, + op: str, + chain: str, + expr: Optional[list[dict]] = None, + handle: Optional[int] = None, + ) -> None: + rule: dict = {'family': self.family, 'table': self.table, 'chain': chain} + if expr is not None: + rule['expr'] = expr + if handle is not None: + rule['handle'] = handle + self.execute_cmd({'nftables': [{op: {'rule': rule}}]}) + + def _ensure_rule( + self, chain: str, expr: list[dict], op: str = 'add', label: str = 'rule' + ) -> None: + if self.rule_exists(chain, expr): + logger.info('%s already exists in chain %s', label, chain) + return + self._execute_rule_with_op(op, chain, expr=expr) + logger.info('Added %s to chain %s', label, chain) + def _find_rule_handle(self, chain: str, expr: list[dict]) -> Optional[int]: + target = self._normalized_expr(expr) + for rule in self.get_rules(chain): + if ( + self._normalized_expr(rule.get('expr', [])) == target + and rule.get('handle') is not None + ): + return rule['handle'] + return None + + def _remove_rule_by_expr(self, chain: str, expr: list[dict]) -> bool: + handle = self._find_rule_handle(chain, expr) + if handle is None: + return False + self.delete_rule_by_handle(handle, chain=chain) + return True + + @staticmethod + def _protocol_drop_expr(rule: Rule) -> list[dict]: + matches = [] if rule.first_port: - if rule.last_port == rule.first_port: - expr.append( - { - 'match': { - 'op': '==', - 'left': {'payload': {'protocol': 'tcp', 'field': 'dport'}}, - 'right': rule.first_port, - } - } - ) - else: - expr.append( - { - 'match': { - 'op': '==', - 'left': {'payload': {'protocol': 'tcp', 'field': 'dport'}}, - 'right': {'range': [rule.first_port, rule.last_port]}, - } - } - ) - expr.append( - { - 'match': { - 'left': {'payload': {'protocol': 'ip', 'field': 'protocol'}}, - 'op': '==', - 'right': rule.protocol, - } - }, + matches.append(dport_match('tcp', rule.first_port, rule.last_port)) + matches.append(ip_protocol_match(rule.protocol)) + return [*matches, {'counter': None}, {'drop': None}] + + def add_drop_rule(self, rule: Rule) -> None: + self._ensure_rule( + rule.chain, + self._protocol_drop_expr(rule), + label=f'{rule.protocol} drop rule', ) - expr.extend([{'counter': None}, {'drop': None}]) - if not self.rule_exists(self.chain, expr): - cmd = { - 'nftables': [ - { - 'add': { - 'rule': { - 'family': self.family, - 'table': self.table, - 'chain': rule.chain, - 'expr': expr, - } - } - } - ] - } - self.execute_cmd(cmd) - logger.info('Added drop rule %s', Rule) def remove_drop_rule(self, protocol: str) -> None: - expr = [ - { - 'match': { - 'op': '==', - 'left': {'payload': {'protocol': 'ip', 'field': 'protocol'}}, - 'right': protocol, - } - }, - {'counter': None}, - {'drop': None}, - ] - - # Check if the drop rule exists before attempting to remove it - if self.rule_exists(self.chain, expr): - cmd = { - 'nftables': [ - { - 'delete': { - 'rule': { - 'family': self.family, - 'table': self.table, - 'chain': self.chain, - 'expr': expr, - } - } - } - ] - } - self.execute_cmd(cmd) + expr = [ip_protocol_match(protocol), {'counter': None}, {'drop': None}] + if self._remove_rule_by_expr(self.chain, expr): logger.info('Removed drop rule for %s', protocol) else: logger.info('Drop rule does not exist for %s', protocol) def add_rule(self, rule: Rule) -> None: - expr = [] - - if rule.protocol in ['tcp', 'udp']: - if rule.first_port: - if rule.last_port == rule.first_port: - expr.append( - { - 'match': { - 'op': '==', - 'left': {'payload': {'protocol': rule.protocol, 'field': 'dport'}}, - 'right': rule.first_port, - } - } - ) - else: - expr.append( - { - 'match': { - 'op': '==', - 'left': {'payload': {'protocol': rule.protocol, 'field': 'dport'}}, - 'right': {'range': [rule.first_port, rule.last_port]}, - } - } - ) - elif rule.protocol in ['icmp', 'icmpv6'] and rule.icmp_type: - expr.append( - { - 'match': { - 'left': {'payload': {'protocol': rule.protocol, 'field': 'type'}}, - 'op': '==', - 'right': rule.icmp_type, - } - } - ) - - expr.append({'counter': None}) - expr.append({rule.action: None}) - - if not self.rule_exists(rule.chain, expr): - cmd = { - 'nftables': [ - { - 'add': { - 'rule': { - 'family': self.family, - 'table': self.table, - 'chain': rule.chain, - 'expr': expr, - } - } - } - ] - } - self.execute_cmd(cmd) - logger.info( - 'Added new rule to chain %s: %s ports [%s, %s]', - rule.chain, - rule.protocol, - rule.first_port, - rule.last_port, - ) - else: - logger.info( - 'Rule already exists in chain %s: %s ports [%s, %s]', - rule.chain, - rule.protocol, - rule.first_port, - rule.last_port, - ) + self._ensure_rule( + rule.chain, + rule.to_expr(), + label=f'{rule.protocol} {rule.icmp_type or rule.first_port} {rule.action} rule', + ) def remove_rule(self, rule: Rule) -> None: - expr = [] - - if rule.protocol in ['tcp', 'udp']: - if rule.first_port: - if rule.last_port == rule.first_port: - expr.append( - { - 'match': { - 'op': '==', - 'left': {'payload': {'protocol': rule.protocol, 'field': 'dport'}}, - 'right': rule.first_port, - } - } - ) - else: - expr.append( - { - 'match': { - 'op': '==', - 'left': {'payload': {'protocol': rule.protocol, 'field': 'dport'}}, - 'right': {'range': [rule.first_port, rule.last_port]}, - } - } - ) - elif rule.protocol == 'icmp' and rule.icmp_type: - expr.append( - { - 'match': { - 'left': {'payload': {'protocol': 'icmp', 'field': 'type'}}, - 'op': '==', - 'right': rule.icmp_type, - } - } - ) - - # Check if the rule exists before attempting to remove it - if self.rule_exists(rule.chain, expr): - cmd = { - 'nftables': [ - { - 'delete': { - 'rule': { - 'family': self.family, - 'table': self.table, - 'chain': rule.chain, - 'expr': expr, - } - } - } - ] - } - self.execute_cmd(cmd) - logger.info( - 'Removed rule from chain %s: %s ports [%s, %s]', - rule.chain, - rule.protocol, - rule.first_port, - rule.last_port, - ) - else: - logger.info( - 'Rule does not exist in chain %s: %s ports [%s, %s]', - rule.chain, - rule.protocol, - rule.first_port, - rule.last_port, - ) - - def add_connection_tracking_rule(self, chain: str) -> None: - expr = [ - { - 'match': { - 'left': {'ct': {'key': 'state'}}, - 'op': 'in', - 'right': ['established', 'related'], - } - }, - {'counter': None}, - {'accept': None}, - ] - - if not self.rule_exists(chain, expr): - cmd = { - 'nftables': [ - { - 'add': { - 'rule': { - 'family': self.family, - 'table': self.table, - 'chain': chain, - 'expr': expr, - } - } - } - ] - } - self.execute_cmd(cmd) - logger.info('Added connection tracking rule to chain %s', chain) - else: - logger.info('Connection tracking rule already exists in chain %s', chain) - - def add_loopback_rule(self, chain) -> None: - expr = [ - {'match': {'left': {'meta': {'key': 'iifname'}}, 'op': '==', 'right': 'lo'}}, - {'counter': None}, - {'accept': None}, - ] - if not self.rule_exists(chain, expr): - json_cmd = { - 'nftables': [ - { - 'add': { - 'rule': { - 'family': self.family, - 'table': self.table, - 'chain': self.chain, - 'expr': expr, - } - } - } - ] - } - self.execute_cmd(json_cmd) + if self._remove_rule_by_expr(rule.chain, rule.to_expr()): + logger.info('Removed %s rule for %s', rule.protocol, rule.first_port) else: - logger.info('Loopback rule already exists in chain %s', chain) + logger.info('No %s rule for %s to remove', rule.protocol, rule.first_port) def get_dynamic_chain_port_ranges(self) -> list[tuple[str, int, int]]: """Min/max tcp dport covered by each dynamic skale-admin chain.""" @@ -604,67 +453,27 @@ def validate_dynamic_ranges(self, envelope: tuple[int, int]) -> None: def verify_critical_accepts(self) -> None: """Ensure lockout-critical accept rules are in place before setting drop policy.""" - conntrack_expr = [ - { - 'match': { - 'left': {'ct': {'key': 'state'}}, - 'op': 'in', - 'right': ['established', 'related'], - } - }, - {'counter': None}, - {'accept': None}, - ] - ssh_expr = [ - { - 'match': { - 'op': '==', - 'left': {'payload': {'protocol': 'tcp', 'field': 'dport'}}, - 'right': get_ssh_port(), - } - }, - {'counter': None}, - {'accept': None}, - ] - for name, expr in (('conntrack', conntrack_expr), ('ssh', ssh_expr)): + ssh_port = get_ssh_port() + ssh_expr = Rule(chain=self.chain, protocol='tcp', first_port=ssh_port).to_expr() + for name, expr in (('conntrack', conntrack_accept_expr()), ('ssh', ssh_expr)): if not self.rule_exists(self.chain, expr): raise NFTablesError( f'Refusing to set drop policy: {name} accept rule is missing ' f'in chain {self.chain}' ) - def ensure_default_drop(self, envelope: tuple[int, int]) -> None: - """Switch the skale chain policy to drop after validating the accepts.""" - self.validate_dynamic_ranges(envelope) + def ensure_default_drop(self) -> None: + """Switch the skale chain policy to drop after verifying the accepts.""" self.verify_critical_accepts() if self.get_chain_policy(self.chain) != POLICY_DROP: self.update_chain_policy(chain=self.chain, policy=POLICY_DROP) def ensure_default_accept(self) -> None: - """Rollback path: switch the skale chain policy back to accept. + if self.get_chain_policy(self.chain) != POLICY_ACCEPT: + self.update_chain_policy(chain=self.chain, policy=POLICY_ACCEPT) - Flips whenever the policy cannot be confirmed as accept, so an - unreadable policy does not silently skip the rollback. - """ - if self.get_chain_policy(self.chain) != POLICY: - self.update_chain_policy(chain=self.chain, policy=POLICY) - - def delete_rule_by_handle(self, handle: int) -> None: - cmd = { - 'nftables': [ - { - 'delete': { - 'rule': { - 'family': self.family, - 'table': self.table, - 'chain': self.chain, - 'handle': handle, - } - } - } - ] - } - self.execute_cmd(cmd) + def delete_rule_by_handle(self, handle: int, chain: Optional[str] = None) -> None: + self._execute_rule_with_op('delete', chain or self.chain, handle=handle) def remove_stale_envelope_rules(self, envelope: tuple[int, int]) -> None: """Remove sChain envelope accepts anchored at a different base port.""" @@ -696,25 +505,9 @@ def remove_misordered_udp_drop(self) -> None: Rulesets created before the udp payload fix have the drop above the accept; setup re-adds the drop after all accept rules. """ - udp_drop = [ - { - 'match': { - 'left': {'payload': {'protocol': 'ip', 'field': 'protocol'}}, - 'op': '==', - 'right': 'udp', - } - }, - {'counter': None}, - {'drop': None}, - ] + udp_drop = [ip_protocol_match('udp'), {'counter': None}, {'drop': None}] udp_dns_accept = [ - { - 'match': { - 'op': '==', - 'left': {'payload': {'protocol': 'udp', 'field': 'dport'}}, - 'right': ServicePort.DNS, - } - }, + dport_match('udp', ServicePort.DNS, ServicePort.DNS), {'counter': None}, {'accept': None}, ] @@ -731,52 +524,78 @@ def remove_misordered_udp_drop(self) -> None: def remove_source_quench_rule(self) -> None: """Remove the legacy icmp source-quench accept (deprecated by RFC 6633).""" - expr = [ - { - 'match': { - 'left': {'payload': {'protocol': 'icmp', 'field': 'type'}}, - 'op': '==', - 'right': 'source-quench', - } - }, - {'counter': None}, - {'accept': None}, - ] - for rule in self.get_rules(self.chain): - if ( - self._normalized_expr(rule.get('expr', [])) == expr - and rule.get('handle') is not None - ): - logger.info('Removing legacy source-quench rule') - self.delete_rule_by_handle(rule['handle']) + expr = [icmp_match('icmp', 'source-quench'), {'counter': None}, {'accept': None}] + if self._remove_rule_by_expr(self.chain, expr): + logger.info('Removed legacy source-quench rule') - def apply_user_rules(self) -> None: - """Load user.conf rules into the live chain. + def create_user_chain_if_not_exists(self) -> None: + """Create the regular chain holding user.conf rules.""" + if not self.chain_exists(USER_CHAIN): + cmd = { + 'nftables': [ + { + 'add': { + 'chain': { + 'family': self.family, + 'table': self.table, + 'name': USER_CHAIN, + } + } + } + ] + } + self.execute_cmd(cmd) + logger.info('Created user rules chain %s', USER_CHAIN) - The file is included into the saved config, but the live chain is - managed through the API - without this, rules added to the file would - apply only after a reboot and would be missing from the live chain - when the policy flips to drop. - """ + def ensure_user_chain_jump(self) -> None: + expr = [{'jump': {'target': USER_CHAIN}}] + self._ensure_rule(self.chain, expr, op='insert', label='user chain jump') + + @staticmethod + def read_user_rule_lines() -> list[str]: if not os.path.isfile(NFTABLES_USER_CONFIG_PATH): - return + return [] with open(NFTABLES_USER_CONFIG_PATH) as user_config: lines = [line.strip() for line in user_config.readlines()] - lines = [line for line in lines if line and not line.startswith('#')] + return [line for line in lines if line and not line.startswith('#')] + + def apply_user_rules(self) -> None: + """Reload user.conf into the live user rules chain. + Flush plus re-add in one transaction keeps the chain exactly in sync + with the file + """ + commands = [f'flush chain {self.family} {self.table} {USER_CHAIN}'] + commands.extend( + f'add rule {self.family} {self.table} {USER_CHAIN} {line}' + for line in self.read_user_rule_lines() + ) + rc, output, error = self.nft.cmd('\n'.join(commands)) + if rc != 0: + raise NFTablesError(f'Failed to apply user.conf rules: {error}') + + def remove_user_rules_from_main_chain(self) -> None: + """Remove user.conf rules that older saved configs loaded into the + skale chain directly, so they are not snapshotted as duplicates.""" + lines = self.read_user_rule_lines() if not lines: return - current_rules = self.get_base_ruleset() - # insert in reverse to keep the file order at the top of the chain, - # mirroring the include position in the saved config - for line in reversed(lines): - if line in current_rules: + self.nft.set_json_output(False) + self.nft.set_handle_output(True) + try: + rc, output, error = self.nft.cmd(f'list chain {self.family} {self.table} {self.chain}') + finally: + self.nft.set_handle_output(False) + self.nft.set_json_output(True) + if rc != 0: + return + for listed in output.split('\n'): + listed = listed.strip() + if ' # handle ' not in listed: continue - rc, output, error = self.nft.cmd( - f'insert rule {self.family} {self.table} {self.chain} {line}' - ) - if rc != 0: - raise NFTablesError(f'Failed to apply user.conf rule "{line}": {error}') - logger.info('Applied user.conf rule: %s', line) + rule_text, _, handle = listed.rpartition(' # handle ') + if rule_text in lines and handle.isdigit(): + logger.info('Moving user rule out of the main chain: %s', rule_text) + self.delete_rule_by_handle(int(handle)) def get_base_ruleset(self) -> str: self.nft.set_json_output(False) @@ -789,16 +608,54 @@ def get_base_ruleset(self) -> str: finally: self.nft.set_json_output(True) + def _setup_user_chain(self) -> None: + self.create_user_chain_if_not_exists() + self.ensure_user_chain_jump() + self.remove_user_rules_from_main_chain() + + def _add_service_accepts(self, enable_monitoring: bool) -> None: + self._ensure_rule(self.chain, conntrack_accept_expr(), label='connection tracking rule') + tcp_ports = [ + get_ssh_port(), + ServicePort.DNS, + ServicePort.HTTPS, + ServicePort.HTTP, + ServicePort.WATCHDOG_HTTP, + ServicePort.WATCHDOG_HTTPS, + ] + if enable_monitoring: + tcp_ports.extend([ServicePort.EXPORTER, ServicePort.CADVISOR]) + for port in tcp_ports: + self.add_rule(Rule(chain=self.chain, protocol='tcp', first_port=port)) + self.remove_misordered_udp_drop() + self.add_rule(Rule(chain=self.chain, protocol='udp', first_port=ServicePort.DNS)) + self._ensure_rule(self.chain, loopback_accept_expr(), label='loopback rule') + + def _add_icmp_accepts(self) -> None: + self.remove_source_quench_rule() + for icmp_type in ICMP_ACCEPT_TYPES: + self.add_rule(Rule(chain=self.chain, protocol='icmp', icmp_type=icmp_type)) + for icmpv6_type in ICMPV6_ACCEPT_TYPES: + self.add_rule(Rule(chain=self.chain, protocol='icmpv6', icmp_type=icmpv6_type)) + + def _ensure_envelope(self, envelope: tuple[int, int]) -> None: + # Fine-grained filtering inside the envelope is enforced by the + # dynamic skale-admin chains that run earlier (priority 0) + self.remove_stale_envelope_rules(envelope) + self.add_rule( + Rule(chain=self.chain, protocol='tcp', first_port=envelope[0], last_port=envelope[1]) + ) + + def _add_drop_rules(self) -> None: + self.add_drop_rule( + Rule(chain=self.chain, protocol='tcp', first_port=SGXPort.HTTPS, last_port=SGXPort.ZMQ) + ) + self.add_drop_rule(Rule(chain=self.chain, protocol='udp')) + def setup_firewall( self, enable_monitoring: bool = False, keep_accept_policy: bool = False ) -> None: - """Setup firewall rules. - - keep_accept_policy leaves the chain on the accept policy for this - run - used when the envelope base port is not known yet (fresh - passive init, where skale-admin computes it only after the - containers start). - """ + """Setup firewall rules.""" logger.info('Configuring firewall rules') envelope = get_schain_ports_envelope() @@ -806,82 +663,34 @@ def setup_firewall( try: self.create_table_if_not_exists() if default_drop: + # fail fast, before any rule is touched self.validate_dynamic_ranges(envelope) + self.create_chain_if_not_exists(chain=self.chain, hook=HOOK, policy=POLICY_ACCEPT) + if not default_drop: + # rollback must not be blocked by any later failing step + self.ensure_default_accept() - base_chains_config = {'skale': {'hook': 'input', 'policy': 'accept'}} - - for chain, config in base_chains_config.items(): - self.create_chain_if_not_exists( - chain=chain, hook=config['hook'], policy=config['policy'] - ) - - self.add_connection_tracking_rule(self.chain) - - tcp_ports = [ - get_ssh_port(), - ServicePort.DNS, - ServicePort.HTTPS, - ServicePort.HTTP, - ServicePort.WATCHDOG_HTTP, - ServicePort.WATCHDOG_HTTPS, - ] - if enable_monitoring: - tcp_ports.extend([ServicePort.EXPORTER, ServicePort.CADVISOR]) - for port in tcp_ports: - self.add_rule(Rule(chain=self.chain, protocol='tcp', first_port=port)) - - self.remove_misordered_udp_drop() - self.add_rule(Rule(chain=self.chain, protocol='udp', first_port=ServicePort.DNS)) - self.add_loopback_rule(chain=self.chain) - - self.remove_source_quench_rule() - icmp_types = ['destination-unreachable', 'time-exceeded'] - for icmp_type in icmp_types: - self.add_rule(Rule(chain=self.chain, protocol='icmp', icmp_type=icmp_type)) - - for icmpv6_type in ICMPV6_ACCEPT_TYPES: - self.add_rule(Rule(chain=self.chain, protocol='icmpv6', icmp_type=icmpv6_type)) - - # Fine-grained filtering inside the envelope is enforced by the - # dynamic skale-admin chains that run earlier (priority 0) - self.remove_stale_envelope_rules(envelope) - self.add_rule( - Rule( - chain=self.chain, - protocol='tcp', - first_port=envelope[0], - last_port=envelope[1], - ) - ) - - self.add_drop_rule( - Rule( - chain=self.chain, - first_port=SGXPort.HTTPS, - last_port=SGXPort.ZMQ, - protocol='tcp', - ) - ) + self._setup_user_chain() + self._add_service_accepts(enable_monitoring) + self._add_icmp_accepts() + self._ensure_envelope(envelope) + self._add_drop_rules() - self.add_drop_rule(Rule(chain=self.chain, protocol='udp')) - logger.info('Making sure legacy chain has default policy %s', POLICY) + logger.info('Making sure legacy chain has default policy %s', POLICY_ACCEPT) self.update_chain_policy( - chain=LEGACY_CHAIN, policy=POLICY, family=LEGACY_FAMILY, table=LEGACY_TABLE + chain=LEGACY_CHAIN, policy=POLICY_ACCEPT, family=LEGACY_FAMILY, table=LEGACY_TABLE ) - self.apply_user_rules() if default_drop: - self.ensure_default_drop(envelope) - else: - self.ensure_default_accept() + self.ensure_default_drop() except Exception as e: logger.error('Failed to setup firewall: %s', e) raise NFTablesError(e) logger.info( 'Firewall rules are configured, default policy: %s', - POLICY_DROP if default_drop else POLICY, + POLICY_DROP if default_drop else POLICY_ACCEPT, ) def cleanup_legacy_rules(self, ssh: bool = False, dns: bool = False) -> None: @@ -926,13 +735,6 @@ def firewall_default_drop_enabled() -> bool: def get_registered_base_port() -> Optional[int]: - """Base port for the envelope, taken from the node config. - - node_base_port is the port the node was registered with (saved by - skale-admin at registration and backfilled from the contracts on admin - restarts). schain_base_port is the fallback for passive and fair nodes, - where it holds the single hosted chain's base port - a valid anchor too. - """ if not os.path.isfile(NODE_CONFIG_PATH): return None try: @@ -991,8 +793,12 @@ def enable_nftables_service() -> None: def save_nftables_base_rules(ruleset: str) -> None: ruleset_lines = ruleset.split('\n') chain_include_line = f'\tinclude "{NFTABLES_CHAIN_CONFIG_WILDCARD}"' - user_include_line = f'\t\tinclude "{NFTABLES_USER_CONFIG_PATH}"' - ruleset_lines.insert(3, user_include_line) + user_chain_lines = [ + f'\tchain {USER_CHAIN} {{', + f'\t\tinclude "{NFTABLES_USER_CONFIG_PATH}"', + '\t}', + ] + ruleset_lines[1:1] = user_chain_lines ruleset_lines.insert(-2, chain_include_line) with open(NFTABLES_SKALE_BASE_CONFIG_PATH, 'w') as f: f.write('\n'.join(ruleset_lines)) diff --git a/node_cli/core/node.py b/node_cli/core/node.py index 55952421..0497d02a 100644 --- a/node_cli/core/node.py +++ b/node_cli/core/node.py @@ -27,6 +27,7 @@ from typing import Optional, Tuple import docker +from filelock import FileLock from node_cli.cli import __version__ from node_cli.configs import ( @@ -154,8 +155,6 @@ def register_node(name, p2p_ip, public_ip, port, domain_name): logger.info('Reconfiguring firewall for the registered base port %d', port) configure_nftables(enable_monitoring=get_settings().monitoring_containers) except Exception: - # on-chain registration already succeeded - retrying register - # would fail, so the error must say the node is registered logger.exception('Post-registration firewall reconfiguration failed') error_exit( 'Node is successfully registered in SKALE manager, but firewall ' @@ -174,13 +173,14 @@ def save_registered_base_port(port: int) -> None: Kept separate from schain_base_port, which holds an already-allocated sChain port in passive mode. skale-admin saves node_base_port during - registration as well - this covers setups where the admin container - predates that behavior. + registration as well """ - node_config = read_json(NODE_CONFIG_PATH) if os.path.isfile(NODE_CONFIG_PATH) else {} - if node_config.get('node_base_port') != port: - node_config['node_base_port'] = port - save_json(NODE_CONFIG_PATH, node_config) + lock = FileLock(f'{NODE_CONFIG_PATH}.lock') + with lock: + node_config = read_json(NODE_CONFIG_PATH) if os.path.isfile(NODE_CONFIG_PATH) else {} + if node_config.get('node_base_port') != port: + node_config['node_base_port'] = port + save_json(NODE_CONFIG_PATH, node_config) @check_not_inited diff --git a/node_cli/migrations/focal_to_jammy.py b/node_cli/migrations/focal_to_jammy.py index 030312c1..7ff5c04a 100644 --- a/node_cli/migrations/focal_to_jammy.py +++ b/node_cli/migrations/focal_to_jammy.py @@ -23,7 +23,7 @@ LEGACY_CHAIN, LEGACY_FAMILY, LEGACY_TABLE, - POLICY, + POLICY_ACCEPT, NFTablesManager, remove_legacy_saved_rules, ) @@ -121,7 +121,7 @@ def remove_old_iptables_rules() -> None: def migrate() -> None: nft = NFTablesManager(family=LEGACY_FAMILY, table=LEGACY_TABLE, chain=LEGACY_CHAIN) logger.info('Making sure legacy chain has default policy accept') - nft.update_chain_policy(chain=LEGACY_CHAIN, policy=POLICY) + nft.update_chain_policy(chain=LEGACY_CHAIN, policy=POLICY_ACCEPT) logger.info('Running migration from focal to jammy') remove_old_iptables_rules() diff --git a/tests/cli/passive_node_test.py b/tests/cli/passive_node_test.py index e8419220..3375c9d6 100644 --- a/tests/cli/passive_node_test.py +++ b/tests/cli/passive_node_test.py @@ -42,6 +42,7 @@ def test_init_passive(mocked_g_config, clean_node_options, passive_user_conf): mock.patch('subprocess.run', new=subprocess_run_mock), mock.patch('node_cli.core.node.init_passive_op'), mock.patch('node_cli.core.node.is_base_containers_alive', return_value=True), + mock.patch('node_cli.core.node.enable_firewall_default_drop_when_port_available'), mock.patch('node_cli.core.resources.get_disk_size', return_value=BIG_DISK_SIZE), mock.patch('node_cli.operations.base.configure_nftables'), mock.patch('node_cli.utils.decorators.is_node_inited', return_value=False), @@ -60,6 +61,7 @@ def test_init_passive_archive(mocked_g_config, clean_node_options, passive_user_ pathlib.Path(NODE_DATA_PATH).mkdir(parents=True, exist_ok=True) with ( mock.patch('node_cli.core.node.is_base_containers_alive', return_value=True), + mock.patch('node_cli.core.node.enable_firewall_default_drop_when_port_available'), mock.patch('node_cli.operations.base.cleanup_volume_artifacts'), mock.patch('node_cli.operations.base.download_skale_node'), mock.patch('node_cli.operations.base.sync_skale_node'), diff --git a/tests/core/nftables_test.py b/tests/core/nftables_test.py index 4c3d6776..e9f12d39 100644 --- a/tests/core/nftables_test.py +++ b/tests/core/nftables_test.py @@ -291,24 +291,21 @@ def test_verify_critical_accepts(nft_manager): def test_ensure_default_drop(nft_manager): with patch.multiple( NFTablesManager, - validate_dynamic_ranges=Mock(), verify_critical_accepts=Mock(), get_chain_policy=Mock(return_value='accept'), update_chain_policy=Mock(), ): - nft_manager.ensure_default_drop((10000, 18191)) - NFTablesManager.validate_dynamic_ranges.assert_called_once_with((10000, 18191)) + nft_manager.ensure_default_drop() NFTablesManager.verify_critical_accepts.assert_called_once() NFTablesManager.update_chain_policy.assert_called_once_with(chain='skale', policy='drop') with patch.multiple( NFTablesManager, - validate_dynamic_ranges=Mock(), verify_critical_accepts=Mock(), get_chain_policy=Mock(return_value='drop'), update_chain_policy=Mock(), ): - nft_manager.ensure_default_drop((10000, 18191)) + nft_manager.ensure_default_drop() NFTablesManager.update_chain_policy.assert_not_called() @@ -462,9 +459,11 @@ def test_setup_firewall(mock_execute, nft_manager, monkeypatch, tmp_path): get_dynamic_chain_port_ranges=Mock(return_value=[]), get_chain_policy=Mock(return_value='accept'), update_chain_policy=Mock(), + apply_user_rules=Mock(), ): nft_manager.setup_firewall() assert mock_execute.called + NFTablesManager.apply_user_rules.assert_called_once() added_exprs = [ call.args[0]['nftables'][0]['add']['rule']['expr'] @@ -513,6 +512,7 @@ def test_setup_firewall_default_drop_disabled(mock_execute, nft_manager, monkeyp ensure_default_drop=Mock(), ensure_default_accept=Mock(), update_chain_policy=Mock(), + apply_user_rules=Mock(), ): nft_manager.setup_firewall() NFTablesManager.validate_dynamic_ranges.assert_not_called() @@ -534,12 +534,35 @@ def test_setup_firewall_keep_accept_policy(mock_execute, nft_manager, monkeypatc ensure_default_drop=Mock(), ensure_default_accept=Mock(), update_chain_policy=Mock(), + apply_user_rules=Mock(), ): nft_manager.setup_firewall(keep_accept_policy=True) NFTablesManager.ensure_default_drop.assert_not_called() NFTablesManager.ensure_default_accept.assert_called_once() +@patch.object(NFTablesManager, 'execute_cmd') +def test_setup_firewall_rollback_flips_accept_early( + mock_execute, nft_manager, monkeypatch, tmp_path +): + """A failing step must not block the rollback to accept.""" + monkeypatch.setenv('FIREWALL_DEFAULT_DROP', 'False') + monkeypatch.setattr(nftables_core, 'NODE_CONFIG_PATH', str(tmp_path / 'nonexistent.json')) + monkeypatch.setattr(nftables_core, 'NFTABLES_USER_CONFIG_PATH', str(tmp_path / 'user.conf')) + with patch.multiple( + NFTablesManager, + table_exists=Mock(return_value=True), + chain_exists=Mock(return_value=True), + rule_exists=Mock(return_value=True), + update_chain_policy=Mock(), + ensure_default_accept=Mock(), + apply_user_rules=Mock(side_effect=NFTablesError('bad user rule')), + ): + with pytest.raises(NFTablesError): + nft_manager.setup_firewall() + NFTablesManager.ensure_default_accept.assert_called_once() + + def test_remove_source_quench_rule(nft_manager): source_quench_rule = { 'handle': 11, @@ -575,7 +598,7 @@ def test_remove_source_quench_rule(nft_manager): delete_rule_by_handle=Mock(), ): nft_manager.remove_source_quench_rule() - NFTablesManager.delete_rule_by_handle.assert_called_once_with(11) + NFTablesManager.delete_rule_by_handle.assert_called_once_with(11, chain='skale') with patch.multiple( NFTablesManager, @@ -652,20 +675,90 @@ def test_apply_user_rules(mock_cmd, nft_manager, monkeypatch, tmp_path): monkeypatch.setattr(nftables_core, 'NFTABLES_USER_CONFIG_PATH', str(user_conf)) mock_cmd.return_value = (0, '', '') - with patch.object( - NFTablesManager, - 'get_base_ruleset', - return_value='chain skale {\n\t\ttcp dport 5000 counter accept\n}', - ): - nft_manager.apply_user_rules() + nft_manager.apply_user_rules() + assert mock_cmd.call_args[0][0] == ( + 'flush chain inet filter skale_user\n' + 'add rule inet filter skale_user tcp dport 5000 counter accept\n' + 'add rule inet filter skale_user tcp dport 6000 counter accept' + ) - applied = [call.args[0] for call in mock_cmd.call_args_list] - assert applied == ['insert rule inet filter skale tcp dport 6000 counter accept'] + # missing file still flushes, so removed rules disappear + monkeypatch.setattr(nftables_core, 'NFTABLES_USER_CONFIG_PATH', str(tmp_path / 'absent')) + nft_manager.apply_user_rules() + assert mock_cmd.call_args[0][0] == 'flush chain inet filter skale_user' mock_cmd.return_value = (1, '', 'syntax error') - with patch.object(NFTablesManager, 'get_base_ruleset', return_value=''): - with pytest.raises(NFTablesError): - nft_manager.apply_user_rules() + with pytest.raises(NFTablesError): + nft_manager.apply_user_rules() + + +@patch.object(NFTablesManager, 'execute_cmd') +def test_ensure_user_chain_jump(mock_execute, nft_manager): + with patch.object(NFTablesManager, 'rule_exists', return_value=False): + nft_manager.ensure_user_chain_jump() + cmd = mock_execute.call_args[0][0]['nftables'][0] + assert cmd['insert']['rule']['expr'] == [{'jump': {'target': 'skale_user'}}] + + mock_execute.reset_mock() + with patch.object(NFTablesManager, 'rule_exists', return_value=True): + nft_manager.ensure_user_chain_jump() + mock_execute.assert_not_called() + + +@patch.object(NFTablesManager, 'execute_cmd') +def test_create_user_chain_if_not_exists(mock_execute, nft_manager): + with patch.object(NFTablesManager, 'chain_exists', return_value=False): + nft_manager.create_user_chain_if_not_exists() + chain = mock_execute.call_args[0][0]['nftables'][0]['add']['chain'] + assert chain == {'family': 'inet', 'table': 'filter', 'name': 'skale_user'} + # regular chain: no hook, priority or policy + + mock_execute.reset_mock() + with patch.object(NFTablesManager, 'chain_exists', return_value=True): + nft_manager.create_user_chain_if_not_exists() + mock_execute.assert_not_called() + + +@patch('nftables.Nftables.cmd') +def test_remove_user_rules_from_main_chain(mock_cmd, nft_manager, monkeypatch, tmp_path): + user_conf = tmp_path / 'user.conf' + user_conf.write_text('tcp dport 5000 counter accept\n') + monkeypatch.setattr(nftables_core, 'NFTABLES_USER_CONFIG_PATH', str(user_conf)) + + listing = ( + 'chain skale { # handle 1\n' + '\ttcp dport 5000 counter accept # handle 7\n' + '\ttcp dport 22 counter accept # handle 8\n' + '}\n' + ) + mock_cmd.return_value = (0, listing, '') + with patch.object(NFTablesManager, 'delete_rule_by_handle') as mock_delete: + nft_manager.remove_user_rules_from_main_chain() + mock_delete.assert_called_once_with(7) + + +def test_save_nftables_base_rules(monkeypatch, tmp_path): + base_conf = tmp_path / 'base.conf' + monkeypatch.setattr(nftables_core, 'NFTABLES_SKALE_BASE_CONFIG_PATH', str(base_conf)) + ruleset = ( + 'table inet firewall {\n' + '\tchain skale {\n' + '\t\ttype filter hook input priority filter + 1; policy drop;\n' + '\t\tjump skale_user\n' + '\t\tct state established,related counter accept\n' + '\t}\n' + '}' + ) + nftables_core.save_nftables_base_rules(ruleset) + saved = base_conf.read_text() + + # user chain is declared before the skale chain that jumps to it + assert saved.index('chain skale_user {') < saved.index('chain skale {') + assert nftables_core.NFTABLES_USER_CONFIG_PATH in saved + assert nftables_core.NFTABLES_CHAIN_CONFIG_WILDCARD in saved + # the include lives only inside the user chain, not in the skale chain + skale_chain_part = saved[saved.index('chain skale {') :] + assert 'include "' + nftables_core.NFTABLES_USER_CONFIG_PATH not in skale_chain_part def test_invalid_protocol(nft_manager): From 8c083b98c660ab87c30aca3d72618757b83bc39b Mon Sep 17 00:00:00 2001 From: badrogger Date: Thu, 10 Sep 2026 20:10:48 +0100 Subject: [PATCH 08/16] Fix for ssh port --- README.md | 17 ++++ node_cli/core/nftables.py | 140 +++++++++++++++------------ node_cli/utils/helper.py | 46 ++++++++- tests/core/iptables_test.py | 72 +++++++++++++- tests/core/nftables_envelope_test.py | 35 +++++++ tests/core/nftables_test.py | 139 +++++++++++++++++++++----- tests/core/ssh_firewall_test.py | 53 ++++++++++ 7 files changed, 412 insertions(+), 90 deletions(-) create mode 100644 tests/core/nftables_envelope_test.py create mode 100644 tests/core/ssh_firewall_test.py diff --git a/README.md b/README.md index 5a2ef37f..e2b133b2 100644 --- a/README.md +++ b/README.md @@ -119,6 +119,23 @@ Options: > Prefix: `skale node` +#### Configure firewall + +SSH allow rules use all listening ports reported by `sshd -T`, including ports +configured through included files and `ListenAddress`. If detection fails, the +command stops before enabling the default-drop policy. + +For a port configured through sshd command-line options or socket activation, +set the `SSH_PORT` environment variable explicitly when running commands that +configure the firewall (including node init and update): + +```shell +sudo SSH_PORT=2222 skale node configure-firewall +``` + +The override replaces automatic detection and accepts one port from 1 to 65535. +It must be passed in the command environment, not only in the node settings file. + #### Node information Get base info about the standard SKALE node. diff --git a/node_cli/core/nftables.py b/node_cli/core/nftables.py index de1e8ef7..670a80ac 100644 --- a/node_cli/core/nftables.py +++ b/node_cli/core/nftables.py @@ -36,7 +36,7 @@ NFTABLES_USER_CONFIG_PATH, NODE_CONFIG_PATH, ) -from node_cli.utils.helper import get_ssh_port, read_json, run_cmd +from node_cli.utils.helper import get_ssh_ports, read_json, run_cmd logger = logging.getLogger(__name__) @@ -86,7 +86,8 @@ class SGXPort: # sChain base ports are allocated as node_base_port + schain_index * 64 # (PORTS_PER_SCHAIN in skale.py); 128 slots cover every possible allocation -SCHAIN_PORTS_PER_NODE = 128 * 64 +PORTS_PER_SCHAIN = 64 +SCHAIN_PORTS_PER_NODE = 128 * PORTS_PER_SCHAIN SCHAIN_BASE_PORT_ENV = 'SCHAIN_BASE_PORT' FIREWALL_DEFAULT_DROP_ENV = 'FIREWALL_DEFAULT_DROP' MIN_SCHAIN_BASE_PORT = 2000 @@ -453,9 +454,12 @@ def validate_dynamic_ranges(self, envelope: tuple[int, int]) -> None: def verify_critical_accepts(self) -> None: """Ensure lockout-critical accept rules are in place before setting drop policy.""" - ssh_port = get_ssh_port() - ssh_expr = Rule(chain=self.chain, protocol='tcp', first_port=ssh_port).to_expr() - for name, expr in (('conntrack', conntrack_accept_expr()), ('ssh', ssh_expr)): + accepts = [('conntrack', conntrack_accept_expr())] + accepts.extend( + (f'ssh port {port}', Rule(chain=self.chain, protocol='tcp', first_port=port).to_expr()) + for port in get_ssh_ports() + ) + for name, expr in accepts: if not self.rule_exists(self.chain, expr): raise NFTablesError( f'Refusing to set drop policy: {name} accept rule is missing ' @@ -476,7 +480,12 @@ def delete_rule_by_handle(self, handle: int, chain: Optional[str] = None) -> Non self._execute_rule_with_op('delete', chain or self.chain, handle=handle) def remove_stale_envelope_rules(self, envelope: tuple[int, int]) -> None: - """Remove sChain envelope accepts anchored at a different base port.""" + """Remove sChain envelope accepts anchored at a different base port. + + Both envelope shapes are recognized: the full node allocation and + the single-chain range used on passive and fair nodes. + """ + envelope_spans = (SCHAIN_PORTS_PER_NODE - 1, PORTS_PER_SCHAIN - 1) for rule in self.get_rules(self.chain): expr = rule.get('expr', []) if {'accept': None} not in expr: @@ -488,17 +497,13 @@ def remove_stale_envelope_rules(self, envelope: tuple[int, int]) -> None: match.get('left', {}).get('payload', {}).get('field') == 'dport' and isinstance(right, dict) and 'range' in right - and right['range'][1] - right['range'][0] == SCHAIN_PORTS_PER_NODE - 1 + and right['range'][1] - right['range'][0] in envelope_spans and tuple(right['range']) != envelope and rule.get('handle') is not None ): logger.info('Removing stale envelope rule %s', right['range']) self.delete_rule_by_handle(rule['handle']) - @staticmethod - def _normalized_expr(expr: list[dict]) -> list[dict]: - return [{'counter': None} if 'counter' in statement else statement for statement in expr] - def remove_misordered_udp_drop(self) -> None: """Delete the blanket udp drop when it shadows the udp DNS accept. @@ -551,51 +556,50 @@ def ensure_user_chain_jump(self) -> None: expr = [{'jump': {'target': USER_CHAIN}}] self._ensure_rule(self.chain, expr, op='insert', label='user chain jump') - @staticmethod - def read_user_rule_lines() -> list[str]: - if not os.path.isfile(NFTABLES_USER_CONFIG_PATH): - return [] - with open(NFTABLES_USER_CONFIG_PATH) as user_config: - lines = [line.strip() for line in user_config.readlines()] - return [line for line in lines if line and not line.startswith('#')] - def apply_user_rules(self) -> None: """Reload user.conf into the live user rules chain. - Flush plus re-add in one transaction keeps the chain exactly in sync - with the file + + The file content is fed through the native nft parser inside the + chain declaration - exactly how the boot include reads it - so + comments and multiline rules behave identically in both paths. + Flush plus reload in one transaction keeps the chain in sync with + the file. """ - commands = [f'flush chain {self.family} {self.table} {USER_CHAIN}'] - commands.extend( - f'add rule {self.family} {self.table} {USER_CHAIN} {line}' - for line in self.read_user_rule_lines() + content = '' + if os.path.isfile(NFTABLES_USER_CONFIG_PATH): + with open(NFTABLES_USER_CONFIG_PATH) as user_config: + content = user_config.read() + commands = ( + f'flush chain {self.family} {self.table} {USER_CHAIN}\n' + f'table {self.family} {self.table} {{\n' + f'chain {USER_CHAIN} {{\n' + f'{content}\n' + f'}}\n' + f'}}' ) - rc, output, error = self.nft.cmd('\n'.join(commands)) + rc, output, error = self.nft.cmd(commands) if rc != 0: raise NFTablesError(f'Failed to apply user.conf rules: {error}') def remove_user_rules_from_main_chain(self) -> None: """Remove user.conf rules that older saved configs loaded into the - skale chain directly, so they are not snapshotted as duplicates.""" - lines = self.read_user_rule_lines() - if not lines: - return - self.nft.set_json_output(False) - self.nft.set_handle_output(True) - try: - rc, output, error = self.nft.cmd(f'list chain {self.family} {self.table} {self.chain}') - finally: - self.nft.set_handle_output(False) - self.nft.set_json_output(True) - if rc != 0: + skale chain directly, so they are not snapshotted as duplicates. + + The freshly reloaded user chain is the parsed form of user.conf, so + rules are matched by expression - immune to comments, multiline + formatting and rendering differences. Runs before the service rules + are re-added, so removing an expression they share cannot last. + """ + user_exprs = [ + self._normalized_expr(rule.get('expr', [])) for rule in self.get_rules(USER_CHAIN) + ] + if not user_exprs: return - for listed in output.split('\n'): - listed = listed.strip() - if ' # handle ' not in listed: - continue - rule_text, _, handle = listed.rpartition(' # handle ') - if rule_text in lines and handle.isdigit(): - logger.info('Moving user rule out of the main chain: %s', rule_text) - self.delete_rule_by_handle(int(handle)) + for rule in self.get_rules(self.chain): + expr = self._normalized_expr(rule.get('expr', [])) + if expr in user_exprs and rule.get('handle') is not None: + logger.info('Moving user rule out of the main chain') + self.delete_rule_by_handle(rule['handle']) def get_base_ruleset(self) -> str: self.nft.set_json_output(False) @@ -611,12 +615,13 @@ def get_base_ruleset(self) -> str: def _setup_user_chain(self) -> None: self.create_user_chain_if_not_exists() self.ensure_user_chain_jump() + self.apply_user_rules() self.remove_user_rules_from_main_chain() def _add_service_accepts(self, enable_monitoring: bool) -> None: self._ensure_rule(self.chain, conntrack_accept_expr(), label='connection tracking rule') tcp_ports = [ - get_ssh_port(), + *get_ssh_ports(), ServicePort.DNS, ServicePort.HTTPS, ServicePort.HTTP, @@ -658,18 +663,20 @@ def setup_firewall( """Setup firewall rules.""" logger.info('Configuring firewall rules') - envelope = get_schain_ports_envelope() default_drop = firewall_default_drop_enabled() and not keep_accept_policy try: self.create_table_if_not_exists() - if default_drop: - # fail fast, before any rule is touched - self.validate_dynamic_ranges(envelope) self.create_chain_if_not_exists(chain=self.chain, hook=HOOK, policy=POLICY_ACCEPT) if not default_drop: - # rollback must not be blocked by any later failing step + # rollback must not be blocked by any later failing step, + # including an invalid envelope configuration self.ensure_default_accept() + envelope = get_schain_ports_envelope() + if default_drop: + # fail fast, before any rule is touched + self.validate_dynamic_ranges(envelope) + self._setup_user_chain() self._add_service_accepts(enable_monitoring) self._add_icmp_accepts() @@ -680,7 +687,6 @@ def setup_firewall( self.update_chain_policy( chain=LEGACY_CHAIN, policy=POLICY_ACCEPT, family=LEGACY_FAMILY, table=LEGACY_TABLE ) - self.apply_user_rules() if default_drop: self.ensure_default_drop() @@ -706,7 +712,7 @@ def cleanup_legacy_rules(self, ssh: bool = False, dns: bool = False) -> None: ServicePort.DNS, # tcp is redundant, making sure it's removed ] if ssh: - tcp_ports.append(get_ssh_port()) + tcp_ports.extend(get_ssh_ports()) for port in tcp_ports: self.remove_rule(Rule(chain=self.chain, protocol='tcp', first_port=port)) if dns: @@ -734,7 +740,13 @@ def firewall_default_drop_enabled() -> bool: return value.lower() not in ('false', '0', 'no', 'off') -def get_registered_base_port() -> Optional[int]: +def get_registered_base_port() -> Optional[tuple[int, int]]: + """Base port and envelope size from the node config. + + node_base_port is a node registration port and reserves the full node + allocation; schain_base_port (passive and fair nodes) is one already + allocated chain's base port and reserves that single chain's range. + """ if not os.path.isfile(NODE_CONFIG_PATH): return None try: @@ -745,10 +757,13 @@ def get_registered_base_port() -> Optional[int]: if not isinstance(node_config, dict): logger.warning('Node config is malformed') return None - for key in ('node_base_port', 'schain_base_port'): + for key, size in ( + ('node_base_port', SCHAIN_PORTS_PER_NODE), + ('schain_base_port', PORTS_PER_SCHAIN), + ): value = node_config.get(key) if isinstance(value, int) and not isinstance(value, bool) and value > 0: - return value + return value, size return None @@ -757,14 +772,17 @@ def get_schain_ports_envelope() -> tuple[int, int]: env_value = os.getenv(SCHAIN_BASE_PORT_ENV) if env_value: try: - base_port = int(env_value) + base_port, size = int(env_value), SCHAIN_PORTS_PER_NODE except ValueError: raise NFTablesError(f'{SCHAIN_BASE_PORT_ENV} must be an integer, got {env_value}') else: - base_port = get_registered_base_port() or DEFAULT_NODE_BASE_PORT - if not MIN_SCHAIN_BASE_PORT <= base_port <= MAX_PORT - SCHAIN_PORTS_PER_NODE + 1: + base_port, size = get_registered_base_port() or ( + DEFAULT_NODE_BASE_PORT, + SCHAIN_PORTS_PER_NODE, + ) + if base_port < MIN_SCHAIN_BASE_PORT or base_port + size - 1 > MAX_PORT: raise NFTablesError(f'Invalid sChain base port {base_port}') - return base_port, base_port + SCHAIN_PORTS_PER_NODE - 1 + return base_port, base_port + size - 1 def prepare_directories() -> None: diff --git a/node_cli/utils/helper.py b/node_cli/utils/helper.py index 8a642aee..8f3b8ed9 100644 --- a/node_cli/utils/helper.py +++ b/node_cli/utils/helper.py @@ -408,7 +408,51 @@ def get_tmp_path(path: str | Path) -> str: return base + salt + '.tmp' + ext -def get_ssh_port(ssh_service_name='ssh'): +def get_ssh_ports() -> list[int]: + """Return SSH_PORT or the ports from the effective default sshd config.""" + override = os.getenv('SSH_PORT') + if override is not None: + values = [override] + else: + try: + result = subprocess.run( + [shutil.which('sshd') or '/usr/sbin/sshd', '-T'], + capture_output=True, + text=True, + check=True, + timeout=10, + ) + except (OSError, subprocess.SubprocessError) as err: + raise RuntimeError( + 'Cannot determine SSH ports from sshd -T. Set SSH_PORT to the ' + 'SSH listening port before configuring the firewall.' + ) from err + ports, listen_ports = [], [] + for line in result.stdout.splitlines(): + fields = line.split() + if len(fields) < 2: + continue + if fields[0] == 'port': + ports.append(fields[1]) + elif fields[0] == 'listenaddress': + # sshd -T expands addresses to IPv4:port or [IPv6]:port. + listen_ports.append(fields[1].rsplit(':', 1)[-1]) + values = listen_ports or ports + try: + parsed_ports = sorted({int(value) for value in values}) + if not parsed_ports or any(not 1 <= port <= 65535 for port in parsed_ports): + raise ValueError('Missing or out-of-range port') + except ValueError as err: + raise ValueError( + 'Cannot determine valid SSH ports. Set SSH_PORT to an integer from 1 to 65535.' + ) from err + return parsed_ports + + +def get_ssh_port(ssh_service_name='ssh') -> int: + """Return the first SSH port; firewall callers must use get_ssh_ports().""" + if ssh_service_name == 'ssh': + return get_ssh_ports()[0] try: return socket.getservbyname(ssh_service_name) except OSError: diff --git a/tests/core/iptables_test.py b/tests/core/iptables_test.py index 2926b917..07144eee 100644 --- a/tests/core/iptables_test.py +++ b/tests/core/iptables_test.py @@ -1,12 +1,76 @@ import socket +import subprocess import mock +import pytest -from node_cli.utils.helper import get_ssh_port +from node_cli.utils.helper import get_ssh_port, get_ssh_ports -def test_get_ssh_port(): - assert get_ssh_port() == 22 +@pytest.fixture(autouse=True) +def clear_ssh_override(monkeypatch): + monkeypatch.delenv('SSH_PORT', raising=False) + + +def test_service_lookup_compatibility(): assert get_ssh_port('http') == 80 with mock.patch.object(socket, 'getservbyname', side_effect=OSError): - assert get_ssh_port() == 22 + assert get_ssh_port('missing-service') == 22 + + +@pytest.mark.parametrize( + 'config,expected', + [ + ('port 22\nlistenaddress 0.0.0.0:22\nlistenaddress [::]:22\n', [22]), + ('port 2222\n', [2222]), + ('port 2222\nport 2200\nport 2222\n', [2200, 2222]), + ('port 22\nlistenaddress 192.0.2.1:2222\nlistenaddress [::1]:2200\n', [2200, 2222]), + ], +) +def test_effective_sshd_ports(config, expected): + with ( + mock.patch('node_cli.utils.helper.shutil.which', return_value='/usr/sbin/sshd'), + mock.patch('node_cli.utils.helper.subprocess.run') as run, + ): + run.return_value.stdout = config + assert get_ssh_ports() == expected + run.assert_called_once_with( + ['/usr/sbin/sshd', '-T'], capture_output=True, text=True, check=True, timeout=10 + ) + + +def test_ssh_port_override(monkeypatch): + monkeypatch.setenv('SSH_PORT', '2222') + with mock.patch('node_cli.utils.helper.subprocess.run') as run: + assert get_ssh_ports() == [2222] + assert get_ssh_port() == 2222 + run.assert_not_called() + + +@pytest.mark.parametrize('value', ['', 'ssh', '0', '-1', '65536']) +def test_invalid_ssh_port_override(monkeypatch, value): + monkeypatch.setenv('SSH_PORT', value) + with pytest.raises(ValueError, match='SSH_PORT'): + get_ssh_ports() + + +@pytest.mark.parametrize( + 'error', + [ + FileNotFoundError(), + subprocess.CalledProcessError(1, 'sshd'), + subprocess.TimeoutExpired('sshd', 10), + ], +) +def test_ssh_detection_failure_does_not_guess(error): + with mock.patch('node_cli.utils.helper.subprocess.run', side_effect=error): + with pytest.raises(RuntimeError, match='SSH_PORT'): + get_ssh_port() + + +@pytest.mark.parametrize('config', ['', 'port invalid\n', 'port 65536\n']) +def test_invalid_sshd_output(config): + with mock.patch('node_cli.utils.helper.subprocess.run') as run: + run.return_value.stdout = config + with pytest.raises(ValueError, match='SSH_PORT'): + get_ssh_ports() diff --git a/tests/core/nftables_envelope_test.py b/tests/core/nftables_envelope_test.py new file mode 100644 index 00000000..1eccc602 --- /dev/null +++ b/tests/core/nftables_envelope_test.py @@ -0,0 +1,35 @@ +from unittest.mock import patch + +import pytest + +from node_cli.core.nftables import NFTablesManager, Rule + + +@pytest.mark.parametrize( + 'old_range,new_range', + [ + ((20128, 20191), (30000, 30063)), + ((10000, 18191), (30000, 38191)), + ((10000, 18191), (10000, 10063)), + ((10000, 10063), (10000, 18191)), + ], +) +def test_replace_envelope_preserves_current_range_and_unrelated_rules(old_range, new_range): + manager = NFTablesManager.__new__(NFTablesManager) + manager.chain = 'skale' + current = Rule('skale', 'tcp', *new_range) + rules = [ + {'handle': 1, 'expr': Rule('skale', 'tcp', *old_range).to_expr()}, + {'handle': 2, 'expr': current.to_expr()}, + {'handle': 3, 'expr': Rule('skale', 'tcp', 1026, 1031, action='drop').to_expr()}, + {'handle': 4, 'expr': Rule('skale', 'tcp', 5000, 5010).to_expr()}, + {'handle': 5, 'expr': Rule('skale', 'tcp', *old_range, action='drop').to_expr()}, + ] + with ( + patch.object(manager, 'get_rules', return_value=rules), + patch.object(manager, 'delete_rule_by_handle') as delete, + patch.object(manager, 'add_rule') as add, + ): + manager._ensure_envelope(new_range) + delete.assert_called_once_with(1) + add.assert_called_once_with(current) diff --git a/tests/core/nftables_test.py b/tests/core/nftables_test.py index e9f12d39..2ea65c5d 100644 --- a/tests/core/nftables_test.py +++ b/tests/core/nftables_test.py @@ -9,6 +9,7 @@ NFTablesError, NFTablesManager, Rule, + dport_match, get_schain_ports_envelope, ) @@ -23,6 +24,11 @@ def nft_manager(): manager.flush() +@pytest.fixture(autouse=True) +def ssh_ports(monkeypatch): + monkeypatch.setattr(nftables_core, 'get_ssh_ports', lambda: [22]) + + @pytest.fixture def mock_nft_output(): """Fixture for mocking nftables output.""" @@ -387,6 +393,36 @@ def test_remove_stale_envelope_rules(nft_manager): nft_manager.remove_stale_envelope_rules((30000, 38191)) NFTablesManager.delete_rule_by_handle.assert_called_once_with(7) + def envelope_rule(handle, first_port, last_port): + return { + 'handle': handle, + 'expr': [ + dport_match('tcp', first_port, last_port), + {'counter': None}, + {'accept': None}, + ], + } + + # passive node: the single-chain envelope moved to a new base port + rules = [envelope_rule(11, 58128, 58191), envelope_rule(12, 58192, 58255)] + with patch.multiple( + NFTablesManager, + get_rules=Mock(return_value=rules), + delete_rule_by_handle=Mock(), + ): + nft_manager.remove_stale_envelope_rules((58192, 58255)) + NFTablesManager.delete_rule_by_handle.assert_called_once_with(11) + + # upgrade transition: full-node envelope replaced by a single-chain one + rules = [envelope_rule(13, 10000, 18191), envelope_rule(14, 58128, 58191)] + with patch.multiple( + NFTablesManager, + get_rules=Mock(return_value=rules), + delete_rule_by_handle=Mock(), + ): + nft_manager.remove_stale_envelope_rules((58128, 58191)) + NFTablesManager.delete_rule_by_handle.assert_called_once_with(13) + def test_get_schain_ports_envelope_default(monkeypatch, tmp_path): monkeypatch.setattr(nftables_core, 'NODE_CONFIG_PATH', str(tmp_path / 'nonexistent.json')) @@ -425,24 +461,39 @@ def test_get_schain_ports_envelope_malformed_node_config(monkeypatch, tmp_path): # falls back to the default base port assert get_schain_ports_envelope() == (10000, 18191) - # an invalid primary value must not mask a valid fallback + # an invalid primary value must not mask a valid fallback, which then + # carries its own single-chain envelope size config_path.write_text(json.dumps({'node_base_port': 'bad', 'schain_base_port': 20128})) - assert get_schain_ports_envelope() == (20128, 28319) + assert get_schain_ports_envelope() == (20128, 20191) def test_get_schain_ports_envelope_from_node_config(monkeypatch, tmp_path): config_path = tmp_path / 'node_config.json' monkeypatch.setattr(nftables_core, 'NODE_CONFIG_PATH', str(config_path)) - # active node: node_base_port saved at registration wins + # active node: node_base_port saved at registration wins, full allocation config_path.write_text( json.dumps({'node_id': 1, 'node_base_port': 20128, 'schain_base_port': 30000}) ) assert get_schain_ports_envelope() == (20128, 28319) - # passive/fair node: only schain_base_port is present + # passive/fair node: schain_base_port is one allocated chain's base port + # and reserves only that chain's range config_path.write_text(json.dumps({'node_id': 1, 'schain_base_port': 20128})) - assert get_schain_ports_envelope() == (20128, 28319) + assert get_schain_ports_envelope() == (20128, 20191) + + # a chain at the top of a high node allocation is valid on passive nodes + config_path.write_text(json.dumps({'node_id': 1, 'schain_base_port': 58128})) + assert get_schain_ports_envelope() == (58128, 58191) + + # while a full node allocation must fit below the port maximum + config_path.write_text(json.dumps({'node_id': 1, 'node_base_port': 58128})) + with pytest.raises(NFTablesError): + get_schain_ports_envelope() + + config_path.write_text(json.dumps({'node_id': 1, 'schain_base_port': 65500})) + with pytest.raises(NFTablesError): + get_schain_ports_envelope() @patch.object(NFTablesManager, 'execute_cmd') @@ -563,6 +614,25 @@ def test_setup_firewall_rollback_flips_accept_early( NFTablesManager.ensure_default_accept.assert_called_once() +@patch.object(NFTablesManager, 'execute_cmd') +def test_setup_firewall_rollback_survives_bad_envelope( + mock_execute, nft_manager, monkeypatch, tmp_path +): + """An invalid base port configuration must not block the rollback.""" + monkeypatch.setenv('FIREWALL_DEFAULT_DROP', 'False') + monkeypatch.setenv('SCHAIN_BASE_PORT', 'not-a-port') + monkeypatch.setattr(nftables_core, 'NFTABLES_USER_CONFIG_PATH', str(tmp_path / 'user.conf')) + with patch.multiple( + NFTablesManager, + table_exists=Mock(return_value=True), + chain_exists=Mock(return_value=True), + ensure_default_accept=Mock(), + ): + with pytest.raises(NFTablesError): + nft_manager.setup_firewall() + NFTablesManager.ensure_default_accept.assert_called_once() + + def test_remove_source_quench_rule(nft_manager): source_quench_rule = { 'handle': 11, @@ -669,23 +739,34 @@ def test_remove_misordered_udp_drop(nft_manager): @patch('nftables.Nftables.cmd') def test_apply_user_rules(mock_cmd, nft_manager, monkeypatch, tmp_path): user_conf = tmp_path / 'user.conf' - user_conf.write_text( - '# custom services\ntcp dport 5000 counter accept\n\ntcp dport 6000 counter accept\n' + # inline comments and multiline rules must reach the nft parser verbatim, + # exactly as the boot include would read them + content = ( + '# custom services\n' + 'tcp dport 5000 counter accept # legacy exporter\n' + 'tcp dport {\n' + ' 6000,\n' + ' 6001,\n' + '} counter accept\n' ) + user_conf.write_text(content) monkeypatch.setattr(nftables_core, 'NFTABLES_USER_CONFIG_PATH', str(user_conf)) mock_cmd.return_value = (0, '', '') nft_manager.apply_user_rules() assert mock_cmd.call_args[0][0] == ( 'flush chain inet filter skale_user\n' - 'add rule inet filter skale_user tcp dport 5000 counter accept\n' - 'add rule inet filter skale_user tcp dport 6000 counter accept' + 'table inet filter {\n' + 'chain skale_user {\n' + f'{content}\n' + '}\n' + '}' ) # missing file still flushes, so removed rules disappear monkeypatch.setattr(nftables_core, 'NFTABLES_USER_CONFIG_PATH', str(tmp_path / 'absent')) nft_manager.apply_user_rules() - assert mock_cmd.call_args[0][0] == 'flush chain inet filter skale_user' + assert mock_cmd.call_args[0][0].startswith('flush chain inet filter skale_user') mock_cmd.return_value = (1, '', 'syntax error') with pytest.raises(NFTablesError): @@ -719,23 +800,33 @@ def test_create_user_chain_if_not_exists(mock_execute, nft_manager): mock_execute.assert_not_called() -@patch('nftables.Nftables.cmd') -def test_remove_user_rules_from_main_chain(mock_cmd, nft_manager, monkeypatch, tmp_path): - user_conf = tmp_path / 'user.conf' - user_conf.write_text('tcp dport 5000 counter accept\n') - monkeypatch.setattr(nftables_core, 'NFTABLES_USER_CONFIG_PATH', str(user_conf)) - - listing = ( - 'chain skale { # handle 1\n' - '\ttcp dport 5000 counter accept # handle 7\n' - '\ttcp dport 22 counter accept # handle 8\n' - '}\n' - ) - mock_cmd.return_value = (0, listing, '') - with patch.object(NFTablesManager, 'delete_rule_by_handle') as mock_delete: +def test_remove_user_rules_from_main_chain(nft_manager): + user_rule_expr = Rule(chain='skale_user', protocol='tcp', first_port=5000).to_expr() + chains = { + # the reloaded user chain is the parsed form of user.conf; native + # comments live outside expr, so matching ignores them + 'skale_user': [{'handle': 3, 'expr': user_rule_expr, 'comment': 'service #1'}], + 'skale': [ + {'handle': 7, 'expr': user_rule_expr}, + {'handle': 8, 'expr': Rule(chain='skale', protocol='tcp', first_port=22).to_expr()}, + ], + } + with ( + patch.object(NFTablesManager, 'get_rules', side_effect=lambda chain: chains[chain]), + patch.object(NFTablesManager, 'delete_rule_by_handle') as mock_delete, + ): nft_manager.remove_user_rules_from_main_chain() mock_delete.assert_called_once_with(7) + # nothing loaded from user.conf - main chain untouched + chains['skale_user'] = [] + with ( + patch.object(NFTablesManager, 'get_rules', side_effect=lambda chain: chains[chain]), + patch.object(NFTablesManager, 'delete_rule_by_handle') as mock_delete, + ): + nft_manager.remove_user_rules_from_main_chain() + mock_delete.assert_not_called() + def test_save_nftables_base_rules(monkeypatch, tmp_path): base_conf = tmp_path / 'base.conf' diff --git a/tests/core/ssh_firewall_test.py b/tests/core/ssh_firewall_test.py new file mode 100644 index 00000000..98496de8 --- /dev/null +++ b/tests/core/ssh_firewall_test.py @@ -0,0 +1,53 @@ +from unittest.mock import Mock, patch + +import pytest + +from node_cli.core.nftables import NFTablesError, NFTablesManager, Rule, conntrack_accept_expr + + +@pytest.fixture +def manager(): + # These checks exercise rule generation without accessing the host firewall. + manager = NFTablesManager.__new__(NFTablesManager) + manager.chain = 'skale' + return manager + + +def test_allow_all_ssh_ports(manager): + rules = [] + with ( + patch('node_cli.core.nftables.get_ssh_ports', return_value=[2200, 2222]), + patch.object(manager, '_ensure_rule'), + patch.object(manager, 'remove_misordered_udp_drop'), + patch.object(manager, 'add_rule', side_effect=rules.append), + ): + manager._add_service_accepts(enable_monitoring=False) + allowed_tcp_ports = {rule.first_port for rule in rules if rule.protocol == 'tcp'} + assert {2200, 2222} <= allowed_tcp_ports + assert 22 not in allowed_tcp_ports + + +def test_refuse_drop_until_all_ssh_ports_are_allowed(manager): + installed = [conntrack_accept_expr(), Rule('skale', 'tcp', 2200).to_expr()] + with ( + patch('node_cli.core.nftables.get_ssh_ports', return_value=[2200, 2222]), + patch.object(manager, 'get_rules', side_effect=lambda _: [{'expr': e} for e in installed]), + patch.object(manager, 'get_chain_policy', return_value='accept'), + patch.object(manager, 'update_chain_policy') as update_policy, + ): + with pytest.raises(NFTablesError, match='ssh port 2222'): + manager.ensure_default_drop() + update_policy.assert_not_called() + installed.append(Rule('skale', 'tcp', 2222).to_expr()) + manager.ensure_default_drop() + update_policy.assert_called_once_with(chain='skale', policy='drop') + + +def test_detection_failure_prevents_drop(manager): + manager.update_chain_policy = Mock() + with patch( + 'node_cli.core.nftables.get_ssh_ports', side_effect=RuntimeError('SSH_PORT required') + ): + with pytest.raises(RuntimeError, match='SSH_PORT'): + manager.ensure_default_drop() + manager.update_chain_policy.assert_not_called() From 1eebf76956bab1643e31c18599303a27c56b49b7 Mon Sep 17 00:00:00 2001 From: badrogger Date: Fri, 11 Sep 2026 11:04:15 +0100 Subject: [PATCH 09/16] Small improvements --- node_cli/core/nftables.py | 21 ++++++++++----------- tests/core/nftables_test.py | 10 ++++++++++ 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/node_cli/core/nftables.py b/node_cli/core/nftables.py index 670a80ac..8ac939f6 100644 --- a/node_cli/core/nftables.py +++ b/node_cli/core/nftables.py @@ -411,22 +411,21 @@ def remove_rule(self, rule: Rule) -> None: def get_dynamic_chain_port_ranges(self) -> list[tuple[str, int, int]]: """Min/max tcp dport covered by each dynamic skale-admin chain.""" + rc, output, error = self.nft.cmd(f'list table {self.family} {self.table}') + if rc != 0: + if error and 'No such file or directory' in error: + return [] + raise NFTablesError(f'Failed to list table {self.table}: {error}') try: - rc, output, error = self.nft.cmd(f'list table {self.family} {self.table}') - if rc != 0: - if error and 'No such file or directory' in error: - return [] - raise NFTablesError(f'Failed to list table {self.table}: {error}') data = json.loads(output) - except NFTablesError: - raise - except Exception as e: - logger.error('Failed to get dynamic chain ranges: %s', e) - raise NFTablesError(e) + except (TypeError, ValueError) as err: + raise NFTablesError(f'Failed to parse table {self.table} listing: {err}') from err + if not isinstance(data, dict): + raise NFTablesError(f'Malformed table {self.table} listing') ports: dict[str, list[int]] = {} for item in data.get('nftables', []): - rule = item.get('rule') + rule = item.get('rule') if isinstance(item, dict) else None if not rule or not rule.get('chain', '').startswith(DYNAMIC_CHAIN_PREFIX): continue for statement in rule.get('expr', []): diff --git a/tests/core/nftables_test.py b/tests/core/nftables_test.py index 2ea65c5d..49d88621 100644 --- a/tests/core/nftables_test.py +++ b/tests/core/nftables_test.py @@ -273,6 +273,16 @@ def test_get_dynamic_chain_port_ranges(mock_cmd, nft_manager): mock_cmd.return_value = (1, '', 'No such file or directory') assert nft_manager.get_dynamic_chain_port_ranges() == [] + mock_cmd.return_value = (1, '', 'some other error') + with pytest.raises(NFTablesError): + nft_manager.get_dynamic_chain_port_ranges() + + # unparseable or wrong-shaped output keeps the typed contract + for output in ('not-json', 'null', '[]'): + mock_cmd.return_value = (0, output, '') + with pytest.raises(NFTablesError): + nft_manager.get_dynamic_chain_port_ranges() + def test_validate_dynamic_ranges(nft_manager): """Test envelope validation against dynamic chain ranges.""" From c969d32cd9b534d91c7ea3e2a6389a49bdd9d8ed Mon Sep 17 00:00:00 2001 From: badrogger Date: Fri, 11 Sep 2026 11:25:15 +0100 Subject: [PATCH 10/16] Improve exception handling --- node_cli/core/nftables.py | 35 +++-------------- node_cli/utils/helper.py | 79 +++++++++++++++++++++------------------ 2 files changed, 48 insertions(+), 66 deletions(-) diff --git a/node_cli/core/nftables.py b/node_cli/core/nftables.py index 8ac939f6..3119415d 100644 --- a/node_cli/core/nftables.py +++ b/node_cli/core/nftables.py @@ -479,11 +479,7 @@ def delete_rule_by_handle(self, handle: int, chain: Optional[str] = None) -> Non self._execute_rule_with_op('delete', chain or self.chain, handle=handle) def remove_stale_envelope_rules(self, envelope: tuple[int, int]) -> None: - """Remove sChain envelope accepts anchored at a different base port. - - Both envelope shapes are recognized: the full node allocation and - the single-chain range used on passive and fair nodes. - """ + """Remove sChain envelope accepts anchored at a different base port.""" envelope_spans = (SCHAIN_PORTS_PER_NODE - 1, PORTS_PER_SCHAIN - 1) for rule in self.get_rules(self.chain): expr = rule.get('expr', []) @@ -504,11 +500,7 @@ def remove_stale_envelope_rules(self, envelope: tuple[int, int]) -> None: self.delete_rule_by_handle(rule['handle']) def remove_misordered_udp_drop(self) -> None: - """Delete the blanket udp drop when it shadows the udp DNS accept. - - Rulesets created before the udp payload fix have the drop above the - accept; setup re-adds the drop after all accept rules. - """ + """Delete the blanket udp drop when it shadows the udp DNS accept. """ udp_drop = [ip_protocol_match('udp'), {'counter': None}, {'drop': None}] udp_dns_accept = [ dport_match('udp', ServicePort.DNS, ServicePort.DNS), @@ -556,14 +548,7 @@ def ensure_user_chain_jump(self) -> None: self._ensure_rule(self.chain, expr, op='insert', label='user chain jump') def apply_user_rules(self) -> None: - """Reload user.conf into the live user rules chain. - - The file content is fed through the native nft parser inside the - chain declaration - exactly how the boot include reads it - so - comments and multiline rules behave identically in both paths. - Flush plus reload in one transaction keeps the chain in sync with - the file. - """ + """Reload user.conf into the live user rules chain.""" content = '' if os.path.isfile(NFTABLES_USER_CONFIG_PATH): with open(NFTABLES_USER_CONFIG_PATH) as user_config: @@ -582,12 +567,7 @@ def apply_user_rules(self) -> None: def remove_user_rules_from_main_chain(self) -> None: """Remove user.conf rules that older saved configs loaded into the - skale chain directly, so they are not snapshotted as duplicates. - - The freshly reloaded user chain is the parsed form of user.conf, so - rules are matched by expression - immune to comments, multiline - formatting and rendering differences. Runs before the service rules - are re-added, so removing an expression they share cannot last. + skale chain directly. """ user_exprs = [ self._normalized_expr(rule.get('expr', [])) for rule in self.get_rules(USER_CHAIN) @@ -740,12 +720,7 @@ def firewall_default_drop_enabled() -> bool: def get_registered_base_port() -> Optional[tuple[int, int]]: - """Base port and envelope size from the node config. - - node_base_port is a node registration port and reserves the full node - allocation; schain_base_port (passive and fair nodes) is one already - allocated chain's base port and reserves that single chain's range. - """ + """Base port and envelope size from the node config.""" if not os.path.isfile(NODE_CONFIG_PATH): return None try: diff --git a/node_cli/utils/helper.py b/node_cli/utils/helper.py index 8f3b8ed9..253ac5a9 100644 --- a/node_cli/utils/helper.py +++ b/node_cli/utils/helper.py @@ -408,45 +408,52 @@ def get_tmp_path(path: str | Path) -> str: return base + salt + '.tmp' + ext +SSH_PORTS_ERROR = 'Cannot determine valid SSH ports. Set SSH_PORT to an integer from 1 to 65535.' + + +def _effective_sshd_config_ports() -> list[str]: + """Port values from `sshd -T`; ListenAddress entries take precedence.""" + try: + result = subprocess.run( + [shutil.which('sshd') or '/usr/sbin/sshd', '-T'], + capture_output=True, + text=True, + check=True, + timeout=10, + ) + except (OSError, subprocess.SubprocessError) as err: + raise RuntimeError( + 'Cannot determine SSH ports from sshd -T. Set SSH_PORT to the ' + 'SSH listening port before configuring the firewall.' + ) from err + ports, listen_ports = [], [] + for line in result.stdout.splitlines(): + fields = line.split() + if len(fields) < 2: + continue + if fields[0] == 'port': + ports.append(fields[1]) + elif fields[0] == 'listenaddress': + # sshd -T expands addresses to IPv4:port or [IPv6]:port. + listen_ports.append(fields[1].rsplit(':', 1)[-1]) + return listen_ports or ports + + +def _validated_ssh_ports(values: list[str]) -> list[int]: + try: + ports = sorted({int(value) for value in values}) + except ValueError as err: + raise ValueError(SSH_PORTS_ERROR) from err + if not ports or any(not 1 <= port <= 65535 for port in ports): + raise ValueError(SSH_PORTS_ERROR) + return ports + + def get_ssh_ports() -> list[int]: """Return SSH_PORT or the ports from the effective default sshd config.""" override = os.getenv('SSH_PORT') - if override is not None: - values = [override] - else: - try: - result = subprocess.run( - [shutil.which('sshd') or '/usr/sbin/sshd', '-T'], - capture_output=True, - text=True, - check=True, - timeout=10, - ) - except (OSError, subprocess.SubprocessError) as err: - raise RuntimeError( - 'Cannot determine SSH ports from sshd -T. Set SSH_PORT to the ' - 'SSH listening port before configuring the firewall.' - ) from err - ports, listen_ports = [], [] - for line in result.stdout.splitlines(): - fields = line.split() - if len(fields) < 2: - continue - if fields[0] == 'port': - ports.append(fields[1]) - elif fields[0] == 'listenaddress': - # sshd -T expands addresses to IPv4:port or [IPv6]:port. - listen_ports.append(fields[1].rsplit(':', 1)[-1]) - values = listen_ports or ports - try: - parsed_ports = sorted({int(value) for value in values}) - if not parsed_ports or any(not 1 <= port <= 65535 for port in parsed_ports): - raise ValueError('Missing or out-of-range port') - except ValueError as err: - raise ValueError( - 'Cannot determine valid SSH ports. Set SSH_PORT to an integer from 1 to 65535.' - ) from err - return parsed_ports + values = [override] if override is not None else _effective_sshd_config_ports() + return _validated_ssh_ports(values) def get_ssh_port(ssh_service_name='ssh') -> int: From 58c0abbd31b542751322892e008160ad7701234c Mon Sep 17 00:00:00 2001 From: badrogger Date: Fri, 11 Sep 2026 13:33:34 +0100 Subject: [PATCH 11/16] Fix setup --- node_cli/utils/helper.py | 5 ++++- tests/core/iptables_test.py | 15 ++++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/node_cli/utils/helper.py b/node_cli/utils/helper.py index 253ac5a9..08ea4f57 100644 --- a/node_cli/utils/helper.py +++ b/node_cli/utils/helper.py @@ -414,9 +414,12 @@ def get_tmp_path(path: str | Path) -> str: def _effective_sshd_config_ports() -> list[str]: """Port values from `sshd -T`; ListenAddress entries take precedence.""" try: + # explicit pipes instead of capture_output: environments that wrap + # subprocess.run with their own stdout/stderr reject the combination result = subprocess.run( [shutil.which('sshd') or '/usr/sbin/sshd', '-T'], - capture_output=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, text=True, check=True, timeout=10, diff --git a/tests/core/iptables_test.py b/tests/core/iptables_test.py index 07144eee..19b34255 100644 --- a/tests/core/iptables_test.py +++ b/tests/core/iptables_test.py @@ -35,10 +35,23 @@ def test_effective_sshd_ports(config, expected): run.return_value.stdout = config assert get_ssh_ports() == expected run.assert_called_once_with( - ['/usr/sbin/sshd', '-T'], capture_output=True, text=True, check=True, timeout=10 + ['/usr/sbin/sshd', '-T'], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + check=True, + timeout=10, ) +def test_sshd_invocation_arguments_are_valid(): + """Run a real process: mocks cannot catch invalid subprocess.run + argument combinations such as capture_output with stdout/stderr.""" + with mock.patch('node_cli.utils.helper.shutil.which', return_value='/bin/echo'): + with pytest.raises(ValueError, match='SSH_PORT'): + get_ssh_ports() + + def test_ssh_port_override(monkeypatch): monkeypatch.setenv('SSH_PORT', '2222') with mock.patch('node_cli.utils.helper.subprocess.run') as run: From 959be02e80c15bcff7b09be4cd023b06cb9d1003 Mon Sep 17 00:00:00 2001 From: badrogger Date: Fri, 11 Sep 2026 13:54:33 +0100 Subject: [PATCH 12/16] Remove MONITORING_PORTS functionality --- README.md | 6 ++ node_cli/cli/node.py | 5 +- node_cli/core/nftables.py | 34 +++++++---- node_cli/core/node.py | 14 ++--- node_cli/operations/base.py | 12 ++-- node_cli/operations/fair.py | 6 +- tests/cli/node_test.py | 14 +++++ tests/core/monitoring_firewall_test.py | 81 ++++++++++++++++++++++++++ tests/core/ssh_firewall_test.py | 2 +- 9 files changed, 141 insertions(+), 33 deletions(-) create mode 100644 tests/core/monitoring_firewall_test.py diff --git a/README.md b/README.md index e2b133b2..b80ea032 100644 --- a/README.md +++ b/README.md @@ -121,6 +121,12 @@ Options: #### Configure firewall +Firewall setup does not automatically open monitoring ports 9100 and 8080. +Reconfiguration removes their legacy allow rules from the managed base chain +and saves the updated rules for reboot. `MONITORING_CONTAINERS` controls the +containers only; the firewall's `--monitoring` option has been removed. +Explicit rules in `/etc/nft.conf.d/skale/user.conf` remain under operator control. + SSH allow rules use all listening ports reported by `sshd -T`, including ports configured through included files and `ListenAddress`. If detection fails, the command stops before enabling the default-drop policy. diff --git a/node_cli/cli/node.py b/node_cli/cli/node.py index b34fd155..20861a55 100644 --- a/node_cli/cli/node.py +++ b/node_cli/cli/node.py @@ -239,7 +239,6 @@ def check(network): @node.command(help='Reconfigure nftables rules') -@click.option('--monitoring', is_flag=True) @click.option( '--yes', is_flag=True, @@ -247,8 +246,8 @@ def check(network): expose_value=False, prompt='Are you sure you want to reconfigure firewall rules?', ) -def configure_firewall(monitoring): - configure_firewall_rules(enable_monitoring=monitoring) +def configure_firewall(): + configure_firewall_rules() @node.command(help='Show node version information') diff --git a/node_cli/core/nftables.py b/node_cli/core/nftables.py index 3119415d..7ca58ef2 100644 --- a/node_cli/core/nftables.py +++ b/node_cli/core/nftables.py @@ -500,7 +500,7 @@ def remove_stale_envelope_rules(self, envelope: tuple[int, int]) -> None: self.delete_rule_by_handle(rule['handle']) def remove_misordered_udp_drop(self) -> None: - """Delete the blanket udp drop when it shadows the udp DNS accept. """ + """Delete the blanket udp drop when it shadows the udp DNS accept.""" udp_drop = [ip_protocol_match('udp'), {'counter': None}, {'drop': None}] udp_dns_accept = [ dport_match('udp', ServicePort.DNS, ServicePort.DNS), @@ -597,7 +597,22 @@ def _setup_user_chain(self) -> None: self.apply_user_rules() self.remove_user_rules_from_main_chain() - def _add_service_accepts(self, enable_monitoring: bool) -> None: + def remove_monitoring_accepts(self) -> None: + """Remove every legacy monitoring accept from the managed base chain.""" + ssh_ports = get_ssh_ports() + monitoring_exprs = [ + Rule(chain=self.chain, protocol='tcp', first_port=port).to_expr() + for port in (ServicePort.EXPORTER, ServicePort.CADVISOR) + if port not in ssh_ports + ] + for rule in self.get_rules(self.chain): + if ( + self._normalized_expr(rule.get('expr', [])) in monitoring_exprs + and rule.get('handle') is not None + ): + self.delete_rule_by_handle(rule['handle']) + + def _add_service_accepts(self) -> None: self._ensure_rule(self.chain, conntrack_accept_expr(), label='connection tracking rule') tcp_ports = [ *get_ssh_ports(), @@ -607,8 +622,6 @@ def _add_service_accepts(self, enable_monitoring: bool) -> None: ServicePort.WATCHDOG_HTTP, ServicePort.WATCHDOG_HTTPS, ] - if enable_monitoring: - tcp_ports.extend([ServicePort.EXPORTER, ServicePort.CADVISOR]) for port in tcp_ports: self.add_rule(Rule(chain=self.chain, protocol='tcp', first_port=port)) self.remove_misordered_udp_drop() @@ -636,9 +649,7 @@ def _add_drop_rules(self) -> None: ) self.add_drop_rule(Rule(chain=self.chain, protocol='udp')) - def setup_firewall( - self, enable_monitoring: bool = False, keep_accept_policy: bool = False - ) -> None: + def setup_firewall(self, keep_accept_policy: bool = False) -> None: """Setup firewall rules.""" logger.info('Configuring firewall rules') @@ -657,7 +668,8 @@ def setup_firewall( self.validate_dynamic_ranges(envelope) self._setup_user_chain() - self._add_service_accepts(enable_monitoring) + self.remove_monitoring_accepts() + self._add_service_accepts() self._add_icmp_accepts() self._ensure_envelope(envelope) self._add_drop_rules() @@ -765,13 +777,11 @@ def prepare_directories() -> None: create_user_config_path() -def configure_nftables(enable_monitoring: bool = False, keep_accept_policy: bool = False) -> None: +def configure_nftables(keep_accept_policy: bool = False) -> None: prepare_directories() enable_nftables_service() nft_mgr = NFTablesManager() - nft_mgr.setup_firewall( - enable_monitoring=enable_monitoring, keep_accept_policy=keep_accept_policy - ) + nft_mgr.setup_firewall(keep_accept_policy=keep_accept_policy) ruleset = nft_mgr.get_base_ruleset() save_nftables_rules(ruleset) remove_legacy_saved_rules() diff --git a/node_cli/core/node.py b/node_cli/core/node.py index 0497d02a..ea37e640 100644 --- a/node_cli/core/node.py +++ b/node_cli/core/node.py @@ -153,7 +153,7 @@ def register_node(name, p2p_ip, public_ip, port, domain_name): try: save_registered_base_port(port) logger.info('Reconfiguring firewall for the registered base port %d', port) - configure_nftables(enable_monitoring=get_settings().monitoring_containers) + configure_nftables() except Exception: logger.exception('Post-registration firewall reconfiguration failed') error_exit( @@ -249,13 +249,11 @@ def init_passive( time.sleep(TM_INIT_TIMEOUT) if not is_base_containers_alive(node_type=NodeType.SKALE, node_mode=node_mode): error_exit('Containers are not running', exit_code=CLIExitCodes.OPERATION_EXECUTION_ERROR) - enable_firewall_default_drop_when_port_available(settings) + enable_firewall_default_drop_when_port_available() logger.info('Passive node initialized successfully') -def enable_firewall_default_drop_when_port_available( - settings, timeout: int = 300, interval: int = 5 -) -> None: +def enable_firewall_default_drop_when_port_available(timeout: int = 300, interval: int = 5) -> None: """Flip the firewall to default drop once admin saves the base port. Passive init configures nftables before skale-admin computes the mirrored @@ -264,7 +262,7 @@ def enable_firewall_default_drop_when_port_available( start = time.monotonic() while time.monotonic() - start < timeout: if get_registered_base_port() is not None: - configure_nftables(enable_monitoring=settings.monitoring_containers) + configure_nftables() return time.sleep(interval) logger.warning( @@ -581,5 +579,5 @@ def run_checks( print_failed_requirements_checks(failed_checks) -def configure_firewall_rules(enable_monitoring: bool = False) -> None: - configure_nftables(enable_monitoring=enable_monitoring) +def configure_firewall_rules() -> None: + configure_nftables() diff --git a/node_cli/operations/base.py b/node_cli/operations/base.py index 21a219dd..54e9e66a 100644 --- a/node_cli/operations/base.py +++ b/node_cli/operations/base.py @@ -133,7 +133,7 @@ def update(settings: BaseNodeSettings, compose_env: dict, node_mode: NodeMode) - if not settings.skip_docker_config: configure_docker() - configure_nftables(enable_monitoring=settings.monitoring_containers) + configure_nftables() lvmpy_install(settings.block_device) generate_nginx_config() @@ -177,7 +177,7 @@ def init(settings: BaseNodeSettings, compose_env: dict, node_mode: NodeMode) -> if not settings.skip_docker_config: configure_docker() - configure_nftables(enable_monitoring=settings.monitoring_containers) + configure_nftables() prepare_host(env_type=settings.env_type) @@ -223,7 +223,7 @@ def init_passive( if not settings.skip_docker_config: configure_docker() - configure_nftables(enable_monitoring=settings.monitoring_containers, keep_accept_policy=True) + configure_nftables(keep_accept_policy=True) prepare_host(env_type=settings.env_type) save_internal_settings(node_type=NodeType.SKALE, node_mode=NodeMode.PASSIVE) @@ -284,7 +284,7 @@ def update_passive(settings: BaseNodeSettings, compose_env: dict) -> bool: if not settings.skip_docker_config: configure_docker() - configure_nftables(enable_monitoring=settings.monitoring_containers) + configure_nftables() ensure_filestorage_mapping() @@ -364,7 +364,7 @@ def turn_on( if not settings.skip_docker_config: configure_docker() - configure_nftables(enable_monitoring=settings.monitoring_containers) + configure_nftables() save_internal_settings(node_type=node_type, node_mode=node_mode, backup_run=backup_run) logger.info('Launching containers on the node...') @@ -398,7 +398,7 @@ def restore( if not settings.skip_docker_config: configure_docker() - configure_nftables(enable_monitoring=settings.monitoring_containers) + configure_nftables() lvmpy_install(settings.block_device) init_shared_space_volume(settings.env_type) diff --git a/node_cli/operations/fair.py b/node_cli/operations/fair.py index efba278b..d2886090 100644 --- a/node_cli/operations/fair.py +++ b/node_cli/operations/fair.py @@ -98,7 +98,7 @@ def init_fair_boot( if not settings.skip_docker_config: configure_docker() - configure_nftables(enable_monitoring=settings.monitoring_containers) + configure_nftables() prepare_host(env_type=settings.env_type) save_internal_settings(node_type=NodeType.FAIR, node_mode=NodeMode.ACTIVE) @@ -212,7 +212,7 @@ def update_fair_boot( if not settings.skip_docker_config: configure_docker() - configure_nftables(enable_monitoring=settings.monitoring_containers) + configure_nftables() generate_nginx_config() fair_settings = get_settings((FairSettings, FairBaseSettings)) @@ -346,7 +346,7 @@ def restore( if not settings.skip_docker_config: configure_docker() - configure_nftables(enable_monitoring=settings.monitoring_containers) + configure_nftables() meta_manager = FairCliMetaManager() meta_manager.update_meta( diff --git a/tests/cli/node_test.py b/tests/cli/node_test.py index 8e3ac3d8..9ba85528 100644 --- a/tests/cli/node_test.py +++ b/tests/cli/node_test.py @@ -31,6 +31,7 @@ _turn_on, backup_node, cleanup_node, + configure_firewall, node_info, register_node, remove_node_from_maintenance, @@ -56,6 +57,19 @@ init_default_logger() +def test_configure_firewall_without_monitoring_option(): + with mock.patch('node_cli.cli.node.configure_firewall_rules') as configure: + result = run_command(configure_firewall, ['--yes']) + assert result.exit_code == 0, result.output + configure.assert_called_once_with() + + configure.reset_mock() + result = run_command(configure_firewall, ['--yes', '--monitoring']) + assert result.exit_code == 2 + assert 'No such option: --monitoring' in result.output + configure.assert_not_called() + + def test_register_node(inited_node, resource_alloc, mocked_g_config): resp_mock = response_mock(requests.codes.ok, {'status': 'ok', 'payload': None}) with ( diff --git a/tests/core/monitoring_firewall_test.py b/tests/core/monitoring_firewall_test.py new file mode 100644 index 00000000..2a17cd17 --- /dev/null +++ b/tests/core/monitoring_firewall_test.py @@ -0,0 +1,81 @@ +from pathlib import Path +from unittest.mock import patch + +import pytest + +import node_cli.core.nftables as firewall +from node_cli.core.nftables import NFTablesManager, Rule + + +@pytest.fixture +def monitoring_firewall(monkeypatch, tmp_path): + """Run only in the isolated nftables test container (requires NET_ADMIN).""" + monkeypatch.setenv('SSH_PORT', '22') + monkeypatch.setenv('MONITORING_CONTAINERS', 'True') + monkeypatch.delenv('SCHAIN_BASE_PORT', raising=False) + monkeypatch.delenv('FIREWALL_DEFAULT_DROP', raising=False) + monkeypatch.setattr(firewall, 'NODE_CONFIG_PATH', str(tmp_path / 'node.json')) + monkeypatch.setattr(firewall, 'NFTABLES_USER_CONFIG_PATH', str(tmp_path / 'user.conf')) + monkeypatch.setattr(firewall, 'NFTABLES_SKALE_BASE_CONFIG_PATH', str(tmp_path / 'base.conf')) + monkeypatch.setattr(firewall, 'NFTABLES_CHAIN_CONFIG_WILDCARD', str(tmp_path / 'chains/*')) + (tmp_path / 'chains').mkdir() + (tmp_path / 'user.conf').touch() + manager = NFTablesManager(table='monitoring_test') + manager.create_table_if_not_exists() + manager.create_chain_if_not_exists(manager.chain, hook='input') + try: + yield manager + finally: + manager.execute_cmd( + {'nftables': [{'delete': {'table': {'family': manager.family, 'name': manager.table}}}]} + ) + + +@pytest.mark.parametrize('legacy_rule_copies', [0, 1, 2]) +def test_monitoring_accepts_absent_after_setup_and_reload(monitoring_firewall, legacy_rule_copies): + manager = monitoring_firewall + expressions = [Rule(manager.chain, 'tcp', port).to_expr() for port in (8080, 9100)] + for _ in range(legacy_rule_copies): + for expr in expressions: + manager._execute_rule_with_op('add', manager.chain, expr) + + for _ in range(2): + manager.setup_firewall() + assert manager.get_chain_policy(manager.chain) == 'drop' + for expr in expressions: + assert not manager.rule_exists(manager.chain, expr) + manager.verify_critical_accepts() + + firewall.save_nftables_base_rules(manager.get_base_ruleset()) + manager.execute_cmd( + {'nftables': [{'delete': {'table': {'family': manager.family, 'name': manager.table}}}]} + ) + rc, _, error = manager.nft.cmd(f'include "{firewall.NFTABLES_SKALE_BASE_CONFIG_PATH}"') + assert rc == 0, error + assert manager.get_chain_policy(manager.chain) == 'drop' + for expr in expressions: + assert not manager.rule_exists(manager.chain, expr) + manager.verify_critical_accepts() + + +def test_monitoring_cleanup_preserves_explicit_user_rules(monitoring_firewall): + manager = monitoring_firewall + Path(firewall.NFTABLES_USER_CONFIG_PATH).write_text('tcp dport 8080 counter accept\n') + manager.add_rule(Rule(manager.chain, 'tcp', 8080)) + manager.setup_firewall() + expr = Rule(manager.chain, 'tcp', 8080).to_expr() + assert not manager.rule_exists(manager.chain, expr) + assert manager.rule_exists(firewall.USER_CHAIN, expr) + + +def test_monitoring_cleanup_preserves_custom_ssh_port(monitoring_firewall, monkeypatch): + monkeypatch.setenv('SSH_PORT', '9100') + manager = monitoring_firewall + manager.add_rule(Rule(manager.chain, 'tcp', 9100)) + with patch.object( + manager, 'delete_rule_by_handle', wraps=manager.delete_rule_by_handle + ) as delete: + manager.setup_firewall() + delete.assert_not_called() + manager.verify_critical_accepts() + assert manager.rule_exists(manager.chain, Rule(manager.chain, 'tcp', 9100).to_expr()) diff --git a/tests/core/ssh_firewall_test.py b/tests/core/ssh_firewall_test.py index 98496de8..7689c881 100644 --- a/tests/core/ssh_firewall_test.py +++ b/tests/core/ssh_firewall_test.py @@ -21,7 +21,7 @@ def test_allow_all_ssh_ports(manager): patch.object(manager, 'remove_misordered_udp_drop'), patch.object(manager, 'add_rule', side_effect=rules.append), ): - manager._add_service_accepts(enable_monitoring=False) + manager._add_service_accepts() allowed_tcp_ports = {rule.first_port for rule in rules if rule.protocol == 'tcp'} assert {2200, 2222} <= allowed_tcp_ports assert 22 not in allowed_tcp_ports From 8cc24cfc11f537171f07019e30328b6a60f5af18 Mon Sep 17 00:00:00 2001 From: badrogger Date: Fri, 11 Sep 2026 16:03:27 +0100 Subject: [PATCH 13/16] Bump version --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index b2c09ef8..9eebed98 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "node-cli" -version = "3.2.0" +version = "3.3.0" description = "Node CLI tools" readme = "README.md" requires-python = ">=3.13" From ed032a4918be940901dd969f20c2c63da109bf1b Mon Sep 17 00:00:00 2001 From: badrogger Date: Fri, 11 Sep 2026 16:43:56 +0100 Subject: [PATCH 14/16] Improve cleanup --- node_cli/core/nftables.py | 78 ++++- node_cli/operations/base.py | 8 +- node_cli/operations/fair.py | 7 +- tests/.skale/.skale-cli-log/.gitkeep | 0 tests/.skale/.skale-cli.yaml | 1 - tests/.skale/.tmp/.keep | 0 tests/.skale/config/docker-compose-fair.yml | 5 - tests/.skale/config/schain_allocation.yml | 203 ------------- tests/.skale/config/static_params.yaml | 306 -------------------- tests/core/nftables_test.py | 99 +++++++ 10 files changed, 180 insertions(+), 527 deletions(-) delete mode 100644 tests/.skale/.skale-cli-log/.gitkeep delete mode 100644 tests/.skale/.skale-cli.yaml delete mode 100644 tests/.skale/.tmp/.keep delete mode 100644 tests/.skale/config/docker-compose-fair.yml delete mode 100644 tests/.skale/config/schain_allocation.yml delete mode 100644 tests/.skale/config/static_params.yaml diff --git a/node_cli/core/nftables.py b/node_cli/core/nftables.py index 7ca58ef2..0d8cf0ba 100644 --- a/node_cli/core/nftables.py +++ b/node_cli/core/nftables.py @@ -36,7 +36,7 @@ NFTABLES_USER_CONFIG_PATH, NODE_CONFIG_PATH, ) -from node_cli.utils.helper import get_ssh_ports, read_json, run_cmd +from node_cli.utils.helper import cleanup_dir_content, get_ssh_ports, read_json, run_cmd logger = logging.getLogger(__name__) @@ -409,12 +409,12 @@ def remove_rule(self, rule: Rule) -> None: else: logger.info('No %s rule for %s to remove', rule.protocol, rule.first_port) - def get_dynamic_chain_port_ranges(self) -> list[tuple[str, int, int]]: - """Min/max tcp dport covered by each dynamic skale-admin chain.""" + def _table_listing(self) -> dict: + """Parsed json listing of the managed table; empty when absent.""" rc, output, error = self.nft.cmd(f'list table {self.family} {self.table}') if rc != 0: if error and 'No such file or directory' in error: - return [] + return {} raise NFTablesError(f'Failed to list table {self.table}: {error}') try: data = json.loads(output) @@ -422,6 +422,18 @@ def get_dynamic_chain_port_ranges(self) -> list[tuple[str, int, int]]: raise NFTablesError(f'Failed to parse table {self.table} listing: {err}') from err if not isinstance(data, dict): raise NFTablesError(f'Malformed table {self.table} listing') + return data + + def _table_chain_names(self) -> list[str]: + return [ + item['chain']['name'] + for item in self._table_listing().get('nftables', []) + if isinstance(item, dict) and isinstance(item.get('chain'), dict) + ] + + def get_dynamic_chain_port_ranges(self) -> list[tuple[str, int, int]]: + """Min/max tcp dport covered by each dynamic skale-admin chain.""" + data = self._table_listing() ports: dict[str, list[int]] = {} for item in data.get('nftables', []): @@ -725,6 +737,47 @@ def flush_chain(self, chain: str) -> None: logger.error(f'Failed to flush chain: {str(e)}') raise NFTablesError('Flushing chain errored') + def delete_chain(self, chain: str) -> None: + chain_spec = {'family': self.family, 'table': self.table, 'name': chain} + self.execute_cmd( + {'nftables': [{'flush': {'chain': chain_spec}}, {'delete': {'chain': chain_spec}}]} + ) + logger.info('Deleted chain %s', chain) + + def _critical_accept_exprs(self) -> list[list[dict]]: + """Rules that keep the node reachable: conntrack, loopback, ssh, DNS.""" + exprs = [conntrack_accept_expr(), loopback_accept_expr()] + try: + ssh_ports = get_ssh_ports() + except (RuntimeError, ValueError): + # the policy is accept during cleanup, so reachability is safe + # even when ssh detection is impossible + ssh_ports = [] + for port in (*ssh_ports, ServicePort.DNS): + exprs.append(Rule(chain=self.chain, protocol='tcp', first_port=port).to_expr()) + exprs.append(Rule(chain=self.chain, protocol='udp', first_port=ServicePort.DNS).to_expr()) + return exprs + + def cleanup_firewall(self) -> None: + """Reset the firewall to a minimal state that keeps the node reachable. + + Restores the accept policy, removes the user and dynamic chains and + every rule except the critical accepts: conntrack, loopback, ssh + and DNS. + """ + self.ensure_default_accept() + self._remove_rule_by_expr(self.chain, [{'jump': {'target': USER_CHAIN}}]) + for chain in self._table_chain_names(): + if chain == USER_CHAIN or chain.startswith(DYNAMIC_CHAIN_PREFIX): + self.delete_chain(chain) + keep = [self._normalized_expr(expr) for expr in self._critical_accept_exprs()] + for rule in self.get_rules(self.chain): + if ( + self._normalized_expr(rule.get('expr', [])) not in keep + and rule.get('handle') is not None + ): + self.delete_rule_by_handle(rule['handle']) + def firewall_default_drop_enabled() -> bool: value = os.getenv(FIREWALL_DEFAULT_DROP_ENV, 'True') @@ -787,6 +840,23 @@ def configure_nftables(keep_accept_policy: bool = False) -> None: remove_legacy_saved_rules() +def cleanup_nftables() -> None: + """Reset the firewall after node cleanup and persist the minimal state.""" + logger.info('Cleaning up firewall rules') + nft_mgr = NFTablesManager() + ruleset = '' + if nft_mgr.table_exists() and nft_mgr.chain_exists(nft_mgr.chain): + nft_mgr.cleanup_firewall() + ruleset = nft_mgr.get_base_ruleset() + if os.path.isdir(NFTABLES_CHAIN_FOLDER_PATH): + cleanup_dir_content(NFTABLES_CHAIN_FOLDER_PATH) + if os.path.isdir(os.path.dirname(NFTABLES_SKALE_BASE_CONFIG_PATH)): + # a plain snapshot with no includes - reboot restores the same + # minimal ruleset + with open(NFTABLES_SKALE_BASE_CONFIG_PATH, 'w') as base_config: + base_config.write(ruleset) + + def enable_nftables_service() -> None: logger.info('Enabling nftables services') run_cmd(['systemctl', 'enable', 'nftables']) diff --git a/node_cli/operations/base.py b/node_cli/operations/base.py index 54e9e66a..d2bb5c57 100644 --- a/node_cli/operations/base.py +++ b/node_cli/operations/base.py @@ -31,7 +31,6 @@ CONTAINER_CONFIG_PATH, CONTAINER_CONFIG_TMP_PATH, GLOBAL_SKALE_DIR, - NFTABLES_CHAIN_FOLDER_PATH, SKALE_DIR, ) from node_cli.core.checks import CheckType @@ -41,7 +40,7 @@ ensure_btrfs_kernel_module_autoloaded, prepare_host, ) -from node_cli.core.nftables import configure_nftables +from node_cli.core.nftables import cleanup_nftables, configure_nftables from node_cli.core.nginx import generate_nginx_config from node_cli.core.node_options import ( mark_active_node, @@ -74,7 +73,7 @@ rm_legacy_containers, system_prune, ) -from node_cli.utils.helper import cleanup_dir_content, rm_dir +from node_cli.utils.helper import rm_dir from node_cli.utils.meta import CliMetaManager, FairCliMetaManager from node_cli.utils.node_type import NodeMode, NodeType from node_cli.utils.print_formatters import print_failed_requirements_checks @@ -432,6 +431,7 @@ def restore( def cleanup_passive(compose_env: dict, schain_name: str) -> None: turn_off(compose_env, node_type=NodeType.SKALE, node_mode=NodeMode.PASSIVE) + cleanup_nftables() cleanup_no_lvm_datadir(chain_name=schain_name) rm_dir(GLOBAL_SKALE_DIR) rm_dir(SKALE_DIR) @@ -441,6 +441,7 @@ def cleanup( node_mode: NodeMode, compose_env: dict, schain_name: Optional[str] = None, prune: bool = False ) -> None: turn_off(compose_env, node_type=NodeType.SKALE, node_mode=node_mode) + cleanup_nftables() if prune: system_prune() if node_mode == NodeMode.PASSIVE: @@ -449,5 +450,4 @@ def cleanup( cleanup_lvm_datadir() rm_dir(GLOBAL_SKALE_DIR) rm_dir(SKALE_DIR) - cleanup_dir_content(NFTABLES_CHAIN_FOLDER_PATH) cleanup_docker_configuration() diff --git a/node_cli/operations/fair.py b/node_cli/operations/fair.py index d2886090..f2e8746c 100644 --- a/node_cli/operations/fair.py +++ b/node_cli/operations/fair.py @@ -30,14 +30,13 @@ from node_cli.configs import ( CONTAINER_CONFIG_PATH, GLOBAL_SKALE_DIR, - NFTABLES_CHAIN_FOLDER_PATH, SKALE_DIR, ) from node_cli.core.checks import CheckType from node_cli.core.checks import run_checks as run_host_checks from node_cli.core.docker_config import cleanup_docker_configuration, configure_docker from node_cli.core.host import ensure_btrfs_kernel_module_autoloaded, prepare_host -from node_cli.core.nftables import configure_nftables +from node_cli.core.nftables import cleanup_nftables, configure_nftables from node_cli.core.nginx import generate_nginx_config from node_cli.core.schains import cleanup_no_lvm_datadir from node_cli.core.static_config import get_fair_chain_name @@ -70,7 +69,7 @@ system_prune, wait_for_container, ) -from node_cli.utils.helper import cleanup_dir_content, rm_dir +from node_cli.utils.helper import rm_dir from node_cli.utils.meta import FairCliMetaManager from node_cli.utils.print_formatters import print_failed_requirements_checks from node_cli.utils.node_type import NodeMode, NodeType @@ -375,12 +374,12 @@ def restore( def cleanup(node_mode: NodeMode, compose_env: dict, prune: bool = False) -> None: turn_off(compose_env, node_type=NodeType.FAIR, node_mode=node_mode) + cleanup_nftables() if prune: system_prune() cleanup_no_lvm_datadir() rm_dir(GLOBAL_SKALE_DIR) rm_dir(SKALE_DIR) - cleanup_dir_content(NFTABLES_CHAIN_FOLDER_PATH) cleanup_docker_configuration() diff --git a/tests/.skale/.skale-cli-log/.gitkeep b/tests/.skale/.skale-cli-log/.gitkeep deleted file mode 100644 index e69de29b..00000000 diff --git a/tests/.skale/.skale-cli.yaml b/tests/.skale/.skale-cli.yaml deleted file mode 100644 index 03d160ba..00000000 --- a/tests/.skale/.skale-cli.yaml +++ /dev/null @@ -1 +0,0 @@ -host: https://test.com diff --git a/tests/.skale/.tmp/.keep b/tests/.skale/.tmp/.keep deleted file mode 100644 index e69de29b..00000000 diff --git a/tests/.skale/config/docker-compose-fair.yml b/tests/.skale/config/docker-compose-fair.yml deleted file mode 100644 index c09f2c08..00000000 --- a/tests/.skale/config/docker-compose-fair.yml +++ /dev/null @@ -1,5 +0,0 @@ -services: - test: - container_name: test - image: alpine:latest - network_mode: host diff --git a/tests/.skale/config/schain_allocation.yml b/tests/.skale/config/schain_allocation.yml deleted file mode 100644 index 14947b77..00000000 --- a/tests/.skale/config/schain_allocation.yml +++ /dev/null @@ -1,203 +0,0 @@ -# DO NOT MODIFY THIS FILE MANUALLY! -# Use generate_schain_allocation.py script from helper-scripts repo. - -devnet: - disk: - large: 71039975424 - medium: 8879996928 - small: 554999808 - test: 8879996928 - test4: 8879996928 - leveldb_limits: - large: - contract_storage: 12787195576 - db_storage: 4262398525 - medium: - contract_storage: 1598399446 - db_storage: 532799815 - small: - contract_storage: 99899965 - db_storage: 33299988 - test: - contract_storage: 1598399446 - db_storage: 532799815 - test4: - contract_storage: 1598399446 - db_storage: 532799815 - shared_space: 8959950848 - volume_limits: - large: - max_consensus_storage_bytes: 21311992627 - max_file_storage_bytes: 21311992627 - max_reserved_storage_bytes: 7103997542 - max_skaled_leveldb_storage_bytes: 21311992627 - medium: - max_consensus_storage_bytes: 2663999078 - max_file_storage_bytes: 2663999078 - max_reserved_storage_bytes: 887999692 - max_skaled_leveldb_storage_bytes: 2663999078 - small: - max_consensus_storage_bytes: 166499942 - max_file_storage_bytes: 166499942 - max_reserved_storage_bytes: 55499980 - max_skaled_leveldb_storage_bytes: 166499942 - test: - max_consensus_storage_bytes: 2663999078 - max_file_storage_bytes: 2663999078 - max_reserved_storage_bytes: 887999692 - max_skaled_leveldb_storage_bytes: 2663999078 - test4: - max_consensus_storage_bytes: 2663999078 - max_file_storage_bytes: 2663999078 - max_reserved_storage_bytes: 887999692 - max_skaled_leveldb_storage_bytes: 2663999078 -mainnet: - disk: - large: 1687199940608 - medium: 210899992576 - small: 13181249536 - test: 210899992576 - test4: 210899992576 - leveldb_limits: - large: - contract_storage: 303695989309 - db_storage: 101231996436 - medium: - contract_storage: 37961998663 - db_storage: 12653999554 - small: - contract_storage: 2372624916 - db_storage: 790874972 - test: - contract_storage: 37961998663 - db_storage: 12653999554 - test4: - contract_storage: 37961998663 - db_storage: 12653999554 - shared_space: 212799979520 - volume_limits: - large: - max_consensus_storage_bytes: 506159982182 - max_file_storage_bytes: 506159982182 - max_reserved_storage_bytes: 168719994060 - max_skaled_leveldb_storage_bytes: 506159982182 - medium: - max_consensus_storage_bytes: 63269997772 - max_file_storage_bytes: 63269997772 - max_reserved_storage_bytes: 21089999257 - max_skaled_leveldb_storage_bytes: 63269997772 - small: - max_consensus_storage_bytes: 3954374860 - max_file_storage_bytes: 3954374860 - max_reserved_storage_bytes: 1318124953 - max_skaled_leveldb_storage_bytes: 3954374860 - test: - max_consensus_storage_bytes: 63269997772 - max_file_storage_bytes: 63269997772 - max_reserved_storage_bytes: 21089999257 - max_skaled_leveldb_storage_bytes: 63269997772 - test4: - max_consensus_storage_bytes: 63269997772 - max_file_storage_bytes: 63269997772 - max_reserved_storage_bytes: 21089999257 - max_skaled_leveldb_storage_bytes: 63269997772 -qanet: - disk: - large: 177599938560 - medium: 22199992320 - small: 1387499520 - test: 22199992320 - test4: 22199992320 - leveldb_limits: - large: - contract_storage: 31967988940 - db_storage: 10655996313 - medium: - contract_storage: 3995998617 - db_storage: 1331999539 - small: - contract_storage: 249749913 - db_storage: 83249971 - test: - contract_storage: 3995998617 - db_storage: 1331999539 - test4: - contract_storage: 3995998617 - db_storage: 1331999539 - shared_space: 22399942656 - volume_limits: - large: - max_consensus_storage_bytes: 53279981568 - max_file_storage_bytes: 53279981568 - max_reserved_storage_bytes: 17759993856 - max_skaled_leveldb_storage_bytes: 53279981568 - medium: - max_consensus_storage_bytes: 6659997696 - max_file_storage_bytes: 6659997696 - max_reserved_storage_bytes: 2219999232 - max_skaled_leveldb_storage_bytes: 6659997696 - small: - max_consensus_storage_bytes: 416249856 - max_file_storage_bytes: 416249856 - max_reserved_storage_bytes: 138749952 - max_skaled_leveldb_storage_bytes: 416249856 - test: - max_consensus_storage_bytes: 6659997696 - max_file_storage_bytes: 6659997696 - max_reserved_storage_bytes: 2219999232 - max_skaled_leveldb_storage_bytes: 6659997696 - test4: - max_consensus_storage_bytes: 6659997696 - max_file_storage_bytes: 6659997696 - max_reserved_storage_bytes: 2219999232 - max_skaled_leveldb_storage_bytes: 6659997696 -testnet: - disk: - large: 177599938560 - medium: 22199992320 - small: 1387499520 - test: 22199992320 - test4: 22199992320 - leveldb_limits: - large: - contract_storage: 31967988940 - db_storage: 10655996313 - medium: - contract_storage: 3995998617 - db_storage: 1331999539 - small: - contract_storage: 249749913 - db_storage: 83249971 - test: - contract_storage: 3995998617 - db_storage: 1331999539 - test4: - contract_storage: 3995998617 - db_storage: 1331999539 - shared_space: 22399942656 - volume_limits: - large: - max_consensus_storage_bytes: 53279981568 - max_file_storage_bytes: 53279981568 - max_reserved_storage_bytes: 17759993856 - max_skaled_leveldb_storage_bytes: 53279981568 - medium: - max_consensus_storage_bytes: 6659997696 - max_file_storage_bytes: 6659997696 - max_reserved_storage_bytes: 2219999232 - max_skaled_leveldb_storage_bytes: 6659997696 - small: - max_consensus_storage_bytes: 416249856 - max_file_storage_bytes: 416249856 - max_reserved_storage_bytes: 138749952 - max_skaled_leveldb_storage_bytes: 416249856 - test: - max_consensus_storage_bytes: 6659997696 - max_file_storage_bytes: 6659997696 - max_reserved_storage_bytes: 2219999232 - max_skaled_leveldb_storage_bytes: 6659997696 - test4: - max_consensus_storage_bytes: 6659997696 - max_file_storage_bytes: 6659997696 - max_reserved_storage_bytes: 2219999232 - max_skaled_leveldb_storage_bytes: 6659997696 diff --git a/tests/.skale/config/static_params.yaml b/tests/.skale/config/static_params.yaml deleted file mode 100644 index a0ea143f..00000000 --- a/tests/.skale/config/static_params.yaml +++ /dev/null @@ -1,306 +0,0 @@ -common: - schain: - cpu: - skaled: 0.8 - ima: 0.2 - mem: - skaled: 0.8 - ima: 0.2 - volume_limits: - max_consensus_storage_bytes: 0.3 - max_skaled_leveldb_storage_bytes: 0.3 - max_file_storage_bytes: 0.3 - max_reserved_storage_bytes: 0.1 - leveldb_limits: - contract_storage: 0.6 - db_storage: 0.2 # leveldb may use x2 storage, so 0.4 divided by 2, actually using 0.4 - shared_space_coefficient: 1 -envs: - mainnet: - server: - cpu_total: 8 - cpu_physical: 1 - memory: 32000000000 - swap: 16000000000 - disk: 1900000000000 - - package: - iptables-persistent: 1.0.4 - lvm2: 2.02.0 - btrfs-progs: 4.15.1 - lsof: "4.89" - psmisc: 23.1-1 - - docker: - docker-api: 1.41.0 - docker-engine: 20.10.7 - docker-compose: 1.27.4 - - schain: - revertableFSPatchTimestamp: 1000000 - contractStoragePatchTimestamp: 1000000 - snapshotIntervalSec: 86400 - emptyBlockIntervalMs: 10000 - snapshotDownloadTimeout: 18000 - snapshotDownloadInactiveTimeout: 120 - - schain_cmd: - ["-v 3", "--web3-trace", "--enable-debug-behavior-apis", "--aa no"] - - node: - common: - bindIP: "0.0.0.0" - logLevel: "info" - logLevelConfig: "info" - small: - minCacheSize: 1000000 - maxCacheSize: 2000000 - collectionQueueSize: 2 - collectionDuration: 10 - transactionQueueSize: 1000 - maxOpenLeveldbFiles: 25 - medium: - minCacheSize: 8000000 - maxCacheSize: 16000000 - collectionQueueSize: 20 - collectionDuration: 60 - transactionQueueSize: 1000 - maxOpenLeveldbFiles: 1000 - large: - minCacheSize: 8000000 - maxCacheSize: 16000000 - collectionQueueSize: 20 - collectionDuration: 60 - transactionQueueSize: 1000 - maxOpenLeveldbFiles: 1000 - test: - minCacheSize: 8000000 - maxCacheSize: 16000000 - collectionQueueSize: 20 - collectionDuration: 60 - transactionQueueSize: 1000 - maxOpenLeveldbFiles: 1000 - test4: - minCacheSize: 8000000 - maxCacheSize: 16000000 - collectionQueueSize: 20 - collectionDuration: 60 - transactionQueueSize: 1000 - maxOpenLeveldbFiles: 1000 - - testnet: - server: - cpu_total: 8 - cpu_physical: 1 - memory: 32000000000 - swap: 16000000000 - disk: 200000000000 - - package: - iptables-persistent: 1.0.4 - lvm2: 2.02.0 - btrfs-progs: 4.15.1 - lsof: "4.89" - psmisc: 23.1-1 - - docker: - docker-api: 1.41.0 - docker-engine: 20.10.7 - docker-compose: 1.27.4 - - schain: - revertableFSPatchTimestamp: 1000000 - contractStoragePatchTimestamp: 1000000 - snapshotIntervalSec: 86400 - emptyBlockIntervalMs: 10000 - snapshotDownloadTimeout: 18000 - snapshotDownloadInactiveTimeout: 120 - - schain_cmd: - ["-v 3", "--web3-trace", "--enable-debug-behavior-apis", "--aa no"] - - node: - common: - bindIP: "0.0.0.0" - logLevel: "info" - logLevelConfig: "info" - small: - minCacheSize: 1000000 - maxCacheSize: 2000000 - collectionQueueSize: 2 - collectionDuration: 10 - transactionQueueSize: 1000 - maxOpenLeveldbFiles: 25 - medium: - minCacheSize: 8000000 - maxCacheSize: 16000000 - collectionQueueSize: 20 - collectionDuration: 60 - transactionQueueSize: 1000 - maxOpenLeveldbFiles: 1000 - large: - minCacheSize: 8000000 - maxCacheSize: 16000000 - collectionQueueSize: 20 - collectionDuration: 60 - transactionQueueSize: 1000 - maxOpenLeveldbFiles: 1000 - test: - minCacheSize: 8000000 - maxCacheSize: 16000000 - collectionQueueSize: 20 - collectionDuration: 60 - transactionQueueSize: 1000 - maxOpenLeveldbFiles: 1000 - test4: - minCacheSize: 8000000 - maxCacheSize: 16000000 - collectionQueueSize: 20 - collectionDuration: 60 - transactionQueueSize: 1000 - maxOpenLeveldbFiles: 1000 - - qanet: - server: - cpu_total: 8 - cpu_physical: 1 - memory: 32000000000 - swap: 16000000000 - disk: 200000000000 - - package: - iptables-persistent: 1.0.4 - lvm2: 2.02.0 - btrfs-progs: 4.15.1 - lsof: "4.89" - psmisc: 23.1-1 - - docker: - docker-api: 1.41.0 - docker-engine: 20.10.7 - docker-compose: 1.27.4 - - schain: - revertableFSPatchTimestamp: 1000000 - contractStoragePatchTimestamp: 1000000 - snapshotIntervalSec: 86400 - emptyBlockIntervalMs: 10000 - snapshotDownloadTimeout: 18000 - snapshotDownloadInactiveTimeout: 120 - - schain_cmd: - ["-v 3", "--web3-trace", "--enable-debug-behavior-apis", "--aa no"] - - node: - common: - bindIP: "0.0.0.0" - logLevel: "info" - logLevelConfig: "info" - small: - minCacheSize: 1000000 - maxCacheSize: 2000000 - collectionQueueSize: 2 - collectionDuration: 10 - transactionQueueSize: 1000 - maxOpenLeveldbFiles: 25 - medium: - minCacheSize: 8000000 - maxCacheSize: 16000000 - collectionQueueSize: 20 - collectionDuration: 60 - transactionQueueSize: 1000 - maxOpenLeveldbFiles: 1000 - large: - minCacheSize: 8000000 - maxCacheSize: 16000000 - collectionQueueSize: 20 - collectionDuration: 60 - transactionQueueSize: 1000 - maxOpenLeveldbFiles: 1000 - test: - minCacheSize: 8000000 - maxCacheSize: 16000000 - collectionQueueSize: 20 - collectionDuration: 60 - transactionQueueSize: 1000 - maxOpenLeveldbFiles: 1000 - test4: - minCacheSize: 8000000 - maxCacheSize: 16000000 - collectionQueueSize: 20 - collectionDuration: 60 - transactionQueueSize: 1000 - maxOpenLeveldbFiles: 1000 - - - devnet: - server: - cpu_total: 1 - cpu_physical: 1 - memory: 2000000000 - swap: 2000000000 - disk: 80000000000 - - package: - iptables-persistent: 1.0.4 - lvm2: 2.02.0 - btrfs-progs: 4.15.1 - lsof: "4.89" - psmisc: 23.1-1 - - docker: - docker-api: 1.41.0 - docker-engine: 20.10.7 - docker-compose: 1.27.4 - - schain: - revertableFSPatchTimestamp: 1000000 - contractStoragePatchTimestamp: 1000000 - snapshotIntervalSec: 86400 - emptyBlockIntervalMs: 10000 - snapshotDownloadTimeout: 18000 - snapshotDownloadInactiveTimeout: 120 - - schain_cmd: - ["-v 3", "--web3-trace", "--enable-debug-behavior-apis", "--aa no"] - - node: - common: - bindIP: "0.0.0.0" - logLevel: "info" - logLevelConfig: "info" - small: - minCacheSize: 1000000 - maxCacheSize: 2000000 - collectionQueueSize: 2 - collectionDuration: 10 - transactionQueueSize: 1000 - maxOpenLeveldbFiles: 25 - medium: - minCacheSize: 8000000 - maxCacheSize: 16000000 - collectionQueueSize: 20 - collectionDuration: 60 - transactionQueueSize: 1000 - maxOpenLeveldbFiles: 1000 - large: - minCacheSize: 8000000 - maxCacheSize: 16000000 - collectionQueueSize: 20 - collectionDuration: 60 - transactionQueueSize: 1000 - maxOpenLeveldbFiles: 1000 - test: - minCacheSize: 8000000 - maxCacheSize: 16000000 - collectionQueueSize: 20 - collectionDuration: 60 - transactionQueueSize: 1000 - maxOpenLeveldbFiles: 1000 - test4: - minCacheSize: 8000000 - maxCacheSize: 16000000 - collectionQueueSize: 20 - collectionDuration: 60 - transactionQueueSize: 1000 - maxOpenLeveldbFiles: 1000 diff --git a/tests/core/nftables_test.py b/tests/core/nftables_test.py index 49d88621..c5e30617 100644 --- a/tests/core/nftables_test.py +++ b/tests/core/nftables_test.py @@ -9,6 +9,7 @@ NFTablesError, NFTablesManager, Rule, + conntrack_accept_expr, dport_match, get_schain_ports_envelope, ) @@ -838,6 +839,104 @@ def test_remove_user_rules_from_main_chain(nft_manager): mock_delete.assert_not_called() +@patch.object(NFTablesManager, 'execute_cmd') +def test_delete_chain(mock_execute, nft_manager): + nft_manager.delete_chain('skale-test') + chain_spec = {'family': 'inet', 'table': 'filter', 'name': 'skale-test'} + assert mock_execute.call_args[0][0] == { + 'nftables': [{'flush': {'chain': chain_spec}}, {'delete': {'chain': chain_spec}}] + } + + +def test_cleanup_firewall(nft_manager): + critical_rule = {'handle': 1, 'expr': conntrack_accept_expr()} + envelope_rule = { + 'handle': 2, + 'expr': [dport_match('tcp', 10000, 18191), {'counter': None}, {'accept': None}], + } + watchdog_rule = { + 'handle': 3, + 'expr': Rule(chain='skale', protocol='tcp', first_port=3009).to_expr(), + } + ssh_rule = {'handle': 4, 'expr': Rule(chain='skale', protocol='tcp', first_port=22).to_expr()} + with patch.multiple( + NFTablesManager, + ensure_default_accept=Mock(), + _remove_rule_by_expr=Mock(), + _table_chain_names=Mock(return_value=['skale', 'skale_user', 'skale-mychain']), + delete_chain=Mock(), + get_rules=Mock(return_value=[critical_rule, envelope_rule, watchdog_rule, ssh_rule]), + delete_rule_by_handle=Mock(), + ): + nft_manager.cleanup_firewall() + NFTablesManager.ensure_default_accept.assert_called_once() + NFTablesManager._remove_rule_by_expr.assert_called_once_with( + 'skale', [{'jump': {'target': 'skale_user'}}] + ) + assert sorted(call.args[0] for call in NFTablesManager.delete_chain.call_args_list) == [ + 'skale-mychain', + 'skale_user', + ] + deleted = sorted( + call.args[0] for call in NFTablesManager.delete_rule_by_handle.call_args_list + ) + assert deleted == [2, 3] + + +def test_cleanup_firewall_without_ssh_detection(nft_manager, monkeypatch): + def raise_runtime_error(): + raise RuntimeError('SSH_PORT required') + + monkeypatch.setattr(nftables_core, 'get_ssh_ports', raise_runtime_error) + ssh_rule = {'handle': 4, 'expr': Rule(chain='skale', protocol='tcp', first_port=22).to_expr()} + with patch.multiple( + NFTablesManager, + ensure_default_accept=Mock(), + _remove_rule_by_expr=Mock(), + _table_chain_names=Mock(return_value=['skale']), + delete_chain=Mock(), + get_rules=Mock(return_value=[ssh_rule]), + delete_rule_by_handle=Mock(), + ): + # without detection the ssh rule is not in the keep set, which is + # safe because the policy is accept by then + nft_manager.cleanup_firewall() + NFTablesManager.delete_rule_by_handle.assert_called_once_with(4) + + +def test_cleanup_nftables(monkeypatch, tmp_path): + chains_dir = tmp_path / 'chains' + chains_dir.mkdir() + (chains_dir / 'skale-x.conf').write_text('chain skale-x {\n}\n') + base_conf = tmp_path / 'base.conf' + base_conf.write_text('old content') + monkeypatch.setattr(nftables_core, 'NFTABLES_CHAIN_FOLDER_PATH', str(chains_dir)) + monkeypatch.setattr(nftables_core, 'NFTABLES_SKALE_BASE_CONFIG_PATH', str(base_conf)) + + with patch.multiple( + NFTablesManager, + table_exists=Mock(return_value=True), + chain_exists=Mock(return_value=True), + cleanup_firewall=Mock(), + get_base_ruleset=Mock(return_value='table inet firewall {\n\tchain skale {\n\t}\n}'), + ): + nftables_core.cleanup_nftables() + NFTablesManager.cleanup_firewall.assert_called_once() + assert list(chains_dir.iterdir()) == [] + assert base_conf.read_text() == 'table inet firewall {\n\tchain skale {\n\t}\n}' + + # nothing configured: only the persisted state is cleared + base_conf.write_text('old content') + with patch.multiple( + NFTablesManager, + table_exists=Mock(return_value=False), + cleanup_firewall=Mock(), + ): + nftables_core.cleanup_nftables() + NFTablesManager.cleanup_firewall.assert_not_called() + assert base_conf.read_text() == '' + + def test_save_nftables_base_rules(monkeypatch, tmp_path): base_conf = tmp_path / 'base.conf' monkeypatch.setattr(nftables_core, 'NFTABLES_SKALE_BASE_CONFIG_PATH', str(base_conf)) From 5aa6b9727ee3bfde72050b63405ec569fa351a09 Mon Sep 17 00:00:00 2001 From: badrogger Date: Fri, 11 Sep 2026 17:24:22 +0100 Subject: [PATCH 15/16] Fix monitoring container removal --- node_cli/utils/docker_utils.py | 15 - scripts/run_nftables_test.sh | 2 +- scripts/run_tests.sh | 2 +- tests/.skale/.skale-cli-log/.gitkeep | 0 tests/.skale/.skale-cli.yaml | 1 + tests/.skale/.tmp/.keep | 0 tests/.skale/config/docker-compose-fair.yml | 5 + tests/.skale/config/schain_allocation.yml | 203 +++++++++++++ tests/.skale/config/static_params.yaml | 306 ++++++++++++++++++++ 9 files changed, 517 insertions(+), 17 deletions(-) create mode 100644 tests/.skale/.skale-cli-log/.gitkeep create mode 100644 tests/.skale/.skale-cli.yaml create mode 100644 tests/.skale/.tmp/.keep create mode 100644 tests/.skale/config/docker-compose-fair.yml create mode 100644 tests/.skale/config/schain_allocation.yml create mode 100644 tests/.skale/config/static_params.yaml diff --git a/node_cli/utils/docker_utils.py b/node_cli/utils/docker_utils.py index 6e4ad0a7..33c20fe9 100644 --- a/node_cli/utils/docker_utils.py +++ b/node_cli/utils/docker_utils.py @@ -94,10 +94,6 @@ **REDIS_SERVICE_DICT, } -MONITORING_COMPOSE_SERVICES = { - 'node-exporter': 'monitor_node_exporter', - 'advisor': 'monitor_cadvisor', -} TELEGRAF_SERVICES = ('telegraf',) NOTIFICATION_COMPOSE_SERVICES = ('celery',) COMPOSE_TIMEOUT = 10 @@ -389,17 +385,6 @@ def compose_up( env=env, ) - if settings.monitoring_containers: - logger.info('Running monitoring containers') - run_cmd( - cmd=get_up_compose_cmd( - node_type=NodeType.SKALE, - node_mode=node_mode, - services=list(MONITORING_COMPOSE_SERVICES), - ), - env=env, - ) - def restart_nginx_container(dutils=None): dutils = dutils or docker_client() diff --git a/scripts/run_nftables_test.sh b/scripts/run_nftables_test.sh index e4bf8563..ac665408 100755 --- a/scripts/run_nftables_test.sh +++ b/scripts/run_nftables_test.sh @@ -10,5 +10,5 @@ docker run \ -e GLOBAL_SKALE_DIR="$PROJECT_DIR/tests/etc/skale" \ -e DOTENV_FILEPATH='tests/test-env' \ --cap-add=NET_ADMIN --cap-add=NET_RAW \ - --name ncli-tester ncli-tester py.test tests/core/migration_test.py tests/core/nftables_test.py $@ + --name ncli-tester ncli-tester py.test tests/core/migration_test.py tests/core/nftables_test.py tests/core/monitoring_firewall_test.py $@ diff --git a/scripts/run_tests.sh b/scripts/run_tests.sh index 1d3a3fe6..27bf2aca 100755 --- a/scripts/run_tests.sh +++ b/scripts/run_tests.sh @@ -5,4 +5,4 @@ PROJECT_DIR=$(dirname $DIR) . "$DIR/export_env.sh" -py.test --cov=$PROJECT_DIR/ --ignore=tests/core/nftables_test.py --ignore=tests/core/migration_test.py tests/ $@ +py.test --cov=$PROJECT_DIR/ --ignore=tests/core/nftables_test.py --ignore=tests/core/migration_test.py --ignore=tests/core/monitoring_firewall_test.py tests/ $@ diff --git a/tests/.skale/.skale-cli-log/.gitkeep b/tests/.skale/.skale-cli-log/.gitkeep new file mode 100644 index 00000000..e69de29b diff --git a/tests/.skale/.skale-cli.yaml b/tests/.skale/.skale-cli.yaml new file mode 100644 index 00000000..03d160ba --- /dev/null +++ b/tests/.skale/.skale-cli.yaml @@ -0,0 +1 @@ +host: https://test.com diff --git a/tests/.skale/.tmp/.keep b/tests/.skale/.tmp/.keep new file mode 100644 index 00000000..e69de29b diff --git a/tests/.skale/config/docker-compose-fair.yml b/tests/.skale/config/docker-compose-fair.yml new file mode 100644 index 00000000..c09f2c08 --- /dev/null +++ b/tests/.skale/config/docker-compose-fair.yml @@ -0,0 +1,5 @@ +services: + test: + container_name: test + image: alpine:latest + network_mode: host diff --git a/tests/.skale/config/schain_allocation.yml b/tests/.skale/config/schain_allocation.yml new file mode 100644 index 00000000..14947b77 --- /dev/null +++ b/tests/.skale/config/schain_allocation.yml @@ -0,0 +1,203 @@ +# DO NOT MODIFY THIS FILE MANUALLY! +# Use generate_schain_allocation.py script from helper-scripts repo. + +devnet: + disk: + large: 71039975424 + medium: 8879996928 + small: 554999808 + test: 8879996928 + test4: 8879996928 + leveldb_limits: + large: + contract_storage: 12787195576 + db_storage: 4262398525 + medium: + contract_storage: 1598399446 + db_storage: 532799815 + small: + contract_storage: 99899965 + db_storage: 33299988 + test: + contract_storage: 1598399446 + db_storage: 532799815 + test4: + contract_storage: 1598399446 + db_storage: 532799815 + shared_space: 8959950848 + volume_limits: + large: + max_consensus_storage_bytes: 21311992627 + max_file_storage_bytes: 21311992627 + max_reserved_storage_bytes: 7103997542 + max_skaled_leveldb_storage_bytes: 21311992627 + medium: + max_consensus_storage_bytes: 2663999078 + max_file_storage_bytes: 2663999078 + max_reserved_storage_bytes: 887999692 + max_skaled_leveldb_storage_bytes: 2663999078 + small: + max_consensus_storage_bytes: 166499942 + max_file_storage_bytes: 166499942 + max_reserved_storage_bytes: 55499980 + max_skaled_leveldb_storage_bytes: 166499942 + test: + max_consensus_storage_bytes: 2663999078 + max_file_storage_bytes: 2663999078 + max_reserved_storage_bytes: 887999692 + max_skaled_leveldb_storage_bytes: 2663999078 + test4: + max_consensus_storage_bytes: 2663999078 + max_file_storage_bytes: 2663999078 + max_reserved_storage_bytes: 887999692 + max_skaled_leveldb_storage_bytes: 2663999078 +mainnet: + disk: + large: 1687199940608 + medium: 210899992576 + small: 13181249536 + test: 210899992576 + test4: 210899992576 + leveldb_limits: + large: + contract_storage: 303695989309 + db_storage: 101231996436 + medium: + contract_storage: 37961998663 + db_storage: 12653999554 + small: + contract_storage: 2372624916 + db_storage: 790874972 + test: + contract_storage: 37961998663 + db_storage: 12653999554 + test4: + contract_storage: 37961998663 + db_storage: 12653999554 + shared_space: 212799979520 + volume_limits: + large: + max_consensus_storage_bytes: 506159982182 + max_file_storage_bytes: 506159982182 + max_reserved_storage_bytes: 168719994060 + max_skaled_leveldb_storage_bytes: 506159982182 + medium: + max_consensus_storage_bytes: 63269997772 + max_file_storage_bytes: 63269997772 + max_reserved_storage_bytes: 21089999257 + max_skaled_leveldb_storage_bytes: 63269997772 + small: + max_consensus_storage_bytes: 3954374860 + max_file_storage_bytes: 3954374860 + max_reserved_storage_bytes: 1318124953 + max_skaled_leveldb_storage_bytes: 3954374860 + test: + max_consensus_storage_bytes: 63269997772 + max_file_storage_bytes: 63269997772 + max_reserved_storage_bytes: 21089999257 + max_skaled_leveldb_storage_bytes: 63269997772 + test4: + max_consensus_storage_bytes: 63269997772 + max_file_storage_bytes: 63269997772 + max_reserved_storage_bytes: 21089999257 + max_skaled_leveldb_storage_bytes: 63269997772 +qanet: + disk: + large: 177599938560 + medium: 22199992320 + small: 1387499520 + test: 22199992320 + test4: 22199992320 + leveldb_limits: + large: + contract_storage: 31967988940 + db_storage: 10655996313 + medium: + contract_storage: 3995998617 + db_storage: 1331999539 + small: + contract_storage: 249749913 + db_storage: 83249971 + test: + contract_storage: 3995998617 + db_storage: 1331999539 + test4: + contract_storage: 3995998617 + db_storage: 1331999539 + shared_space: 22399942656 + volume_limits: + large: + max_consensus_storage_bytes: 53279981568 + max_file_storage_bytes: 53279981568 + max_reserved_storage_bytes: 17759993856 + max_skaled_leveldb_storage_bytes: 53279981568 + medium: + max_consensus_storage_bytes: 6659997696 + max_file_storage_bytes: 6659997696 + max_reserved_storage_bytes: 2219999232 + max_skaled_leveldb_storage_bytes: 6659997696 + small: + max_consensus_storage_bytes: 416249856 + max_file_storage_bytes: 416249856 + max_reserved_storage_bytes: 138749952 + max_skaled_leveldb_storage_bytes: 416249856 + test: + max_consensus_storage_bytes: 6659997696 + max_file_storage_bytes: 6659997696 + max_reserved_storage_bytes: 2219999232 + max_skaled_leveldb_storage_bytes: 6659997696 + test4: + max_consensus_storage_bytes: 6659997696 + max_file_storage_bytes: 6659997696 + max_reserved_storage_bytes: 2219999232 + max_skaled_leveldb_storage_bytes: 6659997696 +testnet: + disk: + large: 177599938560 + medium: 22199992320 + small: 1387499520 + test: 22199992320 + test4: 22199992320 + leveldb_limits: + large: + contract_storage: 31967988940 + db_storage: 10655996313 + medium: + contract_storage: 3995998617 + db_storage: 1331999539 + small: + contract_storage: 249749913 + db_storage: 83249971 + test: + contract_storage: 3995998617 + db_storage: 1331999539 + test4: + contract_storage: 3995998617 + db_storage: 1331999539 + shared_space: 22399942656 + volume_limits: + large: + max_consensus_storage_bytes: 53279981568 + max_file_storage_bytes: 53279981568 + max_reserved_storage_bytes: 17759993856 + max_skaled_leveldb_storage_bytes: 53279981568 + medium: + max_consensus_storage_bytes: 6659997696 + max_file_storage_bytes: 6659997696 + max_reserved_storage_bytes: 2219999232 + max_skaled_leveldb_storage_bytes: 6659997696 + small: + max_consensus_storage_bytes: 416249856 + max_file_storage_bytes: 416249856 + max_reserved_storage_bytes: 138749952 + max_skaled_leveldb_storage_bytes: 416249856 + test: + max_consensus_storage_bytes: 6659997696 + max_file_storage_bytes: 6659997696 + max_reserved_storage_bytes: 2219999232 + max_skaled_leveldb_storage_bytes: 6659997696 + test4: + max_consensus_storage_bytes: 6659997696 + max_file_storage_bytes: 6659997696 + max_reserved_storage_bytes: 2219999232 + max_skaled_leveldb_storage_bytes: 6659997696 diff --git a/tests/.skale/config/static_params.yaml b/tests/.skale/config/static_params.yaml new file mode 100644 index 00000000..a0ea143f --- /dev/null +++ b/tests/.skale/config/static_params.yaml @@ -0,0 +1,306 @@ +common: + schain: + cpu: + skaled: 0.8 + ima: 0.2 + mem: + skaled: 0.8 + ima: 0.2 + volume_limits: + max_consensus_storage_bytes: 0.3 + max_skaled_leveldb_storage_bytes: 0.3 + max_file_storage_bytes: 0.3 + max_reserved_storage_bytes: 0.1 + leveldb_limits: + contract_storage: 0.6 + db_storage: 0.2 # leveldb may use x2 storage, so 0.4 divided by 2, actually using 0.4 + shared_space_coefficient: 1 +envs: + mainnet: + server: + cpu_total: 8 + cpu_physical: 1 + memory: 32000000000 + swap: 16000000000 + disk: 1900000000000 + + package: + iptables-persistent: 1.0.4 + lvm2: 2.02.0 + btrfs-progs: 4.15.1 + lsof: "4.89" + psmisc: 23.1-1 + + docker: + docker-api: 1.41.0 + docker-engine: 20.10.7 + docker-compose: 1.27.4 + + schain: + revertableFSPatchTimestamp: 1000000 + contractStoragePatchTimestamp: 1000000 + snapshotIntervalSec: 86400 + emptyBlockIntervalMs: 10000 + snapshotDownloadTimeout: 18000 + snapshotDownloadInactiveTimeout: 120 + + schain_cmd: + ["-v 3", "--web3-trace", "--enable-debug-behavior-apis", "--aa no"] + + node: + common: + bindIP: "0.0.0.0" + logLevel: "info" + logLevelConfig: "info" + small: + minCacheSize: 1000000 + maxCacheSize: 2000000 + collectionQueueSize: 2 + collectionDuration: 10 + transactionQueueSize: 1000 + maxOpenLeveldbFiles: 25 + medium: + minCacheSize: 8000000 + maxCacheSize: 16000000 + collectionQueueSize: 20 + collectionDuration: 60 + transactionQueueSize: 1000 + maxOpenLeveldbFiles: 1000 + large: + minCacheSize: 8000000 + maxCacheSize: 16000000 + collectionQueueSize: 20 + collectionDuration: 60 + transactionQueueSize: 1000 + maxOpenLeveldbFiles: 1000 + test: + minCacheSize: 8000000 + maxCacheSize: 16000000 + collectionQueueSize: 20 + collectionDuration: 60 + transactionQueueSize: 1000 + maxOpenLeveldbFiles: 1000 + test4: + minCacheSize: 8000000 + maxCacheSize: 16000000 + collectionQueueSize: 20 + collectionDuration: 60 + transactionQueueSize: 1000 + maxOpenLeveldbFiles: 1000 + + testnet: + server: + cpu_total: 8 + cpu_physical: 1 + memory: 32000000000 + swap: 16000000000 + disk: 200000000000 + + package: + iptables-persistent: 1.0.4 + lvm2: 2.02.0 + btrfs-progs: 4.15.1 + lsof: "4.89" + psmisc: 23.1-1 + + docker: + docker-api: 1.41.0 + docker-engine: 20.10.7 + docker-compose: 1.27.4 + + schain: + revertableFSPatchTimestamp: 1000000 + contractStoragePatchTimestamp: 1000000 + snapshotIntervalSec: 86400 + emptyBlockIntervalMs: 10000 + snapshotDownloadTimeout: 18000 + snapshotDownloadInactiveTimeout: 120 + + schain_cmd: + ["-v 3", "--web3-trace", "--enable-debug-behavior-apis", "--aa no"] + + node: + common: + bindIP: "0.0.0.0" + logLevel: "info" + logLevelConfig: "info" + small: + minCacheSize: 1000000 + maxCacheSize: 2000000 + collectionQueueSize: 2 + collectionDuration: 10 + transactionQueueSize: 1000 + maxOpenLeveldbFiles: 25 + medium: + minCacheSize: 8000000 + maxCacheSize: 16000000 + collectionQueueSize: 20 + collectionDuration: 60 + transactionQueueSize: 1000 + maxOpenLeveldbFiles: 1000 + large: + minCacheSize: 8000000 + maxCacheSize: 16000000 + collectionQueueSize: 20 + collectionDuration: 60 + transactionQueueSize: 1000 + maxOpenLeveldbFiles: 1000 + test: + minCacheSize: 8000000 + maxCacheSize: 16000000 + collectionQueueSize: 20 + collectionDuration: 60 + transactionQueueSize: 1000 + maxOpenLeveldbFiles: 1000 + test4: + minCacheSize: 8000000 + maxCacheSize: 16000000 + collectionQueueSize: 20 + collectionDuration: 60 + transactionQueueSize: 1000 + maxOpenLeveldbFiles: 1000 + + qanet: + server: + cpu_total: 8 + cpu_physical: 1 + memory: 32000000000 + swap: 16000000000 + disk: 200000000000 + + package: + iptables-persistent: 1.0.4 + lvm2: 2.02.0 + btrfs-progs: 4.15.1 + lsof: "4.89" + psmisc: 23.1-1 + + docker: + docker-api: 1.41.0 + docker-engine: 20.10.7 + docker-compose: 1.27.4 + + schain: + revertableFSPatchTimestamp: 1000000 + contractStoragePatchTimestamp: 1000000 + snapshotIntervalSec: 86400 + emptyBlockIntervalMs: 10000 + snapshotDownloadTimeout: 18000 + snapshotDownloadInactiveTimeout: 120 + + schain_cmd: + ["-v 3", "--web3-trace", "--enable-debug-behavior-apis", "--aa no"] + + node: + common: + bindIP: "0.0.0.0" + logLevel: "info" + logLevelConfig: "info" + small: + minCacheSize: 1000000 + maxCacheSize: 2000000 + collectionQueueSize: 2 + collectionDuration: 10 + transactionQueueSize: 1000 + maxOpenLeveldbFiles: 25 + medium: + minCacheSize: 8000000 + maxCacheSize: 16000000 + collectionQueueSize: 20 + collectionDuration: 60 + transactionQueueSize: 1000 + maxOpenLeveldbFiles: 1000 + large: + minCacheSize: 8000000 + maxCacheSize: 16000000 + collectionQueueSize: 20 + collectionDuration: 60 + transactionQueueSize: 1000 + maxOpenLeveldbFiles: 1000 + test: + minCacheSize: 8000000 + maxCacheSize: 16000000 + collectionQueueSize: 20 + collectionDuration: 60 + transactionQueueSize: 1000 + maxOpenLeveldbFiles: 1000 + test4: + minCacheSize: 8000000 + maxCacheSize: 16000000 + collectionQueueSize: 20 + collectionDuration: 60 + transactionQueueSize: 1000 + maxOpenLeveldbFiles: 1000 + + + devnet: + server: + cpu_total: 1 + cpu_physical: 1 + memory: 2000000000 + swap: 2000000000 + disk: 80000000000 + + package: + iptables-persistent: 1.0.4 + lvm2: 2.02.0 + btrfs-progs: 4.15.1 + lsof: "4.89" + psmisc: 23.1-1 + + docker: + docker-api: 1.41.0 + docker-engine: 20.10.7 + docker-compose: 1.27.4 + + schain: + revertableFSPatchTimestamp: 1000000 + contractStoragePatchTimestamp: 1000000 + snapshotIntervalSec: 86400 + emptyBlockIntervalMs: 10000 + snapshotDownloadTimeout: 18000 + snapshotDownloadInactiveTimeout: 120 + + schain_cmd: + ["-v 3", "--web3-trace", "--enable-debug-behavior-apis", "--aa no"] + + node: + common: + bindIP: "0.0.0.0" + logLevel: "info" + logLevelConfig: "info" + small: + minCacheSize: 1000000 + maxCacheSize: 2000000 + collectionQueueSize: 2 + collectionDuration: 10 + transactionQueueSize: 1000 + maxOpenLeveldbFiles: 25 + medium: + minCacheSize: 8000000 + maxCacheSize: 16000000 + collectionQueueSize: 20 + collectionDuration: 60 + transactionQueueSize: 1000 + maxOpenLeveldbFiles: 1000 + large: + minCacheSize: 8000000 + maxCacheSize: 16000000 + collectionQueueSize: 20 + collectionDuration: 60 + transactionQueueSize: 1000 + maxOpenLeveldbFiles: 1000 + test: + minCacheSize: 8000000 + maxCacheSize: 16000000 + collectionQueueSize: 20 + collectionDuration: 60 + transactionQueueSize: 1000 + maxOpenLeveldbFiles: 1000 + test4: + minCacheSize: 8000000 + maxCacheSize: 16000000 + collectionQueueSize: 20 + collectionDuration: 60 + transactionQueueSize: 1000 + maxOpenLeveldbFiles: 1000 From 7baaa6d6d5a710245612e81cc07859b38eb28dbf Mon Sep 17 00:00:00 2001 From: badrogger Date: Mon, 14 Sep 2026 15:55:50 +0100 Subject: [PATCH 16/16] Remove redundant env option --- tests/core/monitoring_firewall_test.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/core/monitoring_firewall_test.py b/tests/core/monitoring_firewall_test.py index 2a17cd17..d85f56b2 100644 --- a/tests/core/monitoring_firewall_test.py +++ b/tests/core/monitoring_firewall_test.py @@ -11,7 +11,6 @@ def monitoring_firewall(monkeypatch, tmp_path): """Run only in the isolated nftables test container (requires NET_ADMIN).""" monkeypatch.setenv('SSH_PORT', '22') - monkeypatch.setenv('MONITORING_CONTAINERS', 'True') monkeypatch.delenv('SCHAIN_BASE_PORT', raising=False) monkeypatch.delenv('FIREWALL_DEFAULT_DROP', raising=False) monkeypatch.setattr(firewall, 'NODE_CONFIG_PATH', str(tmp_path / 'node.json'))