204 lines
6.5 KiB
Diff
204 lines
6.5 KiB
Diff
From 7ea59202db8d20806d9ae552acd1875c3a978bcc Mon Sep 17 00:00:00 2001
|
|
From: Stephen Smalley <sds@tycho.nsa.gov>
|
|
Date: Mon, 23 May 2016 10:54:11 -0400
|
|
Subject: [PATCH] selinux: Only apply bounds checking to source types
|
|
|
|
The current bounds checking of both source and target types
|
|
requires allowing any domain that has access to the child
|
|
domain to also have the same permissions to the parent, which
|
|
is undesirable. Drop the target bounds checking.
|
|
|
|
KaiGai Kohei originally removed all use of target bounds in
|
|
commit 7d52a155e38d ("selinux: remove dead code in
|
|
type_attribute_bounds_av()") but this was reverted in
|
|
commit 2ae3ba39389b ("selinux: libsepol: remove dead code in
|
|
check_avtab_hierarchy_callback()") because it would have
|
|
required explicitly allowing the parent any permissions
|
|
to the child that the child is allowed to itself.
|
|
|
|
This change in contrast retains the logic for the case where both
|
|
source and target types are bounded, thereby allowing access
|
|
if the parent of the source is allowed the corresponding
|
|
permissions to the parent of the target. Further, this change
|
|
reworks the logic such that we only perform a single computation
|
|
for each case and there is no ambiguity as to how to resolve
|
|
a bounds violation.
|
|
|
|
Under the new logic, if the source type and target types are both
|
|
bounded, then the parent of the source type must be allowed the same
|
|
permissions to the parent of the target type. If only the source
|
|
type is bounded, then the parent of the source type must be allowed
|
|
the same permissions to the target type.
|
|
|
|
Examples of the new logic and comparisons with the old logic:
|
|
1. If we have:
|
|
typebounds A B;
|
|
then:
|
|
allow B self:process <permissions>;
|
|
will satisfy the bounds constraint iff:
|
|
allow A self:process <permissions>;
|
|
is also allowed in policy.
|
|
|
|
Under the old logic, the allow rule on B satisfies the
|
|
bounds constraint if any of the following three are allowed:
|
|
allow A B:process <permissions>; or
|
|
allow B A:process <permissions>; or
|
|
allow A self:process <permissions>;
|
|
However, either of the first two ultimately require the third to
|
|
satisfy the bounds constraint under the old logic, and therefore
|
|
this degenerates to the same result (but is more efficient - we only
|
|
need to perform one compute_av call).
|
|
|
|
2. If we have:
|
|
typebounds A B;
|
|
typebounds A_exec B_exec;
|
|
then:
|
|
allow B B_exec:file <permissions>;
|
|
will satisfy the bounds constraint iff:
|
|
allow A A_exec:file <permissions>;
|
|
is also allowed in policy.
|
|
|
|
This is essentially the same as #1; it is merely included as
|
|
an example of dealing with object types related to a bounded domain
|
|
in a manner that satisfies the bounds relationship. Note that
|
|
this approach is preferable to leaving B_exec unbounded and having:
|
|
allow A B_exec:file <permissions>;
|
|
in policy because that would allow B's entrypoints to be used to
|
|
enter A. Similarly for _tmp or other related types.
|
|
|
|
3. If we have:
|
|
typebounds A B;
|
|
and an unbounded type T, then:
|
|
allow B T:file <permissions>;
|
|
will satisfy the bounds constraint iff:
|
|
allow A T:file <permissions>;
|
|
is allowed in policy.
|
|
|
|
The old logic would have been identical for this example.
|
|
|
|
4. If we have:
|
|
typebounds A B;
|
|
and an unbounded domain D, then:
|
|
allow D B:unix_stream_socket <permissions>;
|
|
is not subject to any bounds constraints under the new logic
|
|
because D is not bounded. This is desirable so that we can
|
|
allow a domain to e.g. connectto a child domain without having
|
|
to allow it to do the same to its parent.
|
|
|
|
The old logic would have required:
|
|
allow D A:unix_stream_socket <permissions>;
|
|
to also be allowed in policy.
|
|
|
|
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
|
|
[PM: re-wrapped description to appease checkpatch.pl]
|
|
Signed-off-by: Paul Moore <paul@paul-moore.com>
|
|
---
|
|
security/selinux/ss/services.c | 70 +++++++++++++-----------------------------
|
|
1 file changed, 22 insertions(+), 48 deletions(-)
|
|
|
|
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
|
|
index 89df646..082b20c 100644
|
|
--- a/security/selinux/ss/services.c
|
|
+++ b/security/selinux/ss/services.c
|
|
@@ -543,7 +543,7 @@ static void type_attribute_bounds_av(struct context *scontext,
|
|
struct av_decision *avd)
|
|
{
|
|
struct context lo_scontext;
|
|
- struct context lo_tcontext;
|
|
+ struct context lo_tcontext, *tcontextp = tcontext;
|
|
struct av_decision lo_avd;
|
|
struct type_datum *source;
|
|
struct type_datum *target;
|
|
@@ -553,67 +553,41 @@ static void type_attribute_bounds_av(struct context *scontext,
|
|
scontext->type - 1);
|
|
BUG_ON(!source);
|
|
|
|
+ if (!source->bounds)
|
|
+ return;
|
|
+
|
|
target = flex_array_get_ptr(policydb.type_val_to_struct_array,
|
|
tcontext->type - 1);
|
|
BUG_ON(!target);
|
|
|
|
- if (source->bounds) {
|
|
- memset(&lo_avd, 0, sizeof(lo_avd));
|
|
-
|
|
- memcpy(&lo_scontext, scontext, sizeof(lo_scontext));
|
|
- lo_scontext.type = source->bounds;
|
|
+ memset(&lo_avd, 0, sizeof(lo_avd));
|
|
|
|
- context_struct_compute_av(&lo_scontext,
|
|
- tcontext,
|
|
- tclass,
|
|
- &lo_avd,
|
|
- NULL);
|
|
- if ((lo_avd.allowed & avd->allowed) == avd->allowed)
|
|
- return; /* no masked permission */
|
|
- masked = ~lo_avd.allowed & avd->allowed;
|
|
- }
|
|
+ memcpy(&lo_scontext, scontext, sizeof(lo_scontext));
|
|
+ lo_scontext.type = source->bounds;
|
|
|
|
if (target->bounds) {
|
|
- memset(&lo_avd, 0, sizeof(lo_avd));
|
|
-
|
|
memcpy(&lo_tcontext, tcontext, sizeof(lo_tcontext));
|
|
lo_tcontext.type = target->bounds;
|
|
-
|
|
- context_struct_compute_av(scontext,
|
|
- &lo_tcontext,
|
|
- tclass,
|
|
- &lo_avd,
|
|
- NULL);
|
|
- if ((lo_avd.allowed & avd->allowed) == avd->allowed)
|
|
- return; /* no masked permission */
|
|
- masked = ~lo_avd.allowed & avd->allowed;
|
|
+ tcontextp = &lo_tcontext;
|
|
}
|
|
|
|
- if (source->bounds && target->bounds) {
|
|
- memset(&lo_avd, 0, sizeof(lo_avd));
|
|
- /*
|
|
- * lo_scontext and lo_tcontext are already
|
|
- * set up.
|
|
- */
|
|
+ context_struct_compute_av(&lo_scontext,
|
|
+ tcontextp,
|
|
+ tclass,
|
|
+ &lo_avd,
|
|
+ NULL);
|
|
|
|
- context_struct_compute_av(&lo_scontext,
|
|
- &lo_tcontext,
|
|
- tclass,
|
|
- &lo_avd,
|
|
- NULL);
|
|
- if ((lo_avd.allowed & avd->allowed) == avd->allowed)
|
|
- return; /* no masked permission */
|
|
- masked = ~lo_avd.allowed & avd->allowed;
|
|
- }
|
|
+ masked = ~lo_avd.allowed & avd->allowed;
|
|
|
|
- if (masked) {
|
|
- /* mask violated permissions */
|
|
- avd->allowed &= ~masked;
|
|
+ if (likely(!masked))
|
|
+ return; /* no masked permission */
|
|
|
|
- /* audit masked permissions */
|
|
- security_dump_masked_av(scontext, tcontext,
|
|
- tclass, masked, "bounds");
|
|
- }
|
|
+ /* mask violated permissions */
|
|
+ avd->allowed &= ~masked;
|
|
+
|
|
+ /* audit masked permissions */
|
|
+ security_dump_masked_av(scontext, tcontext,
|
|
+ tclass, masked, "bounds");
|
|
}
|
|
|
|
/*
|
|
--
|
|
2.7.4
|
|
|