When restorecond starts, it installs a SIGTERM handler in order to exit
cleanly (by removing its PID file). When restorecond --user starts,
there is no PID file, and g_main_loop_run() does not stop when master_fd
is closed. This leads to an unkillable service, which is an issue.
Fix this by overriding the handler for SIGTERM in restorecond --user.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
When running restorecond in user sessions using D-Bus activation,
restorecond's process is spawned in the CGroup of the D-Bus daemon:
$ systemctl --user status
[...]
CGroup: /user.slice/user-1000.slice/user@1000.service
├─init.scope
│ ├─1206 /usr/lib/systemd/systemd --user
│ └─1208 (sd-pam)
└─dbus.service
├─1628 /usr/bin/dbus-daemon --session --address=systemd:
└─4570 /usr/sbin/restorecond -u
In order to separate it, introduce a systemd unit for
restorecond-started-as-user.
After this patch:
CGroup: /user.slice/user-1000.slice/user@1000.service
├─restorecond-user.service
│ └─2871 /usr/sbin/restorecond -u
├─init.scope
│ ├─481 /usr/lib/systemd/systemd --user
│ └─485 (sd-pam)
└─dbus.service
└─2868 /usr/bin/dbus-daemon --session --address=systemd:
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=955940 states:
dbus-glib is a deprecated D-Bus library with some significant design
flaws, and is essentially unmaintained.
restorecond uses dbus-glib in order to spawn as a D-Bus service on the
session bus of users. This makes restorecond stays so long as the user
session exists.
Migrate from dbus-glib to GDbus API for the implementation of this
feature.
Moreover restorecond currently uses a D-Bus signal to trigger starting
the service. This is quite inappropriate, as stated for example in
https://dbus.freedesktop.org/doc/dbus-tutorial.html#members
Methods are operations that can be invoked on an object, with
optional input (aka arguments or "in parameters") and output (aka
return values or "out parameters"). Signals are broadcasts from the
object to any interested observers of the object; signals may
contain a data payload.
Implementing a method is more appropriate. It appears that all D-Bus
users can implement method Ping from interface org.freedesktop.DBus.Peer
(https://dbus.freedesktop.org/doc/dbus-specification.html#standard-interfaces-peer)
and that calling this method is enough to trigger the launch of the
service. This can be tested in a shell by running:
gdbus call --session --dest=org.selinux.Restorecond \
--object-path=/ --method=org.freedesktop.DBus.Peer.Ping
As this method is automatically provided, there is no need to implement
its handling in the service.
Fixed: https://github.com/SELinuxProject/selinux/issues/217
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
In libselinux, most functions set errno and return -1 when an error
occurs. But some functions return 1 instead, such as context_type_set(),
context_role_set(), etc. This increases the difficulty of writing Python
bindings of these functions without much benefit.
Return -1 instead (errno was already set).
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
The function comment of selinux_status_open() states:
It returns 0 on success, or -1 on error.
However the implementation of this function can also return 1. This is
documented in its manpage (libselinux/man/man3/selinux_status_open.3) as
intended. Copy the reason near the function definition in order to make
the code more auditable.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
The Python bindings for libselinux expose functions such as
avc_has_perm(), get_ordered_context_list(), etc. When these functions
encounter an error, they set errno accordingly and return a negative
value. In order to get the value of errno from Python code, it needs to
be "forwarded" in a way. This is achieved by glue code in
selinuxswig_python_exception.i, which implement raising an OSError
exception from the value of errno.
selinuxswig_python_exception.i was only generating glue code from
functions declared in selinux.h and not in other headers. Add other
headers.
selinuxswig_python_exception.i is generated by "bash exception.sh". Mark
the fact that exception.sh is a Bash script by adding a shebang. This
makes "shellcheck" not warn about the Bash array which is used to list
header files.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Acked-by: William Roberts <william.c.roberts@intel.com>
Replace
python3 -c 'import imp;print([s for s,m,t in imp.get_suffixes() if t == imp.C_EXTENSION][0])'
<string>:1: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
.cpython-38-x86_64-linux-gnu.so
with
python3 -c 'import importlib.machinery;print(importlib.machinery.EXTENSION_SUFFIXES[0])'
.cpython-38-x86_64-linux-gnu.so
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
This routine was never defined, just declared as a prototype.
Thus it never really existed, but remained in the map file.
Remove it.
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Signed-off-by: William Roberts <william.c.roberts@intel.com>
In previous work to cleanup the exports and linker scripts, I introduced
a regression causing symbols to be named in both the 1.0 and 1.1
sections. This went un-noticed and was reported by
nicolas.iooss@m4x.org.
Previous patches checked for correctness by:
This was checked by generating an old export map (from master):
nm --defined-only -g ./src/libsemanage.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/libsemanage.so | cut -d' ' -f 3-3 | grep -v '^_' > new.map
And diffing them:
diff old.map new.map
However, this discards the version information. Nicolas points out a
better way, by using objdump so we can see the version information. A
better sequence of commands for checking is as follows:
git checkout 1967477913
objdump -T ./src/libsemanage.so | grep LIBSEMANAGE | cut -d' ' -f 8- | sed 's/^ //' > map.old
git checkout origin/master
objdump -T ./src/libsemanage.so | grep LIBSEMANAGE | cut -d' ' -f 8- | sed 's/^ //' > map.new
diff map.old map.new
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Signed-off-by: William Roberts <william.c.roberts@intel.com>
Currently, the src/Makefile provides the FTS_LDLIBS when building against musl
or uClibc. However, this is missing from utils/Makefile, which causes linking
to fail.
Add the FTS_LDLIBS variable to the LDLIBS variable in utils/Makefile to fix
compiling against uClibc and musl.
Signed-off-by: Adam Duskett <Aduskett@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>
Follow-up of: 9eb9c93275 ("Get rid of security_context_t and fix const declarations.")
Acked-by: William Roberts <william.c.roberts@intel.com>
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
policy_scan.l:294:3: warning: implicit declaration of function 'yyerror' is
invalid in C99 [-Wimplicit-function-declaration]
{ yyerror("unrecognized character");}
^
policy_scan.l:294:3: warning: this function declaration is not a prototype
[-Wstrict-prototypes]
Acked-by: William Roberts <william.c.roberts@intel.com>
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
The linker map file had inconsistent style in the 1_1 versions.
Drop the mixed tabs and spaces and use the consistent spacing indent
of two spaces.
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Signed-off-by: William Roberts <william.c.roberts@intel.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/libsemanage.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/libsemanage.so | cut -d' ' -f 3-3 | grep -v '^_' > new.map
And diffing them:
diff old.map new.map
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
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 <stephen.smalley.work@gmail.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>
Add a simple test for secilc -O to make sure that it produces the
expected output. This might produce some false positives when the output
of secilc/checkpolicy changes slightly, in which case the expected CIL
will need to be updated along with the change.
The test should normally work even with a checkpolicy built from an
older tree, as long as it produces the same CIL output, so it uses the
checkpolicy it finds in PATH by default.
The test policy is taken from an e-mail from James Carter:
https://lore.kernel.org/selinux/CAP+JOzTQQx6aM81QyVe0yoiPJeDU+7xE6nn=0UMAB1EZ_c9ryA@mail.gmail.com/T/
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Acked-by: James Carter <jwcart2@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>
When the lexer encounters an unexpected character in a policy source file, it prints a warning, discards the character and moves on. In some build environments, these characters could be a symptom of an earlier problem, such as unintended results of expansion of preprocessor macros, and the ability to have the compiler halt on such issues would be helpful for diagnosis.
Signed-off-by: Daniel Burgener <Daniel.Burgener@microsoft.com>
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
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>
Previously the behavior was to warn, discard the character and proceed.
Now the build will halt upon encountering an unexpected character.
Signed-off-by: Daniel Burgener <dburgener@linux.microsoft.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>
Previously, libselinux was exporting the following symbols:
- dir_xattr_list;
- map_class;
- map_decision;
- map_perm;
- myprintf_compat;
- unmap_class;
- unmap_perm;
However, these appear to be unused and can safely be dropped.
This is done as a seperate commit to so it can easily be reverted
seperately for any reasons.
Signed-off-by: William Roberts <william.c.roberts@intel.com>
Add a linker script that exports only what was previosly exported by
libselinux.
This was checked by generating an old export map (from master):
nm --defined-only -g ./src/libselinux.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/libselinux.so | cut -d' ' -f 3-3 | grep -v '^_' > new.map
And diffing them:
diff old.map new.map
Fixes: #179
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 laoding first
in the library list.
Clang has this enabled by default.
Signed-off-by: William Roberts <william.c.roberts@intel.com>
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>
commit 1f89c4e787 ("libselinux: Eliminate
use of security_compute_user()") eliminated the use of
security_compute_user() by get_ordered_context_list(). Deprecate
all use of security_compute_user() by updating the headers and man
pages and logging a warning message on any calls to it. Remove
the example utility that called the interface. While here, also
fix the documentation of correct usage of the user argument to these
interfaces.
Fixes: https://github.com/SELinuxProject/selinux/issues/70
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
Acked-by: Petr Lautrbach <plautrba@redhat.com>
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>
From failsafe_context(5):
"The failsafe_context file allows SELinux-aware applications such as
PAM(8) to obtain a known valid login context for an administrator if
no valid default entries can be found elsewhere."
"Надёжный" means "reliable", "резервный" means "reserve",
the last variant is much closer to what "failsafe" really does.
Discussed with and approved by previous translators:
https://github.com/SELinuxProject/selinux/pull/203
Signed-off-by: Mikhail Novosyolov <m.novosyolov@rosalinux.ru>
Acked-by: Petr Lautrbach <plautrba@redhat.com>
get_ordered_context_list() code used to ask the kernel to compute the complete
set of reachable contexts using /sys/fs/selinux/user aka
security_compute_user(). This set can be so huge so that it doesn't fit into a
kernel page and security_compute_user() fails. Even if it doesn't fail,
get_ordered_context_list() throws away the vast majority of the returned
contexts because they don't match anything in
/etc/selinux/targeted/contexts/default_contexts or
/etc/selinux/targeted/contexts/users/
get_ordered_context_list() is rewritten to compute set of contexts based on
/etc/selinux/targeted/contexts/users/ and
/etc/selinux/targeted/contexts/default_contexts files and to return only valid
contexts, using security_check_context(), from this set.
Fixes: https://github.com/SELinuxProject/selinux/issues/28
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
