diff options
author | stilez <stilez@users.noreply.github.com> | 2016-01-13 11:53:24 +0000 |
---|---|---|
committer | stilez <stilez@users.noreply.github.com> | 2016-01-13 11:53:24 +0000 |
commit | e8d5be8e6d5587e8b7c99411d0c18fb9a2821ee6 (patch) | |
tree | e75a4f6aa7df8fdb25886254188e128bc85005a0 | |
parent | d90598ce9fe7c7a99e5c390402d2260ff056d9c6 (diff) | |
download | pfsense-e8d5be8e6d5587e8b7c99411d0c18fb9a2821ee6.zip pfsense-e8d5be8e6d5587e8b7c99411d0c18fb9a2821ee6.tar.gz |
Fix logic for subnet overlap check + canonical for IPv6
The subnet overlap functions came up as a concern while fixing redmine 5702.
Specifically -
The "canonical" function check_subnets_overlap() doesn't handle IPv6 (util.inc has standardised on v4/v6/agnostic versions, but this doesn't fit). Fixed by adding transparent detection of v4/v6 and a specific IPv4-only version
The IPv6 version is wrong (if sub1 *contains* sub2 then neither of sub1's endpoints will be detected as "inrange" of sub2 and result will be incorrect: this logic error has been fixed recently in other code too)
Bad data isn't detected - this still isn't detected for compatibility and is tagged "FIXME" instead to look at in future. Reason - not to break anything, at present always returns "overlap = true/false", not "true/false/invalid input".
because CIDR overlap implies containment, the IPv4 version uses a very efficient logic, namely calculates largest size subnet and checks this is same for both. Adopting this for both, and simplifying, makes these functions far "neater"
The old v4 version allowed for non-numeric $bits which doesn't make sense and I've omitted. Cannot think of a single situation where we would provide empty or bad data when we actually mean a /32 single IP.
Solution in this commit - a canonical "overlap" test (IPv4/IPv6 agnostic), IPv4/v6 "overlap" versions that actually do the work, in each case using the same logic as the old v4 (identify largest bit size and test if subnets created are valid and identical), and tag lack of "bad data" detection as fixme for now, returning FALSE instead to avoid breaking anything until fixed. Should be transparent from outside.
-rw-r--r-- | src/etc/inc/util.inc | 56 |
1 files changed, 32 insertions, 24 deletions
diff --git a/src/etc/inc/util.inc b/src/etc/inc/util.inc index b542566..3181fc3 100644 --- a/src/etc/inc/util.inc +++ b/src/etc/inc/util.inc @@ -847,36 +847,44 @@ function subnetv4_expand($subnet) { return $result; } -/* find out whether two subnets overlap */ +/* find out whether two IPv4/IPv6 CIDR subnets overlap. + Note: CIDR overlap implies one is identical or included so largest sn will be the same */ function check_subnets_overlap($subnet1, $bits1, $subnet2, $bits2) { - - if (!is_numeric($bits1)) { - $bits1 = 32; - } - if (!is_numeric($bits2)) { - $bits2 = 32; - } - - if ($bits1 < $bits2) { - $relbits = $bits1; + if (is_ipaddrv4($ipaddr)) { + return check_subnetsv4_overlap($subnet1, $bits1, $subnet2, $bits2); } else { - $relbits = $bits2; + return check_subnetsv6_overlap($subnet1, $bits1, $subnet2, $bits2); } - - $sn1 = gen_subnet_mask_long($relbits) & ip2long($subnet1); - $sn2 = gen_subnet_mask_long($relbits) & ip2long($subnet2); - - return ($sn1 == $sn2); } -/* find out whether two IPv6 subnets overlap */ -function check_subnetsv6_overlap($subnet1, $bits1, $subnet2, $bits2) { - $sub1_min = gen_subnetv6($subnet1, $bits1); - $sub1_max = gen_subnetv6_max($subnet1, $bits1); - $sub2_min = gen_subnetv6($subnet2, $bits2); - $sub2_max = gen_subnetv6_max($subnet2, $bits2); +/* find out whether two IPv4 CIDR subnets overlap. + Note: CIDR overlap means sn1/sn2 are identical or one is included in other. So sn using largest $bits will be the same */ +function check_subnetsv4_overlap($subnet1, $bits1, $subnet2, $bits2) { + $largest_sn = max($bits1, $bits2); + $subnetv4_start1 = gen_subnetv4($subnet1, $largest_sn); + $subnetv4_start2 = gen_subnetv4($subnet1, $largest_sn); + + if($subnetv4_start1 == '' || $subnetv4_start2 == '') { + // One or both args is not a valid IPv4 subnet + //FIXME: needs to return "bad data" not true/false if bad. For now return false, best we can do until fixed + return false; + } + return ($subnetv4_start1 == $subnetv4_start2); +} - return (is_inrange_v6($sub1_min, $sub2_min, $sub2_max) || is_inrange_v6($sub1_max, $sub2_min, $sub2_max) || is_inrange_v6($sub2_min, $sub1_min, $sub1_max)); +/* find out whether two IPv6 CIDR subnets overlap. + Note: CIDR overlap means sn1/sn2 are identical or one is included in other. So sn using largest $bits will be the same */ +function check_subnetsv4_overlap($subnet1, $bits1, $subnet2, $bits2) { + $largest_sn = max($bits1, $bits2); + $subnetv4_start1 = gen_subnetv6($subnet1, $largest_sn); + $subnetv4_start2 = gen_subnetv6($subnet1, $largest_sn); + + if($subnetv6_start1 == '' || $subnetv6_start2 == '') { + // One or both args is not a valid IPv6 subnet + //FIXME: needs to return "bad data" not true/false if bad. For now return false, best we can do until fixed + return false; + } + return ($subnetv6_start1 == $subnetv6_start2); } /* return true if $addr is in $subnet, false if not */ |