diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index dad8575d..76b694ab 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -51,7 +51,7 @@ android:screenOrientation="locked" android:theme="@style/AppTheme" /> @@ -117,7 +117,7 @@ android:stopWithTask="false" android:foregroundServiceType="dataSync" /> diff --git a/app/src/main/java/app/notesr/activity/FsaResolver.java b/app/src/main/java/app/notesr/activity/FsaResolver.java index 0f0697de..a34e72ed 100644 --- a/app/src/main/java/app/notesr/activity/FsaResolver.java +++ b/app/src/main/java/app/notesr/activity/FsaResolver.java @@ -7,9 +7,9 @@ import java.util.Set; -import app.notesr.activity.security.ReEncryptionActivity; +import app.notesr.activity.security.SecretsUpdateActivity; import app.notesr.service.AndroidServiceRegistry; -import app.notesr.service.security.crypto.update.SecretsUpdateAndroidService; +import app.notesr.service.security.rotation.SecretsUpdateAndroidService; import lombok.RequiredArgsConstructor; /** @@ -30,7 +30,7 @@ public final class FsaResolver { // new FsaEntry(AppMigrationAndroidService.class, MigrationActivity.class), // new FsaEntry(ExportAndroidService.class, ExportActivity.class), // new FsaEntry(ImportAndroidService.class, ImportActivity.class), - new FsaEntry(SecretsUpdateAndroidService.class, ReEncryptionActivity.class) + new FsaEntry(SecretsUpdateAndroidService.class, SecretsUpdateActivity.class) ); private final AndroidServiceRegistry servicesRegistry; diff --git a/app/src/main/java/app/notesr/activity/note/list/NotesListActivity.java b/app/src/main/java/app/notesr/activity/note/list/NotesListActivity.java index adce0906..eb2b97b7 100644 --- a/app/src/main/java/app/notesr/activity/note/list/NotesListActivity.java +++ b/app/src/main/java/app/notesr/activity/note/list/NotesListActivity.java @@ -105,7 +105,7 @@ public boolean onCreateOptionsMenu(Menu menu) { menuActions.put(R.id.lockAppButton, lockAction::lock); menuActions.put(R.id.changePasswordMenuItem, this::startChangePasswordActivity); - menuActions.put(R.id.generateNewKeyMenuItem, generateNewKeyAction::startActivity); + menuActions.put(R.id.rotateKey, generateNewKeyAction::startActivity); menuActions.put(R.id.exportMenuItem, () -> startActivity(new Intent(this, ExportActivity.class))); menuActions.put(R.id.importMenuItem, diff --git a/app/src/main/java/app/notesr/activity/security/AuthActivity.java b/app/src/main/java/app/notesr/activity/security/AuthActivity.java index c778e757..596e03c9 100644 --- a/app/src/main/java/app/notesr/activity/security/AuthActivity.java +++ b/app/src/main/java/app/notesr/activity/security/AuthActivity.java @@ -20,6 +20,7 @@ import app.notesr.activity.ActivityBase; import app.notesr.core.util.SecureStringBuilder; import app.notesr.service.security.AppSecurityService; +import app.notesr.service.security.rotation.SecretsRotationService; import lombok.AllArgsConstructor; import lombok.Getter; @@ -57,7 +58,11 @@ protected void onCreate(Bundle savedInstanceState) { String mode = getIntent().getStringExtra(EXTRA_MODE); var appSecurityService = new AppSecurityService(getApplicationContext()); - authHandler = new AuthHandler(this, appSecurityService, passwordBuilder); + var secretsRotationService = new SecretsRotationService(getApplicationContext(), + appSecurityService); + + authHandler = new AuthHandler(this, appSecurityService, secretsRotationService, + passwordBuilder); try { currentMode = Mode.valueOf(mode); diff --git a/app/src/main/java/app/notesr/activity/security/AuthHandler.java b/app/src/main/java/app/notesr/activity/security/AuthHandler.java index 8a801f5a..97da88fe 100644 --- a/app/src/main/java/app/notesr/activity/security/AuthHandler.java +++ b/app/src/main/java/app/notesr/activity/security/AuthHandler.java @@ -34,6 +34,7 @@ import app.notesr.service.security.AppSecurityException; import app.notesr.service.security.AppSecurityService; import app.notesr.service.security.AuthenticationFailedException; +import app.notesr.service.security.rotation.SecretsRotationService; import lombok.RequiredArgsConstructor; @RequiredArgsConstructor @@ -43,6 +44,7 @@ public final class AuthHandler { private final AuthActivity activity; private final AppSecurityService appSecurityService; + private final SecretsRotationService secretsRotationService; private final SecureStringBuilder passwordBuilder; private int attempts = MAX_ATTEMPTS; @@ -119,21 +121,20 @@ public void recoverKey() { public void changePassword() { char[] password = proceedPasswordSetting(); - if (password != null) { - try { - Context context = activity.getApplicationContext(); - CryptoSecrets secrets = appSecurityService.getActualSecrets(); + if (password == null) { + // New password entered, but not confirmed (repeated by user) + return; + } - secrets.setPassword(password); - appSecurityService.setSecrets(secrets); - secrets.destroy(); + try { + secretsRotationService.updatePassword(password); - showToastMessage(R.string.updated); - activity.startActivity(new Intent(context, NotesListActivity.class)); - activity.finish(); - } catch (Exception e) { - throw new RuntimeException(e); - } + showToastMessage(R.string.updated); + activity.startActivity(new Intent(activity.getApplicationContext(), + NotesListActivity.class)); + activity.finish(); + } catch (Exception e) { + throw new RuntimeException(e); } } diff --git a/app/src/main/java/app/notesr/activity/security/KeySetupCompletionHandler.java b/app/src/main/java/app/notesr/activity/security/KeySetupCompletionHandler.java index ad14fbcf..68587544 100644 --- a/app/src/main/java/app/notesr/activity/security/KeySetupCompletionHandler.java +++ b/app/src/main/java/app/notesr/activity/security/KeySetupCompletionHandler.java @@ -25,7 +25,7 @@ import app.notesr.core.security.dto.CryptoSecrets; import app.notesr.service.security.AppSecurityService; import app.notesr.service.migration.DataVersionManager; -import app.notesr.service.security.crypto.update.SecretsUpdateAndroidService; +import app.notesr.service.security.rotation.SecretsUpdateAndroidService; import lombok.RequiredArgsConstructor; @RequiredArgsConstructor @@ -73,7 +73,7 @@ private void proceedFirstRun() { private void proceedRegeneration() { new DialogFactory(activity) - .getThemedAlertDialogBuilder(R.layout.dialog_re_encryption_warning) + .getThemedAlertDialogBuilder(R.layout.dialog_secrets_rotation_warning) .setTitle(R.string.warning) .setPositiveButton(R.string.yes, (dialog, which) -> onRegenerationConfirmed()) @@ -94,10 +94,10 @@ private void onRegenerationConfirmed() { throw new RuntimeException(e); } - Intent reEncryptionIntent = new Intent(activity.getApplicationContext(), - ReEncryptionActivity.class); + Intent secretsUpdateIntent = new Intent(activity.getApplicationContext(), + SecretsUpdateActivity.class); - activity.startActivity(reEncryptionIntent); + activity.startActivity(secretsUpdateIntent); activity.finish(); } diff --git a/app/src/main/java/app/notesr/activity/security/ReEncryptionActivity.java b/app/src/main/java/app/notesr/activity/security/SecretsUpdateActivity.java similarity index 75% rename from app/src/main/java/app/notesr/activity/security/ReEncryptionActivity.java rename to app/src/main/java/app/notesr/activity/security/SecretsUpdateActivity.java index 3b72e397..8003847a 100644 --- a/app/src/main/java/app/notesr/activity/security/ReEncryptionActivity.java +++ b/app/src/main/java/app/notesr/activity/security/SecretsUpdateActivity.java @@ -20,26 +20,26 @@ import app.notesr.activity.DialogFactory; import app.notesr.activity.note.list.NotesListActivity; import app.notesr.service.AndroidServiceRegistry; -import app.notesr.service.security.crypto.update.SecretsUpdateAndroidService; -import app.notesr.service.security.crypto.update.SecretsUpdateAndroidServiceStarter; +import app.notesr.service.security.rotation.SecretsUpdateAndroidService; +import app.notesr.service.security.rotation.SecretsUpdateAndroidServiceStarter; -public final class ReEncryptionActivity extends ActivityBase { +public final class SecretsUpdateActivity extends ActivityBase { @Override protected void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); - setContentView(R.layout.activity_re_encryption); + setContentView(R.layout.activity_secrets_update); applyInsets(findViewById(R.id.main)); disableBackButton(this); - ReEncryptionBroadcastReceiver broadcastReceiver = - new ReEncryptionBroadcastReceiver(this::onReEncryptionComplete, - this::onReEncryptionFailed); + SecretsUpdateBroadcastReceiver broadcastReceiver = + new SecretsUpdateBroadcastReceiver(this::onSecretsUpdateComplete, + this::onSecretsUpdateFailed); LocalBroadcastManager.getInstance(this).registerReceiver(broadcastReceiver, new IntentFilter(SecretsUpdateAndroidService.BROADCAST_ACTION)); - startReEncryptionService(); + startSecretsUpdateService(); } @Override @@ -47,7 +47,7 @@ protected boolean requiresSession() { return false; } - private void startReEncryptionService() { + private void startSecretsUpdateService() { AndroidServiceRegistry serviceRegistry = AndroidServiceRegistry .getInstance(getApplicationContext()); @@ -60,14 +60,14 @@ private void startReEncryptionService() { } } - private void onReEncryptionComplete() { + private void onSecretsUpdateComplete() { startActivity(new Intent(getApplicationContext(), NotesListActivity.class)); finish(); } - private void onReEncryptionFailed() { + private void onSecretsUpdateFailed() { DialogFactory dialogFactory = new DialogFactory(this); - dialogFactory.getThemedAlertDialogBuilder(R.layout.dialog_re_encryption_failed) + dialogFactory.getThemedAlertDialogBuilder(R.layout.dialog_secrets_update_failed) .setTitle(R.string.error) .setCancelable(false) .setPositiveButton(R.string.ok, (dialog, which) -> { diff --git a/app/src/main/java/app/notesr/activity/security/ReEncryptionBroadcastReceiver.java b/app/src/main/java/app/notesr/activity/security/SecretsUpdateBroadcastReceiver.java similarity index 69% rename from app/src/main/java/app/notesr/activity/security/ReEncryptionBroadcastReceiver.java rename to app/src/main/java/app/notesr/activity/security/SecretsUpdateBroadcastReceiver.java index 3a5708a8..882a114c 100644 --- a/app/src/main/java/app/notesr/activity/security/ReEncryptionBroadcastReceiver.java +++ b/app/src/main/java/app/notesr/activity/security/SecretsUpdateBroadcastReceiver.java @@ -9,13 +9,13 @@ import android.content.Context; import android.content.Intent; -import app.notesr.service.security.crypto.update.SecretsUpdateAndroidService; +import app.notesr.service.security.rotation.SecretsUpdateAndroidService; import lombok.RequiredArgsConstructor; @RequiredArgsConstructor -public final class ReEncryptionBroadcastReceiver extends BroadcastReceiver { - private final Runnable onReEncryptionComplete; - private final Runnable onReEncryptionFailed; +public final class SecretsUpdateBroadcastReceiver extends BroadcastReceiver { + private final Runnable onSecretsUpdateComplete; + private final Runnable onSecretsUpdateFailed; @Override public void onReceive(Context context, Intent intent) { @@ -27,9 +27,9 @@ public void onReceive(Context context, Intent intent) { false); if (isCompleted) { - onReEncryptionComplete.run(); + onSecretsUpdateComplete.run(); } else if (isFailed) { - onReEncryptionFailed.run(); + onSecretsUpdateFailed.run(); } } } diff --git a/app/src/main/res/layout/activity_re_encryption.xml b/app/src/main/res/layout/activity_secrets_update.xml similarity index 80% rename from app/src/main/res/layout/activity_re_encryption.xml rename to app/src/main/res/layout/activity_secrets_update.xml index 046970a2..003a4e7e 100644 --- a/app/src/main/res/layout/activity_re_encryption.xml +++ b/app/src/main/res/layout/activity_secrets_update.xml @@ -6,10 +6,10 @@ android:layout_width="match_parent" android:layout_height="match_parent" android:background="@color/activity_background" - tools:context=".activity.security.ReEncryptionActivity"> + tools:context=".activity.security.SecretsUpdateActivity"> + app:layout_constraintTop_toBottomOf="@+id/secretsUpdateTitleLabel" /> + app:layout_constraintTop_toBottomOf="@+id/secretsUpdateProgressBar" /> \ No newline at end of file diff --git a/app/src/main/res/layout/dialog_re_encryption_warning.xml b/app/src/main/res/layout/dialog_secrets_rotation_warning.xml similarity index 100% rename from app/src/main/res/layout/dialog_re_encryption_warning.xml rename to app/src/main/res/layout/dialog_secrets_rotation_warning.xml diff --git a/app/src/main/res/layout/dialog_re_encryption_failed.xml b/app/src/main/res/layout/dialog_secrets_update_failed.xml similarity index 93% rename from app/src/main/res/layout/dialog_re_encryption_failed.xml rename to app/src/main/res/layout/dialog_secrets_update_failed.xml index d041268a..5ed5a0e8 100644 --- a/app/src/main/res/layout/dialog_re_encryption_failed.xml +++ b/app/src/main/res/layout/dialog_secrets_update_failed.xml @@ -7,7 +7,7 @@ android:background="@color/dialog_background"> - + \ No newline at end of file diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index f6baaf31..a24b45bd 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -48,7 +48,7 @@ Change access code Create new access code Updated! - Regenerate or import key + Change private key This action will cause all of your data to be re-encrypted.\nAre you sure? Re-encrypting data… Warning diff --git a/core/src/main/java/app/notesr/core/security/dto/CryptoSecrets.java b/core/src/main/java/app/notesr/core/security/dto/CryptoSecrets.java index 862af912..4e6ee485 100644 --- a/core/src/main/java/app/notesr/core/security/dto/CryptoSecrets.java +++ b/core/src/main/java/app/notesr/core/security/dto/CryptoSecrets.java @@ -7,10 +7,9 @@ import java.util.Arrays; -import app.notesr.core.util.CharUtils; -import app.notesr.core.util.KeyUtils; import lombok.AllArgsConstructor; import lombok.Data; +import lombok.EqualsAndHashCode; /** * Data transfer object containing cryptographic secrets. @@ -20,6 +19,7 @@ */ @AllArgsConstructor @Data +@EqualsAndHashCode public final class CryptoSecrets { public static final int MASTER_KEY_SIZE = 48; diff --git a/service/src/main/java/app/notesr/service/security/AppSecurityService.java b/service/src/main/java/app/notesr/service/security/AppSecurityService.java index 049d0ee3..37f22dad 100644 --- a/service/src/main/java/app/notesr/service/security/AppSecurityService.java +++ b/service/src/main/java/app/notesr/service/security/AppSecurityService.java @@ -17,6 +17,7 @@ import app.notesr.core.security.crypto.CryptoManager; import app.notesr.core.security.crypto.CryptoManagerProvider; import app.notesr.core.security.dto.CryptoSecrets; +import app.notesr.core.security.exception.SessionExpiredException; import app.notesr.core.util.CryptoSecretsValidator; import app.notesr.data.DatabaseProvider; import lombok.RequiredArgsConstructor; @@ -79,6 +80,7 @@ public byte[] generateMasterKey() { * Retrieves the currently configured cryptographic secrets. * * @return the current {@link CryptoSecrets} if configured, or null if not yet initialized + * @throws SessionExpiredException if secrets are not configured or have expired. * @see #isAuthConfigured() */ public CryptoSecrets getActualSecrets() { diff --git a/service/src/main/java/app/notesr/service/security/crypto/update/SecretsUpdateFailedException.java b/service/src/main/java/app/notesr/service/security/crypto/update/SecretsUpdateFailedException.java deleted file mode 100644 index 0e39974f..00000000 --- a/service/src/main/java/app/notesr/service/security/crypto/update/SecretsUpdateFailedException.java +++ /dev/null @@ -1,17 +0,0 @@ -/* - * Copyright (c) 2026 zHd4 - * SPDX-License-Identifier: MIT - */ - -package app.notesr.service.security.crypto.update; - -public final class SecretsUpdateFailedException extends RuntimeException { - - public SecretsUpdateFailedException(String message) { - super(message); - } - - public SecretsUpdateFailedException(String message, Throwable cause) { - super(message, cause); - } -} diff --git a/service/src/main/java/app/notesr/service/security/crypto/update/DatabaseManager.java b/service/src/main/java/app/notesr/service/security/rotation/DatabaseManager.java similarity index 83% rename from service/src/main/java/app/notesr/service/security/crypto/update/DatabaseManager.java rename to service/src/main/java/app/notesr/service/security/rotation/DatabaseManager.java index 87f17ede..9f1b37d2 100644 --- a/service/src/main/java/app/notesr/service/security/crypto/update/DatabaseManager.java +++ b/service/src/main/java/app/notesr/service/security/rotation/DatabaseManager.java @@ -3,13 +3,13 @@ * SPDX-License-Identifier: MIT */ -package app.notesr.service.security.crypto.update; +package app.notesr.service.security.rotation; import app.notesr.data.AppDatabase; /** * Interface for managing database instances and the global database provider state - * during secrets updates. + * during secrets rotation. */ public interface DatabaseManager { AppDatabase getDatabase(String name, byte[] key); diff --git a/service/src/main/java/app/notesr/service/security/crypto/update/DatabaseManagerImpl.java b/service/src/main/java/app/notesr/service/security/rotation/DatabaseManagerImpl.java similarity index 96% rename from service/src/main/java/app/notesr/service/security/crypto/update/DatabaseManagerImpl.java rename to service/src/main/java/app/notesr/service/security/rotation/DatabaseManagerImpl.java index 84117feb..2c2fd279 100644 --- a/service/src/main/java/app/notesr/service/security/crypto/update/DatabaseManagerImpl.java +++ b/service/src/main/java/app/notesr/service/security/rotation/DatabaseManagerImpl.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: MIT */ -package app.notesr.service.security.crypto.update; +package app.notesr.service.security.rotation; import android.content.Context; import android.util.Log; diff --git a/service/src/main/java/app/notesr/service/security/rotation/SecretsRotationFailedException.java b/service/src/main/java/app/notesr/service/security/rotation/SecretsRotationFailedException.java new file mode 100644 index 00000000..db218b47 --- /dev/null +++ b/service/src/main/java/app/notesr/service/security/rotation/SecretsRotationFailedException.java @@ -0,0 +1,17 @@ +/* + * Copyright (c) 2026 zHd4 + * SPDX-License-Identifier: MIT + */ + +package app.notesr.service.security.rotation; + +public final class SecretsRotationFailedException extends RuntimeException { + + public SecretsRotationFailedException(String message) { + super(message); + } + + public SecretsRotationFailedException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/service/src/main/java/app/notesr/service/security/crypto/update/SecretsUpdateService.java b/service/src/main/java/app/notesr/service/security/rotation/SecretsRotationService.java similarity index 75% rename from service/src/main/java/app/notesr/service/security/crypto/update/SecretsUpdateService.java rename to service/src/main/java/app/notesr/service/security/rotation/SecretsRotationService.java index 0446030a..bd73102c 100644 --- a/service/src/main/java/app/notesr/service/security/crypto/update/SecretsUpdateService.java +++ b/service/src/main/java/app/notesr/service/security/rotation/SecretsRotationService.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: MIT */ -package app.notesr.service.security.crypto.update; +package app.notesr.service.security.rotation; import android.content.Context; @@ -14,7 +14,6 @@ import app.notesr.core.security.crypto.AesCryptor; import app.notesr.core.security.crypto.AesCryptorFactory; -import app.notesr.core.security.crypto.CryptoManager; import app.notesr.core.security.dto.CryptoSecrets; import app.notesr.core.security.exception.DecryptionFailedException; import app.notesr.core.security.exception.EncryptionFailedException; @@ -23,6 +22,7 @@ import app.notesr.data.AppDatabase; import app.notesr.data.model.FileBlobInfo; import app.notesr.service.file.FileService; +import app.notesr.service.security.AppSecurityService; import lombok.RequiredArgsConstructor; /** @@ -32,26 +32,66 @@ * to the new encryption settings in a transactional manner. */ @RequiredArgsConstructor -public final class SecretsUpdateService { +public final class SecretsRotationService { private final Context context; - private final DatabaseManager databaseManager; + private final AppSecurityService appSecurityService; + + /** + * Updates the password in the crypto secrets. + * After the update, the new password are securely cleared to minimize sensitive data + * exposure in memory. + * + * @param newPassword The new password to set. + * @throws IllegalArgumentException If the new password is invalid. + * @throws SecretsRotationFailedException If the password update fails. + */ + public void updatePassword(char[] newPassword) { + try { + CryptoSecretsValidator.validatePassword(newPassword); + } catch (IllegalArgumentException e) { + throw new IllegalArgumentException("Invalid password", e); + } + + CryptoSecrets currentSecrets = null; + + try { + currentSecrets = appSecurityService.getActualSecrets(); + currentSecrets.setPassword(newPassword); + appSecurityService.setSecrets(CryptoSecrets.from(currentSecrets)); + } catch (Exception e) { + throw new SecretsRotationFailedException("Failed to update password", e); + } finally { + if (currentSecrets != null) { + // Also fills the new password with \0 + currentSecrets.destroy(); + } + } + } /** * Updates the crypto secrets (master key and password) and migrates all encrypted data. + * This could be heavy and long-term operation, so it should be executed + * using {@link SecretsUpdateAndroidServiceStarter}. *

* It performs a migration of the database and file blobs to the new encryption settings. + * After the migration, the newSecrets are destroyed to minimize sensitive data + * exposure in memory. * - * @param txFiles The transactional files utility. - * @param cryptoManager The crypto manager instance. - * @param dbName The name of the database file. - * @param stateHolder The state holder for tracking update progress. - * @param newSecrets The new crypto secrets to be applied. - * @throws SecretsUpdateFailedException If the secrets update fails. + * @param txFiles The transactional files utility. + * @param databaseManager The database manager for handling database operations. + * @param dbName The name of the database file. + * @param stateHolder The state holder for tracking rotation progress. + * @param newSecrets The new crypto secrets to be applied. + * + * @throws IllegalArgumentException If the new secrets are invalid. + * @throws SecretsRotationFailedException If the secrets rotation fails. + * @see SecretsUpdateAndroidService + * @see SecretsUpdateAndroidServiceStarter */ public void updateSecrets( TransactionalFilesUtil txFiles, - CryptoManager cryptoManager, + DatabaseManager databaseManager, String dbName, SecretsUpdateStateHolder stateHolder, CryptoSecrets newSecrets) { @@ -59,12 +99,14 @@ public void updateSecrets( try { CryptoSecretsValidator.validate(newSecrets); } catch (IllegalArgumentException e) { - throw new SecretsUpdateFailedException("Invalid new secrets", e); + throw new IllegalArgumentException("Invalid new secrets", e); } - var currentSecrets = cryptoManager.getSecrets(); + CryptoSecrets currentSecrets = null; try (txFiles) { + currentSecrets = appSecurityService.getActualSecrets(); + if (getStatus(stateHolder) == null) { setStatus(stateHolder, SecretsUpdateStatus.INITIALIZING); } @@ -74,18 +116,19 @@ public void updateSecrets( } if (getStatus(stateHolder) == SecretsUpdateStatus.FAILED) { - throw new SecretsUpdateFailedException("Secrets update is already failed"); + throw new SecretsRotationFailedException("Secrets rotation is already failed"); } databaseManager.closeProvider(); if (!txFiles.isCommitted()) { - var currentCryptor = AesCryptorFactory.createAesGcmCryptor(currentSecrets); - var newCryptor = AesCryptorFactory.createAesGcmCryptor(newSecrets); - var currentBlobsDir = txFiles.getInternalFile(context, FileService.BLOBS_DIR_NAME); + AesCryptor currentCryptor = AesCryptorFactory.createAesGcmCryptor(currentSecrets); + AesCryptor newCryptor = AesCryptorFactory.createAesGcmCryptor(newSecrets); + File currentBlobsDir = txFiles.getInternalFile(context, FileService.BLOBS_DIR_NAME); migrateData( txFiles, + databaseManager, stateHolder, dbName, currentSecrets.getKey(), @@ -102,16 +145,19 @@ public void updateSecrets( } } - cryptoManager.setSecrets(context, newSecrets); + appSecurityService.setSecrets(CryptoSecrets.from(newSecrets)); setStatus(stateHolder, SecretsUpdateStatus.DONE); databaseManager.reinitProvider(newSecrets.getKey()); } catch (Exception e) { txFiles.rollback(); setStatus(stateHolder, SecretsUpdateStatus.FAILED); - throw new SecretsUpdateFailedException("Secrets update failed", e); + throw new SecretsRotationFailedException("Secrets rotation failed", e); } finally { - currentSecrets.destroy(); + if (currentSecrets != null) { + currentSecrets.destroy(); + } + newSecrets.destroy(); } } @@ -120,6 +166,7 @@ public void updateSecrets( * Migrates the database and file blobs from the current encryption settings to the new ones. * * @param txFiles The transactional files utility. + * @param databaseManager The database manager for handling database operations. * @param stateHolder The state holder for the update process. * @param dbName The name of the database to migrate. * @param currentKey The current database encryption key. @@ -133,6 +180,7 @@ public void updateSecrets( */ void migrateData( TransactionalFilesUtil txFiles, + DatabaseManager databaseManager, SecretsUpdateStateHolder stateHolder, String dbName, byte[] currentKey, diff --git a/service/src/main/java/app/notesr/service/security/crypto/update/SecretsUpdateAndroidService.java b/service/src/main/java/app/notesr/service/security/rotation/SecretsUpdateAndroidService.java similarity index 81% rename from service/src/main/java/app/notesr/service/security/crypto/update/SecretsUpdateAndroidService.java rename to service/src/main/java/app/notesr/service/security/rotation/SecretsUpdateAndroidService.java index 65fb0564..b1cf4eca 100644 --- a/service/src/main/java/app/notesr/service/security/crypto/update/SecretsUpdateAndroidService.java +++ b/service/src/main/java/app/notesr/service/security/rotation/SecretsUpdateAndroidService.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: MIT */ -package app.notesr.service.security.crypto.update; +package app.notesr.service.security.rotation; import static java.util.Objects.requireNonNull; import static app.notesr.core.util.CharUtils.bytesToChars; @@ -27,8 +27,6 @@ import java.nio.charset.StandardCharsets; import app.notesr.core.security.SecretCache; -import app.notesr.core.security.crypto.CryptoManager; -import app.notesr.core.security.crypto.CryptoManagerProvider; import app.notesr.core.security.dto.CryptoSecrets; import app.notesr.core.util.FilesTransactionException; @@ -38,6 +36,7 @@ import app.notesr.service.AndroidService; import app.notesr.service.AndroidServiceEntry; import app.notesr.service.AndroidServiceRegistry; +import app.notesr.service.security.AppSecurityService; import lombok.AccessLevel; import lombok.Setter; @@ -48,30 +47,34 @@ public class SecretsUpdateAndroidService extends AndroidService implements Runna public static final String NEW_KEY = "new_key"; public static final String PASSWORD = "password"; - public static final String BROADCAST_ACTION = "re_encryption_service_broadcast"; + public static final String BROADCAST_ACTION = "secrets_update_service_broadcast"; public static final String EXTRA_CURRENT_STATE = "current_state"; - public static final String EXTRA_COMPLETE = "re_encryption_complete"; - public static final String EXTRA_FAIL = "re_encryption_fail"; - private static final String CHANNEL_ID = "re_encryption_service_channel"; - private static final String CHANNEL_NAME = "Re-encryption Service Channel"; + public static final String EXTRA_COMPLETE = "update_completed"; + public static final String EXTRA_FAIL = "update_failed"; + private static final String CHANNEL_ID = "secrets_update_service"; + private static final String CHANNEL_NAME = "Key Rotation"; private String dbName; - private CryptoManager cryptoManager; + private DatabaseManager databaseManager; + private AppSecurityService appSecurityService; + private SecretsRotationService secretsRotationService; private SecretsUpdateStateHolder stateHolder; private CryptoSecrets newSecrets; - private SecretsUpdateService secretsUpdateService; private String encryptedPayload; @Override public int onStartCommand(Intent intent, int flags, int startId) { dbName = DatabaseProvider.DB_NAME; - cryptoManager = CryptoManagerProvider.getInstance(getApplicationContext()); + databaseManager = new DatabaseManagerImpl(getApplicationContext()); + appSecurityService = new AppSecurityService(getApplicationContext()); + secretsRotationService = new SecretsRotationService(getApplicationContext(), + appSecurityService); + newSecrets = getNewSecrets(); var state = (SecretsUpdateState) intent.getSerializableExtra(EXTRA_CURRENT_STATE); stateHolder = new SecretsUpdateStateHolder(this::onStateUpdate).setState(state); - secretsUpdateService = getSecretsUpdateService(); encryptedPayload = encryptPayload(getPayload()); var thread = new Thread(this); @@ -121,7 +124,7 @@ private SecretsUpdateAndroidServiceStarter.Payload getPayload() { } String encryptPayload(SecretsUpdateAndroidServiceStarter.Payload payload) { - return getEncryptedJson(new ObjectMapper(), payload, cryptoManager.getSecrets()); + return getEncryptedJson(new ObjectMapper(), payload, appSecurityService.getActualSecrets()); } String serializeState(SecretsUpdateState state) { @@ -145,13 +148,13 @@ public void run() { var transactionId = txFiles.getTransactionId(); stateHolder.setState(stateHolder.getState().setTransactionId(transactionId)); - secretsUpdateService.updateSecrets(txFiles, cryptoManager, dbName, stateHolder, + secretsRotationService.updateSecrets(txFiles, databaseManager, dbName, stateHolder, newSecrets); onComplete(); - } catch (SecretsUpdateFailedException | FilesTransactionException e) { + } catch (SecretsRotationFailedException | FilesTransactionException e) { onFail(); - Log.e(TAG, "Secrets update failed", e); + Log.e(TAG, "Secrets rotation failed", e); } finally { stopService(); } @@ -208,11 +211,4 @@ CryptoSecrets getNewSecrets() { throw new RuntimeException(e); } } - - SecretsUpdateService getSecretsUpdateService() { - var context = getApplicationContext(); - var databaseManager = new DatabaseManagerImpl(context); - - return new SecretsUpdateService(context, databaseManager); - } } diff --git a/service/src/main/java/app/notesr/service/security/crypto/update/SecretsUpdateAndroidServiceStarter.java b/service/src/main/java/app/notesr/service/security/rotation/SecretsUpdateAndroidServiceStarter.java similarity index 95% rename from service/src/main/java/app/notesr/service/security/crypto/update/SecretsUpdateAndroidServiceStarter.java rename to service/src/main/java/app/notesr/service/security/rotation/SecretsUpdateAndroidServiceStarter.java index f7ef997d..ceb53956 100644 --- a/service/src/main/java/app/notesr/service/security/crypto/update/SecretsUpdateAndroidServiceStarter.java +++ b/service/src/main/java/app/notesr/service/security/rotation/SecretsUpdateAndroidServiceStarter.java @@ -3,11 +3,11 @@ * SPDX-License-Identifier: MIT */ -package app.notesr.service.security.crypto.update; +package app.notesr.service.security.rotation; import static java.util.Objects.requireNonNull; import static app.notesr.core.util.CharUtils.charsToBytes; -import static app.notesr.service.security.crypto.update.SecretsUpdateAndroidService.EXTRA_CURRENT_STATE; +import static app.notesr.service.security.rotation.SecretsUpdateAndroidService.EXTRA_CURRENT_STATE; import android.content.Context; import android.content.Intent; diff --git a/service/src/main/java/app/notesr/service/security/crypto/update/SecretsUpdateState.java b/service/src/main/java/app/notesr/service/security/rotation/SecretsUpdateState.java similarity index 95% rename from service/src/main/java/app/notesr/service/security/crypto/update/SecretsUpdateState.java rename to service/src/main/java/app/notesr/service/security/rotation/SecretsUpdateState.java index a9bb421d..23de7e8f 100644 --- a/service/src/main/java/app/notesr/service/security/crypto/update/SecretsUpdateState.java +++ b/service/src/main/java/app/notesr/service/security/rotation/SecretsUpdateState.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: MIT */ -package app.notesr.service.security.crypto.update; +package app.notesr.service.security.rotation; import java.io.Serializable; diff --git a/service/src/main/java/app/notesr/service/security/crypto/update/SecretsUpdateStateHolder.java b/service/src/main/java/app/notesr/service/security/rotation/SecretsUpdateStateHolder.java similarity index 93% rename from service/src/main/java/app/notesr/service/security/crypto/update/SecretsUpdateStateHolder.java rename to service/src/main/java/app/notesr/service/security/rotation/SecretsUpdateStateHolder.java index 7b86ce79..d3c01eb0 100644 --- a/service/src/main/java/app/notesr/service/security/crypto/update/SecretsUpdateStateHolder.java +++ b/service/src/main/java/app/notesr/service/security/rotation/SecretsUpdateStateHolder.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: MIT */ -package app.notesr.service.security.crypto.update; +package app.notesr.service.security.rotation; import java.util.function.Consumer; diff --git a/service/src/main/java/app/notesr/service/security/crypto/update/SecretsUpdateStatus.java b/service/src/main/java/app/notesr/service/security/rotation/SecretsUpdateStatus.java similarity index 92% rename from service/src/main/java/app/notesr/service/security/crypto/update/SecretsUpdateStatus.java rename to service/src/main/java/app/notesr/service/security/rotation/SecretsUpdateStatus.java index 1dc21809..7976fbbe 100644 --- a/service/src/main/java/app/notesr/service/security/crypto/update/SecretsUpdateStatus.java +++ b/service/src/main/java/app/notesr/service/security/rotation/SecretsUpdateStatus.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: MIT */ -package app.notesr.service.security.crypto.update; +package app.notesr.service.security.rotation; import lombok.Getter; import lombok.RequiredArgsConstructor; diff --git a/service/src/test/java/app/notesr/service/security/AppSecurityServiceTest.java b/service/src/test/java/app/notesr/service/security/AppSecurityServiceTest.java index 4c555942..89c2670b 100644 --- a/service/src/test/java/app/notesr/service/security/AppSecurityServiceTest.java +++ b/service/src/test/java/app/notesr/service/security/AppSecurityServiceTest.java @@ -42,6 +42,7 @@ import app.notesr.core.security.crypto.CryptoManager; import app.notesr.core.security.crypto.CryptoManagerProvider; import app.notesr.core.security.dto.CryptoSecrets; +import app.notesr.core.security.exception.SessionExpiredException; import app.notesr.core.util.CryptoSecretsValidator; import app.notesr.data.DatabaseProvider; @@ -117,6 +118,15 @@ void testGetActualSecretsReturnsNull() { verify(mockCryptoManager).getSecrets(); } + @Test + void testGetActualSecretsThrowsSessionExpiredException() { + when(mockCryptoManager.getSecrets()) + .thenThrow(new SessionExpiredException("Session expired")); + + assertThrows(SessionExpiredException.class, () -> appSecurityService.getActualSecrets(), + "SessionExpiredException should be thrown"); + } + @Test void testIsAppBlockedReturnsTrue() { when(mockCryptoManager.isBlocked(mockContext)).thenReturn(true); diff --git a/service/src/test/java/app/notesr/service/security/crypto/update/SecretsUpdateServiceTest.java b/service/src/test/java/app/notesr/service/security/rotation/SecretsRotationServiceTest.java similarity index 54% rename from service/src/test/java/app/notesr/service/security/crypto/update/SecretsUpdateServiceTest.java rename to service/src/test/java/app/notesr/service/security/rotation/SecretsRotationServiceTest.java index 651d6243..543abf3c 100644 --- a/service/src/test/java/app/notesr/service/security/crypto/update/SecretsUpdateServiceTest.java +++ b/service/src/test/java/app/notesr/service/security/rotation/SecretsRotationServiceTest.java @@ -3,10 +3,12 @@ * SPDX-License-Identifier: MIT */ -package app.notesr.service.security.crypto.update; +package app.notesr.service.security.rotation; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; @@ -14,6 +16,7 @@ import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -22,6 +25,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; @@ -33,7 +37,6 @@ import java.util.function.Consumer; import app.notesr.core.security.crypto.AesCryptor; -import app.notesr.core.security.crypto.CryptoManager; import app.notesr.core.security.dto.CryptoSecrets; import app.notesr.core.security.exception.DecryptionFailedException; import app.notesr.core.security.exception.EncryptionFailedException; @@ -45,24 +48,29 @@ import app.notesr.data.model.FileBlobInfo; import app.notesr.data.model.FileInfo; import app.notesr.data.model.Note; +import app.notesr.service.security.AppSecurityService; @ExtendWith(MockitoExtension.class) -class SecretsUpdateServiceTest { +class SecretsRotationServiceTest { private static final int KEY_SIZE = 48; @Mock private Context context; + @Mock private DatabaseManager databaseManager; + @Mock private TransactionalFilesUtil txFiles; + @Mock - private CryptoManager cryptoManager; + private AppSecurityService appSecurityService; + @Mock private Consumer onUpdate; - private SecretsUpdateService secretsUpdateService; + private SecretsRotationService secretsRotationService; private SecretsUpdateStateHolder stateHolder; private final String dbName = "test.db"; @@ -72,7 +80,7 @@ class SecretsUpdateServiceTest { @BeforeEach void setUp() { - secretsUpdateService = new SecretsUpdateService(context, databaseManager); + secretsRotationService = new SecretsRotationService(context, appSecurityService); stateHolder = new SecretsUpdateStateHolder(onUpdate); // Initialize keys @@ -92,11 +100,12 @@ private CryptoSecrets createNewSecrets() { @Test void testUpdateSecretsAlreadyDoneReturnsImmediately() throws Exception { - when(cryptoManager.getSecrets()).thenReturn(createCurrentSecrets()); + when(appSecurityService.getActualSecrets()).thenReturn(createCurrentSecrets()); stateHolder.setState(new SecretsUpdateState().setStatus(SecretsUpdateStatus.DONE)); CryptoSecrets newSecrets = createNewSecrets(); - secretsUpdateService.updateSecrets(txFiles, cryptoManager, dbName, stateHolder, newSecrets); + secretsRotationService.updateSecrets(txFiles, databaseManager, dbName, + stateHolder, newSecrets); verify(databaseManager, never()).closeProvider(); verify(txFiles, never()).commit(); @@ -106,21 +115,24 @@ void testUpdateSecretsAlreadyDoneReturnsImmediately() throws Exception { @Test void testUpdateSecretsAlreadyFailedThrowsException() { - when(cryptoManager.getSecrets()).thenReturn(createCurrentSecrets()); + when(appSecurityService.getActualSecrets()).thenReturn(createCurrentSecrets()); stateHolder.setState(new SecretsUpdateState().setStatus(SecretsUpdateStatus.FAILED)); CryptoSecrets newSecrets = createNewSecrets(); - assertThrows(SecretsUpdateFailedException.class, () -> secretsUpdateService.updateSecrets( - txFiles, cryptoManager, dbName, stateHolder, newSecrets), + assertThrows(SecretsRotationFailedException.class, + () -> secretsRotationService.updateSecrets(txFiles, databaseManager, + dbName, stateHolder, newSecrets), "Should throw exception if status is already FAILED"); } @Test void testUpdateSecretsSuccessfulUpdateFromStart() throws Exception { - CryptoSecrets currentSecrets = createCurrentSecrets(); - CryptoSecrets newSecrets = createNewSecrets(); + CryptoSecrets currentSecrets = spy(createCurrentSecrets()); + CryptoSecrets newSecrets = spy(createNewSecrets()); - when(cryptoManager.getSecrets()).thenReturn(currentSecrets); + byte[] expectedNewKey = newKey.clone(); + + when(appSecurityService.getActualSecrets()).thenReturn(currentSecrets); when(txFiles.isCommitted()).thenReturn(false); AppDatabase newDbMock = mock(AppDatabase.class); @@ -174,52 +186,86 @@ void testUpdateSecretsSuccessfulUpdateFromStart() throws Exception { return null; }).when(tempDbMock).runInTransaction(any(Callable.class)); - secretsUpdateService.updateSecrets(txFiles, cryptoManager, dbName, stateHolder, newSecrets); + secretsRotationService.updateSecrets(txFiles, databaseManager, dbName, + stateHolder, newSecrets); verify(databaseManager).closeProvider(); verify(txFiles).commit(); - verify(cryptoManager).setSecrets(eq(context), any(CryptoSecrets.class)); + + var secretsCaptor = ArgumentCaptor.forClass(CryptoSecrets.class); + verify(appSecurityService).setSecrets(secretsCaptor.capture()); + + CryptoSecrets passedSecrets = secretsCaptor.getValue(); + assertNotSame(newSecrets, passedSecrets, + "setSecrets should receive a copy, not the original instance"); + assertArrayEquals(expectedNewKey, passedSecrets.getKey(), + "Copied secrets must preserve key bytes"); + verify(databaseManager).reinitProvider(any()); assertEquals(SecretsUpdateStatus.DONE, stateHolder.getState().getStatus(), "Status should be DONE after successful migration"); + + verify(currentSecrets).destroy(); + verify(newSecrets).destroy(); } @Test void testUpdateSecretsAlreadyCommittedUpdatesStatusToDone() throws Exception { - CryptoSecrets currentSecrets = createCurrentSecrets(); - CryptoSecrets newSecrets = createNewSecrets(); + CryptoSecrets currentSecrets = spy(createCurrentSecrets()); + CryptoSecrets newSecrets = spy(createNewSecrets()); + byte[] expectedNewKey = newKey.clone(); - stateHolder.setState(new SecretsUpdateState().setStatus(SecretsUpdateStatus.MOVING_DB_DATA)); + stateHolder.setState( + new SecretsUpdateState().setStatus(SecretsUpdateStatus.MOVING_DB_DATA)); - when(cryptoManager.getSecrets()).thenReturn(currentSecrets); + when(appSecurityService.getActualSecrets()).thenReturn(currentSecrets); when(txFiles.isCommitted()).thenReturn(true); - secretsUpdateService.updateSecrets(txFiles, cryptoManager, dbName, stateHolder, newSecrets); + secretsRotationService.updateSecrets(txFiles, databaseManager, dbName, + stateHolder, newSecrets); verify(txFiles, never()).commit(); - verify(cryptoManager).setSecrets(eq(context), any(CryptoSecrets.class)); + + var secretsCaptor = ArgumentCaptor.forClass(CryptoSecrets.class); + verify(appSecurityService).setSecrets(secretsCaptor.capture()); + + CryptoSecrets passedSecrets = secretsCaptor.getValue(); + assertNotSame(newSecrets, passedSecrets, + "setSecrets should receive a copy, not the original instance"); + assertArrayEquals(expectedNewKey, passedSecrets.getKey(), + "Copied secrets must preserve key bytes"); + verify(databaseManager).reinitProvider(any()); assertEquals(SecretsUpdateStatus.DONE, stateHolder.getState().getStatus(), "Status should be DONE if transaction was already committed"); + + verify(currentSecrets).destroy(); + verify(newSecrets).destroy(); } @Test void testUpdateSecretsMigrationFailureTriggersRollbackAndSetsFailed() { - CryptoSecrets currentSecrets = createCurrentSecrets(); - CryptoSecrets newSecrets = createNewSecrets(); + CryptoSecrets currentSecrets = spy(createCurrentSecrets()); + CryptoSecrets newSecrets = spy(createNewSecrets()); - when(cryptoManager.getSecrets()).thenReturn(currentSecrets); + when(appSecurityService.getActualSecrets()).thenReturn(currentSecrets); when(txFiles.isCommitted()).thenReturn(false); doThrow(new RuntimeException("Migration failed")) .when(txFiles).getInternalFile(any(), anyString()); - assertThrows(SecretsUpdateFailedException.class, () -> secretsUpdateService.updateSecrets( - txFiles, cryptoManager, dbName, stateHolder, newSecrets), - "Should throw SecretsUpdateFailedException and trigger rollback on migration failure"); + assertThrows(SecretsRotationFailedException.class, + () -> secretsRotationService.updateSecrets(txFiles, databaseManager, + dbName, stateHolder, newSecrets), + "Should throw SecretsRotationFailedException" + + " and trigger rollback on migration failure"); verify(txFiles).rollback(); + verify(databaseManager).closeProvider(); + verify(currentSecrets).destroy(); + verify(newSecrets).destroy(); + assertEquals(SecretsUpdateStatus.FAILED, stateHolder.getState().getStatus(), "Status should be FAILED after migration failure"); } @@ -230,7 +276,8 @@ void testMigrateDataDbAlreadyMigratedReturnsImmediately() throws Exception { when(databaseManager.getDatabase(dbName, newKey)).thenReturn(newDb); when(databaseManager.isDbAvailable(newDb)).thenReturn(true); - secretsUpdateService.migrateData(txFiles, stateHolder, dbName, currentKey, newKey, + secretsRotationService.migrateData(txFiles, databaseManager, + stateHolder, dbName, currentKey, newKey, new File("blobs"), mock(AesCryptor.class), mock(AesCryptor.class)); verify(databaseManager, never()).getDatabase(dbName, currentKey); @@ -273,7 +320,7 @@ void testCopyDbDataCopiesAllDataSuccessfully() { return null; }).when(newDb).runInTransaction(any(Callable.class)); - secretsUpdateService.copyDbData(currentDb, newDb); + secretsRotationService.copyDbData(currentDb, newDb); verify(noteDao).insertAll(notes); verify(fileInfoDao).insertAll(fileInfos); @@ -302,7 +349,7 @@ void testUpdateBlobsDataMigratesBlobsSuccessfully() throws Exception { when(currentCryptor.decrypt(oldData)).thenReturn(decryptedData); when(newCryptor.encrypt(decryptedData)).thenReturn(newData); - secretsUpdateService.updateBlobsData(txFiles, oldDb, blobsDir, currentCryptor, newCryptor); + secretsRotationService.updateBlobsData(txFiles, oldDb, blobsDir, currentCryptor, newCryptor); verify(txFiles).writeFileBytes(any(), eq(newData)); } @@ -328,7 +375,8 @@ void testUpdateBlobsDataSkipsAlreadyMigratedBlob() throws Exception { // Successful decryption with new cryptor means it's migrated when(newCryptor.decrypt(migratedData)).thenReturn("plain".getBytes()); - secretsUpdateService.updateBlobsData(txFiles, oldDb, blobsDir, currentCryptor, newCryptor); + secretsRotationService.updateBlobsData(txFiles, oldDb, blobsDir, + currentCryptor, newCryptor); verify(currentCryptor, never()).decrypt(any()); verify(newCryptor, never()).encrypt(any()); @@ -340,9 +388,10 @@ void testEncryptBlobDataWrapsGeneralSecurityException() throws Exception { AesCryptor cryptor = mock(AesCryptor.class); when(cryptor.encrypt(any())).thenThrow(new GeneralSecurityException("Encryption failed")); - assertThrows(EncryptionFailedException.class, () -> secretsUpdateService.encryptBlobData( - cryptor, new byte[0]), - "Should wrap GeneralSecurityException in EncryptionFailedException during blob encryption"); + assertThrows(EncryptionFailedException.class, + () -> secretsRotationService.encryptBlobData(cryptor, new byte[0]), + "Should wrap GeneralSecurityException in EncryptionFailedException" + + " during blob encryption"); } @Test @@ -350,14 +399,15 @@ void testDecryptBlobDataWrapsGeneralSecurityException() throws Exception { AesCryptor cryptor = mock(AesCryptor.class); when(cryptor.decrypt(any())).thenThrow(new GeneralSecurityException("Decryption failed")); - assertThrows(DecryptionFailedException.class, () -> secretsUpdateService.decryptBlobData( - cryptor, new byte[0]), - "Should wrap GeneralSecurityException in DecryptionFailedException during blob decryption"); + assertThrows(DecryptionFailedException.class, + () -> secretsRotationService.decryptBlobData(cryptor, new byte[0]), + "Should wrap GeneralSecurityException in DecryptionFailedException" + + " during blob decryption"); } @Test void testSetStatusUpdatesStateHolderAndTriggersOnUpdate() { - secretsUpdateService.setStatus(stateHolder, SecretsUpdateStatus.MOVING_DB_DATA); + secretsRotationService.setStatus(stateHolder, SecretsUpdateStatus.MOVING_DB_DATA); assertEquals(SecretsUpdateStatus.MOVING_DB_DATA, stateHolder.getState().getStatus(), "Status should be updated in the state holder"); @@ -369,7 +419,7 @@ void testGetStatusReturnsStatusFromStateHolder() { stateHolder.setState(new SecretsUpdateState() .setStatus(SecretsUpdateStatus.MOVING_BLOBS_DATA)); - SecretsUpdateStatus status = secretsUpdateService.getStatus(stateHolder); + SecretsUpdateStatus status = secretsRotationService.getStatus(stateHolder); assertEquals(SecretsUpdateStatus.MOVING_BLOBS_DATA, status, "Should return the correct status from the state holder"); @@ -379,18 +429,20 @@ void testGetStatusReturnsStatusFromStateHolder() { void testUpdateSecretsThrowsWhenNewSecretsKeyIsNull() { CryptoSecrets newSecrets = new CryptoSecrets(null, password.clone()); - assertThrows(SecretsUpdateFailedException.class, () -> secretsUpdateService.updateSecrets( - txFiles, cryptoManager, dbName, stateHolder, newSecrets), - "Should throw SecretsUpdateFailedException when new secrets key is null"); + assertThrows(IllegalArgumentException.class, + () -> secretsRotationService.updateSecrets(txFiles, databaseManager, dbName, + stateHolder, newSecrets), + "Should throw SecretsRotationFailedException when new secrets key is null"); } @Test void testUpdateSecretsThrowsWhenNewSecretsKeyIsEmpty() { CryptoSecrets newSecrets = new CryptoSecrets(new byte[0], password.clone()); - assertThrows(SecretsUpdateFailedException.class, () -> secretsUpdateService.updateSecrets( - txFiles, cryptoManager, dbName, stateHolder, newSecrets), - "Should throw SecretsUpdateFailedException when new secrets key is empty"); + assertThrows(IllegalArgumentException.class, + () -> secretsRotationService.updateSecrets(txFiles, databaseManager, dbName, + stateHolder, newSecrets), + "Should throw SecretsRotationFailedException when new secrets key is empty"); } @Test @@ -398,9 +450,11 @@ void testUpdateSecretsThrowsWhenNewSecretsKeyWrongSize() { byte[] wrongSizedKey = new byte[32]; // Wrong size, should be 48 CryptoSecrets newSecrets = new CryptoSecrets(wrongSizedKey, password.clone()); - assertThrows(SecretsUpdateFailedException.class, () -> secretsUpdateService.updateSecrets( - txFiles, cryptoManager, dbName, stateHolder, newSecrets), - "Should throw SecretsUpdateFailedException when new secrets key has wrong size"); + assertThrows(IllegalArgumentException.class, + () -> secretsRotationService.updateSecrets(txFiles, databaseManager, dbName, + stateHolder, newSecrets), + "Should throw SecretsRotationFailedException" + + " when new secrets key has wrong size"); } @Test @@ -408,27 +462,32 @@ void testUpdateSecretsThrowsWhenNewSecretsKeyIsAllZeros() { byte[] nulledKey = new byte[KEY_SIZE]; // All zeros CryptoSecrets newSecrets = new CryptoSecrets(nulledKey, password.clone()); - assertThrows(SecretsUpdateFailedException.class, () -> secretsUpdateService.updateSecrets( - txFiles, cryptoManager, dbName, stateHolder, newSecrets), - "Should throw SecretsUpdateFailedException when new secrets key is all zeros"); + assertThrows(IllegalArgumentException.class, + () -> secretsRotationService.updateSecrets(txFiles, databaseManager, dbName, + stateHolder, newSecrets), + "Should throw SecretsRotationFailedException" + + " when new secrets key is all zeros"); } @Test void testUpdateSecretsThrowsWhenNewSecretsPasswordIsNull() { CryptoSecrets newSecrets = new CryptoSecrets(newKey.clone(), null); - assertThrows(SecretsUpdateFailedException.class, () -> secretsUpdateService.updateSecrets( - txFiles, cryptoManager, dbName, stateHolder, newSecrets), - "Should throw SecretsUpdateFailedException when new secrets password is null"); + assertThrows(IllegalArgumentException.class, () -> secretsRotationService.updateSecrets( + txFiles, databaseManager, dbName, stateHolder, newSecrets), + "Should throw SecretsRotationFailedException" + + " when new secrets password is null"); } @Test void testUpdateSecretsThrowsWhenNewSecretsPasswordIsEmpty() { CryptoSecrets newSecrets = new CryptoSecrets(newKey.clone(), new char[0]); - assertThrows(SecretsUpdateFailedException.class, () -> secretsUpdateService.updateSecrets( - txFiles, cryptoManager, dbName, stateHolder, newSecrets), - "Should throw SecretsUpdateFailedException when new secrets password is empty"); + assertThrows(IllegalArgumentException.class, + () -> secretsRotationService.updateSecrets(txFiles, databaseManager, dbName, + stateHolder, newSecrets), + "Should throw SecretsRotationFailedException" + + " when new secrets password is empty"); } @Test @@ -436,9 +495,11 @@ void testUpdateSecretsThrowsWhenNewSecretsPasswordTooShort() { char[] shortPassword = "abc".toCharArray(); // Less than 4 characters CryptoSecrets newSecrets = new CryptoSecrets(newKey.clone(), shortPassword); - assertThrows(SecretsUpdateFailedException.class, () -> secretsUpdateService.updateSecrets( - txFiles, cryptoManager, dbName, stateHolder, newSecrets), - "Should throw SecretsUpdateFailedException when new secrets password is too short"); + assertThrows(IllegalArgumentException.class, + () -> secretsRotationService.updateSecrets(txFiles, databaseManager, dbName, + stateHolder, newSecrets), + "Should throw SecretsRotationFailedException" + + " when new secrets password is too short"); } @Test @@ -446,8 +507,122 @@ void testUpdateSecretsThrowsWhenNewSecretsPasswordIsAllZeros() { char[] nulledPassword = new char[4]; // All '\0' characters CryptoSecrets newSecrets = new CryptoSecrets(newKey.clone(), nulledPassword); - assertThrows(SecretsUpdateFailedException.class, () -> secretsUpdateService.updateSecrets( - txFiles, cryptoManager, dbName, stateHolder, newSecrets), - "Should throw SecretsUpdateFailedException when new secrets password is all zeros"); + assertThrows(IllegalArgumentException.class, + () -> secretsRotationService.updateSecrets(txFiles, databaseManager, dbName, + stateHolder, newSecrets), + "Should throw SecretsRotationFailedException" + + " when new secrets password is all zeros"); + } + + @Test + void testUpdateSecretsDestroysNewSecretsEvenWhenCurrentSecretsIsNull() { + CryptoSecrets newSecrets = spy(createNewSecrets()); + + when(appSecurityService.getActualSecrets()).thenReturn(null); + when(txFiles.isCommitted()).thenReturn(false); + + assertThrows(SecretsRotationFailedException.class, + () -> secretsRotationService.updateSecrets(txFiles, databaseManager, + dbName, stateHolder, newSecrets), + "Should throw SecretsRotationFailedException and still destroy newSecrets"); + + verify(newSecrets).destroy(); + } + + @Test + void testUpdatePasswordSuccessfully() { + CryptoSecrets currentSecrets = spy(createCurrentSecrets()); + char[] newPassword = "newPassword123".toCharArray(); + + when(appSecurityService.getActualSecrets()).thenReturn(currentSecrets); + + secretsRotationService.updatePassword(newPassword.clone()); + + var secretsCaptor = ArgumentCaptor.forClass(CryptoSecrets.class); + verify(appSecurityService).setSecrets(secretsCaptor.capture()); + + CryptoSecrets passedSecrets = secretsCaptor.getValue(); + assertArrayEquals(newPassword, passedSecrets.getPassword(), + "Passed secrets should contain the new password"); + + verify(currentSecrets).destroy(); + } + + @Test + void testUpdatePasswordThrowsWhenPasswordIsNull() { + assertThrows(IllegalArgumentException.class, + () -> secretsRotationService.updatePassword(null), + "Should throw IllegalArgumentException when password is null"); + } + + @Test + void testUpdatePasswordThrowsWhenPasswordIsEmpty() { + char[] emptyPassword = new char[0]; + + assertThrows(IllegalArgumentException.class, + () -> secretsRotationService.updatePassword(emptyPassword), + "Should throw IllegalArgumentException when password is empty"); + } + + @Test + void testUpdatePasswordThrowsWhenPasswordIsTooShort() { + char[] shortPassword = "abc".toCharArray(); // Less than 4 characters + + assertThrows(IllegalArgumentException.class, + () -> secretsRotationService.updatePassword(shortPassword), + "Should throw IllegalArgumentException when password is too short"); + } + + @Test + void testUpdatePasswordThrowsWhenPasswordIsAllZeros() { + char[] nulledPassword = new char[4]; // All '\0' characters + + assertThrows(IllegalArgumentException.class, + () -> secretsRotationService.updatePassword(nulledPassword), + "Should throw IllegalArgumentException when password is all zeros"); + } + + @Test + void testUpdatePasswordCleansUpOnException() { + CryptoSecrets currentSecrets = mock(CryptoSecrets.class); + char[] newPassword = "newPassword123".toCharArray(); + + when(appSecurityService.getActualSecrets()).thenReturn(currentSecrets); + doThrow(new RuntimeException("Security service error")) + .when(appSecurityService).setSecrets(any()); + + assertThrows(SecretsRotationFailedException.class, + () -> secretsRotationService.updatePassword(newPassword), + "Should throw SecretsRotationFailedException when setSecrets fails"); + + verify(currentSecrets).destroy(); + } + + @Test + void testUpdatePasswordThrowsSecretsRotationFailedWhenGetSecretsThrows() { + char[] newPassword = "newPassword123".toCharArray(); + + when(appSecurityService.getActualSecrets()) + .thenThrow(new RuntimeException("Failed to get secrets")); + + assertThrows(SecretsRotationFailedException.class, + () -> secretsRotationService.updatePassword(newPassword), + "Should throw SecretsRotationFailedException" + + " when getActualSecrets fails"); + } + + @Test + void testUpdatePasswordNotDestroysCurrentSecretsIfItsNull() { + char[] newPassword = "newPassword123".toCharArray(); + char[] newPasswordCopy = newPassword.clone(); + + when(appSecurityService.getActualSecrets()).thenReturn(null); + + assertThrows(SecretsRotationFailedException.class, + () -> secretsRotationService.updatePassword(newPassword), + "Should throw SecretsRotationFailedException and still destroy newSecrets"); + + assertArrayEquals(newPasswordCopy, newPassword, + "New password should remain unchanged after exception"); } } diff --git a/service/src/test/java/app/notesr/service/security/crypto/update/SecretsUpdateAndroidServiceStarterTest.java b/service/src/test/java/app/notesr/service/security/rotation/SecretsUpdateAndroidServiceStarterTest.java similarity index 99% rename from service/src/test/java/app/notesr/service/security/crypto/update/SecretsUpdateAndroidServiceStarterTest.java rename to service/src/test/java/app/notesr/service/security/rotation/SecretsUpdateAndroidServiceStarterTest.java index 3cb9edec..e9e691eb 100644 --- a/service/src/test/java/app/notesr/service/security/crypto/update/SecretsUpdateAndroidServiceStarterTest.java +++ b/service/src/test/java/app/notesr/service/security/rotation/SecretsUpdateAndroidServiceStarterTest.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: MIT */ -package app.notesr.service.security.crypto.update; +package app.notesr.service.security.rotation; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertThrows; diff --git a/service/src/test/java/app/notesr/service/security/crypto/update/SecretsUpdateAndroidServiceTest.java b/service/src/test/java/app/notesr/service/security/rotation/SecretsUpdateAndroidServiceTest.java similarity index 86% rename from service/src/test/java/app/notesr/service/security/crypto/update/SecretsUpdateAndroidServiceTest.java rename to service/src/test/java/app/notesr/service/security/rotation/SecretsUpdateAndroidServiceTest.java index 1455174d..bbba077f 100644 --- a/service/src/test/java/app/notesr/service/security/crypto/update/SecretsUpdateAndroidServiceTest.java +++ b/service/src/test/java/app/notesr/service/security/rotation/SecretsUpdateAndroidServiceTest.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: MIT */ -package app.notesr.service.security.crypto.update; +package app.notesr.service.security.rotation; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -36,11 +36,11 @@ import org.mockito.MockedStatic; import org.mockito.junit.jupiter.MockitoExtension; -import app.notesr.core.security.crypto.CryptoManager; import app.notesr.core.security.dto.CryptoSecrets; import app.notesr.core.util.TransactionalFilesUtil; import app.notesr.service.AndroidServiceEntry; import app.notesr.service.AndroidServiceRegistry; +import app.notesr.service.security.AppSecurityService; @ExtendWith(MockitoExtension.class) class SecretsUpdateAndroidServiceTest { @@ -52,10 +52,13 @@ class SecretsUpdateAndroidServiceTest { private Intent intent; @Mock - private CryptoManager cryptoManager; + private AppSecurityService appSecurityService; @Mock - private SecretsUpdateService secretsUpdateService; + private DatabaseManager databaseManager; + + @Mock + private SecretsRotationService secretsRotationService; @Mock private TransactionalFilesUtil txFiles; @@ -77,9 +80,10 @@ void setUp() { newSecrets = new CryptoSecrets(new byte[32], "password".toCharArray()); // Inject basic dependencies using setters - service.setCryptoManager(cryptoManager); + service.setAppSecurityService(appSecurityService); + service.setDatabaseManager(databaseManager); + service.setSecretsRotationService(secretsRotationService); service.setNewSecrets(newSecrets); - service.setSecretsUpdateService(secretsUpdateService); service.setDbName("test.db"); } @@ -96,8 +100,8 @@ void testRunSuccess() { service.run(); - verify(secretsUpdateService) - .updateSecrets(eq(txFiles), eq(cryptoManager), eq("test.db"), any(), + verify(secretsRotationService) + .updateSecrets(eq(txFiles), eq(databaseManager), eq("test.db"), any(), eq(newSecrets)); verify(service).onComplete(); verify(service).stopService(); @@ -116,8 +120,8 @@ void testRunFailure() { doNothing().when(service).stopService(); when(txFiles.getTransactionId()).thenReturn("tx-123"); - doThrow(new SecretsUpdateFailedException("Failed")) - .when(secretsUpdateService).updateSecrets(any(), any(), any(), any(), any()); + doThrow(new SecretsRotationFailedException("Failed")) + .when(secretsRotationService).updateSecrets(any(), any(), any(), any(), any()); service.run(); @@ -142,7 +146,9 @@ void testOnFailCallsSendBroadcast() { @Test void testSendUpdateBroadcast() { - try (MockedStatic mockedStatic = mockStatic(LocalBroadcastManager.class)) { + MockedStatic mockedStatic = mockStatic(LocalBroadcastManager.class); + + try (mockedStatic) { mockedStatic.when(() -> LocalBroadcastManager.getInstance(any())) .thenReturn(localBroadcastManager); @@ -162,7 +168,9 @@ void testSendUpdateBroadcast() { @Test void testOnStateUpdateUpdatesRegistry() { - try (MockedStatic mockedStatic = mockStatic(AndroidServiceRegistry.class)) { + MockedStatic mockedStatic = mockStatic(AndroidServiceRegistry.class); + + try (mockedStatic) { mockedStatic.when(() -> AndroidServiceRegistry.getInstance(any())) .thenReturn(androidServiceRegistry); @@ -191,12 +199,13 @@ void testOnStartCommand() { when(intent.getSerializableExtra(SecretsUpdateAndroidService.EXTRA_CURRENT_STATE)) .thenReturn(state); - try (MockedStatic registryMock = mockStatic(AndroidServiceRegistry.class)) { + MockedStatic registryMock = mockStatic(AndroidServiceRegistry.class); + + try (registryMock) { registryMock.when(() -> AndroidServiceRegistry.getInstance(any())) .thenReturn(androidServiceRegistry); doReturn(context).when(service).getApplicationContext(); - doReturn(secretsUpdateService).when(service).getSecretsUpdateService(); doReturn(newSecrets).when(service).getNewSecrets(); doNothing().when(service).showForegroundNotification(anyInt()); doReturn("encryptedPayload").when(service).encryptPayload(any());