diff --git a/AUTHORS b/AUTHORS index 6dd0167f3..0985a70ca 100644 --- a/AUTHORS +++ b/AUTHORS @@ -15,6 +15,7 @@ Aaron Hopkins Arne Hormann Carlos Nieto Chris Moos +Daniel Nichter DisposaBoy Frederick Mayle Gustavo Kristic @@ -25,6 +26,7 @@ INADA Naoki James Harr Jian Zhen Joshua Prunier +Julien Lefevre Julien Schmidt Kamil Dziedzic Leonardo YongUk Kim @@ -37,7 +39,6 @@ Soroush Pour Stan Putrya Xiaobing Jiang Xiuming Chen -Julien Lefevre # Organizations diff --git a/connection.go b/connection.go index 72ed09d69..455fcc1d9 100644 --- a/connection.go +++ b/connection.go @@ -120,18 +120,27 @@ func (mc *mysqlConn) Close() (err error) { // Makes Close idempotent if mc.netConn != nil { err = mc.writeCommandPacket(comQuit) - if err == nil { - err = mc.netConn.Close() - } else { - mc.netConn.Close() + } + + mc.cleanup() + + return +} + +// Closes the network connection and unsets internal variables. Do not call this +// function after successfully authentication, call Close instead. This function +// is called before auth or on auth failure because MySQL will have already +// closed the network connection. +func (mc *mysqlConn) cleanup() { + // Makes cleanup idempotent + if mc.netConn != nil { + if err := mc.netConn.Close(); err != nil { + errLog.Print(err) } mc.netConn = nil } - mc.cfg = nil mc.buf.rd = nil - - return } func (mc *mysqlConn) Prepare(query string) (driver.Stmt, error) { diff --git a/driver.go b/driver.go index d310624ad..7502c57b4 100644 --- a/driver.go +++ b/driver.go @@ -84,43 +84,23 @@ func (d MySQLDriver) Open(dsn string) (driver.Conn, error) { // Reading Handshake Initialization Packet cipher, err := mc.readInitPacket() if err != nil { - mc.Close() + mc.cleanup() return nil, err } // Send Client Authentication Packet if err = mc.writeAuthPacket(cipher); err != nil { - mc.Close() + mc.cleanup() return nil, err } - // Read Result Packet - err = mc.readResultOK() - if err != nil { - // Retry with old authentication method, if allowed - if mc.cfg != nil && mc.cfg.allowOldPasswords && err == ErrOldPassword { - if err = mc.writeOldAuthPacket(cipher); err != nil { - mc.Close() - return nil, err - } - if err = mc.readResultOK(); err != nil { - mc.Close() - return nil, err - } - } else if mc.cfg != nil && mc.cfg.allowCleartextPasswords && err == ErrCleartextPassword { - if err = mc.writeClearAuthPacket(); err != nil { - mc.Close() - return nil, err - } - if err = mc.readResultOK(); err != nil { - mc.Close() - return nil, err - } - } else { - mc.Close() - return nil, err - } - + // Handle response to auth packet, switch methods if possible + if err = handleAuthResult(mc, cipher); err != nil { + // Authentication failed and MySQL has already closed the connection + // (https://dev.mysql.com/doc/internals/en/authentication-fails.html). + // Do not send COM_QUIT, just cleanup and return the error. + mc.cleanup() + return nil, err } // Get max allowed packet size @@ -144,6 +124,38 @@ func (d MySQLDriver) Open(dsn string) (driver.Conn, error) { return mc, nil } +func handleAuthResult(mc *mysqlConn, cipher []byte) error { + // Read Result Packet + err := mc.readResultOK() + if err == nil { + return nil // auth successful + } + + if mc.cfg == nil { + return err // auth failed and retry not possible + } + + // Retry auth if configured to do so. + if mc.cfg.allowOldPasswords && err == ErrOldPassword { + // Retry with old authentication method. Note: there are edge cases + // where this should work but doesn't; this is currently "wontfix": + // https://github.com/go-sql-driver/mysql/issues/184 + if err = mc.writeOldAuthPacket(cipher); err != nil { + return err + } + err = mc.readResultOK() + } else if mc.cfg.allowCleartextPasswords && err == ErrCleartextPassword { + // Retry with clear text password for + // http://dev.mysql.com/doc/refman/5.7/en/cleartext-authentication-plugin.html + // http://dev.mysql.com/doc/refman/5.7/en/pam-authentication-plugin.html + if err = mc.writeClearAuthPacket(); err != nil { + return err + } + err = mc.readResultOK() + } + return err +} + func init() { sql.Register("mysql", &MySQLDriver{}) } diff --git a/driver_test.go b/driver_test.go index f9da416ec..aae9f9fa8 100644 --- a/driver_test.go +++ b/driver_test.go @@ -9,12 +9,14 @@ package mysql import ( + "bytes" "crypto/tls" "database/sql" "database/sql/driver" "fmt" "io" "io/ioutil" + "log" "net" "net/url" "os" @@ -1679,3 +1681,50 @@ func TestInsertRetrieveEscapedData(t *testing.T) { runTests(t, testdsn, testData) } } + +func TestUnixSocketAuthFail(t *testing.T) { + runTests(t, dsn, func(dbt *DBTest) { + // Save the current logger so we can restore it. + oldLogger := errLog + + // Set a new logger so we can capture its output. + buffer := bytes.NewBuffer(make([]byte, 0, 64)) + newLogger := log.New(buffer, "prefix: ", 0) + SetLogger(newLogger) + + // Restore the logger. + defer SetLogger(oldLogger) + + // Make a new DSN that uses the MySQL socket file and a bad password, which + // we can make by simply appending any character to the real password. + badPass := pass + "x" + socket := "" + if prot == "unix" { + socket = addr + } else { + // Get socket file from MySQL. + err := dbt.db.QueryRow("SELECT @@socket").Scan(&socket) + if err != nil { + t.Fatalf("Error on SELECT @@socket: %s", err.Error()) + } + } + t.Logf("socket: %s", socket) + badDSN := fmt.Sprintf("%s:%s@unix(%s)/%s?timeout=30s&strict=true", user, badPass, socket, dbname) + db, err := sql.Open("mysql", badDSN) + if err != nil { + t.Fatalf("Error connecting: %s", err.Error()) + } + defer db.Close() + + // Connect to MySQL for real. This will cause an auth failure. + err = db.Ping() + if err == nil { + t.Error("expected Ping() to return an error") + } + + // The driver should not log anything. + if actual := buffer.String(); actual != "" { + t.Errorf("expected no output, got %q", actual) + } + }) +}