to capture packets with options like -i interface, -o open file, etc using pyshark and python

Question

review my code if possible. I am making network packet capture using pyshark and python. if possible give me some feedback about the code and help to make it better.

!/usr/bin/python

import argparse
import pyshark
import time
import re as regex


parser = argparse.ArgumentParser()
parser.add_argument('-i', '--interface', metavar=" ", type=str, required = True, help = 'To specify the interface ')
parser.add_argument('-v', '--verbose', required = False, action = 'store_true', help = 'To print the all layer of packet')
parser.add_argument('-o', '--output', metavar=' ', help = 'To capture and save the pcap in a file')
parser.add_argument('-p', '--protocol', metavar=' ', help= 'To capture packet using ptotocl filter')
parser.add_argument('-u', '--udp', action = 'store_true', help = 'To capture udp packet only')
parser.add_argument('-t', '--tcp', action = 'store_true', help = 'To capture tcp packet only')
parser.add_argument('-c', '--count', metavar=' ',default=1,  help = 'To capture limited number of packet')

args = parser.parse_args()

if args.protocol:
   capture = pyshark.LiveCapture(interface=args.interface, display_filter=args.protocol)


elif args.udp:
   capture = pyshark.LiveCapture(interface=args.interface, bpf_filter='udp')

elif args.tcp:
   capture = pyshark.LiveCapture(interface=args.interface, bpf_filter='tcp')

else:
   capture = pyshark.LiveCapture(interface=args.interface, output_file=args.output)
   capture.sniff(packet_count = args.count)

packet_list = []

for packet in capture.sniff_continuously():
    if 'IP Layer' in str(packet.layers):
        protocol = regex.search(r'(Protocol:)(.*)',str(packet.ip))
        protocol_type = protocol.group(2).strip().split(' ')[0]
       # proto = protocol_type
    localtime = time.asctime(time.localtime(time.time())) 
    proto = protocol_type
    src_addr = packet.ip.src
    dst_addr = packet.ip.dst
    length = packet.length
    print (localtime, '\t' , proto, '\t' ,src_addr, '\t', dst_addr, '\t' , length)
    if args.verbose:
       print(packet.show())

```

J_H · Accepted Answer · 2023-02-06 06:59:13Z

Nice, I'm glad you use pyshark so much that it's useful to customize it a bit.

High level advice: Use functions! Everything here is at module level, and we could reduce global variable coupling by pushing most of it down into various functions.

!/usr/bin/python

nit, typo: I'm sure you meant #!/usr/....

Consider leaving it as just #! python, to exploit $PATH in the case where a venv is active. I usually write #! /usr/bin/env python, so that even crazy and ancient operating systems will obey $PATH.

import re as regex

Wow, can't say I've ever seen aliasing used to make a module's name longer. More power to you, if you find it clearer.

args = parser.parse_args()

The whole parser thing is lovely, you're using it in exactly the right way. But notice that it took seven lines to add the various arguments, and I'm sure another one will be added next month. So I usually banish such a stanza into a helper, and then assign

args = get_parser().parse_args()

Then again, this whole thing plus the following really belongs within a def main():, so variables like parser don't wind up polluting the top level global namespace.

if args.protocol:

Sooo, I just encouraged you to write a helper. But now I'm going to about-face on that and say parser is a lovely builtin library, but $ pip install typer is even better.

https://pypi.org/project/typer

I used to create arg parser objects all the time. But nowadays we have optional type annotations, and it's much more straightforward to write this:

def main(interface: str, protocol: str, verbose: bool = False):
    ...

if __name__ == "__main__":
    typer.run(main)

and then instead of args.protocol just reference protocol.

(I left out some args in the example, and maybe help text is important. You decide.)

packet_list = []

Looks like a leftover unused variable that you meant to delete before submitting snapshot for review.

If you do intend to keep it, consider renaming to plural packets to indicate it's a container, avoiding the clunky _list suffix.

Thank you for the r-string here, it's a good habit to be in, kudos.

        protocol = regex.search(r'(Protocol:)(.*)', str(packet.ip))

This is within a loop. We are re-compiling a regex each time through, but the source string is a constant so the regex won't be changing. Recommend you hoist the constant expression outside of the loop. It's no big deal one way or the other.

    protocol_re = regex.compile(r'(Protocol:)(.*)')
    for packet in ... :
        ...
        protocol = protocol_re.search(str(packet.ip))

Two more small items:

nit: No need for a capturing group on that constant Protocol: text.

Prefer r'^Protocol:(.*)', if it appears at start of line. That way on a mismatched line we're not scanning the whole thing for capital P. The ^ caret anchor lets the regex engine abandon immediately if the line doesn't start with Protocol.

Overall?

Functions are your friend. Use them. Write small blocks of code that are visible all-at-once in a single window with no scrolling. More code than that? Break it up!

Remaining issues are minor.

LGTM, ship it!

Stack Exchange Network

to capture packets with options like -i interface, -o open file, etc using pyshark and python

1 Answer 1

You must log in to answer this question.

Hot Network Questions

to capture packets with options like -i interface, -o open file, etc using pyshark and python

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions