Remove the cleanup of rules for upgrades as this breaks the kill_bloc…

…k_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

@@ -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.

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_'
 

netfilter_openvpn.py

@@ -328,24 +328,25 @@ 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):
 	"""
 		Create a custom chain for the VPN user, named using his source IP
 		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):
 	"""