Commit Graph

110 Commits

Author SHA1 Message Date
James Carter
2d49a4b41c libsepol/cil: Create new keep field for type attribute sets
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>
2017-11-22 11:49:19 -05:00
Nicolas Iooss
13e5fa3b6b libsepol/cil: drop wrong unused attribute
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>
2017-09-05 12:37:23 -04:00
Nicolas Iooss
12f3ef8280 libsepol/cil: fix -Wwrite-strings warning
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>
2017-09-05 12:37:20 -04:00
Nicolas Iooss
3ab3a218f2 libsepol/cil: __cil_post_db_neverallow_attr_helper() does not use extra_args
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>
2017-09-05 12:37:16 -04:00
James Carter
67b410e80f libsepol/cil: Keep attributes used by generated attributes in neverallow rules
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>
2017-08-31 14:44:59 -04:00
Dan Cashman
fafe4c212b libsepol: cil: Add ability to redeclare types[attributes]
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>
2017-08-31 14:42:25 -04:00
Jan Zarsky
1346746d82 libsepol: reset pointer after free
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>
2017-08-28 15:50:13 -04:00
Dan Cashman
7803c8ca99 libsepol: cil: enable cpp compilation of cil.h.
Signed-off-by: Daniel Cashman <dcashman@google.com>
2017-07-26 13:24:22 -04:00
James Carter
5a553e8287 libsepol/cil: Fix bugs when writing policy.conf rules
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>
2017-06-15 13:21:53 -04:00
Richard Haines
c8e135ba22 libsepol/cil: ibendportcon fails to resolve in CIL policy
Fix named ibendportcon context not resolving correctly.

Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
2017-06-12 11:13:25 -04:00
James Carter
738db6077b libsepol/cil: Fix bug in cil_reset_ibpkeycon()
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>
2017-06-05 11:30:14 -04:00
Steve Lawrence
0be11881d1 libsepol/cil: fix error check in new cil_resolve_name
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>
2017-06-02 12:18:16 -04:00
Stephen Smalley
e41ae676c2 libsepol,libsemanage,libselinux: Fix fallthrough warnings from gcc 7
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>
2017-06-01 13:35:45 -04:00
James Carter
800f6b2a89 libsepol/cil: Remove uneeded null checks of unused parameters
Issue reported by Nicola Iooss <nicolas.iooss@m4x.org>

Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
2017-06-01 12:43:31 -04:00
Steve Lawrence
e501d3b6e8 libsepol/cil: better error message with duplicate aliases + support aliases to aliases
- 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>
2017-06-01 12:17:29 -04:00
Steve Lawrence
5c9fcb02ec libsepol/cil: fix aliasactual resolution errors
- 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>
2017-05-31 12:34:23 -04:00
Daniel Jurgens
28663ff135 libsepol: Add IB end port handling to CIL
Add IB end port parsing, symbol table management, and policy generation
to CIL.

Signed-off-by: Daniel Jurgens <danielj@mellanox.com>
2017-05-23 16:20:55 -04:00
Daniel Jurgens
e564f7b5bd libsepol: Add Infiniband Pkey handling to CIL
Add Infiniband pkey parsing, symbol table management, and policy
generation to CIL.

Signed-off-by: Daniel Jurgens <danielj@mellanox.com>
2017-05-23 16:20:54 -04:00
Nicolas Iooss
4a0fab43cb libsepol/cil: do not use an uninitialized value in __cil_fqn_qualify_blocks
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>
2017-05-15 16:36:49 -04:00
Jeff Vander Stoep
1089665e31 Add attribute expansion options
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>
2017-05-09 12:09:46 -04:00
Nicolas Iooss
b63eb892f9 libsepol: cil: check cil_fill_list return value
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>
2017-04-12 14:40:16 -04:00
James Carter
0be23c3f15 libsepol/cil: Add ability to expand some attributes in binary policy
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>
2017-04-12 14:33:49 -04:00
James Carter
af0ce03ec7 libsepol/cil: Add hexadecimal support for Xen ioportcon statements
Add hexadecimal support for Xen ioportcon statements which was
left out of commit c408c70.

Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
2017-03-28 13:33:05 -04:00
James Carter
da2f2316a3 libsepol/cil: Use hexadecimal numbers when writing Xen rules
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>
2017-03-21 14:22:42 -04:00
James Carter
c408c70b0a libsepol/cil: Allow hexadecimal numbers in Xen context rules
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>
2017-03-21 14:22:34 -04:00
Nicolas Iooss
6707526f1f libsepol/cil: avoid freeing uninitialized values
cil_resolve_ast() begins by checking whether one of its parameters is
NULL and "goto exit;" when it is the case. As extra_args has not been
initialized there, this leads to calling cil_destroy_tree_node_stack(),
__cil_ordered_lists_destroy()... on garbage values.

In practise this cannot happen because cil_resolve_ast() is only called
by cil_compile() after cil_build_ast() succeeded. As the if condition
exists nonetheless, fix the body of the if block in order to silence a
warning reported by clang Static Analyzer.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2017-03-21 14:20:22 -04:00
Nicolas Iooss
0864814583 libsepol/cil: make reporting conflicting type transitions work
When compiling a CIL policy which defines conflicting type transitions,
secilc crashes when trying to format an error message with uninitialized
values. This is caused by __cil_typetransition_to_avtab() not
initializing the ..._str fields of its local variable "struct
cil_type_rule trans" before calling __cil_type_rule_to_avtab().

While at it, make the error report clearer about what is wrong by
showing the types and classes which got expanded in
__cil_type_rule_to_avtab(). Here is an example of the result:

    Conflicting type rules (scontext=testuser_emacs.subj
    tcontext=fs.tmpfs.fs tclass=dir
    result=users.generic_tmpfs.user_tmpfs_file),
    existing=emacs.tmpfs.user_tmpfs_file

    Expanded from type rule (scontext=ARG1 tcontext=fs tclass=ARG3
    result=ARG2)

Reported-By: Dominick Grift <dac.override@gmail.com>
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2017-03-21 14:20:17 -04:00
Nicolas Iooss
ddaf0afec7 libsepol/cil: do not dereference args before checking it was not null
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2017-03-01 10:42:23 -05:00
James Carter
a2d40aaeba libsepol/cil: Move initialization of bitmap in __cil_permx_to_bitmap()
Nicolas Iooss reports:
  When __cil_permx_to_bitmap() calls __cil_permx_str_to_int() on an
  invalid number, local variablt "bitmap" is left initialized when
  the function returns and its memory is leaked.

  This memory leak has been found by running clang's Address Sanitizer
  on a set of policies generated by American Fuzzy Lop.

Move the initialization of bitmap to right before ebitmap_set_bit()
and after the call to __cil_permx_str_to_int().

Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
2017-02-21 13:11:15 -05:00
Nicolas Iooss
95e5c103f3 libsepol/cil: free bitmaps in cil_level_equals()
cil_level_equals() builds two bitmap and compare them but does not
destroy them before returning the result.

This memory leak has been found by running clang's Address Sanitizer on
a set of policies generated by American Fuzzy Lop.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2017-02-21 13:09:49 -05:00
Nicolas Iooss
9feaf0380d libsepol/cil: do not leak left-hand side of an invalid constraint
__cil_fill_constraint_expr() does not destroy the list associated with
the first operand of a two-operand operation when the second operand is
invalid.

This memory leak can be reproduced with the following policy:

    (constrain (files (read))
        (not (or (and (eq t1 exec_t) (%q t2 bin_t)) (eq r1 r2))))

This memory leak has been found by running clang's Address Sanitizer on
a set of policies generated from secilc/test/policy.cil by American
Fuzzy Lop.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2017-02-21 13:09:44 -05:00
Nicolas Iooss
602385d70c libsepol/cil: free the first operand if the second one is invalid
When __cil_expr_to_bitmap() fails to parse the second operand of an
operation with two operands, it returns an error without destroying the
bitmap which has been created for the first operand. Fix this memory
leak.

This has been tested with the following policy:

    (class CLASS (PERM))
    (classorder (CLASS))
    (sid SID)
    (sidorder (SID))
    (user USER)
    (role ROLE)
    (type TYPE)
    (category CAT)
    (categoryorder (CAT))
    (sensitivity SENS)
    (sensitivityorder (SENS))
    (sensitivitycategory SENS (CAT))
    (allow TYPE self (CLASS (PERM)))
    (roletype ROLE TYPE)
    (userrole USER ROLE)
    (userlevel USER (SENS))
    (userrange USER ((SENS)(SENS (CAT))))
    (sidcontext SID (USER ROLE TYPE ((SENS)(SENS))))

    (permissionx ioctl_test (ioctl CLASS
        (and (range 0x1600 0x19FF) (.ot (range 0x1750 0x175F)))))

This memory leak has been found by running clang's Address Sanitizer on
a set of policies generated from secilc/test/policy.cil by American
Fuzzy Lop.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2017-02-21 13:09:39 -05:00
Nicolas Iooss
7fe9a7be31 libsepol/cil: use __cil_ordered_lists_destroy() to free unordered_classorder_lists
In cil_resolve_ast, unordered_classorder_lists is a list of
cil_ordered_list. It needs to be destroyed with
__cil_ordered_lists_destroy() to free all associated memory.

This has been tested with the following policy:

    (class CLASS1 ())
    (class CLASS2 ())
    (classorder (unordered CLASS1))
    (classorder (CLASS2))

This memory leak has been found by running clang's Address Sanitizer on
a set of policies generated by American Fuzzy Lop.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2017-02-21 13:09:22 -05:00
James Carter
9edcf28a04 libsepol/cil: Destroy cil_tree_node stacks when finished resolving AST
CIL uses separate cil_tree_node stacks for optionals and blocks to
check for statements not allowed in optionals or blocks and to know
which optional to disable when necessary. But these stacks were not
being destroyed when exiting cil_resolve_ast(). This is not a problem
normally because the stacks will be empty, but this is not the case
when exiting with an error.

Destroy both tree node stacks when exiting to ensure that they are
empty.

Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
2017-02-17 10:53:52 -05:00
Nicolas Iooss
eeafde1351 libsepol/cil: fix type confusion in cil_copy_ast
When running secilc on the following CIL file, the program tries to free
the data associated with type X using cil_destroy_typeattribute():

    (macro sys_obj_type ((user ARG1)) (typeattribute X))

    (block B
        (type X)
        (call sys_obj_type (Y))
    )

By adding some printf statements to cil_typeattribute_init(),
cil_type_init() and cil_destroy_typeattribute(), the error message I get
when using gcc's address sanitizer is:

$ secilc -o /dev/null -f /dev/null test.cil -vvvvvv
creating TYPE 0x60400000dfd0
Parsing 2017-02-02_crashing_nulptrderef_cil.cil
Building AST from Parse Tree
creating TYPEATTR 0x60600000e420
creating TYPE 0x60400000df50
Destroying Parse Tree
Resolving AST
Failed to resolve call statement at 2017-02-02_crashing_nulptrderef_cil.cil:5
Problem at 2017-02-02_crashing_nulptrderef_cil.cil:5
Pass 8 of resolution failed
Failed to resolve ast
Failed to compile cildb: -2
Destroying TYPEATTR 0x60600000e420, types (nil) name X
Destroying TYPEATTR 0x60400000df50, types 0xbebebebe00000000 name X
ASAN:DEADLYSIGNAL
=================================================================
==30684==ERROR: AddressSanitizer: SEGV on unknown address
0x000000000000 (pc 0x7fc0539d114a bp 0x7ffc1fbcb300 sp
0x7ffc1fbcb2f0 T0)
    #0 0x7fc0539d1149 in ebitmap_destroy /usr/src/selinux/libsepol/src/ebitmap.c:356
    #1 0x7fc053b96201 in cil_destroy_typeattribute ../cil/src/cil_build_ast.c:2370
    #2 0x7fc053b42ea4 in cil_destroy_data ../cil/src/cil.c:616
    #3 0x7fc053c595bf in cil_tree_node_destroy ../cil/src/cil_tree.c:235
    #4 0x7fc053c59819 in cil_tree_children_destroy ../cil/src/cil_tree.c:201
    #5 0x7fc053c59958 in cil_tree_subtree_destroy ../cil/src/cil_tree.c:172
    #6 0x7fc053c59a27 in cil_tree_destroy ../cil/src/cil_tree.c:165
    #7 0x7fc053b44fd7 in cil_db_destroy ../cil/src/cil.c:299
    #8 0x4026a1 in main /usr/src/selinux/secilc/secilc.c:335
    #9 0x7fc0535e5290 in __libc_start_main (/usr/lib/libc.so.6+0x20290)
    #10 0x403af9 in _start (/usr/src/selinux/DESTDIR/usr/bin/secilc+0x403af9)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /usr/src/selinux/libsepol/src/ebitmap.c:356 in ebitmap_destroy
==30684==ABORTING

When copying the AST tree in cil_resolve_call1(),
__cil_copy_node_helper() calls cil_copy_typeattribute() to grab type X
in the symbol table of block B, and creates a node with the data of X
but with CIL_TYPEATTRIBUTE flavor.

This example is a "type confusion" bug between cil_type and
cil_typeattribute structures. It can be generalized to any couple of
structures sharing the same symbol table (an easy way of finding other
couples is by reading the code of cil_flavor_to_symtab_index()).

