Allow for alternative user config locations, deprecate ~/.vnc in favour of XDG Base Directory Specification paths

[62832]

This PR aims to fix #1195 by allowing for alternative locations for different TigerVNC configuration files, both for the client and the server, primarily keeping the XDG Base Directory Specification in mind. In cases where ~/.vnc already exists, both the client and server will continue to use this path to store everything while warning the user that this path is to be deprecated in favour of XDGBDS-compliant paths. Otherwise, TigerVNC will begin to use $XDG_CONFIG_HOME/tigervnc for configuration, $XDG_DATA_HOME/tigervnc for X.509 known hosts data and $XDG_STATE_HOME/tigervnc for history/logs.

At present, the following changes have been implemented:

• VNC Viewer will save the default.tigervnc preferences file under the new config directory and continue using it from there.
• VNC Viewer's history file will be saved under and used from the new state directory.
• VNC Viewer will look for X.509 certificate data and revocations under the new config directory.
• VNC Viewer will keep X.509 known hosts data under the new data directory.
• vncpasswd will generate password files under the new config directory.
• The vncserver script will allow for vncserver-config-defaults to specify an alternative config file location via the userconfig option. If this option is not provided, vncserver will read config under the new config directory by default.
• vncsession will allow for an alternative directory for written logs to be specified via the -l path/to/log/dir option, and by default write log files to the new state directory if this is not specified.
• vncserver will now properly create $XAUTHORITY with the path specified by the auth config option if this is not present, instead of only creating ~/.Xauthority.

Struck-through parts of the list are to be done in separate PRs later on.

[CendioOssman]

This looks really nice! Great work!

[62832]

I just had an epiphany. Considering that vncserver does in fact manage to read the given user's XDG_CONFIG_HOME variable, presumably due to the child environment set up through PAM, would vncsession also then be able to read the value of XDG_STATE_HOME? If so, that makes things significantly easier for the rest of this PR.

LOL, bullshit.

[62832]

@CendioOssman One thing that I'm slightly unsure about is how to handle the legacy path situation for the Java version of VNC Viewer, since I notice that it erroneously also used .vnc on Windows rather than vnc under %APPDATA%. Would I have to consider migrating this on Windows to %APPDATA%/vnc as well for consistency's sake, or...?

[bphinz]

If you don’t mind, that would be great Sent from Gmail Mobile

…

On Mon, Mar 18, 2024 at 12:44 PM 90 ***@***.***> wrote: @CendioOssman <https://github.com/CendioOssman> One thing that I'm slightly unsure about is how to handle the legacy path situation for the Java version of VNC Viewer, since I notice that it erroneously also used .vnc on Windows rather than vnc under %APPDATA%. Would I have to consider migrating this on Windows to %APPDATA%/vnc as well for consistency's sake, or...? — Reply to this email directly, view it on GitHub <#1737 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB45M3MYDPB7RRYME4PCV33YY4KWFAVCNFSM6AAAAABEYXRLQ6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMBUGQZDENRUGQ> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[62832]

What I may do on Windows is simply move the contents of .vnc to vnc altogether in that case, since Windows still has primarily only one directory to deal with. Otherwise, the warning and deprecation will continue on Unices due to the more involved split between different kinds of files.

[62832]

I'm unable to test very easily, though I believe the new paths should now also be implemented for the Java version, along with some automatic movement and cleanup on Windows since this was for whatever reason using %USERPROFILE%\.vnc prior.

@CendioOssman Please let me know if the current layout is alright with you now before I proceed with fixing up documentation and the like, as I'm still not entirely sure whether I made the most sound choice keeping the X.509 CA cert within config, rather than data like the CRL and known hosts database.

EDIT: Come to think of it, might it not be useful to keep all the rest of the X.509 cert data within configs for the sake of version-controlled deployment on other machines?

[62832]

I'm not sure why I duped myself earlier into thinking that the vncsession process, and hence vncserver, would be able to read things like XDG_CONFIG_HOME from another user's login environment, so unless there's something else I'm not aware of I don't think checking for those variables within either of these server processes will do anything.

In the meantime, I figured it might be worth relaying some email correspondence I received from the original poster of issue #1195, raising some points worth addressing.

Without having thought too hard about it, I would be inclined to say that wherever the VNC passwd file would go is a suitable place for [X.509] certs, too. Although certs are not generally edited by the user after creation, they are generally created by the user, and I would account them more config-like than data-like. Also, certs have security considerations similar to those of the passwd file.

