Do not enforce SSL for Tor Hidden Service .onion URLs #7068

Closed
wants to merge 3 commits into
from

4 participants

@saizai

Do not enforce SSL on Tor hidden services (e.g. if this server hosts content both over SSL and Tor).

CAs cannot verify .onion ownership for SSL (so it provides no auth),and SSL introduces leaks that actually degrade Tor security/privacy.

Cf. https://trac.torproject.org/projects/tor/wiki/doc/TorFAQ?version=1390#WhyisitbettertoprovideahiddenserviceWebsitewithHTTPratherthanHTTPSaccess

@rafaelfranca
Ruby on Rails member

Could you add tests to it?

@saizai

@rafaelfranca Kinda hard to do that in any meaningful way (i.e. not just duplicating the code) short of having an actual hidden service, no? The host check is trivial — /.domain$/.

@pixeltrix
Ruby on Rails member

Whilst I think the Tor project is worthy ideal I don't think we should be hardcoding support within Rails. However if the consensus is that it should be merged then shouldn't it redirect from https to http if it's a request via Tor?.

@saizai you need to test that the redirect doesn't take place if it's a Tor request - i.e. you get a 200 status code rather than a 301 or 302.

@saizai

@pixeltrix I intentionally did not make it redirect https->http, and preserved https treatment if it comes in over ssl even if it is a Tor hidden service request. There are some subtleties involved that mean that webmasters should have control over which one is used. This change simply makes it not force http->https; it doesn't say which one it should be (though usually it should be non-SSL).

Do you have a suggestion for where exactly I should add a test patch?

As for hardcoding… it's a two line change which is best made within an existing function; monkeypatching it in a gem would be both excessive and clunky/fragile.

@pixeltrix pixeltrix and 1 other commented on an outdated diff Jul 16, 2012
actionpack/lib/action_dispatch/middleware/ssl.rb
if request.ssl?
status, headers, body = @app.call(env)
headers = hsts_headers.merge(headers)
flag_cookies_as_secure!(headers)
[status, headers, body]
+ elsif URI(request.url).host =~ /\.onion$/
+ # Do not enforce SSL on Tor hidden services (e.g. if this server hosts content both over SSL and Tor).
@pixeltrix
Ruby on Rails member
pixeltrix added a line comment Jul 16, 2012

why are you parsing the url with URI - request.host should work shouldn't it?

@saizai
saizai added a line comment Jul 16, 2012

Just using the same syntax as the similar code below it.

@pixeltrix
Ruby on Rails member
pixeltrix added a line comment Jul 16, 2012

That section of code modifies the url - your addition just needs to check the host so there's no need to use URI()

@saizai
saizai added a line comment Jul 16, 2012

Fair enough. Changing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@pixeltrix
Ruby on Rails member

If there are subtleties involved I would recommend turning off config.force_ssl and using the controller level force_ssl with appropriate filters. If you don't want to handle it at the controller level, then you can turn off config.force_ssl and use your own custom middleware module.

You'd add a test to ssl_test.rb with this:

def test_tor_host_does_not_redirect
  self.app = ActionDispatch::SSL.new(default_app, :host => "example.onion")

  get "http://example.onion/"
  assert_response :success

  get "https://example.onion/"
  assert_response :success
end
@rafaelfranca
Ruby on Rails member

I think we don't need to continue with this patch, since we can use the controller level force_ssl.

@guilleiguaran
Ruby on Rails member

as @pixeltrix and @rafaelfranca pointed we don't want to hardcode support for .onion urls in Rails and this can be achieved at controller level, so I'm closing this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment