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

TOTP: custom settings for SHA-1/SHA-256/SHA-512 #873

Closed
marcaurele opened this issue Aug 16, 2017 · 18 comments · Fixed by #2972
Closed

TOTP: custom settings for SHA-1/SHA-256/SHA-512 #873

marcaurele opened this issue Aug 16, 2017 · 18 comments · Fixed by #2972
Milestone

Comments

@marcaurele
Copy link

Would it be possible to add an extra customization option for TOTP to choose the hash method based to generate the token:

  • SHA-1 (default)
  • SHA-256
  • SHA-512

Those extra hash functions are described in the RFC for TOTP: https://tools.ietf.org/html/rfc6238

@ccoenen
Copy link

ccoenen commented Aug 16, 2017

Are they being used anywhere?

@weslly
Copy link
Contributor

weslly commented Aug 16, 2017

This one is very unlikely to be implemented. I never found any website/service actually using this parameter and even Google Authenticator lacks support for it.

I'm closing this, but I will reopen if anyone finds an example of a website that requires this parameter.

@marcaurele
Copy link
Author

www.kraken.com does let you choose for example (not very explicit in their support doc: https://support.kraken.com/hc/en-us/articles/203395513-How-do-I-set-up-two-factor-authentication-)

@ccoenen
Copy link

ccoenen commented Aug 16, 2017

In case you're afraid of SHA-1 collisions, this will really not be an issue in TOTP. For one thing, the token is usually a second factor after a password (so someone would have to have both), and furthermore the input is mutated by the current time, so any collision that would ever have occurred would only ever have been valid for 30 seconds and the changes of another collision are astronomically small.

So, in contrast to TLS-certificates (where SHA-1 is discouraged and should be replaced as soon as possible), TOTP does not face the same threats.

@phoerious
Copy link
Member

phoerious commented Aug 16, 2017

The same is true for HMAC-SHA1 and HOTP BTW. It's not time-based, but collisions aren't an immediate thread. Bruteforcing the secret is usually a better way to attack them.

@weslly
Copy link
Contributor

weslly commented Aug 16, 2017

Quoting this StackExchange answer:

The implementation calls for the result of the HMAC to be truncated and the initial HMAC uses a very short secret: using a different hash algorithm here wouldn't improve security but it would make it incompatible with existing applications and tokens.

@marcaurele
Copy link
Author

I'm not afraid of the collision at all. I was just trying to use another hashing function and see if tools were compatible. Some are, some don't. I don't think it's a huge change neither to propose such option in the customization settings. Saying that you're not going to implement it because Google Authenticator lacks support of it is sad from a developer point of view.

@phanimahesh
Copy link

It can be interpreted as 'We won't implement it because one of the most popular TOTP apps doesn't, and that indicates that it isn't widespread in its use, and so we won't spend time on it.' And that is a reasonable argument IMO.

I suspect a PR would be welcome if it doesn't look like it'll add to maintenance burden ( code fits with rest of project's style and philosophy and is relatively self contained, etc. )

@weslly
Copy link
Contributor

weslly commented Aug 16, 2017

Saying that you're not going to implement it because Google Authenticator lacks support of it is sad from a developer point of view.

It's not because Google Authenticator lacks support for this, we added support for a couple of parameters that it doesn't support either (digits and period), but these are actually used in a few situations.

The algorithm parameter, however, isn't adopted nor required anywhere (that I'm aware of) and doesn't provide any visible improvement on security, so it wouldn't make a lot of sense to implement that.

I suspect having this parameter on the interface would only make the TOTP setup dialog more confusing for some users.

@droidmonkey
Copy link
Member

droidmonkey commented Aug 16, 2017

Precisely what weslly said. We are very cognizant of scope creep, especially in an open source project. Even if a PR was fashioned, the likely complication on the gui side would make the program less user friendly for zero gain.

@octiron
Copy link

octiron commented Nov 9, 2018

I've discovered that Red Hat defaults to using sha256 (because sha1 is deprecated?), and they maintain the FreeOTP android play store app to support that.

@woodychuck
Copy link

woodychuck commented Apr 5, 2019

For what it's worth, Sophos UTM Firewalls are using SHA-256 in their TOTP implementation, so Google Authenticator doesn't work with them (neither does KeepassXC, nor KeeOTP in the last official release 1.3.9, though beta 1.3.12 supports both SHA-256 and SHA-512 with a simple 3 radio button GUI). Consequently Sophos provide their own App which otherwise is very similar to Google's.

So while it is true that SHA-1 is most widespread, there are existing applications/services using SHA-2 instead, which might not add any substantial security but leave users stranded in terms of Keepass integration, so maybe it's at least worth reconsidering.

@BryanJacobs
Copy link
Contributor

Australia's "MyGov" system used for accessing government services such as the tax office uses TOTP with a SHA-512 hash. The current behavior of keepassxc is that it generates an incorrect TOTP (it uses SHA-1, and ignores the "otpHashMode" parameter in the OTP URL). KeePass 2 generates the correct code.

If what you're worried about is UI complexity, there's no need to put it in the UI, but it'd be nice to have some way to get the program to generate correct codes for the sites that need them. I think it's worth pointing at Steam code integration in the TOTP generator as a comparable feature - one, LITERALLY one, site uses them - and that appears to have been accepted without issue.

@ccoenen
Copy link

ccoenen commented Apr 8, 2019

grafik

Well, having this setting could be 30;6;SHA-512 in TOTP Settings. Paired with some documentation this would make it possible and would not require going through this in every single case while setting up the mostly non-512-entries.

Given that we now have actual use cases (RedHat, Sophos, MyGov) for this, maybe the decision to close the issue should be revisited? After all, there's a special case for Steam and that's not even easy to set up.

@droidmonkey
Copy link
Member

IMO this was a big misstep on the TOTP RFC to allow multiple hash algorithms. The permutation of options is ridiculous for something that is supposed to be quick and easy. The Steam integration was resisted until someone built a PR to integrate it. Even then it required me to spend several hours refactoring the whole TOTP mess to make it more effective. Please don't down play the significance of adding features.

@droidmonkey droidmonkey reopened this Apr 8, 2019
@droidmonkey droidmonkey added this to the v2.5.0 milestone Apr 8, 2019
@octiron
Copy link

octiron commented Apr 8, 2019

For reference, the keycloak documentation: https://www.keycloak.org/docs/3.3/server_admin/topics/authentication/otp-policies.html

For my.gov.au, they are following DTA advice: https://www.dta.gov.au/our-projects/digital-identity/join-identity-federation/accreditation-and-onboarding/trusted-digital-identity-framework

The DTA advice is straight from NIST https://pages.nist.gov/800-63-3/sp800-63b.html

The NIST guidance is to use TOTP codes of at least 6 digits, valid for no longer than two minutes, and accepted only once. For hashes, they want at least 112 bits, i.e. half of 224. For SHA-1 this is now much less than 80 bits. It is a bit vague if this recommendation applies to TOTP. A number of Australian Govt guidance documents (e.g. https://acsc.gov.au/publications/ism/ISM_22_Guidelines_for_Using_Cryptography.pdf page 7) say 'must not' use SHA-1 in a general sense, and you can't expect policy people with no crypto understanding to make a distinction.

https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-107r1.pdf

I guess what I'm getting at is that SHA-256 is just going to get more common.

@droidmonkey
Copy link
Member

This is all great information, but the hash in totp is simply a means to an end. It has zero relevance to using a hash for a signature or similar purpose. It was simply inappropriate for the RFC to be such an open door.

BryanJacobs added a commit to BryanJacobs/keepassxc that referenced this issue Apr 10, 2019
…ot#1566

This implements support for SHA-256 and SHA-512 hash algorithms when
generating TOTP codes. These algorithms are specified by RFC6238. The
implementation is compatible with Google's OTP URL format, as well as
with the KeeOTP plugin for KeePass.

The implementation is not wired into the GUI, as the main project
developer expressed strong negative sentiment about adding more
options there. It is possible to configure codes by putting the
appropriate string into the entry's otp property, or using another
program with a less opinionated UI and a compatible on-disk format.
droidmonkey pushed a commit that referenced this issue Apr 15, 2019
This implements support for SHA-256 and SHA-512 hash algorithms when
generating TOTP codes. These algorithms are specified by RFC6238. The
implementation is compatible with Google's OTP URL format, as well as
with the KeeOTP plugin for KeePass.

The implementation is not wired into the GUI, as the main project
developer expressed strong negative sentiment about adding more
options there. It is possible to configure codes by putting the
appropriate string into the entry's otp property, or using another
program with a less opinionated UI and a compatible on-disk format.
@pepa65
Copy link

pepa65 commented Mar 2, 2024

A 6-digit TOTP is huge on collisions of course, there is a 1 in a million chance to guess it right, it's a less than 20-bit space. SHA1 or SHA2 is just not relevant when simplifying towards a 6 (or even 8) digit TOTP. Just agreeing with @droidmonkey here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants