Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #6184 - JEP-411 will deprecate/remove the SecurityManager from … #9616

Merged
merged 4 commits into from
Apr 6, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import javax.security.auth.message.config.AuthConfigProvider;
import javax.security.auth.message.config.RegistrationListener;

import org.eclipse.jetty.util.security.SecurityUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -60,9 +61,7 @@ public AuthConfigProvider getConfigProvider(String layer, String appContext, Reg
@Override
public String registerConfigProvider(String className, Map properties, String layer, String appContext, String description)
{
SecurityManager sm = System.getSecurityManager();
if (sm != null)
sm.checkPermission(new AuthPermission("registerAuthConfigProvider"));
checkPermission("registerAuthConfigProvider");

String key = getKey(layer, appContext);
AuthConfigProvider configProvider = createConfigProvider(className, properties);
Expand All @@ -76,9 +75,7 @@ public String registerConfigProvider(String className, Map properties, String la
@Override
public String registerConfigProvider(AuthConfigProvider provider, String layer, String appContext, String description)
{
SecurityManager sm = System.getSecurityManager();
if (sm != null)
sm.checkPermission(new AuthPermission("registerAuthConfigProvider"));
checkPermission("registerAuthConfigProvider");

String key = getKey(layer, appContext);
DefaultRegistrationContext context = new DefaultRegistrationContext(provider, layer, appContext, description, false);
Expand All @@ -91,9 +88,7 @@ public String registerConfigProvider(AuthConfigProvider provider, String layer,
@Override
public boolean removeRegistration(String registrationID)
{
SecurityManager sm = System.getSecurityManager();
if (sm != null)
sm.checkPermission(new AuthPermission("removeAuthRegistration"));
checkPermission("removeAuthRegistration");

DefaultRegistrationContext registrationContext = _registrations.remove(registrationID);
if (registrationContext == null)
Expand All @@ -106,9 +101,7 @@ public boolean removeRegistration(String registrationID)
@Override
public String[] detachListener(RegistrationListener listener, String layer, String appContext)
{
SecurityManager sm = System.getSecurityManager();
if (sm != null)
sm.checkPermission(new AuthPermission("detachAuthListener"));
checkPermission("detachAuthListener");

List<String> registrationIds = new ArrayList<>();
for (DefaultRegistrationContext registration : _registrations.values())
Expand Down Expand Up @@ -145,13 +138,16 @@ public RegistrationContext getRegistrationContext(String registrationID)
@Override
public void refresh()
{
SecurityManager sm = System.getSecurityManager();
if (sm != null)
sm.checkPermission(new AuthPermission("refreshAuth"));
checkPermission("refreshAuth");

// TODO: maybe we should re-construct providers created from classname.
}

private static void checkPermission(String permission)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have a util class or method that does this as this is used in a few places.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about this and there are only 2 places, which are quite unrelated: here and ContextHandler.
Given that JASPI is not the most common specification, I thought that the less dependencies the better, so a utility class was not worth the effort.
Note also that all JASPI permissions are AuthPermissions, while a generic utility class would need a more generic signature, making this class a little more verbose.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure this is no more complex than changing the method signature to checkPermission(Permission). So my preference is still for a utility class/method.

{
SecurityUtils.checkPermission(new AuthPermission(permission));
}

private static String getKey(String layer, String appContext)
{
return layer + "/" + appContext;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@
import java.net.URI;
import java.net.URL;
import java.net.URLClassLoader;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand Down Expand Up @@ -87,6 +85,7 @@
import org.eclipse.jetty.util.component.Graceful;
import org.eclipse.jetty.util.component.LifeCycle;
import org.eclipse.jetty.util.resource.Resource;
import org.eclipse.jetty.util.security.SecurityUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -219,7 +218,7 @@ private enum ProtectedTargetType
private int _maxFormKeys = Integer.getInteger(MAX_FORM_KEYS_KEY, DEFAULT_MAX_FORM_KEYS);
private int _maxFormContentSize = Integer.getInteger(MAX_FORM_CONTENT_SIZE_KEY, DEFAULT_MAX_FORM_CONTENT_SIZE);
private boolean _compactPath = false;
private boolean _usingSecurityManager = System.getSecurityManager() != null;
private boolean _usingSecurityManager = getSecurityManager() != null;

private final List<EventListener> _programmaticListeners = new CopyOnWriteArrayList<>();
private final List<ServletContextListener> _servletContextListeners = new CopyOnWriteArrayList<>();
Expand Down Expand Up @@ -326,7 +325,7 @@ public boolean isUsingSecurityManager()

public void setUsingSecurityManager(boolean usingSecurityManager)
{
if (usingSecurityManager && System.getSecurityManager() == null)
if (usingSecurityManager && getSecurityManager() == null)
throw new IllegalStateException("No security manager");
_usingSecurityManager = usingSecurityManager;
}
Expand Down Expand Up @@ -2114,6 +2113,11 @@ public void clearAliasChecks()
_aliasChecks.clear();
}

private static Object getSecurityManager()
{
return SecurityUtils.getSecurityManager();
}

/**
* Context.
* <p>
Expand Down Expand Up @@ -2561,19 +2565,17 @@ public ClassLoader getClassLoader()
{
// check to see if the classloader of the caller is the same as the context
// classloader, or a parent of it, as required by the javadoc specification.

// Wrap in a PrivilegedAction so that only Jetty code will require the
// "createSecurityManager" permission, not also application code that calls this method.
Caller caller = AccessController.doPrivileged((PrivilegedAction<Caller>)Caller::new);
ClassLoader callerLoader = caller.getCallerClassLoader(2);
ClassLoader callerLoader = StackWalker.getInstance(StackWalker.Option.RETAIN_CLASS_REFERENCE)
.getCallerClass()
.getClassLoader();
while (callerLoader != null)
{
if (callerLoader == _classLoader)
return _classLoader;
else
callerLoader = callerLoader.getParent();
}
System.getSecurityManager().checkPermission(new RuntimePermission("getClassLoader"));
SecurityUtils.checkPermission(new RuntimePermission("getClassLoader"));
return _classLoader;
}
}
Expand Down Expand Up @@ -3103,17 +3105,4 @@ public static interface ContextScopeListener extends EventListener
*/
void exitScope(Context context, Request request);
}

private static class Caller extends SecurityManager
{
public ClassLoader getCallerClassLoader(int depth)
{
if (depth < 0)
return null;
Class<?>[] classContext = getClassContext();
if (classContext.length <= depth)
return null;
return classContext[depth].getClassLoader();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
import java.io.IOException;
import java.io.InputStream;
import java.net.URL;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.util.Arrays;
import java.util.Locale;
import java.util.Properties;
Expand Down Expand Up @@ -161,29 +159,26 @@ public TimeZone getTimeZone(String key)
*/
public JettyLoggerConfiguration load(ClassLoader loader)
{
return AccessController.doPrivileged((PrivilegedAction<JettyLoggerConfiguration>)() ->
// First see if the jetty-logging.properties object exists in the classpath.
// * This is an optional feature used by embedded mode use, and test cases to allow for early
// * configuration of the Log class in situations where access to the System.properties are
// * either too late or just impossible.
load(readProperties(loader, "jetty-logging.properties"));

// Next see if an OS specific jetty-logging.properties object exists in the classpath.
// This really for setting up test specific logging behavior based on OS.
String osName = System.getProperty("os.name");
if (osName != null && osName.length() > 0)
{
// First see if the jetty-logging.properties object exists in the classpath.
// * This is an optional feature used by embedded mode use, and test cases to allow for early
// * configuration of the Log class in situations where access to the System.properties are
// * either too late or just impossible.
load(readProperties(loader, "jetty-logging.properties"));

// Next see if an OS specific jetty-logging.properties object exists in the classpath.
// This really for setting up test specific logging behavior based on OS.
String osName = System.getProperty("os.name");
if (osName != null && osName.length() > 0)
{
// NOTE: cannot use jetty-util's StringUtil.replace() as it may initialize logging itself.
osName = osName.toLowerCase(Locale.ENGLISH).replace(' ', '-');
load(readProperties(loader, "jetty-logging-" + osName + ".properties"));
}
// NOTE: cannot use jetty-util's StringUtil.replace() as it may initialize logging itself.
osName = osName.toLowerCase(Locale.ENGLISH).replace(' ', '-');
load(readProperties(loader, "jetty-logging-" + osName + ".properties"));
}

// Now load the System.properties as-is into the properties,
// these values will override any key conflicts in properties.
load(System.getProperties());
return this;
});
// Now load the System.properties as-is into the properties,
// these values will override any key conflicts in properties.
load(System.getProperties());
return this;
}

public String getString(String key, String defValue)
Expand Down
14 changes: 2 additions & 12 deletions jetty-util/src/main/java/org/eclipse/jetty/util/MemoryUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@

package org.eclipse.jetty.util;

import java.security.AccessController;
import java.security.PrivilegedAction;

/**
* MemoryUtils provides an abstraction over memory properties and operations.
*/
Expand All @@ -25,18 +22,11 @@ public class MemoryUtils

static
{
final int defaultValue = 64;
int defaultValue = 64;
int value = defaultValue;
try
{
value = Integer.parseInt(AccessController.doPrivileged(new PrivilegedAction<String>()
{
@Override
public String run()
{
return System.getProperty("org.eclipse.jetty.util.cacheLineBytes", String.valueOf(defaultValue));
}
}));
value = Integer.parseInt(System.getProperty("org.eclipse.jetty.util.cacheLineBytes", String.valueOf(defaultValue)));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If running in < jdk 18 don't we still want to do the privileged action?

}
catch (Exception ignored)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.security.AccessController;
import java.security.CodeSource;
import java.security.PrivilegedAction;
import java.security.ProtectionDomain;
import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -671,7 +669,7 @@ public static URI getCodeSourceLocation(Class<?> clazz)
{
try
{
ProtectionDomain domain = AccessController.doPrivileged((PrivilegedAction<ProtectionDomain>)() -> clazz.getProtectionDomain());
ProtectionDomain domain = clazz.getProtectionDomain();
if (domain != null)
{
CodeSource source = domain.getCodeSource();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
//
// ========================================================================
// Copyright (c) 2023 Mort Bay Consulting Pty Ltd and others.
//
// This program and the accompanying materials are made available under the
// terms of the Eclipse Public License v. 2.0 which is available at
// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
// which is available at https://www.apache.org/licenses/LICENSE-2.0.
//
// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
// ========================================================================
//

package org.eclipse.jetty.util.security;

import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.security.Permission;
import java.security.PrivilegedAction;

/**
* <p>Collections of utility methods to deal with the scheduled removal
* of the security classes defined by <a href="https://openjdk.org/jeps/411">JEP 411</a>.</p>
*/
public class SecurityUtils
{
private static final MethodHandle doPrivileged = lookup();

private static MethodHandle lookup()
{
try
{
// Use reflection to work with Java versions that have and don't have AccessController.
Class<?> klass = ClassLoader.getPlatformClassLoader().loadClass("java.security.AccessController");
MethodHandles.Lookup lookup = MethodHandles.lookup();
return lookup.findStatic(klass, "doPrivileged", MethodType.methodType(Object.class, PrivilegedAction.class));
}
catch (Throwable x)
{
return null;
}
}

/**
* @return the current security manager, if available
*/
public static Object getSecurityManager()
{
try
{
// Use reflection to work with Java versions that have and don't have SecurityManager.
return System.class.getMethod("getSecurityManager").invoke(null);
}
catch (Throwable ignored)
{
return null;
}
}

/**
* <p>Checks the given permission, if the {@link #getSecurityManager() security manager}
* is set.</p>
*
* @param permission the permission to check
* @throws SecurityException if the permission check fails
*/
public static void checkPermission(Permission permission) throws SecurityException
{
Object securityManager = SecurityUtils.getSecurityManager();
if (securityManager == null)
return;
try
{
securityManager.getClass().getMethod("checkPermission")
.invoke(securityManager, permission);
}
catch (SecurityException x)
{
throw x;
}
catch (Throwable ignored)
{
}
}

/**
* <p>Runs the given action with the calling context restricted
* to just the calling frame, not all the frames in the stack.</p>
*
* @param action the action to run
* @return the result of running the action
* @param <T> the type of the result
*/
public static <T> T doPrivileged(PrivilegedAction<T> action)
{
// Keep this method short and inlineable.
MethodHandle methodHandle = doPrivileged;
if (methodHandle == null)
return action.run();
return doPrivileged(methodHandle, action);
}

@SuppressWarnings("unchecked")
private static <T> T doPrivileged(MethodHandle doPrivileged, PrivilegedAction<T> action)
{
try
{
return (T)doPrivileged.invoke(action);
}
catch (RuntimeException | Error x)
{
throw x;
}
catch (Throwable x)
{
throw new RuntimeException(x);
}
}

private SecurityUtils()
{
}
}