marklister / scala-totp-auth

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

toByteArray -- extra byte? #3

Closed marklister closed 11 years ago

marklister commented 11 years ago

In the function toByteArray you write that "there is sometimes an extra zero byte at the start". Are you sure? If the base32 string starts with "AA", the leading byte will of course be a Zero.

Some numbers will on the other hand not generate an Array[Byte] with a size of 10. (16 base32 digits = 80 bit = 10 Bytes)

If the BigInt has a value of 123, the result of toByteArray would be: Array(123). I.e. you would have to add "leading" Zeros to that Array.

If looks like you're restricting the values to a number that is guaranteed to create a Byte-Array with 10 elements.

(reported by MStrecke)

marklister commented 11 years ago
   def toByteArray: Array[Byte] = {
  //Sometimes we have an extra zero byte to start :(
  val b = underlying.toByteArray
  if (b(0) == 0) b.tail else b
}

This code addresses a real problem. If you make the method return underlying.toByteArray then the tests will fail. I'm really not sure what the exact cause is (probably an unwanted sign byte or something).

My goal was to duplicate the behaviour of the RI.

marklister commented 11 years ago

toByteArray is used in generating the hash used to obtain the totp. Dropping the first byte if it's "0" duplicates the RI behaviour:

    static byte[] hexStr2Bytes(String hex){
     // Adding one byte to get the right conversion
     // Values starting with "0" can be converted
     byte[] bArray = new BigInteger("10" + hex,16).toByteArray();

     // Copy all the REAL bytes, not the "first"
     byte[] ret = new byte[bArray.length - 1];
     for (int i = 0; i < ret.length; i++)
         ret[i] = bArray[i+1];
     return ret;
 }
MStrecke commented 11 years ago

Ok, at least I found out when the additional 0 is added.

Like other Integers, BigInt uses the most significant bit as a sign bit.

If the source is unsigned (e.g. hex) and has bit 7 set (e.g. 0x80), it adds the leading 0 to prevent the BigInt to become negative. If the source is a signed value (e.g. -128), the BigInt is also negative and there is no need to do that.

scala> BigInt("7F",16).toByteArray
res5: Array[Byte] = Array(127)
scala> BigInt("80",16).toByteArray
res6: Array[Byte] = Array(0, -128)
scala> BigInt(-128).toByteArray
res7: Array[Byte] = Array(-128)
marklister commented 11 years ago

Good to know! Thanks! I didn't really have the time to research the problem properly when I wrote this code. I think I can close this issue?