From a9c6fe724a50533ac0347c6ac91cd58490d2dffe Mon Sep 17 00:00:00 2001 From: Ilia Ross Date: Fri, 19 Jun 2026 14:50:24 +0200 Subject: [PATCH] Fix IPv6 CIDR access control matching This PR fixes Webmin IP access control handling for IPv6 CIDR prefixes that are not divisible by 8, such as `/29` as mentioned in this https://github.com/webmin/webmin/issues/1570 ticket. Before Webmin validation rejected non-byte-aligned IPv6 network sizes, and the runtime matcher compared IPv6 networks only by whole bytes. This meant valid IPv6 CIDR prefixes could not be used safely in access control rules. Changes: - Allow IPv6 access-control prefixes from `/0` through `/128`, without requiring divisibility by 8. - Add bit-accurate IPv6 prefix matching for ACL checks. - Apply the same matching behavior in both `miniserv.pl` and `webmin/webmin-lib.pl`. - Fix IPv6 canonicalization for `::` and trailing `::` forms used by the matcher. - Add regression tests for `/0`, `/29`, `/32`, `/63`, `/64`, `/127`, and `/128`. --- miniserv.pl | 41 +++++++++++++++++++++++++++------------ t/miniserv.t | 45 +++++++++++++++++++++++++++++++++++++++++++ webmin/webmin-lib.pl | 46 ++++++++++++++++++++++++++++++-------------- 3 files changed, 106 insertions(+), 26 deletions(-) diff --git a/miniserv.pl b/miniserv.pl index 72cdce74f..7dbfa54db 100755 --- a/miniserv.pl +++ b/miniserv.pl @@ -3200,14 +3200,7 @@ for($i=2; $i<@_; $i++) { # Compare with an IPv6 network local $v6size = $2; local $v6addr = &canonicalize_ip6($1); - local $bytes = $v6size / 8; - @mo = &expand_ipv6_bytes($v6addr); - local @io6 = &expand_ipv6_bytes(&canonicalize_ip6($_[0])); - for($j=0; $j<$bytes; $j++) { - if ($mo[$j] ne $io6[$j]) { - $mismatch = 1; - } - } + $mismatch = 1 if (!&ip6_prefix_match($_[0], $v6addr, $v6size)); } elsif ($_[$i] !~ /^[0-9\.]+$/) { # Compare with hostname @@ -7284,16 +7277,20 @@ sub canonicalize_ip6 { my ($addr) = @_; return $addr if (!&check_ip6address($addr)); -my @w = split(/:/, $addr); +my @w = split(/:/, $addr, -1); my $idx = &indexof("", @w); if ($idx >= 0) { # Expand :: - my $mis = 8 - scalar(@w); - my @nw = @w[0..$idx]; + my $endidx = $idx; + while($endidx+1 < @w && $w[$endidx+1] eq "") { + $endidx++; + } + my $mis = 8 - (scalar(@w) - ($endidx - $idx + 1)); + my @nw = @w[0..$idx-1]; for(my $i=0; $i<$mis; $i++) { push(@nw, 0); } - push(@nw, @w[$idx+1 .. $#w]); + push(@nw, @w[$endidx+1 .. $#w]); @w = @nw; } foreach my $w (@w) { @@ -7317,6 +7314,26 @@ foreach my $w (split(/:/, $addr)) { return @rv; } +# ip6_prefix_match(address, network, prefix) +# Returns 1 if an IPv6 address is in a network prefix. +sub ip6_prefix_match +{ +my ($addr, $network, $prefix) = @_; +my @addr = &expand_ipv6_bytes(&canonicalize_ip6($addr)); +my @network = &expand_ipv6_bytes(&canonicalize_ip6($network)); +return 0 if (@addr != 16 || @network != 16 || $prefix < 0 || $prefix > 128); +my $bytes = int($prefix / 8); +my $bits = $prefix % 8; +for(my $i=0; $i<$bytes; $i++) { + return 0 if ($addr[$i] != $network[$i]); + } +if ($bits) { + my $mask = (0xff << (8 - $bits)) & 0xff; + return 0 if (($addr[$bytes] & $mask) != ($network[$bytes] & $mask)); + } +return 1; +} + sub get_somaxconn { return defined(&SOMAXCONN) ? SOMAXCONN : 128; diff --git a/t/miniserv.t b/t/miniserv.t index c53ca94d0..c9335a153 100644 --- a/t/miniserv.t +++ b/t/miniserv.t @@ -321,6 +321,13 @@ subtest 'canonicalize_ip6' => sub { is($c2, '2001:0db8:0000:0000:0000:0000:0000:0001', '2001:DB8::1 lower-cased and zero-expanded'); + is(miniserv::canonicalize_ip6('::'), + '0000:0000:0000:0000:0000:0000:0000:0000', + ':: expands to 8 zero-padded groups'); + is(miniserv::canonicalize_ip6('2001:db8::'), + '2001:0db8:0000:0000:0000:0000:0000:0000', + 'trailing :: expands to 8 zero-padded groups'); + # Idempotency: running canonicalize on canonical input returns the same. is(miniserv::canonicalize_ip6($c), $c, 'canonical form is idempotent'); }; @@ -333,6 +340,44 @@ subtest 'expand_ipv6_bytes' => sub { is($bytes[0], 0, 'high byte is 0 for ::1'); }; +subtest 'ip6_prefix_match' => sub { + ok( miniserv::ip6_prefix_match('2001:db8::1', '::', 0), + '/0 matches any IPv6 address'); + ok( miniserv::ip6_prefix_match('2001:dbf::1', '2001:db8::', 29), + '/29 accepts address inside non-byte-aligned prefix'); + ok(!miniserv::ip6_prefix_match('2001:dc0::1', '2001:db8::', 29), + '/29 rejects address outside non-byte-aligned prefix'); + ok( miniserv::ip6_prefix_match('2001:db8:ffff::1', '2001:db8::', 32), + '/32 matches across the third word'); + ok( miniserv::ip6_prefix_match('2001:db8:0:1::1', '2001:db8:0:1::', 63), + '/63 accepts address inside partial fourth-word prefix'); + ok(!miniserv::ip6_prefix_match('2001:db8:0:3::1', '2001:db8:0:1::', 63), + '/63 rejects address outside partial fourth-word prefix'); + ok( miniserv::ip6_prefix_match('2001:db8:0:1::1', '2001:db8:0:1::', 64), + '/64 matches exact first four words'); + ok( miniserv::ip6_prefix_match('2001:db8::2', '2001:db8::2', 127), + '/127 accepts low address in two-address subnet'); + ok( miniserv::ip6_prefix_match('2001:db8::3', '2001:db8::2', 127), + '/127 accepts high address in two-address subnet'); + ok(!miniserv::ip6_prefix_match('2001:db8::4', '2001:db8::2', 127), + '/127 rejects next subnet'); + ok( miniserv::ip6_prefix_match('2001:db8::1', '2001:db8::1', 128), + '/128 matches identical host address'); + ok(!miniserv::ip6_prefix_match('2001:db8::2', '2001:db8::1', 128), + '/128 rejects neighboring host address'); +}; + +subtest 'ip_match IPv6 CIDR prefixes' => sub { + ok( miniserv::ip_match('2001:dbf::1', '2001:db8::1', '2001:db8::/29'), + 'ip_match accepts non-byte-aligned IPv6 CIDR match'); + ok(!miniserv::ip_match('2001:dc0::1', '2001:db8::1', '2001:db8::/29'), + 'ip_match rejects neighboring non-byte-aligned IPv6 CIDR prefix'); + ok( miniserv::ip_match('2001:db8:0:1::1', '2001:db8::1', '2001:db8:0:1::/63'), + 'ip_match accepts partial-byte IPv6 prefix'); + ok(!miniserv::ip_match('2001:db8:0:3::1', '2001:db8::1', '2001:db8:0:1::/63'), + 'ip_match rejects address outside partial-byte IPv6 prefix'); +}; + # is_bad_header — ShellShock guard subtest 'is_bad_header' => sub { ok( miniserv::is_bad_header('() { :; }; echo pwned'), 'ShellShock-shaped value flagged'); diff --git a/webmin/webmin-lib.pl b/webmin/webmin-lib.pl index 4f9760d95..641c499b3 100755 --- a/webmin/webmin-lib.pl +++ b/webmin/webmin-lib.pl @@ -1699,14 +1699,7 @@ for(my $i=1; $i<@_; $i++) { # Compare with an IPv6 network my $v6size = $2; my $v6addr = &canonicalize_ip6($1); - my $bytes = $v6size / 8; - my @mo = &expand_ipv6_bytes($v6addr); - my @io = &expand_ipv6_bytes(&canonicalize_ip6($_[0])); - for(my $j=0; $j<$bytes; $j++) { - if ($mo[$j] ne $io[$j]) { - $mismatch = 1; - } - } + $mismatch = 1 if (!&ip6_prefix_match($_[0], $v6addr, $v6size)); } elsif ($_[$i] !~ /^[0-9\.]+$/) { # Compare with hostname @@ -1741,6 +1734,29 @@ foreach my $w (split(/:/, $addr)) { return @rv; } +=head2 ip6_prefix_match(address, network, prefix) + +Returns 1 if an IPv6 address is in a network prefix. + +=cut +sub ip6_prefix_match +{ +my ($addr, $network, $prefix) = @_; +my @addr = &expand_ipv6_bytes(&canonicalize_ip6($addr)); +my @network = &expand_ipv6_bytes(&canonicalize_ip6($network)); +return 0 if (@addr != 16 || @network != 16 || $prefix < 0 || $prefix > 128); +my $bytes = int($prefix / 8); +my $bits = $prefix % 8; +for(my $i=0; $i<$bytes; $i++) { + return 0 if ($addr[$i] != $network[$i]); + } +if ($bits) { + my $mask = (0xff << (8 - $bits)) & 0xff; + return 0 if (($addr[$bytes] & $mask) != ($network[$bytes] & $mask)); + } +return 1; +} + =head2 prefix_to_mask(prefix) @@ -1797,8 +1813,6 @@ elsif ($h =~ /^([a-f0-9:]+)\/(\d+)$/) { return &text('access_eip6', $1); $2 >= 0 && $2 <= 128 || return &text('access_ecidr6', "$2"); - $2 % 8 == 0 || - return &text('access_ecidr8', "$2"); } elsif ($h =~ /^[a-f0-9:]+$/) { # IPv6 address @@ -2635,16 +2649,20 @@ sub canonicalize_ip6 { my ($addr) = @_; return $addr if (!&check_ip6address($addr)); -my @w = split(/:/, $addr); +my @w = split(/:/, $addr, -1); my $idx = &indexof("", @w); if ($idx >= 0) { # Expand :: - my $mis = 8 - scalar(@w); - my @nw = @w[0..$idx]; + my $endidx = $idx; + while($endidx+1 < @w && $w[$endidx+1] eq "") { + $endidx++; + } + my $mis = 8 - (scalar(@w) - ($endidx - $idx + 1)); + my @nw = @w[0..$idx-1]; for(my $i=0; $i<$mis; $i++) { push(@nw, 0); } - push(@nw, @w[$idx+1 .. $#w]); + push(@nw, @w[$endidx+1 .. $#w]); @w = @nw; } foreach my $w (@w) {