Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support hot-swapping of webhok secrets #262

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
+329 −148
Draft

Support hot-swapping of webhok secrets #262

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
153 changes: 72 additions & 81 deletions src/Octokit.Webhooks.AspNetCore/GitHubWebhookExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
namespace Octokit.Webhooks.AspNetCore;

using System;
using System.Globalization;
using System.Collections.Generic;
using System.IO;
using System.Net.Mime;
using System.Security.Cryptography;
using System.Text;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Http;
Expand All @@ -19,25 +16,84 @@
/// </summary>
public static partial class GitHubWebhookExtensions
{
public static void MapGitHubWebhooks(this IEndpointRouteBuilder endpoints, string path = "/api/github/webhooks", string secret = null!) =>
/// <inheritdoc cref="MapGitHubWebhooks(IEndpointRouteBuilder,string,Func{Task{IEnumerable{string}?}})"/>
/// <param name="endpoints">
/// The <c>Microsoft.AspNetCore.Routing.IEndpointRouteBuilder</c> to add the route to.
/// </param>
/// <param name="path">The path of the webhook endpoint.</param>
/// <param name="secret">
/// The secret to use to validate each webhook request's signature, or null if no signature is
/// to be expected.
/// </param>
public static void MapGitHubWebhooks(this IEndpointRouteBuilder endpoints, string path = "/api/github/webhooks", string secret = null!)
{
var secrets = string.IsNullOrEmpty(secret) ? null : new string[] { secret };
MapGitHubWebhooks(endpoints, path, () => Task.FromResult<IEnumerable<string>?>(secrets));
}

/// <summary>
/// Adds an endpoint for processing GitHub webhook payloads.
/// </summary>
/// <param name="endpoints">
/// The <c>Microsoft.AspNetCore.Routing.IEndpointRouteBuilder</c> to add the route to.
/// </param>
/// <param name="path">The path of the webhook endpoint.</param>
/// <param name="secrets">
/// A callback delegate that will return the list of secrets to use to validate a request's
/// signature.
/// </param>
/// <remarks>
/// <para>
/// The <paramref name="secrets"/> delegate will be invoked once for each request. If the
/// delegate returns null, then no signature will be expected. If the delegate returns a list of
/// one or more secrets, then the request's signature must match one of those secrets. If the
/// delegate returns an empty list, then the request's signature will be considered invalid.
/// </para>
/// <para>
/// Because the delegate is invoked separately for each request, the webhook secret can be
/// changed without modifying the route. Returning bpth the old and the new secrets for some
/// time will allow any webhook requests already in flight to validate against the old secret
/// while any new requests will validate against the new secret.
/// </para>
/// </remarks>
public static void MapGitHubWebhooks(this IEndpointRouteBuilder endpoints, string path, Func<Task<IEnumerable<string>?>> secrets) =>
endpoints.MapPost(path, async context =>
{
var logger = context.RequestServices.GetRequiredService<ILogger<WebhookEventProcessor>>();

// Verify content type
if (!VerifyContentType(context, MediaTypeNames.Application.Json))
// Validate request
string body;
try
{
Log.IncorrectContentType(logger);
return;
var secretsList = await secrets().ConfigureAwait(false);
body = await WebhookEventProcessor.ValidateWebhookRequestAsync(context.Request.Headers, context.Request.Body, secretsList).ConfigureAwait(false);
}

// Get body
var body = await GetBodyAsync(context).ConfigureAwait(false);

// Verify signature
if (!await VerifySignatureAsync(context, secret, body).ConfigureAwait(false))
catch (WebhookValidationException e)
{
Log.SignatureValidationFailed(logger);
context.Response.StatusCode = (int)e.StatusCode;
switch (e.Error)
{
case WebhookValidationError.IncorrectContentType:
Log.IncorrectContentType(logger);
break;

case WebhookValidationError.InvalidSignature:
case WebhookValidationError.MissingSignature:
case WebhookValidationError.UnexpectedSignature:
Log.SignatureValidationFailed(logger);
if (e.Error == WebhookValidationError.UnexpectedSignature)
{
// Add error response content to help with debugging webhook failure.
await context.Response.WriteAsync("Payload includes a secret, so the webhook receiver must configure a secret.")
.ConfigureAwait(false);
}

break;

default:
break;
}

return;
}

Expand All @@ -56,71 +112,6 @@ await service.ProcessWebhookAsync(context.Request.Headers, body)
}
});

private static bool VerifyContentType(HttpContext context, string expectedContentType)
{
if (context.Request.ContentType is null)
{
return false;
}

var contentType = new ContentType(context.Request.ContentType);
if (contentType.MediaType != expectedContentType)
{
context.Response.StatusCode = 400;
return false;
}

return true;
}

private static async Task<string> GetBodyAsync(HttpContext context)
{
using var reader = new StreamReader(context.Request.Body);
return await reader.ReadToEndAsync().ConfigureAwait(false);
}

private static async Task<bool> VerifySignatureAsync(HttpContext context, string secret, string body)
{
_ = context.Request.Headers.TryGetValue("X-Hub-Signature-256", out var signatureSha256);

var isSigned = signatureSha256.Count > 0;
var isSignatureExpected = !string.IsNullOrEmpty(secret);

if (!isSigned && !isSignatureExpected)
{
// Nothing to do.
return true;
}

if (!isSigned && isSignatureExpected)
{
context.Response.StatusCode = 400;
return false;
}

if (isSigned && !isSignatureExpected)
{
context.Response.StatusCode = 400;
await context.Response.WriteAsync("Payload includes a secret, so the webhook receiver must configure a secret.")
.ConfigureAwait(false);
return false;
}

var keyBytes = Encoding.UTF8.GetBytes(secret);
var bodyBytes = Encoding.UTF8.GetBytes(body);

var hash = HMACSHA256.HashData(keyBytes, bodyBytes);
var hashHex = Convert.ToHexString(hash);
var expectedHeader = $"sha256={hashHex.ToLower(CultureInfo.InvariantCulture)}";
if (signatureSha256.ToString() != expectedHeader)
{
context.Response.StatusCode = 400;
return false;
}

return true;
}

/// <summary>
/// Log messages for the class.
/// </summary>
Expand Down
116 changes: 51 additions & 65 deletions src/Octokit.Webhooks.AzureFunctions/GitHubWebhooksHttpFunction.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
namespace Octokit.Webhooks.AzureFunctions;

