Skip to content

Conversation

@DennisDyallo
Copy link
Collaborator

  • Fixed DigestData to use hash digest size instead of key size for RSA keys
  • Reuses MessageDigestOperations.ComputeMessageDigest for hashing
  • For RSA: returns raw digest (PadRsa handles signature padding)
  • For ECC: pads digest to key size with leading zeros if needed
  • Added unit tests for digest computation logic
  • Updated devcontainer to include .NET 8.0 and 10.0

- Fixed DigestData to use hash digest size instead of key size for RSA keys
- Reuses MessageDigestOperations.ComputeMessageDigest for hashing
- For RSA: returns raw digest (PadRsa handles signature padding)
- For ECC: pads digest to key size with leading zeros if needed
- Added unit tests for digest computation logic
- Updated devcontainer to include .NET 8.0 and 10.0
@github-actions
Copy link
Contributor

github-actions bot commented Jan 22, 2026

Test Results: Windows

    2 files      2 suites   15s ⏱️
4 023 tests 4 004 ✅ 19 💤 0 ❌
4 025 runs  4 006 ✅ 19 💤 0 ❌

Results for commit fb4d208.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 22, 2026

Test Results: Ubuntu

    2 files      2 suites   47s ⏱️
4 015 tests 3 996 ✅ 19 💤 0 ❌
4 017 runs  3 998 ✅ 19 💤 0 ❌

Results for commit fb4d208.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 22, 2026

Test Results: MacOS

    4 files      4 suites   30s ⏱️
3 997 tests 3 997 ✅ 0 💤 0 ❌
3 999 runs  3 999 ✅ 0 💤 0 ❌

Results for commit fb4d208.

♻️ This comment has been updated with latest results.

@DennisDyallo DennisDyallo linked an issue Jan 23, 2026 that may be closed by this pull request
1 task
@DennisDyallo DennisDyallo requested a review from Copilot January 23, 2026 11:20
@DennisDyallo DennisDyallo changed the title fix(sample): Fix YubiKeySignatureGenerator.DigestData regression fix(sample): Fix YubiKeySignatureGenerator.DigestData regression in Sample App Jan 23, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a regression in the YubiKeySignatureGenerator.DigestData method where the key size was incorrectly used instead of the hash digest size for RSA keys, causing ArgumentException when trying to copy more bytes than available in the hash result.

Changes:

  • Fixed DigestData to return raw digest for RSA keys (signature padding is handled by PadRsa)
  • Changed ECC digest handling to pad with leading zeros when digest is smaller than key size
  • Added comprehensive unit tests to verify the fix and prevent future regressions

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
YubiKeySignatureGenerator.cs Fixed DigestData to use hash digest size instead of key size for RSA, reuses MessageDigestOperations.ComputeMessageDigest, and properly pads ECC digests
YubiKeySignatureGeneratorTests.cs Added unit tests verifying correct digest computation for RSA and ECC keys, including tests demonstrating the original bug
devcontainer.json Added .NET 8.0 and 10.0 to the development container configuration

@DennisDyallo DennisDyallo changed the title fix(sample): Fix YubiKeySignatureGenerator.DigestData regression in Sample App fix(piv): Fix YubiKeySignatureGenerator.DigestData regression in Sample App Jan 23, 2026
@github-actions
Copy link
Contributor

Code Coverage

Package Line Rate Branch Rate Complexity Health
Yubico.Core 45% 35% 4557
Yubico.YubiKey 51% 46% 20946
Summary 49% (36846 / 74598) 44% (8837 / 20082) 25503

Minimum allowed line rate is 40%

@DennisDyallo DennisDyallo marked this pull request as ready for review January 23, 2026 11:25
@DennisDyallo DennisDyallo merged commit 0e287a6 into develop Jan 26, 2026
14 of 15 checks passed
@DennisDyallo DennisDyallo deleted the fix/issue-395-digest-data-regression branch January 26, 2026 18:16
@DennisDyallo DennisDyallo mentioned this pull request Jan 28, 2026
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.

[BUG] YubiKeySignatureGenerator.DigestData regression

1 participant