apereo / dotnet-cas-client

Apereo .NET CAS Client
Apache License 2.0
232 stars 176 forks source link

Add Documentation For SAML Ticket Validator Time Tolerance #51

Closed TheHokieCoder closed 7 years ago

TheHokieCoder commented 7 years ago

Myself and others have been bitten by an issue (not a bug with the client) where the time clock difference between a client and server becomes greater than the 1 second default value for the SAML ticket validator (configured via the ticketTimeTolerance attribute of the casClientConfig configuration section). It has been argued that 1 second is not a realistic value, with which I agree. I'd like to help add documentation to the project that states if someone uses the SAML ticket validator they should also set a more realistic value of ticketTimeTolerance based on their preferred balance of security and convenience.

Does anyone have any suggestion for a more realistic value for the allowable time drift? As a reference, the Microsoft recommendation for maximum time drift for Kerberos tickets in a domain is a whopping 5 minutes (see Maximum tolerance for computer clock synchronization). I have heard a suggestion of somewhere around 15-30s before, and I personally think 15s is a perfectly reasonable window that doesn't allow too much time for replay attacks and doesn't allow for too much time drift between the client and server.

Another thing that would be helpful is to add the attribute to the boilerplate casClientConfig that gets injected to web.config when the NuGet package is installed in an application so that consumers of the package are more aware of its existence.

serac commented 7 years ago

I have a fair bit of experience supporting the SAML 1.1 validator, and I can say from experience that 1s is simply more trouble than benefit. I would be wholly in favor of bumping the default to 30s; higher than that would require further analysis and consideration. As for documentation, I also agree that the description of the parameter at https://github.com/apereo/dotnet-cas-client needs to be updated to reflect real-world application. I can't tell you how many times I've seen windows systems presumably configured for NTP simply stop working because the system drifted out of the default 1s window. Clearly the default needs to be increased.

serac commented 7 years ago

The patch to address both the docs and the default is trivial. I'll submit a PR shortly.