openssl icon indicating copy to clipboard operation
openssl copied to clipboard

Add CTX copy function for EVP_MD to optimize the performance of EVP_MD_CTX_copy_ex

Open bangcheng opened this issue 1 year ago • 5 comments

  1. Add OSSL_FUNC_digest_copyctx_fn function for EVP_MD, which is used to copy algctx from the old EVP_MD_CTX to the new one.

  2. Add implementation of OSSL_FUNC_digest_copyctx_fn function for default providers.

  3. Modify EVP_MD_CTX_copy_ex: When the fetched digest is the same in in and out contexts, use the copy function to copy the members in EVP_MD_CTX if the OSSL_FUNC_digest_copyctx_fn function exists. Otherwise, use the previous method to copy.

CLA:trivial

Fixes #25703

bangcheng avatar Oct 17 '24 13:10 bangcheng

I can't justify CLA:trivial, just the changes in crypto/evp/digest.c are too important.

Otherwise, the changes look good... would you mind adding a bit of documentation in doc/man7/provider-digest.pod, though?

levitte avatar Oct 17 '24 13:10 levitte

Could you please submit a CLA? https://openssl-library.org/policies/cla/index.html

Also please remove the CLA: trivial annotation from the commit message.

t8m avatar Oct 17 '24 15:10 t8m

Agreed, this is definitely not a trivial change.

Should the copy context function be expanded to more digests? Most currently support the operation at a low level.

paulidale avatar Oct 17 '24 22:10 paulidale

This needs tests including ones that test a copy where the two contexts are from different providers.

paulidale avatar Oct 17 '24 22:10 paulidale

I'll add test cases, man doc, and sign cla later.

bangcheng avatar Oct 18 '24 03:10 bangcheng

Should the copy context function be expanded to more digests? Most currently support the operation at a low level.

I have added copyctx to all the digests I know. What other digests in openssl need to be expanded?

bangcheng avatar Oct 21 '24 02:10 bangcheng

All review problems have been fixed. @t8m

bangcheng avatar Oct 28 '24 01:10 bangcheng

Your review comment has been modified, please review the code. @beldmit

bangcheng avatar Nov 18 '24 01:11 bangcheng

This pull request is ready to merge

openssl-machine avatar Nov 20 '24 13:11 openssl-machine

Merged to the master branch. Thank you for your contribution.

t8m avatar Nov 20 '24 13:11 t8m