vapor / postgres-kit

🐘 Non-blocking, event-driven Swift client for PostgreSQL.
MIT License
186 stars 70 forks source link

Standard sslmode URL params #247

Closed jphenow closed 1 year ago

jphenow commented 1 year ago

Is your feature request related to a problem? Please describe.

Fly.io generates Database URLs using somewhat standard sslmode params in the GET params. The URL parsing in postgres-kit doesn't take the more standard sslmode params into account as expressed in Postgres docs.

Postgres Docs specify a few options:

sslmode Eavesdropping protection MITM protection Statement
disable No No I don't care about security, and I don't want to pay the overhead of encryption.
allow Maybe No I don't care about security, but I will pay the overhead of encryption if the server insists on it.
prefer Maybe No I don't care about encryption, but I wish to pay the overhead of encryption if the server supports it.
require Yes No I want my data to be encrypted, and I accept the overhead. I trust that the network will make sure I always connect to the server I want.
verify-ca Yes Depends on CA policy I want my data encrypted, and I accept the overhead. I want to be sure that I connect to a server that I trust.
verify-full Yes Yes I want my data encrypted, and I accept the overhead. I want to be sure that I connect to a server I trust, and that it's the one I specify.

Where the current postgres-kit implementation accepts:

https://github.com/vapor/postgres-kit/blob/main/Sources/PostgresKit/SQLPostgresConfiguration.swift#L51C1-L54C37

ssl or tls (whichever is first) the values require, true, false.

It seems that require does the Postgres documented require above, true is prefer, false is disable.

Describe the solution you'd like

I presume some folks are already using these ssl/tls options that are described. I'd propose a solution that accepts the more specific sslmode rules and in the absence of sslmode follows the current behavior.

Perhaps the current behavior can be deprecated at some point to follow one implementation.

I opened a branch to illustrate. Excuse the amateur Swift https://github.com/vapor/postgres-kit/compare/main...jphenow:swift-postgres-kit:jphenow/sslmode-param

Describe alternatives you've considered

We were able to get around it by altering the params by hand.

This is less ideal because Fly.io users receive this URL in their app automatically. Having to dig around that URL and write a secret by hand for this isn't great.

Additional context

https://community.fly.io/t/error-on-deploy-with-vapor-migration/14399/6

gwynne commented 1 year ago

@jphenow Opened https://github.com/vapor/postgres-kit/pull/248 to take care of this and a few other outstanding annoyances about the URL parser in one go - let me know if I missed anything!

jphenow commented 1 year ago

This looks great! Thanks @gwynne!