Implement a new, more space-efficient form of storing filename
transitions in the binary policy. The internal structures have already
been converted to this new representation; this patch just implements
reading/writing an equivalent representation from/to the binary policy.
This new format reduces the size of Fedora policy from 7.6 MB to only
3.3 MB (with policy optimization enabled in both cases). With the
unconfined module disabled, the size is reduced from 3.3 MB to 2.4 MB.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
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>
The comparison function, portcon_data_cmp(), only made use of the
protocol to put tcp before udp, dccp, and sctp. Rules that have
the same port range, but with different protocols would be considered
equal unless one of the protocols was tcp. When generating a CIL or
conf source policy from a binary or using the "-S" option in
checkpolicy the non-tcp portcon rules with the same port range would
not be consistently sorted.
Changed portcon_data_cmp() to sort portcon rules like the CIL function
cil_post_portcon_compare().
Reported-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Improves writing of CIL category rules when converting MLS kernel
policy to CIL. No changes to functionality, but eliminate useless
checks for category aliases when using the p_cat_val_to_name array,
find the actual number of aliases before allocating memory, and
skip the category alias rules if there are no aliases.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Improves writing of CIL sensitivity rules when converting MLS kernel
policy to CIL. No changes to functionality, but eliminate useless
checks for sensitivity aliases when using the p_sens_val_to_name
array, find the actual number of aliases before allocating memory,
and skip the sensitivity alias rules if there are no aliases.
Signed-off-by: James Carter <jwcart2@gmail.com>
When converting a non-MLS kernel binary policy to CIL, write the CIL
default MLS rules (since CIL requires at least one sensitivity,
and sensitivityorder statements) on separate lines.
This improves the readability of the resulting CIL policy.
Signed-off-by: James Carter <jwcart2@gmail.com>
Type alias rules are not written out when converting a binary kernel
policy to a policy.conf. The problem is that type aliases are not in
the type_val_to_struct array and that is what is being used to find
the aliases.
Since type aliases are only in the types hashtable, walk that to
find the type aliases.
Fixed the syntax of the typalias rule which requires "alias" to come
between the type and the aliases (ex/ typealias TYPE alias ALIAS;).
Fixes: 0a08fd1e69 ("libsepol: Add ability to convert binary
policy to policy.conf file")
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Type alias rules are not written out when converting a binary kernel
policy to CIL. The problem is that type aliases are not in the
type_val_to_struct array and that is what is being used to find the
aliases.
Since type aliases are only in the types hashtable, walk that to
find the type aliases.
Fixes: 70a480bfcd ("libsepol: Add ability to convert binary
policy to CIL")
Signed-off-by: James Carter <jwcart2@gmail.com>
CIL allows a type to be redeclared when using the multiple declarations
option ("-m" or "--muliple-decls"), but make it an error for an identifier
to be declared as both a type and an attribute.
Change the error message so that it always gives the location and flavor
of both declarations. The flavors will be the same in all other cases,
but in this case they explain why there is an error even if multiple
declartions are allowed.
Fixes: Commit fafe4c212b ("libsepol: cil: Add ability to redeclare types[attributes]")
Reported-by: Topi Miettinen <toiwoton@gmail.com>
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Initialize the multiple_decls field when intializing the structure
cil_db.
Fixes: fafe4c212b ("libsepol: cil: Add ability to redeclare types[attributes]")
Reported-by: Topi Miettinen <toiwoton@gmail.com
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Signed-off-by: James Carter <jwcart2@gmail.com>
As per the issue below, libsepol segfaults on loading old kernel policies
that contain duplicate filename transition rules. The segfault is due to
the fact that the val_to_name arrays have not yet been populated at this
point in the policydb_read() processing. Since this warning apparently
never worked since it was first introduced, drop it and just silently
discard the duplicate like the kernel does. I was not able to produce a
policy with such duplicates using the current policy toolchain, either
via CIL or via binary modules with manual semodule_link/expand.
Fixes: https://github.com/SELinuxProject/selinux/issues/239
Fixes: 8fdb225521 ("libsepol,checkpolicy: convert rangetrans and filenametrans to hashtabs")
Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Acked-by: Ondrej Mosnacek <omosnace@redhat.com>
This reverts commit 692716fc5f.
Other parts of the SELinux userspace depend on certain attributes,
such as node_type, exisiting and this change breaks those parts.
Before this patch can be reapplied, we need to identify the attributes
that must never be expanded and create a CIL module with the needed
expandtypeattribute statements (or something similar).
Signed-off-by: James Carter <jwcarter@gmail.com>
Fix issues like:
<inline asm>:1:1: error: unknown directive
.symver cil_build_policydb_pdb, cil_build_policydb@LIBSEPOL_1.0
Which was caused by the DISABLE_SYMVER define not being defined
for static, Mac or Android builds.
Acked-by: Joshua Brindle <joshua.brindle@crunchydata.com>
Signed-off-by: William Roberts <william.c.roberts@intel.com>
Currently a constraint `t1 == t2` gets converted to the invalid cil syntax `(mlsconstrain (class_name (perm_name)) (eq t1 ))` and fails to be loaded into the kernel.
Fixes: 893851c0a1 ("policycoreutils: add a HLL compiler to convert policy packages (.pp) to CIL")
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
The iteration over the set ebitmap bits is not implemented very
efficiently in libsepol. It is slowing down the policy optimization
quite significantly, so convert the type_map from an array of ebitmaps
to an array of simple ordered vectors, which can be traveresed more
easily. The worse space efficiency of the vectors is less important than
the speed in this case.
After this change the duration of semodule -BN decreased from 6.4s to
5.5s on Fedora Rawhide x86_64 (and from 6.1s to 5.6s with the unconfined
module disabled).
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Only attributes can be a superset of another attribute, so we can skip
non-attributes right away.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
I copy-pasted it from a different part of the code, which had to deal
with policydb that isn't final yet. Since we only deal with the final
kernel policy here, we can skip the check for the type datum being NULL.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
With the old hidden_def and hidden_proto DSO infrastructure removed,
correctness of the map file becomes paramount, as it is what filters out
public API. Because of this, the wild cards should not be used, as it
lets some functions through that should not be made public API. Thus
remove the wild cards, and sort the list.
Additionally, verify that nothing changed in external symbols as well:
This was checked by generating an old export map (from master):
nm --defined-only -g ./src/libsepol.so | cut -d' ' -f 3-3 | grep -v '^_' > old.map
Then creating a new one for this library after this patch is applied:
nm --defined-only -g ./src/libsepol.so | cut -d' ' -f 3-3 | grep -v '^_' > new.map
And diffing them:
diff old.map new.map
Fixes: #165Fixes: #204
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
Signed-off-by: William Roberts <william.c.roberts@intel.com>
Add -fno-semantic-interposition to CFLAGS. This will restore
the DSO infrastructures protections to insure internal callers
of exported symbols call into libselinux and not something loading first
in the library list.
Clang has this enabled by default.
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
Signed-off-by: William Roberts <william.c.roberts@intel.com>
libsepol already has a linker script controlling it's exports, so this
patch has a net 0 affect, with the exception that internal callers of
external routines, which there could be 0 of, could potentially call a
non-libsepol routine depending on library load order.
NOTE A FEW SYMBOLS ARE EXPORTED THAT NORMALLY WOULDN'T BE
- sepol_context_to_sid
- sepol_ibendport_sid
- sepol_ibpkey_sid
- sepol_msg_default_handler
- sepol_node_sid
- sepol_port_sid
A subsequent map update will follow.
This list was generated by generating an old export map (from master):
nm --defined-only -g ./src/libsepol.so | cut -d' ' -f 3-3 | grep -v '^_' > old.map
Then creating a new one for this library after this patch is applied:
nm --defined-only -g ./src/libsepol.so | cut -d' ' -f 3-3 | grep -v '^_' > new.map
And diffing them:
diff old.map new.map
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
Signed-off-by: William Roberts <william.c.roberts@intel.com>
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>
The value attrs_expand_size == 1 removes all empty attributes, but it
also makes sense to expand all attributes that have only one type. This
removes some redundant rules (there is sometimes the same rule for the
type and the attribute) and reduces the number of attributes that the
kernel has to go through when looking up rules.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Acked-by: James Carter <jwcart2@gmail.com>
A parameter of a macro was only considered to be a duplicate if it
matched both the name and flavor of another parameter. While it is
true that CIL is able to differentiate between those two parameters,
there is no reason to use the same name for two macro parameters and
it is better to return an error for what is probably an error.
Remove the check of the flavors when checking for duplicate parameters.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Ondrej Mosnacek <omosnace@redhat.com>
Type transition file names are stored in a symbol table. Before the
name is added, the symbol table is searched to see if the name had
already been inserted. If it has, then the already existing datum is
returned. If it has not, then the name is added if either the
typetransition rule does not occur in a macro or the name is not one
of the macro parameters.
Checking for a previous insertion before checking if the name is a
macro parameter can cause a macro parameter to be treated as the
actual name if a previous type transition file name is the same as
the parameter.
Now check the name to see if it a macro paramter before checking for
its existence in the symbol table.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Ondrej Mosnacek <omosnace@redhat.com>
This reverts commit 542e878690.
After 6968ea9775 ("libsepol: make ebitmap_cardinality() of linear
complexity"), the caching only saves ~0.06 % of total semodule -BN
running time (on x86_64 without using the POPCNT instruction), so it's
no longer worth the added complexity.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
Create the macro ebitmap_is_empty() to check if an ebitmap is empty.
Use ebitmap_is_empty(), instead of ebitmap_cardinality() or
ebitmap_length(), to check whether or not an ebitmap is empty.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Ondrej Mosnacek <omosnace@redhat.com>
As ebitmap_get_bit() complexity is linear in the size of the bitmap, the
complexity of ebitmap_cardinality() is quadratic. This can be optimized
by browsing the nodes of the bitmap directly in ebitmap_cardinality().
While at it, use built-in function __builtin_popcountll() to count the
ones in the 64-bit value n->map for each bitmap node. This seems better
suited than "count++". This seems to work on gcc and clang on x86,
x86_64, ARM and ARM64 but if it causes compatibility issues with some
compilers or architectures (or with older versions of gcc or clang),
the use of __builtin_popcountll() can be replaced by a C implementation
of a popcount algorithm.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Detect when the hashtab's load factor gets too high and try to grow it
and rehash it in such case. If the reallocation fails, just keep the
hashtab at its current size, since this is not a fatal error (it will
just be slower).
This speeds up semodule -BN on Fedora from ~8.9s to ~7.2s (1.7 seconds
saved).
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
hashtab_replace() and hashtab_map_remove_on_error() aren't used
anywhere, no need to keep them around...
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
According to profiling of semodule -BN, ebitmap_cardinality() is called
quite often and contributes a lot to the total runtime. Cache its result
in the ebitmap struct to reduce this overhead. The cached value is
invalidated on most modifying operations, but ebitmap_cardinality() is
usually called once the ebitmap doesn't change any more.
After this patch, the time to do 'semodule -BN' on Fedora Rawhide has
decreased from ~10.9s to ~8.9s (2s saved).
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
[sds@tycho.nsa.gov: correct times per follow-up on list]
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
The filename_- and range_trans_table ancillary hash tables in
cil_binary.c just duplicate the final policydb content and can be simply
removed.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
The classperms associated with each map class permission and with each
classpermissionset are verified in __cil_verify_classperms() which had
multiple problems with how it did the verification.
1) Verification was short-circuited when the first normal class is found.
The second classpermissionset statement below would not have been
verified.
(classpermission cp1)
(classpermissionset cp1 (CLASS (PERM)))
(classpermissionset cp1 cp2)
2) The classperms of a map class permission and classpermissionset were
not checked for being NULL before the function recursively called itself.
This would result in a segfault if the missing map or set was referred to
before the classmap or classpermission occured. This error was reported by
Dominick Grift (dominick.grift@defensec.nl).
These rules would cause a segfault.
(classmap cm1 (mp1))
(classmapping cm1 mp1 (cm2 (mp2)))
(classmap cm2 (mp2))
But an error would be produced for these rules.
(classmap cm1 (mp1))
(classmap cm2 (mp2))
(classmapping cm2 mp2 (cm1 (mp1)))
3) The loop detection logic was incomplete and could only detect a loop
with a certain statement ordering.
These rules would cause a stack overflow.
(classmap cm1 (mp1))
(classmapping cm1 mp1 (cm2 (mp2)))
(classmap cm2 (mp2))
(classmapping cm2 mp2 (cm3 (mp3)))
(classmap cm3 (mp3))
(classmapping cm3 mp3 (cm2 (mp2)))
Rewrote __cil_verify_classperms() to fix these errors.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
Add support for new SELinux policy capability genfs_seclabel_symlinks.
With this capability enabled symlinks on kernel filesystems will receive
contexts based on genfscon statements, like directories and files,
and not be restricted to the respective filesystem root sid.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Remove restrictions in libsepol and checkpolicy that required all
declared initial SIDs to be assigned a context. With this patch,
it is possible to build and load a policy that drops the sid <sidname>
<context> declarations for the unused initial SIDs. It is still
required to retain the sid <sidname> declarations (in the flask
definitions) in order to preserve the initial SID ordering/values.
The unused initial SIDs can be renamed, e.g. to add an unused_
prefix or similar, if desired, since the names used in the policy
are not stored in the kernel binary policy.
In CIL policies, the (sid ...) and (sidorder (...)) statements
must be left intact for compatibility but the (sidcontext ...)
statements for the unused initial SIDs can be omitted after this change.
With current kernels, if one removes an unused initial SID context
from policy, builds policy with this change applied and loads the
policy into the kernel, cat /sys/fs/selinux/initial_contexts/<sidname>
will show the unlabeled context. With the kernel patch to remove unused
initial SIDs, the /sys/fs/selinux/initial_contexts/<sidname>
file will not be created for unused initial SIDs in the first place.
NB If an unused initial SID was assigned a context different from
the unlabeled context in existing policy, then it is not safe to
remove that initial SID context from policy and reload policy on
the running kernel that was booted with the original policy. This
is because that kernel may have assigned that SID to various kernel
objects already and those objects will then be treated as having
the unlabeled context after the removal. In refpolicy, examples
of such initial SIDs are the "fs" SID and the "sysctl" SID. Even
though these initial SIDs are not directly used (in code) by the current
kernel, their contexts are being applied to filesystems and sysctl files by
policy and therefore the SIDs are being assigned to objects.
NB The "sysctl" SID was in use by the kernel up until
commit 8e6c96935fcc1ed3dbebc96fddfef3f2f2395afc ("security/selinux:
fix /proc/sys/ labeling) circa v2.6.39. Removing its context from
policy will cause sysctl(2) or /proc/sys accesses to end up
performing permission checks against the unlabeled context and
likely encounter denials for kernels < 2.6.39.
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
Commit 4459d635b8 ("libsepol: Remove cil_mem_error_handler() function
pointer") replaced cil_mem_error_handler usage with inline contents of
the default handler. However, it left over the header declaration and
two callers. Convert these as well and remove the header declaration.
This also fixes a build failure with -fno-common.
Fixes: 4459d635b8 ("libsepol: Remove cil_mem_error_handler() function pointer")
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
GCC 10 comes with -fno-common enabled by default - fix the CIL_KEY_*
global variables to be defined only once in cil.c and declared in the
header file correctly with the 'extern' keyword, so that other units
including the file don't generate duplicate definitions.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
When copying an avrule with extended permissions (permx) in
cil_copy_avrule(), the check for a named permx checks the new permx
instead of the old one, so the check will always fail. This leads to a
segfault when trying to copy a named permx because there will be an
attempt to copy the nonexistent permx struct instead of the name of
the named permx.
Check whether the original is a named permx instead of the new one.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
Since failing to resolve a statement in an optional block is normal,
only display messages about the statement failing to resolve and the
optional block being disabled at the highest verbosity level.
These messages are now only at log level CIL_INFO instead of CIL_WARN.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
There's a typo in commit b8213acff8 ("libsepol: add a function to optimize
kernel policy") which added new function sepol_policydb_optimize(), but there's
sepol_optimize_policy in libsepol.map.
LIBSEPOL_3.0 is used to follow the next release version libsepol-3.0
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
Acked-by: Ondrej Mosnacek <omosnace@redhat.com>
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
This improves commit b8213acf (libsepol: add a function to optimize
kernel policy) by Ondrej Mosnacek <omosnace@redhat.com> by always
removing redundant conditional rules which have an identical rule
in the unconditional policy.
Add a flag called not_cond to is_avrule_redundant(). When checking
unconditional rules against the avtab (which stores the unconditional
rules) we need to skip the actual rule that we are checking (otherwise
a rule would be determined to be redundant with itself and bad things
would happen), but when checking a conditional rule against the avtab
we do not want to skip an identical rule (which is what currently
happens), we want to remove the redundant permissions in the conditional
rule.
A couple of examples to illustrate when redundant condtional rules
are not removed.
Example 1
allow t1 t2:class1 perm1;
if (bool1) {
allow t1 t2:class1 perm1;
}
The conditional rule is clearly redundant, but without this change it
will not be removed, because of the check for an identical rule.
Example 2
typeattribute t1 a1;
allow t1 t2:class1 perm1;
allow a1 t2:class1 perm1;
if (bool1) {
allow t1 t2:class1 perm1;
}
The conditional rule is again clearly redundant, but now the order of
processing during the optimization will determine whether or not the
rule is removed. Because a1 contains only t1, a1 and t1 are considered
to be supersets of each other. If the rule with the attribute is
processed first, then it will be determined to be redundant and
removed, so the conditional rule will not be removed. But if the rule
with the type is processed first, then it will be removed and the
conditional rule will be determined to be redundant with the rule with
the attribute and removed as well.
The change reduces the size of policy a bit more than the original
optimization. Looking at the change in number of allow rules, there is
about a 10% improvement over the old optimization.
orig old new
Refpolicy 113284 82467 78053
Fedora 106410 64015 60008
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
Several static analyzers (clang's one, Facebook Infer, etc.) warn about
NULL pointer dereferences after a call to CU_ASSERT_PTR_NOT_NULL_FATAL()
in the test code written using CUnit framework. This is because this
CUnit macro is too complex for them to understand that the pointer
cannot be NULL: it is translated to a call to CU_assertImplementation()
with an argument as TRUE in order to mean that the call is fatal if the
asserted condition failed (cf.
http://cunit.sourceforge.net/doxdocs/group__Framework.html).
A possible solution could consist in replacing the
CU_ASSERT_..._FATAL() calls by assert() ones, as most static analyzers
know about assert(). Nevertheless this seems to go against CUnit's API.
An alternative solution consists in overriding CU_ASSERT_..._FATAL()
macros in order to expand to assert() after a call to the matching
CU_ASSERT_...() non-fatal macro. This appears to work fine and to remove
many false-positive warnings from various static analyzers.
As this substitution should only occur when using static analyzer, put
it under #ifdef __CHECKER__, which is the macro used by sparse when
analyzing the Linux kernel.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
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>
In test_attr_types, the pointer decl is allowed to be NULL in the
beginning, but is dereferenced to produce a helpful message right before
a CU_ASSERT_FATAL. Make this derefence not happen if the pointer is
NULL.
This issue has been found using clang's static analyzer.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
semodule-utils/semodule_link/semodule_link.c contains:
static sepol_module_package_t *load_module(char *filename)
{
/* ... */
if (sepol_module_package_create(&p)) {
/* ... */
goto bad;
/* ... */
bad:
sepol_module_package_free(p);
When sepol_module_package_create() fails while having successfully
allocated p, it currently frees p without setting it back to NULL. This
causes a use-after-free in load_module().
Prevent this use-after-free by setting sepol_module_package_create's
argument back to NULL when an error happens.
This issue has been found using Infer static analyzer.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Doing this looks wrong:
len = scope->decl_ids_len;
if (scope == NULL) {
/* ... */
Move the dereferencing of scope after the NULL check.
This issue has been found using Infer static analyzer.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
When strs_stack_init(&stack) fails to allocate memory and stack is still
NULL, it should not be dereferenced with strs_stack_pop(stack).
This issue has been found using Infer static analyzer.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
As reported by Nicolas Iooss (nicolas.iooss@m4x.org), static analyzers
have problems understanding that the default memory error handler does
not return since it is called through the cil_mem_error_handler()
function pointer. This results in a number of false positive warnings
about null pointer dereferencing.
Since the ability to set the cil_mem_error_handler() is only through
the function cil_set_mem_error_handler() which is never used and whose
definition is not in any header file, remove that function, remove the
use of cil_mem_error_handler() and directly in-line the contents of
the default handler, cil_default_mem_error_handler().
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
This patch is loosely based on a patch by Yuli Khodorkovskiy
<yuli@crunchydata.com> from June 13th, 2019.
Since any permission used in the policy should be defined, CIL
should return an error if it cannot resolve a permission used
in a policy. This was the original behavior of CIL.
The behavior was changed over three commits from July to November
2016 (See commits 46e157b47, da51020d6, and 2eefb20d8). The change
was motivated by Fedora trying to remove permissions from its
policy that were never upstreamed (ex/ process ptrace_child and
capability2 compromise_kernel). Local or third party modules
compiled with those permissions would break policy updates.
After three years it seems unlikely that we need to worry about
those local and third party modules and it is time for CIL to
give an error like it should.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
Policy developers can set a default_range default to glblub and
computed contexts will be the intersection of the ranges of the
source and target contexts. This can be used by MLS userspace
object managers to find the range of clearances that two contexts
have in common. An example usage is computing a transition between
the network context and the context of a user logging into an MLS
application.
For example, one can add a default with
this cil:
(defaultrange db_table glblub)
or in te (base module only):
default_range db_table glblub;
and then test using the compute_create utility:
$ ./compute_create system_u:system_r:kernel_t:s0:c1,c2,c5-s0:c1.c20 system_u:system_r:kernel_t:s0:c0.c20-s0:c0.c36 db_table
system_u:object_r:kernel_t:s0:c1,c2,c5-s0:c1.c20
Some example range transitions are:
User Permitted Range | Network Device Label | Computed Label
---------------------|----------------------|----------------
s0-s1:c0.c12 | s0 | s0
s0-s1:c0.c12 | s0-s1:c0.c1023 | s0-s1:c0.c12
s0-s4:c0.c512 | s1-s1:c0.c1023 | s1-s1:c0.c512
s0-s15:c0,c2 | s4-s6:c0.c128 | s4-s6:c0,c2
s0-s4 | s2-s6 | s2-s4
s0-s4 | s5-s8 | INVALID
s5-s8 | s0-s4 | INVALID
Signed-off-by: Joshua Brindle <joshua.brindle@crunchydata.com>
Remove legacy local boolean and user code, and to preserve API/ABI
compatibility the following functions int values should be set to '0'
as they are no longer used:
selinux_mkload_policy(int preservebools)
security_set_boolean_list(.... int permanent)
and the following are now no-op and return '-1':
security_load_booleans()
sepol_genusers()
sepol_set_delusers()
sepol_genbools()
sepol_genbools_array()
and these still return their paths for compatibility, however they are
marked as deprecated:
selinux_booleans_path()
selinux_users_path()
These have been removed as they are local functions only:
sepol_genusers_policydb()
sepol_genbools_policydb()
Also "SETLOCALDEFS" removed from SELinux config file and code.
Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
In module_to_cil.c, sepol_ppfile_to_module_package() calls functions
from module.c without including the internal header. This makes building
libsepol with "gcc -flto -fuse-ld=gold" fails when linking libsepol.so:
/tmp/ccHYAKVZ.ltrans21.ltrans.o:<artificial>:function
sepol_ppfile_to_module_package: error: undefined reference to
'sepol_module_package_free'
/tmp/ccHYAKVZ.ltrans21.ltrans.o:<artificial>:function
sepol_ppfile_to_module_package: error: undefined reference to
'sepol_module_package_create'
/tmp/ccHYAKVZ.ltrans21.ltrans.o:<artificial>:function
sepol_ppfile_to_module_package: error: undefined reference to
'sepol_module_package_create'
collect2: error: ld returned 1 exit status
Fixes: https://github.com/SELinuxProject/selinux/issues/165
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Add sepol_policydb_optimize(), which checks a kernel policy for
redundant rules (i.e. those that are covered by an existing more general
rule) and removes them.
Results on Fedora 29 policy:
WITHOUT OPTIMIZATION:
# time semodule -B
real 0m21,280s
user 0m18,636s
sys 0m2,525s
$ wc -c /sys/fs/selinux/policy
8692158 /sys/fs/selinux/policy
$ seinfo (edited)
Allow: 113159
Dontaudit: 10297
Total: 123156
WITH OPTIMIZATION ENABLED:
# time semodule -B
real 0m22,825s
user 0m20,178s
sys 0m2,520s
$ wc -c /sys/fs/selinux/policy
8096158 /sys/fs/selinux/policy
$ seinfo (edited)
Allow: 66334
Dontaudit: 7480
Total: 73814
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Installing a cil module with invalid mlsconstrain syntax currently
results in a segfault. In the following module, the right-hand side of
the second operand of the OR is a list (mlstrustedobject):
$ cat test.cil
(class test (foo) )
(classorder (unordered test))
(mlsconstrain (test (foo))
(or
(dom h1 h2)
(eq t2 (mlstrustedobject))
)
)
$ sudo semodule -i test.cil
zsh: segmentation fault sudo semodule -i test.cil
This syntax is invalid and should error accordingly, rather than
segfaulting. This patch provides this syntax error for the same module:
$ sudo semodule -i test.cil
t1, t2, r1, r2, u1, u2 cannot be used on the left side with a list on the right side
Bad expression tree for constraint
Bad constrain declaration at /var/lib/selinux/mls/tmp/modules/400/test/cil:4
semodule: Failed!
Signed-off-by: Mike Palmiotto <mike.palmiotto@crunchydata.com>
When validatetrans rule is in CIL policy it errors with:
u3, r3, and t3 can only be used with mlsvalidatetrans rules
Will now resolve these examples:
(validatetrans binder (and (and (eq t1 t1_t) (eq t2 t2_t)) (eq t3 t3_t)))
(mlsvalidatetrans file (and (and (eq t1 t1_t) (eq t2 t2_t))
(and (eq t3 t3_t) (domby h1 h2))))
Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
Most of the users of ebitmap_for_each_bit() macro only care for the set
bits, so introduce a new ebitmap_for_each_positive_bit() macro that
skips the unset bits. Replace uses of ebitmap_for_each_bit() with the
new macro where appropriate.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
clang's static analyze reports a use-after-free in
__cil_expr_to_string(), when __cil_expr_to_string_helper() does not
modify its third parameter (variable s1 here) in this loop:
for (curr = curr->next; curr; curr = curr->next) {
__cil_expr_to_string_helper(curr, flavor, &s1);
cil_asprintf(&c2, "%s %s", c1, s1);
free(c1);
free(s1);
c1 = c2;
}
Silence this warning by making sure s1 is always NULL at the beginning
of every iteration of the loop.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
clang's static analyzer reports a warning when low_bit is used without
having been initialized in statements such as:
low_value = low_bit << 8;
The warning is: "Result of operation is garbage or undefined".
This is caused by low_bit being only initialized when in_range is true.
This issue is not critical because low_value is only used in an
"if (in_range)" block. Silence this warning by moving low_value's
assignment inside this block.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
These were reported by Petr Lautrbach (plautrba@redhat.com) and this
patch was based on his patch with only a few changes.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
Nicolas Iooss reports:
When using checkpolicy to read a binary policy, permissive types are not
written in the output file. In order to reproduce this issue, a test
policy can be written from minimal.cil with the following commands:
$ cd secilc/test/
$ cp minimum.cil my_policy.cil
$ echo '(typepermissive TYPE)' >> my_policy.cil
$ secilc my_policy.cil
$ checkpolicy -bC -o /dev/stdout policy.31
# There is no "(typepermissive TYPE)" in checkpolicy output.
This is because TYPE_FLAGS_PERMISSIVE is added to typdatum->flags only
when loading a module, which uses the permissive flag in the type
properties. A kernel policy defines permissive types in a dedicated
bitmap, which gets loaded as p->permissive_map before the types are
loaded.
The solution is to use the permissive_map bitmap instead of relying on
the flags field of the struct type_datum when writing out CIL or
policy.conf policy from a binary.
Reported-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
The kernel checks if the port is in the range 1-255 when loading an
ibenportcon rule. Add the same check to libsepol.
Fixes: 118c0cd103 ("libsepol: Add ibendport ocontext handling")
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
We need to convert from little-endian before dong range checks on the
ibpkey port numbers, otherwise we would be checking a wrong value on
big-endian systems.
Fixes: 9fbb311276 ("libsepol: Add ibpkey ocontext handling")
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Xen uses the initial SIDs domU and domDM in its toolstack, so it makes
sense to add these to xen_sid_to_str[] in kernel_to_common.h
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
When writing CIL from a policy module or when writing CIL or policy.conf
from a kernel binary policy, check that the initial sid index is within
the valid range of the selinux_sid_to_str[] array (or xen_sid_to_str[]
array for a XEN policy). If it is not, then create a unique name
("UNKNOWN"+index) for the initial sid.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
Since the initial sid strings are defined in kernel_to_common.h,
module_to_cil.c can use those and its initial sid string definitions
can be removed.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
Want to make use of selinux_sid_to_str[] and xen_sid_to_str[] from
kernel_to_common.h in module_to_cil.c, but stack functions with the
same names exist in module_to_cil.c and kernel_to_common.c (with
the function prototypes in kernel_to_common.h).
Since the stack functions in kernel_to_common.c are less general and
only work with strings, rename those functions from stack_* to
strs_stack_*.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
Create the function called policydb_sort_ocontexts() that calls
the internal function sort_ocontexts() to sort the ocontexts of
a policydb.
The function sort_ocontexts() is already used by
sepol_kernel_policydb_to_conf() and sepol_kernel_policydb_to_cil()
when converting a binary policy to cil or policy.conf format.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
When write_binary_policy() fails to open the binary policy, it calls
sepol_handle_destroy(f.handle) but structure f has not been initialized
at this point. Use variable handle instead.
This issue has been found using clang's static analyzer.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
f.handle is never set in read_binary_policy() so there is no need to
call sepol_handle_destroy() on it. Moreover clang's static analyzer
warns about an uninitialized argument value in the first call.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
va_copy()'s manpage [1] states:
Each invocation of va_copy() must be matched by a corresponding
invocation of va_end() in the same function.
create_str_helper() is using va_copy() without va_end(). Add the missing
call.
[1] https://linux.die.net/man/3/va_copy
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
sepol_ppfile_to_module_package() does not use its variable "FILE *f =
NULL;" but to fclose() it. This variable has been unneeded since the
introduction of function ppfile_to_module_package() in commit
893851c0a1 ("policycoreutils: add a HLL compiler to convert policy
packages (.pp) to CIL").
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
clang's static analyzer reports that ebitmap_to_names() can call
malloc(0) when the bitmap is empty. If malloc() returns NULL, this
triggers a misleading "Out of memory" error.
Work around this by treating empty bitmaps as appropriate.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
clang's static analyzer warns about dead assignments to local variables.
In module_to_cil.c, there are some which are quite straightforward to
review. Remove them.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
When list_prepend() returns an error, it always means it failed to
allocate some memory and does not hold any reference to its argument
data. This argument needs to be freed by the caller in order to prevent
a memory leak.
While reviewing list_prepend() callers, I spend quite some time
understanding why typealiases_gather_map() does not need to strdup(key)
or free(key) when calling list_prepend(..., key) even though "key" comes
from pdb->p_types.table: because typealias_list_destroy() does not free
the inserted items. Add a comment to make this clearer in the code.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
In cil_tree_print_expr(), "rc < 0" is equivalent to "rc != 0" but
clang's static analyzer does not know about this. Help it.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Fix the following build warning:
policydb.c: In function ‘get_symtab_destroy_func’:
policydb.c:1581:9: error: cast between incompatible function types from ‘int (*)(char *, void *, void *)’ to ‘void (*)(char *, void *, void *)’ [-Werror=cast-function-type]
return (hashtab_destroy_func_t) destroy_f[sym_num];
^
It turns out that this function and type are long unused in libsepol
and are not exported APIs for the shared library, so just remove them.
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
When load_users() parses an invalid line with an empty level context
(ie. nothing between "level" and "range" keywords), it allocates memory
with malloc(0) and uses it. The behavior of malloc() in this case is
an unspecified behavior: it might return NULL, which would lead to a
segmentation fault.
Fix this issue by reporting the invalid entry instead. While at it,
ensure that the character before "range" is a space, and change the
logic slightly in order to avoid using "--p; ... p++;".
This issue is reported by clang's static analyzer with the following
message:
genusers.c:222:11: warning: Use of zero-allocated memory
*r++ = *s;
^
genusers.c:225:7: warning: Use of zero-allocated memory
*r = 0;
^
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
In cond_expr_to_cil(), when stack_init() fails to allocate a stack, the
function calls stack_pop() with stack = NULL. Then stack_pop()
dereferences the pointer ("if (stack->pos == -1) {"), which is NULL.
Fix this by moving the stack cleaning loop in a "if (stack != NULL)"
block.
This issue is reported by clang's static analyzer with the following
message:
module_to_cil.c:463:6: warning: Access to field 'pos' results in a
dereference of a null pointer (loaded from variable 'stack')
if (stack->pos == -1) {
^~~~~~~~~~
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Improve the processing of netifcon, genfscon, ibpkeycon, ibendportcon,
portcon, nodecon, fsuse, filecon, iomemcon, ioportcon, pcidevicecon,
and devicetreecon rules.
If the multiple-decls option is not used then report errors if duplicate
context rules are found. If it is used then remove duplicate context rules
and report errors when two rules are identical except for the context.
This also changes the ordering of portcon and filecon rules. The protocol
of portcon rules will be compared if the port numbers are the same and the
path strings of filecon rules will be compared if the number of meta
characters, the stem length, string length and file types are the same.
Based on an initial patch by Pierre-Hugues Husson (phh@phh.me)
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
This commit resolves conflicts in values of expandattribute statements
in policy language and expandtypeattribute in CIL.
For example, these statements resolve to false in policy language:
expandattribute hal_audio true;
expandattribute hal_audio false;
Similarly, in CIL these also resolve to false.
(expandtypeattribute (hal_audio) true)
(expandtypeattribute (hal_audio) false)
A warning will be issued on this conflict.
Motivation
When Android combines multiple .cil files from system.img and vendor.img
it's possible to have conflicting expandattribute statements.
This change deals with this scenario by resolving the value of the
corresponding expandtypeattribute to false. The rationale behind this
override is that true is used for reduce run-time lookups, while
false is used for tests which must pass.
Signed-off-by: Tri Vo <trong@android.com>
Acked-by: Jeff Vander Stoep <jeffv@google.com>
Acked-by: William Roberts <william.c.roberts@intel.com>
Acked-by: James Carter <jwcart2@tycho.nsa.gov>
Nicolas Iooss reports:
In sepol_ibendport_key_create(), if sepol_ibendport_alloc_ibdev_name()
fails to allocate tmp_key->ibdev_name, sepol_ibendport_key_free() is
called to free the memory associated with tmp_key, which results in
free() being called on uninitialized tmp_key->ibdev_name.
This issue is reported by clang's static analyzer with the following
message:
ibendport_record.c:115:2: warning: 1st function call argument is an
uninitialized value
free(key->ibdev_name);
^~~~~~~~~~~~~~~~~~~~~
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
Export the sepol_polcap_getnum/name() functions to users of
the shared library. This will enable SETools to stop depending
on the static library.
Note that we may want to move polcaps.h up one level since
the convention is that headers directly under include/sepol are
shared library APIs while headers under include/sepol/policydb
are limited to static users. However, this will unnecessarily
break the build for existing static users so it is deferred.
Suggested-by: Chris PeBenito <pebenito@ieee.org>
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
cil_tree_print_expr() calls cil_expr_to_string() in order to compute a
string expression into expr_str. If this function fails, expr_str is
left unitialized but its value is dereferenced with:
cil_log(CIL_INFO, "%s)", expr_str);
Prevent such an issue by checking cil_expr_to_string()'s return value
before using expr_str.
This issue has been found with clang's static analyzer.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
This patch solves the following issues:
- The pkg-config files generates odd paths when using DESTDIR without PREFIX
- DESTDIR is needed during compile time to compute library and header paths which it should not.
- Installing with both DESTDIR and PREFIX set gives us odd paths
- Make usage of DESTDIR and PREFIX more standard
Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
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>
When reading policy, ibendport device names are allocated in
ocontext_read_selinux() but they are not freed when calling
sepol_policydb_free();
Fix this by freeing them in ocontext_selinux_free().
Signed-off-by: Jan Zarsky <jzarsky@redhat.com>
When sepol_bool_query() returns NULL response, variable name is not
freed. Fix this by calling free() before returning.
Signed-off-by: Jan Zarsky <jzarsky@redhat.com>
cil_gen_node() has been using its argument "db" since commit
fafe4c212bf6 ("libsepol: cil: Add ability to redeclare
types[attributes]"). Drop attribute "unused" on this argument.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
cil_defaults_to_policy() defines its third argument as non-const "char
*kind" even though it is called with literal strings. This makes gcc
report the following warning when compiling with -Wwrite-strings:
../cil/src/cil_policy.c: In function ‘cil_gen_policy’:
../cil/src/cil_policy.c:1931:60: error: passing argument 3 of
‘cil_defaults_to_policy’ discards ‘const’ qualifier from pointer
target type [-Werror=discarded-qualifiers]
cil_defaults_to_policy(out, lists[CIL_LIST_DEFAULT_USER],
"default_user");
^~~~~~~~~~~~~~
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Since commit 67b410e80f ("libsepol/cil: Keep attributes used by
generated attributes in neverallow rules") gcc reports the following
warning when building libsepol:
../cil/src/cil_post.c: In function
‘__cil_post_db_neverallow_attr_helper’:
../cil/src/cil_post.c:1322:17: error: unused variable ‘db’
[-Werror=unused-variable]
struct cil_db *db = extra_args;
^~
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
When compiling libsepol with clang and some warning flags, the compiler
complains about the way IPv6 addresses are initialized:
kernel_to_cil.c:2795:35: error: suggest braces around initialization
of subobject [-Werror,-Wmissing-braces]
struct in6_addr subnet_prefix = {0};
^
{}
When replacing the initializer as suggested, gcc 4.8.4 complains:
kernel_to_cil.c: In function ‘write_selinux_ibpkey_rules_to_cil’:
kernel_to_cil.c:2795:9: error: missing initializer for field
‘__in6_u’ of ‘struct in6_addr’ [-Werror=missing-field-initializers]
struct in6_addr subnet_prefix = {};
^
Thankfully netinet/in.h provides a macro to initialize struct in6_addr
variables:
#define IN6ADDR_ANY_INIT { { { 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0 } } }
Both clang and gcc no longer report warnings when using this macro.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
In order to reduce policy size, CIL removes attributes that are not used
by a policy rule in the generated binary policy. However, CIL keeps
attributes used by neverallow rules (which are checked at compile time
and not in the binary policy) even if the attribute is not used anywhere
else in the policy. This behavior is useful to Google who pulls neverallow
rules out of the original policy.conf for compatibility testing, but
converts the policy.conf to CIL and uses the CIL compiler to generate
policy. Without this behavior, the generated binary policy might not have
an attribute referred to by one of the neverallow rules used for testing.
The one exception to this behavior is for attributes generated in
module_to_cil (these have an "_typeattr_" in the middle of their name).
Since these attributes are only created because CIL does not allow a
type expression in an AV rule, they are removed if they only appear in
a neverallow rule (which is the case for most of them) or if the
option to expand generated attributes (-G or --expand-generated) is
specified for secilc when compiling the policy.
Removing generated attributes causes a problem, however, if the type
expression that the generated attribute is replacing uses an attribute
that is removed. In this case, the original neverallow rule will refer
to an attribute that does not exist in the generated binary policy.
Now any non-generated attribute used in a typeattributeset rule for a
generated attribute which is used in a neverallow rule will be treated
like it was used in a neverallow rule.
This does not change the behavior of an expandtypeattribute rule for
the attribute. That rule, if it exists, will take precedence.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
Modify cil_gen_node() to check to see if the cil_db supports multiple
declarations, and if so, to check whether or not the
repeated symbol is eligible to share the existing, already-stored datum. The
only types considered so far are CIL_TYPE and CIL_TYPEATTRIBUTE, both of
which intall empty datums during AST building, so they automatically return
true.
Test: Build policy with multilpe type and attribute declarations, and
without. Policies are binary-identical.
Signed-off-by: Dan Cashman <dcashman@android.com>
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
In cil_strpool_destroy(), cil_strpool_tab is freed but it is not reset to NULL.
When cil_strpool_init() is called again it assumes that cil_strpool_tab was
already initialized. Other functions then work with invalid data.
Signed-off-by: Jan Zarsky <jzarsky@redhat.com>
Define the nnp_nosuid_transition policy capability used to enable
SELinux domain transitions under NNP or nosuid if the
nnp_transition permission or nosuid_transition permission is
allowed between the old and new contexts. When this capability is not
enabled, such transitions remain limited to bounded transitions as they
were prior to the introduction of this capability.
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
A build toolchain may override CPPFLAGS on the command line of "make".
Doing so currently breaks libsepol/tests compilation, as it requires
"-I../include/ -I$(CHECKPOLICY)" to be provided in gcc's arguments.
This completes commit 15f2740733 ("Makefiles: override *FLAGS and
*LIBS").
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
There were several places in the makefiles where LDLIBS or CFLAGS were
supposed to include options to build. They were missing the override
keyword so would be skipped if these vars were set on the make cmdline.
Add the override directive to fix this.
Signed-off-by: Jason Zaman <jason@perfinion.com>
While most header files already use the common pattern of `extern "C"`
declarations to enable compiling in a C++ project, some header files in
libsepol instead use the macros `__BEGIN_DECLS` and `__END_DECLS`. These
macros are defined in the "sys/cdefs.h" header file, which provides
some non-standard extensions for glibc.
Convert usage of these declarations with the standard `extern "C"`
pattern. This improves compatibility with other libc implementations,
e.g. musl libc.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
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>
Commit 9e6840e refactored neverallow checking. In the process a bug
was introduced that causes enabled conditional rules to be skipped.
The bug is that the avtab key is checked by comparing the specified
field of the key to the value AVTAB_ALLOWED. Since enabled conditional
rules have an additional bit set as well, these rules are not
considered to match.
The fix is to use a bitwise AND (&) to only check the desired bit.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
In mls_semantic_range_expand(), when a call to
mls_semantic_level_expand() fails, the function destroys the semantic
level instead of the expanded one. This leads to a use-after-free which
is reported by gcc's Address Sanitizer:
libsepol.mls_semantic_level_expand: mls_semantic_level_expand: invalid sensitivity level found 128/0.
libsepol.sepol_module_package_read: invalid module in module package (at section 0)
Failed to read policy package
=================================================================
==24456==ERROR: AddressSanitizer: heap-use-after-free on address 0x60200000ee58 at pc 0x7fe6c4fb96b4 bp 0x7fffa5ea6b70 sp 0x7fffa5ea6b60
READ of size 8 at 0x60200000ee58 thread T0
#0 0x7fe6c4fb96b3 in mls_semantic_level_destroy /usr/src/selinux/libsepol/src/mls.c:755
#1 0x7fe6c4fb9b88 in mls_semantic_range_destroy /usr/src/selinux/libsepol/src/mls.c:802
#2 0x7fe6c500e8ab in user_datum_destroy /usr/src/selinux/libsepol/src/policydb.c:535
#3 0x7fe6c500e980 in user_destroy /usr/src/selinux/libsepol/src/policydb.c:1390
#4 0x7fe6c4f36c48 in hashtab_map /usr/src/selinux/libsepol/src/hashtab.c:235
#5 0x7fe6c50152da in symtabs_destroy /usr/src/selinux/libsepol/src/policydb.c:1595
#6 0x7fe6c5015433 in policydb_destroy /usr/src/selinux/libsepol/src/policydb.c:1503
#7 0x7fe6c5040e0d in sepol_policydb_free /usr/src/selinux/libsepol/src/policydb_public.c:82
#8 0x7fe6c4fbc503 in sepol_module_package_free /usr/src/selinux/libsepol/src/module.c:143
#9 0x7fe6c4fefefb in sepol_ppfile_to_module_package /usr/src/selinux/libsepol/src/module_to_cil.c:4293
#10 0x401e51 in main /usr/src/selinux/policycoreutils/hll/pp/pp.c:124
#11 0x7fe6c4add510 in __libc_start_main (/usr/lib/libc.so.6+0x20510)
#12 0x402589 in _start (/usr/src/selinux/DESTDIR/usr/libexec/selinux/hll/pp+0x402589)
0x60200000ee58 is located 8 bytes inside of 16-byte region [0x60200000ee50,0x60200000ee60)
freed by thread T0 here:
#0 0x7fe6c5537ae0 in __interceptor_free /build/gcc-multilib/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:45
#1 0x7fe6c4fb969b in mls_semantic_level_destroy /usr/src/selinux/libsepol/src/mls.c:757
#2 0x7fe6c4f02a57 in mls_semantic_range_expand /usr/src/selinux/libsepol/src/expand.c:948
#3 0x7fe6c5007a98 in policydb_user_cache /usr/src/selinux/libsepol/src/policydb.c:939
#4 0x7fe6c4f36c48 in hashtab_map /usr/src/selinux/libsepol/src/hashtab.c:235
#5 0x7fe6c5013859 in policydb_index_others /usr/src/selinux/libsepol/src/policydb.c:1286
#6 0x7fe6c5020b65 in policydb_read /usr/src/selinux/libsepol/src/policydb.c:4342
#7 0x7fe6c4fc0cdb in sepol_module_package_read /usr/src/selinux/libsepol/src/module.c:618
#8 0x7fe6c4ff008d in sepol_ppfile_to_module_package /usr/src/selinux/libsepol/src/module_to_cil.c:4276
#9 0x401e51 in main /usr/src/selinux/policycoreutils/hll/pp/pp.c:124
#10 0x7fe6c4add510 in __libc_start_main (/usr/lib/libc.so.6+0x20510)
previously allocated by thread T0 here:
#0 0x7fe6c5537e40 in __interceptor_malloc /build/gcc-multilib/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:62
#1 0x7fe6c5004efc in mls_read_semantic_level_helper /usr/src/selinux/libsepol/src/policydb.c:1976
#2 0x7fe6c500f596 in mls_read_semantic_range_helper /usr/src/selinux/libsepol/src/policydb.c:2010
#3 0x7fe6c500f596 in user_read /usr/src/selinux/libsepol/src/policydb.c:3258
#4 0x7fe6c502055b in policydb_read /usr/src/selinux/libsepol/src/policydb.c:4286
#5 0x7fe6c4fc0cdb in sepol_module_package_read /usr/src/selinux/libsepol/src/module.c:618
#6 0x7fe6c4ff008d in sepol_ppfile_to_module_package /usr/src/selinux/libsepol/src/module_to_cil.c:4276
#7 0x401e51 in main /usr/src/selinux/policycoreutils/hll/pp/pp.c:124
#8 0x7fe6c4add510 in __libc_start_main (/usr/lib/libc.so.6+0x20510)
SUMMARY: AddressSanitizer: heap-use-after-free /usr/src/selinux/libsepol/src/mls.c:755 in mls_semantic_level_destroy
Shadow bytes around the buggy address:
0x0c047fff9d70: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c047fff9d80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c047fff9d90: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c047fff9da0: fa fa fa fa fa fa fa fa fa fa 01 fa fa fa 01 fa
0x0c047fff9db0: fa fa 01 fa fa fa 01 fa fa fa 01 fa fa fa 01 fa
=>0x0c047fff9dc0: fa fa 00 00 fa fa 00 00 fa fa fd[fd]fa fa fd fd
0x0c047fff9dd0: fa fa fd fd fa fa fd fd fa fa fd fa fa fa fd fd
0x0c047fff9de0: fa fa 04 fa fa fa 00 01 fa fa fd fd fa fa fd fd
0x0c047fff9df0: fa fa fd fd fa fa fd fd fa fa 00 00 fa fa fd fd
0x0c047fff9e00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c047fff9e10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Heap right redzone: fb
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack partial redzone: f4
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
==24456==ABORTING
This issue has been found while fuzzing hll/pp with the American Fuzzy
Lop.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Commit 1089665e31a647a5f0ba2eabe8ac6232b384bed9 (Add attribute
expansion options) adds an expandattribute rule to the policy.conf
language which sets a type_datum flag. Currently the flag is used
only when writing out CIL policy from a policy.conf.
Make use of the flag when expanding policy to expand policy rules
and remove all type associations for an attribute that has
TYPE_FLAGS_EXPAND_ATTR_TRUE set. (The attribute will remain in the
policy, but have no types associated with it.)
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
Nicolas Iooss <nicolas.iooss@m4x.org> discovered with clang's static
analyzer that cil_reset_ibpkeycon() was checking that ibpkeycon->context
was NULL and then passing the NULL value to cil_reset_context() which
expected a non-NULL argument.
Instead, cil_reset_ibpkeycon() should check if ibpkeycon->context_str
is NULL. If it is non-NULL then the context field points to a named
context that was created elsewhere and it will be reset there, but if
the context_str field is NULL, then the context is not named and needs
to be reset.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
This prevented cil_resolve_name() from returning an actual thing when a
name resolved to an alias. This appears to have only affected resolution
dealing with sensitivity and category aliases. Type aliases were not
affected since places that dealt with types handled type aliases
specifically and did not rely on this behavior from cil_resolve_name().
Signed-off-by: Steve Lawrence <slawrence@tresys.com>
when building packages (e.g. for openSUSE Linux)
(random) filesystem order of input files
influences ordering of functions in the output,
thus without the patch, builds (in disposable VMs) would usually differ.
See https://reproducible-builds.org/ for why this matters.
Fixes the following warning from gcc 7:
In function ‘name_list_to_string’,
inlined from ‘constraint_expr_to_string’ at module_to_cil.c:1790:8:
module_to_cil.c:1135:6: warning: argument 1 range [18446744071562067968, 18446744073709551615] exceeds maximum object size 9223372036854775807 [-Walloc-size-larger-than=]
str = malloc(len);
~~~~^~~~~~~~~~~~~
In file included from module_to_cil.c:36:0:
module_to_cil.c: In function ‘constraint_expr_to_string’:
/usr/include/stdlib.h:443:14: note: in a call to allocation function ‘malloc’ declared here
extern void *malloc (size_t __size) __THROW __attribute_malloc__ __wur;
^~~~~~
While we are here, fix a few other issues too.
The usage of snprintf was wrong and unnecessary; we just allocated
the string to be the right size, so we should just fill it.
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
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>
Type aliases present a problem for module_to_cil because they are not
in the sym_val_to_name table that it uses to write declarations. Type
aliases are gathered by going through the decl_ids list and then
the alias declaration is written out when the block with that scope
id is handled. This doesn't work if a type alias appears in a require
block, since the require cannot be distinguished from the declaration.
The result is two declarations of the alias and an error when secilc
compiles the policy.
Because of the work cleaning up scope handling, the alias declaration
will always be at the end of the decl_ids list, so now only gather
the last scope id.
Also, when an alias is used in a module it is required as a type and
it will appear in the sym_val_to_name table. When that occurs, just
skip the alias when writing out types.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
Currently, when checking if an identifier is enabled, each scope in
the decl_ids list is checked. This means that if any block that
requires the identifier is enabled, then the identifier will be treated
as being declared.
Now, declarations will be kept at the end of the decl_ids list and
when checking if an identifier is enabled, only the last scope will
be checked (Except for roles and users which allow multiple declarations,
they will have to keep the old behavior.)
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
Assigning NULL to handle does not perform anything useful and clang
complains about this:
ibendports.c:122:2: error: 'handle' was marked unused but was used
[-Werror,-Wused-but-marked-unused]
handle = NULL;
^
ibpkeys.c:115:2: error: 'handle' was marked unused but was used
[-Werror,-Wused-but-marked-unused]
handle = NULL;
^
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
- If two typealiasactual statements exist for the same typealias, we get
a confusing error message mentioning that the actual arguement is not
an alias, which is clearly allowed. This poor error occurs because the
first typealiasactual statement resolves correctly, but when we
resolve the alias in the second typealiasactual statement,
cil_resolve_name tries to return what the alias points to, which is a
type and not the required typealias. This patch creates a new function
that does not perform the alias to actual conversion, used when we
want an alias and not what the alias points to. This allows the
cil_resolve_aliasactual to continue and reach the check for duplicate
typealiasactual statements, resulting in a more meaningful error
message.
- Add back support for aliases to aliases (broken in 5c9fcb02e),
while still ensuring that aliases point to either the correct actual
flavor or alias flavor, and not something else like a typeattribute.
Signed-off-by: Steve Lawrence <slawrence@tresys.com>
- Set rc to SEPOL_ERR if the alias part of an aliasactual statement
does not resolve to the correct alias flavor (e.g. typealias, senalias, catalias)
- Add an error check if the actual part of an aliasactual statement
does not resolve to the correct actual flavor (type, sens, cat)
Signed-off-by: Steve Lawrence <slawrence@tresys.com>
When a function called by sepol_module_policydb_to_cil() fails before
role_list_create() has been called, role_list is still NULL but is
dereferenced in role_list_destroy(). Here is a gdb session on hll/pp:
Unknown value for handle-unknown: 6
Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7a68a37 in role_list_destroy () at module_to_cil.c:215
215 struct list_node *curr = role_list->head;
(gdb) bt
#0 0x00007ffff7a68a37 in role_list_destroy () at
module_to_cil.c:215
#1 sepol_module_policydb_to_cil (fp=fp@entry=0x7ffff79925e0
<_IO_2_1_stdout_>, pdb=<optimized out>, linked=linked@entry=0) at
module_to_cil.c:4060
#2 0x00007ffff7a6ac75 in sepol_module_package_to_cil
(fp=fp@entry=0x7ffff79925e0 <_IO_2_1_stdout_>, mod_pkg=0x604280) at
module_to_cil.c:4080
#3 0x0000000000401a58 in main (argc=<optimized out>,
argv=<optimized out>) at pp.c:150
This issue has been found while fuzzing hll/pp with the American Fuzzy
Lop.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Update libsepol and libsemanage to work with ibendport records. Add local
storage for new and modified ibendport records in ibendports.local.
Update semanage to parse the ibendport command options to add, modify,
and delete them.
Signed-off-by: Daniel Jurgens <danielj@mellanox.com>
Update libsepol and libsemanage to work with pkey records. Add local
storage for new and modified pkey records in pkeys.local. Update semanage
to parse the pkey command options to add, modify, and delete pkeys.
Signed-off-by: Daniel Jurgens <danielj@mellanox.com>
Add support for reading, writing, and copying IB end port ocontext data.
Also add support for querying a IB end port sid to checkpolicy.
Signed-off-by: Daniel Jurgens <danielj@mellanox.com>
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>
Add support for reading, writing, and copying Infiniband Pkey ocontext
data. Also add support for querying a Pkey sid to checkpolicy.
Signed-off-by: Daniel Jurgens <danielj@mellanox.com>
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>
When allocating an array with calloc(), the first argument usually is
the number of items and the second one the size of an item. Doing so
silences a warning reported by clang's static analyzer:
kernel_to_cil.c:2050:14: warning: Call to 'calloc' has an allocation
size of 0 bytes.
cond_data = calloc(sizeof(struct cond_data), num);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
When common_to_cil() or class_to_cil() fail to allocate an array to map
a permissions hashtable (for example when permissions.nprim is too big),
class_perm_to_array() gets called on a NULL pointer. Fix this.
This issue has been found while fuzzing hll/pp with the American Fuzzy
Lop.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Since commit 58962eb3d8 ("libsepol,checkpolicy: add binary module
support for xperms") function avrule_read() has been using its "p"
argument even though it was previously marked unused. This makes clang
report:
policydb.c:3276:7: error: 'p' was marked unused but was used
[-Werror,-Wused-but-marked-unused].
if (p->policyvers < MOD_POLICYDB_VERSION_XPERMS_IOCTL) {
^
Remove the attribute to make the code consistent again.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Presently we support xperms rules in source policy and in CIL modules.
The binary policy module format however was never extended for xperms.
This limitation inhibits use of xperms in refpolicy-based policy modules
(including the selinux-testsuite policy). Update libsepol to support
linking, reading, and writing a new binary policy module version that
supports xperms rules. Update dismod to display xperms rules in binary
policy modules.
Also, to support use of a non-base binary policy module with a newer
version on a system using a base policy module with an older version,
automatically upgrade the version during module linking. This facilitates
usage of newer features in non-base modules without requiring rebuilding
the base module.
Tests:
1. Add an allowxperms rule to the selinux-testsuite policy and
confirm that it is properly written to the binary policy module
(displayed by dismod), converted to CIL (the latter was already supported),
and included in the kernel policy (via dispol and kernel test).
2. Use semodule_link and semodule_expand to manually link and expand
all of the .pp files via libsepol, and confirm that the allowxperms rule
is correctly propagated to the kernel policy. This test is required to
exercise the legacy link/expand code path for binary modules that predated
CIL.
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
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>
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>
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>
fcb5d5c removed ../include from CFLAGS from libsepol/utils/Makefile so
that a build tool can't find sepol/sepol.h when only libsepol is built
and a system is without sepol.h in standard paths. It should use its own
sepol.h file during build. `oveeride` needs to be used in order not to
be overridden by values provided on a command line. Same problem applies
to LDFLAGS.
Fixes:
$ make CFLAGS="" LDFLAGS=""
make[1]: Entering directory '/root/selinux/libsepol/utils'
cc chkcon.c -lsepol -o chkcon
chkcon.c:1:25: fatal error: sepol/sepol.h: No such file or directory
#include <sepol/sepol.h>
$ make CFLAGS="" LDFLAGS=""
...
make -C utils
make[1]: Entering directory '/root/selinux/libsepol/utils'
cc -I../include chkcon.c -lsepol -o chkcon
/usr/bin/ld: cannot find -lsepol
collect2: error: ld returned 1 exit status
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
When compiling with -Wwrite-strings, the compiler complains about
calling strs_add with a const char* value for a char* parameter
(DEFAULT_OBJECT is defined to "object_r"). Silence this warning by
casting the literal string to char*.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
The toolchain automatically handles them and they break cross compiling.
LDFLAGS should also come before object files, some flags (eg,
-Wl,as-needed) can break things if they are in the wrong place)
Gentoo-Bug: https://bugs.gentoo.org/500674
Signed-off-by: Jason Zaman <jason@perfinion.com>
cil_gen_default() and cil_gen_defaultrange() call cil_fill_list()
without checking its return value. If it failed, propagate the return
value to the caller.
This issue has been found using clang's static analyzer. It reported
"warning: Value stored to 'rc' is never read" four times.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Originally, all type attributes were expanded when building a binary
policy. As the policy grew, binary policy sizes became too large, so
changes were made to keep attributes in the binary policy to minimize
policy size.
Keeping attributes works well as long as each type does not have too
many attributes. If an access check fails for types t1 and t2, then
additional checks must be made for every attribute that t1 is a member
of against t2 and all the attributes that t2 is a member of. This is
O(n*m) behavior and there are cases now where this is becoming a
performance issue.
Attributes are more aggressively removed than before. An attribute
will now be removed if it only appears in rules where attributes are
always expanded (typetransition, typechange, typemember, roletransition,
rangetransition, roletype, and AV Rules with self).
Attributes that are used in constraints are always kept because the
attribute name is stored for debugging purposes in the binary policy.
Attributes that are used in neverallow rules, but not in other AV rules,
will be kept unless the attribute is auto-generated.
Attributes that are only used in AV rules other than neverallow rules
are kept unless the number of types assigned to them is less than the
value of attrs_expand_size in the CIL db. The default is 1, which means
that any attribute that has no types assigned to it will be expanded (and
the rule removed from the policy), which is CIL's current behavior. The
value can be set using the function cil_set_attrs_expand_size().
Auto-generated attributes that are used only in neverallow rules are
always expanded. The rest are kept by default, but if the value of
attrs_expand_generated in the CIL db is set to true, they will be
expanded. The function cil_set_attrs_expand_generated() can be used
to set the value.
When creating the binary policy, CIL will expand all attributes that
are being removed and it will expand all attributes with less members
than the value specified by attrs_expand_size. So even if an attribute
is used in a constraint or neverallow and the attribute itself will be
included in the binary policy, it will be expanded when writing AV
rules if it has less members than attrs_expand_size.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
name_list_to_string() and constraint_expr_to_string() both define an
exit label to clean-up dynamically-allocated memory when an error
occurs, but they miss some variables. Free the missing ones too.
This issue has been found using clang's static analyzer.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
When set_to_names() fails to allocate *names, it frees variable
attr_name even though it either came from attr_list or was newly created
and added to attr_list. By doing so, the name is freed a second time
when attr_list is destroyed (with "attr_list_destroy(&attr_list)").
Avoid this double free by not freeing attr_name when it belongs to
attr_list.
This issue has been found using clang's static analyzer.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Some invalid policies might have p->p_types.nprim = 0. When parsing
such a policy, "i > p->p_types.nprim - 1" is always false even though
reading p->type_val_to_struct[i] triggers a segmentation fault.
Make type_set_expand() return an error when parsing such a policy by
handling correctly when p->p_types.nprim is zero.
This issue has been found while fuzzing semodule_package with the
American Fuzzy Lop.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Some functions assumes that p->global is not NULL. For example
range_read() contains:
p->global->enabled->range_tr_rules = rtr;
However p->global may currently be NULL when loading a policy module
with no avrule block. Avoid a NULL pointer dereference by making such a
policy invalid.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
sepol_*_key_free(NULL) should just be a no-op just like
free(NULL). Fix several instances that did not handle this
correctly and would seg fault if called with NULL.
Test: setsebool -P zebra_write_config=1 while non-root
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
CIL does not allow type or role sets in certain rules (such as allow
rules). It does, however, allow sets in typeattributeset and
roleattributeset statements. Because of this, when module_to_cil
translates a policy into CIL, it creates a new attribute for each
set that it encounters. But often the same set is used multiple times
which means that more attributes are created then necessary. As the
number of attributes increases the time required for the kernel to
make each policy decision increases which can be a problem.
To help reduce the number of attributes in a kernel policy,
when module_to_cil encounters a role or type set search to see if the
set was encountered already and, if it was, use the previously
generated attribute instead of creating a new one.
Testing on Android and Refpolicy policies show that this reduces the
number of attributes generated by about 40%.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
It would sometimes be helpful for debugging or verification purposes
to be able to convert a binary policy to a human-readable form.
Create new function, sepol_kernel_policydb_to_conf(), that takes a
policydb created from a binary policy and writes a policy.conf file
to the provided FILE pointer.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
It would sometimes be helpful for debugging or verification purposes
to be able to convert a binary policy to a human-readable form.
Create new function, sepol_kernel_policydb_to_cil(), that takes a
policydb created from a binary policy and writes CIL policy to the
provided FILE pointer.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
When sepol_user_add_role() fails to allocate memory for role_cp but
succeeds in reallocating user->roles memory, it frees this reallocated
memory, thus leaving user->roles referencing a free memory block. When
sepol_user_clone() calls sepol_user_free(new_user) because the
allocation failure made sepol_user_add_role() fail, the following code
is executed:
for (i = 0; i < user->num_roles; i++)
free(user->roles[i]);
free(user->roles);
As user->roles has been freed, this code frees pointers which may be
invalid and then tries to free user->roles again.
Fix this flaw by returning right after strdup() failed in
sepol_user_add_role().
This issue has been found using clang's static analyzer.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
When load_booleans() calls process_boolean() to parse a boolean
definition, process_boolean() returns a successful value when it fails
to use strtok_r() (e.g. when there is no "=" in the parsed line). This
leads load_booleans() to use uninitialized name and/or val when setting
the boolean into the policy.
Rework process_boolean() in order to report errors when a boolean
definition is incorrect.
This issue has been found using clang's static analyzer.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
In cond_expr_to_cil() when stack_init(&stack) fails, stack is set to
NULL and the execution flow jumps to label "exit". This triggers a call
to stack_pop(stack) which dereferences a NULL pointer in "if (stack->pos
== -1)".
This issue has been found using clang's static analyzer.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
When list_init() fails to allocate a list with calloc(), it calls
list_destroy(&l) with l = NULL. This functions starts by dereferencing
its argument ("(*list)->head"), which does not work well when it is
NULL.
This bug can be fixed by returning directly in list_init() when calloc()
fails. Doing so allows making list_init() implementation shorter by
removing label "exit" and local variable "rc".
This issue has been found using clang's static analyzer.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
When writing a policy.conf file from CIL source, use hexadecimal
numbers in ioportcon, iomemcon, and pcidevicecon rules.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
Allow the use of hexadecimal numbers in iomemcon, ioportcon, and
pcidevicecon statements. The use of hexadecimal numbers is often
the natural choice for these rules.
A zero base is now passed to strtol() and strtoull() which will
assume base 16 if the string has a prefix of "0x", base 8 if the
string starts with "0", and base 10 otherwise.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
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>
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>
Define the new cgroup_seclabel policy capability used to
enable userspace setting of security labels on cgroup files
via setfscreatecon() aka /proc/self/attr/fscreate and/or
setfilecon() aka setxattr().
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
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>
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>
__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>
