Friday, May 29, 2020

SessionAuthenticationModule::OnAuthenticateRequest is broken

Relying on base class library modules should be safe. Unless it's the SessionAuthenticationModule that misses correct handling of some edge cases. Here's the story.
We use the SessionAuthenticationModule for ages. It's great and reliable in 99.99% of cases. However, there could be a case where there's something wrong with the cookie. The cookie is there but it has been somehow altered in the browser. Either it doesn't contain a valid base64 stream or the stream is valid but there's no XML inside the stream.
The problem is that the SAM first tries to decode the cookie
protected virtual void OnAuthenticateRequest(object sender, EventArgs eventArgs)
{
 HttpApplication httpApplication = (HttpApplication)sender;
 HttpRequest request = HttpContext.Current.Request;
 SessionSecurityToken sessionSecurityToken = null;
 if (!TryReadSessionTokenFromCookie(out sessionSecurityToken) && string.Equals(request.HttpMethod, "GET", StringComparison.OrdinalIgnoreCase))
 {
               ..
Now, guess what happens when TryReadSessionTokenFromCookie throws an exception? Well, the exception is not caught and thus it's escalated to your code. And now
  • what your code does when an exception occurs? You catch it by a top level exception handler and you possibly shows a generic error page.
  • what a user does when they see the error page? They refresh the browser.
  • is the error gone? Nope, the flawed cookie is still there
What you get is a nice stacktrace of the problem
   w System.Convert.FromBase64_Decode(Char* startInputPtr, Int32 inputLength, Byte* startDestPtr, Int32 destLength)
   w System.Convert.FromBase64CharPtr(Char* inputPtr, Int32 inputLength)
   w System.Convert.FromBase64String(String s)
   w System.IdentityModel.Services.ChunkedCookieHandler.ReadInternal(String name, HttpCookieCollection requestCookies)
   w System.IdentityModel.Services.ChunkedCookieHandler.ReadCore(String name, HttpContext context)
   w System.IdentityModel.Services.CookieHandler.Read(String name, HttpContext context)
   w System.IdentityModel.Services.SessionAuthenticationModule.TryReadSessionTokenFromCookie(SessionSecurityToken& sessionToken)
   w System.IdentityModel.Services.SessionAuthenticationModule.OnAuthenticateRequest(Object sender, EventArgs eventArgs)
My first attempt to this was to replace the cookie handler by a custom cookie handler. It's not that obvious how to do that but anyway
public class CustomCookieHandler : System.IdentityModel.Services.CookieHandler
{
    private CookieHandler _handler; // = new ChunkedCookieHandler();

    public CustomCookieHandler(CookieHandler handler)
    {
        if ( handler != null )
        {
            this._handler = handler;

            this.Name       = handler.Name;
            this.Path       = handler.Path;
            this.RequireSsl = handler.RequireSsl;
        }
        else
        {
            throw new ArgumentNullException();
        }
    }

    // public override 

    protected override void DeleteCore(string name, string path, string domain, HttpContext context)
    {
        _handler.Delete(name, path, domain, context);
    }

    protected override byte[] ReadCore(string name, HttpContext context)
    {
        try
        {
            return _handler.Read(name, context);                
        }
        catch
        {
            return null;
        }
    }

    protected override void WriteCore(byte[] value, string name, string path, string domain, DateTime expirationTime, bool secure, bool httpOnly, HttpContext context)
    {
        _handler.Write(value, name, path, domain, expirationTime, secure, httpOnly, context);
    }
}
This could be plugged into the pipeline somewhere early with
FederatedAuthentication.FederationConfigurationCreated +=
        ( s, fede ) =>
        {
            fede.FederationConfiguration.CookieHandler = new CustomCookieHandler(fede.FederationConfiguration.CookieHandler);
        };
Case closed? Not really, looks like the base64 cookie data can be valid but what comes out is not XML and is not parsed! The exception changes to
   w System.Xml.XmlExceptionHelper.ThrowXmlException(XmlDictionaryReader reader, XmlException exception)
   w System.Xml.XmlUTF8TextReader.VerifyNCName(String s)
   w System.Xml.XmlUTF8TextReader.ReadQualifiedName(PrefixHandle prefix, StringHandle localName)
   w System.Xml.XmlUTF8TextReader.ReadStartElement()
   w System.Xml.XmlUTF8TextReader.Read()
   w System.Xml.XmlBaseReader.MoveToContent()
   w System.IdentityModel.Services.SessionAuthenticationModule.GetKeyId(Byte[] sessionCookie)
   w System.IdentityModel.Services.SessionAuthenticationModule.ReadSessionTokenFromCookie(Byte[] sessionCookie)
   w System.IdentityModel.Services.SessionAuthenticationModule.TryReadSessionTokenFromCookie(SessionSecurityToken& sessionToken)
   w System.IdentityModel.Services.SessionAuthenticationModule.OnAuthenticateRequest(Object sender, EventArgs eventArgs)
What this custom cookie handler should do would be then to make sure whatever is read, can be interpreted as XML further down. But this would slow down the custom cookie handler dramatically! Each time a cookie is read, it would be first tried to contain an XML by my custom code and then read as XML again by the existing code.
An ultimate solution? Delete your SAM cookies in the error page. As simple as that. A side effect is users are logged out of the application after an exception is caught by the top level exception handler.

No comments: