summaryrefslogtreecommitdiffstats
path: root/usr/local
diff options
context:
space:
mode:
authorPhil Davis <phil.davis@inf.org>2015-07-05 23:18:04 +0545
committerPhil Davis <phil.davis@inf.org>2015-07-05 23:18:04 +0545
commit028ff8f8a3d7c09ee5604d6f3eadcdaaef1610c7 (patch)
tree72779176aa19f00e7019e1fd84ce2e86e535f9b0 /usr/local
parent5af646023e447cb386a2bcbeb2900985adce2321 (diff)
downloadpfsense-028ff8f8a3d7c09ee5604d6f3eadcdaaef1610c7.zip
pfsense-028ff8f8a3d7c09ee5604d6f3eadcdaaef1610c7.tar.gz
Fix #4813 validation of enable/disable of gateways and static routes
1) A disabled gateway can always be enabled - no extra validation needed. 2) When disabling an enabled gateway, check to see that the gateway is not used in any gateway group or enabled static route (similar tests to what is already checked before deleting a gateway). 3) A static route can always be disabled - no extra checks needed. 4) When enabling a static route, check that the selected gateway is enabled - you cannot have a static route enabled on a disabled gateway. 5) Do the address family cross-check between static route and gateway even when the static route is disabled - we do not want to save mismatched IP address families in any case. This covers all the cases I can see to ensure that the enable/disable status combinations of Gateways and Static Routes is always valid.
Diffstat (limited to 'usr/local')
-rw-r--r--usr/local/www/system_gateways.php52
-rw-r--r--usr/local/www/system_gateways_edit.php27
-rw-r--r--usr/local/www/system_routes.php23
-rw-r--r--usr/local/www/system_routes_edit.php12
4 files changed, 89 insertions, 25 deletions
diff --git a/usr/local/www/system_gateways.php b/usr/local/www/system_gateways.php
index 0e21706..c78b41a 100644
--- a/usr/local/www/system_gateways.php
+++ b/usr/local/www/system_gateways.php
@@ -78,7 +78,7 @@ if ($_POST) {
}
}
-function can_delete_gateway_item($id) {
+function can_delete_disable_gateway_item($id, $disable = false) {
global $config, $input_errors, $a_gateways;
if (!isset($a_gateways[$id])) {
@@ -90,8 +90,11 @@ function can_delete_gateway_item($id) {
foreach ($group['item'] as $item) {
$items = explode("|", $item);
if ($items[0] == $a_gateways[$id]['name']) {
- $input_errors[] = sprintf(gettext("Gateway '%s' cannot be deleted because it is in use on Gateway Group '%s'"), $a_gateways[$id]['name'], $group['name']);
- break;
+ if ($disable) {
+ $input_errors[] = sprintf(gettext("Gateway '%s' cannot be disabled because it is in use on Gateway Group '%s'"), $a_gateways[$id]['name'], $group['name']);
+ } else {
+ $input_errors[] = sprintf(gettext("Gateway '%s' cannot be deleted because it is in use on Gateway Group '%s'"), $a_gateways[$id]['name'], $group['name']);
+ }
}
}
}
@@ -100,8 +103,16 @@ function can_delete_gateway_item($id) {
if (is_array($config['staticroutes']['route'])) {
foreach ($config['staticroutes']['route'] as $route) {
if ($route['gateway'] == $a_gateways[$id]['name']) {
- $input_errors[] = sprintf(gettext("Gateway '%s' cannot be deleted because it is in use on Static Route '%s'"), $a_gateways[$id]['name'], $route['network']);
- break;
+ if ($disable) {
+ // The user wants to disable this gateway.
+ if (!isset($route['disabled'])) {
+ // But there is a static route that uses this gateway and is enabled (not disabled).
+ $input_errors[] = sprintf(gettext("Gateway '%s' cannot be disabled because it is in use on Static Route '%s'"), $a_gateways[$id]['name'], $route['network']);
+ }
+ } else {
+ // The user wants to delete this gateway, but there is a static route (enabled or disabled) that refers to the gateway.
+ $input_errors[] = sprintf(gettext("Gateway '%s' cannot be deleted because it is in use on Static Route '%s'"), $a_gateways[$id]['name'], $route['network']);
+ }
}
}
}
@@ -140,7 +151,7 @@ function delete_gateway_item($id) {
unset($input_errors);
if ($_GET['act'] == "del") {
- if (can_delete_gateway_item($_GET['id'])) {
+ if (can_delete_disable_gateway_item($_GET['id'])) {
$realid = $a_gateways[$_GET['id']]['attribute'];
delete_gateway_item($_GET['id']);
write_config("Gateways: removed gateway {$realid}");
@@ -154,7 +165,7 @@ if (isset($_POST['del_x'])) {
/* delete selected items */
if (is_array($_POST['rule']) && count($_POST['rule'])) {
foreach ($_POST['rule'] as $rulei) {
- if (!can_delete_gateway_item($rulei)) {
+ if (!can_delete_disable_gateway_item($rulei)) {
break;
}
}
@@ -176,19 +187,28 @@ if (isset($_POST['del_x'])) {
} else if ($_GET['act'] == "toggle" && $a_gateways[$_GET['id']]) {
$realid = $a_gateways[$_GET['id']]['attribute'];
-
- if (isset($a_gateway_item[$realid]['disabled'])) {
- unset($a_gateway_item[$realid]['disabled']);
+ $disable_gw = !isset($a_gateway_item[$realid]['disabled']);
+ if ($disable_gw) {
+ // The user wants to disable the gateway, so check if that is OK.
+ $ok_to_toggle = can_delete_disable_gateway_item($_GET['id'], $disable_gw);
} else {
- $a_gateway_item[$realid]['disabled'] = true;
+ // The user wants to enable the gateway. That is always OK.
+ $ok_to_toggle = true;
}
+ if ($ok_to_toggle) {
+ if ($disable_gw) {
+ $a_gateway_item[$realid]['disabled'] = true;
+ } else {
+ unset($a_gateway_item[$realid]['disabled']);
+ }
- if (write_config("Gateways: enable/disable")) {
- mark_subsystem_dirty('staticroutes');
- }
+ if (write_config("Gateways: enable/disable")) {
+ mark_subsystem_dirty('staticroutes');
+ }
- header("Location: system_gateways.php");
- exit;
+ header("Location: system_gateways.php");
+ exit;
+ }
}
$pgtitle = array(gettext("System"), gettext("Gateways"));
diff --git a/usr/local/www/system_gateways_edit.php b/usr/local/www/system_gateways_edit.php
index 483e102..b80b2bf 100644
--- a/usr/local/www/system_gateways_edit.php
+++ b/usr/local/www/system_gateways_edit.php
@@ -126,6 +126,33 @@ if ($_POST) {
}
if (!is_validaliasname($_POST['name'])) {
$input_errors[] = gettext("The gateway name must not contain invalid characters.");
+ } else {
+ if (isset($_POST['disabled'])) {
+ // We have a valid gateway name that the user wants to mark as disabled.
+ // Check if the gateway name is used in any gateway group.
+ if (is_array($config['gateways']['gateway_group'])) {
+ foreach ($config['gateways']['gateway_group'] as $group) {
+ foreach ($group['item'] as $item) {
+ $items = explode("|", $item);
+ if ($items[0] == $_POST['name']) {
+ $input_errors[] = sprintf(gettext("Gateway '%s' cannot be disabled because it is in use on Gateway Group '%s'"), $_POST['name'], $group['name']);
+ }
+ }
+ }
+ }
+
+ // Check if the gateway name is used in any enabled Static Route.
+ if (is_array($config['staticroutes']['route'])) {
+ foreach ($config['staticroutes']['route'] as $route) {
+ if ($route['gateway'] == $_POST['name']) {
+ if (!isset($route['disabled'])) {
+ // There is a static route that uses this gateway and is enabled (not disabled).
+ $input_errors[] = sprintf(gettext("Gateway '%s' cannot be disabled because it is in use on Static Route '%s'"), $_POST['name'], $route['network']);
+ }
+ }
+ }
+ }
+ }
}
/* skip system gateways which have been automatically added */
if (($_POST['gateway'] && (!is_ipaddr($_POST['gateway'])) && ($_POST['attribute'] !== "system")) && ($_POST['gateway'] != "dynamic")) {
diff --git a/usr/local/www/system_routes.php b/usr/local/www/system_routes.php
index 4d44557..9443d40 100644
--- a/usr/local/www/system_routes.php
+++ b/usr/local/www/system_routes.php
@@ -53,6 +53,7 @@ if (!is_array($config['staticroutes']['route'])) {
$a_routes = &$config['staticroutes']['route'];
$a_gateways = return_gateways_array(true, true, true);
$changedesc_prefix = gettext("Static Routes") . ": ";
+unset($input_errors);
if ($_POST) {
@@ -142,20 +143,29 @@ if (isset($_POST['del_x'])) {
} else if ($_GET['act'] == "toggle") {
if ($a_routes[$_GET['id']]) {
+ $do_update_config = true;
if (isset($a_routes[$_GET['id']]['disabled'])) {
- unset($a_routes[$_GET['id']]['disabled']);
- $changedesc = $changedesc_prefix . gettext("enabled route to") . " " . $a_routes[$_GET['id']]['network'];
+ // Do not enable a route whose gateway is disabled
+ if (isset($a_gateways[$a_routes[$_GET['id']]['gateway']]['disabled'])) {
+ $do_update_config = false;
+ $input_errors[] = $changedesc_prefix . gettext("gateway is disabled, cannot enable route to") . " " . $a_routes[$_GET['id']]['network'];
+ } else {
+ unset($a_routes[$_GET['id']]['disabled']);
+ $changedesc = $changedesc_prefix . gettext("enabled route to") . " " . $a_routes[$_GET['id']]['network'];
+ }
} else {
delete_static_route($_GET['id']);
$a_routes[$_GET['id']]['disabled'] = true;
$changedesc = $changedesc_prefix . gettext("disabled route to") . " " . $a_routes[$_GET['id']]['network'];
}
- if (write_config($changedesc)) {
- mark_subsystem_dirty('staticroutes');
+ if ($do_update_config) {
+ if (write_config($changedesc)) {
+ mark_subsystem_dirty('staticroutes');
+ }
+ header("Location: system_routes.php");
+ exit;
}
- header("Location: system_routes.php");
- exit;
}
} else {
/* yuck - IE won't send value attributes for image buttons, while Mozilla does - so we use .x/.y to find move button clicks instead... */
@@ -225,6 +235,7 @@ include("head.inc");
<?php if (is_subsystem_dirty('staticroutes')): ?><p>
<?php print_info_box_np(sprintf(gettext("The static route configuration has been changed.%sYou must apply the changes in order for them to take effect."), "<br />"));?><br /></p>
<?php endif; ?>
+<?php if ($input_errors) print_input_errors($input_errors); ?>
<table width="100%" border="0" cellpadding="0" cellspacing="0" summary="system routes">
<tr>
diff --git a/usr/local/www/system_routes_edit.php b/usr/local/www/system_routes_edit.php
index 81a821d..ea9047f 100644
--- a/usr/local/www/system_routes_edit.php
+++ b/usr/local/www/system_routes_edit.php
@@ -102,9 +102,15 @@ if ($_POST) {
if (($_POST['gateway']) && is_ipaddr($_POST['network'])) {
if (!isset($a_gateways[$_POST['gateway']])) {
$input_errors[] = gettext("A valid gateway must be specified.");
- }
- if (!validate_address_family($_POST['network'], $_POST['gateway'])) {
- $input_errors[] = gettext("The gateway '{$a_gateways[$_POST['gateway']]['gateway']}' is a different Address Family as network '{$_POST['network']}'.");
+ } else {
+ if (isset($a_gateways[$_POST['gateway']]['disabled']) && !$_POST['disabled']) {
+ $input_errors[] = gettext("The gateway is disabled but the route is not. You must disable the route in order to choose a disabled gateway.");
+ } else {
+ // Note that the 3rd parameter "disabled" must be passed as explicitly true or false.
+ if (!validate_address_family($_POST['network'], $_POST['gateway'], $_POST['disabled'] ? true : false)) {
+ $input_errors[] = gettext("The gateway '{$a_gateways[$_POST['gateway']]['gateway']}' is a different Address Family than network '{$_POST['network']}'.");
+ }
+ }
}
}
OpenPOWER on IntegriCloud