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

Add NIST cipher and hash tests vectors. #698

Closed
wants to merge 7 commits into from

Conversation

omasse-linaro
Copy link
Contributor

@omasse-linaro omasse-linaro commented Oct 11, 2023

Following the commit f6efe24 , proposition is to add more cipher and hash tests vectors to optee-test.

f6efe24 did not included CMake code : f69f34f is adding it

However, is there a reason why CMakeList.txt is not building the GCM NIST test vectors ?

Adds NIST AES-CMAC test vectors to regression case 4002 with
CFG_CMAC_NIST_VECTORS=y. Only the first test in each group is
used if CFG_CMAC_NIST_VECTORS_LIMITED=y.

Without CFG_CMAC_NIST_VECTORS=y, CFG_CMAC_NIST_VECTORS_LIMITED has no
effect.

Signed-off-by: Olivier Masse <[email protected]>
Adds NIST AES-XTS test vectors to regression case 4003 with
CFG_XTS_NIST_VECTORS=y. Only the first test in each group is
used if CFG_XTS_NIST_VECTORS_LIMITED=y.

Without CFG_XTS_NIST_VECTORS=y, CFG_XTS_NIST_VECTORS_LIMITED has no
effect.

Signed-off-by: Olivier Masse <[email protected]>
Adds NIST AES-CCM test vectors to regression case 4005 with
CFG_CCM_NIST_VECTORS=y. Only the first test in each group is
used if CFG_CCM_NIST_VECTORS_LIMITED=y.

Without CFG_CCM_NIST_VECTORS=y, CFG_CCM_NIST_VECTORS_LIMITED has no
effect.

Signed-off-by: Olivier Masse <[email protected]>
Makefile: Add CFG_GCM_NIST_VECTORS to CFLAGS

Signed-off-by: Olivier Masse <[email protected]>
Adds NIST SHA test vectors to regression case 4001 with
CFG_HASH_NIST_VECTORS=y.

Signed-off-by: Olivier Masse <[email protected]>
Adds NIST SHA3 test vectors to regression case 4001 with
CFG_HASH_NIST_VECTORS=y.

Signed-off-by: Olivier Masse <[email protected]>
Adds NIST HMAC test vectors to regression case 4002 with
CFG_HASH_NIST_VECTORS=y. Only the first test in each group is
used if CFG_HASH_NIST_VECTORS_LIMITED=y.

Without CFG_HASH_NIST_VECTORS=y, CFG_HASH_NIST_VECTORS_LIMITED has no
effect.

Signed-off-by: Olivier Masse <[email protected]>
@omasse-linaro omasse-linaro marked this pull request as ready for review October 18, 2023 07:51
@omasse-linaro
Copy link
Contributor Author

Hi @jenswi-linaro
Hi @jforissier

Do you have time to comments this PR ?

Best regards,
Olivier

@jbech-linaro
Copy link
Contributor

I like the idea of introducing test vectors. However, I also wonder whether it'd make sense to just run the conversion once to generate the necessary files, put them into the git and we're done. In the end the existing test vectors won't change (ever) I guess. Only new will be added whenever new algorithms are invented.

@jenswi-linaro
Copy link
Contributor

When commit f6efe24 ("regression: 4005: add NIST aes-gcm vectors") was added it was a quick and dirty to improve the confidence in our own AES-GCM implementation. Enabling all these tests take's quite a lot of memory, perhaps too much memory for a memory root file system. However, if we're to continue on this track I think that the scripts should at least be merged into one to avoid too much duplication.

Another option is to parse the .rsp files in xtest and run the tests on the fly. Perhaps something like:

bzcat gcm*.rsp.bz | xtest --rsp-aes-gcm

Or xtest itself could decompress the files if we'd rather run the tests as test cases.

@jenswi-linaro
Copy link
Contributor

I like the idea of introducing test vectors. However, I also wonder whether it'd make sense to just run the conversion once to generate the necessary files, put them into the git and we're done. In the end the existing test vectors won't change (ever) I guess. Only new will be added whenever new algorithms are invented.

Does the license of the .rsp files permit that?

@jbech-linaro
Copy link
Contributor

Does the license of the .rsp files permit that?

Good question, I find no license when opening the rsp-file itself.

@omasse-linaro
Copy link
Contributor Author

Does the license of the .rsp files permit that?

Good question, I find no license when opening the rsp-file itself.

me neither on the NIST website.

@omasse-linaro
Copy link
Contributor Author

omasse-linaro commented Nov 17, 2023

I like the idea of introducing test vectors. However, I also wonder whether it'd make sense to just run the conversion once to generate the necessary files, put them into the git and we're done. In the end the existing test vectors won't change (ever) I guess. Only new will be added whenever new algorithms are invented.

I've just created all the necessary files for NIST test vectors supported by this PR.
Result is around 70MB more of include files in the xtest/nist directory.
optee-test repo will noticeably become bigger.
if it is ok, I can create a new PR with all that files.

please find the new PR here: #707

