kryptco / krypton-android

DEPRECATED Krypton turns your Android device into a U2F Authenticator: strong, unphishable 2FA.
https://krypt.co
Other
203 stars 50 forks source link

Don't allow DIGEST_NONE #44

Closed divegeek closed 7 years ago

divegeek commented 7 years ago

Allowing use of raw RSA keys is not safe. Signing chosen plaintexts can reveal the bits of the private key. The PKCS#1 v1.5 padding that you require limits this attack, but only if used with a good digest algorithm (the SHA-256 and SHA-512 algorithms you allow are great; even SHA-1 is technically okay in this context, though deprecated).

If your code always does the digesting prior to signing then DIGEST_NONE makes sense... but it's still risky because you do not get the assurance provided by AndroidKeyStore that the key can never be used without digesting. An attacker who somehow gains unlimited use of the key (e.g. compromises the Kryptonite app) can extract the private key. Of course, without DIGEST_NONE, such an attacker can sign anything he likes but only as long as he retains access to the phone; he can't clone the private key, so removing DIGEST_NONE is an increase in security.

kcking commented 7 years ago

Thanks for the issue!

Yes we always digest on the phone so an adversary would need code execution on the phone to exploit this. Just pushed a fix: https://github.com/KryptCo/kryptonite-android/commit/01d97b9011f9a92335b3a7a747e7cd33724a2959

divegeek commented 7 years ago

Good. With that fix you not only digest on the phone, you digest in the TEE... and most importantly you prevent an attacker with code execution on the phone from extracting the private key.