Create may set up wrong permissions for some directories in the path

[dinoallo]

Hi, I have noticed that it might not be correct to create all directories with 0o700 permissions:

xdg/internal/pathutil/pathutil.go

Lines 52 to 54 in d31002f

	if err := os.MkdirAll(dir, os.ModeDir|0o700); err == nil {
	return p, nil
	}

To be specific, the permission of XDG_RUNTIME_DIR is supposed to be 700 but its parent, usually /run/user/, is not. And if above code runs in a part of system daemon and has root privileges, some services that run as normal user may crash. This issue occurs when the system doesn't previously set up /run/user/, hence it's created alongside app/.

[adrg]

Hi @dinoallo.

If I understand correctly, the scenario you are describing is the following:

• One of the /run or /run/user directories does not exist. Maybe both.
• There is an application with root privileges on that system which creates a runtime file for itself at the location suggested by this package. In that case, the directories above would be created. The permissions of those directories would be 700.
• On the same system, an application running with regular user privileges would request a location to store a runtime file as well and the library would try to create the /run/user/UID directory. However, that would fail because /run/user is not writable.

Sure, in that case, the library would return an error specifying that it could not create that path.
However, why would you call that a crash?

Also, the same error would be returned if /run, /run/user or /run/user/UID would not exist because none of them can be created by regular users.

On most systems, /run/user/UID should exist though. Have you encountered a distribution where that is not the case?
I suppose the library could use /tmp as a fallback, but the same case could be made with that as well. What if that directory does not exist? The library does not have the permissions to create it.

In my opinion, applications should treat cases where files cannot be created at a particular location. If xdg.RuntimeFile returns an error, then that application could use xdg.StateFile instead, or choose another location.

[dinoallo]

Hi, @adrg, thank you so much for the speedy reply.

• One of the /run or /run/user directories does not exist. Maybe both.
• There is an application with root privileges on that system which creates a runtime file for itself at the location suggested by this package. In that case, the directories above would be created. The permissions of those directories would be 700.
• On the same system, an application running with regular user privileges would request a location to store a runtime file as well and the library would try to create the /run/user/UID directory. However, that would fail because /run/user is not writable.

This correctly describes the scenario that I brought up, and for convenience, I will call the application that has root privileges foo in the following. To be clear, foo runs as a daemon started by init before user entering their session.

Sure, in that case, the library would return an error specifying that it could not create that path. However, why would you call that a crash?

A crash may be over exaggerated in my opinion, but some applications probably set XDG_RUNTIME_DIR as default to store their files or simply rely on it. In that case, the application will not start at all or stop working when trying to write XDG_RUNTIME_DIR. For instance, gnome-keyring and pulseaudio won't even start if they don't have permissions to write XDG_RUNTIME_DIR.

Also, the same error would be returned if /run, /run/user or /run/user/UID would not exist because none of them can be created by regular users.
On most systems, /run/user/UID should exist though. Have you encountered a distribution where that is not the case?
I suppose the library could use /tmp as a fallback, but the same case could be made with that as well. What if that directory does not exist? The library does not have the permissions to create it.

For example, a distribution that doesn't use systemd as their default init daemon, usually does a poor job when it comes to user session management. Many of other init daemons don't have session support and rely on other login daemon to handle sessions. As a result, /run/user/ might not exist when foo is being turned-on. And if a user doesn't create it elsewhere before foo starts, error will be raised from applications like gnome-keyring that are supposed to run after login.

In my opinion, applications should treat cases where files cannot be created at a particular location. If xdg.RuntimeFile returns an error, then that application could use xdg.StateFile instead, or choose another location.

I think the blame is definitely on the wrongly-configured system, but there are probably some actions to take to prevent clueless developers from messing up system directories when calling xdg.RuntimeFile. IMHO, check XDG_RUNTIME_DIR first and if it doesn't exist, return an error instead of creating it for the application. XDG_RUNTIME_DIR should have been created by the system(pam_systemd, etc), or the application should have informed its users that it couldn't store files in this directory. And yes, the application could also have choose another location.

[adrg]

IMHO, check XDG_RUNTIME_DIR first and if it doesn't exist, return an error instead of creating it for the application. XDG_RUNTIME_DIR should have been created by the system(pam_systemd, etc), or the application should have informed its users that it couldn't store files in this directory. And yes, the application could also have choose another location.

I'm a bit torn regarding this approach because the end result is almost the same as the current one.

Let's say XDG_RUNTIME_DIR points to /run/user/$UID.

Scenario 1:
An application running as root uses this library and in the process creates the /run and the /run/user directories because they did not exist beforehand. In this case, an application running as a regular user calling xdg.RuntimeFile would receive an error from the package saying the runtime file cannot be written at /run/user/UID.

Scenario 2:
The /run and/or /run/user and/or /run/user/UID directories do not exist and an application running as a regular user tries to write a runtime file. Again, the application would receive an error from the package saying the runtime file cannot be written at /run/user/UID.

So in both scenarios, there is an error. A regular user would not be able to write a runtime file at that location.

With the approach you suggested, there would still be an error saying the runtime file cannot be written at that location. The difference would be that the /run and /run/user directories would not be created by the first scenario. But the outcome is the same: a runtime file cannot be written by a regular user.

Now, let's say I'm setting a custom value for $XDG_RUNTIME_DIR for myself in (.profile, .bashrc, .zshrc, whatever the case may be) because I'm on a distro which does not use systemd and I'm choosing a user-writable location like ~/.runtime. Right now, if that directory would not exist, it would be created and it would be usable.

However, with the approach you mentioned, the directory would not be created and instead an error would be returned by xdg.RuntimeFile, saying the ~/.runtime directory does not exist.

Moreover, even applications running as root would not be able to write runtime files in /run, /run/user or /run/user/UID if the directories do not exist in the first place.

[dinoallo]

With the approach you suggested, there would still be an error saying the runtime file cannot be written at that location. The difference would be that the /run and /run/user directories would not be created by the first scenario. But the outcome is the same: a runtime file cannot be written by a regular user.

What I am suggesting is maybe xdg.RuntimeFile shouldn't create $XDG_RUNTIME_DIR at all, and leave the creation to system login daemon or user-defined scripts. $XDG_RUNTIME_DIR will exist after user login as long as the system is well-configured and follow XDG Specification, i.e.

The lifetime of the directory MUST be bound to the user being logged in. It MUST be created when the user first logs in and if the user fully logs out the directory MUST be removed.

Even if xdg.RuntimeFile really needs to create $XDG_RUNTIME_DIR or/and its parents, it's required to know whether the user has logged in or not, and set the corresponding permissions like 755 for /run/user and 700 for /run/user/$UID. It's tremendously dangerous to have an application running as root and calling xdg.RuntimeFile that may control/modify system directory, without respect to login daemon and login session.

Now, let's say I'm setting a custom value for $XDG_RUNTIME_DIR for myself in (.profile, .bashrc, .zshrc, whatever the case may be) because I'm on a distro which does not use systemd and I'm choosing a user-writable location like ~/.runtime. Right now, if that directory would not exist, it would be created and it would be usable.

However, with the approach you mentioned, the directory would not be created and instead an error would be returned by xdg.RuntimeFile, saying the ~/.runtime directory does not exist.

If the system has custom-defined $XDG_RUNTIME_DIR, it's the same as what I mentioned above: this directory should exist beforehand. All applications that make use of xdg.RuntimeFile should assume a correctly set up XDG environment, or they can just turn to another available location.

Moreover, even applications running as root would not be able to write runtime files in /run, /run/user or /run/user/UID if the directories do not exist in the first place.

Assume that /run already exists but /run/user and /run/user/$UID doesn't. $XDG_RUNTIME_DIR is set to /run/user/$UID. To my understanding, in the current approach, the application running as root calls xdg.RuntimeFile, and xdg.RuntimeFile calls Create, which tells the os to create those two directories and set both permissions to 700. As a result, a regular user will be denied access to /run/user/ and can't write anything inside even if they own their $XDG_RUNTIME_DIR. Please correct me if I am wrong.

Also I think the possible solution I mentioned earlier is still rough and needs to be more considerate. It still has room for discussion.

[adrg]

The lifetime of the directory MUST be bound to the user being logged in. It MUST be created when the user first logs in and if the user fully logs out the directory MUST be removed.

This is a strong argument against creating the $XDG_RUNTIME_DIR directory and rely on its existence, although the spec also specifies this:

If, when attempting to write a file, the destination directory is non-existent an attempt should be made to create it with permission 0700. If the destination directory exists already the permissions should not be changed.

But yeah, I tend to agree with you that maybe a check should be made to ensure the existence of the directory pointed to by the $XDG_RUNTIME_DIR directory and skip its creation.

The spec also mentions this:

If $XDG_RUNTIME_DIR is not set applications should fall back to a replacement directory with similar capabilities and print a warning message.

I guess that /tmp could potentially be a candidate as a fallback, if /run/user/UID is not accessible for any reason. Inside /tmp, the existing logic can be maintained. The problem with /tmp is that it usually has 777 permissions so anyone could potentially create anything in there.

[dinoallo]

I guess that /tmp could potentially be a candidate as a fallback, if /run/user/UID is not accessible for any reason. Inside /tmp, the existing logic can be maintained. The problem with /tmp is that it usually has 777 permissions so anyone could potentially create anything in there.

/tmp seems to be the most satisfying location currently. At the same time xdg.RuntimeFile should warn that the replacement directory is taken. Is it possible to set up a directory in /tmp, like appname-ABCDE12345/, only for the application? It might need an extra parameter to make the directory more recognizable. Or the directory name can just be collision-free and random characters, considering xdg.RuntimeFile returns it anyway.

[adrg]

I guess that /tmp could potentially be a candidate as a fallback, if /run/user/UID is not accessible for any reason. Inside /tmp, the existing logic can be maintained. The problem with /tmp is that it usually has 777 permissions so anyone could potentially create anything in there.

/tmp seems to be the most satisfying location currently. At the same time xdg.RuntimeFile should warn that the replacement directory is taken. Is it possible to set up a directory in /tmp, like appname-ABCDE12345/, only for the application? It might need an extra parameter to make the directory more recognizable. Or the directory name can just be collision-free and random characters, considering xdg.RuntimeFile returns it anyway.

This is application logic. The library should not be responsible for naming the directory the application stores files in. Also, generating a random string would kind of go against the spec:

If the user logs in more than once he should get pointed to the same directory, and it is mandatory that the directory continues to exist from his first login to his last logout on the system, and not removed in between. Files in the directory MUST not survive reboot or a full logout/login cycle.

So when an application calls xdg.RuntimeFile("appname/some-runtime-file") the library would:

1. Check if $XDG_RUNTIME_DIR is set. If not set, it would default to /run/user/$UID.
2. Check for the existence of the runtime directory. If it does not exist, it would fall back to /tmp.
3. Create any sub-directories specified to the function, if any. If successful, return the path. Otherwise, return an error.

As stated by the XDG base directory spec:

The application should be prepared to handle the case where the file could not be written, either because the directory was non-existent and could not be created, or for any other reason. In such case it may choose to present an error message to the user.

[dinoallo]

This is application logic. The library should not be responsible for naming the directory the application stores files in. Also, generating a random string would kind of go against the spec:

If the user logs in more than once he should get pointed to the same directory, and it is mandatory that the directory continues to exist from his first login to his last logout on the system, and not removed in between. Files in the directory MUST not survive reboot or a full logout/login cycle.

I am actually on the same page with returning the viable path only. It's re-usable and feels more natural.

So when an application calls xdg.RuntimeFile("appname/some-runtime-file") the library would:

1. Check if `$XDG_RUNTIME_DIR` is set. If not set, it would default to `/run/user/$UID`.

2. Check for the existence of the runtime directory. If it does not exist, it would fall back to `/tmp`.

3. Create any sub-directories specified to the function, if any. If successful, return the path. Otherwise, return an error.

As stated by the XDG base directory spec:

The application should be prepared to handle the case where the file could not be written, either because the directory was non-existent and could not be created, or for any other reason. In such case it may choose to present an error message to the user.

Yea, it seems reasonable to me.