From dfd554117381348bdc0d1f2be7b9305e996f2759 Mon Sep 17 00:00:00 2001 From: Mathieu Guindon Date: Wed, 5 Feb 2025 11:36:47 -0500 Subject: [PATCH 1/2] fix signature check --- .../GitHubAuthenticationHandler.cs | 79 ------------------ .../WebhookAuthenticationHandler.cs | 56 +++++++++++++ .../WebhookSignatureValidationService.cs | 82 +++++++++++++++++++ 3 files changed, 138 insertions(+), 79 deletions(-) create mode 100644 rubberduckvba.Server/WebhookAuthenticationHandler.cs create mode 100644 rubberduckvba.Server/WebhookSignatureValidationService.cs diff --git a/rubberduckvba.Server/GitHubAuthenticationHandler.cs b/rubberduckvba.Server/GitHubAuthenticationHandler.cs index c833d74..1ed3f61 100644 --- a/rubberduckvba.Server/GitHubAuthenticationHandler.cs +++ b/rubberduckvba.Server/GitHubAuthenticationHandler.cs @@ -1,11 +1,8 @@  using Microsoft.AspNetCore.Authentication; using Microsoft.Extensions.Options; -using rubberduckvba.Server.Api.Admin; using rubberduckvba.Server.Services; using System.Security.Claims; -using System.Security.Cryptography; -using System.Text; using System.Text.Encodings.Web; namespace rubberduckvba.Server; @@ -35,79 +32,3 @@ protected async override Task HandleAuthenticateAsync() : AuthenticateResult.NoResult(); } } - -public class WebhookAuthenticationHandler : AuthenticationHandler -{ - private readonly ConfigurationOptions _configuration; - - public WebhookAuthenticationHandler(IOptionsMonitor options, ILoggerFactory logger, UrlEncoder encoder, - ConfigurationOptions configuration) - : base(options, logger, encoder) - { - _configuration = configuration; - } - - protected async override Task HandleAuthenticateAsync() - { - return await Task.Run(() => - { - var xGitHubEvent = Context.Request.Headers["X-GitHub-Event"]; - var xGitHubDelivery = Context.Request.Headers["X-GitHub-Delivery"]; - var xHubSignature = Context.Request.Headers["X-Hub-Signature"]; - var xHubSignature256 = Context.Request.Headers["X-Hub-Signature-256"]; - - if (!xGitHubEvent.Contains("push")) - { - // only authenticate push events - return AuthenticateResult.NoResult(); - } - - if (!Guid.TryParse(xGitHubDelivery.SingleOrDefault(), out _)) - { - // delivery should parse as a GUID - return AuthenticateResult.NoResult(); - } - - if (!xHubSignature.Any()) - { - // signature header should be present - return AuthenticateResult.NoResult(); - } - - var signature = xHubSignature256.SingleOrDefault(); - - using var reader = new StreamReader(Context.Request.Body); - var payload = reader.ReadToEndAsync().GetAwaiter().GetResult(); - - if (!IsValidSignature(signature, payload)) - { - // encrypted signature must be present - return AuthenticateResult.NoResult(); - } - - var identity = new ClaimsIdentity("webhook", ClaimTypes.Name, ClaimTypes.Role); - identity.AddClaim(new Claim(ClaimTypes.Name, "rubberduck-vba-releasebot")); - identity.AddClaim(new Claim(ClaimTypes.Role, "rubberduck-webhook")); - identity.AddClaim(new Claim(ClaimTypes.Authentication, "webhook-signature")); - - var principal = new ClaimsPrincipal(identity); - return AuthenticateResult.Success(new AuthenticationTicket(principal, "webhook-signature")); - }); - } - - private bool IsValidSignature(string? signature, string payload) - { - if (string.IsNullOrWhiteSpace(signature)) - { - return false; - } - - using var sha256 = SHA256.Create(); - - var secret = _configuration.GitHubOptions.Value.WebhookToken; - var bytes = Encoding.UTF8.GetBytes(secret + payload); - var check = $"sha256={Encoding.UTF8.GetString(sha256.ComputeHash(bytes))}"; - - return check == payload; - } -} \ No newline at end of file diff --git a/rubberduckvba.Server/WebhookAuthenticationHandler.cs b/rubberduckvba.Server/WebhookAuthenticationHandler.cs new file mode 100644 index 0000000..9b6c528 --- /dev/null +++ b/rubberduckvba.Server/WebhookAuthenticationHandler.cs @@ -0,0 +1,56 @@ + +using Microsoft.AspNetCore.Authentication; +using Microsoft.Extensions.Options; +using System.Security.Claims; +using System.Text.Encodings.Web; + +namespace rubberduckvba.Server; + +public class WebhookAuthenticationHandler : AuthenticationHandler +{ + private readonly WebhookSignatureValidationService _service; + + public WebhookAuthenticationHandler(IOptionsMonitor options, ILoggerFactory logger, UrlEncoder encoder, + WebhookSignatureValidationService service) + : base(options, logger, encoder) + { + _service = service; + } + + protected async override Task HandleAuthenticateAsync() + { + return await Task.Run(() => + { + var userAgent = Context.Request.Headers.UserAgent; + var xGitHubEvent = Context.Request.Headers["X-GitHub-Event"].OfType().ToArray(); + var xGitHubDelivery = Context.Request.Headers["X-GitHub-Delivery"].OfType().ToArray(); + var xHubSignature = Context.Request.Headers["X-Hub-Signature"].OfType().ToArray(); + var xHubSignature256 = Context.Request.Headers["X-Hub-Signature-256"].OfType().ToArray(); + + using var reader = new StreamReader(Context.Request.Body); + var payload = reader.ReadToEndAsync().GetAwaiter().GetResult(); + + if (_service.Validate(payload, userAgent, xGitHubEvent, xGitHubDelivery, xHubSignature, xHubSignature256)) + { + var principal = CreatePrincipal(); + var ticket = new AuthenticationTicket(principal, "webhook-signature"); + + return AuthenticateResult.Success(ticket); + } + + return AuthenticateResult.NoResult(); + }); + } + + private static ClaimsPrincipal CreatePrincipal() + { + var identity = new ClaimsIdentity("webhook", ClaimTypes.Name, ClaimTypes.Role); + + identity.AddClaim(new Claim(ClaimTypes.Name, "rubberduck-vba-releasebot")); + identity.AddClaim(new Claim(ClaimTypes.Role, "rubberduck-webhook")); + identity.AddClaim(new Claim(ClaimTypes.Authentication, "webhook-signature")); + + var principal = new ClaimsPrincipal(identity); + return principal; + } +} \ No newline at end of file diff --git a/rubberduckvba.Server/WebhookSignatureValidationService.cs b/rubberduckvba.Server/WebhookSignatureValidationService.cs new file mode 100644 index 0000000..f07f9ab --- /dev/null +++ b/rubberduckvba.Server/WebhookSignatureValidationService.cs @@ -0,0 +1,82 @@ +using rubberduckvba.Server.Api.Admin; +using System.Diagnostics; +using System.Security.Cryptography; +using System.Text; + +namespace rubberduckvba.Server; + +public class WebhookSignatureValidationService(ConfigurationOptions configuration) +{ + public bool Validate( + string payload, + string? userAgent, + string[] xGitHubEvent, + string[] xGitHubDelivery, + string[] xHubSignature, + string[] xHubSignature256 + ) + { + if (!(userAgent ?? string.Empty).StartsWith("GitHub-Hookshot/")) + { + // user agent must be GitHub hookshot + LogMissingHeader("USER-AGENT"); + return false; + } + + if (!xGitHubEvent.Contains("push")) + { + // only authenticate push events + LogMissingHeader("X-GITHUB-EVENT"); + return false; + } + + if (!Guid.TryParse(xGitHubDelivery.SingleOrDefault(), out _)) + { + // delivery should parse as a GUID + LogMissingHeader("X-GITHUB-DELIVERY"); + return false; + } + + if (!xHubSignature.Any()) + { + // SHA-1 signature header must be present + LogMissingHeader("X-HUB-SIGNATURE"); + return false; + } + + var signature = xHubSignature256.SingleOrDefault(); + if (signature == default) + { + // SHA-256 signature header must be present + LogMissingHeader("X-HUB-SIGNATURE-256"); + return false; + } + + if (!IsValidSignature(signature, payload)) + { + // SHA-256 signature must match + Debug.WriteLine("Signature validation failed"); + return false; + } + + return true; + } + + //[Conditional("DEBUG")] + private void LogMissingHeader(string header) => Console.WriteLine($"** Webhook validation failed. Missing header: [{header}]"); + + private bool IsValidSignature(string? signature, string payload) + { + if (string.IsNullOrWhiteSpace(signature)) + { + return false; + } + using var sha256 = SHA256.Create(); + + var secret = configuration.GitHubOptions.Value.WebhookToken; + var bytes = Encoding.UTF8.GetBytes(secret + payload); + var check = $"sha256={Encoding.UTF8.GetString(sha256.ComputeHash(bytes))}"; + + return signature == check; + } +} From 1255e09859f555e7619fd9c4a34d31d8cfd26fd2 Mon Sep 17 00:00:00 2001 From: Mathieu Guindon Date: Wed, 5 Feb 2025 11:38:06 -0500 Subject: [PATCH 2/2] remove validation output --- .../WebhookSignatureValidationService.cs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/rubberduckvba.Server/WebhookSignatureValidationService.cs b/rubberduckvba.Server/WebhookSignatureValidationService.cs index f07f9ab..28cc8ab 100644 --- a/rubberduckvba.Server/WebhookSignatureValidationService.cs +++ b/rubberduckvba.Server/WebhookSignatureValidationService.cs @@ -1,5 +1,4 @@ using rubberduckvba.Server.Api.Admin; -using System.Diagnostics; using System.Security.Cryptography; using System.Text; @@ -19,28 +18,24 @@ string[] xHubSignature256 if (!(userAgent ?? string.Empty).StartsWith("GitHub-Hookshot/")) { // user agent must be GitHub hookshot - LogMissingHeader("USER-AGENT"); return false; } if (!xGitHubEvent.Contains("push")) { // only authenticate push events - LogMissingHeader("X-GITHUB-EVENT"); return false; } if (!Guid.TryParse(xGitHubDelivery.SingleOrDefault(), out _)) { // delivery should parse as a GUID - LogMissingHeader("X-GITHUB-DELIVERY"); return false; } if (!xHubSignature.Any()) { // SHA-1 signature header must be present - LogMissingHeader("X-HUB-SIGNATURE"); return false; } @@ -48,23 +43,18 @@ string[] xHubSignature256 if (signature == default) { // SHA-256 signature header must be present - LogMissingHeader("X-HUB-SIGNATURE-256"); return false; } if (!IsValidSignature(signature, payload)) { // SHA-256 signature must match - Debug.WriteLine("Signature validation failed"); return false; } return true; } - //[Conditional("DEBUG")] - private void LogMissingHeader(string header) => Console.WriteLine($"** Webhook validation failed. Missing header: [{header}]"); - private bool IsValidSignature(string? signature, string payload) { if (string.IsNullOrWhiteSpace(signature))