From fa761198cbd96c94e89f5f8c3132adb197605dff Mon Sep 17 00:00:00 2001 From: zHd4 <38856321+zHd4@users.noreply.github.com> Date: Mon, 27 Jul 2026 13:16:28 +0200 Subject: [PATCH 1/3] Add CryptoSecretsValidator --- .../core/util/CryptoSecretsValidator.java | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 core/src/main/java/app/notesr/core/util/CryptoSecretsValidator.java diff --git a/core/src/main/java/app/notesr/core/util/CryptoSecretsValidator.java b/core/src/main/java/app/notesr/core/util/CryptoSecretsValidator.java new file mode 100644 index 00000000..1cd1afac --- /dev/null +++ b/core/src/main/java/app/notesr/core/util/CryptoSecretsValidator.java @@ -0,0 +1,79 @@ +/* + * Copyright (c) 2026 zHd4 + * SPDX-License-Identifier: MIT + */ + +package app.notesr.core.util; + +import static app.notesr.core.security.dto.CryptoSecrets.MASTER_KEY_SIZE; +import static app.notesr.core.security.dto.CryptoSecrets.PASSWORD_MIN_LENGTH; + +import app.notesr.core.security.dto.CryptoSecrets; + +public final class CryptoSecretsValidator { + /** + * Validates the cryptographic secrets. + *

+ * Combines validation checks for the key and password. + * If any validation fails, an {@link IllegalArgumentException} is thrown. + * + * @param secrets the cryptographic secrets to validate + * @throws IllegalArgumentException if any validation check fails + * @see #validateKey(byte[]) + * @see #validatePassword(char[]) + */ + public static void validate(CryptoSecrets secrets) { + validateKey(secrets.getKey()); + validatePassword(secrets.getPassword()); + } + + /** + * Validates the master key. + *

+ * Ensures that the key is not null or empty, + * that it matches the expected 384-bit (48 bytes) length, + * and that it is not nulled. + * + * @param key the master key to validate + * @throws IllegalArgumentException if the key is invalid + */ + public static void validateKey(byte[] key) { + if (key == null || key.length == 0) { + throw new IllegalArgumentException("CryptoSecrets key cannot be null or empty"); + } + + if (key.length != MASTER_KEY_SIZE) { + throw new IllegalArgumentException("Key must be " + + MASTER_KEY_SIZE + " bytes long"); + } + + if (KeyUtils.isKeyNulled(key)) { + throw new IllegalArgumentException("Key cannot be empty"); + } + } + + /** + * Validates the password. + *

+ * Ensures that the password is not null or empty, + * and that it is at least 4 characters long, + * and that it contains at least one non-zero character. + * + * @param password the password to validate + * @throws IllegalArgumentException if the password is invalid + */ + public static void validatePassword(char[] password) { + if (password == null || password.length == 0) { + throw new IllegalArgumentException("CryptoSecrets password cannot be null or empty"); + } + + if (password.length < PASSWORD_MIN_LENGTH) { + throw new IllegalArgumentException("Password must be at least " + + PASSWORD_MIN_LENGTH + " characters long"); + } + + if (!CharUtils.hasNonZeroChars(password)) { + throw new IllegalArgumentException("Password cannot be empty"); + } + } +} From 18b66e558ee5c6f194c035498c970bf7b10662e0 Mon Sep 17 00:00:00 2001 From: zHd4 <38856321+zHd4@users.noreply.github.com> Date: Mon, 27 Jul 2026 14:39:53 +0200 Subject: [PATCH 2/3] Clarify legacy migration usage in KeyUtils --- core/src/main/java/app/notesr/core/util/KeyUtils.java | 1 + 1 file changed, 1 insertion(+) diff --git a/core/src/main/java/app/notesr/core/util/KeyUtils.java b/core/src/main/java/app/notesr/core/util/KeyUtils.java index 0eb6a2d4..21da646e 100644 --- a/core/src/main/java/app/notesr/core/util/KeyUtils.java +++ b/core/src/main/java/app/notesr/core/util/KeyUtils.java @@ -81,6 +81,7 @@ public static byte[] getKeyBytesFromHex(char[] hexChars) { } } + // Used only for supporting migration from older versions of NoteSR public static byte[] getIvFromSecrets(CryptoSecrets cryptoSecrets) { int ivSize = cryptoSecrets.getKey().length - (AesCryptor.KEY_SIZE / 8); byte[] iv = new byte[ivSize]; From e3ce6f2823f9d1c83cfff9e933dc0c13cf27d9a0 Mon Sep 17 00:00:00 2001 From: zHd4 <38856321+zHd4@users.noreply.github.com> Date: Mon, 27 Jul 2026 14:44:34 +0200 Subject: [PATCH 3/3] Refactor master key management and validation logic - Move validation logic from `CryptoSecrets` to a dedicated `CryptoSecretsValidator` class. - Update `AppSecurityService` to generate master keys independently of `CryptoSecrets` via `generateMasterKey()`. - Standardize naming in `KeyUtils` for hex conversions. - Refactor `SetupKeyActivity` and `ImportKeyActivity` to handle raw key bytes and improve security by deferring password retrieval. - Ensure proper activity lifecycle management in `AuthActivityExtension` by explicitly finishing activities after navigation. - Update unit and instrumented tests. --- .../activity/note/list/NotesListActivity.java | 2 +- .../security/AuthActivityExtension.java | 14 ++-- .../security/GenerateNewKeyAction.java | 22 ----- .../activity/security/ImportKeyActivity.java | 50 +++--------- .../security/KeyRecoveryActivity.java | 4 +- .../security/KeySetupCompletionHandler.java | 47 ++++++++--- .../activity/security/SetupKeyActivity.java | 80 ++++++------------- .../core/security/dto/CryptoSecrets.java | 37 --------- .../java/app/notesr/core/util/KeyUtils.java | 26 +++--- .../app/notesr/core/util/KeyUtilsTest.java | 37 ++++----- .../service/exporter/ExportServiceTest.java | 5 +- .../service/security/AppSecurityService.java | 17 ++-- .../crypto/update/SecretsUpdateService.java | 5 +- .../security/AppSecurityServiceTest.java | 73 ++++++++++------- 14 files changed, 172 insertions(+), 247 deletions(-) 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 09fc48ac..adce0906 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 @@ -76,7 +76,7 @@ protected void onCreate(Bundle savedInstanceState) { var appSecurityService = new AppSecurityService(getApplicationContext()); lockAction = new LockAction(this, appSecurityService); - generateNewKeyAction = new GenerateNewKeyAction(this, appSecurityService); + generateNewKeyAction = new GenerateNewKeyAction(this); noteEditorLauncher = registerForActivityResult( new ActivityResultContracts.StartActivityForResult(), getOpenNoteResultCallback()); diff --git a/app/src/main/java/app/notesr/activity/security/AuthActivityExtension.java b/app/src/main/java/app/notesr/activity/security/AuthActivityExtension.java index 9a81a026..dc36ad62 100644 --- a/app/src/main/java/app/notesr/activity/security/AuthActivityExtension.java +++ b/app/src/main/java/app/notesr/activity/security/AuthActivityExtension.java @@ -73,10 +73,6 @@ public void createPassword() { char[] password = proceedPasswordSetting(); if (password != null) { - var context = activity.getApplicationContext(); - var setupKeyActivityIntent = new Intent(context, SetupKeyActivity.class) - .putExtra(SetupKeyActivity.EXTRA_MODE, KeySetupMode.FIRST_RUN.toString()); - try { var passwordBytes = charsToBytes(password, StandardCharsets.UTF_8); SecretCache.put(SetupKeyActivity.CACHE_KEY_PASSWORD, passwordBytes); @@ -84,7 +80,12 @@ public void createPassword() { throw new RuntimeException(e); } + var context = activity.getApplicationContext(); + var setupKeyActivityIntent = new Intent(context, SetupKeyActivity.class) + .putExtra(SetupKeyActivity.EXTRA_MODE, KeySetupMode.FIRST_RUN.toString()); + activity.startActivity(setupKeyActivityIntent); + activity.finish(); } } @@ -103,7 +104,7 @@ public void recoverKey() { char[] hexKey = bytesToChars(hexKeyBytes, StandardCharsets.UTF_8); - CryptoSecrets secrets = KeyUtils.getSecretsFromHex(hexKey, password); + CryptoSecrets secrets = KeyUtils.getSecretsFromKeyHexAndPassword(hexKey, password); appSecurityService.unblockApp(secrets); secrets.destroy(); } catch (AppSecurityException | CharacterCodingException e) { @@ -112,6 +113,7 @@ public void recoverKey() { activity.startActivity(new Intent(activity.getApplicationContext(), NotesListActivity.class)); + activity.finish(); } } @@ -129,6 +131,7 @@ public void changePassword() { showToastMessage(R.string.updated); activity.startActivity(new Intent(context, NotesListActivity.class)); + activity.finish(); } catch (Exception e) { throw new RuntimeException(e); } @@ -150,6 +153,7 @@ private char[] proceedPasswordSetting() { } } else { if (Arrays.equals(password, createdPassword)) { + resetPassword(); return password; } else { showToastMessage(R.string.code_not_match); diff --git a/app/src/main/java/app/notesr/activity/security/GenerateNewKeyAction.java b/app/src/main/java/app/notesr/activity/security/GenerateNewKeyAction.java index 75886aa5..9dfe7943 100644 --- a/app/src/main/java/app/notesr/activity/security/GenerateNewKeyAction.java +++ b/app/src/main/java/app/notesr/activity/security/GenerateNewKeyAction.java @@ -5,40 +5,18 @@ package app.notesr.activity.security; -import static app.notesr.core.util.CharUtils.charsToBytes; - import android.content.Context; import android.content.Intent; import app.notesr.activity.ActivityBase; -import app.notesr.core.security.SecretCache; -import app.notesr.core.security.dto.CryptoSecrets; -import app.notesr.service.security.AppSecurityService; import lombok.RequiredArgsConstructor; -import java.nio.charset.CharacterCodingException; -import java.nio.charset.StandardCharsets; -import java.util.Arrays; - @RequiredArgsConstructor public final class GenerateNewKeyAction { private final ActivityBase activity; - private final AppSecurityService appSecurityService; public void startActivity() { - CryptoSecrets secrets = appSecurityService.getActualSecrets(); - char[] passwordChars = Arrays.copyOf(secrets.getPassword(), secrets.getPassword().length); - - try { - byte[] passwordBytes = charsToBytes(passwordChars, StandardCharsets.UTF_8); - SecretCache.put(SetupKeyActivity.CACHE_KEY_PASSWORD, passwordBytes); - } catch (CharacterCodingException e) { - throw new RuntimeException(e); - } - - secrets.destroy(); - Context context = activity.getApplicationContext(); var intent = new Intent(context, SetupKeyActivity.class) .putExtra(SetupKeyActivity.EXTRA_MODE, KeySetupMode.REGENERATION.toString()); diff --git a/app/src/main/java/app/notesr/activity/security/ImportKeyActivity.java b/app/src/main/java/app/notesr/activity/security/ImportKeyActivity.java index d3dad84c..48273270 100644 --- a/app/src/main/java/app/notesr/activity/security/ImportKeyActivity.java +++ b/app/src/main/java/app/notesr/activity/security/ImportKeyActivity.java @@ -10,9 +10,7 @@ import static java.util.Objects.requireNonNull; import static app.notesr.core.util.ActivityUtils.showToastMessage; -import static app.notesr.core.util.CharUtils.bytesToChars; -import static app.notesr.core.util.CharUtils.charsToBytes; -import static app.notesr.core.util.KeyUtils.getSecretsFromHex; +import static app.notesr.core.util.KeyUtils.getKeyBytesFromKeyHex; import android.os.Bundle; import android.text.Editable; @@ -24,25 +22,21 @@ import androidx.appcompat.app.ActionBar; -import java.nio.charset.CharacterCodingException; -import java.nio.charset.StandardCharsets; import java.util.Arrays; import app.notesr.R; import app.notesr.activity.ActivityBase; import app.notesr.core.security.SecretCache; -import app.notesr.core.security.dto.CryptoSecrets; +import app.notesr.core.util.CryptoSecretsValidator; public final class ImportKeyActivity extends ActivityBase { public static final String CACHE_KEY_HEX_KEY = "hexKey"; - public static final String CACHE_KEY_PASSWORD = "password"; private static final String TAG = ImportKeyActivity.class.getCanonicalName(); private int resultCode = RESULT_CANCELED; private EditText keyField; private char[] hexKey; - private char[] password; @Override protected void onCreate(Bundle savedInstanceState) { @@ -55,8 +49,6 @@ protected void onCreate(Bundle savedInstanceState) { actionBar.setDisplayHomeAsUpEnabled(true); actionBar.setTitle(getResources().getString(R.string.import_key)); - password = getPasswordFromCache(); - keyField = findViewById(R.id.importKeyField); keyField.setImeOptions(IME_FLAG_NO_PERSONALIZED_LEARNING); @@ -72,6 +64,7 @@ protected boolean requiresSession() { @Override public void finish() { wipeUiFields(); + wipeSensitiveClassFields(); setResult(resultCode); super.finish(); } @@ -84,11 +77,12 @@ private View.OnClickListener importKeyButtonOnClick() { hexKeyEditable.getChars(0, hexKeyEditable.length(), hexKey, 0); if (hexKey.length > 0) { + byte[] keyBytes; + try { - CryptoSecrets cryptoSecrets = getCryptoSecrets(hexKey, password); - cryptoSecrets.validate(); - cryptoSecrets.destroy(); - } catch (IllegalArgumentException | IllegalStateException e) { + keyBytes = getKeyBytesFromKeyHex(hexKey); + CryptoSecretsValidator.validateKey(keyBytes); + } catch (IllegalArgumentException e) { Log.e(TAG, "Invalid key", e); showToastMessage(this, getString(R.string.invalid_key), Toast.LENGTH_SHORT); @@ -96,37 +90,17 @@ private View.OnClickListener importKeyButtonOnClick() { return; } - putResultsToCache(hexKey); - + SecretCache.put(CACHE_KEY_HEX_KEY, keyBytes); resultCode = RESULT_OK; finish(); } }; } - - private CryptoSecrets getCryptoSecrets(char[] hexKey, char[] password) { - char[] hexKeyCopy = Arrays.copyOf(hexKey, hexKey.length); - return getSecretsFromHex(hexKeyCopy, password); - } - - private void putResultsToCache(char[] hexKey) { - try { - SecretCache.put(CACHE_KEY_HEX_KEY, charsToBytes(hexKey, - StandardCharsets.UTF_8)); - } catch (CharacterCodingException e) { - throw new RuntimeException(e); - } - SecretCache.removeIfExists(CACHE_KEY_PASSWORD); // Should be already removed - } - private char[] getPasswordFromCache() { - try { - byte[] passwordBytes = requireNonNull(SecretCache.take(CACHE_KEY_PASSWORD), - "Password missing in secret cache"); - return bytesToChars(passwordBytes, StandardCharsets.UTF_8); - } catch (CharacterCodingException e) { - throw new RuntimeException(e); + private void wipeSensitiveClassFields() { + if (hexKey != null) { + Arrays.fill(hexKey, '\0'); } } diff --git a/app/src/main/java/app/notesr/activity/security/KeyRecoveryActivity.java b/app/src/main/java/app/notesr/activity/security/KeyRecoveryActivity.java index 68126d65..08f5eace 100644 --- a/app/src/main/java/app/notesr/activity/security/KeyRecoveryActivity.java +++ b/app/src/main/java/app/notesr/activity/security/KeyRecoveryActivity.java @@ -10,7 +10,7 @@ import static app.notesr.core.util.ActivityUtils.disableBackButton; import static app.notesr.core.util.ActivityUtils.showToastMessage; import static app.notesr.core.util.CharUtils.charsToBytes; -import static app.notesr.core.util.KeyUtils.getKeyBytesFromHex; +import static app.notesr.core.util.KeyUtils.getKeyBytesFromKeyHex; import android.content.Context; import android.content.Intent; @@ -96,7 +96,7 @@ private void apply(EditText hexKeyField, char[] hexKey) throws IOException, NoSuchAlgorithmException { char[] hexKeyCopy = Arrays.copyOf(hexKey, hexKey.length); - byte[] keyBytes = getKeyBytesFromHex(hexKeyCopy); + byte[] keyBytes = getKeyBytesFromKeyHex(hexKeyCopy); Context context = getApplicationContext(); 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 ef5e5e8e..ad14fbcf 100644 --- a/app/src/main/java/app/notesr/activity/security/KeySetupCompletionHandler.java +++ b/app/src/main/java/app/notesr/activity/security/KeySetupCompletionHandler.java @@ -5,6 +5,7 @@ package app.notesr.activity.security; +import static app.notesr.core.util.CharUtils.bytesToChars; import static app.notesr.core.util.CharUtils.charsToBytes; import android.content.Context; @@ -32,7 +33,7 @@ public final class KeySetupCompletionHandler { private final ActivityBase activity; private final AppSecurityService appSecurityService; private final KeySetupMode mode; - private final CryptoSecrets cryptoSecrets; + private final byte[] keyBytes; public void handle() { switch (mode) { @@ -44,7 +45,10 @@ public void handle() { private void proceedFirstRun() { try { - appSecurityService.setSecrets(cryptoSecrets); + char[] password = getCurrentPassword(); + + CryptoSecrets newSecrets = new CryptoSecrets(keyBytes, password); + appSecurityService.setSecrets(newSecrets); Context context = activity.getApplicationContext(); Intent nextIntent = new Intent(context, NotesListActivity.class); @@ -73,19 +77,15 @@ private void proceedRegeneration() { .setTitle(R.string.warning) .setPositiveButton(R.string.yes, (dialog, which) -> onRegenerationConfirmed()) - .setNegativeButton(R.string.no, null) + .setNegativeButton(R.string.no, + (dialog, which) -> onRegenerationCanceled()) .create() .show(); } private void onRegenerationConfirmed() { - byte[] keyBytes = Arrays.copyOf(cryptoSecrets.getKey(), - cryptoSecrets.getKey().length); - - char[] password = Arrays.copyOf(cryptoSecrets.getPassword(), - cryptoSecrets.getPassword().length); - try { + char[] password = getCurrentPassword(); byte[] passwordBytes = charsToBytes(password, StandardCharsets.UTF_8); SecretCache.put(SecretsUpdateAndroidService.NEW_KEY, keyBytes); @@ -94,11 +94,36 @@ private void onRegenerationConfirmed() { throw new RuntimeException(e); } - cryptoSecrets.destroy(); - Intent reEncryptionIntent = new Intent(activity.getApplicationContext(), ReEncryptionActivity.class); + activity.startActivity(reEncryptionIntent); activity.finish(); } + + private void onRegenerationCanceled() { + if (keyBytes != null) { + Arrays.fill(keyBytes, (byte) 0); + } + } + + private char[] getCurrentPassword() throws CharacterCodingException { + if (appSecurityService.isAuthConfigured()) { + CryptoSecrets cryptoSecrets = appSecurityService.getActualSecrets(); + + char[] password = cryptoSecrets.getPassword(); + char[] passwordCopy = Arrays.copyOf(password, password.length); + + cryptoSecrets.destroy(); + return passwordCopy; + } else { + if (SecretCache.contains(SetupKeyActivity.CACHE_KEY_PASSWORD)) { + return bytesToChars(SecretCache.take(SetupKeyActivity.CACHE_KEY_PASSWORD), + StandardCharsets.UTF_8); + } + } + + throw new IllegalStateException("App authentication is not configured" + + " and password is not found in cache"); + } } diff --git a/app/src/main/java/app/notesr/activity/security/SetupKeyActivity.java b/app/src/main/java/app/notesr/activity/security/SetupKeyActivity.java index 3d821d7c..c4be09cd 100644 --- a/app/src/main/java/app/notesr/activity/security/SetupKeyActivity.java +++ b/app/src/main/java/app/notesr/activity/security/SetupKeyActivity.java @@ -9,10 +9,7 @@ import static app.notesr.core.util.ActivityUtils.copyToClipboard; import static app.notesr.core.util.ActivityUtils.showToastMessage; -import static app.notesr.core.util.CharUtils.bytesToChars; -import static app.notesr.core.util.CharUtils.charsToBytes; -import static app.notesr.core.util.KeyUtils.getKeyHexFromSecrets; -import static app.notesr.core.util.KeyUtils.getSecretsFromHex; +import static app.notesr.core.util.KeyUtils.getKeyHexFromKeyBytes; import android.content.Intent; import android.os.Bundle; @@ -30,14 +27,9 @@ import androidx.activity.result.contract.ActivityResultContracts; import androidx.appcompat.app.ActionBar; -import java.nio.charset.CharacterCodingException; -import java.nio.charset.StandardCharsets; -import java.util.Arrays; - import app.notesr.R; import app.notesr.activity.ActivityBase; import app.notesr.core.security.SecretCache; -import app.notesr.core.security.dto.CryptoSecrets; import app.notesr.service.security.AppSecurityService; import lombok.Getter; @@ -50,10 +42,10 @@ public final class SetupKeyActivity extends ActivityBase { private static final float KEY_VIEW_TEXT_SIZE_FOR_LOW_SCREEN_HEIGHT = 16; private KeySetupMode mode; - private char[] password; private ActivityResultLauncher importKeyLauncher; private AppSecurityService appSecurityService; - private CryptoSecrets cryptoSecrets; + + private byte[] newKey; @Override protected void onCreate(Bundle savedInstanceState) { @@ -64,18 +56,15 @@ protected void onCreate(Bundle savedInstanceState) { mode = KeySetupMode.valueOf(requireNonNull(getIntent().getStringExtra(EXTRA_MODE))); ActionBar actionBar = requireNonNull(getSupportActionBar()); - actionBar.setDisplayHomeAsUpEnabled(true); + actionBar.setDisplayHomeAsUpEnabled(mode != KeySetupMode.FIRST_RUN); actionBar.setTitle(R.string.key_setup); - password = getPasswordFromCache(); - importKeyLauncher = registerForActivityResult( new ActivityResultContracts.StartActivityForResult(), getImportKeyCallback()); appSecurityService = getAppSecurityService(); - cryptoSecrets = appSecurityService.getSecretsWithRandomKey(password); - char[] hexKey = getKeyHexFromSecrets(cryptoSecrets); - showHexKey(hexKey); + newKey = appSecurityService.generateMasterKey(); + showKeyHex(newKey); Button copyToClipboardButton = findViewById(R.id.copyAesKeyHex); Button importButton = findViewById(R.id.importHexKeyButton); @@ -96,9 +85,7 @@ protected boolean requiresSession() { @Override public boolean onOptionsItemSelected(MenuItem item) { if (item.getItemId() == android.R.id.home) { - onBackPressedAction(); finish(); - return true; } @@ -108,6 +95,11 @@ public boolean onOptionsItemSelected(MenuItem item) { @Override public void finish() { wipeKeyView(); + + if (mode == KeySetupMode.FIRST_RUN) { + clearCache(); + } + super.finish(); } @@ -115,19 +107,16 @@ private OnBackPressedCallback getOnBackPressedCallback() { return new OnBackPressedCallback(true) { @Override public void handleOnBackPressed() { - onBackPressedAction(); finish(); } }; } - private void onBackPressedAction() { - SecretCache.removeIfExists(CACHE_KEY_PASSWORD); - } + private void showKeyHex(byte[] keyBytes) { + char[] newHexKey = getKeyHexFromKeyBytes(keyBytes); - private void showHexKey(char[] hexKey) { TextView keyView = findViewById(R.id.hexKey); - keyView.setText(hexKey, 0, hexKey.length); + keyView.setText(newHexKey, 0, newHexKey.length); if (getResources().getDisplayMetrics().heightPixels <= LOW_SCREEN_HEIGHT) { keyView.setTextSize(KEY_VIEW_TEXT_SIZE_FOR_LOW_SCREEN_HEIGHT); @@ -145,15 +134,6 @@ private View.OnClickListener copyKeyButtonOnClick() { private View.OnClickListener importKeyButtonOnClick() { return view -> { - char[] passwordCopy = Arrays.copyOf(password, password.length); - - try { - byte[] passwordBytes = charsToBytes(passwordCopy, StandardCharsets.UTF_8); - SecretCache.put(ImportKeyActivity.CACHE_KEY_PASSWORD, passwordBytes); - } catch (CharacterCodingException e) { - throw new RuntimeException(e); - } - Intent intent = new Intent(getApplicationContext(), ImportKeyActivity.class); importKeyLauncher.launch(intent); }; @@ -162,39 +142,27 @@ private View.OnClickListener importKeyButtonOnClick() { private ActivityResultCallback getImportKeyCallback() { return result -> { if (result.getResultCode() == RESULT_OK) { - byte[] hexKeyBytes = SecretCache.take(ImportKeyActivity.CACHE_KEY_HEX_KEY); - - try { - char[] hexKey = bytesToChars(hexKeyBytes, StandardCharsets.UTF_8); - cryptoSecrets = getSecretsFromHex(hexKey, password); - getCompletionHandler(cryptoSecrets).handle(); - } catch (CharacterCodingException e) { - throw new RuntimeException(e); - } + newKey = SecretCache.take(ImportKeyActivity.CACHE_KEY_HEX_KEY); + getCompletionHandler(newKey).handle(); } }; } - private char[] getPasswordFromCache() { - try { - byte[] passwordBytes = requireNonNull(SecretCache.take(CACHE_KEY_PASSWORD), - "Password missing in secret cache"); - return bytesToChars(passwordBytes, StandardCharsets.UTF_8); - } catch (CharacterCodingException e) { - throw new RuntimeException(e); - } - } - private AppSecurityService getAppSecurityService() { return new AppSecurityService(getApplicationContext()); } private View.OnClickListener nextButtonOnClick() { - return view -> getCompletionHandler(cryptoSecrets).handle(); + return view -> getCompletionHandler(newKey).handle(); + } + + private KeySetupCompletionHandler getCompletionHandler(byte[] keyBytes) { + return new KeySetupCompletionHandler(this, appSecurityService, mode, keyBytes); } - private KeySetupCompletionHandler getCompletionHandler(CryptoSecrets cryptoSecrets) { - return new KeySetupCompletionHandler(this, appSecurityService, mode, cryptoSecrets); + private void clearCache() { + SecretCache.removeIfExists(ImportKeyActivity.CACHE_KEY_HEX_KEY); + SecretCache.removeIfExists(SetupKeyActivity.CACHE_KEY_PASSWORD); } private void wipeKeyView() { 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 86bf1df2..862af912 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 @@ -48,43 +48,6 @@ public void destroy() { } } - /** - * Validates the integrity of the cryptographic secrets. - *

- * Ensures that both the key and password arrays are not null or empty, - * and that the key matches the expected 384-bit (48 bytes) length requirement - * and that the password is at least 4 characters long. - * - * @throws IllegalStateException if any validation check fails - */ - public void validate() { - if (key == null || key.length == 0) { - throw new IllegalStateException("CryptoSecrets key cannot be null or empty"); - } - - if (password == null || password.length == 0) { - throw new IllegalStateException("CryptoSecrets password cannot be null or empty"); - } - - if (key.length != MASTER_KEY_SIZE) { - throw new IllegalStateException("Key must be " - + MASTER_KEY_SIZE + " bytes long"); - } - - if (password.length < PASSWORD_MIN_LENGTH) { - throw new IllegalStateException("Password must be at least " - + PASSWORD_MIN_LENGTH + " characters long"); - } - - if (KeyUtils.isKeyNulled(key)) { - throw new IllegalStateException("Key cannot be empty"); - } - - if (!CharUtils.hasNonZeroChars(password)) { - throw new IllegalStateException("Password cannot be empty"); - } - } - /** * Creates a deep copy of the provided {@link CryptoSecrets} instance. * diff --git a/core/src/main/java/app/notesr/core/util/KeyUtils.java b/core/src/main/java/app/notesr/core/util/KeyUtils.java index 21da646e..cd244717 100644 --- a/core/src/main/java/app/notesr/core/util/KeyUtils.java +++ b/core/src/main/java/app/notesr/core/util/KeyUtils.java @@ -28,15 +28,19 @@ public static SecretKey getSecretKeyFromSecrets(CryptoSecrets cryptoSecrets) { return secretKey; } - public static char[] getKeyHexFromSecrets(CryptoSecrets cryptoSecrets) { - return getHexFromKeyBytes(cryptoSecrets.getKey()); - } + public static CryptoSecrets getSecretsFromKeyHexAndPassword(char[] keyHex, char[] password) { + byte[] keyBytes = getKeyBytesFromKeyHex(keyHex); + char[] passwordCopy = Arrays.copyOf(password, password.length); + + var cryptoSecrets = new CryptoSecrets(keyBytes, passwordCopy); + + Arrays.fill(keyHex, '\0'); + Arrays.fill(password, '\0'); - public static CryptoSecrets getSecretsFromHex(char[] hex, char[] password) { - return new CryptoSecrets(getKeyBytesFromHex(hex), password); + return cryptoSecrets; } - public static char[] getHexFromKeyBytes(byte[] key) { + public static char[] getKeyHexFromKeyBytes(byte[] key) { SecureStringBuilder result = new SecureStringBuilder(); int lineLength = 0; @@ -65,16 +69,16 @@ public static char[] getHexFromKeyBytes(byte[] key) { return result.toCharArray(); } - public static byte[] getKeyBytesFromHex(char[] hexChars) { - requireNonNull(hexChars, "hexChars must not be null"); + public static byte[] getKeyBytesFromKeyHex(char[] keyHex) { + requireNonNull(keyHex, "keyHex must not be null"); try { - int tokenCount = countHexTokens(hexChars); + int tokenCount = countHexTokens(keyHex); byte[] key = new byte[tokenCount]; - parseHexTokens(hexChars, key); + parseHexTokens(keyHex, key); - Arrays.fill(hexChars, '\0'); + Arrays.fill(keyHex, '\0'); return key; } catch (Exception e) { throw new IllegalArgumentException("Invalid hex key", e); diff --git a/core/src/test/java/app/notesr/core/util/KeyUtilsTest.java b/core/src/test/java/app/notesr/core/util/KeyUtilsTest.java index 3e98a3b0..f4c9e60a 100644 --- a/core/src/test/java/app/notesr/core/util/KeyUtilsTest.java +++ b/core/src/test/java/app/notesr/core/util/KeyUtilsTest.java @@ -71,14 +71,14 @@ void getKeyHexFromSecretsFormatsCorrectly() { }; char[] expected = "01 23 45 67 \n89 AB CD EF".toCharArray(); - char[] actual = KeyUtils.getHexFromKeyBytes(key); + char[] actual = KeyUtils.getKeyHexFromKeyBytes(key); assertArrayEquals(expected, actual, "Hex conversion should format key with spaces and newlines correctly"); } @Test - void getKeyBytesFromHexParsesCorrectly() { + void getKeyBytesFromKeyHexParsesCorrectly() { char[] hex = "01 23 45 67\n89 AB CD EF".toCharArray(); byte[] expected = new byte[]{ @@ -87,28 +87,28 @@ void getKeyBytesFromHexParsesCorrectly() { (byte) 0xCD, (byte) 0xEF }; - byte[] actual = KeyUtils.getKeyBytesFromHex(hex); + byte[] actual = KeyUtils.getKeyBytesFromKeyHex(hex); assertArrayEquals(expected, actual, "Hex parsing should correctly convert formatted hex string to bytes"); } @Test - void getKeyBytesFromHexHandlesExtraWhitespace() { + void getKeyBytesFromKeyHexHandlesExtraWhitespace() { char[] hex = " 0a 1b \t2c\n3d\r\n4e 5f ".toCharArray(); byte[] expected = new byte[]{0x0a, 0x1b, 0x2c, 0x3d, 0x4e, 0x5f}; - byte[] actual = KeyUtils.getKeyBytesFromHex(hex); + byte[] actual = KeyUtils.getKeyBytesFromKeyHex(hex); assertArrayEquals(expected, actual, "Hex parsing should handle extra whitespace (spaces, tabs, newlines)"); } @Test - void getKeyBytesFromHexThrowsOnInvalidHex() { + void getKeyBytesFromHexThrowsOnInvalidKeyHex() { char[] invalidHex = "zz yy xx".toCharArray(); Exception exception = assertThrows(IllegalArgumentException.class, () -> - KeyUtils.getKeyBytesFromHex(invalidHex), + KeyUtils.getKeyBytesFromKeyHex(invalidHex), "Should throw IllegalArgumentException for invalid hex characters"); assertNotNull(exception.getMessage(), "Exception message should not be null"); @@ -117,13 +117,13 @@ void getKeyBytesFromHexThrowsOnInvalidHex() { } @Test - void getKeyBytesFromHexThrowsOnNullInput() { + void getKeyBytesFromKeyHexThrowsOnNullInput() { Exception exception = assertThrows(NullPointerException.class, () -> - KeyUtils.getKeyBytesFromHex(null), + KeyUtils.getKeyBytesFromKeyHex(null), "Should throw NullPointerException for null hex input"); assertNotNull(exception.getMessage(), "Exception message should not be null"); - assertTrue(exception.getMessage().contains("hexChars must not be null"), + assertTrue(exception.getMessage().contains("keyHex must not be null"), "Exception message should contain 'hexChars must not be null'"); } @@ -131,28 +131,19 @@ void getKeyBytesFromHexThrowsOnNullInput() { void hexConversionRoundTrip() { byte[] originalKey = new byte[]{0x10, 0x20, 0x30, 0x40, 0x50, 0x60}; - char[] hex = KeyUtils.getHexFromKeyBytes(originalKey); - byte[] restoredKey = KeyUtils.getKeyBytesFromHex(hex); + char[] hex = KeyUtils.getKeyHexFromKeyBytes(originalKey); + byte[] restoredKey = KeyUtils.getKeyBytesFromKeyHex(hex); assertArrayEquals(originalKey, restoredKey, "Round-trip hex conversion should restore original key bytes"); } @Test - void getHexFromCryptoSecretsDelegatesCorrectly() { - CryptoSecrets secrets = new CryptoSecrets(new byte[]{0x0A, 0x0B}, "password".toCharArray()); - char[] hex = KeyUtils.getKeyHexFromSecrets(secrets); - - assertArrayEquals("0A 0B".toCharArray(), hex, - "getKeyHexFromSecrets should correctly delegate to getHexFromKeyBytes"); - } - - @Test - void getSecretsFromHexProducesCorrectObject() { + void getSecretsFromKeyHexAndPasswordProducesCorrectObject() { char[] hex = "0C 0D 0E".toCharArray(); String password = "secret"; - CryptoSecrets secrets = KeyUtils.getSecretsFromHex(hex, password.toCharArray()); + CryptoSecrets secrets = KeyUtils.getSecretsFromKeyHexAndPassword(hex, password.toCharArray()); assertArrayEquals(new byte[]{0x0C, 0x0D, 0x0E}, secrets.getKey(), "CryptoSecrets key should match parsed hex"); diff --git a/service/src/androidTest/java/app/notesr/service/exporter/ExportServiceTest.java b/service/src/androidTest/java/app/notesr/service/exporter/ExportServiceTest.java index 0eb43b0f..2df98e98 100644 --- a/service/src/androidTest/java/app/notesr/service/exporter/ExportServiceTest.java +++ b/service/src/androidTest/java/app/notesr/service/exporter/ExportServiceTest.java @@ -56,7 +56,10 @@ public void setUp() { Context context = InstrumentationRegistry.getInstrumentation().getTargetContext(); var appSecurityService = new AppSecurityService(context); - CryptoSecrets cryptoSecrets = appSecurityService.getSecretsWithRandomKey("password".toCharArray()); + byte[] key = appSecurityService.generateMasterKey(); + char[] password = "password".toCharArray(); + + CryptoSecrets cryptoSecrets = new CryptoSecrets(key, password); AesCryptor cryptor = new AesGcmCryptor(getSecretKeyFromSecrets(cryptoSecrets)); FilesUtilsAdapter filesUtils = new FilesUtils(); 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 f124b9c8..049d0ee3 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.util.CryptoSecretsValidator; import app.notesr.data.DatabaseProvider; import lombok.RequiredArgsConstructor; @@ -62,18 +63,16 @@ public AppSecurityService(Context context) { } /** - * Generates new cryptographic secrets which contains randomly generated master key - * and the provided password. - * The key is absolutely random and not derived from the password. + * Generates new 384-bit master key. * - * @param password the password to derive secrets from (will not be modified) - * @return a new {@link CryptoSecrets} object containing derived key material + * @return the generated master key as a byte array + * @see CryptoSecrets#MASTER_KEY_SIZE */ - public CryptoSecrets getSecretsWithRandomKey(char[] password) { + public byte[] generateMasterKey() { byte[] key = new byte[CryptoSecrets.MASTER_KEY_SIZE]; SECURE_RANDOM.nextBytes(key); - return new CryptoSecrets(key, password); + return key; } /** @@ -194,8 +193,8 @@ public void setSecrets(CryptoSecrets newCryptoSecrets) { } try { - newCryptoSecrets.validate(); - } catch (IllegalStateException e) { + CryptoSecretsValidator.validate(newCryptoSecrets); + } catch (IllegalArgumentException e) { newCryptoSecrets.destroy(); throw new IllegalArgumentException("Invalid new secrets", e); } diff --git a/service/src/main/java/app/notesr/service/security/crypto/update/SecretsUpdateService.java b/service/src/main/java/app/notesr/service/security/crypto/update/SecretsUpdateService.java index eb523e63..0446030a 100644 --- a/service/src/main/java/app/notesr/service/security/crypto/update/SecretsUpdateService.java +++ b/service/src/main/java/app/notesr/service/security/crypto/update/SecretsUpdateService.java @@ -18,6 +18,7 @@ import app.notesr.core.security.dto.CryptoSecrets; import app.notesr.core.security.exception.DecryptionFailedException; import app.notesr.core.security.exception.EncryptionFailedException; +import app.notesr.core.util.CryptoSecretsValidator; import app.notesr.core.util.TransactionalFilesUtil; import app.notesr.data.AppDatabase; import app.notesr.data.model.FileBlobInfo; @@ -56,8 +57,8 @@ public void updateSecrets( CryptoSecrets newSecrets) { try { - newSecrets.validate(); - } catch (IllegalStateException e) { + CryptoSecretsValidator.validate(newSecrets); + } catch (IllegalArgumentException e) { throw new SecretsUpdateFailedException("Invalid new secrets", e); } 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 f6fe7f88..4c555942 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.util.CryptoSecretsValidator; import app.notesr.data.DatabaseProvider; @ExtendWith(MockitoExtension.class) @@ -77,25 +78,22 @@ void testConstructorWithContext() { } @Test - void testGetSecretsWithRandomKey() { - char[] password = "testPassword123".toCharArray(); + void testGenerateMasterKey() { - CryptoSecrets secrets = appSecurityService.getSecretsWithRandomKey(password); + byte[] key = appSecurityService.generateMasterKey(); - assertNotNull(secrets); - assertEquals(MASTER_KEY_SIZE, secrets.getKey().length); + assertNotNull(key); + assertEquals(MASTER_KEY_SIZE, key.length); } @Test - void testGetSecretsWithRandomKeyGeneratesDifferentKeys() { - char[] password = "testPassword123".toCharArray(); + void testGenerateMasterKeyGeneratesDifferentKeys() { + byte[] key1 = appSecurityService.generateMasterKey(); + byte[] key2 = appSecurityService.generateMasterKey(); - CryptoSecrets secrets1 = appSecurityService.getSecretsWithRandomKey(password); - CryptoSecrets secrets2 = appSecurityService.getSecretsWithRandomKey(password); - - assertNotNull(secrets1); - assertNotNull(secrets2); - assertNotEquals(secrets1.getKey(), secrets2.getKey()); + assertNotNull(key1); + assertNotNull(key2); + assertNotEquals(key1, key2); } @Test @@ -293,6 +291,9 @@ void testLogoutClosesConnectionsAndClearsCaches() { void testSetSecretsSuccess() throws Exception { CryptoSecrets newSecrets = mock(CryptoSecrets.class); + when(newSecrets.getKey()).thenReturn(generateRandomBytes(MASTER_KEY_SIZE)); + when(newSecrets.getPassword()).thenReturn("validPassword".toCharArray()); + appSecurityService.setSecrets(newSecrets); verify(mockCryptoManager).setSecrets(mockContext, newSecrets); @@ -377,30 +378,44 @@ void testSetSecretsThrowsWhenPasswordIsAllZeros() { @Test void testSetSecretsThrowsAppSecurityExceptionOnEncryptionFailure() throws Exception { - CryptoSecrets newSecrets = mock(CryptoSecrets.class); - doThrow(new GeneralSecurityException()) - .when(mockCryptoManager).setSecrets(mockContext, newSecrets); + MockedStatic mockedValidator = + mockStatic(CryptoSecretsValidator.class); - AppSecurityException exception = assertThrows(AppSecurityException.class, - () -> appSecurityService.setSecrets(newSecrets)); + try (mockedValidator) { + CryptoSecrets newSecrets = mock(CryptoSecrets.class); + doThrow(new GeneralSecurityException()) + .when(mockCryptoManager).setSecrets(mockContext, newSecrets); - assertEquals("Failed to set new secrets, encryption issue", exception.getMessage()); - verify(mockCryptoManager).setSecrets(mockContext, newSecrets); - verify(newSecrets).validate(); - verify(newSecrets).destroy(); + AppSecurityException exception = assertThrows(AppSecurityException.class, + () -> appSecurityService.setSecrets(newSecrets)); + + assertEquals("Failed to set new secrets, encryption issue", + exception.getMessage()); + + verify(mockCryptoManager).setSecrets(mockContext, newSecrets); + mockedValidator.verify(() -> CryptoSecretsValidator.validate(newSecrets)); + verify(newSecrets).destroy(); + } } @Test void testSetSecretsDestroysSecretsEvenOnException() throws Exception { - CryptoSecrets newSecrets = mock(CryptoSecrets.class); - doThrow(new GeneralSecurityException()) - .when(mockCryptoManager).setSecrets(mockContext, newSecrets); + MockedStatic mockedValidator = + mockStatic(CryptoSecretsValidator.class); - assertThrows(AppSecurityException.class, - () -> appSecurityService.setSecrets(newSecrets)); + try (mockedValidator) { + CryptoSecrets newSecrets = mock(CryptoSecrets.class); + doThrow(new GeneralSecurityException()) + .when(mockCryptoManager).setSecrets(mockContext, newSecrets); + + assertThrows(AppSecurityException.class, + () -> appSecurityService.setSecrets(newSecrets)); + + verify(mockCryptoManager).setSecrets(mockContext, newSecrets); + mockedValidator.verify(() -> CryptoSecretsValidator.validate(newSecrets)); + verify(newSecrets).destroy(); + } - verify(newSecrets).validate(); - verify(newSecrets).destroy(); } @Test