-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Add support for LDAPTLS_CACERTDIR \ TrustedCertificateDirectory #111877
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
Changes from all commits
510c98b
4a4427e
51070ed
5c64144
1940a2b
7a11a43
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -382,6 +382,8 @@ public partial class LdapSessionOptions | |||||||
internal LdapSessionOptions() { } | ||||||||
public bool AutoReconnect { get { throw null; } set { } } | ||||||||
public string DomainName { get { throw null; } set { } } | ||||||||
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("windows")] | ||||||||
public string TrustedCertificatesDirectory { get { throw null; } set { } } | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like we expose TrustedCertificatesDirectory in the We may want to ifdef this API didn't catch this because we don't compare netstandard2.0 to netframework if a package doesn't have an assembly for it. cc @ViktorHofer maybe we should add a comparison tuple when we see the empty folder? |
||||||||
public string HostName { get { throw null; } set { } } | ||||||||
public bool HostReachable { get { throw null; } } | ||||||||
public System.DirectoryServices.Protocols.LocatorFlags LocatorFlag { get { throw null; } set { } } | ||||||||
|
@@ -402,6 +404,8 @@ 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("windows")] | ||||||||
public void StartNewTlsSessionContext() { } | ||||||||
public bool TcpKeepAlive { get { throw null; } set { } } | ||||||||
public System.DirectoryServices.Protocols.VerifyServerCertificateCallback VerifyServerCertificate { get { throw null; } set { } } | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if it changed but, we were talking to make
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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() { } | ||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,8 @@ | |
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
using System.ComponentModel; | ||
using System.IO; | ||
using System.Runtime.Versioning; | ||
|
||
namespace System.DirectoryServices.Protocols | ||
{ | ||
|
@@ -11,6 +13,34 @@ 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 certificate file be a symbolic link to the actual certificate file | ||
/// which can be done by using <code>openssl rehash .</code> or <code>c_rehash .</code> in the directory | ||
/// containing the certificate files. | ||
/// </remarks> | ||
/// <exception cref="DirectoryNotFoundException">The directory not exist.</exception> | ||
[UnsupportedOSPlatform("windows")] | ||
public string TrustedCertificatesDirectory | ||
{ | ||
get => GetStringValueHelper(LdapOption.LDAP_OPT_X_TLS_CACERTDIR, releasePtr: true); | ||
|
||
set | ||
{ | ||
if (!Directory.Exists(value)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we know if a relative path is OK to pass to LDAP? Will it resolve in the same way that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes it seems to work locally. All other path-based APIs support relative paths AFAIK and both |
||
{ | ||
throw new DirectoryNotFoundException(SR.Format(SR.DirectoryNotFound, value)); | ||
} | ||
|
||
SetStringOptionHelper(LdapOption.LDAP_OPT_X_TLS_CACERTDIR, value); | ||
} | ||
} | ||
|
||
public bool SecureSocketLayer | ||
{ | ||
get | ||
|
@@ -52,6 +82,16 @@ 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("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); | ||
|
@@ -71,5 +111,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); | ||
ericstj marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
int error = LdapPal.SetStringOption(_connection._ldapHandle, option, value); | ||
|
||
ErrorChecking.CheckAndSetLdapError(error); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,12 +2,10 @@ | |
// 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.IO; | ||
using System.Net; | ||
using System.Text; | ||
using System.Threading; | ||
using Xunit; | ||
|
||
namespace System.DirectoryServices.Protocols.Tests | ||
|
@@ -16,6 +14,7 @@ public partial class DirectoryServicesProtocolsTests | |
{ | ||
internal static bool LdapConfigurationExists => LdapConfiguration.Configuration != null; | ||
internal static bool IsActiveDirectoryServer => LdapConfigurationExists && LdapConfiguration.Configuration.IsActiveDirectoryServer; | ||
internal static bool UseTls => LdapConfigurationExists && LdapConfiguration.Configuration.UseTls; | ||
|
||
internal static bool IsServerSideSortSupported => LdapConfigurationExists && LdapConfiguration.Configuration.SupportsServerSideSort; | ||
|
||
|
@@ -706,6 +705,64 @@ public void TestMultipleServerBind() | |
connection.Timeout = new TimeSpan(0, 3, 0); | ||
} | ||
|
||
#if NET | ||
[ConditionalFact(nameof(UseTls))] | ||
[PlatformSpecific(TestPlatforms.Linux)] | ||
public void StartNewTlsSessionContext() | ||
{ | ||
using (var connection = GetConnection(bind: false)) | ||
{ | ||
// We use "." as the directory since it must be a valid directory for StartNewTlsSessionContext() + Bind() to be successful even | ||
// though there are no client certificates in ".". | ||
connection.SessionOptions.TrustedCertificatesDirectory = "."; | ||
|
||
// For a real-world scenario, we would call 'StartTransportLayerSecurity(null)' here which would do the TLS handshake including | ||
// providing the client certificate to the server and validating the server certificate. However, this requires additional | ||
// setup that we don't have including trusting the server certificate and by specifying "demand" in the setup of the server | ||
// via 'LDAP_TLS_VERIFY_CLIENT=demand' to force the TLS handshake to occur. | ||
|
||
connection.SessionOptions.StartNewTlsSessionContext(); | ||
connection.Bind(); | ||
|
||
SearchRequest searchRequest = new (LdapConfiguration.Configuration.SearchDn, "(objectClass=*)", SearchScope.Subtree); | ||
_ = (SearchResponse)connection.SendRequest(searchRequest); | ||
} | ||
} | ||
|
||
[ConditionalFact(nameof(UseTls))] | ||
[PlatformSpecific(TestPlatforms.Linux)] | ||
public void StartNewTlsSessionContext_ThrowsLdapException() | ||
{ | ||
using (var connection = GetConnection(bind: false)) | ||
{ | ||
// Create a new session context without setting TrustedCertificatesDirectory. | ||
connection.SessionOptions.StartNewTlsSessionContext(); | ||
Assert.Throws<PlatformNotSupportedException>(() => connection.Bind()); | ||
} | ||
} | ||
|
||
[ConditionalFact(nameof(LdapConfigurationExists))] | ||
[PlatformSpecific(TestPlatforms.Linux)] | ||
public void TrustedCertificatesDirectory_ThrowsDirectoryNotFoundException() | ||
{ | ||
using (var connection = GetConnection(bind: false)) | ||
{ | ||
Assert.Throws<DirectoryNotFoundException>(() => connection.SessionOptions.TrustedCertificatesDirectory = "nonexistent"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens if the directory is deleted after our check then user calls There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't care. We would throw a vague exception like "unable to connect to server" based on the underlying LDAP error code. |
||
} | ||
} | ||
|
||
[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; | ||
|
@@ -786,24 +843,24 @@ 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(bool bind = true) | ||
{ | ||
LdapDirectoryIdentifier directoryIdentifier = string.IsNullOrEmpty(LdapConfiguration.Configuration.Port) ? | ||
new LdapDirectoryIdentifier(LdapConfiguration.Configuration.ServerName, fullyQualifiedDnsHostName: true, connectionless: false) : | ||
new LdapDirectoryIdentifier(LdapConfiguration.Configuration.ServerName, | ||
int.Parse(LdapConfiguration.Configuration.Port, NumberStyles.None, CultureInfo.InvariantCulture), | ||
fullyQualifiedDnsHostName: true, connectionless: false); | ||
return GetConnection(directoryIdentifier); | ||
return GetConnection(directoryIdentifier, bind); | ||
} | ||
|
||
private static LdapConnection GetConnection(LdapDirectoryIdentifier directoryIdentifier) | ||
private static LdapConnection GetConnection(LdapDirectoryIdentifier directoryIdentifier, bool bind = true) | ||
{ | ||
NetworkCredential credential = new NetworkCredential(LdapConfiguration.Configuration.UserName, LdapConfiguration.Configuration.Password); | ||
|
||
|
@@ -816,7 +873,11 @@ private static LdapConnection GetConnection(LdapDirectoryIdentifier directoryIde | |
// to LDAP v2, which we do not support, and will return LDAP_PROTOCOL_ERROR | ||
connection.SessionOptions.ProtocolVersion = 3; | ||
connection.SessionOptions.SecureSocketLayer = LdapConfiguration.Configuration.UseTls; | ||
connection.Bind(); | ||
|
||
if (bind) | ||
{ | ||
connection.Bind(); | ||
} | ||
|
||
connection.Timeout = new TimeSpan(0, 3, 0); | ||
return connection; | ||
|
Uh oh!
There was an error while loading. Please reload this page.