[SRU] PY3: python3-ldap does not allow bytes for DN/RDN/field names

[Corey Bryant]

[Impact]
Keystone LDAP backend doesn't work for PY3.

Under Python 2, python-ldap uses bytes by default. Under Python 3 this is removed and bytes aren't allowed for DN/RDN/field names.

More details are here: http://www.python-ldap.org/en/latest/bytes_mode.html#bytes-mode
and here: https://github.com/python-ldap/python-ldap/blob/python-ldap-3.1.0/Lib/ldap/ldapobject.py#L111

== initial traceback ==

Here's the initial traceback from the failure: https://paste.ubuntu.com/p/67THZb2m5m/

The last bit of the error is:

  File "/usr/lib/python3/dist-packages/ldap/ldapobject.py", line 314, in _ldap_call
    result = func(*args,**kwargs)
TypeError: simple_bind() argument 1 must be str or None, not bytes

A closer look at func shows:

func=<built-in method simple_bind of LDAP object at 0x7f9d0177b760>
args=(b'cn=admin,dc=test,dc=com', b'crapper', None, None)

== keystone ldap backend use of python-ldap ==

In simple_bind_s() of keystone's ldap backend, who and cred are encoded as byte strings:

https://github.com/openstack/keystone/blob/14.0.0/keystone/identity/backends/ldap/common.py#L885

but that appears to no longer be valid use of python-ldap for py3.

[Test Case]

Run charm-keystone-ldap functional tests for OpenStack Rocky or above.

[Regression Potential]
The only regression potential would be for PY2 code paths. PY3 code paths never worked for keystone's LDAP backend. The approach to the patch have purposefully minimized amount of code required and therefore regression potential for PY2. Note that Rocky for Ubuntu supports PY2 but as of Stein Ubuntu has dropped PY2 support.

[Corey Bryant (corey.bryant)]

After fixing the first error locally I hit the following: https://paste.ubuntu.com/p/SrNMDnXZfc/

Seems that all use of the python3-ldap API will need to be considered here with dropping bytes usage for py3. It seems as though it may be limited to keystone/identity/backends/ldap/common.py but I'm not overly familiar with the code.

[Corey Bryant (corey.bryant)]

And for completeness for comment #1:

func=<built-in method search_ext of LDAP object at 0x7f5f091f9a80>
args=(b'ou=Users,dc=test,dc=com', 1, b'(&(objectClass=inetOrgPerson)(cn=*))', [b'sn', b'mail', b'enabled', b'description', b'userPassword', b'cn'], 0, None, None, -1, 0)

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/611190

[Corey Bryant (corey.bryant)]

In fixing up unit tests for the above change it appears that ldappool also is affected by this bug.

[Corey Bryant (corey.bryant)]

ldappool$ grep -r utf8
ldappool/__init__.py:_utf8_encoder = codecs.getencoder('utf-8')
ldappool/__init__.py:def utf8_encode(value):
ldappool/__init__.py: return _utf8_encoder(value)[0]
ldappool/__init__.py: passwd = utf8_encode(passwd)
ldappool/__init__.py: passwd = utf8_encode(passwd)
ldappool/__init__.py: passwd = utf8_encode(passwd)

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/queens
Review: https://review.openstack.org/613615

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/611401
Committed: https://git.openstack.org/cgit/openstack/ldappool/commit/?id=7734b7f45d61a22ebc0ce204304486b7484d697a
Submitter: Zuul
Branch: master

commit 7734b7f45d61a22ebc0ce204304486b7484d697a
Author: Corey Bryant <email address hidden>
Date: Wed Oct 17 13:18:35 2018 -0400

    PY3: switch to using unicode text values

    In Python 3, python-ldap no longer allows bytes for DN/RDN/field
    names. Instead, text values are represented as str, the Unicode
    text type.

    This patch updates the code to adhere to this behavior.

    More details about byte/str usage in python-ldap can be found at:
    http://www.python-ldap.org/en/latest/bytes_mode.html#bytes-mode

    Change-Id: I9ef10432229aaffe4ac9bd733d608098cdae3b9a
    Partial-Bug: #1798184

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/rocky
Review: https://review.openstack.org/613648

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/613615
Committed: https://git.openstack.org/cgit/openstack/ldappool/commit/?id=7a071a7ca3303160bf79654de8c85f6502f47ef9
Submitter: Zuul
Branch: stable/queens

commit 7a071a7ca3303160bf79654de8c85f6502f47ef9
Author: Corey Bryant <email address hidden>
Date: Wed Oct 17 13:18:35 2018 -0400

    PY3: switch to using unicode text values

    In Python 3, python-ldap no longer allows bytes for DN/RDN/field
    names. Instead, text values are represented as str, the Unicode
    text type.

    This patch updates the code to adhere to this behavior.

    More details about byte/str usage in python-ldap can be found at:
    http://www.python-ldap.org/en/latest/bytes_mode.html#bytes-mode

    Change-Id: I9ef10432229aaffe4ac9bd733d608098cdae3b9a
    Partial-Bug: #1798184

[David Ames (thedac)]

@Corey,

Potentially related:

python-ldap gets installed when python3-ldappool is installed even if python3-ldap is already installed. Could be a packaging dependency problem.

aptitude why python-ldap
i python3-ldappool Depends python3-pyldap (>= 2.4.20)
i A python3-pyldap Depends python-ldap (= 3.0.0-1)

[Shashinandan Kolar Srinivasa (shashinandan)]

Is this fix available in stable/rocky? I am facing the issue at present
keystone version 14.0.0, using juju charms to deploy and configure

-----
(keystone.common.wsgi): 2018-12-26 20:11:34,937 ERROR simple_bind() argument 1 must be str or None, not bytes
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/keystone/common/wsgi.py", line 148, in __call__
    result = method(req, **params)
  File "/usr/lib/python3/dist-packages/keystone/common/controller.py", line 103, in wrapper
    return f(self, request, filters, **kwargs)
  File "/usr/lib/python3/dist-packages/keystone/identity/controllers.py", line 175, in list_groups
    domain_scope=domain, hints=hints
  File "/usr/lib/python3/dist-packages/keystone/common/manager.py", line 116, in wrapped
    __ret_val = __f(*args, **kwargs)
  File "/usr/lib/python3/dist-packages/keystone/identity/core.py", line 416, in wrapper
    return f(self, *args, **kwargs)
  File "/usr/lib/python3/dist-packages/keystone/identity/core.py", line 426, in wrapper
    return f(self, *args, **kwargs)
  File "/usr/lib/python3/dist-packages/keystone/identity/core.py", line 1339, in list_groups
    ref_list = driver.list_groups(hints)
  File "/usr/lib/python3/dist-packages/keystone/identity/backends/ldap/core.py", line 116, in list_groups
    return self.group.get_all_filtered(hints)
  File "/usr/lib/python3/dist-packages/keystone/identity/backends/ldap/core.py", line 471, in get_all_filtered
    for group in self.get_all(query, hints)]
  File "/usr/lib/python3/dist-packages/keystone/identity/backends/ldap/common.py", line 1541, in get_all
    for x in self._ldap_get_all(hints, ldap_filter)]
  File "/usr/lib/python3/dist-packages/keystone/common/driver_hints.py", line 42, in wrapper
    return f(self, hints, *args, **kwargs)
  File "/usr/lib/python3/dist-packages/keystone/identity/backends/ldap/common.py", line 1489, in _ldap_get_all
    with self.get_connection() as conn:
  File "/usr/lib/python3/dist-packages/keystone/identity/backends/ldap/common.py", line 1249, in get_connection
    conn.simple_bind_s(user, password)
  File "/usr/lib/python3/dist-packages/keystone/identity/backends/ldap/common.py", line 892, in simple_bind_s
    clientctrls=clientctrls)
  File "/usr/lib/python3/dist-packages/keystone/identity/backends/ldap/common.py", line 747, in simple_bind_s
    with self._get_pool_connection() as conn:
  File "/usr/lib/python3.6/contextlib.py", line 81, in __enter__
    return next(self.gen)
  File "/usr/local/lib/python3.6/dist-packages/ldappool/__init__.py", line 375, in connection
    conn = self._get_connection(bind, passwd)
  File "/usr/local/lib/python3.6/dist-packages/ldappool/__init__.py", line 325, in _get_connection
    conn = self._create_connector(bind, passwd)
  File "/usr/local/lib/python3.6/dist-packages/ldappool/__init__.py", line 266, in _create_connector
    self._bind(conn, bind, passwd)
  File "/usr/local/lib/python3.6/dist-packages/ldappool/__init__.py", line 230, in _bind
    conn.simple_bind_s(bind, passwd)
  File "/usr/local/lib/python3.6/dist-packages/ldappool/__init__.py", line 103, in simple_bind_s
    clientctrls)
  File "/usr/lib/python3/dist-packages/ldap/ldapobj...

[Corey Bryant (corey.bryant)]

@shashinandan, This isn't fixed yet. For the charms please see [1]. I'll try to get back to this to fix it as soon as I can.

[1] https://docs.openstack.org/charm-guide/latest/1811.html#python3-keystone-ldap-does-not-allow-bytes-for-dn-rdn-field-names

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/611190
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=eca0829c4c65e6b64f08023ce2d5a55dc329248f
Submitter: Zuul
Branch: master

commit eca0829c4c65e6b64f08023ce2d5a55dc329248f
Author: Corey Bryant <email address hidden>
Date: Tue Oct 16 16:19:15 2018 -0400

    PY3: switch to using unicode text values

    In Python 3, python-ldap no longer allows bytes for some fields (DNs,
    RDNs, attribute names, queries). Instead, text values are represented
    as str, the Unicode text type. Compatibility support is provided for
    Python 2 by setting bytes_mode=False [1].

    Update the keystone LDAP backend to adhere to this behavior by using
    bytes_mode=False for Python 2 and dropping UTF-8 encoding and decoding
    fields that are now represented as text in python-ldap.

    [1] More details about byte/str usage in python-ldap can be found at:
    http://www.python-ldap.org/en/latest/bytes_mode.html#bytes-mode

    Note that at a minimum python-ldappool 2.3.1 is required. For more
    details see Depends-On's below.

    Change-Id: Ifdd0644cd7042407a008c85c0b2c40a971c90bc3
    Closes-Bug: #1798184
    Depends-On: https://review.openstack.org/611401
    Depends-On: https://review.openstack.org/613632
    Depends-On: https://review.openstack.org/614052

[Corey Bryant (corey.bryant)]

Marked as fix released for upstream ldappool as this is fixed in 2.3.1

[Narinder Gupta (narindergupta)]

currently deploying rocky with bionic. So it is important to have packages backport for Rocky release.

[Corey Bryant (corey.bryant)]

New versions of python-ldappool and keystone have been uploaded to the cosmic unapproved queue where they are awaiting review by the SRU team [1]. Once they are accepted into cosmic-proposed they can be backported to rocky-proposed for testing.

[1] https://launchpad.net/ubuntu/cosmic/+queue?queue_state=1&queue_text=

[Brian Murray (brian-murray)]

Hello Corey, or anyone else affected,

Accepted python-ldappool into cosmic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/python-ldappool/2.2.0-3ubuntu2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-cosmic to verification-done-cosmic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-cosmic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[Brian Murray (brian-murray)]

Hello Corey, or anyone else affected,

Accepted keystone into cosmic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/keystone/2:14.0.1-0ubuntu2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-cosmic to verification-done-cosmic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-cosmic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[Corey Bryant (corey.bryant)]

Note: Related bug revealed in testing this SRU - https://bugs.launchpad.net/cloud-archive/+bug/1820333

[Corey Bryant (corey.bryant)]

Verification on cosmic-proposed failed (See bug in comment #19). I'm working on a fix for that.

[Brian Murray (brian-murray)]

Hello Corey, or anyone else affected,

Accepted keystone into cosmic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/keystone/2:14.0.1-0ubuntu3 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-cosmic to verification-done-cosmic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-cosmic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[Corey Bryant (corey.bryant)]

This has been verified successfully on cosmic-proposed and rocky-proposed. I've added testing details to the other bug that is accompanying this fix at:

https://bugs.launchpad.net/keystone/+bug/1820333/comments/7
https://bugs.launchpad.net/keystone/+bug/1820333/comments/8

[Corey Bryant (corey.bryant)]

The verification of the Stable Release Update for keystone has completed successfully and the package has now been released to -updates. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Corey Bryant (corey.bryant)]

