Probe request capturing with Scapy

Question

This is a probe request packet sniffer using python/scapy. It sniffs Dot11ProbeReq packets and display the ssid, MAC of device and manufacturer name from the probe requests. Can also output the data to a log file if desired. Right now I feel the script is a bit slow and I can see it slow down from the first time I wrote it. It works but I know the code is nowhere as efficient as it could be. Can you point out my mistakes and let me know how I can write more efficient code?

#!/usr/bin/env python
# import all the needed libraries
import sys
from netaddr import *
import logging
logging.getLogger("scapy.runtime").setLevel(logging.ERROR)
from scapy.all import *
from subprocess import *
import datetime
import time
# clear the console
call(["clear"]) 
# set date-time parameters 
today = datetime.date.today() 
d=today.strftime("%d, %b %Y")
tf=time.strftime(" %H:%M")
t=time.strftime(" %H:%M:%S")
# define variables 
clients = [] 
uni = 0
mach = []
manu =[]
# our packet handler 
def phandle(p): 
 global uni 
 if p.haslayer(Dot11ProbeReq): 
 mac = str(p.addr2)
 if p.haslayer(Dot11Elt): 
 if p.ID == 0: 
 ssid = p.info 
 if ssid not in clients and ssid != "":
 clients.append(ssid) 
 maco = EUI(mac)
 macf = maco.oui.registration().org 
 print len(clients),mac+" ("+macf+") <--Probing--> "+ssid
 if args.log:
 f.write (str(len(clients))+" "+mac+" ("+macf+") //"+" <--Probing--> "+ssid+"\n")
 if mac not in mach:
 mach.append(mac)
 uni+=1 
 elif mac not in mach:
 mach.append(mac)
 uni+=1 
# our main function 
if __name__ == "__main__":
 import argparse
 parser = argparse.ArgumentParser(description='PyRobe Help')
 parser.add_argument('interface', action="store", help="specify interface (ex. mon0)", default=False)
 parser.add_argument("-l","--log", dest="log",action="store_true", help="print log file")
 args = parser.parse_args()
 if args.log:
 f = open("ProbeLog"+str(today)+str(tf)+".txt","w") 
 sniff(iface=args.interface,prn=phandle, store=0) 
 print ("\n")
 print "Unique MACs: ",uni
 f.write ("\nUnique MACs: "+str(uni))
 f.write ("\nScan performed on: "+str(d)+" at"+str(t)) 
 f.close() 
 print "Log successfully written. Exiting!"
 else:
 sniff(iface=args.interface,prn=phandle, store=0)
 print "\nSuccessfully Exited! No log file written."

I am basically looking for pointers on:

• Handling command line arguments better
• Calculating and assigning required memory/avoid leaks
• Shortening the code to make it faster to execute

I am not looking for someone to edit my script and spoon feed me the solution, just point me in the right direction!

• My Python version: 2.7.3 (default, Mar 13 2014, 11:03:55) [GCC 4.7.2]
• My linux version: Distributor ID:Kali Description:Kali GNU/Linux 1.1.0 Release:1.1.0 Codename:moto Linux version 3.18.0-kali3-amd64 gcc version 4.7.2 Debian 4.7.2-5) ) #1 SMP Debian 3.18.6-1~kali2 (2015年03月02日)

• Danilo: for simple argument and flag parsing you dont need arg parse, you've already imported sys from which you can call sys.argvwhich return argument variable string which you can then search for specific flags/arguments. It will reduce your syntax code and your memory of code since then you only need a comment that describes your flag and that is it.

Answer 1

Code style

Always mention what exactly you are importing - never use import * - I don't know these libraries and when I read this for the first time I need to find all possible places where these methods can be from: call, sniff. If you want to write code which other people can read - always import exactly what you are using:

from netaddr import *

Be consistent when you are importing: import argparse is imported in main function which is really unnecessary - import on top with other imports.

Also - this is picky but it is more readable to have imports in alphabetical order and not mixed with other code like logging.getLogger("scapy.runtime").setLevel(logging.ERROR)

String formatting is awful and not-readable:

Instead of:

str(len(clients))+" "+mac+" ("+macf+") //"+" <--Probing--> "+ssid+"\n"

Use format method:

"{0} mac {1}".format(len(clients), macf)

Or - even more readable:

"{length} mac {macf}".format(
 length=len(clients),
 macf=macf,
 )

Much more readable and is actually a standard.

Formatting (indentations) is a bit off - difficult to understand some pieces.

Performance

You have mach is list and you are constantly searching in it. Search in list is O(n). Make is set or dict - will speed it up - search performance becomes O(1).

mach = []
# this check is expensive
if mac not in mach:

Same comment - for clients list.

Here - remove condition, just leave else - or maybe your formatting is misleading here and I didn't understand logic correctly:

elif mac not in mach: