Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,6 @@ public interface ConsoleAccessManager extends Manager {
boolean isSessionAllowed(String sessionUuid);

void removeSessions(String[] sessionUuids);

void removeSession(String sessionUuid);
}
123 changes: 123 additions & 0 deletions engine/schema/src/main/java/com/cloud/vm/ConsoleSessionVO.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
//
// Licensed to the Apache Software Foundation (ASF) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
//

package com.cloud.vm;

import javax.persistence.Column;
import javax.persistence.Entity;
import javax.persistence.GeneratedValue;
import javax.persistence.GenerationType;
import javax.persistence.Id;
import javax.persistence.Table;
import java.util.Date;

@Entity
@Table(name = "console_session")
public class ConsoleSessionVO {

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
@Column(name = "id")
private long id;

@Column(name = "uuid")
private String uuid;

@Column(name = "created")
private Date created;

@Column(name = "account_id")
private long accountId;

@Column(name = "user_id")
private long userId;

@Column(name = "instance_id")
private long instanceId;

@Column(name = "host_id")
private long hostId;

@Column(name = "removed")
private Date removed;

public long getId() {
return id;
}

public void setId(long id) {
this.id = id;
}

public String getUuid() {
return uuid;
}

public void setUuid(String uuid) {
this.uuid = uuid;
}

public Date getCreated() {
return created;
}

public void setCreated(Date created) {
this.created = created;
}

public long getAccountId() {
return accountId;
}

public void setAccountId(long accountId) {
this.accountId = accountId;
}

public long getUserId() {
return userId;
}

public void setUserId(long userId) {
this.userId = userId;
}

public long getInstanceId() {
return instanceId;
}

public void setInstanceId(long instanceId) {
this.instanceId = instanceId;
}

public long getHostId() {
return hostId;
}

public void setHostId(long hostId) {
this.hostId = hostId;
}

public Date getRemoved() {
return removed;
}

public void setRemoved(Date removed) {
this.removed = removed;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//
// Licensed to the Apache Software Foundation (ASF) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
//

package com.cloud.vm.dao;

import com.cloud.vm.ConsoleSessionVO;
import com.cloud.utils.db.GenericDao;

public interface ConsoleSessionDao extends GenericDao<ConsoleSessionVO, Long> {

void removeSession(String sessionUuid);

boolean isSessionAllowed(String sessionUuid);

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
//
// Licensed to the Apache Software Foundation (ASF) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
//

package com.cloud.vm.dao;

import com.cloud.vm.ConsoleSessionVO;
import com.cloud.utils.db.GenericDaoBase;

public class ConsoleSessionDaoImpl extends GenericDaoBase<ConsoleSessionVO, Long> implements ConsoleSessionDao {

@Override
public void removeSession(String sessionUuid) {
ConsoleSessionVO session = findByUuid(sessionUuid);
remove(session.getId());
}

@Override
public boolean isSessionAllowed(String sessionUuid) {
return findByUuid(sessionUuid) != null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
<bean id="accountJoinDaoImpl" class="com.cloud.api.query.dao.AccountJoinDaoImpl" />
<bean id="accountVlanMapDaoImpl" class="com.cloud.dc.dao.AccountVlanMapDaoImpl" />
<bean id="alertDaoImpl" class="com.cloud.alert.dao.AlertDaoImpl" />
<bean id="consoleSessionDaoImpl" class="com.cloud.vm.dao.ConsoleSessionDaoImpl" />
<bean id="asyncJobJoinDaoImpl" class="com.cloud.api.query.dao.AsyncJobJoinDaoImpl" />
<bean id="autoScalePolicyConditionMapDaoImpl" class="com.cloud.network.as.dao.AutoScalePolicyConditionMapDaoImpl" />
<bean id="autoScalePolicyDaoImpl" class="com.cloud.network.as.dao.AutoScalePolicyDaoImpl" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1035,3 +1035,21 @@ WHERE role_id = (SELECT id FROM `cloud`.`roles` WHERE name = 'Read-Only User -
INSERT INTO `cloud`.`role_permissions` (`uuid`, `role_id`, `rule`, `permission`)
SELECT UUID(), `roles`.`id`, 'isAccountAllowedToCreateOfferingsWithTags', 'ALLOW'
FROM `cloud`.`roles` WHERE `role_type` = 'DomainAdmin';

--- Create table for handling console sessions #7094

CREATE TABLE IF NOT EXISTS `cloud`.`console_session` (
`id` bigint(20) unsigned NOT NULL AUTO_INCREMENT PRIMARY KEY,
`uuid` varchar(40) NOT NULL COMMENT 'UUID generated for the session',
`created` datetime NOT NULL COMMENT 'When the session was created',
`account_id` bigint(20) unsigned NOT NULL COMMENT 'Account who generated the session',
`user_id` bigint(20) unsigned NOT NULL COMMENT 'User who generated the session',
`instance_id` bigint(20) unsigned NOT NULL COMMENT 'VM for which the session was generated',
`host_id` bigint(20) unsigned NOT NULL COMMENT 'Host where the VM was when the session was generated',
`removed` datetime COMMENT 'When the session was removed/used',
CONSTRAINT `fk_consolesession__account_id` FOREIGN KEY(`account_id`) REFERENCES `cloud`.`account` (`id`),
CONSTRAINT `fk_consolesession__user_id` FOREIGN KEY(`user_id`) REFERENCES `cloud`.`user`(`id`),
CONSTRAINT `fk_consolesession__instance_id` FOREIGN KEY(`instance_id`) REFERENCES `cloud`.`vm_instance`(`id`),
CONSTRAINT `fk_consolesession__host_id` FOREIGN KEY(`host_id`) REFERENCES `cloud`.`host`(`id`),
CONSTRAINT `uc_consolesession__uuid` UNIQUE (`uuid`)
);
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,13 @@ public AgentControlAnswer onConsoleAccessAuthentication(ConsoleAccessAuthenticat
}

if (!consoleAccessManager.isSessionAllowed(sessionUuid)) {
s_logger.error("Invalid session, only one session allowed per token");
s_logger.error(String.format("Session [%s] has been already used or does not exist.", sessionUuid));
return new ConsoleAccessAuthenticationAnswer(cmd, false);
}

s_logger.debug(String.format("Removing session [%s] as it was just used.", sessionUuid));
consoleAccessManager.removeSession(sessionUuid);

if (!ticket.equals(ticketInUrl)) {
Date now = new Date();
// considering of minute round-up
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,20 @@
import com.cloud.utils.component.ManagerBase;
import com.cloud.utils.db.EntityManager;
import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.vm.ConsoleSessionVO;
import com.cloud.vm.UserVmDetailVO;
import com.cloud.vm.VirtualMachine;
import com.cloud.vm.VirtualMachineManager;
import com.cloud.vm.VmDetailConstants;
import com.cloud.vm.dao.ConsoleSessionDao;
import com.cloud.vm.dao.UserVmDetailsDao;
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
import org.apache.cloudstack.api.command.user.consoleproxy.ConsoleEndpoint;
import org.apache.cloudstack.context.CallContext;
import org.apache.cloudstack.framework.security.keys.KeysManager;
import org.apache.commons.codec.binary.Base64;
import org.apache.commons.lang3.ArrayUtils;
import org.apache.commons.lang3.ObjectUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.log4j.Logger;
Expand All @@ -60,10 +63,8 @@

import java.util.Arrays;
import java.util.Date;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.UUID;

public class ConsoleAccessManagerImpl extends ManagerBase implements ConsoleAccessManager {
Expand All @@ -84,6 +85,8 @@ public class ConsoleAccessManagerImpl extends ManagerBase implements ConsoleAcce
private AgentManager agentManager;
@Inject
private ConsoleProxyManager consoleProxyManager;
@Inject
private ConsoleSessionDao consoleSessionDao;

private static KeysManager secretKeysManager;
private final Gson gson = new GsonBuilder().create();
Expand All @@ -94,12 +97,9 @@ public class ConsoleAccessManagerImpl extends ManagerBase implements ConsoleAcce
VirtualMachine.State.Stopped, VirtualMachine.State.Error, VirtualMachine.State.Destroyed
);

private static Set<String> allowedSessions;

@Override
public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
ConsoleAccessManagerImpl.secretKeysManager = keysManager;
ConsoleAccessManagerImpl.allowedSessions = new HashSet<>();
return super.configure(name, params);
}

Expand Down Expand Up @@ -146,16 +146,23 @@ public ConsoleEndpoint generateConsoleEndpoint(Long vmId, String extraSecurityTo

@Override
public boolean isSessionAllowed(String sessionUuid) {
return allowedSessions.contains(sessionUuid);
return consoleSessionDao.isSessionAllowed(sessionUuid);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@GutoVeronezi the problem listed previously for one mgmt server comes from the method isSessionAllowed that does not query the database for the session UUID and always returns false, therefore the ConsoleAccessAuthenticationAnswer is false

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@nvazquez, thanks. I missed a line while porting the changes to the PR. I fixed it with commit dec6e3f and was doing some more testing before pinging you.

@Override
public void removeSessions(String[] sessionUuids) {
for (String r : sessionUuids) {
allowedSessions.remove(r);
if (ArrayUtils.isNotEmpty(sessionUuids)) {
for (String sessionUuid : sessionUuids) {
removeSession(sessionUuid);
}
}
}

@Override
public void removeSession(String sessionUuid) {
consoleSessionDao.removeSession(sessionUuid);
}

protected boolean checkSessionPermission(VirtualMachine vm, Account account) {
if (accountManager.isRootAdmin(account.getId())) {
return true;
Expand Down Expand Up @@ -289,7 +296,7 @@ private ConsoleEndpoint composeConsoleAccessEndpoint(String rootUrl, VirtualMach
String url = generateConsoleAccessUrl(rootUrl, param, token, vncPort, vm);

s_logger.debug("Adding allowed session: " + sessionUuid);
allowedSessions.add(sessionUuid);
persistConsoleSession(sessionUuid, vm.getId(), hostVo.getId());
managementServer.setConsoleAccessForVm(vm.getId(), sessionUuid);

ConsoleEndpoint consoleEndpoint = new ConsoleEndpoint(true, url);
Expand All @@ -303,6 +310,16 @@ private ConsoleEndpoint composeConsoleAccessEndpoint(String rootUrl, VirtualMach
return consoleEndpoint;
}

protected void persistConsoleSession(String sessionUuid, long instanceId, long hostId) {
ConsoleSessionVO consoleSessionVo = new ConsoleSessionVO();
consoleSessionVo.setUuid(sessionUuid);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

these lines can be replaced by a constructor

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

On principle, I would rather not create a constructor for informing the object properties, as the more properties it has, the longer the constructor would get, becoming harder to read/interpret the code. For Python, that supports keyword arguments, this approach works fine, though.

consoleSessionVo.setAccountId(CallContext.current().getCallingAccountId());
consoleSessionVo.setUserId(CallContext.current().getCallingUserId());
consoleSessionVo.setInstanceId(instanceId);
consoleSessionVo.setHostId(hostId);
consoleSessionDao.persist(consoleSessionVo);
}

private String generateConsoleAccessUrl(String rootUrl, ConsoleProxyClientParam param, String token, int vncPort,
VirtualMachine vm) {
StringBuilder sb = new StringBuilder(rootUrl);
Expand Down