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

Password variable --auth=env:name is preserved in Xvfb-for-Xpra-3 process #4252

Closed
cherio opened this issue Jun 13, 2024 · 5 comments
Closed
Labels
enhancement New feature or request

Comments

@cherio
Copy link

cherio commented Jun 13, 2024

Describe the bug
xpra server clears all its environment (probably for security reasons) but not before it launches Xvfb-for-Xpra-3 process. The variable containing authentication password is clearly visible in the process environment.

To Reproduce
Steps to reproduce the behavior:

  1. server command
systemd-run --user -q --unit usr-xpra-service-name \
	--property=ExecStop="/usr/bin/xpra stop :$xpra_disp_num" \
	--property=StandardOutput=append:$log_file \
	--property=StandardError=append:$log_file \
	--property=EnvironmentFile="$xpra_env_file" \
	--property=Environment="xpra_pass=$xpra_pass" \
	--collect \
	/usr/bin/xpra start :$xpra_disp_num --bind-tcp=:$xpra_srv_port --ssl=off \
	--start=/usr/bin/thunar --start=/usr/bin/xfce4-appfinder \
	--pulseaudio=no --speaker=off \
	--mdns=no \
	--dbus-launch=no \
	--auth=env:name=xpra_pass \
	--daemon=no

System Information (please complete the following information):

  • Server OS:
$ uname -a
Linux hostname 6.5.0-1022-azure #23~22.04.1-Ubuntu SMP Thu May  9 17:59:24 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux

  • Xpra Server Version:
$ xpra --version
xpra v6.1-r35816 (gefa5e2cf65) beta
@cherio cherio added the bug Something isn't working label Jun 13, 2024
@totaam
Copy link
Collaborator

totaam commented Jun 13, 2024

It's not easy to decide which variables should be sanitized and which ones should not be.
The safe approach is to only use a whitelist, but this is bound to break something, somewhere.
I would recommend using a different authentication module instead.
Otherwise, please submit a PR.

@totaam totaam added enhancement New feature or request and removed bug Something isn't working labels Jun 13, 2024
@cherio
Copy link
Author

cherio commented Jun 13, 2024

I may be missing something but wouldn't it be safe to assume that the variable whose name referenced as --auth=env:name=VAR should be removed from the environment?

@totaam
Copy link
Collaborator

totaam commented Jun 13, 2024

I'm not going to do this sorts of parsing gymnastics, sorry.

@cherio
Copy link
Author

cherio commented Jun 15, 2024

I was just responding to

It's not easy to decide which variables should be sanitized

simply pointing to the fact that the variable name that holds authentication password is readily available, as it is explicitly specified. I realize that because I am not familiar with the code, what seems to be obvious and simple on the surface, may not really be that straightforward.

@totaam
Copy link
Collaborator

totaam commented Jun 16, 2024

@cherio the vfb code knows nothing about authentication modules, and the authentication modules know nothing about the command line.

Please try e1b5660

@totaam totaam closed this as completed Jun 16, 2024
totaam added a commit that referenced this issue Jun 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants