guava: From Guava 32.0.1-jre, Files.createTempDir() and FileBackedOutputStream can fail or create un-deletable directories/files when used from a Windows service

For anyone who’s following here but not following our repo as a whole: The fix for this is part of last week’s https://github.com/google/guava/releases/tag/v32.1.3. If you see any problems with that version, please let us know.

Thanks, I’ll stick with the Java 9+ solution for now, then.

Also, I’ve retitled this bug to reflect that even creating the directories can fail.

If all goes well, I’ll submit the fix this week. Then it’s just a question of how soon we do a release. How did the workaround of setting user.name go?

Because the java process is launched from another process, i couldn’t easily inject that system property. Instead i just let the Jenkins service run with an ordinary Admin User instead of the local service account. That works now fine.

thanks @cpovirk , we are using java 17 LTS

thanks @cpovirk , I’m using Java11

I’m going to implement that approach now, since the Stack Overflow question sounds like just the same situation.

@cpovirk We run into a similar issue. Root cause is the same, but we can not even create a temp dir. We have here a Jenkins based CI running on windows. The jobs are executed by Jenkins nodes which are installed as windows services, hence running as windows SYSTEM account, which is the default service user under windows. Some test code calls Files.createTempDir(), which internally resolves system property user.name to PIPELINE$ (PIPELINE is the computer name) instead of SYSTEM. Hence creating of temp dir fails, as there is no user named PIPELINE$. we see then the following exception

java.lang.IllegalStateException: Failed to create directory
	at com.google.common.io.TempFileCreator$JavaNioCreator.createTempDir(TempFileCreator.java:110)
	at com.google.common.io.Files.createTempDir(Files.java:438)
	[...]
Caused by: java.io.IOException: Could not find user
	at com.google.common.io.TempFileCreator$JavaNioCreator.lambda$userPermissions$4(TempFileCreator.java:178)
	at com.google.common.io.TempFileCreator$JavaNioCreator.createTempDir(TempFileCreator.java:107)
Caused by: java.nio.file.attribute.UserPrincipalNotFoundException
	at java.base/sun.nio.fs.WindowsUserPrincipals.lookup(WindowsUserPrincipals.java:148)
	at java.base/sun.nio.fs.WindowsFileSystem$LookupService$1.lookupPrincipalByName(WindowsFileSystem.java:247)
	at com.google.common.io.TempFileCreator$JavaNioCreator.userPermissions(TempFileCreator.java:153)
	at com.google.common.io.TempFileCreator$JavaNioCreator.<clinit>(TempFileCreator.java:138)
	at com.google.common.io.TempFileCreator.pickSecureCreator(TempFileCreator.java:65)
	at com.google.common.io.TempFileCreator.<clinit>(TempFileCreator.java:51)

windows version

Edition	Windows Server 2022 Standard
Version	21H2
Installed on	‎7/‎18/‎2022
OS build	20348.169

trying to workarround by setting user.name to SYSTEM as java argument when invoking test command

Thanks for the additional details and testing.

It does seem extremely likely that plain java.nio.file.Files.createTempDirectory is secure. But the Linux-related JDK docs are noncommittal, saying something like like “the file/directory created might have more restricted permissions than you asked for.” That sounds like it might be referring to the umask, but I was seeing even more restricted permissions than the umask asked for (which is good from the security perspective). Perhaps because of the ambiguity, some docs suggest setting the permissions explicitly. And then Windows is its own story, with the temporary directory itself normally secure, as you said. Here’s some previous discussion.

I’m going to re-title this bug to reflect that it applies just to the daemon case. If we end up hearing from more affected users, we can reevaluate. Thanks for your flexibility.

Hello, a few comments & answer below:

to cgdecker: I’m not in Guava team but I know that the default windows temp dir is user-specific. So from my (limited) point of view, there is not really a need to set anything specific. In my case I explicitly tell Java to use another dir (through -Djava.io.tmpdir=e:\xxxxxxx\target ) so responsibility for setting limited rights could be on my side. I would find it fair.

to cpovirk 1-: please do not modify anything for me. I already simply moved my tests away from using Guava’s createTempDir() to java.nio.file.Files.createTempDirectory(). That was an easy move. Ijust reported the bug so that you know there is a problem others may face. It’s not a blocker anymore for me.

to cpovirk 2-: you were right, launching my tests with -Duser.name=SYSTEM corrected the problem. The process was able to correctly delete it’s own temporary dir.

for the record, the problem mentioned in JENKINS-71375 is specific to version 32.0.0-jre and was corrected in 32.0.1-jre and I think that only daemon services (and maybe especially the ones changing the default temp dir like I did) will be hurt with the same bug as described here.

Thanks for all the details.

At first glance, I’m puzzled: We test that we can delete the directory, and we have had our tests running under Windows. We have also tested manually under Windows, as have other users, and we’ve heard of apps that started working again at 32.0.1. So there must be some variation between systems that accounts for this.

One thing that I’d wondered was whether the system property user.name is the right one to use. Maybe it’s not? It looks like the code ended up granting permissions to a user “computerName$” (if I’m understanding right). Maybe that’s not actually the user that we need? (I do think I found that Java would fail if we tried to set a user that does not exist, at least in the testing I did.) It’s interesting that the java.nio.file.Files.createTempDirectory() output doesn’t appear to mention a specific user at all (again, if I’m understanding right). I think I’d seen a specific user listed in my testing.

One frustrating part of this is that we might not need to set explicit permissions at all: The Windows temporary directory is very likely to be secure, and the Java’s Linux implementation appears to set restricted permissions by default (but its docs are coy about this). Maybe I should just rip out all the permissions code. I’ve been hesitant to do so because it would look quite bad if such a change somehow reintroduced the security bug. (And of course now we’ve already had a bunch of releases in quick succession…)

Does anything about the username issue give you any ideas? Does anyone else have thoughts?