Merge pull request #2516 from k9mail/insecure-warning

Security Warnings
This commit is contained in:
Vincent Breitmoser 2017-04-30 23:35:32 +02:00 committed by GitHub
commit 231b46f278
10 changed files with 140 additions and 30 deletions

View file

@ -20,13 +20,18 @@ public final class CryptoResultAnnotation {
private final OpenPgpSignatureResult openPgpSignatureResult;
private final OpenPgpError openPgpError;
private final PendingIntent openPgpPendingIntent;
private final PendingIntent openPgpInsecureWarningPendingIntent;
private final boolean overrideCryptoWarning;
private final CryptoResultAnnotation encapsulatedResult;
private CryptoResultAnnotation(@NonNull CryptoError errorType, MimeBodyPart replacementData,
OpenPgpDecryptionResult openPgpDecryptionResult,
OpenPgpSignatureResult openPgpSignatureResult,
PendingIntent openPgpPendingIntent, OpenPgpError openPgpError) {
PendingIntent openPgpPendingIntent,
PendingIntent openPgpInsecureWarningPendingIntent,
OpenPgpError openPgpError,
boolean overrideCryptoWarning) {
this.errorType = errorType;
this.replacementData = replacementData;
@ -34,6 +39,8 @@ public final class CryptoResultAnnotation {
this.openPgpSignatureResult = openPgpSignatureResult;
this.openPgpPendingIntent = openPgpPendingIntent;
this.openPgpError = openPgpError;
this.openPgpInsecureWarningPendingIntent = openPgpInsecureWarningPendingIntent;
this.overrideCryptoWarning = overrideCryptoWarning;
this.encapsulatedResult = null;
}
@ -49,37 +56,43 @@ public final class CryptoResultAnnotation {
this.openPgpDecryptionResult = annotation.openPgpDecryptionResult;
this.openPgpSignatureResult = annotation.openPgpSignatureResult;
this.openPgpPendingIntent = annotation.openPgpPendingIntent;
this.openPgpInsecureWarningPendingIntent = annotation.openPgpInsecureWarningPendingIntent;
this.openPgpError = annotation.openPgpError;
this.overrideCryptoWarning = annotation.overrideCryptoWarning;
this.encapsulatedResult = encapsulatedResult;
}
public static CryptoResultAnnotation createOpenPgpResultAnnotation(OpenPgpDecryptionResult decryptionResult,
OpenPgpSignatureResult signatureResult, PendingIntent pendingIntent, MimeBodyPart replacementPart) {
OpenPgpSignatureResult signatureResult, PendingIntent pendingIntent,
PendingIntent insecureWarningPendingIntent, MimeBodyPart replacementPart,
boolean overrideCryptoWarning) {
return new CryptoResultAnnotation(CryptoError.OPENPGP_OK, replacementPart,
decryptionResult, signatureResult, pendingIntent, null);
decryptionResult, signatureResult, pendingIntent, insecureWarningPendingIntent, null,
overrideCryptoWarning);
}
public static CryptoResultAnnotation createErrorAnnotation(CryptoError error, MimeBodyPart replacementData) {
if (error == CryptoError.OPENPGP_OK) {
throw new AssertionError("CryptoError must be actual error state!");
}
return new CryptoResultAnnotation(error, replacementData, null, null, null, null);
return new CryptoResultAnnotation(error, replacementData, null, null, null, null, null, false);
}
public static CryptoResultAnnotation createOpenPgpCanceledAnnotation() {
return new CryptoResultAnnotation(CryptoError.OPENPGP_UI_CANCELED, null, null, null, null, null);
return new CryptoResultAnnotation(CryptoError.OPENPGP_UI_CANCELED, null, null, null, null, null, null, false);
}
public static CryptoResultAnnotation createOpenPgpSignatureErrorAnnotation(
OpenPgpError error, MimeBodyPart replacementData) {
return new CryptoResultAnnotation(
CryptoError.OPENPGP_SIGNED_API_ERROR, replacementData, null, null, null, error);
CryptoError.OPENPGP_SIGNED_API_ERROR, replacementData, null, null, null, null, error, false);
}
public static CryptoResultAnnotation createOpenPgpEncryptionErrorAnnotation(OpenPgpError error) {
return new CryptoResultAnnotation(CryptoError.OPENPGP_ENCRYPTED_API_ERROR, null, null, null, null, error);
return new CryptoResultAnnotation(
CryptoError.OPENPGP_ENCRYPTED_API_ERROR, null, null, null, null, null, error, false);
}
public boolean isOpenPgpResult() {
@ -117,6 +130,15 @@ public final class CryptoResultAnnotation {
return openPgpPendingIntent;
}
public boolean hasOpenPgpInsecureWarningPendingIntent() {
return openPgpInsecureWarningPendingIntent != null;
}
@Nullable
public PendingIntent getOpenPgpInsecureWarningPendingIntent() {
return openPgpInsecureWarningPendingIntent;
}
@Nullable
public OpenPgpError getOpenPgpError() {
return openPgpError;
@ -136,6 +158,10 @@ public final class CryptoResultAnnotation {
return replacementData;
}
public boolean isOverrideSecurityWarning() {
return overrideCryptoWarning;
}
@NonNull
public CryptoResultAnnotation withEncapsulatedResult(CryptoResultAnnotation resultAnnotation) {
return new CryptoResultAnnotation(this, resultAnnotation);
@ -149,7 +175,6 @@ public final class CryptoResultAnnotation {
return encapsulatedResult;
}
public enum CryptoError {
OPENPGP_OK,
OPENPGP_UI_CANCELED,

View file

@ -251,6 +251,7 @@ public class MessageCryptoHelper {
decryptIntent.putExtra(OpenPgpApi.EXTRA_SENDER_ADDRESS, from[0].getAddress());
}
decryptIntent.putExtra(OpenPgpApi.EXTRA_SUPPORT_OVERRIDE_CRYPTO_WARNING, true);
decryptIntent.putExtra(OpenPgpApi.EXTRA_DECRYPTION_RESULT, cachedDecryptionResult);
return decryptIntent;
@ -514,9 +515,12 @@ public class MessageCryptoHelper {
OpenPgpSignatureResult signatureResult =
currentCryptoResult.getParcelableExtra(OpenPgpApi.RESULT_SIGNATURE);
PendingIntent pendingIntent = currentCryptoResult.getParcelableExtra(OpenPgpApi.RESULT_INTENT);
PendingIntent insecureWarningPendingIntent = currentCryptoResult.getParcelableExtra(OpenPgpApi.RESULT_INSECURE_DETAIL_INTENT);
boolean overrideCryptoWarning = currentCryptoResult.getBooleanExtra(
OpenPgpApi.RESULT_OVERRIDE_CRYPTO_WARNING, false);
CryptoResultAnnotation resultAnnotation = CryptoResultAnnotation.createOpenPgpResultAnnotation(
decryptionResult, signatureResult, pendingIntent, outputPart);
CryptoResultAnnotation resultAnnotation = CryptoResultAnnotation.createOpenPgpResultAnnotation(decryptionResult,
signatureResult, pendingIntent, insecureWarningPendingIntent, outputPart, overrideCryptoWarning);
onCryptoOperationSuccess(resultAnnotation);
}

View file

@ -28,6 +28,7 @@ import com.fsck.k9.view.ThemeUtils;
public class CryptoInfoDialog extends DialogFragment {
public static final String ARG_DISPLAY_STATUS = "display_status";
public static final String ARG_HAS_SECURITY_WARNING = "has_security_warning";
public static final int ICON_ANIM_DELAY = 400;
public static final int ICON_ANIM_DURATION = 350;
@ -46,11 +47,12 @@ public class CryptoInfoDialog extends DialogFragment {
private TextView bottomText;
public static CryptoInfoDialog newInstance(MessageCryptoDisplayStatus displayStatus) {
public static CryptoInfoDialog newInstance(MessageCryptoDisplayStatus displayStatus, boolean hasSecurityWarning) {
CryptoInfoDialog frag = new CryptoInfoDialog();
Bundle args = new Bundle();
args.putString(ARG_DISPLAY_STATUS, displayStatus.toString());
args.putBoolean(ARG_HAS_SECURITY_WARNING, hasSecurityWarning);
frag.setArguments(args);
return frag;
@ -85,7 +87,19 @@ public class CryptoInfoDialog extends DialogFragment {
dismiss();
}
});
if (displayStatus.hasAssociatedKey()) {
boolean hasSecurityWarning = getArguments().getBoolean(ARG_HAS_SECURITY_WARNING);
if (hasSecurityWarning) {
b.setNeutralButton(R.string.crypto_info_view_security_warning, new OnClickListener() {
@Override
public void onClick(DialogInterface dialogInterface, int i) {
Fragment frag = getTargetFragment();
if (! (frag instanceof OnClickShowCryptoKeyListener)) {
throw new AssertionError("Displaying activity must implement OnClickShowCryptoKeyListener!");
}
((OnClickShowCryptoKeyListener) frag).onClickShowSecurityWarning();
}
});
} else if (displayStatus.hasAssociatedKey()) {
b.setNeutralButton(R.string.crypto_info_view_key, new OnClickListener() {
@Override
public void onClick(DialogInterface dialogInterface, int i) {
@ -190,5 +204,6 @@ public class CryptoInfoDialog extends DialogFragment {
public interface OnClickShowCryptoKeyListener {
void onClickShowCryptoKey();
void onClickShowSecurityWarning();
}
}

View file

@ -12,7 +12,6 @@ import android.os.Bundle;
import android.os.Parcelable;
import android.support.annotation.Nullable;
import android.support.annotation.StringRes;
import timber.log.Timber;
import com.fsck.k9.Account;
import com.fsck.k9.K9;
@ -20,10 +19,12 @@ import com.fsck.k9.R;
import com.fsck.k9.mailstore.CryptoResultAnnotation;
import com.fsck.k9.mailstore.MessageViewInfo;
import com.fsck.k9.view.MessageCryptoDisplayStatus;
import timber.log.Timber;
public class MessageCryptoPresenter implements OnCryptoClickListener {
public static final int REQUEST_CODE_UNKNOWN_KEY = 123;
public static final int REQUEST_CODE_SECURITY_WARNING = 124;
// injected state
@ -72,6 +73,10 @@ public class MessageCryptoPresenter implements OnCryptoClickListener {
return false;
}
if (cryptoResultAnnotation.isOverrideSecurityWarning()) {
overrideCryptoWarning = true;
}
messageView.getMessageHeaderView().setCryptoStatus(displayStatus);
switch (displayStatus) {
@ -118,9 +123,15 @@ public class MessageCryptoPresenter implements OnCryptoClickListener {
}
case ENCRYPTED_ERROR:
case ENCRYPTED_INSECURE:
case UNSUPPORTED_ENCRYPTED: {
Drawable providerIcon = getOpenPgpApiProviderIcon(messageView.getContext());
messageView.showMessageCryptoErrorView(messageViewInfo, providerIcon);
if (messageViewInfo.cryptoResultAnnotation.hasReplacementData()) {
showMessageCryptoWarning(messageView, account, messageViewInfo,
R.string.messageview_crypto_warning_insecure);
} else {
messageView.showMessageCryptoErrorView(messageViewInfo, providerIcon);
}
break;
}
@ -151,10 +162,10 @@ public class MessageCryptoPresenter implements OnCryptoClickListener {
return;
}
Drawable providerIcon = getOpenPgpApiProviderIcon(messageView.getContext());
messageView.showMessageCryptoWarning(messageViewInfo, providerIcon, warningStringRes);
boolean showDetailButton = cryptoResultAnnotation.hasOpenPgpInsecureWarningPendingIntent();
messageView.showMessageCryptoWarning(messageViewInfo, providerIcon, warningStringRes, showDetailButton);
}
@Override
public void onCryptoClick() {
if (cryptoResultAnnotation == null) {
@ -177,19 +188,27 @@ public class MessageCryptoPresenter implements OnCryptoClickListener {
@SuppressWarnings("UnusedParameters") // for consistency with Activity.onActivityResult
public void onActivityResult(int requestCode, int resultCode, Intent data) {
if (requestCode != REQUEST_CODE_UNKNOWN_KEY) {
if (requestCode == REQUEST_CODE_UNKNOWN_KEY) {
if (resultCode != Activity.RESULT_OK) {
return;
}
messageCryptoMvpView.restartMessageCryptoProcessing();
} else if (requestCode == REQUEST_CODE_SECURITY_WARNING) {
if (overrideCryptoWarning || resultCode != Activity.RESULT_OK) {
return;
}
overrideCryptoWarning = true;
messageCryptoMvpView.redisplayMessage();
} else {
throw new IllegalStateException("got an activity result that wasn't meant for us. this is a bug!");
}
if (resultCode != Activity.RESULT_OK) {
return;
}
messageCryptoMvpView.restartMessageCryptoProcessing();
}
private void displayCryptoInfoDialog(MessageCryptoDisplayStatus displayStatus) {
messageCryptoMvpView.showCryptoInfoDialog(displayStatus);
messageCryptoMvpView.showCryptoInfoDialog(
displayStatus, cryptoResultAnnotation.hasOpenPgpInsecureWarningPendingIntent());
}
private void launchPendingIntent(CryptoResultAnnotation cryptoResultAnnotation) {
@ -225,6 +244,18 @@ public class MessageCryptoPresenter implements OnCryptoClickListener {
messageCryptoMvpView.redisplayMessage();
}
public void onClickShowCryptoWarningDetails() {
try {
PendingIntent pendingIntent = cryptoResultAnnotation.getOpenPgpInsecureWarningPendingIntent();
if (pendingIntent != null) {
messageCryptoMvpView.startPendingIntentForCryptoPresenter(
pendingIntent.getIntentSender(), REQUEST_CODE_SECURITY_WARNING, null, 0, 0, 0);
}
} catch (IntentSender.SendIntentException e) {
Timber.e(e, "SendIntentException");
}
}
public Parcelable getDecryptionResultForReply() {
if (cryptoResultAnnotation != null && cryptoResultAnnotation.isOpenPgpResult()) {
return cryptoResultAnnotation.getOpenPgpDecryptionResult();
@ -257,7 +288,7 @@ public class MessageCryptoPresenter implements OnCryptoClickListener {
void startPendingIntentForCryptoPresenter(IntentSender si, Integer requestCode, Intent fillIntent,
int flagsMask, int flagValues, int extraFlags) throws IntentSender.SendIntentException;
void showCryptoInfoDialog(MessageCryptoDisplayStatus displayStatus);
void showCryptoInfoDialog(MessageCryptoDisplayStatus displayStatus, boolean hasSecurityWarning);
void showCryptoConfigDialog();
}
}

View file

@ -129,11 +129,24 @@ public class MessageTopView extends LinearLayout {
}
public void showMessageCryptoWarning(final MessageViewInfo messageViewInfo, Drawable providerIcon,
@StringRes int warningTextRes) {
@StringRes int warningTextRes, boolean showDetailButton) {
resetAndPrepareMessageView(messageViewInfo);
View view = mInflater.inflate(R.layout.message_content_crypto_warning, containerView, false);
setCryptoProviderIcon(providerIcon, view);
View detailButton = view.findViewById(R.id.crypto_warning_details);
if(showDetailButton) {
detailButton.setVisibility(View.VISIBLE);
detailButton.setOnClickListener(new OnClickListener() {
@Override
public void onClick(View view) {
messageCryptoPresenter.onClickShowCryptoWarningDetails();
}
});
} else {
detailButton.setVisibility(View.GONE);
}
view.findViewById(R.id.crypto_warning_override).setOnClickListener(new OnClickListener() {
@Override
public void onClick(View view) {

View file

@ -689,8 +689,8 @@ public class MessageViewFragment extends Fragment implements ConfirmationDialogF
}
@Override
public void showCryptoInfoDialog(MessageCryptoDisplayStatus displayStatus) {
CryptoInfoDialog dialog = CryptoInfoDialog.newInstance(displayStatus);
public void showCryptoInfoDialog(MessageCryptoDisplayStatus displayStatus, boolean hasSecurityWarning) {
CryptoInfoDialog dialog = CryptoInfoDialog.newInstance(displayStatus, hasSecurityWarning);
dialog.setTargetFragment(MessageViewFragment.this, 0);
dialog.show(getFragmentManager(), "crypto_info_dialog");
}
@ -707,6 +707,11 @@ public class MessageViewFragment extends Fragment implements ConfirmationDialogF
}
};
@Override
public void onClickShowSecurityWarning() {
messageCryptoPresenter.onClickShowCryptoWarningDetails();
}
@Override
public void onClickShowCryptoKey() {
messageCryptoPresenter.onClickShowCryptoKey();

View file

@ -125,6 +125,12 @@ public enum MessageCryptoDisplayStatus {
R.string.crypto_msg_encrypted_error
),
ENCRYPTED_INSECURE (
R.attr.openpgp_red,
R.drawable.status_lock_error,
R.string.crypto_msg_encrypted_insecure
),
INCOMPLETE_ENCRYPTED (
R.attr.openpgp_black,
R.drawable.status_lock_opportunistic,
@ -254,8 +260,7 @@ public enum MessageCryptoDisplayStatus {
return getStatusForPgpEncryptedResult(signatureResult);
case OpenPgpDecryptionResult.RESULT_INSECURE:
// TODO handle better?
return ENCRYPTED_ERROR;
return ENCRYPTED_INSECURE;
}
throw new AssertionError("all cases must be handled, this is a bug!");

View file

@ -43,6 +43,12 @@
tools:text="@string/messageview_crypto_warning_revoked"
/>
<Button
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:id="@+id/crypto_warning_details"
android:text="@string/messageview_crypto_warning_details"/>
<Button
android:layout_width="wrap_content"
android:layout_height="wrap_content"

View file

@ -1167,6 +1167,7 @@ Please submit bug reports, contribute new features and ask questions at
<string name="crypto_msg_disabled">Message is not encrypted</string>
<string name="crypto_msg_encrypted_error">Message is encrypted, but there was a decryption error.</string>
<string name="crypto_msg_encrypted_insecure">Message is encrypted but insecure!</string>
<string name="crypto_msg_signed_error">Message is signed, but the signature could not be verified.</string>
<string name="crypto_msg_encrypted_no_provider">Message is encrypted, but no crypto app is configured.</string>
<string name="crypto_msg_unsupported_encrypted">Message is encrypted, but in an unsupported format.</string>
@ -1193,6 +1194,7 @@ Please submit bug reports, contribute new features and ask questions at
<string name="crypto_info_ok">OK</string>
<string name="crypto_info_view_key">View Signer</string>
<string name="crypto_info_view_security_warning">Details</string>
<string name="locked_attach_unlock">Unlock</string>
<string name="locked_attach_unencrypted">This part was not encrypted, and may be insecure.</string>
<string name="locked_attach_title">Unprotected Attachment</string>
@ -1230,4 +1232,5 @@ Please submit bug reports, contribute new features and ask questions at
<string name="mail_list_widget_text">K-9 Message List</string>
<string name="mail_list_widget_loading">Loading messages…</string>
<string name="fetching_folders_failed">Fetching folder list failed</string>
<string name="messageview_crypto_warning_details">Show Details</string>
</resources>

View file

@ -263,9 +263,12 @@ public class OpenPgpApi {
public static final String EXTRA_PROGRESS_MESSENGER = "progress_messenger";
public static final String EXTRA_DATA_LENGTH = "data_length";
public static final String EXTRA_SENDER_ADDRESS = "sender_address";
public static final String EXTRA_SUPPORT_OVERRIDE_CRYPTO_WARNING = "support_override_crpto_warning";
public static final String RESULT_SIGNATURE = "signature";
public static final String RESULT_DECRYPTION = "decryption";
public static final String RESULT_METADATA = "metadata";
public static final String RESULT_INSECURE_DETAIL_INTENT = "insecure_detail_intent";
public static final String RESULT_OVERRIDE_CRYPTO_WARNING = "override_crypto_warning";
// This will be the charset which was specified in the headers of ascii armored input, if any
public static final String RESULT_CHARSET = "charset";