feat: Add support for Zlib compression/decompression by oskardanielsson-yubico · Pull Request #417 · Yubico/Yubico.NET.SDK

oskardanielsson-yubico · 2026-02-24T12:56:41Z

Description

Certificates compressed with zlib can now be decompressed in the Piv application

Fixes: # <link to issue(s)>

Type of change

New feature (non-breaking change which adds functionality)
This change requires a documentation update

How has this been tested?

Unit tests that test the compression and decompression have been added and run.

Test configuration:

OS version: MacOS 15.6

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

…irs, add Zlib compression unit tests

github-actions · 2026-02-24T12:59:30Z

Test Results: Windows

2 files 2 suites 16s ⏱️
4 052 tests 4 033 ✅ 19 💤 0 ❌
4 054 runs 4 035 ✅ 19 💤 0 ❌

Results for commit 34d0493.

♻️ This comment has been updated with latest results.

github-actions · 2026-02-24T12:59:38Z

Test Results: Ubuntu

2 files 2 suites 47s ⏱️
4 044 tests 4 025 ✅ 19 💤 0 ❌
4 046 runs 4 027 ✅ 19 💤 0 ❌

Results for commit 34d0493.

♻️ This comment has been updated with latest results.

github-actions · 2026-02-24T12:59:45Z

Test Results: MacOS

4 files 4 suites 28s ⏱️
4 026 tests 4 026 ✅ 0 💤 0 ❌
4 028 runs 4 028 ✅ 0 💤 0 ❌

Results for commit 34d0493.

♻️ This comment has been updated with latest results.

Copilot

Pull request overview

This pull request adds support for Zlib compression/decompression to enable the PIV application to handle certificates compressed with zlib format. The implementation includes a new internal ZLibStream class that provides RFC 1950-compliant zlib compression/decompression, along with format auto-detection in the certificate retrieval logic to support both the standard GZip format and a custom Zlib format used by some third-party PIV middleware.

Changes:

Added a new ZLibStream class implementing RFC 1950 zlib compression/decompression with Adler-32 checksum validation
Enhanced certificate decompression in PivSession to auto-detect and support both GZip and Zlib formats
Added comprehensive unit tests for the ZLibStream class covering compression, decompression, edge cases, and error conditions

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.

File	Description
Yubico.YubiKey/src/Yubico/YubiKey/Cryptography/ZLibStream.cs	New internal class implementing zlib compression/decompression with header validation and Adler-32 checksum
Yubico.YubiKey/src/Yubico/YubiKey/Piv/PivSession.KeyPairs.cs	Updated certificate decompression to detect and handle both GZip and Zlib formats, improved exception handling with inner exception
Yubico.YubiKey/tests/unit/Yubico/YubiKey/Cryptography/ZLibStreamTests.cs	Comprehensive unit tests for ZLibStream including compression levels, error conditions, Adler-32 validation, and format compatibility

Yubico.YubiKey/src/Yubico/YubiKey/Cryptography/ZLibStream.cs

Yubico.YubiKey/src/Yubico/YubiKey/Piv/PivSession.KeyPairs.cs

Copilot · 2026-02-24T22:18:42Z

Yubico.YubiKey/src/Yubico/YubiKey/Cryptography/ZLibStream.cs

+            if (!buffer.IsEmpty)
+            {
+                byte[] temp = buffer.ToArray();
+                UpdateAdler32(temp, 0, temp.Length);
+            }


The async WriteAsync method creates a temporary array copy (line 299) to update the Adler-32 checksum. This allocation and copy operation happens on every async write, which could impact performance. For ReadOnlyMemory<byte>, consider using MemoryMarshal.TryGetArray to access the underlying array when possible, or use the Span property to avoid the allocation. Alternatively, if the buffer is backed by an array, you can avoid the copy by checking if it's contiguous memory.

AI vs AI difference of opinion

Yubico.YubiKey/src/Yubico/YubiKey/Piv/PivSession.KeyPairs.cs

Copilot · 2026-02-24T22:18:43Z

Yubico.YubiKey/src/Yubico/YubiKey/Cryptography/ZLibStream.cs

+        private void UpdateAdler32(byte[] buffer, int offset, int count)
+        {
+            const uint modAdler = 65521;
+
+            for (int i = offset; i < offset + count; i++)
+            {
+                _adlerA = (_adlerA + buffer[i]) % modAdler;
+                _adlerB = (_adlerB + _adlerA) % modAdler;
+            }
+        }


The Adler-32 implementation performs modulo operation on every byte, which can be inefficient for large data. The standard optimization is to defer the modulo operation and only apply it after processing a certain number of bytes (typically 5550 bytes, which is the largest value where overflow won't occur). This optimization is commonly used in production zlib implementations and can significantly improve performance when compressing large data. Consider optimizing UpdateAdler32 to batch the modulo operations.

AI vs AI difference of opinion

DennisDyallo · 2026-02-26T13:12:37Z

@claude can you take a good look at this? Code review, correctness, quality, maintainability, codebase alignment, security, etc.

claude · 2026-02-26T13:13:15Z

Claude finished @DennisDyallo's task in 4m 19s —— View job

Code Review

Tasks

Gather context and understand the request
Read changed files
Analyze ZLibStream implementation (correctness, security, quality)
Analyze PivSession.KeyPairs.cs changes
Analyze ExceptionMessages changes
Analyze unit tests
Post comprehensive review

Overall Assessment

The implementation is solid — well-structured, properly documented, and follows codebase conventions. The core logic (RFC 1950 header parsing, Adler-32 computation, GIDS format detection) is correct. However there are several issues ranging from a meaningful correctness gap to minor style nits worth addressing before merge.

🔴 High: Missing Adler-32 Validation During Decompression

ZLibStream.cs — decompression path

The class advertises RFC 1950 compliance (// This implementation is based on RFC 1950) and the docs say it handles "a 4-byte Adler-32 checksum trailer." However, during decompression the trailer bytes are never consumed or verified. After DeflateStream finishes reading the compressed payload, the last 4 bytes of the underlying stream (the Adler-32) remain unread. Crucially, the decompressed data is never checksummed.

This means:

A zlib stream where the deflate data is intact but the Adler-32 is wrong will silently succeed.
A stream where the deflate data is corrupted might be caught by DeflateStream's own internal CRC, but the Adler-32 integrity layer provides no extra protection.

For the specific use case (certificate decompression off a YubiKey), this is low-risk in practice. But the class's own documentation sets the expectation of RFC 1950 compliance, and callers may use it beyond this one callsite.

Either:

Document the limitation explicitly — update the class summary to state that Adler-32 is computed during compression but is not verified during decompression.
Implement the validation — intercept reads to track the rolling checksum, then after DeflateStream is exhausted, read the 4 trailing bytes from BaseStream and compare. This is non-trivial but makes the class genuinely RFC-compliant.

Fix this →

🟠 Medium: GIDS Length Guard Is Too Loose

PivSession.KeyPairs.cs line 713

if (data[0] == 0x01 && data[1] == 0x00 && data.Length > 4)

data.Length > 4 permits a 5-byte buffer. With a 4-byte GIDS header consumed, DecompressZlib is passed a 1-byte stream. ZLibStream.ReadAndValidateZLibHeader() will then fail on the second ReadByte() (returns -1) and throw InvalidDataException with an internal "Unexpected end of zlib header" message, which gets wrapped by the outer catch in GetCertificate.

The minimum usable size is 4 (GIDS header) + 2 (zlib CMF/FLG) = 6 bytes. Change to:

if (data[0] == 0x01 && data[1] == 0x00 && data.Length >= 6)

Fix this →

🟠 Medium: Hardcoded Exception Message in `GenerateKeyPair`

PivSession.KeyPairs.cs line 179

throw new InvalidOperationException("Error generating key pair: " + response);

Every other exception in this file (and across the SDK) uses ExceptionMessages. This string doesn't belong in source code. It should be added to ExceptionMessages.resx and accessed via ExceptionMessages.YourNewKey.

Fix this →

🟡 Low: Adler-32 Trailer Correctness Is Not Tested End-to-End

ZLibStreamTests.cs

ComputeAdler32_KnownInput_ReturnsExpectedChecksum correctly validates the standalone checksum method. But there is no test that:

Compresses data with ZLibStream
Reads back the final 4 bytes of the compressed output
Asserts those bytes equal the expected Adler-32 of the original data

Without this, a regression in WriteAdler32Trailer (e.g. wrong byte order) would go undetected. A test using the "Wikipedia" constant (0x11E60398) would serve this well.

Fix this →

🟡 Low: Missing Test for GIDS Decompressed Length Mismatch

ZLibStreamTests.cs

DecompressGids validates the decompressed length against the GIDS header value and throws with a formatted message if they differ. This error path has no test coverage. Add a test that constructs a GIDS blob with an incorrect expected length and asserts that InvalidOperationException is thrown.

🟡 Low: GIDS Test Does Not Exercise the Production Code Path

ZLibStreamTests.cs — Decompress_GidsFormat_StripsHeaderAndDecompresses (line 53)

This test manually strips the 4-byte header and passes the rest to ZLibStream directly. It validates the mechanism but doesn't call DecompressWithFormatDetection or DecompressGids. Consider adding a test that passes a complete GIDS-formatted byte array through the same entry point used by GetCertificate (which calls DecompressWithFormatDetection internally). This would give confidence that the format detection and dispatch logic is correct.

🟡 Low: GIDS Expected Length Limited to 16 Bits (Worth a Comment)

PivSession.KeyPairs.cs line 741

int expectedLength = data[2] | (data[3] << 8);

This reads 2 bytes, giving a max expected decompressed length of 65535 bytes. This is fine for X.509 certificates (well under 64 KB), but a comment noting this limitation would prevent future confusion if this code is reused. The GIDS standard may support larger lengths with a 4-byte field.

✅ What's Done Well

Class design: ZLibStream follows the standard .NET Stream pattern cleanly — proper sealed class, CanRead/CanWrite properties, Dispose lifecycle, leaveOpen support. The existing .NET GZipStream API is mirrored well.
RFC 1950 header handling: CMF/FLG validation (compression method check, FDICT flag check, % 31 checksum) is correct and well-commented.
Adler-32 computation: The algorithm is correct — verified against the well-known "Wikipedia" → 0x11E60398 test vector. The UpdateAdler32 instance method and ComputeAdler32 static method are consistent.
Exception messages: All exception strings (except one, noted above) correctly use ExceptionMessages.resx. The new resource entries follow the existing naming and formatting conventions.
Format detection logic: The DecompressWithFormatDetection approach is clean — GZip magic bytes first, then GIDS prefix, then throw. The GIDS decompressed-length validation is a good defensive check.
Test coverage: The test suite is comprehensive for ZLibStream itself — round-trips, all compression levels, edge cases (empty input, large data, truncated header, wrong compression method, disposed stream, leaveOpen). The known-good test vector for decompression (789c...) is valuable for interoperability.
Multi-target compatibility: The #if NETSTANDARD2_1_OR_GREATER guards for ReadAsync(Memory<byte>), WriteAsync(ReadOnlyMemory<byte>), and CopyTo(Stream, int) are correct.
Codebase alignment: The internal sealed modifier, namespace placement (Yubico.YubiKey.Cryptography), XML doc coverage, and resource file usage all align with existing conventions.

DennisDyallo · 2026-03-06T15:03:07Z

@claude Fix the most important issues you uncovered, and make sure there are tests, and existing tests pass, without overfitting.

claude · 2026-03-06T15:03:29Z

Claude finished @DennisDyallo's task in 16m 12s —— View job

Fixes Applied

Changes made (commit 5584386)

🐛 GIDS length guard (`PivSession.KeyPairs.cs`)

- if (data[0] == 0x01 && data[1] == 0x00 && data.Length > 4)
+ if (data[0] == 0x01 && data[1] == 0x00 && data.Length >= 6)

A 5-byte buffer would pass the old guard but fail inside ZLibStream.ReadAndValidateZLibHeader() on the second ReadByte(). Minimum valid size is 4 (GIDS header) + 2 (zlib CMF/FLG) = 6 bytes.

🐛 Hardcoded exception message in `GenerateKeyPair` (`PivSession.KeyPairs.cs` + resources)

- throw new InvalidOperationException("Error generating key pair: " + response);
+ throw new InvalidOperationException(
+     string.Format(CultureInfo.CurrentCulture, ExceptionMessages.GenerateKeyPairFailed, response));

Added GenerateKeyPairFailed = "Error generating key pair: {0}" to ExceptionMessages.resx and the corresponding property to ExceptionMessages.Designer.cs.

📄 Adler-32 decompression limitation documented (`ZLibStream.cs` class summary)

Rather than implement a potentially unreliable streaming Adler-32 validation (.NET's DeflateStream may consume the 4-byte trailer into its internal buffer before Read returns 0, making post-read validation non-portable across target frameworks), the limitation is now explicitly documented in the class XML docs:

During decompression, the 2-byte zlib header is validated, but the 4-byte Adler-32 trailer is not verified. Corruption not caught by the underlying DEFLATE decoder will go undetected.

✅ New test: Adler-32 trailer end-to-end (`ZLibStreamTests.cs`)

Compress_WritesCorrectAdler32Trailer — compresses "Wikipedia", reads the last 4 bytes of the output, and asserts they equal the big-endian Adler-32 of the original data. Uses the well-known "Wikipedia" → 0x11E60398 test vector as a double check.

What was not changed

GIDS path test via DecompressWithFormatDetection — this method is static private on PivSession. Testing it directly would require reflection or a visibility change, both of which are more invasive than warranted given "without overfitting." The existing Decompress_GidsFormat_WithHeaderStripping_Works test covers the mechanism.
Adler-32 validation during decompression — reliably implementing this requires intercepting all read paths (sync/async/CopyTo/CopyToAsync) and working around DeflateStream's internal buffering across .NET Standard 2.0, 2.1, and .NET Framework 4.7. Documenting the limitation is the correct trade-off here.
| Branch

- Fix GIDS length guard: data.Length > 4 → data.Length >= 6 to prevent passing a buffer too small for the zlib header to DecompressZlib - Move hardcoded exception string in GenerateKeyPair to ExceptionMessages.resx (adds GenerateKeyPairFailed with {0} placeholder for the response value) - Document Adler-32 trailer limitation in ZLibStream XML docs: checksum is written during compression but not verified during decompression - Add Compress_WritesCorrectAdler32Trailer unit test verifying that the last 4 bytes of compressed output equal the Adler-32 of the original data, using the well-known "Wikipedia" → 0x11E60398 test vector Co-authored-by: Dennis Dyallo <DennisDyallo@users.noreply.github.com>

DennisDyallo · 2026-03-06T15:24:02Z

@claude Fix the style issues in the project which are causing CICD build issues:

Error: /home/runner/work/Yubico.NET.SDK/Yubico.NET.SDK/Yubico.YubiKey/tests/unit/Yubico/YubiKey/Cryptography/ZLibStreamTests.cs(515,46): error IDE0056: Indexing can be simplified (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0056) [/home/runner/work/Yubico.NET.SDK/Yubico.NET.SDK/Yubico.YubiKey/tests/unit/Yubico.YubiKey.UnitTests.csproj]

claude · 2026-03-06T15:24:28Z

Claude finished @DennisDyallo's task in 29s —— View job

Fix Applied

Tasks

Gather context and understand the request
Read ZLibStreamTests.cs line 515 to identify the indexing issue
Fix the IDE0056 style error
Commit and push the fix

IDE0056: Indexing can be simplified — ZLibStreamTests.cs lines 515–518

Replaced compressed[compressed.Length - N] with the C# index-from-end operator compressed[^N]:

- uint trailer = ((uint)compressed[compressed.Length - 4] << 24)
-              | ((uint)compressed[compressed.Length - 3] << 16)
-              | ((uint)compressed[compressed.Length - 2] << 8)
-              | compressed[compressed.Length - 1];
+ uint trailer = ((uint)compressed[^4] << 24)
+              | ((uint)compressed[^3] << 16)
+              | ((uint)compressed[^2] << 8)
+              | compressed[^1];

Committed as 34d0493.
| Branch

Co-authored-by: Dennis Dyallo <DennisDyallo@users.noreply.github.com>

github-actions · 2026-03-06T15:28:15Z

Package	Line Rate	Branch Rate	Complexity	Health
Yubico.Core	45%	35%	4557	➖
Yubico.YubiKey	50%	46%	21534	➖
Summary	49% (37350 / 76326)	44% (8986 / 20556)	26091	➖

Minimum allowed line rate is 40%

Add ZlibStream class, check for compressed format in PivSession.KeyPa…

9ddc5a2

…irs, add Zlib compression unit tests

oskardanielsson-yubico added 2 commits February 24, 2026 15:53

Change naming, give exception instead of fallback during decompression

3835601

Change header naming, add length check during decompression

b4a3c8a

DennisDyallo requested a review from Copilot February 24, 2026 22:13

Copilot started reviewing on behalf of DennisDyallo February 24, 2026 22:14 View session

Copilot AI reviewed Feb 24, 2026

View reviewed changes

Move exception strings to ExceptionMessages class

ee2a19f

fix: Use index-from-end operator to resolve IDE0056 style error

34d0493

Co-authored-by: Dennis Dyallo <DennisDyallo@users.noreply.github.com>

DennisDyallo approved these changes Mar 6, 2026

View reviewed changes

DennisDyallo merged commit a5812e3 into develop Mar 9, 2026
14 of 15 checks passed

DennisDyallo deleted the oskar/zlib branch March 9, 2026 09:57

DennisDyallo mentioned this pull request Mar 30, 2026

release: 1.16.0 #440

Merged

Conversation

oskardanielsson-yubico commented Feb 24, 2026 • edited by DennisDyallo Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

How has this been tested?

Checklist:

Uh oh!

github-actions bot commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results: Windows

Uh oh!

github-actions bot commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results: Ubuntu

Uh oh!

github-actions bot commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results: MacOS

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

oskardanielsson-yubico Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

oskardanielsson-yubico Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

DennisDyallo commented Feb 26, 2026

Uh oh!

claude bot commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review

Tasks

Overall Assessment

🔴 High: Missing Adler-32 Validation During Decompression

🟠 Medium: GIDS Length Guard Is Too Loose

🟠 Medium: Hardcoded Exception Message in GenerateKeyPair

🟡 Low: Adler-32 Trailer Correctness Is Not Tested End-to-End

🟡 Low: Missing Test for GIDS Decompressed Length Mismatch

🟡 Low: GIDS Test Does Not Exercise the Production Code Path

🟡 Low: GIDS Expected Length Limited to 16 Bits (Worth a Comment)

✅ What's Done Well

Uh oh!

DennisDyallo commented Mar 6, 2026

Uh oh!

claude bot commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fixes Applied

Changes made (commit 5584386)

🐛 GIDS length guard (PivSession.KeyPairs.cs)

🐛 Hardcoded exception message in GenerateKeyPair (PivSession.KeyPairs.cs + resources)

📄 Adler-32 decompression limitation documented (ZLibStream.cs class summary)

✅ New test: Adler-32 trailer end-to-end (ZLibStreamTests.cs)

What was not changed

Uh oh!

DennisDyallo commented Mar 6, 2026

Uh oh!

claude bot commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fix Applied

Tasks

Uh oh!

github-actions bot commented Mar 6, 2026

oskardanielsson-yubico commented Feb 24, 2026 •

edited by DennisDyallo

Loading

github-actions bot commented Feb 24, 2026 •

edited

Loading

github-actions bot commented Feb 24, 2026 •

edited

Loading

github-actions bot commented Feb 24, 2026 •

edited

Loading

claude bot commented Feb 26, 2026 •

edited

Loading

🟠 Medium: Hardcoded Exception Message in `GenerateKeyPair`

claude bot commented Mar 6, 2026 •

edited

Loading

🐛 GIDS length guard (`PivSession.KeyPairs.cs`)

🐛 Hardcoded exception message in `GenerateKeyPair` (`PivSession.KeyPairs.cs` + resources)

📄 Adler-32 decompression limitation documented (`ZLibStream.cs` class summary)

✅ New test: Adler-32 trailer end-to-end (`ZLibStreamTests.cs`)

claude bot commented Mar 6, 2026 •

edited

Loading