From 84084bba071fb0b41354056036ecfd17441b7ea5 Mon Sep 17 00:00:00 2001
From: Subodh Iyengar <subodh@fb.com>
Date: Wed, 18 Jun 2014 10:47:27 -0700
Subject: [PATCH] Improve some of the exception messages.

Summary:
Add details of cause of exception to the call
stack for the exception.

Test Plan:
Tested by throwing exception.
---
 .gitignore                                    |  1 +
 .../crypto/cipher/NativeGCMCipher.java        |  4 +++-
 .../crypto/exception/KeyChainException.java   |  4 ++++
 .../crypto/keychain/SecureRandomFix.java      |  6 +++---
 javatests/com/facebook/crypto/BUCK            |  2 ++
 third-party/junit/BUCK                        | 20 ++++++++-----------
 third-party/robolectric2/BUCK                 | 14 +++----------
 7 files changed, 24 insertions(+), 27 deletions(-)

diff --git a/.gitignore b/.gitignore
index e9be6c6..ac89b25 100644
--- a/.gitignore
+++ b/.gitignore
@@ -2,6 +2,7 @@
 # relevant to BUCK
 buck-out
 /.buckconfig.local
+.buckd
 gen
 
 # Editor generated files
diff --git a/java/com/facebook/crypto/cipher/NativeGCMCipher.java b/java/com/facebook/crypto/cipher/NativeGCMCipher.java
index e2c14b7..527afc9 100644
--- a/java/com/facebook/crypto/cipher/NativeGCMCipher.java
+++ b/java/com/facebook/crypto/cipher/NativeGCMCipher.java
@@ -105,7 +105,9 @@ public void decryptFinal(byte[] expectedTag, int tagLen) throws NativeGCMCipherE
     Assertions.checkState(mCurrentState == STATE.DECRYPT_INITIALIZED, CIPHER_NOT_INIT);
     mCurrentState = STATE.DECRYPT_FINALIZED;
     if (nativeDecryptFinal(expectedTag, tagLen) == nativeFailure()) {
-      throw new NativeGCMCipherException("decryptFinal");
+      throw new NativeGCMCipherException(
+        "The message could not be decrypted successfully." +
+        "It has either been tampered with or the wrong resource is being decrypted.");
     }
   }
 
diff --git a/java/com/facebook/crypto/exception/KeyChainException.java b/java/com/facebook/crypto/exception/KeyChainException.java
index f077603..037f399 100644
--- a/java/com/facebook/crypto/exception/KeyChainException.java
+++ b/java/com/facebook/crypto/exception/KeyChainException.java
@@ -15,4 +15,8 @@ public class KeyChainException extends Exception {
     public KeyChainException(String message) {
         super(message);
     }
+
+    public KeyChainException(String message, Throwable cause) {
+        super(message, cause);
+    }
 }
diff --git a/java/com/facebook/crypto/keychain/SecureRandomFix.java b/java/com/facebook/crypto/keychain/SecureRandomFix.java
index b67aa65..f68814a 100644
--- a/java/com/facebook/crypto/keychain/SecureRandomFix.java
+++ b/java/com/facebook/crypto/keychain/SecureRandomFix.java
@@ -64,9 +64,9 @@ public synchronized void tryApplyFixes() throws KeyChainException {
         } catch (Throwable t) {
             // Practically, this should not crash.
             // However we can't do much in this case if it does.
-            // This is a trade-off between security and usability which is intentional to prevent
-            // the client from crashing.
-            throw new KeyChainException("Random number generated");
+            // We would rather abort than using an insecure random number
+            // generator.
+            throw new KeyChainException("Random number generated", t);
         }
 
         mFixApplied = true;
diff --git a/javatests/com/facebook/crypto/BUCK b/javatests/com/facebook/crypto/BUCK
index 6771286..78a4a04 100644
--- a/javatests/com/facebook/crypto/BUCK
+++ b/javatests/com/facebook/crypto/BUCK
@@ -5,6 +5,7 @@ robolectric_test(
     ':crypto-test-helper',
     '//:crypto',
     '//third-party/junit:junit',
+    '//third-party/junit:hamcrest',
     '//third-party/guava:guava',
     '//third-party/robolectric2:robolectric2',
   ],
@@ -18,6 +19,7 @@ android_library(
   srcs = glob(['**/*.java'], excludes = glob(['**/*Test.java'])),
   deps = [
     '//third-party/junit:junit',
+    '//third-party/junit:hamcrest',
     '//third-party/robolectric2:robolectric2',
   ],
 )
diff --git a/third-party/junit/BUCK b/third-party/junit/BUCK
index 8ba8ed7..79f752a 100644
--- a/third-party/junit/BUCK
+++ b/third-party/junit/BUCK
@@ -1,19 +1,15 @@
-java_library(
-  name = 'junit',
-  export_deps = True,
-  deps = [
-    ':junit-jar',
-    ':hamcrest-jar',
-  ],
-  visibility = [ 'PUBLIC' ],
-)
-
 prebuilt_jar(
-  name = 'junit-jar',
+  name = 'junit',
   binary_jar = 'junit-4.11.jar',
+  visibility = [
+    '//javatests/com/facebook/crypto:',
+  ],
 )
 
 prebuilt_jar(
-  name = 'hamcrest-jar',
+  name = 'hamcrest',
   binary_jar = 'hamcrest-core-1.3.jar',
+  visibility = [
+    '//javatests/com/facebook/crypto:',
+  ],
 )
diff --git a/third-party/robolectric2/BUCK b/third-party/robolectric2/BUCK
index 8ad4ac1..ff0923e 100644
--- a/third-party/robolectric2/BUCK
+++ b/third-party/robolectric2/BUCK
@@ -1,15 +1,7 @@
-java_library(
+prebuilt_jar(
   name = 'robolectric2',
-  export_deps = True,
-  deps = [
-    ':robolectric2-jar',
-  ],
+  binary_jar = 'robolectric-2.2-jar-with-dependencies.jar',
   visibility = [
-    'PUBLIC',
+    '//javatests/com/facebook/crypto:',
   ],
 )
-
-prebuilt_jar(
-  name = 'robolectric2-jar',
-  binary_jar = 'robolectric-2.2-jar-with-dependencies.jar',
-)