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

Session doesn't start on debian 11 if XAUTHORITY is set #2417

Closed
moobyfr opened this issue Nov 9, 2022 · 8 comments
Closed

Session doesn't start on debian 11 if XAUTHORITY is set #2417

moobyfr opened this issue Nov 9, 2022 · 8 comments

Comments

@moobyfr
Copy link
Contributor

moobyfr commented Nov 9, 2022

debian 11, set XAUTHORITY in /etc/profile to /tmp/.Xauth.$USER

session doesn't start . comment the XAUTHORITY -> session start

@matt335672
Copy link
Member

Hi @moobyfr

Has it ever worked like this? The code in sesman.session.c doesn't scan /etc/profile when it starts the X server, but as you say in #1069, setting this using pam_env should work.

I think the only way it could work is by using a login shell to start the X server.

@moobyfr
Copy link
Contributor Author

moobyfr commented Nov 9, 2022

I'm mostly sure to have tested the code with the usage of the variable, when I wrotten it:
If the variable isn't used, the default value is used i.e. $HOME/.Xauthority and the sessions must start.

But now the session doesn't start and this shouldn't happen.

@matt335672
Copy link
Member

matt335672 commented Nov 10, 2022

OK - I've had a look into this, and tried to reconstruct what has happened. Please check my reasoning and tell me what I've got wrong.

Your original PR to add XAUTHORITY support was #545 and this got merged for v0.9.1.

Looking at the code, the X server is started at these lines and following:-

xrdp/sesman/session.c

Lines 656 to 697 in 93c55e5

if (type == SESMAN_SESSION_TYPE_XVNC)
{
env_set_user(s->username,
&passwd_file,
display,
g_cfg->session_variables1,
g_cfg->session_variables2);
}
else
{
env_set_user(s->username,
0,
display,
g_cfg->session_variables1,
g_cfg->session_variables2);
}
g_snprintf(text, 255, "%d", g_cfg->sess.max_idle_time);
g_setenv("XRDP_SESMAN_MAX_IDLE_TIME", text, 1);
g_snprintf(text, 255, "%d", g_cfg->sess.max_disc_time);
g_setenv("XRDP_SESMAN_MAX_DISC_TIME", text, 1);
g_snprintf(text, 255, "%d", g_cfg->sess.kill_disconnected);
g_setenv("XRDP_SESMAN_KILL_DISCONNECTED", text, 1);
/* prepare the Xauthority stuff */
if (g_getenv("XAUTHORITY") != NULL)
{
g_snprintf(authfile, 255, "%s", g_getenv("XAUTHORITY"));
}
else
{
g_snprintf(authfile, 255, "%s", ".Xauthority");
}
/* Add the entry in XAUTHORITY file or exit if error */
if (add_xauth_cookie(display, authfile) != 0)
{
g_exit(1);
}
if (type == SESMAN_SESSION_TYPE_XORG)

env_set_user() is the function which switches the context to the user:-

xrdp/sesman/env.c

Lines 85 to 214 in 93c55e5

int DEFAULT_CC
env_set_user(const char *username, char **passwd_file, int display,
const struct list *env_names, const struct list *env_values)
{
int error;
int pw_uid;
int pw_gid;
int uid;
int index;
int len;
char *name;
char *value;
char *pw_shell;
char *pw_dir;
char text[256];
pw_shell = 0;
pw_dir = 0;
error = g_getuser_info(username, &pw_gid, &pw_uid, &pw_shell, &pw_dir, 0);
if (error == 0)
{
g_rm_temp_dir();
error = g_setgid(pw_gid);
if (error == 0)
{
error = g_initgroups(username, pw_gid);
}
if (error == 0)
{
uid = pw_uid;
error = g_setuid(uid);
}
g_mk_temp_dir(0);
if (error == 0)
{
g_clearenv();
g_setenv("SHELL", pw_shell, 1);
g_setenv("PATH", "/sbin:/bin:/usr/bin:/usr/local/bin", 1);
g_setenv("USER", username, 1);
g_sprintf(text, "%d", uid);
g_setenv("UID", text, 1);
g_setenv("HOME", pw_dir, 1);
g_set_current_dir(pw_dir);
g_sprintf(text, ":%d.0", display);
g_setenv("DISPLAY", text, 1);
g_setenv("XRDP_SESSION", "1", 1);
if ((env_names != 0) && (env_values != 0) &&
(env_names->count == env_values->count))
{
for (index = 0; index < env_names->count; index++)
{
name = (char *) list_get_item(env_names, index),
value = (char *) list_get_item(env_values, index),
g_setenv(name, value, 1);
}
}
if (passwd_file != 0)
{
if (0 == g_cfg->auth_file_path)
{
/* if no auth_file_path is set, then we go for
$HOME/.vnc/sesman_username_passwd:DISPLAY */
if (!g_directory_exist(".vnc"))
{
if (g_mkdir(".vnc") < 0)
{
log_message(LOG_LEVEL_ERROR,
"Error creating .vnc directory: %s",
g_get_strerror());
}
}
len = g_snprintf(NULL, 0, "%s/.vnc/sesman_%s_passwd:%d",
pw_dir, username, display);
*passwd_file = (char *) g_malloc(len + 1, 1);
if (*passwd_file != NULL)
{
/* Try legacy name first, remove if found */
g_sprintf(*passwd_file, "%s/.vnc/sesman_%s_passwd",
pw_dir, username);
if (g_file_exist(*passwd_file))
{
log_message(LOG_LEVEL_WARNING, "Removing insecure "
"password file %s", *passwd_file);
g_file_delete(*passwd_file);
}
g_sprintf(*passwd_file, "%s/.vnc/sesman_%s_passwd:%d",
pw_dir, username, display);
}
}
else
{
/* we use auth_file_path as requested */
len = g_snprintf(NULL, 0, g_cfg->auth_file_path, username);
*passwd_file = (char *) g_malloc(len + 1, 1);
if (*passwd_file != NULL)
{
g_sprintf(*passwd_file, g_cfg->auth_file_path, username);
}
}
if (*passwd_file != NULL)
{
LOG_DBG("pass file: %s", *passwd_file);
}
}
g_free(pw_dir);
g_free(pw_shell);
}
}
else
{
log_message(LOG_LEVEL_ERROR,
"error getting user info for user %s",
username);
}
return error;
}

In there the environment is obtained from some hard-coded variables, and also from any settings in g_cfg->session_variables1 and g_cfg->session_variables2. These are obtained from sesman.ini using config_read_session_variables().

Your code then checks for XAUTHORITY and creates the authorization file if appropriate before starting the X server. There doesn't seem to be any way that this value can be obtained from /etc/profile, but it could be obtained from sesman.ini.

Is that maybe what you're thinking of?

As I said it's fairly complex and I may not have spotted something. Please let me know what you think.

@matt335672
Copy link
Member

BTW I agree we should probably get this working in some way.

LightDM has a setting to either use the system default, or to generate a LightDM-specific file:-

https://github.com/canonical/lightdm/blob/de09f2c34175b44e12c4a49c87905812576f9661/src/session.c#L797-L816

From a quick look, that code runs as root so it can set up the directories(?). It then passes the filename over a pipe to the process which runs as the user so the file can be created in a user context.

@moobyfr
Copy link
Contributor Author

moobyfr commented Nov 10, 2022

I wasn't aware of the config_read_session_variables() function . But some systemd stuff seems to load the variables from /etc/profile: dbus-update-activation-environment
(from my .xsession-errors, some debug lines are printed:
dbus-update-activation-environment: setting XAUTHORITY=/adhome/e.blindauer/.Xauthority
dbus-update-activation-environment: setting XDG_SESSION_DESKTOP=lightdm-xsession

I prefer not to rely on root code to create some directories, mainly for security reason, no root, no problem :). And being root is no more sufficient for creating directories: with nfs4/krb5, even root cannot create directories in the $HOME of the user (and it's my setup at work). I prefer to rely on sysadmin to get a reliable storage for the Xauth file.

I'll have more time this weekend to debug this issue.

@matt335672
Copy link
Member

The system --user process gets started by pam_systemd.so. At that point I don't think XAUTHORITY is set.

On my Ubuntu22.04 system, the communication you refer to above happens in /etc/X11/Xsession.d/20dbus_xdg-runtime when the session is being started.

Hope that's useful.

It's clearly important for the X server, the session and chansrv to share the same setting!

We could do worse than emulate the LightDM functionality I think, and generate a file under /run or /var/run only accessible to the user. It's more secure than having a file available on persistent storage, and (potentially) over NFS for some configurations.

@matt335672
Copy link
Member

@moobyfr - Did you ever get anywhere with this?

I've just raised #3393 which I think will address most use-cases related to XAUTHORITY. I'd like to fold any changes we might need to make for this issue into that PR if possible.

@moobyfr
Copy link
Contributor Author

moobyfr commented Jan 13, 2025

Hello,
didn't get further on this one. (And seems can't reproduce it ever!)

With some background, the XAUTHORITY has to be managed by the program which start the X server, so lightdm, gdm, or xrdp for us.
#3393 is the way to go IMHO

@moobyfr moobyfr closed this as completed Jan 13, 2025
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

No branches or pull requests

2 participants