patternpythonMinor
Probe request capturing with Scapy
Viewed 0 times
capturingwithrequestscapyprobe
Problem
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+") "+ssid
if args.log:
f.write (str(len(clients))+" "+mac+" ("+macf+") //"+" "+ssid+"\n")
if mac not in mach:
mach.append(mac)
```
#!/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+") "+ssid
if args.log:
f.write (str(len(clients))+" "+mac+" ("+macf+") //"+" "+ssid+"\n")
if mac not in mach:
mach.append(mac)
Solution
Code style
Always mention what exactly you are importing - never use
Be consistent when you are importing:
Also - this is picky but it is more readable to have imports in alphabetical order and not mixed with other code like
String formatting is awful and not-readable:
Instead of:
Use
Or - even more readable:
Much more readable and is actually a standard.
Formatting (indentations) is a bit off - difficult to understand some pieces.
Performance
You have
Same comment - for
Here - remove condition, just leave
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+") //"+" "+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:Code Snippets
from netaddr import *str(len(clients))+" "+mac+" ("+macf+") //"+" <--Probing--> "+ssid+"\n""{0} mac {1}".format(len(clients), macf)"{length} mac {macf}".format(
length=len(clients),
macf=macf,
)mach = []
# this check is expensive
if mac not in mach:Context
StackExchange Code Review Q#88901, answer score: 5
Revisions (0)
No revisions yet.