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

Enhancement: optional HSTS #3709

Closed
Cellane opened this issue Oct 10, 2019 · 3 comments · Fixed by #3843
Closed

Enhancement: optional HSTS #3709

Cellane opened this issue Oct 10, 2019 · 3 comments · Fixed by #3843

Comments

@Cellane
Copy link
Member

@Cellane Cellane commented Oct 10, 2019

High Level Plan

  • Add an nginx:set command based on the properties system
    • Valid properties include: hsts, hsts-include-subdomains, hsts-preload, and hsts-max-age. All but the last can be true/false, default to false.
  • Generate the nginx.conf.d/hsts.conf file in nginx_build_config() using a hsts.conf.sigil template and the above setup if hsts == true. Delete if hsts == false.

Original Post

First of all, apologies for not filling in the template, but this ticket is not a bug report, rather an enhancement idea.

Problem

I went through the previous tickets related to SSL and HSTS and found mentions such as “no HSTS at the moment but may do so in a future minor release“ and “I love and use HSTS I don't want to enforce it due to usability concerns - it should be a conscious decision from the operator to use HSTS.

I understand the worry that enabling HSTS by default and automatically could cause unexpected troubles to users, but I think Dokku could go a bit further in enabling “the operator to use HSTS”.

Current situation

Currently, the only (as far as I know) way to enable HSTS on Dokku-level is to customize Nginx config by providing a custom nginx.conf.sigil template that would include the add_header Strict-Transport-Security directive if SSL is enabled for the application.

However, while this could work well for buildpack and Dockerfile-based deploys, it quickly becomes troublesome when deploying an existing Docker image. For example, on my personal website, I run the default, unmodified image of Ghost CMS, and using the above solution would mean the necessity to re-package the vanilla image to add custom Nginx template. Possible, but not exactly user-friendly.

Furthermore, when it is wanted to enable HSTS to all apps running in Dokku, it is suddenly required to add the custom Nginx configuration file into every application’s repository, and perhaps undertake additional steps for Dockerfile-based deploys (I’m not too sure as I don’t use this feature of Dokku, but I remember reading in the documentation something about the necessity of deleting the Nginx configuration file later, for security reasons). Again, possible but at this point, quite troublesome and somewhat annoying.

Another solution would be using a simple plugin – in fact, I was thinking of just forking a completely different plugin that alters Nginx configuration and simply adjusting it to my needs. However, I quickly realized that I’m not quite sure how to ensure that this header is only appended to requests that reached the server through HTTPS…

Proposed solution

I think Dokku could be slightly more helpful in regards to this and while it probably shouldn’t enable HSTS by default for reasons mentioned above, I think it would be a welcome addition to have a new command (perhaps dokku certs:hsts <app> [--enable --disable]?) that would alter generating of the Nginx configuration file in such a way that if the administrator wishes to have HSTS enabled for an application, he or she can do so easily and quickly.

Repeating this task for other applications then just involves re-running the same command with a different application name.

I wish I had the skill to implement this myself, but until then, all I can do is ask you for your kind consideration. Thank you for reading through this, and thank you even more for the amazing work you’ve done on Dokku!

@Cellane
Copy link
Member Author

@Cellane Cellane commented Oct 11, 2019

In the end, I found a way to alter Nginx configuration in (hopefully) desired way, and created a small plugin that allows enabling HSTS on a per-app basis.

You can find it at Cellane/dokku-hsts, although I still think this feature should be present in Dokku itself due to all the reasons mentioned above.

@josegonzalez
Copy link
Member

@josegonzalez josegonzalez commented Dec 19, 2019

I think it's time to set this, and even make hsts default. We've been striving for the most secure setup by default, and while hsts is sometimes annoying to disable, I think the tradeoff is worth it. Thoughts @michaelshobbs?

We can support the following properties:

  • hsts: bool, default true
  • hsts-include-subdomains: bool, default true
  • hsts-max-age: integer, time in seconds
  • hsts-preload: bool, default false

These values/defaults come from what the kubernetes ingress controller does.

@josegonzalez
Copy link
Member

@josegonzalez josegonzalez commented Feb 3, 2020

Closing as there is a pull request open.

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

Successfully merging a pull request may close this issue.

2 participants