Add ability to skip login window / auto login

[GermanDarknes]

I wanted to add the ability to not have to click the OK button if there is only a single session and the username/password is already set in xrdp.ini. This is my concept of how this could be achieved.

Currently, it is possible to set the username and password in xrdp.ini. However, automatic login only functions when the client sends credentials. Even if the client's credentials are incorrect, the session logs in based on the credentials in the .ini file. With the skiploginwindow=True variable, this behavior can now be achieved without the need to send false credentials to trigger the if-condition.

Currently, if skiploginwindow=True and the credentials in the .ini are wrong, the error-log-window loops without showing the login window. So implementing this as a session variable would be a good way to prevent this, if the auto login fails you will see the login window. This could be done by using the skiploginwindow config variable to set rdp_autologin to 1 when creating the session, but I have not found a good place for this logic yet.

[matt335672]

@GermanDarknes - thanks for that.

There seems to be some fundamental security problems with this:-

1. How do you know the user connecting is the correct user? There's no validation at all of the incoming connection.
2. You're storing a password in xrdp.ini which is world readable on the entire machine.

Isn't more secure to simply send the credentials from the client? I'm failing to see a good use-case here.

[GermanDarknes]

The use case here is that it allows skipping the 'OK' window (see images). Everything else is already in the code, and I'm simply utilizing these features.

The auto login functionality already exists and can be triggered by sending credentials, even incorrect ones.

xrdp/xrdp/xrdp_wm.c

Line 639 in 76d12c5

if (self->session->client_info->rdp_autologin)

Regarding the first question: Anyone can connect. With the existing settings, I can already set up a guest session, where a user doesn't need to input a username and password. With this new toggle, they can simply skip the window, avoiding the login screen with a combo box that contains only one entry. This is now skipped and the user is directed straight to the desktop.

Regarding the second question: The functionality to store a password in the .ini file already exists and it's not a new security issue from this PR. In the context of using this functionality for guest accounts, the username and password are intentionally public and the account has restrictions in place.

.ini defined login data always suppresses the sent ones.

xrdp/xrdp/xrdp_wm.c

Lines 720 to 730 in 76d12c5

	if (g_strncasecmp("password", q, 255) == 0)
	{
	/* if the password has been asked for by the module, use what the
	client says.
	if the password has been manually set in the config, use that
	instead of what the client says. */
	if (g_strncmp("ask", r, 3) == 0)
	{
	r = self->session->client_info->password;
	}
	}

xrdp/xrdp/xrdp_wm.c

Lines 731 to 741 in 76d12c5

	else if (g_strncasecmp("username", q, 255) == 0)
	{
	/* if the username has been asked for by the module, use what the
	client says.
	if the username has been manually set in the config, use that
	instead of what the client says. */
	if (g_strncmp("ask", r, 3) == 0)
	{
	r = self->session->client_info->username;
	}
	}

In summary, the new variable leverages an existing functionality, eliminating the need to send fake credentials or pressing the ok button.

[matt335672]

I accept that this is based on existing functionality.

The main use-case for this field in my opinion is not logging in to the local machine - it's providing a password for a VNC service running elsewhere on the network. In this instance, PAM can be used to authenticate the user logging in to xrdp.

Even the implementation of this use-case should be updated. It's very much at odds with the recommendations in ISO 27001/2 and (in the UK at least) with NCSC guidelines. From what I can tell, the BSI in Germany is saying very similar things in ORP.4 Identity and Access Management (link to compendium in English here)

Put more simply, we're already providing insufficient protections around this field and I don't want to make the existing bad behaviour any worse than it is. This field should not be world-readable and should be encrypted at rest. If it was I'd be happy to accept changes in this area, but that's obviously easier said than done.

The password can be stored on Windows side, where it is encrypted and machine-specific. Does this not meet your immediate use-case?

[metalefty]

I overall agree with @matt335672 regarding this PR.

[GermanDarknes]

Storing the password on the Windows side is possible and works for my use case. The autologin was an idea that had been floating around. As for the implementation, I believe further investigation for a better solution is not necessary, perhaps when password storage is revised. Nevertheless, I appreciate your input and all the considerations.