IainHull commented on code in PR #371: URL: https://github.com/apache/incubator-pekko/pull/371#discussion_r1224490114
########## actor/src/main/scala/org/apache/pekko/util/UniqueRandomShortProvider.scala: ########## @@ -0,0 +1,106 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * license agreements; and to You under the Apache License, version 2.0: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * This file is part of the Apache Pekko project, derived from Akka. + */ + +package org.apache.pekko.util + +import org.apache.pekko.annotation.InternalApi + +import java.security.SecureRandom + +/** + * An efficient collision free threadsafe generator of pseudo random + * shorts based on https://en.wikipedia.org/wiki/Double_hashing#Enhanced_double_hashing + * using SecureRandom as a seed. + * + * The avoidance of collisions is achieved by using a Short Array buffer + * that is prefilled with a dimension of 65536 (~ 131072 bytes in size per + * instance of [[UniqueRandomShortProvider]]). This means it will loop + * through all possible 65536 values before starting to re-generating + * the same numbers again. + */ + +@InternalApi +private[pekko] class UniqueRandomShortProvider { + import UniqueRandomShortProvider._ + + @volatile private var index = 0L + @volatile private var increment = 0L + @volatile private var count = 0L + @volatile private var limit = 0 + private val buffer: Array[Short] = new Array[Short](UniqueRandomShortProvider.MaxLimit) + val r = new SecureRandom() + + resetBuffer() + resetSeed() + + count = 1 + + private def resetSeed(): Unit = { + index = r.nextLong + increment = r.nextLong + } + + private def resetBuffer(): Unit = { + System.arraycopy(Filler, 0, buffer, 0, UniqueRandomShortProvider.MaxLimit) Review Comment: Adding collision detection to `DnsClient` seems easier than preventing duplicates when you wrap. Then you can use ` java.util.concurrent.ThreadLocalRandom` to generate the random numbers (like netty does). https://github.com/netty/netty/pull/4455/files To protect DNS poisoning you should also add a check to the response handler in `DnsClient` to ensure the question in the response matches the question asked. Otherwise an attacker can send spurious DNS answers as UDP is connectionless, and as long as they get the transaction id correct `DnsClient` will assume its the ip address for the question it asked. I have a patch to akka 2.6 (based solely on the description of the problem in the CVE and the github issue, I have not looked at the code), but I am waiting for clearance to publish it (hopefully monday or tuesday). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
