insecure option for a host is persistent in a pool

[chulkilee]

It seems like hackeny does not handle request with different options for same host correctly.

Here is elixir code to demonstrate how insecure option is applied to future request for the same host in the same pool.

(env: hackney 1.13.0, elixir 1.8.2, erlang 22.0)

Application.ensure_all_started(:hackney)

bad_url1 = "https://expired.badssl.com/"
bad_url2 = "https://wrong.host.badssl.com/"

{:error, _} = :hackney.request(:get, bad_url1, [], [], [])
{:error, _} = :hackney.request(:get, bad_url2, [], [], [])
# => :error, as expected

{:ok, 200, _headers, ref} = :hackney.request(:get, bad_url1, [], [], [:insecure])

# before reading the body, other requests without insecure fail
{:error, _} = :hackney.request(:get, bad_url1, [], [], [])

# once the body is read..
{:ok, _body} = :hackney.body(ref)
# => :ok, as expected

{:ok, 200, _headers, ref} = :hackney.request(:get, bad_url1, [], [], [])
{:ok, _body} = :hackney.body(ref)
# WHAT? as insecure is missing, this should fail due to ssl error!

:hackney.request(:get, bad_url2, [], [], [])
# => :error, expected - so the config does not stick across hosts

{:ok, _, _, ref} = :hackney.request(:get, bad_url1 <> "/foo", [], [], [])
# => :ok... so seems like the config sticks with host

• Unfortunately, it's possible to "cancel" insecure - so there is no way to neutralize insecure
• Probably only way to reset completely is to kill the pool
• I'm not sure it applies to other options - such as ssl_options.
• This behavior (sharing some request options by pool) doesn't seem to be documented. Probably should be called as a bug

Workaround: for me, keep the separate pool per condition - but not without limitation anyway.

• single bad request (aginst other options in the pool) will remain
• for more dynamic option, it can be tricky to split pools (if other options have the same issue)

Application.ensure_all_started(:hackney)

bad_url1 = "https://expired.badssl.com/"
bad_url2 = "https://wrong.host.badssl.com/"

{:error, _ } = :hackney.request(:get, bad_url1, [], [], [pool: :secure_pool])
{:error, _ } = :hackney.request(:get, bad_url2, [], [], [pool: :secure_pool])
# => :error, as expected

{:ok, 200, _headers, ref} = :hackney.request(:get, bad_url1, [], [], [:insecure, pool: :insecure_pool])
{:ok, _body} = :hackney.body(ref)
# => :ok, as expected

{:error, _} = :hackney.request(:get, bad_url1, [], [], [pool: :secure_pool])
{:error, _} = :hackney.request(:get, bad_url2, [], [], [pool: :secure_pool])

# however.. still single bad request will update the config for this host :(
{:ok, _, _, ref} = :hackney.request(:get, bad_url2, [], [], [:insecure, pool: :secure_pool])
{:ok, _body} = :hackney.body(ref)

{:ok, _, _, ref} = :hackney.request(:get, bad_url2, [], [], [pool: :secure_pool])

[chulkilee]

Hm seems like I have the same problem with httpc

bad_url1 = "https://expired.badssl.com/"
bad_url2 = "https://wrong.host.badssl.com/"

# httpc

# httpc by default does not check ssl
{:ok, _} = :httpc.request(:get, {to_charlist(bad_url1), []}, [], [])

# verify ssl
{:error, _} =
  :httpc.request(
    :get,
    {to_charlist(bad_url1), []},
    [ssl: [verify: :verify_peer, cacertfile: :certifi.cacertfile()]],
    []
  )

# no verify again
{:ok, _} = :httpc.request(:get, {to_charlist(bad_url1), []}, [ssl: [verify: :verify_none]], [])

# should verify ssl but it DOES NOT
{:ok._()} =
  :httpc.request(
    :get,
    {to_charlist(bad_url1), []},
    [ssl: [verify: :verify_peer, cacertfile: :certifi.cacertfile()]],
    []
  )

[ericmj]

Verifications happen when a connection is started. hackney and httpc use connection pooling which reuse connections so you wont get verification on each request. The erlang :ssl module also has an option :reuse_sessions which means even on new connections you wont get verification if it is reusing an existing session.

[chulkilee]

If connection pool manager picks up previous connection with different connect opts, then that's bad behavior.

(I don't think httpc has built-in pool manager correct me if I'm wrong)

I passed reuse_sessions: false inside ssl option, but it still doesn't work (later request is picking up existing opts). Could you provide working code which makes three requests - verify, noverify, then verify?

# verify ssl explicilty - working
{:error, _} =
  :httpc.request(
    :get,
    {to_charlist(bad_url1), []},
    [
      ssl: [verify: :verify_peer, cacertfile: :certifi.cacertfile(), reuse_sessions: false]
    ],
    []
  )

# no verify explicitly - working
{:ok, _} =
  :httpc.request(
    :get,
    {to_charlist(bad_url1), []},
    [ssl: [verify: :verify_none, reuse_sessions: false]],
    []
  )

# verify ssl again explicitly - not working
{:ok, _} =
  :httpc.request(
    :get,
    {to_charlist(bad_url1), []},
    [ssl: [verify: :verify_peer, cacertfile: :certifi.cacertfile(), reuse_sessions: false]],
    []
  )

[ericmj]

Both hackney and httpc use connection pools by default. AFAIK neither of them use the request options to select different connections from the pool.

I would suggest you explicitly use different pools for different connection options or disable pooling and set reuse_sessions: false if you don't want this behavior.

[chulkilee]

Oh, I think the real issue is HTTP connection reuse. If I add connection: close HTTP header to explicit stop reusing HTTP connection, it works without reuse_sessions (as it always recreate HTTP connection). Erlang ssl doesn't say what's the default behavior for client side reuse_sessions - so probably it's up to how httpc or hackney calls ssl socket.

Then my point is: Using HTTP persistent connection without passing reference is bad, as two separate function calls may share the connection while they require different ssl options but the latter call's ssl options will not be honored.

At least, hackeny allows me to control them by using different pool per ssl option 🤔 (not by setting connection: calose to avoid http persistent connection.

Found that hackeny request functions do not take clientref - which makes impossible to control http persistent connection as caller needs. So, probably this is still valid issue in hackney.

[benoitc]

@chulkilee you can control connections per pool. Meaning you can start one pool for insecure connections and another cor secured one. Or if you want to use a single connection you can also use the send_req api like in this example.

[benoitc]

note that if all your connections are closing with the "Connection:close" headers, alls connections are never using the pool (checked in), so it should also work unless there is a bug. Can you share a snippet of what you're doing?

[chulkilee]

I did find those workarounds (already mentioned in the body).

you can control connections per pool. Meaning you can start one pool for insecure connections and another cor secured one

This is workaround to manage same SSL option per pool. However, single "bad request" (e.g. passing :insecure to "secure pool") may result in creating new connection in the pool with different "pool" config. See my (elixir) code in the issue body.

note that if all your connections are closing with the "Connection:close" headers, alls connections are never using the pool (checked in), so it should also work unless there is a bug

This workaround is also mentioned in previous comment. Note that this comes with performance impact.

At least, ideally a connection SHOULD NOT be (re)used for requests if the connection is made with different SSL option than the request.

I couldn't find docs on how existing connection from connection pool will be selected. Probably it should be mentioned that different ssl option is not considered in a pool.

[benoitc]

On Mon 27 May 2019 at 07:02, Chulki Lee [email protected] wrote:

I did find those workarounds (already mentioned in the body).

you can control connections per pool. Meaning you can start one pool for insecure connections and another cor secured one

This is workaround to manage same SSL option per pool. However, single "bad request" (e.g. passing :insecure to "secure pool") will result in updating the pool config. See my (elixir) code in the issue body

note that if all your connections are closing with the "Connection:close" headers, alls connections are never using the pool (checked in), so it should also work unless there is a bug

This workaround is also mentioned in previous comment. Note that this comes with performance impact.

At least, ideally a connection SHOULD NOT be (re)used for requests if the connection is made with different SSL option than the request.

I couldn't find docs on how existing connection from connection pool will be selected. Probably it should be mentioned that different ssl option is not considered in a pool

ssl options are set per connection, not per pool. But in the current versions options are not checked so connecting to the same host + port will reuse an available connection which may be secure or insecure. A simple fix even if it's expensive is to check against these options. There is a workaround for it in the release coming this week.

Benoît

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/benoitc/hackney/issues/570?email_source=notifications&email_token=AAADRIR57XFZ4QPIZFBMKCTPXNTNVA5CNFSM4HPWBHP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWIY2BY#issuecomment-496078087, or mute the thread https://github.com/notifications/unsubscribe-auth/AAADRIQDTOJHBVMRU2OERKDPXNTNVANCNFSM4HPWBHPQ .

-- Sent from my Mobile

[chulkilee]

Thanks a lot for the input and the update!

I think the naivest implementation is: dump ssl option into binary deterministic way (e.g. in elixir "default_" <> inspect(ssloptions)) and use it in pool name.

(It can be used for ddos attack if ssl option contains user input (ssl cert) - so hashing might be better choice.. :)

My thought is also at https://elixirforum.com/t/strange-httpc-hackeny-behavior-on-ssl-verify/22695/5