From f8b6df6961e7bc49e33ffc15107db09a0a881c14 Mon Sep 17 00:00:00 2001 From: Korolev Dmitry Date: Wed, 10 Dec 2025 11:56:59 +0100 Subject: [PATCH 01/13] some impl of deserialize? --- .../DefaultAntiforgeryTokenSerializer.cs | 119 +++++++--- .../Microsoft.AspNetCore.Antiforgery.csproj | 6 +- .../DefaultAntiforgeryTokenSerializerTest.cs | 73 +++--- .../Internal/Int7BitEncodingUtilsTests.cs | 31 --- src/Shared/Encoding/Int7BitEncodingUtils.cs | 62 +++++ src/Shared/WebEncoders/WebEncoders.cs | 32 +++ .../Encoding/Int7BitEncodingUtilsTests.cs | 221 ++++++++++++++++++ .../Microsoft.AspNetCore.Shared.Tests.csproj | 7 +- 8 files changed, 455 insertions(+), 96 deletions(-) delete mode 100644 src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.Tests/Internal/Int7BitEncodingUtilsTests.cs create mode 100644 src/Shared/test/Shared.Tests/Encoding/Int7BitEncodingUtilsTests.cs diff --git a/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenSerializer.cs b/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenSerializer.cs index 0cc346b0de42..bcba7e3aff1f 100644 --- a/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenSerializer.cs +++ b/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenSerializer.cs @@ -1,7 +1,10 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Buffers; +using System.Buffers.Text; using Microsoft.AspNetCore.DataProtection; +using Microsoft.AspNetCore.Shared; using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.ObjectPool; @@ -12,7 +15,9 @@ internal sealed class DefaultAntiforgeryTokenSerializer : IAntiforgeryTokenSeria private const string Purpose = "Microsoft.AspNetCore.Antiforgery.AntiforgeryToken.v1"; private const byte TokenVersion = 0x01; - private readonly IDataProtector _cryptoSystem; + private readonly IDataProtector _defaultCryptoSystem; + private readonly ISpanDataProtector? _perfCryptoSystem; + private readonly ObjectPool _pool; public DefaultAntiforgeryTokenSerializer( @@ -22,34 +27,36 @@ public DefaultAntiforgeryTokenSerializer( ArgumentNullException.ThrowIfNull(provider); ArgumentNullException.ThrowIfNull(pool); - _cryptoSystem = provider.CreateProtector(Purpose); _pool = pool; + + _defaultCryptoSystem = provider.CreateProtector(Purpose); + _perfCryptoSystem = _defaultCryptoSystem as ISpanDataProtector; } public AntiforgeryToken Deserialize(string serializedToken) { - var serializationContext = _pool.Get(); - + byte[]? tokenBytesRent = null; Exception? innerException = null; try { - var count = serializedToken.Length; - var charsRequired = WebEncoders.GetArraySizeRequiredToDecode(count); - var chars = serializationContext.GetChars(charsRequired); - var tokenBytes = WebEncoders.Base64UrlDecode( - serializedToken, - offset: 0, - buffer: chars, - bufferOffset: 0, - count: count); + var tokenDecodedSize = Base64Url.GetMaxDecodedLength(serializedToken.Length); - var unprotectedBytes = _cryptoSystem.Unprotect(tokenBytes); - var stream = serializationContext.Stream; - stream.Write(unprotectedBytes, offset: 0, count: unprotectedBytes.Length); - stream.Position = 0L; + var rent = tokenDecodedSize < 256 + ? stackalloc byte[255] + : (tokenBytesRent = ArrayPool.Shared.Rent(tokenDecodedSize)); + var tokenBytes = rent[..tokenDecodedSize]; + + var status = Base64Url.DecodeFromChars(serializedToken, tokenBytes, out int charsConsumed, out int bytesWritten); + if (status is not OperationStatus.Done) + { + throw new FormatException("Failed to decode token as Base64 char sequence."); + } + + var tokenBytesDecoded = tokenBytes.Slice(0, bytesWritten); + var protectBuffer = new RefPooledArrayBufferWriter(stackalloc byte[255]); + _perfCryptoSystem!.Unprotect(tokenBytesDecoded, ref protectBuffer); - var reader = serializationContext.Reader; - var token = Deserialize(reader); + var token = Deserialize(protectBuffer.WrittenSpan); if (token != null) { return token; @@ -62,7 +69,10 @@ public AntiforgeryToken Deserialize(string serializedToken) } finally { - _pool.Return(serializationContext); + if (tokenBytesRent is not null) + { + ArrayPool.Shared.Return(tokenBytesRent); + } } // if we reached this point, something went wrong deserializing @@ -81,39 +91,80 @@ public AntiforgeryToken Deserialize(string serializedToken) * | `- Username: UTF-8 string with 7-bit integer length prefix * `- AdditionalData: UTF-8 string with 7-bit integer length prefix */ - private static AntiforgeryToken? Deserialize(BinaryReader reader) + private static AntiforgeryToken? Deserialize(ReadOnlySpan tokenBytes) { + var offset = 0; + // we can only consume tokens of the same serialized version that we generate - var embeddedVersion = reader.ReadByte(); + if (tokenBytes.Length < 1) + { + return null; + } + + var embeddedVersion = tokenBytes[offset++]; if (embeddedVersion != TokenVersion) { return null; } var deserializedToken = new AntiforgeryToken(); - var securityTokenBytes = reader.ReadBytes(AntiforgeryToken.SecurityTokenBitLength / 8); - deserializedToken.SecurityToken = - new BinaryBlob(AntiforgeryToken.SecurityTokenBitLength, securityTokenBytes); - deserializedToken.IsCookieToken = reader.ReadBoolean(); + + // Read SecurityToken (16 bytes) + const int securityTokenByteLength = AntiforgeryToken.SecurityTokenBitLength / 8; + if (tokenBytes.Length < offset + securityTokenByteLength) + { + return null; + } + + deserializedToken.SecurityToken = new BinaryBlob( + AntiforgeryToken.SecurityTokenBitLength, + tokenBytes.Slice(offset, securityTokenByteLength).ToArray()); + offset += securityTokenByteLength; + + // Read IsCookieToken (1 byte) + if (tokenBytes.Length < offset + 1) + { + return null; + } + + deserializedToken.IsCookieToken = tokenBytes[offset++] != 0; if (!deserializedToken.IsCookieToken) { - var isClaimsBased = reader.ReadBoolean(); + // Read IsClaimsBased (1 byte) + if (tokenBytes.Length < offset + 1) + { + return null; + } + + var isClaimsBased = tokenBytes[offset++] != 0; if (isClaimsBased) { - var claimUidBytes = reader.ReadBytes(AntiforgeryToken.ClaimUidBitLength / 8); - deserializedToken.ClaimUid = new BinaryBlob(AntiforgeryToken.ClaimUidBitLength, claimUidBytes); + // Read ClaimUid (32 bytes) + const int claimUidByteLength = AntiforgeryToken.ClaimUidBitLength / 8; + if (tokenBytes.Length < offset + claimUidByteLength) + { + return null; + } + + deserializedToken.ClaimUid = new BinaryBlob( + AntiforgeryToken.ClaimUidBitLength, + tokenBytes.Slice(offset, claimUidByteLength).ToArray()); + offset += claimUidByteLength; } else { - deserializedToken.Username = reader.ReadString(); + // Read Username (7-bit encoded length prefix + UTF-8 string) + offset += tokenBytes.Slice(offset).Read7BitEncodedString(out var username); + deserializedToken.Username = username; } - deserializedToken.AdditionalData = reader.ReadString(); + offset += tokenBytes.Slice(offset).Read7BitEncodedString(out var additionalData); + deserializedToken.AdditionalData = additionalData; } - // if there's still unconsumed data in the stream, fail - if (reader.BaseStream.ReadByte() != -1) + // if there's still unconsumed data in the span, fail + if (offset != tokenBytes.Length) { return null; } @@ -153,7 +204,7 @@ public string Serialize(AntiforgeryToken token) writer.Flush(); var stream = serializationContext.Stream; - var bytes = _cryptoSystem.Protect(stream.ToArray()); + var bytes = _defaultCryptoSystem.Protect(stream.ToArray()); var count = bytes.Length; var charsRequired = WebEncoders.GetArraySizeRequiredToEncode(count); diff --git a/src/Antiforgery/src/Microsoft.AspNetCore.Antiforgery.csproj b/src/Antiforgery/src/Microsoft.AspNetCore.Antiforgery.csproj index a70a9e37c045..db92d8f239cc 100644 --- a/src/Antiforgery/src/Microsoft.AspNetCore.Antiforgery.csproj +++ b/src/Antiforgery/src/Microsoft.AspNetCore.Antiforgery.csproj @@ -1,4 +1,4 @@ - + An antiforgery system for ASP.NET Core designed to generate and validate tokens to prevent Cross-Site Request Forgery attacks. @@ -25,6 +25,8 @@ - + + + diff --git a/src/Antiforgery/test/DefaultAntiforgeryTokenSerializerTest.cs b/src/Antiforgery/test/DefaultAntiforgeryTokenSerializerTest.cs index ac1a32675186..3ba404114acf 100644 --- a/src/Antiforgery/test/DefaultAntiforgeryTokenSerializerTest.cs +++ b/src/Antiforgery/test/DefaultAntiforgeryTokenSerializerTest.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System; +using System.Buffers; using Microsoft.AspNetCore.DataProtection; using Microsoft.Extensions.ObjectPool; using Moq; @@ -138,39 +140,15 @@ public void Serialize_CookieToken_TokenRoundTripSuccessful() private static Mock GetDataProtector() { - var mockCryptoSystem = new Mock(); - mockCryptoSystem.Setup(o => o.Protect(It.IsAny())) - .Returns(Protect) - .Verifiable(); - mockCryptoSystem.Setup(o => o.Unprotect(It.IsAny())) - .Returns(UnProtect) - .Verifiable(); + var testSpanDataProtector = new TestSpanDataProtector(); var provider = new Mock(); provider .Setup(p => p.CreateProtector(It.IsAny())) - .Returns(mockCryptoSystem.Object); + .Returns(testSpanDataProtector); return provider; } - private static byte[] Protect(byte[] data) - { - var input = new List(data); - input.Add(_salt); - return input.ToArray(); - } - - private static byte[] UnProtect(byte[] data) - { - var salt = data[data.Length - 1]; - if (salt != _salt) - { - throw new ArgumentException("Invalid salt value in data"); - } - - return data.Take(data.Length - 1).ToArray(); - } - private static void AssertTokensEqual(AntiforgeryToken expected, AntiforgeryToken actual) { Assert.NotNull(expected); @@ -181,4 +159,47 @@ private static void AssertTokensEqual(AntiforgeryToken expected, AntiforgeryToke Assert.Equal(expected.SecurityToken, actual.SecurityToken); Assert.Equal(expected.Username, actual.Username); } + + private sealed class TestSpanDataProtector : ISpanDataProtector + { + public IDataProtector CreateProtector(string purpose) => this; + + public void Protect(ReadOnlySpan plaintext, ref TWriter destination) where TWriter : IBufferWriter, allows ref struct + { + var result = ProtectImpl(plaintext.ToArray()); + var destinationSpan = destination.GetSpan(result.Length); + result.CopyTo(destinationSpan); + destination.Advance(result.Length); + } + + public void Unprotect(ReadOnlySpan protectedData, ref TWriter destination) + where TWriter : IBufferWriter, allows ref struct + { + var result = UnprotectImpl(protectedData.ToArray()); + var destinationSpan = destination.GetSpan(result.Length); + result.CopyTo(destinationSpan); + destination.Advance(result.Length); + } + + public byte[] Protect(byte[] plaintext) => ProtectImpl(plaintext); + + public byte[] Unprotect(byte[] protectedData) => UnprotectImpl(protectedData); + + private static byte[] ProtectImpl(byte[] data) + { + var input = new List(data); + input.Add(_salt); + return input.ToArray(); + } + + private static byte[] UnprotectImpl(byte[] data) + { + var salt = data[data.Length - 1]; + if (salt != _salt) + { + throw new ArgumentException("Invalid salt value in data"); + } + return data.Take(data.Length - 1).ToArray(); + } + } } diff --git a/src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.Tests/Internal/Int7BitEncodingUtilsTests.cs b/src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.Tests/Internal/Int7BitEncodingUtilsTests.cs deleted file mode 100644 index f4ba8ca0db9e..000000000000 --- a/src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.Tests/Internal/Int7BitEncodingUtilsTests.cs +++ /dev/null @@ -1,31 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; -using System.Collections.Generic; -using System.Text; -using Microsoft.AspNetCore.DataProtection.Internal; -using Microsoft.AspNetCore.Shared; - -namespace Microsoft.AspNetCore.DataProtection.Tests.Internal; - -public class Int7BitEncodingUtilsTests -{ - [Theory] - [InlineData(0, 1)] - [InlineData(1, 1)] - [InlineData(0b0_1111111, 1)] - [InlineData(0b1_0000000, 2)] - [InlineData(0b1111111_1111111, 2)] - [InlineData(0b1_0000000_0000000, 3)] - [InlineData(0b1111111_1111111_1111111, 3)] - [InlineData(0b1_0000000_0000000_0000000, 4)] - [InlineData(0b1111111_1111111_1111111_1111111, 4)] - [InlineData(0b1_0000000_0000000_0000000_0000000, 5)] - [InlineData(uint.MaxValue, 5)] - public void Measure7BitEncodedUIntLength_ReturnsExceptedLength(uint value, int expectedSize) - { - var actualSize = value.Measure7BitEncodedUIntLength(); - Assert.Equal(expectedSize, actualSize); - } -} diff --git a/src/Shared/Encoding/Int7BitEncodingUtils.cs b/src/Shared/Encoding/Int7BitEncodingUtils.cs index ca97096d1c17..2f012e541d88 100644 --- a/src/Shared/Encoding/Int7BitEncodingUtils.cs +++ b/src/Shared/Encoding/Int7BitEncodingUtils.cs @@ -49,4 +49,66 @@ public static int Write7BitEncodedInt(this Span target, uint uValue) target[index++] = (byte)uValue; return index; } + + /// + /// Reads a 7-bit encoded unsigned integer from the source span. + /// + /// The source span to read from. + /// The decoded value. + /// The number of bytes consumed from the source span. + /// Thrown when the encoded value is malformed or exceeds 32 bits. + public static int Read7BitEncodedInt(this ReadOnlySpan source, out int value) + { + // Read out an int 7 bits at a time. The high bit of the byte, + // when on, indicates more bytes to read. + // A 32-bit unsigned integer can be encoded in at most 5 bytes. + + value = 0; + var shift = 0; + var index = 0; + + byte b; + do + { + // Check if we've exceeded the maximum number of bytes for a 32-bit integer + // or if we've run out of data. + if (shift == 35 || index >= source.Length) + { + throw new FormatException("Bad 7-bit encoded integer."); + } + + b = source[index++]; + value |= (b & 0x7F) << shift; + shift += 7; + } + while ((b & 0x80) != 0); + + return index; + } + + /// + /// Returns consumed length + /// + /// + /// + /// + /// + internal static int Read7BitEncodedString(this ReadOnlySpan bytes, out string value) + { + value = string.Empty; + var consumed = Read7BitEncodedInt(bytes, out var length); + if (length == 0) + { + return consumed; + } + + if (bytes.Length < length) + { + throw new FormatException("Bad 7-bit encoded string."); + } + + value = System.Text.Encoding.UTF8.GetString(bytes.Slice(consumed, length)); + consumed += length; + return consumed; + } } diff --git a/src/Shared/WebEncoders/WebEncoders.cs b/src/Shared/WebEncoders/WebEncoders.cs index 8d92baa94c51..20518a00ce23 100644 --- a/src/Shared/WebEncoders/WebEncoders.cs +++ b/src/Shared/WebEncoders/WebEncoders.cs @@ -204,6 +204,38 @@ public static byte[] Base64UrlDecode(string input, int offset, char[] buffer, in return Convert.FromBase64CharArray(buffer, bufferOffset, arraySizeRequired); } +#if NET9_0_OR_GREATER + /// + /// Decodes a base64url-encoded into . + /// + /// The base64url-encoded input to decode. + /// The buffer to receive the decoded bytes. + /// The number of bytes written to . + /// + /// The input must not contain any whitespace or padding characters. + /// Throws if the input is malformed. + /// + internal static int Base64UrlDecode(string input, Span output) + { + if (string.IsNullOrEmpty(input)) + { + return 0; + } + + var status = Base64Url.DecodeFromChars(input, output, out _, out int bytesWritten); + if (status != OperationStatus.Done) + { + throw new FormatException( + string.Format( + CultureInfo.CurrentCulture, + EncoderResources.WebEncoders_MalformedInput, + input.Length)); + } + + return bytesWritten; + } +#endif + /// /// Gets the minimum char[] size required for decoding of characters /// with the method. diff --git a/src/Shared/test/Shared.Tests/Encoding/Int7BitEncodingUtilsTests.cs b/src/Shared/test/Shared.Tests/Encoding/Int7BitEncodingUtilsTests.cs new file mode 100644 index 000000000000..04369e4b5b43 --- /dev/null +++ b/src/Shared/test/Shared.Tests/Encoding/Int7BitEncodingUtilsTests.cs @@ -0,0 +1,221 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Text; +using Microsoft.AspNetCore.Shared; + +namespace Microsoft.AspNetCore.Shared.Tests.Encoding; + +public class Int7BitEncodingUtilsTests +{ + [Theory] + [InlineData(0, 1)] + [InlineData(1, 1)] + [InlineData(0b0_1111111, 1)] + [InlineData(0b1_0000000, 2)] + [InlineData(0b1111111_1111111, 2)] + [InlineData(0b1_0000000_0000000, 3)] + [InlineData(0b1111111_1111111_1111111, 3)] + [InlineData(0b1_0000000_0000000_0000000, 4)] + [InlineData(0b1111111_1111111_1111111_1111111, 4)] + [InlineData(0b1_0000000_0000000_0000000_0000000, 5)] + [InlineData(uint.MaxValue, 5)] + public void Measure7BitEncodedUIntLength_ReturnsExceptedLength(uint value, int expectedSize) + { + var actualSize = value.Measure7BitEncodedUIntLength(); + Assert.Equal(expectedSize, actualSize); + } + + [Theory] + [InlineData(0, new byte[] { 0x00 })] + [InlineData(1, new byte[] { 0x01 })] + [InlineData(127, new byte[] { 0x7F })] + [InlineData(128, new byte[] { 0x80, 0x01 })] + [InlineData(255, new byte[] { 0xFF, 0x01 })] + [InlineData(256, new byte[] { 0x80, 0x02 })] + [InlineData(16383, new byte[] { 0xFF, 0x7F })] + [InlineData(16384, new byte[] { 0x80, 0x80, 0x01 })] + [InlineData(2097151, new byte[] { 0xFF, 0xFF, 0x7F })] + [InlineData(2097152, new byte[] { 0x80, 0x80, 0x80, 0x01 })] + [InlineData(268435455, new byte[] { 0xFF, 0xFF, 0xFF, 0x7F })] + [InlineData(268435456, new byte[] { 0x80, 0x80, 0x80, 0x80, 0x01 })] + [InlineData(int.MaxValue, new byte[] { 0xFF, 0xFF, 0xFF, 0xFF, 0x07 })] + public void Read7BitEncodedInt_DecodesCorrectly(int expected, byte[] encoded) + { + ReadOnlySpan source = encoded; + + var bytesConsumed = source.Read7BitEncodedInt(out var value); + + Assert.Equal(expected, value); + Assert.Equal(encoded.Length, bytesConsumed); + } + + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(127)] + [InlineData(128)] + [InlineData(255)] + [InlineData(16383)] + [InlineData(16384)] + [InlineData(2097151)] + [InlineData(2097152)] + [InlineData(268435455)] + [InlineData(268435456)] + [InlineData(int.MaxValue)] + public void Read7BitEncodedInt_RoundTripsWithWrite(int value) + { + Span buffer = stackalloc byte[5]; + + var bytesWritten = buffer.Write7BitEncodedInt(value); + + ReadOnlySpan source = buffer.Slice(0, bytesWritten); + var bytesConsumed = source.Read7BitEncodedInt(out var decoded); + + Assert.Equal(value, decoded); + Assert.Equal(bytesWritten, bytesConsumed); + } + + [Fact] + public void Read7BitEncodedInt_WithEmptySpan_ThrowsFormatException() + { + Assert.Throws(() => + { + var source = ReadOnlySpan.Empty; + return source.Read7BitEncodedInt(out _); + }); + } + + [Fact] + public void Read7BitEncodedInt_WithTruncatedData_ThrowsFormatException() + { + Assert.Throws(() => + { + // This represents the start of a multi-byte encoded value but is incomplete + // 0x80 has continuation bit set, meaning more bytes should follow + ReadOnlySpan source = [0x80]; + + return source.Read7BitEncodedInt(out _); + }); + } + + [Fact] + public void Read7BitEncodedInt_WithOverflow_ThrowsFormatException() + { + Assert.Throws(() => + { + // 6 bytes with continuation bits set would overflow a 32-bit integer + ReadOnlySpan source = [0x80, 0x80, 0x80, 0x80, 0x80, 0x01]; + + return source.Read7BitEncodedInt(out _); + }); + } + + [Fact] + public void Read7BitEncodedInt_WithExtraDataAfterValue_ConsumesOnlyNeededBytes() + { + // Value 127 followed by extra bytes + ReadOnlySpan source = [0x7F, 0xFF, 0xFF]; + + var bytesConsumed = source.Read7BitEncodedInt(out var value); + + Assert.Equal(127, value); + Assert.Equal(1, bytesConsumed); + } + + [Theory] + [InlineData("")] + [InlineData("Hello")] + [InlineData("Hello, World!")] + [InlineData("UTF-8: \u00e9\u00e8\u00ea")] + public void Read7BitEncodedString_DecodesCorrectly(string expected) + { + var stringBytes = System.Text.Encoding.UTF8.GetBytes(expected); + var lengthBytes = new byte[5]; + Span lengthSpan = lengthBytes; + var lengthSize = lengthSpan.Write7BitEncodedInt(stringBytes.Length); + + var encodedBytes = new byte[lengthSize + stringBytes.Length]; + Array.Copy(lengthBytes, 0, encodedBytes, 0, lengthSize); + Array.Copy(stringBytes, 0, encodedBytes, lengthSize, stringBytes.Length); + + ReadOnlySpan source = encodedBytes; + + var bytesConsumed = source.Read7BitEncodedString(out var value); + + Assert.Equal(expected, value); + Assert.Equal(encodedBytes.Length, bytesConsumed); + } + + [Fact] + public void Read7BitEncodedString_WithEmptyString_ReturnsEmptyAndConsumesLengthByte() + { + // Length of 0 + ReadOnlySpan source = new byte[] { 0x00 }; + + var bytesConsumed = source.Read7BitEncodedString(out var value); + + Assert.Equal(string.Empty, value); + Assert.Equal(1, bytesConsumed); + } + + [Fact] + public void Read7BitEncodedString_WithTruncatedStringData_ThrowsFormatException() + { + Assert.Throws(() => + { + // Length says 10 bytes, but only 3 bytes of data follow + ReadOnlySpan source = [0x0A, 0x41, 0x42, 0x43]; + + return source.Read7BitEncodedString(out _); + }); + } + + [Fact] + public void Read7BitEncodedString_WithTruncatedLengthPrefix_ThrowsFormatException() + { + Assert.Throws(() => + { + // Continuation bit set but no more data + ReadOnlySpan source = [0x80]; + + return source.Read7BitEncodedString(out _); + }); + } + + [Fact] + public void Read7BitEncodedString_WithExtraDataAfterString_ConsumesOnlyNeededBytes() + { + // "Hi" (length 2) followed by extra bytes + ReadOnlySpan source = new byte[] { 0x02, 0x48, 0x69, 0xFF, 0xFF }; + + var bytesConsumed = source.Read7BitEncodedString(out var value); + + Assert.Equal("Hi", value); + Assert.Equal(3, bytesConsumed); // 1 byte length + 2 bytes string + } + + [Fact] + public void Read7BitEncodedString_WithMultiByteLengthPrefix_DecodesCorrectly() + { + // Create a string that requires a multi-byte length prefix (> 127 bytes) + var longString = new string('A', 200); + var stringBytes = System.Text.Encoding.UTF8.GetBytes(longString); + var lengthBytes = new byte[5]; + Span lengthSpan = lengthBytes; + var lengthSize = lengthSpan.Write7BitEncodedInt(stringBytes.Length); + + Assert.True(lengthSize > 1); // Verify we're testing multi-byte length + + var encodedBytes = new byte[lengthSize + stringBytes.Length]; + Array.Copy(lengthBytes, 0, encodedBytes, 0, lengthSize); + Array.Copy(stringBytes, 0, encodedBytes, lengthSize, stringBytes.Length); + + ReadOnlySpan source = encodedBytes; + + var bytesConsumed = source.Read7BitEncodedString(out var value); + + Assert.Equal(longString, value); + Assert.Equal(encodedBytes.Length, bytesConsumed); + } +} diff --git a/src/Shared/test/Shared.Tests/Microsoft.AspNetCore.Shared.Tests.csproj b/src/Shared/test/Shared.Tests/Microsoft.AspNetCore.Shared.Tests.csproj index e017c1501705..ea1335a8167a 100644 --- a/src/Shared/test/Shared.Tests/Microsoft.AspNetCore.Shared.Tests.csproj +++ b/src/Shared/test/Shared.Tests/Microsoft.AspNetCore.Shared.Tests.csproj @@ -1,4 +1,4 @@ - + $(DefaultNetCoreTargetFramework) @@ -43,8 +43,9 @@ - - + + + From 12790ceaff3890643bedddf011c7be23cac9f342 Mon Sep 17 00:00:00 2001 From: Korolev Dmitry Date: Wed, 10 Dec 2025 15:41:50 +0100 Subject: [PATCH 02/13] serialize! --- .../DefaultAntiforgeryTokenSerializer.cs | 105 ++++++++++------- .../DefaultAntiforgeryTokenSerializerTest.cs | 10 +- src/Shared/Encoding/Int7BitEncodingUtils.cs | 37 ++++++ .../Encoding/Int7BitEncodingUtilsTests.cs | 108 ++++++++++++++++++ 4 files changed, 214 insertions(+), 46 deletions(-) diff --git a/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenSerializer.cs b/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenSerializer.cs index bcba7e3aff1f..5e9c9c59a00e 100644 --- a/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenSerializer.cs +++ b/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenSerializer.cs @@ -5,8 +5,6 @@ using System.Buffers.Text; using Microsoft.AspNetCore.DataProtection; using Microsoft.AspNetCore.Shared; -using Microsoft.AspNetCore.WebUtilities; -using Microsoft.Extensions.ObjectPool; namespace Microsoft.AspNetCore.Antiforgery; @@ -18,16 +16,9 @@ internal sealed class DefaultAntiforgeryTokenSerializer : IAntiforgeryTokenSeria private readonly IDataProtector _defaultCryptoSystem; private readonly ISpanDataProtector? _perfCryptoSystem; - private readonly ObjectPool _pool; - - public DefaultAntiforgeryTokenSerializer( - IDataProtectionProvider provider, - ObjectPool pool) + public DefaultAntiforgeryTokenSerializer(IDataProtectionProvider provider) { ArgumentNullException.ThrowIfNull(provider); - ArgumentNullException.ThrowIfNull(pool); - - _pool = pool; _defaultCryptoSystem = provider.CreateProtector(Purpose); _perfCryptoSystem = _defaultCryptoSystem as ISpanDataProtector; @@ -53,13 +44,21 @@ public AntiforgeryToken Deserialize(string serializedToken) } var tokenBytesDecoded = tokenBytes.Slice(0, bytesWritten); + var protectBuffer = new RefPooledArrayBufferWriter(stackalloc byte[255]); - _perfCryptoSystem!.Unprotect(tokenBytesDecoded, ref protectBuffer); + try + { + _perfCryptoSystem!.Unprotect(tokenBytesDecoded, ref protectBuffer); - var token = Deserialize(protectBuffer.WrittenSpan); - if (token != null) + var token = Deserialize(protectBuffer.WrittenSpan); + if (token != null) + { + return token; + } + } + finally { - return token; + protectBuffer.Dispose(); } } catch (Exception ex) @@ -177,50 +176,74 @@ public string Serialize(AntiforgeryToken token) { ArgumentNullException.ThrowIfNull(token); - var serializationContext = _pool.Get(); + var securityTokenBytes = token.SecurityToken!.GetData(); + var claimUidBytes = token.ClaimUid?.GetData(); + + var totalSize = + 1 // TokenVersion + + securityTokenBytes.Length + // SecurityToken + + 1; // IsCookieToken + if (!token.IsCookieToken) + { + totalSize += 1; // isClaimsBased + + if (token.ClaimUid is not null) + { + totalSize += claimUidBytes!.Length; + } + else + { + var usernameByteCount = System.Text.Encoding.UTF8.GetByteCount(token.Username!); + totalSize += usernameByteCount.Measure7BitEncodedUIntLength() + usernameByteCount; + } + + var additionalDataByteCount = System.Text.Encoding.UTF8.GetByteCount(token.AdditionalData); + totalSize += additionalDataByteCount.Measure7BitEncodedUIntLength() + additionalDataByteCount; + } + + byte[]? tokenBytesRent = null; + var protectBuffer = new RefPooledArrayBufferWriter(stackalloc byte[255]); + + var rent = totalSize < 256 + ? stackalloc byte[255] + : (tokenBytesRent = ArrayPool.Shared.Rent(totalSize)); + var tokenBytes = rent[..totalSize]; try { - var writer = serializationContext.Writer; - writer.Write(TokenVersion); - writer.Write(token.SecurityToken!.GetData()); - writer.Write(token.IsCookieToken); + var offset = 0; + tokenBytes[offset++] = TokenVersion; + securityTokenBytes.CopyTo(tokenBytes.Slice(offset, securityTokenBytes.Length)); + offset += securityTokenBytes.Length; + tokenBytes[offset++] = token.IsCookieToken ? (byte)1 : (byte)0; if (!token.IsCookieToken) { if (token.ClaimUid != null) { - writer.Write(true /* isClaimsBased */); - writer.Write(token.ClaimUid.GetData()); + tokenBytes[offset++] = 1; // isClaimsBased + claimUidBytes!.CopyTo(tokenBytes.Slice(offset, claimUidBytes!.Length)); + offset += claimUidBytes.Length; } else { - writer.Write(false /* isClaimsBased */); - writer.Write(token.Username!); + tokenBytes[offset++] = 0; // isClaimsBased + offset += tokenBytes.Slice(offset).Write7BitEncodedString(token.Username!); } - - writer.Write(token.AdditionalData); + offset += tokenBytes.Slice(offset).Write7BitEncodedString(token.AdditionalData); } - writer.Flush(); - var stream = serializationContext.Stream; - var bytes = _defaultCryptoSystem.Protect(stream.ToArray()); - - var count = bytes.Length; - var charsRequired = WebEncoders.GetArraySizeRequiredToEncode(count); - var chars = serializationContext.GetChars(charsRequired); - var outputLength = WebEncoders.Base64UrlEncode( - bytes, - offset: 0, - output: chars, - outputOffset: 0, - count: count); - - return new string(chars, startIndex: 0, length: outputLength); + _perfCryptoSystem!.Protect(tokenBytes, ref protectBuffer); + return Base64Url.EncodeToString(protectBuffer.WrittenSpan); } finally { - _pool.Return(serializationContext); + if (tokenBytesRent is not null) + { + ArrayPool.Shared.Return(tokenBytesRent); + } + + protectBuffer.Dispose(); } } } diff --git a/src/Antiforgery/test/DefaultAntiforgeryTokenSerializerTest.cs b/src/Antiforgery/test/DefaultAntiforgeryTokenSerializerTest.cs index 3ba404114acf..aac8522a27ef 100644 --- a/src/Antiforgery/test/DefaultAntiforgeryTokenSerializerTest.cs +++ b/src/Antiforgery/test/DefaultAntiforgeryTokenSerializerTest.cs @@ -46,7 +46,7 @@ public class DefaultAntiforgeryTokenSerializerTest public void Deserialize_BadToken_Throws(string serializedToken) { // Arrange - var testSerializer = new DefaultAntiforgeryTokenSerializer(_dataProtector.Object, _pool); + var testSerializer = new DefaultAntiforgeryTokenSerializer(_dataProtector.Object); // Act & assert var ex = Assert.Throws(() => testSerializer.Deserialize(serializedToken)); @@ -57,7 +57,7 @@ public void Deserialize_BadToken_Throws(string serializedToken) public void Serialize_FieldToken_WithClaimUid_TokenRoundTripSuccessful() { // Arrange - var testSerializer = new DefaultAntiforgeryTokenSerializer(_dataProtector.Object, _pool); + var testSerializer = new DefaultAntiforgeryTokenSerializer(_dataProtector.Object); //"01" // Version //+ "705EEDCC7D42F1D6B3B98A593625BB4C" // SecurityToken @@ -87,7 +87,7 @@ public void Serialize_FieldToken_WithClaimUid_TokenRoundTripSuccessful() public void Serialize_FieldToken_WithUsername_TokenRoundTripSuccessful() { // Arrange - var testSerializer = new DefaultAntiforgeryTokenSerializer(_dataProtector.Object, _pool); + var testSerializer = new DefaultAntiforgeryTokenSerializer(_dataProtector.Object); //"01" // Version //+ "705EEDCC7D42F1D6B3B98A593625BB4C" // SecurityToken @@ -118,7 +118,7 @@ public void Serialize_FieldToken_WithUsername_TokenRoundTripSuccessful() public void Serialize_CookieToken_TokenRoundTripSuccessful() { // Arrange - var testSerializer = new DefaultAntiforgeryTokenSerializer(_dataProtector.Object, _pool); + var testSerializer = new DefaultAntiforgeryTokenSerializer(_dataProtector.Object); //"01" // Version //+ "705EEDCC7D42F1D6B3B98A593625BB4C" // SecurityToken @@ -130,7 +130,7 @@ public void Serialize_CookieToken_TokenRoundTripSuccessful() }; // Act - string actualSerializedData = testSerializer.Serialize(token); + var actualSerializedData = testSerializer.Serialize(token); var deserializedToken = testSerializer.Deserialize(actualSerializedData); // Assert diff --git a/src/Shared/Encoding/Int7BitEncodingUtils.cs b/src/Shared/Encoding/Int7BitEncodingUtils.cs index 2f012e541d88..933bd6907e83 100644 --- a/src/Shared/Encoding/Int7BitEncodingUtils.cs +++ b/src/Shared/Encoding/Int7BitEncodingUtils.cs @@ -86,6 +86,43 @@ public static int Read7BitEncodedInt(this ReadOnlySpan source, out int val return index; } + /// + /// Writes a 7-bit length-prefixed UTF-8 encoded string to the target span. + /// + /// The target span to write to. + /// The string to encode. + /// The number of bytes written to the target span. + internal static int Write7BitEncodedString(this Span target, string value) + { + if (string.IsNullOrEmpty(value)) + { + target[0] = 0; + return 1; + } + + var stringByteCount = System.Text.Encoding.UTF8.GetByteCount(value); + var lengthPrefixSize = target.Write7BitEncodedInt(stringByteCount); + System.Text.Encoding.UTF8.GetBytes(value, target.Slice(lengthPrefixSize)); + + return lengthPrefixSize + stringByteCount; + } + + /// + /// Calculates the number of bytes required to write a 7-bit length-prefixed UTF-8 encoded string. + /// + /// The string to measure. + /// The number of bytes required. + internal static int Measure7BitEncodedStringLength(string value) + { + if (string.IsNullOrEmpty(value)) + { + return 1; + } + + var stringByteCount = System.Text.Encoding.UTF8.GetByteCount(value); + return stringByteCount.Measure7BitEncodedUIntLength() + stringByteCount; + } + /// /// Returns consumed length /// diff --git a/src/Shared/test/Shared.Tests/Encoding/Int7BitEncodingUtilsTests.cs b/src/Shared/test/Shared.Tests/Encoding/Int7BitEncodingUtilsTests.cs index 04369e4b5b43..74e46d173993 100644 --- a/src/Shared/test/Shared.Tests/Encoding/Int7BitEncodingUtilsTests.cs +++ b/src/Shared/test/Shared.Tests/Encoding/Int7BitEncodingUtilsTests.cs @@ -218,4 +218,112 @@ public void Read7BitEncodedString_WithMultiByteLengthPrefix_DecodesCorrectly() Assert.Equal(longString, value); Assert.Equal(encodedBytes.Length, bytesConsumed); } + + [Theory] + [InlineData("")] + [InlineData("Hello")] + [InlineData("Hello, World!")] + [InlineData("UTF-8: \u00e9\u00e8\u00ea")] + public void Write7BitEncodedString_EncodesCorrectly(string value) + { + var expectedByteCount = Int7BitEncodingUtils.Measure7BitEncodedStringLength(value); + Span buffer = stackalloc byte[expectedByteCount]; + + var bytesWritten = buffer.Write7BitEncodedString(value); + + Assert.Equal(expectedByteCount, bytesWritten); + + // Verify by reading back + ReadOnlySpan source = buffer.Slice(0, bytesWritten); + var bytesConsumed = source.Read7BitEncodedString(out var decoded); + + Assert.Equal(value, decoded); + Assert.Equal(bytesWritten, bytesConsumed); + } + + [Fact] + public void Write7BitEncodedString_WithNullString_WritesZeroLength() + { + Span buffer = stackalloc byte[10]; + + var bytesWritten = buffer.Write7BitEncodedString(null!); + + Assert.Equal(1, bytesWritten); + Assert.Equal(0, buffer[0]); + } + + [Fact] + public void Write7BitEncodedString_WithEmptyString_WritesZeroLength() + { + Span buffer = stackalloc byte[10]; + + var bytesWritten = buffer.Write7BitEncodedString(string.Empty); + + Assert.Equal(1, bytesWritten); + Assert.Equal(0, buffer[0]); + } + + [Fact] + public void Write7BitEncodedString_WithLongString_UsesMultiByteLengthPrefix() + { + var longString = new string('A', 200); + var expectedByteCount = Int7BitEncodingUtils.Measure7BitEncodedStringLength(longString); + var buffer = new byte[expectedByteCount]; + + var bytesWritten = buffer.AsSpan().Write7BitEncodedString(longString); + + Assert.Equal(expectedByteCount, bytesWritten); + + // Verify the length prefix is multi-byte (200 > 127) + Assert.True((buffer[0] & 0x80) != 0); // Continuation bit set + + // Verify by reading back + ReadOnlySpan source = buffer; + var bytesConsumed = source.Read7BitEncodedString(out var decoded); + + Assert.Equal(longString, decoded); + Assert.Equal(bytesWritten, bytesConsumed); + } + + [Theory] + [InlineData("", 1)] + [InlineData("A", 2)] + [InlineData("Hello", 6)] + public void Measure7BitEncodedStringLength_ReturnsCorrectLength(string value, int expectedLength) + { + var actualLength = Int7BitEncodingUtils.Measure7BitEncodedStringLength(value); + + Assert.Equal(expectedLength, actualLength); + } + + [Fact] + public void Measure7BitEncodedStringLength_WithNullString_ReturnsOne() + { + var length = Int7BitEncodingUtils.Measure7BitEncodedStringLength(null!); + + Assert.Equal(1, length); + } + + [Fact] + public void Measure7BitEncodedStringLength_WithLongString_IncludesMultiByteLengthPrefix() + { + var longString = new string('A', 200); + + var length = Int7BitEncodingUtils.Measure7BitEncodedStringLength(longString); + + // 200 bytes for string + 2 bytes for length prefix (200 requires 2 bytes in 7-bit encoding) + Assert.Equal(202, length); + } + + [Fact] + public void Measure7BitEncodedStringLength_WithUtf8String_CountsUtf8Bytes() + { + // Each of these characters is 2 bytes in UTF-8 + var utf8String = "\u00e9\u00e8\u00ea"; // 3 chars, 6 bytes + + var length = Int7BitEncodingUtils.Measure7BitEncodedStringLength(utf8String); + + // 6 bytes for string + 1 byte for length prefix + Assert.Equal(7, length); + } } From 47cc61fd2e64748f58d7e5c26f5231cc4f0b7995 Mon Sep 17 00:00:00 2001 From: Korolev Dmitry Date: Thu, 11 Dec 2025 12:31:34 +0100 Subject: [PATCH 03/13] workaround one of byte[] allocations --- src/Antiforgery/src/Internal/BinaryBlob.cs | 2 + .../DefaultAntiforgeryTokenGenerator.cs | 29 +++++- .../src/Internal/DefaultClaimUidExtractor.cs | 93 +++++++++++++++---- .../src/Internal/IClaimUidExtractor.cs | 8 ++ .../DefaultAntiforgeryTokenGeneratorTest.cs | 59 ++++++++---- 5 files changed, 150 insertions(+), 41 deletions(-) diff --git a/src/Antiforgery/src/Internal/BinaryBlob.cs b/src/Antiforgery/src/Internal/BinaryBlob.cs index 1196ffd0bdfe..719eb9f24b38 100644 --- a/src/Antiforgery/src/Internal/BinaryBlob.cs +++ b/src/Antiforgery/src/Internal/BinaryBlob.cs @@ -35,6 +35,8 @@ public BinaryBlob(int bitLength, byte[] data) _data = data; } + internal int Length => _data.Length; + public int BitLength { get diff --git a/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenGenerator.cs b/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenGenerator.cs index 06a08914924d..1982a5bafed8 100644 --- a/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenGenerator.cs +++ b/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenGenerator.cs @@ -137,16 +137,22 @@ public bool TryValidateTokenSet( // Is the incoming token meant for the current user? var currentUsername = string.Empty; - BinaryBlob? currentClaimUid = null; + + var extractedClaimUidBytes = false; + Span currentClaimUidBytes = stackalloc byte[32]; var authenticatedIdentity = GetAuthenticatedIdentity(httpContext.User); if (authenticatedIdentity != null) { - currentClaimUid = GetClaimUidBlob(_claimUidExtractor.ExtractClaimUid(httpContext.User)); - if (currentClaimUid == null) + var requestTokenClaimUidLength = requestToken.ClaimUid?.Length; + if (requestTokenClaimUidLength is null) { currentUsername = authenticatedIdentity.Name ?? string.Empty; } + else + { + extractedClaimUidBytes = _claimUidExtractor.TryExtractClaimUidBytes(httpContext.User, currentClaimUidBytes); + } } // OpenID and other similar authentication schemes use URIs for the username. @@ -164,14 +170,14 @@ public bool TryValidateTokenSet( return false; } - if (!object.Equals(requestToken.ClaimUid, currentClaimUid)) + if (!AreIdenticalClaimUids(currentClaimUidBytes)) { message = Resources.AntiforgeryToken_ClaimUidMismatch; return false; } // Is the AdditionalData valid? - if (_additionalDataProvider != null && + if (_additionalDataProvider is not null && !_additionalDataProvider.ValidateAdditionalData(httpContext, requestToken.AdditionalData)) { message = Resources.AntiforgeryToken_AdditionalDataCheckFailed; @@ -180,6 +186,19 @@ public bool TryValidateTokenSet( message = null; return true; + + bool AreIdenticalClaimUids(Span claimUidBytes) + { + if (requestToken.ClaimUid is null) + { + return !extractedClaimUidBytes; + } + if (requestToken.ClaimUid.Length != claimUidBytes.Length) + { + return false; + } + return requestToken.ClaimUid.GetData().SequenceEqual(claimUidBytes); + } } private static BinaryBlob? GetClaimUidBlob(string? base64ClaimUid) diff --git a/src/Antiforgery/src/Internal/DefaultClaimUidExtractor.cs b/src/Antiforgery/src/Internal/DefaultClaimUidExtractor.cs index 31ffcd37f0f9..ce13acad75bb 100644 --- a/src/Antiforgery/src/Internal/DefaultClaimUidExtractor.cs +++ b/src/Antiforgery/src/Internal/DefaultClaimUidExtractor.cs @@ -1,9 +1,11 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Buffers; using System.Diagnostics; using System.Security.Claims; using System.Security.Cryptography; +using Microsoft.AspNetCore.Shared; using Microsoft.Extensions.ObjectPool; namespace Microsoft.AspNetCore.Antiforgery; @@ -36,12 +38,28 @@ public DefaultClaimUidExtractor(ObjectPool pool return Convert.ToBase64String(claimUidBytes); } + public bool TryExtractClaimUidBytes(ClaimsPrincipal claimsPrincipal, Span destination) + { + Debug.Assert(claimsPrincipal != null); + + var uniqueIdentifierParameters = GetUniqueIdentifierParameters(claimsPrincipal.Identities); + if (uniqueIdentifierParameters is null) + { + return false; + } + + // SHA256 always produces 32 bytes + Span claimUidBytes = stackalloc byte[32]; + ComputeSha256(uniqueIdentifierParameters, claimUidBytes); + return true; + } + public static IList? GetUniqueIdentifierParameters(IEnumerable claimsIdentities) { var identitiesList = claimsIdentities as List; if (identitiesList == null) { - identitiesList = new List(claimsIdentities); + identitiesList = [.. claimsIdentities]; } for (var i = 0; i < identitiesList.Count; i++) @@ -56,36 +74,36 @@ public DefaultClaimUidExtractor(ObjectPool pool claim => string.Equals("sub", claim.Type, StringComparison.Ordinal)); if (subClaim != null && !string.IsNullOrEmpty(subClaim.Value)) { - return new string[] - { - subClaim.Type, - subClaim.Value, - subClaim.Issuer - }; + return + [ + subClaim.Type, + subClaim.Value, + subClaim.Issuer + ]; } var nameIdentifierClaim = identity.FindFirst( claim => string.Equals(ClaimTypes.NameIdentifier, claim.Type, StringComparison.Ordinal)); if (nameIdentifierClaim != null && !string.IsNullOrEmpty(nameIdentifierClaim.Value)) { - return new string[] - { - nameIdentifierClaim.Type, - nameIdentifierClaim.Value, - nameIdentifierClaim.Issuer - }; + return + [ + nameIdentifierClaim.Type, + nameIdentifierClaim.Value, + nameIdentifierClaim.Issuer + ]; } var upnClaim = identity.FindFirst( claim => string.Equals(ClaimTypes.Upn, claim.Type, StringComparison.Ordinal)); if (upnClaim != null && !string.IsNullOrEmpty(upnClaim.Value)) { - return new string[] - { - upnClaim.Type, - upnClaim.Value, - upnClaim.Issuer - }; + return + [ + upnClaim.Type, + upnClaim.Value, + upnClaim.Issuer + ]; } } @@ -119,6 +137,43 @@ public DefaultClaimUidExtractor(ObjectPool pool return identifierParameters; } + private static void ComputeSha256(IList parameters, Span destination) + { + // Calculate total size needed for serialization + var totalSize = 0; + for (var i = 0; i < parameters.Count; i++) + { + var byteCount = System.Text.Encoding.UTF8.GetByteCount(parameters[i]); + totalSize += byteCount.Measure7BitEncodedUIntLength() + byteCount; + } + + // Use stackalloc for small buffers, otherwise rent + byte[]? rentedBuffer = null; + var buffer = totalSize <= 256 + ? stackalloc byte[256] + : (rentedBuffer = ArrayPool.Shared.Rent(totalSize)); + + try + { + var span = buffer[..totalSize]; + var offset = 0; + for (var i = 0; i < parameters.Count; i++) + { + offset += span.Slice(offset).Write7BitEncodedString(parameters[i]); + } + + // Hash directly into destination (SHA256 output is always 32 bytes) + SHA256.HashData(span.Slice(0, offset), destination); + } + finally + { + if (rentedBuffer is not null) + { + ArrayPool.Shared.Return(rentedBuffer); + } + } + } + private byte[] ComputeSha256(IEnumerable parameters) { var serializationContext = _pool.Get(); diff --git a/src/Antiforgery/src/Internal/IClaimUidExtractor.cs b/src/Antiforgery/src/Internal/IClaimUidExtractor.cs index de8c536c34b5..954e86f946b4 100644 --- a/src/Antiforgery/src/Internal/IClaimUidExtractor.cs +++ b/src/Antiforgery/src/Internal/IClaimUidExtractor.cs @@ -16,4 +16,12 @@ internal interface IClaimUidExtractor /// The . /// The claims identifier. string? ExtractClaimUid(ClaimsPrincipal claimsPrincipal); + + /// + /// Computes and writes the SHA256 hash of the claims identifier into the provided destination span. + /// + /// + /// SHA256 will always produce 32 bytes, so pre-allocate or stackalloc a byte span of that size. + /// + bool TryExtractClaimUidBytes(ClaimsPrincipal claimsPrincipal, Span destination); } diff --git a/src/Antiforgery/test/DefaultAntiforgeryTokenGeneratorTest.cs b/src/Antiforgery/test/DefaultAntiforgeryTokenGeneratorTest.cs index 3691b24aa3b0..8520020496a2 100644 --- a/src/Antiforgery/test/DefaultAntiforgeryTokenGeneratorTest.cs +++ b/src/Antiforgery/test/DefaultAntiforgeryTokenGeneratorTest.cs @@ -4,6 +4,7 @@ #nullable disable using System.Security.Claims; using System.Security.Cryptography; +using System.Security.Principal; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.InternalTesting; using Moq; @@ -439,21 +440,15 @@ public void TryValidateTokenSet_ClaimUidMismatch() }; var differentToken = new BinaryBlob(256); - var mockClaimUidExtractor = new Mock(); - mockClaimUidExtractor.Setup(o => o.ExtractClaimUid(It.Is(c => c.Identity == identity))) - .Returns(Convert.ToBase64String(differentToken.GetData())); - - var tokenProvider = new DefaultAntiforgeryTokenGenerator( - claimUidExtractor: mockClaimUidExtractor.Object, - additionalDataProvider: null); + var dummyClaimUidExtractor = new DummyClaimUidExtractor(identity, differentToken); + var tokenProvider = new DefaultAntiforgeryTokenGenerator(claimUidExtractor: dummyClaimUidExtractor, additionalDataProvider: null); - string expectedMessage = + var expectedMessage = "The provided antiforgery token was meant for a different " + "claims-based user than the current user."; // Act - string message; - var result = tokenProvider.TryValidateTokenSet(httpContext, cookieToken, fieldtoken, out message); + var result = tokenProvider.TryValidateTokenSet(httpContext, cookieToken, fieldtoken, out var message); // Assert Assert.False(result); @@ -581,17 +576,13 @@ public void TryValidateTokenSet_Success_ClaimsBasedUser() ClaimUid = new BinaryBlob(256) }; - var mockClaimUidExtractor = new Mock(); - mockClaimUidExtractor.Setup(o => o.ExtractClaimUid(It.Is(c => c.Identity == identity))) - .Returns(Convert.ToBase64String(fieldtoken.ClaimUid.GetData())); - + var dummyClaimUidExtractor = new DummyClaimUidExtractor(identity, fieldtoken.ClaimUid); var tokenProvider = new DefaultAntiforgeryTokenGenerator( - claimUidExtractor: mockClaimUidExtractor.Object, + claimUidExtractor: dummyClaimUidExtractor, additionalDataProvider: null); // Act - string message; - var result = tokenProvider.TryValidateTokenSet(httpContext, cookieToken, fieldtoken, out message); + var result = tokenProvider.TryValidateTokenSet(httpContext, cookieToken, fieldtoken, out var message); // Assert Assert.True(result); @@ -616,5 +607,39 @@ public override string Name get { return String.Empty; } } } + + private class DummyClaimUidExtractor : IClaimUidExtractor + { + private readonly IIdentity _identity; + private readonly BinaryBlob _differentToken; + + public DummyClaimUidExtractor(IIdentity identity, BinaryBlob blob) + { + _identity = identity; + _differentToken = blob; + } + + public string ExtractClaimUid(ClaimsPrincipal claimsPrincipal) + { + if (claimsPrincipal.Identity == _identity) + { + return Convert.ToBase64String(_differentToken.GetData()); + } + + Assert.Fail("Unexpected identity"); + return default; + } + + public bool TryExtractClaimUidBytes(ClaimsPrincipal claimsPrincipal, Span destination) + { + if (claimsPrincipal.Identity == _identity) + { + _differentToken.GetData().CopyTo(destination); + return true; + } + + return false; + } + } } #nullable restore From 2b428da25e822b09bdec7f5dfad7e4bc518be102 Mon Sep 17 00:00:00 2001 From: Korolev Dmitry Date: Thu, 11 Dec 2025 18:47:42 +0100 Subject: [PATCH 04/13] add micro benchmarks proj --- AspNetCore.slnx | 3 + src/Antiforgery/Antiforgery.slnf | 5 +- .../Benchmarks/AntifogeryBenchmarks.cs | 153 ++++++++++++++++++ .../AntiforgeryTokenGeneratorBenchmarks.cs | 150 +++++++++++++++++ .../AntiforgeryTokenSerializerBenchmarks.cs | 60 +++++++ ...t.AspNetCore.Antiforgery.Benchmarks.csproj | 27 ++++ .../Microsoft.AspNetCore.Antiforgery.csproj | 4 + 7 files changed, 400 insertions(+), 2 deletions(-) create mode 100644 src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntifogeryBenchmarks.cs create mode 100644 src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenGeneratorBenchmarks.cs create mode 100644 src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenSerializerBenchmarks.cs create mode 100644 src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks.csproj diff --git a/AspNetCore.slnx b/AspNetCore.slnx index cf6f14abe556..03980d7b60a4 100644 --- a/AspNetCore.slnx +++ b/AspNetCore.slnx @@ -19,6 +19,9 @@ + + + diff --git a/src/Antiforgery/Antiforgery.slnf b/src/Antiforgery/Antiforgery.slnf index 7ce6a6ecf3dd..03f3fd7f5bc3 100644 --- a/src/Antiforgery/Antiforgery.slnf +++ b/src/Antiforgery/Antiforgery.slnf @@ -1,10 +1,11 @@ -{ +{ "solution": { "path": "..\\..\\AspNetCore.slnx", "projects": [ + "src\\Antiforgery\\benchmarks\\Microsoft.AspNetCore.Antiforgery.Benchmarks\\Microsoft.AspNetCore.Antiforgery.Benchmarks.csproj", "src\\Antiforgery\\samples\\MinimalFormSample\\MinimalFormSample.csproj", "src\\Antiforgery\\src\\Microsoft.AspNetCore.Antiforgery.csproj", "src\\Antiforgery\\test\\Microsoft.AspNetCore.Antiforgery.Test.csproj" ] } -} +} \ No newline at end of file diff --git a/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntifogeryBenchmarks.cs b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntifogeryBenchmarks.cs new file mode 100644 index 000000000000..1bcefb6fed4f --- /dev/null +++ b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntifogeryBenchmarks.cs @@ -0,0 +1,153 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Security.Claims; +using BenchmarkDotNet.Attributes; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Features; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using Microsoft.Extensions.Primitives; + +namespace Microsoft.AspNetCore.Antiforgery.Benchmarks.Benchmarks; + +/* + + main branch: + | Method | Mean | Error | StdDev | Op/s | Gen 0 | Gen 1 | Gen 2 | Allocated | + |--------------------- |---------:|---------:|---------:|---------:|------:|------:|------:|----------:| + | GetAndStoreTokens | 59.56 us | 2.482 us | 7.082 us | 16,789.6 | - | - | - | 5 KB | + | ValidateRequestAsync | 50.60 us | 2.150 us | 6.167 us | 19,764.1 | - | - | - | 4 KB | + + */ + +[AspNetCoreBenchmark] +public class AntiforgeryBenchmarks +{ + private IServiceProvider _serviceProvider = null!; + private IAntiforgery _antiforgery = null!; + private string _cookieName = null!; + private string _formFieldName = null!; + + // For GetAndStoreTokens - fresh context each time + private HttpContext _getAndStoreTokensContext = null!; + + // For ValidateRequestAsync - context with valid tokens + private HttpContext _validateRequestContext = null!; + private string _cookieToken = null!; + private string _requestToken = null!; + + [GlobalSetup] + public void Setup() + { + var serviceCollection = new ServiceCollection(); + serviceCollection.AddAntiforgery(); + serviceCollection.AddLogging(); + _serviceProvider = serviceCollection.BuildServiceProvider(); + + _antiforgery = _serviceProvider.GetRequiredService(); + + // Get the actual cookie and form field names from options + var options = _serviceProvider.GetRequiredService>().Value; + _cookieName = options.Cookie.Name!; + _formFieldName = options.FormFieldName; + + // Setup context for GetAndStoreTokens (no existing tokens) + _getAndStoreTokensContext = CreateHttpContext(); + + // Generate tokens for validation benchmark + var tokenContext = CreateHttpContext(); + var tokenSet = _antiforgery.GetAndStoreTokens(tokenContext); + _cookieToken = tokenSet.CookieToken!; + _requestToken = tokenSet.RequestToken!; + + // Setup context for ValidateRequestAsync (with valid tokens) + _validateRequestContext = CreateHttpContextWithTokens(_cookieToken, _requestToken); + } + + [IterationSetup(Target = nameof(GetAndStoreTokens))] + public void SetupGetAndStoreTokens() + { + // Create a fresh context for each iteration to simulate real-world usage + _getAndStoreTokensContext = CreateHttpContext(); + } + + [IterationSetup(Target = nameof(ValidateRequestAsync))] + public void SetupValidateRequest() + { + // Create a fresh context with tokens for each iteration + _validateRequestContext = CreateHttpContextWithTokens(_cookieToken, _requestToken); + } + + [Benchmark] + public AntiforgeryTokenSet GetAndStoreTokens() + { + return _antiforgery.GetAndStoreTokens(_getAndStoreTokensContext); + } + + [Benchmark] + public Task ValidateRequestAsync() + { + return _antiforgery.ValidateRequestAsync(_validateRequestContext); + } + + private HttpContext CreateHttpContext() + { + var context = new DefaultHttpContext(); + context.RequestServices = _serviceProvider; + + // Create an authenticated identity with a Name claim (required by antiforgery) + var identity = new ClaimsIdentity( + [new Claim(ClaimsIdentity.DefaultNameClaimType, "testuser@example.com")], + "TestAuth"); + context.User = new ClaimsPrincipal(identity); + + context.Request.Method = "POST"; + context.Request.ContentType = "application/x-www-form-urlencoded"; + + // Setup response features to allow cookie writing + var responseFeature = new TestHttpResponseFeature(); + context.Features.Set(responseFeature); + context.Features.Set(new StreamResponseBodyFeature(Stream.Null)); + + return context; + } + + private HttpContext CreateHttpContextWithTokens(string cookieToken, string requestToken) + { + var context = new DefaultHttpContext(); + context.RequestServices = _serviceProvider; + + // Create an authenticated identity with a Name claim (required by antiforgery) + var identity = new ClaimsIdentity( + [new Claim(ClaimsIdentity.DefaultNameClaimType, "testuser@example.com")], + "TestAuth"); + context.User = new ClaimsPrincipal(identity); + + context.Request.Method = "POST"; + context.Request.ContentType = "application/x-www-form-urlencoded"; + + // Set the cookie token using the actual cookie name from options + context.Request.Headers.Cookie = $"{_cookieName}={cookieToken}"; + + // Set the request token in form using the actual form field name + context.Request.Form = new FormCollection(new Dictionary + { + { _formFieldName, requestToken } + }); + + return context; + } + + private sealed class TestHttpResponseFeature : IHttpResponseFeature + { + public int StatusCode { get; set; } = 200; + public string? ReasonPhrase { get; set; } + public IHeaderDictionary Headers { get; set; } = new HeaderDictionary(); + public Stream Body { get; set; } = Stream.Null; + public bool HasStarted => false; + + public void OnStarting(Func callback, object state) { } + public void OnCompleted(Func callback, object state) { } + } +} diff --git a/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenGeneratorBenchmarks.cs b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenGeneratorBenchmarks.cs new file mode 100644 index 000000000000..2017a6aefed2 --- /dev/null +++ b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenGeneratorBenchmarks.cs @@ -0,0 +1,150 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Security.Claims; +using BenchmarkDotNet.Attributes; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; + +namespace Microsoft.AspNetCore.Antiforgery.Benchmarks.Benchmarks; + +/* + main branch: + | Method | Mean | Error | StdDev | Op/s | Gen 0 | Gen 1 | Gen 2 | Allocated | + |----------------------------------- |------------:|----------:|----------:|----------------:|-------:|------:|------:|----------:| + | GenerateCookieToken | 5.8804 ns | 0.0756 ns | 0.0670 ns | 170,055,724.4 | 0.0007 | - | - | 56 B | + | GenerateRequestToken_Anonymous | 11.0555 ns | 0.1203 ns | 0.1066 ns | 90,452,434.9 | 0.0007 | - | - | 56 B | + | GenerateRequestToken_Authenticated | 401.2545 ns | 7.1693 ns | 6.3554 ns | 2,492,184.2 | 0.0076 | - | - | 592 B | + | TryValidateTokenSet_Anonymous | 6.7227 ns | 0.0357 ns | 0.0316 ns | 148,750,552.9 | - | - | - | - | + | TryValidateTokenSet_Authenticated | 508.1742 ns | 4.4728 ns | 3.7350 ns | 1,967,829.1 | 0.0095 | - | - | 760 B | + | TryValidateTokenSet_ClaimsBased | 308.4674 ns | 3.3256 ns | 3.1108 ns | 3,241,833.1 | 0.0038 | - | - | 312 B | + + */ + +[AspNetCoreBenchmark] +public class AntiforgeryTokenGeneratorBenchmarks +{ + private IAntiforgeryTokenGenerator _tokenGenerator = null!; + + // Anonymous user scenario + private HttpContext _anonymousHttpContext = null!; + private AntiforgeryToken _anonymousCookieToken = null!; + private AntiforgeryToken _anonymousRequestToken = null!; + + // Authenticated user with username scenario + private HttpContext _authenticatedHttpContext = null!; + private AntiforgeryToken _authenticatedCookieToken = null!; + private AntiforgeryToken _authenticatedRequestToken = null!; + + // Claims-based user scenario + private HttpContext _claimsHttpContext = null!; + private AntiforgeryToken _claimsCookieToken = null!; + private AntiforgeryToken _claimsRequestToken = null!; + + [GlobalSetup] + public void Setup() + { + var serviceCollection = new ServiceCollection(); + serviceCollection.AddAntiforgery(); + var serviceProvider = serviceCollection.BuildServiceProvider(); + + _tokenGenerator = serviceProvider.GetRequiredService(); + var claimUidExtractor = serviceProvider.GetRequiredService(); + + // Setup anonymous user scenario + _anonymousHttpContext = new DefaultHttpContext(); + _anonymousHttpContext.User = new ClaimsPrincipal(new ClaimsIdentity()); + + _anonymousCookieToken = new AntiforgeryToken { IsCookieToken = true }; + _anonymousRequestToken = new AntiforgeryToken + { + IsCookieToken = false, + SecurityToken = _anonymousCookieToken.SecurityToken, + Username = string.Empty + }; + + // Setup authenticated user with username scenario + _authenticatedHttpContext = new DefaultHttpContext(); + var authenticatedIdentity = new ClaimsIdentity( + [new Claim(ClaimsIdentity.DefaultNameClaimType, "testuser@example.com")], + "TestAuthentication"); + _authenticatedHttpContext.User = new ClaimsPrincipal(authenticatedIdentity); + + _authenticatedCookieToken = new AntiforgeryToken { IsCookieToken = true }; + _authenticatedRequestToken = new AntiforgeryToken + { + IsCookieToken = false, + SecurityToken = _authenticatedCookieToken.SecurityToken, + Username = "testuser@example.com" + }; + + // Setup claims-based user scenario + _claimsHttpContext = new DefaultHttpContext(); + var claimsIdentity = new ClaimsIdentity( + [ + new Claim(ClaimsIdentity.DefaultNameClaimType, "claimsuser@example.com"), + new Claim("sub", "user-id-12345"), + new Claim(ClaimTypes.NameIdentifier, "unique-id") + ], + "ClaimsAuthentication"); + _claimsHttpContext.User = new ClaimsPrincipal(claimsIdentity); + + _claimsCookieToken = new AntiforgeryToken { IsCookieToken = true }; + // For claims-based users, we need to extract the ClaimUid + var claimUid = claimUidExtractor.ExtractClaimUid(_claimsHttpContext.User); + _claimsRequestToken = new AntiforgeryToken + { + IsCookieToken = false, + SecurityToken = _claimsCookieToken.SecurityToken, + ClaimUid = claimUid is not null ? new BinaryBlob(256, Convert.FromBase64String(claimUid)) : null + }; + } + + [Benchmark] + public object GenerateCookieToken() + { + return _tokenGenerator.GenerateCookieToken(); + } + + [Benchmark] + public object GenerateRequestToken_Anonymous() + { + return _tokenGenerator.GenerateRequestToken(_anonymousHttpContext, _anonymousCookieToken); + } + + [Benchmark] + public object GenerateRequestToken_Authenticated() + { + return _tokenGenerator.GenerateRequestToken(_authenticatedHttpContext, _authenticatedCookieToken); + } + + [Benchmark] + public bool TryValidateTokenSet_Anonymous() + { + return _tokenGenerator.TryValidateTokenSet( + _anonymousHttpContext, + _anonymousCookieToken, + _anonymousRequestToken, + out _); + } + + [Benchmark] + public bool TryValidateTokenSet_Authenticated() + { + return _tokenGenerator.TryValidateTokenSet( + _authenticatedHttpContext, + _authenticatedCookieToken, + _authenticatedRequestToken, + out _); + } + + [Benchmark] + public bool TryValidateTokenSet_ClaimsBased() + { + return _tokenGenerator.TryValidateTokenSet( + _claimsHttpContext, + _claimsCookieToken, + _claimsRequestToken, + out _); + } +} diff --git a/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenSerializerBenchmarks.cs b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenSerializerBenchmarks.cs new file mode 100644 index 000000000000..c3dc8686e0c4 --- /dev/null +++ b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenSerializerBenchmarks.cs @@ -0,0 +1,60 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using BenchmarkDotNet.Attributes; +using Microsoft.Extensions.DependencyInjection; + +namespace Microsoft.AspNetCore.Antiforgery.Benchmarks.Benchmarks; + +/* + * + main branch: + + | Method | Mean | Error | StdDev | Op/s | Gen 0 | Gen 1 | Gen 2 | Allocated | + |------------ |---------:|----------:|----------:|----------:|-------:|------:|------:|----------:| + | Serialize | 2.221 us | 0.0245 us | 0.0229 us | 450,239.5 | 0.0076 | - | - | 872 B | + | Deserialize | 2.436 us | 0.0463 us | 0.1036 us | 410,492.5 | 0.0076 | - | - | 632 B | + * + */ + +[AspNetCoreBenchmark] +public class AntiforgeryTokenSerializerBenchmarks +{ +#pragma warning disable CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring as nullable. + private IAntiforgeryTokenSerializer _tokenSerializer; + + private AntiforgeryToken _token; + private string _serializedToken; +#pragma warning restore CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring as nullable. + + [GlobalSetup] + public void Setup() + { + var serviceCollection = new ServiceCollection(); + serviceCollection.AddAntiforgery(); + var serviceProvider = serviceCollection.BuildServiceProvider(); + _tokenSerializer = serviceProvider.GetRequiredService(); + + _token = new AntiforgeryToken() + { + IsCookieToken = false, + Username = "user@test.com", + ClaimUid = new BinaryBlob(AntiforgeryToken.ClaimUidBitLength), + AdditionalData = "additional-data-here" + }; + + _serializedToken = _tokenSerializer.Serialize(_token); + } + + [Benchmark] + public string Serialize() + { + return _tokenSerializer.Serialize(_token); + } + + [Benchmark] + public object Deserialize() + { + return _tokenSerializer.Deserialize(_serializedToken); + } +} diff --git a/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks.csproj b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks.csproj new file mode 100644 index 000000000000..72cdf7ac580d --- /dev/null +++ b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks.csproj @@ -0,0 +1,27 @@ + + + + $(DefaultNetCoreTargetFramework) + Exe + true + true + false + $(DefineConstants);IS_BENCHMARKS + true + false + enable + + + + + + + + + + + + + + + diff --git a/src/Antiforgery/src/Microsoft.AspNetCore.Antiforgery.csproj b/src/Antiforgery/src/Microsoft.AspNetCore.Antiforgery.csproj index db92d8f239cc..369284a835b8 100644 --- a/src/Antiforgery/src/Microsoft.AspNetCore.Antiforgery.csproj +++ b/src/Antiforgery/src/Microsoft.AspNetCore.Antiforgery.csproj @@ -29,4 +29,8 @@ + + + + From f30bbe47c0d146e8596bff618034e90e9c5780c0 Mon Sep 17 00:00:00 2001 From: Korolev Dmitry Date: Fri, 12 Dec 2025 15:02:08 +0100 Subject: [PATCH 05/13] some reworks --- .../Benchmarks/AntifogeryBenchmarks.cs | 6 + .../AntiforgeryTokenGeneratorBenchmarks.cs | 23 ++-- .../AntiforgeryTokenSerializerBenchmarks.cs | 9 +- .../AntiforgeryServiceCollectionExtensions.cs | 10 -- .../AntiforgerySerializationContext.cs | 120 ------------------ ...ySerializationContextPooledObjectPolicy.cs | 21 --- .../src/Internal/DefaultAntiforgery.cs | 2 +- .../DefaultAntiforgeryTokenGenerator.cs | 26 +--- .../DefaultAntiforgeryTokenSerializer.cs | 4 +- .../src/Internal/DefaultClaimUidExtractor.cs | 61 +-------- .../src/Internal/IClaimUidExtractor.cs | 27 ---- .../DefaultAntiforgeryTokenSerializerTest.cs | 6 +- .../test/DefaultClaimUidExtractorTest.cs | 46 +++---- 13 files changed, 57 insertions(+), 304 deletions(-) delete mode 100644 src/Antiforgery/src/Internal/AntiforgerySerializationContext.cs delete mode 100644 src/Antiforgery/src/Internal/AntiforgerySerializationContextPooledObjectPolicy.cs delete mode 100644 src/Antiforgery/src/Internal/IClaimUidExtractor.cs diff --git a/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntifogeryBenchmarks.cs b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntifogeryBenchmarks.cs index 1bcefb6fed4f..6d8bae219ff7 100644 --- a/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntifogeryBenchmarks.cs +++ b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntifogeryBenchmarks.cs @@ -19,6 +19,12 @@ namespace Microsoft.AspNetCore.Antiforgery.Benchmarks.Benchmarks; | GetAndStoreTokens | 59.56 us | 2.482 us | 7.082 us | 16,789.6 | - | - | - | 5 KB | | ValidateRequestAsync | 50.60 us | 2.150 us | 6.167 us | 19,764.1 | - | - | - | 4 KB | + this PR: + | Method | Mean | Error | StdDev | Op/s | Gen 0 | Gen 1 | Gen 2 | Allocated | + |--------------------- |---------:|---------:|---------:|---------:|------:|------:|------:|----------:| + | GetAndStoreTokens | 46.66 us | 2.150 us | 5.958 us | 21,432.4 | - | - | - | 4 KB | + | ValidateRequestAsync | 44.79 us | 2.279 us | 6.391 us | 22,327.2 | - | - | - | 3 KB | + */ [AspNetCoreBenchmark] diff --git a/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenGeneratorBenchmarks.cs b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenGeneratorBenchmarks.cs index 2017a6aefed2..122d01f36ee3 100644 --- a/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenGeneratorBenchmarks.cs +++ b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenGeneratorBenchmarks.cs @@ -12,13 +12,21 @@ namespace Microsoft.AspNetCore.Antiforgery.Benchmarks.Benchmarks; main branch: | Method | Mean | Error | StdDev | Op/s | Gen 0 | Gen 1 | Gen 2 | Allocated | |----------------------------------- |------------:|----------:|----------:|----------------:|-------:|------:|------:|----------:| - | GenerateCookieToken | 5.8804 ns | 0.0756 ns | 0.0670 ns | 170,055,724.4 | 0.0007 | - | - | 56 B | | GenerateRequestToken_Anonymous | 11.0555 ns | 0.1203 ns | 0.1066 ns | 90,452,434.9 | 0.0007 | - | - | 56 B | | GenerateRequestToken_Authenticated | 401.2545 ns | 7.1693 ns | 6.3554 ns | 2,492,184.2 | 0.0076 | - | - | 592 B | | TryValidateTokenSet_Anonymous | 6.7227 ns | 0.0357 ns | 0.0316 ns | 148,750,552.9 | - | - | - | - | | TryValidateTokenSet_Authenticated | 508.1742 ns | 4.4728 ns | 3.7350 ns | 1,967,829.1 | 0.0095 | - | - | 760 B | | TryValidateTokenSet_ClaimsBased | 308.4674 ns | 3.3256 ns | 3.1108 ns | 3,241,833.1 | 0.0038 | - | - | 312 B | + this PR: + | Method | Mean | Error | StdDev | Median | Op/s | Gen 0 | Gen 1 | Gen 2 | Allocated | + |----------------------------------- |-----------:|-----------:|-----------:|-----------:|--------------:|-------:|------:|------:|----------:| + | GenerateRequestToken_Anonymous | 17.814 ns | 0.3614 ns | 0.4017 ns | 17.771 ns | 56,134,225.3 | 0.0007 | - | - | 56 B | + | GenerateRequestToken_Authenticated | 459.848 ns | 9.1450 ns | 9.7851 ns | 461.283 ns | 2,174,632.5 | 0.0052 | - | - | 424 B | + | TryValidateTokenSet_Anonymous | 9.784 ns | 0.6728 ns | 1.9837 ns | 10.357 ns | 102,207,305.5 | - | - | - | - | + | TryValidateTokenSet_Authenticated | 17.300 ns | 0.4886 ns | 1.4407 ns | 17.929 ns | 57,804,630.5 | - | - | - | - | + | TryValidateTokenSet_ClaimsBased | 380.134 ns | 12.9953 ns | 38.3168 ns | 394.899 ns | 2,630,649.7 | 0.0014 | - | - | 120 B | + */ [AspNetCoreBenchmark] @@ -49,7 +57,6 @@ public void Setup() var serviceProvider = serviceCollection.BuildServiceProvider(); _tokenGenerator = serviceProvider.GetRequiredService(); - var claimUidExtractor = serviceProvider.GetRequiredService(); // Setup anonymous user scenario _anonymousHttpContext = new DefaultHttpContext(); @@ -90,22 +97,18 @@ [new Claim(ClaimsIdentity.DefaultNameClaimType, "testuser@example.com")], _claimsHttpContext.User = new ClaimsPrincipal(claimsIdentity); _claimsCookieToken = new AntiforgeryToken { IsCookieToken = true }; + // For claims-based users, we need to extract the ClaimUid - var claimUid = claimUidExtractor.ExtractClaimUid(_claimsHttpContext.User); + var claimUid = new byte[32]; + _ = ClaimUidExtractor.TryExtractClaimUidBytes(_claimsHttpContext.User, claimUid); _claimsRequestToken = new AntiforgeryToken { IsCookieToken = false, SecurityToken = _claimsCookieToken.SecurityToken, - ClaimUid = claimUid is not null ? new BinaryBlob(256, Convert.FromBase64String(claimUid)) : null + ClaimUid = claimUid is not null ? new BinaryBlob(256, claimUid) : null }; } - [Benchmark] - public object GenerateCookieToken() - { - return _tokenGenerator.GenerateCookieToken(); - } - [Benchmark] public object GenerateRequestToken_Anonymous() { diff --git a/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenSerializerBenchmarks.cs b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenSerializerBenchmarks.cs index c3dc8686e0c4..a5b73dab2f3b 100644 --- a/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenSerializerBenchmarks.cs +++ b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenSerializerBenchmarks.cs @@ -7,14 +7,17 @@ namespace Microsoft.AspNetCore.Antiforgery.Benchmarks.Benchmarks; /* - * main branch: - | Method | Mean | Error | StdDev | Op/s | Gen 0 | Gen 1 | Gen 2 | Allocated | |------------ |---------:|----------:|----------:|----------:|-------:|------:|------:|----------:| | Serialize | 2.221 us | 0.0245 us | 0.0229 us | 450,239.5 | 0.0076 | - | - | 872 B | | Deserialize | 2.436 us | 0.0463 us | 0.1036 us | 410,492.5 | 0.0076 | - | - | 632 B | - * + + This PR: + | Method | Mean | Error | StdDev | Op/s | Gen 0 | Gen 1 | Gen 2 | Allocated | + |------------ |---------:|----------:|----------:|----------:|-------:|------:|------:|----------:| + | Serialize | 1.932 us | 0.0110 us | 0.0098 us | 517,688.9 | 0.0038 | - | - | 544 B | + | Deserialize | 1.927 us | 0.0107 us | 0.0100 us | 519,058.2 | 0.0038 | - | - | 344 B | */ [AspNetCoreBenchmark] diff --git a/src/Antiforgery/src/AntiforgeryServiceCollectionExtensions.cs b/src/Antiforgery/src/AntiforgeryServiceCollectionExtensions.cs index d40bf9354798..61adcf7e55c0 100644 --- a/src/Antiforgery/src/AntiforgeryServiceCollectionExtensions.cs +++ b/src/Antiforgery/src/AntiforgeryServiceCollectionExtensions.cs @@ -3,7 +3,6 @@ using Microsoft.AspNetCore.Antiforgery; using Microsoft.Extensions.DependencyInjection.Extensions; -using Microsoft.Extensions.ObjectPool; using Microsoft.Extensions.Options; namespace Microsoft.Extensions.DependencyInjection; @@ -32,16 +31,7 @@ public static IServiceCollection AddAntiforgery(this IServiceCollection services services.TryAddSingleton(); services.TryAddSingleton(); services.TryAddSingleton(); - services.TryAddSingleton(); services.TryAddSingleton(); - services.TryAddSingleton(); - - services.TryAddSingleton>(serviceProvider => - { - var provider = serviceProvider.GetRequiredService(); - var policy = new AntiforgerySerializationContextPooledObjectPolicy(); - return provider.Create(policy); - }); return services; } diff --git a/src/Antiforgery/src/Internal/AntiforgerySerializationContext.cs b/src/Antiforgery/src/Internal/AntiforgerySerializationContext.cs deleted file mode 100644 index a4e416155a5b..000000000000 --- a/src/Antiforgery/src/Internal/AntiforgerySerializationContext.cs +++ /dev/null @@ -1,120 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Text; - -namespace Microsoft.AspNetCore.Antiforgery; - -internal sealed class AntiforgerySerializationContext -{ - // Avoid allocating 256 bytes (the default) and using 18 (the AntiforgeryToken minimum). 64 bytes is enough for - // a short username or claim UID and some additional data. MemoryStream bumps capacity to 256 if exceeded. - private const int InitialStreamSize = 64; - - // Don't let the MemoryStream grow beyond 1 MB. - private const int MaximumStreamSize = 0x100000; - - // Start _chars off with length 256 (18 bytes is protected into 116 bytes then encoded into 156 characters). - // Double length from there if necessary. - private const int InitialCharsLength = 256; - - // Don't let _chars grow beyond 512k characters. - private const int MaximumCharsLength = 0x80000; - - private char[]? _chars; - private MemoryStream? _stream; - private BinaryReader? _reader; - private BinaryWriter? _writer; - - public MemoryStream Stream - { - get - { - if (_stream == null) - { - _stream = new MemoryStream(InitialStreamSize); - } - - return _stream; - } - private set - { - _stream = value; - } - } - - public BinaryReader Reader - { - get - { - if (_reader == null) - { - // Leave open to clean up correctly even if only one of the reader or writer has been created. - _reader = new BinaryReader(Stream, Encoding.UTF8, leaveOpen: true); - } - - return _reader; - } - private set - { - _reader = value; - } - } - - public BinaryWriter Writer - { - get - { - if (_writer == null) - { - // Leave open to clean up correctly even if only one of the reader or writer has been created. - _writer = new BinaryWriter(Stream, Encoding.UTF8, leaveOpen: true); - } - - return _writer; - } - private set - { - _writer = value; - } - } - - public char[] GetChars(int count) - { - if (_chars == null || _chars.Length < count) - { - var newLength = _chars == null ? InitialCharsLength : checked(_chars.Length * 2); - while (newLength < count) - { - newLength = checked(newLength * 2); - } - - _chars = new char[newLength]; - } - - return _chars; - } - - public void Reset() - { - if (_chars != null && _chars.Length > MaximumCharsLength) - { - _chars = null; - } - - if (_stream != null) - { - if (Stream.Capacity > MaximumStreamSize) - { - _stream = null; - _reader = null; - _writer = null; - } - else - { - Stream.Position = 0L; - Stream.SetLength(0L); - } - } - } -} diff --git a/src/Antiforgery/src/Internal/AntiforgerySerializationContextPooledObjectPolicy.cs b/src/Antiforgery/src/Internal/AntiforgerySerializationContextPooledObjectPolicy.cs deleted file mode 100644 index dad41c367754..000000000000 --- a/src/Antiforgery/src/Internal/AntiforgerySerializationContextPooledObjectPolicy.cs +++ /dev/null @@ -1,21 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using Microsoft.Extensions.ObjectPool; - -namespace Microsoft.AspNetCore.Antiforgery; - -internal sealed class AntiforgerySerializationContextPooledObjectPolicy : IPooledObjectPolicy -{ - public AntiforgerySerializationContext Create() - { - return new AntiforgerySerializationContext(); - } - - public bool Return(AntiforgerySerializationContext obj) - { - obj.Reset(); - - return true; - } -} diff --git a/src/Antiforgery/src/Internal/DefaultAntiforgery.cs b/src/Antiforgery/src/Internal/DefaultAntiforgery.cs index 129d1f7e3104..e4c27ccd1b73 100644 --- a/src/Antiforgery/src/Internal/DefaultAntiforgery.cs +++ b/src/Antiforgery/src/Internal/DefaultAntiforgery.cs @@ -266,7 +266,7 @@ private void CheckSSLConfig(HttpContext context) private static IAntiforgeryFeature GetAntiforgeryFeature(HttpContext httpContext) { var antiforgeryFeature = httpContext.Features.Get(); - if (antiforgeryFeature == null) + if (antiforgeryFeature is null) { antiforgeryFeature = new AntiforgeryFeature(); httpContext.Features.Set(antiforgeryFeature); diff --git a/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenGenerator.cs b/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenGenerator.cs index 1982a5bafed8..df4bab43ed65 100644 --- a/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenGenerator.cs +++ b/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenGenerator.cs @@ -10,14 +10,10 @@ namespace Microsoft.AspNetCore.Antiforgery; internal sealed class DefaultAntiforgeryTokenGenerator : IAntiforgeryTokenGenerator { - private readonly IClaimUidExtractor _claimUidExtractor; private readonly IAntiforgeryAdditionalDataProvider _additionalDataProvider; - public DefaultAntiforgeryTokenGenerator( - IClaimUidExtractor claimUidExtractor, - IAntiforgeryAdditionalDataProvider additionalDataProvider) + public DefaultAntiforgeryTokenGenerator(IAntiforgeryAdditionalDataProvider additionalDataProvider) { - _claimUidExtractor = claimUidExtractor; _additionalDataProvider = additionalDataProvider; } @@ -59,7 +55,10 @@ public AntiforgeryToken GenerateRequestToken( if (authenticatedIdentity != null) { isIdentityAuthenticated = true; - requestToken.ClaimUid = GetClaimUidBlob(_claimUidExtractor.ExtractClaimUid(httpContext.User)); + + Span claimUidBytesRent = stackalloc byte[32]; + var extractClaimUidBytesResult = ClaimUidExtractor.TryExtractClaimUidBytes(httpContext.User, claimUidBytesRent); + requestToken.ClaimUid = extractClaimUidBytesResult ? new BinaryBlob(256, claimUidBytesRent.ToArray()) : null; if (requestToken.ClaimUid == null) { @@ -151,7 +150,7 @@ public bool TryValidateTokenSet( } else { - extractedClaimUidBytes = _claimUidExtractor.TryExtractClaimUidBytes(httpContext.User, currentClaimUidBytes); + extractedClaimUidBytes = ClaimUidExtractor.TryExtractClaimUidBytes(httpContext.User, currentClaimUidBytes); } } @@ -201,16 +200,6 @@ bool AreIdenticalClaimUids(Span claimUidBytes) } } - private static BinaryBlob? GetClaimUidBlob(string? base64ClaimUid) - { - if (base64ClaimUid == null) - { - return null; - } - - return new BinaryBlob(256, Convert.FromBase64String(base64ClaimUid)); - } - private static ClaimsIdentity? GetAuthenticatedIdentity(ClaimsPrincipal? claimsPrincipal) { if (claimsPrincipal == null) @@ -218,8 +207,7 @@ bool AreIdenticalClaimUids(Span claimUidBytes) return null; } - var identitiesList = claimsPrincipal.Identities as List; - if (identitiesList != null) + if (claimsPrincipal.Identities is List identitiesList) { for (var i = 0; i < identitiesList.Count; i++) { diff --git a/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenSerializer.cs b/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenSerializer.cs index 5e9c9c59a00e..1ab53644e761 100644 --- a/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenSerializer.cs +++ b/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenSerializer.cs @@ -228,9 +228,9 @@ public string Serialize(AntiforgeryToken token) else { tokenBytes[offset++] = 0; // isClaimsBased - offset += tokenBytes.Slice(offset).Write7BitEncodedString(token.Username!); + offset += tokenBytes[offset..].Write7BitEncodedString(token.Username!); } - offset += tokenBytes.Slice(offset).Write7BitEncodedString(token.AdditionalData); + offset += tokenBytes[offset..].Write7BitEncodedString(token.AdditionalData); } _perfCryptoSystem!.Protect(tokenBytes, ref protectBuffer); diff --git a/src/Antiforgery/src/Internal/DefaultClaimUidExtractor.cs b/src/Antiforgery/src/Internal/DefaultClaimUidExtractor.cs index ce13acad75bb..92b4bf14e44e 100644 --- a/src/Antiforgery/src/Internal/DefaultClaimUidExtractor.cs +++ b/src/Antiforgery/src/Internal/DefaultClaimUidExtractor.cs @@ -6,39 +6,12 @@ using System.Security.Claims; using System.Security.Cryptography; using Microsoft.AspNetCore.Shared; -using Microsoft.Extensions.ObjectPool; namespace Microsoft.AspNetCore.Antiforgery; -/// -/// Default implementation of . -/// -internal sealed class DefaultClaimUidExtractor : IClaimUidExtractor +internal static class ClaimUidExtractor { - private readonly ObjectPool _pool; - - public DefaultClaimUidExtractor(ObjectPool pool) - { - _pool = pool; - } - - /// - public string? ExtractClaimUid(ClaimsPrincipal claimsPrincipal) - { - Debug.Assert(claimsPrincipal != null); - - var uniqueIdentifierParameters = GetUniqueIdentifierParameters(claimsPrincipal.Identities); - if (uniqueIdentifierParameters == null) - { - // No authenticated identities containing claims found. - return null; - } - - var claimUidBytes = ComputeSha256(uniqueIdentifierParameters); - return Convert.ToBase64String(claimUidBytes); - } - - public bool TryExtractClaimUidBytes(ClaimsPrincipal claimsPrincipal, Span destination) + public static bool TryExtractClaimUidBytes(ClaimsPrincipal claimsPrincipal, Span destination) { Debug.Assert(claimsPrincipal != null); @@ -173,34 +146,4 @@ private static void ComputeSha256(IList parameters, Span destinati } } } - - private byte[] ComputeSha256(IEnumerable parameters) - { - var serializationContext = _pool.Get(); - - try - { - var writer = serializationContext.Writer; - foreach (string parameter in parameters) - { - writer.Write(parameter); // also writes the length as a prefix; unambiguous - } - - writer.Flush(); - - bool success = serializationContext.Stream.TryGetBuffer(out ArraySegment buffer); - if (!success) - { - throw new InvalidOperationException(); - } - - var bytes = SHA256.HashData(buffer); - - return bytes; - } - finally - { - _pool.Return(serializationContext); - } - } } diff --git a/src/Antiforgery/src/Internal/IClaimUidExtractor.cs b/src/Antiforgery/src/Internal/IClaimUidExtractor.cs deleted file mode 100644 index 954e86f946b4..000000000000 --- a/src/Antiforgery/src/Internal/IClaimUidExtractor.cs +++ /dev/null @@ -1,27 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Security.Claims; - -namespace Microsoft.AspNetCore.Antiforgery; - -/// -/// This interface can extract unique identifers for a . -/// -internal interface IClaimUidExtractor -{ - /// - /// Extracts claims identifier. - /// - /// The . - /// The claims identifier. - string? ExtractClaimUid(ClaimsPrincipal claimsPrincipal); - - /// - /// Computes and writes the SHA256 hash of the claims identifier into the provided destination span. - /// - /// - /// SHA256 will always produce 32 bytes, so pre-allocate or stackalloc a byte span of that size. - /// - bool TryExtractClaimUidBytes(ClaimsPrincipal claimsPrincipal, Span destination); -} diff --git a/src/Antiforgery/test/DefaultAntiforgeryTokenSerializerTest.cs b/src/Antiforgery/test/DefaultAntiforgeryTokenSerializerTest.cs index aac8522a27ef..4d6bbb6c7d74 100644 --- a/src/Antiforgery/test/DefaultAntiforgeryTokenSerializerTest.cs +++ b/src/Antiforgery/test/DefaultAntiforgeryTokenSerializerTest.cs @@ -12,10 +12,8 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal; public class DefaultAntiforgeryTokenSerializerTest { private static readonly Mock _dataProtector = GetDataProtector(); - private static readonly BinaryBlob _claimUid = new BinaryBlob(256, new byte[] { 0x6F, 0x16, 0x48, 0xE9, 0x72, 0x49, 0xAA, 0x58, 0x75, 0x40, 0x36, 0xA6, 0x7E, 0x24, 0x8C, 0xF0, 0x44, 0xF0, 0x7E, 0xCF, 0xB0, 0xED, 0x38, 0x75, 0x56, 0xCE, 0x02, 0x9A, 0x4F, 0x9A, 0x40, 0xE0 }); - private static readonly BinaryBlob _securityToken = new BinaryBlob(128, new byte[] { 0x70, 0x5E, 0xED, 0xCC, 0x7D, 0x42, 0xF1, 0xD6, 0xB3, 0xB9, 0x8A, 0x59, 0x36, 0x25, 0xBB, 0x4C }); - private static readonly ObjectPool _pool = - new DefaultObjectPoolProvider().Create(new AntiforgerySerializationContextPooledObjectPolicy()); + private static readonly BinaryBlob _claimUid = new BinaryBlob(256, [0x6F, 0x16, 0x48, 0xE9, 0x72, 0x49, 0xAA, 0x58, 0x75, 0x40, 0x36, 0xA6, 0x7E, 0x24, 0x8C, 0xF0, 0x44, 0xF0, 0x7E, 0xCF, 0xB0, 0xED, 0x38, 0x75, 0x56, 0xCE, 0x02, 0x9A, 0x4F, 0x9A, 0x40, 0xE0]); + private static readonly BinaryBlob _securityToken = new BinaryBlob(128, [0x70, 0x5E, 0xED, 0xCC, 0x7D, 0x42, 0xF1, 0xD6, 0xB3, 0xB9, 0x8A, 0x59, 0x36, 0x25, 0xBB, 0x4C]); private const byte _salt = 0x05; [Theory] diff --git a/src/Antiforgery/test/DefaultClaimUidExtractorTest.cs b/src/Antiforgery/test/DefaultClaimUidExtractorTest.cs index 8c8dd66334c9..5029bae252ab 100644 --- a/src/Antiforgery/test/DefaultClaimUidExtractorTest.cs +++ b/src/Antiforgery/test/DefaultClaimUidExtractorTest.cs @@ -9,24 +9,19 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal; public class DefaultClaimUidExtractorTest { - private static readonly ObjectPool _pool = - new DefaultObjectPoolProvider().Create(new AntiforgerySerializationContextPooledObjectPolicy()); - [Fact] public void ExtractClaimUid_Unauthenticated() { - // Arrange - var extractor = new DefaultClaimUidExtractor(_pool); - var mockIdentity = new Mock(); mockIdentity.Setup(o => o.IsAuthenticated) .Returns(false); // Act - var claimUid = extractor.ExtractClaimUid(new ClaimsPrincipal(mockIdentity.Object)); + var claimUid = new byte[32]; + var result = ClaimUidExtractor.TryExtractClaimUidBytes(new ClaimsPrincipal(mockIdentity.Object), claimUid); // Assert - Assert.Null(claimUid); + Assert.False(result); } [Fact] @@ -36,16 +31,15 @@ public void ExtractClaimUid_ClaimsIdentity() var mockIdentity = new Mock(); mockIdentity.Setup(o => o.IsAuthenticated) .Returns(true); - mockIdentity.Setup(o => o.Claims).Returns(new Claim[] { new Claim(ClaimTypes.Name, "someName") }); - - var extractor = new DefaultClaimUidExtractor(_pool); + mockIdentity.Setup(o => o.Claims).Returns([new Claim(ClaimTypes.Name, "someName")]); // Act - var claimUid = extractor.ExtractClaimUid(new ClaimsPrincipal(mockIdentity.Object)); + var claimUid = new byte[32]; + var result = ClaimUidExtractor.TryExtractClaimUidBytes(new ClaimsPrincipal(mockIdentity.Object), claimUid); // Assert - Assert.NotNull(claimUid); - Assert.Equal("yhXE+2v4zSXHtRHmzm4cmrhZca2J0g7yTUwtUerdeF4=", claimUid); + Assert.True(result); + Assert.Equal("yhXE+2v4zSXHtRHmzm4cmrhZca2J0g7yTUwtUerdeF4=", Convert.ToBase64String(claimUid)); } [Fact] @@ -58,11 +52,10 @@ public void DefaultUniqueClaimTypes_NotPresent_SerializesAllClaimTypes() identity.AddClaim(new Claim(ClaimTypes.NameIdentifier, string.Empty)); // Arrange - var claimsIdentity = (ClaimsIdentity)identity; + var claimsIdentity = identity; // Act - var identiferParameters = DefaultClaimUidExtractor.GetUniqueIdentifierParameters(new ClaimsIdentity[] { claimsIdentity })! - .ToArray(); + var identiferParameters = ClaimUidExtractor.GetUniqueIdentifierParameters([claimsIdentity])!.ToArray(); var claims = claimsIdentity.Claims.ToList(); claims.Sort((a, b) => string.Compare(a.Type, b.Type, StringComparison.Ordinal)); @@ -85,7 +78,7 @@ public void DefaultUniqueClaimTypes_Present() identity.AddClaim(new Claim(ClaimTypes.NameIdentifier, "nameIdentifierValue")); // Act - var uniqueIdentifierParameters = DefaultClaimUidExtractor.GetUniqueIdentifierParameters(new ClaimsIdentity[] { identity }); + var uniqueIdentifierParameters = ClaimUidExtractor.GetUniqueIdentifierParameters([identity]); // Assert Assert.Equal(new string[] @@ -106,7 +99,7 @@ public void GetUniqueIdentifierParameters_PrefersSubClaimOverNameIdentifierAndUp identity.AddClaim(new Claim(ClaimTypes.Upn, "upnClaimValue")); // Act - var uniqueIdentifierParameters = DefaultClaimUidExtractor.GetUniqueIdentifierParameters(new ClaimsIdentity[] { identity }); + var uniqueIdentifierParameters = ClaimUidExtractor.GetUniqueIdentifierParameters([identity]); // Assert Assert.Equal(new string[] @@ -126,7 +119,7 @@ public void GetUniqueIdentifierParameters_PrefersNameIdentifierOverUpn() identity.AddClaim(new Claim(ClaimTypes.Upn, "upnClaimValue")); // Act - var uniqueIdentifierParameters = DefaultClaimUidExtractor.GetUniqueIdentifierParameters(new ClaimsIdentity[] { identity }); + var uniqueIdentifierParameters = ClaimUidExtractor.GetUniqueIdentifierParameters([identity]); // Assert Assert.Equal(new string[] @@ -146,7 +139,7 @@ public void GetUniqueIdentifierParameters_UsesUpnIfPresent() identity.AddClaim(new Claim(ClaimTypes.Upn, "upnClaimValue")); // Act - var uniqueIdentifierParameters = DefaultClaimUidExtractor.GetUniqueIdentifierParameters(new ClaimsIdentity[] { identity }); + var uniqueIdentifierParameters = ClaimUidExtractor.GetUniqueIdentifierParameters([identity]); // Assert Assert.Equal(new string[] @@ -167,7 +160,7 @@ public void GetUniqueIdentifierParameters_MultipleIdentities_UsesOnlyAuthenticat identity2.AddClaim(new Claim(ClaimTypes.NameIdentifier, "nameIdentifierValue")); // Act - var uniqueIdentifierParameters = DefaultClaimUidExtractor.GetUniqueIdentifierParameters(new ClaimsIdentity[] { identity1, identity2 }); + var uniqueIdentifierParameters = ClaimUidExtractor.GetUniqueIdentifierParameters([identity1, identity2]); // Assert Assert.Equal(new string[] @@ -192,8 +185,7 @@ public void GetUniqueIdentifierParameters_NoKnownClaimTypesFound_SortsAndReturns identity4.AddClaim(new Claim(ClaimTypes.Name, "claimName")); // Act - var uniqueIdentifierParameters = DefaultClaimUidExtractor.GetUniqueIdentifierParameters( - new ClaimsIdentity[] { identity1, identity2, identity3, identity4 }); + var uniqueIdentifierParameters = ClaimUidExtractor.GetUniqueIdentifierParameters([identity1, identity2, identity3, identity4]); // Assert Assert.Equal(new List @@ -220,8 +212,7 @@ public void GetUniqueIdentifierParameters_PrefersNameFromFirstIdentity_OverSubFr identity2.AddClaim(new Claim("sub", "subClaimValue")); // Act - var uniqueIdentifierParameters = DefaultClaimUidExtractor.GetUniqueIdentifierParameters( - new ClaimsIdentity[] { identity1, identity2 }); + var uniqueIdentifierParameters = ClaimUidExtractor.GetUniqueIdentifierParameters([identity1, identity2]); // Assert Assert.Equal(new string[] @@ -242,8 +233,7 @@ public void GetUniqueIdentifierParameters_PrefersUpnFromFirstIdentity_OverNameFr identity2.AddClaim(new Claim(ClaimTypes.NameIdentifier, "nameIdentifierValue")); // Act - var uniqueIdentifierParameters = DefaultClaimUidExtractor.GetUniqueIdentifierParameters( - new ClaimsIdentity[] { identity1, identity2 }); + var uniqueIdentifierParameters = ClaimUidExtractor.GetUniqueIdentifierParameters([identity1, identity2]); // Assert Assert.Equal(new string[] From a05bc262c7ac8bc62ed21033014e7cdc2dccf862 Mon Sep 17 00:00:00 2001 From: Korolev Dmitry Date: Fri, 12 Dec 2025 15:23:58 +0100 Subject: [PATCH 06/13] rerun benchmarks, simplify token generation --- .../Benchmarks/AntifogeryBenchmarks.cs | 4 ++-- .../AntiforgeryTokenGeneratorBenchmarks.cs | 14 +++++++------- .../AntiforgeryTokenSerializerBenchmarks.cs | 4 ++-- .../Internal/DefaultAntiforgeryTokenGenerator.cs | 6 +++--- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntifogeryBenchmarks.cs b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntifogeryBenchmarks.cs index 6d8bae219ff7..db5a6113707a 100644 --- a/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntifogeryBenchmarks.cs +++ b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntifogeryBenchmarks.cs @@ -22,8 +22,8 @@ namespace Microsoft.AspNetCore.Antiforgery.Benchmarks.Benchmarks; this PR: | Method | Mean | Error | StdDev | Op/s | Gen 0 | Gen 1 | Gen 2 | Allocated | |--------------------- |---------:|---------:|---------:|---------:|------:|------:|------:|----------:| - | GetAndStoreTokens | 46.66 us | 2.150 us | 5.958 us | 21,432.4 | - | - | - | 4 KB | - | ValidateRequestAsync | 44.79 us | 2.279 us | 6.391 us | 22,327.2 | - | - | - | 3 KB | + | GetAndStoreTokens | 49.62 us | 1.386 us | 3.954 us | 20,153.9 | - | - | - | 3 KB | + | ValidateRequestAsync | 43.67 us | 1.541 us | 4.471 us | 22,900.6 | - | - | - | 3 KB | */ diff --git a/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenGeneratorBenchmarks.cs b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenGeneratorBenchmarks.cs index 122d01f36ee3..cc30bd2a68d9 100644 --- a/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenGeneratorBenchmarks.cs +++ b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenGeneratorBenchmarks.cs @@ -19,13 +19,13 @@ namespace Microsoft.AspNetCore.Antiforgery.Benchmarks.Benchmarks; | TryValidateTokenSet_ClaimsBased | 308.4674 ns | 3.3256 ns | 3.1108 ns | 3,241,833.1 | 0.0038 | - | - | 312 B | this PR: - | Method | Mean | Error | StdDev | Median | Op/s | Gen 0 | Gen 1 | Gen 2 | Allocated | - |----------------------------------- |-----------:|-----------:|-----------:|-----------:|--------------:|-------:|------:|------:|----------:| - | GenerateRequestToken_Anonymous | 17.814 ns | 0.3614 ns | 0.4017 ns | 17.771 ns | 56,134,225.3 | 0.0007 | - | - | 56 B | - | GenerateRequestToken_Authenticated | 459.848 ns | 9.1450 ns | 9.7851 ns | 461.283 ns | 2,174,632.5 | 0.0052 | - | - | 424 B | - | TryValidateTokenSet_Anonymous | 9.784 ns | 0.6728 ns | 1.9837 ns | 10.357 ns | 102,207,305.5 | - | - | - | - | - | TryValidateTokenSet_Authenticated | 17.300 ns | 0.4886 ns | 1.4407 ns | 17.929 ns | 57,804,630.5 | - | - | - | - | - | TryValidateTokenSet_ClaimsBased | 380.134 ns | 12.9953 ns | 38.3168 ns | 394.899 ns | 2,630,649.7 | 0.0014 | - | - | 120 B | + | Method | Mean | Error | StdDev | Op/s | Gen 0 | Gen 1 | Gen 2 | Allocated | + |----------------------------------- |------------:|----------:|-----------:|--------------:|-------:|------:|------:|----------:| + | GenerateRequestToken_Anonymous | 11.190 ns | 0.2428 ns | 0.6046 ns | 89,364,681.9 | 0.0007 | - | - | 56 B | + | GenerateRequestToken_Authenticated | 338.056 ns | 6.7313 ns | 14.9161 ns | 2,958,092.2 | 0.0052 | - | - | 424 B | + | TryValidateTokenSet_Anonymous | 7.966 ns | 0.1616 ns | 0.2915 ns | 125,531,038.3 | - | - | - | - | + | TryValidateTokenSet_Authenticated | 13.386 ns | 0.2476 ns | 0.3550 ns | 74,707,554.5 | - | - | - | - | + | TryValidateTokenSet_ClaimsBased | 220.111 ns | 4.2723 ns | 5.7034 ns | 4,543,156.3 | 0.0014 | - | - | 120 B | */ diff --git a/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenSerializerBenchmarks.cs b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenSerializerBenchmarks.cs index a5b73dab2f3b..7ddc2581bc1e 100644 --- a/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenSerializerBenchmarks.cs +++ b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenSerializerBenchmarks.cs @@ -15,11 +15,11 @@ namespace Microsoft.AspNetCore.Antiforgery.Benchmarks.Benchmarks; This PR: | Method | Mean | Error | StdDev | Op/s | Gen 0 | Gen 1 | Gen 2 | Allocated | - |------------ |---------:|----------:|----------:|----------:|-------:|------:|------:|----------:| + |------------ |---------:|----------:|----------:| ----------:|-------:|------:|------:|----------:| | Serialize | 1.932 us | 0.0110 us | 0.0098 us | 517,688.9 | 0.0038 | - | - | 544 B | | Deserialize | 1.927 us | 0.0107 us | 0.0100 us | 519,058.2 | 0.0038 | - | - | 344 B | */ - + [AspNetCoreBenchmark] public class AntiforgeryTokenSerializerBenchmarks { diff --git a/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenGenerator.cs b/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenGenerator.cs index df4bab43ed65..ce2dfd812b5c 100644 --- a/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenGenerator.cs +++ b/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenGenerator.cs @@ -56,9 +56,9 @@ public AntiforgeryToken GenerateRequestToken( { isIdentityAuthenticated = true; - Span claimUidBytesRent = stackalloc byte[32]; - var extractClaimUidBytesResult = ClaimUidExtractor.TryExtractClaimUidBytes(httpContext.User, claimUidBytesRent); - requestToken.ClaimUid = extractClaimUidBytesResult ? new BinaryBlob(256, claimUidBytesRent.ToArray()) : null; + var claimUidBytes = new byte[32]; + var extractClaimUidBytesResult = ClaimUidExtractor.TryExtractClaimUidBytes(httpContext.User, claimUidBytes); + requestToken.ClaimUid = extractClaimUidBytesResult ? new BinaryBlob(256, claimUidBytes) : null; if (requestToken.ClaimUid == null) { From da95fcbd7a1adffe73277aed4e7fc1e9ec5427d1 Mon Sep 17 00:00:00 2001 From: Korolev Dmitry Date: Fri, 12 Dec 2025 17:09:09 +0100 Subject: [PATCH 07/13] rollback interface --- .../AntiforgeryTokenGeneratorBenchmarks.cs | 2 +- .../AntiforgeryTokenSerializerBenchmarks.cs | 2 +- .../AntiforgeryServiceCollectionExtensions.cs | 1 + .../DefaultAntiforgeryTokenGenerator.cs | 10 +++-- .../src/Internal/DefaultClaimUidExtractor.cs | 7 +++- .../src/Internal/IClaimUidExtractor.cs | 17 ++++++++ .../DefaultAntiforgeryTokenGeneratorTest.cs | 39 ++++++------------- .../test/DefaultClaimUidExtractorTest.cs | 29 ++++++++------ 8 files changed, 62 insertions(+), 45 deletions(-) create mode 100644 src/Antiforgery/src/Internal/IClaimUidExtractor.cs diff --git a/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenGeneratorBenchmarks.cs b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenGeneratorBenchmarks.cs index cc30bd2a68d9..1c50584f7d0d 100644 --- a/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenGeneratorBenchmarks.cs +++ b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenGeneratorBenchmarks.cs @@ -100,7 +100,7 @@ [new Claim(ClaimsIdentity.DefaultNameClaimType, "testuser@example.com")], // For claims-based users, we need to extract the ClaimUid var claimUid = new byte[32]; - _ = ClaimUidExtractor.TryExtractClaimUidBytes(_claimsHttpContext.User, claimUid); + _ = new DefaultClaimUidExtractor().TryExtractClaimUidBytes(_claimsHttpContext.User, claimUid); _claimsRequestToken = new AntiforgeryToken { IsCookieToken = false, diff --git a/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenSerializerBenchmarks.cs b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenSerializerBenchmarks.cs index 7ddc2581bc1e..32580f6309af 100644 --- a/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenSerializerBenchmarks.cs +++ b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenSerializerBenchmarks.cs @@ -15,7 +15,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Benchmarks.Benchmarks; This PR: | Method | Mean | Error | StdDev | Op/s | Gen 0 | Gen 1 | Gen 2 | Allocated | - |------------ |---------:|----------:|----------:| ----------:|-------:|------:|------:|----------:| + |------------ |---------:|----------:|----------:| ---------:|-------:|------:|------:|----------:| | Serialize | 1.932 us | 0.0110 us | 0.0098 us | 517,688.9 | 0.0038 | - | - | 544 B | | Deserialize | 1.927 us | 0.0107 us | 0.0100 us | 519,058.2 | 0.0038 | - | - | 344 B | */ diff --git a/src/Antiforgery/src/AntiforgeryServiceCollectionExtensions.cs b/src/Antiforgery/src/AntiforgeryServiceCollectionExtensions.cs index 61adcf7e55c0..d2cd8332f2ac 100644 --- a/src/Antiforgery/src/AntiforgeryServiceCollectionExtensions.cs +++ b/src/Antiforgery/src/AntiforgeryServiceCollectionExtensions.cs @@ -31,6 +31,7 @@ public static IServiceCollection AddAntiforgery(this IServiceCollection services services.TryAddSingleton(); services.TryAddSingleton(); services.TryAddSingleton(); + services.TryAddSingleton(); services.TryAddSingleton(); return services; diff --git a/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenGenerator.cs b/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenGenerator.cs index ce2dfd812b5c..0c0b33cce2c5 100644 --- a/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenGenerator.cs +++ b/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenGenerator.cs @@ -10,10 +10,14 @@ namespace Microsoft.AspNetCore.Antiforgery; internal sealed class DefaultAntiforgeryTokenGenerator : IAntiforgeryTokenGenerator { + private readonly IClaimUidExtractor _claimUidExtractor; private readonly IAntiforgeryAdditionalDataProvider _additionalDataProvider; - public DefaultAntiforgeryTokenGenerator(IAntiforgeryAdditionalDataProvider additionalDataProvider) + public DefaultAntiforgeryTokenGenerator( + IClaimUidExtractor claimUidExtractor, + IAntiforgeryAdditionalDataProvider additionalDataProvider) { + _claimUidExtractor = claimUidExtractor; _additionalDataProvider = additionalDataProvider; } @@ -57,7 +61,7 @@ public AntiforgeryToken GenerateRequestToken( isIdentityAuthenticated = true; var claimUidBytes = new byte[32]; - var extractClaimUidBytesResult = ClaimUidExtractor.TryExtractClaimUidBytes(httpContext.User, claimUidBytes); + var extractClaimUidBytesResult = _claimUidExtractor.TryExtractClaimUidBytes(httpContext.User, claimUidBytes); requestToken.ClaimUid = extractClaimUidBytesResult ? new BinaryBlob(256, claimUidBytes) : null; if (requestToken.ClaimUid == null) @@ -150,7 +154,7 @@ public bool TryValidateTokenSet( } else { - extractedClaimUidBytes = ClaimUidExtractor.TryExtractClaimUidBytes(httpContext.User, currentClaimUidBytes); + extractedClaimUidBytes = _claimUidExtractor.TryExtractClaimUidBytes(httpContext.User, currentClaimUidBytes); } } diff --git a/src/Antiforgery/src/Internal/DefaultClaimUidExtractor.cs b/src/Antiforgery/src/Internal/DefaultClaimUidExtractor.cs index 92b4bf14e44e..79603196fcf1 100644 --- a/src/Antiforgery/src/Internal/DefaultClaimUidExtractor.cs +++ b/src/Antiforgery/src/Internal/DefaultClaimUidExtractor.cs @@ -9,9 +9,12 @@ namespace Microsoft.AspNetCore.Antiforgery; -internal static class ClaimUidExtractor +/// +/// Default implementation of . +/// +internal sealed class DefaultClaimUidExtractor : IClaimUidExtractor { - public static bool TryExtractClaimUidBytes(ClaimsPrincipal claimsPrincipal, Span destination) + public bool TryExtractClaimUidBytes(ClaimsPrincipal claimsPrincipal, Span destination) { Debug.Assert(claimsPrincipal != null); diff --git a/src/Antiforgery/src/Internal/IClaimUidExtractor.cs b/src/Antiforgery/src/Internal/IClaimUidExtractor.cs new file mode 100644 index 000000000000..f16ed16a2cf8 --- /dev/null +++ b/src/Antiforgery/src/Internal/IClaimUidExtractor.cs @@ -0,0 +1,17 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Security.Claims; + +namespace Microsoft.AspNetCore.Antiforgery; + +/// +/// This interface can extract unique identifers for a . +/// +internal interface IClaimUidExtractor +{ + /// + /// Extracts claims identifier, and writes into buffer. + /// + bool TryExtractClaimUidBytes(ClaimsPrincipal claimsPrincipal, Span destination); +} diff --git a/src/Antiforgery/test/DefaultAntiforgeryTokenGeneratorTest.cs b/src/Antiforgery/test/DefaultAntiforgeryTokenGeneratorTest.cs index 8520020496a2..ae2b0b4c2a33 100644 --- a/src/Antiforgery/test/DefaultAntiforgeryTokenGeneratorTest.cs +++ b/src/Antiforgery/test/DefaultAntiforgeryTokenGeneratorTest.cs @@ -153,13 +153,8 @@ public void GenerateRequestToken_ClaimsBasedIdentity() var base64ClaimUId = Convert.ToBase64String(data); var expectedClaimUid = new BinaryBlob(256, data); - var mockClaimUidExtractor = new Mock(); - mockClaimUidExtractor.Setup(o => o.ExtractClaimUid(It.Is(c => c.Identity == identity))) - .Returns(base64ClaimUId); - - var tokenProvider = new DefaultAntiforgeryTokenGenerator( - claimUidExtractor: mockClaimUidExtractor.Object, - additionalDataProvider: null); + var claimUidExtractor = new DummyClaimUidExtractor(identity, expectedClaimUid); + var tokenProvider = new DefaultAntiforgeryTokenGenerator(claimUidExtractor, additionalDataProvider: null); // Act var fieldToken = tokenProvider.GenerateRequestToken(httpContext, cookieToken); @@ -402,21 +397,15 @@ public void TryValidateTokenSet_UsernameMismatch(string identityUsername, string IsCookieToken = false }; - var mockClaimUidExtractor = new Mock(); - mockClaimUidExtractor.Setup(o => o.ExtractClaimUid(It.Is(c => c.Identity == identity))) - .Returns((string)null); + var claimUidExtractor = new DummyClaimUidExtractor(identity, null, failsExtraction: true); + var tokenProvider = new DefaultAntiforgeryTokenGenerator(claimUidExtractor, additionalDataProvider: null); - var tokenProvider = new DefaultAntiforgeryTokenGenerator( - claimUidExtractor: mockClaimUidExtractor.Object, - additionalDataProvider: null); - - string expectedMessage = + var expectedMessage = $"The provided antiforgery token was meant for user \"{embeddedUsername}\", " + $"but the current user is \"{identityUsername}\"."; // Act - string message; - var result = tokenProvider.TryValidateTokenSet(httpContext, cookieToken, fieldtoken, out message); + var result = tokenProvider.TryValidateTokenSet(httpContext, cookieToken, fieldtoken, out var message); // Assert Assert.False(result); @@ -612,26 +601,22 @@ private class DummyClaimUidExtractor : IClaimUidExtractor { private readonly IIdentity _identity; private readonly BinaryBlob _differentToken; + private readonly bool _failsExtraction; - public DummyClaimUidExtractor(IIdentity identity, BinaryBlob blob) + public DummyClaimUidExtractor(IIdentity identity, BinaryBlob blob, bool failsExtraction = false) { _identity = identity; _differentToken = blob; + _failsExtraction = failsExtraction; } - public string ExtractClaimUid(ClaimsPrincipal claimsPrincipal) + public bool TryExtractClaimUidBytes(ClaimsPrincipal claimsPrincipal, Span destination) { - if (claimsPrincipal.Identity == _identity) + if (_failsExtraction) { - return Convert.ToBase64String(_differentToken.GetData()); + return false; } - Assert.Fail("Unexpected identity"); - return default; - } - - public bool TryExtractClaimUidBytes(ClaimsPrincipal claimsPrincipal, Span destination) - { if (claimsPrincipal.Identity == _identity) { _differentToken.GetData().CopyTo(destination); diff --git a/src/Antiforgery/test/DefaultClaimUidExtractorTest.cs b/src/Antiforgery/test/DefaultClaimUidExtractorTest.cs index 5029bae252ab..cbc1f9aa9121 100644 --- a/src/Antiforgery/test/DefaultClaimUidExtractorTest.cs +++ b/src/Antiforgery/test/DefaultClaimUidExtractorTest.cs @@ -9,6 +9,13 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal; public class DefaultClaimUidExtractorTest { + private readonly DefaultClaimUidExtractor _claimUidExtractor; + + public DefaultClaimUidExtractorTest() + { + _claimUidExtractor = new DefaultClaimUidExtractor(); + } + [Fact] public void ExtractClaimUid_Unauthenticated() { @@ -18,7 +25,7 @@ public void ExtractClaimUid_Unauthenticated() // Act var claimUid = new byte[32]; - var result = ClaimUidExtractor.TryExtractClaimUidBytes(new ClaimsPrincipal(mockIdentity.Object), claimUid); + var result = _claimUidExtractor.TryExtractClaimUidBytes(new ClaimsPrincipal(mockIdentity.Object), claimUid); // Assert Assert.False(result); @@ -35,7 +42,7 @@ public void ExtractClaimUid_ClaimsIdentity() // Act var claimUid = new byte[32]; - var result = ClaimUidExtractor.TryExtractClaimUidBytes(new ClaimsPrincipal(mockIdentity.Object), claimUid); + var result = _claimUidExtractor.TryExtractClaimUidBytes(new ClaimsPrincipal(mockIdentity.Object), claimUid); // Assert Assert.True(result); @@ -55,7 +62,7 @@ public void DefaultUniqueClaimTypes_NotPresent_SerializesAllClaimTypes() var claimsIdentity = identity; // Act - var identiferParameters = ClaimUidExtractor.GetUniqueIdentifierParameters([claimsIdentity])!.ToArray(); + var identiferParameters = DefaultClaimUidExtractor.GetUniqueIdentifierParameters([claimsIdentity])!.ToArray(); var claims = claimsIdentity.Claims.ToList(); claims.Sort((a, b) => string.Compare(a.Type, b.Type, StringComparison.Ordinal)); @@ -78,7 +85,7 @@ public void DefaultUniqueClaimTypes_Present() identity.AddClaim(new Claim(ClaimTypes.NameIdentifier, "nameIdentifierValue")); // Act - var uniqueIdentifierParameters = ClaimUidExtractor.GetUniqueIdentifierParameters([identity]); + var uniqueIdentifierParameters = DefaultClaimUidExtractor.GetUniqueIdentifierParameters([identity]); // Assert Assert.Equal(new string[] @@ -99,7 +106,7 @@ public void GetUniqueIdentifierParameters_PrefersSubClaimOverNameIdentifierAndUp identity.AddClaim(new Claim(ClaimTypes.Upn, "upnClaimValue")); // Act - var uniqueIdentifierParameters = ClaimUidExtractor.GetUniqueIdentifierParameters([identity]); + var uniqueIdentifierParameters = DefaultClaimUidExtractor.GetUniqueIdentifierParameters([identity]); // Assert Assert.Equal(new string[] @@ -119,7 +126,7 @@ public void GetUniqueIdentifierParameters_PrefersNameIdentifierOverUpn() identity.AddClaim(new Claim(ClaimTypes.Upn, "upnClaimValue")); // Act - var uniqueIdentifierParameters = ClaimUidExtractor.GetUniqueIdentifierParameters([identity]); + var uniqueIdentifierParameters = DefaultClaimUidExtractor.GetUniqueIdentifierParameters([identity]); // Assert Assert.Equal(new string[] @@ -139,7 +146,7 @@ public void GetUniqueIdentifierParameters_UsesUpnIfPresent() identity.AddClaim(new Claim(ClaimTypes.Upn, "upnClaimValue")); // Act - var uniqueIdentifierParameters = ClaimUidExtractor.GetUniqueIdentifierParameters([identity]); + var uniqueIdentifierParameters = DefaultClaimUidExtractor.GetUniqueIdentifierParameters([identity]); // Assert Assert.Equal(new string[] @@ -160,7 +167,7 @@ public void GetUniqueIdentifierParameters_MultipleIdentities_UsesOnlyAuthenticat identity2.AddClaim(new Claim(ClaimTypes.NameIdentifier, "nameIdentifierValue")); // Act - var uniqueIdentifierParameters = ClaimUidExtractor.GetUniqueIdentifierParameters([identity1, identity2]); + var uniqueIdentifierParameters = DefaultClaimUidExtractor.GetUniqueIdentifierParameters([identity1, identity2]); // Assert Assert.Equal(new string[] @@ -185,7 +192,7 @@ public void GetUniqueIdentifierParameters_NoKnownClaimTypesFound_SortsAndReturns identity4.AddClaim(new Claim(ClaimTypes.Name, "claimName")); // Act - var uniqueIdentifierParameters = ClaimUidExtractor.GetUniqueIdentifierParameters([identity1, identity2, identity3, identity4]); + var uniqueIdentifierParameters = DefaultClaimUidExtractor.GetUniqueIdentifierParameters([identity1, identity2, identity3, identity4]); // Assert Assert.Equal(new List @@ -212,7 +219,7 @@ public void GetUniqueIdentifierParameters_PrefersNameFromFirstIdentity_OverSubFr identity2.AddClaim(new Claim("sub", "subClaimValue")); // Act - var uniqueIdentifierParameters = ClaimUidExtractor.GetUniqueIdentifierParameters([identity1, identity2]); + var uniqueIdentifierParameters = DefaultClaimUidExtractor.GetUniqueIdentifierParameters([identity1, identity2]); // Assert Assert.Equal(new string[] @@ -233,7 +240,7 @@ public void GetUniqueIdentifierParameters_PrefersUpnFromFirstIdentity_OverNameFr identity2.AddClaim(new Claim(ClaimTypes.NameIdentifier, "nameIdentifierValue")); // Act - var uniqueIdentifierParameters = ClaimUidExtractor.GetUniqueIdentifierParameters([identity1, identity2]); + var uniqueIdentifierParameters = DefaultClaimUidExtractor.GetUniqueIdentifierParameters([identity1, identity2]); // Assert Assert.Equal(new string[] From 9e7910558a4eb8b15e41088ef64fd87037366d19 Mon Sep 17 00:00:00 2001 From: Korolev Dmitry Date: Fri, 12 Dec 2025 17:48:56 +0100 Subject: [PATCH 08/13] copilot feedback --- ...Benchmarks.cs => AntiforgeryBenchmarks.cs} | 0 .../DefaultAntiforgeryTokenSerializer.cs | 52 ++++++++++++++----- .../src/Internal/DefaultClaimUidExtractor.cs | 4 +- .../DefaultAntiforgeryTokenGeneratorTest.cs | 6 +-- src/Shared/Encoding/Int7BitEncodingUtils.cs | 12 ++--- .../Encoding/Int7BitEncodingUtilsTests.cs | 13 +++++ 6 files changed, 61 insertions(+), 26 deletions(-) rename src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/{AntifogeryBenchmarks.cs => AntiforgeryBenchmarks.cs} (100%) diff --git a/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntifogeryBenchmarks.cs b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryBenchmarks.cs similarity index 100% rename from src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntifogeryBenchmarks.cs rename to src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryBenchmarks.cs diff --git a/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenSerializer.cs b/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenSerializer.cs index 1ab53644e761..55e88014857d 100644 --- a/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenSerializer.cs +++ b/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenSerializer.cs @@ -45,20 +45,31 @@ public AntiforgeryToken Deserialize(string serializedToken) var tokenBytesDecoded = tokenBytes.Slice(0, bytesWritten); - var protectBuffer = new RefPooledArrayBufferWriter(stackalloc byte[255]); - try + if (_perfCryptoSystem is not null) { - _perfCryptoSystem!.Unprotect(tokenBytesDecoded, ref protectBuffer); - - var token = Deserialize(protectBuffer.WrittenSpan); - if (token != null) + var protectBuffer = new RefPooledArrayBufferWriter(stackalloc byte[255]); + try { - return token; + _perfCryptoSystem!.Unprotect(tokenBytesDecoded, ref protectBuffer); + var token = Deserialize(protectBuffer.WrittenSpan); + if (token is not null) + { + return token; + } + } + finally + { + protectBuffer.Dispose(); } } - finally + else { - protectBuffer.Dispose(); + var unprotectedBytes = _defaultCryptoSystem.Unprotect(tokenBytesDecoded.ToArray()); + var token = Deserialize(unprotectedBytes); + if (token is not null) + { + return token; + } } } catch (Exception ex) @@ -202,7 +213,6 @@ public string Serialize(AntiforgeryToken token) } byte[]? tokenBytesRent = null; - var protectBuffer = new RefPooledArrayBufferWriter(stackalloc byte[255]); var rent = totalSize < 256 ? stackalloc byte[255] @@ -233,8 +243,24 @@ public string Serialize(AntiforgeryToken token) offset += tokenBytes[offset..].Write7BitEncodedString(token.AdditionalData); } - _perfCryptoSystem!.Protect(tokenBytes, ref protectBuffer); - return Base64Url.EncodeToString(protectBuffer.WrittenSpan); + if (_perfCryptoSystem is not null) + { + var protectBuffer = new RefPooledArrayBufferWriter(stackalloc byte[255]); + try + { + _perfCryptoSystem!.Protect(tokenBytes, ref protectBuffer); + return Base64Url.EncodeToString(protectBuffer.WrittenSpan); + } + finally + { + protectBuffer.Dispose(); + } + } + else + { + var protectedBytes = _defaultCryptoSystem.Protect(tokenBytes.ToArray()); + return Base64Url.EncodeToString(protectedBytes); + } } finally { @@ -242,8 +268,6 @@ public string Serialize(AntiforgeryToken token) { ArrayPool.Shared.Return(tokenBytesRent); } - - protectBuffer.Dispose(); } } } diff --git a/src/Antiforgery/src/Internal/DefaultClaimUidExtractor.cs b/src/Antiforgery/src/Internal/DefaultClaimUidExtractor.cs index 79603196fcf1..a4d79c38cad0 100644 --- a/src/Antiforgery/src/Internal/DefaultClaimUidExtractor.cs +++ b/src/Antiforgery/src/Internal/DefaultClaimUidExtractor.cs @@ -24,9 +24,7 @@ public bool TryExtractClaimUidBytes(ClaimsPrincipal claimsPrincipal, Span return false; } - // SHA256 always produces 32 bytes - Span claimUidBytes = stackalloc byte[32]; - ComputeSha256(uniqueIdentifierParameters, claimUidBytes); + ComputeSha256(uniqueIdentifierParameters, destination); return true; } diff --git a/src/Antiforgery/test/DefaultAntiforgeryTokenGeneratorTest.cs b/src/Antiforgery/test/DefaultAntiforgeryTokenGeneratorTest.cs index ae2b0b4c2a33..0e1fea0d2bf7 100644 --- a/src/Antiforgery/test/DefaultAntiforgeryTokenGeneratorTest.cs +++ b/src/Antiforgery/test/DefaultAntiforgeryTokenGeneratorTest.cs @@ -86,7 +86,7 @@ public void GenerateRequestToken_AuthenticatedWithoutUsernameAndNoAdditionalData httpContext.User = new ClaimsPrincipal(new MyAuthenticatedIdentityWithoutUsername()); var options = new AntiforgeryOptions(); - var claimUidExtractor = new Mock().Object; + var claimUidExtractor = new DummyClaimUidExtractor(httpContext.User.Identity!, blob: null, failsExtraction: true); var tokenProvider = new DefaultAntiforgeryTokenGenerator( claimUidExtractor: claimUidExtractor, @@ -120,7 +120,7 @@ public void GenerateRequestToken_AuthenticatedWithoutUsername_WithAdditionalData mockAdditionalDataProvider.Setup(o => o.GetAdditionalData(httpContext)) .Returns("additional-data"); - var claimUidExtractor = new Mock().Object; + var claimUidExtractor = new DummyClaimUidExtractor(httpContext.User.Identity!, blob: null, failsExtraction: true); var tokenProvider = new DefaultAntiforgeryTokenGenerator( claimUidExtractor: claimUidExtractor, @@ -183,7 +183,7 @@ public void GenerateRequestToken_RegularUserWithUsername() httpContext.User = new ClaimsPrincipal(mockIdentity.Object); - var claimUidExtractor = new Mock().Object; + var claimUidExtractor = new DummyClaimUidExtractor(httpContext.User.Identity!, blob: null, failsExtraction: true); var tokenProvider = new DefaultAntiforgeryTokenGenerator( claimUidExtractor: claimUidExtractor, diff --git a/src/Shared/Encoding/Int7BitEncodingUtils.cs b/src/Shared/Encoding/Int7BitEncodingUtils.cs index 933bd6907e83..9d6f870933d1 100644 --- a/src/Shared/Encoding/Int7BitEncodingUtils.cs +++ b/src/Shared/Encoding/Int7BitEncodingUtils.cs @@ -124,12 +124,12 @@ internal static int Measure7BitEncodedStringLength(string value) } /// - /// Returns consumed length + /// Reads a 7-bit length-prefixed UTF-8 encoded string from the specified byte span and returns the number of bytes consumed. /// - /// - /// - /// - /// + /// The span of bytes containing the 7-bit encoded length and UTF-8 encoded string data. + /// When this method returns, contains the decoded string value, or if the length is zero. + /// The total number of bytes consumed from to read the string. + /// Thrown if the encoded length is greater than the available bytes in . internal static int Read7BitEncodedString(this ReadOnlySpan bytes, out string value) { value = string.Empty; @@ -139,7 +139,7 @@ internal static int Read7BitEncodedString(this ReadOnlySpan bytes, out str return consumed; } - if (bytes.Length < length) + if (bytes.Length < consumed + length) { throw new FormatException("Bad 7-bit encoded string."); } diff --git a/src/Shared/test/Shared.Tests/Encoding/Int7BitEncodingUtilsTests.cs b/src/Shared/test/Shared.Tests/Encoding/Int7BitEncodingUtilsTests.cs index 74e46d173993..6ef62c0625b3 100644 --- a/src/Shared/test/Shared.Tests/Encoding/Int7BitEncodingUtilsTests.cs +++ b/src/Shared/test/Shared.Tests/Encoding/Int7BitEncodingUtilsTests.cs @@ -171,6 +171,19 @@ public void Read7BitEncodedString_WithTruncatedStringData_ThrowsFormatException( }); } + [Fact] + public void Read7BitEncodedString_WithMultiByteLengthPrefixAndTruncatedData_ThrowsFormatException() + { + Assert.Throws(() => + { + // Length prefix 0xC8 0x01 = 200 (multi-byte), but only 2 bytes of string data follow + // Total: 4 bytes, but need 2 (prefix) + 200 (data) = 202 bytes + ReadOnlySpan source = [0xC8, 0x01, 0x41, 0x42]; + + return source.Read7BitEncodedString(out _); + }); + } + [Fact] public void Read7BitEncodedString_WithTruncatedLengthPrefix_ThrowsFormatException() { From c4bd92f34d1d3dbb816cc2c9ecc575fa4528addc Mon Sep 17 00:00:00 2001 From: Korolev Dmitry Date: Fri, 12 Dec 2025 18:02:12 +0100 Subject: [PATCH 09/13] cleanup comments --- .../Benchmarks/AntiforgeryBenchmarks.cs | 16 -------------- .../AntiforgeryTokenGeneratorBenchmarks.cs | 21 ------------------- .../AntiforgeryTokenSerializerBenchmarks.cs | 16 +------------- 3 files changed, 1 insertion(+), 52 deletions(-) diff --git a/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryBenchmarks.cs b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryBenchmarks.cs index db5a6113707a..ef35cb147e0e 100644 --- a/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryBenchmarks.cs +++ b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryBenchmarks.cs @@ -11,22 +11,6 @@ namespace Microsoft.AspNetCore.Antiforgery.Benchmarks.Benchmarks; -/* - - main branch: - | Method | Mean | Error | StdDev | Op/s | Gen 0 | Gen 1 | Gen 2 | Allocated | - |--------------------- |---------:|---------:|---------:|---------:|------:|------:|------:|----------:| - | GetAndStoreTokens | 59.56 us | 2.482 us | 7.082 us | 16,789.6 | - | - | - | 5 KB | - | ValidateRequestAsync | 50.60 us | 2.150 us | 6.167 us | 19,764.1 | - | - | - | 4 KB | - - this PR: - | Method | Mean | Error | StdDev | Op/s | Gen 0 | Gen 1 | Gen 2 | Allocated | - |--------------------- |---------:|---------:|---------:|---------:|------:|------:|------:|----------:| - | GetAndStoreTokens | 49.62 us | 1.386 us | 3.954 us | 20,153.9 | - | - | - | 3 KB | - | ValidateRequestAsync | 43.67 us | 1.541 us | 4.471 us | 22,900.6 | - | - | - | 3 KB | - - */ - [AspNetCoreBenchmark] public class AntiforgeryBenchmarks { diff --git a/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenGeneratorBenchmarks.cs b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenGeneratorBenchmarks.cs index 1c50584f7d0d..70d6edb70526 100644 --- a/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenGeneratorBenchmarks.cs +++ b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenGeneratorBenchmarks.cs @@ -8,27 +8,6 @@ namespace Microsoft.AspNetCore.Antiforgery.Benchmarks.Benchmarks; -/* - main branch: - | Method | Mean | Error | StdDev | Op/s | Gen 0 | Gen 1 | Gen 2 | Allocated | - |----------------------------------- |------------:|----------:|----------:|----------------:|-------:|------:|------:|----------:| - | GenerateRequestToken_Anonymous | 11.0555 ns | 0.1203 ns | 0.1066 ns | 90,452,434.9 | 0.0007 | - | - | 56 B | - | GenerateRequestToken_Authenticated | 401.2545 ns | 7.1693 ns | 6.3554 ns | 2,492,184.2 | 0.0076 | - | - | 592 B | - | TryValidateTokenSet_Anonymous | 6.7227 ns | 0.0357 ns | 0.0316 ns | 148,750,552.9 | - | - | - | - | - | TryValidateTokenSet_Authenticated | 508.1742 ns | 4.4728 ns | 3.7350 ns | 1,967,829.1 | 0.0095 | - | - | 760 B | - | TryValidateTokenSet_ClaimsBased | 308.4674 ns | 3.3256 ns | 3.1108 ns | 3,241,833.1 | 0.0038 | - | - | 312 B | - - this PR: - | Method | Mean | Error | StdDev | Op/s | Gen 0 | Gen 1 | Gen 2 | Allocated | - |----------------------------------- |------------:|----------:|-----------:|--------------:|-------:|------:|------:|----------:| - | GenerateRequestToken_Anonymous | 11.190 ns | 0.2428 ns | 0.6046 ns | 89,364,681.9 | 0.0007 | - | - | 56 B | - | GenerateRequestToken_Authenticated | 338.056 ns | 6.7313 ns | 14.9161 ns | 2,958,092.2 | 0.0052 | - | - | 424 B | - | TryValidateTokenSet_Anonymous | 7.966 ns | 0.1616 ns | 0.2915 ns | 125,531,038.3 | - | - | - | - | - | TryValidateTokenSet_Authenticated | 13.386 ns | 0.2476 ns | 0.3550 ns | 74,707,554.5 | - | - | - | - | - | TryValidateTokenSet_ClaimsBased | 220.111 ns | 4.2723 ns | 5.7034 ns | 4,543,156.3 | 0.0014 | - | - | 120 B | - - */ - [AspNetCoreBenchmark] public class AntiforgeryTokenGeneratorBenchmarks { diff --git a/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenSerializerBenchmarks.cs b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenSerializerBenchmarks.cs index 32580f6309af..3ea2e5278f88 100644 --- a/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenSerializerBenchmarks.cs +++ b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryTokenSerializerBenchmarks.cs @@ -5,21 +5,7 @@ using Microsoft.Extensions.DependencyInjection; namespace Microsoft.AspNetCore.Antiforgery.Benchmarks.Benchmarks; - -/* - main branch: - | Method | Mean | Error | StdDev | Op/s | Gen 0 | Gen 1 | Gen 2 | Allocated | - |------------ |---------:|----------:|----------:|----------:|-------:|------:|------:|----------:| - | Serialize | 2.221 us | 0.0245 us | 0.0229 us | 450,239.5 | 0.0076 | - | - | 872 B | - | Deserialize | 2.436 us | 0.0463 us | 0.1036 us | 410,492.5 | 0.0076 | - | - | 632 B | - - This PR: - | Method | Mean | Error | StdDev | Op/s | Gen 0 | Gen 1 | Gen 2 | Allocated | - |------------ |---------:|----------:|----------:| ---------:|-------:|------:|------:|----------:| - | Serialize | 1.932 us | 0.0110 us | 0.0098 us | 517,688.9 | 0.0038 | - | - | 544 B | - | Deserialize | 1.927 us | 0.0107 us | 0.0100 us | 519,058.2 | 0.0038 | - | - | 344 B | - */ - + [AspNetCoreBenchmark] public class AntiforgeryTokenSerializerBenchmarks { From c218d52b2c52d48a3754164c63d3d070577d1b77 Mon Sep 17 00:00:00 2001 From: Korolev Dmitry Date: Fri, 12 Dec 2025 18:06:07 +0100 Subject: [PATCH 10/13] try fix for non-netcore --- src/Shared/Encoding/Int7BitEncodingUtils.cs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/Shared/Encoding/Int7BitEncodingUtils.cs b/src/Shared/Encoding/Int7BitEncodingUtils.cs index 9d6f870933d1..06733395f096 100644 --- a/src/Shared/Encoding/Int7BitEncodingUtils.cs +++ b/src/Shared/Encoding/Int7BitEncodingUtils.cs @@ -1,11 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; -using System.Collections.Generic; -using System.Linq; using System.Text; -using System.Threading.Tasks; namespace Microsoft.AspNetCore.Shared; @@ -100,9 +96,14 @@ internal static int Write7BitEncodedString(this Span target, string value) return 1; } - var stringByteCount = System.Text.Encoding.UTF8.GetByteCount(value); + var stringByteCount = Encoding.UTF8.GetByteCount(value); var lengthPrefixSize = target.Write7BitEncodedInt(stringByteCount); - System.Text.Encoding.UTF8.GetBytes(value, target.Slice(lengthPrefixSize)); +#if NETCOREAPP + Encoding.UTF8.GetBytes(value.AsSpan(), target[lengthPrefixSize..]); +#else + var stringBytes = Encoding.UTF8.GetBytes(value); + stringBytes.CopyTo(target.Slice(lengthPrefixSize)); +#endif return lengthPrefixSize + stringByteCount; } @@ -119,7 +120,7 @@ internal static int Measure7BitEncodedStringLength(string value) return 1; } - var stringByteCount = System.Text.Encoding.UTF8.GetByteCount(value); + var stringByteCount = Encoding.UTF8.GetByteCount(value); return stringByteCount.Measure7BitEncodedUIntLength() + stringByteCount; } @@ -144,7 +145,7 @@ internal static int Read7BitEncodedString(this ReadOnlySpan bytes, out str throw new FormatException("Bad 7-bit encoded string."); } - value = System.Text.Encoding.UTF8.GetString(bytes.Slice(consumed, length)); + value = Encoding.UTF8.GetString(bytes.Slice(consumed, length)); consumed += length; return consumed; } From b51f8c18de0b87d24140a441780ae2f01e8cf563 Mon Sep 17 00:00:00 2001 From: Korolev Dmitry Date: Mon, 15 Dec 2025 12:40:29 +0100 Subject: [PATCH 11/13] address PR comments 1 --- .../Internal/DefaultAntiforgeryTokenGenerator.cs | 14 ++++++++------ .../Internal/DefaultAntiforgeryTokenSerializer.cs | 12 ++++++------ 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenGenerator.cs b/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenGenerator.cs index 0c0b33cce2c5..01342c72b49c 100644 --- a/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenGenerator.cs +++ b/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenGenerator.cs @@ -173,7 +173,7 @@ public bool TryValidateTokenSet( return false; } - if (!AreIdenticalClaimUids(currentClaimUidBytes)) + if (!AreIdenticalClaimUids(requestToken, extractedClaimUidBytes, currentClaimUidBytes)) { message = Resources.AntiforgeryToken_ClaimUidMismatch; return false; @@ -190,17 +190,19 @@ public bool TryValidateTokenSet( message = null; return true; - bool AreIdenticalClaimUids(Span claimUidBytes) + static bool AreIdenticalClaimUids(AntiforgeryToken token, bool claimUidBytesExtracted, Span claimUidBytes) { - if (requestToken.ClaimUid is null) + if (token.ClaimUid is null) { - return !extractedClaimUidBytes; + return !claimUidBytesExtracted; } - if (requestToken.ClaimUid.Length != claimUidBytes.Length) + + if (token.ClaimUid.Length != claimUidBytes.Length) { return false; } - return requestToken.ClaimUid.GetData().SequenceEqual(claimUidBytes); + + return token.ClaimUid.GetData().SequenceEqual(claimUidBytes); } } diff --git a/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenSerializer.cs b/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenSerializer.cs index 55e88014857d..b899b7db6e67 100644 --- a/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenSerializer.cs +++ b/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenSerializer.cs @@ -33,7 +33,7 @@ public AntiforgeryToken Deserialize(string serializedToken) var tokenDecodedSize = Base64Url.GetMaxDecodedLength(serializedToken.Length); var rent = tokenDecodedSize < 256 - ? stackalloc byte[255] + ? stackalloc byte[256] : (tokenBytesRent = ArrayPool.Shared.Rent(tokenDecodedSize)); var tokenBytes = rent[..tokenDecodedSize]; @@ -43,14 +43,14 @@ public AntiforgeryToken Deserialize(string serializedToken) throw new FormatException("Failed to decode token as Base64 char sequence."); } - var tokenBytesDecoded = tokenBytes.Slice(0, bytesWritten); + var tokenBytesDecoded = tokenBytes[..bytesWritten]; if (_perfCryptoSystem is not null) { - var protectBuffer = new RefPooledArrayBufferWriter(stackalloc byte[255]); + var protectBuffer = new RefPooledArrayBufferWriter(stackalloc byte[256]); try { - _perfCryptoSystem!.Unprotect(tokenBytesDecoded, ref protectBuffer); + _perfCryptoSystem.Unprotect(tokenBytesDecoded, ref protectBuffer); var token = Deserialize(protectBuffer.WrittenSpan); if (token is not null) { @@ -165,11 +165,11 @@ public AntiforgeryToken Deserialize(string serializedToken) else { // Read Username (7-bit encoded length prefix + UTF-8 string) - offset += tokenBytes.Slice(offset).Read7BitEncodedString(out var username); + offset += tokenBytes[offset..].Read7BitEncodedString(out var username); deserializedToken.Username = username; } - offset += tokenBytes.Slice(offset).Read7BitEncodedString(out var additionalData); + offset += tokenBytes[offset..].Read7BitEncodedString(out var additionalData); deserializedToken.AdditionalData = additionalData; } From 4f9e35c5baf23a466c9974b38f04e0cc0c2c9dac Mon Sep 17 00:00:00 2001 From: Korolev Dmitry Date: Mon, 15 Dec 2025 12:52:21 +0100 Subject: [PATCH 12/13] address PR comments 2 --- .../src/Internal/AntiforgeryToken.cs | 7 +--- .../DefaultAntiforgeryTokenSerializer.cs | 40 +++++++++---------- .../src/Internal/DefaultClaimUidExtractor.cs | 6 ++- 3 files changed, 25 insertions(+), 28 deletions(-) diff --git a/src/Antiforgery/src/Internal/AntiforgeryToken.cs b/src/Antiforgery/src/Internal/AntiforgeryToken.cs index a7fc0072c548..5e530fe7e9ec 100644 --- a/src/Antiforgery/src/Internal/AntiforgeryToken.cs +++ b/src/Antiforgery/src/Internal/AntiforgeryToken.cs @@ -25,14 +25,11 @@ public string AdditionalData public bool IsCookieToken { get; set; } - public BinaryBlob? SecurityToken + public BinaryBlob SecurityToken { get { - if (_securityToken == null) - { - _securityToken = new BinaryBlob(SecurityTokenBitLength); - } + _securityToken ??= new BinaryBlob(SecurityTokenBitLength); return _securityToken; } set diff --git a/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenSerializer.cs b/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenSerializer.cs index b899b7db6e67..d91380beb267 100644 --- a/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenSerializer.cs +++ b/src/Antiforgery/src/Internal/DefaultAntiforgeryTokenSerializer.cs @@ -3,6 +3,7 @@ using System.Buffers; using System.Buffers.Text; +using System.Text; using Microsoft.AspNetCore.DataProtection; using Microsoft.AspNetCore.Shared; @@ -103,14 +104,20 @@ public AntiforgeryToken Deserialize(string serializedToken) */ private static AntiforgeryToken? Deserialize(ReadOnlySpan tokenBytes) { - var offset = 0; - - // we can only consume tokens of the same serialized version that we generate - if (tokenBytes.Length < 1) + // Minimum lengths: + // - Cookie token: 1 (version) + 16 (securityToken) + 1 (isCookieToken) = 18 bytes + // - Request token (username): 18 + 1 (isClaimsBased) + 1 (username prefix) + 1 (additionalData prefix) = 21 bytes + // - Request token (claims): 18 + 1 (isClaimsBased) + 32 (claimUid) + 1 (additionalData prefix) = 52 bytes + const int minCookieTokenLength = 1 + (AntiforgeryToken.SecurityTokenBitLength / 8) + 1; // 18 bytes + const int minRequestTokenLength = minCookieTokenLength + 1 + 1 + 1; // 21 bytes (username-based) + + if (tokenBytes.Length < minCookieTokenLength) { return null; } + var offset = 0; + var embeddedVersion = tokenBytes[offset++]; if (embeddedVersion != TokenVersion) { @@ -121,38 +128,29 @@ public AntiforgeryToken Deserialize(string serializedToken) // Read SecurityToken (16 bytes) const int securityTokenByteLength = AntiforgeryToken.SecurityTokenBitLength / 8; - if (tokenBytes.Length < offset + securityTokenByteLength) - { - return null; - } - deserializedToken.SecurityToken = new BinaryBlob( AntiforgeryToken.SecurityTokenBitLength, tokenBytes.Slice(offset, securityTokenByteLength).ToArray()); offset += securityTokenByteLength; // Read IsCookieToken (1 byte) - if (tokenBytes.Length < offset + 1) - { - return null; - } - deserializedToken.IsCookieToken = tokenBytes[offset++] != 0; if (!deserializedToken.IsCookieToken) { - // Read IsClaimsBased (1 byte) - if (tokenBytes.Length < offset + 1) + // Validate minimum length for request token + if (tokenBytes.Length < minRequestTokenLength) { return null; } + // Read IsClaimsBased (1 byte) var isClaimsBased = tokenBytes[offset++] != 0; if (isClaimsBased) { // Read ClaimUid (32 bytes) const int claimUidByteLength = AntiforgeryToken.ClaimUidBitLength / 8; - if (tokenBytes.Length < offset + claimUidByteLength) + if (tokenBytes.Length < offset + claimUidByteLength + 1) // +1 for additionalData prefix { return null; } @@ -187,7 +185,7 @@ public string Serialize(AntiforgeryToken token) { ArgumentNullException.ThrowIfNull(token); - var securityTokenBytes = token.SecurityToken!.GetData(); + var securityTokenBytes = token.SecurityToken.GetData(); var claimUidBytes = token.ClaimUid?.GetData(); var totalSize = @@ -204,11 +202,11 @@ public string Serialize(AntiforgeryToken token) } else { - var usernameByteCount = System.Text.Encoding.UTF8.GetByteCount(token.Username!); + var usernameByteCount = Encoding.UTF8.GetByteCount(token.Username!); totalSize += usernameByteCount.Measure7BitEncodedUIntLength() + usernameByteCount; } - var additionalDataByteCount = System.Text.Encoding.UTF8.GetByteCount(token.AdditionalData); + var additionalDataByteCount = Encoding.UTF8.GetByteCount(token.AdditionalData); totalSize += additionalDataByteCount.Measure7BitEncodedUIntLength() + additionalDataByteCount; } @@ -248,7 +246,7 @@ public string Serialize(AntiforgeryToken token) var protectBuffer = new RefPooledArrayBufferWriter(stackalloc byte[255]); try { - _perfCryptoSystem!.Protect(tokenBytes, ref protectBuffer); + _perfCryptoSystem.Protect(tokenBytes, ref protectBuffer); return Base64Url.EncodeToString(protectBuffer.WrittenSpan); } finally diff --git a/src/Antiforgery/src/Internal/DefaultClaimUidExtractor.cs b/src/Antiforgery/src/Internal/DefaultClaimUidExtractor.cs index a4d79c38cad0..114b3c8bef1a 100644 --- a/src/Antiforgery/src/Internal/DefaultClaimUidExtractor.cs +++ b/src/Antiforgery/src/Internal/DefaultClaimUidExtractor.cs @@ -28,7 +28,7 @@ public bool TryExtractClaimUidBytes(ClaimsPrincipal claimsPrincipal, Span return true; } - public static IList? GetUniqueIdentifierParameters(IEnumerable claimsIdentities) + public static List? GetUniqueIdentifierParameters(IEnumerable claimsIdentities) { var identitiesList = claimsIdentities as List; if (identitiesList == null) @@ -111,8 +111,10 @@ public bool TryExtractClaimUidBytes(ClaimsPrincipal claimsPrincipal, Span return identifierParameters; } - private static void ComputeSha256(IList parameters, Span destination) + private static void ComputeSha256(List parameters, Span destination) { + Debug.Assert(destination.Length >= SHA256.HashSizeInBytes); + // Calculate total size needed for serialization var totalSize = 0; for (var i = 0; i < parameters.Count; i++) From c76b05178e51481648a0d1d92d3b4b1b5e82a9f0 Mon Sep 17 00:00:00 2001 From: Korolev Dmitry Date: Mon, 15 Dec 2025 14:13:09 +0100 Subject: [PATCH 13/13] benchmarks changes --- .../Benchmarks/AntiforgeryBenchmarks.cs | 66 ++++++++++++------- 1 file changed, 44 insertions(+), 22 deletions(-) diff --git a/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryBenchmarks.cs b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryBenchmarks.cs index ef35cb147e0e..411ac3c51480 100644 --- a/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryBenchmarks.cs +++ b/src/Antiforgery/benchmarks/Microsoft.AspNetCore.Antiforgery.Benchmarks/Benchmarks/AntiforgeryBenchmarks.cs @@ -19,14 +19,18 @@ public class AntiforgeryBenchmarks private string _cookieName = null!; private string _formFieldName = null!; - // For GetAndStoreTokens - fresh context each time - private HttpContext _getAndStoreTokensContext = null!; + // Reusable contexts - reset between iterations instead of recreating + private DefaultHttpContext _getAndStoreTokensContext = null!; + private DefaultHttpContext _validateRequestContext = null!; + private TestHttpResponseFeature _getAndStoreTokensResponseFeature = null!; - // For ValidateRequestAsync - context with valid tokens - private HttpContext _validateRequestContext = null!; + // Pre-generated tokens for validation benchmark private string _cookieToken = null!; private string _requestToken = null!; + // Pre-allocated form collection for validation benchmark + private FormCollection _validationFormCollection = null!; + [GlobalSetup] public void Setup() { @@ -42,31 +46,38 @@ public void Setup() _cookieName = options.Cookie.Name!; _formFieldName = options.FormFieldName; - // Setup context for GetAndStoreTokens (no existing tokens) - _getAndStoreTokensContext = CreateHttpContext(); + // Create reusable context for GetAndStoreTokens + _getAndStoreTokensResponseFeature = new TestHttpResponseFeature(); + _getAndStoreTokensContext = CreateHttpContext(_getAndStoreTokensResponseFeature); // Generate tokens for validation benchmark - var tokenContext = CreateHttpContext(); + var tokenContext = CreateHttpContext(new TestHttpResponseFeature()); var tokenSet = _antiforgery.GetAndStoreTokens(tokenContext); _cookieToken = tokenSet.CookieToken!; _requestToken = tokenSet.RequestToken!; - // Setup context for ValidateRequestAsync (with valid tokens) - _validateRequestContext = CreateHttpContextWithTokens(_cookieToken, _requestToken); + // Pre-allocate form collection for validation + _validationFormCollection = new FormCollection(new Dictionary + { + { _formFieldName, _requestToken } + }); + + // Create reusable context for ValidateRequestAsync + _validateRequestContext = CreateHttpContextWithTokens(); } [IterationSetup(Target = nameof(GetAndStoreTokens))] public void SetupGetAndStoreTokens() { - // Create a fresh context for each iteration to simulate real-world usage - _getAndStoreTokensContext = CreateHttpContext(); + // Reset the context instead of creating a new one + ResetHttpContextForGetAndStoreTokens(); } [IterationSetup(Target = nameof(ValidateRequestAsync))] public void SetupValidateRequest() { - // Create a fresh context with tokens for each iteration - _validateRequestContext = CreateHttpContextWithTokens(_cookieToken, _requestToken); + // Reset the context instead of creating a new one + ResetHttpContextForValidation(); } [Benchmark] @@ -81,7 +92,7 @@ public Task ValidateRequestAsync() return _antiforgery.ValidateRequestAsync(_validateRequestContext); } - private HttpContext CreateHttpContext() + private DefaultHttpContext CreateHttpContext(TestHttpResponseFeature responseFeature) { var context = new DefaultHttpContext(); context.RequestServices = _serviceProvider; @@ -96,14 +107,13 @@ [new Claim(ClaimsIdentity.DefaultNameClaimType, "testuser@example.com")], context.Request.ContentType = "application/x-www-form-urlencoded"; // Setup response features to allow cookie writing - var responseFeature = new TestHttpResponseFeature(); context.Features.Set(responseFeature); context.Features.Set(new StreamResponseBodyFeature(Stream.Null)); return context; } - private HttpContext CreateHttpContextWithTokens(string cookieToken, string requestToken) + private DefaultHttpContext CreateHttpContextWithTokens() { var context = new DefaultHttpContext(); context.RequestServices = _serviceProvider; @@ -118,17 +128,29 @@ [new Claim(ClaimsIdentity.DefaultNameClaimType, "testuser@example.com")], context.Request.ContentType = "application/x-www-form-urlencoded"; // Set the cookie token using the actual cookie name from options - context.Request.Headers.Cookie = $"{_cookieName}={cookieToken}"; + context.Request.Headers.Cookie = $"{_cookieName}={_cookieToken}"; - // Set the request token in form using the actual form field name - context.Request.Form = new FormCollection(new Dictionary - { - { _formFieldName, requestToken } - }); + // Set the request token in form using the pre-allocated form collection + context.Request.Form = _validationFormCollection; return context; } + private void ResetHttpContextForGetAndStoreTokens() + { + // Clear the antiforgery feature so it generates fresh tokens + _getAndStoreTokensContext.Features.Set(null); + + // Reset response headers that antiforgery sets + _getAndStoreTokensResponseFeature.Headers.Clear(); + } + + private void ResetHttpContextForValidation() + { + // Clear the antiforgery feature so it deserializes tokens fresh + _validateRequestContext.Features.Set(null); + } + private sealed class TestHttpResponseFeature : IHttpResponseFeature { public int StatusCode { get; set; } = 200;