From 6163de518014112a4a5bb64359c6eafa30e6652c Mon Sep 17 00:00:00 2001 From: xavierchanth Date: Wed, 4 Oct 2023 18:50:48 -0400 Subject: [PATCH] chore: use new guarantees --- .../lib/src/common/file_system_utils.dart | 44 +++++++++++++++---- .../sshnp_impl/sshnp_forward_exec_impl.dart | 21 +++++---- .../lib/src/sshnp/sshnp_impl/sshnp_impl.dart | 9 ++-- .../sshnp/sshnp_impl/sshnp_legacy_impl.dart | 1 - .../sshnp_impl/sshnp_local_file_mixin.dart | 26 ++++++++++- .../sshnp/sshnp_impl/sshnp_reverse_impl.dart | 1 - .../sshnp/sshnp_impl/sshnp_reverse_mixin.dart | 22 +++------- .../lib/src/sshnpd/sshnpd_impl.dart | 2 +- packages/sshnoports/bin/sshnp.dart | 9 ---- 9 files changed, 84 insertions(+), 51 deletions(-) diff --git a/packages/noports_core/lib/src/common/file_system_utils.dart b/packages/noports_core/lib/src/common/file_system_utils.dart index aa80c8510..f533ecfc7 100644 --- a/packages/noports_core/lib/src/common/file_system_utils.dart +++ b/packages/noports_core/lib/src/common/file_system_utils.dart @@ -1,3 +1,4 @@ +import 'dart:async'; import 'dart:io'; import 'package:at_utils/at_utils.dart'; import 'package:path/path.dart' as path; @@ -60,15 +61,28 @@ String getDefaultSshnpConfigDirectory(String homeDirectory) { return path.normalize('$homeDirectory/.sshnp/config'); } -Future<(String, String)> generateSshKeys( - {required bool rsa, - required String sessionId, - String? sshHomeDirectory}) async { +(String, String, String) _getEphemeralKeysPath( + String? sshHomeDirectory, String sessionId) { sshHomeDirectory ??= getDefaultSshDirectory(getHomeDirectory()!); if (!Directory(sshHomeDirectory).existsSync()) { Directory(sshHomeDirectory).createSync(); } + return ( + sshHomeDirectory, + '$sshHomeDirectory/${sessionId}_sshnp.pub', + '$sshHomeDirectory/${sessionId}_sshnp' + ); +} + +Future<(String, String)> generateEphemeralSshKeys( + {required bool rsa, + required String sessionId, + String? sshHomeDirectory}) async { + var (normalizedSshHomeDirectory, sshPublicKeyPath, sshPrivateKeyPath) = + _getEphemeralKeysPath(sshHomeDirectory, sessionId); + sshHomeDirectory = normalizedSshHomeDirectory; + if (rsa) { await Process.run('ssh-keygen', ['-t', 'rsa', '-b', '4096', '-f', '${sessionId}_sshnp', '-q', '-N', ''], @@ -90,12 +104,24 @@ Future<(String, String)> generateSshKeys( workingDirectory: sshHomeDirectory); } - String sshPublicKey = - await File('$sshHomeDirectory/${sessionId}_sshnp.pub').readAsString(); - String sshPrivateKey = - await File('$sshHomeDirectory/${sessionId}_sshnp').readAsString(); + var keys = await Future.wait([ + File(sshPublicKeyPath).readAsString(), + File(sshPrivateKeyPath).readAsString() + ]); + + return (keys[0], keys[1]); +} - return (sshPublicKey, sshPrivateKey); +Future cleanUpEphemeralSshKeys({ + required String sessionId, + String? sshHomeDirectory, +}) async { + var (_, sshPublicKeyPath, sshPrivateKeyPath) = + _getEphemeralKeysPath(sshHomeDirectory, sessionId); + await Future.wait([ + File(sshPublicKeyPath).delete(), + File(sshPrivateKeyPath).delete(), + ]); } Future addEphemeralKeyToAuthorizedKeys( diff --git a/packages/noports_core/lib/src/sshnp/sshnp_impl/sshnp_forward_exec_impl.dart b/packages/noports_core/lib/src/sshnp/sshnp_impl/sshnp_forward_exec_impl.dart index 3373a10c4..c1d7a71d6 100644 --- a/packages/noports_core/lib/src/sshnp/sshnp_impl/sshnp_forward_exec_impl.dart +++ b/packages/noports_core/lib/src/sshnp/sshnp_impl/sshnp_forward_exec_impl.dart @@ -12,6 +12,7 @@ import 'package:path/path.dart' as path; class SSHNPForwardExecImpl extends SSHNPImpl with SSHNPForwardDirection, SSHNPLocalFileMixin { + late String ephemeralPrivateKeyPath; SSHNPForwardExecImpl({ required AtClient atClient, required SSHNPParams params, @@ -45,21 +46,17 @@ class SSHNPForwardExecImpl extends SSHNPImpl String? errorMessage; Process? process; - // If using exec then we can assume we're on something unix-y - // So we can write the ephemeralPrivateKey to a tmp file, - // set its permissions appropriately, and remove it after we've - // executed the command - var tmpFileName = - path.normalize('$sshHomeDirectory/tmp/ephemeral_$sessionId'); - File tmpFile = File(tmpFileName); + var ephemeralPrivateKeyPath = path.normalize( + '$sshnpHomeDirectory/sessions/$sessionId/ephemeral_private_key'); + File tmpFile = File(ephemeralPrivateKeyPath); await tmpFile.create(recursive: true); await tmpFile.writeAsString(ephemeralPrivateKey, mode: FileMode.write, flush: true); - await Process.run('chmod', ['go-rwx', tmpFileName]); + await Process.run('chmod', ['go-rwx', ephemeralPrivateKeyPath]); String argsString = '$remoteUsername@$host' ' -p $sshrvdPort' - ' -i $tmpFileName' + ' -i $ephemeralPrivateKeyPath' ' -L $localPort:localhost:${params.remoteSshdPort}' ' -o LogLevel=VERBOSE' ' -t -t' @@ -138,4 +135,10 @@ class SSHNPForwardExecImpl extends SSHNPImpl ); } } + + @override + Future cleanUp() async { + await deleteFile(ephemeralPrivateKeyPath); + super.cleanUp(); + } } diff --git a/packages/noports_core/lib/src/sshnp/sshnp_impl/sshnp_impl.dart b/packages/noports_core/lib/src/sshnp/sshnp_impl/sshnp_impl.dart index fa26e2d74..e715474dc 100644 --- a/packages/noports_core/lib/src/sshnp/sshnp_impl/sshnp_impl.dart +++ b/packages/noports_core/lib/src/sshnp/sshnp_impl/sshnp_impl.dart @@ -151,6 +151,12 @@ abstract class SSHNPImpl implements SSHNP { _initializeStarted = true; } + // Schedule a cleanup on exit + unawaited(doneCompleter.future.then((_) async { + logger.info('SSHNPImpl done'); + await cleanUp(); + })); + try { if (!(await atSignIsActivated(atClient, sshnpdAtSign))) { logger.severe('Device address $sshnpdAtSign is not activated.'); @@ -297,7 +303,6 @@ abstract class SSHNPImpl implements SSHNP { try { return (await atClient.get(userNameRecordID)).value as String; } catch (e, s) { - await cleanUp(); throw SSHNPError( "Device unknown, or username not shared\n" "hint: make sure the device shares username or set remote username manually", @@ -344,7 +349,6 @@ abstract class SSHNPImpl implements SSHNP { counter++; if (counter == 100) { logger.warning('Timed out waiting for sshrvd response'); - await cleanUp(); throw ('Connection timeout to sshrvd $host service\nhint: make sure host is valid and online'); } } @@ -379,7 +383,6 @@ abstract class SSHNPImpl implements SSHNP { ..ttl = 10000); await notify(sendOurPublicKeyToSshnpd, toSshPublicKey); } catch (e, s) { - await cleanUp(); throw SSHNPError( 'Error opening or validating public key file or sending to remote atSign', error: e, diff --git a/packages/noports_core/lib/src/sshnp/sshnp_impl/sshnp_legacy_impl.dart b/packages/noports_core/lib/src/sshnp/sshnp_impl/sshnp_legacy_impl.dart index 13e251aa6..3c8f6548d 100644 --- a/packages/noports_core/lib/src/sshnp/sshnp_impl/sshnp_legacy_impl.dart +++ b/packages/noports_core/lib/src/sshnp/sshnp_impl/sshnp_legacy_impl.dart @@ -70,7 +70,6 @@ class SSHNPLegacyImpl extends SSHNPImpl ); bool acked = await waitForDaemonResponse(); - await cleanUp(); if (!acked) { var error = SSHNPError( 'sshnp timed out: waiting for daemon response\nhint: make sure the device is online', diff --git a/packages/noports_core/lib/src/sshnp/sshnp_impl/sshnp_local_file_mixin.dart b/packages/noports_core/lib/src/sshnp/sshnp_impl/sshnp_local_file_mixin.dart index 4d934c9df..03af24465 100644 --- a/packages/noports_core/lib/src/sshnp/sshnp_impl/sshnp_local_file_mixin.dart +++ b/packages/noports_core/lib/src/sshnp/sshnp_impl/sshnp_local_file_mixin.dart @@ -1,10 +1,14 @@ import 'dart:io'; +import 'package:meta/meta.dart'; +import 'package:noports_core/src/common/file_system_utils.dart'; import 'package:noports_core/src/sshnp/sshnp_impl/sshnp_impl.dart'; import 'package:noports_core/src/sshnp/sshnp_result.dart'; mixin SSHNPLocalFileMixin on SSHNPImpl { + late final String homeDirectory; late final String sshHomeDirectory; + late final String sshnpHomeDirectory; final bool _isValidPlatform = Platform.isLinux || Platform.isMacOS || Platform.isWindows; @@ -19,8 +23,28 @@ mixin SSHNPLocalFileMixin on SSHNPImpl { throw SSHNPError( 'The current platform is not supported: ${Platform.operatingSystem}'); } + try { + homeDirectory = getHomeDirectory(throwIfNull: true)!; + } catch (e, s) { + throw SSHNPError('Unable to determine the home directory', + error: e, stackTrace: s); + } + sshHomeDirectory = getDefaultSshDirectory(homeDirectory); + sshnpHomeDirectory = getDefaultSshnpDirectory(homeDirectory); + await super.init(); - if (initializedCompleter.isCompleted) return; } + + @protected + Future deleteFile(String fileName) async { + try { + final file = File(fileName); + await file.delete(); + return true; + } catch (e) { + logger.severe("Error deleting file : $fileName"); + return false; + } + } } diff --git a/packages/noports_core/lib/src/sshnp/sshnp_impl/sshnp_reverse_impl.dart b/packages/noports_core/lib/src/sshnp/sshnp_impl/sshnp_reverse_impl.dart index 8cff3e1e7..298faa385 100644 --- a/packages/noports_core/lib/src/sshnp/sshnp_impl/sshnp_reverse_impl.dart +++ b/packages/noports_core/lib/src/sshnp/sshnp_impl/sshnp_reverse_impl.dart @@ -66,7 +66,6 @@ class SSHNPReverseImpl extends SSHNPImpl sessionId: sessionId); bool acked = await waitForDaemonResponse(); - await cleanUp(); if (!acked) { var error = SSHNPError('sshnp connection timeout: waiting for daemon response'); diff --git a/packages/noports_core/lib/src/sshnp/sshnp_impl/sshnp_reverse_mixin.dart b/packages/noports_core/lib/src/sshnp/sshnp_impl/sshnp_reverse_mixin.dart index fb188caef..e2a483a43 100644 --- a/packages/noports_core/lib/src/sshnp/sshnp_impl/sshnp_reverse_mixin.dart +++ b/packages/noports_core/lib/src/sshnp/sshnp_impl/sshnp_reverse_mixin.dart @@ -1,5 +1,4 @@ import 'dart:async'; -import 'dart:io'; import 'package:noports_core/src/common/file_system_utils.dart'; import 'package:noports_core/src/sshnp/sshnp_impl/sshnp_local_file_mixin.dart'; @@ -10,13 +9,13 @@ import 'package:noports_core/utils.dart'; /// e.g. class [SSHNPReverseImpl] extends [SSHNPImpl] with [SSHNPLocalFileMixin], [SSHNPReverseMixin] /// Note that the order of mixins is important here. mixin SSHNPReverseMixin on SSHNPLocalFileMixin { - /// Set by [generateSshKeys] during [init], if we're not doing direct ssh. + /// Set by [generateEphemeralSshKeys] during [init], if we're not doing direct ssh. /// sshnp generates a new keypair for each ssh session, using ed25519 by /// default but rsa if the [rsa] flag is set to true. sshnp will write /// [sshPublicKey] to ~/.ssh/authorized_keys late final String sshPublicKey; - /// Set by [generateSshKeys] during [init]. + /// Set by [generateEphemeralSshKeys] during [init]. /// sshnp generates a new keypair for each ssh session, using ed25519 by /// default but rsa if the [rsa] flag is set to true. sshnp will send the /// [sshPrivateKey] to sshnpd @@ -35,7 +34,7 @@ mixin SSHNPReverseMixin on SSHNPLocalFileMixin { logger.info('Generating ephemeral keypair'); try { var (String ephemeralPublicKey, String ephemeralPrivateKey) = - await generateSshKeys( + await generateEphemeralSshKeys( rsa: params.rsa, sessionId: sessionId, sshHomeDirectory: sshHomeDirectory, @@ -72,23 +71,12 @@ mixin SSHNPReverseMixin on SSHNPLocalFileMixin { var sshHomeDirectory = getDefaultSshDirectory(homeDirectory); logger.info('Tidying up files'); // Delete the generated RSA keys and remove the entry from ~/.ssh/authorized_keys - await _deleteFile('$sshHomeDirectory/${sessionId}_sshnp'); - await _deleteFile('$sshHomeDirectory/${sessionId}_sshnp.pub'); + await cleanUpEphemeralSshKeys( + sessionId: sessionId, sshHomeDirectory: sshHomeDirectory); await removeEphemeralKeyFromAuthorizedKeys(sessionId, logger, sshHomeDirectory: sshHomeDirectory); super.cleanUp(); } - Future _deleteFile(String fileName) async { - try { - final file = File(fileName); - await file.delete(); - return true; - } catch (e) { - logger.severe("Error deleting file : $fileName"); - return false; - } - } - bool get usingSshrv => sshrvdPort != null; } diff --git a/packages/noports_core/lib/src/sshnpd/sshnpd_impl.dart b/packages/noports_core/lib/src/sshnpd/sshnpd_impl.dart index 0214de670..a4496f43f 100644 --- a/packages/noports_core/lib/src/sshnpd/sshnpd_impl.dart +++ b/packages/noports_core/lib/src/sshnpd/sshnpd_impl.dart @@ -509,7 +509,7 @@ class SSHNPDImpl implements SSHNPD { /// `authorized_keys` file, limiting permissions (e.g. hosts and ports /// which can be forwarded to) as per the `--ephemeral-permissions` option var (String ephemeralPublicKey, String ephemeralPrivateKey) = - await generateSshKeys(rsa: rsa, sessionId: sessionId); + await generateEphemeralSshKeys(rsa: rsa, sessionId: sessionId); await addEphemeralKeyToAuthorizedKeys( sshPublicKey: ephemeralPublicKey, diff --git a/packages/sshnoports/bin/sshnp.dart b/packages/sshnoports/bin/sshnp.dart index 35cb780cb..03ccb7203 100644 --- a/packages/sshnoports/bin/sshnp.dart +++ b/packages/sshnoports/bin/sshnp.dart @@ -33,11 +33,6 @@ void main(List args) async { exit(0); } - ProcessSignal.sigint.watch().listen((signal) async { - await sshnp?.cleanUp(); - exit(1); - }); - await runZonedGuarded(() async { try { params = SSHNPParams.fromPartial(SSHNPPartialParams.fromArgList(args)); @@ -98,8 +93,6 @@ void main(List args) async { if (verbose) { stderr.writeln('\nStack Trace: ${stackTrace.toString()}'); } - - await sshnp?.cleanUp(); exit(1); } }, (Object error, StackTrace stackTrace) async { @@ -109,8 +102,6 @@ void main(List args) async { if (verbose) { stderr.writeln('\nStack Trace: ${stackTrace.toString()}'); } - - await sshnp?.cleanUp(); exit(1); }); }