From 08399dec4bbd63e6377b2bd876fbc07c235c65cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Sch=C3=BCrmann?= Date: Tue, 15 Apr 2014 23:54:05 +0200 Subject: Fix PgpDecryptVerify signature verification: search for right signature subkey instead of using first subkey for verification --- .../keychain/pgp/PgpDecryptVerify.java | 42 ++++++++------ .../keychain/pgp/PgpKeyHelper.java | 67 +++++++++++----------- .../keychain/ui/EncryptAsymmetricFragment.java | 4 +- 3 files changed, 60 insertions(+), 53 deletions(-) (limited to 'OpenKeychain/src/main/java') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerify.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerify.java index 40d487a48..b3572d4a2 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerify.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerify.java @@ -276,6 +276,7 @@ public class PgpDecryptVerify { // continue with the next packet in the while loop continue; } + // get subkey which has been used for this encryption packet secretEncryptionKey = secretKeyRing.getSecretKey(encData.getKeyID()); if (secretEncryptionKey == null) { // continue with the next packet in the while loop @@ -390,7 +391,6 @@ public class PgpDecryptVerify { OpenPgpSignatureResultBuilder signatureResultBuilder = new OpenPgpSignatureResultBuilder(); PGPPublicKey signatureKey = null; int signatureIndex = -1; - boolean isSignatureKeyCertified = false; if (dataChunk instanceof PGPCompressedData) { updateProgress(R.string.progress_decompressing_data, currentProgress, 100); @@ -426,18 +426,23 @@ public class PgpDecryptVerify { if (masterKeyId != null) { // key found in our database! + signature = sigList.get(signatureIndex); + + PGPPublicKeyRing publicKeyRing = null; try { - signatureKey = mProviderHelper - .getPGPPublicKeyRing(masterKeyId).getPublicKey(); + publicKeyRing = mProviderHelper + .getPGPPublicKeyRing(masterKeyId); } catch (ProviderHelper.NotFoundException e) { // can't happen } - signature = sigList.get(signatureIndex); + // get the subkey which has been used to generate this signature + signatureKey = publicKeyRing.getPublicKey(signature.getKeyID()); signatureResultBuilder.knownKey(true); - signatureResultBuilder.userId(PgpKeyHelper.getMainUserId(signatureKey)); - signatureResultBuilder.keyId(signature.getKeyID()); + // TODO: uses the first pubkey for information + signatureResultBuilder.userId(PgpKeyHelper.getMainUserId(publicKeyRing.getPublicKey())); + signatureResultBuilder.keyId(publicKeyRing.getPublicKey().getKeyID()); JcaPGPContentVerifierBuilderProvider contentVerifierBuilderProvider = new JcaPGPContentVerifierBuilderProvider() @@ -449,8 +454,8 @@ public class PgpDecryptVerify { KeychainContract.KeyRings.buildUnifiedKeyRingUri(Long.toString(masterKeyId)), KeyRings.VERIFIED, ProviderHelper.FIELD_TYPE_INTEGER); - - isSignatureKeyCertified = ((Long) data > 0); + boolean isSignatureKeyCertified = ((Long) data > 0); + signatureResultBuilder.signatureKeyCertified(isSignatureKeyCertified); } else { // no key in our database -> return "unknown pub key" status including the first key id signatureResultBuilder.knownKey(false); @@ -529,7 +534,6 @@ public class PgpDecryptVerify { signatureResultBuilder.validSignature(validSignature); signatureResultBuilder.validKeyBinding(validKeyBinding); - signatureResultBuilder.signatureKeyCertified(isSignatureKeyCertified); } } @@ -622,22 +626,25 @@ public class PgpDecryptVerify { PGPSignature signature = null; PGPPublicKey signatureKey = null; - boolean isSignatureKeyCertified = false; if (masterKeyId != null) { // key found in our database! + signature = sigList.get(signatureIndex); + PGPPublicKeyRing publicKeyRing = null; try { - signatureKey = mProviderHelper - .getPGPPublicKeyRing(masterKeyId).getPublicKey(); + publicKeyRing = mProviderHelper + .getPGPPublicKeyRing(masterKeyId); } catch (ProviderHelper.NotFoundException e) { // can't happen } - signature = sigList.get(signatureIndex); + // get the subkey which has been used to generate this signature + signatureKey = publicKeyRing.getPublicKey(signature.getKeyID()); signatureResultBuilder.knownKey(true); - signatureResultBuilder.userId(PgpKeyHelper.getMainUserId(signatureKey)); - signatureResultBuilder.keyId(signature.getKeyID()); + // TODO: uses the first pubkey for information + signatureResultBuilder.userId(PgpKeyHelper.getMainUserId(publicKeyRing.getPublicKey())); + signatureResultBuilder.keyId(publicKeyRing.getPublicKey().getKeyID()); JcaPGPContentVerifierBuilderProvider contentVerifierBuilderProvider = new JcaPGPContentVerifierBuilderProvider() @@ -649,8 +656,8 @@ public class PgpDecryptVerify { KeychainContract.KeyRings.buildUnifiedKeyRingUri(Long.toString(masterKeyId)), KeyRings.VERIFIED, ProviderHelper.FIELD_TYPE_INTEGER); - - isSignatureKeyCertified = ((Long) data > 0); + boolean isSignatureKeyCertified = ((Long) data > 0); + signatureResultBuilder.signatureKeyCertified(isSignatureKeyCertified); } else { // no key in our database -> return "unknown pub key" status including the first key id signatureResultBuilder.knownKey(false); @@ -692,7 +699,6 @@ public class PgpDecryptVerify { signatureResultBuilder.validSignature(validSignature); signatureResultBuilder.validKeyBinding(validKeyBinding); - signatureResultBuilder.signatureKeyCertified(isSignatureKeyCertified); } result.setSignatureResult(signatureResultBuilder.build()); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyHelper.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyHelper.java index d36dd8efe..6450ad63d 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyHelper.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyHelper.java @@ -60,6 +60,34 @@ public class PgpKeyHelper { return key.getPublicKey().getCreationTime(); } + public static Date getExpiryDate(PGPPublicKey key) { + Date creationDate = getCreationDate(key); + if (key.getValidDays() == 0) { + // no expiry + return null; + } + Calendar calendar = GregorianCalendar.getInstance(); + calendar.setTime(creationDate); + calendar.add(Calendar.DATE, key.getValidDays()); + + return calendar.getTime(); + } + + public static Date getExpiryDate(PGPSecretKey key) { + return getExpiryDate(key.getPublicKey()); + } + + public static boolean isExpired(PGPPublicKey key) { + Date creationDate = getCreationDate(key); + Date expiryDate = getExpiryDate(key); + Date now = new Date(); + if (now.compareTo(creationDate) >= 0 + && (expiryDate == null || now.compareTo(expiryDate) <= 0)) { + return false; + } + return true; + } + @SuppressWarnings("unchecked") public static PGPSecretKey getKeyNum(PGPSecretKeyRing keyRing, long num) { long cnt = 0; @@ -77,7 +105,7 @@ public class PgpKeyHelper { } @SuppressWarnings("unchecked") - public static Vector getEncryptKeys(PGPPublicKeyRing keyRing) { + private static Vector getEncryptKeys(PGPPublicKeyRing keyRing) { Vector encryptKeys = new Vector(); for (PGPPublicKey key : new IterableIterator(keyRing.getPublicKeys())) { @@ -90,7 +118,7 @@ public class PgpKeyHelper { } @SuppressWarnings("unchecked") - public static Vector getSigningKeys(PGPSecretKeyRing keyRing) { + private static Vector getSigningKeys(PGPSecretKeyRing keyRing) { Vector signingKeys = new Vector(); for (PGPSecretKey key : new IterableIterator(keyRing.getSecretKeys())) { @@ -103,7 +131,7 @@ public class PgpKeyHelper { } @SuppressWarnings("unchecked") - public static Vector getCertificationKeys(PGPSecretKeyRing keyRing) { + private static Vector getCertificationKeys(PGPSecretKeyRing keyRing) { Vector signingKeys = new Vector(); for (PGPSecretKey key : new IterableIterator(keyRing.getSecretKeys())) { @@ -115,7 +143,7 @@ public class PgpKeyHelper { return signingKeys; } - public static Vector getUsableEncryptKeys(PGPPublicKeyRing keyRing) { + private static Vector getUsableEncryptKeys(PGPPublicKeyRing keyRing) { Vector usableKeys = new Vector(); Vector encryptKeys = getEncryptKeys(keyRing); PGPPublicKey masterKey = null; @@ -135,18 +163,7 @@ public class PgpKeyHelper { return usableKeys; } - public static boolean isExpired(PGPPublicKey key) { - Date creationDate = getCreationDate(key); - Date expiryDate = getExpiryDate(key); - Date now = new Date(); - if (now.compareTo(creationDate) >= 0 - && (expiryDate == null || now.compareTo(expiryDate) <= 0)) { - return false; - } - return true; - } - - public static Vector getUsableCertificationKeys(PGPSecretKeyRing keyRing) { + private static Vector getUsableCertificationKeys(PGPSecretKeyRing keyRing) { Vector usableKeys = new Vector(); Vector signingKeys = getCertificationKeys(keyRing); PGPSecretKey masterKey = null; @@ -164,7 +181,7 @@ public class PgpKeyHelper { return usableKeys; } - public static Vector getUsableSigningKeys(PGPSecretKeyRing keyRing) { + private static Vector getUsableSigningKeys(PGPSecretKeyRing keyRing) { Vector usableKeys = new Vector(); Vector signingKeys = getSigningKeys(keyRing); PGPSecretKey masterKey = null; @@ -182,22 +199,6 @@ public class PgpKeyHelper { return usableKeys; } - public static Date getExpiryDate(PGPPublicKey key) { - Date creationDate = getCreationDate(key); - if (key.getValidDays() == 0) { - // no expiry - return null; - } - Calendar calendar = GregorianCalendar.getInstance(); - calendar.setTime(creationDate); - calendar.add(Calendar.DATE, key.getValidDays()); - - return calendar.getTime(); - } - - public static Date getExpiryDate(PGPSecretKey key) { - return getExpiryDate(key.getPublicKey()); - } public static PGPPublicKey getFirstEncryptSubkey(PGPPublicKeyRing keyRing) { Vector encryptKeys = getUsableEncryptKeys(keyRing); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptAsymmetricFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptAsymmetricFragment.java index 7875358bd..26f437d8d 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptAsymmetricFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptAsymmetricFragment.java @@ -152,8 +152,8 @@ public class EncryptAsymmetricFragment extends Fragment { PGPSecretKey masterKey = keyRing.getSecretKey(); if (masterKey != null) { - Vector signKeys = PgpKeyHelper.getUsableSigningKeys(keyRing); - if (signKeys.size() > 0) { + PGPSecretKey signKey = PgpKeyHelper.getFirstSigningSubkey(keyRing); + if (signKey != null) { setSignatureKeyId(masterKey.getKeyID()); } } -- cgit v1.2.3