From 8e5d5af22454a8eddeac0ee20f06543f6d7f03ed Mon Sep 17 00:00:00 2001 From: Claudia Murialdo Date: Fri, 22 Dec 2023 15:15:46 -0300 Subject: [PATCH 1/5] Extend CSRF validation to all backend services requested through POST/DELETE/PUT methods --- .../dotnetcore/GxNetCoreStartup/CsrfHelper.cs | 12 +++- .../dotnetcore/GxNetCoreStartup/Startup.cs | 4 +- .../GxClasses/Middleware/GXHttp.cs | 3 + .../Middleware/RestServiceTest.cs | 71 +++++++++++++------ 4 files changed, 63 insertions(+), 27 deletions(-) diff --git a/dotnet/src/dotnetcore/GxNetCoreStartup/CsrfHelper.cs b/dotnet/src/dotnetcore/GxNetCoreStartup/CsrfHelper.cs index 2306f028e..2020e7371 100644 --- a/dotnet/src/dotnetcore/GxNetCoreStartup/CsrfHelper.cs +++ b/dotnet/src/dotnetcore/GxNetCoreStartup/CsrfHelper.cs @@ -13,13 +13,15 @@ public class ValidateAntiForgeryTokenMiddleware static readonly IGXLogger log = GXLoggerFactory.GetLogger(); private readonly RequestDelegate _next; private readonly IAntiforgery _antiforgery; + private string _restBasePath; private string _basePath; - public ValidateAntiForgeryTokenMiddleware(RequestDelegate next, IAntiforgery antiforgery, String basePath) + public ValidateAntiForgeryTokenMiddleware(RequestDelegate next, IAntiforgery antiforgery, string basePath) { _next = next; _antiforgery = antiforgery; - _basePath = "/" + basePath; + _restBasePath = $"{basePath}{Startup.REST_BASE_URL}"; + _basePath = $"/{basePath}"; } public async Task Invoke(HttpContext context) @@ -42,9 +44,13 @@ public async Task Invoke(HttpContext context) SetAntiForgeryTokens(_antiforgery, context); } } - if (!context.Request.Path.Value.EndsWith(_basePath)) //VerificationToken + if (!IsVerificationTokenServiceRequest(context)) await _next(context); } + private bool IsVerificationTokenServiceRequest(HttpContext context) + { + return context.Request.Path.Value.EndsWith(_restBasePath); + } internal static void SetAntiForgeryTokens(IAntiforgery _antiforgery, HttpContext context) { AntiforgeryTokenSet tokenSet = _antiforgery.GetAndStoreTokens(context); diff --git a/dotnet/src/dotnetcore/GxNetCoreStartup/Startup.cs b/dotnet/src/dotnetcore/GxNetCoreStartup/Startup.cs index 5cda19ae5..4f8c7968f 100644 --- a/dotnet/src/dotnetcore/GxNetCoreStartup/Startup.cs +++ b/dotnet/src/dotnetcore/GxNetCoreStartup/Startup.cs @@ -133,7 +133,7 @@ public class Startup const string RESOURCES_FOLDER = "Resources"; const string TRACE_FOLDER = "logs"; const string TRACE_PATTERN = "trace.axd"; - const string REST_BASE_URL = "rest/"; + internal const string REST_BASE_URL = "rest/"; const string DATA_PROTECTION_KEYS = "DataProtection-Keys"; const string REWRITE_FILE = "rewrite.config"; const string SWAGGER_DEFAULT_YAML = "default.yaml"; @@ -411,7 +411,7 @@ public void Configure(IApplicationBuilder app, Microsoft.AspNetCore.Hosting.IHos if (RestAPIHelpers.ValidateCsrfToken()) { antiforgery = app.ApplicationServices.GetRequiredService(); - app.UseAntiforgeryTokens(restBasePath); + app.UseAntiforgeryTokens(apiBasePath); } app.UseMvc(routes => { diff --git a/dotnet/src/dotnetframework/GxClasses/Middleware/GXHttp.cs b/dotnet/src/dotnetframework/GxClasses/Middleware/GXHttp.cs index a91181789..bceb6e1ab 100644 --- a/dotnet/src/dotnetframework/GxClasses/Middleware/GXHttp.cs +++ b/dotnet/src/dotnetframework/GxClasses/Middleware/GXHttp.cs @@ -1934,6 +1934,9 @@ public void ProcessRequest(HttpContext httpContext) SetStreaming(); SendHeaders(); string clientid = context.ClientID; //Send clientid cookie (before response HasStarted) if necessary, since UseResponseBuffering is not in .netcore3.0 +#if !NETCORE + CSRFHelper.ValidateAntiforgery(httpContext); +#endif bool validSession = ValidWebSession(); if (validSession && IntegratedSecurityEnabled) diff --git a/dotnet/test/DotNetCoreAttackMitigationTest/Middleware/RestServiceTest.cs b/dotnet/test/DotNetCoreAttackMitigationTest/Middleware/RestServiceTest.cs index d292393c7..42b454992 100644 --- a/dotnet/test/DotNetCoreAttackMitigationTest/Middleware/RestServiceTest.cs +++ b/dotnet/test/DotNetCoreAttackMitigationTest/Middleware/RestServiceTest.cs @@ -24,21 +24,19 @@ public RestServiceTest() : base() server.AllowSynchronousIO = true; } - - public async Task TestSimpleRestPost() + [Fact] + public async Task InvalidPostToRestService() { server.AllowSynchronousIO = true; HttpClient client = server.CreateClient(); StringContent body = new StringContent("{\"Image\":\"imageName\",\"ImageDescription\":\"imageDescription\"}"); HttpResponseMessage response = await client.PostAsync("rest/apps/saveimage", body); - Assert.Equal(System.Net.HttpStatusCode.BadRequest, response.StatusCode); + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); } - - public async Task RunController() + [Fact] + public async Task ValidPostToRestService() { - - - CookieContainer cookies = new System.Net.CookieContainer(); + CookieContainer cookies = new CookieContainer(); HttpClient client = server.CreateClient(); string requestUri = "rest/apps/append"; Uri requestUriObj = new Uri("http://localhost/" + requestUri); @@ -55,39 +53,68 @@ public async Task RunController() csrfToken = setCookie.Value.Value; response.EnsureSuccessStatusCode(); - Assert.Equal(System.Net.HttpStatusCode.OK, response.StatusCode); //When failed, turn on log.config to see server side error. + Assert.Equal(HttpStatusCode.OK, response.StatusCode); //When failed, turn on log.config to see server side error. StringContent body = new StringContent("{\"Image\":\"imageName\",\"ImageDescription\":\"imageDescription\"}"); client.DefaultRequestHeaders.Add(HttpHeader.X_CSRF_TOKEN_HEADER, csrfToken); client.DefaultRequestHeaders.Add("Cookie", values);// //cookies.GetCookieHeader(requestUriObj)); response = await client.PostAsync("rest/apps/saveimage", body); - Assert.Equal(System.Net.HttpStatusCode.OK, response.StatusCode); + Assert.Equal(HttpStatusCode.OK, response.StatusCode); } - public async Task HttpFirstPost() + [Fact] + public async Task ValidPostToHttpService() + { + CookieContainer cookies = new CookieContainer(); + HttpClient client = server.CreateClient(); + string requestUri = "webhook.aspx"; + Uri requestUriObj = new Uri("http://localhost/" + requestUri); + HttpResponseMessage response = await client.GetAsync(requestUri); + string csrfToken = string.Empty; + + IEnumerable values; + Assert.True(response.Headers.TryGetValues("Set-Cookie", out values)); + + foreach (var item in SetCookieHeaderValue.ParseList(values.ToList())) + cookies.Add(requestUriObj, new Cookie(item.Name.Value, item.Value.Value, item.Path.Value)); + + var setCookie = SetCookieHeaderValue.ParseList(values.ToList()).FirstOrDefault(t => t.Name.Equals(HttpHeader.X_CSRF_TOKEN_COOKIE, StringComparison.OrdinalIgnoreCase)); + csrfToken = setCookie.Value.Value; + + response.EnsureSuccessStatusCode(); + Assert.Equal(HttpStatusCode.OK, response.StatusCode); //When failed, turn on log.config to see server side error. + + client.DefaultRequestHeaders.Add(HttpHeader.X_CSRF_TOKEN_HEADER, csrfToken); + client.DefaultRequestHeaders.Add("Cookie", values);// //cookies.GetCookieHeader(requestUriObj)); + + response = await client.PostAsync(requestUri, null); + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } + [Fact] + public async Task InvalidPostToHttpService() { HttpClient client = server.CreateClient(); HttpResponseMessage response = await client.PostAsync("webhook.aspx", null); IEnumerable cookies = response.Headers.SingleOrDefault(header => header.Key == "Set-Cookie").Value; - foreach (string cookie in cookies) + if (cookies != null) { - Assert.False(cookie.StartsWith(HttpHeader.X_CSRF_TOKEN_COOKIE)); + foreach (string cookie in cookies) + { + Assert.False(cookie.StartsWith(HttpHeader.X_CSRF_TOKEN_COOKIE)); + } } - response.EnsureSuccessStatusCode(); - Assert.Equal(System.Net.HttpStatusCode.OK, response.StatusCode); + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + Assert.Contains("InvalidCSRFToken", response.ReasonPhrase, StringComparison.OrdinalIgnoreCase); } - public async Task HttpFirstGet() + [Fact] + public async Task GetToHttpService() { HttpClient client = server.CreateClient(); HttpResponseMessage response = await client.GetAsync("webhook.aspx"); IEnumerable cookies = response.Headers.SingleOrDefault(header => header.Key == "Set-Cookie").Value; - foreach (string cookie in cookies) - { - Assert.False(cookie.StartsWith(HttpHeader.X_CSRF_TOKEN_COOKIE)); - } - + Assert.Contains(cookies, x => x.StartsWith(HttpHeader.X_CSRF_TOKEN_COOKIE, StringComparison.OrdinalIgnoreCase)); response.EnsureSuccessStatusCode(); - Assert.Equal(System.Net.HttpStatusCode.OK, response.StatusCode); + Assert.Equal(HttpStatusCode.OK, response.StatusCode); } } } From 3335e2e6b790fe10ec8bbd61cb56487a9fc28079 Mon Sep 17 00:00:00 2001 From: Claudia Murialdo Date: Fri, 22 Dec 2023 15:31:25 -0300 Subject: [PATCH 2/5] Rename ValidateCSRF to CSRF_PROTECTION in accordance with the spec. --- dotnet/src/dotnetframework/GxClasses/Helpers/GXRestUtils.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dotnet/src/dotnetframework/GxClasses/Helpers/GXRestUtils.cs b/dotnet/src/dotnetframework/GxClasses/Helpers/GXRestUtils.cs index 27e7b1928..ad899d16a 100644 --- a/dotnet/src/dotnetframework/GxClasses/Helpers/GXRestUtils.cs +++ b/dotnet/src/dotnetframework/GxClasses/Helpers/GXRestUtils.cs @@ -141,7 +141,7 @@ public static Dictionary ReadRestBodyParameters(Stream stream) internal static bool ValidateCsrfToken() { - return Config.GetValueOf("ValidateCSRF", Preferences.NO) == Preferences.YES; + return Config.GetValueOf("CSRF_PROTECTION", Preferences.NO) == Preferences.YES; } } } From 0404b50876e283c453f0f73870df66020886f7b1 Mon Sep 17 00:00:00 2001 From: Claudia Murialdo Date: Fri, 22 Dec 2023 15:32:15 -0300 Subject: [PATCH 3/5] Rename ValidateCSRF to CSRF_PROTECTION at appsettings.json in accordance with the spec. --- dotnet/test/DotNetCoreAttackMitigationTest/appsettings.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dotnet/test/DotNetCoreAttackMitigationTest/appsettings.json b/dotnet/test/DotNetCoreAttackMitigationTest/appsettings.json index 4eb415947..f97425b16 100644 --- a/dotnet/test/DotNetCoreAttackMitigationTest/appsettings.json +++ b/dotnet/test/DotNetCoreAttackMitigationTest/appsettings.json @@ -50,7 +50,7 @@ "CACHE_INVALIDATION_TOKEN": "20216211291931", "CORS_ALLOW_ORIGIN": "https://normal-website.com", "MY_CUSTOM_PTY": "DEFAULT_VALUE", - "ValidateCSRF": "1" + "CSRF_PROTECTION": "1" }, "languages": { "English": { From 32eada945184b9c22582b41f76985134bdfd28c2 Mon Sep 17 00:00:00 2001 From: Claudia Murialdo Date: Fri, 22 Dec 2023 16:20:59 -0300 Subject: [PATCH 4/5] Check csrf token is not null. --- .../Middleware/RestServiceTest.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/dotnet/test/DotNetCoreAttackMitigationTest/Middleware/RestServiceTest.cs b/dotnet/test/DotNetCoreAttackMitigationTest/Middleware/RestServiceTest.cs index 42b454992..987a51b60 100644 --- a/dotnet/test/DotNetCoreAttackMitigationTest/Middleware/RestServiceTest.cs +++ b/dotnet/test/DotNetCoreAttackMitigationTest/Middleware/RestServiceTest.cs @@ -49,7 +49,9 @@ public async Task ValidPostToRestService() foreach (var item in SetCookieHeaderValue.ParseList(values.ToList())) cookies.Add(requestUriObj, new Cookie(item.Name.Value, item.Value.Value, item.Path.Value)); - var setCookie = SetCookieHeaderValue.ParseList(values.ToList()).FirstOrDefault(t => t.Name.Equals(HttpHeader.X_CSRF_TOKEN_COOKIE, StringComparison.OrdinalIgnoreCase)); + SetCookieHeaderValue setCookie = SetCookieHeaderValue.ParseList(values.ToList()).FirstOrDefault(t => t.Name.Equals(HttpHeader.X_CSRF_TOKEN_COOKIE, StringComparison.OrdinalIgnoreCase)); + Assert.NotNull(setCookie); + Assert.True(setCookie.Value.HasValue); csrfToken = setCookie.Value.Value; response.EnsureSuccessStatusCode(); @@ -78,7 +80,9 @@ public async Task ValidPostToHttpService() foreach (var item in SetCookieHeaderValue.ParseList(values.ToList())) cookies.Add(requestUriObj, new Cookie(item.Name.Value, item.Value.Value, item.Path.Value)); - var setCookie = SetCookieHeaderValue.ParseList(values.ToList()).FirstOrDefault(t => t.Name.Equals(HttpHeader.X_CSRF_TOKEN_COOKIE, StringComparison.OrdinalIgnoreCase)); + SetCookieHeaderValue setCookie = SetCookieHeaderValue.ParseList(values.ToList()).FirstOrDefault(t => t.Name.Equals(HttpHeader.X_CSRF_TOKEN_COOKIE, StringComparison.OrdinalIgnoreCase)); + Assert.NotNull(setCookie); + Assert.True(setCookie.Value.HasValue); csrfToken = setCookie.Value.Value; response.EnsureSuccessStatusCode(); From 6b8ec0e008358f28b4cc7a1d6684f96ad25c42bb Mon Sep 17 00:00:00 2001 From: Claudia Murialdo Date: Tue, 26 Dec 2023 14:03:11 -0300 Subject: [PATCH 5/5] Set StatusCode to 400 on web panels and REST procedures that fail CSRF token validation --- .../dotnetcore/GxNetCoreStartup/Startup.cs | 4 +-- .../GxClasses/Helpers/HttpHelper.cs | 8 ++++-- .../GxClasses/Middleware/GXHttp.cs | 28 +++++++++++++------ .../GxClasses/Services/GXRestServices.cs | 6 +++- 4 files changed, 32 insertions(+), 14 deletions(-) diff --git a/dotnet/src/dotnetcore/GxNetCoreStartup/Startup.cs b/dotnet/src/dotnetcore/GxNetCoreStartup/Startup.cs index 4f8c7968f..dfac41bc5 100644 --- a/dotnet/src/dotnetcore/GxNetCoreStartup/Startup.cs +++ b/dotnet/src/dotnetcore/GxNetCoreStartup/Startup.cs @@ -501,7 +501,6 @@ bool IsAspx(HttpContext context, string basePath) } public class CustomExceptionHandlerMiddleware { - const string InvalidCSRFToken = "InvalidCSRFToken"; static readonly IGXLogger log = GXLoggerFactory.GetLogger(); public async Task Invoke(HttpContext httpContext) { @@ -516,9 +515,8 @@ public async Task Invoke(HttpContext httpContext) } else if (ex is AntiforgeryValidationException) { - //"The required antiforgery header value "X-GXCSRF-TOKEN" is not present. httpStatusCode = HttpStatusCode.BadRequest; - httpReasonPhrase = InvalidCSRFToken; + httpReasonPhrase = HttpHelper.InvalidCSRFToken; GXLogging.Error(log, $"Validation of antiforgery failed", ex); } else diff --git a/dotnet/src/dotnetframework/GxClasses/Helpers/HttpHelper.cs b/dotnet/src/dotnetframework/GxClasses/Helpers/HttpHelper.cs index 0e19a76d5..a0e94c333 100644 --- a/dotnet/src/dotnetframework/GxClasses/Helpers/HttpHelper.cs +++ b/dotnet/src/dotnetframework/GxClasses/Helpers/HttpHelper.cs @@ -93,7 +93,7 @@ public class HttpHelper const string GAM_CODE_TFA_USER_MUST_VALIDATE = "410"; const string GAM_CODE_TOKEN_EXPIRED = "103"; static Regex CapitalsToTitle = new Regex(@"(?<=[A-Z])(?=[A-Z][a-z]) | (?<=[^A-Z])(?=[A-Z]) | (?<=[A-Za-z])(?=[^A-Za-z])", RegexOptions.IgnorePatternWhitespace); - + internal const string InvalidCSRFToken = "InvalidCSRFToken"; const string CORS_MAX_AGE_SECONDS = "86400"; internal static void CorsHeaders(HttpContext httpContext) { @@ -286,10 +286,14 @@ internal static void TraceUnexpectedError(Exception ex) } internal static void SetUnexpectedError(HttpContext httpContext, HttpStatusCode statusCode, Exception ex) + { + string statusCodeDesc = StatusCodeToTitle(statusCode); + SetUnexpectedError(httpContext, statusCode, statusCodeDesc, ex); + } + internal static void SetUnexpectedError(HttpContext httpContext, HttpStatusCode statusCode, string statusCodeDesc, Exception ex) { TraceUnexpectedError(ex); string statusCodeStr = statusCode.ToString(INT_FORMAT); - string statusCodeDesc = StatusCodeToTitle(statusCode); SetResponseStatus(httpContext, statusCode, statusCodeDesc); SetJsonError(httpContext, statusCodeStr, statusCodeDesc); } diff --git a/dotnet/src/dotnetframework/GxClasses/Middleware/GXHttp.cs b/dotnet/src/dotnetframework/GxClasses/Middleware/GXHttp.cs index bceb6e1ab..ce60e5af2 100644 --- a/dotnet/src/dotnetframework/GxClasses/Middleware/GXHttp.cs +++ b/dotnet/src/dotnetframework/GxClasses/Middleware/GXHttp.cs @@ -36,17 +36,17 @@ namespace GeneXus.Http using System.Web; using System.Web.UI; using System.Web.UI.WebControls; - using System.Web.Script.Serialization; using System.Net; using GeneXus.Notifications; using Web.Security; using System.Web.SessionState; - using GeneXus.Mock; using GeneXus.Data.NTier; -#endif + using System.Web.Mvc; + using System.Security; +#endif #if NETCORE public abstract class GXHttpHandler : GXBaseObject, IHttpHandler #else @@ -1905,8 +1905,9 @@ public bool IsMain get { return _isMain; } } #endif - - +#if !NETCORE + [SecuritySafeCritical] +#endif public void ProcessRequest(HttpContext httpContext) { localHttpContext = httpContext; @@ -1981,9 +1982,20 @@ public void ProcessRequest(HttpContext httpContext) context.CloseConnections(); } catch { } - Exception exceptionToHandle = e.InnerException ?? e; - handleException(exceptionToHandle.GetType().FullName, exceptionToHandle.Message, exceptionToHandle.StackTrace); - throw new Exception("GXApplication exception", e); +#if !NETCORE + if (e is HttpAntiForgeryException) + { + httpContext.Response.StatusCode = (int)HttpStatusCode.BadRequest; + httpContext.Response.StatusDescription = HttpHelper.InvalidCSRFToken; + GXLogging.Error(log, $"Validation of antiforgery failed", e); + } + else +#endif + { + Exception exceptionToHandle = e.InnerException ?? e; + handleException(exceptionToHandle.GetType().FullName, exceptionToHandle.Message, exceptionToHandle.StackTrace); + throw new Exception("GXApplication exception", e); + } } } protected virtual bool ChunkedStreaming() { return false; } diff --git a/dotnet/src/dotnetframework/GxClasses/Services/GXRestServices.cs b/dotnet/src/dotnetframework/GxClasses/Services/GXRestServices.cs index 0d974268a..7f63cc340 100644 --- a/dotnet/src/dotnetframework/GxClasses/Services/GXRestServices.cs +++ b/dotnet/src/dotnetframework/GxClasses/Services/GXRestServices.cs @@ -389,10 +389,14 @@ public void WebException(Exception ex) { throw ex; } - else if (ex is FormatException || (RestAPIHelpers.ValidateCsrfToken() && AntiForgeryException(ex))) + else if (ex is FormatException) { HttpHelper.SetUnexpectedError(httpContext, HttpStatusCode.BadRequest, ex); } + else if (RestAPIHelpers.ValidateCsrfToken() && AntiForgeryException(ex)) + { + HttpHelper.SetUnexpectedError(httpContext, HttpStatusCode.BadRequest, HttpHelper.InvalidCSRFToken, ex); + } else { HttpHelper.SetUnexpectedError(httpContext, HttpStatusCode.InternalServerError, ex);