diff --git a/core/src/main/java/com/google/cloud/sql/core/DefaultConnectionInfoRepository.java b/core/src/main/java/com/google/cloud/sql/core/DefaultConnectionInfoRepository.java index 82f88ff17..8b93a8bed 100644 --- a/core/src/main/java/com/google/cloud/sql/core/DefaultConnectionInfoRepository.java +++ b/core/src/main/java/com/google/cloud/sql/core/DefaultConnectionInfoRepository.java @@ -463,6 +463,9 @@ private SslData createSslData( KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm()); kmf.init(authKeyStore, new char[0]); + // The InstanceCheckingTrustManagerFactory implements the custom certificate validation + // logic. After using the standard TLS CA chain of trust, it will implement a custom + // hostname verification to gracefully handle the hostnames in Cloud SQL server certificates. TrustManagerFactory tmf = InstanceCheckingTrustManagerFactory.newInstance(instanceMetadata); SSLContext sslContext; diff --git a/core/src/main/java/com/google/cloud/sql/core/InstanceCheckingTrustManagerFactory.java b/core/src/main/java/com/google/cloud/sql/core/InstanceCheckingTrustManagerFactory.java index 4c4f9e0b8..734690848 100644 --- a/core/src/main/java/com/google/cloud/sql/core/InstanceCheckingTrustManagerFactory.java +++ b/core/src/main/java/com/google/cloud/sql/core/InstanceCheckingTrustManagerFactory.java @@ -40,19 +40,11 @@ *

class ConscryptWorkaroundTrustManager - the workaround for the Conscrypt bug. * *

class InstanceCheckingTrustManager - delegates TLS checks to the default provider and then - * does custom hostname checking in accordance with these rules: - * - *

If the instance supports CAS certificates (instanceMetadata.casEnabled == true), or the - * connection is being made to a PSC endpoint (instanceMetadata.pscEnabled == true) the connector - * should validate that the server certificate subjectAlternativeNames contains an entry that - * matches instanceMetadata.dnsName. - * - *

Otherwise, the connector should check that the Subject CN field contains the Cloud SQL - * instance ID in the form: "project-name:instance-name" + * does custom hostname verification. */ class InstanceCheckingTrustManagerFactory extends TrustManagerFactory { - static TrustManagerFactory newInstance(InstanceMetadata instanceMetadata) + static InstanceCheckingTrustManagerFactory newInstance(InstanceMetadata instanceMetadata) throws NoSuchAlgorithmException, KeyStoreException, CertificateException, IOException { TrustManagerFactory delegate = TrustManagerFactory.getInstance("X.509"); diff --git a/core/src/main/java/com/google/cloud/sql/core/InstanceCheckingTrustManger.java b/core/src/main/java/com/google/cloud/sql/core/InstanceCheckingTrustManger.java index e39dba995..5c0ac6377 100644 --- a/core/src/main/java/com/google/cloud/sql/core/InstanceCheckingTrustManger.java +++ b/core/src/main/java/com/google/cloud/sql/core/InstanceCheckingTrustManger.java @@ -30,14 +30,33 @@ import javax.net.ssl.X509ExtendedTrustManager; /** - * This is a workaround for a known bug in Conscrypt crypto in how it handles X509 auth type. - * OpenJDK interpres the X509 certificate auth type as "UNKNOWN" while Conscrypt interpret the same - * certificate as auth type "GENERIC". This incompatibility causes problems in the JDK. + * InstanceCheckingTrustManger implements custom TLS verification logic to gracefully and securely + * handle deviations from standard TLS hostname verification in existing Cloud SQL instance server + * certificates. * - *

This adapter works around the issue by creating wrappers around all TrustManager instances. It - * replaces "GENERIC" auth type with "UNKNOWN" auth type before delegating calls. + *

This is the verification algorithm: * - *

See https://github.com/google/conscrypt/issues/1033#issuecomment-982701272 + *

    + *
  1. Verify the server cert CA, using the CA certs from the instance metadata. Reject the + * certificate if the CA is invalid. This is delegated to the default JSSE TLS provider. + *
  2. Check that the server cert contains a SubjectAlternativeName matching the DNS name in the + * connector configuration OR the DNS Name from the instance metadata + *
  3. 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. + *
+ * + *

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. */ class InstanceCheckingTrustManger extends X509ExtendedTrustManager { private final X509ExtendedTrustManager tm; @@ -96,27 +115,31 @@ private void checkCertificateChain(X509Certificate[] chain) throws CertificateEx throw new CertificateException("Subject is missing"); } - // If the instance metadata does not contain a domain name, use legacy CN validation - if (Strings.isNullOrEmpty(instanceMetadata.getDnsName())) { - checkCn(chain); - } else { - // If there is a DNS name, check the Subject Alternative Names. - checkSan(chain); - } - } - - private void checkSan(X509Certificate[] chain) throws CertificateException { final String dns; if (!Strings.isNullOrEmpty(instanceMetadata.getInstanceName().getDomainName())) { // If the connector is configured using a DNS name, validate the DNS name from the connector // config. dns = instanceMetadata.getInstanceName().getDomainName(); - } else { + } else if (!Strings.isNullOrEmpty(instanceMetadata.getDnsName())) { // If the connector is configured with an instance name, validate the DNS name from // the instance metadata. dns = instanceMetadata.getDnsName(); + } else { + dns = null; } + // If the instance metadata does not contain a domain name, and the connector was not + // configured with a domain name, use legacy CN validation. + if (dns == null) { + checkCn(chain); + } else { + // If there is a DNS name, check the Subject Alternative Names. + checkSan(dns, chain); + } + } + + private void checkSan(String dns, X509Certificate[] chain) throws CertificateException { + if (Strings.isNullOrEmpty(dns)) { throw new CertificateException( "Instance metadata for " + instanceMetadata.getInstanceName() + " has an empty dnsName"); @@ -128,11 +151,15 @@ private void checkSan(X509Certificate[] chain) throws CertificateException { return; } } - throw new CertificateException( - "Server certificate does not contain expected name '" - + dns - + "' for Cloud SQL instance " - + instanceMetadata.getInstanceName()); + try { + checkCn(chain); + } catch (CertificateException e) { + throw new CertificateException( + "Server certificate does not contain expected name '" + + dns + + "' for Cloud SQL instance " + + instanceMetadata.getInstanceName()); + } } private List getSans(X509Certificate cert) throws CertificateException { diff --git a/core/src/test/java/com/google/cloud/sql/core/ConnectorTest.java b/core/src/test/java/com/google/cloud/sql/core/ConnectorTest.java index b41ae27d5..c43c5eadf 100644 --- a/core/src/test/java/com/google/cloud/sql/core/ConnectorTest.java +++ b/core/src/test/java/com/google/cloud/sql/core/ConnectorTest.java @@ -452,7 +452,7 @@ public void create_throwsErrorForDomainNameDoesntMatchServerCert() throws Except config.getConnectorConfig(), port, "db.example.com", - "myProject:myRegion:myInstance", + "myProject:myRegion:otherInstance", true); SSLHandshakeException ex = @@ -461,6 +461,28 @@ public void create_throwsErrorForDomainNameDoesntMatchServerCert() throws Except assertThat(ex).hasMessageThat().contains("Server certificate does not contain expected name"); } + @Test + public void create_fallbackToInstanceWhenDomainNameDoesntMatchServerCert() throws Exception { + FakeSslServer sslServer = new FakeSslServer(); + ConnectionConfig config = + new ConnectionConfig.Builder() + .withDomainName("not-in-san.example.com") + .withIpTypes("PRIMARY") + .build(); + + int port = sslServer.start(PUBLIC_IP); + + Connector c = + newConnector( + config.getConnectorConfig(), + port, + "db.example.com", + "myProject:myRegion:myInstance", + true); + + c.connect(config, TEST_MAX_REFRESH_MS); + } + @Test public void create_successfulPublicCasConnection() throws IOException, InterruptedException { PrivateKey privateKey = TestKeys.getServerKeyPair().getPrivate(); diff --git a/core/src/test/java/com/google/cloud/sql/core/InstanceCheckingTrustManagerFactoryTest.java b/core/src/test/java/com/google/cloud/sql/core/InstanceCheckingTrustManagerFactoryTest.java new file mode 100644 index 000000000..ffea33f7e --- /dev/null +++ b/core/src/test/java/com/google/cloud/sql/core/InstanceCheckingTrustManagerFactoryTest.java @@ -0,0 +1,210 @@ +/* + * Copyright 2025 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.cloud.sql.core; + +import static org.junit.Assert.assertThrows; + +import java.security.cert.Certificate; +import java.security.cert.CertificateException; +import java.security.cert.X509Certificate; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; + +@RunWith(Parameterized.class) +public class InstanceCheckingTrustManagerFactoryTest { + + private static TestCertificateGenerator generator; + private final TestCase tc; + + @BeforeClass + public static void beforeClass() { + generator = new TestCertificateGenerator(); + } + + public InstanceCheckingTrustManagerFactoryTest(TestCase tc) { + this.tc = tc; + } + + @Test + public void testValidateCertificate() throws Exception { + + List caCerts; + X509Certificate[] serverCert; + if (tc.cas) { + caCerts = Arrays.asList(generator.getCasServerCertificateChain()); + caCerts = caCerts.subList(1, caCerts.size()); + serverCert = generator.createServerCertificate(tc.cn, tc.san, true); + } else { + caCerts = Collections.singletonList(generator.getServerCaCert()); + serverCert = generator.createServerCertificate(tc.cn, tc.san, false); + } + + InstanceMetadata instanceMetadata = + new InstanceMetadata( + new CloudSqlInstanceName(tc.icn, tc.serverName), + Collections.emptyMap(), + caCerts, + false, + null, + false); + + InstanceCheckingTrustManagerFactory f = + InstanceCheckingTrustManagerFactory.newInstance(instanceMetadata); + InstanceCheckingTrustManger tm = (InstanceCheckingTrustManger) f.getTrustManagers()[0]; + + if (tc.valid) { + tm.checkServerTrusted(serverCert, "UNKNOWN"); + } else { + assertThrows( + CertificateException.class, + () -> { + tm.checkServerTrusted(serverCert, "UNKNOWN"); + }); + } + } + + @Parameters(name = "{index}: {0}") + public static List testCases() { + List cases = + Arrays.asList( + new TestCase( + "cn match", + null, + "myProject:myRegion:myInstance", + "myProject:myInstance", + null, + true), + new TestCase( + "cn no match", + null, + "myProject:myRegion:badInstance", + "myProject:myInstance", + null, + false), + new TestCase( + "cn empty", null, "myProject:myRegion:myInstance", "db.example.com", null, false), + new TestCase( + "san match", + "db.example.com", + "myProject:myRegion:myInstance", + null, + "db.example.com", + true), + new TestCase( + "san no match", + "bad.example.com", + "myProject:myRegion:myInstance", + null, + "db.example.com", + false), + new TestCase( + "san empty match", + "empty.example.com", + "myProject:myRegion:myInstance", + "", + null, + false), + new TestCase( + "san match with cn present", + "db.example.com", + "myProject:myRegion:myInstance", + "myProject:myInstance", + "db.example.com", + true), + new TestCase( + "san no match fallback to cn", + "db.example.com", + "myProject:myRegion:myInstance", + "myProject:myInstance", + "other.example.com", + true), + new TestCase( + "san empty match fallback to cn", + "db.example.com", + "myProject:myRegion:myInstance", + "myProject:myInstance", + null, + true), + new TestCase( + "san no match fallback to cn and fail", + "db.example.com", + "myProject:myRegion:badInstance", + "other.example.com", + "myProject:myInstance", + false)); + List casesWithCas = new ArrayList<>(cases); + for (TestCase tc : cases) { + casesWithCas.add(tc.withCas(true)); + } + return casesWithCas; + } + + private static class TestCase { + /** Testcase description. */ + private final String desc; + /** connector configuration domain name. */ + private final String serverName; + /** connector configuration instance name. */ + private final String icn; + /** server cert CN field value. */ + private final String cn; + /** server cert SAN field value. */ + private final String san; + /** wants validation to succeed. */ + private final boolean valid; + + private final boolean cas; + + public TestCase( + String desc, String serverName, String icn, String cn, String san, boolean valid) { + this(desc, serverName, icn, cn, san, valid, false); + } + + public TestCase( + String desc, + String serverName, + String icn, + String cn, + String san, + boolean valid, + boolean cas) { + this.desc = desc; + this.serverName = serverName; + this.icn = icn; + this.cn = cn; + this.san = san; + this.valid = valid; + this.cas = cas; + } + + @Override + public String toString() { + return desc; + } + + private TestCase withCas(boolean cas) { + return new TestCase(this.desc, this.serverName, this.icn, this.cn, this.san, this.valid, cas); + } + } +} diff --git a/core/src/test/java/com/google/cloud/sql/core/TestCertificateGenerator.java b/core/src/test/java/com/google/cloud/sql/core/TestCertificateGenerator.java index 00c7c0d52..706f4030b 100644 --- a/core/src/test/java/com/google/cloud/sql/core/TestCertificateGenerator.java +++ b/core/src/test/java/com/google/cloud/sql/core/TestCertificateGenerator.java @@ -253,6 +253,40 @@ public X509Certificate createEphemeralCert(String cn, Duration shiftIntoPast) return new JcaX509CertificateConverter().getCertificate(certificateHolder); } + public X509Certificate[] createServerCertificate(String cn, String san, boolean cas) + throws GeneralSecurityException, OperatorCreationException, CertIOException { + Duration validFor = Duration.ofHours(1); + ZonedDateTime notBefore = ZonedDateTime.now(ZoneId.of("UTC")); + ZonedDateTime notAfter = notBefore.plus(validFor); + + Collection sans = + san != null + ? Collections.singletonList(new GeneralName(GeneralName.dNSName, san)) + : Collections.emptyList(); + + if (cas) { + X509Certificate cert = + buildSignedCertificate( + new X500Name("CN=" + cn), + serverKeyPair.getPublic(), + SERVER_INTERMEDIATE_CA_SUBJECT, + serverIntermediateCaKeyPair.getPrivate(), + notAfter.toInstant(), + sans); + return new X509Certificate[] {cert, this.serverIntemediateCaCert, this.serverCaCert}; + } else { + X509Certificate cert = + buildSignedCertificate( + new X500Name("CN=" + cn), + serverKeyPair.getPublic(), + SERVER_CA_SUBJECT, + serverCaKeyPair.getPrivate(), + notAfter.toInstant(), + sans); + return new X509Certificate[] {cert, this.serverIntemediateCaCert, this.serverCaCert}; + } + } + /** Returns the PEM encoded certificate. */ public String getPemForCert(X509Certificate certificate) { StringBuilder sb = new StringBuilder();