usnistgov / ACVP-Server

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

X9.42 Concatenation KDF Issues #98

Closed powersmc closed 3 years ago

powersmc commented 3 years ago

environment Demo and Prod

testSessionId Demo - 138970 (good) and 150351 (bad) Production - 6540 and 6454 (both bad)

vsId Issue is present in: -Demo: 455797 -Production: 62795, 63323

Issue was not present in: -Demo: 436480

Algorithm registration { "algorithm":"kdf-components", "mode":"ansix9.42", "revision":"1.0", "kdfType":[ "concatenation" ], "keyLen":[ 384 ], "otherInfoLen":[ 384 ], "zzLen":[ { "min":64, "max":4096, "increment":1 } ], "hashAlg":[ "SHA-1", "SHA2-224", "SHA2-256", "SHA2-384", "SHA2-512", "SHA3-224", "SHA3-256", "SHA3-384", "SHA3-512" ] },

Endpoint in which the error is experienced /testSessions POST

Expected behavior With the original set of demo sample vectors referenced in this post, the vendor we're working with was obtaining the correct expected results. At some point between January 22, 2021 and April 19, 2021 it seems like there were changes pushed to the servers that have caused the server's expected results to deviate from the results being produced by our vendor's implementation. Some suspected changes would be: -https://github.com/usnistgov/ACVP/issues/1056 -https://github.com/usnistgov/ACVP/issues/1127

With the changes above both being for the "DER" encoding, and the vectors we're generating all being "concatenation", the expected behavior here would be that nothing would have changed server-side that would cause a sudden mismatch between the vendor's implementation's results and the server results. The vendor went from 100% passing results to now having mostly failing results (with a few passes scattered throughout)

Their current suspicion is that the 'OtherInfo' field calculation was unintentionally changed for the 'concatenation' encoded KDF as part of changes to the 'DER' encoding.

Additional context The vendor has done many investigations, inclusive of checking: -Checking if they were somehow failing to increment the counter -Checking input lengths for ZZ -Checking hash function selection -Checking even vs. odd, 4-byte, and 8-byte multiples to see if there are any obvious trends for the cases that now fail

Kritner commented 3 years ago

It's looking like this is an issue of us selecting zz values that don't fall on the byte boundary and not communicating the zzLen. Using this test case for example:

{
    "tgId": 1,
    "testType": "AFT",
    "hashAlg": "SHA-1",
    "kdfType": "concatenation",
    "tests": [{
        "tcId": 1,
        "zz": "51020F0B4423F066FFA2B9CA5FA536603F307A52048D7FBE4FA2D0A8F5B6ACF116BE68A28F6D21C37A4A76F082A5CE0D240FB84D6E3291C9B49E6865E76AB166934C701DB25A7FF77DE8849657244CB818B813B72D404D86DC1B0CFF64AFABFEE0CE24DF6DD6C537B6E58DBBC96DD4B18DDD0583699234B82BDDF4778236DD0B944E042BE5DA951E4B7D4962086964D0929F3641AB403CE4FA0AB9F2E500A5004786153E3E7740C41EA0C396773EC2D5D0E98837372491B97C42A336AEA3CF1F13C090CB3F45BA1AC67D763D09B9279D4158A6B41FB8C0E8515473F882174184EDDD5FAECC043E779DA7F0080CE31DE03B255DE038ACEC636F6CDF7DCD47B2C6757310179952B29F48DF510C2C204520C2313034DDB7080DE4F9018EA37756FEA41C285213E0216B5A3BA730AAC15C60EB72E5143656E2ADD533F7CACE1CCAE0100BDD789130A3B755669BC064DD31ECB3CFAB6E5766FA7194C982B79776D1F5FC1C1DECBE102096E14C7B30C8876757EB9F419002C6FC2DE88B98B1C784F3C0976447DC352D8BF56FBE255394E2FD3D7B5A4C9B4770",
        "keyLen": 384,
        "otherInfo": "82852BF64C6CF56399CC0E5F7ADAE2DF286B51E0D1A274BE949AF3D72A29CBB21F8AC17574BB12550804F4A8AA498082"
    }]
}

our "expected" derived key is: DD8469A101758AA949516B0AE4B8F6B56438A0A119575D3265163C6BBBEA2CB58629F387A1AA163D5AE164C5206E1DB5 according to our internal files at the time of generation. If I plug in the above test case again our crypto implementation, I get a different answer, one that matches your DKM of: 3898c4b53b7d0361d0447684a6f33b947ce72e6fcd3c37bc9ccb80eea878a16465238d754970d0bfab3603af263f150b.

As you stated we have had a few changes to this portion of our crypto in recent releases, so I started there, but nothing was obvious as to what could have changed this for concatenation mode. I then wrote a few tests for the above test case, where I'd take the full "zz" then chop off up to 8 bits and run through the crypto, eventually got a pass:

image

Basically we were generating zz values that weren't at the byte boundary, then not communicating the bit length of the zz value. This is most likely the case for all the failures in the vector set, and would explain why there's so many more failures, but not all failures; since some randomly chosen zz lengths would fall on the byte boundary.

Looking through https://csrc.nist.gov/CSRC/media/Events/Key-Management-Workshop-2000/documents/x942.pdf I'm going to assume that the zz values should be on a byte boundary, as well as the other inputs for this kdf (key length, other info length, supplemental info length).

I will try to get this assumption confirmed, but in the meantime, if a registration is provided with an increment of 8, that should cause our generation of zz to occur only on the byte boundary.

Example registration:

{
    "algorithm": "kdf-components",
    "mode": "ansix9.42",
    "revision": "1.0",
    "kdfType": [
        "concatenation"
    ],
    "keyLen": [
        384
    ],
    "otherInfoLen": [
        384
    ],
    "zzLen": [{
        "min": 64,
        "max": 4096,
        "increment": 8
    }],
    "hashAlg": [
        "SHA-1",
        "SHA2-224",
        "SHA2-256",
        "SHA2-384",
        "SHA2-512",
        "SHA3-224",
        "SHA3-256",
        "SHA3-384",
        "SHA3-512"
    ]
}
darrentjohns commented 3 years ago

Like in most specs, it can probably be debated either way.

In the X9.42 spec, section 7.7 defines the KDF as a standalone section and defines the ZZ input parameter as a bit string. So it could be argued that the KDF as it is defined in the X9.42 spec does support any bit length of input.

However this appears to be somewhat inconsistent with section 8 which outlines each individual scheme. Section 8 defines ZZ as oct(Ze), among other examples in section 8. The output of oct() is a octet string which is a multiple of 8-bits.

I agree with you that if a registration requests data in increments of 8-bits, then the generated vectors would have to respect that. I don't think there will be any disagreement on that, and that would trivially solve the majority of issues surrounding this.

As for supporting ZZ values that are any bit length, I don't see any need to support that in the context of X9.42. It isn't a valid permutation as per the spec as a whole. However the KDF as a standalone primitive does support bit strings.

Kritner commented 3 years ago

This change is on demo in release v1.1.0.18

Kritner commented 3 years ago

This change has been released to prod in release v1.1.0.18.