Commit graph

84 commits

Author SHA1 Message Date
lujiev
27e1c7c8e9 checkpolicy: delete invalid spaces
Closes: https://github.com/SELinuxProject/selinux/pull/372
Signed-off-by: lujiev <572084868@qq.com>
Acked-by: Jason Zaman <jason@perfinion.com>
2023-01-15 14:52:25 -08:00
Vit Mojzis
1d33c911f5 checkpolicy: Improve error message for type bounds
Make the error message consistent with other occurrences of the
same issue:
https://github.com/SELinuxProject/selinux/blob/master/checkpolicy/module_compiler.c#L243
https://github.com/SELinuxProject/selinux/blob/master/checkpolicy/module_compiler.c#L488

Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
Acked-by: James Carter <jwcart2@gmail.com>
2022-12-16 16:29:55 -05:00
Christian Göttsche
aaaed69911 checkpolicy: simplify string copying
Use strdup(3) instead of allocating memory and then manually copying the
content.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
2022-11-21 14:30:06 -05:00
Juraj Marcin
c916f0884b checkpolicy: avoid passing NULL pointer to memset()
Function `class_perm_node_init()` is called with `dest_perms` before it
is checked that its allocation succeeded. If the allocation fails, then
a NULL pointer is passed to `memset()` inside the
`class_perm_node_init()` function.

Signed-off-by: Juraj Marcin <juraj@jurajmarcin.com>
2022-09-01 09:27:01 -04:00
Christian Göttsche
2a9c619b5f checkpolicy: use strict function prototype for definitions
Clang 15 starts to complain about non strict function definitions:

    policy_define.c:4907:30: error: a function declaration without a prototype is deprecated in all versions of C [-Werror,-Wstrict-prototypes]
    int define_devicetree_context()
                                 ^
                                  void
    policy_define.c:5298:29: error: a function declaration without a prototype is deprecated in all versions of C [-Werror,-Wstrict-prototypes]
    int define_ipv4_node_context()
                                ^
                                 void

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Reviewed-by: Daniel Burgener <dburgener@linux.microsoft.com>
Acked-by: James Carter <jwcart2@gmail.com>
2022-08-15 08:46:41 -04:00
Ondrej Mosnacek
9e096e6ef0 libsepol,checkpolicy: add support for self keyword in type transitions
With the addition of the anon_inode class in the kernel, 'self'
transition rules became useful, but haven't been implemented.

The typetransition, typemember, and typechange statements share the
relevant code, so this patch implements the self keyword in all of them
at the TE language level and adds the support to the module policydb
format. Note that changing the kernel policydb format is not necessary
at all, as type transitions are always expanded in the kernel policydb.

Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Acked-by: James Carter <jwcart2@gmail.com>
2022-05-16 10:37:17 -04:00
Christian Göttsche
5645f803e1 checkpolicy: mention class name on invalid permission
When a permission for a constraint statement cannot be found also
mention the related class name.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2022-05-04 15:12:04 +02:00
Christian Göttsche
4be0e2e19c checkpolicy: allow wildcard permissions in constraints
Allow all and complement permission sets in constraints, e.g.:

    constrain service ~ { status } (...);
    constrain service * (...);

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2022-02-18 14:48:57 -05:00
Christian Göttsche
01b88ac323 checkpolicy: warn on bogus IP address or netmask in nodecon statement
Warn if the netmask is not contiguous or the address has host bits set,
e.g.:

    127.0.0.0 255.255.245.0
    127.0.0.1 255.255.255.0

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-12-15 12:47:22 -05:00
James Carter
ce815bd11b checkpolicy: Fix potential undefined shifts
An expression of the form "1 << x" is undefined if x == 31 because
the "1" is an int and cannot be left shifted by 31.

Instead, use "UINT32_C(1) << x" which will be an unsigned int of
at least 32 bits.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-10-12 13:44:55 -04:00
Christian Göttsche
3d27e5a410 checkpolicy: policy_define: cleanup declarations
The variable curfile is nowhere used.

Static functions do not need to be forward declared if not used before
their definition.

