From b3ad904238746616b41c042b38ce67b1e2ff5464 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Wed, 13 Sep 2017 14:03:56 +0200 Subject: [PATCH] Skip signature verification if it is not going to be displayed anyways This also removes replacement of clearsigned data with its content from MessageCryptoHelper, to be moved to text extraction later on. --- .../fsck/k9/activity/MessageLoaderHelper.java | 5 +- .../k9/ui/crypto/MessageCryptoHelper.java | 18 +++++- .../k9/ui/crypto/MessageCryptoHelperTest.java | 55 ++++++++++++++++--- 3 files changed, 66 insertions(+), 12 deletions(-) diff --git a/k9mail/src/main/java/com/fsck/k9/activity/MessageLoaderHelper.java b/k9mail/src/main/java/com/fsck/k9/activity/MessageLoaderHelper.java index 54787eb63..954cf6208 100644 --- a/k9mail/src/main/java/com/fsck/k9/activity/MessageLoaderHelper.java +++ b/k9mail/src/main/java/com/fsck/k9/activity/MessageLoaderHelper.java @@ -81,6 +81,7 @@ public class MessageLoaderHelper { private LoaderManager loaderManager; @Nullable // make this explicitly nullable, make sure to cancel/ignore any operation if this is null private MessageLoaderCallbacks callback; + private final boolean processSignedOnly; // transient state @@ -100,6 +101,8 @@ public class MessageLoaderHelper { this.loaderManager = loaderManager; this.fragmentManager = fragmentManager; this.callback = callback; + + processSignedOnly = K9.getOpenPgpSupportSignOnly(); } @@ -276,7 +279,7 @@ public class MessageLoaderHelper { retainCryptoHelperFragment.setData(messageCryptoHelper); } messageCryptoHelper.asyncStartOrResumeProcessingMessage( - localMessage, messageCryptoCallback, cachedDecryptionResult); + localMessage, messageCryptoCallback, cachedDecryptionResult, processSignedOnly); } private void cancelAndClearCryptoOperation() { diff --git a/k9mail/src/main/java/com/fsck/k9/ui/crypto/MessageCryptoHelper.java b/k9mail/src/main/java/com/fsck/k9/ui/crypto/MessageCryptoHelper.java index b8e7e2e86..94eacf467 100644 --- a/k9mail/src/main/java/com/fsck/k9/ui/crypto/MessageCryptoHelper.java +++ b/k9mail/src/main/java/com/fsck/k9/ui/crypto/MessageCryptoHelper.java @@ -84,6 +84,7 @@ public class MessageCryptoHelper { private State state; private CancelableBackgroundOperation cancelableBackgroundOperation; private boolean isCancelled; + private boolean processSignedOnly; private OpenPgpApi openPgpApi; private OpenPgpServiceConnection openPgpServiceConnection; @@ -108,7 +109,7 @@ public class MessageCryptoHelper { } public void asyncStartOrResumeProcessingMessage(Message message, MessageCryptoCallback callback, - OpenPgpDecryptionResult cachedDecryptionResult) { + OpenPgpDecryptionResult cachedDecryptionResult, boolean processSignedOnly) { if (this.currentMessage != null) { reattachCallback(message, callback); return; @@ -119,6 +120,7 @@ public class MessageCryptoHelper { this.currentMessage = message; this.cachedDecryptionResult = cachedDecryptionResult; this.callback = callback; + this.processSignedOnly = processSignedOnly; nextStep(); } @@ -143,6 +145,13 @@ public class MessageCryptoHelper { List signedParts = MessageCryptoStructureDetector .findMultipartSignedParts(currentMessage, messageAnnotations); for (Part part : signedParts) { + if (!processSignedOnly) { + boolean isEncapsulatedSignature = + messageAnnotations.findKeyForAnnotationWithReplacementPart(part) != null; + if (!isEncapsulatedSignature) { + continue; + } + } if (!MessageHelper.isCompletePartAvailable(part)) { MimeBodyPart replacementPart = getMultipartSignedContentPartIfAvailable(part); addErrorAnnotation(part, CryptoError.OPENPGP_SIGNED_BUT_INCOMPLETE, replacementPart); @@ -161,12 +170,15 @@ public class MessageCryptoHelper { private void findPartsForPgpInlinePass() { List inlineParts = MessageCryptoStructureDetector.findPgpInlineParts(currentMessage); for (Part part : inlineParts) { + if (!processSignedOnly && !MessageCryptoStructureDetector.isPartPgpInlineEncrypted(part)) { + continue; + } + if (!currentMessage.getFlags().contains(Flag.X_DOWNLOADED_FULL)) { if (MessageCryptoStructureDetector.isPartPgpInlineEncrypted(part)) { addErrorAnnotation(part, CryptoError.OPENPGP_ENCRYPTED_BUT_INCOMPLETE, NO_REPLACEMENT_PART); } else { - MimeBodyPart replacementPart = extractClearsignedTextReplacementPart(part); - addErrorAnnotation(part, CryptoError.OPENPGP_SIGNED_BUT_INCOMPLETE, replacementPart); + addErrorAnnotation(part, CryptoError.OPENPGP_SIGNED_BUT_INCOMPLETE, NO_REPLACEMENT_PART); } continue; } diff --git a/k9mail/src/test/java/com/fsck/k9/ui/crypto/MessageCryptoHelperTest.java b/k9mail/src/test/java/com/fsck/k9/ui/crypto/MessageCryptoHelperTest.java index d35f529d0..02ea5f485 100644 --- a/k9mail/src/test/java/com/fsck/k9/ui/crypto/MessageCryptoHelperTest.java +++ b/k9mail/src/test/java/com/fsck/k9/ui/crypto/MessageCryptoHelperTest.java @@ -84,7 +84,7 @@ public class MessageCryptoHelperTest { message.setHeader("Content-Type", "text/plain"); MessageCryptoCallback messageCryptoCallback = mock(MessageCryptoCallback.class); - messageCryptoHelper.asyncStartOrResumeProcessingMessage(message, messageCryptoCallback, null); + messageCryptoHelper.asyncStartOrResumeProcessingMessage(message, messageCryptoCallback, null, false); ArgumentCaptor captor = ArgumentCaptor.forClass(MessageCryptoAnnotations.class); verify(messageCryptoCallback).onCryptoOperationsFinished(captor.capture()); @@ -107,7 +107,7 @@ public class MessageCryptoHelperTest { MessageCryptoCallback messageCryptoCallback = mock(MessageCryptoCallback.class); - messageCryptoHelper.asyncStartOrResumeProcessingMessage(message, messageCryptoCallback, null); + messageCryptoHelper.asyncStartOrResumeProcessingMessage(message, messageCryptoCallback, null, false); ArgumentCaptor captor = ArgumentCaptor.forClass(MessageCryptoAnnotations.class); @@ -132,7 +132,7 @@ public class MessageCryptoHelperTest { ); MessageCryptoCallback messageCryptoCallback = mock(MessageCryptoCallback.class); - messageCryptoHelper.asyncStartOrResumeProcessingMessage(message, messageCryptoCallback, null); + messageCryptoHelper.asyncStartOrResumeProcessingMessage(message, messageCryptoCallback, null, true); assertPartAnnotationHasState(message, messageCryptoCallback, CryptoError.OPENPGP_SIGNED_BUT_INCOMPLETE, null, null, null, null); @@ -148,7 +148,7 @@ public class MessageCryptoHelperTest { ); MessageCryptoCallback messageCryptoCallback = mock(MessageCryptoCallback.class); - messageCryptoHelper.asyncStartOrResumeProcessingMessage(message, messageCryptoCallback, null); + messageCryptoHelper.asyncStartOrResumeProcessingMessage(message, messageCryptoCallback, null, false); assertPartAnnotationHasState( message, messageCryptoCallback, CryptoError.OPENPGP_ENCRYPTED_BUT_INCOMPLETE, null, null, null, null); @@ -164,7 +164,7 @@ public class MessageCryptoHelperTest { ); MessageCryptoCallback messageCryptoCallback = mock(MessageCryptoCallback.class); - messageCryptoHelper.asyncStartOrResumeProcessingMessage(message, messageCryptoCallback, null); + messageCryptoHelper.asyncStartOrResumeProcessingMessage(message, messageCryptoCallback, null, false); assertPartAnnotationHasState(message, messageCryptoCallback, CryptoError.ENCRYPTED_BUT_UNSUPPORTED, null, null, null, null); @@ -180,7 +180,7 @@ public class MessageCryptoHelperTest { ); MessageCryptoCallback messageCryptoCallback = mock(MessageCryptoCallback.class); - messageCryptoHelper.asyncStartOrResumeProcessingMessage(message, messageCryptoCallback, null); + messageCryptoHelper.asyncStartOrResumeProcessingMessage(message, messageCryptoCallback, null, true); assertPartAnnotationHasState(message, messageCryptoCallback, CryptoError.SIGNED_BUT_UNSUPPORTED, null, null, null, null); @@ -210,6 +210,36 @@ public class MessageCryptoHelperTest { verifyNoMoreInteractions(autocryptOperations); } + @Test + public void multipartSigned__withSignOnlyDisabled__shouldReturnNothing() throws Exception { + Message message = messageFromBody( + multipart("signed", "application/pgp-signature", + bodypart("text/plain", "content"), + bodypart("application/pgp-signature", "content") + ) + ); + + MessageCryptoCallback messageCryptoCallback = mock(MessageCryptoCallback.class); + messageCryptoHelper.asyncStartOrResumeProcessingMessage(message, messageCryptoCallback, null, false); + + assertReturnsWithNoCryptoAnnotations(messageCryptoCallback); + } + + @Test + public void multipartSigned__withSignOnlyDisabledAndNullBody__shouldReturnNothing() throws Exception { + Message message = messageFromBody( + multipart("signed", "application/pgp-signature", + bodypart("text/plain"), + bodypart("application/pgp-signature") + ) + ); + + MessageCryptoCallback messageCryptoCallback = mock(MessageCryptoCallback.class); + messageCryptoHelper.asyncStartOrResumeProcessingMessage(message, messageCryptoCallback, null, false); + + assertReturnsWithNoCryptoAnnotations(messageCryptoCallback); + } + @Test public void multipartEncrypted__shouldCallOpenPgpApiAsync() throws Exception { Body encryptedBody; @@ -249,7 +279,7 @@ public class MessageCryptoHelperTest { private void processEncryptedMessageAndCaptureMocks(Message message, Body encryptedBody, OutputStream outputStream) throws Exception { - messageCryptoHelper.asyncStartOrResumeProcessingMessage(message, messageCryptoCallback, null); + messageCryptoHelper.asyncStartOrResumeProcessingMessage(message, messageCryptoCallback, null, false); ArgumentCaptor intentCaptor = ArgumentCaptor.forClass(Intent.class); ArgumentCaptor dataSourceCaptor = ArgumentCaptor.forClass(OpenPgpDataSource.class); @@ -267,7 +297,7 @@ public class MessageCryptoHelperTest { private void processSignedMessageAndCaptureMocks(Message message, BodyPart signedBodyPart, OutputStream outputStream) throws Exception { - messageCryptoHelper.asyncStartOrResumeProcessingMessage(message, messageCryptoCallback, null); + messageCryptoHelper.asyncStartOrResumeProcessingMessage(message, messageCryptoCallback, null, true); ArgumentCaptor intentCaptor = ArgumentCaptor.forClass(Intent.class); ArgumentCaptor dataSourceCaptor = ArgumentCaptor.forClass(OpenPgpDataSource.class); @@ -283,6 +313,15 @@ public class MessageCryptoHelperTest { verify(signedBodyPart).writeTo(outputStream); } + private void assertReturnsWithNoCryptoAnnotations(MessageCryptoCallback messageCryptoCallback) { + ArgumentCaptor captor = ArgumentCaptor.forClass(MessageCryptoAnnotations.class); + verify(messageCryptoCallback).onCryptoOperationsFinished(captor.capture()); + verifyNoMoreInteractions(messageCryptoCallback); + + MessageCryptoAnnotations annotations = captor.getValue(); + assertTrue(annotations.isEmpty()); + } + private void assertPartAnnotationHasState(Message message, MessageCryptoCallback messageCryptoCallback, CryptoError cryptoErrorState, MimeBodyPart replacementPart, OpenPgpDecryptionResult openPgpDecryptionResult, OpenPgpSignatureResult openPgpSignatureResult, PendingIntent openPgpPendingIntent) {