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

General exception after submitting result for 1227513 #233

Closed jvdsn closed 1 year ago

jvdsn commented 1 year ago

environment Demo

testSessionId 318354

vsId 1227513

Algorithm registration Unsure, as the error only occurred after submitting the results. However, it's a pretty standard RSA keyGen registration, modulo sizes 2048, 3072, 4096, Appendix B.3.6, standard key format, random public exponent, Table C.2 prime test.

Endpoint in which the error is experienced POST results

Expected behavior The results are processed by the server.

Additional context The server returns an error:

[
  {
    "acvVersion": "1.0"
  },
  {
    "error": "General exception. Contact service provider."
  }
]
livebe01 commented 1 year ago

Hi @jvdsn, I took a first look at this and nothing jumped out as to why the validation is failing. I will keep looking and let you know what I find.

livebe01 commented 1 year ago

Hi @jvdsn, I'm still troubleshooting this, but it looks like we have an issue on our end with how we're verifying the primes that are being supplied.

jvdsn commented 1 year ago

Thanks for looking into it, let me know if there's anything I can do to help.

livebe01 commented 1 year ago

Sorry I haven't been able to get back to you on this before now. It's turned out to be a bit time consuming to debug and I've had to set it down and come back to it a few times. I don't have the complete answer for you at the moment, but I can tell you that the issue is with tcId: 7. Our code that attempts to build the keys using the parameters supplied by the IUT for tcId: 7 is erroring out.

livebe01 commented 1 year ago

Okay, it looks like when our code tries to generate p using the method described in FIPS 186-4 C.9, there is a failure at step #9 of the process (see step #9 in FIPS 186-4 C.9). I.e., our code is returning an error because i >= 5(nlen/2). Given the xP1 and xP2 provided in the response for tcId: 7, the implementation you're testing should have similarly errored out during the generation of p assuming it's compliant with FIPS 186-4. Let me know if you need any further information.

-Ben

jvdsn commented 1 year ago

Hi Ben,

Thanks for the detective work. I implemented C.9 myself to test and you are right. The implementation in question is actually OpenSSL. They justify their decision as follows:

    /*
     * In FIPS 186-4 imax was set to 5 * nlen/2.
     * Analysis by Allen Roginsky
     * (See https://csrc.nist.gov/CSRC/media/Publications/fips/186/4/final/documents/comments-received-fips186-4-december-2015.pdf
     * page 68) indicates this has a 1 in 2 million chance of failure.
     * The number has been updated to 20 * nlen/2 as used in
     * FIPS186-5 Appendix B.9 Step 9.
     */

https://github.com/openssl/openssl/blob/master/crypto/bn/bn_rsa_fips186_4.c#L343-L350

It seems like we are in the unfortunate 1 in 2 million case here. When I update my implementation to use 20 * nlen / 2, all works fine. I will open an issue with the OpenSSL team to see how to proceed here.

One final suggestion though: perhaps you can fail the test case with an informative error verdict. 1 in 2 million sounds like a very small probability, but apparently it happens...

livebe01 commented 1 year ago

Great, thanks for the followup. From what I understand, the move from 5 * nlen/2 to (4*5) * nlen/2 between FIPS 186-4 and the 186-5 draft is related to the addition of the optional requirement in B.9 step 4.1 of the 186-5 draft.

Agreed on the error handling. I'll look into how we can improve that.

livebe01 commented 1 year ago

The fix for this is on Demo in release v1.1.0.28.

livebe01 commented 1 year ago

The fix for this is on Prod in release v1.1.0.28.