spring-projects / spring-security

Spring Security
http://spring.io/projects/spring-security
Apache License 2.0
8.83k stars 5.9k forks source link

SEC-2678: Remove default HSTS header to avoid accidental DoS of HTTP content #2898

Closed spring-projects-issues closed 9 years ago

spring-projects-issues commented 10 years ago

David Eckel (Migrated from SEC-2678) said:

It is trivial to cause an outage of HTTP-only (insecure) content for an entire domain it is hosted on if you deploy an application using Spring Security 3.2+ due to the addition of Strict-Transport-Security by default. It is not intuitive that explicit configuration of security headers is necessary to support HTTP only pages in the same application and in the same domain.

Unlike the other security headers, HSTS affects an entire domain and not just the current URL, and HSTS is cached (currently set to 1 year), making it impossible to restore HTTP functionality for that domain once a browser has been served the HSTS header. This could be disastrous for domains with HTTP-only pages, far worse than the SSL-stripping vulnerability that HSTS mitigates. We have had this happen to us, causing requests to our webapps that redirect from HTTPS to HTTP to go into infinite redirect loop after a user visited an unrelated application under the same domain that set HSTS after it upgraded to Spring Security 3.2.

To avoid this risk, please remove the call to httpStrictTransportSecurity() in https://github.com/spring-projects/spring-security/blob/master/config/src/main/java/org/springframework/security/config/annotation/web/configurers/HeadersConfigurer.java#L239 (added by SEC-2116), and update documentation to reflect that this is an opt-in header, not opt-out.

spring-projects-issues commented 10 years ago

Rob Winch said:

Thank you for reporting this. I understand why you might not want this to happen, but it is best for a framework to default to be as secure as possible (otherwise user's won't know the feature even exists). Also keep in mind that Spring Security's Java Configuration was brand new in 3.2 so the update to Spring Security 3.2 was passive. For these reasons, I'm doubting this change will be made.

making it impossible to restore HTTP functionality for that domain once a browser has been served the HSTS header.

You can easily change the Spring Security configuration and send back an updated max-age=0 to disable it. For example, send a response that includes the following header:

Strict-Transport-Security "max-age=0 ; includeSubDomains"

Perhaps this issue really needs to become a documentation issue (i.e. explain how to fix this situation if it arrises)

spring-projects-issues commented 10 years ago

David Eckel said:

Thanks for taking the time to consider this.

Yes, you can set max-age=0, but you have to do it on pages loaded over HTTPS. If you have parts of your domain or subdomains that don't listen on 443, you'll be forced to make sure that you do support it. To be safe, you'd also have to add that header to all pages under the domain for at least a year to make sure that return visits to the site didn't get blocked. I'd have to imagine that large enterprise sites with many heterogeneous technologies would not have an easy time discovering and implementing the recovery for this.

For example, we have a site where the home page is forced to HTTP, but infrequently visited sub-sections are forced HTTPS (e.g. edit your subscription payment method). Someone would visit the secure section, get HSTS set, then try to navigate later to view insecure content like a blog and find themselves blocked or stuck in a redirect loop. Rolling out the HSTS max-age=0 across hundreds of webapps would take some time. Meanwhile customers would be contacting support channels (assuming they could access support info over HTTPS), or worse, not returning at all.

Using the argument that users may not know if a feature even exists goes both ways in this case. You can cause damage to all users if you don't understand what's going on under the hood. Browsers give zero feedback to the user if they've been blocked by HSTS - it's next to impossible to diagnose unless you know about HSTS. I'd have to assume that if someone don't care enough about security to customize/review the headers, then they probably haven't spent much time digesting the nuances of HSTS.

The web existed for a long time without HSTS, and still isn't supported in any version of IE, I'd argue that it's not critical enough to outweigh the accidental DoS risks. That said, I think it's awesome that all the other security headers are enabled by default. There are other opportunities to improve security by default that won't impact other unrelated parts of the site, like through CSP headers, but it may be hard to find a safe default configuration that won't disrupt other features, since CSP directives can be fairly aggressive.

spring-projects-issues commented 10 years ago

Daniel Serodio said:

I agree with David, this should be opt-in instead of opt-out.

spring-projects-issues commented 9 years ago

David Eckel said:

We got burned by this again internally because of this scenario:

Developer introduces or upgrades Spring security in their webapp, https://example.com/app1

New version of Spring Security introduces HSTS headers by default

Users visit /app1

When any of those people then visit HTTP-only webapps, like http://example.com/app2, they get an error screen that they don't know what to do with.

If this application had been deployed live before the dev realized that HSTS headers were causing outages in unrelated applications, it would be catastrophic. While I feel dirty arguing to reduce security by default, especially now that those defaults are in the wild, I'm assuming that there are other users of Spring Security that still need to support HTTP-only traffic today. None of the other security headers have persistent cross-application impact like HSTS.

Rob's poll from last year: https://twitter.com/rob_winch/status/486349313040056320

Pull request for this (code only, no doc changes): https://github.com/spring-projects/spring-security/pull/203

spring-projects-issues commented 9 years ago

David Eckel said:

As a safeguard, we've added config on our HTTP front-end to remove the strict-transport-security header, if one is accidentally set via Spring in our application layer.

spring-projects-issues commented 9 years ago

Thomas Timbul said:

Having recently migrated from 3.1 to 4.1 I've finally found this after chasing seemingly random redirect errors for several weeks! If anything there should be a massive upgrade warning explaining this change and how it may affect your site (it breaks!)

If hsts is set, it seems you might as well ignore require-channel attributes, because they become meaningless. You should also emit an error on startup, as combining the default strict transport header with require-channel="http" would amount to incompatible configuration.

spring-projects-issues commented 9 years ago

Rob Winch said:

Thank you for the continued feedback.

At the moment I'm still not convinced of this change. As a security framework, we should be secure by default.

Removing the HSTS header is to workaround an issue (i.e. using HTTP) that we do not want to encourage. In modern day web, there really is no good reason to use HTTP and plenty of reason to use HTTPS everywhere. In fact, many implementations of HTTP/2 do not plan to support HTTP. Providing a less secure default seems like a step backwards to me.

I'm going to leave this issue open to keep the discussion and feedback coming. Please do keep in mind that only complaints are likely to be posted here as those in support will not be looking for such an issue.

More than likely, we will instead be improving the documentation to discuss how to undo HSTS in the event someone runs into this scenario.

I added the PR link. NOTE: The current PR doesn't change the defaults for XML configuraiton.

spring-projects-issues commented 9 years ago

Rob Winch said:

ttimbul

If anything there should be a massive upgrade warning explaining this change and how it may affect your site (it breaks!)

We do specify that all headers are added by default now in the migration documentation. However, we could make the impact more apparent. Can you log a JIRA for this?

You should also emit an error on startup, as combining the default strict transport header with require-channel="http" would amount to incompatible configuration.

That is a good point. Can you please log a JIRA for this?

spring-projects-issues commented 9 years ago

Thomas Timbul said:

Created as: https://jira.spring.io/browse/SEC-3038 and https://jira.spring.io/browse/SEC-3039 respectively.

spring-projects-issues commented 9 years ago

Mariano said:

Rob, Agreed. At the moment I'm still not convinced of this change. As a security framework, we should be secure by default.

However the purpose of any framework should be always be customizable, regardless of what you think about security (how secure we want to be is not your decision, that is a users decision).

spring-projects-issues commented 9 years ago

Rob Winch said:

However the purpose of any framework should be always be customizable, regardless of what you think about security (how secure we want to be is not your decision, that is a users decision).

eschizoid I agree that we should empower users to change the defaults. In fact changing the default is quite trivial. Perhaps there is some confusion? You can very easily disable HSTS using the following Java configuration:

http
       ...
    .headers()
        .httpStrictTransportSecurity().disable();

or the following XML configuration

<http>
    <headers>
        <hsts disabled="true" />
    </headers>
</http>

