From 1c2e1a07f521773b604f9204414bc19423050972 Mon Sep 17 00:00:00 2001 From: Moxie Marlinspike Date: Fri, 13 Jun 2014 14:22:53 -0700 Subject: [PATCH] Fixes for outgoing SMS/MMS direct and fallback behavior. 1) Correct MMS fallback settings. 2) Prevent SMS/MMS messages from leaking out under certain circumstances when they shouldn't. --- .../ApplicationPreferencesActivity.java | 7 +-- .../securesms/ConversationActivity.java | 2 +- .../components/OutgoingSmsPreference.java | 12 ++-- .../securesms/contacts/ContactsDatabase.java | 3 +- .../securesms/transport/MmsTransport.java | 5 -- .../securesms/transport/SmsTransport.java | 4 -- .../transport/UniversalTransport.java | 56 ++++++++++++++----- .../securesms/util/TextSecurePreferences.java | 31 +++++----- 8 files changed, 68 insertions(+), 52 deletions(-) diff --git a/src/org/thoughtcrime/securesms/ApplicationPreferencesActivity.java b/src/org/thoughtcrime/securesms/ApplicationPreferencesActivity.java index 10c737d748..33ba85a895 100644 --- a/src/org/thoughtcrime/securesms/ApplicationPreferencesActivity.java +++ b/src/org/thoughtcrime/securesms/ApplicationPreferencesActivity.java @@ -85,7 +85,6 @@ public class ApplicationPreferencesActivity extends PassphraseRequiredSherlockPr private static final String PUSH_MESSAGING_PREF = "pref_toggle_push_messaging"; private static final String MMS_PREF = "pref_mms_preferences"; private static final String KITKAT_DEFAULT_PREF = "pref_set_default"; - private static final String UPDATE_DIRECTORY_PREF = "pref_update_directory"; private static final String SUBMIT_DEBUG_LOG_PREF = "pref_submit_debug_logs"; private static final String OUTGOING_SMS_PREF = "pref_outgoing_sms"; @@ -566,9 +565,9 @@ public class ApplicationPreferencesActivity extends PassphraseRequiredSherlockPr private String buildOutgoingSmsDescription() { final StringBuilder builder = new StringBuilder(); - final boolean dataFallback = TextSecurePreferences.isSmsFallbackEnabled(this); - final boolean dataFallbackAsk = TextSecurePreferences.isSmsFallbackAskEnabled(this); - final boolean nonData = TextSecurePreferences.isSmsNonDataOutEnabled(this); + final boolean dataFallback = TextSecurePreferences.isFallbackSmsAllowed(this); + final boolean dataFallbackAsk = TextSecurePreferences.isFallbackSmsAskRequired(this); + final boolean nonData = TextSecurePreferences.isDirectSmsAllowed(this); if (dataFallback) { builder.append(getString(R.string.preferences__sms_outgoing_push_users)); diff --git a/src/org/thoughtcrime/securesms/ConversationActivity.java b/src/org/thoughtcrime/securesms/ConversationActivity.java index b3acf3a357..683728647d 100644 --- a/src/org/thoughtcrime/securesms/ConversationActivity.java +++ b/src/org/thoughtcrime/securesms/ConversationActivity.java @@ -658,7 +658,7 @@ public class ConversationActivity extends PassphraseRequiredSherlockFragmentActi private void initializeCharactersLeftViewEnabledCheck() { isCharactersLeftViewEnabled = !(isPushGroupConversation() || - (TextSecurePreferences.isPushRegistered(this) && !TextSecurePreferences.isSmsFallbackEnabled(this))); + (TextSecurePreferences.isPushRegistered(this) && !TextSecurePreferences.isFallbackSmsAllowed(this))); } private void initializeDraftFromDatabase() { diff --git a/src/org/thoughtcrime/securesms/components/OutgoingSmsPreference.java b/src/org/thoughtcrime/securesms/components/OutgoingSmsPreference.java index 3386a877b4..946b8608b1 100644 --- a/src/org/thoughtcrime/securesms/components/OutgoingSmsPreference.java +++ b/src/org/thoughtcrime/securesms/components/OutgoingSmsPreference.java @@ -26,9 +26,9 @@ public class OutgoingSmsPreference extends DialogPreference { askForFallback = (CheckBox) view.findViewById(R.id.ask_before_fallback_data); nonDataUsers = (CheckBox) view.findViewById(R.id.non_data_users); - dataUsers.setChecked(TextSecurePreferences.isSmsFallbackEnabled(getContext())); - askForFallback.setChecked(TextSecurePreferences.isSmsFallbackAskEnabled(getContext())); - nonDataUsers.setChecked(TextSecurePreferences.isSmsNonDataOutEnabled(getContext())); + dataUsers.setChecked(TextSecurePreferences.isFallbackSmsAllowed(getContext())); + askForFallback.setChecked(TextSecurePreferences.isFallbackSmsAskRequired(getContext())); + nonDataUsers.setChecked(TextSecurePreferences.isDirectSmsAllowed(getContext())); dataUsers.setOnClickListener(new View.OnClickListener() { @Override @@ -45,9 +45,9 @@ public class OutgoingSmsPreference extends DialogPreference { super.onDialogClosed(positiveResult); if (positiveResult) { - TextSecurePreferences.setSmsFallbackEnabled(getContext(), dataUsers.isChecked()); - TextSecurePreferences.setSmsFallbackAskEnabled(getContext(), askForFallback.isChecked()); - TextSecurePreferences.setSmsNonDataOutEnabled(getContext(), nonDataUsers.isChecked()); + TextSecurePreferences.setFallbackSmsAllowed(getContext(), dataUsers.isChecked()); + TextSecurePreferences.setFallbackSmsAskRequired(getContext(), askForFallback.isChecked()); + TextSecurePreferences.setDirectSmsAllowed(getContext(), nonDataUsers.isChecked()); if (getOnPreferenceChangeListener() != null) getOnPreferenceChangeListener().onPreferenceChange(this, null); } } diff --git a/src/org/thoughtcrime/securesms/contacts/ContactsDatabase.java b/src/org/thoughtcrime/securesms/contacts/ContactsDatabase.java index 007c1471a1..5431d7eb3c 100644 --- a/src/org/thoughtcrime/securesms/contacts/ContactsDatabase.java +++ b/src/org/thoughtcrime/securesms/contacts/ContactsDatabase.java @@ -27,7 +27,6 @@ import android.database.sqlite.SQLiteOpenHelper; import android.net.Uri; import android.provider.ContactsContract; import android.util.Log; -import android.util.Pair; import org.thoughtcrime.securesms.R; import org.thoughtcrime.securesms.util.NumberUtil; @@ -95,7 +94,7 @@ public class ContactsDatabase { } public Cursor query(String filter, boolean pushOnly) { - final boolean includeAndroidContacts = !pushOnly && TextSecurePreferences.isSmsNonDataOutEnabled(context); + final boolean includeAndroidContacts = !pushOnly && TextSecurePreferences.isDirectSmsAllowed(context); final Cursor localCursor = queryLocalDb(filter); final Cursor androidCursor; final MatrixCursor newNumberCursor; diff --git a/src/org/thoughtcrime/securesms/transport/MmsTransport.java b/src/org/thoughtcrime/securesms/transport/MmsTransport.java index 1c18fd07d9..a33b73556e 100644 --- a/src/org/thoughtcrime/securesms/transport/MmsTransport.java +++ b/src/org/thoughtcrime/securesms/transport/MmsTransport.java @@ -67,11 +67,6 @@ public class MmsTransport { public MmsSendResult deliver(SendReq message) throws UndeliverableMessageException, InsecureFallbackApprovalException { - if (TextSecurePreferences.isPushRegistered(context) && - !TextSecurePreferences.isSmsFallbackEnabled(context)) - { - throw new UndeliverableMessageException("MMS Transport is not enabled!"); - } validateDestinations(message); diff --git a/src/org/thoughtcrime/securesms/transport/SmsTransport.java b/src/org/thoughtcrime/securesms/transport/SmsTransport.java index 4817aa219c..2709c58159 100644 --- a/src/org/thoughtcrime/securesms/transport/SmsTransport.java +++ b/src/org/thoughtcrime/securesms/transport/SmsTransport.java @@ -51,10 +51,6 @@ public class SmsTransport extends BaseTransport { public void deliver(SmsMessageRecord message) throws UndeliverableMessageException, InsecureFallbackApprovalException { - if (!TextSecurePreferences.isSmsNonDataOutEnabled(context) && !TextSecurePreferences.isSmsFallbackEnabled(context)) { - throw new UndeliverableMessageException("SMS Transport is not enabled!"); - } - if (!NumberUtil.isValidSmsOrEmail(message.getIndividualRecipient().getNumber())) { throw new UndeliverableMessageException("Not a valid SMS destination! " + message.getIndividualRecipient().getNumber()); } diff --git a/src/org/thoughtcrime/securesms/transport/UniversalTransport.java b/src/org/thoughtcrime/securesms/transport/UniversalTransport.java index eff7cc51e6..bd7d6eb783 100644 --- a/src/org/thoughtcrime/securesms/transport/UniversalTransport.java +++ b/src/org/thoughtcrime/securesms/transport/UniversalTransport.java @@ -20,6 +20,7 @@ import android.content.Context; import android.util.Log; import org.thoughtcrime.securesms.database.DatabaseFactory; +import org.thoughtcrime.securesms.database.MmsDatabase; import org.thoughtcrime.securesms.database.model.SmsMessageRecord; import org.thoughtcrime.securesms.mms.MmsSendResult; import org.thoughtcrime.securesms.push.PushServiceSocketFactory; @@ -65,16 +66,21 @@ public class UniversalTransport { throws UndeliverableMessageException, UntrustedIdentityException, RetryLaterException, SecureFallbackApprovalException, InsecureFallbackApprovalException { - if (!TextSecurePreferences.isPushRegistered(context)) { + if (message.isForcedSms()) { smsTransport.deliver(message); return; } + if (!TextSecurePreferences.isPushRegistered(context)) { + deliverDirectSms(message); + return; + } + try { Recipient recipient = message.getIndividualRecipient(); - String number = Util.canonicalizeNumber(context, recipient.getNumber()); + String number = Util.canonicalizeNumber(context, recipient.getNumber()); - if (isPushTransport(number) && !message.isKeyExchange() && !message.isForcedSms()) { + if (isPushTransport(number) && !message.isKeyExchange()) { boolean isSmsFallbackSupported = isSmsFallbackSupported(number); try { @@ -89,15 +95,13 @@ public class UniversalTransport { if (isSmsFallbackSupported) fallbackOrAskApproval(message, number); else throw new RetryLaterException(ioe); } - } else if (!message.isForcedSms() && !TextSecurePreferences.isSmsNonDataOutEnabled(context)) { - throw new UndeliverableMessageException("User disallows non-push outgoing SMS"); } else { Log.w("UniversalTransport", "Using SMS as transport..."); - smsTransport.deliver(message); + deliverDirectSms(message); } } catch (InvalidNumberException e) { Log.w("UniversalTransport", e); - smsTransport.deliver(message); + deliverDirectSms(message); } } @@ -105,20 +109,24 @@ public class UniversalTransport { throws UndeliverableMessageException, RetryLaterException, UntrustedIdentityException, SecureFallbackApprovalException, InsecureFallbackApprovalException { - if (Util.isEmpty(mediaMessage.getTo())) { + if (MmsDatabase.Types.isForcedSms(mediaMessage.getDatabaseMessageBox())) { return mmsTransport.deliver(mediaMessage); } + if (Util.isEmpty(mediaMessage.getTo())) { + return deliverDirectMms(mediaMessage); + } + if (GroupUtil.isEncodedGroup(mediaMessage.getTo()[0].getString())) { return deliverGroupMessage(mediaMessage, threadId); } if (!TextSecurePreferences.isPushRegistered(context)) { - return mmsTransport.deliver(mediaMessage); + return deliverDirectMms(mediaMessage); } if (isMultipleRecipients(mediaMessage)) { - return mmsTransport.deliver(mediaMessage); + return deliverDirectMms(mediaMessage); } try { @@ -150,11 +158,11 @@ public class UniversalTransport { } } else { Log.w("UniversalTransport", "Delivering media message with MMS..."); - return mmsTransport.deliver(mediaMessage); + return deliverDirectMms(mediaMessage); } } catch (InvalidNumberException ine) { Log.w("UniversalTransport", ine); - return mmsTransport.deliver(mediaMessage); + return deliverDirectMms(mediaMessage); } } @@ -238,6 +246,26 @@ public class UniversalTransport { } } + private void deliverDirectSms(SmsMessageRecord message) + throws InsecureFallbackApprovalException, UndeliverableMessageException + { + if (TextSecurePreferences.isDirectSmsAllowed(context)) { + smsTransport.deliver(message); + } else { + throw new UndeliverableMessageException("Direct SMS delivery is disabled!"); + } + } + + private MmsSendResult deliverDirectMms(SendReq message) + throws InsecureFallbackApprovalException, UndeliverableMessageException + { + if (TextSecurePreferences.isDirectSmsAllowed(context)) { + return mmsTransport.deliver(message); + } else { + throw new UndeliverableMessageException("Direct MMS delivery is disabled!"); + } + } + public boolean isMultipleRecipients(SendReq mediaMessage) { int recipientCount = 0; @@ -257,7 +285,7 @@ public class UniversalTransport { } private boolean isSmsFallbackApprovalRequired(String destination) { - return (isSmsFallbackSupported(destination) && TextSecurePreferences.isSmsFallbackAskEnabled(context)); + return (isSmsFallbackSupported(destination) && TextSecurePreferences.isFallbackSmsAskRequired(context)); } private boolean isSmsFallbackSupported(String destination) { @@ -266,7 +294,7 @@ public class UniversalTransport { } if (TextSecurePreferences.isPushRegistered(context) && - !TextSecurePreferences.isSmsFallbackEnabled(context)) + !TextSecurePreferences.isFallbackSmsAllowed(context)) { return false; } diff --git a/src/org/thoughtcrime/securesms/util/TextSecurePreferences.java b/src/org/thoughtcrime/securesms/util/TextSecurePreferences.java index 1e8a0f7d90..2109af9c09 100644 --- a/src/org/thoughtcrime/securesms/util/TextSecurePreferences.java +++ b/src/org/thoughtcrime/securesms/util/TextSecurePreferences.java @@ -48,35 +48,34 @@ public class TextSecurePreferences { private static final String IN_THREAD_NOTIFICATION_PREF = "pref_key_inthread_notifications"; private static final String LOCAL_REGISTRATION_ID_PREF = "pref_local_registration_id"; - private static final String ALLOW_SMS_FALLBACK_PREF = "pref_allow_sms_traffic_out"; - private static final String SMS_FALLBACK_ASK_PREF = "pref_sms_fallback_ask"; - private static final String ALLOW_SMS_NON_DATA_PREF = "pref_sms_non_data_out"; + private static final String FALLBACK_SMS_ALLOWED_PREF = "pref_allow_sms_traffic_out"; + private static final String FALLBACK_SMS_ASK_REQUIRED_PREF = "pref_sms_fallback_ask"; + private static final String DIRECT_SMS_ALLOWED_PREF = "pref_sms_non_data_out"; - public static boolean isSmsFallbackEnabled(Context context) { - return getBooleanPreference(context, ALLOW_SMS_FALLBACK_PREF, true); + public static boolean isFallbackSmsAllowed(Context context) { + return getBooleanPreference(context, FALLBACK_SMS_ALLOWED_PREF, true); } - public static void setSmsFallbackEnabled(Context context, boolean enabled) { - setBooleanPreference(context, ALLOW_SMS_FALLBACK_PREF, enabled); + public static void setFallbackSmsAllowed(Context context, boolean allowed) { + setBooleanPreference(context, FALLBACK_SMS_ALLOWED_PREF, allowed); } - public static boolean isSmsNonDataOutEnabled(Context context) { - return getBooleanPreference(context, ALLOW_SMS_NON_DATA_PREF, true); + public static boolean isFallbackSmsAskRequired(Context context) { + return getBooleanPreference(context, FALLBACK_SMS_ASK_REQUIRED_PREF, false); } - public static void setSmsNonDataOutEnabled(Context context, boolean enabled) { - setBooleanPreference(context, ALLOW_SMS_NON_DATA_PREF, enabled); + public static void setFallbackSmsAskRequired(Context context, boolean required) { + setBooleanPreference(context, FALLBACK_SMS_ASK_REQUIRED_PREF, required); } - public static boolean isSmsFallbackAskEnabled(Context context) { - return getBooleanPreference(context, SMS_FALLBACK_ASK_PREF, false); + public static boolean isDirectSmsAllowed(Context context) { + return getBooleanPreference(context, DIRECT_SMS_ALLOWED_PREF, true); } - public static void setSmsFallbackAskEnabled(Context context, boolean enabled) { - setBooleanPreference(context, SMS_FALLBACK_ASK_PREF, enabled); + public static void setDirectSmsAllowed(Context context, boolean allowed) { + setBooleanPreference(context, DIRECT_SMS_ALLOWED_PREF, allowed); } - public static int getLocalRegistrationId(Context context) { return getIntegerPreference(context, LOCAL_REGISTRATION_ID_PREF, 0); }