Codecov Report

❌ Patch coverage is 11.11111% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.78%. Comparing base (28d1562) to head (5654ffc).
⚠️ Report is 8 commits behind head on master.

@@            Coverage Diff             @@
##           master    #3809      +/-   ##
==========================================
- Coverage   89.83%   89.78%   -0.06%     
==========================================
  Files          86       86              
  Lines       13596    13605       +9     
  Branches     1606     1606              
==========================================
+ Hits        12214    12215       +1     
- Misses       1382     1390       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Thanks, @viladimiru! I didn't get to test it in practice, but just bringing the quote in question here (the same applies to MySQL 8 and 9):

For clients that use the sha256_password plugin, passwords are never exposed as cleartext when connecting to the server. How password transmission occurs depends on whether a secure connection or RSA encryption is used:

• If the connection is secure, an RSA key pair is unnecessary and is not used. This applies to connections encrypted using TLS. The password is sent as cleartext but cannot be snooped because the connection is secure.

• If the connection is not secure, and an RSA key pair is available, the connection remains unencrypted. This applies to connections not encrypted using TLS. RSA is used only for password exchange between client and server, to prevent password snooping. When the server receives the encrypted password, it decrypts it. A scramble is used in the encryption to prevent repeat attacks.

• If a secure connection is not used and RSA encryption is not available, the connection attempt fails because the password cannot be sent without being exposed as cleartext.

Also, regarding the missing tests, it is not actually related to this PR: this part of the logic was never covered.

I believe that instead of relying on the configuration provided by the user (if (connection.config.ssl) ...), we can ensure it through the connection itself, for example:

const tls = require('tls');

// ...

if (
  connection.stream instanceof tls.TLSSocket &&
  connection.stream.encrypted === true
) {
// ...

• The tests that passed in the bellow screenshot are based on this approach 🙋🏻‍♂️

Note

Testing locally by forcing sha256_password and SSL, I was able to see both the error mentioned in #2888 and the solution working:

I believe that instead of relying on the configuration provided by the user (if (connection.config.ssl) ...), we can ensure it through the connection itself, for example:

const tls = require('tls');

// ...

if (
  connection.stream instanceof tls.TLSSocket &&
  connection.stream.encrypted === true
) {
// ...

Hi @wellwelwel! Thank you for the suggestion!

Unfortunately, I'm not very knowledgeable about this particular issue, so if you think it's better this way, I will adjust the condition.

Unfortunately, I'm not very knowledgeable about this particular issue [...]

When using rejectUnauthorized, the connection.config.ssl will be truthy because it 'll be an object ({}), while the connection could be unencrypted. It is not an issue for MySQL2 or MySQL, but I consider it worth covering this possibility.

By checking the connection itself, we can confirm that it is actually secure before sending data as plain text.

Thanks again, @viladimiru! It's already available in mysql2@canary and soon in the latest version too 🤝