LdapConnection causes an NullReferenceException when authenticating with Authentication type external and a client certificate

[Davidvanluijk]

Description

When calling the Bind method without credentials and with authentication type external a null reference is thrown.

In case an empty credential object is passed an exception is thrown (LDAP Authentication failed: System.DirectoryServices.Protocols.LdapException: A bad parameter was passed to a routine), so this is no useable alternative.

When we create a test application which does not use LdapConnection but calls the ldap_bind directly it does work with a null credential object.

Reproduction Steps

`using System.DirectoryServices.Protocols;
using System.Security.Cryptography.X509Certificates;

namespace X509Store;

public class SystemLdap
{
public static void Run(X509Certificate2 cert)
{
string ldapPath = "dc";
int LDAPPort = 636;
var username = "user";

    using LdapConnection ldapConnection = new LdapConnection(new LdapDirectoryIdentifier(ldapPath, LDAPPort));
    ldapConnection.AuthType = AuthType.External;
    LdapSessionOptions options = ldapConnection.SessionOptions;
    options.SecureSocketLayer = true;
    options.ProtocolVersion = 3;
    ldapConnection.ClientCertificates.Add(cert);
    ldapConnection.Bind();
    // Perform further actions after this
}

}
`

Expected behavior

Bind succesfully completes

Actual behavior

LDAP Authentication failed: System.NullReferenceException: Object reference not set to an instance of an object.
at Interop.Ldap.ldap_bind_s(ConnectionHandle ldapHandle, String dn, SEC_WINNT_AUTH_IDENTITY_EX& credentials, BindMethod method)
at System.DirectoryServices.Protocols.LdapConnection.InternalBind(NetworkCredential tempCredential, SEC_WINNT_AUTH_IDENTITY_EX cred, BindMethod method)
at System.DirectoryServices.Protocols.LdapConnection.BindHelper(NetworkCredential newCredential, Boolean needSetCredential)
at System.DirectoryServices.Protocols.LdapConnection.Bind()
at X509Store.SystemLdap.Run(X509Certificate2 smartCardCert) in SystemLdap.cs:line 22

Regression?

No response

Known Workarounds

None

Configuration

x64 Net. 8.0 on a Windows 11 system
Using System.DirectoryServices.AccountManagement version 9.0.2

Other information

No response

[dotnet-policy-service]

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

[ericstj]

It looks to me like this exception might be coming from the interop source generated code (cc @elinor-fung)

@Davidvanluijk can you please fill out the complete template indicating if this is a regression or if it never worked? Try the 6.0 package and also .NETFramework to see if they worked - both predate the use of the source generator.

[dotnet-policy-service]

This issue has been marked needs-author-action and may be missing some important information.

[ericstj]

I took a closer look at this today and was able to reproduce. It does seem to be a regression from this change ccd67b0#diff-25eb162cb88bf1dcd67cdf438cb303832bb8f16bbe5fa7266d9faabda8100922R41

@jkoritzinsky you changed this type from class to struct ccd67b0#diff-cd60d68ca0e274c1592c8edcce8c98cc4044670de44970b1affcad9616c377eaR33

But the marshaller doesn't account for null and it's passed in this case. I'm not too familiar with Unsafe.NullRef<T> should the marshaller be using Unsafe.IsNullRef<SEC_WINNT_AUTH_IDENTITY_EX>? And if so - what should it pass down for Native?

runtime/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/ldap/LdapConnection.Windows.cs

Lines 64 to 65 in aba4de8

	private int InternalBind(NetworkCredential tempCredential, SEC_WINNT_AUTH_IDENTITY_EX cred, BindMethod method)
	=> tempCredential == null && AuthType == AuthType.External ? Interop.Ldap.ldap_bind_s(_ldapHandle, null, Unsafe.NullRef<SEC_WINNT_AUTH_IDENTITY_EX>(), method) : Interop.Ldap.ldap_bind_s(_ldapHandle, null, cred, method);

runtime/src/libraries/Common/src/Interop/Windows/Wldap32/Interop.Ldap.cs

Lines 13 to 15 in aba4de8

	[LibraryImport(Libraries.Wldap32, EntryPoint = "ldap_bind_sW", StringMarshalling = StringMarshalling.Utf16)]
	[UnmanagedCallConv(CallConvs = [typeof(CallConvCdecl)])]
	public static partial int ldap_bind_s(ConnectionHandle ldapHandle, string dn, in SEC_WINNT_AUTH_IDENTITY_EX credentials, BindMethod method);

runtime/src/libraries/Common/src/Interop/Interop.Ldap.cs

Lines 34 to 81 in aba4de8

	#if NET
	[NativeMarshalling(typeof(Marshaller))]
	#endif
	[StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)]
	internal struct SEC_WINNT_AUTH_IDENTITY_EX
	{
	public int version;
	public int length;
	public string user;
	public int userLength;
	public string domain;
	public int domainLength;
	public string password;
	public int passwordLength;
	public int flags;
	public string packageList;
	public int packageListLength;
	#if NET
	[CustomMarshaller(typeof(SEC_WINNT_AUTH_IDENTITY_EX), MarshalMode.ManagedToUnmanagedIn, typeof(Marshaller))]
	internal static class Marshaller
	{
	public static Native ConvertToUnmanaged(SEC_WINNT_AUTH_IDENTITY_EX managed)
	{
	Native n = default;
	n.version = managed.version;
	n.length = managed.length;
	n.user = Marshal.StringToCoTaskMemUni(managed.user);
	n.userLength = managed.userLength;
	n.domain = Marshal.StringToCoTaskMemUni(managed.domain);
	n.domainLength = managed.domainLength;
	n.password = Marshal.StringToCoTaskMemUni(managed.password);
	n.passwordLength = managed.passwordLength;
	n.flags = managed.flags;
	n.packageList = Marshal.StringToCoTaskMemUni(managed.packageList);
	n.packageListLength = managed.packageListLength;
	return n;
	}
	public static void Free(Native native)
	{
	Marshal.FreeCoTaskMem(native.user);
	Marshal.FreeCoTaskMem(native.domain);
	Marshal.FreeCoTaskMem(native.password);
	Marshal.FreeCoTaskMem(native.packageList);
	}
	}
	#endif

[jkoritzinsky]

Yeah it looks like I didn't really account for the case that the struct was non-blittable and that we don't support null refs in that case. This type should be changed back to a class and the marshaller logic should change to handle allocating and freeing the Native struct as part of marshalling (and passing a null pointer for the null case)

[ericstj]

handle allocating and freeing the Native struct as part of marshalling

@jkoritzinsky Can you provide an example of that? I understand what to change with SEC_WINNT_AUTH_IDENTITY_EX but not the Native type. My naive guess resulted in

error SYSLIB1056: The return type of 'System.DirectoryServices.Protocols.SEC_WINNT_AUTH_IDENTITY_EX.Marshaller.ConvertToUnmanaged(System.DirectoryServices.Protocols.SEC_WINNT_AUTH_IDENTITY_EX)' must be unmanaged 

[ericstj]

I came up with what might be a simpler approach - just an overload of the PInvoke that will accept an IntPtr expected to be null. I'll give this a try and see if it works.

@Davidvanluijk -- you can try 6.0.2 and see if it works as a temporary workaround.

You can also try this private build with the proposed fix. System.DirectoryServices.Protocols.10.0.0-dev.nupkg.zip

[jkoritzinsky]

The simpler approach looks reasonable. We can just go with that.

[Davidvanluijk]

I can confirm this error does not occur with 6.0.2, haven't tested with the private build.

As I'm not sure how this process works: Will this fix be available for .net 8 or only for .net 10? It will be released when .net 10 is released?

[Davidvanluijk]

First of all: thank you for the fast response on this ticket.

After more testing with 6.0.2 I do have a follow up question/bug: the above code throws an exception "The authentication method is not supported". The LdapConnection code seems to be calling "ldap_bind_sw", but the winapi docs (https://learn.microsoft.com/en-us/windows/win32/api/winldap/nf-winldap-ldap_bind_sw) don't mention external as supported method. Is it correct to assume this doesn't work because of this reason? Could the alternative ldap_sasl_bind_s be more approriate in this case?

[steveharter]

@Davidvanluijk if possible can you verify your test against .NET Framework -- that code looks to be essentially the same as v6 which uses "ldap_bind_sw".

[ericstj]

I did see examples similar to this in other codebases, so I assume it should work. It's also been this way for ages - same in .NETFramework. I get the same result from this library after the fix, and from .NETFramework. I debugged winldap to see what's happening, the client library is not the source of the error - it successfully sends the LDAP request to the server. I see the server responds with a result of 7 authMethodNotSupported - so this would indicate to me that in my case the server did not allow for this.

Now I'm not sure if that's because the client formed the request wrong, or if indeed my AD server doesn't allow this type of auth, but at least it's consistent. We're not really the LDAP experts here so I'm not sure if we can get a better answer. @BRDPM @grubioe @jay98014 own this on Windows and may be able to help answer.

I did find these docs -- https://learn.microsoft.com/en-us/entra/identity/domain-services/tutorial-configure-ldaps -- which imply server side settings might be required.

[Davidvanluijk]

For anyone stumbling across this issue and wondering how to get it to work, I posted the final working example on stackoverflow:
https://stackoverflow.com/questions/79492708/dotnet-ldapconnection-with-authentication-method-external-and-client-certificate/79514211#79514211