Fix vulnerability in AESCounterBuiltinRNG

* The problem is that that internal counter isn’t incremented before cipher.doFinal() is used
* AES/CTR has a counter internally and cipher.update() should be used
* Another issue is that AES128CounterSecureRNG is initially seeded with a 16 bytes seed/key, but once it
  hits ReseedingThreshold it reseeds itself with a 32 bytes seed, effectively becoming AES256. This will
  crash if strong encryption is disabled.
This commit is contained in:
Rafał Sumisławski 2018-08-28 09:34:47 +02:00 committed by Patrik Nordwall
parent f0db59816a
commit 56498e7e58

View file

@ -41,7 +41,6 @@ private[akka] class AESCounterBuiltinRNG(val seed: Array[Byte], implicit val exe
private val entropySource = new SecureRandom private val entropySource = new SecureRandom
// mutable state below, concurrent accesses need synchronized or lock // mutable state below, concurrent accesses need synchronized or lock
private val counter: Array[Byte] = Array.fill[Byte](CounterSizeBytes)(0)
private var index: Int = 0 private var index: Int = 0
private var currentBlock: Array[Byte] = null private var currentBlock: Array[Byte] = null
private var reseedFuture: Future[Array[Byte]] = null private var reseedFuture: Future[Array[Byte]] = null
@ -52,29 +51,31 @@ private[akka] class AESCounterBuiltinRNG(val seed: Array[Byte], implicit val exe
// this algorithm can be further improved by better selection of the iv // this algorithm can be further improved by better selection of the iv
// here and at re-seeding time further below // here and at re-seeding time further below
private val ivArr = Array.fill[Byte](CounterSizeBytes)(0) private val ivArr = Array.fill[Byte](CounterSizeBytes)(0)
ivArr(0) = (ivArr(0) + 1.toByte).toByte ivArr(0) = (ivArr(0) + 1).toByte
private val ivSpec = new IvParameterSpec(ivArr) private val ivSpec = new IvParameterSpec(ivArr)
cipher.init(Cipher.ENCRYPT_MODE, new this.AESKey(seed), ivSpec) cipher.init(Cipher.ENCRYPT_MODE, new this.AESKey(seed), ivSpec)
private val zeros: Array[Byte] = Array.fill[Byte](CounterSizeBytes)(0)
@Override @Override
override protected def next(bits: Int): Int = synchronized { override protected def next(bits: Int): Int = synchronized {
// random result generation phase - if there is not enough bits in counter variable // random result generation phase - if there is not enough bits in the currentBlock
// we generate some more with AES/CTR // we generate some more with AES/CTR
bitsSinceSeeding += bits bitsSinceSeeding += bits
if (currentBlock == null || currentBlock.length - index < 4) { if (currentBlock == null || currentBlock.length - index < 4) {
try { try {
currentBlock = cipher.doFinal(counter) currentBlock = cipher.update(zeros)
index = 0 index = 0
} catch { } catch {
case ex: Exception case ex: Exception
// Generally Cipher.doFinal() from nextBlock may throw various exceptions. // Generally Cipher.update() from nextBlock may throw various exceptions.
// However this should never happen. If initialisation succeeds without exceptions // However this should never happen. If initialisation succeeds without exceptions
// we should be able to proceed indefinitely without exceptions. // we should be able to proceed indefinitely without exceptions.
throw new IllegalStateException("Failed creating next random block.", ex) throw new IllegalStateException("Failed creating next random block.", ex)
} }
} }
// now, enough bits in counter, generate pseudo-random result // now, enough bits in currentBlock, generate pseudo-random result
val result = (BitwiseByteToInt & currentBlock(index + 3)) | val result = (BitwiseByteToInt & currentBlock(index + 3)) |
((BitwiseByteToInt & currentBlock(index + 2)) << 8) | ((BitwiseByteToInt & currentBlock(index + 2)) << 8) |
((BitwiseByteToInt & currentBlock(index + 1)) << 16) | ((BitwiseByteToInt & currentBlock(index + 1)) << 16) |
@ -86,7 +87,7 @@ private[akka] class AESCounterBuiltinRNG(val seed: Array[Byte], implicit val exe
if (bitsSinceSeeding > reseedingThreshold) { if (bitsSinceSeeding > reseedingThreshold) {
if (reseedFuture == null) { if (reseedFuture == null) {
// ask for a seed and process async on a separate thread using AESCounterBuiltinRNGReSeeder threadpool // ask for a seed and process async on a separate thread using AESCounterBuiltinRNGReSeeder threadpool
reseedFuture = Future { entropySource.generateSeed(32) } reseedFuture = Future { entropySource.generateSeed(seed.length) }
} }
// check if reseedingDeadline is exceeded - in that case we cannot proceed, as that would be insecure // check if reseedingDeadline is exceeded - in that case we cannot proceed, as that would be insecure
// we need to block on the future to wait for entropy // we need to block on the future to wait for entropy