6
\$\begingroup\$

HTTP Strict Transport Security (HSTS) is an opt-in security enhancement that is specified by a web application through the use of a special response header. Once a supported browser receives this header that browser will prevent any communications from being sent over HTTP to the specified domain and will instead send all communications over HTTPS.

Currently, there is no built in support for HSTS in MVC. So I decided to create an filter attribute that I can use as part of my application. There are a few questions about the code I have written which are:

  • I have used an action filter for granularity, but would a module/handler been better? If so, why?
  • Should the default of the attribute be to include sub domains, or exclude them (currently it excludes them)
  • The default timeout for the header is 300 seconds. This is a randomly picked arbitrary value. Is this value too much/too little?

The attribute is designed to compliment the RequireHttpsAttribute

    using System;
    using System.Globalization;
    using System.Web.Mvc;

    public class HSTSAttribute : ActionFilterAttribute
    {
            private const string HeaderName = "Strict-Transport-Security";
            private readonly string _headerValue;
            private readonly bool _includeSubDomains;

    public HSTSAttribute(bool includeSubDomains = false) 
        : this(300, includeSubDomains)
    {
    }

    public HSTSAttribute(TimeSpan duration, bool includeSubDomains = false) 
        : this(duration == null ? 300 : duration.Seconds, includeSubDomains)
    {
    }

    public HSTSAttribute(int duration, bool includeSubDomains = false)
    {
        if (duration <= 0)
        {
            throw new ArgumentException();
        }

        var includeSubDomainsSuffix = includeSubDomains ? String.Empty : "; includeSubDomains";
        _headerValue = String.Format("max-age={0}{1}", duration.ToString(CultureInfo.InvariantCulture)
                                , includeSubDomainsSuffix);
    }

    public override void OnResultExecuted(ResultExecutedContext filterContext)
    {
        // HSTS headers should be sent via HTTPS responses only : http://tools.ietf.org/html/draft-ietf-websec-strict-transport-sec-14#section-7.2
        // They should also not be duplicated
        if (filterContext.HttpContext.Request.IsSecureConnection 
                && filterContext.HttpContext.Response.Headers[HeaderName] == null)
        {
            filterContext.HttpContext.Response.AddHeader(HeaderName, _headerValue);
        }

        base.OnResultExecuted(filterContext);
    }                                                                           
}
\$\endgroup\$
3
  • \$\begingroup\$ In production you'll use durations of at least a month. The only reason for small values is that you can recover more quickly when you discover that part of you website doesn't work with HTTPS. \$\endgroup\$ May 2, 2014 at 15:59
  • \$\begingroup\$ Why have defaults at all? These are so important decisions that forcing the user to choose explicitly may be a good idea. \$\endgroup\$ May 2, 2014 at 16:01
  • \$\begingroup\$ You should also use TotalSeconds on the TimeSpan instead of Seconds. \$\endgroup\$
    – Haukman
    Feb 28, 2017 at 1:31

1 Answer 1

2
\$\begingroup\$

I would suggest setting the message on ArgumentException to your parameter name

e.g.

throw new ArgumentException("'duration' must be greater than 0");
\$\endgroup\$
4
  • \$\begingroup\$ can you explain why you would do this please? it makes for a better review. Thank you \$\endgroup\$
    – Malachi
    Apr 29, 2014 at 13:47
  • \$\begingroup\$ it should also be throw new ArgumentException(duration + " must be greater than 0"); duration is a variable and doesn't belong inside of quotes. \$\endgroup\$
    – Malachi
    Apr 29, 2014 at 13:56
  • 1
    \$\begingroup\$ @Malachi that would end up being the contents of the parameter being included in the message, not the name of the parameter. That was included as part of my edit :) \$\endgroup\$ Apr 29, 2014 at 14:51
  • \$\begingroup\$ my bad, you wanted the variable to be written out in the exception, sorry took me a minute to follow that @StuartBlackler \$\endgroup\$
    – Malachi
    Apr 29, 2014 at 15:18

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.