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.
This commit is contained in:
Vincent Breitmoser 2017-09-13 14:03:56 +02:00
parent 96bca146c0
commit b3ad904238
3 changed files with 66 additions and 12 deletions

View file

@ -81,6 +81,7 @@ public class MessageLoaderHelper {
private LoaderManager loaderManager; private LoaderManager loaderManager;
@Nullable // make this explicitly nullable, make sure to cancel/ignore any operation if this is null @Nullable // make this explicitly nullable, make sure to cancel/ignore any operation if this is null
private MessageLoaderCallbacks callback; private MessageLoaderCallbacks callback;
private final boolean processSignedOnly;
// transient state // transient state
@ -100,6 +101,8 @@ public class MessageLoaderHelper {
this.loaderManager = loaderManager; this.loaderManager = loaderManager;
this.fragmentManager = fragmentManager; this.fragmentManager = fragmentManager;
this.callback = callback; this.callback = callback;
processSignedOnly = K9.getOpenPgpSupportSignOnly();
} }
@ -276,7 +279,7 @@ public class MessageLoaderHelper {
retainCryptoHelperFragment.setData(messageCryptoHelper); retainCryptoHelperFragment.setData(messageCryptoHelper);
} }
messageCryptoHelper.asyncStartOrResumeProcessingMessage( messageCryptoHelper.asyncStartOrResumeProcessingMessage(
localMessage, messageCryptoCallback, cachedDecryptionResult); localMessage, messageCryptoCallback, cachedDecryptionResult, processSignedOnly);
} }
private void cancelAndClearCryptoOperation() { private void cancelAndClearCryptoOperation() {

View file

@ -84,6 +84,7 @@ public class MessageCryptoHelper {
private State state; private State state;
private CancelableBackgroundOperation cancelableBackgroundOperation; private CancelableBackgroundOperation cancelableBackgroundOperation;
private boolean isCancelled; private boolean isCancelled;
private boolean processSignedOnly;
private OpenPgpApi openPgpApi; private OpenPgpApi openPgpApi;
private OpenPgpServiceConnection openPgpServiceConnection; private OpenPgpServiceConnection openPgpServiceConnection;
@ -108,7 +109,7 @@ public class MessageCryptoHelper {
} }
public void asyncStartOrResumeProcessingMessage(Message message, MessageCryptoCallback callback, public void asyncStartOrResumeProcessingMessage(Message message, MessageCryptoCallback callback,
OpenPgpDecryptionResult cachedDecryptionResult) { OpenPgpDecryptionResult cachedDecryptionResult, boolean processSignedOnly) {
if (this.currentMessage != null) { if (this.currentMessage != null) {
reattachCallback(message, callback); reattachCallback(message, callback);
return; return;
@ -119,6 +120,7 @@ public class MessageCryptoHelper {
this.currentMessage = message; this.currentMessage = message;
this.cachedDecryptionResult = cachedDecryptionResult; this.cachedDecryptionResult = cachedDecryptionResult;
this.callback = callback; this.callback = callback;
this.processSignedOnly = processSignedOnly;
nextStep(); nextStep();
} }
@ -143,6 +145,13 @@ public class MessageCryptoHelper {
List<Part> signedParts = MessageCryptoStructureDetector List<Part> signedParts = MessageCryptoStructureDetector
.findMultipartSignedParts(currentMessage, messageAnnotations); .findMultipartSignedParts(currentMessage, messageAnnotations);
for (Part part : signedParts) { for (Part part : signedParts) {
if (!processSignedOnly) {
boolean isEncapsulatedSignature =
messageAnnotations.findKeyForAnnotationWithReplacementPart(part) != null;
if (!isEncapsulatedSignature) {
continue;
}
}
if (!MessageHelper.isCompletePartAvailable(part)) { if (!MessageHelper.isCompletePartAvailable(part)) {
MimeBodyPart replacementPart = getMultipartSignedContentPartIfAvailable(part); MimeBodyPart replacementPart = getMultipartSignedContentPartIfAvailable(part);
addErrorAnnotation(part, CryptoError.OPENPGP_SIGNED_BUT_INCOMPLETE, replacementPart); addErrorAnnotation(part, CryptoError.OPENPGP_SIGNED_BUT_INCOMPLETE, replacementPart);
@ -161,12 +170,15 @@ public class MessageCryptoHelper {
private void findPartsForPgpInlinePass() { private void findPartsForPgpInlinePass() {
List<Part> inlineParts = MessageCryptoStructureDetector.findPgpInlineParts(currentMessage); List<Part> inlineParts = MessageCryptoStructureDetector.findPgpInlineParts(currentMessage);
for (Part part : inlineParts) { for (Part part : inlineParts) {
if (!processSignedOnly && !MessageCryptoStructureDetector.isPartPgpInlineEncrypted(part)) {
continue;
}
if (!currentMessage.getFlags().contains(Flag.X_DOWNLOADED_FULL)) { if (!currentMessage.getFlags().contains(Flag.X_DOWNLOADED_FULL)) {
if (MessageCryptoStructureDetector.isPartPgpInlineEncrypted(part)) { if (MessageCryptoStructureDetector.isPartPgpInlineEncrypted(part)) {
addErrorAnnotation(part, CryptoError.OPENPGP_ENCRYPTED_BUT_INCOMPLETE, NO_REPLACEMENT_PART); addErrorAnnotation(part, CryptoError.OPENPGP_ENCRYPTED_BUT_INCOMPLETE, NO_REPLACEMENT_PART);
} else { } else {
MimeBodyPart replacementPart = extractClearsignedTextReplacementPart(part); addErrorAnnotation(part, CryptoError.OPENPGP_SIGNED_BUT_INCOMPLETE, NO_REPLACEMENT_PART);
addErrorAnnotation(part, CryptoError.OPENPGP_SIGNED_BUT_INCOMPLETE, replacementPart);
} }
continue; continue;
} }

View file

@ -84,7 +84,7 @@ public class MessageCryptoHelperTest {
message.setHeader("Content-Type", "text/plain"); message.setHeader("Content-Type", "text/plain");
MessageCryptoCallback messageCryptoCallback = mock(MessageCryptoCallback.class); MessageCryptoCallback messageCryptoCallback = mock(MessageCryptoCallback.class);
messageCryptoHelper.asyncStartOrResumeProcessingMessage(message, messageCryptoCallback, null); messageCryptoHelper.asyncStartOrResumeProcessingMessage(message, messageCryptoCallback, null, false);
ArgumentCaptor<MessageCryptoAnnotations> captor = ArgumentCaptor.forClass(MessageCryptoAnnotations.class); ArgumentCaptor<MessageCryptoAnnotations> captor = ArgumentCaptor.forClass(MessageCryptoAnnotations.class);
verify(messageCryptoCallback).onCryptoOperationsFinished(captor.capture()); verify(messageCryptoCallback).onCryptoOperationsFinished(captor.capture());
@ -107,7 +107,7 @@ public class MessageCryptoHelperTest {
MessageCryptoCallback messageCryptoCallback = mock(MessageCryptoCallback.class); MessageCryptoCallback messageCryptoCallback = mock(MessageCryptoCallback.class);
messageCryptoHelper.asyncStartOrResumeProcessingMessage(message, messageCryptoCallback, null); messageCryptoHelper.asyncStartOrResumeProcessingMessage(message, messageCryptoCallback, null, false);
ArgumentCaptor<MessageCryptoAnnotations> captor = ArgumentCaptor.forClass(MessageCryptoAnnotations.class); ArgumentCaptor<MessageCryptoAnnotations> captor = ArgumentCaptor.forClass(MessageCryptoAnnotations.class);
@ -132,7 +132,7 @@ public class MessageCryptoHelperTest {
); );
MessageCryptoCallback messageCryptoCallback = mock(MessageCryptoCallback.class); 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, assertPartAnnotationHasState(message, messageCryptoCallback, CryptoError.OPENPGP_SIGNED_BUT_INCOMPLETE, null,
null, null, null); null, null, null);
@ -148,7 +148,7 @@ public class MessageCryptoHelperTest {
); );
MessageCryptoCallback messageCryptoCallback = mock(MessageCryptoCallback.class); MessageCryptoCallback messageCryptoCallback = mock(MessageCryptoCallback.class);
messageCryptoHelper.asyncStartOrResumeProcessingMessage(message, messageCryptoCallback, null); messageCryptoHelper.asyncStartOrResumeProcessingMessage(message, messageCryptoCallback, null, false);
assertPartAnnotationHasState( assertPartAnnotationHasState(
message, messageCryptoCallback, CryptoError.OPENPGP_ENCRYPTED_BUT_INCOMPLETE, null, null, null, null); message, messageCryptoCallback, CryptoError.OPENPGP_ENCRYPTED_BUT_INCOMPLETE, null, null, null, null);
@ -164,7 +164,7 @@ public class MessageCryptoHelperTest {
); );
MessageCryptoCallback messageCryptoCallback = mock(MessageCryptoCallback.class); 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, assertPartAnnotationHasState(message, messageCryptoCallback, CryptoError.ENCRYPTED_BUT_UNSUPPORTED, null, null,
null, null); null, null);
@ -180,7 +180,7 @@ public class MessageCryptoHelperTest {
); );
MessageCryptoCallback messageCryptoCallback = mock(MessageCryptoCallback.class); 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, assertPartAnnotationHasState(message, messageCryptoCallback, CryptoError.SIGNED_BUT_UNSUPPORTED, null, null,
null, null); null, null);
@ -210,6 +210,36 @@ public class MessageCryptoHelperTest {
verifyNoMoreInteractions(autocryptOperations); 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 @Test
public void multipartEncrypted__shouldCallOpenPgpApiAsync() throws Exception { public void multipartEncrypted__shouldCallOpenPgpApiAsync() throws Exception {
Body encryptedBody; Body encryptedBody;
@ -249,7 +279,7 @@ public class MessageCryptoHelperTest {
private void processEncryptedMessageAndCaptureMocks(Message message, Body encryptedBody, OutputStream outputStream) private void processEncryptedMessageAndCaptureMocks(Message message, Body encryptedBody, OutputStream outputStream)
throws Exception { throws Exception {
messageCryptoHelper.asyncStartOrResumeProcessingMessage(message, messageCryptoCallback, null); messageCryptoHelper.asyncStartOrResumeProcessingMessage(message, messageCryptoCallback, null, false);
ArgumentCaptor<Intent> intentCaptor = ArgumentCaptor.forClass(Intent.class); ArgumentCaptor<Intent> intentCaptor = ArgumentCaptor.forClass(Intent.class);
ArgumentCaptor<OpenPgpDataSource> dataSourceCaptor = ArgumentCaptor.forClass(OpenPgpDataSource.class); ArgumentCaptor<OpenPgpDataSource> dataSourceCaptor = ArgumentCaptor.forClass(OpenPgpDataSource.class);
@ -267,7 +297,7 @@ public class MessageCryptoHelperTest {
private void processSignedMessageAndCaptureMocks(Message message, BodyPart signedBodyPart, private void processSignedMessageAndCaptureMocks(Message message, BodyPart signedBodyPart,
OutputStream outputStream) throws Exception { OutputStream outputStream) throws Exception {
messageCryptoHelper.asyncStartOrResumeProcessingMessage(message, messageCryptoCallback, null); messageCryptoHelper.asyncStartOrResumeProcessingMessage(message, messageCryptoCallback, null, true);
ArgumentCaptor<Intent> intentCaptor = ArgumentCaptor.forClass(Intent.class); ArgumentCaptor<Intent> intentCaptor = ArgumentCaptor.forClass(Intent.class);
ArgumentCaptor<OpenPgpDataSource> dataSourceCaptor = ArgumentCaptor.forClass(OpenPgpDataSource.class); ArgumentCaptor<OpenPgpDataSource> dataSourceCaptor = ArgumentCaptor.forClass(OpenPgpDataSource.class);
@ -283,6 +313,15 @@ public class MessageCryptoHelperTest {
verify(signedBodyPart).writeTo(outputStream); verify(signedBodyPart).writeTo(outputStream);
} }
private void assertReturnsWithNoCryptoAnnotations(MessageCryptoCallback messageCryptoCallback) {
ArgumentCaptor<MessageCryptoAnnotations> 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, private void assertPartAnnotationHasState(Message message, MessageCryptoCallback messageCryptoCallback,
CryptoError cryptoErrorState, MimeBodyPart replacementPart, OpenPgpDecryptionResult openPgpDecryptionResult, CryptoError cryptoErrorState, MimeBodyPart replacementPart, OpenPgpDecryptionResult openPgpDecryptionResult,
OpenPgpSignatureResult openPgpSignatureResult, PendingIntent openPgpPendingIntent) { OpenPgpSignatureResult openPgpSignatureResult, PendingIntent openPgpPendingIntent) {