support for master key modifications, among other stuff

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

@@ -376,6 +376,20 @@ public class PgpKeyOperationTest {
                     ring.getPublicKey(keyId).getKeyUsage(), modified.getPublicKey(keyId).getKeyUsage());
         }
 
+        { // change expiry
+            expiry += 60*60*24;
+
+            parcel.mChangeSubKeys.add(new SubkeyChange(keyId, null, expiry));
+            modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB);
+
+            Assert.assertNotNull("modified key must have an expiry date",
+                    modified.getPublicKey(keyId).getExpiryTime());
+            Assert.assertEquals("modified key must have expected expiry date",
+                    expiry, modified.getPublicKey(keyId).getExpiryTime().getTime()/1000);
+            Assert.assertEquals("modified key must have same flags as before",
+                    ring.getPublicKey(keyId).getKeyUsage(), modified.getPublicKey(keyId).getKeyUsage());
+        }
+
         {
             int flags = KeyFlags.SIGN_DATA | KeyFlags.ENCRYPT_COMMS;
             parcel.reset();
@@ -422,16 +436,114 @@ public class PgpKeyOperationTest {
             parcel.reset();
             parcel.mChangeSubKeys.add(new SubkeyChange(keyId, null, new Date().getTime()/1000-10));
 
-            CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
+            assertModifyFailure("setting subkey expiry to a past date should fail", ring, parcel);
-            assertModifyFailure("setting subkey expiry to a past date should fail", secretRing, parcel);
         }
 
-        { // modifying nonexistent keyring should fail
+        { // modifying nonexistent subkey should fail
             parcel.reset();
             parcel.mChangeSubKeys.add(new SubkeyChange(123, null, null));
 
-            CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
+            assertModifyFailure("modifying non-existent subkey should fail", ring, parcel);
-            assertModifyFailure("modifying non-existent subkey should fail", secretRing, parcel);
+        }
+
+    }
+
+    @Test
+    public void testMasterModify() throws Exception {
+
+        long expiry = new Date().getTime()/1000 + 1024;
+        long keyId = ring.getMasterKeyId();
+
+        UncachedKeyRing modified = ring;
+
+        // to make this check less trivial, we add a user id, change the primary one and revoke one
+        parcel.mAddUserIds.add("aloe");
+        parcel.mChangePrimaryUserId = "aloe";
+        parcel.mRevokeUserIds.add("pink");
+        modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB);
+
+        {
+            parcel.mChangeSubKeys.add(new SubkeyChange(keyId, null, expiry));
+            modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB);
+
+            // this implies that only the two non-revoked signatures were changed!
+            Assert.assertEquals("two extra packets in original", 2, onlyA.size());
+            Assert.assertEquals("two extra packets in modified", 2, onlyB.size());
+
+            Assert.assertEquals("first original packet must be a signature",
+                    PacketTags.SIGNATURE, onlyA.get(0).tag);
+            Assert.assertEquals("second original packet must be a signature",
+                    PacketTags.SIGNATURE, onlyA.get(1).tag);
+            Assert.assertEquals("first new packet must be signature",
+                    PacketTags.SIGNATURE, onlyB.get(0).tag);
+            Assert.assertEquals("first new packet must be signature",
+                    PacketTags.SIGNATURE, onlyB.get(1).tag);
+
+            Assert.assertNotNull("modified key must have an expiry date",
+                    modified.getPublicKey().getExpiryTime());
+            Assert.assertEquals("modified key must have expected expiry date",
+                    expiry, modified.getPublicKey().getExpiryTime().getTime() / 1000);
+            Assert.assertEquals("modified key must have same flags as before",
+                    ring.getPublicKey().getKeyUsage(), modified.getPublicKey().getKeyUsage());
+        }
+
+        { // change expiry
+            expiry += 60*60*24;
+
+            parcel.mChangeSubKeys.add(new SubkeyChange(keyId, null, expiry));
+            modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB);
+
+            Assert.assertNotNull("modified key must have an expiry date",
+                    modified.getPublicKey(keyId).getExpiryTime());
+            Assert.assertEquals("modified key must have expected expiry date",
+                    expiry, modified.getPublicKey(keyId).getExpiryTime().getTime()/1000);
+            Assert.assertEquals("modified key must have same flags as before",
+                    ring.getPublicKey(keyId).getKeyUsage(), modified.getPublicKey(keyId).getKeyUsage());
+        }
+
+        {
+            int flags = KeyFlags.CERTIFY_OTHER | KeyFlags.SIGN_DATA;
+            parcel.reset();
+            parcel.mChangeSubKeys.add(new SubkeyChange(keyId, flags, null));
+            modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB);
+
+            Assert.assertEquals("modified key must have expected flags",
+                    flags, modified.getPublicKey(keyId).getKeyUsage());
+            Assert.assertNotNull("key must retain its expiry",
+                    modified.getPublicKey(keyId).getExpiryTime());
+            Assert.assertEquals("key expiry must be unchanged",
+                    expiry, modified.getPublicKey(keyId).getExpiryTime().getTime()/1000);
+        }
+
+        { // expiry of 0 should be "no expiry"
+            parcel.reset();
+            parcel.mChangeSubKeys.add(new SubkeyChange(keyId, null, 0L));
+            modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB);
+
+            Assert.assertNull("key must not expire anymore", modified.getPublicKey(keyId).getExpiryTime());
+        }
+
+        { // if we revoke everything, nothing is left to properly sign...
+            parcel.reset();
+            parcel.mRevokeUserIds.add("twi");
+            parcel.mRevokeUserIds.add("pink");
+            parcel.mChangeSubKeys.add(new SubkeyChange(keyId, KeyFlags.CERTIFY_OTHER, null));
+
+            assertModifyFailure("master key modification with all user ids revoked should fail", ring, parcel);
+        }
+
+        { // any flag not including CERTIFY_OTHER should fail
+            parcel.reset();
+            parcel.mChangeSubKeys.add(new SubkeyChange(keyId, KeyFlags.SIGN_DATA, null));
+
+            assertModifyFailure("setting master key flags without certify should fail", ring, parcel);
+        }
+
+        { // a past expiry should fail
+            parcel.reset();
+            parcel.mChangeSubKeys.add(new SubkeyChange(keyId, null, new Date().getTime()/1000-10));
+
+            assertModifyFailure("setting subkey expiry to a past date should fail", ring, parcel);
         }
 
     }

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

