richardcocks commented on code in PR #4076:
URL: https://github.com/apache/iggy/pull/4076#discussion_r3945547850
##########
foreign/java/java-sdk/src/main/java/org/apache/iggy/serde/BytesSerializer.java:
##########
@@ -340,7 +339,9 @@ private static long batchChecksum(
*/
private static byte[] encodedMessageId(MessageId id) {
if (id.toBigInteger().signum() == 0) {
- return readAllBytes(new
UuidMessageId(UUID.randomUUID()).toBytes());
+ byte[] minted = new byte[16];
+ ThreadLocalRandom.current().nextBytes(minted);
Review Comment:
Thank you for spotting this, I'll go back to a CSPRNG in Java too.
Unlike in node/v8, I seem to be hitting a performance quirk where asking for
more bytes out of the Java SecureRandom class seems closer to a linear cost, so
pooling isn't helping. I'll investigate this a little more because it doesn't
feel right to me.
What I can do instead is use AES-CTR to generate a collision resistant
stream, as that will be both fast and sufficiently random. This is the
primitive that `SecureRandom` itself uses, but it also does extra re-seeding,
which isn't necessary for collision resistance.
I've checked C# and Go and they both have higher random state in their PRNGs
( 256 bits ), so have collision resistance,
However, with Java now also going back to CSPRNG source, I would understand
a desire to keep the SDKs more consistent, so I can source their message Ids
from their CSPRNGs too if desired. ( This has the benefit of being better for
anyone downstream who might accidentally treat message IDs as being securely
generated. )
--
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]
