From 363358d30b54ad70c92df59b9496dcdda5becc79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Sch=C3=BCrmann?= Date: Mon, 16 Sep 2013 13:00:47 +0200 Subject: [PATCH] Better error handling --- .../org/openintents/openpgp/OpenPgpError.java | 6 +- .../org/openintents/openpgp/OpenPgpError.java | 6 +- .../service/exception/NoUserIdsException.java | 10 ++ .../UserInteractionRequiredException.java | 10 ++ .../exception/WrongPassphraseException.java | 10 ++ .../service/remote/OpenPgpService.java | 133 ++++++++++-------- .../service/remote/RemoteService.java | 9 +- 7 files changed, 116 insertions(+), 68 deletions(-) create mode 100644 OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/exception/NoUserIdsException.java create mode 100644 OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/exception/UserInteractionRequiredException.java create mode 100644 OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/exception/WrongPassphraseException.java diff --git a/OpenPGP-Keychain-API-Demo/src/org/openintents/openpgp/OpenPgpError.java b/OpenPGP-Keychain-API-Demo/src/org/openintents/openpgp/OpenPgpError.java index 66f168d89..f108d3169 100644 --- a/OpenPGP-Keychain-API-Demo/src/org/openintents/openpgp/OpenPgpError.java +++ b/OpenPGP-Keychain-API-Demo/src/org/openintents/openpgp/OpenPgpError.java @@ -20,8 +20,10 @@ import android.os.Parcel; import android.os.Parcelable; public class OpenPgpError implements Parcelable { - public static final int ID_NO_OR_WRONG_PASSPHRASE = 1; - public static final int ID_NO_USER_IDS = 2; + public static final int GENERIC_ERROR = 0; + public static final int NO_OR_WRONG_PASSPHRASE = 1; + public static final int NO_USER_IDS = 2; + public static final int USER_INTERACTION_REQUIRED = 3; int errorId; String message; diff --git a/OpenPGP-Keychain/src/org/openintents/openpgp/OpenPgpError.java b/OpenPGP-Keychain/src/org/openintents/openpgp/OpenPgpError.java index 66f168d89..f108d3169 100644 --- a/OpenPGP-Keychain/src/org/openintents/openpgp/OpenPgpError.java +++ b/OpenPGP-Keychain/src/org/openintents/openpgp/OpenPgpError.java @@ -20,8 +20,10 @@ import android.os.Parcel; import android.os.Parcelable; public class OpenPgpError implements Parcelable { - public static final int ID_NO_OR_WRONG_PASSPHRASE = 1; - public static final int ID_NO_USER_IDS = 2; + public static final int GENERIC_ERROR = 0; + public static final int NO_OR_WRONG_PASSPHRASE = 1; + public static final int NO_USER_IDS = 2; + public static final int USER_INTERACTION_REQUIRED = 3; int errorId; String message; diff --git a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/exception/NoUserIdsException.java b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/exception/NoUserIdsException.java new file mode 100644 index 000000000..555303238 --- /dev/null +++ b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/exception/NoUserIdsException.java @@ -0,0 +1,10 @@ +package org.sufficientlysecure.keychain.service.exception; + +public class NoUserIdsException extends Exception { + + private static final long serialVersionUID = 7009311527126696207L; + + public NoUserIdsException(String message) { + super(message); + } +} \ No newline at end of file diff --git a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/exception/UserInteractionRequiredException.java b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/exception/UserInteractionRequiredException.java new file mode 100644 index 000000000..1152d6796 --- /dev/null +++ b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/exception/UserInteractionRequiredException.java @@ -0,0 +1,10 @@ +package org.sufficientlysecure.keychain.service.exception; + +public class UserInteractionRequiredException extends Exception { + + private static final long serialVersionUID = -60128148603511936L; + + public UserInteractionRequiredException(String message) { + super(message); + } +} \ No newline at end of file diff --git a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/exception/WrongPassphraseException.java b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/exception/WrongPassphraseException.java new file mode 100644 index 000000000..14b774eb5 --- /dev/null +++ b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/exception/WrongPassphraseException.java @@ -0,0 +1,10 @@ +package org.sufficientlysecure.keychain.service.exception; + +public class WrongPassphraseException extends Exception { + + private static final long serialVersionUID = -5309689232853485740L; + + public WrongPassphraseException(String message) { + super(message); + } +} \ No newline at end of file diff --git a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/remote/OpenPgpService.java b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/remote/OpenPgpService.java index af3554237..f3916bdc6 100644 --- a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/remote/OpenPgpService.java +++ b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/remote/OpenPgpService.java @@ -39,6 +39,9 @@ import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException; import org.sufficientlysecure.keychain.provider.KeychainContract; import org.sufficientlysecure.keychain.service.KeychainIntentService; import org.sufficientlysecure.keychain.service.PassphraseCacheService; +import org.sufficientlysecure.keychain.service.exception.NoUserIdsException; +import org.sufficientlysecure.keychain.service.exception.UserInteractionRequiredException; +import org.sufficientlysecure.keychain.service.exception.WrongPassphraseException; import org.sufficientlysecure.keychain.util.InputData; import org.sufficientlysecure.keychain.util.Log; @@ -46,10 +49,8 @@ import android.content.Intent; import android.database.Cursor; import android.net.Uri; import android.os.Bundle; -import android.os.Handler; import android.os.IBinder; import android.os.Message; -import android.os.Messenger; import android.os.RemoteException; public class OpenPgpService extends RemoteService { @@ -71,21 +72,24 @@ public class OpenPgpService extends RemoteService { return mBinder; } - private String getCachedPassphrase(long keyId) { + private String getCachedPassphrase(long keyId, boolean allowUserInteraction) + throws UserInteractionRequiredException { String passphrase = PassphraseCacheService.getCachedPassphrase(getContext(), keyId); if (passphrase == null) { + if (!allowUserInteraction) { + throw new UserInteractionRequiredException( + "Passphrase not found in cache! User interaction required!"); + } + Log.d(Constants.TAG, "No passphrase! Activity required!"); // start passphrase dialog + PassphraseActivityCallback callback = new PassphraseActivityCallback(); Bundle extras = new Bundle(); extras.putLong(RemoteServiceActivity.EXTRA_SECRET_KEY_ID, keyId); - - PassphraseActivityCallback callback = new PassphraseActivityCallback(); - Messenger messenger = new Messenger(new Handler(getMainLooper(), callback)); - - pauseQueueAndStartServiceActivity(RemoteServiceActivity.ACTION_CACHE_PASSPHRASE, - messenger, extras); + pauseAndStartUserInteraction(RemoteServiceActivity.ACTION_CACHE_PASSPHRASE, callback, + extras); if (callback.isSuccess()) { Log.d(Constants.TAG, "New passphrase entered!"); @@ -127,7 +131,8 @@ public class OpenPgpService extends RemoteService { * @param encryptionUserIds * @return */ - private long[] getKeyIdsFromEmails(String[] encryptionUserIds, long ownKeyId) { + private long[] getKeyIdsFromEmails(String[] encryptionUserIds, long ownKeyId, + boolean allowUserInteraction) throws UserInteractionRequiredException { // find key ids to given emails in database ArrayList keyIds = new ArrayList(); @@ -163,9 +168,9 @@ public class OpenPgpService extends RemoteService { keyIdsArray[i] = keyIds.get(i); } - if (missingUserIdsCheck || dublicateUserIdsCheck) { + // allow the user to verify pub key selection + if (allowUserInteraction && (missingUserIdsCheck || dublicateUserIdsCheck)) { SelectPubKeysActivityCallback callback = new SelectPubKeysActivityCallback(); - Messenger messenger = new Messenger(new Handler(getMainLooper(), callback)); Bundle extras = new Bundle(); extras.putLongArray(RemoteServiceActivity.EXTRA_SELECTED_MASTER_KEY_IDS, keyIdsArray); @@ -173,8 +178,8 @@ public class OpenPgpService extends RemoteService { extras.putStringArrayList(RemoteServiceActivity.EXTRA_DUBLICATE_USER_IDS, dublicateUserIds); - pauseQueueAndStartServiceActivity(RemoteServiceActivity.ACTION_SELECT_PUB_KEYS, - messenger, extras); + pauseAndStartUserInteraction(RemoteServiceActivity.ACTION_SELECT_PUB_KEYS, callback, + extras); if (callback.isSuccess()) { Log.d(Constants.TAG, "New selection of pub keys!"); @@ -185,6 +190,17 @@ public class OpenPgpService extends RemoteService { } } + // if no user interaction is allow throw exceptions on duplicate or missing pub keys + if (!allowUserInteraction) { + if (missingUserIdsCheck) + throw new UserInteractionRequiredException( + "Pub keys for these user ids are missing:" + missingUserIds.toString()); + if (dublicateUserIdsCheck) + throw new UserInteractionRequiredException( + "More than one pub key with these user ids exist:" + + dublicateUserIds.toString()); + } + if (keyIdsArray.length == 0) { return null; } @@ -227,19 +243,18 @@ public class OpenPgpService extends RemoteService { OutputStream outputStream = new ByteArrayOutputStream(); - long[] keyIds = getKeyIdsFromEmails(encryptionUserIds, appSettings.getKeyId()); + long[] keyIds = getKeyIdsFromEmails(encryptionUserIds, appSettings.getKeyId(), + allowUserInteraction); if (keyIds == null) { - callback.onError(new OpenPgpError(OpenPgpError.ID_NO_USER_IDS, "No user ids!")); - return; + throw new NoUserIdsException("No user ids!"); } PgpOperation operation = new PgpOperation(getContext(), null, inputData, outputStream); if (sign) { - String passphrase = getCachedPassphrase(appSettings.getKeyId()); + String passphrase = getCachedPassphrase(appSettings.getKeyId(), + allowUserInteraction); if (passphrase == null) { - callback.onError(new OpenPgpError(OpenPgpError.ID_NO_OR_WRONG_PASSPHRASE, - "No or wrong passphrase!")); - return; + throw new WrongPassphraseException("No or wrong passphrase!"); } operation.signAndEncrypt(asciiArmor, appSettings.getCompression(), keyIds, null, @@ -257,14 +272,14 @@ public class OpenPgpService extends RemoteService { // return over handler on client side callback.onSuccess(outputBytes, null); + } catch (UserInteractionRequiredException e) { + callbackOpenPgpError(callback, OpenPgpError.USER_INTERACTION_REQUIRED, e.getMessage()); + } catch (NoUserIdsException e) { + callbackOpenPgpError(callback, OpenPgpError.NO_USER_IDS, e.getMessage()); + } catch (WrongPassphraseException e) { + callbackOpenPgpError(callback, OpenPgpError.NO_OR_WRONG_PASSPHRASE, e.getMessage()); } catch (Exception e) { - Log.e(Constants.TAG, "KeychainService, Exception!", e); - - try { - callback.onError(new OpenPgpError(0, e.getMessage())); - } catch (Exception t) { - Log.e(Constants.TAG, "Error returning exception to client", t); - } + callbackOpenPgpError(callback, OpenPgpError.GENERIC_ERROR, e.getMessage()); } } @@ -281,11 +296,9 @@ public class OpenPgpService extends RemoteService { OutputStream outputStream = new ByteArrayOutputStream(); - String passphrase = getCachedPassphrase(appSettings.getKeyId()); + String passphrase = getCachedPassphrase(appSettings.getKeyId(), allowUserInteraction); if (passphrase == null) { - callback.onError(new OpenPgpError(OpenPgpError.ID_NO_OR_WRONG_PASSPHRASE, - "No or wrong passphrase!")); - return; + throw new WrongPassphraseException("No or wrong passphrase!"); } PgpOperation operation = new PgpOperation(getContext(), null, inputData, outputStream); @@ -298,14 +311,12 @@ public class OpenPgpService extends RemoteService { // return over handler on client side callback.onSuccess(outputBytes, null); + } catch (UserInteractionRequiredException e) { + callbackOpenPgpError(callback, OpenPgpError.USER_INTERACTION_REQUIRED, e.getMessage()); + } catch (WrongPassphraseException e) { + callbackOpenPgpError(callback, OpenPgpError.NO_OR_WRONG_PASSPHRASE, e.getMessage()); } catch (Exception e) { - Log.e(Constants.TAG, "KeychainService, Exception!", e); - - try { - callback.onError(new OpenPgpError(0, e.getMessage())); - } catch (Exception t) { - Log.e(Constants.TAG, "Error returning exception to client", t); - } + callbackOpenPgpError(callback, OpenPgpError.GENERIC_ERROR, e.getMessage()); } } @@ -347,13 +358,8 @@ public class OpenPgpService extends RemoteService { // TODO: This allows to decrypt messages with ALL secret keys, not only the one for the // app, Fix this? - // long secretKeyId = PgpMain.getDecryptionKeyId(getContext(), inputStream); - // if (secretKeyId == Id.key.none) { - // throw new PgpMain.PgpGeneralException(getString(R.string.error_noSecretKeyFound)); - // } String passphrase = null; - boolean assumeSymmetricEncryption = false; if (!signedOnly) { // BEGIN Get key // TODO: this input stream is consumed after PgpMain.getDecryptionKeyId()... do it @@ -361,7 +367,6 @@ public class OpenPgpService extends RemoteService { InputStream inputStream2 = new ByteArrayInputStream(inputBytes); // TODO: duplicates functions from DecryptActivity! - // TODO: we need activity to input symmetric passphrase long secretKeyId; try { if (inputStream2.markSupported()) { @@ -374,7 +379,6 @@ public class OpenPgpService extends RemoteService { if (secretKeyId == Id.key.none) { throw new PgpGeneralException(getString(R.string.error_noSecretKeyFound)); } - assumeSymmetricEncryption = false; } catch (NoAsymmetricEncryptionException e) { if (inputStream2.markSupported()) { inputStream2.reset(); @@ -384,16 +388,15 @@ public class OpenPgpService extends RemoteService { throw new PgpGeneralException( getString(R.string.error_noKnownEncryptionFound)); } - assumeSymmetricEncryption = true; + // we do not support symmetric decryption from the API! + throw new Exception("Symmetric decryption is not supported!"); } Log.d(Constants.TAG, "secretKeyId " + secretKeyId); - passphrase = getCachedPassphrase(secretKeyId); + passphrase = getCachedPassphrase(secretKeyId, allowUserInteraction); if (passphrase == null) { - callback.onError(new OpenPgpError(OpenPgpError.ID_NO_OR_WRONG_PASSPHRASE, - "No or wrong passphrase!")); - return; + throw new WrongPassphraseException("No or wrong passphrase!"); } } @@ -410,8 +413,7 @@ public class OpenPgpService extends RemoteService { // TODO: download missing keys from keyserver? outputBundle = operation.verifyText(false); } else { - // TODO: assume symmetric: callback to enter symmetric pass - outputBundle = operation.decryptAndVerify(passphrase, assumeSymmetricEncryption); + outputBundle = operation.decryptAndVerify(passphrase, false); } outputStream.close(); @@ -444,14 +446,27 @@ public class OpenPgpService extends RemoteService { // return over handler on client side callback.onSuccess(outputBytes, sigResult); + } catch (UserInteractionRequiredException e) { + callbackOpenPgpError(callback, OpenPgpError.USER_INTERACTION_REQUIRED, e.getMessage()); + } catch (WrongPassphraseException e) { + callbackOpenPgpError(callback, OpenPgpError.NO_OR_WRONG_PASSPHRASE, e.getMessage()); } catch (Exception e) { - Log.e(Constants.TAG, "KeychainService, Exception!", e); + callbackOpenPgpError(callback, OpenPgpError.GENERIC_ERROR, e.getMessage()); + } + } - try { - callback.onError(new OpenPgpError(0, e.getMessage())); - } catch (Exception t) { - Log.e(Constants.TAG, "Error returning exception to client", t); - } + /** + * Returns error to IOpenPgpCallback + * + * @param callback + * @param errorId + * @param message + */ + private void callbackOpenPgpError(IOpenPgpCallback callback, int errorId, String message) { + try { + callback.onError(new OpenPgpError(0, message)); + } catch (Exception t) { + Log.e(Constants.TAG, "Error returning exception to client", t); } } diff --git a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/remote/RemoteService.java b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/remote/RemoteService.java index 1db9b5f66..4e8c4678a 100644 --- a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/remote/RemoteService.java +++ b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/remote/RemoteService.java @@ -112,10 +112,8 @@ public abstract class RemoteService extends Service { extras.putString(RemoteServiceActivity.EXTRA_PACKAGE_NAME, callingPackages[0]); RegisterActivityCallback callback = new RegisterActivityCallback(); - Messenger messenger = new Messenger(new Handler(getMainLooper(), callback)); - pauseQueueAndStartServiceActivity(RemoteServiceActivity.ACTION_REGISTER, messenger, - extras); + pauseAndStartUserInteraction(RemoteServiceActivity.ACTION_REGISTER, callback, extras); if (callback.isAllowed()) { mThreadPool.execute(r); @@ -133,8 +131,7 @@ public abstract class RemoteService extends Service { * @param messenger * @param extras */ - protected void pauseQueueAndStartServiceActivity(String action, Messenger messenger, - Bundle extras) { + protected void pauseAndStartUserInteraction(String action, BaseCallback callback, Bundle extras) { synchronized (userInputLock) { mThreadPool.pause(); @@ -143,6 +140,8 @@ public abstract class RemoteService extends Service { intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK); intent.setAction(action); + Messenger messenger = new Messenger(new Handler(getMainLooper(), callback)); + extras.putParcelable(RemoteServiceActivity.EXTRA_MESSENGER, messenger); intent.putExtras(extras);