Skip to content

[Code Quality] ProtectedKeyProvider.GetKey/GetIV — no in-memory caching, full DPAPI roundtrip + 3-retry file read on every call #1069

@Christophe-Rogiers

Description

@Christophe-Rogiers

Severity: Warning

File: src/Servy.Core/Security/ProtectedKeyProvider.cs

Lines: 92, 95 (public surface), 160-329 (GetOrGenerate)

Code snippet:

public byte[] GetKey() => GetOrGenerate(_keyFilePath, 32);
public byte[] GetIV() => GetOrGenerate(_ivFilePath, 16);

private byte[] GetOrGenerate(string path, int length)
{
    ...
    encrypted = File.ReadAllBytes(path);     // synchronous disk I/O
    ...
    var unprotectResult = ProtectedData.Unprotect(encrypted, dynamicEntropy, DataProtectionScope);  // CSP roundtrip
    ...
}

Explanation:
Every GetKey() or GetIV() call performs a fresh File.ReadAllBytes (with up to 3 retries / Thread.Sleep backoff on IOException) plus a ProtectedData.Unprotect Win32 call. The class caches the file path, the machine entropy (Lazy<byte[]> MachineEntropy), and the retry counters, but not the unprotected key itself.

For consumers that call GetKey/GetIV per encryption operation (the typical AES-Encrypt/AES-Decrypt pattern around row-level secrets, e.g. in SecureData and connection-string handling), this means each protected field roundtrip pays the DPAPI cost twice (once for key, once for IV). Under load — bulk import/export, batch list refresh — this is N × 2 file reads + DPAPI unprotect calls, all serialized through a single static FileLock if first-run generation races with reads.

Suggested fix:
Cache the decrypted key/IV in private fields after the first successful unprotect, with explicit invalidation on SaveProtected (key rotation). Memoize per-instance, not per-call. If cache lifetime is a concern, add a configurable TTL or rely on IDisposable to wipe the cached buffer with CryptographicOperations.ZeroMemory on dispose.

🤖 Generated with Claude Code

Metadata

Metadata

Assignees

Labels

enhancementNew feature or request

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions