diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/Constants.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/Constants.java index 5e4fa5f80..ffdbe950e 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/Constants.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/Constants.java @@ -179,12 +179,12 @@ public final class Constants { /** * Default key configuration: 3072 bit RSA (certify, sign, encrypt) */ - public static void addDefaultSubkeys(SaveKeyringParcel saveKeyringParcel) { - saveKeyringParcel.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd(SaveKeyringParcel.Algorithm.RSA, + public static void addDefaultSubkeys(SaveKeyringParcel.Builder builder) { + builder.addSubkeyAdd(SubkeyAdd.createSubkeyAdd(SaveKeyringParcel.Algorithm.RSA, 3072, null, KeyFlags.CERTIFY_OTHER, 0L)); - saveKeyringParcel.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd(SaveKeyringParcel.Algorithm.RSA, + builder.addSubkeyAdd(SubkeyAdd.createSubkeyAdd(SaveKeyringParcel.Algorithm.RSA, 3072, null, KeyFlags.SIGN_DATA, 0L)); - saveKeyringParcel.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd(SaveKeyringParcel.Algorithm.RSA, + builder.addSubkeyAdd(SubkeyAdd.createSubkeyAdd(SaveKeyringParcel.Algorithm.RSA, 3072, null, KeyFlags.ENCRYPT_COMMS | KeyFlags.ENCRYPT_STORAGE, 0L)); } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/EditKeyOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/EditKeyOperation.java index 57161b61b..f7d2d86c6 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/EditKeyOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/EditKeyOperation.java @@ -88,13 +88,13 @@ public class EditKeyOperation extends BaseReadWriteOperation new PgpKeyOperation(new ProgressScaler(mProgressable, 10, 60, 100), mCancelled); // If a key id is specified, fetch and edit - if (saveParcel.mMasterKeyId != null) { + if (saveParcel.getMasterKeyId() != null) { try { log.add(LogType.MSG_ED_FETCHING, 1, - KeyFormattingUtils.convertKeyIdToHex(saveParcel.mMasterKeyId)); + KeyFormattingUtils.convertKeyIdToHex(saveParcel.getMasterKeyId())); CanonicalizedSecretKeyRing secRing = - mKeyRepository.getCanonicalizedSecretKeyRing(saveParcel.mMasterKeyId); + mKeyRepository.getCanonicalizedSecretKeyRing(saveParcel.getMasterKeyId()); modifyResult = keyOperations.modifySecretKeyRing(secRing, cryptoInput, saveParcel); if (modifyResult.isPending()) { @@ -133,7 +133,7 @@ public class EditKeyOperation extends BaseReadWriteOperation // It's a success, so this must be non-null now UncachedKeyRing ring = modifyResult.getRing(); - if (saveParcel.isUpload()) { + if (saveParcel.isShouldUpload()) { byte[] keyringBytes; try { UncachedKeyRing publicKeyRing = ring.extractPublicKeyRing(); @@ -154,7 +154,7 @@ public class EditKeyOperation extends BaseReadWriteOperation if (uploadResult.isPending()) { return new EditKeyResult(log, uploadResult); - } else if (!uploadResult.success() && saveParcel.isUploadAtomic()) { + } else if (!uploadResult.success() && saveParcel.isShouldUploadAtomic()) { // if atomic, update fail implies edit operation should also fail and not save return new EditKeyResult(log, RequiredInputParcel.createRetryUploadOperation(), cryptoInput); } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/RevokeOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/RevokeOperation.java index 197842ed9..d0808f4db 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/RevokeOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/RevokeOperation.java @@ -71,17 +71,18 @@ public class RevokeOperation extends BaseReadWriteOperation return new RevokeResult(RevokeResult.RESULT_ERROR, log, masterKeyId); } - SaveKeyringParcel saveKeyringParcel = - new SaveKeyringParcel(masterKeyId, keyRing.getFingerprint()); + SaveKeyringParcel.Builder saveKeyringParcel = + SaveKeyringParcel.buildChangeKeyringParcel(masterKeyId, keyRing.getFingerprint()); // all revoke operations are made atomic as of now saveKeyringParcel.setUpdateOptions(revokeKeyringParcel.isShouldUpload(), true, revokeKeyringParcel.getKeyserver()); - saveKeyringParcel.mRevokeSubKeys.add(masterKeyId); + saveKeyringParcel.addRevokeSubkey(masterKeyId); EditKeyResult revokeAndUploadResult = new EditKeyOperation(mContext, - mKeyWritableRepository, mProgressable, mCancelled).execute(saveKeyringParcel, cryptoInputParcel); + mKeyWritableRepository, mProgressable, mCancelled).execute( + saveKeyringParcel.build(), cryptoInputParcel); if (revokeAndUploadResult.isPending()) { return revokeAndUploadResult; 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 91dc06921..011657a95 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java @@ -29,10 +29,10 @@ import java.security.NoSuchProviderException; import java.security.SecureRandom; import java.security.SignatureException; import java.security.spec.ECGenParameterSpec; -import java.util.ArrayList; import java.util.Arrays; import java.util.Date; import java.util.Iterator; +import java.util.List; import java.util.Stack; import java.util.concurrent.atomic.AtomicBoolean; @@ -80,6 +80,7 @@ import org.sufficientlysecure.keychain.operations.results.PgpEditKeyResult; import org.sufficientlysecure.keychain.service.ChangeUnlockParcel; import org.sufficientlysecure.keychain.service.SaveKeyringParcel; import org.sufficientlysecure.keychain.service.SaveKeyringParcel.Algorithm; +import org.sufficientlysecure.keychain.service.SaveKeyringParcel.Builder; import org.sufficientlysecure.keychain.service.SaveKeyringParcel.Curve; import org.sufficientlysecure.keychain.service.SaveKeyringParcel.SubkeyAdd; import org.sufficientlysecure.keychain.service.SaveKeyringParcel.SubkeyChange; @@ -287,23 +288,23 @@ public class PgpKeyOperation { progress(R.string.progress_building_key, 0); indent += 1; - if (saveParcel.mAddSubKeys.isEmpty()) { + if (saveParcel.getAddSubKeys().isEmpty()) { log.add(LogType.MSG_CR_ERROR_NO_MASTER, indent); return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null); } - if (saveParcel.mAddUserIds.isEmpty()) { + if (saveParcel.getAddUserIds().isEmpty()) { log.add(LogType.MSG_CR_ERROR_NO_USER_ID, indent); return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null); } - SubkeyAdd add = saveParcel.mAddSubKeys.remove(0); - if ((add.getFlags() & KeyFlags.CERTIFY_OTHER) != KeyFlags.CERTIFY_OTHER) { + SubkeyAdd certificationKey = saveParcel.getAddSubKeys().get(0); + if ((certificationKey.getFlags() & KeyFlags.CERTIFY_OTHER) != KeyFlags.CERTIFY_OTHER) { log.add(LogType.MSG_CR_ERROR_NO_CERTIFY, indent); return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null); } - if (add.getExpiry() == null) { + if (certificationKey.getExpiry() == null) { log.add(LogType.MSG_CR_ERROR_NULL_EXPIRY, indent); return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null); } @@ -311,7 +312,7 @@ public class PgpKeyOperation { Date creationTime = new Date(); subProgressPush(10, 30); - PGPKeyPair keyPair = createKey(add, creationTime, log, indent); + PGPKeyPair keyPair = createKey(certificationKey, creationTime, log, indent); subProgressPop(); // return null if this failed (an error will already have been logged by createKey) @@ -337,9 +338,14 @@ public class PgpKeyOperation { PGPSecretKeyRing sKR = new PGPSecretKeyRing( masterSecretKey.getEncoded(), new JcaKeyFingerprintCalculator()); + // Remove certification key from remaining SaveKeyringParcel + Builder builder = SaveKeyringParcel.buildUpon(saveParcel); + builder.getMutableAddSubKeys().remove(certificationKey); + saveParcel = builder.build(); + subProgressPush(50, 100); CryptoInputParcel cryptoInput = CryptoInputParcel.createCryptoInputParcel(creationTime, new Passphrase("")); - return internal(sKR, masterSecretKey, add.getFlags(), add.getExpiry(), cryptoInput, saveParcel, log, indent); + return internal(sKR, masterSecretKey, certificationKey.getFlags(), certificationKey.getExpiry(), cryptoInput, saveParcel, log, indent); } catch (PGPException e) { log.add(LogType.MSG_CR_ERROR_INTERNAL_PGP, indent); @@ -394,7 +400,7 @@ public class PgpKeyOperation { progress(R.string.progress_building_key, 0); // Make sure this is called with a proper SaveKeyringParcel - if (saveParcel.mMasterKeyId == null || saveParcel.mMasterKeyId != wsKR.getMasterKeyId()) { + if (saveParcel.getMasterKeyId() == null || saveParcel.getMasterKeyId() != wsKR.getMasterKeyId()) { log.add(LogType.MSG_MF_ERROR_KEYID, indent); return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null); } @@ -404,75 +410,29 @@ public class PgpKeyOperation { PGPSecretKey masterSecretKey = sKR.getSecretKey(); // Make sure the fingerprint matches - if (saveParcel.mFingerprint == null || !Arrays.equals(saveParcel.mFingerprint, + if (saveParcel.getFingerprint() == null || !Arrays.equals(saveParcel.getFingerprint(), masterSecretKey.getPublicKey().getFingerprint())) { log.add(LogType.MSG_MF_ERROR_FINGERPRINT, indent); return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null); } - if (saveParcel.isEmpty()) { + if (isParcelEmpty(saveParcel)) { log.add(LogType.MSG_MF_ERROR_NOOP, indent); return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null); } - // Ensure we don't have multiple keys for the same slot. - boolean hasSign = false; - boolean hasEncrypt = false; - boolean hasAuth = false; - for (SaveKeyringParcel.SubkeyChange change : new ArrayList<>(saveParcel.mChangeSubKeys)) { - if (change.getMoveKeyToSecurityToken()) { - // If this is a moveKeyToSecurityToken operation, see if it was completed: look for a hash - // matching the given subkey ID in cryptoData. - byte[] subKeyId = new byte[8]; - ByteBuffer buf = ByteBuffer.wrap(subKeyId); - buf.putLong(change.getSubKeyId()).rewind(); + saveParcel = parseSecurityTokenSerialNumberIntoSubkeyChanges(cryptoInput, saveParcel); - byte[] serialNumber = cryptoInput.getCryptoData().get(buf); - if (serialNumber != null) { - saveParcel.addOrReplaceSubkeyChange( - SubkeyChange.createSecurityTokenSerialNo(change.getSubKeyId(), serialNumber)); - } - } - - if (change.getMoveKeyToSecurityToken()) { - // Pending moveKeyToSecurityToken operation. Need to make sure that we don't have multiple - // subkeys pending for the same slot. - CanonicalizedSecretKey wsK = wsKR.getSecretKey(change.getSubKeyId()); - - if ((wsK.canSign() || wsK.canCertify())) { - if (hasSign) { - log.add(LogType.MSG_MF_ERROR_DUPLICATE_KEYTOCARD_FOR_SLOT, indent + 1); - return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null); - } else { - hasSign = true; - } - } else if ((wsK.canEncrypt())) { - if (hasEncrypt) { - log.add(LogType.MSG_MF_ERROR_DUPLICATE_KEYTOCARD_FOR_SLOT, indent + 1); - return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null); - } else { - hasEncrypt = true; - } - } else if ((wsK.canAuthenticate())) { - if (hasAuth) { - log.add(LogType.MSG_MF_ERROR_DUPLICATE_KEYTOCARD_FOR_SLOT, indent + 1); - return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null); - } else { - hasAuth = true; - } - } else { - log.add(LogType.MSG_MF_ERROR_INVALID_FLAGS_FOR_KEYTOCARD, indent + 1); - return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null); - } - } + if (!checkCapabilitiesAreUnique(wsKR, saveParcel, log, indent)) { + return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null); } - if (isDummy(masterSecretKey) && ! saveParcel.isRestrictedOnly()) { + if (isDummy(masterSecretKey) && ! isParcelRestrictedOnly(saveParcel)) { log.add(LogType.MSG_EK_ERROR_DUMMY, indent); return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null); } - if (isDummy(masterSecretKey) || saveParcel.isRestrictedOnly()) { + if (isDummy(masterSecretKey) || isParcelRestrictedOnly(saveParcel)) { log.add(LogType.MSG_MF_RESTRICTED_MODE, indent); return internalRestricted(sKR, saveParcel, log, indent + 1); } @@ -496,6 +456,70 @@ public class PgpKeyOperation { } + private SaveKeyringParcel parseSecurityTokenSerialNumberIntoSubkeyChanges(CryptoInputParcel cryptoInput, + SaveKeyringParcel saveParcel) { + SaveKeyringParcel.Builder builder = SaveKeyringParcel.buildUpon(saveParcel); + for (SubkeyChange change : saveParcel.getChangeSubKeys()) { + if (change.getMoveKeyToSecurityToken()) { + // If this is a moveKeyToSecurityToken operation, see if it was completed: look for a hash + // matching the given subkey ID in cryptoData. + byte[] subKeyId = new byte[8]; + ByteBuffer buf = ByteBuffer.wrap(subKeyId); + buf.putLong(change.getSubKeyId()).rewind(); + + byte[] serialNumber = cryptoInput.getCryptoData().get(buf); + if (serialNumber != null) { + builder.addOrReplaceSubkeyChange( + SubkeyChange.createSecurityTokenSerialNo(change.getSubKeyId(), serialNumber)); + } + } + } + saveParcel = builder.build(); + return saveParcel; + } + + private boolean checkCapabilitiesAreUnique(CanonicalizedSecretKeyRing wsKR, SaveKeyringParcel saveParcel, + OperationLog log, int indent) { + boolean hasSign = false; + boolean hasEncrypt = false; + boolean hasAuth = false; + + for (SubkeyChange change : saveParcel.getChangeSubKeys()) { + if (change.getMoveKeyToSecurityToken()) { + // Pending moveKeyToSecurityToken operation. Need to make sure that we don't have multiple + // subkeys pending for the same slot. + CanonicalizedSecretKey wsK = wsKR.getSecretKey(change.getSubKeyId()); + + if ((wsK.canSign() || wsK.canCertify())) { + if (hasSign) { + log.add(LogType.MSG_MF_ERROR_DUPLICATE_KEYTOCARD_FOR_SLOT, indent + 1); + return false; + } else { + hasSign = true; + } + } else if ((wsK.canEncrypt())) { + if (hasEncrypt) { + log.add(LogType.MSG_MF_ERROR_DUPLICATE_KEYTOCARD_FOR_SLOT, indent + 1); + return false; + } else { + hasEncrypt = true; + } + } else if ((wsK.canAuthenticate())) { + if (hasAuth) { + log.add(LogType.MSG_MF_ERROR_DUPLICATE_KEYTOCARD_FOR_SLOT, indent + 1); + return false; + } else { + hasAuth = true; + } + } else { + log.add(LogType.MSG_MF_ERROR_INVALID_FLAGS_FOR_KEYTOCARD, indent + 1); + return false; + } + } + } + return true; + } + private PgpEditKeyResult internal(PGPSecretKeyRing sKR, PGPSecretKey masterSecretKey, int masterKeyFlags, long masterKeyExpiry, CryptoInputParcel cryptoInput, @@ -549,10 +573,11 @@ public class PgpKeyOperation { // 2a. Add certificates for new user ids subProgressPush(15, 23); - for (int i = 0; i < saveParcel.mAddUserIds.size(); i++) { + String changePrimaryUserId = saveParcel.getChangePrimaryUserId(); + for (int i = 0; i < saveParcel.getAddUserIds().size(); i++) { - progress(R.string.progress_modify_adduid, (i - 1) * (100 / saveParcel.mAddUserIds.size())); - String userId = saveParcel.mAddUserIds.get(i); + progress(R.string.progress_modify_adduid, (i - 1) * (100 / saveParcel.getAddUserIds().size())); + String userId = saveParcel.getAddUserIds().get(i); log.add(LogType.MSG_MF_UID_ADD, indent, userId); if ("".equals(userId)) { @@ -583,8 +608,8 @@ public class PgpKeyOperation { } // if it's supposed to be primary, we can do that here as well - boolean isPrimary = saveParcel.mChangePrimaryUserId != null - && userId.equals(saveParcel.mChangePrimaryUserId); + boolean isPrimary = changePrimaryUserId != null + && userId.equals(changePrimaryUserId); // generate and add new certificate try { PGPSignature cert = generateUserIdSignature( @@ -601,10 +626,10 @@ public class PgpKeyOperation { // 2b. Add certificates for new user ids subProgressPush(23, 32); - for (int i = 0; i < saveParcel.mAddUserAttribute.size(); i++) { - - progress(R.string.progress_modify_adduat, (i - 1) * (100 / saveParcel.mAddUserAttribute.size())); - WrappedUserAttribute attribute = saveParcel.mAddUserAttribute.get(i); + List addUserAttributes = saveParcel.getAddUserAttribute(); + for (int i = 0; i < addUserAttributes.size(); i++) { + progress(R.string.progress_modify_adduat, (i - 1) * (100 / addUserAttributes.size())); + WrappedUserAttribute attribute = addUserAttributes.get(i); switch (attribute.getType()) { // the 'none' type must not succeed @@ -637,10 +662,10 @@ public class PgpKeyOperation { // 2c. Add revocations for revoked user ids subProgressPush(32, 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); + List revokeUserIds = saveParcel.getRevokeUserIds(); + for (int i = 0, j = revokeUserIds.size(); i < j; i++) { + progress(R.string.progress_modify_revokeuid, (i - 1) * (100 / revokeUserIds.size())); + String userId = revokeUserIds.get(i); log.add(LogType.MSG_MF_UID_REVOKE, indent, userId); // Make sure the user id exists (yes these are 10 LoC in Java!) @@ -672,12 +697,12 @@ public class PgpKeyOperation { subProgressPop(); // 3. If primary user id changed, generate new certificates for both old and new - if (saveParcel.mChangePrimaryUserId != null) { + if (changePrimaryUserId != null) { progress(R.string.progress_modify_primaryuid, 40); // keep track if we actually changed one boolean ok = false; - log.add(LogType.MSG_MF_UID_PRIMARY, indent, saveParcel.mChangePrimaryUserId); + log.add(LogType.MSG_MF_UID_PRIMARY, indent, changePrimaryUserId); indent += 1; // we work on the modifiedPublicKey here, to respect new or newly revoked uids @@ -718,7 +743,7 @@ public class PgpKeyOperation { // 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)) { + if (userId.equals(changePrimaryUserId)) { log.add(LogType.MSG_MF_ERROR_REVOKED_PRIMARY, indent); return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null); } @@ -729,7 +754,7 @@ public class PgpKeyOperation { if (currentCert.getHashedSubPackets() != null && currentCert.getHashedSubPackets().isPrimaryUserID()) { // if it's the one we want, just leave it as is - if (userId.equals(saveParcel.mChangePrimaryUserId)) { + if (userId.equals(changePrimaryUserId)) { ok = true; continue; } @@ -755,7 +780,7 @@ public class PgpKeyOperation { // if we are here, this is not currently a primary user id // if it should be - if (userId.equals(saveParcel.mChangePrimaryUserId)) { + if (userId.equals(changePrimaryUserId)) { // add shiny new primary user id certificate log.add(LogType.MSG_MF_PRIMARY_NEW, indent); modifiedPublicKey = PGPPublicKey.removeCertification( @@ -803,10 +828,11 @@ public class PgpKeyOperation { // 4a. For each subkey change, generate new subkey binding certificate subProgressPush(50, 60); - for (int i = 0; i < saveParcel.mChangeSubKeys.size(); i++) { + List changeSubKeys = saveParcel.getChangeSubKeys(); + for (int i = 0, j = changeSubKeys.size(); i < j; i++) { - progress(R.string.progress_modify_subkeychange, (i-1) * (100 / saveParcel.mChangeSubKeys.size())); - SaveKeyringParcel.SubkeyChange change = saveParcel.mChangeSubKeys.get(i); + progress(R.string.progress_modify_subkeychange, (i-1) * (100 / changeSubKeys.size())); + SaveKeyringParcel.SubkeyChange change = changeSubKeys.get(i); log.add(LogType.MSG_MF_SUBKEY_CHANGE, indent, KeyFormattingUtils.convertKeyIdToHex(change.getSubKeyId())); @@ -944,10 +970,10 @@ public class PgpKeyOperation { // 4b. For each subkey revocation, generate new subkey revocation certificate subProgressPush(60, 65); - for (int i = 0; i < saveParcel.mRevokeSubKeys.size(); i++) { - - progress(R.string.progress_modify_subkeyrevoke, (i-1) * (100 / saveParcel.mRevokeSubKeys.size())); - long revocation = saveParcel.mRevokeSubKeys.get(i); + List revokeSubKeys = saveParcel.getRevokeSubKeys(); + for (int i = 0, j = revokeSubKeys.size(); i < j; i++) { + progress(R.string.progress_modify_subkeyrevoke, (i-1) * (100 / revokeSubKeys.size())); + long revocation = revokeSubKeys.get(i); log.add(LogType.MSG_MF_SUBKEY_REVOKE, indent, KeyFormattingUtils.convertKeyIdToHex(revocation)); @@ -976,16 +1002,16 @@ public class PgpKeyOperation { // 5. Generate and add new subkeys subProgressPush(70, 90); - for (int i = 0; i < saveParcel.mAddSubKeys.size(); i++) { - + List addSubKeys = saveParcel.getAddSubKeys(); + for (int i = 0, j = addSubKeys.size(); i < j; i++) { // Check if we were cancelled - again. This operation is expensive so we do it each loop. if (checkCancelled()) { log.add(LogType.MSG_OPERATION_CANCELLED, indent); return new PgpEditKeyResult(PgpEditKeyResult.RESULT_CANCELLED, log, null); } - progress(R.string.progress_modify_subkeyadd, (i-1) * (100 / saveParcel.mAddSubKeys.size())); - SaveKeyringParcel.SubkeyAdd add = saveParcel.mAddSubKeys.get(i); + progress(R.string.progress_modify_subkeyadd, (i-1) * (100 / addSubKeys.size())); + SaveKeyringParcel.SubkeyAdd add = addSubKeys.get(i); log.add(LogType.MSG_MF_SUBKEY_NEW, indent, KeyFormattingUtils.getAlgorithmInfo(add.getAlgorithm(), add.getKeySize(), add.getCurve()) ); @@ -1006,8 +1032,8 @@ public class PgpKeyOperation { // generate a new secret key (privkey only for now) subProgressPush( - (i-1) * (100 / saveParcel.mAddSubKeys.size()), - i * (100 / saveParcel.mAddSubKeys.size()) + (i-1) * (100 / addSubKeys.size()), + i * (100 / addSubKeys.size()) ); PGPKeyPair keyPair = createKey(add, cryptoInput.getSignatureTime(), log, indent); subProgressPop(); @@ -1060,13 +1086,13 @@ public class PgpKeyOperation { } // 6. If requested, change passphrase - if (saveParcel.getChangeUnlockParcel() != null) { + if (saveParcel.getNewUnlock() != null) { progress(R.string.progress_modify_passphrase, 90); log.add(LogType.MSG_MF_PASSPHRASE, indent); indent += 1; sKR = applyNewPassphrase(sKR, masterPublicKey, cryptoInput.getPassphrase(), - saveParcel.getChangeUnlockParcel().getNewPassphrase(), log, indent); + saveParcel.getNewUnlock().getNewPassphrase(), log, indent); if (sKR == null) { // The error has been logged above, just return a bad state return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null); @@ -1076,21 +1102,21 @@ public class PgpKeyOperation { } // 7. if requested, change PIN and/or Admin PIN on security token - if (saveParcel.mSecurityTokenPin != null) { + if (saveParcel.getSecurityTokenPin() != null) { progress(R.string.progress_modify_pin, 90); log.add(LogType.MSG_MF_PIN, indent); indent += 1; - nfcKeyToCardOps.setPin(saveParcel.mSecurityTokenPin); + nfcKeyToCardOps.setPin(saveParcel.getSecurityTokenPin()); indent -= 1; } - if (saveParcel.mSecurityTokenAdminPin != null) { + if (saveParcel.getSecurityTokenAdminPin() != null) { progress(R.string.progress_modify_admin_pin, 90); log.add(LogType.MSG_MF_ADMIN_PIN, indent); indent += 1; - nfcKeyToCardOps.setAdminPin(saveParcel.mSecurityTokenAdminPin); + nfcKeyToCardOps.setAdminPin(saveParcel.getSecurityTokenAdminPin()); indent -= 1; } @@ -1141,7 +1167,7 @@ public class PgpKeyOperation { progress(R.string.progress_modify, 0); // Make sure the saveParcel includes only operations available without passphrase! - if (!saveParcel.isRestrictedOnly()) { + if (!isParcelRestrictedOnly(saveParcel)) { log.add(LogType.MSG_MF_ERROR_RESTRICTED, indent); return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null); } @@ -1155,10 +1181,10 @@ public class PgpKeyOperation { // 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); + List changeSubKeys = saveParcel.getChangeSubKeys(); + for (int i = 0, j = changeSubKeys.size(); i < j; i++) { + progress(R.string.progress_modify_subkeychange, (i - 1) * (100 / changeSubKeys.size())); + SaveKeyringParcel.SubkeyChange change = changeSubKeys.get(i); log.add(LogType.MSG_MF_SUBKEY_CHANGE, indent, KeyFormattingUtils.convertKeyIdToHex(change.getSubKeyId())); @@ -1700,4 +1726,31 @@ public class PgpKeyOperation { return true; } + + /** Returns true iff this parcel does not contain any operations which require a passphrase. */ + private static boolean isParcelRestrictedOnly(SaveKeyringParcel saveKeyringParcel) { + if (saveKeyringParcel.getNewUnlock() != null + || !saveKeyringParcel.getAddUserIds().isEmpty() + || !saveKeyringParcel.getAddUserAttribute().isEmpty() + || !saveKeyringParcel.getAddSubKeys().isEmpty() + || saveKeyringParcel.getChangePrimaryUserId() != null + || !saveKeyringParcel.getRevokeUserIds().isEmpty() + || !saveKeyringParcel.getRevokeSubKeys().isEmpty()) { + return false; + } + + for (SubkeyChange change : saveKeyringParcel.getChangeSubKeys()) { + if (change.getRecertify() || change.getFlags() != null || change.getExpiry() != null + || change.getMoveKeyToSecurityToken()) { + return false; + } + } + + return true; + } + + private static boolean isParcelEmpty(SaveKeyringParcel saveKeyringParcel) { + return isParcelRestrictedOnly(saveKeyringParcel) && saveKeyringParcel.getChangeSubKeys().isEmpty(); + } + } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/ECKeyFormat.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/ECKeyFormat.java index 744de6e3d..278b510b3 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/ECKeyFormat.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/ECKeyFormat.java @@ -86,7 +86,7 @@ public class ECKeyFormat extends KeyFormat { } } - public void addToSaveKeyringParcel(SaveKeyringParcel keyring, int keyFlags) { + public void addToSaveKeyringParcel(SaveKeyringParcel.Builder builder, int keyFlags) { final X9ECParameters params = NISTNamedCurves.getByOID(mECCurveOID); final ECCurve curve = params.getCurve(); @@ -107,7 +107,6 @@ public class ECKeyFormat extends KeyFormat { throw new IllegalArgumentException("Unsupported curve " + mECCurveOID); } - keyring.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd(algo, - curve.getFieldSize(), scurve, keyFlags, 0L)); + builder.addSubkeyAdd(SubkeyAdd.createSubkeyAdd(algo, curve.getFieldSize(), scurve, keyFlags, 0L)); } } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/KeyFormat.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/KeyFormat.java index 6fdd91daa..9d612517b 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/KeyFormat.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/KeyFormat.java @@ -94,6 +94,6 @@ public abstract class KeyFormat { throw new IllegalArgumentException("Unsupported Algorithm id " + t); } - public abstract void addToSaveKeyringParcel(SaveKeyringParcel keyring, int keyFlags); + public abstract void addToSaveKeyringParcel(SaveKeyringParcel.Builder builder, int keyFlags); } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/RSAKeyFormat.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/RSAKeyFormat.java index f1d5d9e74..cbba61781 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/RSAKeyFormat.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/RSAKeyFormat.java @@ -86,8 +86,8 @@ public class RSAKeyFormat extends KeyFormat { } } - public void addToSaveKeyringParcel(SaveKeyringParcel keyring, int keyFlags) { - keyring.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd(SaveKeyringParcel.Algorithm.RSA, + public void addToSaveKeyringParcel(SaveKeyringParcel.Builder builder, int keyFlags) { + builder.addSubkeyAdd(SubkeyAdd.createSubkeyAdd(SaveKeyringParcel.Algorithm.RSA, mModulusLength, null, keyFlags, 0L)); } } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/SaveKeyringParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/SaveKeyringParcel.java index 6ff7f5c75..2f667f7a3 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/SaveKeyringParcel.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/SaveKeyringParcel.java @@ -18,18 +18,20 @@ package org.sufficientlysecure.keychain.service; -import android.os.Parcel; + +import java.io.Serializable; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + import android.os.Parcelable; import android.support.annotation.Nullable; import com.google.auto.value.AutoValue; -import org.sufficientlysecure.keychain.pgp.WrappedUserAttribute; import org.sufficientlysecure.keychain.keyimport.ParcelableHkpKeyserver; +import org.sufficientlysecure.keychain.pgp.WrappedUserAttribute; import org.sufficientlysecure.keychain.util.Passphrase; -import java.io.Serializable; -import java.util.ArrayList; - /** * This class is a a transferable representation for a collection of changes * to be done on a keyring. @@ -45,107 +47,183 @@ import java.util.ArrayList; * error in any included operation (for example revocation of a non-existent * subkey) will cause the operation as a whole to fail. */ -public class SaveKeyringParcel implements Parcelable { +@AutoValue +public abstract class SaveKeyringParcel implements Parcelable { // the master key id to be edited. if this is null, a new one will be created - public Long mMasterKeyId; + @Nullable + public abstract Long getMasterKeyId(); // the key fingerprint, for safety. MUST be null for a new key. - public byte[] mFingerprint; + @Nullable + public abstract byte[] getFingerprint(); - public ArrayList mAddUserIds; - public ArrayList mAddUserAttribute; - public ArrayList mAddSubKeys; + public abstract List getAddUserIds(); + public abstract List getAddUserAttribute(); + public abstract List getAddSubKeys(); - public ArrayList mChangeSubKeys; - public String mChangePrimaryUserId; + public abstract List getChangeSubKeys(); + @Nullable + public abstract String getChangePrimaryUserId(); - public ArrayList mRevokeUserIds; - public ArrayList mRevokeSubKeys; + public abstract List getRevokeUserIds(); + public abstract List getRevokeSubKeys(); // if these are non-null, PINs will be changed on the token - public Passphrase mSecurityTokenPin; - public Passphrase mSecurityTokenAdminPin; + @Nullable + public abstract Passphrase getSecurityTokenPin(); + @Nullable + public abstract Passphrase getSecurityTokenAdminPin(); - // private because they have to be set together with setUpdateOptions - private boolean mUpload; - private boolean mUploadAtomic; - private ParcelableHkpKeyserver mKeyserver; + public abstract boolean isShouldUpload(); + public abstract boolean isShouldUploadAtomic(); + @Nullable + public abstract ParcelableHkpKeyserver getUploadKeyserver(); - // private because we have to set other details like key id - private ChangeUnlockParcel mNewUnlock; + @Nullable + public abstract ChangeUnlockParcel getNewUnlock(); - public SaveKeyringParcel() { - reset(); + public static Builder buildNewKeyringParcel() { + return new AutoValue_SaveKeyringParcel.Builder() + .setShouldUpload(false) + .setShouldUploadAtomic(false); } - public SaveKeyringParcel(long masterKeyId, byte[] fingerprint) { - this(); - mMasterKeyId = masterKeyId; - mFingerprint = fingerprint; + public static Builder buildChangeKeyringParcel(long masterKeyId, byte[] fingerprint) { + return buildNewKeyringParcel() + .setMasterKeyId(masterKeyId) + .setFingerprint(fingerprint); } - public void reset() { - mNewUnlock = null; - mAddUserIds = new ArrayList<>(); - mAddUserAttribute = new ArrayList<>(); - mAddSubKeys = new ArrayList<>(); - mChangePrimaryUserId = null; - mChangeSubKeys = new ArrayList<>(); - mRevokeUserIds = new ArrayList<>(); - mRevokeSubKeys = new ArrayList<>(); - mSecurityTokenPin = null; - mSecurityTokenAdminPin = null; - mUpload = false; - mUploadAtomic = false; - mKeyserver = null; + abstract Builder toBuilder(); + + public static Builder buildUpon(SaveKeyringParcel saveKeyringParcel) { + SaveKeyringParcel.Builder builder = saveKeyringParcel.toBuilder(); + builder.addUserIds.addAll(saveKeyringParcel.getAddUserIds()); + builder.revokeUserIds.addAll(saveKeyringParcel.getRevokeUserIds()); + builder.addUserAttribute.addAll(saveKeyringParcel.getAddUserAttribute()); + builder.addSubKeys.addAll(saveKeyringParcel.getAddSubKeys()); + builder.changeSubKeys.addAll(saveKeyringParcel.getChangeSubKeys()); + builder.revokeSubKeys.addAll(saveKeyringParcel.getRevokeSubKeys()); + return builder; } - public void setUpdateOptions(boolean upload, boolean uploadAtomic, ParcelableHkpKeyserver keyserver) { - mUpload = upload; - mUploadAtomic = uploadAtomic; - mKeyserver = keyserver; - } + @AutoValue.Builder + public static abstract class Builder { + private ArrayList addUserIds = new ArrayList<>(); + private ArrayList revokeUserIds = new ArrayList<>(); + private ArrayList addUserAttribute = new ArrayList<>(); + private ArrayList addSubKeys = new ArrayList<>(); + private ArrayList changeSubKeys = new ArrayList<>(); + private ArrayList revokeSubKeys = new ArrayList<>(); - public void setNewUnlock(ChangeUnlockParcel parcel) { - mNewUnlock = parcel; - } - public ChangeUnlockParcel getChangeUnlockParcel() { - return mNewUnlock; - } + public abstract Builder setChangePrimaryUserId(String changePrimaryUserId); + public abstract Builder setSecurityTokenPin(Passphrase securityTokenPin); + public abstract Builder setSecurityTokenAdminPin(Passphrase securityTokenAdminPin); + public abstract Builder setNewUnlock(ChangeUnlockParcel newUnlock); - public boolean isUpload() { - return mUpload; - } + public abstract Long getMasterKeyId(); + public abstract byte[] getFingerprint(); + public abstract String getChangePrimaryUserId(); - public boolean isUploadAtomic() { - return mUploadAtomic; - } - public ParcelableHkpKeyserver getUploadKeyserver() { - return mKeyserver; - } - - public boolean isEmpty() { - return isRestrictedOnly() && mChangeSubKeys.isEmpty(); - } - - /** 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 || !mRevokeUserIds.isEmpty() - || !mRevokeSubKeys.isEmpty()) { - return false; + public ArrayList getMutableAddSubKeys() { + return addSubKeys; + } + public ArrayList getMutableAddUserIds() { + return addUserIds; + } + public ArrayList getMutableRevokeSubKeys() { + return revokeSubKeys; + } + public ArrayList getMutableRevokeUserIds() { + return revokeUserIds; } - for (SubkeyChange change : mChangeSubKeys) { - if (change.getRecertify() || change.getFlags() != null || change.getExpiry() != null - || change.getMoveKeyToSecurityToken()) { - return false; + abstract Builder setMasterKeyId(Long masterKeyId); + abstract Builder setFingerprint(byte[] fingerprint); + abstract Builder setAddUserIds(List addUserIds); + abstract Builder setAddUserAttribute(List addUserAttribute); + abstract Builder setAddSubKeys(List addSubKeys); + abstract Builder setChangeSubKeys(List changeSubKeys); + abstract Builder setRevokeUserIds(List revokeUserIds); + abstract Builder setRevokeSubKeys(List revokeSubKeys); + abstract Builder setShouldUpload(boolean upload); + abstract Builder setShouldUploadAtomic(boolean uploadAtomic); + abstract Builder setUploadKeyserver(ParcelableHkpKeyserver keyserver); + + public void setUpdateOptions(boolean upload, boolean uploadAtomic, ParcelableHkpKeyserver keyserver) { + setShouldUpload(upload); + setShouldUploadAtomic(uploadAtomic); + setUploadKeyserver(keyserver); + } + + public void addSubkeyAdd(SubkeyAdd subkeyAdd) { + addSubKeys.add(subkeyAdd); + } + + public void addUserId(String userId) { + addUserIds.add(userId); + } + + public void addRevokeSubkey(long masterKeyId) { + revokeSubKeys.add(masterKeyId); + } + + public void removeRevokeSubkey(long keyId) { + revokeSubKeys.remove(keyId); + } + + public void addRevokeUserId(String userId) { + revokeUserIds.add(userId); + } + + public void removeRevokeUserId(String userId) { + revokeUserIds.remove(userId); + } + + public void addOrReplaceSubkeyChange(SubkeyChange newChange) { + SubkeyChange foundSubkeyChange = getSubkeyChange(newChange.getSubKeyId()); + + if (foundSubkeyChange != null) { + changeSubKeys.remove(foundSubkeyChange); } + changeSubKeys.add(newChange); } - return true; + public void removeSubkeyChange(SubkeyChange change) { + changeSubKeys.remove(change); + } + + public SubkeyChange getSubkeyChange(long keyId) { + if (changeSubKeys == null) { + return null; + } + + for (SubkeyChange subkeyChange : changeSubKeys) { + if (subkeyChange.getSubKeyId() == keyId) { + return subkeyChange; + } + } + return null; + } + + public void addUserAttribute(WrappedUserAttribute ua) { + addUserAttribute.add(ua); + } + + abstract SaveKeyringParcel autoBuild(); + + public SaveKeyringParcel build() { + setAddUserAttribute(Collections.unmodifiableList(addUserAttribute)); + setRevokeSubKeys(Collections.unmodifiableList(revokeSubKeys)); + setRevokeUserIds(Collections.unmodifiableList(revokeUserIds)); + setAddSubKeys(Collections.unmodifiableList(addSubKeys)); + setAddUserIds(Collections.unmodifiableList(addUserIds)); + setChangeSubKeys(Collections.unmodifiableList(changeSubKeys)); + + return autoBuild(); + } } // performance gain for using Parcelable here would probably be negligible, @@ -208,114 +286,6 @@ public class SaveKeyringParcel implements Parcelable { } } - public SubkeyChange getSubkeyChange(long keyId) { - for (SubkeyChange subkeyChange : mChangeSubKeys) { - if (subkeyChange.getSubKeyId() == keyId) { - return subkeyChange; - } - } - return null; - } - - public void addOrReplaceSubkeyChange(SubkeyChange change) { - SubkeyChange foundSubkeyChange = getSubkeyChange(change.getSubKeyId()); - if (foundSubkeyChange != null) { - mChangeSubKeys.remove(foundSubkeyChange); - } - - mChangeSubKeys.add(change); - } - - public void removeSubkeyChange(SubkeyChange change) { - mChangeSubKeys.remove(change); - } - - @SuppressWarnings("unchecked") // we verify the reads against writes in writeToParcel - public SaveKeyringParcel(Parcel source) { - mMasterKeyId = source.readInt() != 0 ? source.readLong() : null; - mFingerprint = source.createByteArray(); - - mNewUnlock = source.readParcelable(getClass().getClassLoader()); - - mAddUserIds = source.createStringArrayList(); - mAddUserAttribute = (ArrayList) source.readSerializable(); - mAddSubKeys = (ArrayList) source.readSerializable(); - - mChangeSubKeys = (ArrayList) source.readSerializable(); - mChangePrimaryUserId = source.readString(); - - mRevokeUserIds = source.createStringArrayList(); - mRevokeSubKeys = (ArrayList) source.readSerializable(); - - mSecurityTokenPin = source.readParcelable(Passphrase.class.getClassLoader()); - mSecurityTokenAdminPin = source.readParcelable(Passphrase.class.getClassLoader()); - - mUpload = source.readByte() != 0; - mUploadAtomic = source.readByte() != 0; - mKeyserver = source.readParcelable(ParcelableHkpKeyserver.class.getClassLoader()); - } - - @Override - public void writeToParcel(Parcel destination, int flags) { - destination.writeInt(mMasterKeyId == null ? 0 : 1); - if (mMasterKeyId != null) { - destination.writeLong(mMasterKeyId); - } - destination.writeByteArray(mFingerprint); - - // yes, null values are ok for parcelables - destination.writeParcelable(mNewUnlock, flags); - - destination.writeStringList(mAddUserIds); - destination.writeSerializable(mAddUserAttribute); - destination.writeSerializable(mAddSubKeys); - - destination.writeSerializable(mChangeSubKeys); - destination.writeString(mChangePrimaryUserId); - - destination.writeStringList(mRevokeUserIds); - destination.writeSerializable(mRevokeSubKeys); - - destination.writeParcelable(mSecurityTokenPin, flags); - destination.writeParcelable(mSecurityTokenAdminPin, flags); - - destination.writeByte((byte) (mUpload ? 1 : 0)); - destination.writeByte((byte) (mUploadAtomic ? 1 : 0)); - destination.writeParcelable(mKeyserver, flags); - } - - public static final Creator CREATOR = new Creator() { - public SaveKeyringParcel createFromParcel(final Parcel source) { - return new SaveKeyringParcel(source); - } - - public SaveKeyringParcel[] newArray(final int size) { - return new SaveKeyringParcel[size]; - } - }; - - @Override - public int describeContents() { - return 0; - } - - @Override - public String toString() { - String out = "mMasterKeyId: " + mMasterKeyId + "\n"; - out += "mNewUnlock: " + mNewUnlock + "\n"; - out += "mAddUserIds: " + mAddUserIds + "\n"; - out += "mAddUserAttribute: " + mAddUserAttribute + "\n"; - out += "mAddSubKeys: " + mAddSubKeys + "\n"; - out += "mChangeSubKeys: " + mChangeSubKeys + "\n"; - out += "mChangePrimaryUserId: " + mChangePrimaryUserId + "\n"; - out += "mRevokeUserIds: " + mRevokeUserIds + "\n"; - out += "mRevokeSubKeys: " + mRevokeSubKeys + "\n"; - out += "mSecurityTokenPin: " + mSecurityTokenPin + "\n"; - out += "mSecurityTokenAdminPin: " + mSecurityTokenAdminPin; - - return out; - } - // All supported algorithms public enum Algorithm { RSA, DSA, ELGAMAL, ECDSA, ECDH @@ -330,7 +300,4 @@ public class SaveKeyringParcel implements Parcelable { // (adding support would be trivial though -> JcaPGPKeyConverter.java:190) // BRAINPOOL_P256, BRAINPOOL_P384, BRAINPOOL_P512 } - - - } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyFinalFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyFinalFragment.java index 26db42c63..bbf4a906a 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyFinalFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyFinalFragment.java @@ -295,7 +295,7 @@ public class CreateKeyFinalFragment extends Fragment { } private static SaveKeyringParcel createDefaultSaveKeyringParcel(CreateKeyActivity createKeyActivity) { - SaveKeyringParcel saveKeyringParcel = new SaveKeyringParcel(); + SaveKeyringParcel.Builder builder = SaveKeyringParcel.buildNewKeyringParcel(); if (createKeyActivity.mCreateSecurityToken) { if (createKeyActivity.mSecurityTokenSign == null) { @@ -303,38 +303,40 @@ public class CreateKeyFinalFragment extends Fragment { createKeyActivity.mSecurityTokenDec = Constants.SECURITY_TOKEN_V2_DEC; createKeyActivity.mSecurityTokenAuth = Constants.SECURITY_TOKEN_V2_AUTH; } - createKeyActivity.mSecurityTokenSign.addToSaveKeyringParcel(saveKeyringParcel, KeyFlags.SIGN_DATA | KeyFlags.CERTIFY_OTHER); - createKeyActivity.mSecurityTokenDec.addToSaveKeyringParcel(saveKeyringParcel, KeyFlags.ENCRYPT_COMMS | KeyFlags.ENCRYPT_STORAGE); - createKeyActivity.mSecurityTokenAuth.addToSaveKeyringParcel(saveKeyringParcel, KeyFlags.AUTHENTICATION); + createKeyActivity.mSecurityTokenSign.addToSaveKeyringParcel( + builder, KeyFlags.SIGN_DATA | KeyFlags.CERTIFY_OTHER); + createKeyActivity.mSecurityTokenDec.addToSaveKeyringParcel( + builder, KeyFlags.ENCRYPT_COMMS | KeyFlags.ENCRYPT_STORAGE); + createKeyActivity.mSecurityTokenAuth.addToSaveKeyringParcel(builder, KeyFlags.AUTHENTICATION); // use empty passphrase - saveKeyringParcel.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(new Passphrase())); + builder.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(new Passphrase())); } else { - Constants.addDefaultSubkeys(saveKeyringParcel); + Constants.addDefaultSubkeys(builder); if (createKeyActivity.mPassphrase != null) { - saveKeyringParcel.setNewUnlock( + builder.setNewUnlock( ChangeUnlockParcel.createUnLockParcelForNewKey(createKeyActivity.mPassphrase)); } else { - saveKeyringParcel.setNewUnlock(null); + builder.setNewUnlock(null); } } String userId = KeyRing.createUserId( new OpenPgpUtils.UserId(createKeyActivity.mName, createKeyActivity.mEmail, null) ); - saveKeyringParcel.mAddUserIds.add(userId); - saveKeyringParcel.mChangePrimaryUserId = userId; + builder.addUserId(userId); + builder.setChangePrimaryUserId(userId); if (createKeyActivity.mAdditionalEmails != null && createKeyActivity.mAdditionalEmails.size() > 0) { for (String email : createKeyActivity.mAdditionalEmails) { String thisUserId = KeyRing.createUserId( new OpenPgpUtils.UserId(createKeyActivity.mName, email, null) ); - saveKeyringParcel.mAddUserIds.add(thisUserId); + builder.addUserId(thisUserId); } } - return saveKeyringParcel; + return builder.build(); } private void checkEmailValidity() { @@ -427,11 +429,11 @@ public class CreateKeyFinalFragment extends Fragment { private void moveToCard(final EditKeyResult saveKeyResult) { CreateKeyActivity activity = (CreateKeyActivity) getActivity(); - final SaveKeyringParcel changeKeyringParcel; + SaveKeyringParcel.Builder builder; CachedPublicKeyRing key = (KeyRepository.createDatabaseInteractor(getContext())) .getCachedPublicKeyRing(saveKeyResult.mMasterKeyId); try { - changeKeyringParcel = new SaveKeyringParcel(key.getMasterKeyId(), key.getFingerprint()); + builder = SaveKeyringParcel.buildChangeKeyringParcel(key.getMasterKeyId(), key.getFingerprint()); } catch (PgpKeyNotFoundException e) { Log.e(Constants.TAG, "Key that should be moved to Security Token not found in database!"); return; @@ -439,13 +441,13 @@ public class CreateKeyFinalFragment extends Fragment { // define subkeys that should be moved to the card Cursor cursor = activity.getContentResolver().query( - KeychainContract.Keys.buildKeysUri(changeKeyringParcel.mMasterKeyId), + KeychainContract.Keys.buildKeysUri(builder.getMasterKeyId()), new String[]{KeychainContract.Keys.KEY_ID,}, null, null, null ); try { while (cursor != null && cursor.moveToNext()) { long subkeyId = cursor.getLong(0); - changeKeyringParcel.mChangeSubKeys.add(SubkeyChange.createMoveToSecurityTokenChange(subkeyId)); + builder.addOrReplaceSubkeyChange(SubkeyChange.createMoveToSecurityTokenChange(subkeyId)); } } finally { if (cursor != null) { @@ -454,15 +456,17 @@ public class CreateKeyFinalFragment extends Fragment { } // define new PIN and Admin PIN for the card - changeKeyringParcel.mSecurityTokenPin = activity.mSecurityTokenPin; - changeKeyringParcel.mSecurityTokenAdminPin = activity.mSecurityTokenAdminPin; + builder.setSecurityTokenPin(activity.mSecurityTokenPin); + builder.setSecurityTokenAdminPin(activity.mSecurityTokenAdminPin); + + final SaveKeyringParcel saveKeyringParcel = builder.build(); CryptoOperationHelper.Callback callback = new CryptoOperationHelper.Callback() { @Override public SaveKeyringParcel createOperationInput() { - return changeKeyringParcel; + return saveKeyringParcel; } @Override diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditIdentitiesFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditIdentitiesFragment.java index e7944f41a..ee3b9fa27 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditIdentitiesFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditIdentitiesFragment.java @@ -82,7 +82,7 @@ public class EditIdentitiesFragment extends Fragment private Uri mDataUri; - private SaveKeyringParcel mSaveKeyringParcel; + private SaveKeyringParcel.Builder mSkpBuilder; private CryptoOperationHelper mEditOpHelper; private CryptoOperationHelper mUploadOpHelper; @@ -180,7 +180,7 @@ public class EditIdentitiesFragment extends Fragment return; } - mSaveKeyringParcel = new SaveKeyringParcel(masterKeyId, keyRing.getFingerprint()); + mSkpBuilder = SaveKeyringParcel.buildChangeKeyringParcel(masterKeyId, keyRing.getFingerprint()); mPrimaryUserId = keyRing.getPrimaryUserIdWithFallback(); } catch (PgpKeyNotFoundException | NotFoundException e) { @@ -193,11 +193,11 @@ public class EditIdentitiesFragment extends Fragment getLoaderManager().initLoader(LOADER_ID_USER_IDS, null, EditIdentitiesFragment.this); mUserIdsAdapter = new UserIdsAdapter(getActivity(), null, 0); - mUserIdsAdapter.setEditMode(mSaveKeyringParcel); + mUserIdsAdapter.setEditMode(mSkpBuilder); mUserIdsList.setAdapter(mUserIdsAdapter); // TODO: SaveParcel from savedInstance?! - mUserIdsAddedAdapter = new UserIdsAddedAdapter(getActivity(), mSaveKeyringParcel.mAddUserIds, false); + mUserIdsAddedAdapter = new UserIdsAddedAdapter(getActivity(), mSkpBuilder.getMutableAddUserIds(), false); mUserIdsAddedList.setAdapter(mUserIdsAddedAdapter); } @@ -266,23 +266,23 @@ public class EditIdentitiesFragment extends Fragment switch (message.what) { case EditUserIdDialogFragment.MESSAGE_CHANGE_PRIMARY_USER_ID: // toggle - if (mSaveKeyringParcel.mChangePrimaryUserId != null - && mSaveKeyringParcel.mChangePrimaryUserId.equals(userId)) { - mSaveKeyringParcel.mChangePrimaryUserId = null; + if (mSkpBuilder.getChangePrimaryUserId() != null + && mSkpBuilder.getChangePrimaryUserId().equals(userId)) { + mSkpBuilder.setChangePrimaryUserId(null); } else { - mSaveKeyringParcel.mChangePrimaryUserId = userId; + mSkpBuilder.setChangePrimaryUserId(userId); } break; case EditUserIdDialogFragment.MESSAGE_REVOKE: // toggle - if (mSaveKeyringParcel.mRevokeUserIds.contains(userId)) { - mSaveKeyringParcel.mRevokeUserIds.remove(userId); + if (mSkpBuilder.getMutableRevokeUserIds().contains(userId)) { + mSkpBuilder.removeRevokeUserId(userId); } else { - mSaveKeyringParcel.mRevokeUserIds.add(userId); + mSkpBuilder.addRevokeUserId(userId); // not possible to revoke and change to primary user id - if (mSaveKeyringParcel.mChangePrimaryUserId != null - && mSaveKeyringParcel.mChangePrimaryUserId.equals(userId)) { - mSaveKeyringParcel.mChangePrimaryUserId = null; + if (mSkpBuilder.getChangePrimaryUserId() != null + && mSkpBuilder.getChangePrimaryUserId().equals(userId)) { + mSkpBuilder.setChangePrimaryUserId(null); } } break; @@ -339,7 +339,7 @@ public class EditIdentitiesFragment extends Fragment = new CryptoOperationHelper.Callback() { @Override public SaveKeyringParcel createOperationInput() { - return mSaveKeyringParcel; + return mSkpBuilder.build(); } @Override diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java index 65ef848d9..50e6b617f 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java @@ -100,7 +100,7 @@ public class EditKeyFragment extends QueueingCryptoOperationFragment 0; // for edit key - if (mSaveKeyringParcel != null) { - boolean changeAnyPrimaryUserId = (mSaveKeyringParcel.mChangePrimaryUserId != null); - boolean changeThisPrimaryUserId = (mSaveKeyringParcel.mChangePrimaryUserId != null - && mSaveKeyringParcel.mChangePrimaryUserId.equals(userId)); - boolean revokeThisUserId = (mSaveKeyringParcel.mRevokeUserIds.contains(userId)); + if (mSkpBuilder != null) { + String changePrimaryUserId = mSkpBuilder.getChangePrimaryUserId(); + boolean changeAnyPrimaryUserId = (changePrimaryUserId != null); + boolean changeThisPrimaryUserId = (changeAnyPrimaryUserId && changePrimaryUserId.equals(userId)); + boolean revokeThisUserId = (mSkpBuilder.getMutableRevokeUserIds().contains(userId)); // only if primary user id will be changed // (this is not triggered if the user id is currently the primary one) @@ -161,8 +157,8 @@ public class UserIdsAdapter extends UserAttributesAdapter { String userId = mCursor.getString(INDEX_USER_ID); boolean isRevokedPending = false; - if (mSaveKeyringParcel != null) { - if (mSaveKeyringParcel.mRevokeUserIds.contains(userId)) { + if (mSkpBuilder != null) { + if (mSkpBuilder.getMutableRevokeUserIds().contains(userId)) { isRevokedPending = true; } @@ -181,8 +177,8 @@ public class UserIdsAdapter extends UserAttributesAdapter { * * @param saveKeyringParcel The parcel to get info from, or null to leave edit mode. */ - public void setEditMode(@Nullable SaveKeyringParcel saveKeyringParcel) { - mSaveKeyringParcel = saveKeyringParcel; + public void setEditMode(@Nullable SaveKeyringParcel.Builder saveKeyringParcel) { + mSkpBuilder = saveKeyringParcel; } @Override diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/keyview/ViewKeyFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/keyview/ViewKeyFragment.java index 13e46e8e7..d9139d09a 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/keyview/ViewKeyFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/keyview/ViewKeyFragment.java @@ -143,7 +143,7 @@ public class ViewKeyFragment extends LoaderFragment implements LoaderManager.Loa mIsSecret = getArguments().getBoolean(ARG_IS_SECRET); // load user ids after we know if it's a secret key - mUserIdsAdapter = new UserIdsAdapter(getActivity(), null, 0, !mIsSecret, null); + mUserIdsAdapter = new UserIdsAdapter(getActivity(), null, 0, !mIsSecret); mUserIds.setAdapter(mUserIdsAdapter); // initialize loaders, which will take care of auto-refresh on change diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/linked/LinkedIdCreateFinalFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/linked/LinkedIdCreateFinalFragment.java index 6eb48ab7f..f8ff6421b 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/linked/LinkedIdCreateFinalFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/linked/LinkedIdCreateFinalFragment.java @@ -210,15 +210,11 @@ public abstract class LinkedIdCreateFinalFragment extends CryptoOperationFragmen @Nullable @Override public Parcelable createOperationInput() { - SaveKeyringParcel skp = - new SaveKeyringParcel(mLinkedIdWizard.mMasterKeyId, mLinkedIdWizard.mFingerprint); - - WrappedUserAttribute ua = - LinkedAttribute.fromResource(mVerifiedResource).toUserAttribute(); - - skp.mAddUserAttribute.add(ua); - - return skp; + SaveKeyringParcel.Builder builder= + SaveKeyringParcel.buildChangeKeyringParcel(mLinkedIdWizard.mMasterKeyId, mLinkedIdWizard.mFingerprint); + WrappedUserAttribute ua = LinkedAttribute.fromResource(mVerifiedResource).toUserAttribute(); + builder.addUserAttribute(ua); + return builder.build(); } @Override diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/linked/LinkedIdCreateGithubFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/linked/LinkedIdCreateGithubFragment.java index 30f217d50..efe610196 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/linked/LinkedIdCreateGithubFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/linked/LinkedIdCreateGithubFragment.java @@ -96,7 +96,7 @@ public class LinkedIdCreateGithubFragment extends CryptoOperationFragment(), new ArrayList(), CryptoInputParcel.createCryptoInputParcel(new Date(), mKeyPhrase1)); @@ -644,10 +644,10 @@ public class PgpEncryptDecryptTest { { // change flags of second encrypted subkey, decryption should skip it - SaveKeyringParcel parcel = - new SaveKeyringParcel(mStaticRing1.getMasterKeyId(), mStaticRing1.getFingerprint()); - parcel.mChangeSubKeys.add(SubkeyChange.createFlagsOrExpiryChange(encKeyId1, KeyFlags.CERTIFY_OTHER, null)); - UncachedKeyRing modified = PgpKeyOperationTest.applyModificationWithChecks(parcel, mStaticRing1, + SaveKeyringParcel.Builder builder = SaveKeyringParcel.buildChangeKeyringParcel( + mStaticRing1.getMasterKeyId(), mStaticRing1.getFingerprint()); + builder.addOrReplaceSubkeyChange(SubkeyChange.createFlagsOrExpiryChange(encKeyId1, KeyFlags.CERTIFY_OTHER, null)); + UncachedKeyRing modified = PgpKeyOperationTest.applyModificationWithChecks(builder.build(), mStaticRing1, new ArrayList(), new ArrayList(), CryptoInputParcel.createCryptoInputParcel(new Date(), mKeyPhrase1)); @@ -673,9 +673,10 @@ public class PgpEncryptDecryptTest { String plaintext = "dies ist ein plaintext ☭" + TestingUtils.genPassphrase(true); { // revoke first encryption subkey of keyring in database - SaveKeyringParcel parcel = new SaveKeyringParcel(mStaticRing1.getMasterKeyId(), mStaticRing1.getFingerprint()); - parcel.mRevokeSubKeys.add(KeyringTestingHelper.getSubkeyId(mStaticRing1, 2)); - UncachedKeyRing modified = PgpKeyOperationTest.applyModificationWithChecks(parcel, mStaticRing1, + SaveKeyringParcel.Builder builder = SaveKeyringParcel.buildChangeKeyringParcel( + mStaticRing1.getMasterKeyId(), mStaticRing1.getFingerprint()); + builder.addRevokeSubkey(KeyringTestingHelper.getSubkeyId(mStaticRing1, 2)); + UncachedKeyRing modified = PgpKeyOperationTest.applyModificationWithChecks(builder.build(), mStaticRing1, new ArrayList(), new ArrayList(), CryptoInputParcel.createCryptoInputParcel(new Date(), mKeyPhrase1)); diff --git a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java index 843fe87ca..06d112b6a 100644 --- a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java +++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java @@ -18,8 +18,20 @@ package org.sufficientlysecure.keychain.pgp; -import junit.framework.AssertionFailedError; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.nio.ByteBuffer; +import java.security.Security; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Date; +import java.util.HashSet; +import java.util.Iterator; +import java.util.List; +import java.util.Random; + +import junit.framework.AssertionFailedError; import org.bouncycastle.bcpg.BCPGInputStream; import org.bouncycastle.bcpg.Packet; import org.bouncycastle.bcpg.PacketTags; @@ -58,16 +70,18 @@ import org.sufficientlysecure.keychain.util.Passphrase; import org.sufficientlysecure.keychain.util.ProgressScaler; import org.sufficientlysecure.keychain.util.TestingUtils; -import java.io.ByteArrayInputStream; -import java.io.IOException; -import java.nio.ByteBuffer; -import java.security.Security; -import java.util.ArrayList; -import java.util.Date; -import java.util.HashSet; -import java.util.Iterator; -import java.util.List; -import java.util.Random; +import static org.bouncycastle.bcpg.sig.KeyFlags.CERTIFY_OTHER; +import static org.bouncycastle.bcpg.sig.KeyFlags.SIGN_DATA; +import static org.sufficientlysecure.keychain.operations.results.OperationResult.LogType.MSG_MF_ERROR_FINGERPRINT; +import static org.sufficientlysecure.keychain.service.SaveKeyringParcel.Algorithm.ECDSA; +import static org.sufficientlysecure.keychain.service.SaveKeyringParcel.Algorithm.RSA; +import static org.sufficientlysecure.keychain.service.SaveKeyringParcel.Curve.NIST_P256; +import static org.sufficientlysecure.keychain.service.SaveKeyringParcel.SubkeyAdd.createSubkeyAdd; +import static org.sufficientlysecure.keychain.service.SaveKeyringParcel.SubkeyChange.createFlagsOrExpiryChange; +import static org.sufficientlysecure.keychain.service.SaveKeyringParcel.SubkeyChange.createRecertifyChange; +import static org.sufficientlysecure.keychain.service.SaveKeyringParcel.SubkeyChange.createStripChange; +import static org.sufficientlysecure.keychain.service.SaveKeyringParcel.buildChangeKeyringParcel; + @RunWith(KeychainTestRunner.class) public class PgpKeyOperationTest { @@ -77,7 +91,7 @@ public class PgpKeyOperationTest { UncachedKeyRing ring; PgpKeyOperation op; - SaveKeyringParcel parcel; + SaveKeyringParcel.Builder builder; ArrayList onlyA = new ArrayList<>(); ArrayList onlyB = new ArrayList<>(); @@ -88,28 +102,28 @@ public class PgpKeyOperationTest { Security.insertProviderAt(new BouncyCastleProvider(), 1); ShadowLog.stream = System.out; - SaveKeyringParcel parcel = new SaveKeyringParcel(); - parcel.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd( + SaveKeyringParcel.Builder builder = SaveKeyringParcel.buildNewKeyringParcel(); + builder.addSubkeyAdd(SubkeyAdd.createSubkeyAdd( Algorithm.ECDSA, 0, SaveKeyringParcel.Curve.NIST_P256, KeyFlags.CERTIFY_OTHER, 0L)); - parcel.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd( + builder.addSubkeyAdd(SubkeyAdd.createSubkeyAdd( Algorithm.ECDSA, 0, SaveKeyringParcel.Curve.NIST_P256, KeyFlags.SIGN_DATA, 0L)); - parcel.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd( + builder.addSubkeyAdd(SubkeyAdd.createSubkeyAdd( Algorithm.ECDH, 0, SaveKeyringParcel.Curve.NIST_P256, KeyFlags.ENCRYPT_COMMS, 0L)); - parcel.mAddUserIds.add("twi"); - parcel.mAddUserIds.add("pink"); + builder.addUserId("twi"); + builder.addUserId("pink"); { int type = 42; byte[] data = new byte[] { 0, 1, 2, 3, 4 }; WrappedUserAttribute uat = WrappedUserAttribute.fromSubpacket(type, data); - parcel.mAddUserAttribute.add(uat); + builder.addUserAttribute(uat); } - parcel.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(passphrase)); + builder.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(passphrase)); PgpKeyOperation op = new PgpKeyOperation(null); - PgpEditKeyResult result = op.createSecretKeyRing(parcel); + PgpEditKeyResult result = op.createSecretKeyRing(builder.build()); Assert.assertTrue("initial test key creation must succeed", result.success()); Assert.assertNotNull("initial test key creation must succeed", result.getRing()); @@ -123,7 +137,8 @@ public class PgpKeyOperationTest { } - @Before public void setUp() throws Exception { + @Before + public void setUp() throws Exception { // show Log.x messages in system.out ShadowLog.stream = System.out; ring = staticRing; @@ -131,76 +146,76 @@ public class PgpKeyOperationTest { // setting up some parameters just to reduce code duplication op = new PgpKeyOperation(new ProgressScaler(null, 0, 100, 100)); - // set this up, gonna need it more than once - parcel = new SaveKeyringParcel(); - parcel.mMasterKeyId = ring.getMasterKeyId(); - parcel.mFingerprint = ring.getFingerprint(); + resetBuilder(); + } + private void resetBuilder() { + builder = SaveKeyringParcel.buildChangeKeyringParcel(ring.getMasterKeyId(), ring.getFingerprint()); } @Test public void createSecretKeyRingTests() { { - parcel.reset(); - parcel.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd( + resetBuilder(); + builder.addSubkeyAdd(SubkeyAdd.createSubkeyAdd( Algorithm.RSA, new Random().nextInt(256)+255, null, KeyFlags.CERTIFY_OTHER, 0L)); - parcel.mAddUserIds.add("shy"); - parcel.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(passphrase)); + builder.addUserId("shy"); + builder.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(passphrase)); - assertFailure("creating ring with < 2048 bit keysize should fail", parcel, + assertFailure("creating ring with < 2048 bit keysize should fail", builder.build(), LogType.MSG_CR_ERROR_KEYSIZE_2048); } { - parcel.reset(); - parcel.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd( + resetBuilder(); + builder.addSubkeyAdd(SubkeyAdd.createSubkeyAdd( Algorithm.ELGAMAL, 2048, null, KeyFlags.CERTIFY_OTHER, 0L)); - parcel.mAddUserIds.add("shy"); - parcel.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(passphrase)); + builder.addUserId("shy"); + builder.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(passphrase)); - assertFailure("creating ring with ElGamal master key should fail", parcel, + assertFailure("creating ring with ElGamal master key should fail", builder.build(), LogType.MSG_CR_ERROR_FLAGS_ELGAMAL); } { - parcel.reset(); - parcel.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd( + resetBuilder(); + builder.addSubkeyAdd(SubkeyAdd.createSubkeyAdd( Algorithm.ECDSA, 0, SaveKeyringParcel.Curve.NIST_P256, KeyFlags.CERTIFY_OTHER, null)); - parcel.mAddUserIds.add("lotus"); - parcel.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(passphrase)); + builder.addUserId("lotus"); + builder.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(passphrase)); - assertFailure("creating master key with null expiry should fail", parcel, + assertFailure("creating master key with null expiry should fail", builder.build(), LogType.MSG_CR_ERROR_NULL_EXPIRY); } { - parcel.reset(); - parcel.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd( + resetBuilder(); + builder.addSubkeyAdd(SubkeyAdd.createSubkeyAdd( Algorithm.ECDSA, 0, SaveKeyringParcel.Curve.NIST_P256, KeyFlags.SIGN_DATA, 0L)); - parcel.mAddUserIds.add("shy"); - parcel.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(passphrase)); + builder.addUserId("shy"); + builder.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(passphrase)); - assertFailure("creating ring with non-certifying master key should fail", parcel, + assertFailure("creating ring with non-certifying master key should fail", builder.build(), LogType.MSG_CR_ERROR_NO_CERTIFY); } { - parcel.reset(); - parcel.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd( + resetBuilder(); + builder.addSubkeyAdd(SubkeyAdd.createSubkeyAdd( Algorithm.ECDSA, 0, SaveKeyringParcel.Curve.NIST_P256, KeyFlags.CERTIFY_OTHER, 0L)); - parcel.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(passphrase)); + builder.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(passphrase)); - assertFailure("creating ring without user ids should fail", parcel, + assertFailure("creating ring without user ids should fail", builder.build(), LogType.MSG_CR_ERROR_NO_USER_ID); } { - parcel.reset(); - parcel.mAddUserIds.add("shy"); - parcel.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(passphrase)); + resetBuilder(); + builder.addUserId("shy"); + builder.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(passphrase)); - assertFailure("creating ring with no master key should fail", parcel, + assertFailure("creating ring with no master key should fail", builder.build(), LogType.MSG_CR_ERROR_NO_MASTER); } @@ -210,11 +225,11 @@ public class PgpKeyOperationTest { // this is a special case since the flags are in user id certificates rather than // subkey binding certificates public void testMasterFlags() throws Exception { - SaveKeyringParcel parcel = new SaveKeyringParcel(); - parcel.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd( + SaveKeyringParcel.Builder builder = SaveKeyringParcel.buildNewKeyringParcel(); + builder.addSubkeyAdd(SubkeyAdd.createSubkeyAdd( Algorithm.ECDSA, 0, SaveKeyringParcel.Curve.NIST_P256, KeyFlags.CERTIFY_OTHER | KeyFlags.SIGN_DATA, 0L)); - parcel.mAddUserIds.add("luna"); - ring = assertCreateSuccess("creating ring with master key flags must succeed", parcel); + builder.addUserId("luna"); + ring = assertCreateSuccess("creating ring with master key flags must succeed", builder.build()); Assert.assertEquals("the keyring should contain only the master key", 1, KeyringTestingHelper.itToList(ring.getPublicKeys()).size()); @@ -280,43 +295,27 @@ public class PgpKeyOperationTest { public void testBadKeyModification() throws Exception { { - SaveKeyringParcel parcel = new SaveKeyringParcel(); - // off by one - parcel.mMasterKeyId = ring.getMasterKeyId() -1; - parcel.mFingerprint = ring.getFingerprint(); + SaveKeyringParcel.Builder builder = SaveKeyringParcel.buildChangeKeyringParcel( + ring.getMasterKeyId() -1, ring.getFingerprint()); assertModifyFailure("keyring modification with bad master key id should fail", - ring, parcel, LogType.MSG_MF_ERROR_KEYID); + ring, builder.build(), LogType.MSG_MF_ERROR_KEYID); } { - SaveKeyringParcel parcel = new SaveKeyringParcel(); - // off by one - parcel.mMasterKeyId = null; - parcel.mFingerprint = ring.getFingerprint(); + byte[] fingerprint = Arrays.copyOf(ring.getFingerprint(), ring.getFingerprint().length); + fingerprint[5] += 1; - assertModifyFailure("keyring modification with null master key id should fail", - ring, parcel, LogType.MSG_MF_ERROR_KEYID); - } - - { - SaveKeyringParcel parcel = new SaveKeyringParcel(); - parcel.mMasterKeyId = ring.getMasterKeyId(); - parcel.mFingerprint = ring.getFingerprint(); - // some byte, off by one - parcel.mFingerprint[5] += 1; + SaveKeyringParcel.Builder builder = buildChangeKeyringParcel(ring.getMasterKeyId(), fingerprint); assertModifyFailure("keyring modification with bad fingerprint should fail", - ring, parcel, LogType.MSG_MF_ERROR_FINGERPRINT); + ring, builder.build(), MSG_MF_ERROR_FINGERPRINT); } { - SaveKeyringParcel parcel = new SaveKeyringParcel(); - parcel.mMasterKeyId = ring.getMasterKeyId(); - parcel.mFingerprint = null; - + SaveKeyringParcel.Builder builder = buildChangeKeyringParcel(ring.getMasterKeyId(), null); assertModifyFailure("keyring modification with null fingerprint should fail", - ring, parcel, LogType.MSG_MF_ERROR_FINGERPRINT); + ring, builder.build(), MSG_MF_ERROR_FINGERPRINT); } { @@ -324,16 +323,16 @@ public class PgpKeyOperationTest { if (badphrase.equals(passphrase)) { badphrase = new Passphrase("a"); } - parcel.mAddUserIds.add("allure"); + builder.addUserId("allure"); assertModifyFailure("keyring modification with bad passphrase should fail", - ring, parcel, CryptoInputParcel.createCryptoInputParcel(badphrase), LogType.MSG_MF_UNLOCK_ERROR); + ring, builder.build(), CryptoInputParcel.createCryptoInputParcel(badphrase), LogType.MSG_MF_UNLOCK_ERROR); } { - parcel.reset(); + resetBuilder(); assertModifyFailure("no-op should fail", - ring, parcel, cryptoInput, LogType.MSG_MF_ERROR_NOOP); + ring, builder.build(), cryptoInput, LogType.MSG_MF_ERROR_NOOP); } } @@ -343,10 +342,10 @@ public class PgpKeyOperationTest { long expiry = new Date().getTime() / 1000 + 159; int flags = KeyFlags.SIGN_DATA; - parcel.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd( + builder.addSubkeyAdd(SubkeyAdd.createSubkeyAdd( Algorithm.ECDSA, 0, SaveKeyringParcel.Curve.NIST_P256, flags, expiry)); - UncachedKeyRing modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB); + UncachedKeyRing modified = applyModificationWithChecks(builder.build(), ring, onlyA, onlyB); Assert.assertEquals("no extra packets in original", 0, onlyA.size()); Assert.assertEquals("exactly two extra packets in modified", 2, onlyB.size()); @@ -381,26 +380,27 @@ public class PgpKeyOperationTest { flags, (long) newKey.getKeyUsage()); { // bad keysize should fail - parcel.reset(); - parcel.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd( - Algorithm.RSA, new Random().nextInt(512), null, KeyFlags.SIGN_DATA, 0L)); - assertModifyFailure("creating a subkey with keysize < 2048 should fail", ring, parcel, + resetBuilder(); + builder.addSubkeyAdd(createSubkeyAdd( + RSA, new Random().nextInt(512), null, SIGN_DATA, 0L)); + assertModifyFailure("creating a subkey with keysize < 2048 should fail", ring, builder.build(), LogType.MSG_CR_ERROR_KEYSIZE_2048); } { // null expiry should fail - parcel.reset(); - parcel.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd( - Algorithm.ECDSA, 0, SaveKeyringParcel.Curve.NIST_P256, KeyFlags.SIGN_DATA, null)); - assertModifyFailure("creating master key with null expiry should fail", ring, parcel, + resetBuilder(); + builder.addSubkeyAdd(createSubkeyAdd( + ECDSA, 0, NIST_P256, SIGN_DATA, null)); + assertModifyFailure("creating master key with null expiry should fail", ring, builder.build(), LogType.MSG_MF_ERROR_NULL_EXPIRY); } { // a past expiry should fail - parcel.reset(); - parcel.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd( - Algorithm.ECDSA, 0, SaveKeyringParcel.Curve.NIST_P256, KeyFlags.SIGN_DATA, new Date().getTime()/1000-10)); - assertModifyFailure("creating subkey with past expiry date should fail", ring, parcel, + resetBuilder(); + builder.addSubkeyAdd(createSubkeyAdd( + ECDSA, 0, NIST_P256, SIGN_DATA, + new Date().getTime() / 1000 - 10)); + assertModifyFailure("creating subkey with past expiry date should fail", ring, builder.build(), LogType.MSG_MF_ERROR_PAST_EXPIRY); } @@ -414,8 +414,8 @@ public class PgpKeyOperationTest { UncachedKeyRing modified = ring; { - parcel.mChangeSubKeys.add(SubkeyChange.createFlagsOrExpiryChange(keyId, null, expiry)); - modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB); + builder.addOrReplaceSubkeyChange(createFlagsOrExpiryChange(keyId, null, expiry)); + modified = applyModificationWithChecks(builder.build(), modified, onlyA, onlyB); Assert.assertEquals("one extra packet in original", 1, onlyA.size()); Assert.assertEquals("one extra packet in modified", 1, onlyB.size()); @@ -441,8 +441,8 @@ public class PgpKeyOperationTest { { // change expiry expiry += 60*60*24; - parcel.mChangeSubKeys.add(SubkeyChange.createFlagsOrExpiryChange(keyId, null, expiry)); - modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB); + builder.addOrReplaceSubkeyChange(createFlagsOrExpiryChange(keyId, null, expiry)); + modified = applyModificationWithChecks(builder.build(), modified, onlyA, onlyB); Assert.assertNotNull("modified key must have an expiry date", modified.getPublicKey(keyId).getUnsafeExpiryTimeForTesting()); @@ -454,9 +454,9 @@ public class PgpKeyOperationTest { { int flags = KeyFlags.SIGN_DATA | KeyFlags.ENCRYPT_COMMS; - parcel.reset(); - parcel.mChangeSubKeys.add(SubkeyChange.createFlagsOrExpiryChange(keyId, flags, null)); - modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB); + resetBuilder(); + builder.addOrReplaceSubkeyChange(createFlagsOrExpiryChange(keyId, flags, null)); + modified = applyModificationWithChecks(builder.build(), modified, onlyA, onlyB); Assert.assertEquals("old packet must be signature", PacketTags.SIGNATURE, onlyA.get(0).tag); @@ -477,9 +477,9 @@ public class PgpKeyOperationTest { } { // expiry of 0 should be "no expiry" - parcel.reset(); - parcel.mChangeSubKeys.add(SubkeyChange.createFlagsOrExpiryChange(keyId, null, 0L)); - modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB); + resetBuilder(); + builder.addOrReplaceSubkeyChange(createFlagsOrExpiryChange(keyId, null, 0L)); + modified = applyModificationWithChecks(builder.build(), modified, onlyA, onlyB); Assert.assertEquals("old packet must be signature", PacketTags.SIGNATURE, onlyA.get(0).tag); @@ -495,18 +495,18 @@ public class PgpKeyOperationTest { } { // a past expiry should fail - parcel.reset(); - parcel.mChangeSubKeys.add(SubkeyChange.createFlagsOrExpiryChange(keyId, null, new Date().getTime()/1000-10)); + resetBuilder(); + builder.addOrReplaceSubkeyChange(createFlagsOrExpiryChange(keyId, null, new Date().getTime() / 1000 - 10)); - assertModifyFailure("setting subkey expiry to a past date should fail", ring, parcel, + assertModifyFailure("setting subkey expiry to a past date should fail", ring, builder.build(), LogType.MSG_MF_ERROR_PAST_EXPIRY); } { // modifying nonexistent subkey should fail - parcel.reset(); - parcel.mChangeSubKeys.add(SubkeyChange.createFlagsOrExpiryChange(123, null, null)); + resetBuilder(); + builder.addOrReplaceSubkeyChange(createFlagsOrExpiryChange(123, null, null)); - assertModifyFailure("modifying non-existent subkey should fail", ring, parcel, + assertModifyFailure("modifying non-existent subkey should fail", ring, builder.build(), LogType.MSG_MF_ERROR_SUBKEY_MISSING); } @@ -521,15 +521,15 @@ public class PgpKeyOperationTest { 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); + builder.addUserId("aloe"); + builder.setChangePrimaryUserId("aloe"); + builder.addRevokeUserId("pink"); + modified = applyModificationWithChecks(builder.build(), modified, onlyA, onlyB); { - parcel.reset(); - parcel.mChangeSubKeys.add(SubkeyChange.createFlagsOrExpiryChange(keyId, null, expiry)); - modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB); + resetBuilder(); + builder.addOrReplaceSubkeyChange(createFlagsOrExpiryChange(keyId, null, expiry)); + modified = applyModificationWithChecks(builder.build(), modified, onlyA, onlyB); // this implies that only the two non-revoked signatures were changed! Assert.assertEquals("two extra packets in original", 2, onlyA.size()); @@ -555,8 +555,8 @@ public class PgpKeyOperationTest { { // change expiry expiry += 60*60*24; - parcel.mChangeSubKeys.add(SubkeyChange.createFlagsOrExpiryChange(keyId, null, expiry)); - modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB); + builder.addOrReplaceSubkeyChange(createFlagsOrExpiryChange(keyId, null, expiry)); + modified = applyModificationWithChecks(builder.build(), modified, onlyA, onlyB); Assert.assertNotNull("modified key must have an expiry date", modified.getPublicKey(keyId).getUnsafeExpiryTimeForTesting()); @@ -574,9 +574,9 @@ public class PgpKeyOperationTest { { int flags = KeyFlags.CERTIFY_OTHER | KeyFlags.SIGN_DATA; - parcel.reset(); - parcel.mChangeSubKeys.add(SubkeyChange.createFlagsOrExpiryChange(keyId, flags, null)); - modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB); + resetBuilder(); + builder.addOrReplaceSubkeyChange(createFlagsOrExpiryChange(keyId, flags, null)); + modified = applyModificationWithChecks(builder.build(), modified, onlyA, onlyB); Assert.assertEquals("modified key must have expected flags", flags, (long) modified.getPublicKey(keyId).getKeyUsage()); @@ -590,13 +590,13 @@ public class PgpKeyOperationTest { // even if there is a non-expiring user id while all others are revoked, it doesn't count! // for this purpose we revoke one while they still have expiry times - parcel.reset(); - parcel.mRevokeUserIds.add("aloe"); - modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB); + resetBuilder(); + builder.addRevokeUserId("aloe"); + modified = applyModificationWithChecks(builder.build(), modified, onlyA, onlyB); - parcel.reset(); - parcel.mChangeSubKeys.add(SubkeyChange.createFlagsOrExpiryChange(keyId, null, 0L)); - modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB); + resetBuilder(); + builder.addOrReplaceSubkeyChange(createFlagsOrExpiryChange(keyId, null, 0L)); + modified = applyModificationWithChecks(builder.build(), modified, onlyA, onlyB); // for this check, it is relevant that we DON'T use the unsafe one! Assert.assertNull("key must not expire anymore", @@ -607,28 +607,28 @@ public class PgpKeyOperationTest { } { // if we revoke everything, nothing is left to properly sign... - parcel.reset(); - parcel.mRevokeUserIds.add("twi"); - parcel.mRevokeUserIds.add("pink"); - parcel.mChangeSubKeys.add(SubkeyChange.createFlagsOrExpiryChange(keyId, KeyFlags.CERTIFY_OTHER, null)); + resetBuilder(); + builder.addRevokeUserId("twi"); + builder.addRevokeUserId("pink"); + builder.addOrReplaceSubkeyChange(createFlagsOrExpiryChange(keyId, CERTIFY_OTHER, null)); - assertModifyFailure("master key modification with all user ids revoked should fail", ring, parcel, + assertModifyFailure("master key modification with all user ids revoked should fail", ring, builder.build(), LogType.MSG_MF_ERROR_MASTER_NONE); } { // any flag not including CERTIFY_OTHER should fail - parcel.reset(); - parcel.mChangeSubKeys.add(SubkeyChange.createFlagsOrExpiryChange(keyId, KeyFlags.SIGN_DATA, null)); + resetBuilder(); + builder.addOrReplaceSubkeyChange(createFlagsOrExpiryChange(keyId, SIGN_DATA, null)); - assertModifyFailure("setting master key flags without certify should fail", ring, parcel, + assertModifyFailure("setting master key flags without certify should fail", ring, builder.build(), LogType.MSG_MF_ERROR_NO_CERTIFY); } { // a past expiry should fail - parcel.reset(); - parcel.mChangeSubKeys.add(SubkeyChange.createFlagsOrExpiryChange(keyId, null, new Date().getTime()/1000-10)); + resetBuilder(); + builder.addOrReplaceSubkeyChange(createFlagsOrExpiryChange(keyId, null, new Date().getTime() / 1000 - 10)); - assertModifyFailure("setting subkey expiry to a past date should fail", ring, parcel, + assertModifyFailure("setting subkey expiry to a past date should fail", ring, builder.build(), LogType.MSG_MF_ERROR_PAST_EXPIRY); } @@ -637,10 +637,10 @@ public class PgpKeyOperationTest { @Test public void testMasterRevoke() throws Exception { - parcel.reset(); - parcel.mRevokeSubKeys.add(ring.getMasterKeyId()); + resetBuilder(); + builder.addRevokeSubkey(ring.getMasterKeyId()); - UncachedKeyRing modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB); + UncachedKeyRing modified = applyModificationWithChecks(builder.build(), ring, onlyA, onlyB); Assert.assertEquals("no extra packets in original", 0, onlyA.size()); Assert.assertEquals("exactly one extra packet in modified", 1, onlyB.size()); @@ -669,11 +669,11 @@ public class PgpKeyOperationTest { { - parcel.reset(); - parcel.mRevokeSubKeys.add(123L); + resetBuilder(); + builder.addRevokeSubkey(123L); CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), 0); - UncachedKeyRing otherModified = op.modifySecretKeyRing(secretRing, cryptoInput, parcel).getRing(); + UncachedKeyRing otherModified = op.modifySecretKeyRing(secretRing, cryptoInput, builder.build()).getRing(); Assert.assertNull("revoking a nonexistent subkey should fail", otherModified); @@ -681,10 +681,10 @@ public class PgpKeyOperationTest { { // revoked second subkey - parcel.reset(); - parcel.mRevokeSubKeys.add(keyId); + resetBuilder(); + builder.addRevokeSubkey(keyId); - modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB, + modified = applyModificationWithChecks(builder.build(), ring, onlyA, onlyB, CryptoInputParcel.createCryptoInputParcel(new Date(), passphrase)); Assert.assertEquals("no extra packets in original", 0, onlyA.size()); @@ -705,11 +705,11 @@ public class PgpKeyOperationTest { { // re-add second subkey - parcel.reset(); + resetBuilder(); // re-certify the revoked subkey - parcel.mChangeSubKeys.add(SubkeyChange.createRecertifyChange(keyId, true)); + builder.addOrReplaceSubkeyChange(createRecertifyChange(keyId, true)); - modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB); + modified = applyModificationWithChecks(builder.build(), modified, onlyA, onlyB); Assert.assertEquals("exactly two outdated packets in original", 2, onlyA.size()); Assert.assertEquals("exactly one extra packet in modified", 1, onlyB.size()); @@ -749,8 +749,8 @@ public class PgpKeyOperationTest { public void testSubkeyStrip() throws Exception { long keyId = KeyringTestingHelper.getSubkeyId(ring, 1); - parcel.mChangeSubKeys.add(SubkeyChange.createStripChange(keyId)); - applyModificationWithChecks(parcel, ring, onlyA, onlyB); + builder.addOrReplaceSubkeyChange(createStripChange(keyId)); + applyModificationWithChecks(builder.build(), ring, onlyA, onlyB); Assert.assertEquals("one extra packet in original", 1, onlyA.size()); Assert.assertEquals("one extra packet in modified", 1, onlyB.size()); @@ -775,8 +775,8 @@ public class PgpKeyOperationTest { public void testMasterStrip() throws Exception { long keyId = ring.getMasterKeyId(); - parcel.mChangeSubKeys.add(SubkeyChange.createStripChange(keyId)); - applyModificationWithChecks(parcel, ring, onlyA, onlyB); + builder.addOrReplaceSubkeyChange(createStripChange(keyId)); + applyModificationWithChecks(builder.build(), ring, onlyA, onlyB); Assert.assertEquals("one extra packet in original", 1, onlyA.size()); Assert.assertEquals("one extra packet in modified", 1, onlyB.size()); @@ -803,9 +803,9 @@ public class PgpKeyOperationTest { UncachedKeyRing modified; { // we should be able to change the stripped status of subkeys without passphrase - parcel.reset(); - parcel.mChangeSubKeys.add(SubkeyChange.createStripChange(keyId)); - modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB, + resetBuilder(); + builder.addOrReplaceSubkeyChange(createStripChange(keyId)); + modified = applyModificationWithChecks(builder.build(), ring, onlyA, onlyB, CryptoInputParcel.createCryptoInputParcel()); Assert.assertEquals("one extra packet in modified", 1, onlyB.size()); Packet p = new BCPGInputStream(new ByteArrayInputStream(onlyB.get(0).buf)).readPacket(); @@ -816,11 +816,11 @@ public class PgpKeyOperationTest { } { // trying to edit a subkey with signing capability should fail - parcel.reset(); - parcel.mChangeSubKeys.add(SubkeyChange.createRecertifyChange(keyId, true)); + resetBuilder(); + builder.addOrReplaceSubkeyChange(createRecertifyChange(keyId, true)); assertModifyFailure("subkey modification for signing-enabled but stripped subkey should fail", - modified, parcel, LogType.MSG_MF_ERROR_SUB_STRIPPED); + modified, builder.build(), LogType.MSG_MF_ERROR_SUB_STRIPPED); } } @@ -829,51 +829,49 @@ public class PgpKeyOperationTest { public void testKeyToSecurityToken() throws Exception { // Special keyring for security token tests with 2048 bit RSA as a subkey - SaveKeyringParcel parcelKey = new SaveKeyringParcel(); - parcelKey.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd( + SaveKeyringParcel.Builder keyBuilder = SaveKeyringParcel.buildNewKeyringParcel(); + keyBuilder.addSubkeyAdd(SubkeyAdd.createSubkeyAdd( Algorithm.DSA, 2048, null, KeyFlags.CERTIFY_OTHER, 0L)); - parcelKey.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd( + keyBuilder.addSubkeyAdd(SubkeyAdd.createSubkeyAdd( Algorithm.RSA, 2048, null, KeyFlags.SIGN_DATA, 0L)); - parcelKey.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd( + keyBuilder.addSubkeyAdd(SubkeyAdd.createSubkeyAdd( Algorithm.RSA, 3072, null, KeyFlags.ENCRYPT_COMMS, 0L)); - parcelKey.mAddUserIds.add("yubikey"); + keyBuilder.addUserId("yubikey"); - parcelKey.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(passphrase)); + keyBuilder.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(passphrase)); PgpKeyOperation opSecurityToken = new PgpKeyOperation(null); - PgpEditKeyResult resultSecurityToken = opSecurityToken.createSecretKeyRing(parcelKey); + PgpEditKeyResult resultSecurityToken = opSecurityToken.createSecretKeyRing(keyBuilder.build()); Assert.assertTrue("initial test key creation must succeed", resultSecurityToken.success()); Assert.assertNotNull("initial test key creation must succeed", resultSecurityToken.getRing()); UncachedKeyRing ringSecurityToken = resultSecurityToken.getRing(); - SaveKeyringParcel parcelSecurityToken = new SaveKeyringParcel(); - parcelSecurityToken.mMasterKeyId = ringSecurityToken.getMasterKeyId(); - parcelSecurityToken.mFingerprint = ringSecurityToken.getFingerprint(); - UncachedKeyRing modified; { // moveKeyToSecurityToken should fail with BAD_NFC_ALGO when presented with the DSA-1024 key long keyId = KeyringTestingHelper.getSubkeyId(ringSecurityToken, 0); - parcelSecurityToken.reset(); - parcelSecurityToken.mChangeSubKeys.add(SubkeyChange.createMoveToSecurityTokenChange(keyId)); + SaveKeyringParcel.Builder securityTokenBuilder = SaveKeyringParcel.buildChangeKeyringParcel( + ringSecurityToken.getMasterKeyId(), ringSecurityToken.getFingerprint()); + securityTokenBuilder.addOrReplaceSubkeyChange(SubkeyChange.createMoveToSecurityTokenChange(keyId)); assertModifyFailure("moveKeyToSecurityToken operation should fail on invalid key algorithm", ringSecurityToken, - parcelSecurityToken, cryptoInput, LogType.MSG_MF_ERROR_BAD_SECURITY_TOKEN_ALGO); + securityTokenBuilder.build(), cryptoInput, LogType.MSG_MF_ERROR_BAD_SECURITY_TOKEN_ALGO); } long keyId = KeyringTestingHelper.getSubkeyId(ringSecurityToken, 1); { // moveKeyToSecurityToken should return a pending SECURITY_TOKEN_MOVE_KEY_TO_CARD result when presented with the RSA-2048 // key, and then make key divert-to-card when it gets a serial in the cryptoInputParcel. - parcelSecurityToken.reset(); - parcelSecurityToken.mChangeSubKeys.add(SubkeyChange.createMoveToSecurityTokenChange(keyId)); + SaveKeyringParcel.Builder securityTokenBuilder = SaveKeyringParcel.buildChangeKeyringParcel( + ringSecurityToken.getMasterKeyId(), ringSecurityToken.getFingerprint()); + securityTokenBuilder.addOrReplaceSubkeyChange(SubkeyChange.createMoveToSecurityTokenChange(keyId)); CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ringSecurityToken.getEncoded(), 0); PgpKeyOperation op = new PgpKeyOperation(null); - PgpEditKeyResult result = op.modifySecretKeyRing(secretRing, cryptoInput, parcelSecurityToken); + PgpEditKeyResult result = op.modifySecretKeyRing(secretRing, cryptoInput, securityTokenBuilder.build()); Assert.assertTrue("moveKeyToSecurityToken operation should be pending", result.isPending()); Assert.assertEquals("required input should be RequiredInputType.SECURITY_TOKEN_MOVE_KEY_TO_CARD", result.getRequiredInputParcel().mType, RequiredInputType.SECURITY_TOKEN_MOVE_KEY_TO_CARD); @@ -889,7 +887,7 @@ public class PgpKeyOperationTest { CryptoInputParcel inputParcel = CryptoInputParcel.createCryptoInputParcel(); inputParcel = inputParcel.withCryptoData(keyIdBytes, serial); - modified = applyModificationWithChecks(parcelSecurityToken, ringSecurityToken, onlyA, onlyB, inputParcel); + modified = applyModificationWithChecks(securityTokenBuilder.build(), ringSecurityToken, onlyA, onlyB, inputParcel); 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", @@ -901,13 +899,14 @@ public class PgpKeyOperationTest { } { // editing a signing subkey requires a primary key binding sig -> pendinginput - parcelSecurityToken.reset(); - parcelSecurityToken.mChangeSubKeys.add(SubkeyChange.createRecertifyChange(keyId, true)); + SaveKeyringParcel.Builder securityTokenBuilder = SaveKeyringParcel.buildChangeKeyringParcel( + ringSecurityToken.getMasterKeyId(), ringSecurityToken.getFingerprint()); + securityTokenBuilder.addOrReplaceSubkeyChange(SubkeyChange.createRecertifyChange(keyId, true)); CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(modified.getEncoded(), 0); PgpKeyOperation op = new PgpKeyOperation(null); - PgpEditKeyResult result = op.modifySecretKeyRing(secretRing, cryptoInput, parcelSecurityToken); + PgpEditKeyResult result = op.modifySecretKeyRing(secretRing, cryptoInput, securityTokenBuilder.build()); Assert.assertTrue("moveKeyToSecurityToken operation should be pending", result.isPending()); Assert.assertEquals("required input should be RequiredInputType.SECURITY_TOKEN_SIGN", RequiredInputType.SECURITY_TOKEN_SIGN, result.getRequiredInputParcel().mType); @@ -922,10 +921,8 @@ public class PgpKeyOperationTest { String uid = ring.getPublicKey().getUnorderedUserIds().get(1); { // revoke second user id - - parcel.mRevokeUserIds.add(uid); - - modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB); + builder.addRevokeUserId(uid); + modified = applyModificationWithChecks(builder.build(), ring, onlyA, onlyB); Assert.assertEquals("no extra packets in original", 0, onlyA.size()); Assert.assertEquals("exactly one extra packet in modified", 1, onlyB.size()); @@ -943,20 +940,20 @@ public class PgpKeyOperationTest { { // re-add second user id - parcel.reset(); - parcel.mChangePrimaryUserId = uid; + resetBuilder(); + builder.setChangePrimaryUserId(uid); - assertModifyFailure("setting primary user id to a revoked user id should fail", modified, parcel, + assertModifyFailure("setting primary user id to a revoked user id should fail", modified, builder.build(), LogType.MSG_MF_ERROR_REVOKED_PRIMARY); } { // re-add second user id - parcel.reset(); - parcel.mAddUserIds.add(uid); + resetBuilder(); + builder.addUserId(uid); - applyModificationWithChecks(parcel, modified, onlyA, onlyB); + applyModificationWithChecks(builder.build(), modified, onlyA, onlyB); Assert.assertEquals("exactly two outdated packets in original", 2, onlyA.size()); Assert.assertEquals("exactly one extra packet in modified", 1, onlyB.size()); @@ -986,10 +983,10 @@ public class PgpKeyOperationTest { } { // revocation of non-existent user id should fail - parcel.reset(); - parcel.mRevokeUserIds.add("nonexistent"); + resetBuilder(); + builder.addRevokeUserId("nonexistent"); - assertModifyFailure("revocation of nonexistent user id should fail", modified, parcel, + assertModifyFailure("revocation of nonexistent user id should fail", modified, builder.build(), LogType.MSG_MF_ERROR_NOEXIST_REVOKE); } @@ -999,15 +996,15 @@ public class PgpKeyOperationTest { public void testUserIdAdd() throws Exception { { - parcel.mAddUserIds.add(""); - assertModifyFailure("adding an empty user id should fail", ring, parcel, + builder.addUserId(""); + assertModifyFailure("adding an empty user id should fail", ring, builder.build(), LogType.MSG_MF_UID_ERROR_EMPTY); } - parcel.reset(); - parcel.mAddUserIds.add("rainbow"); + resetBuilder(); + builder.addUserId("rainbow"); - UncachedKeyRing modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB); + UncachedKeyRing modified = applyModificationWithChecks(builder.build(), ring, onlyA, onlyB); Assert.assertTrue("keyring must contain added user id", modified.getPublicKey().getUnorderedUserIds().contains("rainbow")); @@ -1036,12 +1033,12 @@ public class PgpKeyOperationTest { public void testUserAttributeAdd() throws Exception { { - parcel.mAddUserAttribute.add(WrappedUserAttribute.fromData(new byte[0])); - assertModifyFailure("adding an empty user attribute should fail", ring, parcel, + builder.addUserAttribute(WrappedUserAttribute.fromData(new byte[0])); + assertModifyFailure("adding an empty user attribute should fail", ring, builder.build(), LogType.MSG_MF_UAT_ERROR_EMPTY); } - parcel.reset(); + resetBuilder(); Random r = new Random(); int type = r.nextInt(110)+2; // any type except image attribute, to avoid interpretation of these @@ -1049,9 +1046,9 @@ public class PgpKeyOperationTest { new Random().nextBytes(data); WrappedUserAttribute uat = WrappedUserAttribute.fromSubpacket(type, data); - parcel.mAddUserAttribute.add(uat); + builder.addUserAttribute(uat); - UncachedKeyRing modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB); + UncachedKeyRing modified = applyModificationWithChecks(builder.build(), ring, onlyA, onlyB); Assert.assertEquals("no extra packets in original", 0, onlyA.size()); Assert.assertEquals("exactly two extra packets in modified", 2, onlyB.size()); @@ -1082,7 +1079,7 @@ public class PgpKeyOperationTest { // applying the same modification AGAIN should not add more certifications but drop those // as duplicates - modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB, + applyModificationWithChecks(builder.build(), modified, onlyA, onlyB, CryptoInputParcel.createCryptoInputParcel(new Date(), passphrase), true, false); Assert.assertEquals("duplicate modification: one extra packet in original", 1, onlyA.size()); @@ -1103,20 +1100,20 @@ public class PgpKeyOperationTest { String uid = ring.getPublicKey().getUnorderedUserIds().get(1); { // first part, add new user id which is also primary - parcel.mAddUserIds.add("jack"); - parcel.mChangePrimaryUserId = "jack"; + builder.addUserId("jack"); + builder.setChangePrimaryUserId("jack"); - modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB); + modified = applyModificationWithChecks(builder.build(), modified, onlyA, onlyB); Assert.assertEquals("primary user id must be the one added", "jack", modified.getPublicKey().getPrimaryUserId()); } { // second part, change primary to a different one - parcel.reset(); - parcel.mChangePrimaryUserId = uid; + resetBuilder(); + builder.setChangePrimaryUserId(uid); - modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB); + modified = applyModificationWithChecks(builder.build(), modified, onlyA, onlyB); Assert.assertEquals("old keyring must have two outdated certificates", 2, onlyA.size()); Assert.assertEquals("new keyring must have two new packets", 2, onlyB.size()); @@ -1126,15 +1123,11 @@ public class PgpKeyOperationTest { } { // third part, change primary to a non-existent one - parcel.reset(); + resetBuilder(); //noinspection SpellCheckingInspection - parcel.mChangePrimaryUserId = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; - if (parcel.mChangePrimaryUserId.equals(passphrase)) { - parcel.mChangePrimaryUserId += "A"; - } - + builder.setChangePrimaryUserId("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"); assertModifyFailure("changing primary user id to a non-existent one should fail", - ring, parcel, LogType.MSG_MF_ERROR_NOEXIST_PRIMARY); + ring, builder.build(), LogType.MSG_MF_ERROR_NOEXIST_PRIMARY); } // check for revoked primary user id already done in revoke test @@ -1145,9 +1138,9 @@ public class PgpKeyOperationTest { public void testPassphraseChange() throws Exception { // change passphrase to empty - parcel.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(new Passphrase())); + builder.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(new Passphrase())); // note that canonicalization here necessarily strips the empty notation packet - UncachedKeyRing modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB, cryptoInput); + UncachedKeyRing modified = applyModificationWithChecks(builder.build(), ring, onlyA, onlyB, cryptoInput); Assert.assertEquals("exactly three packets should have been modified (the secret keys)", 3, onlyB.size()); @@ -1160,15 +1153,15 @@ public class PgpKeyOperationTest { // modify keyring, change to non-empty passphrase Passphrase otherPassphrase = TestingUtils.genPassphrase(true); CryptoInputParcel otherCryptoInput = CryptoInputParcel.createCryptoInputParcel(otherPassphrase); - parcel.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(otherPassphrase)); - modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB, + builder.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(otherPassphrase)); + modified = applyModificationWithChecks(builder.build(), modified, onlyA, onlyB, CryptoInputParcel.createCryptoInputParcel(new Date(), new Passphrase())); Assert.assertEquals("exactly three packets should have been modified (the secret keys)", 3, onlyB.size()); { // quick check to make sure no two secret keys have the same IV - HashSet ivs = new HashSet(); + HashSet ivs = new HashSet<>(); for (int i = 0; i < 3; i++) { SecretKeyPacket p = (SecretKeyPacket) new BCPGInputStream( new ByteArrayInputStream(onlyB.get(i).buf)).readPacket(); @@ -1186,7 +1179,7 @@ public class PgpKeyOperationTest { PacketTags.SECRET_SUBKEY, sKeyNoPassphrase.tag); Passphrase otherPassphrase2 = TestingUtils.genPassphrase(true); - parcel.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(otherPassphrase2)); + builder.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(otherPassphrase2)); { // if we replace a secret key with one without passphrase modified = KeyringTestingHelper.removePacket(modified, sKeyNoPassphrase.position); @@ -1195,7 +1188,7 @@ public class PgpKeyOperationTest { // we should still be able to modify it (and change its passphrase) without errors PgpKeyOperation op = new PgpKeyOperation(null); CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(modified.getEncoded(), 0); - PgpEditKeyResult result = op.modifySecretKeyRing(secretRing, otherCryptoInput, parcel); + PgpEditKeyResult result = op.modifySecretKeyRing(secretRing, otherCryptoInput, builder.build()); Assert.assertTrue("key modification must succeed", result.success()); Assert.assertFalse("log must not contain a warning", result.getLog().containsWarnings()); @@ -1212,7 +1205,7 @@ public class PgpKeyOperationTest { PgpKeyOperation op = new PgpKeyOperation(null); CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(modified.getEncoded(), 0); PgpEditKeyResult result = op.modifySecretKeyRing(secretRing, - CryptoInputParcel.createCryptoInputParcel(otherPassphrase2), parcel); + CryptoInputParcel.createCryptoInputParcel(otherPassphrase2), builder.build()); Assert.assertTrue("key modification must succeed", result.success()); Assert.assertTrue("log must contain a failed passphrase change warning", result.getLog().containsType(LogType.MSG_MF_PASSPHRASE_FAIL)); @@ -1225,10 +1218,10 @@ public class PgpKeyOperationTest { CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), 0); - parcel.mAddUserIds.add("discord"); + builder.addUserId("discord"); PgpKeyOperation op = new PgpKeyOperation(null); PgpEditKeyResult result = op.modifySecretKeyRing(secretRing, - CryptoInputParcel.createCryptoInputParcel(new Date()), parcel); + CryptoInputParcel.createCryptoInputParcel(new Date()), builder.build()); Assert.assertFalse("non-restricted operations should fail without passphrase", result.success()); } @@ -1299,8 +1292,8 @@ public class PgpKeyOperationTest { CanonicalizedKeyRing canonicalized = inputKeyRing.canonicalize(new OperationLog(), 0); Assert.assertNotNull("canonicalization must succeed", canonicalized); - ArrayList onlyA = new ArrayList(); - ArrayList onlyB = new ArrayList(); + ArrayList onlyA = new ArrayList<>(); + ArrayList onlyB = new ArrayList<>(); //noinspection unchecked Assert.assertTrue("keyrings differ", !KeyringTestingHelper.diffKeyrings( expectedKeyRing.getEncoded(), expectedKeyRing.getEncoded(), onlyA, onlyB)); diff --git a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringCanonicalizeTest.java b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringCanonicalizeTest.java index c9de64f30..36d54bcb2 100644 --- a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringCanonicalizeTest.java +++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringCanonicalizeTest.java @@ -96,27 +96,27 @@ public class UncachedKeyringCanonicalizeTest { Security.insertProviderAt(new BouncyCastleProvider(), 1); ShadowLog.stream = System.out; - SaveKeyringParcel parcel = new SaveKeyringParcel(); - parcel.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd( + SaveKeyringParcel.Builder builder = SaveKeyringParcel.buildNewKeyringParcel(); + builder.addSubkeyAdd(SubkeyAdd.createSubkeyAdd( Algorithm.ECDSA, 0, SaveKeyringParcel.Curve.NIST_P256, KeyFlags.CERTIFY_OTHER, 0L)); - parcel.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd( + builder.addSubkeyAdd(SubkeyAdd.createSubkeyAdd( Algorithm.ECDSA, 0, SaveKeyringParcel.Curve.NIST_P256, KeyFlags.SIGN_DATA, 0L)); - parcel.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd( + builder.addSubkeyAdd(SubkeyAdd.createSubkeyAdd( Algorithm.ECDH, 0, SaveKeyringParcel.Curve.NIST_P256, KeyFlags.ENCRYPT_COMMS, 0L)); - parcel.mAddUserIds.add("twi"); - parcel.mAddUserIds.add("pink"); + builder.addUserId("twi"); + builder.addUserId("pink"); { WrappedUserAttribute uat = WrappedUserAttribute.fromSubpacket(100, "sunshine, sunshine, ladybugs awake~".getBytes()); - parcel.mAddUserAttribute.add(uat); + builder.addUserAttribute(uat); } // passphrase is tested in PgpKeyOperationTest, just use empty here - parcel.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(new Passphrase())); + builder.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(new Passphrase())); PgpKeyOperation op = new PgpKeyOperation(null); - PgpEditKeyResult result = op.createSecretKeyRing(parcel); + PgpEditKeyResult result = op.createSecretKeyRing(builder.build()); Assert.assertTrue("initial test key creation must succeed", result.success()); staticRing = result.getRing(); Assert.assertNotNull("initial test key creation must succeed", staticRing); @@ -352,14 +352,14 @@ public class UncachedKeyringCanonicalizeTest { @Test public void testForeignSignature() throws Exception { - SaveKeyringParcel parcel = new SaveKeyringParcel(); - parcel.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd( + SaveKeyringParcel.Builder builder = SaveKeyringParcel.buildNewKeyringParcel(); + builder.addSubkeyAdd(SubkeyAdd.createSubkeyAdd( Algorithm.ECDSA, 0, SaveKeyringParcel.Curve.NIST_P256, KeyFlags.CERTIFY_OTHER, 0L)); - parcel.mAddUserIds.add("trix"); + builder.addUserId("trix"); PgpKeyOperation op = new PgpKeyOperation(null); OperationResult.OperationLog log = new OperationResult.OperationLog(); - UncachedKeyRing foreign = op.createSecretKeyRing(parcel).getRing(); + UncachedKeyRing foreign = op.createSecretKeyRing(builder.build()).getRing(); Assert.assertNotNull("initial test key creation must succeed", foreign); PGPSecretKey foreignSecretKey = diff --git a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringMergeTest.java b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringMergeTest.java index 4b9ed015d..abc71cd65 100644 --- a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringMergeTest.java +++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringMergeTest.java @@ -89,7 +89,7 @@ public class UncachedKeyringMergeTest { ArrayList onlyB = new ArrayList<>(); OperationResult.OperationLog log = new OperationResult.OperationLog(); PgpKeyOperation op; - SaveKeyringParcel parcel; + SaveKeyringParcel.Builder builder; @BeforeClass public static void setUpOnce() throws Exception { @@ -97,43 +97,42 @@ public class UncachedKeyringMergeTest { ShadowLog.stream = System.out; { - SaveKeyringParcel parcel = new SaveKeyringParcel(); - parcel.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd( + SaveKeyringParcel.Builder builder = SaveKeyringParcel.buildNewKeyringParcel(); + builder.addSubkeyAdd(SubkeyAdd.createSubkeyAdd( Algorithm.ECDSA, 0, SaveKeyringParcel.Curve.NIST_P256, KeyFlags.CERTIFY_OTHER, 0L)); - parcel.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd( + builder.addSubkeyAdd(SubkeyAdd.createSubkeyAdd( Algorithm.ECDSA, 0, SaveKeyringParcel.Curve.NIST_P256, KeyFlags.SIGN_DATA, 0L)); - parcel.mAddUserIds.add("twi"); - parcel.mAddUserIds.add("pink"); + builder.addUserId("twi"); + builder.addUserId("pink"); { WrappedUserAttribute uat = WrappedUserAttribute.fromSubpacket(100, "sunshine, sunshine, ladybugs awake~".getBytes()); - parcel.mAddUserAttribute.add(uat); + builder.addUserAttribute(uat); } // passphrase is tested in PgpKeyOperationTest, just use empty here - parcel.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(new Passphrase())); + builder.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(new Passphrase())); PgpKeyOperation op = new PgpKeyOperation(null); OperationResult.OperationLog log = new OperationResult.OperationLog(); - PgpEditKeyResult result = op.createSecretKeyRing(parcel); + PgpEditKeyResult result = op.createSecretKeyRing(builder.build()); staticRingA = result.getRing(); staticRingA = staticRingA.canonicalize(new OperationLog(), 0).getUncachedKeyRing(); } { - SaveKeyringParcel parcel = new SaveKeyringParcel(); - parcel.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd( + SaveKeyringParcel.Builder builder = SaveKeyringParcel.buildNewKeyringParcel(); + builder.addSubkeyAdd(SubkeyAdd.createSubkeyAdd( Algorithm.ECDSA, 0, SaveKeyringParcel.Curve.NIST_P256, KeyFlags.CERTIFY_OTHER, 0L)); - parcel.mAddUserIds.add("shy"); + builder.addUserId("shy"); // passphrase is tested in PgpKeyOperationTest, just use empty here - parcel.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(new Passphrase())); + builder.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(new Passphrase())); PgpKeyOperation op = new PgpKeyOperation(null); - OperationResult.OperationLog log = new OperationResult.OperationLog(); - PgpEditKeyResult result = op.createSecretKeyRing(parcel); + PgpEditKeyResult result = op.createSecretKeyRing(builder.build()); staticRingB = result.getRing(); staticRingB = staticRingB.canonicalize(new OperationLog(), 0).getUncachedKeyRing(); } @@ -155,10 +154,11 @@ public class UncachedKeyringMergeTest { // setting up some parameters just to reduce code duplication op = new PgpKeyOperation(new ProgressScaler(null, 0, 100, 100)); - // set this up, gonna need it more than once - parcel = new SaveKeyringParcel(); - parcel.mMasterKeyId = ringA.getMasterKeyId(); - parcel.mFingerprint = ringA.getFingerprint(); + resetBuilder(); + } + + private void resetBuilder() { + builder = SaveKeyringParcel.buildChangeKeyringParcel(ringA.getMasterKeyId(), ringA.getFingerprint()); } public void testSelfNoOp() throws Exception { @@ -190,15 +190,15 @@ public class UncachedKeyringMergeTest { CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ringA.getEncoded(), 0); - parcel.reset(); - parcel.mAddUserIds.add("flim"); + resetBuilder(); + builder.addUserId("flim"); modifiedA = op.modifySecretKeyRing(secretRing, - CryptoInputParcel.createCryptoInputParcel(new Date(), new Passphrase()), parcel).getRing(); + CryptoInputParcel.createCryptoInputParcel(new Date(), new Passphrase()), builder.build()).getRing(); - parcel.reset(); - parcel.mAddUserIds.add("flam"); + resetBuilder(); + builder.addUserId("flam"); modifiedB = op.modifySecretKeyRing(secretRing, - CryptoInputParcel.createCryptoInputParcel(new Date(), new Passphrase()), parcel).getRing(); + CryptoInputParcel.createCryptoInputParcel(new Date(), new Passphrase()), builder.build()).getRing(); } { // merge A into base @@ -232,13 +232,13 @@ public class UncachedKeyringMergeTest { { CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ringA.getEncoded(), 0); - parcel.reset(); - parcel.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd( + resetBuilder(); + builder.addSubkeyAdd(SubkeyAdd.createSubkeyAdd( Algorithm.ECDSA, 0, SaveKeyringParcel.Curve.NIST_P256, KeyFlags.SIGN_DATA, 0L)); modifiedA = op.modifySecretKeyRing(secretRing, - CryptoInputParcel.createCryptoInputParcel(new Date(), new Passphrase()), parcel).getRing(); + CryptoInputParcel.createCryptoInputParcel(new Date(), new Passphrase()), builder.build()).getRing(); modifiedB = op.modifySecretKeyRing(secretRing, - CryptoInputParcel.createCryptoInputParcel(new Date(), new Passphrase()), parcel).getRing(); + CryptoInputParcel.createCryptoInputParcel(new Date(), new Passphrase()), builder.build()).getRing(); subKeyIdA = KeyringTestingHelper.getSubkeyId(modifiedA, 2); subKeyIdB = KeyringTestingHelper.getSubkeyId(modifiedB, 2); @@ -275,12 +275,12 @@ public class UncachedKeyringMergeTest { public void testAddedKeySignature() throws Exception { final UncachedKeyRing modified; { - parcel.reset(); - parcel.mRevokeSubKeys.add(KeyringTestingHelper.getSubkeyId(ringA, 1)); + resetBuilder(); + builder.addRevokeSubkey(KeyringTestingHelper.getSubkeyId(ringA, 1)); CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing( ringA.getEncoded(), 0); modified = op.modifySecretKeyRing(secretRing, - CryptoInputParcel.createCryptoInputParcel(new Date(), new Passphrase()), parcel).getRing(); + CryptoInputParcel.createCryptoInputParcel(new Date(), new Passphrase()), builder.build()).getRing(); } { @@ -368,7 +368,7 @@ public class UncachedKeyringMergeTest { public void testAddedUserAttributeSignature() throws Exception { final UncachedKeyRing modified; { - parcel.reset(); + resetBuilder(); Random r = new Random(); int type = r.nextInt(110)+1; @@ -376,12 +376,12 @@ public class UncachedKeyringMergeTest { new Random().nextBytes(data); WrappedUserAttribute uat = WrappedUserAttribute.fromSubpacket(type, data); - parcel.mAddUserAttribute.add(uat); + builder.addUserAttribute(uat); CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing( ringA.getEncoded(), 0); modified = op.modifySecretKeyRing(secretRing, - CryptoInputParcel.createCryptoInputParcel(new Date(), new Passphrase()), parcel).getRing(); + CryptoInputParcel.createCryptoInputParcel(new Date(), new Passphrase()), builder.build()).getRing(); } { diff --git a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringTest.java b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringTest.java index 88b60e5e4..221264707 100644 --- a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringTest.java +++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringTest.java @@ -54,16 +54,16 @@ public class UncachedKeyringTest { Security.insertProviderAt(new BouncyCastleProvider(), 1); ShadowLog.stream = System.out; - SaveKeyringParcel parcel = new SaveKeyringParcel(); - parcel.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd( + SaveKeyringParcel.Builder builder = SaveKeyringParcel.buildNewKeyringParcel(); + builder.addSubkeyAdd(SubkeyAdd.createSubkeyAdd( Algorithm.ECDSA, 0, SaveKeyringParcel.Curve.NIST_P256, KeyFlags.CERTIFY_OTHER, 0L)); - parcel.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd( + builder.addSubkeyAdd(SubkeyAdd.createSubkeyAdd( Algorithm.ECDSA, 0, SaveKeyringParcel.Curve.NIST_P256, KeyFlags.SIGN_DATA, 0L)); - parcel.mAddSubKeys.add(SubkeyAdd.createSubkeyAdd( + builder.addSubkeyAdd(SubkeyAdd.createSubkeyAdd( Algorithm.ECDH, 0, SaveKeyringParcel.Curve.NIST_P256, KeyFlags.ENCRYPT_COMMS, 0L)); - parcel.mAddUserIds.add("twi"); - parcel.mAddUserIds.add("pink"); + builder.addUserId("twi"); + builder.addUserId("pink"); { Random r = new Random(); int type = r.nextInt(110)+1; @@ -71,13 +71,13 @@ public class UncachedKeyringTest { new Random().nextBytes(data); WrappedUserAttribute uat = WrappedUserAttribute.fromSubpacket(type, data); - parcel.mAddUserAttribute.add(uat); + builder.addUserAttribute(uat); } // passphrase is tested in PgpKeyOperationTest, just use empty here - parcel.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(new Passphrase())); + builder.setNewUnlock(ChangeUnlockParcel.createUnLockParcelForNewKey(new Passphrase())); PgpKeyOperation op = new PgpKeyOperation(null); - PgpEditKeyResult result = op.createSecretKeyRing(parcel); + PgpEditKeyResult result = op.createSecretKeyRing(builder.build()); staticRing = result.getRing(); staticPubRing = staticRing.extractPublicKeyRing();