From 5aa07edb16dfdb2ee339d9fdc43ce01d60763577 Mon Sep 17 00:00:00 2001 From: Sitaram Kalluri Date: Thu, 26 Sep 2024 21:58:25 +0530 Subject: [PATCH 1/4] fix: Implement no-encrypt-traffic in npt --- .../noports_core/lib/src/srv/srv_impl.dart | 262 +++++++++++++----- packages/dart/sshnoports/bin/npt.dart | 13 +- packages/dart/sshnoports/lib/src/version.dart | 2 +- 3 files changed, 201 insertions(+), 76 deletions(-) diff --git a/packages/dart/noports_core/lib/src/srv/srv_impl.dart b/packages/dart/noports_core/lib/src/srv/srv_impl.dart index 875177704..ff7ef7340 100644 --- a/packages/dart/noports_core/lib/src/srv/srv_impl.dart +++ b/packages/dart/noports_core/lib/src/srv/srv_impl.dart @@ -407,11 +407,15 @@ class SrvImplDart implements Srv { Future run() async { try { var hosts = await InternetAddress.lookup(streamingHost); - late SocketConnector sc; + // Determines whether the traffic in the socket is encrypted or transmitted in plain text. + bool encryptRvdTraffic = + (sessionAESKeyString != null && sessionIVString != null); + if (bindLocalPort) { if (multi) { - if (sessionAESKeyString == null || sessionIVString == null) { + if (encryptRvdTraffic == true && + (sessionAESKeyString == null || sessionIVString == null)) { throw ArgumentError('Symmetric session encryption key required'); } sc = await _runClientSideMulti(hosts: hosts, timeout: timeout); @@ -421,7 +425,8 @@ class SrvImplDart implements Srv { } else { // daemon side if (multi) { - if (sessionAESKeyString == null || sessionIVString == null) { + if (encryptRvdTraffic == true && + (sessionAESKeyString == null || sessionIVString == null)) { throw ArgumentError('Symmetric session encryption key required'); } sc = await _runDaemonSideMulti(hosts: hosts, timeout: timeout); @@ -485,7 +490,6 @@ class SrvImplDart implements Srv { } }, ); - return sc; } @@ -495,7 +499,6 @@ class SrvImplDart implements Srv { }) async { // client side SocketConnector? socketConnector; - Socket sessionControlSocket = await Socket.connect( streamingHost, streamingPort, timeout: Duration(seconds: 10)); @@ -505,6 +508,74 @@ class SrvImplDart implements Srv { ' control socket connection to rvd'); sessionControlSocket.writeln(rvdAuthString); } + + if (sessionAESKeyString != null && sessionIVString != null) { + logger + .info('_runClientSideMulti: On the client-side traffic is encrypted'); + socketConnector = await _clientSideEncryptedSocket( + sessionControlSocket, socketConnector, hosts, timeout); + } else { + logger.info( + '_runClientSideMulti: On the client-side traffic is transmitted in plain text'); + socketConnector = await _clientSidePlainSocket( + sessionControlSocket, socketConnector, hosts, timeout); + } + + logger.info('_runClientSideMulti serverToSocket is ready'); + // upon socketConnector.done, destroy the control socket, and complete + unawaited(socketConnector.done.whenComplete(() { + logger.info('_runClientSideMulti sc.done'); + sessionControlSocket.destroy(); + })); + return socketConnector; + } + + /// On the client side, the data in this socket remains unencrypted and is transmitted in plain text + Future _clientSidePlainSocket( + Socket sessionControlSocket, + SocketConnector? socketConnector, + List hosts, + Duration timeout) async { + sessionControlSocket.listen((event) { + String response = String.fromCharCodes(event).trim(); + logger.info('_runClientSideMulti' + ' Received control socket response: [$response]'); + }, onError: (e) { + logger.severe('_runClientSideMulti controlSocket error: $e'); + socketConnector?.close(); + }, onDone: () { + logger.info('_runClientSideMulti controlSocket done'); + socketConnector?.close(); + }); + socketConnector = await SocketConnector.serverToSocket( + portA: localPort, + addressB: hosts[0], + portB: streamingPort, + verbose: false, + logger: ioSinkForLogger(logger), + multi: multi, + timeout: timeout, + beforeJoining: (Side sideA, Side sideB) { + logger.info('_runClientSideMulti Sending connect request'); + sessionControlSocket + .add(Uint8List.fromList('connect:no:encrypt\n'.codeUnits)); + // Authenticate the sideB socket (to the rvd) + if (rvdAuthString != null) { + logger + .info('_runClientSideMulti authenticating new connection to rvd'); + sideB.socket.writeln(rvdAuthString); + } + }, + ); + return socketConnector; + } + + /// On the client side, the data in encrypted and is transmitted through this socket. + Future _clientSideEncryptedSocket( + Socket sessionControlSocket, + SocketConnector? socketConnector, + List hosts, + Duration timeout) async { DataTransformer controlEncrypter = createEncrypter(sessionAESKeyString!, sessionIVString!); DataTransformer controlDecrypter = @@ -556,14 +627,6 @@ class SrvImplDart implements Srv { sideB.transformer = createDecrypter(socketAESKey, socketIV); }, ); - logger.info('_runClientSideMulti serverToSocket is ready'); - - // upon socketConnector.done, destroy the control socket, and complete - unawaited(socketConnector.done.whenComplete(() { - logger.info('_runClientSideMulti sc.done'); - sessionControlSocket.destroy(); - })); - return socketConnector; } @@ -575,30 +638,50 @@ class SrvImplDart implements Srv { List args = request.split(":"); switch (args.first) { case 'connect': - if (args.length != 3) { - logger.severe('Unknown request to control socket: [$request]'); + // Handles the request from the socket where data needs no encryption. + // When --no-encrypt-rvd-traffic flag is set to true. + if (request == 'connect:no:encrypt') { + await SocketConnector.socketToSocket( + connector: sc, + addressA: + (await InternetAddress.lookup(localHost ?? 'localhost'))[0], + portA: localPort, + addressB: hosts[0], + portB: streamingPort, + verbose: false, + logger: ioSinkForLogger(logger)); + if (rvdAuthString != null) { + logger.info('_runDaemonSideMulti authenticating' + ' new socket connection to rvd'); + sc.connections.last.sideB.socket.writeln(rvdAuthString); + } return; + } else { + // In this case, the data in the socket is encrypted. + if (args.length != 3) { + logger.severe('Unknown request to control socket: [$request]'); + return; + } + logger.info('_runDaemonSideMulti' + ' Control socket received ${args.first} request - ' + ' creating new socketToSocket connection'); + await SocketConnector.socketToSocket( + connector: sc, + addressA: + (await InternetAddress.lookup(localHost ?? 'localhost'))[0], + portA: localPort, + addressB: hosts[0], + portB: streamingPort, + verbose: false, + logger: ioSinkForLogger(logger), + transformAtoB: createEncrypter(args[1], args[2]), + transformBtoA: createDecrypter(args[1], args[2])); + if (rvdAuthString != null) { + logger.info('_runDaemonSideMulti authenticating' + ' new socket connection to rvd'); + sc.connections.last.sideB.socket.writeln(rvdAuthString); + } } - logger.info('_runDaemonSideMulti' - ' Control socket received ${args.first} request - ' - ' creating new socketToSocket connection'); - await SocketConnector.socketToSocket( - connector: sc, - addressA: - (await InternetAddress.lookup(localHost ?? 'localhost'))[0], - portA: localPort, - addressB: hosts[0], - portB: streamingPort, - verbose: false, - logger: ioSinkForLogger(logger), - transformAtoB: createEncrypter(args[1], args[2]), - transformBtoA: createDecrypter(args[1], args[2])); - if (rvdAuthString != null) { - logger.info('_runDaemonSideMulti authenticating' - ' new socket connection to rvd'); - sc.connections.last.sideB.socket.writeln(rvdAuthString); - } - break; default: logger.severe('Unknown request to control socket: [$request]'); @@ -622,6 +705,41 @@ class SrvImplDart implements Srv { ' control socket connection to rvd'); sessionControlSocket.writeln(rvdAuthString); } + + if (sessionAESKeyString != null && sessionIVString != null) { + logger + .info('_runDaemonSideMulti: On the daemon side traffic is encrypted'); + _daemonSideEncryptedSocket(sessionControlSocket, sc, hosts); + } else { + logger.info( + '_runDaemonSideMulti: On the daemon side traffic is transmitted in plain text'); + _daemonSidePlainSocket(sessionControlSocket, sc, hosts); + } + + // upon socketConnector.done, destroy the control socket, and complete + unawaited(sc.done.whenComplete(() { + sessionControlSocket.destroy(); + })); + + return sc; + } + + void _daemonSidePlainSocket(Socket sessionControlSocket, SocketConnector sc, + List hosts) { + Mutex controlStreamMutex = Mutex(); + sessionControlSocket.listen((event) async { + await _sessionControlSocketListener(controlStreamMutex, event, sc, hosts); + }, onError: (e) { + logger.severe('controlSocket error: $e'); + sc.close(); + }, onDone: () { + logger.info('controlSocket done'); + sc.close(); + }); + } + + void _daemonSideEncryptedSocket(Socket sessionControlSocket, + SocketConnector sc, List hosts) { DataTransformer controlEncrypter = createEncrypter(sessionAESKeyString!, sessionIVString!); DataTransformer controlDecrypter = @@ -636,38 +754,7 @@ class SrvImplDart implements Srv { Mutex controlStreamMutex = Mutex(); controlStream.listen((event) async { logger.info('Received event on control socket.'); - try { - await controlStreamMutex.acquire(); - if (event.isEmpty) { - logger.info('Empty control message (Uint8List) received'); - return; - } - String eventStr = String.fromCharCodes(event).trim(); - if (eventStr.isEmpty) { - logger.info('Empty control message (String) received'); - return; - } - // TODO The code below (splitting by `connect:`) resolves a - // particular issue for the moment, but the overall approach - // to handling control messages needs to be redone, e.g. : - // Ideally - send the control request, and a newline - // => as of this commit, this is the case - // Receive - wait for newline, handle the request, repeat - // => older npt clients don't send `\n` so we will need to add some - // magic to handle both (a) older clients which don't send `\n` - // as well as (b) newer ones which do. Cleanest is to add a - // flag to the npt request from the client stating that it sends - // `\n` . If so then we handle that cleanly; if not then we use - // this approach (split by `connect:`) - List requests = eventStr.split('connect:'); - for (String request in requests) { - if (request.isNotEmpty) { - await _handleMultiConnectRequest(sc, hosts, 'connect:$request'); - } - } - } finally { - controlStreamMutex.release(); - } + await _sessionControlSocketListener(controlStreamMutex, event, sc, hosts); }, onError: (e) { logger.severe('controlSocket error: $e'); sc.close(); @@ -675,13 +762,42 @@ class SrvImplDart implements Srv { logger.info('controlSocket done'); sc.close(); }); + } - // upon socketConnector.done, destroy the control socket, and complete - unawaited(sc.done.whenComplete(() { - sessionControlSocket.destroy(); - })); - - return sc; + Future _sessionControlSocketListener(Mutex controlStreamMutex, + List event, SocketConnector sc, List hosts) async { + try { + await controlStreamMutex.acquire(); + if (event.isEmpty) { + logger.info('Empty control message (Uint8List) received'); + return; + } + String eventStr = String.fromCharCodes(event).trim(); + if (eventStr.isEmpty) { + logger.info('Empty control message (String) received'); + return; + } + // TODO The code below (splitting by `connect:`) resolves a + // particular issue for the moment, but the overall approach + // to handling control messages needs to be redone, e.g. : + // Ideally - send the control request, and a newline + // => as of this commit, this is the case + // Receive - wait for newline, handle the request, repeat + // => older npt clients don't send `\n` so we will need to add some + // magic to handle both (a) older clients which don't send `\n` + // as well as (b) newer ones which do. Cleanest is to add a + // flag to the npt request from the client stating that it sends + // `\n` . If so then we handle that cleanly; if not then we use + // this approach (split by `connect:`) + List requests = eventStr.split('connect:'); + for (String request in requests) { + if (request.isNotEmpty) { + await _handleMultiConnectRequest(sc, hosts, 'connect:$request'); + } + } + } finally { + controlStreamMutex.release(); + } } Future _runDaemonSideSingle({ diff --git a/packages/dart/sshnoports/bin/npt.dart b/packages/dart/sshnoports/bin/npt.dart index c0f0792bc..d57faab02 100644 --- a/packages/dart/sshnoports/bin/npt.dart +++ b/packages/dart/sshnoports/bin/npt.dart @@ -4,7 +4,6 @@ import 'dart:io'; // other packages import 'package:args/args.dart'; - // atPlatform packages import 'package:at_cli_commons/at_cli_commons.dart' as cli; import 'package:at_utils/at_utils.dart'; @@ -12,7 +11,6 @@ import 'package:duration/duration.dart'; import 'package:noports_core/npt.dart'; import 'package:noports_core/sshnp_foundation.dart'; import 'package:sshnoports/src/extended_arg_parser.dart'; - // local packages import 'package:sshnoports/src/print_version.dart'; @@ -203,6 +201,16 @@ void main(List args) async { ' it has started its session.', ); + parser.addFlag( + 'encrypt-rvd-traffic', + aliases: ['et'], + help: 'When true, traffic via the socket rendezvous is encrypted,' + ' in addition to whatever encryption the traffic already has' + ' (e.g. an ssh session)', + defaultsTo: DefaultArgs.encryptRvdTraffic, + negatable: true, + ); + // Parse Args ArgResults parsedArgs = parser.parse(args); @@ -321,6 +329,7 @@ void main(List args) async { inline: inline, daemonPingTimeout: Duration(seconds: int.parse(parsedArgs['daemon-ping-timeout'])), + encryptRvdTraffic: parsedArgs['encrypt-rvd-traffic'], timeout: parseDuration(timeoutArg), ); diff --git a/packages/dart/sshnoports/lib/src/version.dart b/packages/dart/sshnoports/lib/src/version.dart index fb0682dfd..3bb93112f 100644 --- a/packages/dart/sshnoports/lib/src/version.dart +++ b/packages/dart/sshnoports/lib/src/version.dart @@ -1,2 +1,2 @@ // Generated code. Do not modify. -const packageVersion = '5.6.1'; +const packageVersion = '5.6.2'; From f3e019a1f681ce124021a936ccd8c964b18d06e0 Mon Sep 17 00:00:00 2001 From: Sitaram Kalluri Date: Tue, 1 Oct 2024 08:32:03 +0530 Subject: [PATCH 2/4] fix: Add E2E test for no-encrypt-traffic --- .../common/common_functions.include.sh | 12 ++- .../tests/npt_to_port_22_no_encrypt_traffic | 88 +++++++++++++++++++ 2 files changed, 97 insertions(+), 3 deletions(-) create mode 100755 tests/e2e_all/scripts/tests/npt_to_port_22_no_encrypt_traffic diff --git a/tests/e2e_all/scripts/common/common_functions.include.sh b/tests/e2e_all/scripts/common/common_functions.include.sh index 4a4d29cf5..90582b762 100644 --- a/tests/e2e_all/scripts/common/common_functions.include.sh +++ b/tests/e2e_all/scripts/common/common_functions.include.sh @@ -56,14 +56,20 @@ getBaseSshnpCommand() { } getBaseNptCommand() { - if (($# != 1)); then - logErrorAndExit "getBaseNptCommand requires 1 argument (clientBinaryPath)" + if (($# < 1 || $# > 2)); then + logErrorAndExit "getBaseNptCommand requires 1 mandatory argument (clientBinaryPath) and optionally a second argument (encryptRvdTraffic)" fi clientBinaryPath="$1" l1="$clientBinaryPath/npt -f $clientAtSign -d $deviceName" l2=" -t $daemonAtSign -r $srvAtSign" l3=" --root-domain $atDirectoryHost" - echo "$l1" "$l2" "$l3" + if [ -z "$2" ]; then + echo "npt command running with encrypt traffic" + echo "$l1" "$l2" "$l3" + else + echo "npt command running with no-encrypt traffic" + echo "$l1" "$l2" "$l3" "--no-encrypt-rvd-traffic" + fi } getTestSshCommand() { diff --git a/tests/e2e_all/scripts/tests/npt_to_port_22_no_encrypt_traffic b/tests/e2e_all/scripts/tests/npt_to_port_22_no_encrypt_traffic new file mode 100755 index 000000000..49ca7a6e2 --- /dev/null +++ b/tests/e2e_all/scripts/tests/npt_to_port_22_no_encrypt_traffic @@ -0,0 +1,88 @@ +#!/bin/bash + +scriptName=$(basename -- "$0") +testToRun="$scriptName" + +if test -z "$testScriptsDir"; then + echo -e " ${RED}check_env: testScriptsDir is not set${NC}" && exit 1 +fi + +source "$testScriptsDir/common/common_functions.include.sh" +source "$testScriptsDir/common/check_env.include.sh" || exit $? + +daemonVersion="$1" +clientVersion="$2" +extraFlags="--remote-port 22 --exit-when-connected" + +if [[ $(versionIsAtLeast "$clientVersion" "d:5.3.0") == "true" ]]; then + apkamApp=$(getApkamAppName) + apkamDev=$(getApkamDeviceName "client" "$commitId") + keysFile=$(getApkamKeysFile "$clientAtSign" "$apkamApp" "$apkamDev") + extraFlags="$extraFlags -k $keysFile" +fi + +# If client has already been released +# then it has already have been tested against all released daemon versions +# So only test it against the 'current' daemon +# i.e. if client != current and daemon != current then exit 50 + +if ! grep -q "current" <<<"$clientVersion" && ! grep -q "current" <<<"$daemonVersion"; then + logInfo " N/A because released client $(getVersionDescription "$clientVersion") has already been tested against released daemon $(getVersionDescription "$daemonVersion")" + exit 50 +fi + +# Require a v5.1+ client to test v5.1+ features +if [[ $(versionIsLessThan "$clientVersion" "d:5.1.0") == "true" ]] || + [[ $(versionIsLessThan "$daemonVersion" "d:5.1.0") == "true" ]]; then + logInfo " N/A because npt requires client and daemon versions >= v5.1.x" + exit 50 # test rig interprets this exit status as 'test was not applicable' +fi + +deviceName=$(getDeviceNameWithFlags "$commitId" "$daemonVersion") + +# We will capture daemon log from now until end of test +outputDir=$(getOutputDir) +daemonLogFile="${outputDir}/daemons/${deviceName}.log" +daemonLogFragmentName="$(getDaemonLogFragmentName $testToRun $daemonVersion $clientVersion)" +tail -f -n 0 "$daemonLogFile" >>"$daemonLogFragmentName" & +tailPid=$! # We'll kill this later + +clientBinaryPath=$(getPathToBinariesForTypeAndVersion "$clientVersion") + +baseNptCommand=$(getBaseNptCommand "$clientBinaryPath" "--no-encrypt-rvd-traffic") + +# Let's put together the npt command we will execute +nptCommand="$baseNptCommand $extraFlags --verbose" + +# 1. Execute the npt command - its output is the port that npt is using +echo "$(iso8601Date) | Executing $nptCommand" +nptPort=$($nptCommand) + +# 2. Check the exit status +nptExitStatus=$? +if ((nptExitStatus != 0)); then + # Kill the daemon log tail, and exit with the exit status of the npt command + kill "$tailPid" + exit $nptExitStatus +fi + +echo "$(iso8601Date) | npt OK, local port is $nptPort" +echo "$(iso8601Date) | Running ps for the spawned srv process with port $nptPort BEFORE running ssh" +ps -ef | grep "srv " | grep "$nptPort" + +# 3. Execute an ssh +sshCommand="ssh -p $nptPort -o StrictHostKeyChecking=accept-new -o IdentitiesOnly=yes" +sshCommand="${sshCommand} ${remoteUsername}@localhost -i $identityFilename" + +echo "$(iso8601Date) | Executing $sshCommand" + +# shellcheck disable=SC2091 +$(getTestSshCommand "$sshCommand") +sshExitStatus=$? + +echo "$(iso8601Date) | Running ps for the spawned srv process with port $nptPort AFTER running ssh" +ps -ef | grep "srv " | grep "$nptPort" + +# 4. Kill the daemon log tail, and exit with the exit status of the ssh command +kill "$tailPid" +exit $sshExitStatus From 2413fcac778ab6850cd4fa155cc006bd333adfa1 Mon Sep 17 00:00:00 2001 From: Sitaram Kalluri Date: Tue, 1 Oct 2024 09:32:20 +0530 Subject: [PATCH 3/4] fix: Remove echo lines from the getBaseNptCommand function --- tests/e2e_all/scripts/common/common_functions.include.sh | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/e2e_all/scripts/common/common_functions.include.sh b/tests/e2e_all/scripts/common/common_functions.include.sh index 90582b762..10ab020e8 100644 --- a/tests/e2e_all/scripts/common/common_functions.include.sh +++ b/tests/e2e_all/scripts/common/common_functions.include.sh @@ -64,11 +64,9 @@ getBaseNptCommand() { l2=" -t $daemonAtSign -r $srvAtSign" l3=" --root-domain $atDirectoryHost" if [ -z "$2" ]; then - echo "npt command running with encrypt traffic" echo "$l1" "$l2" "$l3" else - echo "npt command running with no-encrypt traffic" - echo "$l1" "$l2" "$l3" "--no-encrypt-rvd-traffic" + echo "$l1" "$l2" "$l3" "$2" fi } From 63fbe426dd46f2b3418db641e13b53f902b3e69d Mon Sep 17 00:00:00 2001 From: Sitaram Kalluri Date: Tue, 1 Oct 2024 14:33:48 +0530 Subject: [PATCH 4/4] fix: Add version constraints for the E2E of npt_to_port_22_no_encrypt_traffic --- .../scripts/tests/npt_to_port_22_no_encrypt_traffic | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/e2e_all/scripts/tests/npt_to_port_22_no_encrypt_traffic b/tests/e2e_all/scripts/tests/npt_to_port_22_no_encrypt_traffic index 49ca7a6e2..3c1bb6402 100755 --- a/tests/e2e_all/scripts/tests/npt_to_port_22_no_encrypt_traffic +++ b/tests/e2e_all/scripts/tests/npt_to_port_22_no_encrypt_traffic @@ -31,10 +31,9 @@ if ! grep -q "current" <<<"$clientVersion" && ! grep -q "current" <<<"$daemonVer exit 50 fi -# Require a v5.1+ client to test v5.1+ features -if [[ $(versionIsLessThan "$clientVersion" "d:5.1.0") == "true" ]] || - [[ $(versionIsLessThan "$daemonVersion" "d:5.1.0") == "true" ]]; then - logInfo " N/A because npt requires client and daemon versions >= v5.1.x" +# The -no-encrypt-rvd-traffic is supported from 5.6.2 release which is the current as of now. +if [[ "$clientVersion" != "d:current" ]] || [[ "$daemonVersion" != "d:current" ]]; then + logInfo " N/A The feature is supported on client and daemon version >= 5.6.2" exit 50 # test rig interprets this exit status as 'test was not applicable' fi