support for master key modifications, among other stuff

2 files changed, 310 insertions, 182 deletions

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 d7658efef..710bd42c6 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java
+++ b/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,189 +354,196 @@ public class PgpKeyOperation {
             }
         }
 
-        // work on master secret key
         try {
 
-            PGPPublicKey modifiedPublicKey = masterPublicKey;
+            { // work on master secret key
 
-            // 2a. Add certificates for new user ids
-            subProgressPush(15, 25);
-            for (int i = 0; i < saveParcel.mAddUserIds.size(); i++) {
+                PGPPublicKey modifiedPublicKey = masterPublicKey;
 
-                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);
+                // 2a. Add certificates for new user ids
+                subProgressPush(15, 25);
+                for (int i = 0; i < saveParcel.mAddUserIds.size(); i++) {
 
-                if (userId.equals("")) {
-                    log.add(LogLevel.ERROR, LogType.MSG_MF_UID_ERROR_EMPTY, indent+1);
-                    return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
-                }
+                    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);
 
-                // this operation supersedes all previous binding and revocation certificates,
-                // so remove those to retain assertions from canonicalization for later operations
-                @SuppressWarnings("unchecked")
-                Iterator<PGPSignature> it = modifiedPublicKey.getSignaturesForID(userId);
-                if (it != null) {
-                    for (PGPSignature cert : new IterableIterator<PGPSignature>(it)) {
-                        if (cert.getKeyID() != masterPublicKey.getKeyID()) {
-                            // foreign certificate?! error error error
-                            log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_INTEGRITY, indent);
-                            return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
-                        }
-                        if (cert.getSignatureType() == PGPSignature.CERTIFICATION_REVOCATION
-                                || cert.getSignatureType() == PGPSignature.NO_CERTIFICATION
-                                || cert.getSignatureType() == PGPSignature.CASUAL_CERTIFICATION
-                                || cert.getSignatureType() == PGPSignature.POSITIVE_CERTIFICATION
-                                || cert.getSignatureType() == PGPSignature.DEFAULT_CERTIFICATION) {
-                            modifiedPublicKey = PGPPublicKey.removeCertification(
-                                    modifiedPublicKey, userId, cert);
+                    if (userId.equals("")) {
+                        log.add(LogLevel.ERROR, LogType.MSG_MF_UID_ERROR_EMPTY, indent + 1);
+                        return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
+                    }
+
+                    // this operation supersedes all previous binding and revocation certificates,
+                    // so remove those to retain assertions from canonicalization for later operations
+                    @SuppressWarnings("unchecked")
+                    Iterator<PGPSignature> it = modifiedPublicKey.getSignaturesForID(userId);
+                    if (it != null) {
+                        for (PGPSignature cert : new IterableIterator<PGPSignature>(it)) {
+                            if (cert.getKeyID() != masterPublicKey.getKeyID()) {
+                                // foreign certificate?! error error error
+                                log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_INTEGRITY, indent);
+                                return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
+                            }
+                            if (cert.getSignatureType() == PGPSignature.CERTIFICATION_REVOCATION
+                                    || cert.getSignatureType() == PGPSignature.NO_CERTIFICATION
+                                    || cert.getSignatureType() == PGPSignature.CASUAL_CERTIFICATION
+                                    || cert.getSignatureType() == PGPSignature.POSITIVE_CERTIFICATION
+                                    || cert.getSignatureType() == PGPSignature.DEFAULT_CERTIFICATION) {
+                                modifiedPublicKey = PGPPublicKey.removeCertification(
+                                        modifiedPublicKey, userId, cert);
+                            }
                         }
                     }
-                }
 
-                // if it's supposed to be primary, we can do that here as well
-                boolean isPrimary = saveParcel.mChangePrimaryUserId != null
-                        && userId.equals(saveParcel.mChangePrimaryUserId);
-                // generate and add new certificate
-                PGPSignature cert = generateUserIdSignature(masterPrivateKey,
-                        masterPublicKey, userId, isPrimary, masterKeyFlags);
-                modifiedPublicKey = PGPPublicKey.addCertification(modifiedPublicKey, userId, cert);
-            }
-            subProgressPop();
+                    // if it's supposed to be primary, we can do that here as well
+                    boolean isPrimary = saveParcel.mChangePrimaryUserId != null
+                            && userId.equals(saveParcel.mChangePrimaryUserId);
+                    // generate and add new certificate
+                    PGPSignature cert = generateUserIdSignature(masterPrivateKey,
+                            masterPublicKey, userId, isPrimary, masterKeyFlags, masterKeyExpiry);
+                    modifiedPublicKey = PGPPublicKey.addCertification(modifiedPublicKey, userId, cert);
+                }
+                subProgressPop();
 
-            // 2b. Add revocations for revoked user ids
-            subProgressPush(25, 40);
-            for (int i = 0; i < saveParcel.mRevokeUserIds.size(); i++) {
-
-                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;
-                for (String uid : new IterableIterator<String>(modifiedPublicKey.getUserIDs())) {
-                    if (userId.equals(uid)) {
-                        exists = true;
-                        break;
+                // 2b. Add revocations for revoked user ids
+                subProgressPush(25, 40);
+                for (int i = 0; i < saveParcel.mRevokeUserIds.size(); i++) {
+
+                    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;
+                            break;
+                        }
+                    }
+                    if (!exists) {
+                        log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_NOEXIST_REVOKE, indent);
+                        return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
                     }
-                }
-                if (!exists) {
-                    log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_NOEXIST_REVOKE, indent);
-                    return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
-                }
 
-                // a duplicate revocation will be removed during canonicalization, so no need to
-                // take care of that here.
-                PGPSignature cert = generateRevocationSignature(masterPrivateKey,
-                        masterPublicKey, userId);
-                modifiedPublicKey = PGPPublicKey.addCertification(modifiedPublicKey, userId, cert);
-            }
-            subProgressPop();
+                    // a duplicate revocation will be removed during canonicalization, so no need to
+                    // take care of that here.
+                    PGPSignature cert = generateRevocationSignature(masterPrivateKey,
+                            masterPublicKey, userId);
+                    modifiedPublicKey = PGPPublicKey.addCertification(modifiedPublicKey, userId, cert);
+                }
+                subProgressPop();
 
-            // 3. If primary user id changed, generate new certificates for both old and new
-            if (saveParcel.mChangePrimaryUserId != null) {
-                progress(R.string.progress_modify_primaryuid, 40);
+                // 3. If primary user id changed, generate new certificates for both old and new
+                if (saveParcel.mChangePrimaryUserId != null) {
+                    progress(R.string.progress_modify_primaryuid, 40);
 
-                // keep track if we actually changed one
-                boolean ok = false;
-                log.add(LogLevel.INFO, LogType.MSG_MF_UID_PRIMARY, indent);
-                indent += 1;
+                    // keep track if we actually changed one
+                    boolean ok = false;
+                    log.add(LogLevel.INFO, LogType.MSG_MF_UID_PRIMARY, indent);
+                    indent += 1;
 
-                // 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;
+                    // we work on the modifiedPublicKey here, to respect new or newly revoked uids
                     // noinspection unchecked
-                    for (PGPSignature cert : new IterableIterator<PGPSignature>(
-                            modifiedPublicKey.getSignaturesForID(userId))) {
-                        if (cert.getKeyID() != masterPublicKey.getKeyID()) {
-                            // foreign certificate?! error error error
+                    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 new EditKeyResult(EditKeyResult.RESULT_ERROR, log, 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 new EditKeyResult(EditKeyResult.RESULT_ERROR, log, 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;
+
+                        // we definitely should not update certifications of revoked keys, so just leave it.
+                        if (isRevoked) {
+                            // revoked user ids cannot be primary!
+                            if (userId.equals(saveParcel.mChangePrimaryUserId)) {
+                                log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_REVOKED_PRIMARY, indent);
+                                return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
+                            }
                             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 new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
-                    }
-
-                    // we definitely should not update certifications of revoked keys, so just leave it.
-                    if (isRevoked) {
-                        // revoked user ids cannot be primary!
-                        if (userId.equals(saveParcel.mChangePrimaryUserId)) {
-                            log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_REVOKED_PRIMARY, indent);
-                            return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
+                        // if this is~ the/a primary user id
+                        if (currentCert.getHashedSubPackets() != null
+                                && currentCert.getHashedSubPackets().isPrimaryUserID()) {
+                            // if it's the one we want, just leave it as is
+                            if (userId.equals(saveParcel.mChangePrimaryUserId)) {
+                                ok = true;
+                                continue;
+                            }
+                            // otherwise, generate new non-primary certification
+                            log.add(LogLevel.DEBUG, LogType.MSG_MF_PRIMARY_REPLACE_OLD, indent);
+                            modifiedPublicKey = PGPPublicKey.removeCertification(
+                                    modifiedPublicKey, userId, currentCert);
+                            PGPSignature newCert = generateUserIdSignature(
+                                    masterPrivateKey, masterPublicKey, userId, false,
+                                    masterKeyFlags, masterKeyExpiry);
+                            modifiedPublicKey = PGPPublicKey.addCertification(
+                                    modifiedPublicKey, userId, newCert);
+                            continue;
                         }
-                        continue;
-                    }
 
-                    // if this is~ the/a primary user id
-                    if (currentCert.getHashedSubPackets() != null
-                            && currentCert.getHashedSubPackets().isPrimaryUserID()) {
-                        // if it's the one we want, just leave it as is
+                        // if we are here, this is not currently a primary user id
+
+                        // if it should be
                         if (userId.equals(saveParcel.mChangePrimaryUserId)) {
+                            // add shiny new primary user id certificate
+                            log.add(LogLevel.DEBUG, LogType.MSG_MF_PRIMARY_NEW, indent);
+                            modifiedPublicKey = PGPPublicKey.removeCertification(
+                                    modifiedPublicKey, userId, currentCert);
+                            PGPSignature newCert = generateUserIdSignature(
+                                    masterPrivateKey, masterPublicKey, userId, true,
+                                    masterKeyFlags, masterKeyExpiry);
+                            modifiedPublicKey = PGPPublicKey.addCertification(
+                                    modifiedPublicKey, userId, newCert);
                             ok = true;
-                            continue;
                         }
-                        // otherwise, generate new non-primary certification
-                        log.add(LogLevel.DEBUG, LogType.MSG_MF_PRIMARY_REPLACE_OLD, indent);
-                        modifiedPublicKey = PGPPublicKey.removeCertification(
-                                modifiedPublicKey, userId, currentCert);
-                        PGPSignature newCert = generateUserIdSignature(
-                                masterPrivateKey, masterPublicKey, userId, false, masterKeyFlags);
-                        modifiedPublicKey = PGPPublicKey.addCertification(
-                                modifiedPublicKey, userId, newCert);
-                        continue;
-                    }
 
-                    // if we are here, this is not currently a primary user id
-
-                    // if it should be
-                    if (userId.equals(saveParcel.mChangePrimaryUserId)) {
-                        // add shiny new primary user id certificate
-                        log.add(LogLevel.DEBUG, LogType.MSG_MF_PRIMARY_NEW, indent);
-                        modifiedPublicKey = PGPPublicKey.removeCertification(
-                                modifiedPublicKey, userId, currentCert);
-                        PGPSignature newCert = generateUserIdSignature(
-                                masterPrivateKey, masterPublicKey, userId, true, masterKeyFlags);
-                        modifiedPublicKey = PGPPublicKey.addCertification(
-                                modifiedPublicKey, userId, newCert);
-                        ok = true;
+                        // user id is not primary and is not supposed to be - nothing to do here.
+
                     }
 
-                    // user id is not primary and is not supposed to be - nothing to do here.
+                    indent -= 1;
 
+                    if (!ok) {
+                        log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_NOEXIST_PRIMARY, indent);
+                        return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
+                    }
                 }
 
-                indent -= 1;
-
-                if (!ok) {
-                    log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_NOEXIST_PRIMARY, indent);
-                    return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
+                // Update the secret key ring
+                if (modifiedPublicKey != masterPublicKey) {
+                    masterSecretKey = PGPSecretKey.replacePublicKey(masterSecretKey, modifiedPublicKey);
+                    masterPublicKey = modifiedPublicKey;
+                    sKR = PGPSecretKeyRing.insertSecretKey(sKR, masterSecretKey);
                 }
-            }
 
-            // Update the secret key ring
-            if (modifiedPublicKey != masterPublicKey) {
-                masterSecretKey = PGPSecretKey.replacePublicKey(masterSecretKey, modifiedPublicKey);
-                masterPublicKey = modifiedPublicKey;
-                sKR = PGPSecretKeyRing.insertSecretKey(sKR, masterSecretKey);
             }
 
             // 4a. For each subkey change, generate new subkey binding certificate
@@ -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())) {
-                    log.add(LogLevel.ERROR, LogType.MSG_MF_SUBKEY_PAST_EXPIRY, indent +1);
+                if (add.mExpiry == null) {
+                    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());
-        subHashedPacketsGen.setPreferredSymmetricAlgorithms(true, PREFERRED_SYMMETRIC_ALGORITHMS);
-        subHashedPacketsGen.setPreferredHashAlgorithms(true, PREFERRED_HASH_ALGORITHMS);
-        subHashedPacketsGen.setPreferredCompressionAlgorithms(true, PREFERRED_COMPRESSION_ALGORITHMS);
-        subHashedPacketsGen.setPrimaryUserID(false, primary);
-        subHashedPacketsGen.setKeyFlags(false, flags);
-        sGen.setHashedSubpackets(subHashedPacketsGen.generate());
+
+        PGPSignatureSubpacketGenerator hashedPacketsGen = new PGPSignatureSubpacketGenerator();
+        {
+            hashedPacketsGen.setSignatureCreationTime(false, new Date());
+            hashedPacketsGen.setPreferredSymmetricAlgorithms(true, PREFERRED_SYMMETRIC_ALGORITHMS);
+            hashedPacketsGen.setPreferredHashAlgorithms(true, PREFERRED_HASH_ALGORITHMS);
+            hashedPacketsGen.setPreferredCompressionAlgorithms(true, PREFERRED_COMPRESSION_ALGORITHMS);
+            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, expiry - creationTime);
+            if (expiry > 0) {
+                hashedPacketsGen.setKeyExpirationTime(false,
+                        expiry - pKey.getCreationTime().getTime() / 1000);
+            }
         }
 
         PGPContentSignerBuilder signerBuilder = new JcaPGPContentSignerBuilder(
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java
index 01c7611ff..138283b81 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java
+++ b/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) {