LDAP connection code does not provide ldap.set_option(ldap.OPT_X_TLS_CACERTFILE) for ldaps protocol

Bug #1209343 reported by Mark Miller
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
OpenStack Identity (keystone)
Fix Released
Wishlist
Nathan Kinder
Icehouse
Fix Released
Wishlist
Nathan Kinder

Bug Description

The HP Enterprise Directory LDAP servers require a ca certificate file for ldaps connections. Sample working Python code:

        ldap.set_option(ldap.OPT_X_TLS_CACERTFILE, "d:/etc/ssl/certs/hpca2ssG2_ns.cer")
        ldap_client = ldap.initialize(host)
        ldap_client.protocol_version = ldap.VERSION3

        ldap_client.simple_bind_s(binduser,bindpw)

        filter = '(uid=mark.m*)'
        attrs = ['cn', 'mail', 'uid', 'hpStatus']

        r = ldap_client.search_s(base, scope, filter, attrs)

        for dn, entry in r:
            print 'dn=', repr(dn)

            for k in entry.keys():
                print '\t', k, '=', entry[k]

The current H-2 " keystone/common/ldap/core.py" file only provides this ldap.set_option for TLS connections. I have attached a picture of a screen shot showing the change I had to make to file core.py to enable the "ldap.set_option(ldap.OPT_X_TLS_CACERTFILE, tls_cacertfile)" statement to also get executed for ldaps connections. Basically I pulled the set_option code out of the "if tls_cacertfile:" block.

Revision history for this message
Mark Miller (mark-m-miller) wrote :
Revision history for this message
Adam Young (ayoung) wrote :

Do not have access to HP ED, so assigning to gyee, who is at HP.

Changed in keystone:
assignee: nobody → Guang Yee (guang-yee)
Dolph Mathews (dolph)
Changed in keystone:
importance: Undecided → Wishlist
status: New → Triaged
Revision history for this message
Guang Yee (guang-yee) wrote :

Not sure if this is a valid bug, code says we do support CA certs

https://github.com/openstack/keystone/blob/master/keystone/common/ldap/core.py#L481
https://github.com/openstack/keystone/blob/master/etc/keystone.conf.sample#L285

Did you try configuring the CA certs in your keystone.conf file?

[ldap]

use_tls = True
tls_cacertfile =/etc/ssl/certs/hpca2ssG2_ns.cer

Revision history for this message
Mark Miller (mark-m-miller) wrote : RE: [Bug 1209343] Re: Split backend does not provide ldap.set_option(ldap.OPT_X_TLS_CACERTFILE) for ldaps connections
Download full text (6.1 KiB)

With 28 years of programming experience behind me I am almost certain that this is a bug, a general “LDAPS” bug that I just happened to run into while working at HP.

File keystone.conf:

        # ldap TLS options
      # if both tls_cacertfile and tls_cacertdir are set then
      # tls_cacertfile will be used and tls_cacertdir is ignored
      # valid options for tls_req_cert are demand, never, and allow
      use_tls = True
      tls_cacertfile = "/etc/keystone/ssl/certs/hpca2ssG2_ns.cer"

Class LdapWrapper will not let you specify both "user_tls" and "ldaps". If you do it raises an exception.

              if use_tls and using_ldaps:
                  raise AssertionError(_('Invalid TLS / LDAPS combination'))

