Security patches to address vulnerabilities discuss in #10

alexhiggins732 · Feb 11, 2024 · 5f7c2b9 · 5f7c2b9
1 parent f992cdd
commit 5f7c2b9
Show file tree

Hide file tree

Showing 26 changed files with 705 additions and 46 deletions.
diff --git a/Directory.Packages.props b/Directory.Packages.props
@@ -11,7 +11,7 @@
     <GrpcVersion>2.59.0</GrpcVersion>
     <SerilogVersion>8.0.0</SerilogVersion>
     <NoWarn>$(NoWarn);AD0001;ASP0003;ASP0004;ASP0005;ASP0007;ASP0020;ASP0021;ASP0022;ASP0024</NoWarn>
-		<AutoGenerateBindingRedirects>true</AutoGenerateBindingRedirects>
+    <AutoGenerateBindingRedirects>true</AutoGenerateBindingRedirects>
   </PropertyGroup>
   <ItemGroup>
     <PackageVersion Include="AspNet.Security.OAuth.GitHub" Version="$(AspnetVersion)" />
@@ -61,7 +61,6 @@
     <PackageVersion Include="Microsoft.AspNetCore.Mvc.Razor.RuntimeCompilation" Version="$(AspnetVersion)" />
     <PackageVersion Include="Microsoft.AspNetCore.Mvc.Testing" Version="$(AspnetVersion)" />
     <PackageVersion Include="Microsoft.AspNetCore.OpenApi" Version="$(AspnetVersion)" />
-
     <PackageVersion Include="Microsoft.AspNetCore.TestHost" Version="8.0.1" />
     <PackageVersion Include="Microsoft.Data.SqlClient" Version="5.1.5" />
     <PackageVersion Include="Microsoft.EntityFrameworkCore.Design" Version="8.0.1">
@@ -70,7 +69,6 @@
     </PackageVersion>
     <PackageVersion Include="Microsoft.EntityFrameworkCore.InMemory" Version="$(AspnetVersion)" />
     <PackageVersion Include="Microsoft.EntityFrameworkCore.Relational" Version="$(AspnetMinorVersion)" />
-
     <PackageVersion Include="Microsoft.EntityFrameworkCore.Sqlite" Version="$(AspnetMinorVersion)" />
     <PackageVersion Include="Microsoft.EntityFrameworkCore.SqlServer" Version="$(AspnetMinorVersion)" />
     <PackageVersion Include="Microsoft.EntityFrameworkCore.Tools" Version="$(AspnetMinorVersion)">
@@ -86,8 +84,8 @@
     <PackageVersion Include="Microsoft.Extensions.Configuration.FileExtensions" Version="$(AspnetVersion)" />
     <PackageVersion Include="Microsoft.Extensions.Configuration.Json" Version="$(AspnetVersion)" />
     <PackageVersion Include="Microsoft.Extensions.Configuration.UserSecrets" Version="$(AspnetVersion)" />
-    <PackageVersion Include="Microsoft.Extensions.DependencyInjection" Version="$(AspnetVersion)" />
-    <PackageVersion Include="Microsoft.Extensions.DependencyInjection.Abstractions" Version="$(AspnetVersion)" />
+    <PackageVersion Include="Microsoft.Extensions.DependencyInjection" Version="8.0.0" />
+    <PackageVersion Include="Microsoft.Extensions.DependencyInjection.Abstractions" Version="8.0.0" />
     <PackageVersion Include="Microsoft.Extensions.Diagnostics.HealthChecks" Version="$(AspnetVersion)" />
     <PackageVersion Include="Microsoft.Extensions.Diagnostics.HealthChecks.EntityFrameworkCore" Version="$(AspnetVersion)" />
     <PackageVersion Include="Microsoft.Extensions.Http" Version="$(AspnetVersion)" />
