From 036480a210f0a2439a1c8dfbd06d32180fb26f8c Mon Sep 17 00:00:00 2001 From: Ashley Hughes Date: Mon, 13 Jan 2014 14:55:59 +0000 Subject: small tidy --- .../src/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'OpenPGP-Keychain/src/org/sufficientlysecure') diff --git a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java index e07c802b7..225167fce 100644 --- a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java +++ b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java @@ -378,7 +378,7 @@ public class PgpKeyOperation { keyFlags |= KeyFlags.SIGN_DATA; //cross-certify signing keys PGPContentSignerBuilder signerBuilder = new JcaPGPContentSignerBuilder( - subKey.getPublicKey().getAlgorithm(), PGPUtil.SHA1) + subPublicKey.getAlgorithm(), PGPUtil.SHA1) .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME); PGPSignatureGenerator sGen = new PGPSignatureGenerator(signerBuilder); sGen.init(PGPSignature.PRIMARYKEY_BINDING, subPrivateKey); -- cgit v1.2.3 From 2b71b12b24e6f5c8f8193513ae0d81fb159d519d Mon Sep 17 00:00:00 2001 From: Ashley Hughes Date: Tue, 14 Jan 2014 00:30:58 +0000 Subject: subkey binding check, start primary binding check --- .../keychain/pgp/PgpOperation.java | 48 +++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) (limited to 'OpenPGP-Keychain/src/org/sufficientlysecure') diff --git a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/pgp/PgpOperation.java b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/pgp/PgpOperation.java index 755537012..804d22187 100644 --- a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/pgp/PgpOperation.java +++ b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/pgp/PgpOperation.java @@ -34,6 +34,8 @@ import java.util.Iterator; import org.spongycastle.bcpg.ArmoredInputStream; import org.spongycastle.bcpg.ArmoredOutputStream; import org.spongycastle.bcpg.BCPGOutputStream; +import org.spongycastle.bcpg.SignatureSubpacket; +import org.spongycastle.bcpg.SignatureSubpacketTags; import org.spongycastle.openpgp.PGPCompressedData; import org.spongycastle.openpgp.PGPCompressedDataGenerator; import org.spongycastle.openpgp.PGPEncryptedData; @@ -56,6 +58,7 @@ import org.spongycastle.openpgp.PGPSignature; import org.spongycastle.openpgp.PGPSignatureGenerator; import org.spongycastle.openpgp.PGPSignatureList; import org.spongycastle.openpgp.PGPSignatureSubpacketGenerator; +import org.spongycastle.openpgp.PGPSignatureSubpacketVector; import org.spongycastle.openpgp.PGPUtil; import org.spongycastle.openpgp.PGPV3SignatureGenerator; import org.spongycastle.openpgp.operator.PBEDataDecryptorFactory; @@ -887,7 +890,50 @@ public class PgpOperation { } while (lookAhead != -1); } - returnData.putBoolean(KeychainIntentService.RESULT_SIGNATURE_SUCCESS, signature.verify()); + boolean sig_isok = signature.verify(); + + //We should only do the next part if the singing key was a subkey - not the master key! + signatureKeyId = signature.getKeyID(); + String userId = null; + PGPPublicKeyRing signKeyRing = ProviderHelper.getPGPPublicKeyRingByKeyId(mContext, + signatureKeyId); + PGPPublicKey mKey = null; + if (signKeyRing != null) { + mKey = PgpKeyHelper.getMasterKey(signKeyRing); + } + Iterator itr = signatureKey.getSignatures(); + + boolean subkeyBinding_isok = false; + boolean tmp_subkeyBinding_isok = false; + boolean primkeyBinding_isok = false; + 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() == mKey.getKeyID() && sig.getSignatureType() == PGPSignature.SUBKEY_BINDING) { + //check and if ok, check primary key binding. + sig.init(contentVerifierBuilderProvider, mKey); + tmp_subkeyBinding_isok = sig.verifyCertification(mKey, signatureKey); + if (tmp_subkeyBinding_isok) + subkeyBinding_isok = true; + if (tmp_subkeyBinding_isok) { + PGPSignatureSubpacketVector hPkts = sig.getHashedSubPackets(); + PGPSignatureSubpacketVector uhPkts = sig.getUnhashedSubPackets(); + if (hPkts.hasSubpacket(SignatureSubpacketTags.EMBEDDED_SIGNATURE)) { + SignatureSubpacket[] subsigpkts = hPkts.getSubpackets(SignatureSubpacketTags.EMBEDDED_SIGNATURE); + PGPSignature[] vals = new PGPSignature[subsigpkts.length]; + for (int i = 0; i < subsigpkts.length; i++) + { + vals[i] = (PGPSignature)subsigpkts[i]; + } + } + if (uhPkts.hasSubpacket(SignatureSubpacketTags.EMBEDDED_SIGNATURE)) { + } + } + } + } + returnData.putBoolean(KeychainIntentService.RESULT_SIGNATURE_SUCCESS, sig_isok & subkeyBinding_isok); updateProgress(R.string.progress_done, 100, 100); return returnData; -- cgit v1.2.3 From bb0baa815e224004a72bee5ff1fad46c530df5d8 Mon Sep 17 00:00:00 2001 From: Ashley Hughes Date: Tue, 14 Jan 2014 15:03:35 +0000 Subject: check primary bindings --- .../keychain/pgp/PgpOperation.java | 75 +++++++++++++--------- 1 file changed, 45 insertions(+), 30 deletions(-) (limited to 'OpenPGP-Keychain/src/org/sufficientlysecure') diff --git a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/pgp/PgpOperation.java b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/pgp/PgpOperation.java index 804d22187..6f0821fdc 100644 --- a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/pgp/PgpOperation.java +++ b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/pgp/PgpOperation.java @@ -33,7 +33,11 @@ import java.util.Iterator; import org.spongycastle.bcpg.ArmoredInputStream; import org.spongycastle.bcpg.ArmoredOutputStream; +import org.spongycastle.bcpg.BCPGInputStream; import org.spongycastle.bcpg.BCPGOutputStream; + +import org.spongycastle.bcpg.SignaturePacket; + import org.spongycastle.bcpg.SignatureSubpacket; import org.spongycastle.bcpg.SignatureSubpacketTags; import org.spongycastle.openpgp.PGPCompressedData; @@ -892,7 +896,11 @@ public class PgpOperation { boolean sig_isok = signature.verify(); - //We should only do the next part if the singing key was a subkey - not the master key! + //Now check binding signatures + boolean subkeyBinding_isok = false; + boolean tmp_subkeyBinding_isok = false; + boolean primkeyBinding_isok = false; + signatureKeyId = signature.getKeyID(); String userId = null; PGPPublicKeyRing signKeyRing = ProviderHelper.getPGPPublicKeyRingByKeyId(mContext, @@ -901,39 +909,46 @@ public class PgpOperation { if (signKeyRing != null) { mKey = PgpKeyHelper.getMasterKey(signKeyRing); } - Iterator itr = signatureKey.getSignatures(); - - boolean subkeyBinding_isok = false; - boolean tmp_subkeyBinding_isok = false; - boolean primkeyBinding_isok = false; - 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() == mKey.getKeyID() && sig.getSignatureType() == PGPSignature.SUBKEY_BINDING) { - //check and if ok, check primary key binding. - sig.init(contentVerifierBuilderProvider, mKey); - tmp_subkeyBinding_isok = sig.verifyCertification(mKey, signatureKey); - if (tmp_subkeyBinding_isok) - subkeyBinding_isok = true; - if (tmp_subkeyBinding_isok) { - PGPSignatureSubpacketVector hPkts = sig.getHashedSubPackets(); - PGPSignatureSubpacketVector uhPkts = sig.getUnhashedSubPackets(); - if (hPkts.hasSubpacket(SignatureSubpacketTags.EMBEDDED_SIGNATURE)) { - SignatureSubpacket[] subsigpkts = hPkts.getSubpackets(SignatureSubpacketTags.EMBEDDED_SIGNATURE); - PGPSignature[] vals = new PGPSignature[subsigpkts.length]; - for (int i = 0; i < subsigpkts.length; i++) - { - vals[i] = (PGPSignature)subsigpkts[i]; - } - } - if (uhPkts.hasSubpacket(SignatureSubpacketTags.EMBEDDED_SIGNATURE)) { + if (signature.getKeyID() != mKey.getKeyID()) { + Iterator itr = signatureKey.getSignatures(); + + subkeyBinding_isok = false; + tmp_subkeyBinding_isok = false; + primkeyBinding_isok = false; + 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() == mKey.getKeyID() && sig.getSignatureType() == PGPSignature.SUBKEY_BINDING) { + //check and if ok, check primary key binding. + sig.init(contentVerifierBuilderProvider, mKey); + tmp_subkeyBinding_isok = sig.verifyCertification(mKey, signatureKey); + if (tmp_subkeyBinding_isok) + subkeyBinding_isok = true; + if (tmp_subkeyBinding_isok) { + PGPSignatureSubpacketVector hPkts = sig.getHashedSubPackets(); + PGPSignatureSubpacketVector uhPkts = sig.getUnhashedSubPackets(); + if (uhPkts.hasSubpacket(SignatureSubpacketTags.EMBEDDED_SIGNATURE)) { + PGPSignatureList eSigList = uhPkts.getEmbeddedSignatures(); + for (int j = 0; j < eSigList.size(); ++j) { + PGPSignature emSig = eSigList.get(j); + emSig.init(contentVerifierBuilderProvider, signatureKey); + primkeyBinding_isok = emSig.verifyCertification(mKey, signatureKey); + if (primkeyBinding_isok) + break; + } + } + if (hPkts.hasSubpacket(SignatureSubpacketTags.EMBEDDED_SIGNATURE)) { + } } } } + } else { //if the key used to make the signature was the master key, no need to check binding sigs + subkeyBinding_isok = true; + primkeyBinding_isok = true; } - returnData.putBoolean(KeychainIntentService.RESULT_SIGNATURE_SUCCESS, sig_isok & subkeyBinding_isok); + returnData.putBoolean(KeychainIntentService.RESULT_SIGNATURE_SUCCESS, sig_isok & subkeyBinding_isok & primkeyBinding_isok); updateProgress(R.string.progress_done, 100, 100); return returnData; -- cgit v1.2.3 From cd4a3dd2377d64f73da08711007e89c4733f9cef Mon Sep 17 00:00:00 2001 From: Ashley Hughes Date: Tue, 14 Jan 2014 15:48:05 +0000 Subject: begin refactor --- .../keychain/pgp/PgpOperation.java | 54 ++++++++++++++++------ 1 file changed, 40 insertions(+), 14 deletions(-) (limited to 'OpenPGP-Keychain/src/org/sufficientlysecure') diff --git a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/pgp/PgpOperation.java b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/pgp/PgpOperation.java index 6f0821fdc..1525c9462 100644 --- a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/pgp/PgpOperation.java +++ b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/pgp/PgpOperation.java @@ -927,20 +927,12 @@ public class PgpOperation { if (tmp_subkeyBinding_isok) subkeyBinding_isok = true; if (tmp_subkeyBinding_isok) { - PGPSignatureSubpacketVector hPkts = sig.getHashedSubPackets(); - PGPSignatureSubpacketVector uhPkts = sig.getUnhashedSubPackets(); - if (uhPkts.hasSubpacket(SignatureSubpacketTags.EMBEDDED_SIGNATURE)) { - PGPSignatureList eSigList = uhPkts.getEmbeddedSignatures(); - for (int j = 0; j < eSigList.size(); ++j) { - PGPSignature emSig = eSigList.get(j); - emSig.init(contentVerifierBuilderProvider, signatureKey); - primkeyBinding_isok = emSig.verifyCertification(mKey, signatureKey); - if (primkeyBinding_isok) - break; - } - } - if (hPkts.hasSubpacket(SignatureSubpacketTags.EMBEDDED_SIGNATURE)) { - } + primkeyBinding_isok = verifyPrimaryBinding(sig.getUnhashedSubPackets(), mKey, signatureKey); + if (primkeyBinding_isok) + break; + primkeyBinding_isok = verifyPrimaryBinding(sig.getHashedSubPackets(), mKey, signatureKey); + if (primkeyBinding_isok) + break; } } } @@ -954,6 +946,40 @@ public class PgpOperation { return returnData; } + private boolean verifyPrimaryBinding(PGPSignatureSubpacketVector Pkts, PGPPublicKey masterPublicKey, PGPPublicKey signingPublicKey) + { + boolean primkeyBinding_isok = 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); + primkeyBinding_isok = emSig.verifyCertification(masterPublicKey, signingPublicKey); + if (primkeyBinding_isok) + break; + } catch (PGPException e) { + continue; + } catch (SignatureException e) { + continue; + } + } + } + } + return primkeyBinding_isok; + } + private static void processLine(final String pLine, final ArmoredOutputStream pArmoredOutput, final PGPSignatureGenerator pSignatureGenerator) throws IOException, SignatureException { -- cgit v1.2.3 From c95a52c07041371f54d6315bddc8a60fcac69245 Mon Sep 17 00:00:00 2001 From: Ashley Hughes Date: Tue, 14 Jan 2014 22:12:57 +0000 Subject: refactor --- .../keychain/pgp/PgpOperation.java | 80 +++++++++++++--------- 1 file changed, 48 insertions(+), 32 deletions(-) (limited to 'OpenPGP-Keychain/src/org/sufficientlysecure') diff --git a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/pgp/PgpOperation.java b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/pgp/PgpOperation.java index 1525c9462..90934cbd9 100644 --- a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/pgp/PgpOperation.java +++ b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/pgp/PgpOperation.java @@ -897,9 +897,7 @@ public class PgpOperation { boolean sig_isok = signature.verify(); //Now check binding signatures - boolean subkeyBinding_isok = false; - boolean tmp_subkeyBinding_isok = false; - boolean primkeyBinding_isok = false; + boolean keyBinding_isok = false; signatureKeyId = signature.getKeyID(); String userId = null; @@ -910,42 +908,60 @@ public class PgpOperation { mKey = PgpKeyHelper.getMasterKey(signKeyRing); } if (signature.getKeyID() != mKey.getKeyID()) { - Iterator itr = signatureKey.getSignatures(); - - subkeyBinding_isok = false; - tmp_subkeyBinding_isok = false; - primkeyBinding_isok = false; - 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() == mKey.getKeyID() && sig.getSignatureType() == PGPSignature.SUBKEY_BINDING) { - //check and if ok, check primary key binding. - sig.init(contentVerifierBuilderProvider, mKey); - tmp_subkeyBinding_isok = sig.verifyCertification(mKey, signatureKey); - if (tmp_subkeyBinding_isok) - subkeyBinding_isok = true; - if (tmp_subkeyBinding_isok) { - primkeyBinding_isok = verifyPrimaryBinding(sig.getUnhashedSubPackets(), mKey, signatureKey); - if (primkeyBinding_isok) - break; - primkeyBinding_isok = verifyPrimaryBinding(sig.getHashedSubPackets(), mKey, signatureKey); - if (primkeyBinding_isok) - break; - } - } - } + keyBinding_isok = verifyKeyBinding(mKey, signatureKey); } else { //if the key used to make the signature was the master key, no need to check binding sigs - subkeyBinding_isok = true; - primkeyBinding_isok = true; + keyBinding_isok = true; } - returnData.putBoolean(KeychainIntentService.RESULT_SIGNATURE_SUCCESS, sig_isok & subkeyBinding_isok & primkeyBinding_isok); + returnData.putBoolean(KeychainIntentService.RESULT_SIGNATURE_SUCCESS, sig_isok & keyBinding_isok); updateProgress(R.string.progress_done, 100, 100); return returnData; } + private boolean verifyKeyBinding(PGPPublicKey masterPublicKey, PGPPublicKey signingPublicKey) + { + boolean subkeyBinding_isok = false; + boolean tmp_subkeyBinding_isok = false; + boolean primkeyBinding_isok = false; + JcaPGPContentVerifierBuilderProvider contentVerifierBuilderProvider = new JcaPGPContentVerifierBuilderProvider() + .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME); + + Iterator itr = signingPublicKey.getSignatures(); + + subkeyBinding_isok = false; + tmp_subkeyBinding_isok = false; + primkeyBinding_isok = false; + 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() == masterPublicKey.getKeyID() && sig.getSignatureType() == PGPSignature.SUBKEY_BINDING) { + //check and if ok, check primary key binding. + try { + sig.init(contentVerifierBuilderProvider, masterPublicKey); + tmp_subkeyBinding_isok = sig.verifyCertification(masterPublicKey, signingPublicKey); + } catch (PGPException e) { + continue; + } catch (SignatureException e) { + continue; + } + + if (tmp_subkeyBinding_isok) + subkeyBinding_isok = true; + if (tmp_subkeyBinding_isok) { + primkeyBinding_isok = verifyPrimaryBinding(sig.getUnhashedSubPackets(), masterPublicKey, signingPublicKey); + if (primkeyBinding_isok) + break; + primkeyBinding_isok = verifyPrimaryBinding(sig.getHashedSubPackets(), masterPublicKey, signingPublicKey); + if (primkeyBinding_isok) + break; + } + } + } + return (subkeyBinding_isok & primkeyBinding_isok); + } + private boolean verifyPrimaryBinding(PGPSignatureSubpacketVector Pkts, PGPPublicKey masterPublicKey, PGPPublicKey signingPublicKey) { boolean primkeyBinding_isok = false; -- cgit v1.2.3 From 0bca0a4b08fc0be62c64d7f8b8185cf6db620ead Mon Sep 17 00:00:00 2001 From: Ashley Hughes Date: Wed, 15 Jan 2014 00:41:18 +0000 Subject: always check binding when verifying --- .../keychain/pgp/PgpOperation.java | 30 +++++++++++++--------- 1 file changed, 18 insertions(+), 12 deletions(-) (limited to 'OpenPGP-Keychain/src/org/sufficientlysecure') diff --git a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/pgp/PgpOperation.java b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/pgp/PgpOperation.java index 90934cbd9..de1973702 100644 --- a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/pgp/PgpOperation.java +++ b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/pgp/PgpOperation.java @@ -764,11 +764,11 @@ public class PgpOperation { PGPSignatureList signatureList = (PGPSignatureList) plainFact.nextObject(); PGPSignature messageSignature = signatureList.get(signatureIndex); - if (signature.verify(messageSignature)) { - returnData.putBoolean(KeychainIntentService.RESULT_SIGNATURE_SUCCESS, true); - } else { - returnData.putBoolean(KeychainIntentService.RESULT_SIGNATURE_SUCCESS, false); - } + + //Now check binding signatures + boolean keyBinding_isok = verifyKeyBinding(mContext, messageSignature, signatureKey); + boolean sig_isok = signature.verify(messageSignature); + returnData.putBoolean(KeychainIntentService.RESULT_SIGNATURE_SUCCESS, keyBinding_isok & sig_isok); } } @@ -897,9 +897,18 @@ public class PgpOperation { boolean sig_isok = signature.verify(); //Now check binding signatures - boolean keyBinding_isok = false; + boolean keyBinding_isok = verifyKeyBinding(mContext, signature, signatureKey); + + returnData.putBoolean(KeychainIntentService.RESULT_SIGNATURE_SUCCESS, sig_isok & keyBinding_isok); + + updateProgress(R.string.progress_done, 100, 100); + return returnData; + } - signatureKeyId = signature.getKeyID(); + public boolean verifyKeyBinding(Context mContext, PGPSignature signature, PGPPublicKey signatureKey) + { + long signatureKeyId = signature.getKeyID(); + boolean keyBinding_isok = false; String userId = null; PGPPublicKeyRing signKeyRing = ProviderHelper.getPGPPublicKeyRingByKeyId(mContext, signatureKeyId); @@ -912,13 +921,10 @@ public class PgpOperation { } else { //if the key used to make the signature was the master key, no need to check binding sigs keyBinding_isok = true; } - returnData.putBoolean(KeychainIntentService.RESULT_SIGNATURE_SUCCESS, sig_isok & keyBinding_isok); - - updateProgress(R.string.progress_done, 100, 100); - return returnData; + return keyBinding_isok; } - private boolean verifyKeyBinding(PGPPublicKey masterPublicKey, PGPPublicKey signingPublicKey) + public boolean verifyKeyBinding(PGPPublicKey masterPublicKey, PGPPublicKey signingPublicKey) { boolean subkeyBinding_isok = false; boolean tmp_subkeyBinding_isok = false; -- cgit v1.2.3