Add support for TLS and connectionless LDAP connections on Linux #52904
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |
|
|
||
| using System.Diagnostics; | ||
| using System.Net; | ||
| using System.Text; | ||
| using System.Runtime.InteropServices; | ||
|
|
||
| namespace System.DirectoryServices.Protocols | ||
|
|
@@ -12,13 +13,67 @@ public partial class LdapConnection | |
| // Linux doesn't support setting FQDN so we mark the flag as if it is already set so we don't make a call to set it again. | ||
| private bool _setFQDNDone = true; | ||
|
|
||
| private void InternalInitConnectionHandle(string hostname) | ||
| { | ||
| if ((LdapDirectoryIdentifier)_directoryIdentifier == null) | ||
| { | ||
| throw new NullReferenceException(); | ||
| } | ||
|
|
||
| _ldapHandle = new ConnectionHandle(); | ||
| } | ||
|
|
||
| private int InternalConnectToServer() | ||
| { | ||
| // In Linux you don't have to call Connect after calling init. You | ||
| // directly call bind. However, we set the URI for the connection | ||
| // here instead of during initialization because we need access to | ||
| // the SessionOptions property to properly define it, which is not | ||
| // available during init. | ||
| Debug.Assert(!_ldapHandle.IsInvalid); | ||
|
|
||
| string scheme = null; | ||
| LdapDirectoryIdentifier directoryIdentifier = (LdapDirectoryIdentifier)_directoryIdentifier; | ||
| if (directoryIdentifier.Connectionless) | ||
| { | ||
| scheme = "cldap://"; | ||
|
There was a problem hiding this comment. do we have any test for connectionless? There was a problem hiding this comment. No, I wasn't sure how to test it. I think that since connectionless LDAP isn't standardized, it's only implemented in AD (or as far as I can tell, it's not implemented in slapd). I've only got access to one [production] instance of AD, so I didn't want to test there! :) I imagine that this is currently broken in Linux since there are no provisions anywhere else to configure connectionless. I think that this patch should work for connectionless should work though. There was a problem hiding this comment. Normally we don't add features we can't test 🙂. But if the feature is simply "given this flag, use this scheme prefix" maybe it's reasonable to add if testing isn't straightforward. @joperezr preferences? There was a problem hiding this comment. I agree that having a test would be ideally, but given the context here I think that if we can't come up with a good way to test this it may be reasonable. |
||
| } | ||
| else if (SessionOptions.SecureSocketLayer) | ||
| { | ||
| scheme = "ldaps://"; | ||
| } | ||
| else | ||
| { | ||
| scheme = "ldap://"; | ||
| } | ||
|
|
||
| string uris = null; | ||
| string[] servers = directoryIdentifier.Servers; | ||
| if (servers != null && servers.Length != 0) | ||
| { | ||
| StringBuilder temp = new StringBuilder(200); | ||
| for (int i = 0; i < servers.Length; i++) | ||
| { | ||
| if (i != 0) | ||
| { | ||
| temp.Append(' '); | ||
| } | ||
| temp.Append(scheme); | ||
| temp.Append(servers[i]); | ||
| temp.Append(':'); | ||
| temp.Append(directoryIdentifier.PortNumber); | ||
| } | ||
| if (temp.Length != 0) | ||
| { | ||
| uris = temp.ToString(); | ||
| } | ||
| } | ||
| else | ||
| { | ||
| uris = $"{scheme}:{directoryIdentifier.PortNumber}"; | ||
| } | ||
|
|
||
| return LdapPal.SetStringOption(_ldapHandle, LdapOption.LDAP_OPT_URI, uris); | ||
|
There was a problem hiding this comment. Is it possible for There was a problem hiding this comment. Yes, it is possible. As you said, OpenLDAP falls back to its default hostname (which can be configured in I think that's OK... though I don't know how the Windows side handles empty string hostnames. If someone wrote code on Linux, then tried to run on Windows, that could be an issue if Windows doesn't support default hostnames. |
||
| } | ||
|
|
||
| private int InternalBind(NetworkCredential tempCredential, SEC_WINNT_AUTH_IDENTITY_EX cred, BindMethod method) | ||
|
|
@@ -30,7 +85,7 @@ private int InternalBind(NetworkCredential tempCredential, SEC_WINNT_AUTH_IDENTI | |
| } | ||
| else | ||
| { | ||
| error = LdapPal.BindToDirectory(_ldapHandle, cred.user, cred.password); | ||
| } | ||
|
|
||
| return error; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,11 +9,12 @@ public partial class LdapSessionOptions | |
| { | ||
| private static void PALCertFreeCRLContext(IntPtr certPtr) { /* No op */ } | ||
|
|
||
danmoseley marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| public bool SecureSocketLayer { get; set; } | ||
|
|
||
| public int ProtocolVersion | ||
| { | ||
| get => GetPtrValueHelper(LdapOption.LDAP_OPT_VERSION).ToInt32(); | ||
|
There was a problem hiding this comment.
Could you say more? There was a problem hiding this comment. In my testing, I had a problem where the version was not being set properly when using an integer rather than an IntPtr, so it was defaulting to LDAPv2 on my machine and failing to connect. The API calls for an pointer to an int:
I think this might work now because the value is being cast to an integer pointer on the C side, but from what I understand (which is very little) I think that this can fail on systems with different pointer sizes? |
||
| set => SetPtrValueHelper(LdapOption.LDAP_OPT_VERSION, new IntPtr(value)); | ||
| } | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.