Skip to content
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

rfc2696 paging not working when server returns additional, empty pagedResultsControl items #313

Open
martindorey opened this issue Aug 7, 2018 · 1 comment

Comments

@martindorey
Copy link

I've got a customer whose LDAP server repeatably returns a result like this:

LDAPMessage searchResDone(2) success [126 results]
    messageID: 2
    protocolOp: searchResDone (5)
        searchResDone
            resultCode: success (0)
            matchedDN:
            errorMessage:
    [Response To: 28]
    [Time: 1.821359773 seconds]
    controls: 4 items
        Control
            controlType: 1.2.840.113556.1.4.319 (pagedResultsControl)
            SearchControlValue
                size: 0
                cookie: 006f753d7573657273006f753d70656f706c65006f753d65...
        Control
            controlType: 1.2.840.113556.1.4.319 (pagedResultsControl)
            SearchControlValue
                size: 0
                cookie: <MISSING>
        Control
            controlType: 1.2.840.113556.1.4.319 (pagedResultsControl)
            SearchControlValue
                size: 0
                cookie: <MISSING>
        Control
            controlType: 1.2.840.113556.1.4.319 (pagedResultsControl)
            SearchControlValue
                size: 0
                cookie: <MISSING>

That is to say one containing four pagedResultsControls, only the first of which has a non-empty cookie. I don't think it's important but, for the record, the query looked like this:

LDAPMessage searchRequest(2) "DC=example,DC=com" wholeSubtree
    messageID: 2
    protocolOp: searchRequest (3)
        searchRequest
            baseObject: DC=example,DC=com
            scope: wholeSubtree (2)
            derefAliases: neverDerefAliases (0)
            sizeLimit: 0
            timeLimit: 0
            typesOnly: False
            Filter: (objectClass=posixAccount)
                filter: equalityMatch (3)
                    equalityMatch
                        attributeDesc: objectClass
                        assertionValue: posixAccount
            attributes: 7 items
                AttributeDescription: uid
                AttributeDescription: userPassword
                AttributeDescription: uidNumber
                AttributeDescription: gidNumber
                AttributeDescription: cn
                AttributeDescription: homeDirectory
                AttributeDescription: loginShell
    controls: 1 item
        Control
            controlType: 1.2.840.113556.1.4.319 (pagedResultsControl)
            criticality: False
            SearchControlValue
                size: 126
                cookie: <MISSING>

Judging by a comment in the commit where the code for handling paging was originally added, b67a91e, the original author, @blackhedd, had seen such multiple-control results:

          more_pages = false # just in case some bogus server sends us >1 of these.

The effect of that line was that the last pagedResultsControl wins. I can see myself writing such code... but it stops paging from continuing for this customer, where it's only the first one that contains anything useful. If I comment out that assignment, then all seems well.

The contribution guidelines ask that I disclose that the customer's using ruby-net-ldap 0.8.0, the last version that supports Ruby 1.8.7, which is the last version readily available for the Linux distro on which this code runs. Source code inspection leads me to think that the current code would behave the same way, so I might tentatively suggest "fixing" it with:

martind@swiftboat:~/work/ruby-net-ldap$ git diff ./lib/net/ldap/connection.rb 
diff --git a/lib/net/ldap/connection.rb b/lib/net/ldap/connection.rb
index 61aacb5..22f80af 100644
--- a/lib/net/ldap/connection.rb
+++ b/lib/net/ldap/connection.rb
@@ -486,7 +486,7 @@ class Net::LDAP::Connection #:nodoc:
           controls.each do |c|
             if c.oid == Net::LDAP::LDAPControls::PAGED_RESULTS
               # just in case some bogus server sends us more than 1 of these.
-              more_pages = false
+              #more_pages = false
               if c.value and c.value.length > 0
                 cookie = c.value.read_ber[1]
                 if cookie and cookie.length > 0
martind@swiftboat:~/work/ruby-net-ldap$ 

The contributing guidelines also ask that I say which LDAP server the issue happens with. That's a perfectly reasonable question but not one that I can currently answer, though I have asked and live in hope of receiving the information. I can say that I just see a single, sane cookie with a Windows 2012R2 server. I don't see anything in https://www.ietf.org/rfc/rfc2696.txt that would sanction multiple cookies.

The contribution guidelines ask for test code. I did look to see if there was any current test for paging that I could leverage but didn't find one. If I downgrade to rake 10.5.0 (at the suggestion of hanami/utils#123 (comment)), then I get 100% passed with the change. Newer versions of rake fail before the change too because of a Ruby warning polluting a deprecation warning:

<"Deprecation warning: Net::LDAP::ConnectionRefused will be deprecated. Use Errno::ECONNREFUSED instead.\n"> expected but was
<"/home/martind/work/ruby-net-ldap/lib/net/ldap/connection.rb:184: warning: instance variable @conn not initialized\nDeprecation warning: Net::LDAP::ConnectionRefused will be deprecated. Use Errno::ECONNREFUSED instead.\n">

rubocop fails too but in a way that it failed before my change, so perhaps there too I could downgrade to get a pass. There were some warnings about .rubocop_todo.yml before this, which was mainly in red:

Error: The `Style/OpMethod` cop has been renamed and moved to `Naming/BinaryOperatorParameterName`.
(obsolete configuration found in .rubocop_todo.yml, please update it)
The `Style/MethodMissing` cop has been split into `Style/MethodMissingSuper` and `Style/MissingRespondToMissing`.
(obsolete configuration found in .rubocop_todo.yml, please update it)
RuboCop failed!

My code was previously manually paging on top of the other Ruby LDAP library, https://github.com/bearded/ruby-ldap. I think my code would only have looked at the first paged results control. That wasn't reported as having caused a problem, so I imagine the cookie is always in the first control for all the customers using this piece of my code.

Our-ref: D135858

@martindorey
Copy link
Author

I'm told that "the OS is RHEL 6.5 and the software is Open DJ version 2.6". I've had a look at the source for that and didn't find anything that looked like it could plausibly generate the response seen. I installed a version advertising itself as 2.6.4, given it some example data and performed the same sort of query against it. That generated a perfectly normal looking single cookie response. If it weren't for @blackhedd's code comment, I would assume that there's something odd with this particular customer's installation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant