From 7f2b4b00aa7a20feba1555443cd10b2be824641d Mon Sep 17 00:00:00 2001 From: Ilia Ross Date: Sat, 2 May 2026 17:08:35 +0200 Subject: [PATCH] Fix to scope direct-mode changes to selected tables MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix nftables direct-mode operations so create, edit, delete, and move actions apply only the selected table instead of rewriting or applying the full ruleset. This avoids copying firewalld-owned rules, or any other externally managed rules, into Webmin’s save file and prevents operations from failing against externally managed tables. Also remove previously added unsafe full-ruleset flush action and keep Apply Configuration out of direct mode (will be further reworked). --- nftables/apply.cgi | 5 +- nftables/create_table.cgi | 17 ++--- nftables/delete_chain.cgi | 2 +- nftables/delete_chains.cgi | 56 +++++++++++++++++ nftables/delete_set.cgi | 6 +- nftables/delete_sets.cgi | 53 ++++++++++++++++ nftables/delete_table.cgi | 29 +++++++-- nftables/flush.cgi | 25 -------- nftables/index.cgi | 21 +++++-- nftables/lang/en | 17 ++--- nftables/move_rule.cgi | 2 +- nftables/nftables-lib.pl | 125 ++++++++++++++++++++++++++++++++----- nftables/save_chain.cgi | 4 +- nftables/save_rule.cgi | 5 +- nftables/save_set.cgi | 4 +- nftables/setup.cgi | 8 ++- 16 files changed, 298 insertions(+), 81 deletions(-) create mode 100755 nftables/delete_chains.cgi create mode 100755 nftables/delete_sets.cgi delete mode 100755 nftables/flush.cgi diff --git a/nftables/apply.cgi b/nftables/apply.cgi index be53676f3..f00267123 100755 --- a/nftables/apply.cgi +++ b/nftables/apply.cgi @@ -5,11 +5,12 @@ require './nftables-lib.pl'; ## no critic use strict; use warnings; -our (%in, %text); +our (%config, %in, %text); ReadParse(); error_setup($text{'apply_err'}); -my @tables = get_nftables_save(); +redirect("index.cgi") if ($config{'direct'}); + my $err = apply_restore(); error($err) if ($err); diff --git a/nftables/create_table.cgi b/nftables/create_table.cgi index 8b90fca18..4c5718354 100755 --- a/nftables/create_table.cgi +++ b/nftables/create_table.cgi @@ -26,17 +26,18 @@ if ($in{'create'}) { } } - push(@tables, { 'name' => $name, - 'family' => $family, - 'rules' => [], - 'chains' => {}, - 'sets' => {} }); - my $err = save_configuration(@tables); + my $table = { 'name' => $name, + 'family' => $family, + 'rules' => [], + 'chains' => {}, + 'sets' => {} }; + push(@tables, $table); + my $err = create_table_configuration($table, @tables); error(text('create_failed', $err)) if ($err); webmin_log("create", "table", $name, { 'family' => $family }); - my $idx = $#tables; - redirect("index.cgi?table=$idx"); + redirect("index.cgi?table_family=".urlize($family). + "&table_name=".urlize($name)); } ui_print_header(undef, $text{'create_title'}, "", "intro", 1, 1); diff --git a/nftables/delete_chain.cgi b/nftables/delete_chain.cgi index e609240ee..8f38730ec 100644 --- a/nftables/delete_chain.cgi +++ b/nftables/delete_chain.cgi @@ -27,7 +27,7 @@ if ($in{'confirm'}) { @{$table->{'rules'}} = grep { $_->{'chain'} ne $in{'chain'} } @{$table->{'rules'}}; delete($table->{'chains'}->{$in{'chain'}}); - my $err = save_configuration(@tables); + my $err = save_table_configuration($table, @tables); error(text('delete_chain_failed', $err)) if ($err); webmin_log("delete", "chain", $in{'chain'}, { 'table' => $table->{'name'}, 'family' => $table->{'family'} }); diff --git a/nftables/delete_chains.cgi b/nftables/delete_chains.cgi new file mode 100755 index 000000000..291d054e0 --- /dev/null +++ b/nftables/delete_chains.cgi @@ -0,0 +1,56 @@ +#!/usr/bin/perl +# delete_chains.cgi +# Delete selected nftables chains + +require './nftables-lib.pl'; ## no critic +use strict; +use warnings; +our (%in, %text); +ReadParse(); +error_setup($text{'delete_chains_err'}); + +my @tables = get_nftables_save(); +my $table_idx = $in{'table'}; +my $table; +if (defined($in{'table_family'}) && defined($in{'table_name'})) { + for (my $i = 0; $i <= $#tables; $i++) { + if ($tables[$i]->{'family'} eq $in{'table_family'} && + $tables[$i]->{'name'} eq $in{'table_name'}) { + $table_idx = $i; + $table = $tables[$i]; + last; + } + } +} +$table ||= $tables[$table_idx]; +$table || error($text{'chain_notable'}); + +my @chains = split(/\0/, $in{'d'} || ""); +my %seen; +@chains = grep { defined($_) && $_ ne '' && !$seen{$_}++ } @chains; +@chains || error($text{'delete_chains_enone'}); + +foreach my $c (@chains) { + $table->{'chains'}->{$c} || error(text('delete_chains_nochain', $c)); +} + +my %delete = map { $_ => 1 } @chains; +my @refs = grep { + !$delete{$_->{'chain'}} && + (($_->{'jump'} && $delete{$_->{'jump'}}) || + ($_->{'goto'} && $delete{$_->{'goto'}})) +} @{$table->{'rules'}}; +@refs && error(text('delete_chains_inuse', scalar(@refs))); + +@{$table->{'rules'}} = grep { !$delete{$_->{'chain'}} } @{$table->{'rules'}}; +foreach my $c (@chains) { + delete($table->{'chains'}->{$c}); +} + +my $err = save_table_configuration($table, @tables); +error(text('delete_chains_failed', $err)) if ($err); +webmin_log("delete", "chains", scalar(@chains), + { 'table' => $table->{'name'}, + 'family' => $table->{'family'} }); +redirect("index.cgi?table_family=".urlize($table->{'family'}). + "&table_name=".urlize($table->{'name'})); diff --git a/nftables/delete_set.cgi b/nftables/delete_set.cgi index 82747b457..24c97759f 100755 --- a/nftables/delete_set.cgi +++ b/nftables/delete_set.cgi @@ -22,11 +22,11 @@ if ($in{'confirm'}) { $refs && error(text('delete_set_inuse', $in{'set'}, $refs)); delete($table->{'sets'}->{$in{'set'}}); - my $err = save_configuration(@tables); + my $err = save_table_configuration($table, @tables); error(text('delete_set_failed', $err)) if ($err); webmin_log("delete", "set", $in{'set'}, { 'table' => $table->{'name'}, 'family' => $table->{'family'} }); - redirect("index.cgi?table=$in{'table'}"); + redirect("index.cgi?table=$in{'table'}&view=sets"); } ui_print_header(undef, $text{'delete_set_title'}, "", "intro", 1, 1); @@ -45,4 +45,4 @@ print "

\n"; print ui_submit($text{'delete'}, "confirm"); print "\n"; print ui_form_end(); -ui_print_footer("index.cgi?table=$in{'table'}", $text{'index_return'}); +ui_print_footer("index.cgi?table=$in{'table'}&view=sets", $text{'index_return'}); diff --git a/nftables/delete_sets.cgi b/nftables/delete_sets.cgi new file mode 100755 index 000000000..e7d2ec187 --- /dev/null +++ b/nftables/delete_sets.cgi @@ -0,0 +1,53 @@ +#!/usr/bin/perl +# delete_sets.cgi +# Delete selected nftables sets + +require './nftables-lib.pl'; ## no critic +use strict; +use warnings; +our (%in, %text); +ReadParse(); +error_setup($text{'delete_sets_err'}); + +my @tables = get_nftables_save(); +my $table_idx = $in{'table'}; +my $table; +if (defined($in{'table_family'}) && defined($in{'table_name'})) { + for (my $i = 0; $i <= $#tables; $i++) { + if ($tables[$i]->{'family'} eq $in{'table_family'} && + $tables[$i]->{'name'} eq $in{'table_name'}) { + $table_idx = $i; + $table = $tables[$i]; + last; + } + } +} +$table ||= $tables[$table_idx]; +$table || error($text{'set_notable'}); + +my @sets = split(/\0/, $in{'s'} || ""); +my %seen; +@sets = grep { defined($_) && $_ ne '' && !$seen{$_}++ } @sets; +@sets || error($text{'delete_sets_enone'}); + +foreach my $s (@sets) { + $table->{'sets'}->{$s} || error(text('delete_sets_noset', $s)); +} + +my $refs = 0; +foreach my $s (@sets) { + $refs += count_set_references($table, $s); +} +$refs && error(text('delete_sets_inuse', $refs)); + +foreach my $s (@sets) { + delete($table->{'sets'}->{$s}); +} + +my $err = save_table_configuration($table, @tables); +error(text('delete_sets_failed', $err)) if ($err); +webmin_log("delete", "sets", scalar(@sets), + { 'table' => $table->{'name'}, + 'family' => $table->{'family'} }); +redirect("index.cgi?table_family=".urlize($table->{'family'}). + "&table_name=".urlize($table->{'name'})."&view=sets"); diff --git a/nftables/delete_table.cgi b/nftables/delete_table.cgi index 0621e0d50..0159b7aa8 100755 --- a/nftables/delete_table.cgi +++ b/nftables/delete_table.cgi @@ -10,12 +10,27 @@ ReadParse(); error_setup($text{'delete_err'}); my @tables = get_nftables_save(); -my $table = $tables[$in{'table'}]; +my $table_idx = $in{'table'}; +my $table; +if (defined($in{'table_family'}) && defined($in{'table_name'})) { + for (my $i = 0; $i <= $#tables; $i++) { + if ($tables[$i]->{'family'} eq $in{'table_family'} && + $tables[$i]->{'name'} eq $in{'table_name'}) { + $table_idx = $i; + $table = $tables[$i]; + last; + } + } + $table || error($text{'delete_notable'}); +} +else { + $table = $tables[$table_idx]; +} $table || error($text{'delete_notable'}); if ($in{'confirm'}) { - splice(@tables, $in{'table'}, 1); - my $err = save_configuration(@tables); + splice(@tables, $table_idx, 1); + my $err = delete_table_configuration($table, @tables); error(text('delete_failed', $err)) if ($err); webmin_log("delete", "table", $table->{'name'}, { 'family' => $table->{'family'} }); @@ -24,7 +39,9 @@ if ($in{'confirm'}) { ui_print_header(undef, $text{'delete_title'}, "", "intro", 1, 1); print ui_form_start("delete_table.cgi"); -print ui_hidden("table", $in{'table'}); +print ui_hidden("table", $table_idx); +print ui_hidden("table_family", $table->{'family'}); +print ui_hidden("table_name", $table->{'name'}); print "

