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

#854 Security Headers #1680

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/OrchardCore.Build/Dependencies.AspNetCore.props
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
<MicrosoftAspNetCoreHttpFeaturesPackageVersion>2.0.2</MicrosoftAspNetCoreHttpFeaturesPackageVersion>
<MicrosoftAspNetCoreHttpOverridesPackageVersion>2.0.2</MicrosoftAspNetCoreHttpOverridesPackageVersion>
<MicrosoftAspNetCoreHttpPackageVersion>2.0.2</MicrosoftAspNetCoreHttpPackageVersion>
<MicrosoftAspNetCoreHttpsPolicyPackageVersion>2.1.0-preview2-final</MicrosoftAspNetCoreHttpsPolicyPackageVersion>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will update this when we move to 2.1.

<MicrosoftAspNetCoreIdentityEntityFrameworkCorePackageVersion>2.0.2</MicrosoftAspNetCoreIdentityEntityFrameworkCorePackageVersion>
<MicrosoftAspNetCoreIdentityPackageVersion>2.0.2</MicrosoftAspNetCoreIdentityPackageVersion>
<MicrosoftAspNetCoreIdentityServiceAbstractionsPackageVersion>2.0.1</MicrosoftAspNetCoreIdentityServiceAbstractionsPackageVersion>
Expand Down Expand Up @@ -89,7 +90,6 @@
<MicrosoftAspNetCoreResponseCachingAbstractionsPackageVersion>2.0.2</MicrosoftAspNetCoreResponseCachingAbstractionsPackageVersion>
<MicrosoftAspNetCoreResponseCachingPackageVersion>2.0.2</MicrosoftAspNetCoreResponseCachingPackageVersion>
<MicrosoftAspNetCoreResponseCompressionPackageVersion>2.0.2</MicrosoftAspNetCoreResponseCompressionPackageVersion>
<MicrosoftAspNetCoreRewritePackageVersion>2.0.2</MicrosoftAspNetCoreRewritePackageVersion>
<MicrosoftAspNetCoreRoutingAbstractionsPackageVersion>2.0.2</MicrosoftAspNetCoreRoutingAbstractionsPackageVersion>
<MicrosoftAspNetCoreRoutingPackageVersion>2.0.2</MicrosoftAspNetCoreRoutingPackageVersion>
<MicrosoftAspNetCoreServerHttpSysPackageVersion>2.0.2</MicrosoftAspNetCoreServerHttpSysPackageVersion>
Expand Down
1 change: 1 addition & 0 deletions src/OrchardCore.Cms.Web/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ public static void Main(string[] args)

public static IWebHost BuildWebHost(string[] args)
=> WebHost.CreateDefaultBuilder(args)
.UseKestrel(c => c.AddServerHeader = false)
.UseNLogWeb()
.UseStartup<Startup>()
.Build();
Expand Down
5 changes: 5 additions & 0 deletions src/OrchardCore.Cms.Web/web.config
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,10 @@
<aspNetCore processPath="%LAUNCHER_PATH%" arguments="%LAUNCHER_ARGS%" stdoutLogEnabled="false" stdoutLogFile=".\logs\stdout" forwardWindowsAuthToken="false" startupTimeLimit="3600" requestTimeout="23:00:00">
<environmentVariables />
</aspNetCore>
<httpProtocol>
<customHeaders>
<remove name="X-Powered-By" />
</customHeaders>
</httpProtocol>
</system.webServer>
</configuration>
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.AspNetCore.Rewrite" Version="$(MicrosoftAspNetCoreRewritePackageVersion)" />
<PackageReference Include="Microsoft.AspNetCore.HttpsPolicy" Version="$(MicrosoftAspNetCoreHttpsPolicyPackageVersion)" />
</ItemGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Options;
using Microsoft.AspNetCore.HttpsPolicy;

namespace OrchardCore.Https.Services
{
public class HttpsRedirectionConfiguration : IConfigureOptions<HttpsRedirectionOptions>
{
private readonly IHttpsService _httpsService;

public HttpsRedirectionConfiguration(IHttpsService httpsService)
{
_httpsService = httpsService;
}

public void Configure(HttpsRedirectionOptions options)
{
var httpsSettings = _httpsService.GetSettingsAsync().GetAwaiter().GetResult();

options.RedirectStatusCode = httpsSettings.RequireHttpsPermanent
? StatusCodes.Status301MovedPermanently
: StatusCodes.Status302Found;
options.HttpsPort = httpsSettings.SslPort;
}
}
}

This file was deleted.

42 changes: 27 additions & 15 deletions src/OrchardCore.Modules/OrchardCore.Https/Startup.cs
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
using System;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Rewrite;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.DependencyInjection.Extensions;
using Microsoft.Extensions.Options;
using OrchardCore.DisplayManagement.Handlers;
using OrchardCore.Environment.Navigation;
using OrchardCore.Https.Drivers;
Expand All @@ -16,24 +14,38 @@ namespace OrchardCore.Https
{
public class Startup : StartupBase
{
public override void Configure(IApplicationBuilder app, IRouteBuilder routes, IServiceProvider serviceProvider)
public override void ConfigureServices(IServiceCollection services)
{
var rewriteOptions = new RewriteOptions();
services.AddScoped<INavigationProvider, AdminMenu>();
services.AddScoped<IDisplayDriver<ISite>, HttpsSettingsDisplayDriver>();
services.AddSingleton<IHttpsService, HttpsService>();

// Configure the rewrite options.
serviceProvider.GetService<IConfigureOptions<RewriteOptions>>().Configure(rewriteOptions);
services.AddHttpsRedirection(options => {});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

options => {} may look odd since we don't configure any options but I want to still call this in case in the future the library registers required services in this method.

services.ConfigureOptions<HttpsRedirectionConfiguration>();

app.UseRewriter(rewriteOptions);
services.AddHsts(options =>
{
options.MaxAge = TimeSpan.FromDays(365);
options.Preload = false;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making preload false by default. Preloading can have a permanent impact on users and should be an explicit choice. See here for additional explanation: roots/trellis#727

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should add these as comments in the code

options.IncludeSubDomains = true;
});
}

public override void ConfigureServices(IServiceCollection services)
public override void Configure(IApplicationBuilder app, IRouteBuilder routes, IServiceProvider serviceProvider)
{
services.AddScoped<INavigationProvider, AdminMenu>();
services.AddScoped<IDisplayDriver<ISite>, HttpsSettingsDisplayDriver>();
services.AddSingleton<IHttpsService, HttpsService>();
services.AddSingleton(new RewriteOptions());

services.TryAddEnumerable(ServiceDescriptor.Transient<IConfigureOptions<RewriteOptions>, RewriteOptionsHttpsConfiguration>());
// Determine if SSL redirects are enabled
var settings = serviceProvider.GetService<IHttpsService>().GetSettingsAsync().GetAwaiter().GetResult();

if (settings.RequireHttps)
{
var env = serviceProvider.GetService<IHostingEnvironment>();
if (!env.IsDevelopment())
{
app.UseHsts();
}

app.UseHttpsRedirection();
}
}
}
}
4 changes: 4 additions & 0 deletions src/OrchardCore.Modules/OrchardCore.Users/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
using OrchardCore.Modules;
using OrchardCore.Security;
using OrchardCore.Security.Permissions;
using OrchardCore.Security.SecurityHeaders;
using OrchardCore.Settings;
using OrchardCore.Setup.Events;
using OrchardCore.Users.Commands;
Expand Down Expand Up @@ -48,11 +49,14 @@ public override void Configure(IApplicationBuilder builder, IRouteBuilder routes
template: LoginPath,
defaults: new { controller = "Account", action = "Login" }
);

builder.UseSecurityHeaders();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where should we call this from instead of the Users module?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be in its own feature, and we should rename the Https module to Security. WE might even want to split all these options in separate features if they make sense independently.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review @sebastienros , I'll go ahead and split up the security headers into different features and initialize them from the OrchardCore.Security module after renaming it from OrchardCore.Https.

}

public override void ConfigureServices(IServiceCollection services)
{
services.AddSecurity();
services.AddSecurityHeaders();

// Adds the default token providers used to generate tokens for reset passwords, change email
// and change telephone number operations, and for two factor authentication token generation.
Expand Down
1 change: 1 addition & 0 deletions src/OrchardCore.Mvc.Web/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ public static void Main(string[] args)

public static IWebHost BuildWebHost(string[] args)
=> WebHost.CreateDefaultBuilder(args)
.UseKestrel(c => c.AddServerHeader = false)
.UseStartup<Startup>()
.Build();
}
Expand Down
5 changes: 5 additions & 0 deletions src/OrchardCore.Mvc.Web/web.config
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,10 @@
<add name="aspNetCore" path="*" verb="*" modules="AspNetCoreModule" resourceType="Unspecified"/>
</handlers>
<aspNetCore processPath="%LAUNCHER_PATH%" arguments="%LAUNCHER_ARGS%" stdoutLogEnabled="false" stdoutLogFile=".\logs\stdout" forwardWindowsAuthToken="false"/>
<httpProtocol>
<customHeaders>
<remove name="X-Powered-By" />
</customHeaders>
</httpProtocol>
</system.webServer>
</configuration>
1 change: 1 addition & 0 deletions src/OrchardCore.Nancy.Web/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ public static void Main(string[] args)

public static IWebHost BuildWebHost(string[] args)
=> WebHost.CreateDefaultBuilder(args)
.UseKestrel(c => c.AddServerHeader = false)
.UseStartup<Startup>()
.Build();
}
Expand Down
5 changes: 5 additions & 0 deletions src/OrchardCore.Nancy.Web/web.config
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,10 @@
<add name="aspNetCore" path="*" verb="*" modules="AspNetCoreModule" resourceType="Unspecified"/>
</handlers>
<aspNetCore processPath="%LAUNCHER_PATH%" arguments="%LAUNCHER_ARGS%" stdoutLogEnabled="false" stdoutLogFile=".\logs\stdout" forwardWindowsAuthToken="false"/>
<httpProtocol>
<customHeaders>
<remove name="X-Powered-By" />
</customHeaders>
</httpProtocol>
</system.webServer>
</configuration>
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@

<ItemGroup>
<PackageReference Include="Microsoft.AspNetCore.Authorization" Version="$(MicrosoftAspNetCoreAuthorizationPackageVersion)" />
<PackageReference Include="Microsoft.Extensions.Options" Version="2.1.0-preview2-final" />
</ItemGroup>

<ItemGroup>
<Folder Include="SecurityHeaders\" />
</ItemGroup>
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
using System;
using Microsoft.AspNetCore.Builder;
using OrchardCore.Security.SecurityHeaders;

namespace OrchardCore.Security
{
/// <summary>
/// Extension methods for the security headers middleware.
/// </summary>
public static class ApplicationBuilderExtensions
{
/// <summary>
/// Adds middleware for using common security headers.
/// </summary>
/// <param name="app">The <see cref="IApplicationBuilder"/> instance this method extends.</param>
/// <returns>The <see cref="IApplicationBuilder"/>.</returns>
public static IApplicationBuilder UseSecurityHeaders(this IApplicationBuilder app)
{
if (app == null)
{
throw new ArgumentNullException(nameof(app));
}

return app.UseMiddleware<SecurityHeadersMiddleware>();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
namespace OrchardCore.Security.SecurityHeaders
{
/// <summary>
/// The referrer policy for the security header.
/// </summary>
/// <remarks>
/// See: https://w3c.github.io/webappsec-referrer-policy/#referrer-policy-header
/// </remarks>
public enum ReferrerPolicy
{
/// <summary>
/// The Referer header will be omitted entirely from browser requests. No referrer information is sent along with requests.
/// </summary>
NoReferrer,

/// <summary>
/// This is the user agent's default behavior if no policy is specified. The origin is sent as a referrer
/// when the protocol security level stays the same (HTTPS->HTTPS), but isn't sent to a less secure
/// destination (HTTPS->HTTP).
/// </summary>
NoReferrerWhenDowngrade,

/// <summary>
/// Only send the origin of the document as the referrer in all cases.
/// The document https://example.com/page.html will send the referrer https://example.com/.
/// </summary>
Origin,

/// <summary>
/// Send a full URL when performing a same-origin request, but only send the origin of the document for other cases.
/// </summary>
OriginWhenCrossOrigin,

/// <summary>
/// A referrer will be sent for same-site origins, but cross-origin requests will contain no referrer information.
/// </summary>
SameOrigin,

/// <summary>
/// Only send the origin of the document as the referrer to a-priori as-much-secure destination (HTTPS->HTTPS),
/// but don't send it to a less secure destination (HTTPS->HTTP).
/// </summary>
StrictOrigin,

/// <summary>
/// Send a full URL when performing a same-origin request, only send the origin of the document to a-priori as-much-secure destination (HTTPS->HTTPS),
/// and send no header to a less secure destination (HTTPS->HTTP).
/// </summary>
StrictOriginWhenCrossOrigin,

/// <summary>
/// Send a full URL when performing a same-origin or cross-origin request.
/// This policy will leak origins and paths from TLS-protected resources to insecure origins. Carefully consider the impact of this setting.
/// </summary>
UnsafeUrl
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
using System;
using System.Collections.Generic;

namespace OrchardCore.Security.SecurityHeaders
{
/// <summary>
/// Options for the security headers to be added to all responses
/// </summary>
public class SecurityHeaderOptions
{
/// <summary>
/// A collection of headers to be added to all the server's responses.
/// </summary>
public IDictionary<string, string> AddHeaders { get; } = new Dictionary<string, string>();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
using System;
using Microsoft.Extensions.Options;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Extensions;
using Microsoft.AspNetCore.Http.Internal;
using Microsoft.Extensions.Configuration;
using Microsoft.Net.Http.Headers;
using System.Threading.Tasks;

namespace OrchardCore.Security.SecurityHeaders
{
public class SecurityHeadersMiddleware
{
private readonly RequestDelegate _next;
private readonly SecurityHeaderOptions _headers;

public SecurityHeadersMiddleware(RequestDelegate next, IOptions<SecurityHeaderOptions> headers)
{
if (headers == null)
{
throw new ArgumentNullException(nameof(headers));
}

_next = next ?? throw new ArgumentNullException(nameof(next));

_headers = headers.Value;
}

public async Task Invoke(HttpContext context)
{
if (context == null)
{
throw new ArgumentNullException(nameof(context));
}

var response = context.Response;

if (response == null)
{
throw new ArgumentNullException(nameof(response));
}

var headers = response.Headers;

foreach (var header in _headers.AddHeaders)
{
headers[header.Key] = header.Value;
}

await _next(context);
}
}
}
Loading