CTM-679: mutual TLS for Galaxy VM connections by LizBaldo · Pull Request #4934 · DataBiosphere/leonardo

LizBaldo · 2026-10-02T13:38:29Z

Establishes mutual TLS between Leo and Galaxy VMs.

Server authentication (Leo → VM)
Leo validates each Galaxy VM's TLS certificate against a CA. At provisioning time, Leo signs a fresh RSA key pair for the VM using its CA key and injects the cert+key into GCE instance metadata as galaxy_tls_cert / galaxy_tls_key. The galaxy-k8s-boot playbook reads these and configures nginx to serve them (galaxyproject/galaxy-k8s-boot#144).

Client authentication (VM → Leo)
The Galaxy VM verifies that the connecting client is Leo. Leo injects galaxy_tls_client_ca (the CA cert) into VM metadata so nginx can verify Leo's client cert. Leo presents GALAXY_TLS_LEO_CLIENT_CERT / GALAXY_TLS_LEO_CLIENT_KEY on every connection via a KeyManager in its SSLContext.

Startup behaviour when GALAXY_TLS_CA_CERT is absent
GALAXY_TLS_CA_CERT is optional at startup. If absent (e.g. BEEs before the Terraform secret is deployed), Leo logs a warning and falls back to SSLContext.getDefault. Leo starts normally; Galaxy VM connections fail at the TLS handshake per-request rather than blocking startup. Once the Terraform and Helmfile PRs are deployed the cert is present and full mTLS is enforced.

Dependencies

BouncyCastle bcpkix-jdk18on 1.86 for per-VM cert generation (added explicitly; global exclusion rule removed)
Terraform PR broadinstitute/terraform-ap-deployments#2074
Helmfile PR broadinstitute/terra-helmfile#6508
galaxy-k8s-boot PR Serve a launcher-injected TLS certificate as the ingress default galaxyproject/galaxy-k8s-boot#144 (server TLS) + issue SPIKE: Remove code duplication in unit tests #150 (mTLS)

Replaces trust-all with a CA-pinned SSLContext when GALAXY_TLS_CA_CERT, GALAXY_TLS_VM_CERT and GALAXY_TLS_VM_KEY env vars are set (from Secret Manager). Falls back to trust-all when absent (BEE / dev without provisioned secret). Changes: - GalaxyVmConfig gains tlsCaCert/tlsVmCert/tlsVmKey (all Option[String]) read from env vars via reference.conf substitution - BaselineDependenciesBuilder.buildGalaxySslContext: pins to the CA cert when present (X509ExtendedTrustManager wrapping the chain-verifying inner TM, bypassing hostname check since Galaxy VMs are addressed by IP), else trust-all - galaxySslContext shared between vmClient (HttpAppDAO) and ProxyService, removing the duplicate inline trust-all context from ProxyService - GKEInterpreter: injects galaxy-tls-cert / galaxy-tls-key instance metadata when VM cert+key are configured, for the playbook to use Ticket: https://broadworkbench.atlassian.net/browse/CTM-678 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

GcpDependenciesBuilderSpec and MockProxyService were missing the new galaxySslContext parameter added to BaselineDependencies and ProxyService. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

codecov · 2026-10-02T15:03:31Z

Codecov Report

❌ Patch coverage is 4.16667% with 69 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.36%. Comparing base (6867b33) to head (b333adb).

Files with missing lines	Patch %	Lines
...ch/leonardo/http/BaselineDependenciesBuilder.scala	0.00%	40 Missing ⚠️
.../dsde/workbench/leonardo/util/GKEInterpreter.scala	0.00%	25 Missing ⚠️
...titute/dsde/workbench/leonardo/config/Config.scala	33.33%	4 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #4934      +/-   ##
===========================================
- Coverage    73.71%   73.36%   -0.35%     
===========================================
  Files          131      131              
  Lines        11263    11314      +51     
  Branches       933      927       -6     
===========================================
- Hits          8303     8301       -2     
- Misses        2960     3013      +53

Files with missing lines	Coverage Δ
...orkbench/leonardo/config/KubernetesAppConfig.scala	`95.00% <ø> (ø)`
...rkbench/leonardo/http/GcpDependenciesBuilder.scala	`72.83% <ø> (ø)`
...workbench/leonardo/http/service/ProxyService.scala	`75.11% <100.00%> (+2.06%)`	⬆️
...titute/dsde/workbench/leonardo/config/Config.scala	`97.15% <33.33%> (-0.66%)`	⬇️
.../dsde/workbench/leonardo/util/GKEInterpreter.scala	`61.44% <0.00%> (-1.94%)`	⬇️
...ch/leonardo/http/BaselineDependenciesBuilder.scala	`0.00% <0.00%> (ø)`

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6867b33...b333adb. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Adds tlsAllowTrustAllFallback (GALAXY_TLS_ALLOW_TRUST_ALL_FALLBACK) to GalaxyVmConfig. Defaults to false: Leo fails at startup if tlsCaCert is absent, so a missing or removed Secret Manager secret is immediately visible rather than silently degrading to trust-all. BEEs set this to true via the helmfile since the leo-galaxy-tls secret is not provisioned there. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…ning time Each Galaxy VM now gets a unique TLS cert+key signed by Leo's root CA and injected via GCE instance metadata at createInstance time (GKEInterpreter). A user reading their VM's metadata can only compromise their own connection, not other VMs. Changes: - Adds org.bouncycastle:bcpkix-jdk18on for cert generation; removes the global BouncyCastle exclusion (it was excluded as unused transitive dep) - Replaces tlsVmCert/tlsVmKey config fields with tlsCaKey (the CA private key Leo signs per-VM certs with) - GKEInterpreter.generateVmTlsCert signs a fresh 2048-bit RSA cert per VM - Startup validation: tlsCaCert and tlsCaKey must both be set or both absent Ticket: https://broadworkbench.atlassian.net/browse/CTM-678 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

sarahgibs

This is a good improvement.

- Remove trust-all fallback path; CA cert is now required at startup - Add Leo client cert+key (GALAXY_TLS_LEO_CLIENT_CERT/KEY) loaded into SSLContext KeyManager for mTLS handshake with Galaxy VMs - Inject galaxy_tls_client_ca into VM metadata so nginx can verify Leo - Fix metadata key names: hyphens -> underscores to match galaxy-k8s-boot PR #144 - Remove tlsAllowTrustAllFallback; BEEs now use real GSM secret (broad-dsde-qa) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

… merge strategy bcpkix, bcprov, and bcutil 1.86 each contain META-INF/versions/9/OSGI-INF/MANIFEST.MF with different contents. The default deduplicate strategy fails; use first. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Hard-throwing on absent GALAXY_TLS_CA_CERT prevents Leo from starting in environments where the leo-galaxy-tls Secret Manager secret has not been provisioned yet (e.g. BEEs before Terraform PR #2074 is deployed). Replace the throw with an SLF4J warning and fall back to SSLContext.getDefault. Galaxy VM connections will fail at the TLS handshake in those environments, but that is caught per-request and does not block startup. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

sarahgibs

Looks good to me. Thank you for getting this together so quickly.

lucymcnatt · 2026-10-05T20:44:16Z

-  )
+  // Previously excluded BouncyCastle (arrived transitively via io.kubernetes:client-java)
+  // but it is now explicitly depended on for per-VM Galaxy TLS cert generation.
+  val commonExcludes = List.empty[ExclusionRule]


nit, but you could probably just delete this exclude rule

lucymcnatt · 2026-10-05T20:46:03Z

  orchUrl: String,
-  drsUrl: String
+  drsUrl: String,
+  // Root-CA cert (PEM). Added to Leo's trust store for Galaxy VM connection validation.


nit, but these could be set together with something like GalaxyTlsConfig(caCert, caKey, leoClientCert, leoClientKey) to make it explicit that they must all be set together

lucymcnatt

left a couple nits, but overall LGTM

- Remove empty commonExcludes and its Settings.scala reference; BouncyCastle is now an explicit dependency so there is nothing to exclude transitively. - Introduce GalaxyTlsConfig grouping all four TLS secrets (caCert, caKey, leoClientCert, leoClientKey) into one type. The reader yields None when any field is missing, enforcing all-or-nothing semantics instead of partial state. Replace the four individual Option[String] fields in GalaxyVmConfig with tls: Option[GalaxyTlsConfig] and update all call sites. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

BouncyCastle PEMParser yields PrivateKeyInfo for PKCS#8 keys (OpenSSL 3.x `BEGIN PRIVATE KEY`) and PEMKeyPair for traditional RSA keys (`BEGIN RSA PRIVATE KEY`). The cast to PEMKeyPair crashed on PKCS#8 input. Ticket: https://broadworkbench.atlassian.net/browse/CTM-678 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

… TLS cert X500Name(String) re-encodes from RFC 2253, which can reorder RDNs and break PKIX issuer/subject byte matching. Use JcaX509CertificateHolder.getSubject to copy the CA cert's subject field verbatim into the server cert's issuer. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

LizBaldo changed the title ~~feat(access-controller): cert-pinned TLS for Galaxy VM connections~~ CTM-678: cert-pinned TLS for Galaxy VM connections Oct 2, 2026

fix(access-controller): update test mocks for galaxySslContext param

81f0eee

GcpDependenciesBuilderSpec and MockProxyService were missing the new galaxySslContext parameter added to BaselineDependencies and ProxyService. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

LizBaldo requested a review from sarahgibs October 2, 2026 14:55

Liz Baldo and others added 2 commits October 2, 2026 12:06

sarahgibs reviewed Oct 2, 2026

View reviewed changes

Comment thread project/Dependencies.scala Outdated

chore: bump bcpkix-jdk18on 1.78.1 → 1.86 (CVE remediation)

719fd08

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

sarahgibs approved these changes Oct 5, 2026

View reviewed changes

LizBaldo changed the title ~~CTM-678: cert-pinned TLS for Galaxy VM connections~~ CTM-678: mutual TLS for Galaxy VM connections Oct 5, 2026

LizBaldo requested a review from sarahgibs October 5, 2026 15:18

Liz Baldo and others added 2 commits October 5, 2026 11:22

sarahgibs approved these changes Oct 5, 2026

View reviewed changes

lucymcnatt reviewed Oct 5, 2026

View reviewed changes

lucymcnatt approved these changes Oct 5, 2026

View reviewed changes

Liz Baldo and others added 3 commits October 5, 2026 16:54

Merge branch 'develop' into feat/galaxy-tls-root-ca

37b4ad1

LizBaldo changed the title ~~CTM-678: mutual TLS for Galaxy VM connections~~ CTM-679: mutual TLS for Galaxy VM connections Oct 8, 2026

Liz Baldo and others added 3 commits October 8, 2026 09:38

style: scalafmt BaselineDependenciesBuilder.scala

442db30

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Merge branch 'develop' into feat/galaxy-tls-root-ca

b333adb

LizBaldo merged commit 826d4c0 into develop Oct 15, 2026
14 of 15 checks passed

LizBaldo deleted the feat/galaxy-tls-root-ca branch October 15, 2026 14:57

Navigation Menu

Repository navigation

CTM-679: mutual TLS for Galaxy VM connections - #4934

CTM-679: mutual TLS for Galaxy VM connections#4934
LizBaldo merged 14 commits into
developfrom
feat/galaxy-tls-root-ca

LizBaldo commented Oct 2, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Oct 2, 2026 •

edited

Loading

Uh oh!

Uh oh!

sarahgibs left a comment

Uh oh!

sarahgibs left a comment

Uh oh!

lucymcnatt Oct 5, 2026

Uh oh!

lucymcnatt Oct 5, 2026

Uh oh!

lucymcnatt left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

LizBaldo commented Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

sarahgibs left a comment

Choose a reason for hiding this comment

Uh oh!

sarahgibs left a comment

Choose a reason for hiding this comment

Uh oh!

lucymcnatt Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

lucymcnatt Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

lucymcnatt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

LizBaldo commented Oct 2, 2026 •

edited

Loading

codecov Bot commented Oct 2, 2026 •

edited

Loading