QAT_Engine icon indicating copy to clipboard operation
QAT_Engine copied to clipboard

Missing "#if OPENSSL_VERSION_NUMBER < 0x30200000" and patch

Open psheer-spirent opened this issue 1 year ago • 7 comments

Sir,

Yourselves are missing many "#if OPENSSL_VERSION_NUMBER < 0x30200000" necessary for OpenSSL-3,2 compatibility.

Attached is a patch.

Thank you

Paul

patch-for-3.2.diff.gz.uu.txt

psheer-spirent avatar Mar 27 '24 17:03 psheer-spirent

I may have confused 3.2.0 and 3.2.1

psheer-spirent avatar Mar 27 '24 18:03 psheer-spirent

Hi Paul, When we checked with OpenSSL3.2 & Qatengine v1.5.0 we faced some issues and added required changes to support OpenSSL3.2 in Qatengine v1.6.0 and working as expected.

Did you observe any issues with the current changes added in Qatengine V1.6.0 to support OpenSSL3.2.1. Could you please share test details? It will help us to understand further.

bjayanax avatar Mar 28 '24 07:03 bjayanax

Yes. But my changes should be clear from the patch. Are you able to open the patch and see my changes?

psheer-spirent avatar Apr 01 '24 16:04 psheer-spirent

Yes, able to see your changes in the patch.  On top of our changes, we saw some additional code "export_types_ex and import_types_ex" for OpenSSL3.2 alone. Without these changes, OpenSSL 3.2 works fine for us. Could you please explain why we require this code?

bjayanax avatar Apr 02 '24 08:04 bjayanax

I have manually compared each struct used by QAT_Engine that does "shadow" a struct in the OpenSSL source. In the first place, it is an unfortunate engineering choice to copy struct definitions in this way such that there are two definitions that need to be maintained. In any case, since they are copied, it is prudent to compare them manually to each struct in the OpenSSL source and do so for every new commit of OpenSSL. For example QAT_Engine has a struct QAT_RSA_KEYMGMT which is a copy of the definition of struct evp_keymgmt_st / EVP_KEYMGMT. To make them identical, I have added fields as shown. I am unsure if it is true that absolutely every field is required, however without my patch there is misalignment in some tests and the implementation segfaults and dumps core.

If you say OpenSSL 3.2 works fine for you, then I would say that you have not tested every signature and KEM supported, since QAT_Engine will certainly segfault.

psheer-spirent avatar Apr 02 '24 17:04 psheer-spirent

Hi Paul, Would you kindly share the test commands that you have run on your end? which will be very beneficial to us. We have a plan to avoid duplication of structures further, thanks for the inputs and suggestion.

bjayanax avatar Apr 04 '24 09:04 bjayanax

Hi Paul, Would you kindly share the test commands

Our software suite is a very large infrastructure of many software components. We do not have a small code-sample that works standalone.

psheer-spirent avatar Apr 09 '24 19:04 psheer-spirent