On 04/04/2013 04:25 PM, Rob Crittenden wrote:
Tomas Babej wrote:
On Tue 02 Apr 2013 10:05:06 AM CEST, Tomas Babej wrote:
On Mon 01 Apr 2013 10:01:14 PM CEST, Rob Crittenden wrote:
Tomas Babej wrote:
On Tue 19 Feb 2013 08:37:26 PM CET, Rob Crittenden wrote:
Tomas Babej wrote:
On 02/04/2013 04:21 PM, Rob Crittenden wrote:
Tomas Babej wrote:
On 01/30/2013 05:12 PM, Tomas Babej wrote:
Hi,

The checks make sure that SELinux is:
  - installed and enabled (on server install)
  - installed and enabled OR not installed (on client install)

Please note that client installs with SELinux not installed are
allowed since freeipa-client package has no dependency on SELinux.
(any objections to this approach?)

The (unsupported) option --allow-no-selinux has been added. It can
used to bypass the checks.

Parts of platform-dependant code were refactored to use newly
added
is_selinux_enabled() function.

https://fedorahosted.org/freeipa/ticket/3359

Tomas

I forgot to edit the man pages. Thanks Rob!

Updated patch attached.

Tomas

After a bit of off-line discussion I don't think we're quite ready
yet
to require SELinux by default on client installations (even with a
flag to work around it). The feeling is this would be disruptive to
existing automation.

Can you still do the check but not enforce it, simply display a big
warning if SELinux is disabled?

rob


Sure, here is the updated patch.

I edited the commit message, RFE description and man pages
according to
the new behaviour.

Tomas

The patch looks good, I'm just wondering about one thing. The default
value for is_selinux_enabled() is True in ipapython/services.py.in.

So this means that any non-Red Hat/non-Fedora system, by default, is
going to assume that SELinux is enabled.

My hesitation has to when we call check_selinux_status(). It may
incorrectly error out. I suspect that the user would have to work
around this using --allow-selinux-disabled but this wouldn't make a
lot of sense since they actually do have SELinux disabled.

Yes, you're right. And the error message would not even be helpful
since
it would tell the user to install policycoreutils package. This
would be
the
case both with server and client installs when selinux would not be
installed
at all.

What do you think?

rob

Well we have 2 options as I see it:

1.) We can either return None as default, and add checks to
check_selinux_status, restore_context and install scripts that would
ensure that we behave properly when is_selinux_enabled() is not
implemented.

2.) We can remove the default value, since it would cause forementioned
crash and add comment that this function needs to be implemented
properly in every platform file.

I'm probably for option 2, there's no need to clutter the code with
checks
that compensate for improper platform file implementations.

Tomas

I agree with you on option 2.

rob

I updated the patch accordingly.

Tomas

Sorry, wrong patch. Correct version attached.

Tomas

I'm sorry to throw this back again after so long (and having agreed with the approach).

So I was thinking about how another distro maintainer would have to deal with this. By default with this patch check_selinux_status() returns None which is evaluated as False, so the warning will get thrown. If they set it to be True to avoid the warning then other things may blow up because SELinux really isn't enabled, so we really haven't gotten anywhere.

I think the problem is we're trying to cram too much into one function. I wonder if a is_selinux_available() command would help which would short-circuit all of this.

While trying to figure out how this worked I found httpinstance.configure_selinux_for_httpd() which makes a similar call to see if SELinux is available, so maybe we should convert that as well.

rob
I added the is_selinux_available function. Both is_selinux_enabled and is_selinux_available default to False in services.py.in. Maintainer that would want to implement platform file, would have to implement both functions for server install. We require SELinux for server anyway. For client installs, default versions work fine.

I converted httpinstance.configure_selinux_for_httpd() to use is_selinux_enabled(). I also found a similar call in adtrustinstance.py

Tomas

>From b1a3be80c440681a1fc46da86db42bde5fa6dd4a Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Thu, 24 Jan 2013 15:37:21 -0500
Subject: [PATCH] Add checks for SELinux in install scripts

The checks make sure that SELinux is:
  - installed and enabled, otherwise the installation aborts
    (on server install)
  - installed and enabled OR not installed, otherwise
    the warning message is displayed (on client install)

The (unsupported) option --allow-selinux-disabled has been added. It can
used to bypass the checks. Documented in man pages altered accordingly.

Parts of platform-dependant code were refactored to use newly added
is_selinux_enabled() function.

https://fedorahosted.org/freeipa/ticket/3359
---
 install/tools/ipa-server-install          | 16 ++++++++
 install/tools/man/ipa-server-install.1    |  3 ++
 ipa-client/ipa-install/ipa-client-install | 16 ++++++++
 ipa-client/man/ipa-client-install.1       |  3 ++
 ipapython/platform/fedora16/__init__.py   |  5 ++-
 ipapython/platform/fedora16/selinux.py    |  6 +++
 ipapython/platform/fedora18/__init__.py   |  5 ++-
 ipapython/platform/redhat/__init__.py     | 61 +++++++++++++++++++------------
 ipapython/services.py.in                  | 13 ++++++-
 ipaserver/install/adtrustinstance.py      | 10 +----
 ipaserver/install/httpinstance.py         | 11 +-----
 11 files changed, 104 insertions(+), 45 deletions(-)

diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index da3caa08d2b00fe3a750ef53573d7d2275635327..6b01d2c9cc044b273b64263263946d1489add69b 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -159,6 +159,8 @@ def parse_options():
                       help="do not configure OpenSSH client")
     basic_group.add_option("--no-sshd", dest="conf_sshd", default=True, action="store_false",
                       help="do not configure OpenSSH server")
+    basic_group.add_option("--allow-selinux-disabled", dest="selinux_disabled", action="store_true",
+                      default=False, help="allow installation with SELinux disabled (not supported)")
     basic_group.add_option("-d", "--debug", dest="debug", action="store_true",
                       default=False, help="print debugging information")
     basic_group.add_option("-U", "--unattended", dest="unattended", action="store_true",
@@ -653,6 +655,18 @@ def main():
             print "CA is not installed yet. To install with an external CA is a two-stage process.\nFirst run the installer with --external-ca."
             sys.exit(1)
 
+    # Should not happen
+    if not ipaservices.is_selinux_available():
+        print("SELinux is required for IPA server.")
+        sys.exit(1)
+
+    is_selinux_disabled = not ipaservices.is_selinux_enabled()
+    if (not options.selinux_disabled) and is_selinux_disabled:
+        print("Installation with SELinux disabled is not supported. \n"
+              "Put SELinux either in permissive / enabled mode or use"
+              " --allow-selinux-disabled option.")
+        sys.exit(1)
+
     # This will override any settings passed in on the cmdline
     if ipautil.file_exists(ANSWER_CACHE):
         if options.dm_password is not None:
@@ -1176,6 +1190,8 @@ def main():
             args.append("--no-dns-sshfp")
         if options.trust_sshfp:
             args.append("--ssh-trust-dns")
+        if options.selinux_disabled:
+            args.append("--allow-selinux-disabled")
         if not options.conf_ssh:
             args.append("--no-ssh")
         if not options.conf_sshd:
diff --git a/install/tools/man/ipa-server-install.1 b/install/tools/man/ipa-server-install.1
index 6959a314785e5020ed1d7701873baf3c2260c2df..7ed47a1644450116f3d223b24712ec74280f5497 100644
--- a/install/tools/man/ipa-server-install.1
+++ b/install/tools/man/ipa-server-install.1
@@ -75,6 +75,9 @@ Do not configure OpenSSH client.
 \fB\-\-no\-sshd\fR
 Do not configure OpenSSH server.
 .TP
+\fB\-\-allow\-selinux\-disabled\fR
+Allow installation with SELinux disabled. This is unsupported. Please consider setting SELinux to permissive / enforcing mode.
+.TP
 \fB\-d\fR, \fB\-\-debug\fR
 Enable debug logging when more verbose output is needed
 .TP
diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 126611a824f072bbfba1a7fe28584a5b921d5704..db89484ec9e65f03dbf703855a39c6b6181e358b 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -130,6 +130,8 @@ def parse_options():
                       help="do not automatically create DNS SSHFP records")
     basic_group.add_option("--noac", dest="no_ac", default=False, action="store_true",
                       help="do not use Authconfig to modify the nsswitch.conf and PAM configuration")
+    basic_group.add_option("--allow-selinux-disabled", dest="selinux_disabled", action="store_true",
+                      default=False, help="supress warning message when SELinux is disabled")
     basic_group.add_option("-f", "--force", dest="force", action="store_true",
                       default=False, help="force setting of LDAP/Kerberos conf")
     basic_group.add_option("-d", "--debug", dest="debug", action="store_true",
@@ -2423,7 +2425,9 @@ def main():
 
     if not os.getegid() == 0:
         sys.exit("\nYou must be root to run ipa-client-install.\n")
+
     ipaservices.check_selinux_status()
+
     logging_setup(options)
     root_logger.debug(
         '%s was invoked with options: %s', sys.argv[0], safe_options)
@@ -2447,6 +2451,18 @@ def main():
             "using 'ipa-client-install --uninstall'.")
         return CLIENT_ALREADY_CONFIGURED
 
+    # We allow clients with no SELinux installed
+    is_selinux_disabled = ipaservices.is_selinux_available() and\
+                              not ipaservices.is_selinux_enabled()
+
+    if (not options.selinux_disabled) and is_selinux_disabled:
+        root_logger.warning("Installation with SELinux disabled is not "
+                            "recommended.")
+        root_logger.warning("Putting SELinux either in permissive/enabled mode "
+                            "is advised for higher security.")
+        root_logger.info("To supress this warning message, use "
+                         "--allow-selinux-disabled option.")
+
     rval = install(options, env, fstore, statestore)
     if rval == CLIENT_INSTALL_ERROR:
         if options.force:
diff --git a/ipa-client/man/ipa-client-install.1 b/ipa-client/man/ipa-client-install.1
index d98318eeda1d6b60d4a6bcb1321db03bfabe15a8..1495aadc7d5199853812ac43f975305d33b86477 100644
--- a/ipa-client/man/ipa-client-install.1
+++ b/ipa-client/man/ipa-client-install.1
@@ -106,6 +106,9 @@ Do not configure OpenSSH client.
 \fB\-\-no\-sshd\fR
 Do not configure OpenSSH server.
 .TP
+\fB\-\-allow\-selinux\-disabled\fR
+Supress warning message about disabled SELinux.
+.TP
 \fB\-\-no\-dns\-sshfp\fR
 Do not automatically create DNS SSHFP records.
 .TP
diff --git a/ipapython/platform/fedora16/__init__.py b/ipapython/platform/fedora16/__init__.py
index 26a6afd286f83f7c2781f1edf3e80fec8ebff06e..1989a37ab9cda320eb280a191e777bdca4db5df4 100644
--- a/ipapython/platform/fedora16/__init__.py
+++ b/ipapython/platform/fedora16/__init__.py
@@ -38,7 +38,8 @@ from ipapython.platform.fedora16.service import f16_service, Fedora16Services
 #                         and restorecon is installed.
 __all__ = ['authconfig', 'service', 'knownservices',
     'backup_and_replace_hostname', 'restore_context', 'check_selinux_status',
-    'restore_network_configuration', 'timedate_services']
+    'restore_network_configuration', 'timedate_services','is_selinux_enabled',
+    'is_selinux_available']
 
 # Just copy a referential list of timedate services
 timedate_services = list(base.timedate_services)
@@ -49,4 +50,6 @@ knownservices = Fedora16Services()
 backup_and_replace_hostname = redhat.backup_and_replace_hostname
 restore_context = selinux.restore_context
 check_selinux_status = selinux.check_selinux_status
+is_selinux_enabled = selinux.is_selinux_enabled
+is_selinux_available = selinux.is_selinux_available
 restore_network_configuration = redhat.restore_network_configuration
diff --git a/ipapython/platform/fedora16/selinux.py b/ipapython/platform/fedora16/selinux.py
index cf71a38e43355420b5ff7fbedbfea6e9e82c2d1c..c46831a0a7406ffc1e620d7e83e3ddc3ca29a98e 100644
--- a/ipapython/platform/fedora16/selinux.py
+++ b/ipapython/platform/fedora16/selinux.py
@@ -24,3 +24,9 @@ def restore_context(filepath, restorecon='/usr/sbin/restorecon'):
 
 def check_selinux_status(restorecon='/usr/sbin/restorecon'):
     return redhat.check_selinux_status(restorecon)
+
+def is_selinux_available():
+    return redhat.is_selinux_available()
+
+def is_selinux_enabled():
+    return redhat.is_selinux_enabled()
diff --git a/ipapython/platform/fedora18/__init__.py b/ipapython/platform/fedora18/__init__.py
index d12bdcad5eb53881db5fe94cef97f2fafe2c6442..140fa2949c6089c8085cb7b5e4dd5eeb6e0d1f5c 100644
--- a/ipapython/platform/fedora18/__init__.py
+++ b/ipapython/platform/fedora18/__init__.py
@@ -44,7 +44,8 @@ from ipapython.platform import fedora16, base
 #                         and restorecon is installed.
 __all__ = ['authconfig', 'service', 'knownservices',
     'backup_and_replace_hostname', 'restore_context', 'check_selinux_status',
-    'restore_network_configuration', 'timedate_services']
+    'restore_network_configuration', 'timedate_services', 'is_selinux_enabled',
+    'is_selinux_available']
 
 # Just copy a referential list of timedate services
 timedate_services = list(base.timedate_services)
@@ -111,3 +112,5 @@ service = fedora16.service
 knownservices = fedora16.knownservices
 restore_context = fedora16.restore_context
 check_selinux_status = fedora16.check_selinux_status
+is_selinux_enabled = fedora16.is_selinux_enabled
+is_selinux_available = fedora16.is_selinux_available
diff --git a/ipapython/platform/redhat/__init__.py b/ipapython/platform/redhat/__init__.py
index f7680e7ec510d3a2fde4febc88be15fe8d9f98d5..9e3ea78c9885aa77c58a0b4652efd90a3ed23d24 100644
--- a/ipapython/platform/redhat/__init__.py
+++ b/ipapython/platform/redhat/__init__.py
@@ -48,7 +48,8 @@ from ipapython.platform.redhat.service import redhat_service, RedHatServices
 #                         and restorecon is installed.
 __all__ = ['authconfig', 'service', 'knownservices',
     'backup_and_replace_hostname', 'restore_context', 'check_selinux_status',
-    'restore_network_configuration', 'timedate_services']
+    'restore_network_configuration', 'timedate_services', 'is_selinux_enabled',
+    'is_selinux_available']
 
 # Just copy a referential list of timedate services
 timedate_services = list(base.timedate_services)
@@ -57,6 +58,7 @@ authconfig = RedHatAuthConfig
 service = redhat_service
 knownservices = RedHatServices()
 
+
 def restore_context(filepath, restorecon='/sbin/restorecon'):
     """
     restore security context on the file path
@@ -67,19 +69,11 @@ def restore_context(filepath, restorecon='/sbin/restorecon'):
 
     ipautil.run() will do the logging.
     """
-    try:
-        if (os.path.exists('/usr/sbin/selinuxenabled')):
-            ipautil.run(["/usr/sbin/selinuxenabled"])
-        else:
-            # No selinuxenabled, no SELinux
-            return
-    except ipautil.CalledProcessError:
-        # selinuxenabled returns 1 if not enabled
-        return
 
-    if (os.path.exists(restorecon)):
+    if is_selinux_enabled() and os.path.exists(restorecon):
         ipautil.run([restorecon, filepath], raiseonerr=False)
 
+
 def backup_and_replace_hostname(fstore, statestore, hostname):
     old_hostname = socket.gethostname()
     try:
@@ -104,6 +98,7 @@ def backup_and_replace_hostname(fstore, statestore, hostname):
     else:
         statestore.backup_state('network', 'hostname', old_hostname)
 
+
 def check_selinux_status(restorecon='/sbin/restorecon'):
     """
     We don't have a specific package requirement for policycoreutils
@@ -114,18 +109,38 @@ def check_selinux_status(restorecon='/sbin/restorecon'):
     This function returns nothing but may raise a Runtime exception
     if SELinux is enabled but restorecon is not available.
     """
-    try:
-        if (os.path.exists('/usr/sbin/selinuxenabled')):
-            ipautil.run(["/usr/sbin/selinuxenabled"])
-        else:
-            # No selinuxenabled, no SELinux
-            return
-    except ipautil.CalledProcessError:
-        # selinuxenabled returns 1 if not enabled
-        return
-
-    if not os.path.exists(restorecon):
-        raise RuntimeError('SELinux is enabled but %s does not exist.\nInstall the policycoreutils package and start the installation again.' % restorecon)
+
+    if is_selinux_enabled() and not os.path.exists(restorecon):
+        raise RuntimeError('SELinux is enabled but %s does not exist.\n'
+            'Install the policycoreutils package and start the installation'
+            ' again.' % restorecon)
+
+
+def is_selinux_available():
+    """
+    Checks if SELinux available. Uses /usr/sbin/selinuxenabled as an indicator.
+    """
+
+    if os.path.exists('/usr/sbin/selinuxenabled'):
+        return True
+    else:
+        return False
+
+
+def is_selinux_enabled():
+    """"
+    Checks if SELinux is enabled. Returns False if SELinux is not installed
+    at all.
+    """
+
+    if not is_selinux_available():
+        return False
+
+    (stdout, stderr, rc) = ipautil.run(['/usr/sbin/selinuxenabled'],
+                                       raiseonerr=False,
+                                       capture_output=False)
+
+    return (rc == 0)
 
 def restore_network_configuration(fstore, statestore):
     filepath = '/etc/sysconfig/network'
diff --git a/ipapython/services.py.in b/ipapython/services.py.in
index 16b62ca8508d4078e896cd1da6fd664f52a3930e..a1a0a01f9d5eae347c6b44b40ed0bca5fa48934f 100644
--- a/ipapython/services.py.in
+++ b/ipapython/services.py.in
@@ -21,7 +21,7 @@
 authconfig = None
 
 # knownservices is an entry point to known platform services
-# (instance of ipapython.platform.base.KnownServices) 
+# (instance of ipapython.platform.base.KnownServices)
 knownservices = None
 
 # service is a class to instantiate ipapython.platform.base.PlatformService
@@ -51,6 +51,17 @@ backup_and_replace_hostname = backup_and_replace_hostname_default
 def check_selinux_status():
     return
 
+# Check if SELinux is enabled
+# In case SELinux is not available at all, returns False
+# Every platform should implement this function properly.
+def is_selinux_enabled():
+    return False
+
+# Checks if SELinux is available
+# Defaults to False
+def is_selinux_available():
+    return False
+
 from ipapython.platform.base import SVC_LIST_FILE
 def get_svc_list_file():
     return SVC_LIST_FILE
diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py
index a47c80b3983f3086199353694ddb629e2c1c4492..26d076d9a4d684ff3828c8393f1907c40f953e4b 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -539,16 +539,8 @@ class ADTRUSTInstance(service.Service):
                     add_rr(zone, win_srv, "SRV", rec)
 
     def __configure_selinux_for_smbd(self):
-        selinux = False
-        try:
-            if (os.path.exists('/usr/sbin/selinuxenabled')):
-                ipautil.run(["/usr/sbin/selinuxenabled"])
-                selinux = True
-        except ipautil.CalledProcessError:
-            # selinuxenabled returns 1 if not enabled
-            pass
 
-        if selinux:
+        if ipaservices.is_selinux_enabled():
             # Don't assume all booleans are available
             sebools = []
             for var in self.selinux_booleans:
diff --git a/ipaserver/install/httpinstance.py b/ipaserver/install/httpinstance.py
index c34073546b34161723479e295a43a6f03a34edf5..f9beb516a98975a31e109cd5a41e6cd41dd1921e 100644
--- a/ipaserver/install/httpinstance.py
+++ b/ipaserver/install/httpinstance.py
@@ -130,16 +130,7 @@ class HTTPInstance(service.Service):
 
             return args
 
-        selinux = False
-        try:
-            if (os.path.exists('/usr/sbin/selinuxenabled')):
-                ipautil.run(["/usr/sbin/selinuxenabled"])
-                selinux = True
-        except ipautil.CalledProcessError:
-            # selinuxenabled returns 1 if not enabled
-            pass
-
-        if selinux:
+        if ipaservices.is_selinux_enabled():
             # Don't assume all vars are available
             updated_vars = {}
             failed_vars = {}
-- 
1.8.1.4

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to