Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Implement no-encrypt-traffic in npt #1401

Merged
merged 11 commits into from
Oct 3, 2024

Conversation

sitaram-kalluri
Copy link
Member

@sitaram-kalluri sitaram-kalluri commented Sep 27, 2024

- What I did

  • Enabled the --no-encrypt-rvd-traffic flag, which allows data transmission between the client and the daemon in plain text format

- How I did it

  • In bin/npt.dart, introduced the encrypt-rvd-traffic flag, allowing users to toggle whether data transmission should be encrypted or sent in plain text.

  • In srv_impl.dart, modified the run() method in the SrvImplDart class to support data transmission in plain text. Previously, the run() method only supported encrypted transmission.

  • Changes in _runClientSideMulti() and _runDaemonSideMulti() methods:

    • With the existing behaviour, data transmission always remains encrypted.
    • Now, encryption is applied if both sessionAESKeyString and sessionIVString are non-null.
    • The existing encrypted transmission logic has been refactored into:
      • _clientSideEncryptedSocket() on the client side.
      • _daemonSideEncryptedSocket() on the daemon side.
    • Introduced new methods for plain text transmission:
      • _clientSidePlainSocket(): Sends the command "connect:no:encrypt" to the sessionControlSocket on the daemon side. Aligned the connect command format with the existing "connect:sessionAESKey:sessionIV"
      • _daemonSidePlainSocket(): Handles plain text transmission on the daemon side.
    • In _runDaemonSideMulti(), moved the controlStream.listen() logic to a reusable method, "_sessionControlSocketListener", which is shared between encrypted and plain text transmission.
    • In _handleMultiConnectRequest(), added support for the connect:no:encrypt command, which the client sends when encryption is not needed.

- How to verify it

  • Tested the below scenarios manually:

    • Ran npt command with " --no-encrypt-rvd-traffic" able to run command successfully and connect to remote machine with SSH command.
    • Able to launch the multiple SSH teminal.
  • Attaching the demon and client logs:

  • Daemon logs

INFO|2024-09-27 13:32:47.181029|SrvImplExec|SrvImplExec.run(): executing /home/sitaram/IdeaProjects/atsign/noports/packages/dart/sshnoports/bin/srv -h 127.0.0.1 -p 42397 --local-port 22 --local-host localhost --timeout 30 --multi --rv-auth
INFO|2024-09-27 13:32:47.186497|SrvImplExec|rv stderr | INFO|2024-09-27 13:32:47.186177| SrvImplDart |New SrvImplDart - localPort 22
INFO|2024-09-27 13:32:47.186893|SrvImplExec|rv stderr | INFO|2024-09-27 13:32:47.186768| SrvImplDart |_runDaemonSideMulti authenticating control socket connection to rvd
INFO|2024-09-27 13:32:47.186940|SrvImplExec|rv stderr | INFO|2024-09-27 13:32:47.186856| SrvImplDart |_runDaemonSideMulti: On the daemon side traffic is transmitted in plain text
INFO|2024-09-27 13:32:47.186960|SrvImplExec|rv stderr | rv started successfully
INFO|2024-09-27 13:32:47.187021|SrvImplExec|rv stderr |
INFO|2024-09-27 13:32:47.288131| sshnpd |Started rv - pid is 18521
  • Client logs:
INFO|2024-09-27 13:32:47.464032| SshnpdChannel |sshnpdAck: SshnpdAck.acknowledged
INFO|2024-09-27 13:32:47.471544| SrvImplDart |New SrvImplDart - localPort 45863
INFO|2024-09-27 13:32:47.472116| SrvImplDart |_runClientSideMulti authenticating control socket connection to rvd
INFO|2024-09-27 13:32:47.472184| SrvImplDart |_runClientSideMulti: On the client-side traffic is transmitted in plain text
INFO|2024-09-27 13:32:47.472274| SrvImplDart |_runClientSideMulti serverToSocket is ready
INFO|2024-09-27 13:33:00.073582| SrvImplDart |_runClientSideMulti Sending connect request
INFO|2024-09-27 13:33:00.073744| SrvImplDart |_runClientSideMulti authenticating new connection to rvd
  • Ran npt command without " --no-encrypt-rvd-traffic" able to run command successfully and connect to remote machine with SSH command. This is to verify the existing behaviour.

  • Daemon logs:

