URL: https://github.com/freeipa/freeipa/pull/1590
Author: tiran
 Title: #1590: [Backport][ipa-4-6] Unified ldap_initialize() function
Action: opened

PR body:
"""
Manual backport of PR #1587 

Replace all ldap.initialize() calls with a helper function
ldap_initialize(). It handles cacert and cert validation correctly. It
also provides a unique place to handle python-ldap 3.0 bytes warnings in
the future.

Fixes: https://pagure.io/freeipa/issue/7411
Signed-off-by: Christian Heimes <chei...@redhat.com>
Reviewed-By: Alexander Bokovoy <aboko...@redhat.com>
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/1590/head:pr1590
git checkout pr1590
From 647c2695435d653172a0028007bf6d145b598192 Mon Sep 17 00:00:00 2001
From: Christian Heimes <chei...@redhat.com>
Date: Thu, 15 Feb 2018 12:30:06 +0100
Subject: [PATCH] Unified ldap_initialize() function

Replace all ldap.initialize() calls with a helper function
ldap_initialize(). It handles cacert and cert validation correctly. It
also provides a unique place to handle python-ldap 3.0 bytes warnings in
the future.

Fixes: https://pagure.io/freeipa/issue/7411
Signed-off-by: Christian Heimes <chei...@redhat.com>
Reviewed-By: Alexander Bokovoy <aboko...@redhat.com>
---
 ipapython/ipaldap.py                     | 36 +++++++++++++++++++++++++-------
 ipaserver/dcerpc.py                      |  3 ++-
 ipaserver/install/replication.py         | 12 +++++------
 ipaserver/secrets/common.py              |  4 +++-
 ipatests/test_ipaserver/test_changepw.py |  5 +++--
 ipatests/test_xmlrpc/test_user_plugin.py |  6 ++++--
 ipatests/util.py                         |  3 ++-
 7 files changed, 48 insertions(+), 21 deletions(-)

diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 339271b670..800c254df1 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -85,6 +85,34 @@
     )
 
 
+def ldap_initialize(uri, cacertfile=None):
+    """Wrapper around ldap.initialize()
+    """
+    conn = ldap.initialize(uri)
+
+    if not uri.startswith('ldapi://'):
+        if cacertfile:
+            conn.set_option(ldap.OPT_X_TLS_CACERTFILE, cacertfile)
+            newctx = True
+        else:
+            newctx = False
+
+        req_cert = conn.get_option(ldap.OPT_X_TLS_REQUIRE_CERT)
+        if req_cert != ldap.OPT_X_TLS_DEMAND:
+            # libldap defaults to cert validation, but the default can be
+            # overridden in global or user local ldap.conf.
+            conn.set_option(
+                ldap.OPT_X_TLS_REQUIRE_CERT, ldap.OPT_X_TLS_DEMAND
+            )
+            newctx = True
+
+        # reinitialize TLS context
+        if newctx:
+            conn.set_option(ldap.OPT_X_TLS_NEWCTX, 0)
+
+    return conn
+
+
 class _ServerSchema(object):
     '''
     Properties of a schema retrieved from an LDAP server.
@@ -1091,13 +1119,7 @@ def close(self):
 
     def _connect(self):
         with self.error_handler():
-            conn = ldap.initialize(self.ldap_uri)
-
-            if self._start_tls or self._protocol == 'ldaps':
-                ldap.set_option(ldap.OPT_X_TLS_CACERTFILE, self._cacert)
-                ldap.set_option(ldap.OPT_X_TLS_REQUIRE_CERT, True)
-                conn.set_option(ldap.OPT_X_TLS_DEMAND, True)
-
+            conn = ldap_initialize(self.ldap_uri, cacertfile=self._cacert)
             if self._sasl_nocanon:
                 conn.set_option(ldap.OPT_X_SASL_NOCANON, ldap.OPT_ON)
 
diff --git a/ipaserver/dcerpc.py b/ipaserver/dcerpc.py
index 8b8e84a1b0..aa17db58f9 100644
--- a/ipaserver/dcerpc.py
+++ b/ipaserver/dcerpc.py
@@ -30,6 +30,7 @@
 from ipalib import errors
 from ipapython import ipautil
 from ipapython.dn import DN
+from ipapython.ipaldap import ldap_initialize
 from ipaserver.install import installutils
 from ipaserver.dcerpc_common import (TRUST_BIDIRECTIONAL,
                                      TRUST_JOIN_EXTERNAL,
@@ -933,7 +934,7 @@ def retrieve_anonymously(self, remote_host,
         # We need to do rootDSE search with LDAP_SERVER_EXTENDED_DN_OID
         # control to reveal the SID
         ldap_uri = 'ldap://%s' % (result.pdc_dns_name)
-        conn = _ldap.initialize(ldap_uri)
+        conn = ldap_initialize(ldap_uri)
         conn.set_option(_ldap.OPT_SERVER_CONTROLS, [ExtendedDNControl()])
         search_result = None
         try:
diff --git a/ipaserver/install/replication.py b/ipaserver/install/replication.py
index 43f00a7304..e25a2f2cd7 100644
--- a/ipaserver/install/replication.py
+++ b/ipaserver/install/replication.py
@@ -36,6 +36,7 @@
 from ipapython import ipautil, ipaldap, kerberos
 from ipapython.admintool import ScriptError
 from ipapython.dn import DN
+from ipapython.ipaldap import ldap_initialize
 from ipaplatform.paths import paths
 from ipaserver.install import installutils
 
@@ -1077,13 +1078,10 @@ def setup_winsync_replication(self,
         self.ad_suffix = ""
         try:
             # Validate AD connection
-            ad_conn = ldap.initialize('ldap://%s' % ipautil.format_netloc(ad_dc_name))
-            # the next one is to workaround bugs arounf opendalp libs+NSS db
-            # we need to first specify the OPT_X_TLS_CACERTFILE and _after_
-            # that initialize the context to prevent TLS connection errors:
-            # https://bugzilla.redhat.com/show_bug.cgi?id=800787
-            ad_conn.set_option(ldap.OPT_X_TLS_CACERTFILE, cacert)
-            ad_conn.set_option(ldap.OPT_X_TLS_NEWCTX, 0)
+            ad_conn = ldap_initialize(
+                'ldap://%s' % ipautil.format_netloc(ad_dc_name),
+                cacertfile=cacert
+            )
             ad_conn.start_tls_s()
             ad_conn.simple_bind_s(str(ad_binddn), ad_pwd)
             res = ad_conn.search_s("", ldap.SCOPE_BASE, '(objectClass=*)',
diff --git a/ipaserver/secrets/common.py b/ipaserver/secrets/common.py
index 3c152516ae..610d87c59a 100644
--- a/ipaserver/secrets/common.py
+++ b/ipaserver/secrets/common.py
@@ -4,6 +4,8 @@
 import ldap.sasl
 import ldap.filter
 
+from ipapython.ipaldap import ldap_initialize
+
 
 class iSecLdap(object):
 
@@ -27,7 +29,7 @@ def basedn(self):
         return self._basedn
 
     def connect(self):
-        conn = ldap.initialize(self.uri)
+        conn = ldap_initialize(self.uri)
         if self.auth_type == 'EXTERNAL':
             auth_tokens = ldap.sasl.external(None)
         elif self.auth_type == 'GSSAPI':
diff --git a/ipatests/test_ipaserver/test_changepw.py b/ipatests/test_ipaserver/test_changepw.py
index 9c74f2ef3e..3f83312668 100644
--- a/ipatests/test_ipaserver/test_changepw.py
+++ b/ipatests/test_ipaserver/test_changepw.py
@@ -18,7 +18,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import nose
-import ldap
+
 import pytest
 
 from ipatests.test_ipaserver.httptest import Unauthorized_HTTP_test
@@ -26,6 +26,7 @@
 from ipatests.util import assert_equal
 from ipalib import api, errors
 from ipapython.dn import DN
+from ipapython.ipaldap import ldap_initialize
 
 testuser = u'tuser'
 old_password = u'old_password'
@@ -59,7 +60,7 @@ def _changepw(self, user, old_password, new_password):
 
     def _checkpw(self, user, password):
         dn = str(DN(('uid', user), api.env.container_user, api.env.basedn))
-        conn = ldap.initialize(api.env.ldap_uri)
+        conn = ldap_initialize(api.env.ldap_uri)
         try:
             conn.simple_bind_s(dn, password)
         finally:
diff --git a/ipatests/test_xmlrpc/test_user_plugin.py b/ipatests/test_xmlrpc/test_user_plugin.py
index 7393a23529..af825f79da 100644
--- a/ipatests/test_xmlrpc/test_user_plugin.py
+++ b/ipatests/test_xmlrpc/test_user_plugin.py
@@ -37,6 +37,7 @@
     XMLRPC_test, fuzzy_digits, fuzzy_uuid, fuzzy_password,
     Fuzzy, fuzzy_dergeneralizedtime, add_sid, add_oc, raises_exact)
 from ipapython.dn import DN
+from ipapython.ipaldap import ldap_initialize
 
 from ipatests.test_xmlrpc.tracker.base import Tracker
 from ipatests.test_xmlrpc.tracker.group_plugin import GroupTracker
@@ -913,8 +914,9 @@ class TestDeniedBindWithExpiredPrincipal(XMLRPC_test):
     def setup_class(cls):
         super(TestDeniedBindWithExpiredPrincipal, cls).setup_class()
 
-        cls.connection = ldap.initialize('ldap://{host}'
-                                         .format(host=api.env.host))
+        cls.connection = ldap_initialize(
+            'ldap://{host}'.format(host=api.env.host)
+        )
 
     @classmethod
     def teardown_class(cls):
diff --git a/ipatests/util.py b/ipatests/util.py
index db4c121329..826e90025d 100644
--- a/ipatests/util.py
+++ b/ipatests/util.py
@@ -42,6 +42,7 @@
 from ipalib.plugable import Plugin
 from ipalib.request import context
 from ipapython.dn import DN
+from ipapython.ipaldap import ldap_initialize
 from ipapython.ipautil import run
 
 
@@ -726,7 +727,7 @@ def _calledall(self):
 
 class MockLDAP(object):
     def __init__(self):
-        self.connection = ldap.initialize(
+        self.connection = ldap_initialize(
             'ldap://{host}'.format(host=ipalib.api.env.host)
         )
 
_______________________________________________
FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org
To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org

Reply via email to