Drop dependency on secure_headers, fix response headers

[ClearlyClaire]

Alternate take at fixing #15560 without adding a middleware or additional dependency.

This should set it so “secure” cookies are sent if an “.onion” domain is configured as either LOCAL_DOMAIN, WEB_DOMAIN or any of the ALTERNATE_DOMAINS, without modifying the cookie itself.

[ClearlyClaire]

One minor downside to this approach is that the “secure” cookie might be sent over HTTP even without going through the Tor Hidden Service, if such a service is configured in one of the alternate domains. Such a misconfiguration is highly unlikely though, and it's slightly better than sending the same cookie with the “secure” flag stripped, anyway.

cc @cohosh @dr-bonez @jtracey I don't have a Tor Hidden Service set up to try that out, would be cool if any of you could confirm it works as intended

[cohosh]

I have a onion service test deployment I can try this out on but it won't be until later tonight (about 3 hours from now).

[ClearlyClaire]

Converted back to a draft as the initializer is called too late for the always_write_cookie value to matter.

[dr-bonez]

Instead of using LOCAL_DOMAIN, etc, why not decide based on the Host header?

[ClearlyClaire]

Instead of using LOCAL_DOMAIN, etc, why not decide based on the Host header?

Because that would require a middleware or monkey-patching, the config option rails use is set at application boot.

[ClearlyClaire]

The monkey-patching would be really minimal, but would touch a private method of ActionDispatch::Cookies::CookieJar, something like:

module ActionDispatch
  module CookieJarExtensions
    def write_cookie?(cookie)
      request.headers["Host"].ends_with?('.onion') || super
    end
  end
end

ActionDispatch::Cookies::CookieJar.prepend(ActionDispatch::CookieJarExtensions)

I'm open to both solutions.

[ClearlyClaire]

Pushed the monkey-patch which should work as well. I have no strong preference to either alternative, the monkey-patching might be cleaner functionality-wise, but involves patching a private method.

[dr-bonez]

It looks like this isn't quite working. I deployed this to my hidden service instance, and tried to sign up a user, and I'm getting:

Security verification failed. Are you blocking cookies?

The browser I'm using does recognize it as a secure context.

[cohosh]

@dr-bonez Which protocol are you forwarding in your nginx config? I had to update my config because before it was always forwarding https, even when the request or response was over http. See #15498

[dr-bonez]

My config uses $scheme.

[cohosh]

I finally got around to testing this and it's failing for me too :/ looking into it now...

[ClearlyClaire]

Can you check in the network monitor whether the Set-Cookie header is set by the server? And if the browser does actually send the Cookie back?

[cohosh]

Yeah I did that and I'm not seeing the cookies being sent still. I even set

def write_cookie?(*)
      true || super
     end

in the monkey patch and it didn't work. After more debugging, it looks like the monkey patch is getting called, but that the onion service is throwing the 422 error before it even gets to the patch. I do think the monkey patch was necessary to get this working but it seems like there's something else going on as well.

[cohosh]

Okay I just tracked down more information. There are three different cookies Mastodon uses: _mastodon_session, _session_id, and remember_user_token. The latter two are sent after login, but the first one is sent even before the user signs in.

When I turn off the secure flag for only _mastodon_session, everything works. The _session_id and remember_user_token cookies are sent and are flagged secure. So, perhaps there's something different in the handling of _mastodon_session that is doing an additional check for HTTPS when it's marked secure?

Here's the code for how I got it working, btw (just for debugging purposes, I know we don't want to actually do this):

 Rails.application.config.session_store :cookie_store, {
   key: '_mastodon_session',
-  secure: (Rails.env.production? || ENV['LOCAL_HTTPS'] == 'true'),
+  secure: false,
   same_site: :lax,
 }

[cohosh]

Okay I found it! We need another monkey patch for this function:
https://github.com/rack/rack/blob/5ce5b2ccb151c62652e25b4711218ede11497cc3/lib/rack/session/abstract/id.rb#L363

When I patch that, everything works :) I'll clean this up and submit another PR.