Block-gateway-attacks-via-websockets.patch 11 KB

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208
  1. From: csagan5 <32685696+csagan5@users.noreply.github.com>
  2. Date: Tue, 28 Jul 2020 12:28:58 +0200
  3. Subject: Block gateway attacks via websockets
  4. ---
  5. .../renderer/core/loader/base_fetch_context.h | 1 +
  6. .../core/loader/frame_fetch_context.cc | 18 +++++++++++++
  7. .../core/loader/frame_fetch_context.h | 1 +
  8. .../core/loader/worker_fetch_context.cc | 19 +++++++++++++
  9. .../core/loader/worker_fetch_context.h | 1 +
  10. .../background_fetch_manager.cc | 2 --
  11. .../websockets/websocket_channel_impl.cc | 5 ++++
  12. .../modules/websockets/websocket_common.cc | 27 +++++++++++++++++++
  13. .../modules/websockets/websocket_common.h | 4 +++
  14. 9 files changed, 76 insertions(+), 2 deletions(-)
  15. diff --git a/third_party/blink/renderer/core/loader/base_fetch_context.h b/third_party/blink/renderer/core/loader/base_fetch_context.h
  16. --- a/third_party/blink/renderer/core/loader/base_fetch_context.h
  17. +++ b/third_party/blink/renderer/core/loader/base_fetch_context.h
  18. @@ -83,6 +83,7 @@ class CORE_EXPORT BaseFetchContext : public FetchContext {
  19. virtual SubresourceFilter* GetSubresourceFilter() const = 0;
  20. virtual bool ShouldBlockWebSocketByMixedContentCheck(const KURL&) const = 0;
  21. + virtual bool ShouldBlockGateWayAttacks(network::mojom::IPAddressSpace requestor_space, const KURL&) const = 0;
  22. virtual std::unique_ptr<WebSocketHandshakeThrottle>
  23. CreateWebSocketHandshakeThrottle() = 0;
  24. diff --git a/third_party/blink/renderer/core/loader/frame_fetch_context.cc b/third_party/blink/renderer/core/loader/frame_fetch_context.cc
  25. --- a/third_party/blink/renderer/core/loader/frame_fetch_context.cc
  26. +++ b/third_party/blink/renderer/core/loader/frame_fetch_context.cc
  27. @@ -566,6 +566,24 @@ bool FrameFetchContext::ShouldBlockRequestByInspector(const KURL& url) const {
  28. return should_block_request;
  29. }
  30. +bool FrameFetchContext::ShouldBlockGateWayAttacks(network::mojom::IPAddressSpace requestor_space, const KURL& request_url) const {
  31. + // TODO(mkwst): This only checks explicit IP addresses. We'll have to move
  32. + // all this up to //net and //content in order to have any real impact on
  33. + // gateway attacks. That turns out to be a TON of work (crbug.com/378566).
  34. + network::mojom::IPAddressSpace target_space =
  35. + network::mojom::IPAddressSpace::kPublic;
  36. + if (network_utils::IsReservedIPAddress(request_url.Host()))
  37. + target_space = network::mojom::IPAddressSpace::kPrivate;
  38. + if (SecurityOrigin::Create(request_url)->IsLocalhost())
  39. + target_space = network::mojom::IPAddressSpace::kLocal;
  40. +
  41. + bool is_external_request = requestor_space > target_space;
  42. + if (is_external_request)
  43. + return true;
  44. +
  45. + return false;
  46. +}
  47. +
  48. void FrameFetchContext::DispatchDidBlockRequest(
  49. const ResourceRequest& resource_request,
  50. const ResourceLoaderOptions& options,
  51. diff --git a/third_party/blink/renderer/core/loader/frame_fetch_context.h b/third_party/blink/renderer/core/loader/frame_fetch_context.h
  52. --- a/third_party/blink/renderer/core/loader/frame_fetch_context.h
  53. +++ b/third_party/blink/renderer/core/loader/frame_fetch_context.h
  54. @@ -166,6 +166,7 @@ class CORE_EXPORT FrameFetchContext final : public BaseFetchContext,
  55. bool ShouldBlockWebSocketByMixedContentCheck(const KURL&) const override;
  56. std::unique_ptr<WebSocketHandshakeThrottle> CreateWebSocketHandshakeThrottle()
  57. override;
  58. + bool ShouldBlockGateWayAttacks(network::mojom::IPAddressSpace requestor_space, const KURL&) const override;
  59. bool ShouldBlockFetchByMixedContentCheck(
  60. mojom::blink::RequestContextType request_context,
  61. const absl::optional<ResourceRequest::RedirectInfo>& redirect_info,
  62. diff --git a/third_party/blink/renderer/core/loader/worker_fetch_context.cc b/third_party/blink/renderer/core/loader/worker_fetch_context.cc
  63. --- a/third_party/blink/renderer/core/loader/worker_fetch_context.cc
  64. +++ b/third_party/blink/renderer/core/loader/worker_fetch_context.cc
  65. @@ -24,6 +24,7 @@
  66. #include "third_party/blink/renderer/platform/loader/fetch/resource_timing_info.h"
  67. #include "third_party/blink/renderer/platform/loader/fetch/worker_resource_timing_notifier.h"
  68. #include "third_party/blink/renderer/platform/network/network_state_notifier.h"
  69. +#include "third_party/blink/renderer/platform/network/network_utils.h"
  70. #include "third_party/blink/renderer/platform/runtime_enabled_features.h"
  71. #include "third_party/blink/renderer/platform/supplementable.h"
  72. #include "third_party/blink/renderer/platform/weborigin/security_policy.h"
  73. @@ -90,6 +91,24 @@ bool WorkerFetchContext::ShouldBlockRequestByInspector(const KURL& url) const {
  74. return should_block_request;
  75. }
  76. +bool WorkerFetchContext::ShouldBlockGateWayAttacks(network::mojom::IPAddressSpace requestor_space, const KURL& request_url) const {
  77. + // TODO(mkwst): This only checks explicit IP addresses. We'll have to move
  78. + // all this up to //net and //content in order to have any real impact on
  79. + // gateway attacks. That turns out to be a TON of work (crbug.com/378566).
  80. + network::mojom::IPAddressSpace target_space =
  81. + network::mojom::IPAddressSpace::kPublic;
  82. + if (network_utils::IsReservedIPAddress(request_url.Host()))
  83. + target_space = network::mojom::IPAddressSpace::kPrivate;
  84. + if (SecurityOrigin::Create(request_url)->IsLocalhost())
  85. + target_space = network::mojom::IPAddressSpace::kLocal;
  86. +
  87. + bool is_external_request = requestor_space > target_space;
  88. + if (is_external_request)
  89. + return true;
  90. +
  91. + return false;
  92. +}
  93. +
  94. void WorkerFetchContext::DispatchDidBlockRequest(
  95. const ResourceRequest& resource_request,
  96. const ResourceLoaderOptions& options,
  97. diff --git a/third_party/blink/renderer/core/loader/worker_fetch_context.h b/third_party/blink/renderer/core/loader/worker_fetch_context.h
  98. --- a/third_party/blink/renderer/core/loader/worker_fetch_context.h
  99. +++ b/third_party/blink/renderer/core/loader/worker_fetch_context.h
  100. @@ -62,6 +62,7 @@ class WorkerFetchContext final : public BaseFetchContext {
  101. bool ShouldBlockWebSocketByMixedContentCheck(const KURL&) const override;
  102. std::unique_ptr<WebSocketHandshakeThrottle> CreateWebSocketHandshakeThrottle()
  103. override;
  104. + bool ShouldBlockGateWayAttacks(network::mojom::IPAddressSpace requestor_space, const KURL&) const override;
  105. bool ShouldBlockFetchByMixedContentCheck(
  106. mojom::blink::RequestContextType request_context,
  107. const absl::optional<ResourceRequest::RedirectInfo>& redirect_info,
  108. 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
  109. --- a/third_party/blink/renderer/modules/background_fetch/background_fetch_manager.cc
  110. +++ b/third_party/blink/renderer/modules/background_fetch/background_fetch_manager.cc
  111. @@ -103,7 +103,6 @@ bool ShouldBlockDanglingMarkup(const KURL& request_url) {
  112. bool ShouldBlockGateWayAttacks(ExecutionContext* execution_context,
  113. const KURL& request_url) {
  114. - if (RuntimeEnabledFeatures::CorsRFC1918Enabled()) {
  115. network::mojom::IPAddressSpace requestor_space =
  116. execution_context->AddressSpace();
  117. @@ -120,7 +119,6 @@ bool ShouldBlockGateWayAttacks(ExecutionContext* execution_context,
  118. bool is_external_request = requestor_space > target_space;
  119. if (is_external_request)
  120. return true;
  121. - }
  122. return false;
  123. }
  124. diff --git a/third_party/blink/renderer/modules/websockets/websocket_channel_impl.cc b/third_party/blink/renderer/modules/websockets/websocket_channel_impl.cc
  125. --- a/third_party/blink/renderer/modules/websockets/websocket_channel_impl.cc
  126. +++ b/third_party/blink/renderer/modules/websockets/websocket_channel_impl.cc
  127. @@ -274,6 +274,11 @@ bool WebSocketChannelImpl::Connect(const KURL& url, const String& protocol) {
  128. return false;
  129. }
  130. + if (GetBaseFetchContext()->ShouldBlockGateWayAttacks(execution_context_->AddressSpace(), url)) {
  131. + has_initiated_opening_handshake_ = false;
  132. + return false;
  133. + }
  134. +
  135. if (auto* scheduler = execution_context_->GetScheduler()) {
  136. feature_handle_for_scheduler_ = scheduler->RegisterFeature(
  137. SchedulingPolicy::Feature::kWebSocket,
  138. diff --git a/third_party/blink/renderer/modules/websockets/websocket_common.cc b/third_party/blink/renderer/modules/websockets/websocket_common.cc
  139. --- a/third_party/blink/renderer/modules/websockets/websocket_common.cc
  140. +++ b/third_party/blink/renderer/modules/websockets/websocket_common.cc
  141. @@ -124,9 +124,36 @@ WebSocketCommon::ConnectResult WebSocketCommon::Connect(
  142. return ConnectResult::kException;
  143. }
  144. + network::mojom::IPAddressSpace requestor_space =
  145. + execution_context->AddressSpace();
  146. + if (ShouldBlockGateWayAttacks(requestor_space, url_)) {
  147. + state_ = kClosed;
  148. + exception_state.ThrowSecurityError(
  149. + "Access to address of '" + url_.Host() + "' is not allowed from '" + execution_context->addressSpaceForBindings() + "' address space.");
  150. + return ConnectResult::kException;
  151. + }
  152. +
  153. return ConnectResult::kSuccess;
  154. }
  155. +bool WebSocketCommon::ShouldBlockGateWayAttacks(network::mojom::IPAddressSpace requestor_space, const KURL& request_url) const {
  156. + // TODO(mkwst): This only checks explicit IP addresses. We'll have to move
  157. + // all this up to //net and //content in order to have any real impact on
  158. + // gateway attacks. That turns out to be a TON of work (crbug.com/378566).
  159. + network::mojom::IPAddressSpace target_space =
  160. + network::mojom::IPAddressSpace::kPublic;
  161. + if (network_utils::IsReservedIPAddress(request_url.Host()))
  162. + target_space = network::mojom::IPAddressSpace::kPrivate;
  163. + if (SecurityOrigin::Create(request_url)->IsLocalhost())
  164. + target_space = network::mojom::IPAddressSpace::kLocal;
  165. +
  166. + bool is_external_request = requestor_space > target_space;
  167. + if (is_external_request)
  168. + return true;
  169. +
  170. + return false;
  171. +}
  172. +
  173. void WebSocketCommon::CloseInternal(int code,
  174. const String& reason,
  175. WebSocketChannel* channel,
  176. diff --git a/third_party/blink/renderer/modules/websockets/websocket_common.h b/third_party/blink/renderer/modules/websockets/websocket_common.h
  177. --- a/third_party/blink/renderer/modules/websockets/websocket_common.h
  178. +++ b/third_party/blink/renderer/modules/websockets/websocket_common.h
  179. @@ -8,6 +8,8 @@
  180. #define THIRD_PARTY_BLINK_RENDERER_MODULES_WEBSOCKETS_WEBSOCKET_COMMON_H_
  181. #include "base/macros.h"
  182. +#include "services/network/public/mojom/ip_address_space.mojom.h"
  183. +#include "third_party/blink/renderer/platform/network/network_utils.h"
  184. #include "third_party/blink/renderer/modules/modules_export.h"
  185. #include "third_party/blink/renderer/platform/weborigin/kurl.h"
  186. #include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
  187. @@ -55,6 +57,8 @@ class MODULES_EXPORT WebSocketCommon {
  188. void SetState(State state) { state_ = state; }
  189. const KURL& Url() const { return url_; }
  190. + bool ShouldBlockGateWayAttacks(network::mojom::IPAddressSpace requestor_space, const KURL& url) const;
  191. +
  192. // The following methods are public for testing.
  193. // Returns true if |protocol| is a valid WebSocket subprotocol name.