From 7b5f53419f5e8b513ebc8fc217d63aa5aa99d4d5 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Tue, 3 Mar 2026 00:02:47 +0200 Subject: [PATCH 1/7] fix: lenient mTLS cert reload A Coder customer reported that the cert refresh command can return 1 while still generating new certs. Right now Coder Toolbox does not reload the certs if the refresh command exits with a status other than 0. - resolves #276 --- CHANGELOG.md | 4 +++ .../com/coder/toolbox/sdk/CoderRestClient.kt | 28 ++++++------------- 2 files changed, 13 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index edb6895a..5ba7f88d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +# Changed + +- mTLS certificate reload is now more lenient, reload will be triggered even when refresh command returns a non 0 code + ## 0.8.5 - 2026-02-03 ### Added diff --git a/src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt b/src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt index d96e82ad..6adb886f 100644 --- a/src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt +++ b/src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt @@ -347,10 +347,9 @@ open class CoderRestClient( } catch (e: Exception) { if (context.settingsStore.requiresMTlsAuth && isCertExpired(e)) { context.logger.info("Certificate expired detected. Attempting refresh...") - if (refreshCertificates()) { - context.logger.info("Certificates refreshed, retrying the request...") - return block() - } + refreshCertificates() + context.logger.info("Retrying the request...") + return block() } throw e } @@ -361,29 +360,20 @@ open class CoderRestClient( e.message?.contains("certificate_expired", ignoreCase = true) == true } - private suspend fun refreshCertificates(): Boolean = withContext(Dispatchers.IO) { + private suspend fun refreshCertificates() = withContext(Dispatchers.IO) { val command = context.settingsStore.readOnly().tls.certRefreshCommand if (command.isNullOrBlank()) return@withContext false return@withContext try { val result = ProcessExecutor() .command(command.split(" ").toList()) - .exitValueNormal() + .exitValueAny() .readOutput(true) .execute() - - if (result.exitValue == 0) { - context.logger.info("Certificate refresh successful. Reloading TLS and evicting pool.") - tlsContext.reload() - - // This is the "Magic Fix": - // It forces OkHttp to close the broken HTTP/2 connection. - httpClient.connectionPool.evictAll() - return@withContext true - } else { - context.logger.error("Refresh command failed with code ${result.exitValue}") - false - } + context.logger.info("Certificate refresh finished with code ${result.exitValue}. Reloading TLS and evicting pool.") + tlsContext.reload() + // forces OkHttp to close the broken HTTP/2 connection. + httpClient.connectionPool.evictAll() } catch (ex: Exception) { context.logger.error(ex, "Failed to execute refresh command") false From 55fd9de98655804f6f1b180545f211de3c636d3e Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Tue, 3 Mar 2026 00:04:26 +0200 Subject: [PATCH 2/7] chore: next version is 0.8.6 --- gradle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle.properties b/gradle.properties index 37aba2ea..eb319e2e 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,3 +1,3 @@ -version=0.8.5 +version=0.8.6 group=com.coder.toolbox name=coder-toolbox \ No newline at end of file From 8d2c2b283953dd9a71ae107bb142bb5e8b668882 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Tue, 3 Mar 2026 01:06:58 +0200 Subject: [PATCH 3/7] chore: fix Changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ba7f88d..db52ef59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## Unreleased -# Changed +### Changed - mTLS certificate reload is now more lenient, reload will be triggered even when refresh command returns a non 0 code From f723f6a35d9a2391b57850d682ca6526fde72965 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Tue, 3 Mar 2026 01:17:16 +0200 Subject: [PATCH 4/7] chore: update CHANGELOG.md Co-authored-by: Zach <3724288+zedkipp@users.noreply.github.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index db52ef59..9f059a81 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Changed -- mTLS certificate reload is now more lenient, reload will be triggered even when refresh command returns a non 0 code +- mTLS connections no longer disconnect when the certificate refresh command exits with a non-zero code ## 0.8.5 - 2026-02-03 From 454eb401b797e8cc2b271a90fbfe71829433409e Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Wed, 4 Mar 2026 00:09:24 +0200 Subject: [PATCH 5/7] fix: verify certificate changes before reloading TLS context Instead of relying on the refresh command return code to report the issue upstream (which most of the time means breaking the poll loop and go back to the login scree), the code could detect whether the CA files actually changed and use that information to preserve the earlier control flow if needed. --- .../com/coder/toolbox/sdk/CoderRestClient.kt | 21 ++++++--- src/main/kotlin/com/coder/toolbox/util/TLS.kt | 44 ++++++++++++++++--- 2 files changed, 51 insertions(+), 14 deletions(-) diff --git a/src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt b/src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt index 6adb886f..ee538d70 100644 --- a/src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt +++ b/src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt @@ -347,9 +347,10 @@ open class CoderRestClient( } catch (e: Exception) { if (context.settingsStore.requiresMTlsAuth && isCertExpired(e)) { context.logger.info("Certificate expired detected. Attempting refresh...") - refreshCertificates() - context.logger.info("Retrying the request...") - return block() + if (refreshCertificates()) { + context.logger.info("Certificates refreshed, retrying the request...") + return block() + } } throw e } @@ -360,7 +361,7 @@ open class CoderRestClient( e.message?.contains("certificate_expired", ignoreCase = true) == true } - private suspend fun refreshCertificates() = withContext(Dispatchers.IO) { + private suspend fun refreshCertificates(): Boolean = withContext(Dispatchers.IO) { val command = context.settingsStore.readOnly().tls.certRefreshCommand if (command.isNullOrBlank()) return@withContext false @@ -371,9 +372,15 @@ open class CoderRestClient( .readOutput(true) .execute() context.logger.info("Certificate refresh finished with code ${result.exitValue}. Reloading TLS and evicting pool.") - tlsContext.reload() - // forces OkHttp to close the broken HTTP/2 connection. - httpClient.connectionPool.evictAll() + if (tlsContext.reload()) { + context.logger.info("Certificate refresh successful. Reloading TLS and evicting pool.") + // forces OkHttp to close the broken HTTP/2 connection. + httpClient.connectionPool.evictAll() + return@withContext true + } else { + context.logger.error("Refresh command failed with code ${result.exitValue}") + false + } } catch (ex: Exception) { context.logger.error(ex, "Failed to execute refresh command") false diff --git a/src/main/kotlin/com/coder/toolbox/util/TLS.kt b/src/main/kotlin/com/coder/toolbox/util/TLS.kt index 101370d2..bca20619 100644 --- a/src/main/kotlin/com/coder/toolbox/util/TLS.kt +++ b/src/main/kotlin/com/coder/toolbox/util/TLS.kt @@ -284,16 +284,29 @@ class MergedSystemTrustManger(private val otherTrustManager: X509TrustManager) : class ReloadableX509TrustManager( private val caPath: String?, ) : X509TrustManager { + private var lastHash: String? = null + @Volatile private var delegate: X509TrustManager = loadTrustManager() private fun loadTrustManager(): X509TrustManager { + if (!caPath.isNullOrBlank()) { + lastHash = sha1(FileInputStream(expand(caPath))) + } val trustManagers = coderTrustManagers(caPath) return trustManagers.first { it is X509TrustManager } as X509TrustManager } - fun reload() { - delegate = loadTrustManager() + fun reload(): Boolean { + if (caPath.isNullOrBlank()) { + return false + } + val newHash = sha1(FileInputStream(expand(caPath))) + if (lastHash != newHash) { + delegate = loadTrustManager() + return true + } + return false } override fun checkClientTrusted(chain: Array?, authType: String?) { @@ -312,15 +325,31 @@ class ReloadableX509TrustManager( class ReloadableSSLSocketFactory( private val settings: ReadOnlyTLSSettings, ) : SSLSocketFactory() { + private var lastCertHash: String? = null + private var lastKeyHash: String? = null + @Volatile private var delegate: SSLSocketFactory = loadSocketFactory() private fun loadSocketFactory(): SSLSocketFactory { + if (!settings.certPath.isNullOrBlank() && !settings.keyPath.isNullOrBlank()) { + lastCertHash = sha1(FileInputStream(expand(settings.certPath!!))) + lastKeyHash = sha1(FileInputStream(expand(settings.keyPath!!))) + } return coderSocketFactory(settings) } - fun reload() { - delegate = loadSocketFactory() + fun reload(): Boolean { + if (settings.certPath.isNullOrBlank() || settings.keyPath.isNullOrBlank()) { + return false + } + val newCertHash = sha1(FileInputStream(expand(settings.certPath!!))) + val newKeyHash = sha1(FileInputStream(expand(settings.keyPath!!))) + if (lastCertHash != newCertHash || lastKeyHash != newKeyHash) { + delegate = loadSocketFactory() + return true + } + return false } override fun getDefaultCipherSuites(): Array = delegate.defaultCipherSuites @@ -349,8 +378,9 @@ class ReloadableTlsContext( val sslSocketFactory = ReloadableSSLSocketFactory(settings) val trustManager = ReloadableX509TrustManager(settings.caPath) - fun reload() { - sslSocketFactory.reload() - trustManager.reload() + fun reload(): Boolean { + val socketFactoryReloaded = sslSocketFactory.reload() + val trustManagerReloaded = trustManager.reload() + return socketFactoryReloaded || trustManagerReloaded } } From 5048e1b8705e54a3cca357aa075f4e372bccd3a5 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Wed, 4 Mar 2026 00:19:20 +0200 Subject: [PATCH 6/7] impl: use a buffer to read input streams Using a buffer instead of reading byte-by-byte is a more efficient mechanism. --- src/main/kotlin/com/coder/toolbox/util/Hash.kt | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/main/kotlin/com/coder/toolbox/util/Hash.kt b/src/main/kotlin/com/coder/toolbox/util/Hash.kt index e23a11d7..ed5715f3 100644 --- a/src/main/kotlin/com/coder/toolbox/util/Hash.kt +++ b/src/main/kotlin/com/coder/toolbox/util/Hash.kt @@ -5,17 +5,19 @@ import java.io.InputStream import java.security.DigestInputStream import java.security.MessageDigest +private const val BUFFER_SIZE = 8192 + fun ByteArray.toHex() = joinToString(separator = "") { byte -> "%02x".format(byte) } /** * Return the SHA-1 for the provided stream. */ -@Suppress("ControlFlowWithEmptyBody") fun sha1(stream: InputStream): String { val md = MessageDigest.getInstance("SHA-1") - val dis = DigestInputStream(BufferedInputStream(stream), md) - stream.use { - while (dis.read() != -1) { + DigestInputStream(BufferedInputStream(stream), md).use { dis -> + val buffer = ByteArray(BUFFER_SIZE) + while (dis.read(buffer) != -1) { + // Read until EOF } } return md.digest().toHex() From 6225ef06249bf98f9fde14b3d5f46e0b0dd0f61a Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Wed, 4 Mar 2026 22:05:41 +0200 Subject: [PATCH 7/7] fix: remove log line --- src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt b/src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt index ee538d70..31255d99 100644 --- a/src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt +++ b/src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt @@ -371,7 +371,6 @@ open class CoderRestClient( .exitValueAny() .readOutput(true) .execute() - context.logger.info("Certificate refresh finished with code ${result.exitValue}. Reloading TLS and evicting pool.") if (tlsContext.reload()) { context.logger.info("Certificate refresh successful. Reloading TLS and evicting pool.") // forces OkHttp to close the broken HTTP/2 connection.