This bug was fixed in the package keystone - 2:14.0.1-0ubuntu3~cloud0
---------------

 keystone (2:14.0.1-0ubuntu3~cloud0) bionic-rocky; urgency=medium
 .
   * New update for the Ubuntu Cloud Archive.
 .
 keystone (2:14.0.1-0ubuntu3) cosmic; urgency=medium
 .
   * d/control, d/p/py3-switch-to-using-unicode-text-values.patch: Enable
     Keystone LDAP Python 3 support. Patch is cherry-picked from
     https://review.openstack.org/613648 (LP: #1798184).
   * d/p/ensure-LDAP-searches-use-unicode-attributes.patch: Cherry-picked
     from https://review.openstack.org/#/c/643670/ to fix LDAP backend
     searches (LP: #1820333).

[Corey Bryant (corey.bryant)]

The verification of the Stable Release Update for python-ldappool has completed successfully and the package has now been released to -updates. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/613648
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=a2e7ccb4b32140f122c0beee0f3fcc1109db36bf
Submitter: Zuul
Branch: stable/rocky

commit a2e7ccb4b32140f122c0beee0f3fcc1109db36bf
Author: Corey Bryant <email address hidden>
Date: Tue Oct 16 16:19:15 2018 -0400

    PY3: switch to using unicode text values

    In Python 3, python-ldap no longer allows bytes for some fields (DNs,
    RDNs, attribute names, queries). Instead, text values are represented
    as str, the Unicode text type. Compatibility support is provided for
    Python 2 by setting bytes_mode=False [1].

    Update the keystone LDAP backend to adhere to this behavior by using
    bytes_mode=False for Python 2 and dropping UTF-8 encoding and decoding
    fields that are now represented as text in python-ldap.

    [1] More details about byte/str usage in python-ldap can be found at:
    http://www.python-ldap.org/en/latest/bytes_mode.html#bytes-mode

    Note that at a minimum python-ldappool 2.3.1 is required. For more
    details see Depends-On's below.

    Change-Id: Ifdd0644cd7042407a008c85c0b2c40a971c90bc3
    Closes-Bug: #1798184
    Depends-On: https://review.openstack.org/611401
    Depends-On: https://review.openstack.org/613632
    Depends-On: https://review.openstack.org/614052
    (cherry picked from commit eca0829c4c65e6b64f08023ce2d5a55dc329248f)

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/keystone 15.0.0.0rc1 release candidate.

[Corey Bryant (corey.bryant)]

This is fixed in rocky with keystone version 2:14.0.1-0ubuntu3.

[Launchpad Janitor (janitor)]

This bug was fixed in the package python-ldappool - 2.2.0-3ubuntu2

---------------
python-ldappool (2.2.0-3ubuntu2) cosmic; urgency=medium

  * d/gbp.conf: Create stable/rocky branch.
  * d/p/py3-switch-to-using-unicode-text-values.patch: Cherry-picked from
    upstream version 2.3.1 to enable Python 3 support (LP: #1798184).

 -- Corey Bryant <email address hidden> Wed, 13 Mar 2019 11:53:28 -0400

[Launchpad Janitor (janitor)]

This bug was fixed in the package keystone - 2:14.0.1-0ubuntu3

---------------
keystone (2:14.0.1-0ubuntu3) cosmic; urgency=medium

  * d/control, d/p/py3-switch-to-using-unicode-text-values.patch: Enable
    Keystone LDAP Python 3 support. Patch is cherry-picked from
    https://review.openstack.org/613648 (LP: #1798184).
  * d/p/ensure-LDAP-searches-use-unicode-attributes.patch: Cherry-picked
    from https://review.openstack.org/#/c/643670/ to fix LDAP backend
    searches (LP: #1820333).

 -- Corey Bryant <email address hidden> Tue, 19 Mar 2019 07:35:02 -0400

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/keystone 14.1.0 release.

[Shashinandan Kolar Srinivasa (shashinandan)]

Any pointers to upgrade my existing bionic-rocky deployment to 14.1.0 version of keystone?
Asking cause, "juju upgrade-charm keystone" did not upgrade the application version.

[Corey Bryant (corey.bryant)]

shashinandan, 'juju upgrade-charm' will just upgrade the charm to the latest charm version. To upgrade the keystone charm payload (ie. the openstack packages) take a look at the openstack-origin config option. See [1] for more details.

[1] https://docs.openstack.org/charm-guide/latest/1904.html#upgrading-openstack

[James Page (james-page)]

For stable updates, the charms won't provide any package upgrade capability; either logon to the server and apt dist-upgrade OR use one of the many systems management tools that allow you todo the same.

[Drew Freiberger (afreiberger)]

James, Corey,

UCA needs updating to provide 14.1.0 packages to bionic-rocky clouds.

ubuntu@juju-3e01e3-24-lxd-3:~$ sudo apt-cache policy keystone
keystone:
  Installed: 2:14.0.1-0ubuntu3~cloud0
  Candidate: 2:14.0.1-0ubuntu3~cloud0
  Version table:
 *** 2:14.0.1-0ubuntu3~cloud0 500
        500 http://ubuntu-cloud.archive.canonical.com/ubuntu bionic-updates/rocky/main amd64 Packages
        100 /var/lib/dpkg/status
     2:13.0.2-0ubuntu1 500
        500 http://archive.ubuntu.com/ubuntu bionic-updates/main amd64 Packages
     2:13.0.0-0ubuntu1 500
        500 http://archive.ubuntu.com/ubuntu bionic/main amd64 Packages

[Drew Freiberger (afreiberger)]

Sorry for my assumption there. I see that it appears it was backported into the UCA 14.0.1-0ubuntu3.

I should update that this does not fix all keystone-ldap functionality for rocky.

Please review lp#1832265 for additional code paths which may need similar patching, as this cloud is running 14.0.1-0ubuntu3.

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.opendev.org/672519

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/672519
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=03531a56910b12922afde32b40e270b7d68a334b
Submitter: Zuul
Branch: master

commit 03531a56910b12922afde32b40e270b7d68a334b
Author: Raildo Mascena <email address hidden>
Date: Wed Jul 24 10:20:17 2019 -0300

    Fix python3 compatibility on LDAP search DN from id

    In Python 3, python-ldap no longer allows bytes for some fields (DNs,
    RDNs, attribute names, queries). Instead, text values are represented
    as str, the Unicode text type.

    [1] More details about byte/str usage in python-ldap can be found at:
    http://www.python-ldap.org/en/latest/bytes_mode.html#bytes-mode

    Change-Id: I63e3715032cd8edb11fbff7651f5ba1af506dc9d
    Related-Bug: #1798184

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: stable/stein
Review: https://review.opendev.org/674022

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: stable/rocky
Review: https://review.opendev.org/674028

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: stable/queens
Review: https://review.opendev.org/674031

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/674031
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=79ed42ee67915383242541329dd5aa186f087ff2
Submitter: Zuul
Branch: stable/queens

commit 79ed42ee67915383242541329dd5aa186f087ff2
Author: Raildo Mascena <email address hidden>
Date: Wed Jul 24 10:20:17 2019 -0300

    Fix python3 compatibility on LDAP search DN from id

    In Python 3, python-ldap no longer allows bytes for some fields (DNs,
    RDNs, attribute names, queries). Instead, text values are represented
    as str, the Unicode text type.

    [1] More details about byte/str usage in python-ldap can be found at:
    http://www.python-ldap.org/en/latest/bytes_mode.html#bytes-mode

    Change-Id: I63e3715032cd8edb11fbff7651f5ba1af506dc9d
    Related-Bug: #1798184
    (cherry picked from commit 03531a56910b12922afde32b40e270b7d68a334b)

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.opendev.org/706791

[OpenStack Infra (hudson-openstack)]

Change abandoned by "Gage Hugo <email address hidden>" on branch: master
Review: https://review.opendev.org/c/openstack/keystone/+/706791
Reason: Abandoning since there hasn't been any recent activity, if anyone wants to continue this work, please feel free to restore this or create a new change.