If you have already sent the headers, you can tell the browser to expire them using the following Java configuration:

http
    ...
    .headers()
        .httpStrictTransportSecurity()
            .includeSubdomains(true)
            .maxAgeSeconds(0);

or the following XML configuration

<http>
    <headers>
        <hsts
            include-subdomains="true"
            max-age-seconds="0" />
    </headers>
</http>
spring-projects-issues commented 9 years ago

Mariano said:

Rob, yea that's exactly what I'm doing. Thanks for the guidance though.

Since the HSTS was a new feature, the default behavior should be disabled (otherwise you are breaking existing functionality, complete full working sites). Sites that require this specific feature should enable it, not make the existing sites disable it.

I see the good intentions of spring security encouraging developers to use the HSTS token, but I dunno if it is valid to completely break existing sites in order to be more "secure". Especially when the change was introduced in spring 3.2 (not even a major version). If the change would have been introduced from version 3.x to 4.x, this will be a completely different story.

spring-projects-issues commented 9 years ago

Thomas Timbul said:

Second that - upgrading to 3.2 or higher has disastrous effects, so at least put a warning next to the download link, and to the log at runtime. In fact startup should be prevented altogether if incompatibility is found. Or you should require that users configure HSTS. Whether to disable or enable - their choice, but do not allow a blank configuration. That would be another way to encourage users to think about security without breaking their site and causing headaches.

The problem is not the fact that you're trying to encourage security, or how easy it is to disable HSTS again. It is the fact that the change to the default is poorly publicised, and when you do find yourself stuck with infinite redirect loops, the cause is difficult to find for anyone who has never heard of HSTS before (myself included - it took me 2 weeks and even then only due to finding this bug report).

spring-projects-issues commented 9 years ago

Rob Winch said:

eschizoid ttimbul Thanks for the feedback. I agree the migration guide could better document HSTS and its impact and this is why SEC-3038 was added. This issue is specifically addressing what the default values should be.

I'm curious if you could elaborate on:

Especially when the change was introduced in spring 3.2 (not even a major version). If the change would have been introduced from version 3.x to 4.x, this will be a completely different story.

and

It is the fact that the change to the default is poorly publicised, and when you do find yourself stuck with infinite redirect loops, the cause is difficult to find for anyone who has never heard of HSTS before (myself included - it took me 2 weeks and even then only due to finding this bug report).

I find these statements to be misleading.

It is true that Spring Security 3.2 added support for HSTS. It is also true that by default HSTS was enabled for Java configuration. However, Spring Security 3.2 was the first release to include Spring Security Java configuration. This means that HSTS has always been enabled by default for Java based configuration.

In Spring Security 3.2 HSTS requires explicit configuration to enable it when using XML based configuration. It was not until Spring Security 4 that HSTS was enabled by default with XML configuration. This (and all other) non passive change is listed in the migration guide.

From my perspective this means the default setting for HSTS was passive until a major release. The non passive change was very explicitly called out within the migration guide along with a very simple way to revert to the previous behavior.

Can you elaborate on why you feel that the change was non passive?

spring-projects-issues commented 9 years ago

Thomas Timbul said:

Have you even considered the implications for other webapps deployed in the same container? They are all affected by this one change, and your migration guide and documentation are simply inadequate in highlighting the danger.

From the migration guide:

In many instances, leaving the Security HTTP Response Headers enabled will not have a negative impact on an application. Now that is the modern definition of the word "misleading".

Followed by an unnoticeable information-level panel (with greyed out text, no less):

Strict Transport Security will cause infinite redirects if anywhere within your domain forcefully redirects from HTTPS to HTTP for a subset of pages. If your domain forcfully redirects to HTTP when HTTPS is requested, you will need to ensure to remove the redirect (recommended) or disable Strict Transport Security.

It isn't clear in the slightest that this includes configurations which use requires-channel="http" or the Java config equivalent, hence my feature request to fail startup when such an invalid configuration is detected (linked above).

