From 32829dac3b1c7b95217be22956873a8e2cc9c148 Mon Sep 17 00:00:00 2001 From: Leumor <116955025+leumor@users.noreply.github.com> Date: Sun, 22 Feb 2026 19:59:58 +0000 Subject: [PATCH 1/2] fix(startup): Fix wizard response flow and first-run log noise Return immediately after wizard redirect to avoid double header sends. Use changeMasterPassword in first-time wizard to avoid AlreadySetPasswordException. Treat missing first-run bookmark/uptime files as normal fallback conditions. Update wizard unit test assertions for redirect-only success path. --- .../http/FirstTimeWizardNewToadlet.java | 42 ++++++++++++------- .../http/bookmark/BookmarkManager.java | 30 ++++++++----- .../network/crypta/node/UptimeEstimator.java | 8 +++- .../http/FirstTimeWizardNewToadletTest.java | 8 ++-- 4 files changed, 56 insertions(+), 32 deletions(-) diff --git a/src/main/java/network/crypta/clients/http/FirstTimeWizardNewToadlet.java b/src/main/java/network/crypta/clients/http/FirstTimeWizardNewToadlet.java index 3e2432a254..877c1b2938 100644 --- a/src/main/java/network/crypta/clients/http/FirstTimeWizardNewToadlet.java +++ b/src/main/java/network/crypta/clients/http/FirstTimeWizardNewToadlet.java @@ -88,8 +88,6 @@ public class FirstTimeWizardNewToadlet extends WebTemplateToadlet { private static final String UNEXPECTED_ERROR_MESSAGE = "Should not happen, please report! {}"; - private boolean isPasswordAlreadySet; - FirstTimeWizardNewToadlet(HighLevelSimpleClient client, NodeClientCore core, Config config) { super(client); this.core = core; @@ -123,12 +121,7 @@ public void handleMethodGET(URI uri, HTTPRequest request, ToadletContext ctx) return; } - // if the threat level is high, the password must already be set: user is running the wizard - // again? - isPasswordAlreadySet = - core.getNode().services().securityLevels().getPhysicalThreatLevel() - == SecurityLevels.PHYSICAL_THREAT_LEVEL.HIGH; - showForm(ctx, new FormModel().toModel()); + showForm(ctx, new FormModel(isPasswordAlreadySet()).toModel()); } /** @@ -158,11 +151,12 @@ public void handleMethodPOST(URI uri, HTTPRequest request, ToadletContext ctx) return; } - FormModel formModel = new FormModel(request); + FormModel formModel = new FormModel(request, isPasswordAlreadySet()); if (formModel.isValid()) { formModel.save(); super.writeTemporaryRedirect(ctx, "Wizard complete", WelcomeToadlet.ROOT_PATH); + return; } // form model not valid @@ -205,7 +199,19 @@ private static String l10n(String key, String value) { return NodeL10n.getBase().getString(L10N_PREFIX + key, value); } + /** + * Returns whether physical security policy currently indicates an already-configured password. + * + *

In the wizard flow, HIGH physical threat implies the user has already completed password + * setup and should not be prompted to set it again. + */ + private boolean isPasswordAlreadySet() { + return core.getNode().services().securityLevels().getPhysicalThreatLevel() + == SecurityLevels.PHYSICAL_THREAT_LEVEL.HIGH; + } + private final class FormModel { + private final boolean passwordAlreadySet; private String knowSomeone = ""; @@ -234,7 +240,8 @@ private final class FormModel { private final Map errors = new HashMap<>(); - FormModel() { + FormModel(boolean passwordAlreadySet) { + this.passwordAlreadySet = passwordAlreadySet; float storage = 100; Option sizeOption = network.crypta.config.Config.longOption(config.get("node"), "storeSize"); @@ -267,7 +274,8 @@ private final class FormModel { } } - FormModel(HTTPRequest request) { + FormModel(HTTPRequest request, boolean passwordAlreadySet) { + this.passwordAlreadySet = passwordAlreadySet; knowSomeone = request.getPartAsStringFailsafe("knowSomeone", 20); connectToStrangers = request.getPartAsStringFailsafe("connectToStrangers", 20); haveMonthlyLimit = request.getPartAsStringFailsafe("haveMonthlyLimit", 20); @@ -432,10 +440,10 @@ private Map toModel() { model.put("minBandwidthMonthlyLimit", "%.2f".formatted(BandwidthLimit.MIN_MONTHLY_LIMIT)); model.put("storageLimit", storageLimit); model.put("minStorageLimit", minStorageLimit); - if (!isPasswordAlreadySet) { + if (!passwordAlreadySet) { model.put("setPassword", !setPassword.isEmpty() ? CHECKED_VALUE : ""); } - model.put("isPasswordAlreadySet", isPasswordAlreadySet); + model.put("isPasswordAlreadySet", passwordAlreadySet); if (downloadLimitDetected == null || uploadLimitDetected == null) { detectBandwidthLimit(); @@ -495,21 +503,23 @@ private void save() { DatastoreSize.setDatastoreSize(storageLimit + "GiB", config); - if (!isPasswordAlreadySet) { + if (!passwordAlreadySet) { try { + String newPassword; if (setPassword.isEmpty()) { // no password protection requested core.getNode() .services() .securityLevels() .setThreatLevel(SecurityLevels.PHYSICAL_THREAT_LEVEL.NORMAL); - core.getNode().storage().setMasterPassword("", true); + newPassword = ""; } else { core.getNode() .services() .securityLevels() .setThreatLevel(SecurityLevels.PHYSICAL_THREAT_LEVEL.HIGH); - core.getNode().storage().setMasterPassword(password, true); + newPassword = password; } + core.getNode().storage().changeMasterPassword("", newPassword, true); } catch (Node.AlreadySetPasswordException | MasterKeysWrongPasswordException | MasterKeysFileSizeException diff --git a/src/main/java/network/crypta/clients/http/bookmark/BookmarkManager.java b/src/main/java/network/crypta/clients/http/bookmark/BookmarkManager.java index 4ed6dd7fa6..415677868f 100644 --- a/src/main/java/network/crypta/clients/http/bookmark/BookmarkManager.java +++ b/src/main/java/network/crypta/clients/http/bookmark/BookmarkManager.java @@ -132,17 +132,27 @@ public BookmarkManager(NodeClientCore n, boolean publicGateway) { this.bookmarksFile = n.getNode().userDir().file("bookmarks.dat"); this.backupBookmarksFile = n.getNode().userDir().file("bookmarks.dat.bak"); - try { - // Read the backup file if necessary - if (!bookmarksFile.exists() || bookmarksFile.length() == 0) throw new IOException(); - LOG.info("Attempting to read the bookmark file from {}", bookmarksFile); - SimpleFieldSet sfs = SimpleFieldSet.readFrom(bookmarksFile, false, true); - readBookmarks(MAIN_CATEGORY, sfs); - } catch (MalformedURLException _) { - // Bookmark file contains a malformed key; ignore and fall back to the backup/defaults. - } catch (IOException ioe) { - LOG.error("Error reading the bookmark file ({}):{}", bookmarksFile, ioe.getMessage(), ioe); + boolean loadedPrimary = false; + if (!bookmarksFile.exists() || bookmarksFile.length() == 0) { + LOG.info( + "Bookmark file {} is missing or empty; loading backup/default bookmarks", bookmarksFile); + } else { + try { + LOG.info("Attempting to read the bookmark file from {}", bookmarksFile); + SimpleFieldSet sfs = SimpleFieldSet.readFrom(bookmarksFile, false, true); + readBookmarks(MAIN_CATEGORY, sfs); + loadedPrimary = true; + } catch (MalformedURLException e) { + LOG.warn( + "Bookmark file {} contains malformed keys; falling back to backup/default bookmarks", + bookmarksFile, + e); + } catch (IOException ioe) { + LOG.error("Error reading the bookmark file ({}):{}", bookmarksFile, ioe.getMessage(), ioe); + } + } + if (!loadedPrimary) { try { if (backupBookmarksFile.exists() && backupBookmarksFile.canRead() diff --git a/src/main/java/network/crypta/node/UptimeEstimator.java b/src/main/java/network/crypta/node/UptimeEstimator.java index fb6c7cd7d1..6f9db6cf3c 100644 --- a/src/main/java/network/crypta/node/UptimeEstimator.java +++ b/src/main/java/network/crypta/node/UptimeEstimator.java @@ -106,6 +106,10 @@ public void start() { } private void readData(File file, int base) { + if (!file.exists()) { + LOG.debug("Uptime history file not found at startup: {}", file); + return; + } try (FileInputStream fis = new FileInputStream(file); DataInputStream dis = new DataInputStream(fis)) { while (true) { @@ -127,7 +131,9 @@ private void readData(File file, int base) { } } } catch (EOFException _) { - // Reached end of file; no more samples to load. + // Reached the end of the file; no more samples to load. + } catch (FileNotFoundException _) { + LOG.debug("Uptime history file disappeared before it could be read: {}", file); } catch (IOException _) { LOG.error("Read old uptime file failed: {}; treating slots as offline", file); } diff --git a/src/test/java/network/crypta/clients/http/FirstTimeWizardNewToadletTest.java b/src/test/java/network/crypta/clients/http/FirstTimeWizardNewToadletTest.java index da1671885a..e9a1d16391 100644 --- a/src/test/java/network/crypta/clients/http/FirstTimeWizardNewToadletTest.java +++ b/src/test/java/network/crypta/clients/http/FirstTimeWizardNewToadletTest.java @@ -232,7 +232,7 @@ void handleMethodPOST_whenValidInput_savesConfigAndRedirects() throws Exception verify(fproxySubConfig).set("hasCompletedWizard", true); verify(securityLevels).setThreatLevel(SecurityLevels.NETWORK_THREAT_LEVEL.NORMAL); verify(securityLevels).setThreatLevel(SecurityLevels.PHYSICAL_THREAT_LEVEL.NORMAL); - verify(storage).setMasterPassword("", true); + verify(storage).changeMasterPassword("", "", true); verify(core).storeConfig(); verify(ctx) .sendReplyHeaders( @@ -241,10 +241,8 @@ void handleMethodPOST_whenValidInput_savesConfigAndRedirects() throws Exception ArgumentMatchers.any(), eq("text/html; charset=UTF-8"), anyLong()); - assertTrue(toadlet.htmlWritten); - @SuppressWarnings("unchecked") - Map errors = (Map) toadlet.lastModel.get("errors"); - assertTrue(errors.isEmpty()); + assertFalse(toadlet.htmlWritten); + assertNull(toadlet.lastModel); } @Test From 07a111e52be3d660c2f1e067864ddb5c4c7fb024 Mon Sep 17 00:00:00 2001 From: Leumor <116955025+leumor@users.noreply.github.com> Date: Sun, 22 Feb 2026 20:13:48 +0000 Subject: [PATCH 2/2] fix(storage): Avoid NPE when changing password without loaded keys Load master keys from disk with oldPassword when master.keys exists but in-memory keys are null. Preserve existing password-change flow and add regression test for the null-keys path. --- .../node/subsystem/NodeStorageSubsystem.java | 18 +++++++- .../subsystem/NodeStorageSubsystemTest.java | 43 +++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/src/main/java/network/crypta/node/subsystem/NodeStorageSubsystem.java b/src/main/java/network/crypta/node/subsystem/NodeStorageSubsystem.java index 3ee504f7e7..9b62c1a651 100644 --- a/src/main/java/network/crypta/node/subsystem/NodeStorageSubsystem.java +++ b/src/main/java/network/crypta/node/subsystem/NodeStorageSubsystem.java @@ -3,6 +3,7 @@ import java.io.File; import java.io.IOException; import java.nio.file.Path; +import java.security.SecureRandom; import java.util.concurrent.atomic.AtomicReference; import network.crypta.clients.http.PasswordFormOptions; import network.crypta.config.InvalidConfigValueException; @@ -2169,9 +2170,22 @@ public void changeMasterPassword( } if (node.services().securityLevels().getPhysicalThreatLevel() == PHYSICAL_THREAT_LEVEL.MAXIMUM) LOG.error("Changing password while physical threat level is at MAXIMUM???"); + SecureRandom secureRandom = node.bootstrap().secureRandom(); if (masterKeysFile.exists()) { - keys.changePassword(masterKeysFile, newPassword, node.bootstrap().secureRandom()); - setPasswordInner(keys, inFirstTimeWizard); + MasterKeys activeKeys = keys; + if (activeKeys == null) { + activeKeys = MasterKeys.read(masterKeysFile, secureRandom, oldPassword); + synchronized (node) { + if (keys == null) { + keys = activeKeys; + databaseKey = activeKeys.createDatabaseKey(); + } else { + activeKeys = keys; + } + } + } + activeKeys.changePassword(masterKeysFile, newPassword, secureRandom); + setPasswordInner(activeKeys, inFirstTimeWizard); } else { setMasterPassword(newPassword, inFirstTimeWizard); } diff --git a/src/test/java/network/crypta/node/subsystem/NodeStorageSubsystemTest.java b/src/test/java/network/crypta/node/subsystem/NodeStorageSubsystemTest.java index 63a0a310d0..bccfd3f7be 100644 --- a/src/test/java/network/crypta/node/subsystem/NodeStorageSubsystemTest.java +++ b/src/test/java/network/crypta/node/subsystem/NodeStorageSubsystemTest.java @@ -2,13 +2,17 @@ import java.io.File; import java.lang.reflect.Field; +import java.security.SecureRandom; import network.crypta.config.InvalidConfigValueException; import network.crypta.config.NodeNeedRestartException; import network.crypta.keys.CHKBlock; +import network.crypta.node.DatabaseKey; +import network.crypta.node.MasterKeys; import network.crypta.node.Node; import network.crypta.node.NodeClientCore; import network.crypta.node.NodeInitException; import network.crypta.node.ProgramDirectory; +import network.crypta.node.SecurityLevels; import network.crypta.node.useralerts.UserAlert; import network.crypta.node.useralerts.UserAlertManager; import network.crypta.store.CHKStore; @@ -20,6 +24,7 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; import org.mockito.Mock; +import org.mockito.MockedStatic; import org.mockito.junit.jupiter.MockitoExtension; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; @@ -29,6 +34,7 @@ import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -237,6 +243,43 @@ void setStorePreallocate_whenStoreIsWrapped_appliesToUnderlyingSaltedHashStore() verify(saltedHashStore).setPreallocate(true); } + @Test + void changeMasterPassword_whenKeysUnavailableAndMasterKeysFileExists_loadsKeysAndChangesPassword() + throws Exception { + File masterKeysFile = File.createTempFile("master-keys", ".tmp"); + masterKeysFile.deleteOnExit(); + subsystem.setMasterKeysFile(masterKeysFile); + + MasterKeys loadedKeys = org.mockito.Mockito.mock(MasterKeys.class); + DatabaseKey loadedDatabaseKey = org.mockito.Mockito.mock(DatabaseKey.class); + SecurityLevels securityLevels = org.mockito.Mockito.mock(SecurityLevels.class); + NodeBootstrap bootstrap = org.mockito.Mockito.mock(NodeBootstrap.class); + SecureRandom secureRandom = new SecureRandom(); + + when(node.services()).thenReturn(services); + when(services.securityLevels()).thenReturn(securityLevels); + when(services.clientCore()).thenReturn(clientCore); + when(securityLevels.getPhysicalThreatLevel()) + .thenReturn(SecurityLevels.PHYSICAL_THREAT_LEVEL.NORMAL); + when(node.bootstrap()).thenReturn(bootstrap); + when(bootstrap.secureRandom()).thenReturn(secureRandom); + when(loadedKeys.createDatabaseKey()).thenReturn(loadedDatabaseKey); + + try (MockedStatic masterKeysStatic = mockStatic(MasterKeys.class)) { + masterKeysStatic + .when(() -> MasterKeys.read(masterKeysFile, secureRandom, "old-password")) + .thenReturn(loadedKeys); + + subsystem.changeMasterPassword("old-password", "new-password", true); + + masterKeysStatic.verify(() -> MasterKeys.read(masterKeysFile, secureRandom, "old-password")); + } + + verify(loadedKeys).changePassword(masterKeysFile, "new-password", secureRandom); + assertSame(loadedKeys, subsystem.getKeys()); + assertSame(loadedDatabaseKey, subsystem.getDatabaseKey()); + } + private static void setChkDatastore(NodeStorageSubsystem target, CHKStore value) throws ReflectiveOperationException { Field field = target.getClass().getDeclaredField("chkDatastore");