On 06/08/12 13:36, Paul Moore wrote:
> On Thursday, June 07, 2012 02:28:01 PM Chris PeBenito wrote:
>> Currently the packet class in SELinux is not checked if there are no
>> SECMARK rules in the security or mangle netfilter tables. Some systems
>> prefer that packets are always checked, for example, to protect the system
>> should the netfilter rules fail to load or if the nefilter rules
>> were maliciously flushed.
>>
>> Add the always_check_network policy capability which, when enabled, treats
>> SECMARK as enabled, even if there are no netfilter SECMARK rules.
>>
>> Signed-off-by: Chris PeBenito <cpebenito@xxxxxxxxxx>
>
> ...
>
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index 372ec65..ec7151b 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>
> ...
>
>> static int selinux_secmark_enabled(void)
>> {
>> - return (atomic_read(&selinux_secmark_refcount) > 0);
>> + if (selinux_policycap_alwaysnetwork)
>> + return 1;
>> + else
>> + return (atomic_read(&selinux_secmark_refcount) > 0);
>> }
>
> Nit picky, but why not simply:
>
> return (selinux_policycap_alwaysnetwork || atomic_read( ...
Sure it can be done that way, but I'd argue that the intent is clearer the way its implemented in the patch.
>> /*
>> diff --git a/security/selinux/include/security.h
>> b/security/selinux/include/security.h index dde2005..981c4ac 100644
>> --- a/security/selinux/include/security.h
>> +++ b/security/selinux/include/security.h
>> @@ -68,12 +68,14 @@ extern int selinux_enabled;
>> enum {
>> POLICYDB_CAPABILITY_NETPEER,
>> POLICYDB_CAPABILITY_OPENPERM,
>> + POLICYDB_CAPABILITY_ALWAYSNETWORK,
>> __POLICYDB_CAPABILITY_MAX
>> };
>> #define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1)
>>
>> extern int selinux_policycap_netpeer;
>> extern int selinux_policycap_openperm;
>> +extern int selinux_policycap_alwaysnetwork;
>
> Also nit picky, but it would seem like "selinux_policycap_netalways" is a bit
> more consistent with the other variables.
This response stems on what you wrote on a later cunch about the capability name, so you should probably read below first.
I don't agree. If you go by current capabilities names/variables, based on the ordering of the words in the capability name, the words in the variable are in the same order. e.g.
NETwork_PEER_controls -> selinux_policycap_netpeer
OPEN_PERMs -> selinux_policycap_openperm
hence
ALWAYS_check_NETWORK -> selinux_policycap_alwaysnetwork
>> diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
>> index 3ad2902..cb893f9 100644
>> --- a/security/selinux/selinuxfs.c
>> +++ b/security/selinux/selinuxfs.c
>> @@ -44,7 +44,8 @@
>> /* Policy capability filenames */
>> static char *policycap_names[] = {
>> "network_peer_controls",
>> - "open_perms"
>> + "open_perms",
>> + "always_check_network"
>> };
>
> Similarly, I think "network_always" is more consistent.
Earlier discussions concluded with this capability name. (to be specific the discussion resolved with always_check_packets, but that was before the peer class was included)
--
Chris PeBenito
Tresys Technology, LLC
www.tresys.com | oss.tresys.com
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@xxxxxxxxxxxxx with
the words "unsubscribe selinux" without quotes as the message.
