SecurityException from I/O code in a parallel stream

2.3k views Asked by At

I have no way to explain this one, but I found this phenomenon in somebody else's code:

import java.io.IOException;
import java.io.UncheckedIOException;
import java.nio.file.Files;
import java.util.stream.Stream;

import org.junit.Test;

public class TestDidWeBreakJavaAgain
{
    @Test
    public void testIoInSerialStream()
    {
        doTest(false);
    }

    @Test
    public void testIoInParallelStream()
    {
        doTest(true);
    }

    private void doTest(boolean parallel)
    {
        Stream<String> stream = Stream.of("1", "2", "3");
        if (parallel)
        {
            stream = stream.parallel();
        }
        stream.forEach(name -> {
            try
            {
                Files.createTempFile(name, ".dat");
            }
            catch (IOException e)
            {
                throw new UncheckedIOException("Failed to create temp file", e);
            }
        });
    }
}

When run with the security manager enabled, merely calling parallel() on a stream, or parallelStream() when getting the stream from a collection, seems to guarantee that all attempts to perform I/O will throw SecurityException. (Most likely, calling any method which can throw SecurityException, will throw.)

I understand that parallel() means that it will be running in another thread which might not have the same privileges as the one we started with, but I guess I thought that the framework would take care of that for us.

Removing calls to parallel() or parallelStream() throughout the codebase avoids the risk. Inserting an AccessController.doPrivileged also fixes it, but doesn't sound safe to me, at least not in all situations. Is there any other option?

2

There are 2 answers

4
Paul Sandoz On BEST ANSWER

Parallel stream execution will use the Fork/Join framework, more specifically it will use the Fork/Join common pool. This is an implementation detail, but as observed in this case such details can leak out in unexpected ways.

Note that the same behaviour can also occur when executing a task asynchronously using CompletableFuture.

When a security manager is present the thread factory of the Fork/Join common pool is set to a factory that creates innocuous threads. Such an innocuous thread has no permissions granted to it, is not a member of any defined thread group, and after a top-level Fork/Join task has completed its execution all thread locals (if created) are cleared. Such behaviour ensures Fork/Join tasks are isolated from each other when sharing the common pool.

This is why in the example a SecurityException is thrown, probably:

java.lang.SecurityException: Unable to create temporary file or directory

There are two potential work arounds. Depending on the reasons a security manager utilized, each work around may increase the risk of being insecure.

The first, more general, work around is to register a Fork/Join thread factory via a system property to tell the Fork/Join framework what the default thread factory should be for the common pool. For example here is a really simple thread factory:

public class MyForkJoinWorkerThreadFactory
        implements ForkJoinPool.ForkJoinWorkerThreadFactory {
    public final ForkJoinWorkerThread newThread(ForkJoinPool pool) {
        return new ForkJoinWorkerThread(pool) {};
    }
}

Which can be registered with the following system property:

-Djava.util.concurrent.ForkJoinPool.common.threadFactory=MyForkJoinWorkerThreadFactory

The behaviour of MyForkJoinWorkerThreadFactory is currently equivalent to that of ForkJoinPool.defaultForkJoinWorkerThreadFactory.

The second, more specific, work around is to create a new Fork/Join pool. In this case the ForkJoinPool.defaultForkJoinWorkerThreadFactory will be utilized for constructors not accepting a ForkJoinWorkerThreadFactory argument. Any parallel stream execution would need to be performed from within a task executed from within that pool. Note that this is an implementation detail and may or may not work in future releases.

0
Holger On

Your worrying about AccessController.doPrivileged is unnecessary. It does not reduce the security if done right. The versions taking a single action argument will execute the action in your context, ignoring your callers but there are overloaded methods, having an additional argument, a previously recorded context:

private void doTest(boolean parallel)
{
    Consumer<String> createFile=name -> {
        try {
            Files.createTempFile(name, ".dat");
        }
        catch (IOException e) {
            throw new UncheckedIOException("Failed to create temp file", e);
        }
    }, actualAction;
    Stream<String> stream = Stream.of("1", "2", "3");

    if(parallel)
    {
        stream = stream.parallel();
        AccessControlContext ctx=AccessController.getContext();
        actualAction=name -> AccessController.doPrivileged(
          (PrivilegedAction<?>)()->{ createFile.accept(name); return null; }, ctx);
    }
    else actualAction = createFile;

    stream.forEach(actualAction);
}

The first important line is the AccessControlContext ctx=AccessController.getContext(); statement it records your current security context which includes your code and the current callers. (Remember that the effective permissions are the intersection of the sets of all callers). By providing the resulting context object ctx to the doPrivileged method within the Consumer you are reestablishing the context, in other words, the PrivilegedAction will have the same permissions as in your single-threaded scenario.