From 78c3e17d0a27fe9cbeea63aa6b256b3ea4b84675 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 27 Apr 2017 21:48:54 +0200 Subject: [PATCH] improve presentation of security problems --- .../RemoteSecurityProblemDialogActivity.java | 61 ++++++++--- .../remote/ui/SecurityProblemPresenter.java | 103 +++++++++++------- .../layout/remote_security_issue_dialog.xml | 9 ++ OpenKeychain/src/main/res/values/strings.xml | 21 +++- 4 files changed, 129 insertions(+), 65 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/ui/RemoteSecurityProblemDialogActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/ui/RemoteSecurityProblemDialogActivity.java index 2426e572e..ff100390c 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/ui/RemoteSecurityProblemDialogActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/ui/RemoteSecurityProblemDialogActivity.java @@ -83,6 +83,7 @@ public class RemoteSecurityProblemDialogActivity extends FragmentActivity { private RemoteSecurityProblemView mvpView; private Button buttonGotIt; + private Button buttonViewKey; @NonNull @Override @@ -97,6 +98,7 @@ public class RemoteSecurityProblemDialogActivity extends FragmentActivity { alert.setView(view); buttonGotIt = (Button) view.findViewById(R.id.button_allow); + buttonViewKey = (Button) view.findViewById(R.id.button_view_key); setupListenersForPresenter(); mvpView = createMvpView(view); @@ -168,6 +170,11 @@ public class RemoteSecurityProblemDialogActivity extends FragmentActivity { recommendLayout.setVisibility(View.GONE); } + private void showGeneric(String explanationString) { + explanationText.setText(explanationString); + recommendLayout.setVisibility(View.GONE); + } + private void showGenericWithRecommendation( @StringRes int explanationStringRes, @StringRes int recommendationStringRes) { explanationText.setText(explanationStringRes); @@ -175,59 +182,73 @@ public class RemoteSecurityProblemDialogActivity extends FragmentActivity { recommendLayout.setVisibility(View.VISIBLE); } + private void showGenericWithRecommendation( + String explanationString, @StringRes int recommendationStringRes) { + explanationText.setText(explanationString); + recommendText.setText(recommendationStringRes); + recommendLayout.setVisibility(View.VISIBLE); + } + @Override public void showLayoutMissingMdc() { - showGenericWithRecommendation(R.string.insecure_msg_mdc, R.string.insecure_recom_mdc); + showGenericWithRecommendation(R.string.insecure_mdc, R.string.insecure_mdc_suggestion); } @Override public void showLayoutInsecureSymmetric(int symmetricAlgorithm) { - showGeneric(R.string.insecure_msg_unidentified_key); + showGeneric(R.string.insecure_symmetric_algo); } @Override public void showLayoutInsecureHashAlgorithm(int hashAlgorithm) { - showGeneric(R.string.insecure_msg_unidentified_key); + showGeneric(R.string.insecure_hash_algo); } @Override public void showLayoutEncryptInsecureBitsize(int algorithmId, int bitStrength) { String algorithmName = KeyFormattingUtils.getAlgorithmInfo(algorithmId, null, null); - explanationText.setText( - getString(R.string.insecure_msg_bitstrength, algorithmName, - Integer.toString(bitStrength), "2010")); - recommendText.setText(R.string.insecure_recom_new_key); - recommendText.setVisibility(View.VISIBLE); + + showGenericWithRecommendation( + getString(R.string.insecure_encrypt_bitstrength, algorithmName, + Integer.toString(bitStrength), "2010"), + R.string.insecure_sign_bitstrength_suggestion); } @Override public void showLayoutSignInsecureBitsize(int algorithmId, int bitStrength) { String algorithmName = KeyFormattingUtils.getAlgorithmInfo(algorithmId, null, null); - explanationText.setText( - getString(R.string.insecure_msg_bitstrength, algorithmName, - Integer.toString(bitStrength), "2010")); - recommendText.setText(R.string.insecure_recom_new_key); - recommendText.setVisibility(View.VISIBLE); + + showGenericWithRecommendation( + getString(R.string.insecure_sign_bitstrength, algorithmName, + Integer.toString(bitStrength), "2010"), + R.string.insecure_sign_bitstrength_suggestion); } @Override public void showLayoutEncryptNotWhitelistedCurve(String curveOid) { - showGeneric(R.string.insecure_msg_not_whitelisted_curve); + showGeneric(getString(R.string.insecure_encrypt_not_whitelisted_curve, + KeyFormattingUtils.getCurveInfo(getContext(), curveOid))); } @Override public void showLayoutSignNotWhitelistedCurve(String curveOid) { - showGeneric(R.string.insecure_msg_not_whitelisted_curve); + showGeneric(getString(R.string.insecure_sign_not_whitelisted_curve, + KeyFormattingUtils.getCurveInfo(getContext(), curveOid))); } @Override public void showLayoutEncryptUnidentifiedKeyProblem() { - showGeneric(R.string.insecure_msg_unidentified_key); + showGeneric(R.string.insecure_encrypt_unidentified); } @Override public void showLayoutSignUnidentifiedKeyProblem() { - showGeneric(R.string.insecure_msg_unidentified_key); + showGeneric(R.string.insecure_sign_unidentified); + } + + @Override + public void showViewKeyButton() { + buttonViewKey.setVisibility(View.VISIBLE); } }; } @@ -239,6 +260,12 @@ public class RemoteSecurityProblemDialogActivity extends FragmentActivity { presenter.onClickGotIt(); } }); + buttonViewKey.setOnClickListener(new OnClickListener() { + @Override + public void onClick(View view) { + presenter.onClickViewKey(); + } + }); } } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/ui/SecurityProblemPresenter.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/ui/SecurityProblemPresenter.java index bbc5786a5..51ec46fd0 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/ui/SecurityProblemPresenter.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/ui/SecurityProblemPresenter.java @@ -4,32 +4,36 @@ package org.sufficientlysecure.keychain.remote.ui; import java.io.Serializable; import android.content.Context; +import android.content.Intent; import android.content.pm.ApplicationInfo; import android.content.pm.PackageManager; import android.content.pm.PackageManager.NameNotFoundException; import android.graphics.drawable.Drawable; -import org.sufficientlysecure.keychain.Constants; +import org.sufficientlysecure.keychain.pgp.DecryptVerifySecurityProblem; import org.sufficientlysecure.keychain.pgp.SecurityProblem.InsecureBitStrength; -import org.sufficientlysecure.keychain.pgp.SecurityProblem.InsecureHashAlgorithm; -import org.sufficientlysecure.keychain.pgp.SecurityProblem.InsecureSymmetricAlgorithm; +import org.sufficientlysecure.keychain.pgp.SecurityProblem.InsecureSigningAlgorithm; +import org.sufficientlysecure.keychain.pgp.SecurityProblem.InsecureEncryptionAlgorithm; import org.sufficientlysecure.keychain.pgp.SecurityProblem.KeySecurityProblem; import org.sufficientlysecure.keychain.pgp.SecurityProblem.MissingMdc; import org.sufficientlysecure.keychain.pgp.SecurityProblem.NotWhitelistedCurve; -import org.sufficientlysecure.keychain.pgp.SecurityProblem.SymmetricAlgorithmProblem; +import org.sufficientlysecure.keychain.pgp.SecurityProblem.EncryptionAlgorithmProblem; import org.sufficientlysecure.keychain.pgp.SecurityProblem.UnidentifiedKeyProblem; -import org.sufficientlysecure.keychain.pgp.SecurityProblem.UsageType; -import org.sufficientlysecure.keychain.util.Log; +import org.sufficientlysecure.keychain.provider.KeychainContract.KeyRings; +import org.sufficientlysecure.keychain.ui.ViewKeyActivity; class SecurityProblemPresenter { + private final Context context; private final PackageManager packageManager; private RemoteSecurityProblemView view; + private Long viewKeyMasterKeyId; SecurityProblemPresenter(Context context) { + this.context = context; packageManager = context.getPackageManager(); } @@ -38,13 +42,8 @@ class SecurityProblemPresenter { } void setupFromIntentData(String packageName, Serializable securityProblem) { - - if (securityProblem instanceof KeySecurityProblem) { -// setupFromKeySecurityProblem((KeySecurityProblem) securityProblem); - } else if (securityProblem instanceof SymmetricAlgorithmProblem) { - setupFromNonKeySecurityProblem((SymmetricAlgorithmProblem) securityProblem); - } else if (securityProblem instanceof InsecureHashAlgorithm) { - setupFromInsecureHashAlgorithm((InsecureHashAlgorithm) securityProblem); + if (securityProblem instanceof DecryptVerifySecurityProblem) { + setupFromDecryptVerifySecurityProblem((DecryptVerifySecurityProblem) securityProblem); } else { throw new IllegalArgumentException("Unhandled security problem type!"); } @@ -56,53 +55,65 @@ class SecurityProblemPresenter { } } - /* - private void setupFromKeySecurityProblem(KeySecurityProblem keySecurityProblem) { + private void setupFromDecryptVerifySecurityProblem(DecryptVerifySecurityProblem securityProblem) { + if (securityProblem.encryptionKeySecurityProblem != null) { + setupFromEncryptionKeySecurityProblem(securityProblem.encryptionKeySecurityProblem); + } else if (securityProblem.signingKeySecurityProblem != null) { + setupFromSigningKeySecurityProblem(securityProblem.signingKeySecurityProblem); + } else if (securityProblem.symmetricSecurityProblem != null) { + setupFromEncryptionAlgorithmSecurityProblem(securityProblem.symmetricSecurityProblem); + } else if (securityProblem.signatureSecurityProblem != null) { + setupFromSignatureSecurityProblem(securityProblem.signatureSecurityProblem); + } + } + + private void setupFromEncryptionKeySecurityProblem(KeySecurityProblem keySecurityProblem) { + viewKeyMasterKeyId = keySecurityProblem.masterKeyId; + view.showViewKeyButton(); + if (keySecurityProblem instanceof InsecureBitStrength) { InsecureBitStrength problem = (InsecureBitStrength) keySecurityProblem; - if (problem.usageType == UsageType.ENCRYPT) { - view.showLayoutEncryptInsecureBitsize(problem.algorithm, problem.bitStrength); - } else if (problem.usageType == UsageType.SIGN) { - view.showLayoutSignInsecureBitsize(problem.algorithm, problem.bitStrength); - } else { - throw new IllegalStateException("Should never happen here!"); - } + view.showLayoutEncryptInsecureBitsize(problem.algorithm, problem.bitStrength); } else if (keySecurityProblem instanceof NotWhitelistedCurve) { NotWhitelistedCurve problem = (NotWhitelistedCurve) keySecurityProblem; - if (problem.usageType == UsageType.ENCRYPT) { - view.showLayoutEncryptNotWhitelistedCurve(problem.curveOid); - } else if (problem.usageType == UsageType.SIGN) { - view.showLayoutSignNotWhitelistedCurve(problem.curveOid); - } else { - throw new IllegalStateException("Should never happen here!"); - } + view.showLayoutEncryptNotWhitelistedCurve(problem.curveOid); } else if (keySecurityProblem instanceof UnidentifiedKeyProblem) { - if (keySecurityProblem.usageType == UsageType.ENCRYPT) { - view.showLayoutEncryptUnidentifiedKeyProblem(); - } else if (keySecurityProblem.usageType == UsageType.SIGN) { - view.showLayoutSignUnidentifiedKeyProblem(); - } else { - throw new IllegalStateException("Should never happen here!"); - } + view.showLayoutEncryptUnidentifiedKeyProblem(); } else { throw new IllegalArgumentException("Unhandled key security problem type!"); } } - */ - private void setupFromNonKeySecurityProblem(SymmetricAlgorithmProblem securityProblem) { + private void setupFromSigningKeySecurityProblem(KeySecurityProblem keySecurityProblem) { + viewKeyMasterKeyId = keySecurityProblem.masterKeyId; + view.showViewKeyButton(); + + if (keySecurityProblem instanceof InsecureBitStrength) { + InsecureBitStrength problem = (InsecureBitStrength) keySecurityProblem; + view.showLayoutSignInsecureBitsize(problem.algorithm, problem.bitStrength); + } else if (keySecurityProblem instanceof NotWhitelistedCurve) { + NotWhitelistedCurve problem = (NotWhitelistedCurve) keySecurityProblem; + view.showLayoutSignNotWhitelistedCurve(problem.curveOid); + } else if (keySecurityProblem instanceof UnidentifiedKeyProblem) { + view.showLayoutSignUnidentifiedKeyProblem(); + } else { + throw new IllegalArgumentException("Unhandled key security problem type!"); + } + } + + private void setupFromEncryptionAlgorithmSecurityProblem(EncryptionAlgorithmProblem securityProblem) { if (securityProblem instanceof MissingMdc) { view.showLayoutMissingMdc(); - } else if (securityProblem instanceof InsecureSymmetricAlgorithm) { - InsecureSymmetricAlgorithm insecureSymmetricAlgorithm = (InsecureSymmetricAlgorithm) securityProblem; + } else if (securityProblem instanceof InsecureEncryptionAlgorithm) { + InsecureEncryptionAlgorithm insecureSymmetricAlgorithm = (InsecureEncryptionAlgorithm) securityProblem; view.showLayoutInsecureSymmetric(insecureSymmetricAlgorithm.symmetricAlgorithm); } else { throw new IllegalArgumentException("Unhandled symmetric algorithm problem type!"); } } - private void setupFromInsecureHashAlgorithm(InsecureHashAlgorithm securityProblem) { - view.showLayoutInsecureHashAlgorithm(securityProblem.hashAlgorithm); + private void setupFromSignatureSecurityProblem(InsecureSigningAlgorithm signatureSecurityProblem) { + view.showLayoutInsecureHashAlgorithm(signatureSecurityProblem.hashAlgorithm); } private void setPackageInfo(String packageName) throws NameNotFoundException { @@ -117,6 +128,12 @@ class SecurityProblemPresenter { view.finishAsCancelled(); } + void onClickViewKey() { + Intent viewKeyIntent = new Intent(context, ViewKeyActivity.class); + viewKeyIntent.setData(KeyRings.buildGenericKeyRingUri(viewKeyMasterKeyId)); + context.startActivity(viewKeyIntent); + } + void onCancel() { view.finishAsCancelled(); } @@ -136,5 +153,7 @@ class SecurityProblemPresenter { void showLayoutInsecureSymmetric(int symmetricAlgorithm); void showLayoutInsecureHashAlgorithm(int hashAlgorithm); + + void showViewKeyButton(); } } diff --git a/OpenKeychain/src/main/res/layout/remote_security_issue_dialog.xml b/OpenKeychain/src/main/res/layout/remote_security_issue_dialog.xml index 1eeef98c8..cc6575370 100644 --- a/OpenKeychain/src/main/res/layout/remote_security_issue_dialog.xml +++ b/OpenKeychain/src/main/res/layout/remote_security_issue_dialog.xml @@ -82,6 +82,15 @@ tools:layout_marginBottom="50dp" style="?buttonBarStyle"> +