using System;
using System.Globalization;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Net;
using System.Net.Mime;
using System.Security.Cryptography;
using System.Text;
using System.Threading.Tasks;
using Microsoft.Azure.Functions.Worker;
using Microsoft.Azure.Functions.Worker.Http;
Expand All @@ -22,8 +19,28 @@ namespace Octokit.Webhooks.AzureFunctions;
public sealed partial class GitHubWebhooksHttpFunction
{
private readonly IOptions<GitHubWebhooksOptions> options;
private readonly Func<Task<IEnumerable<string>?>> secretsCallback;

public GitHubWebhooksHttpFunction(IOptions<GitHubWebhooksOptions> options) => this.options = options;
public GitHubWebhooksHttpFunction(IOptions<GitHubWebhooksOptions> options)
{
this.options = options;

if ((options.Value.Secret != null) && (options.Value.SecretsCallback != null))
{
throw new ArgumentException($"The '{nameof(options.Value.Secret)}' and '{nameof(options.Value.SecretsCallback)}' properties are mutually exclusive.");
}

if (options.Value.SecretsCallback != null)
{
this.secretsCallback = options.Value.SecretsCallback;
}
else
{
var secret = options.Value.Secret;
var secrets = (secret != null) ? new string[] { secret } : null;
this.secretsCallback = () => Task.FromResult<IEnumerable<string>?>(secrets);
}
}

[Function(nameof(MapGitHubWebhooksAsync))]
public async Task<HttpResponseData?> MapGitHubWebhooksAsync(
Expand All @@ -32,31 +49,44 @@ public sealed partial class GitHubWebhooksHttpFunction
{
var logger = ctx.GetLogger(nameof(GitHubWebhooksHttpFunction));

// Verify content type
if (!VerifyContentType(req, MediaTypeNames.Application.Json))
{
Log.IncorrectContentType(logger);
return req.CreateResponse(HttpStatusCode.BadRequest);
}
var secrets = await this.secretsCallback().ConfigureAwait(false);

// Get body
var body = await GetBodyAsync(req).ConfigureAwait(false);
var headers = req.Headers.ToDictionary(
kv => kv.Key,
kv => new StringValues(kv.Value.ToArray()),
StringComparer.OrdinalIgnoreCase);

// Verify signature
if (!VerifySignature(req, this.options.Value.Secret, body))
// Validate the request.
string body;
try
{
Log.SignatureValidationFailed(logger);
return req.CreateResponse(HttpStatusCode.BadRequest);
body = await WebhookEventProcessor.ValidateWebhookRequestAsync(headers, req.Body, secrets).ConfigureAwait(false);
}
catch (WebhookValidationException e)
{
switch (e.Error)
{
case WebhookValidationError.IncorrectContentType:
Log.IncorrectContentType(logger);
break;

case WebhookValidationError.InvalidSignature:
case WebhookValidationError.MissingSignature:
case WebhookValidationError.UnexpectedSignature:
Log.SignatureValidationFailed(logger);
break;

default:
break;
}

return req.CreateResponse(e.StatusCode);
}

// Process body
try
{
var service = ctx.InstanceServices.GetRequiredService<WebhookEventProcessor>();
var headers = req.Headers.ToDictionary(
kv => kv.Key,
kv => new StringValues(kv.Value.ToArray()),
StringComparer.OrdinalIgnoreCase);
await service.ProcessWebhookAsync(headers, body)
.ConfigureAwait(false);
return req.CreateResponse(HttpStatusCode.OK);
Expand All @@ -68,56 +98,12 @@ await service.ProcessWebhookAsync(headers, body)
}
}

private static bool VerifyContentType(HttpRequestData req, string expectedContentType)
{
var contentHeader = req.Headers.GetValues("Content-Type").FirstOrDefault();
if (contentHeader is null)
{
return false;
}

var contentType = new ContentType(contentHeader);
return contentType.MediaType == expectedContentType;
}

private static async Task<string> GetBodyAsync(HttpRequestData req)
{
using var reader = new StreamReader(req.Body);
return await reader.ReadToEndAsync().ConfigureAwait(false);
}

private static bool VerifySignature(HttpRequestData req, string? secret, string body)
{
var isSigned = req.Headers.TryGetValues("X-Hub-Signature-256", out var signatureHeader);
var signature = signatureHeader?.FirstOrDefault();

var isSignatureExpected = !string.IsNullOrEmpty(secret);

if (!isSigned && !isSignatureExpected)
{
// Nothing to do.
return true;
}

if (!isSigned && isSignatureExpected)
{
return false;
}

if (isSigned && !isSignatureExpected)
{
return false;
}

var keyBytes = Encoding.UTF8.GetBytes(secret!);
var bodyBytes = Encoding.UTF8.GetBytes(body);

var hash = HMACSHA256.HashData(keyBytes, bodyBytes);
var hashHex = Convert.ToHexString(hash);
var expectedHeader = $"sha256={hashHex.ToLower(CultureInfo.InvariantCulture)}";
return signature == expectedHeader;
}

/// <summary>
/// Log messages for the class.
/// </summary>
Expand Down
34 changes: 34 additions & 0 deletions src/Octokit.Webhooks.AzureFunctions/GitHubWebhooksOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,40 @@
namespace Octokit.Webhooks.AzureFunctions;

using System;
using System.Collections.Generic;
using System.Threading.Tasks;

public sealed record GitHubWebhooksOptions
{
/// <summary>
/// Gets or sets the secret to use to validate each webhook request's signature, or null if no
/// signature is to be expected.
/// </summary>
/// <remarks>
/// The property may not be set if <see cref="SecretsCallback"/> is set.
/// </remarks>
public string? Secret { get; set; }

/// <summary>
/// Gets or sets a callback delegate that will return the list of secrets to use to validate a
/// request's signature.
/// </summary>
/// <remarks>
/// <para>
/// The delegate will be invoked once for each request. If the delegate returns null, then no
/// signature will be expected. If the delegate returns a list of one or more secrets, then the
/// request's signature must match one of those secrets. If the delegate returns an empty list,
/// then the request's signature will be considered invalid.
/// </para>
/// <para>
/// Because the delegate is invoked separately for each request, the webhook secret can be
/// changed without modifying the route. Returning bpth the old and the new secrets for some
/// time will allow any webhook requests already in flight to validate against the old secret
/// while any new requests will validate against the new secret.
/// </para>
/// <para>
/// This property may not be set if <see cref="Secret"/> is set.
/// </para>
/// </remarks>
public Func<Task<IEnumerable<string>?>>? SecretsCallback { get; set; }
}