elixir-mint / mint

Functional HTTP client for Elixir with support for HTTP/1 and HTTP/2 🌱
Apache License 2.0
1.36k stars 112 forks source link

Proper usage of :public_key.cacerts_get/0 #377

Closed nherzing closed 1 year ago

nherzing commented 1 year ago

The documentation shows that the result of :public_keys.cacerts_get/0 can be used as

Mint.connect(:https, host, port, transport_opts: [cacerts: :public_key.cacerts_get()])

(I'm assuming it should be Mint.HTTP.connect)

However, this raises the following for me

** (FunctionClauseError) no function clause matching in :public_key.pkix_decode_cert/2

    The following arguments were given to :public_key.pkix_decode_cert/2:

        # 1
        {:cert,
         <<48, 130, 4, ...
        # full cert details elided

        # 2
        :plain

    (public_key 1.13.1) public_key.erl:516: :public_key.pkix_decode_cert/2
    (elixir 1.14.1) lib/enum.ex:1658: Enum."-map/2-lists^map/1-0-"/2
    (mint 1.4.2) lib/mint/core/transport/ssl.ex:572: Mint.Core.Transport.SSL.add_partial_chain_fun/1
    (mint 1.4.2) lib/mint/core/transport/ssl.ex:443: Mint.Core.Transport.SSL.add_verify_opts/2
    (mint 1.4.2) lib/mint/core/transport/ssl.ex:432: Mint.Core.Transport.SSL.ssl_opts/2
    (mint 1.4.2) lib/mint/core/transport/ssl.ex:328: Mint.Core.Transport.SSL.connect/4
    (mint 1.4.2) lib/mint/negotiate.ex:67: Mint.Negotiate.connect_negotiate/4
    iex:1: (file)

Reading the erlang :public_key docs, this seems expected to me. Mint is passing the result of :public_key.cacerts_get/0 which is already decoded into :public_key.pkix_decode_cert/2 which expects a binary. Deleting line 572 of ssl.ex makes it function as expected but that would obviously break passing in encoded certs.

Is there some other existing way to make use of :public_keys.cacerts_get/0 in Mint that I am missing? Or will Mint need to be updated to properly work with :public_keys.cacerts_get/0?

Elixir version: 1.14.1 Erlang version: 25.1.1 Mint version: 1.4.2

whatyouhide commented 1 year ago

I think we'll need to patch Mint, yep. Any chance you'd like to work on a PR? πŸ™ƒ

beligante commented 1 year ago

Ohh, this is a good finding hahaha. I've merged an adapter to use mint on elixir-grpc https://github.com/elixir-grpc/grpc/pull/272 and I notice that ssl connections stoped to work.

@nherzing are you working on this? Otherwise I can

whatyouhide commented 1 year ago

@beligante go ahead please, very appreciated! πŸ’Ÿ πŸ™ƒ

beligante commented 1 year ago

Hey @whatyouhide working on this now - sorry for the delay.

I could follow a few options here. I like your input. The :public_key.cacerts_get/0 returns a list with a three position tuple. {:cert, encoded_cert_in_binary, #'OTPCertificate'{}}

in the map function I thought in add a pattern match case for handling this list

  defp decode_cacerts(certs) do
    Enum.map(certs, fn 
      cert when is_binary(cert) ->  :public_key.pkix_decode_cert(cert, :plain)
      {:cert, cert, _otp_cert} when is_binary(cert) -> :public_key.pkix_decode_cert(cert, :plain)
    end)
  end

Second option is enforce that option to be a list of binaries for that option and raise in case something doesn't match. This way we could simply update the docs like:

certs = :public_key.cacerts_get() |> Enum.map(fn {:cert, binary, _otp_cert} -> binary end)
Mint.connect(:https, host, port, transport_opts: [cacerts: :public_key.cacerts_get()])

Thoughts?

PS: @nherzing the above code could be an option for you to use the trusted erlang certs without code changes on mint

nherzing commented 1 year ago

@beligante Just seeing these comments now. I initially took a shot at making essentially the exact same change you are proposing. Testing with my local certs, I came across another case with a 4-tuple tagged with :Certificate. I also think in the case that we see a tuple, we can just pass the value through. I don't believe there's a reason to re-decode it. Having said that, I'm definitely out of my depth with Erlang's handling of ssl certs so take this all with a grain of salt.

defp decode_cacerts(certs) do
  Enum.map(certs, fn
    {:Certificate, _, _ , _} = cert -> cert
    {:cert, _, _} = cert -> cert
    cert -> :public_key.pkix_decode_cert(cert, :plain)
  end)
end

I ended up just directly specifying the certfile I needed as a workaround.

Mint.connect(:https, host, port, transport_opts: [cacertfile: Application.app_dir(:my_app) <> "/priv/internal_root.crt"])
pvthuyen commented 1 year ago

@beligante Are you still working on a PR for this? If not, I can probably try to work on it.

beligante commented 1 year ago

@beligante Just seeing these comments now. I initially took a shot at making essentially the exact same change you are proposing. Testing with my local certs, I came across another case with a 4-tuple tagged with :Certificate. I also think in the case that we see a tuple, we can just pass the value through. I don't believe there's a reason to re-decode it. Having said that, I'm definitely out of my depth with Erlang's handling of ssl certs so take this all with a grain of salt.

defp decode_cacerts(certs) do
  Enum.map(certs, fn
    {:Certificate, _, _ , _} = cert -> cert
    {:cert, _, _} = cert -> cert
    cert -> :public_key.pkix_decode_cert(cert, :plain)
  end)
end

I ended up just directly specifying the certfile I needed as a workaround.

Mint.connect(:https, host, port, transport_opts: [cacertfile: Application.app_dir(:my_app) <> "/priv/internal_root.crt"])

Sorry, I missed your message. But yeah. That makes sense. I thought that the OTP cert couldn't be used, as you mentioned. But quickly testing here. worked for me.

I'm also in a gray area for erlang and SSL here. I know a few things, but I would need some more depth to make sure that I'm doing the right this. Aka: study a little more.

That being said, @pvthuyen I'm not actively working on this. So, feel free to take this over :D I'll do my studying to understand what I'm doing. Hopefully, I can provide some help with reviews in an upcoming PR. Thanks!

pvthuyen commented 1 year ago

Thanks, I've sent a PR for this.