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

Security bug: HSTS header is sent over HTTP and not HTTPS #1267

Closed
ZimbiX opened this issue Sep 22, 2020 · 1 comment · Fixed by #1268
Closed

Security bug: HSTS header is sent over HTTP and not HTTPS #1267

ZimbiX opened this issue Sep 22, 2020 · 1 comment · Fixed by #1268

Comments

@ZimbiX
Copy link
Contributor

ZimbiX commented Sep 22, 2020

Lucky's ForceSSLHandler class adds the HTTP Strict Transport Security (HSTS) header to HTTP requests only, when it needs to instead add it to HTTPS requests only.


I discovered this after enabling the force TLS redirect and HSTS settings as documented in the generated config/server.cr, using:

Lucky::ForceSSLHandler.configure do |settings|
  settings.enabled = Lucky::Env.production?
  settings.strict_transport_security = {max_age: 1.year, include_subdomains: true}
end

I was looking into submitting my site for the HSTS preload list (so that on supported browsers, an insecure request doesn't need to be made at all), when I found my site was ineligible. The result is the same for Lucky's site:

Screenshot from 2020-09-22 20-12-21

The information provided in that message and the MDN docs (which are linked to from the Lucky docs, ironically) says that the HSTS header needs to be provided when the site is requested over HTTPS only. Which makes sense, as an HTTP request could be tampered with to remove the header.

I checked what the current behaviour is:

➜ curl -I luckyframework.org         
HTTP/1.1 308 Permanent Redirect
Server: Cowboy
Date: Tue, 22 Sep 2020 09:55:19 GMT
Connection: keep-alive
Strict-Transport-Security: max-age=31104000; includeSubDomains
Location: https://luckyframework.org/
Content-Length: 0
Via: 1.1 vegur

➜ curl -I https://luckyframework.org
HTTP/1.1 200 OK
Server: Cowboy
Date: Tue, 22 Sep 2020 09:55:25 GMT
Connection: keep-alive
Content-Type: text/html
Transfer-Encoding: chunked
Set-Cookie: _website_v2_session=bHVja3k%3D%0A--epgdvfAmkR4kgescH8frKE05vnnMfTKAob1Q5VYO%2FX2EnmB6QuJh54pQudlnpzw%2BS8DRPIdpwBtsYloC5zk8b4%2FRmQWCDGQNeZOyMd1eyZEb7eBdukOZoEbb4IkH%2Fi8z; path=/; Secure; HttpOnly; SameSite=Lax
Via: 1.1 vegur

And yes, the Strict-Transport-Security was present in the wrong response.

Fixing it should be relatively simple, even for a Crystal-newbie like myself, so I'll start on a PR now.

btw, you folks really should provide info on somewhere to report security vulnerabilities =P e.g. https://securitytxt.org/

Cheers for the awesome framework!


CC @jwoertink - just a heads up since you implemented it in #734 ;)

@jwoertink
Copy link
Member

😅 figures I'd miss something lol. Thanks for catching this!

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

Successfully merging a pull request may close this issue.

2 participants