noxrepo / pox

The POX network software platform
https://noxrepo.github.io/pox-doc/html/
Apache License 2.0
619 stars 470 forks source link

Python 2/3 compatiblity fix on dns decoder #288

Closed Emzi0767 closed 1 year ago

Emzi0767 commented 1 year ago

This commit introduces a compatibility fix in the dns packet decoder. This prevents the output from being spammed by failures to decode DNS requests (and presumably responses). The fix is tested under Python 3.9.2 on Debian 11, but steps were taken to retain Python 2 compatibility as well.

The original problems stems from how Python 2 and 3 handle bytes objects differently - Python 2 seems to just handle them as str objects, whereas under python 3 they are bytes. This obviously leads to issues where conversion happen - in this case bytes to int and str. Whereas under Python 2, ord($OBJ) is required to convert a byte to int, under Python 3 it is sufficient to just use the byte as int. Similarly, sliced bytestring under Python 2 is already a str object, whereas in Python 3 it requires .decode()ing first (in Python 2 you can decode a str object, but it yields a unicode object, which might not be desirable).

Emzi0767 commented 1 year ago

I have also now tested the changes under Python 3.10 on Ubuntu 22.04 LTS.

MurphyMc commented 1 year ago

Can you check whether this bug still exists in the current development branch (halosaur)? I think it's probably fixed.

Emzi0767 commented 1 year ago

Quick glace shows the path to have been updated similar to my changes (sans retaining py2 compat). One thing I suspect is https://github.com/noxrepo/pox/blob/20d291825de5e1038e4500382e842e4f95b5731a/pox/lib/packet/dns.py#L401 due to retlist being a str list in the past. It appears to be a bytes list now. I will properly test this in a couple hours.

Emzi0767 commented 1 year ago

I ran a more proper test, and it appears the issue is indeed resolved in halosaur.

MurphyMc commented 1 year ago

Awesome; thanks for checking!