From 4e408fae5319c427195895a325d07231c292806c Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 10 Apr 2014 20:33:19 +0200 Subject: [PATCH 1/5] remove drawer layout remnants from import key dialogue --- .../res/layout-large/import_keys_activity.xml | 21 -------- .../main/res/layout/import_keys_activity.xml | 53 ++++++++++++++++--- .../main/res/layout/import_keys_content.xml | 50 ----------------- 3 files changed, 46 insertions(+), 78 deletions(-) delete mode 100644 OpenKeychain/src/main/res/layout-large/import_keys_activity.xml delete mode 100644 OpenKeychain/src/main/res/layout/import_keys_content.xml diff --git a/OpenKeychain/src/main/res/layout-large/import_keys_activity.xml b/OpenKeychain/src/main/res/layout-large/import_keys_activity.xml deleted file mode 100644 index 2cb408441..000000000 --- a/OpenKeychain/src/main/res/layout-large/import_keys_activity.xml +++ /dev/null @@ -1,21 +0,0 @@ - - - - - - - - - - - - - \ No newline at end of file diff --git a/OpenKeychain/src/main/res/layout/import_keys_activity.xml b/OpenKeychain/src/main/res/layout/import_keys_activity.xml index c82607a33..eb1333704 100644 --- a/OpenKeychain/src/main/res/layout/import_keys_activity.xml +++ b/OpenKeychain/src/main/res/layout/import_keys_activity.xml @@ -1,11 +1,50 @@ - + - + - + - \ No newline at end of file + + + + + + \ No newline at end of file diff --git a/OpenKeychain/src/main/res/layout/import_keys_content.xml b/OpenKeychain/src/main/res/layout/import_keys_content.xml deleted file mode 100644 index eb1333704..000000000 --- a/OpenKeychain/src/main/res/layout/import_keys_content.xml +++ /dev/null @@ -1,50 +0,0 @@ - - - - - - - - - - - - - \ No newline at end of file From 0f06b8a1d6df7682d4216ec685c907bbf5774ff0 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 10 Apr 2014 20:33:54 +0200 Subject: [PATCH 2/5] display correct primary user id in import dialogue MOSTLY This is an incomplete fix; due to use of machine readable output, there is no way to know the primary user id for keys fetched from a key server. Pending https://bitbucket.org/skskeyserver/sks-keyserver/issue/28/primary-uid-in-machine-readable-index --- .../ui/adapter/ImportKeysListEntry.java | 61 ++++++++++++------- .../keychain/util/HkpKeyServer.java | 1 + 2 files changed, 41 insertions(+), 21 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/adapter/ImportKeysListEntry.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/adapter/ImportKeysListEntry.java index 5631d40ea..44bde963e 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/adapter/ImportKeysListEntry.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/adapter/ImportKeysListEntry.java @@ -21,9 +21,12 @@ import android.os.Parcel; import android.os.Parcelable; import android.util.SparseArray; +import org.spongycastle.bcpg.SignatureSubpacketTags; import org.spongycastle.openpgp.PGPKeyRing; import org.spongycastle.openpgp.PGPPublicKey; import org.spongycastle.openpgp.PGPSecretKeyRing; +import org.spongycastle.openpgp.PGPSignature; +import org.spongycastle.openpgp.operator.jcajce.JcaPGPContentVerifierBuilderProvider; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.pgp.PgpKeyHelper; import org.sufficientlysecure.keychain.util.IterableIterator; @@ -46,31 +49,19 @@ public class ImportKeysListEntry implements Serializable, Parcelable { public int bitStrength; public String algorithm; public boolean secretKey; + public String mPrimaryUserId; private boolean mSelected; private byte[] mBytes = new byte[]{}; - public ImportKeysListEntry(ImportKeysListEntry b) { - this.userIds = b.userIds; - this.keyId = b.keyId; - this.revoked = b.revoked; - this.date = b.date; - this.fingerPrintHex = b.fingerPrintHex; - this.keyIdHex = b.keyIdHex; - this.bitStrength = b.bitStrength; - this.algorithm = b.algorithm; - this.secretKey = b.secretKey; - this.mSelected = b.mSelected; - this.mBytes = b.mBytes; - } - public int describeContents() { return 0; } @Override public void writeToParcel(Parcel dest, int flags) { + dest.writeString(mPrimaryUserId); dest.writeStringList(userIds); dest.writeLong(keyId); dest.writeByte((byte) (revoked ? 1 : 0)); @@ -88,6 +79,7 @@ public class ImportKeysListEntry implements Serializable, Parcelable { public static final Creator CREATOR = new Creator() { public ImportKeysListEntry createFromParcel(final Parcel source) { ImportKeysListEntry vr = new ImportKeysListEntry(); + vr.mPrimaryUserId = source.readString(); vr.userIds = new ArrayList(); source.readStringList(vr.userIds); vr.keyId = source.readLong(); @@ -198,6 +190,14 @@ public class ImportKeysListEntry implements Serializable, Parcelable { this.userIds = userIds; } + public String getPrimaryUserId() { + return mPrimaryUserId; + } + + public void setPrimaryUserId(String uid) { + mPrimaryUserId = uid; + } + /** * Constructor for later querying from keyserver */ @@ -229,20 +229,39 @@ public class ImportKeysListEntry implements Serializable, Parcelable { } else { secretKey = false; } + PGPPublicKey key = pgpKeyRing.getPublicKey(); userIds = new ArrayList(); - for (String userId : new IterableIterator(pgpKeyRing.getPublicKey().getUserIDs())) { + for (String userId : new IterableIterator(key.getUserIDs())) { userIds.add(userId); + for(PGPSignature sig : new IterableIterator(key.getSignaturesForID(userId))) { + if(sig.getHashedSubPackets().hasSubpacket(SignatureSubpacketTags.PRIMARY_USER_ID)) { + try { + // make sure it's actually valid + sig.init(new JcaPGPContentVerifierBuilderProvider().setProvider( + Constants.BOUNCY_CASTLE_PROVIDER_NAME), key); + if (sig.verifyCertification(userId, key)) { + mPrimaryUserId = userId; + } + } catch(Exception e) { + // nothing bad happens, the key is just not considered the primary key id + } + } + + } + } + // if there was no user id flagged as primary, use the first one + if(mPrimaryUserId == null) { + mPrimaryUserId = userIds.get(0); } - this.keyId = pgpKeyRing.getPublicKey().getKeyID(); + this.keyId = key.getKeyID(); this.keyIdHex = PgpKeyHelper.convertKeyIdToHex(keyId); - this.revoked = pgpKeyRing.getPublicKey().isRevoked(); - this.fingerPrintHex = PgpKeyHelper.convertFingerprintToHex(pgpKeyRing.getPublicKey() - .getFingerprint()); - this.bitStrength = pgpKeyRing.getPublicKey().getBitStrength(); - final int algorithm = pgpKeyRing.getPublicKey().getAlgorithm(); + this.revoked = key.isRevoked(); + this.fingerPrintHex = PgpKeyHelper.convertFingerprintToHex(key.getFingerprint()); + this.bitStrength = key.getBitStrength(); + final int algorithm = key.getAlgorithm(); this.algorithm = getAlgorithmFromId(algorithm); } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/HkpKeyServer.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/HkpKeyServer.java index d1753de83..02149124c 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/HkpKeyServer.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/HkpKeyServer.java @@ -294,6 +294,7 @@ public class HkpKeyServer extends KeyServer { userIds.add(tmp); } entry.setUserIds(userIds); + entry.setPrimaryUserId(userIds.get(0)); results.add(entry); } From 9af532880c728ccd343769078b008a9b31dc4ce1 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Fri, 11 Apr 2014 01:48:08 +0200 Subject: [PATCH 3/5] fix EditKeyActivity --- .../service/PassphraseCacheService.java | 40 ++++++------- .../keychain/ui/EditKeyActivity.java | 56 +++++++++---------- 2 files changed, 46 insertions(+), 50 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/PassphraseCacheService.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/PassphraseCacheService.java index cbc71ce6a..66411ce0b 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/PassphraseCacheService.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/PassphraseCacheService.java @@ -200,6 +200,26 @@ public class PassphraseCacheService extends Service { return cachedPassphrase; } + public static boolean hasPassphrase(PGPSecretKeyRing secretKeyRing) throws PGPException { + PGPSecretKey secretKey = null; + boolean foundValidKey = false; + for (Iterator keys = secretKeyRing.getSecretKeys(); keys.hasNext(); ) { + secretKey = (PGPSecretKey) keys.next(); + if (!secretKey.isPrivateKeyEmpty()) { + foundValidKey = true; + break; + } + } + if(!foundValidKey) { + return false; + } + + PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder() + .setProvider("SC").build("".toCharArray()); + PGPPrivateKey testKey = secretKey.extractPrivateKey(keyDecryptor); + return testKey == null; + } + /** * Checks if key has a passphrase. * @@ -210,25 +230,7 @@ public class PassphraseCacheService extends Service { // check if the key has no passphrase try { PGPSecretKeyRing secRing = ProviderHelper.getPGPSecretKeyRing(context, secretKeyId); - PGPSecretKey secretKey = null; - boolean foundValidKey = false; - for (Iterator keys = secRing.getSecretKeys(); keys.hasNext(); ) { - secretKey = (PGPSecretKey) keys.next(); - if (!secretKey.isPrivateKeyEmpty()) { - foundValidKey = true; - break; - } - } - - if (!foundValidKey) { - return false; - } - PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder().setProvider( - "SC").build("".toCharArray()); - PGPPrivateKey testKey = secretKey.extractPrivateKey(keyDecryptor); - if (testKey != null) { - return false; - } + return hasPassphrase(secRing); } catch (PGPException e) { // silently catch } catch (ProviderHelper.NotFoundException e) { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyActivity.java index 84f0c41cf..a009410a6 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyActivity.java @@ -39,10 +39,12 @@ import android.widget.CheckBox; import android.widget.CompoundButton; import android.widget.CompoundButton.OnCheckedChangeListener; import android.widget.LinearLayout; +import android.widget.Toast; import com.beardedhen.androidbootstrap.BootstrapButton; import com.devspark.appmsg.AppMsg; +import org.spongycastle.openpgp.PGPException; import org.spongycastle.openpgp.PGPSecretKey; import org.spongycastle.openpgp.PGPSecretKeyRing; import org.sufficientlysecure.keychain.Constants; @@ -287,34 +289,16 @@ public class EditKeyActivity extends ActionBarActivity implements EditorListener Log.d(Constants.TAG, "uri: " + mDataUri); try { - // get master key id using row id - long masterKeyId = ProviderHelper.getMasterKeyId(this, mDataUri); - finallyEdit(masterKeyId); - } catch (ProviderHelper.NotFoundException e) { - Log.e(Constants.TAG, "key not found!", e); - } - } - } + Uri secretUri = KeychainContract.KeyRingData.buildSecretKeyRingUri(mDataUri); + mKeyRing = (PGPSecretKeyRing) ProviderHelper.getPGPKeyRing(this, secretUri); - @SuppressWarnings("unchecked") - private void finallyEdit(final long masterKeyId) { - if (masterKeyId != 0) { - PGPSecretKey masterKey = null; - try { - mKeyRing = ProviderHelper.getPGPSecretKeyRing(this, masterKeyId); - - masterKey = mKeyRing.getSecretKey(); + PGPSecretKey masterKey = mKeyRing.getSecretKey(); mMasterCanSign = PgpKeyHelper.isCertificationKey(mKeyRing.getSecretKey()); for (PGPSecretKey key : new IterableIterator(mKeyRing.getSecretKeys())) { mKeys.add(key); mKeysUsages.add(-1); // get usage when view is created } - } catch (ProviderHelper.NotFoundException e) { - Log.e(Constants.TAG, "Keyring not found with masterKeyId: " + masterKeyId); - AppMsg.makeText(this, R.string.error_no_secret_key_found, AppMsg.STYLE_ALERT).show(); - // TODO - } - if (masterKey != null) { + boolean isSet = false; for (String userId : new IterableIterator(masterKey.getUserIDs())) { Log.d(Constants.TAG, "Added userId " + userId); @@ -327,17 +311,27 @@ public class EditKeyActivity extends ActionBarActivity implements EditorListener } mUserIds.add(userId); } + + buildLayout(false); + + mCurrentPassphrase = ""; + mIsPassphraseSet = PassphraseCacheService.hasPassphrase(mKeyRing); + if (!mIsPassphraseSet) { + // check "no passphrase" checkbox and remove button + mNoPassphrase.setChecked(true); + mChangePassphrase.setVisibility(View.GONE); + } + + } catch (ProviderHelper.NotFoundException e) { + Log.e(Constants.TAG, "Keyring not found: " + e.getMessage(), e); + Toast.makeText(this, R.string.error_no_secret_key_found, Toast.LENGTH_SHORT).show(); + finish(); + } catch (PGPException e) { + Log.e(Constants.TAG, "Error extracting key: " + e.getMessage(), e); + Toast.makeText(this, R.string.error_no_secret_key_found, Toast.LENGTH_LONG).show(); + finish(); } - } - mCurrentPassphrase = ""; - buildLayout(false); - - mIsPassphraseSet = PassphraseCacheService.hasPassphrase(this, masterKeyId); - if (!mIsPassphraseSet) { - // check "no passphrase" checkbox and remove button - mNoPassphrase.setChecked(true); - mChangePassphrase.setVisibility(View.GONE); } } From b77fb2fcc0371248beaf86a1db092e7ad99e6446 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Fri, 11 Apr 2014 03:21:39 +0200 Subject: [PATCH 4/5] get rid of more getMasterKeyId usage, work on getKeyRingsAsArmoredString --- .../keychain/provider/ProviderHelper.java | 126 +++++++++--------- .../service/PassphraseCacheService.java | 17 ++- .../keychain/ui/EditKeyActivity.java | 4 - .../ui/EncryptAsymmetricFragment.java | 5 +- .../keychain/ui/ViewKeyActivity.java | 24 ++-- .../ui/dialog/ShareQrCodeDialogFragment.java | 23 ++-- OpenKeychain/src/main/res/values/strings.xml | 2 + 7 files changed, 107 insertions(+), 94 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java index 3e0520c2a..43fbf7045 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java @@ -461,70 +461,49 @@ public class ProviderHelper { return ContentProviderOperation.newInsert(uri).withValues(values).build(); } - public static ArrayList getKeyRingsAsArmoredString(Context context, long[] masterKeyIds) { - ArrayList output = new ArrayList(); - - if (masterKeyIds != null && masterKeyIds.length > 0) { - - Cursor cursor = getCursorWithSelectedKeyringMasterKeyIds(context, masterKeyIds); - - if (cursor != null) { - int masterIdCol = cursor.getColumnIndex(KeyRingData.MASTER_KEY_ID); - int dataCol = cursor.getColumnIndex(KeyRingData.KEY_RING_DATA); - if (cursor.moveToFirst()) { - do { - Log.d(Constants.TAG, "masterKeyId: " + cursor.getLong(masterIdCol)); - - // get actual keyring data blob and write it to ByteArrayOutputStream - try { - Object keyRing = null; - byte[] data = cursor.getBlob(dataCol); - if (data != null) { - keyRing = PgpConversionHelper.BytesToPGPKeyRing(data); - } - - ByteArrayOutputStream bos = new ByteArrayOutputStream(); - ArmoredOutputStream aos = new ArmoredOutputStream(bos); - aos.setHeader("Version", PgpHelper.getFullVersion(context)); - - if (keyRing instanceof PGPSecretKeyRing) { - aos.write(((PGPSecretKeyRing) keyRing).getEncoded()); - } else if (keyRing instanceof PGPPublicKeyRing) { - aos.write(((PGPPublicKeyRing) keyRing).getEncoded()); - } - aos.close(); - - String armoredKey = bos.toString("UTF-8"); - - Log.d(Constants.TAG, "armoredKey:" + armoredKey); - - output.add(armoredKey); - } catch (IOException e) { - Log.e(Constants.TAG, "IOException", e); - } - } while (cursor.moveToNext()); - } - } - - if (cursor != null) { - cursor.close(); - } - - } else { - Log.e(Constants.TAG, "No master keys given!"); + private static String getKeyRingAsArmoredString(Context context, byte[] data) throws IOException { + Object keyRing = null; + if (data != null) { + keyRing = PgpConversionHelper.BytesToPGPKeyRing(data); } - if (output.size() > 0) { - return output; - } else { - return null; + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + ArmoredOutputStream aos = new ArmoredOutputStream(bos); + aos.setHeader("Version", PgpHelper.getFullVersion(context)); + + if (keyRing instanceof PGPSecretKeyRing) { + aos.write(((PGPSecretKeyRing) keyRing).getEncoded()); + } else if (keyRing instanceof PGPPublicKeyRing) { + aos.write(((PGPPublicKeyRing) keyRing).getEncoded()); } + aos.close(); + + String armoredKey = bos.toString("UTF-8"); + + Log.d(Constants.TAG, "armoredKey:" + armoredKey); + + return armoredKey; } - private static Cursor getCursorWithSelectedKeyringMasterKeyIds(Context context, long[] masterKeyIds) { - Cursor cursor = null; - if (masterKeyIds != null && masterKeyIds.length > 0) { + public static String getKeyRingAsArmoredString(Context context, Uri uri) + throws NotFoundException, IOException { + byte[] data = (byte[]) ProviderHelper.getGenericData( + context, uri, KeyRingData.KEY_RING_DATA, ProviderHelper.FIELD_TYPE_BLOB); + return getKeyRingAsArmoredString(context, data); + } + // TODO This method is NOT ACTUALLY USED. Is this preparation for something, or just dead code? + public static ArrayList getKeyRingsAsArmoredString(Context context, long[] masterKeyIds) + throws IOException { + ArrayList output = new ArrayList(); + + if (masterKeyIds == null || masterKeyIds.length == 0) { + Log.e(Constants.TAG, "No master keys given!"); + return output; + } + + // Build a cursor for the selected masterKeyIds + Cursor cursor = null; { String inMasterKeyList = KeyRingData.MASTER_KEY_ID + " IN ("; for (int i = 0; i < masterKeyIds.length; ++i) { if (i != 0) { @@ -536,10 +515,37 @@ public class ProviderHelper { cursor = context.getContentResolver().query(KeyRingData.buildPublicKeyRingUri(), new String[] { KeyRingData._ID, KeyRingData.MASTER_KEY_ID, KeyRingData.KEY_RING_DATA - }, inMasterKeyList, null, null); + }, inMasterKeyList, null, null); } - return cursor; + if (cursor != null) { + int masterIdCol = cursor.getColumnIndex(KeyRingData.MASTER_KEY_ID); + int dataCol = cursor.getColumnIndex(KeyRingData.KEY_RING_DATA); + if (cursor.moveToFirst()) { + do { + Log.d(Constants.TAG, "masterKeyId: " + cursor.getLong(masterIdCol)); + + byte[] data = cursor.getBlob(dataCol); + + // get actual keyring data blob and write it to ByteArrayOutputStream + try { + output.add(getKeyRingAsArmoredString(context, data)); + } catch (IOException e) { + Log.e(Constants.TAG, "IOException", e); + } + } while (cursor.moveToNext()); + } + } + + if (cursor != null) { + cursor.close(); + } + + if (output.size() > 0) { + return output; + } else { + return null; + } } public static ArrayList getRegisteredApiApps(Context context) { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/PassphraseCacheService.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/PassphraseCacheService.java index 66411ce0b..91d9b5584 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/PassphraseCacheService.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/PassphraseCacheService.java @@ -200,7 +200,7 @@ public class PassphraseCacheService extends Service { return cachedPassphrase; } - public static boolean hasPassphrase(PGPSecretKeyRing secretKeyRing) throws PGPException { + public static boolean hasPassphrase(PGPSecretKeyRing secretKeyRing) { PGPSecretKey secretKey = null; boolean foundValidKey = false; for (Iterator keys = secretKeyRing.getSecretKeys(); keys.hasNext(); ) { @@ -214,10 +214,15 @@ public class PassphraseCacheService extends Service { return false; } - PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder() - .setProvider("SC").build("".toCharArray()); - PGPPrivateKey testKey = secretKey.extractPrivateKey(keyDecryptor); - return testKey == null; + try { + PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder() + .setProvider("SC").build("".toCharArray()); + PGPPrivateKey testKey = secretKey.extractPrivateKey(keyDecryptor); + return testKey == null; + } catch(PGPException e) { + // this means the crc check failed -> passphrase required + return true; + } } /** @@ -231,8 +236,6 @@ public class PassphraseCacheService extends Service { try { PGPSecretKeyRing secRing = ProviderHelper.getPGPSecretKeyRing(context, secretKeyId); return hasPassphrase(secRing); - } catch (PGPException e) { - // silently catch } catch (ProviderHelper.NotFoundException e) { Log.e(Constants.TAG, "key not found!", e); } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyActivity.java index a009410a6..696f79cd7 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyActivity.java @@ -326,10 +326,6 @@ public class EditKeyActivity extends ActionBarActivity implements EditorListener Log.e(Constants.TAG, "Keyring not found: " + e.getMessage(), e); Toast.makeText(this, R.string.error_no_secret_key_found, Toast.LENGTH_SHORT).show(); finish(); - } catch (PGPException e) { - Log.e(Constants.TAG, "Error extracting key: " + e.getMessage(), e); - Toast.makeText(this, R.string.error_no_secret_key_found, Toast.LENGTH_LONG).show(); - finish(); } } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptAsymmetricFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptAsymmetricFragment.java index 2ec4dd89e..a99c9eca8 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptAsymmetricFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptAsymmetricFragment.java @@ -165,10 +165,11 @@ public class EncryptAsymmetricFragment extends Fragment { if (preselectedEncryptionKeyIds != null) { Vector goodIds = new Vector(); for (int i = 0; i < preselectedEncryptionKeyIds.length; ++i) { - // TODO check for available encrypt keys... is this even relevant? + // TODO One query per selected key?! wtf try { long id = ProviderHelper.getMasterKeyId(getActivity(), - KeyRings.buildUnifiedKeyRingsFindBySubkeyUri(Long.toString(preselectedEncryptionKeyIds[i])) + KeyRings.buildUnifiedKeyRingsFindBySubkeyUri( + Long.toString(preselectedEncryptionKeyIds[i])) ); goodIds.add(id); } catch (ProviderHelper.NotFoundException e) { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewKeyActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewKeyActivity.java index dceb9a0fc..ca26cac1c 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewKeyActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewKeyActivity.java @@ -219,12 +219,8 @@ public class ViewKeyActivity extends ActionBarActivity { } else { // get public keyring as ascii armored string try { - long masterKeyId = ProviderHelper.getMasterKeyId(this, dataUri); - - ArrayList keyringArmored = ProviderHelper.getKeyRingsAsArmoredString( - this, new long[]{masterKeyId}); - - content = keyringArmored.get(0); + Uri uri = KeychainContract.KeyRingData.buildPublicKeyRingUri(dataUri); + content = ProviderHelper.getKeyRingAsArmoredString(this, uri); // Android will fail with android.os.TransactionTooLargeException if key is too big // see http://www.lonestarprod.com/?p=34 @@ -233,8 +229,12 @@ public class ViewKeyActivity extends ActionBarActivity { AppMsg.STYLE_ALERT).show(); return; } + } catch (IOException e) { + Log.e(Constants.TAG, "error processing key!", e); + AppMsg.makeText(this, R.string.error_invalid_data, AppMsg.STYLE_ALERT).show(); } catch (ProviderHelper.NotFoundException e) { Log.e(Constants.TAG, "key not found!", e); + AppMsg.makeText(this, R.string.error_key_not_found, AppMsg.STYLE_ALERT).show(); } } @@ -259,16 +259,18 @@ public class ViewKeyActivity extends ActionBarActivity { private void copyToClipboard(Uri dataUri) { // get public keyring as ascii armored string try { - long masterKeyId = ProviderHelper.getMasterKeyId(this, dataUri); + Uri uri = KeychainContract.KeyRingData.buildPublicKeyRingUri(dataUri); + String keyringArmored = ProviderHelper.getKeyRingAsArmoredString(this, uri); - ArrayList keyringArmored = ProviderHelper.getKeyRingsAsArmoredString( - this, new long[]{masterKeyId}); - - ClipboardReflection.copyToClipboard(this, keyringArmored.get(0)); + ClipboardReflection.copyToClipboard(this, keyringArmored); AppMsg.makeText(this, R.string.key_copied_to_clipboard, AppMsg.STYLE_INFO) .show(); + } catch (IOException e) { + Log.e(Constants.TAG, "error processing key!", e); + AppMsg.makeText(this, R.string.error_key_processing, AppMsg.STYLE_ALERT).show(); } catch (ProviderHelper.NotFoundException e) { Log.e(Constants.TAG, "key not found!", e); + AppMsg.makeText(this, R.string.error_key_not_found, AppMsg.STYLE_ALERT).show(); } } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/ShareQrCodeDialogFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/ShareQrCodeDialogFragment.java index d2d21093e..01d3db235 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/ShareQrCodeDialogFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/ShareQrCodeDialogFragment.java @@ -28,14 +28,19 @@ import android.view.View; import android.widget.Button; import android.widget.ImageView; import android.widget.TextView; + +import com.devspark.appmsg.AppMsg; + import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.R; import org.sufficientlysecure.keychain.pgp.PgpKeyHelper; +import org.sufficientlysecure.keychain.provider.KeychainContract; import org.sufficientlysecure.keychain.provider.KeychainContract.KeyRings; import org.sufficientlysecure.keychain.provider.ProviderHelper; import org.sufficientlysecure.keychain.util.Log; import org.sufficientlysecure.keychain.util.QrCodeUtils; +import java.io.IOException; import java.util.ArrayList; public class ShareQrCodeDialogFragment extends DialogFragment { @@ -106,20 +111,18 @@ public class ShareQrCodeDialogFragment extends DialogFragment { } else { mText.setText(R.string.share_qr_code_dialog_start); - // TODO works, but - long masterKeyId = 0; try { - masterKeyId = ProviderHelper.getMasterKeyId(getActivity(), dataUri); + Uri uri = KeychainContract.KeyRingData.buildPublicKeyRingUri(dataUri); + content = ProviderHelper.getKeyRingAsArmoredString(getActivity(), uri); + } catch (IOException e) { + Log.e(Constants.TAG, "error processing key!", e); + AppMsg.makeText(getActivity(), R.string.error_invalid_data, AppMsg.STYLE_ALERT).show(); + return null; } catch (ProviderHelper.NotFoundException e) { Log.e(Constants.TAG, "key not found!", e); + AppMsg.makeText(getActivity(), R.string.error_key_not_found, AppMsg.STYLE_ALERT).show(); + return null; } - // get public keyring as ascii armored string - ArrayList keyringArmored = ProviderHelper.getKeyRingsAsArmoredString( - getActivity(), new long[] { masterKeyId }); - - // TODO: binary? - - content = keyringArmored.get(0); // OnClickListener are set in onResume to prevent automatic dismissing of Dialogs // http://bit.ly/O5vfaR diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml index c21a120a2..a58e3a484 100644 --- a/OpenKeychain/src/main/res/values/strings.xml +++ b/OpenKeychain/src/main/res/values/strings.xml @@ -525,5 +525,7 @@ Type can certify cannot certify + Key not found! + Error processing key! From baa3c86e1246d486d80c963ddc6c64b58f2a5093 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Fri, 11 Apr 2014 03:29:00 +0200 Subject: [PATCH 5/5] split getMasterKeyId for the remaining use cases Closes #549 --- .../keychain/helper/ExportHelper.java | 2 +- .../keychain/provider/ProviderHelper.java | 21 ++++++++++++------- .../remote/ui/AccountSettingsFragment.java | 3 ++- .../keychain/ui/ViewKeyMainFragment.java | 5 +---- .../ui/dialog/ShareQrCodeDialogFragment.java | 4 ++-- 5 files changed, 19 insertions(+), 16 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/helper/ExportHelper.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/helper/ExportHelper.java index c42446c9e..c0aaf6d89 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/helper/ExportHelper.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/helper/ExportHelper.java @@ -51,7 +51,7 @@ public class ExportHelper { public void deleteKey(Uri dataUri, Handler deleteHandler) { try { - long masterKeyId = ProviderHelper.getMasterKeyId(mActivity, dataUri); + long masterKeyId = ProviderHelper.extractOrGetMasterKeyId(mActivity, dataUri); // Create a new Messenger for the communication back Messenger messenger = new Messenger(deleteHandler); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java index 43fbf7045..8e1bb3f60 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java @@ -122,15 +122,20 @@ public class ProviderHelper { * Find the master key id related to a given query. The id will either be extracted from the * query, which should work for all specific /key_rings/ queries, or will be queried if it can't. */ - public static long getMasterKeyId(Context context, Uri queryUri) throws NotFoundException { + public static long extractOrGetMasterKeyId(Context context, Uri queryUri) + throws NotFoundException { // try extracting from the uri first -// String firstSegment = queryUri.getPathSegments().get(1); -// if(!firstSegment.equals("find")) try { -// return Long.parseLong(firstSegment); -// } catch(NumberFormatException e) { -// // didn't work? oh well. -// Log.d(Constants.TAG, "Couldn't get masterKeyId from URI, querying..."); -// } + String firstSegment = queryUri.getPathSegments().get(1); + if(!firstSegment.equals("find")) try { + return Long.parseLong(firstSegment); + } catch(NumberFormatException e) { + // didn't work? oh well. + Log.d(Constants.TAG, "Couldn't get masterKeyId from URI, querying..."); + } + return getMasterKeyId(context, queryUri); + } + + public static long getMasterKeyId(Context context, Uri queryUri) throws NotFoundException { Object data = getGenericData(context, queryUri, KeyRings.MASTER_KEY_ID, FIELD_TYPE_INTEGER); if(data != null) { return (Long) data; diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/ui/AccountSettingsFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/ui/AccountSettingsFragment.java index 321d2a83f..a13c7a953 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/ui/AccountSettingsFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/ui/AccountSettingsFragment.java @@ -180,7 +180,8 @@ public class AccountSettingsFragment extends Fragment implements if (resultCode == Activity.RESULT_OK) { // select newly created key try { - long masterKeyId = ProviderHelper.getMasterKeyId(getActivity(), data.getData()); + long masterKeyId = ProviderHelper.extractOrGetMasterKeyId( + getActivity(), data.getData()); mSelectKeyFragment.selectKey(masterKeyId); } catch (ProviderHelper.NotFoundException e) { Log.e(Constants.TAG, "key not found!", e); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewKeyMainFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewKeyMainFragment.java index a7543d194..386342a48 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewKeyMainFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewKeyMainFragment.java @@ -331,11 +331,8 @@ public class ViewKeyMainFragment extends Fragment implements } private void encryptToContact(Uri dataUri) { - // TODO preselect from uri? should be feasible without trivial query try { - long keyId = ProviderHelper.getMasterKeyId(getActivity(), - KeyRingData.buildPublicKeyRingUri(dataUri)); - + long keyId = ProviderHelper.extractOrGetMasterKeyId(getActivity(), dataUri); long[] encryptionKeyIds = new long[]{ keyId }; Intent intent = new Intent(getActivity(), EncryptActivity.class); intent.setAction(EncryptActivity.ACTION_ENCRYPT); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/ShareQrCodeDialogFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/ShareQrCodeDialogFragment.java index 01d3db235..7e9a3d800 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/ShareQrCodeDialogFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/ShareQrCodeDialogFragment.java @@ -82,7 +82,6 @@ public class ShareQrCodeDialogFragment extends DialogFragment { mFingerprintOnly = getArguments().getBoolean(ARG_FINGERPRINT_ONLY); AlertDialog.Builder alert = new AlertDialog.Builder(getActivity()); - alert.setTitle(R.string.share_qr_code_dialog_title); LayoutInflater inflater = activity.getLayoutInflater(); @@ -100,7 +99,8 @@ public class ShareQrCodeDialogFragment extends DialogFragment { getActivity(), KeyRings.buildUnifiedKeyRingUri(dataUri), KeyRings.FINGERPRINT, ProviderHelper.FIELD_TYPE_BLOB); if(blob == null) { - // TODO error handling?! + Log.e(Constants.TAG, "key not found!"); + AppMsg.makeText(getActivity(), R.string.error_key_not_found, AppMsg.STYLE_ALERT).show(); return null; }