Before the CIL post processing phase (where expressions are evaluated,
various ebitmaps are set, etc) there is a pre-verification where
checks are made to find self references or loops in bounds, attribute
sets, and class permissions. The class permission checking is faulty
in two ways.
First, it does not check for the use of "all" in a permission expression
for a class that has no permissions. An error will still be generated
later and secilc will exit cleanly, but without an error message that
explains the problem.
Second, it does not properly handle lists in permission expressions.
For example, "(C ((P)))" is a legitimate class permission. The
permissions expression contains one item that is a list containing
one permission. This permission expression will be properly evaluated.
Unfortunately, the class permission verification assumes that each
item in the permission expression is either an operator or a
permission datum and a segmenation fault will occur.
Refactor the class permission checking to give a proper error when
"all" is used in a permission expression for a class that has no
permissions and so that it can handle lists in permission
expressions. Also, check for the actual flavor of each item in
the permission expression and return an error if an unexpected
flavor is found.
The failure to properly handle lists in permission expressions was
found by oss-fuzz (#58085).
Tested-by: Christian Göttsche <cgzones@googlemail.com>
Signed-off-by: James Carter <jwcart2@gmail.com>
This patch implements the support for prefix/suffix filename transitions
in the CIL structures as well as in the CIL policy parser.
Syntax of the new prefix/suffix filename transition rule:
(typetransition source_type_id target_type_id class_id object_name match_type default_type_id)
where match_type is either the keyword "prefix" or "suffix".
Examples:
(typetransition ta tb CLASS01 "file01" prefix td)
(typetransition td te CLASS01 "file02" suffix tf)
Reviewed-by: Ondrej Mosnacek <omosnace@redhat.com>
Signed-off-by: Juraj Marcin <juraj@jurajmarcin.com>
Acked-by: James Carter <jwcart2@gmail.com>
This patch extends the structures for module and base policy (avrule_t)
to support prefix/suffix transitions. In addition to this, it implements
the necessary changes to functions for reading and writing the binary
policy, as well as parsing the policy conf.
Syntax of the new prefix/suffix filename transition rule:
type_transition source_type target_type : class default_type object_name match_type;
where match_type is either keyword "prefix" or "suffix"
Examples:
type_transition ta tb:CLASS01 tc "file01" prefix;
type_transition td te:CLASS01 tf "file02" suffix;
Reviewed-by: Ondrej Mosnacek <omosnace@redhat.com>
Signed-off-by: Juraj Marcin <juraj@jurajmarcin.com>
Acked-by: James Carter <jwcart2@gmail.com>
Similarly to the previous patch, filename transition rules are stored
and parsed separately from other type enforcement rules. Moving them to
avrule makes it consistent with the filename transitions in avtab and
makes future improvements easier to implement.
This patch adds an optional object name attribute to the avrule
structure and uses this new attribute to move filename transition rules
to avrule. It also updates functions for parsing type enforcement rules
to accept rules with a filename as their last argument (filename
transition rules), separate functions for parsing filename transitions
are therefore no longer needed.
Reviewed-by: Ondrej Mosnacek <omosnace@redhat.com>
Signed-off-by: Juraj Marcin <juraj@jurajmarcin.com>
Acked-by: James Carter <jwcart2@gmail.com>
Currently, filename transitions are stored separately from other type
enforcement rules. This leads to possibly sub-optimal performance and
makes further improvements cumbersome.
This patch adds a symbol table with filename transitions to the
transition structure added to avtab in the previous patch. It also
implements functions required for reading and writing filename
transitions (either binary or source formats) and updates the code for
expanding attributes. Last but not least, it updates the conflict check
in the conditional avtab to account for empty transitions in the
non-conditional avtab.
These changes are expected to cause higher memory usage, as now there
needs to be a filename transition structure for every stype. This patch
effectively undoes most of the commit 42ae834a ("libsepol,checkpolicy:
optimize storage of filename transitions"), but this will be mitigated
by providing support for matching prefix/suffix of the filename for
filename transitions in future patches which will reduce to need to have
so many of them.
Reviewed-by: Ondrej Mosnacek <omosnace@redhat.com>
Signed-off-by: Juraj Marcin <juraj@jurajmarcin.com>
Acked-by: James Carter <jwcart2@gmail.com>
To move filename transitions to be part of avtab, we need to create
space for it in the avtab_datum structure which holds the rule for
a certain combination of stype, ttype and tclass.
As only type transitions have a special variant that uses a filename, it
would be suboptimal to add a (mostly empty) pointer to some structure to
all avtab rules.
Therefore, this patch adds a new structure to the avtab_datum and moves
the otype of the transition to this structure. In the next patch, this
structure will also hold filename transitions for the combination of
stype, ttype and tclass.
Reviewed-by: Ondrej Mosnacek <omosnace@redhat.com>
Signed-off-by: Juraj Marcin <juraj@jurajmarcin.com>
Acked-by: James Carter <jwcart2@gmail.com>
Avoid using the identifier `bool` to improve support with future C
standards. C23 is about to make `bool` a predefined macro (see N2654).
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Avoid using the identifier `bool` to improve support with future C
standards. C23 is about to make `bool` a predefined macro (see N2654).
Since the type `cond_expr_t` is part of the public API it will break
client applications. A quick search of the code in Debian shows only
usages in checkpolicy and setools.
Define a new macro signaling the renaming to simplify support of client
applications for new and older versions of libsepol.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Found by codespell(1) and typos[1].
[1]: https://github.com/crate-ci/typos
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Commit bc26ddc59c ("libsepol/cil: Limit the amount of reporting for
context rule conflicts") reworked the processing of context rule
conflicts to limit the number of written conflicting statements to
increase readability of the printed error message. It forgot to set the
return value, signaling a context conflict, in the case the logging
level is higher than warning (e.g. in semodule(8), which defaults to
error).
Reported-by: Milos Malik <mmalik@redhat.com> [1]
Fixes: bc26ddc59c ("libsepol/cil: Limit the amount of reporting for context rule conflicts")
[1]: https://lore.kernel.org/selinux/87y1u1rkoo.fsf@redhat.com/
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Especially in the case of __cil_permissionx_expr_range_to_bitmap_helper()
it substitutes hundreds of thousand of calls to ebitmap_set_bit() during
semodule(8) on a policy widely using extended permissions.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Fixes:
cil/src/cil_build_ast.c:4622:4: warning[deadcode.DeadStores]: Value stored to 'rc' is never read
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
Acked-by: James Carter <jwcart2@gmail.com>
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 CIL level. It also adds basic coverage for the such 'self' rules
to the secilc test policy.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Acked-by: James Carter <jwcart2@gmail.com>
The function __cil_verify_rule() is currently not used as all call sites
are commented out. Keep the function for future references.
Acked-by: James Carter <jwcart2@gmail.com>
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
It seems to be unused since its initial addition in 76ba6eaa
("Squashed 'libsepol/cil/' changes from 08520e9..28ad56e").
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
GCC 12 produces an array-bounds warning:
In file included from ../include/sepol/policydb/context.h:23,
from ../include/sepol/policydb/policydb.h:62,
from ../cil/src/cil_binary.c:41:
In function ‘mls_level_init’,
inlined from ‘mls_level_destroy’ at ../include/sepol/policydb/mls_types.h:99:2,
inlined from ‘mls_level_destroy’ at ../include/sepol/policydb/mls_types.h:92:20,
inlined from ‘mls_range_destroy’ at ../include/sepol/policydb/mls_types.h:149:2,
inlined from ‘cil_rangetransition_to_policydb’ at ../cil/src/cil_binary.c:3231:6:
../include/sepol/policydb/mls_types.h:89:9: error: ‘memset’ offset [0, 23] is out of the bounds [0, 0] [-Werror=array-bounds]
89 | memset(level, 0, sizeof(mls_level_t));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../include/sepol/policydb/mls_types.h:89:9: error: ‘memset’ offset [0, 23] is out of the bounds [0, 0] [-Werror=array-bounds]
cc1: all warnings being treated as errors
This is a false positive, by inspecting the code and compiling with -O3
and -flto.
Closes: https://github.com/SELinuxProject/selinux/issues/339
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
The MAX_LOG_SIZE is 512. It is possible that a log message could
exceed the max size (such as for neverallowx rules). If so, then
write out "<LOG MESSAGE TRUNCATED>", so that it is obvious that
the log message has been truncated.
Reported-by: Jonathan Hettwer <j2468h@googlemail.com>
Signed-off-by: James Carter <jwcart2@gmail.com>
Since CIL allows permission expressions, it is possible for the
expression to evaluate to no permissions. If this is the case,
then don't add the constraint.
Signed-off-by: James Carter <jwcart2@gmail.com>
When there are conflicting context rules, the location of the
conflicting rules are written out. If there are many duplicates of
the same context rule, there will be many pairs of conflicts written
out. This hides the fact that all of the rules are the same and can
make it hard to see the different conflicts.
First, since these are warnings and not reported at the default log
verbosity level (which only reports errors), only search for the
locations of the conflicting rules when the verbosity level means
that the warnings will actually be reported.
Second, Report all the duplicate conflicting rules together.
Third, Report the first four conflicts of the same rule if when
the verbosity level is at CIL_WARN ("-v") and report all of them
when the verbosity level is at CIL_INFO or higher ("-v -v").
Fixes problem found by oss-fuzz (#39735)
Signed-off-by: James Carter <jwcart2@gmail.com>
When there is a neverallow violation, a search is made for all of
the rules that violate the neverallow. The violating rules as well
as their parents are written out to make it easier to find these
rules.
If there is a lot of rules that violate a neverallow, then this
amount of reporting is too much. Instead, only print out the first
four rules (with their parents) that match the violated neverallow
rule along with the total number of rules that violate the
neverallow at the default log level. Report all the violations when
at a higher verbosity level.
Signed-off-by: James Carter <jwcart2@gmail.com>
Commit 4b2e2a248e48b2902ab1ef3cab86322a3c6ef055 (libsepol/cil: Limit
the amount of reporting for bounds failures) limited the number of
bounds failures that were reported to the first two matching rules
for the first two bad rules.
Instead, report the first two matching rules for the first four bad
rules at the default log level and report all matching rules for all
bad rules for higher verbosity levels.
Signed-off-by: James Carter <jwcart2@gmail.com>
Map classes use the same struct as kernel classes, but only the kernel
class uses the pointer to a common class. When resolving a classcommon,
make sure that the class that is found is a kernel class and not a
map class. If not, then return an error.
Found by oss-fuzz (#43209)
Signed-off-by: James Carter <jwcart2@gmail.com>
Since abstract blocks will not appear in the final policy, do not
resolve names to a declaration inside one.
When resolving blockabstract rules, they must be collected in a list
and processed at the end of the pass because if a parent block is
marked as abstract, then a blockabstract rule for a sub-block will
fail to resolve.
Found by oss-fuzz (#42981)
Signed-off-by: James Carter <jwcart2@gmail.com>
If a block is marked as abstract, then it will be skipped during
every pass after blockabstracts are resolved (only tunables,
in-befores, and blockinherits are before blockabstracts), so mark
all of its sub-blocks as abstract to reflect their actual status.
Signed-off-by: James Carter <jwcart2@gmail.com>
Do not copy any blockabstract statements when copying a block to
resolve a blockinherit statement. Inheriting a block from what was
just inherited does not work, so there is no reason to create an
abstract block.
Signed-off-by: James Carter <jwcart2@gmail.com>
Do not continue with a negative return value once a string append
operation fails to avoid increasing the buffer length variable
`str_len`, potentially leading to an out-of-bounds write.
Found by GitHub CodeQL.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Accept IPv4 addresses embedded in IPv6, like `::ffff:127.0.0.1`.
This allows using those in nodecon statements leading to fine grained
access control:
type=AVC msg=audit(11/29/21 20:27:44.437:419) : avc: granted { node_bind } for pid=27500 comm=intercept saddr=::ffff:127.0.0.1 src=46293 scontext=xuser_u:xuser_r:xuser_t:s0 tcontext=system_u:object_r:lo_node_t:s0 tclass=tcp_socket
This does effect policies in the traditional language due to CIL usage
in semodule(8).
Also print on conversion failures the address in question.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
The optional specification of a file type for a genfscon rule to
make it apply only to a specific security class is allowed by
checkpolicy and checkmodule and should be allowed for CIL policies
as well.
Allow an optional file type to be specified for a genfscon rule.
The new syntax:
(genfscon FSNAME PATH [FILE_TYPE] CONTEXT)
FSNAME - The name of the supported filesystem
PATH - If FSNAME is proc then this is the partial path,
othewise this must be "/".
FILE_TYPE - A single keyword representing the file type.
file type security class
any Same as not specifying a file type
file file
dir dir
char chr_file
block blk_file
socket sock_file
pipe fifo_file
symlink lnk_file
CONTEXT - Either a previously declared security context identifier
or an anonymous security context.
Signed-off-by: James Carter <jwcart2@gmail.com>
Prepare for the addition of an optional file type in genfscon rules
by refactoring filecon file type handling.
Make the "any" file type be the first value in enum cil_filecon_types
because it will be the most common file type.
Signed-off-by: James Carter <jwcart2@gmail.com>
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.
This bug was found by the secilc-fuzzer.
Signed-off-by: James Carter <jwcart2@gmail.com>
Since only tunableifs need to be resolved in a macro before the macro
is copied for each call, macros were being skipped after resolving
tunableifs. Statments not allowed to be in macros would be found during
the pass that resolved tunableifs. Unfortunately, in-statments are
resolved after tunableifs and they can be used to add statements to
macros that are not allowed.
Instead, do not skip macros until after the pass that resolves in-
statements that are to be resolved after block inheritance. This
allows blocks, blockinherits, blockabstracts, and macros that were
added by an in-statement to be found and an error reported.
This bug was found by the secilc-fuzzer.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Type bounds are checked when creating the CIL binary using libsepol
functions on the binary policy db. The bad rule is reported and, to
provide better error reporting, a search is made for matching rules
in the CIL policy. These matching rules as well as their parents are
written out with their locations to make it easier to find the rules
that violate the type bounds.
It is possible to craft CIL policies where there are many rules
that violate a bounds check each with many matching rules as well.
This can make the error messages very difficult to deal with. For
example, if there are 100 rules in the binary policy db that violate
a type bounds and each of these rules has 100 matches, then 10,000
matching rules along with their parents will be written out as part
of the error message.
Limit the error reporting to two rules for each type bounds violation
along with two matches for each of those rules.
This problem was found with the secilc-fuzzer.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Add an intermediate cast to uintptr_t to silence the clang specific
warning about casting a void pointer to an enum.
../cil/src/cil_verify.c:1749:28: error: cast to smaller integer type 'enum cil_flavor' from 'void *' [-Werror,-Wvoid-pointer-to-enum-cast]
enum cil_flavor op = (enum cil_flavor)i->data;
^~~~~~~~~~~~~~~~~~~~~~~~
Similar to 32f8ed3d6b.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
When checking for circular class permission declarations and a class
mapping is encountered, the class permissions for each map permission
must be checked. An assumption was made that there were no operators
in the class permissions. An operator in the class permissions would
cause a segfault.
Example causing segault:
(classmap cm1 (mp1))
(classmapping cm1 mp1 (CLASS (PERM)))
(classpermission cp1)
(classpermissionset cp1 (cm1 (all)))
For map class permissions, check each item in the permission list to
see if it is an operator. If it is not, then verify the class
permissions associated with the map permission. If it is an operator
and the operator is "all", then create a list of all permissions for
that map class and verify the class permissions associated with each
map permission. If it is a different operator, then it can be skipped.
This bug was found by the secilc-fuzzer.
Signed-off-by: James Carter <jwcart2@gmail.com>
When compiling CIL policy using secilc's "-m" option (which allows
duplicate declarations for types and type attributes), a segfault
will occur if the type or type attribute being copied has already
been declared. This is because a search of the symbol table is made
during the copy and the original datum will be used if one is found.
The original datum will be considered a duplicate when an attempt is
made to add it to the symbol table. The original datum, which is still
in use, will then be destroyed and a segfault will follow soon after
that.
Instead, always create a new datum. When it is added the new datum
will be destroyed if it is a duplicate and duplicate declarations
are allowed.
Signed-off-by: James Carter <jwcart2@gmail.com>
Duplicate declarations are allowed for type, typeattribute, and
optional statements. When an allowed duplicate declaration is found,
the duplicate datum is free'd in cil_add_decl_to_symtab() and SEPOL_OK
is returned. This works for all the rules where a duplicate declaration
is allowed, but it confuses scanning tools.
When cil_add_decl_to_symtab() finds an allowed duplicate declaration,
return SEPOL_EEXIST and free the duplicate datum in the original
calling function.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Petr Lautrbach <plautrba@redhat.com>
libsepol/cil/src/cil_binary.c:4823: alloc_arg: "bounds_check_type" allocates memory that is stored into "bad".
libsepol/cil/src/cil_binary.c:4840: var_assign: Assigning: "cur" = "bad".
libsepol/cil/src/cil_binary.c:4844: noescape: Resource "cur" is not freed or pointed-to in "cil_avrule_from_sepol".
libsepol/cil/src/cil_binary.c:4847: leaked_storage: Variable "cur" going out of scope leaks the storage it points to.
libsepol/cil/src/cil_binary.c:4847: leaked_storage: Variable "bad" going out of scope leaks the storage it points to.
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
Acked-by: James Carter <jwcart2@gmail.com>
A line mark functions like an open parenthesis, so the number of
active line marks should be limited like the number of open
parenthesis.
This issue was found by the secilc-fuzzer.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
The function __cil_verify_syntax() is used to check the syntax of
CIL rules (and a few other common things like contexts and class
permissions). It does not correctly check the syntax combination
"CIL_SYN_STRING | CIL_SYN_N_LISTS, CIL_SYN_N_LISTS | CIL_SYN_END".
This should mean either a string followed by any number of lists
or any number of lists followed by the end of the rule. Instead,
while allowing the correct syntax, it allows any number of lists
followed by a string followed by any number of more lists followed
by the end of the rule and, also, any number of lists followed by a
string followed by the end of the rule.
Refactor the function to make it clearer to follow and so that once
checking begins for CIL_SYN_N_LISTS or CIL_SYN_N_STRINGS, then only
strings or lists are allowed until the end of the rule is found. In
addition, always check for CIL_SYN_END at the end.
Signed-off-by: James Carter <jwcart2@gmail.com>
Since the value passed into __cil_verify_syntax() as the len
parameter is always calculated from sizeof(syntax)/sizeof(*syntax),
use size_t for the calculated value in the calling function and for
the len parameter. In __cil_verify_syntax(), the variable i is only
compared to len, so make that size_t as well.
Signed-off-by: James Carter <jwcart2@gmail.com>
