Bug 1371389

Summary: libsemanage calls mkdir() without setting the umask properly
Product: Red Hat Enterprise Linux 6 Reporter: Jan Zarsky <jzarsky>
Component: libsemanageAssignee: Petr Lautrbach <plautrba>
Status: CLOSED WONTFIX QA Contact: Jan Zarsky <jzarsky>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 6.8CC: dpal, dwalsh, jhrozek, jzarsky, ksrot, lvrabec, mgrepl, mmalik, plautrba, pvrabec, ssekidde
Target Milestone: rc   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: 1186422 Environment:
Last Closed: 2016-10-17 09:25:08 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Description Jan Zarsky 2016-08-30 06:38:21 UTC
+++ This bug was initially created as a clone of Bug #1186422 +++

Description of problem:
On some places, like semanage_copy_dir(), libsemanage creates a directory with certain permissions using mkdir(path, 0700) and relies on the permissions being correct. But it never sets or checks the umask, so programs using libsemanage with a very restrictive umask (like deamons) might end up creating the directory with wrong permissions.

Version-Release number of selected component (if applicable):
libsemanage-2.0.43-5.1.el6.x86_64

How reproducible:
always

Steps to Reproduce:
1. set a restrictive umask
2. call libsemanage functions
3. ls -ld /etc/selinux/targeted/modules/active

Actual results:
clobbered permissions

Expected results:
permissions are as libsemanage expects them.

Additional info:

--- Additional comment from Roland Mainz on 2015-01-27 11:39:56 EST ---

Basically libsemanage should fix the permissions after the |mkdir()| call like the mkdir(1) code in https://searchcode.com/codesearch/view/5481405/ line 160 does.

An even better way would be to use |mkdirat()| and |fchmodat()| to prevent attacks which swap the parent directory, e.g. you |dirfd = open(parentdirname, O_SEARCH)| and then do |mkdirat(dirfd, ...| and |fchmodat(dirfd, ...)|.

Note that Linux does not have POSIX |O_SEARCH| but aliasing it to |O_PATH| works fine, e.g. do this ...
-- snip --
#ifndef O_SEARCH
#define O_SEARCH (O_PATH)
#endif
-- snip --
... and you're done...

--- Additional comment from Petr Lautrbach on 2015-07-21 10:49:59 EDT ---

It needs to be fixed and discussed upstream first. When an user sets umask, she could have a reason for that. I,m postponing this bug to 7.3. Sorry.