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.

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

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

Merged
merged 1 commit into from
Feb 28, 2020
Merged

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions readpass.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,19 @@ read_passphrase(const char *prompt, int flags)
askpass = getenv(SSH_ASKPASS_ENV);
else
askpass = _PATH_SSH_ASKPASS_DEFAULT;

#ifdef WINDOWS
Copy link

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 •
edited
Loading

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?

Copy link

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 •
edited
Loading

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 •
edited
Loading

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.

Copy link

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.

if (getenv(SSH_ASKPASS_ENV)) {
#endif

if ((ret = ssh_askpass(askpass, prompt)) == NULL)
if (!(flags & RP_ALLOW_EOF))
return xstrdup("");
return ret;

#ifdef WINDOWS
}
#endif
}

if (readpassphrase(prompt, buf, sizeof buf, rppflags) == NULL) {
Expand Down