diff --git a/src/opnsense/scripts/OPNsense/CaptivePortal/cp-background-process.py b/src/opnsense/scripts/OPNsense/CaptivePortal/cp-background-process.py index 040f8d097..888c10f30 100755 --- a/src/opnsense/scripts/OPNsense/CaptivePortal/cp-background-process.py +++ b/src/opnsense/scripts/OPNsense/CaptivePortal/cp-background-process.py @@ -226,7 +226,10 @@ def main(): # process accounting messages (uses php script, for reuse of Auth classes) try: - subprocess.call(['/usr/local/opnsense/scripts/OPNsense/CaptivePortal/process_accounting_messages.php']) + subprocess.run( + ['/usr/local/opnsense/scripts/OPNsense/CaptivePortal/process_accounting_messages.php'], + capture_output=True + ) except OSError: # if accounting script crashes don't exit backgroung process pass diff --git a/src/opnsense/scripts/OPNsense/CaptivePortal/lib/arp.py b/src/opnsense/scripts/OPNsense/CaptivePortal/lib/arp.py index e8118ca50..3214847d9 100755 --- a/src/opnsense/scripts/OPNsense/CaptivePortal/lib/arp.py +++ b/src/opnsense/scripts/OPNsense/CaptivePortal/lib/arp.py @@ -24,7 +24,6 @@ POSSIBILITY OF SUCH DAMAGE. """ -import tempfile import subprocess @@ -47,31 +46,29 @@ class ARP(object): """ # parse arp table self._arp_table = dict() - with tempfile.NamedTemporaryFile() as output_stream: - subprocess.check_call(['/usr/sbin/arp', '-an'], stdout=output_stream, stderr=subprocess.STDOUT) - output_stream.seek(0) - for line in output_stream: - line_parts = line.decode().split() + sp = subprocess.run(['/usr/sbin/arp', '-an'], capture_output=True, text=True) + for line in sp.stdout.split("\n"): + line_parts = line.split() - if len(line_parts) < 6 or line_parts[2] != 'at' or line_parts[4] != 'on': - continue - elif len(line_parts[1]) < 2 or line_parts[1][0] != '(' or line_parts[1][-1] != ')': - continue + if len(line_parts) < 6 or line_parts[2] != 'at' or line_parts[4] != 'on': + continue + elif len(line_parts[1]) < 2 or line_parts[1][0] != '(' or line_parts[1][-1] != ')': + continue - address = line_parts[1][1:-1] - physical_intf = line_parts[5] - mac = line_parts[3] - expires = -1 + address = line_parts[1][1:-1] + physical_intf = line_parts[5] + mac = line_parts[3] + expires = -1 - for index in range(len(line_parts) - 3): - if line_parts[index] == 'expires' and line_parts[index + 1] == 'in': - if line_parts[index + 2].isdigit(): - expires = int(line_parts[index + 2]) + for index in range(len(line_parts) - 3): + if line_parts[index] == 'expires' and line_parts[index + 1] == 'in': + if line_parts[index + 2].isdigit(): + expires = int(line_parts[index + 2]) - if address in self._arp_table: - self._arp_table[address]['intf'].append(physical_intf) - elif mac.find('incomplete') == -1: - self._arp_table[address] = {'mac': mac, 'intf': [physical_intf], 'expires': expires} + if address in self._arp_table: + self._arp_table[address]['intf'].append(physical_intf) + elif mac.find('incomplete') == -1: + self._arp_table[address] = {'mac': mac, 'intf': [physical_intf], 'expires': expires} def list_items(self): """ return parsed arp list diff --git a/src/opnsense/scripts/OPNsense/CaptivePortal/lib/ipfw.py b/src/opnsense/scripts/OPNsense/CaptivePortal/lib/ipfw.py index 7e967a953..bf691feea 100755 --- a/src/opnsense/scripts/OPNsense/CaptivePortal/lib/ipfw.py +++ b/src/opnsense/scripts/OPNsense/CaptivePortal/lib/ipfw.py @@ -25,7 +25,6 @@ """ import os -import tempfile import subprocess @@ -41,23 +40,18 @@ class IPFW(object): """ devnull = open(os.devnull, 'w') result = list() - with tempfile.NamedTemporaryFile() as output_stream: - subprocess.check_call(['/sbin/ipfw', 'table', str(table_number), 'list'], - stdout=output_stream, - stderr=devnull) - output_stream.seek(0) - for line in output_stream: - line = line.decode() - if line.split(' ')[0].strip() != "": - # process / 32 nets as single addresses to align better with the rule syntax - # and local administration. - if line.split(' ')[0].split('/')[-1] == '32': - # single IPv4 address - result.append(line.split(' ')[0].split('/')[0]) - else: - # network - result.append(line.split(' ')[0]) - return result + sp = subprocess.run(['/sbin/ipfw', 'table', str(table_number), 'list'], capture_output=True, text=True) + for line in sp.stdout.split('\n'): + if line.split(' ')[0].strip() != "" and not line.startswith('--'): + # process / 32 nets as single addresses to align better with the rule syntax + # and local administration. + if line.split(' ')[0].split('/')[-1] == '32': + # single IPv4 address + result.append(line.split(' ')[0].split('/')[0]) + else: + # network + result.append(line.split(' ')[0]) + return result def ip_or_net_in_table(self, table_number, address): """ check if address or net is in this zone's table @@ -78,8 +72,7 @@ class IPFW(object): :param address: ip address or net to add to table :return: """ - devnull = open(os.devnull, 'w') - subprocess.call(['/sbin/ipfw', 'table', table_number, 'add', address], stdout=devnull, stderr=devnull) + subprocess.run(['/sbin/ipfw', 'table', str(table_number), 'add', address], capture_output=True) @staticmethod def delete_from_table(table_number, address): @@ -88,8 +81,7 @@ class IPFW(object): :param address: ip address or net to add to table :return: """ - devnull = open(os.devnull, 'w') - subprocess.call(['/sbin/ipfw', 'table', table_number, 'delete', address], stdout=devnull, stderr=devnull) + subprocess.run(['/sbin/ipfw', 'table', str(table_number), 'delete', address], capture_output=True) @staticmethod def list_accounting_info(): @@ -97,45 +89,40 @@ class IPFW(object): instead of trying to map addresses back to zones. :return: list accounting info per ip address """ - devnull = open(os.devnull, 'w') result = dict() - with tempfile.NamedTemporaryFile() as output_stream: - subprocess.check_call(['/sbin/ipfw', '-aT', 'list'], - stdout=output_stream, - stderr=devnull) - output_stream.seek(0) - for line in output_stream: - parts = line.decode().split() - if len(parts) > 5: - if 30001 <= int(parts[0]) <= 50000 and parts[4] == 'count': - line_pkts = int(parts[1]) - line_bytes = int(parts[2]) - last_accessed = int(parts[3]) - if parts[7] != 'any': - ip_address = parts[7] - else: - ip_address = parts[9] + sp = subprocess.run(['/sbin/ipfw', '-aT', 'list'], capture_output=True, text=True) + for line in sp.stdout.split('\n'): + parts = line.split() + if len(parts) > 5: + if 30001 <= int(parts[0]) <= 50000 and parts[4] == 'count': + line_pkts = int(parts[1]) + line_bytes = int(parts[2]) + last_accessed = int(parts[3]) + if parts[7] != 'any': + ip_address = parts[7] + else: + ip_address = parts[9] - if ip_address not in result: - result[ip_address] = {'rule': int(parts[0]), - 'last_accessed': 0, - 'in_pkts': 0, - 'in_bytes': 0, - 'out_pkts': 0, - 'out_bytes': 0 - } - result[ip_address]['last_accessed'] = max(result[ip_address]['last_accessed'], - last_accessed) - if parts[7] != 'any': - # count input - result[ip_address]['in_pkts'] = line_pkts - result[ip_address]['in_bytes'] = line_bytes - else: - # count output - result[ip_address]['out_pkts'] = line_pkts - result[ip_address]['out_bytes'] = line_bytes + if ip_address not in result: + result[ip_address] = {'rule': int(parts[0]), + 'last_accessed': 0, + 'in_pkts': 0, + 'in_bytes': 0, + 'out_pkts': 0, + 'out_bytes': 0 + } + result[ip_address]['last_accessed'] = max(result[ip_address]['last_accessed'], + last_accessed) + if parts[7] != 'any': + # count input + result[ip_address]['in_pkts'] = line_pkts + result[ip_address]['in_bytes'] = line_bytes + else: + # count output + result[ip_address]['out_pkts'] = line_pkts + result[ip_address]['out_bytes'] = line_bytes - return result + return result def add_accounting(self, address): """ add ip address for accounting @@ -158,11 +145,11 @@ class IPFW(object): # add accounting rule if new_rule_id != -1: - devnull = open(os.devnull, 'w') - subprocess.call(['/sbin/ipfw', 'add', str(new_rule_id), 'count', 'ip', 'from', address, 'to', 'any'], - stdout=devnull, stderr=devnull) - subprocess.call(['/sbin/ipfw', 'add', str(new_rule_id), 'count', 'ip', 'from', 'any', 'to', address], - stdout=devnull, stderr=devnull) + subprocess.run(['/sbin/ipfw', 'add', str(new_rule_id), 'count', 'ip', 'from', address, 'to', 'any'], + capture_output=True) + subprocess.run(['/sbin/ipfw', 'add', str(new_rule_id), 'count', 'ip', 'from', 'any', 'to', address], + capture_output=True) + def del_accounting(self, address): """ remove ip address from accounting rules @@ -171,9 +158,7 @@ class IPFW(object): """ acc_info = self.list_accounting_info() if address in acc_info: - devnull = open(os.devnull, 'w') - subprocess.call(['/sbin/ipfw', 'delete', str(acc_info[address]['rule'])], - stdout=devnull, stderr=devnull) + subprocess.run(['/sbin/ipfw', 'delete', str(acc_info[address]['rule'])], capture_output=True) def delete(self, table_number, address): """ remove entry from both ipfw table and accounting rules