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

Relax implicit ASKPASS requirements on Windows in X11 forwarding scenarios #428

Merged
merged 1 commit into from
Feb 28, 2020

Conversation

riverar
Copy link

@riverar riverar commented Feb 14, 2020

For historical reasons, if environment variable DISPLAY exists, read_passphrase will force users down a path that requires use of an askpass implementation specified via SSH_ASKPASS (or internal defaults). On Windows, where askpass generally won't exist, this will result in complete login failure.

There is on-going discussion--18 years and going--about refactoring read_passphrase to eliminate this behavior and introduce controls to give users explicit control over SSH_ASKPASS behavior.

The following constraints were assumed:

  • Don't change non-Windows behavior at all
  • Keep compatibility with SSH_ASKPASS users on Windows
  • Minimize diff to maintain attractiveness for upstream consideration

(fixes PowerShell/Win32-OpenSSH#1515)

@riverar
Copy link
Author

riverar commented Feb 14, 2020

@bagajjal Can I get an assist here? These test failures seem unrelated to my changes. The tests pass locally too.

@riverar
Copy link
Author

riverar commented Feb 20, 2020

@riverar
Copy link
Author

riverar commented Feb 25, 2020

Ping? Would be a real shame if this didn't make it into Windows vNext.

@bagajjal
Copy link

@riverar - We are seeing random failures specific to proxycommand. I have disabled them for now. Please sync again and do a push with minor change (adding a space in a file) so the AppVeyor build will start again.

Do you know what is the default askpass (_PATH_SSH_ASKPASS_DEFAULT) used in Ubuntu? They don't have "/usr/X11R6/bin/ssh-askpass" on Ubuntu.

@riverar riverar force-pushed the change/relax-askpass-x11 branch from 48919b4 to 911105a Compare February 25, 2020 06:54
@riverar
Copy link
Author

riverar commented Feb 25, 2020

@bagajjal Thanks, triggered a new build. I pulled down the sources for openssh on Ubuntu Bionic and it has #define _PATH_SSH_ASKPASS_DEFAULT "/usr/X11R6/bin/ssh-askpass" as well.

Installing ssh-askpass results in:

update-alternatives: using /usr/lib/ssh/x11-ssh-askpass to provide /usr/bin/ssh-askpass
(ssh-askpass) in auto mode

Going to assume PATH_SSH_ASKPASS_DEFAULT just, errrrr, fails, on most platforms today. 😂 I'm not surprised, given this piece of code has been contested for 18+ years.

@@ -183,10 +183,19 @@ read_passphrase(const char *prompt, int flags)
askpass = getenv(SSH_ASKPASS_ENV);
else
askpass = _PATH_SSH_ASKPASS_DEFAULT;

#ifdef WINDOWS

Choose a reason for hiding this comment

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

DISPLAY environment isn't applicable for windows?
Change the logic to have line 188 to197 for windows.
Disable and don't touch the existing code for windows.

This makes the users not to worry about additional DISPLAY environment variable which is not applicable for windows.

Copy link
Author

@riverar riverar Feb 26, 2020

Choose a reason for hiding this comment

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

DISPLAY use is required on the source side for X11 forwarding to work. It points to an X Display Server of the user's choosing (e.g. DISPLAY=localhost:0.0). XMing is a popular X Display Server that runs on Windows.

Basically, ssh connects to the configured X Display Server and tunnels traffic to/from the target. sshd on the target side will automatically set up DISPLAY for application use.

So the commit only introduces one change: On Windows, it stops forcing the user to use ASKPASS if SSH_ASKPASS is not defined. If it is, business as usual. No impact to non-Windows platforms.

Am I missing something?

Choose a reason for hiding this comment

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

I am not familiar with the x11 forwarding. Could you please share the steps on both the client and server so I can test?

Copy link
Author

@riverar riverar Feb 26, 2020

Choose a reason for hiding this comment

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

Absolutely, happy to share. It's way easier than it sounds too.

Try this:

  1. On Windows, install XMing
    • Chose to display programs in Multiple windows and keep the Display Number at -1.
    • Keep all other defaults
  2. Start the display server, if not already running (XLaunch in Start). This will install an icon in your notification area (tray).
  3. In a PowerShell window, issue the following commands (replacing text as needed):
    $env:DISPLAY="localhost:0.0"
    ssh -Y user@host
    
  4. On the target (assumed Linux with working package manager), install xorg-x11-apps package.
  5. On the target, start xclock and observe window appears on Windows desktop!

Copy link

@bagajjal bagajjal Feb 27, 2020

Choose a reason for hiding this comment

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

Thanks. It worked.
I was puzzled how the request is forwarded to xming server when we use ssh -Y user@host.
ssh client logs shows there is no xauth program but still we are able to forward the X data to xming.

debug1: No xauth program.
Warning: No xauth data; using fake authentication data for X11 forwarding.
debug1: Requesting X11 forwarding with authentication spoofing.
debug2: channel 0: request x11-req confirm 1

Copy link
Author

Choose a reason for hiding this comment

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

Great, so now to reproduce the issue this PR fixes:

  1. Ensure C:\dev\tty doesn't exist
  2. Ensure DISPLAY is set, then ssh -Y user@host
  3. Observe authentication failure

The failure is a result of ssh passing the check at line 181 (use_askpass is 1 because we don't have a tty), then entering a block of code that assumes ASKPASS is present, which fails.

Choose a reason for hiding this comment

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

I got the need for the fix.

I didn't understand how the data is forwarded as we didn't specify the xauth program when we do ssh -Y user@host.
debug1: No xauth program.
Warning: No xauth data; using fake authentication data for X11 forwarding.
debug1: Requesting X11 forwarding with authentication spoofing.
debug2: channel 0: request x11-req confirm 1

Also one observation, GUI doesn't display colors on windows side.

Copy link
Author

Choose a reason for hiding this comment

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

Should see colors, try xclock -bg pink.

@bagajjal bagajjal merged commit 5207e6f into PowerShell:latestw_all Feb 28, 2020
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.

Support X11 Forwarding
2 participants