Skip to content

feat(spanner): Support dynamic certificate and key rotation in Spanner Omni - #14433

Draft
sagnghos wants to merge 1 commit into
googleapis:mainfrom
sagnghos:sagnghos/dynamicRotation
Draft

sagnghos wants to merge 1 commit into
googleapis:mainfrom
sagnghos:sagnghos/dynamicRotation

Conversation

@sagnghos

Copy link
Copy Markdown
Contributor

Summary

This PR adds support for zero-downtime dynamic reloading of client certificates/keys (mTLS) and server root CA certificates in Spanner Omni without requiring application or connection pool restarts.

Changes

  • DynamicKeyManager (com.google.cloud.spanner.omni): An X509ExtendedKeyManager that monitors file modification timestamps and lengths, dynamically reloading rotated client certificates and RSA/EC private keys.
  • DynamicTrustManager (com.google.cloud.spanner.omni): An X509ExtendedTrustManager that dynamically reloads updated server root CA certificates into an in-memory keystore/trust manager upon file changes.
  • SpannerOptions & Connection API:
    • Added Builder.setCaCertificate(String caCertificate) and getCaCertificate() across SpannerOptions, ConnectionProperties, ConnectionOptions, and SpannerPool.
    • Updated Builder.useClientCert(String, String) to use dynamic key management.
  • SpannerOmniHelper: Added support for spanner.ca_cert_path and updated mTLS setup detection when client certificates are provided.
  • Testing: Added unit tests covering dynamic certificate/key rotation, CA rotation, multi-CA bundles, fallback handling, and options configuration.

Fixes b/562755231

@sagnghos
sagnghos requested review from a team as code owners September 18, 2026 11:14

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces dynamic loading and automatic reloading of client certificates, private keys, and server root CA certificates for Spanner Omni instances by implementing DynamicKeyManager and DynamicTrustManager. It also updates SpannerOptions, ConnectionOptions, and related classes to support the new caCertificate configuration. Feedback on these changes highlights critical performance concerns regarding blocking file I/O operations performed on every TLS handshake or trust check, which could block Netty's EventLoop threads; throttling these checks is recommended. Additionally, it is advised to remove the direct dependency on BouncyCastle in DynamicKeyManager to prevent classpath conflicts, relying instead on standard Java APIs for PKCS#8 private keys.

Comment on lines +79 to +114
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);
}
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Performing blocking file I/O (File.lastModified() and File.length()) on every single TLS handshake/alias selection can severely degrade performance and throughput, especially under high connection concurrency. Furthermore, in gRPC Netty, these calls may execute on Netty's EventLoop threads, where blocking I/O is a major anti-pattern that can lead to thread starvation.

Consider throttling the file checks (e.g., checking at most once every 5 seconds) to minimize disk I/O overhead.

  private static final long CHECK_INTERVAL_MS = 5000;
  private volatile long lastCheckedMs;
  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();
    this.lastCheckedMs = System.currentTimeMillis();
  }

  private void checkAndReload() {
    long now = System.currentTimeMillis();
    if (now - lastCheckedMs < CHECK_INTERVAL_MS) {
      return;
    }
    lastCheckedMs = now;
    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);
      }
    }
  }

Comment on lines +62 to +95
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);
}
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Performing blocking file I/O (File.lastModified() and File.length()) on every single TLS handshake/trust check can severely degrade performance and throughput, especially under high connection concurrency. Furthermore, in gRPC Netty, these calls may execute on Netty's EventLoop threads, where blocking I/O is a major anti-pattern that can lead to thread starvation.

Consider throttling the file checks (e.g., checking at most once every 5 seconds) to minimize disk I/O overhead.

  private static final long CHECK_INTERVAL_MS = 5000;
  private volatile long lastCheckedMs;
  private volatile TrustMaterial currentMaterial;

  public DynamicTrustManager(@Nullable File caCertFile) {
    this.caCertFile = caCertFile;
    reloadMaterial();
    this.lastCheckedMs = System.currentTimeMillis();
  }

  private void checkAndReload() {
    if (this.caCertFile == null) {
      return;
    }
    long now = System.currentTimeMillis();
    if (now - lastCheckedMs < CHECK_INTERVAL_MS) {
      return;
    }
    lastCheckedMs = now;
    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);
      }
    }
  }

Comment on lines +152 to +189
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);
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Directly depending on BouncyCastle (org.bouncycastle.asn1.pkcs.RSAPrivateKey) can introduce classpath conflicts and runtime NoClassDefFoundError if BouncyCastle is not present on the application's classpath. Since PKCS#8 is the standard and recommended format for Java private keys, we should avoid BouncyCastle and only support PKCS#8 format natively using standard Java APIs. Please also remove the BouncyCastle import on line 41.

  private static PrivateKey parsePrivateKey(byte[] keyBytes) throws Exception {
    String keyStr = new String(keyBytes, StandardCharsets.US_ASCII);
    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);
    }
  }

@sagnghos
sagnghos force-pushed the sagnghos/dynamicRotation branch from bb6716b to fafa3ac Compare September 18, 2026 11:19
@sagnghos
sagnghos marked this pull request as draft September 18, 2026 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant