From 004242ccc6a1479bbd9ecc41e6d2c4b3ad9fb2b5 Mon Sep 17 00:00:00 2001 From: Rajani Karuturi Date: Wed, 16 Sep 2015 12:34:49 +0530 Subject: [PATCH 1/3] removed commented code --- .../cloud/server/ConfigurationServerImpl.java | 24 ------------------- 1 file changed, 24 deletions(-) diff --git a/server/src/com/cloud/server/ConfigurationServerImpl.java b/server/src/com/cloud/server/ConfigurationServerImpl.java index 5ff548c7f3e..926d9a747d4 100644 --- a/server/src/com/cloud/server/ConfigurationServerImpl.java +++ b/server/src/com/cloud/server/ConfigurationServerImpl.java @@ -416,30 +416,6 @@ public class ConfigurationServerImpl extends ManagerBase implements Configuratio }); } - /* - private void updateUuids() { - _identityDao.initializeDefaultUuid("disk_offering"); - _identityDao.initializeDefaultUuid("network_offerings"); - _identityDao.initializeDefaultUuid("vm_template"); - _identityDao.initializeDefaultUuid("user"); - _identityDao.initializeDefaultUuid("domain"); - _identityDao.initializeDefaultUuid("account"); - _identityDao.initializeDefaultUuid("guest_os"); - _identityDao.initializeDefaultUuid("guest_os_category"); - _identityDao.initializeDefaultUuid("hypervisor_capabilities"); - _identityDao.initializeDefaultUuid("snapshot_policy"); - _identityDao.initializeDefaultUuid("security_group"); - _identityDao.initializeDefaultUuid("security_group_rule"); - _identityDao.initializeDefaultUuid("physical_network"); - _identityDao.initializeDefaultUuid("physical_network_traffic_types"); - _identityDao.initializeDefaultUuid("physical_network_service_providers"); - _identityDao.initializeDefaultUuid("virtual_router_providers"); - _identityDao.initializeDefaultUuid("networks"); - _identityDao.initializeDefaultUuid("user_ip_address"); - _identityDao.initializeDefaultUuid("counter"); - } - */ - private String getMountParent() { return getEnvironmentProperty("mount.parent"); } From 97a5d6bd20da8cd4a03b4ea038c1664134f812cc Mon Sep 17 00:00:00 2001 From: Rajani Karuturi Date: Wed, 16 Sep 2015 12:28:03 +0530 Subject: [PATCH 2/3] CLOUDSTACK-8868: use same method to generate passwords for system/guest vms generateRandomPassword() is supposed to create root user passwords. Right now it is only used on the guest VMs. The format of the passwords it creates are of the form "random 3-character string with a lowercase character, uppercase character, and a digit" + random n-character string with only lowercase characters". For whatever reason it was that we use generateRandomPassword() for guest VM root user passwords(maybe more secure?) we should use the same function for system VM root user passwords. --- .../cloud/server/ConfigurationServerImpl.java | 4 +- .../server/ConfigurationServerImplTest.java | 78 +++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/server/src/com/cloud/server/ConfigurationServerImpl.java b/server/src/com/cloud/server/ConfigurationServerImpl.java index 926d9a747d4..16e4ff05902 100644 --- a/server/src/com/cloud/server/ConfigurationServerImpl.java +++ b/server/src/com/cloud/server/ConfigurationServerImpl.java @@ -155,6 +155,8 @@ public class ConfigurationServerImpl extends ManagerBase implements Configuratio protected ConfigDepot _configDepot; @Inject protected ConfigurationManager _configMgr; + @Inject + protected ManagementService _mgrService; public ConfigurationServerImpl() { @@ -668,7 +670,7 @@ public class ConfigurationServerImpl extends ManagerBase implements Configuratio if (already == null) { TransactionLegacy txn = TransactionLegacy.currentTxn(); try { - String rpassword = PasswordGenerator.generatePresharedKey(8); + String rpassword = _mgrService.generateRandomPassword(); String wSql = "INSERT INTO `cloud`.`configuration` (category, instance, component, name, value, description) " + "VALUES ('Secure','DEFAULT', 'management-server','system.vm.password', ?,'randmon password generated each management server starts for system vm')"; PreparedStatement stmt = txn.prepareAutoCloseStatement(wSql); diff --git a/server/test/com/cloud/server/ConfigurationServerImplTest.java b/server/test/com/cloud/server/ConfigurationServerImplTest.java index 38dc1bc0e09..b64f3f72aa2 100644 --- a/server/test/com/cloud/server/ConfigurationServerImplTest.java +++ b/server/test/com/cloud/server/ConfigurationServerImplTest.java @@ -19,14 +19,74 @@ package com.cloud.server; import java.io.File; import java.io.IOException; +import com.cloud.configuration.ConfigurationManager; +import com.cloud.configuration.dao.ResourceCountDao; +import com.cloud.dc.dao.DataCenterDao; +import com.cloud.dc.dao.HostPodDao; +import com.cloud.dc.dao.VlanDao; +import com.cloud.domain.dao.DomainDao; +import com.cloud.network.dao.NetworkDao; +import com.cloud.offerings.dao.NetworkOfferingDao; +import com.cloud.offerings.dao.NetworkOfferingServiceMapDao; +import com.cloud.service.dao.ServiceOfferingDao; +import com.cloud.storage.dao.DiskOfferingDao; +import com.cloud.user.dao.AccountDao; +import com.cloud.utils.db.TransactionLegacy; +import org.apache.cloudstack.framework.config.ConfigDepot; +import org.apache.cloudstack.framework.config.ConfigDepotAdmin; +import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.commons.codec.binary.Base64; import org.apache.commons.io.FileUtils; import org.junit.Assert; import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Mockito; import org.mockito.Spy; +import org.mockito.runners.MockitoJUnitRunner; +@RunWith(MockitoJUnitRunner.class) public class ConfigurationServerImplTest { + @Mock + private ConfigurationDao _configDao; + @Mock + private DataCenterDao _zoneDao; + @Mock + private HostPodDao _podDao; + @Mock + private DiskOfferingDao _diskOfferingDao; + @Mock + private ServiceOfferingDao _serviceOfferingDao; + @Mock + private NetworkOfferingDao _networkOfferingDao; + @Mock + private DataCenterDao _dataCenterDao; + @Mock + private NetworkDao _networkDao; + @Mock + private VlanDao _vlanDao; + @Mock + private DomainDao _domainDao; + @Mock + private AccountDao _accountDao; + @Mock + private ResourceCountDao _resourceCountDao; + @Mock + private NetworkOfferingServiceMapDao _ntwkOfferingServiceMapDao; + @Mock + private ConfigDepotAdmin _configDepotAdmin; + @Mock + private ConfigDepot _configDepot; + @Mock + private ConfigurationManager _configMgr; + @Mock + private ManagementService _mgrService; + + @InjectMocks + private ConfigurationServerImpl configurationServer; + @Spy ConfigurationServerImpl windowsImpl = new ConfigurationServerImpl() { protected boolean isOnWindows() { @@ -84,4 +144,22 @@ public class ConfigurationServerImplTest { Assert.assertFalse(linuxImpl.isOnWindows()); Assert.assertEquals("scripts/vm/systemvm/injectkeys.sh", linuxImpl.getInjectScript()); } + + @Test + public void testUpdateSystemvmPassword() { + //setup + String realusername = System.getProperty("user.name"); + System.setProperty("user.name", "cloud"); + Mockito.when(_configDao.getValue("system.vm.random.password")).thenReturn(String.valueOf(true)); + TransactionLegacy.open("cloud"); + Mockito.when(_mgrService.generateRandomPassword()).thenReturn("randomPassword"); + + //call the method to test + configurationServer.updateSystemvmPassword(); + + //verify that generateRandomPassword() is called + Mockito.verify(_mgrService, Mockito.times(1)).generateRandomPassword(); + //teardown + System.setProperty("user.name", realusername); + } } From f53188192fb51fe8f7bb56bf71b43fc5c283f2ba Mon Sep 17 00:00:00 2001 From: Rajani Karuturi Date: Mon, 21 Sep 2015 17:11:52 +0530 Subject: [PATCH 3/3] CLOUDSTACK-8868: change the default vm.password.length to 10 also moved it to ConfigDepot --- server/src/com/cloud/configuration/Config.java | 8 -------- .../configuration/ConfigurationManagerImpl.java | 2 +- .../com/cloud/server/ManagementServerImpl.java | 17 +++++++++++++++-- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/server/src/com/cloud/configuration/Config.java b/server/src/com/cloud/configuration/Config.java index 182ec50e300..25d84374ac6 100644 --- a/server/src/com/cloud/configuration/Config.java +++ b/server/src/com/cloud/configuration/Config.java @@ -899,14 +899,6 @@ public enum Config { "0", "Default disk I/O read rate in requests per second allowed in User vm's disk.", null), - VmPasswordLength( - "Advanced", - ManagementServer.class, - Integer.class, - "vm.password.length", - "6", - "Specifies the length of a randomly generated password", - null), VmDiskThrottlingIopsWriteRate( "Advanced", ManagementServer.class, diff --git a/server/src/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/com/cloud/configuration/ConfigurationManagerImpl.java index 69e70e6cd9e..74af277dfbd 100644 --- a/server/src/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/com/cloud/configuration/ConfigurationManagerImpl.java @@ -770,7 +770,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati throw new InvalidParameterValueException("Please enter a positive value for the configuration parameter:" + name); } //TODO - better validation for all password pamameters - if ("vm.password.length".equalsIgnoreCase(name) && val < 6) { + if ("vm.password.length".equalsIgnoreCase(name) && val < 10) { throw new InvalidParameterValueException("Please enter a value greater than 6 for the configuration parameter:" + name); } } catch (final NumberFormatException e) { diff --git a/server/src/com/cloud/server/ManagementServerImpl.java b/server/src/com/cloud/server/ManagementServerImpl.java index 25071a2b28f..ee6a42dd8db 100644 --- a/server/src/com/cloud/server/ManagementServerImpl.java +++ b/server/src/com/cloud/server/ManagementServerImpl.java @@ -504,6 +504,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator; import org.apache.cloudstack.framework.config.ConfigDepot; import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.config.Configurable; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.framework.config.impl.ConfigurationVO; import org.apache.cloudstack.framework.security.keystore.KeystoreManager; @@ -671,9 +672,11 @@ import com.cloud.vm.dao.SecondaryStorageVmDao; import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.VMInstanceDao; -public class ManagementServerImpl extends ManagerBase implements ManagementServer { +public class ManagementServerImpl extends ManagerBase implements ManagementServer, Configurable { public static final Logger s_logger = Logger.getLogger(ManagementServerImpl.class.getName()); + static final ConfigKey vmPasswordLength = new ConfigKey("Advanced", Integer.class, "vm.password.length", "10", + "Specifies the length of a randomly generated password", false); @Inject public AccountManager _accountMgr; @Inject @@ -905,7 +908,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe @Override public String generateRandomPassword() { - final Integer passwordLength = Integer.parseInt(_configDao.getValue("vm.password.length")); + final Integer passwordLength = vmPasswordLength.value(); return PasswordGenerator.generateRandomPassword(passwordLength); } @@ -3021,6 +3024,16 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe return cmdList; } + @Override + public String getConfigComponentName() { + return ManagementServer.class.getSimpleName(); + } + + @Override + public ConfigKey[] getConfigKeys() { + return new ConfigKey[] {vmPasswordLength}; + } + protected class EventPurgeTask extends ManagedContextRunnable { @Override protected void runInContext() {