From 83c6b8ce0a51a42af3e675141418948233262358 Mon Sep 17 00:00:00 2001
From: Dirkjan Bussink <d.bussink@gmail.com>
Date: Fri, 14 Jan 2022 10:08:37 +0100
Subject: [PATCH] Add identify verification option for TLS

Right now the hostname presented on a TLS certificate is never
validated. This means that man-in-the-middle attacks are trivial when
using for example public CAs to sign certificates.

The issue is that anyone can have a public CA sign a certificate for a
hostname they own, which is key to the security model there. But with
the current functionality in the `mysql2` driver, the hostname is never
verified.

This means an attacker could do a man in the middle attack and only has
to present a signed certificate but doesn't have to match the hostname.

It does not only apply to public CAs in system roots, but also for
example Amazon RDS where a single root chain is also used.

The changes here add an option to explicitly verify the identity on the
certificate. This option is available in other MySQL drivers as well,
such as `libmysqlclient` and everything based on that (often drivers for
Ruby, Python etc.). Also other languages like Go, Java, .Net etc.
provide an option to have strict identity verification.

Ideally of course, this validation would be the default and always
enforced, but that also would likely be a significant breaking change,
so right now in the code here the setting is opt-in for having a secure
connection to MySQL.

Additionally, it adds SNI on the connection which is a best practice as
well.
---
 lib/connection.js                 | 16 +++++++++++++++-
 typings/mysql/lib/Connection.d.ts |  6 ++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/lib/connection.js b/lib/connection.js
index e2720be078..c2b73248b1 100644
--- a/lib/connection.js
+++ b/lib/connection.js
@@ -342,6 +342,9 @@ class Connection extends EventEmitter {
       minVersion: this.config.ssl.minVersion
     });
     const rejectUnauthorized = this.config.ssl.rejectUnauthorized;
+    const verifyIdentity = this.config.ssl.verifyIdentity;
+    const host = this.config.host;
+
     let secureEstablished = false;
     const secureSocket = new Tls.TLSSocket(this.stream, {
       rejectUnauthorized: rejectUnauthorized,
@@ -349,6 +352,9 @@ class Connection extends EventEmitter {
       secureContext: secureContext,
       isServer: false
     });
+    if (typeof host === 'string') {
+      secureSocket.setServername(host);
+    }
     // error handler for secure socket
     secureSocket.on('_tlsError', err => {
       if (secureEstablished) {
@@ -359,7 +365,15 @@ class Connection extends EventEmitter {
     });
     secureSocket.on('secure', () => {
       secureEstablished = true;
-      onSecure(rejectUnauthorized ? secureSocket.ssl.verifyError() : null);
+      let callbackValue = null;
+      if (rejectUnauthorized) {
+        callbackValue = secureSocket.ssl.verifyError()
+        if (!callbackValue && typeof host === 'string' && verifyIdentity) {
+          const cert = secureSocket.ssl.getPeerCertificate(true);
+          callbackValue = Tls.checkServerIdentity(host, cert)
+        }
+      }
+      onSecure(callbackValue);
     });
     secureSocket.on('data', data => {
       this.packetParser.execute(data);
diff --git a/typings/mysql/lib/Connection.d.ts b/typings/mysql/lib/Connection.d.ts
index dbce3e67a4..4882c113b3 100644
--- a/typings/mysql/lib/Connection.d.ts
+++ b/typings/mysql/lib/Connection.d.ts
@@ -226,6 +226,12 @@ declare namespace Connection {
          * Configure the minimum supported version of SSL, the default is TLSv1.2.
          */
         minVersion?: string;
+
+        /**
+         * You can verify the server name identity presented on the server certificate when connecting to a MySQL server.
+         * You should enable this but it is disabled by default right now for backwards compatibility.
+         */
+         verifyIdentity?: boolean;
     }
 }