Files#createTempDir stopped working on Windows with 32.0.0

[snuyanzin]

After update to guava 32.0.0
on Windows we start having such exception

 Caused by: java.lang.UnsupportedOperationException: 'posix:permissions' not supported as initial attribute
            at sun.nio.fs.WindowsSecurityDescriptor.fromAttribute(WindowsSecurityDescriptor.java:358)
            at sun.nio.fs.WindowsFileSystemProvider.createDirectory(WindowsFileSystemProvider.java:492)
            at java.nio.file.Files.createDirectory(Files.java:674)
            at java.nio.file.TempFileHelper.create(TempFileHelper.java:136)
            at java.nio.file.TempFileHelper.createTempDirectory(TempFileHelper.java:173)
            at java.nio.file.Files.createTempDirectory(Files.java:950)
            at com.google.common.io.TempFileCreator$JavaNioCreator.createTempDir(TempFileCreator.java:102)
            at com.google.common.io.Files.createTempDir(Files.java:439)

seems it is related to
feb83a1

[cpovirk]

Sorry about that. I was aware that some version of Windows had gotten some kind of POSIX support but probably also had heard enough during our jimfs work that I should have realized that that didn't solve this. A teammate ended thinking to test Windows after the release, at which point he confirmed that it didn't work. But all we've done so far is update the release notes, add a warning to the Javadoc at head, and try to read some about how to create the directory securely under Windows:

• https://wiki.sei.cmu.edu/confluence/display/java/FIO01-J.+Create+files+with+appropriate+access+permissions
• https://stackoverflow.com/q/58700035/28465

In response to a similar problem, the JUnit people took the position that createTempDirectory is secure by default, even if the caller doesn't pass any restrictions on permissions. So far, I haven't found any documentation that confirms that (and the first link above implies that opposite), though I can say that I haven't found a way to make it insecure on Linux, and the Windows behavior that my teammate saw looked secure, as well. We would really like to know for sure, though, given how much Guava users have already been through for this vulnerability....

After some more searching around, I've come across https://codeql.github.com/codeql-query-help/java/java-local-temp-file-or-directory-information-disclosure/, which at least suggests that Windows is different than Unix. It says that Windows provides each app with its own temporary directory and so you don't have to worry about permissions there. But that source would be more reassuring if it didn't suggest that you can change the permissions of an existing directory and then rely on it to be secure... and probably also if it didn't assume that all non-POSIX systems have secure temporary directories.

I found some more information that I haven't confirmed on Stack Overflow. (And I found that the Javadoc for java.io.File still suggests that the default is typically "C:\WINNT\TEMP"... :))

I see from the Calcite issue you linked (thanks!) that this comes from a file in embedded-redis, a project that hasn't had a commit in over 4 years. So I assume it's not going to get fixed there.

I also see that you have a workaround. I'd be interested to hear:

• from other users who are affected
• from anyone who can provide more official-looking guarantees of how to get secure behavior under Windows

[basil]

I'd be interested to hear […] from other users who are affected

See JENKINS-71375. There are about 21,984 installations (across all operating systems) of the Jenkins Artifactory plugin, which is subject to jfrog/build-info#737 on Windows. As a result, Jenkins Artifactory plugin users who are running Jenkins on Windows are impacted by this issue.

[cpovirk]

Thanks, and sorry: That's pretty impressively bad. I'll see what I can do about this tomorrow.

If anyone else ends up here and has any implementation tips, let me know. I'm tentatively planning to try to set ACL permissions on the directory, even though it's probably already in a secure directory, just to be safe. (By the time I'm convinced that I can prove that the code is running on NTFS or similar, I'm hoping it's not much harder to set the permissions, too.)

[basil]

I think we should only be setting POSIX attributes when FileSystem#supportedFileAttributeViews contains posix. The default value of java.io.tmpdir on Windows is the return value of GetTempPathW, which is defined to return the first of these paths:

1. The path specified by the TMP environment variable.
2. The path specified by the TEMP environment variable.
3. The path specified by the USERPROFILE environment variable.
4. The Windows directory.

(This blog post goes into the gory details regarding why both TMP and TEMP are set.)

My Windows 10 system had TMP and TEMP defined to %USERPROFILE%\AppData\Local\Temp, to which only SYSTEM, the current user, and members of the Administrators group had permissions. My Windows 2000 system defined these variables to %USERPROFILE%\Local Settings\Temp with the same permissions.

So it appears that the common case is for the directory returned by GetTempPathW to be secure, but one could imagine various corner cases:

• The user has changed the permissions on the default TMP/TEMP/USERPROFILE to allow broader access
• The user has redefined TMP/TEMP/USERPROFILE or java.io.tmpdir to a directory with broader access
• The user has redefined TMP/TEMP/USERPROFILE or java.io.tmpdir to a directory on a less secure filesystem, like FAT32
• The user has installed Windows on a less secure filesystem, like FAT32 (I think I did this once with Windows 2000, but not sure if that is still supported nowadays)

OpenJDK itself does not attempt to deal with these corner cases. If this approach is good enough for OpenJDK, it might be good enough for Guava as well. If not, it might be best to simply fail fast with e.g. UnsupportedOperationException if we detect a corner case. An approach that correctly handles all of these corner cases sounds like it would be challenging to implement (and test).

[cpovirk]

Thanks, that's all reassuring! I think I have figured out how to create the file with the desired ACL in the first place (by implementing FileAttribute myself with the name "acl:acl" and the value of the desired ACL). (And now that I know how to do it, I can find the ~2 total places on the Internet that it seems to be explained: 1, 2 :)) I have commandeered my wife's Windows machine to test it out. If it falls through, I'll be reasonably comfortable just calling createTempDirectory/createTempFile with no attributes, as you suggest.

[cpovirk]

OK, I have pushed #6540 (but you should ignore the PR with the number just before that one...) with my attempt at a fix. If any Windows users are in a position to easily test whether it solves the problems of 32.0.0, please give it a shot. Otherwise, I will probably release 32.0.1 with that change and wait to hear comments after release :)