From aaddd26b51a9d3a8e230a704cb33de8e9a5bcbef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Sch=C3=BCrmann?= Date: Fri, 21 Feb 2014 02:31:30 +0100 Subject: [PATCH] verification of cleartext signatures works --- .../demo/OpenPgpProviderActivity.java | 19 +++++ .../keychain/pgp/PgpOperationIncoming.java | 77 +++++++++++++++---- .../keychain/pgp/PgpOperationOutgoing.java | 16 +++- .../service/KeychainIntentService.java | 17 ++-- .../service/remote/OpenPgpService.java | 39 +++++----- .../keychain/ui/DecryptActivity.java | 6 +- 6 files changed, 125 insertions(+), 49 deletions(-) diff --git a/OpenPGP-Keychain-API/example-app/src/main/java/org/sufficientlysecure/keychain/demo/OpenPgpProviderActivity.java b/OpenPGP-Keychain-API/example-app/src/main/java/org/sufficientlysecure/keychain/demo/OpenPgpProviderActivity.java index ef40ee5ac..2f7f085c2 100644 --- a/OpenPGP-Keychain-API/example-app/src/main/java/org/sufficientlysecure/keychain/demo/OpenPgpProviderActivity.java +++ b/OpenPGP-Keychain-API/example-app/src/main/java/org/sufficientlysecure/keychain/demo/OpenPgpProviderActivity.java @@ -31,6 +31,7 @@ import android.widget.EditText; import android.widget.Toast; import org.openintents.openpgp.OpenPgpError; +import org.openintents.openpgp.OpenPgpSignatureResult; import org.openintents.openpgp.util.OpenPgpApi; import org.openintents.openpgp.util.OpenPgpConstants; import org.openintents.openpgp.util.OpenPgpServiceConnection; @@ -121,6 +122,18 @@ public class OpenPgpProviderActivity extends Activity { }); } + private void handleSignature(final OpenPgpSignatureResult sigResult) { + runOnUiThread(new Runnable() { + + @Override + public void run() { + Toast.makeText(OpenPgpProviderActivity.this, + sigResult.toString(), + Toast.LENGTH_LONG).show(); + } + }); + } + /** * Takes input from message or ciphertext EditText and turns it into a ByteArrayInputStream * @@ -171,6 +184,12 @@ public class OpenPgpProviderActivity extends Activity { } catch (UnsupportedEncodingException e) { Log.e(Constants.TAG, "UnsupportedEncodingException", e); } + + if (result.containsKey(OpenPgpConstants.RESULT_SIGNATURE)) { + OpenPgpSignatureResult sigResult + = result.getParcelable(OpenPgpConstants.RESULT_SIGNATURE); + handleSignature(sigResult); + } break; } case OpenPgpConstants.RESULT_CODE_USER_INTERACTION_REQUIRED: { diff --git a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpOperationIncoming.java b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpOperationIncoming.java index 3f0cb0d1a..7f78f6b07 100644 --- a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpOperationIncoming.java +++ b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpOperationIncoming.java @@ -168,15 +168,37 @@ public class PgpOperationIncoming { return false; } - public Bundle decryptAndVerify() + /** + * Decrypts and/or verifies data based on parameters of class + * + * @return + * @throws IOException + * @throws PgpGeneralException + * @throws PGPException + * @throws SignatureException + */ + public Bundle decryptVerify() throws IOException, PgpGeneralException, PGPException, SignatureException { - Bundle returnData = new Bundle(); + // automatically works with ascii armor input and binary InputStream in = PGPUtil.getDecoderStream(data.getInputStream()); + if (in instanceof ArmoredInputStream) { + ArmoredInputStream aIn = (ArmoredInputStream) in; + // it is ascii armored + Log.d(Constants.TAG, "ASCII Armor Header Line: " + aIn.getArmorHeaderLine()); + + if (aIn.isClearText()) { + // a cleartext signature, verify it with the other method + return verifyCleartextSignature(aIn); + } else { + // go on... + } + } PGPObjectFactory pgpF = new PGPObjectFactory(in); PGPEncryptedDataList enc; Object o = pgpF.nextObject(); + Log.d(Constants.TAG, "o: " + o.getClass().getName()); int currentProgress = 0; updateProgress(R.string.progress_reading_data, currentProgress, 100); @@ -345,7 +367,6 @@ public class PgpOperationIncoming { updateProgress(R.string.progress_decrypting, currentProgress, 100); PGPLiteralData literalData = (PGPLiteralData) dataChunk; - OutputStream out = outStream; byte[] buffer = new byte[1 << 16]; InputStream dataIn = literalData.getInputStream(); @@ -359,10 +380,11 @@ public class PgpOperationIncoming { } int n; + // TODO: progress calculation is broken here! Try to rework it based on commented code! // int progress = 0; long startPos = data.getStreamPosition(); while ((n = dataIn.read(buffer)) > 0) { - out.write(buffer, 0, n); + outStream.write(buffer, 0, n); // progress += n; if (signature != null) { try { @@ -392,9 +414,13 @@ public class PgpOperationIncoming { PGPSignatureList signatureList = (PGPSignatureList) plainFact.nextObject(); PGPSignature messageSignature = signatureList.get(signatureIndex); + // these are not cleartext signatures! + returnData.putBoolean(KeychainIntentService.RESULT_CLEARTEXT_SIGNATURE_ONLY, false); + //Now check binding signatures boolean keyBinding_isok = verifyKeyBinding(context, messageSignature, signatureKey); boolean sig_isok = signature.verify(messageSignature); + returnData.putBoolean(KeychainIntentService.RESULT_SIGNATURE_SUCCESS, keyBinding_isok & sig_isok); } } @@ -420,17 +446,29 @@ public class PgpOperationIncoming { return returnData; } - // TODO: merge into decryptAndVerify by checking what the input is - public Bundle verifyText() throws IOException, PgpGeneralException, - PGPException, SignatureException { + /** + * This method verifies cleartext signatures + * as defined in http://tools.ietf.org/html/rfc4880#section-7 + *

+ * The method is heavily based on + * pg/src/main/java/org/spongycastle/openpgp/examples/ClearSignedFileProcessor.java + * + * @return + * @throws IOException + * @throws PgpGeneralException + * @throws PGPException + * @throws SignatureException + */ + private Bundle verifyCleartextSignature(ArmoredInputStream aIn) + throws IOException, PgpGeneralException, PGPException, SignatureException { Bundle returnData = new Bundle(); + // cleartext signatures are never encrypted ;) + returnData.putBoolean(KeychainIntentService.RESULT_CLEARTEXT_SIGNATURE_ONLY, true); ByteArrayOutputStream out = new ByteArrayOutputStream(); - ArmoredInputStream aIn = new ArmoredInputStream(data.getInputStream()); updateProgress(R.string.progress_done, 0, 100); - // mostly taken from pg/src/main/java/org/spongycastle/openpgp/examples/ClearSignedFileProcessor.java ByteArrayOutputStream lineOut = new ByteArrayOutputStream(); int lookAhead = readInputLine(lineOut, aIn); byte[] lineSep = getLineSeparator(); @@ -489,13 +527,13 @@ public class PgpOperationIncoming { if (signature == null) { returnData.putBoolean(KeychainIntentService.RESULT_SIGNATURE_UNKNOWN, true); - if (progress != null) - progress.setProgress(R.string.progress_done, 100, 100); + updateProgress(R.string.progress_done, 100, 100); return returnData; } - JcaPGPContentVerifierBuilderProvider contentVerifierBuilderProvider = new JcaPGPContentVerifierBuilderProvider() - .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME); + JcaPGPContentVerifierBuilderProvider contentVerifierBuilderProvider = + new JcaPGPContentVerifierBuilderProvider() + .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME); signature.init(contentVerifierBuilderProvider, signatureKey); @@ -527,11 +565,11 @@ public class PgpOperationIncoming { return returnData; } - private static boolean verifyKeyBinding(Context mContext, PGPSignature signature, PGPPublicKey signatureKey) { + private static boolean verifyKeyBinding(Context context, PGPSignature signature, PGPPublicKey signatureKey) { long signatureKeyId = signature.getKeyID(); boolean keyBinding_isok = false; String userId = null; - PGPPublicKeyRing signKeyRing = ProviderHelper.getPGPPublicKeyRingByKeyId(mContext, + PGPPublicKeyRing signKeyRing = ProviderHelper.getPGPPublicKeyRingByKeyId(context, signatureKeyId); PGPPublicKey mKey = null; if (signKeyRing != null) { @@ -621,7 +659,14 @@ public class PgpOperationIncoming { return primkeyBinding_isok; } - // taken from ClearSignedFileProcessor in BC + /** + * Mostly taken from ClearSignedFileProcessor in Bouncy Castle + * + * @param sig + * @param line + * @throws SignatureException + * @throws IOException + */ private static void processLine(PGPSignature sig, byte[] line) throws SignatureException, IOException { int length = getLengthWithoutWhiteSpace(line); diff --git a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpOperationOutgoing.java b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpOperationOutgoing.java index 044fe5aad..fd859fd5a 100644 --- a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpOperationOutgoing.java +++ b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpOperationOutgoing.java @@ -187,7 +187,17 @@ public class PgpOperationOutgoing { } } - public void signAndEncrypt() + /** + * Signs and/or encrypts data based on parameters of class + * + * @throws IOException + * @throws PgpGeneralException + * @throws PGPException + * @throws NoSuchProviderException + * @throws NoSuchAlgorithmException + * @throws SignatureException + */ + public void signEncrypt() throws IOException, PgpGeneralException, PGPException, NoSuchProviderException, NoSuchAlgorithmException, SignatureException { @@ -383,6 +393,8 @@ public class PgpOperationOutgoing { } armorOut.write(newline); + + // update signature buffer with input line if (signatureForceV3) { signatureV3Generator.update(newline); processLine(line, armorOut, signatureV3Generator); @@ -430,7 +442,7 @@ public class PgpOperationOutgoing { updateProgress(R.string.progress_done, 100, 100); } - // TODO: merge this into signAndEncrypt method! + // TODO: merge this into signEncrypt method! // TODO: allow binary input for this class public void generateSignature() throws PgpGeneralException, PGPException, IOException, NoSuchAlgorithmException, diff --git a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java index 422ce010c..4ce1569f0 100644 --- a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java +++ b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java @@ -120,7 +120,6 @@ public class KeychainIntentService extends IntentService implements ProgressDial public static final String ENCRYPT_PROVIDER_URI = "provider_uri"; // decrypt/verify - public static final String DECRYPT_SIGNED_ONLY = "signed_only"; public static final String DECRYPT_RETURN_BYTES = "return_binary"; public static final String DECRYPT_CIPHERTEXT_BYTES = "ciphertext_bytes"; public static final String DECRYPT_ASSUME_SYMMETRIC = "assume_symmetric"; @@ -185,6 +184,7 @@ public class KeychainIntentService extends IntentService implements ProgressDial public static final String RESULT_SIGNATURE = "signature"; public static final String RESULT_SIGNATURE_KEY_ID = "signature_key_id"; public static final String RESULT_SIGNATURE_USER_ID = "signature_user_id"; + public static final String RESULT_CLEARTEXT_SIGNATURE_ONLY = "signature_only"; public static final String RESULT_SIGNATURE_SUCCESS = "signature_success"; public static final String RESULT_SIGNATURE_UNKNOWN = "signature_unknown"; @@ -338,7 +338,7 @@ public class KeychainIntentService extends IntentService implements ProgressDial .signatureHashAlgorithm(Preferences.getPreferences(this).getDefaultHashAlgorithm()) .signaturePassphrase(PassphraseCacheService.getCachedPassphrase(this, secretKeyId)); - builder.build().signAndEncrypt(); + builder.build().signEncrypt(); } else { Log.d(Constants.TAG, "encrypt..."); builder.enableAsciiArmorOutput(useAsciiArmor) @@ -351,7 +351,7 @@ public class KeychainIntentService extends IntentService implements ProgressDial .signatureHashAlgorithm(Preferences.getPreferences(this).getDefaultHashAlgorithm()) .signaturePassphrase(PassphraseCacheService.getCachedPassphrase(this, secretKeyId)); - builder.build().signAndEncrypt(); + builder.build().signEncrypt(); } outStream.close(); @@ -404,7 +404,6 @@ public class KeychainIntentService extends IntentService implements ProgressDial long secretKeyId = data.getLong(ENCRYPT_SECRET_KEY_ID); byte[] bytes = data.getByteArray(DECRYPT_CIPHERTEXT_BYTES); - boolean signedOnly = data.getBoolean(DECRYPT_SIGNED_ONLY); boolean returnBytes = data.getBoolean(DECRYPT_RETURN_BYTES); boolean assumeSymmetricEncryption = data.getBoolean(DECRYPT_ASSUME_SYMMETRIC); @@ -484,14 +483,10 @@ public class KeychainIntentService extends IntentService implements ProgressDial PgpOperationIncoming.Builder builder = new PgpOperationIncoming.Builder(this, inputData, outStream); builder.progress(this); - if (signedOnly) { - resultData = builder.build().verifyText(); - } else { - builder.assumeSymmetric(assumeSymmetricEncryption) - .passphrase(PassphraseCacheService.getCachedPassphrase(this, secretKeyId)); + builder.assumeSymmetric(assumeSymmetricEncryption) + .passphrase(PassphraseCacheService.getCachedPassphrase(this, secretKeyId)); - resultData = builder.build().decryptAndVerify(); - } + resultData = builder.build().decryptVerify(); outStream.close(); diff --git a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/service/remote/OpenPgpService.java b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/service/remote/OpenPgpService.java index 74e311294..688537be5 100644 --- a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/service/remote/OpenPgpService.java +++ b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/service/remote/OpenPgpService.java @@ -168,7 +168,7 @@ public class OpenPgpService extends RemoteService { .signatureForceV3(false) .signatureKeyId(appSettings.getKeyId()) .signaturePassphrase(passphrase); - builder.build().signAndEncrypt(); + builder.build().signEncrypt(); } finally { is.close(); os.close(); @@ -257,7 +257,7 @@ public class OpenPgpService extends RemoteService { builder.signatureKeyId(Id.key.none); } // execute PGP operation! - builder.build().signAndEncrypt(); + builder.build().signEncrypt(); } finally { is.close(); os.close(); @@ -297,7 +297,7 @@ public class OpenPgpService extends RemoteService { // checked if it is text with BEGIN and END tags // String message = new String(inputBytes); // Log.d(Constants.TAG, "in: " + message); - boolean signedOnly = false; +// boolean signedOnly = false; // Matcher matcher = PgpHelper.PGP_MESSAGE.matcher(message); // if (matcher.matches()) { // Log.d(Constants.TAG, "PGP_MESSAGE matched"); @@ -386,35 +386,37 @@ public class OpenPgpService extends RemoteService { Bundle outputBundle; PgpOperationIncoming.Builder builder = new PgpOperationIncoming.Builder(this, inputData, os); - if (signedOnly) { - outputBundle = builder.build().verifyText(); - } else { - builder.assumeSymmetric(false) - .passphrase(passphrase); +// if (signedOnly) { +// outputBundle = builder.build().verifyText(); +// } else { + builder.assumeSymmetric(false) + .passphrase(passphrase); - // Do we want to do this: instead of trying to get the passphrase before - // pause stream when passphrase is missing and then resume??? + // Do we want to do this: instead of trying to get the passphrase before + // pause stream when passphrase is missing and then resume??? - // TODO: this also decrypts with other secret keys without passphrase!!! - outputBundle = builder.build().decryptAndVerify(); - } + // TODO: this also decrypts with other secret keys without passphrase!!! + outputBundle = builder.build().decryptVerify(); +// } // outputStream.close(); // byte[] outputBytes = ((ByteArrayOutputStream) outputStream).toByteArray(); // get signature informations from bundle - boolean signature = outputBundle.getBoolean(KeychainIntentService.RESULT_SIGNATURE); + boolean signature = outputBundle.getBoolean(KeychainIntentService.RESULT_SIGNATURE, false); if (signature) { long signatureKeyId = outputBundle - .getLong(KeychainIntentService.RESULT_SIGNATURE_KEY_ID); + .getLong(KeychainIntentService.RESULT_SIGNATURE_KEY_ID, 0); String signatureUserId = outputBundle .getString(KeychainIntentService.RESULT_SIGNATURE_USER_ID); boolean signatureSuccess = outputBundle - .getBoolean(KeychainIntentService.RESULT_SIGNATURE_SUCCESS); + .getBoolean(KeychainIntentService.RESULT_SIGNATURE_SUCCESS, false); boolean signatureUnknown = outputBundle - .getBoolean(KeychainIntentService.RESULT_SIGNATURE_UNKNOWN); + .getBoolean(KeychainIntentService.RESULT_SIGNATURE_UNKNOWN, false); + boolean signatureOnly = outputBundle + .getBoolean(KeychainIntentService.RESULT_CLEARTEXT_SIGNATURE_ONLY, false); int signatureStatus = OpenPgpSignatureResult.SIGNATURE_ERROR; if (signatureSuccess) { @@ -423,8 +425,9 @@ public class OpenPgpService extends RemoteService { signatureStatus = OpenPgpSignatureResult.SIGNATURE_UNKNOWN_PUB_KEY; } + // TODO: signed only?!?!?! sigResult = new OpenPgpSignatureResult(signatureStatus, signatureUserId, - signedOnly, signatureKeyId); + signatureOnly, signatureKeyId); } } finally { is.close(); diff --git a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/ui/DecryptActivity.java b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/ui/DecryptActivity.java index 1f3280af3..7df416417 100644 --- a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/ui/DecryptActivity.java +++ b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/ui/DecryptActivity.java @@ -83,6 +83,9 @@ public class DecryptActivity extends DrawerActivity { private long mSignatureKeyId = 0; private boolean mReturnResult = false; + + // TODO: replace signed only checks with something more intelligent + // PgpOperationIncoming should handle all automatically!!! private boolean mSignedOnly = false; private boolean mAssumeSymmetricEncryption = false; @@ -456,7 +459,7 @@ public class DecryptActivity extends DrawerActivity { } else { if (mDecryptTarget == Id.target.file) { askForOutputFilename(); - } else { + } else { // mDecryptTarget == Id.target.message decryptStart(); } } @@ -633,7 +636,6 @@ public class DecryptActivity extends DrawerActivity { data.putLong(KeychainIntentService.ENCRYPT_SECRET_KEY_ID, mSecretKeyId); - data.putBoolean(KeychainIntentService.DECRYPT_SIGNED_ONLY, mSignedOnly); data.putBoolean(KeychainIntentService.DECRYPT_RETURN_BYTES, mReturnBinary); data.putBoolean(KeychainIntentService.DECRYPT_ASSUME_SYMMETRIC, mAssumeSymmetricEncryption);