Skip to content

Fix CMS CTR-ACPKM decrypt after ASN.1 parameter reinit - #528

Merged
chipitsine merged 3 commits into
gost-engine:masterfrom
rostislaved:master
Oct 11, 2026
Merged

chipitsine merged 3 commits into
gost-engine:masterfrom
rostislaved:master

Conversation

@rostislaved

Copy link
Copy Markdown
Contributor

I ran into an issue with Kuznyechik where, after decrypting files, bytes after offset 4096 were corrupted.

I do not understand the C code or these algorithms well enough, so I asked Codex to investigate it. It produced this pull request.

Codex found that CMS kuznyechik-ctr-acpkm parameters set the ACPKM section size from ASN.1, but a later cipher reinit overwrote that value with the default section size. As a result, decryption switched keys at the wrong boundary.

This PR preserves the ACPKM section size when it was explicitly set from ASN.1 or via ctrl. It also adds a regression test that reproduces the ASN.1 parameter + reinit flow.

Please take a look if this makes sense.

@rostislaved

Copy link
Copy Markdown
Contributor Author

The issue looked like this. It is a file after decryption: bytes after 4096 were incorrect
image

Reject negative values in EVP_CTRL_KEY_MESH: Kuznyechik CTR-ACPKM
(arg <= 0; -16 previously passed the modulo check and wrapped into the
unsigned section_size) and Magma CTR-ACPKM (arg < 0).

Rework test_engine_asn1_acpkm_reinit:
- run round-trips for lengths around the CMS section boundary
  (256 KiB for Kuznyechik, 8192 for Magma) and split the decrypt
  Update right before the boundary;
- verify section_size/key_meshing and the *_set flag after
  param_to_asn1, asn1_to_param and key re-init, not just the round-trip;
- check that invalid section sizes are rejected without changing
  context state and that the context stays usable;
- replace T() with ACPKM_CHECK so failures are reported as test
  results and all resources are freed.
@chipitsine
chipitsine merged commit 5a52371 into gost-engine:master Oct 11, 2026
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants