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

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