stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

6721651: Security problem with out-of-the-box management

Browse Source 
Reviewed-by: emcmanus, lmalvent
This commit is contained in:
Daniel Fuchs 2009-03-09 23:50:11 +01:00
parent 6c11535cdd
commit 576a962dcb
3 changed files with 389 additions and 86 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
jdk/src/share/classes/com/sun/jmx/remote/security/MBeanServerAccessController.java
View File

@ -111,6 +111,22 @@ public abstract class MBeanServerAccessController
*/
protected abstract void checkWrite();
/**
* Check if the caller can create the named class. The default
* implementation of this method calls {@link #checkWrite()}.
*/
protected void checkCreate(String className) {
checkWrite();
}
/**
* Check if the caller can unregister the named MBean. The default
* implementation of this method calls {@link #checkWrite()}.
*/
protected void checkUnregister(ObjectName name) {
checkWrite();
}
//--------------------------------------------
//--------------------------------------------
//
@ -148,7 +164,7 @@ public abstract class MBeanServerAccessController
}
/**
* Call <code>checkWrite()</code>, then forward this method to the
* Call <code>checkCreate(className)</code>, then forward this method to the
* wrapped object.
*/
public ObjectInstance createMBean(String className, ObjectName name)
@ -158,7 +174,7 @@ public abstract class MBeanServerAccessController
MBeanRegistrationException,
MBeanException,
NotCompliantMBeanException {
checkWrite();
checkCreate(className);
SecurityManager sm = System.getSecurityManager();
if (sm == null) {
Object object = getMBeanServer().instantiate(className);
@ -170,7 +186,7 @@ public abstract class MBeanServerAccessController
}
/**
* Call <code>checkWrite()</code>, then forward this method to the
* Call <code>checkCreate(className)</code>, then forward this method to the
* wrapped object.
*/
public ObjectInstance createMBean(String className, ObjectName name,
@ -181,7 +197,7 @@ public abstract class MBeanServerAccessController
MBeanRegistrationException,
MBeanException,
NotCompliantMBeanException {
checkWrite();
checkCreate(className);
SecurityManager sm = System.getSecurityManager();
if (sm == null) {
Object object = getMBeanServer().instantiate(className,
@ -196,7 +212,7 @@ public abstract class MBeanServerAccessController
}
/**
* Call <code>checkWrite()</code>, then forward this method to the
* Call <code>checkCreate(className)</code>, then forward this method to the
* wrapped object.
*/
public ObjectInstance createMBean(String className,
@ -209,7 +225,7 @@ public abstract class MBeanServerAccessController
MBeanException,
NotCompliantMBeanException,
InstanceNotFoundException {
checkWrite();
checkCreate(className);
SecurityManager sm = System.getSecurityManager();
if (sm == null) {
Object object = getMBeanServer().instantiate(className,
@ -222,7 +238,7 @@ public abstract class MBeanServerAccessController
}
/**
* Call <code>checkWrite()</code>, then forward this method to the
* Call <code>checkCreate(className)</code>, then forward this method to the
* wrapped object.
*/
public ObjectInstance createMBean(String className,
@ -237,7 +253,7 @@ public abstract class MBeanServerAccessController
MBeanException,
NotCompliantMBeanException,
InstanceNotFoundException {
checkWrite();
checkCreate(className);
SecurityManager sm = System.getSecurityManager();
if (sm == null) {
Object object = getMBeanServer().instantiate(className,
@ -394,45 +410,45 @@ public abstract class MBeanServerAccessController
}
/**
* Call <code>checkWrite()</code>, then forward this method to the
* Call <code>checkCreate(className)</code>, then forward this method to the
* wrapped object.
*/
public Object instantiate(String className)
throws ReflectionException, MBeanException {
checkWrite();
checkCreate(className);
return getMBeanServer().instantiate(className);
}
/**
* Call <code>checkWrite()</code>, then forward this method to the
* Call <code>checkCreate(className)</code>, then forward this method to the
* wrapped object.
*/
public Object instantiate(String className,
Object params[],
String signature[])
throws ReflectionException, MBeanException {
checkWrite();
checkCreate(className);
return getMBeanServer().instantiate(className, params, signature);
}
/**
* Call <code>checkWrite()</code>, then forward this method to the
* Call <code>checkCreate(className)</code>, then forward this method to the
* wrapped object.
*/
public Object instantiate(String className, ObjectName loaderName)
throws ReflectionException, MBeanException, InstanceNotFoundException {
checkWrite();
checkCreate(className);
return getMBeanServer().instantiate(className, loaderName);
}
/**
* Call <code>checkWrite()</code>, then forward this method to the
* Call <code>checkCreate(className)</code>, then forward this method to the
* wrapped object.
*/
public Object instantiate(String className, ObjectName loaderName,
Object params[], String signature[])
throws ReflectionException, MBeanException, InstanceNotFoundException {
checkWrite();
checkCreate(className);
return getMBeanServer().instantiate(className, loaderName,
params, signature);
}
@ -579,12 +595,12 @@ public abstract class MBeanServerAccessController
}
/**
* Call <code>checkWrite()</code>, then forward this method to the
* Call <code>checkUnregister()</code>, then forward this method to the
* wrapped object.
*/
public void unregisterMBean(ObjectName name)
throws InstanceNotFoundException, MBeanRegistrationException {
checkWrite();
checkUnregister(name);
getMBeanServer().unregisterMBean(name);
}

376
jdk/src/share/classes/com/sun/jmx/remote/security/MBeanServerFileAccessController.java
View File

@ -31,11 +31,17 @@ import java.security.AccessControlContext;
import java.security.AccessController;
import java.security.Principal;
import java.security.PrivilegedAction;
import java.util.Collection;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Properties;
import java.util.Set;
import java.util.StringTokenizer;
import java.util.regex.Pattern;
import javax.management.MBeanServer;
import javax.management.ObjectName;
import javax.security.auth.Subject;
/**
@ -46,7 +52,8 @@ import javax.security.auth.Subject;
* not allowed; in this case the request is not forwarded to the
* wrapped object.</p>
*
* <p>This class implements the {@link #checkRead()} and {@link #checkWrite()}
* <p>This class implements the {@link #checkRead()}, {@link #checkWrite()},
* {@link #checkCreate(String)}, and {@link #checkUnregister(ObjectName)}
* methods based on an access level properties file containing username/access
* level pairs. The set of username/access level pairs is passed either as a
* filename which denotes a properties file on disk, or directly as an instance
@ -56,14 +63,50 @@ import javax.security.auth.Subject;
* has exactly one access level. The same access level can be shared by several
* usernames.</p>
*
* <p>The supported access level values are <i>readonly</i> and
* <i>readwrite</i>.</p>
* <p>The supported access level values are {@code readonly} and
* {@code readwrite}. The {@code readwrite} access level can be
* qualified by one or more <i>clauses</i>, where each clause looks
* like <code>create <i>classNamePattern</i></code> or {@code
* unregister}. For example:</p>
*
* <pre>
* monitorRole readonly
* controlRole readwrite \
* create javax.management.timer.*,javax.management.monitor.* \
* unregister
* </pre>
*
* <p>(The continuation lines with {@code \} come from the parser for
* Properties files.)</p>
*/
public class MBeanServerFileAccessController
extends MBeanServerAccessController {
public static final String READONLY = "readonly";
public static final String READWRITE = "readwrite";
static final String READONLY = "readonly";
static final String READWRITE = "readwrite";
static final String CREATE = "create";
static final String UNREGISTER = "unregister";
private enum AccessType {READ, WRITE, CREATE, UNREGISTER};
private static class Access {
final boolean write;
final String[] createPatterns;
private boolean unregister;
Access(boolean write, boolean unregister, List<String> createPatternList) {
this.write = write;
int npats = (createPatternList == null) ? 0 : createPatternList.size();
if (npats == 0)
this.createPatterns = NO_STRINGS;
else
this.createPatterns = createPatternList.toArray(new String[npats]);
this.unregister = unregister;
}
private final String[] NO_STRINGS = new String[0];
}
/**
* <p>Create a new MBeanServerAccessController that forwards all the
@ -87,8 +130,8 @@ public class MBeanServerFileAccessController
throws IOException {
super();
this.accessFileName = accessFileName;
props = propertiesFromFile(accessFileName);
checkValues(props);
Properties props = propertiesFromFile(accessFileName);
parseProperties(props);
}
/**
@ -123,14 +166,14 @@ public class MBeanServerFileAccessController
* #setMBeanServer} method after doing access checks based on read and
* write permissions.</p>
*
* <p>This instance is initialized from the specified properties instance.
* This constructor makes a copy of the properties instance using its
* <code>clone</code> method and it is the copy that is consulted to check
* the username and access level of an incoming connection. The original
* properties object can be modified without affecting the copy. If the
* {@link #refresh} method is then called, the
* <code>MBeanServerFileAccessController</code> will make a new copy of the
* properties object at that time.</p>
* <p>This instance is initialized from the specified properties
* instance. This constructor makes a copy of the properties
* instance and it is the copy that is consulted to check the
* username and access level of an incoming connection. The
* original properties object can be modified without affecting
* the copy. If the {@link #refresh} method is then called, the
* <code>MBeanServerFileAccessController</code> will make a new
* copy of the properties object at that time.</p>
*
* @param accessFileProps properties list containing the username/access
* level entries.
@ -145,8 +188,7 @@ public class MBeanServerFileAccessController
if (accessFileProps == null)
throw new IllegalArgumentException("Null properties");
originalProps = accessFileProps;
props = (Properties) accessFileProps.clone();
checkValues(props);
parseProperties(accessFileProps);
}
/**
@ -155,14 +197,14 @@ public class MBeanServerFileAccessController
* #setMBeanServer} method after doing access checks based on read and
* write permissions.</p>
*
* <p>This instance is initialized from the specified properties instance.
* This constructor makes a copy of the properties instance using its
* <code>clone</code> method and it is the copy that is consulted to check
* the username and access level of an incoming connection. The original
* properties object can be modified without affecting the copy. If the
* {@link #refresh} method is then called, the
* <code>MBeanServerFileAccessController</code> will make a new copy of the
* properties object at that time.</p>
* <p>This instance is initialized from the specified properties
* instance. This constructor makes a copy of the properties
* instance and it is the copy that is consulted to check the
* username and access level of an incoming connection. The
* original properties object can be modified without affecting
* the copy. If the {@link #refresh} method is then called, the
* <code>MBeanServerFileAccessController</code> will make a new
* copy of the properties object at that time.</p>
*
* @param accessFileProps properties list containing the username/access
* level entries.
@ -184,16 +226,36 @@ public class MBeanServerFileAccessController
* Check if the caller can do read operations. This method does
* nothing if so, otherwise throws SecurityException.
*/
@Override
public void checkRead() {
checkAccessLevel(READONLY);
checkAccess(AccessType.READ, null);
}
/**
* Check if the caller can do write operations. This method does
* nothing if so, otherwise throws SecurityException.
*/
@Override
public void checkWrite() {
checkAccessLevel(READWRITE);
checkAccess(AccessType.WRITE, null);
}
/**
* Check if the caller can create MBeans or instances of the given class.
* This method does nothing if so, otherwise throws SecurityException.
*/
@Override
public void checkCreate(String className) {
checkAccess(AccessType.CREATE, className);
}
/**
* Check if the caller can do unregister operations. This method does
* nothing if so, otherwise throws SecurityException.
*/
@Override
public void checkUnregister(ObjectName name) {
checkAccess(AccessType.UNREGISTER, null);
}
/**
@ -218,14 +280,13 @@ public class MBeanServerFileAccessController
* @exception IllegalArgumentException if any of the supplied access
* level values differs from "readonly" or "readwrite".
*/
public void refresh() throws IOException {
synchronized (props) {
if (accessFileName == null)
props = (Properties) originalProps.clone();
else
props = propertiesFromFile(accessFileName);
checkValues(props);
}
public synchronized void refresh() throws IOException {
Properties props;
if (accessFileName == null)
props = (Properties) originalProps;
else
props = propertiesFromFile(accessFileName);
parseProperties(props);
}
private static Properties propertiesFromFile(String fname)
@ -240,7 +301,7 @@ public class MBeanServerFileAccessController
}
}
private void checkAccessLevel(String accessLevel) {
private synchronized void checkAccess(AccessType requiredAccess, String arg) {
final AccessControlContext acc = AccessController.getContext();
final Subject s =
AccessController.doPrivileged(new PrivilegedAction<Subject>() {
@ -250,39 +311,234 @@ public class MBeanServerFileAccessController
});
if (s == null) return; /* security has not been enabled */
final Set principals = s.getPrincipals();
String newPropertyValue = null;
for (Iterator i = principals.iterator(); i.hasNext(); ) {
final Principal p = (Principal) i.next();
String grantedAccessLevel;
synchronized (props) {
grantedAccessLevel = props.getProperty(p.getName());
}
if (grantedAccessLevel != null) {
if (accessLevel.equals(READONLY) &&
(grantedAccessLevel.equals(READONLY) ||
grantedAccessLevel.equals(READWRITE)))
return;
if (accessLevel.equals(READWRITE) &&
grantedAccessLevel.equals(READWRITE))
Access access = accessMap.get(p.getName());
if (access != null) {
boolean ok;
switch (requiredAccess) {
case READ:
ok = true; // all access entries imply read
break;
case WRITE:
ok = access.write;
break;
case UNREGISTER:
ok = access.unregister;
if (!ok && access.write)
newPropertyValue = "unregister";
break;
case CREATE:
ok = checkCreateAccess(access, arg);
if (!ok && access.write)
newPropertyValue = "create " + arg;
break;
default:
throw new AssertionError();
}
if (ok)
return;
}
}
throw new SecurityException("Access denied! Invalid access level for " +
"requested MBeanServer operation.");
SecurityException se = new SecurityException("Access denied! Invalid " +
"access level for requested MBeanServer operation.");
// Add some more information to help people with deployments that
// worked before we required explicit create clauses. We're not giving
// any information to the bad guys, other than that the access control
// is based on a file, which they could have worked out from the stack
// trace anyway.
if (newPropertyValue != null) {
SecurityException se2 = new SecurityException("Access property " +
"for this identity should be similar to: " + READWRITE +
" " + newPropertyValue);
se.initCause(se2);
}
throw se;
}
private void checkValues(Properties props) {
Collection c = props.values();
for (Iterator i = c.iterator(); i.hasNext(); ) {
final String accessLevel = (String) i.next();
if (!accessLevel.equals(READONLY) &&
!accessLevel.equals(READWRITE)) {
throw new IllegalArgumentException(
"Syntax error in access level entry [" + accessLevel + "]");
}
private static boolean checkCreateAccess(Access access, String className) {
for (String classNamePattern : access.createPatterns) {
if (classNameMatch(classNamePattern, className))
return true;
}
return false;
}
private static boolean classNameMatch(String pattern, String className) {
// We studiously avoided regexes when parsing the properties file,
// because that is done whenever the VM is started with the
// appropriate -Dcom.sun.management options, even if nobody ever
// creates an MBean. We don't want to incur the overhead of loading
// all the regex code whenever those options are specified, but if we
// get as far as here then the VM is already running and somebody is
// doing the very unusual operation of remotely creating an MBean.
// Because that operation is so unusual, we don't try to optimize
// by hand-matching or by caching compiled Pattern objects.
StringBuilder sb = new StringBuilder();
StringTokenizer stok = new StringTokenizer(pattern, "*", true);
while (stok.hasMoreTokens()) {
String tok = stok.nextToken();
if (tok.equals("*"))
sb.append("[^.]*");
else
sb.append(Pattern.quote(tok));
}
return className.matches(sb.toString());
}
private void parseProperties(Properties props) {
this.accessMap = new HashMap<String, Access>();
for (Map.Entry<Object, Object> entry : props.entrySet()) {
String identity = (String) entry.getKey();
String accessString = (String) entry.getValue();
Access access = Parser.parseAccess(identity, accessString);
accessMap.put(identity, access);
}
}
private Properties props;
private static class Parser {
private final static int EOS = -1; // pseudo-codepoint "end of string"
static {
assert !Character.isWhitespace(EOS);
}
private final String identity; // just for better error messages
private final String s; // the string we're parsing
private final int len; // s.length()
private int i;
private int c;
// At any point, either c is s.codePointAt(i), or i == len and
// c is EOS. We use int rather than char because it is conceivable
// (if unlikely) that a classname in a create clause might contain
// "supplementary characters", the ones that don't fit in the original
// 16 bits for Unicode.
private Parser(String identity, String s) {
this.identity = identity;
this.s = s;
this.len = s.length();
this.i = 0;
if (i < len)
this.c = s.codePointAt(i);
else
this.c = EOS;
}
static Access parseAccess(String identity, String s) {
return new Parser(identity, s).parseAccess();
}
private Access parseAccess() {
skipSpace();
String type = parseWord();
Access access;
if (type.equals(READONLY))
access = new Access(false, false, null);
else if (type.equals(READWRITE))
access = parseReadWrite();
else {
throw syntax("Expected " + READONLY + " or " + READWRITE +
": " + type);
}
if (c != EOS)
throw syntax("Extra text at end of line");
return access;
}
private Access parseReadWrite() {
List<String> createClasses = new ArrayList<String>();
boolean unregister = false;
while (true) {
skipSpace();
if (c == EOS)
break;
String type = parseWord();
if (type.equals(UNREGISTER))
unregister = true;
else if (type.equals(CREATE))
parseCreate(createClasses);
else
throw syntax("Unrecognized keyword " + type);
}
return new Access(true, unregister, createClasses);
}
private void parseCreate(List<String> createClasses) {
while (true) {
skipSpace();
createClasses.add(parseClassName());
skipSpace();
if (c == ',')
next();
else
break;
}
}
private String parseClassName() {
// We don't check that classname components begin with suitable
// characters (so we accept 1.2.3 for example). This means that
// there are only two states, which we can call dotOK and !dotOK
// according as a dot (.) is legal or not. Initially we're in
// !dotOK since a classname can't start with a dot; after a dot
// we're in !dotOK again; and after any other characters we're in
// dotOK. The classname is only accepted if we end in dotOK,
// so we reject an empty name or a name that ends with a dot.
final int start = i;
boolean dotOK = false;
while (true) {
if (c == '.') {
if (!dotOK)
throw syntax("Bad . in class name");
dotOK = false;
} else if (c == '*' || Character.isJavaIdentifierPart(c))
dotOK = true;
else
break;
next();
}
String className = s.substring(start, i);
if (!dotOK)
throw syntax("Bad class name " + className);
return className;
}
// Advance c and i to the next character, unless already at EOS.
private void next() {
if (c != EOS) {
i += Character.charCount(c);
if (i < len)
c = s.codePointAt(i);
else
c = EOS;
}
}
private void skipSpace() {
while (Character.isWhitespace(c))
next();
}
private String parseWord() {
skipSpace();
if (c == EOS)
throw syntax("Expected word at end of line");
final int start = i;
while (c != EOS && !Character.isWhitespace(c))
next();
String word = s.substring(start, i);
skipSpace();
return word;
}
private IllegalArgumentException syntax(String msg) {
return new IllegalArgumentException(
msg + " [" + identity + " " + s + "]");
}
}
private Map<String, Access> accessMap;
private Properties originalProps;
private String accessFileName;
}

47
jdk/src/share/lib/management/jmxremote.access
View File

@ -8,7 +8,7 @@
# passwords. To be functional, a role must have an entry in
# both the password and the access files.
#
# Default location of this file is $JRE/lib/management/jmxremote.access
# The default location of this file is $JRE/lib/management/jmxremote.access
# You can specify an alternate location by specifying a property in
# the management config file $JRE/lib/management/management.properties
# (See that file for details)
@ -16,7 +16,7 @@
# The file format for password and access files is syntactically the same
# as the Properties file format. The syntax is described in the Javadoc
# for java.util.Properties.load.
# Typical access file has multiple lines, where each line is blank,
# A typical access file has multiple lines, where each line is blank,
# a comment (like this one), or an access control entry.
#
# An access control entry consists of a role name, and an
@ -29,10 +29,38 @@
# role can read measurements but cannot perform any action
# that changes the environment of the running program.
# "readwrite" grants access to read and write attributes of MBeans,
# to invoke operations on them, and to create or remove them.
# This access should be granted to only trusted clients,
# since they can potentially interfere with the smooth
# operation of a running program
# to invoke operations on them, and optionally
# to create or remove them. This access should be granted
# only to trusted clients, since they can potentially
# interfere with the smooth operation of a running program.
#
# The "readwrite" access level can optionally be followed by the "create" and/or
# "unregister" keywords. The "unregister" keyword grants access to unregister
# (delete) MBeans. The "create" keyword grants access to create MBeans of a
# particular class or of any class matching a particular pattern. Access
# should only be granted to create MBeans of known and trusted classes.
#
# For example, the following entry would grant readwrite access
# to "controlRole", as well as access to create MBeans of the class
# javax.management.monitor.CounterMonitor and to unregister any MBean:
# controlRole readwrite \
# create javax.management.monitor.CounterMonitorMBean \
# unregister
# or equivalently:
# controlRole readwrite unregister create javax.management.monitor.CounterMBean
#
# The following entry would grant readwrite access as well as access to create
# MBeans of any class in the packages javax.management.monitor and
# javax.management.timer:
# controlRole readwrite \
# create javax.management.monitor.*,javax.management.timer.* \
# unregister
#
# The \ character is defined in the Properties file syntax to allow continuation
# lines as shown here. A * in a class pattern matches a sequence of characters
# other than dot (.), so javax.management.monitor.* matches
# javax.management.monitor.CounterMonitor but not
# javax.management.monitor.foo.Bar.
#
# A given role should have at most one entry in this file. If a role
# has no entry, it has no access.
@ -42,7 +70,10 @@
#
# Default access control entries:
# o The "monitorRole" role has readonly access.
# o The "controlRole" role has readwrite access.
# o The "controlRole" role has readwrite access and can create the standard
# Timer and Monitor MBeans defined by the JMX API.
monitorRole readonly
controlRole readwrite
controlRole readwrite \
create javax.management.monitor.*,javax.management.timer.* \
unregister