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

Add TLS certificate validation for DB connection #1294

Closed
condemil opened this issue Feb 28, 2019 · 9 comments
Closed

Add TLS certificate validation for DB connection #1294

condemil opened this issue Feb 28, 2019 · 9 comments
Labels
upstream Issue is caused by an upstream dependency.
Milestone

Comments

@condemil
Copy link
Contributor

Is your feature request related to a problem? Please describe.
A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]

As discussed in the chat it is not possible to verify certificate for TLS database connection at that moment. It would be nice to add it.

Describe the solution you'd like
Add a way (probably though config yaml / environment variable) to specify path to PEM certificate file to validate TLS database connection.

Describe alternatives you've considered
For not the only possible to connect though TLS to database without certificate validation, it is workable, but not that secure.

Additional context
How it can be achieved with mysql driver is mentioned here: go-sql-driver/mysql#908

@aeneasr
Copy link
Member

aeneasr commented Feb 28, 2019

Ok so the idea would be to add this somewhere around herre. One difficulty is that the mysql driver doesn't handle this automatically so we would have to add a hardcoded check here against mysql to make this work which kinda sucks (the postgresdriver supports setting the CA via a flag). Any ideas how to solve this in a better way?

@condemil
Copy link
Contributor Author

The better way is to add another DNS argument (to be consistent with postgres), get it from query as you get other options in here and add it to SQLConnection struct. Later when we do mysql connection we can check that path to tls certificate is specified in SQLConnection.

@aeneasr
Copy link
Member

aeneasr commented Feb 28, 2019

Ah my description wasn't very clear, that's definitely the idea here, have something like ?tlsCAPath or whatever (we can mimic what postgres is doing).

@condemil
Copy link
Contributor Author

?tlsCAPath or some other parameter on DSN sound better than my initial idea with config / environment.

@condemil
Copy link
Contributor Author

The only other idea I have how to make it better without hardcoded check here against mysql is to file another feature request on mysql driver to support it tough DNS the way postgres driver does it.

@aeneasr
Copy link
Member

aeneasr commented Feb 28, 2019

The only other idea I have how to make it better without hardcoded check here against mysql is to file another feature request on mysql driver to support it tough DNS the way postgres driver does it.

I think that would be the most reasonable path as this should really be part of the driver spec IMO and it would benefit the community using the upstream dependency (mysql driver) the most.

@condemil
Copy link
Contributor Author

Created issue go-sql-driver/mysql#926

@aeneasr aeneasr added this to the unplanned milestone Feb 28, 2019
@aeneasr aeneasr added the upstream Issue is caused by an upstream dependency. label Feb 28, 2019
@aeneasr
Copy link
Member

aeneasr commented Feb 28, 2019

Awesome!

@aeneasr
Copy link
Member

aeneasr commented Dec 8, 2020

I will close this issue as it really is something which needs to be done upstream. Looks like there is some progress on the issue, but it's not yet merged. Once it's merged, it will be in the next update for ORY Hydra!

@aeneasr aeneasr closed this as completed Dec 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upstream Issue is caused by an upstream dependency.
Projects
None yet
Development

No branches or pull requests

2 participants