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

Startup interface to Xorg backend needs improvement #1816

Open
matt335672 opened this issue Mar 2, 2021 · 6 comments
Open

Startup interface to Xorg backend needs improvement #1816

matt335672 opened this issue Mar 2, 2021 · 6 comments

Comments

@matt335672
Copy link
Member

The Xorg backend is, for many users, the preferred backend to use in a variety of situations. However, the startup sequence for this backend is relatively fragile and could do with some improvement.

Currently we get a fair few fault reports mentioning a "blue screen" (e.g. #1798, #1794, #1722, and others) or "black screen" (e.g. #1764, #1743, #1723, #1674 and others). No useful information is presented to the user, and so resolution of these situations often requires investigation by a knowledgeable developer via a github issue. Better error reporting at session startup time may reduce the number of these reports.

Common causes of "blue screen" faults are:-

  1. The Xorg driver not starting, and xrdp timing out while trying to connect. During this process, no feedback is displayed to the user (i.e. the log window).
  2. The Xorg driver starts, the session immediately fails and Xorg dies before the initial connection is achieve by xrdp, User experience is identical to 1.

Black screen faults are less common. These occur when the Xorg server starts and connection completes satisfactorily, but for some reason the X server or the session does not function correctly.

This issue proposes the following changes:-

  • During the connect phase, the log window should be presented to the user, along with some more useful hints in the event of the connection failing.
  • In lib_mod_connect() in xup.c, a positive ack from the Xorg server, maybe containing the compiled in CLIENT_INFO_CURRENT_VERSION(see Add versioning to xrdp_client_info #1813) gives us confidence the X server has actually started before removing the log window.

These won't fix all of our connection problems by any means, but I think it's a start in the right direction.

@metalefty
Copy link
Member

This is a very good idea. The "blue screen" is a common point new xrdp users stuck.

@EmperorArthur
Copy link

As a user who's faced issues, having feedback on what went wrong would have been extremely helpful.

@danielperna84
Copy link

danielperna84 commented Jun 10, 2021

Not sure how much this is related, but I would love to see the pam error (like PAM_CRED_INSUFFICIENT or PAM_SYSTEM_ERR) that was raised during an authentication failure. Ideally the user should see the error code as well (in the login-UI where we usually get the login failed for display 0 message) so the user can easily report the issue.

@matt335672
Copy link
Member Author

Hi @danielperna84

It isn't directly related to the issue above, but it's an interesting discussion point

It would be reasonably simple to broaden the existing interfaces to return a PAM error code or similar for authentication and/or authorization, but this may not be the entirety of the error(s) actually logged by PAM, as additional messages can be logged by modules during the course of the PAM transaction. And in fact, it's probably not a good idea from a security perspective, as these codes are not really intended for consumption by unauthorized entities.

For example, the pam_unix(8) module can return PAM_USER_UNKNOWN which is definitely not something we should be reporting on a generic user interface as it allows an attacker to work out what the user names might be on a system, or even whether some daemon accounts with privileges exist.

At the moment, any PAM error(s) raised during authentication will be logged to the sesman log and/or syslog which can of course be read by system managers who are authorized to read such info.

Fell free to come back to me on this if I'm missing something.

@danielperna84
Copy link

Ok, I thought because the mentioned error popup happens in the context where the blue/black screens happen, such error messages were relevant here as well. I didn't want to open an issue for such a minor feature request. 😇

Anyways, I totally agree that exposing too much information is bad in terms of security. As a matter of fact, I'm using my own PAM written with pam-python, and there I'm already logging the interesting / debug messages. So with your statement about the security in mind, my feature request isn't all that necessary. As I'm very precise with those error-codes in my PAM, it seemed useful to me, to be able to know which error happened straight away.

But I would still see a benefit if there was a dedicated "Authentication error" message, even without the specific error code. That way the user would at least know it's (probably) his fault (invalid credentials etc.) and not an issue with xrdp itself. At the very least it would make clear at which stage of the login-process the error is happening.

@matt335672
Copy link
Member Author

Thanks for coming back to me @danielperna84 - I agree with you that something should be possible here.

I've started a discussion (#1921). We'll see where it goes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants