FreeRADIUS / freeradius-server

FreeRADIUS - A multi-protocol policy server.
http://freeradius.org
GNU General Public License v2.0
2.05k stars 1.06k forks source link

Update rlm_totp to follow the standards better #4809

Open alandekok opened 1 year ago

alandekok commented 1 year ago

Message

Implement window and anti-retry of https://datatracker.ietf.org/doc/html/rfc6238#section-5.2

plambrechtsen commented 1 week ago

To align with common implementations and include "nice to haves" :

Secret format: BASE32 or HEX Hashing Algorithm: SHA1 or SHA256 Digit: 6 or 8 Digits Time-step Size: 30 or 60 Seconds Token specific Hardware Time Skew in seconds.

Better examples

Include links or at least references to 3rd party sites that can generate TOTP tokens which is very helpful for debugging. In 3.2 there is a sites-available example and master doesn't seem to include a totp example with just a numeric example:

And that site example doesn't work as it should be an update control:

    if (&User-Name == "bob") {
        update control {
            TOTP-Secret := JBSWY3DPEHPK3PXP
        }
    }

And there are sites you can pass an example key to such as:

https://totp.danhersam.com/?key=JBSWY3DPEHPK3PXP https://www.token2.com/site/page/totp-toolset?key=BBSWY3DPEHPK3PXPJBSWY3DPEHPK3PXP

I think changing the example to be an uppercase alphanumeric BASE32 string similar to the above examples makes it clear how they should look rather than a pure numeric value in the sites available example.

Include debug steps

I found this post where "-Xx" includes extra debugging to include what the expected code was and what was received. That was super handy for debugging so if the expected codes were was included in -X, or at least documented in the sites-available example that -Xx is the best way to debug would be very handy to know why the module was failing showing the expected token codes in debug.

Secret format reasoning

It's standard to have a seed or secret in BASE32 format. But I have seen situations for hardware tokens where a HEX format is supplied. Token2 has an excellent TOTP config site allowing conversion between BASE32 <-> HEX formatted seeds. https://www.token2.com/site/page/totp-toolset

Hashing Algorithm

SHA1 is by far and above the standard here, but some security folks for whatever reason prefer SHA256 so if that could be adjustable per token that would make sense.

Digit length

The standard is 6 digits, and 8 is supported but shouldn't be the default in any examples so thank you for updating 3.2 to align to master.

Time step size

30 Seconds is also the default, but I have deployed hardware tokens with 60 seconds time step as end users can have a challenge getting the 6 digit code within 30 seconds so my personal view is 60 seconds is better for hardware tokens so having this option adjustable per user via the update control TOTP-Time-Step = 60 or similar would be super helpful.

Skew support

This is already there with lookback_steps and lookforward_steps. I'm not sure why lookback_interval is in there as the time_step already sets the time step size and time_step plus lookback_steps or lookforward_steps is all the calculation that should be required.

Token specific Hardware Time Skew in seconds

This is also quite an edge case but I have seen on hardware tokens the UTC Clock that was set when the token was manufactured is way off UTC time, or it can skew over time. So one hardware token may be 180 seconds behind UTC so you would need a lookback_steps of 8 to have it within the window. And others are forward 40 or 50 seconds. Ideally keeping the lookback_steps and lookforward_steps to 1 or 2 is ideal, and that is a module setting where as the Hardware skew would be a set in the token database per hardware token setting that should be passed in with the secret to adjust the per device calculation rather than something on the module that you need to set a very large lookback_steps and lookforward_steps value to get them to work as doing that would create a situation where 10 different token codes were valid and the possibility for token re-use which is bad.