catch null pointer exceptions when the private key cannot be extracted, also prevent such keys from getting imported, so the error message should never be encountered anyway

Fixes issue 66

5 files changed, 62 insertions, 6 deletions

diff --git a/res/values/strings.xml b/res/values/strings.xml
index 1e83bb7bd..104b81251 100644
--- a/res/values/strings.xml
+++ b/res/values/strings.xml
@@ -210,6 +210,7 @@
     <string name="keysFound">Found %s key(s).</string>
     <string name="unknownSignatureKeyTouchToLookUp">Unknown signature, touch to look up key.</string>
     <string name="keyEditingIsBeta">Key editing is still kind of beta.</string>
+    <string name="badKeysEncountered">%s bad secret key(s) ignored. Perhaps you exported with the option\n --export-secret-subkeys\nMake sure you export with\n --export-secret-keys\ninstead.</string>
 
     <!-- error_lowerCase: phrases, no punctuation, all lowercase,
          they will be put after "errorMessage", e.g. "Error: file not found" -->
@@ -240,6 +241,7 @@
     <string name="error_noSymmetricEncryptionPacket">couldn\'t find a packet with symmetric encryption</string>
     <string name="error_wrongPassPhrase">wrong pass phrase</string>
     <string name="error_savingKeys">error saving some key(s)</string>
+    <string name="error_couldNotExtractPrivateKey">could not extract private key</string>
 
     <!-- progress_lowerCase: lowercase, phrases, usually ending in '...' -->
     <string name="progress_done">done.</string>
diff --git a/src/org/thialfihar/android/apg/Apg.java b/src/org/thialfihar/android/apg/Apg.java
index f1615f6a5..4db544556 100644
--- a/src/org/thialfihar/android/apg/Apg.java
+++ b/src/org/thialfihar/android/apg/Apg.java
@@ -101,6 +101,7 @@ import android.net.Uri;
 import android.os.Bundle;
 import android.os.Environment;
 import android.view.ViewGroup;
+import android.widget.Toast;
 
 public class Apg {
     private static final String mApgPackageName = "org.thialfihar.android.apg";
@@ -616,6 +617,7 @@ public class Apg {
         BufferedInputStream bufferedInput = new BufferedInputStream(progressIn);
         int newKeys = 0;
         int oldKeys = 0;
+        int badKeys = 0;
         try {
             while (true) {
                 InputStream in = PGPUtil.getDecoderStream(bufferedInput);
@@ -635,7 +637,22 @@ public class Apg {
                     try {
                         if (type == Id.type.secret_key && obj instanceof PGPSecretKeyRing) {
                             secretKeyRing = (PGPSecretKeyRing) obj;
-                            retValue = mDatabase.saveKeyRing(secretKeyRing);
+                            boolean save = true;
+                            try {
+                                PGPPrivateKey testKey = secretKeyRing.getSecretKey()
+                                        .extractPrivateKey(new char[] {}, new BouncyCastleProvider());
+                                if (testKey == null) {
+                                    // this is bad, something is very wrong... likely a
+                                    // --export-secret-subkeys export
+                                    retValue = Id.return_value.bad;
+                                    save = false;
+                                }
+                            } catch (PGPException e) {
+                                // all good if this fails, we likely didn't use the right password
+                            }
+                            if (save) {
+                                retValue = mDatabase.saveKeyRing(secretKeyRing);
+                            }
                         } else if (type == Id.type.public_key && obj instanceof PGPPublicKeyRing) {
                             publicKeyRing = (PGPPublicKeyRing) obj;
                             retValue = mDatabase.saveKeyRing(publicKeyRing);
@@ -654,6 +671,8 @@ public class Apg {
                         ++oldKeys;
                     } else if (retValue == Id.return_value.ok) {
                         ++newKeys;
+                    } else if (retValue == Id.return_value.bad) {
+                        ++badKeys;
                     }
                     progress.setProgress((int)(100 * progressIn.position() / data.getSize()), 100);
                     obj = objectFactory.nextObject();
@@ -665,6 +684,7 @@ public class Apg {
 
         returnData.putInt("added", newKeys);
         returnData.putInt("updated", oldKeys);
+        returnData.putInt("bad", badKeys);
 
         progress.setProgress(R.string.progress_done, 100, 100);
 
@@ -1194,6 +1214,9 @@ public class Apg {
             progress.setProgress(R.string.progress_extractingSignatureKey, 0, 100);
             signaturePrivateKey = signingKey.extractPrivateKey(signaturePassPhrase.toCharArray(),
                                                                new BouncyCastleProvider());
+            if (signaturePrivateKey == null) {
+                throw new GeneralException(context.getString(R.string.error_couldNotExtractPrivateKey));
+            }
         }
 
         progress.setProgress(R.string.progress_preparingStreams, 5, 100);
@@ -1334,13 +1357,16 @@ public class Apg {
         signaturePrivateKey =
                 signingKey.extractPrivateKey(signaturePassPhrase.toCharArray(),
                                              new BouncyCastleProvider());
-
+        if (signaturePrivateKey == null) {
+            throw new GeneralException(context.getString(R.string.error_couldNotExtractPrivateKey));
+        }
         progress.setProgress(R.string.progress_preparingStreams, 0, 100);
 
         progress.setProgress(R.string.progress_preparingSignature, 30, 100);
 
         PGPSignatureGenerator signatureGenerator = null;
         PGPV3SignatureGenerator signatureV3Generator = null;
+
         if (forceV3Signature) {
             signatureV3Generator =
                 new PGPV3SignatureGenerator(signingKey.getPublicKey().getAlgorithm(),
@@ -1567,6 +1593,9 @@ public class Apg {
             } catch (PGPException e) {
                 throw new PGPException(context.getString(R.string.error_wrongPassPhrase));
             }
+            if (privateKey == null) {
+                throw new GeneralException(context.getString(R.string.error_couldNotExtractPrivateKey));
+            }
             currentProgress += 5;
             progress.setProgress(R.string.progress_preparingStreams, currentProgress, 100);
             clear = pbe.getDataStream(privateKey, new BouncyCastleProvider());
diff --git a/src/org/thialfihar/android/apg/AskForSecretKeyPassPhrase.java b/src/org/thialfihar/android/apg/AskForSecretKeyPassPhrase.java
index 1e3dd3726..829244e0d 100644
--- a/src/org/thialfihar/android/apg/AskForSecretKeyPassPhrase.java
+++ b/src/org/thialfihar/android/apg/AskForSecretKeyPassPhrase.java
@@ -18,7 +18,9 @@ package org.thialfihar.android.apg;
 
 import org.bouncycastle2.jce.provider.BouncyCastleProvider;
 import org.bouncycastle2.openpgp.PGPException;
+import org.bouncycastle2.openpgp.PGPPrivateKey;
 import org.bouncycastle2.openpgp.PGPSecretKey;
+import org.thialfihar.android.apg.Apg.GeneralException;
 
 import android.app.Activity;
 import android.app.AlertDialog;
@@ -84,8 +86,14 @@ public class AskForSecretKeyPassPhrase {
                         long keyId;
                         if (secretKey != null) {
                             try {
-                                secretKey.extractPrivateKey(passPhrase.toCharArray(),
-                                                            new BouncyCastleProvider());
+                                PGPPrivateKey testKey = secretKey.extractPrivateKey(passPhrase.toCharArray(),
+                                                                                    new BouncyCastleProvider());
+                                if (testKey == null) {
+                                    Toast.makeText(activity,
+                                                   R.string.error_couldNotExtractPrivateKey,
+                                                   Toast.LENGTH_SHORT).show();
+                                    return;
+                                }
                             } catch (PGPException e) {
                                 Toast.makeText(activity,
                                                R.string.wrongPassPhrase,
diff --git a/src/org/thialfihar/android/apg/Id.java b/src/org/thialfihar/android/apg/Id.java
index 341d54212..9431bc505 100644
--- a/src/org/thialfihar/android/apg/Id.java
+++ b/src/org/thialfihar/android/apg/Id.java
@@ -136,6 +136,7 @@ public final class Id {
         public static final int error = -1;
         public static final int no_master_key = -2;
         public static final int updated = 1;
+        public static final int bad = -3;
     }
 
     public static final class target {
diff --git a/src/org/thialfihar/android/apg/KeyListActivity.java b/src/org/thialfihar/android/apg/KeyListActivity.java
index a33818e59..2d5d0e5a5 100644
--- a/src/org/thialfihar/android/apg/KeyListActivity.java
+++ b/src/org/thialfihar/android/apg/KeyListActivity.java
@@ -400,6 +400,7 @@ public class KeyListActivity extends BaseActivity {
                     } else {
                         int added = data.getInt("added");
                         int updated = data.getInt("updated");
+                        int bad = data.getInt("bad");
                         String message;
                         if (added > 0 && updated > 0) {
                             message = getString(R.string.keysAddedAndUpdated, added, updated);
@@ -412,8 +413,23 @@ public class KeyListActivity extends BaseActivity {
                         }
                         Toast.makeText(KeyListActivity.this, message,
                                        Toast.LENGTH_SHORT).show();
-                        // everything went well, so now delete, if that was turned on
-                        if (mDeleteAfterImport) {
+                        if (bad > 0) {
+                            AlertDialog.Builder alert = new AlertDialog.Builder(this);
+
+                            alert.setIcon(android.R.drawable.ic_dialog_alert);
+                            alert.setTitle(R.string.warning);
+                            alert.setMessage(this.getString(R.string.badKeysEncountered, bad));
+
+                            alert.setPositiveButton(android.R.string.ok,
+                                    new DialogInterface.OnClickListener() {
+                                        public void onClick(DialogInterface dialog, int id) {
+                                            dialog.cancel();
+                                        }
+                                    });
+                            alert.setCancelable(true);
+                            alert.create().show();
+                        } else if (mDeleteAfterImport) {
+                            // everything went well, so now delete, if that was turned on
                             setDeleteFile(mImportFilename);
                             showDialog(Id.dialog.delete_file);
                         }