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 version in Windows runners #856

Closed
aantron opened this issue Sep 3, 2024 · 8 comments · Fixed by #892
Closed

OpenSSL version in Windows runners #856

aantron opened this issue Sep 3, 2024 · 8 comments · Fixed by #892
Labels

Comments

@aantron
Copy link
Contributor

aantron commented Sep 3, 2024

Building on windows-latest, installing package ssl 0.7.0 as a dependency results in

# ssl_stubs.c:311:32: error: 'SSL_OP_NO_TLSv1_3' undeclared here (not in a function); did you mean 'SSL_OP_NO_TLSv1_1'?
#   311 |                                SSL_OP_NO_TLSv1_3};
#       |                                ^~~~~~~~~~~~~~~~~
#       |                                SSL_OP_NO_TLSv1_1
# ssl_stubs.c: In function 'get_method':
# ssl_stubs.c:319:14: warning: implicit declaration of function 'TLS_client_method'; did you mean 'DTLS_client_method'? [-Wimplicit-function-declaration]
#   319 |     method = TLS_client_method();
#       |              ^~~~~~~~~~~~~~~~~
#       |              DTLS_client_method
# ssl_stubs.c:319:12: warning: assignment to 'const SSL_METHOD *' {aka 'const struct ssl_method_st *'} from 'int' makes pointer from integer without a cast [-Wint-conversion]
#   319 |     method = TLS_client_method();
#       |            ^
# ssl_stubs.c:323:14: warning: implicit declaration of function 'TLS_server_method'; did you mean 'DTLS_server_method'? [-Wimplicit-function-declaration]
#   323 |     method = TLS_server_method();
#       |              ^~~~~~~~~~~~~~~~~
#       |              DTLS_server_method
# ssl_stubs.c:323:12: warning: assignment to 'const SSL_METHOD *' {aka 'const struct ssl_method_st *'} from 'int' makes pointer from integer without a cast [-Wint-conversion]
#   323 |     method = TLS_server_method();
#       |            ^
# ssl_stubs.c:327:14: warning: implicit declaration of function 'TLS_method'; did you mean 'DTLS_method'? [-Wimplicit-function-declaration]
#   327 |     method = TLS_method();
#       |              ^~~~~~~~~~
#       |              DTLS_method
# ssl_stubs.c:327:12: warning: assignment to 'const SSL_METHOD *' {aka 'const struct ssl_method_st *'} from 'int' makes pointer from integer without a cast [-Wint-conversion]
#   327 |     method = TLS_method();
#       |            ^
# ssl_stubs.c: In function 'ocaml_ssl_version_of_tls_version':
# ssl_stubs.c:358:8: error: 'TLS1_3_VERSION' undeclared (first use in this function); did you mean 'TLS1_2_VERSION'?
#   358 |   case TLS1_3_VERSION:
#       |        ^~~~~~~~~~~~~~
#       |        TLS1_2_VERSION
# ssl_stubs.c:358:8: note: each undeclared identifier is reported only once for each function it appears in
# ssl_stubs.c: In function 'tls_version_of_ocaml_ssl_version':
# ssl_stubs.c:391:11: error: 'TLS1_3_VERSION' undeclared (first use in this function); did you mean 'TLS1_2_VERSION'?
#   391 |     ret = TLS1_3_VERSION;
#       |           ^~~~~~~~~~~~~~
#       |           TLS1_2_VERSION
# ssl_stubs.c: In function 'ocaml_ssl_ctx_set_min_proto_version':
# ssl_stubs.c:413:8: warning: implicit declaration of function 'SSL_CTX_set_min_proto_version'; did you mean 'ocaml_ssl_ctx_set_min_proto_version'? [-Wimplicit-function-declaration]
#   413 |   if (!SSL_CTX_set_min_proto_version(ssl_context, ssl_protocol)) {
#       |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#       |        ocaml_ssl_ctx_set_min_proto_version
# ssl_stubs.c: In function 'ocaml_ssl_ctx_get_min_proto_version':
# ssl_stubs.c:425:21: warning: implicit declaration of function 'SSL_CTX_get_min_proto_version'; did you mean 'ocaml_ssl_ctx_get_min_proto_version'? [-Wimplicit-function-declaration]
#   425 |   int tls_version = SSL_CTX_get_min_proto_version(ssl_context);
#       |                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#       |                     ocaml_ssl_ctx_get_min_proto_version
# ssl_stubs.c: In function 'ocaml_ssl_ctx_set_max_proto_version':
# ssl_stubs.c:[453](https://github.com/aantron/dream/actions/runs/10683497296/job/29611702223?pr=337#step:5:454):8: warning: implicit declaration of function 'SSL_CTX_set_max_proto_version'; did you mean 'ocaml_ssl_ctx_set_max_proto_version'? [-Wimplicit-function-declaration]
#   453 |   if (!SSL_CTX_set_max_proto_version(ssl_context, ssl_protocol)) {
#       |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#       |        ocaml_ssl_ctx_set_max_proto_version
# ssl_stubs.c: In function 'ocaml_ssl_ctx_get_max_proto_version':
# ssl_stubs.c:465:21: warning: implicit declaration of function 'SSL_CTX_get_max_proto_version'; did you mean 'ocaml_ssl_ctx_get_max_proto_version'? [-Wimplicit-function-declaration]
#   465 |   int tls_version = SSL_CTX_get_max_proto_version(ssl_context);
#       |                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#       |                     ocaml_ssl_ctx_get_max_proto_version
# ssl_stubs.c: In function 'set_protocol':
# ssl_stubs.c:485:19: error: 'TLS1_3_VERSION' undeclared (first use in this function); did you mean 'TLS1_2_VERSION'?
#   485 |   int max_proto = TLS1_3_VERSION;
#       |                   ^~~~~~~~~~~~~~
#       |                   TLS1_2_VERSION

We encountered this during aantron/dream#337, I've opened savonet/ocaml-ssl#155 and savonet/ocaml-ssl#156 to try to observe it from the ssl repo. It appears that an old version of libssl is being used, pre-1.1.1. Do you know where the libssl that is being linked with is coming from? Is this something that should be addressed by setup-ocaml?

@aantron
Copy link
Contributor Author

aantron commented Sep 5, 2024

We've reproduced this in the ocaml-ssl repo with @anmonteiro in savonet/ocaml-ssl#156, with the log here. Could you comment on what is needed to link against a modern libssl in the Windows runners with setup-ocaml, as this would be needed for testing any relatively serious networked application in GitHub Actions, not just in Dream?

@vouillon
Copy link
Member

It seems Github Actions are stuck with a very old version of OpenSSL: actions/runner-images#6830

@aantron
Copy link
Contributor Author

aantron commented Sep 23, 2024

Is setup-ocaml linking against some OpenSSL installed by Cygwin, or some kind of more "native" OpenSSL on Windows?

@smorimoto
Copy link
Member

Rather than setup-ocaml; opam is linking against to OpenSSL installed by Cygwin. It should be more of an issue on the savonet/ocaml-ssl side, and should work if proper support is added. CC: @dra27

@aantron
Copy link
Contributor Author

aantron commented Oct 4, 2024

This would seem to be an issue with which OpenSSL is installed by Cygwin, in that case, and what conf-libssl is doing.

@dra27
Copy link
Member

dra27 commented Oct 27, 2024

It is using a mingw build of OpenSSL (from mingw64-x86_64-openssl but the “stable” version is very out of date.

setup-ocaml could mitigate this if either a mechanism to specify additional Cygwin packages as an input is added or, cheekily, if mingw64-x86_64-openssl=1.1.1w-0.1 is added to https://github.com/ocaml/setup-ocaml/blob/master/packages/setup-ocaml/src/windows.ts#L91.

It has to be OpenSSL 1.1.1w as there seems to be a packaging error in the 3.x packages (but that’s enough for the OCaml ssl package). I’ve asked the maintainer to mark the 1.1.1 package as stable and hopefully fix the 3.x package (see here)

@smorimoto
Copy link
Member

Can't we just update our depext to a specific version meanwhile?

@dra27
Copy link
Member

dra27 commented Nov 11, 2024

Yes - by installing both mingw64-x86_64-openssl=1.1.1w-0.1 and mingw64-i686-openssl=1.1.1w-0.1 in windows.ts (as above)

punchagan added a commit to punchagan/semgrep that referenced this issue Jan 29, 2025
The version of openssl installed in Cygwin was updated based on this
report.[1]. But, tracing.ppx doesn't install and fails with a DLL error.
Using an older version of openssl seems to fix it.

[1]: ocaml/setup-ocaml#856
punchagan added a commit to punchagan/semgrep that referenced this issue Jan 29, 2025
The version of openssl installed in Cygwin was updated based on this
report.[1]. But, tracing.ppx doesn't install and fails with a DLL error.
Using an older version of openssl seems to fix it.

[1]: ocaml/setup-ocaml#856
punchagan added a commit to punchagan/semgrep that referenced this issue Jan 29, 2025
The version of openssl installed in Cygwin was updated based on this
report.[1]. But, we need an older version for the depexts compiled with
the stable version of openssl to work correctly.

[1]: ocaml/setup-ocaml#856 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants