From 0653d6d7f6283752984ea181ef27559bed4b05af Mon Sep 17 00:00:00 2001 From: Kelven Yang Date: Fri, 12 Nov 2010 16:59:28 -0800 Subject: [PATCH] Improve console access security with random generated hash key --- .../configuration/dao/ConfigurationDao.java | 7 ++-- .../dao/ConfigurationDaoImpl.java | 34 ++++++++++++++++++- .../src/com/cloud/configuration/Config.java | 5 +-- .../consoleproxy/ConsoleProxyManagerImpl.java | 10 ++++-- .../com/cloud/server/ManagementServer.java | 1 + .../cloud/server/ManagementServerImpl.java | 12 +++++++ .../cloud/servlet/ConsoleProxyServlet.java | 10 ++++-- 7 files changed, 69 insertions(+), 10 deletions(-) diff --git a/core/src/com/cloud/configuration/dao/ConfigurationDao.java b/core/src/com/cloud/configuration/dao/ConfigurationDao.java index b44343ffe52..3786ccc483e 100644 --- a/core/src/com/cloud/configuration/dao/ConfigurationDao.java +++ b/core/src/com/cloud/configuration/dao/ConfigurationDao.java @@ -57,7 +57,10 @@ public interface ConfigurationDao extends GenericDao { * Gets the value for the specified configuration name * @return value */ - public String getValue(String name); + public String getValue(String name); + + public String getValueAndInitIfNotExist(String name, String initValue); + /** * returns whether or not this is a premium configuration @@ -65,5 +68,5 @@ public interface ConfigurationDao extends GenericDao { */ boolean isPremium(); - ConfigurationVO findByName(String name); + ConfigurationVO findByName(String name); } diff --git a/core/src/com/cloud/configuration/dao/ConfigurationDaoImpl.java b/core/src/com/cloud/configuration/dao/ConfigurationDaoImpl.java index 5a5e684ee8a..507717ad77a 100644 --- a/core/src/com/cloud/configuration/dao/ConfigurationDaoImpl.java +++ b/core/src/com/cloud/configuration/dao/ConfigurationDaoImpl.java @@ -19,6 +19,7 @@ package com.cloud.configuration.dao; import java.sql.PreparedStatement; +import java.sql.ResultSet; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -29,10 +30,12 @@ import javax.naming.ConfigurationException; import org.apache.log4j.Logger; import com.cloud.configuration.ConfigurationVO; +import com.cloud.utils.db.DB; import com.cloud.utils.db.GenericDaoBase; import com.cloud.utils.db.SearchBuilder; import com.cloud.utils.db.SearchCriteria; import com.cloud.utils.db.Transaction; +import com.cloud.utils.exception.CloudRuntimeException; @Local(value={ConfigurationDao.class}) public class ConfigurationDaoImpl extends GenericDaoBase implements ConfigurationDao { @@ -144,7 +147,36 @@ public class ConfigurationDaoImpl extends GenericDaoBase _componentClass; private final Class _type; diff --git a/server/src/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java b/server/src/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java index 1aba0f8f880..9031386bfbd 100644 --- a/server/src/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java +++ b/server/src/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java @@ -1299,8 +1299,14 @@ public class ConsoleProxyManagerImpl implements ConsoleProxyManager, VirtualMach String ticket = ConsoleProxyServlet.genAccessTicket(cmd.getHost(), cmd.getPort(), cmd.getSid(), cmd.getVmId()); if(!ticket.startsWith(ticketInUrl)) { - s_logger.error("Access ticket expired or has been modified. vmId: " + cmd.getVmId()); - return new ConsoleAccessAuthenticationAnswer(cmd, false); + Date now = new Date(); + // considering of minute round-up + String minuteEarlyTicket = ConsoleProxyServlet.genAccessTicket(cmd.getHost(), cmd.getPort(), cmd.getSid(), cmd.getVmId(), + new Date(now.getTime() - 60*1000)); + if(!minuteEarlyTicket.startsWith(ticketInUrl)) { + s_logger.error("Access ticket expired or has been modified. vmId: " + cmd.getVmId()); + return new ConsoleAccessAuthenticationAnswer(cmd, false); + } } if (cmd.getVmId() != null && cmd.getVmId().isEmpty()) { diff --git a/server/src/com/cloud/server/ManagementServer.java b/server/src/com/cloud/server/ManagementServer.java index 73db6a9fec3..5eb61d01842 100755 --- a/server/src/com/cloud/server/ManagementServer.java +++ b/server/src/com/cloud/server/ManagementServer.java @@ -1047,4 +1047,5 @@ public interface ManagementServer { public List searchForRemoteAccessVpns(ListRemoteAccessVpnsCmd cmd); public List searchForVpnUsers(ListVpnUsersCmd cmd); + public String getHashKey(); } diff --git a/server/src/com/cloud/server/ManagementServerImpl.java b/server/src/com/cloud/server/ManagementServerImpl.java index 05826ca5145..b24e11ad4cc 100755 --- a/server/src/com/cloud/server/ManagementServerImpl.java +++ b/server/src/com/cloud/server/ManagementServerImpl.java @@ -48,6 +48,7 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.TimeZone; +import java.util.UUID; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; @@ -400,6 +401,7 @@ public class ManagementServerImpl implements ManagementServer { private boolean _networkGroupsEnabled = false; private boolean _isHypervisorSnapshotCapable = false; + private String _hashKey = null; protected ManagementServerImpl() { ComponentLocator locator = ComponentLocator.getLocator(Name); @@ -5765,4 +5767,14 @@ public class ManagementServerImpl implements ManagementServer { return _vpnUsersDao.search(sc, searchFilter); } + + @Override + public String getHashKey() { + // although we may have race conditioning here, database transaction serialization should + // give us the same key + if(_hashKey == null) { + _hashKey = _configDao.getValueAndInitIfNotExist(Config.HashKey.key(), UUID.randomUUID().toString()); + } + return _hashKey; + } } diff --git a/server/src/com/cloud/servlet/ConsoleProxyServlet.java b/server/src/com/cloud/servlet/ConsoleProxyServlet.java index 328aa0376f0..4e148a617c5 100644 --- a/server/src/com/cloud/servlet/ConsoleProxyServlet.java +++ b/server/src/com/cloud/servlet/ConsoleProxyServlet.java @@ -58,7 +58,7 @@ public class ConsoleProxyServlet extends HttpServlet { private static final int DEFAULT_THUMBNAIL_WIDTH = 144; private static final int DEFAULT_THUMBNAIL_HEIGHT = 110; - private final ManagementServer _ms = (ManagementServer)ComponentLocator.getComponent(ManagementServer.Name); + private final static ManagementServer _ms = (ManagementServer)ComponentLocator.getComponent(ManagementServer.Name); @Override protected void doPost(HttpServletRequest req, HttpServletResponse resp) { @@ -298,14 +298,18 @@ public class ConsoleProxyServlet extends HttpServlet { } public static String genAccessTicket(String host, String port, String sid, String tag) { + return genAccessTicket(host, port, sid, tag, new Date()); + } + + public static String genAccessTicket(String host, String port, String sid, String tag, Date normalizedHashTime) { String params = "host=" + host + "&port=" + port + "&sid=" + sid + "&tag=" + tag; try { Mac mac = Mac.getInstance("HmacSHA1"); - long ts = (new Date()).getTime(); + long ts = normalizedHashTime.getTime(); ts = ts/60000; // round up to 1 minute - String secretKey = "cloud.com"; + String secretKey = _ms.getHashKey(); SecretKeySpec keySpec = new SecretKeySpec(secretKey.getBytes(), "HmacSHA1"); mac.init(keySpec);