#48305 perl module conditional test is not conditional when checking SELinux policies
Closed: wontfix None Opened 8 years ago by nhosoi.

Description of problem:

this kind of a corner case, may be minor, but the following conditional tests
are not really conditional, they are *always* true:

In /usr/lib64/dirsrv/perl/DSCreate.pm

There is a test to use or not use semanage for setup-ds-admin.pl calling
createDSInstance which calls updateSelinuxPolicy.

The test is supposed to do nothing if SELinux is disabled, but we have this:

...snip...
sub updateSelinuxPolicy {
    my $inf = shift;

    # if selinux is not available, do nothing
    if ("yes") {
...snip...


Same test with remove-ds-admin.pl calling removeDSInstance in the perl module
DSCreate.pm
It will try to access policy files that do not exist when SELinux is disabled
on a system:

...snip...
    # remove the selinux label from the ports if needed
    if ("yes") {
        foreach my $port (@{$entry->{"nsslapd-port"}})
        {
            my $semanage_err = `semanage port -d -t ldap_port_t -p tcp $port
2>&1`;
...snip...


The actions with
   if ("yes") {
are very ironic.

The "problem" is there are systems configured without SELInux out there, and
when some admin do not want SELinux, they just do not want it, the perception
is "wrong", and opposite of what we want.

How reproducible:
always


Steps to Reproduce:

0. have a RHEL 6.7 or RHEL 7.1 system with 389-ds-base installed

1. Disable SELinux
cp -p /etc/sysconfig/selinux /etc/sysconfig/selinux.orig
perl -pi -e 's/^SELINUX=.*/SELINUX=disabled/' /etc/sysconfig/selinux
diff -u /etc/sysconfig/selinux.orig /etc/sysconfig/selinux
...
-SELINUX=permissive
+SELINUX=disabled
...

2. reboot

note: I had an "unexpected" problem on my test RHEL 7.1 VM: SELINUX staid
enabled even though I followed the usual simple steps as confirmed in the guide
Red_Hat_Enterprise_Linux-7-SELinux_Users_and_Administrators_Guide-en-US.pdf
I do not know if SELinux can be disabled on 7.1, so after several re-check and
reboots, so I only tested with a RHEL 6.7 VM, like a customer report in SF case
number 01501498

getenforce
Disabled

sestatus
SELinux status:                 disabled


3. now, to mimic the fact some users do not want SELinux at all, remove the
package selinux-policy-targeted and delete left over policy:

rpm -e selinux-policy-targeted
mv  /etc/selinux/targeted/policy  /etc/selinux/targeted/policy-1


4. create an instance with verbose log

cat > /root/test.inf << EOF
[General]
AdminDomain = example.com
ConfigDirectoryAdminID =
uid=admin,ou=Administrators,ou=TopologyManagement,o=NetscapeRoot
ConfigDirectoryAdminPwd = password
ConfigDirectoryLdapURL = ldap://m1.example.com:389/o=NetscapeRoot
FullMachineName = m1.example.com
ServerRoot = /usr/lib64/dirsrv
SuiteSpotGroup = nobody
SuiteSpotUserID = nobody
[admin]
Port = 8888
ServerAdminID = admin
ServerAdminPwd = {SHA}W6ph5Mm5Pz8GgiULbPgzG37mj9g=
ServerIpAddress = 0.0.0.0
SysUser = nobody
[slapd]
AddOrgEntries = Yes
AddSampleEntries = No
HashedRootDNPwd = {SSHA}hYlIdKA3Ln3LrzZrd8mWSycaYnfIJw4TK70iOg==
InstallLdifFile = suggest
RootDN = cn=Directory Manager
RootDNPwd = password
ServerIdentifier = m1bis
ServerPort = 31081
Suffix = dc=example,dc=com
UseExistingMC = 1
bak_dir = /var/lib/dirsrv/slapd-m1bis/bak
bindir = /usr/bin
cert_dir = /etc/dirsrv/slapd-m1bis
config_dir = /etc/dirsrv/slapd-m1bis
datadir = /usr/share
db_dir = /var/lib/dirsrv/slapd-m1bis/db
ds_bename = userRoot
inst_dir = /usr/lib64/dirsrv/slapd-m1bis
ldif_dir = /var/lib/dirsrv/slapd-m1bis/ldif
localstatedir = /var
lock_dir = /var/lock/dirsrv/slapd-m1bis
log_dir = /var/log/dirsrv/slapd-m1bis
naming_value = example
run_dir = /var/run/dirsrv
sbindir = /usr/sbin
schema_dir = /etc/dirsrv/slapd-m1bis/schema
sysconfdir = /etc
tmp_dir = /tmp
EOF


/usr/sbin/setup-ds-admin.pl -ddddddd --silent --file=/root/test.inf 2>&1 | tee
~/log.setup-ds-admin.pl.ddddddd.m1bis.selinux.disabled.no.rpm.selinux-policy-ta
rgeted.and.no.etc.selinux.targeted.policy.directory.txt



Actual results:

...
[03/Sep/2015:17:31:10 -0700] - import userRoot: Import complete.  Processed 9
entries in 1 seconds. (9.00 entries/sec)
libsemanage.semanage_install_active: Could not copy
/etc/selinux/targeted/modules/active/policy.kern to
/etc/selinux/targeted/policy/policy.24. (No such file or directory).
libsemanage.semanage_install_active: Could not copy
/etc/selinux/targeted/modules/active/policy.kern to
/etc/selinux/targeted/policy/policy.24. (No such file or directory).
/usr/sbin/semanage: Could not commit semanage transaction
+++no tmpfiles.d - skipping
+++no systemd - skipping
++Found brand specific inf file/usr/share/dirsrv/inf/redhat-slapd.inf
...


Expected results:

So of course there is no such file
/etc/selinux/targeted/policy/policy.24
because I removed it, but:

As SELinux is disabled, even if this is a corner case, the install call to
createDSInstance in  /usr/lib64/dirsrv/perl/DSCreate.pm should not attempt to
use the semanage tool in updateSelinuxPolicy
There should be no libsemanage or semanage related calls or trace.
The tests with
   if ("yes") {
are wrong.


Additional info:

to verify the error:
semanage port -a -t ldap_port_t -p tcp 31081
libsemanage.semanage_install_active: Could not copy
/etc/selinux/targeted/modules/active/policy.kern to
/etc/selinux/targeted/policy/policy.24. (No such file or directory).
libsemanage.semanage_install_active: Could not copy
/etc/selinux/targeted/modules/active/policy.kern to
/etc/selinux/targeted/policy/policy.24. (No such file or directory).
/usr/sbin/semanage: Could not commit semanage transaction


less /usr/lib64/dirsrv/perl/DSCreate.pm
...
sub updateSelinuxPolicy {
    my $inf = shift;

    # if selinux is not available, do nothing
    if ("yes") {
...
...
            if ($need_label == 1) {
                system("semanage port -a -t ldap_port_t -p tcp
$inf->{slapd}->{ServerPort}");
            }
        }
    }
}

...
sub createDSInstance {
    my $inf = shift;
    my @errs;
...
    if (@errs = initDatabase($inf)) {
        return @errs;
    }

    updateSelinuxPolicy($inf);

    if (@errs = updateTmpfilesDotD($inf)) {
        return @errs;
    }

    if (@errs = updateSystemD($inf)) {
        return @errs;
...

Reviewed by Mark (Thanks!!)

Pushed to master:
2c702c4..9fefc13 master -> master
commit 9fefc13

Pushed to 389-ds-base-1.3.4:
7ee822e..2a05a3a 389-ds-base-1.3.4 -> 389-ds-base-1.3.4
commit 2a05a3a

Pushed to 389-ds-base-1.3.3:
8b097d1..b20756b 389-ds-base-1.3.3 -> 389-ds-base-1.3.3
commit b20756b

Pushed to 389-ds-base-1.2.11:
265c6e3..72a6380 389-ds-base-1.2.11 -> 389-ds-base-1.2.11
commit 72a6380

The -f /dev/null test is not working correctly. Reopening.

git patch file (master) -- fixing a regression caused by checking /dev/null with "-f".
0001-Ticket-48305-perl-module-conditional-test-is-not-con.2.patch

Reviewed by Mark (Thank you!!)

Pushed to master:
00513db..c4233ec master -> master
commit c4233ec

Pushed to 389-ds-base-1.3.4:
4a53592..48308f3 389-ds-base-1.3.4 -> 389-ds-base-1.3.4
commit 48308f3

Pushed to 389-ds-base-1.3.3:
5cd8f73..aab7eda 389-ds-base-1.3.3 -> 389-ds-base-1.3.3
commit aab7eda

Pushed to 389-ds-base-1.2.11:
c7cf000..aa59677 389-ds-base-1.2.11 -> 389-ds-base-1.2.11
commit aa59677

Metadata Update from @nhosoi:
- Issue assigned to nhosoi
- Issue set to the milestone: 1.2.11.33

7 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/1636

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.

Metadata Update from @spichugi:
- Issue close_status updated to: wontfix (was: Fixed)

3 years ago

Login to comment on this ticket.

Metadata