1
0
mirror of https://github.com/dart-lang/sdk synced 2024-07-05 09:20:04 +00:00

Disable security-related headers on redirect.

TESTED=updated unit tests
Bug: https://github.com/dart-lang/sdk/issues/45410
Change-Id: I7c555a4818fd719d42748b6a18780e3d9b3ee147
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/229947
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Brian Quinlan <bquinlan@google.com>
This commit is contained in:
Brian Quinlan 2022-01-27 20:49:42 +00:00 committed by Commit Bot
parent a2b7623d01
commit 57db739be0
5 changed files with 226 additions and 8 deletions

View File

@ -17,6 +17,13 @@
- `IdbFactory.supportsDatabaseNames` has been deprecated. It will always return
`false`.
#### `dart:io`
- **Breaking Change** [#45410](https://github.com/dart-lang/sdk/issues/45410):
`HttpClient` no longer transmits some headers (i.e. `authorization`,
`www-authenticate`, `cookie`, `cookie2`) when processing redirects to
a different domain.
### Tools
#### Dart command line

View File

@ -1715,8 +1715,39 @@ abstract class HttpClientRequest implements IOSink {
/// following the redirect.
///
/// All headers added to the request will be added to the redirection
/// request(s). However, any body send with the request will not be
/// part of the redirection request(s).
/// request(s) except when forwarding sensitive headers like
/// "Authorization", "WWW-Authenticate", and "Cookie". Those headers
/// will be skipped if following a redirect to a domain that is not a
/// subdomain match or exact match of the initial domain.
/// For example, a redirect from "foo.com" to either "foo.com" or
/// "sub.foo.com" will forward the sensitive headers, but a redirect to
/// "bar.com" will not.
///
/// Any body send with the request will not be part of the redirection
/// request(s).
///
/// For precise control of redirect handling, set this property to `false`
/// and make a separate HTTP request to process the redirect. For example:
///
/// ```dart
/// final client = HttpClient();
/// var uri = Uri.parse("http://localhost/");
/// var request = await client.getUrl(uri);
/// request.followRedirects = false;
/// var response = await request.close();
/// while (response.isRedirect) {
/// response.drain();
/// final location = response.headers.value(HttpHeaders.locationHeader);
/// if (location != null) {
/// uri = uri.resolve(location);
/// request = await client.getUrl(uri);
/// // Set the body or headers as desired.
/// request.followRedirects = false;
/// response = await request.close();
/// }
/// }
/// // Do something with the final response.
/// ```
bool followRedirects = true;
/// Set this property to the maximum number of redirects to follow

View File

@ -667,7 +667,7 @@ class _HttpClientResponse extends _HttpInboundMessageListInt
}
}
return _httpClient
._openUrlFromRequest(method, url, _httpRequest)
._openUrlFromRequest(method, url, _httpRequest, isRedirect: true)
.then((request) {
request._responseRedirects
..addAll(redirects)
@ -751,7 +751,8 @@ class _HttpClientResponse extends _HttpInboundMessageListInt
return drain().then((_) {
return _httpClient
._openUrlFromRequest(
_httpRequest.method, _httpRequest.uri, _httpRequest)
_httpRequest.method, _httpRequest.uri, _httpRequest,
isRedirect: false)
.then((request) => request.close());
});
}
@ -2715,8 +2716,31 @@ class _HttpClient implements HttpClient {
});
}
static bool _isSubdomain(Uri subdomain, Uri domain) {
return (subdomain.scheme == domain.scheme &&
subdomain.port == domain.port &&
(subdomain.host == domain.host ||
subdomain.host.endsWith("." + domain.host)));
}
static bool _shouldCopyHeaderOnRedirect(
String headerKey, Uri originalUrl, Uri redirectUri) {
if (_isSubdomain(redirectUri, originalUrl)) {
return true;
}
const nonRedirectHeaders = [
"authorization",
"www-authenticate",
"cookie",
"cookie2"
];
return !nonRedirectHeaders.contains(headerKey.toLowerCase());
}
Future<_HttpClientRequest> _openUrlFromRequest(
String method, Uri uri, _HttpClientRequest previous) {
String method, Uri uri, _HttpClientRequest previous,
{required bool isRedirect}) {
// If the new URI is relative (to either '/' or some sub-path),
// construct a full URI from the previous one.
Uri resolved = previous.uri.resolveUri(uri);
@ -2728,7 +2752,9 @@ class _HttpClient implements HttpClient {
..maxRedirects = previous.maxRedirects;
// Copy headers.
for (var header in previous.headers._headers.keys) {
if (request.headers[header] == null) {
if (request.headers[header] == null &&
(!isRedirect ||
_shouldCopyHeaderOnRedirect(header, resolved, previous.uri))) {
request.headers.set(header, previous.headers[header]!);
}
}

View File

@ -7,7 +7,7 @@ import "package:expect/expect.dart";
import "dart:async";
import "dart:io";
Future<HttpServer> setupServer() {
Future<HttpServer> setupServer({Uri? targetServer}) {
final completer = new Completer<HttpServer>();
HttpServer.bind("127.0.0.1", 0).then((server) {
var handlers = new Map<String, Function>();
@ -128,6 +128,8 @@ Future<HttpServer> setupServer() {
// Setup redirect checking headers.
addRequestHandler("/src", (HttpRequest request, HttpResponse response) {
Expect.equals("value", request.headers.value("X-Request-Header"));
Expect.isNotNull(request.headers.value("Authorization"),
"expected 'Authorization' header to be set");
response.headers.set(
HttpHeaders.locationHeader, "http://127.0.0.1:${server.port}/target");
response.statusCode = HttpStatus.movedPermanently;
@ -135,9 +137,24 @@ Future<HttpServer> setupServer() {
});
addRequestHandler("/target", (HttpRequest request, HttpResponse response) {
Expect.equals("value", request.headers.value("X-Request-Header"));
Expect.isNotNull(request.headers.value("Authorization"),
"expected 'Authorization' header to be set");
response.close();
});
if (targetServer != null) {
addRequestHandler("/src-crossdomain",
(HttpRequest request, HttpResponse response) {
Expect.equals("value", request.headers.value("X-Request-Header"));
Expect.isNotNull(request.headers.value("Authorization"),
"expected 'Authorization' header to be set");
response.headers
.set(HttpHeaders.locationHeader, targetServer.toString());
response.statusCode = HttpStatus.movedPermanently;
response.close();
});
}
// Setup redirect for 301 where POST should not redirect.
addRequestHandler("/301src", (HttpRequest request, HttpResponse response) {
Expect.equals("POST", request.method);
@ -183,6 +200,36 @@ Future<HttpServer> setupServer() {
return completer.future;
}
// A second HTTP server used to validate that redirect requests accross domains
// do *not* include security-related headers.
Future<HttpServer> setupTargetServer() {
final completer = new Completer<HttpServer>();
HttpServer.bind("127.0.0.1", 0).then((server) {
var handlers = new Map<String, Function>();
addRequestHandler(
String path, void handler(HttpRequest request, HttpResponse response)) {
handlers[path] = handler;
}
server.listen((HttpRequest request) {
if (request.uri.path == "/target") {
Expect.equals("value", request.headers.value("X-Request-Header"));
Expect.isNull(request.headers.value("Authorization"),
"expected 'Authorization' header to be removed on redirect");
request.response.close();
} else {
request.listen((_) {}, onDone: () {
request.response.statusCode = 404;
request.response.close();
});
}
});
completer.complete(server);
});
return completer.future;
}
void checkRedirects(int redirectCount, HttpClientResponse response) {
if (redirectCount < 2) {
Expect.isTrue(response.redirects.isEmpty);
@ -250,6 +297,7 @@ void testManualRedirectWithHeaders() {
.then((HttpClientRequest request) {
request.followRedirects = false;
request.headers.add("X-Request-Header", "value");
request.headers.add("Authorization", "Basic ...");
return request.close();
}).then(handleResponse);
});
@ -282,6 +330,7 @@ void testAutoRedirectWithHeaders() {
.getUrl(Uri.parse("http://127.0.0.1:${server.port}/src"))
.then((HttpClientRequest request) {
request.headers.add("X-Request-Header", "value");
request.headers.add("Authorization", "Basic ...");
return request.close();
}).then((HttpClientResponse response) {
response.listen((_) => Expect.fail("Response data not expected"),
@ -294,6 +343,33 @@ void testAutoRedirectWithHeaders() {
});
}
void testCrossDomainAutoRedirectWithHeaders() {
setupTargetServer().then((targetServer) {
setupServer(
targetServer:
Uri.parse("http://127.0.0.1:${targetServer.port}/target"))
.then((server) {
HttpClient client = new HttpClient();
client
.getUrl(Uri.parse("http://127.0.0.1:${server.port}/src-crossdomain"))
.then((HttpClientRequest request) {
request.headers.add("X-Request-Header", "value");
request.headers.add("Authorization", "Basic ...");
return request.close();
}).then((HttpClientResponse response) {
response.listen((_) => Expect.fail("Response data not expected"),
onDone: () {
Expect.equals(1, response.redirects.length);
targetServer.close();
server.close();
client.close();
});
});
});
});
}
void testAutoRedirect301POST() {
setupServer().then((server) {
HttpClient client = new HttpClient();
@ -441,6 +517,7 @@ main() {
testManualRedirectWithHeaders();
testAutoRedirect();
testAutoRedirectWithHeaders();
testCrossDomainAutoRedirectWithHeaders();
testAutoRedirect301POST();
testAutoRedirect303POST();
testAutoRedirectLimit();

View File

@ -9,7 +9,7 @@ import "package:expect/expect.dart";
import "dart:async";
import "dart:io";
Future<HttpServer> setupServer() {
Future<HttpServer> setupServer({Uri targetServer}) {
Completer completer = new Completer<HttpServer>();
HttpServer.bind("127.0.0.1", 0).then((server) {
var handlers = new Map<String, Function>();
@ -130,6 +130,8 @@ Future<HttpServer> setupServer() {
// Setup redirect checking headers.
addRequestHandler("/src", (HttpRequest request, HttpResponse response) {
Expect.equals("value", request.headers.value("X-Request-Header"));
Expect.isNotNull(request.headers.value("Authorization"),
"expected 'Authorization' header to be set");
response.headers.set(
HttpHeaders.locationHeader, "http://127.0.0.1:${server.port}/target");
response.statusCode = HttpStatus.movedPermanently;
@ -137,9 +139,24 @@ Future<HttpServer> setupServer() {
});
addRequestHandler("/target", (HttpRequest request, HttpResponse response) {
Expect.equals("value", request.headers.value("X-Request-Header"));
Expect.isNotNull(request.headers.value("Authorization"),
"expected 'Authorization' header to be set");
response.close();
});
if (targetServer != null) {
addRequestHandler("/src-crossdomain",
(HttpRequest request, HttpResponse response) {
Expect.equals("value", request.headers.value("X-Request-Header"));
Expect.isNotNull(request.headers.value("Authorization"),
"expected 'Authorization' header to be set");
response.headers
.set(HttpHeaders.locationHeader, targetServer.toString());
response.statusCode = HttpStatus.movedPermanently;
response.close();
});
}
// Setup redirect for 301 where POST should not redirect.
addRequestHandler("/301src", (HttpRequest request, HttpResponse response) {
Expect.equals("POST", request.method);
@ -185,6 +202,36 @@ Future<HttpServer> setupServer() {
return completer.future;
}
// A second HTTP server used to validate that redirect requests accross domains
// do *not* include security-related headers.
Future<HttpServer> setupTargetServer() {
Completer completer = new Completer<HttpServer>();
HttpServer.bind("127.0.0.1", 0).then((server) {
var handlers = new Map<String, Function>();
addRequestHandler(
String path, void handler(HttpRequest request, HttpResponse response)) {
handlers[path] = handler;
}
server.listen((HttpRequest request) {
if (request.uri.path == "/target") {
Expect.equals("value", request.headers.value("X-Request-Header"));
Expect.isNull(request.headers.value("Authorization"),
"expected 'Authorization' header to be removed on redirect");
request.response.close();
} else {
request.listen((_) {}, onDone: () {
request.response.statusCode = 404;
request.response.close();
});
}
});
completer.complete(server);
});
return completer.future;
}
void checkRedirects(int redirectCount, HttpClientResponse response) {
if (redirectCount < 2) {
Expect.isTrue(response.redirects.isEmpty);
@ -252,6 +299,7 @@ void testManualRedirectWithHeaders() {
.then((HttpClientRequest request) {
request.followRedirects = false;
request.headers.add("X-Request-Header", "value");
request.headers.add("Authorization", "Basic ...");
return request.close();
}).then(handleResponse);
});
@ -284,6 +332,7 @@ void testAutoRedirectWithHeaders() {
.getUrl(Uri.parse("http://127.0.0.1:${server.port}/src"))
.then((HttpClientRequest request) {
request.headers.add("X-Request-Header", "value");
request.headers.add("Authorization", "Basic ...");
return request.close();
}).then((HttpClientResponse response) {
response.listen((_) => Expect.fail("Response data not expected"),
@ -296,6 +345,33 @@ void testAutoRedirectWithHeaders() {
});
}
void testCrossDomainAutoRedirectWithHeaders() {
setupTargetServer().then((targetServer) {
setupServer(
targetServer:
Uri.parse("http://127.0.0.1:${targetServer.port}/target"))
.then((server) {
HttpClient client = new HttpClient();
client
.getUrl(Uri.parse("http://127.0.0.1:${server.port}/src-crossdomain"))
.then((HttpClientRequest request) {
request.headers.add("X-Request-Header", "value");
request.headers.add("Authorization", "Basic ...");
return request.close();
}).then((HttpClientResponse response) {
response.listen((_) => Expect.fail("Response data not expected"),
onDone: () {
Expect.equals(1, response.redirects.length);
targetServer.close();
server.close();
client.close();
});
});
});
});
}
void testAutoRedirect301POST() {
setupServer().then((server) {
HttpClient client = new HttpClient();
@ -443,6 +519,7 @@ main() {
testManualRedirectWithHeaders();
testAutoRedirect();
testAutoRedirectWithHeaders();
testCrossDomainAutoRedirectWithHeaders();
testAutoRedirect301POST();
testAutoRedirect303POST();
testAutoRedirectLimit();