HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Probe request capturing with Scapy

Submitted by: @import:stackexchange-codereview··
0
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)

Solution

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+") //"+"  "+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.