-
Notifications
You must be signed in to change notification settings - Fork 511
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 client and server cert auth in postgresql #1160
Conversation
(For both client and server) Status: Server cert validation is successful, client (notary signer and server) fails. Signed-off-by: Ashwini Oruganti <[email protected]>
Signed-off-by: Ashwini Oruganti <[email protected]>
Signed-off-by: Ashwini Oruganti <[email protected]>
Signed-off-by: Ashwini Oruganti <[email protected]>
Signed-off-by: Ashwini Oruganti <[email protected]>
Some thoughts:
TODO:
|
Could we add generating the certs to |
Looks like tests are failing because we have one more place we specify the DB_URL for db-specific unit tests :| https://github.com/docker/notary/blob/master/buildscripts/dbtests.sh#L19 |
Ah, thanks! |
Signed-off-by: Ashwini Oruganti <[email protected]>
Signed-off-by: Ashwini Oruganti <[email protected]>
eaf300e
to
deb6db3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for tackling this! Looks like the postgres tests on circle are running into a connection error:
time="2017-05-15T20:26:12Z" level=fatal msg="Unable to connect to postgres://server@postgresql_tests:5432/notaryserver?sslmode=verify-ca&sslrootcert=/go/src/github.com/docker/notary/fixtures/database/ca.pem&sslcert=/go/src/github.com/docker/notary/fixtures/database/notary-server.pem&sslkey=/go/src/github.com/docker/notary/fixtures/database/notary-server-key.pem after 60 seconds"
# started with TLS enabled. | ||
set -e | ||
|
||
sed -i "s/#ssl = off/ssl = on/" "$PGDATA"/postgresql.conf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for my own understanding: where does $PGDATA
come from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is set in the Dockerfile for the image: https://github.com/docker-library/postgres/blob/master/9.5/Dockerfile#L60
@riyazdf Thanks for reviewing! It appears that the DBURL per #1160 (comment) doesn't lead to a successful connection, I am trying to figure out what might be different. (Since this script/setup is specifically for our CI, I'm unable to debug much locally. Any thoughts/insights?) |
Signed-off-by: Ashwini Oruganti <[email protected]>
Signed-off-by: Ashwini Oruganti <[email protected]>
90d25ff
to
fb1735b
Compare
jenkins is back up now, I kicked off a new test run |
jenkins, test this please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM on yubikey green
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one clarifying question but LGTM on green!
@@ -40,6 +40,7 @@ services: | |||
- mdb | |||
volumes: | |||
- ./notarysql/postgresql-initdb.d:/docker-entrypoint-initdb.d | |||
command: -l |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for my own understanding: where does this come from? I didn't find this option on the official image description...tried searching around but couldn't find the flag elsewhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per https://www.postgresql.org/docs/9.5/static/app-postgres.html, -l
"Enables secure connections using SSL.".
FWIW, I think ssl=on
is implicit after passing -l
, but since I was editing the conf for ssl_ca_file
path, I thought it would be clearer to set ssl=on
explicitly, in case anything changes silently in the future versions. (e.g. Not sure what happens if we use -l
and ssl=off
-- which one takes a higher priority is never mentioned. :( In fact, the docs that mention ssl=on
never refer to -l
, and vice versa.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome that helps a lot, thank you @ashfall!
./buildscripts/integrationtest.sh postgresql
should run cleanly after this change.