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

multissl: make openssl + wolfssl builds work #15596

Closed
wants to merge 12 commits into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Nov 16, 2024

  • make colliding vtls static function names unique.
  • wolfssl: stop including an unused compatibility header.
  • cmake: adapt detection logic for openssl+wolfssl coexist.
  • wolfssl: fix to use native wolfSSL API in ECH codepath.
  • openssl+wolfssl: fix ECH code to coexist.

Requires a post wolfSSL v5.7.4, recent master for OPENSSL_COEXIST
feature, and CPPFLAGS=-DOPENSSL_COEXIST.

Ref: wolfSSL/wolfssl#8194


@bagder
Copy link
Member

bagder commented Nov 16, 2024

You need wolfSSL from their git master still to do a combo-build with OpenSSL. I presume that will come proper in their next release.

@vszakats
Copy link
Member Author

vszakats commented Nov 16, 2024

You need wolfSSL from their git master still to do a combo-build with OpenSSL. I presume that will come proper in their next release.

Yes, I'm testing this with wolfSSL master. This clears the way for the wolfSSL feature when it's released.

@github-actions github-actions bot added the CI Continuous Integration label Nov 17, 2024
@vszakats vszakats force-pushed the unity-wolf-ossl-fix-1 branch 2 times, most recently from b6c7825 to ca72e1f Compare November 17, 2024 03:46
@vszakats vszakats changed the title vtls: fix unity build symbol collisions with openssl + wolfssl multissl: make openssl + wolfssl builds work Nov 18, 2024
@vszakats
Copy link
Member Author

It seems the way we build wolfSSL in CI isn't going to be
compatible with "coexist" mode, nor will be any distros
building it with --enable-all:

In file included from /path/to/wolfssl.git/_pkg/include/wolfssl/ssl.h:33:
/path/to/wolfssl.git/_pkg/include/wolfssl/wolfcrypt/settings.h:4239:6: error: "OPENSSL_ALL can not be defined with OPENSSL_COEXIST"
    #error "OPENSSL_ALL can not be defined with OPENSSL_COEXIST"
     ^

Such distros are Homebrew and Debian AFAIK.

@talregev
Copy link
Contributor

wolfssl vcpkg have now ech feature. Can you check that it correct and work in the tests?

@talregev
Copy link
Contributor

Can we add in the curl --version if the ech feature is working or not?

@vszakats
Copy link
Member Author

Can we add in the curl --version if the ech feature is working or not?

It's already there if enabled:

Features: [...] ECH [...]

@talregev
Copy link
Contributor

talregev commented Nov 26, 2024

Can we add in the curl --version if the ech feature is working or not?

It's already there if enabled:

Features: [...] ECH [...]

It not working, while wolfssl vcpkg is compile with:
What is missing?

      -DWOLFSSL_ECH=yes
      -DWOLFSSL_HPKE=yes
      -DWOLFSSL_SNI=yes

https://github.com/curl/curl/actions/runs/12034376811/job/33550798676?pr=15438#step:10:36

@talregev
Copy link
Contributor

Can we add in the curl --version if the ech feature is working or not?

It's already there if enabled:

Features: [...] ECH [...]

We need add a curl flag to enable ech?

@talregev
Copy link
Contributor

I now understand:
#15647

@vszakats
Copy link
Member Author

Can we add in the curl --version if the ech feature is working or not?

It's already there if enabled:

Features: [...] ECH [...]

We need add a curl flag to enable ech?

Yes, the two options already enabled for BoringSSL.

@talregev
Copy link
Contributor

Can we add in the curl --version if the ech feature is working or not?

It's already there if enabled:

Features: [...] ECH [...]

We need add a curl flag to enable ech?

Yes, the two options already enabled for BoringSSL.

I enable only -DUSE_ECH=ON for wolfssl. It have a compilation error. Am I missing something?
Can you take a look on my PR?

@talregev
Copy link
Contributor

talregev commented Dec 4, 2024

@vszakats Happy to see you that you back! :)

@vszakats
Copy link
Member Author

vszakats commented Dec 4, 2024

@vszakats Happy to see you that you back! :)

Thanks @talregev! :) I'm dealing with a little bit of offline stuff these weeks (and looking forward for the feature window).

@talregev
Copy link
Contributor

talregev commented Dec 4, 2024

Looking forward the feature windows too!
Good to see you back :)

@vszakats vszakats force-pushed the unity-wolf-ossl-fix-1 branch from 7538185 to 0f69c10 Compare December 17, 2024 03:18
@vszakats vszakats force-pushed the unity-wolf-ossl-fix-1 branch from 0f69c10 to 971cc24 Compare December 17, 2024 11:06
@vszakats
Copy link
Member Author

I dropped the auto-enable logic from this PR and will open it separately once a stable wolfSSL release allows it.

It means this PR is ready to merge to fix remaining coexist issues and enable using it with compatible wolfSSL revisions.

@vszakats vszakats closed this in fd067bf Dec 17, 2024
@vszakats vszakats deleted the unity-wolf-ossl-fix-1 branch December 17, 2024 11:35
vszakats added a commit to vszakats/curl that referenced this pull request Dec 31, 2024
vszakats added a commit to vszakats/curl that referenced this pull request Dec 31, 2024
vszakats added a commit to vszakats/curl that referenced this pull request Dec 31, 2024
vszakats added a commit that referenced this pull request Dec 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build CI Continuous Integration TLS
Development

Successfully merging this pull request may close these issues.

3 participants