-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Support authentication switch request and basic auth plugin. #1776
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
module.exports = AuthSwitchPacket; | ||
function AuthSwitchPacket(options) { | ||
options = options || {}; | ||
this.scrambleBuff = options.scrambleBuff; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this is greenfield, I'm just changing these props to have the same name as in the protocol docs for sanity. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure. |
||
} | ||
|
||
AuthSwitchPacket.prototype.parse = function(parser) { | ||
this.scrambleBuff = parser.parsePacketTerminatedBuffer(); | ||
}; | ||
|
||
AuthSwitchPacket.prototype.write = function(writer) { | ||
writer.writeBuffer(this.scrambleBuff); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,19 +2,26 @@ module.exports = ComChangeUserPacket; | |
function ComChangeUserPacket(options) { | ||
options = options || {}; | ||
|
||
this.command = 0x11; | ||
this.user = options.user; | ||
this.scrambleBuff = options.scrambleBuff; | ||
this.database = options.database; | ||
this.charsetNumber = options.charsetNumber; | ||
this.command = 0x11; | ||
this.user = options.user; | ||
this.scrambleBuff = options.scrambleBuff; | ||
this.database = options.database; | ||
this.charsetNumber = options.charsetNumber; | ||
this.clientPluginAuth = options.clientPluginAuth; | ||
this.authPluginName = options.authPluginName; | ||
} | ||
|
||
ComChangeUserPacket.prototype.parse = function(parser) { | ||
this.command = parser.parseUnsignedNumber(1); | ||
this.user = parser.parseNullTerminatedString(); | ||
this.scrambleBuff = parser.parseLengthCodedBuffer(); | ||
this.database = parser.parseNullTerminatedString(); | ||
this.charsetNumber = parser.parseUnsignedNumber(1); | ||
if (!parser.reachedPacketEnd()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this key off the fact that plugin auth is used, not if packet end was reached? For example, this will not work with CLIENT_CONNECT_ATTRS because if client auth is off but CLIENT_CONNECT_ATTRS is on, then the packet end is indeed not reached, but the rest of the packet is not related to client plugin auth. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure. Let me change the parse. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why this check here? Shouldn't the charsetNumber just be outside the if like it used to be, or is there a reason behind this change? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @dougwilson , this is for the document COM_CHANGE_USER packet, it include the charset in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, sorry :) I was thinking the handshake. Nevermind on this, then :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm writing a test to exercise this case, there the charsetNumber may not get populated. |
||
this.charsetNumber = parser.parseUnsignedNumber(2); | ||
if (this.clientPluginAuth === true) { | ||
this.authPluginName = parser.parseNullTerminatedString(); | ||
} | ||
} | ||
}; | ||
|
||
ComChangeUserPacket.prototype.write = function(writer) { | ||
|
@@ -23,4 +30,7 @@ ComChangeUserPacket.prototype.write = function(writer) { | |
writer.writeLengthCodedBuffer(this.scrambleBuff); | ||
writer.writeNullTerminatedString(this.database); | ||
writer.writeUnsignedNumber(2, this.charsetNumber); | ||
if (this.clientPluginAuth) { | ||
writer.writeNullTerminatedString(this.authPluginName); | ||
} | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,11 +20,17 @@ function HandshakeInitializationPacket(options) { | |
this.filler3 = options.filler3; | ||
this.pluginData = options.pluginData; | ||
this.protocol41 = options.protocol41; | ||
this.clientPluginAuth = options.clientPluginAuth; | ||
this.authPluginName = options.authPluginName; | ||
|
||
if (this.protocol41) { | ||
// force set the bit in serverCapabilities1 | ||
this.serverCapabilities1 |= Client.CLIENT_PROTOCOL_41; | ||
} | ||
|
||
if (this.clientPluginAuth) { | ||
this.serverCapabilities2 |= Client.CLIENT_PLUGIN_AUTH >> 16; | ||
} | ||
} | ||
|
||
HandshakeInitializationPacket.prototype.parse = function(parser) { | ||
|
@@ -38,16 +44,22 @@ HandshakeInitializationPacket.prototype.parse = function(parser) { | |
this.serverStatus = parser.parseUnsignedNumber(2); | ||
|
||
this.protocol41 = (this.serverCapabilities1 & (1 << 9)) > 0; | ||
this.clientPluginAuth = false; | ||
|
||
if (this.protocol41) { | ||
this.serverCapabilities2 = parser.parseUnsignedNumber(2); | ||
this.clientPluginAuth = (this.serverCapabilities2 & (1 << 3)) > 0; | ||
this.scrambleLength = parser.parseUnsignedNumber(1); | ||
this.filler2 = parser.parseFiller(10); | ||
// scrambleBuff2 should be 0x00 terminated, but sphinx does not do this | ||
// so we assume scrambleBuff2 to be 12 byte and treat the next byte as a | ||
// filler byte. | ||
this.scrambleBuff2 = parser.parseBuffer(12); | ||
scrambleBuff2Length = Math.max(12, this.scrambleLength - 9); | ||
this.scrambleBuff2 = parser.parseBuffer(scrambleBuff2Length); | ||
this.filler3 = parser.parseFiller(1); | ||
if (this.clientPluginAuth) { | ||
this.authPluginName = parser.parseNullTerminatedString(); | ||
} | ||
} else { | ||
this.filler2 = parser.parseFiller(13); | ||
} | ||
|
@@ -80,8 +92,11 @@ HandshakeInitializationPacket.prototype.write = function(writer) { | |
writer.writeUnsignedNumber(2, this.serverCapabilities2); | ||
writer.writeUnsignedNumber(1, this.scrambleLength); | ||
writer.writeFiller(10); | ||
writer.writeNullTerminatedBuffer(this.scrambleBuff2); | ||
if (this.clientPluginAuth) { | ||
writer.writeNullTerminatedString(this.authPluginName); | ||
} | ||
} | ||
writer.writeNullTerminatedBuffer(this.scrambleBuff2); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was a miss in the first, and since it's fixed in here, I'm trying to write a test case for this bug. |
||
|
||
if (this.pluginData !== undefined) { | ||
writer.writeNullTerminatedString(this.pluginData); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
module.exports = UseAuthSwitchPacket; | ||
function UseAuthSwitchPacket(options) { | ||
options = options || {}; | ||
this.firstByte = options.firstByte || 0xfe; | ||
this.authPluginName = options.authPluginName || ''; | ||
this.authPluginData = options.authPluginData || ''; | ||
} | ||
|
||
UseAuthSwitchPacket.prototype.parse = function(parser) { | ||
this.firstByte = parser.parseUnsignedNumber(1); | ||
this.authPluginName = parser.parseNullTerminatedString(); | ||
this.authPluginData = parser.parsePacketTerminatedBuffer(); | ||
}; | ||
|
||
UseAuthSwitchPacket.prototype.write = function(writer) { | ||
writer.writeUnsignedNumber(1, this.firstByte); | ||
writer.writeNullTerminatedString(this.authPluginName); | ||
writer.writeBuffer(this.authPluginData); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,22 +8,25 @@ Util.inherits(ChangeUser, Sequence); | |
function ChangeUser(options, callback) { | ||
Sequence.call(this, options, callback); | ||
|
||
this._user = options.user; | ||
this._password = options.password; | ||
this._database = options.database; | ||
this._charsetNumber = options.charsetNumber; | ||
this._currentConfig = options.currentConfig; | ||
this._user = options.user; | ||
this._password = options.password; | ||
this._database = options.database; | ||
this._charsetNumber = options.charsetNumber; | ||
this._currentConfig = options.currentConfig; | ||
this._authPluginName = options.authPluginName; | ||
} | ||
|
||
ChangeUser.prototype.start = function(handshakeInitializationPacket) { | ||
var scrambleBuff = handshakeInitializationPacket.scrambleBuff(); | ||
scrambleBuff = Auth.token(this._password, scrambleBuff); | ||
|
||
var packet = new Packets.ComChangeUserPacket({ | ||
user : this._user, | ||
scrambleBuff : scrambleBuff, | ||
database : this._database, | ||
charsetNumber : this._charsetNumber | ||
user : this._user, | ||
scrambleBuff : scrambleBuff, | ||
database : this._database, | ||
charsetNumber : this._charsetNumber, | ||
clientPluginAuth : this._currentConfig.clientPluginAuth, | ||
authPluginName : this._authPluginName || this._currentConfig.authPluginName | ||
}); | ||
|
||
this._currentConfig.user = this._user; | ||
|
@@ -34,8 +37,31 @@ ChangeUser.prototype.start = function(handshakeInitializationPacket) { | |
this.emit('packet', packet); | ||
}; | ||
|
||
ChangeUser.prototype.determinePacket = function(firstByte) { | ||
if (firstByte === 0xff) { | ||
return Packets.ErrorPacket; | ||
} | ||
|
||
if (firstByte === 0xfe) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am fixing up this edge case where the server can actually respond here with the old password packet, of 0xfe doesn't always mean auth switch as implemented here, so just gotta add the same logic from the handshake. |
||
return Packets.UseAuthSwitchPacket; | ||
} | ||
|
||
return Packets.OkPacket; | ||
}; | ||
|
||
ChangeUser.prototype['ErrorPacket'] = function(packet) { | ||
var err = this._packetToError(packet); | ||
err.fatal = true; | ||
this.end(err); | ||
}; | ||
|
||
ChangeUser.prototype['UseAuthSwitchPacket'] = function(packet) { | ||
try { | ||
var scrambleBuff = Auth.tokenByPlugin(packet.authPluginName, packet.authPluginData, this._password); | ||
this.emit('packet', new Packets.AuthSwitchPacket({ | ||
scrambleBuff: scrambleBuff | ||
})); | ||
} catch (err) { | ||
this.end(err); | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to rework how this is tracked so it stops changing the hidden class of the config object.