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

Update rlm_totp to follow the standards better #4809

Open
alandekok opened this issue Nov 21, 2022 · 1 comment
Open

Update rlm_totp to follow the standards better #4809

alandekok opened this issue Nov 21, 2022 · 1 comment
Labels
feature enhancement category: a new feature (an extension of functionality) v3.2.x meta: relates to the v3.2.x branch v4.0.x meta: relates to the v4.0.x branch

Comments

@alandekok
Copy link
Member

Message

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

  • allow for a configurable window (if current OTP doesn't match, try +/- window)
  • allow for tracking which tokens have already been used. This likely requires adding attributes such as "old-token", and asking the user to save them somewhere, probably in the cache module
@alandekok alandekok added feature enhancement category: a new feature (an extension of functionality) v4.0.x meta: relates to the v4.0.x branch close state: auto close the issue v3.2.x meta: relates to the v3.2.x branch and removed close state: auto close the issue labels Nov 21, 2022
@plambrechtsen
Copy link
Contributor

plambrechtsen commented Jun 19, 2024

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

  • Low priority requirement in my view

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.

  • Also low priority requirement

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature enhancement category: a new feature (an extension of functionality) v3.2.x meta: relates to the v3.2.x branch v4.0.x meta: relates to the v4.0.x branch
Projects
None yet
Development

No branches or pull requests

2 participants