From c6b6b8e14486b009897e4b5585f2a52477b1cde6 Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Fri, 16 Aug 2024 12:59:03 +0200 Subject: [PATCH] stop 'encrypted database' experiment this PR removes options to create encrypted database. existing encrypted databases will continue working for now. the 'encrypted database' experiment started some years ago was never finishes and already stopped working partly, eg. importing backups as encrypted accounts stopped working some years ago for newer android. in the light of Signal's "encryption key flaw", but also recent issues with worsending core things in Delta Chat, it seems wise to concentrate on the core things ("you have one job!"), and not on side topics comparable few persons are interested in. also, it will become harder to maintain that path on upcoming api changes, leaving the database open as opening is slow is needed but will become harder, this will make notifications tricy, may need additional effort to run in background etc. all that is doable, however, it will remove resources from far more important parts, dangerously up to a level that breaks the app - a focus is limited. also, no one really complained as it is already partly not-working, it was _never_ working on desktop (where it would make much more sense), removal on iOS was flawless, see https://github.com/deltachat/deltachat-ios/pull/2011 tl;dr: better leave the protection up to the operating system and concentrate on other things. --- jni/dc_wrapper.c | 6 --- .../java/com/b44t/messenger/DcAccounts.java | 1 - .../securesms/RegistrationActivity.java | 38 -------------- .../securesms/WelcomeActivity.java | 52 ++----------------- .../securesms/connect/AccountManager.java | 22 -------- src/main/res/layout/registration_activity.xml | 15 +----- 6 files changed, 6 insertions(+), 128 deletions(-) diff --git a/jni/dc_wrapper.c b/jni/dc_wrapper.c index 4a92c5b0a..f49a9c693 100644 --- a/jni/dc_wrapper.c +++ b/jni/dc_wrapper.c @@ -282,12 +282,6 @@ JNIEXPORT jint Java_com_b44t_messenger_DcAccounts_addAccount(JNIEnv *env, jobjec } -JNIEXPORT jint Java_com_b44t_messenger_DcAccounts_addClosedAccount(JNIEnv *env, jobject obj) -{ - return dc_accounts_add_closed_account(get_dc_accounts(env, obj)); -} - - JNIEXPORT jint Java_com_b44t_messenger_DcAccounts_migrateAccount(JNIEnv *env, jobject obj, jstring dbfile) { CHAR_REF(dbfile); diff --git a/src/main/java/com/b44t/messenger/DcAccounts.java b/src/main/java/com/b44t/messenger/DcAccounts.java index afe6b5041..de50793a9 100644 --- a/src/main/java/com/b44t/messenger/DcAccounts.java +++ b/src/main/java/com/b44t/messenger/DcAccounts.java @@ -27,7 +27,6 @@ public class DcAccounts { public native void setPushDeviceToken (String token); public native int addAccount (); - public native int addClosedAccount (); public native int migrateAccount (String dbfile); public native boolean removeAccount (int accountId); public native int[] getAll (); diff --git a/src/main/java/org/thoughtcrime/securesms/RegistrationActivity.java b/src/main/java/org/thoughtcrime/securesms/RegistrationActivity.java index f152ec172..f41c22896 100644 --- a/src/main/java/org/thoughtcrime/securesms/RegistrationActivity.java +++ b/src/main/java/org/thoughtcrime/securesms/RegistrationActivity.java @@ -56,7 +56,6 @@ import com.b44t.messenger.util.concurrent.ListenableFuture; import com.b44t.messenger.util.concurrent.SettableFuture; import com.google.android.material.textfield.TextInputEditText; -import org.thoughtcrime.securesms.connect.AccountManager; import org.thoughtcrime.securesms.connect.DcEventCenter; import org.thoughtcrime.securesms.connect.DcHelper; import org.thoughtcrime.securesms.permissions.Permissions; @@ -87,8 +86,6 @@ public class RegistrationActivity extends BaseActionBarActivity implements DcEve private TextView providerLink; private @Nullable DcProvider provider; - private CheckBox encryptCheckbox; - private Group advancedGroup; private ImageView advancedIcon; private ProgressDialog progressDialog; @@ -116,8 +113,6 @@ public class RegistrationActivity extends BaseActionBarActivity implements DcEve providerLink = findViewById(R.id.provider_link); providerLink.setOnClickListener(l -> onProviderLink()); - encryptCheckbox = findViewById(R.id.encrypt_checkbox); - advancedGroup = findViewById(R.id.advanced_group); advancedIcon = findViewById(R.id.advanced_icon); TextView advancedTextView = findViewById(R.id.advanced_text); @@ -248,10 +243,6 @@ public class RegistrationActivity extends BaseActionBarActivity implements DcEve certCheck.setSelection(certCheckFlags); expandAdvanced = expandAdvanced || certCheckFlags != 0; - encryptCheckbox.setHeight(0); - encryptCheckbox.setClickable(false); - encryptCheckbox.setFocusable(false); - if (expandAdvanced) { onAdvancedSettings(); } } else if (getIntent() != null && getIntent().getBundleExtra(ACCOUNT_DATA) != null) { // Companion app might have sent account data @@ -599,35 +590,6 @@ public class RegistrationActivity extends BaseActionBarActivity implements DcEve return; } - if (encryptCheckbox.isChecked()) { - AccountManager accountManager = AccountManager.getInstance(); - - if (progressDialog != null) { - progressDialog.dismiss(); - progressDialog = null; - } - - progressDialog = new ProgressDialog(this); - progressDialog.setMessage(getString(R.string.one_moment)); - progressDialog.setCanceledOnTouchOutside(false); - progressDialog.setCancelable(false); - progressDialog.show(); - - // Prevent the user from disabling the checkbox again, switching to unencrypted account is currently not implemented - encryptCheckbox.setEnabled(false); - Util.runOnBackground(() -> { - DcHelper.getEventCenter(this).removeObservers(this); - accountManager.switchToEncrypted(this); - // Event center changed, register for events again - registerForEvents(); - Util.runOnMain(this::continueLogin); - }); - } else { - continueLogin(); - } - } - - private void continueLogin() { setupConfig(); if (progressDialog != null) { diff --git a/src/main/java/org/thoughtcrime/securesms/WelcomeActivity.java b/src/main/java/org/thoughtcrime/securesms/WelcomeActivity.java index e977f695e..b141216ea 100644 --- a/src/main/java/org/thoughtcrime/securesms/WelcomeActivity.java +++ b/src/main/java/org/thoughtcrime/securesms/WelcomeActivity.java @@ -181,27 +181,11 @@ public class WelcomeActivity extends BaseActionBarActivity implements DcEventCen } else { final String backupFile = dcContext.imexHasBackup(imexDir.getAbsolutePath()); if (backupFile != null) { - - - View gl = View.inflate(this, R.layout.dialog_with_checkbox, null); - CheckBox encryptCheckbox = gl.findViewById(R.id.dialog_checkbox); - TextView msg = gl.findViewById(R.id.dialog_message); - - // If we'd use both `setMessage()` and `setView()` on the same AlertDialog, on small screens the - // "OK" and "Cancel" buttons would not be show. So, put the message into our custom view: - msg.setText(String.format(getResources().getString(R.string.import_backup_ask), backupFile) ); - encryptCheckbox.setText("Encrypt database (highly experimental, use at your own risk)"); - int[] tintAttr = new int[]{android.R.attr.textColorSecondary}; - TypedArray typedArray = obtainStyledAttributes(tintAttr); - int color = typedArray.getColor(0, Color.GRAY); - typedArray.recycle(); - encryptCheckbox.setTextColor(color); - new AlertDialog.Builder(this) .setTitle(R.string.import_backup_title) - .setView(gl) + .setMessage(String.format(getResources().getString(R.string.import_backup_ask), backupFile)) .setNegativeButton(android.R.string.cancel, null) - .setPositiveButton(android.R.string.ok, (dialog, which) -> startImport(backupFile, null, encryptCheckbox.isChecked())) + .setPositiveButton(android.R.string.ok, (dialog, which) -> startImport(backupFile, null)) .show(); } else { @@ -216,38 +200,10 @@ public class WelcomeActivity extends BaseActionBarActivity implements DcEventCen .execute(); } - private void startImport(@Nullable final String backupFile, final @Nullable Uri backupFileUri, boolean encrypt) + private void startImport(@Nullable final String backupFile, final @Nullable Uri backupFileUri) { notificationController = GenericForegroundService.startForegroundTask(this, getString(R.string.import_backup_title)); - if (encrypt) { - AccountManager accountManager = AccountManager.getInstance(); - - if (progressDialog != null) { - progressDialog.dismiss(); - progressDialog = null; - } - - progressDialog = new ProgressDialog(this); - progressDialog.setMessage(getString(R.string.one_moment)); - progressDialog.setCanceledOnTouchOutside(false); - progressDialog.setCancelable(false); - progressDialog.show(); - - Util.runOnBackground(() -> { - DcHelper.getEventCenter(this).removeObservers(this); - accountManager.switchToEncrypted(this); - // Event center changed, register for events again - registerForEvents(); - Util.runOnMain(() -> continueStartBackup(backupFile, backupFileUri)); - }); - } else { - continueStartBackup(backupFile, backupFileUri); - } - - } - - private void continueStartBackup(String backupFile, Uri backupFileUri) { if( progressDialog!=null ) { progressDialog.dismiss(); progressDialog = null; @@ -411,7 +367,7 @@ public class WelcomeActivity extends BaseActionBarActivity implements DcEventCen Log.e(TAG, " Can't import null URI"); return; } - startImport(null, uri, false); + startImport(null, uri); } } diff --git a/src/main/java/org/thoughtcrime/securesms/connect/AccountManager.java b/src/main/java/org/thoughtcrime/securesms/connect/AccountManager.java index 7014cc0c3..920715bd8 100644 --- a/src/main/java/org/thoughtcrime/securesms/connect/AccountManager.java +++ b/src/main/java/org/thoughtcrime/securesms/connect/AccountManager.java @@ -139,28 +139,6 @@ public class AccountManager { } } - /** - * Deletes the current account (must be unconfigured) and creates an encrypted account instead. - * @throws IllegalStateException if the currently selected account is already configured. - */ - public void switchToEncrypted(Activity activity) { - Log.i(TAG, "Switching to encrypted account..."); - DcAccounts accounts = DcHelper.getAccounts(activity); - DcContext selectedAccount = accounts.getSelectedAccount(); - if (selectedAccount.isConfigured() == 1) { - throw new IllegalStateException("Can't switch to encrypted account if already configured"); - } - - int selectedAccountId = selectedAccount.getAccountId(); - accounts.addClosedAccount(); - accounts.removeAccount(selectedAccountId); - - DcContext newAccount = accounts.getSelectedAccount(); - DatabaseSecret secret = DatabaseSecretProvider.getOrCreateDatabaseSecret(activity, newAccount.getAccountId()); - newAccount.open(secret.asString()); - resetDcContext(activity); - } - // ui public void showSwitchAccountMenu(Activity activity) { diff --git a/src/main/res/layout/registration_activity.xml b/src/main/res/layout/registration_activity.xml index 66c31e899..eae1b7be8 100644 --- a/src/main/res/layout/registration_activity.xml +++ b/src/main/res/layout/registration_activity.xml @@ -122,7 +122,7 @@ android:layout_height="wrap_content" android:visibility="gone" tools:visibility="visible" - app:constraint_referenced_ids="encrypt_checkbox, inbox, imap_login, imap_server, imap_port, imap_security_label, imap_security, outbox_view_spacer_top, + app:constraint_referenced_ids="inbox, imap_login, imap_server, imap_port, imap_security_label, imap_security, outbox_view_spacer_top, outbox, smtp_login, smtp_password, smtp_server, smtp_port, smtp_security_label, smtp_security, auth_method_label, auth_method, cert_check_label, cert_check, view_log_button, socks5_switch" /> - - + app:layout_constraintTop_toBottomOf="@id/advanced_text" />