Decryption-path review

An internal review of every decryption path where a mistake is expensive: plaintext released before it is authenticated, error oracles, and resource exhaustion. It covers CMS (EnvelopedData, AuthEnvelopedData, EncryptedData, CompressedData and every RecipientInfo form), AES key wrap, PKCS#8 and PKCS#12 decryption, and OpenPGP message decryption (PgpMessageReader, the session-key packets, sop decrypt). This is an internal review by an engineer who did not write the code. It is not an external audit; the library is not independently audited.

Method: the code was read against the RFCs; adversarial inputs were built with the library's own writers, byte mutation and a few hand-assembled structures. Each finding had one committed test that failed and was skipped; the Status column says how each was fixed and which test now pins it. Tests are in tests/OneCrypto.Tests/Review/DecryptionReviewTests.cs and tests/OneCrypto.OpenPgp.Tests/Review/DecryptionReviewTests.cs. Tests without a Skip pin behaviour the review found correct. The review itself changed no source; the fixes came after (Status column). No oracle was needed: every open question was settled by a crafted input against this library (no behaviour came up that the specifications leave open), so no openssl cms, GnuPG or Sequoia run is part of this review. The one timing check (D12, D13) is a local, single-run, indicative measurement, not a timing-test campaign.

Findings

Severity classes: plaintext-before-auth, oracle, accept-tampered, DoS, report-misleading, docs-only; one extra class, disclosure, for plaintext at rest.

Counts: oracle 2 (D1, D12), accept-tampered 3 (D3, D8, D11), DoS 4 (D2, D4, D7, D13), report-misleading 2 (D6, D9), plaintext-before-auth 1 (D10, by design and documented), disclosure 1 (D5). Docs: docs/openpgp.md says every decryption failure is the same CannotDecrypt (D1 contradicts it) and does not say how a caller can tell SED output from authenticated output (D6). No finding releases unauthenticated plaintext from an integrity-protected format.

Decision: what Standard means for CMS CBC (D9, D10) — proposed

Standard's "fails unauthenticated encryption closed" cannot mean refusal for CMS CBC EnvelopedData: S/MIME relies on an outer signature over the same bytes, and refusing it would break every mail client's messages. It should mean: never silently. Proposed wording for docs/policy.md and docs/cms.md:

Under Standard, CBC EnvelopedData and EncryptedData decrypt, and the result says so: the decrypt call returns a report whose Authenticated is false and whose reasons name cipher.cbc.unauthenticated.decrypt. The plain byte[] overload is kept for callers who have an outer signature, and its documentation says that a successful call proves nothing about the content. Strict refuses CBC. The streaming call reports the same through an exception that carries the number of octets already released.

Today's behaviour is honest in the XML docs and docs/cms.md (they say CBC is unauthenticated and the stream releases earlier blocks), but the policy flag is never surfaced, and CmsDecryptOptions.Policy defaults to none, so Standard is not what a caller gets unless they ask for it.

What was checked and found correct

By question in the brief. "Tested" means a passing test in the two review files unless another file is named.

Release before authentication

• SEIPD v1: the whole body is decrypted into the spool and the MDC (SHA-1 over everything but the 22-octet trailer, plus the trailer's two octets) is compared with FixedTimeEquals before the spool is rewound; nothing reaches the output earlier, on the sync path, the async twins (output is spooled too) and sop decrypt. Tampering, truncation and a missing or wrong MDC release zero octets (MessageTests.Seipd1MdcAndTruncationFailClosed, TamperedSeipd1ReleasesNothing). The library holds at most the spool's 4 MiB threshold plus pooled scratch in memory and puts the rest in the temporary file, so the 108 MB peak of the 1 GiB v1 decrypt is streaming through a file, not buffering the message (the peak itself was not profiled here). That is safe for authentication, with the disclosure issue of D5.
• SEIPD v2 and LibrePGP OCB: a chunk is handed over only after its own tag verifies; the last, short chunk is withheld until the final tag (over the total length) verifies; truncation at the final tag, inside it, inside the last chunk's tag, and by whole chunks; swapped, duplicated and dropped chunks; one trailing octet; all fail with PgpException and are never released (AeadTruncationFailsAtEveryBoundary, AeadReorderedDuplicatedAndExtendedChunksFail, MessageTests.TamperedAeadMessagesFailClosed). The chunk index is in the nonce and in the associated data; the chunk-size octet is bounded at 16 before any buffer is allocated (ChunkSizeAboveTheSpecificationLimitIsRefusedBeforeAnyAllocation). A late failure is unmistakable to the caller: Read throws and returns no result; but chunks that authenticated were already written (AeadReleasesVerifiedChunksBeforeALateFailureAndNeverTheBadOne), as docs/openpgp.md says, and sop decrypt has by then written them to stdout before it exits with code 29. The async twins write nothing on failure.
• SED: refused without AllowUnauthenticatedEncryption before any candidate or key work, whatever the key source, including a caller-supplied session key (SedNeedsTheOptInAndEveryOtherFailureIsTyped); sop decrypt has no opt-in.
• AuthEnvelopedData: the AEAD is one-shot, so nothing is produced before the tag check; ciphertext, tag, and authenticated-attribute tampering and the streaming twin give DecryptionFailed and write nothing (AuthEnvelopedDataFailsClosedOnEveryTamperingAndGivesOneError); the tag length is compared with the parameter; the content-type attribute is checked; MaxContentBytes bounds the buffer.

Error oracles

• CMS RSA PKCS#1 v1.5 (KeyTrans): a failed unwrap or a wrong-length key substitutes a random key of the right length and continues, so a bad transport and bad CBC padding end in the same exception type, code and text (RsaPkcs1FailuresAreIndistinguishableByErrorFromBadPadding); AllowPkcs1V15KeyTransport can turn the form off. (Residual: D12 timing, and the default stays on.)
• OpenPGP session-key validation (FromChecksummed): padding, length, algorithm octet and checksum accumulate in one mask with a fixed pass over the data; every single-octet corruption is refused (MessageTests.UnwrappedSessionKeysAreValidatedWholly). The class-level leak is D1.
• AES-KW (AesKeyWrap.TryUnwrap): the integrity value is compared with FixedTimeEquals, the key data is zeroed on failure, and lengths are public; the CMS and OpenPGP callers treat failure as "no candidate" without an exception.
• Multiple recipients: in CMS each RecipientInfo is tried in its own try, so a malformed one does not mask a good one (RecipientsThatCannotBeReadDoNotMaskAGoodOne); matched reveals only whether an identifier matches (public). In OpenPGP the masking cases are D2 and D7.
• OpenPGP quick check: used only as a candidate filter for SEIPD v1, with the same error as every other failure (D7 is its one flaw); SEIPD v2 and SKESK v6 use the AEAD tag as the check.

Key agreement

• CMS ECDH/X25519/X448 (KeyAdapter.DeriveAgreementSecret): platform point validation for NIST/Brainpool curves; X25519/X448 all-zero outputs raise and become "no key"; the KEK comes from X9.63 or HKDF with ECC-CMS-SharedInfo (wrap algorithm, UKM, KEK length) as RFC 5753 §7 / RFC 8418 §2.1 say; the wrap algorithm is read from the message but its key size must equal the KEK length asked for. ML-KEM KEMRecipientInfo: the wrap size must equal kekLength; the KDF and wrap are bound into the info (RFC 9629 §5); implicit rejection of the platform ML-KEM gives a wrong secret, which the key unwrap then rejects.
• OpenPGP X25519/X448 (DecryptNative): low-order shared secrets give no candidate; the HKDF info strings and key lengths match RFC 9580 §5.1.6–5.1.7 and the key material is bound into the IKM; ECDH validates the point format and curve, the KDF hash and wrap cipher are taken from the recipient's own key (not the message), and the unwrap goes through the same checks; RFC 9980 composites combine the ML-KEM and ECDH shares with the ciphertexts and public key in the KDF, require an AES cipher octet for v3 packets and pass through the same unwrap.

Resource limits (all on by default, typed errors)

• Decompression: OpenPGP ZIP/ZLIB InflateLimitStream (1 MiB plus 1032 × compressed octets read) with MaxNesting 8 (DeeplyNestedCompressionIsRefused, ZeroesCompressedToTheDeflateLimitStillPassTheRatioCeiling); CMS MaxDecompressedBytes and MaxDecompressionRatio (CompressedDataBombsAreRefusedWithATypedError). Not checked: the CMS ratio is 1000 while OpenPGP's is 1032, so data near deflate's maximum ratio is refused by CMS (a functional inconsistency, no security effect).
• Partial-length chunking: the first chunk must be at least 512 octets; bodies read as typed packets are capped at MaxPacketBodyLength (16 MiB); streamed bodies are not buffered.
• S2K and Argon2 from the message: the exponent, passes and lanes are checked against KdfLimits before allocation (MessageTests.Argon2CostBombsInPasswordPacketsAreRefusedBeforeAnyWork); iterated-and-salted S2K is capped by its one-octet count; PBKDF2 in CMS and PKCS#8/#12 is capped (MaxPbkdf2Iterations, PkcsLimits.MaxIterations, scrypt through KdfLimits). The multiplication across packets is D4 and D13.
• CMS recipient count (1,024), content size (1 GiB), the small-element budget of the streaming reader (64 MiB) and the ASN.1 limits apply to every decrypting entry.

PKCS#8 / PKCS#12 (read, not exercised by new tests): PBES2 decryption derives the key under limits, checks the GCM tag or the CBC padding, and reports one WrongPasswordOrCorrupt for a wrong password, bad padding or a body that is not a PrivateKeyInfo; PKCS#12 compares the MAC with FixedTimeEquals (the order of MAC check and decryption was not traced); legacy PEM encryption (MD5 EVP_BytesToKey) is read-only. The padding check shape is D12.

Not reviewed

LibrePGP Kyber and v5-protected secret keys (not implemented); the key backends (a later review); SKESK v5 beyond reading the code; the .Pdf and .AdES paths; the PQC composite combiner's construction beyond comparing its inputs with RFC 9980 §4.2; timing campaigns (only the two indicative measurements above); interop against openssl cms, GnuPG, Sequoia and RNP for the crafted inputs (none was needed to settle a behaviour).

Follow-ups seen, outside this review

• CompressedData ratio 1000 against OpenPGP's 1032; EnvelopedData.DecryptStream with Triple-DES (D10).
• OpenAead allocates a chunk-sized window (up to 4 MiB) and one probe reader (three chunk-sized buffers) per candidate key; with many candidates that is memory churn (bounded once D4 lands).
• CmsDecryptOptions.AllowPkcs1V15KeyTransport defaults to true; a stricter default would shrink the D12/D1 exposure for new integrations.
• sop decrypt prints the library's failure text on stderr; with D1 fixed the text is uniform.