commit ae472dc1752bd0f60e56ac284dac818b0afc1034
Author: Shawn Bruce <kantlivelong@users.noreply.github.com>
Date:   Thu Feb 13 08:21:43 2020 +0300

    OpenVPN radius ACL enhancements. Issue #9206

diff --git a/src/etc/inc/openvpn.attributes.php b/src/etc/inc/openvpn.attributes.php
index 353ba938e2..46f071b908 100644
--- a/src/etc/inc/openvpn.attributes.php
+++ b/src/etc/inc/openvpn.attributes.php
@@ -21,17 +21,17 @@
  * limitations under the License.
  */
 
-global $username;
+global $username, $dev, $untrusted_port;
 
-$devname = getenv("dev");
-if (empty($devname)) {
-	$devname = "openvpn";
+if (empty($dev)) {
+	$dev = "openvpn";
 }
 
 function cisco_to_cidr($addr) {
 	if (!is_ipaddr($addr)) {
-		return 0;
+		throw new Exception('Invalid IP Addr');
 	}
+
 	$mask = decbin(~ip2long($addr));
 	$mask = substr($mask, -32);
 	$k = 0;
@@ -42,7 +42,6 @@ function cisco_to_cidr($addr) {
 }
 
 function cisco_extract_index($prule) {
-
 	$index = explode("#", $prule);
 	if (is_numeric($index[1])) {
 		return intval($index[1]);
@@ -52,8 +51,176 @@ function cisco_extract_index($prule) {
 	return -1;;
 }
 
+function parse_cisco_acl_rule($rule, $devname, $dir) {
+	$rule_orig = $rule;
+	$rule = explode(" ", $rule);
+	$tmprule = "";
+	$index = 0;
+
+	if ($rule[$index] == "permit") {
+		$tmprule = "pass {$dir} quick on {$devname} ";
+	} else if ($rule[$index] == "deny") {
+		$tmprule = "block {$dir} quick on {$devname} ";
+	} else {
+		return;
+	}
+
+	$index++;
+
+	switch ($rule[$index]) {
+		case "ip":
+			$tmprule .= "inet ";
+			break;
+		case "icmp":
+		case "tcp":
+		case "udp":
+			$tmprule .= "proto {$rule[$index]} ";
+			break;
+		default:
+			syslog(LOG_WARNING, "Error parsing rule {$rule_orig}: Invalid protocol.");
+			return;
+	}
+	$index++;
+
+	/* Source */
+	if (trim($rule[$index]) == "host") {
+		$index++;
+		$tmprule .= "from {$rule[$index]} ";
+		$index++;
+	} else if (trim($rule[$index]) == "any") {
+		$tmprule .= "from any ";
+		$index++;
+	} else {
+		$network = $rule[$index];
+		$netmask = $rule[++$index];
+
+
+		if(!is_ipaddr($network)) {
+			syslog(LOG_WARNING, "Error parsing rule {$rule_orig}: Invalid source network '$network'.");
+			return;
+		}
+
+		try {
+			$netmask = cisco_to_cidr($netmask);
+		} catch(Exception $e) {
+			syslog(LOG_WARNING, "Error parsing rule {$rule_orig}: Invalid source netmask '$netmask'.");
+			return;
+		}
+		$tmprule .= "from {network}/{$netmask}";
+
+		$index++;
+	}
+
+	/* Source Operator */
+	if (in_array(trim($rule[$index]), array("lt", "gt", "eq", "neq"))) {
+		switch(trim($rule[$index])) {
+			case "lt":
+				$operator = "<";
+				break;
+			case "gt":
+				$operator = ">";
+				break;
+			case "eq":
+				$operator = "=";
+				break;
+			case "neq":
+				$operator = "!=";
+				break;
+		}
+
+		$port = $rule[++$index];
+		if (is_port($port)) {
+			$tmprule .= "port {$operator} {$port} ";
+		} else {
+			syslog(LOG_WARNING, "Error parsing rule {$rule_orig}: Invalid source port: '$port' not a numeric value between 0 and 65535.");
+			return;
+		}
+		$index++;
+	} else if (trim($rule[$index]) == "range") {
+		$port = array($rule[++$index], $rule[++$index]);
+		if (is_port($port[0]) && is_port($port[1])) {
+			$port[0]--;
+			$port[1]++;
+			$tmprule .= "port {$port[0]} >< {$port[1]} ";
+		} else {
+			syslog(LOG_WARNING, "Error parsing rule {$rule_orig}: Invalid source ports: '$port[0]' & '$port[1]' one or both are not a numeric value between 0 and 65535.");
+			return;
+		}
+		$index++;
+	}
+
+	/* Destination */
+	if (trim($rule[$index]) == "host") {
+		$index++;
+		$tmprule .= "to {$rule[$index]} ";
+		$index++;
+	} else if (trim($rule[$index]) == "any") {
+		$tmprule .= "to any ";
+		$index++;
+	} else {
+		$network = $rule[$index];
+		$netmask = $rule[++$index];
+
+
+		if(!is_ipaddr($network)) {
+			syslog(LOG_WARNING, "Error parsing rule {$rule_orig}: Invalid destination network '$network'.");
+			return;
+		}
+
+		try {
+			$netmask = cisco_to_cidr($netmask);
+		} catch(Exception $e) {
+			syslog(LOG_WARNING, "Error parsing rule {$rule_orig}: Invalid destination netmask '$netmask'.");
+			return;
+		}
+		$tmprule .= "to {network}/{$netmask}";
+
+		$index++;
+	}
+
+	/* Destination Operator */
+	if (in_array(trim($rule[$index]), array("lt", "gt", "eq", "neq"))) {
+		switch(trim($rule[$index])) {
+			case "lt":
+				$operator = "<";
+				break;
+			case "gt":
+				$operator = ">";
+				break;
+			case "eq":
+				$operator = "=";
+				break;
+			case "neq":
+				$operator = "!=";
+				break;
+		}
+
+		$port = $rule[++$index];
+		if (is_port($port)) {
+			$tmprule .= "port {$operator} {$port} ";
+		} else {
+			syslog(LOG_WARNING, "Error parsing rule {$rule_orig}: Invalid destination port: '$port' not a numeric value between 0 and 65535.");
+			return;
+		}
+		$index++;
+	} else if (trim($rule[$index]) == "range") {
+		$port = array($rule[++$index], $rule[++$index]);
+		if (is_port($port[0]) && is_port($port[1])) {
+			$port[0]--;
+			$port[1]++;
+			$tmprule .= "port {$port[0]} >< {$port[1]} ";
+		} else {
+			syslog(LOG_WARNING, "Error parsing rule {$rule_orig}: Invalid destination ports: '$port[0]' '$port[1]' one or both are not a numeric value between 0 and 65535.");
+			return;
+		}
+		$index++;
+	}
+
+	return $tmprule;
+}
+
 function parse_cisco_acl($attribs) {
-	global $devname, $attributes;
+	global $dev, $attributes;
 	if (!is_array($attribs)) {
 		return "";
 	}
@@ -83,77 +250,7 @@ function parse_cisco_acl($attribs) {
 				continue;
 			}
 
-			$rule = $rule[1];
-			$rule = explode(" ", $rule);
-			$tmprule = "";
-			$index = 0;
-			$isblock = false;
-			if ($rule[$index] == "permit") {
-				$tmprule = "pass {$dir} quick on {$devname} ";
-			} else if ($rule[$index] == "deny") {
-				//continue;
-				$isblock = true;
-				$tmprule = "block {$dir} quick on {$devname} ";
-			} else {
-				continue;
-			}
-
-			$index++;
-
-			switch ($rule[$index]) {
-				case "tcp":
-				case "udp":
-					$tmprule .= "proto {$rule[$index]} ";
-					break;
-			}
-
-			$index++;
-			/* Source */
-			if (trim($rule[$index]) == "host") {
-				$index++;
-				$tmprule .= "from {$rule[$index]} ";
-				$index++;
-				if ($isblock == true) {
-					$isblock = false;
-				}
-			} else if (trim($rule[$index]) == "any") {
-				$tmprule .= "from any ";
-				$index++;
-			} else {
-				$tmprule .= "from {$rule[$index]}";
-				$index++;
-				$netmask = cisco_to_cidr($rule[$index]);
-				$tmprule .= "/{$netmask} ";
-				$index++;
-				if ($isblock == true) {
-					$isblock = false;
-				}
-			}
-			/* Destination */
-			if (trim($rule[$index]) == "host") {
-				$index++;
-				$tmprule .= "to {$rule[$index]} ";
-				$index++;
-				if ($isblock == true) {
-					$isblock = false;
-				}
-			} else if (trim($rule[$index]) == "any") {
-				$index++;
-				$tmprule .= "to any";
-			} else {
-				$tmprule .= "to {$rule[$index]}";
-				$index++;
-				$netmask = cisco_to_cidr($rule[$index]);
-				$tmprule .= "/{$netmask} ";
-				$index++;
-				if ($isblock == true) {
-					$isblock = false;
-				}
-			}
-
-			if ($isblock == true) {
-				continue;
-			}
+			$tmprule = parse_cisco_acl_rule($rule[1], $dev, $dir);
 
 			if ($dir == "in") {
 				$inrules[$rindex] = $tmprule;
@@ -183,11 +280,8 @@ function parse_cisco_acl($attribs) {
 
 $rules = parse_cisco_acl($attributes);
 if (!empty($rules)) {
-	$pid = posix_getpid();
-	$filename = "{$g['tmp_path']}/ovpn_{$pid}{$username}.rules";
+	$filename = "{$g['tmp_path']}/ovpn_{$dev}_{$username}_{$untrusted_port}.rules";
 	@file_put_contents($filename, $rules);
-	mwexec("/sbin/pfctl -a " . escapeshellarg("openvpn/{$username}") . " -f " . escapeshellarg($filename));
-	@unlink($filename);
 }
 
 ?>
diff --git a/src/etc/inc/openvpn.auth-user.php b/src/etc/inc/openvpn.auth-user.php
index d673a7a9b3..83a1b16f56 100644
--- a/src/etc/inc/openvpn.auth-user.php
+++ b/src/etc/inc/openvpn.auth-user.php
@@ -36,7 +36,7 @@ require_once("interfaces.inc");
 /* setup syslog logging */
 openlog("openvpn", LOG_ODELAY, LOG_AUTH);
 
-global $common_name, $username;
+global $common_name, $username, $dev, $untrusted_port;
 
 if (isset($_GET['username'])) {
 	$authmodes = explode(",", base64_decode($_GET['authcfg']));
@@ -46,11 +46,15 @@ if (isset($_GET['username'])) {
 	$common_name = $_GET['cn'];
 	$modeid = $_GET['modeid'];
 	$strictusercn = $_GET['strictcn'] == "false" ? false : true;
+	$dev = $_GET['dev'];
+	$untrusted_port = $_GET['untrusted_port'];
 } else {
 	/* read data from environment */
 	$username = getenv("username");
 	$password = getenv("password");
 	$common_name = getenv("common_name");
+	$dev = getenv("dev");
+	$untrusted_port = getenv("untrusted_port");
 }
 
 if (!$username) {
diff --git a/src/usr/local/sbin/openvpn.attributes.sh b/src/usr/local/sbin/openvpn.attributes.sh
index 87f9b5bbd7..ee60551f56 100755
--- a/src/usr/local/sbin/openvpn.attributes.sh
+++ b/src/usr/local/sbin/openvpn.attributes.sh
@@ -20,16 +20,53 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+
+lockfile="/tmp/ovpn_${dev}_${username}_${trusted_port}.lock"
+rulesfile="/tmp/ovpn_${dev}_${username}_${trusted_port}.rules"
+anchorname="openvpn/${dev}_${username}_${trusted_port}"
+
 if [ "$script_type" = "client-connect" ]; then
+	i=1
+	while [ -f "${lockfile}" ]; do
+		if [ $i -ge 30 ]; then
+			/bin/echo "Timeout while waiting for lockfile"
+			exit 1
+		fi
+
+		/bin/sleep 1
+		i=$(( i + 1 ))
+	done
+	/usr/bin/touch "${lockfile}"
+
+	/bin/cat "${rulesfile}" | /usr/bin/sed "s/{clientip}/${ifconfig_pool_remote_ip}/g" > "${rulesfile}.tmp" && /bin/mv "${rulesfile}.tmp" "${rulesfile}"
+	/sbin/pfctl -a "openvpn/${dev}_${username}_${trusted_port}" -f "${rulesfile}"
+	/bin/rm "${rulesfile}"
+
 	if [ -f /tmp/$common_name ]; then
 		/bin/cat /tmp/$common_name > $1
 		/bin/rm /tmp/$common_name
 	fi
+
+	/bin/rm "${lockfile}"
 elif [ "$script_type" = "client-disconnect" ]; then
-	command="/sbin/pfctl -a 'openvpn/$common_name' -F rules"
+	i=1
+	while [ -f "${lockfile}" ]; do
+		if [ $i -ge 30 ]; then
+			/bin/echo "Timeout while waiting for lockfile"
+			exit 1
+		fi
+
+		/bin/sleep 1
+		i=$(( i + 1 ))
+	done
+	/usr/bin/touch "${lockfile}"
+
+	command="/sbin/pfctl -a '${anchorname}' -F rules"
 	eval $command
 	/sbin/pfctl -k $ifconfig_pool_remote_ip
 	/sbin/pfctl -K $ifconfig_pool_remote_ip
+
+	/bin/rm "${lockfile}"
 fi
 
 exit 0
diff --git a/src/usr/local/sbin/ovpn_auth_verify_async b/src/usr/local/sbin/ovpn_auth_verify_async
index 1d9059f87f..b09773acdc 100755
--- a/src/usr/local/sbin/ovpn_auth_verify_async
+++ b/src/usr/local/sbin/ovpn_auth_verify_async
@@ -62,7 +62,7 @@ username=$(printf "%s" "${username}" | "${openssl}" enc -base64 | \
 # ---------- Perform Check
 auth_credentials="username=${username}&password=${password}"
 # Note that common_name is also assumed to be set by the environment
-auth_server_1="cn=${common_name}&strictcn=${strictcn}&authcfg=${authcfg}"
+auth_server_1="cn=${common_name}&strictcn=${strictcn}&authcfg=${authcfg}&dev=${dev}&untrusted_port=${untrusted_port}"
 auth_server_2="modeid=${modeid}&nas_port=${nas_port}"
 auth_args="${auth_credentials}&${auth_server_1}&${auth_server_2}"
 
@@ -72,7 +72,7 @@ result=$("${fcgicli}" -f "${auth_user_php}" -d "${auth_args}")
 
 auth_result="${auth_failure}"
 if [ "${result}" = "OK" ]; then
-    auth_result="${auth_success}"
+	auth_result="${auth_success}"
 fi
 
 # The output file path should be set in the environment
