Users are allowed to be declared in modules. Modules do not get expanded
leaving the `struct user_datum` members `exp_range` and `exp_dfltlevel`
empty.
Do no validate the expanded range and level for modular polices.
Reported-by: bauen1 <j2468h@gmail.com>
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Fixes:
cil/src/cil_build_ast.c:4622:4: warning[deadcode.DeadStores]: Value stored to 'rc' is never read
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
Acked-by: James Carter <jwcart2@gmail.com>
sepolgen-ifgen-attr-helper.c: In function ‘load_policy’:
sepolgen-ifgen-attr-helper.c:196:17: warning: leak of FILE ‘fp’ [CWE-775] [-Wanalyzer-file-leak]
196 | fprintf(stderr, "Out of memory!\n");
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
security_load_policy(3) takes a read-only memory address for a binary
policy to be loaded.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
The following interfaces are documented but do not have a redirection:
- context_str(3)
- security_get_checkreqprot(3)
- security_set_boolean_list(3)
- selinux_sepgsql_context_path(3)
- setexecfilecon(3)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
To copy string safely, by always NULL-terminating them, and provide an
easy way to check for truncation introduce the nonstandard function
strlcpy(3). Use the system implementation if available.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
In case the function __policy_init() gets called with a NULL pointer,
the stack variable path remains uninitialized (except at its last
index). If parsing the binary policy fails in sepol_policydb_read() the
error branch would access those uninitialized memory.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
First transaction applies all deletion operations, so that there are no
collisions when applying the rest of the changes.
Fixes:
# semanage port -a -t http_cache_port_t -r s0 -p tcp 3024
# semanage export | semanage import
ValueError: Port tcp/3024 already defined
Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
libselinux implements a cache mechanism for get*con() functions, such
that when a thread calls setcon(...) then getcon(...), the context is
directly returned. Unfortunately, getpidcon(pid, &context) uses the same
cached variable, so when a program uses setcon("something"), all later
calls to getpidcon(pid, ...) returns "something". This is a bug.
Here is a program which illustrates this bug:
#include <stdio.h>
#include <selinux/selinux.h>
int main() {
char *context = "";
if (getpidcon(1, &context) < 0) {
perror("getpidcon(1)");
}
printf("getpidcon(1) = %s\n", context);
if (getcon(&context) < 0) {
perror("getcon()");
}
printf("getcon() = %s\n", context);
if (setcon(context) < 0) {
perror("setcon()");
}
if (getpidcon(1, &context) < 0) {
perror("getpidcon(1)");
}
printf("getpidcon(1) = %s\n", context);
return 0;
}
On an Arch Linux system using unconfined user, this program displays:
getpidcon(1) = system_u:system_r:init_t
getcon() = unconfined_u:unconfined_r:unconfined_t
getpidcon(1) = unconfined_u:unconfined_r:unconfined_t
With this commit, this program displays:
getpidcon(1) = system_u:system_r:init_t
getcon() = unconfined_u:unconfined_r:unconfined_t
getpidcon(1) = system_u:system_r:init_t
This bug was present in the first commit of
https://github.com/SELinuxProject/selinux git history. It was reported
in https://lore.kernel.org/selinux/20220121084012.GS7643@suse.com/ and a
patch to fix it was sent in
https://patchwork.kernel.org/project/selinux/patch/20220127130741.31940-1-jsegitz@suse.de/
without a clear explanation. This patch added pid checks, which made
sense but were difficult to read. Instead, it is possible to change the
way the functions are called so that they directly know which cache
variable to use.
Moreover, as the code is not clear at all (I spent too much time trying
to understand what the switch did and what the thread-local variable
contained), this commit also reworks libselinux/src/procattr.c to:
- not use hard-to-understand switch/case constructions on strings (they
are replaced by a new argument filled by macros)
- remove getpidattr_def macro (it was only used once, for pidcon, and
the code is clearer with one less macro)
- remove the pid parameter of setprocattrcon() and setprocattrcon_raw()
(it is always zero)
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Cc: Johannes Segitz <jsegitz@suse.de>
This reverts commit 7e979b56fd.
The reverted commit broke `setfiles` when it's run from a chroot
without /proc mounted, e.g.
# chroot /mnt/sysimage
chroot# setfiles -e /proc -e /sys /sys /etc/selinux/targeted/contexts/files/file_contexts /
[strace]
openat(AT_FDCWD, "/", O_RDONLY|O_EXCL|O_NOFOLLOW|O_PATH) = 3
newfstatat(3, "", {st_mode=S_IFDIR|0555, st_size=4096, ...}, AT_EMPTY_PATH) = 0
mmap(NULL, 2101248, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f1697c91000
fgetxattr(3, "security.selinux", 0x55be8881d3f0, 255) = -1 EBADF (Bad file descriptor)
fcntl(3, F_GETFL) = 0x220000 (flags O_RDONLY|O_NOFOLLOW|O_PATH)
getxattr("/proc/self/fd/3", "security.selinux", 0x55be8881d3f0, 255) = -1 ENOENT (No such file or directory)
[/strace]
setfiles: Could not set context for /: No such file or directory
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
Acked-by: James Carter <jwcart2@gmail.com>
Do not check for file existence and open afterwards, open with the
exclusive flag (supported in Glibc and musl 0.9.6 and also standardized
in C11).
Found by GitHub CodeQL.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
context_str(3) returns a string representation of the given context.
This string is owned by the context and free'd on context_free(3).
Declare it const, as already done in the man page, since it must not be
free'd by the caller.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Since version 2.35.2, due to CVE-2022-24765, git refuses to operate by
default on a repository owned by a different user.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: Petr Lautrbach <plautrba@redhat.com>
The family of setfilecon(3) functions take the context as a read-only
`const char *` parameter.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
With the addition of the anon_inode class in the kernel, 'self'
transition rules became useful, but haven't been implemented.
The typetransition, typemember, and typechange statements share the
relevant code, so this patch implements the self keyword in all of them
at the TE language level and adds the support to the module policydb
format. Note that changing the kernel policydb format is not necessary
at all, as type transitions are always expanded in the kernel policydb.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Acked-by: James Carter <jwcart2@gmail.com>
With the addition of the anon_inode class in the kernel, 'self'
transition rules became useful, but haven't been implemented.
The typetransition, typemember, and typechange statements share the
relevant code, so this patch implements the self keyword in all of them
at the CIL level. It also adds basic coverage for the such 'self' rules
to the secilc test policy.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Acked-by: James Carter <jwcart2@gmail.com>
Print error description on failure after functions known to set errno.
Also mention the library function name in getenforce, policyvers and
setenforce instead of the program name twice.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Some calls to "_" where unsuccessful because the function was
initialized with a different translation domain than the string.
e.g. selinux-polgengui calls functions from sepolicy.generate, which end
up printing untranslated strings because polgengui uses selinux-gui
domain while sepolicy uses selinux-python
- Set "_" in module namespace instead of "builtins"
- Set the whole "sepolicy.generate()" confirmation as translatable
- Drop "codeset" parameter since it is deprecated
Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
Acked-by: James Carter <jwcart2@gmail.com>
selinux_log() is used in many error branches, where the caller might
expect errno to bet set, e.g. label_file.c::lookup_all():
if (match_count) {
*match_count = 0;
result = calloc(data->nspec, sizeof(struct spec*));
} else {
result = calloc(1, sizeof(struct spec*));
}
if (!result) {
selinux_log(SELINUX_ERROR, "Failed to allocate %zu bytes of data\n",
data->nspec * sizeof(struct spec*));
goto finish;
}
Preserve errno in the macro wrapper itself, also preventing accidental
errno modifications in client specified SELINUX_CB_LOG callbacks.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
In case the allocation for the filename fails, free the memory of the context.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Pin the file to operate on in restorecon_sb() to prevent symlink attacks
in between the label database lookup, the current context query and the
final context write. Also don't use the file information from
fts_read(3), which might also be out of sync.
Due to querying file information twice, one in fts_read(3) needed for
the cross device check and one on the pinned file descriptor for the
database lookup, there is a slight slowdown:
[current]
Time (mean ± σ): 14.456 s ± 0.306 s [User: 45.863 s, System: 4.463 s]
Range (min … max): 14.275 s … 15.294 s 10 runs
[changed]
Time (mean ± σ): 15.843 s ± 0.045 s [User: 46.274 s, System: 9.495 s]
Range (min … max): 15.787 s … 15.916 s 10 runs
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
selabel_lookup_raw(3) can fail for other reasons than no corresponding
context found, e.g. ENOMEM or EINVAL for invalid key or type.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
* mark read-only parameters const
* check for overflow when adding exclude directory
* use 64 bit integer for file counting
* avoid implicit conversions
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Operating on a file descriptor avoids TOCTOU issues and one opened via
O_PATH avoids the requirement of having read access to the file. Since
Linux does not natively support file descriptors opened via O_PATH in
fgetxattr(2) and at least glibc and musl does not emulate O_PATH support
in their implementations, fgetfilecon(3) and fsetfilecon(3) also do not
currently support file descriptors opened with O_PATH.
Inspired by CVE-2013-4392: https://github.com/systemd/systemd/pull/8583
Implementation adapted from: 2825f10b7f%5E%21/
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
When a permission for a constraint statement cannot be found also
mention the related class name.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
setfiles(8) exits with status 255 if it encounters any error. Introduce
the "-C" option: if the only errors that setfiles(8) encounters are
labeling errors seen during the file tree walk(s), then let setfiles(8)
exit with status 1.
Cc: "Richard W.M. Jones" <rjones@redhat.com>
Cc: Petr Lautrbach <plautrba@redhat.com>
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1794518
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Currently, if the SELINUX_RESTORECON_ABORT_ON_ERROR flag is clear, then
selinux_restorecon[_parallel]() does not abort the file tree walk upon an
error, but the function itself fails the same, with the same (-1) return
value. This in turn is reported by the setfiles(8) utility to its parent
process with the same exit code (255).
In libguestfs we want to proceed after setfiles(8) fails *at most* with
such errors that occur during the file tree walk. We need setfiles(8) to
exit with a distinct exit status in that situation.
For this, introduce the SELINUX_RESTORECON_COUNT_ERRORS flag, and the
corresponding selinux_restorecon_get_skipped_errors() function, for
selinux_restorecon[_parallel]() to count, but otherwise ignore, errors
during the file tree walk. When no other kind of error occurs, the
relabeling functions will return zero, and the caller can fetch the number
of errors ignored during the file tree walk with
selinux_restorecon_get_skipped_errors().
Importantly, when at least one such error is skipped, we don't write
partial match digests for subdirectories, as any masked error means that
any subdirectory may not have been completely relabeled.
Cc: "Richard W.M. Jones" <rjones@redhat.com>
Cc: Petr Lautrbach <plautrba@redhat.com>
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1794518
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Commit 219eea83ce ("policycoreutils: setfiles/restorecon: fix -r/-R
option", 2015-04-16) split the option strings between "setfiles" and
"restorecon". Since that commit, an "iamrestorecon" check has only been
necessary for an option that is (a) accepted by both "setfiles" and
"restorecon", but (b) behaves differently between "setfiles" and
"restorecon". Currently, the only such options are "-r" and "-R". Remove
the "iamrestorecon" checks from the "setfiles"-only "-c" and "-d" options,
and from the "restorecon"-only "-x" option.
Cc: "Richard W.M. Jones" <rjones@redhat.com>
Cc: Petr Lautrbach <plautrba@redhat.com>
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1794518
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-By: Daniel Burgener <dburgener@linux.microsoft.com>
Commit 9207823c8f ("setfiles: Do not abort on labeling error",
2021-02-01) hoisted the zeroing of "r_opts.abort_on_error" above the
branching on "setfiles vs. restorecon". Clean up two aspects:
- "r_opts" is altogether zeroed a bit higher up, so remove the explicit
zero-assignment;
- neither "setfiles" nor "restorecon" aborts on errors during the file
tree walk now, so remove the comment "Do not abort on errors during the
file tree walk" from the "restorecon" branch as well.
Cc: "Richard W.M. Jones" <rjones@redhat.com>
Cc: Petr Lautrbach <plautrba@redhat.com>
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1794518
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Commit 7ad84e7c8d ("Add restorecon -x option to not cross FS
boundaries", 2020-06-18) used spaces vs. TABs inconsistently; run
"unexpand" on the affected lines to make the indentation conform to the
rest of the source code.
Cc: "Richard W.M. Jones" <rjones@redhat.com>
Cc: Petr Lautrbach <plautrba@redhat.com>
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1794518
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
The internal Sha1Update() functions only handles buffers up to a size of
UINT32_MAX, due to its usage of the type uint32_t. This causes issues
when processing more than UINT32_MAX bytes, e.g. with a specfile larger
than 4G. 0aa974a4 ("libselinux: limit has buffer size") tried to
address this issue, but failed since the overflow check
if (digest->hashbuf_size + buf_len < digest->hashbuf_size) {
will be done in the widest common type, which is size_t, the type of
`buf_len`.
Revert the type of `hashbuf_size` to size_t and instead process the data
in blocks of supported size.
Acked-by: James Carter <jwcart2@gmail.com>
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Reverts: 0aa974a4 ("libselinux: limit has buffer size")
If selabel_open(3) fails, e.g. when a specfile has the wrong file
permissions, free the memory allocated for digests.
Fixes: e40bbea9 ("libselinux: Add selabel_digest function")
Acked-by: James Carter <jwcart2@gmail.com>
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
selabel_open(3) takes an `unsigned int` as backend parameter.
Acked-by: James Carter <jwcart2@gmail.com>
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Check for missing prototypes like file local functions not declared
static or external functions not being declared to avoid declaration/
definition desynchronizations.
Acked-by: James Carter <jwcart2@gmail.com>
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
The code generated by swig triggers the following warning:
semanageswig_wrap.c:2759:24: warning: no previous prototype for ‘PyInit__semanage’ [-Wmissing-prototypes]
2759 | # define SWIG_init PyInit__semanage
| ^~~~~~~~~~~~~~~~
semanageswig_wrap.c:17772:1: note: in expansion of macro ‘SWIG_init’
17772 | SWIG_init(void) {
| ^~~~~~~~~
Ignore -Wmissing-prototypes for swig generated source files.
Acked-by: James Carter <jwcart2@gmail.com>
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
The function __cil_verify_rule() is currently not used as all call sites
are commented out. Keep the function for future references.
Acked-by: James Carter <jwcart2@gmail.com>
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
checkpolicy(8) since 01b88ac3 ("checkpolicy: warn on bogus IP address or
netmask in nodecon statement") warns about host bits set in IPv6
addresses.
Adjust IPv6 netmasks in the libsepol tests so that the used address ::1
does not set any host bits and running the tests does not print several
of the following warnings:
net_contexts:15:WARNING 'host bits in ipv6 address set' at token '' on line 594:
Acked-by: James Carter <jwcart2@gmail.com>
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
The functions helper_port_validate_local_proto(), get_type(), and
get_fcontext_new() are not used, so remove them.
Signed-off-by: James Carter <jwcart2@gmail.com>
Since test-linker-cond-map.h and test-linker-types.h references
policydb_t, include the policydb header file.
Signed-off-by: James Carter <jwcart2@gmail.com>
