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 - Linux -> search query with escape sequence character (\) #38609

Open
GrowSharp opened this issue Jun 30, 2020 · 16 comments
Assignees
Labels
Milestone

Comments

@GrowSharp
Copy link

Description

When I try to search for a role by it's CN that contains the \ character, I get The LDAP server returned an unknown error. (ldap error code: -7)
Search request:
var searchRequest = new SearchRequest(LdapConstants.SETTING_GROUP_BASE_DIRECTORY, searchFilter, SearchScope.Subtree, LdapConstants.SettingGroupSearchAttributeFilter.ToArray());

Configuration

Domain joined CentOS 7.8.2003
also Ubuntu 20.04
.NET Core 3.1.301, commit: 7feb845744
.Protocols version 5.0.0-preview.5.20278.1

Regression?

On Windows it works correctly.

Other information

Only stack trace I got:
System.DirectoryServices.Protocols.LdapException: The LDAP server returned an unknown error. at System.DirectoryServices.Protocols.LdapConnection.SendRequest(DirectoryRequest request, TimeSpan requestTimeout) at System.DirectoryServices.Protocols.LdapConnection.SendRequest(DirectoryRequest request)

@GrowSharp
Copy link
Author

Some more info.
I debugged trough the whole process of sending search request trough System.DirectoryServices.Protocols and didn't encounter error. I also figured out that the underlying library that System.DirectoryServices.Protocols uses on linux is open ldap. My current suspicion is that the open ldap doesn't automatically escape special characters in accepted strings (search request also as authentication method (#38611) and so on), but underlying windows library (Wldap32) does.

If that is the case I propose to add the escaping functionality to System.DirectoryServices.Protocols.LdapConnection, so both libraries act the same on the windows as well as on linux.

As I said this is just a suspicion. I haven't been able to find a correct solution to ldap string escaping, to actually prove this theory. I will get to it again probably next week, but if anyone want's to try, or if anyone knows that this is a wrong theory, please let me know.

@ericstj ericstj added this to the 5.0.0 milestone Jul 9, 2020
@ericstj ericstj removed the untriaged New issue has not been triaged by the area owner label Jul 9, 2020
@ericstj ericstj added bug os-linux Linux OS (any supported distro) labels Jul 9, 2020
@danmoseley
Copy link
Member

@GrowSharp thanks for the investigation. Did you manage to get further -- did it work if you added escaping? That might help establish whether there is anything we can safely do for 5.0 here (which is almost done)

@danmoseley
Copy link
Member

Perhaps the same fix for #38611?

@GrowSharp
Copy link
Author

@danmosemsft yes, I tried replacing special characters with their hexadecimal representation. Didn't worked for me tho. And yes, I agree that fixing this might also fix #38611.

@joperezr
Copy link
Member

@GrowSharp can you share the filter you are using in the snippet above? Also is there a chance you can share the full CN you are trying to search here so that we can try reproing exactly the problem you are seeing?

@GrowSharp
Copy link
Author

@joperezr sure.
Original CN = CN=Portal\#23\#Práva supervisora

Filter with my custom escaping experiment that doesn't work:
(&(objectClass=group)(distinguishedName=CN=Portal\5c#23\5c#Práva supervisora,OU=Portal Logistika,OU=Skupiny,OU=Users,OU=ADLER,DC=adler,DC=local))

Without my escaping:
(&(objectClass=group)(distinguishedName=CN=Portal\#23\#Práva supervisora,OU=Portal Logistika,OU=Skupiny,OU=Users,OU=ADLER,DC=adler,DC=local))

@GrowSharp
Copy link
Author

I would just like to add for all these issues that the AD that I'm working with is a bloody mess. But it should still work tho, because on windows it does.

@joperezr
Copy link
Member

I looked a little bit more on this, and it seems like this is a limitation on openldap. Seems like it encodes the distinguishedName property whenever it has special characters, which is why you probably can't find it when looking for an object via distinguishedName. In order to investigate this, I tried playing with ldapsearch tool since it uses openldap library underneath as well so it is a good exercise to compare against that to find out potential issues with S.DS.P. When searching for all of the groups on my ldap server, I get results like:

# Domain Users, Users, joesads.com
dn: CN=Domain Users,CN=Users,DC=joesads,DC=com
objectClass: top
objectClass: group
cn: Domain Users
description: All domain users
distinguishedName: CN=Domain Users,CN=Users,DC=joesads,DC=com
instanceType: 4
whenCreated: 20200512193423.0Z
whenChanged: 20200512193423.0Z
uSNCreated: 12348
memberOf: CN=Users,CN=Builtin,DC=joesads,DC=com
uSNChanged: 12350
name: Domain Users
objectGUID:: GMnpKzHc2E2f0m2RHcPWmA==
objectSid:: AQUAAAAAAAUVAAAABKFYKz4jVTZmyCfZAQIAAA==
sAMAccountName: Domain Users
sAMAccountType: 268435456
groupType: -2147483646
objectCategory: CN=Group,CN=Schema,CN=Configuration,DC=joesads,DC=com
isCriticalSystemObject: TRUE
dSCorePropagationData: 20200512193424.0Z
dSCorePropagationData: 16010101000001.0Z

As you can see above, distinguishedName is correctly the full DN of the group, but when I try with a group that has special characters like yours, I get:

# Portal\5C#23\5C#Pr\C3\A1va supervisora, Users, joesads.com
dn:: Q049UG9ydGFsXFxcIzIzXFxcI1Byw6F2YSBzdXBlcnZpc29yYSxDTj1Vc2VycyxEQz1qb2VzY
 WRzLERDPWNvbQ==
objectClass: top
objectClass: group
cn:: UG9ydGFsXCMyM1wjUHLDoXZhIHN1cGVydmlzb3Jh
distinguishedName:: Q049UG9ydGFsXFxcIzIzXFxcI1Byw6F2YSBzdXBlcnZpc29yYSxDTj1Vc2
 VycyxEQz1qb2VzYWRzLERDPWNvbQ==
instanceType: 4
whenCreated: 20200818192523.0Z
whenChanged: 20200818192523.0Z
uSNCreated: 65600
uSNChanged: 65600
name:: UG9ydGFsXCMyM1wjUHLDoXZhIHN1cGVydmlzb3Jh
objectGUID:: C2bdnRxYe0eWewwz0Wbs6g==
objectSid:: AQUAAAAAAAUVAAAABKFYKz4jVTZmyCfZVQQAAA==
sAMAccountName: prava
sAMAccountType: 268435456
groupType: -2147483646
objectCategory: CN=Group,CN=Schema,CN=Configuration,DC=joesads,DC=com
dSCorePropagationData: 16010101000000.0Z
mail: prava

As you can see above, disntinguishedName property is encoded in this case which is likely why the search isn't really returning results. All of this points to a problem with the underneath library so far, but one thing apart from this is that we shouldn't be throwing an exception for filters like that, so I will do some more investigation on why are we throwing in this case.

@GrowSharp
Copy link
Author

Just to add a note. When I explicitly remove the \ chars from the search filter query, it works. It's probably not a solution for this issue in general, but it's sufficient enough for me atm. What's no-go for me is the #38610 and the most importantly #38611.

@joperezr
Copy link
Member

I see, well given you have a workaround for now and this is not blocking you I will push the fix/investigation for this to 6.0. Even if this is an underlying issue with the native library, I think we should either provide a workaround or do more research to see if there are other ways to add filters with special characters.

@joperezr joperezr modified the milestones: 5.0.0, 6.0.0 Aug 26, 2020
@GrowSharp
Copy link
Author

I completely agree.

@smbecker
Copy link

I am having a similar issue with running on Linux. I can successfully search using ldapsearch but when running in a .NET app, I am getting the following (ErrorCode=-12).

System.DirectoryServices.Protocols.LdapException: The LDAP server returned an unknown error.
   at System.DirectoryServices.Protocols.LdapConnection.SendRequest(DirectoryRequest request, TimeSpan requestTimeout)
   at System.DirectoryServices.Protocols.LdapConnection.SendRequest(DirectoryRequest request)

@danmoseley
Copy link
Member

@smbecker the error above is -7 LDAP_FILTER_ERROR
whereas you have -12 LDAP_NOT_SUPPORTED. (You see the same message due to issue #46021). Could you please open a new issue for your problem as it likely has some other cause?

@RoadTrain
Copy link

RoadTrain commented Aug 12, 2021

Another case with escaping is when filter argument (e.g. group CN) contains parentheses, e.g. Some (very) important group.
Since parentheses are a part of filter syntax itself, it might be problematic to correctly parse the filter.

(&(objectClass=group)(sAMAccountName=Some (very) important group))

The correct representation, as per RFC, is:

(&(objectClass=group)(sAMAccountName=Some \28very\29 important group))

What's interesting is that the first one works on Windows, but fails on Linux.
The second one works on both Windows and Linux, as per my tests.

If that is the case I propose to add the escaping functionality to System.DirectoryServices.Protocols.LdapConnection, so both libraries act the same on the windows as well as on linux.

I think this might benefit from some general escape method which we can call manually when constructing filters, like:

var filter = $"(&(objectClass=group)(sAMAccountName={LdapFilter.Escape("Some (very) important group")}))";

Something along the lines of what I did here: dotnet/aspnetcore@7f72499
Is it a good idea to provide such a method as part of core System.DirectoryServices.Protocols functionality?

@ghost ghost added the needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration label Aug 12, 2021
@joperezr
Copy link
Member

Thanks for the report @RoadTrain, and for the diagnosis of the right representation along with the examples.

Is it a good idea to provide such a method as part of core System.DirectoryServices.Protocols functionality?

I think this would be a great addition to System.DirectoryServices.Protocols. The search query language is part of the LDAP spec, so having an additional class which encodes the filter/query so that the query would be sent correctly would be very useful. That said, I would probably just focus on the encoding part of the equation, without necessarily validating that the query itself is valid as that would be a whole new can of worms, but at least ensuring the query is enconded correctly as per spec it would be great. Feel free to either use this issue or log a new one with the API proposal so that we can take a look at it.

@joperezr joperezr removed the needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration label Aug 12, 2021
@RoadTrain
Copy link

@joperezr I've submitted an API proposal: #59900

@ghost ghost added the needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration label Oct 2, 2021
@joperezr joperezr removed the needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration label Nov 3, 2021
@joperezr joperezr moved this from Untriaged to Future in Triage POD for Reflection, META, etc. Nov 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

No branches or pull requests

8 participants