Bug 1230373 - Improper use of python's ipaddress
Summary: Improper use of python's ipaddress
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: pypolicyd-spf
Version: 22
Hardware: Unspecified
OS: Unspecified
urgent
urgent
Target Milestone: ---
Assignee: Bojan Smojver
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 1223173 1241863 (view as bug list)
Depends On:
Blocks: 1232595
TreeView+ depends on / blocked
 
Reported: 2015-06-10 17:58 UTC by Nathaniel McCallum
Modified: 2017-02-03 21:57 UTC (History)
9 users (show)

Fixed In Version: pypolicyd-spf-1.3.1-4.fc22
Doc Type: Bug Fix
Doc Text:
Clone Of:
: 1232595 (view as bug list)
Environment:
Last Closed: 2015-12-04 21:20:17 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)

Description Nathaniel McCallum 2015-06-10 17:58:08 UTC
The daemon tries to use either the ipaddr module or the ipaddress module.

...
try:
    import ipaddress
except ImportError:
    import ipaddr as ipaddress
...

However, the ipaddress module codepath contains a bug as reported by user bojan against python-ipaddress: https://admin.fedoraproject.org/updates/FEDORA-2015-8290/python-ipaddress-1.0.7-1.fc21

I believe this is because it presumes that if the ipaddress module is present that the environment is Python 3.x+. This is a bad assumption. The end result is that the daemon breaks when python-ipaddress is installed.

I propose that pypolicyd-spf be fixed and that it depend on either python-ipaddr or python-ipaddress.

Comment 1 Bojan Smojver 2015-06-17 05:53:53 UTC
Not as simple as that, it seems. For instance, this package:
----------------
python-pyspf-2.0.11-1.fc22.noarch
----------------

Has the file:
----------------
/usr/lib/python2.7/site-packages/spf.py
----------------

Which does:
----------------
try:
    # Python standard libarary as of python3.3
    import ipaddress
except ImportError:
    try:
        import ipaddr as ipaddress
    except ImportError:
        print('ipaddr module required: http://code.google.com/p/ipaddr-py/')
----------------

So, essentially the same thing as policyd-spf.

Even if I just import ipaddr inside /usr/libexec/postfix/policyd-spf, that's not sufficient. It still throws a whole lot of exceptions because the python-pyspf is also using the new ipaddress thing and fails. The python-spf package needs the same change to make things work.

And I'm not sure whether that's the whole story...

PS. The comment on the updates page is me. :-)

Comment 2 Bojan Smojver 2015-06-17 06:39:51 UTC
OK, so something like this maybe:
-------------------------------------------
--- policyd-spf.original	2014-10-01 13:06:12.000000000 +1000
+++ policyd-spf	2015-06-17 16:38:06.869298047 +1000
@@ -38,10 +38,16 @@
 if int(micro) < 7:
     raise ImportError("At least pyspf 2.0.7 is required.")
 import policydspfsupp
-try:
-    import ipaddress
-except ImportError:
-    import ipaddr as ipaddress
+if sys.version_info[0] >= 3:
+    try:
+        import ipaddress
+    except ImportError:
+        print('ipaddress module required: http://code.google.com/p/ipaddr-py/')
+else:
+    try:
+        import ipaddr as ipaddress
+    except ImportError:
+        print('ipaddr module required: http://code.google.com/p/ipaddr-py/')
 try:
     import authres
 except:
-------------------------------------------

And:
-------------------------------------------
--- spf.py.original	2014-12-06 03:20:07.000000000 +1100
+++ spf.py	2015-06-17 16:35:40.192512523 +1000
@@ -98,10 +98,13 @@
     from email.message import Message
 except ImportError:
     from email.Message import Message
-try:
+if sys.version_info[0] >= 3:
     # Python standard libarary as of python3.3
-    import ipaddress
-except ImportError:
+    try:
+        import ipaddress
+    except ImportError:
+        print('ipaddress module required: http://code.google.com/p/ipaddr-py/')
+else:
     try:
         import ipaddr as ipaddress
     except ImportError:
-------------------------------------------

Comment 3 Nathaniel McCallum 2015-06-17 12:42:28 UTC
No. That's not right. The package should be able to use the ipaddress module so long as it does not presume Python 3 semantics for the ipaddress module. Properly speaking, I actually think the right thing to do is to remove the dependency on python-ipaddr altogether.

Comment 4 Bojan Smojver 2015-06-17 14:29:26 UTC
I'm not a python guy at all, so I have no idea what I'm doing there. If you have a better approach/patch etc., please feel free.

Comment 5 Nathaniel McCallum 2015-06-17 14:31:07 UTC
Can you get a quick resolution for this problem upstream?

Comment 6 Christian Heimes 2015-06-17 15:43:24 UTC
I agree with Nathaniels statement in comment 3. The code should just use the backport of the Python 3.4 ipaddress module. The Python 2.7 backport is fully compatible, with one exception: for packed IP addresses ('\x7f\x00\x00\x01' for 127.0.0.1) the input must be bytesarray instead of bytes.

I'm currently travelling, I don't have the sources on my laptop and wifi in my hotel room isn't great. Please try something like this for e.g. policyd-spf


import ipaddress
import sys

def _cidrmatch(ip, netwrk):
    """Match connect IP against a CIDR network of other IP addresses."""
    # ipaddress refuses to work on Python 2.x str
    # use this workaround if your code never passed packed IP addresses as str
    if sys.version_info.major < 3:
        if isinstance(ip, str):
            ip = ip.decode('ascii')
        if isinstance(netwrk, str):
            netwrk = netwrk.decode('ascii')
    address = ipaddress.ip_address(ip)
    network = ipaddress.ip_network(netwrk)
    return address in network

print(_cidrmatch(u'192.168.1.15', u'192.168.0.0/22'))
print(_cidrmatch('192.168.1.15', '192.168.0.0/22'))

