From ebcd243e975ce8f04298f6cd94cb940bfcea3d59 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Wed, 2 Jul 2014 16:02:56 +0200 Subject: [PATCH] support changing primary user id in SaveKeyringParcel Closes #695 --- .../keychain/pgp/PgpKeyOperation.java | 115 +++++++++++++++++- .../service/OperationResultParcel.java | 2 + OpenKeychain/src/main/res/values/strings.xml | 2 + 3 files changed, 115 insertions(+), 4 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 528d36da2..9a08290e4 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java @@ -52,6 +52,7 @@ import org.sufficientlysecure.keychain.service.OperationResultParcel.LogType; import org.sufficientlysecure.keychain.service.OperationResultParcel.OperationLog; import org.sufficientlysecure.keychain.service.SaveKeyringParcel; import org.sufficientlysecure.keychain.service.SaveKeyringParcel.SubkeyAdd; +import org.sufficientlysecure.keychain.util.IterableIterator; import org.sufficientlysecure.keychain.util.Log; import org.sufficientlysecure.keychain.util.Primes; @@ -66,6 +67,7 @@ import java.security.SignatureException; import java.util.Arrays; import java.util.Calendar; import java.util.Date; +import java.util.Iterator; import java.util.TimeZone; /** @@ -289,14 +291,42 @@ public class PgpKeyOperation { // 2a. Add certificates for new user ids for (String userId : saveParcel.addUserIds) { log.add(LogLevel.INFO, LogType.MSG_MF_UID_ADD, indent); + + // this operation supersedes all previous binding and revocation certificates, + // so remove those to retain assertions from canonicalization for later operations + @SuppressWarnings("unchecked") + Iterator it = modifiedPublicKey.getSignaturesForID(userId); + if (it != null) { + for (PGPSignature cert : new IterableIterator(it)) { + // if it's not a self cert, never mind + if (cert.getKeyID() != masterPublicKey.getKeyID()) { + continue; + } + 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.changePrimaryUserId != null + && userId.equals(saveParcel.changePrimaryUserId); + // generate and add new certificate PGPSignature cert = generateUserIdSignature(masterPrivateKey, - masterPublicKey, userId, false); + masterPublicKey, userId, isPrimary); modifiedPublicKey = PGPPublicKey.addCertification(masterPublicKey, userId, cert); } // 2b. Add revocations for revoked user ids for (String userId : saveParcel.revokeUserIds) { log.add(LogLevel.INFO, LogType.MSG_MF_UID_REVOKE, indent); + // a duplicate revocatin will be removed during canonicalization, so no need to + // take care of that here. PGPSignature cert = generateRevocationSignature(masterPrivateKey, masterPublicKey, userId); modifiedPublicKey = PGPPublicKey.addCertification(masterPublicKey, userId, cert); @@ -305,7 +335,84 @@ public class PgpKeyOperation { // 3. If primary user id changed, generate new certificates for both old and new if (saveParcel.changePrimaryUserId != null) { log.add(LogLevel.INFO, LogType.MSG_MF_UID_PRIMARY, indent); - // todo + + // we work on the modifiedPublicKey here, to respect new or newly revoked uids + // noinspection unchecked + for (String userId : new IterableIterator(modifiedPublicKey.getUserIDs())) { + boolean isRevoked = false; + PGPSignature currentCert = null; + // noinspection unchecked + for (PGPSignature cert : new IterableIterator( + masterPublicKey.getSignaturesForID(userId))) { + // if it's not a self cert, never mind + if (cert.getKeyID() != masterPublicKey.getKeyID()) { + continue; + } + // 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) { + // revoked user ids cannot be primary! + if (userId.equals(saveParcel.changePrimaryUserId)) { + log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_REVOKED_PRIMARY, indent); + return null; + } + continue; + } + + // if this is~ the/a primary user id + if (currentCert.hasSubpackets() && currentCert.getHashedSubPackets().isPrimaryUserID()) { + // if it's the one we want, just leave it as is + if (userId.equals(saveParcel.changePrimaryUserId)) { + continue; + } + // otherwise, generate new non-primary certification + modifiedPublicKey = PGPPublicKey.removeCertification( + modifiedPublicKey, userId, currentCert); + PGPSignature newCert = generateUserIdSignature( + masterPrivateKey, masterPublicKey, userId, false); + 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.changePrimaryUserId)) { + // add shiny new primary user id certificate + modifiedPublicKey = PGPPublicKey.removeCertification( + modifiedPublicKey, userId, currentCert); + PGPSignature newCert = generateUserIdSignature( + masterPrivateKey, masterPublicKey, userId, true); + modifiedPublicKey = PGPPublicKey.addCertification( + modifiedPublicKey, userId, newCert); + } + + // user id is not primary and is not supposed to be - nothing to do here. + + } + } // Update the secret key ring @@ -315,7 +422,6 @@ public class PgpKeyOperation { sKR = PGPSecretKeyRing.insertSecretKey(sKR, masterSecretKey); } - // 4a. For each subkey change, generate new subkey binding certificate for (SaveKeyringParcel.SubkeyChange change : saveParcel.changeSubKeys) { log.add(LogLevel.INFO, LogType.MSG_MF_SUBKEY_CHANGE, @@ -334,7 +440,8 @@ public class PgpKeyOperation { return null; } - // generate and add new signature + // generate and add new signature. we can be sloppy here and just leave the old one, + // it will be removed during canonicalization PGPSignature sig = generateSubkeyBindingSignature(masterPublicKey, masterPrivateKey, sKey, pKey, change.mFlags, change.mExpiry, passphrase); pKey = PGPPublicKey.addCertification(pKey, sig); 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 535fa08cf..f5d5b8f97 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java @@ -241,6 +241,8 @@ public class OperationResultParcel implements Parcelable { MSG_MF_ERROR_ENCODE (R.string.msg_mf_error_encode), 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_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_PASSPHRASE (R.string.msg_mf_passphrase), diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml index d886fe241..d91707567 100644 --- a/OpenKeychain/src/main/res/values/strings.xml +++ b/OpenKeychain/src/main/res/values/strings.xml @@ -628,6 +628,8 @@ Encoding exception! Actual key fingerprint does not match expected! No keyid. This is a programming error, please file a bug report! + Internal error, integrity check failed! + Revoked user ids cannot be primary! PGP internal exception! Signature exception! Changing passphrase