Skip to content
This repository has been archived by the owner on Jul 8, 2022. It is now read-only.

AES should enforce minimal length of IV #699

Closed
benkuly opened this issue May 29, 2022 · 5 comments
Closed

AES should enforce minimal length of IV #699

benkuly opened this issue May 29, 2022 · 5 comments
Labels
check enhancement New feature or request good first issue Good for newcomers

Comments

@benkuly
Copy link

benkuly commented May 29, 2022

The following does fail with native JVM or JS implementations:

AES.decryptAesCtr(
            Random.Default.nextBytes(ByteArray(1)),
            Random.Default.nextBytes(ByteArray(128 / 8)),
            ByteArray(0)
)

On JVM it throws: java.security.InvalidAlgorithmParameterException: Wrong IV length: must be 16 bytes long

@soywiz
Copy link
Contributor

soywiz commented May 30, 2022

Can you create an assertEquals based on JS and JVM implementation with the expected behavior to make it easier to create a fix?

@benkuly
Copy link
Author

benkuly commented May 30, 2022

Because JVM and JS throw Exception, there is nothing to compare with assertations.

@Test
fun decryptNoPaddingJVM(){
   try {
    val content = Random.Default.nextBytes(ByteArray(1))
    val key = Random.Default.nextBytes(ByteArray(128 / 8))
    val iv = ByteArray(0)

    val cipher = Cipher.getInstance("AES/CTR/NoPadding")
    val keySpec: Key = SecretKeySpec(key, "AES")
    val ivSpec = IvParameterSpec(iv)
    cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec)
    cipher.doFinal(content)

    fail("should throw exception")
   } catch(exception: Exception){
       
   }
}

JS is a bit more complicated, because it needs externals.

@soywiz
Copy link
Contributor

soywiz commented May 30, 2022

Oh, you mean it should throw if IV length is less than 16, and currently it doesn't?

So (assertFailsWith):

assertFailsWith<IllegalStateException> {
  val anyData = Random.Default.nextBytes(ByteArray(1))
  val anyKey = Random.Default.nextBytes(ByteArray(128 / 8))
  val invalidIV = ByteArray(15) // This should be at least 16 bytes long
  AES.decryptAesCtr(
    data = anyData,
    key = anykey,
    iv = invalidIV,
    padding = NoPadding
  )
}

Is, that the behaviour you would expect? Is this then a minor validation issue?

@soywiz soywiz added enhancement New feature or request question Further information is requested check labels May 30, 2022
@benkuly
Copy link
Author

benkuly commented May 30, 2022

Exactly :)

@soywiz soywiz added good first issue Good for newcomers and removed question Further information is requested labels May 30, 2022
soywiz added a commit that referenced this issue Jun 18, 2022
@soywiz soywiz closed this as completed in 302a6e8 Jun 18, 2022
@benkuly
Copy link
Author

benkuly commented Jun 19, 2022

Thank you :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
check enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants