Skip to content

Support postgres with SSL requirements #23

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

Merged
merged 3 commits into from
Feb 22, 2021

Conversation

ananace
Copy link
Contributor

@ananace ananace commented Jan 30, 2021

This currently just naively strips any ?sslmode= arguments from the postgres URI and enables SSL without verification, regardless of which mode the user specifies.

This stupidly just strips any ?sslmode= arguments from the postgres URI
and enables SSL without verification regardless of which mode the user
specifies.
@Half-Shot
Copy link

Fixes #24

src/database.rs Outdated
let mut client = Client::connect(db_url, postgres::NoTls).unwrap();
let mut client : postgres::Client;

if db_url.contains("sslmode=") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh of interest did you try without doing these checks? I have a feeling it Does The Right Thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't actually have any non-SSL Postgres cluster to test against at the moment, but my quick perusal of the code seemed to say that it was better to keep it at nossl - though I'm definitely no Rust expert

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just tested it and setting the TLS connector works even if TLS is disabled on postgres, so we can get rid of the else statement

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I'll clean up that bit as soon as I have time

@erikjohnston
Copy link
Member

Thanks for this! Sorry for forgetting about it...

@erikjohnston erikjohnston merged commit 4a56406 into matrix-org:master Feb 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants