Do not seed /dev/urandom. urandom is non-writeable on some devices. Because only OpenSSL seeding is broken, this should not result in security problems!

This commit is contained in:
Dominik Schürmann 2013-09-15 22:27:09 +02:00
parent c90e776055
commit e05d17eec2

View File

@ -18,9 +18,7 @@ import java.io.DataInputStream;
import java.io.DataOutputStream; import java.io.DataOutputStream;
import java.io.File; import java.io.File;
import java.io.FileInputStream; import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException; import java.io.IOException;
import java.io.OutputStream;
import java.io.UnsupportedEncodingException; import java.io.UnsupportedEncodingException;
import java.security.NoSuchAlgorithmException; import java.security.NoSuchAlgorithmException;
import java.security.Provider; import java.security.Provider;
@ -35,6 +33,19 @@ import java.security.Security;
* Architecture primitives. A good place to invoke them is in the application's {@code onCreate}. * Architecture primitives. A good place to invoke them is in the application's {@code onCreate}.
* *
* copied from http://android-developers.blogspot.de/2013/08/some-securerandom-thoughts.html * copied from http://android-developers.blogspot.de/2013/08/some-securerandom-thoughts.html
*
*
* Sep 15, 2013:
* On some devices /dev/urandom is non-writable!
* No need to seed /dev/urandom. urandom should have enough seeds from the OS and kernel.
* Only OpenSSL seeds are broken. See http://emboss.github.io/blog/2013/08/21/openssl-prng-is-not-really-fork-safe
*
* see also:
* https://github.com/k9mail/k-9/commit/dda8f64276d4d29c43f86237cd77819c28f22f21
* In addition to a couple of custom ROMs linking /dev/urandom to a non-writable
* random version, now Samsung's SELinux policy also prevents apps from opening
* /dev/urandom for writing. Since we shouldn't need to write to /dev/urandom anyway
* we now simply don't.
*/ */
public final class PRNGFixes { public final class PRNGFixes {
@ -138,6 +149,7 @@ public final class PRNGFixes {
* {@code Provider} of {@code SecureRandom} engines which pass through all requests to the Linux * {@code Provider} of {@code SecureRandom} engines which pass through all requests to the Linux
* PRNG. * PRNG.
*/ */
@SuppressWarnings("serial")
private static class LinuxPRNGSecureRandomProvider extends Provider { private static class LinuxPRNGSecureRandomProvider extends Provider {
public LinuxPRNGSecureRandomProvider() { public LinuxPRNGSecureRandomProvider() {
@ -155,6 +167,7 @@ public final class PRNGFixes {
/** /**
* {@link SecureRandomSpi} which passes all requests to the Linux PRNG ({@code /dev/urandom}). * {@link SecureRandomSpi} which passes all requests to the Linux PRNG ({@code /dev/urandom}).
*/ */
@SuppressWarnings("serial")
public static class LinuxPRNGSecureRandom extends SecureRandomSpi { public static class LinuxPRNGSecureRandom extends SecureRandomSpi {
/* /*
@ -178,40 +191,40 @@ public final class PRNGFixes {
*/ */
private static DataInputStream sUrandomIn; private static DataInputStream sUrandomIn;
/** // /**
* Output stream for writing to Linux PRNG or {@code null} if not yet opened. // * Output stream for writing to Linux PRNG or {@code null} if not yet opened.
* // *
* @GuardedBy("sLock") // * @GuardedBy("sLock")
*/ // */
private static OutputStream sUrandomOut; // private static OutputStream sUrandomOut;
//
/** // /**
* Whether this engine instance has been seeded. This is needed because each instance needs // * Whether this engine instance has been seeded. This is needed because each instance needs
* to seed itself if the client does not explicitly seed it. // * to seed itself if the client does not explicitly seed it.
*/ // */
private boolean mSeeded; // private boolean mSeeded;
@Override @Override
protected void engineSetSeed(byte[] bytes) { protected void engineSetSeed(byte[] bytes) {
try { // try {
OutputStream out; // OutputStream out;
synchronized (sLock) { // synchronized (sLock) {
out = getUrandomOutputStream(); // out = getUrandomOutputStream();
} // }
out.write(bytes); // out.write(bytes);
out.flush(); // out.flush();
mSeeded = true; // mSeeded = true;
} catch (IOException e) { // } catch (IOException e) {
throw new SecurityException("Failed to mix seed into " + URANDOM_FILE, e); // throw new SecurityException("Failed to mix seed into " + URANDOM_FILE, e);
} // }
} }
@Override @Override
protected void engineNextBytes(byte[] bytes) { protected void engineNextBytes(byte[] bytes) {
if (!mSeeded) { // if (!mSeeded) {
// Mix in the device- and invocation-specific seed. // // Mix in the device- and invocation-specific seed.
engineSetSeed(generateSeed()); // engineSetSeed(generateSeed());
} // }
try { try {
DataInputStream in; DataInputStream in;
@ -251,19 +264,19 @@ public final class PRNGFixes {
} }
} }
private OutputStream getUrandomOutputStream() { // private OutputStream getUrandomOutputStream() {
synchronized (sLock) { // synchronized (sLock) {
if (sUrandomOut == null) { // if (sUrandomOut == null) {
try { // try {
sUrandomOut = new FileOutputStream(URANDOM_FILE); // sUrandomOut = new FileOutputStream(URANDOM_FILE);
} catch (IOException e) { // } catch (IOException e) {
throw new SecurityException("Failed to open " + URANDOM_FILE // throw new SecurityException("Failed to open " + URANDOM_FILE
+ " for writing", e); // + " for writing", e);
} // }
} // }
return sUrandomOut; // return sUrandomOut;
} // }
} // }
} }
/** /**