diff mbox series

Avoid using getprotobyname()

Message ID 20200602141935.24722-1-toiwoton@gmail.com (mailing list archive)
State Superseded
Headers show
Series Avoid using getprotobyname() | expand

Commit Message

Topi Miettinen June 2, 2020, 2:19 p.m. UTC
At least on Debian, /etc/protocols, which is used by
socket.getprotobyname() to resolve protocols to names, does not
contain an entry for "ipv4", so let's avoid using
socket.getprotobyname() since the protocol names are not used in
socket context anyway.

Signed-off-by: Topi Miettinen <toiwoton@gmail.com>
---
 python/semanage/seobject.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Stephen Smalley June 4, 2020, 8:30 p.m. UTC | #1
On Tue, Jun 2, 2020 at 10:21 AM Topi Miettinen <toiwoton@gmail.com> wrote:
>
> At least on Debian, /etc/protocols, which is used by
> socket.getprotobyname() to resolve protocols to names, does not
> contain an entry for "ipv4", so let's avoid using
> socket.getprotobyname() since the protocol names are not used in
> socket context anyway.
>
> Signed-off-by: Topi Miettinen <toiwoton@gmail.com>

Only concern I have here is that it could change the resulting audit
record content. Not sure how the audit people feel about that.
Maybe ask on linux-audit mailing list?

> ---
>  python/semanage/seobject.py | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/python/semanage/seobject.py b/python/semanage/seobject.py
> index 6e0b87f2..dfb165a2 100644
> --- a/python/semanage/seobject.py
> +++ b/python/semanage/seobject.py
> @@ -1942,7 +1942,7 @@ class nodeRecords(semanageRecords):
>          semanage_node_key_free(k)
>          semanage_node_free(node)
>
> -        self.mylog.log_change("resrc=node op=add laddr=%s netmask=%s proto=%s tcontext=%s:%s:%s:%s" % (addr, mask, socket.getprotobyname(self.protocol[proto]), "system_u", "object_r", ctype, serange))
> +        self.mylog.log_change("resrc=node op=add laddr=%s netmask=%s proto=%s tcontext=%s:%s:%s:%s" % (addr, mask, self.protocol[proto], "system_u", "object_r", ctype, serange))
>
>      def add(self, addr, mask, proto, serange, ctype):
>          self.begin()
> @@ -1987,7 +1987,7 @@ class nodeRecords(semanageRecords):
>          semanage_node_key_free(k)
>          semanage_node_free(node)
>
> -        self.mylog.log_change("resrc=node op=modify laddr=%s netmask=%s proto=%s tcontext=%s:%s:%s:%s" % (addr, mask, socket.getprotobyname(self.protocol[proto]), "system_u", "object_r", setype, serange))
> +        self.mylog.log_change("resrc=node op=modify laddr=%s netmask=%s proto=%s tcontext=%s:%s:%s:%s" % (addr, mask, self.protocol[proto], "system_u", "object_r", setype, serange))
>
>      def modify(self, addr, mask, proto, serange, setype):
>          self.begin()
> @@ -2020,7 +2020,7 @@ class nodeRecords(semanageRecords):
>
>          semanage_node_key_free(k)
>
> -        self.mylog.log_change("resrc=node op=delete laddr=%s netmask=%s proto=%s" % (addr, mask, socket.getprotobyname(self.protocol[proto])))
> +        self.mylog.log_change("resrc=node op=delete laddr=%s netmask=%s proto=%s" % (addr, mask, self.protocol[proto]))
>
>      def delete(self, addr, mask, proto):
>          self.begin()
> --
> 2.26.2
>
Paul Moore June 4, 2020, 8:40 p.m. UTC | #2
On Thu, Jun 4, 2020 at 4:30 PM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
> On Tue, Jun 2, 2020 at 10:21 AM Topi Miettinen <toiwoton@gmail.com> wrote:
> >
> > At least on Debian, /etc/protocols, which is used by
> > socket.getprotobyname() to resolve protocols to names, does not
> > contain an entry for "ipv4", so let's avoid using
> > socket.getprotobyname() since the protocol names are not used in
> > socket context anyway.
> >
> > Signed-off-by: Topi Miettinen <toiwoton@gmail.com>
>
> Only concern I have here is that it could change the resulting audit
> record content. Not sure how the audit people feel about that.
> Maybe ask on linux-audit mailing list?

If/when you do, it would be good to show before/after audit records.
However, record formatting is a very tricky issue and it's best to not
change them unless absolutely necessary.
Topi Miettinen June 4, 2020, 9:34 p.m. UTC | #3
On 4.6.2020 23.40, Paul Moore wrote:
> On Thu, Jun 4, 2020 at 4:30 PM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
>> On Tue, Jun 2, 2020 at 10:21 AM Topi Miettinen <toiwoton@gmail.com> wrote:
>>>
>>> At least on Debian, /etc/protocols, which is used by
>>> socket.getprotobyname() to resolve protocols to names, does not
>>> contain an entry for "ipv4", so let's avoid using
>>> socket.getprotobyname() since the protocol names are not used in
>>> socket context anyway.
>>>
>>> Signed-off-by: Topi Miettinen <toiwoton@gmail.com>
>>
>> Only concern I have here is that it could change the resulting audit
>> record content. Not sure how the audit people feel about that.
>> Maybe ask on linux-audit mailing list?
> 
> If/when you do, it would be good to show before/after audit records.
> However, record formatting is a very tricky issue and it's best to not
> change them unless absolutely necessary.

Right, let's not change it.

One solution would be to try to resolve "ipv4" first and if it fails, 
try something else. On Fedora "ipv4" resolves to 4. For Debian "IP" 
would be 0 and 4 can be found with "ipencap".

The original problem was that the protocol "ipv4" is not accepted by 
"semanage node":
# semanage node -a -t internet_node_t -p ipv4 -M /4 208.0.0.0
OSError: protocol not found

This makes me believe that nobody before me had ever used "semanage 
node" successfully on Debian. Therefore there shouldn't be compatibility 
issues with old audit logs, but I suppose it's better to try to match 
the value used by Fedora (4), so "ipencap" would be a better choice or 
perhaps simply hardcode the value as 4 if "ipv4" does not resolve.

-Topi
diff mbox series

Patch

diff --git a/python/semanage/seobject.py b/python/semanage/seobject.py
index 6e0b87f2..dfb165a2 100644
--- a/python/semanage/seobject.py
+++ b/python/semanage/seobject.py
@@ -1942,7 +1942,7 @@  class nodeRecords(semanageRecords):
         semanage_node_key_free(k)
         semanage_node_free(node)
 
-        self.mylog.log_change("resrc=node op=add laddr=%s netmask=%s proto=%s tcontext=%s:%s:%s:%s" % (addr, mask, socket.getprotobyname(self.protocol[proto]), "system_u", "object_r", ctype, serange))
+        self.mylog.log_change("resrc=node op=add laddr=%s netmask=%s proto=%s tcontext=%s:%s:%s:%s" % (addr, mask, self.protocol[proto], "system_u", "object_r", ctype, serange))
 
     def add(self, addr, mask, proto, serange, ctype):
         self.begin()
@@ -1987,7 +1987,7 @@  class nodeRecords(semanageRecords):
         semanage_node_key_free(k)
         semanage_node_free(node)
 
-        self.mylog.log_change("resrc=node op=modify laddr=%s netmask=%s proto=%s tcontext=%s:%s:%s:%s" % (addr, mask, socket.getprotobyname(self.protocol[proto]), "system_u", "object_r", setype, serange))
+        self.mylog.log_change("resrc=node op=modify laddr=%s netmask=%s proto=%s tcontext=%s:%s:%s:%s" % (addr, mask, self.protocol[proto], "system_u", "object_r", setype, serange))
 
     def modify(self, addr, mask, proto, serange, setype):
         self.begin()
@@ -2020,7 +2020,7 @@  class nodeRecords(semanageRecords):
 
         semanage_node_key_free(k)
 
-        self.mylog.log_change("resrc=node op=delete laddr=%s netmask=%s proto=%s" % (addr, mask, socket.getprotobyname(self.protocol[proto])))
+        self.mylog.log_change("resrc=node op=delete laddr=%s netmask=%s proto=%s" % (addr, mask, self.protocol[proto]))
 
     def delete(self, addr, mask, proto):
         self.begin()