This is an automated email from the ASF dual-hosted git repository.
sureshanaparti pushed a commit to branch 4.16
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.16 by this push:
new a6d9fa6 Role escalation prevention (#5879)
a6d9fa6 is described below
commit a6d9fa61b9b3aa55a3974afcd76109b361c5547b
Author: dahn <[email protected]>
AuthorDate: Thu Feb 10 07:20:27 2022 +0100
Role escalation prevention (#5879)
* prevent role access escallation
* hierarchy issue fixed
* create api list in account manager for checking new account access
* full api list check
* strange role restriction removed for BareMetal
* add role check on upfdate account as well
* more selective use of api checkers
* error msg and var name
Co-authored-by: Daan Hoogland <[email protected]>
---
.../main/java/com/cloud/user/AccountService.java | 4 +-
.../java/org/apache/cloudstack/acl/APIChecker.java | 3 +
.../command/admin/account/CreateAccountCmd.java | 3 +-
.../admin/account/CreateAccountCmdTest.java | 6 +-
.../acl/DynamicRoleBasedAPIAccessChecker.java | 9 ++
.../acl/ProjectRoleBasedApiAccessChecker.java | 11 +-
.../acl/StaticRoleBasedAPIAccessChecker.java | 8 ++
.../cloudstack/discovery/ApiDiscoveryService.java | 5 +
.../discovery/ApiDiscoveryServiceImpl.java | 19 +++
.../ratelimit/ApiRateLimitServiceImpl.java | 13 +-
.../api/BaremetalProvisionDoneNotificationCmd.java | 3 +-
.../contrail/management/MockAccountManager.java | 11 +-
.../java/com/cloud/user/AccountManagerImpl.java | 154 ++++++++++++++++++++-
.../core/spring-server-core-managers-context.xml | 4 +-
.../com/cloud/user/MockAccountManagerImpl.java | 11 +-
15 files changed, 237 insertions(+), 27 deletions(-)
diff --git a/api/src/main/java/com/cloud/user/AccountService.java
b/api/src/main/java/com/cloud/user/AccountService.java
index 98b1618..338a8d3 100644
--- a/api/src/main/java/com/cloud/user/AccountService.java
+++ b/api/src/main/java/com/cloud/user/AccountService.java
@@ -21,6 +21,7 @@ import java.util.Map;
import org.apache.cloudstack.acl.ControlledEntity;
import org.apache.cloudstack.acl.RoleType;
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
+import org.apache.cloudstack.api.command.admin.account.CreateAccountCmd;
import org.apache.cloudstack.api.command.admin.user.GetUserKeysCmd;
import org.apache.cloudstack.api.command.admin.user.RegisterCmd;
import org.apache.cloudstack.api.command.admin.user.UpdateUserCmd;
@@ -39,8 +40,7 @@ public interface AccountService {
* Creates a new user and account, stores the password as is so encrypted
passwords are recommended.
* @return the user if created successfully, null otherwise
*/
- UserAccount createUserAccount(String userName, String password, String
firstName, String lastName, String email, String timezone, String accountName,
short accountType, Long roleId, Long domainId,
- String networkDomain, Map<String, String> details, String
accountUUID, String userUUID);
+ UserAccount createUserAccount(CreateAccountCmd accountCmd);
UserAccount createUserAccount(String userName, String password, String
firstName, String lastName, String email, String timezone, String accountName,
short accountType, Long roleId, Long domainId,
String networkDomain, Map<String, String> details, String
accountUUID, String userUUID, User.Source source);
diff --git a/api/src/main/java/org/apache/cloudstack/acl/APIChecker.java
b/api/src/main/java/org/apache/cloudstack/acl/APIChecker.java
index 0d0dfd1..6cf4545 100644
--- a/api/src/main/java/org/apache/cloudstack/acl/APIChecker.java
+++ b/api/src/main/java/org/apache/cloudstack/acl/APIChecker.java
@@ -17,6 +17,7 @@
package org.apache.cloudstack.acl;
import com.cloud.exception.PermissionDeniedException;
+import com.cloud.user.Account;
import com.cloud.user.User;
import com.cloud.utils.component.Adapter;
@@ -27,4 +28,6 @@ public interface APIChecker extends Adapter {
// If false, apiChecker is unable to handle the operation or not
implemented
// On exception, checkAccess failed don't allow
boolean checkAccess(User user, String apiCommandName) throws
PermissionDeniedException;
+ boolean checkAccess(Account account, String apiCommandName) throws
PermissionDeniedException;
+ boolean isEnabled();
}
diff --git
a/api/src/main/java/org/apache/cloudstack/api/command/admin/account/CreateAccountCmd.java
b/api/src/main/java/org/apache/cloudstack/api/command/admin/account/CreateAccountCmd.java
index 508750a..159f17f 100644
---
a/api/src/main/java/org/apache/cloudstack/api/command/admin/account/CreateAccountCmd.java
+++
b/api/src/main/java/org/apache/cloudstack/api/command/admin/account/CreateAccountCmd.java
@@ -186,8 +186,7 @@ public class CreateAccountCmd extends BaseCmd {
validateParams();
CallContext.current().setEventDetails("Account Name: " + getUsername()
+ ", Domain Id:" + getDomainId());
UserAccount userAccount =
- _accountService.createUserAccount(getUsername(), getPassword(),
getFirstName(), getLastName(), getEmail(), getTimeZone(), getAccountName(),
getAccountType(), getRoleId(),
- getDomainId(), getNetworkDomain(), getDetails(),
getAccountUUID(), getUserUUID());
+ _accountService.createUserAccount(this);
if (userAccount != null) {
AccountResponse response =
_responseGenerator.createUserAccountResponse(ResponseView.Full, userAccount);
response.setResponseName(getCommandName());
diff --git
a/api/src/test/java/org/apache/cloudstack/api/command/admin/account/CreateAccountCmdTest.java
b/api/src/test/java/org/apache/cloudstack/api/command/admin/account/CreateAccountCmdTest.java
index b50b226..cf32a67 100644
---
a/api/src/test/java/org/apache/cloudstack/api/command/admin/account/CreateAccountCmdTest.java
+++
b/api/src/test/java/org/apache/cloudstack/api/command/admin/account/CreateAccountCmdTest.java
@@ -73,7 +73,7 @@ public class CreateAccountCmdTest {
} catch (ServerApiException e) {
Assert.assertTrue("Received exception as the mock accountService
createUserAccount returns null user", true);
}
- Mockito.verify(accountService,
Mockito.times(1)).createUserAccount(null, "Test", null, null, null, null, null,
accountType, roleId, domainId, null, null, null, null);
+ Mockito.verify(accountService,
Mockito.times(1)).createUserAccount(createAccountCmd);
}
@Test
@@ -86,7 +86,7 @@ public class CreateAccountCmdTest {
Assert.assertEquals(ApiErrorCode.PARAM_ERROR, e.getErrorCode());
Assert.assertEquals("Empty passwords are not allowed",
e.getMessage());
}
- Mockito.verify(accountService,
Mockito.never()).createUserAccount(null, null, null, null, null, null, null,
accountType, roleId, domainId, null, null, null, null);
+ Mockito.verify(accountService,
Mockito.never()).createUserAccount(createAccountCmd);
}
@Test
@@ -99,6 +99,6 @@ public class CreateAccountCmdTest {
Assert.assertEquals(ApiErrorCode.PARAM_ERROR, e.getErrorCode());
Assert.assertEquals("Empty passwords are not allowed",
e.getMessage());
}
- Mockito.verify(accountService,
Mockito.never()).createUserAccount(null, null, null, null, null, null, null,
accountType, roleId, domainId, null, null, null, null);
+ Mockito.verify(accountService,
Mockito.never()).createUserAccount(createAccountCmd);
}
}
diff --git
a/plugins/acl/dynamic-role-based/src/main/java/org/apache/cloudstack/acl/DynamicRoleBasedAPIAccessChecker.java
b/plugins/acl/dynamic-role-based/src/main/java/org/apache/cloudstack/acl/DynamicRoleBasedAPIAccessChecker.java
index f693bae..76d700e 100644
---
a/plugins/acl/dynamic-role-based/src/main/java/org/apache/cloudstack/acl/DynamicRoleBasedAPIAccessChecker.java
+++
b/plugins/acl/dynamic-role-based/src/main/java/org/apache/cloudstack/acl/DynamicRoleBasedAPIAccessChecker.java
@@ -75,6 +75,10 @@ public class DynamicRoleBasedAPIAccessChecker extends
AdapterBase implements API
throw new PermissionDeniedException("The account id=" +
user.getAccountId() + "for user id=" + user.getId() + "is null");
}
+ return checkAccess(account, commandName);
+ }
+
+ public boolean checkAccess(Account account, String commandName) {
final Role accountRole = roleService.findRole(account.getRoleId());
if (accountRole == null || accountRole.getId() < 1L) {
denyApiAccess(commandName);
@@ -106,6 +110,11 @@ public class DynamicRoleBasedAPIAccessChecker extends
AdapterBase implements API
throw new UnavailableCommandException("The API " + commandName + "
does not exist or is not available for this account.");
}
+ @Override
+ public boolean isEnabled() {
+ return roleService.isEnabled();
+ }
+
public void addApiToRoleBasedAnnotationsMap(final RoleType roleType, final
String commandName) {
if (roleType == null || Strings.isNullOrEmpty(commandName)) {
return;
diff --git
a/plugins/acl/project-role-based/src/main/java/org/apache/cloudstack/acl/ProjectRoleBasedApiAccessChecker.java
b/plugins/acl/project-role-based/src/main/java/org/apache/cloudstack/acl/ProjectRoleBasedApiAccessChecker.java
index 5648a96..4d33b26 100644
---
a/plugins/acl/project-role-based/src/main/java/org/apache/cloudstack/acl/ProjectRoleBasedApiAccessChecker.java
+++
b/plugins/acl/project-role-based/src/main/java/org/apache/cloudstack/acl/ProjectRoleBasedApiAccessChecker.java
@@ -58,9 +58,13 @@ public class ProjectRoleBasedApiAccessChecker extends
AdapterBase implements AP
throw new PermissionDeniedException("The API " + commandName + " is
denied for the user's/account's project role.");
}
+ @Override
+ public boolean isEnabled() {
+ return roleService.isEnabled();
+ }
public boolean isDisabled() {
- return !roleService.isEnabled();
+ return !isEnabled();
}
@Override
@@ -103,6 +107,11 @@ public class ProjectRoleBasedApiAccessChecker extends
AdapterBase implements AP
throw new UnavailableCommandException("The API " + apiCommandName + "
does not exist or is not available for this account/user in project
"+project.getUuid());
}
+ @Override
+ public boolean checkAccess(Account account, String apiCommandName) throws
PermissionDeniedException {
+ return true;
+ }
+
private boolean isPermitted(Project project, ProjectAccount projectUser,
String apiCommandName) {
ProjectRole projectRole = null;
if(projectUser.getProjectRoleId() != null) {
diff --git
a/plugins/acl/static-role-based/src/main/java/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java
b/plugins/acl/static-role-based/src/main/java/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java
index 7550483..2b2464f 100644
---
a/plugins/acl/static-role-based/src/main/java/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java
+++
b/plugins/acl/static-role-based/src/main/java/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java
@@ -65,6 +65,10 @@ public class StaticRoleBasedAPIAccessChecker extends
AdapterBase implements APIA
}
}
+ @Override
+ public boolean isEnabled() {
+ return !isDisabled();
+ }
public boolean isDisabled() {
return roleService.isEnabled();
}
@@ -80,6 +84,10 @@ public class StaticRoleBasedAPIAccessChecker extends
AdapterBase implements APIA
throw new PermissionDeniedException("The account id=" +
user.getAccountId() + "for user id=" + user.getId() + "is null");
}
+ return checkAccess(account, commandName);
+ }
+
+ public boolean checkAccess(Account account, String commandName) {
RoleType roleType = accountService.getRoleType(account);
boolean isAllowed =
commandsPropertiesOverrides.contains(commandName) ?
commandsPropertiesRoleBasedApisMap.get(roleType).contains(commandName) :
annotationRoleBasedApisMap.get(
diff --git
a/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryService.java
b/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryService.java
index ce8722e..5a6eab7 100644
---
a/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryService.java
+++
b/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryService.java
@@ -16,12 +16,17 @@
// under the License.
package org.apache.cloudstack.discovery;
+import com.cloud.user.Account;
import org.apache.cloudstack.api.BaseResponse;
import org.apache.cloudstack.api.response.ListResponse;
import com.cloud.user.User;
import com.cloud.utils.component.PluggableService;
+import java.util.List;
+
public interface ApiDiscoveryService extends PluggableService {
+ List<String> listApiNames(Account account);
+
ListResponse<? extends BaseResponse> listApis(User user, String apiName);
}
diff --git
a/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java
b/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java
index 5d22856..0a0a608 100644
---
a/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java
+++
b/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java
@@ -27,6 +27,7 @@ import java.util.Set;
import javax.inject.Inject;
+import com.cloud.user.Account;
import org.apache.cloudstack.acl.APIChecker;
import org.apache.cloudstack.api.APICommand;
import org.apache.cloudstack.api.BaseAsyncCmd;
@@ -211,6 +212,24 @@ public class ApiDiscoveryServiceImpl extends
ComponentLifecycleBase implements A
}
@Override
+ public List<String> listApiNames(Account account) {
+ List<String> apiNames = new ArrayList<>();
+ for (String apiName : s_apiNameDiscoveryResponseMap.keySet()) {
+ boolean isAllowed = true;
+ for (APIChecker apiChecker : _apiAccessCheckers) {
+ try {
+ apiChecker.checkAccess(account, apiName);
+ } catch (Exception ex) {
+ isAllowed = false;
+ }
+ }
+ if (isAllowed)
+ apiNames.add(apiName);
+ }
+ return apiNames;
+ }
+
+ @Override
public ListResponse<? extends BaseResponse> listApis(User user, String
name) {
ListResponse<ApiDiscoveryResponse> response = new
ListResponse<ApiDiscoveryResponse>();
List<ApiDiscoveryResponse> responseList = new
ArrayList<ApiDiscoveryResponse>();
diff --git
a/plugins/api/rate-limit/src/main/java/org/apache/cloudstack/ratelimit/ApiRateLimitServiceImpl.java
b/plugins/api/rate-limit/src/main/java/org/apache/cloudstack/ratelimit/ApiRateLimitServiceImpl.java
index e35a332..3ed3551 100644
---
a/plugins/api/rate-limit/src/main/java/org/apache/cloudstack/ratelimit/ApiRateLimitServiceImpl.java
+++
b/plugins/api/rate-limit/src/main/java/org/apache/cloudstack/ratelimit/ApiRateLimitServiceImpl.java
@@ -147,16 +147,20 @@ public class ApiRateLimitServiceImpl extends AdapterBase
implements APIChecker,
}
Long accountId = user.getAccountId();
Account account = _accountService.getAccount(accountId);
+ return checkAccess(account, apiCommandName);
+ }
+
+ public boolean checkAccess(Account account, String commandName) {
if (_accountService.isRootAdmin(account.getId())) {
// no API throttling on root admin
return true;
}
- StoreEntry entry = _store.get(accountId);
+ StoreEntry entry = _store.get(account.getId());
if (entry == null) {
/* Populate the entry, thus unlocking any underlying mutex */
- entry = _store.create(accountId, timeToLive);
+ entry = _store.create(account.getId(), timeToLive);
}
/* Increment the client count and see whether we have hit the maximum
allowed clients yet. */
@@ -175,6 +179,11 @@ public class ApiRateLimitServiceImpl extends AdapterBase
implements APIChecker,
}
@Override
+ public boolean isEnabled() {
+ return enabled;
+ }
+
+ @Override
public List<Class<?>> getCommands() {
List<Class<?>> cmdList = new ArrayList<Class<?>>();
cmdList.add(ResetApiLimitCmd.class);
diff --git
a/plugins/hypervisors/baremetal/src/main/java/org/apache/cloudstack/api/BaremetalProvisionDoneNotificationCmd.java
b/plugins/hypervisors/baremetal/src/main/java/org/apache/cloudstack/api/BaremetalProvisionDoneNotificationCmd.java
index a5248aa..c712849 100644
---
a/plugins/hypervisors/baremetal/src/main/java/org/apache/cloudstack/api/BaremetalProvisionDoneNotificationCmd.java
+++
b/plugins/hypervisors/baremetal/src/main/java/org/apache/cloudstack/api/BaremetalProvisionDoneNotificationCmd.java
@@ -24,7 +24,6 @@ import com.cloud.exception.InsufficientCapacityException;
import com.cloud.exception.NetworkRuleConflictException;
import com.cloud.exception.ResourceAllocationException;
import com.cloud.exception.ResourceUnavailableException;
-import org.apache.cloudstack.acl.RoleType;
import org.apache.cloudstack.api.response.SuccessResponse;
import org.apache.cloudstack.context.CallContext;
@@ -35,7 +34,7 @@ import org.apache.log4j.Logger;
* Created by frank on 9/17/14.
*/
@APICommand(name = "notifyBaremetalProvisionDone", description = "Notify
provision has been done on a host. This api is for baremetal virtual router
service, not for end user", responseObject = SuccessResponse.class,
- requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,
authorized = {RoleType.User})
+ requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
public class BaremetalProvisionDoneNotificationCmd extends BaseAsyncCmd {
public static final Logger s_logger =
Logger.getLogger(BaremetalProvisionDoneNotificationCmd.class);
private static final String s_name = "baremetalprovisiondone";
diff --git
a/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java
b/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java
index 5cd90c9..6326d7e 100644
---
a/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java
+++
b/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java
@@ -24,6 +24,7 @@ import java.net.InetAddress;
import javax.inject.Inject;
import javax.naming.ConfigurationException;
+import org.apache.cloudstack.api.command.admin.account.CreateAccountCmd;
import org.apache.cloudstack.api.command.admin.user.GetUserKeysCmd;
import org.apache.cloudstack.api.command.admin.user.MoveUserCmd;
import org.apache.cloudstack.framework.config.ConfigKey;
@@ -140,10 +141,12 @@ public class MockAccountManager extends ManagerBase
implements AccountManager {
}
@Override
- public UserAccount createUserAccount(String arg0, String arg1, String
arg2, String arg3, String arg4, String arg5, String arg6, short arg7, Long
roleId, Long arg8, String arg9,
- Map<String, String> arg10, String arg11, String arg12) {
- // TODO Auto-generated method stub
- return null;
+ public UserAccount createUserAccount(CreateAccountCmd cmd) {
+ return createUserAccount(cmd.getUsername(), cmd.getPassword(),
cmd.getFirstName(),
+ cmd.getLastName(), cmd.getEmail(), cmd.getTimeZone(),
cmd.getAccountName(),
+ cmd.getAccountType(), cmd.getRoleId(), cmd.getDomainId(),
+ cmd.getNetworkDomain(), cmd.getDetails(), cmd.getAccountUUID(),
+ cmd.getUserUUID(), User.Source.UNKNOWN);
}
@Override
diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java
b/server/src/main/java/com/cloud/user/AccountManagerImpl.java
index 88cd217..bd51cd4 100644
--- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java
+++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java
@@ -23,8 +23,10 @@ import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
+import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import java.util.UUID;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
@@ -37,14 +39,19 @@ import javax.crypto.spec.SecretKeySpec;
import javax.inject.Inject;
import javax.naming.ConfigurationException;
+import com.cloud.utils.component.PluggableService;
+import org.apache.cloudstack.acl.APIChecker;
import org.apache.cloudstack.acl.ControlledEntity;
import org.apache.cloudstack.acl.QuerySelector;
import org.apache.cloudstack.acl.Role;
+import org.apache.cloudstack.acl.RoleService;
import org.apache.cloudstack.acl.RoleType;
import org.apache.cloudstack.acl.SecurityChecker;
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
import org.apache.cloudstack.affinity.AffinityGroup;
import org.apache.cloudstack.affinity.dao.AffinityGroupDao;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.command.admin.account.CreateAccountCmd;
import org.apache.cloudstack.api.command.admin.account.UpdateAccountCmd;
import org.apache.cloudstack.api.command.admin.user.DeleteUserCmd;
import org.apache.cloudstack.api.command.admin.user.GetUserKeysCmd;
@@ -176,6 +183,7 @@ import com.cloud.vm.snapshot.VMSnapshot;
import com.cloud.vm.snapshot.VMSnapshotManager;
import com.cloud.vm.snapshot.VMSnapshotVO;
import com.cloud.vm.snapshot.dao.VMSnapshotDao;
+import org.jetbrains.annotations.NotNull;
public class AccountManagerImpl extends ManagerBase implements AccountManager,
Manager {
public static final Logger s_logger =
Logger.getLogger(AccountManagerImpl.class);
@@ -279,9 +287,13 @@ public class AccountManagerImpl extends ManagerBase
implements AccountManager, M
private List<UserAuthenticator> _userAuthenticators;
protected List<UserAuthenticator> _userPasswordEncoders;
+ protected List<PluggableService> services;
+ private List<APIChecker> apiAccessCheckers;
@Inject
private IpAddressManager _ipAddrMgr;
+ @Inject
+ private RoleService roleService;
private final ScheduledExecutorService _executor =
Executors.newScheduledThreadPool(1, new NamedThreadFactory("AccountChecker"));
@@ -292,6 +304,11 @@ public class AccountManagerImpl extends ManagerBase
implements AccountManager, M
private List<SecurityChecker> _securityCheckers;
private int _cleanupInterval;
+ private List<String> apiNameList;
+
+ protected AccountManagerImpl() {
+ super();
+ }
public List<UserAuthenticator> getUserAuthenticators() {
return _userAuthenticators;
@@ -317,6 +334,22 @@ public class AccountManagerImpl extends ManagerBase
implements AccountManager, M
_securityCheckers = securityCheckers;
}
+ public List<PluggableService> getServices() {
+ return services;
+ }
+
+ public void setServices(List<PluggableService> services) {
+ this.services = services;
+ }
+
+ public List<APIChecker> getApiAccessCheckers() {
+ return apiAccessCheckers;
+ }
+
+ public void setApiAccessCheckers(List<APIChecker> apiAccessCheckers) {
+ this.apiAccessCheckers = apiAccessCheckers;
+ }
+
public List<QuerySelector> getQuerySelectors() {
return _querySelectors;
}
@@ -358,10 +391,46 @@ public class AccountManagerImpl extends ManagerBase
implements AccountManager, M
@Override
public boolean start() {
+ if (apiNameList == null) {
+ long startTime = System.nanoTime();
+ apiNameList = new ArrayList<String>();
+ Set<Class<?>> cmdClasses = new LinkedHashSet<Class<?>>();
+ for (PluggableService service : services) {
+ s_logger.debug(String.format("getting api commands of service:
%s", service.getClass().getName()));
+ cmdClasses.addAll(service.getCommands());
+ }
+ apiNameList = createApiNameList(cmdClasses);
+ long endTime = System.nanoTime();
+ s_logger.info("Api Discovery Service: Annotation, docstrings, api
relation graph processed in " + (endTime - startTime) / 1000000.0 + " ms");
+ }
_executor.scheduleAtFixedRate(new AccountCleanupTask(),
_cleanupInterval, _cleanupInterval, TimeUnit.SECONDS);
return true;
}
+ protected List<String> createApiNameList(Set<Class<?>> cmdClasses) {
+ List<String> apiNameList = new ArrayList<String>();
+
+ for (Class<?> cmdClass : cmdClasses) {
+ APICommand apiCmdAnnotation =
cmdClass.getAnnotation(APICommand.class);
+ if (apiCmdAnnotation == null) {
+ apiCmdAnnotation =
cmdClass.getSuperclass().getAnnotation(APICommand.class);
+ }
+ if (apiCmdAnnotation == null ||
!apiCmdAnnotation.includeInApiDoc() || apiCmdAnnotation.name().isEmpty()) {
+ continue;
+ }
+
+ String apiName = apiCmdAnnotation.name();
+ if (s_logger.isTraceEnabled()) {
+ s_logger.trace("Found api: " + apiName);
+ }
+
+ apiNameList.add(apiName);
+ }
+
+ return apiNameList;
+ }
+
+
@Override
public boolean stop() {
return true;
@@ -1003,14 +1072,16 @@ public class AccountManagerImpl extends ManagerBase
implements AccountManager, M
@Override
@ActionEvents({@ActionEvent(eventType = EventTypes.EVENT_ACCOUNT_CREATE,
eventDescription = "creating Account"),
- @ActionEvent(eventType = EventTypes.EVENT_USER_CREATE,
eventDescription = "creating User")})
- public UserAccount createUserAccount(final String userName, final String
password, final String firstName, final String lastName, final String email,
final String timezone, String accountName,
- final short accountType, final Long roleId, Long domainId, final
String networkDomain, final Map<String, String> details, String accountUUID,
final String userUUID) {
-
- return createUserAccount(userName, password, firstName, lastName,
email, timezone, accountName, accountType, roleId, domainId, networkDomain,
details, accountUUID, userUUID,
- User.Source.UNKNOWN);
+ @ActionEvent(eventType = EventTypes.EVENT_USER_CREATE,
eventDescription = "creating User")})
+ public UserAccount createUserAccount(CreateAccountCmd accountCmd) {
+ return createUserAccount(accountCmd.getUsername(),
accountCmd.getPassword(), accountCmd.getFirstName(),
+ accountCmd.getLastName(), accountCmd.getEmail(),
accountCmd.getTimeZone(), accountCmd.getAccountName(),
+ accountCmd.getAccountType(), accountCmd.getRoleId(),
accountCmd.getDomainId(),
+ accountCmd.getNetworkDomain(), accountCmd.getDetails(),
accountCmd.getAccountUUID(),
+ accountCmd.getUserUUID(), User.Source.UNKNOWN);
}
+
// ///////////////////////////////////////////////////
// ////////////// API commands /////////////////////
// ///////////////////////////////////////////////////
@@ -1080,6 +1151,8 @@ public class AccountManagerImpl extends ManagerBase
implements AccountManager, M
AccountVO account = createAccount(accountNameFinal,
accountType, roleId, domainIdFinal, networkDomain, details, accountUUID);
long accountId = account.getId();
+ checkRoleEscalation(getCurrentCallingAccount(), account);
+
// create the first user for the account
UserVO user = createUser(accountId, userName, password,
firstName, lastName, email, timezone, userUUID, source);
@@ -1110,6 +1183,74 @@ public class AccountManagerImpl extends ManagerBase
implements AccountManager, M
return _userAccountDao.findById(userId);
}
+ /**
+ * if there is any permission under the requested role that is not
permitted for the caller, refuse
+ */
+ private void checkRoleEscalation(Account caller, Account requested) {
+ if (s_logger.isDebugEnabled()) {
+ s_logger.debug(String.format("checking if user of account %s [%s]
with role-id [%d] can create an account of type %s [%s] with role-id [%d]",
+ caller.getAccountName(),
+ caller.getUuid(),
+ caller.getRoleId(),
+ requested.getAccountName(),
+ requested.getUuid(),
+ requested.getRoleId()));
+ }
+ List<APIChecker> apiCheckers = getEnabledApiCheckers();
+ for (String command : apiNameList) {
+ try {
+ checkApiAccess(apiCheckers, requested, command);
+ } catch (PermissionDeniedException pde) {
+ if (s_logger.isTraceEnabled()) {
+ s_logger.trace(String.format("checking for permission to
\"%s\" is irrelevant as it is not requested for %s [%s]",
+ command,
+ pde.getAccount().getAccountName(),
+ pde.getAccount().getUuid(),
+ pde.getEntitiesInViolation()
+ ));
+ }
+ continue;
+ }
+ // so requested can, now make sure caller can as well
+ try {
+ if (s_logger.isTraceEnabled()) {
+ s_logger.trace(String.format("permission to \"%s\" is
requested",
+ command));
+ }
+ checkApiAccess(apiCheckers, caller, command);
+ } catch (PermissionDeniedException pde) {
+ String msg = String.format("User of Account %s/%s (%s) can not
create an account with access to more privileges they have themself.",
+ caller.getAccountName(),
+ caller.getDomainId(),
+ caller.getUuid());
+ s_logger.warn(msg);
+ throw new PermissionDeniedException(msg,pde);
+ }
+ }
+ }
+
+ private void checkApiAccess(List<APIChecker> apiCheckers, Account caller,
String command) {
+ for (final APIChecker apiChecker : apiCheckers) {
+ apiChecker.checkAccess(caller, command);
+ }
+ }
+
+ @NotNull
+ private List<APIChecker> getEnabledApiCheckers() {
+ // we are really only interested in the dynamic access checker
+ List<APIChecker> usableApiCheckers = new ArrayList<>();
+ for (APIChecker apiChecker : apiAccessCheckers) {
+ if (apiChecker.isEnabled()) {
+ usableApiCheckers.add(apiChecker);
+ if (s_logger.isTraceEnabled()) {
+ s_logger.trace(String.format("using api checker \"%s\"",
+ apiChecker.getName()));
+ }
+ }
+ }
+ return usableApiCheckers;
+ }
+
@Override
@ActionEvent(eventType = EventTypes.EVENT_USER_CREATE, eventDescription =
"creating User")
public UserVO createUser(String userName, String password, String
firstName, String lastName, String email, String timeZone, String accountName,
Long domainId, String userUUID,
@@ -1759,6 +1900,7 @@ public class AccountManagerImpl extends ManagerBase
implements AccountManager, M
}
acctForUpdate.setRoleId(roleId);
+ checkRoleEscalation(getCurrentCallingAccount(), acctForUpdate);
}
if (networkDomain != null) {
diff --git
a/server/src/main/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml
b/server/src/main/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml
index d79908e..30248d7 100644
---
a/server/src/main/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml
+++
b/server/src/main/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml
@@ -47,7 +47,9 @@
<property name="userPasswordEncoders"
value="#{userPasswordEncodersRegistry.registered}" />
<property name="securityCheckers"
value="#{securityCheckersRegistry.registered}" />
- <property name="querySelectors"
value="#{querySelectorsRegistry.registered}" />
+ <property name="querySelectors"
value="#{querySelectorsRegistry.registered}" />
+ <property name="apiAccessCheckers"
value="#{apiAclCheckersRegistry.registered}" />
+ <property name="services" value="#{apiCommandsRegistry.registered}" />
</bean>
<bean id="managementServerImpl"
class="com.cloud.server.ManagementServerImpl">
diff --git a/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java
b/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java
index 7916007..f222368 100644
--- a/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java
+++ b/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java
@@ -22,6 +22,7 @@ import java.net.InetAddress;
import javax.naming.ConfigurationException;
+import org.apache.cloudstack.api.command.admin.account.CreateAccountCmd;
import org.apache.cloudstack.api.command.admin.user.GetUserKeysCmd;
import org.apache.cloudstack.api.command.admin.user.MoveUserCmd;
import org.apache.cloudstack.framework.config.ConfigKey;
@@ -346,10 +347,12 @@ public class MockAccountManagerImpl extends ManagerBase
implements Manager, Acco
}
@Override
- public UserAccount createUserAccount(String userName, String password,
String firstName, String lastName, String email, String timezone, String
accountName,
- short accountType, Long roleId, Long domainId, String networkDomain,
Map<String, String> details, String accountUUID, String userUUID) {
- // TODO Auto-generated method stub
- return null;
+ public UserAccount createUserAccount(CreateAccountCmd cmd) {
+ return createUserAccount(cmd.getUsername(), cmd.getPassword(),
cmd.getFirstName(),
+ cmd.getLastName(), cmd.getEmail(), cmd.getTimeZone(),
cmd.getAccountName(),
+ cmd.getAccountType(), cmd.getRoleId(), cmd.getDomainId(),
+ cmd.getNetworkDomain(), cmd.getDetails(), cmd.getAccountUUID(),
+ cmd.getUserUUID(), User.Source.UNKNOWN);
}
@Override
