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

Add support for multiplexing for NativeSsh #918

Merged
merged 2 commits into from
Jan 19, 2017

Conversation

kszymukowicz
Copy link
Contributor

@kszymukowicz kszymukowicz commented Dec 3, 2016

Q A
Bug fix? No
New feature? Yes
BC breaks? No
Deprecations? No
Fixed tickets N/A

Implementing ssh multiplexing speeds up NativeSsh extremely. Its faster than other methods.
https://en.wikibooks.org/wiki/OpenSSH/Cookbook/Multiplexing

How to test? In your deploy.php file put:

   set('ssh_type', 'native');
   set('ssh_type_native_mux', true);

You should observe considerable increase of speed. When using mux the ssh authentication is done only once.

@kszymukowicz kszymukowicz changed the title Add support for multiplexing for NativeSsh. Add support for multiplexing for NativeSsh Dec 3, 2016
@antonmedv
Copy link
Member

Awesome stuff!

@antonmedv
Copy link
Member

Maybe it's better to turn on it by default?

@kszymukowicz
Copy link
Contributor Author

Tnx elfet! I was also amazed by the speed results :)

For "default". In my understanding multiplexing can be received as unsafe for some folks because it creates linux socket that can be then accessed without any authorization. The mux in the proposed implementation creates socket for time of deploy (+5 sec). So in the time of deploying someone who has access to our local machine from which we do deploy can use the socket to gain access to remote server.

The judgment if this is safe enough or not is probably individual perception.

So if this would be done by default then documentation must, imho, clearly state the fact that:
a) multiplexing is used by default,
b) multiplexing brings some security problems,
c) multiplexing can be deactivated with set('ssh_type_native_mux', false);

Other than that I am for setting mux as default :)

@antonmedv
Copy link
Member

So socket can be accessed during deployment time? Then deploying from local machine there may be no security issue as keys for example available for everybody.

I'd like to make it by default (btw, what about Windows?) as a lot of users does not change default, so this can bring a speedup in deployment time. It will be good if we can find proper solution for this.

@kszymukowicz
Copy link
Contributor Author

Yes. Socket can be accessed during deployment time.
Keys can be additionally protected with passphrase and then the mux is less secure.

For windows. Well - I do not know and I will have not way to test because I am on mac.

@kszymukowicz
Copy link
Contributor Author

hi @Elfet
is there anything I can do to push this forward ? Do you want to change this PR somehow? Plz tell me - I can make some changes.

@antonmedv
Copy link
Member

I'd like to test in on mac, linux and windows to make in default by default. Or write better docs with explanation of how tu turn in on.

/**
* Init multiplexing with exec() command
*
* Background: Symfony Process hangs on creating multiplex connection
Copy link
Member

Choose a reason for hiding this comment

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

Why symfony process does not working here?

Copy link
Contributor Author

@kszymukowicz kszymukowicz Dec 11, 2016

Choose a reason for hiding this comment

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

Sry but I could not debug the problem.
I can only describe you the behaviour. When ssh command was run with multiplexing options then using the Symfony Process whole task just hungs but multiplexing socket was created in background. If I canceled such hanged process with "ctrl+c" and run again then Symfony Process was using multiplexing socket and all was working well.

@antonmedv antonmedv merged commit 48abf05 into deployphp:master Jan 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants