search

rogerta / secrets-for-android

Securely store and manage passwords and secrets on your Android phone.
70 stars 36 forks source link

Why is the PBDKeySpec created with a length of 32 instead of 256? #103

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
What steps will reproduce the problem?
1. Refer to SecurityUtils.java which contains this line:
2. PBEKeySpec keyspec = new PBEKeySpec(password.toCharArray(),
                                          salt,
                                          KEY_ITERATION_COUNT,
                                          32);

3. Should not the key length be 256 bits (not 32)?

What is the expected output? What do you see instead?

What version of the product are you using? On what operating system?

Please provide any additional information below.

Original issue reported on code.google.com by RJKova...@gmail.com on 26 May 2011 at 2:18

GoogleCodeExporter commented 9 years ago

Original comment by roge...@google.com on 27 May 2011 at 2:50

GoogleCodeExporter commented 9 years ago 
After close investigation of the code and testing, this is a bug in the code, 
however it luckily has no ill effect.

The key length argument of the PBEKeySpec constructor is supposed to be 
specified in bits, as the OP says, whereas it is currently specified in bytes.  
However, the documentation for PBEKeySpec, 
"http://download.oracle.com/javase/7/docs/api/javax/crypto/spec/PBEKeySpec.html#
getKeyLength()" , says the following about the key length property:

   Note: this is used to indicate the preference on key length for
   variable-key-size ciphers. The actual key size depends on each
   provider's implementation.

Because the algorithm used to create the ciphers in Secrets 
("PBEWITHSHA-256AND256BITAES-CBC-BC") does not result in variable-key-size 
ciphers, this argument is in fact ignored.

The ciphers created do use 256-bit keys.  I confirmed that secrets encrypted 
with version 1.9.8 (current version as of this writing) can be loaded when the 
key length argument is set to 256 or left out completely.  Also, I was able to 
verify that the keySize field of the key variable is 256.

The best fix would be to remove the key length argument completely, since the 
ciphers used are not variable-key-size ciphers.  However, since I am already at 
work to change the KDF to use bcrypt (see bcrypt branch at 
http://code.google.com/p/secrets-for-android/source/browse/#svn%2Fbranches%2Fbcr
ypt) there is no reason to change the existing code.

Original comment by roge...@google.com on 28 May 2011 at 3:23