", text('delete_confirm', "$table->{'family'} $table->{'name'}"), @@ -32,5 +49,5 @@ print "
", print ui_submit($text{'delete'}, "confirm"); print "
\n"; print ui_form_end(); -ui_print_footer("index.cgi?table=$in{'table'}", $text{'index_return'}); - +ui_print_footer("index.cgi?table_family=".urlize($table->{'family'}). + "&table_name=".urlize($table->{'name'}), $text{'index_return'}); diff --git a/nftables/flush.cgi b/nftables/flush.cgi deleted file mode 100755 index 8551a6854..000000000 --- a/nftables/flush.cgi +++ /dev/null @@ -1,25 +0,0 @@ -#!/usr/bin/perl -# flush.cgi -# Flush the active nftables ruleset - -require './nftables-lib.pl'; ## no critic -use strict; -use warnings; -our (%in, %text); -ReadParse(); -error_setup($text{'flush_err'}); - -if ($in{'confirm'}) { - my $err = flush_ruleset(); - error(text('flush_failed', $err)) if ($err); - webmin_log("flush", "ruleset"); - redirect("index.cgi"); -} - -ui_print_header(undef, $text{'flush_title'}, "", "intro", 1, 1); -print ui_form_start("flush.cgi"); -print "
$text{'flush_confirm'}