@@ -64,11 +64,11 @@ public class UncachedKeyringCanonicalizeTest {
 
         SaveKeyringParcel parcel = new SaveKeyringParcel();
         parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
-                PublicKeyAlgorithmTags.RSA_GENERAL, 1024, KeyFlags.CERTIFY_OTHER, null));
+                PublicKeyAlgorithmTags.RSA_GENERAL, 1024, KeyFlags.CERTIFY_OTHER, 0L));
         parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
-                PublicKeyAlgorithmTags.RSA_GENERAL, 1024, KeyFlags.SIGN_DATA, null));
+                PublicKeyAlgorithmTags.RSA_GENERAL, 1024, KeyFlags.SIGN_DATA, 0L));
         parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
-                PublicKeyAlgorithmTags.RSA_GENERAL, 1024, KeyFlags.ENCRYPT_COMMS, null));
+                PublicKeyAlgorithmTags.RSA_GENERAL, 1024, KeyFlags.ENCRYPT_COMMS, 0L));
 
         parcel.mAddUserIds.add("twi");
         parcel.mAddUserIds.add("pink");
@@ -277,7 +277,7 @@ public class UncachedKeyringCanonicalizeTest {
 
         SaveKeyringParcel parcel = new SaveKeyringParcel();
         parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
-                PublicKeyAlgorithmTags.RSA_GENERAL, 1024, KeyFlags.CERTIFY_OTHER, null));
+                PublicKeyAlgorithmTags.RSA_GENERAL, 1024, KeyFlags.CERTIFY_OTHER, 0L));
         parcel.mAddUserIds.add("trix");
         PgpKeyOperation op = new PgpKeyOperation(null);
 

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

@@ -64,9 +64,9 @@ public class UncachedKeyringMergeTest {
         {
             SaveKeyringParcel parcel = new SaveKeyringParcel();
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
-                    PublicKeyAlgorithmTags.RSA_GENERAL, 1024, KeyFlags.CERTIFY_OTHER, null));
+                    PublicKeyAlgorithmTags.RSA_GENERAL, 1024, KeyFlags.CERTIFY_OTHER, 0L));
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
-                    PublicKeyAlgorithmTags.RSA_GENERAL, 1024, KeyFlags.SIGN_DATA, null));
+                    PublicKeyAlgorithmTags.RSA_GENERAL, 1024, KeyFlags.SIGN_DATA, 0L));
 
             parcel.mAddUserIds.add("twi");
             parcel.mAddUserIds.add("pink");
@@ -83,7 +83,7 @@ public class UncachedKeyringMergeTest {
         {
             SaveKeyringParcel parcel = new SaveKeyringParcel();
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
-                    PublicKeyAlgorithmTags.RSA_GENERAL, 1024, KeyFlags.CERTIFY_OTHER, null));
+                    PublicKeyAlgorithmTags.RSA_GENERAL, 1024, KeyFlags.CERTIFY_OTHER, 0L));
 
             parcel.mAddUserIds.add("shy");
             // passphrase is tested in PgpKeyOperationTest, just use empty here
@@ -189,7 +189,7 @@ public class UncachedKeyringMergeTest {
 
             parcel.reset();
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
-                    PublicKeyAlgorithmTags.RSA_GENERAL, 1024, KeyFlags.SIGN_DATA, null));
+                    PublicKeyAlgorithmTags.RSA_GENERAL, 1024, KeyFlags.SIGN_DATA, 0L));
             modifiedA = op.modifySecretKeyRing(secretRing, parcel, "").getRing();
             modifiedB = op.modifySecretKeyRing(secretRing, parcel, "").getRing();
 

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

@@ -37,11 +37,11 @@ public class UncachedKeyringTest {
 
         SaveKeyringParcel parcel = new SaveKeyringParcel();
         parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
-                PublicKeyAlgorithmTags.RSA_GENERAL, 1024, KeyFlags.CERTIFY_OTHER, null));
+                PublicKeyAlgorithmTags.RSA_GENERAL, 1024, KeyFlags.CERTIFY_OTHER, 0L));
         parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
-                PublicKeyAlgorithmTags.RSA_GENERAL, 1024, KeyFlags.SIGN_DATA, null));
+                PublicKeyAlgorithmTags.RSA_GENERAL, 1024, KeyFlags.SIGN_DATA, 0L));
         parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
-                PublicKeyAlgorithmTags.RSA_GENERAL, 1024, KeyFlags.ENCRYPT_COMMS, null));
+                PublicKeyAlgorithmTags.RSA_GENERAL, 1024, KeyFlags.ENCRYPT_COMMS, 0L));
 
         parcel.mAddUserIds.add("twi");
         parcel.mAddUserIds.add("pink");

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

@@ -253,7 +253,7 @@ public class PgpKeyOperation {
                     masterSecretKey.getEncoded(), new JcaKeyFingerprintCalculator());
 
             subProgressPush(50, 100);
-            return internal(sKR, masterSecretKey, add.mFlags, saveParcel, "", log);
+            return internal(sKR, masterSecretKey, add.mFlags, add.mExpiry, saveParcel, "", log);
 
         } catch (PGPException e) {
             log.add(LogLevel.ERROR, LogType.MSG_CR_ERROR_INTERNAL_PGP, indent);
@@ -319,14 +319,17 @@ public class PgpKeyOperation {
 
         // read masterKeyFlags, and use the same as before.
         // since this is the master key, this contains at least CERTIFY_OTHER
-        int masterKeyFlags = readKeyFlags(masterSecretKey.getPublicKey()) | KeyFlags.CERTIFY_OTHER;
+        PGPPublicKey masterPublicKey = masterSecretKey.getPublicKey();
+        int masterKeyFlags = readKeyFlags(masterPublicKey) | KeyFlags.CERTIFY_OTHER;
+        long masterKeyExpiry = masterPublicKey.getValidSeconds() == 0L ? 0L :
+                masterPublicKey.getCreationTime().getTime() / 1000 + masterPublicKey.getValidSeconds();
 
-        return internal(sKR, masterSecretKey, masterKeyFlags, saveParcel, passphrase, log);
+        return internal(sKR, masterSecretKey, masterKeyFlags, masterKeyExpiry, saveParcel, passphrase, log);
 
     }
 
     private EditKeyResult internal(PGPSecretKeyRing sKR, PGPSecretKey masterSecretKey,
-                                     int masterKeyFlags,
+                                     int masterKeyFlags, long masterKeyExpiry,
                                      SaveKeyringParcel saveParcel, String passphrase,
                                      OperationLog log) {
 
@@ -351,21 +354,22 @@ public class PgpKeyOperation {
             }
         }
 
-        // work on master secret key
         try {
 
+            { // work on master secret key
+
                 PGPPublicKey modifiedPublicKey = masterPublicKey;
 
                 // 2a. Add certificates for new user ids
                 subProgressPush(15, 25);
                 for (int i = 0; i < saveParcel.mAddUserIds.size(); i++) {
 
-                progress(R.string.progress_modify_adduid, (i-1) * (100 / saveParcel.mAddUserIds.size()));
+                    progress(R.string.progress_modify_adduid, (i - 1) * (100 / saveParcel.mAddUserIds.size()));
                     String userId = saveParcel.mAddUserIds.get(i);
                     log.add(LogLevel.INFO, LogType.MSG_MF_UID_ADD, indent, userId);
 
                     if (userId.equals("")) {
-                    log.add(LogLevel.ERROR, LogType.MSG_MF_UID_ERROR_EMPTY, indent+1);
+                        log.add(LogLevel.ERROR, LogType.MSG_MF_UID_ERROR_EMPTY, indent + 1);
                         return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
                     }
 
@@ -396,7 +400,7 @@ public class PgpKeyOperation {
                             && userId.equals(saveParcel.mChangePrimaryUserId);
                     // generate and add new certificate
                     PGPSignature cert = generateUserIdSignature(masterPrivateKey,
-                        masterPublicKey, userId, isPrimary, masterKeyFlags);
+                            masterPublicKey, userId, isPrimary, masterKeyFlags, masterKeyExpiry);
                     modifiedPublicKey = PGPPublicKey.addCertification(modifiedPublicKey, userId, cert);
                 }
                 subProgressPop();
@@ -405,11 +409,13 @@ public class PgpKeyOperation {
                 subProgressPush(25, 40);
                 for (int i = 0; i < saveParcel.mRevokeUserIds.size(); i++) {
 
-                progress(R.string.progress_modify_revokeuid, (i-1) * (100 / saveParcel.mRevokeUserIds.size()));
+                    progress(R.string.progress_modify_revokeuid, (i - 1) * (100 / saveParcel.mRevokeUserIds.size()));
                     String userId = saveParcel.mRevokeUserIds.get(i);
                     log.add(LogLevel.INFO, LogType.MSG_MF_UID_REVOKE, indent, userId);
+
                     // Make sure the user id exists (yes these are 10 LoC in Java!)
                     boolean exists = false;
+                    //noinspection unchecked
                     for (String uid : new IterableIterator<String>(modifiedPublicKey.getUserIDs())) {
                         if (userId.equals(uid)) {
                             exists = true;
@@ -496,7 +502,8 @@ public class PgpKeyOperation {
                             modifiedPublicKey = PGPPublicKey.removeCertification(
                                     modifiedPublicKey, userId, currentCert);
                             PGPSignature newCert = generateUserIdSignature(
-                                masterPrivateKey, masterPublicKey, userId, false, masterKeyFlags);
+                                    masterPrivateKey, masterPublicKey, userId, false,
+                                    masterKeyFlags, masterKeyExpiry);
                             modifiedPublicKey = PGPPublicKey.addCertification(
                                     modifiedPublicKey, userId, newCert);
                             continue;
@@ -511,7 +518,8 @@ public class PgpKeyOperation {
                             modifiedPublicKey = PGPPublicKey.removeCertification(
                                     modifiedPublicKey, userId, currentCert);
                             PGPSignature newCert = generateUserIdSignature(
-                                masterPrivateKey, masterPublicKey, userId, true, masterKeyFlags);
+                                    masterPrivateKey, masterPublicKey, userId, true,
+                                    masterKeyFlags, masterKeyExpiry);
                             modifiedPublicKey = PGPPublicKey.addCertification(
                                     modifiedPublicKey, userId, newCert);
                             ok = true;
@@ -536,6 +544,8 @@ public class PgpKeyOperation {
                     sKR = PGPSecretKeyRing.insertSecretKey(sKR, masterSecretKey);
                 }
 
+            }
+
             // 4a. For each subkey change, generate new subkey binding certificate
             subProgressPush(50, 60);
             for (int i = 0; i < saveParcel.mChangeSubKeys.size(); i++) {
@@ -545,28 +555,47 @@ public class PgpKeyOperation {
                 log.add(LogLevel.INFO, LogType.MSG_MF_SUBKEY_CHANGE,
                         indent, PgpKeyHelper.convertKeyIdToHex(change.mKeyId));
 
-                // TODO allow changes in master key? this implies generating new user id certs...
-                if (change.mKeyId == masterPublicKey.getKeyID()) {
-                    Log.e(Constants.TAG, "changing the master key not supported");
-                    return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
-                }
-
                 PGPSecretKey sKey = sKR.getSecretKey(change.mKeyId);
                 if (sKey == null) {
                     log.add(LogLevel.ERROR, LogType.MSG_MF_SUBKEY_MISSING,
                             indent + 1, PgpKeyHelper.convertKeyIdToHex(change.mKeyId));
                     return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
                 }
-                PGPPublicKey pKey = sKey.getPublicKey();
 
                 // expiry must not be in the past
                 if (change.mExpiry != null && change.mExpiry != 0 &&
                         new Date(change.mExpiry*1000).before(new Date())) {
-                    log.add(LogLevel.ERROR, LogType.MSG_MF_SUBKEY_PAST_EXPIRY,
+                    log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_PAST_EXPIRY,
                             indent + 1, PgpKeyHelper.convertKeyIdToHex(change.mKeyId));
                     return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
                 }
 
+                // if this is the master key, update uid certificates instead
+                if (change.mKeyId == masterPublicKey.getKeyID()) {
+                    int flags = change.mFlags == null ? masterKeyFlags : change.mFlags;
+                    long expiry = change.mExpiry == null ? masterKeyExpiry : change.mExpiry;
+
+                    if ((flags & KeyFlags.CERTIFY_OTHER) != KeyFlags.CERTIFY_OTHER) {
+                        log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_NO_CERTIFY, indent + 1);
+                        return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
+                    }
+
+                    PGPPublicKey pKey =
+                            updateMasterCertificates(masterPrivateKey, masterPublicKey,
+                                    flags, expiry, indent, log);
+                    if (pKey == null) {
+                        // error log entry has already been added by updateMasterCertificates itself
+                        return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
+                    }
+                    masterSecretKey = PGPSecretKey.replacePublicKey(masterSecretKey, pKey);
+                    masterPublicKey = pKey;
+                    sKR = PGPSecretKeyRing.insertSecretKey(sKR, masterSecretKey);
+                    continue;
+                }
+
+                // otherwise, continue working on the public key
+                PGPPublicKey pKey = sKey.getPublicKey();
+
                 // keep old flags, or replace with new ones
                 int flags = change.mFlags == null ? readKeyFlags(pKey) : change.mFlags;
                 long expiry;
@@ -583,7 +612,7 @@ public class PgpKeyOperation {
                 //noinspection unchecked
                 for (PGPSignature sig : new IterableIterator<PGPSignature>(pKey.getSignatures())) {
                     // special case: if there is a revocation, don't use expiry from before
-                    if (change.mExpiry == null
+                    if ( (change.mExpiry == null || change.mExpiry == 0L)
                             && sig.getSignatureType() == PGPSignature.SUBKEY_REVOCATION) {
                         expiry = 0;
                     }
@@ -631,8 +660,13 @@ public class PgpKeyOperation {
                 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())) {
+                if (add.mExpiry == null) {
-                    log.add(LogLevel.ERROR, LogType.MSG_MF_SUBKEY_PAST_EXPIRY, indent +1);
+                    log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_NULL_EXPIRY, indent +1);
+                    return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
+                }
+
+                if (add.mExpiry > 0L && new Date(add.mExpiry*1000).before(new Date())) {
+                    log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_PAST_EXPIRY, indent +1);
                     return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
                 }
 
@@ -652,7 +686,7 @@ public class PgpKeyOperation {
                 PGPPublicKey pKey = keyPair.getPublicKey();
                 PGPSignature cert = generateSubkeyBindingSignature(
                         masterPublicKey, masterPrivateKey, keyPair.getPrivateKey(), pKey,
-                        add.mFlags, add.mExpiry == null ? 0 : add.mExpiry);
+                        add.mFlags, add.mExpiry);
                 pKey = PGPPublicKey.addSubkeyBindingCertification(pKey, cert);
 
                 PGPSecretKey sKey; {
@@ -713,21 +747,104 @@ public class PgpKeyOperation {
 
     }
 
+    /** Update all (non-revoked) uid signatures with new flags and expiry time. */
+    private static PGPPublicKey updateMasterCertificates(
+            PGPPrivateKey masterPrivateKey, PGPPublicKey masterPublicKey,
+            int flags, long expiry, int indent, OperationLog log)
+            throws PGPException, IOException, SignatureException {
+
+        // keep track if we actually changed one
+        boolean ok = false;
+        log.add(LogLevel.DEBUG, LogType.MSG_MF_MASTER, indent);
+        indent += 1;
+
+        PGPPublicKey modifiedPublicKey = masterPublicKey;
+
+        // we work on the modifiedPublicKey here, to respect new or newly revoked uids
+        // noinspection unchecked
+        for (String userId : new IterableIterator<String>(modifiedPublicKey.getUserIDs())) {
+            boolean isRevoked = false;
+            PGPSignature currentCert = null;
+            // noinspection unchecked
+            for (PGPSignature cert : new IterableIterator<PGPSignature>(
+                    modifiedPublicKey.getSignaturesForID(userId))) {
+                if (cert.getKeyID() != masterPublicKey.getKeyID()) {
+                    // foreign certificate?! error error error
+                    log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_INTEGRITY, indent);
+                    return null;
+                }
+                // we know from canonicalization that if there is any revocation here, it
+                // is valid and not superseded by a newer certification.
+                if (cert.getSignatureType() == PGPSignature.CERTIFICATION_REVOCATION) {
+                    isRevoked = true;
+                    continue;
+                }
+                // we know from canonicalization that there is only one binding
+                // certification here, so we can just work with the first one.
+                if (cert.getSignatureType() == PGPSignature.NO_CERTIFICATION ||
+                        cert.getSignatureType() == PGPSignature.CASUAL_CERTIFICATION ||
+                        cert.getSignatureType() == PGPSignature.POSITIVE_CERTIFICATION ||
+                        cert.getSignatureType() == PGPSignature.DEFAULT_CERTIFICATION) {
+                    currentCert = cert;
+                }
+            }
+
+            if (currentCert == null) {
+                // no certificate found?! error error error
+                log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_INTEGRITY, indent);
+                return null;
+            }
+
+            // we definitely should not update certifications of revoked keys, so just leave it.
+            if (isRevoked) {
+                continue;
+            }
+
+            // add shiny new user id certificate
+            modifiedPublicKey = PGPPublicKey.removeCertification(
+                    modifiedPublicKey, userId, currentCert);
+            PGPSignature newCert = generateUserIdSignature(
+                    masterPrivateKey, masterPublicKey, userId, true, flags, expiry);
+            modifiedPublicKey = PGPPublicKey.addCertification(
+                    modifiedPublicKey, userId, newCert);
+            ok = true;
+
+        }
+
+        if (!ok) {
+            // might happen, theoretically, if there is a key with no uid..
+            log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_INTEGRITY, indent);
+            return null;
+        }
+
+        return modifiedPublicKey;
+
+    }
+
     private static PGPSignature generateUserIdSignature(
-            PGPPrivateKey masterPrivateKey, PGPPublicKey pKey, String userId, boolean primary, int flags)
+            PGPPrivateKey masterPrivateKey, PGPPublicKey pKey, String userId, boolean primary,
+            int flags, long expiry)
             throws IOException, PGPException, SignatureException {
         PGPContentSignerBuilder signerBuilder = new JcaPGPContentSignerBuilder(
                 pKey.getAlgorithm(), PGPUtil.SHA1)
                 .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME);
         PGPSignatureGenerator sGen = new PGPSignatureGenerator(signerBuilder);
-        PGPSignatureSubpacketGenerator subHashedPacketsGen = new PGPSignatureSubpacketGenerator();
+
-        subHashedPacketsGen.setSignatureCreationTime(false, new Date());
+        PGPSignatureSubpacketGenerator hashedPacketsGen = new PGPSignatureSubpacketGenerator();
-        subHashedPacketsGen.setPreferredSymmetricAlgorithms(true, PREFERRED_SYMMETRIC_ALGORITHMS);
+        {
-        subHashedPacketsGen.setPreferredHashAlgorithms(true, PREFERRED_HASH_ALGORITHMS);
+            hashedPacketsGen.setSignatureCreationTime(false, new Date());
-        subHashedPacketsGen.setPreferredCompressionAlgorithms(true, PREFERRED_COMPRESSION_ALGORITHMS);
+            hashedPacketsGen.setPreferredSymmetricAlgorithms(true, PREFERRED_SYMMETRIC_ALGORITHMS);
-        subHashedPacketsGen.setPrimaryUserID(false, primary);
+            hashedPacketsGen.setPreferredHashAlgorithms(true, PREFERRED_HASH_ALGORITHMS);
-        subHashedPacketsGen.setKeyFlags(false, flags);
+            hashedPacketsGen.setPreferredCompressionAlgorithms(true, PREFERRED_COMPRESSION_ALGORITHMS);
-        sGen.setHashedSubpackets(subHashedPacketsGen.generate());
+            hashedPacketsGen.setPrimaryUserID(false, primary);
+            hashedPacketsGen.setKeyFlags(false, flags);
+            if (expiry > 0) {
+                hashedPacketsGen.setKeyExpirationTime(
+                        false, expiry - pKey.getCreationTime().getTime() / 1000);
+            }
+        }
+
+        sGen.setHashedSubpackets(hashedPacketsGen.generate());
         sGen.init(PGPSignature.POSITIVE_CERTIFICATION, masterPrivateKey);
         return sGen.generateCertification(userId, pKey);
     }
@@ -784,14 +901,15 @@ public class PgpKeyOperation {
             throws IOException, PGPException, SignatureException {
 
         // date for signing
-        Date todayDate = new Date();
+        Date creationTime = new Date();
+
         PGPSignatureSubpacketGenerator unhashedPacketsGen = new PGPSignatureSubpacketGenerator();
 
         // If this key can sign, we need a primary key binding signature
         if ((flags & KeyFlags.SIGN_DATA) > 0) {
             // cross-certify signing keys
             PGPSignatureSubpacketGenerator subHashedPacketsGen = new PGPSignatureSubpacketGenerator();
-            subHashedPacketsGen.setSignatureCreationTime(false, todayDate);
+            subHashedPacketsGen.setSignatureCreationTime(false, creationTime);
             PGPContentSignerBuilder signerBuilder = new JcaPGPContentSignerBuilder(
                     pKey.getAlgorithm(), PGPUtil.SHA1)
                     .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME);
@@ -805,13 +923,12 @@ public class PgpKeyOperation {
         PGPSignatureSubpacketGenerator hashedPacketsGen;
         {
             hashedPacketsGen = new PGPSignatureSubpacketGenerator();
-            hashedPacketsGen.setSignatureCreationTime(false, todayDate);
+            hashedPacketsGen.setSignatureCreationTime(false, creationTime);
             hashedPacketsGen.setKeyFlags(false, flags);
-        }
-
             if (expiry > 0) {
-            long creationTime = pKey.getCreationTime().getTime() / 1000;
+                hashedPacketsGen.setKeyExpirationTime(false,
-            hashedPacketsGen.setKeyExpirationTime(false, expiry - creationTime);
+                        expiry - pKey.getCreationTime().getTime() / 1000);
+            }
         }
 
         PGPContentSignerBuilder signerBuilder = new JcaPGPContentSignerBuilder(

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

@@ -359,12 +359,15 @@ public class OperationResultParcel implements Parcelable {
         MSG_MF_ERROR_FINGERPRINT (R.string.msg_mf_error_fingerprint),
         MSG_MF_ERROR_KEYID (R.string.msg_mf_error_keyid),
         MSG_MF_ERROR_INTEGRITY (R.string.msg_mf_error_integrity),
+        MSG_MF_ERROR_NO_CERTIFY (R.string.msg_cr_error_no_certify),
         MSG_MF_ERROR_NOEXIST_PRIMARY (R.string.msg_mf_error_noexist_primary),
         MSG_MF_ERROR_NOEXIST_REVOKE (R.string.msg_mf_error_noexist_revoke),
         MSG_MF_ERROR_NULL_EXPIRY (R.string.msg_mf_error_null_expiry),
+        MSG_MF_ERROR_PAST_EXPIRY(R.string.msg_mf_error_past_expiry),
         MSG_MF_ERROR_REVOKED_PRIMARY (R.string.msg_mf_error_revoked_primary),
         MSG_MF_ERROR_PGP (R.string.msg_mf_error_pgp),
         MSG_MF_ERROR_SIG (R.string.msg_mf_error_sig),
+        MSG_MF_MASTER (R.string.msg_mf_master),
         MSG_MF_PASSPHRASE (R.string.msg_mf_passphrase),
         MSG_MF_PRIMARY_REPLACE_OLD (R.string.msg_mf_primary_replace_old),
         MSG_MF_PRIMARY_NEW (R.string.msg_mf_primary_new),
@@ -372,7 +375,6 @@ public class OperationResultParcel implements Parcelable {
         MSG_MF_SUBKEY_MISSING (R.string.msg_mf_subkey_missing),
         MSG_MF_SUBKEY_NEW_ID (R.string.msg_mf_subkey_new_id),
         MSG_MF_SUBKEY_NEW (R.string.msg_mf_subkey_new),
-        MSG_MF_SUBKEY_PAST_EXPIRY (R.string.msg_mf_subkey_past_expiry),
         MSG_MF_SUBKEY_REVOKE (R.string.msg_mf_subkey_revoke),
         MSG_MF_SUCCESS (R.string.msg_mf_success),
         MSG_MF_UID_ADD (R.string.msg_mf_uid_add),
@@ -436,6 +438,15 @@ public class OperationResultParcel implements Parcelable {
             mParcels.add(new OperationResultParcel.LogEntryParcel(level, type, indent, (Object[]) null));
         }
 
+        public boolean containsType(LogType type) {
+            for(LogEntryParcel entry : new IterableIterator<LogEntryParcel>(mParcels.iterator())) {
+                if (entry.mType == type) {
+                    return true;
+                }
+            }
+            return false;
+        }
+
         public boolean containsWarnings() {
             for(LogEntryParcel entry : new IterableIterator<LogEntryParcel>(mParcels.iterator())) {
                 if (entry.mLevel == LogLevel.WARN || entry.mLevel == LogLevel.ERROR) {

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

@@ -588,7 +588,7 @@
   <string name="msg_mf_subkey_missing">¡Intentó operar sobre una subclave ausente %s!</string>
   <string name="msg_mf_subkey_new">Generando nueva subclave %2$s de %1$s bits</string>
   <string name="msg_mf_subkey_new_id">Nueva identidad de subclave: %s</string>
-  <string name="msg_mf_subkey_past_expiry">¡La fecha de expiración no puede ser del pasado!</string>
+  <string name="msg_mf_error_past_expiry">¡La fecha de expiración no puede ser del pasado!</string>
   <string name="msg_mf_subkey_revoke">Revocando subclave %s</string>
   <string name="msg_mf_success">Juego de claves modificado con éxito</string>
   <string name="msg_mf_uid_add">Añadiendo identidad de usuario %s</string>

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

@@ -588,7 +588,7 @@
   <string name="msg_mf_subkey_missing">Une action a été tentée sur la sous-clef manquante %s !</string>
   <string name="msg_mf_subkey_new">Génération d\'une nouvelle sous-clef %2$s de %1$s bit</string>
   <string name="msg_mf_subkey_new_id">ID de la nouvelle sous-clef : %s</string>
-  <string name="msg_mf_subkey_past_expiry">La date d\'expiration ne peut pas être dans le passé !</string>
+  <string name="msg_mf_error_past_expiry">La date d\'expiration ne peut pas être dans le passé !</string>
   <string name="msg_mf_subkey_revoke">Révocation de la sous-clef %s</string>
   <string name="msg_mf_success">Trousseau modifié avec succès</string>
   <string name="msg_mf_uid_add">Ajout de l\'ID d\'utilisateur %s</string>

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

@@ -528,7 +528,7 @@
   <string name="msg_mf_subkey_missing">Tentativo di operare su sottochiave mancante %s!</string>
   <string name="msg_mf_subkey_new">Generazione nuovi %1$s bit %2$s sottochiave</string>
   <string name="msg_mf_subkey_new_id">Nuovo ID sottochiave: %s</string>
-  <string name="msg_mf_subkey_past_expiry">La data di scadenza non può essere passata!</string>
+  <string name="msg_mf_error_past_expiry">La data di scadenza non può essere passata!</string>
   <string name="msg_mf_subkey_revoke">Revoca sottochiave %s</string>
   <string name="msg_mf_success">Portachiavi modificato con successo</string>
   <string name="msg_mf_uid_add">Aggiunta id utente %s</string>

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

@@ -574,7 +574,7 @@
   <string name="msg_mf_subkey_missing">遺失した副鍵 %s の操作をしようとした!</string>
   <string name="msg_mf_subkey_new">新しい %1$s ビットの %2$s 副鍵の生成中</string>
   <string name="msg_mf_subkey_new_id">新しい副鍵 ID: %s</string>
-  <string name="msg_mf_subkey_past_expiry">期限切れ日を過去にはできません!</string>
+  <string name="msg_mf_error_past_expiry">期限切れ日を過去にはできません!</string>
   <string name="msg_mf_subkey_revoke">副鍵 %s を破棄中</string>
   <string name="msg_mf_success">鍵輪の変更に成功</string>
   <string name="msg_mf_uid_add">ユーザID %s を追加中</string>

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

@@ -384,7 +384,7 @@
   <string name="msg_mf_error_pgp">Внутренняя ошибка PGP!</string>
   <string name="msg_mf_error_sig">Ошибка подписи!</string>
   <string name="msg_mf_passphrase">Изменение пароля</string>
-  <string name="msg_mf_subkey_past_expiry">Срок годности не может быть в прошлом!</string>
+  <string name="msg_mf_error_past_expiry">Срок годности не может быть в прошлом!</string>
   <string name="msg_mf_success">Связка успешно изменена</string>
   <string name="msg_mf_uid_add">Добавление id %s</string>
   <string name="msg_mf_uid_primary">Изменение основного uid на %s</string>

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

@@ -639,12 +639,14 @@
     <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>
     <string name="msg_mf_error_integrity">Internal error, integrity check failed!</string>
+    <string name="msg_mf_error_noexist_master">No master certificate found to modify!</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_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_pgp">PGP internal exception!</string>
     <string name="msg_mf_error_sig">Signature exception!</string>
+    <string name="msg_mf_master">Modifying master certifications</string>
     <string name="msg_mf_passphrase">Changing passphrase</string>
     <string name="msg_mf_primary_replace_old">Replacing certificate of previous primary user id</string>
     <string name="msg_mf_primary_new">Generating new certificate for new primary user id</string>
@@ -652,7 +654,7 @@
     <string name="msg_mf_subkey_missing">Tried to operate on missing subkey %s!</string>
     <string name="msg_mf_subkey_new">Generating new %1$s bit %2$s subkey</string>
     <string name="msg_mf_subkey_new_id">New subkey ID: %s</string>
-    <string name="msg_mf_subkey_past_expiry">Expiry date cannot be in the past!</string>
+    <string name="msg_mf_error_past_expiry">Expiry date cannot be in the past!</string>
     <string name="msg_mf_subkey_revoke">Revoking subkey %s</string>
     <string name="msg_mf_success">Keyring successfully modified</string>
     <string name="msg_mf_uid_add">Adding user id %s</string>