diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d187fc38..20ff65cd6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ We use [semantic versioning](http://semver.org/): - PATCH version when you make backwards compatible bug fixes. # Next version +- [feature] _agent_: The profiler can now read the system proxy configuration including Proxy-Auto-Config (PAC) files to reach Teamscale. Set `-Djava.net.useSystemProxies=true` to opt in. Explicitly configured proxies (via the `proxy-http-host`/`proxy-https-host` options or the `-Dhttp.proxyHost`/`-Dhttps.proxyHost` JVM properties) take precedence. +- [fix] _agent_: The standard JVM proxy properties (`-Dhttp.proxyHost`/`-Dhttp.proxyPort`/`-Dhttps.proxyHost`/...) were looked up under a malformed key and thus ignored. - [security fix] _agent_: The Azure shared-key error messages no longer include the configured `azure-key` value in the exception text (previously leaked the access key into stack traces and logs) # 37.0.0 diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index a6683cd3e..0285ff520 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -13,6 +13,8 @@ picocli = "4.7.7" maven = "3.9.16" asm = "9.10.1" coroutines = "1.11.0" +# We need to stay on 1.1.x as 2.0.x requires Java 17. +proxyVole = "1.1.9" [libraries] jersey-bom = { module = "org.glassfish.jersey:jersey-bom", version = "2.48" } @@ -45,6 +47,7 @@ commonsCodec = { module = "commons-codec:commons-codec", version = "1.22.0" } commonsLang = { module = "org.apache.commons:commons-lang3", version = "3.20.0" } commonsIo = { module = "commons-io:commons-io", version = "2.22.0" } slf4j-api = { module = "org.slf4j:slf4j-api", version = "2.0.18" } +proxy-vole = { module = "org.bidib.com.github.markusbernhardt:proxy-vole", version.ref = "proxyVole" } jgit = { module = "org.eclipse.jgit:org.eclipse.jgit", version = "7.7.0.202606012155-r" } okio = { module = "com.squareup.okio:okio", version = "3.17.0" } diff --git a/teamscale-client/build.gradle.kts b/teamscale-client/build.gradle.kts index eb78f22d7..07ae8b532 100644 --- a/teamscale-client/build.gradle.kts +++ b/teamscale-client/build.gradle.kts @@ -17,5 +17,6 @@ dependencies { implementation(libs.commonsCodec) implementation(libs.slf4j.api) implementation(libs.retrofit.converter.jackson) + implementation(libs.proxy.vole) testImplementation(libs.okhttp.mockwebserver) } diff --git a/teamscale-client/src/main/kotlin/com/teamscale/client/HttpUtils.kt b/teamscale-client/src/main/kotlin/com/teamscale/client/HttpUtils.kt index 8f7fbd177..cef09c632 100644 --- a/teamscale-client/src/main/kotlin/com/teamscale/client/HttpUtils.kt +++ b/teamscale-client/src/main/kotlin/com/teamscale/client/HttpUtils.kt @@ -1,5 +1,6 @@ package com.teamscale.client +import com.github.markusbernhardt.proxy.ProxySearch import okhttp3.Authenticator import okhttp3.Credentials.basic import okhttp3.Interceptor @@ -9,12 +10,12 @@ import retrofit2.Retrofit import java.io.IOException import java.net.InetSocketAddress import java.net.Proxy +import java.net.ProxySelector import java.security.GeneralSecurityException import java.security.SecureRandom import java.security.cert.X509Certificate import java.time.Duration import java.util.* -import java.util.function.Consumer import javax.net.ssl.* /** @@ -79,13 +80,76 @@ object HttpUtils { * [https://stackoverflow.com/a/35567936](https://stackoverflow.com/a/35567936) */ private fun Builder.setUpProxyServer() { - val setHttpsProxyWasSuccessful = setUpProxyServerForProtocol( - ProxySystemProperties.Protocol.HTTPS, - this - ) - if (!setHttpsProxyWasSuccessful) { - setUpProxyServerForProtocol(ProxySystemProperties.Protocol.HTTP, this) + val explicitProxyConfigured = setUpProxyServerForProtocol(ProxySystemProperties.Protocol.HTTPS, this) || + setUpProxyServerForProtocol(ProxySystemProperties.Protocol.HTTP, this) + + // No explicit proxy configured: auto-detect the proxy from the operating system (including PAC files). + if (!explicitProxyConfigured) { + setUpSystemProxySelector() + } + } + + /** + * Supplies the operating system's [ProxySelector] including Proxy-Auto-Config (PAC) support. + * Overridable in tests to inject a deterministic selector. + */ + internal var systemProxySelectorSupplier: () -> ProxySelector? = { + ProxySearch.getDefaultProxySearch().proxySelector + } + set(value) = synchronized(this) { + field = value + cachedSystemProxySelector = null + } + + /** + * Caches a *successful* system [ProxySelector] detection (wrapped in an [Optional] to also cache the + * "no proxy configured" result). The OS proxy configuration does not change during the lifetime of the JVM, so the + * potentially expensive detection (PAC download, WPAD lookup, native OS calls) only needs to run once. `null` means + * "not yet detected". A detection *failure* is deliberately not cached (see [detectSystemProxySelector]). + */ + private var cachedSystemProxySelector: Optional? = null + + /** + * Installs a [ProxySelector] that reads the operating system's proxy configuration including Proxy-Auto-Config (PAC) + * files. This is only enabled when the user sets `-Djava.net.useSystemProxies=true`. + * + * Any failure while detecting the system proxy is logged and ignored so that the connection falls back to a direct + * connection instead of preventing the profiler from starting. + */ + private fun Builder.setUpSystemProxySelector() { + if (!System.getProperty("java.net.useSystemProxies").toBoolean()) { + return + } + + val proxySelector = detectSystemProxySelector() + if (proxySelector == null) { + LOGGER.debug("java.net.useSystemProxies is set but no system proxy could be detected.") + return + } + LOGGER.debug("Using the system proxy configuration (including PAC files) to reach Teamscale.") + proxySelector(proxySelector) + } + + /** + * Detects the system [ProxySelector] once and caches a successful result (see [cachedSystemProxySelector]). + * Synchronized so that concurrent client construction runs the potentially expensive detection at most once. + */ + @Synchronized + private fun detectSystemProxySelector(): ProxySelector? { + cachedSystemProxySelector?.let { return it.orElse(null) } + + val proxySelector = try { + systemProxySelectorSupplier() + } catch (e: Exception) { + LOGGER.warn( + "Failed to detect the system proxy configuration. Falling back to a direct connection." + + " Configure the proxy explicitly via the proxy-http-host/proxy-https-host options if needed.", + e + ) + return null } + cachedSystemProxySelector = Optional.ofNullable(proxySelector) + return proxySelector } private fun setUpProxyServerForProtocol( diff --git a/teamscale-client/src/main/kotlin/com/teamscale/client/ProxySystemProperties.kt b/teamscale-client/src/main/kotlin/com/teamscale/client/ProxySystemProperties.kt index d2f846ad9..942b32d98 100644 --- a/teamscale-client/src/main/kotlin/com/teamscale/client/ProxySystemProperties.kt +++ b/teamscale-client/src/main/kotlin/com/teamscale/client/ProxySystemProperties.kt @@ -13,10 +13,10 @@ package com.teamscale.client open class ProxySystemProperties(private val protocol: Protocol) { companion object { - private const val PROXY_HOST_SYSTEM_PROPERTY = ".proxyHost" - private const val PROXY_PORT_SYSTEM_PROPERTY = ".proxyPort" - private const val PROXY_USER_SYSTEM_PROPERTY = ".proxyUser" - private const val PROXY_PASSWORD_SYSTEM_PROPERTY = ".proxyPassword" + private const val PROXY_HOST_SYSTEM_PROPERTY = "proxyHost" + private const val PROXY_PORT_SYSTEM_PROPERTY = "proxyPort" + private const val PROXY_USER_SYSTEM_PROPERTY = "proxyUser" + private const val PROXY_PASSWORD_SYSTEM_PROPERTY = "proxyPassword" } /** diff --git a/teamscale-client/src/test/kotlin/com/teamscale/client/ProxySystemPropertiesTest.kt b/teamscale-client/src/test/kotlin/com/teamscale/client/ProxySystemPropertiesTest.kt index dac5a70b8..1dcc87c2e 100644 --- a/teamscale-client/src/test/kotlin/com/teamscale/client/ProxySystemPropertiesTest.kt +++ b/teamscale-client/src/test/kotlin/com/teamscale/client/ProxySystemPropertiesTest.kt @@ -20,6 +20,23 @@ internal class ProxySystemPropertiesTest { assertThat(properties.proxyPort).isEqualTo(-1) } + @Test + fun testUsesStandardJvmProxyPropertyKeys() { + properties.proxyHost = "myHost" + properties.proxyPort = 1234 + + assertThat(System.getProperty("http.proxyHost")).isEqualTo("myHost") + assertThat(System.getProperty("http.proxyPort")).isEqualTo("1234") + assertThat(System.getProperty("http..proxyHost")).isNull() + properties.clear() + + // The Teamscale-specific variant keeps its "teamscale." prefix so it does not collide with the JVM properties. + val teamscaleProperties = TeamscaleProxySystemProperties(ProxySystemProperties.Protocol.HTTP) + teamscaleProperties.proxyHost = "teamscaleHost" + assertThat(System.getProperty("teamscale.http.proxyHost")).isEqualTo("teamscaleHost") + teamscaleProperties.clear() + } + companion object { private val properties = ProxySystemProperties(ProxySystemProperties.Protocol.HTTP) diff --git a/teamscale-client/src/test/kotlin/com/teamscale/client/SystemProxyTest.kt b/teamscale-client/src/test/kotlin/com/teamscale/client/SystemProxyTest.kt new file mode 100644 index 000000000..8890a91db --- /dev/null +++ b/teamscale-client/src/test/kotlin/com/teamscale/client/SystemProxyTest.kt @@ -0,0 +1,151 @@ +package com.teamscale.client + +import com.github.markusbernhardt.proxy.selector.pac.PacProxySelector +import com.github.markusbernhardt.proxy.selector.pac.UrlPacScriptSource +import com.teamscale.client.TeamscaleServiceGenerator.buildUserAgent +import com.teamscale.client.TeamscaleServiceGenerator.createService +import okhttp3.HttpUrl.Companion.toHttpUrl +import okhttp3.mockwebserver.Dispatcher +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import okhttp3.mockwebserver.RecordedRequest +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import java.io.IOException +import java.net.Proxy +import java.net.ProxySelector +import java.net.SocketAddress +import java.net.URI + +/** + * Tests that the profiler resolves the operating system's proxy configuration, including Proxy-Auto-Config (PAC) files. + * + * These tests install a direct (no-proxy) [ProxySelector] as the JVM default so that a request is only ever routed + * through [proxyServer] if the profiler installs the detected system proxy selector on its OkHttp client. + */ +internal class SystemProxyTest { + + /** The proxy that the system configuration / PAC file points at and that requests should be routed through. */ + private lateinit var proxyServer: MockWebServer + + private val originalDefaultSelector: ProxySelector = ProxySelector.getDefault() + + private val originalSystemProxySelectorSupplier = HttpUtils.systemProxySelectorSupplier + + /** A [ProxySelector] that never returns a proxy, i.e., every connection stays direct. */ + private val directSelector = object : ProxySelector() { + override fun select(uri: URI?) = listOf(Proxy.NO_PROXY) + override fun connectFailed(uri: URI?, sa: SocketAddress?, ioe: IOException?) { + // Nothing to do + } + } + + @BeforeEach + fun setUp() { + proxyServer = MockWebServer() + proxyServer.start() + // Make the JVM default a direct connection, so requests only reach proxyServer if the profiler installs the + // system proxy selector itself. + ProxySelector.setDefault(directSelector) + } + + @AfterEach + fun tearDown() { + ProxySelector.setDefault(originalDefaultSelector) + HttpUtils.systemProxySelectorSupplier = originalSystemProxySelectorSupplier + System.clearProperty("java.net.useSystemProxies") + System.clearProperty("http.proxyHost") + System.clearProperty("http.proxyPort") + proxyServer.shutdown() + proxyServer.close() + } + + /** + * When `-Djava.net.useSystemProxies=true` is set and no explicit `teamscale.*` proxy is configured, the request is + * routed through the OS-configured proxy (here provided via the standard `http.proxyHost` JVM properties, which + * proxy-vole reads). The direct default selector guarantees the proxy is only hit because the profiler installed the + * detected selector. + */ + @Test + fun `system proxy is used when useSystemProxies is enabled`() { + System.setProperty("java.net.useSystemProxies", "true") + System.setProperty("http.proxyHost", proxyServer.hostName) + System.setProperty("http.proxyPort", proxyServer.port.toString()) + + makeRequestThroughProfiler() + + assertThat(proxyServer.requestCount).isEqualTo(1) + } + + /** + * Verifies that a PAC file is evaluated and that the resulting proxy is used by our OkHttp client. The PAC-aware + * selector is what the system detection resolves to, and the profiler installs it on its client; OkHttp then + * evaluates the PAC script and routes through the returned proxy. + */ + @Test + fun `proxy from a PAC file is evaluated and used`() { + val pacServer = MockWebServer() + val pacScript = + """function FindProxyForURL(url, host) { return "PROXY ${proxyServer.hostName}:${proxyServer.port}"; }""" + pacServer.dispatcher = object : Dispatcher() { + override fun dispatch(request: RecordedRequest) = + MockResponse() + .setHeader("Content-Type", "application/x-ns-proxy-autoconfig") + .setBody(pacScript) + } + pacServer.start() + + try { + System.setProperty("java.net.useSystemProxies", "true") + // Simulate the OS proxy detection resolving to a PAC-based selector. + HttpUtils.systemProxySelectorSupplier = { + PacProxySelector(UrlPacScriptSource(pacServer.url("/proxy.pac").toString())) + } + + makeRequestThroughProfiler() + + assertThat(proxyServer.requestCount).isEqualTo(1) + } finally { + pacServer.shutdown() + pacServer.close() + } + } + + /** + * Without the opt-in flag, the system proxy is never consulted and the connection stays direct, even when the OS + * would report a proxy. + */ + @Test + fun `no proxy is used without opt-in`() { + HttpUtils.systemProxySelectorSupplier = { directSelectorReturning(proxyServer) } + + makeRequestThroughProfiler() + + assertThat(proxyServer.requestCount).isZero() + } + + /** A [ProxySelector] that routes every request through the given server. */ + private fun directSelectorReturning(server: MockWebServer) = object : ProxySelector() { + override fun select(uri: URI?) = + listOf(Proxy(Proxy.Type.HTTP, java.net.InetSocketAddress(server.hostName, server.port))) + + override fun connectFailed(uri: URI?, sa: SocketAddress?, ioe: IOException?) { + // Nothing to do + } + } + + /** Sends one request to a (non-existent) Teamscale server through a client built the same way the profiler builds it. */ + private fun makeRequestThroughProfiler() { + val service = createService( + "http://teamscale.example.com".toHttpUrl(), + "someUser", "someAccessToken", + userAgent = buildUserAgent("Test Tool", "1.0.0") + ) + proxyServer.enqueue(MockResponse().setResponseCode(200)) + runCatching { + service.sendHeartbeat("", ProfilerInfo(ProcessInformation("", "", 0), null)).execute() + } + } +}