Restrict deployment redirects and credential headers

CHANGELOG.md

@@ -24,6 +24,8 @@ It follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ### Security
 
+- Restrict HTTP redirects and deployment credentials to their original destination. Disable redirects in CLI v2.38.0 and newer.
+
 - Ask for confirmation before a workspace link opens an unknown Coder deployment. Links select the deployment by
   scheme, host, and port. For a known deployment, Toolbox asks before using a link token if the accounts differ or
   it cannot verify the saved account.

src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt

@@ -113,6 +113,7 @@ class CoderCLIManager(
 
     // Keep credential storage consistent with existing SSH commands until the next sign-in.
     private val useKeyring = context.settingsStore.useKeyring && usesTokenAuth
+    private val supportsRedirectFlag by lazy { tryVersion()?.let { it >= SemVer(2, 38, 0) } == true }
     private val downloader = createDownloadService()
     private val gpgVerifier = GPGVerifier(context)
 
@@ -661,6 +662,7 @@ class CoderCLIManager(
     private fun globalConfigArgs(): List<String> = listOf("--global-config", coderConfigPath.toString())
 
     private fun workspaceAuthArgs(): List<String> = buildList {
+        if (supportsRedirectFlag) add("--allow-redirects=false")
         val useKeyringAuth = shouldUseKeyringAuth()
         if (!useKeyringAuth) addAll(globalConfigArgs())
         // Override inherited CODER_URL and CODER_USE_KEYRING for every authenticated command.

src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt

@@ -4,6 +4,7 @@ import com.coder.toolbox.CoderToolboxContext
 import com.coder.toolbox.cli.ex.ResponseException
 import com.coder.toolbox.util.OS
 import com.coder.toolbox.util.SemVer
+import com.coder.toolbox.util.hasSameOrigin
 import com.coder.toolbox.util.getHeaders
 import com.coder.toolbox.util.getOS
 import com.coder.toolbox.util.sha1
@@ -130,7 +131,7 @@ class CoderDownloadService(
     }
 
     private fun getRequestHeaders(): Map<String, String> {
-        return if (context.settingsStore.headerCommand.isNullOrBlank()) {
+        return if (!remoteBinaryURL.hasSameOrigin(deploymentUrl) || context.settingsStore.headerCommand.isNullOrBlank()) {
             emptyMap()
         } else {
             getHeaders(deploymentUrl, context.settingsStore.headerCommand)

src/main/kotlin/com/coder/toolbox/sdk/CoderHttpClientBuilder.kt

@@ -21,6 +21,8 @@ object CoderHttpClientBuilder {
             .sslSocketFactory(tlsContext.sslSocketFactory, tlsContext.trustManager)
             .hostnameVerifier(CoderHostnameVerifier(context.settingsStore.tls.altHostname))
             .retryOnConnectionFailure(true)
+            .followRedirects(false)
+            .addInterceptor(Interceptors.sameOriginRedirects())
             // Detect silent connection loss even when a workspace build produces no log messages.
             .pingInterval(30, TimeUnit.SECONDS)
 

src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt

@@ -95,7 +95,7 @@ open class CoderRestClient(
 
         val interceptors = buildList {
             if (context.settingsStore.requiresTokenAuth) {
-                add(Interceptors.tokenAuth(requireSessionToken()))
+                add(Interceptors.tokenAuth(requireSessionToken(), url))
             }
             add((Interceptors.userAgent(pluginVersion)))
             add(Interceptors.externalHeaders(context, url))

src/main/kotlin/com/coder/toolbox/sdk/interceptors/Interceptors.kt

@@ -4,7 +4,10 @@ import com.coder.toolbox.CoderToolboxContext
 import com.coder.toolbox.util.getArch
 import com.coder.toolbox.util.getHeaders
 import com.coder.toolbox.util.getOS
+import com.coder.toolbox.util.hasSameOrigin
+import com.coder.toolbox.util.origin
 import okhttp3.Interceptor
+import java.net.ProtocolException
 import java.net.URL
 
 const val CODER_SESSION_TOKEN_HEADER_NAME = "Coder-Session-Token"
@@ -14,13 +17,46 @@ const val CODER_SESSION_TOKEN_HEADER_NAME = "Coder-Session-Token"
  */
 object Interceptors {
 
+    private const val MAX_REDIRECTS = 20
+
+    /**
+     * Follows redirects only for GET and HEAD requests and only within the original scheme, host, and port.
+     * HTTP defines these methods as read-only, and OkHttp sends them without a request body. Checking each
+     * target prevents forwarding deployment credentials to another origin. Other methods are never replayed.
+     *
+     * The cap of [MAX_REDIRECTS] matches OkHttp 4.12.1's limit for follow-up requests and bounds redirect loops.
+     * The initial request does not count. A further redirect after the twentieth fails before the next request is sent.
+     */
+    fun sameOriginRedirects(): Interceptor = Interceptor { chain ->
+        var request = chain.request()
+        val origin = request.url.origin()
+        var redirects = 0
+        var response = chain.proceed(request)
+        while (response.isRedirect) {
+            val location = response.header("Location") ?: break
+            val target = request.url.resolve(location)
+            if (target == null || target.origin() != origin || target.username.isNotEmpty() || target.password.isNotEmpty()) {
+                response.close()
+                throw ProtocolException("Refused a redirect outside the request origin or to an invalid URL.")
+            }
+            if (request.method != "GET" && request.method != "HEAD") break
+            response.close()
+            if (++redirects > MAX_REDIRECTS) throw ProtocolException("Too many redirects.")
+            request = request.newBuilder().url(target).build()
+            response = chain.proceed(request)
+        }
+        response
+    }
+
     /**
      * Creates a token authentication interceptor
      */
-    fun tokenAuth(token: String): Interceptor {
+    fun tokenAuth(token: String, deploymentUrl: URL): Interceptor {
         return Interceptor { chain ->
+            val request = chain.request()
+            if (!request.url.hasSameOrigin(deploymentUrl)) return@Interceptor chain.proceed(request)
             chain.proceed(
-                chain.request().newBuilder()
+                request.newBuilder()
                     .addHeader(CODER_SESSION_TOKEN_HEADER_NAME, token)
                     .build()
             )
@@ -47,6 +83,7 @@ object Interceptors {
         val settings = context.settingsStore.readOnly()
         return Interceptor { chain ->
             var request = chain.request()
+            if (!request.url.hasSameOrigin(url)) return@Interceptor chain.proceed(request)
             val headers = getHeaders(url, settings.headerCommand)
             if (headers.isNotEmpty()) {
                 val reqBuilder = request.newBuilder()
@@ -63,4 +100,4 @@ object Interceptors {
     fun logging(context: CoderToolboxContext): Interceptor {
         return LoggingInterceptor(context)
     }
-}
+}

src/main/kotlin/com/coder/toolbox/util/URLExtensions.kt

@@ -24,7 +24,9 @@ fun HttpUrl.origin(): String = newBuilder()
     .toString()
     .removeSuffix("/")
 
-fun URL.hasSameOrigin(other: URL?): Boolean = origin()?.let { it == other?.origin() } ?: false
+fun URL.hasSameOrigin(other: URL?): Boolean = toString().toHttpUrlOrNull()?.hasSameOrigin(other) == true
+
+fun HttpUrl.hasSameOrigin(other: URL?): Boolean = origin() == other?.origin()
 
 fun String.toBaseURL(): URL {
     val url = this.toURL()

src/main/kotlin/com/coder/toolbox/views/CoderSetupWizardPage.kt

@@ -48,14 +48,14 @@ class CoderSetupWizardPage private constructor(
     override val fields: MutableStateFlow<List<UiField>> = MutableStateFlow(emptyList())
     override val actionButtons: MutableStateFlow<List<RunnableActionDescription>> = MutableStateFlow(emptyList())
 
-    fun pageForDisplay(): CoderSetupWizardPage? = when (model.currentStep()) {
-        WizardStep.CONNECT -> if (model.connectionOrigin == ConnectionOrigin.SAVED_SESSION) null else this
-        WizardStep.SIGNED_IN -> null
+    fun pageForDisplay(): CoderSetupWizardPage? = when (val step = model.currentStep()) {
+        is WizardStep.Connecting -> if (step.origin == ConnectionOrigin.SAVED_SESSION) null else this
+        WizardStep.SignedIn -> null
         else -> this
     }
 
     internal fun startSavedSessionRestore() {
-        if (model.connectionOrigin == ConnectionOrigin.SAVED_SESSION) connectStep.startConnection()
+        if (model.currentStep() == WizardStep.Connecting(ConnectionOrigin.SAVED_SESSION)) connectStep.startConnection()
     }
 
     override fun beforeShow() {
@@ -77,7 +77,7 @@ class CoderSetupWizardPage private constructor(
 
     private fun displayStep(step: WizardStep) {
         when (step) {
-            WizardStep.URL_REQUEST -> {
+            WizardStep.UrlRequest -> {
                 fields.update {
                     listOf(deploymentUrlStep.panel)
                 }
@@ -90,7 +90,7 @@ class CoderSetupWizardPage private constructor(
                 deploymentUrlStep.onVisible()
             }
 
-            WizardStep.TOKEN_REQUEST -> {
+            WizardStep.TokenRequest -> {
                 fields.update {
                     listOf(tokenStep.panel)
                 }
@@ -104,7 +104,7 @@ class CoderSetupWizardPage private constructor(
                 tokenStep.onVisible()
             }
 
-            WizardStep.CONNECT -> {
+            is WizardStep.Connecting -> {
                 fields.update {
                     listOf(connectStep.panel)
                 }
@@ -119,7 +119,7 @@ class CoderSetupWizardPage private constructor(
 
             // Toolbox can bring this wizard back after Settings or during startup.
             // Ask for the workspace list when the completed wizard is actually shown.
-            WizardStep.SIGNED_IN -> context.envPageManager.showPluginEnvironmentsPage(false)
+            WizardStep.SignedIn -> context.envPageManager.showPluginEnvironmentsPage(false)
         }
     }
 
@@ -130,14 +130,14 @@ class CoderSetupWizardPage private constructor(
     }
 
     private fun navigateBackFromConnect() {
-        if (model.connectionOrigin == ConnectionOrigin.FORM) {
-            model.goTo(if (context.settingsStore.requiresTokenAuth) WizardStep.TOKEN_REQUEST else WizardStep.URL_REQUEST)
+        val phase = model.currentStep() as? WizardStep.Connecting ?: return
+        if (phase.origin == ConnectionOrigin.FORM) {
+            model.goTo(if (context.settingsStore.requiresTokenAuth) WizardStep.TokenRequest else WizardStep.UrlRequest)
             return
         }
-        val revealForm = model.connectionOrigin == ConnectionOrigin.SAVED_SESSION
+        val revealForm = phase.origin == ConnectionOrigin.SAVED_SESSION
         model.clearFormData()
         model.goToFirst()
-        model.connectionOrigin = ConnectionOrigin.FORM
         if (revealForm) {
             context.cs.launch {
                 if (!disposed) context.ui.showUiPageSuspending(this@CoderSetupWizardPage)
@@ -188,14 +188,13 @@ class CoderSetupWizardPage private constructor(
             onConnect = onConnect,
             onTokenRefreshed = onTokenRefreshed,
         ).apply {
-            model.connectionOrigin = origin
             model.url = url
             when (credentials) {
                 is Credentials.MTls -> Unit
                 is Credentials.Token -> model.token = credentials.value
                 is Credentials.OAuth -> model.oauthSession = credentials.session
             }
-            model.goTo(WizardStep.CONNECT)
+            model.goTo(WizardStep.Connecting(origin))
         }
     }
 }

src/main/kotlin/com/coder/toolbox/views/ConnectStep.kt

@@ -10,8 +10,8 @@ import com.coder.toolbox.util.origin
 import com.coder.toolbox.views.state.CoderOAuthSessionContext
 import com.coder.toolbox.views.state.ConnectionOrigin
 import com.coder.toolbox.views.state.WizardModel
-import com.coder.toolbox.views.state.WizardStep.CONNECT
-import com.coder.toolbox.views.state.WizardStep.SIGNED_IN
+import com.coder.toolbox.views.state.WizardStep.Connecting
+import com.coder.toolbox.views.state.WizardStep.SignedIn
 import com.jetbrains.toolbox.api.ui.components.LabelField
 import com.jetbrains.toolbox.api.ui.components.RowGroup
 import com.jetbrains.toolbox.api.ui.components.ValidationErrorField
@@ -62,7 +62,7 @@ class ConnectStep(
 
     @Synchronized
     private fun prepareConnection(): Job? {
-        if (disposed || signInJob != null || model.currentStep() != CONNECT) return null
+        if (disposed || signInJob != null || model.currentStep() !is Connecting) return null
 
         errorField.textState.update { context.i18n.pnotr("") }
         if (context.settingsStore.requiresTokenAuth && model.isNotReadyForAuth()) {
@@ -160,7 +160,7 @@ class ConnectStep(
     private fun reportConnectionFailure(job: Job, url: URL, ex: Exception) {
         job.ensureActive()
         if (signInJob !== job) return
-        if (model.currentStep() == SIGNED_IN) {
+        if (model.currentStep() == SignedIn) {
             context.logger.warn(ex, "Failed to update session credentials after sign-in")
             return
         }
@@ -179,7 +179,8 @@ class ConnectStep(
 
     @Synchronized
     override fun onBack() {
-        if (disposed || model.currentStep() != CONNECT || model.connectionOrigin == ConnectionOrigin.SAVED_SESSION) return
+        val phase = model.currentStep() as? Connecting ?: return
+        if (disposed || phase.origin == ConnectionOrigin.SAVED_SESSION) return
         try {
             context.logger.info("Back button was pressed, cancelling in-progress connection setup...")
             signInJob?.cancel(CancellationException(USER_HIT_THE_BACK_BUTTON))

src/main/kotlin/com/coder/toolbox/views/DeploymentUrlStep.kt

@@ -12,8 +12,8 @@ import com.coder.toolbox.util.toURL
 import com.coder.toolbox.util.validateStrictWebUrl
 import com.coder.toolbox.views.state.CoderOAuthSessionContext
 import com.coder.toolbox.views.state.WizardModel
-import com.coder.toolbox.views.state.WizardStep.CONNECT
-import com.coder.toolbox.views.state.WizardStep.TOKEN_REQUEST
+import com.coder.toolbox.views.state.WizardStep.Connecting
+import com.coder.toolbox.views.state.WizardStep.TokenRequest
 import com.jetbrains.toolbox.api.localization.LocalizableString
 import com.jetbrains.toolbox.api.ui.components.CheckboxField
 import com.jetbrains.toolbox.api.ui.components.FieldModifier
@@ -90,7 +90,7 @@ class DeploymentUrlStep(
         }
 
         if (context.settingsStore.requiresMTlsAuth) {
-            model.goTo(CONNECT)
+            model.goTo(Connecting())
             return true
         }
         if (context.settingsStore.requiresTokenAuth && context.settingsStore.preferOAuth2IfAvailable) {
@@ -104,7 +104,7 @@ class DeploymentUrlStep(
             }
         }
         // if all else fails try the good old API token auth
-        model.goTo(TOKEN_REQUEST)
+        model.goTo(TokenRequest)
         return true
     }
 

src/main/kotlin/com/coder/toolbox/views/TokenStep.kt

@@ -3,8 +3,8 @@ package com.coder.toolbox.views
 import com.coder.toolbox.CoderToolboxContext
 import com.coder.toolbox.util.withPath
 import com.coder.toolbox.views.state.WizardModel
-import com.coder.toolbox.views.state.WizardStep.CONNECT
-import com.coder.toolbox.views.state.WizardStep.URL_REQUEST
+import com.coder.toolbox.views.state.WizardStep.Connecting
+import com.coder.toolbox.views.state.WizardStep.UrlRequest
 import com.jetbrains.toolbox.api.localization.LocalizableString
 import com.jetbrains.toolbox.api.ui.components.FieldModifier
 import com.jetbrains.toolbox.api.ui.components.LinkField
@@ -58,12 +58,12 @@ class TokenStep(
         }
 
         model.token = token
-        model.goTo(CONNECT)
+        model.goTo(Connecting())
         return true
     }
 
     override fun onBack() {
-        model.goTo(URL_REQUEST)
+        model.goTo(UrlRequest)
     }
 
     private fun reportError(message: LocalizableString?) {

src/main/kotlin/com/coder/toolbox/views/state/PageRouter.kt

@@ -57,6 +57,6 @@ class PageRouter {
         startSession()
         val wizard = activePage ?: return
         activePage = null
-        wizard.model.goTo(WizardStep.SIGNED_IN)
+        wizard.model.goTo(WizardStep.SignedIn)
     }
 }

src/main/kotlin/com/coder/toolbox/views/state/WizardModel.kt

@@ -12,14 +12,12 @@ import java.net.URL
  * cycles without leaking across wizard recreations.
  */
 class WizardModel {
-    private val _step: MutableStateFlow<WizardStep> = MutableStateFlow(WizardStep.URL_REQUEST)
+    private val _step: MutableStateFlow<WizardStep> = MutableStateFlow(WizardStep.UrlRequest)
     val step: StateFlow<WizardStep> = _step
 
     var url: URL? = null
     var token: String? = null
     var oauthSession: CoderOAuthSessionContext? = null
-    @Volatile
-    var connectionOrigin = ConnectionOrigin.FORM
 
     fun currentStep(): WizardStep = _step.value
 
@@ -28,7 +26,7 @@ class WizardModel {
     }
 
     fun goToFirst() {
-        _step.value = WizardStep.URL_REQUEST
+        _step.value = WizardStep.UrlRequest
     }
 
     fun hasUrl(): Boolean = url != null
@@ -44,8 +42,11 @@ class WizardModel {
     }
 }
 
-enum class WizardStep {
-    URL_REQUEST, TOKEN_REQUEST, CONNECT, SIGNED_IN;
+sealed interface WizardStep {
+    data object UrlRequest : WizardStep
+    data object TokenRequest : WizardStep
+    data class Connecting(val origin: ConnectionOrigin = ConnectionOrigin.FORM) : WizardStep
+    data object SignedIn : WizardStep
 }
 
 enum class ConnectionOrigin {