Skip to content

AES-NI acceleration with mandatory pure-Pascal fallback (package C) - #7

Open
omonien wants to merge 2 commits into
package/chacha-poly1305from
package/aes-ni
Open

AES-NI acceleration with mandatory pure-Pascal fallback (package C)#7
omonien wants to merge 2 commits into
package/chacha-poly1305from
package/aes-ni

Conversation

@omonien

@omonien omonien commented Jul 25, 2026

Copy link
Copy Markdown
Owner

Package C — AES-NI (optional, cross-platform safe)

Stacked on package B (package/chacha-poly1305). No PR against MHumm for now (fork only).

Policy

Layer Rule
Compile-time AES-NI asm only under X86ASM / X64ASM
Always Pure Pascal Rijndael/AES path remains
Runtime UseAESAsm and TDEC_CPUSupport.AES and key size 16/24/32
Default Auto-on when CPUID reports AES-NI (x86/x64 ASM builds); else off
Force PAS TCipher_Rijndael.UseAESAsm := False before Init

Out of scope

  • ARM Crypto Extensions
  • Changing public AES API beyond optional class var

Tests

  • FIPS-197 single-block KATs AES-128/192/256
  • PAS vs AES-NI must match when both available
  • ChaCha / GCM / CCM regression green

See Docs/plans/2026-07-25-aes-ni.md.


Note

Medium Risk
Touches core block-cipher encrypt/decrypt and key scheduling in DECCiphers.pas; dual paths must stay bit-identical, though tests and compile/runtime gates reduce exposure.

Overview
Adds optional AES-NI for TCipher_Rijndael and AES-128/192/256 on x86/x64 builds where X86ASM/X64ASM is defined, while keeping the existing pure Pascal path as the correctness baseline everywhere else.

Runtime control: new class var TCipher_Rijndael.UseAESAsm defaults from TDEC_CPUSupport.AES at unit init; per-instance FAESAsmActive is set in DoInit only when UseAESAsm, CPU AES-NI, and key size is 16/24/32 bytes—non-standard Rijndael key lengths and toggling UseAESAsm after init stay on Pascal until re-init. Implementation: large inline assembler block for key expansion (128/192/256), AESIMC decode schedule, and AESEncode/AESDecode; AdditionalBufferSize increases by $20 for 32-byte-aligned AES-NI schedules (existing context tests updated 480→512).

Tests & docs: new TestDECAESNI (FIPS-197 ECB KATs, PAS vs AES-NI parity) wired into the DUnit suite; plan doc Docs/plans/2026-07-25-aes-ni.md describes policy and gates.

Reviewed by Cursor Bugbot for commit e1df722. Bugbot is set up for automated code reviews on this repo. Configure here.

@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4fde7ac3-58dd-4cfe-948e-4ffbeccee399

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch package/aes-ni

Comment @coderabbitai help to get the list of available commands.

@omonien
omonien force-pushed the package/chacha-poly1305 branch from ec6b252 to 156e504 Compare July 27, 2026 09:17

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 62dd0eb. Configure here.

Comment thread Source/DECCipherModes.pas Outdated
Comment thread Source/DECCiphers.pas
@omonien
omonien force-pushed the package/chacha-poly1305 branch from 156e504 to c6fb30c Compare July 27, 2026 11:39
omonien added 2 commits July 27, 2026 13:39
Port AES-NI key expand/encode/decode from pr-90 into TCipher_Rijndael,
gated by X86ASM/X64ASM at compile time and UseAESAsm + CPUID AES +
standard 16/24/32 key sizes at runtime. Instance flag FAESAsmActive
keeps encode/decode on the schedule actually built by DoInit.

- BuildAsmKey128/192/256 (196 renamed to 192), BuildAsmDecodeKey (AESIMC)
- AESEncode/AESDecode; Context buffer +$20 for AlignPtr32
- class var UseAESAsm defaulted from TDEC_CPUSupport.AES when ASM compiled
- FIPS-197 ECB KATs in TestDECAESNI (PAS and AES-NI must match)
- Docs/plans/2026-07-25-aes-ni.md

ARM/FPC/PUREPASCAL: pure Pascal only; correctness never requires AES-NI.
Stop writing a 16th AES-256 encode round key at offset 240; place the AESIMC
inverse schedule after (FRounds+1) encode keys so AES-256 no longer overwrites
the last encode key; point AESDecode at the same offset.
@omonien

omonien commented Jul 27, 2026

Copy link
Copy Markdown
Owner Author

Addressed 2026-07-27 (fork cleanup pass)

Linear stack restored (A → B → C) and Cursor Bugbot / Augment findings handled:

Area Fix
GCM AAD after Encode/Done rejected (sGCMAADLocked)
GCM tag length > 128 EDECAuthLengthException + defensive copy
EncodeCCM / DecodeCCM independent FAuthObj dispatch (no longer forwards through EncodeGCM)
Poly1305 + block ciphers stream-only (BlockSize < 16)
ChaCha Poly init FBufferSize restored to Context.BufferSize
Win32 AVX XMM save/restore on X86ASM
AES-256 AES-NI schedule no +240 encode key; decode at (FRounds+1)*Blocks

Tests (D13 Win32 Console): GCM 21/21, ChaCha20Poly1305 12/12, AESNI 7/7. Pre-existing Keccak reds only.

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.

1 participant