From 08bf8a459f784968832cd6e000e4195b35d210e5 Mon Sep 17 00:00:00 2001 From: John Luo Date: Mon, 7 Sep 2020 18:34:23 -0700 Subject: [PATCH 1/5] Add cache for retrieved RBAC claims --- .../Negotiate/src/Internal/LdapAdapter.cs | 47 +++++++++++++++---- .../Negotiate/src/LdapSettings.cs | 14 ++++++ 2 files changed, 51 insertions(+), 10 deletions(-) diff --git a/src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs b/src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs index 4ddad3c5e31b..082a87a423c6 100644 --- a/src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs +++ b/src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs @@ -1,11 +1,13 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System.Collections.Generic; using System.DirectoryServices.Protocols; using System.Linq; using System.Security.Claims; using System.Text; using System.Threading.Tasks; +using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.Logging; namespace Microsoft.AspNetCore.Authentication.Negotiate @@ -16,7 +18,24 @@ public static async Task RetrieveClaimsAsync(LdapSettings settings, ClaimsIdenti { var user = identity.Name; var userAccountName = user.Substring(0, user.IndexOf('@')); + + if (settings.ClaimsCache == null) + { + settings.ClaimsCache = new MemoryCache(new MemoryCacheOptions { SizeLimit = settings.ClaimsCacheSize }); + } + + if (settings.ClaimsCache.TryGetValue>(user, out var cachedClaims)) + { + foreach (var claim in cachedClaims) + { + identity.AddClaim(claim); + } + + return; + } + var distinguishedName = settings.Domain.Split('.').Select(name => $"dc={name}").Aggregate((a, b) => $"{a},{b}"); + var retrievedClaims = new List(); var filter = $"(&(objectClass=user)(sAMAccountName={userAccountName}))"; // This is using ldap search query language, it is looking on the server for someUser var searchRequest = new SearchRequest(distinguishedName, filter, SearchScope.Subtree, null); @@ -45,13 +64,24 @@ public static async Task RetrieveClaimsAsync(LdapSettings settings, ClaimsIdenti if (!settings.IgnoreNestedGroups) { - GetNestedGroups(settings.LdapConnection, identity, distinguishedName, groupCN, logger); + GetNestedGroups(settings.LdapConnection, identity, distinguishedName, groupCN, logger, retrievedClaims); } else { - AddRole(identity, groupCN); + retrievedClaims.Add(CreateClaim(identity, groupCN)); } } + + foreach (var claim in retrievedClaims) + { + identity.AddClaim(claim); + } + + settings.ClaimsCache.Set(user, + retrievedClaims, + new MemoryCacheEntryOptions() + .SetSize(1) + .SetSlidingExpiration(settings.ClaimsCacheEntryExpiration)); } else { @@ -59,10 +89,10 @@ public static async Task RetrieveClaimsAsync(LdapSettings settings, ClaimsIdenti } } - private static void GetNestedGroups(LdapConnection connection, ClaimsIdentity principal, string distinguishedName, string groupCN, ILogger logger) + private static void GetNestedGroups(LdapConnection connection, ClaimsIdentity principal, string distinguishedName, string groupCN, ILogger logger, IList retrievedClaims) { var filter = $"(&(objectClass=group)(sAMAccountName={groupCN}))"; // This is using ldap search query language, it is looking on the server for someUser - var searchRequest = new SearchRequest(distinguishedName, filter, System.DirectoryServices.Protocols.SearchScope.Subtree, null); + var searchRequest = new SearchRequest(distinguishedName, filter, SearchScope.Subtree, null); var searchResponse = (SearchResponse)connection.SendRequest(searchRequest); if (searchResponse.Entries.Count > 0) @@ -74,7 +104,7 @@ private static void GetNestedGroups(LdapConnection connection, ClaimsIdentity pr var group = searchResponse.Entries[0]; //Get the object that was found on ldap string name = group.DistinguishedName; - AddRole(principal, name); + retrievedClaims.Add(CreateClaim(principal, name)); var memberof = group.Attributes["memberof"]; // You can access ldap Attributes with Attributes property if (memberof != null) @@ -83,15 +113,12 @@ private static void GetNestedGroups(LdapConnection connection, ClaimsIdentity pr { var groupDN = $"{Encoding.UTF8.GetString((byte[])member)}"; var nestedGroupCN = groupDN.Split(',')[0].Substring("CN=".Length); - GetNestedGroups(connection, principal, distinguishedName, nestedGroupCN, logger); + GetNestedGroups(connection, principal, distinguishedName, nestedGroupCN, logger, retrievedClaims); } } } } - private static void AddRole(ClaimsIdentity identity, string role) - { - identity.AddClaim(new Claim(identity.RoleClaimType, role)); - } + private static Claim CreateClaim(ClaimsIdentity identity, string role) => new Claim(identity.RoleClaimType, role); } } diff --git a/src/Security/Authentication/Negotiate/src/LdapSettings.cs b/src/Security/Authentication/Negotiate/src/LdapSettings.cs index 1e26c26c14d4..ef7193455cbc 100644 --- a/src/Security/Authentication/Negotiate/src/LdapSettings.cs +++ b/src/Security/Authentication/Negotiate/src/LdapSettings.cs @@ -3,6 +3,7 @@ using System; using System.DirectoryServices.Protocols; +using Microsoft.Extensions.Caching.Memory; namespace Microsoft.AspNetCore.Authentication.Negotiate { @@ -56,6 +57,19 @@ public class LdapSettings /// public LdapConnection LdapConnection { get; set; } + /// + /// The expiration that should be used for entries in the cache for user claims, defaults to 10 minutes. + /// This is a sliding expiration that will extend each time claims for a user is retrieved. + /// + public TimeSpan ClaimsCacheEntryExpiration { get; set; } = TimeSpan.FromMinutes(10); + + /// + /// How many user claim results to store in the cache, defaults to 1024. + /// + public int ClaimsCacheSize { get; set; } = 1024; + + internal MemoryCache ClaimsCache { get; set; } + public void Validate() { if (EnableLdapClaimResolution) From 70959c3918aa80d931178223d62e05145aedb12d Mon Sep 17 00:00:00 2001 From: John Luo Date: Wed, 9 Sep 2020 01:19:58 -0700 Subject: [PATCH 2/5] Feedback --- .../Negotiate/src/Internal/LdapAdapter.cs | 23 ++++----- .../Negotiate/src/LdapSettings.cs | 4 +- ...tCore.Authentication.Negotiate.Test.csproj | 1 + .../Negotiate.Test/NegotiateHandlerTests.cs | 48 ++++++++++++++++++- 4 files changed, 62 insertions(+), 14 deletions(-) diff --git a/src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs b/src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs index 082a87a423c6..42b9b38f210f 100644 --- a/src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs +++ b/src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs @@ -17,25 +17,26 @@ internal static class LdapAdapter public static async Task RetrieveClaimsAsync(LdapSettings settings, ClaimsIdentity identity, ILogger logger) { var user = identity.Name; - var userAccountName = user.Substring(0, user.IndexOf('@')); + var userAccountNameIndex = user.IndexOf('@'); + var userAccountName = userAccountNameIndex == -1 ? user : user.Substring(0, userAccountNameIndex); if (settings.ClaimsCache == null) { settings.ClaimsCache = new MemoryCache(new MemoryCacheOptions { SizeLimit = settings.ClaimsCacheSize }); } - if (settings.ClaimsCache.TryGetValue>(user, out var cachedClaims)) + if (settings.ClaimsCache.TryGetValue>(user, out var cachedClaims)) { foreach (var claim in cachedClaims) { - identity.AddClaim(claim); + identity.AddClaim(new Claim(identity.RoleClaimType, claim)); } return; } var distinguishedName = settings.Domain.Split('.').Select(name => $"dc={name}").Aggregate((a, b) => $"{a},{b}"); - var retrievedClaims = new List(); + var retrievedClaims = new List(); var filter = $"(&(objectClass=user)(sAMAccountName={userAccountName}))"; // This is using ldap search query language, it is looking on the server for someUser var searchRequest = new SearchRequest(distinguishedName, filter, SearchScope.Subtree, null); @@ -68,19 +69,21 @@ public static async Task RetrieveClaimsAsync(LdapSettings settings, ClaimsIdenti } else { - retrievedClaims.Add(CreateClaim(identity, groupCN)); + retrievedClaims.Add(groupCN); } } + var entrySize = Encoding.Default.GetByteCount(user); foreach (var claim in retrievedClaims) { - identity.AddClaim(claim); + identity.AddClaim(new Claim(identity.RoleClaimType, claim)); + entrySize += Encoding.Default.GetByteCount(claim); } settings.ClaimsCache.Set(user, retrievedClaims, new MemoryCacheEntryOptions() - .SetSize(1) + .SetSize(entrySize) .SetSlidingExpiration(settings.ClaimsCacheEntryExpiration)); } else @@ -89,7 +92,7 @@ public static async Task RetrieveClaimsAsync(LdapSettings settings, ClaimsIdenti } } - private static void GetNestedGroups(LdapConnection connection, ClaimsIdentity principal, string distinguishedName, string groupCN, ILogger logger, IList retrievedClaims) + private static void GetNestedGroups(LdapConnection connection, ClaimsIdentity principal, string distinguishedName, string groupCN, ILogger logger, IList retrievedClaims) { var filter = $"(&(objectClass=group)(sAMAccountName={groupCN}))"; // This is using ldap search query language, it is looking on the server for someUser var searchRequest = new SearchRequest(distinguishedName, filter, SearchScope.Subtree, null); @@ -104,7 +107,7 @@ private static void GetNestedGroups(LdapConnection connection, ClaimsIdentity pr var group = searchResponse.Entries[0]; //Get the object that was found on ldap string name = group.DistinguishedName; - retrievedClaims.Add(CreateClaim(principal, name)); + retrievedClaims.Add(name); var memberof = group.Attributes["memberof"]; // You can access ldap Attributes with Attributes property if (memberof != null) @@ -118,7 +121,5 @@ private static void GetNestedGroups(LdapConnection connection, ClaimsIdentity pr } } } - - private static Claim CreateClaim(ClaimsIdentity identity, string role) => new Claim(identity.RoleClaimType, role); } } diff --git a/src/Security/Authentication/Negotiate/src/LdapSettings.cs b/src/Security/Authentication/Negotiate/src/LdapSettings.cs index ef7193455cbc..e6689ee34066 100644 --- a/src/Security/Authentication/Negotiate/src/LdapSettings.cs +++ b/src/Security/Authentication/Negotiate/src/LdapSettings.cs @@ -64,9 +64,9 @@ public class LdapSettings public TimeSpan ClaimsCacheEntryExpiration { get; set; } = TimeSpan.FromMinutes(10); /// - /// How many user claim results to store in the cache, defaults to 1024. + /// The maximum size of the claim results cache, defaults to 100 MB. /// - public int ClaimsCacheSize { get; set; } = 1024; + public int ClaimsCacheSize { get; set; } = 100 * 1024 * 1024; internal MemoryCache ClaimsCache { get; set; } diff --git a/src/Security/Authentication/Negotiate/test/Negotiate.Test/Microsoft.AspNetCore.Authentication.Negotiate.Test.csproj b/src/Security/Authentication/Negotiate/test/Negotiate.Test/Microsoft.AspNetCore.Authentication.Negotiate.Test.csproj index 40c623bd117a..b14d633eabbf 100644 --- a/src/Security/Authentication/Negotiate/test/Negotiate.Test/Microsoft.AspNetCore.Authentication.Negotiate.Test.csproj +++ b/src/Security/Authentication/Negotiate/test/Negotiate.Test/Microsoft.AspNetCore.Authentication.Negotiate.Test.csproj @@ -10,6 +10,7 @@ + diff --git a/src/Security/Authentication/Negotiate/test/Negotiate.Test/NegotiateHandlerTests.cs b/src/Security/Authentication/Negotiate/test/Negotiate.Test/NegotiateHandlerTests.cs index 0b0b7b14cd2d..0d84e1f5d4a4 100644 --- a/src/Security/Authentication/Negotiate/test/Negotiate.Test/NegotiateHandlerTests.cs +++ b/src/Security/Authentication/Negotiate/test/Negotiate.Test/NegotiateHandlerTests.cs @@ -13,7 +13,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.TestHost; -using Microsoft.AspNetCore.Testing; +using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; using Microsoft.Net.Http.Headers; @@ -208,6 +208,28 @@ public async Task AuthHeaderAfterNtlmCompleted_ReAuthenticates(bool persist) await NtlmStage1And2Auth(server, testConnection); } + [Fact] + public async Task RBACClaimsRetrievedFromCacheAfterKerberosCompleted() + { + var claimsCache = new MemoryCache(new MemoryCacheOptions()); + claimsCache.Set("name", new string[] { "CN=Domain Admins,CN=Users,DC=domain,DC=net" }); + NegotiateOptions negotiateOptions = null; + using var host = await CreateHostAsync(options => + { + options.EnableLdap(ldapSettings => + { + ldapSettings.Domain = "domain.NET"; + ldapSettings.ClaimsCache = claimsCache; + ldapSettings.EnableLdapClaimResolution = false; // This disables binding to the LDAP connection on startup + }); + negotiateOptions = options; + }); + var server = host.GetTestServer(); + var testConnection = new TestConnection(); + negotiateOptions.EnableLdap(_ => { }); // Forcefully re-enable ldap claims resolution to trigger RBAC claims retrieval from cache + await AuthenticateAndRetrieveRBACClaims(server, testConnection); + } + [Theory] [InlineData(false)] [InlineData(true)] @@ -304,6 +326,12 @@ public async Task OtherError_Throws() var ex = await Assert.ThrowsAsync(() => SendAsync(server, "/404", testConnection, "Negotiate OtherError")); Assert.Equal("A test other error occurred", ex.Message); } + private static async Task AuthenticateAndRetrieveRBACClaims(TestServer server, TestConnection testConnection) + { + var result = await SendAsync(server, "/AuthenticateAndRetrieveRBACClaims", testConnection, "Negotiate ClientKerberosBlob"); + Assert.Equal(StatusCodes.Status200OK, result.Response.StatusCode); + Assert.Equal("Negotiate ServerKerberosBlob", result.Response.Headers[HeaderNames.WWWAuthenticate]); + } // Single Stage private static async Task KerberosAuth(TestServer server, TestConnection testConnection) @@ -408,6 +436,24 @@ private static void ConfigureEndpoints(IEndpointRouteBuilder builder) await context.Response.WriteAsync(name); }); + builder.Map("/AuthenticateAndRetrieveRBACClaims", async context => + { + if (!context.User.Identity.IsAuthenticated) + { + await context.ChallengeAsync(); + return; + } + + Assert.Equal("HTTP/1.1", context.Request.Protocol); // Not HTTP/2 + var name = context.User.Identity.Name; + Assert.False(string.IsNullOrEmpty(name), "name"); + Assert.Contains( + context.User.Claims, + claim => claim.Type == "http://schemas.microsoft.com/ws/2008/06/identity/claims/role" + && claim.Value == "CN=Domain Admins,CN=Users,DC=domain,DC=net"); + await context.Response.WriteAsync(name); + }); + builder.Map("/AlreadyAuthenticated", async context => { Assert.Equal("HTTP/1.1", context.Request.Protocol); // Not HTTP/2 From 0218e9c8a0a59491b599e71a2f6eab33ec8f9fa2 Mon Sep 17 00:00:00 2001 From: John Luo Date: Wed, 9 Sep 2020 10:36:40 -0700 Subject: [PATCH 3/5] Feedback --- .../Negotiate/src/Internal/LdapAdapter.cs | 7 ++++--- .../Authentication/Negotiate/src/LdapSettings.cs | 10 ++++++++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs b/src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs index 42b9b38f210f..586b9b638b5c 100644 --- a/src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs +++ b/src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs @@ -73,18 +73,19 @@ public static async Task RetrieveClaimsAsync(LdapSettings settings, ClaimsIdenti } } - var entrySize = Encoding.Default.GetByteCount(user); + var entrySize = user.Length * 2; //Approximate the size of stored key in memory cache. foreach (var claim in retrievedClaims) { identity.AddClaim(new Claim(identity.RoleClaimType, claim)); - entrySize += Encoding.Default.GetByteCount(claim); + entrySize += claim.Length * 2; //Approximate the size of stored value in memory cache. } settings.ClaimsCache.Set(user, retrievedClaims, new MemoryCacheEntryOptions() .SetSize(entrySize) - .SetSlidingExpiration(settings.ClaimsCacheEntryExpiration)); + .SetSlidingExpiration(settings.ClaimsCacheEntrySlidingExpiration) + .SetAbsoluteExpiration(settings.ClaimsCacheEntryAbsoluteExpiration)); } else { diff --git a/src/Security/Authentication/Negotiate/src/LdapSettings.cs b/src/Security/Authentication/Negotiate/src/LdapSettings.cs index e6689ee34066..2adfb5652bd7 100644 --- a/src/Security/Authentication/Negotiate/src/LdapSettings.cs +++ b/src/Security/Authentication/Negotiate/src/LdapSettings.cs @@ -58,10 +58,16 @@ public class LdapSettings public LdapConnection LdapConnection { get; set; } /// - /// The expiration that should be used for entries in the cache for user claims, defaults to 10 minutes. + /// The sliding expiration that should be used for entries in the cache for user claims, defaults to 10 minutes. /// This is a sliding expiration that will extend each time claims for a user is retrieved. /// - public TimeSpan ClaimsCacheEntryExpiration { get; set; } = TimeSpan.FromMinutes(10); + public TimeSpan ClaimsCacheEntrySlidingExpiration { get; set; } = TimeSpan.FromMinutes(10); + + /// + /// The absolute expiration that should be used for entries in the cache for user claims, defaults to 60 minutes. + /// This is an absolute expiration that starts when a claims for a user is retrieved for the first time. + /// + public TimeSpan ClaimsCacheEntryAbsoluteExpiration { get; set; } = TimeSpan.FromMinutes(60); /// /// The maximum size of the claim results cache, defaults to 100 MB. From 61b9aa3b9b446323d3ccbf3c15ebe203a8b142a5 Mon Sep 17 00:00:00 2001 From: John Luo Date: Wed, 9 Sep 2020 10:54:49 -0700 Subject: [PATCH 4/5] Update src/Security/Authentication/Negotiate/src/LdapSettings.cs Co-authored-by: Chris Ross --- src/Security/Authentication/Negotiate/src/LdapSettings.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Security/Authentication/Negotiate/src/LdapSettings.cs b/src/Security/Authentication/Negotiate/src/LdapSettings.cs index 2adfb5652bd7..cdefe6f676e1 100644 --- a/src/Security/Authentication/Negotiate/src/LdapSettings.cs +++ b/src/Security/Authentication/Negotiate/src/LdapSettings.cs @@ -61,13 +61,13 @@ public class LdapSettings /// The sliding expiration that should be used for entries in the cache for user claims, defaults to 10 minutes. /// This is a sliding expiration that will extend each time claims for a user is retrieved. /// - public TimeSpan ClaimsCacheEntrySlidingExpiration { get; set; } = TimeSpan.FromMinutes(10); + public TimeSpan ClaimsCacheSlidingExpiration { get; set; } = TimeSpan.FromMinutes(10); /// /// The absolute expiration that should be used for entries in the cache for user claims, defaults to 60 minutes. /// This is an absolute expiration that starts when a claims for a user is retrieved for the first time. /// - public TimeSpan ClaimsCacheEntryAbsoluteExpiration { get; set; } = TimeSpan.FromMinutes(60); + public TimeSpan ClaimsCacheAbsoluteExpiration { get; set; } = TimeSpan.FromMinutes(60); /// /// The maximum size of the claim results cache, defaults to 100 MB. From 3cc45023c85120d8887b6ccef43a8d4e6602b485 Mon Sep 17 00:00:00 2001 From: John Luo Date: Wed, 9 Sep 2020 11:21:37 -0700 Subject: [PATCH 5/5] Need to react to rename --- .../Authentication/Negotiate/src/Internal/LdapAdapter.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs b/src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs index 586b9b638b5c..ca47c0282280 100644 --- a/src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs +++ b/src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs @@ -84,8 +84,8 @@ public static async Task RetrieveClaimsAsync(LdapSettings settings, ClaimsIdenti retrievedClaims, new MemoryCacheEntryOptions() .SetSize(entrySize) - .SetSlidingExpiration(settings.ClaimsCacheEntrySlidingExpiration) - .SetAbsoluteExpiration(settings.ClaimsCacheEntryAbsoluteExpiration)); + .SetSlidingExpiration(settings.ClaimsCacheSlidingExpiration) + .SetAbsoluteExpiration(settings.ClaimsCacheAbsoluteExpiration)); } else {