Comment 7 Bojan Smojver 2015-06-18 03:55:50 UTC
(In reply to Christian Heimes from comment #6)
> The Python 2.7 backport is
> fully compatible, with one exception: for packed IP addresses
> ('\x7f\x00\x00\x01' for 127.0.0.1) the input must be bytesarray instead of
> bytes.

Again, not a python guy, but can't someone just overload that method to take both? Or is this something python cannot do?

Comment 8 Nathaniel McCallum 2015-06-18 03:58:49 UTC
No. This is a language difference between Python 2.x and Python 3.x.

If you're not able to fix the package code itself, you should:
1. File an upstream bug.
2. Provide a link to the upstream bug in this bug.
3. Add a Conflicts statement to your package until it works properly.

Comment 9 Bojan Smojver 2015-06-18 04:01:29 UTC
OK, I'll give patch a along the lines of comment #6 a try. However, python-pyspf will also need to be fixed in a similar way before any of this can work.

Comment 10 Bojan Smojver 2015-06-18 05:04:49 UTC
I'm building the new packages now, but the update will have to wait for python-pyspf fix (i.e. fix of bug #1232595).

Comment 11 Bojan Smojver 2015-06-28 21:34:30 UTC
*** Bug 1223173 has been marked as a duplicate of this bug. ***

Comment 12 Bojan Smojver 2015-07-10 09:36:59 UTC
*** Bug 1241863 has been marked as a duplicate of this bug. ***

Comment 13 Alex Regan 2015-10-01 00:45:37 UTC
It appears bug #1232595 has been resolved. Is there any update for policyd-spf available for fc22 yet?

Comment 14 Bojan Smojver 2015-10-01 22:20:57 UTC
(In reply to Alex Regan from comment #13)
> It appears bug #1232595 has been resolved. Is there any update for
> policyd-spf available for fc22 yet?

That other bug is not resolved yet.

Comment 15 Adam Williamson 2015-11-05 22:13:50 UTC
This is really pretty bad: if you have a server configured with pypolicyd-spf and you upgrade to 23 (or 22?) email delivery will silently break! I only just realized that my server's been rejecting mail to happyassassin.net since I upgraded it on Sunday...

Comment 16 Bojan Smojver 2015-11-05 22:53:05 UTC
(In reply to awilliam from comment #15)
> This is really pretty bad: if you have a server configured with
> pypolicyd-spf and you upgrade to 23 (or 22?) email delivery will silently
> break! I only just realized that my server's been rejecting mail to
> happyassassin.net since I upgraded it on Sunday...

Yes, I know. I've been trying to get the maintainer of the other package to release a build with a fix, but to no avail.

BTW, the workaround on F-22 for me was to remove python-ipaddress and just leave python-ipaddr.

Comment 17 Adam Williamson 2015-11-05 22:54:20 UTC
sadly I can't do that as freeipa-client requires it :( (my servers are all members of my freeipa domain)

Comment 18 Bojan Smojver 2015-11-05 22:59:55 UTC
(In reply to awilliam from comment #17)
> sadly I can't do that as freeipa-client requires it :( (my servers are all
> members of my freeipa domain)

I'll try to obtain commit access to that other package later on today to fix this. Have to do some other things in the meantime, but hopefully we get this fixed soon.

Comment 19 Bojan Smojver 2015-11-06 06:03:10 UTC
(In reply to awilliam from comment #17)
> sadly I can't do that as freeipa-client requires it :( (my servers are all
> members of my freeipa domain)

I didn't get commit access yet, but I just attached a package patch to bug #1232595. If you get that, clone the python-pyspf repo, apply the patch and rebuild, you'll get a package that works (at least it did for me).

https://bugzilla.redhat.com/attachment.cgi?id=1090446

Comment 20 Bojan Smojver 2015-11-26 02:42:28 UTC
(In reply to awilliam from comment #15)
> This is really pretty bad

Still didn't get commit access to that other package (python-pyspf) to fix it and there was no other action in that bug. If you have have The Force with these things, please feel free to wave your hand now... :-)

Comment 21 Peter Pindsle 2015-11-26 14:18:02 UTC
Bojan, firstly, thanks for creating the patch.  Since it looks like we might still be waiting a while for an official fix, any chance of elaborating on your instructions above so I can patch my F23 install?

Comment 22 Adam Williamson 2015-11-26 19:55:34 UTC
I can do it, but I'd just like to be clear: your patch appears to make unnecessary/cosmetic changes to pyspf-2.0.11-python2.patch , but the only practical change is to make python-pyspf depend on python-ipaddress instead of python-ipaddr. Is that what it's intended to do, and you're sure that solves the problem?

Comment 23 Bojan Smojver 2015-11-26 20:47:36 UTC
(In reply to awilliam from comment #22)
> I can do it, but I'd just like to be clear: your patch appears to make
> unnecessary/cosmetic changes to pyspf-2.0.11-python2.patch , but the only
> practical change is to make python-pyspf depend on python-ipaddress instead
> of python-ipaddr. Is that what it's intended to do, and you're sure that
> solves the problem?

It is not a cosmetic change. It removes attempts to use ipaddr as ipaddress, which, if successful, causes the failures. Instead, it just uses ipaddress directly.

Comment 24 Bojan Smojver 2015-11-26 20:49:24 UTC
(In reply to Peter Pindsle from comment #21)
> Bojan, firstly, thanks for creating the patch.  Since it looks like we might
> still be waiting a while for an official fix, any chance of elaborating on
> your instructions above so I can patch my F23 install?

Clone python-pyspf git repository (fedpkg clone python-pyspf). Apply the patch to checked out directory. Build locally (fedpkg local).

Comment 25 Adam Williamson 2015-11-26 20:50:54 UTC
Bojan: huh, that's odd - when I looked at the patch a couple of hours back it did not have all the changes in it that it has now. Strange.

Comment 26 Adam Williamson 2015-11-26 20:51:57 UTC
I'd certainly like to see a change like this submitted upstream too, btw. It's not a good thing to carry downstream in the long term.

Comment 27 Adam Williamson 2015-11-26 21:58:05 UTC
I just submitted F22 and F23 updates with Bojan's patch.

Comment 28 Peter Pindsle 2015-11-27 11:35:53 UTC
(In reply to Bojan Smojver from comment #24)
> (In reply to Peter Pindsle from comment #21)
> > Bojan, firstly, thanks for creating the patch.  Since it looks like we might
> > still be waiting a while for an official fix, any chance of elaborating on
> > your instructions above so I can patch my F23 install?
> 
> Clone python-pyspf git repository (fedpkg clone python-pyspf). Apply the
> patch to checked out directory. Build locally (fedpkg local).

Thanks Bojan, I instead downloaded the RPM from https://bugzilla.redhat.com/show_bug.cgi?id=1232595 and confirmed that it works for me

Comment 29 Fedora Update System 2015-11-30 20:28:35 UTC
pypolicyd-spf-1.3.1-4.fc22 has been submitted as an update to Fedora 22. https://bodhi.fedoraproject.org/updates/FEDORA-2015-87253fc4b9

Comment 30 Fedora Update System 2015-12-03 16:03:18 UTC
pypolicyd-spf-1.3.1-4.fc22 has been pushed to the Fedora 22 testing repository. If problems still persist, please make note of it in this bug report.
If you want to test the update, you can install it with
$ su -c 'dnf --enablerepo=updates-testing update pypolicyd-spf'
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-87253fc4b9

Comment 31 Fedora Update System 2015-12-04 21:20:12 UTC
pypolicyd-spf-1.3.1-4.fc22 has been pushed to the Fedora 22 stable repository. If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.