-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
If you fail to load_policy in the init or SELinux is disabled, you need
to free the selinux_mnt variable and clear the memory.
systemd was calling load_polcy on a DISABLED system then later on it
would call is_selinux_enabled() and get incorrect response, since
selinux_mnt still had valid data.
The second bug in libselinux, resolves around calling the
selinux_key_delete(destructor_key) if the selinux_key_create call had
never been called. This was causing data to be freed in other
applications that loaded an unloaded the libselinux library but never
setup setrans or matchpathcon.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAk2c0/UACgkQrlYvE4MpobMP1QCfXAFD3pfWFLd1lylU/vjsZmpM
mcUAnA2l3/GKGC3hT8XB9E+2pTfpy+uj
=jpyr
-----END PGP SIGNATURE-----
Signed-off-by: Steve Lawrence <slawrence@tresys.com>
The attached patch add support db_language object class
to the selabel_lookup(_raw) interfaces.
It is needed to inform object manager initial label of
procedural language object.
Thanks,
--
KaiGai Kohei <kaigai@ak.jp.nec.com>
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
This prevents the key destructors, intented to free per-thread
heap storage, from being called after libselinux has been unloaded.
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=680887
Signed-off-by: Eamon Walsh <ewalsh@tycho.nsa.gov>
The attached patch adds several interfaces to reference /selinux/status
according to sequential-lock logic.
selinux_status_open() open the kernel status page and mmap it with
read-only mode, or open netlink socket as a fallback in older kernels.
Then, we can obtain status information from the mmap'ed page using
selinux_status_updated(), selinux_status_getenfoce(),
selinux_status_policyload() or selinux_status_deny_unknown().
It enables to help to implement userspace avc with heavy access control
decision; that we cannot ignore the cost to communicate with kernel for
validation of userspace caches.
Signed-off-by: Steve Lawrence <slawrence@tresys.com>
Email: dwalsh@redhat.com
Subject: I think it is time to turn off default user handling in libselinux
Date: Mon, 13 Dec 2010 13:28:01 -0500
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
This patch will turn this handling off. Meaning you will not end up
with some bizarro context and fail to login if the login program can not
figure how to log you in.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAk0GZbEACgkQrlYvE4MpobOF7QCgsD1XYuNC6B5MyIezCZvN9mYL
UX4AoOe9GsP3bhuvMBPea9LXeV/7tCPS
=B9Pk
-----END PGP SIGNATURE-----
Signed-off-by: Chad Sellers <csellers@tresys.com>
Description of problem:
Use of __thread variables is great for creating a thread-safe variable, but
only insofar as the contents of that variable can safely be abandoned on
pthread_exit(). The moment you store malloc()d data into a __thread void*
variable, you have leaked memory when the thread exits, since there is no way
to associate a destructor with __thread variables.
The _only_ safe way to use thread-local caching of malloc()d data is to use
pthread_key_create, and associate a destructor that will call free() on the
resulting data when the thread exits.
libselinux is guilty of abusing __thread variables to store malloc()d data as a
form of a cache, to minimize computation by reusing earlier results from the
same thread. As a result of this memory leak, repeated starting and stopping
of domains via libvirt can result in the OOM killer triggering, since libvirt
fires up a thread per domain, and each thread uses selinux calls such as
fgetfilecon.
Version-Release number of selected component (if applicable):
libselinux-2.0.94-2.el6.x86_64
libvirt-0.8.1-27.el6.x86_64
How reproducible:
100%
Steps to Reproduce:
0. These steps are run as root, assuming hardware kvm support and existence of
a VM named fedora (adjust the steps below as appropriate); if desired, I can
reduce this to a simpler test case that does not rely on libvirt, by using a
single .c file that links against libselinux and repeatedly spawns threads.
1. service libvirtd stop
2. valgrind --quiet --leak-check=full /usr/sbin/libvirtd& pid=$!
3. virsh start fedora
4. kill $pid
Actual results:
The biggest leak reported is due to libselinux' abuse of __thread:
==26696== 829,730 (40 direct, 829,690 indirect) bytes in 1 blocks are
definitely lost in loss record 500 of 500
==26696== at 0x4A0515D: malloc (vg_replace_malloc.c:195)
==26696== by 0x3022E0D48C: selabel_open (label.c:165)
==26696== by 0x3022E11646: matchpathcon_init_prefix (matchpathcon.c:296)
==26696== by 0x3022E1190D: matchpathcon (matchpathcon.c:317)
==26696== by 0x3033ED7FB5: SELinuxRestoreSecurityFileLabel (security_selinux.c:381)
==26696== by 0x3033ED8539: SELinuxRestoreSecurityAllLabel (security_selinux.c:749)
==26696== by 0x459153: qemuSecurityStackedRestoreSecurityAllLabel (qemu_security_stacked.c:257)
==26696== by 0x43F0C5: qemudShutdownVMDaemon (qemu_driver.c:4311)
==26696== by 0x4555C9: qemudStartVMDaemon (qemu_driver.c:4234)
==26696== by 0x458416: qemudDomainObjStart (qemu_driver.c:7268)
==26696== by 0x45896F: qemudDomainStart (qemu_driver.c:7308)
==26696== by 0x3033E75412: virDomainCreate (libvirt.c:4881)
==26696==
Basically, libvirt created a thread that used matchpathcon during 'virsh start
fedora', and matchpathcon stuffed over 800k of malloc'd data into:
static __thread char **con_array;
which are then inaccessible when libvirt exits the thread as part of shutting
down on SIGTERM.
Expected results:
valgrind should not report any memory leaks related to libselinux.
Signed-off-by: Eamon Walsh <ewalsh@tycho.nsa.gov>
Reported-by: Eric Blake <eblake@redhat.com>
Tested-by: Eric Blake <eblake@redhat.com>
Email: kaigai@ak.jp.nec.com
Subject: libselinux APIs should take "const" qualifier?
Date: Tue, 23 Mar 2010 11:56:36 +0900
(2010/03/19 22:32), Stephen Smalley wrote:
> On Fri, 2010-03-19 at 16:52 +0900, KaiGai Kohei wrote:
>> Right now, security_context_t is an alias of char *, declared in selinux.h.
>>
>> Various kind of libselinux API takes security_context_t arguments,
>> however, it is inconvenience in several situations.
>>
>> For example, the following query is parsed, then delivered to access
>> control subsystem with the security context as "const char *" cstring.
>>
>> ALTER TABLE my_tbl SECURITY LABEL TO 'system_u:object_r:sepgsql_table_t:SystemHigh';
>> const char *<---- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>
>> In this case, we want to call selinux_trans_to_raw_context() to translate
>> the given security context into raw format. But it takes security_context_t
>> argument for the source context, although this pointer is read-only.
>> In the result, compiler raises warnings because we gave "const char *" pointer
>> into functions which take security_context_t (= char *).
>>
>> Any comments?
>>
>> It seems to me the following functions' prototype should be qualified by
>> "const".
>
> That seems reasonable and should have no impact on library ABI.
> On the other hand, others have pointed out that security_context_t is
> not a properly encapsulated data type at all, and perhaps should be
> deprecated and replaced with direct use of char*/const char* throughout.
>
> There are other library API issues as well that have come up in the
> past, such as lack of adequate namespacing (with approaches put forth),
> but we don't ever seem to get a round tuit.
At first, I tried to add const qualifiers read-only security_context_t
pointers, but didn't replace them by char */const char * yet, right now.
BTW, I could find out the following code:
int security_compute_create(security_context_t scon,
security_context_t tcon,
security_class_t tclass,
security_context_t * newcon)
{
int ret;
security_context_t rscon = scon;
security_context_t rtcon = tcon;
security_context_t rnewcon;
if (selinux_trans_to_raw_context(scon, &rscon))
return -1;
if (selinux_trans_to_raw_context(tcon, &rtcon)) {
freecon(rscon);
return -1;
}
:
In this case, scon and tcon can be qualified by const, and the first
argument of selinux_trans_to_raw_context() can take const pointer.
But it tries to initialize rscon and tscon by const pointer, although
these are used to store raw security contexts.
The selinux_trans_to_raw_context() always set dynamically allocated
text string on the second argument, so we don't need to initialize it
anyway. I also removed these initializations in this patch.
Does the older mcstrans code could return without allocation of raw
format when the given scon is already raw format? I don't know why
these are initialized in this manner.
Thanks.
--
KaiGai Kohei <kaigai@ak.jp.nec.com>
Signed-off-by: Chad Sellers <csellers@tresys.com>
Email: slawrence@tresys.com
Subject: Add chcon method to libselinux python bindings
Date: Mon, 7 Jun 2010 17:40:05 -0400
Adds a chcon method to the libselinux python bindings to change the
context of a file/directory tree.
Signed-off-by: Chad Sellers <csellers@tresys.com>
This patch simply removes duplicate slashes (meaning "//") from
pathnames passed into selabel_lookup. It does not do a full
realpath() calculation (e.g. following symlinks, etc.), as the
client should really do that before calling into libselinux.
Signed-off-by: Chad Sellers <csellers@tresys.com>
for the given database object identified by its name and object class.
It is necessary to implement a feature something like the restorecon on databases.
The specfile shall be described as follows:
------------------------
#
# The specfile for database objects
# (for SE-PostgreSQL)
#
# <object class> <object name> <security context>
#
db_database * system_u:object_r:sepgsql_db_t:s0
db_schema *.pg_catalog system_u:obejct_r:sepgsql_sys_schema_t:s0
db_schema *.* system_u:object_r:sepgsql_schema_t:s0
db_table *.pg_catalog.* system_u:object_r:sepgsql_sysobj_t:s0
db_table *.*.* system_u:object_r:sepgsql_table_t:s0
------------------------
- All the characters after the '#' are ignored.
- Wildcards ('*' and '?') are available.
- It returns the first match security context.
Note that hierarchy of the namespace of database objects depends on RDBMS.
So, author of the specfile needs to write correct patterns which are suitable
for the target RDBMS. The patched selabel_*() interfaces don't have any
heuristics for the namespace hierarchy to be suitable for widespread RDBMSs.
In the case of SE-PgSQL, when we lookup an expected security context for the
'my_table' table in the 'public' schema and 'postgres' database, the caller
shall provide 'postgres.public.my_table' as a key.
In the default, it tries to read a specfile which maps database objects and security
context from the /etc/selinux/$POLICYTYPE/contexts/sepgsql_contexts.
Note that when another RDBMS uses this interface, it needs to give an explicit
SELABEL_OPT_PATH option on the selabel_open().
Signed-off-by: KaiGai Kohei <kaigai@ak.jp.nec.com>
Acked-by: Eamon Walsh <ewalsh@tycho.nsa.gov>
avc_open() creates the netlink socket in nonblocking mode. If the
application later takes control of the netlink socket with
avc_netlink_acquire_fd() and then calls avc_netlink_loop(), it
will fail with EWOULDBLOCK.
To remedy this, remove the O_NONBLOCK flag from the netlink socket
at the start of avc_netlink_loop(). Also, with this fix, there is
no need for avc_open() to ever create a blocking socket, so change
that and update the man page.
-v2: use poll() in avc_netlink_check_nb(). This makes both
avc_netlink_loop() and avc_netlink_check_nb() independent of the
O_NONBLOCK flag.
-v3: move poll() to avc_receive() internal function; patch by
KaiGai Kohei <kaigai@kaigai.gr.jp>
Signed-off-by: Eamon Walsh <ewalsh@tycho.nsa.gov>
On 02/24/2010 02:24 PM, Daniel J Walsh wrote:
>
Ignore the first patch it was missing pc.in files.
Acked-by: Eamon Walsh <ewalsh@tycho.nsa.gov>
Signed-off-by: Joshua Brindle <method@manicmethod.com>
Email: dwalsh@redhat.com
Subject: Fix memory leak on disabled selinux machines.
Date: Wed, 24 Feb 2010 14:15:31 -0500
I think this patch originally came from Eric Paris and was updated by
others but has not been adopted yet. Not sure why.
Always free buf on exit.
Signed-off-by: Joshua Brindle <method@manicmethod.com>
On Mon, 2010-02-15 at 14:19 -0800, Justin Mattock wrote:
> this is new:
>
>
> make[2]: Leaving directory `/home/kernel/selinux/libselinux/include'
> make -C src install
> make[2]: Entering directory `/home/kernel/selinux/libselinux/src'
> cc -Werror -Wall -W -Wundef -Wshadow -Wmissing-noreturn
> -Wmissing-format-attribute -I../include -I/usr/include -D_GNU_SOURCE
> -D_FILE_OFFSET_BITS=64 -c -o label_file.o label_file.c
> cc1: warnings being treated as errors
> label_file.c: In function 'init':
> label_file.c:434: error: implicit declaration of function 'fstat'
> label_file.c:436: error: implicit declaration of function 'S_ISREG'
> make[2]: *** [label_file.o] Error 1
> make[2]: Leaving directory `/home/kernel/selinux/libselinux/src'
> make[1]: *** [install] Error 2
> make[1]: Leaving directory `/home/kernel/selinux/libselinux'
> make: *** [install] Error 1
>
> three areas where this could of been created
> update glibc
> updated kernel
> update userspace(altohugh there was not vary many commits in the pull).
Newer glibc headers expose a failure to #include the required headers
for stat(2). Also exposes a conflict in redefining close() in that
file. Patch below should fix.
Only audit the permissions specified by the policy, excluding any
permissions specified via dontaudit or not specified via auditallow.
This only shows up when a single avc_has_perm() call is made with
multiple permissions where some of those permissions are dontaudit'd or
auditallow'd while others are not. The corresponding kernel patch has
already been applied, see:
http://git.kernel.org/?p=linux/kernel/git/jmorris/security-testing-2.6.git;a=commit;h=b6cac5a30b325e14cda425670bb3568d3cad0aa8
Signed-off-by: Stephen D. Smalley <sds@tycho.nsa.gov>
Hi folks,
The script, src/exception.sh, contains so called bashisms
(constructs not supported by POSIX, but present as bash
extensions). This means when trying to build on systems where /bin/sh
is not bash, the build fails with an error. This patch uses bash to
run exception.sh. This bug affects a significant subset of Debian and
Debian derivative machines.
manoj
Signed-off-by: Manoj Srivastava <srivasta@debian.org>
Signed-off-by: Joshua Brindle <method@manicmethod.com>
Email: guido@trentalancia.com
Subject: Contributed manual pages for libselinux
Date: Sat, 21 Nov 2009 20:51:17 +0100
Hello Eamon !
On Fri, 2009-11-20 at 21:42 -0500, Eamon Walsh wrote:
> Hi, thanks for doing this. Some quick review below.
You are welcome, I suppose it was a boring task for many...
Thanks very much for reviewing the changes. And please accept my
apologies for not placing "[PATCH]" in the subject of the original post.
I had just subscribed to the list.
I left you cc address intact here...
> There is too much in matchpathcon(3) now. It's going to need to be
> split up into different pages, perhaps the init/fini/teardown stuff in
> one page, the lookup calls in another, and the non-matchpathcon prefixed
> calls in a third page.
>
> Also, .so manpage links are needed for all the calls here.
Yes, matchpathcon is a mess. Following your guidelines, I have now
splitted the huge and messy page in several different man pages. It's
easier to consult and easier to maintain.
The first part (page) is strictly related to _init, its variant
_init_index, _fini, matchpathcon and its variant matchpathcon_index.
Nice and concise. References are provided in the "SEE ALSO" section to
the rest.
The second page describes the auxiliary lookup calls
(matchpathcon_checkmatches) and the inode associations functions
(matchpathcon_filespec_{add,destroy,eval}). The reference section points
to the main matchpathcon page.
A third page has been created for the functions that are used to set the
flags (set_matchpathcon_flags) or to configure the behaviour of the main
matchpathcon functions (set_matchpathcon_invalidcon and
set_matchpathcon_printf).
A fourth and fifth page is devoted to functions that should never had
ended up in matchpathcon (selinux_file_context_cmp and
selinux_file_context_verify in one page and selinux_lsetfilecon_default
in another one): we do not really need to save electrons needed for new
pages...
>
>
> > * print_access_vector
> >
>
> Looks good.
No modifications.
> > * security_disable
> >
>
> See the selinux.h comments for this. It needs to be documented that
> this function can only be called at startup time.
Ok. I have stressed that now and also mentioned that after the policy
has been loaded at startup, then only "setenforce" can be used to alter
(not disable) the mode of the SELinux kernel code (for example by
placing it into "permissive" mode).
> > * security_set_boolean_list
> >
>
> a RETURN VALUE section is needed in this page, documenting at least this
> call if not the others in that page.
I have now added a "RETURN VALUE" section.
Also, to avoid confusion, I have rephrased the word "returns" in
"provides" when not strictly referring the to the return value of the
function (take for example security_get_boolean_names(), strictly
speaking the function returns an integer representing 0=success or
-1=failure, although from a conceptual point of view it also returns a
list trough modification of one of its parameters passed by reference).
Usually when an application developer looks at the "RETURN VALUE"
section it is because he/she has already planned/coded the call to the
function (and thus also the handling to parameters passed by reference)
and only needs to check for the function exit status so that it can be
handled properly at the call point.
> > * selinux_check_passwd_access
> >
>
> This is a replacement for the inconsistently named "checkPasswdAccess"
> function. So, the existing description of checkPasswdAccess should be
> moved to this function, and checkPasswdAccess should be changed to "this
> is a deprecated alias for selinux_check_passwd_access".
Yes, I have now mentioned that checkPasswdAccess is deprecated. We are
referring to file security_compute_av.3 as the description of these two
functions lives there...
By the way, it has been pointed out that this function should not
hard-code a string. I also agree with him, there is a generic constant
for such "passwd" object class, it is defined in flask.h could be used
instead of the string, thus avoiding hard-coding and also allowing to
save a few cycles and be theoretically future-proof (if ever the name
would change, say to "password", "auth-token" or anything else).
libselinux/src/checkAccess.c.orig 2009-11-21 20:07:21.000000000
libselinux/src/checkAccess.c 2009-11-21 20:08:36.000000000
@@ -13,17 +13,12 @@ int selinux_check_passwd_access(access_v
if (is_selinux_enabled() == 0)
return 0;
if (getprevcon_raw(&user_context) == 0) {
- security_class_t passwd_class;
struct av_decision avd;
int retval;
- passwd_class = string_to_security_class("passwd");
- if (passwd_class == 0)
- return 0;
-
retval = security_compute_av_raw(user_context,
user_context,
- passwd_class,
+ SECCLASS_PASSWD,
requested,
&avd);
Note that the above code, should really live in the application and not
in the selinux library. It used to be like that, then for some reason it
has been introduced. Redhat's passwd and cronie are calling the library
function and thus at the moment they rely on it. But for example,
util-linux-ng has the code in it and does not call this function, as I
believe it should be. A very minor issue anyway...
> > * selinux_init_load_policy
> >
>
> A paragraph break is needed in the DESCRIPTION section before this function.
Done. I have also added a note to the already mentioned fact that after
initial policy load, SELinux cannot be anymore disabled using calls to
security_disable(3).
> > * selinux_lsetfilecon_default
> >
>
> See notes above about the matchpathcon manpage.
Yes, separate man page now.
> > * selinux_mkload_policy
> >
>
> Looks good.
No modifications.
> > * set_selinuxmnt
> >
>
> This manpage includes two static functions that are not part of the
> libselinux API (at least, not anymore) and should be removed.
>
> Also, I'm not comfortable with the description given. Instead, use the
> comments in selinux.h, which are more accurate and verbose.
>
Please let me know if things are any better now.
I did also provide on the same day a patch for beautifying and improving
the command-line option parsing of a few utilities (a ticket had been
created by somebody). That patch provides those improvement according to
GNU-style parsing of "help" and "version" options (including long-option
variants). I think it also fixes a couple of typos here and there. Feel
free to include that patch too if you like it, so that the ticket can be
closed ! I will attach it again in another separate message: it has been
slightly modified in order to apply cleanly to the latest git snapshot.
More important, I was also thinking about fingerprinting (and
subsequently checking) the libraries with some cryptographic hash
function such as the NIST-recommended SHA2. It is beginning to be done
for security-related projects like OpenSSL, so I believe it is even more
essential for SELinux. Ever thought about anything like that ?
Best regards,
Guido
Signed-off-By: Joshua Brindle <method@manicmethod.com>
Having a pkgconfig files allows the pkg-config tool to be used to
query the presence of the library (or a particular version of it),
and to obtain the C flags and linker arguments to build with it.
Based on Debian patches by Manoj Srivastava <srivasta@debian.org>.
Signed-off-by: Eamon Walsh <ewalsh@tycho.nsa.gov>
In integrating SELinux policy into rpm, we have a need to be
able to reset the configuration data (e.g. policy type) loaded
into libselinux. These values are currently loaded lazily by a
number of different functions (e.g. matchpatchcon_init()).
Since we are changing rpm to install policy, including initial
base policy, we need to be able to reload these configuration
items after the policy has been installed.
reset_selinux_config() already exists and is used by
selinux_init_load_policy() for a similar reason, but it is not
exported. This was probably intentionaly since it is not thread
safe at all. That said, rpm needs to do the same thing. This
patch makes the function public, and places a warning in the
header comment that it is not thread safe.
Signed-off-by: Chad Sellers <csellers@tresys.com>
On 09/16/2009 03:35 PM, Joshua Brindle wrote:
>
>
> Joshua Brindle wrote:
>>
>>
>> Daniel J Walsh wrote:
>>> What do you think of this one. Removed excess swig cruft,
>>>
>>> You need to run
>>>
>>> make swigify to generate those changes.
>>>
>>
>> Ok, looking at this now. I don't completely get how it works. I'm trying
>> to reproduce what you are doing by hand but nothing comes out of gcc:
>>
>> [root@localhost src]# echo '#include "../include/selinux/selinux.h"' >
>> temp.c
>> [root@localhost src]# gcc -c temp.c -aux-info temp.aux
>> [root@localhost src]# ls temp.*
>> temp.c temp.o
>>
>>
>> What is the purpose of the aux-info thing, and why doesn't it work on my
>> F11 machine?
>>
>> also, I'm not sure if the best place for selinuxswig_exception.i is
>> swigify or pywrap. In the swigify case it shouldn't be in the clean
>> target because if you check out the repo and do make clean; make pywrap
>> you'll get an error. (I can make these fixes, I'm just trying to figure
>> out how it all works first).
>>
>
> Oh, one more thing, should this be python specific? (E.g, should it be
> named selinuxswig_python_exception.i ?)
Changed name to selinux_python_exception.i
WOrks for me on F11 and F12
dwalsh@localhost$ echo '#include "../include/selinux/selinux.h"' > temp.c
dwalsh@localhost$ gcc -c temp.c -aux-info temp.aux
dwalsh@localhost$ ls temp.*
temp.aux temp.c temp.o
cat temp.aux
/* compiled from: . */
/* /usr/include/sys/select.h:109:NC */ extern int select (int, fd_set *, fd_set *, fd_set *, struct timeval *);
/* /usr/include/sys/select.h:121:NC */ extern int pselect (int, fd_set *, fd_set *, fd_set *, const struct timespec *, const __sigset_t *);
/* /usr/include/sys/sysmacros.h:31:NC */ extern unsigned int gnu_dev_major (long long unsigned int);
/* /usr/include/sys/sysmacros.h:34:NC */ extern unsigned int gnu_dev_minor (long long unsigned int);
/* /usr/include/sys/sysmacros.h:37:NC */ extern long long unsigned int gnu_dev_makedev (unsigned int, unsigned int);
/* ../include/selinux/selinux.h:12:NC */ extern int is_selinux_enabled (void);
/* ../include/selinux/selinux.h:14:NC */ extern int is_selinux_mls_enabled (void);
/* ../include/selinux/selinux.h:19:NC */ extern void freecon (security_context_t);
/* ../include/selinux/selinux.h:22:NC */ extern void freeconary (security_context_t *);
...
commit 38d98bd958f42ea18c9376e624d733795665ee22
Author: Dan Walsh <dwalsh@redhat.com>
Date: Wed Sep 16 16:51:14 2009 -0400
Add exception code
