8000 Fixes SSH Remoting KeyFilePath Parameter by PaulHigin · Pull Request #4529 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]
Skip to content

Fixes SSH Remoting KeyFilePath Parameter #4529

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Aug 15, 2017
Merged

Fixes SSH Remoting KeyFilePath Parameter #4529

merged 2 commits into from
Aug 15, 2017

Conversation

PaulHigin
Copy link
Contributor

This change addresses issue #4475.

When creating the SSH child process for remoting the KeyFilePath parameter path was enclosed in double quotes to handle potential space characters in the file path. The problem was that the new SSH child process creation code was leaving the quote characters in the key filepath causing SSH to fail to find the key file. In addition KeyFilePath paths with space characters were not being handled correctly.

With this change the KeyFilePath parameter now works as expected:

PS > New-PSSession -HostName Machine1 -UserName User1 -KeyFilePath c:\users\User1\Keys\User1Key

PS > New-PSSession -HostName Machine2 -UserName User2 -KeyFilePath "c:\users\User2\My Keys\User2Key"

This worked correctly on Windows and only failed on Linux systems using the new SSH process creation code.

I also noticed and fixed a problem where trying to back out of a SSH remoting password prompt resulted in a hang (until the 1 minute protocol timeout occurred).

PS > New-PSSession -HostName Machine1 -UserName User1
User1@Machine1's password:  
Ctrl+C

The problem was that the client side connection did not end. Fix is to add CloseAsync override to close the connection.

@PaulHigin PaulHigin added WG-Remoting PSRP issues with any transport layer Issue-Bug Issue has been identified as a bug in the product OS-Linux OS-macOS labels Aug 8, 2017
@PaulHigin PaulHigin added this to the 6.0.0-HighPriority milestone Aug 8, 2017
@PaulHigin PaulHigin requested a review from mirichmo August 8, 2017 21:27
mirichmo
mirichmo reviewed Aug 9, 2017
View reviewed changes
src/System.Management.Automation/engine/remoting/common/RunspaceConnectionInfo.cs
case '"':
// Special case for arguments within quotes
// Just return argument value within the quotes
while ((++i < argsLength) && argsToParse[i] != '"') { };
Copy link
Member

Choose a reason for hiding this comment

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

Is there enforcement for the string ending in a '"'? Does that happen in a different layer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No enforcement here. This expects the argument string to be correctly formatted. If it is not then the argument list is wrong and any errors are generated when the process is created.

This is currently scoped internally so we currently have complete control in how it is used. However, I did intend this to be robust against malformed strings (empty, single quote, unmatched quotes, single space, only spaces, etc.). If you see something I missed please let me know.

Copy link
Member

Choose a reason for hiding this comment

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

My initial concern regarded unmatched double quotes. Thinking about it some more, two other things came to mind:

  1. Strings that use single quotes
  2. Argument lists that include other commands. Do we do validation on it? I'm wondering about something like this: arg1 arg2; rm -rf *

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Single quotes are not supported and not needed. It is something we could add if we need it in the future. Argument validation is not intended to be performed here. This just parses space delimited strings with a special case for double quoted arguments.

I just tried to make sure it doesn't blow up with malformed strings and cause an access violation or null reference exception.

@PaulHigin
Copy link
Contributor Author

@adityapatwardhan
Can this be merged? Thanks.

@adityapatwardhan
Copy link
Member

@PaulHigin Can we add tests for this?

@PaulHigin
Copy link
Contributor Author

#3904

@adityapatwardhan adityapatwardhan merged commit 6b8d86f into PowerShell:master Aug 15, 2017
@PaulHigin
Copy link
Contributor Author

Thanks!

@PaulHigin PaulHigin mentioned this pull request Aug 15, 2017
Closed
@PaulHigin PaulHigin deleted the Fix-KeyFilePath-Parameter branch August 28, 2017 23:11
@Hema-dell
Copy link
Hema-dell commented Jan 13, 2020 •
edited by unfurl-links bot
Loading

HI ,
I have installed PoSH6 and 7 on Windows 10 box, and PoSh6 on linux box, trying to do remote throush PsSession using -keyfilepath, followed instructions for ssh as provided in https://docs.microsoft.com/en-us/windows-server/administration/openssh/openssh_install_firstuse, after following the instructions and providing this command : New-PSSession -HostName root@x.x.x.x:22 -KeyFilePath C:\Users\dell.ssh\authorized_keys i get prompt for password , i have copied authorized_keys into ./ssh folder of linux box too!!! why is it prompting for password?

@PaulHigin
Copy link
Contributor Author
PaulHigin commented Jan 13, 2020 •
edited by unfurl-links bot
Loading

@Hema-dell It sounds like your sshd_config configuration file still allows password authentication and should be disabled. FYI, I have a module with 'Enable-SSHRemomting' cmdlet that I have been working on. It is not been published by you can experiment with it if you like:
https://gist.github.com/PaulHigin/b12b552b12dc4e8c940efe4f402e5362

We decided not to touch general SSH settings, and assume that the user has it set up as needed. The 'Enable-SSHRemoting' cmdlet merely updates the sshd_config file with subsytem entry that becomes a PowerShell SSH remoting end point, so that the machine can receive connections.

Gist
PSRemotingTools module. GitHub Gist: instantly share code, notes, and snippets.

@Hema-dell
Copy link

Hi Paul!!,
I tried the above steps as shared in the link by you ,however i am still unable to do pssession using -keyfilepath parameter!!!!, Need your help on this!!!!.

Thanks,
Hema-dell

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mirichmo mirichmo mirichmo approved these changes

Assignees

@adityapatwardhan adityapatwardhan

Labels
Issue-Bug Issue has been identified as a bug in the product OS-Linux OS-macOS WG-Remoting PSRP issues with any transport layer
Projects
None yet
Milestone
6.0.0-GA
Development

Successfully merging this pull request may close these issues.
5 participants
@PaulHigin @adityapatwardhan @Hema-dell @mirichmo @msftclas
0