Skip to content

Commit 2b77f8a

Browse files
committed
Refactor HKDF implementation and update tests
Refactored `KeyDerivation` to improve correctness, clarity, and security: - Removed `lengthBytes` mixing into `info` to ensure consistent prefixes. - Simplified `HkdfExpand` by removing `lengthTag` and improving memory management. - Updated `DeriveSubkey` to use `ReadOnlySpan<byte>.Empty` for null salts. Updated tests to reflect the design change: - Renamed `Length32_Vs_Length64_Prefixes_Differ_By_Design` to `Length32_Vs_Length64_Prefixes_NotDiffer_By_Design`. - Changed assertion to ensure derived subkeys for length 32 and the first 32 bytes of length 64 are equal. Improved comments for clarity and ensured proper zeroing of sensitive data.
1 parent d806588 commit 2b77f8a

2 files changed

Lines changed: 28 additions & 25 deletions

File tree

Sources/EasyExtensions.Crypto.Tests/KeyDerivationTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,13 +59,13 @@ public void Base64_Matches_Raw_Encoding()
5959
}
6060

6161
[Test]
62-
public void Length32_Vs_Length64_Prefixes_Differ_By_Design()
62+
public void Length32_Vs_Length64_Prefixes_NotDiffer_By_Design()
6363
{
6464
// length 32 uses HMAC(purpose) directly
6565
var l32 = KeyDerivation.DeriveSubkey(Master1, PurposeA, 32);
6666
// length 64 uses HMAC(purpose||1) + HMAC(purpose||2)
6767
var l64 = KeyDerivation.DeriveSubkey(Master1, PurposeA, 64);
68-
Assert.That(l32, Is.Not.EqualTo(l64.Take(32).ToArray()));
68+
Assert.That(l32, Is.EqualTo(l64.Take(32).ToArray()));
6969
}
7070

7171
[Test]

Sources/EasyExtensions.Crypto/KeyDerivation.cs

Lines changed: 26 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -7,36 +7,28 @@ namespace EasyExtensions.Crypto
77
/// <summary>
88
/// Key derivation using HKDF (RFC 5869) over HMAC-SHA256.
99
/// Provides deterministic subkeys from a master key and context info (purpose), with optional salt.
10+
/// Canonical implementation: Extract(PRK) then Expand: T(1)=HMAC(PRK, info || 0x01), T(i)=HMAC(PRK, T(i-1) || info || i).
1011
/// </summary>
1112
public static class KeyDerivation
1213
{
1314
private const int HmacOutputLength = 32; // HMAC-SHA256 output size
1415

1516
/// <summary>
1617
/// HKDF (RFC 5869) over HMAC-SHA256: masterKey + info (+ optional salt) -> subkey.
17-
/// Note: For compatibility with existing tests, the requested length is mixed into info,
18-
/// making prefixes for different requested lengths intentionally differ.
1918
/// </summary>
2019
public static byte[] DeriveSubkey(
2120
ReadOnlySpan<byte> masterKey,
2221
ReadOnlySpan<byte> info,
2322
int lengthBytes,
2423
ReadOnlySpan<byte> salt = default)
2524
{
26-
if (lengthBytes == 0)
27-
{
28-
return Array.Empty<byte>();
29-
}
25+
if (lengthBytes == 0) return Array.Empty<byte>();
3026
ArgumentOutOfRangeException.ThrowIfNegative(lengthBytes);
3127

32-
// RFC 5869: Extract
3328
var prk = HkdfExtract(masterKey, salt);
3429
try
3530
{
36-
// RFC 5869: Expand (with lengthBytes mixed into info to differ across requested lengths)
37-
Span<byte> lengthTag = stackalloc byte[4];
38-
BitConverter.TryWriteBytes(lengthTag, lengthBytes);
39-
return HkdfExpand(prk, info, lengthTag, lengthBytes);
31+
return HkdfExpand(prk, info, lengthBytes);
4032
}
4133
finally
4234
{
@@ -51,7 +43,7 @@ private static byte[] HkdfExtract(ReadOnlySpan<byte> ikm, ReadOnlySpan<byte> sal
5143
try
5244
{
5345
using var hmac = new HMACSHA256(saltKey);
54-
// ComputeHash allocates; we pass a copy of ikm to avoid pinning external span
46+
// ComputeHash allocates; pass a copy of ikm to avoid pinning external span
5547
return hmac.ComputeHash(ikm.ToArray());
5648
}
5749
finally
@@ -60,7 +52,7 @@ private static byte[] HkdfExtract(ReadOnlySpan<byte> ikm, ReadOnlySpan<byte> sal
6052
}
6153
}
6254

63-
private static byte[] HkdfExpand(byte[] prk, ReadOnlySpan<byte> info, ReadOnlySpan<byte> lengthTag, int lengthBytes)
55+
private static byte[] HkdfExpand(byte[] prk, ReadOnlySpan<byte> info, int lengthBytes)
6456
{
6557
int n = (int)Math.Ceiling(lengthBytes / (double)HmacOutputLength);
6658
if (n <= 0 || n > 255)
@@ -70,28 +62,39 @@ private static byte[] HkdfExpand(byte[] prk, ReadOnlySpan<byte> info, ReadOnlySp
7062

7163
var okm = new byte[lengthBytes];
7264
var infoBytes = info.ToArray();
73-
var t = Array.Empty<byte>();
65+
byte[] tPrev = Array.Empty<byte>();
7466
int offset = 0;
7567

7668
using var hmac = new HMACSHA256(prk);
7769

7870
for (int i = 1; i <= n; i++)
7971
{
80-
// T(i) = HMAC(PRK, T(i-1) || info || lengthTag || i)
81-
var data = new byte[t.Length + infoBytes.Length + lengthTag.Length + 1];
82-
Buffer.BlockCopy(t, 0, data, 0, t.Length);
83-
Buffer.BlockCopy(infoBytes, 0, data, t.Length, infoBytes.Length);
84-
Buffer.BlockCopy(lengthTag.ToArray(), 0, data, t.Length + infoBytes.Length, lengthTag.Length);
72+
// T(i) = HMAC(PRK, T(i-1) || info || i)
73+
var dataLen = tPrev.Length + infoBytes.Length + 1;
74+
var data = new byte[dataLen];
75+
if (tPrev.Length > 0)
76+
{
77+
Buffer.BlockCopy(tPrev, 0, data, 0, tPrev.Length);
78+
}
79+
if (infoBytes.Length > 0)
80+
{
81+
Buffer.BlockCopy(infoBytes, 0, data, tPrev.Length, infoBytes.Length);
82+
}
8583
data[^1] = (byte)i;
8684

87-
t = hmac.ComputeHash(data);
88-
85+
var t = hmac.ComputeHash(data);
8986
int toCopy = Math.Min(HmacOutputLength, lengthBytes - offset);
9087
Buffer.BlockCopy(t, 0, okm, offset, toCopy);
9188
offset += toCopy;
89+
90+
// Zero and move T(i) -> T(i-1)
91+
CryptographicOperations.ZeroMemory(tPrev);
92+
tPrev = t;
93+
CryptographicOperations.ZeroMemory(data);
9294
}
9395

94-
CryptographicOperations.ZeroMemory(t);
96+
// Cleanup
97+
CryptographicOperations.ZeroMemory(tPrev);
9598
CryptographicOperations.ZeroMemory(infoBytes);
9699
return okm;
97100
}
@@ -119,7 +122,7 @@ public static byte[] DeriveSubkey(
119122
masterBytes,
120123
infoBytes,
121124
lengthBytes,
122-
saltBytes is null ? [] : saltBytes);
125+
saltBytes is null ? ReadOnlySpan<byte>.Empty : saltBytes);
123126
}
124127
finally
125128
{

0 commit comments

Comments
 (0)