From 168b18ec68dd5488704cf76895d2449cd86428a6 Mon Sep 17 00:00:00 2001 From: Roger Riggs Date: Wed, 13 Nov 2024 20:49:59 +0000 Subject: [PATCH] 8343958: Remove security manager impl in java.lang.Process and java.lang.Runtime.exec Reviewed-by: jpai, mullan, alanb --- .../classes/java/lang/ProcessBuilder.java | 25 +----- .../classes/java/lang/ProcessHandleImpl.java | 89 +++++-------------- .../unix/classes/java/lang/ProcessImpl.java | 21 +---- .../classes/java/lang/ProcessImpl.java | 89 ++++++------------- 4 files changed, 55 insertions(+), 169 deletions(-) diff --git a/src/java.base/share/classes/java/lang/ProcessBuilder.java b/src/java.base/share/classes/java/lang/ProcessBuilder.java index c3cb9bfd145..9cb5848bdff 100644 --- a/src/java.base/share/classes/java/lang/ProcessBuilder.java +++ b/src/java.base/share/classes/java/lang/ProcessBuilder.java @@ -339,11 +339,6 @@ public final class ProcessBuilder * @see System#getenv() */ public Map environment() { - @SuppressWarnings("removal") - SecurityManager security = System.getSecurityManager(); - if (security != null) - security.checkPermission(new RuntimePermission("getenv.*")); - if (environment == null) environment = ProcessEnvironment.environment(); @@ -1069,11 +1064,6 @@ public final class ProcessBuilder // Throws IndexOutOfBoundsException if command is empty String prog = cmdarray[0]; - @SuppressWarnings("removal") - SecurityManager security = System.getSecurityManager(); - if (security != null) - security.checkExec(prog); - String dir = directory == null ? null : directory.toString(); for (String s : cmdarray) { @@ -1112,24 +1102,13 @@ public final class ProcessBuilder } return process; } catch (IOException | IllegalArgumentException e) { - String exceptionInfo = ": " + e.getMessage(); - Throwable cause = e; - if ((e instanceof IOException) && security != null) { - // Can not disclose the fail reason for read-protected files. - try { - security.checkRead(prog); - } catch (SecurityException se) { - exceptionInfo = ""; - cause = se; - } - } // It's much easier for us to create a high-quality error // message than the low-level C code which found the problem. throw new IOException( "Cannot run program \"" + prog + "\"" + (dir == null ? "" : " (in directory \"" + dir + "\")") - + exceptionInfo, - cause); + + ": " + e.getMessage(), + e); } } diff --git a/src/java.base/share/classes/java/lang/ProcessHandleImpl.java b/src/java.base/share/classes/java/lang/ProcessHandleImpl.java index 7dd6bf6a5e4..a7243b8b6f4 100644 --- a/src/java.base/share/classes/java/lang/ProcessHandleImpl.java +++ b/src/java.base/share/classes/java/lang/ProcessHandleImpl.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2014, 2024, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -84,28 +84,28 @@ final class ProcessHandleImpl implements ProcessHandle { /** * The thread pool of "process reaper" daemon threads. */ - @SuppressWarnings("removal") - private static final Executor processReaperExecutor = - AccessController.doPrivileged((PrivilegedAction) () -> { - // Initialize ThreadLocalRandom now to avoid using the smaller stack - // of the processReaper threads. - ThreadLocalRandom.current(); + private static final Executor processReaperExecutor = initReaper(); - // For a debug build, the stack shadow zone is larger; - // Increase the total stack size to avoid potential stack overflow. - int debugDelta = "release".equals(System.getProperty("jdk.debug")) ? 0 : (4*4096); - final long stackSize = Boolean.getBoolean("jdk.lang.processReaperUseDefaultStackSize") - ? 0 : REAPER_DEFAULT_STACKSIZE + debugDelta; + private static Executor initReaper() { + // Initialize ThreadLocalRandom now to avoid using the smaller stack + // of the processReaper threads. + ThreadLocalRandom.current(); - ThreadFactory threadFactory = grimReaper -> { - Thread t = InnocuousThread.newSystemThread("process reaper", grimReaper, - stackSize, Thread.MAX_PRIORITY); - privilegedThreadSetDaemon(t, true); - return t; - }; + // For a debug build, the stack shadow zone is larger; + // Increase the total stack size to avoid potential stack overflow. + int debugDelta = "release".equals(System.getProperty("jdk.debug")) ? 0 : (4 * 4096); + final long stackSize = Boolean.getBoolean("jdk.lang.processReaperUseDefaultStackSize") + ? 0 : REAPER_DEFAULT_STACKSIZE + debugDelta; - return Executors.newCachedThreadPool(threadFactory); - }); + ThreadFactory threadFactory = grimReaper -> { + Thread t = InnocuousThread.newSystemThread("process reaper", grimReaper, + stackSize, Thread.MAX_PRIORITY); + t.setDaemon(true); + return t; + }; + + return Executors.newCachedThreadPool(threadFactory); + } private static class ExitCompletion extends CompletableFuture { final boolean isReaping; @@ -115,22 +115,6 @@ final class ProcessHandleImpl implements ProcessHandle { } } - @SuppressWarnings("removal") - private static void privilegedThreadSetName(Thread thread, String name) { - AccessController.doPrivileged((PrivilegedAction) () -> { - thread.setName(name); - return null; - }); - } - - @SuppressWarnings("removal") - private static void privilegedThreadSetDaemon(Thread thread, boolean on) { - AccessController.doPrivileged((PrivilegedAction) () -> { - thread.setDaemon(on); - return null; - }); - } - /** * Returns a CompletableFuture that completes with process exit status when * the process completes. @@ -158,7 +142,7 @@ final class ProcessHandleImpl implements ProcessHandle { public void run() { Thread t = Thread.currentThread(); String threadName = t.getName(); - privilegedThreadSetName(t, "process reaper (pid " + pid + ")"); + t.setName("process reaper (pid " + pid + ")"); try { int exitValue = waitForProcessExit0(pid, shouldReap); if (exitValue == NOT_A_CHILD) { @@ -189,7 +173,7 @@ final class ProcessHandleImpl implements ProcessHandle { completions.remove(pid, newCompletion); } finally { // Restore thread name - privilegedThreadSetName(t, threadName); + t.setName(threadName); } } }); @@ -255,14 +239,8 @@ final class ProcessHandleImpl implements ProcessHandle { * @param pid the native process identifier * @return The ProcessHandle for the pid if the process is alive; * or {@code null} if the process ID does not exist in the native system. - * @throws SecurityException if RuntimePermission("manageProcess") is not granted */ static Optional get(long pid) { - @SuppressWarnings("removal") - SecurityManager sm = System.getSecurityManager(); - if (sm != null) { - sm.checkPermission(new RuntimePermission("manageProcess")); - } long start = isAlive0(pid); return (start >= 0) ? Optional.of(new ProcessHandleImpl(pid, start)) @@ -296,14 +274,8 @@ final class ProcessHandleImpl implements ProcessHandle { * Returns the ProcessHandle for the current native process. * * @return The ProcessHandle for the OS process. - * @throws SecurityException if RuntimePermission("manageProcess") is not granted */ public static ProcessHandleImpl current() { - @SuppressWarnings("removal") - SecurityManager sm = System.getSecurityManager(); - if (sm != null) { - sm.checkPermission(new RuntimePermission("manageProcess")); - } return current; } @@ -319,15 +291,8 @@ final class ProcessHandleImpl implements ProcessHandle { * * @return a ProcessHandle of the parent process; {@code null} is returned * if the child process does not have a parent - * @throws SecurityException if permission is not granted by the - * security policy */ public Optional parent() { - @SuppressWarnings("removal") - SecurityManager sm = System.getSecurityManager(); - if (sm != null) { - sm.checkPermission(new RuntimePermission("manageProcess")); - } long ppid = parent0(pid, startTime); if (ppid <= 0) { return Optional.empty(); @@ -442,11 +407,6 @@ final class ProcessHandleImpl implements ProcessHandle { * @return a stream of ProcessHandles */ static Stream children(long pid) { - @SuppressWarnings("removal") - SecurityManager sm = System.getSecurityManager(); - if (sm != null) { - sm.checkPermission(new RuntimePermission("manageProcess")); - } int size = 100; long[] childpids = null; long[] starttimes = null; @@ -463,11 +423,6 @@ final class ProcessHandleImpl implements ProcessHandle { @Override public Stream descendants() { - @SuppressWarnings("removal") - SecurityManager sm = System.getSecurityManager(); - if (sm != null) { - sm.checkPermission(new RuntimePermission("manageProcess")); - } int size = 100; long[] pids = null; long[] ppids = null; diff --git a/src/java.base/unix/classes/java/lang/ProcessImpl.java b/src/java.base/unix/classes/java/lang/ProcessImpl.java index da4874b0181..7b5d27f1cc1 100644 --- a/src/java.base/unix/classes/java/lang/ProcessImpl.java +++ b/src/java.base/unix/classes/java/lang/ProcessImpl.java @@ -42,14 +42,10 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.Condition; import java.util.concurrent.locks.ReentrantLock; -import java.security.AccessController; -import java.security.PrivilegedActionException; -import java.security.PrivilegedExceptionAction; import jdk.internal.access.JavaIOFileDescriptorAccess; import jdk.internal.access.SharedSecrets; import jdk.internal.util.OperatingSystem; import jdk.internal.util.StaticProperty; -import sun.security.action.GetPropertyAction; /** * java.lang.Process subclass in the UNIX environment. @@ -95,7 +91,7 @@ final class ProcessImpl extends Process { * @throws Error if the requested launch mechanism is not found or valid */ private static LaunchMechanism launchMechanism() { - String s = GetPropertyAction.privilegedGetProperty("jdk.lang.Process.launchMechanism"); + String s = System.getProperty("jdk.lang.Process.launchMechanism"); if (s == null) { return LaunchMechanism.POSIX_SPAWN; } @@ -282,7 +278,6 @@ final class ProcessImpl extends Process { boolean redirectErrorStream) throws IOException; - @SuppressWarnings("removal") private ProcessImpl(final byte[] prog, final byte[] argBlock, final int argc, final byte[] envBlock, final int envc, @@ -302,14 +297,7 @@ final class ProcessImpl extends Process { redirectErrorStream); processHandle = ProcessHandleImpl.getInternal(pid); - try { - AccessController.doPrivileged((PrivilegedExceptionAction) () -> { - initStreams(fds, forceNullOutputStream); - return null; - }); - } catch (PrivilegedActionException ex) { - throw (IOException) ex.getCause(); - } + initStreams(fds, forceNullOutputStream); } static FileDescriptor newFileDescriptor(int fd) { @@ -507,11 +495,6 @@ final class ProcessImpl extends Process { @Override public ProcessHandle toHandle() { - @SuppressWarnings("removal") - SecurityManager sm = System.getSecurityManager(); - if (sm != null) { - sm.checkPermission(new RuntimePermission("manageProcess")); - } return processHandle; } diff --git a/src/java.base/windows/classes/java/lang/ProcessImpl.java b/src/java.base/windows/classes/java/lang/ProcessImpl.java index fd0d5b03b0c..967693dcbc3 100644 --- a/src/java.base/windows/classes/java/lang/ProcessImpl.java +++ b/src/java.base/windows/classes/java/lang/ProcessImpl.java @@ -35,8 +35,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.lang.ProcessBuilder.Redirect; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.util.ArrayList; import java.util.Locale; import java.util.concurrent.CompletableFuture; @@ -48,7 +46,6 @@ import jdk.internal.access.JavaIOFileDescriptorAccess; import jdk.internal.access.SharedSecrets; import jdk.internal.ref.CleanerFactory; import jdk.internal.misc.Blocker; -import sun.security.action.GetPropertyAction; /* This class is for the exclusive use of ProcessBuilder.start() to * create new processes. @@ -71,25 +68,15 @@ final class ProcessImpl extends Process { * to append to a file does not open the file in a manner that guarantees * that writes by the child process will be atomic. */ - @SuppressWarnings("removal") private static FileOutputStream newFileOutputStream(File f, boolean append) throws IOException { if (append) { String path = f.getPath(); - SecurityManager sm = System.getSecurityManager(); - if (sm != null) - sm.checkWrite(path); long handle = openForAtomicAppend(path); final FileDescriptor fd = new FileDescriptor(); fdAccess.setHandle(fd, handle); - return AccessController.doPrivileged( - new PrivilegedAction() { - public FileOutputStream run() { - return new FileOutputStream(fd); - } - } - ); + return new FileOutputStream(fd); } else { return new FileOutputStream(f); } @@ -424,7 +411,6 @@ final class ProcessImpl extends Process { private InputStream stdout_stream; private InputStream stderr_stream; - @SuppressWarnings("removal") private ProcessImpl(String cmd[], final String envblock, final String path, @@ -434,13 +420,10 @@ final class ProcessImpl extends Process { throws IOException { String cmdstr; - final SecurityManager security = System.getSecurityManager(); - final String value = GetPropertyAction. - privilegedGetProperty("jdk.lang.Process.allowAmbiguousCommands", - (security == null ? "true" : "false")); + final String value = System.getProperty("jdk.lang.Process.allowAmbiguousCommands", "true"); final boolean allowAmbiguousCommands = !"false".equalsIgnoreCase(value); - if (allowAmbiguousCommands && security == null) { + if (allowAmbiguousCommands) { // Legacy mode. // Normalize path if possible. @@ -478,10 +461,6 @@ final class ProcessImpl extends Process { // Parse the command line again. cmd = getTokensFromCommand(join.toString()); executablePath = getExecutablePath(cmd[0]); - - // Check new executable name once more - if (security != null) - security.checkExec(executablePath); } // Quotation protects from interpretation of the [path] argument as @@ -505,39 +484,34 @@ final class ProcessImpl extends Process { processHandle = ProcessHandleImpl.getInternal(getProcessId0(handle)); - java.security.AccessController.doPrivileged( - new java.security.PrivilegedAction() { - public Void run() { - if (stdHandles[0] == -1L) - stdin_stream = ProcessBuilder.NullOutputStream.INSTANCE; - else { - FileDescriptor stdin_fd = new FileDescriptor(); - fdAccess.setHandle(stdin_fd, stdHandles[0]); - fdAccess.registerCleanup(stdin_fd); - stdin_stream = new BufferedOutputStream( - new PipeOutputStream(stdin_fd)); - } + if (stdHandles[0] == -1L) + stdin_stream = ProcessBuilder.NullOutputStream.INSTANCE; + else { + FileDescriptor stdin_fd = new FileDescriptor(); + fdAccess.setHandle(stdin_fd, stdHandles[0]); + fdAccess.registerCleanup(stdin_fd); + stdin_stream = new BufferedOutputStream( + new PipeOutputStream(stdin_fd)); + } - if (stdHandles[1] == -1L || forceNullOutputStream) - stdout_stream = ProcessBuilder.NullInputStream.INSTANCE; - else { - FileDescriptor stdout_fd = new FileDescriptor(); - fdAccess.setHandle(stdout_fd, stdHandles[1]); - fdAccess.registerCleanup(stdout_fd); - stdout_stream = new BufferedInputStream( - new PipeInputStream(stdout_fd)); - } + if (stdHandles[1] == -1L || forceNullOutputStream) + stdout_stream = ProcessBuilder.NullInputStream.INSTANCE; + else { + FileDescriptor stdout_fd = new FileDescriptor(); + fdAccess.setHandle(stdout_fd, stdHandles[1]); + fdAccess.registerCleanup(stdout_fd); + stdout_stream = new BufferedInputStream( + new PipeInputStream(stdout_fd)); + } - if (stdHandles[2] == -1L) - stderr_stream = ProcessBuilder.NullInputStream.INSTANCE; - else { - FileDescriptor stderr_fd = new FileDescriptor(); - fdAccess.setHandle(stderr_fd, stdHandles[2]); - fdAccess.registerCleanup(stderr_fd); - stderr_stream = new PipeInputStream(stderr_fd); - } - - return null; }}); + if (stdHandles[2] == -1L) + stderr_stream = ProcessBuilder.NullInputStream.INSTANCE; + else { + FileDescriptor stderr_fd = new FileDescriptor(); + fdAccess.setHandle(stderr_fd, stdHandles[2]); + fdAccess.registerCleanup(stderr_fd); + stderr_stream = new PipeInputStream(stderr_fd); + } } public OutputStream getOutputStream() { @@ -632,11 +606,6 @@ final class ProcessImpl extends Process { @Override public ProcessHandle toHandle() { - @SuppressWarnings("removal") - SecurityManager sm = System.getSecurityManager(); - if (sm != null) { - sm.checkPermission(new RuntimePermission("manageProcess")); - } return processHandle; }