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

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