Updated PRNGFixes from official blogpost

2 files changed, 120 insertions, 97 deletions

diff --git a/OpenPGP-Keychain/AndroidManifest.xml b/OpenPGP-Keychain/AndroidManifest.xml
index fdaf3b3e3..a38d9b071 100644
--- a/OpenPGP-Keychain/AndroidManifest.xml
+++ b/OpenPGP-Keychain/AndroidManifest.xml
@@ -350,7 +350,7 @@
             android:name=".ui.HelpActivity"
             android:label="@string/title_help" />
 
-        <!-- Internal services/content provider (not exported) -->
+        <!-- Internal services/content providers (not exported) -->
         <service
             android:name=".service.PassphraseCacheService"
             android:exported="false"
@@ -364,7 +364,7 @@
             android:authorities="org.sufficientlysecure.keychain.provider"
             android:exported="false" />
 
-        <!-- Internal classes of OpenPGP (not exported) -->
+        <!-- Internal classes of the remote APIs (not exported) -->
         <activity
             android:name="org.sufficientlysecure.keychain.service.remote.RemoteServiceActivity"
             android:exported="false"
@@ -398,7 +398,6 @@
         </service>
 
         <!-- Extended Remote API -->
-
         <service
             android:name="org.sufficientlysecure.keychain.service.remote.ExtendedApiService"
             android:enabled="true"
@@ -416,7 +415,7 @@
         <!-- TODO: authority! Make this API with content provider uris -->
         <!-- <provider -->
         <!-- android:name="org.sufficientlysecure.keychain.provider.KeychainServiceBlobProvider" -->
-        <!-- android:authorities="org.sufficientlysecure.keychain.provider.apgserviceblobprovider" -->
+        <!-- android:authorities="org.sufficientlysecure.keychain.provider.KeychainServiceBlobProvider" -->
         <!-- android:permission="org.sufficientlysecure.keychain.permission.ACCESS_API" /> -->
     </application>
 
diff --git a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/util/PRNGFixes.java b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/util/PRNGFixes.java
index a7919e5e3..530a81044 100644
--- a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/util/PRNGFixes.java
+++ b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/util/PRNGFixes.java
@@ -12,13 +12,16 @@ package org.sufficientlysecure.keychain.util;
 
 import android.os.Build;
 import android.os.Process;
+import android.util.Log;
 
 import java.io.ByteArrayOutputStream;
 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;
@@ -51,22 +54,27 @@ import java.security.Security;
  * 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.
+ *
+ *
+ * Sep 17, 2013:
+ * Updated from official blogpost:
+ * Update: the original code sample below crashed on a small fraction of Android
+ * devices due to /dev/urandom not being writable. We have now updated the code sample to handle this case gracefully.
  */
 public final class PRNGFixes {
 
     private static final int VERSION_CODE_JELLY_BEAN = 16;
     private static final int VERSION_CODE_JELLY_BEAN_MR2 = 18;
-    private static final byte[] BUILD_FINGERPRINT_AND_DEVICE_SERIAL = getBuildFingerprintAndDeviceSerial();
+    private static final byte[] BUILD_FINGERPRINT_AND_DEVICE_SERIAL =
+        getBuildFingerprintAndDeviceSerial();
 
     /** Hidden constructor to prevent instantiation. */
-    private PRNGFixes() {
-    }
+    private PRNGFixes() {}
 
     /**
      * Applies all fixes.
-     * 
-     * @throws SecurityException
-     *             if a fix is needed but could not be applied.
+     *
+     * @throws SecurityException if a fix is needed but could not be applied.
      */
     public static void apply() {
         applyOpenSSLFix();
@@ -74,10 +82,10 @@ public final class PRNGFixes {
     }
 
     /**
-     * Applies the fix for OpenSSL PRNG having low entropy. Does nothing if the fix is not needed.
-     * 
-     * @throws SecurityException
-     *             if the fix is needed but could not be applied.
+     * Applies the fix for OpenSSL PRNG having low entropy. Does nothing if the
+     * fix is not needed.
+     *
+     * @throws SecurityException if the fix is needed but could not be applied.
      */
     private static void applyOpenSSLFix() throws SecurityException {
         if ((Build.VERSION.SDK_INT < VERSION_CODE_JELLY_BEAN)
@@ -89,16 +97,18 @@ public final class PRNGFixes {
         try {
             // Mix in the device- and invocation-specific seed.
             Class.forName("org.apache.harmony.xnet.provider.jsse.NativeCrypto")
-                    .getMethod("RAND_seed", byte[].class).invoke(null, generateSeed());
+                    .getMethod("RAND_seed", byte[].class)
+                    .invoke(null, generateSeed());
 
             // Mix output of Linux PRNG into OpenSSL's PRNG
-            int bytesRead = (Integer) Class
-                    .forName("org.apache.harmony.xnet.provider.jsse.NativeCrypto")
+            int bytesRead = (Integer) Class.forName(
+                    "org.apache.harmony.xnet.provider.jsse.NativeCrypto")
                     .getMethod("RAND_load_file", String.class, long.class)
                     .invoke(null, "/dev/urandom", 1024);
             if (bytesRead != 1024) {
-                throw new IOException("Unexpected number of bytes read from Linux PRNG: "
-                        + bytesRead);
+                throw new IOException(
+                        "Unexpected number of bytes read from Linux PRNG: "
+                                + bytesRead);
             }
         } catch (Exception e) {
             throw new SecurityException("Failed to seed OpenSSL PRNG", e);
@@ -106,14 +116,14 @@ public final class PRNGFixes {
     }
 
     /**
-     * Installs a Linux PRNG-backed {@code SecureRandom} implementation as the default. Does nothing
-     * if the implementation is already the default or if there is not need to install the
-     * implementation.
-     * 
-     * @throws SecurityException
-     *             if the fix is needed but could not be applied.
+     * Installs a Linux PRNG-backed {@code SecureRandom} implementation as the
+     * default. Does nothing if the implementation is already the default or if
+     * there is not need to install the implementation.
+     *
+     * @throws SecurityException if the fix is needed but could not be applied.
      */
-    private static void installLinuxPRNGSecureRandom() throws SecurityException {
+    private static void installLinuxPRNGSecureRandom()
+            throws SecurityException {
         if (Build.VERSION.SDK_INT > VERSION_CODE_JELLY_BEAN_MR2) {
             // No need to apply the fix
             return;
@@ -121,11 +131,12 @@ public final class PRNGFixes {
 
         // Install a Linux PRNG-based SecureRandom implementation as the
         // default, if not yet installed.
-        Provider[] secureRandomProviders = Security.getProviders("SecureRandom.SHA1PRNG");
+        Provider[] secureRandomProviders =
+                Security.getProviders("SecureRandom.SHA1PRNG");
         if ((secureRandomProviders == null)
                 || (secureRandomProviders.length < 1)
-                || (!LinuxPRNGSecureRandomProvider.class
-                        .equals(secureRandomProviders[0].getClass()))) {
+                || (!LinuxPRNGSecureRandomProvider.class.equals(
+                        secureRandomProviders[0].getClass()))) {
             Security.insertProviderAt(new LinuxPRNGSecureRandomProvider(), 1);
         }
 
@@ -133,9 +144,11 @@ public final class PRNGFixes {
         // SecureRandom.getInstance("SHA1PRNG") return a SecureRandom backed
         // by the Linux PRNG-based SecureRandom implementation.
         SecureRandom rng1 = new SecureRandom();
-        if (!LinuxPRNGSecureRandomProvider.class.equals(rng1.getProvider().getClass())) {
-            throw new SecurityException("new SecureRandom() backed by wrong Provider: "
-                    + rng1.getProvider().getClass());
+        if (!LinuxPRNGSecureRandomProvider.class.equals(
+                rng1.getProvider().getClass())) {
+            throw new SecurityException(
+                    "new SecureRandom() backed by wrong Provider: "
+                            + rng1.getProvider().getClass());
         }
 
         SecureRandom rng2;
@@ -144,22 +157,25 @@ public final class PRNGFixes {
         } catch (NoSuchAlgorithmException e) {
             throw new SecurityException("SHA1PRNG not available", e);
         }
-        if (!LinuxPRNGSecureRandomProvider.class.equals(rng2.getProvider().getClass())) {
-            throw new SecurityException("SecureRandom.getInstance(\"SHA1PRNG\") backed by wrong"
+        if (!LinuxPRNGSecureRandomProvider.class.equals(
+                rng2.getProvider().getClass())) {
+            throw new SecurityException(
+                    "SecureRandom.getInstance(\"SHA1PRNG\") backed by wrong"
                     + " Provider: " + rng2.getProvider().getClass());
         }
     }
 
     /**
-     * {@code Provider} of {@code SecureRandom} engines which pass through all requests to the Linux
-     * PRNG.
+     * {@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() {
-            super("LinuxPRNG", 1.0, "A Linux-specific random number provider that uses"
-                    + " /dev/urandom");
+            super("LinuxPRNG",
+                    1.0,
+                    "A Linux-specific random number provider that uses"
+                        + " /dev/urandom");
             // Although /dev/urandom is not a SHA-1 PRNG, some apps
             // explicitly request a SHA1PRNG SecureRandom and we thus need to
             // prevent them from getting the default implementation whose output
@@ -170,19 +186,21 @@ 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 {
 
         /*
-         * IMPLEMENTATION NOTE: Requests to generate bytes and to mix in a seed are passed through
-         * to the Linux PRNG (/dev/urandom). Instances of this class seed themselves by mixing in
-         * the current time, PID, UID, build fingerprint, and hardware serial number (where
-         * available) into Linux PRNG.
-         * 
-         * Concurrency: Read requests to the underlying Linux PRNG are serialized (on sLock) to
-         * ensure that multiple threads do not get duplicated PRNG output.
+         * IMPLEMENTATION NOTE: Requests to generate bytes and to mix in a seed
+         * are passed through to the Linux PRNG (/dev/urandom). Instances of
+         * this class seed themselves by mixing in the current time, PID, UID,
+         * build fingerprint, and hardware serial number (where available) into
+         * Linux PRNG.
+         *
+         * Concurrency: Read requests to the underlying Linux PRNG are
+         * serialized (on sLock) to ensure that multiple threads do not get
+         * duplicated PRNG output.
          */
 
         private static final File URANDOM_FILE = new File("/dev/urandom");
@@ -190,46 +208,53 @@ public final class PRNGFixes {
         private static final Object sLock = new Object();
 
         /**
-         * Input stream for reading from Linux PRNG or {@code null} if not yet opened.
-         * 
+         * Input stream for reading from Linux PRNG or {@code null} if not yet
+         * opened.
+         *
          * @GuardedBy("sLock")
          */
         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();
+            } catch (IOException e) {
+                // On a small fraction of devices /dev/urandom is not writable.
+                // Log and ignore.
+                Log.w(PRNGFixes.class.getSimpleName(),
+                        "Failed to mix seed into " + URANDOM_FILE);
+            } finally {
+                mSeeded = true;
+            }
         }
 
         @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;
@@ -240,7 +265,8 @@ public final class PRNGFixes {
                     in.readFully(bytes);
                 }
             } catch (IOException e) {
-                throw new SecurityException("Failed to read from " + URANDOM_FILE, e);
+                throw new SecurityException(
+                        "Failed to read from " + URANDOM_FILE, e);
             }
         }
 
@@ -259,38 +285,36 @@ public final class PRNGFixes {
                     // PRNG output performance and can live with future PRNG
                     // output being pulled into this process prematurely.
                     try {
-                        sUrandomIn = new DataInputStream(new FileInputStream(URANDOM_FILE));
+                        sUrandomIn = new DataInputStream(
+                                new FileInputStream(URANDOM_FILE));
                     } catch (IOException e) {
-                        throw new SecurityException("Failed to open " + URANDOM_FILE
-                                + " for reading", e);
+                        throw new SecurityException("Failed to open "
+                                + URANDOM_FILE + " for reading", e);
                     }
                 }
                 return sUrandomIn;
             }
         }
 
-//        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() throws IOException {
+            synchronized (sLock) {
+                if (sUrandomOut == null) {
+                    sUrandomOut = new FileOutputStream(URANDOM_FILE);
+                }
+                return sUrandomOut;
+            }
+        }
     }
 
     /**
-     * Generates a device- and invocation-specific seed to be mixed into the Linux PRNG.
+     * Generates a device- and invocation-specific seed to be mixed into the
+     * Linux PRNG.
      */
     private static byte[] generateSeed() {
         try {
             ByteArrayOutputStream seedBuffer = new ByteArrayOutputStream();
-            DataOutputStream seedBufferOut = new DataOutputStream(seedBuffer);
+            DataOutputStream seedBufferOut =
+                    new DataOutputStream(seedBuffer);
             seedBufferOut.writeLong(System.currentTimeMillis());
             seedBufferOut.writeLong(System.nanoTime());
             seedBufferOut.writeInt(Process.myPid());
@@ -305,7 +329,7 @@ public final class PRNGFixes {
 
     /**
      * Gets the hardware serial number of this device.
-     * 
+     *
      * @return serial number or {@code null} if not available.
      */
     private static String getDeviceSerialNumber() {