Skip to content

Commit

Permalink
[WFLY-490][WFLY-456] Fixes to JMX rbac/audit log coming from review
Browse files Browse the repository at this point in the history
  • Loading branch information
kabir authored and bstansberry committed Aug 28, 2013
1 parent b1fc489 commit 463453c
Show file tree
Hide file tree
Showing 29 changed files with 533 additions and 264 deletions.
2 changes: 1 addition & 1 deletion build/src/main/resources/docs/schema/jboss-as-jmx_1_3.xsd
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@
Configures access control sensititity for the mbeans in the jmx subsystem
</xs:documentation>
</xs:annotation>
<xs:attribute name="core-mbeans" type="xs:boolean" use="optional" default="false"/>
<xs:attribute name="non-core-mbeans" type="xs:boolean" use="optional" default="false"/>
</xs:complexType>

</xs:schema>
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ private AbstractControllerService(final ProcessType processType, final RunningMo
assert rootDescriptionProvider != null || rootResourceDefinition != null: rootDescriptionProvider == null ? "Null root description provider" : "Null root resource definition";
assert expressionResolver != null : "Null expressionResolver";
assert auditLogger != null : "Null auditLogger";
assert authorizer != null : "Null authorizer";
this.processType = processType;
this.runningModeControl = runningModeControl;
this.configurationPersister = configurationPersister;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,14 @@ public interface Authorizer {

AuthorizationResult authorize(Caller caller, Environment callEnvironment, Action action, TargetAttribute target);
AuthorizationResult authorize(Caller caller, Environment callEnvironment, Action action, TargetResource target);
AuthorizationResult authorizeJmxOperation(Caller caller, Environment callEnvironment, JmxTarget target);

/**
* Authorize a JMX operation. This operation should NOT be called for the non-management facade MBeans
*
* @param caller the caller
* @param callEnvironment the call environment
* @param action
* @return the authorization result
*/
AuthorizationResult authorizeJmxOperation(Caller caller, Environment callEnvironment, JmxAction action);
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,6 @@ public interface ConfigurableAuthorizer extends Authorizer {
void addScopedRole(String roleName, String baseRole, ScopingConstraint scopingConstraint);

void removeScopedRole(String roleName);

void setNonFacadeMBeansSensitive(boolean sensitive);
}
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,13 @@ public AuthorizationResult authorize(Caller caller, Environment callEnvironment,
return delegate.authorize(caller, callEnvironment, action, target);
}

public AuthorizationResult authorizeJmxOperation(Caller caller, Environment callEnvironment, JmxTarget target) {
return delegate.authorizeJmxOperation(caller, callEnvironment, target);
public AuthorizationResult authorizeJmxOperation(Caller caller, Environment callEnvironment, JmxAction action) {
return delegate.authorizeJmxOperation(caller, callEnvironment, action);
}

@Override
public void setNonFacadeMBeansSensitive(boolean sensitive) {
delegate.setNonFacadeMBeansSensitive(sensitive);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
* JBoss, Home of Professional Open Source.
* Copyright 2012, Red Hat, Inc., and individual contributors
* as indicated by the @author tags. See the copyright.txt file in the
* distribution for a full listing of individual contributors.
*
* This is free software; you can redistribute it and/or modify it
* under the terms of the GNU Lesser General Public License as
* published by the Free Software Foundation; either version 2.1 of
* the License, or (at your option) any later version.
*
* This software is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this software; if not, write to the Free
* Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
* 02110-1301 USA, or see the FSF site: http://www.fsf.org.
*/
package org.jboss.as.controller.access;


/**
* Encapsulates authorization information about an MBean call.
*
* @author <a href="[email protected]">Kabir Khan</a>
*/
public class JmxAction {
private final String methodName;
private final Impact impact;

public JmxAction(String methodName, Impact impact) {
this.methodName = methodName;
this.impact = impact;
}

/**
* Gets the impact of the call
*
* @return the impact
*/
public Impact getImpact() {
return impact;
}

/**
* Gets the {@link javax.management.MBeanServer} method name that was called
*/
public String getMethodName() {
return methodName;
}

/**
* The impact of the call
*/
public enum Impact {
/** The call is read-only */
READ_ONLY,
/** The call writes data */
WRITE,
/** The call is special, and will normally only work for a (@link org.jboss.as.controller.access.rbac.StandardRole#SUPERUSER} or a (@link org.jboss.as.controller.access.rbac.StandardRole#ADMINISTRATOR} */
EXTRA_SENSITIVE
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,13 @@
import java.util.LinkedHashSet;
import java.util.Set;

import org.jboss.as.controller.PathAddress;
import org.jboss.as.controller.access.AuthorizationResult.Decision;
import org.jboss.as.controller.access.constraint.ScopingConstraint;
import org.jboss.as.controller.access.permission.CombinationPolicy;
import org.jboss.as.controller.access.permission.ManagementPermissionAuthorizer;
import org.jboss.as.controller.access.rbac.DefaultPermissionFactory;
import org.jboss.as.controller.access.rbac.MockRoleMapper;
import org.jboss.as.controller.access.rbac.RoleMapper;
import org.jboss.as.controller.access.rbac.StandardRole;
import org.jboss.as.controller.operations.common.Util;

/**
* Simple {@link ConfigurableAuthorizer} implementation that gives all permissions to any authenticated
Expand All @@ -59,8 +56,6 @@ public class SimpleConfigurableAuthorizer implements ConfigurableAuthorizer {
private final DefaultPermissionFactory permissionFactory;
private final Authorizer authorizer;
private final Set<String> addedRoles = new HashSet<String>();
/** A fake action for JMX, just to have an operation with superuser permissions */
private static final Action FAKE_JMX_ACTION = new Action(Util.createOperation("test", PathAddress.EMPTY_ADDRESS), null);

public SimpleConfigurableAuthorizer() {
this(MockRoleMapper.INSTANCE);
Expand All @@ -69,7 +64,7 @@ public SimpleConfigurableAuthorizer() {
public SimpleConfigurableAuthorizer(final RoleMapper roleMapper) {
this.roleMapper = roleMapper;
permissionFactory = new DefaultPermissionFactory(CombinationPolicy.PERMISSIVE, roleMapper);
authorizer = new ManagementPermissionAuthorizer(permissionFactory);
authorizer = new ManagementPermissionAuthorizer(permissionFactory, permissionFactory);
}

@Override
Expand Down Expand Up @@ -122,27 +117,12 @@ public AuthorizationResult authorize(Caller caller, Environment callEnvironment,
}

@Override
public AuthorizationResult authorizeJmxOperation(Caller caller, Environment callEnvironment, JmxTarget target) {
Set<String> roles = roleMapper.mapRoles(caller, null, FAKE_JMX_ACTION, (TargetAttribute) null);
if (target.isNonFacadeMBeansSensitive() || target.isSuperUserOrAdminOnly()) {
return authorize(roles, StandardRole.SUPERUSER, StandardRole.ADMINISTRATOR);
} else {
if (target.isReadOnly()) {
//Everybody can read mbeans when not sensitive
return AuthorizationResult.PERMITTED;
//authorize(exception, roles, StandardRole.SUPERUSER, StandardRole.ADMINISTRATOR, StandardRole.OPERATOR, StandardRole.MAINTAINER, StandardRole.AUDITOR, StandardRole.MONITOR, StandardRole.DEPLOYER);
} else {
return authorize(roles, StandardRole.SUPERUSER, StandardRole.ADMINISTRATOR, StandardRole.OPERATOR, StandardRole.MAINTAINER);
}
}
public AuthorizationResult authorizeJmxOperation(Caller caller, Environment callEnvironment, JmxAction action) {
return authorizer.authorizeJmxOperation(caller, callEnvironment, action);
}

private AuthorizationResult authorize(Set<String> callerRoles, StandardRole...roles) {
for (StandardRole role : roles) {
if (callerRoles.contains(role.toString())) {
return AuthorizationResult.PERMITTED;
}
}
return new AuthorizationResult(Decision.DENY);
@Override
public void setNonFacadeMBeansSensitive(boolean sensitive) {
permissionFactory.setNonFacadeMBeansSensitive(sensitive);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,33 +19,20 @@
* Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
* 02110-1301 USA, or see the FSF site: http://www.fsf.org.
*/
package org.jboss.as.controller.access;
package org.jboss.as.controller.access.permission;

import java.util.Set;

import org.jboss.as.controller.access.Action;
import org.jboss.as.controller.access.Caller;
import org.jboss.as.controller.access.Environment;
import org.jboss.as.controller.access.TargetResource;

/**
*
* @author <a href="[email protected]">Kabir Khan</a>
*/
public class JmxTarget {
final boolean superUserOrAdminOnly;
final boolean nonFacadeMBeansSensitive;
final boolean readOnly;

public JmxTarget(boolean superUserOrAdminOnly, boolean nonFacadeMBeansSensitive, boolean readOnly) {
this.superUserOrAdminOnly = superUserOrAdminOnly;
this.nonFacadeMBeansSensitive = nonFacadeMBeansSensitive;
this.readOnly = readOnly;
}

boolean isSuperUserOrAdminOnly() {
return superUserOrAdminOnly;
}

boolean isNonFacadeMBeansSensitive() {
return nonFacadeMBeansSensitive;
}

public boolean isReadOnly() {
return readOnly;
}
public interface JmxPermissionFactory {
boolean isNonFacadeMBeansSensitive();
Set<String> getUserRoles(Caller caller, Environment callEnvironment, Action action, TargetResource target);
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,22 @@
import java.security.Permission;
import java.security.PermissionCollection;
import java.util.Enumeration;
import java.util.Set;

import org.jboss.as.controller.ControlledProcessState;
import org.jboss.as.controller.PathAddress;
import org.jboss.as.controller.access.Action;
import org.jboss.as.controller.access.AuthorizationResult;
import org.jboss.as.controller.access.AuthorizationResult.Decision;
import org.jboss.as.controller.access.Authorizer;
import org.jboss.as.controller.access.Caller;
import org.jboss.as.controller.access.Environment;
import org.jboss.as.controller.access.JmxTarget;
import org.jboss.as.controller.access.JmxAction;
import org.jboss.as.controller.access.JmxAction.Impact;
import org.jboss.as.controller.access.TargetAttribute;
import org.jboss.as.controller.access.TargetResource;
import org.jboss.as.controller.access.rbac.StandardRole;
import org.jboss.as.controller.operations.common.Util;
import org.jboss.dmr.ModelNode;

/**
Expand All @@ -44,10 +50,15 @@
*/
public class ManagementPermissionAuthorizer implements Authorizer {

/** A fake action for JMX, just to have an operation with superuser permissions */
private static final Action FAKE_JMX_ACTION = new Action(Util.createOperation("test", PathAddress.EMPTY_ADDRESS), null);

private final PermissionFactory permissionFactory;
private final JmxPermissionFactory jmxPermissionFactory;

public ManagementPermissionAuthorizer(PermissionFactory permissionFactory) {
public ManagementPermissionAuthorizer(PermissionFactory permissionFactory, JmxPermissionFactory jmxPermissionFactory) {
this.permissionFactory = permissionFactory;
this.jmxPermissionFactory = jmxPermissionFactory;
}

@Override
Expand Down Expand Up @@ -86,8 +97,26 @@ private AuthorizationResult authorize(PermissionCollection userPermissions, Perm
}

@Override
public AuthorizationResult authorizeJmxOperation(Caller caller, Environment callEnvironment, JmxTarget target) {
//We should never end up here?
throw new IllegalStateException();
public AuthorizationResult authorizeJmxOperation(Caller caller, Environment callEnvironment, JmxAction action) {
Set<String> roles = jmxPermissionFactory.getUserRoles(caller, null, FAKE_JMX_ACTION, (TargetResource) null);
if (jmxPermissionFactory.isNonFacadeMBeansSensitive() || action.getImpact() == Impact.EXTRA_SENSITIVE) {
return authorize(roles, StandardRole.SUPERUSER, StandardRole.ADMINISTRATOR);
} else {
if (action.getImpact() == Impact.READ_ONLY) {
//Everybody can read mbeans when not sensitive
return AuthorizationResult.PERMITTED;
//authorize(exception, roles, StandardRole.SUPERUSER, StandardRole.ADMINISTRATOR, StandardRole.OPERATOR, StandardRole.MAINTAINER, StandardRole.AUDITOR, StandardRole.MONITOR, StandardRole.DEPLOYER);
} else {
return authorize(roles, StandardRole.SUPERUSER, StandardRole.ADMINISTRATOR, StandardRole.OPERATOR, StandardRole.MAINTAINER);
}
}
}
}

private AuthorizationResult authorize(Set<String> callerRoles, StandardRole...roles) {
for (StandardRole role : roles) {
if (callerRoles.contains(role.toString())) {
return AuthorizationResult.PERMITTED;
}
}
return new AuthorizationResult(Decision.DENY);
}}
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import org.jboss.as.controller.access.constraint.ServerGroupEffectConstraint;
import org.jboss.as.controller.access.permission.CombinationManagementPermission;
import org.jboss.as.controller.access.permission.CombinationPolicy;
import org.jboss.as.controller.access.permission.JmxPermissionFactory;
import org.jboss.as.controller.access.permission.ManagementPermission;
import org.jboss.as.controller.access.permission.ManagementPermissionCollection;
import org.jboss.as.controller.access.permission.PermissionFactory;
Expand All @@ -64,7 +65,7 @@
*
* @author Brian Stansberry (c) 2013 Red Hat Inc.
*/
public class DefaultPermissionFactory implements PermissionFactory {
public class DefaultPermissionFactory implements PermissionFactory, JmxPermissionFactory {

private static final PermissionCollection NO_PERMISSIONS = new NoPermissionsCollection();
private final CombinationPolicy combinationPolicy;
Expand All @@ -73,6 +74,7 @@ public class DefaultPermissionFactory implements PermissionFactory {
private final Map<String, ManagementPermissionCollection> permissionsByRole = new HashMap<String, ManagementPermissionCollection>();
private final Map<String, ScopedBase> scopedBaseMap = new HashMap<String, ScopedBase>();
private boolean rolePermissionsConfigured;
private volatile boolean nonFacadeMBeansSensitive;

public DefaultPermissionFactory(CombinationPolicy combinationPolicy, RoleMapper roleMapper) {
this(combinationPolicy, roleMapper, getStandardConstraintFactories());
Expand All @@ -96,6 +98,19 @@ public PermissionCollection getUserPermissions(Caller caller, Environment callEn
return getUserPermissions(roleMapper.mapRoles(caller, callEnvironment, action, target));
}

@Override
public Set<String> getUserRoles(Caller caller, Environment callEnvironment, Action action, TargetResource target){
return roleMapper.mapRoles(caller, callEnvironment, action, target);
}

public void setNonFacadeMBeansSensitive(boolean sensitive) {
nonFacadeMBeansSensitive = sensitive;
}

public boolean isNonFacadeMBeansSensitive() {
return nonFacadeMBeansSensitive;
}

private PermissionCollection getUserPermissions(Set<String> roles) {
configureRolePermissions(false);
ManagementPermissionCollection simple = null;
Expand Down Expand Up @@ -356,5 +371,4 @@ public Permission nextElement() {
}

}

}
Loading

0 comments on commit 463453c

Please sign in to comment.