aspnetcore-security-headers icon indicating copy to clipboard operation
aspnetcore-security-headers copied to clipboard

Add Security Headers in Response.OnStarting

Open agilenut opened this issue 5 years ago • 7 comments

It looks like you are adding headers before calling next() in the middleware. This means that any middleware registered after the security header middleware does not have a chance to preempt the middleware and add their own more appropriate headers.

Have you considered registering an action to be invoked on Response.OnStarting? This would allow for you to check to see if any other middleware closer to the response generation had already added the header.

agilenut avatar Apr 11 '19 19:04 agilenut

Hmm, could you link to a sample of its usage? That certainly sounds like something that could be done.

juunas11 avatar Apr 11 '19 19:04 juunas11

Sure, here's an example: https://stackoverflow.com/questions/35047563/how-to-intercept-response-in-asp-net-5-after-all-other-middlewares

agilenut avatar Apr 11 '19 19:04 agilenut

Thanks, I'll try to look at doing this in the weekend.

juunas11 avatar Apr 11 '19 19:04 juunas11

FYI, I just did a quick test with this and it seemed to work

    public class ContentSecurityPolicyMiddleware
    {
        private const string CspHeaderName = "Content-Security-Policy";
        private const string CspReportOnlyHeaderName = "Content-Security-Policy-Report-Only";
        private readonly RequestDelegate _next;
        private readonly CspOptions _options;
        private readonly string _headerName;
        private readonly string _headerValue;

        /// <summary>
        /// Initializes a new instance of the <see cref="ContentSecurityPolicyMiddleware"/> class.
        /// </summary>
        /// <param name="next">The next.</param>
        /// <param name="options">The options.</param>
        public ContentSecurityPolicyMiddleware(RequestDelegate next, IOptions<CspOptions> options)
        {
            if (options == null)
            {
                throw new ArgumentNullException(nameof(options));
            }

            _next = next;
            _options = options.Value;
            if (_options.IsNonceNeeded)
            {
                _headerName = null;
                _headerValue = null;
            }
            else
            {
                var valueTuple = _options.ToString(null);
                var str1 = valueTuple.Item1;
                var str2 = valueTuple.Item2;
                _headerName = str1;
                _headerValue = str2;
            }
        }

        /// <summary>
        /// Invokes the specified context.
        /// </summary>
        /// <param name="context">The context.</param>
        /// <returns></returns>
        public async Task Invoke(HttpContext context)
        {
            context.Response.OnStarting(
                async () =>
                {
                    if (!ContainsCspHeader(context.Response))
                    {
                        await AddCspHeader(context);
                    }
                });

            await _next(context);
        }

        private async Task AddCspHeader(HttpContext context)
        {
            var sendingHeaderContext = new CspSendingHeaderContext(context);
            await _options.OnSendingHeader(sendingHeaderContext);
            if (!sendingHeaderContext.ShouldNotSend)
            {
                string headerName;
                string headerValue;
                if (_options.IsNonceNeeded)
                {
                    var valueTuple = _options.ToString((ICspNonceService)context.RequestServices.GetService(typeof(ICspNonceService)));
                    headerName = valueTuple.Item1;
                    headerValue = valueTuple.Item2;
                }
                else
                {
                    headerName = _headerName;
                    headerValue = _headerValue;
                }

                context.Response.Headers.Add(headerName, headerValue);
            }
        }

        private bool ContainsCspHeader(HttpResponse response)
        {
            return response.Headers.Any(h =>
            {
                if (!h.Key.Equals(CspHeaderName, StringComparison.OrdinalIgnoreCase))
                    return h.Key.Equals(CspReportOnlyHeaderName, StringComparison.OrdinalIgnoreCase);
                return true;
            });
        }
    }

agilenut avatar Apr 11 '19 19:04 agilenut

Couldn't implement this quite yet, the change is incompatible with the current unit tests. Getting a NotImplementedException when calling OnStarting to register the delegate.

juunas11 avatar Apr 16 '19 18:04 juunas11

Understood. Thank you for looking at it.

I took a bit of time today to play around with it. I was able to get something similar to this working in some tests of a similar middleware:

https://stackoverflow.com/questions/49740194/unit-testing-asp-net-core-httpresponse-onstarting

Not sure if you ran across this approach.

agilenut avatar Apr 17 '19 21:04 agilenut

Ooh, mocking the response feature interface with Moq might work :)

juunas11 avatar Apr 23 '19 18:04 juunas11