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

Add support for LDAPTLS_CACERTDIR \ TrustedCertificateDirectory #111877

Merged
merged 6 commits into from
Feb 5, 2025
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/libraries/Common/src/Interop/Interop.Ldap.cs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,8 @@ internal enum LdapOption
LDAP_OPT_ROOTDSE_CACHE = 0x9a, // Not Supported in Linux
LDAP_OPT_DEBUG_LEVEL = 0x5001,
LDAP_OPT_URI = 0x5006, // Not Supported in Windows
LDAP_OPT_X_TLS_CACERTDIR = 0x6003, // Not Supported in Windows
LDAP_OPT_X_TLS_NEWCTX = 0x600F, // Not Supported in Windows
LDAP_OPT_X_SASL_REALM = 0x6101,
LDAP_OPT_X_SASL_AUTHCID = 0x6102,
LDAP_OPT_X_SASL_AUTHZID = 0x6103
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ To ship, we should test on both an Active Directory LDAP server, and at least on

When testing with later of versions of LDAP, the ldapsearch commands below may need to use

-H ldap://localhost:<PORT>
-H ldap://localhost:YOURPORT

instead of

-h localhost -p <PORT>
-h localhost -p YOURPORT

OPENDJ SERVER
=============
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,12 @@ public partial class LdapSessionOptions
internal LdapSessionOptions() { }
public bool AutoReconnect { get { throw null; } set { } }
public string DomainName { get { throw null; } set { } }
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("android")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are redundant, the assembly has <UnsupportedOSPlatforms>browser;android;ios;tvos</UnsupportedOSPlatforms> attribute
https://github.com/dotnet/runtime/blob/f552567173b706812d11579f8059a14a748dab81/src/libraries/System.DirectoryServices.Protocols/Directory.Build.props#L6C1-L6C78

Suggested change
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("android")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")]

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Buyaa! Removed those.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed from source but not from ref

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again - done.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still seeing these, @steveharter. I think we can omit the attributes already defined at the assembly level, and just add the one additional windows one here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, for some reason was looking at subset of commits.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming the previously excluded browser;android;ios;tvos were because the browser and mobile platforms don't support LDAP \ DirectoryServices.

[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("windows")]
public string TrustedCertificatesDirectory { get { throw null; } set { } }
public string HostName { get { throw null; } set { } }
public bool HostReachable { get { throw null; } }
public System.DirectoryServices.Protocols.LocatorFlags LocatorFlag { get { throw null; } set { } }
Expand All @@ -402,6 +408,12 @@ internal LdapSessionOptions() { }
public bool Signing { get { throw null; } set { } }
public System.DirectoryServices.Protocols.SecurityPackageContextConnectionInformation SslInformation { get { throw null; } }
public int SspiFlag { get { throw null; } set { } }
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("android")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("windows")]
public void StartNewTlsSessionContext() { }
public bool TcpKeepAlive { get { throw null; } set { } }
public System.DirectoryServices.Protocols.VerifyServerCertificateCallback VerifyServerCertificate { get { throw null; } set { } }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if it changed but, we were talking to make VerifyServerCertificate windows only with this change

Suggested change
public System.DirectoryServices.Protocols.VerifyServerCertificateCallback VerifyServerCertificate { get { throw null; } set { } }
[System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")]
public System.DirectoryServices.Protocols.VerifyServerCertificateCallback VerifyServerCertificate { get { throw null; } set { } }

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Buyaa! I'll put up another PR for that (per this PR's description I elected not to do any of that, but VerifyServerCertificateCallback has caused a lot of pain so I will do that one-off).

