From ed8b62c32b704ce2150bfdf7d4047c67648423a1 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Wed, 28 May 2014 21:52:45 +0200 Subject: wrapped-key-ring: no UncachedSecretKeyRing after all --- .../keychain/pgp/PgpImportExport.java | 6 ++-- .../keychain/pgp/UncachedKeyRing.java | 26 +++++++++++++++-- .../keychain/pgp/UncachedSecretKeyRing.java | 33 ---------------------- .../keychain/pgp/WrappedPublicKeyRing.java | 15 +++++++++- .../keychain/pgp/WrappedSecretKeyRing.java | 24 ++++++++++++---- .../keychain/provider/KeychainDatabase.java | 4 +-- .../keychain/provider/KeychainProvider.java | 2 +- .../keychain/provider/ProviderHelper.java | 17 ++++++----- .../keychain/service/KeychainIntentService.java | 9 +++--- 9 files changed, 75 insertions(+), 61 deletions(-) delete mode 100644 OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedSecretKeyRing.java 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 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 result = new ArrayList(); + // then, mark exactly the keys we have available + for (PGPSecretKey sub : new IterableIterator( + ((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 getAvailableSubkeys() { - ArrayList result = new ArrayList(); - // then, mark exactly the keys we have available - for (PGPSecretKey sub : new IterableIterator( - ((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) { -- cgit v1.2.3