search

samdjstevens / java-totp

A java library for implementing Time-based One Time Passwords for Multi-Factor Authentication.
MIT License
448 stars 109 forks source link

Discussion : Hashing algorithm other than SHA-1 #27

Closed OlivierJaquemet closed 4 years ago

OlivierJaquemet commented 4 years ago

Were you able to successfully use hashing algorithm other than SHA-1 with any application ? I could not...

I have tested the following application and I could not make it work :

On Android :

On Windows :

Do you known any application supporting hashing algorithm other than SHA-1 ?

Stexxen commented 4 years ago

I think their are 2 problems here.

App makers are not clear on whether they support SHA-256/512 and I personally I think the getDigitsFromHash function in the DefaultCodeGenerator has an error for SHA-256/512.

I used oathtool to verify the results, and I also couldn't make it work. (SHA-1 matched, SHA-256/512 did not) I eventually replaced it with the below which I took from the similiar aerogear function, this then matched the oathttool for all hash types.

  private String getDigitsFromHash(byte[] hash) {
    // put selected bytes into result int
    int offset = hash[hash.length - 1] & 0xf;

    int binary = ((hash[offset] & 0x7f) << 24) |
                         ((hash[offset + 1] & 0xff) << 16) |
                         ((hash[offset + 2] & 0xff) << 8) |
                         (hash[offset + 3] & 0xff);

    return String.format("%0" + digits + "d", binary % (int) Math.pow(10, digits));
  }

Back to your first question, apps that I have verified working with at SHA-256 after this change are

Android

iOS

And apps that appear to not support SHA-256

And what I found most annoying is that these apps fail silently back to SHA-1, without erroring. So we've had to put a warning on our setup pages to not use them.

samdjstevens commented 4 years ago

Thanks for raising the issue @OlivierJaquemet and for your investigation/fix @Stexxen !

Will try and get some unit tests in place for all the different hash types and make a fix if needed as soon as I can.

Stexxen commented 4 years ago

I have this small test for SHA-256 if its any help? oathtool result

~ % oathtool --totp=sha256 --now="2019-09-04 21:12:16 UTC" --digits=6 --base32 W3C5B3WKR4AUKFVWYU2WNMYB756OAKWY
272978
~ % oathtool --totp=sha256 --now="2019-09-04 21:12:16 UTC" --digits=8 --base32 W3C5B3WKR4AUKFVWYU2WNMYB756OAKWY
76272978

and the Test Case

  @Test
  public void testSHA256Result() {
    // 2019-09-04 21:12:16 UTC
    int time = 1567631536;
    long currentBucket = Math.floorDiv(time, 30);

    CodeGenerator g = new DefaultCodeGenerator(HashingAlgorithm.SHA256);
    String code = g.generate("W3C5B3WKR4AUKFVWYU2WNMYB756OAKWY", currentBucket);
    assertEquals("272978", code);

    g = new DefaultCodeGenerator(HashingAlgorithm.SHA256, 8);
    code = g.generate("W3C5B3WKR4AUKFVWYU2WNMYB756OAKWY", currentBucket);
    assertEquals("76272978", code);
}
OlivierJaquemet commented 4 years ago

Awesome work @Stexxen, thank you !

Thank you @samdjstevens for taking the time to maintain your project. If I can help, do not hesitate to ask.

samdjstevens commented 4 years ago

I've just pushed a fix for this to master with some unit tests that should hopefuly ensure it's working for all hashing algorithms. Please do have a look over if you can, and if all good I will publish v1.7 shortly.

Thanks!

Stexxen commented 4 years ago

Ahhh that's the problem was it. My binary maths was just not up to the challenge of working out what was wrong...

Thanks for fixing 👍

samdjstevens commented 4 years ago

Just released v1.6.1 to maven central with this fix

OlivierJaquemet commented 4 years ago

Great work. Version 1.6.1 tested and approved. :) Other hashing algorithm are working perfectly fine with FreeOTP, andOTP and KeePassXC 👍🏻 Thanks again.