From b06767d479b91a700905d7638bffd392311f6e93 Mon Sep 17 00:00:00 2001
From: d2dyno <53011783+d2dyno1@users.noreply.github.com>
Date: Mon, 3 Aug 2026 08:30:22 +0200
Subject: [PATCH 1/5] Backport HMAC-SHA verification for recycle bin items
---
.../DataModels/RecycleBinItemDataModel.cs | 120 +++++++++++++++++-
.../AbstractRecycleBinHelpers.Operational.cs | 6 +-
.../AbstractRecycleBinHelpers.Shared.cs | 15 ++-
.../NativeRecycleBinHelpers.Operational.cs | 16 ++-
.../RecycleBinService.cs | 2 +-
.../Storage/RecycleBinFolder.cs | 11 +-
.../FileSystemTests/BaseFileSystemTests.cs | 11 +-
.../FileSystemTests/RecycleBinTests.cs | 36 +++++-
8 files changed, 194 insertions(+), 23 deletions(-)
diff --git a/src/Core/SecureFolderFS.Core.FileSystem/DataModels/RecycleBinItemDataModel.cs b/src/Core/SecureFolderFS.Core.FileSystem/DataModels/RecycleBinItemDataModel.cs
index e7751300c..9d43ae210 100644
--- a/src/Core/SecureFolderFS.Core.FileSystem/DataModels/RecycleBinItemDataModel.cs
+++ b/src/Core/SecureFolderFS.Core.FileSystem/DataModels/RecycleBinItemDataModel.cs
@@ -1,5 +1,9 @@
using System;
+using System.Buffers.Binary;
using System.IO;
+using System.Runtime.CompilerServices;
+using System.Security.Cryptography;
+using System.Text;
using System.Text.Json.Serialization;
using SecureFolderFS.Core.Cryptography;
@@ -38,14 +42,122 @@ public sealed record class RecycleBinItemDataModel
[JsonPropertyName("size")]
public required long? Size { get; init; }
- // TODO: Add MAC key signing for tamper proofing
+ ///
+ /// Gets the HMAC-SHA256 tag binding this model's fields to the payload it describes.
+ ///
+ [JsonPropertyName("hmacsha256mac")]
+ public byte[]? PayloadMac { get; set; }
public string? DecryptName(Security security)
{
- if (security.NameCrypt is null)
- return Name;
+ var plaintextName = security.NameCrypt is null
+ ? Name
+ : security.NameCrypt.DecryptName(Path.GetFileNameWithoutExtension(Name), DirectoryId);
+
+ // The name is later joined onto a folder path to reattach or restore the payload, and a name
+ // that is not a single path component would land that payload wherever the attacker chose (e.g., autostart dir).
+ // AES-SIV already yields separator-free tokens, but CipherId.NONE returns the name verbatim.
+ // Checked independently of the MAC, because the vault's own code writes these names and a correctly signed one must not escape either
+ if (plaintextName is null || !IsSingleNameComponent(plaintextName))
+ return null;
+
+ return plaintextName;
+ }
+
+ ///
+ /// Returns a copy of this model carrying a over its fields and .
+ ///
+ /// The name of the payload in the recycle bin that this model describes.
+ /// The instance holding the vault's MAC key.
+ public RecycleBinItemDataModel WithMac(string itemName, Security security)
+ {
+ return this with { PayloadMac = ComputeMac(this, itemName, security) };
+ }
+
+ ///
+ /// Determines whether authenticates this model against .
+ ///
+ /// The name of the payload in the recycle bin that this model describes.
+ /// The instance holding the vault's MAC key.
+ ///
+ /// Every gate on the reattachment and restore paths - the Directory ID lineage match, the deletion
+ /// recency window, and the original parent path - reads its evidence from this model. Without the
+ /// tag those gates are authored by whoever can write into the vault's ciphertext directory.
+ ///
+ public bool VerifyMac(string itemName, Security security)
+ {
+ if (PayloadMac is not { Length: HMACSHA256.HashSizeInBytes })
+ return false;
+
+ return CryptographicOperations.FixedTimeEquals(ComputeMac(this, itemName, security), PayloadMac);
+ }
+
+ // [SkipLocalsInit] - deliberately not used here.
+ private static byte[] ComputeMac(RecycleBinItemDataModel dataModel, string itemName, Security security)
+ {
+ var mac = new byte[HMACSHA256.HashSizeInBytes];
+ security.KeyPair.MacKey.UseKey(macKey =>
+ {
+ using var hmac = IncrementalHash.CreateHMAC(HashAlgorithmName.SHA256, macKey);
+
+ // Every field is length-prefixed. Concatenating them raw would let bytes be shifted
+ // across the boundary between two adjacent attacker-influenced fields (Name + ParentId) to yield a different model with an identical MAC input.
+ // The payload's own name is bound too, so a valid configuration cannot be paired with a different payload file
+ AppendField(hmac, Encoding.UTF8.GetBytes(itemName), true);
+ AppendField(hmac, dataModel.Name is null ? default : Encoding.UTF8.GetBytes(dataModel.Name), dataModel.Name is not null);
+ AppendField(hmac, dataModel.ParentId is null ? default : Encoding.UTF8.GetBytes(dataModel.ParentId), dataModel.ParentId is not null);
+ AppendField(hmac, dataModel.DirectoryId, dataModel.DirectoryId is not null);
+
+ // Ticks and Kind are bound exactly as serialized, so no timezone conversion sits
+ // between signing and verification
+ Span timestamp = stackalloc byte[sizeof(long) + 1];
+ if (dataModel.DeletionTimestamp is { } deletionTimestamp)
+ {
+ BinaryPrimitives.WriteInt64LittleEndian(timestamp, deletionTimestamp.Ticks);
+ timestamp[sizeof(long)] = (byte)deletionTimestamp.Kind;
+ }
+
+ AppendField(hmac, timestamp, dataModel.DeletionTimestamp is not null);
+
+ Span size = stackalloc byte[sizeof(long)];
+ if (dataModel.Size is { } sizeValue)
+ BinaryPrimitives.WriteInt64LittleEndian(size, sizeValue);
+
+ AppendField(hmac, size, dataModel.Size is not null);
- return security.NameCrypt.DecryptName(Path.GetFileNameWithoutExtension(Name), DirectoryId);
+ _ = hmac.GetHashAndReset(mac);
+ });
+
+ return mac;
+
+ [SkipLocalsInit]
+ static void AppendField(IncrementalHash hmac, ReadOnlySpan value, bool isPresent)
+ {
+ // The presence flag keeps an absent field distinct from a present-but-empty one
+ Span header = stackalloc byte[sizeof(int) + 1];
+ header[0] = isPresent ? (byte)1 : (byte)0;
+ BinaryPrimitives.WriteInt32LittleEndian(header[1..], isPresent ? value.Length : 0);
+
+ hmac.AppendData(header);
+ if (isPresent)
+ hmac.AppendData(value);
+ }
+ }
+
+ ///
+ /// Determines whether is a single path component that cannot escape its parent.
+ ///
+ /// The name to check.
+ ///
+ /// Both separators are checked regardless of the running platform.
+ ///
+ private static bool IsSingleNameComponent(string name)
+ {
+ return !string.IsNullOrEmpty(name)
+ && name is not ("." or "..")
+ && name.IndexOf('/') < 0
+ && name.IndexOf('\\') < 0
+ && !Path.IsPathRooted(name);
}
public string? DecryptParentId(Security security)
diff --git a/src/Core/SecureFolderFS.Core.FileSystem/Helpers/RecycleBin/Abstract/AbstractRecycleBinHelpers.Operational.cs b/src/Core/SecureFolderFS.Core.FileSystem/Helpers/RecycleBin/Abstract/AbstractRecycleBinHelpers.Operational.cs
index 7d2314936..3f5f226da 100644
--- a/src/Core/SecureFolderFS.Core.FileSystem/Helpers/RecycleBin/Abstract/AbstractRecycleBinHelpers.Operational.cs
+++ b/src/Core/SecureFolderFS.Core.FileSystem/Helpers/RecycleBin/Abstract/AbstractRecycleBinHelpers.Operational.cs
@@ -193,7 +193,7 @@ public static async Task DeleteOrRecycleAsync(
// whereas a payload without a configuration file would be unrestorable.
var guid = Guid.NewGuid().ToString();
var configurationFile = await modifiableRecycleBin.CreateFileAsync($"{guid}.json", false, cancellationToken);
- await WriteItemDataModelAsync(configurationFile, dataModel, streamSerializer, cancellationToken);
+ await WriteItemDataModelAsync(configurationFile, dataModel, specifics.Security, streamSerializer, cancellationToken);
IStorableChild movedItem;
try
@@ -218,7 +218,7 @@ public static async Task DeleteOrRecycleAsync(
// The folded sizes were already part of the occupied total; only the
// folder's own entry needs to account for its regained contents
if (foldedSize > 0L)
- await WriteItemDataModelAsync(configurationFile, dataModel with { Size = sizeHint + foldedSize }, streamSerializer, cancellationToken);
+ await WriteItemDataModelAsync(configurationFile, dataModel with { Size = sizeHint + foldedSize }, specifics.Security, streamSerializer, cancellationToken);
}
// Update occupied size
@@ -287,7 +287,7 @@ internal static async Task FoldDescendantEntriesAsync(
// A single unreadable entry must not abandon the remaining ones
await SafetyHelpers.NoFailureAsync(async () =>
{
- var dataModel = await GetItemDataModelAsync(configurationFile, recycleBin, streamSerializer, cancellationToken);
+ var dataModel = await GetItemDataModelAsync(configurationFile, recycleBin, specifics.Security, streamSerializer, cancellationToken);
if (dataModel is not { Name: not null, ParentId: not null, DirectoryId: { Length: Constants.DIRECTORY_ID_SIZE } childDirectoryId })
return;
diff --git a/src/Core/SecureFolderFS.Core.FileSystem/Helpers/RecycleBin/Abstract/AbstractRecycleBinHelpers.Shared.cs b/src/Core/SecureFolderFS.Core.FileSystem/Helpers/RecycleBin/Abstract/AbstractRecycleBinHelpers.Shared.cs
index e83f5f2ed..32de42e00 100644
--- a/src/Core/SecureFolderFS.Core.FileSystem/Helpers/RecycleBin/Abstract/AbstractRecycleBinHelpers.Shared.cs
+++ b/src/Core/SecureFolderFS.Core.FileSystem/Helpers/RecycleBin/Abstract/AbstractRecycleBinHelpers.Shared.cs
@@ -4,6 +4,7 @@
using System.Threading;
using System.Threading.Tasks;
using OwlCore.Storage;
+using SecureFolderFS.Core.Cryptography;
using SecureFolderFS.Core.FileSystem.DataModels;
using SecureFolderFS.Core.FileSystem.Helpers.Paths;
using SecureFolderFS.Shared.ComponentModel;
@@ -182,22 +183,25 @@ private static async Task GetFolderPlaintextSizeAsync(IFolder ciphertextFo
///
/// Serializes into , truncating any previous content.
///
- internal static Task WriteItemDataModelAsync(IFile configurationFile, RecycleBinItemDataModel dataModel, IAsyncSerializer streamSerializer, CancellationToken cancellationToken)
+ internal static Task WriteItemDataModelAsync(IFile configurationFile, RecycleBinItemDataModel dataModel, Security security, IAsyncSerializer streamSerializer, CancellationToken cancellationToken)
{
+ // Sign on the way out so every write (including the size rewrite after folding) is covered
+ var signedDataModel = dataModel.WithMac(Path.GetFileNameWithoutExtension(configurationFile.Name), security);
+
return WithTransientIoRetryAsync