Permalink
Browse files

Permit same-origin connections by default

WebSocket always defers the decision to the server, because it didn't
have to deal with legacy compatibility... but the same-origin policy is
still a reasonable default.

Origin checks do not protect against a directly connecting attacker --
they can lie about their host, but can also lie about their origin.
Origin checks protect against a connection from 3rd-party controlled
script in a context where a victim browser's cookies will be passed
along. And if an attacker has breached that protection, they've already
compromised the HTTP session, so treating the WebSocket connection in
the same way seems reasonable.

In case this logic proves incorrect (or anyone just wants to be more
paranoid), we retain a config option to disable it.
  • Loading branch information...
1 parent f8c53ef commit dae404473409fcab0e07976aec626df670e52282 @matthewd matthewd committed Oct 11, 2016
@@ -1,3 +1,10 @@
+* Permit same-origin connections by default.
+
+ New option `config.action_cable.allow_same_origin_as_host = false`
+ to disable.
+
+ *Dávid Halász*, *Matthew Draper*
+
* Prevent race where the client could receive and act upon a
subscription confirmation before the channel's `subscribed` method
completed.
View
@@ -326,23 +326,28 @@ Rails.application.paths.add "config/cable", with: "somewhere/else/cable.yml"
### Allowed Request Origins
-Action Cable will only accept requests from specified origins, which are passed to the server config as an array. The origins can be instances of strings or regular expressions, against which a check for match will be performed.
+Action Cable will only accept requests from specific origins.
+
+By default, only an origin matching the cable server itself will be permitted.
+Additional origins can be specified using strings or regular expressions, provided in an array.
```ruby
Rails.application.config.action_cable.allowed_request_origins = ['http://rubyonrails.com', /http:\/\/ruby.*/]
```
When running in the development environment, this defaults to "http://localhost:3000".
-To disable and allow requests from any origin:
+To disable protection and allow requests from any origin:
```ruby
Rails.application.config.action_cable.disable_request_forgery_protection = true
```
-It is also possible to allow origins that are starting with the actual HTTP HOST header:
+To disable automatic access for same-origin requests, and strictly allow
+only the configured origins:
+
```ruby
-Rails.application.config.action_cable.allow_same_origin_as_host = true
+Rails.application.config.action_cable.allow_same_origin_as_host = false
```
### Consumer Configuration
@@ -196,9 +196,9 @@ def allow_request_origin?
return true if server.config.disable_request_forgery_protection
proto = Rack::Request.new(env).ssl? ? "https" : "http"
- if Array(server.config.allowed_request_origins).any? { |allowed_origin| allowed_origin === env["HTTP_ORIGIN"] }
+ if server.config.allow_same_origin_as_host && env["HTTP_ORIGIN"] == "#{proto}://#{env['HTTP_HOST']}"
true
- elsif server.config.allow_same_origin_as_host && env["HTTP_ORIGIN"] == "#{proto}://#{env['HTTP_HOST']}"
+ elsif Array(server.config.allowed_request_origins).any? { |allowed_origin| allowed_origin === env["HTTP_ORIGIN"] }
true
else
logger.error("Request origin not allowed: #{env['HTTP_ORIGIN']}")
@@ -15,7 +15,7 @@ def initialize
@worker_pool_size = 4
@disable_request_forgery_protection = false
- @allow_same_origin_as_host = false
+ @allow_same_origin_as_host = true
end
# Returns constant of subscription adapter specified in config/cable.yml.
@@ -13,12 +13,13 @@ def send_async(method, *args)
setup do
@server = TestServer.new
@server.config.allowed_request_origins = %w( http://rubyonrails.com )
+ @server.config.allow_same_origin_as_host = false
end
teardown do
@server.config.disable_request_forgery_protection = false
@server.config.allowed_request_origins = []
- @server.config.allow_same_origin_as_host = false
+ @server.config.allow_same_origin_as_host = true
end
test "disable forgery protection" do

1 comment on commit dae4044

@matthewd
Member

This is a follow-on from #26568.

Backported to 5-0-stable: db70978 + 549d732

Please sign in to comment.