From 3490b8ddaea5944d9dd4b93ab1f28398170ee181 Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Fri, 10 Apr 2015 17:59:46 +0545 Subject: Check for overlapping subnets when saving interface addresses This checks if a static IP address entered for an interface has a subnet that overlaps with any other configured subnet. e.g.: LAN is IPv4 10.10.12.1/24 Then try to set OPT1 to 10.10.13.1/23 - it overlaps with LAN because "/23" covers the "12" and "13" together. In the input errors message, display to the user the other interfaces and subnets that overlap/conflict. Then the user has some idea what it is that conflicts and can easily go looking in the right place for the problem. Do the same thing for IPv6 address/CIDR. Note: I have not enhanced any of the checks for conflicts with static routes - there could be cases where a user has a static route like 10.0.0.0/8 pointing to some internal router that has the rest of 10.0.0.0/8 behind it, but the user has some direct-attached subnet inside that - e.g. 10.1.2.0/24 - the routing table should cope with this, delivering directly to 10.1.2.0/24 and routing for the rest of 10.0.0.0/8. So we cannot invalidate all overlaps with static routes. I think this validation will not invalidate any exotic-but-valid use cases. I can't think of when the interface subnets on 2 interfaces can overlap and still be a valid/useful configuration. This should stop people setting up dumb mixes of LAN/OPT1/OPT2... with random addresses and CIDR prefix that overlap each other. --- etc/inc/pfsense-utils.inc | 94 +++++++++++++++++++++++++++++++++++--------- usr/local/www/interfaces.php | 20 ++++++++-- 2 files changed, 92 insertions(+), 22 deletions(-) diff --git a/etc/inc/pfsense-utils.inc b/etc/inc/pfsense-utils.inc index f969ada..f3a54cf 100644 --- a/etc/inc/pfsense-utils.inc +++ b/etc/inc/pfsense-utils.inc @@ -2709,13 +2709,41 @@ function load_mac_manufacturer_table() { * INPUTS * IP Address to check. * If ignore_if is a VIP (not carp), vip array index is passed after string _virtualip + * check_localip - if true then also check for matches with PPTP and LT2P addresses + * check_subnets - if true then check if the given ipaddr is contained anywhere in the subnet of any other configured IP address + * cidrprefix - the CIDR prefix (16, 20, 24, 64...) of ipaddr. + * If check_subnets is true and cidrprefix is specified, + * then check if the ipaddr/cidrprefix subnet overlaps the subnet of any other configured IP address * RESULT - * returns true if the IP Address is - * configured and present on this device. + * returns true if the IP Address is configured and present on this device or overlaps a configured subnet. */ -function is_ipaddr_configured($ipaddr, $ignore_if = "", $check_localip = false, $check_subnets = false) { +function is_ipaddr_configured($ipaddr, $ignore_if = "", $check_localip = false, $check_subnets = false, $cidrprefix = "") { + if (count(where_is_ipaddr_configured($ipaddr, $ignore_if, $check_localip, $check_subnets, $cidrprefix))) { + return true; + } + return false; +} + +/****f* pfsense-utils/where_is_ipaddr_configured + * NAME + * where_is_ipaddr_configured + * INPUTS + * IP Address to check. + * If ignore_if is a VIP (not carp), vip array index is passed after string _virtualip + * check_localip - if true then also check for matches with PPTP and LT2P addresses + * check_subnets - if true then check if the given ipaddr is contained anywhere in the subnet of any other configured IP address + * cidrprefix - the CIDR prefix (16, 20, 24, 64...) of ipaddr. + * If check_subnets is true and cidrprefix is specified, + * then check if the ipaddr/cidrprefix subnet overlaps the subnet of any other configured IP address + * RESULT + * Returns an array of the interfaces 'if' plus IP address or subnet 'ip_or_subnet' that match or overlap the IP address to check. + * If there are no matches then an empty array is returned. +*/ +function where_is_ipaddr_configured($ipaddr, $ignore_if = "", $check_localip = false, $check_subnets = false, $cidrprefix = "") { global $config; + $where_configured = array(); + $pos = strpos($ignore_if, '_virtualip'); if ($pos !== false) { $ignore_vip_id = substr($ignore_if, $pos+10); @@ -2728,26 +2756,44 @@ function is_ipaddr_configured($ipaddr, $ignore_if = "", $check_localip = false, $isipv6 = is_ipaddrv6($ipaddr); if ($check_subnets) { + $cidrprefix = intval($cidrprefix); + if ($isipv6) { + if (($cidrprefix < 1) || ($cidrprefix > 128)) { + $cidrprefix = 128; + } + } else { + if (($cidrprefix < 1) || ($cidrprefix > 32)) { + $cidrprefix = 32; + } + } $iflist = get_configured_interface_list(); foreach ($iflist as $if => $ifname) { if ($ignore_if == $if) { continue; } - if ($isipv6 === true) { - $bitmask = get_interface_subnetv6($if); - $subnet = gen_subnetv6(get_interface_ipv6($if), $bitmask); + if ($isipv6) { + $if_ipv6 = get_interface_ipv6($if); + $if_snbitsv6 = get_interface_subnetv6($if); + if ($if_ipv6 && $if_snbitsv6 && check_subnetsv6_overlap($ipaddr, $cidrprefix, $if_ipv6, $if_snbitsv6)) { + $where_entry = array(); + $where_entry['if'] = $if; + $where_entry['ip_or_subnet'] = get_interface_ipv6($if) . "/" . get_interface_subnetv6($if); + $where_configured[] = $where_entry; + } } else { - $bitmask = get_interface_subnet($if); - $subnet = gen_subnet(get_interface_ip($if), $bitmask); - } - - if (ip_in_subnet($ipaddr, $subnet . '/' . $bitmask)) { - return true; + $if_ipv4 = get_interface_ip($if); + $if_snbitsv4 = get_interface_subnet($if); + if ($if_ipv4 && $if_snbitsv4 && check_subnets_overlap($ipaddr, $cidrprefix, $if_ipv4, $if_snbitsv4)) { + $where_entry = array(); + $where_entry['if'] = $if; + $where_entry['ip_or_subnet'] = get_interface_ip($if) . "/" . get_interface_subnet($if); + $where_configured[] = $where_entry; + } } } } else { - if ($isipv6 === true) { + if ($isipv6) { $interface_list_ips = get_configured_ipv6_addresses(); } else { $interface_list_ips = get_configured_ip_addresses(); @@ -2758,7 +2804,10 @@ function is_ipaddr_configured($ipaddr, $ignore_if = "", $check_localip = false, continue; } if (strcasecmp($ipaddr, $ilips) == 0) { - return true; + $where_entry = array(); + $where_entry['if'] = $if; + $where_entry['ip_or_subnet'] = $ilips; + $where_configured[] = $where_entry; } } } @@ -2770,21 +2819,30 @@ function is_ipaddr_configured($ipaddr, $ignore_if = "", $check_localip = false, continue; } if (strcasecmp($ipaddr, $vip['ipaddr']) == 0) { - return true; + $where_entry = array(); + $where_entry['if'] = $vip['if']; + $where_entry['ip_or_subnet'] = $vip['ipaddr']; + $where_configured[] = $where_entry; } } if ($check_localip) { if (is_array($config['pptpd']) && !empty($config['pptpd']['localip']) && (strcasecmp($ipaddr, $config['pptpd']['localip']) == 0)) { - return true; + $where_entry = array(); + $where_entry['if'] = 'pptp'; + $where_entry['ip_or_subnet'] = $config['pptpd']['localip']; + $where_configured[] = $where_entry; } if (!is_array($config['l2tp']) && !empty($config['l2tp']['localip']) && (strcasecmp($ipaddr, $config['l2tp']['localip']) == 0)) { - return true; + $where_entry = array(); + $where_entry['if'] = 'l2tp'; + $where_entry['ip_or_subnet'] = $config['l2tp']['localip']; + $where_configured[] = $where_entry; } } - return false; + return $where_configured; } /****f* pfsense-utils/pfSense_handle_custom_code diff --git a/usr/local/www/interfaces.php b/usr/local/www/interfaces.php index 62cf658..89cff44 100644 --- a/usr/local/www/interfaces.php +++ b/usr/local/www/interfaces.php @@ -636,8 +636,14 @@ if ($_POST['apply']) { if (!is_ipaddrv4($_POST['ipaddr'])) $input_errors[] = gettext("A valid IPv4 address must be specified."); else { - if (is_ipaddr_configured($_POST['ipaddr'], $if, true)) - $input_errors[] = gettext("This IPv4 address is being used by another interface or VIP."); + $where_ipaddr_configured = where_is_ipaddr_configured($_POST['ipaddr'], $if, true, true, $_POST['subnet']); + if (count($where_ipaddr_configured)) { + $subnet_conflict_text = sprintf(gettext("IPv4 address %s is being used by or overlaps with:"), $_POST['ipaddr'] . "/" . $_POST['subnet']); + foreach ($where_ipaddr_configured as $subnet_conflict) { + $subnet_conflict_text .= " " . convert_friendly_interface_to_friendly_descr($subnet_conflict['if']) . " (" . $subnet_conflict['ip_or_subnet'] . ")"; + } + $input_errors[] = $subnet_conflict_text; + } /* Do not accept network or broadcast address, except if subnet is 31 or 32 */ if ($_POST['subnet'] < 31) { @@ -661,8 +667,14 @@ if ($_POST['apply']) { if (!is_ipaddrv6($_POST['ipaddrv6'])) $input_errors[] = gettext("A valid IPv6 address must be specified."); else { - if (is_ipaddr_configured($_POST['ipaddrv6'], $if, true)) - $input_errors[] = gettext("This IPv6 address is being used by another interface or VIP."); + $where_ipaddr_configured = where_is_ipaddr_configured($_POST['ipaddrv6'], $if, true, true, $_POST['subnetv6']); + if (count($where_ipaddr_configured)) { + $subnet_conflict_text = sprintf(gettext("IPv6 address %s is being used by or overlaps with:"), $_POST['ipaddrv6'] . "/" . $_POST['subnetv6']); + foreach ($where_ipaddr_configured as $subnet_conflict) { + $subnet_conflict_text .= " " . convert_friendly_interface_to_friendly_descr($subnet_conflict['if']) . " (" . $subnet_conflict['ip_or_subnet'] . ")"; + } + $input_errors[] = $subnet_conflict_text; + } foreach ($staticroutes as $route_subnet) { list($network, $subnet) = explode("/", $route_subnet); -- cgit v1.1