From b47412eb1e2b983f803e6227ea5d07fdead9fe5a Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Sun, 12 Oct 2014 19:22:34 +0200 Subject: [PATCH] CertifyOperation is not a Pgp- operation --- .../keychain/operations/BaseOperation.java | 25 ++++++++++++- ...fyOperation.java => CertifyOperation.java} | 26 +++++++------- .../keychain/pgp/CanonicalizedSecretKey.java | 21 +++++++---- .../service/KeychainIntentService.java | 36 +++++++------------ 4 files changed, 66 insertions(+), 42 deletions(-) rename OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/{PgpCertifyOperation.java => CertifyOperation.java} (87%) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/BaseOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/BaseOperation.java index 01889ed82..09d7a0063 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/BaseOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/BaseOperation.java @@ -2,12 +2,15 @@ package org.sufficientlysecure.keychain.operations; import android.content.Context; +import org.sufficientlysecure.keychain.pgp.PassphraseCacheInterface; import org.sufficientlysecure.keychain.pgp.Progressable; import org.sufficientlysecure.keychain.provider.ProviderHelper; +import org.sufficientlysecure.keychain.provider.ProviderHelper.NotFoundException; +import org.sufficientlysecure.keychain.service.PassphraseCacheService; import java.util.concurrent.atomic.AtomicBoolean; -public class BaseOperation { +public class BaseOperation implements PassphraseCacheInterface { final public Context mContext; final public Progressable mProgressable; @@ -52,4 +55,24 @@ public class BaseOperation { return mCancelled != null && mCancelled.get(); } + @Override + public String getCachedPassphrase(long subKeyId) throws NoSecretKeyException { + try { + long masterKeyId = mProviderHelper.getMasterKeyId(subKeyId); + return getCachedPassphrase(masterKeyId, subKeyId); + } catch (NotFoundException e) { + throw new PassphraseCacheInterface.NoSecretKeyException(); + } + } + + @Override + public String getCachedPassphrase(long masterKeyId, long subKeyId) throws NoSecretKeyException { + try { + return PassphraseCacheService.getCachedPassphrase( + mContext, masterKeyId, subKeyId); + } catch (PassphraseCacheService.KeyNotFoundException e) { + throw new PassphraseCacheInterface.NoSecretKeyException(); + } + } + } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/PgpCertifyOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/CertifyOperation.java similarity index 87% rename from OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/PgpCertifyOperation.java rename to OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/CertifyOperation.java index 845326791..d27221c20 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/PgpCertifyOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/CertifyOperation.java @@ -2,8 +2,6 @@ package org.sufficientlysecure.keychain.operations; import android.content.Context; -import org.spongycastle.openpgp.PGPException; -import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.pgp.CanonicalizedPublicKeyRing; import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKey; import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKeyRing; @@ -19,18 +17,17 @@ import org.sufficientlysecure.keychain.operations.results.OperationResult.LogTyp import org.sufficientlysecure.keychain.operations.results.OperationResult.OperationLog; import org.sufficientlysecure.keychain.operations.results.SaveKeyringResult; import org.sufficientlysecure.keychain.ui.util.KeyFormattingUtils; -import org.sufficientlysecure.keychain.util.Log; import java.util.ArrayList; import java.util.concurrent.atomic.AtomicBoolean; -public class PgpCertifyOperation extends BaseOperation { +public class CertifyOperation extends BaseOperation { - public PgpCertifyOperation(Context context, ProviderHelper providerHelper, Progressable progressable, AtomicBoolean cancelled) { + public CertifyOperation(Context context, ProviderHelper providerHelper, Progressable progressable, AtomicBoolean cancelled) { super(context, providerHelper, progressable, cancelled); } - public CertifyResult certify(CertifyActionsParcel parcel, String passphrase) { + public CertifyResult certify(CertifyActionsParcel parcel) { OperationLog log = new OperationLog(); log.add(LogType.MSG_CRT, 0); @@ -38,6 +35,10 @@ public class PgpCertifyOperation extends BaseOperation { // Retrieve and unlock secret key CanonicalizedSecretKey certificationKey; try { + + // certification is always with the master key id, so use that one + String passphrase = getCachedPassphrase(parcel.mMasterKeyId, parcel.mMasterKeyId); + log.add(LogType.MSG_CRT_MASTER_FETCH, 1); CanonicalizedSecretKeyRing secretKeyRing = mProviderHelper.getCanonicalizedSecretKeyRing(parcel.mMasterKeyId); @@ -53,6 +54,9 @@ public class PgpCertifyOperation extends BaseOperation { } catch (NotFoundException e) { log.add(LogType.MSG_CRT_ERROR_MASTER_NOT_FOUND, 2); return new CertifyResult(CertifyResult.RESULT_ERROR, log); + } catch (NoSecretKeyException e) { + log.add(LogType.MSG_CRT_ERROR_MASTER_NOT_FOUND, 2); + return new CertifyResult(CertifyResult.RESULT_ERROR, log); } ArrayList certifiedKeys = new ArrayList(); @@ -84,15 +88,15 @@ public class PgpCertifyOperation extends BaseOperation { mProviderHelper.getCanonicalizedPublicKeyRing(action.mMasterKeyId); UncachedKeyRing certifiedKey = certificationKey.certifyUserIds(publicRing, action.mUserIds, null, null); + if (certifiedKey == null) { + certifyError += 1; + log.add(LogType.MSG_CRT_WARN_CERT_FAILED, 3); + } certifiedKeys.add(certifiedKey); } catch (NotFoundException e) { certifyError += 1; log.add(LogType.MSG_CRT_WARN_NOT_FOUND, 3); - } catch (PGPException e) { - certifyError += 1; - log.add(LogType.MSG_CRT_WARN_CERT_FAILED, 3); - Log.e(Constants.TAG, "Encountered PGPException during certification", e); } } @@ -128,8 +132,6 @@ public class PgpCertifyOperation extends BaseOperation { log.add(result, 2); - // TODO do something with import results - } if (certifyOk == 0) { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKey.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKey.java index a65759a3b..f9fa41528 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKey.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKey.java @@ -285,8 +285,7 @@ public class CanonicalizedSecretKey extends CanonicalizedPublicKey { * @return A keyring with added certifications */ public UncachedKeyRing certifyUserIds(CanonicalizedPublicKeyRing publicKeyRing, List userIds, - byte[] nfcSignedHash, Date nfcCreationTimestamp) - throws PGPException { + byte[] nfcSignedHash, Date nfcCreationTimestamp) { if (mPrivateKeyState == PRIVATE_KEY_STATE_LOCKED) { throw new PrivateKeyNotUnlockedException(); } @@ -299,7 +298,12 @@ public class CanonicalizedSecretKey extends CanonicalizedPublicKey { nfcSignedHash, nfcCreationTimestamp); signatureGenerator = new PGPSignatureGenerator(contentSignerBuilder); - signatureGenerator.init(PGPSignature.DEFAULT_CERTIFICATION, mPrivateKey); + try { + signatureGenerator.init(PGPSignature.DEFAULT_CERTIFICATION, mPrivateKey); + } catch (PGPException e) { + Log.e(Constants.TAG, "signing error", e); + return null; + } } { // supply signatureGenerator with a SubpacketVector @@ -318,9 +322,14 @@ public class CanonicalizedSecretKey extends CanonicalizedPublicKey { // fetch public key ring, add the certification and return it Iterable it = userIds != null ? userIds : new IterableIterator(publicKey.getUserIDs()); - for (String userId : it) { - PGPSignature sig = signatureGenerator.generateCertification(userId, publicKey); - publicKey = PGPPublicKey.addCertification(publicKey, userId, sig); + try { + for (String userId : it) { + PGPSignature sig = signatureGenerator.generateCertification(userId, publicKey); + publicKey = PGPPublicKey.addCertification(publicKey, userId, sig); + } + } catch (PGPException e) { + Log.e(Constants.TAG, "signing error", e); + return null; } PGPPublicKeyRing ring = PGPPublicKeyRing.insertPublicKey(publicKeyRing.getRing(), publicKey); 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 1bbeaf936..0dcfa1721 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java @@ -29,8 +29,8 @@ import android.os.RemoteException; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.R; +import org.sufficientlysecure.keychain.operations.CertifyOperation; import org.sufficientlysecure.keychain.operations.DeleteOperation; -import org.sufficientlysecure.keychain.operations.PgpCertifyOperation; import org.sufficientlysecure.keychain.operations.results.DeleteResult; import org.sufficientlysecure.keychain.pgp.exception.PgpKeyNotFoundException; import org.sufficientlysecure.keychain.provider.ProviderHelper.NotFoundException; @@ -250,42 +250,34 @@ public class KeychainIntentService extends IntentService implements Progressable Log.logDebugBundle(data, "EXTRA_DATA"); + ProviderHelper providerHelper = new ProviderHelper(this); + String action = intent.getAction(); // executeServiceMethod action from extra bundle if (ACTION_CERTIFY_KEYRING.equals(action)) { - try { + // Input + CertifyActionsParcel parcel = data.getParcelable(CERTIFY_PARCEL); - /* Input */ - CertifyActionsParcel parcel = data.getParcelable(CERTIFY_PARCEL); + // Operation + CertifyOperation op = new CertifyOperation(this, providerHelper, this, mActionCanceled); + CertifyResult result = op.certify(parcel); - /* Operation */ - String passphrase = PassphraseCacheService.getCachedPassphrase(this, - // certification is always with the master key id, so use that one - parcel.mMasterKeyId, parcel.mMasterKeyId); - if (passphrase == null) { - throw new PgpGeneralException("Unable to obtain passphrase"); - } - - ProviderHelper providerHelper = new ProviderHelper(this); - PgpCertifyOperation op = new PgpCertifyOperation(this, providerHelper, this, mActionCanceled); - CertifyResult result = op.certify(parcel, passphrase); - - sendMessageToHandler(KeychainIntentServiceHandler.MESSAGE_OKAY, result); - - } catch (Exception e) { - sendErrorToHandler(e); - } + // Result + sendMessageToHandler(KeychainIntentServiceHandler.MESSAGE_OKAY, result); } else if (ACTION_CONSOLIDATE.equals(action)) { + // Operation ConsolidateResult result; if (data.containsKey(CONSOLIDATE_RECOVERY) && data.getBoolean(CONSOLIDATE_RECOVERY)) { result = new ProviderHelper(this).consolidateDatabaseStep2(this); } else { result = new ProviderHelper(this).consolidateDatabaseStep1(this); } + + // Result sendMessageToHandler(KeychainIntentServiceHandler.MESSAGE_OKAY, result); } else if (ACTION_DECRYPT_METADATA.equals(action)) { @@ -617,7 +609,6 @@ public class KeychainIntentService extends IntentService implements Progressable numEntries = it.getSize(); } - ProviderHelper providerHelper = new ProviderHelper(this); ImportExportOperation importExportOperation = new ImportExportOperation( this, providerHelper, this, mActionCanceled); ImportKeyResult result = importExportOperation.importKeyRings(entries, numEntries); @@ -733,7 +724,6 @@ public class KeychainIntentService extends IntentService implements Progressable /* Operation */ HkpKeyserver server = new HkpKeyserver(keyServer); - ProviderHelper providerHelper = new ProviderHelper(this); CanonicalizedPublicKeyRing keyring = providerHelper.getCanonicalizedPublicKeyRing(dataUri); ImportExportOperation importExportOperation = new ImportExportOperation(this, new ProviderHelper(this), this);