The error buffer errormsg can be a simple scoped variable. Also
vsnprintf(3) always NUL-terminates the buffer, so the whole length can
be passed.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-10-04 09:25:07 -04:00
Christian Göttsche
a0a342c37b checkpolicy: free extended permission memory
define_te_avtab_xperms_helper() allocates memory for the avrule, while
define_te_avtab_ioctl() does not transfer any ownership of it.
Free the affected memory.

    Direct leak of 272 byte(s) in 2 object(s) allocated from:
        #0 0x49bb8d in __interceptor_malloc (./checkpolicy/checkmodule+0x49bb8d)
        #1 0x4f379c in define_te_avtab_xperms_helper ./checkpolicy/policy_define.c:2047:24
        #2 0x4f379c in define_te_avtab_extended_perms ./checkpolicy/policy_define.c:2469:6
        #3 0x4cf417 in yyparse ./checkpolicy/policy_parse.y:494:30
        #4 0x4eaf35 in read_source_policy ./checkpolicy/parse_util.c:63:6
        #5 0x50cccd in main ./checkpolicy/checkmodule.c:278:7
        #6 0x7fbfa455ce49 in __libc_start_main csu/../csu/libc-start.c:314:16

    Direct leak of 32 byte(s) in 2 object(s) allocated from:
        #0 0x49bb8d in __interceptor_malloc (./checkpolicy/checkmodule+0x49bb8d)
        #1 0x4f4a38 in avrule_sort_ioctls ./checkpolicy/policy_define.c:1844:12
        #2 0x4f4a38 in avrule_ioctl_ranges ./checkpolicy/policy_define.c:2021:6
        #3 0x4f4a38 in define_te_avtab_ioctl ./checkpolicy/policy_define.c:2399:6
        #4 0x4f4a38 in define_te_avtab_extended_perms ./checkpolicy/policy_define.c:2475:7
        #5 0x4cf417 in yyparse ./checkpolicy/policy_parse.y:494:30
        #6 0x4eaf35 in read_source_policy ./checkpolicy/parse_util.c:63:6
        #7 0x50cccd in main ./checkpolicy/checkmodule.c:278:7
        #8 0x7fbfa455ce49 in __libc_start_main csu/../csu/libc-start.c:314:16

Reported-by: liwugang <liwugang@163.com>
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-09-15 10:14:42 -04:00
Christian Göttsche
e6bab7bf45 checkpolicy: add missing function declarations
Ideally they should be declared in the corresponding header file, but
the overall include style in the checkpolicy code is quite messy.
Declare them for now in the source file before defining them to silence
related compiler warnings:

    policy_define.c:84:6: error: no previous prototype for function 'init_parser' [-Werror,-Wmissing-prototypes]
    void init_parser(int pass_number)
         ^
    policy_define.c:93:6: error: no previous prototype for function 'yyerror2' [-Werror,-Wmissing-prototypes]
    void yyerror2(const char *fmt, ...)
         ^

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-09-15 10:14:09 -04:00
Christian Göttsche
5c376d6db1 checkpolicy: mark file local functions in policy_define static
Also remove the unused function `avrule_ioctl_freeranges()`.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-09-15 10:14:00 -04:00
Christian Göttsche
5570c2e394 checkpolicy: enclose macro argument in parentheses
Found by clang-tidy

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-09-15 10:13:48 -04:00
Christian Göttsche
1711757378
checkpolicy: mark read-only parameters in policy define const
Make it more obvious which parameters are read-only and not being
modified and allow callers to pass const pointers.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-07-13 21:02:10 +02:00
Christian Göttsche
4e3d0990c6
checkpolicy: drop redundant cast to the same type
Found by clang-tidy.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-07-13 21:02:09 +02:00
Christian Göttsche
5a10f05f53
checkpolicy: check before potential NULL dereference
policy_define.c: In function ‘define_te_avtab_extended_perms’:
    policy_define.c:1946:17: error: potential null pointer dereference [-Werror=null-dereference]
     1946 |         r->omit = omit;
          |                 ^

In the case of `r` being NULL, avrule_read_ioctls() would return
with its parameter `rangehead` being a pointer to NULL, which is
considered a failure in its caller `avrule_ioctl_ranges`.
So it is not necessary to alter the return value.

Found by GCC 11 with LTO enabled.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-07-13 21:02:08 +02:00
Christian Göttsche
79e7724930
checkpolicy: follow declaration-after-statement
Follow the project style of no declaration after statement.

Found by the GCC warning -Wdeclaration-after-statement.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-07-13 21:02:07 +02:00
Christian Göttsche
db674bf218
checkpolicy: drop dead condition
The variable `id` is guaranteed to be non-NULL due to the preceding
while condition.

    policy_define.c:1171:7: style: Condition '!id' is always false [knownConditionTrueFalse]
      if (!id) {
          ^
    policy_define.c:1170:13: note: Assuming that condition 'id=queue_remove(id_queue)' is not redundant
     while ((id = queue_remove(id_queue))) {
                ^
    policy_define.c:1171:7: note: Condition '!id' is always false
      if (!id) {
          ^

Found by Cppcheck.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-07-13 21:02:05 +02:00
James Carter
dcd07fdcbf
libsepol/checkpolicy: Set user roles using role value instead of dominance
Roles in an optional block have two datums, one in the global block
and one in the avrule_decl where it is declared. The datum in the
global block does not have its dominace set. This is a problem because
the function set_user_role() sets the user's roles based on the global
datum's dominance ebitmap. If a user is declared with an associated role
that was declared in an optional block, then it will not have any roles
set for it because the dominance ebitmap is empty.

Example/
  # handle_unknown deny
  class CLASS1
  sid kernel
  class CLASS1 { PERM1 }
  type TYPE1;
  allow TYPE1 self:CLASS1 PERM1;
  role ROLE1;
  role ROLE1 types { TYPE1 };
  optional {
    require {
      class CLASS1 { PERM1 };
    }
    role ROLE1A;
    user USER1A roles ROLE1A;
  }
  user USER1 roles ROLE1;
  sid kernel USER1:ROLE1:TYPE1

In this example, USER1A would not have ROLE1A associated with it.

Instead of using dominance, which has been deprecated anyway, just
set the bit corresponding to the role's value in the user's roles
ebitmap in set_user_role().

Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>

[N.I: added spaces around "-" operator]
2021-03-15 21:50:58 +01:00
Nicolas Iooss
b320291888 libsepol/cil: fix memory leak when a constraint expression is too deep
When __cil_validate_constrain_expr() fails,
cil_constrain_to_policydb_helper() does not destroy the constraint
expression. This leads to a memory leak reported by OSS-Fuzz with the
following CIL 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))))

    (constrain
        (CLASS (PERM))
        (or
            (eq t1 TYPE)
            (or
                (eq t1 TYPE)
                (or
                    (eq t1 TYPE)
                    (or
                        (eq t1 TYPE)
                        (or
                            (eq t1 TYPE)
                            (eq t1 TYPE)
                        )
                    )
                )
            )
        )
    )

Add constraint_expr_destroy(sepol_expr) to destroy the expression.

Moreover constraint_expr_destroy() was not freeing all items of an
expression. Code in libsepol/src and checkpolicy contained while loop to
free all the items of a constraint expression, but not the one in
libsepol/cil. As freeing only the first item of an expression is
misleading, change the semantic of constraint_expr_destroy() to iterate
the list of constraint_expr_t and to free all items.

Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28938
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Acked-by: James Carter <jwcart2@gmail.com>
2021-02-03 09:28:39 +01:00
Nicolas Iooss
521e6a2f47 libsepol/cil: fix signed overflow caused by using (1 << 31) - 1
When compiling SELinux userspace tools with -ftrapv (this option
generates traps for signed overflow on addition, subtraction,
multiplication operations, instead of silently wrapping around),
semodule crashes when running the tests from
scripts/ci/fedora-test-runner.sh in a Fedora 32 virtual machine:

    [root@localhost selinux-testsuite]# make test
    make -C policy load
    make[1]: Entering directory '/root/selinux-testsuite/policy'
    # Test for "expand-check = 0" in /etc/selinux/semanage.conf
    # General policy build
    make[2]: Entering directory '/root/selinux-testsuite/policy/test_policy'
    Compiling targeted test_policy module
    Creating targeted test_policy.pp policy package
    rm tmp/test_policy.mod.fc
    make[2]: Leaving directory '/root/selinux-testsuite/policy/test_policy'
    # General policy load
    domain_fd_use --> off
    /usr/sbin/semodule -i test_policy/test_policy.pp test_mlsconstrain.cil test_overlay_defaultrange.cil test_add_levels.cil test_glblub.cil
    make[1]: *** [Makefile:174: load] Aborted (core dumped)

