* [PATCH 1/2] Add SELinux policy capability for always checking packet class.
@ 2012-06-07 18:28 Chris PeBenito
2012-06-07 18:28 ` [PATCH 2/2] Update SELinux policy capability to always check peer class Chris PeBenito
2012-06-08 17:36 ` [PATCH 1/2] Add SELinux policy capability for always checking packet class Paul Moore
0 siblings, 2 replies; 16+ messages in thread
From: Chris PeBenito @ 2012-06-07 18:28 UTC (permalink / raw)
To: SELinux; +Cc: Chris PeBenito
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@tresys.com>
---
security/selinux/hooks.c | 8 ++++++--
security/selinux/include/security.h | 2 ++
security/selinux/selinuxfs.c | 3 ++-
security/selinux/ss/services.c | 3 +++
4 files changed, 13 insertions(+), 3 deletions(-)
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
@@ -137,12 +137,16 @@ static struct kmem_cache *sel_inode_cache;
* This function checks the SECMARK reference counter to see if any SECMARK
* targets are currently configured, if the reference counter is greater than
* zero SECMARK is considered to be enabled. Returns true (1) if SECMARK is
- * enabled, false (0) if SECMARK is disabled.
+ * enabled, false (0) if SECMARK is disabled. If the always_check_network
+ * policy capability is enabled, SECMARK is always considered enabled.
*
*/
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);
}
/*
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;
/*
* type_datum properties
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"
};
unsigned int selinux_checkreqprot = CONFIG_SECURITY_SELINUX_CHECKREQPROT_VALUE;
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 4321b8f..e124d8f 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -72,6 +72,7 @@
int selinux_policycap_netpeer;
int selinux_policycap_openperm;
+int selinux_policycap_alwaysnetwork;
static DEFINE_RWLOCK(policy_rwlock);
@@ -1812,6 +1813,8 @@ static void security_load_policycaps(void)
POLICYDB_CAPABILITY_NETPEER);
selinux_policycap_openperm = ebitmap_get_bit(&policydb.policycaps,
POLICYDB_CAPABILITY_OPENPERM);
+ selinux_policycap_alwaysnetwork = ebitmap_get_bit(&policydb.policycaps,
+ POLICYDB_CAPABILITY_ALWAYSNETWORK);
}
static int security_preserve_bools(struct policydb *p);
--
1.7.8.6
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 2/2] Update SELinux policy capability to always check peer class.
2012-06-07 18:28 [PATCH 1/2] Add SELinux policy capability for always checking packet class Chris PeBenito
@ 2012-06-07 18:28 ` Chris PeBenito
2012-06-08 17:36 ` Paul Moore
2012-06-08 17:36 ` [PATCH 1/2] Add SELinux policy capability for always checking packet class Paul Moore
1 sibling, 1 reply; 16+ messages in thread
From: Chris PeBenito @ 2012-06-07 18:28 UTC (permalink / raw)
To: SELinux; +Cc: Chris PeBenito
Update the always_check_network policy capability which, when enabled,
treats peer labeling as enabled, even if there is no Netlabel or
labeled IPSEC configuration.
Signed-off-by: Chris PeBenito <cpebenito@tresys.com>
---
security/selinux/hooks.c | 24 +++++++++++++++++++++---
1 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index ec7151b..e8f612e 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -149,6 +149,24 @@ static int selinux_secmark_enabled(void)
return (atomic_read(&selinux_secmark_refcount) > 0);
}
+/**
+ * selinux_peerlbl_enabled - Check to see if peer labeling is currently enabled
+ *
+ * Description:
+ * This function checks if NetLabel or labeled IPSEC is enabled. Returns true
+ * (1) if any are enabled or false (0) if neither are enabled. If the
+ * always_check_network policy capability is enabled, peer labeling
+ * is always considered enabled.
+ *
+ */
+static int selinux_peerlbl_enabled(void)
+{
+ if (selinux_policycap_alwaysnetwork)
+ return 1;
+ else
+ return (netlbl_enabled() || selinux_xfrm_enabled());
+}
+
/*
* initialise the security for the init task
*/
@@ -4188,7 +4206,7 @@ static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
return selinux_sock_rcv_skb_compat(sk, skb, family);
secmark_active = selinux_secmark_enabled();
- peerlbl_active = netlbl_enabled() || selinux_xfrm_enabled();
+ peerlbl_active = selinux_peerlbl_enabled();
if (!secmark_active && !peerlbl_active)
return 0;
@@ -4540,7 +4558,7 @@ static unsigned int selinux_ip_forward(struct sk_buff *skb, int ifindex,
secmark_active = selinux_secmark_enabled();
netlbl_active = netlbl_enabled();
- peerlbl_active = netlbl_active || selinux_xfrm_enabled();
+ peerlbl_active = selinux_peerlbl_enabled();
if (!secmark_active && !peerlbl_active)
return NF_ACCEPT;
@@ -4692,7 +4710,7 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb, int ifindex,
return NF_ACCEPT;
#endif
secmark_active = selinux_secmark_enabled();
- peerlbl_active = netlbl_enabled() || selinux_xfrm_enabled();
+ peerlbl_active = selinux_peerlbl_enabled();
if (!secmark_active && !peerlbl_active)
return NF_ACCEPT;
--
1.7.8.6
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 2/2] Update SELinux policy capability to always check peer class.
2012-06-07 18:28 ` [PATCH 2/2] Update SELinux policy capability to always check peer class Chris PeBenito
@ 2012-06-08 17:36 ` Paul Moore
2012-06-08 18:29 ` Christopher J. PeBenito
0 siblings, 1 reply; 16+ messages in thread
From: Paul Moore @ 2012-06-08 17:36 UTC (permalink / raw)
To: Chris PeBenito; +Cc: SELinux
On Thursday, June 07, 2012 02:28:02 PM Chris PeBenito wrote:
> Update the always_check_network policy capability which, when enabled,
> treats peer labeling as enabled, even if there is no Netlabel or
> labeled IPSEC configuration.
>
> Signed-off-by: Chris PeBenito <cpebenito@tresys.com>
...
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index ec7151b..e8f612e 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -149,6 +149,24 @@ static int selinux_secmark_enabled(void)
> +static int selinux_peerlbl_enabled(void)
> +{
> + if (selinux_policycap_alwaysnetwork)
> + return 1;
> + else
> + return (netlbl_enabled() || selinux_xfrm_enabled());
> +}
Why not make this more consistent?
return (selinux_policycap_alwaysnetwork || ...
--
paul moore
www.paul-moore.com
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 2/2] Update SELinux policy capability to always check peer class.
2012-06-08 17:36 ` Paul Moore
@ 2012-06-08 18:29 ` Christopher J. PeBenito
0 siblings, 0 replies; 16+ messages in thread
From: Christopher J. PeBenito @ 2012-06-08 18:29 UTC (permalink / raw)
To: Paul Moore; +Cc: SELinux
On 06/08/12 13:36, Paul Moore wrote:
> On Thursday, June 07, 2012 02:28:02 PM Chris PeBenito wrote:
>> Update the always_check_network policy capability which, when enabled,
>> treats peer labeling as enabled, even if there is no Netlabel or
>> labeled IPSEC configuration.
>>
>> Signed-off-by: Chris PeBenito <cpebenito@tresys.com>
>
> ...
>
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index ec7151b..e8f612e 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -149,6 +149,24 @@ static int selinux_secmark_enabled(void)
>> +static int selinux_peerlbl_enabled(void)
>> +{
>> + if (selinux_policycap_alwaysnetwork)
>> + return 1;
>> + else
>> + return (netlbl_enabled() || selinux_xfrm_enabled());
>> +}
>
> Why not make this more consistent?
>
> return (selinux_policycap_alwaysnetwork || ...
Same response as the other patch.
--
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@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] Add SELinux policy capability for always checking packet class.
2012-06-07 18:28 [PATCH 1/2] Add SELinux policy capability for always checking packet class Chris PeBenito
2012-06-07 18:28 ` [PATCH 2/2] Update SELinux policy capability to always check peer class Chris PeBenito
@ 2012-06-08 17:36 ` Paul Moore
2012-06-08 18:28 ` Christopher J. PeBenito
1 sibling, 1 reply; 16+ messages in thread
From: Paul Moore @ 2012-06-08 17:36 UTC (permalink / raw)
To: Chris PeBenito; +Cc: SELinux
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@tresys.com>
...
> 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( ...
> /*
> 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.
> 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.
> a/security/selinux/ss/services.c b/security/selinux/ss/services.c index
> 4321b8f..e124d8f 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -72,6 +72,7 @@
>
> int selinux_policycap_netpeer;
> int selinux_policycap_openperm;
> +int selinux_policycap_alwaysnetwork;
See above.
--
paul moore
www.paul-moore.com
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 1/2] Add SELinux policy capability for always checking packet class.
2012-06-08 17:36 ` [PATCH 1/2] Add SELinux policy capability for always checking packet class Paul Moore
@ 2012-06-08 18:28 ` Christopher J. PeBenito
2012-06-08 18:55 ` Paul Moore
2012-06-08 19:33 ` Casey Schaufler
0 siblings, 2 replies; 16+ messages in thread
From: Christopher J. PeBenito @ 2012-06-08 18:28 UTC (permalink / raw)
To: Paul Moore; +Cc: SELinux
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@tresys.com>
>
> ...
>
>> 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@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 1/2] Add SELinux policy capability for always checking packet class.
2012-06-08 18:28 ` Christopher J. PeBenito
@ 2012-06-08 18:55 ` Paul Moore
2012-06-08 19:43 ` Christopher J. PeBenito
2012-06-08 19:33 ` Casey Schaufler
1 sibling, 1 reply; 16+ messages in thread
From: Paul Moore @ 2012-06-08 18:55 UTC (permalink / raw)
To: Christopher J. PeBenito; +Cc: SELinux
On Friday, June 08, 2012 02:28:44 PM Christopher J. PeBenito wrote:
> On 06/08/12 13:36, Paul Moore wrote:
> > On Thursday, June 07, 2012 02:28:01 PM Chris PeBenito wrote:
> >> 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.
It looks very hacky and inconsistent to me.
Oh well, you can sort it out with Eric.
> >> 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.
What?
I don't believe I ever commented on the capability names until now, please
correct me with a link to the archives if I'm mistaken.
> 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
I look at it as a bit of "namespacing" ... "netpeer" enables the network peer
controls, "openperm" affects the open permission. Since this capability
affects the network controls I believe "netalways" makes more sense.
> >> 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)
Once again, I don't believe I ever commented on the name of the capability,
but I am now. Disregard my comments or incorporate them, that is up to you,
and ultimately Eric, to decide.
--
paul moore
www.paul-moore.com
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 1/2] Add SELinux policy capability for always checking packet class.
2012-06-08 18:55 ` Paul Moore
@ 2012-06-08 19:43 ` Christopher J. PeBenito
0 siblings, 0 replies; 16+ messages in thread
From: Christopher J. PeBenito @ 2012-06-08 19:43 UTC (permalink / raw)
To: Paul Moore; +Cc: SELinux
On 06/08/12 14:55, Paul Moore wrote:
> On Friday, June 08, 2012 02:28:44 PM Christopher J. PeBenito wrote:
>>>> 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)
>
> Once again, I don't believe I ever commented on the name of the capability,
> but I am now. Disregard my comments or incorporate them, that is up to you,
> and ultimately Eric, to decide.
My apologies, you did not comment on the capability name in the previous thread.
--
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@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] Add SELinux policy capability for always checking packet class.
2012-06-08 18:28 ` Christopher J. PeBenito
2012-06-08 18:55 ` Paul Moore
@ 2012-06-08 19:33 ` Casey Schaufler
1 sibling, 0 replies; 16+ messages in thread
From: Casey Schaufler @ 2012-06-08 19:33 UTC (permalink / raw)
To: Christopher J. PeBenito; +Cc: Paul Moore, SELinux
On 6/8/2012 11:28 AM, Christopher J. PeBenito wrote:
> 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@tresys.com>
>> ...
>>
>>> 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.
Oh come on, no one does
if (...)
return x;
else
return y;
Much better to do
if (...)
return x;
return y;
The else is unnecessary.
>
>
>>> /*
>>> 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)
>
>
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/2] Add SELinux policy capability for always checking packet class.
@ 2012-06-06 17:28 Chris PeBenito
2012-06-06 17:28 ` [PATCH 2/2] Update SELinux policy capability to always check peer class Chris PeBenito
0 siblings, 1 reply; 16+ messages in thread
From: Chris PeBenito @ 2012-06-06 17:28 UTC (permalink / raw)
To: SELinux; +Cc: Chris PeBenito
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@tresys.com>
---
security/selinux/hooks.c | 8 ++++++--
security/selinux/include/security.h | 2 ++
security/selinux/selinuxfs.c | 3 ++-
security/selinux/ss/services.c | 3 +++
4 files changed, 13 insertions(+), 3 deletions(-)
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
@@ -137,12 +137,16 @@ static struct kmem_cache *sel_inode_cache;
* This function checks the SECMARK reference counter to see if any SECMARK
* targets are currently configured, if the reference counter is greater than
* zero SECMARK is considered to be enabled. Returns true (1) if SECMARK is
- * enabled, false (0) if SECMARK is disabled.
+ * enabled, false (0) if SECMARK is disabled. If the always_check_network
+ * policy capability is enabled, SECMARK is always considered enabled.
*
*/
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);
}
/*
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;
/*
* type_datum properties
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"
};
unsigned int selinux_checkreqprot = CONFIG_SECURITY_SELINUX_CHECKREQPROT_VALUE;
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 4321b8f..e124d8f 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -72,6 +72,7 @@
int selinux_policycap_netpeer;
int selinux_policycap_openperm;
+int selinux_policycap_alwaysnetwork;
static DEFINE_RWLOCK(policy_rwlock);
@@ -1812,6 +1813,8 @@ static void security_load_policycaps(void)
POLICYDB_CAPABILITY_NETPEER);
selinux_policycap_openperm = ebitmap_get_bit(&policydb.policycaps,
POLICYDB_CAPABILITY_OPENPERM);
+ selinux_policycap_alwaysnetwork = ebitmap_get_bit(&policydb.policycaps,
+ POLICYDB_CAPABILITY_ALWAYSNETWORK);
}
static int security_preserve_bools(struct policydb *p);
--
1.7.8.6
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 2/2] Update SELinux policy capability to always check peer class.
2012-06-06 17:28 Chris PeBenito
@ 2012-06-06 17:28 ` Chris PeBenito
2012-06-07 14:28 ` Paul Moore
0 siblings, 1 reply; 16+ messages in thread
From: Chris PeBenito @ 2012-06-06 17:28 UTC (permalink / raw)
To: SELinux; +Cc: Chris PeBenito
Update the always_check_network policy capability which, when enabled,
treats peer labeling as enabled, even if there is no Netlabel or
labeled IPSEC configuration.
Signed-off-by: Chris PeBenito <cpebenito@tresys.com>
---
security/selinux/hooks.c | 20 +++++++++++++++++++-
1 files changed, 19 insertions(+), 1 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index ec7151b..01c52b7 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -149,6 +149,24 @@ static int selinux_secmark_enabled(void)
return (atomic_read(&selinux_secmark_refcount) > 0);
}
+/**
+ * selinux_peerlbl_enabled - Check to see if peer labeling is currently enabled
+ *
+ * Description:
+ * This function checks if NetLabel or labeled IPSEC is enabled. Returns true
+ * (1) if any are enabled or false (0) if neither are enabled. If the
+ * always_check_network policy capability is enabled, peer labeling
+ * is always considered enabled.
+ *
+ */
+static int selinux_peerlbl_enabled(void)
+{
+ if (selinux_policycap_alwaysnetwork)
+ return 1;
+ else
+ return (netlbl_enabled() || selinux_xfrm_enabled());
+}
+
/*
* initialise the security for the init task
*/
@@ -4188,7 +4206,7 @@ static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
return selinux_sock_rcv_skb_compat(sk, skb, family);
secmark_active = selinux_secmark_enabled();
- peerlbl_active = netlbl_enabled() || selinux_xfrm_enabled();
+ peerlbl_active = selinux_peerlbl_enabled();
if (!secmark_active && !peerlbl_active)
return 0;
--
1.7.8.6
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 2/2] Update SELinux policy capability to always check peer class.
2012-06-06 17:28 ` [PATCH 2/2] Update SELinux policy capability to always check peer class Chris PeBenito
@ 2012-06-07 14:28 ` Paul Moore
2012-06-07 15:27 ` Christopher J. PeBenito
0 siblings, 1 reply; 16+ messages in thread
From: Paul Moore @ 2012-06-07 14:28 UTC (permalink / raw)
To: SELinux; +Cc: Chris PeBenito
On Wednesday, June 06, 2012 01:28:51 PM Chris PeBenito wrote:
> Update the always_check_network policy capability which, when enabled,
> treats peer labeling as enabled, even if there is no Netlabel or
> labeled IPSEC configuration.
>
> Signed-off-by: Chris PeBenito <cpebenito@tresys.com>
I still object to this patchset for all the same old reasons, but I feel
obligated to point out that this patchset is still incomplete/incorrect in
that it only deals with the socket_sock_rcv_skb hook.
--
paul moore
www.paul-moore.com
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] Update SELinux policy capability to always check peer class.
2012-06-07 14:28 ` Paul Moore
@ 2012-06-07 15:27 ` Christopher J. PeBenito
2012-06-07 15:33 ` Paul Moore
0 siblings, 1 reply; 16+ messages in thread
From: Christopher J. PeBenito @ 2012-06-07 15:27 UTC (permalink / raw)
To: Paul Moore; +Cc: SELinux
On 06/07/12 10:28, Paul Moore wrote:
> On Wednesday, June 06, 2012 01:28:51 PM Chris PeBenito wrote:
>> Update the always_check_network policy capability which, when enabled,
>> treats peer labeling as enabled, even if there is no Netlabel or
>> labeled IPSEC configuration.
>>
>> Signed-off-by: Chris PeBenito <cpebenito@tresys.com>
>
> I still object to this patchset for all the same old reasons, but I feel
> obligated to point out that this patchset is still incomplete/incorrect in
> that it only deals with the socket_sock_rcv_skb hook.
I found the missing hooks, but does this need to affect selinux_ip_output()? It seems like the answer is no, as it looks like its just applying the NetLabel on outgoing packets.
--
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@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] Update SELinux policy capability to always check peer class.
2012-06-07 15:27 ` Christopher J. PeBenito
@ 2012-06-07 15:33 ` Paul Moore
2012-06-07 16:08 ` Christopher J. PeBenito
0 siblings, 1 reply; 16+ messages in thread
From: Paul Moore @ 2012-06-07 15:33 UTC (permalink / raw)
To: Christopher J. PeBenito; +Cc: SELinux
On Thursday, June 07, 2012 11:27:00 AM Christopher J. PeBenito wrote:
> On 06/07/12 10:28, Paul Moore wrote:
> > On Wednesday, June 06, 2012 01:28:51 PM Chris PeBenito wrote:
> >> Update the always_check_network policy capability which, when enabled,
> >> treats peer labeling as enabled, even if there is no Netlabel or
> >> labeled IPSEC configuration.
> >>
> >> Signed-off-by: Chris PeBenito <cpebenito@tresys.com>
> >
> > I still object to this patchset for all the same old reasons, but I feel
> > obligated to point out that this patchset is still incomplete/incorrect in
> > that it only deals with the socket_sock_rcv_skb hook.
>
> I found the missing hooks, but does this need to affect selinux_ip_output()?
Nope.
> It seems like the answer is no, as it looks like its just applying the
> NetLabel on outgoing packets.
--
paul moore
www.paul-moore.com
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] Update SELinux policy capability to always check peer class.
2012-06-07 15:33 ` Paul Moore
@ 2012-06-07 16:08 ` Christopher J. PeBenito
2012-06-07 16:52 ` Paul Moore
0 siblings, 1 reply; 16+ messages in thread
From: Christopher J. PeBenito @ 2012-06-07 16:08 UTC (permalink / raw)
To: Paul Moore; +Cc: SELinux
On 06/07/12 11:33, Paul Moore wrote:
> On Thursday, June 07, 2012 11:27:00 AM Christopher J. PeBenito wrote:
>> On 06/07/12 10:28, Paul Moore wrote:
>>> On Wednesday, June 06, 2012 01:28:51 PM Chris PeBenito wrote:
>>>> Update the always_check_network policy capability which, when enabled,
>>>> treats peer labeling as enabled, even if there is no Netlabel or
>>>> labeled IPSEC configuration.
>>>>
>>>> Signed-off-by: Chris PeBenito <cpebenito@tresys.com>
>>>
>>> I still object to this patchset for all the same old reasons, but I feel
>>> obligated to point out that this patchset is still incomplete/incorrect in
>>> that it only deals with the socket_sock_rcv_skb hook.
>>
>> I found the missing hooks, but does this need to affect selinux_ip_output()?
>
> Nope.
Is there anything I'm missing other than selinux_ip_forward() and selinux_ip_postroute()?
--
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@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] Update SELinux policy capability to always check peer class.
2012-06-07 16:08 ` Christopher J. PeBenito
@ 2012-06-07 16:52 ` Paul Moore
2012-06-07 17:43 ` Eric Paris
0 siblings, 1 reply; 16+ messages in thread
From: Paul Moore @ 2012-06-07 16:52 UTC (permalink / raw)
To: Christopher J. PeBenito, SELinux
On Thursday, June 07, 2012 12:08:31 PM Christopher J. PeBenito wrote:
> On 06/07/12 11:33, Paul Moore wrote:
> > On Thursday, June 07, 2012 11:27:00 AM Christopher J. PeBenito wrote:
> >> On 06/07/12 10:28, Paul Moore wrote:
> >>> On Wednesday, June 06, 2012 01:28:51 PM Chris PeBenito wrote:
> >>>> Update the always_check_network policy capability which, when enabled,
> >>>> treats peer labeling as enabled, even if there is no Netlabel or
> >>>> labeled IPSEC configuration.
> >>>>
> >>>> Signed-off-by: Chris PeBenito <cpebenito@tresys.com>
> >>>
> >>> I still object to this patchset for all the same old reasons, but I feel
> >>> obligated to point out that this patchset is still incomplete/incorrect
> >>> in
> >>> that it only deals with the socket_sock_rcv_skb hook.
> >>
> >> I found the missing hooks, but does this need to affect
> >> selinux_ip_output()?>
> > Nope.
>
> Is there anything I'm missing other than selinux_ip_forward() and
> selinux_ip_postroute()?
It is probably best just to post the patches so that they can be (re)reviewed.
I'm also curious if you are going to do anything about incorporating the
secmark configuration into the greater SELinux policy. As far as I'm
concerned, that is the only way I would consider ack'ing these patches (I did
mention this in the original thread).
--
paul moore
www.paul-moore.com
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] Update SELinux policy capability to always check peer class.
2012-06-07 16:52 ` Paul Moore
@ 2012-06-07 17:43 ` Eric Paris
0 siblings, 0 replies; 16+ messages in thread
From: Eric Paris @ 2012-06-07 17:43 UTC (permalink / raw)
To: Paul Moore; +Cc: Christopher J. PeBenito, SELinux
On Thu, Jun 7, 2012 at 12:52 PM, Paul Moore <paul@paul-moore.com> wrote:
> On Thursday, June 07, 2012 12:08:31 PM Christopher J. PeBenito wrote:
> I'm also curious if you are going to do anything about incorporating the
> secmark configuration into the greater SELinux policy. As far as I'm
> concerned, that is the only way I would consider ack'ing these patches (I did
> mention this in the original thread).
As soon as this series covers all paths I am willing to take it, even
over Paul's larger objection. I don't think what Paul wants makes
enough sense to warrant not allowing enforcement in the face of a
mislabeled system. Sorry Paul. That doesn't mean I wouldn't be happy
to see something along those lines, but I think allowing people to use
and control the system the way we have it today trumps some potential
work tomorrow.
-Eric
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2012-06-08 19:43 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-06-07 18:28 [PATCH 1/2] Add SELinux policy capability for always checking packet class Chris PeBenito
2012-06-07 18:28 ` [PATCH 2/2] Update SELinux policy capability to always check peer class Chris PeBenito
2012-06-08 17:36 ` Paul Moore
2012-06-08 18:29 ` Christopher J. PeBenito
2012-06-08 17:36 ` [PATCH 1/2] Add SELinux policy capability for always checking packet class Paul Moore
2012-06-08 18:28 ` Christopher J. PeBenito
2012-06-08 18:55 ` Paul Moore
2012-06-08 19:43 ` Christopher J. PeBenito
2012-06-08 19:33 ` Casey Schaufler
-- strict thread matches above, loose matches on Subject: below --
2012-06-06 17:28 Chris PeBenito
2012-06-06 17:28 ` [PATCH 2/2] Update SELinux policy capability to always check peer class Chris PeBenito
2012-06-07 14:28 ` Paul Moore
2012-06-07 15:27 ` Christopher J. PeBenito
2012-06-07 15:33 ` Paul Moore
2012-06-07 16:08 ` Christopher J. PeBenito
2012-06-07 16:52 ` Paul Moore
2012-06-07 17:43 ` Eric Paris
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.