Avoid unresolved symbols from libcrypto when compiled with OQS_DLOPEN_OPENSSL

[ueno]

When liboqs is compiled with OQS_DLOPEN_OPENSSL, the final shared library may contain unresolved symbols:

$ cmake -GNinja -DBUILD_SHARED_LIBS=ON -DOQS_USE_AES_OPENSSL=ON -DOQS_USE_AES_INSTRUCTIONS=OFF -DOQS_DIST_BUILD=ON -DOQS_USE_SHA3_OPENSSL=ON -DOQS_DLOPEN_OPENSSL=ON -DCMAKE_BUILD_TYPE=Debug -LAH ..
$ ninja
$ nm -g lib/liboqs.so.0.12.1-dev | grep '^[[:space:]]*U '
                 U __assert_fail@GLIBC_2.2.5
                 U CRYPTO_free
                 U CRYPTO_malloc
                 U dlopen@GLIBC_2.34
                 U dlsym@GLIBC_2.34

This PR fixes it in two ways: first only use OpenSSL memory functions when OQS_USE_OPENSSL=ON, and secondly wrap those symbols with OSSL_FUNC so they can be dynamically resolved.

• Does this PR change the input/output behaviour of a cryptographic algorithm (i.e., does it change known answer test values)? (If so, a version bump will be required from x.y.z to x.(y+1).0.)
• Does this PR change the list of algorithms available -- either adding, removing, or renaming? Does this PR otherwise change an API? (If so, PRs in fully supported downstream projects dependent on these, i.e., oqs-provider will also need to be ready for review and merge by the time this is merged.)

[baentsch]

Thanks for this PR, @ueno . At first glance, it seems it should also take into consideration #2039. Also, CI failures seem relevant to address before definite review.

[ueno]

@baentsch The <openssl/crypto.h> include has been moved to src/common/ossl_helpers.h, which is included when OQS_USE_OPENSSL is defined (without OPENSSL_VERSION_NUMBER check), so I believe #2038 is already addressed. I'm not sure what is the point of checking OPENSSL_VERSION_NUMBER in the first place though; liboqs requires OpenSSL 1.1.1 and OPENSSL_VERSION_NUMBER is defined there and the OPENSSL_*alloc and CRYPTO_*alloc etc. are also available and haven't changed since then.

[baentsch]

Thanks for the update @ueno : This IMO indeed seems to fix #2038 now. Tagging @SWilson4 for a second review as he already looked into this and @baluduvvuri1 for checking the PR addresses the concerns raised.

Just a final question: Would you care about adding a CI job testing also a config with OQS_DLOPEN_OPENSSL to avoid this from re-occurring, @ueno ?

[SWilson4]

Thanks for the contribution! I agree that this should fix #2039.

[baentsch]

So no CI testing this :-(

[SWilson4]

So no CI testing this :-(

We do have a couple of CI jobs that test the OQS_DLOPEN_OPENSSL option. I believe they were passing because the #2039 bug ensured that the OpenSSL memory management functions were never actually used.

[ueno]

So no CI testing this :-(

We do have a couple of CI jobs that test the OQS_DLOPEN_OPENSSL option. I believe they were passing because the #2039 bug ensured that the OpenSSL memory management functions were never actually used.

That said, I guess it wouldn't hurt to have an extra check to ensure there are no unresolved symbols. I've filed #2058 for that; sorry for not doing that when this PR was being reviewed.