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

feat: implement direct ssh #332

Merged
merged 15 commits into from Aug 17, 2023
Merged

feat: implement direct ssh #332

merged 15 commits into from Aug 17, 2023

Conversation

gkc
Copy link
Contributor

@gkc gkc commented Aug 12, 2023

- What I did

  • Removes necessity for client side to have an sshd running by enabling clients to request that the daemon set things up for a direct ssh
  • Daemon sets up a socket connector between the rvd and port 22 on localhost
  • Client runs ssh to rvd host and port (which is connected through to the daemon's socket connector) with a request for local port forwarding of some available local port. Once this session is running, then client may execute other ssh commands by running ssh to local host and that local port.

NOTE 1 There is one bug I know about right now. If you are doing a direct ssh then you MUST provide -s <xxx.pub> argument to the sshnp program. If you do not then the initial background ssh (to set up the local port forwarding) will fail. I will add some defensive code later

NOTE 2 Added signing and verification of sshnp-to-sshnpd request notification payloads. TODO: Add signing and verification of sshnpd-to-sshnp response notification payloads

- How I did it
Easiest to see by looking at the individual commits, which show the progress step by step

- How to verify it

  • Automated tests should pass - they are currently only testing the legacy behaviour (reverse ssh)
    • Before finalizing this PR I will augment the test pack so that it tests the new behaviour where appropriate, in addition to testing all of the legacy behaviour
  • I have manually verified that direct ssh works
  • sshnpd output fragment
INFO|2023-08-12 17:37:03.127502| sshnpd |>=3.5.0 request for ssh received from @garycasey ( {id: ccd980e5-badf-48de-8cde-c15e5018b6cb, key: @baboonblue18:ssh_request.mbp.sshnp@garycasey, from: @garycasey, to: @baboonblue18, epochMillis: 1691858223075, value: {"direct":true,"sessionId":"8e3d4701-b3a6-419f-a9e7-706f28817760","host":"85.239.63.180","port":43593}, operation: update, messageType: MessageType.key, isEncrypted: true, notificationStatus: , expiresAt: null, metadata: Metadata{ttl: null, ttb: null, ttr: null,ccd: null, isPublic: false, isHidden: false, availableAt : null, expiresAt : null, refreshAt : null, createdAt : null, updatedAt : null, isBinary : false, isEncrypted : null, isCached : false, dataSignature: null, sharedKeyStatus: null, encryptedSharedKey: null, pubKeyCheckSum: null, encoding: null, encKeyName: null, encAlgo: null, ivNonce: ybUQhFR7S8FP33hybQ+MyQ==, skeEncKeyName: null, skeEncAlgo: null}} ) 
SHOUT|2023-08-12 17:37:03.127524| sshnpd |Setting up ports for direct ssh session using hostSsh (/usr/bin/ssh) from: @garycasey session: 8e3d4701-b3a6-419f-a9e7-706f28817760 
INFO|2023-08-12 17:37:03.133149| sshnpd |Started rv - pid is 52752 
INFO|2023-08-12 17:37:04.086938| sshnpd |SUCCESS:id: 0c0340e0-215b-47e6-a957-6be4b38fb92c status: NotificationStatusEnum.delivered for: 8e3d4701-b3a6-419f-a9e7-706f28817760 with value: connected 
  • sshnp output fragment
gary@gkc2019-2 sshnoports % bin/sshnp -f @garycasey -t @baboonblue18 -d mbp -h @rv_eu -s noports.pub -u gary --no-legacy-daemon              
SHOUT|2023-08-12 17:36:58.170618| sshnp |useDirectSsh(false, @rv_eu) returned true 
ssh -p 52204 gary@localhost -o StrictHostKeyChecking=accept-new -o IdentitiesOnly=yes -i /Users/gary/.ssh/noports
gary@gkc2019-2 sshnoports % ssh -p 52204 gary@localhost -o StrictHostKeyChecking=accept-new -o IdentitiesOnly=yes -i /Users/gary/.ssh/noports

Last login: Sat Aug 12 17:37:06 2023 from 127.0.0.1
Setting ulimit to 10000
ulimit is now 10000
Setting umask 002
umask is now u=rwx,g=rwx,o=rx
gary@gkc2019-2 ~ % ps -ef | egrep "ssh |sshrv"                                                                                      
  502 52752     1   0  5:37pm ??         0:00.05 /Users/gary/dev/atsign/repos/sshnoports/packages/sshnoports/bin/sshrv 85.239.63.180 43593
  502 52766     1   0  5:37pm ??         0:00.01 /usr/bin/ssh gary@85.239.63.180 -p 46005 -i /Users/gary/.ssh/noports -L 52204:localhost:22 -o LogLevel=VERBOSE -t -t -o StrictHostKeyChecking=accept-new -o IdentitiesOnly=yes -o BatchMode=yes -o ExitOnForwardFailure=yes -f sleep 15
  502 52777 36331   0  5:37pm ttys013    0:00.10 ssh -p 52204 gary@localhost -o StrictHostKeyChecking=accept-new -o IdentitiesOnly=yes -i /Users/gary/.ssh/noports
  502 53081 52784   0  5:37pm ttys015    0:00.00 egrep ssh |sshrv
gary@gkc2019-2 ~ % 
gary@gkc2019-2 ~ % date
Sat 12 Aug 2023 17:39:13 BST
gary@gkc2019-2 ~ % 
gary@gkc2019-2 ~ % exit
Connection to localhost closed.
gary@gkc2019-2 sshnoports % ps -ef | egrep "ssh | sshrv"                                                                                     
  502 53471 36331   0  5:39pm ttys013    0:00.00 egrep ssh | sshrv

gkc added 10 commits August 12, 2023 09:50
…value for a given key in a Map m is non-null and of a specific Type
refactor: re-ordered method parameters for consistency
feat: added (empty) `startDirectSsh` method
…refactoring.

- Function renames / moves
- Instance variables renames / removals
- The `cleanupAfterReverseSsh` function now takes an SSHNP object as its parameter, so it can check whether the initialization had completed and whether a reverse ssh was actually used. This was necessary since that function is called from many different places
- Added `legacy-daemon` flag
- For the interim, `legacy-daemon` defaults to true, so that the existing test pack can run unchanged.
…refactoring.

- Function renames / moves
- Instance variables renames / removals
- The `cleanupAfterReverseSsh` function now takes an SSHNP object as its parameter, so it can check whether the initialization had completed and whether a reverse ssh was actually used. This was necessary since that function is called from many different places
- Added `legacy-daemon` flag
- For the interim, `legacy-daemon` defaults to true, so that the existing test pack can run unchanged.
…refactoring.

- Function renames / moves
- Instance variables renames / removals
- The `cleanupAfterReverseSsh` function now takes an SSHNP object as its parameter, so it can check whether the initialization had completed and whether a reverse ssh was actually used. This was necessary since that function is called from many different places
- Added `legacy-daemon` flag
- For the interim, `legacy-daemon` defaults to true, so that the existing test pack can run unchanged.
@gkc
Copy link
Contributor Author

gkc commented Aug 12, 2023

@cconstab All working. Ready for initial review.

I've not yet decided on the best way to decide on what flavour of notification to send to the daemon. Maybe better would be an option like --daemon-version={3.1|3.2|3.3|3.4|3.5|discover} where 'discover' would be the default and would mean

  • send a ping to the daemon to find out
  • if there's no response, then assume the version is 3.4 or less
  • direct ssh would require daemon 3.5 or greater

Currently there is a flag called --legacy-daemon flag. This defaults to true, so that I could easily prove that I hadn't broken any backwards compatibility by just checking that all of the existing automated tests pass. To do a manual test of a direct ssh right now, you need to set the --no-legacy-daemon flag. For example
bin/sshnp -f @garycasey -t @baboonblue18 -d mbp -h @rv_eu -s noports.pub -u gary --no-legacy-daemon

@gkc gkc requested a review from cconstab August 12, 2023 16:54
@cconstab
Copy link
Member

The bug... At present we create a ssh key pair and use that to auth to get to the sshd on localhost/22.
Not sure if you are doing the same as yet for forward ssh? We should..

The sshnp aim should just to be to get to the localhist/22 and auth with the new key but not to get a shell. Via the sleep command in .ssh/authorized_hosts that only alllows the forwarding of port 22. Then a ssh clint can auth in the normal manner.

Why do this ?

Because many people already have ssh auth / key rotation in place and we do not want to break that we just want to get tp the daemon..

You may have done just this already not been able to look at the code as yet.

Very exciting !

@gkc
Copy link
Contributor Author

gkc commented Aug 12, 2023

Ephemeral keys are required for the server to ssh to the client. There is no need for an ephemeral keypair for the client to ssh to the server - user just specifies the identity file they want to use

@gkc
Copy link
Contributor Author

gkc commented Aug 12, 2023

@cconstab and I have chatted and believe best is to make this v4.0.0 with "--legacy-daemon" defaulting to false, and with release notes that say “client is fully backwards compatible with previous versions’ daemons if you supply the —legacy-daemon flag” and “v4 daemons are fully compatible with previous versions’ clients”

I will adjust the e2e tests accordingly, and also add tests of direct ssh where appropriate (local-local only, to start with)

XavierChanth
XavierChanth previously approved these changes Aug 13, 2023
- Added utility functions which are used by sshnp to sign and for sshnpd to verify request notification payloads
- Having sshnpd sign and sshnp verify the response notification payloads will be straightforward next step
style: renamed a variable
@gkc gkc marked this pull request as ready for review August 13, 2023 21:52
@gkc gkc changed the title feat: implement direct ssh feat: implement direct ssh : DO NOT MERGE Aug 13, 2023
@gkc gkc changed the title feat: implement direct ssh : DO NOT MERGE feat: implement direct ssh Aug 15, 2023
# Conflicts:
#	packages/sshnoports/lib/version.dart
@gkc gkc merged commit 61b72ad into trunk Aug 17, 2023
22 checks passed
@gkc gkc deleted the gkc-direct-ssh branch August 17, 2023 10:36
@gkc gkc restored the gkc-direct-ssh branch August 17, 2023 10:36
@gkc gkc deleted the gkc-direct-ssh branch February 7, 2024 13:29
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.

None yet

3 participants