move key stripping into ChangeSubkey, support divert-to-card

OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java

@@ -701,7 +701,7 @@ public class PgpKeyOperationTest {
     public void testSubkeyStrip() throws Exception {
 
         long keyId = KeyringTestingHelper.getSubkeyId(ring, 1);
-        parcel.mStripSubKeys.add(keyId);
+        parcel.mChangeSubKeys.add(new SubkeyChange(keyId, true, false));
         applyModificationWithChecks(parcel, ring, onlyA, onlyB);
 
         Assert.assertEquals("one extra packet in original", 1, onlyA.size());
@@ -727,7 +727,7 @@ public class PgpKeyOperationTest {
     public void testMasterStrip() throws Exception {
 
         long keyId = ring.getMasterKeyId();
-        parcel.mStripSubKeys.add(keyId);
+        parcel.mChangeSubKeys.add(new SubkeyChange(keyId, true, false));
         applyModificationWithChecks(parcel, ring, onlyA, onlyB);
 
         Assert.assertEquals("one extra packet in original", 1, onlyA.size());

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java

@@ -20,6 +20,7 @@ package org.sufficientlysecure.keychain.pgp;
 
 import org.spongycastle.bcpg.CompressionAlgorithmTags;
 import org.spongycastle.bcpg.HashAlgorithmTags;
+import org.spongycastle.bcpg.S2K;
 import org.spongycastle.bcpg.SymmetricKeyAlgorithmTags;
 import org.spongycastle.bcpg.sig.Features;
 import org.spongycastle.bcpg.sig.KeyFlags;
@@ -715,6 +716,24 @@ public class PgpKeyOperation {
                     return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null);
                 }
 
+                if (change.mDummyStrip || change.mDummyDivert) {
+                    // IT'S DANGEROUS~
+                    // no really, it is. this operation irrevocably removes the private key data from the key
+                    if (change.mDummyStrip) {
+                        sKey = PGPSecretKey.constructGnuDummyKey(sKey.getPublicKey(),
+                                S2K.GNU_PROTECTION_MODE_NO_PRIVATE_KEY);
+                    } else {
+                        sKey = PGPSecretKey.constructGnuDummyKey(sKey.getPublicKey(),
+                                S2K.GNU_PROTECTION_MODE_DIVERT_TO_CARD);
+                    }
+                    sKR = PGPSecretKeyRing.insertSecretKey(sKR, sKey);
+                }
+
+                // This doesn't concern us any further
+                if (change.mExpiry == null && change.mFlags == null) {
+                    continue;
+                }
+
                 // expiry must not be in the past
                 if (change.mExpiry != null && change.mExpiry != 0 &&
                         new Date(change.mExpiry*1000).before(new Date())) {
@@ -805,30 +824,6 @@ public class PgpKeyOperation {
             }
             subProgressPop();
 
-            // 4c. For each subkey to be stripped... do so
-            subProgressPush(65, 70);
-            for (int i = 0; i < saveParcel.mStripSubKeys.size(); i++) {
-
-                progress(R.string.progress_modify_subkeystrip, (i-1) * (100 / saveParcel.mStripSubKeys.size()));
-                long strip = saveParcel.mStripSubKeys.get(i);
-                log.add(LogType.MSG_MF_SUBKEY_STRIP,
-                        indent, KeyFormattingUtils.convertKeyIdToHex(strip));
-
-                PGPSecretKey sKey = sKR.getSecretKey(strip);
-                if (sKey == null) {
-                    log.add(LogType.MSG_MF_ERROR_SUBKEY_MISSING,
-                            indent+1, KeyFormattingUtils.convertKeyIdToHex(strip));
-                    return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null);
-                }
-
-                // IT'S DANGEROUS~
-                // no really, it is. this operation irrevocably removes the private key data from the key
-                sKey = PGPSecretKey.constructGnuDummyKey(sKey.getPublicKey());
-                sKR = PGPSecretKeyRing.insertSecretKey(sKR, sKey);
-
-            }
-            subProgressPop();
-
             // 5. Generate and add new subkeys
             subProgressPush(70, 90);
             for (int i = 0; i < saveParcel.mAddSubKeys.size(); i++) {

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java

@@ -20,6 +20,7 @@ package org.sufficientlysecure.keychain.pgp;
 
 import org.spongycastle.bcpg.ArmoredOutputStream;
 import org.spongycastle.bcpg.PublicKeyAlgorithmTags;
+import org.spongycastle.bcpg.S2K;
 import org.spongycastle.bcpg.SignatureSubpacketTags;
 import org.spongycastle.bcpg.UserAttributeSubpacketTags;
 import org.spongycastle.bcpg.sig.KeyFlags;
@@ -1221,7 +1222,8 @@ public class UncachedKeyRing {
             // if this is a secret key which does not yet occur in the secret ring
             if (sKey == null) {
                 // generate a stripped secret (sub)key
-                sKey = PGPSecretKey.constructGnuDummyKey(key);
+                sKey = PGPSecretKey.constructGnuDummyKey(key,
+                        S2K.GNU_PROTECTION_MODE_NO_PRIVATE_KEY);
             }
             sKey = PGPSecretKey.replacePublicKey(sKey, key);
             return PGPSecretKeyRing.insertSecretKey(secRing, sKey);

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

@@ -58,7 +58,6 @@ public class SaveKeyringParcel implements Parcelable {
 
     public ArrayList<String> mRevokeUserIds;
     public ArrayList<Long> mRevokeSubKeys;
-    public ArrayList<Long> mStripSubKeys;
 
     public SaveKeyringParcel() {
         reset();
@@ -79,7 +78,6 @@ public class SaveKeyringParcel implements Parcelable {
         mChangeSubKeys = new ArrayList<SubkeyChange>();
         mRevokeUserIds = new ArrayList<String>();
         mRevokeSubKeys = new ArrayList<Long>();
-        mStripSubKeys = new ArrayList<Long>();
     }
 
     // performance gain for using Parcelable here would probably be negligible,
@@ -112,10 +110,14 @@ public class SaveKeyringParcel implements Parcelable {
     }
 
     public static class SubkeyChange implements Serializable {
-        public long mKeyId;
+        public final long mKeyId;
         public Integer mFlags;
         // this is a long unix timestamp, in seconds (NOT MILLISECONDS!)
         public Long mExpiry;
+        // if this flag is true, the subkey should be changed to a stripped key
+        public boolean mDummyStrip;
+        // if this flag is true, the subkey should be changed to a divert-to-card key
+        public boolean mDummyDivert;
 
         public SubkeyChange(long keyId) {
             mKeyId = keyId;
@@ -127,11 +129,25 @@ public class SaveKeyringParcel implements Parcelable {
             mExpiry = expiry;
         }
 
+        public SubkeyChange(long keyId, boolean dummyStrip, boolean dummyDivert) {
+            this(keyId, null, null);
+
+            // these flags are mutually exclusive!
+            if (dummyStrip && dummyDivert) {
+                throw new AssertionError(
+                        "cannot set strip and divert flags at the same time - this is a bug!");
+            }
+            mDummyStrip = dummyStrip;
+            mDummyDivert = dummyDivert;
+        }
+
         @Override
         public String toString() {
             String out = "mKeyId: " + mKeyId + ", ";
             out += "mFlags: " + mFlags + ", ";
-            out += "mExpiry: " + mExpiry;
+            out += "mExpiry: " + mExpiry + ", ";
+            out += "mDummyStrip: " + mDummyStrip + ", ";
+            out += "mDummyDivert: " + mDummyDivert;
 
             return out;
         }
@@ -173,7 +189,6 @@ public class SaveKeyringParcel implements Parcelable {
 
         mRevokeUserIds = source.createStringArrayList();
         mRevokeSubKeys = (ArrayList<Long>) source.readSerializable();
-        mStripSubKeys = (ArrayList<Long>) source.readSerializable();
     }
 
     @Override
@@ -196,7 +211,6 @@ public class SaveKeyringParcel implements Parcelable {
 
         destination.writeStringList(mRevokeUserIds);
         destination.writeSerializable(mRevokeSubKeys);
-        destination.writeSerializable(mStripSubKeys);
     }
 
     public static final Creator<SaveKeyringParcel> CREATOR = new Creator<SaveKeyringParcel>() {
@@ -224,8 +238,7 @@ public class SaveKeyringParcel implements Parcelable {
         out += "mChangeSubKeys: " + mChangeSubKeys + "\n";
         out += "mChangePrimaryUserId: " + mChangePrimaryUserId + "\n";
         out += "mRevokeUserIds: " + mRevokeUserIds + "\n";
-        out += "mRevokeSubKeys: " + mRevokeSubKeys + "\n";
+        out += "mRevokeSubKeys: " + mRevokeSubKeys;
-        out += "mStripSubKeys: " + mStripSubKeys;
 
         return out;
     }

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

@@ -55,6 +55,7 @@ import org.sufficientlysecure.keychain.service.KeychainIntentServiceHandler;
 import org.sufficientlysecure.keychain.service.PassphraseCacheService;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel.ChangeUnlockParcel;
+import org.sufficientlysecure.keychain.service.SaveKeyringParcel.SubkeyChange;
 import org.sufficientlysecure.keychain.ui.adapter.SubkeysAdapter;
 import org.sufficientlysecure.keychain.ui.adapter.SubkeysAddedAdapter;
 import org.sufficientlysecure.keychain.ui.adapter.UserIdsAdapter;
@@ -478,12 +479,13 @@ public class EditKeyFragment extends LoaderFragment implements
                         }
                         break;
                     case EditSubkeyDialogFragment.MESSAGE_STRIP:
-                        // toggle
+                        SubkeyChange change = mSaveKeyringParcel.getSubkeyChange(keyId);
-                        if (mSaveKeyringParcel.mStripSubKeys.contains(keyId)) {
+                        if (change == null) {
-                            mSaveKeyringParcel.mStripSubKeys.remove(keyId);
+                            mSaveKeyringParcel.mChangeSubKeys.add(new SubkeyChange(keyId, true, false));
-                        } else {
+                            break;
-                            mSaveKeyringParcel.mStripSubKeys.add(keyId);
                         }
+                        // toggle
+                        change.mDummyStrip = !change.mDummyStrip;
                         break;
                 }
                 getLoaderManager().getLoader(LOADER_ID_SUBKEYS).forceLoad();

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/adapter/SubkeysAdapter.java

@@ -35,6 +35,7 @@ import android.widget.ImageView;
 import android.widget.TextView;
 
 import org.sufficientlysecure.keychain.R;
+import org.sufficientlysecure.keychain.service.SaveKeyringParcel.SubkeyChange;
 import org.sufficientlysecure.keychain.ui.util.FormattingUtils;
 import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKey.SecretKeyType;
 import org.sufficientlysecure.keychain.ui.util.KeyFormattingUtils;
@@ -160,7 +161,11 @@ public class SubkeysAdapter extends CursorAdapter {
                 cursor.getString(INDEX_KEY_CURVE_OID)
         ));
 
-        if (mSaveKeyringParcel != null && mSaveKeyringParcel.mStripSubKeys.contains(keyId)) {
+        SubkeyChange change = mSaveKeyringParcel != null
+                ? mSaveKeyringParcel.getSubkeyChange(keyId)
+                : null;
+
+        if (change.mDummyStrip) {
             algorithmStr.append(", ");
             final SpannableString boldStripped = new SpannableString(
                     context.getString(R.string.key_stripped)

extern/spongycastle

@@ -1 +1 @@
-Subproject commit f13d9c202f7470ede75f2c02cc8c578acd3acee9
+Subproject commit c2a91166cef1a08c97ad2e988e368fe36babfc05