Pass user ID and group ID to docker native image build in Linux

[johnaohara]

…efault docker container will run as root

[Sanne]

it works locally, but I'm not sure about us using com.sun.security.auth.module.UnixSystem(). David just cleaned up some com.sun.security usage in another module.

@dmlloyd ?

[johnaohara]

@Sanne It doesn't work well with JDK11 (well anything after JDK9) due to modules, am looking for a different solution. I need to determine the numerical user ID, passing in the current user name does not work, due to this docker issue: https://success.docker.com/article/KB000447

[johnaohara]

And I don't think command substitution works in process builder either e.g.

Collections.addAll(nativeImage, "--user", "$(id -u):$(id -g)");

will not work

[dmlloyd]

How about id -ur/id -gr?

[johnaohara]

The only other way I have thought about so far is to have a DOCKER_USER environment variable in the docker file for the build and modify the dockerfile to pass the DOCKER_USER environment variable as one of the build parameters
(https://docs.docker.com/engine/reference/builder/#user)

[dmlloyd]

Alternatively we could develop a "stubs" module for the JDK security things. This would allow the com.sun.security classes to be used.

[dmlloyd]

You could try using JAAS to create a login context that authenticates using the UNIX login module. That would yield a principal with the real UID/GID. I'm not 100% sure it's possible to do that without actually referencing a com.sun class directly though.

[johnaohara]

How about id -ur/id -gr?

@dmlloyd when I tried passing those command into ProcessBuilder, they were not resolved, and docker tried to execute with the literals "$(id -ur):$(id -gr)"

[dmlloyd]

How about id -ur/id -gr?
@dmlloyd when I tried passing those command into ProcessBuilder, they were not resolved, and docker tried to execute with the literals "$(id -ur):$(id -gr)"

No I meant create new ProcessBuilders to get each value.

At any rate - you could simply call com.sun.security.auth.module.UnixSystem#getUid reflectively. Ugly but OK :)

[johnaohara]

No I meant create new ProcessBuilders to get each value.

@dmlloyd ah, that would work

you could simply call com.sun.security.auth.module.UnixSystem#getUid reflectively. Ugly but OK :)

ok, I'll take a look at those suggestions

[johnaohara]

@dmlloyd please could you review? thanks

[dmlloyd]

I didn't notice this at first but @gsmet pointed out that the process streams are never closed. These should use try-with-resources.

[johnaohara]

@dmlloyd rather than tying th CI up again. I have read through your comments, I think this addresses all the issues you raised

        try {
            StringBuilder responseBuilder = new StringBuilder();
            String line;

            ProcessBuilder idPB = new ProcessBuilder().command("id", option);
            idPB.redirectError(new File("/dev/null"));
            idPB.redirectOutput(new File("/dev/null"));

            process = idPB.start();
            try(InputStream inputStream = process.getInputStream()) {
                try( BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream))){
                    while ((line = reader.readLine()) != null) {
                        responseBuilder.append(line);
                    }
                    return responseBuilder.toString();
                }
            }
        } catch (IOException e) { //from process.start()
            //swallow and return null id
            return null;
        }

[dmlloyd]

Yeah I think that should do the job, except it's missing a process.waitFor() which should be done even in the exceptional case. You might want to add a catch to the try-with-resources:

        try {
            StringBuilder responseBuilder = new StringBuilder();
            String line;

            ProcessBuilder idPB = new ProcessBuilder().command("id", option);
            idPB.redirectError(new File("/dev/null"));
            idPB.redirectOutput(new File("/dev/null"));

            process = idPB.start();
            try(InputStream inputStream = process.getInputStream()) {
                try( BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream))){
                    while ((line = reader.readLine()) != null) {
                        responseBuilder.append(line);
                    }
                    return responseBuilder.toString();
                }
            } catch (Throwable t) {
                safeWaitFor(process);
                throw t;
            }
            safeWaitFor(process);
        } catch (IOException e) { //from process.start()
            //swallow and return null id
            return null;
        }

where safeWaitFor is simply:

static void safeWaitFor(Process process) {
    boolean intr = false;
    try {
        for (;;) try {
            process.waitFor();
            return;
        } catch (InterruptedException ex) {
            intr = true;
        }
    } finally {
        if (intr) Thread.currentThread.interrupt();
    }
}

Concerns were addressed in refactor