usnistgov / ACVP-Server

A repository tracking releases of NIST's ACVP server. See www.github.com/usnistgov/ACVP for the protocol.
39 stars 14 forks source link

SHA MCT with fixed size message smaller than digest*3 incorrect results and repeating expected MCT results when using a domain #272

Closed sbailey9 closed 10 months ago

sbailey9 commented 1 year ago

environment Demo v1.1.0.29

testSessionId 418518

vsId 1709735 1709736

Algorithm registration { "algorithm": "SHA2-384", "revision": "1.0", "messageLength": [800] }, { "algorithm": "SHA2-384", "revision": "1.0", "messageLength": [{"min": 0, "max": 800, "increment": 8}] }

Expected behavior Following the updated MCT psuedo-code from ACVP should provide results matching the expected.

Additional context I have been following the updated MCT pseudo code from the ACVP SHA specification and have been unable to reproduce the expected results.

For vsId 1790735, the expected results array for the MCT is the same hash every time. The first md in the array after running my test is:935A8010D7E667D50727FF6540DEE1EDA336955D711056F3C43CFD9428B2F42592A1C3CCFDBA21A6570E91F4819D63BA

For vsId 1709736, when using a fixed size message of 800 and following the ACVP specification for the updated MCT test, I cannot produce the correct results for the MCT. In the psuedocode, if !SupportedMessageLengths.Contains(LEN(MSG)): after concatenating a,b,c we truncate back to the original message, this causes the same message to be hashed 3 times in a row. I am not sure if this is the expected behavior. The first md in my results array is: FE12F176FE76964B68A97D36B3F4191FF3B9590749F2503B5C28C937EF996EDB402E2EE5465543EC5FCF9858D84DAE19

celic commented 1 year ago

For the first one, it is likely the hash is the same every time because it is hashing a 1-bit message and the digest happens to have a MSB of 1, leading to the loop.

For the second one, this is OK. This could technically be avoided by doing C || B || A so the recently generated digest is taken in faster, but we decided against that to keep the pseudocode more similar to the original.

We are working on changes for this MCT to make it a bit simpler, and to account for the potential of the first case.

sbailey9 commented 1 year ago

I had the vsIDs mixed up on the first comment, 1709736 is the vector using the 0-800bits increment 8 registration and it is the one with every expected result for the MCT being the same hash. Why is this a 1-bit message? The increment is 8, so shouldn't my smallest supported message size be 8 bits not 1 bit?

livebe01 commented 1 year ago

Like Chris mentioned, we're redoing the MCT test and the updated testing will be included in the v1.1.0.30 release. I'm going to tag this ticket with v1.1.0.30. Can you retest when v1.1.0.30 hits demo later this week?

sbailey9 commented 1 year ago

Sure, I can retest when the demo server is updated.

livebe01 commented 1 year ago

Just a ping that the new MCT test is now on Demo in release v1.1.0.30.

livebe01 commented 11 months ago

Hi @sbailey9, just a ping to see if this is still an issue now that the v1.1.0.30 release is out. Thanks!

Kneckter commented 11 months ago

Hello @livebe01, I work with sbailey9 and we have started a test with demo vectors. We coordinate with a few teams so it's taking a while to get results. I'll let you know the outcome.

livebe01 commented 11 months ago

Sounds good. Thank you!

Ben

On Aug 1, 2023, at 1:43 PM, Kneckter @.***> wrote:

Hello @livebe01https://github.com/livebe01, I work with sbailey9 and we have started a test with demo vectors. We coordinate with a few teams so it's taking a while to get results. I'll let you know the outcome.

— Reply to this email directly, view it on GitHubhttps://github.com/usnistgov/ACVP-Server/issues/272#issuecomment-1660804444, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AGMA5ASXMS7LLNBHLYS3TSLXTE54NANCNFSM6AAAAAAZ2CRTSQ. You are receiving this because you were mentioned.Message ID: @.***>

Kneckter commented 11 months ago

Hello @livebe01, our testing did pass on a set of demo vectors. Sorry about the slow reply; we had to make changes to our code. You can close this issue

livebe01 commented 10 months ago

Great. Thank you!