Merge pull request #1090 from atsign-foundation/no-double-ssh-when-so…

…ckets-encrypted

packages/dart/noports_core/lib/src/sshnp/impl/sshnp_dart_pure_impl.dart

@@ -47,6 +47,7 @@ class SshnpDartPureImpl extends SshnpCore
   }
 
   SSHClient? tunnelSshClient;
+  SSHSocket? _sshSocketForUserSession;
 
   @override
   Future<void> dispose() async {
@@ -97,45 +98,51 @@ class SshnpDartPureImpl extends SshnpCore
     } else {
       sendProgress('Received response from the device daemon');
     }
-
-    if (sshnpdChannel.ephemeralPrivateKey == null) {
+    if (sshnpdChannel.ephemeralPrivateKey == null &&
+        !params.encryptRvdTraffic) {
       throw SshnpError(
         'Expected an ephemeral private key from device daemon, but it was not set',
       );
     }
 
-    /// Load the ephemeral private key into a key pair
-    AtSshKeyPair ephemeralKeyPair = AtSshKeyPair.fromPem(
-      sshnpdChannel.ephemeralPrivateKey!,
-      identifier: 'ephemeral_$sessionId',
-    );
-
-    /// Add the key pair to the key utility
-    await keyUtil.addKeyPair(keyPair: ephemeralKeyPair);
-
     /// Start srv
     sendProgress('Creating connection to socket rendezvous');
-    SSHSocket? sshSocket = await srvdChannel.runSrv(
+    SSHSocket? temp = await srvdChannel.runSrv(
       sessionAESKeyString: sshnpdChannel.sessionAESKeyString,
       sessionIVString: sshnpdChannel.sessionIVString,
       multi: false,
       detached: false,
     );
 
-    try {
-      /// Start the initial tunnel
-      sendProgress('Starting tunnel session');
-      tunnelSshClient = await startInitialTunnelSession(
-        ephemeralKeyPairIdentifier: ephemeralKeyPair.identifier,
-        sshSocket: sshSocket,
+    // If we're not encrypting traffic on the sockets, then we create an extra
+    // ssh session in order to encrypt the user's "real" ssh session.
+    if (params.encryptRvdTraffic == false) {
+      /// Load the ephemeral private key into a key pair
+      AtSshKeyPair ephemeralKeyPair = AtSshKeyPair.fromPem(
+        sshnpdChannel.ephemeralPrivateKey!,
+        identifier: 'ephemeral_$sessionId',
       );
-    } finally {
-      /// Remove the key pair from the key utility
+
+      /// Add the key pair to the key utility
+      await keyUtil.addKeyPair(keyPair: ephemeralKeyPair);
+
       try {
-        await keyUtil.deleteKeyPair(identifier: ephemeralKeyPair.identifier);
-      } catch (e) {
-        logger.shout('Failed to delete ephemeral keyPair: $e');
+        /// Start the initial tunnel
+        sendProgress('Starting tunnel session');
+        tunnelSshClient = await startInitialTunnelSession(
+          ephemeralKeyPairIdentifier: ephemeralKeyPair.identifier,
+          sshSocket: temp,
+        );
+      } finally {
+        /// Remove the key pair from the key utility
+        try {
+          await keyUtil.deleteKeyPair(identifier: ephemeralKeyPair.identifier);
+        } catch (e) {
+          logger.shout('Failed to delete ephemeral keyPair: $e');
+        }
       }
+    } else {
+      _sshSocketForUserSession = temp;
     }
 
     /// Ensure that we clean up after ourselves
@@ -158,14 +165,9 @@ class SshnpDartPureImpl extends SshnpCore
 
   @override
   Future<SshnpRemoteProcess> runShell() async {
-    if (tunnelSshClient == null) {
-      throw StateError(
-          'Cannot execute runShell, tunnel has not yet been created');
-    }
-
     sendProgress('Starting user session');
     SSHClient userSession =
-        await startUserSession(tunnelSession: tunnelSshClient!);
+        await startUserSession(sshSocket: _sshSocketForUserSession);
 
     sendProgress('Starting remote shell');
     SSHSession shell = await userSession.shell();

packages/dart/noports_core/lib/src/sshnp/impl/sshnp_openssh_local_impl.dart

@@ -90,16 +90,25 @@ class SshnpOpensshLocalImpl extends SshnpCore
       sendProgress('Received response from the device daemon');
     }
 
-    if (sshnpdChannel.ephemeralPrivateKey == null) {
+    if (sshnpdChannel.ephemeralPrivateKey == null &&
+        !params.encryptRvdTraffic) {
       throw SshnpError(
         'Expected an ephemeral private key from sshnpd, but it was not set',
       );
     }
 
-    /// Find a port to use
-    final server = await ServerSocket.bind(InternetAddress.anyIPv4, 0);
-    int localRvPort = server.port;
-    await server.close();
+    final int localRvPort;
+    // If we're not encrypting traffic on the sockets, then we create an extra
+    // ssh session in order to encrypt the user's "real" ssh session. And we
+    // need to grab an unused local port.
+    if (params.encryptRvdTraffic == false) {
+      /// Find a port to use for the tunnel ssh session
+      final server = await ServerSocket.bind(InternetAddress.anyIPv4, 0);
+      localRvPort = server.port;
+      await server.close();
+    } else {
+      localRvPort = localPort;
+    }
 
     /// Start srv
     sendProgress('Creating connection to socket rendezvous');
@@ -111,30 +120,35 @@ class SshnpOpensshLocalImpl extends SshnpCore
       detached: true,
     );
 
-    /// Load the ephemeral private key into a key pair
-    AtSshKeyPair ephemeralKeyPair = AtSshKeyPair.fromPem(
-      sshnpdChannel.ephemeralPrivateKey!,
-      identifier: 'ephemeral_$sessionId',
-      directory: keyUtil.sshnpHomeDirectory,
-    );
-
-    /// Add the key pair to the key utility
-    await keyUtil.addKeyPair(keyPair: ephemeralKeyPair);
-
     Process? bean;
-    try {
-      /// Start the initial tunnel
-      sendProgress('Starting tunnel session');
-      bean = await startInitialTunnelSession(
-        ephemeralKeyPairIdentifier: ephemeralKeyPair.identifier,
-        localRvPort: localRvPort,
+
+    // If we're not encrypting traffic on the sockets, then we create an extra
+    // ssh session in order to encrypt the user's "real" ssh session.
+    if (params.encryptRvdTraffic == false) {
+      /// Load the ephemeral private key into a key pair
+      AtSshKeyPair ephemeralKeyPair = AtSshKeyPair.fromPem(
+        sshnpdChannel.ephemeralPrivateKey!,
+        identifier: 'ephemeral_$sessionId',
+        directory: keyUtil.sshnpHomeDirectory,
       );
-    } finally {
-      /// Remove the key pair from the key utility.
+
+      /// Add the key pair to the key utility
+      await keyUtil.addKeyPair(keyPair: ephemeralKeyPair);
+
       try {
-        await keyUtil.deleteKeyPair(identifier: ephemeralKeyPair.identifier);
-      } catch (e) {
-        logger.shout('Failed to delete ephemeral keyPair: $e');
+        /// Start the initial tunnel
+        sendProgress('Starting tunnel session');
+        bean = await startInitialTunnelSession(
+          ephemeralKeyPairIdentifier: ephemeralKeyPair.identifier,
+          localRvPort: localRvPort,
+        );
+      } finally {
+        /// Remove the key pair from the key utility.
+        try {
+          await keyUtil.deleteKeyPair(identifier: ephemeralKeyPair.identifier);
+        } catch (e) {
+          logger.shout('Failed to delete ephemeral keyPair: $e');
+        }
       }
     }
 

packages/dart/noports_core/lib/src/sshnp/util/ssh_session_handler/dart_ssh_session_handler.dart

@@ -89,9 +89,7 @@ mixin DartSshSessionHandler on SshnpCore
   }
 
   @override
-  Future<SSHClient> startUserSession({
-    required SSHClient tunnelSession,
-  }) async {
+  Future<SSHClient> startUserSession({SSHSocket? sshSocket}) async {
     if (identityKeyPair == null) {
       throw SshnpError('Identity Key pair is mandatory with the dart client.');
     }
@@ -101,13 +99,24 @@ mixin DartSshSessionHandler on SshnpCore
     logger
         .info('Starting user ssh session as $username to localhost:$localPort');
 
+    SSHClient userSshClient;
     SshClientHelper helper = SshClientHelper(logger);
-    SSHClient userSshClient = await helper.createDirectSshClient(
-      host: 'localhost',
-      port: localPort,
-      username: username,
-      keyPair: identityKeyPair!,
-    );
+    if (sshSocket == null) {
+      userSshClient = await helper.createDirectSshClient(
+        host: 'localhost',
+        port: localPort,
+        username: username,
+        keyPair: identityKeyPair!,
+      );
+    } else {
+      userSshClient = await helper.createSshClientWithSshSocket(
+        sshSocket: sshSocket,
+        host: 'localhost',
+        port: localPort,
+        username: username,
+        keyPair: identityKeyPair!,
+      );
+    }
 
     if (!params.addForwardsToTunnel) {
       var optionsSplitBySpace = params.localSshOptions.join(' ').split(' ');

packages/dart/noports_core/lib/src/sshnp/util/ssh_session_handler/openssh_ssh_session_handler.dart

@@ -108,9 +108,7 @@ mixin OpensshSshSessionHandler on SshnpCore
   }
 
   @override
-  Future<Process?> startUserSession({
-    required Process? tunnelSession,
-  }) async {
+  Future<Process?> startUserSession() async {
     throw UnimplementedError();
   }
 }

packages/dart/noports_core/lib/src/sshnp/util/ssh_session_handler/ssh_session_handler.dart

@@ -13,7 +13,5 @@ mixin SshSessionHandler<T> {
 
   @protected
   @visibleForTesting
-  Future<T> startUserSession({
-    required T tunnelSession,
-  });
+  Future<T> startUserSession();
 }

packages/dart/noports_core/test/sshnp/util/ssh_session_handler/openssh_ssh_session_handler_mocks.dart

@@ -70,7 +70,7 @@ class StubbedSshnp extends SshnpCore
   final SrvdChannel _srvdChannel;
 
   @override
-  Future<Process?> startUserSession({required Process? tunnelSession}) {
+  Future<Process?> startUserSession() {
     throw UnimplementedError();
   }
 

tests/e2e_all/scripts/common/apkam_setup.sh

@@ -38,6 +38,8 @@ enroll() {
   apkamDev=$(getApkamDeviceName "$which" "$commitId")
   keysFileName=$(getApkamKeysFile "$atSign" "$apkamApp" "$apkamDev")
 
+  rm -f "$keysFileName"
+
   logInfo "Denying any pending enrollment requests for $atSign with apkamAppName $apkamApp and apkamDeviceName $apkamDev"
   $authBinary deny -r "$atDirectoryHost" -a "$atSign" --arx "$apkamApp" --drx "$apkamDev" || return $?