From 5ed5f873dc21e6c358ac70b6e21d710fdb391ce2 Mon Sep 17 00:00:00 2001
From: Mark Reynolds <mreynolds@redhat.com>
Date: Dec 10 2018 17:26:25 +0000
Subject: Ticket 50056 - Fix CLI/UI bugs


Description:  Fix several issues discovered during QE testing

https://bugzilla.redhat.com/show_bug.cgi?id=1654101 - dscreate issues
https://bugzilla.redhat.com/show_bug.cgi?id=1654585 - dsidm sys ext error
https://bugzilla.redhat.com/show_bug.cgi?id=1654105 - dsconf related issues
https://bugzilla.redhat.com/show_bug.cgi?id=1654116 - dsctl remove "confirm with "Yes"
https://bugzilla.redhat.com/show_bug.cgi?id=1654134 - backups fixed
https://bugzilla.redhat.com/show_bug.cgi?id=1654451 - dscreate permissions and selinux issues
https://bugzilla.redhat.com/show_bug.cgi?id=1654566 - dbtasks no attr _instance
https://bugzilla.redhat.com/show_bug.cgi?id=1631461 - selinux reserved ports
https://bugzilla.redhat.com/show_bug.cgi?id=1654518 - issues with selinux ports
https://bugzilla.redhat.com/show_bug.cgi?id=1654581 - dsidm sys ext error
https://bugzilla.redhat.com/show_bug.cgi?id=1654577 - check if backup already exists
https://bugzilla.redhat.com/show_bug.cgi?id=1654693 - add password option for dsconf tools
https://pagure.io/389-ds-base/issue/50056 - dscreate defaults for instance name

https://pagure.io/389-ds-base/issue/50056

Reviewed by: spichugi(Thanks!)

---

diff --git a/src/cockpit/389-console/src/servers.js b/src/cockpit/389-console/src/servers.js
index 8dbd1bf..10c4715 100644
--- a/src/cockpit/389-console/src/servers.js
+++ b/src/cockpit/389-console/src/servers.js
@@ -404,6 +404,39 @@ function save_config() {
   }
 }
 
+function do_backup(server_inst, backup_name) {
+  var cmd = ['status-dirsrv', server_inst];
+  $("#backup-spinner").show();
+  cockpit.spawn(cmd, { superuser: true}).
+  done(function() {
+    var cmd = [DSCONF, server_inst, 'backup', 'create',  backup_name];
+    log_cmd('#ds-backup-btn (click)', 'Backup server instance', cmd);
+    cockpit.spawn(cmd, { superuser: true, "err": "message", "environ": [ENV]}).
+    done(function(data) {
+      $("#backup-spinner").hide();
+      popup_success("Backup has been created");
+      $("#backup-form").modal('toggle');
+    }).
+    fail(function(data) {
+      $("#backup-spinner").hide();
+      popup_err("Failed to backup the server", data.message);
+    })
+  }).
+  fail(function() {
+    var cmd = [DSCTL, server_inst, 'db2bak', backup_name];
+    log_cmd('#ds-backup-btn (click)', 'Backup server instance (offline)', cmd);
+    cockpit.spawn(cmd, { superuser: true, "err": "message", "environ": [ENV]}).
+    done(function(data) {
+      $("#backup-spinner").hide();
+      popup_success("Backup has been created");
+      $("#backup-form").modal('toggle');
+    }).
+    fail(function(data) {
+      $("#backup-spinner").hide();
+      popup_err("Failed to backup the server", data.message);
+    });
+  });
+}
 
 /*
  * load the server config pages
@@ -1253,36 +1286,30 @@ $(document).ready( function() {
                            "Backups are written to the server's backup directory (nsslapd-bakdir)");
         return;
       }
-      var cmd = ['status-dirsrv', server_inst];
-      $("#backup-spinner").show();
-      cockpit.spawn(cmd, { superuser: true}).
-      done(function() {
-        var cmd = [DSCONF, server_inst, 'backup', 'create',  backup_name];
-        log_cmd('#ds-backup-btn (click)', 'Backup server instance', cmd);
-        cockpit.spawn(cmd, { superuser: true, "err": "message", "environ": [ENV]}).
-        done(function(data) {
-          $("#backup-spinner").hide();
-          popup_success("Backup has been created");
-          $("#backup-form").modal('toggle');
-        }).
-        fail(function(data) {
-          $("#backup-spinner").hide();
-          popup_err("Failed to backup the server", data.message);
-        })
-      }).
-      fail(function() {
-        var cmd = [DSCTL, server_inst, 'db2bak', backup_name];
-        log_cmd('#ds-backup-btn (click)', 'Backup server instance (offline)', cmd);
-        cockpit.spawn(cmd, { superuser: true, "err": "message", "environ": [ENV]}).
-        done(function(data) {
-          $("#backup-spinner").hide();
-          popup_success("Backup has been created");
-          $("#backup-form").modal('toggle');
-        }).
-        fail(function(data) {
-          $("#backup-spinner").hide();
-          popup_err("Failed to backup the server", data.message);
-        });
+      
+      // First check if backup name is already used
+      var check_cmd = [DSCTL, '-j', server_id, 'backups'];
+      log_cmd('#restore-server-btn (click)', 'Restore server instance', check_cmd);
+      cockpit.spawn(check_cmd, { superuser: true, "err": "message", "environ": [ENV]}).done(function(data) {
+        var obj = JSON.parse(data);
+        var found_backup = false;
+        for (var i = 0; i < obj.items.length; i++) {
+          if (obj.items[i][0] == backup_name) {
+            found_backup = true;
+            break;
+          }
+        }
+        if (found_backup) {
+          popup_confirm("A backup already exists with this name, replace it?", "Confirmation", function (yes) {
+            if (yes) {
+              do_backup(server_inst, backup_name);
+            } else {
+              return;
+            }
+          });
+        } else {
+          do_backup(server_inst, backup_name);
+        }
       });
     });
 
@@ -1292,7 +1319,7 @@ $(document).ready( function() {
 
     /* Restore.  load restore table with current backups */
     $("#restore-server-btn").on('click', function () {
-      var cmd = [DSCTL, server_id, '-j', 'backups'];
+      var cmd = [DSCTL, '-j', server_id, 'backups'];
       log_cmd('#restore-server-btn (click)', 'Restore server instance', cmd);
       cockpit.spawn(cmd, { superuser: true, "err": "message", "environ": [ENV]}).done(function(data) {
         var backup_btn = "<button class=\"btn btn-default restore-btn\" type=\"button\">Restore</button>";
diff --git a/src/lib389/cli/dsconf b/src/lib389/cli/dsconf
index c963467..33b3b5a 100755
--- a/src/lib389/cli/dsconf
+++ b/src/lib389/cli/dsconf
@@ -51,6 +51,18 @@ parser.add_argument('-D', '--binddn',
         help="The account to bind as for executing operations",
         default=None
     )
+parser.add_argument('-w', '--bindpw',
+        help="Password for binddn",
+        default=None
+    )
+parser.add_argument('-W', '--prompt',
+        action='store_true', default=False,
+        help="Prompt for password for the bind DN"
+    )
+parser.add_argument('-y', '--pwdfile',
+        help="Specifies a file containing the password for the binddn",
+        default=None
+    )
 parser.add_argument('-b', '--basedn',
         help="Basedn (root naming context) of the instance to manage",
         default=None
@@ -126,7 +138,7 @@ if __name__ == '__main__':
     inst = None
     result = False
     try:
-        inst = connect_instance(dsrc_inst=dsrc_inst, verbose=args.verbose)
+        inst = connect_instance(dsrc_inst=dsrc_inst, verbose=args.verbose, args=args)
         result = args.func(inst, None, log, args)
         if args.verbose:
             log.info("Command successful.")
diff --git a/src/lib389/cli/dsidm b/src/lib389/cli/dsidm
index e67a081..386bc11 100755
--- a/src/lib389/cli/dsidm
+++ b/src/lib389/cli/dsidm
@@ -43,6 +43,18 @@ parser.add_argument('-D', '--binddn',
         help="The account to bind as for executing operations",
         default=None
     )
+parser.add_argument('-w', '--bindpw',
+        help="Password for binddn",
+        default=None
+    )
+parser.add_argument('-W', '--prompt',
+        action='store_true', default=False,
+        help="Prompt for password for binddn"
+    )
+parser.add_argument('-y', '--pwdfile',
+        help="Specifies a file containing the password for the bind DN",
+        default=None
+    )
 parser.add_argument('-Z', '--starttls',
         help="Connect with StartTLS",
         default=False, action='store_true'
@@ -98,7 +110,7 @@ if __name__ == '__main__':
 
     if dsrc_inst['basedn'] is None:
         log.error("Must provide a basedn!")
-        sys.ext(1)
+        sys.exit(1)
 
     if not args.verbose:
         signal.signal(signal.SIGINT, signal_handler)
@@ -109,7 +121,7 @@ if __name__ == '__main__':
     inst = None
     result = False
     try:
-        inst = connect_instance(dsrc_inst=dsrc_inst, verbose=args.verbose)
+        inst = connect_instance(dsrc_inst=dsrc_inst, verbose=args.verbose, args=args)
         result = args.func(inst, dsrc_inst['basedn'], log, args)
         if args.verbose:
             log.info("Command successful.")
diff --git a/src/lib389/lib389/__init__.py b/src/lib389/lib389/__init__.py
index ad09fc4..62fc324 100644
--- a/src/lib389/lib389/__init__.py
+++ b/src/lib389/lib389/__init__.py
@@ -2856,9 +2856,9 @@ class DirSrv(SimpleLDAPObject, object):
             cmd.append('-a')
             tnow = datetime.now().strftime("%Y_%m_%d_%H_%M_%S")
             if bename:
-                ldifname = os.path.join(self._instance.ds_paths.ldif_dir, "%s-%s-%s.ldif" % (self._instance.serverid, be_name, tnow))
+                ldifname = os.path.join(self.ds_paths.ldif_dir, "%s-%s-%s.ldif" % (self.serverid, be_name, tnow))
             else:
-                ldifname = os.path.join(self._instance.ds_paths.ldif_dir, "%s-%s.ldif" % (self._instance.serverid, tnow))
+                ldifname = os.path.join(self.ds_paths.ldif_dir, "%s-%s.ldif" % (self.serverid, tnow))
             cmd.append(ldifname)
         try:
             result = subprocess.check_output(cmd, encoding='utf-8')
diff --git a/src/lib389/lib389/cli_base/__init__.py b/src/lib389/lib389/cli_base/__init__.py
index 664a9be..b4acb49 100644
--- a/src/lib389/lib389/cli_base/__init__.py
+++ b/src/lib389/lib389/cli_base/__init__.py
@@ -79,7 +79,7 @@ def _warn(data, msg=None):
     return data
 
 
-def connect_instance(dsrc_inst, verbose):
+def connect_instance(dsrc_inst, verbose, args):
     dsargs = dsrc_inst['args']
     if '//' not in dsargs['ldapurl']:
         # Connecting to the local instance
@@ -99,10 +99,25 @@ def connect_instance(dsrc_inst, verbose):
             raise ValueError("Could not find configuration for instance: " + dsargs['ldapurl'])
     ds = DirSrv(verbose=verbose)
     ds.allocate(dsargs)
-    if not ds.can_autobind() and dsrc_inst['binddn'] is not None:
+
+    if args.pwdfile is not None or args.bindpw is not None or args.prompt is True:
+        if args.pwdfile is not None:
+            # Read password from file
+            try:
+                with open(args.pwdfile, "r") as f:
+                    dsargs[SER_ROOT_PW] = f.readline().rstrip()
+            except EnvironmentError as e:
+                raise ValueError("Failed to open password file: " + str(e))
+        elif args.bindpw is not None:
+            # Password provided
+            dsargs[SER_ROOT_PW] = args.bindpw
+        else:
+            # No password or we chose to prompt
+            dsargs[SER_ROOT_PW] = getpass("Enter password for {} on {}: ".format(dsrc_inst['binddn'], dsrc_inst['uri']))
+    elif not ds.can_autobind():
+        # No LDAPI, prompt for password
         dsargs[SER_ROOT_PW] = getpass("Enter password for {} on {}: ".format(dsrc_inst['binddn'], dsrc_inst['uri']))
-    elif not ds.can_autobind() and dsrc_inst['binddn'] is None:
-        raise Exception("Must provide a binddn to connect with")
+
     ds.allocate(dsargs)
     ds.open(saslmethod=dsrc_inst['saslmech'],
             certdir=dsrc_inst['tls_cacertdir'],
diff --git a/src/lib389/lib389/cli_conf/plugin.py b/src/lib389/lib389/cli_conf/plugin.py
index 8f59433..39a9fed 100644
--- a/src/lib389/lib389/cli_conf/plugin.py
+++ b/src/lib389/lib389/cli_conf/plugin.py
@@ -102,9 +102,11 @@ def plugin_enable(inst, basedn, log, args):
     dn = _get_arg(args.dn, msg="Enter plugin dn to enable")
     mc = MANY(inst, basedn)
     o = mc.get(dn=dn)
-    o.enable()
-    o_str = o.display()
-    print('Enabled %s', o_str)
+    if o.status():
+        print('Plugin already enabled')
+    else:
+        o.enable()
+        print('Enabled plugin')
 
 
 # Plugin disable
@@ -114,9 +116,11 @@ def plugin_disable(inst, basedn, log, args, warn=True):
         _warn(dn, msg="Disabling %s %s" % (SINGULAR.__name__, dn))
     mc = MANY(inst, basedn)
     o = mc.get(dn=dn)
-    o.disable()
-    o_str = o.display()
-    print('Disabled %s', o_str)
+    if not o.state():
+        print("Plugin already disabled")
+    else:
+        o.disable()
+        print('Disabled plugin')
 
 
 # Plugin configure?
@@ -132,22 +136,28 @@ def generic_show(inst, basedn, log, args):
 
 def generic_enable(inst, basedn, log, args):
     plugin = args.plugin_cls(inst)
-    plugin.enable()
-    print("Enabled %s", plugin.rdn)
+    if plugin.status():
+        print("Plugin '%s' already enabled" % plugin.rdn)
+    else:
+        plugin.enable()
+        print("Enabled plugin '%s'" % plugin.rdn)
 
 
 def generic_disable(inst, basedn, log, args):
     plugin = args.plugin_cls(inst)
-    plugin.disable()
-    print("Disabled %s", plugin.rdn)
+    if not plugin.status():
+        print("Plugin '%s' already disabled " % plugin.rdn)
+    else:
+        plugin.disable()
+        print("Disabled plugin '%s'" % plugin.rdn)
 
 
 def generic_status(inst, basedn, log, args):
     plugin = args.plugin_cls(inst)
     if plugin.status() is True:
-        print("%s is enabled", plugin.rdn)
+        print("Plugin '%s' is enabled" % plugin.rdn)
     else:
-        print("%s is disabled", plugin.rdn)
+        print("Plugin '%s' is disabled" % plugin.rdn)
 
 
 def add_generic_plugin_parsers(subparser, plugin_cls):
diff --git a/src/lib389/lib389/cli_ctl/instance.py b/src/lib389/lib389/cli_ctl/instance.py
index 4825a61..ae17307 100644
--- a/src/lib389/lib389/cli_ctl/instance.py
+++ b/src/lib389/lib389/cli_ctl/instance.py
@@ -135,7 +135,7 @@ def instance_remove_all(log, args):
 
     inst_names = get_instance_list(args.remove_all)
     if len(inst_names) > 0:
-        answer = input("Are you sure you want to remove all the Directory Server instances? (Yes/no): ")
+        answer = input("Are you sure you want to remove all the Directory Server instances?  Enter \"Yes\" to continue: ")
         if answer != 'Yes':
             print("Aborted removal of all instances")
             return
diff --git a/src/lib389/lib389/instance/remove.py b/src/lib389/lib389/instance/remove.py
index 21da083..70ebfcc 100644
--- a/src/lib389/lib389/instance/remove.py
+++ b/src/lib389/lib389/instance/remove.py
@@ -12,7 +12,7 @@ import subprocess
 from lib389.utils import selinux_label_port
 
 
-def remove_ds_instance(dirsrv):
+def remove_ds_instance(dirsrv, force=False):
     """
     This will delete the instance as it is define. This must be a local instance.
     """
@@ -37,6 +37,7 @@ def remove_ds_instance(dirsrv):
     remove_paths['log_dir'] = dirsrv.ds_paths.log_dir
     # remove_paths['run_dir'] = dirsrv.ds_paths.run_dir
     remove_paths['tmpfiles_d'] = dirsrv.ds_paths.tmpfiles_d + "/dirsrv-" + dirsrv.serverid + ".conf"
+    remove_paths['inst_dir'] = dirsrv.ds_paths.inst_dir
 
     marker_path = "%s/sysconfig/dirsrv-%s" % (dirsrv.ds_paths.sysconf_dir, dirsrv.serverid)
 
@@ -66,8 +67,9 @@ def remove_ds_instance(dirsrv):
     for path_k in remove_paths:
         _log.debug("Removing %s" % remove_paths[path_k])
         shutil.rmtree(remove_paths[path_k], ignore_errors=True)
+
     # Remove parent (/var/lib/dirsrv/slapd-INST)
-    shutil.rmtree(remove_paths['db_dir'].replace('db', ''))
+    shutil.rmtree(remove_paths['db_dir'].replace('db', ''), ignore_errors=True)
 
     # Finally remove the sysconfig marker.
     os.remove(marker_path)
@@ -78,8 +80,16 @@ def remove_ds_instance(dirsrv):
     subprocess.check_call(["systemctl", "disable", "dirsrv@{}".format(dirsrv.serverid)])
 
     # Remove selinux port label
-    if dirsrv.port is not None:
+    _log.debug("Removing the port label")
+    try:
         selinux_label_port(dirsrv.port, remove_label=True)
+    except ValueError as e:
+        if force:
+            pass
+        else:
+            _log.error(str(e))
+            raise e
+
     if dirsrv.sslport is not None:
         selinux_label_port(dirsrv.sslport, remove_label=True)
 
diff --git a/src/lib389/lib389/instance/setup.py b/src/lib389/lib389/instance/setup.py
index 2033f5c..1a25a4e 100644
--- a/src/lib389/lib389/instance/setup.py
+++ b/src/lib389/lib389/instance/setup.py
@@ -16,7 +16,8 @@ import socket
 import subprocess
 import getpass
 import configparser
-from lib389 import _ds_shutil_copytree
+import selinux
+from lib389 import _ds_shutil_copytree, DirSrv
 from lib389._constants import *
 from lib389.properties import *
 from lib389.passwd import password_hash, password_generate
@@ -26,6 +27,7 @@ from lib389.configurations.sample import create_base_domain
 from lib389.instance.options import General2Base, Slapd2Base, Backend2Base
 from lib389.paths import Paths
 from lib389.saslmap import SaslMappings
+from lib389.instance.remove import remove_ds_instance
 from lib389.utils import (
     assert_c,
     is_a_dn,
@@ -202,6 +204,36 @@ class SetupDs(object):
         else:
             assert_c(False, "Unsupported config_version in section [general]")
 
+    def _remove_failed_install(self, serverid):
+        """The install failed, remove the scraps
+        :param serverid - The server ID of the instance
+        """
+        inst = DirSrv()
+
+        # Allocate the instance based on name
+        insts = []
+        insts = inst.list(serverid=serverid)
+
+        if len(insts) != 1:
+            log.error("No such instance to remove {}".format(serverid))
+            return
+        inst.allocate(insts[0])
+        remove_ds_instance(inst, force=True)
+
+    def _server_id_taken(self, serverid, prefix='/usr'):
+        """Check if instance name is already taken
+        :param serverid - name of the server instance
+        :param prefix - name of prefix build location
+        :return True - if the serfver id is already in use
+                False - if the server id is available
+        """
+        if prefix != "/usr":
+            inst_dir = prefix + "/etc/dirsrv/slapd-" + serverid
+        else:
+            inst_dir = "/etc/dirsrv/slapd-" + serverid
+
+        return os.path.isdir(inst_dir)
+
     def create_from_cli(self):
         # Ask questions to generate general, slapd, and backends
         print('Install Directory Server (interactive mode)')
@@ -268,8 +300,16 @@ class SetupDs(object):
         # Instance name - adjust defaults once set
         while 1:
             slapd['instance_name'] = general['full_machine_name'].split('.', 1)[0]
+
+            # Check if default server id is taken
+            if self._server_id_taken(slapd['instance_name'], prefix=slapd['prefix']):
+                slapd['instance_name'] = ""
+
             val = input('\nEnter the instance name [{}]: '.format(slapd['instance_name']))
             if val != "":
+                if not all(ord(c) < 128 for c in val):
+                    print("Server identifier can not contain non ascii characters")
+                    continue
                 if ' ' in val:
                     print("Server identifier can not contain a space")
                     continue
@@ -284,21 +324,18 @@ class SetupDs(object):
                     continue
 
                 # Check if server id is taken
-                inst_dir = slapd['config_dir'] + "/" + val
-                if os.path.isdir(inst_dir):
+                if self._server_id_taken(val, prefix=slapd['prefix']):
                     print("Server identifier \"{}\" is already taken, please choose a new name".format(val))
                     continue
 
                 # instance name is good
                 slapd['instance_name'] = val
                 break
+            elif slapd['instance_name'] == "":
+                continue
             else:
                 # Check if default server id is taken
-                if slapd['prefix'] != "/usr":
-                    inst_dir = slapd['prefix'] + slapd['config_dir'] + "/" + slapd['instance_name']
-                else:
-                    inst_dir = slapd['config_dir'] + "/" + slapd['instance_name']
-                if os.path.isdir(inst_dir):
+                if self._server_id_taken(slapd['instance_name'], prefix=slapd['prefix']):
                     print("Server identifier \"{}\" is already taken, please choose a new name".format(slapd['instance_name']))
                     continue
                 break
@@ -404,21 +441,22 @@ class SetupDs(object):
                 break
 
         # Add sample entries?
-        while 1:
-            val = input("\nCreate sample entries in the suffix [no]: ".format(suffix))
-            if val != "":
-                if val.lower() == "no" or val.lower() == "n":
-                    break
-                if val.lower() == "yes" or val.lower() == "y":
-                    backend['sample_entries'] = INSTALL_LATEST_CONFIG
+        if len(backends) > 0:
+            while 1:
+                val = input("\nCreate sample entries in the suffix [no]: ".format(suffix))
+                if val != "":
+                    if val.lower() == "no" or val.lower() == "n":
+                        break
+                    if val.lower() == "yes" or val.lower() == "y":
+                        backend['sample_entries'] = INSTALL_LATEST_CONFIG
+                        break
+
+                    # Unknown value
+                    print ("Value \"{}\" is invalid, please use \"yes\" or \"no\"".format(val))
+                    continue
+                else:
                     break
 
-                # Unknown value
-                print ("Value \"{}\" is invalid, please use \"yes\" or \"no\"".format(val))
-                continue
-            else:
-                break
-
         # Are you ready?
         while 1:
             val = input('\nAre you ready to install? [no]: ')
@@ -574,7 +612,13 @@ class SetupDs(object):
             self.log.info("NOOP: Dry run requested")
         else:
             # Actually trigger the installation.
-            self._install_ds(general, slapd, backends)
+            try:
+                self._install_ds(general, slapd, backends)
+            except ValueError as e:
+                self.log.fatal("Error: " + str(e) + ", removing incomplete installation...")
+                self._remove_failed_install(slapd['instance_name'])
+                raise ValueError("Instance creation failed!")
+
             # Call the child api to do anything it needs.
             self._install(extra)
         if self.verbose:
@@ -596,10 +640,12 @@ class SetupDs(object):
             for line in template_init.readlines():
                 initconfig += line.replace('{{', '{', 1).replace('}}', '}', 1).replace('-', '_')
         try:
-            os.makedirs("%s" % slapd['initconfig_dir'], mode=0o775)
+            # /etc/sysconfig
+            os.makedirs("%s" % slapd['initconfig_dir'], mode=0o770)
         except FileExistsError:
             pass
-        with open("%s/dirsrv-%s" % (slapd['initconfig_dir'], slapd['instance_name']), 'w') as f:
+        sysconfig_filename = "%s/dirsrv-%s" % (slapd['initconfig_dir'], slapd['instance_name'])
+        with open(sysconfig_filename, 'w') as f:
             f.write(initconfig.format(
                 SERVER_DIR=slapd['lib_dir'],
                 SERVERBIN_DIR=slapd['sbin_dir'],
@@ -609,17 +655,25 @@ class SetupDs(object):
                 DS_ROOT='',
                 PRODUCT_NAME='slapd',
             ))
+        os.chmod(sysconfig_filename, 0o440)
+        os.chown(sysconfig_filename, slapd['user_uid'], slapd['group_gid'])
 
         # Create all the needed paths
-        # we should only need to make bak_dir, cert_dir, config_dir, db_dir, ldif_dir, lock_dir, log_dir, run_dir? schema_dir,
+        # we should only need to make bak_dir, cert_dir, config_dir, db_dir, ldif_dir, lock_dir, log_dir, run_dir?
         for path in ('backup_dir', 'cert_dir', 'config_dir', 'db_dir', 'ldif_dir', 'lock_dir', 'log_dir', 'run_dir'):
             if self.verbose:
                 self.log.info("ACTION: creating %s", slapd[path])
             try:
-                os.makedirs(slapd[path], mode=0o775)
+                os.umask(0o007)  # For parent dirs that get created -> sets 770 for perms
+                os.makedirs(slapd[path], mode=0o770)
             except OSError:
                 pass
             os.chown(slapd[path], slapd['user_uid'], slapd['group_gid'])
+
+        # /var/lock/dirsrv needs special attention...
+        parentdir = os.path.abspath(os.path.join(slapd['lock_dir'], os.pardir))
+        os.chown(parentdir, slapd['user_uid'], slapd['group_gid'])
+
         ### Warning! We need to down the directory under db too for .restore to work.
         # See dblayer.c for more!
         db_parent = os.path.join(slapd['db_dir'], '..')
@@ -632,18 +686,21 @@ class SetupDs(object):
 
         _ds_shutil_copytree(os.path.join(slapd['sysconf_dir'], 'dirsrv/schema'), slapd['schema_dir'])
         os.chown(slapd['schema_dir'], slapd['user_uid'], slapd['group_gid'])
+        os.chmod(slapd['schema_dir'], 0o770)
 
         # Copy in the collation
         srcfile = os.path.join(slapd['sysconf_dir'], 'dirsrv/config/slapd-collations.conf')
         dstfile = os.path.join(slapd['config_dir'], 'slapd-collations.conf')
         shutil.copy2(srcfile, dstfile)
         os.chown(dstfile, slapd['user_uid'], slapd['group_gid'])
+        os.chmod(dstfile, 0o440)
 
         # Copy in the certmap configuration
         srcfile = os.path.join(slapd['sysconf_dir'], 'dirsrv/config/certmap.conf')
         dstfile = os.path.join(slapd['config_dir'], 'certmap.conf')
         shutil.copy2(srcfile, dstfile)
         os.chown(dstfile, slapd['user_uid'], slapd['group_gid'])
+        os.chmod(dstfile, 0o440)
 
         # If we are on the correct platform settings, systemd
         if general['systemd'] and not self.containerised:
@@ -758,10 +815,17 @@ class SetupDs(object):
                 # Set selinux port label
                 selinux_label_port(slapd['secure_port'])
 
-        ## LAST CHANCE, FIX PERMISSIONS.
-        # Selinux fixups?
-        # Restorecon of paths?
-        if not self.containerised and general['selinux']:
+        # Do selinux fixups
+        if not self.containerised and general['selinux'] and selinux.is_selinux_enabled():
+            selinux_paths = ('backup_dir', 'cert_dir', 'config_dir', 'db_dir', 'ldif_dir',
+                             'lock_dir', 'log_dir', 'run_dir', 'schema_dir', 'tmp_dir')
+            for path in selinux_paths:
+                try:
+                    selinux.restorecon(slapd[path], recursive=True)
+                except:
+                    self.log.debug("Failed to run restorecon on: " + slapd[path])
+                    pass
+
             selinux_label_port(slapd['port'])
 
         # Start the server
diff --git a/src/lib389/lib389/nss_ssl.py b/src/lib389/lib389/nss_ssl.py
index 0c71e9c..ccd15d0 100644
--- a/src/lib389/lib389/nss_ssl.py
+++ b/src/lib389/lib389/nss_ssl.py
@@ -106,6 +106,7 @@ class NssSsl(object):
         noise = password_generate(256)
         with open(fpath, 'w') as f:
             f.write(noise)
+        os.chmod(fpath, 0o660)
 
     def reinit(self):
         """
@@ -122,12 +123,17 @@ class NssSsl(object):
         # In the future we may add the needed option to avoid writing the pin
         # files.
         # Write the pin.txt, and the pwdfile.txt
-        if not os.path.exists('%s/%s' % (self._certdb, PIN_TXT)):
-            with open('%s/%s' % (self._certdb, PIN_TXT), 'w') as f:
+        pin_file = '%s/%s' % (self._certdb, PIN_TXT)
+        if not os.path.exists(pin_file):
+            with open(pin_file, 'w') as f:
                 f.write('Internal (Software) Token:%s' % self.dbpassword)
-        if not os.path.exists('%s/%s' % (self._certdb, PWD_TXT)):
-            with open('%s/%s' % (self._certdb, PWD_TXT), 'w') as f:
+            os.chmod(pin_file, 0o660)
+
+        pwd_text_file = '%s/%s' % (self._certdb, PWD_TXT)
+        if not os.path.exists(pwd_text_file):
+            with open(pwd_text_file, 'w') as f:
                 f.write('%s' % self.dbpassword)
+            os.chmod(pwd_text_file, 0o660)
 
         # Init the db.
         # 48886; This needs to be sql format ...
diff --git a/src/lib389/lib389/utils.py b/src/lib389/lib389/utils.py
index bfa5616..55ef7c7 100644
--- a/src/lib389/lib389/utils.py
+++ b/src/lib389/lib389/utils.py
@@ -184,7 +184,8 @@ def selinux_label_port(port, remove_label=False):
     :raises: ValueError: Error message
     """
 
-    if not selinux.is_selinux_enabled() or port == 389 or port == 636:
+    selinux_default_ports = [389, 636, 3268, 3269, 7389]
+    if not selinux.is_selinux_enabled() or port in selinux_default_ports:
         return
 
     label_set = False
@@ -204,12 +205,14 @@ def selinux_label_port(port, remove_label=False):
                     # The port is within the range, just return
                     return
             break
-        else:
+        elif not remove_label:
             # Port belongs to someone else (bad)
-            raise ValueError("Port " + port + " was already labelled with: " + policy['type'])
+            # This is only an issue during setting a label, not removing a label
+            raise ValueError("Port {} was already labelled with: ({})  Please choose a different port number".format(port, policy['type']))
 
     if (remove_label and label_set) or (not remove_label and not label_set):
         for i in range(3):
+
             try:
                 subprocess.check_call(["semanage", "port",
                                        "-d" if remove_label else "-a",

