Fix webhook signature validation

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<AuthenticateResult> HandleAuthenticateAsync()
             : AuthenticateResult.NoResult();
     }
 }
-
-public class WebhookAuthenticationHandler : AuthenticationHandler<AuthenticationSchemeOptions>
-{
-    private readonly ConfigurationOptions _configuration;
-
-    public WebhookAuthenticationHandler(IOptionsMonitor<AuthenticationSchemeOptions> options, ILoggerFactory logger, UrlEncoder encoder,
-        ConfigurationOptions configuration)
-        : base(options, logger, encoder)
-    {
-        _configuration = configuration;
-    }
-
-    protected async override Task<AuthenticateResult> 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;
-    }
-}

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<AuthenticationSchemeOptions>
+{
+    private readonly WebhookSignatureValidationService _service;
+
+    public WebhookAuthenticationHandler(IOptionsMonitor<AuthenticationSchemeOptions> options, ILoggerFactory logger, UrlEncoder encoder,
+        WebhookSignatureValidationService service)
+        : base(options, logger, encoder)
+    {
+        _service = service;
+    }
+
+    protected async override Task<AuthenticateResult> HandleAuthenticateAsync()
+    {
+        return await Task.Run(() =>
+        {
+            var userAgent = Context.Request.Headers.UserAgent;
+            var xGitHubEvent = Context.Request.Headers["X-GitHub-Event"].OfType<string>().ToArray();
+            var xGitHubDelivery = Context.Request.Headers["X-GitHub-Delivery"].OfType<string>().ToArray();
+            var xHubSignature = Context.Request.Headers["X-Hub-Signature"].OfType<string>().ToArray();
+            var xHubSignature256 = Context.Request.Headers["X-Hub-Signature-256"].OfType<string>().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;
+    }
+}

rubberduckvba.Server/WebhookSignatureValidationService.cs

@@ -0,0 +1,72 @@
+using rubberduckvba.Server.Api.Admin;
+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
+            return false;
+        }
+
+        if (!xGitHubEvent.Contains("push"))
+        {
+            // only authenticate push events
+            return false;
+        }
+
+        if (!Guid.TryParse(xGitHubDelivery.SingleOrDefault(), out _))
+        {
+            // delivery should parse as a GUID
+            return false;
+        }
+
+        if (!xHubSignature.Any())
+        {
+            // SHA-1 signature header must be present
+            return false;
+        }
+
+        var signature = xHubSignature256.SingleOrDefault();
+        if (signature == default)
+        {
+            // SHA-256 signature header must be present
+            return false;
+        }
+
+        if (!IsValidSignature(signature, payload))
+        {
+            // SHA-256 signature must match
+            return false;
+        }
+
+        return true;
+    }
+
+    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;
+    }
+}