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

Doesn't use database from DSN if tls=true #884

Closed
jpeirson opened this issue Oct 26, 2018 · 20 comments
Closed

Doesn't use database from DSN if tls=true #884

jpeirson opened this issue Oct 26, 2018 · 20 comments
Labels
Milestone

Comments

@jpeirson
Copy link

jpeirson commented Oct 26, 2018

Issue description

Using tls=true in the DSN causes the db name in the DSN to be ignored when queries are executed.

Example code

package main

import (
	"database/sql"
	"fmt"
	"os"

	_ "github.com/go-sql-driver/mysql" // import mysql driver
)

func main() {
	dsn := os.Args[1] // e.g. "test:mypass@tcp(myserver)/mydb?tls=true"
	db, err := sql.Open("mysql", dsn)
	if err != nil {
		panic(err)
	}

	var cipher, dbname, ignore sql.NullString
	db.QueryRow("SHOW SESSION STATUS LIKE 'Ssl_cipher';").Scan(&ignore, &cipher)
	err = db.QueryRow("SELECT DATABASE();").Scan(&dbname)
	if err != nil {
		panic(err) // no error occurs
	}

	fmt.Printf("Ssl_cipher = %s\n", cipher.String)
	fmt.Printf("DATABASE() = %s\n", dbname.String)
}

Edit: I modified the example to verify that no error occurs, and fixed to use NullString to handle null return value.

I've added an all-access test user to my MySQL server using:

create user 'test'@'%' identified with 'sha256_password' by 'mypass';
grant all privileges on *.* to 'test'@'%';
flush privileges;

With TLS turned off, this produces, as expected:

myapp "test:mypass@tcp(myserver)/mydb?tls=false"

Ssl_cipher = 
DATABASE() = mydb

With TLS turned on, this produces, which is unexpected:

myapp "test:mypass@tcp(myserver)/mydb?tls=true"

Ssl_cipher = ECDHE-RSA-AES128-GCM-SHA256
DATABASE() =

In the latter case, I'd expect DATABASE() = mydb. I can manually run a USE test; statement from my sample program, but it defeats the purpose of specifying the db name in the DSN, and in a real app, I'd have to run that prior to every statement to ensure newly created connections also use this db name.

Error log

No errors logged, that I could see.

Configuration

Driver version (or git SHA):
7daee5b

Go version:
go version go1.11.1 linux/amd64

Server version:
5.7.21-21-log MySQL Community Server (GPL)

Server OS:
Red Hat Enterprise Linux Server release 6.8 (Santiago)

@jpeirson
Copy link
Author

This sounds similar to issue #825 but the commit in which this is fixed produces the same problem in my scenario.

@jpeirson
Copy link
Author

Note that I can also reproduce this in v1.4.0 (d523deb).

@methane
Copy link
Member

methane commented Oct 30, 2018

You didn't check error returned by db.QueryRow()

It says: x509: cannot validate certificate for 127.0.0.1 because it doesn't contain any IP SANs.

You need to specify tls=skip-verify, not tls=true.
https://github.com/go-sql-driver/mysql#tls

@methane methane closed this as completed Oct 30, 2018
@jpeirson
Copy link
Author

The error check from QueryRow() was only excluded from my example. I've updated the example accordingly.

I can verify in my case that the certificate is valid and signed by a trusted CA, and I don't receive the x509 validation error in @methane 's case. Also, this can't be the case from my example since the connection is being made and I can successfully query and show output for SHOW SESSION STATUS.

@jpeirson
Copy link
Author

Also, just as a check, using tls=skip-verify in my case, where the server-side TLS cert is signed using a trusted CA and matches the FQDN, I get the same behavior as if I use tls=true, where no database is selected.

@methane
Copy link
Member

methane commented Oct 30, 2018

I can't reproduce. I can get right database name.
Have you customized MySQL auth settings?
Especially, what is your default auth plugin?

https://dev.mysql.com/doc/refman/5.7/en/server-system-variables.html#sysvar_default_authentication_plugin

@methane methane reopened this Oct 30, 2018
@jpeirson
Copy link
Author

The default auth plugin is set to sha256_password, and I can verify that the user account I'm testing against also uses the default plugin (via select user,host,plugin from mysql.user;).

@methane
Copy link
Member

methane commented Oct 30, 2018

It's very important information!
Can you try native_password for default auth plugin setting?

@jpeirson
Copy link
Author

jpeirson commented Oct 30, 2018

If I change the default auth plugin for the server to mysql_native_password, my sample test app works, which tests against a user identified with sha256_password. FWIW, it also works against a different user identified with mysql_native_password.

With the server's default set to sha256_password then I can reproduce the issue for a user with either plugin type.

Before I ask our db security team whether we can permanently change the default plugin auth to native, is there something in the go-sql-driver/mysql package that can be fixed to address this?

@methane
Copy link
Member

methane commented Oct 30, 2018

Before I ask our db security team whether we can permanently change the default plugin auth to native, is there something in the go-sql-driver/mysql package that can be fixed to address this?

Yes. But it may take very long time to fix.
Auth is complicated and our review speed is slow.

@julienschmidt julienschmidt added this to the v1.4.1 milestone Oct 31, 2018
@fiorix
Copy link

fiorix commented Nov 11, 2018

Not sure if related to this, but since d523deb and head something broke TLS authentication. I use client certs, and using the latest commit of this driver causes:

 Error 1045: Access denied for user '<user>'@'<host>' (using password: NO)

Any clues? This is blocking to upgrade to latest github.com/go-sql-driver/mysql.

@methane
Copy link
Member

methane commented Nov 11, 2018

@fiorix Try #887.

@fiorix
Copy link

fiorix commented Nov 11, 2018

Didn't work. I downloaded from https://patch-diff.githubusercontent.com/raw/go-sql-driver/mysql/pull/887.patch and applied with patch -p1 on HEAD. It applied cleanly, I rebuilt my thing and got the same error.

@methane
Copy link
Member

methane commented Nov 12, 2018

@fiorix I used several hours to test client cert (I had not used it yet. So I need to use several hours to set up)

Then, I can connect to MySQL with/without password.
Code is here: https://gist.github.com/methane/42d05355ce4599093b00b701918b5d72

Could you provide reproducible example demonstrates your issue?
If you can not, please don't steal maintainer's time by posting random "I can't XXX" comment.

This is issue tracker, not user forum. We're maintainer, not free tech support.

@fiorix
Copy link

fiorix commented Nov 21, 2018

I cannot provide any easy way to repro this as of now. Our use case is a bit more complex.

@methane
Copy link
Member

methane commented Nov 21, 2018

Regardless how your application is complicated, authentication error must be simple. It is not relating to your app.

Without information to reproduce, "it doesn't work for me" is just a spam to maintainer.

@fiorix
Copy link

fiorix commented Nov 22, 2018

Hey sorry, spamming wasn't my intention. Let me give you a bit more context about why I think there's an issue with recent changes:

Our internal codebase is pinned to d523deb. We have our own "fbmysql" driver which figures out which instances to connect to (master or slaves), it then hits our CA server to issue a x509 cert for the specific client and server, then it configures go-sql-driver/mysql to use those TLS settings, and generate the DSN to connect.

When we upgrade past d523deb, we get that Access Denied for any connection. I haven't had the time yet to nail it down to which change actually introduced the problem, and like I said it's not easy to provide an easy standalone version of the code to reproduce.

@methane
Copy link
Member

methane commented Nov 22, 2018

We have our own "fbmysql" driver which figures out which instances to connect to (master or slaves), it then hits our CA server to issue a x509 cert for the specific client and server, then it configures go-sql-driver/mysql to use those TLS settings, and generate the DSN to connect.

This special flow is totally not ralating to this driver.
Driver just use x509 cert to connect the server.

it's not easy to provide an easy standalone version of the code to reproduce.

You know:

  • How cert is created
  • Your server's version
  • And configuration
  • How DB user is created
  • Actual DSL, and TLSConfig

We don't know nothing about these. If it's "not easy" to you, it's "impossible" to us.

@fiorix
Copy link

fiorix commented Nov 22, 2018

We don't know nothing about these. If it's "not easy" to you, it's "impossible" to us.

Maybe I wasn't clear enough. I wanted to clarify why I didn't provide more information, wasn't asking for anything. We have yet to figure out what's up with recent changes in this repo.

@methane
Copy link
Member

methane commented Nov 23, 2018

I wanted to clarify why I didn't provide more information, wasn't asking for anything.

Read the ISSUE_TEMPLATE.md!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants