Merge branch 'master' of github.com:open-keychain/open-keychain

OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/provider/ProviderHelperSaveTest.java

@@ -0,0 +1,55 @@
+package org.sufficientlysecure.keychain.provider;
+
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.Robolectric;
+import org.robolectric.RobolectricTestRunner;
+import org.robolectric.shadows.ShadowLog;
+import org.sufficientlysecure.keychain.pgp.UncachedKeyRing;
+import org.sufficientlysecure.keychain.service.OperationResults.SaveKeyringResult;
+
+import java.io.IOException;
+import java.util.Iterator;
+
+@RunWith(RobolectricTestRunner.class)
+@org.robolectric.annotation.Config(emulateSdk = 18) // Robolectric doesn't yet support 19
+public class ProviderHelperSaveTest {
+
+    @BeforeClass
+    public static void setUpOnce() throws Exception {
+        ShadowLog.stream = System.out;
+    }
+
+    @Test
+    public void testLongKeyIdCollision() throws Exception {
+
+        UncachedKeyRing first =
+                readRingFromResource("/cooperpair/9E669861368BCA0BE42DAF7DDDA252EBB8EBE1AF.asc");
+        UncachedKeyRing second =
+                readRingFromResource("/cooperpair/A55120427374F3F7AA5F1166DDA252EBB8EBE1AF.asc");
+
+        SaveKeyringResult result;
+
+        // insert both keys, second should fail
+        result = new ProviderHelper(Robolectric.application).savePublicKeyRing(first);
+        Assert.assertTrue("first keyring import should succeed", result.success());
+        result = new ProviderHelper(Robolectric.application).savePublicKeyRing(second);
+        Assert.assertFalse("second keyring import should fail", result.success());
+
+        new KeychainDatabase(Robolectric.application).clearDatabase();
+
+        // and the other way around
+        result = new ProviderHelper(Robolectric.application).savePublicKeyRing(second);
+        Assert.assertTrue("first keyring import should succeed", result.success());
+        result = new ProviderHelper(Robolectric.application).savePublicKeyRing(first);
+        Assert.assertFalse("second keyring import should fail", result.success());
+
+    }
+
+    UncachedKeyRing readRingFromResource(String name) throws Exception {
+        return UncachedKeyRing.fromStream(ProviderHelperSaveTest.class.getResourceAsStream(name)).next();
+    }
+
+}

OpenKeychain-Test/src/test/resources/cooperpair/9E669861368BCA0BE42DAF7DDDA252EBB8EBE1AF.asc

@@ -0,0 +1,29 @@
+-----BEGIN PGP PUBLIC KEY BLOCK-----
+Version: GnuPG v1
+
+mQINBFJtd/UBEACpw/psXoGNM8RHczviD7FnGdjMQPEJQ+nuWQ2AEGYouulg5hFv
+0ChuSQVLiqQht2k5K2liyW1MeXoJ8tr9nSn/Zi9nttc0Wo6K7pvrDD40r2HNg305
+qLCzItr5st3x8cq2cIXvN4LOm2rqpBLZ/sqMmNiW2Y7/aAQqV1xtR35joHqamWHD
+UPOmzBMs07YSUjXgC1EMx8kWQSV6cuARj93kxWj8R6eoYHHfrWCEGR313wov6QST
+zIfVU7FqQqOmdLW3LaPHxcrI/TjsnkUN99qdlpjJH/YW925LDPJHAkliqPP5AvhU
+F9KbY2F8mcIZBCDd8TH+xXynuN3BbIU4kCwVbdx/tcpO1npuJcKB1Go/udyow/Ei
+Z3nHzJsCVkezvopek77wnwPaP0nAb7f4iIY3gJCoGirOx6N075TgF6MBe00q9oFE
+y4rvnUnU9/QzOOes95eUMhM+9eK1cuLFEV5t47DfxRdq+fQip3FJ2l6v19sZvQ0G
+j06pjYqg0of273rG8oXcDrFjb1Zqhj8x1mLl6u7d/ide5wTm9HylBWcYKQjIJJAi
+WIScxEPIOINDJKgsKTuKtoyNvISJ3xUeS1yzxiIb3YGLIyPgFFx0vFyqJfbkXq70
+m1n2xnJlkTidfzbZvc6EA7vRGSDYK6FqqhlGhc7UypUEVW8FM/jZNAOS6QARAUGt
+tCg5RTY2OTg2MTM2OEJDQTBCRTQyREFGN0REREEyNTJFQkI4RUJFMUFGiQI3BBMB
+CgAhBQJSg/uTAhsDBQsJCAcDBRUKCQgLBRYCAwEAAh4BAheAAAoJEN2iUuu46+Gv
++Z0P+wQhkLwm+WGcEsS98Lei9O7hit/k4g/VkLUUQV7BOR3n8uRZIFkdOtpvrFU3
+aKf246uCy6GM48Oh+1U2cv5InX/WEuKaFo5uF6t79wyt18BUn1weDcU+DQdOSG4f
+fSnNa55wkN0l0svW4fGIthjmDTz6HZFntYD+9A20wZAqpPIs+vyG9Jp+e9E9Y/W/
+EFQbNlxHHb9+BMT2+DtNP+HSl3MPFlQPKOLZxyLAU5uzT0Sa0LxhrQy5FgkW6Jog
+sbAJVM9z0pZw+grzGPciM66ZW1rxeICvbYsdWLytRjqxpY8GS8XudyseUGd+dZim
+ptarsrE5yfSMg2gW5Z1PTc0tEMXJLUwtpyzQjpFpbb7dPuo2TUp09LgZKX63WCbS
+Nb1RTaGfkeYudOTo2rh4Jfg+Tb/JRpO6clo0rxAq8nPH2WmG+9TB8Zbb7YRzGWuV
+/e5SeVNR+zY8tXZKnmUIH1HIprc+BtT6Bupdvd0CT14Mg9MmsFvUXofwHLa4gahr
+8/iG9y3uHSA6Rhz++yOpyOmNvO1LDxsYNaRCIXQJbqgNwF5YNYlMPsEeY/CG7FOb
+Afv7rHiYtRRQfz2P4OF900DJO7QL9gdNXJ1+Hajy/5Lvvl7qwqMG4GvVQEsgFc5O
+jjFCUhE2i20j2kEMxvA5RLBH/fOoGARn87tiKSfb+pqLNZQb
+=fDJ8
+-----END PGP PUBLIC KEY BLOCK-----

OpenKeychain-Test/src/test/resources/cooperpair/A55120427374F3F7AA5F1166DDA252EBB8EBE1AF.asc

@@ -0,0 +1,29 @@
+-----BEGIN PGP PUBLIC KEY BLOCK-----
+Version: GnuPG v1
+
+mQINBFKD+38BEADSv5l4xOx9hCRJVcybq6yK5hTpGSFf3xo1bkhoMvyC62ehb4jD
+MDLwwNRyzCBEWQJLbq/LLizPFN2qXFJpXJcsuqsHNYRtDqDBEjtriRQwSqHnqTXt
+c0K46FYHldCJQ4/tBXxPI+WwtXjcNRWaV7n2BvR/Jk+B5e4Zz3LPnN0C4w5vORHs
+hN1jil8A3Hs/F+OmlQYrU8ZtNwTpSo2EXxe2fVgSDCsKRyNsPZj++OyujPzW+yaN
+lJ9I/q6s9gvX9o9o7nwZbqBETipWsdRK6RfBdTKpnyLNordbWwWTk6GxN8T5Ppit
+P6a3UlQ71VuflcswCTmEQ1pEfZrlRFKa9psBOW+cZLNxT9h0jGFMh6/B3w48Sag+
+cFcPBFWParC+cAXBIURDxT9G6bzNLogg7YKoaPsyiXnLDH2VJUCXs27D2wPJL24Q
+S7npvsg63MPPssWgG5cauLznmNR4y5pQi6oH/C10v0zrUJy6FPJzQhYRhWOvhtz6
+j88RGMrFNNCdB2VACtn699D+ixu3nRlXHIKCT+xLSfgslVYifmJOCNljBLGHOQ1e
+FJxQuNVpmmxjvk/8kqK+pHLB9Qn6M1ZYzip7OyUL3OAWabCabgEw2bQmUhiBWD3u
+buv0WAVOJEAFvBCAeYNQzrQMY+Rc3RnvynG4pI6Tbo8wC6/IJcDOw516JwARASB3
+tChBNTUxMjA0MjczNzRGM0Y3QUE1RjExNjZEREEyNTJFQkI4RUJFMUFGiQI3BBMB
+CgAhBQJSg/uTAhsDBQsJCAcDBRUKCQgLBRYCAwEAAh4BAheAAAoJEN2iUuu46+Gv
+9L0P/3tFu0LOZ/dAPjUNfKJCZqcIuVnD5xShMTsUbVx+QoXMy7rt4iRLD7ofGi/I
+vTAZehxk3sk/Slx5nbews+3NItyw6mcaP9HlmwKNr6k7BC2kJHcCxH4DNzhmIx1H
+3T/CggtHX42JBYKlGf22y+M8jAbvsPOUfTznx96mYNrOY6s1dJyn0kRleqJ8+tGj
+/5+0y90iZnGCa0FtacQkKUPkXwVodeZVxk8z5OEipShYKc+8dl+5WsvOzHqLC/KY
+xCGRb4JaqEMwouLNg8dTNAXXUvFGqJNDX4+andggogmI1hdD9xExfSU9cAGegg2t
+vvveC4S+CCHd+zt88iK5ze6F61RxwYhhNbkuFGjdgNGCpHtG/BQhKnYJuKEbq3oi
+mgNyxJERlfgaWXveiMG0AmACXN+jCkTtqZjQnsg2N2QDL3tjY7usmuiwRL1aVOFG
+Kw5/Cc+2nDeANS3Xi1403Ni269b1c6kNSoLe4zd0WsbO3Kouds8F8EQfeheXQe97
+ZxuvBOMsR9wHC3f0sl/vfxCGdUC+khmKk5taKnUeUFJmVmh5ghlVy8FySHGB0QHO
+zd8GUl59rFpQJNpNFQW2YKDhrcjxIr2AeJrdoDI6NsQ02+Qtep/bbq53hqtAD4jF
+t3S8vBbTXtRk6g2qn4ojF4SOIc8SAiZcURgVFuSJX8ngFbO4
+=OEw/
+-----END PGP PUBLIC KEY BLOCK-----

OpenKeychain-Test/src/test/resources/cooperpair/readme

@@ -0,0 +1,3 @@
+"Cooperpair" testcase under public domain license, by @coruus:
+
+https://github.com/coruus/cooperpair/tree/master/pgpv4

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainDatabase.java

@@ -349,4 +349,9 @@ public class KeychainDatabase extends SQLiteOpenHelper {
         copy(in, out);
     }
 
+    // for test cases ONLY!!
+    public void clearDatabase() {
+        getWritableDatabase().execSQL("delete from " + Tables.KEY_RINGS_PUBLIC);
+    }
+
 }

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java

@@ -86,6 +86,10 @@ public class ProviderHelper {
         this(context, new OperationLog(), 0);
     }
 
+    public ProviderHelper(Context context, OperationLog log) {
+        this(context, log, 0);
+    }
+
     public ProviderHelper(Context context, OperationLog log, int indent) {
         mContext = context;
         mContentResolver = context.getContentResolver();

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java

@@ -54,6 +54,7 @@ import org.sufficientlysecure.keychain.provider.KeychainDatabase;
 import org.sufficientlysecure.keychain.provider.ProviderHelper;
 import org.sufficientlysecure.keychain.service.OperationResults.EditKeyResult;
 import org.sufficientlysecure.keychain.service.OperationResults.ImportKeyResult;
+import org.sufficientlysecure.keychain.service.OperationResults.SaveKeyringResult;
 import org.sufficientlysecure.keychain.util.FileImportCache;
 import org.sufficientlysecure.keychain.util.InputData;
 import org.sufficientlysecure.keychain.util.Log;
@@ -391,23 +392,36 @@ public class KeychainIntentService extends IntentService
                 }
 
                 /* Operation */
-                ProviderHelper providerHelper = new ProviderHelper(this);
                 PgpKeyOperation keyOperations = new PgpKeyOperation(new ProgressScaler(this, 10, 60, 100));
-                EditKeyResult result;
+                EditKeyResult modifyResult;
 
                 if (saveParcel.mMasterKeyId != null) {
                     String passphrase = data.getString(SAVE_KEYRING_PASSPHRASE);
                     CanonicalizedSecretKeyRing secRing =
-                            providerHelper.getCanonicalizedSecretKeyRing(saveParcel.mMasterKeyId);
+                            new ProviderHelper(this).getCanonicalizedSecretKeyRing(saveParcel.mMasterKeyId);
 
-                    result = keyOperations.modifySecretKeyRing(secRing, saveParcel, passphrase);
+                    modifyResult = keyOperations.modifySecretKeyRing(secRing, saveParcel, passphrase);
                 } else {
-                    result = keyOperations.createSecretKeyRing(saveParcel);
+                    modifyResult = keyOperations.createSecretKeyRing(saveParcel);
                 }
 
-                UncachedKeyRing ring = result.getRing();
+                // If the edit operation didn't succeed, exit here
+                if ( ! modifyResult.success()) {
+                    sendMessageToHandler(KeychainIntentServiceHandler.MESSAGE_OKAY, modifyResult);
+                    return;
+                }
 
-                providerHelper.saveSecretKeyRing(ring, new ProgressScaler(this, 60, 95, 100));
+                UncachedKeyRing ring = modifyResult.getRing();
+
+                // Save the keyring. The ProviderHelper is initialized with the previous log
+                SaveKeyringResult saveResult = new ProviderHelper(this, modifyResult.getLog())
+                        .saveSecretKeyRing(ring, new ProgressScaler(this, 60, 95, 100));
+
+                // If the edit operation didn't succeed, exit here
+                if ( ! saveResult.success()) {
+                    sendMessageToHandler(KeychainIntentServiceHandler.MESSAGE_OKAY, saveResult);
+                    return;
+                }
 
                 // cache new passphrase
                 if (saveParcel.mNewPassphrase != null) {
@@ -418,7 +432,7 @@ public class KeychainIntentService extends IntentService
                 setProgress(R.string.progress_done, 100, 100);
 
                 /* Output */
-                sendMessageToHandler(KeychainIntentServiceHandler.MESSAGE_OKAY, result);
+                sendMessageToHandler(KeychainIntentServiceHandler.MESSAGE_OKAY, saveResult);
             } catch (Exception e) {
                 sendErrorToHandler(e);
             }

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java

@@ -48,8 +48,7 @@ import org.sufficientlysecure.keychain.provider.KeychainContract;
 import org.sufficientlysecure.keychain.provider.ProviderHelper;
 import org.sufficientlysecure.keychain.service.KeychainIntentService;
 import org.sufficientlysecure.keychain.service.KeychainIntentServiceHandler;
-import org.sufficientlysecure.keychain.service.OperationResults;
+import org.sufficientlysecure.keychain.service.OperationResultParcel;
-import org.sufficientlysecure.keychain.service.OperationResults.EditKeyResult;
 import org.sufficientlysecure.keychain.service.PassphraseCacheService;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
 import org.sufficientlysecure.keychain.ui.adapter.SubkeysAdapter;
@@ -408,9 +407,9 @@ public class EditKeyFragment extends LoaderFragment implements
             public void handleMessage(Message message) {
                 switch (message.what) {
                     case EditSubkeyExpiryDialogFragment.MESSAGE_NEW_EXPIRY_DATE:
-                        Long expiry = (Long) message.getData().
+                        mSaveKeyringParcel.getOrCreateSubkeyChange(keyId).mExpiry =
-                                getSerializable(EditSubkeyExpiryDialogFragment.MESSAGE_DATA_EXPIRY_DATE);
+                                (Long) message.getData().getSerializable(
-                        mSaveKeyringParcel.getOrCreateSubkeyChange(keyId).mExpiry = expiry;
+                                        EditSubkeyExpiryDialogFragment.MESSAGE_DATA_EXPIRY_DATE);
                         break;
                 }
                 getLoaderManager().getLoader(LOADER_ID_SUBKEYS).forceLoad();
@@ -520,8 +519,8 @@ public class EditKeyFragment extends LoaderFragment implements
                     if (returnData == null) {
                         return;
                     }
-                    final OperationResults.EditKeyResult result =
+                    final OperationResultParcel result =
-                            returnData.getParcelable(EditKeyResult.EXTRA_RESULT);
+                            returnData.getParcelable(OperationResultParcel.EXTRA_RESULT);
                     if (result == null) {
                         return;
                     }
@@ -534,7 +533,7 @@ public class EditKeyFragment extends LoaderFragment implements
 
                     // if good -> finish, return result to showkey and display there!
                     Intent intent = new Intent();
-                    intent.putExtra(EditKeyResult.EXTRA_RESULT, result);
+                    intent.putExtra(OperationResultParcel.EXTRA_RESULT, result);
                     getActivity().setResult(EditKeyActivity.RESULT_OK, intent);
                     getActivity().finish();