wrapped-key-ring: no UncachedSecretKeyRing after all

9 files changed, 75 insertions, 61 deletions

diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java
index 268906037..91bb89995 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java
@@ -168,7 +168,7 @@ public class PgpImportExport {
                         status = storeKeyRingInCache(new UncachedKeyRing(newPubRing),
                                                      new UncachedKeyRing(secretKeyRing));
                     } else {
-                        status = storeKeyRingInCache(new UncachedKeyRing((PGPPublicKeyRing) keyring));
+                        status = storeKeyRingInCache(new UncachedKeyRing(keyring));
                     }
 
                     if (status == RETURN_ERROR) {
@@ -288,13 +288,13 @@ public class PgpImportExport {
     public int storeKeyRingInCache(UncachedKeyRing ring, UncachedKeyRing secretRing) {
         int status;
         try {
-            UncachedSecretKeyRing secretKeyRing = null;
+            UncachedKeyRing secretKeyRing = null;
             // see what type we have. we can either have a secret + public keyring, or just public
             if (secretKeyRing != null) {
                 mProviderHelper.saveKeyRing(ring, secretRing);
                 status = RETURN_OK;
             } else {
-                mProviderHelper.saveKeyRing(ring);
+                mProviderHelper.savePublicKeyRing(ring);
                 status = RETURN_OK;
             }
         } catch (IOException e) {
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java
index 8bacb32c0..e17d07390 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java
@@ -1,13 +1,16 @@
 package org.sufficientlysecure.keychain.pgp;
 
 import org.spongycastle.bcpg.ArmoredOutputStream;
+import org.spongycastle.bcpg.S2K;
 import org.spongycastle.openpgp.PGPKeyRing;
 import org.spongycastle.openpgp.PGPObjectFactory;
 import org.spongycastle.openpgp.PGPPublicKey;
+import org.spongycastle.openpgp.PGPSecretKey;
 import org.spongycastle.openpgp.PGPSecretKeyRing;
 import org.spongycastle.openpgp.PGPUtil;
 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.BufferedInputStream;
@@ -15,6 +18,7 @@ import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Vector;
@@ -70,7 +74,7 @@ public class UncachedKeyRing {
         return mRing.getPublicKey().getFingerprint();
     }
 
-    public static UncachedKeyRing decodePubkeyFromData(byte[] data)
+    public static UncachedKeyRing decodePublicFromData(byte[] data)
             throws PgpGeneralException, IOException {
         UncachedKeyRing ring = decodeFromData(data);
         if(ring.isSecret()) {
@@ -90,7 +94,6 @@ public class UncachedKeyRing {
             // get first object in block
             Object obj;
             if ((obj = objectFactory.nextObject()) != null && obj instanceof PGPKeyRing) {
-                // the constructor will take care of the public/secret part
                 return new UncachedKeyRing((PGPKeyRing) obj);
             } else {
                 throw new PgpGeneralException("Object not recognized as PGPKeyRing!");
@@ -128,4 +131,23 @@ public class UncachedKeyRing {
         aos.close();
     }
 
+    public ArrayList<Long> getAvailableSubkeys() {
+        if(!isSecret()) {
+            throw new RuntimeException("Tried to find available subkeys from non-secret keys. " +
+                    "This is a programming error and should never happen!");
+        }
+
+        ArrayList<Long> result = new ArrayList<Long>();
+        // then, mark exactly the keys we have available
+        for (PGPSecretKey sub : new IterableIterator<PGPSecretKey>(
+                ((PGPSecretKeyRing) mRing).getSecretKeys())) {
+            S2K s2k = sub.getS2K();
+            // Set to 1, except if the encryption type is GNU_DUMMY_S2K
+            if(s2k == null || s2k.getType() != S2K.GNU_DUMMY_S2K) {
+                result.add(sub.getKeyID());
+            }
+        }
+        return result;
+    }
+
 }
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedSecretKeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedSecretKeyRing.java
deleted file mode 100644
index ca784fbde..000000000
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedSecretKeyRing.java
+++ /dev/null
@@ -1,33 +0,0 @@
-package org.sufficientlysecure.keychain.pgp;
-
-import org.spongycastle.bcpg.S2K;
-import org.spongycastle.openpgp.PGPSecretKey;
-import org.spongycastle.openpgp.PGPSecretKeyRing;
-import org.sufficientlysecure.keychain.provider.KeychainContract;
-import org.sufficientlysecure.keychain.util.IterableIterator;
-
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Iterator;
-
-public class UncachedSecretKeyRing extends UncachedKeyRing {
-
-    UncachedSecretKeyRing(PGPSecretKeyRing secretRing) {
-        super(secretRing);
-    }
-
-    public ArrayList<Long> getAvailableSubkeys() {
-        ArrayList<Long> result = new ArrayList<Long>();
-        // then, mark exactly the keys we have available
-        for (PGPSecretKey sub : new IterableIterator<PGPSecretKey>(
-                ((PGPSecretKeyRing) mRing).getSecretKeys())) {
-            S2K s2k = sub.getS2K();
-            // Set to 1, except if the encryption type is GNU_DUMMY_S2K
-            if(s2k == null || s2k.getType() != S2K.GNU_DUMMY_S2K) {
-                result.add(sub.getKeyID());
-            }
-        }
-        return result;
-    }
-
-}
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedPublicKeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedPublicKeyRing.java
index 624382165..99dc99436 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedPublicKeyRing.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedPublicKeyRing.java
@@ -3,6 +3,8 @@ package org.sufficientlysecure.keychain.pgp;
 import org.spongycastle.bcpg.ArmoredOutputStream;
 import org.spongycastle.bcpg.SignatureSubpacketTags;
 import org.spongycastle.openpgp.PGPException;
+import org.spongycastle.openpgp.PGPKeyRing;
+import org.spongycastle.openpgp.PGPObjectFactory;
 import org.spongycastle.openpgp.PGPPublicKey;
 import org.spongycastle.openpgp.PGPPublicKeyRing;
 import org.spongycastle.openpgp.PGPSignature;
@@ -12,6 +14,7 @@ import org.spongycastle.openpgp.operator.jcajce.JcaPGPContentVerifierBuilderProv
 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.SignatureException;
@@ -30,7 +33,17 @@ public class WrappedPublicKeyRing extends WrappedKeyRing {
 
     PGPPublicKeyRing getRing() {
         if(mRing == null) {
-            mRing = (PGPPublicKeyRing) PgpConversionHelper.BytesToPGPKeyRing(mPubKey);
+            PGPObjectFactory factory = new PGPObjectFactory(mPubKey);
+            PGPKeyRing keyRing = null;
+            try {
+                if ((keyRing = (PGPKeyRing) factory.nextObject()) == null) {
+                    Log.e(Constants.TAG, "No keys given!");
+                }
+            } catch (IOException e) {
+                Log.e(Constants.TAG, "Error while converting to PGPKeyRing!", e);
+            }
+
+            mRing = (PGPPublicKeyRing) keyRing;
         }
         return mRing;
     }
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 656430969..91d4286f4 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedSecretKeyRing.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedSecretKeyRing.java
@@ -1,7 +1,10 @@
 package org.sufficientlysecure.keychain.pgp;
 
 import org.spongycastle.openpgp.PGPException;
+import org.spongycastle.openpgp.PGPKeyRing;
+import org.spongycastle.openpgp.PGPObjectFactory;
 import org.spongycastle.openpgp.PGPPrivateKey;
+import org.spongycastle.openpgp.PGPPublicKeyRing;
 import org.spongycastle.openpgp.PGPSecretKey;
 import org.spongycastle.openpgp.PGPSecretKeyRing;
 import org.spongycastle.openpgp.operator.PBESecretKeyDecryptor;
@@ -11,6 +14,7 @@ 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;
@@ -23,7 +27,17 @@ public class WrappedSecretKeyRing extends WrappedKeyRing {
     public WrappedSecretKeyRing(byte[] blob, boolean isRevoked, int verified)
     {
         super(isRevoked, verified);
-        mRing = (PGPSecretKeyRing) PgpConversionHelper.BytesToPGPKeyRing(blob);
+        PGPObjectFactory factory = new PGPObjectFactory(blob);
+        PGPKeyRing keyRing = null;
+        try {
+            if ((keyRing = (PGPKeyRing) factory.nextObject()) == null) {
+                Log.e(Constants.TAG, "No keys given!");
+            }
+        } catch (IOException e) {
+            Log.e(Constants.TAG, "Error while converting to PGPKeyRing!", e);
+        }
+
+        mRing = (PGPSecretKeyRing) keyRing;
     }
 
     PGPSecretKeyRing getRing() {
@@ -77,7 +91,7 @@ public class WrappedSecretKeyRing extends WrappedKeyRing {
         }
     }
 
-    public UncachedSecretKeyRing changeSecretKeyPassphrase(String oldPassphrase,
+    public UncachedKeyRing changeSecretKeyPassphrase(String oldPassphrase,
                                                      String newPassphrase)
             throws IOException, PGPException, NoSuchProviderException {
 
@@ -96,7 +110,7 @@ public class WrappedSecretKeyRing extends WrappedKeyRing {
                 new JcePBESecretKeyEncryptorBuilder(mRing.getSecretKey()
                         .getKeyEncryptionAlgorithm()).build(newPassphrase.toCharArray()));
 
-        return new UncachedSecretKeyRing(newKeyRing);
+        return new UncachedKeyRing(newKeyRing);
 
     }
 
@@ -120,8 +134,8 @@ public class WrappedSecretKeyRing extends WrappedKeyRing {
         });
     }
 
-    public UncachedSecretKeyRing getUncached() {
-        return new UncachedSecretKeyRing(mRing);
+    public UncachedKeyRing getUncached() {
+        return new UncachedKeyRing(mRing);
     }
 
 }
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainDatabase.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainDatabase.java
index ed8171587..ceaa93f9b 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainDatabase.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainDatabase.java
@@ -268,7 +268,7 @@ public class KeychainDatabase extends SQLiteOpenHelper {
                     byte[] data = cursor.getBlob(0);
                     try {
                         UncachedKeyRing ring = UncachedKeyRing.decodeFromData(data);
-                        providerHelper.saveKeyRing(ring);
+                        providerHelper.savePublicKeyRing(ring);
                     } catch(PgpGeneralException e) {
                         Log.e(Constants.TAG, "Error decoding keyring blob!");
                     }
@@ -292,7 +292,7 @@ public class KeychainDatabase extends SQLiteOpenHelper {
                     byte[] data = cursor.getBlob(0);
                     try {
                         UncachedKeyRing ring = UncachedKeyRing.decodeFromData(data);
-                        providerHelper.saveKeyRing(ring);
+                        providerHelper.savePublicKeyRing(ring);
                     } catch(PgpGeneralException e) {
                         Log.e(Constants.TAG, "Error decoding keyring blob!");
                     }
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainProvider.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainProvider.java
index 2f6cded91..b651069e9 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainProvider.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainProvider.java
@@ -621,7 +621,7 @@ public class KeychainProvider extends ContentProvider {
 
                 case KEY_RING_CERTS:
                     // we replace here, keeping only the latest signature
-                    // TODO this would be better handled in saveKeyRing directly!
+                    // TODO this would be better handled in savePublicKeyRing directly!
                     db.replaceOrThrow(Tables.CERTS, null, values);
                     keyId = values.getAsLong(Certs.MASTER_KEY_ID);
                     break;
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java
index b6f75e00d..55e8c77d1 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java
@@ -35,7 +35,6 @@ import org.sufficientlysecure.keychain.pgp.WrappedPublicKeyRing;
 import org.sufficientlysecure.keychain.pgp.PgpHelper;
 import org.sufficientlysecure.keychain.pgp.PgpKeyHelper;
 import org.sufficientlysecure.keychain.pgp.UncachedKeyRing;
-import org.sufficientlysecure.keychain.pgp.UncachedSecretKeyRing;
 import org.sufficientlysecure.keychain.pgp.WrappedSignature;
 import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException;
 import org.sufficientlysecure.keychain.provider.KeychainContract.ApiApps;
@@ -151,7 +150,7 @@ public class ProviderHelper {
                 if (data != null) {
                     try {
                         result.put(masterKeyId,
-                                UncachedKeyRing.decodePubkeyFromData(data).getPublicKey());
+                                UncachedKeyRing.decodePublicFromData(data).getPublicKey());
                     } catch(PgpGeneralException e) {
                         Log.e(Constants.TAG, "Error parsing keyring, skipping.");
                     } catch(IOException e) {
@@ -221,12 +220,12 @@ public class ProviderHelper {
      * Saves PGPPublicKeyRing with its keys and userIds in DB
      */
     @SuppressWarnings("unchecked")
-    public void saveKeyRing(UncachedKeyRing keyRing) throws IOException {
+    public void savePublicKeyRing(UncachedKeyRing keyRing) throws IOException {
         UncachedPublicKey masterKey = keyRing.getPublicKey();
         long masterKeyId = masterKey.getKeyId();
 
         // IF there is a secret key, preserve it!
-        UncachedSecretKeyRing secretRing = null;
+        UncachedKeyRing secretRing = null;
         try {
             secretRing = getWrappedSecretKeyRing(masterKeyId).getUncached();
         } catch (NotFoundException e) {
@@ -343,7 +342,7 @@ public class ProviderHelper {
 
         // Save the saved keyring (if any)
         if (secretRing != null) {
-            saveKeyRing(secretRing);
+            saveSecretKeyRing(secretRing);
         }
 
     }
@@ -373,7 +372,7 @@ public class ProviderHelper {
      * Saves a PGPSecretKeyRing in the DB. This will only work if a corresponding public keyring
      * is already in the database!
      */
-    public void saveKeyRing(UncachedSecretKeyRing keyRing) throws IOException {
+    public void saveSecretKeyRing(UncachedKeyRing keyRing) throws IOException {
         long masterKeyId = keyRing.getMasterKeyId();
 
         {
@@ -413,12 +412,12 @@ public class ProviderHelper {
     public void saveKeyRing(UncachedKeyRing pubRing, UncachedKeyRing secRing) throws IOException {
         long masterKeyId = pubRing.getPublicKey().getKeyId();
 
-        // delete secret keyring (so it isn't unnecessarily saved by public-saveKeyRing below)
+        // delete secret keyring (so it isn't unnecessarily saved by public-savePublicKeyRing below)
         mContentResolver.delete(KeyRingData.buildSecretKeyRingUri(Long.toString(masterKeyId)), null, null);
 
         // save public keyring
-        saveKeyRing(pubRing);
-        saveKeyRing(secRing);
+        savePublicKeyRing(pubRing);
+        savePublicKeyRing(secRing);
     }
 
     /**
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 69eab9d4e..2655e9aa0 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java
@@ -45,7 +45,6 @@ import org.sufficientlysecure.keychain.pgp.PgpKeyOperation;
 import org.sufficientlysecure.keychain.pgp.PgpSignEncrypt;
 import org.sufficientlysecure.keychain.pgp.Progressable;
 import org.sufficientlysecure.keychain.pgp.UncachedKeyRing;
-import org.sufficientlysecure.keychain.pgp.UncachedSecretKeyRing;
 import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException;
 import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralMsgIdException;
 import org.sufficientlysecure.keychain.provider.KeychainContract.KeyRings;
@@ -514,10 +513,10 @@ public class KeychainIntentService extends IntentService
                 if (!canSign) {
                     setProgress(R.string.progress_building_key, 0, 100);
                     WrappedSecretKeyRing keyRing = providerHelper.getWrappedSecretKeyRing(masterKeyId);
-                    UncachedSecretKeyRing newKeyRing =
+                    UncachedKeyRing newKeyRing =
                             keyRing.changeSecretKeyPassphrase(oldPassphrase, newPassphrase);
                     setProgress(R.string.progress_saving_key_ring, 50, 100);
-                    providerHelper.saveKeyRing(newKeyRing);
+                    providerHelper.saveSecretKeyRing(newKeyRing);
                     setProgress(R.string.progress_done, 100, 100);
                 } else {
                     PgpKeyOperation keyOperations = new PgpKeyOperation(new ProgressScaler(this, 0, 90, 100));
@@ -533,7 +532,7 @@ public class KeychainIntentService extends IntentService
                         UncachedKeyRing ring = keyOperations.buildNewSecretKey(saveParcel); //new Keyring
                         // save the pair
                         setProgress(R.string.progress_saving_key_ring, 90, 100);
-                        providerHelper.saveKeyRing(ring);
+                        providerHelper.savePublicKeyRing(ring);
                     }
 
                     setProgress(R.string.progress_done, 100, 100);
@@ -795,7 +794,7 @@ public class KeychainIntentService extends IntentService
 
                     // create PGPKeyRing object based on downloaded armored key
                     UncachedKeyRing downloadedKey =
-                            UncachedKeyRing.decodePubkeyFromData(downloadedKeyBytes);
+                            UncachedKeyRing.decodePublicFromData(downloadedKeyBytes);
 
                     // verify downloaded key by comparing fingerprints
                     if (entry.getFingerprintHex() != null) {