feat: Update TLS validation to use both SAN and CN fields. (#446)

This updates the logic used by the connector to validate server certificates.
When connecting to the instance, the connector's TLS validator will first check the SAN field,
and then if that fails check the CN field in the certificate for the instance name. This will enable
the connector to work smoothly with both legacy and newer instances.

To summarize the deviations from standard TLS hostname verification:

Historically, Cloud SQL creates server certificates with the instance name in the Subject.CN field in
the format "my-project:my-instance". The connector is expected to check that the instance name
that the connector was configured to dial matches the server certificate Subject.CN field. Thus,
the Subject.CN field for most Cloud SQL instances does not contain a well-formed DNS Name. This
breaks standard TLS hostname verification.

Also, there are times when the instance metadata reports that an instance has a DNS name, but
that DNS name does not yet appear in the SAN records of the server certificate. The client should
fall back to validating the hostname using the instance name in the Subject.CN field.

See also: GoogleCloudPlatform/cloud-sql-go-connector#979

Author: hessjcg

src/socket.ts

@@ -30,35 +30,92 @@ interface SocketOptions {
   serverName: string;
 }
 
+/**
+ * validateCertificate implements custom TLS verification logic to gracefully and securely
+ * handle deviations from standard TLS hostname verification in existing Cloud SQL instance server
+ * certificates.
+ *
+ *
+ * @param instanceInfo the instance connection name
+ * @param instanceDnsName the instance's DNS name according to instance metadata
+ * @param serverName the dns name from the connector configuration.
+ *
+ * This is run AFTER the NodeJS TLS library has validated the CA for the
+ * server certificate.
+ *
+ * <p>This is the verification algorithm:
+ *
+ * <ol>
+ *   <li>Verify the server cert CA, using the CA certs from the instance metadata. Reject the
+ *       certificate if the CA is invalid.
+ *   <li>Check that the server cert contains a SubjectAlternativeName matching the DNS name in the
+ *       connector configuration OR the DNS Name from the instance metadata
+ *   <li>If the SubjectAlternativeName does not match, and if the server cert Subject.CN field is
+ *       not empty, check that the Subject.CN field contains the instance name. Reject the
+ *       certificate if both the #2 SAN check and #3 CN checks fail.
+ * </ol>
+ *
+ * <p>To summarize the deviations from standard TLS hostname verification:
+ *
+ * <p>Historically, Cloud SQL creates server certificates with the instance name in the Subject.CN
+ * field in the format "my-project:my-instance". The connector is expected to check that the
+ * instance name that the connector was configured to dial matches the server certificate Subject.CN
+ * field. Thus, the Subject.CN field for most Cloud SQL instances does not contain a well-formed DNS
+ * Name. This breaks standard TLS hostname verification.
+ *
+ * <p>Also, there are times when the instance metadata reports that an instance has a DNS name, but
+ * that DNS name does not yet appear in the SAN records of the server certificate. The client should
+ * fall back to validating the hostname using the instance name in the Subject.CN field.
+ */
 export function validateCertificate(
   instanceInfo: InstanceConnectionInfo,
   instanceDnsName: string,
   serverName: string
 ) {
   return (hostname: string, cert: tls.PeerCertificate): Error | undefined => {
+    if (!cert) {
+      return new CloudSQLConnectorError({
+        message: 'Certificate missing',
+        code: 'ENOSQLADMINVERIFYCERT',
+      });
+    }
+
     if (!instanceDnsName) {
-      // Legacy CA Mode
-      if (!cert || !cert.subject) {
-        return new CloudSQLConnectorError({
-          message: 'No certificate to verify',
-          code: 'ENOSQLADMINVERIFYCERT',
-        });
-      }
-      const expectedCN = `${instanceInfo.projectId}:${instanceInfo.instanceId}`;
-      if (cert.subject.CN !== expectedCN) {
-        return new CloudSQLConnectorError({
-          message: `Certificate had CN ${cert.subject.CN}, expected ${expectedCN}`,
-          code: 'EBADSQLADMINVERIFYCERT',
-        });
-      }
-      return undefined;
+      return checkCn(instanceInfo, cert);
     } else {
-      // Standard TLS Verify Full hostname verification using SAN
-      return tls.checkServerIdentity(serverName, cert);
+      const err = tls.checkServerIdentity(serverName, cert);
+      if (err) {
+        const cnErr = checkCn(instanceInfo, cert);
+        if (cnErr) {
+          return err;
+        }
+      }
     }
+
+    return undefined;
   };
 }
 
+function checkCn(
+  instanceInfo: InstanceConnectionInfo,
+  cert: tls.PeerCertificate
+) {
+  const expectedCN = `${instanceInfo.projectId}:${instanceInfo.instanceId}`;
+  if (!cert.subject || !cert.subject.CN) {
+    return new CloudSQLConnectorError({
+      message: `Certificate missing CN, expected ${expectedCN}`,
+      code: 'ENOSQLADMINVERIFYCERT',
+    });
+  }
+  if (cert.subject.CN && cert.subject.CN !== expectedCN) {
+    return new CloudSQLConnectorError({
+      message: `Certificate had CN ${cert.subject.CN}, expected ${expectedCN}`,
+      code: 'EBADSQLADMINVERIFYCERT',
+    });
+  }
+  return undefined;
+}
+
 export function getSocket({
   ephemeralCert,
   host,
@@ -78,6 +135,8 @@ export function getSocket({
       key: privateKey,
       minVersion: 'TLSv1.3',
     }),
+    // This checks the provided serverName against the server certificate. It
+    // is called after the TLS CA chain of is validated.
     checkServerIdentity: validateCertificate(
       instanceInfo,
       instanceDnsName,

test/socket.ts

@@ -17,6 +17,7 @@ import t from 'tap';
 import {getSocket, validateCertificate} from '../src/socket';
 import {CA_CERT, CLIENT_CERT, CLIENT_KEY} from './fixtures/certs';
 import {setupTLSServer} from './fixtures/setup-tls-server';
+import {parseInstanceConnectionName} from '../src/parse-instance-connection-name';
 
 t.test('getSocket', async t => {
   // the mock tls server for this test will use a custom port in order
@@ -72,7 +73,9 @@ t.test('validateCertificate no cert', async t => {
       null,
       'abcde.12345.us-central1.sql.goog'
     )('hostname', {} as tls.PeerCertificate),
-    {code: 'ENOSQLADMINVERIFYCERT'},
+    {
+      code: 'ENOSQLADMINVERIFYCERT',
+    },
     'should return a missing cert to verify error'
   );
 });
@@ -94,8 +97,6 @@ t.test('validateCertificate mismatch', async t => {
       'abcde.12345.us-central1.sql.goog'
     )('hostname', cert),
     {
-      message:
-        'Certificate had CN other-project:other-instance, expected my-project:my-instance',
       code: 'EBADSQLADMINVERIFYCERT',
     },
     'should return a missing cert to verify error'
@@ -143,3 +144,110 @@ t.test('validateCertificate valid CAS CA', async t => {
     'DNS name matches SAN in cert'
   );
 });
+
+t.test('validateCertificate cases', async t => {
+  const tcs = [
+    {
+      desc: 'cn match',
+      icn: 'myProject:myRegion:myInstance',
+      cn: 'myProject:myInstance',
+      valid: true,
+    },
+    {
+      desc: 'cn no match',
+      icn: 'myProject:myRegion:badInstance',
+      cn: 'myProject:myInstance',
+      valid: false,
+    },
+    {
+      desc: 'cn empty',
+      icn: 'myProject:myRegion:myInstance',
+      san: 'db.example.com',
+      valid: false,
+    },
+    {
+      desc: 'san match',
+      serverName: 'db.example.com',
+      icn: 'myProject:myRegion:myInstance',
+      san: 'db.example.com',
+      valid: true,
+    },
+    {
+      desc: 'san no match',
+      serverName: 'bad.example.com',
+      icn: 'myProject:myRegion:myInstance',
+      san: 'db.example.com',
+      valid: false,
+    },
+    {
+      desc: 'san empty match',
+      serverName: 'empty.example.com',
+      icn: 'myProject:myRegion:myInstance',
+      cn: '',
+      valid: false,
+    },
+    {
+      desc: 'san match with cn present',
+      serverName: 'db.example.com',
+      icn: 'myProject:myRegion:myInstance',
+      san: 'db.example.com',
+      cn: 'myProject:myInstance',
+      valid: true,
+    },
+    {
+      desc: 'san no match fallback to cn',
+      serverName: 'db.example.com',
+      icn: 'myProject:myRegion:myInstance',
+      san: 'other.example.com',
+      cn: 'myProject:myInstance',
+      valid: true,
+    },
+    {
+      desc: 'san empty match fallback to cn',
+      serverName: 'db.example.com',
+      icn: 'myProject:myRegion:myInstance',
+      cn: 'myProject:myInstance',
+      valid: true,
+    },
+    {
+      desc: 'san no match fallback to cn and fail',
+      serverName: 'db.example.com',
+      icn: 'myProject:myRegion:badInstance',
+      san: 'other.example.com',
+      cn: 'myProject:myInstance',
+      valid: false,
+    },
+  ];
+
+  await Promise.all(
+    tcs.map(tc =>
+      t.test(tc.desc + ' cas', async t => {
+        runCertificateNameTest(t, tc);
+      })
+    )
+  );
+});
+
+function runCertificateNameTest(t, tc) {
+  const cert = {
+    subject: {CN: tc.cn},
+    subjectaltname: tc.san ? 'DNS:' + tc.san : undefined,
+  } as tls.PeerCertificate;
+
+  const instanceInfo = parseInstanceConnectionName(tc.icn);
+  instanceInfo.domainName = tc.san;
+
+  const err = validateCertificate(
+    instanceInfo,
+    tc.san,
+    tc.serverName
+  )(tc.serverName, cert);
+
+  if (tc.valid) {
+    t.match(err, undefined, 'want no error');
+  } else {
+    if (!err) {
+      t.fail('want error, got no error');
+    }
+  }
+}