How can I contribute to py-pf? Port to python 3?

[PiRAT4]

Hey Daniele,

Note: not an actual issue.

Hope you are doing good.
First of all, thank you for the time and effort you put into py-pf. It really makes life a little more bearable for newbies like me. :>
I'm currently having fun with flask and py-pf on my free time. Working on a Web GUI for pf. So far, so good. Neat. 👍

I noticed that you don't update py-pf often lately (your last commit is recent but the one before that was from 2019). I did test it from OpenBSD 6.6 to 6.9 and everything worked as expected (at least from what I tested).

I would like to contribute to this project. What can I do? From the top of my mind, one thing that could be done is to port your code to python 3 (flask will drop python 2.7 and 3.5 support soon for example). Would this be a good first PR?

Care to share some lights on where py-pf can be improved? What would you think should be addressed first? I really don't have any clue but I'm willing to learn. :>

Thank you and have a nice week.
Cheers,

[dotpy]

Hi PiRAT4,
Thanks a lot for your interest in py-pf.
Yes, I think the most urgent improvement for py-pf is porting it to Python3; it's something I've been postponing for too long.
I think the most boring part would be all the strings-to-bytes and bytes-to-strings conversions...
I would really appreciate your help on this task very much! :-)
Best regards,
Daniele

[PiRAT4]

Thanks for your feedback. Need to read some documentation before I start (new to python). :)

[PiRAT4]

Hi @dotpy,

I'm a bit stuck with string to bytes conversion.
How do you suggest that I approach this?

Getting this kind of error:

Traceback (most recent call last):
  File "/home/pirata/py-pf/pf/tests/test_filter.py", line 20, in setUp
    "ruleset": self.pf.get_ruleset()}
  File "/home/pirata/py-pf/pf/filter.py", line 497, in get_ruleset
    for rule in self._get_rules(path, d, clear):
  File "/home/pirata/py-pf/pf/filter.py", line 465, in _get_rules
    pr = pfioc_rule(anchor=path)
TypeError: expected bytes, str found

Btw, why is DIOCGETRULESET commented out? pfioc_ruleset is not defined.
Would be something like this?

PF_ANCHOR_NAME_SIZE  = 64              # From /usr/include/net/pfvar.h
PF_ANCHOR_MAXPATH    = PATH_MAX - PF_ANCHOR_NAME_SIZE - 1

class pfioc_ruleset(Structure):        # From /usr/include/net/pfvar.h
    _fields_ = [("nr",                 c_uint32),
                ("path",               c_char * PATH_MAX),
                ("name",               c_char * PF_ANCHOR_MAXPATH)]

# TODO: pf_ruleset?

Sorry if this is a dumb question.

Thank you.
Have a nice week.
Ricardo

[dotpy]

Hi @PiRAT4 ,

Thanks for your effort, I know those string-to-bytes conversions are the most annoying thing to take care of when porting py-pf to Python3... :-(

I've made a new branch (py3) with a few changes to get to the point where you were stuck.
I think there are two ways to approach this:

• leave to the user the responsibility of converting strings and only pass bytes to py-pf;
• keep the conversion "hidden" from the user and do it internally whenever a string has to be inserted in a C-like struct;

I prefer the second strategy, so I've made this commit with a few conversion examples, such as:

diff --git a/pf/filter.py b/pf/filter.py
index 7188db0..55231f6 100644
--- a/pf/filter.py
+++ b/pf/filter.py
@@ -117,7 +117,7 @@ class _PFTrans(object):
 
         for a, t in zip(self.array, trans_type):
             a.type = t
-            a.anchor = path
+            a.anchor = path.encode()
 
         self._pt = pfioc_trans(size=self.size, esize=sizeof(pfioc_trans_e),
                                array=addressof(self.array))
@@ -458,7 +458,7 @@ class PacketFilter(object):
         if path.endswith("/*"):
             path = path[:-2]
 
-        pr = pfioc_rule(anchor=path)
+        pr = pfioc_rule(anchor=path.encode())
         if clear:
             pr.action = PF_GET_CLR_CNTR
 
@@ -606,7 +606,7 @@ class PacketFilter(object):
         io = pfioc_table(pfrio_esize=sizeof(pfr_table))
 
         if filter is not None:
-            io.pfrio_table = pfr_table(pfrt_anchor=filter.anchor)
+            io.pfrio_table = pfr_table(pfrt_anchor=filter.anchor.encode())

I think this is the best way to go...

WRT the second question: DIOCGETRULESET, as well as a few other syscalls, is commented out simply because it's not used by py-pf.
All syscalls were initially commented out in py-pf, and were progressively uncommented as they were being implemented in py-pf; they acted like a kind of "to-do" list. :-D

[PiRAT4]

Hey @dotpy,

Thank you for your help. I really appreciate it. :)
I agree that the 2nd option is better but do we need to this manually to all strings or can we create a general function? Something similar to this?

def _to_bytes(self):
    if type(self) is str:
        return codecs.encode(self, 'utf-8')
    elif type(self) is bytes:
        return self
    else:
        raise TypeError("Expected bytes or string but got %s instead." % type(self))

OK, I fixed some syntax and added your latest commit https://github.com/PiRAT4/py-pf/commit/690c7f58afcaef83f6e0d35fed048e1acb3b421b.

I'll keep trying to fix these type errors. I probably need to read more documentation.

EEEEEEEEEEEEEEEEEEEEEEEE
======================================================================
ERROR: test_add_addrs (pf.tests.test_filter.TestPacketFilter)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/pirata/py-pf/tests/test_filter.py", line 20, in setUp
    "ruleset": self.pf.get_ruleset()}
  File "/home/pirata/py-pf/filter.py", line 500, in get_ruleset
    for rule in self._get_rules(path, d, clear):
  File "/home/pirata/py-pf/filter.py", line 463, in _get_rules
    if path.endswith("/*"):
TypeError: endswith first arg must be bytes or a tuple of bytes, not str

OK, that was my first thought when I saw all those constants commented out. I guess I could help with you that too later on. :)
Thank you and have a nice weekend.
Ricardo

[dotpy]

Hi @PiRAT4,

I don't think we need a generic function like _to_bytes(), the idea is simpler than that: every time a c_char * is accessed, it must be decoded to a string, and every time a string is assigned to a member of a structure, it must be bytes-encoded.

I've made a few more changes to get the same error you're hitting, but I couldn't, so I tried to fix it anyway.

Now the tests pass, but I must have missed some conversions that might not be triggered by tests.
I think now we could do a grep c_char _struct.py and check that all those c_char * are correctly converted in the code.

Cheers,
Daniele