Skip to content

Allow LDAP connection from file descriptor #337

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion Doc/reference/ldap.rst
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ Functions

This module defines the following functions:

.. py:function:: initialize(uri [, trace_level=0 [, trace_file=sys.stdout [, trace_stack_limit=None, [bytes_mode=None, [bytes_strictness=None]]]]]) -> LDAPObject object
.. py:function:: initialize(uri [, trace_level=0 [, trace_file=sys.stdout [, trace_stack_limit=None, [bytes_mode=None, [bytes_strictness=None, [fileno=None]]]]]]) -> LDAPObject object

Initializes a new connection object for accessing the given LDAP server,
and return an :class:`~ldap.ldapobject.LDAPObject` used to perform operations
Expand All @@ -40,6 +40,16 @@ This module defines the following functions:
when using multiple URIs you cannot determine to which URI your client
gets connected.

If *fileno* parameter is given then the file descriptor will be used to
connect to an LDAP server. The *fileno* must either be a socket file
descriptor as :class:`int` or a file-like object with a *fileno()* method
that returns a socket file descriptor. The socket file descriptor must
already be connected. :class:`~ldap.ldapobject.LDAPObject` does not take
ownership of the file descriptor. It must be kept open during operations
and explicitly closed after the :class:`~ldap.ldapobject.LDAPObject` is
unbound. The internal connection type is determined from the URI, ``TCP``
for ``ldap://`` / ``ldaps://``, ``IPC`` (``AF_UNIX``) for ``ldapi://``.

Note that internally the OpenLDAP function
`ldap_initialize(3) <https://www.openldap.org/software/man.cgi?query=ldap_init&sektion=3>`_
is called which just initializes the LDAP connection struct in the C API
Expand Down Expand Up @@ -72,6 +82,10 @@ This module defines the following functions:

:rfc:`4516` - Lightweight Directory Access Protocol (LDAP): Uniform Resource Locator

.. versionadded:: 3.3

The *fileno* argument was added.


.. py:function:: get_option(option) -> int|string

Expand Down
9 changes: 7 additions & 2 deletions Lib/ldap/functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def _ldap_function_call(lock,func,*args,**kwargs):

def initialize(
uri, trace_level=0, trace_file=sys.stdout, trace_stack_limit=None,
bytes_mode=None, **kwargs
bytes_mode=None, fileno=None, **kwargs
):
"""
Return LDAPObject instance by opening LDAP connection to
Expand All @@ -84,12 +84,17 @@ def initialize(
Default is to use stdout.
bytes_mode
Whether to enable :ref:`bytes_mode` for backwards compatibility under Py2.
fileno
If not None the socket file descriptor is used to connect to an
LDAP server.

Additional keyword arguments (such as ``bytes_strictness``) are
passed to ``LDAPObject``.
"""
return LDAPObject(
uri, trace_level, trace_file, trace_stack_limit, bytes_mode, **kwargs)
uri, trace_level, trace_file, trace_stack_limit, bytes_mode,
fileno=fileno, **kwargs
)


def get_option(option):
Expand Down
16 changes: 12 additions & 4 deletions Lib/ldap/ldapobject.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,14 +96,21 @@ class SimpleLDAPObject:
def __init__(
self,uri,
trace_level=0,trace_file=None,trace_stack_limit=5,bytes_mode=None,
bytes_strictness=None,
bytes_strictness=None, fileno=None
):
self._trace_level = trace_level or ldap._trace_level
self._trace_file = trace_file or ldap._trace_file
self._trace_stack_limit = trace_stack_limit
self._uri = uri
self._ldap_object_lock = self._ldap_lock('opcall')
self._l = ldap.functions._ldap_function_call(ldap._ldap_module_lock,_ldap.initialize,uri)
if fileno is not None:
if hasattr(fileno, "fileno"):
fileno = fileno.fileno()
self._l = ldap.functions._ldap_function_call(
ldap._ldap_module_lock, _ldap.initialize_fd, fileno, uri
)
else:
self._l = ldap.functions._ldap_function_call(ldap._ldap_module_lock,_ldap.initialize,uri)
self.timeout = -1
self.protocol_version = ldap.VERSION3

Expand Down Expand Up @@ -1086,7 +1093,7 @@ class ReconnectLDAPObject(SimpleLDAPObject):
def __init__(
self,uri,
trace_level=0,trace_file=None,trace_stack_limit=5,bytes_mode=None,
bytes_strictness=None, retry_max=1, retry_delay=60.0
bytes_strictness=None, retry_max=1, retry_delay=60.0, fileno=None
):
"""
Parameters like SimpleLDAPObject.__init__() with these
Expand All @@ -1102,7 +1109,8 @@ def __init__(
self._last_bind = None
SimpleLDAPObject.__init__(self, uri, trace_level, trace_file,
trace_stack_limit, bytes_mode,
bytes_strictness=bytes_strictness)
bytes_strictness=bytes_strictness,
fileno=fileno)
self._reconnect_lock = ldap.LDAPLock(desc='reconnect lock within %s' % (repr(self)))
self._retry_max = retry_max
self._retry_delay = retry_delay
Expand Down
12 changes: 10 additions & 2 deletions Lib/slapdtest/_slapdtest.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ class SlapdObject(object):
root_cn = 'Manager'
root_pw = 'password'
slapd_loglevel = 'stats stats2'
local_host = '127.0.0.1'
local_host = LOCALHOST
testrunsubdirs = (
'schema',
)
Expand Down Expand Up @@ -214,7 +214,7 @@ def __init__(self):
self._schema_prefix = os.path.join(self.testrundir, 'schema')
self._slapd_conf = os.path.join(self.testrundir, 'slapd.conf')
self._db_directory = os.path.join(self.testrundir, "openldap-data")
self.ldap_uri = "ldap://%s:%d/" % (LOCALHOST, self._port)
self.ldap_uri = "ldap://%s:%d/" % (self.local_host, self._port)
if HAVE_LDAPI:
ldapi_path = os.path.join(self.testrundir, 'ldapi')
self.ldapi_uri = "ldapi://%s" % quote_plus(ldapi_path)
Expand Down Expand Up @@ -243,6 +243,14 @@ def __init__(self):
def root_dn(self):
return 'cn={self.root_cn},{self.suffix}'.format(self=self)

@property
def hostname(self):
return self.local_host

@property
def port(self):
return self._port

def _find_commands(self):
self.PATH_LDAPADD = self._find_command('ldapadd')
self.PATH_LDAPDELETE = self._find_command('ldapdelete')
Expand Down
12 changes: 8 additions & 4 deletions Modules/LDAPObject.c
Original file line number Diff line number Diff line change
Expand Up @@ -1365,14 +1365,16 @@ l_ldap_start_tls_s(LDAPObject *self, PyObject *args)
/* ldap_set_option */

static PyObject *
l_ldap_set_option(PyObject *self, PyObject *args)
l_ldap_set_option(LDAPObject *self, PyObject *args)
{
PyObject *value;
int option;

if (!PyArg_ParseTuple(args, "iO:set_option", &option, &value))
return NULL;
if (!LDAP_set_option((LDAPObject *)self, option, value))
if (not_valid(self))
return NULL;
if (!LDAP_set_option(self, option, value))
return NULL;
Py_INCREF(Py_None);
return Py_None;
Expand All @@ -1381,13 +1383,15 @@ l_ldap_set_option(PyObject *self, PyObject *args)
/* ldap_get_option */

static PyObject *
l_ldap_get_option(PyObject *self, PyObject *args)
l_ldap_get_option(LDAPObject *self, PyObject *args)
{
int option;

if (!PyArg_ParseTuple(args, "i:get_option", &option))
return NULL;
return LDAP_get_option((LDAPObject *)self, option);
if (not_valid(self))
return NULL;
return LDAP_get_option(self, option);
}

/* ldap_passwd */
Expand Down
72 changes: 72 additions & 0 deletions Modules/functions.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,75 @@ l_ldap_initialize(PyObject *unused, PyObject *args)
return (PyObject *)newLDAPObject(ld);
}

#ifdef HAVE_LDAP_INIT_FD

/* initialize_fd(fileno, url)
*
* ldap_init_fd() is not a private API but it's not in a public header either
* SSSD has been using the function for a while, so it's probably OK.
Comment on lines +37 to +38
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any plans to make it public?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why it hasn't landed in upstream headers. The API has been around for a while and it's stable. SSSD uses it heavily.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an RFE open?
I'm worried by the extern declaration: OpenLDAP is free to add a ldap_init_fd function to the public headers, and even make the declaration a bit different. (Would e.g. unsigned int proto be enough to cause python-ldap to fail to build?).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's been document with the exact same function signature for 13 years, openldap/openldap@282a3c5 . It's unlikely that the function will disappear or change signature.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK.
Could you make sure OpenLDAP devs know, though?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK.
Could you make sure OpenLDAP devs know, though?

The statement was false. It is provided in the public openldap.h header file.

Copy link
Member Author

@tiran tiran Jun 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... which is not packaged by most distributions like Debian/Ubuntu or Fedora/RHEL.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... which is not packaged by most distributions like Debian/Ubuntu or Fedora/RHEL.

Then you need to talk to the distributions, not the OpenLDAP developers.

*/

#ifndef LDAP_PROTO_TCP
#define LDAP_PROTO_TCP 1
#define LDAP_PROTO_UDP 2
#define LDAP_PROTO_IPC 3
#endif

extern int
ldap_init_fd(ber_socket_t fd, int proto, LDAP_CONST char *url, LDAP **ldp);

static PyObject *
l_ldap_initialize_fd(PyObject *unused, PyObject *args)
{
char *url;
LDAP *ld = NULL;
int ret;
int fd;
int proto = -1;
LDAPURLDesc *lud = NULL;

PyThreadState *save;

if (!PyArg_ParseTuple(args, "is:initialize_fd", &fd, &url))
return NULL;

/* Get LDAP protocol from scheme */
ret = ldap_url_parse(url, &lud);
if (ret != LDAP_SUCCESS)
return LDAPerr(ret);

if (strcmp(lud->lud_scheme, "ldap") == 0) {
proto = LDAP_PROTO_TCP;
}
else if (strcmp(lud->lud_scheme, "ldaps") == 0) {
proto = LDAP_PROTO_TCP;
}
else if (strcmp(lud->lud_scheme, "ldapi") == 0) {
proto = LDAP_PROTO_IPC;
}
#ifdef LDAP_CONNECTIONLESS
else if (strcmp(lud->lud_scheme, "cldap") == 0) {
proto = LDAP_PROTO_UDP;
}
#endif
else {
ldap_free_urldesc(lud);
PyErr_SetString(PyExc_ValueError, "unsupported URL scheme");
return NULL;
}
ldap_free_urldesc(lud);

save = PyEval_SaveThread();
ret = ldap_init_fd((ber_socket_t) fd, proto, url, &ld);
PyEval_RestoreThread(save);

if (ret != LDAP_SUCCESS)
return LDAPerror(ld);

return (PyObject *)newLDAPObject(ld);
}
#endif /* HAVE_LDAP_INIT_FD */

/* ldap_str2dn */

static PyObject *
Expand Down Expand Up @@ -137,6 +206,9 @@ l_ldap_get_option(PyObject *self, PyObject *args)

static PyMethodDef methods[] = {
{"initialize", (PyCFunction)l_ldap_initialize, METH_VARARGS},
#ifdef HAVE_LDAP_INIT_FD
{"initialize_fd", (PyCFunction)l_ldap_initialize_fd, METH_VARARGS},
#endif
{"str2dn", (PyCFunction)l_ldap_str2dn, METH_VARARGS},
{"set_option", (PyCFunction)l_ldap_set_option, METH_VARARGS},
{"get_option", (PyCFunction)l_ldap_get_option, METH_VARARGS},
Expand Down
59 changes: 53 additions & 6 deletions Tests/t_cext.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@

from __future__ import unicode_literals

import contextlib
import errno
import os
import socket
import unittest

# Switch off processing .ldaprc or ldap.conf before importing _ldap
Expand Down Expand Up @@ -92,14 +94,35 @@ def _open_conn(self, bind=True):
"""
l = _ldap.initialize(self.server.ldap_uri)
if bind:
# Perform a simple bind
l.set_option(_ldap.OPT_PROTOCOL_VERSION, _ldap.VERSION3)
m = l.simple_bind(self.server.root_dn, self.server.root_pw)
result, pmsg, msgid, ctrls = l.result4(m, _ldap.MSG_ONE, self.timeout)
self.assertEqual(result, _ldap.RES_BIND)
self.assertEqual(type(msgid), type(0))
self._bind_conn(l)
return l

@contextlib.contextmanager
def _open_conn_fd(self, bind=True):
sock = socket.create_connection(
(self.server.hostname, self.server.port)
)
try:
l = _ldap.initialize_fd(sock.fileno(), self.server.ldap_uri)
if bind:
self._bind_conn(l)
yield sock, l
finally:
try:
sock.close()
except OSError:
# already closed
pass

def _bind_conn(self, l):
# Perform a simple bind
l.set_option(_ldap.OPT_PROTOCOL_VERSION, _ldap.VERSION3)
m = l.simple_bind(self.server.root_dn, self.server.root_pw)
result, pmsg, msgid, ctrls = l.result4(m, _ldap.MSG_ONE, self.timeout)
self.assertEqual(result, _ldap.RES_BIND)
self.assertEqual(type(msgid), type(0))


# Test for the existence of a whole bunch of constants
# that the C module is supposed to export
def test_constants(self):
Expand Down Expand Up @@ -224,6 +247,30 @@ def test_test_flags(self):
def test_simple_bind(self):
l = self._open_conn()

def test_simple_bind_fileno(self):
with self._open_conn_fd() as (sock, l):
self.assertEqual(l.whoami_s(), "dn:" + self.server.root_dn)

def test_simple_bind_fileno_invalid(self):
with open(os.devnull) as f:
l = _ldap.initialize_fd(f.fileno(), self.server.ldap_uri)
with self.assertRaises(_ldap.SERVER_DOWN):
self._bind_conn(l)

def test_simple_bind_fileno_closed(self):
with self._open_conn_fd() as (sock, l):
self.assertEqual(l.whoami_s(), "dn:" + self.server.root_dn)
sock.close()
with self.assertRaises(_ldap.SERVER_DOWN):
l.whoami_s()

def test_simple_bind_fileno_rebind(self):
with self._open_conn_fd() as (sock, l):
self.assertEqual(l.whoami_s(), "dn:" + self.server.root_dn)
l.unbind_ext()
with self.assertRaises(_ldap.LDAPError):
self._bind_conn(l)

def test_simple_anonymous_bind(self):
l = self._open_conn(bind=False)
m = l.simple_bind("", "")
Expand Down
27 changes: 27 additions & 0 deletions Tests/t_ldapobject.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import contextlib
import linecache
import os
import socket
import unittest
import warnings
import pickle
Expand Down Expand Up @@ -103,6 +104,9 @@ def setUp(self):
# open local LDAP connection
self._ldap_conn = self._open_ldap_conn(bytes_mode=False)

def tearDown(self):
del self._ldap_conn

def test_reject_bytes_base(self):
base = self.server.suffix
l = self._ldap_conn
Expand Down Expand Up @@ -754,5 +758,28 @@ def test105_reconnect_restore(self):
self.assertEqual(l1.whoami_s(), 'dn:'+bind_dn)


class Test03_SimpleLDAPObjectWithFileno(Test00_SimpleLDAPObject):
def _get_bytes_ldapobject(self, explicit=True, **kwargs):
raise unittest.SkipTest("Test opens two sockets")

def _search_wrong_type(self, bytes_mode, strictness):
raise unittest.SkipTest("Test opens two sockets")

def _open_ldap_conn(self, who=None, cred=None, **kwargs):
if hasattr(self, '_sock'):
raise RuntimeError("socket already connected")
self._sock = socket.create_connection(
(self.server.hostname, self.server.port)
)
return super(Test03_SimpleLDAPObjectWithFileno, self)._open_ldap_conn(
who=who, cred=cred, fileno=self._sock.fileno(), **kwargs
)

def tearDown(self):
self._sock.close()
del self._sock
super(Test03_SimpleLDAPObjectWithFileno, self).tearDown()


if __name__ == '__main__':
unittest.main()
Loading