Using "coredumpctl gdb" leads to the following strack trace:

    (gdb) bt
    #0  0x00007f608fe4fa25 in raise () from /lib64/libc.so.6
    #1  0x00007f608fe38895 in abort () from /lib64/libc.so.6
    #2  0x00007f6090028aca in __addvsi3.cold () from /lib64/libsepol.so.1
    #3  0x00007f6090096f59 in __avrule_xperm_setrangebits (low=30, high=30, xperms=0x8b9eea0)
        at ../cil/src/cil_binary.c:1551
    #4  0x00007f60900970dd in __cil_permx_bitmap_to_sepol_xperms_list (xperms=0xb650a30, xperms_list=0x7ffce2653b18)
        at ../cil/src/cil_binary.c:1596
    #5  0x00007f6090097286 in __cil_avrulex_ioctl_to_policydb (k=0xb8ec200 "@\023\214\022\006", datum=0xb650a30,
        args=0x239a640) at ../cil/src/cil_binary.c:1649
    #6  0x00007f609003f1e5 in hashtab_map (h=0x41f8710, apply=0x7f60900971da <__cil_avrulex_ioctl_to_policydb>,
        args=0x239a640) at hashtab.c:234
    #7  0x00007f609009ea19 in cil_binary_create_allocated_pdb (db=0x2394f10, policydb=0x239a640)
        at ../cil/src/cil_binary.c:4969
    #8  0x00007f609009d19d in cil_binary_create (db=0x2394f10, policydb=0x7ffce2653d30) at ../cil/src/cil_binary.c:4329
    #9  0x00007f609008ec23 in cil_build_policydb_create_pdb (db=0x2394f10, sepol_db=0x7ffce2653d30)
        at ../cil/src/cil.c:631
    #10 0x00007f608fff4bf3 in semanage_direct_commit () from /lib64/libsemanage.so.1
    #11 0x00007f608fff9fae in semanage_commit () from /lib64/libsemanage.so.1
    #12 0x0000000000403e2b in main (argc=7, argv=0x7ffce2655058) at semodule.c:753

    (gdb) f 3
    #3  0x00007f6090096f59 in __avrule_xperm_setrangebits (low=30, high=30, xperms=0x8b9eea0)
        at ../cil/src/cil_binary.c:1551
    1551     xperms->perms[i] |= XPERM_SETBITS(h) - XPERM_SETBITS(low);

A signed integer overflow therefore occurs in XPERM_SETBITS(h):

    #define XPERM_SETBITS(x) ((1 << (x & 0x1f)) - 1)

This macro is expanded with h=31, so "(1 << 31) - 1" is computed:

* (1 << 31) = -0x80000000 is the lowest signed 32-bit integer value
* (1 << 31) - 1 overflows the capacity of a signed 32-bit integer and
  results in 0x7fffffff (which is unsigned)

Using unsigned integers (with "1U") fixes the crash, as
(1U << 31) = 0x80000000U has no overflowing issues.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Acked-by: Petr Lautrbach <plautrba@redhat.com>
2020-10-15 19:25:05 +02:00
Ondrej Mosnacek
42ae834a74 libsepol,checkpolicy: optimize storage of filename transitions
In preparation to support a new policy format with a more optimal
representation of filename transition rules, this patch applies an
equivalent change from kernel commit c3a276111ea2 ("selinux: optimize
storage of filename transitions").

See the kernel commit's description [1] for the rationale behind this
representation. This change doesn't bring any measurable difference of
policy build performance (semodule -B) on Fedora.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git/commit/?id=c3a276111ea2572399281988b3129683e2a6b60b

Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
2020-08-03 08:52:12 -04:00
Stephen Smalley
f8c110c8a6 libsepol,checkpolicy: remove use of hardcoded security class values
libsepol carried its own (outdated) copy of flask.h with the generated
security class and initial SID values for use by the policy
compiler and the forked copy of the security server code
leveraged by tools such as audit2why.  Convert libsepol and
checkpolicy entirely to looking up class values from the policy,
remove the SECCLASS_* definitions from its flask.h header, and move
the header with its remaining initial SID definitions private to
libsepol.  While we are here, fix the sepol_compute_sid() logic to
properly support features long since added to the policy and kernel,
although there are no users of it other than checkpolicy -d (debug)
and it is not exported to users of the shared library.  There
are still some residual differences between the kernel logic and
libsepol.

Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
Acked-by: Petr Lautrbach <plautrba@redhat.com>
2020-03-12 07:50:55 +01:00
Nicolas Iooss
b550c0e202
Fix many misspellings
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>
2019-09-18 22:47:35 +02:00
Ondrej Mosnacek
3e506bda3b libsepol: add ebitmap_for_each_set_bit macro
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>
2019-05-20 14:00:32 -04:00
liwugang
98a951fa76
checkpolicy: check the result value of hashtable_search
Signed-off-by: liwugang <liwugang@xiaomi.com>
2018-09-19 20:43:39 +02:00
Tri Vo
ea8d689b53 Resolve conflicts in expandattribute.
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>
2018-03-26 12:29:37 -07:00
Richard Haines via Selinux
cf0ab12414 selinux: Add support for the SCTP portcon keyword
Update libsepol, checkpolicy and the CIL compiler to support the SCTP
portcon keyword.

Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
2018-03-19 12:34:29 -04:00
Stephen Smalley
53bb2a11c2 checkpolicy,libselinux,libsepol,policycoreutils: Update my email address
Update my email address since epoch.ncsc.mil no longer exists.

Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
2017-08-17 14:17:12 -04:00
Daniel Jurgens
5bc05dd2a5 checkpolicy: Add support for ibendportcon labels
Add checkpolicy support for scanning and parsing ibendportcon labels.
Also create a new ocontext for IB end ports.

Signed-off-by: Daniel Jurgens <danielj@mellanox.com>
2017-05-23 16:20:55 -04:00
Daniel Jurgens
5b203145fd checkpolicy: Add support for ibpkeycon labels
Add checkpolicy support for scanning and parsing ibpkeycon labels. Also
create a new ocontext for Infiniband Pkeys and define a new policydb
version for infiniband support.

Signed-off-by: Daniel Jurgens <danielj@mellanox.com>
2017-05-23 16:20:54 -04:00
Stephen Smalley
c3118041df checkpolicy,libsepol: drop unnecessary usage of s6_addr32
s6_addr32 is not portable; use s6_addr instead.
This obviates the need for #ifdef __APPLE__ conditionals in these cases.

Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
2017-05-10 10:25:56 -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
9087bb9c5a checkpolicy: dereference rangehead after checking it was not NULL
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2017-03-21 14:20:28 -04:00
Nicolas Iooss
7da9bc00f2 checkpolicy: do not leak memory when a class is not found in an avrule
While checkmodule tries to compile the following policy file and fails
because class "process" is not found, it does not free some allocated
memory:

    module ckpol_leaktest 1.0.0;
    require {type TYPE1;}
    allow TYPE1 self:process fork;

clang memory sanitier output is:

=================================================================
==16050==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 136 byte(s) in 1 object(s) allocated from:
    #0 0x7f8bd8127608 in malloc (/usr/lib/clang/3.9.1/lib/linux/libclang_rt.asan-x86_64.so+0xf6608)
    #1 0x41a620 in define_te_avtab_helper /usr/src/selinux/checkpolicy/policy_define.c:2450:24
    #2 0x41b6c8 in define_te_avtab /usr/src/selinux/checkpolicy/policy_define.c:2621:6
    #3 0x40522b in yyparse /usr/src/selinux/checkpolicy/policy_parse.y:470:10
    #4 0x411816 in read_source_policy /usr/src/selinux/checkpolicy/parse_util.c:64:6
    #5 0x7f8bd7cb3290 in __libc_start_main (/usr/lib/libc.so.6+0x20290)

Direct leak of 8 byte(s) in 1 object(s) allocated from:
    #0 0x7f8bd8127608 in malloc (/usr/lib/clang/3.9.1/lib/linux/libclang_rt.asan-x86_64.so+0xf6608)
    #1 0x411c87 in insert_id /usr/src/selinux/checkpolicy/policy_define.c:120:18

Indirect leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x7f8bd8127608 in malloc (/usr/lib/clang/3.9.1/lib/linux/libclang_rt.asan-x86_64.so+0xf6608)
    #1 0x43133c in ebitmap_set_bit /usr/src/selinux/libsepol/src/ebitmap.c:321:27

Indirect leak of 18 byte(s) in 1 object(s) allocated from:
    #0 0x7f8bd80b5eb0 in __interceptor___strdup (/usr/lib/clang/3.9.1/lib/linux/libclang_rt.asan-x86_64.so+0x84eb0)
    #1 0x41a6e5 in define_te_avtab_helper /usr/src/selinux/checkpolicy/policy_define.c:2460:28
    #2 0x41b6c8 in define_te_avtab /usr/src/selinux/checkpolicy/policy_define.c:2621:6
    #3 0x40522b in yyparse /usr/src/selinux/checkpolicy/policy_parse.y:470:10
    #4 0x411816 in read_source_policy /usr/src/selinux/checkpolicy/parse_util.c:64:6
    #5 0x7f8bd7cb3290 in __libc_start_main (/usr/lib/libc.so.6+0x20290)

SUMMARY: AddressSanitizer: 186 byte(s) leaked in 4 allocation(s).

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2017-01-23 11:26:44 -05:00
Nicolas Iooss
42658e729f checkpolicy: add a missing free(id) in define_roleattribute()
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2017-01-23 11:26:40 -05:00
Nicolas Iooss
0a0d055283 checkpolicy: fix memory leaks in define_filename_trans()
When parsing type_transition statements with names, the memory allocated
by the type set bitmaps of variable stypes and ttypes was never freed.

Call type_set_destroy() to free this memory and, while at it, make the
function exits without leaking memory when exiting with an error.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2017-01-23 11:26:34 -05:00
Nicolas Iooss
aa1a8a3c84 checkpolicy: always free id in define_type()
In function define_type(), some error conditions between "id =
queue_remove(id_queue)" and "get_local_type(id, attr->s.value, 1)"
returned without freeing id. Fix theses memory leaks.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2017-01-23 11:26:30 -05:00
Nick Kralevich
908898846a policy_define.c: don't free memory returned from queue_head()
Unlike queue_remove(), queue_head() does not modify the queue, but
rather, returns a pointer to an element within the queue. Freeing the
memory associated with a value returned from that function corrupts
subsequent users of the queue, who may try to reference this
now-deallocated memory.

This causes the following policy generation errors on Android:

  FAILED:
  out/target/product/bullhead/obj/ETC/plat_sepolicy.cil_intermediates/plat_policy_nvr.cil
  /bin/bash -c "out/host/linux-x86/bin/checkpolicy -M -C -c 30 -o
  out/target/product/bullhead/obj/ETC/plat_sepolicy.cil_intermediates/plat_policy_nvr.cil
  out/target/product/bullhead/obj/ETC/plat_sepolicy.cil_intermediates/plat_policy.conf"
  system/sepolicy/public/app.te:241:ERROR 'only ioctl extended permissions
  are supported' at token ';' on line 6784:
  #line 241
  } };
  checkpolicy:  error(s) encountered while parsing configuration

because the value of "id" in:

  id = queue_remove(id_queue);
  if (strcmp(id,"ioctl") == 0) {
    ...
  } else {
    yyerror("only ioctl extended permissions are supported");
    ...
  }

is now garbage.

This is a partial revert of the following commit:

  c1ba8311 checkpolicy: free id where it was leaked

Signed-off-by: Nick Kralevich <nnk@google.com>
2017-01-13 14:43:38 -05:00
Nicolas Iooss
d7b0941eed checkpolicy: fix memory usage in define_bool_tunable()
In an error path of define_bool_tunable(), variable id is freed after
being used by a successful call to declare_symbol(). This may cause
trouble as this pointer may have been used as-is in the policy symtab
hash table.

Moreover bool_value is never freed after being used. Fix this memory
leak too. This leak has been detected with gcc Address Sanitizer.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2017-01-09 15:01:33 -05:00
Nicolas Iooss
c1ba831122 checkpolicy: free id where it was leaked
Several functions in policy_define.c do not free id after handling it.
Add the missing free(id) statements.

The places where free(id) was missing were found both with gcc Address
Sanitizer and manual code inspection.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2017-01-06 13:43:21 -05:00
Nicolas Iooss
6ef96094d3 checkpolicy: fix memory leaks in genfscon statements parsing
When parsing several genfscon statements for the same filesystem, the
content of local variable "fstype" is never freed. Moreover variable
"type" is never freed when define_genfs_context_helper() succeeds.

Fix these leaks by calling free() appropriately.

