Merge pull request #2028 from rhari991/insecure-keys

Display insecure key warnings
This commit is contained in:
Dominik Schürmann 2017-02-03 13:36:44 +01:00 committed by GitHub
commit 015ef4aaae
12 changed files with 104 additions and 39 deletions

View file

@ -136,6 +136,10 @@ public class CanonicalizedPublicKey extends UncachedPublicKey {
return expiry != null && expiry.before(new Date());
}
public boolean isSecure() {
return PgpSecurityConstants.isSecureKey(this);
}
public long getValidSeconds() {
long seconds;

View file

@ -45,6 +45,7 @@ public class KeychainContract {
String CAN_CERTIFY = "can_certify";
String CAN_AUTHENTICATE = "can_authenticate";
String IS_REVOKED = "is_revoked";
String IS_SECURE = "is_secure";
String HAS_SECRET = "has_secret";
String CREATION = "creation";
@ -128,6 +129,7 @@ public class KeychainContract {
public static class KeyRings implements BaseColumns, KeysColumns, UserPacketsColumns {
public static final String MASTER_KEY_ID = KeysColumns.MASTER_KEY_ID;
public static final String IS_REVOKED = KeysColumns.IS_REVOKED;
public static final String IS_SECURE = KeysColumns.IS_SECURE;
public static final String VERIFIED = CertsColumns.VERIFIED;
public static final String IS_EXPIRED = "is_expired";
public static final String HAS_ANY_SECRET = "has_any_secret";

View file

@ -54,7 +54,7 @@ import java.io.IOException;
*/
public class KeychainDatabase extends SQLiteOpenHelper {
private static final String DATABASE_NAME = "openkeychain.db";
private static final int DATABASE_VERSION = 18;
private static final int DATABASE_VERSION = 19;
static Boolean apgHack = false;
private Context mContext;
@ -101,6 +101,7 @@ public class KeychainDatabase extends SQLiteOpenHelper {
+ KeysColumns.CAN_AUTHENTICATE + " INTEGER, "
+ KeysColumns.IS_REVOKED + " INTEGER, "
+ KeysColumns.HAS_SECRET + " INTEGER, "
+ KeysColumns.IS_SECURE + " INTEGER, "
+ KeysColumns.CREATION + " INTEGER, "
+ KeysColumns.EXPIRY + " INTEGER, "
@ -322,6 +323,8 @@ public class KeychainDatabase extends SQLiteOpenHelper {
// splitUserId changed: Execute consolidate for new parsing of name, email
case 17:
// splitUserId changed: Execute consolidate for new parsing of name, email
case 18:
db.execSQL("ALTER TABLE keys ADD COLUMN is_secure INTEGER");
}
// always do consolidate after upgrade

View file

@ -299,6 +299,7 @@ public class KeychainProvider extends ContentProvider {
projectionMap.put(KeyRings.KEY_SIZE, Tables.KEYS + "." + Keys.KEY_SIZE);
projectionMap.put(KeyRings.KEY_CURVE_OID, Tables.KEYS + "." + Keys.KEY_CURVE_OID);
projectionMap.put(KeyRings.IS_REVOKED, Tables.KEYS + "." + Keys.IS_REVOKED);
projectionMap.put(KeyRings.IS_SECURE, Tables.KEYS + "." + Keys.IS_SECURE);
projectionMap.put(KeyRings.CAN_CERTIFY, Tables.KEYS + "." + Keys.CAN_CERTIFY);
projectionMap.put(KeyRings.CAN_ENCRYPT, Tables.KEYS + "." + Keys.CAN_ENCRYPT);
projectionMap.put(KeyRings.CAN_SIGN, Tables.KEYS + "." + Keys.CAN_SIGN);
@ -383,6 +384,7 @@ public class KeychainProvider extends ContentProvider {
+"kE." + Keys.MASTER_KEY_ID
+ " = " + Tables.KEYS + "." + Keys.MASTER_KEY_ID
+ " AND kE." + Keys.IS_REVOKED + " = 0"
+ " AND kE." + Keys.IS_SECURE + " = 1"
+ " AND kE." + Keys.CAN_ENCRYPT + " = 1"
+ " AND ( kE." + Keys.EXPIRY + " IS NULL OR kE." + Keys.EXPIRY
+ " >= " + new Date().getTime() / 1000 + " )"
@ -392,6 +394,7 @@ public class KeychainProvider extends ContentProvider {
+"kS." + Keys.MASTER_KEY_ID
+ " = " + Tables.KEYS + "." + Keys.MASTER_KEY_ID
+ " AND kS." + Keys.IS_REVOKED + " = 0"
+ " AND kS." + Keys.IS_SECURE + " = 1"
+ " AND kS." + Keys.CAN_SIGN + " = 1"
+ " AND kS." + Keys.HAS_SECRET + " > 1"
+ " AND ( kS." + Keys.EXPIRY + " IS NULL OR kS." + Keys.EXPIRY
@ -402,6 +405,7 @@ public class KeychainProvider extends ContentProvider {
+"kA." + Keys.MASTER_KEY_ID
+ " = " + Tables.KEYS + "." + Keys.MASTER_KEY_ID
+ " AND kA." + Keys.IS_REVOKED + " = 0"
+ " AND kA." + Keys.IS_SECURE + " = 1"
+ " AND kA." + Keys.CAN_AUTHENTICATE + " = 1"
+ " AND kA." + Keys.HAS_SECRET + " > 1"
+ " AND ( kA." + Keys.EXPIRY + " IS NULL OR kA." + Keys.EXPIRY
@ -412,6 +416,7 @@ public class KeychainProvider extends ContentProvider {
+"kC." + Keys.MASTER_KEY_ID
+ " = " + Tables.KEYS + "." + Keys.MASTER_KEY_ID
+ " AND kC." + Keys.IS_REVOKED + " = 0"
+ " AND kC." + Keys.IS_SECURE + " = 1"
+ " AND kC." + Keys.CAN_CERTIFY + " = 1"
+ " AND kC." + Keys.HAS_SECRET + " > 1"
+ " AND ( kC." + Keys.EXPIRY + " IS NULL OR kC." + Keys.EXPIRY
@ -500,6 +505,7 @@ public class KeychainProvider extends ContentProvider {
projectionMap.put(Keys.KEY_SIZE, Keys.KEY_SIZE);
projectionMap.put(Keys.KEY_CURVE_OID, Keys.KEY_CURVE_OID);
projectionMap.put(Keys.IS_REVOKED, Tables.KEYS + "." + Keys.IS_REVOKED);
projectionMap.put(Keys.IS_SECURE, Tables.KEYS + "." + Keys.IS_SECURE);
projectionMap.put(Keys.CAN_CERTIFY, Keys.CAN_CERTIFY);
projectionMap.put(Keys.CAN_ENCRYPT, Keys.CAN_ENCRYPT);
projectionMap.put(Keys.CAN_SIGN, Keys.CAN_SIGN);

View file

@ -421,6 +421,7 @@ public class ProviderHelper {
values.put(Keys.CAN_SIGN, s);
values.put(Keys.CAN_AUTHENTICATE, a);
values.put(Keys.IS_REVOKED, key.isRevoked());
values.put(Keys.IS_SECURE, key.isSecure());
// see above
if (masterKeyId == keyId) {

View file

@ -62,15 +62,14 @@ import android.widget.RelativeLayout;
import android.widget.TextView;
import android.widget.Toast;
import org.openintents.openpgp.util.OpenPgpUtils;
import org.sufficientlysecure.keychain.Constants;
import org.sufficientlysecure.keychain.R;
import org.sufficientlysecure.keychain.keyimport.ParcelableHkpKeyserver;
import org.sufficientlysecure.keychain.keyimport.ParcelableKeyRing;
import org.sufficientlysecure.keychain.operations.results.EditKeyResult;
import org.sufficientlysecure.keychain.operations.results.ImportKeyResult;
import org.sufficientlysecure.keychain.operations.results.OperationResult;
import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKey.SecretKeyType;
import org.sufficientlysecure.keychain.pgp.KeyRing;
import org.sufficientlysecure.keychain.pgp.exception.PgpKeyNotFoundException;
import org.sufficientlysecure.keychain.provider.CachedPublicKeyRing;
import org.sufficientlysecure.keychain.provider.KeychainContract;
@ -95,7 +94,6 @@ import org.sufficientlysecure.keychain.ui.util.QrCodeUtils;
import org.sufficientlysecure.keychain.util.ContactHelper;
import org.sufficientlysecure.keychain.util.Log;
import org.sufficientlysecure.keychain.util.NfcHelper;
import org.sufficientlysecure.keychain.keyimport.ParcelableHkpKeyserver;
import org.sufficientlysecure.keychain.util.Passphrase;
import org.sufficientlysecure.keychain.util.Preferences;
@ -163,6 +161,7 @@ public class ViewKeyActivity extends BaseSecurityTokenActivity implements
private boolean mHasEncrypt = false;
private boolean mIsVerified = false;
private boolean mIsRevoked = false;
private boolean mIsSecure = true;
private boolean mIsExpired = false;
private boolean mShowSecurityTokenAfterCreation = false;
@ -819,6 +818,7 @@ public class ViewKeyActivity extends BaseSecurityTokenActivity implements
KeychainContract.KeyRings.USER_ID,
KeychainContract.KeyRings.IS_REVOKED,
KeychainContract.KeyRings.IS_EXPIRED,
KeychainContract.KeyRings.IS_SECURE,
KeychainContract.KeyRings.VERIFIED,
KeychainContract.KeyRings.HAS_ANY_SECRET,
KeychainContract.KeyRings.FINGERPRINT,
@ -832,13 +832,14 @@ public class ViewKeyActivity extends BaseSecurityTokenActivity implements
static final int INDEX_USER_ID = 2;
static final int INDEX_IS_REVOKED = 3;
static final int INDEX_IS_EXPIRED = 4;
static final int INDEX_VERIFIED = 5;
static final int INDEX_HAS_ANY_SECRET = 6;
static final int INDEX_FINGERPRINT = 7;
static final int INDEX_HAS_ENCRYPT = 8;
static final int INDEX_NAME = 9;
static final int INDEX_EMAIL = 10;
static final int INDEX_COMMENT = 11;
static final int INDEX_IS_SECURE = 5;
static final int INDEX_VERIFIED = 6;
static final int INDEX_HAS_ANY_SECRET = 7;
static final int INDEX_FINGERPRINT = 8;
static final int INDEX_HAS_ENCRYPT = 9;
static final int INDEX_NAME = 10;
static final int INDEX_EMAIL = 11;
static final int INDEX_COMMENT = 12;
@Override
public Loader<Cursor> onCreateLoader(int id, Bundle args) {
@ -915,6 +916,7 @@ public class ViewKeyActivity extends BaseSecurityTokenActivity implements
mHasEncrypt = data.getInt(INDEX_HAS_ENCRYPT) != 0;
mIsRevoked = data.getInt(INDEX_IS_REVOKED) > 0;
mIsExpired = data.getInt(INDEX_IS_EXPIRED) != 0;
mIsSecure = data.getInt(INDEX_IS_SECURE) == 1;
mIsVerified = data.getInt(INDEX_VERIFIED) > 0;
// if the refresh animation isn't playing
@ -952,6 +954,19 @@ public class ViewKeyActivity extends BaseSecurityTokenActivity implements
// noinspection deprecation, fix requires api level 23
color = getResources().getColor(R.color.key_flag_red);
mActionEncryptFile.setVisibility(View.INVISIBLE);
mActionEncryptText.setVisibility(View.INVISIBLE);
mActionNfc.setVisibility(View.INVISIBLE);
hideFab();
mQrCodeLayout.setVisibility(View.GONE);
} else if (!mIsSecure) {
mStatusText.setText(R.string.view_key_insecure);
mStatusImage.setVisibility(View.VISIBLE);
KeyFormattingUtils.setStatusImage(this, mStatusImage, mStatusText,
State.INSECURE, R.color.icons, true);
// noinspection deprecation, fix requires api level 23
color = getResources().getColor(R.color.key_flag_red);
mActionEncryptFile.setVisibility(View.INVISIBLE);
mActionEncryptText.setVisibility(View.INVISIBLE);
mActionNfc.setVisibility(View.INVISIBLE);

View file

@ -17,13 +17,6 @@
package org.sufficientlysecure.keychain.ui.adapter;
import java.io.Serializable;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Date;
import java.util.List;
import java.util.Date;
import android.content.Context;
import android.database.Cursor;
import android.graphics.PorterDuff;
@ -31,7 +24,6 @@ import android.support.v4.widget.CursorAdapter;
import android.text.format.DateUtils;
import android.view.LayoutInflater;
import android.view.View;
import android.view.View.OnClickListener;
import android.view.ViewGroup;
import android.widget.ImageButton;
import android.widget.ImageView;
@ -43,11 +35,17 @@ import org.sufficientlysecure.keychain.pgp.CanonicalizedPublicKey;
import org.sufficientlysecure.keychain.pgp.CanonicalizedPublicKeyRing;
import org.sufficientlysecure.keychain.pgp.KeyRing;
import org.sufficientlysecure.keychain.provider.KeychainContract.KeyRings;
import org.sufficientlysecure.keychain.ui.util.Highlighter;
import org.sufficientlysecure.keychain.ui.util.FormattingUtils;
import org.sufficientlysecure.keychain.ui.util.Highlighter;
import org.sufficientlysecure.keychain.ui.util.KeyFormattingUtils;
import org.sufficientlysecure.keychain.ui.util.KeyFormattingUtils.State;
import java.io.Serializable;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Date;
import java.util.List;
public class KeyAdapter extends CursorAdapter {
protected String mQuery;
@ -61,6 +59,7 @@ public class KeyAdapter extends CursorAdapter {
KeyRings.USER_ID,
KeyRings.IS_REVOKED,
KeyRings.IS_EXPIRED,
KeyRings.IS_SECURE,
KeyRings.VERIFIED,
KeyRings.HAS_ANY_SECRET,
KeyRings.HAS_DUPLICATE_USER_ID,
@ -76,15 +75,16 @@ public class KeyAdapter extends CursorAdapter {
public static final int INDEX_USER_ID = 2;
public static final int INDEX_IS_REVOKED = 3;
public static final int INDEX_IS_EXPIRED = 4;
public static final int INDEX_VERIFIED = 5;
public static final int INDEX_HAS_ANY_SECRET = 6;
public static final int INDEX_HAS_DUPLICATE_USER_ID = 7;
public static final int INDEX_FINGERPRINT = 8;
public static final int INDEX_CREATION = 9;
public static final int INDEX_HAS_ENCRYPT = 10;
public static final int INDEX_NAME = 11;
public static final int INDEX_EMAIL = 12;
public static final int INDEX_COMMENT = 13;
public static final int INDEX_IS_SECURE = 5;
public static final int INDEX_VERIFIED = 6;
public static final int INDEX_HAS_ANY_SECRET = 7;
public static final int INDEX_HAS_DUPLICATE_USER_ID = 8;
public static final int INDEX_FINGERPRINT = 9;
public static final int INDEX_CREATION = 10;
public static final int INDEX_HAS_ENCRYPT = 11;
public static final int INDEX_NAME = 12;
public static final int INDEX_EMAIL = 13;
public static final int INDEX_COMMENT = 14;
public KeyAdapter(Context context, Cursor c, int flags) {
super(context, c, flags);
@ -161,6 +161,11 @@ public class KeyAdapter extends CursorAdapter {
mStatus.setVisibility(View.VISIBLE);
mSlinger.setVisibility(View.GONE);
textColor = context.getResources().getColor(R.color.key_flag_gray);
} else if (!item.mIsSecure) {
KeyFormattingUtils.setStatusImage(context, mStatus, null, State.INSECURE, R.color.key_flag_gray);
mStatus.setVisibility(View.VISIBLE);
mSlinger.setVisibility(View.GONE);
textColor = context.getResources().getColor(R.color.key_flag_gray);
} else if (item.mIsSecret) {
mStatus.setVisibility(View.GONE);
if (mSlingerButton.hasOnClickListeners()) {
@ -280,7 +285,7 @@ public class KeyAdapter extends CursorAdapter {
public final boolean mHasEncrypt;
public final Date mCreation;
public final String mFingerprint;
public final boolean mIsSecret, mIsRevoked, mIsExpired, mIsVerified;
public final boolean mIsSecret, mIsRevoked, mIsExpired, mIsSecure, mIsVerified;
private KeyItem(Cursor cursor) {
String userId = cursor.getString(INDEX_USER_ID);
@ -298,6 +303,7 @@ public class KeyAdapter extends CursorAdapter {
mIsSecret = cursor.getInt(INDEX_HAS_ANY_SECRET) != 0;
mIsRevoked = cursor.getInt(INDEX_IS_REVOKED) > 0;
mIsExpired = cursor.getInt(INDEX_IS_EXPIRED) > 0;
mIsSecure = cursor.getInt(INDEX_IS_SECURE) > 0;
mIsVerified = cursor.getInt(INDEX_VERIFIED) > 0;
}
@ -317,6 +323,7 @@ public class KeyAdapter extends CursorAdapter {
ring.getFingerprint());
mIsRevoked = key.isRevoked();
mIsExpired = key.isExpired();
mIsSecure = key.isSecure();
// these two are actually "don't know"s
mIsSecret = false;

View file

@ -35,20 +35,19 @@ import android.widget.TextView;
import com.futuremind.recyclerviewfastscroll.SectionTitleProvider;
import org.openintents.openpgp.util.OpenPgpUtils;
import org.sufficientlysecure.keychain.Constants;
import org.sufficientlysecure.keychain.R;
import org.sufficientlysecure.keychain.provider.KeychainContract;
import org.sufficientlysecure.keychain.ui.util.FormattingUtils;
import org.sufficientlysecure.keychain.ui.util.Highlighter;
import org.sufficientlysecure.keychain.ui.util.KeyFormattingUtils;
import org.sufficientlysecure.keychain.ui.util.adapter.*;
import org.sufficientlysecure.keychain.ui.util.adapter.CursorAdapter;
import org.sufficientlysecure.keychain.ui.util.adapter.SectionCursorAdapter;
import org.sufficientlysecure.keychain.util.Log;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
public class KeySectionedListAdapter extends SectionCursorAdapter<KeySectionedListAdapter.KeyListCursor, Character,
SectionCursorAdapter.ViewHolder, KeySectionedListAdapter.KeyHeaderViewHolder> implements SectionTitleProvider {
@ -418,6 +417,18 @@ public class KeySectionedListAdapter extends SectionCursorAdapter<KeySectionedLi
R.color.key_flag_gray
);
mStatus.setVisibility(View.VISIBLE);
mSlinger.setVisibility(View.GONE);
textColor = ContextCompat.getColor(context, R.color.key_flag_gray);
} else if (!keyItem.isSecure()) {
KeyFormattingUtils.setStatusImage(
context,
mStatus,
null,
KeyFormattingUtils.State.INSECURE,
R.color.key_flag_gray
);
mStatus.setVisibility(View.VISIBLE);
mSlinger.setVisibility(View.GONE);
textColor = ContextCompat.getColor(context, R.color.key_flag_gray);

View file

@ -66,6 +66,7 @@ public class SubkeysAdapter extends CursorAdapter {
Keys.CAN_SIGN,
Keys.CAN_AUTHENTICATE,
Keys.IS_REVOKED,
Keys.IS_SECURE,
Keys.CREATION,
Keys.EXPIRY,
Keys.FINGERPRINT
@ -82,9 +83,10 @@ public class SubkeysAdapter extends CursorAdapter {
private static final int INDEX_CAN_SIGN = 9;
private static final int INDEX_CAN_AUTHENTICATE = 10;
private static final int INDEX_IS_REVOKED = 11;
private static final int INDEX_CREATION = 12;
private static final int INDEX_EXPIRY = 13;
private static final int INDEX_FINGERPRINT = 14;
private static final int INDEX_IS_SECURE = 12;
private static final int INDEX_CREATION = 13;
private static final int INDEX_EXPIRY = 14;
private static final int INDEX_FINGERPRINT = 15;
public SubkeysAdapter(Context context, Cursor c, int flags) {
super(context, c, flags);
@ -232,6 +234,7 @@ public class SubkeysAdapter extends CursorAdapter {
vAuthenticateIcon.setVisibility(cursor.getInt(INDEX_CAN_AUTHENTICATE) != 0 ? View.VISIBLE : View.GONE);
boolean isRevoked = cursor.getInt(INDEX_IS_REVOKED) > 0;
boolean isSecure = cursor.getInt(INDEX_IS_SECURE) > 0;
Date expiryDate = null;
if (!cursor.isNull(INDEX_EXPIRY)) {
@ -279,7 +282,7 @@ public class SubkeysAdapter extends CursorAdapter {
}
// if key is expired or revoked...
boolean isInvalid = isRevoked || isExpired;
boolean isInvalid = isRevoked || isExpired || !isSecure;
if (isInvalid) {
vStatus.setVisibility(View.VISIBLE);
@ -306,6 +309,11 @@ public class SubkeysAdapter extends CursorAdapter {
vStatus.setColorFilter(
mContext.getResources().getColor(R.color.key_flag_gray),
PorterDuff.Mode.SRC_IN);
} else if (!isSecure) {
vStatus.setImageResource(R.drawable.status_signature_invalid_cutout_24dp);
vStatus.setColorFilter(
mContext.getResources().getColor(R.color.key_flag_gray),
PorterDuff.Mode.SRC_IN);
}
} else {
vStatus.setVisibility(View.GONE);

View file

@ -25,9 +25,7 @@ import android.database.DataSetObserver;
import android.os.Handler;
import android.support.v7.widget.RecyclerView;
import org.openintents.openpgp.util.OpenPgpUtils;
import org.sufficientlysecure.keychain.Constants;
import org.sufficientlysecure.keychain.pgp.KeyRing;
import org.sufficientlysecure.keychain.provider.KeychainContract;
import org.sufficientlysecure.keychain.util.Log;
@ -389,6 +387,7 @@ public abstract class CursorAdapter<C extends CursorAdapter.AbstractCursor, VH e
KeychainContract.KeyRings.USER_ID,
KeychainContract.KeyRings.IS_REVOKED,
KeychainContract.KeyRings.IS_EXPIRED,
KeychainContract.KeyRings.IS_SECURE,
KeychainContract.KeyRings.HAS_DUPLICATE_USER_ID,
KeychainContract.KeyRings.CREATION,
KeychainContract.KeyRings.NAME,
@ -451,6 +450,11 @@ public abstract class CursorAdapter<C extends CursorAdapter.AbstractCursor, VH e
return getInt(index) > 0;
}
public boolean isSecure() {
int index = getColumnIndexOrThrow(KeychainContract.KeyRings.IS_SECURE);
return getInt(index) > 0;
}
public long getCreationTime() {
int index = getColumnIndexOrThrow(KeychainContract.KeyRings.CREATION);
return getLong(index) * 1000;

View file

@ -78,6 +78,9 @@ public class SignKeySpinner extends KeySpinner {
if (cursor.getInt(KeyAdapter.INDEX_IS_EXPIRED) != 0) {
return false;
}
if (cursor.getInt(KeyAdapter.INDEX_IS_SECURE) == 0) {
return false;
}
if (cursor.isNull(mIndexHasSign)) {
return false;
}

View file

@ -822,6 +822,7 @@
<!-- View key -->
<string name="view_key_revoked">"Revoked: Key must not be used anymore!"</string>
<string name="view_key_insecure">"Insecure: Key must not be used anymore!"</string>
<string name="view_key_expired">"Expired: The contact needs to extend the key's validity!"</string>
<string name="view_key_expired_secret">"Expired: You can extend the keys validity by editing it!"</string>
<string name="view_key_my_key">"My Key"</string>