From 1e8d5bdad3b114ff7e43e7de42d7744122548483 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Tue, 14 Feb 2017 17:55:06 +0100 Subject: [PATCH] use info from SecurityProblem class in health card --- .../keychain/pgp/PgpSecurityConstants.java | 2 +- .../keychain/ui/util/KeyFormattingUtils.java | 4 ++ .../keychain/ui/widget/KeyHealthCardView.java | 33 ++++++++++++- .../ui/widget/KeyHealthPresenter.java | 40 ++++++++++------ .../ui/widget/SubkeyStatusLoader.java | 23 ++++++--- .../res/layout/key_health_card_content.xml | 48 ++++++++++++++++++- OpenKeychain/src/main/res/values/strings.xml | 6 +++ 7 files changed, 132 insertions(+), 24 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSecurityConstants.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSecurityConstants.java index 35ac9058a..acd023a7c 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSecurityConstants.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSecurityConstants.java @@ -145,7 +145,7 @@ public class PgpSecurityConstants { } @Nullable - private static KeySecurityProblem getKeySecurityProblem(long masterKeyId, long subKeyId, int algorithm, + public static KeySecurityProblem getKeySecurityProblem(long masterKeyId, long subKeyId, int algorithm, Integer bitStrength, String curveOid) { switch (algorithm) { case PublicKeyAlgorithmTags.RSA_GENERAL: { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/util/KeyFormattingUtils.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/util/KeyFormattingUtils.java index 279fe1832..c6c4b6954 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/util/KeyFormattingUtils.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/util/KeyFormattingUtils.java @@ -62,6 +62,10 @@ public class KeyFormattingUtils { return getAlgorithmInfo(null, algorithm, keySize, oid); } + public static String getAlgorithmInfo(int algorithm) { + return getAlgorithmInfo(null, algorithm, null, null); + } + /** * Based on OpenPGP Message Format */ diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/widget/KeyHealthCardView.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/widget/KeyHealthCardView.java index a2f904974..1f321610c 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/widget/KeyHealthCardView.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/widget/KeyHealthCardView.java @@ -32,6 +32,10 @@ import android.widget.ImageView; import android.widget.TextView; import org.sufficientlysecure.keychain.R; +import org.sufficientlysecure.keychain.pgp.SecurityProblem.InsecureBitStrength; +import org.sufficientlysecure.keychain.pgp.SecurityProblem.KeySecurityProblem; +import org.sufficientlysecure.keychain.pgp.SecurityProblem.NotWhitelistedCurve; +import org.sufficientlysecure.keychain.ui.util.KeyFormattingUtils; import org.sufficientlysecure.keychain.ui.widget.KeyHealthPresenter.KeyHealthClickListener; import org.sufficientlysecure.keychain.ui.widget.KeyHealthPresenter.KeyHealthMvpView; import org.sufficientlysecure.keychain.ui.widget.KeyHealthPresenter.KeyHealthStatus; @@ -45,6 +49,9 @@ public class KeyHealthCardView extends CardView implements KeyHealthMvpView, OnC private final ImageView vExpander; private final KeyStatusList vKeyStatusList; private final View vKeyStatusDivider; + private final View vInsecureLayout; + private final TextView vInsecureProblem; + private final TextView vInsecureSolution; private KeyHealthClickListener keyHealthClickListener; @@ -63,9 +70,13 @@ public class KeyHealthCardView extends CardView implements KeyHealthMvpView, OnC vKeyStatusDivider = view.findViewById(R.id.key_health_divider); vKeyStatusList = (KeyStatusList) view.findViewById(R.id.key_health_status_list); + + vInsecureLayout = view.findViewById(R.id.key_insecure_layout); + vInsecureProblem = (TextView) view.findViewById(R.id.key_insecure_problem); + vInsecureSolution = (TextView) view.findViewById(R.id.key_insecure_solution); } - enum KeyHealthDisplayStatus { + private enum KeyHealthDisplayStatus { OK (R.string.key_health_ok_title, R.string.key_health_ok_subtitle, R.drawable.ic_check_black_24dp, R.color.android_green_light), DIVERT (R.string.key_health_divert_title, R.string.key_health_divert_subtitle, @@ -134,6 +145,26 @@ public class KeyHealthCardView extends CardView implements KeyHealthMvpView, OnC } } + @Override + public void setPrimarySecurityProblem(KeySecurityProblem securityProblem) { + vInsecureLayout.setVisibility(View.VISIBLE); + + if (securityProblem instanceof InsecureBitStrength) { + InsecureBitStrength insecureBitStrength = (InsecureBitStrength) securityProblem; + vInsecureProblem.setText(getResources().getString(R.string.key_insecure_bitstrength_2048_problem, + KeyFormattingUtils.getAlgorithmInfo(insecureBitStrength.algorithm), + Integer.toString(insecureBitStrength.bitStrength))); + vInsecureSolution.setText(R.string.key_insecure_bitstrength_2048_solution); + } else if (securityProblem instanceof NotWhitelistedCurve) { + NotWhitelistedCurve notWhitelistedCurve = (NotWhitelistedCurve) securityProblem; + + String curveName = KeyFormattingUtils.getCurveInfo(getContext(), notWhitelistedCurve.curveOid); + vInsecureProblem.setText(getResources().getString(R.string.key_insecure_unknown_curve_problem, curveName)); + vInsecureSolution.setText(R.string.key_insecure_unknown_curve_solution); + } + + } + @Override public void onClick(View view) { if (keyHealthClickListener != null) { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/widget/KeyHealthPresenter.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/widget/KeyHealthPresenter.java index d1f500781..6ba2ae0e0 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/widget/KeyHealthPresenter.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/widget/KeyHealthPresenter.java @@ -27,6 +27,7 @@ import android.support.v4.app.LoaderManager.LoaderCallbacks; import android.support.v4.content.Loader; import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKey.SecretKeyType; +import org.sufficientlysecure.keychain.pgp.SecurityProblem.KeySecurityProblem; import org.sufficientlysecure.keychain.ui.widget.KeyStatusList.KeyDisplayStatus; import org.sufficientlysecure.keychain.ui.widget.SubkeyStatusLoader.KeySubkeyStatus; import org.sufficientlysecure.keychain.ui.widget.SubkeyStatusLoader.SubKeyItem; @@ -45,8 +46,8 @@ public class KeyHealthPresenter implements LoaderCallbacks { if (usability != 0) { return usability; } - if (one.mIsSecure ^ two.mIsSecure) { - return one.mIsSecure ? -1 : 1; + if ((one.mSecurityProblem == null) ^ (two.mSecurityProblem == null)) { + return one.mSecurityProblem == null ? -1 : 1; } // otherwise, the newer one comes first return one.newerThan(two) ? -1 : 1; @@ -94,11 +95,19 @@ public class KeyHealthPresenter implements LoaderCallbacks { this.subkeyStatus = subkeyStatus; KeyHealthStatus keyHealthStatus = determineKeyHealthStatus(subkeyStatus); - boolean forceExpanded = keyHealthStatus == KeyHealthStatus.INSECURE; - if (forceExpanded) { - view.setKeyStatus(keyHealthStatus); - view.setShowExpander(false); - displayExpandedInfo(false); + + boolean isInsecure = keyHealthStatus == KeyHealthStatus.INSECURE; + if (isInsecure) { + boolean primaryKeySecurityProblem = subkeyStatus.keyCertify.mSecurityProblem != null; + if (primaryKeySecurityProblem) { + view.setKeyStatus(keyHealthStatus); + view.setPrimarySecurityProblem(subkeyStatus.keyCertify.mSecurityProblem); + view.setShowExpander(false); + } else { + view.setKeyStatus(keyHealthStatus); + view.setShowExpander(false); + displayExpandedInfo(false); + } } else { view.setKeyStatus(keyHealthStatus); view.setShowExpander( @@ -116,7 +125,7 @@ public class KeyHealthPresenter implements LoaderCallbacks { return KeyHealthStatus.EXPIRED; } - if (!keyCertify.mIsSecure) { + if (keyCertify.mSecurityProblem != null) { return KeyHealthStatus.INSECURE; } @@ -126,7 +135,7 @@ public class KeyHealthPresenter implements LoaderCallbacks { return KeyHealthStatus.SPECIAL; } - if (!keySign.mIsSecure) { + if (keySign.mSecurityProblem != null) { return KeyHealthStatus.INSECURE; } @@ -140,8 +149,8 @@ public class KeyHealthPresenter implements LoaderCallbacks { SubKeyItem keySign = subkeyStatus.keysSign.get(0); SubKeyItem keyEncrypt = subkeyStatus.keysEncrypt.get(0); - if (!keySign.mIsSecure && keySign.isValid() - || !keyEncrypt.mIsSecure && keyEncrypt.isValid()) { + if (keySign.mSecurityProblem != null && keySign.isValid() + || keyEncrypt.mSecurityProblem != null && keyEncrypt.isValid()) { return KeyHealthStatus.INSECURE; } @@ -224,7 +233,7 @@ public class KeyHealthPresenter implements LoaderCallbacks { if (subKeyItem.mIsExpired) { return KeyDisplayStatus.EXPIRED; } - if (!subKeyItem.mIsSecure) { + if (subKeyItem.mSecurityProblem != null) { return KeyDisplayStatus.INSECURE; } if (subKeyItem.mSecretKeyType == SecretKeyType.GNU_DUMMY) { @@ -243,13 +252,14 @@ public class KeyHealthPresenter implements LoaderCallbacks { interface KeyHealthMvpView { void setKeyStatus(KeyHealthStatus keyHealthStatus); - void setShowExpander(boolean showExpander); - void setOnHealthClickListener(KeyHealthClickListener keyHealthClickListener); + void setPrimarySecurityProblem(KeySecurityProblem securityProblem); + void setShowExpander(boolean showExpander); void showExpandedState(KeyDisplayStatus certifyStatus, KeyDisplayStatus signStatus, KeyDisplayStatus encryptStatus); - void hideExpandedInfo(); + + void setOnHealthClickListener(KeyHealthClickListener keyHealthClickListener); } interface KeyStatusMvpView { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/widget/SubkeyStatusLoader.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/widget/SubkeyStatusLoader.java index 02c7782bd..748f38c95 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/widget/SubkeyStatusLoader.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/widget/SubkeyStatusLoader.java @@ -16,6 +16,8 @@ import android.util.Log; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKey.SecretKeyType; +import org.sufficientlysecure.keychain.pgp.PgpSecurityConstants; +import org.sufficientlysecure.keychain.pgp.SecurityProblem.KeySecurityProblem; import org.sufficientlysecure.keychain.provider.KeychainContract.Keys; import org.sufficientlysecure.keychain.ui.widget.SubkeyStatusLoader.KeySubkeyStatus; @@ -30,7 +32,9 @@ class SubkeyStatusLoader extends AsyncTaskLoader { Keys.HAS_SECRET, Keys.EXPIRY, Keys.IS_REVOKED, - Keys.IS_SECURE + Keys.ALGORITHM, + Keys.KEY_SIZE, + Keys.KEY_CURVE_OID }; private static final int INDEX_KEY_ID = 0; private static final int INDEX_CREATION = 1; @@ -40,7 +44,9 @@ class SubkeyStatusLoader extends AsyncTaskLoader { private static final int INDEX_HAS_SECRET = 5; private static final int INDEX_EXPIRY = 6; private static final int INDEX_IS_REVOKED = 7; - private static final int INDEX_IS_SECURE = 8; + private static final int INDEX_ALGORITHM = 8; + private static final int INDEX_KEY_SIZE = 9; + private static final int INDEX_KEY_CURVE_OID = 10; private final ContentResolver contentResolver; @@ -71,7 +77,7 @@ class SubkeyStatusLoader extends AsyncTaskLoader { ArrayList keysSign = new ArrayList<>(); ArrayList keysEncrypt = new ArrayList<>(); while (cursor.moveToNext()) { - SubKeyItem ski = new SubKeyItem(cursor); + SubKeyItem ski = new SubKeyItem(masterKeyId, cursor); if (ski.mKeyId == masterKeyId) { keyCertify = ski; @@ -138,9 +144,9 @@ class SubkeyStatusLoader extends AsyncTaskLoader { final SecretKeyType mSecretKeyType; final boolean mIsRevoked, mIsExpired; final boolean mCanCertify, mCanSign, mCanEncrypt; - final boolean mIsSecure; + final KeySecurityProblem mSecurityProblem; - SubKeyItem(Cursor cursor) { + SubKeyItem(long masterKeyId, Cursor cursor) { mPosition = cursor.getPosition(); mKeyId = cursor.getLong(INDEX_KEY_ID); @@ -159,7 +165,12 @@ class SubkeyStatusLoader extends AsyncTaskLoader { mCanSign = cursor.getInt(INDEX_CAN_SIGN) > 0; mCanEncrypt = cursor.getInt(INDEX_CAN_ENCRYPT) > 0; - mIsSecure = cursor.getInt(INDEX_IS_SECURE) > 0; + int algorithm = cursor.getInt(INDEX_ALGORITHM); + Integer bitStrength = cursor.isNull(INDEX_KEY_SIZE) ? null : cursor.getInt(INDEX_KEY_SIZE); + String curveOid = cursor.getString(INDEX_KEY_CURVE_OID); + + mSecurityProblem = PgpSecurityConstants.getKeySecurityProblem( + masterKeyId, mKeyId, algorithm, bitStrength, curveOid); } boolean newerThan(SubKeyItem other) { diff --git a/OpenKeychain/src/main/res/layout/key_health_card_content.xml b/OpenKeychain/src/main/res/layout/key_health_card_content.xml index 11c7eff10..6fbb6ef6e 100644 --- a/OpenKeychain/src/main/res/layout/key_health_card_content.xml +++ b/OpenKeychain/src/main/res/layout/key_health_card_content.xml @@ -79,6 +79,52 @@ android:id="@+id/key_health_divider" android:background="?android:attr/listDivider" /> + + + + + + + + + + + + diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml index dfa22f56d..69ed653d7 100644 --- a/OpenKeychain/src/main/res/values/strings.xml +++ b/OpenKeychain/src/main/res/values/strings.xml @@ -1855,4 +1855,10 @@ "Healthy (Partially Stripped)" "Click for details" + "This key uses the %1$s algorithm with a strength of %2$s bits. A secure key should have a strength of 2048 bits." + "This key can\'t be upgraded. For secure communication, the owner must generate a new key." + + "This key uses the %1$s algorithm, which is not whitelisted." + "This key can\'t be upgraded. For secure communication, the owner must generate a new key." +