@@ -100,8 +98,6 @@
     <PackageVersion Include="Microsoft.Identity.Web" Version="1.22.1" />
     <PackageVersion Include="Microsoft.Identity.Web.UI" Version="1.16.0" />
     <PackageVersion Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" Version="7.3.1" />
-
-
     <PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.8.0" />
     <PackageVersion Include="Microsoft.OpenApi" Version="1.6.10" />
     <PackageVersion Include="Microsoft.VisualStudio.Azure.Containers.Tools.Targets" Version="1.19.6" />
@@ -150,7 +146,6 @@
     <PackageVersion Include="System.Configuration.ConfigurationManager" Version="$(AspnetVersion)" />
     <PackageVersion Include="System.IdentityModel.Tokens.Jwt" Version="7.3.1" />
     <PackageVersion Include="System.Reactive" Version="6.0.0" />
-
     <PackageVersion Include="System.Security.Principal.Windows" Version="5.0.0" />
     <PackageVersion Include="System.ServiceModel.Primitives" Version="$(AspnetVersion)" />
     <PackageVersion Include="WireMock.Net" Version="1.5.39" />
@@ -159,8 +154,7 @@
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
       <PrivateAssets>all</PrivateAssets>
     </PackageVersion>
-
     <PackageVersion Include="Microsoft.CodeAnalysis.NetAnalyzers" Version="$(AspnetVersion)" PrivateAssets="All" />
-		<PackageVersion Include="Microsoft.SourceLink.GitHub" Version="$(AspnetVersion)" PrivateAssets="All" />
+    <PackageVersion Include="Microsoft.SourceLink.GitHub" Version="$(AspnetVersion)" PrivateAssets="All" />
   </ItemGroup>
 </Project>
diff --git a/src/AspNetIdentity/host/Quickstart/Consent/ConsentController.cs b/src/AspNetIdentity/host/Quickstart/Consent/ConsentController.cs
@@ -25,7 +25,7 @@ copies or substantial portions of the Software.
 using IdentityServer8.Validation;
 using System.Collections.Generic;
 using System;
-
+using Microsoft.DependencyInjection.Extensions;
 namespace IdentityServerHost.Quickstart.UI
 {
     /// <summary>
@@ -181,7 +181,7 @@ private async Task<ConsentViewModel> BuildViewModelAsync(string returnUrl, Conse
             }
             else
             {
-                _logger.LogError("No consent request matching request: {0}", returnUrl);
+                _logger.LogError("No consent request matching request: {0}", Ioc.Sanitizer.Log.Sanitize(returnUrl));
             }
 
             return null;

diff --git a/src/EntityFramework.Storage/src/IdentityServer8.EntityFramework.Storage.csproj b/src/EntityFramework.Storage/src/IdentityServer8.EntityFramework.Storage.csproj
@@ -17,6 +17,7 @@
     </ItemGroup>
 
     <ItemGroup>
+        <ProjectReference Include="..\..\Security\IdentityServer8.Security\IdentityServer8.Security.csproj" />
         <ProjectReference Include="..\..\Storage\src\IdentityServer8.Storage.csproj" />
     </ItemGroup>
 

diff --git a/src/EntityFramework.Storage/src/Stores/ClientStore.cs b/src/EntityFramework.Storage/src/Stores/ClientStore.cs
@@ -22,7 +22,8 @@ copies or substantial portions of the Software.
 using IdentityServer8.Stores;
 using Microsoft.EntityFrameworkCore;
 using Microsoft.Extensions.Logging;
-
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.DependencyInjection.Extensions;
 namespace IdentityServer8.EntityFramework.Stores
 {
     /// <summary>
@@ -81,7 +82,7 @@ public virtual async Task<Client> FindClientByIdAsync(string clientId)
 
             var model = client.ToModel();
 
-            Logger.LogDebug("{clientId} found in database: {clientIdFound}", clientId, model != null);
+            Logger.LogDebug("{clientId} found in database: {clientIdFound}", Ioc.Sanitizer.Log.Sanitize(clientId), model != null);
 
             return model;
         }

diff --git a/src/EntityFramework/src/Services/CorsPolicyService.cs b/src/EntityFramework/src/Services/CorsPolicyService.cs
@@ -22,6 +22,7 @@ copies or substantial portions of the Software.
 using Microsoft.AspNetCore.Http;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.EntityFrameworkCore;
+using Microsoft.DependencyInjection.Extensions;
 
 namespace IdentityServer8.EntityFramework.Services
 {
@@ -64,7 +65,7 @@ public async Task<bool> IsOriginAllowedAsync(string origin)
 
             var isAllowed = await query.AnyAsync();
 
-            _logger.LogDebug("Origin {origin} is allowed: {originAllowed}", origin, isAllowed);
+            _logger.LogDebug("Origin {origin} is allowed: {originAllowed}", Ioc.Sanitizer.Log.Sanitize(origin), isAllowed);
 
             return isAllowed;
         }

diff --git a/src/IdentityServer8.sln b/src/IdentityServer8.sln
@@ -100,6 +100,14 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "docs", "docs", "{21B3EAAF-1
 		..\docs\CHANGELOG.md = ..\docs\CHANGELOG.md
 	EndProjectSection
 EndProject
+Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Security", "Security", "{9B9C47C4-560E-4F2E-8F53-97F9FDE46008}"
+EndProject
+Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Security", "Security", "{19B652D0-0AA1-415C-AA62-065EE8C77182}"
+EndProject
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "IdentityServer8.Security", "Security\IdentityServer8.Security\IdentityServer8.Security.csproj", "{284DF9E0-8007-4E84-949D-5B610D76B1CF}"
+EndProject
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "IdentityServer8.Sanitizer.Tests", "Security\test\IdentityServer8.Santizer.Tests\IdentityServer8.Sanitizer.Tests.csproj", "{00BDF864-B06A-4A48-B8B4-85C219B33CD1}"
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -169,6 +177,14 @@ Global
 		{E3685B06-F135-4318-B841-889C35479D5C}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{E3685B06-F135-4318-B841-889C35479D5C}.Release|Any CPU.ActiveCfg = Release|Any CPU
 		{E3685B06-F135-4318-B841-889C35479D5C}.Release|Any CPU.Build.0 = Release|Any CPU
+		{284DF9E0-8007-4E84-949D-5B610D76B1CF}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{284DF9E0-8007-4E84-949D-5B610D76B1CF}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{284DF9E0-8007-4E84-949D-5B610D76B1CF}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{284DF9E0-8007-4E84-949D-5B610D76B1CF}.Release|Any CPU.Build.0 = Release|Any CPU
+		{00BDF864-B06A-4A48-B8B4-85C219B33CD1}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{00BDF864-B06A-4A48-B8B4-85C219B33CD1}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{00BDF864-B06A-4A48-B8B4-85C219B33CD1}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{00BDF864-B06A-4A48-B8B4-85C219B33CD1}.Release|Any CPU.Build.0 = Release|Any CPU
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE
@@ -205,6 +221,10 @@ Global
 		{0BC5E76A-A280-49C8-8E96-A43FEA357A4B} = {A7171FEC-FEC1-4AF0-9625-E69D93F08A42}
 		{C5474352-0715-41C0-92C2-BABA1A4103A0} = {A7171FEC-FEC1-4AF0-9625-E69D93F08A42}
 		{21B3EAAF-1A7D-4D46-AB3F-843296EDBDC2} = {29DC1E06-3EC5-4F52-9EC1-0363BD571369}
+		{9B9C47C4-560E-4F2E-8F53-97F9FDE46008} = {5461C61B-B06E-46BA-B206-925B660BE727}
+		{19B652D0-0AA1-415C-AA62-065EE8C77182} = {45C22EDD-91B1-4AEF-8620-77F4E3E7C544}
+		{284DF9E0-8007-4E84-949D-5B610D76B1CF} = {9B9C47C4-560E-4F2E-8F53-97F9FDE46008}
+		{00BDF864-B06A-4A48-B8B4-85C219B33CD1} = {19B652D0-0AA1-415C-AA62-065EE8C77182}
 	EndGlobalSection
 	GlobalSection(ExtensibilityGlobals) = postSolution
 		SolutionGuid = {176723F7-4A9B-4F05-A9F3-3BA715E4BDC3}

diff --git a/src/IdentityServer8/src/Extensions/ICacheExtensions.cs b/src/IdentityServer8/src/Extensions/ICacheExtensions.cs
@@ -14,6 +14,7 @@ copies or substantial portions of the Software.
 */
 
 using IdentityServer8.Services;
+using Microsoft.DependencyInjection.Extensions;
 using Microsoft.Extensions.Logging;
 using System;
 using System.Threading.Tasks;
@@ -53,19 +54,19 @@ public static async Task<T> GetAsync<T>(this ICache<T> cache, string key, TimeSp
 
             if (item == null)
             {
-                logger.LogTrace("Cache miss for {cacheKey}", key);
+                logger.LogTrace("Cache miss for {cacheKey}", Ioc.Sanitizer.Log.Sanitize(key));
 
                 item = await get();
 
                 if (item != null)
                 {
-                    logger.LogTrace("Setting item in cache for {cacheKey}", key);
+                    logger.LogTrace("Setting item in cache for {cacheKey}", Ioc.Sanitizer.Log.Sanitize(key));
                     await cache.SetAsync(key, item, duration);
                 }
             }
             else
             {
-                logger.LogTrace("Cache hit for {cacheKey}", key);
+                logger.LogTrace("Cache hit for {cacheKey}", Ioc.Sanitizer.Log.Sanitize(key));
             }
 
             return item;

diff --git a/src/IdentityServer8/src/Hosting/CorsPolicyProvider.cs b/src/IdentityServer8/src/Hosting/CorsPolicyProvider.cs
@@ -23,6 +23,7 @@ copies or substantial portions of the Software.
 using IdentityServer8.Services;
 using Microsoft.Extensions.DependencyInjection;
 using IdentityServer8.Extensions;
+using Microsoft.DependencyInjection.Extensions;
 
 namespace IdentityServer8.Hosting
 {
@@ -56,7 +57,6 @@ public Task<CorsPolicy> GetPolicyAsync(HttpContext context, string policyName)
                 return _inner.GetPolicyAsync(context, policyName);
             }
         }
-
         private async Task<CorsPolicy> ProcessAsync(HttpContext context)
         {
             var origin = context.Request.GetCorsOrigin();
@@ -73,17 +73,17 @@ private async Task<CorsPolicy> ProcessAsync(HttpContext context)
 
                     if (await corsPolicyService.IsOriginAllowedAsync(origin))
                     {
-                        _logger.LogDebug("CorsPolicyService allowed origin: {origin}", origin);
+                        _logger.LogDebug("CorsPolicyService allowed origin: {origin}", Ioc.Sanitizer.Log.Sanitize(origin));
                         return Allow(origin);
                     }
                     else
                     {
-                        _logger.LogWarning("CorsPolicyService did not allow origin: {origin}", origin);
+                        _logger.LogWarning("CorsPolicyService did not allow origin: {origin}", Ioc.Sanitizer.Log.Sanitize(origin));
                     }
                 }
                 else
                 {
-                    _logger.LogDebug("CORS request made for path: {path} from origin: {origin} but was ignored because path was not for an allowed IdentityServer CORS endpoint", path, origin);
+                    _logger.LogDebug("CORS request made for path: {path} from origin: {origin} but was ignored because path was not for an allowed IdentityServer CORS endpoint", Ioc.Sanitizer.Log.Sanitize(path), Ioc.Sanitizer.Log.Sanitize(origin));
                 }
             }
 

diff --git a/src/IdentityServer8/src/Hosting/EndpointRouter.cs b/src/IdentityServer8/src/Hosting/EndpointRouter.cs
@@ -16,6 +16,7 @@ copies or substantial portions of the Software.
 using IdentityServer8.Configuration;
 using IdentityServer8.Extensions;
 using Microsoft.AspNetCore.Http;
+using Microsoft.DependencyInjection.Extensions;
 using Microsoft.Extensions.Logging;
 using System;
 using System.Collections.Generic;
@@ -45,13 +46,13 @@ public IEndpointHandler Find(HttpContext context)
                 if (context.Request.Path.Equals(path, StringComparison.OrdinalIgnoreCase))
                 {
                     var endpointName = endpoint.Name;
-                    _logger.LogDebug("Request path {path} matched to endpoint type {endpoint}", context.Request.Path, endpointName);
+                    _logger.LogDebug("Request path {path} matched to endpoint type {endpoint}", Ioc.Sanitizer.Log.Sanitize(context.Request.Path), endpointName);
 
                     return GetEndpointHandler(endpoint, context);
                 }
             }
 
-            _logger.LogTrace("No endpoint entry found for request path: {path}", context.Request.Path);
+            _logger.LogTrace("No endpoint entry found for request path: {path}", Ioc.Sanitizer.Log.Sanitize(context.Request.Path));
 
             return null;
         }

