Skip to content

[openssl] port update to OpenSSL 3.0.0 #20428

Closed
luncliff wants to merge 10 commits intomicrosoft:masterfrom
luncliff:port/openssl3
Closed

[openssl] port update to OpenSSL 3.0.0 #20428
luncliff wants to merge 10 commits intomicrosoft:masterfrom
luncliff:port/openssl3

Conversation

@luncliff
Copy link
Copy Markdown
Contributor

@luncliff luncliff commented Sep 29, 2021

What does your PR fix?

Resolve #20031

Most of the part is from #17949.

Currently, no feature is defined. It was too many for me to check.
Hope (future) actual users can contribute to it.

Which triplets are supported/not supported? Have you updated the CI baseline?

Every non-community triplet in the mainstream.

Does your PR follow the maintainer guide?

Checked.

  • Build Techniques
    • Do not use vendored dependencies --> no-zlib, no-makedepend
  • Versioning
    • version-semver
    • Update the version files in versions/ of any modified ports
  • Enable existing users of the library to switch to vcpkg

@luncliff luncliff marked this pull request as draft September 29, 2021 14:36
@JackBoosY JackBoosY added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Sep 30, 2021
@luncliff luncliff marked this pull request as ready for review October 4, 2021 06:45
@ras0219-msft
Copy link
Copy Markdown
Contributor

ras0219-msft commented Oct 5, 2021

We will be seeking to replace our openssl port with this; could you move this PR to the name openssl instead?

@luncliff
Copy link
Copy Markdown
Contributor Author

luncliff commented Oct 6, 2021

Not sure I can work for existing ports with this. As I said it's too many for me.
Let me try...

@luncliff luncliff marked this pull request as draft October 6, 2021 01:29
@luncliff luncliff changed the title [openssl3] create a new port [openssl] port update to OpenSSL 3.0.0 Oct 6, 2021
@dg0yt
Copy link
Copy Markdown
Contributor

dg0yt commented Oct 6, 2021

We will be seeking to replace our openssl port with this; could you move this PR to the name openssl instead?

AFAIU there is relevant API change. Wouldn't it be better to use separate names? Or at least move the old port to openssl1? (Cf. the wxwidgets gtk problem on x64-linux.)

@luncliff
Copy link
Copy Markdown
Contributor Author

luncliff commented Oct 6, 2021

@dg0yt If so, I'd like to hear about the vcpkg team opinion. I didn't check there was an API change.

@dg0yt
Copy link
Copy Markdown
Contributor

dg0yt commented Oct 6, 2021

@luncliff I don't know if it is that bad after re-reading https://www.openssl.org/blog/blog/2021/09/07/OpenSSL3.Final/:
"OpenSSL 3.0 is a major release and not fully backwards compatible with the previous release. Most applications that worked with OpenSSL 1.1.1 will still work unchanged and will simply need to be recompiled (although you may see numerous compilation warnings about using deprecated APIs). Some applications may need to make changes to compile and work correctly, and many applications will need to be changed to avoid the deprecations warnings. We have put together a migration guide to describe the major differences in OpenSSL 3.0 compared to previous releases."

@luncliff
Copy link
Copy Markdown
Contributor Author

luncliff commented Oct 6, 2021

Thanks for the link, @dg0yt. I will check it with the guides in the source. (I have to find where, but anyway)

vcpkg CI can check whether ports break or not. So I will try port rename then review the failure logs...
(If things are bad than expectation, I can revert them and use name openssl3).

@dg0yt
Copy link
Copy Markdown
Contributor

dg0yt commented Oct 6, 2021

vcpkg CI can check whether ports break or not.

Yes, but be aware of cascaded skips, in particular for linux, and non-default features. On windows and osx, the default features may select OS crypto functions instead of openssl.
There is also a license change. It is probably only an issue for GPL2-only software but IANAL.
(Anyway, I hope it is possible to just use the openssl port name.)

@wrobelda
Copy link
Copy Markdown
Contributor

wrobelda commented Oct 6, 2021

Yes, but be aware of cascaded skips, in particular for linux, and non-default features. On windows and osx, the default features may select OS crypto functions instead of openssl. There is also a license change. It is probably only an issue for GPL2-only software but IANAL. (Anyway, I hope it is possible to just use the openssl port name.)

All is true, but this wouldn't really be a problem if specifying a minimum/maximum/particular version of a dependency was actually possible in the manifest file.

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated
git add -u && git commit
git checkout 33f02c0ae50c262da487d21ace4f5d67ae949c18 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index 2a9df52..199fc1e 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -4900,6 +4900,10 @@
       "baseline": "1.1.1h",
       "port-version": 2
     },
