go-ldap / ldap

Basic LDAP v3 functionality for the GO programming language.
Other
2.19k stars 352 forks source link

Server Side Sorting on OpenLDAP #506

Open fletch-hogan opened 2 months ago

fletch-hogan commented 2 months ago

There looks to be a bug that has been introduced at some point with regards to SSS Control on OpenLDAP. If you add a ControlServerSideSorting to a SearchRequest, it returns an error "failed to decode child control: bad packet". If I filter out that error, the SearchResult contains the sorted results, so clearly the sss control is being sent and processed correctly.

I haven't spent the time to reverse the code to understand why, but it seems to me that if there is an sss control, for some reason the code is expecting there to the an sss control result and breaks if there isn't. There is a note in the code saying OpenLDAP doesn't return sss control result. ref: https://github.com/go-ldap/ldap/blob/master/control.go#L891. That isn't a reason to break search functionally.

Perhaps a re-think on the logic to return nil rather than a "bad packet" error. i.e. Allow the sss control result to be optional. ref: https://github.com/go-ldap/ldap/blob/master/control.go#L870

Cheers

fletch-hogan commented 2 months ago

In a related find, it seems this issue also breaks SearchWithPaging, as it will only return "pagingSize" results due to hitting the error on the first packet return. The test ou has 100 entries, however only the first 10 are returned. When the control is removed, all 100 are returned which is the expected behaviour.

func TestSearchManual(t *testing.T) {
    request := &ldap.SearchRequest{
        BaseDN:     "ou=test,dc=example,dc=com",
        Scope:      ldap.ScopeWholeSubtree,
        Filter:     "(cn=*)",
        Attributes: []string{"cn"},
        Controls: []ldap.Control{ldap.NewControlServerSideSortingWithSortKeys([]*ldap.SortKey{
            {AttributeType: "cn", MatchingRule: "caseIgnoreOrderingMatch", Reverse: false},
        })},
    }

    result, err := ldapConn.SearchWithPaging(request, 10)
    if err != nil {
        if err.Error() != "failed to decode child control: bad packet" {
            t.Fatalf("Search Failed: %s", err.Error())
        }
    }

    t.Errorf("%+v\n", request)
    t.Errorf("%+v\n", result)
    t.Errorf("%d\n", len(result.Entries))
}

Result: &{Entries:[0x140000ca990 0x140000ca9c0 0x14000280270 0x140001ebaa0 0x140001ebad0 0x140001ebb00 0x140001ebb30 0x140001ebb60 0x140001ebb90 0x140001ebbc0] Referrals:[] Controls:[]}

cpuschma commented 2 months ago

Thanks for pointing this out, I'll look into this and extend our test cases.

cpuschma commented 2 months ago

@fletch-hogan What directory server were you using?

fletch-hogan commented 2 months ago

OpenLDAP with the sssvlv overlay.