\n"; -print ui_submit($text{'flush_ok'}, "confirm"); -print "

\n"; -print ui_form_end(); -ui_print_footer("index.cgi", $text{'index_return'}); diff --git a/nftables/index.cgi b/nftables/index.cgi index a3f4c417f..9e814c12c 100755 --- a/nftables/index.cgi +++ b/nftables/index.cgi @@ -46,9 +46,21 @@ if (!@tables) { $rules_html .= ui_buttons_end(); } else { # Select table - if (!defined($in{'table'}) || $in{'table'} !~ /^\d+$/ || - $in{'table'} > $#tables) { - $in{'table'} = 0; + my $found_table; + if (defined($in{'table_family'}) && defined($in{'table_name'})) { + for (my $i = 0; $i <= $#tables; $i++) { + if ($tables[$i]->{'family'} eq $in{'table_family'} && + $tables[$i]->{'name'} eq $in{'table_name'}) { + $in{'table'} = $i; + $found_table = 1; + last; + } + } + } + if (!$found_table && + (!defined($in{'table'}) || $in{'table'} !~ /^\d+$/ || + $in{'table'} > $#tables)) { + $in{'table'} = 0; } my @table_opts; for (my $i = 0; $i <= $#tables; $i++) { @@ -194,13 +206,12 @@ print "
\n"; print $rules_html; print "
\n"; -if (@tables) { +if (@tables && !$config{'direct'}) { print ui_hr(); print ui_buttons_start(); print ui_buttons_row("create_table.cgi", $text{'index_table_create'}, $text{'index_table_createdesc'}); print ui_buttons_row("apply.cgi", $text{'index_apply'}, $text{'index_applydesc'}); - print ui_buttons_row("flush.cgi", $text{'index_flush'}, $text{'index_flushdesc'}); print ui_buttons_end(); } diff --git a/nftables/lang/en b/nftables/lang/en index 074575742..cee3fba0a 100644 --- a/nftables/lang/en +++ b/nftables/lang/en @@ -50,8 +50,6 @@ index_cmovesel=Move Selected index_radd=Add Rule index_apply=Apply Configuration index_applydesc=Click this button to load the saved firewall configuration into the active nftables ruleset. -index_flush=Flush Active Ruleset -index_flushdesc=Click this button to remove all active nftables tables, chains, sets and rules without changing the saved configuration. index_unapply=Revert Configuration index_unapplydesc=Click this button to reset the configuration listed above to the one that is currently active. index_bootup=Activate at Boot @@ -65,11 +63,6 @@ save=Save delete=Delete save_err=Failed to save rule apply_err=Failed to apply configuration -flush_title=Flush active ruleset -flush_err=Failed to flush active ruleset -flush_failed=Failed to flush active ruleset: $1 -flush_confirm=Are you sure you want to remove all active nftables tables, chains, sets and rules? -flush_ok=Flush Ruleset setup_title=Setup Default Ruleset setup_header=Create Default Ruleset setup_desc=This page allows you to create a default nftables ruleset. Select one of the options below and click 'Create'. @@ -175,6 +168,11 @@ delete_chain_err=Failed to delete chain delete_chain_failed=Failed to delete chain: 
$1
delete_chain_confirm=Are you sure you want to delete chain $1 from table $2? delete_chain_inuse=Chain $1 is referenced by $2 rule(s) via jump/goto. Remove those rules first. +delete_chains_err=Failed to delete selected chains +delete_chains_failed=Failed to delete selected chains: 
$1
+delete_chains_enone=No chains selected +delete_chains_nochain=No such chain selected: $1 +delete_chains_inuse=The selected chains are referenced by $1 rule(s) in chains that are not being deleted. Remove those rules first. rename_chain_title=Rename chain rename_chain_header=Rename chain rename_chain_old=Current name @@ -227,3 +225,8 @@ delete_set_err=Failed to delete set delete_set_failed=Failed to delete set: 
$1
delete_set_confirm=Are you sure you want to delete set $1 from table $2? delete_set_inuse=Set $1 is referenced by $2 rule(s). Remove those rules first. +delete_sets_err=Failed to delete selected sets +delete_sets_failed=Failed to delete selected sets: 
$1
+delete_sets_enone=No sets selected +delete_sets_noset=No such set selected: $1 +delete_sets_inuse=The selected sets are referenced by $1 rule(s). Remove those rules first. diff --git a/nftables/move_rule.cgi b/nftables/move_rule.cgi index 456bc26de..a29ee766a 100755 --- a/nftables/move_rule.cgi +++ b/nftables/move_rule.cgi @@ -28,7 +28,7 @@ if (!defined($rv)) { } if ($rv) { - my $err = save_configuration(@tables); + my $err = save_table_configuration($table, @tables); error(text('move_failed', $err)) if ($err); webmin_log("move", "rule", undef, { 'table' => $table->{'name'}, diff --git a/nftables/nftables-lib.pl b/nftables/nftables-lib.pl index 1f5edf878..4e5475636 100644 --- a/nftables/nftables-lib.pl +++ b/nftables/nftables-lib.pl @@ -997,6 +997,20 @@ foreach my $t (@tables) { return $rv; } +# write_configuration(@tables) +# Writes the configuration to the save file +sub write_configuration +{ +my (@tables) = @_; +my $out = dump_nftables_save(@tables); +my $file = $config{'save_file'} || "$module_config_directory/nftables.conf"; + +open_tempfile(my $fh, ">$file"); +print_tempfile($fh, $out); +close_tempfile($fh); +return; +} + # save_table(&table) # Saves a single table to the save file or applies it sub save_table @@ -1010,21 +1024,57 @@ my ($table) = @_; } # save_configuration(@tables) -# Writes the configuration to the save file. If direct mode is on, applies it. +# Writes the configuration to the save file. If direct mode is on, applies it +# without creating a persistent save file. sub save_configuration { my (@tables) = @_; -my $out = dump_nftables_save(@tables); -my $file = $config{'save_file'} || "$module_config_directory/nftables.conf"; - -# Write to file -open_tempfile(my $fh, ">$file"); -print_tempfile($fh, $out); -close_tempfile($fh); - if ($config{'direct'}) { - return apply_restore($file); + my $tmp = tempname(); + open_tempfile(my $fh, ">$tmp"); + print_tempfile($fh, dump_nftables_save(@tables)); + close_tempfile($fh); + my $err = apply_restore($tmp); + unlink_file($tmp); + return $err; } +write_configuration(@tables); +return; +} + +# create_table_configuration(&table, @tables) +# Writes the full configuration, but in direct mode creates only the selected table +sub create_table_configuration +{ +my ($table, @tables) = @_; +if ($config{'direct'}) { + return apply_table_restore($table, 0); +} +write_configuration(@tables); +return; +} + +# save_table_configuration(&table, @tables) +# Writes the full configuration, but in direct mode replaces only the selected table +sub save_table_configuration +{ +my ($table, @tables) = @_; +if ($config{'direct'}) { + return apply_table_restore($table, 1); +} +write_configuration(@tables); +return; +} + +# delete_table_configuration(&table, @tables) +# Writes the full configuration, but in direct mode deletes only the selected table +sub delete_table_configuration +{ +my ($table, @tables) = @_; +if ($config{'direct'}) { + return apply_table_delete($table); +} +write_configuration(@tables); return; } @@ -1043,13 +1093,60 @@ if ($?) { return; } -# flush_ruleset() -# Flushes all active nftables tables, chains, sets and rules -sub flush_ruleset +# nft_table_spec(&table) +# Returns a table spec for nft commands +sub nft_table_spec { +my ($table) = @_; +return $table->{'family'} ? "$table->{'family'} $table->{'name'}" : + $table->{'name'}; +} + +# apply_table_restore(&table, [replace-existing]) +# Applies a single table without touching unrelated tables +sub apply_table_restore +{ +my ($table, $replace) = @_; my $cmd = get_nft_command(); return text('index_ecommand', "nft") if (!$cmd); -my $out = backquote_logged("$cmd flush ruleset 2>&1"); + +my $spec = nft_table_spec($table); +my $tmp = tempname(); +open_tempfile(my $fh, ">$tmp"); +print_tempfile($fh, "delete table $spec\n") if ($replace); +print_tempfile($fh, dump_nftables_save($table)); +close_tempfile($fh); + +my $out = backquote_logged("$cmd -c -f $tmp 2>&1"); +if (!$?) { + $out = backquote_logged("$cmd -f $tmp 2>&1"); +} +unlink_file($tmp); +if ($?) { + return "
$out
"; +} +return; +} + +# apply_table_delete(&table) +# Deletes a single active table without touching unrelated tables +sub apply_table_delete +{ +my ($table) = @_; +my $cmd = get_nft_command(); +return text('index_ecommand', "nft") if (!$cmd); + +my $spec = nft_table_spec($table); +my $tmp = tempname(); +open_tempfile(my $fh, ">$tmp"); +print_tempfile($fh, "delete table $spec\n"); +close_tempfile($fh); + +my $out = backquote_logged("$cmd -c -f $tmp 2>&1"); +if (!$?) { + $out = backquote_logged("$cmd -f $tmp 2>&1"); +} +unlink_file($tmp); if ($?) { return "
$out
"; } diff --git a/nftables/save_chain.cgi b/nftables/save_chain.cgi index d5808f074..4be511e8f 100644 --- a/nftables/save_chain.cgi +++ b/nftables/save_chain.cgi @@ -58,7 +58,7 @@ if ($is_rename) { } } - my $err = save_configuration(@tables); + my $err = save_table_configuration($table, @tables); error(text('rename_chain_failed', $err)) if ($err); webmin_log("rename", "chain", $old, { 'new' => $name, @@ -91,7 +91,7 @@ $chain->{'priority'} = $priority; $chain->{'policy'} = $policy; $table->{'chains'}->{$name} = $chain; -my $err = save_configuration(@tables); +my $err = save_table_configuration($table, @tables); error(text('chain_failed', $err)) if ($err); webmin_log($is_new ? "create" : "modify", "chain", $name, diff --git a/nftables/save_rule.cgi b/nftables/save_rule.cgi index 2464e3dbc..e15ce745e 100755 --- a/nftables/save_rule.cgi +++ b/nftables/save_rule.cgi @@ -165,7 +165,8 @@ if ($in{'delete'}) { if ($cmd) { my $tmp = tempname(); open_tempfile(my $fh, ">$tmp"); - print_tempfile($fh, dump_nftables_save(@tables)); + print_tempfile($fh, dump_nftables_save( + $config{'direct'} ? ($table) : @tables)); close_tempfile($fh); my $out = backquote_logged("$cmd -c -f $tmp 2>&1"); unlink_file($tmp); @@ -175,6 +176,6 @@ if ($in{'delete'}) { webmin_log("save", $in{'new'} ? "create" : "modify", $rule->{'text'}); } -my $err = save_configuration(@tables); +my $err = save_table_configuration($table, @tables); error(text('save_failed', $err)) if ($err); redirect("index.cgi?table=$in{'table'}"); diff --git a/nftables/save_set.cgi b/nftables/save_set.cgi index f12556229..fe3649988 100755 --- a/nftables/save_set.cgi +++ b/nftables/save_set.cgi @@ -54,9 +54,9 @@ $set->{'raw_lines'} ||= [ ]; $table->{'sets'}->{$name} = $set; -my $err = save_configuration(@tables); +my $err = save_table_configuration($table, @tables); error(text('set_failed', $err)) if ($err); webmin_log($is_new ? "create" : "save", "set", $name, { 'table' => $table->{'name'}, 'family' => $table->{'family'} }); -redirect("index.cgi?table=$in{'table'}"); +redirect("index.cgi?table=$in{'table'}&view=sets"); diff --git a/nftables/setup.cgi b/nftables/setup.cgi index 206a826b1..f0f6d09c0 100644 --- a/nftables/setup.cgi +++ b/nftables/setup.cgi @@ -27,9 +27,11 @@ if ($in{'action'} eq 'create') { if ($error) { error(text('setup_failed', $error)); } - $error = apply_restore(); - if ($error) { - error(text('setup_failed', $error)); + if (!$config{'direct'}) { + $error = apply_restore(); + if ($error) { + error(text('setup_failed', $error)); + } } webmin_log("setup", "create", $type); redirect("index.cgi");