New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make config.force_ssl
less dangerous to try and easier to disable
#21520
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,72 +1,128 @@ | ||
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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 180.days now There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Already fixed on master: 7c47160 😁 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, missed that. Awesome. |
||
# * `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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an explanation for why this was lowered?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. The rest of the sentence (sorry to be glip) 😁
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That doesn't answer the question... 1 year was perfectly fine, so why lower it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the nugget is in here: #21520 (comment) - but I don't know how to elaborate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One year was fine, but it was arbitrary, and we offered no way to back out of the choice. To reset expectations and improve new-user experience with a dangerous-feeling feature like this, I looked for community guidance. Browser preload qualification starts at 18-week max-age; SSL Labs' recommends 180+ days. That's where most people look to improve their grade, and it's a sensible starting point.