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

Enable setting custom pgsql connection parameters #4711

Merged
merged 1 commit into from Jan 26, 2017

Conversation

Projects
None yet
6 participants
@thusoy
Contributor

thusoy commented Nov 26, 2016

Short description

Enable setting custom pgsql connection parameters, like TLS parameters.

Should this be added to documentation or examples anywhere? To validate TLS connections against a local trust root you can set gpgsql-extra-connection-args=sslmode=verify-ca sslrootcert=<path-to-certs>.

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled and tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added regression tests
  • added unit tests
  • checked that this code was merged to master

Closes #2138.

@thusoy

This comment has been minimized.

Show comment
Hide comment
@thusoy

thusoy Nov 26, 2016

Contributor

I tested that this actually stops a MitM by using this script: https://raw.githubusercontent.com/thusoy/postgres-mitm/master/postgres_mitm.py. As long as postgres is set to accept connections over tls (hostssl option in pg_hha.conf and ssl on; in postgresql.conf, run this script with python postgres_mitm.py -p 5433 127.0.0.1 and set pdns to connect to localhost on 5433, you'll see that the mitm can intercept the authentication. If this patch is applied and gpgsql-extra-connection-args=sslmode=verify-ca sslrootcert=<path-to-certs> is set, the connection will be refused and the mitm won't learn the credentials.

Contributor

thusoy commented Nov 26, 2016

I tested that this actually stops a MitM by using this script: https://raw.githubusercontent.com/thusoy/postgres-mitm/master/postgres_mitm.py. As long as postgres is set to accept connections over tls (hostssl option in pg_hha.conf and ssl on; in postgresql.conf, run this script with python postgres_mitm.py -p 5433 127.0.0.1 and set pdns to connect to localhost on 5433, you'll see that the mitm can intercept the authentication. If this patch is applied and gpgsql-extra-connection-args=sslmode=verify-ca sslrootcert=<path-to-certs> is set, the connection will be refused and the mitm won't learn the credentials.

@thusoy

This comment has been minimized.

Show comment
Hide comment
@thusoy

thusoy Dec 4, 2016

Contributor

@rgacogne Anything more I can do to get this merged? Should I add documentation somewhere?

Contributor

thusoy commented Dec 4, 2016

@rgacogne Anything more I can do to get this merged? Should I add documentation somewhere?

@zeha

This comment has been minimized.

Show comment
Hide comment
@zeha

zeha Dec 4, 2016

Collaborator

Please add docs, indeed. Apart from that, I think the pdns team is just somewhat busy right now.

Collaborator

zeha commented Dec 4, 2016

Please add docs, indeed. Apart from that, I think the pdns team is just somewhat busy right now.

@rgacogne

This comment has been minimized.

Show comment
Hide comment
@rgacogne

rgacogne Dec 4, 2016

Member

@thusoy Thank you for this PR! Adding a few words in docs/markdown/authoritative/backend-generic-postgresql.md would be great, otherwise it looks fine to me.

Member

rgacogne commented Dec 4, 2016

@thusoy Thank you for this PR! Adding a few words in docs/markdown/authoritative/backend-generic-postgresql.md would be great, otherwise it looks fine to me.

@thusoy

This comment has been minimized.

Show comment
Hide comment
@thusoy

thusoy Dec 5, 2016

Contributor

@rgacogne Cool, wrote some words and included an example to show how it can be used. I also renamed the variable to _parameters instead of _args since the postgres docs are pretty consistent about calling it parameters.

Contributor

thusoy commented Dec 5, 2016

@rgacogne Cool, wrote some words and included an example to show how it can be used. I also renamed the variable to _parameters instead of _args since the postgres docs are pretty consistent about calling it parameters.

@zeha

zeha approved these changes Jan 23, 2017

Enable setting custom psql connection parameters
This makes it possible to ensure we're connecting over TLS and validate
the connection against a known CA. And everything else that be
configured as connection parameters, like TCP keepalive behavior,
timeouts, etc. Full spec:
https://www.postgresql.org/docs/current/static/libpq-connect.html#LIBPQ-PARAMKEYWORDS

Closes #2138.
@thusoy

This comment has been minimized.

Show comment
Hide comment
@thusoy

thusoy Jan 23, 2017

Contributor

@Habbie @zeha Good catch, both of you. Fixed.

Contributor

thusoy commented Jan 23, 2017

@Habbie @zeha Good catch, both of you. Fixed.

@Habbie

This comment has been minimized.

Show comment
Hide comment
@Habbie

Habbie Jan 23, 2017

Member

Ready for merge when Travis passes.

Member

Habbie commented Jan 23, 2017

Ready for merge when Travis passes.

@Habbie

Habbie approved these changes Jan 23, 2017

@thusoy

This comment has been minimized.

Show comment
Hide comment
@thusoy

thusoy Jan 26, 2017

Contributor

@Habbie Just a heads up, Travis is happy!

Contributor

thusoy commented Jan 26, 2017

@Habbie Just a heads up, Travis is happy!

@Habbie

This comment has been minimized.

Show comment
Hide comment
@Habbie

Habbie Jan 26, 2017

Member

@thusoy thanks for the ping, never hurts!

Member

Habbie commented Jan 26, 2017

@thusoy thanks for the ping, never hurts!

@Habbie Habbie merged commit 8a2ceab into PowerDNS:master Jan 26, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@thusoy thusoy deleted the thusoy:pgsql-extra-connection-args branch Jan 26, 2017

@porbas

This comment has been minimized.

Show comment
Hide comment
@porbas

porbas Jun 2, 2017

It was merged after latest (auth-4.0.3) release. When should I expect next release including this PR?

porbas commented Jun 2, 2017

It was merged after latest (auth-4.0.3) release. When should I expect next release including this PR?

@pieterlexis

This comment has been minimized.

Show comment
Hide comment
@pieterlexis

pieterlexis Jun 2, 2017

Member

@porbas this PR is not backported to 4.0.x, it exists only in master. If you need this in 4.0.x, can you open an issue about this?

Member

pieterlexis commented Jun 2, 2017

@porbas this PR is not backported to 4.0.x, it exists only in master. If you need this in 4.0.x, can you open an issue about this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment