In function define_type(), some error conditions between "id =
queue_remove(id_queue)" and "get_local_type(id, attr->s.value, 1)"
returned without freeing id. Fix theses memory leaks.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
After libsepol is modified (for example while developing new features or
fixing bugs), running "make install" in the top-level directory does not
update the programs which use libsepol.a. Add this static library to the
target dependencies in order to force their updates. This makes "make"
use libsepol.a in the linking command without using LDLIBS.
While at it, copy what commit 14d7064348 ("libselinux: Allow
overriding libsepol.a location during build") introduced in libselinux
Makefile by using a new LIBSEPOLA variable in all Makefiles.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Unlike queue_remove(), queue_head() does not modify the queue, but
rather, returns a pointer to an element within the queue. Freeing the
memory associated with a value returned from that function corrupts
subsequent users of the queue, who may try to reference this
now-deallocated memory.
This causes the following policy generation errors on Android:
FAILED:
out/target/product/bullhead/obj/ETC/plat_sepolicy.cil_intermediates/plat_policy_nvr.cil
/bin/bash -c "out/host/linux-x86/bin/checkpolicy -M -C -c 30 -o
out/target/product/bullhead/obj/ETC/plat_sepolicy.cil_intermediates/plat_policy_nvr.cil
out/target/product/bullhead/obj/ETC/plat_sepolicy.cil_intermediates/plat_policy.conf"
system/sepolicy/public/app.te:241:ERROR 'only ioctl extended permissions
are supported' at token ';' on line 6784:
#line 241
} };
checkpolicy: error(s) encountered while parsing configuration
because the value of "id" in:
id = queue_remove(id_queue);
if (strcmp(id,"ioctl") == 0) {
...
} else {
yyerror("only ioctl extended permissions are supported");
...
}
is now garbage.
This is a partial revert of the following commit:
c1ba8311 checkpolicy: free id where it was leaked
Signed-off-by: Nick Kralevich <nnk@google.com>
The prototype of isdigit() is provided by ctypes.h header. Without
including this file, gcc fails to build checkpolicy using musl libc:
checkpolicy.c: In function ‘main’:
checkpolicy.c:705:8: error: implicit declaration of function
‘isdigit’ [-Werror=implicit-function-declaration]
if (isdigit(ans[0])) {
^~~~~~~
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
In an error path of define_bool_tunable(), variable id is freed after
being used by a successful call to declare_symbol(). This may cause
trouble as this pointer may have been used as-is in the policy symtab
hash table.
Moreover bool_value is never freed after being used. Fix this memory
leak too. This leak has been detected with gcc Address Sanitizer.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Several functions in policy_define.c do not free id after handling it.
Add the missing free(id) statements.
The places where free(id) was missing were found both with gcc Address
Sanitizer and manual code inspection.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Elements which are inserted into a queue_t object are either NULL (from
insert_separator()) or strings allocated with malloc() in insert_id().
They would be freed if there are still present in the queue when it is
destroyed. Otherwise the memory allocated for these elements would be
leaked.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
When parsing several genfscon statements for the same filesystem, the
content of local variable "fstype" is never freed. Moreover variable
"type" is never freed when define_genfs_context_helper() succeeds.
Fix these leaks by calling free() appropriately.
These leaks have been detected with gcc Address Sanitizer.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Variable id is almost never freed in define_port_context().
This leak has been detected with gcc Address Sanitizer.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
When building checkpolicy/test, the linker reports the following error:
cc dispol.o -lfl /usr/src/selinux/DESTDIR/usr/lib/libsepol.a
-L/usr/src/selinux/DESTDIR/usr/lib -o dispol
/usr/lib/gcc/x86_64-pc-linux-gnu/6.2.1/../../../../lib/libfl.so:
undefined reference to `yylex'
collect2: error: ld returned 1 exit status
According to flex documentation
(https://github.com/westes/flex/blob/master/doc/flex.texi), -lfl is used
to provide an implementation for yywrap(). However every flex file now
uses "%option noyywrap", which makes -lfl no longer mandatory. Remove
this option from checkpolicy Makefiles.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
range transition and name-based type transition rules were originally
simple unordered lists. They were converted to hashtabs in the kernel
by commit 2f3e82d694d3d7a2db019db1bb63385fbc1066f3 ("selinux: convert range
transition list to a hashtab") and by commit
2463c26d50adc282d19317013ba0ff473823ca47 ("SELinux: put name based
create rules in a hashtable"), but left unchanged in libsepol and
checkpolicy. Convert libsepol and checkpolicy to use the same hashtabs
as the kernel for the range transitions and name-based type transitions.
With this change and the preceding one, it is possible to directly compare
a policy file generated by libsepol/checkpolicy and the kernel-generated
/sys/fs/selinux/policy pseudo file after normalizing them both through
checkpolicy. To do so, you can run the following sequence of commands:
checkpolicy -M -b /etc/selinux/targeted/policy/policy.30 -o policy.1
checkpolicy -M -b /sys/fs/selinux/policy -o policy.2
cmp policy.1 policy.2
Normalizing the two files via checkpolicy is still necessary to ensure
consistent ordering of the avtab entries. There may still be potential
for other areas of difference, e.g. xperms entries may lack a well-defined
order.
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
checkpolicy wrongly handles "-self". At the least, it should handle it as
an error. At best, it should support it correctly (which would involve
libsepol support as well). At present, it looks like it will end up
negating (-) the next type/attribute in the list after self, or if
there are no entries after self, ignoring it entirely.
This originally was raised by the Android team, which wanted to support
something like the following:
neverallow domain { domain -self }:dir search;
to prohibit cross domain access to some resource but allow access within
the same domain.
This change just makes it a fatal error during compilation.
Implementing real support for -self is left as future work.
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
This fixes most of the errors reported in "make -C libsepol test":
./libsepol-tests
CUnit - A unit testing framework for C - Version 2.1-3
http://cunit.sourceforge.net/
Suite: cond
Test: cond_expr_equal ...passed
Suite: linker
Test: linker_indexes ...passed
Test: linker_types ...passed
Test: linker_roles ...
role o1_b_role_1 has 0 types, 1 expected
role o1_b_role_1 has 0 types, 1 expected
role o1_m1_role_1 has 0 types, 1 expected
sym g_b_role_2 has 1 decls, 2 expected
Role o1_b_role_2 had type o1_b_type_1 not in types array
role o1_b_role_2 has 0 types, 1 expected
Role g_b_role_4 had type g_m1_type_2 not in types array
role g_b_role_4 has 0 types, 1 expected
role o3_b_role_1 has 0 types, 1 expected
role o3_b_role_1 has 0 types, 1 expected
role o4_b_role_1 has 0 types, 1 expected
Role o4_b_role_1 had type g_m1_type_1 not in types array
FAILED
1. test-common.c:216 - found == len
2. test-common.c:216 - found == len
3. test-common.c:216 - found == len
4. test-common.c:43 - scope->decl_ids_len == len
5. test-common.c:52 - found == 1
6. test-common.c:213 - new == 1
7. test-common.c:216 - found == len
8. test-common.c:213 - new == 1
9. test-common.c:216 - found == len
10. test-common.c:216 - found == len
11. test-common.c:216 - found == len
12. test-common.c:216 - found == len
13. test-common.c:213 - new == 1
Test: linker_cond ...passed
Suite: expander
Test: expander_indexes ...passed
Test: expander_attr_mapping ...passed
Test: expander_role_mapping ...passed
Test: expander_user_mapping ...passed
Test: expander_alias ...passed
Suite: deps
Test: deps_modreq_global ...passed
Test: deps_modreq_opt ...passed
Suite: downgrade
Test: downgrade ...passed
Run Summary: Type Total Ran Passed Failed Inactive
suites 5 5 n/a 0 0
tests 13 13 12 1 0
asserts 1269 1269 1256 13 n/a
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
checkpolicy currently imposes arbitrary limits on pathnames used
in genfscon and other statements. This prevents specifying certain
paths in /proc such as those containing comma (,) characters.
Generalize the PATH, QPATH, and FILENAME patterns to support most
legal pathnames.
For simplicity, we do not support pathnames containing newlines or
quotes.
Reported-by: Inamdar Sharif <isharif@nvidia.com>
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
Originally checkmodule stated that it wrote to the input file instead of
to the output file.
Reported-By: Milos Malik <mmalik@redhat.com>
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
As per discussion in https://android-review.googlesource.com/#/c/221980,
we should be using #ifdef __APPLE__ rather than our own custom-defined
DARWIN for building on MacOS X.
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
Commit 3895fbbe0c ("selinux: Add support
for portcon dccp protocol") added support for the (portcon dccp ..)
statement. This fix will allow policy to be built on platforms
(see [1]) that do not have DCCP support by defining the IANA
assigned IP Protocol Number 33 to IPPROTO_DCCP.
[1] https://android-review.googlesource.com/#/c/219568/
Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
Since CIL treats files as modules and does not have a separate
module statement it can cause confusion when a Refpolicy module
has a name that is different than its base filename because older
SELinux userspaces will refer to the module by its module name while
a CIL-based userspace will refer to it by its filename.
Because of this, have checkmodule fail when compiling a module and
the output base filename is different than the module name.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
This adds CIL and checkpolicy support for the (portcon dccp ...)
statement. The kernel already handles name_bind and name_connect
permissions for the dccp_socket class.
Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
Neverallow rules for ioctl extended permissions will pass in two
cases:
1. If extended permissions exist for the source-target-class set
the test will pass if the neverallow values are excluded.
2. If extended permissions do not exist for the source-target-class
set the test will pass if the ioctl permission is not granted.
Signed-off-by: Jeff Vander Stoep <jeffv@google.com>
Acked-by: Nick Kralevich <nnk@google.com>
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
checkpolicy was directly assigning type sets rather than using
type_set_cpy() and therefore creating pointer aliases to the
same type set from multiple filename-based type transition rules
if they specified multiple classes. This would then yield a double
free when destroying the rules afterward and a segmentation fault.
Fix it to use type_set_cpy().
Reported-by: William C Roberts <william.c.roberts@intel.com>
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
The ioctl operations code is being renamed to the more generic
"extended permissions." This commit brings the policy compiler
up to date with the kernel patch.
Signed-off-by: Jeff Vander Stoep <jeffv@google.com>
Fixes compiler warnings all similar to the following:
host C: checkpolicy <= external/selinux/checkpolicy/policy_define.c
external/selinux/checkpolicy/policy_define.c:1572:2: warning: comparison of integers of different signs: 'int' and 'uint32_t' (aka 'unsigned int') [-Wsign-compare]
ebitmap_for_each_bit(&tclasses, node, i) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
external/selinux/checkpolicy/../libsepol/include/sepol/policydb/ebitmap.h:76:39: note: expanded from macro 'ebitmap_for_each_bit'
for (bit = ebitmap_start(e, &n); bit < ebitmap_length(e); bit = ebitmap_next(&n, bit)) \
^ ~~~~~~~~~~~~~~~~~
Signed-off-by: Nick Kralevich <nnk@google.com>
--089e013a1a2abb8ecf0518469d04
Content-Type: text/plain; charset=UTF-8
assert() only prevents -Wreturn-type from firing if asserts are
enabled. Use abort() so we don't do unexpected things even if we use
-UNDEBUG.
<div dir="ltr"><div>assert() only prevents -Wreturn-type from firing if asserts are</div><div>enabled. Use abort() so we don't do unexpected things even if we use</div><div>-UNDEBUG.</div></div>
From b53ad041da Mon Sep 17 00:00:00 2001
From: Dan Albert <danalbert@google.com>
Date: Wed, 10 Jun 2015 17:01:23 -0700
Subject: [PATCH] Fix -Wreturn-type issues.
To: selinux@tycho.nsa.gov
Cc: nnk@google.com,
sds@tycho.nsa.gov
assert() only prevents -Wreturn-type from firing if asserts are
enabled. Use abort() so we don't do unexpected things even if we use
-UNDEBUG.
Also drop expanding of rules; just display the rules in their
original form. I think expansion was a relic of an older policy
version where we did not preserve attributes in the kernel policy.
In any event, it seems more useful to display the rules unmodified.
Change-Id: I85095a35cfb48138cd9cf01cde6dd0330e342c61
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
Adds support for new policy statements whitelisting individual ioctl
commands. Ioctls provide many of the operations necessary for driver control.
The typical driver supports a device specific set of operations accessible
by the ioctl system call and specified by the command argument. SELinux
provides per operation access control to many system operations e.g. chown,
kill, setuid, ipc_lock, etc. Ioclts on the other hand are granted on a per
file descriptor basis using the ioctl permission, meaning that the set of
operations provided by the driver are granted on an all-or-nothing basis.
In some cases this may be acceptable, but often the same driver provides a
large and diverse set of operations such as benign and necessary functionality
as well as dangerous capabilities or access to system information that should
be restricted.
Example policy:
allow <source> <target>:<class> { 0x8900-0x8905 0x8910 }
auditallow <source> <target>:<class> 0x8901
The ioctl permission is still required in order to make an ioctl call. If no
individual ioctl commands are specified, only the ioctl permission is
checked by the kernel - i.e. status quo. This allows ioctl whitelisting to
done in a targeted manner, protecting desired drivers without requiring every
ioctl command to be known and specified before use and otherwise allowing
existing policy to be used as-is.
This only implements ioctl whitelisting support for monolithic kernel policies
built via checkpolicy. Support for modules and CIL remains to be done.
Bug: 19419509
Change-Id: I198e8c9279b94d8ce4ae5625018daa99577ee970
Signed-off-by: Jeff Vander Stoep <jeffv@google.com>
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
Add support to checkpolicy and checkmodule for generating CIL as their
output.
Add new options "-C" and "--cil" to specify CIL as the output format.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
In Xen on ARM, device tree nodes identified by a path (string) need to
be labeled by the security policy.
Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
This expands IOMEMCON device context entries to 64 bits. This change is
required to support static I/O memory range labeling for systems with
over 16TB of physical address space. The policy version number change
is shared with the next patch.
While this makes no changes to SELinux policy, a new SELinux policy
compatibility entry was added in order to avoid breaking compilation of
an SELinux policy without explicitly specifying the policy version.
Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
In order to support paths containing spaces or other characters, allow a
quoted string with these characters to be parsed as a path in addition
to the existing unquoted string.
Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
When the FILESYSTEM token was added to support filesystem names that
start with a digit (e.g. 9p), it was given higher precedence than
NUMBER and therefore all values specified in hex (with 0x prefix)
in policy will incorrectly match FILESYSTEM and yield a syntax error.
This breaks use of iomem ranges in Xen policy and will break ioctl
command ranges in a future SELinux policy version. Switch the
precedence. This does mean that you cannot currently have a filesystem
with a name that happens to be 0x followed by a hexval but hopefully
that isn't an issue.
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
Our build system compiles flex/bison as C++ rather than C, but a few
projects add `-x c` to their flags, forcing the compiler to compile
them as C. This causes the compiler to reject the global C++ standard
flag, so we need to explicitly provide a C standard flag to override
it.
Bug: 18466763
Change-Id: I49a6aeecf4abc563bd77127778b6d214e3851037
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
Since strtol(3) doesn't clear errno on success, anything that sets
errno prior to this call will make it look like the call failed. This
happens when built with ASAN.
Signed-off-by: Dan Albert <danalbert@google.com>
Acked-by: Steve Lawrence <slawrence@tresys.com>
In C, defining a function with () means "any number of parameters", not
"no parameter". Use (void) instead where applicable and add unused
parameters when needed.
Acked-by: Steve Lawrence <slawrence@tresys.com>
Once __attribute__ ((format(printf, 1, 2))) is added to yyerror2,
"gcc -Wformat -Wformat-security" shows some issues. Fix them.
Acked-by: Steve Lawrence <slawrence@tresys.com>
This is a program for displaying the contents of a binary policy file.
Change-Id: Iba94d6b13ac1abbc084da5631dc2bf4107e548d1
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
checkmodule -m and -b are fundamentally incompatible with each other,
so reject attempts to use them together.
Resolves
https://bugzilla.redhat.com/show_bug.cgi?id=1064603
Also fix the error message for -m with -U to use stderr.
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
Allow mls/constraint mls/validatetrans constraints to be displayed
in debug mode. If POLICY_KERN version is >=
POLICYDB_VERSION_CONSTRAINT_NAMES then the policy defined
types/attributes will be returned.
Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
2.1.99 is just a placeholder to distinguish it from the prior release.
2.2 will be the released version. Switching to 2-component versions.
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
