Skip to content
Permalink
Browse files
[JENKINS-20108]
Prefer SHA1PRNG, which avoids VM-wide global lock on Unix.

Another way to fix this is to avoid repeated calls to "rnd.nextInt()" in
TransportConnection.sendMessage() and instead get all the padding in one call.

That would require another byte array as there's no SecureRandom.nextBytes(byte[],int,int),
but that'd be a monitor cost in the face of VM-wide lock across all the SecureRandom instances.

The amount of padding is fairly small, so yet another way to fix this is to buffer random bytes
by hitting SecureRandom in a bigger block (thus infrequentl) and save the rest at Connection.
  • Loading branch information
kohsuke committed Oct 12, 2014
1 parent 93e10b1 commit 961adfb2762d84e952c3b49126adaa5fa2c4933e
Showing with 35 additions and 2 deletions.
  1. +2 −1 src/com/trilead/ssh2/Connection.java
  2. +2 −1 src/com/trilead/ssh2/KnownHosts.java
  3. +31 −0 src/com/trilead/ssh2/RandomFactory.java
@@ -22,6 +22,7 @@
import java.io.InputStream;
import java.net.InetSocketAddress;
import java.net.SocketTimeoutException;
import java.security.NoSuchAlgorithmException;
import java.security.SecureRandom;
import java.util.Vector;

@@ -1105,7 +1106,7 @@ public synchronized boolean isAuthMethodAvailable(String user, String method) th
private final SecureRandom getOrCreateSecureRND()
{
if (generator == null)
generator = new SecureRandom();
generator = RandomFactory.create();

return generator;
}
@@ -52,6 +52,7 @@
public static final int HOSTKEY_IS_OK = 0;
public static final int HOSTKEY_IS_NEW = 1;
public static final int HOSTKEY_HAS_CHANGED = 2;
private static final SecureRandom SECURE_RANDOM = RandomFactory.create();

private class KnownHostsEntry
{
@@ -151,7 +152,7 @@ public static final String createHashedHostname(String hostname)

byte[] salt = new byte[sha1.getDigestLength()];

new SecureRandom().nextBytes(salt);
SECURE_RANDOM.nextBytes(salt);

byte[] hash = hmacSha1Hash(salt, hostname);

@@ -0,0 +1,31 @@
package com.trilead.ssh2;

import java.security.NoSuchAlgorithmException;
import java.security.SecureRandom;

/**
* Creates {@link SecureRandom}
*
* @author Kohsuke Kawaguchi
*/
class RandomFactory {
static SecureRandom create() {
try {
// JENKINS-20108
// on Unix, "new SecureRandom()" uses NativePRNG that uses a VM-wide lock, which results in
// SecureRandom.nextInt() contention when there are lots of concurrent connections.
// SHA1PRNG avoids this problem. This PRNG still gets seeded from (blocking) /dev/random,
// which assures security.
//
// note that SHA1PRNG is not a standard. See http://security.stackexchange.com/questions/47871/
//
// there's also http://coding.tocea.com/scertify-code/dont-use-the-sha1-prng-randomness-generator/
// which claims SHA1PRNG has "statistical defects" without details. I discount the credibility of
// this claim based on the lack of details, and that this is not reported as a vulnerability upstream.
return SecureRandom.getInstance("SHA1PRNG");
} catch (NoSuchAlgorithmException e) {
// fall back
return new SecureRandom();
}
}
}

0 comments on commit 961adfb

Please sign in to comment.