-
Notifications
You must be signed in to change notification settings - Fork 200
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
regression_4006: explicit ECDSA tests are SHA1 based #772
base: master
Are you sure you want to change the base?
Conversation
Explicitly use algorithm ID TEE_ALG_TEE_ECDSA_SHA1 for current ECDSA tests in regression_4006 instead of old API ID TEE_ALG_ECDSA_P* that are deprecated in GP TEE Internal Core API v1.3.1. This change is required to allow changes in OP-TEE OS where TEE_ALG_ECDSA_P* will no more straight alias IDs TEE_ALG_ECDSA_SHA* because the GP TEE specification says ECDSA key size is not related to the hash algorithm. Duplicate however few tests using legacy _OPTEE_ALG_ECDSA_P* for testing these OP-TEE specific still supported IDs. The key size for the related tests is found from the NIST key data material used in the test instead of the crypto algorithm ID when it does not provide this information. By the way, add an explicit inline comment telling why we currently force SHA1 hashing for TEE_MAIN_ALGO_ECDSA regression_4006 tests. Signed-off-by: Etienne Carriere <[email protected]>
e963428
to
f5690f9
Compare
Fix typos. Signed-off-by: Etienne Carriere <[email protected]>
* Current ECDSA tests are based on SHA1 hashed payload. | ||
* Current RSASSA_PKCS1_V1_5 tests are based on SHA256 | ||
* hashed payload. | ||
*/ | ||
if (TEE_ALG_GET_MAIN_ALG(tv->algo) == TEE_MAIN_ALGO_ECDSA) | ||
hash_algo = TEE_ALG_SHA1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about deriving this from the algorithm in the case of TEE_ALG_ECDSA_SHA*?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have the change ready to __tee_alg_get_digest_hash()
to support ECDSA algo IDs. I create a P-R.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, such change requires 1-to-1 relation ship between TEE_ALG_ECDSA_P*
and TEE_ALG_ECDSA_SHA*
is cut. That what's proposed in OP-TEE/optee_os#7251. I'll append a commit on that P-R.
Comments (bugs) addressed. |
Explicitly use algorithm ID TEE_ALG_TEE_ECDSA_SHA1 for current ECDSA tests in regression_4006 instead of old API ID TEE_ALG_ECDSA_P* that are deprecated in GP TEE Internal Core API v1.3.1. This change is required to allow changes in OP-TEE OS where TEE_ALG_ECDSA_P* will no more straight alias IDs TEE_ALG_ECDSA_SHA* because the GP TEE specification says ECDSA key size is not related to the hash algorithm.
Duplicate however few tests using legacy _OPTEE_ALG_ECDSA_P* for testing these OP-TEE specific still supported IDs.
The key size for the related tests is found from the NIST key data material used in the test instead of the crypto algorithm ID when it does not provide this information.
By the way, add an explicit inline comment telling why we currently force SHA1 hashing for TEE_MAIN_ALGO_ECDSA regression_4006 tests.