openwall / openwall/john

mbedTLS AES code, intrinsics vs. assembly, alignment

Open
#5,593 39 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement maintenance/cleanup
Dominant language
C
Stars
13.6k
Forks
2.6k
PR merge metrics
No merged PRs in 30d

Description

I believe it builds asm for now. Brief tests with intrinsics show a slight performance drop. From PR comments:

@solardiz said:

I notice there are pieces of inline asm code in mbedTLS, which use non-VEX SSE instructions. Hopefully this works OK, but there's risk of it being slow (or of VEX-encoded code being slow afterwards) without vzeroupper on transitions (which would also be slow, just without the risk of being an order of magnitude slower). I don't suggest changing this yet, just writing down this note.

@magnumripper said:

 * \note AESNI is only supported with certain compilers and target options:
 * - Visual Studio: supported
 * - GCC, x86-64, target not explicitly supporting AESNI:
 *   requires MBEDTLS_HAVE_ASM.
 * - GCC, x86-32, target not explicitly supporting AESNI:
 *   not supported.
 * - GCC, x86-64 or x86-32, target supporting AESNI: supported.
 *   For this assembly-less implementation, you must currently compile
 *   `library/aesni.c` and `library/aes.c` with machine options to enable
 *   SSE2 and AESNI instructions: `gcc -msse2 -maes -mpclmul` or
 *   `clang -maes -mpclmul`.
 * - Non-x86 targets: this option is silently ignored.
 * - Other compilers: this option is silently ignored.
 *
 * \note
 * Above, "GCC" includes compatible compilers such as Clang.
 * The limitations on target support are likely to be relaxed in the future.

Perhaps we do need some tweak to ensure intrinsics and not asm, but I did just now manually build with -mavx2 -maes -mpclmul per above, and that resulted in a 62% larger aes.a and definitely worse performance (not a lot, but worse).

Disregarding the performance drop, we do have @CC_CPU@ from configure.ac to put in Makefile.in (will add -mavx2 for my laptop) but the -maes -mpclmul would need to be added too. I assume those two can be added even for machines not supporting it (bc cpuid checking) but it would need testing, and obviously can't be blindly added - the machine could be a Sparc and/or the compiler could be one that hasn't got a clue what those options are.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Read the AES implementation and build configuration in library/aesni.c, library/aes.c, configure.ac, and Makefile.in, then reproduce the reported manual build with -mavx2 -maes -mpclmul. Compare the assembly and intrinsic paths across the compiler and target cases described in the issue; done means the supported build behavior, alignment considerations, and performance impact are validated.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
cryptography, performance
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.