INFO|2024-09-27 13:33:46.724168|SrvImplExec|SrvImplExec.run(): executing /home/sitaram/IdeaProjects/atsign/noports/packages/dart/sshnoports/bin/srv -h 127.0.0.1 -p 43819 --local-port 22 --local-host localhost --timeout 30 --multi --rv-auth --rv-e2ee
INFO|2024-09-27 13:33:46.729217|SrvImplExec|rv stderr | INFO|2024-09-27 13:33:46.729029| SrvImplDart |New SrvImplDart - localPort 22
INFO|2024-09-27 13:33:46.729729|SrvImplExec|rv stderr | INFO|2024-09-27 13:33:46.729647| SrvImplDart |_runDaemonSideMulti authenticating control socket connection to rvd
INFO|2024-09-27 13:33:46.729807|SrvImplExec|rv stderr | INFO|2024-09-27 13:33:46.729746| SrvImplDart |_runDaemonSideMulti: On the daemon side traffic is encrypted
INFO|2024-09-27 13:33:46.729830|SrvImplExec|rv stderr | rv started successfully
INFO|2024-09-27 13:33:46.830295| sshnpd |Started rv - pid is 19449
  • Client logs:
INFO|2024-09-27 13:33:47.011133| SshnpdChannel |sshnpdAck: SshnpdAck.acknowledged
INFO|2024-09-27 13:33:47.015811| SrvImplDart |New SrvImplDart - localPort 46751
INFO|2024-09-27 13:33:47.016365| SrvImplDart |_runClientSideMulti authenticating control socket connection to rvd
INFO|2024-09-27 13:33:47.016428| SrvImplDart |_runClientSideMulti: On the client-side traffic is encrypted
INFO|2024-09-27 13:33:47.016454| SrvImplDart |_runClientSideMulti calling SocketConnector.serverToSocket
INFO|2024-09-27 13:33:47.016544| SrvImplDart |_runClientSideMulti serverToSocket is ready
INFO|2024-09-27 13:34:02.792352| SrvImplDart |_runClientSideMulti Sending connect request
INFO|2024-09-27 13:34:02.792563| SrvImplDart |_runClientSideMulti authenticating new connection to rvd
  • Added an End-to-End test : npt_to_port_22_no_encrypt_traffic to verify the changes
    - Description for the changelog
  • fix: Implement no-encrypt-traffic in npt

Copy link
Contributor

@gkc gkc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add an e2e test for npt which uses no-encrypt-traffic. You can copy npt_to_port_22 to a new file in the tests/e2e_all/scripts/tests directory and modify it appropriately

@sitaram-kalluri
Copy link
Member Author

Please add an e2e test for npt which uses no-encrypt-traffic. You can copy npt_to_port_22 to a new file in the tests/e2e_all/scripts/tests directory and modify it appropriately

@gkc : Added E2E test : npt_to_port_22_no_encrypt_traffic. Please review the changes.

@sitaram-kalluri sitaram-kalluri requested a review from gkc October 1, 2024 11:11
Copy link
Contributor

@gkc gkc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@XavierChanth can you take a quick look over this also please?

@sitaram-kalluri We will need to do the same for the C sshnpd and srv implementations. Can you create a ticket for that please?

@XavierChanth
Copy link
Member

So, in C multi, I need to check for connect:no:encrypt and if so, disable encryption?

@sitaram-kalluri
Copy link
Member Author

LGTM

@XavierChanth can you take a quick look over this also please?

@sitaram-kalluri We will need to do the same for the C sshnpd and srv implementations. Can you create a ticket for that please?

@gkc : Sure, will create tickets for both.

@gkc
Copy link
Contributor

gkc commented Oct 2, 2024

So, in C multi, I need to check for connect:no:encrypt and if so, disable encryption?

I think so. @sitaram-kalluri plz confirm

@sitaram-kalluri
Copy link
Member Author

sitaram-kalluri commented Oct 3, 2024

So, in C multi, I need to check for connect:no:encrypt and if so, disable encryption?

I think so. @sitaram-kalluri plz confirm

"Yes, that's correct. We need to check for 'connect:no:encrypt' and disable encryption accordingly. Just to clarify, I introduced 'connect:no:encrypt' in the srv, so it may not be present in the at_c repo."

@gkc gkc merged commit d0eff12 into trunk Oct 3, 2024
9 checks passed
@gkc gkc deleted the 1181-provide-ability-to-use-no-encrypt-traffic-in-npt branch October 3, 2024 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide ability to use no-encrypt-traffic in npt
3 participants