lpil / pgo

🐘 Use PostgreSQL databases with PGO
https://hexdocs.pm/gleam_pgo/
Apache License 2.0
123 stars 12 forks source link

Add support for SSL options #22

Closed ghivert closed 2 months ago

ghivert commented 4 months ago

That PR add support for SSL options. This is required since OTP 26.

PR to merge after arrays has been merged. #22

lpil commented 4 months ago

Hello! I was under the impression that since OTP26 this is no longer required due to the default being to use system cacerts. This is why HTTPC et al can now be safely used without further configuration, while before you needed to manually provide this configuration for each connection.

Have you been having issues with TLS? Thanks

ghivert commented 4 months ago

That's weird, because I effectively had no problem with HTTPC without further configuration.

However, when trying to reach a Render (render.com) PG instance through HTTPS, I had to explicitly ask to disable peer verification. I can provide you an example if you need. And a non-working HTTPS endpoint too!


Quoting the docs, that's what I did:

In OTP 26, the default value for the verify option is now verify_peer instead of verify_none. Host verification requires trusted CA certificates to be supplied using one of the options cacerts or cacertsfile. Therefore, a connection attempt with an empty option list will fail in OTP 26:

Erlang/OTP 26 . . .

Eshell V14.0 (press Ctrl+G to abort, type help(). for help)
1> application:ensure_all_started(ssl).
{ok,[crypto,asn1,public_key,ssl]}
2> ssl:connect("www.erlang.org", 443, []).
{error,{options,incompatible,
                [{verify,verify_peer},{cacerts,undefined}]}}
The default value for the cacerts option is undefined, which is not compatible with the {verify,verify_peer} option.

To make the connection succeed, the recommended way is to use the cacerts option to supply CA certificates to be used for verifying. For example:

1> application:ensure_all_started(ssl).
{ok,[crypto,asn1,public_key,ssl]}
2> ssl:connect("www.erlang.org", 443, [{cacerts, public_key:cacerts_get()}]).
{ok,{sslsocket,{gen_tcp,#Port<0.5>,tls_connection,undefined},
               [<0.137.0>,<0.136.0>]}}
Alternatively, host verification can be explicitly disabled. For example:

1> application:ensure_all_started(ssl).
{ok,[crypto,asn1,public_key,ssl]}
2> ssl:connect("www.erlang.org", 443, [{verify,verify_none}]).
{ok,{sslsocket,{gen_tcp,#Port<0.6>,tls_connection,undefined},
               [<0.143.0>,<0.142.0>]}}
ghivert commented 3 months ago

Can confirm it's not working anymore with OTP26. Maybe we can wait for pgo issue to handle it correctly.

ghivert commented 2 months ago

Maybe we can have an API like

pub type Config {
  /// Other fields omitted for brevity
  Config(ssl: Ssl)
}

pub type Ssl {
  Disabled
  Enabled
  VerifyPeers(Cacerts)
}

pub type Cacerts {
  /// List of certificates to verify against.
  Cacerts(certs: List(BitArray))
  /// Filename of certificate to verify against.
  Cacertfile(filename: String)
}

That way, the SSL is always in a correct state:

The only required part would be to map that types to the proper pgo tuples used by pgo during initialization. Maybe we could do better and directly fetch the CA cert on the host? I can work on that if that can help to ship it faster. 😊 Meanwhile I'm forced to use my (dummy) implementation for SSL, and I suspect users that target OTP 26+ have to use a similar hack.

lpil commented 2 months ago

Wouldn't we always want to verify peers? What does enabled do if it doesn't verify?

ghivert commented 2 months ago

In some environments, the certificate can be self-issued, and you'd have to add the CA certificate to the chain. In this case, verification will fail if using standard certificates. Skipping verification would solve this (in an unsound way). It seems fair enough if you're running in a secured environment that requires you to use SSL (like, I don't know, AWS). I'm not sure about that argument, mainly making assumptions here.

lpil commented 2 months ago

Which option is skipping verification? Wouldn't that be a 4th option?

ghivert commented 2 months ago

I tried another way, and it's working like a charm! I'm curious to have your thoughts on that flow. In the end, I took decision to do an opinionated checking, otherwise SSL is useless. gleam_pgo now grabs the cacerts from the OS, and check everything correctly. The SSL module and some help from Erlang Forums helped me to have the correct defaults!


I'm also trying to see if providing defaults on pgo directly is not a better choice. Meanwhile, having defaults for gleam_pgo could be a nice way to have a solution right away, and to not be dependent of pgo. I let you see what seems best for you. 🙂

ghivert commented 2 months ago

It's done! Feel free to update the branch or merge whenever you want! For history, I'm trying to see what I can do for pgo in the erlang/pgo#80 issue. :)