diff --git a/src/IdentityServer8/src/Hosting/LocalApiAuthentication/LocalApiAuthenticationHandler.cs b/src/IdentityServer8/src/Hosting/LocalApiAuthentication/LocalApiAuthenticationHandler.cs
@@ -16,6 +16,7 @@ copies or substantial portions of the Software.
 using IdentityModel;
 using IdentityServer8.Validation;
 using Microsoft.AspNetCore.Authentication;
+using Microsoft.DependencyInjection.Extensions;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
 using System;
@@ -78,7 +79,7 @@ protected override async Task<AuthenticateResult> HandleAuthenticateAsync()
                 return AuthenticateResult.Fail("No Access Token is sent.");
             }
 
-            _logger.LogTrace("Token found: {token}", token);
+            _logger.LogTrace("Token found: {token}", Ioc.Sanitizer.Log.Sanitize(token));
 
             TokenValidationResult result = await _tokenValidator.ValidateAccessTokenAsync(token, Options.ExpectedScope);
 

diff --git a/src/IdentityServer8/src/IdentityServer8.csproj b/src/IdentityServer8/src/IdentityServer8.csproj
@@ -17,6 +17,7 @@
     </ItemGroup>
 
     <ItemGroup>
+        <ProjectReference Include="..\..\Security\IdentityServer8.Security\IdentityServer8.Security.csproj" />
         <ProjectReference Include="..\..\Storage\src\IdentityServer8.Storage.csproj" />
     </ItemGroup>
 </Project>
diff --git a/src/IdentityServer8/src/Services/Default/DefaultCorsPolicyService.cs b/src/IdentityServer8/src/Services/Default/DefaultCorsPolicyService.cs
@@ -13,12 +13,12 @@ copies or substantial portions of the Software.
 
 */
 
+using Microsoft.DependencyInjection.Extensions;
 using Microsoft.Extensions.Logging;
 using System;
 using System.Collections.Generic;
 using System.Linq;
 using System.Threading.Tasks;
-
 namespace IdentityServer8.Services
 {
     /// <summary>
@@ -67,24 +67,24 @@ public virtual Task<bool> IsOriginAllowedAsync(string origin)
             {
                 if (AllowAll)
                 {
-                    Logger.LogDebug("AllowAll true, so origin: {0} is allowed", origin);
+                    Logger.LogDebug("AllowAll true, so origin: {0} is allowed", (origin));
                     return Task.FromResult(true);
                 }
 
                 if (AllowedOrigins != null)
                 {
                     if (AllowedOrigins.Contains(origin, StringComparer.OrdinalIgnoreCase))
                     {
-                        Logger.LogDebug("AllowedOrigins configured and origin {0} is allowed", origin);
+                        Logger.LogDebug("AllowedOrigins configured and origin {0} is allowed", Ioc.Sanitizer.Log.Sanitize(origin));
                         return Task.FromResult(true);
                     }
                     else
                     {
-                        Logger.LogDebug("AllowedOrigins configured and origin {0} is not allowed", origin);
+                        Logger.LogDebug("AllowedOrigins configured and origin {0} is not allowed", Ioc.Sanitizer.Log.Sanitize(origin));
                     }
                 }
 
-                Logger.LogDebug("Exiting; origin {0} is not allowed", origin);
+                Logger.LogDebug("Exiting; origin {0} is not allowed", Ioc.Sanitizer.Log.Sanitize(origin));
             }
 
             return Task.FromResult(false);

diff --git a/src/IdentityServer8/src/Services/Default/LogoutNotificationService.cs b/src/IdentityServer8/src/Services/Default/LogoutNotificationService.cs
@@ -18,6 +18,7 @@ copies or substantial portions of the Software.
 using IdentityServer8.Models;
 using IdentityServer8.Stores;
 using Microsoft.AspNetCore.Http;
+using Microsoft.DependencyInjection.Extensions;
 using Microsoft.Extensions.Logging;
 using System.Collections.Generic;
 using System.Linq;
@@ -83,7 +84,7 @@ public async Task<IEnumerable<string>> GetFrontChannelLogoutNotificationsUrlsAsy
             if (frontChannelUrls.Any())
             {
                 var msg = frontChannelUrls.Aggregate((x, y) => x + ", " + y);
-                _logger.LogDebug("Client front-channel logout URLs: {0}", msg);
+                _logger.LogDebug("Client front-channel logout URLs: {0}", Ioc.Sanitizer.Log.Sanitize(msg));
             }
             else
             {

diff --git a/src/IdentityServer8/src/Services/InMemory/InMemoryCorsPolicyService.cs b/src/IdentityServer8/src/Services/InMemory/InMemoryCorsPolicyService.cs
@@ -20,7 +20,7 @@ copies or substantial portions of the Software.
 using System.Collections.Generic;
 using System.Linq;
 using System.Threading.Tasks;
-
+using Microsoft.DependencyInjection.Extensions;
 namespace IdentityServer8.Services
 {
     /// <summary>
@@ -64,11 +64,11 @@ from url in client.AllowedCorsOrigins
 
             if (result)
             {
-                Logger.LogDebug("Client list checked and origin: {0} is allowed", origin);
+                Logger.LogDebug("Client list checked and origin: {0} is allowed", Ioc.Sanitizer.Log.Sanitize(origin));
             }
             else
             {
-                Logger.LogDebug("Client list checked and origin: {0} is not allowed", origin);
+                Logger.LogDebug("Client list checked and origin: {0} is not allowed", Ioc.Sanitizer.Log.Sanitize(origin));
             }
 
             return Task.FromResult(result);

diff --git a/src/IdentityServer8/src/Stores/Default/DefaultGrantStore.cs b/src/IdentityServer8/src/Stores/Default/DefaultGrantStore.cs
@@ -20,7 +20,7 @@ copies or substantial portions of the Software.
 using Microsoft.Extensions.Logging;
 using System;
 using System.Threading.Tasks;
-
+using Microsoft.DependencyInjection.Extensions;
 namespace IdentityServer8.Stores
 {
     /// <summary>
@@ -113,7 +113,7 @@ protected virtual async Task<T> GetItemAsync(string key)
             }
             else
             {
-                Logger.LogDebug("{grantType} grant with value: {key} not found in store.", GrantType, key);
+                Logger.LogDebug("{grantType} grant with value: {key} not found in store.", GrantType, Ioc.Sanitizer.Log.Sanitize(key));
             }
 
             return default;