+    "openssl1": {
+      "baseline": "1.1.1l",
+      "port-version": 1
+    },
     "opensubdiv": {
       "baseline": "3.4.3",
       "port-version": 2

* move baseline to openssl 3.0
Copy link
Copy Markdown
Contributor

@dg0yt dg0yt left a comment

Choose a reason for hiding this comment

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

👍 for this work.

I don't see the installation of pkg-config files. Ideally, there should be three .pc files, on every platform, cf. #20563.

Comment thread ports/openssl/portfile.cmake Outdated
Comment thread ports/openssl/portfile.cmake Outdated
get_filename_component(NASM_EXE_PATH ${NASM} DIRECTORY)
vcpkg_add_to_path(PREPEND "${NASM_EXE_PATH}")
# note: jom is not for `vcpkg_add_to_path`
vcpkg_find_acquire_program(JOM)
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.

Do we want to use jom when cross-compiling to windows?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We can use NMake but I wanted to use parallel build with it.

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.

No, it is not about jom vs nmake. It is about Linux hosts building with VCPKG_TARGET_IS_WINDOWS. We probably want to use make there. Since you start that block with if not else I thought you really mean VCPKG_TARGET_IS_WINDOWS not VCPKG_HOST_IS_WINDOWS, didn't you?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Indeed. I didn't considered cross compiling from Linux to Windows.

Copy link
Copy Markdown
Contributor Author

@luncliff luncliff Oct 7, 2021

Choose a reason for hiding this comment

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

I will try the case when I get some time. That would be an intersting case for this port 🤔

@luncliff
Copy link
Copy Markdown
Contributor Author

luncliff commented Oct 7, 2021

I don't see the installation of pkg-config files. Ideally, there should be three .pc files, on every platform, cf. #20563.

At a glance the project's windows-makefile.tmpl doesn't contain .pc file install. unix-makefile.tmpl has.

Supporting .pc for Windows sounds good to me. But can't find where the files are generated... yet.

@dg0yt
Copy link
Copy Markdown
Contributor

dg0yt commented Oct 7, 2021

But can't find where the files are generated... yet.

I see. IIRC I also looked into this aspect some months ago. It is probably easier to add three files to the port.

@luncliff
Copy link
Copy Markdown
Contributor Author

luncliff commented Oct 8, 2021

Work Note + Error Review (In Progress)

1. azure-c-shared-utility

Unexpected version range

#if (OPENSSL_VERSION_NUMBER >= 0x10100000L) && (OPENSSL_VERSION_NUMBER < 0x20000000L)
                SSL_CTX_clear_extra_chain_certs(ssl_ctx);
#else

Proper expression for OpenSSL 3.0 ?

#if (OPENSSL_VERSION_NUMBER >= 0x10100000L) && (OPENSSL_VERSION_NUMBER < 0x30100000L)
                SSL_CTX_clear_extra_chain_certs(ssl_ctx);
#else

2. libwebsockets, open62541

Using deprecaed API with -Werror=deprecated-declarations

3. libmysql, freerdp

Requires FIPS. This is removed API.
enable-fips won't work.

/home/luncliff/Desktop/vcpkg/buildtrees/libmysql/src/a9fb86d137-9c03d2b524/vio/viosslfactories.cc: In function ‘int set_fips_mode(uint, char*)’:
/home/luncliff/Desktop/vcpkg/buildtrees/libmysql/src/a9fb86d137-9c03d2b524/vio/viosslfactories.cc:500:19: error: ‘FIPS_mode’ was not declared in this scope
  500 |   fips_mode_old = FIPS_mode();
      |                   ^~~~~~~~~
/home/luncliff/Desktop/vcpkg/buildtrees/libmysql/src/a9fb86d137-9c03d2b524/vio/viosslfactories.cc:505:14: error: ‘FIPS_mode_set’ was not declared in this scope
  505 |   if (!(rc = FIPS_mode_set(fips_mode))) {
      |              ^~~~~~~~~~~~~
/home/luncliff/Desktop/vcpkg/buildtrees/libmysql/src/a9fb86d137-9c03d2b524/vio/viosslfactories.cc: In function ‘uint get_fips_mode()’:
/home/luncliff/Desktop/vcpkg/buildtrees/libmysql/src/a9fb86d137-9c03d2b524/vio/viosslfactories.cc:527:31: error: ‘FIPS_mode’ was not declared in this scope
  527 | uint get_fips_mode() { return FIPS_mode(); }
      |                               ^~~~~~~~~

4. s2n

Using deprecated API.
-Werror=discarded-qualifiers

/usr/bin/cc -D_POSIX_C_SOURCE=200809L -I/home/luncliff/Desktop/vcpkg/buildtrees/s2n/src/99fd2a9297-25115c1f26.clean -I/home/luncliff/Desktop/vcpkg/buildtrees/s2n/src/99fd2a9297-25115c1f26.clean/api -isystem /home/luncliff/Desktop/vcpkg/installed/x64-linux/include -fPIC -g -pedantic -std=gnu99 -Wall -Wimplicit -Wunused -Wcomment -Wchar-subscripts -Wuninitialized -Wshadow -Wcast-qual -Wcast-align -Wwrite-strings -Wno-deprecated-declarations -Wno-unknown-pragmas -Wformat-security -Wno-missing-braces -Wa,--noexecstack -Werror -fvisibility=hidden -DS2N_EXPORTS -DS2N_SIKE_P434_R3_ASM -DS2N_BIKE_R3_AVX2 -DS2N_BIKE_R3_AVX512 -DS2N_BIKE_R3_PCLMUL -DS2N_BIKE_R3_VPCLMUL -DS2N_KYBER512R3_AVX2_BMI2 -DS2N_ADX -DS2N_HAVE_EXECINFO -DS2N_CPUID_AVAILABLE -fPIC -DS2N_FALL_THROUGH_SUPPORTED -DS2N___RESTRICT__SUPPORTED -pthread -MD -MT CMakeFiles/s2n.dir/crypto/s2n_rsa_pss.c.o -MF CMakeFiles/s2n.dir/crypto/s2n_rsa_pss.c.o.d -o CMakeFiles/s2n.dir/crypto/s2n_rsa_pss.c.o -c /home/luncliff/Desktop/vcpkg/buildtrees/s2n/src/99fd2a9297-25115c1f26.clean/crypto/s2n_rsa_pss.c
/home/luncliff/Desktop/vcpkg/buildtrees/s2n/src/99fd2a9297-25115c1f26.clean/crypto/s2n_rsa_pss.c: In function ‘s2n_evp_pkey_to_rsa_pss_public_key’:
/home/luncliff/Desktop/vcpkg/buildtrees/s2n/src/99fd2a9297-25115c1f26.clean/crypto/s2n_rsa_pss.c:185:24: error: initialization discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
  185 |     RSA *pub_rsa_key = EVP_PKEY_get0_RSA(pkey);
      |                        ^~~~~~~~~~~~~~~~~
/home/luncliff/Desktop/vcpkg/buildtrees/s2n/src/99fd2a9297-25115c1f26.clean/crypto/s2n_rsa_pss.c: In function ‘s2n_evp_pkey_to_rsa_pss_private_key’:
/home/luncliff/Desktop/vcpkg/buildtrees/s2n/src/99fd2a9297-25115c1f26.clean/crypto/s2n_rsa_pss.c:195:25: error: initialization discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
  195 |     RSA *priv_rsa_key = EVP_PKEY_get0_RSA(pkey);
      |                         ^~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

5. drogon

Using deprecated API (MD5)

6. wangle

invalid conversion. -fpermissive.

FAILED: CMakeFiles/wangle.dir/acceptor/EvbHandshakeHelper.cpp.o 
/usr/bin/c++ -DFMT_LOCALE -DGFLAGS_DLL_DECLARE_FLAG="" -DGFLAGS_DLL_DEFINE_FLAG="" -DGFLAGS_IS_A_DLL=0 -DGOOGLE_GLOG_DLL_DECL="" -I/home/luncliff/Desktop/vcpkg/buildtrees/wangle/src/1.06.14.00-283fcf9ffc/wangle/.. -isystem /home/luncliff/Desktop/vcpkg/installed/x64-linux/include -fPIC -g -fPIC -std=c++17 -MD -MT CMakeFiles/wangle.dir/acceptor/EvbHandshakeHelper.cpp.o -MF CMakeFiles/wangle.dir/acceptor/EvbHandshakeHelper.cpp.o.d -o CMakeFiles/wangle.dir/acceptor/EvbHandshakeHelper.cpp.o -c /home/luncliff/Desktop/vcpkg/buildtrees/wangle/src/1.06.14.00-283fcf9ffc/wangle/acceptor/EvbHandshakeHelper.cpp
In file included from /home/luncliff/Desktop/vcpkg/installed/x64-linux/include/folly/portability/OpenSSL.h:39,
                 from /home/luncliff/Desktop/vcpkg/installed/x64-linux/include/folly/io/async/AsyncTransportCertificate.h:19,
                 from /home/luncliff/Desktop/vcpkg/installed/x64-linux/include/folly/io/async/AsyncTransport.h:25,
                 from /home/luncliff/Desktop/vcpkg/installed/x64-linux/include/folly/io/async/AsyncSocket.h:34,
                 from /home/luncliff/Desktop/vcpkg/buildtrees/wangle/src/1.06.14.00-283fcf9ffc/wangle/../wangle/acceptor/EvbHandshakeHelper.h:22,
                 from /home/luncliff/Desktop/vcpkg/buildtrees/wangle/src/1.06.14.00-283fcf9ffc/wangle/acceptor/EvbHandshakeHelper.cpp:17:
/home/luncliff/Desktop/vcpkg/buildtrees/wangle/src/1.06.14.00-283fcf9ffc/wangle/../wangle/ssl/SSLUtil.h: In static member function ‘static void wangle::SSLUtil::getSSLSessionExStrIndex(int*)’:
/home/luncliff/Desktop/vcpkg/buildtrees/wangle/src/1.06.14.00-283fcf9ffc/wangle/../wangle/ssl/SSLUtil.h:153:17: error: invalid conversion from ‘int (*)(CRYPTO_EX_DATA*, wangle::SSLUtil::ex_data_dup_from_arg_t, wangle::SSLUtil::ex_data_dup_ptr_arg_t, int, long int, void*)’ {aka ‘int (*)(crypto_ex_data_st*, const crypto_ex_data_st*, void*, int, long int, void*)’} to ‘int (*)(CRYPTO_EX_DATA*, const CRYPTO_EX_DATA*, void**, int, long int, void*)’ {aka ‘int (*)(crypto_ex_data_st*, const crypto_ex_data_st*, void**, int, long int, void*)’} [-fpermissive]
  153 |       *pindex = SSL_SESSION_get_ex_new_index(
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                 |
      |                 int (*)(CRYPTO_EX_DATA*, wangle::SSLUtil::ex_data_dup_from_arg_t, wangle::SSLUtil::ex_data_dup_ptr_arg_t, int, long int, void*) {aka int (*)(crypto_ex_data_st*, const crypto_ex_data_st*, void*, int, long int, void*)}
In file included from /home/luncliff/Desktop/vcpkg/installed/x64-linux/include/openssl/bio.h:30,
                 from /home/luncliff/Desktop/vcpkg/installed/x64-linux/include/openssl/asn1.h:27,
                 from /home/luncliff/Desktop/vcpkg/installed/x64-linux/include/folly/portability/OpenSSL.h:28,
                 from /home/luncliff/Desktop/vcpkg/installed/x64-linux/include/folly/io/async/AsyncTransportCertificate.h:19,
                 from /home/luncliff/Desktop/vcpkg/installed/x64-linux/include/folly/io/async/AsyncTransport.h:25,
                 from /home/luncliff/Desktop/vcpkg/installed/x64-linux/include/folly/io/async/AsyncSocket.h:34,
                 from /home/luncliff/Desktop/vcpkg/buildtrees/wangle/src/1.06.14.00-283fcf9ffc/wangle/../wangle/acceptor/EvbHandshakeHelper.h:22,
                 from /home/luncliff/Desktop/vcpkg/buildtrees/wangle/src/1.06.14.00-283fcf9ffc/wangle/acceptor/EvbHandshakeHelper.cpp:17:

@luncliff
Copy link
Copy Markdown
Contributor Author

luncliff commented Oct 8, 2021

Ah, indeed there is a removed API... It doesn't look like a simple problem if there are more.

@PhoebeHui PhoebeHui added category:port-update The issue is with a library, which is requesting update new revision and removed category:new-port The issue is requesting a new library to be added; consider making a PR! labels Dec 2, 2021
@luncliff
Copy link
Copy Markdown
Contributor Author

luncliff commented Dec 15, 2021

Sorry for no updates... I'm lack of time to keep up this work.

I don't know how to deal with those removed API now. 🤔 I will resume this work in next week

@JackBoosY
Copy link
Copy Markdown
Contributor

So can you continue this PR?

@luncliff
Copy link
Copy Markdown
Contributor Author

luncliff commented Jan 6, 2022

Honestly, no. I will close this. 😑

@luncliff luncliff closed this Jan 6, 2022
@talregev
Copy link
Copy Markdown
Contributor

@luncliff I think we should have do it in a different approach.
Do it as a separate port. opensssl3 like we have for opencv. and in the new port, we will not support backward computability or other packages that depend on the old openssl. that how slowly other packages will switch to new openssl.

@luncliff
Copy link
Copy Markdown
Contributor Author

Resurrecting e392457 is easy, but there was another releases after that. Hope there were not much changes.

Sadly I can't manage my schedule right now. 😭
I think I won't be able to do this until Feb week 2. I will try then if there is no volunteer for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:port-update The issue is with a library, which is requesting update new revision

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[openssl] add new port openssl 3.0.0

7 participants