URL: https://github.com/freeipa/freeipa/pull/2697
Author: tiran
 Title: #2697: LDAPClient: Prefer LDAPI connections in installer and simplify 
API
Action: opened

PR body:
"""
On several occasions the installer uses LDAP + simple_bind rather than LDAPI 
with EXTERNAL bind. LDAPI (LDAP over Unix socket) is more secure, a tiny bit 
faster, and is immune to some network/DNS related issues.

The changeset simplifies the LDAPClient API by introducing additional 
constructors. The new constructors are not only easier to use but also prevent 
some common pitfalls like LDAP connection without StartTSL. Several code paths 
now use ``LDAPClient.from_realm(realm_name)`` with external bind as root 
instead of LDAP with DM password.

### Add constructors to ldap client

Add LDAPClient.from_realm(), LDAPClient.from_hostname_secure(), and
LDAPClient.from_hostname_plain() constructors.

The simple_bind() method now also refuses to transmit a password over a
plain, unencrypted line.

LDAPClient.from_hostname_secure() uses start_tls and FreeIPA's CA cert
by default. The constructor also automatically disables start_tls for
ldaps and ldapi connections.

### Use new LDAPClient constructors

Replace get_ldap_uri() + LDAPClient() with new LDAPClient constructors
like LDAPClient.from_realm().

Some places now use LDAPI with external bind instead of LDAP with simple
bind. Although the FQDN *should* resolve to 127.0.0.1 / [::1], there is
no hard guarantee. The draft
tools.ietf.org/html/draft-west-let-localhost-be-localhost-04#section-5.1
specifies that applications must verify that the resulting IP is a
loopback API. LDAPI is always local and a bit more efficient, too.

The simple_bind() method also prevents the caller from sending a
password over an insecure line.

### Use LDAPS when installing CA on replica

On a replica, 389-DS is already configured for secure connections when
the CA is installed.

### Move realm_to_serverid/ldap_uri to ipaldap

The helper function realm_to_serverid() and realm_to_ldap_uri() are
useful outside the server installation framework. They are now in
ipapython.ipaldap along other helpers for LDAP handling in FreeIPA.
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/2697/head:pr2697
git checkout pr2697
From 446ea11ca3768ee7bf399e63636cbe78fbee9514 Mon Sep 17 00:00:00 2001
From: Christian Heimes <chei...@redhat.com>
Date: Thu, 29 Nov 2018 14:49:43 +0100
Subject: [PATCH 1/5] Move realm_to_serverid/ldap_uri to ipaldap

The helper function realm_to_serverid() and realm_to_ldap_uri() are
useful outside the server installation framework. They are now in
ipapython.ipaldap along other helpers for LDAP handling in FreeIPA.

Signed-off-by: Christian Heimes <chei...@redhat.com>
---
 install/tools/ipactl.in                       |  4 +--
 ipapython/ipaldap.py                          | 14 ++++++++++
 ipaserver/install/adtrustinstance.py          |  3 ++-
 ipaserver/install/bindinstance.py             |  3 ++-
 ipaserver/install/ca.py                       |  5 ++--
 ipaserver/install/custodiainstance.py         |  5 ++--
 ipaserver/install/dsinstance.py               |  7 ++---
 ipaserver/install/installutils.py             | 26 ++++++++++++-------
 ipaserver/install/ipa_backup.py               |  4 +--
 ipaserver/install/ipa_restore.py              |  8 +++---
 ipaserver/install/ipa_server_certinstall.py   |  3 ++-
 ipaserver/install/krbinstance.py              |  2 +-
 ipaserver/install/ldapupdate.py               |  2 +-
 ipaserver/install/plugins/upload_cacrt.py     |  2 +-
 ipaserver/install/server/install.py           |  6 ++---
 ipaserver/install/server/replicainstall.py    |  5 ++--
 ipaserver/install/server/upgrade.py           | 12 +++++----
 ipaserver/install/upgradeinstance.py          |  4 ++-
 .../test_integration/test_uninstallation.py   |  2 +-
 19 files changed, 74 insertions(+), 43 deletions(-)

diff --git a/install/tools/ipactl.in b/install/tools/ipactl.in
index 82e7412b07..68e93a6139 100644
--- a/install/tools/ipactl.in
+++ b/install/tools/ipactl.in
@@ -30,7 +30,7 @@ from ipaserver.install import service, installutils
 from ipaserver.install.dsinstance import config_dirname
 from ipaserver.install.installutils import is_ipa_configured, ScriptError
 from ipalib import api, errors
-from ipapython.ipaldap import LDAPClient
+from ipapython.ipaldap import LDAPClient, realm_to_serverid
 from ipapython.ipautil import wait_for_open_ports, wait_for_open_socket
 from ipapython.ipautil import run
 from ipapython import config
@@ -75,7 +75,7 @@ def is_dirsrv_debugging_enabled():
     returns True or False
     """
     debugging = False
-    serverid = installutils.realm_to_serverid(api.env.realm)
+    serverid = realm_to_serverid(api.env.realm)
     dselist = [config_dirname(serverid)]
     for dse in dselist:
         try:
diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 066f65dc43..78330a4d1a 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -39,12 +39,14 @@
 import ldap.sasl
 import ldap.filter
 from ldap.controls import SimplePagedResultsControl, GetEffectiveRightsControl
+import ldapurl
 import six
 
 # pylint: disable=ipa-forbidden-import
 from ipalib import errors, x509, _
 from ipalib.constants import LDAP_GENERALIZED_TIME_FORMAT
 # pylint: enable=ipa-forbidden-import
+from ipaplatform.paths import paths
 from ipapython.ipautil import format_netloc, CIDict
 from ipapython.dn import DN
 from ipapython.dnsutil import DNSName
@@ -90,6 +92,18 @@
     )
 
 
+def realm_to_serverid(realm_name):
+    """Convert Kerberos realm name to 389-DS server id"""
+    return "-".join(realm_name.split("."))
+
+
+def realm_to_ldapi_uri(realm_name):
+    """Get ldapi:// URI to 389-DS's Unix socket"""
+    serverid = realm_to_serverid(realm_name)
+    socketname = paths.SLAPD_INSTANCE_SOCKET_TEMPLATE % (serverid,)
+    return 'ldapi://' + ldapurl.ldapUrlEscape(socketname)
+
+
 def ldap_initialize(uri, cacertfile=None):
     """Wrapper around ldap.initialize()
 
diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py
index a21be5fba3..d117e18d58 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -40,6 +40,7 @@
 from ipalib import errors, api
 from ipalib.util import normalize_zone
 from ipapython.dn import DN
+from ipapython import ipaldap
 from ipapython import ipautil
 import ipapython.errors
 
@@ -178,7 +179,7 @@ def __setup_default_attributes(self):
 
         self.suffix = ipautil.realm_to_suffix(self.realm)
         self.ldapi_socket = "%%2fvar%%2frun%%2fslapd-%s.socket" % \
-                            installutils.realm_to_serverid(self.realm)
+                            ipaldap.realm_to_serverid(self.realm)
 
         # DN definitions
         self.trust_dn = DN(api.env.container_trusts, self.suffix)
diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py
index 41c42c4b9c..be32bffe1d 100644
--- a/ipaserver/install/bindinstance.py
+++ b/ipaserver/install/bindinstance.py
@@ -40,6 +40,7 @@
 from ipaserver.install import installutils
 from ipaserver.install import service
 from ipaserver.install import sysupgrade
+from ipapython import ipaldap
 from ipapython import ipautil
 from ipapython import dnsutil
 from ipapython.dnsutil import DNSName
@@ -803,7 +804,7 @@ def __setup_sub_dict(self):
 
         self.sub_dict = dict(
             FQDN=self.fqdn,
-            SERVER_ID=installutils.realm_to_serverid(self.realm),
+            SERVER_ID=ipaldap.realm_to_serverid(self.realm),
             SUFFIX=self.suffix,
             BINDKEYS_FILE=paths.NAMED_BINDKEYS_FILE,
             MANAGED_KEYS_DIR=paths.NAMED_MANAGED_KEYS_DIR,
diff --git a/ipaserver/install/ca.py b/ipaserver/install/ca.py
index b30fbed654..2f08b279e2 100644
--- a/ipaserver/install/ca.py
+++ b/ipaserver/install/ca.py
@@ -22,6 +22,7 @@
 from ipapython.install.core import group, knob, extend_knob
 from ipaserver.install import cainstance, bindinstance, dsinstance
 from ipapython import ipautil, certdb
+from ipapython import ipaldap
 from ipapython.admintool import ScriptError
 from ipaplatform import services
 from ipaplatform.paths import paths
@@ -209,7 +210,7 @@ def install_check(standalone, replica_config, options):
 
     if standalone:
         dirname = dsinstance.config_dirname(
-            installutils.realm_to_serverid(realm_name))
+            ipaldap.realm_to_serverid(realm_name))
         cadb = certs.CertDB(realm_name, nssdir=paths.PKI_TOMCAT_ALIAS_DIR,
                             subject_base=options._subject_base)
         dsdb = certs.CertDB(
@@ -343,7 +344,7 @@ def install_step_1(standalone, replica_config, options, custodia):
     #
     ca.setup_lightweight_ca_key_retrieval()
 
-    serverid = installutils.realm_to_serverid(realm_name)
+    serverid = ipaldap.realm_to_serverid(realm_name)
 
     if standalone and replica_config is None:
         dirname = dsinstance.config_dirname(serverid)
diff --git a/ipaserver/install/custodiainstance.py b/ipaserver/install/custodiainstance.py
index e856e851d5..383c17f4eb 100644
--- a/ipaserver/install/custodiainstance.py
+++ b/ipaserver/install/custodiainstance.py
@@ -5,6 +5,7 @@
 import enum
 import logging
 
+import ipapython.ipaldap
 from ipalib import api
 from ipaserver.secrets.kem import IPAKEMKeys, KEMLdap
 from ipaserver.secrets.client import CustodiaClient
@@ -104,7 +105,7 @@ def __init__(self, host_name=None, realm=None, custodia_peer=None):
     @property
     def ldap_uri(self):
         if self.custodia_peer is None:
-            return installutils.realm_to_ldapi_uri(self.realm)
+            return ipapython.ipaldap.realm_to_ldapi_uri(self.realm)
         else:
             return "ldap://{}".format(self.custodia_peer)
 
@@ -117,7 +118,7 @@ def __config_file(self):
             IPA_CUSTODIA_KEYS=paths.IPA_CUSTODIA_KEYS,
             IPA_CUSTODIA_SOCKET=paths.IPA_CUSTODIA_SOCKET,
             IPA_CUSTODIA_AUDIT_LOG=paths.IPA_CUSTODIA_AUDIT_LOG,
-            LDAP_URI=installutils.realm_to_ldapi_uri(self.realm),
+            LDAP_URI=ipapython.ipaldap.realm_to_ldapi_uri(self.realm),
             UID=httpd_info.pw_uid,
             GID=httpd_info.pw_gid
         )
diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index 05b8daa50d..33ab3d122b 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -294,7 +294,7 @@ def init_info(self, realm_name, fqdn, domain_name, dm_password,
                   idstart, idmax, pkcs12_info, ca_file=None,
                   setup_pkinit=False):
         self.realm = realm_name.upper()
-        self.serverid = installutils.realm_to_serverid(self.realm)
+        self.serverid = ipaldap.realm_to_serverid(self.realm)
         self.suffix = ipautil.realm_to_suffix(self.realm)
         self.fqdn = fqdn
         self.dm_password = dm_password
@@ -1217,7 +1217,8 @@ def add_ca_cert(self, cacert_fname, cacert_name=''):
         # shutdown the server
         self.stop()
 
-        dirname = config_dirname(installutils.realm_to_serverid(self.realm))
+        dirname = config_dirname(
+            ipaldap.realm_to_serverid(self.realm))
         certdb = certs.CertDB(
             self.realm,
             nssdir=dirname,
@@ -1362,7 +1363,7 @@ def request_service_keytab(self):
 
 def write_certmap_conf(realm, ca_subject):
     """(Re)write certmap.conf with given CA subject DN."""
-    serverid = installutils.realm_to_serverid(realm)
+    serverid = ipaldap.realm_to_serverid(realm)
     ds_dirname = config_dirname(serverid)
     certmap_filename = os.path.join(ds_dirname, "certmap.conf")
     shutil.copyfile(
diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index b0e8f93bf4..5408cf4e25 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -34,6 +34,7 @@
 import shutil
 import traceback
 import textwrap
+import warnings
 from contextlib import contextmanager
 from configparser import ConfigParser as SafeConfigParser
 from configparser import NoOptionError
@@ -41,16 +42,14 @@
 from dns import resolver, rdatatype
 from dns.exception import DNSException
 import ldap
-import ldapurl
 import six
 
 from ipalib.install import sysrestore
 from ipalib.install.kinit import kinit_password
 import ipaplatform
-from ipapython import ipautil, admintool, version
+from ipapython import ipautil, admintool, version, ipaldap
 from ipapython.admintool import ScriptError, SERVER_NOT_CONFIGURED  # noqa: E402
 from ipapython.certdb import EXTERNAL_CA_TRUST_FLAGS
-from ipapython.ipaldap import DIRMAN_DN, LDAPClient
 from ipalib.util import validate_hostname
 from ipalib import api, errors, x509
 from ipapython.dn import DN
@@ -338,9 +337,9 @@ def validate_dm_password_ldap(password):
     Validate DM password by attempting to connect to LDAP. api.env has to
     contain valid ldap_uri.
     """
-    client = LDAPClient(api.env.ldap_uri, cacert=paths.IPA_CA_CRT)
+    client = ipaldap.LDAPClient(api.env.ldap_uri, cacert=paths.IPA_CA_CRT)
     try:
-        client.simple_bind(DIRMAN_DN, password)
+        client.simple_bind(ipaldap.DIRMAN_DN, password)
     except errors.ACIError:
         raise ValueError("Invalid Directory Manager password")
     else:
@@ -1106,14 +1105,23 @@ def check_version():
     else:
         raise UpgradeMissingVersionError("no data_version stored")
 
+
 def realm_to_serverid(realm_name):
-    return "-".join(realm_name.split("."))
+    warnings.warn(
+        "Use 'ipapython.ipaldap.realm_to_serverid'",
+        DeprecationWarning,
+        stacklevel=2
+    )
+    return ipaldap.realm_to_serverid(realm_name)
 
 
 def realm_to_ldapi_uri(realm_name):
-    serverid = realm_to_serverid(realm_name)
-    socketname = paths.SLAPD_INSTANCE_SOCKET_TEMPLATE % (serverid,)
-    return 'ldapi://' + ldapurl.ldapUrlEscape(socketname)
+    warnings.warn(
+        "Use 'ipapython.ipaldap.realm_to_ldapi_uri'",
+        DeprecationWarning,
+        stacklevel=2
+    )
+    return ipaldap.realm_to_ldapi_uri(realm_name)
 
 
 def check_creds(options, realm_name):
diff --git a/ipaserver/install/ipa_backup.py b/ipaserver/install/ipa_backup.py
index a5cdf20ad6..90d16d68ff 100644
--- a/ipaserver/install/ipa_backup.py
+++ b/ipaserver/install/ipa_backup.py
@@ -320,7 +320,7 @@ def run(self):
                 logger.info('Stopping IPA services')
                 run([paths.IPACTL, 'stop'])
 
