sdgathman / pyspf

Other
49 stars 26 forks source link

Getting list of ip networks fails if prefix is 32 or more #24

Closed shay-cyesec closed 4 years ago

shay-cyesec commented 4 years ago

When using check2 in python3 using the builtin ipaddress, with i='list', if the spf record has a network of prefix 32 or more, the code crashes. my code: `q = spf.query(i='list', s='office@microsoft.com', h=None, local=None, receiver=None, timeout=spf.MAX_PER_LOOKUP_TIME, verbose=False, querytime=20)

print(q.check())`

Traceback of the error:

Traceback (most recent call last): File "/home/shay/PycharmProjects/testing/venv/lib/python3.8/site-packages/spf.py", line 1433, in cidrmatch self.iplist.append(network.ip) AttributeError: 'IPv4Network' object has no attribute 'ip'

During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/home/shay/PycharmProjects/testing/sp.py", line 7, in print(q.check()) File "/home/shay/PycharmProjects/testing/venv/lib/python3.8/site-packages/spf.py", line 600, in check rc = self.check1(spf, self.d, 0) File "/home/shay/PycharmProjects/testing/venv/lib/python3.8/site-packages/spf.py", line 639, in check1 return self.check0(spf, recursion) File "/home/shay/PycharmProjects/testing/venv/lib/python3.8/site-packages/spf.py", line 924, in check0 res, code, txt = self.check1(d,arg, recursion + 1) File "/home/shay/PycharmProjects/testing/venv/lib/python3.8/site-packages/spf.py", line 639, in check1 return self.check0(spf, recursion) File "/home/shay/PycharmProjects/testing/venv/lib/python3.8/site-packages/spf.py", line 959, in check0 if self.cidrmatch([arg], cidrlength): break File "/home/shay/PycharmProjects/testing/venv/lib/python3.8/site-packages/spf.py", line 1435, in cidrmatch for netwrk in [ipaddress.IPNetwork(ip,strict=False) for ip in ipaddrs]: File "/home/shay/PycharmProjects/testing/venv/lib/python3.8/site-packages/spf.py", line 1435, in for netwrk in [ipaddress.IPNetwork(ip,strict=False) for ip in ipaddrs]: AttributeError: module 'ipaddress' has no attribute 'IPNetwork'

This is because of line 1433 in spf.py: self.iplist.append(network.ip) which uses non existing ip attirbute for netwrok in the ipaddress python3 library.

proposed fix: change to network.exploded

kitterma commented 4 years ago

Thanks. I can confirm the problem and I've added a test that replicates it.

sdgathman commented 4 years ago

Would like to have a "quantum" mode for i='list' that both matches and does not match. Right now, it never matches, which is a useful heuristic.

kitterma commented 4 years ago

I guess not crashing would be a nice first step.

sdgathman commented 4 years ago

It is broken in python3. The problem is that ipaddress has ip_network whereas ipaddr (py2) has IPNetwork. Should we ditch or fork python2 at this point?

Fedora has dropped python2-ipaddress and python2-ipaddr as of Fedora 32.

kitterma commented 4 years ago

Debian has dropped them as well. I have no problem at all with dropping support for python2. It's no longer supported by the Python developers, so I don't think we need to do so for future development.

sdgathman commented 4 years ago

I think I will drop support for ipaddr. If you must run py2, then python2-ipaddress can be installed from older versions of Fedora. EL6 dies in Nov 2020. EL7 has python3.6. So python3.6 is my new minimum target. But there is no point in throwing away already existing py2 compatibility without a good reason. Therefore, I will drop support for ipaddr.

kitterma commented 4 years ago

Does ipaddress on python2 work? I haven't tried it with pyspf, but in other contexts I have run into huge UTF-8 issues because ipaddress assumes a UTF-8 environment that python2 doesn't reliably provide.

sdgathman commented 4 years ago

A hacked ipaddress on python2 works. Such as released on Fedora. For instance, the hacked ipaddress provides a Bytes class, which I import on python2.

sdgathman commented 4 years ago

I pushed dropping ipaddr support to master. This fixes this issue for python3. Python2 and ipaddr support can continue in another branch as suggested by @kitterma.

kitterma commented 4 years ago

OK. As long as it works, I have no objection to dropping ipaddr.

sdgathman commented 4 years ago

At this point, hacking on ipaddress for python2 would be easier than try to match the not quite the same APIs.

sdgathman commented 4 years ago

@shay-cyesec The list feature is not a full implementation, but a quick heuristic that works by always not matching and listing the ips not matched. A full implementation would keep a list for each result, and make recursive calls for both matching and not matching.

evgeni commented 3 years ago

FWIW, tests still fail on Python 3.7 and 3.8 for me with:

File "/home/evgeni/Devel/pyspf/.tox/py38/lib/python3.8/site-packages/spf.py", line 576, in spf.query.check
Failed example:
    r.check()
Expected:
    ('fail', 550, 'SPF fail - not authorized')
Got:
    ('none', 250, '')