Type transition file names are stored in a symbol table. Before the
name is added, the symbol table is searched to see if the name had
already been inserted. If it has, then the already existing datum is
returned. If it has not, then the name is added if either the
typetransition rule does not occur in a macro or the name is not one
of the macro parameters.
Checking for a previous insertion before checking if the name is a
macro parameter can cause a macro parameter to be treated as the
actual name if a previous type transition file name is the same as
the parameter.
Now check the name to see if it a macro paramter before checking for
its existence in the symbol table.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Ondrej Mosnacek <omosnace@redhat.com>
The filename_- and range_trans_table ancillary hash tables in
cil_binary.c just duplicate the final policydb content and can be simply
removed.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
The classperms associated with each map class permission and with each
classpermissionset are verified in __cil_verify_classperms() which had
multiple problems with how it did the verification.
1) Verification was short-circuited when the first normal class is found.
The second classpermissionset statement below would not have been
verified.
(classpermission cp1)
(classpermissionset cp1 (CLASS (PERM)))
(classpermissionset cp1 cp2)
2) The classperms of a map class permission and classpermissionset were
not checked for being NULL before the function recursively called itself.
This would result in a segfault if the missing map or set was referred to
before the classmap or classpermission occured. This error was reported by
Dominick Grift (dominick.grift@defensec.nl).
These rules would cause a segfault.
(classmap cm1 (mp1))
(classmapping cm1 mp1 (cm2 (mp2)))
(classmap cm2 (mp2))
But an error would be produced for these rules.
(classmap cm1 (mp1))
(classmap cm2 (mp2))
(classmapping cm2 mp2 (cm1 (mp1)))
3) The loop detection logic was incomplete and could only detect a loop
with a certain statement ordering.
These rules would cause a stack overflow.
(classmap cm1 (mp1))
(classmapping cm1 mp1 (cm2 (mp2)))
(classmap cm2 (mp2))
(classmapping cm2 mp2 (cm3 (mp3)))
(classmap cm3 (mp3))
(classmapping cm3 mp3 (cm2 (mp2)))
Rewrote __cil_verify_classperms() to fix these errors.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
Remove restrictions in libsepol and checkpolicy that required all
declared initial SIDs to be assigned a context. With this patch,
it is possible to build and load a policy that drops the sid <sidname>
<context> declarations for the unused initial SIDs. It is still
required to retain the sid <sidname> declarations (in the flask
definitions) in order to preserve the initial SID ordering/values.
The unused initial SIDs can be renamed, e.g. to add an unused_
prefix or similar, if desired, since the names used in the policy
are not stored in the kernel binary policy.
In CIL policies, the (sid ...) and (sidorder (...)) statements
must be left intact for compatibility but the (sidcontext ...)
statements for the unused initial SIDs can be omitted after this change.
With current kernels, if one removes an unused initial SID context
from policy, builds policy with this change applied and loads the
policy into the kernel, cat /sys/fs/selinux/initial_contexts/<sidname>
will show the unlabeled context. With the kernel patch to remove unused
initial SIDs, the /sys/fs/selinux/initial_contexts/<sidname>
file will not be created for unused initial SIDs in the first place.
NB If an unused initial SID was assigned a context different from
the unlabeled context in existing policy, then it is not safe to
remove that initial SID context from policy and reload policy on
the running kernel that was booted with the original policy. This
is because that kernel may have assigned that SID to various kernel
objects already and those objects will then be treated as having
the unlabeled context after the removal. In refpolicy, examples
of such initial SIDs are the "fs" SID and the "sysctl" SID. Even
though these initial SIDs are not directly used (in code) by the current
kernel, their contexts are being applied to filesystems and sysctl files by
policy and therefore the SIDs are being assigned to objects.
NB The "sysctl" SID was in use by the kernel up until
commit 8e6c96935fcc1ed3dbebc96fddfef3f2f2395afc ("security/selinux:
fix /proc/sys/ labeling) circa v2.6.39. Removing its context from
policy will cause sysctl(2) or /proc/sys accesses to end up
performing permission checks against the unlabeled context and
likely encounter denials for kernels < 2.6.39.
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
Commit 4459d635b8 ("libsepol: Remove cil_mem_error_handler() function
pointer") replaced cil_mem_error_handler usage with inline contents of
the default handler. However, it left over the header declaration and
two callers. Convert these as well and remove the header declaration.
This also fixes a build failure with -fno-common.
Fixes: 4459d635b8 ("libsepol: Remove cil_mem_error_handler() function pointer")
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
GCC 10 comes with -fno-common enabled by default - fix the CIL_KEY_*
global variables to be defined only once in cil.c and declared in the
header file correctly with the 'extern' keyword, so that other units
including the file don't generate duplicate definitions.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
When copying an avrule with extended permissions (permx) in
cil_copy_avrule(), the check for a named permx checks the new permx
instead of the old one, so the check will always fail. This leads to a
segfault when trying to copy a named permx because there will be an
attempt to copy the nonexistent permx struct instead of the name of
the named permx.
Check whether the original is a named permx instead of the new one.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
Since failing to resolve a statement in an optional block is normal,
only display messages about the statement failing to resolve and the
optional block being disabled at the highest verbosity level.
These messages are now only at log level CIL_INFO instead of CIL_WARN.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
Use codespell (https://github.com/codespell-project/codespell) in order
to find many common misspellings that are present in English texts.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
As reported by Nicolas Iooss (nicolas.iooss@m4x.org), static analyzers
have problems understanding that the default memory error handler does
not return since it is called through the cil_mem_error_handler()
function pointer. This results in a number of false positive warnings
about null pointer dereferencing.
Since the ability to set the cil_mem_error_handler() is only through
the function cil_set_mem_error_handler() which is never used and whose
definition is not in any header file, remove that function, remove the
use of cil_mem_error_handler() and directly in-line the contents of
the default handler, cil_default_mem_error_handler().
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
This patch is loosely based on a patch by Yuli Khodorkovskiy
<yuli@crunchydata.com> from June 13th, 2019.
Since any permission used in the policy should be defined, CIL
should return an error if it cannot resolve a permission used
in a policy. This was the original behavior of CIL.
The behavior was changed over three commits from July to November
2016 (See commits 46e157b47, da51020d6, and 2eefb20d8). The change
was motivated by Fedora trying to remove permissions from its
policy that were never upstreamed (ex/ process ptrace_child and
capability2 compromise_kernel). Local or third party modules
compiled with those permissions would break policy updates.
After three years it seems unlikely that we need to worry about
those local and third party modules and it is time for CIL to
give an error like it should.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
Policy developers can set a default_range default to glblub and
computed contexts will be the intersection of the ranges of the
source and target contexts. This can be used by MLS userspace
object managers to find the range of clearances that two contexts
have in common. An example usage is computing a transition between
the network context and the context of a user logging into an MLS
application.
For example, one can add a default with
this cil:
(defaultrange db_table glblub)
or in te (base module only):
default_range db_table glblub;
and then test using the compute_create utility:
$ ./compute_create system_u:system_r:kernel_t:s0:c1,c2,c5-s0:c1.c20 system_u:system_r:kernel_t:s0:c0.c20-s0:c0.c36 db_table
system_u:object_r:kernel_t:s0:c1,c2,c5-s0:c1.c20
Some example range transitions are:
User Permitted Range | Network Device Label | Computed Label
---------------------|----------------------|----------------
s0-s1:c0.c12 | s0 | s0
s0-s1:c0.c12 | s0-s1:c0.c1023 | s0-s1:c0.c12
s0-s4:c0.c512 | s1-s1:c0.c1023 | s1-s1:c0.c512
s0-s15:c0,c2 | s4-s6:c0.c128 | s4-s6:c0,c2
s0-s4 | s2-s6 | s2-s4
s0-s4 | s5-s8 | INVALID
s5-s8 | s0-s4 | INVALID
Signed-off-by: Joshua Brindle <joshua.brindle@crunchydata.com>
Installing a cil module with invalid mlsconstrain syntax currently
results in a segfault. In the following module, the right-hand side of
the second operand of the OR is a list (mlstrustedobject):
$ cat test.cil
(class test (foo) )
(classorder (unordered test))
(mlsconstrain (test (foo))
(or
(dom h1 h2)
(eq t2 (mlstrustedobject))
)
)
$ sudo semodule -i test.cil
zsh: segmentation fault sudo semodule -i test.cil
This syntax is invalid and should error accordingly, rather than
segfaulting. This patch provides this syntax error for the same module:
$ sudo semodule -i test.cil
t1, t2, r1, r2, u1, u2 cannot be used on the left side with a list on the right side
Bad expression tree for constraint
Bad constrain declaration at /var/lib/selinux/mls/tmp/modules/400/test/cil:4
semodule: Failed!
Signed-off-by: Mike Palmiotto <mike.palmiotto@crunchydata.com>
When validatetrans rule is in CIL policy it errors with:
u3, r3, and t3 can only be used with mlsvalidatetrans rules
Will now resolve these examples:
(validatetrans binder (and (and (eq t1 t1_t) (eq t2 t2_t)) (eq t3 t3_t)))
(mlsvalidatetrans file (and (and (eq t1 t1_t) (eq t2 t2_t))
(and (eq t3 t3_t) (domby h1 h2))))
Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
Most of the users of ebitmap_for_each_bit() macro only care for the set
bits, so introduce a new ebitmap_for_each_positive_bit() macro that
skips the unset bits. Replace uses of ebitmap_for_each_bit() with the
new macro where appropriate.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
clang's static analyze reports a use-after-free in
__cil_expr_to_string(), when __cil_expr_to_string_helper() does not
modify its third parameter (variable s1 here) in this loop:
for (curr = curr->next; curr; curr = curr->next) {
__cil_expr_to_string_helper(curr, flavor, &s1);
cil_asprintf(&c2, "%s %s", c1, s1);
free(c1);
free(s1);
c1 = c2;
}
Silence this warning by making sure s1 is always NULL at the beginning
of every iteration of the loop.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
These were reported by Petr Lautrbach (plautrba@redhat.com) and this
patch was based on his patch with only a few changes.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
In cil_tree_print_expr(), "rc < 0" is equivalent to "rc != 0" but
clang's static analyzer does not know about this. Help it.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Improve the processing of netifcon, genfscon, ibpkeycon, ibendportcon,
portcon, nodecon, fsuse, filecon, iomemcon, ioportcon, pcidevicecon,
and devicetreecon rules.
If the multiple-decls option is not used then report errors if duplicate
context rules are found. If it is used then remove duplicate context rules
and report errors when two rules are identical except for the context.
This also changes the ordering of portcon and filecon rules. The protocol
of portcon rules will be compared if the port numbers are the same and the
path strings of filecon rules will be compared if the number of meta
characters, the stem length, string length and file types are the same.
Based on an initial patch by Pierre-Hugues Husson (phh@phh.me)
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
This commit resolves conflicts in values of expandattribute statements
in policy language and expandtypeattribute in CIL.
For example, these statements resolve to false in policy language:
expandattribute hal_audio true;
expandattribute hal_audio false;
Similarly, in CIL these also resolve to false.
(expandtypeattribute (hal_audio) true)
(expandtypeattribute (hal_audio) false)
A warning will be issued on this conflict.
Motivation
When Android combines multiple .cil files from system.img and vendor.img
it's possible to have conflicting expandattribute statements.
This change deals with this scenario by resolving the value of the
corresponding expandtypeattribute to false. The rationale behind this
override is that true is used for reduce run-time lookups, while
false is used for tests which must pass.
Signed-off-by: Tri Vo <trong@android.com>
Acked-by: Jeff Vander Stoep <jeffv@google.com>
Acked-by: William Roberts <william.c.roberts@intel.com>
Acked-by: James Carter <jwcart2@tycho.nsa.gov>
cil_tree_print_expr() calls cil_expr_to_string() in order to compute a
string expression into expr_str. If this function fails, expr_str is
left unitialized but its value is dereferenced with:
cil_log(CIL_INFO, "%s)", expr_str);
Prevent such an issue by checking cil_expr_to_string()'s return value
before using expr_str.
This issue has been found with clang's static analyzer.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Daniel Cashman <dcashman@android.com> discovered the following:
When using cil_db multiple_decls, the different cil_attribute nodes
all point to the same underlying cil_attribute struct. This leads
to problems, though, when modifying the used value in the struct.
__cil_post_db_attr() changes the value of the field to based on
the output of cil_typeattribute_used(), for use later in
cil_typeattribute_to_policydb and cil_typeattribute_to_bitmap, but
due to the multiple declarations, cil_typeattribute_used() could be
called again by a second node. In this second call, the value used
is the modifed value of CIL_TRUE or CIL_FALSE, not the flags actually
needed. This could result in the field being reset again, to an
incorrect CIL_FALSE value.
Add the field "keep" to struct cil_typeattributeset, set its value
using cil_typeattribute_used(), and use it when determining whether
the attribute is to be kept or if it should be expanded.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
cil_gen_node() has been using its argument "db" since commit
fafe4c212b ("libsepol: cil: Add ability to redeclare
types[attributes]"). Drop attribute "unused" on this argument.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
cil_defaults_to_policy() defines its third argument as non-const "char
*kind" even though it is called with literal strings. This makes gcc
report the following warning when compiling with -Wwrite-strings:
../cil/src/cil_policy.c: In function ‘cil_gen_policy’:
../cil/src/cil_policy.c:1931:60: error: passing argument 3 of
‘cil_defaults_to_policy’ discards ‘const’ qualifier from pointer
target type [-Werror=discarded-qualifiers]
cil_defaults_to_policy(out, lists[CIL_LIST_DEFAULT_USER],
"default_user");
^~~~~~~~~~~~~~
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Since commit 67b410e80f ("libsepol/cil: Keep attributes used by
generated attributes in neverallow rules") gcc reports the following
warning when building libsepol:
../cil/src/cil_post.c: In function
‘__cil_post_db_neverallow_attr_helper’:
../cil/src/cil_post.c:1322:17: error: unused variable ‘db’
[-Werror=unused-variable]
struct cil_db *db = extra_args;
^~
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
In order to reduce policy size, CIL removes attributes that are not used
by a policy rule in the generated binary policy. However, CIL keeps
attributes used by neverallow rules (which are checked at compile time
and not in the binary policy) even if the attribute is not used anywhere
else in the policy. This behavior is useful to Google who pulls neverallow
rules out of the original policy.conf for compatibility testing, but
converts the policy.conf to CIL and uses the CIL compiler to generate
policy. Without this behavior, the generated binary policy might not have
an attribute referred to by one of the neverallow rules used for testing.
The one exception to this behavior is for attributes generated in
module_to_cil (these have an "_typeattr_" in the middle of their name).
Since these attributes are only created because CIL does not allow a
type expression in an AV rule, they are removed if they only appear in
a neverallow rule (which is the case for most of them) or if the
option to expand generated attributes (-G or --expand-generated) is
specified for secilc when compiling the policy.
Removing generated attributes causes a problem, however, if the type
expression that the generated attribute is replacing uses an attribute
that is removed. In this case, the original neverallow rule will refer
to an attribute that does not exist in the generated binary policy.
Now any non-generated attribute used in a typeattributeset rule for a
generated attribute which is used in a neverallow rule will be treated
like it was used in a neverallow rule.
This does not change the behavior of an expandtypeattribute rule for
the attribute. That rule, if it exists, will take precedence.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
Modify cil_gen_node() to check to see if the cil_db supports multiple
declarations, and if so, to check whether or not the
repeated symbol is eligible to share the existing, already-stored datum. The
only types considered so far are CIL_TYPE and CIL_TYPEATTRIBUTE, both of
which intall empty datums during AST building, so they automatically return
true.
Test: Build policy with multilpe type and attribute declarations, and
without. Policies are binary-identical.
Signed-off-by: Dan Cashman <dcashman@android.com>
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
In cil_strpool_destroy(), cil_strpool_tab is freed but it is not reset to NULL.
When cil_strpool_init() is called again it assumes that cil_strpool_tab was
already initialized. Other functions then work with invalid data.
Signed-off-by: Jan Zarsky <jzarsky@redhat.com>
The typebounds rules should end with a ";".
The netifcon and nodecon rules should not end with a ";".
The default rules are missing a "_". They should be "default_user",
"default_role" and "default_type".
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
Nicolas Iooss <nicolas.iooss@m4x.org> discovered with clang's static
analyzer that cil_reset_ibpkeycon() was checking that ibpkeycon->context
was NULL and then passing the NULL value to cil_reset_context() which
expected a non-NULL argument.
Instead, cil_reset_ibpkeycon() should check if ibpkeycon->context_str
is NULL. If it is non-NULL then the context field points to a named
context that was created elsewhere and it will be reset there, but if
the context_str field is NULL, then the context is not named and needs
to be reset.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
This prevented cil_resolve_name() from returning an actual thing when a
name resolved to an alias. This appears to have only affected resolution
dealing with sensitivity and category aliases. Type aliases were not
affected since places that dealt with types handled type aliases
specifically and did not rely on this behavior from cil_resolve_name().
Signed-off-by: Steve Lawrence <slawrence@tresys.com>
https://developers.redhat.com/blog/2017/03/10/wimplicit-fallthrough-in-gcc-7/
Fixes the following warnings by annotating with a /* FALLTHRU */ comment.
Unfortunately, the __attribute__ ((fallthrough)); approach does not appear
to work with older compilers.
../cil/src/cil_parser.c: In function ‘cil_parser’:
../cil/src/cil_parser.c:253:14: warning: this statement may fall through [-Wimplicit-fallthrough=]
tok.value = tok.value+1;
~~~~~~~~~~^~~~~~~~~~~~~
../cil/src/cil_parser.c:254:3: note: here
case SYMBOL:
^~~~
../cil/src/cil_parser.c:275:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
if (tok.type != END_OF_FILE) {
^
../cil/src/cil_parser.c:279:3: note: here
case END_OF_FILE:
^~~~
../cil/src/cil_post.c: In function ‘cil_post_fc_fill_data’:
../cil/src/cil_post.c:104:5: warning: this statement may fall through [-Wimplicit-fallthrough=]
c++;
~^~
../cil/src/cil_post.c:105:3: note: here
default:
^~~~~~~
regex.c: In function ‘regex_format_error’:
regex.c:541:10: warning: this statement may fall through [-Wimplicit-fallthrough=]
*ptr++ = '.';
~~~~~~~^~~~~
regex.c:542:2: note: here
case 3:
^~~~
regex.c:543:10: warning: this statement may fall through [-Wimplicit-fallthrough=]
*ptr++ = '.';
~~~~~~~^~~~~
regex.c:544:2: note: here
case 2:
^~~~
regex.c:545:10: warning: this statement may fall through [-Wimplicit-fallthrough=]
*ptr++ = '.';
~~~~~~~^~~~~
regex.c:546:2: note: here
case 1:
^~~~
regex.c: In function ‘regex_format_error’:
regex.c:541:10: warning: this statement may fall through [-Wimplicit-fallthrough=]
*ptr++ = '.';
~~~~~~~^~~~~
regex.c:542:2: note: here
case 3:
^~~~
regex.c:543:10: warning: this statement may fall through [-Wimplicit-fallthrough=]
*ptr++ = '.';
~~~~~~~^~~~~
regex.c:544:2: note: here
case 2:
^~~~
regex.c:545:10: warning: this statement may fall through [-Wimplicit-fallthrough=]
*ptr++ = '.';
~~~~~~~^~~~~
regex.c:546:2: note: here
case 1:
^~~~
modules.c: In function ‘semanage_module_get_path’:
modules.c:602:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
if (file == NULL) file = "hll";
^
modules.c:603:3: note: here
case SEMANAGE_MODULE_PATH_CIL:
^~~~
modules.c:604:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
if (file == NULL) file = "cil";
^
modules.c:605:3: note: here
case SEMANAGE_MODULE_PATH_LANG_EXT:
^~~~
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
- If two typealiasactual statements exist for the same typealias, we get
a confusing error message mentioning that the actual arguement is not
an alias, which is clearly allowed. This poor error occurs because the
first typealiasactual statement resolves correctly, but when we
resolve the alias in the second typealiasactual statement,
cil_resolve_name tries to return what the alias points to, which is a
type and not the required typealias. This patch creates a new function
that does not perform the alias to actual conversion, used when we
want an alias and not what the alias points to. This allows the
cil_resolve_aliasactual to continue and reach the check for duplicate
typealiasactual statements, resulting in a more meaningful error
message.
- Add back support for aliases to aliases (broken in 5c9fcb02e),
while still ensuring that aliases point to either the correct actual
flavor or alias flavor, and not something else like a typeattribute.
Signed-off-by: Steve Lawrence <slawrence@tresys.com>
- Set rc to SEPOL_ERR if the alias part of an aliasactual statement
does not resolve to the correct alias flavor (e.g. typealias, senalias, catalias)
- Add an error check if the actual part of an aliasactual statement
does not resolve to the correct actual flavor (type, sens, cat)
Signed-off-by: Steve Lawrence <slawrence@tresys.com>
In __cil_fqn_qualify_blocks(), when newlen >= CIL_MAX_NAME_LENGTH,
cil_tree_log() is called with child_args.node as argument but this value
has not been initialized yet. Use local variable node instead, which is
initialized early enough in the function.
This issue has been found using clang's static analyzer.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
This commit adds attribute expansion statements to the policy
language allowing compiler defaults to be overridden.
Always expands an attribute example:
expandattribute { foo } true;
CIL example:
(expandtypeattribute (foo) true)
Never expand an attribute example:
expandattribute { bar } false;
CIL example:
(expandtypeattribute (bar) false)
Adding the annotations directly to policy was chosen over other
methods as it is consistent with how targeted runtime optimizations
are specified in other languages. For example, in C the "inline"
command.
Motivation
expandattribute true:
Android has been moving away from a monolithic policy binary to
a two part split policy representing the Android platform and the
underlying vendor-provided hardware interface. The goal is a stable
API allowing these two parts to be updated independently of each
other. Attributes provide an important mechanism for compatibility.
For example, when the vendor provides a HAL for the platform,
permissions needed by clients of the HAL can be granted to an
attribute. Clients need only be assigned the attribute and do not
need to be aware of the underlying types and permissions being
granted.
Inheriting permissions via attribute creates a convenient mechanism
for independence between vendor and platform policy, but results
in the creation of many attributes, and the potential for performance
issues when processes are clients of many HALs. [1] Annotating these
attributes for expansion at compile time allows us to retain the
compatibility benefits of using attributes without the performance
costs. [2]
expandattribute false:
Commit 0be23c3f15 added the capability to aggresively remove unused
attributes. This is generally useful as too many attributes assigned
to a type results in lengthy policy look up times when there is a
cache miss. However, removing attributes can also result in loss of
information used in external tests. On Android, we're considering
stripping neverallow rules from on-device policy. This is consistent
with the kernel policy binary which also did not contain neverallows.
Removing neverallow rules results in a 5-10% decrease in on-device
policy build and load and a policy size decrease of ~250k. Neverallow
rules are still asserted at build time and during device
certification (CTS). If neverallow rules are absent when secilc is
run, some attributes are being stripped from policy and neverallow
tests in CTS may be violated. [3] This change retains the aggressive
attribute stripping behavior but adds an override mechanism to
preserve attributes marked as necessary.
[1] https://github.com/SELinuxProject/cil/issues/9
[2] Annotating all HAL client attributes for expansion resulted in
system_server's dropping from 19 attributes to 8. Because these
attributes were not widely applied to other types, the final
policy size change was negligible.
[3] data_file_type and service_manager_type are stripped from AOSP
policy when using secilc's -G option. This impacts 11 neverallow
tests in CTS.
Test: Build and boot Marlin with all hal_*_client attributes marked
for expansion. Verify (using seinfo and sesearch) that permissions
are correctly expanded from attributes to types.
Test: Mark types being stripped by secilc with "preserve" and verify
that they are retained in policy and applied to the same types.
Signed-off-by: Jeff Vander Stoep <jeffv@google.com>
cil_gen_default() and cil_gen_defaultrange() call cil_fill_list()
without checking its return value. If it failed, propagate the return
value to the caller.
This issue has been found using clang's static analyzer. It reported
"warning: Value stored to 'rc' is never read" four times.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Originally, all type attributes were expanded when building a binary
policy. As the policy grew, binary policy sizes became too large, so
changes were made to keep attributes in the binary policy to minimize
policy size.
Keeping attributes works well as long as each type does not have too
many attributes. If an access check fails for types t1 and t2, then
additional checks must be made for every attribute that t1 is a member
of against t2 and all the attributes that t2 is a member of. This is
O(n*m) behavior and there are cases now where this is becoming a
performance issue.
Attributes are more aggressively removed than before. An attribute
will now be removed if it only appears in rules where attributes are
always expanded (typetransition, typechange, typemember, roletransition,
rangetransition, roletype, and AV Rules with self).
Attributes that are used in constraints are always kept because the
attribute name is stored for debugging purposes in the binary policy.
Attributes that are used in neverallow rules, but not in other AV rules,
will be kept unless the attribute is auto-generated.
Attributes that are only used in AV rules other than neverallow rules
are kept unless the number of types assigned to them is less than the
value of attrs_expand_size in the CIL db. The default is 1, which means
that any attribute that has no types assigned to it will be expanded (and
the rule removed from the policy), which is CIL's current behavior. The
value can be set using the function cil_set_attrs_expand_size().
Auto-generated attributes that are used only in neverallow rules are
always expanded. The rest are kept by default, but if the value of
attrs_expand_generated in the CIL db is set to true, they will be
expanded. The function cil_set_attrs_expand_generated() can be used
to set the value.
When creating the binary policy, CIL will expand all attributes that
are being removed and it will expand all attributes with less members
than the value specified by attrs_expand_size. So even if an attribute
is used in a constraint or neverallow and the attribute itself will be
included in the binary policy, it will be expanded when writing AV
rules if it has less members than attrs_expand_size.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
When writing a policy.conf file from CIL source, use hexadecimal
numbers in ioportcon, iomemcon, and pcidevicecon rules.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
Allow the use of hexadecimal numbers in iomemcon, ioportcon, and
pcidevicecon statements. The use of hexadecimal numbers is often
the natural choice for these rules.
A zero base is now passed to strtol() and strtoull() which will
assume base 16 if the string has a prefix of "0x", base 8 if the
string starts with "0", and base 10 otherwise.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
