Skip to content
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

HSTS without IncludeSubdomains is often useless #22663

Closed
wants to merge 1 commit into from
Closed

HSTS without IncludeSubdomains is often useless #22663

wants to merge 1 commit into from

Conversation

homakov
Copy link
Contributor

@homakov homakov commented Dec 18, 2015

  1. Because if you forget to add Secure; to the session cookie, it will leak to http:// subdomain in some cases
  2. Because http:// subdomain can Cookie Bomb/cookie force main domain or be used for phishing.

That's why by default it must include subdomains as it's much more common scenario. Very few websites intend to leave their blog.app.com working over http:// while having everything else encrypted.

Yes, many developers forget to add subdomains=true manually, believe me :)

1) Because if you forget to add Secure; to the session cookie, it will leak to http:// subdomain in some cases
2) Because http:// subdomain can Cookie Bomb/cookie force main domain or be used for phishing.

That's why *by default* it must include subdomains as it's much more common scenario. Very few websites *intend* to leave their blog.app.com working over http:// while having everything else encrypted. 

Yes, many developers forget to add subdomains=true by default, believe me :)
@rails-bot
Copy link

r? @senny

(@rails-bot has picked a reviewer for you, use r? to override)

@FiloSottile
Copy link

I tend to agree (but put a good warning somewhere in the docs). Without IncludeSubdomains it also does not save you from https://example.com -> http://www.example.com redirects. https://twitter.com/jgrahamc/status/677829158366543872

@jeremy
Copy link
Member

jeremy commented Dec 18, 2015

Agree, but it's a breaking change. Path forward:

  1. Generate new apps with config that sets subdomains: true. Add a test.
  2. Add a deprecation warning that _un_specified subdomains will mean true instead of false in Rails 5.1. Add a test.
  3. After 5-0-stable branches, flip the default and remove the deprecation in master.

@maclover7 maclover7 added this to the 5.0.0 milestone Jan 9, 2016
@prathamesh-sonpatki
Copy link
Member

@homakov are you working on Jeremy's suggestions? Otherwise I can start off on your branch and work on suggestions.

@senny
Copy link
Member

senny commented Feb 1, 2016

r? @jeremy

@rails-bot rails-bot assigned jeremy and unassigned senny Feb 1, 2016
@homakov
Copy link
Contributor Author

homakov commented Feb 2, 2016

@prathamesh-sonpatki no, I dont code. Feel free to help thanks.

@prathamesh-sonpatki
Copy link
Member

@homakov Thanks, I will work on Jeremy's suggestions.

@dhh dhh assigned prathamesh-sonpatki and unassigned jeremy Feb 10, 2016
@prathamesh-sonpatki
Copy link
Member

I have a branch locally, will get it wrapped in next few days.

@dhh
Copy link
Member

dhh commented Feb 24, 2016

@prathamesh-sonpatki Did you look into this?

@prathamesh-sonpatki
Copy link
Member

Yes, I am working on it.

@prathamesh-sonpatki
Copy link
Member

Closing in favor of #23852

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

Successfully merging this pull request may close these issues.

None yet

8 participants