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

openssl binary usage #685

Closed
jonmason opened this issue Jul 14, 2023 · 9 comments
Closed

openssl binary usage #685

jonmason opened this issue Jul 14, 2023 · 9 comments

Comments

@jonmason
Copy link
Contributor

I came across this issue while building and testing optee with Yocto Project and the meta-arm layer

WARNING: optee-test-3.20.0-r0 do_package_qa: QA Issue: /usr/bin/xtest uses 32-bit api 'localtime_r'
/usr/bin/xtest uses 32-bit api 'time'
/usr/bin/xtest uses 32-bit api 'gmtime_r'

A quick grep finds the only instance of localtime_r to be
Binary file host/openssl/lib/aarch64/libcrypto.a matches
Binary file host/openssl/lib/arm/libcrypto.a matches

A git log on that file shows a single commit of

commit 27054ff
Author: Jerome Forissier jerome.forissier@linaro.org
Date: Tue Jun 5 11:09:01 2018 +0200

Add host/openssl/lib/{arm,aarch64}/libcrypto.a

Adds 32-bit and 64-bit builds of libcrypto.a (OpenSSL version 1.2.0o).

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Acked-by: Jens Wiklander <jens.wiklander@linaro.org>

Obviously, having such an old version of openssl is a security issue, along with the 2038 time issue that discovered it.
I humbly request that this file is removed completely, and either pull it in via a prebuild stage or build it from scratch.

@rossburton
Copy link
Contributor

As they're host binaries, presumably just expecting libcrypto on the host is acceptable?

@jforissier
Copy link
Contributor

As they're host binaries, presumably just expecting libcrypto on the host is acceptable?

The word "host" is incorrectly used here, we're talking about the client application (xtest) which contains the test drivers that invoke the TAs. So to be clear, this application runs on the target platform, in Linux (not in OP-TEE).

I agree that the OpenSSL binaries need to be removed. The CMake build doesn't use them already. For example the QEMU build documented at https://optee.readthedocs.io/en/latest/building/devices/qemu.html#qemu-v8 uses Buildroot and the CMake files and will build OpenSSL from source.

@jonmason
Copy link
Contributor Author

I was able to remove the openssl binaries and openssl include files, modify the host/xtest/Makefile to use -lcyrpto and not reference the openssl include files, and everything appears to be mostly happy. I had to add " -Wno-error=deprecated-declarations" because it is using old versions of some functions.

If you want, I can upload the changes as a PR.

@jforissier
Copy link
Contributor

@jonmason yes please!

@jenswi-linaro
Copy link
Contributor

Feel free to take this jenswi-linaro@7238495

@jonmason
Copy link
Contributor Author

@jenswi-linaro this looks good to me. Please close this issue when it is merged

@jforissier
Copy link
Contributor

Feel free to take this jenswi-linaro@7238495

The above commit could probably be merged without waiting for OP-TEE 4.x, provided that this is added too: #689 [1]. @jenswi-linaro @jonmason what do you think?

[1] Tested with the Linaro TRS build which has OpenSSL 3.0 which is why the PR is needed

@jenswi-linaro
Copy link
Contributor

Sound good to me. How about adding jenswi-linaro/optee_test@7238495 to #689?

@jforissier
Copy link
Contributor

Sound good to me. How about adding jenswi-linaro/optee_test@7238495 to #689?

Done!

@jonmason jonmason closed this as completed Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants