From 37cb5c4c78880b5b4737cffdaa4ead76506a3843 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 2 Oct 2014 19:23:08 +0200 Subject: [PATCH 1/6] make getSignId a secret key operation, and respect unavailable keys This one should remedy #811, but waiting for a test --- .../keychain/pgp/CanonicalizedKeyRing.java | 20 +------ .../keychain/pgp/CanonicalizedPublicKey.java | 6 ++ .../pgp/CanonicalizedSecretKeyRing.java | 47 ++++----------- .../keychain/pgp/KeyRing.java | 4 -- .../keychain/pgp/UncachedPublicKey.java | 9 +-- .../provider/CachedPublicKeyRing.java | 58 +++++-------------- .../keychain/remote/OpenPgpService.java | 4 +- .../service/KeychainIntentService.java | 2 +- 8 files changed, 38 insertions(+), 112 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedKeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedKeyRing.java index 554899843..f43cbbeef 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedKeyRing.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedKeyRing.java @@ -78,7 +78,7 @@ public abstract class CanonicalizedKeyRing extends KeyRing { public long getEncryptId() throws PgpGeneralException { for(CanonicalizedPublicKey key : publicKeyIterator()) { - if(key.canEncrypt()) { + if (key.canEncrypt() && key.isValid()) { return key.getKeyId(); } } @@ -94,24 +94,6 @@ public abstract class CanonicalizedKeyRing extends KeyRing { } } - public long getSignId() throws PgpGeneralException { - for(CanonicalizedPublicKey key : publicKeyIterator()) { - if(key.canSign()) { - return key.getKeyId(); - } - } - throw new PgpGeneralException("No valid signing key found!"); - } - - public boolean hasSign() throws PgpGeneralException { - try { - getSignId(); - return true; - } catch (PgpGeneralException e) { - return false; - } - } - public void encode(OutputStream stream) throws IOException { getRing().encode(stream); } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedPublicKey.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedPublicKey.java index 8fb3402b2..3539a4ceb 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedPublicKey.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedPublicKey.java @@ -104,4 +104,10 @@ public class CanonicalizedPublicKey extends UncachedPublicKey { public Integer getKeyUsage() { return super.getKeyUsage(); } + + /** Returns whether this key is valid, ie not expired or revoked. */ + public boolean isValid() { + return !isRevoked() && !isExpired(); + } + } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKeyRing.java index 48a2aaeb6..e20155cc6 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKeyRing.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKeyRing.java @@ -30,6 +30,8 @@ import org.spongycastle.openpgp.operator.PBESecretKeyDecryptor; import org.spongycastle.openpgp.operator.jcajce.JcePBESecretKeyDecryptorBuilder; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException; +import org.sufficientlysecure.keychain.provider.KeychainContract; +import org.sufficientlysecure.keychain.provider.ProviderHelper; import org.sufficientlysecure.keychain.util.IterableIterator; import org.sufficientlysecure.keychain.util.Log; @@ -74,43 +76,18 @@ public class CanonicalizedSecretKeyRing extends CanonicalizedKeyRing { return new CanonicalizedSecretKey(this, mRing.getSecretKey(id)); } - /** Getter that returns the subkey that should be used for signing. */ - CanonicalizedSecretKey getSigningSubKey() throws PgpGeneralException { - PGPSecretKey key = mRing.getSecretKey(getSignId()); - if(key != null) { - CanonicalizedSecretKey cKey = new CanonicalizedSecretKey(this, key); - if(!cKey.canSign()) { - throw new PgpGeneralException("key error"); - } - return cKey; - } - // TODO handle with proper exception - throw new PgpGeneralException("no signing key available"); - } - - public boolean hasPassphrase() { - PGPSecretKey secretKey = null; - boolean foundValidKey = false; - for (Iterator keys = mRing.getSecretKeys(); keys.hasNext(); ) { - secretKey = (PGPSecretKey) keys.next(); - if (!secretKey.isPrivateKeyEmpty()) { - foundValidKey = true; - break; + /** Returns the key id which should be used for signing. + * + * This method returns keys which are actually available (ie. secret available, and not stripped, + * revoked, or expired), hence only works on keyrings where a secret key is available! + */ + public long getSecretSignId() throws PgpGeneralException { + for(CanonicalizedSecretKey key : secretKeyIterator()) { + if (key.canSign() && key.isValid() && key.getSecretKeyType().isUsable()) { + return key.getKeyId(); } } - if(!foundValidKey) { - return false; - } - - try { - PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder() - .setProvider("SC").build("".toCharArray()); - PGPPrivateKey testKey = secretKey.extractPrivateKey(keyDecryptor); - return testKey == null; - } catch(PGPException e) { - // this means the crc check failed -> passphrase required - return true; - } + throw new PgpGeneralException("no valid signing key available"); } public IterableIterator secretKeyIterator() { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/KeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/KeyRing.java index 3ef4b336e..b682378e9 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/KeyRing.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/KeyRing.java @@ -56,10 +56,6 @@ public abstract class KeyRing { abstract public boolean hasEncrypt() throws PgpGeneralException; - abstract public long getSignId() throws PgpGeneralException; - - abstract public boolean hasSign() throws PgpGeneralException; - abstract public int getVerified() throws PgpGeneralException; private static final Pattern USER_ID_PATTERN = Pattern.compile("^(.*?)(?: \\((.*)\\))?(?: <(.*)>)?$"); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedPublicKey.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedPublicKey.java index bb9c7d51c..c4cacaca7 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedPublicKey.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedPublicKey.java @@ -51,12 +51,9 @@ public class UncachedPublicKey { /** The revocation signature is NOT checked here, so this may be false! */ public boolean isRevoked() { - for (PGPSignature sig : new IterableIterator( - mPublicKey.getSignaturesOfType(isMasterKey() ? PGPSignature.KEY_REVOCATION - : PGPSignature.SUBKEY_REVOCATION))) { - return true; - } - return false; + return mPublicKey.getSignaturesOfType(isMasterKey() + ? PGPSignature.KEY_REVOCATION + : PGPSignature.SUBKEY_REVOCATION).hasNext(); } public Date getCreationTime() { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/CachedPublicKeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/CachedPublicKeyRing.java index cfb4a915e..ad3ebae5f 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/CachedPublicKeyRing.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/CachedPublicKeyRing.java @@ -135,65 +135,33 @@ public class CachedPublicKeyRing extends KeyRing { @Override public long getEncryptId() throws PgpGeneralException { - try { - Cursor subkeys = getSubkeys(); - if (subkeys != null) { - try { - while (subkeys.moveToNext()) { - if (subkeys.getInt(subkeys.getColumnIndexOrThrow(KeychainContract.Keys.CAN_ENCRYPT)) != 0) { - return subkeys.getLong(subkeys.getColumnIndexOrThrow(KeychainContract.Keys.KEY_ID)); - } - } - } finally { - subkeys.close(); - } - } - } catch(Exception e) { - throw new PgpGeneralException(e); - } - throw new PgpGeneralException("No encrypt key found"); - } - - @Override - public boolean hasEncrypt() throws PgpGeneralException { try { Object data = mProviderHelper.getGenericData(mUri, - KeychainContract.KeyRings.HAS_ENCRYPT, + KeyRings.HAS_ENCRYPT, ProviderHelper.FIELD_TYPE_INTEGER); - return (Long) data > 0; + return (Long) data; } catch(ProviderHelper.NotFoundException e) { throw new PgpGeneralException(e); } } @Override - public long getSignId() throws PgpGeneralException { - try { - Cursor subkeys = getSubkeys(); - if (subkeys != null) { - try { - while (subkeys.moveToNext()) { - if (subkeys.getInt(subkeys.getColumnIndexOrThrow(KeychainContract.Keys.CAN_SIGN)) != 0) { - return subkeys.getLong(subkeys.getColumnIndexOrThrow(KeychainContract.Keys.KEY_ID)); - } - } - } finally { - subkeys.close(); - } - } - } catch(Exception e) { - throw new PgpGeneralException(e); - } - throw new PgpGeneralException("No sign key found"); + public boolean hasEncrypt() throws PgpGeneralException { + return getEncryptId() != 0; } - @Override - public boolean hasSign() throws PgpGeneralException { + /** Returns the key id which should be used for signing. + * + * This method returns keys which are actually available (ie. secret available, and not stripped, + * revoked, or expired), hence only works on keyrings where a secret key is available! + * + */ + public long getSecretSignId() throws PgpGeneralException { try { Object data = mProviderHelper.getGenericData(mUri, - KeychainContract.KeyRings.HAS_SIGN, + KeyRings.HAS_SIGN, ProviderHelper.FIELD_TYPE_INTEGER); - return (Long) data > 0; + return (Long) data; } catch(ProviderHelper.NotFoundException e) { throw new PgpGeneralException(e); } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java index b7ee453d5..ed1f19dbd 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java @@ -293,7 +293,7 @@ public class OpenPgpService extends RemoteService { // Find the appropriate subkey to sign with CachedPublicKeyRing signingRing = new ProviderHelper(this).getCachedPublicKeyRing(accSettings.getKeyId()); - final long sigSubKeyId = signingRing.getSignId(); + final long sigSubKeyId = signingRing.getSecretSignId(); // sign-only PgpSignEncrypt.Builder builder = new PgpSignEncrypt.Builder( @@ -405,7 +405,7 @@ public class OpenPgpService extends RemoteService { // Find the appropriate subkey to sign with CachedPublicKeyRing signingRing = new ProviderHelper(this).getCachedPublicKeyRing(accSettings.getKeyId()); - final long sigSubKeyId = signingRing.getSignId(); + final long sigSubKeyId = signingRing.getSecretSignId(); String passphrase; if (data.hasExtra(OpenPgpApi.EXTRA_PASSPHRASE)) { 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 bb966fec8..0ee3c7093 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java @@ -294,7 +294,7 @@ public class KeychainIntentService extends IntentService implements Progressable // Find the appropriate subkey to sign with CachedPublicKeyRing signingRing = new ProviderHelper(this).getCachedPublicKeyRing(sigMasterKeyId); - long sigSubKeyId = signingRing.getSignId(); + long sigSubKeyId = signingRing.getSecretSignId(); // Set signature settings builder.setSignatureMasterKeyId(sigMasterKeyId) From b6916a9b40ab6edf6396471a966c455f6054da74 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 2 Oct 2014 19:25:20 +0200 Subject: [PATCH 2/6] add test case for stripped master key, correct signing subkey should still be selected --- .../provider/ProviderHelperSaveTest.java | 40 +++++++++++-- .../resources/test-keys/stripped_flags.asc | 60 +++++++++++++++++++ 2 files changed, 95 insertions(+), 5 deletions(-) create mode 100644 OpenKeychain-Test/src/test/resources/test-keys/stripped_flags.asc diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/provider/ProviderHelperSaveTest.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/provider/ProviderHelperSaveTest.java index 3dc107daa..a47d57a69 100644 --- a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/provider/ProviderHelperSaveTest.java +++ b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/provider/ProviderHelperSaveTest.java @@ -89,14 +89,12 @@ public class ProviderHelperSaveTest { CachedPublicKeyRing cachedRing = mProviderHelper.getCachedPublicKeyRing(keyId); CanonicalizedPublicKeyRing pubRing = mProviderHelper.getCanonicalizedPublicKeyRing(keyId); - Assert.assertEquals("master key should be signing key", pubRing.getSignId(), keyId); - Assert.assertEquals("master key should be signing key (cached)", cachedRing.getSignId(), keyId); - Assert.assertEquals("master key should be encryption key", pubRing.getEncryptId(), keyId); - Assert.assertEquals("master key should be signing key (cached)", cachedRing.getEncryptId(), keyId); + Assert.assertEquals("master key should be encryption key", keyId, pubRing.getEncryptId()); + Assert.assertEquals("master key should be encryption key (cached)", keyId, cachedRing.getEncryptId()); Assert.assertNull("canonicalized key flags should be null", pubRing.getPublicKey().getKeyUsage()); Assert.assertTrue("master key should be able to certify", pubRing.getPublicKey().canCertify()); - Assert.assertTrue("master key should be able to sign", pubRing.getPublicKey().canSign()); + Assert.assertTrue("master key should be allowed to sign", pubRing.getPublicKey().canSign()); Assert.assertTrue("master key should be able to encrypt", pubRing.getPublicKey().canEncrypt()); } @@ -188,6 +186,38 @@ public class ProviderHelperSaveTest { Assert.assertTrue("import of the badly encoded user id should succeed", found); } + @Test + /** Tests a master key which may sign, but is stripped. In this case, if there is a different + * subkey available which can sign, that one should be selected. + */ + public void testImportStrippedFlags() throws Exception { + + UncachedKeyRing key = readRingFromResource("/test-keys/stripped_flags.asc"); + long masterKeyId = key.getMasterKeyId(); + + SaveKeyringResult result; + + result = mProviderHelper.saveSecretKeyRing(key, new ProgressScaler()); + Assert.assertTrue("import of keyring should succeed", result.success()); + + long signId; + { + CanonicalizedSecretKeyRing ring = mProviderHelper.getCanonicalizedSecretKeyRing(masterKeyId); + Assert.assertTrue("master key should have sign flag", ring.getPublicKey().canSign()); + Assert.assertTrue("master key should have encrypt flag", ring.getPublicKey().canEncrypt()); + + signId = ring.getSecretSignId(); + Assert.assertNotEquals("encrypt id should not be 0", 0, signId); + Assert.assertNotEquals("encrypt key should be different from master key", masterKeyId, signId); + } + + { + CachedPublicKeyRing ring = mProviderHelper.getCachedPublicKeyRing(masterKeyId); + Assert.assertEquals("signing key should be same id cached as uncached", signId, ring.getSecretSignId()); + } + + } + UncachedKeyRing readRingFromResource(String name) throws Exception { return UncachedKeyRing.fromStream(ProviderHelperSaveTest.class.getResourceAsStream(name)).next(); } diff --git a/OpenKeychain-Test/src/test/resources/test-keys/stripped_flags.asc b/OpenKeychain-Test/src/test/resources/test-keys/stripped_flags.asc new file mode 100644 index 000000000..0f4728297 --- /dev/null +++ b/OpenKeychain-Test/src/test/resources/test-keys/stripped_flags.asc @@ -0,0 +1,60 @@ +-----BEGIN PGP PRIVATE KEY BLOCK----- + +lQIVBFQtfsYBEADhyTqxezePQuP9PCmzdXYavyaaBPCJLpfNhrufe++7xMmkIzGO +l7X8hVrrnqhRs1pVe/rpaSJ8iUVpsMN2BWRrrsmpysf/idjE4OXDGvA5dCRJOhqy +waHA7dO94x2mvcUCVWo5V4cu5+Qv2GejhorPsTNuXXUZrFKWSgEwLpg9lY7M39wB +1SIeQLxMLwixCod87b0A0p9UBCU+QeBx4WFl0NPE3qneQgmFe1Idk6nfwSZFEfYM +6iWepEzhv3GQjdNfXZxLIPqeo4Jowen53HlYYNp/d5fIOpyJ7FxWsutltcGA4GrD +FSfhi/Wh+1/KKwRwCqhFQEZIQjWbmaiQlf3anERfL8Dy+WWiJmZt9W4TDZGY+3Ay +3nDyqVHZJVo4gE+BUiNdx6oITM/mhwVJYpaOnHGMAmWz0Nj0OO5pa86Un/V25gaG +/lmpkvrzHeTpM109xHJt4/WBWt+gUClDd32KCVaSx1b+ECagAf/PJJWpbQspLsNq +DwJCyE8pYnGcoChIVIAPvE5wrAH890FgE43f0utlYW13vp76TXSwuk18XYYCglMl +zuOTFSOkOEflZP4UvJTTjlHoKMJ+iOW4yIS+vvt9jCIYAGp6Fso4pcjakTZ1iSSD +j5QN0ybcJHxKN+fSXZspuW2LijK8aZS2xhWjWwhWJWMDvl5pu9Nx5u4SwwARAQAB +/wNlAkdOVQG0FVN0cmlwcGVkIFRlc3QgPHhAeS56PokCNwQTAQoAIQULCQgHAwYV +CgkLCAMEFgIDAQIZAQWCVC1+xgKeAQKbDwAKCRCf3UpNJ7bxZ9GDEADDEsKrTKvz +c1H6MbBNqsKOWI6Wc8D5MgggphuBmZwXP1FSYVsYTf9UMFsjrlE/JoFr4Yfjslav +ywxbC5Vr74OAL7r25n9MuH7V8xcL14vDssbuIB/aOEmjOkHS2bRgR3hzkGfCSo5Y +zb+uGZrYyJ+72FlTIbV3xAGtX+jYCVXCfzxuavmR5OlVFIWE4d/3sWFmyJG85Twh +9JAl66eP+OK5UbfP1Y01SFmSR/g6Rb0splNq9BOSiurr/cs4z57lDNOheE10UODe +TWtZLuwwy9+ajy1cCPAHtHZIx17d5VFc0I1u2UFoPq3HCQX+PNS8Maq2Nl83ZQNk +z8+k1ZF58ojRgc5KX/wNI6t+aYZQCwEzzTiHAYxkQu8nvCC9M2QnU/LF5uHJyVi8 +QlVCwiuLqwV0PnNBdaLysNpeAZ7B21DA/cwliphOiK8qTIHwbTQvaOTzuahPizt9 +zCDzdzgrNu4RmJmscvh/PQHZTU4wpX3q0Vx/BpZdEIHFCJ0NnHwfXk2uXkbbwQnA +6mvdtGteCL0ffEnjIbKPkjJ8qgWGVrIQP5XrJFTmHBHTdrA4sSCBRm7R0TtmBcOt +JZhUXwPqpfZCcik5BYpptskwwi9J+Di3caCcYFak6xOe0hrYTBOtP3Ztca64hTzW +Q66EYC4TOIqj0Cp5RFXKc6blIcCymbDvI5ylBFQtfscTCCqGSM49AwEHAgMENK58 +neORF9s3/idis8T/u45bCa5Az5hcNlrgX+UFcNoVBESsoIaJT/EGN+8/wIJgIsvo +dUMQXrLvoLuZKw0wzv4JAwha337PCMiUiZDitD0nW3W1nso1Cilb/DmAXfxkZ6zh +D+CU6Km3w3BcIChFQW8R/J+b598UQuq0dhtgg527pxveEfOCPhLwLKAAtBtOCUHU +iQJ/BBgBCgAJBYJULX7HApsCAGoJEJ/dSk0ntvFnX6AEGRMKAAYFAlQtfscACgkQ +tRqCgorJb7ZqQQEAoUsJ5P8GSiLriVSMPqNaFjjL/RBQ6ITwj8SaOyheJicA/1mJ +f1XqAae/Bf9I6Dn5km7RQ6Z9wjL43w/N/E6mIhvgFswP/jv93r0JomWcXienLxQw +2scLjwTH/BpIrf9vGLyzlfkh1T+S3roM2Ul3j5Kc7ycaumMgKNxJpBpMjnpn4Unr +pq0P7oVtjonKs53UqfVUl+/ZNuYG1vqEVH5clW1QL4Xvir6bOM4TXHOXvWQozy5w +z9A5gkzWMMNnXov60J7o7QHbZOBgzsLLvXmP/HiXJoS95RLW8/yHH2I5hBkSbkqm +NIE7g/gD2diVACy/mEmmVmkmdDRnBzknicxoSi1z2r9OrEgDlGeX4iRlgwTVKXf0 +t65wh8fc0tBuPggZrw2mdI8CGDhvDzaQLbwf549XbBgaggAIBzne0V7LsqXDHp5m +qQx7j7WaLuhcv0DkHcTDxXa48Xt4Auhzw64VxqzaleLrsdv0klCmIM+oxZWND2pZ ++VK2pouFNGALJJcbt4sgaX9BkhKqJgW8Rtc7l5OqsqAt7WlmHbOn1E4b6lGAXftH +pX59k3LzVX+KHdIdUhXm0bShEa4GhiQLrP1pMJ1js+JWir3r4uHHwDRM1jt/n899 +llGnGpz/SmyMSLeQ9LjJK7Pt8JcLmK5SKj3FUrl1+Aa+KQNLDJGOvx8cjcFd09p0 +UHi2zF8YbnXbFmj50OVGvFkUbLhYY11t3JLrnLNug2CkcygL40FbBszjrcJxMucJ ++ZSMgM4OkWsqtqRmU4WtGFIQnKkEVC1+xxIIKoZIzj0DAQcCAwTVE/OsK5w9j1Wl +R2U2KtBTCPpc+ED3niUS63kKawp7pudX4BJqnbXwX4DqnMA+iMYX9rPvXymAyjT/ +yEhfGlaHAwEIB/4JAwha337PCMiUiZAtspy93vg+lA7XwKz0K4zScQRW0JiFWEp0 +j8BdiH3B5TOuTD+D5HMhx2hxek/DtW337zQbh5SS0RFckCYOK/qa1qysCNhgiQIf +BBgBCgAJBYJULX7HApsMAAoJEJ/dSk0ntvFnMbYQAJgXLnV7d3xv6hJlCOI8A9Wk +qt5aF032hzsQzd+lhSb1kveuEt07XNZ07Plj/MOdrdNduqVMqJ21A3Pqo1iUr6PC +B+co/BGqUmbkx+16Ebj20SKb48xeHrFtQZb3ciDMzcixXY8pfeFUdb7O2M/3NURV +caDuU4e3FW+eNOnTriW8beRkC2FYud+kMiLfbYT76MufVLERQN94x0T0OCrCI7nQ +GfM8lxgRVJ4hoTygadlv19LCq16wGCVFKIPw/DtFcavkdoN3TboNh/aHia6moKR1 +RSC6II9IcKLMSbnqZdBIJpqpXJIbkCgAOV6Fr3blVg/sub2Mpe9XRT879sO1I8sf +vAP1VI+/I3WE3mZyelJ8xQlR6t81upfpN29DGfKq/194P/mq8b8LYcvO6xdVNkUl +2ZO4ojqY0PBdiFG0VMeGLgzmuxROYVCNV66hg9GvbVPnILFreL4RHy+mDPBa1XQW +YsxIrz1wElOud2HjSLIHdwDEiNuqqQEY4yEmRaD0ULmRXTWINvcMz6mQ3343Np1z +s4ppb6W4OJoVnkW8OIQzTJaPHIItTfLLQFLSWf8L00FVOsV3WtauCO9B688H5JNw +RMmNFoqHMKmplEveTG1beRBRZ7UaLJXh9mO96nS1G1YRTPv5+BWRf9ZhKxoat3GN +9uUAwtb1XXM6DVP1glMx +=kEZ2 +-----END PGP PRIVATE KEY BLOCK----- From a361e82a4de54fe844523db29f2b97ca28d79045 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 2 Oct 2014 19:35:03 +0200 Subject: [PATCH 3/6] fix NullPointerException in keyboard hack --- .../keychain/ui/dialog/PassphraseDialogFragment.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/PassphraseDialogFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/PassphraseDialogFragment.java index a89d4be1c..43f869f02 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/PassphraseDialogFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/PassphraseDialogFragment.java @@ -209,6 +209,10 @@ public class PassphraseDialogFragment extends DialogFragment implements OnEditor mPassphraseEditText.post(new Runnable() { @Override public void run() { + // The activity might already be gone! Nvm in that case. + if (getActivity() == null) { + return; + } InputMethodManager imm = (InputMethodManager) getActivity() .getSystemService(Context.INPUT_METHOD_SERVICE); imm.showSoftInput(mPassphraseEditText, InputMethodManager.SHOW_IMPLICIT); @@ -342,13 +346,18 @@ public class PassphraseDialogFragment extends DialogFragment implements OnEditor } private void hideKeyboard() { + // The activity which called the dialog might no longer exist. Nvm in that case... + if (getActivity() == null) { + return; + } InputMethodManager inputManager = (InputMethodManager) getActivity() .getSystemService(Context.INPUT_METHOD_SERVICE); //check if no view has focus: View v = getActivity().getCurrentFocus(); - if (v == null) + if (v == null) { return; + } inputManager.hideSoftInputFromWindow(v.getWindowToken(), 0); } From 08bfc2ae73e8409caacf1d9dfa9b92e853ae96d6 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 2 Oct 2014 19:40:43 +0200 Subject: [PATCH 4/6] fix hideKeyboard() in other activities as well, just to be safe --- .../sufficientlysecure/keychain/ui/CreateKeyInputFragment.java | 3 +++ .../keychain/ui/ImportKeysCloudFragment.java | 3 +++ .../keychain/ui/dialog/AddUserIdDialogFragment.java | 3 +++ .../keychain/ui/dialog/SetPassphraseDialogFragment.java | 3 +++ 4 files changed, 12 insertions(+) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyInputFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyInputFragment.java index 12887eca5..6079062a7 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyInputFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyInputFragment.java @@ -166,6 +166,9 @@ public class CreateKeyInputFragment extends Fragment { } private void hideKeyboard() { + if (getActivity() == null) { + return; + } InputMethodManager inputManager = (InputMethodManager) getActivity() .getSystemService(Context.INPUT_METHOD_SERVICE); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysCloudFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysCloudFragment.java index 2b5a9793c..3ea107c48 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysCloudFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysCloudFragment.java @@ -159,6 +159,9 @@ public class ImportKeysCloudFragment extends Fragment { } private void hideKeyboard() { + if (getActivity() == null) { + return; + } InputMethodManager inputManager = (InputMethodManager) getActivity() .getSystemService(Context.INPUT_METHOD_SERVICE); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/AddUserIdDialogFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/AddUserIdDialogFragment.java index 27339a02f..3eef04aa7 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/AddUserIdDialogFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/AddUserIdDialogFragment.java @@ -206,6 +206,9 @@ public class AddUserIdDialogFragment extends DialogFragment implements OnEditorA } private void hideKeyboard() { + if (getActivity() == null) { + return; + } InputMethodManager inputManager = (InputMethodManager) getActivity() .getSystemService(Context.INPUT_METHOD_SERVICE); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/SetPassphraseDialogFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/SetPassphraseDialogFragment.java index 5e2bec0e9..a05719072 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/SetPassphraseDialogFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/SetPassphraseDialogFragment.java @@ -200,6 +200,9 @@ public class SetPassphraseDialogFragment extends DialogFragment implements OnEdi } private void hideKeyboard() { + if (getActivity() == null) { + return; + } InputMethodManager inputManager = (InputMethodManager) getActivity() .getSystemService(Context.INPUT_METHOD_SERVICE); From 34e4dabea9165f1cbad0b5efb21ffbf7a6348d39 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Fri, 3 Oct 2014 02:07:09 +0200 Subject: [PATCH 5/6] fix nullpointer with subtle attention seeker --- .../keychain/ui/DecryptActivity.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/DecryptActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/DecryptActivity.java index ac58c63dc..7789ca9c4 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/DecryptActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/DecryptActivity.java @@ -76,10 +76,15 @@ public class DecryptActivity extends DrawerActivity { if (Build.VERSION.SDK_INT >= VERSION_CODES.ICE_CREAM_SANDWICH) { // get text from clipboard - final String clipboardText = - ClipboardReflection.getClipboardText(DecryptActivity.this).toString(); + final CharSequence clipboardText = + ClipboardReflection.getClipboardText(DecryptActivity.this); - AsyncTask tadaTask = new AsyncTask() { + // if it's null, nothing to do here /o/ + if (clipboardText == null) { + return; + } + + new AsyncTask() { @Override protected Boolean doInBackground(Void... params) { @@ -104,11 +109,7 @@ public class DecryptActivity extends DrawerActivity { SubtleAttentionSeeker.tada(findViewById(R.id.clipboard_icon), 1.5f).start(); } } - }; - - if (clipboardText != null) { - tadaTask.execute(); - } + }.execute(); } } From 4b4e885e55f788bd2cf05f3c83f5a4bccb6cd0c0 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Fri, 3 Oct 2014 02:10:57 +0200 Subject: [PATCH 6/6] more getActivity() checks --- .../keychain/ui/PassphraseDialogActivity.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/PassphraseDialogActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/PassphraseDialogActivity.java index decd1757f..4bfca9e1d 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/PassphraseDialogActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/PassphraseDialogActivity.java @@ -157,8 +157,8 @@ public class PassphraseDialogActivity extends FragmentActivity { /* Get key type for message */ // find a master key id for our key - long masterKeyId = new ProviderHelper(getActivity()).getMasterKeyId(mSubKeyId); - CachedPublicKeyRing keyRing = new ProviderHelper(getActivity()).getCachedPublicKeyRing(masterKeyId); + long masterKeyId = new ProviderHelper(activity).getMasterKeyId(mSubKeyId); + CachedPublicKeyRing keyRing = new ProviderHelper(activity).getCachedPublicKeyRing(masterKeyId); // get the type of key (from the database) CanonicalizedSecretKey.SecretKeyType keyType = keyRing.getSecretKeyType(mSubKeyId); switch (keyType) { @@ -324,6 +324,11 @@ public class PassphraseDialogActivity extends FragmentActivity { } private void finishCaching(String passphrase) { + // any indication this isn't needed anymore, don't do it. + if (mIsCancelled || getActivity() == null) { + return; + } + if (mServiceIntent != null) { // TODO: Not routing passphrase through OpenPGP API currently // due to security concerns... @@ -352,6 +357,10 @@ public class PassphraseDialogActivity extends FragmentActivity { public void onDismiss(DialogInterface dialog) { super.onDismiss(dialog); + if (getActivity() == null) { + return; + } + hideKeyboard(); getActivity().setResult(RESULT_CANCELED);