Fix this issue in a "generic" way in __cil_copy_node_helper(), by
verifying that the flavor of the found data is the same as expected and
triggering an error when it is not.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2017-02-08 10:48:40 -05:00
Nicolas Iooss
d6b5b037f9 libsepol: fix -Wwrite-strings warnings
When compiling with -Wwrite-strings, clang reports some warnings like:

    module_to_cil.c:784:13: error: assigning to 'char *' from 'const
    char [5]' discards qualifiers
    [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
                    statement = "type";
                              ^ ~~~~~~
    module_to_cil.c:787:13: error: assigning to 'char *' from 'const
    char [5]' discards qualifiers
    [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
                    statement = "role";
                              ^ ~~~~~~

Add a const type attribute to local variables which only handle constant
strings.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2017-02-06 11:05:33 -05:00
Nicolas Iooss
69ec21ce6a libsepol: remove useless assignments
There is no point in initializing a variable which gets
almost-immediately assigned an other value.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2017-01-19 08:48:36 -05:00
Nicolas Iooss
fd9e5ef7b7 libsepol: use constant keys in hashtab functions
Even though "hashtab_key_t" is an alias for "char *", "const
hashtab_key_t" is not an alias for "(const char) *" but means "(char *)
const".

Introduce const_hashtab_key_t to map "(const char) *" and use it in
hashtab_search() and hashtab key comparison functions.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2017-01-19 08:46:19 -05:00
Gary Tierney
af18b86e0b libsepol/cil: remove avrules with no affected types
Adds a check for avrules with type attributes that have a bitmap cardinality
of 0 (i.e., no types in their set) before adding them to the libsepol policy in
__cil_avrule_to_avtab().  Also adds an exception for neverallow rules to
prevent breaking anything from AOSP mentioned in
f9927d9370.

Signed-off-by: Gary Tierney <gary.tierney@gmx.com>
2016-12-13 10:56:59 -05:00
James Carter
3fe4499f7d libsepol/cil: Add ability to write policy.conf file from CIL AST
The ability to create a policy.conf file from the CIL AST has been
a desire from the beginning to assist in debugging and for general
flexibility. Some work towards this end was started early in CIL's
history, but cil_policy.c has not been remotely functional in a long
time. Until now.

The function cil_write_policy_conf() will write a policy.conf file
from a CIL AST after cil_build_ast(), cil_resolve_ast(),
cil_fqn_qualify(), and cil_post_process() have been called.

Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
2016-11-30 10:18:12 -05:00
Stephen Smalley
8fdb225521 libsepol,checkpolicy: convert rangetrans and filenametrans to hashtabs
range transition and name-based type transition rules were originally
simple unordered lists.  They were converted to hashtabs in the kernel
by commit 2f3e82d694d3d7a2db019db1bb63385fbc1066f3 ("selinux: convert range
transition list to a hashtab") and by commit
2463c26d50adc282d19317013ba0ff473823ca47 ("SELinux: put name based
create rules in a hashtable"), but left unchanged in libsepol and
checkpolicy. Convert libsepol and checkpolicy to use the same hashtabs
as the kernel for the range transitions and name-based type transitions.

With this change and the preceding one, it is possible to directly compare
a policy file generated by libsepol/checkpolicy and the kernel-generated
/sys/fs/selinux/policy pseudo file after normalizing them both through
checkpolicy.  To do so, you can run the following sequence of commands:

checkpolicy -M -b /etc/selinux/targeted/policy/policy.30 -o policy.1
checkpolicy -M -b /sys/fs/selinux/policy -o policy.2
cmp policy.1 policy.2

Normalizing the two files via checkpolicy is still necessary to ensure
consistent ordering of the avtab entries.  There may still be potential
for other areas of difference, e.g. xperms entries may lack a well-defined
order.

Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
2016-11-28 13:10:59 -05:00
dcashman
4750ec2ed5 libsepol: cil: remove double-free.
Test: Untested patch.
Bug: https://code.google.com/p/android/issues/detail?id=226519
Change-Id: Icaf992ba1487098f2c4f16ac1017012f611281e9
Signed-off-by: Daniel Cashman <dcashman@android.com>
2016-11-15 10:48:26 -05:00
James Carter
2eefb20d8f libsepol/cil: Exit with an error for an unknown map permission
Nicholas Iooss discovered that using an unknown permission with a
map class will cause a segfault.

CIL will only give a warning when it fails to resolve an unknown
permission to support the use of policy module packages that use
permissions that don't exit on the current system. When resolving
the unknown map class permission an empty list is used to represent
the unknown permission. When it is evaluated later the list is
assumed to be a permission and a segfault occurs.

There is no reason to allow unknown class map permissions because
the class maps and permissions are defined by the policy.

Exit with an error when failing to resolve a class map permission.

Reported-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
2016-11-02 12:14:21 -04:00
Jason Zaman
2d1380f811 libsepol: Add symver with explicit version to build with ld.gold
The blank default symver fails to compile with ld.gold. This updates the
symver from blank to LIBSEPOL_1.0. The dynamic linker will first look
for the symbol with the explicit version specified. If there is none, it
will pick the first listed symbol so there is no breakage.
This also matches how symvers are defined in libsemanage.

Signed-off-by: Jason Zaman <jason@perfinion.com>
2016-10-31 12:50:24 -04:00
dcashman
d7cb38ff87 libsepol: cil: cil_strpool: Allow multiple strpool users.
cil_strpool currently provides an interface to a statically stored
global data structure.  This interface does not accomodate multiple
consumers, however, as two calls to cil_strpool_init() will lead to a
memory leak and a call to cil_strpool_destroy() by one consumer will
remove data from use by others, and subsequently lead to a segfault on
the next cil_strpool_destroy() invocation.

Add a reference counter so that the strpool is only initialized once and
protect the exported interface with a mutex.

Tested by calling cil_db_init() on two cil_dbs and then calling
cil_db_destroy() on each.

Signed-off-by: Daniel Cashman <dcashman@android.com>
2016-10-19 10:17:03 -04:00
James Carter
410634d650 libsepol/cil: Verify neither child nor parent in a bounds is an attribute
Nicolas Iooss found while fuzzing secilc with AFL that using an attribute
as a child in a typebounds statement will cause a segfault.

This happens because the child datum is assumed to be part of a cil_type
struct when it is really part of a cil_typeattribute struct. The check to
verify that it is a type and not an attribute comes after it is used.

This bug effects user and role bounds as well because they do not check
whether a datum refers to an attribute or not.

Add checks to verify that neither the child nor the parent datum refer
to an attribute before using them in user, role, and type bounds.

Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
2016-10-19 10:08:15 -04:00
James Carter
0fcc430add libsepol/cil: Verify alias in aliasactual statement is really an alias
Nicolas Iooss found while fuzzing secilc with AFL that the statement
"(sensitivityaliasactual SENS SENS)" will cause a segfault.

The segfault occurs because when the aliasactual is resolved the first
identifier is assumed to refer to an alias structure, but it is not.

Add a check to verify that the datum retrieved is actually an alias
and exit with an error if it is not.

Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
2016-10-19 10:08:09 -04:00
James Carter
166b260d75 libsepol/cil: Check that permission is not an empty list
Nicolas Iooss found while fuzzing secilc with AFL that the statement
"(class C (()))" will cause a segfault.

CIL expects a list of permissions in the class declaration and "(())"
is a valid list. Each item of the list is expected to be an identifier
and as the list is processed each item is checked to see if it is a
list. An error is given if it is a list, otherwise the item is assumed
to be an identifier. Unfortunately, the check only works if the list
is not empty. In this case, the item passes the check and is assumed
to be an identifier and a NULL is passed as the string for name
verification. If name verification assumes that a non-NULL value will
be passed in, a segfault will occur.

Add a check for an empty list when processing a permission list and
improve the error handling for permissions when building the AST.

Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
2016-10-19 10:08:02 -04:00
James Carter
3aa292620c libsepol/cil: Check if identifier is NULL when verifying name
Nicolas Iooss found while fuzzing secilc with AFL that the statement
"(class C (()))" will cause a segfault.

When CIL checks the syntax of the class statement it sees "(())" as a
valid permission list, but since "()" is not an identifier a NULL is
passed as the string for name verification. A segfault occurs because
name verification assumes that the string being checked is non-NULL.

Check if identifier is NULL when verifying name.

Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
2016-10-19 10:07:51 -04:00
James Carter
da51020d6f libsepol/cil: Use an empty list to represent an unknown permission
Nicolas Iooss found while fuzzing secilc with AFL that the statement
"(classpermissionset CPERM (CLASS (and unknow PERM)))" will cause a
segfault.

In order to support a policy module package using a permission that
does not exist on the system it is loaded on, CIL will only give a
warning when it fails to resolve an unknown permission. CIL itself will
just ignore the unknown permission. This means that an expression like
"(and UNKNOWN p1)" will look like "(and p1)" to CIL, but, since syntax
checking has already been done, CIL won't know that the expression is not
well-formed. When the expression is evaluated a segfault will occur
because all expressions are assumed to be well-formed at evaluation time.

Use an empty list to represent an unknown permission so that expressions
will continue to be well-formed and expression evaluation will work but
the unknown permission will still be ignored.

Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
2016-10-19 10:07:43 -04:00