* [PATCH] selinux: fix bug in conditional rules handling
@ 2015-11-23 21:07 Stephen Smalley
2015-11-23 21:23 ` Paul Moore
2015-11-24 18:51 ` Paul Moore
0 siblings, 2 replies; 4+ messages in thread
From: Stephen Smalley @ 2015-11-23 21:07 UTC (permalink / raw)
To: selinux; +Cc: bigon, paul, jeffv, Stephen Smalley
commit fa1aa143ac4a ("selinux: extended permissions for ioctls") introduced
a bug into the handling of conditional rules, skipping the processing
entirely when the caller does not provide an extended permissions (xperms)
structure. Access checks from userspace using /sys/fs/selinux/access
do not include such a structure since that interface does not presently
expose extended permission information. As a result, conditional rules
were being ignored entirely on userspace access requests, producing
denials when access was allowed by conditional rules in the policy.
Fix the bug by only skipping computation of extended permissions
in this situation, not the entire conditional rules processing.
Reported-by: Laurent Bigonville <bigon@debian.org>
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
---
security/selinux/ss/conditional.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c
index 18643bf..456e1a9 100644
--- a/security/selinux/ss/conditional.c
+++ b/security/selinux/ss/conditional.c
@@ -638,7 +638,7 @@ void cond_compute_av(struct avtab *ctab, struct avtab_key *key,
{
struct avtab_node *node;
- if (!ctab || !key || !avd || !xperms)
+ if (!ctab || !key || !avd)
return;
for (node = avtab_search_node(ctab, key); node;
@@ -657,7 +657,7 @@ void cond_compute_av(struct avtab *ctab, struct avtab_key *key,
if ((u16)(AVTAB_AUDITALLOW|AVTAB_ENABLED) ==
(node->key.specified & (AVTAB_AUDITALLOW|AVTAB_ENABLED)))
avd->auditallow |= node->datum.u.data;
- if ((node->key.specified & AVTAB_ENABLED) &&
+ if (xperms && (node->key.specified & AVTAB_ENABLED) &&
(node->key.specified & AVTAB_XPERMS))
services_compute_xperms_drivers(xperms, node);
}
--
2.4.3
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] selinux: fix bug in conditional rules handling
2015-11-23 21:07 [PATCH] selinux: fix bug in conditional rules handling Stephen Smalley
@ 2015-11-23 21:23 ` Paul Moore
2015-11-23 21:30 ` Stephen Smalley
2015-11-24 18:51 ` Paul Moore
1 sibling, 1 reply; 4+ messages in thread
From: Paul Moore @ 2015-11-23 21:23 UTC (permalink / raw)
To: Stephen Smalley; +Cc: selinux, bigon, jeffv
On Mon, Nov 23, 2015 at 4:07 PM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> commit fa1aa143ac4a ("selinux: extended permissions for ioctls") introduced
> a bug into the handling of conditional rules, skipping the processing
> entirely when the caller does not provide an extended permissions (xperms)
> structure. Access checks from userspace using /sys/fs/selinux/access
> do not include such a structure since that interface does not presently
> expose extended permission information. As a result, conditional rules
> were being ignored entirely on userspace access requests, producing
> denials when access was allowed by conditional rules in the policy.
> Fix the bug by only skipping computation of extended permissions
> in this situation, not the entire conditional rules processing.
>
> Reported-by: Laurent Bigonville <bigon@debian.org>
> Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
> ---
> security/selinux/ss/conditional.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Looks fine to me, thanks for the fix guys. I'm going to mark this for
stable unless I hear a strong objection from anyone.
> diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c
> index 18643bf..456e1a9 100644
> --- a/security/selinux/ss/conditional.c
> +++ b/security/selinux/ss/conditional.c
> @@ -638,7 +638,7 @@ void cond_compute_av(struct avtab *ctab, struct avtab_key *key,
> {
> struct avtab_node *node;
>
> - if (!ctab || !key || !avd || !xperms)
> + if (!ctab || !key || !avd)
> return;
>
> for (node = avtab_search_node(ctab, key); node;
> @@ -657,7 +657,7 @@ void cond_compute_av(struct avtab *ctab, struct avtab_key *key,
> if ((u16)(AVTAB_AUDITALLOW|AVTAB_ENABLED) ==
> (node->key.specified & (AVTAB_AUDITALLOW|AVTAB_ENABLED)))
> avd->auditallow |= node->datum.u.data;
> - if ((node->key.specified & AVTAB_ENABLED) &&
> + if (xperms && (node->key.specified & AVTAB_ENABLED) &&
> (node->key.specified & AVTAB_XPERMS))
> services_compute_xperms_drivers(xperms, node);
> }
> --
> 2.4.3
>
--
paul moore
www.paul-moore.com
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] selinux: fix bug in conditional rules handling
2015-11-23 21:23 ` Paul Moore
@ 2015-11-23 21:30 ` Stephen Smalley
0 siblings, 0 replies; 4+ messages in thread
From: Stephen Smalley @ 2015-11-23 21:30 UTC (permalink / raw)
To: Paul Moore; +Cc: selinux, bigon, jeffv
On 11/23/2015 04:23 PM, Paul Moore wrote:
> On Mon, Nov 23, 2015 at 4:07 PM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
>> commit fa1aa143ac4a ("selinux: extended permissions for ioctls") introduced
>> a bug into the handling of conditional rules, skipping the processing
>> entirely when the caller does not provide an extended permissions (xperms)
>> structure. Access checks from userspace using /sys/fs/selinux/access
>> do not include such a structure since that interface does not presently
>> expose extended permission information. As a result, conditional rules
>> were being ignored entirely on userspace access requests, producing
>> denials when access was allowed by conditional rules in the policy.
>> Fix the bug by only skipping computation of extended permissions
>> in this situation, not the entire conditional rules processing.
>>
>> Reported-by: Laurent Bigonville <bigon@debian.org>
>> Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
>> ---
>> security/selinux/ss/conditional.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> Looks fine to me, thanks for the fix guys. I'm going to mark this for
> stable unless I hear a strong objection from anyone.
Yes, definitely should go to stable.
>
>> diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c
>> index 18643bf..456e1a9 100644
>> --- a/security/selinux/ss/conditional.c
>> +++ b/security/selinux/ss/conditional.c
>> @@ -638,7 +638,7 @@ void cond_compute_av(struct avtab *ctab, struct avtab_key *key,
>> {
>> struct avtab_node *node;
>>
>> - if (!ctab || !key || !avd || !xperms)
>> + if (!ctab || !key || !avd)
>> return;
>>
>> for (node = avtab_search_node(ctab, key); node;
>> @@ -657,7 +657,7 @@ void cond_compute_av(struct avtab *ctab, struct avtab_key *key,
>> if ((u16)(AVTAB_AUDITALLOW|AVTAB_ENABLED) ==
>> (node->key.specified & (AVTAB_AUDITALLOW|AVTAB_ENABLED)))
>> avd->auditallow |= node->datum.u.data;
>> - if ((node->key.specified & AVTAB_ENABLED) &&
>> + if (xperms && (node->key.specified & AVTAB_ENABLED) &&
>> (node->key.specified & AVTAB_XPERMS))
>> services_compute_xperms_drivers(xperms, node);
>> }
>> --
>> 2.4.3
>>
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] selinux: fix bug in conditional rules handling
2015-11-23 21:07 [PATCH] selinux: fix bug in conditional rules handling Stephen Smalley
2015-11-23 21:23 ` Paul Moore
@ 2015-11-24 18:51 ` Paul Moore
1 sibling, 0 replies; 4+ messages in thread
From: Paul Moore @ 2015-11-24 18:51 UTC (permalink / raw)
To: Stephen Smalley; +Cc: selinux, bigon, jeffv
On Monday, November 23, 2015 04:07:41 PM Stephen Smalley wrote:
> commit fa1aa143ac4a ("selinux: extended permissions for ioctls") introduced
> a bug into the handling of conditional rules, skipping the processing
> entirely when the caller does not provide an extended permissions (xperms)
> structure. Access checks from userspace using /sys/fs/selinux/access
> do not include such a structure since that interface does not presently
> expose extended permission information. As a result, conditional rules
> were being ignored entirely on userspace access requests, producing
> denials when access was allowed by conditional rules in the policy.
> Fix the bug by only skipping computation of extended permissions
> in this situation, not the entire conditional rules processing.
>
> Reported-by: Laurent Bigonville <bigon@debian.org>
> Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
> ---
> security/selinux/ss/conditional.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Merged into the SELinux stable branch, I'll push it to James tomorrow.
> diff --git a/security/selinux/ss/conditional.c
> b/security/selinux/ss/conditional.c index 18643bf..456e1a9 100644
> --- a/security/selinux/ss/conditional.c
> +++ b/security/selinux/ss/conditional.c
> @@ -638,7 +638,7 @@ void cond_compute_av(struct avtab *ctab, struct
> avtab_key *key, {
> struct avtab_node *node;
>
> - if (!ctab || !key || !avd || !xperms)
> + if (!ctab || !key || !avd)
> return;
>
> for (node = avtab_search_node(ctab, key); node;
> @@ -657,7 +657,7 @@ void cond_compute_av(struct avtab *ctab, struct
> avtab_key *key, if ((u16)(AVTAB_AUDITALLOW|AVTAB_ENABLED) ==
> (node->key.specified & (AVTAB_AUDITALLOW|AVTAB_ENABLED)))
> avd->auditallow |= node->datum.u.data;
> - if ((node->key.specified & AVTAB_ENABLED) &&
> + if (xperms && (node->key.specified & AVTAB_ENABLED) &&
> (node->key.specified & AVTAB_XPERMS))
> services_compute_xperms_drivers(xperms, node);
> }
--
paul moore
www.paul-moore.com
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-11-25 19:07 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-23 21:07 [PATCH] selinux: fix bug in conditional rules handling Stephen Smalley
2015-11-23 21:23 ` Paul Moore
2015-11-23 21:30 ` Stephen Smalley
2015-11-24 18:51 ` Paul Moore
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.