From 055fb59ec34a986ea9f5620554b51c438d809361 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Mon, 8 Sep 2014 14:32:55 +0200 Subject: [PATCH] Properly handle uneditable key types in EditKeyFragment More specifically, return an OperationResultParcel for stripped and divert-to-card keys, with an appropriate error message in the log. part of #828 --- .../keychain/pgp/PgpKeyOperation.java | 4 ++ .../service/OperationResultParcel.java | 6 +++ .../keychain/ui/EditKeyFragment.java | 48 +++++++++++++++---- OpenKeychain/src/main/res/values/strings.xml | 5 ++ 4 files changed, 54 insertions(+), 9 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java index ddb00305a..3f5528b05 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java @@ -381,6 +381,10 @@ public class PgpKeyOperation { * This is a natural workflow since pgp keyrings are immutable data structures: Old semantics * are changed by adding new certificates, which implicitly override older certificates. * + * Note that this method does not care about any "special" type of master key. If unlocking + * with a passphrase fails, the operation will fail with an unlocking error. More specific + * handling of errors should be done in UI code! + * */ public EditKeyResult modifySecretKeyRing(CanonicalizedSecretKeyRing wsKR, SaveKeyringParcel saveParcel, String passphrase) { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java index 438443644..fefc3e4b1 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java @@ -427,6 +427,12 @@ public class OperationResultParcel implements Parcelable { MSG_CON_SUCCESS (R.string.msg_con_success), MSG_CON_WARN_DELETE_PUBLIC (R.string.msg_con_warn_delete_public), MSG_CON_WARN_DELETE_SECRET (R.string.msg_con_warn_delete_secret), + + // messages used in UI code + MSG_EK_ERROR_DIVERT (R.string.msg_ek_error_divert), + MSG_EK_ERROR_DUMMY (R.string.msg_ek_error_dummy), + MSG_EK_ERROR_NOT_FOUND (R.string.msg_ek_error_not_found), + ; private final int mMsgId; diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java index 830e856aa..3de01adcd 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java @@ -35,7 +35,6 @@ import android.view.View.OnClickListener; import android.view.ViewGroup; import android.widget.AdapterView; import android.widget.ListView; -import android.widget.Toast; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.R; @@ -46,9 +45,13 @@ import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException; import org.sufficientlysecure.keychain.provider.CachedPublicKeyRing; import org.sufficientlysecure.keychain.provider.KeychainContract; import org.sufficientlysecure.keychain.provider.ProviderHelper; +import org.sufficientlysecure.keychain.provider.ProviderHelper.NotFoundException; import org.sufficientlysecure.keychain.service.KeychainIntentService; import org.sufficientlysecure.keychain.service.KeychainIntentServiceHandler; import org.sufficientlysecure.keychain.service.OperationResultParcel; +import org.sufficientlysecure.keychain.service.OperationResultParcel.LogLevel; +import org.sufficientlysecure.keychain.service.OperationResultParcel.LogType; +import org.sufficientlysecure.keychain.service.OperationResultParcel.OperationLog; import org.sufficientlysecure.keychain.service.PassphraseCacheService; import org.sufficientlysecure.keychain.service.SaveKeyringParcel; import org.sufficientlysecure.keychain.ui.adapter.SubkeysAdapter; @@ -169,15 +172,26 @@ public class EditKeyFragment extends LoaderFragment implements Uri secretUri = KeychainContract.KeyRings.buildUnifiedKeyRingUri(mDataUri); CachedPublicKeyRing keyRing = new ProviderHelper(getActivity()).getCachedPublicKeyRing(secretUri); + long masterKeyId = keyRing.getMasterKeyId(); - mSaveKeyringParcel = new SaveKeyringParcel(keyRing.getMasterKeyId(), - keyRing.getFingerprint()); + // check if this is a master secret key we can work with + switch (keyRing.getSecretKeyType(masterKeyId)) { + case GNU_DUMMY: + finishWithError(LogType.MSG_EK_ERROR_DUMMY); + return; + case DIVERT_TO_CARD: + finishWithError(LogType.MSG_EK_ERROR_DIVERT); + break; + } + + mSaveKeyringParcel = new SaveKeyringParcel(masterKeyId, keyRing.getFingerprint()); mPrimaryUserId = keyRing.getPrimaryUserIdWithFallback(); + } catch (NotFoundException e) { + finishWithError(LogType.MSG_EK_ERROR_NOT_FOUND); + return; } catch (PgpGeneralException e) { - Log.e(Constants.TAG, "PgpGeneralException", e); - Toast.makeText(getActivity(), R.string.error_no_secret_key_found, Toast.LENGTH_SHORT).show(); - getActivity().finish(); + finishWithError(LogType.MSG_EK_ERROR_NOT_FOUND); return; } @@ -185,9 +199,7 @@ public class EditKeyFragment extends LoaderFragment implements mCurrentPassphrase = PassphraseCacheService.getCachedPassphrase(getActivity(), mSaveKeyringParcel.mMasterKeyId); } catch (PassphraseCacheService.KeyNotFoundException e) { - Log.e(Constants.TAG, "Key not found!", e); - Toast.makeText(getActivity(), R.string.error_no_secret_key_found, Toast.LENGTH_SHORT).show(); - getActivity().finish(); + finishWithError(LogType.MSG_EK_ERROR_NOT_FOUND); return; } @@ -564,4 +576,22 @@ public class EditKeyFragment extends LoaderFragment implements getActivity().startService(intent); } + + /** Closes this activity, returning a result parcel with a single error log entry. */ + void finishWithError(LogType reason) { + + // Prepare the log + OperationLog log = new OperationLog(); + log.add(LogLevel.ERROR, reason, 0); + + // Prepare an intent with an EXTRA_RESULT + Intent intent = new Intent(); + intent.putExtra(OperationResultParcel.EXTRA_RESULT, + new OperationResultParcel(OperationResultParcel.RESULT_ERROR, log)); + + // Finish with result + getActivity().setResult(EditKeyActivity.RESULT_OK, intent); + getActivity().finish(); + } + } diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml index 24effb7f1..2a89f15bc 100644 --- a/OpenKeychain/src/main/res/values/strings.xml +++ b/OpenKeychain/src/main/res/values/strings.xml @@ -756,6 +756,11 @@ "Exception deleting public cache file" "Exception deleting secret cache file" + + "editing of nfc keys is not (yet) supported" + "cannot edit keyring with stripped master key!" + "key not found" + "Click to clear cached passphrases" "OpenKeychain has cached %d passphrases"