From: csagan5 <32685696+csagan5@users.noreply.github.com> Date: Tue, 28 Jul 2020 12:28:58 +0200 Subject: Block gateway attacks via websockets --- .../renderer/core/loader/base_fetch_context.h | 1 + .../core/loader/frame_fetch_context.cc | 18 +++++++++++++ .../core/loader/frame_fetch_context.h | 1 + .../core/loader/worker_fetch_context.cc | 19 +++++++++++++ .../core/loader/worker_fetch_context.h | 1 + .../background_fetch_manager.cc | 2 -- .../websockets/websocket_channel_impl.cc | 5 ++++ .../modules/websockets/websocket_common.cc | 27 +++++++++++++++++++ .../modules/websockets/websocket_common.h | 4 +++ 9 files changed, 76 insertions(+), 2 deletions(-) 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 @@ -83,6 +83,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 @@ -566,6 +566,24 @@ 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). + 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 @@ -166,6 +166,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, const absl::optional& redirect_info, 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 @@ -24,6 +24,7 @@ #include "third_party/blink/renderer/platform/loader/fetch/resource_timing_info.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/supplementable.h" #include "third_party/blink/renderer/platform/weborigin/security_policy.h" @@ -90,6 +91,24 @@ 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). + 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 @@ -62,6 +62,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, const absl::optional& redirect_info, 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 @@ -103,7 +103,6 @@ bool ShouldBlockDanglingMarkup(const KURL& request_url) { bool ShouldBlockGateWayAttacks(ExecutionContext* execution_context, const KURL& request_url) { - if (RuntimeEnabledFeatures::CorsRFC1918Enabled()) { network::mojom::IPAddressSpace requestor_space = execution_context->AddressSpace(); @@ -120,7 +119,6 @@ bool ShouldBlockGateWayAttacks(ExecutionContext* execution_context, bool is_external_request = requestor_space > target_space; if (is_external_request) return true; - } return false; } 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 @@ -274,6 +274,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()) { feature_handle_for_scheduler_ = scheduler->RegisterFeature( SchedulingPolicy::Feature::kWebSocket, 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 @@ -124,9 +124,36 @@ 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 '" + execution_context->addressSpaceForBindings() + "' 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). + 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 @@ -8,6 +8,8 @@ #define THIRD_PARTY_BLINK_RENDERER_MODULES_WEBSOCKETS_WEBSOCKET_COMMON_H_ #include "base/macros.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" @@ -55,6 +57,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.