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

Parameter to configure database connection encryption #586

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Parameter to configure database connection encryption #586

wants to merge 1 commit into from

Conversation

isuldor
Copy link
Contributor

@isuldor isuldor commented Jan 21, 2020

--dbtls is a configurable parameter to set the go-sql-driver/mysql option for tls. [1]

1: https://github.com/go-sql-driver/mysql#tls

@JoeGruffins
Copy link
Member

I don't need to supply a certificate?

@isuldor
Copy link
Contributor Author

isuldor commented Jan 21, 2020

I'm glad you brought that up because I'm not sure, and frankly puzzled as to why go's mysql driver would require that with a valid certificate. On the one hand, a simple test script with http.Get("https://test.stakey.net/") will happily consume the letsencrypt cert. But running stakepoold with --dbtls=true --dbhost=test.stakey.net results in a [ERR] DB: Unable to establish connection to db: x509: certificate signed by unknown authority. This is while the standard mysql client connects with --ssl-verify-server-cert=on just fine. It's as if the mysql driver doesn't use the same root store as the http library.

Either way, adding a configurable pem file with mysql.RegisterTLSConfig for folks using self-signed certs makes sense. I also omitted it since I'm not exactly sure where and how it should be implemented.

@JoeGruffins
Copy link
Member

This looks good to me, but I would like to enable custom certificates using the code you linked. If you don't feel comfortable doing it, I can in another pr.

@jholdstock
Copy link
Member

This PR only adds support for stakepoold, but dcrstakepool also connects to the db. We should probably add support for both.

@dajohi
Copy link
Member

dajohi commented Mar 11, 2020

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.

4 participants