| Message ID | 20200424145941.242949-1-plautrba@redhat.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | python/semanage: Use ipaddress module instead of IPy | expand |
> -----Original Message----- > From: selinux-owner@vger.kernel.org [mailto:selinux-owner@vger.kernel.org] > On Behalf Of Petr Lautrbach > Sent: Friday, April 24, 2020 10:00 AM > To: selinux@vger.kernel.org > Cc: Petr Lautrbach <plautrba@redhat.com> > Subject: [PATCH] python/semanage: Use ipaddress module instead of IPy > > ipaddress python module was added to standard library in Python 3.3 - > https://docs.python.org/3/library/ipaddress.html > > seobject.py was the only consumer of IPy module so this dependency is not > needed anymore. > > Signed-off-by: Petr Lautrbach <plautrba@redhat.com> > --- > python/semanage/seobject.py | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/python/semanage/seobject.py b/python/semanage/seobject.py > index f2a139c970bd..c89c67e491b6 100644 > --- a/python/semanage/seobject.py > +++ b/python/semanage/seobject.py > @@ -32,7 +32,7 @@ from semanage import * PROGNAME = "policycoreutils" > import sepolicy > import setools > -from IPy import IP > +import ipaddress > > try: > import gettext > @@ -1860,13 +1860,13 @@ class nodeRecords(semanageRecords): > > # verify valid combination > if len(mask) == 0 or mask[0] == "/": > - i = IP(addr + mask) > - newaddr = i.strNormal(0) > - newmask = str(i.netmask()) > - if newmask == "0.0.0.0" and i.version() == 6: > + i = ipaddress.ip_network(addr + mask) > + newaddr = str(i.network_address) > + newmask = str(i.netmask) > + if newmask == "0.0.0.0" and i.version == 6: > newmask = "::" > > - protocol = "ipv%d" % i.version() > + protocol = "ipv%d" % i.version > > try: > newprotocol = self.protocol.index(protocol) > -- > 2.26.0 LGTM <Reviewed-by: William.c.Roberts@intel.com> I can give it an acked by if you give me testing steps.
On Fri, Apr 24, 2020 at 9:46 PM Roberts, William C <william.c.roberts@intel.com> wrote: > > > -----Original Message----- > > From: selinux-owner@vger.kernel.org [mailto:selinux-owner@vger.kernel.org] > > On Behalf Of Petr Lautrbach > > Sent: Friday, April 24, 2020 10:00 AM > > To: selinux@vger.kernel.org > > Cc: Petr Lautrbach <plautrba@redhat.com> > > Subject: [PATCH] python/semanage: Use ipaddress module instead of IPy > > > > ipaddress python module was added to standard library in Python 3.3 - > > https://docs.python.org/3/library/ipaddress.html > > > > seobject.py was the only consumer of IPy module so this dependency is not > > needed anymore. > > > > Signed-off-by: Petr Lautrbach <plautrba@redhat.com> > > --- > > python/semanage/seobject.py | 12 ++++++------ > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/python/semanage/seobject.py b/python/semanage/seobject.py > > index f2a139c970bd..c89c67e491b6 100644 > > --- a/python/semanage/seobject.py > > +++ b/python/semanage/seobject.py > > @@ -32,7 +32,7 @@ from semanage import * PROGNAME = "policycoreutils" > > import sepolicy > > import setools > > -from IPy import IP > > +import ipaddress > > > > try: > > import gettext > > @@ -1860,13 +1860,13 @@ class nodeRecords(semanageRecords): > > > > # verify valid combination > > if len(mask) == 0 or mask[0] == "/": > > - i = IP(addr + mask) > > - newaddr = i.strNormal(0) > > - newmask = str(i.netmask()) > > - if newmask == "0.0.0.0" and i.version() == 6: > > + i = ipaddress.ip_network(addr + mask) > > + newaddr = str(i.network_address) > > + newmask = str(i.netmask) I was wondering whether there was a change of behavior with addresses that are not network masks, such as 10.0.0.1/24 (and why ipaddress.ip_network is used instead of ipaddress.ip_interface). But it seems that "semanage node" already only accepts valid network masks: Before: >>> IP('10.0.0.1/24').netmask() Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/usr/lib/python3.8/site-packages/IPy.py", line 260, in __init__ raise ValueError("%s has invalid prefix length (%s)" % (repr(self), self._prefixlen)) ValueError: IP('10.0.0.1/24') has invalid prefix length (24) After: >>> ipaddress.ip_network('10.0.0.1/24') Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/usr/lib/python3.8/ipaddress.py", line 74, in ip_network return IPv4Network(address, strict) File "/usr/lib/python3.8/ipaddress.py", line 1454, in __init__ raise ValueError('%s has host bits set' % self) ValueError: 10.0.0.1/24 has host bits set So this change looks good to me :) Nevertheless the comment can be improved, from "# verify valid combination" to "verify that (addr, mask) is either a IP address (without a mask) or a valid network mask", for example. > > + if newmask == "0.0.0.0" and i.version == 6: > > newmask = "::" What does this check do? It seems to be a workaround for some bug in IPy related to IPv6, where an IPv6 address could return a 0.0.0.0 netmask. Does this bug also exist in ipaddress? If yes, it would be useful to add a comment to document how to reproduce this bug. > > > > - protocol = "ipv%d" % i.version() > > + protocol = "ipv%d" % i.version > > > > try: > > newprotocol = self.protocol.index(protocol) > > -- > > 2.26.0 > > LGTM > <Reviewed-by: William.c.Roberts@intel.com> > > I can give it an acked by if you give me testing steps. For testing such things, I usually use a Vagrant virtual machine: I have one to test Arch Linux packages (https://github.com/archlinuxhardened/selinux/tree/master/_vagrant), and I also contributed to a Debian VM to test refpolicy (https://github.com/SELinuxProject/refpolicy/blob/2b2b5bad06f0eb2f45217ae337542e5e15bacda8/Vagrantfile#L121). I usually patch the Arch Linux packages when testing small patches that can be applied on top of previous releases. When the previous release is quite old, I copy the whole project in the VM, run "make clean distclean && make DESTDIR=/tmp/dest install install-pywrap", relabel binaries in /tmp/dest and run them. Anyway, +1 for migrating to ipaddress :) Thanks! Nicolas
diff --git a/python/semanage/seobject.py b/python/semanage/seobject.py index f2a139c970bd..c89c67e491b6 100644 --- a/python/semanage/seobject.py +++ b/python/semanage/seobject.py @@ -32,7 +32,7 @@ from semanage import * PROGNAME = "policycoreutils" import sepolicy import setools -from IPy import IP +import ipaddress try: import gettext @@ -1860,13 +1860,13 @@ class nodeRecords(semanageRecords): # verify valid combination if len(mask) == 0 or mask[0] == "/": - i = IP(addr + mask) - newaddr = i.strNormal(0) - newmask = str(i.netmask()) - if newmask == "0.0.0.0" and i.version() == 6: + i = ipaddress.ip_network(addr + mask) + newaddr = str(i.network_address) + newmask = str(i.netmask) + if newmask == "0.0.0.0" and i.version == 6: newmask = "::" - protocol = "ipv%d" % i.version() + protocol = "ipv%d" % i.version try: newprotocol = self.protocol.index(protocol)
ipaddress python module was added to standard library in Python 3.3 - https://docs.python.org/3/library/ipaddress.html seobject.py was the only consumer of IPy module so this dependency is not needed anymore. Signed-off-by: Petr Lautrbach <plautrba@redhat.com> --- python/semanage/seobject.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)