dohertj2/scadaproj
1
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(auth): prove the AD domain of a u:DOMAIN\name user bind

Browse Source 
Auth-007's sAMAccountName match assumed the search base covers one AD
domain; across a forest (Global Catalog) it could accept another domain's
same-named account and return its groups. For a u:DOMAIN\name Who-am-I
answer the entry must now also be that domain's account: AD's
msDS-PrincipalName (objectSid-derived DOMAIN\name, read by a base search
through the new ILdapConnection.Read) must equal the answer where offered;
otherwise the name must have exactly one holder under SearchBase and it must
be the entry found. Three forest cases fail pre-fix; single-domain and
sAMAccountName-search compat cases pass both ways. README notes this and
Auth-009.

(scadaproj#50)

Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
This commit is contained in:
Joseph Doherty
2026-11-09 15:18:46 -05:00
parent 9ec0da1b2c
commit b8d3fa2ba1
6 changed files with 281 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ZB.MOM.WW.Auth/src/ZB.MOM.WW.Auth.Ldap/Internal/ILdapConnection.cs
+8
View File
@@ -46,6 +46,14 @@ internal interface ILdapConnection : IDisposable
/// <summary>Executes a subtree search and returns all matching entries.</summary>
IReadOnlyList<LdapSearchEntry> Search(string searchBase, string filter, IReadOnlyList<string> attributes);
/// <summary>
/// Reads one entry by DN (a base-scope search) and returns the requested attributes the server
/// sent, or <see langword="null"/> when the entry does not exist. Throws <c>LdapException</c> on
/// any other failure. Used for constructed attributes such as AD's <c>msDS-PrincipalName</c>,
/// which a server may compute for a base read but not for a subtree search.
/// </summary>
LdapSearchEntry? Read(string dn, IReadOnlyList<string> attributes);
/// <summary>
/// RFC 4532 "Who am I?": the authorization identity the directory associates with the current
/// bind, as the server sends it (<c>dn:&lt;dn&gt;</c>, <c>u:DOMAIN\name</c>, or empty for an
ZB.MOM.WW.Auth/src/ZB.MOM.WW.Auth.Ldap/Internal/NovellLdapConnection.cs
+21
View File
@@ -102,6 +102,27 @@ internal sealed class NovellLdapConnection : ILdapConnection
return ReadEntries(results);
}
/// <inheritdoc/>
public LdapSearchEntry? Read(string dn, IReadOnlyList<string> attributes)
{
var results = _conn.Search(
dn,
LdapConnection.ScopeBase,
"(objectClass=*)",
attributes.ToArray(),
typesOnly: false);
try
{
var entries = ReadEntries(results);
return entries.Count == 0 ? null : entries[0];
}
catch (LdapException ex) when (ex.ResultCode == LdapException.NoSuchObject)
{
return null;
}
}
/// <inheritdoc/>
public string WhoAmI()
{
ZB.MOM.WW.Auth/src/ZB.MOM.WW.Auth.Ldap/LdapAuthService.cs
+109 -10
View File
@@ -369,7 +369,7 @@ public sealed class LdapAuthService : ILdapAuthService
if (entries.Count != 1)
return LdapAuthResult.Fail(LdapAuthFailure.GroupLookupFailed);
if (!IsBoundAccount(entries[0], bound))
if (!IsBoundAccount(conn, entries[0], bound, username))
{
_logger?.LogWarning(
"LDAP user bind authenticated {BoundIdentity} but the entry found for the typed name is " +
@@ -384,13 +384,18 @@ public sealed class LdapAuthService : ILdapAuthService
/// <summary>The AD attribute that an RFC 4532 <c>u:DOMAIN\name</c> authorization identity names.</summary>
private const string SamAccountNameAttribute = "sAMAccountName";
/// <summary>AD's constructed attribute holding a principal's <c>DOMAIN\name</c>, derived from its <c>objectSid</c>.</summary>
private const string PrincipalNameAttribute = "msDS-PrincipalName";
/// <summary>
/// The account a user bind authenticated: either its DN, or (AD's Who-am-I form) its
/// <c>sAMAccountName</c>. Exactly one member is set.
/// <c>sAMAccountName</c> plus, when the answer carried one, its NetBIOS <see cref="Domain"/>.
/// Exactly one of <see cref="Dn"/> / <see cref="AccountName"/> is set.
/// </summary>
private sealed record BoundIdentity(string? Dn, string? AccountName)
private sealed record BoundIdentity(string? Dn, string? AccountName, string? Domain = null)
{
public override string ToString() => Dn is not null ? $"dn:{Dn}" : $"u:{AccountName}";
public override string ToString() =>
Dn is not null ? $"dn:{Dn}" : Domain is null ? $"u:{AccountName}" : $@"u:{Domain}\{AccountName}";
}
/// <summary>A template is a DN template when it contains an attribute assertion (<c>=</c>); UPN and down-level forms never do.</summary>
@@ -424,11 +429,15 @@ public sealed class LdapAuthService : ILdapAuthService
if (authzId.StartsWith("u:", StringComparison.OrdinalIgnoreCase) && authzId.Length > 2)
{
var account = authzId[2..].Trim();
string? domain = null;
var slash = account.LastIndexOf('\\');
if (slash >= 0)
{
domain = account[..slash].Trim();
account = account[(slash + 1)..];
}
if (account.Length > 0)
return new BoundIdentity(null, account);
return new BoundIdentity(null, account, string.IsNullOrEmpty(domain) ? null : domain);
}
return dnTemplate ? new BoundIdentity(bindName, null) : null;
@@ -437,9 +446,11 @@ public sealed class LdapAuthService : ILdapAuthService
/// <summary>
/// <see langword="true"/> only when <paramref name="entry"/> is the account that bound: its DN
/// equals the bound DN (RFC 4514 <c>distinguishedNameMatch</c>, case- and space-insensitive), or
/// its <c>sAMAccountName</c> equals the Who-am-I account name (case-insensitive, as AD compares it).
/// — for AD's <c>u:DOMAIN\name</c> answer — it is provably that domain's account of that name
/// (see <see cref="IsBoundAdAccount"/>).
/// </summary>
private static bool IsBoundAccount(LdapSearchEntry entry, BoundIdentity bound)
private bool IsBoundAccount(
ZB.MOM.WW.Auth.Ldap.Internal.ILdapConnection conn, LdapSearchEntry entry, BoundIdentity bound, string username)
{
if (bound.Dn is not null)
{
@@ -454,9 +465,97 @@ public sealed class LdapAuthService : ILdapAuthService
}
}
return entry.Attributes.TryGetValue(SamAccountNameAttribute, out var values)
&& values.Count == 1
&& string.Equals(values[0], bound.AccountName, StringComparison.OrdinalIgnoreCase);
return IsBoundAdAccount(conn, entry, bound, username);
}
/// <summary>
/// The <c>u:DOMAIN\name</c> case. A matching <c>sAMAccountName</c> is necessary but not enough:
/// <c>sAMAccountName</c> is unique per DOMAIN, so a search base that spans a forest (a Global
/// Catalog) can find another domain's account of the same name and hand back its groups
/// (scadaproj#50). In order of strength:
/// <list type="number">
/// <item>Where the directory offers AD's constructed <c>msDS-PrincipalName</c> for the entry —
/// its <c>DOMAIN\name</c>, computed from the entry's <c>objectSid</c> — it must equal the
/// Who-am-I answer, domain included. That is the decisive, SID-anchored check. (The bound
/// account's own <c>objectSid</c> cannot be compared directly: Who-am-I does not return it.)</item>
/// <item>Otherwise the name must have exactly ONE holder under <see cref="LdapOptions.SearchBase"/>,
/// and that holder's DN must be the entry's: two holders mean which one bound cannot be proven,
/// so the sign-in fails closed. The user search already proves this when it was itself
/// <c>(sAMAccountName=&lt;the bound name&gt;)</c> and found one entry, so no second search is
/// made then. Residual: a search base that covers another domain's holder but NOT the bound
/// account's own domain is not detectable without <c>msDS-PrincipalName</c>.</item>
/// </list>
/// </summary>
private bool IsBoundAdAccount(
ZB.MOM.WW.Auth.Ldap.Internal.ILdapConnection conn, LdapSearchEntry entry, BoundIdentity bound, string username)
{
if (!entry.Attributes.TryGetValue(SamAccountNameAttribute, out var values)
|| values.Count != 1
|| !string.Equals(values[0], bound.AccountName, StringComparison.OrdinalIgnoreCase))
{
return false;
}
if (bound.Domain is not null && ReadPrincipalName(conn, entry.Dn) is { } principal)
return string.Equals(principal, $@"{bound.Domain}\{bound.AccountName}", StringComparison.OrdinalIgnoreCase);
var searchedOnTheBoundName =
string.Equals(_options.UserNameAttribute, SamAccountNameAttribute, StringComparison.OrdinalIgnoreCase)
&& string.Equals(username, bound.AccountName, StringComparison.OrdinalIgnoreCase);
if (searchedOnTheBoundName)
return true;
IReadOnlyList<LdapSearchEntry> holders;
try
{
holders = conn.Search(
_options.SearchBase,
$"({SamAccountNameAttribute}={LdapEscaping.Filter(bound.AccountName!)})",
new[] { SamAccountNameAttribute });
}
catch (LdapException)
{
// Same bucket as the user search: the directory could not answer, which is system-side.
throw new StageFailure(LdapAuthFailure.ServiceAccountBindFailed, "search");
}
if (holders.Count != 1)
return false;
try
{
return LdapDn.Equals(holders[0].Dn, entry.Dn);
}
catch (Exception)
{
return false;
}
}
/// <summary>
/// The entry's <c>msDS-PrincipalName</c> when the directory computes one in <c>DOMAIN\name</c>
/// form; <see langword="null"/> when it is absent, unreadable, or a SID string (AD's form for a
/// principal it cannot name) — all of which mean "not available", never "mismatch".
/// </summary>
private static string? ReadPrincipalName(ZB.MOM.WW.Auth.Ldap.Internal.ILdapConnection conn, string dn)
{
try
{
var read = conn.Read(dn, new[] { PrincipalNameAttribute });
if (read is not null
&& read.Attributes.TryGetValue(PrincipalNameAttribute, out var values)
&& values.Count == 1
&& values[0].Contains('\\', StringComparison.Ordinal))
{
return values[0].Trim();
}
}
catch (LdapException)
{
// Not offered, or not readable by this account: fall back to the uniqueness check.
}
return null;
}
/// <summary>Step 7: search for the user entry; a search fault is system-side.</summary>