Skip to content

Commit 38e45c9

Browse files
adrian05-msadrian05-ms
authored
Merge pull request #2081 from microsoft/ramsess/fix-redirectHeaderVulnerability
fix(security): vulnerability where all headers are passed on redirect
2 parents 430aeac + 9b590c8 commit 38e45c9

3 files changed

Lines changed: 476 additions & 18 deletions

File tree

‎components/http/okHttp/src/main/java/com/microsoft/kiota/http/middleware/RedirectHandler.java‎

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -29,24 +29,38 @@
2929
*/
3030
public class RedirectHandler implements Interceptor {
3131
@Nonnull private final RedirectHandlerOption mRedirectOption;
32+
@Nullable private final java.net.ProxySelector mProxySelector;
3233

3334
/**
3435
* Initialize using default redirect options, default IShouldRedirect and max redirect value
3536
*/
3637
public RedirectHandler() {
37-
this(null);
38+
this(null, null);
3839
}
3940

4041
/**
4142
* Initialize using custom redirect options.
4243
* @param redirectOption pass instance of redirect options to be used
4344
*/
4445
public RedirectHandler(@Nullable final RedirectHandlerOption redirectOption) {
46+
this(redirectOption, null);
47+
}
48+
49+
/**
50+
* Initialize using custom redirect options and proxy selector.
51+
* @param redirectOption pass instance of redirect options to be used
52+
* @param proxySelector The ProxySelector to use for determining proxy configuration, or null to use the system default
53+
*/
54+
public RedirectHandler(
55+
@Nullable final RedirectHandlerOption redirectOption,
56+
@Nullable final java.net.ProxySelector proxySelector) {
4557
if (redirectOption == null) {
4658
this.mRedirectOption = new RedirectHandlerOption();
4759
} else {
4860
this.mRedirectOption = redirectOption;
4961
}
62+
this.mProxySelector =
63+
proxySelector != null ? proxySelector : java.net.ProxySelector.getDefault();
5064
}
5165

5266
boolean isRedirected(
@@ -81,7 +95,10 @@ boolean isRedirected(
8195
return false;
8296
}
8397

84-
Request getRedirect(final Request request, final Response userResponse)
98+
Request getRedirect(
99+
final Request request,
100+
final Response userResponse,
101+
final RedirectHandlerOption redirectOption)
85102
throws ProtocolException {
86103
String location = userResponse.header("Location");
87104
if (location == null || location.length() == 0) return null;
@@ -95,32 +112,29 @@ Request getRedirect(final Request request, final Response userResponse)
95112
location = request.url() + location;
96113
}
97114

98-
HttpUrl requestUrl = userResponse.request().url();
115+
HttpUrl requestUrl = request.url();
99116

100-
HttpUrl locationUrl = userResponse.request().url().resolve(location);
117+
HttpUrl locationUrl = request.url().resolve(location);
101118

102119
// Don't follow redirects to unsupported protocols.
103120
if (locationUrl == null) return null;
104121

105122
// Most redirects don't include a request body.
106-
Request.Builder requestBuilder = userResponse.request().newBuilder();
107-
108-
// When redirecting across hosts, drop all authentication headers. This
109-
// is potentially annoying to the application layer since they have no
110-
// way to retain them.
111-
boolean sameScheme = locationUrl.scheme().equalsIgnoreCase(requestUrl.scheme());
112-
boolean sameHost =
113-
locationUrl.host().toString().equalsIgnoreCase(requestUrl.host().toString());
114-
if (!sameScheme || !sameHost) {
115-
requestBuilder.removeHeader("Authorization");
116-
}
123+
Request.Builder requestBuilder = userResponse.request().newBuilder().url(locationUrl);
124+
125+
// Scrub sensitive headers before following the redirect
126+
java.util.function.Function<HttpUrl, java.net.Proxy> proxyResolver =
127+
RedirectHandlerOption.getProxyResolver(mProxySelector);
128+
redirectOption
129+
.scrubSensitiveHeaders()
130+
.scrubHeaders(requestBuilder, requestUrl, proxyResolver);
117131

118132
// Response status code 303 See Other then POST changes to GET
119133
if (userResponse.code() == HTTP_SEE_OTHER) {
120134
requestBuilder.method("GET", null);
121135
}
122136

123-
return requestBuilder.url(locationUrl).build();
137+
return requestBuilder.build();
124138
}
125139

126140
// Intercept request and response made to network
@@ -163,7 +177,8 @@ Request getRedirect(final Request request, final Response userResponse)
163177
isRedirected(request, response, requestsCount, redirectOption)
164178
&& redirectOption.shouldRedirect().shouldRedirect(response);
165179

166-
final Request followup = shouldRedirect ? getRedirect(request, response) : null;
180+
final Request followup =
181+
shouldRedirect ? getRedirect(request, response, redirectOption) : null;
167182
if (followup != null) {
168183
response.close();
169184
request = followup;

‎components/http/okHttp/src/main/java/com/microsoft/kiota/http/middleware/options/RedirectHandlerOption.java‎

Lines changed: 114 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,16 @@
55
import jakarta.annotation.Nonnull;
66
import jakarta.annotation.Nullable;
77

8+
import okhttp3.HttpUrl;
9+
import okhttp3.Request;
10+
11+
import java.net.Proxy;
12+
import java.net.ProxySelector;
13+
import java.net.URI;
14+
import java.util.List;
15+
import java.util.Objects;
16+
import java.util.function.Function;
17+
818
/**
919
* Options to be passed to the redirect middleware.
1020
*/
@@ -28,11 +38,63 @@ public class RedirectHandlerOption implements RequestOption {
2838
*/
2939
@Nonnull public static final IShouldRedirect DEFAULT_SHOULD_REDIRECT = response -> true;
3040

41+
@Nonnull private final IScrubSensitiveHeaders scrubSensitiveHeaders;
42+
43+
/**
44+
* Functional interface for scrubbing sensitive headers during redirects.
45+
*/
46+
@FunctionalInterface
47+
public interface IScrubSensitiveHeaders {
48+
/**
49+
* Scrubs sensitive headers from the request before following a redirect.
50+
* @param requestBuilder The request builder to modify
51+
* @param originalUrl The original request URL
52+
* @param proxyResolver A function that returns the proxy for a given destination, or null if no proxy applies
53+
*/
54+
void scrubHeaders(
55+
@Nonnull Request.Builder requestBuilder,
56+
@Nonnull HttpUrl originalUrl,
57+
@Nullable Function<HttpUrl, Proxy> proxyResolver);
58+
}
59+
60+
/**
61+
* The default implementation for scrubbing sensitive headers during redirects.
62+
* This method removes Authorization and Cookie headers when the host, scheme, or port changes,
63+
* and removes Proxy-Authorization headers when no proxy is configured or the proxy is bypassed for the new URL.
64+
*/
65+
@Nonnull public static final IScrubSensitiveHeaders DEFAULT_SCRUB_SENSITIVE_HEADERS =
66+
(requestBuilder, originalUrl, proxyResolver) -> {
67+
Objects.requireNonNull(requestBuilder, "parameter requestBuilder cannot be null");
68+
Objects.requireNonNull(originalUrl, "parameter originalUrl cannot be null");
69+
70+
// Get the new URL from the request builder
71+
HttpUrl newUrl = requestBuilder.build().url();
72+
Objects.requireNonNull(newUrl, "The request URL cannot be null");
73+
74+
// Remove Authorization and Cookie headers if the request's scheme, host, or port
75+
// changes
76+
boolean isDifferentOrigin =
77+
!newUrl.host().equalsIgnoreCase(originalUrl.host())
78+
|| !newUrl.scheme().equalsIgnoreCase(originalUrl.scheme())
79+
|| newUrl.port() != originalUrl.port();
80+
if (isDifferentOrigin) {
81+
requestBuilder.removeHeader("Authorization");
82+
requestBuilder.removeHeader("Cookie");
83+
}
84+
85+
// Remove Proxy-Authorization if no proxy is configured or the URL is bypassed
86+
boolean isProxyInactive =
87+
proxyResolver == null || proxyResolver.apply(newUrl) == null;
88+
if (isProxyInactive) {
89+
requestBuilder.removeHeader("Proxy-Authorization");
90+
}
91+
};
92+
3193
/**
3294
* Create default instance of redirect options, with default values of max redirects and should redirect
3395
*/
3496
public RedirectHandlerOption() {
35-
this(DEFAULT_MAX_REDIRECTS, DEFAULT_SHOULD_REDIRECT);
97+
this(DEFAULT_MAX_REDIRECTS, DEFAULT_SHOULD_REDIRECT, DEFAULT_SCRUB_SENSITIVE_HEADERS);
3698
}
3799

38100
/**
@@ -41,13 +103,30 @@ public RedirectHandlerOption() {
41103
* @param shouldRedirect Should redirect callback called before every redirect
42104
*/
43105
public RedirectHandlerOption(int maxRedirects, @Nullable final IShouldRedirect shouldRedirect) {
106+
this(maxRedirects, shouldRedirect, DEFAULT_SCRUB_SENSITIVE_HEADERS);
107+
}
108+
109+
/**
110+
* Create an instance with provided values
111+
* @param maxRedirects Max redirects to occur
112+
* @param shouldRedirect Should redirect callback called before every redirect
113+
* @param scrubSensitiveHeaders Callback to scrub sensitive headers during redirects
114+
*/
115+
public RedirectHandlerOption(
116+
int maxRedirects,
117+
@Nullable final IShouldRedirect shouldRedirect,
118+
@Nullable final IScrubSensitiveHeaders scrubSensitiveHeaders) {
44119
if (maxRedirects < 0)
45120
throw new IllegalArgumentException("Max redirects cannot be negative");
46121
if (maxRedirects > MAX_REDIRECTS)
47122
throw new IllegalArgumentException("Max redirect cannot exceed " + MAX_REDIRECTS);
48123

49124
this.maxRedirects = maxRedirects;
50125
this.shouldRedirect = shouldRedirect != null ? shouldRedirect : DEFAULT_SHOULD_REDIRECT;
126+
this.scrubSensitiveHeaders =
127+
scrubSensitiveHeaders != null
128+
? scrubSensitiveHeaders
129+
: DEFAULT_SCRUB_SENSITIVE_HEADERS;
51130
}
52131

53132
/**
@@ -66,6 +145,40 @@ public int maxRedirects() {
66145
return this.shouldRedirect;
67146
}
68147

148+
/**
149+
* Gets the callback for scrubbing sensitive headers during redirects.
150+
* @return scrub sensitive headers callback
151+
*/
152+
@Nonnull public IScrubSensitiveHeaders scrubSensitiveHeaders() {
153+
return this.scrubSensitiveHeaders;
154+
}
155+
156+
/**
157+
* Helper method to get a proxy resolver from a ProxySelector.
158+
* @param proxySelector The ProxySelector to use, or null if no proxy is configured
159+
* @return A function that resolves proxies for a given HttpUrl, or null if no proxy selector is provided
160+
*/
161+
@Nullable public static Function<HttpUrl, Proxy> getProxyResolver(
162+
@Nullable final ProxySelector proxySelector) {
163+
if (proxySelector == null) {
164+
return null;
165+
}
166+
return url -> {
167+
try {
168+
URI uri = new URI(url.scheme(), null, url.host(), url.port(), null, null, null);
169+
List<Proxy> proxies = proxySelector.select(uri);
170+
if (proxies != null && !proxies.isEmpty()) {
171+
Proxy proxy = proxies.get(0);
172+
// Return null for DIRECT proxies (no proxy)
173+
return proxy.type() == Proxy.Type.DIRECT ? null : proxy;
174+
}
175+
return null;
176+
} catch (Exception e) {
177+
return null;
178+
}
179+
};
180+
}
181+
69182
/** {@inheritDoc} */
70183
@SuppressWarnings("unchecked")
71184
@Override

0 commit comments

Comments
 (0)