-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Add doc on Unix temporary file security practice #70585
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
Changes from 1 commit
ef07497
ef25c1a
d8555f2
5ff7105
045841b
b2a9a8e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
|
|
||
| # Unix temporary files | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't have a Mac to experiment with. Does anything in here differ for Mac? It seems that Macs support ACLs as well, and in some circumstances they are used in Linux as well. Should we include guidance for those? (@tmds)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My expectation is that mac will be similar to Linux: /tmp has the sticky bit, and a umask of at least 002.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are ACL's relevant to many Linux users, enough that (in theory) we should set them if available? Or are they mainly for special purposes.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mac has its own restrictions, and doesn't necessarily have a shared temp directory. Just in case, the above should probably be followed, but it may not be necessary, depending on the configuration. |
||
|
|
||
| The Unix support for temporary files is different from the Windows model and developers who | ||
| are used to Windows may inadvertantly create security risk if they use the same practices on Unix. | ||
|
|
||
| Most notably, the Windows model for temporary files is that the operating system provides each user with a *unique*, *user-owned* temporary directory. | ||
| Moreover, all Windows users, including the service and system users, have designated user folders, including temporary folders. | ||
|
|
||
| The Unix model is very different. The temp directory, assuming there is one, is often a global folder (except on MacOS). | ||
| The `TMPDIR` environment variable is used to store the location of this folder. This variable is | ||
| widely used and supported, but it is not mandatory for all Unix implementations. It should be the preferred | ||
| mechanism for finding the Unix temporary folder, if a library method is not available. It will commonly | ||
| point to either the `/tmp` or `/var/tmp` folder. These folders are not used for MacOS, so it is not recommended | ||
| to use them directly. | ||
|
|
||
| Because the temporary directory is often global, any use of the temp directory should be carefully | ||
| considered. In general, the best use of the temp directory is for programs which, | ||
|
|
||
| 1. Will create the temporary file during their process execution | ||
| 1. Do not depend on predictable temporary file/folder names | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's ok to depend on a predictable prefix, which you might wish to use if you want to use it to communicate between processes. |
||
| 1. Will not access the file after the process exits | ||
|
|
||
| In these cases, the process can create a file or files with | ||
| 1. A pseudorandom name, unlikely to cause collisions | ||
| 1. Permissions which restrict all access to owner-only | ||
|
|
||
| Any other use needs to be carefully audited, particularly if the temporary file is intended for use across | ||
| multiple processes. Some considerations: | ||
|
|
||
| - **Never** write files with global access permissions | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I was hoping for guidance specifically for directory permissions. In the case we were discussing, the customer expected drwxrwxrwt rather than drwxrwxrwx. I'm guessing your guidance is drwx------ ? (This is not my area of expertise)
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know why you would have different settings for files and directories. 700 seems right for both, to me.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, their semantics are a little different (eg the sticky bit). I guess though if what you want is "complete access for current user and zero for everyone else" then it's 700.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. .NET 7 will introduce APIs to work with unix permissions. With earlier versions, users cannot control them.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, 700 is more than necessary if you don't need the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup, I meant |
||
| - **Always** verify that the owner of the file is the current user and that the permissions | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. .NET doesn't have an API to verify file ownership.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In that case I think P/Invoking into
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cc @stephentoub on this one. Verifying file ownership is a super critical part of the unix security model. It's the reason why only root can change the ownership of files (you can't give ownership of your files to another user). It might be worth some sort of .NET API.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In most cases you don't need to verify ownership. If you can open the dir/file you know you are permitted, and you can check permissions to verify they are not too permissive. |
||
| only allow write access by the owner when reading existing files | ||
| - **Never** rely on having ownership of a particular file name. Any process can write a file with that name, | ||
| creating a denial of service. | ||
| - When creating files, consider likelihood of file name collision and performance impact of attempting | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It can be simplest to create a single folder and do everything else below that. if (Path.GetTempPath() == "/tmp/")
{
string tmpDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString("N"));
Directory.CreateDirectory(tmpDir, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute);
Environment.SetEnvironmentVariable("TMPDIR", tmpDir + "/");
}
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there any value in using a cryptographically secure name here? Guid is not (or at least not enough). It can use GetRandomFileName()
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would only make a difference if the attacker has permission to open files in the temp directory but not to enumerate them.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a question of the app's threat model and can't be answered generally. |
||
| to create new names, if supported. | ||
|
|
||
| If any of the above conflict with the feature requirements, consider instead writing temporary files to a | ||
| location in the user home folder. Some considerations for this model: | ||
|
|
||
| - There is no automatic cleanup in user folders. Files will remain permanently or require cleanup by the app | ||
| - Some environments do not have user home folders (e.g., systemd). Consider providing an environment variable | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. systemd has a
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe also worth mentioning:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some interesting requirements there, including that files not survive reboot. It might be worth specifying here that we should not expect our temp files to survive reboot, but we should not expect them to be cleaned up by reboot either. |
||
| to override the location of the temporary folder, and provide user documentation for this variable. | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be reasonable to expand the scope a little to cover "Unix file and directory permissions" in general? Whereever we write a file, we should set minimum permissions on it. And we would have the same guidance that there should be no perms for group and world unless they are required.