pynetwork / pypcap

pypcap - python libpcap module, forked from code.google.com/p/pypcap
Other
299 stars 74 forks source link

Pcap iteration possibly broken #49

Closed rgom closed 6 years ago

rgom commented 7 years ago

I have discovered that when using threading to read packets from multiple files simultaneously, they get corrupted.

I have written the following test (will try to attach the files):

class PktsThread(Thread):
    def __init__(self, name):
        Thread.__init__(self)
        self.name = name
        self.pkts = {}

    def run(self):
        self.pkts = pcap.pcap(relative_file(self.name))

def test_pcap_overwritten():

    for repetitions in range(1000):
        thread_pkts_a = PktsThread('test.pcap')
        thread_pkts_b = PktsThread('arp.pcap')
        thread_pkts_a.start()
        thread_pkts_b.start()
        thread_pkts_a.join()
        thread_pkts_b.join()
        packets_a = [x[1] for x in thread_pkts_a.pkts ]
        packets_a_copy = [t[1] for t in pcap.pcap(relative_file('test.pcap'))]
        # debug only
        # if packets_a != packets_a_copy:
        #    import pdb; pdb.set_trace()
        packets_b = [x[1] for x in thread_pkts_b.pkts ]
        packets_b_copy = [t[1] for t in pcap.pcap(relative_file('arp.pcap'))]

        # two pcaps to check cross-influence (overwriting)
        assert all(packets_a[i][:] == packets_a_copy[i][:] for i in range(len(packets_a)))
        assert all(packets_b[i][:] == packets_b_copy[i][:] for i in range(len(packets_b)))

        # single pcap to check if a single buffer isn't reused
        assert len(set(p[:] for p in packets_a)) > 1
        assert len(set(p[:] for p in packets_b)) > 1

And it always fails. When I replace threaded reading with sequential reading, the errors are gone. Additionally, when removing thread b related stuff, the errors are gone as well.

tests.zip

(Sorry if the tests look excessive - I tried to test other features as well)

The behaviour I have observed on pypcap 1.1.3 on MS Windows 7.

I hope I didn't make a mistake in test code - if the tests are ok, then the issue would be pretty severe (silent data corruption). What led to this test? Some other engineers noted that test code for some applications was failing when using threading + scapy + sniffing live traffic and disabling threading (by removing all but one threads) helped with the issue.

Regards, Robert

guyharris commented 7 years ago

Note that, in versions of libpcap prior to 1.8 - and versions of WinPcap and Npcap based on versions of libpcap prior to 1.8, which means "all versions of WinPcap" - pcap_compile() is not thread-safe, and Weird Stuff can happen if two threads enter pcap_compile() at the same time. This may, or may not, be the cause of what you're seeing.

Reading from two different pcap_ts in two different threads should be safe.

rgom commented 7 years ago

Thank you very much for the feedback!

With that in mind, I performed additional experiments. The results is as follows: threading is not necessary to reproduce the issue. The factors required to observe it are:

Here comes simplified and updated code (I'm not sure if I should reflect that in bug report):

class PacketsClassReader(object):
    def __init__(self, name):
        self.name = name
        self.pkts = {}

    def read_pkts(self):
        self.pkts = pcap.pcap(relative_file(self.name))

def test_pcap_overwritten():

    thread_pkts_a = PacketsClassReader('test.pcap')
    thread_pkts_b = PacketsClassReader('arp.pcap')
    thread_pkts_a.read_pkts()

    # commenting below line makes the problem disappear
    thread_pkts_b.read_pkts()

    # [1] below to access payload only
    packets_a = [x[1] for x in thread_pkts_a.pkts ]

    # this shows the problem:
    packets_a_copy = [x[1] for x in pcap.pcap(relative_file('test.pcap'))]
    # but that doesn't (!!!)
    # p2 = pcap.pcap(relative_file('test.pcap'))
    # packets_a_copy = [x[1] for x in p2]

    # debug only
    if packets_a != packets_a_copy:
        print binascii.b2a_hex(packets_a[0])
        print binascii.b2a_hex(packets_a_copy[0])
        # import pdb; pdb.set_trace()
    assert packets_a == packets_a_copy

And the results are (for the example files):

c:\repos\git\pypcap>python tests/test.py
0002b3056f15000d602dc8610800450000283e7340004006e5490a0001920a00018201bb06840000
0000d47d1e31501400009dcf0000020405b401010402
b8020089bc02000d602dc8610800450000283e7340004006e5490a0001920a00018201bb06840000
0000d47d1e31501400009dcf0000020405b401010402

So payload stays the same, but mac address portion is messed up (src mac address, if I see correctly). I don't know if it's a general rule or just accident, though. Additionally, I don't know if that's the same root cause as in our local live capture observations.

HTH, Robert

nuald commented 7 years ago

I've verified, the aforementioned merge #47 (feature/python3 branch) has fixed the problem. The tests will be added to the test suite.

hellais commented 6 years ago

This should be fixed in the latest release.