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

database/sql: mysql TLS support for Open #5737

Closed
lukescott opened this issue Jun 19, 2013 · 10 comments
Closed

database/sql: mysql TLS support for Open #5737

lukescott opened this issue Jun 19, 2013 · 10 comments
Assignees

Comments

@lukescott
Copy link

@lukescott lukescott commented Jun 19, 2013

I need to open a TLS connection with a MySQL database using a client certificate.
Unfortunately there doesn't seem to be a clean way to specify this. The only thing we
can think of is specifying a filename in the custom "dataSourceName" string
format. But this gets rather complicated when you want to customize things like the
RootCA. If you want to provide an embedded certificate, you really can't.

We really need something like this:

OpenMore(driverName string, settings map[string]interface{}) (*DB, error)

So you could do something like this:

OpenMore("mysql", Settings{..., "tls": &tls.Config{...}}
@lukescott
Copy link
Author

@lukescott lukescott commented Jun 20, 2013

Comment 1:

For the MySQL driver, I accomplished it like this:
db, err := sql.Open("mysql", "user@tcp(localhost:3306)/test?tls=custom")
    db.Driver().(mysql.TLSConfig).SetTLSConfig("custom", &tls.Config{
        RootCAs:      rootCAs,
        Certificates: clientCerts,
    })
(Pull request here: go-sql-driver/mysql#101
The solution is somewhat a hack. Issue #4804 has a discussion whether or not to open a
connection on the Open function. If it did, this patch wouldn't work. 
So there are several problems:
- "Open" shouldn't be called Open, if it will not actually Open any connections.
- "Open" is too restrictive. It feels very PHP like, not Go like - which btw, I moved
away from PHP for a reason.
There are also other assumptions made by database/sql. The prepared statements interface
would seem to suggest that only indexed parameters "?" are allowed. What about :named
parameters?
@robpike
Copy link
Contributor

@robpike robpike commented Jun 21, 2013

Comment 2:

Labels changed: added priority-later, removed priority-triage.

Owner changed to @bradfitz.

Status changed to Accepted.

@rsc
Copy link
Contributor

@rsc rsc commented Jun 21, 2013

Comment 3:

I disagree with the claim that Open is inflexible. It passes the string through to the
driver, and the driver can do whatever it likes. These names are always going to be
driver-specific.
The fix here lies with the mysql driver. Define a syntax for TLS.
@lukescott
Copy link
Author

@lukescott lukescott commented Jun 21, 2013

Comment 4:

IMO it's inflexible because you can only pass a string into the driver. If sql.Open had
opened a connection (which could have been the case, see issue #4804) the patch I
presented would have been impossible. The second argument should allow you to pass
anything required by the driver - either a map[string]interface{} or just an interface{}.
One way you could fix this is by changing the second argument to an interface{}. That
would allow a string to be passed as it is now, and allow certain drivers to accept
other things, like maps.
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jun 21, 2013

Comment 5:

If a driver wants a complex type, the driver can provide its own Open function.  That's
what I've done elsewhere in Google's Cloud SQL driver.
I don't want to encourage complexity. I want connection strings that people can put in
config files.

Status changed to WorkingAsIntended.

@lukescott
Copy link
Author

@lukescott lukescott commented Jun 21, 2013

Comment 6:

Brad, how can a driver do this without forgoing the connection poll management provided
by database/sql? Wouldn't the driver also have to expose itself? What you're suggesting
seems to be "dont use database/sql". Perhaps I'm misunderstanding your suggestion?
@lukescott
Copy link
Author

@lukescott lukescott commented Jun 21, 2013

Comment 7:

Brad, how can a driver do this without forgoing the connection pool management provided
by database/sql? Wouldn't the driver also have to expose itself? What you're suggesting
seems to be "dont use database/sql". Perhaps I'm misunderstanding your suggestion?
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jun 21, 2013

Comment 8:

Your driver can provide:
package yourdriver
func Open(host, username, password string, wantTLS bool) (*DB, error)
And internally, it can marshal host/user/pass/wantTLS into a URL-encoded string (or gob,
or proto, or whatever) and pass that to DB.Open, and then your driver will
URL-decode/whatever it.
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jun 21, 2013

Comment 9:

Rather,
func Open(host, username, password string, wantTLS bool) (*sql.DB, error) {
     return sql.Open("your_driver_name", encodeDSN(....))
}
@lukescott
Copy link
Author

@lukescott lukescott commented Jun 21, 2013

Comment 10:

I'm not sure I would want to marshal/unmarshal tls.Config to/from a string. Although I
suppose I could have a unique string that would have a key to a map that contained it...
Still, I feel that is more complicated than simply allowing the second argument to be an
interface{} :). Oh well.
@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.