ctubbsii commented on a change in pull request #2339:
URL: https://github.com/apache/accumulo/pull/2339#discussion_r741341286
##########
File path:
core/src/main/java/org/apache/accumulo/core/spi/crypto/AESCryptoService.java
##########
@@ -315,8 +313,8 @@ public FileDecrypter getDecrypter(Key fek) {
private final byte[] initVector = new byte[GCM_IV_LENGTH_IN_BYTES];
AESGCMFileEncrypter() {
- this.fek = generateKey(sr, KEY_LENGTH_IN_BYTES);
- sr.nextBytes(this.initVector);
+ this.fek = generateKey(random, KEY_LENGTH_IN_BYTES);
+ random.nextBytes(this.initVector);
Review comment:
I provided an explanation in the commit message / description above. See
"Remove use of explicit SHA1PRNG implementation of SecureRandom". SHA1PRNG is,
by far, a weaker implementation than whatever /dev/urandom (the default) would
give, and specifying the SUN implementation explicitly bypasses user's ability
to configure their own Java security providers. In future, the SUN
implementation may not even be available. If a specific provider/implementation
is desired, it should be configured in the Java security config files, not
hard-coded here, bypassing the secure settings set by a system administrator.
##########
File path:
core/src/main/java/org/apache/accumulo/core/spi/fs/RandomVolumeChooser.java
##########
@@ -19,14 +19,13 @@
package org.apache.accumulo.core.spi.fs;
import java.security.SecureRandom;
-import java.util.Random;
import java.util.Set;
/**
* @since 2.1.0
*/
public class RandomVolumeChooser implements VolumeChooser {
- protected final Random random = new SecureRandom();
Review comment:
I thought about this, but decided against exposing it. There's no reason
a subclass needs to rely on this specific instance of the Random. And, exposing
the field to sub-classes really restricts our ability to evolve implementation
(fields are harder to evolve than methods). If it becomes necessary/useful to
expose this, it can be done in a getter method that can be overridden. But, it
doesn't seem to be necessary at this time.
--
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]