-            instance = installutils.realm_to_serverid(api.env.realm)
+            instance = ipaldap.realm_to_serverid(api.env.realm)
             if os.path.exists(paths.VAR_LIB_SLAPD_INSTANCE_DIR_TEMPLATE %
                               instance):
                 if os.path.exists(paths.SLAPD_INSTANCE_DB_DIR_TEMPLATE %
@@ -364,7 +364,7 @@ def add_instance_specific_data(self):
 
         NOTE: this adds some things that may not get backed up.
         '''
-        serverid = installutils.realm_to_serverid(api.env.realm)
+        serverid = ipaldap.realm_to_serverid(api.env.realm)
 
         for dir in [paths.ETC_DIRSRV_SLAPD_INSTANCE_TEMPLATE % serverid,
                     paths.VAR_LIB_DIRSRV_INSTANCE_SCRIPTS_TEMPLATE % serverid,
diff --git a/ipaserver/install/ipa_restore.py b/ipaserver/install/ipa_restore.py
index b6b02a9ef0..889dbf4b8b 100644
--- a/ipaserver/install/ipa_restore.py
+++ b/ipaserver/install/ipa_restore.py
@@ -465,7 +465,7 @@ def get_connection(self):
         '''
         Create an ldapi connection and bind to it using autobind as root.
         '''
-        instance_name = installutils.realm_to_serverid(api.env.realm)
+        instance_name = ipaldap.realm_to_serverid(api.env.realm)
 
         if not services.knownservices.dirsrv.is_running(instance_name):
             raise admintool.ScriptError(
@@ -879,7 +879,7 @@ def cert_restore_prepare(self):
         httpinstance.HTTPInstance().stop_tracking_certificates()
         try:
             dsinstance.DsInstance().stop_tracking_certificates(
-                installutils.realm_to_serverid(api.env.realm))
+                ipaldap.realm_to_serverid(api.env.realm))
         except (OSError, IOError):
             # When IPA is not installed, DS NSS DB does not exist
             pass
@@ -910,13 +910,13 @@ def init_api(self, **overrides):
         api.bootstrap(in_server=True, context='restore', **overrides)
         api.finalize()
 
-        self.instances = [installutils.realm_to_serverid(api.env.realm)]
+        self.instances = [ipaldap.realm_to_serverid(api.env.realm)]
         self.backends = ['userRoot', 'ipaca']
 
         # no IPA config means we are reinstalling from nothing so
         # there is nothing to test the DM password against.
         if os.path.exists(paths.IPA_DEFAULT_CONF):
-            instance_name = installutils.realm_to_serverid(api.env.realm)
+            instance_name = ipapython.ipaldap.realm_to_serverid(api.env.realm)
             if not services.knownservices.dirsrv.is_running(instance_name):
                 raise admintool.ScriptError(
                     "directory server instance is not running"
diff --git a/ipaserver/install/ipa_server_certinstall.py b/ipaserver/install/ipa_server_certinstall.py
index 7901f0a739..ac0134086e 100644
--- a/ipaserver/install/ipa_server_certinstall.py
+++ b/ipaserver/install/ipa_server_certinstall.py
@@ -30,6 +30,7 @@
 from ipapython import admintool
 from ipapython.certdb import NSSDatabase, get_ca_nickname
 from ipapython.dn import DN
+from ipapython import ipaldap
 from ipalib import api, errors
 from ipaserver.install import certs, dsinstance, installutils, krbinstance
 
@@ -125,7 +126,7 @@ def run(self):
         api.Backend.ldap2.disconnect()
 
     def install_dirsrv_cert(self):
-        serverid = installutils.realm_to_serverid(api.env.realm)
+        serverid = ipaldap.realm_to_serverid(api.env.realm)
         dirname = dsinstance.config_dirname(serverid)
 
         conn = api.Backend.ldap2
diff --git a/ipaserver/install/krbinstance.py b/ipaserver/install/krbinstance.py
index 850946afb4..4346149ca7 100644
--- a/ipaserver/install/krbinstance.py
+++ b/ipaserver/install/krbinstance.py
@@ -262,7 +262,7 @@ def __setup_sub_dict(self):
                              SUFFIX=self.suffix,
                              DOMAIN=self.domain,
                              HOST=self.host,
-                             SERVER_ID=installutils.realm_to_serverid(self.realm),
+                             SERVER_ID=ipaldap.realm_to_serverid(self.realm),
                              REALM=self.realm,
                              KRB5KDC_KADM5_ACL=paths.KRB5KDC_KADM5_ACL,
                              DICT_WORDS=paths.DICT_WORDS,
diff --git a/ipaserver/install/ldapupdate.py b/ipaserver/install/ldapupdate.py
index 55eabad431..1fe076ff38 100644
--- a/ipaserver/install/ldapupdate.py
+++ b/ipaserver/install/ldapupdate.py
@@ -279,7 +279,7 @@ def __init__(self, dm_password=None, sub_dict={},
             self.realm = api.env.realm
             suffix = ipautil.realm_to_suffix(self.realm) if self.realm else None
 
-        self.ldapuri = installutils.realm_to_ldapi_uri(self.realm)
+        self.ldapuri = ipaldap.realm_to_ldapi_uri(self.realm)
         if suffix is not None:
             assert isinstance(suffix, DN)
 
diff --git a/ipaserver/install/plugins/upload_cacrt.py b/ipaserver/install/plugins/upload_cacrt.py
index 763da1e142..16b5e80c6a 100644
--- a/ipaserver/install/plugins/upload_cacrt.py
+++ b/ipaserver/install/plugins/upload_cacrt.py
@@ -21,7 +21,7 @@
 
 from ipalib.install import certstore
 from ipaserver.install import certs, dsinstance
-from ipaserver.install.installutils import realm_to_serverid
+from ipapython.ipaldap import realm_to_serverid
 from ipalib import Registry, errors
 from ipalib import Updater
 from ipapython import certdb
diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py
index efccca77bd..b4c57752ff 100644
--- a/ipaserver/install/server/install.py
+++ b/ipaserver/install/server/install.py
@@ -22,6 +22,7 @@
 from ipapython import ipautil, version
 from ipapython.ipautil import (
     ipa_generate_password, run, user_input)
+from ipapython import ipaldap
 from ipapython.admintool import ScriptError
 from ipaplatform import services
 from ipaplatform.paths import paths
@@ -591,8 +592,7 @@ def install_check(installer):
 
     xmlrpc_uri = 'https://{0}/ipa/xml'.format(
                     ipautil.format_netloc(host_name))
-    ldapi_uri = 'ldapi://%2fvar%2frun%2fslapd-{0}.socket\n'.format(
-                    installutils.realm_to_serverid(realm_name))
+    ldapi_uri = ipaldap.realm_to_ldapi_uri(realm_name)
 
     # [global] section
     gopts = [
@@ -1166,7 +1166,7 @@ def uninstall(installer):
 
     # Note that this name will be wrong after the first uninstall.
     dirname = dsinstance.config_dirname(
-        installutils.realm_to_serverid(api.env.realm))
+        ipaldap.realm_to_serverid(api.env.realm))
     dirs = [dirname, paths.PKI_TOMCAT_ALIAS_DIR, paths.HTTPD_ALIAS_DIR]
     ids = certmonger.check_state(dirs)
     if ids:
diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index 1a173e64d0..5001606890 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -220,8 +220,7 @@ def create_ipa_conf(fstore, config, ca_enabled, master=None):
     else:
         xmlrpc_uri = 'https://{0}/ipa/xml'.format(
                         ipautil.format_netloc(config.host_name))
-    ldapi_uri = 'ldapi://%2fvar%2frun%2fslapd-{0}.socket\n'.format(
-                    installutils.realm_to_serverid(config.realm_name))
+    ldapi_uri = ipaldap.realm_to_ldapi_uri(config.realm_name)
 
     # [global] section
     gopts = [
@@ -802,7 +801,7 @@ def promote_check(installer):
     api.bootstrap(in_server=True,
                   context='installer',
                   confdir=paths.ETC_IPA,
-                  ldap_uri=installutils.realm_to_ldapi_uri(env.realm),
+                  ldap_uri=ipaldap.realm_to_ldapi_uri(env.realm),
                   xmlrpc_uri=xmlrpc_uri)
     # pylint: enable=no-member
     api.finalize()
diff --git a/ipaserver/install/server/upgrade.py b/ipaserver/install/server/upgrade.py
index 02ffaa10b9..65d7046cc3 100644
--- a/ipaserver/install/server/upgrade.py
+++ b/ipaserver/install/server/upgrade.py
@@ -17,6 +17,7 @@
 from contextlib import contextmanager
 from augeas import Augeas
 import dns.exception
+
 from ipalib import api, x509
 from ipalib.install import certmonger, sysrestore
 import SSSDConfig
@@ -27,6 +28,7 @@
 from ipaplatform import services
 from ipaplatform.tasks import tasks
 from ipapython import ipautil, version
+from ipapython import ipaldap
 from ipapython import dnsutil, directivesetter
 from ipapython.dn import DN
 from ipaplatform.constants import constants
@@ -939,7 +941,7 @@ def certificate_renewal_update(ca, ds, http):
     """
 
     template = paths.CERTMONGER_COMMAND_TEMPLATE
-    serverid = installutils.realm_to_serverid(api.env.realm)
+    serverid = ipaldap.realm_to_serverid(api.env.realm)
 
     requests = [
         {
@@ -1357,7 +1359,7 @@ def fix_schema_file_syntax():
         logger.info('Syntax already fixed')
         return
 
-    serverid = installutils.realm_to_serverid(api.env.realm)
+    serverid = ipaldap.realm_to_serverid(api.env.realm)
     ds_dir = dsinstance.config_dirname(serverid)
 
     # 1. 60ipadns.ldif: Add parenthesis to idnsRecord
@@ -1432,7 +1434,7 @@ def remove_ds_ra_cert(subject_base):
         return
 
     dbdir = dsinstance.config_dirname(
-        installutils.realm_to_serverid(api.env.realm))
+        ipaldap.realm_to_serverid(api.env.realm))
     dsdb = certs.CertDB(api.env.realm, nssdir=dbdir, subject_base=subject_base)
 
     nickname = 'CN=IPA RA,%s' % subject_base
@@ -1763,7 +1765,7 @@ def upgrade_configuration():
     fqdn = api.env.host
 
     # Ok, we are an IPA server, do the additional tests
-    ds_serverid = installutils.realm_to_serverid(api.env.realm)
+    ds_serverid = ipaldap.realm_to_serverid(api.env.realm)
     ds = dsinstance.DsInstance()
 
     # start DS, CA will not start without running DS, and cause error
@@ -2050,7 +2052,7 @@ def upgrade_configuration():
                         SUFFIX=krb.suffix,
                         DOMAIN=api.env.domain,
                         HOST=api.env.host,
-                        SERVER_ID=installutils.realm_to_serverid(krb.realm),
+                        SERVER_ID=ipaldap.realm_to_serverid(krb.realm),
                         REALM=krb.realm,
                         KRB5KDC_KADM5_ACL=paths.KRB5KDC_KADM5_ACL,
                         DICT_WORDS=paths.DICT_WORDS,
diff --git a/ipaserver/install/upgradeinstance.py b/ipaserver/install/upgradeinstance.py
index cb83d7bb18..e2b11b6140 100644
--- a/ipaserver/install/upgradeinstance.py
+++ b/ipaserver/install/upgradeinstance.py
@@ -25,9 +25,11 @@
 import shutil
 import random
 import traceback
+
 from ipalib import api
 from ipaplatform.paths import paths
 from ipaplatform import services
+from ipapython import ipaldap
 
 from ipaserver.install import installutils
 from ipaserver.install import schemaupdate
@@ -88,7 +90,7 @@ def __init__(self, realm_name, files=[], schema_files=[]):
             h = "%02x" % rand.randint(0,255)
             ext += h
         super(IPAUpgrade, self).__init__("dirsrv", realm_name=realm_name)
-        serverid = installutils.realm_to_serverid(realm_name)
+        serverid = ipaldap.realm_to_serverid(realm_name)
         self.filename = '%s/%s' % (paths.ETC_DIRSRV_SLAPD_INSTANCE_TEMPLATE % serverid, DSE)
         self.savefilename = '%s/%s.ipa.%s' % (paths.ETC_DIRSRV_SLAPD_INSTANCE_TEMPLATE % serverid, DSE, ext)
         self.files = files
diff --git a/ipatests/test_integration/test_uninstallation.py b/ipatests/test_integration/test_uninstallation.py
index 2b9b926065..4607e8b707 100644
--- a/ipatests/test_integration/test_uninstallation.py
+++ b/ipatests/test_integration/test_uninstallation.py
@@ -18,7 +18,7 @@
 from ipatests.pytest_ipa.integration import tasks
 from ipaplatform.paths import paths
 from ipaserver.install import dsinstance
-from ipaserver.install.installutils import realm_to_serverid
+from ipapython.ipaldap import realm_to_serverid
 
 
 class TestUninstallBase(IntegrationTest):

From febcab6e43088d8058f2ddeae06c3df5a96b0919 Mon Sep 17 00:00:00 2001
From: Christian Heimes <chei...@redhat.com>
Date: Fri, 30 Nov 2018 10:28:02 +0100
Subject: [PATCH 2/5] Add constructors to ldap client

Add LDAPClient.from_realm(), LDAPClient.from_hostname_secure(), and
LDAPClient.from_hostname_plain() constructors.

The simple_bind() method now also refuses to transmit a password over a
plain, unencrypted line.

LDAPClient.from_hostname_secure() uses start_tls and FreeIPA's CA cert
by default. The constructor also automatically disables start_tls for
ldaps and ldapi connections.

Signed-off-by: Christian Heimes <chei...@redhat.com>
---
 install/tools/ipa-replica-manage.in |  2 -
 ipapython/ipaldap.py                | 63 +++++++++++++++++++++++------
 ipaserver/install/ipa_backup.py     |  4 +-
 makeaci.in                          |  2 +-
 4 files changed, 54 insertions(+), 17 deletions(-)

diff --git a/install/tools/ipa-replica-manage.in b/install/tools/ipa-replica-manage.in
index 35d412c2e2..67f678d44a 100644
--- a/install/tools/ipa-replica-manage.in
+++ b/install/tools/ipa-replica-manage.in
@@ -1218,7 +1218,6 @@ def re_initialize(realm, thishost, fromhost, dirman_passwd, nolookup=False):
         # we did not replicate memberOf, do so now.
         if not agreement.single_value.get('nsDS5ReplicatedAttributeListTotal'):
             ds = dsinstance.DsInstance(realm_name=realm)
-            ds.ldapi = os.getegid() == 0
             ds.init_memberof()
 
 def force_sync(realm, thishost, fromhost, dirman_passwd, nolookup=False):
@@ -1238,7 +1237,6 @@ def force_sync(realm, thishost, fromhost, dirman_passwd, nolookup=False):
         repl.force_sync(repl.conn, fromhost)
     else:
         ds = dsinstance.DsInstance(realm_name=realm)
-        ds.ldapi = os.getegid() == 0
         ds.replica_manage_time_skew(prevent=False)
         repl = replication.ReplicationManager(realm, fromhost, dirman_passwd)
         repl.force_sync(repl.conn, thishost)
diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 78330a4d1a..b264157d0e 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -29,7 +29,6 @@
 import contextlib
 import os
 import pwd
-from urllib.parse import urlparse
 import warnings
 
 from cryptography import x509 as crypto_x509
@@ -778,15 +777,9 @@ def __init__(self, ldap_uri, start_tls=False, force_schema_updates=False,
             syntax.
         """
         if ldap_uri is not None:
+            # special case for ldap2 sever plugin
             self.ldap_uri = ldap_uri
-            self.host = 'localhost'
-            self.port = None
-            url_data = urlparse(ldap_uri)
-            self._protocol = url_data.scheme
-            if self._protocol in ('ldap', 'ldaps'):
-                self.host = url_data.hostname
-                self.port = url_data.port
-
+            assert self.protocol in {'ldaps', 'ldapi', 'ldap'}
         self._start_tls = start_tls
         self._force_schema_updates = force_schema_updates
         self._no_schema = no_schema
@@ -797,7 +790,42 @@ def __init__(self, ldap_uri, start_tls=False, force_schema_updates=False,
         self._has_schema = False
         self._schema = None
 
-        self._conn = self._connect()
+        if ldap_uri is not None:
+            self._conn = self._connect()
+
+    @classmethod
+    def from_realm(cls, realm_name, **kwargs):
+        """Create a LDAPI connection to local 389-DS instance
+        """
+        uri = realm_to_ldapi_uri(realm_name)
+        return cls(uri, start_tls=False, cacert=None, **kwargs)
+
+    @classmethod
+    def from_hostname_secure(cls, hostname, cacert=paths.IPA_CA_CRT,
+                             start_tls=True, **kwargs):
+        """Create LDAP or LDAPS connection to a remote 389-DS instance
+
+        This constructor is opinionated and doesn't let you shoot yourself in
+        the foot. It always creates a secure connection. By default it
+        returns a LDAP connection to port 389 and performs STARTTLS using the
+        default CA cert. With start_tls=False, it creates a LDAPS connection
+        to port 636 instead.
+        """
+        if start_tls:
+            uri = 'ldap://%s' % format_netloc(hostname, 389)
+        else:
+            uri = 'ldaps://%s' % format_netloc(hostname, 636)
+        return cls(uri, start_tls=start_tls, cacert=cacert, **kwargs)
+
+    @classmethod
+    def from_hostname_plain(cls, hostname):
+        """Create a plain LDAP connection with TLS/SSL
+
+        Note: A plain TLS connection should only be used in combination with
+        GSSAPI bind.
+        """
+        uri = 'ldap://%s' % format_netloc(hostname, 389)
+        return cls(uri)
 
     def __str__(self):
         return self.ldap_uri
@@ -813,6 +841,13 @@ def modify_s(self, dn, modlist):
     def conn(self):
         return self._conn
 
+    @property
+    def protocol(self):
+        if self.ldap_uri:
+            return self.ldap_uri.split('://', 1)[0]
+        else:
+            return None
+
     def _get_schema(self):
         if self._no_schema:
             return None
@@ -1156,7 +1191,8 @@ def _connect(self):
             if not self._sasl_nocanon:
                 conn.set_option(ldap.OPT_X_SASL_NOCANON, ldap.OPT_OFF)
 
-            if self._start_tls:
+            if self._start_tls and self.protocol == 'ldap':
+                # STARTTLS applies only to ldap:// connections
                 conn.start_tls_s()
 
         return conn
@@ -1166,6 +1202,9 @@ def simple_bind(self, bind_dn, bind_password, server_controls=None,
         """
         Perform simple bind operation.
         """
+        if self.protocol == 'ldap' and not self._start_tls and bind_password:
+            # non-empty bind must use a secure connection
+            raise ValueError('simple_bind over insecure LDAP connection')
         with self.error_handler():
             self._flush_schema()
             assert isinstance(bind_dn, DN)
@@ -1190,7 +1229,7 @@ def gssapi_bind(self, server_controls=None, client_controls=None):
         Perform SASL bind operation using the SASL GSSAPI mechanism.
         """
         with self.error_handler():
-            if self._protocol == 'ldapi':
+            if self.protocol == 'ldapi':
                 auth_tokens = SASL_GSS_SPNEGO
             else:
                 auth_tokens = SASL_GSSAPI
diff --git a/ipaserver/install/ipa_backup.py b/ipaserver/install/ipa_backup.py
index 90d16d68ff..a6af840f94 100644
--- a/ipaserver/install/ipa_backup.py
+++ b/ipaserver/install/ipa_backup.py
@@ -400,7 +400,7 @@ def get_connection(self):
             self._conn.external_bind()
         except Exception as e:
             logger.error("Unable to bind to LDAP server %s: %s",
-                         self._conn.host, e)
+                         self._conn.ldap_uri, e)
 
         return self._conn
 
@@ -594,7 +594,7 @@ def create_header(self, data_only):
               "Unable to obtain list of master services, continuing anyway")
         except Exception as e:
             logger.error("Failed to read services from '%s': %s",
-                         conn.host, e)
+                         conn.ldap_uri, e)
         else:
             services_cns = [s.single_value['cn'] for s in services]
 
diff --git a/makeaci.in b/makeaci.in
index fe341f6f55..8840ccc535 100644
--- a/makeaci.in
+++ b/makeaci.in
@@ -50,7 +50,7 @@ def generate_aci_lines(api):
     """Yields ACI lines as they appear in ACI.txt, with trailing newline"""
     update_plugin = api.Updater['update_managed_permissions']
     perm_plugin = api.Object['permission']
-    fake_ldap = LDAPClient('', force_schema_updates=False, no_schema=True)
+    fake_ldap = LDAPClient(None, force_schema_updates=False, no_schema=True)
     for name, template, obj in update_plugin.get_templates():
         dn = perm_plugin.get_dn(name)
         entry = fake_ldap.make_entry(dn)

From 8961a1481821be03ad8c7788462868a343768d99 Mon Sep 17 00:00:00 2001
From: Christian Heimes <chei...@redhat.com>
Date: Fri, 30 Nov 2018 10:28:32 +0100
Subject: [PATCH 3/5] Use new LDAPClient constructors

Replace get_ldap_uri() + LDAPClient() with new LDAPClient constructors
like LDAPClient.from_realm().

Some places now use LDAPI with external bind instead of LDAP with simple
bind. Although the FQDN *should* resolve to 127.0.0.1 / [::1], there is
no hard guarantee. The draft
https://tools.ietf.org/html/draft-west-let-localhost-be-localhost-04#section-5.1
specifies that applications must verify that the resulting IP is a
loopback API. LDAPI is always local and a bit more efficient, too.

The simple_bind() method also prevents the caller from sending a
password over an insecure line.

Signed-off-by: Christian Heimes <chei...@redhat.com>
---
 install/migration/migration.py          |  2 +-
 ipaclient/install/client.py             |  3 +-
 ipaclient/install/ipa_certupdate.py     |  3 +-
 ipaserver/dcerpc.py                     |  5 +--
 ipaserver/dnssec/ldapkeydb.py           |  2 +-
 ipaserver/install/custodiainstance.py   |  6 +--
 ipaserver/install/dogtaginstance.py     |  8 ++--
 ipaserver/install/dsinstance.py         | 57 +++++++++----------------
 ipaserver/install/ipa_backup.py         |  3 +-
 ipaserver/install/ipa_restore.py        |  3 +-
 ipaserver/install/ldapupdate.py         |  8 +++-
 ipaserver/install/replication.py        | 37 +++++-----------
 ipatests/pytest_ipa/integration/host.py |  3 +-
 ipatests/test_install/test_updates.py   |  3 +-
 14 files changed, 54 insertions(+), 89 deletions(-)

diff --git a/install/migration/migration.py b/install/migration/migration.py
index 9b614fc9fc..7a5a046434 100644
--- a/install/migration/migration.py
+++ b/install/migration/migration.py
@@ -55,7 +55,7 @@ def bind(ldap_uri, base_dn, username, password):
         raise IOError(errno.EIO, 'Cannot get Base DN')
     bind_dn = DN(('uid', username), ('cn', 'users'), ('cn', 'accounts'), base_dn)
     try:
-        conn = ipaldap.LDAPClient(ldap_uri)
+        conn = ipaldap.LDAPClient(ldap_uri, start_tls=True)
         conn.simple_bind(bind_dn, password)
     except (errors.ACIError, errors.DatabaseError, errors.NotFound) as e:
         logger.error(
diff --git a/ipaclient/install/client.py b/ipaclient/install/client.py
index 28f23ecc57..79c1b38efb 100644
--- a/ipaclient/install/client.py
+++ b/ipaclient/install/client.py
@@ -1637,8 +1637,7 @@ def cert_summary(msg, certs, indent='    '):
 
 
 def get_certs_from_ldap(server, base_dn, realm, ca_enabled):
-    ldap_uri = ipaldap.get_ldap_uri(server)
-    conn = ipaldap.LDAPClient(ldap_uri)
+    conn = ipaldap.LDAPClient.from_hostname_plain(server)
     try:
         conn.gssapi_bind()
         certs = certstore.get_ca_certs(conn, base_dn, realm, ca_enabled)
diff --git a/ipaclient/install/ipa_certupdate.py b/ipaclient/install/ipa_certupdate.py
index 6ac136bbaa..03105d1670 100644
--- a/ipaclient/install/ipa_certupdate.py
+++ b/ipaclient/install/ipa_certupdate.py
@@ -70,8 +70,7 @@ def run_with_args(api):
 
     """
     server = urlsplit(api.env.jsonrpc_uri).hostname
-    ldap_uri = ipaldap.get_ldap_uri(server)
-    ldap = ipaldap.LDAPClient(ldap_uri)
+    ldap = ipaldap.LDAPClient.from_hostname_secure(server)
 
     tmpdir = tempfile.mkdtemp(prefix="tmp-")
     ccache_name = os.path.join(tmpdir, 'ccache')
diff --git a/ipaserver/dcerpc.py b/ipaserver/dcerpc.py
index 5e841350b8..26a26176b2 100644
--- a/ipaserver/dcerpc.py
+++ b/ipaserver/dcerpc.py
@@ -723,9 +723,8 @@ def __search_in_dc(self, info, host, port, filter, attrs, scope,
                 entries = None
 
                 try:
-                    ldap_uri = ipaldap.get_ldap_uri(host)
-                    conn = ipaldap.LDAPClient(
-                        ldap_uri,
+                    conn = ipaldap.LDAPClient.from_hostname_secure(
+                        host,
                         no_schema=True,
                         decode_attrs=False
                     )
diff --git a/ipaserver/dnssec/ldapkeydb.py b/ipaserver/dnssec/ldapkeydb.py
index d47b03732f..6bffba2bd2 100644
--- a/ipaserver/dnssec/ldapkeydb.py
+++ b/ipaserver/dnssec/ldapkeydb.py
@@ -467,7 +467,7 @@ def zone_keypairs(self):
 
     # LDAP initialization
     dns_dn = DN(ipalib.api.env.container_dns, ipalib.api.env.basedn)
-    ldap = ipaldap.LDAPClient(ipalib.api.env.ldap_uri)
+    ldap = ipaldap.LDAPClient(ipalib.api.env.ldap_uri, start_tls=True)
     logger.debug('Connecting to LDAP')
     # GSSAPI will be used, used has to be kinited already
     ldap.gssapi_bind()
diff --git a/ipaserver/install/custodiainstance.py b/ipaserver/install/custodiainstance.py
index 383c17f4eb..00b2c4c446 100644
--- a/ipaserver/install/custodiainstance.py
+++ b/ipaserver/install/custodiainstance.py
@@ -5,7 +5,6 @@
 import enum
 import logging
 
-import ipapython.ipaldap
 from ipalib import api
 from ipaserver.secrets.kem import IPAKEMKeys, KEMLdap
 from ipaserver.secrets.client import CustodiaClient
@@ -13,6 +12,7 @@
 from ipaplatform.constants import constants
 from ipaserver.install.service import SimpleServiceInstance
 from ipapython import ipautil
+from ipapython import ipaldap
 from ipapython.certdb import NSSDatabase
 from ipaserver.install import installutils
 from ipaserver.install import ldapupdate
@@ -105,7 +105,7 @@ def __init__(self, host_name=None, realm=None, custodia_peer=None):
     @property
     def ldap_uri(self):
         if self.custodia_peer is None:
-            return ipapython.ipaldap.realm_to_ldapi_uri(self.realm)
+            return ipaldap.realm_to_ldapi_uri(self.realm)
         else:
             return "ldap://{}".format(self.custodia_peer)
 
@@ -118,7 +118,7 @@ def __config_file(self):
             IPA_CUSTODIA_KEYS=paths.IPA_CUSTODIA_KEYS,
             IPA_CUSTODIA_SOCKET=paths.IPA_CUSTODIA_SOCKET,
             IPA_CUSTODIA_AUDIT_LOG=paths.IPA_CUSTODIA_AUDIT_LOG,
-            LDAP_URI=ipapython.ipaldap.realm_to_ldapi_uri(self.realm),
+            LDAP_URI=ipaldap.realm_to_ldapi_uri(self.realm),
             UID=httpd_info.pw_uid,
             GID=httpd_info.pw_gid
         )
diff --git a/ipaserver/install/dogtaginstance.py b/ipaserver/install/dogtaginstance.py
index d05e401618..5edb4674e9 100644
--- a/ipaserver/install/dogtaginstance.py
+++ b/ipaserver/install/dogtaginstance.py
@@ -386,8 +386,7 @@ def get_admin_cert(self):
         conn = None
 
         try:
-            ldap_uri = ipaldap.get_ldap_uri(protocol='ldapi', realm=self.realm)
-            conn = ipaldap.LDAPClient(ldap_uri)
+            conn = ipaldap.LDAPClient.from_realm(self.realm)
             conn.external_bind()
 
             entry_attrs = conn.get_entry(self.admin_dn, ['usercertificate'])
@@ -465,8 +464,9 @@ def setup_admin(self):
                 wait_groups.append(group_dn)
 
         # Now wait until the other server gets replicated this data
-        ldap_uri = ipaldap.get_ldap_uri(self.master_host)
-        master_conn = ipaldap.LDAPClient(ldap_uri, start_tls=True)
+        master_conn = ipaldap.LDAPClient.from_hostname_secure(
+            self.master_host
+        )
         logger.debug(
             "Waiting for %s to appear on %s", self.admin_dn, master_conn
         )
diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index 33ab3d122b..6cdec5c5fc 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -162,18 +162,17 @@ def is_ds_running(server_id=''):
 
 
 def get_domain_level(api=api):
-    ldap_uri = ipaldap.get_ldap_uri(protocol='ldapi', realm=api.env.realm)
-    conn = ipaldap.LDAPClient(ldap_uri)
-    conn.external_bind()
-
     dn = DN(('cn', 'Domain Level'),
             ('cn', 'ipa'), ('cn', 'etc'), api.env.basedn)
 
-    try:
-        entry = conn.get_entry(dn, ['ipaDomainLevel'])
-    except errors.NotFound:
-        return constants.DOMAIN_LEVEL_0
-    return int(entry.single_value['ipaDomainLevel'])
+    with ipaldap.LDAPClient.from_realm(api.env.realm) as conn:
+        conn.external_bind()
+        try:
+            entry = conn.get_entry(dn, ['ipaDomainLevel'])
+        except errors.NotFound:
+            return constants.DOMAIN_LEVEL_0
+        else:
+            return int(entry.single_value['ipaDomainLevel'])
 
 
 def get_all_external_schema_files(root):
@@ -418,8 +417,7 @@ def create_replica(self, realm_name, master_fqdn, fqdn,
 
     def _get_replication_manager(self):
         # Always connect to self over ldapi
-        ldap_uri = ipaldap.get_ldap_uri(protocol='ldapi', realm=self.realm)
-        conn = ipaldap.LDAPClient(ldap_uri)
+        conn = ipaldap.LDAPClient.from_realm(self.realm)
         conn.external_bind()
         repl = replication.ReplicationManager(
             self.realm, self.fqdn, self.dm_password, conn=conn
@@ -706,7 +704,6 @@ def __add_memberof_module(self):
         self._ldap_mod("memberof-conf.ldif")
 
     def init_memberof(self):
-
         if not self.run_init_memberof:
             return
 
@@ -715,15 +712,9 @@ def init_memberof(self):
         dn = DN(('cn', 'IPA install %s' % self.sub_dict["TIME"]), ('cn', 'memberof task'),
                 ('cn', 'tasks'), ('cn', 'config'))
         logger.debug("Waiting for memberof task to complete.")
-        ldap_uri = ipaldap.get_ldap_uri(self.fqdn)
-        conn = ipaldap.LDAPClient(ldap_uri)
-        if self.dm_password:
-            conn.simple_bind(bind_dn=ipaldap.DIRMAN_DN,
-                             bind_password=self.dm_password)
-        else:
-            conn.gssapi_bind()
-        replication.wait_for_task(conn, dn)
-        conn.unbind()
+        with ipaldap.LDAPClient.from_realm(self.realm) as conn:
+            conn.external_bind()
+            replication.wait_for_task(conn, dn)
 
     def apply_updates(self):
         schema_files = get_all_external_schema_files(paths.EXTERNAL_SCHEMA_DIR)
@@ -887,10 +878,9 @@ def __enable_ssl(self):
 
         self.cacert_name = dsdb.cacert_name
 
-        ldap_uri = ipaldap.get_ldap_uri(self.fqdn)
-        conn = ipaldap.LDAPClient(ldap_uri)
-        conn.simple_bind(bind_dn=ipaldap.DIRMAN_DN,
-                         bind_password=self.dm_password)
+        # use LDAPI?
+        conn = ipaldap.LDAPClient.from_realm(self.realm)
+        conn.external_bind()
 
         encrypt_entry = conn.make_entry(
             DN(('cn', 'encryption'), ('cn', 'config')),
@@ -943,10 +933,8 @@ def __upload_ca_cert(self):
                             subject_base=self.subject_base)
         trust_flags = dict(reversed(dsdb.list_certs()))
 
-        ldap_uri = ipaldap.get_ldap_uri(self.fqdn)
-        conn = ipaldap.LDAPClient(ldap_uri)
-        conn.simple_bind(bind_dn=ipaldap.DIRMAN_DN,
-                         bind_password=self.dm_password)
+        conn = ipaldap.LDAPClient.from_realm(self.realm)
+        conn.external_bind()
 
         nicknames = dsdb.find_root_cert(self.cacert_name)[:-1]
         for nickname in nicknames:
@@ -977,14 +965,9 @@ def __import_ca_certs(self):
         dsdb = certs.CertDB(self.realm, nssdir=dirname,
                             subject_base=self.subject_base)
 
-        ldap_uri = ipaldap.get_ldap_uri(self.fqdn)
-        conn = ipaldap.LDAPClient(ldap_uri)
-        conn.simple_bind(bind_dn=ipaldap.DIRMAN_DN,
-                         bind_password=self.dm_password)
-
-        self.export_ca_certs_nssdb(dsdb, self.ca_is_configured, conn)
-
-        conn.unbind()
+        with ipaldap.LDAPClient.from_realm(self.realm) as conn:
+            conn.external_bind()
+            self.export_ca_certs_nssdb(dsdb, self.ca_is_configured, conn)
 
     def __add_default_layout(self):
         self._ldap_mod("bootstrap-template.ldif", self.sub_dict)
diff --git a/ipaserver/install/ipa_backup.py b/ipaserver/install/ipa_backup.py
index a6af840f94..9d74000eb4 100644
--- a/ipaserver/install/ipa_backup.py
+++ b/ipaserver/install/ipa_backup.py
@@ -393,8 +393,7 @@ def get_connection(self):
         if self._conn is not None:
             return self._conn
 
-        ldap_uri = ipaldap.get_ldap_uri(protocol='ldapi', realm=api.env.realm)
-        self._conn = ipaldap.LDAPClient(ldap_uri)
+        self._conn = ipaldap.LDAPClient.from_realm(api.env.realm)
 
         try:
             self._conn.external_bind()
diff --git a/ipaserver/install/ipa_restore.py b/ipaserver/install/ipa_restore.py
index 889dbf4b8b..2f73c6daf2 100644
--- a/ipaserver/install/ipa_restore.py
+++ b/ipaserver/install/ipa_restore.py
@@ -475,8 +475,7 @@ def get_connection(self):
         if self._conn is not None:
             return self._conn
 
-        ldap_uri = ipaldap.get_ldap_uri(protocol='ldapi', realm=api.env.realm)
-        self._conn = ipaldap.LDAPClient(ldap_uri)
+        self._conn = ipaldap.LDAPClient.from_realm(api.env.realm)
 
         try:
             self._conn.external_bind()
diff --git a/ipaserver/install/ldapupdate.py b/ipaserver/install/ldapupdate.py
index 1fe076ff38..3a8861ac3d 100644
--- a/ipaserver/install/ldapupdate.py
+++ b/ipaserver/install/ldapupdate.py
@@ -54,8 +54,12 @@
 
 def connect(ldapi=False, realm=None, fqdn=None, dm_password=None):
     """Create a connection for updates"""
-    ldap_uri = ipaldap.get_ldap_uri(fqdn, ldapi=ldapi, realm=realm)
-    conn = ipaldap.LDAPClient(ldap_uri, decode_attrs=False)
+    if ldapi:
+        conn = ipaldap.LDAPClient.from_realm(realm, decode_attrs=False)
+    else:
+        conn = ipaldap.LDAPClient.from_hostname_secure(
+            fqdn, decode_attrs=False
+        )
     try:
         if dm_password:
             conn.simple_bind(bind_dn=ipaldap.DIRMAN_DN,
diff --git a/ipaserver/install/replication.py b/ipaserver/install/replication.py
index ae1c279b40..14d62ca1d3 100644
--- a/ipaserver/install/replication.py
+++ b/ipaserver/install/replication.py
@@ -137,8 +137,7 @@ def enable_replication_version_checking(realm, dirman_passwd):
     enabled then enable it and restart 389-ds. If it is enabled
     the do nothing.
     """
-    ldap_uri = ipaldap.get_ldap_uri(protocol='ldapi', realm=realm)
-    conn = ipaldap.LDAPClient(ldap_uri)
+    conn = ipaldap.LDAPClient.from_realm(realm)
     if dirman_passwd:
         conn.simple_bind(bind_dn=ipaldap.DIRMAN_DN,
                          bind_password=dirman_passwd)
@@ -619,8 +618,9 @@ def finalize_replica_config(self, r_hostname, r_binddn=None,
         """
         self._finalize_replica_settings(self.conn)
 
-        ldap_uri = ipaldap.get_ldap_uri(r_hostname)
-        r_conn = ipaldap.LDAPClient(ldap_uri, cacert=cacert)
+        r_conn = ipaldap.LDAPClient.from_hostname_secure(
+            r_hostname, cacert=cacert
+        )
         if r_bindpw:
             r_conn.simple_bind(r_binddn, r_bindpw)
         else:
@@ -1148,12 +1148,7 @@ def setup_replication(self, r_hostname, r_port=389, r_sslport=636,
             local_port = r_port
         # note - there appears to be a bug in python-ldap - it does not
         # allow connections using two different CA certs
-        ldap_uri = ipaldap.get_ldap_uri(r_hostname, r_port,
-                                        cacert=paths.IPA_CA_CRT,
-                                        protocol='ldap')
-        r_conn = ipaldap.LDAPClient(ldap_uri,
-                                    cacert=paths.IPA_CA_CRT,
-                                    start_tls=True)
+        r_conn = ipaldap.LDAPClient.from_hostname_secure(r_hostname)
 
         if r_bindpw:
             r_conn.simple_bind(r_binddn, r_bindpw)
@@ -1259,9 +1254,7 @@ def setup_winsync_replication(self,
             raise RuntimeError("Failed to start replication")
 
     def convert_to_gssapi_replication(self, r_hostname, r_binddn, r_bindpw):
-        ldap_uri = ipaldap.get_ldap_uri(r_hostname, PORT,
-                                        cacert=paths.IPA_CA_CRT)
-        r_conn = ipaldap.LDAPClient(ldap_uri, cacert=paths.IPA_CA_CRT)
+        r_conn = ipaldap.LDAPClient.from_hostname_secure(r_hostname)
         if r_bindpw:
             r_conn.simple_bind(r_binddn, r_bindpw)
         else:
@@ -1289,11 +1282,7 @@ def setup_gssapi_replication(self, r_hostname, r_binddn=None, r_bindpw=None):
         Only usable to connect 2 existing replicas (needs existing kerberos
         principals)
         """
-        # note - there appears to be a bug in python-ldap - it does not
-        # allow connections using two different CA certs
-        ldap_uri = ipaldap.get_ldap_uri(r_hostname, PORT,
-                                        cacert=paths.IPA_CA_CRT)
-        r_conn = ipaldap.LDAPClient(ldap_uri, cacert=paths.IPA_CA_CRT)
+        r_conn = ipaldap.LDAPClient.from_hostname_secure(r_hostname)
         if r_bindpw:
             r_conn.simple_bind(r_binddn, r_bindpw)
         else:
@@ -1789,10 +1778,8 @@ def remove_temp_replication_user(self, conn, r_hostname):
 
     def setup_promote_replication(self, r_hostname, r_binddn=None,
                                   r_bindpw=None, cacert=paths.IPA_CA_CRT):
-        # note - there appears to be a bug in python-ldap - it does not
-        # allow connections using two different CA certs
-        ldap_uri = ipaldap.get_ldap_uri(r_hostname)
-        r_conn = ipaldap.LDAPClient(ldap_uri, cacert=cacert)
+        r_conn = ipaldap.LDAPClient.from_hostname_secure(
+            r_hostname, cacert=cacert)
         if r_bindpw:
             r_conn.simple_bind(r_binddn, r_bindpw)
         else:
@@ -1931,8 +1918,7 @@ class CAReplicationManager(ReplicationManager):
 
     def __init__(self, realm, hostname):
         # Always connect to self over ldapi
-        ldap_uri = ipaldap.get_ldap_uri(protocol='ldapi', realm=realm)
-        conn = ipaldap.LDAPClient(ldap_uri)
+        conn = ipaldap.LDAPClient.from_realm(realm)
         conn.external_bind()
         super(CAReplicationManager, self).__init__(
             realm, hostname, None, port=DEFAULT_PORT, conn=conn)
@@ -1944,8 +1930,7 @@ def setup_cs_replication(self, r_hostname):
         Assumes a promote replica with working GSSAPI for replication
         and unified DS instance.
         """
-        ldap_uri = ipaldap.get_ldap_uri(r_hostname)
-        r_conn = ipaldap.LDAPClient(ldap_uri)
+        r_conn = ipaldap.LDAPClient.from_hostname_secure(r_hostname)
         r_conn.gssapi_bind()
 
         # Setup the first half
diff --git a/ipatests/pytest_ipa/integration/host.py b/ipatests/pytest_ipa/integration/host.py
index 6aed58ae96..cbe5ba7c01 100644
--- a/ipatests/pytest_ipa/integration/host.py
+++ b/ipatests/pytest_ipa/integration/host.py
@@ -45,8 +45,7 @@ def ldap_connect(self):
         """Return an LDAPClient authenticated to this host as directory manager
         """
         self.log.info('Connecting to LDAP at %s', self.external_hostname)
-        ldap_uri = ipaldap.get_ldap_uri(self.external_hostname)
-        ldap = ipaldap.LDAPClient(ldap_uri)
+        ldap = ipaldap.LDAPClient.from_hostname_secure(self.external_hostname)
         binddn = self.config.dirman_dn
         self.log.info('LDAP bind as %s' % binddn)
         ldap.simple_bind(binddn, self.config.dirman_password)
diff --git a/ipatests/test_install/test_updates.py b/ipatests/test_install/test_updates.py
index dc88f2fbeb..5e9b2cac8e 100644
--- a/ipatests/test_install/test_updates.py
+++ b/ipatests/test_install/test_updates.py
@@ -65,8 +65,7 @@ def setUp(self):
         else:
             raise unittest.SkipTest("No directory manager password")
         self.updater = LDAPUpdate(dm_password=self.dm_password, sub_dict={})
-        ldap_uri = ipaldap.get_ldap_uri(fqdn)
-        self.ld = ipaldap.LDAPClient(ldap_uri)
+        self.ld = ipaldap.LDAPClient.from_hostname_secure(fqdn)
         self.ld.simple_bind(bind_dn=ipaldap.DIRMAN_DN,
                             bind_password=self.dm_password)
         self.testdir = os.path.abspath(os.path.dirname(__file__))

From 1035e0cbfd207815fb0169ab9dfde667952c9b43 Mon Sep 17 00:00:00 2001
From: Christian Heimes <chei...@redhat.com>
Date: Fri, 30 Nov 2018 17:14:41 +0100
Subject: [PATCH 4/5] Use secure LDAP connection in tests

Integration tests are now using StartTLS with IPA's CA cert instead of
plain text connections.

Signed-off-by: Christian Heimes <chei...@redhat.com>
---
 ipatests/pytest_ipa/integration/host.py       | 26 +++++++++++++++----
 ipatests/pytest_ipa/integration/tasks.py      |  2 +-
 .../test_backup_and_restore.py                |  1 +
 ipatests/test_integration/test_commands.py    |  6 ++++-
 ipatests/test_integration/test_external_ca.py |  5 ++--
 .../test_replica_promotion.py                 |  1 +
 ipatests/test_xmlrpc/test_user_plugin.py      |  1 +
 7 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/ipatests/pytest_ipa/integration/host.py b/ipatests/pytest_ipa/integration/host.py
index cbe5ba7c01..e6a3853e5c 100644
--- a/ipatests/pytest_ipa/integration/host.py
+++ b/ipatests/pytest_ipa/integration/host.py
@@ -19,9 +19,11 @@
 
 """Host class for integration testing"""
 import subprocess
+import tempfile
 
 import pytest_multihost.host
 
+from ipaplatform.paths import paths
 from ipapython import ipaldap
 
 
@@ -45,11 +47,25 @@ def ldap_connect(self):
         """Return an LDAPClient authenticated to this host as directory manager
         """
         self.log.info('Connecting to LDAP at %s', self.external_hostname)
-        ldap = ipaldap.LDAPClient.from_hostname_secure(self.external_hostname)
-        binddn = self.config.dirman_dn
-        self.log.info('LDAP bind as %s' % binddn)
-        ldap.simple_bind(binddn, self.config.dirman_password)
-        return ldap
+        # get IPA CA cert to establish a secure connection
+        cacert = self.get_file_contents(paths.IPA_CA_CRT)
+        with tempfile.NamedTemporaryFile() as f:
+            f.write(cacert)
+            f.flush()
+
+            conn = ipaldap.LDAPClient.from_hostname_secure(
+                self.external_hostname,
+                cacert=f.name
+            )
+
+            binddn = self.config.dirman_dn
+            self.log.info('LDAP bind as %s', binddn)
+            conn.simple_bind(binddn, self.config.dirman_password)
+
+            # The CA cert file  has been loaded into the SSL_CTX and is no
+            # longer required.
+
+        return conn
 
     @classmethod
     def from_env(cls, env, domain, hostname, role, index, domain_index):
diff --git a/ipatests/pytest_ipa/integration/tasks.py b/ipatests/pytest_ipa/integration/tasks.py
index 4b5ddd855b..987ccb7548 100644
--- a/ipatests/pytest_ipa/integration/tasks.py
+++ b/ipatests/pytest_ipa/integration/tasks.py
@@ -303,7 +303,7 @@ def enable_replication_debugging(host, log_level=0):
         replace: nsslapd-errorlog-level
         nsslapd-errorlog-level: {log_level}
         """.format(log_level=log_level))
-    host.run_command(['ldapmodify', '-x',
+    host.run_command(['ldapmodify', '-x', '-ZZ',
                       '-D', str(host.config.dirman_dn),
                       '-w', host.config.dirman_password,
                       '-h', host.hostname],
diff --git a/ipatests/test_integration/test_backup_and_restore.py b/ipatests/test_integration/test_backup_and_restore.py
index c3cbcc8d8c..cb045a22b7 100644
--- a/ipatests/test_integration/test_backup_and_restore.py
+++ b/ipatests/test_integration/test_backup_and_restore.py
@@ -815,6 +815,7 @@ def test_replica_install_after_restore(self):
 
         # disable replication agreement
         arg = ['ldapmodify',
+               '-ZZ',
                '-h', master.hostname,
                '-p', '389', '-D',
                str(master.config.dirman_dn),  # pylint: disable=no-member
diff --git a/ipatests/test_integration/test_commands.py b/ipatests/test_integration/test_commands.py
index cfb2fa48d8..5a4cd75be5 100644
--- a/ipatests/test_integration/test_commands.py
+++ b/ipatests/test_integration/test_commands.py
@@ -140,6 +140,7 @@ def test_change_sysaccount_password_issue7561(self):
             original_passwd=original_passwd)
         master.put_file_contents(ldif_file, entry_ldif)
         arg = ['ldapmodify',
+               '-ZZ',
                '-h', master.hostname,
                '-p', '389', '-D',
                str(master.config.dirman_dn),   # pylint: disable=no-member
@@ -173,7 +174,9 @@ def test_ldapmodify_password_issue7601(self):
         master.run_command(['kinit', user], stdin_text=user_kinit_stdin_text)
         # Retrieve krblastpwdchange and krbpasswordexpiration
         search_cmd = [
-            'ldapsearch', '-x',
+            'ldapsearch', '-x', '-ZZ',
+            '-h', master.hostname,
+            '-p', '389',
             '-D', 'cn=directory manager',
             '-w', master.config.dirman_password,
             '-s', 'base',
@@ -208,6 +211,7 @@ def test_ldapmodify_password_issue7601(self):
             new_passwd=new_passwd)
         master.put_file_contents(ldif_file, entry_ldif)
         arg = ['ldapmodify',
+               '-ZZ',
                '-h', master.hostname,
                '-p', '389', '-D',
                str(master.config.dirman_dn),   # pylint: disable=no-member
diff --git a/ipatests/test_integration/test_external_ca.py b/ipatests/test_integration/test_external_ca.py
index a8e0ea0bfb..a93ab6f691 100644
--- a/ipatests/test_integration/test_external_ca.py
+++ b/ipatests/test_integration/test_external_ca.py
@@ -128,8 +128,9 @@ def test_external_ca(self):
         result = self.master.run_command([
             'ldapsearch',
             '-x',
-            '-D',
-            'cn=directory manager',
+            '-ZZ',
+            '-h', self.master.hostname,
+            '-D', 'cn=directory manager',
             '-w', self.master.config.dirman_password,
             '-b', 'cn=mapping tree,cn=config',
             '(cn=replica)',
diff --git a/ipatests/test_integration/test_replica_promotion.py b/ipatests/test_integration/test_replica_promotion.py
index cf97d4f2b2..db7b1b57d2 100644
--- a/ipatests/test_integration/test_replica_promotion.py
+++ b/ipatests/test_integration/test_replica_promotion.py
@@ -403,6 +403,7 @@ def test_replica_install_with_existing_entry(self):
             realm=replica.domain.name.upper())
         master.put_file_contents(ldif_file, entry_ldif)
         arg = ['ldapmodify',
+               '-ZZ',
                '-h', master.hostname,
                '-p', '389', '-D',
                str(master.config.dirman_dn),   # pylint: disable=no-member
diff --git a/ipatests/test_xmlrpc/test_user_plugin.py b/ipatests/test_xmlrpc/test_user_plugin.py
index 46bdeff287..e6f2bc2ef9 100644
--- a/ipatests/test_xmlrpc/test_user_plugin.py
+++ b/ipatests/test_xmlrpc/test_user_plugin.py
@@ -996,6 +996,7 @@ def setup_class(cls):
         cls.connection = ldap_initialize(
             'ldap://{host}'.format(host=api.env.host)
         )
+        cls.connection.start_tls_s()
 
     @classmethod
     def teardown_class(cls):

From 85ed9c66b9ab79c2679aade061632d8dcf264862 Mon Sep 17 00:00:00 2001
From: Christian Heimes <chei...@redhat.com>
Date: Fri, 30 Nov 2018 20:54:35 +0100
Subject: [PATCH 5/5] Use LDAPS when installing CA on replica

On a replica, 389-DS is already configured for secure connections when
the CA is installed.

Signed-off-by: Christian Heimes <chei...@redhat.com>
---
 ipaserver/install/ca.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/ipaserver/install/ca.py b/ipaserver/install/ca.py
index 2f08b279e2..557af07f07 100644
--- a/ipaserver/install/ca.py
+++ b/ipaserver/install/ca.py
@@ -298,6 +298,10 @@ def install_step_0(standalone, replica_config, options, custodia):
         'certmap.conf', 'subject_base', str(subject_base))
     dsinstance.write_certmap_conf(realm_name, ca_subject)
 
+    # use secure ldaps when installing a replica or upgrading to CA-ful
+    # In both cases, 389-DS is already configured to have a trusted cert.
+    use_ldaps = standalone or replica_config is not None
+
     ca = cainstance.CAInstance(
         realm=realm_name, host_name=host_name, custodia=custodia
     )
@@ -316,7 +320,7 @@ def install_step_0(standalone, replica_config, options, custodia):
                           ra_p12=ra_p12,
                           ra_only=ra_only,
                           promote=promote,
-                          use_ldaps=standalone)
+                          use_ldaps=use_ldaps)
 
 
 def install_step_1(standalone, replica_config, options, custodia):
_______________________________________________
FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org
To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/freeipa-devel@lists.fedorahosted.org

Reply via email to