Skip to content

[RFC] Remove OpenSSL 1.x support - #157

Closed
juimonen wants to merge 1 commit into
thesofproject:mainfrom
juimonen:remove_ssl1
Closed

[RFC] Remove OpenSSL 1.x support#157
juimonen wants to merge 1 commit into
thesofproject:mainfrom
juimonen:remove_ssl1

Conversation

@juimonen

Copy link
Copy Markdown

Remove OpenSSL 1.x support as it is reaching end-of-life in many distros.

@marc-hb

marc-hb commented Apr 14, 2023

Copy link
Copy Markdown
Contributor

This definitely requires a TEST PR in SOF before merge.

@marc-hb

marc-hb commented Apr 14, 2023

Copy link
Copy Markdown
Contributor

thesofproject/sof#7444 fails because (among others?) https://github.com/zephyrproject-rtos/docker-image/pkgs/container/zephyr-build v0.24 was still based on Ubuntu 20.04

@marc-hb

marc-hb commented Apr 14, 2023

Copy link
Copy Markdown
Contributor

The latest ghcr.io/zephyrproject-rtos/zephyr-build:v0.26.1 is based on Ubuntu 22.04, which has openssl3. Yeah!

However it also has /opt/toolchains/zephyr-sdk-0.16.0, which will mismatch with zephyr-sdk-0.15.2_windows-x86_64.zip (df1ba222306d)

@aborisovich could you please upgrade zephyr-sdk-0.15.2_windows-x86_64.zip to zephyr-sdk-0.16.0_windows-x86_64.zip in sof/.github/workflows/zephyr.yml?

Then this will be enough on the Linux side (TESTED!)

EDIT: this has already been merged, not needed anymore.

--- a/zephyr/docker-run.sh
+++ b/zephyr/docker-run.sh
@@ -54,7 +54,7 @@ main()
 
 run_command()
 {
-    # zephyr-build:v0.24.13 has /opt/toolchains/zephyr-sdk-0.15.2
+    # zephyr-build:v0.26.1 has /opt/toolchains/zephyr-sdk-0.16.0
     # https://hub.docker.com/r/zephyrprojectrtos/zephyr-build/tags
     #
     # Keep this SDK version identical to the one in
@@ -63,7 +63,7 @@ run_command()
            --workdir /zep_workspace \
            $SOF_DOCKER_RUN \
            --env REAL_CC --env http_proxy --env https_proxy \
-           ghcr.io/zephyrproject-rtos/zephyr-build:v0.24.13 \
+           ghcr.io/zephyrproject-rtos/zephyr-build:v0.26.1 \
            ./sof/scripts/sudo-cwd.sh "$@"
 }

Comment thread CMakeLists.txt
endif()

target_link_libraries(rimage PRIVATE crypto)
find_package(OpenSSL REQUIRED)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Could you go one step further and try this? It should make openssl 1 fail faster and much more clearly.

Suggested change
find_package(OpenSSL REQUIRED)
find_package(OpenSSL 3...3.9999 REQUIRED)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would hopefully provide a much clearer error than the D:/a/sof/sof/workspace/sof/rimage/src/pkcs1_5.c:93:15: error: implicit declaration of function ‘EVP_PKEY_private_check’; did you mean ‘EVP_PKEY_param_check’? and similar found in https://github.com/thesofproject/sof/actions/runs/4830399599/jobs/8606585483?pr=7444

@lgirdwood lgirdwood left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@juimonen any update ?

@juimonen

Copy link
Copy Markdown
Author

@lgirdwood not sure how we should test this.... @marc-hb any ideas? how could I use temporarily "test updated" build container, so to get all images signed with ssl3 and CI tested...

@marc-hb

marc-hb commented Apr 25, 2023

Copy link
Copy Markdown
Contributor

@juimonen you can test this with my zephyr/docker-run.sh patch above. This will make the build different from Windows which will temporarily break the Linux versus Windows check but for temporary testing that's OK.

EDIT: this will NOT test everything because some systems may not use that script. But it will provide some testing and feedback.

@marc-hb

marc-hb commented Apr 28, 2023

Copy link
Copy Markdown
Contributor

Linux build is OK but https://github.com/thesofproject/sof/actions/runs/4830399599/jobs/8606585483?pr=7444 looks like the Windows build misses OpenSSL3? @aborisovich can you help?

@marc-hb

marc-hb commented May 14, 2023

Copy link
Copy Markdown
Contributor

@marc-hb

marc-hb commented May 25, 2023

Copy link
Copy Markdown
Contributor

Great news: the Zephyr container has been upgraded to Ubuntu 22 by thesofproject/sof#7645 and we had some Windows updates too.

@juimonen can you give this another try?

@marc-hb marc-hb added the bug Something isn't working label May 25, 2023
@lgirdwood

Copy link
Copy Markdown
Member

@juimonen ping.

@lgirdwood

Copy link
Copy Markdown
Member

@juimonen ping

Remove OpenSSL 1.x support as it is reaching end-of-life in many
distros.

Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>
@juimonen

Copy link
Copy Markdown
Author

@lgirdwood @marc-hb @aborisovich still not compiling for windows... not sure can it be somehow updated for ssl3?
test PR here thesofproject/sof#7444

@lgirdwood

Copy link
Copy Markdown
Member

@mwasko @abonislawski @aborisovich SDL requires SSL3, need to update Windows.

@juimonen

Copy link
Copy Markdown
Author

@marc-hb @fredoh9 @aiChaoSONG (put here everyone who might know about CI)... I tried this locally with both sof docker image and zephyr docker image, and it compiles in both -> means they have ssl3 support. However there is some errors still in some CI compilation (even outside windows builds)... Can someone take a look at thesofproject/sof#7444 results, so do we need to do still some CI update, so that we don't have any ssl1 things left?

@marc-hb

marc-hb commented Jul 28, 2023

Copy link
Copy Markdown
Contributor

Can someone take a look at thesofproject/sof#7444 results, so do we need to do still some CI update, so that we don't have any ssl1 things left?

Yes, both the Jenkins nodes and Github Action for windows need some updates.

@lgirdwood

Copy link
Copy Markdown
Member

@wszypelt @mwasko fyi - needs Windows update to use sssl3. Thanks!

@wszypelt

wszypelt commented Jul 31, 2023

Copy link
Copy Markdown

@lgirdwood @mwasko @juimonen I will issue the appropriate ticket

@lgirdwood

Copy link
Copy Markdown
Member

@alex-cri fyi.

@kv2019i kv2019i left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marking as -1, this needs to be resubmitted directly to SOF repo (rimage moved to sof/tools/rimage).

@marc-hb

marc-hb commented Oct 11, 2023

Copy link
Copy Markdown
Contributor

To easily move these commits to the main sof repo, see:

marc-hb added a commit to marc-hb/sof that referenced this pull request Oct 18, 2023
Support unusual locations and prepares for OpenSSL 1 deprecation (as
previously attempted in thesofproject/rimage#157)

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
marc-hb added a commit to marc-hb/sof that referenced this pull request Oct 19, 2023
Support unusual locations and deprecates OpenSSL 1 (as previously
attempted in thesofproject/rimage#157)

Will remove the OpenSSL 1 code later to give people some time to
upgrade.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
Comment thread CMakeLists.txt

target_link_libraries(rimage PRIVATE crypto)
find_package(OpenSSL REQUIRED)
target_link_libraries(rimage PRIVATE crypto OpenSSL::SSL)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Smaller dependency enough:

Suggested change
target_link_libraries(rimage PRIVATE crypto OpenSSL::SSL)
target_link_libraries(rimage PRIVATE crypto OpenSSL::crypto)

(OpenSSL::SSL depends on OpenSSL::crypto)

@marc-hb

marc-hb commented Sep 5, 2024

Copy link
Copy Markdown
Contributor

Filed new issue:

@marc-hb marc-hb closed this Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants