Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,12 @@ public boolean exitAfterIntercept(Channel channel, NettyResponseFuture<?> future
ProxyServer proxyServer = future.getProxyServer();
int statusCode = response.status().code();
Request request = future.getCurrentRequest();
Realm realm = request.getRealm() != null ? request.getRealm() : config.getRealm();
// Use the realm the future is carrying (seeded from the request or client config when the
// exchange started, and reset to null by Redirect30xInterceptor on a cross-origin or
// scheme-downgrade redirect). Re-deriving from config.getRealm() here re-attaches the
// client-wide credentials to a redirect target whose auth was just stripped, leaking them
// to a different origin that answers 401.
Realm realm = future.getRealm();

// This MUST BE called before Redirect30xInterceptor because latter assumes cookie store is already updated
CookieStore cookieStore = config.getCookieStore();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ public class RedirectCredentialSecurityTest {
private static final AtomicReference<String> cookieOnBounceBack = new AtomicReference<>();
private static final AtomicReference<String> authAfterHttpsDowngrade = new AtomicReference<>();
private static final AtomicReference<String> cookieAfterHttpsDowngrade = new AtomicReference<>();
private static final AtomicReference<String> authOn401Target = new AtomicReference<>();

@BeforeAll
public static void startServers() throws Exception {
Expand Down Expand Up @@ -194,6 +195,24 @@ public static void startServers() throws Exception {
exchange.close();
});

// Cross-domain redirect to a target that answers 401: the target must never receive
// credentials, even those configured client-wide via config.setRealm(...).
serverA.createContext("/redirect-to-b-401", exchange -> {
exchange.getResponseHeaders().add("Location", "http://127.0.0.1:" + portB + "/target-401");
exchange.sendResponseHeaders(302, -1);
exchange.close();
});

serverB.createContext("/target-401", exchange -> {
String auth = exchange.getRequestHeaders().getFirst("Authorization");
if (auth != null) {
authOn401Target.set(auth);
}
exchange.getResponseHeaders().add("WWW-Authenticate", "Basic realm=\"target\"");
exchange.sendResponseHeaders(401, -1);
exchange.close();
});

// HTTPS server on 127.0.0.1: issues a redirect downgrading to plain HTTP on server B
httpsServer = HttpsServer.create(new InetSocketAddress("127.0.0.1", 0), 0);
httpsServer.setHttpsConfigurator(new HttpsConfigurator(buildSslContext()));
Expand Down Expand Up @@ -700,6 +719,29 @@ void portChangeOnSameHostIsTreatedAsCrossOrigin() throws Exception {
}
}

/**
* Client-wide credentials set via {@code config.setRealm(...)} must not be sent to a
* cross-domain redirect target, even when that target answers 401 to solicit them. The
* redirect clears the request/future realm, but the config realm must not be re-applied.
*/
@Test
void crossDomainRedirectTo401TargetDoesNotLeakConfigRealm() throws Exception {
DefaultAsyncHttpClientConfig config = new DefaultAsyncHttpClientConfig.Builder()
.setFollowRedirect(true)
.setRealm(basicAuthRealm("user", "password").build())
.build();
try (DefaultAsyncHttpClient client = new DefaultAsyncHttpClient(config)) {
authOn401Target.set(null);

client.prepareGet("http://127.0.0.1:" + portA + "/redirect-to-b-401")
.execute()
.get(5, TimeUnit.SECONDS);

assertNull(authOn401Target.get(),
"client-wide config Realm must not be sent to a cross-domain 401 target after redirect");
}
}

/**
* HTTPS-to-HTTP same-host downgrade strips both Cookie and Authorization. Exercises the
* {@code schemeDowngrade} branch in Redirect30xInterceptor.
Expand Down
Loading