Merge pull request #21520 from jeremy/friendlier-force-ssl

Make `config.force_ssl` less dangerous to try and easier to disable

actionpack/CHANGELOG.md

+*   Make it easier to opt in to `config.force_ssl` and `config.ssl_options` by
+    making them less dangerous to try and easier to disable.
+
+    SSL redirect:
+      * Move `:host` and `:port` options within `redirect: { … }`. Deprecate.
+      * Introduce `:status` and `:body` to customize the redirect response.
+        The 301 permanent default makes it difficult to test the redirect and
+        back out of it since browsers remember the 301. Test with a 302 or 307
+        instead, then switch to 301 once you're confident that all is well.
+
+    HTTP Strict Transport Security (HSTS):
+      * Shorter max-age. Shorten the default max-age from 1 year to 180 days,
+        the low end for https://www.ssllabs.com/ssltest/ grading and greater
+        than the 18-week minimum to qualify for browser preload lists.
+      * Disabling HSTS. Setting `hsts: false` now sets `hsts { expires: 0 }`
+        instead of omitting the header. Omitting does nothing to disable HSTS
+        since browsers hang on to your previous settings until they expire.
+        Sending `{ hsts: { expires: 0 }}` flushes out old browser settings and
+        actually disables HSTS:
+          http://tools.ietf.org/html/rfc6797#section-6.1.1
+      * HSTS Preload. Introduce `preload: true` to set the `preload` flag,
+        indicating that your site may be included in browser preload lists,
+        including Chrome, Firefox, Safari, IE11, and Edge. Submit your site:
+          https://hstspreload.appspot.com
+
+    *Jeremy Daer*
+
 *   Update `ActionController::TestSession#fetch` to behave more like
     `ActionDispatch::Request::Session#fetch` when using non-string keys.
 

actionpack/lib/action_dispatch/middleware/ssl.rb

 module ActionDispatch
+  # This middleware is added to the stack when `config.force_ssl = true`.
+  # It does three jobs to enforce secure HTTP requests:
+  #
+  #   1. TLS redirect. http:// requests are permanently redirected to https://
+  #      with the same URL host, path, etc. Pass `:host` and/or `:port` to
+  #      modify the destination URL. This is always enabled.
+  #
+  #   2. Secure cookies. Sets the `secure` flag on cookies to tell browsers they
+  #      mustn't be sent along with http:// requests. This is always enabled.
+  #
+  #   3. HTTP Strict Transport Security (HSTS). Tells the browser to remember
+  #      this site as TLS-only and automatically redirect non-TLS requests.
+  #      Enabled by default. Pass `hsts: false` to disable.
+  #
+  # Configure HSTS with `hsts: { … }`:
+  #   * `expires`: How long, in seconds, these settings will stick. Defaults to
+  #     `18.weeks`, the minimum required to qualify for browser preload lists.
+  #   * `subdomains`: Set to `true` to tell the browser to apply these settings
+  #     to all subdomains. This protects your cookies from interception by a
+  #     vulnerable site on a subdomain. Defaults to `false`.
+  #   * `preload`: Advertise that this site may be included in browsers'
+  #     preloaded HSTS lists. HSTS protects your site on every visit *except the
+  #     first visit* since it hasn't seen your HSTS header yet. To close this
+  #     gap, browser vendors include a baked-in list of HSTS-enabled sites.
+  #     Go to https://hstspreload.appspot.com to submit your site for inclusion.
+  #
+  # Disabling HSTS: To turn off HSTS, omitting the header is not enough.
+  # Browsers will remember the original HSTS directive until it expires.
+  # Instead, use the header to tell browsers to expire HSTS immediately.
+  # Setting `hsts: false` is a shortcut for `hsts: { expires: 0 }`.
   class SSL
-    YEAR = 31536000
+    # Default to 180 days, the low end for https://www.ssllabs.com/ssltest/
+    # and greater than the 18-week requirement for browser preload lists.
+    HSTS_EXPIRES_IN = 15552000
 
     def self.default_hsts_options
-      { :expires => YEAR, :subdomains => false }
+      { expires: HSTS_EXPIRES_IN, subdomains: false, preload: false }
     end
 
-    def initialize(app, options = {})
+    def initialize(app, redirect: {}, hsts: {}, **options)
       @app = app
 
-      @hsts = options.fetch(:hsts, {})
-      @hsts = {} if @hsts == true
-      @hsts = self.class.default_hsts_options.merge(@hsts) if @hsts
+      if options[:host] || options[:port]
+        ActiveSupport::Deprecation.warn <<-end_warning.strip_heredoc
+          The `:host` and `:port` options are moving within `:redirect`:
+          `config.ssl_options = { redirect: { host: …, port: … }}`.
+        end_warning
+        @redirect = options.slice(:host, :port)
+      else
+        @redirect = redirect
+      end
 
-      @host    = options[:host]
-      @port    = options[:port]
+      @hsts_header = build_hsts_header(normalize_hsts_options(hsts))
     end
 
     def call(env)
-      request = Request.new(env)
+      request = Request.new env
 
       if request.ssl?
-        status, headers, body = @app.call(env)
-        headers.reverse_merge!(hsts_headers)
-        flag_cookies_as_secure!(headers)
-        [status, headers, body]
+        @app.call(env).tap do |status, headers, body|
+          set_hsts_header! headers
+          flag_cookies_as_secure! headers
+        end
       else
-        redirect_to_https(request)
+        redirect_to_https request
       end
     end
 
     private
-      def redirect_to_https(request)
-        host = @host || request.host
-        port = @port || request.port
-
-        location = "https://#{host}"
-        location << ":#{port}" if port != 80
-        location << request.fullpath
-
-        headers = { 'Content-Type' => 'text/html', 'Location' => location }
-
-        [301, headers, []]
+      def set_hsts_header!(headers)
+        headers['Strict-Transport-Security'.freeze] ||= @hsts_header
       end
 
-      # http://tools.ietf.org/html/draft-hodges-strict-transport-sec-02
-      def hsts_headers
-        if @hsts
-          value = "max-age=#{@hsts[:expires].to_i}"
-          value += "; includeSubDomains" if @hsts[:subdomains]
-          { 'Strict-Transport-Security' => value }
+      def normalize_hsts_options(options)
+        case options
+        # Explicitly disabling HSTS clears the existing setting from browsers
+        # by setting expiry to 0.
+        when false
+          self.class.default_hsts_options.merge(expires: 0)
+        # Default to enabled, with default options.
+        when nil, true
+          self.class.default_hsts_options
         else
-          {}
+          self.class.default_hsts_options.merge(options)
         end
       end
 
+      # http://tools.ietf.org/html/rfc6797#section-6.1
+      def build_hsts_header(hsts)
+        value = "max-age=#{hsts[:expires].to_i}"
+        value << "; includeSubDomains" if hsts[:subdomains]
+        value << "; preload" if hsts[:preload]
+        value
+      end
+
       def flag_cookies_as_secure!(headers)
-        if cookies = headers['Set-Cookie']
-          cookies = cookies.split("\n")
+        if cookies = headers['Set-Cookie'.freeze]
+          cookies = cookies.split("\n".freeze)
 
-          headers['Set-Cookie'] = cookies.map { |cookie|
+          headers['Set-Cookie'.freeze] = cookies.map { |cookie|
             if cookie !~ /;\s*secure\s*(;|$)/i
               "#{cookie}; secure"
             else
               cookie
             end
-          }.join("\n")
+          }.join("\n".freeze)
         end
       end
+
+      def redirect_to_https(request)
+        [ @redirect.fetch(:status, 301),
+          { 'Content-Type' => 'text/html',
+            'Location' => https_location_for(request) },
+          @redirect.fetch(:body, []) ]
+      end
+
+      def https_location_for(request)
+        host = @redirect[:host] || request.host
+        port = @redirect[:port] || request.port
+
+        location = "https://#{host}"
+        location << ":#{port}" if port != 80 && port != 443
+        location << request.fullpath
+        location
+      end
   end
 end

actionpack/test/dispatch/ssl_test.rb

 require 'abstract_unit'
 
 class SSLTest < ActionDispatch::IntegrationTest
-  def default_app
-    lambda { |env|
-      headers = {'Content-Type' => "text/html"}
-      headers['Set-Cookie'] = "id=1; path=/\ntoken=abc; path=/; secure; HttpOnly"
-      [200, headers, ["OK"]]
+  HEADERS = Rack::Utils::HeaderHash.new 'Content-Type' => 'text/html'
+
+  attr_accessor :app
+
+  def build_app(headers: {}, ssl_options: {})
+    headers = HEADERS.merge(headers)
+    ActionDispatch::SSL.new lambda { |env| [200, headers, []] }, ssl_options
+  end
+end
+
+class RedirectSSLTest < SSLTest
+  def assert_not_redirected(url, headers: {})
+    self.app = build_app
+    get url, headers: headers
+    assert_response :ok
+  end
+
+  def assert_redirected(host: nil, port: nil, status: 301, body: [],
+    deprecated_host: nil, deprecated_port: nil,
+    from: 'http://a/b?c=d', to: from.sub('http', 'https'))
+
+    self.app = build_app ssl_options: {
+      redirect: { host: host, port: port, status: status, body: body },
+      host: deprecated_host, port: deprecated_port
     }
+
+    get from
+    assert_response status
+    assert_redirected_to to
+    assert_equal body.join, @response.body
   end
 
-  def app
-    @app ||= ActionDispatch::SSL.new(default_app)
+  test 'https is not redirected' do
+    assert_not_redirected 'https://example.org'
   end
-  attr_writer :app
 
-  def test_allows_https_url
-    get "https://example.org/path?key=value"
-    assert_response :success
+  test 'proxied https is not redirected' do
+    assert_not_redirected 'http://example.org', headers: { 'HTTP_X_FORWARDED_PROTO' => 'https' }
   end
 
-  def test_allows_https_proxy_header_url
-    get "http://example.org/", headers: { 'HTTP_X_FORWARDED_PROTO' => "https" }
-    assert_response :success
+  test 'http is redirected to https' do
+    assert_redirected
   end
 
-  def test_redirects_http_to_https
-    get "http://example.org/path?key=value"
-    assert_response :redirect
-    assert_equal "https://example.org/path?key=value",
-      response.headers['Location']
+  test 'redirect with non-301 status' do
+    assert_redirected status: 307
   end
 
-  def test_hsts_header_by_default
-    get "https://example.org/"
-    assert_equal "max-age=31536000",
-      response.headers['Strict-Transport-Security']
+  test 'redirect with custom body' do
+    assert_redirected body: ['foo']
   end
 
-  def test_no_hsts_with_insecure_connection
-    get "http://example.org/"
-    assert_not response.headers['Strict-Transport-Security']
+  test 'redirect to specific host' do
+    assert_redirected host: 'ssl', to: 'https://ssl/b?c=d'
   end
 
-  def test_hsts_header
-    self.app = ActionDispatch::SSL.new(default_app, :hsts => true)
-    get "https://example.org/"
-    assert_equal "max-age=31536000",
-      response.headers['Strict-Transport-Security']
+  test 'redirect to default port' do
+    assert_redirected port: 443
   end
 
-  def test_disable_hsts_header
-    self.app = ActionDispatch::SSL.new(default_app, :hsts => false)
-    get "https://example.org/"
-    assert_not response.headers['Strict-Transport-Security']
+  test 'redirect to non-default port' do
+    assert_redirected port: 8443, to: 'https://a:8443/b?c=d'
   end
 
-  def test_hsts_expires
-    self.app = ActionDispatch::SSL.new(default_app, :hsts => { :expires => 500 })
-    get "https://example.org/"
-    assert_equal "max-age=500",
-      response.headers['Strict-Transport-Security']
+  test 'redirect to different host and non-default port' do
+    assert_redirected host: 'ssl', port: 8443, to: 'https://ssl:8443/b?c=d'
   end
 
-  def test_hsts_expires_with_duration
-    self.app = ActionDispatch::SSL.new(default_app, :hsts => { :expires => 1.year })
-    get "https://example.org/"
-    assert_equal "max-age=31557600",
-      response.headers['Strict-Transport-Security']
+  test 'redirect to different host including port' do
+    assert_redirected host: 'ssl:443', to: 'https://ssl:443/b?c=d'
   end
 
-  def test_hsts_include_subdomains
-    self.app = ActionDispatch::SSL.new(default_app, :hsts => { :subdomains => true })
-    get "https://example.org/"
-    assert_equal "max-age=31536000; includeSubDomains",
-      response.headers['Strict-Transport-Security']
+  test ':host is deprecated, moved within redirect: { host: … }' do
+    assert_deprecated do
+      assert_redirected deprecated_host: 'foo', to: 'https://foo/b?c=d'
+    end
   end
 
-  def test_flag_cookies_as_secure
-    get "https://example.org/"
-    assert_equal ["id=1; path=/; secure", "token=abc; path=/; secure; HttpOnly" ],
-      response.headers['Set-Cookie'].split("\n")
+  test ':port is deprecated, moved within redirect: { port: … }' do
+    assert_deprecated do
+      assert_redirected deprecated_port: 1, to: 'https://a:1/b?c=d'
+    end
   end
+end
 
-  def test_flag_cookies_as_secure_at_end_of_line
-    self.app = ActionDispatch::SSL.new(lambda { |env|
-      headers = {
-        'Content-Type' => "text/html",
-        'Set-Cookie' => "problem=def; path=/; HttpOnly; secure"
-      }
-      [200, headers, ["OK"]]
-    })
+class StrictTransportSecurityTest < SSLTest
+  EXPECTED = 'max-age=15552000'
 
-    get "https://example.org/"
-    assert_equal ["problem=def; path=/; HttpOnly; secure"],
-      response.headers['Set-Cookie'].split("\n")
+  def assert_hsts(expected, url: 'https://example.org', hsts: {}, headers: {})
+    self.app = build_app ssl_options: { hsts: hsts }, headers: headers
+    get url
+    assert_equal expected, response.headers['Strict-Transport-Security']
   end
 
-  def test_flag_cookies_as_secure_with_more_spaces_before
-    self.app = ActionDispatch::SSL.new(lambda { |env|
-      headers = {
-        'Content-Type' => "text/html",
-        'Set-Cookie' => "problem=def; path=/; HttpOnly;  secure"
-      }
-      [200, headers, ["OK"]]
-    })
+  test 'enabled by default' do
+    assert_hsts EXPECTED
+  end
 
-    get "https://example.org/"
-    assert_equal ["problem=def; path=/; HttpOnly;  secure"],
-      response.headers['Set-Cookie'].split("\n")
+  test 'not sent with http:// responses' do
+    assert_hsts nil, url: 'http://example.org'
   end
 
-  def test_flag_cookies_as_secure_with_more_spaces_after
-    self.app = ActionDispatch::SSL.new(lambda { |env|
-      headers = {
-        'Content-Type' => "text/html",
-        'Set-Cookie' => "problem=def; path=/; secure;  HttpOnly"
-      }
-      [200, headers, ["OK"]]
-    })
+  test 'defers to app-provided header' do
+    assert_hsts 'app-provided', headers: { 'Strict-Transport-Security' => 'app-provided' }
+  end
 
-    get "https://example.org/"
-    assert_equal ["problem=def; path=/; secure;  HttpOnly"],
-      response.headers['Set-Cookie'].split("\n")
+  test 'hsts: true enables default settings' do
+    assert_hsts EXPECTED, hsts: true
   end
 
+  test 'hsts: false sets max-age to zero, clearing browser HSTS settings' do
+    assert_hsts 'max-age=0', hsts: false
+  end
 
-  def test_flag_cookies_as_secure_with_has_not_spaces_before
-    self.app = ActionDispatch::SSL.new(lambda { |env|
-      headers = {
-        'Content-Type' => "text/html",
-        'Set-Cookie' => "problem=def; path=/;secure; HttpOnly"
-      }
-      [200, headers, ["OK"]]
-    })
+  test ':expires sets max-age' do
+    assert_hsts 'max-age=500', hsts: { expires: 500 }
+  end
 
-    get "https://example.org/"
-    assert_equal ["problem=def; path=/;secure; HttpOnly"],
-      response.headers['Set-Cookie'].split("\n")
+  test ':expires supports AS::Duration arguments' do
+    assert_hsts 'max-age=31557600', hsts: { expires: 1.year }
   end
 
-  def test_flag_cookies_as_secure_with_has_not_spaces_after
-    self.app = ActionDispatch::SSL.new(lambda { |env|
-      headers = {
-        'Content-Type' => "text/html",
-        'Set-Cookie' => "problem=def; path=/; secure;HttpOnly"
-      }
-      [200, headers, ["OK"]]
-    })
+  test 'include subdomains' do
+    assert_hsts "#{EXPECTED}; includeSubDomains", hsts: { subdomains: true }
+  end
 
-    get "https://example.org/"
-    assert_equal ["problem=def; path=/; secure;HttpOnly"],
-      response.headers['Set-Cookie'].split("\n")
+  test 'exclude subdomains' do
+    assert_hsts EXPECTED, hsts: { subdomains: false }
   end
 
-  def test_flag_cookies_as_secure_with_ignore_case
-    self.app = ActionDispatch::SSL.new(lambda { |env|
-      headers = {
-        'Content-Type' => "text/html",
-        'Set-Cookie' => "problem=def; path=/; Secure; HttpOnly"
-      }
-      [200, headers, ["OK"]]
-    })
+  test 'opt in to browser preload lists' do
+    assert_hsts "#{EXPECTED}; preload", hsts: { preload: true }
+  end
 
-    get "https://example.org/"
-    assert_equal ["problem=def; path=/; Secure; HttpOnly"],
-      response.headers['Set-Cookie'].split("\n")
+  test 'opt out of browser preload lists' do
+    assert_hsts EXPECTED, hsts: { preload: false }
   end
+end
 
-  def test_no_cookies
-    self.app = ActionDispatch::SSL.new(lambda { |env|
-      [200, {'Content-Type' => "text/html"}, ["OK"]]
-    })
-    get "https://example.org/"
-    assert !response.headers['Set-Cookie']
+class SecureCookiesTest < SSLTest
+  DEFAULT = %(id=1; path=/\ntoken=abc; path=/; secure; HttpOnly)
+
+  def get(**options)
+    self.app = build_app(**options)
+    super 'https://example.org'
+  end
+
+  def assert_cookies(*expected)
+    assert_equal expected, response.headers['Set-Cookie'].split("\n")
+  end
+
+  def test_flag_cookies_as_secure
+    get headers: { 'Set-Cookie' => DEFAULT }
+    assert_cookies 'id=1; path=/; secure', 'token=abc; path=/; secure; HttpOnly'
   end
 
-  def test_redirect_to_host
-    self.app = ActionDispatch::SSL.new(default_app, :host => "ssl.example.org")
-    get "http://example.org/path?key=value"
-    assert_equal "https://ssl.example.org/path?key=value",
-      response.headers['Location']
+  def test_flag_cookies_as_secure_at_end_of_line
+    get headers: { 'Set-Cookie' => 'problem=def; path=/; HttpOnly; secure' }
+    assert_cookies 'problem=def; path=/; HttpOnly; secure'
+  end
+
+  def test_flag_cookies_as_secure_with_more_spaces_before
+    get headers: { 'Set-Cookie' => 'problem=def; path=/; HttpOnly;  secure' }
+    assert_cookies 'problem=def; path=/; HttpOnly;  secure'
   end
 
-  def test_redirect_to_port
-    self.app = ActionDispatch::SSL.new(default_app, :port => 8443)
-    get "http://example.org/path?key=value"
-    assert_equal "https://example.org:8443/path?key=value",
-      response.headers['Location']
+  def test_flag_cookies_as_secure_with_more_spaces_after
+    get headers: { 'Set-Cookie' => 'problem=def; path=/; secure;  HttpOnly' }
+    assert_cookies 'problem=def; path=/; secure;  HttpOnly'
   end
 
-  def test_redirect_to_host_and_port
-    self.app = ActionDispatch::SSL.new(default_app, :host => "ssl.example.org", :port => 8443)
-    get "http://example.org/path?key=value"
-    assert_equal "https://ssl.example.org:8443/path?key=value",
-      response.headers['Location']
+  def test_flag_cookies_as_secure_with_has_not_spaces_before
+    get headers: { 'Set-Cookie' => 'problem=def; path=/;secure; HttpOnly' }
+    assert_cookies 'problem=def; path=/;secure; HttpOnly'
   end
 
-  def test_redirect_to_host_with_port
-    self.app = ActionDispatch::SSL.new(default_app, :host => "ssl.example.org:443")
-    get "http://example.org/path?key=value"
-    assert_equal "https://ssl.example.org:443/path?key=value",
-      response.headers['Location']
+  def test_flag_cookies_as_secure_with_has_not_spaces_after
+    get headers: { 'Set-Cookie' => 'problem=def; path=/; secure;HttpOnly' }
+    assert_cookies 'problem=def; path=/; secure;HttpOnly'
   end
 
-  def test_redirect_to_secure_host_when_on_subdomain
-    self.app = ActionDispatch::SSL.new(default_app, :host => "ssl.example.org")
-    get "http://ssl.example.org/path?key=value"
-    assert_equal "https://ssl.example.org/path?key=value",
-      response.headers['Location']
+  def test_flag_cookies_as_secure_with_ignore_case
+    get headers: { 'Set-Cookie' => 'problem=def; path=/; Secure; HttpOnly' }
+    assert_cookies 'problem=def; path=/; Secure; HttpOnly'
   end
 
-  def test_redirect_to_secure_subdomain_when_on_deep_subdomain
-    self.app = ActionDispatch::SSL.new(default_app, :host => "example.co.uk")
-    get "http://double.rainbow.what.does.it.mean.example.co.uk/path?key=value"
-    assert_equal "https://example.co.uk/path?key=value",
-      response.headers['Location']
+  def test_no_cookies
+    get
+    assert_nil response.headers['Set-Cookie']
   end
 
   def test_keeps_original_headers_behavior
-    headers = Rack::Utils::HeaderHash.new(
-      "Content-Type" => "text/html",
-      "Connection" => ["close"]
-    )
-    self.app = ActionDispatch::SSL.new(lambda { |env| [200, headers, ["OK"]] })
-
-    get "https://example.org/"
-    assert_equal "close", response.headers["Connection"]
+    get headers: { 'Connection' => %w[close] }
+    assert_equal 'close', response.headers['Connection']
   end
 end