These leaks have been detected with gcc Address Sanitizer.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2017-01-06 13:28:23 -05:00
Nicolas Iooss
da00246827 checkpolicy: free id in define_port_context()
Variable id is almost never freed in define_port_context().

This leak has been detected with gcc Address Sanitizer.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
2017-01-06 13:22:38 -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
Stephen Smalley
49bfee8562 checkpolicy: treat -self as an error
checkpolicy wrongly handles "-self". At the least, it should handle it as
an error. At best, it should support it correctly (which would involve
libsepol support as well). At present, it looks like it will end up
negating (-) the next type/attribute in the list after self, or if
there are no entries after self, ignoring it entirely.

This originally was raised by the Android team, which wanted to support
something like the following:
neverallow domain { domain -self }:dir search;
to prohibit cross domain access to some resource but allow access within
the same domain.

This change just makes it a fatal error during compilation.
Implementing real support for -self is left as future work.

Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
2016-11-18 11:09:38 -05:00
Nicolas Iooss
112f86d489 checkpolicy: add types associated to a role in the current scope when parsing
This fixes most of the errors reported in "make -C libsepol test":

    ./libsepol-tests
         CUnit - A unit testing framework for C - Version 2.1-3
         http://cunit.sourceforge.net/
    Suite: cond
      Test: cond_expr_equal ...passed
    Suite: linker
      Test: linker_indexes ...passed
      Test: linker_types ...passed
      Test: linker_roles ...
    role o1_b_role_1 has 0 types, 1 expected
    role o1_b_role_1 has 0 types, 1 expected
    role o1_m1_role_1 has 0 types, 1 expected
    sym g_b_role_2 has 1 decls, 2 expected
    Role o1_b_role_2 had type o1_b_type_1 not in types array
    role o1_b_role_2 has 0 types, 1 expected
    Role g_b_role_4 had type g_m1_type_2 not in types array
    role g_b_role_4 has 0 types, 1 expected
    role o3_b_role_1 has 0 types, 1 expected
    role o3_b_role_1 has 0 types, 1 expected
    role o4_b_role_1 has 0 types, 1 expected
    Role o4_b_role_1 had type g_m1_type_1 not in types array

    FAILED
        1. test-common.c:216  - found == len
        2. test-common.c:216  - found == len
        3. test-common.c:216  - found == len
        4. test-common.c:43  - scope->decl_ids_len == len
        5. test-common.c:52  - found == 1
        6. test-common.c:213  - new == 1
        7. test-common.c:216  - found == len
        8. test-common.c:213  - new == 1
        9. test-common.c:216  - found == len
        10. test-common.c:216  - found == len
        11. test-common.c:216  - found == len
        12. test-common.c:216  - found == len
        13. test-common.c:213  - new == 1
      Test: linker_cond ...passed
    Suite: expander
      Test: expander_indexes ...passed
      Test: expander_attr_mapping ...passed
      Test: expander_role_mapping ...passed
      Test: expander_user_mapping ...passed
      Test: expander_alias ...passed
    Suite: deps
      Test: deps_modreq_global ...passed
      Test: deps_modreq_opt ...passed
    Suite: downgrade
      Test: downgrade ...passed

    Run Summary:    Type  Total    Ran Passed Failed Inactive
                  suites      5      5    n/a      0        0
                   tests     13     13     12      1        0
                 asserts   1269   1269   1256     13      n/a

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2016-09-06 10:49:56 -04:00
Stephen Smalley
bedef7d124 libsepol,checkpolicy,secilc: Replace #ifdef DARWIN with __APPLE__.
As per discussion in https://android-review.googlesource.com/#/c/221980,
we should be using #ifdef __APPLE__ rather than our own custom-defined
DARWIN for building on MacOS X.

Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
2016-05-03 11:54:20 -04:00
Richard Haines
aac9360581 selinux: Build policy on systems not supporting DCCP protocol
Commit 3895fbbe0c ("selinux: Add support
for portcon dccp protocol") added support for the (portcon dccp ..)
statement. This fix will allow policy to be built on platforms
(see [1]) that do not have DCCP support by defining the IANA
assigned IP Protocol Number 33 to IPPROTO_DCCP.

[1] https://android-review.googlesource.com/#/c/219568/

Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
2016-04-25 15:31:45 -04:00