From 37cb5c4c78880b5b4737cffdaa4ead76506a3843 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 2 Oct 2014 19:23:08 +0200 Subject: 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 +++ .../keychain/pgp/CanonicalizedSecretKeyRing.java | 47 ++++++---------------- .../sufficientlysecure/keychain/pgp/KeyRing.java | 4 -- .../keychain/pgp/UncachedPublicKey.java | 9 ++--- 5 files changed, 22 insertions(+), 64 deletions(-) (limited to 'OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp') 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"); + /** 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(); } - 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; - } - } - 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() { -- cgit v1.2.3