From 11e5261f07d85d76d6fe58aabb5981cda9ead22b Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Fri, 1 Aug 2014 16:47:06 +0200 Subject: [PATCH 1/4] move classes around a bit --- .../{tests => pgp}/PgpDecryptVerifyTest.java | 0 .../{tests => pgp}/PgpKeyOperationTest.java | 8 +- .../UncachedKeyringCanonicalizeTest.java | 7 +- .../UncachedKeyringMergeTest.java | 9 +- .../{tests => pgp}/UncachedKeyringTest.java | 8 +- .../ProviderHelperKeyringTest.java | 2 +- .../keychain/keyimport/FileImportCache.java | 97 ---------- .../keychain/util/FileImportCache.java | 179 ++++++++++++++++++ 8 files changed, 184 insertions(+), 126 deletions(-) rename OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/{tests => pgp}/PgpDecryptVerifyTest.java (100%) rename OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/{tests => pgp}/PgpKeyOperationTest.java (98%) rename OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/{tests => pgp}/UncachedKeyringCanonicalizeTest.java (98%) rename OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/{tests => pgp}/UncachedKeyringMergeTest.java (97%) rename OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/{tests => pgp}/UncachedKeyringTest.java (92%) rename OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/{tests => provider}/ProviderHelperKeyringTest.java (98%) delete mode 100644 OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/FileImportCache.java create mode 100644 OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/FileImportCache.java diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/PgpDecryptVerifyTest.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerifyTest.java similarity index 100% rename from OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/PgpDecryptVerifyTest.java rename to OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerifyTest.java diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/PgpKeyOperationTest.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java similarity index 98% rename from OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/PgpKeyOperationTest.java rename to OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java index 4f6694049..516d1ac63 100644 --- a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/PgpKeyOperationTest.java +++ b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java @@ -1,4 +1,4 @@ -package org.sufficientlysecure.keychain.tests; +package org.sufficientlysecure.keychain.pgp; import junit.framework.AssertionFailedError; @@ -19,12 +19,6 @@ import org.spongycastle.bcpg.sig.KeyFlags; import org.spongycastle.openpgp.PGPSignature; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.Constants.choice.algorithm; -import org.sufficientlysecure.keychain.pgp.CanonicalizedKeyRing; -import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKeyRing; -import org.sufficientlysecure.keychain.pgp.PgpKeyOperation; -import org.sufficientlysecure.keychain.pgp.UncachedKeyRing; -import org.sufficientlysecure.keychain.pgp.UncachedPublicKey; -import org.sufficientlysecure.keychain.pgp.WrappedSignature; import org.sufficientlysecure.keychain.service.OperationResultParcel.OperationLog; import org.sufficientlysecure.keychain.service.SaveKeyringParcel; import org.sufficientlysecure.keychain.service.SaveKeyringParcel.SubkeyAdd; diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/UncachedKeyringCanonicalizeTest.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringCanonicalizeTest.java similarity index 98% rename from OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/UncachedKeyringCanonicalizeTest.java rename to OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringCanonicalizeTest.java index 97e0d8a68..24b9a3fb6 100644 --- a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/UncachedKeyringCanonicalizeTest.java +++ b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringCanonicalizeTest.java @@ -1,4 +1,4 @@ -package org.sufficientlysecure.keychain.tests; +package org.sufficientlysecure.keychain.pgp; import org.junit.BeforeClass; import org.junit.runner.RunWith; @@ -26,11 +26,6 @@ import org.spongycastle.openpgp.operator.jcajce.JcaKeyFingerprintCalculator; import org.spongycastle.openpgp.operator.jcajce.JcaPGPContentSignerBuilder; import org.spongycastle.openpgp.operator.jcajce.JcePBESecretKeyDecryptorBuilder; import org.sufficientlysecure.keychain.Constants; -import org.sufficientlysecure.keychain.pgp.CanonicalizedKeyRing; -import org.sufficientlysecure.keychain.pgp.PgpKeyOperation; -import org.sufficientlysecure.keychain.pgp.UncachedKeyRing; -import org.sufficientlysecure.keychain.pgp.UncachedPublicKey; -import org.sufficientlysecure.keychain.pgp.WrappedSignature; import org.sufficientlysecure.keychain.service.OperationResultParcel; import org.sufficientlysecure.keychain.service.OperationResults.EditKeyResult; import org.sufficientlysecure.keychain.service.SaveKeyringParcel; diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/UncachedKeyringMergeTest.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringMergeTest.java similarity index 97% rename from OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/UncachedKeyringMergeTest.java rename to OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringMergeTest.java index 977ddfc52..6b19e3c55 100644 --- a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/UncachedKeyringMergeTest.java +++ b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringMergeTest.java @@ -1,4 +1,4 @@ -package org.sufficientlysecure.keychain.tests; +package org.sufficientlysecure.keychain.pgp; import org.junit.Assert; import org.junit.Before; @@ -10,13 +10,6 @@ import org.robolectric.shadows.ShadowLog; import org.spongycastle.bcpg.PacketTags; import org.spongycastle.bcpg.sig.KeyFlags; import org.sufficientlysecure.keychain.Constants; -import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKeyRing; -import org.sufficientlysecure.keychain.pgp.PgpKeyOperation; -import org.sufficientlysecure.keychain.pgp.UncachedKeyRing; -import org.sufficientlysecure.keychain.pgp.UncachedPublicKey; -import org.sufficientlysecure.keychain.pgp.CanonicalizedPublicKeyRing; -import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKey; -import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKeyRing; import org.sufficientlysecure.keychain.service.OperationResultParcel; import org.sufficientlysecure.keychain.service.OperationResults.EditKeyResult; import org.sufficientlysecure.keychain.service.SaveKeyringParcel; diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/UncachedKeyringTest.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringTest.java similarity index 92% rename from OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/UncachedKeyringTest.java rename to OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringTest.java index 15aaa4c5d..e0772a7dc 100644 --- a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/UncachedKeyringTest.java +++ b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringTest.java @@ -1,4 +1,4 @@ -package org.sufficientlysecure.keychain.tests; +package org.sufficientlysecure.keychain.pgp; import org.junit.Assert; import org.junit.Before; @@ -9,21 +9,15 @@ import org.robolectric.RobolectricTestRunner; import org.robolectric.shadows.ShadowLog; import org.spongycastle.bcpg.sig.KeyFlags; import org.sufficientlysecure.keychain.Constants; -import org.sufficientlysecure.keychain.pgp.PgpKeyOperation; -import org.sufficientlysecure.keychain.pgp.UncachedKeyRing; -import org.sufficientlysecure.keychain.pgp.UncachedPublicKey; import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException; -import org.sufficientlysecure.keychain.service.OperationResultParcel; import org.sufficientlysecure.keychain.service.OperationResults.EditKeyResult; import org.sufficientlysecure.keychain.service.SaveKeyringParcel; import org.sufficientlysecure.keychain.support.KeyringTestingHelper.RawPacket; -import org.sufficientlysecure.keychain.util.ProgressScaler; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.util.ArrayList; import java.util.Iterator; -import java.util.List; @RunWith(RobolectricTestRunner.class) @org.robolectric.annotation.Config(emulateSdk = 18) // Robolectric doesn't yet support 19 diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/ProviderHelperKeyringTest.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/provider/ProviderHelperKeyringTest.java similarity index 98% rename from OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/ProviderHelperKeyringTest.java rename to OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/provider/ProviderHelperKeyringTest.java index 665e8ef2b..b0f47af29 100644 --- a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/ProviderHelperKeyringTest.java +++ b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/provider/ProviderHelperKeyringTest.java @@ -15,7 +15,7 @@ * along with this program. If not, see . */ -package tests; +package org.sufficientlysecure.keychain.provider; import java.util.Collections; import java.util.Arrays; diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/FileImportCache.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/FileImportCache.java deleted file mode 100644 index 08b8afae7..000000000 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/FileImportCache.java +++ /dev/null @@ -1,97 +0,0 @@ -/* - * Copyright (C) 2014 Dominik Schürmann - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - */ - -package org.sufficientlysecure.keychain.keyimport; - -import android.content.Context; -import android.os.Bundle; -import android.os.Parcel; - -import org.sufficientlysecure.keychain.KeychainApplication; - -import java.io.File; -import java.io.FileInputStream; -import java.io.FileOutputStream; -import java.io.IOException; -import java.util.ArrayList; -import java.util.List; - -/** - * When sending large data (over 1MB) through Androids Binder IPC you get - * JavaBinder E !!! FAILED BINDER TRANSACTION !!! - *

- * To overcome this problem, we cache large Parcelables into a file in our private cache directory - * instead of sending them through IPC. - */ -public class FileImportCache { - - private Context mContext; - - private static final String FILENAME = "key_import.pcl"; - private static final String BUNDLE_DATA = "data"; - - public FileImportCache(Context context) { - this.mContext = context; - } - - public void writeCache(ArrayList selectedEntries) throws IOException { - Bundle in = new Bundle(); - in.putParcelableArrayList(BUNDLE_DATA, selectedEntries); - File cacheDir = mContext.getCacheDir(); - if (cacheDir == null) { - // https://groups.google.com/forum/#!topic/android-developers/-694j87eXVU - throw new IOException("cache dir is null!"); - } - File tempFile = new File(mContext.getCacheDir(), FILENAME); - - FileOutputStream fos = new FileOutputStream(tempFile); - Parcel p = Parcel.obtain(); // creating empty parcel object - in.writeToParcel(p, 0); // saving bundle as parcel - fos.write(p.marshall()); // writing parcel to file - fos.flush(); - fos.close(); - } - - public List readCache() throws IOException { - Parcel parcel = Parcel.obtain(); // creating empty parcel object - Bundle out; - File cacheDir = mContext.getCacheDir(); - if (cacheDir == null) { - // https://groups.google.com/forum/#!topic/android-developers/-694j87eXVU - throw new IOException("cache dir is null!"); - } - - File tempFile = new File(cacheDir, FILENAME); - try { - FileInputStream fis = new FileInputStream(tempFile); - byte[] array = new byte[(int) fis.getChannel().size()]; - fis.read(array, 0, array.length); - fis.close(); - - parcel.unmarshall(array, 0, array.length); - parcel.setDataPosition(0); - out = parcel.readBundle(KeychainApplication.class.getClassLoader()); - out.putAll(out); - - return out.getParcelableArrayList(BUNDLE_DATA); - } finally { - parcel.recycle(); - // delete temp file - tempFile.delete(); - } - } -} diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/FileImportCache.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/FileImportCache.java new file mode 100644 index 000000000..e09cdd502 --- /dev/null +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/FileImportCache.java @@ -0,0 +1,179 @@ +/* + * Copyright (C) 2014 Dominik Schürmann + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package org.sufficientlysecure.keychain.util; + +import android.content.Context; +import android.os.Parcel; +import android.os.Parcelable; + +import org.sufficientlysecure.keychain.Constants; +import org.sufficientlysecure.keychain.KeychainApplication; +import org.sufficientlysecure.keychain.util.Log; + +import java.io.File; +import java.io.FileInputStream; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.ObjectInputStream; +import java.io.ObjectOutputStream; +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; + +/** + * When sending large data (over 1MB) through Androids Binder IPC you get + * JavaBinder E !!! FAILED BINDER TRANSACTION !!! + *

+ * To overcome this problem, we cache large Parcelables into a file in our private cache directory + * instead of sending them through IPC. + */ +public class FileImportCache { + + private Context mContext; + + private static final String FILENAME = "key_import.pcl"; + + public FileImportCache(Context context) { + this.mContext = context; + } + + public void writeCache(ArrayList selectedEntries) throws IOException { + writeCache(selectedEntries.iterator()); + } + + public void writeCache(Iterator it) throws IOException { + + File cacheDir = mContext.getCacheDir(); + if (cacheDir == null) { + // https://groups.google.com/forum/#!topic/android-developers/-694j87eXVU + throw new IOException("cache dir is null!"); + } + + File tempFile = new File(mContext.getCacheDir(), FILENAME); + + ObjectOutputStream oos = new ObjectOutputStream(new FileOutputStream(tempFile)); + + while (it.hasNext()) { + E ring = it.next(); + Parcel p = Parcel.obtain(); // creating empty parcel object + p.writeParcelable(ring, 0); // saving bundle as parcel + oos.writeObject(p.marshall()); // writing parcel to file + p.recycle(); + } + + oos.close(); + + } + + public List readCacheIntoList() throws IOException { + ArrayList result = new ArrayList(); + Iterator it = readCache(); + while (it.hasNext()) { + result.add(it.next()); + } + return result; + } + + public Iterator readCache() throws IOException { + + File cacheDir = mContext.getCacheDir(); + if (cacheDir == null) { + // https://groups.google.com/forum/#!topic/android-developers/-694j87eXVU + throw new IOException("cache dir is null!"); + } + + final File tempFile = new File(cacheDir, FILENAME); + final ObjectInputStream ois = new ObjectInputStream(new FileInputStream(tempFile)); + + return new Iterator() { + + E mRing = null; + boolean closed = false; + + private void readNext() { + if (mRing != null || closed) { + Log.e(Constants.TAG, "err!"); + return; + } + + try { + if (ois.available() == 0) { + return; + } + + byte[] data = (byte[]) ois.readObject(); + Log.e(Constants.TAG, "bla"); + if (data == null) { + if (!closed) { + closed = true; + ois.close(); + tempFile.delete(); + } + return; + } + + Parcel parcel = Parcel.obtain(); // creating empty parcel object + parcel.unmarshall(data, 0, data.length); + parcel.setDataPosition(0); + mRing = parcel.readParcelable(KeychainApplication.class.getClassLoader()); + parcel.recycle(); + } catch (ClassNotFoundException e) { + Log.e(Constants.TAG, "Encountered ClassNotFoundException during cache read, this is a bug!"); + } catch (IOException e) { + Log.e(Constants.TAG, "Encountered IOException during cache read!"); + } + + } + + @Override + public boolean hasNext() { + readNext(); + return mRing != null; + } + + @Override + public E next() { + readNext(); + try { + return mRing; + } finally { + mRing = null; + } + } + + @Override + public void remove() { + throw new UnsupportedOperationException(); + } + + @Override + public void finalize() throws Throwable { + super.finalize(); + if (!closed) { + try { + ois.close(); + tempFile.delete(); + } catch (IOException e) { + // never mind + } + } + } + + }; + } +} From c0edaf9a5ea7629ebfcf2250cf5684370120bb17 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Fri, 1 Aug 2014 16:49:41 +0200 Subject: [PATCH 2/4] make FileImportCache generic, iterable, and add unit test --- .../keychain/util/FileImportCacheTest.java | 54 +++++++++++++++++++ .../service/KeychainIntentService.java | 10 ++-- .../keychain/ui/ImportKeysActivity.java | 4 +- .../keychain/util/FileImportCache.java | 52 +++++++++--------- 4 files changed, 89 insertions(+), 31 deletions(-) create mode 100644 OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/util/FileImportCacheTest.java diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/util/FileImportCacheTest.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/util/FileImportCacheTest.java new file mode 100644 index 000000000..b5708b46f --- /dev/null +++ b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/util/FileImportCacheTest.java @@ -0,0 +1,54 @@ +package org.sufficientlysecure.keychain.util; + +import android.os.Bundle; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.Robolectric; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.shadows.ShadowLog; + +import java.util.ArrayList; +import java.util.List; + +@RunWith(RobolectricTestRunner.class) +@org.robolectric.annotation.Config(emulateSdk = 18) // Robolectric doesn't yet support 19 +public class FileImportCacheTest { + + @Before + public void setUp() throws Exception { + ShadowLog.stream = System.out; + } + + @Test + public void testInputOutput() throws Exception { + + FileImportCache cache = new FileImportCache(Robolectric.application); + + ArrayList list = new ArrayList(); + + for (int i = 0; i < 50; i++) { + Bundle b = new Bundle(); + b.putInt("key1", i); + b.putString("key2", Integer.toString(i)); + list.add(b); + } + + // write to cache file + cache.writeCache(list); + + // read back + List last = cache.readCacheIntoList(); + + for (int i = 0; i < list.size(); i++) { + Assert.assertEquals("input values should be equal to output values", + list.get(i).getInt("key1"), last.get(i).getInt("key1")); + Assert.assertEquals("input values should be equal to output values", + list.get(i).getString("key2"), last.get(i).getString("key2")); + } + + } + +} diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java index 8d4717eeb..c87e490be 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java @@ -24,6 +24,7 @@ import android.net.Uri; import android.os.Bundle; import android.os.Message; import android.os.Messenger; +import android.os.Parcel; import android.os.RemoteException; import org.sufficientlysecure.keychain.Constants; @@ -31,7 +32,7 @@ import org.sufficientlysecure.keychain.R; import org.sufficientlysecure.keychain.helper.FileHelper; import org.sufficientlysecure.keychain.helper.OtherHelper; import org.sufficientlysecure.keychain.helper.Preferences; -import org.sufficientlysecure.keychain.keyimport.FileImportCache; +import org.sufficientlysecure.keychain.util.FileImportCache; import org.sufficientlysecure.keychain.keyimport.HkpKeyserver; import org.sufficientlysecure.keychain.keyimport.ImportKeysListEntry; import org.sufficientlysecure.keychain.keyimport.KeybaseKeyserver; @@ -387,14 +388,16 @@ public class KeychainIntentService extends IntentService } } else if (ACTION_IMPORT_KEYRING.equals(action)) { try { + List entries; if (data.containsKey(IMPORT_KEY_LIST)) { // get entries from intent entries = data.getParcelableArrayList(IMPORT_KEY_LIST); } else { // get entries from cached file - FileImportCache cache = new FileImportCache(this); - entries = cache.readCache(); + FileImportCache cache = + new FileImportCache(this); + entries = cache.readCacheIntoList(); } PgpImportExport pgpImportExport = new PgpImportExport(this, this); @@ -523,6 +526,7 @@ public class KeychainIntentService extends IntentService Intent importIntent = new Intent(this, KeychainIntentService.class); importIntent.setAction(ACTION_IMPORT_KEYRING); + Bundle importData = new Bundle(); // This is not going through binder, nothing to fear of importData.putParcelableArrayList(IMPORT_KEY_LIST, keyRings); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysActivity.java index dbc557f9a..4a606a1b3 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysActivity.java @@ -40,7 +40,7 @@ import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.R; import org.sufficientlysecure.keychain.helper.OtherHelper; import org.sufficientlysecure.keychain.helper.Preferences; -import org.sufficientlysecure.keychain.keyimport.FileImportCache; +import org.sufficientlysecure.keychain.util.FileImportCache; import org.sufficientlysecure.keychain.keyimport.ImportKeysListEntry; import org.sufficientlysecure.keychain.keyimport.ParcelableKeyRing; import org.sufficientlysecure.keychain.pgp.PgpKeyHelper; @@ -503,7 +503,7 @@ public class ImportKeysActivity extends ActionBarActivity { // to prevent Java Binder problems on heavy imports // read FileImportCache for more info. try { - FileImportCache cache = new FileImportCache(this); + FileImportCache cache = new FileImportCache(this); cache.writeCache(selectedEntries); intent.putExtra(KeychainIntentService.EXTRA_DATA, data); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/FileImportCache.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/FileImportCache.java index e09cdd502..5a4bf5311 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/FileImportCache.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/FileImportCache.java @@ -23,14 +23,14 @@ import android.os.Parcelable; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.KeychainApplication; -import org.sufficientlysecure.keychain.util.Log; +import java.io.DataInputStream; +import java.io.DataOutputStream; +import java.io.EOFException; import java.io.File; import java.io.FileInputStream; import java.io.FileOutputStream; import java.io.IOException; -import java.io.ObjectInputStream; -import java.io.ObjectOutputStream; import java.util.ArrayList; import java.util.Iterator; import java.util.List; @@ -66,13 +66,14 @@ public class FileImportCache { File tempFile = new File(mContext.getCacheDir(), FILENAME); - ObjectOutputStream oos = new ObjectOutputStream(new FileOutputStream(tempFile)); + DataOutputStream oos = new DataOutputStream(new FileOutputStream(tempFile)); while (it.hasNext()) { - E ring = it.next(); Parcel p = Parcel.obtain(); // creating empty parcel object - p.writeParcelable(ring, 0); // saving bundle as parcel - oos.writeObject(p.marshall()); // writing parcel to file + p.writeParcelable(it.next(), 0); // saving bundle as parcel + byte[] buf = p.marshall(); + oos.writeInt(buf.length); + oos.write(buf); p.recycle(); } @@ -98,44 +99,37 @@ public class FileImportCache { } final File tempFile = new File(cacheDir, FILENAME); - final ObjectInputStream ois = new ObjectInputStream(new FileInputStream(tempFile)); + final DataInputStream ois = new DataInputStream(new FileInputStream(tempFile)); return new Iterator() { E mRing = null; boolean closed = false; + byte[] buf = new byte[512]; private void readNext() { if (mRing != null || closed) { - Log.e(Constants.TAG, "err!"); return; } try { - if (ois.available() == 0) { - return; - } - byte[] data = (byte[]) ois.readObject(); - Log.e(Constants.TAG, "bla"); - if (data == null) { - if (!closed) { - closed = true; - ois.close(); - tempFile.delete(); - } - return; + int length = ois.readInt(); + while (buf.length < length) { + buf = new byte[buf.length * 2]; } + ois.readFully(buf, 0, length); Parcel parcel = Parcel.obtain(); // creating empty parcel object - parcel.unmarshall(data, 0, data.length); + parcel.unmarshall(buf, 0, length); parcel.setDataPosition(0); mRing = parcel.readParcelable(KeychainApplication.class.getClassLoader()); parcel.recycle(); - } catch (ClassNotFoundException e) { - Log.e(Constants.TAG, "Encountered ClassNotFoundException during cache read, this is a bug!"); + } catch (EOFException e) { + // aight + close(); } catch (IOException e) { - Log.e(Constants.TAG, "Encountered IOException during cache read!"); + Log.e(Constants.TAG, "Encountered IOException during cache read!", e); } } @@ -163,17 +157,23 @@ public class FileImportCache { @Override public void finalize() throws Throwable { + close(); super.finalize(); + } + + private void close() { if (!closed) { try { ois.close(); tempFile.delete(); } catch (IOException e) { - // never mind + // nvm } } + closed = true; } + }; } } From 827a958e123184ae70c2c5e5b10ab47f50ef1a52 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Fri, 1 Aug 2014 17:00:08 +0200 Subject: [PATCH 3/4] remove debug output about security providers (cleaner unit tests) --- .../org/sufficientlysecure/keychain/KeychainApplication.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/KeychainApplication.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/KeychainApplication.java index dfd39b345..e70b134aa 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/KeychainApplication.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/KeychainApplication.java @@ -61,6 +61,7 @@ public class KeychainApplication extends Application { PRNGFixes.apply(); Log.d(Constants.TAG, "Bouncy Castle set and PRNG Fixes applied!"); + /* if (Constants.DEBUG) { Provider[] providers = Security.getProviders(); Log.d(Constants.TAG, "Installed Security Providers:"); @@ -68,6 +69,7 @@ public class KeychainApplication extends Application { Log.d(Constants.TAG, "provider class: " + p.getClass().getName()); } } + */ // Create APG directory on sdcard if not existing if (Environment.getExternalStorageState().equals(Environment.MEDIA_MOUNTED)) { From f555447011a4d975e87d99ddc5f512c3205c2211 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Fri, 1 Aug 2014 17:36:34 +0200 Subject: [PATCH 4/4] fix dumb mistake in modifySecretKey --- .../keychain/pgp/PgpKeyOperationTest.java | 18 ++++++++---------- .../keychain/pgp/PgpKeyOperation.java | 2 +- .../keychain/pgp/UncachedPublicKey.java | 6 +----- 3 files changed, 10 insertions(+), 16 deletions(-) diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java index 516d1ac63..0e0ec7ee2 100644 --- a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java +++ b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java @@ -34,6 +34,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Date; import java.util.Iterator; +import java.util.List; import java.util.Random; @RunWith(RobolectricTestRunner.class) @@ -202,8 +203,8 @@ public class PgpKeyOperationTest { Assert.assertEquals("number of user ids must be two", 2, ring.getPublicKey().getUnorderedUserIds().size()); - Assert.assertEquals("number of subkeys must be three", - 3, KeyringTestingHelper.itToList(ring.getPublicKeys()).size()); + List subkeys = KeyringTestingHelper.itToList(ring.getPublicKeys()); + Assert.assertEquals("number of subkeys must be three", 3, subkeys.size()); Assert.assertTrue("key ring should have been created in the last 120 seconds", ring.getPublicKey().getCreationTime().after(new Date(new Date().getTime()-1000*120))); @@ -211,24 +212,21 @@ public class PgpKeyOperationTest { Assert.assertNull("key ring should not expire", ring.getPublicKey().getExpiryTime()); - Iterator it = ring.getPublicKeys(); - Assert.assertEquals("first (master) key can certify", - KeyFlags.CERTIFY_OTHER, it.next().getKeyUsage()); + KeyFlags.CERTIFY_OTHER, subkeys.get(0).getKeyUsage()); - UncachedPublicKey signingKey = it.next(); Assert.assertEquals("second key can sign", - KeyFlags.SIGN_DATA, signingKey.getKeyUsage()); - ArrayList sigs = signingKey.getSignatures().next().getEmbeddedSignatures(); + KeyFlags.SIGN_DATA, subkeys.get(1).getKeyUsage()); + ArrayList sigs = subkeys.get(1).getSignatures().next().getEmbeddedSignatures(); Assert.assertEquals("signing key signature should have one embedded signature", 1, sigs.size()); Assert.assertEquals("embedded signature should be of primary key binding type", PGPSignature.PRIMARYKEY_BINDING, sigs.get(0).getSignatureType()); Assert.assertEquals("primary key binding signature issuer should be signing subkey", - signingKey.getKeyId(), sigs.get(0).getKeyId()); + subkeys.get(1).getKeyId(), sigs.get(0).getKeyId()); Assert.assertEquals("third key can encrypt", - KeyFlags.ENCRYPT_COMMS, it.next().getKeyUsage()); + KeyFlags.ENCRYPT_COMMS, subkeys.get(2).getKeyUsage()); } 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 861f93446..19b0d81b7 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java @@ -609,7 +609,7 @@ public class PgpKeyOperation { for (int i = 0; i < saveParcel.mAddSubKeys.size(); i++) { progress(R.string.progress_modify_subkeyadd, (i-1) * (100 / saveParcel.mAddSubKeys.size())); - SaveKeyringParcel.SubkeyAdd add = saveParcel.mAddSubKeys.get(0); + SaveKeyringParcel.SubkeyAdd add = saveParcel.mAddSubKeys.get(i); log.add(LogLevel.INFO, LogType.MSG_MF_SUBKEY_NEW, indent); if (add.mExpiry != null && new Date(add.mExpiry*1000).before(new Date())) { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedPublicKey.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedPublicKey.java index 358b1c552..4a03d942b 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedPublicKey.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedPublicKey.java @@ -169,6 +169,7 @@ public class UncachedPublicKey { } @SuppressWarnings("unchecked") + // TODO make this safe public int getKeyUsage() { if(mCacheUsage == null) { mCacheUsage = 0; @@ -182,11 +183,6 @@ public class UncachedPublicKey { if (hashed != null) { mCacheUsage |= hashed.getKeyFlags(); } - - PGPSignatureSubpacketVector unhashed = sig.getUnhashedSubPackets(); - if (unhashed != null) { - mCacheUsage |= unhashed.getKeyFlags(); - } } } }