From 2fe57dd456121c7a0f35f27f242d55a9dba7fc89 Mon Sep 17 00:00:00 2001 From: Ilia Ross Date: Sat, 2 May 2026 22:16:13 +0200 Subject: [PATCH] Fix to validate nftables set usage in rules * Note: Prevent incompatible nftables sets from being used in rule fields. The rule editor now only offers address sets for address matches and port/service sets for port matches, while save and apply paths validate existing set references before writing or loading rules. This avoids nft datatype mismatch errors such as using inet_proto sets with tcp dport. --- nftables/edit_rule.cgi | 5 +++-- nftables/help/dport.html | 2 +- nftables/help/sport.html | 2 +- nftables/lang/en | 7 +++++-- nftables/nftables-lib.pl | 35 +++++++++++++++++++++++++++++++++++ nftables/save_rule.cgi | 15 +++++++++++++++ nftables/save_set.cgi | 3 +++ 7 files changed, 63 insertions(+), 6 deletions(-) diff --git a/nftables/edit_rule.cgi b/nftables/edit_rule.cgi index 5fb0688a7..f531a2964 100755 --- a/nftables/edit_rule.cgi +++ b/nftables/edit_rule.cgi @@ -104,11 +104,11 @@ if ($table && $table->{'sets'} && ref($table->{'sets'}) eq 'HASH') { my $label = $s; $label .= " ($set->{'type'})" if ($set->{'type'}); my $kind = set_type_kind($set->{'type'}); - if (!$kind || $kind eq 'addr') { + if ($kind && $kind eq 'addr') { push(@addr_set_opts, [ $s, $label ]); $addr_set_seen{$s} = 1; } - if (!$kind || $kind eq 'port') { + if ($kind && $kind eq 'port') { push(@port_set_opts, [ $s, $label ]); $port_set_seen{$s} = 1; } @@ -234,6 +234,7 @@ if (@port_set_opts > 1) { ui_select("dport_set", $dport_set, \@port_set_opts, 1, 0, 1)); } print ui_table_row(hlink($text{'edit_dport'}, "dport"), $dport_row); +print ui_table_row(undef, ui_note($text{'edit_ports_note'}, 0), 2); print ui_table_end(); diff --git a/nftables/help/dport.html b/nftables/help/dport.html index 45089e007..7bbbd3a3e 100644 --- a/nftables/help/dport.html +++ b/nftables/help/dport.html @@ -1,3 +1,3 @@
Destination port
-

TCP/UDP destination port number or range (e.g., 80 or 1000-2000).

+

TCP/UDP destination port number or range on the local service being reached (e.g., 22, 80 or 1000-2000).

diff --git a/nftables/help/sport.html b/nftables/help/sport.html index ee9c8730e..01d842532 100644 --- a/nftables/help/sport.html +++ b/nftables/help/sport.html @@ -1,3 +1,3 @@
Source port
-

TCP/UDP source port number or range (e.g., 22 or 1000-2000).

+

TCP/UDP source port number or range on the remote side of the connection (e.g., 22 or 1000-2000).

diff --git a/nftables/lang/en b/nftables/lang/en index ec8058de1..4017f373a 100644 --- a/nftables/lang/en +++ b/nftables/lang/en @@ -69,6 +69,7 @@ save_err=Failed to save rule apply_err=Failed to apply configuration apply_enone=No saved nftables tables were found to apply. apply_eexternal=Cannot apply configuration because table $1 is currently marked as externally managed. +apply_esettype=Set $1 in table $2 has type $3, but chain $4 uses it for $5. Use ipv4_addr or ipv6_addr sets for address fields, and inet_service sets for port fields. setup_title=Create Ruleset Profile setup_header=Ruleset profile setup_err=Failed to create ruleset profile @@ -170,13 +171,14 @@ edit_proto=Protocol edit_proto_any=Any edit_saddr=Source address edit_daddr=Destination address -edit_sport=Source Port -edit_dport=Destination Port +edit_sport=Source port +edit_dport=Destination port edit_set_none=None edit_saddr_set=Use set $1 edit_daddr_set=Use set $1 edit_sport_set=Use set $1 edit_dport_set=Use set $1 +edit_ports_note=For inbound services such as SSH, match the destination port. Rules are evaluated from top to bottom, so a later drop will not override an earlier accept. edit_icmp_type=ICMP/ICMPv6 type edit_ct_state=Conntrack state edit_tcp_flags=TCP flags @@ -248,6 +250,7 @@ save_raw_empty=Raw rule cannot be empty. save_raw_multiline=Raw rule must be a single line. save_invalid_rule=Raw rule is invalid: $1 save_set_missing=Selected set $1 does not exist. +save_set_type=Selected set $1 has type $2, which cannot be used for $3. move_err=Failed to move rule move_failed=Failed to move rule: 
$1
move_notable=No such table selected diff --git a/nftables/nftables-lib.pl b/nftables/nftables-lib.pl index cc4d145ef..ec2b6fa89 100644 --- a/nftables/nftables-lib.pl +++ b/nftables/nftables-lib.pl @@ -958,6 +958,39 @@ foreach my $r (@{$table->{'rules'}}) { return $count; } +# validate_set_references(&table) +# Returns an error if any structured rule uses a set in an incompatible field +sub validate_set_references +{ +my ($table) = @_; +return if (!$table || ref($table) ne 'HASH'); +return if (!$table->{'sets'} || ref($table->{'sets'}) ne 'HASH'); +return if (!$table->{'rules'} || ref($table->{'rules'}) ne 'ARRAY'); +foreach my $r (@{$table->{'rules'}}) { + next if (!$r || ref($r) ne 'HASH'); + foreach my $check ( + [ 'saddr', 'addr', text('edit_saddr') ], + [ 'daddr', 'addr', text('edit_daddr') ], + [ 'sport', 'port', text('edit_sport') ], + [ 'dport', 'port', text('edit_dport') ], + ) { + my ($field, $want, $label) = @$check; + my $setname = set_name_from_value($r->{$field}); + next if (!$setname); + my $set = $table->{'sets'}->{$setname}; + next if (!$set); + my $kind = set_type_kind($set->{'type'}); + if (!$kind || $kind ne $want) { + my $type = $set->{'type'} || text('set_type_select'); + return text('apply_esettype', $setname, + nft_table_spec($table), $type, + $r->{'chain'} || "-", $label); + } + } + } +return; +} + # dump_nftables_save(@tables) # Returns a string representation of the firewall rules @@ -1097,6 +1130,8 @@ foreach my $t (@$active) { $active{table_key($t)} = $t; } foreach my $t (@tables) { + my $set_err = validate_set_references($t); + return $set_err if ($set_err); my $active_table = $active{table_key($t)}; if ($active_table && table_is_externally_managed($active_table)) { return text('apply_eexternal', nft_table_spec($t)); diff --git a/nftables/save_rule.cgi b/nftables/save_rule.cgi index 7efc803e2..cb2e0461e 100755 --- a/nftables/save_rule.cgi +++ b/nftables/save_rule.cgi @@ -17,6 +17,21 @@ foreach my $sfield (qw(saddr_set daddr_set sport_set dport_set)) { error(text('save_set_missing', $in{$sfield})); } } +foreach my $check ( + [ 'saddr_set', 'addr', $text{'edit_saddr'} ], + [ 'daddr_set', 'addr', $text{'edit_daddr'} ], + [ 'sport_set', 'port', $text{'edit_sport'} ], + [ 'dport_set', 'port', $text{'edit_dport'} ], + ) { + my ($sfield, $want, $label) = @$check; + next if (!$in{$sfield}); + my $set = $table->{'sets'}->{$in{$sfield}}; + my $kind = set_type_kind($set->{'type'}); + if (!$kind || $kind ne $want) { + my $type = $set->{'type'} || $text{'set_type_select'}; + error(text('save_set_type', $in{$sfield}, $type, $label)); + } +} sub join_multi_value { diff --git a/nftables/save_set.cgi b/nftables/save_set.cgi index fe3649988..ff5f9267e 100755 --- a/nftables/save_set.cgi +++ b/nftables/save_set.cgi @@ -54,6 +54,9 @@ $set->{'raw_lines'} ||= [ ]; $table->{'sets'}->{$name} = $set; +my $set_err = validate_set_references($table); +error($set_err) if ($set_err); + my $err = save_table_configuration($table, @tables); error(text('set_failed', $err)) if ($err);