Skip to content

Conversation

@Alonof
Copy link

@Alonof Alonof commented Aug 13, 2018

Notes:

Description

Replace macros in mbedtls SHA256 & SHA512 to function ro reduce code size
the tradeoff in replacing the macro is size over speed benchmark testing shows the following results:
Macro
SHA-256 515 Kb/s
SHA-512 338 Kb/s

Functions
SHA-256 456 Kb/s
SHA-512 309 Kb/s
SHA256 drops by 12% and SHA512 drops by 9%

Status

READY

Requires Backporting

NO

Migrations

NO

Additional comments

Tested on K64F

Todos

  • Tests
  • Documentation
  • Changelog updated
  • Backported

@RonEld RonEld added enhancement Arm Contribution component-crypto Crypto primitives and low-level interfaces labels Aug 13, 2018
@irwir
Copy link
Contributor

irwir commented Aug 14, 2018

the tradeoff in replacing the macro is size over speed

Speed measurements showed how much was lost.
It would be interesting to know, what exactly was the gain in absolute and relative numbers?

@mpg
Copy link
Contributor

mpg commented Jul 17, 2019

I tried that for SHA-256 (on top of the existing configuration MBEDTLS_SHA256_SMALLER, which people will have enabled anyway if they care about code size), and building for Cortex-M0 with ARM-GCC 7 it saves 112 bytes (84 with armcc5). What's more interesting, making P a function only saves 16 bytes with ARM-GCC and 8 with armcc5 (which is no so surprising as it's only called in one place with the SMALLER option), so I think we could get most of the code-size savings by just making PUT_UINT32_BE() a function.

@mpg
Copy link
Contributor

mpg commented Jul 17, 2019

@Patater @Alonof I suspect now the PR should probably be re-raised to https://github.com/ARMmbed/mbedtls-restricted/pull/621?

@Alonof
Copy link
Author

Alonof commented Jul 17, 2019

Hi @kjbracey-arm
Do you have thoughts?

@mpg
Copy link
Contributor

mpg commented Jul 17, 2019

Actually I just ported the SHA-256 part of if to baremetal here: https://github.com/ARMmbed/mbedtls-restricted/pull/624 (which will be side-ported to Mbed Crypto at some point) and raised the SHA-512 part (plus some more) directly to Mbed Crypto here: ARMmbed/mbed-crypto#178

So I think this PR can now be closed.

@mpg mpg closed this Jul 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component-crypto Crypto primitives and low-level interfaces enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants