Recursively chaging ownership of nginx owned directories

[sshedi@…]

While nginx service starts, it creates temp directories like uwsgi_temp, proxy_temp etc. If nginx is running under nginx user context and these directories are not owned by nginx, nginx does a chown on these top directories but not the files and directories within these directories in other words, it doesn't happen recursively.

More details on this issue at:
​https://mailman.nginx.org/pipermail/nginx/2023-March/SPJTYXFKEKNRQPWAGXD6C24RFFOQONSZ.html

[sshedi@…]

Patch to fix the file ownership recursively.

[Maxim Dounin]

When nginx is started, it creating required directories, and also makes sure that appropriate permissions are set on the the created directories, so nginx worker processes will be able to create files in these directories.

If directories already exists, and permissions are incorrect, this also happens to correct permissions in simple cases: for example, if temporary directories were created under root during nginx installation, nginx will be able to change the ownership to the correct user as configured in nginx.conf.

It is, however, is not expected to scan the directories for conflicting files and directories inside them. If there are any, you may want to fix the conflicts yourself before starting nginx. In most cases, simply removing the directory with all the contents should be a good enough solution.

Note well that trying to teach nginx to resolve such conflicts might not be a good idea. For example, trying to chown()/chmod() contents of a large enough cache directory might take hours, and doing so on each nginx startup is not going to work. Further, this might imply a much larger risk if an incorrect directory is somehow specified in the configuration, for example, with something like proxy_temp_path /etc;.

[sshedi@…]

Hi Maxim Dounin,

Thanks for your inputs. However I have few more questions.

For example, trying to chown()/chmod() contents of a large enough cache directory might take hours, and doing so on each nginx startup is not going to work.

I don't think so. Even on a really large directory with way too many files and directories, it might take a minute but again it depends on underlying hardware but considering that this is a machine where server is hosted, it will probably have good hardware configuration. And these temp directories won't create those many files as there is an default upper limit and cache managed keeps cleaning up this directory periodically. And if a machine has those many temp files, user has bigger problems than dealing with starting time of nginx service because the temp directories would have crossed multiple GBs and nginx would be running slow no matter what.

Further, this might imply a much larger risk if an incorrect directory is somehow specified in the configuration, for example, with something like proxy_temp_path /etc;.

Please check
​https://github.com/nginx/nginx/blob/master/src/core/ngx_file.c#LL632C12-L632C12

Even in the existing approach, if user configures /etc as temp directory, it will change ownership of /etc, I verified it and it is indeed the case. So if a user configures /etc as temp dir, it's an error from user end and nginx documentation should cover this topic and clearly mention that users should not do this. Hence in this case, the operation should be binary, if we are changing directory permission, do it on all files and directories or don't do it at all.

Also, I see a design issue here:

If user provides proxy_temp_path /etc; as temp dir path, nginx should not use it directly. It should be something like:

If the given directory path already exists and not owned by nginx:

• Create a sub directory *_temp within it.

For example: if /etc is given as proxy_temp_path, create /etc/proxy_temp and proceed.

Else:
We can proceed further and no need to do anything.

Probably not a perfect solution but there is definitely room for improvement here.

Also, if you check my patch, the recursive chown is done only after changing ownership of top directory. So, I see no harm in it.

[Maxim Dounin]

I don't think so. Even on a really large directory with way too many files and directories, it might take a minute but again it depends on underlying hardware but considering that this is a machine where server is hosted, it will probably have good hardware configuration. And these temp directories won't create those many files as there is an default upper limit and cache managed keeps cleaning up this directory periodically. And if a machine has those many temp files, user has bigger problems than dealing with starting time of nginx service because the temp directories would have crossed multiple GBs and nginx would be running slow no matter what.

Thanks for your opinion. Examples of nginx installations with cache directories using multiple gigabytes are well known, and scanning these cache directories for loading the cache is known to take hours in extreme cases. The code in question equally applies to cache directories, not just to temporary directories.

Even in the existing approach, if user configures /etc as temp directory, it will change ownership of /etc, I verified it and it is indeed the case.

As said, suggested change will imply "a much larger risk". Right now it is mostly trivial to recover things after such misconfiguration. With the suggested change it will corrupt not only permissions on the /etc directory itself, but also on all its contents, making it hard-to-impossible to recover things.

[sshedi@…]

That makes sense, thanks for your time and inputs. Have a good time ahead.