Security concerns with existing password implementation for the Server

[gdsmith]

I was looking to implement a version of the CredentialProvider for a MySQL proxy. I was trying to understand how to store the password so I could return what was expected. Looking at how the password is used during authentication it is required that the server gets the password in plain text, therefore it is required for the CredentialProvider to be able to provide the password in plain text.

I believe this is a big security issue in the code that can be easily avoided if you correctly follow the MySQL implementations.

As an example, if you look at how this is implemented for the mysql_native_password in MySQL proper you will see that they rather use the salt/seed to encrypt the hash_stage1 and then generate the hash_stage2 from that for comparison with what is stored on the server.

  SERVER:  public_seed=generate_user_salt()
           send(public_seed)

  CLIENT:  recv(public_seed)
           hash_stage1=sha1("password")
           hash_stage2=sha1(hash_stage1)
           reply=xor(hash_stage1, sha1(public_seed,hash_stage2)

           // this three steps are done in scramble()

           send(reply)


  SERVER:  recv(reply)
           hash_stage1=xor(reply, sha1(public_seed,hash_stage2))
           candidate_hash2=sha1(hash_stage1)
           check(candidate_hash2==hash_stage2)

The reason for doing this is so the actual password does not need to be stored anywhere on the server, you just need to store the hash_stage2 of the password.

[lance6716]

Thanks for report this problem. Currently I don't have enough knowledge to discuss about it. It seems your suggestion is correct and improves the security. I'll take a look some days later. Can you provide more details like how we should change the code when I'm learning?

[gdsmith]

I would suggest looking at the implementation in vitess for an example

[dveeden]

@lance6716 I can have a look at this over the next few weeks

[gdsmith]

Happy to make a stab at this, but looking for some input on some questions:

1. When looking at the implementation, it seems like the library currently does on the fly password negotiation (because you have access to the underlying password in the store). If we are looking at updating the store to rather keep the hashes, we can either:
   a. have a single store that uses a single auth type requiring changes to the auth to explicitly offer only the methods offered by the store
   b. store any kind of supported auth plugin hashes in the standard way mysql does and negotiate with the client to ensure they are using the correct method
2. The hashing methods for storage are different to the comparison functions so we either need to:
   a. import code (from e.g. vitess)
   b. copy code from same (with comments on the source I assume)
   c. rewrite the code
   d. fork one of the libraries and import that as a new standard lib for mysql server auth

Rewriting is simple enough for mysql_native_password but gets complicated quickly for the other two plugins

My preference would be 1b and 2a if that's relevant.

[dveeden]

This might also be helpful:

• https://github.com/pingcap/tidb/blob/master/pkg/parser/auth/caching_sha2.go
• https://github.com/pingcap/tidb/blob/master/pkg/parser/auth/mysql_native_password.go

I think staying close to the MySQL behavior where possible would be good. So 1b seems like a good choice to me.

With importing and/or copying code etc: make sure that licenses are compatible and the right attribution is given.

As github.com/pingcap/tidb/pkg/parser is already in go.mod it would be my preference to use that if possible. If that doesn't work then adding a dependency for vitess might be the second best choice.

Note that in MySQL mysql_native_password has been deprecated and removed from the server. It is only available as a client plugin for the latest versions. I do think it makes sense to add support in go-mysql for this.

For caching_sha2_password either TLS or RSA keys should be used to secure the communication.