From 0fb7c7e5fd194fa6fe36d362419a085c6ba0fc8a Mon Sep 17 00:00:00 2001 From: sagnghos Date: Fri, 18 Sep 2026 11:11:18 +0000 Subject: [PATCH 01/22] feat(spanner): Support Dynamic Certificate/Key rotation in Spanner Omni --- .../google/cloud/spanner/SpannerOptions.java | 51 +++- .../spanner/connection/ConnectionOptions.java | 24 ++ .../connection/ConnectionProperties.java | 9 + .../cloud/spanner/connection/SpannerPool.java | 7 + .../cloud/spanner/omni/DynamicKeyManager.java | 251 ++++++++++++++++++ .../spanner/omni/DynamicTrustManager.java | 246 +++++++++++++++++ .../spanner/testing/SpannerOmniHelper.java | 9 + .../cloud/spanner/SpannerOptionsTest.java | 50 ++++ .../connection/ConnectionOptionsTest.java | 27 ++ .../spanner/connection/SpannerPoolTest.java | 45 ++++ .../spanner/omni/DynamicKeyManagerTest.java | 136 ++++++++++ .../spanner/omni/DynamicTrustManagerTest.java | 138 ++++++++++ 12 files changed, 980 insertions(+), 13 deletions(-) create mode 100644 java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java create mode 100644 java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java create mode 100644 java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java create mode 100644 java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicTrustManagerTest.java diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index ca1f9abec4f0..ee7b193e2513 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -53,6 +53,8 @@ import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStubSettings; import com.google.cloud.spanner.admin.instance.v1.InstanceAdminSettings; import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStubSettings; +import com.google.cloud.spanner.omni.DynamicKeyManager; +import com.google.cloud.spanner.omni.DynamicTrustManager; import com.google.cloud.spanner.omni.SpannerOmniCredentials; import com.google.cloud.spanner.spi.SpannerRpcFactory; import com.google.cloud.spanner.spi.v1.ChannelEndpointCacheFactory; @@ -85,6 +87,7 @@ import io.grpc.netty.shaded.io.grpc.netty.GrpcSslContexts; import io.grpc.netty.shaded.io.grpc.netty.NettyChannelBuilder; import io.grpc.netty.shaded.io.netty.handler.ssl.SslContext; +import io.grpc.netty.shaded.io.netty.handler.ssl.SslContextBuilder; import io.opencensus.trace.Tracing; import io.opentelemetry.api.GlobalOpenTelemetry; import io.opentelemetry.api.OpenTelemetry; @@ -941,14 +944,14 @@ protected SpannerOptions(Builder builder) { transportChannelExecutorThreadNameFormat = builder.transportChannelExecutorThreadNameFormat; channelProvider = builder.channelProvider; channelEndpointCacheFactory = builder.channelEndpointCacheFactory; - if (builder.mTLSContext != null) { + if (builder.omniSslContext != null) { channelConfigurator = channelBuilder -> { if (builder.channelConfigurator != null) { channelBuilder = builder.channelConfigurator.apply(channelBuilder); } if (channelBuilder instanceof NettyChannelBuilder) { - ((NettyChannelBuilder) channelBuilder).sslContext(builder.mTLSContext); + ((NettyChannelBuilder) channelBuilder).sslContext(builder.omniSslContext); } return channelBuilder; }; @@ -1292,6 +1295,13 @@ public GoogleCredentials getDefaultSpannerOmniCredentials() { public static class Builder extends ServiceOptions.Builder { private static Builder prepareBuilder(Builder builder) { + if (builder.sslContextBuilder != null) { + try { + builder.omniSslContext = builder.sslContextBuilder.build(); + } catch (Exception e) { + throw SpannerExceptionFactory.asSpannerException(e); + } + } if (builder.instanceType == InstanceType.OMNI) { builder.enableBuiltInMetrics = false; builder.setProjectId(SPANNER_OMNI_PROJECT_ID); @@ -1314,7 +1324,7 @@ private static Builder prepareBuilder(Builder builder) { } if (builder.credentials instanceof SpannerOmniCredentials) { ((SpannerOmniCredentials) builder.credentials) - .initChannel(builder.usePlainText, builder.mTLSContext); + .initChannel(builder.usePlainText, builder.omniSslContext); } } else { if (builder.username != null || builder.secretBytes != null) { @@ -1399,7 +1409,8 @@ private static Builder prepareBuilder(Builder builder) { private MetricsProvider metricsProvider = DefaultMetricsProvider.INSTANCE; private boolean enableLocationApi = SpannerOptions.environment.isEnableLocationApi(); private String monitoringHost = SpannerOptions.environment.getMonitoringHost(); - private SslContext mTLSContext = null; + private SslContextBuilder sslContextBuilder = null; + private SslContext omniSslContext = null; private boolean usePlainText = false; private TransactionOptions defaultTransactionOptions = TransactionOptions.getDefaultInstance(); private RequestOptions.ClientContext clientContext; @@ -2240,21 +2251,35 @@ public Builder setEmulatorHost(String emulatorHost) { /** * Configures mTLS authentication using the provided client certificate and key files. mTLS via - * useClientCert is only supported for Spanner Omni instances. + * useClientCert is only supported for Spanner Omni instances. Certificates and keys are loaded + * dynamically and reloaded automatically when rotated on disk. * * @param clientCertificate Path to the client certificate file. * @param clientCertificateKey Path to the client private key file. - * @throws SpannerException If an error occurs while configuring the mTLS context */ public Builder useClientCert(String clientCertificate, String clientCertificateKey) { - try { - this.mTLSContext = - GrpcSslContexts.forClient() - .keyManager(new File(clientCertificate), new File(clientCertificateKey)) - .build(); - } catch (Exception e) { - throw SpannerExceptionFactory.asSpannerException(e); + Preconditions.checkNotNull(clientCertificate, "clientCertificate cannot be null"); + Preconditions.checkNotNull(clientCertificateKey, "clientCertificateKey cannot be null"); + if (this.sslContextBuilder == null) { + this.sslContextBuilder = GrpcSslContexts.forClient(); + } + this.sslContextBuilder.keyManager( + new DynamicKeyManager(new File(clientCertificate), new File(clientCertificateKey))); + return this; + } + + /** + * Configures the server root CA certificate for SSL/TLS authentication. The CA certificate is + * loaded dynamically and reloaded automatically when rotated on disk. + * + * @param caCertificate Path to the server root CA certificate file. + */ + public Builder setCaCertificate(String caCertificate) { + Preconditions.checkNotNull(caCertificate, "caCertificate cannot be null"); + if (this.sslContextBuilder == null) { + this.sslContextBuilder = GrpcSslContexts.forClient(); } + this.sslContextBuilder.trustManager(new DynamicTrustManager(new File(caCertificate))); return this; } diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java index 00d616c53a6b..1237e88170eb 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java @@ -19,6 +19,7 @@ import static com.google.cloud.spanner.connection.ConnectionProperties.AUTOCOMMIT; import static com.google.cloud.spanner.connection.ConnectionProperties.AUTO_CONFIG_EMULATOR; import static com.google.cloud.spanner.connection.ConnectionProperties.AUTO_PARTITION_MODE; +import static com.google.cloud.spanner.connection.ConnectionProperties.CA_CERTIFICATE; import static com.google.cloud.spanner.connection.ConnectionProperties.CHANNEL_PROVIDER; import static com.google.cloud.spanner.connection.ConnectionProperties.CLIENT_CERTIFICATE; import static com.google.cloud.spanner.connection.ConnectionProperties.CLIENT_KEY; @@ -168,6 +169,7 @@ public class ConnectionOptions { static final String DEFAULT_CREDENTIALS = null; static final String DEFAULT_CLIENT_CERTIFICATE = null; static final String DEFAULT_CLIENT_KEY = null; + static final String DEFAULT_CA_CERTIFICATE = null; static final String DEFAULT_OAUTH_TOKEN = null; static final Integer DEFAULT_MIN_SESSIONS = null; static final Integer DEFAULT_MAX_SESSIONS = null; @@ -242,6 +244,9 @@ public class ConnectionOptions { /** Client key path to establish mTLS */ static final String CLIENT_KEY_PROPERTY_NAME = "clientKey"; + /** Server root CA certificate path for SSL/TLS */ + static final String CA_CERTIFICATE_PROPERTY_NAME = "caCertificate"; + /** Name of the 'autocommit' connection property. */ public static final String AUTOCOMMIT_PROPERTY_NAME = "autocommit"; @@ -676,6 +681,21 @@ public Builder setType(SpannerOptions.InstanceType instanceType) { return this; } + public Builder setClientCertificate(String clientCertificate) { + setConnectionPropertyValue(CLIENT_CERTIFICATE, clientCertificate); + return this; + } + + public Builder setClientCertificateKey(String clientCertificateKey) { + setConnectionPropertyValue(CLIENT_KEY, clientCertificateKey); + return this; + } + + public Builder setCaCertificate(String caCertificate) { + setConnectionPropertyValue(CA_CERTIFICATE, caCertificate); + return this; + } + /** * @return the {@link ConnectionOptions} */ @@ -1300,6 +1320,10 @@ String getClientCertificateKey() { return getInitialConnectionPropertyValue(CLIENT_KEY); } + String getCaCertificate() { + return getInitialConnectionPropertyValue(CA_CERTIFICATE); + } + /** * The (custom) user agent string to use for this connection. If null, then the * default JDBC user agent string will be used. diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionProperties.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionProperties.java index d501ab11b138..16dc4e22553c 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionProperties.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionProperties.java @@ -22,6 +22,7 @@ import static com.google.cloud.spanner.connection.ConnectionOptions.AUTO_BATCH_DML_UPDATE_COUNT_VERIFICATION_PROPERTY_NAME; import static com.google.cloud.spanner.connection.ConnectionOptions.AUTO_PARTITION_MODE_PROPERTY_NAME; import static com.google.cloud.spanner.connection.ConnectionOptions.BATCH_DML_UPDATE_COUNT_PROPERTY_NAME; +import static com.google.cloud.spanner.connection.ConnectionOptions.CA_CERTIFICATE_PROPERTY_NAME; import static com.google.cloud.spanner.connection.ConnectionOptions.CHANNEL_PROVIDER_PROPERTY_NAME; import static com.google.cloud.spanner.connection.ConnectionOptions.CLIENT_CERTIFICATE_PROPERTY_NAME; import static com.google.cloud.spanner.connection.ConnectionOptions.CLIENT_KEY_PROPERTY_NAME; @@ -42,6 +43,7 @@ import static com.google.cloud.spanner.connection.ConnectionOptions.DEFAULT_AUTO_BATCH_DML_UPDATE_COUNT_VERIFICATION; import static com.google.cloud.spanner.connection.ConnectionOptions.DEFAULT_AUTO_PARTITION_MODE; import static com.google.cloud.spanner.connection.ConnectionOptions.DEFAULT_BATCH_DML_UPDATE_COUNT; +import static com.google.cloud.spanner.connection.ConnectionOptions.DEFAULT_CA_CERTIFICATE; import static com.google.cloud.spanner.connection.ConnectionOptions.DEFAULT_CHANNEL_PROVIDER; import static com.google.cloud.spanner.connection.ConnectionOptions.DEFAULT_CLIENT_CERTIFICATE; import static com.google.cloud.spanner.connection.ConnectionOptions.DEFAULT_CLIENT_KEY; @@ -329,6 +331,13 @@ public class ConnectionProperties { DEFAULT_CLIENT_KEY, StringValueConverter.INSTANCE, Context.STARTUP); + static final ConnectionProperty CA_CERTIFICATE = + create( + CA_CERTIFICATE_PROPERTY_NAME, + "Specifies the file path to the server root CA certificate for SSL/TLS validation.", + DEFAULT_CA_CERTIFICATE, + StringValueConverter.INSTANCE, + Context.STARTUP); static final ConnectionProperty CREDENTIALS_URL = create( CREDENTIALS_PROPERTY_NAME, diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/SpannerPool.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/SpannerPool.java index 785d2c80cd3e..19772fba973e 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/SpannerPool.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/SpannerPool.java @@ -176,6 +176,7 @@ static class SpannerPoolKey { private final boolean enableEndToEndTracing; private final String clientCertificate; private final String clientCertificateKey; + private final String caCertificate; private final SpannerOptions.InstanceType instanceType; private final Boolean enableDirectAccess; private final String universeDomain; @@ -221,6 +222,7 @@ private SpannerPoolKey(ConnectionOptions options) throws IOException { this.enableEndToEndTracing = options.isEndToEndTracingEnabled(); this.clientCertificate = options.getClientCertificate(); this.clientCertificateKey = options.getClientCertificateKey(); + this.caCertificate = options.getCaCertificate(); this.instanceType = options.getInstanceType(); this.enableDirectAccess = options.isEnableDirectAccess(); this.universeDomain = options.getUniverseDomain(); @@ -261,6 +263,7 @@ public boolean equals(Object o) { && Objects.equals(this.enableEndToEndTracing, other.enableEndToEndTracing) && Objects.equals(this.clientCertificate, other.clientCertificate) && Objects.equals(this.clientCertificateKey, other.clientCertificateKey) + && Objects.equals(this.caCertificate, other.caCertificate) && Objects.equals(this.instanceType, other.instanceType) && Objects.equals(this.enableDirectAccess, other.enableDirectAccess) && Objects.equals(this.universeDomain, other.universeDomain) @@ -296,6 +299,7 @@ public int hashCode() { this.enableEndToEndTracing, this.clientCertificate, this.clientCertificateKey, + this.caCertificate, this.instanceType, this.enableDirectAccess, this.universeDomain, @@ -540,6 +544,9 @@ Spanner createSpanner(SpannerPoolKey key, ConnectionOptions options) { if (key.clientCertificate != null && key.clientCertificateKey != null) { builder.useClientCert(key.clientCertificate, key.clientCertificateKey); } + if (key.caCertificate != null) { + builder.setCaCertificate(key.caCertificate); + } if (key.instanceType != null) { builder.setType(key.instanceType); } diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java new file mode 100644 index 000000000000..130e472071f3 --- /dev/null +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java @@ -0,0 +1,251 @@ +/* + * Copyright 2026 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.spanner.omni; + +import com.google.api.core.InternalApi; +import com.google.common.base.Preconditions; +import java.io.ByteArrayInputStream; +import java.io.File; +import java.net.Socket; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.security.KeyFactory; +import java.security.Principal; +import java.security.PrivateKey; +import java.security.cert.Certificate; +import java.security.cert.CertificateException; +import java.security.cert.CertificateFactory; +import java.security.cert.X509Certificate; +import java.security.spec.PKCS8EncodedKeySpec; +import java.security.spec.RSAPrivateCrtKeySpec; +import java.util.Base64; +import java.util.Collection; +import java.util.logging.Level; +import java.util.logging.Logger; +import javax.net.ssl.SSLEngine; +import javax.net.ssl.X509ExtendedKeyManager; +import org.bouncycastle.asn1.pkcs.RSAPrivateKey; + +/** + * An {@link X509ExtendedKeyManager} that dynamically reloads client certificates and private keys + * from disk whenever the underlying files are modified or rotated. + */ +@InternalApi +public class DynamicKeyManager extends X509ExtendedKeyManager { + private static final Logger logger = Logger.getLogger(DynamicKeyManager.class.getName()); + private static final String CLIENT_ALIAS = "client"; + + private final File certFile; + private final File keyFile; + + private static class KeyMaterial { + final long certLastModified; + final long certLength; + final long keyLastModified; + final long keyLength; + final X509Certificate[] certificateChain; + final PrivateKey privateKey; + + KeyMaterial( + long certLastModified, + long certLength, + long keyLastModified, + long keyLength, + X509Certificate[] certificateChain, + PrivateKey privateKey) { + this.certLastModified = certLastModified; + this.certLength = certLength; + this.keyLastModified = keyLastModified; + this.keyLength = keyLength; + this.certificateChain = certificateChain; + this.privateKey = privateKey; + } + } + + private volatile KeyMaterial currentMaterial; + + public DynamicKeyManager(File certFile, File keyFile) { + this.certFile = Preconditions.checkNotNull(certFile, "certFile cannot be null"); + this.keyFile = Preconditions.checkNotNull(keyFile, "keyFile cannot be null"); + reloadMaterial(); + } + + private void checkAndReload() { + KeyMaterial existing = this.currentMaterial; + if (existing != null + && certFile.lastModified() == existing.certLastModified + && certFile.length() == existing.certLength + && keyFile.lastModified() == existing.keyLastModified + && keyFile.length() == existing.keyLength) { + return; + } + synchronized (this) { + existing = this.currentMaterial; + if (existing != null + && certFile.lastModified() == existing.certLastModified + && certFile.length() == existing.certLength + && keyFile.lastModified() == existing.keyLastModified + && keyFile.length() == existing.keyLength) { + return; + } + try { + reloadMaterial(); + } catch (Exception e) { + logger.log( + Level.WARNING, + "Failed to reload rotated client certificate/key from disk, retaining current material", + e); + } + } + } + + private void reloadMaterial() { + try { + long certMod = certFile.lastModified(); + long certLen = certFile.length(); + long keyMod = keyFile.lastModified(); + long keyLen = keyFile.length(); + + byte[] certBytes = Files.readAllBytes(certFile.toPath()); + byte[] keyBytes = Files.readAllBytes(keyFile.toPath()); + + X509Certificate[] chain = parseCertificates(certBytes); + PrivateKey key = parsePrivateKey(keyBytes); + + this.currentMaterial = new KeyMaterial(certMod, certLen, keyMod, keyLen, chain, key); + } catch (Exception e) { + if (this.currentMaterial != null) { + logger.log( + Level.WARNING, + "Error reloading client certificate or key, falling back to cached credentials", + e); + } else { + throw new RuntimeException("Failed to initialize client certificate/key", e); + } + } + } + + private static X509Certificate[] parseCertificates(byte[] certBytes) throws CertificateException { + CertificateFactory cf = CertificateFactory.getInstance("X.509"); + Collection certs = + cf.generateCertificates(new ByteArrayInputStream(certBytes)); + if (certs == null || certs.isEmpty()) { + throw new CertificateException("No certificates found in certificate file"); + } + return certs.toArray(new X509Certificate[0]); + } + + private static PrivateKey parsePrivateKey(byte[] keyBytes) throws Exception { + String keyStr = new String(keyBytes, StandardCharsets.US_ASCII); + if (keyStr.contains("-----BEGIN RSA PRIVATE KEY-----")) { + byte[] der = + extractPemContent( + keyStr, "-----BEGIN RSA PRIVATE KEY-----", "-----END RSA PRIVATE KEY-----"); + RSAPrivateKey rsaPrivKey = RSAPrivateKey.getInstance(der); + RSAPrivateCrtKeySpec keySpec = + new RSAPrivateCrtKeySpec( + rsaPrivKey.getModulus(), + rsaPrivKey.getPublicExponent(), + rsaPrivKey.getPrivateExponent(), + rsaPrivKey.getPrime1(), + rsaPrivKey.getPrime2(), + rsaPrivKey.getExponent1(), + rsaPrivKey.getExponent2(), + rsaPrivKey.getCoefficient()); + return KeyFactory.getInstance("RSA").generatePrivate(keySpec); + } + + byte[] der; + if (keyStr.contains("-----BEGIN PRIVATE KEY-----")) { + der = extractPemContent(keyStr, "-----BEGIN PRIVATE KEY-----", "-----END PRIVATE KEY-----"); + } else { + try { + der = Base64.getMimeDecoder().decode(keyBytes); + } catch (IllegalArgumentException e) { + der = keyBytes; + } + } + + PKCS8EncodedKeySpec spec = new PKCS8EncodedKeySpec(der); + try { + return KeyFactory.getInstance("RSA").generatePrivate(spec); + } catch (Exception e) { + return KeyFactory.getInstance("EC").generatePrivate(spec); + } + } + + private static byte[] extractPemContent(String pem, String beginMarker, String endMarker) { + int start = pem.indexOf(beginMarker); + if (start < 0) { + throw new IllegalArgumentException("PEM does not contain marker: " + beginMarker); + } + start += beginMarker.length(); + int end = pem.indexOf(endMarker, start); + if (end < 0) { + throw new IllegalArgumentException("PEM does not contain marker: " + endMarker); + } + String base64 = pem.substring(start, end).replaceAll("\\s+", ""); + return Base64.getDecoder().decode(base64); + } + + @Override + public String chooseClientAlias(String[] keyType, Principal[] issuers, Socket socket) { + checkAndReload(); + return CLIENT_ALIAS; + } + + @Override + public String chooseEngineClientAlias(String[] keyType, Principal[] issuers, SSLEngine engine) { + checkAndReload(); + return CLIENT_ALIAS; + } + + @Override + public X509Certificate[] getCertificateChain(String alias) { + checkAndReload(); + KeyMaterial mat = this.currentMaterial; + return mat != null ? mat.certificateChain.clone() : null; + } + + @Override + public PrivateKey getPrivateKey(String alias) { + checkAndReload(); + KeyMaterial mat = this.currentMaterial; + return mat != null ? mat.privateKey : null; + } + + @Override + public String[] getClientAliases(String keyType, Principal[] issuers) { + checkAndReload(); + return new String[] {CLIENT_ALIAS}; + } + + @Override + public String[] getServerAliases(String keyType, Principal[] issuers) { + return null; + } + + @Override + public String chooseServerAlias(String keyType, Principal[] issuers, Socket socket) { + return null; + } + + @Override + public String chooseEngineServerAlias(String keyType, Principal[] issuers, SSLEngine engine) { + return null; + } +} diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java new file mode 100644 index 000000000000..870aaaa1d458 --- /dev/null +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java @@ -0,0 +1,246 @@ +/* + * Copyright 2026 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.spanner.omni; + +import com.google.api.core.InternalApi; +import java.io.ByteArrayInputStream; +import java.io.File; +import java.net.Socket; +import java.nio.file.Files; +import java.security.GeneralSecurityException; +import java.security.KeyStore; +import java.security.cert.Certificate; +import java.security.cert.CertificateException; +import java.security.cert.CertificateFactory; +import java.security.cert.X509Certificate; +import java.util.Collection; +import java.util.logging.Level; +import java.util.logging.Logger; +import javax.annotation.Nullable; +import javax.net.ssl.SSLEngine; +import javax.net.ssl.TrustManager; +import javax.net.ssl.TrustManagerFactory; +import javax.net.ssl.X509ExtendedTrustManager; +import javax.net.ssl.X509TrustManager; + +/** + * An {@link X509ExtendedTrustManager} that dynamically reloads root CA certificates from disk + * whenever the certificate file is modified or rotated. + */ +@InternalApi +public class DynamicTrustManager extends X509ExtendedTrustManager { + private static final Logger logger = Logger.getLogger(DynamicTrustManager.class.getName()); + + private final File caCertFile; + + private static class TrustMaterial { + final long lastModified; + final long length; + final X509ExtendedTrustManager delegate; + + TrustMaterial(long lastModified, long length, X509ExtendedTrustManager delegate) { + this.lastModified = lastModified; + this.length = length; + this.delegate = delegate; + } + } + + private volatile TrustMaterial currentMaterial; + + public DynamicTrustManager(@Nullable File caCertFile) { + this.caCertFile = caCertFile; + reloadMaterial(); + } + + private void checkAndReload() { + if (this.caCertFile == null) { + return; + } + TrustMaterial existing = this.currentMaterial; + if (existing != null + && caCertFile.lastModified() == existing.lastModified + && caCertFile.length() == existing.length) { + return; + } + synchronized (this) { + existing = this.currentMaterial; + if (existing != null + && caCertFile.lastModified() == existing.lastModified + && caCertFile.length() == existing.length) { + return; + } + try { + reloadMaterial(); + } catch (Exception e) { + logger.log( + Level.WARNING, + "Failed to reload rotated CA certificate from disk, retaining previous material", + e); + } + } + } + + private void reloadMaterial() { + try { + if (this.caCertFile == null) { + TrustManagerFactory tmf = + TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()); + tmf.init((KeyStore) null); + this.currentMaterial = new TrustMaterial(0, 0, findExtendedTrustManager(tmf)); + return; + } + + long mod = caCertFile.lastModified(); + long len = caCertFile.length(); + byte[] certBytes = Files.readAllBytes(caCertFile.toPath()); + + CertificateFactory cf = CertificateFactory.getInstance("X.509"); + Collection certs = + cf.generateCertificates(new ByteArrayInputStream(certBytes)); + if (certs == null || certs.isEmpty()) { + throw new CertificateException("No certificates found in CA certificate file"); + } + + KeyStore ks = KeyStore.getInstance(KeyStore.getDefaultType()); + ks.load(null, null); + int index = 0; + for (Certificate cert : certs) { + ks.setCertificateEntry("spanner-ca-" + (++index), cert); + } + + TrustManagerFactory tmf = + TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()); + tmf.init(ks); + + this.currentMaterial = new TrustMaterial(mod, len, findExtendedTrustManager(tmf)); + } catch (Exception e) { + if (this.currentMaterial != null) { + logger.log( + Level.WARNING, + "Error reloading CA certificate, falling back to cached trust manager", + e); + } else { + throw new RuntimeException("Failed to initialize CA certificate", e); + } + } + } + + private static X509ExtendedTrustManager findExtendedTrustManager(TrustManagerFactory tmf) + throws GeneralSecurityException { + for (TrustManager tm : tmf.getTrustManagers()) { + if (tm instanceof X509ExtendedTrustManager) { + return (X509ExtendedTrustManager) tm; + } else if (tm instanceof X509TrustManager) { + return wrapTrustManager((X509TrustManager) tm); + } + } + throw new GeneralSecurityException("No X509TrustManager found in TrustManagerFactory"); + } + + private static X509ExtendedTrustManager wrapTrustManager(final X509TrustManager tm) { + return new X509ExtendedTrustManager() { + @Override + public void checkClientTrusted(X509Certificate[] chain, String authType, Socket socket) + throws CertificateException { + tm.checkClientTrusted(chain, authType); + } + + @Override + public void checkServerTrusted(X509Certificate[] chain, String authType, Socket socket) + throws CertificateException { + tm.checkServerTrusted(chain, authType); + } + + @Override + public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngine engine) + throws CertificateException { + tm.checkClientTrusted(chain, authType); + } + + @Override + public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngine engine) + throws CertificateException { + tm.checkServerTrusted(chain, authType); + } + + @Override + public void checkClientTrusted(X509Certificate[] chain, String authType) + throws CertificateException { + tm.checkClientTrusted(chain, authType); + } + + @Override + public void checkServerTrusted(X509Certificate[] chain, String authType) + throws CertificateException { + tm.checkServerTrusted(chain, authType); + } + + @Override + public X509Certificate[] getAcceptedIssuers() { + return tm.getAcceptedIssuers(); + } + }; + } + + @Override + public void checkClientTrusted(X509Certificate[] chain, String authType, Socket socket) + throws CertificateException { + checkAndReload(); + this.currentMaterial.delegate.checkClientTrusted(chain, authType, socket); + } + + @Override + public void checkServerTrusted(X509Certificate[] chain, String authType, Socket socket) + throws CertificateException { + checkAndReload(); + this.currentMaterial.delegate.checkServerTrusted(chain, authType, socket); + } + + @Override + public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngine engine) + throws CertificateException { + checkAndReload(); + this.currentMaterial.delegate.checkClientTrusted(chain, authType, engine); + } + + @Override + public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngine engine) + throws CertificateException { + checkAndReload(); + this.currentMaterial.delegate.checkServerTrusted(chain, authType, engine); + } + + @Override + public void checkClientTrusted(X509Certificate[] chain, String authType) + throws CertificateException { + checkAndReload(); + this.currentMaterial.delegate.checkClientTrusted(chain, authType); + } + + @Override + public void checkServerTrusted(X509Certificate[] chain, String authType) + throws CertificateException { + checkAndReload(); + this.currentMaterial.delegate.checkServerTrusted(chain, authType); + } + + @Override + public X509Certificate[] getAcceptedIssuers() { + checkAndReload(); + return this.currentMaterial.delegate.getAcceptedIssuers(); + } +} diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/testing/SpannerOmniHelper.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/testing/SpannerOmniHelper.java index 463c485ca737..3bad14c5cb13 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/testing/SpannerOmniHelper.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/testing/SpannerOmniHelper.java @@ -26,6 +26,7 @@ public class SpannerOmniHelper { private static final String USE_MTLS = "spanner.mtls"; private static final String CLIENT_CERT_PATH = "spanner.client_cert_path"; private static final String CLIENT_CERT_KEY_PATH = "spanner.client_cert_key_path"; + private static final String CA_CERT_PATH = "spanner.ca_cert_path"; private static final String USERNAME = "spanner.username"; private static final String PASSWORD = "spanner.password"; @@ -56,6 +57,10 @@ public static void appendSpannerOmniProperties(StringBuilder uri) { uri.append(";clientCertificate=").append(clientCertificate); uri.append(";clientKey=").append(clientKey); } + String caCertPath = System.getProperty(CA_CERT_PATH, ""); + if (!Strings.isNullOrEmpty(caCertPath)) { + uri.append(";caCertificate=").append(caCertPath); + } } public static boolean isMtlsSetup() { @@ -79,10 +84,14 @@ public static void setSpannerOmniOptions(SpannerOptions.Builder builder) { if (usePlainText) { builder.usePlainText(); } + String caCertPath = System.getProperty(CA_CERT_PATH, ""); if (isMtlsSetup()) { String clientCertificate = System.getProperty(CLIENT_CERT_PATH, ""); String clientKey = System.getProperty(CLIENT_CERT_KEY_PATH, ""); builder.useClientCert(clientCertificate, clientKey); } + if (!Strings.isNullOrEmpty(caCertPath)) { + builder.setCaCertificate(caCertPath); + } } } diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java index 4b754c74027f..83003b144ce0 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java @@ -1684,4 +1684,54 @@ public ApiCallContext configure( customOptions.toBuilder().setCallContextConfigurator(null).build(); assertNull(clearedOptions.getCallContextConfigurator()); } + + @Test + public void testUseClientCertAndTrustCertificate() throws Exception { + io.grpc.netty.shaded.io.netty.handler.ssl.util.SelfSignedCertificate ssc = + new io.grpc.netty.shaded.io.netty.handler.ssl.util.SelfSignedCertificate("spanner.test"); + io.grpc.netty.shaded.io.netty.handler.ssl.util.SelfSignedCertificate ca = + new io.grpc.netty.shaded.io.netty.handler.ssl.util.SelfSignedCertificate("spanner.ca"); + + String certPath = ssc.certificate().getAbsolutePath(); + String keyPath = ssc.privateKey().getAbsolutePath(); + String caPath = ca.certificate().getAbsolutePath(); + + SpannerOptions options = + SpannerOptions.newBuilder() + .setProjectId("test-project") + .setCredentials(NoCredentials.getInstance()) + .setHost("https://localhost:1234") + .useClientCert(certPath, keyPath) + .setCaCertificate(caPath) + .build(); + + assertNotNull(options.getChannelConfigurator()); + + SpannerOptions fromBuilder = options.toBuilder().build(); + assertNotNull(fromBuilder.getChannelConfigurator()); + + // Test standalone setCaCertificate + SpannerOptions caOnlyOptions = + SpannerOptions.newBuilder() + .setProjectId("test-project") + .setCredentials(NoCredentials.getInstance()) + .setHost("https://localhost:1234") + .setCaCertificate(caPath) + .build(); + + assertNotNull(caOnlyOptions.getChannelConfigurator()); + + // Test setCaCertificate combined with login (username/password) + SpannerOptions loginWithCaOptions = + SpannerOptions.newBuilder() + .setProjectId("test-project") + .setType(SpannerOptions.InstanceType.OMNI) + .setHost("https://localhost:1234") + .setCaCertificate(caPath) + .login("test-user", "test-pass".toCharArray()) + .build(); + + assertTrue(loginWithCaOptions.getCredentials() instanceof SpannerOmniCredentials); + assertNotNull(loginWithCaOptions.getChannelConfigurator()); + } } diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionOptionsTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionOptionsTest.java index 38ab65b1523f..6722466ef495 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionOptionsTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionOptionsTest.java @@ -1637,4 +1637,31 @@ public void testGrpcKeepAliveTimeoutOption() { .build(); assertNull(defaultOptions.getGrpcKeepAliveTimeout()); } + + @Test + public void testCertificateAndTrustOptions() { + ConnectionOptions optionsFromUri = + ConnectionOptions.newBuilder() + .setUri( + "cloudspanner:/projects/test-project-123/instances/test-instance/databases/test-database" + + "?clientCertificate=/path/to/client.crt;clientKey=/path/to/client.key;caCertificate=/path/to/ca.crt") + .setCredentials(NoCredentials.getInstance()) + .build(); + assertEquals("/path/to/client.crt", optionsFromUri.getClientCertificate()); + assertEquals("/path/to/client.key", optionsFromUri.getClientCertificateKey()); + assertEquals("/path/to/ca.crt", optionsFromUri.getCaCertificate()); + + ConnectionOptions optionsFromBuilder = + ConnectionOptions.newBuilder() + .setUri( + "cloudspanner:/projects/test-project-123/instances/test-instance/databases/test-database") + .setClientCertificate("/path/to/builder/client.crt") + .setClientCertificateKey("/path/to/builder/client.key") + .setCaCertificate("/path/to/builder/ca.crt") + .setCredentials(NoCredentials.getInstance()) + .build(); + assertEquals("/path/to/builder/client.crt", optionsFromBuilder.getClientCertificate()); + assertEquals("/path/to/builder/client.key", optionsFromBuilder.getClientCertificateKey()); + assertEquals("/path/to/builder/ca.crt", optionsFromBuilder.getCaCertificate()); + } } diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/SpannerPoolTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/SpannerPoolTest.java index 68951cc57618..91f671199620 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/SpannerPoolTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/SpannerPoolTest.java @@ -848,4 +848,49 @@ public void testGrpcGcpSettings() { .setCredentials(NoCredentials.getInstance()) .build())); } + + @Test + public void testCertificateAndTrustSettings() { + SpannerPoolKey keyDefault = + SpannerPoolKey.of( + ConnectionOptions.newBuilder() + .setUri("cloudspanner:/projects/p/instances/i/databases/d") + .setCredentials(NoCredentials.getInstance()) + .build()); + SpannerPoolKey keyWithTrustCert1 = + SpannerPoolKey.of( + ConnectionOptions.newBuilder() + .setUri( + "cloudspanner:/projects/p/instances/i/databases/d?caCertificate=/path/to/ca1.crt") + .setCredentials(NoCredentials.getInstance()) + .build()); + SpannerPoolKey keyWithTrustCert2 = + SpannerPoolKey.of( + ConnectionOptions.newBuilder() + .setUri( + "cloudspanner:/projects/p/instances/i/databases/d?caCertificate=/path/to/ca2.crt") + .setCredentials(NoCredentials.getInstance()) + .build()); + SpannerPoolKey keyWithClientCert = + SpannerPoolKey.of( + ConnectionOptions.newBuilder() + .setUri( + "cloudspanner:/projects/p/instances/i/databases/d" + + "?clientCertificate=/path/to/client.crt;clientKey=/path/to/client.key;caCertificate=/path/to/ca1.crt") + .setCredentials(NoCredentials.getInstance()) + .build()); + + assertNotEquals(keyDefault, keyWithTrustCert1); + assertNotEquals(keyWithTrustCert1, keyWithTrustCert2); + assertNotEquals(keyWithTrustCert1, keyWithClientCert); + + assertEquals( + keyWithTrustCert1, + SpannerPoolKey.of( + ConnectionOptions.newBuilder() + .setUri( + "cloudspanner:/projects/p/instances/i/databases/d?caCertificate=/path/to/ca1.crt") + .setCredentials(NoCredentials.getInstance()) + .build())); + } } diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java new file mode 100644 index 000000000000..49aae9307415 --- /dev/null +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java @@ -0,0 +1,136 @@ +/* + * Copyright 2026 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.spanner.omni; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThrows; + +import io.grpc.netty.shaded.io.netty.handler.ssl.util.SelfSignedCertificate; +import java.io.File; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.security.PrivateKey; +import java.security.cert.X509Certificate; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class DynamicKeyManagerTest { + + @Rule public TemporaryFolder tempFolder = new TemporaryFolder(); + + @Test + public void testInitialLoadAndDynamicRotation() throws Exception { + SelfSignedCertificate ssc1 = new SelfSignedCertificate("spanner.test.1"); + File certFile = tempFolder.newFile("client.crt"); + File keyFile = tempFolder.newFile("client.key"); + + Files.write(certFile.toPath(), Files.readAllBytes(ssc1.certificate().toPath())); + Files.write(keyFile.toPath(), Files.readAllBytes(ssc1.privateKey().toPath())); + + DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile); + + String alias1 = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); + assertNotNull(alias1); + assertEquals(alias1, keyManager.chooseEngineClientAlias(new String[] {"RSA"}, null, null)); + + X509Certificate[] chain1 = keyManager.getCertificateChain(alias1); + assertNotNull(chain1); + assertEquals(1, chain1.length); + assertEquals(ssc1.cert().getSubjectDN(), chain1[0].getSubjectDN()); + + PrivateKey pk1 = keyManager.getPrivateKey(alias1); + assertNotNull(pk1); + assertEquals(ssc1.key().getAlgorithm(), pk1.getAlgorithm()); + + String[] aliases1 = keyManager.getClientAliases("RSA", null); + assertNotNull(aliases1); + assertEquals(1, aliases1.length); + assertEquals(alias1, aliases1[0]); + + // Ensure lastModified timestamp changes upon rotation + Thread.sleep(1100); + + SelfSignedCertificate ssc2 = new SelfSignedCertificate("spanner.test.2"); + Files.write(certFile.toPath(), Files.readAllBytes(ssc2.certificate().toPath())); + Files.write(keyFile.toPath(), Files.readAllBytes(ssc2.privateKey().toPath())); + + String alias2 = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); + assertNotNull(alias2); + + X509Certificate[] chain2 = keyManager.getCertificateChain(alias2); + assertNotNull(chain2); + assertEquals(ssc2.cert().getSubjectDN(), chain2[0].getSubjectDN()); + + PrivateKey pk2 = keyManager.getPrivateKey(alias2); + assertNotNull(pk2); + } + + @Test + public void testCorruptRotationFallsBackToPrevious() throws Exception { + SelfSignedCertificate ssc = new SelfSignedCertificate("spanner.test.fallback"); + File certFile = tempFolder.newFile("client-fallback.crt"); + File keyFile = tempFolder.newFile("client-fallback.key"); + + Files.write(certFile.toPath(), Files.readAllBytes(ssc.certificate().toPath())); + Files.write(keyFile.toPath(), Files.readAllBytes(ssc.privateKey().toPath())); + + DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile); + String aliasBefore = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); + assertNotNull(aliasBefore); + + Thread.sleep(1100); + + // Overwrite certFile with corrupt bytes + Files.write(certFile.toPath(), "NOT A CERTIFICATE CONTENT".getBytes(StandardCharsets.UTF_8)); + + // DynamicKeyManager should catch reload error and retain previous material + String aliasAfter = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); + assertEquals(aliasBefore, aliasAfter); + assertNotNull(keyManager.getCertificateChain(aliasAfter)); + assertNotNull(keyManager.getPrivateKey(aliasAfter)); + } + + @Test + public void testNonExistentFileFailsInitialization() { + File nonExistentCert = new File(tempFolder.getRoot(), "missing.crt"); + File nonExistentKey = new File(tempFolder.getRoot(), "missing.key"); + + assertThrows( + RuntimeException.class, () -> new DynamicKeyManager(nonExistentCert, nonExistentKey)); + } + + @Test + public void testServerAliasesReturnNull() throws Exception { + SelfSignedCertificate ssc = new SelfSignedCertificate("spanner.test.server"); + File certFile = tempFolder.newFile("server-test.crt"); + File keyFile = tempFolder.newFile("server-test.key"); + + Files.write(certFile.toPath(), Files.readAllBytes(ssc.certificate().toPath())); + Files.write(keyFile.toPath(), Files.readAllBytes(ssc.privateKey().toPath())); + + DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile); + assertNull(keyManager.getServerAliases("RSA", null)); + assertNull(keyManager.chooseServerAlias("RSA", null, null)); + assertNull(keyManager.chooseEngineServerAlias("RSA", null, null)); + } +} diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicTrustManagerTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicTrustManagerTest.java new file mode 100644 index 000000000000..5571a17f9086 --- /dev/null +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicTrustManagerTest.java @@ -0,0 +1,138 @@ +/* + * Copyright 2026 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.spanner.omni; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; + +import io.grpc.netty.shaded.io.netty.handler.ssl.util.SelfSignedCertificate; +import java.io.File; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.security.cert.CertificateException; +import java.security.cert.X509Certificate; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class DynamicTrustManagerTest { + + @Rule public TemporaryFolder tempFolder = new TemporaryFolder(); + + @Test + public void testDefaultTrustManagerWithNull() throws Exception { + DynamicTrustManager trustManager = new DynamicTrustManager((File) null); + X509Certificate[] issuers = trustManager.getAcceptedIssuers(); + assertNotNull(issuers); + assertTrue(issuers.length > 0); + } + + @Test + public void testCustomTrustManagerAndDynamicRotation() throws Exception { + SelfSignedCertificate ca1 = new SelfSignedCertificate("spanner.ca.1"); + File caFile = tempFolder.newFile("ca.crt"); + Files.write(caFile.toPath(), Files.readAllBytes(ca1.certificate().toPath())); + + DynamicTrustManager trustManager = new DynamicTrustManager(caFile); + + X509Certificate[] issuers1 = trustManager.getAcceptedIssuers(); + assertNotNull(issuers1); + assertEquals(1, issuers1.length); + assertEquals(ca1.cert().getSubjectDN(), issuers1[0].getSubjectDN()); + + // Validating ca1 cert should succeed + trustManager.checkServerTrusted(new X509Certificate[] {ca1.cert()}, "RSA"); + + SelfSignedCertificate ca2 = new SelfSignedCertificate("spanner.ca.2"); + + // Validating ca2 cert with ca1 trusted should fail + assertThrows( + CertificateException.class, + () -> trustManager.checkServerTrusted(new X509Certificate[] {ca2.cert()}, "RSA")); + + Thread.sleep(1100); + + // Rotate CA file on disk to ca2 + Files.write(caFile.toPath(), Files.readAllBytes(ca2.certificate().toPath())); + + // Now ca2 should be accepted and ca1 should be rejected + X509Certificate[] issuers2 = trustManager.getAcceptedIssuers(); + assertNotNull(issuers2); + assertEquals(1, issuers2.length); + assertEquals(ca2.cert().getSubjectDN(), issuers2[0].getSubjectDN()); + + trustManager.checkServerTrusted(new X509Certificate[] {ca2.cert()}, "RSA"); + + assertThrows( + CertificateException.class, + () -> trustManager.checkServerTrusted(new X509Certificate[] {ca1.cert()}, "RSA")); + } + + @Test + public void testMultipleCAsInFile() throws Exception { + SelfSignedCertificate ca1 = new SelfSignedCertificate("spanner.multi.ca.1"); + SelfSignedCertificate ca2 = new SelfSignedCertificate("spanner.multi.ca.2"); + + File caFile = tempFolder.newFile("multi-ca.crt"); + byte[] bundle = + (new String(Files.readAllBytes(ca1.certificate().toPath()), StandardCharsets.UTF_8) + + "\n" + + new String( + Files.readAllBytes(ca2.certificate().toPath()), StandardCharsets.UTF_8)) + .getBytes(StandardCharsets.UTF_8); + Files.write(caFile.toPath(), bundle); + + DynamicTrustManager trustManager = new DynamicTrustManager(caFile); + + X509Certificate[] issuers = trustManager.getAcceptedIssuers(); + assertNotNull(issuers); + assertEquals(2, issuers.length); + + trustManager.checkServerTrusted(new X509Certificate[] {ca1.cert()}, "RSA"); + trustManager.checkServerTrusted(new X509Certificate[] {ca2.cert()}, "RSA"); + } + + @Test + public void testCorruptRotationFallsBackToPrevious() throws Exception { + SelfSignedCertificate ca = new SelfSignedCertificate("spanner.ca.fallback"); + File caFile = tempFolder.newFile("ca-fallback.crt"); + Files.write(caFile.toPath(), Files.readAllBytes(ca.certificate().toPath())); + + DynamicTrustManager trustManager = new DynamicTrustManager(caFile); + trustManager.checkServerTrusted(new X509Certificate[] {ca.cert()}, "RSA"); + + Thread.sleep(1100); + + // Corrupt the file + Files.write(caFile.toPath(), "CORRUPT CERT DATA".getBytes(StandardCharsets.UTF_8)); + + // Trust manager should retain previous CA + trustManager.checkServerTrusted(new X509Certificate[] {ca.cert()}, "RSA"); + assertEquals(1, trustManager.getAcceptedIssuers().length); + } + + @Test + public void testNonExistentFileFailsInitialization() { + File nonExistent = new File(tempFolder.getRoot(), "missing-ca.crt"); + assertThrows(RuntimeException.class, () -> new DynamicTrustManager(nonExistent)); + } +} From 8473bb730ddeae2fd65c2833996cd40d7de92030 Mon Sep 17 00:00:00 2001 From: sagnghos Date: Mon, 21 Sep 2026 05:56:08 +0000 Subject: [PATCH 02/22] fix(spanner): Throttle key/trust manager file stat checks and use native PKCS8 key parser --- .../cloud/spanner/omni/DynamicKeyManager.java | 34 ++++++++----------- .../spanner/omni/DynamicTrustManager.java | 14 ++++++++ .../spanner/omni/DynamicKeyManagerTest.java | 30 ++++++++++++++-- .../spanner/omni/DynamicTrustManagerTest.java | 26 ++++++++++++-- 4 files changed, 80 insertions(+), 24 deletions(-) diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java index 130e472071f3..45f39ddeded2 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java @@ -31,14 +31,12 @@ import java.security.cert.CertificateFactory; import java.security.cert.X509Certificate; import java.security.spec.PKCS8EncodedKeySpec; -import java.security.spec.RSAPrivateCrtKeySpec; import java.util.Base64; import java.util.Collection; import java.util.logging.Level; import java.util.logging.Logger; import javax.net.ssl.SSLEngine; import javax.net.ssl.X509ExtendedKeyManager; -import org.bouncycastle.asn1.pkcs.RSAPrivateKey; /** * An {@link X509ExtendedKeyManager} that dynamically reloads client certificates and private keys @@ -48,9 +46,12 @@ public class DynamicKeyManager extends X509ExtendedKeyManager { private static final Logger logger = Logger.getLogger(DynamicKeyManager.class.getName()); private static final String CLIENT_ALIAS = "client"; + private static final long DEFAULT_CHECK_INTERVAL_MS = 5000L; private final File certFile; private final File keyFile; + private final long checkIntervalMs; + private volatile long lastCheckedMs; private static class KeyMaterial { final long certLastModified; @@ -79,12 +80,23 @@ private static class KeyMaterial { private volatile KeyMaterial currentMaterial; public DynamicKeyManager(File certFile, File keyFile) { + this(certFile, keyFile, DEFAULT_CHECK_INTERVAL_MS); + } + + DynamicKeyManager(File certFile, File keyFile, long checkIntervalMs) { this.certFile = Preconditions.checkNotNull(certFile, "certFile cannot be null"); this.keyFile = Preconditions.checkNotNull(keyFile, "keyFile cannot be null"); + this.checkIntervalMs = checkIntervalMs; reloadMaterial(); + this.lastCheckedMs = System.currentTimeMillis(); } private void checkAndReload() { + long now = System.currentTimeMillis(); + if (now - lastCheckedMs < checkIntervalMs) { + return; + } + lastCheckedMs = now; KeyMaterial existing = this.currentMaterial; if (existing != null && certFile.lastModified() == existing.certLastModified @@ -151,24 +163,6 @@ private static X509Certificate[] parseCertificates(byte[] certBytes) throws Cert private static PrivateKey parsePrivateKey(byte[] keyBytes) throws Exception { String keyStr = new String(keyBytes, StandardCharsets.US_ASCII); - if (keyStr.contains("-----BEGIN RSA PRIVATE KEY-----")) { - byte[] der = - extractPemContent( - keyStr, "-----BEGIN RSA PRIVATE KEY-----", "-----END RSA PRIVATE KEY-----"); - RSAPrivateKey rsaPrivKey = RSAPrivateKey.getInstance(der); - RSAPrivateCrtKeySpec keySpec = - new RSAPrivateCrtKeySpec( - rsaPrivKey.getModulus(), - rsaPrivKey.getPublicExponent(), - rsaPrivKey.getPrivateExponent(), - rsaPrivKey.getPrime1(), - rsaPrivKey.getPrime2(), - rsaPrivKey.getExponent1(), - rsaPrivKey.getExponent2(), - rsaPrivKey.getCoefficient()); - return KeyFactory.getInstance("RSA").generatePrivate(keySpec); - } - byte[] der; if (keyStr.contains("-----BEGIN PRIVATE KEY-----")) { der = extractPemContent(keyStr, "-----BEGIN PRIVATE KEY-----", "-----END PRIVATE KEY-----"); diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java index 870aaaa1d458..5c0134eccb66 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java @@ -44,8 +44,11 @@ @InternalApi public class DynamicTrustManager extends X509ExtendedTrustManager { private static final Logger logger = Logger.getLogger(DynamicTrustManager.class.getName()); + private static final long DEFAULT_CHECK_INTERVAL_MS = 5000L; private final File caCertFile; + private final long checkIntervalMs; + private volatile long lastCheckedMs; private static class TrustMaterial { final long lastModified; @@ -62,14 +65,25 @@ private static class TrustMaterial { private volatile TrustMaterial currentMaterial; public DynamicTrustManager(@Nullable File caCertFile) { + this(caCertFile, DEFAULT_CHECK_INTERVAL_MS); + } + + DynamicTrustManager(@Nullable File caCertFile, long checkIntervalMs) { this.caCertFile = caCertFile; + this.checkIntervalMs = checkIntervalMs; reloadMaterial(); + this.lastCheckedMs = System.currentTimeMillis(); } private void checkAndReload() { if (this.caCertFile == null) { return; } + long now = System.currentTimeMillis(); + if (now - lastCheckedMs < checkIntervalMs) { + return; + } + lastCheckedMs = now; TrustMaterial existing = this.currentMaterial; if (existing != null && caCertFile.lastModified() == existing.lastModified diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java index 49aae9307415..72eb28e901af 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java @@ -47,7 +47,7 @@ public void testInitialLoadAndDynamicRotation() throws Exception { Files.write(certFile.toPath(), Files.readAllBytes(ssc1.certificate().toPath())); Files.write(keyFile.toPath(), Files.readAllBytes(ssc1.privateKey().toPath())); - DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile); + DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile, 0L); String alias1 = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); assertNotNull(alias1); @@ -85,6 +85,32 @@ public void testInitialLoadAndDynamicRotation() throws Exception { assertNotNull(pk2); } + @Test + public void testFileCheckThrottling() throws Exception { + SelfSignedCertificate ssc1 = new SelfSignedCertificate("spanner.test.throttle1"); + File certFile = tempFolder.newFile("client-throttle.crt"); + File keyFile = tempFolder.newFile("client-throttle.key"); + + Files.write(certFile.toPath(), Files.readAllBytes(ssc1.certificate().toPath())); + Files.write(keyFile.toPath(), Files.readAllBytes(ssc1.privateKey().toPath())); + + // 60-second check interval + DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile, 60000L); + + String alias1 = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); + assertEquals( + ssc1.cert().getSubjectDN(), keyManager.getCertificateChain(alias1)[0].getSubjectDN()); + + // Rotate files immediately on disk + SelfSignedCertificate ssc2 = new SelfSignedCertificate("spanner.test.throttle2"); + Files.write(certFile.toPath(), Files.readAllBytes(ssc2.certificate().toPath())); + Files.write(keyFile.toPath(), Files.readAllBytes(ssc2.privateKey().toPath())); + + // Within the throttle interval, the manager should retain and return previous certificate + assertEquals( + ssc1.cert().getSubjectDN(), keyManager.getCertificateChain(alias1)[0].getSubjectDN()); + } + @Test public void testCorruptRotationFallsBackToPrevious() throws Exception { SelfSignedCertificate ssc = new SelfSignedCertificate("spanner.test.fallback"); @@ -94,7 +120,7 @@ public void testCorruptRotationFallsBackToPrevious() throws Exception { Files.write(certFile.toPath(), Files.readAllBytes(ssc.certificate().toPath())); Files.write(keyFile.toPath(), Files.readAllBytes(ssc.privateKey().toPath())); - DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile); + DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile, 0L); String aliasBefore = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); assertNotNull(aliasBefore); diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicTrustManagerTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicTrustManagerTest.java index 5571a17f9086..743a74e18d1d 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicTrustManagerTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicTrustManagerTest.java @@ -52,7 +52,7 @@ public void testCustomTrustManagerAndDynamicRotation() throws Exception { File caFile = tempFolder.newFile("ca.crt"); Files.write(caFile.toPath(), Files.readAllBytes(ca1.certificate().toPath())); - DynamicTrustManager trustManager = new DynamicTrustManager(caFile); + DynamicTrustManager trustManager = new DynamicTrustManager(caFile, 0L); X509Certificate[] issuers1 = trustManager.getAcceptedIssuers(); assertNotNull(issuers1); @@ -87,6 +87,28 @@ public void testCustomTrustManagerAndDynamicRotation() throws Exception { () -> trustManager.checkServerTrusted(new X509Certificate[] {ca1.cert()}, "RSA")); } + @Test + public void testFileCheckThrottling() throws Exception { + SelfSignedCertificate ca1 = new SelfSignedCertificate("spanner.ca.throttle1"); + File caFile = tempFolder.newFile("ca-throttle.crt"); + Files.write(caFile.toPath(), Files.readAllBytes(ca1.certificate().toPath())); + + // 60-second check interval + DynamicTrustManager trustManager = new DynamicTrustManager(caFile, 60000L); + + X509Certificate[] issuers1 = trustManager.getAcceptedIssuers(); + assertEquals(1, issuers1.length); + assertEquals(ca1.cert().getSubjectDN(), issuers1[0].getSubjectDN()); + + // Rotate CA on disk immediately + SelfSignedCertificate ca2 = new SelfSignedCertificate("spanner.ca.throttle2"); + Files.write(caFile.toPath(), Files.readAllBytes(ca2.certificate().toPath())); + + // Within throttle interval, trust manager should retain previous CA + assertEquals(ca1.cert().getSubjectDN(), trustManager.getAcceptedIssuers()[0].getSubjectDN()); + trustManager.checkServerTrusted(new X509Certificate[] {ca1.cert()}, "RSA"); + } + @Test public void testMultipleCAsInFile() throws Exception { SelfSignedCertificate ca1 = new SelfSignedCertificate("spanner.multi.ca.1"); @@ -117,7 +139,7 @@ public void testCorruptRotationFallsBackToPrevious() throws Exception { File caFile = tempFolder.newFile("ca-fallback.crt"); Files.write(caFile.toPath(), Files.readAllBytes(ca.certificate().toPath())); - DynamicTrustManager trustManager = new DynamicTrustManager(caFile); + DynamicTrustManager trustManager = new DynamicTrustManager(caFile, 0L); trustManager.checkServerTrusted(new X509Certificate[] {ca.cert()}, "RSA"); Thread.sleep(1100); From c5ec704d8d40c4fee0a31d66a813e4f3d46d0bd7 Mon Sep 17 00:00:00 2001 From: sagnghos Date: Mon, 21 Sep 2026 06:41:20 +0000 Subject: [PATCH 03/22] fix(spanner): Detect PKCS#1 keys with actionable error and clean up test certs --- .../cloud/spanner/omni/DynamicKeyManager.java | 17 +- .../cloud/spanner/SpannerOptionsTest.java | 73 ++++--- .../spanner/omni/DynamicKeyManagerTest.java | 204 +++++++++++------- .../spanner/omni/DynamicTrustManagerTest.java | 159 ++++++++------ 4 files changed, 267 insertions(+), 186 deletions(-) diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java index 45f39ddeded2..7a8c997051b7 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java @@ -139,6 +139,15 @@ private void reloadMaterial() { PrivateKey key = parsePrivateKey(keyBytes); this.currentMaterial = new KeyMaterial(certMod, certLen, keyMod, keyLen, chain, key); + } catch (IllegalArgumentException e) { + if (this.currentMaterial != null) { + logger.log( + Level.WARNING, + "Error reloading client certificate or key, falling back to cached credentials", + e); + } else { + throw e; + } } catch (Exception e) { if (this.currentMaterial != null) { logger.log( @@ -162,7 +171,13 @@ private static X509Certificate[] parseCertificates(byte[] certBytes) throws Cert } private static PrivateKey parsePrivateKey(byte[] keyBytes) throws Exception { - String keyStr = new String(keyBytes, StandardCharsets.US_ASCII); + String keyStr = new String(keyBytes, StandardCharsets.UTF_8); + if (keyStr.contains("-----BEGIN RSA PRIVATE KEY-----") + || keyStr.contains("-----BEGIN EC PRIVATE KEY-----")) { + throw new IllegalArgumentException( + "PKCS#1 private keys are not supported. Please convert your key to PKCS#8 format using: " + + "openssl pkcs8 -topk8 -nocrypt -in -out "); + } byte[] der; if (keyStr.contains("-----BEGIN PRIVATE KEY-----")) { der = extractPemContent(keyStr, "-----BEGIN PRIVATE KEY-----", "-----END PRIVATE KEY-----"); diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java index 83003b144ce0..8a139e0d0347 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java @@ -1692,46 +1692,51 @@ public void testUseClientCertAndTrustCertificate() throws Exception { io.grpc.netty.shaded.io.netty.handler.ssl.util.SelfSignedCertificate ca = new io.grpc.netty.shaded.io.netty.handler.ssl.util.SelfSignedCertificate("spanner.ca"); - String certPath = ssc.certificate().getAbsolutePath(); - String keyPath = ssc.privateKey().getAbsolutePath(); - String caPath = ca.certificate().getAbsolutePath(); + try { + String certPath = ssc.certificate().getAbsolutePath(); + String keyPath = ssc.privateKey().getAbsolutePath(); + String caPath = ca.certificate().getAbsolutePath(); - SpannerOptions options = - SpannerOptions.newBuilder() - .setProjectId("test-project") - .setCredentials(NoCredentials.getInstance()) - .setHost("https://localhost:1234") - .useClientCert(certPath, keyPath) - .setCaCertificate(caPath) - .build(); + SpannerOptions options = + SpannerOptions.newBuilder() + .setProjectId("test-project") + .setCredentials(NoCredentials.getInstance()) + .setHost("https://localhost:1234") + .useClientCert(certPath, keyPath) + .setCaCertificate(caPath) + .build(); - assertNotNull(options.getChannelConfigurator()); + assertNotNull(options.getChannelConfigurator()); - SpannerOptions fromBuilder = options.toBuilder().build(); - assertNotNull(fromBuilder.getChannelConfigurator()); + SpannerOptions fromBuilder = options.toBuilder().build(); + assertNotNull(fromBuilder.getChannelConfigurator()); - // Test standalone setCaCertificate - SpannerOptions caOnlyOptions = - SpannerOptions.newBuilder() - .setProjectId("test-project") - .setCredentials(NoCredentials.getInstance()) - .setHost("https://localhost:1234") - .setCaCertificate(caPath) - .build(); + // Test standalone setCaCertificate + SpannerOptions caOnlyOptions = + SpannerOptions.newBuilder() + .setProjectId("test-project") + .setCredentials(NoCredentials.getInstance()) + .setHost("https://localhost:1234") + .setCaCertificate(caPath) + .build(); - assertNotNull(caOnlyOptions.getChannelConfigurator()); + assertNotNull(caOnlyOptions.getChannelConfigurator()); - // Test setCaCertificate combined with login (username/password) - SpannerOptions loginWithCaOptions = - SpannerOptions.newBuilder() - .setProjectId("test-project") - .setType(SpannerOptions.InstanceType.OMNI) - .setHost("https://localhost:1234") - .setCaCertificate(caPath) - .login("test-user", "test-pass".toCharArray()) - .build(); + // Test setCaCertificate combined with login (username/password) + SpannerOptions loginWithCaOptions = + SpannerOptions.newBuilder() + .setProjectId("test-project") + .setType(SpannerOptions.InstanceType.OMNI) + .setHost("https://localhost:1234") + .setCaCertificate(caPath) + .login("test-user", "test-pass".toCharArray()) + .build(); - assertTrue(loginWithCaOptions.getCredentials() instanceof SpannerOmniCredentials); - assertNotNull(loginWithCaOptions.getChannelConfigurator()); + assertTrue(loginWithCaOptions.getCredentials() instanceof SpannerOmniCredentials); + assertNotNull(loginWithCaOptions.getChannelConfigurator()); + } finally { + ssc.delete(); + ca.delete(); + } } } diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java index 72eb28e901af..9ab58b3d4b06 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java @@ -16,6 +16,7 @@ package com.google.cloud.spanner.omni; +import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; @@ -41,99 +42,138 @@ public class DynamicKeyManagerTest { @Test public void testInitialLoadAndDynamicRotation() throws Exception { SelfSignedCertificate ssc1 = new SelfSignedCertificate("spanner.test.1"); - File certFile = tempFolder.newFile("client.crt"); - File keyFile = tempFolder.newFile("client.key"); + SelfSignedCertificate ssc2 = new SelfSignedCertificate("spanner.test.2"); + try { + File certFile = tempFolder.newFile("client.crt"); + File keyFile = tempFolder.newFile("client.key"); - Files.write(certFile.toPath(), Files.readAllBytes(ssc1.certificate().toPath())); - Files.write(keyFile.toPath(), Files.readAllBytes(ssc1.privateKey().toPath())); + Files.write(certFile.toPath(), Files.readAllBytes(ssc1.certificate().toPath())); + Files.write(keyFile.toPath(), Files.readAllBytes(ssc1.privateKey().toPath())); - DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile, 0L); + DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile, 0L); - String alias1 = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); - assertNotNull(alias1); - assertEquals(alias1, keyManager.chooseEngineClientAlias(new String[] {"RSA"}, null, null)); + String alias1 = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); + assertNotNull(alias1); + assertEquals(alias1, keyManager.chooseEngineClientAlias(new String[] {"RSA"}, null, null)); - X509Certificate[] chain1 = keyManager.getCertificateChain(alias1); - assertNotNull(chain1); - assertEquals(1, chain1.length); - assertEquals(ssc1.cert().getSubjectDN(), chain1[0].getSubjectDN()); + X509Certificate[] chain1 = keyManager.getCertificateChain(alias1); + assertNotNull(chain1); + assertEquals(1, chain1.length); + assertEquals(ssc1.cert().getSubjectDN(), chain1[0].getSubjectDN()); - PrivateKey pk1 = keyManager.getPrivateKey(alias1); - assertNotNull(pk1); - assertEquals(ssc1.key().getAlgorithm(), pk1.getAlgorithm()); + PrivateKey pk1 = keyManager.getPrivateKey(alias1); + assertNotNull(pk1); + assertEquals(ssc1.key().getAlgorithm(), pk1.getAlgorithm()); - String[] aliases1 = keyManager.getClientAliases("RSA", null); - assertNotNull(aliases1); - assertEquals(1, aliases1.length); - assertEquals(alias1, aliases1[0]); + String[] aliases1 = keyManager.getClientAliases("RSA", null); + assertNotNull(aliases1); + assertEquals(1, aliases1.length); + assertEquals(alias1, aliases1[0]); - // Ensure lastModified timestamp changes upon rotation - Thread.sleep(1100); + // Ensure lastModified timestamp changes upon rotation + Thread.sleep(1100); - SelfSignedCertificate ssc2 = new SelfSignedCertificate("spanner.test.2"); - Files.write(certFile.toPath(), Files.readAllBytes(ssc2.certificate().toPath())); - Files.write(keyFile.toPath(), Files.readAllBytes(ssc2.privateKey().toPath())); + Files.write(certFile.toPath(), Files.readAllBytes(ssc2.certificate().toPath())); + Files.write(keyFile.toPath(), Files.readAllBytes(ssc2.privateKey().toPath())); - String alias2 = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); - assertNotNull(alias2); + String alias2 = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); + assertNotNull(alias2); - X509Certificate[] chain2 = keyManager.getCertificateChain(alias2); - assertNotNull(chain2); - assertEquals(ssc2.cert().getSubjectDN(), chain2[0].getSubjectDN()); + X509Certificate[] chain2 = keyManager.getCertificateChain(alias2); + assertNotNull(chain2); + assertEquals(ssc2.cert().getSubjectDN(), chain2[0].getSubjectDN()); - PrivateKey pk2 = keyManager.getPrivateKey(alias2); - assertNotNull(pk2); + PrivateKey pk2 = keyManager.getPrivateKey(alias2); + assertNotNull(pk2); + } finally { + ssc1.delete(); + ssc2.delete(); + } } @Test public void testFileCheckThrottling() throws Exception { SelfSignedCertificate ssc1 = new SelfSignedCertificate("spanner.test.throttle1"); - File certFile = tempFolder.newFile("client-throttle.crt"); - File keyFile = tempFolder.newFile("client-throttle.key"); - - Files.write(certFile.toPath(), Files.readAllBytes(ssc1.certificate().toPath())); - Files.write(keyFile.toPath(), Files.readAllBytes(ssc1.privateKey().toPath())); - - // 60-second check interval - DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile, 60000L); - - String alias1 = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); - assertEquals( - ssc1.cert().getSubjectDN(), keyManager.getCertificateChain(alias1)[0].getSubjectDN()); - - // Rotate files immediately on disk SelfSignedCertificate ssc2 = new SelfSignedCertificate("spanner.test.throttle2"); - Files.write(certFile.toPath(), Files.readAllBytes(ssc2.certificate().toPath())); - Files.write(keyFile.toPath(), Files.readAllBytes(ssc2.privateKey().toPath())); - - // Within the throttle interval, the manager should retain and return previous certificate - assertEquals( - ssc1.cert().getSubjectDN(), keyManager.getCertificateChain(alias1)[0].getSubjectDN()); + try { + File certFile = tempFolder.newFile("client-throttle.crt"); + File keyFile = tempFolder.newFile("client-throttle.key"); + + Files.write(certFile.toPath(), Files.readAllBytes(ssc1.certificate().toPath())); + Files.write(keyFile.toPath(), Files.readAllBytes(ssc1.privateKey().toPath())); + + // 60-second check interval + DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile, 60000L); + + String alias1 = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); + assertEquals( + ssc1.cert().getSubjectDN(), keyManager.getCertificateChain(alias1)[0].getSubjectDN()); + + // Rotate files immediately on disk + Files.write(certFile.toPath(), Files.readAllBytes(ssc2.certificate().toPath())); + Files.write(keyFile.toPath(), Files.readAllBytes(ssc2.privateKey().toPath())); + + // Within the throttle interval, the manager should retain and return previous certificate + assertEquals( + ssc1.cert().getSubjectDN(), keyManager.getCertificateChain(alias1)[0].getSubjectDN()); + } finally { + ssc1.delete(); + ssc2.delete(); + } } @Test public void testCorruptRotationFallsBackToPrevious() throws Exception { SelfSignedCertificate ssc = new SelfSignedCertificate("spanner.test.fallback"); - File certFile = tempFolder.newFile("client-fallback.crt"); - File keyFile = tempFolder.newFile("client-fallback.key"); - - Files.write(certFile.toPath(), Files.readAllBytes(ssc.certificate().toPath())); - Files.write(keyFile.toPath(), Files.readAllBytes(ssc.privateKey().toPath())); - - DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile, 0L); - String aliasBefore = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); - assertNotNull(aliasBefore); - - Thread.sleep(1100); - - // Overwrite certFile with corrupt bytes - Files.write(certFile.toPath(), "NOT A CERTIFICATE CONTENT".getBytes(StandardCharsets.UTF_8)); + try { + File certFile = tempFolder.newFile("client-fallback.crt"); + File keyFile = tempFolder.newFile("client-fallback.key"); + + Files.write(certFile.toPath(), Files.readAllBytes(ssc.certificate().toPath())); + Files.write(keyFile.toPath(), Files.readAllBytes(ssc.privateKey().toPath())); + + DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile, 0L); + String aliasBefore = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); + assertNotNull(aliasBefore); + + Thread.sleep(1100); + + // Overwrite certFile with corrupt bytes + Files.write(certFile.toPath(), "NOT A CERTIFICATE CONTENT".getBytes(StandardCharsets.UTF_8)); + + // DynamicKeyManager should catch reload error and retain previous material + String aliasAfter = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); + assertEquals(aliasBefore, aliasAfter); + assertNotNull(keyManager.getCertificateChain(aliasAfter)); + assertNotNull(keyManager.getPrivateKey(aliasAfter)); + } finally { + ssc.delete(); + } + } - // DynamicKeyManager should catch reload error and retain previous material - String aliasAfter = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); - assertEquals(aliasBefore, aliasAfter); - assertNotNull(keyManager.getCertificateChain(aliasAfter)); - assertNotNull(keyManager.getPrivateKey(aliasAfter)); + @Test + public void testPkcs1KeyThrowsIllegalArgumentException() throws Exception { + SelfSignedCertificate ssc = new SelfSignedCertificate("spanner.test.pkcs1"); + try { + File certFile = tempFolder.newFile("client-pkcs1.crt"); + File keyFile = tempFolder.newFile("client-pkcs1.key"); + + Files.write(certFile.toPath(), Files.readAllBytes(ssc.certificate().toPath())); + Files.write( + keyFile.toPath(), + ("-----BEGIN RSA PRIVATE KEY-----\n" + + "MIIEowIBAAKCAQEA0Y3...\n" + + "-----END RSA PRIVATE KEY-----\n") + .getBytes(StandardCharsets.UTF_8)); + + IllegalArgumentException exception = + assertThrows( + IllegalArgumentException.class, () -> new DynamicKeyManager(certFile, keyFile)); + assertThat(exception.getMessage()).contains("PKCS#1 private keys are not supported"); + assertThat(exception.getMessage()).contains("openssl pkcs8"); + } finally { + ssc.delete(); + } } @Test @@ -148,15 +188,19 @@ public void testNonExistentFileFailsInitialization() { @Test public void testServerAliasesReturnNull() throws Exception { SelfSignedCertificate ssc = new SelfSignedCertificate("spanner.test.server"); - File certFile = tempFolder.newFile("server-test.crt"); - File keyFile = tempFolder.newFile("server-test.key"); - - Files.write(certFile.toPath(), Files.readAllBytes(ssc.certificate().toPath())); - Files.write(keyFile.toPath(), Files.readAllBytes(ssc.privateKey().toPath())); - - DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile); - assertNull(keyManager.getServerAliases("RSA", null)); - assertNull(keyManager.chooseServerAlias("RSA", null, null)); - assertNull(keyManager.chooseEngineServerAlias("RSA", null, null)); + try { + File certFile = tempFolder.newFile("server-test.crt"); + File keyFile = tempFolder.newFile("server-test.key"); + + Files.write(certFile.toPath(), Files.readAllBytes(ssc.certificate().toPath())); + Files.write(keyFile.toPath(), Files.readAllBytes(ssc.privateKey().toPath())); + + DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile); + assertNull(keyManager.getServerAliases("RSA", null)); + assertNull(keyManager.chooseServerAlias("RSA", null, null)); + assertNull(keyManager.chooseEngineServerAlias("RSA", null, null)); + } finally { + ssc.delete(); + } } } diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicTrustManagerTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicTrustManagerTest.java index 743a74e18d1d..114b6620d1ce 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicTrustManagerTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicTrustManagerTest.java @@ -49,107 +49,124 @@ public void testDefaultTrustManagerWithNull() throws Exception { @Test public void testCustomTrustManagerAndDynamicRotation() throws Exception { SelfSignedCertificate ca1 = new SelfSignedCertificate("spanner.ca.1"); - File caFile = tempFolder.newFile("ca.crt"); - Files.write(caFile.toPath(), Files.readAllBytes(ca1.certificate().toPath())); - - DynamicTrustManager trustManager = new DynamicTrustManager(caFile, 0L); + SelfSignedCertificate ca2 = new SelfSignedCertificate("spanner.ca.2"); + try { + File caFile = tempFolder.newFile("ca.crt"); + Files.write(caFile.toPath(), Files.readAllBytes(ca1.certificate().toPath())); - X509Certificate[] issuers1 = trustManager.getAcceptedIssuers(); - assertNotNull(issuers1); - assertEquals(1, issuers1.length); - assertEquals(ca1.cert().getSubjectDN(), issuers1[0].getSubjectDN()); + DynamicTrustManager trustManager = new DynamicTrustManager(caFile, 0L); - // Validating ca1 cert should succeed - trustManager.checkServerTrusted(new X509Certificate[] {ca1.cert()}, "RSA"); + X509Certificate[] issuers1 = trustManager.getAcceptedIssuers(); + assertNotNull(issuers1); + assertEquals(1, issuers1.length); + assertEquals(ca1.cert().getSubjectDN(), issuers1[0].getSubjectDN()); - SelfSignedCertificate ca2 = new SelfSignedCertificate("spanner.ca.2"); + // Validating ca1 cert should succeed + trustManager.checkServerTrusted(new X509Certificate[] {ca1.cert()}, "RSA"); - // Validating ca2 cert with ca1 trusted should fail - assertThrows( - CertificateException.class, - () -> trustManager.checkServerTrusted(new X509Certificate[] {ca2.cert()}, "RSA")); + // Validating ca2 cert with ca1 trusted should fail + assertThrows( + CertificateException.class, + () -> trustManager.checkServerTrusted(new X509Certificate[] {ca2.cert()}, "RSA")); - Thread.sleep(1100); + Thread.sleep(1100); - // Rotate CA file on disk to ca2 - Files.write(caFile.toPath(), Files.readAllBytes(ca2.certificate().toPath())); + // Rotate CA file on disk to ca2 + Files.write(caFile.toPath(), Files.readAllBytes(ca2.certificate().toPath())); - // Now ca2 should be accepted and ca1 should be rejected - X509Certificate[] issuers2 = trustManager.getAcceptedIssuers(); - assertNotNull(issuers2); - assertEquals(1, issuers2.length); - assertEquals(ca2.cert().getSubjectDN(), issuers2[0].getSubjectDN()); + // Now ca2 should be accepted and ca1 should be rejected + X509Certificate[] issuers2 = trustManager.getAcceptedIssuers(); + assertNotNull(issuers2); + assertEquals(1, issuers2.length); + assertEquals(ca2.cert().getSubjectDN(), issuers2[0].getSubjectDN()); - trustManager.checkServerTrusted(new X509Certificate[] {ca2.cert()}, "RSA"); + trustManager.checkServerTrusted(new X509Certificate[] {ca2.cert()}, "RSA"); - assertThrows( - CertificateException.class, - () -> trustManager.checkServerTrusted(new X509Certificate[] {ca1.cert()}, "RSA")); + assertThrows( + CertificateException.class, + () -> trustManager.checkServerTrusted(new X509Certificate[] {ca1.cert()}, "RSA")); + } finally { + ca1.delete(); + ca2.delete(); + } } @Test public void testFileCheckThrottling() throws Exception { SelfSignedCertificate ca1 = new SelfSignedCertificate("spanner.ca.throttle1"); - File caFile = tempFolder.newFile("ca-throttle.crt"); - Files.write(caFile.toPath(), Files.readAllBytes(ca1.certificate().toPath())); - - // 60-second check interval - DynamicTrustManager trustManager = new DynamicTrustManager(caFile, 60000L); - - X509Certificate[] issuers1 = trustManager.getAcceptedIssuers(); - assertEquals(1, issuers1.length); - assertEquals(ca1.cert().getSubjectDN(), issuers1[0].getSubjectDN()); - - // Rotate CA on disk immediately SelfSignedCertificate ca2 = new SelfSignedCertificate("spanner.ca.throttle2"); - Files.write(caFile.toPath(), Files.readAllBytes(ca2.certificate().toPath())); - - // Within throttle interval, trust manager should retain previous CA - assertEquals(ca1.cert().getSubjectDN(), trustManager.getAcceptedIssuers()[0].getSubjectDN()); - trustManager.checkServerTrusted(new X509Certificate[] {ca1.cert()}, "RSA"); + try { + File caFile = tempFolder.newFile("ca-throttle.crt"); + Files.write(caFile.toPath(), Files.readAllBytes(ca1.certificate().toPath())); + + // 60-second check interval + DynamicTrustManager trustManager = new DynamicTrustManager(caFile, 60000L); + + X509Certificate[] issuers1 = trustManager.getAcceptedIssuers(); + assertEquals(1, issuers1.length); + assertEquals(ca1.cert().getSubjectDN(), issuers1[0].getSubjectDN()); + + // Rotate CA on disk immediately + Files.write(caFile.toPath(), Files.readAllBytes(ca2.certificate().toPath())); + + // Within throttle interval, trust manager should retain previous CA + assertEquals(ca1.cert().getSubjectDN(), trustManager.getAcceptedIssuers()[0].getSubjectDN()); + trustManager.checkServerTrusted(new X509Certificate[] {ca1.cert()}, "RSA"); + } finally { + ca1.delete(); + ca2.delete(); + } } @Test public void testMultipleCAsInFile() throws Exception { SelfSignedCertificate ca1 = new SelfSignedCertificate("spanner.multi.ca.1"); SelfSignedCertificate ca2 = new SelfSignedCertificate("spanner.multi.ca.2"); - - File caFile = tempFolder.newFile("multi-ca.crt"); - byte[] bundle = - (new String(Files.readAllBytes(ca1.certificate().toPath()), StandardCharsets.UTF_8) - + "\n" - + new String( - Files.readAllBytes(ca2.certificate().toPath()), StandardCharsets.UTF_8)) - .getBytes(StandardCharsets.UTF_8); - Files.write(caFile.toPath(), bundle); - - DynamicTrustManager trustManager = new DynamicTrustManager(caFile); - - X509Certificate[] issuers = trustManager.getAcceptedIssuers(); - assertNotNull(issuers); - assertEquals(2, issuers.length); - - trustManager.checkServerTrusted(new X509Certificate[] {ca1.cert()}, "RSA"); - trustManager.checkServerTrusted(new X509Certificate[] {ca2.cert()}, "RSA"); + try { + File caFile = tempFolder.newFile("multi-ca.crt"); + byte[] bundle = + (new String(Files.readAllBytes(ca1.certificate().toPath()), StandardCharsets.UTF_8) + + "\n" + + new String( + Files.readAllBytes(ca2.certificate().toPath()), StandardCharsets.UTF_8)) + .getBytes(StandardCharsets.UTF_8); + Files.write(caFile.toPath(), bundle); + + DynamicTrustManager trustManager = new DynamicTrustManager(caFile); + + X509Certificate[] issuers = trustManager.getAcceptedIssuers(); + assertNotNull(issuers); + assertEquals(2, issuers.length); + + trustManager.checkServerTrusted(new X509Certificate[] {ca1.cert()}, "RSA"); + trustManager.checkServerTrusted(new X509Certificate[] {ca2.cert()}, "RSA"); + } finally { + ca1.delete(); + ca2.delete(); + } } @Test public void testCorruptRotationFallsBackToPrevious() throws Exception { SelfSignedCertificate ca = new SelfSignedCertificate("spanner.ca.fallback"); - File caFile = tempFolder.newFile("ca-fallback.crt"); - Files.write(caFile.toPath(), Files.readAllBytes(ca.certificate().toPath())); + try { + File caFile = tempFolder.newFile("ca-fallback.crt"); + Files.write(caFile.toPath(), Files.readAllBytes(ca.certificate().toPath())); - DynamicTrustManager trustManager = new DynamicTrustManager(caFile, 0L); - trustManager.checkServerTrusted(new X509Certificate[] {ca.cert()}, "RSA"); + DynamicTrustManager trustManager = new DynamicTrustManager(caFile, 0L); + trustManager.checkServerTrusted(new X509Certificate[] {ca.cert()}, "RSA"); - Thread.sleep(1100); + Thread.sleep(1100); - // Corrupt the file - Files.write(caFile.toPath(), "CORRUPT CERT DATA".getBytes(StandardCharsets.UTF_8)); + // Corrupt the file + Files.write(caFile.toPath(), "CORRUPT CERT DATA".getBytes(StandardCharsets.UTF_8)); - // Trust manager should retain previous CA - trustManager.checkServerTrusted(new X509Certificate[] {ca.cert()}, "RSA"); - assertEquals(1, trustManager.getAcceptedIssuers().length); + // Trust manager should retain previous CA + trustManager.checkServerTrusted(new X509Certificate[] {ca.cert()}, "RSA"); + assertEquals(1, trustManager.getAcceptedIssuers().length); + } finally { + ca.delete(); + } } @Test From a077de2bebb5cd28e98f8c7e9972b02a53b2928b Mon Sep 17 00:00:00 2001 From: sagnghos Date: Mon, 21 Sep 2026 08:40:08 +0000 Subject: [PATCH 04/22] Address review comments: avoid capturing builder in SpannerOptions lambda and use monotonic nanoTime for interval throttling --- .../com/google/cloud/spanner/SpannerOptions.java | 10 +++++++--- .../cloud/spanner/omni/DynamicKeyManager.java | 14 +++++++------- .../cloud/spanner/omni/DynamicTrustManager.java | 14 +++++++------- 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index ee7b193e2513..f6835f9874dd 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -945,13 +945,17 @@ protected SpannerOptions(Builder builder) { channelProvider = builder.channelProvider; channelEndpointCacheFactory = builder.channelEndpointCacheFactory; if (builder.omniSslContext != null) { + final SslContext sslContext = builder.omniSslContext; + @SuppressWarnings("rawtypes") + final ApiFunction parentConfigurator = + builder.channelConfigurator; channelConfigurator = channelBuilder -> { - if (builder.channelConfigurator != null) { - channelBuilder = builder.channelConfigurator.apply(channelBuilder); + if (parentConfigurator != null) { + channelBuilder = parentConfigurator.apply(channelBuilder); } if (channelBuilder instanceof NettyChannelBuilder) { - ((NettyChannelBuilder) channelBuilder).sslContext(builder.omniSslContext); + ((NettyChannelBuilder) channelBuilder).sslContext(sslContext); } return channelBuilder; }; diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java index 7a8c997051b7..f0fdbabaafcb 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java @@ -50,8 +50,8 @@ public class DynamicKeyManager extends X509ExtendedKeyManager { private final File certFile; private final File keyFile; - private final long checkIntervalMs; - private volatile long lastCheckedMs; + private final long checkIntervalNs; + private volatile long lastCheckedNs; private static class KeyMaterial { final long certLastModified; @@ -86,17 +86,17 @@ public DynamicKeyManager(File certFile, File keyFile) { DynamicKeyManager(File certFile, File keyFile, long checkIntervalMs) { this.certFile = Preconditions.checkNotNull(certFile, "certFile cannot be null"); this.keyFile = Preconditions.checkNotNull(keyFile, "keyFile cannot be null"); - this.checkIntervalMs = checkIntervalMs; + this.checkIntervalNs = checkIntervalMs * 1_000_000L; reloadMaterial(); - this.lastCheckedMs = System.currentTimeMillis(); + this.lastCheckedNs = System.nanoTime(); } private void checkAndReload() { - long now = System.currentTimeMillis(); - if (now - lastCheckedMs < checkIntervalMs) { + long now = System.nanoTime(); + if (now - lastCheckedNs < checkIntervalNs) { return; } - lastCheckedMs = now; + lastCheckedNs = now; KeyMaterial existing = this.currentMaterial; if (existing != null && certFile.lastModified() == existing.certLastModified diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java index 5c0134eccb66..7071fdb65b18 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java @@ -47,8 +47,8 @@ public class DynamicTrustManager extends X509ExtendedTrustManager { private static final long DEFAULT_CHECK_INTERVAL_MS = 5000L; private final File caCertFile; - private final long checkIntervalMs; - private volatile long lastCheckedMs; + private final long checkIntervalNs; + private volatile long lastCheckedNs; private static class TrustMaterial { final long lastModified; @@ -70,20 +70,20 @@ public DynamicTrustManager(@Nullable File caCertFile) { DynamicTrustManager(@Nullable File caCertFile, long checkIntervalMs) { this.caCertFile = caCertFile; - this.checkIntervalMs = checkIntervalMs; + this.checkIntervalNs = checkIntervalMs * 1_000_000L; reloadMaterial(); - this.lastCheckedMs = System.currentTimeMillis(); + this.lastCheckedNs = System.nanoTime(); } private void checkAndReload() { if (this.caCertFile == null) { return; } - long now = System.currentTimeMillis(); - if (now - lastCheckedMs < checkIntervalMs) { + long now = System.nanoTime(); + if (now - lastCheckedNs < checkIntervalNs) { return; } - lastCheckedMs = now; + lastCheckedNs = now; TrustMaterial existing = this.currentMaterial; if (existing != null && caCertFile.lastModified() == existing.lastModified From 959f887a8c032bc38c95b5bce0434de05d44439a Mon Sep 17 00:00:00 2001 From: sagnghos Date: Mon, 21 Sep 2026 09:15:54 +0000 Subject: [PATCH 05/22] Address review comments: simplify reloadMaterial and suppress exceptions on private key parsing --- .../cloud/spanner/omni/DynamicKeyManager.java | 61 ++++++++-------- .../spanner/omni/DynamicTrustManager.java | 71 +++++++++---------- 2 files changed, 60 insertions(+), 72 deletions(-) diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java index f0fdbabaafcb..0bd76c8d4ab9 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java @@ -23,6 +23,7 @@ import java.net.Socket; import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.security.GeneralSecurityException; import java.security.KeyFactory; import java.security.Principal; import java.security.PrivateKey; @@ -87,7 +88,13 @@ public DynamicKeyManager(File certFile, File keyFile) { this.certFile = Preconditions.checkNotNull(certFile, "certFile cannot be null"); this.keyFile = Preconditions.checkNotNull(keyFile, "keyFile cannot be null"); this.checkIntervalNs = checkIntervalMs * 1_000_000L; - reloadMaterial(); + try { + reloadMaterial(); + } catch (IllegalArgumentException e) { + throw e; + } catch (Exception e) { + throw new RuntimeException("Failed to initialize client certificate/key", e); + } this.lastCheckedNs = System.nanoTime(); } @@ -125,39 +132,19 @@ private void checkAndReload() { } } - private void reloadMaterial() { - try { - long certMod = certFile.lastModified(); - long certLen = certFile.length(); - long keyMod = keyFile.lastModified(); - long keyLen = keyFile.length(); + private void reloadMaterial() throws Exception { + long certMod = certFile.lastModified(); + long certLen = certFile.length(); + long keyMod = keyFile.lastModified(); + long keyLen = keyFile.length(); - byte[] certBytes = Files.readAllBytes(certFile.toPath()); - byte[] keyBytes = Files.readAllBytes(keyFile.toPath()); + byte[] certBytes = Files.readAllBytes(certFile.toPath()); + byte[] keyBytes = Files.readAllBytes(keyFile.toPath()); - X509Certificate[] chain = parseCertificates(certBytes); - PrivateKey key = parsePrivateKey(keyBytes); + X509Certificate[] chain = parseCertificates(certBytes); + PrivateKey key = parsePrivateKey(keyBytes); - this.currentMaterial = new KeyMaterial(certMod, certLen, keyMod, keyLen, chain, key); - } catch (IllegalArgumentException e) { - if (this.currentMaterial != null) { - logger.log( - Level.WARNING, - "Error reloading client certificate or key, falling back to cached credentials", - e); - } else { - throw e; - } - } catch (Exception e) { - if (this.currentMaterial != null) { - logger.log( - Level.WARNING, - "Error reloading client certificate or key, falling back to cached credentials", - e); - } else { - throw new RuntimeException("Failed to initialize client certificate/key", e); - } - } + this.currentMaterial = new KeyMaterial(certMod, certLen, keyMod, keyLen, chain, key); } private static X509Certificate[] parseCertificates(byte[] certBytes) throws CertificateException { @@ -192,8 +179,16 @@ private static PrivateKey parsePrivateKey(byte[] keyBytes) throws Exception { PKCS8EncodedKeySpec spec = new PKCS8EncodedKeySpec(der); try { return KeyFactory.getInstance("RSA").generatePrivate(spec); - } catch (Exception e) { - return KeyFactory.getInstance("EC").generatePrivate(spec); + } catch (Exception rsaException) { + try { + return KeyFactory.getInstance("EC").generatePrivate(spec); + } catch (Exception ecException) { + GeneralSecurityException ex = + new GeneralSecurityException("Failed to parse private key as RSA or EC"); + ex.addSuppressed(rsaException); + ex.addSuppressed(ecException); + throw ex; + } } } diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java index 7071fdb65b18..f03b42437fb0 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java @@ -71,7 +71,11 @@ public DynamicTrustManager(@Nullable File caCertFile) { DynamicTrustManager(@Nullable File caCertFile, long checkIntervalMs) { this.caCertFile = caCertFile; this.checkIntervalNs = checkIntervalMs * 1_000_000L; - reloadMaterial(); + try { + reloadMaterial(); + } catch (Exception e) { + throw new RuntimeException("Failed to initialize CA certificate", e); + } this.lastCheckedNs = System.nanoTime(); } @@ -108,49 +112,38 @@ private void checkAndReload() { } } - private void reloadMaterial() { - try { - if (this.caCertFile == null) { - TrustManagerFactory tmf = - TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()); - tmf.init((KeyStore) null); - this.currentMaterial = new TrustMaterial(0, 0, findExtendedTrustManager(tmf)); - return; - } + private void reloadMaterial() throws Exception { + if (this.caCertFile == null) { + TrustManagerFactory tmf = + TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()); + tmf.init((KeyStore) null); + this.currentMaterial = new TrustMaterial(0, 0, findExtendedTrustManager(tmf)); + return; + } - long mod = caCertFile.lastModified(); - long len = caCertFile.length(); - byte[] certBytes = Files.readAllBytes(caCertFile.toPath()); + long mod = caCertFile.lastModified(); + long len = caCertFile.length(); + byte[] certBytes = Files.readAllBytes(caCertFile.toPath()); - CertificateFactory cf = CertificateFactory.getInstance("X.509"); - Collection certs = - cf.generateCertificates(new ByteArrayInputStream(certBytes)); - if (certs == null || certs.isEmpty()) { - throw new CertificateException("No certificates found in CA certificate file"); - } + CertificateFactory cf = CertificateFactory.getInstance("X.509"); + Collection certs = + cf.generateCertificates(new ByteArrayInputStream(certBytes)); + if (certs == null || certs.isEmpty()) { + throw new CertificateException("No certificates found in CA certificate file"); + } - KeyStore ks = KeyStore.getInstance(KeyStore.getDefaultType()); - ks.load(null, null); - int index = 0; - for (Certificate cert : certs) { - ks.setCertificateEntry("spanner-ca-" + (++index), cert); - } + KeyStore ks = KeyStore.getInstance(KeyStore.getDefaultType()); + ks.load(null, null); + int index = 0; + for (Certificate cert : certs) { + ks.setCertificateEntry("spanner-ca-" + (++index), cert); + } - TrustManagerFactory tmf = - TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()); - tmf.init(ks); + TrustManagerFactory tmf = + TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()); + tmf.init(ks); - this.currentMaterial = new TrustMaterial(mod, len, findExtendedTrustManager(tmf)); - } catch (Exception e) { - if (this.currentMaterial != null) { - logger.log( - Level.WARNING, - "Error reloading CA certificate, falling back to cached trust manager", - e); - } else { - throw new RuntimeException("Failed to initialize CA certificate", e); - } - } + this.currentMaterial = new TrustMaterial(mod, len, findExtendedTrustManager(tmf)); } private static X509ExtendedTrustManager findExtendedTrustManager(TrustManagerFactory tmf) From 7fcbedf4e44979656443d62dd2938079e41a4d8f Mon Sep 17 00:00:00 2001 From: sagnghos Date: Mon, 21 Sep 2026 09:25:30 +0000 Subject: [PATCH 06/22] docs(spanner): Add Javadocs for ConnectionOptions builder and DynamicKey/TrustManager constructors --- .../spanner/connection/ConnectionOptions.java | 21 +++++++++++++++++++ .../cloud/spanner/omni/DynamicKeyManager.java | 7 +++++++ .../spanner/omni/DynamicTrustManager.java | 8 +++++++ .../cloud/spanner/SpannerOptionsTest.java | 7 +++---- 4 files changed, 39 insertions(+), 4 deletions(-) diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java index 1237e88170eb..90bfc6b24155 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java @@ -681,16 +681,37 @@ public Builder setType(SpannerOptions.InstanceType instanceType) { return this; } + /** + * Sets the path to the client certificate file to use for mTLS authentication with Spanner + * Omni. + * + * @param clientCertificate The path to the client certificate file. + * @return this builder + */ public Builder setClientCertificate(String clientCertificate) { setConnectionPropertyValue(CLIENT_CERTIFICATE, clientCertificate); return this; } + /** + * Sets the path to the client private key file to use for mTLS authentication with Spanner + * Omni. + * + * @param clientCertificateKey The path to the client private key file. + * @return this builder + */ public Builder setClientCertificateKey(String clientCertificateKey) { setConnectionPropertyValue(CLIENT_KEY, clientCertificateKey); return this; } + /** + * Sets the path to the server root CA certificate file to use for SSL/TLS verification with + * Spanner Omni. + * + * @param caCertificate The path to the root CA certificate file. + * @return this builder + */ public Builder setCaCertificate(String caCertificate) { setConnectionPropertyValue(CA_CERTIFICATE, caCertificate); return this; diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java index 0bd76c8d4ab9..2d723a55e9ab 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java @@ -80,6 +80,13 @@ private static class KeyMaterial { private volatile KeyMaterial currentMaterial; + /** + * Creates a {@link DynamicKeyManager} that dynamically reloads the given certificate and key + * files when modified on disk. + * + * @param certFile File containing the X.509 client certificate chain. + * @param keyFile File containing the PKCS#8 private key. + */ public DynamicKeyManager(File certFile, File keyFile) { this(certFile, keyFile, DEFAULT_CHECK_INTERVAL_MS); } diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java index f03b42437fb0..8291693c377b 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java @@ -64,6 +64,14 @@ private static class TrustMaterial { private volatile TrustMaterial currentMaterial; + /** + * Creates a {@link DynamicTrustManager} that dynamically reloads the given root CA certificate + * file when modified on disk, or delegates to the default JVM trust store if {@code caCertFile} + * is null. + * + * @param caCertFile File containing the X.509 CA certificate(s), or null for the default JVM + * trust store. + */ public DynamicTrustManager(@Nullable File caCertFile) { this(caCertFile, DEFAULT_CHECK_INTERVAL_MS); } diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java index 8a139e0d0347..209f41d1d0e3 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java @@ -71,6 +71,7 @@ import com.google.spanner.v1.SpannerGrpc; import com.google.spanner.v1.TransactionOptions.IsolationLevel; import io.grpc.MethodDescriptor; +import io.grpc.netty.shaded.io.netty.handler.ssl.util.SelfSignedCertificate; import io.opentelemetry.api.GlobalOpenTelemetry; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.sdk.OpenTelemetrySdk; @@ -1687,10 +1688,8 @@ public ApiCallContext configure( @Test public void testUseClientCertAndTrustCertificate() throws Exception { - io.grpc.netty.shaded.io.netty.handler.ssl.util.SelfSignedCertificate ssc = - new io.grpc.netty.shaded.io.netty.handler.ssl.util.SelfSignedCertificate("spanner.test"); - io.grpc.netty.shaded.io.netty.handler.ssl.util.SelfSignedCertificate ca = - new io.grpc.netty.shaded.io.netty.handler.ssl.util.SelfSignedCertificate("spanner.ca"); + SelfSignedCertificate ssc = new SelfSignedCertificate("spanner.test"); + SelfSignedCertificate ca = new SelfSignedCertificate("spanner.ca"); try { String certPath = ssc.certificate().getAbsolutePath(); From 6993ccf53e7cb2fa4421b553f39014be569a4c84 Mon Sep 17 00:00:00 2001 From: sagnghos Date: Mon, 21 Sep 2026 09:29:32 +0000 Subject: [PATCH 07/22] fix(spanner): Validate private key matches certificate and optimize file stat calls --- .../cloud/spanner/omni/DynamicKeyManager.java | 33 ++++++++++-- .../spanner/omni/DynamicTrustManager.java | 4 +- .../spanner/omni/DynamicKeyManagerTest.java | 53 +++++++++++++++++++ 3 files changed, 83 insertions(+), 7 deletions(-) diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java index 2d723a55e9ab..0dc454b22297 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java @@ -27,6 +27,8 @@ import java.security.KeyFactory; import java.security.Principal; import java.security.PrivateKey; +import java.security.PublicKey; +import java.security.Signature; import java.security.cert.Certificate; import java.security.cert.CertificateException; import java.security.cert.CertificateFactory; @@ -140,20 +142,41 @@ private void checkAndReload() { } private void reloadMaterial() throws Exception { - long certMod = certFile.lastModified(); - long certLen = certFile.length(); - long keyMod = keyFile.lastModified(); - long keyLen = keyFile.length(); - byte[] certBytes = Files.readAllBytes(certFile.toPath()); byte[] keyBytes = Files.readAllBytes(keyFile.toPath()); + long certMod = certFile.lastModified(); + long certLen = certBytes.length; + long keyMod = keyFile.lastModified(); + long keyLen = keyBytes.length; + X509Certificate[] chain = parseCertificates(certBytes); PrivateKey key = parsePrivateKey(keyBytes); + verifyKeyMatch(chain[0].getPublicKey(), key); this.currentMaterial = new KeyMaterial(certMod, certLen, keyMod, keyLen, chain, key); } + private static void verifyKeyMatch(PublicKey publicKey, PrivateKey privateKey) + throws GeneralSecurityException { + String algorithm = privateKey.getAlgorithm(); + String sigAlg = + "RSA".equalsIgnoreCase(algorithm) + ? "SHA256withRSA" + : "EC".equalsIgnoreCase(algorithm) ? "SHA256withECDSA" : null; + if (sigAlg != null) { + Signature sig = Signature.getInstance(sigAlg); + sig.initSign(privateKey); + sig.update(new byte[0]); + byte[] signature = sig.sign(); + sig.initVerify(publicKey); + sig.update(new byte[0]); + if (!sig.verify(signature)) { + throw new GeneralSecurityException("Private key does not match the certificate public key"); + } + } + } + private static X509Certificate[] parseCertificates(byte[] certBytes) throws CertificateException { CertificateFactory cf = CertificateFactory.getInstance("X.509"); Collection certs = diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java index 8291693c377b..cd07b761dc42 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java @@ -129,9 +129,9 @@ private void reloadMaterial() throws Exception { return; } - long mod = caCertFile.lastModified(); - long len = caCertFile.length(); byte[] certBytes = Files.readAllBytes(caCertFile.toPath()); + long mod = caCertFile.lastModified(); + long len = certBytes.length; CertificateFactory cf = CertificateFactory.getInstance("X.509"); Collection certs = diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java index 9ab58b3d4b06..a0dade6de818 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java @@ -203,4 +203,57 @@ public void testServerAliasesReturnNull() throws Exception { ssc.delete(); } } + + @Test + public void testMismatchedCertificateAndKeyFailsInitialization() throws Exception { + SelfSignedCertificate ssc1 = new SelfSignedCertificate("spanner.test.cert1"); + SelfSignedCertificate ssc2 = new SelfSignedCertificate("spanner.test.cert2"); + try { + File certFile = tempFolder.newFile("mismatched-init.crt"); + File keyFile = tempFolder.newFile("mismatched-init.key"); + + // Pair cert from ssc1 with key from ssc2 + Files.write(certFile.toPath(), Files.readAllBytes(ssc1.certificate().toPath())); + Files.write(keyFile.toPath(), Files.readAllBytes(ssc2.privateKey().toPath())); + + RuntimeException exception = + assertThrows(RuntimeException.class, () -> new DynamicKeyManager(certFile, keyFile)); + assertThat(exception.getCause().getMessage()) + .contains("Private key does not match the certificate public key"); + } finally { + ssc1.delete(); + ssc2.delete(); + } + } + + @Test + public void testMismatchedRotationFallsBackToPrevious() throws Exception { + SelfSignedCertificate ssc1 = new SelfSignedCertificate("spanner.test.match1"); + SelfSignedCertificate ssc2 = new SelfSignedCertificate("spanner.test.match2"); + try { + File certFile = tempFolder.newFile("mismatched-rotate.crt"); + File keyFile = tempFolder.newFile("mismatched-rotate.key"); + + Files.write(certFile.toPath(), Files.readAllBytes(ssc1.certificate().toPath())); + Files.write(keyFile.toPath(), Files.readAllBytes(ssc1.privateKey().toPath())); + + DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile, 0L); + String alias1 = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); + assertEquals( + ssc1.cert().getSubjectDN(), keyManager.getCertificateChain(alias1)[0].getSubjectDN()); + + Thread.sleep(1100); + + // Rotate only cert file (e.g., intermediate state during rotation) + Files.write(certFile.toPath(), Files.readAllBytes(ssc2.certificate().toPath())); + + // Key manager should detect mismatch and retain ssc1 credentials + String aliasAfter = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); + assertEquals( + ssc1.cert().getSubjectDN(), keyManager.getCertificateChain(aliasAfter)[0].getSubjectDN()); + } finally { + ssc1.delete(); + ssc2.delete(); + } + } } From ab27a983e86e331b5433c6e313bb94038a5a3daf Mon Sep 17 00:00:00 2001 From: sagnghos Date: Mon, 21 Sep 2026 10:22:40 +0000 Subject: [PATCH 08/22] fix(spanner): Address review comments: versioned alias map, binary DER support, file stat ordering, and SpannerOptions SSL context preservation --- .../google/cloud/spanner/SpannerOptions.java | 3 ++ .../cloud/spanner/omni/DynamicKeyManager.java | 53 ++++++++++++++----- .../spanner/omni/DynamicTrustManager.java | 4 +- .../cloud/spanner/SpannerOptionsTest.java | 4 ++ .../spanner/omni/DynamicKeyManagerTest.java | 20 +++++++ 5 files changed, 68 insertions(+), 16 deletions(-) diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index f6835f9874dd..caa8f19c2357 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -359,6 +359,7 @@ static GcpChannelPoolOptions mergeWithDefaultChannelPoolOptions( private final boolean autoTaggingEnabled; private final List autoTaggingPackages; private final int autoTaggingTracerLimit; + private final SslContext omniSslContext; enum TracingFramework { OPEN_CENSUS, @@ -944,6 +945,7 @@ protected SpannerOptions(Builder builder) { transportChannelExecutorThreadNameFormat = builder.transportChannelExecutorThreadNameFormat; channelProvider = builder.channelProvider; channelEndpointCacheFactory = builder.channelEndpointCacheFactory; + omniSslContext = builder.omniSslContext; if (builder.omniSslContext != null) { final SslContext sslContext = builder.omniSslContext; @SuppressWarnings("rawtypes") @@ -1532,6 +1534,7 @@ protected Builder() { this.autoTaggingEnabled = options.autoTaggingEnabled; this.autoTaggingPackages = options.autoTaggingPackages; this.autoTaggingTracerLimit = options.autoTaggingTracerLimit; + this.omniSslContext = options.omniSslContext; } @Override diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java index 0dc454b22297..76306eff7410 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java @@ -36,6 +36,8 @@ import java.security.spec.PKCS8EncodedKeySpec; import java.util.Base64; import java.util.Collection; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicLong; import java.util.logging.Level; import java.util.logging.Logger; import javax.net.ssl.SSLEngine; @@ -48,15 +50,17 @@ @InternalApi public class DynamicKeyManager extends X509ExtendedKeyManager { private static final Logger logger = Logger.getLogger(DynamicKeyManager.class.getName()); - private static final String CLIENT_ALIAS = "client"; private static final long DEFAULT_CHECK_INTERVAL_MS = 5000L; private final File certFile; private final File keyFile; private final long checkIntervalNs; + private final ConcurrentHashMap materials = new ConcurrentHashMap<>(); + private final AtomicLong versionCounter = new AtomicLong(); private volatile long lastCheckedNs; private static class KeyMaterial { + final String alias; final long certLastModified; final long certLength; final long keyLastModified; @@ -65,12 +69,14 @@ private static class KeyMaterial { final PrivateKey privateKey; KeyMaterial( + String alias, long certLastModified, long certLength, long keyLastModified, long keyLength, X509Certificate[] certificateChain, PrivateKey privateKey) { + this.alias = alias; this.certLastModified = certLastModified; this.certLength = certLength; this.keyLastModified = keyLastModified; @@ -142,19 +148,29 @@ private void checkAndReload() { } private void reloadMaterial() throws Exception { - byte[] certBytes = Files.readAllBytes(certFile.toPath()); - byte[] keyBytes = Files.readAllBytes(keyFile.toPath()); - long certMod = certFile.lastModified(); - long certLen = certBytes.length; + long certLen = certFile.length(); long keyMod = keyFile.lastModified(); - long keyLen = keyBytes.length; + long keyLen = keyFile.length(); + + byte[] certBytes = Files.readAllBytes(certFile.toPath()); + byte[] keyBytes = Files.readAllBytes(keyFile.toPath()); X509Certificate[] chain = parseCertificates(certBytes); PrivateKey key = parsePrivateKey(keyBytes); verifyKeyMatch(chain[0].getPublicKey(), key); - this.currentMaterial = new KeyMaterial(certMod, certLen, keyMod, keyLen, chain, key); + String alias = "client-" + versionCounter.incrementAndGet(); + KeyMaterial newMaterial = new KeyMaterial(alias, certMod, certLen, keyMod, keyLen, chain, key); + materials.put(alias, newMaterial); + this.currentMaterial = newMaterial; + if (materials.size() > 10) { + for (String oldAlias : materials.keySet()) { + if (!oldAlias.equals(alias) && materials.size() > 10) { + materials.remove(oldAlias); + } + } + } } private static void verifyKeyMatch(PublicKey publicKey, PrivateKey privateKey) @@ -198,6 +214,8 @@ private static PrivateKey parsePrivateKey(byte[] keyBytes) throws Exception { byte[] der; if (keyStr.contains("-----BEGIN PRIVATE KEY-----")) { der = extractPemContent(keyStr, "-----BEGIN PRIVATE KEY-----", "-----END PRIVATE KEY-----"); + } else if (keyBytes.length > 0 && keyBytes[0] == 0x30) { + der = keyBytes; } else { try { der = Base64.getMimeDecoder().decode(keyBytes); @@ -239,33 +257,40 @@ private static byte[] extractPemContent(String pem, String beginMarker, String e @Override public String chooseClientAlias(String[] keyType, Principal[] issuers, Socket socket) { checkAndReload(); - return CLIENT_ALIAS; + KeyMaterial mat = this.currentMaterial; + return mat != null ? mat.alias : null; } @Override public String chooseEngineClientAlias(String[] keyType, Principal[] issuers, SSLEngine engine) { checkAndReload(); - return CLIENT_ALIAS; + KeyMaterial mat = this.currentMaterial; + return mat != null ? mat.alias : null; } @Override public X509Certificate[] getCertificateChain(String alias) { - checkAndReload(); - KeyMaterial mat = this.currentMaterial; + KeyMaterial mat = alias != null ? materials.get(alias) : null; + if (mat == null) { + mat = this.currentMaterial; + } return mat != null ? mat.certificateChain.clone() : null; } @Override public PrivateKey getPrivateKey(String alias) { - checkAndReload(); - KeyMaterial mat = this.currentMaterial; + KeyMaterial mat = alias != null ? materials.get(alias) : null; + if (mat == null) { + mat = this.currentMaterial; + } return mat != null ? mat.privateKey : null; } @Override public String[] getClientAliases(String keyType, Principal[] issuers) { checkAndReload(); - return new String[] {CLIENT_ALIAS}; + KeyMaterial mat = this.currentMaterial; + return mat != null ? new String[] {mat.alias} : null; } @Override diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java index cd07b761dc42..8291693c377b 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java @@ -129,9 +129,9 @@ private void reloadMaterial() throws Exception { return; } - byte[] certBytes = Files.readAllBytes(caCertFile.toPath()); long mod = caCertFile.lastModified(); - long len = certBytes.length; + long len = caCertFile.length(); + byte[] certBytes = Files.readAllBytes(caCertFile.toPath()); CertificateFactory cf = CertificateFactory.getInstance("X.509"); Collection certs = diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java index 209f41d1d0e3..f484b7e2e3b6 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java @@ -1733,6 +1733,10 @@ public void testUseClientCertAndTrustCertificate() throws Exception { assertTrue(loginWithCaOptions.getCredentials() instanceof SpannerOmniCredentials); assertNotNull(loginWithCaOptions.getChannelConfigurator()); + + SpannerOptions loginFromBuilder = loginWithCaOptions.toBuilder().build(); + assertTrue(loginFromBuilder.getCredentials() instanceof SpannerOmniCredentials); + assertNotNull(loginFromBuilder.getChannelConfigurator()); } finally { ssc.delete(); ca.delete(); diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java index a0dade6de818..d46c1b497325 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java @@ -256,4 +256,24 @@ public void testMismatchedRotationFallsBackToPrevious() throws Exception { ssc2.delete(); } } + + @Test + public void testBinaryDerKeySupported() throws Exception { + SelfSignedCertificate ssc = new SelfSignedCertificate("spanner.test.der"); + try { + File certFile = tempFolder.newFile("client-der.crt"); + File keyFile = tempFolder.newFile("client-der.key"); + + Files.write(certFile.toPath(), Files.readAllBytes(ssc.certificate().toPath())); + Files.write(keyFile.toPath(), ssc.key().getEncoded()); + + DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile); + String alias = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); + assertNotNull(alias); + assertNotNull(keyManager.getCertificateChain(alias)); + assertNotNull(keyManager.getPrivateKey(alias)); + } finally { + ssc.delete(); + } + } } From f78415fedd562e87da78061d6136aafc513816ba Mon Sep 17 00:00:00 2001 From: sagnghos Date: Mon, 21 Sep 2026 10:33:02 +0000 Subject: [PATCH 09/22] fix(spanner): Non-blocking Netty handshakes via background scheduled reloader, lazy CertificateFactoryHolder, and deterministic alias eviction --- .../cloud/spanner/omni/DynamicKeyManager.java | 71 +++++++--- .../spanner/omni/DynamicTrustManager.java | 98 +++++++++---- .../spanner/omni/DynamicKeyManagerTest.java | 131 +++++++++++++++--- .../spanner/omni/DynamicTrustManagerTest.java | 92 +++++++++--- 4 files changed, 301 insertions(+), 91 deletions(-) diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java index 76306eff7410..eebeee26c0b4 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java @@ -37,6 +37,10 @@ import java.util.Base64; import java.util.Collection; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; import java.util.logging.Level; import java.util.logging.Logger; @@ -48,16 +52,33 @@ * from disk whenever the underlying files are modified or rotated. */ @InternalApi -public class DynamicKeyManager extends X509ExtendedKeyManager { +public class DynamicKeyManager extends X509ExtendedKeyManager implements AutoCloseable { private static final Logger logger = Logger.getLogger(DynamicKeyManager.class.getName()); private static final long DEFAULT_CHECK_INTERVAL_MS = 5000L; + private static final ThreadFactory DAEMON_THREAD_FACTORY = + r -> { + Thread t = new Thread(r, "spanner-omni-key-manager-reloader"); + t.setDaemon(true); + return t; + }; private final File certFile; private final File keyFile; - private final long checkIntervalNs; + private final ScheduledExecutorService scheduler; private final ConcurrentHashMap materials = new ConcurrentHashMap<>(); private final AtomicLong versionCounter = new AtomicLong(); - private volatile long lastCheckedNs; + + private static class CertificateFactoryHolder { + static final CertificateFactory INSTANCE; + + static { + try { + INSTANCE = CertificateFactory.getInstance("X.509"); + } catch (CertificateException e) { + throw new ExceptionInInitializerError(e); + } + } + } private static class KeyMaterial { final String alias; @@ -102,7 +123,6 @@ public DynamicKeyManager(File certFile, File keyFile) { DynamicKeyManager(File certFile, File keyFile, long checkIntervalMs) { this.certFile = Preconditions.checkNotNull(certFile, "certFile cannot be null"); this.keyFile = Preconditions.checkNotNull(keyFile, "keyFile cannot be null"); - this.checkIntervalNs = checkIntervalMs * 1_000_000L; try { reloadMaterial(); } catch (IllegalArgumentException e) { @@ -110,15 +130,16 @@ public DynamicKeyManager(File certFile, File keyFile) { } catch (Exception e) { throw new RuntimeException("Failed to initialize client certificate/key", e); } - this.lastCheckedNs = System.nanoTime(); + if (checkIntervalMs > 0) { + this.scheduler = Executors.newSingleThreadScheduledExecutor(DAEMON_THREAD_FACTORY); + this.scheduler.scheduleWithFixedDelay( + this::checkAndReload, checkIntervalMs, checkIntervalMs, TimeUnit.MILLISECONDS); + } else { + this.scheduler = null; + } } - private void checkAndReload() { - long now = System.nanoTime(); - if (now - lastCheckedNs < checkIntervalNs) { - return; - } - lastCheckedNs = now; + void checkAndReload() { KeyMaterial existing = this.currentMaterial; if (existing != null && certFile.lastModified() == existing.certLastModified @@ -160,15 +181,21 @@ private void reloadMaterial() throws Exception { PrivateKey key = parsePrivateKey(keyBytes); verifyKeyMatch(chain[0].getPublicKey(), key); - String alias = "client-" + versionCounter.incrementAndGet(); + long currentVersion = versionCounter.incrementAndGet(); + String alias = "client-" + currentVersion; KeyMaterial newMaterial = new KeyMaterial(alias, certMod, certLen, keyMod, keyLen, chain, key); materials.put(alias, newMaterial); this.currentMaterial = newMaterial; - if (materials.size() > 10) { - for (String oldAlias : materials.keySet()) { - if (!oldAlias.equals(alias) && materials.size() > 10) { - materials.remove(oldAlias); + + long oldestToKeep = currentVersion - 10; + for (String keyStr : materials.keySet()) { + try { + long ver = Long.parseLong(keyStr.substring("client-".length())); + if (ver < oldestToKeep) { + materials.remove(keyStr); } + } catch (Exception ignored) { + materials.remove(keyStr); } } } @@ -194,7 +221,7 @@ private static void verifyKeyMatch(PublicKey publicKey, PrivateKey privateKey) } private static X509Certificate[] parseCertificates(byte[] certBytes) throws CertificateException { - CertificateFactory cf = CertificateFactory.getInstance("X.509"); + CertificateFactory cf = CertificateFactoryHolder.INSTANCE; Collection certs = cf.generateCertificates(new ByteArrayInputStream(certBytes)); if (certs == null || certs.isEmpty()) { @@ -256,14 +283,12 @@ private static byte[] extractPemContent(String pem, String beginMarker, String e @Override public String chooseClientAlias(String[] keyType, Principal[] issuers, Socket socket) { - checkAndReload(); KeyMaterial mat = this.currentMaterial; return mat != null ? mat.alias : null; } @Override public String chooseEngineClientAlias(String[] keyType, Principal[] issuers, SSLEngine engine) { - checkAndReload(); KeyMaterial mat = this.currentMaterial; return mat != null ? mat.alias : null; } @@ -288,7 +313,6 @@ public PrivateKey getPrivateKey(String alias) { @Override public String[] getClientAliases(String keyType, Principal[] issuers) { - checkAndReload(); KeyMaterial mat = this.currentMaterial; return mat != null ? new String[] {mat.alias} : null; } @@ -307,4 +331,11 @@ public String chooseServerAlias(String keyType, Principal[] issuers, Socket sock public String chooseEngineServerAlias(String keyType, Principal[] issuers, SSLEngine engine) { return null; } + + @Override + public void close() { + if (scheduler != null) { + scheduler.shutdown(); + } + } } diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java index 8291693c377b..5e051d6864de 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java @@ -28,6 +28,10 @@ import java.security.cert.CertificateFactory; import java.security.cert.X509Certificate; import java.util.Collection; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; import javax.annotation.Nullable; @@ -42,13 +46,30 @@ * whenever the certificate file is modified or rotated. */ @InternalApi -public class DynamicTrustManager extends X509ExtendedTrustManager { +public class DynamicTrustManager extends X509ExtendedTrustManager implements AutoCloseable { private static final Logger logger = Logger.getLogger(DynamicTrustManager.class.getName()); private static final long DEFAULT_CHECK_INTERVAL_MS = 5000L; + private static final ThreadFactory DAEMON_THREAD_FACTORY = + r -> { + Thread t = new Thread(r, "spanner-omni-trust-manager-reloader"); + t.setDaemon(true); + return t; + }; private final File caCertFile; - private final long checkIntervalNs; - private volatile long lastCheckedNs; + private final ScheduledExecutorService scheduler; + + private static class CertificateFactoryHolder { + static final CertificateFactory INSTANCE; + + static { + try { + INSTANCE = CertificateFactory.getInstance("X.509"); + } catch (CertificateException e) { + throw new ExceptionInInitializerError(e); + } + } + } private static class TrustMaterial { final long lastModified; @@ -78,24 +99,24 @@ public DynamicTrustManager(@Nullable File caCertFile) { DynamicTrustManager(@Nullable File caCertFile, long checkIntervalMs) { this.caCertFile = caCertFile; - this.checkIntervalNs = checkIntervalMs * 1_000_000L; try { reloadMaterial(); } catch (Exception e) { throw new RuntimeException("Failed to initialize CA certificate", e); } - this.lastCheckedNs = System.nanoTime(); + if (caCertFile != null && checkIntervalMs > 0) { + this.scheduler = Executors.newSingleThreadScheduledExecutor(DAEMON_THREAD_FACTORY); + this.scheduler.scheduleWithFixedDelay( + this::checkAndReload, checkIntervalMs, checkIntervalMs, TimeUnit.MILLISECONDS); + } else { + this.scheduler = null; + } } - private void checkAndReload() { + void checkAndReload() { if (this.caCertFile == null) { return; } - long now = System.nanoTime(); - if (now - lastCheckedNs < checkIntervalNs) { - return; - } - lastCheckedNs = now; TrustMaterial existing = this.currentMaterial; if (existing != null && caCertFile.lastModified() == existing.lastModified @@ -133,7 +154,7 @@ private void reloadMaterial() throws Exception { long len = caCertFile.length(); byte[] certBytes = Files.readAllBytes(caCertFile.toPath()); - CertificateFactory cf = CertificateFactory.getInstance("X.509"); + CertificateFactory cf = CertificateFactoryHolder.INSTANCE; Collection certs = cf.generateCertificates(new ByteArrayInputStream(certBytes)); if (certs == null || certs.isEmpty()) { @@ -214,48 +235,73 @@ public X509Certificate[] getAcceptedIssuers() { @Override public void checkClientTrusted(X509Certificate[] chain, String authType, Socket socket) throws CertificateException { - checkAndReload(); - this.currentMaterial.delegate.checkClientTrusted(chain, authType, socket); + TrustMaterial mat = this.currentMaterial; + if (mat == null) { + throw new CertificateException("Trust manager is not initialized"); + } + mat.delegate.checkClientTrusted(chain, authType, socket); } @Override public void checkServerTrusted(X509Certificate[] chain, String authType, Socket socket) throws CertificateException { - checkAndReload(); - this.currentMaterial.delegate.checkServerTrusted(chain, authType, socket); + TrustMaterial mat = this.currentMaterial; + if (mat == null) { + throw new CertificateException("Trust manager is not initialized"); + } + mat.delegate.checkServerTrusted(chain, authType, socket); } @Override public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngine engine) throws CertificateException { - checkAndReload(); - this.currentMaterial.delegate.checkClientTrusted(chain, authType, engine); + TrustMaterial mat = this.currentMaterial; + if (mat == null) { + throw new CertificateException("Trust manager is not initialized"); + } + mat.delegate.checkClientTrusted(chain, authType, engine); } @Override public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngine engine) throws CertificateException { - checkAndReload(); - this.currentMaterial.delegate.checkServerTrusted(chain, authType, engine); + TrustMaterial mat = this.currentMaterial; + if (mat == null) { + throw new CertificateException("Trust manager is not initialized"); + } + mat.delegate.checkServerTrusted(chain, authType, engine); } @Override public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException { - checkAndReload(); - this.currentMaterial.delegate.checkClientTrusted(chain, authType); + TrustMaterial mat = this.currentMaterial; + if (mat == null) { + throw new CertificateException("Trust manager is not initialized"); + } + mat.delegate.checkClientTrusted(chain, authType); } @Override public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException { - checkAndReload(); - this.currentMaterial.delegate.checkServerTrusted(chain, authType); + TrustMaterial mat = this.currentMaterial; + if (mat == null) { + throw new CertificateException("Trust manager is not initialized"); + } + mat.delegate.checkServerTrusted(chain, authType); } @Override public X509Certificate[] getAcceptedIssuers() { - checkAndReload(); - return this.currentMaterial.delegate.getAcceptedIssuers(); + TrustMaterial mat = this.currentMaterial; + return mat != null ? mat.delegate.getAcceptedIssuers() : new X509Certificate[0]; + } + + @Override + public void close() { + if (scheduler != null) { + scheduler.shutdown(); + } } } diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java index d46c1b497325..6c9c02660af3 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java @@ -76,6 +76,8 @@ public void testInitialLoadAndDynamicRotation() throws Exception { Files.write(certFile.toPath(), Files.readAllBytes(ssc2.certificate().toPath())); Files.write(keyFile.toPath(), Files.readAllBytes(ssc2.privateKey().toPath())); + keyManager.checkAndReload(); + String alias2 = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); assertNotNull(alias2); @@ -91,6 +93,86 @@ public void testInitialLoadAndDynamicRotation() throws Exception { } } + @Test + public void testBackgroundReloadScheduled() throws Exception { + SelfSignedCertificate ssc1 = new SelfSignedCertificate("spanner.test.bg1"); + SelfSignedCertificate ssc2 = new SelfSignedCertificate("spanner.test.bg2"); + try { + File certFile = tempFolder.newFile("client-bg.crt"); + File keyFile = tempFolder.newFile("client-bg.key"); + + Files.write(certFile.toPath(), Files.readAllBytes(ssc1.certificate().toPath())); + Files.write(keyFile.toPath(), Files.readAllBytes(ssc1.privateKey().toPath())); + + try (DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile, 50L)) { + String alias1 = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); + assertNotNull(alias1); + assertEquals( + ssc1.cert().getSubjectDN(), keyManager.getCertificateChain(alias1)[0].getSubjectDN()); + + Thread.sleep(1100); + + Files.write(certFile.toPath(), Files.readAllBytes(ssc2.certificate().toPath())); + Files.write(keyFile.toPath(), Files.readAllBytes(ssc2.privateKey().toPath())); + + // Background scheduler should reload material within ~1 second + String alias2 = null; + for (int i = 0; i < 40; i++) { + alias2 = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); + if (alias2 != null && !alias2.equals(alias1)) { + break; + } + Thread.sleep(50); + } + + assertNotNull(alias2); + assertThat(alias2).isNotEqualTo(alias1); + assertEquals( + ssc2.cert().getSubjectDN(), keyManager.getCertificateChain(alias2)[0].getSubjectDN()); + } + } finally { + ssc1.delete(); + ssc2.delete(); + } + } + + @Test + public void testDeterministicAliasEviction() throws Exception { + SelfSignedCertificate ssc = new SelfSignedCertificate("spanner.test.eviction"); + try { + File certFile = tempFolder.newFile("client-eviction.crt"); + File keyFile = tempFolder.newFile("client-eviction.key"); + + Files.write(certFile.toPath(), Files.readAllBytes(ssc.certificate().toPath())); + Files.write(keyFile.toPath(), Files.readAllBytes(ssc.privateKey().toPath())); + + DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile, 0L); + String firstAlias = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); + assertEquals("client-1", firstAlias); + assertNotNull(keyManager.getCertificateChain(firstAlias)); + + // Trigger 15 rotations + for (int i = 2; i <= 15; i++) { + Thread.sleep(10); + certFile.setLastModified(System.currentTimeMillis() + i * 1000L); + keyFile.setLastModified(System.currentTimeMillis() + i * 1000L); + keyManager.checkAndReload(); + } + + String latestAlias = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); + assertEquals("client-15", latestAlias); + assertNotNull(keyManager.getCertificateChain(latestAlias)); + + // Oldest alias "client-1" should have been evicted (oldest kept is 15 - 10 = 5) + // When lookup for evicted alias happens, it falls back to currentMaterial + // Let's verify alias "client-1" is not retained in old version map + assertNotNull(keyManager.getCertificateChain("client-15")); + assertNotNull(keyManager.getCertificateChain("client-5")); + } finally { + ssc.delete(); + } + } + @Test public void testFileCheckThrottling() throws Exception { SelfSignedCertificate ssc1 = new SelfSignedCertificate("spanner.test.throttle1"); @@ -103,19 +185,20 @@ public void testFileCheckThrottling() throws Exception { Files.write(keyFile.toPath(), Files.readAllBytes(ssc1.privateKey().toPath())); // 60-second check interval - DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile, 60000L); - - String alias1 = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); - assertEquals( - ssc1.cert().getSubjectDN(), keyManager.getCertificateChain(alias1)[0].getSubjectDN()); - - // Rotate files immediately on disk - Files.write(certFile.toPath(), Files.readAllBytes(ssc2.certificate().toPath())); - Files.write(keyFile.toPath(), Files.readAllBytes(ssc2.privateKey().toPath())); - - // Within the throttle interval, the manager should retain and return previous certificate - assertEquals( - ssc1.cert().getSubjectDN(), keyManager.getCertificateChain(alias1)[0].getSubjectDN()); + try (DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile, 60000L)) { + String alias1 = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); + assertEquals( + ssc1.cert().getSubjectDN(), keyManager.getCertificateChain(alias1)[0].getSubjectDN()); + + // Rotate files immediately on disk + Files.write(certFile.toPath(), Files.readAllBytes(ssc2.certificate().toPath())); + Files.write(keyFile.toPath(), Files.readAllBytes(ssc2.privateKey().toPath())); + + // Without waiting for background poller, the manager immediately returns previous + // certificate + assertEquals( + ssc1.cert().getSubjectDN(), keyManager.getCertificateChain(alias1)[0].getSubjectDN()); + } } finally { ssc1.delete(); ssc2.delete(); @@ -140,6 +223,7 @@ public void testCorruptRotationFallsBackToPrevious() throws Exception { // Overwrite certFile with corrupt bytes Files.write(certFile.toPath(), "NOT A CERTIFICATE CONTENT".getBytes(StandardCharsets.UTF_8)); + keyManager.checkAndReload(); // DynamicKeyManager should catch reload error and retain previous material String aliasAfter = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); @@ -195,10 +279,11 @@ public void testServerAliasesReturnNull() throws Exception { Files.write(certFile.toPath(), Files.readAllBytes(ssc.certificate().toPath())); Files.write(keyFile.toPath(), Files.readAllBytes(ssc.privateKey().toPath())); - DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile); - assertNull(keyManager.getServerAliases("RSA", null)); - assertNull(keyManager.chooseServerAlias("RSA", null, null)); - assertNull(keyManager.chooseEngineServerAlias("RSA", null, null)); + try (DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile)) { + assertNull(keyManager.getServerAliases("RSA", null)); + assertNull(keyManager.chooseServerAlias("RSA", null, null)); + assertNull(keyManager.chooseEngineServerAlias("RSA", null, null)); + } } finally { ssc.delete(); } @@ -246,6 +331,7 @@ public void testMismatchedRotationFallsBackToPrevious() throws Exception { // Rotate only cert file (e.g., intermediate state during rotation) Files.write(certFile.toPath(), Files.readAllBytes(ssc2.certificate().toPath())); + keyManager.checkAndReload(); // Key manager should detect mismatch and retain ssc1 credentials String aliasAfter = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); @@ -267,11 +353,12 @@ public void testBinaryDerKeySupported() throws Exception { Files.write(certFile.toPath(), Files.readAllBytes(ssc.certificate().toPath())); Files.write(keyFile.toPath(), ssc.key().getEncoded()); - DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile); - String alias = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); - assertNotNull(alias); - assertNotNull(keyManager.getCertificateChain(alias)); - assertNotNull(keyManager.getPrivateKey(alias)); + try (DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile)) { + String alias = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); + assertNotNull(alias); + assertNotNull(keyManager.getCertificateChain(alias)); + assertNotNull(keyManager.getPrivateKey(alias)); + } } finally { ssc.delete(); } diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicTrustManagerTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicTrustManagerTest.java index 114b6620d1ce..dbcc0e13bcf3 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicTrustManagerTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicTrustManagerTest.java @@ -16,6 +16,7 @@ package com.google.cloud.spanner.omni; +import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThrows; @@ -40,10 +41,11 @@ public class DynamicTrustManagerTest { @Test public void testDefaultTrustManagerWithNull() throws Exception { - DynamicTrustManager trustManager = new DynamicTrustManager((File) null); - X509Certificate[] issuers = trustManager.getAcceptedIssuers(); - assertNotNull(issuers); - assertTrue(issuers.length > 0); + try (DynamicTrustManager trustManager = new DynamicTrustManager((File) null)) { + X509Certificate[] issuers = trustManager.getAcceptedIssuers(); + assertNotNull(issuers); + assertTrue(issuers.length > 0); + } } @Test @@ -73,6 +75,7 @@ public void testCustomTrustManagerAndDynamicRotation() throws Exception { // Rotate CA file on disk to ca2 Files.write(caFile.toPath(), Files.readAllBytes(ca2.certificate().toPath())); + trustManager.checkAndReload(); // Now ca2 should be accepted and ca1 should be rejected X509Certificate[] issuers2 = trustManager.getAcceptedIssuers(); @@ -91,6 +94,47 @@ public void testCustomTrustManagerAndDynamicRotation() throws Exception { } } + @Test + public void testBackgroundReloadScheduled() throws Exception { + SelfSignedCertificate ca1 = new SelfSignedCertificate("spanner.ca.bg1"); + SelfSignedCertificate ca2 = new SelfSignedCertificate("spanner.ca.bg2"); + try { + File caFile = tempFolder.newFile("ca-bg.crt"); + Files.write(caFile.toPath(), Files.readAllBytes(ca1.certificate().toPath())); + + try (DynamicTrustManager trustManager = new DynamicTrustManager(caFile, 50L)) { + X509Certificate[] issuers1 = trustManager.getAcceptedIssuers(); + assertNotNull(issuers1); + assertEquals(1, issuers1.length); + assertEquals(ca1.cert().getSubjectDN(), issuers1[0].getSubjectDN()); + + Thread.sleep(1100); + + Files.write(caFile.toPath(), Files.readAllBytes(ca2.certificate().toPath())); + + // Background scheduler should reload CA material within ~1 second + X509Certificate[] issuers2 = null; + for (int i = 0; i < 40; i++) { + issuers2 = trustManager.getAcceptedIssuers(); + if (issuers2 != null + && issuers2.length > 0 + && issuers2[0].getSubjectDN().equals(ca2.cert().getSubjectDN())) { + break; + } + Thread.sleep(50); + } + + assertNotNull(issuers2); + assertThat(issuers2.length).isEqualTo(1); + assertEquals(ca2.cert().getSubjectDN(), issuers2[0].getSubjectDN()); + trustManager.checkServerTrusted(new X509Certificate[] {ca2.cert()}, "RSA"); + } + } finally { + ca1.delete(); + ca2.delete(); + } + } + @Test public void testFileCheckThrottling() throws Exception { SelfSignedCertificate ca1 = new SelfSignedCertificate("spanner.ca.throttle1"); @@ -100,18 +144,19 @@ public void testFileCheckThrottling() throws Exception { Files.write(caFile.toPath(), Files.readAllBytes(ca1.certificate().toPath())); // 60-second check interval - DynamicTrustManager trustManager = new DynamicTrustManager(caFile, 60000L); - - X509Certificate[] issuers1 = trustManager.getAcceptedIssuers(); - assertEquals(1, issuers1.length); - assertEquals(ca1.cert().getSubjectDN(), issuers1[0].getSubjectDN()); - - // Rotate CA on disk immediately - Files.write(caFile.toPath(), Files.readAllBytes(ca2.certificate().toPath())); - - // Within throttle interval, trust manager should retain previous CA - assertEquals(ca1.cert().getSubjectDN(), trustManager.getAcceptedIssuers()[0].getSubjectDN()); - trustManager.checkServerTrusted(new X509Certificate[] {ca1.cert()}, "RSA"); + try (DynamicTrustManager trustManager = new DynamicTrustManager(caFile, 60000L)) { + X509Certificate[] issuers1 = trustManager.getAcceptedIssuers(); + assertEquals(1, issuers1.length); + assertEquals(ca1.cert().getSubjectDN(), issuers1[0].getSubjectDN()); + + // Rotate CA on disk immediately + Files.write(caFile.toPath(), Files.readAllBytes(ca2.certificate().toPath())); + + // Without background poller firing, trust manager returns previous CA immediately + assertEquals( + ca1.cert().getSubjectDN(), trustManager.getAcceptedIssuers()[0].getSubjectDN()); + trustManager.checkServerTrusted(new X509Certificate[] {ca1.cert()}, "RSA"); + } } finally { ca1.delete(); ca2.delete(); @@ -132,14 +177,14 @@ public void testMultipleCAsInFile() throws Exception { .getBytes(StandardCharsets.UTF_8); Files.write(caFile.toPath(), bundle); - DynamicTrustManager trustManager = new DynamicTrustManager(caFile); - - X509Certificate[] issuers = trustManager.getAcceptedIssuers(); - assertNotNull(issuers); - assertEquals(2, issuers.length); + try (DynamicTrustManager trustManager = new DynamicTrustManager(caFile)) { + X509Certificate[] issuers = trustManager.getAcceptedIssuers(); + assertNotNull(issuers); + assertEquals(2, issuers.length); - trustManager.checkServerTrusted(new X509Certificate[] {ca1.cert()}, "RSA"); - trustManager.checkServerTrusted(new X509Certificate[] {ca2.cert()}, "RSA"); + trustManager.checkServerTrusted(new X509Certificate[] {ca1.cert()}, "RSA"); + trustManager.checkServerTrusted(new X509Certificate[] {ca2.cert()}, "RSA"); + } } finally { ca1.delete(); ca2.delete(); @@ -160,6 +205,7 @@ public void testCorruptRotationFallsBackToPrevious() throws Exception { // Corrupt the file Files.write(caFile.toPath(), "CORRUPT CERT DATA".getBytes(StandardCharsets.UTF_8)); + trustManager.checkAndReload(); // Trust manager should retain previous CA trustManager.checkServerTrusted(new X509Certificate[] {ca.cert()}, "RSA"); From 53f59f95958b1a7c0bb20a1ee1943426fd9df7dd Mon Sep 17 00:00:00 2001 From: sagnghos Date: Mon, 21 Sep 2026 10:41:51 +0000 Subject: [PATCH 10/22] fix(spanner): On-demand rate-limited checking on TLS handshake, remove background scheduler to prevent leaks, and support ECDSA key verification --- .../cloud/spanner/omni/DynamicKeyManager.java | 49 +++++----- .../spanner/omni/DynamicTrustManager.java | 49 +++++----- .../spanner/omni/DynamicKeyManagerTest.java | 97 +++++-------------- .../spanner/omni/DynamicTrustManagerTest.java | 90 ++++------------- 4 files changed, 89 insertions(+), 196 deletions(-) diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java index eebeee26c0b4..ae8a9580cb7b 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java @@ -37,10 +37,6 @@ import java.util.Base64; import java.util.Collection; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ThreadFactory; -import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; import java.util.logging.Level; import java.util.logging.Logger; @@ -52,21 +48,16 @@ * from disk whenever the underlying files are modified or rotated. */ @InternalApi -public class DynamicKeyManager extends X509ExtendedKeyManager implements AutoCloseable { +public class DynamicKeyManager extends X509ExtendedKeyManager { private static final Logger logger = Logger.getLogger(DynamicKeyManager.class.getName()); private static final long DEFAULT_CHECK_INTERVAL_MS = 5000L; - private static final ThreadFactory DAEMON_THREAD_FACTORY = - r -> { - Thread t = new Thread(r, "spanner-omni-key-manager-reloader"); - t.setDaemon(true); - return t; - }; private final File certFile; private final File keyFile; - private final ScheduledExecutorService scheduler; + private final long checkIntervalMs; private final ConcurrentHashMap materials = new ConcurrentHashMap<>(); private final AtomicLong versionCounter = new AtomicLong(); + private volatile long lastCheckedMs; private static class CertificateFactoryHolder { static final CertificateFactory INSTANCE; @@ -123,6 +114,7 @@ public DynamicKeyManager(File certFile, File keyFile) { DynamicKeyManager(File certFile, File keyFile, long checkIntervalMs) { this.certFile = Preconditions.checkNotNull(certFile, "certFile cannot be null"); this.keyFile = Preconditions.checkNotNull(keyFile, "keyFile cannot be null"); + this.checkIntervalMs = checkIntervalMs; try { reloadMaterial(); } catch (IllegalArgumentException e) { @@ -130,31 +122,34 @@ public DynamicKeyManager(File certFile, File keyFile) { } catch (Exception e) { throw new RuntimeException("Failed to initialize client certificate/key", e); } - if (checkIntervalMs > 0) { - this.scheduler = Executors.newSingleThreadScheduledExecutor(DAEMON_THREAD_FACTORY); - this.scheduler.scheduleWithFixedDelay( - this::checkAndReload, checkIntervalMs, checkIntervalMs, TimeUnit.MILLISECONDS); - } else { - this.scheduler = null; - } + this.lastCheckedMs = System.currentTimeMillis(); } void checkAndReload() { + long now = System.currentTimeMillis(); + if (checkIntervalMs > 0 && now - lastCheckedMs < checkIntervalMs) { + return; + } KeyMaterial existing = this.currentMaterial; if (existing != null && certFile.lastModified() == existing.certLastModified && certFile.length() == existing.certLength && keyFile.lastModified() == existing.keyLastModified && keyFile.length() == existing.keyLength) { + lastCheckedMs = now; return; } synchronized (this) { + if (checkIntervalMs > 0 && now - lastCheckedMs < checkIntervalMs) { + return; + } existing = this.currentMaterial; if (existing != null && certFile.lastModified() == existing.certLastModified && certFile.length() == existing.certLength && keyFile.lastModified() == existing.keyLastModified && keyFile.length() == existing.keyLength) { + lastCheckedMs = now; return; } try { @@ -164,6 +159,8 @@ void checkAndReload() { Level.WARNING, "Failed to reload rotated client certificate/key from disk, retaining current material", e); + } finally { + lastCheckedMs = now; } } } @@ -206,7 +203,9 @@ private static void verifyKeyMatch(PublicKey publicKey, PrivateKey privateKey) String sigAlg = "RSA".equalsIgnoreCase(algorithm) ? "SHA256withRSA" - : "EC".equalsIgnoreCase(algorithm) ? "SHA256withECDSA" : null; + : ("EC".equalsIgnoreCase(algorithm) || "ECDSA".equalsIgnoreCase(algorithm)) + ? "SHA256withECDSA" + : null; if (sigAlg != null) { Signature sig = Signature.getInstance(sigAlg); sig.initSign(privateKey); @@ -283,12 +282,14 @@ private static byte[] extractPemContent(String pem, String beginMarker, String e @Override public String chooseClientAlias(String[] keyType, Principal[] issuers, Socket socket) { + checkAndReload(); KeyMaterial mat = this.currentMaterial; return mat != null ? mat.alias : null; } @Override public String chooseEngineClientAlias(String[] keyType, Principal[] issuers, SSLEngine engine) { + checkAndReload(); KeyMaterial mat = this.currentMaterial; return mat != null ? mat.alias : null; } @@ -313,6 +314,7 @@ public PrivateKey getPrivateKey(String alias) { @Override public String[] getClientAliases(String keyType, Principal[] issuers) { + checkAndReload(); KeyMaterial mat = this.currentMaterial; return mat != null ? new String[] {mat.alias} : null; } @@ -331,11 +333,4 @@ public String chooseServerAlias(String keyType, Principal[] issuers, Socket sock public String chooseEngineServerAlias(String keyType, Principal[] issuers, SSLEngine engine) { return null; } - - @Override - public void close() { - if (scheduler != null) { - scheduler.shutdown(); - } - } } diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java index 5e051d6864de..e66e3342cba3 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java @@ -28,10 +28,6 @@ import java.security.cert.CertificateFactory; import java.security.cert.X509Certificate; import java.util.Collection; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ThreadFactory; -import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; import javax.annotation.Nullable; @@ -46,18 +42,13 @@ * whenever the certificate file is modified or rotated. */ @InternalApi -public class DynamicTrustManager extends X509ExtendedTrustManager implements AutoCloseable { +public class DynamicTrustManager extends X509ExtendedTrustManager { private static final Logger logger = Logger.getLogger(DynamicTrustManager.class.getName()); private static final long DEFAULT_CHECK_INTERVAL_MS = 5000L; - private static final ThreadFactory DAEMON_THREAD_FACTORY = - r -> { - Thread t = new Thread(r, "spanner-omni-trust-manager-reloader"); - t.setDaemon(true); - return t; - }; private final File caCertFile; - private final ScheduledExecutorService scheduler; + private final long checkIntervalMs; + private volatile long lastCheckedMs; private static class CertificateFactoryHolder { static final CertificateFactory INSTANCE; @@ -99,35 +90,39 @@ public DynamicTrustManager(@Nullable File caCertFile) { DynamicTrustManager(@Nullable File caCertFile, long checkIntervalMs) { this.caCertFile = caCertFile; + this.checkIntervalMs = checkIntervalMs; try { reloadMaterial(); } catch (Exception e) { throw new RuntimeException("Failed to initialize CA certificate", e); } - if (caCertFile != null && checkIntervalMs > 0) { - this.scheduler = Executors.newSingleThreadScheduledExecutor(DAEMON_THREAD_FACTORY); - this.scheduler.scheduleWithFixedDelay( - this::checkAndReload, checkIntervalMs, checkIntervalMs, TimeUnit.MILLISECONDS); - } else { - this.scheduler = null; - } + this.lastCheckedMs = System.currentTimeMillis(); } void checkAndReload() { if (this.caCertFile == null) { return; } + long now = System.currentTimeMillis(); + if (checkIntervalMs > 0 && now - lastCheckedMs < checkIntervalMs) { + return; + } TrustMaterial existing = this.currentMaterial; if (existing != null && caCertFile.lastModified() == existing.lastModified && caCertFile.length() == existing.length) { + lastCheckedMs = now; return; } synchronized (this) { + if (checkIntervalMs > 0 && now - lastCheckedMs < checkIntervalMs) { + return; + } existing = this.currentMaterial; if (existing != null && caCertFile.lastModified() == existing.lastModified && caCertFile.length() == existing.length) { + lastCheckedMs = now; return; } try { @@ -137,6 +132,8 @@ void checkAndReload() { Level.WARNING, "Failed to reload rotated CA certificate from disk, retaining previous material", e); + } finally { + lastCheckedMs = now; } } } @@ -235,6 +232,7 @@ public X509Certificate[] getAcceptedIssuers() { @Override public void checkClientTrusted(X509Certificate[] chain, String authType, Socket socket) throws CertificateException { + checkAndReload(); TrustMaterial mat = this.currentMaterial; if (mat == null) { throw new CertificateException("Trust manager is not initialized"); @@ -245,6 +243,7 @@ public void checkClientTrusted(X509Certificate[] chain, String authType, Socket @Override public void checkServerTrusted(X509Certificate[] chain, String authType, Socket socket) throws CertificateException { + checkAndReload(); TrustMaterial mat = this.currentMaterial; if (mat == null) { throw new CertificateException("Trust manager is not initialized"); @@ -255,6 +254,7 @@ public void checkServerTrusted(X509Certificate[] chain, String authType, Socket @Override public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngine engine) throws CertificateException { + checkAndReload(); TrustMaterial mat = this.currentMaterial; if (mat == null) { throw new CertificateException("Trust manager is not initialized"); @@ -265,6 +265,7 @@ public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngi @Override public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngine engine) throws CertificateException { + checkAndReload(); TrustMaterial mat = this.currentMaterial; if (mat == null) { throw new CertificateException("Trust manager is not initialized"); @@ -275,6 +276,7 @@ public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngi @Override public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException { + checkAndReload(); TrustMaterial mat = this.currentMaterial; if (mat == null) { throw new CertificateException("Trust manager is not initialized"); @@ -285,6 +287,7 @@ public void checkClientTrusted(X509Certificate[] chain, String authType) @Override public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException { + checkAndReload(); TrustMaterial mat = this.currentMaterial; if (mat == null) { throw new CertificateException("Trust manager is not initialized"); @@ -294,14 +297,8 @@ public void checkServerTrusted(X509Certificate[] chain, String authType) @Override public X509Certificate[] getAcceptedIssuers() { + checkAndReload(); TrustMaterial mat = this.currentMaterial; return mat != null ? mat.delegate.getAcceptedIssuers() : new X509Certificate[0]; } - - @Override - public void close() { - if (scheduler != null) { - scheduler.shutdown(); - } - } } diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java index 6c9c02660af3..74c0a8428df2 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java @@ -76,8 +76,6 @@ public void testInitialLoadAndDynamicRotation() throws Exception { Files.write(certFile.toPath(), Files.readAllBytes(ssc2.certificate().toPath())); Files.write(keyFile.toPath(), Files.readAllBytes(ssc2.privateKey().toPath())); - keyManager.checkAndReload(); - String alias2 = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); assertNotNull(alias2); @@ -93,49 +91,6 @@ public void testInitialLoadAndDynamicRotation() throws Exception { } } - @Test - public void testBackgroundReloadScheduled() throws Exception { - SelfSignedCertificate ssc1 = new SelfSignedCertificate("spanner.test.bg1"); - SelfSignedCertificate ssc2 = new SelfSignedCertificate("spanner.test.bg2"); - try { - File certFile = tempFolder.newFile("client-bg.crt"); - File keyFile = tempFolder.newFile("client-bg.key"); - - Files.write(certFile.toPath(), Files.readAllBytes(ssc1.certificate().toPath())); - Files.write(keyFile.toPath(), Files.readAllBytes(ssc1.privateKey().toPath())); - - try (DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile, 50L)) { - String alias1 = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); - assertNotNull(alias1); - assertEquals( - ssc1.cert().getSubjectDN(), keyManager.getCertificateChain(alias1)[0].getSubjectDN()); - - Thread.sleep(1100); - - Files.write(certFile.toPath(), Files.readAllBytes(ssc2.certificate().toPath())); - Files.write(keyFile.toPath(), Files.readAllBytes(ssc2.privateKey().toPath())); - - // Background scheduler should reload material within ~1 second - String alias2 = null; - for (int i = 0; i < 40; i++) { - alias2 = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); - if (alias2 != null && !alias2.equals(alias1)) { - break; - } - Thread.sleep(50); - } - - assertNotNull(alias2); - assertThat(alias2).isNotEqualTo(alias1); - assertEquals( - ssc2.cert().getSubjectDN(), keyManager.getCertificateChain(alias2)[0].getSubjectDN()); - } - } finally { - ssc1.delete(); - ssc2.delete(); - } - } - @Test public void testDeterministicAliasEviction() throws Exception { SelfSignedCertificate ssc = new SelfSignedCertificate("spanner.test.eviction"); @@ -164,8 +119,6 @@ public void testDeterministicAliasEviction() throws Exception { assertNotNull(keyManager.getCertificateChain(latestAlias)); // Oldest alias "client-1" should have been evicted (oldest kept is 15 - 10 = 5) - // When lookup for evicted alias happens, it falls back to currentMaterial - // Let's verify alias "client-1" is not retained in old version map assertNotNull(keyManager.getCertificateChain("client-15")); assertNotNull(keyManager.getCertificateChain("client-5")); } finally { @@ -185,20 +138,20 @@ public void testFileCheckThrottling() throws Exception { Files.write(keyFile.toPath(), Files.readAllBytes(ssc1.privateKey().toPath())); // 60-second check interval - try (DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile, 60000L)) { - String alias1 = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); - assertEquals( - ssc1.cert().getSubjectDN(), keyManager.getCertificateChain(alias1)[0].getSubjectDN()); - - // Rotate files immediately on disk - Files.write(certFile.toPath(), Files.readAllBytes(ssc2.certificate().toPath())); - Files.write(keyFile.toPath(), Files.readAllBytes(ssc2.privateKey().toPath())); - - // Without waiting for background poller, the manager immediately returns previous - // certificate - assertEquals( - ssc1.cert().getSubjectDN(), keyManager.getCertificateChain(alias1)[0].getSubjectDN()); - } + DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile, 60000L); + String alias1 = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); + assertEquals( + ssc1.cert().getSubjectDN(), keyManager.getCertificateChain(alias1)[0].getSubjectDN()); + + // Rotate files immediately on disk + Files.write(certFile.toPath(), Files.readAllBytes(ssc2.certificate().toPath())); + Files.write(keyFile.toPath(), Files.readAllBytes(ssc2.privateKey().toPath())); + + // Within the throttle interval, the manager should retain and return previous certificate + String aliasThrottled = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); + assertEquals(alias1, aliasThrottled); + assertEquals( + ssc1.cert().getSubjectDN(), keyManager.getCertificateChain(alias1)[0].getSubjectDN()); } finally { ssc1.delete(); ssc2.delete(); @@ -223,7 +176,6 @@ public void testCorruptRotationFallsBackToPrevious() throws Exception { // Overwrite certFile with corrupt bytes Files.write(certFile.toPath(), "NOT A CERTIFICATE CONTENT".getBytes(StandardCharsets.UTF_8)); - keyManager.checkAndReload(); // DynamicKeyManager should catch reload error and retain previous material String aliasAfter = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); @@ -279,11 +231,10 @@ public void testServerAliasesReturnNull() throws Exception { Files.write(certFile.toPath(), Files.readAllBytes(ssc.certificate().toPath())); Files.write(keyFile.toPath(), Files.readAllBytes(ssc.privateKey().toPath())); - try (DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile)) { - assertNull(keyManager.getServerAliases("RSA", null)); - assertNull(keyManager.chooseServerAlias("RSA", null, null)); - assertNull(keyManager.chooseEngineServerAlias("RSA", null, null)); - } + DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile); + assertNull(keyManager.getServerAliases("RSA", null)); + assertNull(keyManager.chooseServerAlias("RSA", null, null)); + assertNull(keyManager.chooseEngineServerAlias("RSA", null, null)); } finally { ssc.delete(); } @@ -331,7 +282,6 @@ public void testMismatchedRotationFallsBackToPrevious() throws Exception { // Rotate only cert file (e.g., intermediate state during rotation) Files.write(certFile.toPath(), Files.readAllBytes(ssc2.certificate().toPath())); - keyManager.checkAndReload(); // Key manager should detect mismatch and retain ssc1 credentials String aliasAfter = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); @@ -353,12 +303,11 @@ public void testBinaryDerKeySupported() throws Exception { Files.write(certFile.toPath(), Files.readAllBytes(ssc.certificate().toPath())); Files.write(keyFile.toPath(), ssc.key().getEncoded()); - try (DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile)) { - String alias = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); - assertNotNull(alias); - assertNotNull(keyManager.getCertificateChain(alias)); - assertNotNull(keyManager.getPrivateKey(alias)); - } + DynamicKeyManager keyManager = new DynamicKeyManager(certFile, keyFile); + String alias = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); + assertNotNull(alias); + assertNotNull(keyManager.getCertificateChain(alias)); + assertNotNull(keyManager.getPrivateKey(alias)); } finally { ssc.delete(); } diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicTrustManagerTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicTrustManagerTest.java index dbcc0e13bcf3..05e8c145fe7f 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicTrustManagerTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicTrustManagerTest.java @@ -16,7 +16,6 @@ package com.google.cloud.spanner.omni; -import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThrows; @@ -41,11 +40,10 @@ public class DynamicTrustManagerTest { @Test public void testDefaultTrustManagerWithNull() throws Exception { - try (DynamicTrustManager trustManager = new DynamicTrustManager((File) null)) { - X509Certificate[] issuers = trustManager.getAcceptedIssuers(); - assertNotNull(issuers); - assertTrue(issuers.length > 0); - } + DynamicTrustManager trustManager = new DynamicTrustManager((File) null); + X509Certificate[] issuers = trustManager.getAcceptedIssuers(); + assertNotNull(issuers); + assertTrue(issuers.length > 0); } @Test @@ -75,7 +73,6 @@ public void testCustomTrustManagerAndDynamicRotation() throws Exception { // Rotate CA file on disk to ca2 Files.write(caFile.toPath(), Files.readAllBytes(ca2.certificate().toPath())); - trustManager.checkAndReload(); // Now ca2 should be accepted and ca1 should be rejected X509Certificate[] issuers2 = trustManager.getAcceptedIssuers(); @@ -94,47 +91,6 @@ public void testCustomTrustManagerAndDynamicRotation() throws Exception { } } - @Test - public void testBackgroundReloadScheduled() throws Exception { - SelfSignedCertificate ca1 = new SelfSignedCertificate("spanner.ca.bg1"); - SelfSignedCertificate ca2 = new SelfSignedCertificate("spanner.ca.bg2"); - try { - File caFile = tempFolder.newFile("ca-bg.crt"); - Files.write(caFile.toPath(), Files.readAllBytes(ca1.certificate().toPath())); - - try (DynamicTrustManager trustManager = new DynamicTrustManager(caFile, 50L)) { - X509Certificate[] issuers1 = trustManager.getAcceptedIssuers(); - assertNotNull(issuers1); - assertEquals(1, issuers1.length); - assertEquals(ca1.cert().getSubjectDN(), issuers1[0].getSubjectDN()); - - Thread.sleep(1100); - - Files.write(caFile.toPath(), Files.readAllBytes(ca2.certificate().toPath())); - - // Background scheduler should reload CA material within ~1 second - X509Certificate[] issuers2 = null; - for (int i = 0; i < 40; i++) { - issuers2 = trustManager.getAcceptedIssuers(); - if (issuers2 != null - && issuers2.length > 0 - && issuers2[0].getSubjectDN().equals(ca2.cert().getSubjectDN())) { - break; - } - Thread.sleep(50); - } - - assertNotNull(issuers2); - assertThat(issuers2.length).isEqualTo(1); - assertEquals(ca2.cert().getSubjectDN(), issuers2[0].getSubjectDN()); - trustManager.checkServerTrusted(new X509Certificate[] {ca2.cert()}, "RSA"); - } - } finally { - ca1.delete(); - ca2.delete(); - } - } - @Test public void testFileCheckThrottling() throws Exception { SelfSignedCertificate ca1 = new SelfSignedCertificate("spanner.ca.throttle1"); @@ -144,19 +100,17 @@ public void testFileCheckThrottling() throws Exception { Files.write(caFile.toPath(), Files.readAllBytes(ca1.certificate().toPath())); // 60-second check interval - try (DynamicTrustManager trustManager = new DynamicTrustManager(caFile, 60000L)) { - X509Certificate[] issuers1 = trustManager.getAcceptedIssuers(); - assertEquals(1, issuers1.length); - assertEquals(ca1.cert().getSubjectDN(), issuers1[0].getSubjectDN()); - - // Rotate CA on disk immediately - Files.write(caFile.toPath(), Files.readAllBytes(ca2.certificate().toPath())); - - // Without background poller firing, trust manager returns previous CA immediately - assertEquals( - ca1.cert().getSubjectDN(), trustManager.getAcceptedIssuers()[0].getSubjectDN()); - trustManager.checkServerTrusted(new X509Certificate[] {ca1.cert()}, "RSA"); - } + DynamicTrustManager trustManager = new DynamicTrustManager(caFile, 60000L); + X509Certificate[] issuers1 = trustManager.getAcceptedIssuers(); + assertEquals(1, issuers1.length); + assertEquals(ca1.cert().getSubjectDN(), issuers1[0].getSubjectDN()); + + // Rotate CA on disk immediately + Files.write(caFile.toPath(), Files.readAllBytes(ca2.certificate().toPath())); + + // Within throttle interval, trust manager should retain previous CA + assertEquals(ca1.cert().getSubjectDN(), trustManager.getAcceptedIssuers()[0].getSubjectDN()); + trustManager.checkServerTrusted(new X509Certificate[] {ca1.cert()}, "RSA"); } finally { ca1.delete(); ca2.delete(); @@ -177,14 +131,13 @@ public void testMultipleCAsInFile() throws Exception { .getBytes(StandardCharsets.UTF_8); Files.write(caFile.toPath(), bundle); - try (DynamicTrustManager trustManager = new DynamicTrustManager(caFile)) { - X509Certificate[] issuers = trustManager.getAcceptedIssuers(); - assertNotNull(issuers); - assertEquals(2, issuers.length); + DynamicTrustManager trustManager = new DynamicTrustManager(caFile); + X509Certificate[] issuers = trustManager.getAcceptedIssuers(); + assertNotNull(issuers); + assertEquals(2, issuers.length); - trustManager.checkServerTrusted(new X509Certificate[] {ca1.cert()}, "RSA"); - trustManager.checkServerTrusted(new X509Certificate[] {ca2.cert()}, "RSA"); - } + trustManager.checkServerTrusted(new X509Certificate[] {ca1.cert()}, "RSA"); + trustManager.checkServerTrusted(new X509Certificate[] {ca2.cert()}, "RSA"); } finally { ca1.delete(); ca2.delete(); @@ -205,7 +158,6 @@ public void testCorruptRotationFallsBackToPrevious() throws Exception { // Corrupt the file Files.write(caFile.toPath(), "CORRUPT CERT DATA".getBytes(StandardCharsets.UTF_8)); - trustManager.checkAndReload(); // Trust manager should retain previous CA trustManager.checkServerTrusted(new X509Certificate[] {ca.cert()}, "RSA"); From c1f0f39578ec0bb949947cddf0d0f4e435d0c59f Mon Sep 17 00:00:00 2001 From: sagnghos Date: Mon, 21 Sep 2026 10:51:35 +0000 Subject: [PATCH 11/22] fix(spanner): Use monotonic System.nanoTime() and ReentrantLock for thread-safe dynamic certificate checking --- .../cloud/spanner/omni/DynamicKeyManager.java | 27 +++++++++++-------- .../spanner/omni/DynamicTrustManager.java | 27 +++++++++++-------- 2 files changed, 32 insertions(+), 22 deletions(-) diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java index ae8a9580cb7b..9c46b8909f3e 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java @@ -38,6 +38,7 @@ import java.util.Collection; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.locks.ReentrantLock; import java.util.logging.Level; import java.util.logging.Logger; import javax.net.ssl.SSLEngine; @@ -54,10 +55,11 @@ public class DynamicKeyManager extends X509ExtendedKeyManager { private final File certFile; private final File keyFile; - private final long checkIntervalMs; + private final long checkIntervalNs; private final ConcurrentHashMap materials = new ConcurrentHashMap<>(); private final AtomicLong versionCounter = new AtomicLong(); - private volatile long lastCheckedMs; + private final ReentrantLock lock = new ReentrantLock(); + private volatile long lastCheckedNs; private static class CertificateFactoryHolder { static final CertificateFactory INSTANCE; @@ -114,7 +116,7 @@ public DynamicKeyManager(File certFile, File keyFile) { DynamicKeyManager(File certFile, File keyFile, long checkIntervalMs) { this.certFile = Preconditions.checkNotNull(certFile, "certFile cannot be null"); this.keyFile = Preconditions.checkNotNull(keyFile, "keyFile cannot be null"); - this.checkIntervalMs = checkIntervalMs; + this.checkIntervalNs = checkIntervalMs * 1_000_000L; try { reloadMaterial(); } catch (IllegalArgumentException e) { @@ -122,12 +124,12 @@ public DynamicKeyManager(File certFile, File keyFile) { } catch (Exception e) { throw new RuntimeException("Failed to initialize client certificate/key", e); } - this.lastCheckedMs = System.currentTimeMillis(); + this.lastCheckedNs = System.nanoTime(); } void checkAndReload() { - long now = System.currentTimeMillis(); - if (checkIntervalMs > 0 && now - lastCheckedMs < checkIntervalMs) { + long now = System.nanoTime(); + if (checkIntervalNs > 0 && now - lastCheckedNs < checkIntervalNs) { return; } KeyMaterial existing = this.currentMaterial; @@ -136,11 +138,12 @@ void checkAndReload() { && certFile.length() == existing.certLength && keyFile.lastModified() == existing.keyLastModified && keyFile.length() == existing.keyLength) { - lastCheckedMs = now; + lastCheckedNs = now; return; } - synchronized (this) { - if (checkIntervalMs > 0 && now - lastCheckedMs < checkIntervalMs) { + lock.lock(); + try { + if (checkIntervalNs > 0 && now - lastCheckedNs < checkIntervalNs) { return; } existing = this.currentMaterial; @@ -149,7 +152,7 @@ void checkAndReload() { && certFile.length() == existing.certLength && keyFile.lastModified() == existing.keyLastModified && keyFile.length() == existing.keyLength) { - lastCheckedMs = now; + lastCheckedNs = now; return; } try { @@ -160,8 +163,10 @@ void checkAndReload() { "Failed to reload rotated client certificate/key from disk, retaining current material", e); } finally { - lastCheckedMs = now; + lastCheckedNs = now; } + } finally { + lock.unlock(); } } diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java index e66e3342cba3..0197372013cd 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java @@ -28,6 +28,7 @@ import java.security.cert.CertificateFactory; import java.security.cert.X509Certificate; import java.util.Collection; +import java.util.concurrent.locks.ReentrantLock; import java.util.logging.Level; import java.util.logging.Logger; import javax.annotation.Nullable; @@ -47,8 +48,9 @@ public class DynamicTrustManager extends X509ExtendedTrustManager { private static final long DEFAULT_CHECK_INTERVAL_MS = 5000L; private final File caCertFile; - private final long checkIntervalMs; - private volatile long lastCheckedMs; + private final long checkIntervalNs; + private final ReentrantLock lock = new ReentrantLock(); + private volatile long lastCheckedNs; private static class CertificateFactoryHolder { static final CertificateFactory INSTANCE; @@ -90,39 +92,40 @@ public DynamicTrustManager(@Nullable File caCertFile) { DynamicTrustManager(@Nullable File caCertFile, long checkIntervalMs) { this.caCertFile = caCertFile; - this.checkIntervalMs = checkIntervalMs; + this.checkIntervalNs = checkIntervalMs * 1_000_000L; try { reloadMaterial(); } catch (Exception e) { throw new RuntimeException("Failed to initialize CA certificate", e); } - this.lastCheckedMs = System.currentTimeMillis(); + this.lastCheckedNs = System.nanoTime(); } void checkAndReload() { if (this.caCertFile == null) { return; } - long now = System.currentTimeMillis(); - if (checkIntervalMs > 0 && now - lastCheckedMs < checkIntervalMs) { + long now = System.nanoTime(); + if (checkIntervalNs > 0 && now - lastCheckedNs < checkIntervalNs) { return; } TrustMaterial existing = this.currentMaterial; if (existing != null && caCertFile.lastModified() == existing.lastModified && caCertFile.length() == existing.length) { - lastCheckedMs = now; + lastCheckedNs = now; return; } - synchronized (this) { - if (checkIntervalMs > 0 && now - lastCheckedMs < checkIntervalMs) { + lock.lock(); + try { + if (checkIntervalNs > 0 && now - lastCheckedNs < checkIntervalNs) { return; } existing = this.currentMaterial; if (existing != null && caCertFile.lastModified() == existing.lastModified && caCertFile.length() == existing.length) { - lastCheckedMs = now; + lastCheckedNs = now; return; } try { @@ -133,8 +136,10 @@ void checkAndReload() { "Failed to reload rotated CA certificate from disk, retaining previous material", e); } finally { - lastCheckedMs = now; + lastCheckedNs = now; } + } finally { + lock.unlock(); } } From c0318258724474a7b9af5e9debc75f49d4d0269f Mon Sep 17 00:00:00 2001 From: sagnghos Date: Mon, 21 Sep 2026 11:05:52 +0000 Subject: [PATCH 12/22] Store certificate and key paths in SpannerOptions and Builder --- .../google/cloud/spanner/SpannerOptions.java | 63 +++++++++++++------ .../cloud/spanner/SpannerOptionsTest.java | 11 ++++ 2 files changed, 56 insertions(+), 18 deletions(-) diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index caa8f19c2357..94080ac2fd6a 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -359,7 +359,9 @@ static GcpChannelPoolOptions mergeWithDefaultChannelPoolOptions( private final boolean autoTaggingEnabled; private final List autoTaggingPackages; private final int autoTaggingTracerLimit; - private final SslContext omniSslContext; + private final String clientCertificate; + private final String clientCertificateKey; + private final String caCertificate; enum TracingFramework { OPEN_CENSUS, @@ -945,7 +947,9 @@ protected SpannerOptions(Builder builder) { transportChannelExecutorThreadNameFormat = builder.transportChannelExecutorThreadNameFormat; channelProvider = builder.channelProvider; channelEndpointCacheFactory = builder.channelEndpointCacheFactory; - omniSslContext = builder.omniSslContext; + clientCertificate = builder.clientCertificate; + clientCertificateKey = builder.clientCertificateKey; + caCertificate = builder.caCertificate; if (builder.omniSslContext != null) { final SslContext sslContext = builder.omniSslContext; @SuppressWarnings("rawtypes") @@ -1301,9 +1305,19 @@ public GoogleCredentials getDefaultSpannerOmniCredentials() { public static class Builder extends ServiceOptions.Builder { private static Builder prepareBuilder(Builder builder) { - if (builder.sslContextBuilder != null) { + if (builder.clientCertificate != null || builder.caCertificate != null) { try { - builder.omniSslContext = builder.sslContextBuilder.build(); + SslContextBuilder sslContextBuilder = GrpcSslContexts.forClient(); + if (builder.clientCertificate != null && builder.clientCertificateKey != null) { + sslContextBuilder.keyManager( + new DynamicKeyManager( + new File(builder.clientCertificate), new File(builder.clientCertificateKey))); + } + if (builder.caCertificate != null) { + sslContextBuilder.trustManager( + new DynamicTrustManager(new File(builder.caCertificate))); + } + builder.omniSslContext = sslContextBuilder.build(); } catch (Exception e) { throw SpannerExceptionFactory.asSpannerException(e); } @@ -1415,7 +1429,9 @@ private static Builder prepareBuilder(Builder builder) { private MetricsProvider metricsProvider = DefaultMetricsProvider.INSTANCE; private boolean enableLocationApi = SpannerOptions.environment.isEnableLocationApi(); private String monitoringHost = SpannerOptions.environment.getMonitoringHost(); - private SslContextBuilder sslContextBuilder = null; + private String clientCertificate = null; + private String clientCertificateKey = null; + private String caCertificate = null; private SslContext omniSslContext = null; private boolean usePlainText = false; private TransactionOptions defaultTransactionOptions = TransactionOptions.getDefaultInstance(); @@ -1534,7 +1550,9 @@ protected Builder() { this.autoTaggingEnabled = options.autoTaggingEnabled; this.autoTaggingPackages = options.autoTaggingPackages; this.autoTaggingTracerLimit = options.autoTaggingTracerLimit; - this.omniSslContext = options.omniSslContext; + this.clientCertificate = options.clientCertificate; + this.clientCertificateKey = options.clientCertificateKey; + this.caCertificate = options.caCertificate; } @Override @@ -2265,13 +2283,10 @@ public Builder setEmulatorHost(String emulatorHost) { * @param clientCertificateKey Path to the client private key file. */ public Builder useClientCert(String clientCertificate, String clientCertificateKey) { - Preconditions.checkNotNull(clientCertificate, "clientCertificate cannot be null"); - Preconditions.checkNotNull(clientCertificateKey, "clientCertificateKey cannot be null"); - if (this.sslContextBuilder == null) { - this.sslContextBuilder = GrpcSslContexts.forClient(); - } - this.sslContextBuilder.keyManager( - new DynamicKeyManager(new File(clientCertificate), new File(clientCertificateKey))); + this.clientCertificate = + Preconditions.checkNotNull(clientCertificate, "clientCertificate cannot be null"); + this.clientCertificateKey = + Preconditions.checkNotNull(clientCertificateKey, "clientCertificateKey cannot be null"); return this; } @@ -2282,11 +2297,8 @@ public Builder useClientCert(String clientCertificate, String clientCertificateK * @param caCertificate Path to the server root CA certificate file. */ public Builder setCaCertificate(String caCertificate) { - Preconditions.checkNotNull(caCertificate, "caCertificate cannot be null"); - if (this.sslContextBuilder == null) { - this.sslContextBuilder = GrpcSslContexts.forClient(); - } - this.sslContextBuilder.trustManager(new DynamicTrustManager(new File(caCertificate))); + this.caCertificate = + Preconditions.checkNotNull(caCertificate, "caCertificate cannot be null"); return this; } @@ -3207,6 +3219,21 @@ protected boolean shouldRefreshRpc(ServiceRpc cachedRpc) { return cachedRpc == null || ((SpannerRpc) cachedRpc).isClosed(); } + @Nullable + public String getClientCertificate() { + return clientCertificate; + } + + @Nullable + public String getClientCertificateKey() { + return clientCertificateKey; + } + + @Nullable + public String getCaCertificate() { + return caCertificate; + } + @SuppressWarnings("unchecked") @Override public Builder toBuilder() { diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java index f484b7e2e3b6..5f96ae07933a 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java @@ -1706,9 +1706,15 @@ public void testUseClientCertAndTrustCertificate() throws Exception { .build(); assertNotNull(options.getChannelConfigurator()); + assertEquals(certPath, options.getClientCertificate()); + assertEquals(keyPath, options.getClientCertificateKey()); + assertEquals(caPath, options.getCaCertificate()); SpannerOptions fromBuilder = options.toBuilder().build(); assertNotNull(fromBuilder.getChannelConfigurator()); + assertEquals(certPath, fromBuilder.getClientCertificate()); + assertEquals(keyPath, fromBuilder.getClientCertificateKey()); + assertEquals(caPath, fromBuilder.getCaCertificate()); // Test standalone setCaCertificate SpannerOptions caOnlyOptions = @@ -1720,6 +1726,9 @@ public void testUseClientCertAndTrustCertificate() throws Exception { .build(); assertNotNull(caOnlyOptions.getChannelConfigurator()); + assertNull(caOnlyOptions.getClientCertificate()); + assertNull(caOnlyOptions.getClientCertificateKey()); + assertEquals(caPath, caOnlyOptions.getCaCertificate()); // Test setCaCertificate combined with login (username/password) SpannerOptions loginWithCaOptions = @@ -1733,10 +1742,12 @@ public void testUseClientCertAndTrustCertificate() throws Exception { assertTrue(loginWithCaOptions.getCredentials() instanceof SpannerOmniCredentials); assertNotNull(loginWithCaOptions.getChannelConfigurator()); + assertEquals(caPath, loginWithCaOptions.getCaCertificate()); SpannerOptions loginFromBuilder = loginWithCaOptions.toBuilder().build(); assertTrue(loginFromBuilder.getCredentials() instanceof SpannerOmniCredentials); assertNotNull(loginFromBuilder.getChannelConfigurator()); + assertEquals(caPath, loginFromBuilder.getCaCertificate()); } finally { ssc.delete(); ca.delete(); From 0b61f6eba0c3c44679bbcfaa1e2c3fbe94f2ee7e Mon Sep 17 00:00:00 2001 From: sagnghos Date: Mon, 21 Sep 2026 11:13:56 +0000 Subject: [PATCH 13/22] Validate client certificate and key are both provided in prepareBuilder --- .../java/com/google/cloud/spanner/SpannerOptions.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index 94080ac2fd6a..ba7433b49107 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -1305,10 +1305,16 @@ public GoogleCredentials getDefaultSpannerOmniCredentials() { public static class Builder extends ServiceOptions.Builder { private static Builder prepareBuilder(Builder builder) { - if (builder.clientCertificate != null || builder.caCertificate != null) { + if (builder.clientCertificate != null + || builder.clientCertificateKey != null + || builder.caCertificate != null) { + if ((builder.clientCertificate == null) != (builder.clientCertificateKey == null)) { + throw new IllegalArgumentException( + "Both clientCertificate and clientCertificateKey must be provided together"); + } try { SslContextBuilder sslContextBuilder = GrpcSslContexts.forClient(); - if (builder.clientCertificate != null && builder.clientCertificateKey != null) { + if (builder.clientCertificate != null) { sslContextBuilder.keyManager( new DynamicKeyManager( new File(builder.clientCertificate), new File(builder.clientCertificateKey))); From 8e80a78d59fe78a3de90a2397193b469432fa82a Mon Sep 17 00:00:00 2001 From: sagnghos Date: Mon, 21 Sep 2026 11:30:23 +0000 Subject: [PATCH 14/22] Use Strings.isNullOrEmpty for certificate and key validation --- .../google/cloud/spanner/SpannerOptions.java | 30 +++++++++++-------- .../cloud/spanner/SpannerOptionsTest.java | 20 +++++++++++++ 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index ba7433b49107..8e139a3c000e 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -1305,21 +1305,23 @@ public GoogleCredentials getDefaultSpannerOmniCredentials() { public static class Builder extends ServiceOptions.Builder { private static Builder prepareBuilder(Builder builder) { - if (builder.clientCertificate != null - || builder.clientCertificateKey != null - || builder.caCertificate != null) { - if ((builder.clientCertificate == null) != (builder.clientCertificateKey == null)) { + boolean hasClientCert = !Strings.isNullOrEmpty(builder.clientCertificate); + boolean hasClientKey = !Strings.isNullOrEmpty(builder.clientCertificateKey); + boolean hasCaCert = !Strings.isNullOrEmpty(builder.caCertificate); + + if (hasClientCert || hasClientKey || hasCaCert) { + if (hasClientCert != hasClientKey) { throw new IllegalArgumentException( "Both clientCertificate and clientCertificateKey must be provided together"); } try { SslContextBuilder sslContextBuilder = GrpcSslContexts.forClient(); - if (builder.clientCertificate != null) { + if (hasClientCert) { sslContextBuilder.keyManager( new DynamicKeyManager( new File(builder.clientCertificate), new File(builder.clientCertificateKey))); } - if (builder.caCertificate != null) { + if (hasCaCert) { sslContextBuilder.trustManager( new DynamicTrustManager(new File(builder.caCertificate))); } @@ -2289,10 +2291,13 @@ public Builder setEmulatorHost(String emulatorHost) { * @param clientCertificateKey Path to the client private key file. */ public Builder useClientCert(String clientCertificate, String clientCertificateKey) { - this.clientCertificate = - Preconditions.checkNotNull(clientCertificate, "clientCertificate cannot be null"); - this.clientCertificateKey = - Preconditions.checkNotNull(clientCertificateKey, "clientCertificateKey cannot be null"); + Preconditions.checkArgument( + !Strings.isNullOrEmpty(clientCertificate), "clientCertificate cannot be null or empty"); + Preconditions.checkArgument( + !Strings.isNullOrEmpty(clientCertificateKey), + "clientCertificateKey cannot be null or empty"); + this.clientCertificate = clientCertificate; + this.clientCertificateKey = clientCertificateKey; return this; } @@ -2303,8 +2308,9 @@ public Builder useClientCert(String clientCertificate, String clientCertificateK * @param caCertificate Path to the server root CA certificate file. */ public Builder setCaCertificate(String caCertificate) { - this.caCertificate = - Preconditions.checkNotNull(caCertificate, "caCertificate cannot be null"); + Preconditions.checkArgument( + !Strings.isNullOrEmpty(caCertificate), "caCertificate cannot be null or empty"); + this.caCertificate = caCertificate; return this; } diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java index 5f96ae07933a..dcdc4f428068 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java @@ -1753,4 +1753,24 @@ public void testUseClientCertAndTrustCertificate() throws Exception { ca.delete(); } } + + @Test + public void testUseClientCertAndCaCertificateEmptyValidation() { + assertThrows( + IllegalArgumentException.class, + () -> SpannerOptions.newBuilder().useClientCert("", "/path/to/key")); + assertThrows( + IllegalArgumentException.class, + () -> SpannerOptions.newBuilder().useClientCert("/path/to/cert", "")); + assertThrows( + IllegalArgumentException.class, + () -> SpannerOptions.newBuilder().useClientCert(null, "key")); + assertThrows( + IllegalArgumentException.class, + () -> SpannerOptions.newBuilder().useClientCert("cert", null)); + assertThrows( + IllegalArgumentException.class, () -> SpannerOptions.newBuilder().setCaCertificate("")); + assertThrows( + IllegalArgumentException.class, () -> SpannerOptions.newBuilder().setCaCertificate(null)); + } } From 0789fb9c42e765c2a5bbf12835da88f3fdda61a6 Mon Sep 17 00:00:00 2001 From: sagnghos Date: Mon, 21 Sep 2026 11:41:15 +0000 Subject: [PATCH 15/22] Use lock.tryLock() in DynamicKeyManager and DynamicTrustManager to prevent thread blocking --- .../google/cloud/spanner/omni/DynamicKeyManager.java | 11 +++++++---- .../cloud/spanner/omni/DynamicTrustManager.java | 11 +++++++---- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java index 9c46b8909f3e..cbc428c189b1 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java @@ -141,9 +141,12 @@ void checkAndReload() { lastCheckedNs = now; return; } - lock.lock(); + if (!lock.tryLock()) { + return; + } try { - if (checkIntervalNs > 0 && now - lastCheckedNs < checkIntervalNs) { + long nowInLock = System.nanoTime(); + if (checkIntervalNs > 0 && nowInLock - lastCheckedNs < checkIntervalNs) { return; } existing = this.currentMaterial; @@ -152,7 +155,7 @@ void checkAndReload() { && certFile.length() == existing.certLength && keyFile.lastModified() == existing.keyLastModified && keyFile.length() == existing.keyLength) { - lastCheckedNs = now; + lastCheckedNs = nowInLock; return; } try { @@ -163,7 +166,7 @@ void checkAndReload() { "Failed to reload rotated client certificate/key from disk, retaining current material", e); } finally { - lastCheckedNs = now; + lastCheckedNs = System.nanoTime(); } } finally { lock.unlock(); diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java index 0197372013cd..6de4c8dc67ab 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java @@ -116,16 +116,19 @@ void checkAndReload() { lastCheckedNs = now; return; } - lock.lock(); + if (!lock.tryLock()) { + return; + } try { - if (checkIntervalNs > 0 && now - lastCheckedNs < checkIntervalNs) { + long nowInLock = System.nanoTime(); + if (checkIntervalNs > 0 && nowInLock - lastCheckedNs < checkIntervalNs) { return; } existing = this.currentMaterial; if (existing != null && caCertFile.lastModified() == existing.lastModified && caCertFile.length() == existing.length) { - lastCheckedNs = now; + lastCheckedNs = nowInLock; return; } try { @@ -136,7 +139,7 @@ void checkAndReload() { "Failed to reload rotated CA certificate from disk, retaining previous material", e); } finally { - lastCheckedNs = now; + lastCheckedNs = System.nanoTime(); } } finally { lock.unlock(); From c3ddcc72f220af7359353852922dda50804d8ef7 Mon Sep 17 00:00:00 2001 From: sagnghos Date: Mon, 21 Sep 2026 11:49:09 +0000 Subject: [PATCH 16/22] Perform file attribute checks only after acquiring lock in DynamicKeyManager and DynamicTrustManager --- .../google/cloud/spanner/omni/DynamicKeyManager.java | 11 +---------- .../cloud/spanner/omni/DynamicTrustManager.java | 9 +-------- 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java index cbc428c189b1..206180d07215 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java @@ -132,15 +132,6 @@ void checkAndReload() { if (checkIntervalNs > 0 && now - lastCheckedNs < checkIntervalNs) { return; } - KeyMaterial existing = this.currentMaterial; - if (existing != null - && certFile.lastModified() == existing.certLastModified - && certFile.length() == existing.certLength - && keyFile.lastModified() == existing.keyLastModified - && keyFile.length() == existing.keyLength) { - lastCheckedNs = now; - return; - } if (!lock.tryLock()) { return; } @@ -149,7 +140,7 @@ void checkAndReload() { if (checkIntervalNs > 0 && nowInLock - lastCheckedNs < checkIntervalNs) { return; } - existing = this.currentMaterial; + KeyMaterial existing = this.currentMaterial; if (existing != null && certFile.lastModified() == existing.certLastModified && certFile.length() == existing.certLength diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java index 6de4c8dc67ab..311cb1cb22d2 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java @@ -109,13 +109,6 @@ void checkAndReload() { if (checkIntervalNs > 0 && now - lastCheckedNs < checkIntervalNs) { return; } - TrustMaterial existing = this.currentMaterial; - if (existing != null - && caCertFile.lastModified() == existing.lastModified - && caCertFile.length() == existing.length) { - lastCheckedNs = now; - return; - } if (!lock.tryLock()) { return; } @@ -124,7 +117,7 @@ void checkAndReload() { if (checkIntervalNs > 0 && nowInLock - lastCheckedNs < checkIntervalNs) { return; } - existing = this.currentMaterial; + TrustMaterial existing = this.currentMaterial; if (existing != null && caCertFile.lastModified() == existing.lastModified && caCertFile.length() == existing.length) { From 188664937022b68a70e517b1809bf1e31c947906 Mon Sep 17 00:00:00 2001 From: sagnghos Date: Mon, 21 Sep 2026 11:56:42 +0000 Subject: [PATCH 17/22] Use removeIf on materials.keySet() in DynamicKeyManager --- .../cloud/spanner/omni/DynamicKeyManager.java | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java index 206180d07215..03ccd0239680 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java @@ -184,16 +184,17 @@ private void reloadMaterial() throws Exception { this.currentMaterial = newMaterial; long oldestToKeep = currentVersion - 10; - for (String keyStr : materials.keySet()) { - try { - long ver = Long.parseLong(keyStr.substring("client-".length())); - if (ver < oldestToKeep) { - materials.remove(keyStr); - } - } catch (Exception ignored) { - materials.remove(keyStr); - } - } + materials + .keySet() + .removeIf( + keyStr -> { + try { + long ver = Long.parseLong(keyStr.substring("client-".length())); + return ver < oldestToKeep; + } catch (Exception e) { + return true; + } + }); } private static void verifyKeyMatch(PublicKey publicKey, PrivateKey privateKey) From 4da5bf94decbd42139fccc99c514cdaad63a5078 Mon Sep 17 00:00:00 2001 From: sagnghos Date: Mon, 21 Sep 2026 12:07:55 +0000 Subject: [PATCH 18/22] Optimize parsePrivateKey and extractPemContent in DynamicKeyManager --- .../cloud/spanner/omni/DynamicKeyManager.java | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java index 03ccd0239680..f1674b1700a7 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java @@ -230,23 +230,25 @@ private static X509Certificate[] parseCertificates(byte[] certBytes) throws Cert } private static PrivateKey parsePrivateKey(byte[] keyBytes) throws Exception { - String keyStr = new String(keyBytes, StandardCharsets.UTF_8); - if (keyStr.contains("-----BEGIN RSA PRIVATE KEY-----") - || keyStr.contains("-----BEGIN EC PRIVATE KEY-----")) { - throw new IllegalArgumentException( - "PKCS#1 private keys are not supported. Please convert your key to PKCS#8 format using: " - + "openssl pkcs8 -topk8 -nocrypt -in -out "); - } byte[] der; - if (keyStr.contains("-----BEGIN PRIVATE KEY-----")) { - der = extractPemContent(keyStr, "-----BEGIN PRIVATE KEY-----", "-----END PRIVATE KEY-----"); - } else if (keyBytes.length > 0 && keyBytes[0] == 0x30) { + if (keyBytes.length > 0 && keyBytes[0] == 0x30) { der = keyBytes; } else { - try { - der = Base64.getMimeDecoder().decode(keyBytes); - } catch (IllegalArgumentException e) { - der = keyBytes; + String keyStr = new String(keyBytes, StandardCharsets.UTF_8); + if (keyStr.contains("-----BEGIN RSA PRIVATE KEY-----") + || keyStr.contains("-----BEGIN EC PRIVATE KEY-----")) { + throw new IllegalArgumentException( + "PKCS#1 private keys are not supported. Please convert your key to PKCS#8 format using: " + + "openssl pkcs8 -topk8 -nocrypt -in -out "); + } + if (keyStr.contains("-----BEGIN PRIVATE KEY-----")) { + der = extractPemContent(keyStr, "-----BEGIN PRIVATE KEY-----", "-----END PRIVATE KEY-----"); + } else { + try { + der = Base64.getMimeDecoder().decode(keyBytes); + } catch (IllegalArgumentException e) { + der = keyBytes; + } } } @@ -276,8 +278,8 @@ private static byte[] extractPemContent(String pem, String beginMarker, String e if (end < 0) { throw new IllegalArgumentException("PEM does not contain marker: " + endMarker); } - String base64 = pem.substring(start, end).replaceAll("\\s+", ""); - return Base64.getDecoder().decode(base64); + String base64 = pem.substring(start, end); + return Base64.getMimeDecoder().decode(base64); } @Override From 27d701fd3c4fa4a40029cb53e530b43e052e5864 Mon Sep 17 00:00:00 2001 From: sagnghos Date: Mon, 21 Sep 2026 12:15:04 +0000 Subject: [PATCH 19/22] Safely evict key material aliases in DynamicKeyManager --- .../com/google/cloud/spanner/omni/DynamicKeyManager.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java index f1674b1700a7..efb0d9fc8224 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicKeyManager.java @@ -188,11 +188,14 @@ private void reloadMaterial() throws Exception { .keySet() .removeIf( keyStr -> { + if (!keyStr.startsWith("client-")) { + return false; + } try { long ver = Long.parseLong(keyStr.substring("client-".length())); return ver < oldestToKeep; - } catch (Exception e) { - return true; + } catch (NumberFormatException e) { + return false; } }); } From cacf044569d014b67589a4d9f91ddc1e6caf8edd Mon Sep 17 00:00:00 2001 From: sagnghos Date: Mon, 21 Sep 2026 12:27:30 +0000 Subject: [PATCH 20/22] Safely handle null issuers in DynamicTrustManager and speed up tests using setLastModified --- .../cloud/spanner/omni/DynamicTrustManager.java | 6 +++++- .../cloud/spanner/omni/DynamicKeyManagerTest.java | 11 ++++------- .../cloud/spanner/omni/DynamicTrustManagerTest.java | 6 ++---- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java index 311cb1cb22d2..da7ffc1f3fd7 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java @@ -300,6 +300,10 @@ public void checkServerTrusted(X509Certificate[] chain, String authType) public X509Certificate[] getAcceptedIssuers() { checkAndReload(); TrustMaterial mat = this.currentMaterial; - return mat != null ? mat.delegate.getAcceptedIssuers() : new X509Certificate[0]; + if (mat == null) { + return new X509Certificate[0]; + } + X509Certificate[] issuers = mat.delegate.getAcceptedIssuers(); + return issuers != null ? issuers : new X509Certificate[0]; } } diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java index 74c0a8428df2..e94d21dcbf55 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicKeyManagerTest.java @@ -70,11 +70,10 @@ public void testInitialLoadAndDynamicRotation() throws Exception { assertEquals(1, aliases1.length); assertEquals(alias1, aliases1[0]); - // Ensure lastModified timestamp changes upon rotation - Thread.sleep(1100); - Files.write(certFile.toPath(), Files.readAllBytes(ssc2.certificate().toPath())); Files.write(keyFile.toPath(), Files.readAllBytes(ssc2.privateKey().toPath())); + certFile.setLastModified(System.currentTimeMillis() + 2000L); + keyFile.setLastModified(System.currentTimeMillis() + 2000L); String alias2 = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); assertNotNull(alias2); @@ -172,10 +171,9 @@ public void testCorruptRotationFallsBackToPrevious() throws Exception { String aliasBefore = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); assertNotNull(aliasBefore); - Thread.sleep(1100); - // Overwrite certFile with corrupt bytes Files.write(certFile.toPath(), "NOT A CERTIFICATE CONTENT".getBytes(StandardCharsets.UTF_8)); + certFile.setLastModified(System.currentTimeMillis() + 2000L); // DynamicKeyManager should catch reload error and retain previous material String aliasAfter = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); @@ -278,10 +276,9 @@ public void testMismatchedRotationFallsBackToPrevious() throws Exception { assertEquals( ssc1.cert().getSubjectDN(), keyManager.getCertificateChain(alias1)[0].getSubjectDN()); - Thread.sleep(1100); - // Rotate only cert file (e.g., intermediate state during rotation) Files.write(certFile.toPath(), Files.readAllBytes(ssc2.certificate().toPath())); + certFile.setLastModified(System.currentTimeMillis() + 2000L); // Key manager should detect mismatch and retain ssc1 credentials String aliasAfter = keyManager.chooseClientAlias(new String[] {"RSA"}, null, null); diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicTrustManagerTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicTrustManagerTest.java index 05e8c145fe7f..99cfe3878071 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicTrustManagerTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/omni/DynamicTrustManagerTest.java @@ -69,10 +69,9 @@ public void testCustomTrustManagerAndDynamicRotation() throws Exception { CertificateException.class, () -> trustManager.checkServerTrusted(new X509Certificate[] {ca2.cert()}, "RSA")); - Thread.sleep(1100); - // Rotate CA file on disk to ca2 Files.write(caFile.toPath(), Files.readAllBytes(ca2.certificate().toPath())); + caFile.setLastModified(System.currentTimeMillis() + 2000L); // Now ca2 should be accepted and ca1 should be rejected X509Certificate[] issuers2 = trustManager.getAcceptedIssuers(); @@ -154,10 +153,9 @@ public void testCorruptRotationFallsBackToPrevious() throws Exception { DynamicTrustManager trustManager = new DynamicTrustManager(caFile, 0L); trustManager.checkServerTrusted(new X509Certificate[] {ca.cert()}, "RSA"); - Thread.sleep(1100); - // Corrupt the file Files.write(caFile.toPath(), "CORRUPT CERT DATA".getBytes(StandardCharsets.UTF_8)); + caFile.setLastModified(System.currentTimeMillis() + 2000L); // Trust manager should retain previous CA trustManager.checkServerTrusted(new X509Certificate[] {ca.cert()}, "RSA"); From a376228f633000b93a8f58d98c372e8a45475361 Mon Sep 17 00:00:00 2001 From: sagnghos Date: Mon, 21 Sep 2026 12:35:16 +0000 Subject: [PATCH 21/22] Use standard JKS keystore in DynamicTrustManager --- .../java/com/google/cloud/spanner/omni/DynamicTrustManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java index da7ffc1f3fd7..e861a52c8372 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java @@ -159,7 +159,7 @@ private void reloadMaterial() throws Exception { throw new CertificateException("No certificates found in CA certificate file"); } - KeyStore ks = KeyStore.getInstance(KeyStore.getDefaultType()); + KeyStore ks = KeyStore.getInstance("JKS"); ks.load(null, null); int index = 0; for (Certificate cert : certs) { From 8d9c73746fbefa2457ecabb41375e6dd893cd22b Mon Sep 17 00:00:00 2001 From: sagnghos Date: Mon, 21 Sep 2026 12:44:59 +0000 Subject: [PATCH 22/22] Use KeyStore.getDefaultType() and clone accepted issuers in DynamicTrustManager --- .../com/google/cloud/spanner/omni/DynamicTrustManager.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java index e861a52c8372..2d5d1bd5b0fe 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/DynamicTrustManager.java @@ -159,7 +159,7 @@ private void reloadMaterial() throws Exception { throw new CertificateException("No certificates found in CA certificate file"); } - KeyStore ks = KeyStore.getInstance("JKS"); + KeyStore ks = KeyStore.getInstance(KeyStore.getDefaultType()); ks.load(null, null); int index = 0; for (Certificate cert : certs) { @@ -304,6 +304,6 @@ public X509Certificate[] getAcceptedIssuers() { return new X509Certificate[0]; } X509Certificate[] issuers = mat.delegate.getAcceptedIssuers(); - return issuers != null ? issuers : new X509Certificate[0]; + return issuers != null ? issuers.clone() : new X509Certificate[0]; } }