Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Static SHA::Transform fails on Intel SHA cpu's #455

Closed
noloader opened this issue Aug 5, 2017 · 1 comment
Closed

Static SHA::Transform fails on Intel SHA cpu's #455

noloader opened this issue Aug 5, 2017 · 1 comment
Labels

Comments

@noloader
Copy link
Collaborator

noloader commented Aug 5, 2017

Below are from self tests that exercise SHA::Transform (SHA is SHA1, SHA256, etc), which is a static member function. The code path for SHA::Transform is a little different than the code path when using an SHA object with Update and Final.

HASH::InitState and HASH::Transform are unique to hashes which derive from IteratedHashWithStaticTransform. Most hash classes simply derive from IteratedHash. The reason for the static HASH::Transform is, some classes like SEAL and MDC, call the transform function directly. The static functions allow the other classes to use, say, a user supplied key as state instead of the traditional SHA initialization values.

The self tests are fine on non-SHA-accelerated processors. However, due to the slightly different code paths, when ARM or Intel SHA is available, the code misses a required endian conversion. And again, calling Update and Final on a SHA object is fine because the conversions are present.

SHA static transform suite running...

passed:  SHA1::Transform, message 1
passed:  SHA1::Transform, message 2
FAILED:  SHA1::Transform, message 3
passed:  SHA256::Transform, message 1
passed:  SHA256::Transform, message 2
FAILED:  SHA256::Transform, message 3
passed:  SHA512::Transform, message 1
passed:  SHA512::Transform, message 2
passed:  SHA512::Transform, message 3

The reason only Message 3 fails can be seen in the sample messages. A string of zero's (Message 1) and a string of 1's (Message 2) don't have endianness problems per se. However, the bytes in Message 3 does suffer endianness.

const byte SM1[MAX_PARAM] = {0};
const byte SM2[MAX_PARAM] = {
    0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,
    0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,
    0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,
    0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,
};
const byte SM3[MAX_PARAM] = {
    0x00,0x04,0x08,0x0C,0x10,0x14,0x18,0x1C,0x20,0x24,0x28,0x2C,0x30,0x34,0x38,0x3C,
    0x40,0x44,0x48,0x4C,0x50,0x54,0x58,0x5C,0x60,0x64,0x68,0x6C,0x70,0x74,0x78,0x7C,
    0x80,0x84,0x88,0x8C,0x90,0x94,0x98,0x9C,0xA0,0xA4,0xA8,0xAC,0xB0,0xB4,0xB8,0xBC,
    0xC0,0xC4,0xC8,0xCC,0xD0,0xD4,0xD8,0xDC,0xE0,0xE4,0xE8,0xEC,0xF0,0xF4,0xF8,0xFC
};
@noloader noloader added the Bug label Aug 5, 2017
noloader referenced this issue Aug 13, 2017
Reworked SHA class internals to align all the implementations. Formerly all hashes were software based, IterHashBase handled endian conversions, IterHashBase repeatedly called the single block SHA{N}::Transform. The rework added SHA{N}::HashMultipleBlocks, and the SHA classes attempt to always use it.

Now SHA{N}::Transform calls into SHA{N}_HashMultipleBlocks, which is a free standing function. An added wrinkle is hardware wants little endian data and software presents big endian data, so HashMultipleBlocks accepts a ByteOrder for the incoming data. Hardware based SHA{N}_HashMultipleBlocks can often perform the endian swap much easier by setting an EPI mask so it was profitable to defer to hardware when available.

The rework also removed the hacked-in pointers to implementations. The class now looks more like AES, GCM, etc.
noloader referenced this issue Aug 14, 2017
Update comments and use class constants when available
@noloader
Copy link
Collaborator Author

Self tests checked in at:

Fix committed at:

Used anonymous namespace and class constants:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant