CertifyOperation is not a Pgp- operation

4 files changed, 66 insertions, 42 deletions

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
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<UncachedKeyRing> certifiedKeys = new ArrayList<UncachedKeyRing>();
@@ -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<String> 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<String> it = userIds != null ? userIds
                 : new IterableIterator<String>(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);
-
-                /* 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);
+            // Input
+            CertifyActionsParcel parcel = data.getParcelable(CERTIFY_PARCEL);
 
-                sendMessageToHandler(KeychainIntentServiceHandler.MESSAGE_OKAY, result);
+            // Operation
+            CertifyOperation op = new CertifyOperation(this, providerHelper, this, mActionCanceled);
+            CertifyResult result = op.certify(parcel);
 
-            } 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);