Bug #57442 Pluggable auth - extra bytes sent to client with MYSQL_CLIENT_VIO::write_packet
Submitted: 13 Oct 2010 18:00 Modified: 17 Jan 2014 15:58
Reporter: Vladislav Vaintroub Email Updates:
Status: Won't fix Impact on me:
None 
Category:MySQL Server: Pluggable Authentication Severity:S3 (Non-critical)
Version: OS:Any
Assigned to: Georgi Kodinov

[13 Oct 2010 18:00] Vladislav Vaintroub
Description:
Found by  while implement Windows authentication in Connector/NET.

After getting handshake initialization from server, Connector/NET client sets CLIENT_PLUGIN_AUTH capability flag and writes the name of the plugin it wants to use (currently "auhentication_win_client").

Server  activates server side windows authentication plugin, which writes a single byte 0x00 using MYSQL_CLIENT_VIO::write_packet() (single 0x00 byte us sent in non-domain environments, in a domain environment server would send 
user principal name instead).

On the client side, the packet content is
0xfe"authentication_win_client"0x00 0x00
or ,in domain envirionment as 

0xfe"authentication_win_client"0x00<UPN>,

so what comes back is what one would expect in a "change plugin" packet, and original data server sent is appended at the end.

WL1054 describes the protocol differently:
1) no "change plugin" if the name specified by client is already correct. 
2) MYSQL_CLIENT_VIO::write_packet()'s description only mentions that it sends data wrapped as MySQL packet, and it is not supposed to send extras(from the description).

Note: in debugger, the place where extra data is added seems to be this place
in sql_acl.cc  (function server_mpvio_write_packet)

>  else if (mpvio->status == MPVIO_EXT::RESTART)
>    res= send_plugin_request_packet(mpvio, packet, packet_len);

How to repeat:
Contact me if you need the Connector/NET  client program for debugging purposes.
I had no luck to reproduce it with mysql.exe (--default-auth crashes for me)

I guess a dummy authentication plugin written such that server rather then client sends first packet via MYSQL_CLIENT_VIO::write_packet()  would suffice to reproduce it. Of course, client should also request this plugin by putting its name into after the database name in the client handshake reply packet.
[14 Oct 2010 12:55] Vladislav Vaintroub
I also see 0xfe'mysql_native_password' after Connector/NET sends CHANGE_USER packet to reset the connection (this is sometimes used in for the connection pooling) and authenticate afterwards. 

What is interesting in this specific case, is that no authentication plugin is involved, in the old and the new user is "root" and it authenitates using password, and there are no whatever plugin names sent after the database name.
[11 Nov 2010 11:24] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/123584

3119 Georgi Kodinov	2010-11-11
      Bug #57442: Pluggable auth - extra bytes sent to client with 
      MYSQL_CLIENT_VIO::write_packet
      
      Added a test for the mysql's --default-auth and --plugin-dir options.
[11 Nov 2010 12:37] Georgi Kodinov
Wlad,

I have debugged the sequence throughly today.
Here're my observations : 
- The test plugin that mysql_test/t/plugin_auth.test uses happens to be written so that the server is sending the first packet.
- It's been tested with mysql_test and specifying a client plugin.

- The server sends a COM_CHANGE_USER whenever any of the following don't match : 
  * the default server plugin (typically mysql_native_password)
  * the default client plugin (typically mysql_native_password, but configurable through --default-auth)
  * the plugin the user being authenticated is defined to use.
Yes, it seems a bit silly to always go through the COM_CHANGE_USER, specially if it's only the server default plugin that doesn't match (as is the case described above). It's tempting to say that the server can just flip the plugin and redo the authentication using the data already received.  But this is not a very wise design decision, as it limits the functionality that the plugins can have. It's always safer to start afresh the conversation when there's a new participant in it instead of depending on the record of the conversation with the previous parties. This assumption can lead to hard to fix errors.
This is the reason you're getting the 0xfe"authentication_win_client"0x00 every time when you connect.

While looking at the test suite I've found that we have no test of mysql's binary --default-auth parameter. So I've added one with the commit above.
[11 Nov 2010 13:50] Vladislav Vaintroub
Joro, thanks for debugging and for reply.
I still feel like what you describe is not documented properly in the WL, i.e I have no idea how to correctly program a plugin reading a plugin description.

What I see now, how the communication works:

->Server sends handshake initialization packet
->Client replies with handshake reply, sets CLIENT_PLUGIN_AUTH bit, and the name of the plugin after the database name

So far so good.

Now, server detects server-side plugin, activates it, server-side plugin writes 
something e.g for simplicity single byte 0x00 via MYSQL_PLUGIN_VIO::write.

What would one expect on the client side of plugin *byte-wise*. I "would" expect a packet that contains 0x00, because this is how it is documented.
However packet contains 0xfe'plugin_name'0x00 0x00 (only the last 0x00 is the usable data). So far so good , this is workaround-able  but undocumented, I can strip the junk 0xfe'plugin_name'x00 bytes.

Now, what actually happens if client send COM_CHANGE_USER. I use the old format COM_CHANGE_USER (even no charset there), because I know it will be the same  authentication method, (it will even be the very same user, due to pool implementation but this is not important). We client sends "old-style" COM_CHANGE_USER. It is not deprecated according to the WL, or is it?

And what should I expect here in the packet (byte-wise), and what should be the client action. So far (with <= 5.1 ) we just replay the authentication procedure, and I bet it should be the same with 5.5.  It dies with 5.5, I'm seeing 0xfe'plugin_name', which I do not expect. Shall I try to strip the junk 0xfe'plugin_name'0x0 and handle the rest of the packet as if I never seen it? 

If it is so and it is by design rather than bug, then I think the protocol changes are not adequately documented yet.  

But the fact that during plugin talk (server plugin sends data to client plugin via MYSQL_PLUGIN_VIO::write) , client can get first packet prefixed by 0xfe'plugin_name'  is a real bug. I do workaround it, but this is just because I want to get it work "somehow".
[11 Nov 2010 15:43] Horst Hunger
Changeset alright. Ready to push.
[11 Nov 2010 15:50] Vladislav Vaintroub
I reset the status to open because changeset does not fix the described bug, so it does not get closed after joro pushes it.

It might be ok to push though nevertheless:)
[15 Dec 2010 12:31] Georgi Kodinov
Wlad,

concatenating packets like this is a feature. If we blindly observe the each-call-to-write-that-the-plugin-makes-is-a-packet rule then the server would have to send two packets in sequence : the change plugin packet and the first packet that the new plugin wrote. 
Instead it appends the first plugin packet to the change user packet.
[13 Jan 2011 16:31] Vladislav Vaintroub
Where is that feature described?
[13 Jan 2011 16:57] Vladislav Vaintroub
If we blindly observe the each-call-to-write-that-the-plugin-makes-is-a-packet.
you dsid not have to do that. writing packets was your idea, and if it is described as "packets", it should be packets. You could simply choose another strategy where server plugin comminicates with client using byte streams.

Right now, it is sort of "in-the-middle" stuff, you send size-prefixed blobs, where one cannot tell where one packet ends and another one starts.
[3 Feb 2011 12:28] Georgi Kodinov
No plans to fix this in immediate future.
[17 Jan 2014 15:58] Georgi Kodinov
Re-reading this conversation today I think the only remaining "issue" is the fact that a flush is not done after the change user request.
Obviously the Connector/Net angle is long gone (http://dev.mysql.com/doc/refman/5.6/en/connector-net-versions.html says C/NET 6.4 supports windows native authentication).
Thus I'm closing this as won't fix.