From b40b429bc0db920e36351a8fd4189e473dc554c5 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Wed, 18 Jun 2014 22:07:14 +0200 Subject: remove obsolete subkey binding check from signature verification --- .../pgp/OpenPgpSignatureResultBuilder.java | 10 +- .../keychain/pgp/PgpDecryptVerify.java | 12 --- .../keychain/pgp/WrappedPublicKeyRing.java | 105 +-------------------- 3 files changed, 3 insertions(+), 124 deletions(-) (limited to 'OpenKeychain/src/main/java/org/sufficientlysecure/keychain') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OpenPgpSignatureResultBuilder.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OpenPgpSignatureResultBuilder.java index 5e49497c0..75f8bdb66 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OpenPgpSignatureResultBuilder.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OpenPgpSignatureResultBuilder.java @@ -35,7 +35,6 @@ public class OpenPgpSignatureResultBuilder { private boolean mSignatureAvailable = false; private boolean mKnownKey = false; private boolean mValidSignature = false; - private boolean mValidKeyBinding = false; private boolean mIsSignatureKeyCertified = false; public void signatureOnly(boolean signatureOnly) { @@ -58,10 +57,6 @@ public class OpenPgpSignatureResultBuilder { this.mValidSignature = validSignature; } - public void validKeyBinding(boolean validKeyBinding) { - this.mValidKeyBinding = validKeyBinding; - } - public void signatureKeyCertified(boolean isSignatureKeyCertified) { this.mIsSignatureKeyCertified = isSignatureKeyCertified; } @@ -77,7 +72,7 @@ public class OpenPgpSignatureResultBuilder { // valid sig! if (mKnownKey) { - if (mValidKeyBinding && mValidSignature) { + if (mValidSignature) { result.setKeyId(mKeyId); result.setUserId(mUserId); @@ -89,8 +84,7 @@ public class OpenPgpSignatureResultBuilder { result.setStatus(OpenPgpSignatureResult.SIGNATURE_SUCCESS_UNCERTIFIED); } } else { - Log.d(Constants.TAG, "Error!\nvalidKeyBinding: " + mValidKeyBinding - + "\nvalidSignature: " + mValidSignature); + Log.d(Constants.TAG, "Error! Invalid signature."); result.setStatus(OpenPgpSignatureResult.SIGNATURE_ERROR); } } else { 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 c009d1b5c..a5ccfbd3b 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerify.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerify.java @@ -122,9 +122,6 @@ public class PgpDecryptVerify { /** * Allow these key ids alone for decryption. * This means only ciphertexts encrypted for one of these private key can be decrypted. - * - * @param allowedKeyIds - * @return */ public Builder setAllowedKeyIds(Set allowedKeyIds) { this.mAllowedKeyIds = allowedKeyIds; @@ -496,10 +493,7 @@ public class PgpDecryptVerify { // Verify signature and check binding signatures boolean validSignature = signature.verify(messageSignature); - boolean validKeyBinding = signingRing.verifySubkeyBinding(signingKey); - signatureResultBuilder.validSignature(validSignature); - signatureResultBuilder.validKeyBinding(validKeyBinding); } } @@ -643,10 +637,8 @@ public class PgpDecryptVerify { // Verify signature and check binding signatures boolean validSignature = signature.verify(); - boolean validKeyBinding = signingRing.verifySubkeyBinding(signingKey); signatureResultBuilder.validSignature(validSignature); - signatureResultBuilder.validKeyBinding(validKeyBinding); } result.setSignatureResult(signatureResultBuilder.build()); @@ -657,10 +649,6 @@ public class PgpDecryptVerify { /** * Mostly taken from ClearSignedFileProcessor in Bouncy Castle - * - * @param sig - * @param line - * @throws SignatureException */ private static void processLine(PGPSignature sig, byte[] line) throws SignatureException { 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 0bb84aee7..b2abf15a4 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedPublicKeyRing.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedPublicKeyRing.java @@ -1,24 +1,16 @@ 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; -import org.spongycastle.openpgp.PGPSignatureList; -import org.spongycastle.openpgp.PGPSignatureSubpacketVector; -import org.spongycastle.openpgp.operator.jcajce.JcaPGPContentVerifierBuilderProvider; 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; -import java.util.Arrays; import java.util.Iterator; public class WrappedPublicKeyRing extends WrappedKeyRing { @@ -70,106 +62,11 @@ public class WrappedPublicKeyRing extends WrappedKeyRing { } return cKey; } - // TODO handle with proper exception throw new PgpGeneralException("no encryption key available"); } - public boolean verifySubkeyBinding(WrappedPublicKey cachedSubkey) { - boolean validSubkeyBinding = false; - boolean validTempSubkeyBinding = false; - boolean validPrimaryKeyBinding = false; - - PGPPublicKey masterKey = getRing().getPublicKey(); - PGPPublicKey subKey = cachedSubkey.getPublicKey(); - - // Is this the master key? Match automatically, then. - if(Arrays.equals(masterKey.getFingerprint(), subKey.getFingerprint())) { - return true; - } - - JcaPGPContentVerifierBuilderProvider contentVerifierBuilderProvider = - new JcaPGPContentVerifierBuilderProvider() - .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME); - - Iterator itr = subKey.getSignatures(); - - while (itr.hasNext()) { //what does gpg do if the subkey binding is wrong? - //gpg has an invalid subkey binding error on key import I think, but doesn't shout - //about keys without subkey signing. Can't get it to import a slightly broken one - //either, so we will err on bad subkey binding here. - PGPSignature sig = itr.next(); - if (sig.getKeyID() == masterKey.getKeyID() && - sig.getSignatureType() == PGPSignature.SUBKEY_BINDING) { - //check and if ok, check primary key binding. - try { - sig.init(contentVerifierBuilderProvider, masterKey); - validTempSubkeyBinding = sig.verifyCertification(masterKey, subKey); - } catch (PGPException e) { - continue; - } catch (SignatureException e) { - continue; - } - - if (validTempSubkeyBinding) { - validSubkeyBinding = true; - } - if (validTempSubkeyBinding) { - validPrimaryKeyBinding = verifyPrimaryKeyBinding(sig.getUnhashedSubPackets(), - masterKey, subKey); - if (validPrimaryKeyBinding) { - break; - } - validPrimaryKeyBinding = verifyPrimaryKeyBinding(sig.getHashedSubPackets(), - masterKey, subKey); - if (validPrimaryKeyBinding) { - break; - } - } - } - } - return validSubkeyBinding && validPrimaryKeyBinding; - - } - - static boolean verifyPrimaryKeyBinding(PGPSignatureSubpacketVector pkts, - PGPPublicKey masterPublicKey, - PGPPublicKey signingPublicKey) { - boolean validPrimaryKeyBinding = false; - JcaPGPContentVerifierBuilderProvider contentVerifierBuilderProvider = - new JcaPGPContentVerifierBuilderProvider() - .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME); - PGPSignatureList eSigList; - - if (pkts.hasSubpacket(SignatureSubpacketTags.EMBEDDED_SIGNATURE)) { - try { - eSigList = pkts.getEmbeddedSignatures(); - } catch (IOException e) { - return false; - } catch (PGPException e) { - return false; - } - for (int j = 0; j < eSigList.size(); ++j) { - PGPSignature emSig = eSigList.get(j); - if (emSig.getSignatureType() == PGPSignature.PRIMARYKEY_BINDING) { - try { - emSig.init(contentVerifierBuilderProvider, signingPublicKey); - validPrimaryKeyBinding = emSig.verifyCertification(masterPublicKey, signingPublicKey); - if (validPrimaryKeyBinding) { - break; - } - } catch (PGPException e) { - continue; - } catch (SignatureException e) { - continue; - } - } - } - } - - return validPrimaryKeyBinding; - } - public IterableIterator publicKeyIterator() { + @SuppressWarnings("unchecked") final Iterator it = getRing().getPublicKeys(); return new IterableIterator(new Iterator() { @Override -- cgit v1.2.3