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

Simplified packetIO #14

Closed
2 tasks
xhebox opened this issue Jul 26, 2022 · 4 comments
Closed
2 tasks

Simplified packetIO #14

xhebox opened this issue Jul 26, 2022 · 4 comments

Comments

@xhebox
Copy link
Collaborator

xhebox commented Jul 26, 2022

@djshow832
Copy link
Collaborator

it is completely safe to remove most code of authenticator.go

No, there are 2 reasons to parse authentication packets:

  • You won't know whether to wait from the client or the server for the next packet if you don't parse it. For example, you need to parse the capability to know whether to upgrade to TLS.
  • You need to record the capability, attrs, and some more information to reconnect to the new server.

@xhebox
Copy link
Collaborator Author

xhebox commented Aug 3, 2022

You won't know whether to wait from the client or the server for the next packet if you don't parse it.

We don't need to know, just forward packages in both direction if there is one. (I mean connection phase).

In handshake:

  1. For client, parsing is ended once SSLRequest is identified.
  2. For server, parsing is ended once initialHandshake is identified. Reconnection needs to fake handshakeResp or SSLRequest(if needed).

For example, you need to parse the capability to know whether to upgrade to TLS.

TLS is always enabled. If client does not set TLS capability, reject. Anyway, I only need to verify SSLRequest.

For server, the initialHandshake is required to be parsed. But I only need to respond with SSLRequest and HandshakeResp(with session token).

You need to record the capability, attrs, and some more information to reconnect to the new server.

Correct. I only need to record but not process. SSLRequest and HandshakeResp is enough.

it is completely safe to remove most code of authenticator.go

By this word, I mean authenticator is not for auth anymore. It is more like a part of connection management. Some common parts among client, backend and authenticator made up connection management. And I think it can be extracted out.

@djshow832
Copy link
Collaborator

TLS is always enabled. If client does not set TLS capability, reject.

It's incompatible with the current situation. The client doesn't need to enable TLS when using mysql_native_password. The client won't query the session token anyway.

@xhebox
Copy link
Collaborator Author

xhebox commented Aug 3, 2022

It's incompatible with the current situation. The client doesn't need to enable TLS when using mysql_native_password. The client won't query the session token anyway.

There is a workaround. If it there is no TLS, connect the default backend and pass packets until handshake is finished or ErrPacket.

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

No branches or pull requests

2 participants