elixir-nx / axon

Nx-powered Neural Networks
Apache License 2.0
1.55k stars 104 forks source link

Req usage in OTP 26 #570

Closed mwhitworth closed 5 months ago

mwhitworth commented 6 months ago

With the release of OTP 26, the client default verify option is now :verify_peer. (ref), which breaks Req downloads by default with certificate verification errors

To ensure that the examples in the documentation continue to work, there are two options:

  1. use libraries like tls_certificate_check to make it easy to establish more secure HTTPS connections regardless of OTP version:

    book_text = Req.get!("example.com", connect_options: [transport_opts: tls_certificate_check:options('example.com')]).body
  2. set request transport options to :verify_none, to restore the previous behaviour of skipping SSL certificate verification (by default)

I'm happy to implement either, but perhaps this has been solved elsewhere or with global defaults.

wojtekmach commented 6 months ago

Could you check on latest Req? Maybe it’s a matter of loosening up version requirement, eg:

https://github.com/elixir-nx/axon/blob/main/examples/generative/text_generator.exs#L6

Is not picking the latest version.

mwhitworth commented 6 months ago
iex> Application.spec(:req, :vsn)
~c"0.4.14"
iex> Req.get!("https://www.gutenberg.org").body
** (Mint.TransportError) TLS client: In state wait_cert_cr at ssl_handshake.erl:2126 generated CLIENT ALERT: Fatal - Bad Certificate

though curiously https://www.example.com works in this example without additional options.

Edit:

$ uname -a
Darwin dia 23.4.0 Darwin Kernel Version 23.4.0: Fri Mar 15 00:11:05 PDT 2024; root:xnu-10063.101.17~1/RELEASE_X86_64 x86_64
wojtekmach commented 6 months ago

Thanks, I can reproduce this locally, I will look into it.

wojtekmach commented 6 months ago

I could reproduce this with just erl:

~% rtx current erlang
26.2.5
~% erl
Erlang/OTP 26 [erts-14.2.5] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [jit]

Eshell V14.2.5 (press Ctrl+G to abort, type help(). for help)
1> ssl:start(), {ok, _} = ssl:connect("elixir-lang.org", 443, [{cacerts, public_key:cacerts_get()}]).
{ok,{sslsocket,{gen_tcp,#Port<0.5>,tls_connection,undefined},
               [<0.126.0>,<0.125.0>]}}
2> ssl:start(), {ok, _} = ssl:connect("www.gutenberg.org", 443, [{cacerts, public_key:cacerts_get()}]).
=NOTICE REPORT==== 8-May-2024::15:47:06.645138 ===
TLS client: In state wait_cert_cr at ssl_handshake.erl:2113 generated CLIENT ALERT: Fatal - Bad Certificate

** exception error: no match of right hand side value {error,{tls_alert,{bad_certificate,"TLS client: In state wait_cert_cr at ssl_handshake.erl:2113 generated CLIENT ALERT: Fatal - Bad Certificate\n"}}}

so it might be an OTP bug, I'll report this upstream.

wojtekmach commented 6 months ago

Per https://github.com/erlang/otp/issues/8466#issuecomment-2102641396

I noticed that the problem with the cert chain is that it has one cert that uses sha (sha1) in its signature algorithm and that is no longer supported by default.

It is marked as "not a bug" in OTP

Anyway, I could use this with Req and I got a successful response:

iex> options = [transport_opts: [signature_algs_cert: :ssl.signature_algs(:default, :"tlsv1.3") ++ [sha: :rsa]]]
iex> Req.get!("https://www.gutenberg.org", connect_options: options)

I am not sure what is the best path forward for Axon.

FWIW Req has a checksum feature so you can use that along with verify: :verify_none:

iex> options = [transport_opts: [verify: :verify_none]]
iex> Req.get!("https://www.gutenberg.org", connect_options: options, checksum: "md5:5f72f1f7bdee16a64e26b46cf2e7c40f")
mwhitworth commented 6 months ago

I am not sure what is the best path forward for Axon.

IMO, extending the first example quoted. We can view it as "configuration" for now and place the options variable at the head of each notebook. Then passing it to Req.get! calls seems like a reasonable way to go - as secure, functional and clear as possible for all OTP versions until the gutenberg.org certificate chain is updated.