Skip to content

Commit 4655b11

Browse files
authored
SSH/server utility cleanup and small code security improvements (#1221)
* Update dartssh2 package * Remove dead code: DecyptPem and tryParseCompletedOutput - Remove unused DecyptPem function (zero call sites) - Remove unused tryParseCompletedOutput static method (only used in tests) - Fix typo: loadIndentity -> loadIdentity - Update persistent_shell_test.dart to drop tests for removed method * Harden security: avoid password in process args, use system temp for keys - sftp_sudo: use shell builtin printf instead of echo to pipe sudo password, so the password does not appear in the process argument list visible via ps - server_func_btns: write temp private key to system temp directory instead of the predictable app doc path, and extend retention to 5s for SSH connection establishment - server_func_btns: unconditionally clear clipboard after password copy timeout instead of skipping when content changed * Reduce redundant store fetches and reuse listEquals - server_dedup: accept optional existingServers param to avoid duplicate ServerStore.fetch() calls in _resolveServers - all.dart: replace manual _isSameOrder with listEquals - server_private_info: replace _sameStringList with listEquals * Improve robustness: fix async void, skip empty workers, narrow failover - chan: change updateHomeWidget from void async to Future<void> with try-catch so callers can use unawaited properly - home: mark updateHomeWidget calls with unawaited - main: skip Computer.shared.turnOn when no servers configured - server: narrow _isJumpFailoverError keyword matching to avoid false positives on unrelated errors containing 'network' or 'socket' * Fix naming inconsistencies and document enum ordering - setting: rename backupasswd -> backupPassword - misc: rename multiBlankreg -> multiBlankReg for consistency - server: add ordering invariant comment for ServerConn operator< * Restore decryptPem and fix typo in caller - Restore decryptPem (was wrongly removed as dead code, actually called in private_key/edit.dart via Computer.shared.start) - Fix typo: decyptPem -> decryptPem in both definition and call site * Preserve user clipboard content when clearing SSH password Revert to checking clipboard content before clearing: only wipe the clipboard if it still holds the password. If the user copied something else during the 25s window, preserve their content. * Revert dartssh2 package * Address review findings: failover match, Computer init, temp key, dedup - server.dart: add 'network is unreachable' to failover error matching - main.dart: restore unconditional Computer.shared.turnOn to avoid hangs when backup/private-key flows call Computer.shared.start with zero servers configured - server_func_btns: use unique temp directory per launch instead of stable filename, and extend key cleanup delay to 30s, delete the whole temp directory on cleanup - ssh.dart: reuse a single Stores.server.fetch() call in _processSSHServers for both deduplicateServers and resolveNameConflicts * Delete temp SSH key dir immediately when launch fails Track whether SSH actually started via sshLaunched flag set after each successful Process.start. On failure paths (exception, unsupported platform, or key prep error) the temp key directory is deleted right away in the finally block instead of waiting 30 seconds.
1 parent 0e7b71b commit 4655b11

17 files changed

Lines changed: 110 additions & 118 deletions

File tree

lib/core/chan.dart

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,14 @@ abstract final class MethodChans {
1818
_channel.invokeMethod('stopService');
1919
}
2020

21-
static void updateHomeWidget() async {
21+
static Future<void> updateHomeWidget() async {
2222
if (!isIOS && !isAndroid) return;
2323
if (!Stores.setting.autoUpdateHomeWidget.fetch()) return;
24-
await _channel.invokeMethod('updateHomeWidget');
24+
try {
25+
await _channel.invokeMethod('updateHomeWidget');
26+
} catch (e, s) {
27+
Loggers.app.warning('Failed to update home widget', e, s);
28+
}
2529
}
2630

2731
/// Update Android foreground service notifications for SSH sessions

lib/core/utils/server.dart

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,17 @@ import 'package:server_box/data/res/store.dart';
1717
/// Because of this function is called by [compute].
1818
///
1919
/// https://stackoverflow.com/questions/51998995/invalid-arguments-illegal-argument-in-isolate-message-object-is-a-closure
20-
List<SSHKeyPair> loadIndentity(String key) {
20+
List<SSHKeyPair> loadIdentity(String key) {
2121
return SSHKeyPair.fromPem(key);
2222
}
2323

24+
/// Decrypts an encrypted PEM private key.
25+
///
26+
/// Must also be a top-level function because it is called via [Computer]
27+
/// (isolate) — see comment on [loadIdentity].
28+
///
2429
/// [args] : [key, pwd]
25-
String decyptPem(List<String> args) {
30+
String decryptPem(List<String> args) {
2631
/// skip when the key is not encrypted, or will throw exception
2732
if (!SSHKeyPair.isEncryptedPem(args[0])) return args[0];
2833
final sshKey = SSHKeyPair.fromPem(args[0], args[1]);
@@ -214,7 +219,7 @@ Future<SSHClient> genClient(
214219
socket,
215220
username: spi.user,
216221
// Must use [compute] here, instead of [Computer.shared.start]
217-
identities: await compute(loadIndentity, privateKey),
222+
identities: await compute(loadIdentity, privateKey),
218223
onUserInfoRequest: onKeyboardInteractive,
219224
onVerifyHostKey: hostKeyVerifier.call,
220225
);
@@ -249,10 +254,11 @@ bool _isJumpFailoverError(Object error) {
249254
errStr.contains('connection reset') ||
250255
errStr.contains('connection closed') ||
251256
errStr.contains('no route to host') ||
252-
errStr.contains('network') ||
253-
errStr.contains('socket') ||
257+
errStr.contains('network unreachable') ||
258+
errStr.contains('network is unreachable') ||
259+
errStr.contains('socketexception') ||
254260
errStr.contains('failed host lookup') ||
255-
errStr.contains('forward') ||
261+
errStr.contains('forwardlocal') ||
256262
errStr.contains('proxycommand exited') ||
257263
errStr.contains('proxycommand timed out');
258264
}

lib/core/utils/server_dedup.dart

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,15 @@ import 'package:server_box/data/store/server.dart';
88
class ServerDeduplication {
99
/// Remove duplicate servers from the import list based on existing servers
1010
/// Returns the deduplicated list
11-
static List<Spi> deduplicateServers(List<Spi> importedServers) {
12-
final existingServers = ServerStore.instance.fetch();
11+
static List<Spi> deduplicateServers(
12+
List<Spi> importedServers, {
13+
List<Spi>? existingServers,
14+
}) {
15+
final existing = existingServers ?? ServerStore.instance.fetch();
1316
final deduplicated = <Spi>[];
1417

1518
for (final imported in importedServers) {
16-
if (!_isDuplicate(imported, existingServers)) {
19+
if (!_isDuplicate(imported, existing)) {
1720
deduplicated.add(imported);
1821
}
1922
}
@@ -33,9 +36,12 @@ class ServerDeduplication {
3336
}
3437

3538
/// Resolve name conflicts by appending suffixes
36-
static List<Spi> resolveNameConflicts(List<Spi> importedServers) {
37-
final existingServers = ServerStore.instance.fetch();
38-
final existingNames = existingServers.map((s) => s.name).toSet();
39+
static List<Spi> resolveNameConflicts(
40+
List<Spi> importedServers, {
41+
List<Spi>? existingServers,
42+
}) {
43+
final existing = existingServers ?? ServerStore.instance.fetch();
44+
final existingNames = existing.map((s) => s.name).toSet();
3945
final processedNames = <String>{};
4046
final result = <Spi>[];
4147

@@ -111,8 +117,12 @@ class ServerDeduplication {
111117
}
112118

113119
static List<Spi> _resolveServers(List<Spi> servers) {
114-
final deduplicated = deduplicateServers(servers);
115-
final resolved = resolveNameConflicts(deduplicated);
120+
final existing = ServerStore.instance.fetch();
121+
final deduplicated = deduplicateServers(servers, existingServers: existing);
122+
final resolved = resolveNameConflicts(
123+
deduplicated,
124+
existingServers: existing,
125+
);
116126
return resolved;
117127
}
118128
}

lib/core/utils/sftp_sudo.dart

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -237,10 +237,13 @@ final class SftpSudoHelper {
237237
}
238238

239239
static String _buildSudoCommand(String command, String password) {
240-
final pwdBase64 = base64Encode(utf8.encode(password));
241240
final wrapped = '($command) 2>&1';
242241
final escapedWrapped = wrapped.replaceAll("'", "'\\''");
243-
return 'echo "$pwdBase64" | base64 -d | sudo -S -- sh -c \'$escapedWrapped\'';
242+
// Use shell builtin printf to pipe password to sudo -S.
243+
// printf is a shell builtin so the password does not appear in
244+
// the process argument list (unlike external `echo`).
245+
final escapedPwd = password.replaceAll("'", "'\\''");
246+
return "printf '%s\\n' '$escapedPwd' | sudo -S -- sh -c '$escapedWrapped'";
244247
}
245248

246249
static SftpFileMode _buildMode(String typeChar, int permOct) {

lib/data/model/container/ps.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ final class DockerPs implements ContainerPs {
149149
/// CONTAINER ID NAMES IMAGE STATUS
150150
/// a049d689e7a1 aria2-pro p3terx/aria2-pro Up 3 weeks
151151
factory DockerPs.parse(String raw) {
152-
final parts = raw.split(Miscs.multiBlankreg);
152+
final parts = raw.split(Miscs.multiBlankReg);
153153
return DockerPs(
154154
id: parts[0],
155155
state: parts[1],

lib/data/model/server/server.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,5 +64,8 @@ enum ServerConn {
6464
/// Status parsing finished
6565
finished;
6666

67+
/// Orders by declaration index: failed < disconnected < connecting <
68+
/// connected < loading < finished. Do NOT reorder the enum values
69+
/// above without auditing all call sites that rely on this ordering.
6770
bool operator <(ServerConn other) => index < other.index;
6871
}

lib/data/model/server/server_private_info.dart

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import 'dart:convert';
22

33
import 'package:fl_lib/fl_lib.dart';
4+
import 'package:flutter/foundation.dart' show listEquals;
45
import 'package:freezed_annotation/freezed_annotation.dart';
56
import 'package:server_box/data/model/app/error.dart';
67
import 'package:server_box/data/model/server/custom.dart';
@@ -148,7 +149,7 @@ extension Spix on Spi {
148149
port == other.port &&
149150
pwd == other.pwd &&
150151
keyId == other.keyId &&
151-
_sameStringList(resolvedJumpIds, other.resolvedJumpIds) &&
152+
listEquals(resolvedJumpIds, other.resolvedJumpIds) &&
152153
proxyCommand == other.proxyCommand;
153154
}
154155

@@ -208,11 +209,3 @@ extension Spix on Spi {
208209
/// Returns true if the user is 'root'.
209210
bool get isRoot => user == 'root';
210211
}
211-
212-
bool _sameStringList(List<String> a, List<String> b) {
213-
if (a.length != b.length) return false;
214-
for (var i = 0; i < a.length; i++) {
215-
if (a[i] != b[i]) return false;
216-
}
217-
return true;
218-
}

lib/data/provider/server/all.dart

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import 'dart:async';
22

33
import 'package:fl_lib/fl_lib.dart';
4+
import 'package:flutter/foundation.dart' show listEquals;
45
import 'package:freezed_annotation/freezed_annotation.dart';
56
import 'package:riverpod_annotation/riverpod_annotation.dart';
67
import 'package:server_box/core/sync.dart';
@@ -335,18 +336,8 @@ class ServersNotifier extends _$ServersNotifier {
335336
}
336337

337338
bool _isSameOrder(List<String> a, List<String> b) {
338-
if (identical(a, b)) {
339-
return true;
340-
}
341-
if (a.length != b.length) {
342-
return false;
343-
}
344-
for (var i = 0; i < a.length; i++) {
345-
if (a[i] != b[i]) {
346-
return false;
347-
}
348-
}
349-
return true;
339+
if (identical(a, b)) return true;
340+
return listEquals(a, b);
350341
}
351342

352343
Future<void> updateServer(Spi old, Spi newSpi) async {

lib/data/res/misc.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import 'dart:convert';
22

33
abstract final class Miscs {
44
static final blankReg = RegExp(r'\s+');
5-
static final multiBlankreg = RegExp(r'\s{2,}');
5+
static final multiBlankReg = RegExp(r'\s{2,}');
66

77
/// RegExp for password request
88
static final pwdRequestWithUserReg = RegExp(r'\[sudo\] password for (.+):');

lib/data/ssh/persistent_shell.dart

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -287,16 +287,6 @@ printf '\\n$_donePrefix$commandId:%s\\n' "\$__server_box_exit"
287287
}
288288
}
289289

290-
static PersistentShellCommandResult? tryParseCompletedOutput(
291-
String raw, {
292-
required String expectedCommandId,
293-
}) {
294-
return _parseCompletedOutput(
295-
raw,
296-
expectedCommandId: expectedCommandId,
297-
)?.result;
298-
}
299-
300290
static _ParsedCompletedOutput? _parseCompletedOutput(
301291
String raw, {
302292
required String expectedCommandId,

0 commit comments

Comments
 (0)