usnistgov / ACVP-Server

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

TupleHash expectedResults.json sample files include the customization string for MCT when it shouldn't #211

Closed blackbird1999 closed 1 year ago

blackbird1999 commented 2 years ago

environment Demo

testSessionId N/A

vsId 0 (see https://github.com/usnistgov/ACVP-Server/blob/master/gen-val/json-files/TUPLEHASH-128-1.0/expectedResults.json)

Algorithm registration N/A

Endpoint in which the error is experienced N/A

Expected behavior It should match the ACVP specification and not include the customization string as documented in https://pages.nist.gov/ACVP/draft-celi-acvp-xof.html

Additional context https://pages.nist.gov/ACVP/draft-celi-acvp-xof.html section 8.3 details what the response data should include. It does not list the customization string as being included. It should just be the md and outLen. The ACVP server is outputting the customization string as well. It's unclear what the server expects from other clients. Maybe it's properly ignoring them? Maybe it requires them?

blackbird1999 commented 2 years ago

Also incorrectly outputs the intermediate tuples.

markowitz-isc commented 2 years ago

I agree that per the spec, resultsArray should not have tuple and customization properties, but in case anyone is interested, I want to point out that those values are Output[i] and Customization respectively, at the start of each inner loop, while md is, of course, the value of OutputJ[j] after the inner loop.

The main problem here, though, is that the line:

workingBits = Left(T[i-1][0] || ZeroBits(288), 288);

in the TupleHash MCT pseudocode (see draft-celi-acvp-xof.html#name-tuplehash-monte-carlo-test) should be

workingBits = Left(Output[i-1] || ZeroBits(288), 288);

with Output[0] properly initialized to the input tuple (rather than putting that in T[0][0])!

As the pseudocode stands, there is no feedback between the output of the hash function and workingBits -- in fact, its first 9 bytes and those of T[i][0] remain invariant, so tupleSize is also invariant. Clearly that is not what the server is executing.

livebe01 commented 2 years ago

@blackbird1999 thanks for mentioning these issues with https://github.com/usnistgov/ACVP-Server/blob/master/gen-val/json-files/TUPLEHASH-128-1.0/expectedResults.json. For the longest time, https://github.com/usnistgov/ACVP-Server/blob/master/gen-val/json-files/ were just example files that we referenced internally for our own purposes. Some of the capabilities registrations and expected results files, for example, happened to contain extra information/stuff that's not supposed to be there, but wouldn't necessarily cause an error on the server. But now that these are public domain and folks are looking at and referencing them, we should clean these up/make them correct. I'll fix TupleHash MCT and verify the other XOFs right now and we can see about doing a more comprehensive review of all the sample files in the future.

Also, I noticed that we don't have any example test results for TupleHash and ParallelHash here. I'll check into getting some examples into that doc as well.

livebe01 commented 2 years ago

The fix for this is now on Demo, v1.1.0.25.

livebe01 commented 1 year ago

The fix for this is now on Prod in release v1.1.0.25.