[SRU] PooledLDAPHandler.result3 does not release pool connection back when an exception is raised

Bug #1998789 reported by Mustafa Kemal Gilor
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Identity (keystone)
Fix Released
Undecided
Mustafa Kemal Gilor
Ubuntu Cloud Archive
Fix Released
Undecided
Unassigned
Antelope
Fix Committed
Undecided
Unassigned
Ussuri
Triaged
Undecided
Unassigned
Victoria
Fix Committed
Undecided
Unassigned
Wallaby
Fix Committed
Undecided
Unassigned
Xena
Fix Committed
Undecided
Unassigned
Yoga
Fix Committed
Undecided
Unassigned
Zed
Fix Committed
Undecided
Unassigned
keystone (Ubuntu)
Fix Released
Undecided
Unassigned
Focal
Triaged
Undecided
Unassigned
Jammy
Fix Committed
Undecided
Unassigned
Lunar
Fix Committed
Undecided
Unassigned

Bug Description

[Impact]

This SRU is a backport of https://review.opendev.org/c/openstack/keystone/+/866723 to the respective Ubuntu and UCA releases. The patch is merged to the all respective upstream branches (master & stable/[u,v,w,x,y,z]).

This SRU intends to fix a denial-of-service bug that happens when keystone uses pooled ldap connections. In pooled ldap connection mode, keystone borrows a connection from the pool, do the LDAP operation and release it back to the pool. But, if an exception or error happens while the LDAP connection is still borrowed, Keystone fails to release the connection back to the pool, hogging it forever. If this happens for all the pooled connections, the connection pool will be exhausted and Keystone will no longer be able to perform LDAP operations.

The fix corrects this behavior by allowing the connection to release back to the pool even if an exception/error happens during the LDAP operation.

[Test Case]

- Deploy an LDAP server of your choice
- Fill it with many data so the search takes more than `pool_connection_timeout` seconds
- Define a keystone domain with the LDAP driver with following options:

[ldap]
use_pool = True
page_size = 100
pool_connection_timeout = 3
pool_retry_max = 3
pool_size = 10

- Point the domain to the LDAP server
- Try to login to the OpenStack dashboard, or try to do anything that uses the LDAP user
- Observe the /var/log/apache2/keystone_error.log, it should contain ldap.TIMEOUT() stack traces followed by `ldappool.MaxConnectionReachedError` stack traces

To confirm the fix, repeat the scenario and observe that the "/var/log/apache2/keystone_error.log" does not contain `ldappool.MaxConnectionReachedError` stack traces and LDAP operation in motion is successful (e.g. OpenStack Dashboard login)

[Regression Potential]
The patch is quite trivial and should not affect any deployment in a negative way. The LDAP pool functionality can be disabled by setting "use_pool=False" in case of any regression.

summary: PooledLDAPHandler.result3 does not release pool connection back when an
- exception raises
+ exception is raised
Changed in keystone:
assignee: nobody → Mustafa Kemal Gilor (mustafakemalgilor)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to keystone (master)

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/keystone/+/866723

Changed in keystone:
status: New → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to keystone (master)

Reviewed: https://review.opendev.org/c/openstack/keystone/+/866723
Committed: https://opendev.org/openstack/keystone/commit/ff632a81fb09e6d9f3298e494d53eb6df50269cf
Submitter: "Zuul (22348)"
Branch: master

commit ff632a81fb09e6d9f3298e494d53eb6df50269cf
Author: Mustafa Kemal Gilor <email address hidden>
Date: Mon Dec 5 17:33:47 2022 +0300

    [PooledLDAPHandler] Ensure result3() invokes message.clean()

    result3 does not invoke message.clean() when an exception is thrown
    by `message.connection.result3()` call, causing pool connection
    associated with the message to be marked active forever. This causes
    a denial-of-service on ldappool.

    The fix ensures message.clean() is invoked by wrapping the offending
    call in try-except-finally and putting the message.clean() in finally
    block.

    Closes-Bug: #1998789

    Change-Id: I59ebf0fa77391d49b2349e918fc55f96318c42a6
    Signed-off-by: Mustafa Kemal Gilor <email address hidden>

Changed in keystone:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to keystone (stable/zed)

Fix proposed to branch: stable/zed
Review: https://review.opendev.org/c/openstack/keystone/+/874841

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to keystone (stable/yoga)

Fix proposed to branch: stable/yoga
Review: https://review.opendev.org/c/openstack/keystone/+/874842

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to keystone (stable/xena)

Fix proposed to branch: stable/xena
Review: https://review.opendev.org/c/openstack/keystone/+/874843

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to keystone (stable/wallaby)

Fix proposed to branch: stable/wallaby
Review: https://review.opendev.org/c/openstack/keystone/+/874844

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to keystone (stable/train)

Fix proposed to branch: stable/train
Review: https://review.opendev.org/c/openstack/keystone/+/874845

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to keystone (stable/ussuri)

Fix proposed to branch: stable/ussuri
Review: https://review.opendev.org/c/openstack/keystone/+/874846

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to keystone (stable/victoria)

Fix proposed to branch: stable/victoria
Review: https://review.opendev.org/c/openstack/keystone/+/874847

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/keystone 23.0.0.0rc1

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on keystone (stable/train)

Change abandoned by "Mustafa Kemal Gilor <email address hidden>" on branch: stable/train
Review: https://review.opendev.org/c/openstack/keystone/+/874845

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to keystone (stable/zed)

Reviewed: https://review.opendev.org/c/openstack/keystone/+/874841
Committed: https://opendev.org/openstack/keystone/commit/7c30c9e000b58055b61d8cf58e52493f2b5aba8a
Submitter: "Zuul (22348)"
Branch: stable/zed

commit 7c30c9e000b58055b61d8cf58e52493f2b5aba8a
Author: Mustafa Kemal Gilor <email address hidden>
Date: Mon Dec 5 17:33:47 2022 +0300

    [PooledLDAPHandler] Ensure result3() invokes message.clean()

    result3 does not invoke message.clean() when an exception is thrown
    by `message.connection.result3()` call, causing pool connection
    associated with the message to be marked active forever. This causes
    a denial-of-service on ldappool.

    The fix ensures message.clean() is invoked by wrapping the offending
    call in try-except-finally and putting the message.clean() in finally
    block.

    Closes-Bug: #1998789

    Change-Id: I59ebf0fa77391d49b2349e918fc55f96318c42a6
    Signed-off-by: Mustafa Kemal Gilor <email address hidden>
    (cherry picked from commit ff632a81fb09e6d9f3298e494d53eb6df50269cf)

tags: added: in-stable-zed
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to keystone (stable/yoga)

Reviewed: https://review.opendev.org/c/openstack/keystone/+/874842
Committed: https://opendev.org/openstack/keystone/commit/7c96280555d1de5ef5e7e3b12362439669427e4e
Submitter: "Zuul (22348)"
Branch: stable/yoga

commit 7c96280555d1de5ef5e7e3b12362439669427e4e
Author: Mustafa Kemal Gilor <email address hidden>
Date: Mon Dec 5 17:33:47 2022 +0300

    [PooledLDAPHandler] Ensure result3() invokes message.clean()

    result3 does not invoke message.clean() when an exception is thrown
    by `message.connection.result3()` call, causing pool connection
    associated with the message to be marked active forever. This causes
    a denial-of-service on ldappool.

    The fix ensures message.clean() is invoked by wrapping the offending
    call in try-except-finally and putting the message.clean() in finally
    block.

    Closes-Bug: #1998789

    Change-Id: I59ebf0fa77391d49b2349e918fc55f96318c42a6
    Signed-off-by: Mustafa Kemal Gilor <email address hidden>
    (cherry picked from commit ff632a81fb09e6d9f3298e494d53eb6df50269cf)

tags: added: in-stable-yoga
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to keystone (stable/xena)

Reviewed: https://review.opendev.org/c/openstack/keystone/+/874843
Committed: https://opendev.org/openstack/keystone/commit/3e5805e40e26b51ce5c4a69aa31d81226d38ccf9
Submitter: "Zuul (22348)"
Branch: stable/xena

commit 3e5805e40e26b51ce5c4a69aa31d81226d38ccf9
Author: Mustafa Kemal Gilor <email address hidden>
Date: Mon Dec 5 17:33:47 2022 +0300

    [PooledLDAPHandler] Ensure result3() invokes message.clean()

    result3 does not invoke message.clean() when an exception is thrown
    by `message.connection.result3()` call, causing pool connection
    associated with the message to be marked active forever. This causes
    a denial-of-service on ldappool.

    The fix ensures message.clean() is invoked by wrapping the offending
    call in try-except-finally and putting the message.clean() in finally
    block.

    Closes-Bug: #1998789

    Change-Id: I59ebf0fa77391d49b2349e918fc55f96318c42a6
    Signed-off-by: Mustafa Kemal Gilor <email address hidden>
    (cherry picked from commit ff632a81fb09e6d9f3298e494d53eb6df50269cf)

tags: added: in-stable-xena
Revision history for this message
Mustafa Kemal Gilor (mustafakemalgilor) wrote :
summary: - PooledLDAPHandler.result3 does not release pool connection back when an
- exception is raised
+ [SRU] PooledLDAPHandler.result3 does not release pool connection back
+ when an exception is raised
description: updated
Revision history for this message
Mustafa Kemal Gilor (mustafakemalgilor) wrote :
Revision history for this message
Mustafa Kemal Gilor (mustafakemalgilor) wrote :
Revision history for this message
Mustafa Kemal Gilor (mustafakemalgilor) wrote :
Revision history for this message
Mustafa Kemal Gilor (mustafakemalgilor) wrote :
Revision history for this message
Mustafa Kemal Gilor (mustafakemalgilor) wrote :
Revision history for this message
Mustafa Kemal Gilor (mustafakemalgilor) wrote :
Revision history for this message
Mustafa Kemal Gilor (mustafakemalgilor) wrote :
Revision history for this message
Ubuntu Foundations Team Bug Bot (crichton) wrote :

The attachment "focal.debdiff" seems to be a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. If the attachment isn't a patch, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are member of the ~ubuntu-sponsors, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issue please contact him.]

tags: added: patch
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/keystone 21.0.1

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/keystone 22.0.1

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

Changed in keystone (Ubuntu Lunar):
status: New → Fix Released
Revision history for this message
Corey Bryant (corey.bryant) wrote :

Thanks Mustafa. For the SRU, this will be included for victoria+ in https://bugs.launchpad.net/ubuntu/+source/keystone/+bug/2039176. Let's target this bug for focal/ussuri.

Changed in keystone (Ubuntu Lunar):
status: Fix Released → Fix Committed
Changed in keystone (Ubuntu Jammy):
status: New → Fix Committed
Changed in keystone (Ubuntu):
status: New → Fix Released
Changed in cloud-archive:
status: New → Fix Released
Changed in keystone (Ubuntu Focal):
status: New → Triaged
Revision history for this message
Corey Bryant (corey.bryant) wrote :

Mustafa, this has been uploaded to the focal unapproved queue: https://launchpad.net/ubuntu/focal/+queue?queue_state=1&queue_text=keystone

Revision history for this message
Lukas Märdian (slyon) wrote :

Removing ~ubuntu-sponsors, as this got sponsored already for Focal and is included in bug #2039176 for the other series.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.