Commit df0de364 authored by lambroslambrou's avatar lambroslambrou Committed by Commit bot

Cleanup: Consolidate initialization of Android's SecureRandom class.

This replaces some copy-pasted snippets of code with a centralized
utility for securely initializing a SecureRandom instance (needed for
Android versions <= 4.3).

BUG=402732

Review URL: https://codereview.chromium.org/624183002

Cr-Commit-Position: refs/heads/master@{#299788}
parent f7023d2a
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
package org.chromium.chromoting; package org.chromium.base;
import java.io.FileInputStream; import java.io.FileInputStream;
import java.io.IOException; import java.io.IOException;
...@@ -16,6 +16,8 @@ import java.security.SecureRandom; ...@@ -16,6 +16,8 @@ import java.security.SecureRandom;
public class SecureRandomInitializer { public class SecureRandomInitializer {
private static final int NUM_RANDOM_BYTES = 16; private static final int NUM_RANDOM_BYTES = 16;
private static byte[] sSeedBytes = new byte[NUM_RANDOM_BYTES];
/** /**
* Safely initializes the random number generator, by seeding it with data from /dev/urandom. * Safely initializes the random number generator, by seeding it with data from /dev/urandom.
*/ */
...@@ -23,11 +25,10 @@ public class SecureRandomInitializer { ...@@ -23,11 +25,10 @@ public class SecureRandomInitializer {
FileInputStream fis = null; FileInputStream fis = null;
try { try {
fis = new FileInputStream("/dev/urandom"); fis = new FileInputStream("/dev/urandom");
byte[] bytes = new byte[NUM_RANDOM_BYTES]; if (fis.read(sSeedBytes) != sSeedBytes.length) {
if (bytes.length != fis.read(bytes)) {
throw new IOException("Failed to get enough random data."); throw new IOException("Failed to get enough random data.");
} }
generator.setSeed(bytes); generator.setSeed(sSeedBytes);
} finally { } finally {
try { try {
if (fis != null) { if (fis != null) {
......
...@@ -8,6 +8,8 @@ import android.content.Context; ...@@ -8,6 +8,8 @@ import android.content.Context;
import android.os.AsyncTask; import android.os.AsyncTask;
import android.util.Log; import android.util.Log;
import org.chromium.base.SecureRandomInitializer;
import java.io.File; import java.io.File;
import java.io.FileInputStream; import java.io.FileInputStream;
import java.io.FileOutputStream; import java.io.FileOutputStream;
...@@ -191,20 +193,7 @@ public class WebappAuthenticator { ...@@ -191,20 +193,7 @@ public class WebappAuthenticator {
public SecretKey call() throws Exception { public SecretKey call() throws Exception {
KeyGenerator generator = KeyGenerator.getInstance(MAC_ALGORITHM_NAME); KeyGenerator generator = KeyGenerator.getInstance(MAC_ALGORITHM_NAME);
SecureRandom random = SecureRandom.getInstance("SHA1PRNG"); SecureRandom random = SecureRandom.getInstance("SHA1PRNG");
SecureRandomInitializer.initialize(random);
// Versions of SecureRandom from Android <= 4.3 do not seed themselves as
// securely as possible. This workaround should suffice until the fixed version
// is deployed to all users. getRandomBytes, which reads from /dev/urandom,
// which is as good as the platform can get.
//
// TODO(palmer): Consider getting rid of this once the updated platform has
// shipped to everyone. Alternately, leave this in as a defense against other
// bugs in SecureRandom.
byte[] seed = getRandomBytes(MAC_KEY_BYTE_COUNT);
if (seed == null) {
return null;
}
random.setSeed(seed);
generator.init(MAC_KEY_BYTE_COUNT * 8, random); generator.init(MAC_KEY_BYTE_COUNT * 8, random);
return generator.generateKey(); return generator.generateKey();
} }
...@@ -213,29 +202,6 @@ public class WebappAuthenticator { ...@@ -213,29 +202,6 @@ public class WebappAuthenticator {
} }
} }
private static byte[] getRandomBytes(int count) {
FileInputStream fis = null;
try {
fis = new FileInputStream("/dev/urandom");
byte[] bytes = new byte[count];
if (bytes.length != fis.read(bytes)) {
return null;
}
return bytes;
} catch (Throwable t) {
// This causes the ultimate caller, i.e. getMac, to fail.
return null;
} finally {
try {
if (fis != null) {
fis.close();
}
} catch (IOException e) {
// Nothing we can do.
}
}
}
/** /**
* @return A Mac, or null if it is not possible to instantiate one. * @return A Mac, or null if it is not possible to instantiate one.
*/ */
......
...@@ -8,6 +8,8 @@ import android.os.AsyncTask; ...@@ -8,6 +8,8 @@ import android.os.AsyncTask;
import android.os.Bundle; import android.os.Bundle;
import android.util.Log; import android.util.Log;
import org.chromium.base.SecureRandomInitializer;
import java.io.IOException; import java.io.IOException;
import java.security.GeneralSecurityException; import java.security.GeneralSecurityException;
import java.security.Key; import java.security.Key;
...@@ -150,9 +152,8 @@ public class CipherFactory { ...@@ -150,9 +152,8 @@ public class CipherFactory {
@Override @Override
public CipherData call() { public CipherData call() {
// Poll random data to generate initialization parameters for the Cipher. // Poll random data to generate initialization parameters for the Cipher.
byte[] seed, iv; byte[] iv;
try { try {
seed = mRandomNumberProvider.getBytes(NUM_BYTES);
iv = mRandomNumberProvider.getBytes(NUM_BYTES); iv = mRandomNumberProvider.getBytes(NUM_BYTES);
} catch (IOException e) { } catch (IOException e) {
Log.e(TAG, "Couldn't get generator data."); Log.e(TAG, "Couldn't get generator data.");
...@@ -163,20 +164,15 @@ public class CipherFactory { ...@@ -163,20 +164,15 @@ public class CipherFactory {
} }
try { try {
// Old versions of SecureRandom do not seed themselves as securely as possible.
// This workaround should suffice until the fixed version is deployed to all
// users. The seed comes from RandomNumberProvider.getBytes(), which reads
// from /dev/urandom, which is as good as the platform can get.
//
// TODO(palmer): Consider getting rid of this once the updated platform has
// shipped to everyone. Alternately, leave this in as a defense against other
// bugs in SecureRandom.
SecureRandom random = SecureRandom.getInstance("SHA1PRNG"); SecureRandom random = SecureRandom.getInstance("SHA1PRNG");
random.setSeed(seed); SecureRandomInitializer.initialize(random);
KeyGenerator generator = KeyGenerator.getInstance("AES"); KeyGenerator generator = KeyGenerator.getInstance("AES");
generator.init(128, random); generator.init(128, random);
return new CipherData(generator.generateKey(), iv); return new CipherData(generator.generateKey(), iv);
} catch (IOException e) {
Log.e(TAG, "Couldn't get generator data.");
return null;
} catch (GeneralSecurityException e) { } catch (GeneralSecurityException e) {
Log.e(TAG, "Couldn't get generator instances."); Log.e(TAG, "Couldn't get generator instances.");
return null; return null;
...@@ -270,4 +266,4 @@ public class CipherFactory { ...@@ -270,4 +266,4 @@ public class CipherFactory {
private CipherFactory() { private CipherFactory() {
mRandomNumberProvider = new ByteArrayGenerator(); mRandomNumberProvider = new ByteArrayGenerator();
} }
} }
\ No newline at end of file
...@@ -15,6 +15,8 @@ import android.text.TextUtils; ...@@ -15,6 +15,8 @@ import android.text.TextUtils;
import android.util.Base64; import android.util.Base64;
import android.util.Log; import android.util.Log;
import org.chromium.base.SecureRandomInitializer;
import java.io.IOException; import java.io.IOException;
import java.security.SecureRandom; import java.security.SecureRandom;
import java.util.ArrayList; import java.util.ArrayList;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment