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

Default to SSL with hardcoded AWS Redshift CA #20

Merged
merged 1 commit into from Aug 22, 2015

Conversation

Projects
None yet
3 participants
@graingert
Collaborator

graingert commented Aug 22, 2015

You can still override this by setting {'sslmode': 'disable'}

In fact I'd recommend using {'sslmode': 'verify-full', 'sslrootcert': '/path/to/redshift-ssl-ca-cert.pem'}

@graingert

This comment has been minimized.

Show comment
Hide comment
@graingert

graingert Aug 22, 2015

Collaborator

@jklukas @thisfred @bouk: Thoughts?

Collaborator

graingert commented Aug 22, 2015

@jklukas @thisfred @bouk: Thoughts?

@thisfred

This comment has been minimized.

Show comment
Hide comment
@thisfred

thisfred Aug 22, 2015

lgtm, and I think it makes sense to use SSL by default.

thisfred commented Aug 22, 2015

lgtm, and I think it makes sense to use SSL by default.

graingert added a commit that referenced this pull request Aug 22, 2015

Merge pull request #20 from graingert/enable-tls-by-default
Default to SSL with hardcoded AWS Redshift CA

@graingert graingert merged commit a7afb1e into master Aug 22, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@@ -249,6 +250,20 @@ def _get_column_info(self, *args, **kwargs):
return column_info
def create_connect_args(self, *args, **kwargs):
default_args = {
'sslmode': 'verify-full',

This comment has been minimized.

@jklukas

jklukas Aug 24, 2015

Collaborator

From http://docs.aws.amazon.com/redshift/latest/mgmt/connecting-ssl-support.html:

Amazon Redshift does not support verify-full. For more information about sslmode options, see SSL Support in the PostgreSQL documentation.

So, if this is working, it's probably not doing what we think it's doing.

@jklukas

jklukas Aug 24, 2015

Collaborator

From http://docs.aws.amazon.com/redshift/latest/mgmt/connecting-ssl-support.html:

Amazon Redshift does not support verify-full. For more information about sslmode options, see SSL Support in the PostgreSQL documentation.

So, if this is working, it's probably not doing what we think it's doing.

This comment has been minimized.

@graingert

graingert Aug 24, 2015

Collaborator

I've been running with verify-full in production for several months now

@graingert

graingert Aug 24, 2015

Collaborator

I've been running with verify-full in production for several months now

This comment has been minimized.

@graingert

graingert Aug 24, 2015

Collaborator

I think using "sslrootcert" is making this work for us

@graingert

graingert Aug 24, 2015

Collaborator

I think using "sslrootcert" is making this work for us

This comment has been minimized.

@graingert

graingert Aug 24, 2015

Collaborator

I'll mess about with wireshark on this at home.

@graingert

graingert Aug 24, 2015

Collaborator

I'll mess about with wireshark on this at home.

This comment has been minimized.

@graingert

graingert Aug 24, 2015

Collaborator

It looks like the redshift cluster is sending a valid ServerCertificate (when validated with redshift-ssl-ca-cert.pem) with the correct common name: "redshift-sqlalchemy-test.cforsfjmjsja.us-west-2.redshift.amazonaws.com" see https://gist.github.com/graingert/3a46c493520db7caa460#file-redshift-tls-server-hello-txt-L188

So there is no reason that verify-ssl should not be working as designed.

@graingert

graingert Aug 24, 2015

Collaborator

It looks like the redshift cluster is sending a valid ServerCertificate (when validated with redshift-ssl-ca-cert.pem) with the correct common name: "redshift-sqlalchemy-test.cforsfjmjsja.us-west-2.redshift.amazonaws.com" see https://gist.github.com/graingert/3a46c493520db7caa460#file-redshift-tls-server-hello-txt-L188

So there is no reason that verify-ssl should not be working as designed.

This comment has been minimized.

@graingert

graingert Aug 24, 2015

Collaborator

I'll be sticking with "verify-full" unless someone reports issues with it, because the documentation on that page that's not wrong is dangerous.

Under the configuration here: https://docs.aws.amazon.com/redshift/latest/mgmt/connecting-ssl-support.html clients will accept certificates minted by Amazon for any Internet server, and anyone with any valid certificate will be able to MITM connections to the redshift server.

If you're using "verify-ca" with your system ca-store your connection can be easily compromised with any free x509 certificate.

@graingert

graingert Aug 24, 2015

Collaborator

I'll be sticking with "verify-full" unless someone reports issues with it, because the documentation on that page that's not wrong is dangerous.

Under the configuration here: https://docs.aws.amazon.com/redshift/latest/mgmt/connecting-ssl-support.html clients will accept certificates minted by Amazon for any Internet server, and anyone with any valid certificate will be able to MITM connections to the redshift server.

If you're using "verify-ca" with your system ca-store your connection can be easily compromised with any free x509 certificate.

@graingert graingert deleted the enable-tls-by-default branch Aug 24, 2015

haleemur pushed a commit to haleemur/redshift_sqlalchemy that referenced this pull request Sep 2, 2015

Merge pull request sqlalchemy-redshift#20 from graingert/enable-tls-b…
…y-default

Default to SSL with hardcoded AWS Redshift CA
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment