ninenines / gun

HTTP/1.1, HTTP/2, Websocket client (and more) for Erlang/OTP.
ISC License
891 stars 232 forks source link

gun:open times out in OTP/26 for tls if tls options are not set #328

Closed peffis closed 3 months ago

peffis commented 5 months ago

It used to be, in older Erlang versions than 26, that you could just call gun:open on tls targets without specify an option, like so:

{ok, ConnPid} = gun:open("google.com", 443),
{ok, Protocol} = gun:await_up(ConnPid)

I have old code that opens connections like this and the gun examples on for instance https://ninenines.eu/docs/en/gun/1.3/manual/gun.open/ opens tls connections like this.

But whenever I do this now on OTP26 I get a timeout because of the following error in "initial_tls_handshake":

{options,incompatible,[{verify,verify_peer},{cacerts,undefined}]}

I believe Erlang ssl is more strict these days in how you configure ssl, so, in order to get it to work, I need to always do this instead:

TLSOpts = [{verify, verify_peer}, {cacerts, public_key:cacerts_get()}],
{ok, ConnPid} = gun:open("google.com", 443, #{tls_opts => TLSOpts}),
{ok, Protocol} = gun:await_up(ConnPid)

There are two problems here as the error about incompatible options is not propagated back, so one eventually just gets a timeout from gun which makes it hard to figure out that there was a configuration error to begin with. The other problem is that the new Erlang version breaks the API for gun as one needs to specify a tls option now. I have briefly looked into the gun code but I haven't figured out why it gets this config wrong yet.

The OTP version I tested this on is 26.0.2 (on a mac though, but probably that is not the cause)

essen commented 5 months ago

To be fair you always needed to configure TLS otherwise you wouldn't get a secure connection. That aside, the plan is to use the public_key:cacerts_get() by default in a future version. This function was what was missing for Gun to provide secure TLS connections by default.

About the timeout, this likely needs to be fixed to behave better. I believe if you increase the await_up timeout you should get the error propagated. It just propagates too late. Gun probably shouldn't retry if it gets an options incompatible error since retrying won't get it to succeed.

peffis commented 5 months ago

...ok, all good then. I used this in some internal old test and was confused that it stopped working when erlang was upgraded. I was not aware that it did not verify the host before when opened without options, but that does not matter for my use case. Feel free to close this issue if you want then. Just wanted to document it somewhere if someone else stumbles upon this issue with old code and spends time on understanding why there is suddenly a timeout.

essen commented 5 months ago

I'll leave it open to make Gun stop early when configuration is wrong. Thank you!

tamwile commented 5 months ago

As an erlang beginner, this was very confusing, and i ended up losing a lot of time yesterday. I was so lost, i decided to try httpc and hackney, which were working. But i knew i would eventually need websocket support, so i went back to gun, and i saw this issue. So thank you peffis.

As a beginner, it's far easier to think that you don't understand something and the mistake is on your part than thinking a bug is in widely used lib with good reputation and with a fairly large community.

"To be fair you always needed to configure TLS otherwise you wouldn't get a secure connection." But on the page "Connection", in the section "Opening a new connection", it is said: "If the port given is 443, Gun will attempt to connect using TLS." with this example provided: {ok, ConnPid} = gun:open("example.org", 443). So if TLS needed to always be configured even in previous version, the doc is still misleading.

essen commented 5 months ago

Yes the examples no longer work because OTP-26 broke them:

  OTP-18455    Application(s): ssl
               Related Id(s): GH-5899

               *** POTENTIAL INCOMPATIBILITY ***

               Change the client default verify option to verify_peer.
               Note that this makes it mandatory to also supply
               trusted CA certificates or explicitly set verify to
               verify_none. This also applies when using the so called
               anonymous test cipher suites defined in TLS versions
               pre TLS-1.3.

Before, the connection would be established, but it couldn't be considered secure if you didn't provide additional configuration. Now the connection cannot be established.

As you can expect, this will be handled in a future Gun release, using the function I mentioned. Patches are of course welcome. There will likely be a Gun release soon, after Cowboy 2.11, before the HTTP/3 work gets merged into both Cowboy and Gun. That release would be a good fit for changing this behavior.

zuiderkwast commented 4 months ago

public_key:cacerts_get() was added in OTP 25 so I suppose we need to use a feature check like erlang:function_exported/3. What default should we use for OTP < 25?

Should we add verify_none to the examples? Or can we add {cacerts, public_key:cacerts_get()} to the examples and write a note that they require OTP 25+?

essen commented 4 months ago

OTP < 25.0 can keep the current behavior and we can use cacerts_get by default when it is available. Note that on 25 adding the cacerts doesn't mean verification is enabled. Perhaps when cacerts_get is available we can enable verification explicitly (unless the user configured verify or cacerts already).

Examples shouldn't need to be updated. We can decide on what version we target for the next version later, and make version-specific notes then.

essen commented 3 months ago

This was done in Gun 2.1. Closing, thanks!

randysecrist commented 1 month ago

@essen thanks as always for the continual updates