From b38f8ca566c97b4c26c368d71da0e1db65f7c83a Mon Sep 17 00:00:00 2001
From: Guillaume Destuynder <kang@mozilla.com>
Date: Thu, 15 May 2014 12:47:53 -0700
Subject: [PATCH] Remove the cleanup of rules for upgrades as this breaks the
 kill_block_hack.

Warn loudly on kill_block_hack failure: if it cant remove the rule it means the rule wasnt there in the first place
This is a potential security issue as the user may have had more access than configured. Obviously, this is supposed to
never happen, the check is there just in case.
---
 README.rst                 |  6 ++++++
 netfilter_openvpn.conf.inc |  1 -
 netfilter_openvpn.py       | 16 +++++++---------
 3 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/README.rst b/README.rst
index 75b4e80..778c70f 100644
--- a/README.rst
+++ b/README.rst
@@ -103,3 +103,9 @@ Maintenance
 You can list the rules and sets of a particular user with the script named 'vpn-fw-find-user.sh'.
 
 You can delete all of the rules and sets of a given VPN IP using the script named 'vpn-netfilter-cleanup-ip.sh'.
+
+Upgrading
+=========
+
+On rule upgrades, start the script 'vpn-netfilter-cleanup-ip.sh' within 'netfilter_openvpn.sh' until all rules are
+upgraded. You may also do this all the time but this has a performance cost associated.
diff --git a/netfilter_openvpn.conf.inc b/netfilter_openvpn.conf.inc
index 2d7814b..4d6c8c3 100644
--- a/netfilter_openvpn.conf.inc
+++ b/netfilter_openvpn.conf.inc
@@ -10,7 +10,6 @@ IPTABLES='/sbin/iptables'
 IPSET='/usr/sbin/ipset'
 
 # Rules location
-RULESCLEANUP='<%= confdir %>/plugins/netfilter/vpn-netfilter-cleanup-ip.sh'
 RULES='<%= confdir %>/plugins/netfilter/rules'
 PER_USER_RULES_PREFIX='users/vpn_'
 
diff --git a/netfilter_openvpn.py b/netfilter_openvpn.py
index 900b3cf..26b2bfc 100755
--- a/netfilter_openvpn.py
+++ b/netfilter_openvpn.py
@@ -328,14 +328,18 @@ def chain_exists(name):
 	"""
 	return iptables('-L ' + name, False)
 
-def kill_block_hack(usersrcip):
+def kill_block_hack(usersrcip, usercn):
 	"""
 		Removes the general block on the vpn IP.
 		This is done because we just block the IP and start the script so that openvpn doesnt block.
 		But we don't know if the operation will succeed yet, so it doesnt allow traffic just to be safe.
 		This function allows traffic through.
 	"""
-	iptables('-D INPUT -s %s -j DROP', % usersrcip)
+	try:
+		iptables('-D INPUT -s ' + usersrcip + ' -j DROP')
+	except:
+		mdmsg.send(summary='Failed to delete blocking rule, potential security issue', severity='CRITICAL',
+		details={'vpnip': usersrcip, 'user': usercn})
 
 def add_chain(usersrcip, usercn, dev):
 	"""
@@ -343,9 +347,6 @@ def add_chain(usersrcip, usercn, dev):
 		Load the LDAP rules into the custom chain
 		Jump traffic to the custom chain from the INPUT,OUTPUT & FORWARD chains
 	"""
-	# safe cleanup, just in case
-	command = "%s %s" % (config.RULESCLEANUP, usersrcip)
-	status = os.system(command)
 	usergroups = ""
 	if chain_exists(usersrcip):
 		mdmsg.send(summary='Attempted to replace an existing chain, failing.',
@@ -369,7 +370,7 @@ def add_chain(usersrcip, usercn, dev):
 			 ' "' + ' -m comment --comment "' + usercn + ' at ' + usersrcip + '"')
 	iptables('-A ' + usersrcip + ' -j DROP' +
 			 ' -m comment --comment "' + usercn + ' at ' + usersrcip + '"')
-	kill_block_hack(usersrcip)
+	kill_block_hack(usersrcip, usercn)
 	return True
 
 def del_chain(usersrcip, dev):
@@ -382,9 +383,6 @@ def del_chain(usersrcip, dev):
 	iptables('-F ' + usersrcip, False)
 	iptables('-X ' + usersrcip, False)
 	ipset("--destroy " + usersrcip, False)
-	# safe cleanup, just in case
-	command = "%s %s" % (config.RULESCLEANUP, usersrcip)
-	status = os.system(command)
 
 def update_chain(usersrcip, usercn, dev):
 	"""