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

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208209210211212213214215216217218219220221222223224225226227228229230231232233234235236237238239240241242243244245246247248249250251252253254255256257258259260261262263264265266267268269270271272273274275276277278279280281282283284
  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. This approach is not comprehensive, see also:
  5. * https://bugs.chromium.org/p/chromium/issues/detail?id=590714
  6. ---
  7. .../execution_context/execution_context.cc | 16 ++++++++++
  8. .../execution_context/execution_context.h | 1 +
  9. .../renderer/core/loader/base_fetch_context.h | 1 +
  10. .../core/loader/frame_fetch_context.cc | 20 ++++++++++++
  11. .../core/loader/frame_fetch_context.h | 1 +
  12. .../core/loader/worker_fetch_context.cc | 21 +++++++++++++
  13. .../core/loader/worker_fetch_context.h | 1 +
  14. .../background_fetch_manager.cc | 31 +++++++++++++++++++
  15. .../websockets/websocket_channel_impl.cc | 5 +++
  16. .../modules/websockets/websocket_common.cc | 29 +++++++++++++++++
  17. .../modules/websockets/websocket_common.h | 4 +++
  18. 11 files changed, 130 insertions(+)
  19. diff --git a/third_party/blink/renderer/core/execution_context/execution_context.cc b/third_party/blink/renderer/core/execution_context/execution_context.cc
  20. --- a/third_party/blink/renderer/core/execution_context/execution_context.cc
  21. +++ b/third_party/blink/renderer/core/execution_context/execution_context.cc
  22. @@ -672,4 +672,20 @@ bool ExecutionContext::RequireTrustedTypes() const {
  23. RuntimeEnabledFeatures::TrustedDOMTypesEnabled(this);
  24. }
  25. +String ExecutionContext::addressSpaceForBindings() const {
  26. + switch (AddressSpace()) {
  27. + case network::mojom::IPAddressSpace::kPublic:
  28. + case network::mojom::IPAddressSpace::kUnknown:
  29. + return "public";
  30. +
  31. + case network::mojom::IPAddressSpace::kPrivate:
  32. + return "private";
  33. +
  34. + case network::mojom::IPAddressSpace::kLocal:
  35. + return "local";
  36. + }
  37. + NOTREACHED();
  38. + return "public";
  39. +}
  40. +
  41. } // namespace blink
  42. diff --git a/third_party/blink/renderer/core/execution_context/execution_context.h b/third_party/blink/renderer/core/execution_context/execution_context.h
  43. --- a/third_party/blink/renderer/core/execution_context/execution_context.h
  44. +++ b/third_party/blink/renderer/core/execution_context/execution_context.h
  45. @@ -374,6 +374,7 @@ class CORE_EXPORT ExecutionContext : public Supplementable<ExecutionContext>,
  46. const String& message = g_empty_string,
  47. const String& source_file = g_empty_string) const {}
  48. + String addressSpaceForBindings() const;
  49. network::mojom::IPAddressSpace AddressSpace() const;
  50. void SetAddressSpace(network::mojom::blink::IPAddressSpace ip_address_space);
  51. diff --git a/third_party/blink/renderer/core/loader/base_fetch_context.h b/third_party/blink/renderer/core/loader/base_fetch_context.h
  52. --- a/third_party/blink/renderer/core/loader/base_fetch_context.h
  53. +++ b/third_party/blink/renderer/core/loader/base_fetch_context.h
  54. @@ -83,6 +83,7 @@ class CORE_EXPORT BaseFetchContext : public FetchContext {
  55. virtual SubresourceFilter* GetSubresourceFilter() const = 0;
  56. virtual bool ShouldBlockWebSocketByMixedContentCheck(const KURL&) const = 0;
  57. + virtual bool ShouldBlockGateWayAttacks(network::mojom::IPAddressSpace requestor_space, const KURL&) const = 0;
  58. virtual std::unique_ptr<WebSocketHandshakeThrottle>
  59. CreateWebSocketHandshakeThrottle() = 0;
  60. diff --git a/third_party/blink/renderer/core/loader/frame_fetch_context.cc b/third_party/blink/renderer/core/loader/frame_fetch_context.cc
  61. --- a/third_party/blink/renderer/core/loader/frame_fetch_context.cc
  62. +++ b/third_party/blink/renderer/core/loader/frame_fetch_context.cc
  63. @@ -546,6 +546,26 @@ bool FrameFetchContext::ShouldBlockRequestByInspector(const KURL& url) const {
  64. return should_block_request;
  65. }
  66. +bool FrameFetchContext::ShouldBlockGateWayAttacks(network::mojom::IPAddressSpace requestor_space, const KURL& request_url) const {
  67. + // TODO(mkwst): This only checks explicit IP addresses. We'll have to move
  68. + // all this up to //net and //content in order to have any real impact on
  69. + // gateway attacks. That turns out to be a TON of work (crbug.com/378566).
  70. + if (requestor_space == network::mojom::IPAddressSpace::kUnknown)
  71. + requestor_space = network::mojom::IPAddressSpace::kPublic;
  72. + network::mojom::IPAddressSpace target_space =
  73. + network::mojom::IPAddressSpace::kPublic;
  74. + if (network_utils::IsReservedIPAddress(request_url.Host()))
  75. + target_space = network::mojom::IPAddressSpace::kPrivate;
  76. + if (SecurityOrigin::Create(request_url)->IsLocalhost())
  77. + target_space = network::mojom::IPAddressSpace::kLocal;
  78. +
  79. + bool is_external_request = requestor_space > target_space;
  80. + if (is_external_request)
  81. + return true;
  82. +
  83. + return false;
  84. +}
  85. +
  86. void FrameFetchContext::DispatchDidBlockRequest(
  87. const ResourceRequest& resource_request,
  88. const ResourceLoaderOptions& options,
  89. diff --git a/third_party/blink/renderer/core/loader/frame_fetch_context.h b/third_party/blink/renderer/core/loader/frame_fetch_context.h
  90. --- a/third_party/blink/renderer/core/loader/frame_fetch_context.h
  91. +++ b/third_party/blink/renderer/core/loader/frame_fetch_context.h
  92. @@ -171,6 +171,7 @@ class CORE_EXPORT FrameFetchContext final : public BaseFetchContext,
  93. bool ShouldBlockWebSocketByMixedContentCheck(const KURL&) const override;
  94. std::unique_ptr<WebSocketHandshakeThrottle> CreateWebSocketHandshakeThrottle()
  95. override;
  96. + bool ShouldBlockGateWayAttacks(network::mojom::IPAddressSpace requestor_space, const KURL&) const override;
  97. bool ShouldBlockFetchByMixedContentCheck(
  98. mojom::blink::RequestContextType request_context,
  99. const absl::optional<ResourceRequest::RedirectInfo>& redirect_info,
  100. diff --git a/third_party/blink/renderer/core/loader/worker_fetch_context.cc b/third_party/blink/renderer/core/loader/worker_fetch_context.cc
  101. --- a/third_party/blink/renderer/core/loader/worker_fetch_context.cc
  102. +++ b/third_party/blink/renderer/core/loader/worker_fetch_context.cc
  103. @@ -24,6 +24,7 @@
  104. #include "third_party/blink/renderer/platform/loader/fetch/resource_timing_info.h"
  105. #include "third_party/blink/renderer/platform/loader/fetch/worker_resource_timing_notifier.h"
  106. #include "third_party/blink/renderer/platform/network/network_state_notifier.h"
  107. +#include "third_party/blink/renderer/platform/network/network_utils.h"
  108. #include "third_party/blink/renderer/platform/runtime_enabled_features.h"
  109. #include "third_party/blink/renderer/platform/supplementable.h"
  110. #include "third_party/blink/renderer/platform/weborigin/security_policy.h"
  111. @@ -90,6 +91,26 @@ bool WorkerFetchContext::ShouldBlockRequestByInspector(const KURL& url) const {
  112. return should_block_request;
  113. }
  114. +bool WorkerFetchContext::ShouldBlockGateWayAttacks(network::mojom::IPAddressSpace requestor_space, const KURL& request_url) const {
  115. + // TODO(mkwst): This only checks explicit IP addresses. We'll have to move
  116. + // all this up to //net and //content in order to have any real impact on
  117. + // gateway attacks. That turns out to be a TON of work (crbug.com/378566).
  118. + if (requestor_space == network::mojom::IPAddressSpace::kUnknown)
  119. + requestor_space = network::mojom::IPAddressSpace::kPublic;
  120. + network::mojom::IPAddressSpace target_space =
  121. + network::mojom::IPAddressSpace::kPublic;
  122. + if (network_utils::IsReservedIPAddress(request_url.Host()))
  123. + target_space = network::mojom::IPAddressSpace::kPrivate;
  124. + if (SecurityOrigin::Create(request_url)->IsLocalhost())
  125. + target_space = network::mojom::IPAddressSpace::kLocal;
  126. +
  127. + bool is_external_request = requestor_space > target_space;
  128. + if (is_external_request)
  129. + return true;
  130. +
  131. + return false;
  132. +}
  133. +
  134. void WorkerFetchContext::DispatchDidBlockRequest(
  135. const ResourceRequest& resource_request,
  136. const ResourceLoaderOptions& options,
  137. diff --git a/third_party/blink/renderer/core/loader/worker_fetch_context.h b/third_party/blink/renderer/core/loader/worker_fetch_context.h
  138. --- a/third_party/blink/renderer/core/loader/worker_fetch_context.h
  139. +++ b/third_party/blink/renderer/core/loader/worker_fetch_context.h
  140. @@ -62,6 +62,7 @@ class WorkerFetchContext final : public BaseFetchContext {
  141. bool ShouldBlockWebSocketByMixedContentCheck(const KURL&) const override;
  142. std::unique_ptr<WebSocketHandshakeThrottle> CreateWebSocketHandshakeThrottle()
  143. override;
  144. + bool ShouldBlockGateWayAttacks(network::mojom::IPAddressSpace requestor_space, const KURL&) const override;
  145. bool ShouldBlockFetchByMixedContentCheck(
  146. mojom::blink::RequestContextType request_context,
  147. const absl::optional<ResourceRequest::RedirectInfo>& redirect_info,
  148. 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
  149. --- a/third_party/blink/renderer/modules/background_fetch/background_fetch_manager.cc
  150. +++ b/third_party/blink/renderer/modules/background_fetch/background_fetch_manager.cc
  151. @@ -101,6 +101,30 @@ bool ShouldBlockDanglingMarkup(const KURL& request_url) {
  152. request_url.ProtocolIsInHTTPFamily();
  153. }
  154. +bool ShouldBlockGateWayAttacks(ExecutionContext* execution_context,
  155. + const KURL& request_url) {
  156. + network::mojom::IPAddressSpace requestor_space =
  157. + execution_context->AddressSpace();
  158. + if (requestor_space == network::mojom::IPAddressSpace::kUnknown)
  159. + requestor_space = network::mojom::IPAddressSpace::kPublic;
  160. +
  161. + // TODO(mkwst): This only checks explicit IP addresses. We'll have to move
  162. + // all this up to //net and //content in order to have any real impact on
  163. + // gateway attacks. That turns out to be a TON of work (crbug.com/378566).
  164. + network::mojom::IPAddressSpace target_space =
  165. + network::mojom::IPAddressSpace::kPublic;
  166. + if (network_utils::IsReservedIPAddress(request_url.Host()))
  167. + target_space = network::mojom::IPAddressSpace::kPrivate;
  168. + if (SecurityOrigin::Create(request_url)->IsLocalhost())
  169. + target_space = network::mojom::IPAddressSpace::kLocal;
  170. +
  171. + bool is_external_request = requestor_space > target_space;
  172. + if (is_external_request)
  173. + return true;
  174. +
  175. + return false;
  176. +}
  177. +
  178. scoped_refptr<BlobDataHandle> ExtractBlobHandle(
  179. Request* request,
  180. ExceptionState& exception_state) {
  181. @@ -222,6 +246,13 @@ ScriptPromise BackgroundFetchManager::fetch(
  182. exception_state);
  183. }
  184. + if (ShouldBlockGateWayAttacks(execution_context, request_url)) {
  185. + return RejectWithTypeError(script_state, request_url,
  186. + "Requestor IP address space doesn't match the "
  187. + "target address space.",
  188. + exception_state);
  189. + }
  190. +
  191. kurls.insert(request_url);
  192. }
  193. diff --git a/third_party/blink/renderer/modules/websockets/websocket_channel_impl.cc b/third_party/blink/renderer/modules/websockets/websocket_channel_impl.cc
  194. --- a/third_party/blink/renderer/modules/websockets/websocket_channel_impl.cc
  195. +++ b/third_party/blink/renderer/modules/websockets/websocket_channel_impl.cc
  196. @@ -274,6 +274,11 @@ bool WebSocketChannelImpl::Connect(const KURL& url, const String& protocol) {
  197. return false;
  198. }
  199. + if (GetBaseFetchContext()->ShouldBlockGateWayAttacks(execution_context_->AddressSpace(), url)) {
  200. + has_initiated_opening_handshake_ = false;
  201. + return false;
  202. + }
  203. +
  204. if (auto* scheduler = execution_context_->GetScheduler()) {
  205. feature_handle_for_scheduler_ = scheduler->RegisterFeature(
  206. SchedulingPolicy::Feature::kWebSocket,
  207. diff --git a/third_party/blink/renderer/modules/websockets/websocket_common.cc b/third_party/blink/renderer/modules/websockets/websocket_common.cc
  208. --- a/third_party/blink/renderer/modules/websockets/websocket_common.cc
  209. +++ b/third_party/blink/renderer/modules/websockets/websocket_common.cc
  210. @@ -124,9 +124,38 @@ WebSocketCommon::ConnectResult WebSocketCommon::Connect(
  211. return ConnectResult::kException;
  212. }
  213. + network::mojom::IPAddressSpace requestor_space =
  214. + execution_context->AddressSpace();
  215. + if (ShouldBlockGateWayAttacks(requestor_space, url_)) {
  216. + state_ = kClosed;
  217. + exception_state.ThrowSecurityError(
  218. + "Access to address of '" + url_.Host() + "' is not allowed from '" + execution_context->addressSpaceForBindings() + "' address space.");
  219. + return ConnectResult::kException;
  220. + }
  221. +
  222. return ConnectResult::kSuccess;
  223. }
  224. +bool WebSocketCommon::ShouldBlockGateWayAttacks(network::mojom::IPAddressSpace requestor_space, const KURL& request_url) const {
  225. + // TODO(mkwst): This only checks explicit IP addresses. We'll have to move
  226. + // all this up to //net and //content in order to have any real impact on
  227. + // gateway attacks. That turns out to be a TON of work (crbug.com/378566).
  228. + if (requestor_space == network::mojom::IPAddressSpace::kUnknown)
  229. + requestor_space = network::mojom::IPAddressSpace::kPublic;
  230. + network::mojom::IPAddressSpace target_space =
  231. + network::mojom::IPAddressSpace::kPublic;
  232. + if (network_utils::IsReservedIPAddress(request_url.Host()))
  233. + target_space = network::mojom::IPAddressSpace::kPrivate;
  234. + if (SecurityOrigin::Create(request_url)->IsLocalhost())
  235. + target_space = network::mojom::IPAddressSpace::kLocal;
  236. +
  237. + bool is_external_request = requestor_space > target_space;
  238. + if (is_external_request)
  239. + return true;
  240. +
  241. + return false;
  242. +}
  243. +
  244. void WebSocketCommon::CloseInternal(int code,
  245. const String& reason,
  246. WebSocketChannel* channel,
  247. diff --git a/third_party/blink/renderer/modules/websockets/websocket_common.h b/third_party/blink/renderer/modules/websockets/websocket_common.h
  248. --- a/third_party/blink/renderer/modules/websockets/websocket_common.h
  249. +++ b/third_party/blink/renderer/modules/websockets/websocket_common.h
  250. @@ -7,6 +7,8 @@
  251. #ifndef THIRD_PARTY_BLINK_RENDERER_MODULES_WEBSOCKETS_WEBSOCKET_COMMON_H_
  252. #define THIRD_PARTY_BLINK_RENDERER_MODULES_WEBSOCKETS_WEBSOCKET_COMMON_H_
  253. +#include "services/network/public/mojom/ip_address_space.mojom.h"
  254. +#include "third_party/blink/renderer/platform/network/network_utils.h"
  255. #include "third_party/blink/renderer/modules/modules_export.h"
  256. #include "third_party/blink/renderer/platform/weborigin/kurl.h"
  257. #include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
  258. @@ -54,6 +56,8 @@ class MODULES_EXPORT WebSocketCommon {
  259. void SetState(State state) { state_ = state; }
  260. const KURL& Url() const { return url_; }
  261. + bool ShouldBlockGateWayAttacks(network::mojom::IPAddressSpace requestor_space, const KURL& url) const;
  262. +
  263. // The following methods are public for testing.
  264. // Returns true if |protocol| is a valid WebSocket subprotocol name.
  265. --
  266. 2.25.1