From bc50ca00930a0b65bd49ea758283efa1c1644036 Mon Sep 17 00:00:00 2001 From: Thialfihar Date: Fri, 10 Sep 2010 20:36:38 +0000 Subject: [PATCH] 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 --- res/values/strings.xml | 2 ++ src/org/thialfihar/android/apg/Apg.java | 33 +++++++++++++++++-- .../apg/AskForSecretKeyPassPhrase.java | 12 +++++-- src/org/thialfihar/android/apg/Id.java | 1 + .../android/apg/KeyListActivity.java | 20 +++++++++-- 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 @@ Found %s key(s). Unknown signature, touch to look up key. Key editing is still kind of beta. + %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. @@ -240,6 +241,7 @@ couldn\'t find a packet with symmetric encryption wrong pass phrase error saving some key(s) + could not extract private key done. 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); }