Security Headers

[jrestall]

Hi guys,

What are your thoughts on implementing the best practice security headers out of the box in Orchard? We don't necessarily need to have them enabled by default but I was thinking a config switch might be helpful to users of the CMS and developers using the framework.

https://securityheaders.io/ would give us an F rating currently which I'd like to move up.

These are the ones we are currently missing support for:

• Access-Control-Allow-* (CORS) (Orchard.Cors In Progress)
• X-XSS-Protection
• X-Frame-Options
• X-Content-Type-Options
• Referrer-Policy
• Content-Security-Policy (CSP)
• Strict-Transport-Security
• Public-Key-Pins (PKP)

In addition I would recommend removing the Kestrel Server header and IIS X-Powered-By. I could submit a pull request with the web.config changes for IIS and the AddServerHeader = false for Kestrel.

For the other headers I could create a middleware in the Security module and have a new SecurityHeaderOptions class so that they could be toggled and configured from appsettings.json?

 "Modules": {
    "Orchard.Security":{
      "SecurityHeaders":{
        "RemoveServer": true,
        "XFrameOptions":"deny",
        "XssProtection":"1;mode=block",
        "XContentTypeOptions":"nosniff",
        "StrictTransportSecurity":"max-age=31536000; preload",
        "ReferrerPolicy":"none",
        "PublicKeyPins":"pin-sha256="<pin-value>"; 
                 max-age=<expire-time>; 
                 includeSubDomains; 
                 report-uri="<uri>"",
        "ContentSecurityPolicy":"..."
      }
    }
  }

Feedback would be needed on sensible default values and which to toggle on by default.

Please let me know if there is interest in this and thoughts on the technical design and source location of such functionality.

[Jetski5822]

I like the idea, and hate the idea of an F!!! Hey @PinpointTownes and @jersiovic - as the chaps that did the security side of things so far, what do you think? I know you both would hate having an F!

[jersiovic]

+1 to move up. So, please go for that PR

[kevinchalet]

Sounds like a very good idea, indeed 👍

That said, please consider leveraging an existing library instead of reinventing the wheel.

I personally use NWebsec for all my projects (https://github.com/NWebsec/NWebsec/tree/master/src/NWebsec.AspNetCore.Middleware), but there are other projects like https://github.com/TerribleDev/HardHat or https://github.com/andrewlock/NetEscapades.AspNetCore.SecurityHeaders that can help you with that.

The hard part will be making all that stuff not too confusing/too hard to use for the developers. Specially since an invalid configuration can lead to terrible consequences 😅

[Jetski5822]

hey @jrestall Did you manage to make any head way with this?

[rserj]

I guess we should decide where to store such configuration in Config file or Db.
I think it would be better to store configuration in Db, so Admin can easily view/change configuration at runtime, +we can add some helpful hints on UI to Admin about each option.
(Changing configuration may require restarting Tenant)

I'm thinking about multiple proposals

1. What if we create OrchardCore.HttpSecurity module
   And create a class
   HttpSecurityHeaderSettings : Entity
   like it was done with SiteSettings and User, so we can create drivers responsible for each of the configuration, like Cors, XSS, Force ContentType, X-Frame-Options, ...
   Adv.
   We can easily extend configuration by adding New Drivers
   Disadv.
   Since all of these Config sections rendered in one page it will be messy.
   Until, we migrate that feature from Orchard 1.10.x which allows to organize Part configuration by Tabs
   http://schouten-online.nl/projecten/created-content-editor-tabs-feature-in-the-core-of-orchard-cms

2. We may create multiple Orchard Features in one module. Each Feature per "Security Header" Configuration (Cors, XSS, Force ContentType, X-Frame-Options, ...).
   Each of the features will have Controllers/View as usual.
   I'm not sure about Admin menu, how we should organize it per "Security Header" Configuration, Does Orchard Core renders third level menus as Tabs like Orchard 1?

I think in this case 2nd approach will be better.
What do you think about using one of the above @PinpointTownes suggested libraries? Each Feature can read it's own configuration and use one of the NWebsec middlewares for example.

cc: @sebastienros

[sebastienros]

I think we can handle it with a single, specialized module, don't need for extensibility here. Just settings to enable specific properties.

[sebastienros]

As mentioned in #1277, we should have this as features in the same Security module.

Another solution is to have a single feature with a single configuration page, and suggest to users that they should check everything. Bonus points if the settings point to articles about why the setting is important.

I am setting the milestone to beta2.

[sebastienros]

/cc @petedavis

[petedavis]

Using cloudflare as an example, clicking enable HSTS should come with some level of acknowledgement before enabling it on a site:

And we should try and configure the middle ware with from settings:

[sebastienros]

But we set HSTS when the "force https" option is set, isn't that good enough?

[petedavis]

Having HSTS enabled by default and a zero max-age is missing some of the main benefits of HSTS. Knowing that you cannot disable HTTPS support until HSTS is disabled, and the HSTS max age expires should be known. And then max age can be used effectively as people should understand this is on and cant be turned of like a switch like just HTTPS can.

[jptissot]

I am currently configuring a deployment of Orchard and attempted to implement a CSP for it. I found some issues that need to be addressed. I wanted to force these headers to all tenants, so a module would not be useful for my use case.

• We depend on many CDN's and not all of the resources are being validated by a checksum.
• There is no easy way to change the source of all these scripts (I would like to host them all locally)
• The Asset manager uses vue.js that compiles the templates on the fly. This forces enabling the script-src: 'unsafe-eval'. Vue recommends precompiling the templates to avoid this.
• I would also like to be able to enable to remove script-src: 'unsafe-inline' but that would require all scripts be loaded from files, I am not sure how feasible this is or if it is that big of a security issue.
• Is it possible to add some of this configuration only to the Admin pages ? Maybe through some kind of Middleware ?

Using the https://github.com/andrewlock/NetEscapades.AspNetCore.SecurityHeaders package.
This is what my Startup.cs looks like:

using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting;
using Microsoft.Extensions.DependencyInjection;

namespace web
{
    public class Startup
    {
        public void ConfigureServices(IServiceCollection services)
        {
            services.AddOrchardCms();
        }

        public void Configure(IApplicationBuilder app, IHostingEnvironment env)
        {
            var policyCollection = new HeaderPolicyCollection()
              .AddFrameOptionsSameOrigin()
              .AddXssProtectionBlock()
              .AddContentTypeOptionsNoSniff()
              .AddReferrerPolicyStrictOriginWhenCrossOrigin()
              .RemoveServerHeader()
              .AddContentSecurityPolicy(builder =>
              {
                  builder.AddObjectSrc().Self();
                  builder.AddFormAction().Self();
                  builder.AddFrameAncestors().Self();
                  builder.AddDefaultSrc().Self();
                  builder.AddFontSrc().Self().From("cdn.jsdelivr.net").From("fonts.googleapis.com").From("fonts.gstatic.com");
                  
                  builder.AddStyleSrc().UnsafeInline().Self().From("cdn.jsdelivr.net").From("fonts.googleapis.com").From("cdn.datatables.net")
                  .From("unpkg.com")
                  .From("cdnjs.cloudflare.com")
                  ;
                  // unsafe-evail needed for vue.js runtime templates
                  builder.AddScriptSrc().UnsafeEval().UnsafeInline().Self()
                   .From("cdn.jsdelivr.net").From("cdn.datatables.net").From("cdnjs.cloudflare.com").From("vuejs.org")
                   .From("unpkg.com")
                   .From("cdnjs.cloudflare.com")
                   ;
              });

            if (env.IsDevelopment())
            {
                app.UseDeveloperExceptionPage();
            } else
            {
                policyCollection.AddStrictTransportSecurityMaxAgeIncludeSubDomains(maxAgeInSeconds: 60 * 60 * 24 * 365); // maxage = one year in seconds
            }

            app.UseSecurityHeaders(policyCollection)
                .UsePoweredByOrchardCore(false)
                .UseStaticFiles()
                .UseOrchardCore(builder=>builder
                    .UsePoweredByOrchardCore(false)
                    .UseCookiePolicy(new CookiePolicyOptions()
                    {
                        HttpOnly = Microsoft.AspNetCore.CookiePolicy.HttpOnlyPolicy.Always,
                        Secure = Microsoft.AspNetCore.Http.CookieSecurePolicy.SameAsRequest
                    })
                );
        }
    }
}

Edit: Some of the issues I mentionned might be fixed when this lands: dotnet/aspnetcore#6001

[lahma]

Mozilla's Observatory gives a pretty readable report

[hishamco]

@sebastienros shall we add this as a middleware, something similar to PoweredByMiddleware or shall we add as a module?

[hishamco]

I think we can handle it with a single, specialized module, don't need for extensibility here. Just settings to enable specific properties.

@sebastienros shall we add new module for that OrchardCore.Security, or can we add a simple middleware as I suggested above?

[sebastienros]

If we have a security module use it, I think we have one. Dedicated feature might be nice.

[hishamco]

I think we have one.

Where? Do you mean Https?

[sebastienros]

If we just have an https module, we could create a security one and move the https feature there

[hishamco]

Make sense, but for now we can introduce a Security module, then add HTTPs as feature before release 2.0.0 to avoid breaking changes

[hishamco]

The OrchardCore.Security module should have at least the following features:

1. Content Security Policy
2. XSS Protection
3. Strict Transport Security
4. Referrer Policy
5. Frame Options

Perhaps there are few features related to Https & Cors module, so we could add them here by enabling related modules or move some of them to the OrchardCore.Security as suggested above

@Piedone your feedback please I'm planning to start a PR soon ...

[Piedone]

What does https://securityheaders.com/ say?

[hishamco]

https://securityheaders.com/?q=https%3A%2F%2Forchardcore.net%2F states the following:

• Strict-Transport-Security
• Content-Security-Policy
• X-Frame-Options
• X-Content-Type-Options
• Referrer-Policy
• Permissions-Policy

[Piedone]

So, I think the goal should be that a vanilla Orchard site gives a flawless score with the extension, with corresponding docs on the necessary setup.

[hishamco]

Few of them are optional, so can we add the above list as features or we they should be added once the module is enabled with no option to toggle them? I think make them as features is a very good choice

So, I think the goal should be that a vanilla Orchard site gives a flawless score with the extension, with corresponding docs on the necessary setup.

We could enable this module on setup

[jptissot]

Have a look at this:

https://github.com/StatCan/StatCan.OrchardCore/blob/master/src/Lib/StatCan.OrchardCore.Security/SecurityExtensions.cs

Maybe a good starting point.

[sebastienros]

@hishamco good, watching

[hishamco]

Adding those header as features are good and bad because the intend to make the default OC site supports those header. So, I will leave this for now

[Piedone]

I don't really understand what you mean by "So, I will leave this for now"

[hishamco]

My intend at the beginning to add these header as separated feature, so they can be enabled or disabled, but this conflict with the original issue where the header should be present

What I did in the related PR is add the headers when the module is enabled (which should be always enabled)

[hishamco]

BTW @Piedone seems the extension that you refer to is working only for a live site, for that my testing is to check the response header

[Piedone]

@sebastienros once we're done here, it would be great if some security expert from your team could also test it while it's running (no need for a detailed code review but that would be even better).

[hishamco]

I already checked the response headers, everything seems works as expected, also I may look for an extension for test this too

[hishamco]

@jrestall finally ;)

[jrestall]

@hishamco Haha that's epic to see this complete after 5 years, really nice job! 🎉

[hishamco]

It happens ;) you remind me with one of my PR which take two years or more to get merged