Please note also, however, that I never asked for ~/.vnc to be deprecated, only that it be made configurable, and as a secondary request that the destination for log files be made separately configurable. I'm not personally acquainted with any of the VNC dev team, but I would be inclined to guess that deprecating use of ~/.vnc or replacing it with a different default might be a step too far for them.

Based on the response, I gather the following:

• Much like the initial proposal to allow configuring the directory where VNC session logs are stored beyond simply setting $XDG_STATE_HOME, it seems perfectly fine to also still allow further configuring the location of the server config file as it was done earlier.
• There may be cases in which users may wish to retain backwards compatibility with the legacy location for the sake of inter-interoperability with other VNC implementations which also use this common longer-standing path. Therefore, it would probably be unwise to outright deprecate the legacy path and force paths that are implied to be exclusive to this one implementation.

[CendioOssman]

It's probably best to have all CMake generated files in the same place.

[CendioOssman]

I'm unable to test very easily, though I believe the new paths should now also be implemented for the Java version, along with some automatic movement and cleanup on Windows since this was for whatever reason using %USERPROFILE%\.vnc prior.

@bphinz, can you check if this looks good?

@CendioOssman Please let me know if the current layout is alright with you now before I proceed with fixing up documentation and the like, as I'm still not entirely sure whether I made the most sound choice keeping the X.509 CA cert within config, rather than data like the CRL and known hosts database.

The CA cert and CRL are both read-only stuff provided by the user (from TigerVNC's point of view), so I think that "config" is appropriate.

EDIT: Come to think of it, might it not be useful to keep all the rest of the X.509 cert data within configs for the sake of version-controlled deployment on other machines?

Are you referring to the known hosts file? Users should only interact with that using the UI, not modifying the file directly. So that makes it more "database" than "config" IMO.

I'm not sure why I duped myself earlier into thinking that the vncsession process, and hence vncserver, would be able to read things like XDG_CONFIG_HOME from another user's login environment, so unless there's something else I'm not aware of I don't think checking for those variables within either of these server processes will do anything.

vncsession is run as root, so it won't see the user's XDG variables. vncserver runs as the user, but starting the user's shell is done much later. So it won't see things like ~/.bash_profile. That is considered the "old" way, though, and user environment overrides should be set by systemd before vncserver executes.

All of this is not unique to TigerVNC. It should work the same way as e.g. gdm or lightdm. So it could be useful to see how those work when it comes to respecting the users' settings.

• There may be cases in which users may wish to retain backwards compatibility with the legacy location for the sake of inter-interoperability with other VNC implementations which also use this common longer-standing path. Therefore, it would probably be unwise to outright deprecate the legacy path and force paths that are implied to be exclusive to this one implementation.

Any such interoperability is accidental, and not per design. There has not been any coordination on these files and formats. So I think it is a good thing that we are moving to TigerVNC specific directories to clearly signal that these are TigerVNC specific files.

[62832]

Are you referring to the known hosts file? Users should only interact with that using the UI, not modifying the file directly. So that makes it more "database" than "config" IMO.

I assumed that "known hosts" would be something that could be worth copying over on multiple machines like the rest of the cert data, just as something of an integrity check, and hence worth considering as config rather than just data even if it isn't necessarily meant for user editing.

vncsession is run as root, so it won't see the user's XDG variables. vncserver runs as the user, but starting the user's shell is done much later. So it won't see things like ~/.bash_profile. That is considered the "old" way, though, and user environment overrides should be set by systemd before vncserver executes.

Do you mean by explicitly setting these variables through systemd before running vncsession? I personally use non-systemd distros, but I believe those also should have the facilities to set such variables if you do mean explicitly setting them prior to running the service. Otherwise, I'd be glad to know what mechanism you're referring to for this.

Any such interoperability is accidental, and not per design. There has not been any coordination on these files and formats. So I think it is a good thing that we are moving to TigerVNC specific directories to clearly signal that these are TigerVNC specific files.

I take it this means then that ~/.vnc should eventually go altogether? If so, I'll bring back the deprecation warnings.

[CendioOssman]

I assumed that "known hosts" would be something that could be worth copying over on multiple machines like the rest of the cert data, just as something of an integrity check, and hence worth considering as config rather than just data even if it isn't necessarily meant for user editing.

I don't see a conflict with that? "data" is still something you want to keep, so backups and synchronisation is still on the table.

vncsession is run as root, so it won't see the user's XDG variables. vncserver runs as the user, but starting the user's shell is done much later. So it won't see things like ~/.bash_profile. That is considered the "old" way, though, and user environment overrides should be set by systemd before vncserver executes.

Do you mean by explicitly setting these variables through systemd before running vncsession? I personally use non-systemd distros, but I believe those also should have the facilities to set such variables if you do mean explicitly setting them prior to running the service. Otherwise, I'd be glad to know what mechanism you're referring to for this.

I dug around in gdm, and it is interested in $XDG_CACHE_HOME for storing the log (they haven't noticed the new state directory, it seems). It only seems to grab the environment from PAM, so it does not look at things like ~/.profile. For systemd-systems, you should be able to get things in there using systemd's "environment.d" mechanism. Not sure what you'd need to on non-systemd systemd. pam_env?

I take it this means then that ~/.vnc should eventually go altogether? If so, I'll bring back the deprecation warnings.

Yes, that thing should ideally be removed. It is not as interoperable as the name suggests, and dot files should all be moved in to the XDG directories in some form.

[62832]

To preface, I'm very much a novice when it comes to working with things like PAM and passing environment variables using it, hence I would like to clarify these points in particular.

I dug around in gdm, and it is interested in $XDG_CACHE_HOME for storing the log (they haven't noticed the new state directory, it seems). It only seems to grab the environment from PAM, so it does not look at things like ~/.profile. For systemd-systems, you should be able to get things in there using systemd's "environment.d" mechanism. Not sure what you'd need to on non-systemd systemd. pam_env?

If I'm not mistaken, isn't environment.d supposed to apply to user instances of systemd? Nothing I've read implies that the system instance of systemd would be able to gather these variables from a user's environment, so it's still beyond me how gdm-session-worker would be able to (presumably) run as root and still read a user's $XDG_CACHE_HOME.

In any case, I've been playing around a bit with using pam_env (particularly setting variables in /etc/security/pam_env.conf) and I've still got some issues when it comes to it. I don't expect to get any specific support for these in the slightest, but it would be interesting to know what might be at play here.

• When run as a service with my system's init (in this case, Dinit), vncsession seemingly ignores any variables set within /etc/security/pam_env.conf, always falling back to ~/.local/state/tigervnc for logs. Presumably, pam_env doesn't even get invoked here?
• When directly invoking vncsession using sudo as a logged-in user, pam_env.conf does get read, but the process immediately and silently exits if a variable happens to involve the @{HOME} variable for substitution. I have not been able to find any sort of log or stack trace to indicate what's causing this.

Should you require any more information about this, I'm happy to provide it and potentially coordinate with the maintainers of my init system as well. Otherwise, I think I've addressed everything else for this PR now, so I can probably proceed with documentation changes and then mark it as ready.

[CendioOssman]

To preface, I'm very much a novice when it comes to working with things like PAM and passing environment variables using it, hence I would like to clarify these points in particular.

I'm familiar with PAM, but pam_env is new to me as well. So let's take it one step at a time. Moving to XDG directories and variables is likely useful no matter the final solution, so let's try to get that in place. :)

If I'm not mistaken, isn't environment.d supposed to apply to user instances of systemd? Nothing I've read implies that the system instance of systemd would be able to gather these variables from a user's environment, so it's still beyond me how gdm-session-worker would be able to (presumably) run as root and still read a user's $XDG_CACHE_HOME.

It's sufficient that these things are in place once we've switched to the real user. We don't need them whilst we are still root. Which should simplify things.

But looking again at gdm, I think they might have overlooked this in handling environment.d. Those variables don't seem to be set by pam_systemd. Instead, they are set primarily on the user instance of systemd (as you said), but gdm also reads them back so that they influence gdm-x-session and gdm-wayland-session.

I think more experimenting is needed. But let's get the basic bits in place first, and then we can figure out how to best manipulate the environment. More fixes might be needed to the vncserver and vncsession.

• When run as a service with my system's init (in this case, Dinit), vncsession seemingly ignores any variables set within /etc/security/pam_env.conf, always falling back to ~/.local/state/tigervnc for logs. Presumably, pam_env doesn't even get invoked here?

That's very odd. I would have expected that to work. Have you double checked that pam_env is in the PAM configuration that vncsession uses?

Should you require any more information about this, I'm happy to provide it and potentially coordinate with the maintainers of my init system as well. Otherwise, I think I've addressed everything else for this PR now, so I can probably proceed with documentation changes and then mark it as ready.

Let's sort out the startup environment as step two, so we can get this first step in place. So please proceed with the documentation updates.

[CendioOssman]

Looking good! I think we're almost there.

SELinux might need an extra check. Do you have a system to test on, or do you need assistance with that?

[CendioOssman]

Hmm... I'm not sure these are correct anymore. .config and friends are not tagged as user_home_t.

@grulja, @zpytela, do you have some insight here? This might also mean #1425 needs to be adjusted.

[zpytela]

In the .fc file the legacy entries are still present, so you can keep the original userdom_* lines (or remove them if this is your intention) and add

gnome_config_filetrans(vnc_session_t, vnc_home_t, dir, "tigervnc")

which should work for both admin and non-admin users. This aligns with the added fc entries, type of ~/.config is config_home_t.

[CendioOssman]

Hmm... Thinking a bit more on this, do we even need the complexity of a custom type now that we use the XDG directories? Can't we just use the standard config_home_t? @zpytela?

[zpytela]

It makes sense to have private types and keep them separated if some domain needs to manage its data/config files or if there are some sensitive data. For reading it usually is not worth bothering.

[CendioOssman]

vncsession will only read out of ~/.config, so no need for it there then. It will write to .local/state/tigervnc, but only the log file.

Looking at gdm, it wants to create .cache/gdm/session.log, and I can see that .cache/gdm is tagged as xdm_home_t.

That directory is specific to gdm, though, not a general GNOME directory. So I'm not sure that it's equivalent.

Looking at my home directory, only PulseAudio and libvirt have custom types for .config. And only Telepathy and GNOME keyring for .local/share. So it doesn't seem to be particularly common for custom types?

My ~/.local/state is also gconf_home_t, something it seems to have inherited from ~/.local. Is that really correct? Or some historical artefact we'll have to live with? :/

@zpytela?

[CendioOssman]

One advantage of not having our own type is that the different XDG directories retain their type separation. Otherwise, our config, cache, data, and state will all have the same type.

[62832]

SELinux might need an extra check. Do you have a system to test on, or do you need assistance with that?

Unfortunately I do not have an SELinux-enabled system to test on, nor have I really worked with SELinux to begin with. I definitely will need some assistance with this part since I was more or less winging it there up until now.

[zpytela]

In the .fc file the legacy entries are still present, so you can keep the original userdom_* lines (or remove them if this is your intention) and add

gnome_config_filetrans(vnc_session_t, vnc_home_t, dir, "tigervnc")

which should work for both admin and non-admin users. This aligns with the added fc entries, type of ~/.config is config_home_t.

[zpytela]

Similar here, just add

gnome_data_filetrans(userdomain, vnc_home_t, dir, "tigervnc")

if you meant it for ~/.local/share/tigervnc.

[62832]

A bit odd that they're named in reference to GNOME specifically. Is this still a DE-agnostic setting?

[62832]

Also, does a gnome_state_filetrans exist for targeting $XDG_STATE_HOME / ~/.local/state/?

[62832]

Weirdly, it seems as though gnome_data_filetrans does not simply search the user home directory (and hence ~/.local/share), but instead makes use of gconf. Is this still the appropriate interface to use?

[zpytela]

gnome_data_filetrans ensures domain can walk through the common directory path starting from / before allowing additional access and transitions.
~/.local/state inherits its type from ~/.local, i. e. gconf_home_t.
The interfaces names are probably just legacy, a dozen years old and sometimes even older.

[62832]

I assume you meant ~/.local/share there, but yes, I can see that the gnome_data_filetrans interface is still fine here. Would I have to define an own interface in vncsession.if in order to have this apply to ~/.local/state as well?

[zpytela]

I meant ~/.local/state: it inherits the type from ~/.local so no other action is needed, while ~/.local/share has its own type. Files created in ~/.local/state will use the same transition rules for ~/.local if there are any.

[CendioOssman]

We also need to add permissions to create ~/.local, ~/.local/state and ~/.local/state/tigervnc. Unfortunately, I cannot find the proper interfaces for it. There is one for ~/.cache (which GDM needs), so I suspect there simply isn't an interface.

@zpytela ?

gnome_filetrans_home_content() seems to be part of it, as it sets up the transition. But I assume we also need gnome_filetrans_admin_home_content(). But the compilation breaks if I add both. :/

[CendioOssman]

This seems to give the proper SELinux behaviour, but it's not very clean given that it directly uses the funky gconf_home_t type:

diff --git a/unix/vncserver/selinux/vncsession.te b/unix/vncserver/selinux/vncsession.te
index 8e17842b..9afc4fe1 100644
--- a/unix/vncserver/selinux/vncsession.te
+++ b/unix/vncserver/selinux/vncsession.te
@@ -37,6 +37,19 @@ allow vnc_session_t self:fifo_file rw_fifo_file_perms;
 allow vnc_session_t vnc_session_var_run_t:file manage_file_perms;
 files_pid_filetrans(vnc_session_t, vnc_session_var_run_t, file)
 
+# Allowed to create ~/.local
+optional_policy(`
+	gnome_filetrans_home_content(vnc_session_t)
+	#gnome_filetrans_admin_home_content(vnc_session_t)
+')
+optional_policy(`
+	gen_require(`
+		type gconf_home_t;
+	')
+	create_dirs_pattern(vnc_session_t, gconf_home_t, gconf_home_t)
+')
+
+# Manage TigerVNC files (mainly ~/.local/state/*.log)
 create_dirs_pattern(vnc_session_t, vnc_home_t, vnc_home_t)
 manage_files_pattern(vnc_session_t, vnc_home_t, vnc_home_t)
 manage_fifo_files_pattern(vnc_session_t, vnc_home_t, vnc_home_t)
@@ -72,16 +85,16 @@ optional_policy(`
 	userdom_spec_domtrans_all_users(vnc_session_t)
 	userdom_signal_all_users(vnc_session_t)
 
-	userdom_user_home_dir_filetrans(vnc_session_t, vnc_home_t, dir, ".vnc")
-	userdom_admin_home_dir_filetrans(vnc_session_t, vnc_home_t, dir, ".vnc")
-
-	# This also affects other tools, e.g. vncpasswd
+	# Make sure legacy path has correct type
 	gen_require(`
 		attribute userdomain;
+		type gconf_home_t;
 	')
 	userdom_admin_home_dir_filetrans(userdomain, vnc_home_t, dir, ".vnc")
 	userdom_user_home_dir_filetrans(userdomain, vnc_home_t, dir, ".vnc")
 
-	gnome_config_filetrans(vnc_session_t, vnc_home_t, dir, "tigervnc")
+	gnome_config_filetrans(userdomain, vnc_home_t, dir, "tigervnc")
 	gnome_data_filetrans(userdomain, vnc_home_t, dir, "tigervnc")
+	filetrans_pattern(userdomain, gconf_home_t, vnc_home_t, dir, "tigervnc")
+	filetrans_pattern(vnc_session_t, gconf_home_t, vnc_home_t, dir, "tigervnc")
 ')

@zpytela, does this seem like what's needed?

[CendioOssman]

Let's try to get this finished, and we can do any tweaks to the SELinux policy later.

@62832, let me have one last look at my suggested SELinux patch, and then we can hopefully close this.

[CendioOssman]

The bug with gnome_filetrans_home_content() and gnome_filetrans_admin_home_content() seem to be that both call gnome_filetrans_gstreamer_home_content(), and it can only be called once. Not sure we can work around that, so I guess we'll have to not support running a session as root at the moment?

@62832, could you include my patch but remove the commented out gnome_filetrans_admin_home_content()? After that I think we are ready for a final review.

[62832]

Ready when you are @CendioOssman.

Should note that I'm also still awaiting a review from @bphinz to make sure that the Java and Windows side of things is all in order.

[CendioOssman]

Almost ready to go from my point of view. Nice work. :)

Could you squash some of the commit? We don't need to see every bug fix we've applied as part of the review process.

[62832]

Squashed everything into a single commit, including the last amendment to the HOWTO.md bit of documentation.

[CendioOssman]

Looks good to me.

@bphinz?

[bphinz]

I think it looks great. Thanks for all of your hard work on this! Sent from Gmail Mobile

…

On Tue, Apr 30, 2024 at 1:18 AM Pierre Ossman (ThinLinc team) < ***@***.***> wrote: ***@***.**** approved this pull request. Looks good to me. @bphinz <https://github.com/bphinz>? — Reply to this email directly, view it on GitHub <#1737 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB45M3IQN7X7X4MNOEFXUL3Y74STJAVCNFSM6AAAAABEYXRLQ6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMZQGIZDKNZVGA> . You are receiving this because you were mentioned.Message ID: ***@***.***>