From: csagan5 <32685696+csagan5@users.noreply.github.com> Date: Tue, 28 Jul 2020 12:28:58 +0200 Subject: Block gateway attacks via websockets This approach is not comprehensive, see also: * https://bugs.chromium.org/p/chromium/issues/detail?id=590714 License: GPL-3.0-only - https://spdx.org/licenses/GPL-3.0-only.html --- .../execution_context/execution_context.cc | 16 ++++++++++ .../execution_context/execution_context.h | 1 + .../renderer/core/loader/base_fetch_context.h | 1 + .../core/loader/frame_fetch_context.cc | 20 ++++++++++++ .../core/loader/frame_fetch_context.h | 1 + .../core/loader/worker_fetch_context.cc | 21 +++++++++++++ .../core/loader/worker_fetch_context.h | 1 + .../core/workers/installed_scripts_manager.cc | 4 +-- .../background_fetch_manager.cc | 31 +++++++++++++++++++ .../websockets/websocket_channel_impl.cc | 5 +++ .../modules/websockets/websocket_common.cc | 30 ++++++++++++++++++ .../modules/websockets/websocket_common.h | 4 +++ 12 files changed, 133 insertions(+), 2 deletions(-) diff --git a/third_party/blink/renderer/core/execution_context/execution_context.cc b/third_party/blink/renderer/core/execution_context/execution_context.cc --- a/third_party/blink/renderer/core/execution_context/execution_context.cc +++ b/third_party/blink/renderer/core/execution_context/execution_context.cc @@ -713,4 +713,20 @@ void ExecutionContext::WriteIntoTrace( proto->set_world_type(GetWorldType(*this)); } +String ExecutionContext::addressSpaceForBindings() const { + switch (AddressSpace()) { + case network::mojom::IPAddressSpace::kPublic: + case network::mojom::IPAddressSpace::kUnknown: + return "public"; + + case network::mojom::IPAddressSpace::kPrivate: + return "private"; + + case network::mojom::IPAddressSpace::kLocal: + return "local"; + } + NOTREACHED(); + return "public"; +} + } // namespace blink diff --git a/third_party/blink/renderer/core/execution_context/execution_context.h b/third_party/blink/renderer/core/execution_context/execution_context.h --- a/third_party/blink/renderer/core/execution_context/execution_context.h +++ b/third_party/blink/renderer/core/execution_context/execution_context.h @@ -391,6 +391,7 @@ class CORE_EXPORT ExecutionContext : public Supplementable, void SetAddressSpace(network::mojom::blink::IPAddressSpace ip_address_space); HeapObserverSet& ContextLifecycleObserverSet(); + String addressSpaceForBindings() const; unsigned ContextLifecycleStateObserverCountForTesting() const; // Implementation of WindowOrWorkerGlobalScope.crossOriginIsolated. diff --git a/third_party/blink/renderer/core/loader/base_fetch_context.h b/third_party/blink/renderer/core/loader/base_fetch_context.h --- a/third_party/blink/renderer/core/loader/base_fetch_context.h +++ b/third_party/blink/renderer/core/loader/base_fetch_context.h @@ -89,6 +89,7 @@ class CORE_EXPORT BaseFetchContext : public FetchContext { virtual SubresourceFilter* GetSubresourceFilter() const = 0; virtual bool ShouldBlockWebSocketByMixedContentCheck(const KURL&) const = 0; + virtual bool ShouldBlockGateWayAttacks(network::mojom::IPAddressSpace requestor_space, const KURL&) const = 0; virtual std::unique_ptr CreateWebSocketHandshakeThrottle() = 0; diff --git a/third_party/blink/renderer/core/loader/frame_fetch_context.cc b/third_party/blink/renderer/core/loader/frame_fetch_context.cc --- a/third_party/blink/renderer/core/loader/frame_fetch_context.cc +++ b/third_party/blink/renderer/core/loader/frame_fetch_context.cc @@ -588,6 +588,26 @@ bool FrameFetchContext::ShouldBlockRequestByInspector(const KURL& url) const { return should_block_request; } +bool FrameFetchContext::ShouldBlockGateWayAttacks(network::mojom::IPAddressSpace requestor_space, const KURL& request_url) const { + // TODO(mkwst): This only checks explicit IP addresses. We'll have to move + // all this up to //net and //content in order to have any real impact on + // gateway attacks. That turns out to be a TON of work (crbug.com/378566). + if (requestor_space == network::mojom::IPAddressSpace::kUnknown) + requestor_space = network::mojom::IPAddressSpace::kPublic; + network::mojom::IPAddressSpace target_space = + network::mojom::IPAddressSpace::kPublic; + if (network_utils::IsReservedIPAddress(request_url.Host())) + target_space = network::mojom::IPAddressSpace::kPrivate; + if (SecurityOrigin::Create(request_url)->IsLocalhost()) + target_space = network::mojom::IPAddressSpace::kLocal; + + bool is_external_request = requestor_space > target_space; + if (is_external_request) + return true; + + return false; +} + void FrameFetchContext::DispatchDidBlockRequest( const ResourceRequest& resource_request, const ResourceLoaderOptions& options, diff --git a/third_party/blink/renderer/core/loader/frame_fetch_context.h b/third_party/blink/renderer/core/loader/frame_fetch_context.h --- a/third_party/blink/renderer/core/loader/frame_fetch_context.h +++ b/third_party/blink/renderer/core/loader/frame_fetch_context.h @@ -175,6 +175,7 @@ class CORE_EXPORT FrameFetchContext final : public BaseFetchContext, bool ShouldBlockWebSocketByMixedContentCheck(const KURL&) const override; std::unique_ptr CreateWebSocketHandshakeThrottle() override; + bool ShouldBlockGateWayAttacks(network::mojom::IPAddressSpace requestor_space, const KURL&) const override; bool ShouldBlockFetchByMixedContentCheck( mojom::blink::RequestContextType request_context, network::mojom::blink::IPAddressSpace target_address_space, diff --git a/third_party/blink/renderer/core/loader/worker_fetch_context.cc b/third_party/blink/renderer/core/loader/worker_fetch_context.cc --- a/third_party/blink/renderer/core/loader/worker_fetch_context.cc +++ b/third_party/blink/renderer/core/loader/worker_fetch_context.cc @@ -25,6 +25,7 @@ #include "third_party/blink/renderer/platform/loader/fetch/url_loader/url_loader_factory.h" #include "third_party/blink/renderer/platform/loader/fetch/worker_resource_timing_notifier.h" #include "third_party/blink/renderer/platform/network/network_state_notifier.h" +#include "third_party/blink/renderer/platform/network/network_utils.h" #include "third_party/blink/renderer/platform/runtime_enabled_features.h" #include "third_party/blink/renderer/platform/scheduler/public/virtual_time_controller.h" #include "third_party/blink/renderer/platform/supplementable.h" @@ -94,6 +95,26 @@ bool WorkerFetchContext::ShouldBlockRequestByInspector(const KURL& url) const { return should_block_request; } +bool WorkerFetchContext::ShouldBlockGateWayAttacks(network::mojom::IPAddressSpace requestor_space, const KURL& request_url) const { + // TODO(mkwst): This only checks explicit IP addresses. We'll have to move + // all this up to //net and //content in order to have any real impact on + // gateway attacks. That turns out to be a TON of work (crbug.com/378566). + if (requestor_space == network::mojom::IPAddressSpace::kUnknown) + requestor_space = network::mojom::IPAddressSpace::kPublic; + network::mojom::IPAddressSpace target_space = + network::mojom::IPAddressSpace::kPublic; + if (network_utils::IsReservedIPAddress(request_url.Host())) + target_space = network::mojom::IPAddressSpace::kPrivate; + if (SecurityOrigin::Create(request_url)->IsLocalhost()) + target_space = network::mojom::IPAddressSpace::kLocal; + + bool is_external_request = requestor_space > target_space; + if (is_external_request) + return true; + + return false; +} + void WorkerFetchContext::DispatchDidBlockRequest( const ResourceRequest& resource_request, const ResourceLoaderOptions& options, diff --git a/third_party/blink/renderer/core/loader/worker_fetch_context.h b/third_party/blink/renderer/core/loader/worker_fetch_context.h --- a/third_party/blink/renderer/core/loader/worker_fetch_context.h +++ b/third_party/blink/renderer/core/loader/worker_fetch_context.h @@ -64,6 +64,7 @@ class WorkerFetchContext final : public BaseFetchContext { bool ShouldBlockWebSocketByMixedContentCheck(const KURL&) const override; std::unique_ptr CreateWebSocketHandshakeThrottle() override; + bool ShouldBlockGateWayAttacks(network::mojom::IPAddressSpace requestor_space, const KURL&) const override; bool ShouldBlockFetchByMixedContentCheck( mojom::blink::RequestContextType request_context, network::mojom::blink::IPAddressSpace target_address_space, diff --git a/third_party/blink/renderer/core/workers/installed_scripts_manager.cc b/third_party/blink/renderer/core/workers/installed_scripts_manager.cc --- a/third_party/blink/renderer/core/workers/installed_scripts_manager.cc +++ b/third_party/blink/renderer/core/workers/installed_scripts_manager.cc @@ -33,9 +33,9 @@ InstalledScriptsManager::ScriptData::ScriptData( // place so that this is shareable out of worker code. response_address_space_ = network::mojom::IPAddressSpace::kPublic; if (network_utils::IsReservedIPAddress(script_url_.Host())) - response_address_space_ = network::mojom::IPAddressSpace::kLocal; + response_address_space_ = network::mojom::IPAddressSpace::kPrivate; if (SecurityOrigin::Create(script_url_)->IsLocalhost()) - response_address_space_ = network::mojom::IPAddressSpace::kLoopback; + response_address_space_ = network::mojom::IPAddressSpace::kLocal; } ContentSecurityPolicyResponseHeaders diff --git a/third_party/blink/renderer/modules/background_fetch/background_fetch_manager.cc b/third_party/blink/renderer/modules/background_fetch/background_fetch_manager.cc --- a/third_party/blink/renderer/modules/background_fetch/background_fetch_manager.cc +++ b/third_party/blink/renderer/modules/background_fetch/background_fetch_manager.cc @@ -101,6 +101,30 @@ bool ShouldBlockDanglingMarkup(const KURL& request_url) { request_url.ProtocolIsInHTTPFamily(); } +bool ShouldBlockGateWayAttacks(ExecutionContext* execution_context, + const KURL& request_url) { + network::mojom::IPAddressSpace requestor_space = + execution_context->AddressSpace(); + if (requestor_space == network::mojom::IPAddressSpace::kUnknown) + requestor_space = network::mojom::IPAddressSpace::kPublic; + + // TODO(mkwst): This only checks explicit IP addresses. We'll have to move + // all this up to //net and //content in order to have any real impact on + // gateway attacks. That turns out to be a TON of work (crbug.com/378566). + network::mojom::IPAddressSpace target_space = + network::mojom::IPAddressSpace::kPublic; + if (network_utils::IsReservedIPAddress(request_url.Host())) + target_space = network::mojom::IPAddressSpace::kPrivate; + if (SecurityOrigin::Create(request_url)->IsLocalhost()) + target_space = network::mojom::IPAddressSpace::kLocal; + + bool is_external_request = requestor_space > target_space; + if (is_external_request) + return true; + + return false; +} + scoped_refptr ExtractBlobHandle( Request* request, ExceptionState& exception_state) { @@ -187,6 +211,13 @@ ScriptPromise BackgroundFetchManager::fetch( exception_state); } + if (ShouldBlockGateWayAttacks(execution_context, request_url)) { + return RejectWithTypeError(script_state, request_url, + "Requestor IP address space doesn't match the " + "target address space.", + exception_state); + } + if (ShouldBlockPort(request_url)) { return RejectWithTypeError(script_state, request_url, "that port is not allowed", exception_state); diff --git a/third_party/blink/renderer/modules/websockets/websocket_channel_impl.cc b/third_party/blink/renderer/modules/websockets/websocket_channel_impl.cc --- a/third_party/blink/renderer/modules/websockets/websocket_channel_impl.cc +++ b/third_party/blink/renderer/modules/websockets/websocket_channel_impl.cc @@ -276,6 +276,11 @@ bool WebSocketChannelImpl::Connect(const KURL& url, const String& protocol) { return false; } + if (GetBaseFetchContext()->ShouldBlockGateWayAttacks(execution_context_->AddressSpace(), url)) { + has_initiated_opening_handshake_ = false; + return false; + } + if (auto* scheduler = execution_context_->GetScheduler()) { // Two features are registered here: // - `kWebSocket`: a non-sticky feature that will disable BFCache for any diff --git a/third_party/blink/renderer/modules/websockets/websocket_common.cc b/third_party/blink/renderer/modules/websockets/websocket_common.cc --- a/third_party/blink/renderer/modules/websockets/websocket_common.cc +++ b/third_party/blink/renderer/modules/websockets/websocket_common.cc @@ -125,9 +125,39 @@ WebSocketCommon::ConnectResult WebSocketCommon::Connect( return ConnectResult::kException; } + network::mojom::IPAddressSpace requestor_space = + execution_context->AddressSpace(); + if (ShouldBlockGateWayAttacks(requestor_space, url_)) { + state_ = kClosed; + exception_state.ThrowSecurityError( + "Access to address of '" + url_.Host() + + "' is not allowed from current address space."); + return ConnectResult::kException; + } + return ConnectResult::kSuccess; } +bool WebSocketCommon::ShouldBlockGateWayAttacks(network::mojom::IPAddressSpace requestor_space, const KURL& request_url) const { + // TODO(mkwst): This only checks explicit IP addresses. We'll have to move + // all this up to //net and //content in order to have any real impact on + // gateway attacks. That turns out to be a TON of work (crbug.com/378566). + if (requestor_space == network::mojom::IPAddressSpace::kUnknown) + requestor_space = network::mojom::IPAddressSpace::kPublic; + network::mojom::IPAddressSpace target_space = + network::mojom::IPAddressSpace::kPublic; + if (network_utils::IsReservedIPAddress(request_url.Host())) + target_space = network::mojom::IPAddressSpace::kPrivate; + if (SecurityOrigin::Create(request_url)->IsLocalhost()) + target_space = network::mojom::IPAddressSpace::kLocal; + + bool is_external_request = requestor_space > target_space; + if (is_external_request) + return true; + + return false; +} + void WebSocketCommon::CloseInternal(int code, const String& reason, WebSocketChannel* channel, diff --git a/third_party/blink/renderer/modules/websockets/websocket_common.h b/third_party/blink/renderer/modules/websockets/websocket_common.h --- a/third_party/blink/renderer/modules/websockets/websocket_common.h +++ b/third_party/blink/renderer/modules/websockets/websocket_common.h @@ -7,6 +7,8 @@ #ifndef THIRD_PARTY_BLINK_RENDERER_MODULES_WEBSOCKETS_WEBSOCKET_COMMON_H_ #define THIRD_PARTY_BLINK_RENDERER_MODULES_WEBSOCKETS_WEBSOCKET_COMMON_H_ +#include "services/network/public/mojom/ip_address_space.mojom.h" +#include "third_party/blink/renderer/platform/network/network_utils.h" #include "third_party/blink/renderer/modules/modules_export.h" #include "third_party/blink/renderer/platform/weborigin/kurl.h" #include "third_party/blink/renderer/platform/wtf/allocator/allocator.h" @@ -54,6 +56,8 @@ class MODULES_EXPORT WebSocketCommon { void SetState(State state) { state_ = state; } const KURL& Url() const { return url_; } + bool ShouldBlockGateWayAttacks(network::mojom::IPAddressSpace requestor_space, const KURL& url) const; + // The following methods are public for testing. // Returns true if |protocol| is a valid WebSocket subprotocol name. -- 2.25.1