Skip to content

v12: Add HMAC image processing protection#14181

Merged
bergmania merged 47 commits intov12/devfrom
v12/feature/imagesharp3-hmacsecretkey
May 11, 2023
Merged

v12: Add HMAC image processing protection#14181
bergmania merged 47 commits intov12/devfrom
v12/feature/imagesharp3-hmacsecretkey

Conversation

@ronaldbarendse
Copy link
Copy Markdown
Contributor

@ronaldbarendse ronaldbarendse commented Apr 28, 2023

Prerequisites

  • I have added steps to test this contribution in the description below

Description

Continuing on PR #14180 that updates the ImageSharp dependency to v3 (and PR #12376 that's still a draft), this adds support for HMAC image processing protection using the RequestAuthorizationUtilities helper that's introduced in this major (but is potentially also coming to 2.1.0).

This feature prevents bad actors from manipulating processing parameters and DOS your site by requesting a large amount of slightly different image URLs that all need to process the image (a potentially CPU and memory expensive process). These processed images are then also saved to the ImageSharp cache (stored in umbraco\Data\TEMP\MediaCache by default), which doesn't have a built-in way to clean up old/infrequently used files and could therefore fill up your disk.

Testing

Because the media overview in the backoffice generates thumbnails, you can easily test this out by adding the following composer and adding some media files. You'll see the backoffice first request the generated image URL using a protected API endpoint (e.g. /umbraco/backoffice/umbracoapi/images/GetBigThumbnail?originalImagePath=%2Fmedia%2Finefcvn3%2F4730684907_8a7f8759cb_b.jpg), which redirects to the final URL that contains the hmac token (e.g. /media/inefcvn3/4730684907_8a7f8759cb_b.jpg?rmode=max&width=500&hmac=24f8c0e1b2df28d107e87df1a6252704fa6c1f7c16e15f872f856f5ecae4e7ea&v=1d8613e39d3c794).

using System.Security.Cryptography;
using Umbraco.Cms.Core.Composing;
using Umbraco.Cms.Core.Configuration.Models;

public class HMACSecretKeyComposer : IComposer
{
    public void Compose(IUmbracoBuilder builder)
        => builder.Services.Configure<ImagingSettings>(options =>
        {
            if (options.HMACSecretKey.Length == 0)
            {
                byte[] secret = new byte[64]; // Change to 128 when using HMACSHA384 or HMACSHA512
                RandomNumberGenerator.Create().GetBytes(secret);
                options.HMACSecretKey = secret;

                var logger = builder.BuilderLoggerFactory.CreateLogger<HMACSecretKeyComposer>();
                logger.LogInformation("Imaging settings is now using HMACSecretKey: {HMACSecretKey}", Convert.ToBase64String(secret));
            }
        });
}

Because this composer regenerates a random HMAC secret key on each boot, restarting the site will cause all existing URLs to return a 400 response code and all new URLs to contain a new valid token.

On a real-life site, you'd set a static HMAC secret key in the configuration. Because ASP.NET Core supports setting byte arrays using a Base64 string, you can set this value in your appsettings.json (we might want to automate this in the project template or installer screen). You can copy the value logged by the above HMACSecretKeyComposer and save this as:

{
  "Umbraco": {
    "CMS": {
      "Imaging": {
        "HMACSecretKey": "zJvn5pdgAJpGAVXWWsbECWfH7zQ8xM5Gg2hZ0y3esoVmu7M+YFv3L/wFvEd7j/Uaj+4J9atF/ONo6BDdTM9KxA=="
      }
    }
  }
}

Functional breaking changes

  • The cache buster key has changed from rnd to v and is now a hexadecimal value to shorten the generated URL: this won't impact the generated images, but the changed URL will require clients to download this image again (it will invalidate the browser/CDN cache);
  • Configuring the HMACSecretKey will result in all previously generated image crop URLs (without HMAC token) to return a 400 Bad Request response and requires all new image crop URLs to be valid (including any back-office requests), so be aware of this before enabling it on an existing site.

if (options.CacheBusterValue is not null && !string.IsNullOrWhiteSpace(options.CacheBusterValue))
if (options.CacheBusterValue is string cacheBusterValue && !string.IsNullOrEmpty(cacheBusterValue))
{
queryString.Add("v", cacheBusterValue);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cache buster value is now using the v parameter, since it represents the version (or 'just' a value, not a random value like rnd suggested) and makes the URL slightly shorter.

options.OnParseCommandsAsync = context =>
{
if (context.Commands.Count == 0)
if (context.Commands.Count == 0 || _imagingSettings.HMACSecretKey.Length > 0)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can safely skip validating the maximum width/height when using HMAC authentication (the default callback in V2 also removed the hardcoded width/height check).


var cacheBusterValue =
cacheBuster ? mediaItem.UpdateDate.ToFileTimeUtc().ToString(CultureInfo.InvariantCulture) : null;
cacheBuster ? mediaItem.UpdateDate.ToFileTimeUtc().ToString("x", CultureInfo.InvariantCulture) : null;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of using a decimal digit for the cache buster value (that always had a lot of trailing zeros, because the update date only has a resolution of 1 second, but the filetime value 100 nanoseconds), I've changed this to a hexadecimal value to make it a lot shorter.

@ronaldbarendse ronaldbarendse changed the base branch from v12/feature/imagesharp3 to v12/dev May 9, 2023 20:58
@ronaldbarendse ronaldbarendse requested a review from bergmania May 9, 2023 22:06
@bergmania bergmania merged commit 27ae8bd into v12/dev May 11, 2023
@bergmania bergmania deleted the v12/feature/imagesharp3-hmacsecretkey branch May 11, 2023 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants