Reading WLAN packets

Question

The script reads WLAN packets, looks for probe requests and then extracts SSID, MAC address and then stores them in a list if the MAC address/SSID combo is unique.

if p.haslayer(Dot11ProbeReq):
    if pc == 0:
      print O+'[+]'+W+' Got first packet! Many more to go...\n'                         
        if p.haslayer(Dot11Elt):
          pc +=1
          vali = 0                            
            if p.ID == 0: 
              ssid = p.info
                if ssid != "":
                  for (i,j) in obs:
                    if (i,j) != (p.addr2,ssid):
                        vali += 1
                    else:
                        break
                    if vali == len(obs):
                        obs.append((p.addr2,ssid))
                        checkmac(p.addr2)
                        checknet(ssid)
                        count +=1
                        get_oui(p.addr2)
                        print str(count)+'>',p.addr2+' ('+G+macf+W+') <--Probing--> '+O+ssid+W
                        wr_log(p.addr2,ssid,macf)   
                    else:
                            pass

What modifications can I make to make the code faster, specially the for block and the validation process? Any other optimizations or practices that I should be following?

This script is reliant on the scapy library. Here p is a packet that scapy intercepts. It could be any type of packet - management, data or control etc. and can be of any subtype - deauthentication, probe response, authentication, probe request etc. The function that I have mentioned above reads each packet p and processes it the way I desire. The code begins with intercepting any packet p from the air and checking if the packet is a probe request. A probe request is emitted by all wifi enabled devices when they are looking to connect to a wifi network if they can't find one or even if they are connected, then to find a backup network in case the currently connected network fails. These probe requests are automatic and the users don't get to choose anything about them. The devices will emit these requests broadcasting a list of known wireless networks to check if there is any of them available nearby. After the packet is confirmed to be a probe request, the packet counter (pc) checks if it is the first packet, if it is then it displays a message confirming that we are able to receive requests and the network is up. Then the function checks whether the packet has a Dot11Elt layer, which is the information element that contains all the info about the device and previously connected wireless networks. If it does have an information layer then the value in the first information layer, which happens to be the ssid of a previously connected network, is stored in a variable called ssid. the variable p.addr2 is the MAC address of the device that the packet came from. Each MAC/SSID pair found is stored in an orderly fashion inside the list obs. Each packet intercepted, as many can be duplicates, are checked against the ones already stored in obs and if unique against each pair already stored, then the value of the vali (validation counter) is incremented. Needless to say, if the value of vali is the same as the length of obs then the packet is unique and did not match any other (MAC/SSID) pair stored in obs and is hence, printed. Before printing, the unique (MAC/SSID) pair is appended to the list obs and the functions checkmac() and checknet() check whether the MAC address or the SSID is unique, if they are then a counter is incremented and that helps us to identify unique devices and unique networks after the script stops running. The function get_oui() gets the manufacturer name though the use of netaddr library for each and every MAC address we find and wr_log() writes the MAC address, SSID name and manufacturer info in a log file.

Answer 1

Your variable names are quite hard to make sense of. I had to read your explanatory text to figure out what vali was supposed to do; I still have no idea what obs means. Judging by the line print O+'[+]'+W+' Got first packet! Many more to go...\n', it also looks like you might have a variable called "O" (capital letter oh), which most style guides advise against because it's hard to distinguish it from 0 (the number zero). I would use longer, more descriptive names. Write out validation or valid_pairs instead of using vali. Write packet and packet_counter instead of p and pc. I could have written my first sentence as "ur vrbl nms r q hd 2 mk sns of", and you might have been able to understand it, but ovssly ts mch mr dfclt 2 ndrstnd if i wrt lk ths. Apply the same principle to your code.

Spaces between your string concatenation operators also make code easier to read: print O + '[+]' + W + ' Got first packet! Many more to go...\n' is easier to read than the version with everything all scrunched up. In this case, though, I would prefer format strings to using concatenation: print "{} [+] {} Got first packet!".format(O, W). (Or the percent-sign version, if you're stuck on a version of Python before 2.6.)

Your code is rather deeply nested, which makes it harder to read and understand overall. The Zen of Python says flat is better than nested, and while I don't always agree with the Zen, in this case I think it's right. To relieve this problem, I would make some functions. Pick out some nested blocks and drop them into their own functions, passing in whatever arguments they need, so your code is flatter and easier to read and understand. At the very least, your entire block under if p.haslayer(Dot11Elt) could be its own function.

---

There are improvements both big and small that could be made to your for loop. It looks like you could do the same thing you currently do in a lot less code, like this:

if (p.addr2, ssid) not in obs:
    obs.append((p.addr2,ssid))
    checkmac(p.addr2)
    checknet(ssid)
    count +=1
    get_oui(p.addr2)
    print str(count)+'>',p.addr2+' ('+G+macf+W+') <--Probing--> '+O+ssid+W
    wr_log(p.addr2,ssid,macf)

This does a linear search of obs for the tuple (p.addr2, ssid); it checks every element in obs to see if it's equal to (p.addr2, ssid) as your current code does.

But there are faster ways to do this. I would probably rethink the way obs is stored. Right now, you're storing the valid pairs of MAC addresses and SSIDs as a list, so in order to find duplicates, you have to do a list search. You could speed this up by sorting the list and using a binary search, but you can speed it up even more by bringing a set into the equation.

It wasn't clear to me from your explanation whether you need obs to be in order or not. If you don't need it in order, you can just switch from a list to a set:

if (p.addr2, ssid) not in obs_set:
    obs_set.add((p.addr2, ssid))
    # other stuff

If you do need it in order, you can still use a set to speed this particular operation up, but unfortunately, there's no ordered set in Python. The Stack Overflow question Does Python have an ordered set? gives several ways you can mimic one. Another option: you could just keep your current list, but also add everything to a set. Maybe even wrap it in a class so you don't have to remember to do both. It's sort of hacky, and it takes more memory, but it's simple, quick to implement, avoids adding more dependencies, and is much easier than implementing your own red-black tree. (I consider an OrderedDict with all the values as True to be kind of hacky and a waste of memory too.)