Skip to content

security(core,fido2,piv): fix sensitive data handling#447

Open
DennisDyallo wants to merge 1 commit intoyubikit-appletsfrom
worktree-security-remediation
Open

security(core,fido2,piv): fix sensitive data handling#447
DennisDyallo wants to merge 1 commit intoyubikit-appletsfrom
worktree-security-remediation

Conversation

@DennisDyallo
Copy link
Copy Markdown
Collaborator

Summary

  • Remove 38 Console.WriteLine statements from SCP implementation that unconditionally dumped session encryption keys (S-ENC, S-MAC, S-RMAC), cryptograms, MAC chains, and plaintext APDU data to stdout
  • Zero all sensitive buffers in FIDO2 ClientPin (PIN bytes, hash intermediates, encrypted buffers), PinUvAuthProtocol V1/V2 (.ToArray() key copies), and PivSession.Authentication (.ToArray() AES key copies) via try/finally with CryptographicOperations.ZeroMemory()
  • Implement IDisposable on ScpState with full disposal chain (PcscProtocolScp → ScpProcessor → ScpState) to zero MAC chain and dispose SessionKeys
  • Reduce OTP HID protocol logging from hex-dumping payloads/reports to byte-count-only metadata

Security Impact

Finding Severity Status
Console.WriteLine dumps SCP session keys to stdout CRITICAL Fixed
LogTrace dumps plaintext APDU data as hex CRITICAL Fixed
FIDO2 PIN bytes never zeroed after use HIGH Fixed
.ToArray() creates untracked key copies (5 files) HIGH Fixed
ScpState holds SessionKeys without IDisposable HIGH Fixed
OTP HID logs payload hex at Trace level MEDIUM Fixed

Files Changed (10)

File Change
src/Core/src/SmartCard/Scp/ScpState.Scp03.cs Remove 13 Console.WriteLine (session keys, cryptograms)
src/Core/src/SmartCard/Scp/ScpState.cs Remove Console.WriteLine, fix LogTrace, add IDisposable, zero .ToArray() keys
src/Core/src/SmartCard/Scp/ScpProcessor.cs Remove 13 Console.WriteLine, add IDisposable to dispose ScpState
src/Core/src/SmartCard/Scp/StaticKeys.cs Remove 5 Console.WriteLine (key derivation data)
src/Core/src/SmartCard/Scp/PcscProtocolScp.cs Dispose ScpProcessor in disposal chain
src/Fido2/src/Pin/ClientPin.cs Zero PIN bytes in PadPin/ComputePinHash, zero all intermediates in async finally blocks
src/Fido2/src/Pin/PinUvAuthProtocolV1.cs Capture and zero .ToArray() key/plaintext copies in Encrypt/Decrypt
src/Fido2/src/Pin/PinUvAuthProtocolV2.cs Same pattern as V1
src/Piv/src/PivSession.Authentication.cs Zero .ToArray() AES key copy in DecryptBlock/EncryptBlock
src/Core/src/Hid/Otp/OtpHidProtocol.cs Replace 6 hex-dump log calls with byte-count metadata

Test plan

  • dotnet build.cs build — 0 errors
  • dotnet build.cs test — 9/9 unit test projects passing
  • dotnet format --verify-no-changes — clean
  • grep -rn "Console.WriteLine" src/ — 0 matches in production code
  • Integration test on YubiKey with SCP03 session (verifies SCP still works after removing debug logging)

🤖 Generated with Claude Code

…, and key operations

Remove 38 Console.WriteLine statements from SCP implementation that unconditionally
dumped session encryption keys (S-ENC, S-MAC, S-RMAC), cryptograms, MAC chains, and
plaintext APDU data to stdout. Replace LogTrace plaintext hex dumps with length-only
logging. Zero PIN bytes in FIDO2 ClientPin PadPin/ComputePinHash after use. Zero all
intermediate buffers (pinHashEnc, newPinEnc, message, pinHash) in ClientPin async
method finally blocks. Capture and zero .ToArray() key copies in PinUvAuthProtocol
V1/V2 Encrypt/Decrypt and PivSession.Authentication DecryptBlock/EncryptBlock.
Implement IDisposable on ScpState to zero MAC chain and dispose SessionKeys, with
disposal chain through ScpProcessor and PcscProtocolScp. Reduce OTP HID protocol
logger hex dumps to length-only metadata.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant