2017-06-07 16:03:37

by Mira Ressel

[permalink] [raw]
Subject: [refpolicy] [PATCH] netutils: Add some permissions required by nmap to traceroute_t

---
policy/modules/admin/netutils.te | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/policy/modules/admin/netutils.te b/policy/modules/admin/netutils.te
index 4ea58479..b5bdda2d 100644
--- a/policy/modules/admin/netutils.te
+++ b/policy/modules/admin/netutils.te
@@ -171,9 +171,7 @@ optional_policy(`
#

allow traceroute_t self:capability { net_admin net_raw setgid setuid };
-allow traceroute_t self:rawip_socket create_socket_perms;
-allow traceroute_t self:packet_socket create_socket_perms;
-allow traceroute_t self:udp_socket create_socket_perms;
+allow traceroute_t self:{ packet_socket rawip_socket socket udp_socket } create_socket_perms;

kernel_read_system_state(traceroute_t)
kernel_read_network_state(traceroute_t)
@@ -215,6 +213,15 @@ miscfiles_read_localization(traceroute_t)
userdom_use_user_terminals(traceroute_t)

#rules needed for nmap
+allow traceroute_t self:process signal;
+
dev_read_rand(traceroute_t)
dev_read_urand(traceroute_t)
+dev_read_sysfs(traceroute_t)
+
+corecmd_search_bin(traceroute_t)
files_read_usr_files(traceroute_t)
+
+# nmap searches .
+userdom_dontaudit_search_user_home_dirs(traceroute_t)
+userdom_dontaudit_search_user_home_content(traceroute_t)
--
2.13.1


2017-06-07 23:22:07

by Chris PeBenito

[permalink] [raw]
Subject: [refpolicy] [PATCH] netutils: Add some permissions required by nmap to traceroute_t

On 06/07/2017 12:03 PM, Luis Ressel via refpolicy wrote:
> ---
> policy/modules/admin/netutils.te | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/policy/modules/admin/netutils.te b/policy/modules/admin/netutils.te
> index 4ea58479..b5bdda2d 100644
> --- a/policy/modules/admin/netutils.te
> +++ b/policy/modules/admin/netutils.te
> @@ -171,9 +171,7 @@ optional_policy(`
> #
>
> allow traceroute_t self:capability { net_admin net_raw setgid setuid };
> -allow traceroute_t self:rawip_socket create_socket_perms;
> -allow traceroute_t self:packet_socket create_socket_perms;
> -allow traceroute_t self:udp_socket create_socket_perms;
> +allow traceroute_t self:{ packet_socket rawip_socket socket udp_socket } create_socket_perms;

I'd prefer not to have changes like this.


> kernel_read_system_state(traceroute_t)
> kernel_read_network_state(traceroute_t)
> @@ -215,6 +213,15 @@ miscfiles_read_localization(traceroute_t)
> userdom_use_user_terminals(traceroute_t)
>
> #rules needed for nmap
> +allow traceroute_t self:process signal;

This needs to go up with the other self rules.

> dev_read_rand(traceroute_t)
> dev_read_urand(traceroute_t)
> +dev_read_sysfs(traceroute_t)
> +
> +corecmd_search_bin(traceroute_t)

This should go with the other corenet rules.

> files_read_usr_files(traceroute_t)
> +
> +# nmap searches .
> +userdom_dontaudit_search_user_home_dirs(traceroute_t)
> +userdom_dontaudit_search_user_home_content(traceroute_t)
>


--
Chris PeBenito

2017-06-08 00:26:36

by Mira Ressel

[permalink] [raw]
Subject: [refpolicy] [PATCH] netutils: Add some permissions required by nmap to traceroute_t

On Wed, 7 Jun 2017 19:22:07 -0400
Chris PeBenito <[email protected]> wrote:

> On 06/07/2017 12:03 PM, Luis Ressel via refpolicy wrote:
> > ---
> > policy/modules/admin/netutils.te | 13 ++++++++++---
> > 1 file changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/policy/modules/admin/netutils.te
> > b/policy/modules/admin/netutils.te index 4ea58479..b5bdda2d 100644
> > --- a/policy/modules/admin/netutils.te
> > +++ b/policy/modules/admin/netutils.te
> > @@ -171,9 +171,7 @@ optional_policy(`
> > #
> >
> > allow traceroute_t self:capability { net_admin net_raw setgid
> > setuid }; -allow traceroute_t self:rawip_socket create_socket_perms;
> > -allow traceroute_t self:packet_socket create_socket_perms;
> > -allow traceroute_t self:udp_socket create_socket_perms;
> > +allow traceroute_t self:{ packet_socket rawip_socket socket
> > udp_socket } create_socket_perms;
>
> I'd prefer not to have changes like this.

Okay, I'll add a separate rule for self:socket, then. I'm curious,
though: Why don't you want to use the :{ ... } syntax here?

> > kernel_read_system_state(traceroute_t)
> > kernel_read_network_state(traceroute_t)
> > @@ -215,6 +213,15 @@ miscfiles_read_localization(traceroute_t)
> > userdom_use_user_terminals(traceroute_t)
> >
> > #rules needed for nmap
> > +allow traceroute_t self:process signal;
>
> This needs to go up with the other self rules.

Okay. But while we're at this: Should we perhaps drop the whole "rules
needed for nmap" block and mix those rules with the others above? In
particular, the files_read_usr_files() rule could be moved to the other
files_ rules.

> > dev_read_rand(traceroute_t)
> > dev_read_urand(traceroute_t)
> > +dev_read_sysfs(traceroute_t)
> > +
> > +corecmd_search_bin(traceroute_t)
>
> This should go with the other corenet rules.
>

You probably misread; this is a core*cmd* rule.

> > files_read_usr_files(traceroute_t)
> > +
> > +# nmap searches .
> > +userdom_dontaudit_search_user_home_dirs(traceroute_t)
> > +userdom_dontaudit_search_user_home_content(traceroute_t)
> >
>
>

Thanks for your feedback!

Regards,
Luis
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
Url : http://oss.tresys.com/pipermail/refpolicy/attachments/20170608/ef1276a4/attachment.bin

2017-06-08 22:15:16

by Chris PeBenito

[permalink] [raw]
Subject: [refpolicy] [PATCH] netutils: Add some permissions required by nmap to traceroute_t

On 06/07/2017 08:26 PM, Luis Ressel wrote:
> On Wed, 7 Jun 2017 19:22:07 -0400
> Chris PeBenito <[email protected]> wrote:
>
>> On 06/07/2017 12:03 PM, Luis Ressel via refpolicy wrote:
>>> ---
>>> policy/modules/admin/netutils.te | 13 ++++++++++---
>>> 1 file changed, 10 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/policy/modules/admin/netutils.te
>>> b/policy/modules/admin/netutils.te index 4ea58479..b5bdda2d 100644
>>> --- a/policy/modules/admin/netutils.te
>>> +++ b/policy/modules/admin/netutils.te
>>> @@ -171,9 +171,7 @@ optional_policy(`
>>> #
>>>
>>> allow traceroute_t self:capability { net_admin net_raw setgid
>>> setuid }; -allow traceroute_t self:rawip_socket create_socket_perms;
>>> -allow traceroute_t self:packet_socket create_socket_perms;
>>> -allow traceroute_t self:udp_socket create_socket_perms;
>>> +allow traceroute_t self:{ packet_socket rawip_socket socket
>>> udp_socket } create_socket_perms;
>>
>> I'd prefer not to have changes like this.
>
> Okay, I'll add a separate rule for self:socket, then. I'm curious,
> though: Why don't you want to use the :{ ... } syntax here?

I find it harder to read. For example, I missed that you added the
socket class. What socket type is being used? Did you try enabling
policycap extended_socket_class (assuming kernel 4.11+ and libsepol 2.7+)?

>>> kernel_read_system_state(traceroute_t)
>>> kernel_read_network_state(traceroute_t)
>>> @@ -215,6 +213,15 @@ miscfiles_read_localization(traceroute_t)
>>> userdom_use_user_terminals(traceroute_t)
>>>
>>> #rules needed for nmap
>>> +allow traceroute_t self:process signal;
>>
>> This needs to go up with the other self rules.
>
> Okay. But while we're at this: Should we perhaps drop the whole "rules
> needed for nmap" block and mix those rules with the others above? In
> particular, the files_read_usr_files() rule could be moved to the other
> files_ rules.

I'm fine with that.

>>> dev_read_rand(traceroute_t)
>>> dev_read_urand(traceroute_t)
>>> +dev_read_sysfs(traceroute_t)
>>> +
>>> +corecmd_search_bin(traceroute_t)
>>
>> This should go with the other corenet rules.
>>
>
> You probably misread; this is a core*cmd* rule.

True. It should go above the corenet rules.


--
Chris PeBenito

2017-06-09 01:56:46

by Mira Ressel

[permalink] [raw]
Subject: [refpolicy] [PATCH] netutils: Add some permissions required by nmap to traceroute_t

On Thu, 8 Jun 2017 18:15:16 -0400
Chris PeBenito <[email protected]> wrote:

> > Okay, I'll add a separate rule for self:socket, then. I'm curious,
> > though: Why don't you want to use the :{ ... } syntax here?
>
> I find it harder to read. For example, I missed that you added the
> socket class. What socket type is being used? Did you try enabling
> policycap extended_socket_class (assuming kernel 4.11+ and libsepol
> 2.7+)?

Thanks for the hint; I wasn't aware of this new policycap. I tried
enabling it, but I must've done something wrong: I upgraded libse*,
checkpolicy and policycoreutils to the latest git HEAD, edited
policy/policy_capabilities and recompiled the policy. Now seinfo
--polcaps shows a new polcap "redhat1", but the denial in the audit logs
still reports the class as "socket".

(I didn't upgrade setools, but those don't have anything to do with the
policy compilation, right? And before you ask, yes, I am on linux 4.11,
4.11.3 to be exact).

Any ideas? I'll have another look at this during the weekend.

Regards,
Luis Ressel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
Url : http://oss.tresys.com/pipermail/refpolicy/attachments/20170609/7c24c21f/attachment.bin

2017-06-12 22:32:35

by Chris PeBenito

[permalink] [raw]
Subject: [refpolicy] [PATCH] netutils: Add some permissions required by nmap to traceroute_t

On 06/08/2017 09:56 PM, Luis Ressel wrote:
> On Thu, 8 Jun 2017 18:15:16 -0400
> Chris PeBenito <[email protected]> wrote:
>
>>> Okay, I'll add a separate rule for self:socket, then. I'm curious,
>>> though: Why don't you want to use the :{ ... } syntax here?
>>
>> I find it harder to read. For example, I missed that you added the
>> socket class. What socket type is being used? Did you try enabling
>> policycap extended_socket_class (assuming kernel 4.11+ and libsepol
>> 2.7+)?
>
> Thanks for the hint; I wasn't aware of this new policycap. I tried
> enabling it, but I must've done something wrong: I upgraded libse*,
> checkpolicy and policycoreutils to the latest git HEAD, edited
> policy/policy_capabilities and recompiled the policy. Now seinfo
> --polcaps shows a new polcap "redhat1", but the denial in the audit logs
> still reports the class as "socket".
>
> (I didn't upgrade setools, but those don't have anything to do with the
> policy compilation, right? And before you ask, yes, I am on linux 4.11,
> 4.11.3 to be exact).
>
> Any ideas? I'll have another look at this during the weekend.

You need to recompile SETools, as it is statically linked to libsepol.


--
Chris PeBenito

2017-06-18 19:45:17

by Mira Ressel

[permalink] [raw]
Subject: [refpolicy] [PATCH] netutils: Add some permissions required by nmap to traceroute_t

On Mon, 12 Jun 2017 18:32:35 -0400
Chris PeBenito via refpolicy <[email protected]> wrote:

> On 06/08/2017 09:56 PM, Luis Ressel wrote:
> > On Thu, 8 Jun 2017 18:15:16 -0400
> > Chris PeBenito <[email protected]> wrote:
> >
> >>> Okay, I'll add a separate rule for self:socket, then. I'm curious,
> >>> though: Why don't you want to use the :{ ... } syntax here?
> >>
> >> I find it harder to read. For example, I missed that you added the
> >> socket class. What socket type is being used? Did you try
> >> enabling policycap extended_socket_class (assuming kernel 4.11+
> >> and libsepol 2.7+)?
> >
> > Thanks for the hint; I wasn't aware of this new policycap. I tried
> > enabling it, but I must've done something wrong: I upgraded libse*,
> > checkpolicy and policycoreutils to the latest git HEAD, edited
> > policy/policy_capabilities and recompiled the policy. Now seinfo
> > --polcaps shows a new polcap "redhat1", but the denial in the audit
> > logs still reports the class as "socket".
> >
> > (I didn't upgrade setools, but those don't have anything to do with
> > the policy compilation, right? And before you ask, yes, I am on
> > linux 4.11, 4.11.3 to be exact).
> >extended_socket_class
> > Any ideas? I'll have another look at this during the weekend.
>
> You need to recompile SETools, as it is statically linked to libsepol.

I recompiled SETools in the meantime; seinfo --polcap now properly
reports the extended_socket_class capability.

However, the avc denail message still reports tclass=socket. The full
denial message, as reported by ausearch, is
----
type=PROCTITLE msg=audit(06/18/2017 21:34:45.744:1043) : proctitle=/usr/bin/nmap -sn 192.168.2.1
type=SYSCALL msg=audit(06/18/2017 21:34:45.744:1043) : arch=x86_64 syscall=socket success=no exit=EACCES(Permission denied) a0=local a1=SOCK_RAW a2=ip a3=0x311 items=0 ppid=18173 pid=18174 auid=lre uid=root gid=root euid=root suid=root fsuid=root egid=root sgid=root fsgid=root tty=pts1 ses=3 comm=nmap exe=/usr/bin/nmap subj=staff_u:sysadm_r:traceroute_t key=(null)
type=AVC msg=audit(06/18/2017 21:34:45.744:1043) : avc: denied { create } for pid=18174 comm=nmap scontext=staff_u:sysadm_r:traceroute_t tcontext=staff_u:sysadm_r:traceroute_t tclass=socket permissive=0
----

Doesn't this mean the proper class should be rawip_socket? traceroute_t
already has the create permission for this class. At this point, I'm
inclined to just add the ":socket create" permission, but I'm still
interested if anyone has an idea why it is needed.

Regards,
Luis Ressel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
Url : http://oss.tresys.com/pipermail/refpolicy/attachments/20170618/ff50fbe5/attachment.bin

2017-06-18 20:50:43

by Mira Ressel

[permalink] [raw]
Subject: [refpolicy] [PATCH] netutils: Add some permissions required by nmap to traceroute_t

On Sun, 18 Jun 2017 21:45:17 +0200
Luis Ressel via refpolicy <[email protected]> wrote:

> On Mon, 12 Jun 2017 18:32:35 -0400
> Chris PeBenito via refpolicy <[email protected]> wrote:
>
> > On 06/08/2017 09:56 PM, Luis Ressel wrote:
> > > On Thu, 8 Jun 2017 18:15:16 -0400
> > > Chris PeBenito <[email protected]> wrote:
> > >
> > >>> Okay, I'll add a separate rule for self:socket, then. I'm
> > >>> curious, though: Why don't you want to use the :{ ... } syntax
> > >>> here?
> > >>
> > >> I find it harder to read. For example, I missed that you added
> > >> the socket class. What socket type is being used? Did you try
> > >> enabling policycap extended_socket_class (assuming kernel 4.11+
> > >> and libsepol 2.7+)?
> > >
> > > Thanks for the hint; I wasn't aware of this new policycap. I tried
> > > enabling it, but I must've done something wrong: I upgraded
> > > libse*, checkpolicy and policycoreutils to the latest git HEAD,
> > > edited policy/policy_capabilities and recompiled the policy. Now
> > > seinfo --polcaps shows a new polcap "redhat1", but the denial in
> > > the audit logs still reports the class as "socket".
> > >
> > > (I didn't upgrade setools, but those don't have anything to do
> > > with the policy compilation, right? And before you ask, yes, I am
> > > on linux 4.11, 4.11.3 to be exact).
> > >extended_socket_class
> > > Any ideas? I'll have another look at this during the weekend.
> >
> > You need to recompile SETools, as it is statically linked to
> > libsepol.
>
> I recompiled SETools in the meantime; seinfo --polcap now properly
> reports the extended_socket_class capability.
>
> However, the avc denail message still reports tclass=socket. The full
> denial message, as reported by ausearch, is
> ----
> type=PROCTITLE msg=audit(06/18/2017 21:34:45.744:1043) :
> proctitle=/usr/bin/nmap -sn 192.168.2.1 type=SYSCALL
> msg=audit(06/18/2017 21:34:45.744:1043) : arch=x86_64 syscall=socket
> success=no exit=EACCES(Permission denied) a0=local a1=SOCK_RAW a2=ip
> a3=0x311 items=0 ppid=18173 pid=18174 auid=lre uid=root gid=root
> euid=root suid=root fsuid=root egid=root sgid=root fsgid=root
> tty=pts1 ses=3 comm=nmap exe=/usr/bin/nmap
> subj=staff_u:sysadm_r:traceroute_t key=(null) type=AVC
> msg=audit(06/18/2017 21:34:45.744:1043) : avc: denied { create }
> for pid=18174 comm=nmap scontext=staff_u:sysadm_r:traceroute_t
> tcontext=staff_u:sysadm_r:traceroute_t tclass=socket permissive=0 ----
>
> Doesn't this mean the proper class should be rawip_socket?
> traceroute_t already has the create permission for this class. At
> this point, I'm inclined to just add the ":socket create" permission,
> but I'm still interested if anyone has an idea why it is needed.

Nevermind, I figured this out. The "a2=ip" had misled me to believe
rawip_socket would be the right class here, but in fact it only means
that the third argument to socket() is zero. I'd missed "a0=local" (aka
PF_UNIX).

In other words, nmap is creating a raw *Unix domain socket*
here (it doesn't actually use this socket, it just needs a random
socket to get some info from the SIOCETHTOOL ioctl). The kernel's
socket_type_to_security_class() doesn't care about this kind of socket
(no matter if extended_socket_class is enabled or not), hence it assigns
the "socket" class.

Regards,
Luis Ressel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
Url : http://oss.tresys.com/pipermail/refpolicy/attachments/20170618/27adfefd/attachment.bin