lazysodium-android icon indicating copy to clipboard operation
lazysodium-android copied to clipboard

LazySodiumAndroid.cryptoKdfDeriveFromKey() throws an exception if key length exceeds 32 bytes

Open 0x4f53 opened this issue 1 year ago • 1 comments

We are trying to use the cryptoKdfDeriveFromKey(subkey: ByteArray?, subkeyLen: Int, subkeyId: Long, context: ByteArray?, masterKey: ByteArray?): Int function on Android to generate keys. For the masterKey parameter, we supply a 64B ByteArray seed output from a bip39 library we use, but there seems to be a 32B check of some kind. We were thrown the following exception:

 E  FATAL EXCEPTION: main
                                                                                                    Process: cloud.keyspace.android, PID: 6710
                                                                                                    java.lang.IllegalArgumentException: Master key length is wrong: 64
                                                                                                    	at com.goterl.lazysodium.LazySodium.cryptoKdfDeriveFromKey(LazySodium.java:266)
                                                                                                    	at cloud.keyspace.android.DeveloperOptions.onCreate$lambda-3(DeveloperOptions.kt:138)
                                                                                                    	at cloud.keyspace.android.DeveloperOptions.$r8$lambda$0j6JCNDxJawYhzlPWAHhcVQ--qI(Unknown Source:0)
                                                                                                    	at cloud.keyspace.android.DeveloperOptions$$ExternalSyntheticLambda2.onClick(Unknown Source:11)
                                                                                                    	at android.view.View.performClick(View.java:7506)
                                                                                                    	at com.google.android.material.button.MaterialButton.performClick(MaterialButton.java:1219)
                                                                                                    	at android.view.View.performClickInternal(View.java:7483)
                                                                                                    	at android.view.View.-$$Nest$mperformClickInternal(Unknown Source:0)
                                                                                                    	at android.view.View$PerformClick.run(View.java:29335)
                                                                                                    	at android.os.Handler.handleCallback(Handler.java:942)
                                                                                                    	at android.os.Handler.dispatchMessage(Handler.java:99)
                                                                                                    	at android.os.Looper.loopOnce(Looper.java:201)
                                                                                                    	at android.os.Looper.loop(Looper.java:288)
                                                                                                    	at android.app.ActivityThread.main(ActivityThread.java:7898)
                                                                                                    	at java.lang.reflect.Method.invoke(Native Method)
                                                                                                    	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
                                                                                                    	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:936)

Upon further investigation, the root cause of the issue seems to be at LazySodium.java#258, where the function appears to have a length check for the masterKey ByteArray:

@Override
    public int cryptoKdfDeriveFromKey(byte[] subKey, int subKeyLen, long subKeyId, byte[] context, byte[] masterKey) {
        if (subKeyLen < KeyDerivation.BYTES_MIN || KeyDerivation.BYTES_MAX < subKeyLen) {
            throw new IllegalArgumentException("Sub Key Length is out of bounds: " + subKeyLen);
        }
        if (subKey.length < subKeyLen) {
            throw new IllegalArgumentException("Sub Key array is less than specified size");
        }
        if (masterKey.length != KeyDerivation.MASTER_KEY_BYTES) {
            throw new IllegalArgumentException("Master key length is wrong: " + masterKey.length);
        }
        if (context.length != KeyDerivation.CONTEXT_BYTES) {
            throw new IllegalArgumentException("Context length is wrong: " + context.length);
        }
        return getSodium().crypto_kdf_derive_from_key(subKey, subKeyLen, subKeyId, context, masterKey);
    }

The JavaScript implementation we use (called libsodium.js), doesn't seem to have it either.

To solve our issue for now, we decided to override the function and remove the line ourselves, as such:

class CustomSodium(sodium: SodiumAndroid, charset: Charset) : LazySodiumAndroid (sodium, charset) {
                override fun cryptoKdfDeriveFromKey(subkey: ByteArray?, subkeyLen: Int, subkeyId: Long, context: ByteArray?, masterKey: ByteArray?): Int {
                    require(!(subkeyLen < KeyDerivation.BYTES_MIN || KeyDerivation.BYTES_MAX < subkeyLen)) { "Sub Key Length is out of bounds: $subkeyLen" }
                    require(subkey!!.size >= subkeyLen) { "Sub Key array is less than specified size" }
                    require(context!!.size == KeyDerivation.CONTEXT_BYTES) { "Context length is wrong: " + context.size }
                    return sodium.crypto_kdf_derive_from_key(subkey, subkeyLen, subkeyId, context, masterKey)
                }
            }

            val sodium = CustomSodium(SodiumAndroid(), StandardCharsets.UTF_8)

            val masterKey = bip39.seed!!
            val hardcodedContext = "KEYSPACE".toByteArray(StandardCharsets.UTF_8)
            val outputKeySize = KeyDerivation.MASTER_KEY_BYTES
            val outputKey = ByteArray(outputKeySize)

            val kdfStatusCode = sodium.cryptoKdfDeriveFromKey(outputKey, outputKeySize, 1, hardcodedContext, masterKey)
           ...

This has us asking the following questions:

  1. Why is there a check in the first place?
  2. What are the consequences of removing the check via an override (exceptions? performance?).

0x4f53 avatar Nov 08 '22 10:11 0x4f53