Harden ASN.1 sequence parsing with cursor helpers; consolidate block-cipher test scaffolding#138
Merged
Merged
Conversation
Route every Asn1Sequence element read through cursor helpers on TAsn1Utilities so short/malformed sequences fail cleanly instead of throwing raw index errors or silently ignoring trailing elements. Cursor helpers (TAsn1Utilities, threading `var ASequencePosition`): - CheckSequenceSize - nil + min/max size guard (shared SBadSequenceSize) - ReadEncodable - bounds-guarded mandatory read (SPrematureEndOfSequence) - Read<T> - ReadEncodable + T.GetInstance - ReadOptional<T> - peek; advance only if the constructor returns non-nil - Read/ReadContext/ReadOptional*Tagged, TryReadOptional*Tagged - RequireEndOfSequence - reject trailing elements (SUnexpectedElementsInSequence) A truncated sequence now raises EArgumentCryptoLibException instead of a raw index error, and unexpected trailing elements are rejected. Add const-first-arg function pointer types TCryptoLibConstFunc<T1,TResult> and <T1,T2,TResult> in ClpCryptoLibTypes so T.GetInstance / T.GetOptional / T.GetTagged bind directly, eliminating per-type forwarder wrappers. Migrate all 13 ASN.1 object units to the cursor helpers and remove 37 now-dead per-class reader/optional wrappers. Add Asn1SequenceCursorTests (9 edge-case tests)
Consolidate the AES, Speck, and bulk-parity test suites around a single engine-factory type and a common base class, removing duplicated scaffolding and making new engine suites cheap to add. New CryptoLib.Tests/src/Crypto/BlockCipherTestBase.pas: - TBlockCipherFactory = function: IBlockCipher — one canonical factory type replacing the generic TCryptoLibFunc<...> and three private nested TEngineFactory declarations. - TBlockCipherTestBase: engine-agnostic scaffolding inherited by every block-cipher suite — DoBlockCipherVectorTest (previously byte-duplicated in the AES and Speck bases), the data-driven RunBlockCipherVectorTests, plus the generic IBlockCipher contract checks AssertEngineRejectsBadParameters and RunCipherEngineChecks. TAesBlockCipherTestBase now derives from TBlockCipherTestBase and keeps only AES-specific pieces: known-answer + Monte-Carlo vectors, DoBlockCipherMonteCarloTest, and the published contract-test trio (TestBlockCipherVector / TestMonteCarloAES / TestBadParameters) driven by GetEngineFactory / EngineLabel / EngineSupported hooks (EngineSupported defaults True). AesHardwareEngineTests, AESTests, and AesLightTests inherit those published tests via the hooks instead of hand-writing delegating bodies; the hardware suite also drops its local TEngineFactory and gains a guarded TestBadParameters. TSpeckBlockCipherTestBase derives from TBlockCipherTestBase, dropping its duplicated DoBlockCipherVectorTest and its two byte-identical vector runners (now a single inherited runner); Speck factory functions return IBlockCipher. Cbc/Ecb/Sic bulk-parity suites use the shared TBlockCipherFactory.
The sequence-cursor helpers in ClpAsn1Utilities were the only users of the
general-purpose TCryptoLibConstFunc<> family in ClpCryptoLibTypes. Move those
callbacks next to their sole consumer and give them intent-revealing names, so
ClpCryptoLibTypes stays usage-neutral and the helper signatures self-document.
- Remove TCryptoLibConstFunc<T1,TResult> and <T1,T2,TResult> from
ClpCryptoLibTypes.
- Add to ClpAsn1Utilities:
TAsn1Constructor<TInput, TResult> = function(const AElement: TInput): TResult;
TAsn1TaggedConstructor<TResult> = function(const ATagged: IAsn1TaggedObject;
ADeclaredExplicit: Boolean): TResult;
The const first argument still binds the ARC-const T.GetInstance /
T.GetOptional / T.GetTagged statics directly (no wrappers).
Collapse the vestigial generic state on the tagged cursor helpers: every call
site only ever passed Boolean (the declaredExplicit flag), so drop the <TState>
parameter and take ADeclaredExplicit: Boolean directly. Read/ReadOptional keep
TAsn1Constructor<TInput,TResult>; ReadTagged/ReadContextTagged/
ReadOptional(Context)Tagged/TryReadOptional(Context)Tagged move from
<TState,TResult> to <TResult>. Tagged call sites across the ASN.1 object units
and Asn1SequenceCursorTests shed the now-redundant "Boolean," generic argument
(e.g. ReadOptionalContextTagged<Boolean, IAsn1Set> -> <IAsn1Set>).
Element-level GetOptionalContextTagged / TryGetOptionalContextTagged are
unchanged (they keep TCryptoLibFunc<...,TState,TResult>).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ASN.1 sequence parsing
TAsn1Utilities:CheckSequenceSize,ReadEncodable,Read<T>,ReadOptional<T>, tagged/context-tagged variants, andRequireEndOfSequenceEArgumentCryptoLibException(SPrematureEndOfSequence) instead of raw index errors; trailing elements are rejected (SUnexpectedElementsInSequence)Asn1Utilities.Read(seq, ref pos, T.GetInstance)TAsn1Constructor/TAsn1TaggedConstructorsoT.GetInstance,T.GetOptional, andT.GetTaggedbind directly — no per-type forwarder wrappers and no@workaroundsAsn1SequenceCursorTests(9 edge-case tests)Block-cipher test consolidation
BlockCipherTestBase.paswithTBlockCipherFactoryandTBlockCipherTestBase