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

Fixing regression for AuthType.Anonymous which leads to a NullReferenceException be thrown. #62807

Merged
merged 2 commits into from
Dec 14, 2021

Conversation

joperezr
Copy link
Member

Fixes #61683 regression.

When we added support for LDAPS #52904 we accidentally regressed the scenario for AuthType.Anonymous since you would have passed in a null password, and we would later try to dereference it. This change fixes that by setting the value length of the struct to 0, and I have validated that this does work for the anonymous scenario.

@ghost
Copy link

ghost commented Dec 14, 2021

Tagging subscribers to this area: @dotnet/area-system-directoryservices, @jay98014
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #61683 regression.

When we added support for LDAPS #52904 we accidentally regressed the scenario for AuthType.Anonymous since you would have passed in a null password, and we would later try to dereference it. This change fixes that by setting the value length of the struct to 0, and I have validated that this does work for the anonymous scenario.

Author: joperezr
Assignees: -
Labels:

area-System.DirectoryServices

Milestone: -

@ericstj
Copy link
Member

ericstj commented Dec 14, 2021

Is it possible to add a regression test for this scenario?

@joperezr
Copy link
Member Author

I can add one that will validate the NullRef is not thrown, but we can't really adda test that actually ensures that allowAnonymous succeeds to bind, which is kind of why this regression wasn't catched. The problem is that in order for AllowAnonymous to work, we would need to have our tests run against an ActiveDirectory Server for which we are domain joined, which we don't currently have the infrastructure for.

@ericstj
Copy link
Member

ericstj commented Dec 14, 2021

I can see it being valuable to get coverage of these APIs up to the point that they hit the network. I know a lot of our other APIs (like IO for example) have tests that ensure a certain precedence of exceptions. If you think it's possible to do and adds value here that might be something to consider.

@joperezr joperezr merged commit 2e8d09b into dotnet:main Dec 14, 2021
@joperezr joperezr deleted the FixNullRef branch December 14, 2021 23:25
@joperezr
Copy link
Member Author

/backport to release/6.0

@github-actions
Copy link
Contributor

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1580574117

@ghost ghost locked as resolved and limited conversation to collaborators Jan 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BindToDirectory throws NullReferenceException on Linux when LdapConnection AuthType is Anonymous
3 participants