@jforissier
Copy link
Contributor

I like the idea of introducing test vectors. However, I also wonder whether it'd make sense to just run the conversion once to generate the necessary files, put them into the git and we're done. In the end the existing test vectors won't change (ever) I guess. Only new will be added whenever new algorithms are invented.

I've just created all the necessary files for NIST test vectors supported by this PR. Result is around 70MB more of include files in the xtest/nist directory. optee-test repo will noticeably become bigger. if it is ok, I can create a new PR with all that files.

please find the new PR here: #707

As mentioned in #707 I much prefer the approach taken here, and in fact I even prefer what Jens suggested above:

Another option is to parse the .rsp files in xtest and run the tests on the fly. Perhaps something like:

bzcat gcm*.rsp.bz | xtest --rsp-aes-gcm

I find it quite elegant, kind of a minimal implementation so to speak.

@omasse-linaro
Copy link
Contributor Author

@jbech-linaro , should we keep #707 as an option ?
The parsing on the fly option is adding a new way to execute tests with xtest, which could add more complexity to the CI.
Moreover parsing rsp file in xtest C code is maybe not as elegant as doing it in python script.

@jbech-linaro
Copy link
Contributor

@jbech-linaro , should we keep #707 as an option ?

Was it meant to be addressed to me? Or was it Jerome you had in mind?

The parsing on the fly option is adding a new way to execute tests with xtest, which could add more complexity to the CI.
Moreover parsing rsp file in xtest C code is maybe not as elegant as doing it in python script.

I cannot make up my mind here. I also don't like to add 300k code like in #707 , I'm also not a fan of downloading a separate file every time running this. However, if we cache the rsp-file similar to what we do with the toolchains, buildroot cache and so on and package it into buildroot root-fs, so it's available when running xtest, then I think we've a pretty good trade-off in general.

I don't think this is what anyone is asking for, but to make it clear. I really don't want to try to download anything from within the environment itself (i.e., OP-TEE setup up and running to a prompt).

@jenswi-linaro
Copy link
Contributor

I also don't like to add 300k code like in #707 , I'm also not a fan of downloading a separate file every time running this.

TBH, I don't expect that we'll run this in CI. I suppose we'll run this on special occasions when someone feels like it.

@jforissier
Copy link
Contributor

@jenswi-linaro

I also don't like to add 300k code like in #707 , I'm also not a fan of downloading a separate file every time running this.

TBH, I don't expect that we'll run this in CI. I suppose we'll run this on special occasions when someone feels like it.

Agreed. For quarterly release testing for example.

@jbech-linaro

I don't think this is what anyone is asking for, but to make it clear. I really don't want to try to download anything from within the environment itself (i.e., OP-TEE setup up and running to a prompt).

That's a valid point. The target might not even have network access (unlikely but possible), which makes the "cat *.rsp | xtest --rsp-aes-gsm" solution impractical.

Also considering @omasse-linaro argument:

Moreover parsing rsp file in xtest C code is maybe not as elegant as doing it in python script.

...I believe what is proposed in this PR is a good compromise. Maybe as suggested by @jbech-linaro ("However, if we cache the rsp-file similar to what we do with the toolchains...") do not save the *.rsp files into $(out-dir) so they don't get deleted so easily since they basically never change.

@omasse-linaro
Copy link
Contributor Author

That's a valid point. The target might not even have network access (unlikely but possible), which makes the "cat *.rsp | xtest --rsp-aes-gsm" solution impractical.

indeed, moreover in our case, building with all supported NIST test vectors is a plus to stress test a new crypto driver supporting a new crypto hardware IP.

@jenswi-linaro
Copy link
Contributor

That's a valid point. The target might not even have network access (unlikely but possible), which makes the "cat *.rsp | xtest --rsp-aes-gsm" solution impractical.

My idea here was to download during build, unpack, re-zip, and copy it into the target file system.
I'm not too concerned about parsing the .rsp files in C.

@clementfaure
Copy link
Contributor

Hi all,
Do we have a consensus on this feature?
I believe the solution chosen is having the *.rsp test vector files parsed by xtest at runtime somehow, right?

On our side, we were fine with the header files generation at compilation time. I agree this might look a bit dirty but since it's made to be run on special occasions like releases, we believed that was a good compromise.

Please let us know what's your preference, so we can go forward with this PR.

@jforissier
Copy link
Contributor

@omasse-linaro my preference goes to runtime parsing by xtest indeed. Only if that's really impractical, then my second choice is generation of header files at compile time from the *.rsp.

@jenswi-linaro
Copy link
Contributor

@omasse-linaro my preference goes to runtime parsing by xtest indeed. Only if that's really impractical, then my second choice is generation of header files at compile time from the *.rsp.

+1

@clementfaure
Copy link
Contributor

clementfaure commented Nov 30, 2023

Ok, we did not really anticipate a re-work on this pull-request. We will close both PR and eventually come back to you later with something that meets your requirements.

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

Successfully merging this pull request may close these issues.

5 participants