Because there are many changes in the migration, a lot of which do not necessarily apply depending on your previous configuration, it is easy to overlook the entry, as low down as it is at heading 6.6, and even then as a smallish sub-item!

Developers are encouraged to read Security HTTP Response Headers for details on using this feature. The link within this sentence doesn't work. The immediate "workaround" seemingly appearing in the migration guide is to disable headers altogether. HSTS is not really mentioned (why not at least add the acronym somewhere?), and at the linked http://docs.spring.io/spring-security/site/docs/current/reference/html/headers.html#headers-hsts the implication for the host is never even mentioned. It should be more explicit in saying that you CANNOT use http on the same domain if and once you have HSTS enabled. Same to the migration guide itself.

In both you should directly say something like: Enabling HSTS means that the browser will ALWAYS want to connect to https on your domain in the future (for as long as the HSTS cookie remains valid), so attempting to redirect to http or configuring requires-channel=http will cause infinite redirects. Remember that this will affect other webapps that you may be deploying on the same domain. If you rely on http in other parts of your website or on the same domain that cannot be changed to https, consider disabling HSTS like this: ... Then put the code examples you wrote above. If you have already sent the cookie, do this:... However, we encourage you to consider migrating to full https whenever possible. See Google's take on the issue: (link) In order to do so, remove all explicit http redirects and requires-channel=http In your html ensure that you use protocol relative links or explicitly request https for resources such as images, css and javascript (add any other steps required) Now that would be truly helpful in moving Spring Security applications to be more secure.

The entry in the migration guide can also be improved by using a warning triangle instead of information symbol and formatting "infinite redirects" as bold text. This is a single breaking change that, even if you overlooked everything else, will probably cause your site to fail. Therefore it should perhaps even be the very first heading - the most important configuration item the user must address at all cost, or mentioned in bold text in the preface. So even if you disagree with disabling by default, you must ensure that it is very, very obvious for an upgrader that they will most likely be breaking their site if not paying closest attention to HSTS.

In summary: Failing on startup whenever possible is an absolute must, but it only works at single application level. Therefore documentation must be improved a lot to be able to call the default enabling behaviour acceptable, and the change being "explicitly called out". Your "simple way to revert" is nowhere to be found other than to disable headers completely. It is therefore irrelevant whether the change was passive or not.

spring-projects-issues commented 9 years ago

Mariano said:

Rob,

It is true that Spring Security 3.2 added support for HSTS. It is also true that by default HSTS was enabled for Java configuration. It was not until Spring Security 4 that HSTS was enabled by default with XML configuration.

That's exactly my point, the fact that spring security does not provide the same behavior for java config and xml using the exact same version.

Don't you think it is very confusing to have a different default behavior for java config and XML with the same spring security version? I might be completely wrong, but I do really think the behavior should be equivalent. Probably you need to add a couple of warnings to your guide saying something:

  1. Beware, java config and xml configuration default behavior are not equivalent.
  2. If you deploy spring security within the same container of other apps, these might break.

Whatever is your final decision, thanks for having the time and replying to this post.

Keep us posted ;)

spring-projects-issues commented 9 years ago

Rob Winch said:

ttimbul Thank you for your feedback.

Now that is the modern definition of the word "misleading".

I think there probably is room for improvement on this issue. If you have ideas on how to make that better please comment on SEC-3038

It isn't clear in the slightest that this includes configurations which use requires-channel="http" or the Java config equivalent, hence my feature request to fail startup when such an invalid configuration is detected (linked above).

I am not disputing that we should fail if there is a requires-channel="http" along with HSTS enabled. This is why I requested you create the issue :) Once again, thank you for this suggestion.

In regards to the rest of the documentation improvements, documentation is certainly something that can always be improved. Please feel free to create additional issues as you see fit. This issue should be focused on "should we change the default settings" not how things are documented. The reason we must try and stay focused is that if we try to do too much within a single issue, then things will never be completed.

spring-projects-issues commented 9 years ago

Rob Winch said:

eschizoid Thanks for your feedback.

I agree it is somewhat confusing to have different defaults for Java and XML based configuration. However, it was decided that being a security focused framework that we should prefer security to consistency. The way I look at the two types of configuration is that you are actually using two different implementations of configuration. This means the end result will potentially vary. This is actually quite consistent with the rest of Spring Framework's XML vs Java Configuration.

Probably you need to add a couple of warnings to your guide saying something: ...

We did try to make this obvious. For example, the following is in the opening statements of the HTTP response headers:

For passivity reasons, if you are using Spring Security’s XML namespace support, you must explicitly enable the security headers. All of the default headers can be easily added using the element with no child elements:

SEC-2348 is logged to ensure Spring Security 4.x’s XML namespace configuration will enable Security headers by default.

...

If you are using Spring Security’s Java configuration, all of the default security headers are added by default. They can be disabled using the Java configuration below:

Whatever is your final decision, thanks for having the time and replying to this post.

Thank you for taking the time to report your concerns. I'm sure many users will benefit from the changes of SEC-3039 and SEC-3038 that were a direct result of our discussion on this issue. It sounds as though there may be additional documentation issues coming with the suggestions that were brought up by ttimbul so it is quite likely we will see even more improvements.

spring-projects-issues commented 9 years ago

Rob Winch said:

I'm closing this as "Works as Designed". The defaults of a security framework should be as secure as possible by default (without breaking passivity).

Spring Security 3.2 ensured that existing users experienced the same behavior they had in previous versions. To this end there is no reason in my mind to change in regards to passivity.

Spring Security's newly added Java configuration had different defaults to the XML configuration. Admittedly, this is somewhat confusing. However, the defaults ensured that users with Java Configuration had more secure defaults and a smoother upgrade experience to 4.x all while remaining passive.

One of the major goals of the changes made in Spring Security 4.x was to ensure a more "secure by default" implementation was provided. This can be seen in multiple different changes throughout the framework (i.e. CSRF enabled by default, HTTP Response Headers included by default, etc).

If there are suggestions on how to better document the defaults, then please create separate tickets so we can address those separately.

spring-projects-issues commented 9 years ago

David Eckel said:

If HSTS was scoped only to the application or path/origin, I would agree. But because HSTS (and HPKP) apply to the entire domain, the Spring HSTS default does break passivity for other applications and resources use HTTP.

TinCongHuynh commented 8 years ago

It seems that we could not disable HSTS header in default end point /oauth/token of spring oauth2 with the below source code.

@EnableWebSecurity
public class WebSecurityConfig extends WebSecurityConfigurerAdapter {
        @Override
        protected void configure(HttpSecurity http) throws Exception {
            http
            // ...
            .headers()
                .httpStrictTransportSecurity().disable();
        }
}

Anyone can help me ?

dvdckl commented 7 years ago

We just experienced a self-inflicted DoS because of this issue. Again. One of our teams wasn't aware of this default Spring Security behavior after an upgrade. Our CS reps now need to educate users on how to get out of this state when trying to navigate to properties that aren't HTTPS compatible yet.

The documentation update and warning on startup for same-app mixing of HTTP and HTTPS isn't sufficient to handle the multi-webapp case using a shared host. @rwinch I hope you'll reconsider your stance on this.

dvdckl commented 7 years ago

@rwinch We discovered another team and webapp on our shared domain has fallen victim to accidentally setting the HSTS header via Spring Security. And this is a team made up of Spring and appsec experts that are well aware of how HSTS works, they just didn't catch this side effect of the headers() call. A warning in documentation is insufficient to prevent outages due to this.

While we are trying to move more web pages to HTTPS, external dependencies that don't support HTTPS yet are getting in the way. We also have use cases where we do not want to move some content to HTTPS because we want to take advantage of HTTP transparent caching proxies, especially for mobile and in developing countries.

I understand the aversion to opt-in security, and the philosophy of promoting HTTPS. But I hope everyone can see that HSTS by default is causing more damage than TLS stripping attacks. It behaves very differently than other security headers, and warrants a different default behavior than other security headers.