Skip to content

add support for authentication plugins. #552

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

add support for authentication plugins. #552

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e273589
add support for authentication plugins.
craiggwilson Mar 17, 2017
f972a49
added support for closing an authPlugin for plugins whose resources n…
craiggwilson Mar 21, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions auth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ type AuthPlugin interface {
// Next takes a server's challenge and returns
// the bytes to send back or an error.
Next(challenge []byte) ([]byte, error)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this called Next and not e.g. Auth (which I believe is what this does)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is called next because it might be called more than once. All the current plugins, clear, native, and old all are single step. Next is called once and they are complete. However, not all plugins will be this way, so calling it Auth and having it called multiple times is weird (to me). Precedent was pulled from Java's driver, which calls it Next as well. However, I'm certainly not tied to that name.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. Maybe also add a short comment to the code that it might be called more than once.


// Close cleans up the resources of the plugin.
Close()
}

type clearTextPlugin struct {
Expand All @@ -51,6 +54,8 @@ func (p *clearTextPlugin) Next(challenge []byte) ([]byte, error) {
return append([]byte(p.cfg.Passwd), 0), nil
}

func (p *clearTextPlugin) Close() {}

type nativePasswordPlugin struct {
cfg *Config
}
Expand All @@ -64,6 +69,8 @@ func (p *nativePasswordPlugin) Next(challenge []byte) ([]byte, error) {
return scramblePassword(challenge, []byte(p.cfg.Passwd)), nil
}

func (p *nativePasswordPlugin) Close() {}

type oldPasswordPlugin struct {
cfg *Config
}
Expand All @@ -77,7 +84,9 @@ func (p *oldPasswordPlugin) Next(challenge []byte) ([]byte, error) {
return append(scrambleOldPassword(challenge, []byte(p.cfg.Passwd)), 0), nil
}

func handleAuthResult(mc *mysqlConn, plugin AuthPlugin, oldCipher []byte) error {
func (p *oldPasswordPlugin) Close() {}

func handleAuthResult(mc *mysqlConn, oldCipher []byte) error {
data, err := mc.readPacket()
if err != nil {
return err
Expand All @@ -91,11 +100,12 @@ func handleAuthResult(mc *mysqlConn, plugin AuthPlugin, oldCipher []byte) error
return mc.handleOkPacket(data)

case iEOF: // auth switch
mc.authPlugin.Close()
if len(data) > 1 {
pluginEndIndex := bytes.IndexByte(data, 0x00)
pluginName := string(data[1:pluginEndIndex])
if apf, ok := authPluginFactories[pluginName]; ok {
plugin = apf(mc.cfg)
mc.authPlugin = apf(mc.cfg)
} else {
return ErrUnknownPlugin
}
Expand All @@ -105,7 +115,7 @@ func handleAuthResult(mc *mysqlConn, plugin AuthPlugin, oldCipher []byte) error
}
} else {
// https://dev.mysql.com/doc/internals/en/connection-phase-packets.html#packet-Protocol::OldAuthSwitchRequest
plugin = authPluginFactories[mysqlOldPassword](mc.cfg)
mc.authPlugin = authPluginFactories[mysqlOldPassword](mc.cfg)
authData = oldCipher
}
case iAuthContinue:
Expand All @@ -115,7 +125,7 @@ func handleAuthResult(mc *mysqlConn, plugin AuthPlugin, oldCipher []byte) error
return mc.handleErrorPacket(data)
}

authData, err = plugin.Next(authData)
authData, err = mc.authPlugin.Next(authData)
if err != nil {
return err
}
Expand All @@ -125,5 +135,5 @@ func handleAuthResult(mc *mysqlConn, plugin AuthPlugin, oldCipher []byte) error
return err
}

return handleAuthResult(mc, plugin, authData)
return handleAuthResult(mc, authData)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like recursion. Please loop outside of this function.

}
5 changes: 5 additions & 0 deletions connection.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ type mysqlConn struct {
sequence uint8
parseTime bool
strict bool
authPlugin AuthPlugin
}

// Handles parameters set in DSN after the connection is established
Expand Down Expand Up @@ -92,6 +93,10 @@ func (mc *mysqlConn) Close() (err error) {
// closed the network connection.
func (mc *mysqlConn) cleanup() {
// Makes cleanup idempotent
if mc.authPlugin != nil {
mc.authPlugin.Close()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The auth plugin is not used again after the connection is established and can therefore be closed in the init phase already, I believe. We also don't have to add it as a field to mc then.

mc.authPlugin = nil
}
if mc.netConn != nil {
if err := mc.netConn.Close(); err != nil {
errLog.Print(err)
Expand Down
10 changes: 5 additions & 5 deletions driver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,18 +106,18 @@ func (d MySQLDriver) Open(dsn string) (driver.Conn, error) {
authPluginName = defaultAuthPluginName
}

var authPlugin AuthPlugin
if apf, ok := authPluginFactories[authPluginName]; ok {
authPlugin = apf(mc.cfg)
authData, err = authPlugin.Next(authData)
mc.authPlugin = apf(mc.cfg)
authData, err = mc.authPlugin.Next(authData)
if err != nil {
mc.cleanup()
return nil, err
}
} else {
// we'll tell the server in response that we are switching to our
// default plugin because we didn't recognize the one they sent us.
authPluginName = defaultAuthPluginName
authPlugin = authPluginFactories[authPluginName](mc.cfg)
mc.authPlugin = authPluginFactories[authPluginName](mc.cfg)

// zero-out the authData because the current authData was for
// a plugin we don't know about.
Expand All @@ -131,7 +131,7 @@ func (d MySQLDriver) Open(dsn string) (driver.Conn, error) {
}

// Handle response to auth packet, switch methods if possible
if err = handleAuthResult(mc, authPlugin, oldCipher); err != nil {
if err = handleAuthResult(mc, oldCipher); 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.
Expand Down