When writing a conf file or CIL policy out from a kernel binary,
do not write out a constraint rule if it has no permissions.
Signed-off-by: James Carter <jwcart2@gmail.com>
Since CIL allows permission expressions, it is possible for the
expression to evaluate to no permissions. If this is the case,
then don't add the constraint.
Signed-off-by: James Carter <jwcart2@gmail.com>
Shorten "CAPABILITY" to "CAP" following the kernel naming convention.
The SELinux policy capability enum names should now follow the
"POLICYDB_CAP_XXX" format.
Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
Acked-by: James Carter <jwcart2@gmail.com>
Extended permission and neverallow rules are not permitted in
conditional statements.
This causes issues on policy optimization where avtab_search() might
return a non extended permission rule when searching for one.
Found by oss-fuzz (#45327)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
When validating a policydb, do a more thorough validation of the
constraints.
- No permissions if it is a (mls)validatetrans.
- Only mlsvalidatetrans can use u3, r3, and t3.
- Expressions not involving types should have an empty type set.
- Only "==" and "!=" are allowed when there are names.
- If names are not used in an expression then both the names bitmap
and the type set should be empty.
- Only roles and mls expressions can used "dom", "domby", and "incomp".
- An mls expression cannot use names.
- If the expression is "not", "and", or "or", then the names bitmap
and the type set should be empty.
Signed-off-by: James Carter <jwcart2@gmail.com>
In f0a5f6e, calls to reallocarray were introduced. Ensure that the
correct header (private.h) is included when necessary.
Fixes: f0a5f6e ("libsepol: use reallocarray wrapper to avoid overflows")
Signed-off-by: Thiébaud Weksteen <tweek@google.com>
Acked-by: James Carter <jwcart2@gmail.com>
Test: Built using Android CI (glibc 2.17)
Commit 93902fc834 ("setfiles/restorecon: support parallel relabeling")
implemented support for parallel relabeling in setfiles. This is
available for fixfiles now.
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
Acked-by: Ondrej Mosnacek <omosnace@redhat.com>
Compiler can optimize calls to memset(3), due to the as-if rule, away if
the object is not accessed later on. Use a wrapper using volatile
pointers to ensure the memory is guaranteed to be erased. Also erase
the encrypted password.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Depending on the implementation crypt(3) can fail either by returning
NULL, or returning a pointer to an invalid hash and setting errno.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
newrole.c:636:12: warning: function declaration isn’t a prototype [-Wstrict-prototypes]
636 | static int transition_to_caller_uid()
| ^~~~~~~~~~~~~~~~~~~~~~~~
newrole.c:103:9: warning: macro is not used [-Wunused-macros]
#define DEFAULT_CONTEXT_SIZE 255 /* first guess at context size */
^
newrole.c:862:4: warning: 'break' will never be executed [-Wunreachable-code-break]
break;
^~~~~
newrole.c:168:13: warning: no previous extern declaration for non-static variable 'service_name' [-Wmissing-variable-declarations]
const char *service_name = "newrole";
^
hashtab.c:53:11: warning: implicit conversion changes signedness: 'unsigned int' to 'int' [-Wsign-conversion]
hvalue = h->hash_value(h, key);
~ ^~~~~~~~~~~~~~~~~~~~~
hashtab.c:92:11: warning: implicit conversion changes signedness: 'unsigned int' to 'int' [-Wsign-conversion]
hvalue = h->hash_value(h, key);
~ ^~~~~~~~~~~~~~~~~~~~~
hashtab.c:124:11: warning: implicit conversion changes signedness: 'unsigned int' to 'int' [-Wsign-conversion]
hvalue = h->hash_value(h, key);
~ ^~~~~~~~~~~~~~~~~~~~~
hashtab.c:172:10: warning: implicit conversion changes signedness: 'int' to 'unsigned int' [-Wsign-conversion]
ret = apply(cur->key, cur->datum, args);
~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
hashtab.c:174:12: warning: implicit conversion changes signedness: 'unsigned int' to 'int' [-Wsign-conversion]
return ret;
~~~~~~ ^~~
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
On the first loop iteration the variables `r_buf` and `reason_buf_used`
are NULL respective 0. Please UBSAN by not adding them but instead
directly assign NULL.
services.c:800:16: runtime error: applying zero offset to null pointer
#0 0x4d4fce in constraint_expr_eval_reason ./libsepol/src/services.c:800:16
#1 0x4cf31a in sepol_validate_transition_reason_buffer ./libsepol/src/services.c:1079:8
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
"semanage login -a" accepts whitespaces in user/group name
(e.g. users/groups from Active Directory), which may lead to issues down
the line since libsemanage doesn't expect whitespaces in
/var/lib/selinux/targeted/active/seusers and other config files.
Fixes:
Artificial but simple reproducer
# groupadd server_admins
# sed -i "s/^server_admins/server admins/" /etc/group
# semanage login -a -s staff_u %server\ admins
# semanage login -l (or "semodule -B")
libsemanage.parse_assert_ch: expected character ':', but found 'a' (/var/lib/selinux/targeted/active/seusers: 6):
%server admins:staff_u:s0-s0:c0.c1023 (No such file or directory).
libsemanage.seuser_parse: could not parse seuser record (No such file or directory).
libsemanage.dbase_file_cache: could not cache file database (No such file or directory).
libsemanage.enter_ro: could not enter read-only section (No such file or directory).
FileNotFoundError: [Errno 2] No such file or directory
Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
Spaces before values in /etc/selinux/config should be ignored just as
spaces after them are.
E.g. "SELINUXTYPE= targeted" should be a valid value.
Fixes:
# sed -i 's/^SELINUXTYPE=/SELINUXTYPE= /g' /etc/selinux/config
# dnf install <any_package>
...
RPM: error: selabel_open: (/etc/selinux/ targeted/contexts/files/file_contexts) No such file or directory
RPM: error: Plugin selinux: hook tsm_pre failed
...
Error: Could not run transaction.
Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
When parsing Reference Policy style files accept square brackets in file
names. The FILENAME token is used in the TYPE_TRANSITION grammar rule
for the optional name based argument. This name can contain square
brackets, e.g. for anonymous inodes like "[io_uring]".
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
When an assertion fails, the error message refers to a generic
"policy.conf" file. When parsing a policy in checkpolicy, populate its
name using the original filename (source_filename is still build using
the #line directives within the policy).
Signed-off-by: Thiébaud Weksteen <tweek@google.com>
[Merge conflicts fixed by: James Carter <jwcart2@gmail.com>]
Signed-off-by: James Carter <jwcart2@gmail.com>
When there are conflicting context rules, the location of the
conflicting rules are written out. If there are many duplicates of
the same context rule, there will be many pairs of conflicts written
out. This hides the fact that all of the rules are the same and can
make it hard to see the different conflicts.
First, since these are warnings and not reported at the default log
verbosity level (which only reports errors), only search for the
locations of the conflicting rules when the verbosity level means
that the warnings will actually be reported.
Second, Report all the duplicate conflicting rules together.
Third, Report the first four conflicts of the same rule if when
the verbosity level is at CIL_WARN ("-v") and report all of them
when the verbosity level is at CIL_INFO or higher ("-v -v").
Fixes problem found by oss-fuzz (#39735)
Signed-off-by: James Carter <jwcart2@gmail.com>
When there is a neverallow violation, a search is made for all of
the rules that violate the neverallow. The violating rules as well
as their parents are written out to make it easier to find these
rules.
If there is a lot of rules that violate a neverallow, then this
amount of reporting is too much. Instead, only print out the first
four rules (with their parents) that match the violated neverallow
rule along with the total number of rules that violate the
neverallow at the default log level. Report all the violations when
at a higher verbosity level.
Signed-off-by: James Carter <jwcart2@gmail.com>
Commit 4b2e2a248e (libsepol/cil: Limit
the amount of reporting for bounds failures) limited the number of
bounds failures that were reported to the first two matching rules
for the first two bad rules.
Instead, report the first two matching rules for the first four bad
rules at the default log level and report all matching rules for all
bad rules for higher verbosity levels.
Signed-off-by: James Carter <jwcart2@gmail.com>
Not all violations of neverallowxperm rules were being reported.
In check_assertion_extended_permissions_avtab(), a break was
performed after finding a match rather than just returning right
away. This means that if other src and tgt pairs were checked
afterward that did not match, then no match would be reported.
Example:
allow attr attr:CLASS ioctl;
allowxperm attr attr:CLASS ioctl 0x9401;
allowxperm t1 self:CLASS ioctl 0x9421;
neverallowxperm attr self:CLASS ioctl 0x9421;
Would result in no assertion violations being found.
Another problem was that the reporting function did not properly
recognize when there was a valid allowxperm rule and falsely
reported additional violations that did not exist. (There had
to be at least one legitimate violation.)
Using the same example as above (and assuming t1 and t2 both have
attribute attr), the following would be reported as:
neverallowxperm on line 4 of policy.conf (or line 4 of policy.conf)
violated by
allowxperm t1 t1:CLASS ioctl { 0x9421 };
neverallowxperm on line 4 of policy.conf (or line 4 of policy.conf)
violated by
allow t2 t2:CLASS4 { ioctl };
There is no violation for t2 because there is a valid allowxperm
rule for it.
With this patch, only the first error message (which is the correct
one) is printed.
Signed-off-by: James Carter <jwcart2@gmail.com>
The changes are the same as in a patch sent by Christian Göttsche
<cgzones@googlemail.com> to support adding not-self to neverallowxperm
checking, but it is needed for normal neverallowxperm checking as well
and the following explanation reflects that.
When reporting neverallowxperm violations, the avtab is searched to
find the rule that violates the assertion. If the avtab pointer of
the args is not set, then it will report the error as if no extended
permissions existed for the source and target (so allowing the ioctl
permission at all violates the neverallowxperm).
Example (where t1 has attribute attr):
allow attr attr:CLASS ioctl;
allowxperm attr attr:CLASS ioctl 0x9411;
neverallowxperm t1 self:CLASS ioctl 0x9411;
Would be reported as:
neverallowxperm on line 3 of policy.conf (or line 3 of policy.conf)
violated by
allow t1 t1:CLASS { ioctl };
Instead of:
neverallowxperm on line 3 of policy.conf (or line 3 of policy.conf)
violated by
allowxperm attr attr:CLASS ioctl { 0x9411 };
Reported-by: Christian Göttsche <cgzones@googlemail.com>
Signed-off-by: James Carter <jwcart2@gmail.com>
When checking for violations of neverallow rules, if the neverallow
uses self, then the src and tgt must be the same when checking
extended permissions and when reporting violations.
Example:
allow attr attr : CLASS PERM;
neverallow attr self : CLASS PERM;
If the types t1 and t2 have attribute attr, then the violations
that would be reported would be:
allow t1 t1 : CLASS PERM;
allow t1 t2 : CLASS PERM;
allow t2 t1 : CLASS PERM;
allow t2 t2 : CLASS PERM;
instead of:
allow t1 t1 : CLASS PERM;
allow t2 t2 : CLASS PERM;
Signed-off-by: James Carter <jwcart2@gmail.com>
The value returned from report_assertion_extended_permissions() is
the nubmer of errors, so call it that instead of ret.
Signed-off-by: James Carter <jwcart2@gmail.com>
In both check_assertion_extended_permissions() and
report_assertion_avtab_matches(), when checking for a match involving
a rule using self, the matches between the source and target of the
rule being checked are found using ebitmap_and() and then the matches
between that result and the source of the neverallow are found using
another ebitmap_and() call.
Since the matches between the sources of the rule being checked and
the neverallow have already been found, just find the matches between
that result and the target of the rule being checked. This only
requires one call to ebitmap_and() instead of two.
Signed-off-by: James Carter <jwcart2@gmail.com>
When check_assertion_extended_permissions() is called, it has already
been determined that there is a match, and, since neither the class
nor the permissions are used, there is no need for the check.
Signed-off-by: James Carter <jwcart2@gmail.com>
Inorder to differentiate errors from matches, use "(rc < 0)" when
calling ebitmap_* functions while checking neverallow rules.
Also, just use rc instead of having a separate variable (ret) in
check_assertion_extended_permissions().
Signed-off-by: James Carter <jwcart2@gmail.com>
If a neverallow has target types as well as using self and a match
is found with the target types, then self does not even need to
be checked, since the rule is already in violation of the assertion.
So move the check for a match of the target types before dealing with
self.
Signed-off-by: James Carter <jwcart2@gmail.com>
In check_assertion_avtab_match(), for the functions that do not return
an error, but only returns 0 or 1 depending on if a match is found,
call the function in an if statement.
Signed-off-by: James Carter <jwcart2@gmail.com>
Return an error if check_assertion_extended_permissions() returns
an error instead of treating it as an assertion violation.
Signed-off-by: James Carter <jwcart2@gmail.com>
An out of memory condition is unlikely and the general message
that an error occured while checking neverallows is sufficient.
Signed-off-by: James Carter <jwcart2@gmail.com>
Instead of calling report_assertion_failures() and treating an
error like it was a neverallow violation, just return an error.
Signed-off-by: James Carter <jwcart2@gmail.com>
Allow all and complement permission sets in constraints, e.g.:
constrain service ~ { status } (...);
constrain service * (...);
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Check the type for type and role sets is valid.
Check the scope of a scope datum is valid.
Check the flavor and flags of a type datum are valid.
Check xperms are set if and only if it is an extended permission avrule.
Check xperms has a valid specified field.
Check the flag of avrule blocks is valid.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Check the permission bitset in normal constraints is not empty and has
no invalid bits set.
Check the names and type_names members are empty in case they are not
used.
Check the operator and attribute type are not set for simple expression
types.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
The callback function apply in hashtap_map has a return type of int and
can return -1 on error. Use int as type to save the return value to
avoid implicit conversions:
hashtab.c:236:10: runtime error: implicit conversion from type 'int' of value -1 (32-bit, signed) to type 'unsigned int' changed the value to 4294967295 (32-bit, unsigned)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Add a new command-line option "--rebuild-if-modules-changed" to control
the newly introduced check_ext_changes libsemanage flag.
For example, running `semodule --rebuild-if-modules-changed` will ensure
that any externally added/removed modules (e.g. by an RPM transaction)
are reflected in the compiled policy, while skipping the most expensive
part of the rebuild if no module change was deteceted since the last
libsemanage transaction.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
In Fedora/RHEL's selinux-policy package we ship a pre-built SELinux
policy store in the RPMs. When updating the main policy RPM, care must
be taken to rebuild the policy using `semodule -B` if there are any
other SELinux modules installed (whether shipped via another RPM or
manually installed locally).
However, this way of shipping/managing the policy creates complications
on systems, where system files are managed by rpm-ostree (such as Fedora
CoreOS or Red Hat CoreOS), where the "package update" process is more
sophisticated.
(Disclaimer: The following is written according to my current limited
understanding of rpm-ostree and may not be entirely accurate, but the
gist of it should match the reality.)
Basically, one can think of rpm-ostree as a kind of Git for system
files. The package content is provided on a "branch", where each
"commit" represents a set of package updates layered on top of the
previous commit (i.e. it is a rolling release with some defined
package content snapshots). The user can then maintain their own branch
with additional package updates/installations/... and "rebase" it on top
of the main branch as needed. On top of that, the user can also have
additional configuration files (or modifications to existing files) in
/etc, which represent an additional layer on top of the package content.
When updating the system (i.e. rebasing on a new "commit" of the "main
branch"), the files on the running system are not touched and the new
system state is prepared under a new root directory, which is chrooted
into on the next reboot.
When an rpm-ostree system is updated, there are three moments when the
SELinux module store needs to be rebuilt to ensure that all modules are
included in the binary policy:
1. When the local RPM installations are applied on top of the base
system snapshot.
2. When local user configuartion is applied on top of that.
3. On system shutdown, to ensure that any changes in local configuration
performed since (2.) are reflected in the final new system image.
Forcing a full rebuild at each step is not optimal and in many cases is
not necessary, as the user may not have any custom modules installed.
Thus, this patch extends libsemanage to compute a checksum of the
content of all enabled modules, which is stored in the store, and adds a
flag to the libsemanage handle that instructs it to check the module
content checksum against the one from the last successful transaction
and force a full policy rebuild if they don't match.
This will allow rpm-ostree systems to potentially reduce delays when
reconciling the module store when applying updates.
I wasn't able to measure any noticeable overhead of the hash
computation, which is now added for every transaction (both before and
after this change a full policy rebuild took about 7 seconds on my test
x86 VM). With the new option check_ext_changes enabled, rebuilding a
policy store with unchanged modules took only about 0.96 seconds.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
In order to reduce exisiting and future code duplication and to avoid
some unnecessary allocations and copying, factor the compressed file
utility functions out into a separate C/header file and refactor their
interface.
Note that this change effectively removes the __fsetlocking(3) call from
semanage_load_files() - I haven't been able to figure out what purpose
it serves, but it seems pointless...
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
The main goal of this move is to have the SHA-256 implementation under
libsemanage, since upcoming patches will make use of SHA-256 for a
different (but similar) purpose in libsemanage. Having the hashing code
in libsemanage will reduce code duplication and allow for easier hash
algorithm upgrade in the future.
Note that libselinux currently also contains a hash function
implementation (for yet another different purpose). This patch doesn't
make any effort to address that duplicity yet.
This patch also changes the format of the hash string printed by
semodule to include the name of the hash. The intent is to avoid
ambiguity and potential collisions when the algorithm is potentially
changed in the future.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Map classes use the same struct as kernel classes, but only the kernel
class uses the pointer to a common class. When resolving a classcommon,
make sure that the class that is found is a kernel class and not a
map class. If not, then return an error.
Found by oss-fuzz (#43209)
Signed-off-by: James Carter <jwcart2@gmail.com>
