work on divert-to-key and other keyring stuff

- allow modifySecretKeyRing operation without passphrase, but a only
  restricted subset of operations (ie, s2k strip/divert)
- pass byte array with serial number to key edit operation to initialize
  divert-to-card key
- update spongycastle to support serial numbers in iv for divert-to-card

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

@@ -702,7 +702,7 @@ public class PgpKeyOperationTest {
     public void testSubkeyStrip() throws Exception {
 
         long keyId = KeyringTestingHelper.getSubkeyId(ring, 1);
-        parcel.mChangeSubKeys.add(new SubkeyChange(keyId, true, false));
+        parcel.mChangeSubKeys.add(new SubkeyChange(keyId, true, null));
         applyModificationWithChecks(parcel, ring, onlyA, onlyB);
 
         Assert.assertEquals("one extra packet in original", 1, onlyA.size());
@@ -728,7 +728,7 @@ public class PgpKeyOperationTest {
     public void testMasterStrip() throws Exception {
 
         long keyId = ring.getMasterKeyId();
-        parcel.mChangeSubKeys.add(new SubkeyChange(keyId, true, false));
+        parcel.mChangeSubKeys.add(new SubkeyChange(keyId, true, null));
         applyModificationWithChecks(parcel, ring, onlyA, onlyB);
 
         Assert.assertEquals("one extra packet in original", 1, onlyA.size());
@@ -747,6 +747,44 @@ public class PgpKeyOperationTest {
         Assert.assertEquals("new packet secret key data should have length zero",
                 0, ((SecretKeyPacket) p).getSecretKeyData().length);
         Assert.assertNull("new packet should have no iv data", ((SecretKeyPacket) p).getIV());
+    }
+
+    @Test
+    public void testRestrictedStrip() throws Exception {
+
+        long keyId = KeyringTestingHelper.getSubkeyId(ring, 1);
+        UncachedKeyRing modified;
+
+        { // we should be able to change the stripped/divert status of subkeys without passphrase
+            parcel.reset();
+            parcel.mChangeSubKeys.add(new SubkeyChange(keyId, true, null));
+            modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB, null);
+            Assert.assertEquals("one extra packet in modified", 1, onlyB.size());
+            Packet p = new BCPGInputStream(new ByteArrayInputStream(onlyB.get(0).buf)).readPacket();
+            Assert.assertEquals("new packet should have GNU_DUMMY S2K type",
+                    S2K.GNU_DUMMY_S2K, ((SecretKeyPacket) p).getS2K().getType());
+            Assert.assertEquals("new packet should have GNU_DUMMY protection mode stripped",
+                    S2K.GNU_PROTECTION_MODE_NO_PRIVATE_KEY, ((SecretKeyPacket) p).getS2K().getProtectionMode());
+        }
+
+        { // and again, changing to divert-to-card
+            parcel.reset();
+            byte[] serial = new byte[] {
+                    0x6a, 0x6f, 0x6c, 0x6f, 0x73, 0x77, 0x61, 0x67,
+                    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+            };
+            parcel.mChangeSubKeys.add(new SubkeyChange(keyId, false, serial));
+            modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB, null);
+            Assert.assertEquals("one extra packet in modified", 1, onlyB.size());
+            Packet p = new BCPGInputStream(new ByteArrayInputStream(onlyB.get(0).buf)).readPacket();
+            Assert.assertEquals("new packet should have GNU_DUMMY S2K type",
+                    S2K.GNU_DUMMY_S2K, ((SecretKeyPacket) p).getS2K().getType());
+            Assert.assertEquals("new packet should have GNU_DUMMY protection mode divert-to-card",
+                    S2K.GNU_PROTECTION_MODE_DIVERT_TO_CARD, ((SecretKeyPacket) p).getS2K().getProtectionMode());
+            Assert.assertArrayEquals("new packet should have correct serial number as iv",
+                    serial, ((SecretKeyPacket) p).getIV());
+
+        }
 
     }
 
@@ -1093,6 +1131,17 @@ public class PgpKeyOperationTest {
 
     }
 
+    @Test
+    public void testRestricted () throws Exception {
+
+        CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
+
+        parcel.mAddUserIds.add("discord");
+        PgpKeyOperation op = new PgpKeyOperation(null);
+        PgpEditKeyResult result = op.modifySecretKeyRing(secretRing, parcel, null);
+        Assert.assertFalse("non-restricted operations should fail without passphrase", result.success());
+    }
+
     private static UncachedKeyRing applyModificationWithChecks(SaveKeyringParcel parcel,
                                                                UncachedKeyRing ring,
                                                                ArrayList<RawPacket> onlyA,

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java

@@ -473,6 +473,7 @@ public abstract class OperationResult implements Parcelable {
 
         // secret key modify
         MSG_MF (LogLevel.START, R.string.msg_mr),
+        MSG_MF_ERROR_DIVERT_SERIAL (LogLevel.ERROR, R.string.msg_mf_error_divert_serial),
         MSG_MF_ERROR_ENCODE (LogLevel.ERROR, R.string.msg_mf_error_encode),
         MSG_MF_ERROR_FINGERPRINT (LogLevel.ERROR, R.string.msg_mf_error_fingerprint),
         MSG_MF_ERROR_KEYID (LogLevel.ERROR, R.string.msg_mf_error_keyid),
@@ -485,6 +486,7 @@ public abstract class OperationResult implements Parcelable {
         MSG_MF_ERROR_PASSPHRASE_MASTER(LogLevel.ERROR, R.string.msg_mf_error_passphrase_master),
         MSG_MF_ERROR_PAST_EXPIRY(LogLevel.ERROR, R.string.msg_mf_error_past_expiry),
         MSG_MF_ERROR_PGP (LogLevel.ERROR, R.string.msg_mf_error_pgp),
+        MSG_MF_ERROR_RESTRICTED(LogLevel.ERROR, R.string.msg_mf_error_restricted),
         MSG_MF_ERROR_REVOKED_PRIMARY (LogLevel.ERROR, R.string.msg_mf_error_revoked_primary),
         MSG_MF_ERROR_SIG (LogLevel.ERROR, R.string.msg_mf_error_sig),
         MSG_MF_ERROR_SUBKEY_MISSING(LogLevel.ERROR, R.string.msg_mf_error_subkey_missing),

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

@@ -20,7 +20,6 @@ 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;
@@ -390,6 +389,9 @@ public class PgpKeyOperation {
      * with a passphrase fails, the operation will fail with an unlocking error. More specific
      * handling of errors should be done in UI code!
      *
+     * If the passphrase is null, only a restricted subset of operations will be available,
+     * namely stripping of subkeys and changing the protection mode of dummy keys.
+     *
      */
     public PgpEditKeyResult modifySecretKeyRing(CanonicalizedSecretKeyRing wsKR, SaveKeyringParcel saveParcel,
                                                String passphrase) {
@@ -430,6 +432,11 @@ public class PgpKeyOperation {
             return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null);
         }
 
+        // If we have no passphrase, only allow restricted operation
+        if (passphrase == null) {
+            return internalRestricted(sKR, saveParcel, log);
+        }
+
         // read masterKeyFlags, and use the same as before.
         // since this is the master key, this contains at least CERTIFY_OTHER
         PGPPublicKey masterPublicKey = masterSecretKey.getPublicKey();
@@ -716,15 +723,18 @@ public class PgpKeyOperation {
                     return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null);
                 }
 
-                if (change.mDummyStrip || change.mDummyDivert) {
+                if (change.mDummyStrip || change.mDummyDivert != null) {
                     // 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(),
+                        sKey = PGPSecretKey.constructGnuDummyKey(sKey.getPublicKey());
-                                S2K.GNU_PROTECTION_MODE_NO_PRIVATE_KEY);
                     } else {
-                        sKey = PGPSecretKey.constructGnuDummyKey(sKey.getPublicKey(),
+                        // the serial number must be 16 bytes in length
-                                S2K.GNU_PROTECTION_MODE_DIVERT_TO_CARD);
+                        if (change.mDummyDivert.length != 16) {
+                            log.add(LogType.MSG_MF_ERROR_DIVERT_SERIAL,
+                                    indent + 1, KeyFormattingUtils.convertKeyIdToHex(change.mKeyId));
+                            return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null);
+                        }
                     }
                     sKR = PGPSecretKeyRing.insertSecretKey(sKR, sKey);
                 }
@@ -932,6 +942,73 @@ public class PgpKeyOperation {
 
     }
 
+    /** This method does the actual modifications in a keyring just like internal, except it
+     * supports only the subset of operations which require no passphrase, and will error
+     * otherwise.
+     */
+    private PgpEditKeyResult internalRestricted(PGPSecretKeyRing sKR, SaveKeyringParcel saveParcel,
+                                                OperationLog log) {
+
+        int indent = 1;
+
+        progress(R.string.progress_modify, 0);
+
+        // Make sure the saveParcel includes only operations available without passphrae!
+        if (!saveParcel.isRestrictedOnly()) {
+            log.add(LogType.MSG_MF_ERROR_RESTRICTED, indent);
+            return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null);
+        }
+
+        // Check if we were cancelled
+        if (checkCancelled()) {
+            log.add(LogType.MSG_OPERATION_CANCELLED, indent);
+            return new PgpEditKeyResult(PgpEditKeyResult.RESULT_CANCELLED, log, null);
+        }
+
+        // The only operation we can do here:
+        // 4a. Strip secret keys, or change their protection mode (stripped/divert-to-card)
+        subProgressPush(50, 60);
+        for (int i = 0; i < saveParcel.mChangeSubKeys.size(); i++) {
+
+            progress(R.string.progress_modify_subkeychange, (i - 1) * (100 / saveParcel.mChangeSubKeys.size()));
+            SaveKeyringParcel.SubkeyChange change = saveParcel.mChangeSubKeys.get(i);
+            log.add(LogType.MSG_MF_SUBKEY_CHANGE,
+                    indent, KeyFormattingUtils.convertKeyIdToHex(change.mKeyId));
+
+            PGPSecretKey sKey = sKR.getSecretKey(change.mKeyId);
+            if (sKey == null) {
+                log.add(LogType.MSG_MF_ERROR_SUBKEY_MISSING,
+                        indent + 1, KeyFormattingUtils.convertKeyIdToHex(change.mKeyId));
+                return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null);
+            }
+
+            if (change.mDummyStrip || change.mDummyDivert != null) {
+                // 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());
+                } else {
+                    // the serial number must be 16 bytes in length
+                    if (change.mDummyDivert.length != 16) {
+                        log.add(LogType.MSG_MF_ERROR_DIVERT_SERIAL,
+                                indent + 1, KeyFormattingUtils.convertKeyIdToHex(change.mKeyId));
+                        return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null);
+                    }
+                    sKey = PGPSecretKey.constructGnuDummyKey(sKey.getPublicKey(), change.mDummyDivert);
+                }
+                sKR = PGPSecretKeyRing.insertSecretKey(sKR, sKey);
+            }
+
+        }
+
+        // And we're done!
+        progress(R.string.progress_done, 100);
+        log.add(LogType.MSG_MF_SUCCESS, indent);
+        return new PgpEditKeyResult(OperationResult.RESULT_OK, log, new UncachedKeyRing(sKR));
+
+    }
+
+
     private static PGPSecretKeyRing applyNewUnlock(
             PGPSecretKeyRing sKR,
             PGPPublicKey masterPublicKey,

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

@@ -20,7 +20,6 @@ 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;
@@ -1222,8 +1221,7 @@ 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

@@ -80,6 +80,23 @@ public class SaveKeyringParcel implements Parcelable {
         mRevokeSubKeys = new ArrayList<Long>();
     }
 
+    /** Returns true iff this parcel does not contain any operations which require a passphrase. */
+    public boolean isRestrictedOnly() {
+        if (mNewUnlock != null || !mAddUserIds.isEmpty() || !mAddUserAttribute.isEmpty()
+                || !mAddSubKeys.isEmpty() || mChangePrimaryUserId != null || !mRevokeSubKeys .isEmpty()
+                || !mRevokeSubKeys.isEmpty()) {
+            return false;
+        }
+
+        for (SubkeyChange change : mChangeSubKeys) {
+            if (change.mRecertify || change.mFlags != null || change.mExpiry != null) {
+                return false;
+            }
+        }
+
+        return true;
+    }
+
     // performance gain for using Parcelable here would probably be negligible,
     // use Serializable instead.
     public static class SubkeyAdd implements Serializable {
@@ -114,12 +131,14 @@ public class SaveKeyringParcel implements Parcelable {
         public Integer mFlags;
         // this is a long unix timestamp, in seconds (NOT MILLISECONDS!)
         public Long mExpiry;
+        // if this flag is true, the key will be recertified even if all above
+        // values are no-ops
+        public boolean mRecertify;
         // 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
+        // if this is non-null, the subkey will be changed to a divert-to-card
-        public boolean mDummyDivert;
+        // key for the given serial number
-        // if this flag is true, the key will be recertified even if the above values are no-ops
+        public byte[] mDummyDivert;
-        public boolean mRecertify;
 
         public SubkeyChange(long keyId) {
             mKeyId = keyId;
@@ -136,11 +155,11 @@ public class SaveKeyringParcel implements Parcelable {
             mExpiry = expiry;
         }
 
-        public SubkeyChange(long keyId, boolean dummyStrip, boolean dummyDivert) {
+        public SubkeyChange(long keyId, boolean dummyStrip, byte[] dummyDivert) {
             this(keyId, null, null);
 
             // these flags are mutually exclusive!
-            if (dummyStrip && dummyDivert) {
+            if (dummyStrip && dummyDivert != null) {
                 throw new AssertionError(
                         "cannot set strip and divert flags at the same time - this is a bug!");
             }

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

@@ -481,7 +481,7 @@ public class EditKeyFragment extends LoaderFragment implements
                     case EditSubkeyDialogFragment.MESSAGE_STRIP:
                         SubkeyChange change = mSaveKeyringParcel.getSubkeyChange(keyId);
                         if (change == null) {
-                            mSaveKeyringParcel.mChangeSubKeys.add(new SubkeyChange(keyId, true, false));
+                            mSaveKeyringParcel.mChangeSubKeys.add(new SubkeyChange(keyId, true, null));
                             break;
                         }
                         // toggle

OpenKeychain/src/main/res/values/strings.xml

@@ -826,6 +826,7 @@
 
     <!-- modifySecretKeyRing -->
     <string name="msg_mr">"Modifying keyring %s"</string>
+    <string name="msg_mf_error_divert_serial">"The serial number of a divert-to-card key must be 16 bytes! This is a programming error, please file a bug report!"</string>
     <string name="msg_mf_error_encode">"Encoding exception!"</string>
     <string name="msg_mf_error_fingerprint">"Actual key fingerprint does not match the expected one!"</string>
     <string name="msg_mf_error_keyid">"No key ID. This is an internal error, please file a bug report!"</string>
@@ -833,6 +834,7 @@
     <string name="msg_mf_error_master_none">"No master certificate found to operate on! (All revoked?)"</string>
     <string name="msg_mf_error_noexist_primary">"Bad primary user ID specified!"</string>
     <string name="msg_mf_error_noexist_revoke">"Bad user ID for revocation specified!"</string>
+    <string name="msg_mf_error_restricted">"Tried to execute restricted operation without passphrase! This is a programming error, please file a bug report!"</string>
     <string name="msg_mf_error_revoked_primary">"Revoked user IDs cannot be primary!"</string>
     <string name="msg_mf_error_null_expiry">"Expiry time cannot be "same as before" on subkey creation. This is a programming error, please file a bug report!"</string>
     <string name="msg_mf_error_passphrase_master">"Fatal error decrypting master key! This is likely a programming error, please file a bug report!"</string>

extern/spongycastle

@@ -1 +1 @@
-Subproject commit c2a91166cef1a08c97ad2e988e368fe36babfc05
+Subproject commit 26c232f31b62404ecb5d3ae3d2c1730fd0a0a0eb