[PATCH] python/semanage: Use ipaddress module instead of IPy

[PATCH] python/semanage: Use ipaddress module instead of IPy

[Petr Lautrbach]

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

RE: [PATCH] python/semanage: Use ipaddress module instead of IPy

[Roberts, William C]

> -----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.

Re: [PATCH] python/semanage: Use ipaddress module instead of IPy

[Nicolas Iooss]

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

[PATCH v2] python/semanage: Use ipaddress module instead of IPy

[Petr Lautrbach]

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>
---

Based on Nicolas input:

- improved the check comment
- dropped the unnecessary check

 python/semanage/seobject.py | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/python/semanage/seobject.py b/python/semanage/seobject.py
index f2a139c970bd..6e0b87f2fa3c 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
@@ -1858,15 +1858,12 @@ class nodeRecords(semanageRecords):
         if addr == "":
             raise ValueError(_("Node Address is required"))
 
-        # verify valid combination
+        # verify that (addr, mask) is either a IP address (without a mask) or a valid network mask
         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:
-                newmask = "::"
-
-            protocol = "ipv%d" % i.version()
+            i = ipaddress.ip_network(addr + mask)
+            newaddr = str(i.network_address)
+            newmask = str(i.netmask)
+            protocol = "ipv%d" % i.version
 
         try:
             newprotocol = self.protocol.index(protocol)
-- 
2.26.2

Re: [PATCH v2] python/semanage: Use ipaddress module instead of IPy

[Nicolas Iooss]

On Mon, Apr 27, 2020 at 5:35 PM Petr Lautrbach <plautrba@redhat.com> wrote:
>
> 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>
> ---
>
> Based on Nicolas input:
>
> - improved the check comment
> - dropped the unnecessary check

Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>

I will apply this patch tomorrow if nobody else has any comment.
Thanks,
Nicolas

>  python/semanage/seobject.py | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/python/semanage/seobject.py b/python/semanage/seobject.py
> index f2a139c970bd..6e0b87f2fa3c 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
> @@ -1858,15 +1858,12 @@ class nodeRecords(semanageRecords):
>          if addr == "":
>              raise ValueError(_("Node Address is required"))
>
> -        # verify valid combination
> +        # verify that (addr, mask) is either a IP address (without a mask) or a valid network mask
>          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:
> -                newmask = "::"
> -
> -            protocol = "ipv%d" % i.version()
> +            i = ipaddress.ip_network(addr + mask)
> +            newaddr = str(i.network_address)
> +            newmask = str(i.netmask)
> +            protocol = "ipv%d" % i.version
>
>          try:
>              newprotocol = self.protocol.index(protocol)
> --
> 2.26.2
>

Re: [PATCH v2] python/semanage: Use ipaddress module instead of IPy

[Nicolas Iooss]

On Tue, Apr 28, 2020 at 11:28 PM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
>
> On Mon, Apr 27, 2020 at 5:35 PM Petr Lautrbach <plautrba@redhat.com> wrote:
> >
> > 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>
> > ---
> >
> > Based on Nicolas input:
> >
> > - improved the check comment
> > - dropped the unnecessary check
>
> Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
>

Applied.
Thanks,
Nicolas

> >  python/semanage/seobject.py | 15 ++++++---------
> >  1 file changed, 6 insertions(+), 9 deletions(-)
> >
> > diff --git a/python/semanage/seobject.py b/python/semanage/seobject.py
> > index f2a139c970bd..6e0b87f2fa3c 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
> > @@ -1858,15 +1858,12 @@ class nodeRecords(semanageRecords):
> >          if addr == "":
> >              raise ValueError(_("Node Address is required"))
> >
> > -        # verify valid combination
> > +        # verify that (addr, mask) is either a IP address (without a mask) or a valid network mask
> >          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:
> > -                newmask = "::"
> > -
> > -            protocol = "ipv%d" % i.version()
> > +            i = ipaddress.ip_network(addr + mask)
> > +            newaddr = str(i.network_address)
> > +            newmask = str(i.netmask)
> > +            protocol = "ipv%d" % i.version
> >
> >          try:
> >              newprotocol = self.protocol.index(protocol)
> > --
> > 2.26.2
> >