With ldaps, not ldap, you need to specify a cacertificate to use with the secure connection which means that you need to execute statement "ldap.set_option(ldap.OPT_X_TLS_CACERTFILE, tls_cacertfile)" which unfortuntely is enclosed in an “if use_tls” block that you cannot get to if you are using ldaps. Bottom line, ldaps connections also need access to this line of code.

      class LdapWrapper(object):
          def __init__(self, url, page_size, alias_dereferencing=None,
                       use_tls=False, tls_cacertfile=None, tls_cacertdir=None,
                       tls_req_cert='demand'):
                        'tls_cacertfile=%(tls_cacertfile)s\n'
                        'tls_cacertdir=%(tls_cacertdir)s\n'
                        'tls_req_cert=%(tls_req_cert)s\n'
                        'tls_avail=%(tls_avail)s\n') %
                        {'use_tls': use_tls,
                         'tls_cacertfile': tls_cacertfile,
                         'tls_cacertdir': tls_cacertdir,
                         'tls_req_cert': tls_req_cert,
                         'tls_avail': ldap.TLS_AVAIL
                         })

              #NOTE(topol)
              #for extra debugging uncomment the following line
              #ldap.set_option(ldap.OPT_DEBUG_LEVEL, 4095)

              using_ldaps = url.lower().startswith("ldaps")

              if use_tls and using_ldaps:
                  raise AssertionError(_('Invalid TLS / LDAPS combination'))

              if use_tls:
                  if not ldap.TLS_AVAIL:
                      raise ValueError(_('Invalid LDAP TLS_AVAIL option: %s. TLS '
                                         'not available') % ldap.TLS_AVAIL)
                  if tls_cacertfile:
                      #NOTE(topol)
                      #python ldap TLS does not verify CACERTFILE or CACERTDIR
                      #so we add some extra simple sanity check verification
                      #Also, setting these values globally (i.e. on the ldap object)
                      #works but these values are ignored when setting them on the
                      #connection
                      if not os.path.isfile(tls_cacertfile):
                          raise IOError(_("tls_cacertfile %s not found "
                                          "or is not a file") %
                                        tls_cacertfile)
                      ldap.set_optio...

Read more...

Revision history for this message
Mark Miller (mark-m-miller) wrote : Re: Split backend does not provide ldap.set_option(ldap.OPT_X_TLS_CACERTFILE) for ldaps connections

I just tested the HP Enterprise Directory LDAP connection using an "ldap" connection with "user_tls=True" and it works so we have a viable solution. However, I still think that LDAPS connections should allow for certificates as this is a standard connection methodology with LDAPS:

The following link has a code sample that matches the example from the HP Enterprise Directory web site:

    http://www.techques.com/question/1-2193362/How-to-connect-to-a-LDAP-server-using-a-p12-certificate

----

    ldap.set_option(ldap.OPT_X_TLS_CACERTFILE, "/path/to/trustedcerts.pem")
    ldap.set_option(ldap.OPT_X_TLS_CERTFILE, "/path/to/usercert.pem")
    ldap.set_option(ldap.OPT_X_TLS_KEYFILE, "/path/to/user.key.pem")

    ds = ldap.initialize("ldaps://ldap.example.com:port/")
    # If using START_TLS instead of ldaps:
    # ds = ldap.initialize("ldap://ldap.example.com:port/")
    # ds.start_tls_s()

Revision history for this message
Fabius Lovato (fabius-lovato) wrote :

What do you think about this patch?
I suggest pull all set_option code out of the "if tls_cacertfile:" block.
It because once the 'ldap.set_option(ldap.OPT_X_TLS_REQUIRE_CERT, tls_req_cert)' code does not depend on the 'use_tls' option, we can use self-signed certificate with LDAPS.

Revision history for this message
Guang Yee (guang-yee) wrote :

That should work. Not sure how are we going to unit test this though.

Revision history for this message
Mark Miller (mark-m-miller) wrote : RE: [Bug 1209343] Re: Split backend does not provide ldap.set_option(ldap.OPT_X_TLS_CACERTFILE) for ldaps connections

Sorry for taking so long to answer, I flagged this email for follow up but almost forgot about it until now.

In looking at the proposed solution, I think the last if/else statement (if tls_req_cert in LDAP_TLS_CERTS.values() )may need to remain indented under the "if use_tls" block. I found that for LDAPS using a cert file, I only need the "if tls_cacertfile" block to load the certificiate for the HP LDAPS connection. If the last if/else statement is left un-indented as proposed, then the else condition will log a misleading debug line to the log files.

Mark

> -----Original Message-----
> From: <email address hidden> [mailto:<email address hidden>] On Behalf
> Of Lovato, Fabius (Brazil R&D-ECL)
> Sent: Friday, November 08, 2013 4:42 AM
> To: Miller, Mark M (EB SW Cloud - R&D - Corvallis)
> Subject: [Bug 1209343] Re: Split backend does not provide
> ldap.set_option(ldap.OPT_X_TLS_CACERTFILE) for ldaps connections
>
> What do you think about this patch?
> I suggest pull all set_option code out of the "if tls_cacertfile:" block.
> It because once the 'ldap.set_option(ldap.OPT_X_TLS_REQUIRE_CERT,
> tls_req_cert)' code does not depend on the 'use_tls' option, we can use self-
> signed certificate with LDAPS.
>
> ** Attachment added: "suggested_patch.png"
>
> https://bugs.launchpad.net/keystone/+bug/1209343/+attachment/3903141/
> +files/suggested_patch.png
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1209343
>
> Title:
> Split backend does not provide
> ldap.set_option(ldap.OPT_X_TLS_CACERTFILE) for ldaps connections
>
> Status in OpenStack Identity (Keystone):
> Triaged
>
> Bug description:
> The HP Enterprise Directory LDAP servers require a ca certificate file
> for ldaps connections. Sample working Python code:
>
> ldap.set_option(ldap.OPT_X_TLS_CACERTFILE,
> "d:/etc/ssl/certs/hpca2ssG2_ns.cer")
> ldap_client = ldap.initialize(host)
> ldap_client.protocol_version = ldap.VERSION3
>
> ldap_client.simple_bind_s(binduser,bindpw)
>
> filter = '(uid=mark.m*)'
> attrs = ['cn', 'mail', 'uid', 'hpStatus']
>
> r = ldap_client.search_s(base, scope, filter, attrs)
>
> for dn, entry in r:
> print 'dn=', repr(dn)
>
> for k in entry.keys():
> print '\t', k, '=', entry[k]
>
> The current H-2 " keystone/common/ldap/core.py" file only provides
> this ldap.set_option for TLS connections. I have attached a picture of
> a screen shot showing the change I had to make to file core.py to
> enable the "ldap.set_option(ldap.OPT_X_TLS_CACERTFILE,
> tls_cacertfile)" statement to also get executed for ldaps connections.
> Basically I pulled the set_option code out of the "if tls_cacertfile:"
> block.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/keystone/+bug/1209343/+subscriptions

Dolph Mathews (dolph)
tags: added: ldap
removed: backend keystone ldaps split
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to keystone (master)

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

Changed in keystone:
assignee: Guang Yee (guang-yee) → Nathan Kinder (nkinder)
status: Triaged → In Progress
Nathan Kinder (nkinder)
tags: added: icehouse-backport-potential
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to keystone (stable/icehouse)

Fix proposed to branch: stable/icehouse
Review: https://review.openstack.org/120959

Changed in keystone:
milestone: none → juno-rc1
Nathan Kinder (nkinder)
summary: - Split backend does not provide
- ldap.set_option(ldap.OPT_X_TLS_CACERTFILE) for ldaps connections
+ LDAP connection code does not provide
+ ldap.set_option(ldap.OPT_X_TLS_CACERTFILE) for ldaps protocol
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to keystone (master)

Reviewed: https://review.openstack.org/120954
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=ca50b6315909faf92db9eaf2981782bcd80cfab8
Submitter: Jenkins
Branch: master

commit ca50b6315909faf92db9eaf2981782bcd80cfab8
Author: Nathan Kinder <email address hidden>
Date: Thu Sep 11 17:41:09 2014 -0700

    Set LDAP certificate trust options for LDAPS and TLS

    We currently only set the LDAP library certificate trust options
    when TLS is being used (via startTLS). If regular LDAPS is being
    used, the certificate trust options that are defined in Keystone's
    configuration file are never actually set.

    This patch sets the certificate trust options for both LDAPS and
    TLS cases.

    Closes-bug: #1209343
    Change-Id: Ieb94b732f623695920feb34995bb863175ddf27a

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

Reviewed: https://review.openstack.org/120959
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=1c8f9f98fe901a0aaee5a223ce55a8431989c4ac
Submitter: Jenkins
Branch: stable/icehouse

commit 1c8f9f98fe901a0aaee5a223ce55a8431989c4ac
Author: Nathan Kinder <email address hidden>
Date: Thu Sep 11 17:41:09 2014 -0700

    Set LDAP certificate trust options for LDAPS and TLS

    We currently only set the LDAP library certificate trust options
    when TLS is being used (via startTLS). If regular LDAPS is being
    used, the certificate trust options that are defined in Keystone's
    configuration file are never actually set.

    This patch sets the certificate trust options for both LDAPS and
    TLS cases.

    Conflicts:
            keystone/common/ldap/core.py
            keystone/tests/unit/common/test_ldap.py

    Closes-bug: #1209343
    Change-Id: Ieb94b732f623695920feb34995bb863175ddf27a
    (cherry picked from commit ca50b6315909faf92db9eaf2981782bcd80cfab8)

tags: added: in-stable-icehouse
Thierry Carrez (ttx)
Changed in keystone:
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in keystone:
milestone: juno-rc1 → 2014.2
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.