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

Add Security Headers in Response.OnStarting #39

Open
agilenut opened this issue Apr 11, 2019 · 7 comments
Open

Add Security Headers in Response.OnStarting #39

agilenut opened this issue Apr 11, 2019 · 7 comments

Comments

@agilenut
Copy link

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.

@juunas11
Copy link
Owner

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

@agilenut
Copy link
Author

@juunas11
Copy link
Owner

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

@agilenut
Copy link
Author

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;
            });
        }
    }

@juunas11
Copy link
Owner

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.

@agilenut
Copy link
Author

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.

@juunas11
Copy link
Owner

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

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

No branches or pull requests

2 participants