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

System.DirectoryServices.Protocols - Cannot use SortControl with custom attribute on linux #41618

Closed
null-d3v opened this issue Aug 31, 2020 · 9 comments · Fixed by #65548
Closed
Labels
area-System.DirectoryServices enhancement Product code improvement that does NOT require public API changes/additions needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration os-linux Linux OS (any supported distro)
Milestone

Comments

@null-d3v
Copy link

Description

On linux, using SortRequestControl with a custom attribute will cause the search request to fail with the following:

System.DirectoryServices.Protocols.DirectoryOperationException: The server does not support the control. The control is critical.
    at System.DirectoryServices.Protocols.LdapConnection.ConstructResponseAsync(Int32 messageId, LdapOperation operation, ResultAll resultType, TimeSpan requestTimeOut, Boolean exceptionOnTimeOut, Boolean sync)
    at System.DirectoryServices.Protocols.LdapConnection.SendRequest(DirectoryRequest request, TimeSpan requestTimeout)
    at System.DirectoryServices.Protocols.LdapConnection.SendRequest(DirectoryRequest request)

I am able to use a sort control with something basic, like common name.

Sample:

var searchRequest = new SearchRequest(
    $"{LdapOptions.DefaultContainer},{LdapOptions.DnFragment}",
    $"(objectCategory={LdapConstants.UserObjectClass})",
    SearchScope.Subtree,
    LdapConstants.RequestedAttributes);

var sortRequestControl = new SortRequestControl(
    "customAttribute", false);

var vlvRequestControl = new VlvRequestControl(
    0,
    searchOptions.PageSize.Value - 1,
    (searchOptions.PageSize.Value * searchOptions.PageIndex.Value) + 1);

searchRequest.Controls.Add(sortRequestControl);
searchRequest.Controls.Add(vlvRequestControl);

var searchOptionsControl = new SearchOptionsControl(SearchOption.DomainScope);
searchRequest.Controls.Add(searchOptionsControl);

var searchResults = (SearchResponse)LdapConnection.SendRequest(searchRequest);

Configuration

  • .NET 5.0.0-preview.820414.8
  • aspnet:5.0-alpine docker image
  • x86_64
  • libldap 2.4
  • Windows Server 2019 DC

Regression

The exact code is functional when executing from the docker container's Windows host.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.DirectoryServices untriaged New issue has not been triaged by the area owner labels Aug 31, 2020
@ericstj ericstj added enhancement Product code improvement that does NOT require public API changes/additions os-linux Linux OS (any supported distro) and removed untriaged New issue has not been triaged by the area owner labels Sep 9, 2020
@ericstj ericstj added this to the Future milestone Sep 9, 2020
@ericstj
Copy link
Member

ericstj commented Sep 9, 2020

@joperezr any idea if this is a feature gap in OpenLDAP or something we could fix in our implementation?

@lscorcia
Copy link
Contributor

lscorcia commented Feb 5, 2022

+1 for this. Encountered the very same issue when querying AD (2008R2 domain) from Linux, using a sort control for the uid attribute. No issue when the same code is run from Windows.

I compared the exchanged packets on Linux and Windows and it seems that on Linux the attribute name gets truncated - possibly some marshalling issue?

immagine

@ghost ghost added the needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration label Feb 5, 2022
@lscorcia
Copy link
Contributor

lscorcia commented Feb 9, 2022

I think I have a possible explanation for the issue. I built the S.DS.P library on Linux in Debug mode and referenced that in my application instead of the framework provided one in order to debug it using VS Code. Everything is fine until the call to

This is the call that should produce the correct BER encoded value for the sort control. Stepping into it, we arrive at

Here, the value of each sort key is marshalled to an unmanaged pointer. The SortKey structure is annotated as using Unicode marshalling:

[StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)]
public class SortKey

This is fine on Windows, as the Interop layer is referencing the Unicode APIs:

[GeneratedDllImport(Libraries.Wldap32, EntryPoint = "ldap_create_sort_controlW", CharSet = CharSet.Unicode)]
[UnmanagedCallConv(CallConvs = new Type[] { typeof(System.Runtime.CompilerServices.CallConvCdecl) })]
public static partial int ldap_create_sort_control(ConnectionHandle handle, IntPtr keys, byte critical, ref IntPtr control);

But on Linux, we are actually invoking the ANSI ones:

[GeneratedDllImport(Libraries.OpenLdap, EntryPoint = "ldap_create_sort_control", CharSet = CharSet.Ansi)]
public static partial int ldap_create_sort_control(ConnectionHandle handle, IntPtr keys, byte critical, ref IntPtr control);

So what is happening is that the API finds a null terminator right after the first character and generates the truncated value I found in the network trace. Unless I am mistaken in the details of the marshalling process, I guess this is really a bug and that there is no applicable workaround. I would love some feedback, at least to validate that what I learnt from this investigation is correct!

Also - is there a way to visualize the content of unmanaged memory during debugging in VS Code? It would be nice to have visual confirmation of the issue by inspecting the marshalled pointers.

@lscorcia
Copy link
Contributor

Anyone that can triage this? Maybe @joperezr ?

@joperezr joperezr modified the milestones: Future, 7.0.0 Feb 15, 2022
@joperezr
Copy link
Member

Hey @lscorcia so sorry for missing the notification and thank you a lot for the very detailed report. I do think what you describe about the difference in Ansi vs Unicode is the culprit and it does sound like a very reasonable explanation of why SortControl (and any other controls for that matter) won't work today in Linux. I also agree with you that this is a bug and that we should fix it.

I've adjusted the milestone to 7.0.0, but I don't currently have the cycles to work on this. @lscorcia Would you be interested in contributing a fix for this issue 😃 ?

@lscorcia
Copy link
Contributor

Thank you @joperezr . I just committed a fix on my fork (see above), if it's ok for you I'll open a PR.
I also noticed #34679 which could be related. Should I touch that ActiveIssue attribute?

@joperezr
Copy link
Member

Thanks for sharing that commit @lscorcia, there might be some small comments in there, but in general I think it has the right idea so please do open a PR with your fix. I assume you have been able to validate that this fix addresses the issue and now you are able to use the Sort Controls?

@lscorcia
Copy link
Contributor

lscorcia commented Feb 17, 2022

I tested it by calling it with my code and it works as expected, I did not run the full runtime test suite as it would be too heavy for my VM but I suppose that will run as a Github action when I submit the PR tomorrow. Thanks for your help!

@lscorcia
Copy link
Contributor

PR #65548 is ready for feedback!

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Feb 18, 2022
Repository owner moved this from Future to Done in Triage POD for Reflection, META, etc. Mar 22, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Mar 22, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Apr 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.DirectoryServices enhancement Product code improvement that does NOT require public API changes/additions needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration os-linux Linux OS (any supported distro)
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

5 participants