From e05d17eec2d8f2d2f168bd8be488590e540d8bf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Sch=C3=BCrmann?= Date: Sun, 15 Sep 2013 22:27:09 +0200 Subject: [PATCH] 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! --- .../keychain/util/PRNGFixes.java | 97 +++++++++++-------- 1 file changed, 55 insertions(+), 42 deletions(-) diff --git a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/util/PRNGFixes.java b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/util/PRNGFixes.java index 9736bb4b8..c24023e48 100644 --- a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/util/PRNGFixes.java +++ b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/util/PRNGFixes.java @@ -18,9 +18,7 @@ import java.io.DataInputStream; import java.io.DataOutputStream; import java.io.File; import java.io.FileInputStream; -import java.io.FileOutputStream; import java.io.IOException; -import java.io.OutputStream; import java.io.UnsupportedEncodingException; import java.security.NoSuchAlgorithmException; 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}. * * 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 { @@ -138,6 +149,7 @@ public final class PRNGFixes { * {@code Provider} of {@code SecureRandom} engines which pass through all requests to the Linux * PRNG. */ + @SuppressWarnings("serial") private static class LinuxPRNGSecureRandomProvider extends Provider { public LinuxPRNGSecureRandomProvider() { @@ -155,6 +167,7 @@ public final class PRNGFixes { /** * {@link SecureRandomSpi} which passes all requests to the Linux PRNG ({@code /dev/urandom}). */ + @SuppressWarnings("serial") public static class LinuxPRNGSecureRandom extends SecureRandomSpi { /* @@ -178,40 +191,40 @@ public final class PRNGFixes { */ private static DataInputStream sUrandomIn; - /** - * Output stream for writing to Linux PRNG or {@code null} if not yet opened. - * - * @GuardedBy("sLock") - */ - private static OutputStream sUrandomOut; - - /** - * 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. - */ - private boolean mSeeded; +// /** +// * Output stream for writing to Linux PRNG or {@code null} if not yet opened. +// * +// * @GuardedBy("sLock") +// */ +// private static OutputStream sUrandomOut; +// +// /** +// * 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. +// */ +// private boolean mSeeded; @Override protected void engineSetSeed(byte[] bytes) { - try { - OutputStream out; - synchronized (sLock) { - out = getUrandomOutputStream(); - } - out.write(bytes); - out.flush(); - mSeeded = true; - } catch (IOException e) { - throw new SecurityException("Failed to mix seed into " + URANDOM_FILE, e); - } +// try { +// OutputStream out; +// synchronized (sLock) { +// out = getUrandomOutputStream(); +// } +// out.write(bytes); +// out.flush(); +// mSeeded = true; +// } catch (IOException e) { +// throw new SecurityException("Failed to mix seed into " + URANDOM_FILE, e); +// } } @Override protected void engineNextBytes(byte[] bytes) { - if (!mSeeded) { - // Mix in the device- and invocation-specific seed. - engineSetSeed(generateSeed()); - } +// if (!mSeeded) { +// // Mix in the device- and invocation-specific seed. +// engineSetSeed(generateSeed()); +// } try { DataInputStream in; @@ -251,19 +264,19 @@ public final class PRNGFixes { } } - private OutputStream getUrandomOutputStream() { - synchronized (sLock) { - if (sUrandomOut == null) { - try { - sUrandomOut = new FileOutputStream(URANDOM_FILE); - } catch (IOException e) { - throw new SecurityException("Failed to open " + URANDOM_FILE - + " for writing", e); - } - } - return sUrandomOut; - } - } +// private OutputStream getUrandomOutputStream() { +// synchronized (sLock) { +// if (sUrandomOut == null) { +// try { +// sUrandomOut = new FileOutputStream(URANDOM_FILE); +// } catch (IOException e) { +// throw new SecurityException("Failed to open " + URANDOM_FILE +// + " for writing", e); +// } +// } +// return sUrandomOut; +// } +// } } /**