Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Standard sslmode URL params #247

Closed
jphenow opened this issue Jul 26, 2023 · 2 comments · Fixed by #248
Closed

Standard sslmode URL params #247

jphenow opened this issue Jul 26, 2023 · 2 comments · Fixed by #248
Assignees
Labels
enhancement New feature or request

Comments

@jphenow
Copy link

jphenow commented Jul 26, 2023

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 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

@jphenow jphenow added the enhancement New feature or request label Jul 26, 2023
@gwynne gwynne self-assigned this Jul 27, 2023
@gwynne
Copy link
Member

gwynne commented Jul 27, 2023

@jphenow Opened #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
Copy link
Author

jphenow commented Jul 27, 2023

This looks great! Thanks @gwynne!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants