Skip to content

Commit bb254dd

Browse files
committed
SCANJLIB-306 Restore authentication support for proxy + SSL
1 parent d81e195 commit bb254dd

File tree

5 files changed

+213
-14
lines changed

5 files changed

+213
-14
lines changed

.github/workflows/build.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ jobs:
8383
- name: Run ITs
8484
run: |
8585
cd its
86-
mvn -B -e verify -Prun-its -Dsonar.runtimeVersion=${{ matrix.SQ_VERSION }} -DjavaVersion=${{ env.JAVA_VERSION }}
86+
mvn -B -e verify -Prun-its -Dsonar.runtimeVersion=${{ matrix.SQ_VERSION }}
8787
8888
- name: Upload server logs
8989
if: failure()

its/it-tests/src/test/java/com/sonar/scanner/lib/it/ProxyTest.java

Lines changed: 181 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,19 @@
2626
import com.sonar.scanner.lib.it.tools.SimpleScanner;
2727
import java.io.IOException;
2828
import java.net.InetAddress;
29+
import java.nio.charset.StandardCharsets;
2930
import java.nio.file.Path;
3031
import java.nio.file.Paths;
32+
import java.util.Base64;
3133
import java.util.HashMap;
3234
import java.util.Map;
3335
import java.util.concurrent.ConcurrentLinkedDeque;
3436
import javax.servlet.ServletException;
3537
import javax.servlet.http.HttpServletRequest;
3638
import javax.servlet.http.HttpServletResponse;
3739
import org.eclipse.jetty.client.api.Request;
40+
import org.eclipse.jetty.http.HttpVersion;
41+
import org.eclipse.jetty.proxy.ConnectHandler;
3842
import org.eclipse.jetty.proxy.ProxyServlet;
3943
import org.eclipse.jetty.security.ConstraintMapping;
4044
import org.eclipse.jetty.security.ConstraintSecurityHandler;
@@ -46,12 +50,15 @@
4650
import org.eclipse.jetty.server.HttpConnectionFactory;
4751
import org.eclipse.jetty.server.Server;
4852
import org.eclipse.jetty.server.ServerConnector;
53+
import org.eclipse.jetty.server.SslConnectionFactory;
4954
import org.eclipse.jetty.server.handler.DefaultHandler;
5055
import org.eclipse.jetty.server.handler.HandlerCollection;
5156
import org.eclipse.jetty.servlet.ServletContextHandler;
5257
import org.eclipse.jetty.servlet.ServletHandler;
58+
import org.eclipse.jetty.servlet.ServletHolder;
5359
import org.eclipse.jetty.util.security.Constraint;
5460
import org.eclipse.jetty.util.security.Credential;
61+
import org.eclipse.jetty.util.ssl.SslContextFactory;
5562
import org.eclipse.jetty.util.thread.QueuedThreadPool;
5663
import org.junit.After;
5764
import org.junit.Before;
@@ -64,45 +71,62 @@ public class ProxyTest {
6471

6572
private static final String PROXY_USER = "scott";
6673
private static final String PROXY_PASSWORD = "tiger";
74+
75+
// SSL resources reused from SSLTest
76+
private static final String SERVER_KEYSTORE = "/SSLTest/server.p12";
77+
private static final String SERVER_KEYSTORE_PASSWORD = "pwdServerP12";
78+
private static final String KEYSTORE_CLIENT_WITH_CA = "/SSLTest/client-with-ca-keytool.p12";
79+
private static final String KEYSTORE_CLIENT_WITH_CA_PASSWORD = "pwdClientCAP12";
80+
6781
private static Server server;
6882
private static int httpProxyPort;
83+
// HTTPS reverse-proxy target, used for the HTTPS CONNECT tests
84+
private static Server httpsTargetServer;
85+
private static int httpsTargetPort;
6986

7087
@ClassRule
7188
public static final OrchestratorRule ORCHESTRATOR = ScannerJavaLibraryTestSuite.ORCHESTRATOR;
7289

73-
private static ConcurrentLinkedDeque<String> seenByProxy = new ConcurrentLinkedDeque<>();
90+
private static final ConcurrentLinkedDeque<String> seenByProxy = new ConcurrentLinkedDeque<>();
91+
private static final ConcurrentLinkedDeque<String> seenConnectByProxy = new ConcurrentLinkedDeque<>();
7492

7593
@Before
7694
public void deleteData() {
7795
ScannerJavaLibraryTestSuite.resetData(ORCHESTRATOR);
7896
seenByProxy.clear();
97+
seenConnectByProxy.clear();
7998
}
8099

81100
@After
82101
public void stopProxy() throws Exception {
83102
if (server != null && server.isStarted()) {
84103
server.stop();
85104
}
105+
if (httpsTargetServer != null && httpsTargetServer.isStarted()) {
106+
httpsTargetServer.stop();
107+
}
86108
}
87109

88110
private static void startProxy(boolean needProxyAuth) throws Exception {
89111
httpProxyPort = NetworkUtils.getNextAvailablePort(InetAddress.getLocalHost());
90112

91-
// Setup Threadpool
92113
QueuedThreadPool threadPool = new QueuedThreadPool();
93114
threadPool.setMaxThreads(500);
94115

95116
server = new Server(threadPool);
96117

97-
// HTTP Configuration
98118
HttpConfiguration httpConfig = new HttpConfiguration();
99119
httpConfig.setSecureScheme("https");
100120
httpConfig.setSendServerVersion(true);
101121
httpConfig.setSendDateHeader(false);
102122

103-
// Handler Structure
123+
// Wrap the ProxyServlet handler with a ConnectHandler so HTTPS CONNECT
124+
// tunnels are also handled (and authenticated) by the same proxy.
125+
TrackingConnectHandler connectHandler = new TrackingConnectHandler(needProxyAuth);
126+
connectHandler.setHandler(proxyHandler(needProxyAuth));
127+
104128
HandlerCollection handlers = new HandlerCollection();
105-
handlers.setHandlers(new Handler[] {proxyHandler(needProxyAuth), new DefaultHandler()});
129+
handlers.setHandlers(new Handler[] {connectHandler, new DefaultHandler()});
106130
server.setHandler(handlers);
107131

108132
ServerConnector http = new ServerConnector(server, new HttpConnectionFactory(httpConfig));
@@ -112,6 +136,55 @@ private static void startProxy(boolean needProxyAuth) throws Exception {
112136
server.start();
113137
}
114138

139+
/**
140+
* Starts a simple HTTPS reverse-proxy that forwards all traffic to the Orchestrator SonarQube
141+
* instance. Used as the HTTPS target in proxy-CONNECT tests.
142+
*/
143+
private static void startHttpsTargetServer() throws Exception {
144+
httpsTargetPort = NetworkUtils.getNextAvailablePort(InetAddress.getLocalHost());
145+
146+
QueuedThreadPool threadPool = new QueuedThreadPool();
147+
threadPool.setMaxThreads(500);
148+
149+
httpsTargetServer = new Server(threadPool);
150+
151+
HttpConfiguration httpConfig = new HttpConfiguration();
152+
httpConfig.setSecureScheme("https");
153+
httpConfig.setSecurePort(httpsTargetPort);
154+
httpConfig.setSendServerVersion(true);
155+
httpConfig.setSendDateHeader(false);
156+
157+
Path serverKeyStore = Paths.get(ProxyTest.class.getResource(SERVER_KEYSTORE).toURI()).toAbsolutePath();
158+
assertThat(serverKeyStore).exists();
159+
160+
ServerConnector sslConnector = buildServerConnector(serverKeyStore, httpConfig);
161+
httpsTargetServer.addConnector(sslConnector);
162+
163+
// Transparently forward all requests to the Orchestrator instance
164+
ServletContextHandler context = new ServletContextHandler();
165+
ServletHandler servletHandler = new ServletHandler();
166+
ServletHolder holder = servletHandler.addServletWithMapping(ProxyServlet.Transparent.class, "/*");
167+
holder.setInitParameter("proxyTo", ORCHESTRATOR.getServer().getUrl());
168+
context.setServletHandler(servletHandler);
169+
httpsTargetServer.setHandler(context);
170+
171+
httpsTargetServer.start();
172+
}
173+
174+
private static ServerConnector buildServerConnector(Path serverKeyStore, HttpConfiguration httpConfig) {
175+
SslContextFactory.Server sslContextFactory = new SslContextFactory.Server();
176+
sslContextFactory.setKeyStorePath(serverKeyStore.toString());
177+
sslContextFactory.setKeyStorePassword(SERVER_KEYSTORE_PASSWORD);
178+
sslContextFactory.setKeyManagerPassword(SERVER_KEYSTORE_PASSWORD);
179+
180+
HttpConfiguration httpsConfig = new HttpConfiguration(httpConfig);
181+
ServerConnector sslConnector = new ServerConnector(httpsTargetServer,
182+
new SslConnectionFactory(sslContextFactory, HttpVersion.HTTP_1_1.asString()),
183+
new HttpConnectionFactory(httpsConfig));
184+
sslConnector.setPort(httpsTargetPort);
185+
return sslConnector;
186+
}
187+
115188
private static ServletContextHandler proxyHandler(boolean needProxyAuth) {
116189
ServletContextHandler contextHandler = new ServletContextHandler();
117190
if (needProxyAuth) {
@@ -155,6 +228,65 @@ private static ServletHandler newServletHandler() {
155228
return handler;
156229
}
157230

231+
/**
232+
* ConnectHandler subclass that:
233+
* <ul>
234+
* <li>Optionally requires {@code Proxy-Authorization} on CONNECT requests</li>
235+
* <li>Records the host:port of every successfully-authenticated CONNECT</li>
236+
* </ul>
237+
* <p>
238+
* When authentication is required and credentials are missing, the handler sends a {@code 407}
239+
* and <em>immediately closes the TCP connection</em> rather than leaving it open for a
240+
* challenge-response retry. This is intentional: it forces the JDK {@code HttpClient} to throw
241+
* an {@code IOException} (tunnel failed) rather than returning the {@code 407} as a readable HTTP
242+
* response to the application. Without the {@link java.net.Authenticator}-based fix in
243+
* {@code HttpClientFactory}, the scanner has no way to recover from that {@code IOException} and
244+
* the scan fails — reproducing the real-world regression.
245+
*/
246+
private static class TrackingConnectHandler extends ConnectHandler {
247+
248+
private final boolean requireAuth;
249+
250+
TrackingConnectHandler(boolean requireAuth) {
251+
this.requireAuth = requireAuth;
252+
}
253+
254+
@Override
255+
protected void handleConnect(org.eclipse.jetty.server.Request baseRequest, HttpServletRequest request,
256+
HttpServletResponse response, String serverAddress) {
257+
if (requireAuth && !hasValidCredentials(request)) {
258+
// Send a minimal 407 and then immediately shut down the connection so
259+
// the JDK cannot read a clean HTTP response — it will throw IOException.
260+
try {
261+
response.setStatus(HttpServletResponse.SC_PROXY_AUTHENTICATION_REQUIRED);
262+
response.setHeader("Proxy-Authenticate", "Basic realm=\"proxy\"");
263+
response.flushBuffer();
264+
} catch (IOException ignored) {
265+
// best-effort flush before closing
266+
}
267+
baseRequest.setHandled(true);
268+
baseRequest.getHttpChannel().getEndPoint().close();
269+
return;
270+
}
271+
seenConnectByProxy.add(serverAddress);
272+
super.handleConnect(baseRequest, request, response, serverAddress);
273+
}
274+
275+
private static boolean hasValidCredentials(HttpServletRequest request) {
276+
String credentials = request.getHeader("Proxy-Authorization");
277+
if (credentials != null && credentials.startsWith("Basic ")) {
278+
String decoded = new String(Base64.getDecoder().decode(credentials.substring(6)), StandardCharsets.ISO_8859_1);
279+
int colon = decoded.indexOf(':');
280+
if (colon > 0) {
281+
String user = decoded.substring(0, colon);
282+
String pass = decoded.substring(colon + 1);
283+
return PROXY_USER.equals(user) && PROXY_PASSWORD.equals(pass);
284+
}
285+
}
286+
return false;
287+
}
288+
}
289+
158290
public static class MyProxyServlet extends ProxyServlet {
159291

160292
@Override
@@ -202,6 +334,8 @@ public void simple_analysis_with_proxy_auth() throws Exception {
202334
SimpleScanner scanner = new SimpleScanner();
203335

204336
Map<String, String> params = new HashMap<>();
337+
// By default no request to localhost will use proxy
338+
params.put("http.nonProxyHosts", "");
205339
params.put("sonar.scanner.proxyHost", "localhost");
206340
params.put("sonar.scanner.proxyPort", "" + httpProxyPort);
207341

@@ -218,4 +352,46 @@ public void simple_analysis_with_proxy_auth() throws Exception {
218352
assertThat(buildResult.getLastStatus()).isZero();
219353
}
220354

355+
/**
356+
* Reproduces the regression reported for SonarScanner CLI 8.0 (java-library 4.0):
357+
* HTTPS proxy authentication was broken — the {@code Proxy-Authorization} header was
358+
* not sent on the CONNECT tunnel, so the proxy kept returning 407.
359+
* <p>
360+
* This test uses a local HTTP forward proxy that enforces authentication on CONNECT
361+
* requests, plus a local HTTPS reverse-proxy that forwards to the running SonarQube
362+
* instance. This mirrors the real-world topology: scanner → HTTP proxy (CONNECT) →
363+
* HTTPS SonarQube.
364+
*/
365+
@Test
366+
public void simple_analysis_with_https_proxy_auth() throws Exception {
367+
startProxy(true);
368+
startHttpsTargetServer();
369+
SimpleScanner scanner = new SimpleScanner();
370+
371+
Path clientTruststore = Paths.get(ProxyTest.class.getResource(KEYSTORE_CLIENT_WITH_CA).toURI()).toAbsolutePath();
372+
assertThat(clientTruststore).exists();
373+
374+
Map<String, String> params = new HashMap<>();
375+
// By default no request to localhost will use proxy
376+
params.put("http.nonProxyHosts", "");
377+
params.put("sonar.scanner.proxyHost", "localhost");
378+
params.put("sonar.scanner.proxyPort", "" + httpProxyPort);
379+
// Trust the self-signed certificate used by the local HTTPS target
380+
params.put("sonar.scanner.truststorePath", clientTruststore.toString());
381+
params.put("sonar.scanner.truststorePassword", KEYSTORE_CLIENT_WITH_CA_PASSWORD);
382+
383+
// Without proxy credentials the CONNECT tunnel should be rejected (407)
384+
BuildResult buildResult = scanner.executeSimpleProject(project("js-sample"), "https://localhost:" + httpsTargetPort, params, Map.of());
385+
assertThat(buildResult.getLastStatus()).isNotZero();
386+
assertThat(buildResult.getLogs()).containsIgnoringCase("Failed to query server version");
387+
assertThat(seenConnectByProxy).isEmpty();
388+
389+
// With proxy credentials the CONNECT tunnel must succeed and the full analysis must pass
390+
params.put("sonar.scanner.proxyUser", PROXY_USER);
391+
params.put("sonar.scanner.proxyPassword", PROXY_PASSWORD);
392+
buildResult = scanner.executeSimpleProject(project("js-sample"), "https://localhost:" + httpsTargetPort, params, Map.of());
393+
assertThat(buildResult.getLastStatus()).isZero();
394+
assertThat(seenConnectByProxy).isNotEmpty();
395+
}
396+
221397
}

lib/src/main/java/org/sonarsource/scanner/lib/internal/http/HttpConfig.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,6 @@ private static Duration loadDuration(Map<String, String> bootstrapProperties, St
144144

145145
@Nullable
146146
private static Proxy loadProxy(Map<String, String> bootstrapProperties) {
147-
// OkHttp detects 'http.proxyHost' java property already, so just focus on sonar-specific properties
148147
String proxyHost = defaultIfBlank(bootstrapProperties.get(SONAR_SCANNER_PROXY_HOST), null);
149148
if (proxyHost != null) {
150149
int proxyPort;

lib/src/main/java/org/sonarsource/scanner/lib/internal/http/ScannerHttpClient.java

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -146,21 +146,21 @@ private <G> G callUrlWithRedirects(String url, boolean authentication, @Nullable
146146
}
147147

148148
private <G> G callUrlWithRedirectsAndProxyAuth(String url, boolean authentication, @Nullable String acceptHeader, ResponseHandler<G> responseHandler,
149-
int redirectCount, boolean proxyAuthAttempted) {
149+
int redirectCount, boolean proxyAuthRetry) {
150150
if (redirectCount > 10) {
151151
throw new IllegalStateException("Too many redirects (>10) for URL: " + url);
152152
}
153153

154-
var request = prepareRequest(url, acceptHeader, authentication, proxyAuthAttempted);
154+
var request = prepareRequest(url, acceptHeader, authentication, proxyAuthRetry);
155155

156156
HttpResponse<InputStream> response = null;
157157
Instant start = Instant.now();
158158
try {
159159
LOG.debug("--> {} {}", request.method(), request.uri());
160160
response = sharedHttpClient.send(request, HttpResponse.BodyHandlers.ofInputStream());
161161

162-
if (response.statusCode() == 407 && !proxyAuthAttempted && httpConfig.getProxyUser() != null) {
163-
LOG.debug("Received 407 Proxy Authentication Required, retrying with Proxy-Authorization header");
162+
if (response.statusCode() == 407 && !proxyAuthRetry && httpConfig.getProxyUser() != null) {
163+
LOG.debug("Received 407 Proxy Authentication Required, retrying request");
164164
return callUrlWithRedirectsAndProxyAuth(url, authentication, acceptHeader, responseHandler, redirectCount, true);
165165
}
166166

@@ -172,7 +172,7 @@ private <G> G callUrlWithRedirectsAndProxyAuth(String url, boolean authenticatio
172172
URI originalUri = URI.create(url);
173173
redirectUrl = originalUri.getScheme() + "://" + originalUri.getAuthority() + redirectUrl;
174174
}
175-
return callUrlWithRedirectsAndProxyAuth(redirectUrl, authentication, acceptHeader, responseHandler, redirectCount + 1, proxyAuthAttempted);
175+
return callUrlWithRedirectsAndProxyAuth(redirectUrl, authentication, acceptHeader, responseHandler, redirectCount + 1, proxyAuthRetry);
176176
}
177177
}
178178

@@ -216,7 +216,7 @@ private interface ResponseHandler<G> {
216216
G apply(HttpResponse<InputStream> response) throws IOException;
217217
}
218218

219-
private HttpRequest prepareRequest(String url, @Nullable String acceptHeader, boolean authentication, boolean addProxyAuth) {
219+
private HttpRequest prepareRequest(String url, @Nullable String acceptHeader, boolean authentication, boolean proxyAuthRetry) {
220220
var timeout = httpConfig.getResponseTimeout().isZero() ? httpConfig.getSocketTimeout() : httpConfig.getResponseTimeout();
221221

222222
var requestBuilder = HttpRequest.newBuilder()
@@ -239,7 +239,11 @@ private HttpRequest prepareRequest(String url, @Nullable String acceptHeader, bo
239239
}
240240
}
241241

242-
if (addProxyAuth && httpConfig.getProxyUser() != null) {
242+
// Preemptively send proxy credentials on every request. The JDK HttpClient forwards
243+
// Proxy-Authorization from the application request to CONNECT tunnel requests for HTTPS
244+
// targets, so sending it upfront avoids a round-trip 407 challenge and works reliably
245+
// across JDK versions.
246+
if (httpConfig.getProxyUser() != null) {
243247
String proxyCredentials = httpConfig.getProxyUser() + ":" + (httpConfig.getProxyPassword() != null ? httpConfig.getProxyPassword() : "");
244248
String encodedProxyCredentials = Base64.getEncoder().encodeToString(proxyCredentials.getBytes(StandardCharsets.UTF_8));
245249
requestBuilder.header("Proxy-Authorization", "Basic " + encodedProxyCredentials);

lib/src/test/java/org/sonarsource/scanner/lib/internal/http/ScannerHttpClientTest.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,26 @@ void should_honor_scanner_proxy_settings_with_auth() {
316316
.withHeader("Proxy-Authorization", equalTo("Basic " + Base64.getEncoder().encodeToString("proxyUser:proxyPassword".getBytes(StandardCharsets.UTF_8)))));
317317
}
318318

319+
@Test
320+
void should_send_proxy_auth_preemptively_without_407_challenge() {
321+
sonarqube.stubFor(get("/batch/index.txt").willReturn(aResponse().withBody(HELLO_WORLD)));
322+
323+
Map<String, String> props = new HashMap<>();
324+
props.put(ScannerProperties.SONAR_SCANNER_PROXY_HOST, "localhost");
325+
props.put(ScannerProperties.SONAR_SCANNER_PROXY_PORT, String.valueOf(proxyMock.getPort()));
326+
props.put(ScannerProperties.SONAR_SCANNER_PROXY_USER, "proxyUser");
327+
props.put(ScannerProperties.SONAR_SCANNER_PROXY_PASSWORD, "proxyPassword");
328+
329+
ScannerHttpClient underTest = create(sonarqube.baseUrl(), props);
330+
String response = underTest.callWebApi("/batch/index.txt");
331+
332+
assertThat(response).isEqualTo(HELLO_WORLD);
333+
// Proxy-Authorization must be present on the very first request — no 407 round-trip
334+
proxyMock.verify(1, getRequestedFor(urlMatching("/batch/.*"))
335+
.withHeader("Proxy-Authorization",
336+
equalTo("Basic " + Base64.getEncoder().encodeToString("proxyUser:proxyPassword".getBytes(StandardCharsets.UTF_8)))));
337+
}
338+
319339
@Test
320340
@Tag(PROXY_AUTH_ENABLED)
321341
@RestoreSystemProperties

0 commit comments

Comments
 (0)