public void FastConcurrentBind() { }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -955,13 +955,13 @@ private unsafe Interop.BOOL ProcessClientCertificate(IntPtr ldapHandle, IntPtr C

private void Connect()
{
//Ccurrently ldap does not accept more than one certificate.
// Currently ldap does not accept more than one certificate.
if (ClientCertificates.Count > 1)
{
throw new InvalidOperationException(SR.InvalidClientCertificates);
}

// Set the certificate callback routine here if user adds the certifcate to the certificate collection.
// Set the certificate callback routine here if user adds the certificate to the certificate collection.
if (ClientCertificates.Count != 0)
{
int certError = LdapPal.SetClientCertOption(_ldapHandle, LdapOption.LDAP_OPT_CLIENT_CERTIFICATE, _clientCertificateRoutine);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.ComponentModel;
using System.Runtime.Versioning;

namespace System.DirectoryServices.Protocols
{
Expand All @@ -11,6 +12,26 @@ public partial class LdapSessionOptions

private bool _secureSocketLayer;

/// <summary>
/// Specifies the path of the directory containing CA certificates in the PEM format.
/// Multiple directories may be specified by separating with a semi-colon.
/// </summary>
/// <remarks>
/// The certificate files are looked up by the CA subject name hash value where that hash can be
/// obtained by using, for example, <code>openssl x509 -hash -noout -in CA.crt</code>.
/// It is a common practice to have the file be a symbolic link to the actual certificate file.
/// </remarks>
[UnsupportedOSPlatform("android")]
[UnsupportedOSPlatform("browser")]
[UnsupportedOSPlatform("ios")]
[UnsupportedOSPlatform("tvos")]
[UnsupportedOSPlatform("windows")]
public string TrustedCertificatesDirectory
{
get => GetStringValueHelper(LdapOption.LDAP_OPT_X_TLS_CACERTDIR, releasePtr: true);
set => SetStringOptionHelper(LdapOption.LDAP_OPT_X_TLS_CACERTDIR, value);
}

public bool SecureSocketLayer
{
get
Expand Down Expand Up @@ -52,6 +73,20 @@ public ReferralChasingOptions ReferralChasing
}
}

/// <summary>
/// Create a new TLS library context.
/// Calling this is necessary after setting TLS-based options, such as <c>TrustedCertificatesDirectory</c>.
/// </summary>
[UnsupportedOSPlatform("android")]
[UnsupportedOSPlatform("browser")]
[UnsupportedOSPlatform("ios")]
[UnsupportedOSPlatform("tvos")]
[UnsupportedOSPlatform("windows")]
public void StartNewTlsSessionContext()
{
SetIntValueHelper(LdapOption.LDAP_OPT_X_TLS_NEWCTX, 0);
}

private bool GetBoolValueHelper(LdapOption option)
{
if (_connection._disposed) throw new ObjectDisposedException(GetType().Name);
Expand All @@ -71,5 +106,14 @@ private void SetBoolValueHelper(LdapOption option, bool value)

ErrorChecking.CheckAndSetLdapError(error);
}

private void SetStringOptionHelper(LdapOption option, string value)
{
if (_connection._disposed) throw new ObjectDisposedException(GetType().Name);

int error = LdapPal.SetStringOption(_connection._ldapHandle, option, value);

ErrorChecking.CheckAndSetLdapError(error);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,17 @@ public partial class LdapSessionOptions
{
private static void PALCertFreeCRLContext(IntPtr certPtr) => Interop.Ldap.CertFreeCRLContext(certPtr);

[UnsupportedOSPlatform("android")]
[UnsupportedOSPlatform("browser")]
[UnsupportedOSPlatform("ios")]
[UnsupportedOSPlatform("tvos")]
[UnsupportedOSPlatform("windows")]
public string TrustedCertificatesDirectory
{
get => throw new PlatformNotSupportedException();
set => throw new PlatformNotSupportedException();
}

public bool SecureSocketLayer
{
get
Expand All @@ -24,6 +35,13 @@ public bool SecureSocketLayer
}
}

[UnsupportedOSPlatform("android")]
[UnsupportedOSPlatform("browser")]
[UnsupportedOSPlatform("ios")]
[UnsupportedOSPlatform("tvos")]
[UnsupportedOSPlatform("windows")]
public void StartNewTlsSessionContext() => throw new PlatformNotSupportedException();

public int ProtocolVersion
{
get => GetIntValueHelper(LdapOption.LDAP_OPT_VERSION);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,9 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.Diagnostics;
using System.DirectoryServices.Tests;
using System.Globalization;
using System.Net;
using System.Text;
using System.Threading;
using Xunit;

namespace System.DirectoryServices.Protocols.Tests
Expand Down Expand Up @@ -706,6 +703,32 @@ public void TestMultipleServerBind()
connection.Timeout = new TimeSpan(0, 3, 0);
}

#if NET
[ConditionalFact(nameof(LdapConfigurationExists))]
[PlatformSpecific(TestPlatforms.Linux)]
public void StartNewTlsSessionContext()
{
using (var connection = new LdapConnection("server"))
{
LdapSessionOptions options = connection.SessionOptions;

// A complete test would be to use TrustedCertificatesDirectory along with other valid options to connect.
options.StartNewTlsSessionContext();
}
}

[ConditionalFact(nameof(LdapConfigurationExists))]
[PlatformSpecific(TestPlatforms.Windows)]
public void StartNewTlsSessionContext_ThrowsPlatformNotSupportedException()
{
using (var connection = new LdapConnection("server"))
{
LdapSessionOptions options = connection.SessionOptions;
Assert.Throws<PlatformNotSupportedException>(() => options.StartNewTlsSessionContext());
}
}
#endif

private void DeleteAttribute(LdapConnection connection, string entryDn, string attributeName)
{
string dn = entryDn + "," + LdapConfiguration.Configuration.SearchDn;
Expand Down Expand Up @@ -786,14 +809,14 @@ private SearchResultEntry SearchUser(LdapConnection connection, string rootDn, s
return null;
}

private LdapConnection GetConnection(string server)
private static LdapConnection GetConnection(string server)
{
LdapDirectoryIdentifier directoryIdentifier = new LdapDirectoryIdentifier(server, fullyQualifiedDnsHostName: true, connectionless: false);

return GetConnection(directoryIdentifier);
}

private LdapConnection GetConnection()
private static LdapConnection GetConnection()
{
LdapDirectoryIdentifier directoryIdentifier = string.IsNullOrEmpty(LdapConfiguration.Configuration.Port) ?
new LdapDirectoryIdentifier(LdapConfiguration.Configuration.ServerName, fullyQualifiedDnsHostName: true, connectionless: false) :
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,18 @@ public static bool IsLibLdapInstalled
}
else
{
_isLibLdapInstalled = NativeLibrary.TryLoad("libldap-2.4.so.2", out _);
_isLibLdapInstalled =
NativeLibrary.TryLoad("libldap.so.2", out _) ||
NativeLibrary.TryLoad("libldap-2.6.so.0", out _) ||
NativeLibrary.TryLoad("libldap-2.5.so.0", out _) ||
NativeLibrary.TryLoad("libldap-2.4.so.2", out _);
}
}
return _isLibLdapInstalled.Value;
#else
_isLibLdapInstalled = true; // In .NET Framework ldap is always installed.
return _isLibLdapInstalled.Value;
#endif

return _isLibLdapInstalled.Value;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

namespace System.DirectoryServices.Protocols.Tests
{
// To enable these tests locally for Mono, comment out this line in DirectoryServicesTestHelpers.cs:
// [assembly: ActiveIssue("https://github.com/dotnet/runtime/issues/35912", TestRuntimes.Mono)]
[ConditionalClass(typeof(DirectoryServicesTestHelpers), nameof(DirectoryServicesTestHelpers.IsWindowsOrLibLdapIsInstalled))]
public class LdapSessionOptionsTests
{
Expand Down Expand Up @@ -756,5 +758,32 @@ public void StopTransportLayerSecurity_Disposed_ThrowsObjectDisposedException()

Assert.Throws<ObjectDisposedException>(() => connection.SessionOptions.StopTransportLayerSecurity());
}

#if NET
[Fact]
[PlatformSpecific(TestPlatforms.Linux)]
public void CertificateDirectoryProperty()
{
using (var connection = new LdapConnection("server"))
{
LdapSessionOptions options = connection.SessionOptions;
Assert.Null(options.TrustedCertificatesDirectory);

options.TrustedCertificatesDirectory = "CertificateDirectory";
Assert.Equal("CertificateDirectory", options.TrustedCertificatesDirectory);
}
}

[Fact]
[PlatformSpecific(TestPlatforms.Windows)]
public void CertificateDirectoryProperty_ThrowsPlatformNotSupportedException()
{
using (var connection = new LdapConnection("server"))
{
LdapSessionOptions options = connection.SessionOptions;
Assert.Throws<PlatformNotSupportedException>(() => options.TrustedCertificatesDirectory = "CertificateDirectory");
}
}
#endif
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2789,4 +2789,4 @@
<Compile Include="$(MSBuildThisFileDirectory)System\Threading\Wasi\WasiPollWorld.wit.imports.wasi.io.v0_2_0.IPoll.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Threading\Wasi\WasiPollWorld.wit.imports.wasi.io.v0_2_0.PollInterop.cs" />
</ItemGroup>
</Project>
</Project>
Loading