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

Crypto power-on self-test #1917

Open
mojtaba-bisheh opened this issue Jan 28, 2025 · 7 comments
Open

Crypto power-on self-test #1917

mojtaba-bisheh opened this issue Jan 28, 2025 · 7 comments
Assignees
Labels
Caliptra v2.0 Items to be considered for v2.0 Release

Comments

@mojtaba-bisheh
Copy link
Contributor

  1. we need to add SHA operation into ECDSA signing self-test
  2. we also need to have HMAC-DRBG self-test by using ECC KeyGen operation and comparing private key as a DRBG output
@swenson swenson assigned mhatrevi and unassigned vsonims Jan 28, 2025
@swenson swenson added the Caliptra v2.0 Items to be considered for v2.0 Release label Jan 28, 2025
@JohnTraverAmd
Copy link
Collaborator

@mojtaba-bisheh

I think this KAT is fulfilling your request, can you comment? https://github.com/chipsalliance/caliptra-sw/blob/main/kat/src/ecc384_kat.rs

@mojtaba-bisheh
Copy link
Contributor Author

Correct, we can also reuse it for HMAC-DRBG. The KEY_GEN_PRIV_KEY can be referred to as the HMAC-DRBG output.
I couldn't find the seed/nonce vectors in this file, but those are the same required inputs for HMAC-DRBG.

@nquarton
Copy link
Contributor

Hi Mojtaba. That KAT calls a KAT-specific wrapper for the key_gen function that fixes the seed and nonce to 0 as well as exposes the signature used during the PCT. Using 0s for the seed/nonce was done because the inputs for the KAT could be any fixed value and the hope was rust was going to optimize that a bit to take up less space in the ROM than some non-zero arbitrary data.

The resultant private key is verified against the expected value.

There isn't any SHA operation happening here that I am aware of though. Can you explain the reason that would be needed?

@mojtaba-bisheh
Copy link
Contributor Author

Based on feedback from the FIPS lab, we need to add a standalone KAT for DRBG in addition to the existing tests for ECC. Although this might seem redundant, it is a requirement for each cryptographic engine.

We must perform a KAT for the entire ECC flow, which includes both hashing and signing processes. The current KAT only verifies the signing steps. While we have an individual KAT for SHA, we need to integrate another KAT for the ECC signing/verifying process to meet the FIPS requirement of examining the entire flow.

@nquarton
Copy link
Contributor

I think we still have a disconnect here on the statement "The current KAT only verifies the signing steps". The signature is checked, but so it the public and private key generated. My understanding is that is the lowest level of granularity for exercising the DRBG that is accessible by firmware so it seems like that is the most a KAT would be able to verify.

I'll have to ask for more input from our FIPS consultants and other experts on the team for the requirement about performing hashing as well. It seems odd that we would need to be adding KATs for processes that combine discrete crypto engines that were tested separately. That would seem to open the door to many other permutations where multiple crypto engines are used in a process. But perhaps I am missing something that makes this a unique case.

@mojtaba-bisheh
Copy link
Contributor Author

mojtaba-bisheh commented Jan 29, 2025

The main reason is ECC signature generation includes both h= SHA(message) and (r,s)=SIGN(privkey, h).

That means ECC KATs require to verify (r,s) based on given message and privkey. However, the current KAT includes h and privkey as the input.

@nquarton
Copy link
Contributor

I see what you mean now. Yesterday afternoon I ran this by our FIPS consultant from KeyPair who also checked with a contact at Dekra and confirmed they think having the SHA engine tested separately and testing ECC with a fixed "hash" as the input should be allowed. One reason why this even makes more sense for Caliptra is because the hash of that message may come from different sources. Either the internal SHA engine, or the externally accessible SHA accelerator.

That being said, if there is still a concern about it, I don't see why this couldn't be changed in 2.0. There would still be the open of which SHA engine to use since I expect there to be some pushback with performing this twice (once for each engine) since this is quite a slow KAT.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Caliptra v2.0 Items to be considered for v2.0 Release
Projects
None yet
Development

No branches or pull requests

6 participants