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 1b59e7cc0..9a08290e4 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java @@ -38,6 +38,7 @@ import org.spongycastle.openpgp.operator.PBESecretKeyDecryptor; import org.spongycastle.openpgp.operator.PBESecretKeyEncryptor; import org.spongycastle.openpgp.operator.PGPContentSignerBuilder; import org.spongycastle.openpgp.operator.PGPDigestCalculator; +import org.spongycastle.openpgp.operator.jcajce.JcaKeyFingerprintCalculator; import org.spongycastle.openpgp.operator.jcajce.JcaPGPContentSignerBuilder; import org.spongycastle.openpgp.operator.jcajce.JcaPGPDigestCalculatorProviderBuilder; import org.spongycastle.openpgp.operator.jcajce.JcaPGPKeyPair; @@ -50,6 +51,9 @@ import org.sufficientlysecure.keychain.service.OperationResultParcel.LogLevel; import org.sufficientlysecure.keychain.service.OperationResultParcel.LogType; import org.sufficientlysecure.keychain.service.OperationResultParcel.OperationLog; import org.sufficientlysecure.keychain.service.SaveKeyringParcel; +import org.sufficientlysecure.keychain.service.SaveKeyringParcel.SubkeyAdd; +import org.sufficientlysecure.keychain.util.IterableIterator; +import org.sufficientlysecure.keychain.util.Log; import org.sufficientlysecure.keychain.util.Primes; import java.io.IOException; @@ -63,6 +67,7 @@ import java.security.SignatureException; import java.util.Arrays; import java.util.Calendar; import java.util.Date; +import java.util.Iterator; import java.util.TimeZone; /** @@ -99,18 +104,13 @@ public class PgpKeyOperation { } /** Creates new secret key. */ - private PGPSecretKey createKey(int algorithmChoice, int keySize, String passphrase, - boolean isMasterKey) throws PgpGeneralMsgIdException { + private PGPKeyPair createKey(int algorithmChoice, int keySize) throws PgpGeneralMsgIdException { try { if (keySize < 512) { throw new PgpGeneralMsgIdException(R.string.error_key_size_minimum512bit); } - if (passphrase == null) { - passphrase = ""; - } - int algorithm; KeyPairGenerator keyGen; @@ -123,9 +123,6 @@ public class PgpKeyOperation { } case Constants.choice.algorithm.elgamal: { - if (isMasterKey) { - throw new PgpGeneralMsgIdException(R.string.error_master_key_must_not_be_el_gamal); - } keyGen = KeyPairGenerator.getInstance("ElGamal", Constants.BOUNCY_CASTLE_PROVIDER_NAME); BigInteger p = Primes.getBestPrime(keySize); BigInteger g = new BigInteger("2"); @@ -151,19 +148,8 @@ public class PgpKeyOperation { } // build new key pair - PGPKeyPair keyPair = new JcaPGPKeyPair(algorithm, keyGen.generateKeyPair(), new Date()); + return new JcaPGPKeyPair(algorithm, keyGen.generateKeyPair(), new Date()); - // define hashing and signing algos - PGPDigestCalculator sha1Calc = new JcaPGPDigestCalculatorProviderBuilder().build().get( - HashAlgorithmTags.SHA1); - - // Build key encrypter and decrypter based on passphrase - PBESecretKeyEncryptor keyEncryptor = new JcePBESecretKeyEncryptorBuilder( - PGPEncryptedData.CAST5, sha1Calc) - .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(passphrase.toCharArray()); - - return new PGPSecretKey(keyPair.getPrivateKey(), keyPair.getPublicKey(), - sha1Calc, isMasterKey, keyEncryptor); } catch(NoSuchProviderException e) { throw new RuntimeException(e); } catch(NoSuchAlgorithmException e) { @@ -175,6 +161,55 @@ public class PgpKeyOperation { } } + public UncachedKeyRing createSecretKeyRing(SaveKeyringParcel saveParcel, OperationLog log, + int indent) { + + try { + + log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_KEYID, indent); + indent += 1; + updateProgress(R.string.progress_building_key, 0, 100); + + if (saveParcel.addSubKeys == null || saveParcel.addSubKeys.isEmpty()) { + log.add(LogLevel.ERROR, LogType.MSG_CR_ERROR_NO_MASTER, indent); + return null; + } + + SubkeyAdd add = saveParcel.addSubKeys.remove(0); + PGPKeyPair keyPair = createKey(add.mAlgorithm, add.mKeysize); + + if (add.mAlgorithm == Constants.choice.algorithm.elgamal) { + throw new PgpGeneralMsgIdException(R.string.error_master_key_must_not_be_el_gamal); + } + + // define hashing and signing algos + PGPDigestCalculator sha1Calc = new JcaPGPDigestCalculatorProviderBuilder() + .build().get(HashAlgorithmTags.SHA1); + // Build key encrypter and decrypter based on passphrase + PBESecretKeyEncryptor keyEncryptor = new JcePBESecretKeyEncryptorBuilder( + PGPEncryptedData.CAST5, sha1Calc) + .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build("".toCharArray()); + PGPSecretKey masterSecretKey = new PGPSecretKey(keyPair.getPrivateKey(), keyPair.getPublicKey(), + sha1Calc, true, keyEncryptor); + + PGPSecretKeyRing sKR = new PGPSecretKeyRing( + masterSecretKey.getEncoded(), new JcaKeyFingerprintCalculator()); + + return internal(sKR, masterSecretKey, saveParcel, "", log, indent); + + } catch (PGPException e) { + Log.e(Constants.TAG, "pgp error encoding key", e); + return null; + } catch (IOException e) { + Log.e(Constants.TAG, "io error encoding key", e); + return null; + } catch (PgpGeneralMsgIdException e) { + Log.e(Constants.TAG, "pgp msg id error", e); + return null; + } + + } + /** This method introduces a list of modifications specified by a SaveKeyringParcel to a * WrappedSecretKeyRing. * @@ -204,28 +239,49 @@ public class PgpKeyOperation { indent += 1; updateProgress(R.string.progress_building_key, 0, 100); + // Make sure this is called with a proper SaveKeyringParcel + if (saveParcel.mMasterKeyId == null || saveParcel.mMasterKeyId != wsKR.getMasterKeyId()) { + log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_KEYID, indent); + return null; + } + // We work on bouncycastle object level here PGPSecretKeyRing sKR = wsKR.getRing(); - PGPPublicKey masterPublicKey = sKR.getPublicKey(); PGPSecretKey masterSecretKey = sKR.getSecretKey(); + // Make sure the fingerprint matches + if (saveParcel.mFingerprint == null + || !Arrays.equals(saveParcel.mFingerprint, + masterSecretKey.getPublicKey().getFingerprint())) { + log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_FINGERPRINT, indent); + return null; + } + + return internal(sKR, masterSecretKey, saveParcel, passphrase, log, indent); + + } + + private UncachedKeyRing internal(PGPSecretKeyRing sKR, PGPSecretKey masterSecretKey, + SaveKeyringParcel saveParcel, String passphrase, + OperationLog log, int indent) { + + updateProgress(R.string.progress_certifying_master_key, 20, 100); + + PGPPublicKey masterPublicKey = masterSecretKey.getPublicKey(); + // 1. Unlock private key log.add(LogLevel.DEBUG, LogType.MSG_MF_UNLOCK, indent); - PGPPrivateKey masterPrivateKey; { + PGPPrivateKey masterPrivateKey; + { try { PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder().setProvider( Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(passphrase.toCharArray()); masterPrivateKey = masterSecretKey.extractPrivateKey(keyDecryptor); } catch (PGPException e) { - log.add(LogLevel.ERROR, LogType.MSG_MF_UNLOCK_ERROR, indent+1); + log.add(LogLevel.ERROR, LogType.MSG_MF_UNLOCK_ERROR, indent + 1); return null; } } - if (!Arrays.equals(saveParcel.mFingerprint, sKR.getPublicKey().getFingerprint())) { - return null; - } - - updateProgress(R.string.progress_certifying_master_key, 20, 100); // work on master secret key try { @@ -235,14 +291,42 @@ public class PgpKeyOperation { // 2a. Add certificates for new user ids for (String userId : saveParcel.addUserIds) { log.add(LogLevel.INFO, LogType.MSG_MF_UID_ADD, indent); + + // this operation supersedes all previous binding and revocation certificates, + // so remove those to retain assertions from canonicalization for later operations + @SuppressWarnings("unchecked") + Iterator it = modifiedPublicKey.getSignaturesForID(userId); + if (it != null) { + for (PGPSignature cert : new IterableIterator(it)) { + // if it's not a self cert, never mind + if (cert.getKeyID() != masterPublicKey.getKeyID()) { + continue; + } + if (cert.getSignatureType() == PGPSignature.CERTIFICATION_REVOCATION + || cert.getSignatureType() == PGPSignature.NO_CERTIFICATION + || cert.getSignatureType() == PGPSignature.CASUAL_CERTIFICATION + || cert.getSignatureType() == PGPSignature.POSITIVE_CERTIFICATION + || cert.getSignatureType() == PGPSignature.DEFAULT_CERTIFICATION) { + modifiedPublicKey = PGPPublicKey.removeCertification( + modifiedPublicKey, userId, cert); + } + } + } + + // if it's supposed to be primary, we can do that here as well + boolean isPrimary = saveParcel.changePrimaryUserId != null + && userId.equals(saveParcel.changePrimaryUserId); + // generate and add new certificate PGPSignature cert = generateUserIdSignature(masterPrivateKey, - masterPublicKey, userId, false); + masterPublicKey, userId, isPrimary); modifiedPublicKey = PGPPublicKey.addCertification(masterPublicKey, userId, cert); } // 2b. Add revocations for revoked user ids for (String userId : saveParcel.revokeUserIds) { log.add(LogLevel.INFO, LogType.MSG_MF_UID_REVOKE, indent); + // a duplicate revocatin will be removed during canonicalization, so no need to + // take care of that here. PGPSignature cert = generateRevocationSignature(masterPrivateKey, masterPublicKey, userId); modifiedPublicKey = PGPPublicKey.addCertification(masterPublicKey, userId, cert); @@ -251,7 +335,84 @@ public class PgpKeyOperation { // 3. If primary user id changed, generate new certificates for both old and new if (saveParcel.changePrimaryUserId != null) { log.add(LogLevel.INFO, LogType.MSG_MF_UID_PRIMARY, indent); - // todo + + // we work on the modifiedPublicKey here, to respect new or newly revoked uids + // noinspection unchecked + for (String userId : new IterableIterator(modifiedPublicKey.getUserIDs())) { + boolean isRevoked = false; + PGPSignature currentCert = null; + // noinspection unchecked + for (PGPSignature cert : new IterableIterator( + masterPublicKey.getSignaturesForID(userId))) { + // if it's not a self cert, never mind + if (cert.getKeyID() != masterPublicKey.getKeyID()) { + continue; + } + // we know from canonicalization that if there is any revocation here, it + // is valid and not superseded by a newer certification. + if (cert.getSignatureType() == PGPSignature.CERTIFICATION_REVOCATION) { + isRevoked = true; + continue; + } + // we know from canonicalization that there is only one binding + // certification here, so we can just work with the first one. + if (cert.getSignatureType() == PGPSignature.NO_CERTIFICATION || + cert.getSignatureType() == PGPSignature.CASUAL_CERTIFICATION || + cert.getSignatureType() == PGPSignature.POSITIVE_CERTIFICATION || + cert.getSignatureType() == PGPSignature.DEFAULT_CERTIFICATION) { + currentCert = cert; + } + } + + if (currentCert == null) { + // no certificate found?! error error error + log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_INTEGRITY, indent); + return null; + } + + // we definitely should not update certifications of revoked keys, so just leave it. + if (isRevoked) { + // revoked user ids cannot be primary! + if (userId.equals(saveParcel.changePrimaryUserId)) { + log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_REVOKED_PRIMARY, indent); + return null; + } + continue; + } + + // if this is~ the/a primary user id + if (currentCert.hasSubpackets() && currentCert.getHashedSubPackets().isPrimaryUserID()) { + // if it's the one we want, just leave it as is + if (userId.equals(saveParcel.changePrimaryUserId)) { + continue; + } + // otherwise, generate new non-primary certification + modifiedPublicKey = PGPPublicKey.removeCertification( + modifiedPublicKey, userId, currentCert); + PGPSignature newCert = generateUserIdSignature( + masterPrivateKey, masterPublicKey, userId, false); + modifiedPublicKey = PGPPublicKey.addCertification( + modifiedPublicKey, userId, newCert); + continue; + } + + // if we are here, this is not currently a primary user id + + // if it should be + if (userId.equals(saveParcel.changePrimaryUserId)) { + // add shiny new primary user id certificate + modifiedPublicKey = PGPPublicKey.removeCertification( + modifiedPublicKey, userId, currentCert); + PGPSignature newCert = generateUserIdSignature( + masterPrivateKey, masterPublicKey, userId, true); + modifiedPublicKey = PGPPublicKey.addCertification( + modifiedPublicKey, userId, newCert); + } + + // user id is not primary and is not supposed to be - nothing to do here. + + } + } // Update the secret key ring @@ -261,8 +422,7 @@ public class PgpKeyOperation { sKR = PGPSecretKeyRing.insertSecretKey(sKR, masterSecretKey); } - - // 4a. For each subkey change, generate new subkey binding certificate + // 4a. For each subkey change, generate new subkey binding certificate for (SaveKeyringParcel.SubkeyChange change : saveParcel.changeSubKeys) { log.add(LogLevel.INFO, LogType.MSG_MF_SUBKEY_CHANGE, indent, PgpKeyHelper.convertKeyIdToHex(change.mKeyId)); @@ -280,7 +440,8 @@ public class PgpKeyOperation { return null; } - // generate and add new signature + // generate and add new signature. we can be sloppy here and just leave the old one, + // it will be removed during canonicalization PGPSignature sig = generateSubkeyBindingSignature(masterPublicKey, masterPrivateKey, sKey, pKey, change.mFlags, change.mExpiry, passphrase); pKey = PGPPublicKey.addCertification(pKey, sig); @@ -316,16 +477,36 @@ public class PgpKeyOperation { } log.add(LogLevel.INFO, LogType.MSG_MF_SUBKEY_NEW, indent); - PGPSecretKey sKey = createKey(add.mAlgorithm, add.mKeysize, passphrase, false); + + // generate a new secret key (privkey only for now) + PGPKeyPair keyPair = createKey(add.mAlgorithm, add.mKeysize); + + // add subkey binding signature (making this a sub rather than master key) + PGPPublicKey pKey = keyPair.getPublicKey(); + PGPSignature cert = generateSubkeyBindingSignature( + masterPublicKey, masterPrivateKey, keyPair.getPrivateKey(), pKey, + add.mFlags, add.mExpiry); + pKey = PGPPublicKey.addSubkeyBindingCertification(pKey, cert); + + PGPSecretKey sKey; { + // define hashing and signing algos + PGPDigestCalculator sha1Calc = new JcaPGPDigestCalculatorProviderBuilder() + .build().get(HashAlgorithmTags.SHA1); + + // Build key encrypter and decrypter based on passphrase + PBESecretKeyEncryptor keyEncryptor = new JcePBESecretKeyEncryptorBuilder( + PGPEncryptedData.CAST5, sha1Calc) + .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build("".toCharArray()); + + sKey = new PGPSecretKey(keyPair.getPrivateKey(), pKey, + sha1Calc, false, keyEncryptor); + } + log.add(LogLevel.DEBUG, LogType.MSG_MF_SUBKEY_NEW_ID, indent+1, PgpKeyHelper.convertKeyIdToHex(sKey.getKeyID())); - PGPPublicKey pKey = sKey.getPublicKey(); - PGPSignature cert = generateSubkeyBindingSignature(masterPublicKey, masterPrivateKey, - sKey, pKey, add.mFlags, add.mExpiry, passphrase); - pKey = PGPPublicKey.addCertification(pKey, cert); - sKey = PGPSecretKey.replacePublicKey(sKey, pKey); - sKR = PGPSecretKeyRing.insertSecretKey(sKR, PGPSecretKey.replacePublicKey(sKey, pKey)); + sKR = PGPSecretKeyRing.insertSecretKey(sKR, sKey); + } catch (PgpGeneralMsgIdException e) { return null; } @@ -420,6 +601,18 @@ public class PgpKeyOperation { PGPPublicKey masterPublicKey, PGPPrivateKey masterPrivateKey, PGPSecretKey sKey, PGPPublicKey pKey, int flags, Long expiry, String passphrase) throws IOException, PGPException, SignatureException { + PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder() + .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build( + passphrase.toCharArray()); + PGPPrivateKey subPrivateKey = sKey.extractPrivateKey(keyDecryptor); + return generateSubkeyBindingSignature(masterPublicKey, masterPrivateKey, subPrivateKey, + pKey, flags, expiry); + } + + private static PGPSignature generateSubkeyBindingSignature( + PGPPublicKey masterPublicKey, PGPPrivateKey masterPrivateKey, + PGPPrivateKey subPrivateKey, PGPPublicKey pKey, int flags, Long expiry) + throws IOException, PGPException, SignatureException { // date for signing Date todayDate = new Date(); @@ -427,12 +620,6 @@ public class PgpKeyOperation { // If this key can sign, we need a primary key binding signature if ((flags & KeyFlags.SIGN_DATA) != 0) { - - PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder() - .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build( - passphrase.toCharArray()); - PGPPrivateKey subPrivateKey = sKey.extractPrivateKey(keyDecryptor); - // cross-certify signing keys PGPSignatureSubpacketGenerator subHashedPacketsGen = new PGPSignatureSubpacketGenerator(); subHashedPacketsGen.setSignatureCreationTime(false, todayDate); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedSecretKeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedSecretKeyRing.java index d7148f710..c737b7c46 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedSecretKeyRing.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedSecretKeyRing.java @@ -8,16 +8,13 @@ import org.spongycastle.openpgp.PGPPublicKey; import org.spongycastle.openpgp.PGPSecretKey; import org.spongycastle.openpgp.PGPSecretKeyRing; import org.spongycastle.openpgp.operator.PBESecretKeyDecryptor; -import org.spongycastle.openpgp.operator.jcajce.JcaPGPDigestCalculatorProviderBuilder; import org.spongycastle.openpgp.operator.jcajce.JcePBESecretKeyDecryptorBuilder; -import org.spongycastle.openpgp.operator.jcajce.JcePBESecretKeyEncryptorBuilder; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException; import org.sufficientlysecure.keychain.util.IterableIterator; import org.sufficientlysecure.keychain.util.Log; import java.io.IOException; -import java.security.NoSuchProviderException; import java.util.Iterator; public class WrappedSecretKeyRing extends WrappedKeyRing { @@ -91,29 +88,6 @@ public class WrappedSecretKeyRing extends WrappedKeyRing { } } - public UncachedKeyRing changeSecretKeyPassphrase(String oldPassphrase, - String newPassphrase) - throws IOException, PGPException, NoSuchProviderException { - - if (oldPassphrase == null) { - oldPassphrase = ""; - } - if (newPassphrase == null) { - newPassphrase = ""; - } - - PGPSecretKeyRing newKeyRing = PGPSecretKeyRing.copyWithNewPassword( - mRing, - new JcePBESecretKeyDecryptorBuilder(new JcaPGPDigestCalculatorProviderBuilder() - .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build()).setProvider( - Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(oldPassphrase.toCharArray()), - new JcePBESecretKeyEncryptorBuilder(mRing.getSecretKey() - .getKeyEncryptionAlgorithm()).build(newPassphrase.toCharArray())); - - return new UncachedKeyRing(newKeyRing); - - } - public IterableIterator secretKeyIterator() { final Iterator it = mRing.getSecretKeys(); return new IterableIterator(new Iterator() { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java index 48119a831..c4c31bdad 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java @@ -209,6 +209,10 @@ public class KeychainIntentService extends IntentService mMessenger = (Messenger) extras.get(EXTRA_MESSENGER); Bundle data = extras.getBundle(EXTRA_DATA); + if (data == null) { + Log.e(Constants.TAG, "data extra is null!"); + return; + } OtherHelper.logDebugBundle(data, "EXTRA_DATA"); @@ -320,34 +324,41 @@ public class KeychainIntentService extends IntentService try { /* Input */ SaveKeyringParcel saveParcel = data.getParcelable(SAVE_KEYRING_PARCEL); - long masterKeyId = saveParcel.mMasterKeyId; + if (saveParcel == null) { + Log.e(Constants.TAG, "bug: missing save_keyring_parcel in data!"); + return; + } /* Operation */ ProviderHelper providerHelper = new ProviderHelper(this); PgpKeyOperation keyOperations = new PgpKeyOperation(new ProgressScaler(this, 10, 50, 100)); try { - String passphrase = data.getString(SAVE_KEYRING_PASSPHRASE); - WrappedSecretKeyRing secRing = providerHelper.getWrappedSecretKeyRing(masterKeyId); - OperationLog log = new OperationLog(); - UncachedKeyRing ring = keyOperations.modifySecretKeyRing(secRing, saveParcel, - passphrase, log, 0); - providerHelper.saveSecretKeyRing(ring, new ProgressScaler(this, 60, 95, 100)); + UncachedKeyRing ring; + if (saveParcel.mMasterKeyId != null) { + String passphrase = data.getString(SAVE_KEYRING_PASSPHRASE); + WrappedSecretKeyRing secRing = + providerHelper.getWrappedSecretKeyRing(saveParcel.mMasterKeyId); + + ring = keyOperations.modifySecretKeyRing(secRing, saveParcel, + passphrase, log, 0); + } else { + ring = keyOperations.createSecretKeyRing(saveParcel, log, 0); + } + + providerHelper.saveSecretKeyRing(ring, new ProgressScaler(this, 10, 95, 100)); + + // cache new passphrase + if (saveParcel.newPassphrase != null) { + PassphraseCacheService.addCachedPassphrase(this, ring.getMasterKeyId(), + saveParcel.newPassphrase); + } } catch (ProviderHelper.NotFoundException e) { - // UncachedKeyRing ring = keyOperations.(saveParcel); //new Keyring - // save the pair - setProgress(R.string.progress_saving_key_ring, 95, 100); - // providerHelper.saveSecretKeyRing(ring); sendErrorToHandler(e); } setProgress(R.string.progress_done, 100, 100); - // cache new passphrase - if (saveParcel.newPassphrase != null) { - PassphraseCacheService.addCachedPassphrase(this, masterKeyId, saveParcel.newPassphrase); - } - /* Output */ sendMessageToHandler(KeychainIntentServiceHandler.MESSAGE_OKAY); } catch (Exception e) { @@ -438,7 +449,7 @@ public class KeychainIntentService extends IntentService new FileOutputStream(outputFile)); if (mIsCanceled) { - boolean isDeleted = new File(outputFile).delete(); + new File(outputFile).delete(); } sendMessageToHandler(KeychainIntentServiceHandler.MESSAGE_OKAY, resultData); @@ -594,6 +605,7 @@ public class KeychainIntentService extends IntentService return; } Message msg = Message.obtain(); + assert msg != null; msg.arg1 = arg1; if (arg2 != null) { msg.arg2 = arg2; diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentServiceHandler.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentServiceHandler.java index d5d02081a..755827482 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentServiceHandler.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentServiceHandler.java @@ -27,8 +27,10 @@ import android.support.v4.app.FragmentManager; import com.devspark.appmsg.AppMsg; +import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.R; import org.sufficientlysecure.keychain.ui.dialog.ProgressDialogFragment; +import org.sufficientlysecure.keychain.util.Log; public class KeychainIntentServiceHandler extends Handler { @@ -126,6 +128,7 @@ public class KeychainIntentServiceHandler extends Handler { break; default: + Log.e(Constants.TAG, "unknown handler message!"); break; } } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java index 575180505..6bf6b655d 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java @@ -234,9 +234,16 @@ public class OperationResultParcel implements Parcelable { MSG_MG_NEW_SUBKEY (R.string.msg_mg_new_subkey), MSG_MG_FOUND_NEW (R.string.msg_mg_found_new), + // secret key create + MSG_CR_ERROR_NO_MASTER (R.string.msg_mr), + // secret key modify MSG_MF (R.string.msg_mr), MSG_MF_ERROR_ENCODE (R.string.msg_mf_error_encode), + MSG_MF_ERROR_FINGERPRINT (R.string.msg_mf_error_fingerprint), + MSG_MF_ERROR_KEYID (R.string.msg_mf_error_keyid), + MSG_MF_ERROR_INTEGRITY (R.string.msg_mf_error_integrity), + MSG_MF_ERROR_REVOKED_PRIMARY (R.string.msg_mf_error_revoked_primary), MSG_MF_ERROR_PGP (R.string.msg_mf_error_pgp), MSG_MF_ERROR_SIG (R.string.msg_mf_error_sig), MSG_MF_PASSPHRASE (R.string.msg_mf_passphrase), diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/SaveKeyringParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/SaveKeyringParcel.java index 020b808b9..1ad19cdd0 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/SaveKeyringParcel.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/SaveKeyringParcel.java @@ -22,10 +22,10 @@ import java.util.ArrayList; */ public class SaveKeyringParcel implements Parcelable { - // the master key id to be edited - public final long mMasterKeyId; - // the key fingerprint, for safety - public final byte[] mFingerprint; + // the master key id to be edited. if this is null, a new one will be created + public Long mMasterKeyId; + // the key fingerprint, for safety. MUST be null for a new key. + public byte[] mFingerprint; public String newPassphrase; @@ -38,9 +38,7 @@ public class SaveKeyringParcel implements Parcelable { public ArrayList revokeUserIds; public ArrayList revokeSubKeys; - public SaveKeyringParcel(long masterKeyId, byte[] fingerprint) { - mMasterKeyId = masterKeyId; - mFingerprint = fingerprint; + public SaveKeyringParcel() { addUserIds = new ArrayList(); addSubKeys = new ArrayList(); changeSubKeys = new ArrayList(); @@ -48,6 +46,12 @@ public class SaveKeyringParcel implements Parcelable { revokeSubKeys = new ArrayList(); } + public SaveKeyringParcel(long masterKeyId, byte[] fingerprint) { + this(); + mMasterKeyId = masterKeyId; + mFingerprint = fingerprint; + } + // performance gain for using Parcelable here would probably be negligible, // use Serializable instead. public static class SubkeyAdd implements Serializable { @@ -75,7 +79,7 @@ public class SaveKeyringParcel implements Parcelable { } public SaveKeyringParcel(Parcel source) { - mMasterKeyId = source.readLong(); + mMasterKeyId = source.readInt() != 0 ? source.readLong() : null; mFingerprint = source.createByteArray(); addUserIds = source.createStringArrayList(); @@ -90,7 +94,10 @@ public class SaveKeyringParcel implements Parcelable { @Override public void writeToParcel(Parcel destination, int flags) { - destination.writeLong(mMasterKeyId); + destination.writeInt(mMasterKeyId == null ? 0 : 1); + if(mMasterKeyId != null) { + destination.writeLong(mMasterKeyId); + } destination.writeByteArray(mFingerprint); destination.writeStringList(addUserIds); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/KeyListActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/KeyListActivity.java index 7ce7a06aa..849576284 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/KeyListActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/KeyListActivity.java @@ -17,21 +17,33 @@ package org.sufficientlysecure.keychain.ui; +import android.app.ProgressDialog; import android.content.Intent; import android.os.Bundle; +import android.os.Message; +import android.os.Messenger; import android.view.Menu; import android.view.MenuItem; import com.devspark.appmsg.AppMsg; +import org.spongycastle.bcpg.sig.KeyFlags; import org.sufficientlysecure.keychain.Constants; +import org.sufficientlysecure.keychain.Constants.choice.algorithm; import org.sufficientlysecure.keychain.R; import org.sufficientlysecure.keychain.helper.ExportHelper; +import org.sufficientlysecure.keychain.helper.OtherHelper; +import org.sufficientlysecure.keychain.keyimport.ImportKeysListEntry; import org.sufficientlysecure.keychain.provider.KeychainContract; import org.sufficientlysecure.keychain.provider.KeychainDatabase; +import org.sufficientlysecure.keychain.service.KeychainIntentService; +import org.sufficientlysecure.keychain.service.KeychainIntentServiceHandler; +import org.sufficientlysecure.keychain.service.SaveKeyringParcel; +import org.sufficientlysecure.keychain.service.SaveKeyringParcel.SubkeyAdd; import org.sufficientlysecure.keychain.util.Log; import java.io.IOException; +import java.util.ArrayList; public class KeyListActivity extends DrawerActivity { @@ -121,9 +133,42 @@ public class KeyListActivity extends DrawerActivity { } private void createKeyExpert() { - Intent intent = new Intent(this, EditKeyActivity.class); - intent.setAction(EditKeyActivity.ACTION_CREATE_KEY); - startActivityForResult(intent, 0); - } + Intent intent = new Intent(this, KeychainIntentService.class); + intent.setAction(KeychainIntentService.ACTION_SAVE_KEYRING); -} + // Message is received after importing is done in KeychainIntentService + KeychainIntentServiceHandler saveHandler = new KeychainIntentServiceHandler( + this, + getString(R.string.progress_importing), + ProgressDialog.STYLE_HORIZONTAL) { + public void handleMessage(Message message) { + // handle messages by standard KeychainIntentServiceHandler first + super.handleMessage(message); + Bundle data = message.getData(); + // OtherHelper.logDebugBundle(data, "message reply"); + } + }; + + // fill values for this action + Bundle data = new Bundle(); + + SaveKeyringParcel parcel = new SaveKeyringParcel(); + parcel.addSubKeys.add(new SubkeyAdd(algorithm.rsa, 1024, KeyFlags.CERTIFY_OTHER, null)); + parcel.addSubKeys.add(new SubkeyAdd(algorithm.rsa, 1024, KeyFlags.SIGN_DATA, null)); + parcel.addUserIds.add("swagerinho"); + parcel.newPassphrase = "swag"; + + // get selected key entries + data.putParcelable(KeychainIntentService.SAVE_KEYRING_PARCEL, parcel); + + intent.putExtra(KeychainIntentService.EXTRA_DATA, data); + + // Create a new Messenger for the communication back + Messenger messenger = new Messenger(saveHandler); + intent.putExtra(KeychainIntentService.EXTRA_MESSENGER, messenger); + + saveHandler.showProgressDialog(this); + + startService(intent); + } +} \ No newline at end of file diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml index 54ce523ce..be4892e96 100644 --- a/OpenKeychain/src/main/res/values/strings.xml +++ b/OpenKeychain/src/main/res/values/strings.xml @@ -626,6 +626,10 @@ Modifying keyring %s Encoding exception! + Actual key fingerprint does not match expected! + No keyid. This is a programming error, please file a bug report! + Internal error, integrity check failed! + Revoked user ids cannot be primary! PGP internal exception! Signature exception! Changing passphrase diff --git a/OpenKeychain/src/test/java/tests/PgpDecryptVerifyTest.java b/OpenKeychain/src/test/java/tests/PgpDecryptVerifyTest.java index 346a1f9df..d759bce05 100644 --- a/OpenKeychain/src/test/java/tests/PgpDecryptVerifyTest.java +++ b/OpenKeychain/src/test/java/tests/PgpDecryptVerifyTest.java @@ -22,7 +22,6 @@ public class PgpDecryptVerifyTest { Assert.assertEquals(expectedSignatureResult, status); } - @Test public void testVerifyFailure() throws Exception {