marklister / scala-totp-auth

scala implementation of RFC 6238; Supports base32 & hex.
29 stars 7 forks source link

Why not using a real 80 bit secret? #2

Closed MStrecke closed 11 years ago

MStrecke commented 11 years ago

When you generate a new random TOTPSecret the first digit you create is a number between 1 and 31, obviously to avoid a leading "Zero" (which would be an "A" in base32).

Is there a compelling reason for this? You loose 2 ^ 75 possible keys.

[snip] -- moved to it's own issue....

marklister commented 11 years ago
   def this(b32Digits: Int, r: Random) =
     this((2 to b32Digits).foldLeft(BigInt(r.nextInt(31)) + 1: BigInt)((a, b) => a * 32 + r.nextInt(32)))
marklister commented 11 years ago

I think that we're losing only one bit of randomness. Eg on a 2 digit secret there are 31_32 possible secrets vs 32_32 in a "pure" solution. My intent was to fulfil this contract:

//Create a new TOTP secret key of b32Digits * 5 bits in length ...

Whilst avoiding this:

 scala> TOTPSecret("AAAA").toBase32
 res6: String = A
MStrecke commented 11 years ago

When using BigInt, you lose the length information of the underlying data. I can see how ensuring that the most significant digit is not zero (or base32 "A") makes things easier.

This all works fine as long as your code is creating the secrets. If someone pulls secrets from a third party database, these secrets may well be start with "A" and your code breaks.

I suggest putting a note to the appropriate TOTPSecret constructor methods (from base32 and fromHex and fromArray) informing about this restriction, because this is very hard to find if you don't know about it.

marklister commented 11 years ago

Good idea. The philosophy of scala-totp-auth is very tied to BigInt and easy conversion between hex decimal and base32.

Thanks for your input.

marklister commented 11 years ago

Comments inline

When using BigInt, you loose the length information of the underlying data.

This duplicates the behaviour of the RI see method hexStr2Bytes in the test (java) source.

I can see how ensuring that the most significant digit is not zero (or base32 "A") makes things easier.

I think it's worth the cost of losing 1/32 of the key space.

This all works fine as long as your code is creating the secrets. If someone pulls secrets from a third party database, these secrets may well be start with "A" and your code >breaks.

The RI would treat 0x0000 as the same as 0x00. By extension "AAAA" b32 should be the same as "A" b32. scala-totp-auth would treat "AB" as the same secret as "B" [base 32]. This is exactly what the RI would do if you supplied a hex secret of (say) "0x0010". I think the current behaviour is acceptable and obeys the "principle of least surprise."

I suggest putting a note to the appropriate TOTPSecret constructor methods (from base32 and fromHex and >fromArray) informing about this restriction, because this is very hard to find if you don't know about it.

Good idea, I'll do that, along with a link to this issue. If anyone has anything to add that might make me change the current behaviour please do so on this issue. But for now I'm closing the issue.