-
Notifications
You must be signed in to change notification settings - Fork 15
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: make sshnpd execute ssh command via Process.start #278
Conversation
…d sshrvd via `AtSignLogger.defaultLoggingHandler = AtSignLogger.stdErrLoggingHandler;`
…and on the host rather than using SSHClient - writes privatekey which it received earlier from sshnp into a tmp file which is later deleted - added a bunch of comments explaining how the command is constructed refactor: remove _sshPublicKey instance variable from SSHNPDImpl and make it a local variable in the `case 'sshpublickey'` in the notification handler
@cconstab My local manual tests worked but the e2e tests are failing with 'Host key verification failed', because of something I'm getting wrong. Please take a look at how I'm constructing the ssh command - a fresh pair of eyes might see what the problem is straight away. |
I believe the tests are failing because SSHClient does not do host key verification, but ssh command by default does Fix would be to add this to the ssh command args -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null |
…` to the ssh command
Applied the fix mentioned in the previous comment, and now only one of the e2e tests is failing, but it's failing in a different way.
I think the repeat failure of the local-to-local test is an artefact of the test rig All of the other tests which are running sshnpd from the branch (local) are passing (i.e. trunk to local, latest release to local, 3.2.0 to local are all passing) In this run, only the first test (local to local) failed: https://github.com/atsign-foundation/sshnoports/actions/runs/5637947000 with the But in this run, both the local to local test failed, AND the v3.2.0 to local test failed for the same ( The local-to-local test will not pass no matter how many times I re-run it @JeremyTubongbanua Can you think of anything particularly special about the local-local test with regards to timings, setup, etc? |
Yes, @XavierChanth recently refactored the tests and I believe the bug is addressed in a comment in this ticket - #276 |
Fix should be in #280 |
@JeremyTubongbanua thanks - the problem I was encountering was actually a different problem ( |
Awesome :) |
feat: add `-o StrictHostKeyChecking=accept-new` to the command that sshnp outputs
… execs ssh on the host
…`; `-o StrictHostKeyChecking` now set to `accept-new`, and removed `-o UserKnownHostsFile=/dev/null`
…ariable refactor: made the SSHNPDParams args tests a bit easier to read test: Add tests for --ssh-client option
…Impl classes, and their factories / constructors feat: implemented feature for new command-line flag --ssh-client so that it will use either the host's ssh command OR the pure dart SSHClient from dartssh2 package build: re-added dartssh2 and ssh-key dependencies
refactor: made the creation of the args easier to read and not possible to break by accidentally omitting a comma style: ran dart format
@cconstab Ready for final review |
- What I did
case 'sshpublickey'
in the notification handlerAtSignLogger.defaultLoggingHandler = AtSignLogger.stdErrLoggingHandler;
--ssh-client
which can currently have values '/usr/bin/ssh' (the default) or 'pure-dart'. If set topure-dart
then we'll do it using SSHClient as we used to.-o StrictHostKeyChecking=accept-new
to silently accept new, but still check existing-o IdentitiesOnly=yes
to use only the identity in the specified identity file